refactor(apigateway): upstream authentication and transport policy move to a shared seam under internal/, so a second HTTP-based connection kind reuses them instead of copying them (#1647)

CLAUDE.md

@@ -236,7 +236,9 @@ mcp-data-platform/
 │   ├── agentinstructions/          # The deployment's customized agent-instruction layer as a policy rather than a config value (#1607): the byte bound and size advisory both its writers enforce, the config-store adapter it is read and written through, and the `mcp:knowledge_page:<slug>` index-entry form BOTH instruction layers point at a page with
 │   ├── admin/                      # Admin-API seams built only by pkg/admin: auditapi/ (events + metrics), callapi/ (the call catalog + its review actions), catalogapi/ (OpenAPI spec bundles + embedding jobs), connoauthapi/ (connection OAuth, unified + legacy per-kind), notifyapi/ (notification delivery history + status counts), settingsapi/ (SMTP + review-queue-alert settings REST) — extracted by #1078
 │   ├── apigwmetrics/               # The api gateway's outbound HTTP instrumentation as an http.RoundTripper: the connection/status labels, and the persona the call was authorized under, read off the request context the tool call carries (#1615). Holds no gateway types, and was extracted when pkg/toolkits/apigateway reached its package-size budget
-│   ├── apigwtls/                   # The api gateway's TLS material: what a connection's mTLS keypair and CA bundle must satisfy, and the *tls.Config its outbound transport is built with. Knows nothing about API connections — it takes the four values one carries — and was extracted when pkg/toolkits/apigateway reached its package-size budget (#1626)
+│   ├── apigwtls/                   # The TLS material an HTTP upstream connection carries: what its mTLS keypair and CA bundle must satisfy, and the *tls.Config its outbound transport is built with. Knows nothing about API connections — it takes the values one carries, including the kind name its refusals speak in — and is reached through internal/upstreamauth (#1626, #1647)
+│   ├── upstreamauth/               # What every HTTP-based connection kind does to reach its upstream, in one copy: the Authenticator and its modes (none, bearer, api_key, basic, oauth, mtls), the operator-owned static headers and the header names a model may not claim, the connect/call timeouts and response read cap, and the TLS material. Owns the config keys those rules belong to and validates them; error text is the caller's, through Config.ErrPrefix. Extracted from pkg/toolkits/apigateway so a second kind reuses it instead of forking it (#1647)
+│   ├── cfgmap/                     # The typed readers over the map[string]any a connection is stored as: the tolerant String/Duration/Int64/Bool/StringMap that absorb what a JSON round-trip did to an operator's value, in one place so two kinds cannot disagree about what `"call_timeout": 30` means (#1647)
 │   ├── httpjson/                   # RFC 9457 Problem Details responder + admin list-query param parsing, shared by the admin/portal decomposition seams (#1078)
 │   ├── httpserver/                 # HTTP composition root: mux/route assembly (MCP streamable+SSE, OAuth, admin/portal/resources/gateway/observability REST, portal UI), CORS, drain/shutdown sequencing — extracted from main.go (#895). Subpackages are the adapters it mounts: accessgate/, attachhttp/, datahubapi/, gatewayhttp/, health/, httpauth/, mentionhttp/, notifyhttp/ (self-scoped notification prefs), scripthttp/ (managed-script admin + portal routes, including the administrator's owner transfer), sources/, unsubhttp/ (no-login unsubscribe + its tokens), versionhttp/ (#1076, #1080)
 │   ├── sqlgate/                    # Collects the module's SQL and hands each statement to a real PostgreSQL to parse and plan (#1512); integration-tagged, so it is absent from the default build

docs/library/stability.md

@@ -68,6 +68,22 @@ aliased back in `pkg/portal`, so `portal.Asset`, `portal.Collection`,
 were never a supported integration surface; the location now enforces that so
 their evolution cannot break an external build.
 
+What a connection kind does to reach an HTTP upstream is one of these seams:
+`internal/upstreamauth` holds the outbound authenticator and every auth mode
+(`none`, `bearer`, `api_key`, `basic`, `oauth`, `mtls`), the operator-owned
+static headers and the header names a model may not claim, the connect and call
+timeouts, the response read cap, and the TLS material the handshake presents.
+It is shared rather than copied because a second HTTP-based connection kind
+answers the same questions, and one copy is what keeps a fix to, say, the
+token-fetch error scrubber from landing in one kind and not the other. The
+generic readers that pull a typed value out of a stored connection's
+`map[string]any` sit beside it in `internal/cfgmap`. The API gateway's own
+names are aliased back, so `apigateway.Authenticator`,
+`apigateway.NewAuthenticator`, `apigateway.ErrNeedsReauth` and the
+`AuthMode*`, `CredentialPlacement*` and `OAuth2AuthStyle*` constants are
+spelled exactly as before, and every configuration key and error message an
+operator sees is unchanged.
+
 If you were importing one of these while it still lived under `pkg/`, the
 package moved but its API did not: the type and function names are unchanged,
 and the functionality is reachable through the supported surface, which

docs/llms-full.txt

@@ -2196,7 +2196,7 @@ Supported import surface (breaking changes only in a major release):
 | `pkg/middleware` | Request/response middleware contracts |
 | `pkg/toolkits/*` | Toolkit adapters' exported config types |
 
-Other exported packages under `pkg/` are importable but are implementation packages, not a committed integration surface; their API may change in a minor release with a release-note callout. The set is bounded by a build gate (`TestPublicSurfacePolicy`, `pkg_stability_policy_test.go`) that fails when a package is added under `pkg/` outside the supported table with a single first-party importer, since that shape is an implementation seam and belongs under `internal/`; the remaining exemptions are the reference store and provider implementations a consumer passes to `platform.WithSessionStore`/`WithQueryProvider`/`WithStorageProvider` and their siblings, plus `pkg/admin` (a mountable router) and `pkg/database/migrate` (the embedded schema migrations). Facade-internal seams live under `internal/platform/`, the HTTP adapters the server mounts under `internal/httpserver/`, and the portal's own seams under `internal/portal/` (its domain types and store contracts, PostgreSQL and no-database stores, authorization core, feedback surface, public-viewer templates, rate limiter and share cache — every moved name aliased back so `portal.Asset`, `portal.Collection`, `portal.User` and the store constructors are spelled as before); all are unimportable from outside the module by Go's internal rule and were never a supported surface.
+Other exported packages under `pkg/` are importable but are implementation packages, not a committed integration surface; their API may change in a minor release with a release-note callout. The set is bounded by a build gate (`TestPublicSurfacePolicy`, `pkg_stability_policy_test.go`) that fails when a package is added under `pkg/` outside the supported table with a single first-party importer, since that shape is an implementation seam and belongs under `internal/`; the remaining exemptions are the reference store and provider implementations a consumer passes to `platform.WithSessionStore`/`WithQueryProvider`/`WithStorageProvider` and their siblings, plus `pkg/admin` (a mountable router) and `pkg/database/migrate` (the embedded schema migrations). Facade-internal seams live under `internal/platform/`, the HTTP adapters the server mounts under `internal/httpserver/`, and the portal's own seams under `internal/portal/` (its domain types and store contracts, PostgreSQL and no-database stores, authorization core, feedback surface, public-viewer templates, rate limiter and share cache — every moved name aliased back so `portal.Asset`, `portal.Collection`, `portal.User` and the store constructors are spelled as before); all are unimportable from outside the module by Go's internal rule and were never a supported surface. What a connection kind does to reach an HTTP upstream is one of these seams too: `internal/upstreamauth` holds the outbound authenticator and every auth mode (`none`, `bearer`, `api_key`, `basic`, `oauth`, `mtls`), the operator-owned static headers and the header names a model may not claim, the connect/call timeouts, the response read cap and the TLS material, shared by the HTTP-based kinds rather than copied into each; `internal/cfgmap` holds the typed readers over a stored connection's `map[string]any`. `apigateway.Authenticator`, `apigateway.NewAuthenticator`, `apigateway.ErrNeedsReauth` and the `AuthMode*`, `CredentialPlacement*` and `OAuth2AuthStyle*` constants are aliased back, so the toolkit's API, its configuration keys and its error messages are unchanged.
 
 Configuration-file compatibility is handled more conservatively: additive keys ship in minor releases, and breaking renames or removals are called out in that version's release notes with an admonition (old key, new key, runtime effect). Precedent: `workflow.require_search` replaced the former `workflow.require_discovery_before_query` as a hard, non-aliased rename documented in the release notes. Set `config.strict: true` to turn an unaccounted-for rename into a hard startup error instead of a silent no-op.
 

internal/apigwtls/apigwtls.go

@@ -3,11 +3,12 @@
 // transport is built with.
 //
 // It is a seam of pkg/toolkits/apigateway, extracted when that package reached
-// its size budget. Nothing here knows what an API connection is -- it takes
-// the four values one carries -- which is why X.509 parsing, key-strength
+// its size budget, and is now reached through internal/upstreamauth by every
+// HTTP-based connection kind. Nothing here knows what an API connection is --
+// it takes the values one carries -- which is why X.509 parsing, key-strength
 // policy and PEM handling sit together rather than beside operation discovery.
-// The messages keep their "apigateway:" prefix: an operator sees them when a
-// connection is refused, and the prefix names that subsystem.
+// Material.ErrPrefix names the kind in every message, because an operator
+// reading a refused connection save should see the surface they configured.
 package apigwtls
 
 import (
@@ -35,11 +36,32 @@ const minRSABits = 2048
 // presents, the extra CA bundle it trusts, and whether that keypair is the
 // connection's credential (auth_mode=mtls) rather than an addition to it. The
 // caller resolves ClientPairRequired from the auth mode.
+//
+// ErrPrefix names the connection kind in the messages Validate and Build
+// produce. Empty falls back to this package's own name, which no caller should
+// let an operator see: pass the kind through.
 type Material struct {
 	ClientCertPEM      string
 	ClientKeyPEM       string
 	CABundlePEM        string
 	ClientPairRequired bool
+	ErrPrefix          string
+}
+
+// prefix returns the caller-supplied error prefix, or this package's name when
+// a caller left it unset.
+func (m Material) prefix() string {
+	if m.ErrPrefix == "" {
+		return "apigwtls"
+	}
+	return m.ErrPrefix
+}
+
+// errf builds an error in the calling kind's voice. The prefix is joined to
+// the format string rather than passed as an argument so the literal a reader
+// (and the string-format linter) sees is the message itself.
+func errf(prefix, format string, a ...any) error {
+	return fmt.Errorf(prefix+": "+format, a...) //nolint:err113,perfsprint // one formatting helper for the whole package
 }
 
 // Validate enforces the mTLS and CA-trust rules. Three independent checks:
@@ -59,21 +81,22 @@ type Material struct {
 //     certificate when set. Empty string means "no extra CAs", which is
 //     the existing default.
 func Validate(m Material) error {
+	prefix := m.prefix()
 	if m.ClientPairRequired {
 		if m.ClientCertPEM == "" || m.ClientKeyPEM == "" {
-			return errors.New("apigateway: mtls_client_cert_pem and mtls_client_key_pem are required when auth_mode is \"mtls\"")
+			return errf(prefix, "mtls_client_cert_pem and mtls_client_key_pem are required when auth_mode is %q", "mtls")
 		}
 	}
 	if (m.ClientCertPEM == "") != (m.ClientKeyPEM == "") {
-		return errors.New("apigateway: mtls_client_cert_pem and mtls_client_key_pem must both be set or both be empty")
+		return errf(prefix, "mtls_client_cert_pem and mtls_client_key_pem must both be set or both be empty")
 	}
 	if m.ClientCertPEM != "" {
-		if err := validateClientKeyPair(m.ClientCertPEM, m.ClientKeyPEM); err != nil {
+		if err := validateClientKeyPair(prefix, m.ClientCertPEM, m.ClientKeyPEM); err != nil {
 			return err
 		}
 	}
 	if m.CABundlePEM != "" {
-		if err := validateCABundle(m.CABundlePEM); err != nil {
+		if err := validateCABundle(prefix, m.CABundlePEM); err != nil {
 			return err
 		}
 	}
@@ -85,19 +108,19 @@ func Validate(m Material) error {
 // x509.ParseCertificate, and the key-matches-cert signature check; the
 // extra leaf-cert parse here gives a clean place to enforce minimum
 // key strength without re-deriving the key from raw bytes.
-func validateClientKeyPair(certPEM, keyPEM string) error {
+func validateClientKeyPair(prefix, certPEM, keyPEM string) error {
 	pair, err := tls.X509KeyPair([]byte(certPEM), []byte(keyPEM))
 	if err != nil {
-		return fmt.Errorf("apigateway: mtls cert/key invalid: %s", sanitizeKeyPairError(err))
+		return errf(prefix, "mtls cert/key invalid: %s", sanitizeKeyPairError(err))
 	}
 	if len(pair.Certificate) == 0 {
-		return errors.New("apigateway: mtls_client_cert_pem contained no certificates")
+		return errf(prefix, "mtls_client_cert_pem contained no certificates")
 	}
 	leaf, err := x509.ParseCertificate(pair.Certificate[0])
 	if err != nil {
-		return fmt.Errorf("apigateway: mtls leaf certificate unreadable: %s", err.Error())
+		return errf(prefix, "mtls leaf certificate unreadable: %s", err.Error())
 	}
-	return checkKeyStrength(leaf.PublicKey)
+	return checkKeyStrength(prefix, leaf.PublicKey)
 }
 
 // sanitizeKeyPairError strips any PEM content from tls.X509KeyPair's
@@ -122,23 +145,23 @@ func sanitizeKeyPairError(err error) string {
 // is not interoperable with most peers and stronger curves are not
 // supported by Go's TLS stack as of this writing); Ed25519 is
 // always accepted. Unknown key algorithms are rejected loudly.
-func checkKeyStrength(pub any) error {
+func checkKeyStrength(prefix string, pub any) error {
 	switch k := pub.(type) {
 	case *rsa.PublicKey:
 		if k.N == nil || k.N.BitLen() < minRSABits {
-			return fmt.Errorf("apigateway: mtls private key RSA-%d is below the minimum %d bits", k.N.BitLen(), minRSABits)
+			return errf(prefix, "mtls private key RSA-%d is below the minimum %d bits", k.N.BitLen(), minRSABits)
 		}
 		return nil
 	case *ecdsa.PublicKey:
 		switch k.Curve {
 		case elliptic.P256(), elliptic.P384(), elliptic.P521():
 			return nil
 		}
-		return errors.New("apigateway: mtls private key uses an unsupported ECDSA curve (want P-256, P-384, or P-521)")
+		return errf(prefix, "mtls private key uses an unsupported ECDSA curve (want P-256, P-384, or P-521)")
 	case ed25519.PublicKey:
 		return nil
 	default:
-		return fmt.Errorf("apigateway: mtls private key uses an unsupported algorithm %T", pub)
+		return errf(prefix, "mtls private key uses an unsupported algorithm %T", pub)
 	}
 }
 
@@ -147,7 +170,7 @@ func checkKeyStrength(pub any) error {
 // already filtered out the no-bundle case); a bundle with zero
 // CERTIFICATE blocks (e.g., one that contains only PRIVATE KEY blocks)
 // is rejected as misconfigured.
-func validateCABundle(bundle string) error {
+func validateCABundle(prefix, bundle string) error {
 	rest := []byte(bundle)
 	count := 0
 	for len(rest) > 0 {
@@ -160,12 +183,12 @@ func validateCABundle(bundle string) error {
 			continue
 		}
 		if _, err := x509.ParseCertificate(block.Bytes); err != nil {
-			return fmt.Errorf("apigateway: tls_ca_bundle_pem contains an unparseable certificate: %s", err.Error())
+			return errf(prefix, "tls_ca_bundle_pem contains an unparseable certificate: %s", err.Error())
 		}
 		count++
 	}
 	if count == 0 {
-		return errors.New("apigateway: tls_ca_bundle_pem must contain at least one CERTIFICATE block")
+		return errf(prefix, "tls_ca_bundle_pem must contain at least one CERTIFICATE block")
 	}
 	return nil
 }
@@ -189,18 +212,19 @@ func Build(m Material) (*tls.Config, error) {
 	if !hasClient && !hasCABundle {
 		return nil, nil //nolint:nilnil // nil config = use http.Transport defaults
 	}
+	prefix := m.prefix()
 	out := &tls.Config{MinVersion: tls.VersionTLS12}
 	if hasClient {
 		pair, err := tls.X509KeyPair([]byte(m.ClientCertPEM), []byte(m.ClientKeyPEM))
 		if err != nil {
-			return nil, fmt.Errorf("apigateway: building mtls keypair: %s", sanitizeKeyPairError(err))
+			return nil, errf(prefix, "building mtls keypair: %s", sanitizeKeyPairError(err))
 		}
 		out.Certificates = []tls.Certificate{pair}
 	}
 	if hasCABundle {
 		pool, err := RootPool(m.CABundlePEM)
 		if err != nil {
-			return nil, err
+			return nil, errf(prefix, "%w", err)
 		}
 		out.RootCAs = pool
 	}
@@ -221,7 +245,7 @@ func RootPool(bundle string) (*x509.CertPool, error) {
 		pool = x509.NewCertPool()
 	}
 	if ok := pool.AppendCertsFromPEM([]byte(bundle)); !ok {
-		return nil, errors.New("apigateway: tls_ca_bundle_pem contained no valid certificates")
+		return nil, errors.New("tls_ca_bundle_pem contained no valid certificates")
 	}
 	return pool, nil
 }