From bd614c3a1a2bad91d6e53a52c71c12648e079c93 Mon Sep 17 00:00:00 2001 From: Sheng Kun Chang Date: Sun, 5 Jul 2026 04:40:10 +0800 Subject: [PATCH 1/2] fix(sdk): promote validated forwarder identity on Internal-scope requests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On the dev/tunnel plane, dispatch strip-then-injects the X-MS-User-ID / X-MS-App-ID / X-MS-App-Role identity headers on every Internal-scope forward, but internalAuth only validated the platform secret and never promoted those headers into ctx. Any Internal handler reading ms.AppID(r.Context()) therefore got "" when tunnel-served and 401'd: ms-app-modules#30 fixed oauth-google's platformCallback to read ctx identity (correct for the deployed path) leaving it latently broken over the tunnel, and the new cross-read route (ms-app-modules#31) hit the same wall, blocking the dev-to-dev and dev-to-deployed E2Es. Fix at the shared seam: after internalAuth's constant-time secret check succeeds, promote the forwarder-injected identity triple into ctx via promoteForwarderIdentity — the exact mechanism extracted from requireProxy's success path, so both HTTP header-ingestion paths share one helper. The validated secret is the same trust signal requireProxy relies on: identity headers on this path can only come from the platform wire (dispatch / dev tunnel proxy). Fail-closed properties preserved: - promotion fires ONLY on the secret-validated success path — never on the local no-secret bypass (unauthenticated transport), never on the 401/503 rejection branches - an identity already on ctx wins (deployed path: PayloadTrusted early-returns before promotion, so the envelope identity set by runtime.InjectResources stays authoritative) - empty headers never mint an identity (same rule InjectResources applies to an all-empty envelope) Covers every internal surface through the single cached m.internalAuth closure: Module.Internal() routes plus the system event/cron/task mounts. Note: PR #137 (feat/ctx-identity-accessors) touches auth/middleware.go doc comments and adds ms.UserID/ms.AppRole accessors; this branch is cut from origin/main with edits kept minimal so a rebase across #137 (in either landing order) stays trivial. Co-Authored-By: Claude Fable 5 --- auth/middleware.go | 77 +++++++++-- auth/middleware_test.go | 175 ++++++++++++++++++++++++ internal/core/internal_identity_test.go | 65 +++++++++ 3 files changed, 303 insertions(+), 14 deletions(-) create mode 100644 internal/core/internal_identity_test.go diff --git a/auth/middleware.go b/auth/middleware.go index eab8e9b..737e843 100644 --- a/auth/middleware.go +++ b/auth/middleware.go @@ -1,6 +1,7 @@ package auth import ( + "context" "crypto/subtle" "log" "net/http" @@ -171,6 +172,15 @@ func InternalAuth() func(http.Handler) http.Handler { // while still preventing tunnel mode from accidentally accepting // unauthenticated traffic forwarded by dispatch (the CLI sets the secret // when tunneling). +// +// On the secret-validated success path, the forwarder-injected X-MS-User-ID / +// X-MS-App-ID / X-MS-App-Role headers are promoted to auth.Identity (see +// promoteForwarderIdentity) — the same trust signal RequireProxy uses on the +// public surface, and the same identity triple InjectResources sets on the +// deployed path. Without this, an Internal handler served over the dev tunnel +// reads ms.AppID(ctx) == "" even though dispatch injected the headers. +// Promotion NEVER happens on the local bypass (unauthenticated transport) or +// on any rejection branch. func internalAuth(inLambda bool) func(http.Handler) http.Handler { readSecret := platformSecretReader() if _, _, configured := readSecret(); !configured { @@ -217,7 +227,20 @@ func internalAuth(inLambda bool) func(http.Handler) http.Handler { httputil.JSON(w, http.StatusUnauthorized, httputil.ErrorResponse{Error: "internal authentication required"}) return } - next.ServeHTTP(w, r) + + // SUCCESS PATH ONLY. The secret validated, which proves the request + // came off the platform wire (dispatch / dev tunnel proxy) — the + // same trust signal RequireProxy relies on — so the X-MS-* identity + // headers it injected are now trusted. Promote them so an Internal + // handler can read ms.AppID(ctx) on the dev/tunnel plane just like + // on the deployed plane (where InjectResources populates ctx from + // the typed envelope and the PayloadTrusted early-return above + // keeps that identity authoritative). + // + // CRITICAL ORDERING: this MUST stay below the constant-time check — + // promoting on the local bypass or any rejection branch would trust + // spoofable headers from an unauthenticated caller. + next.ServeHTTP(w, r.WithContext(promoteForwarderIdentity(r))) }) } } @@ -330,19 +353,7 @@ func requireProxy(inLambda bool) func(http.Handler) http.Handler { // // CRITICAL ORDERING: this MUST run only after the token check above // passes. Promoting before validation would trust spoofable headers. - // - // Don't clobber an identity already on the context: if something - // upstream (e.g. PlatformAuth on the platform surface, or a future - // authorizer) already set one, that wins. - ctx := r.Context() - if Get(ctx) == nil { - ctx = Set(ctx, Identity{ - UserID: r.Header.Get(HeaderUserID), - AppID: r.Header.Get(HeaderAppID), - AppRole: r.Header.Get(HeaderAppRole), - }) - } - next.ServeHTTP(w, r.WithContext(ctx)) + next.ServeHTTP(w, r.WithContext(promoteForwarderIdentity(r))) }) } } @@ -358,6 +369,44 @@ func rejectNotProxied(w http.ResponseWriter) { }) } +// promoteForwarderIdentity returns r's context with the forwarder-injected +// X-MS-User-ID / X-MS-App-ID / X-MS-App-Role headers promoted to +// auth.Identity. It is the single header-ingestion mechanism shared by the +// two secret-validated HTTP paths (requireProxy and internalAuth) and MUST +// only be called on a success path, AFTER the platform token / internal +// secret passed its constant-time check — the validated secret is the proof +// that the headers came from dispatch, not a direct caller. +// +// Two guards, mirroring the deployed path's precedents: +// +// - Don't clobber an identity already on the context (Lambda's +// InjectResources — or an upstream middleware — set it from a source at +// least as trusted as these headers; requireProxy pins this via +// TestRequireProxy_PresetIdentity_NotClobbered). +// - Empty headers never mint an identity (same rule InjectResources +// applies to an all-empty envelope): a headerless-but-authenticated +// request leaves auth.Get(ctx) == nil rather than planting an +// all-empty Identity. +// +// Unlike platformAuth step 4 there is no all-three-required rejection here: +// internal system calls legitimately carry no user (events, crons), and on +// this plane dispatch always injects app id + app role. +func promoteForwarderIdentity(r *http.Request) context.Context { + ctx := r.Context() + if Get(ctx) != nil { + return ctx + } + id := Identity{ + UserID: r.Header.Get(HeaderUserID), + AppID: r.Header.Get(HeaderAppID), + AppRole: r.Header.Get(HeaderAppRole), + } + if id == (Identity{}) { + return ctx + } + return Set(ctx, id) +} + // secretReader returns the current secret, which header carries it, and // whether a secret SOURCE is configured at all. The file-backed variant // re-reads on every call so a refreshed token (CLI reconnect) is picked up diff --git a/auth/middleware_test.go b/auth/middleware_test.go index 195fc05..30b888e 100644 --- a/auth/middleware_test.go +++ b/auth/middleware_test.go @@ -827,3 +827,178 @@ func TestRequireProxy_PresetIdentity_NotClobbered(t *testing.T) { t.Errorf("preset identity must win over headers; got %+v", seen) } } + +// --- InternalAuth identity promotion (Internal-scope trusted forwarder identity) --- + +func TestInternalAuth_ValidSecret_PromotesForwarderIdentity(t *testing.T) { + // REGRESSION PIN for the tunnel-Internal 401: on the dev/tunnel plane, + // dispatch strips-then-injects X-MS-User-ID / X-MS-App-ID / X-MS-App-Role + // on Internal-scope forwards, but internalAuth only validated the secret + // and never promoted those headers into ctx — so any Internal handler + // reading ms.AppID(r.Context()) got "" and 401'd. Bit twice: + // ms-app-modules#30 (oauth-google platformCallback reads ctx identity — + // correct when deployed, latently broken when tunnel-served) and + // ms-app-modules#31 (cross-read route hit the same wall, blocking the + // dev-to-dev / dev-to-deployed E2Es). After the secret validates, the + // headers are dispatch-injected (strip-then-inject) and must be promoted + // — the same trust signal requireProxy promotes on. + t.Setenv("MS_INTERNAL_SECRET", "real-secret") + var seen *Identity + handler := internalAuth(false)(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + seen = Get(r.Context()) + })) + + req := httptest.NewRequest("POST", "/internal/crossread", nil) + req.Header.Set(HeaderInternalSecret, "real-secret") + req.Header.Set(HeaderUserID, "u-owner-1") + req.Header.Set(HeaderAppID, "a-int-2") + req.Header.Set(HeaderAppRole, RoleAdmin) + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("expected 200 for valid internal request, got %d", rec.Code) + } + if seen == nil { + t.Fatal("expected promoted Identity on the context; got nil") + } + if seen.AppID != "a-int-2" { + t.Errorf("AppID = %q, want a-int-2 (the trusted, dispatch-injected app id)", seen.AppID) + } + if seen.UserID != "u-owner-1" || seen.AppRole != RoleAdmin { + t.Errorf("identity mismatch: got %+v", seen) + } +} + +func TestInternalAuth_RejectedRequest_NeverPromotes(t *testing.T) { + // A request rejected by internalAuth (wrong secret) must NEVER reach the + // handler, so no identity is ever promoted from its (spoofable) headers. + t.Setenv("MS_INTERNAL_SECRET", "real-secret") + reached := false + handler := internalAuth(false)(http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) { + reached = true + })) + + req := httptest.NewRequest("POST", "/internal/crossread", nil) + req.Header.Set(HeaderInternalSecret, "wrong") + req.Header.Set(HeaderAppID, "spoofed-app") + req.Header.Set(HeaderUserID, "spoofed-user") + req.Header.Set(HeaderAppRole, RoleAdmin) + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + + if rec.Code != http.StatusUnauthorized { + t.Fatalf("expected 401, got %d", rec.Code) + } + if reached { + t.Error("handler must not run on a rejected request — promotion would trust spoofed headers") + } +} + +func TestInternalAuth_LocalBypass_DoesNotPromote(t *testing.T) { + // Local dev with NO secret configured: the bypass passes the request + // through, but the transport is unauthenticated — nothing proved the + // X-MS-* headers came from dispatch, so they must NOT be promoted. + // Identity promotion only ever follows a validated secret (fail closed). + t.Setenv("MS_PLATFORM_TOKEN_FILE", "") + t.Setenv("MS_PLATFORM_TOKEN", "") + t.Setenv("MS_INTERNAL_SECRET", "") + var seen *Identity + handler := internalAuth(false)(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + seen = Get(r.Context()) + })) + + req := httptest.NewRequest("POST", "/internal/crossread", nil) + req.Header.Set(HeaderAppID, "spoofed-app") + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("expected 200 (local bypass), got %d", rec.Code) + } + if seen != nil { + t.Errorf("bypass must not promote unvalidated headers; got identity %+v", seen) + } +} + +func TestInternalAuth_PayloadTrusted_EnvelopeIdentitySurvives(t *testing.T) { + // Deployed simulation: runtime.NewLambdaHandler strips X-MS-* identity + // headers, InjectResources sets Identity from the typed envelope, and the + // payload-trust mark short-circuits internalAuth BEFORE any header read. + // The envelope identity must reach the handler untouched — with no + // headers on the request, promotion must neither clobber nor blank it. + t.Setenv("MS_PLATFORM_TOKEN", "tunnel-session-token") + var seen *Identity + handler := internalAuth(false)(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + seen = Get(r.Context()) + })) + + req := httptest.NewRequest("POST", "/internal/crossread", nil) // no headers (stripped) + ctx := WithPayloadTrust(Set(req.Context(), Identity{ + UserID: "envelope-user", + AppID: "envelope-app", + AppRole: RoleAdmin, + })) + req = req.WithContext(ctx) + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("expected 200 for payload-trusted request, got %d", rec.Code) + } + if seen == nil || seen.AppID != "envelope-app" { + t.Errorf("envelope identity must survive untouched; got %+v", seen) + } +} + +func TestInternalAuth_ValidSecret_EmptyHeaders_NoIdentityMinted(t *testing.T) { + // A secret-validated request with NO identity headers must not mint an + // all-empty Identity — auth.Get stays nil (same rule InjectResources + // applies to an all-empty envelope). + t.Setenv("MS_INTERNAL_SECRET", "real-secret") + var seen *Identity + handler := internalAuth(false)(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + seen = Get(r.Context()) + })) + + req := httptest.NewRequest("POST", "/internal/crossread", nil) + req.Header.Set(HeaderInternalSecret, "real-secret") + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("expected 200, got %d", rec.Code) + } + if seen != nil { + t.Errorf("empty headers must not mint an identity; got %+v", seen) + } +} + +func TestInternalAuth_ValidSecret_PresetIdentity_NotClobbered(t *testing.T) { + // If an upstream layer already attached an identity, the promotion must + // not overwrite it with header values — mirrors + // TestRequireProxy_PresetIdentity_NotClobbered. + t.Setenv("MS_INTERNAL_SECRET", "real-secret") + var seen *Identity + handler := internalAuth(false)(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + seen = Get(r.Context()) + })) + + req := httptest.NewRequest("POST", "/internal/crossread", nil) + req.Header.Set(HeaderInternalSecret, "real-secret") + req.Header.Set(HeaderAppID, "header-app") + req = req.WithContext(Set(req.Context(), Identity{ + UserID: "preset-user", + AppID: "preset-app", + AppRole: RoleAdmin, + })) + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("expected 200, got %d", rec.Code) + } + if seen == nil || seen.AppID != "preset-app" { + t.Errorf("preset identity must win over headers; got %+v", seen) + } +} diff --git a/internal/core/internal_identity_test.go b/internal/core/internal_identity_test.go new file mode 100644 index 0000000..f475f21 --- /dev/null +++ b/internal/core/internal_identity_test.go @@ -0,0 +1,65 @@ +package core + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/go-chi/chi/v5" + "github.com/mirrorstack-ai/app-module-sdk/auth" +) + +// TestInternalRoute_TunnelForwarded_AppIDResolves is the end-to-end pin for +// the tunnel-Internal identity fix (ms-app-modules#30/#31): a request that +// dispatch forwards to an Internal-scope route on the dev/tunnel plane — +// valid secret plus strip-then-injected X-MS-* identity headers — must reach +// the handler with ms.AppID(r.Context()) resolving to the dispatch-injected +// app id (previously "" because internalAuth validated the secret but never +// promoted the headers). +func TestInternalRoute_TunnelForwarded_AppIDResolves(t *testing.T) { + m := newTestModuleWithSecret(t, "media") + var gotAppID string + var gotIdentity *auth.Identity + m.Internal(func(r chi.Router) { + r.Get("/whoami", func(w http.ResponseWriter, r *http.Request) { + gotAppID = AppID(r.Context()) + gotIdentity = auth.Get(r.Context()) + w.WriteHeader(http.StatusOK) + }) + }) + + // Shape of a dispatch tunnel forward: secret header + injected identity + // triple (dev_tunnel.go strips all inbound X-MS-* then injects these). + req := httptest.NewRequest("GET", "/internal/whoami", nil) + req.Header.Set(auth.HeaderInternalSecret, "secret") + req.Header.Set(auth.HeaderUserID, "u-owner-1") + req.Header.Set(auth.HeaderAppID, "a-tunnel-1") + req.Header.Set(auth.HeaderAppRole, auth.RoleAdmin) + rec := httptest.NewRecorder() + m.Router().ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("expected 200, got %d (body %q)", rec.Code, rec.Body.String()) + } + if gotAppID != "a-tunnel-1" { + t.Errorf("ms.AppID(ctx) = %q, want a-tunnel-1 (dispatch-injected app id)", gotAppID) + } + if gotIdentity == nil || gotIdentity.UserID != "u-owner-1" || gotIdentity.AppRole != auth.RoleAdmin { + t.Errorf("identity mismatch: got %+v", gotIdentity) + } + + // The unauthenticated twin (no secret, spoofed headers) must be rejected + // before the handler — identity promotion never fires without a + // validated secret. + gotAppID, gotIdentity = "", nil + spoof := httptest.NewRequest("GET", "/internal/whoami", nil) + spoof.Header.Set(auth.HeaderAppID, "spoofed-app") + rec2 := httptest.NewRecorder() + m.Router().ServeHTTP(rec2, spoof) + if rec2.Code != http.StatusUnauthorized { + t.Fatalf("expected 401 without secret, got %d", rec2.Code) + } + if gotAppID != "" || gotIdentity != nil { + t.Error("handler must not run (and no identity promote) on an unauthenticated request") + } +} From f41b6dfb40afb1c9df59a3a4c30117f708e52621 Mon Sep 17 00:00:00 2001 From: Sheng Kun Chang Date: Sun, 5 Jul 2026 04:59:17 +0800 Subject: [PATCH 2/2] refactor(sdk): promoteForwarderIdentity skips the request clone on no-op branches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Simplify-pass follow-up on the Internal-scope identity promotion. - Efficiency: the helper returned context.Context, forcing both call sites into an unconditional r.WithContext() — an http.Request clone (~500B) even on the two no-op branches (identity already preset, or all-empty headers). For internalAuth that was a new regression: its pre-fix success path was a zero-alloc next.ServeHTTP(w, r). Return *http.Request instead and hand back r unchanged on the no-op branches, so a headerless system-to-system internal call (events, crons) stays allocation-free on its hot path. - Doc: the helper comment claimed it was shared by "the two secret-validated HTTP paths", reading as if only two exist. platformAuth step 4 is a third, deliberately stricter (all-three-required) hand-rolled variant — name it as the intentional exception so the doc no longer overclaims. gofmt/build/vet clean; go test ./auth/... ./internal/core/... green. Co-Authored-By: Claude Fable 5 --- auth/middleware.go | 37 ++++++++++++++++++++----------------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/auth/middleware.go b/auth/middleware.go index 737e843..905c7ee 100644 --- a/auth/middleware.go +++ b/auth/middleware.go @@ -1,7 +1,6 @@ package auth import ( - "context" "crypto/subtle" "log" "net/http" @@ -240,7 +239,7 @@ func internalAuth(inLambda bool) func(http.Handler) http.Handler { // CRITICAL ORDERING: this MUST stay below the constant-time check — // promoting on the local bypass or any rejection branch would trust // spoofable headers from an unauthenticated caller. - next.ServeHTTP(w, r.WithContext(promoteForwarderIdentity(r))) + next.ServeHTTP(w, promoteForwarderIdentity(r)) }) } } @@ -353,7 +352,7 @@ func requireProxy(inLambda bool) func(http.Handler) http.Handler { // // CRITICAL ORDERING: this MUST run only after the token check above // passes. Promoting before validation would trust spoofable headers. - next.ServeHTTP(w, r.WithContext(promoteForwarderIdentity(r))) + next.ServeHTTP(w, promoteForwarderIdentity(r)) }) } } @@ -369,13 +368,16 @@ func rejectNotProxied(w http.ResponseWriter) { }) } -// promoteForwarderIdentity returns r's context with the forwarder-injected -// X-MS-User-ID / X-MS-App-ID / X-MS-App-Role headers promoted to -// auth.Identity. It is the single header-ingestion mechanism shared by the -// two secret-validated HTTP paths (requireProxy and internalAuth) and MUST -// only be called on a success path, AFTER the platform token / internal -// secret passed its constant-time check — the validated secret is the proof -// that the headers came from dispatch, not a direct caller. +// promoteForwarderIdentity returns the request to serve onward, with the +// forwarder-injected X-MS-User-ID / X-MS-App-ID / X-MS-App-Role headers +// promoted to auth.Identity on its context. It is the header-ingestion +// mechanism shared by the two secret-validated HTTP paths that admit partial +// identity (requireProxy and internalAuth); platformAuth step 4 deliberately +// hand-rolls a STRICTER variant (all-three-required, rejecting otherwise) and +// so does not route through here. It MUST only be called on a success path, +// AFTER the platform token / internal secret passed its constant-time check — +// the validated secret is the proof that the headers came from dispatch, not a +// direct caller. // // Two guards, mirroring the deployed path's precedents: // @@ -388,13 +390,14 @@ func rejectNotProxied(w http.ResponseWriter) { // request leaves auth.Get(ctx) == nil rather than planting an // all-empty Identity. // -// Unlike platformAuth step 4 there is no all-three-required rejection here: -// internal system calls legitimately carry no user (events, crons), and on -// this plane dispatch always injects app id + app role. -func promoteForwarderIdentity(r *http.Request) context.Context { +// Both no-op branches return r UNCHANGED so the caller skips the +// http.Request clone that r.WithContext always makes — a headerless +// system-to-system internal call (events, crons) stays zero-allocation on its +// hot path. +func promoteForwarderIdentity(r *http.Request) *http.Request { ctx := r.Context() if Get(ctx) != nil { - return ctx + return r } id := Identity{ UserID: r.Header.Get(HeaderUserID), @@ -402,9 +405,9 @@ func promoteForwarderIdentity(r *http.Request) context.Context { AppRole: r.Header.Get(HeaderAppRole), } if id == (Identity{}) { - return ctx + return r } - return Set(ctx, id) + return r.WithContext(Set(ctx, id)) } // secretReader returns the current secret, which header carries it, and