feat(broker): per-user credential injection + per-(user,server) connection keying (spec 074 T7)

[Dumbris]

Summary

Spec 074 T7 (MCP-1040) — wires the per-user upstream credential resolved by T6's CredentialResolver into the outbound proxied request, and keys brokered connections per (user, server). Builds on the merged T6 (#688/#690).

Gate 3: open for review — I do not merge.

What it does

• Injection (FR-016/FR-017) — internal/transport: a new edition-neutral BrokeredAuth + EffectiveHeaders primitive injects the resolved credential into the configured header (default Authorization: Bearer {token}), dropping any same-named inbound/configured header (case-insensitive) first. The inbound gateway/IdP token is never forwarded. Applied in both the HTTP and SSE client builders.
• Connection layer — internal/upstream/core: Client carries an optional per-user BrokeredAuth; the headers-auth strategy (HTTP + SSE) injects it and runs even when the upstream has no static headers.
• Broker bridge (FR-002/FR-018) — internal/serveredition/broker: HeaderInjector turns CredentialResolver.Resolve(user, server) into a transport.BrokeredAuth, rejects stdio brokering as defense-in-depth, and ConnectionKey(user, server) binds a brokered connection to one (user, server) so one user's credential is never reused for another.
• Multiuser routing — internal/serveredition/multiuser: Router.BrokeredConnectionKey exposes the per-(user, server) pooling key for the brokered connection pool.
• Docs — docs/features/auth-broker.md: header-injection + per-(user, server) connection keying sections.

Tests (TDD)

• outbound carries the user-specific token (transport + injector)
• inbound token absent on the outbound request — asserted on the wire across both HTTP and SSE paths (httptest capture)
• two users → two distinct tokens and connection keys
• stdio brokering rejected (ErrBrokerStdioUnsupported)

Verification

• go build ./cmd/mcpproxy (personal) + go build -tags server ./cmd/mcpproxy (server) — both OK
• go test -race ./internal/transport/ ./internal/upstream/core/ — pass
• go test -tags server -race ./internal/serveredition/... ./internal/auth/... — pass
• golangci-lint v2.5.0 --config .github/.golangci.yml (incl. --build-tags server) — 0 issues
• gofmt -l — clean

Scope note

The workspace today is a config container (no live MCP connection pool yet). This PR delivers the injection + per-(user,server) keying contract (ConnectionKey / Router.BrokeredConnectionKey) the pool must use; wiring an actual per-user live-connection pool belongs to the REST/runtime layer (T8) that consumes this key.

Related #688

FR-016, FR-017, FR-018. SC-002, SC-003.

[cloudflare-workers-and-pages]

Deploying mcpproxy-docs with    Cloudflare Pages

Latest commit:	deb8008
Status:	✅  Deploy successful!
Preview URL:	https://9600327c.mcpproxy-docs.pages.dev
Branch Preview URL:	https://074-t7-injection-connection.mcpproxy-docs.pages.dev

View logs

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 70.96774% with 18 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
internal/upstream/core/connection_http.go	61.11%	14 Missing ⚠️
internal/transport/http.go	73.33%	3 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

[Dumbris]

Claude review → ACCEPT on the isolation logic (credential injection + (user,server) connection keying both verified, fail-closed at the broker boundary, -race green, on-the-wire HTTP+SSE tests prove A's token reaches upstream, inbound auth header dropped). One change requested before merge (per maintainer): close the fail-closed gap now rather than deferring it to the activating PR.

[FIX] Brokered connect must fail CLOSED — no fallback to no-auth/OAuth. The HTTP auth-strategy chain (connection_http.go:~14) runs headers→no-auth→OAuth. Today the injection primitives have no live callers, but once a caller sets brokeredAuth, a failure of tryHeadersAuth (e.g. a 403 on the brokered token) would fall through to no-auth or shared OAuth — defeating per-user isolation. Fix in THIS PR: when brokeredAuth is set on a connection, the only permitted auth strategy is the brokered headers; if it fails, refuse the connection (error) — never fall back to no-auth or config/OAuth. Add a test: a connection with brokeredAuth whose headers-auth fails does NOT connect unauthenticated and does NOT use the shared OAuth path.

[github-actions]

📦 Build Artifacts

Workflow Run: View Run
Branch: 074-t7-injection-connection-keying

Available Artifacts

• archive-darwin-amd64 (28 MB)
• archive-darwin-arm64 (25 MB)
• archive-linux-amd64 (16 MB)
• archive-linux-arm64 (14 MB)
• archive-windows-amd64 (28 MB)
• archive-windows-arm64 (25 MB)
• frontend-dist-pr (0 MB)
• installer-dmg-darwin-amd64 (21 MB)
• installer-dmg-darwin-arm64 (19 MB)

How to Download

Option 1: GitHub Web UI (easiest)

1. Go to the workflow run page linked above
2. Scroll to the bottom "Artifacts" section
3. Click on the artifact you want to download

Option 2: GitHub CLI

gh run download 27570086880 --repo smart-mcp-proxy/mcpproxy-go

Note: Artifacts expire in 14 days.

[Dumbris]

CEO Code Review: PR #691 — spec 074 T7, per-user credential injection + connection keying

Verdict: ACCEPT (CodexReviewer was unavailable; CEO performed review directly)

What I checked

• Edition-neutral transport layer (BrokeredAuth, EffectiveHeaders) — clean, no server-edition imports bleed into transport
• HeaderInjector bridge — correct guards for anon user, stdio, unconfigured broker
• ConnectionKey — uses \x00 separator + oauth.GenerateServerKey (matches credential store keying; stable, distinct per user and per server)
• canUseHeadersStrategy — correctly enables headers-auth when only brokered auth is set (no static headers needed, FR-016)
• EffectiveHeaders — fresh copy returned (no server config mutation risk), case-insensitive inbound header drop
• Wire-level FR-017 asserted via httptest capture on both HTTP and SSE paths
• Two-user token/key isolation tests (FR-018)
• Stdio guard: double-gated (config validation + runtime ErrBrokerStdioUnsupported)

Minor observation (not blocking)

Client.brokeredAuth has no mutex around SetBrokeredAuth. Acceptable: the live connection pool belongs to T8 and SetBrokeredAuth precedes connection hand-off. T8 should establish the synchronisation contract.

All spec requirements verified

FR	Verified
FR-016	Resolved credential injected into configured header ✓
FR-017	Inbound token never forwarded; wire-tested HTTP + SSE ✓
FR-018	Per-(user,server) key; two users never share connection ✓
FR-002	stdio brokering rejected with clear error ✓
FR-014	Anonymous caller rejected before any resolution ✓

go build, go test -race, golangci-lint all pass (per PR description).

Gate 3 satisfied — ready to merge.

[Dumbris]

Fail-closed fix applied — deb80087

Addressed the maintainer's security-critical fix-before-merge: a brokered (per-user) connection no longer falls back to no-auth or shared OAuth.

Change (internal/upstream/core/connection_http.go): extracted httpAuthStrategies() / sseAuthStrategies(). When brokeredAuth != nil, the returned strategy list is [headers] only — a failure of the per-user headers auth (e.g. 403 on the user's token) ends the loop and refuses the connection (all authentication strategies failed), never attempting no-auth or OAuth. Non-brokered connections keep the historical headers → no-auth → OAuth chain byte-for-byte.

Test TestClient_BrokeredConnection_FailsClosed_OnlyHeadersStrategy: asserts the brokered strategy list is [headers] (no no-auth/OAuth) for both HTTP and SSE, and that non-brokered keeps the full chain.

Acceptance

• Brokered + headers-auth failure → connection refused, no no-auth, no shared OAuth ✅
• Non-brokered chain unchanged ✅
• go test -tags server ./internal/upstream/core/ ./internal/transport/ ./internal/serveredition/broker/ -race green ✅
• Both editions build ✅ · golangci-lint v2.5.0 (server tag) 0 issues ✅

Pushed to the same branch 074-t7-injection-connection-keying; not merging — back to re-review + human Gate-3 sign-off.

[Dumbris]

ACCEPT — spec 074 T7 (MCP-1040). Reviewed at head deb80087; independently re-verified, all green.

Verification (clean worktree, not trusting the PR description)

• go build ./cmd/mcpproxy (personal) + go build -tags server ./cmd/mcpproxy — both OK
• go test -race ./internal/transport/ ./internal/upstream/core/ — pass
• go test -tags server -race ./internal/serveredition/... ./internal/auth/... — pass
• golangci-lint --config .github/.golangci.yml --build-tags server on changed pkgs — 0 issues

Security review — the invariants that matter here all hold

• Fail-closed brokered auth (connection_http.go): when brokeredAuth != nil the strategy list collapses to {headers} only — no fallback to no-auth or shared OAuth. A brokered connection that can't inject the per-user token fails rather than silently connecting with the wrong identity. This is the right call and is the security crux of the change.
• Inbound token never forwarded (EffectiveHeaders): same-named header is dropped case-insensitively (strings.EqualFold) before the resolved credential is set; the map is always a fresh copy so the config's header map is never mutated. The integration test proves this on the wire via an httptest capture across both HTTP and SSE paths (inbound Bearer INBOUND-GATEWAY → outbound carries only the per-user token).
• Per-(user,server) keying (ConnectionKey): userID + "\x00" + oauth.GenerateServerKey(name,url) — NUL separator avoids cross-field collisions and reuses the store's keying scheme, so one user's credential/connection can't be reused for another.
• Injection boundary checks (HeaderInjector.InjectFor): empty userID → ErrUnauthenticated (FR-014); stdio rejected as defense-in-depth (FR-002); nil/empty token rejected.
• No import cycle: broker → transport/oauth/config; multiuser/router → broker. transport stays edition-neutral (receives BrokeredAuth as plain data).

Notes (non-blocking)

• EffectiveHeaders assumes a non-empty brokered.Header; the injector always supplies one (fallback Authorization), so this is safe in practice. Fine as-is.
• Scope note is accurate: this lands the injection + keying contract; the live per-user pool consuming Router.BrokeredConnectionKey is T8.

No blocking findings. Clean to merge per the gate.