From b3e206d4b028f4ebcb95ae7588efa75c0ca63f29 Mon Sep 17 00:00:00 2001 From: Sheng Kun Chang Date: Sun, 5 Jul 2026 03:42:10 +0800 Subject: [PATCH 1/2] feat(sdk): ms.UserID/ms.AppRole trusted context accessors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The facade exposed only ms.AppID while auth.HeaderUserID/HeaderAppRole stayed exported, so a module needing the user id or app role read the X-MS-* header instead. That works under the dev tunnel (middleware promotes validated headers) and silently breaks deployed: the Lambda shim strips every client-settable X-MS-* identity header before the router runs, with trusted identity riding the typed invoke payload into ctx via runtime.InjectResources. ms-app-modules#30 shipped this exact bug — oauth-google bound a broker assertion to r.Header.Get(auth.HeaderAppID) and every deployed callback was rejected. Both ingestion paths already populate all three identity fields; this is purely read-side: - internal/core: UserID(ctx) / AppRole(ctx) beside AppID, same nil-checked auth.Get pattern, not module-bound so they work pre-Init. - facade: ms.UserID / ms.AppRole beside ms.AppID, docs naming them the ONLY correct identity read and spelling out the deployed-strip footgun; ms.AppID's doc gains the same warning. Empty is documented as legitimate (internal/system/cron/task calls, anonymous Public). - auth: Header* const block + package doc now state the headers are the internal platform-to-shim/tunnel wire, not a module identity API. Constants stay exported (middleware ingestion + dev schema middleware + dispatch wire contract depend on them). - regression tests (identity_test.go): all three accessors resolve via the envelope-inject path (InjectResources), via both middleware header-ingestion paths (PlatformAuth, RequireProxy validated-token), and the #30 footgun pinned end-to-end: through NewLambdaHandler the raw X-MS-* headers read "" while the ctx accessors resolve the typed payload identity. Also normalizes five pre-existing gofmt whitespace nits (import-block blank lines, comment alignment) so gofmt -l is empty. Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 1 + auth/context.go | 6 + auth/middleware.go | 17 ++- identity_test.go | 219 +++++++++++++++++++++++++++++ internal/core/call.go | 30 ++++ internal/core/cron.go | 1 - internal/core/event.go | 1 - internal/core/task.go | 1 - internal/registry/exposure_test.go | 12 +- internal/runtime/lambda_test.go | 8 +- mirrorstack.go | 67 ++++++++- 11 files changed, 345 insertions(+), 18 deletions(-) create mode 100644 identity_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index a00ff59..300dd4a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ A module can now write into its app's notification feed. The design mirrors `ms. ### Added - **`ms.Notify(ctx, ms.Notification{...})`** — sends an in-app notification to the current app's members. `Notification` carries an i18n `Title` (required) and `Body` (optional) as `ms.Text`/`ms.T` Labels resolved to per-locale maps **at send time** (the platform picks each recipient's locale, the module never does), plus optional `Icon`/`Link` and an `Audience` (`ms.NotifyAdmins`, the default, or `ms.NotifyAllMembers`; anything else is an error). The SDK POSTs a `{id, sentAt, sourceModuleID, title, body, icon, link, audience}` envelope to the platform dispatch notification ingress at `{MS_DISPATCH_URL | dev fallback}/apps/{appID}/notifications` — the same transport idiom as `ms.Emit`/`ms.Call`, with the same error contract: an empty app-scope context, a Title that resolves to no message, or a non-2xx dispatch response (body truncated to ~2 KB) is a **returned error, never a panic**. +- **`ms.UserID(ctx)` and `ms.AppRole(ctx)`** — trusted identity accessors completing the read surface next to `ms.AppID` — together with `auth.Get` (the full-`Identity` read) the ONLY correct way for module code to read the request's identity: always the context, never the `X-MS-*` headers. All three resolve from the context identity the SDK promotes on every guarded surface (Platform via `PlatformAuth`, Public via the proxy guard's validated-token path, Lambda/task via the typed envelope through `runtime.InjectResources`). Reading the `X-MS-*` headers (`auth.HeaderUserID` / `auth.HeaderAppRole`) instead is the footgun these close: headers exist on the dev tunnel but the deployed Lambda shim **strips** every client-settable identity header, so header reads silently break in production — the exact bug shipped in ms-app-modules#30, now pinned by a regression test. `""` is a legitimate return (internal/system/cron/task calls carry no user; an anonymous Public request may carry an empty user id). The `auth.Header*` constants remain exported as the internal platform-to-SDK wire; their docs now say so. ## [v0.2.6] - 2026-06-20 diff --git a/auth/context.go b/auth/context.go index 6ff099b..e189ac6 100644 --- a/auth/context.go +++ b/auth/context.go @@ -1,4 +1,10 @@ // Package auth provides authentication context and middleware for MirrorStack modules. +// +// Module code reads the request identity from the CONTEXT — ms.UserID / +// ms.AppID / ms.AppRole (or auth.Get) — never from the X-MS-* Header* +// constants, which are the internal platform-to-SDK wire: the deployed Lambda +// shim strips those headers before the router runs, so a header read works +// under the dev tunnel and silently breaks deployed. package auth import "context" diff --git a/auth/middleware.go b/auth/middleware.go index eab8e9b..ce6afb9 100644 --- a/auth/middleware.go +++ b/auth/middleware.go @@ -11,10 +11,19 @@ import ( "github.com/mirrorstack-ai/app-module-sdk/internal/lambdaenv" ) -// Trusted-forwarder identity-injection headers. A caller proves it's a -// trusted forwarder (the platform or its dispatch) by sending a valid -// X-MS-Internal-Secret; if proven, PlatformAuth trusts the user identity -// those callers assert via these headers. +// Trusted-forwarder identity-injection headers — the INTERNAL wire between +// the platform (dispatch/tunnel) and this SDK's middleware, NOT a module +// identity API. A caller proves it's a trusted forwarder (the platform or its +// dispatch) by sending a valid X-MS-Internal-Secret; if proven, PlatformAuth +// trusts the user identity those callers assert via these headers. +// +// Module code must NEVER read the identity-claim headers (HeaderUserID / +// HeaderAppID / HeaderAppRole) directly: the deployed Lambda shim strips +// every client-settable X-MS-* identity header before the router runs +// (trusted identity rides the typed invoke payload instead), so a header +// read works under the dev tunnel and silently breaks deployed — the exact +// bug shipped in ms-app-modules#30. Read identity via ms.UserID / ms.AppID / +// ms.AppRole (or auth.Get), which resolve on every ingestion path. const ( HeaderInternalSecret = "X-MS-Internal-Secret" HeaderPlatformToken = "X-MS-Platform-Token" diff --git a/identity_test.go b/identity_test.go new file mode 100644 index 0000000..bf3562d --- /dev/null +++ b/identity_test.go @@ -0,0 +1,219 @@ +package mirrorstack_test + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + ms "github.com/mirrorstack-ai/app-module-sdk" + "github.com/mirrorstack-ai/app-module-sdk/auth" + "github.com/mirrorstack-ai/app-module-sdk/internal/runtime" +) + +func TestUserID(t *testing.T) { + t.Run("returns empty string when no identity is set", func(t *testing.T) { + if got := ms.UserID(context.Background()); got != "" { + t.Errorf("UserID = %q, want empty string", got) + } + }) + + t.Run("reads the user id from the context identity", func(t *testing.T) { + ctx := auth.Set(context.Background(), auth.Identity{ + UserID: "u-1", + AppID: "app-7", + AppRole: auth.RoleAdmin, + }) + if got := ms.UserID(ctx); got != "u-1" { + t.Errorf("UserID = %q, want u-1", got) + } + }) + + t.Run("empty user id inside a set identity is legitimate", func(t *testing.T) { + // Anonymous Public requests and internal/system calls carry an + // identity with no user — that is a valid state, not an error. + ctx := auth.Set(context.Background(), auth.Identity{AppID: "app-7"}) + if got := ms.UserID(ctx); got != "" { + t.Errorf("UserID = %q, want empty string", got) + } + }) +} + +func TestAppRole(t *testing.T) { + t.Run("returns empty string when no identity is set", func(t *testing.T) { + if got := ms.AppRole(context.Background()); got != "" { + t.Errorf("AppRole = %q, want empty string", got) + } + }) + + t.Run("reads the app role from the context identity", func(t *testing.T) { + ctx := auth.Set(context.Background(), auth.Identity{ + UserID: "u-1", + AppID: "app-7", + AppRole: auth.RoleAdmin, + }) + if got := ms.AppRole(ctx); got != auth.RoleAdmin { + t.Errorf("AppRole = %q, want %q", got, auth.RoleAdmin) + } + }) + + t.Run("empty role inside a set identity is legitimate", func(t *testing.T) { + // Internal/system/cron/task invocations carry no user role. + ctx := auth.Set(context.Background(), auth.Identity{AppID: "app-7"}) + if got := ms.AppRole(ctx); got != "" { + t.Errorf("AppRole = %q, want empty string", got) + } + }) +} + +// TestIdentityAccessors_EnvelopeInjectPath pins that all three accessors +// resolve identity injected by runtime.InjectResources — the shared trusted +// gate the Lambda handler AND the task worker feed from the typed envelope. +func TestIdentityAccessors_EnvelopeInjectPath(t *testing.T) { + ctx, err := runtime.InjectResources(context.Background(), runtime.InjectParams{ + UserID: "u-envelope", + AppID: "app-envelope", + AppRole: auth.RoleMember, + }) + if err != nil { + t.Fatalf("InjectResources: %v", err) + } + + if got := ms.UserID(ctx); got != "u-envelope" { + t.Errorf("UserID = %q, want u-envelope", got) + } + if got := ms.AppID(ctx); got != "app-envelope" { + t.Errorf("AppID = %q, want app-envelope", got) + } + if got := ms.AppRole(ctx); got != auth.RoleMember { + t.Errorf("AppRole = %q, want %q", got, auth.RoleMember) + } +} + +// TestIdentityAccessors_PlatformAuthHeaderPath pins that all three accessors +// resolve identity the PLATFORM surface ingests from validated trusted- +// forwarder headers (the dev tunnel / dispatch path). +func TestIdentityAccessors_PlatformAuthHeaderPath(t *testing.T) { + t.Setenv("MS_PLATFORM_TOKEN", "") + t.Setenv("MS_INTERNAL_SECRET", "test-secret") + + var gotUser, gotApp, gotRole string + handler := auth.PlatformAuth()(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + gotUser = ms.UserID(r.Context()) + gotApp = ms.AppID(r.Context()) + gotRole = ms.AppRole(r.Context()) + })) + + req := httptest.NewRequest("GET", "/platform/users", nil) + req.Header.Set(auth.HeaderInternalSecret, "test-secret") + req.Header.Set(auth.HeaderUserID, "u-tunnel") + req.Header.Set(auth.HeaderAppID, "app-tunnel") + req.Header.Set(auth.HeaderAppRole, auth.RoleViewer) + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("expected 200, got %d", rec.Code) + } + if gotUser != "u-tunnel" || gotApp != "app-tunnel" || gotRole != auth.RoleViewer { + t.Errorf("accessors = %q/%q/%q, want u-tunnel/app-tunnel/%q", + gotUser, gotApp, gotRole, auth.RoleViewer) + } +} + +// TestIdentityAccessors_RequireProxyHeaderPath pins that all three accessors +// resolve identity the PUBLIC surface ingests on the proxy guard's +// validated-token path (the dev tunnel / dispatch path). +func TestIdentityAccessors_RequireProxyHeaderPath(t *testing.T) { + t.Setenv("MS_INTERNAL_SECRET", "") // avoid ambient env cross-talk + t.Setenv("MS_PLATFORM_TOKEN", "test-token") + + var gotUser, gotApp, gotRole string + handler := auth.RequireProxy()(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + gotUser = ms.UserID(r.Context()) + gotApp = ms.AppID(r.Context()) + gotRole = ms.AppRole(r.Context()) + })) + + req := httptest.NewRequest("GET", "/public/me", nil) + req.Header.Set(auth.HeaderPlatformToken, "test-token") + req.Header.Set(auth.HeaderUserID, "u-pub") + req.Header.Set(auth.HeaderAppID, "app-pub") + req.Header.Set(auth.HeaderAppRole, auth.RoleViewer) + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("expected 200, got %d", rec.Code) + } + if gotUser != "u-pub" || gotApp != "app-pub" || gotRole != auth.RoleViewer { + t.Errorf("accessors = %q/%q/%q, want u-pub/app-pub/%q", + gotUser, gotApp, gotRole, auth.RoleViewer) + } +} + +// TestIdentityAccessors_DeployedPath_HeadersStrippedCtxResolves pins the +// ms-app-modules#30 footgun as a test: on the deployed Lambda path the shim +// STRIPS the client-settable X-MS-* identity headers, so a module that reads +// r.Header.Get(auth.Header*) gets "" — while ms.UserID / ms.AppID / ms.AppRole +// still resolve the trusted identity from the typed invoke payload. Reading +// headers works on the tunnel and silently breaks deployed; the ctx accessors +// work on both. +func TestIdentityAccessors_DeployedPath_HeadersStrippedCtxResolves(t *testing.T) { + var ( + headerUser, headerApp, headerRole string + ctxUser, ctxApp, ctxRole string + ) + mux := http.NewServeMux() + mux.HandleFunc("/whoami", func(w http.ResponseWriter, r *http.Request) { + headerUser = r.Header.Get(auth.HeaderUserID) + headerApp = r.Header.Get(auth.HeaderAppID) + headerRole = r.Header.Get(auth.HeaderAppRole) + ctxUser = ms.UserID(r.Context()) + ctxApp = ms.AppID(r.Context()) + ctxRole = ms.AppRole(r.Context()) + }) + + handler := runtime.NewLambdaHandler(mux) + payload, err := json.Marshal(runtime.LambdaRequest{ + Method: "GET", + Path: "/whoami", + // Spoofed identity claims riding the forwarded headers — the shim + // must drop every one of them. + Headers: map[string]string{ + auth.HeaderUserID: "spoofed-user", + auth.HeaderAppID: "spoofed-app", + auth.HeaderAppRole: auth.RoleAdmin, + }, + // Trusted identity — typed envelope fields injected by the platform. + UserID: "u-payload", + AppID: "app-payload", + AppRole: auth.RoleMember, + }) + if err != nil { + t.Fatalf("marshal payload: %v", err) + } + + resp, err := handler(context.Background(), payload) + if err != nil { + t.Fatalf("lambda handler: %v", err) + } + if resp.StatusCode != http.StatusOK { + t.Fatalf("expected 200, got %d (body %q)", resp.StatusCode, resp.Body) + } + + if headerUser != "" || headerApp != "" || headerRole != "" { + t.Errorf("X-MS-* identity headers must be stripped on the deployed path, got %q/%q/%q", + headerUser, headerApp, headerRole) + } + if ctxUser != "u-payload" { + t.Errorf("UserID = %q, want u-payload (typed payload identity)", ctxUser) + } + if ctxApp != "app-payload" { + t.Errorf("AppID = %q, want app-payload (typed payload identity)", ctxApp) + } + if ctxRole != auth.RoleMember { + t.Errorf("AppRole = %q, want %q (typed payload identity)", ctxRole, auth.RoleMember) + } +} diff --git a/internal/core/call.go b/internal/core/call.go index 7a7835f..6fe6d40 100644 --- a/internal/core/call.go +++ b/internal/core/call.go @@ -144,3 +144,33 @@ func AppID(ctx context.Context) string { } return "" } + +// UserID returns the user id from the request context's auth identity, or "" +// if no identity is set. Same promotion paths as AppID (Platform via +// PlatformAuth, Public via the proxy guard's success path; Lambda via +// runtime.InjectResources). "" is a legitimate value on surfaces with no user +// (internal/system/cron/task invocations, anonymous Public requests). +// +// Not module-bound (no *Module receiver) — identity lives on the context, so +// this works before Init and in tests. +func UserID(ctx context.Context) string { + if a := auth.Get(ctx); a != nil { + return a.UserID + } + return "" +} + +// AppRole returns the app role from the request context's auth identity, or +// "" if no identity is set. Same promotion paths as AppID (Platform via +// PlatformAuth, Public via the proxy guard's success path; Lambda via +// runtime.InjectResources). "" is a legitimate value on surfaces with no user +// (internal/system/cron/task invocations, anonymous Public requests). +// +// Not module-bound (no *Module receiver) — identity lives on the context, so +// this works before Init and in tests. +func AppRole(ctx context.Context) string { + if a := auth.Get(ctx); a != nil { + return a.AppRole + } + return "" +} diff --git a/internal/core/cron.go b/internal/core/cron.go index cc50d22..97dbbc7 100644 --- a/internal/core/cron.go +++ b/internal/core/cron.go @@ -2,7 +2,6 @@ package core import ( "net/http" - ) // cronPathPrefix mounts under the reserved /__mirrorstack/ namespace. diff --git a/internal/core/event.go b/internal/core/event.go index 6d1cac3..2b723d6 100644 --- a/internal/core/event.go +++ b/internal/core/event.go @@ -3,7 +3,6 @@ package core import ( "net/http" - "github.com/mirrorstack-ai/app-module-sdk/internal/registry" ) diff --git a/internal/core/task.go b/internal/core/task.go index 94427d4..6ed3c47 100644 --- a/internal/core/task.go +++ b/internal/core/task.go @@ -7,7 +7,6 @@ import ( "net/http" "time" - "github.com/mirrorstack-ai/app-module-sdk/auth" "github.com/mirrorstack-ai/app-module-sdk/cache" "github.com/mirrorstack-ai/app-module-sdk/db" diff --git a/internal/registry/exposure_test.go b/internal/registry/exposure_test.go index b0ed371..7d913e7 100644 --- a/internal/registry/exposure_test.go +++ b/internal/registry/exposure_test.go @@ -83,13 +83,13 @@ func TestAddExposedTable_PanicsOnInvalidName(t *testing.T) { t.Parallel() bad := []string{ - "", // empty - "Orders", // uppercase - "1orders", // leading digit - "my orders", // whitespace + "", // empty + "Orders", // uppercase + "1orders", // leading digit + "my orders", // whitespace "orders-table", // hyphen (not a Postgres-safe bare identifier) - "mod.orders", // dot / schema-qualified - "../etc", // path traversal shape + "mod.orders", // dot / schema-qualified + "../etc", // path traversal shape } for _, name := range bad { name := name diff --git a/internal/runtime/lambda_test.go b/internal/runtime/lambda_test.go index 0d07241..87f6d26 100644 --- a/internal/runtime/lambda_test.go +++ b/internal/runtime/lambda_test.go @@ -197,10 +197,10 @@ func TestNewLambdaHandler_AuthSecretHeadersSurvive(t *testing.T) { Path: "/check", Headers: map[string]string{ auth.HeaderInternalSecret: "platform-secret", // exempt — must survive - auth.HeaderPlatformToken: "platform-token", // exempt — must survive - auth.HeaderUserID: "spoofed-user", // claim — must be stripped - auth.HeaderAppID: "spoofed-app", // claim — must be stripped - auth.HeaderAppRole: "admin", // claim — must be stripped + auth.HeaderPlatformToken: "platform-token", // exempt — must survive + auth.HeaderUserID: "spoofed-user", // claim — must be stripped + auth.HeaderAppID: "spoofed-app", // claim — must be stripped + auth.HeaderAppRole: "admin", // claim — must be stripped }, }) diff --git a/mirrorstack.go b/mirrorstack.go index b15e72e..41eda6a 100644 --- a/mirrorstack.go +++ b/mirrorstack.go @@ -315,11 +315,76 @@ func Notify(ctx context.Context, n Notification) error { // appID := ms.AppID(r.Context()) // // Do NOT read the app id from request data (query string, body, path) — those -// are caller-controlled and forgeable. ms.AppID is the trusted source. +// are caller-controlled and forgeable. And do NOT read it from the +// auth.HeaderAppID header: the deployed Lambda shim STRIPS every client- +// settable X-MS-* identity header before the router runs (identity rides the +// typed invoke payload instead), so a header read works under the dev tunnel +// and silently breaks deployed — the exact bug shipped in ms-app-modules#30. +// ms.AppID is the trusted source on every path. func AppID(ctx context.Context) string { return core.AppID(ctx) } +// UserID returns the current user id from the request context, or "" if no +// identity is set. Together with ms.AppID and ms.AppRole (or auth.Get, which +// returns the full Identity in one read) it is the ONLY correct way for +// module code to read the request's identity: always the context, never the +// X-MS-* headers. +// +// On every guarded surface the SDK promotes the platform's trusted, dispatch- +// injected user id into the context identity BEFORE the handler runs — Platform +// via PlatformAuth, Public via the proxy guard's validated-token path, and +// Lambda via the typed invoke payload (runtime.InjectResources). So a handler +// reads the requesting user with: +// +// userID := ms.UserID(r.Context()) +// +// Do NOT read the auth.HeaderUserID (X-MS-User-ID) header instead: the +// deployed Lambda shim STRIPS every client-settable X-MS-* identity header +// before the router runs, so a header read works under the dev tunnel and +// silently breaks deployed (empty value / rejected request) — the exact bug +// class shipped in ms-app-modules#30. Those headers are the platform-to-SDK +// wire, not a module identity API. +// +// "" is a legitimate value, not only an unauthenticated-middleware artifact: +// internal/system/cron/task invocations carry no user, and an anonymous Public +// request may carry an identity whose user id is empty. Under the local-dev +// bypass (no secret configured) it returns the synthetic "local-dev-user". +func UserID(ctx context.Context) string { + return core.UserID(ctx) +} + +// AppRole returns the current user's role in the app from the request context +// ("admin", "member", or "viewer" — compare against auth.RoleAdmin / +// auth.RoleMember / auth.RoleViewer), or "" if no identity is set. Together +// with ms.AppID and ms.UserID (or auth.Get, which returns the full Identity +// in one read) it is the ONLY correct way for module code to read the +// request's identity: always the context, never the X-MS-* headers. Note: +// ms.AppRole is a read of WHO the platform says the caller is — for gating +// routes by role, prefer the declarative RequirePermission middleware. +// +// On every guarded surface the SDK promotes the platform's trusted, dispatch- +// injected role into the context identity BEFORE the handler runs — Platform +// via PlatformAuth, Public via the proxy guard's validated-token path, and +// Lambda via the typed invoke payload (runtime.InjectResources). So a handler +// reads the caller's role with: +// +// if ms.AppRole(r.Context()) == auth.RoleAdmin { ... } +// +// Do NOT read the auth.HeaderAppRole (X-MS-App-Role) header instead: the +// deployed Lambda shim STRIPS every client-settable X-MS-* identity header +// before the router runs, so a header read works under the dev tunnel and +// silently breaks deployed (empty value / rejected request) — the exact bug +// class shipped in ms-app-modules#30. Those headers are the platform-to-SDK +// wire, not a module identity API. +// +// "" is a legitimate value: internal/system/cron/task invocations carry no +// user role. Under the local-dev bypass (no secret configured) it returns the +// synthetic auth.RoleAdmin. +func AppRole(ctx context.Context) string { + return core.AppRole(ctx) +} + // Log returns the request's structured logger, pre-tagged with the trusted // app_id / request_id / module_id correlation fields so every line is // attributable in the platform Logcat. Prefer it over the standard library From 8e479b13ced4670b2a4cd5d1ca9f7ad53dfadd75 Mon Sep 17 00:00:00 2001 From: Sheng Kun Chang Date: Sun, 5 Jul 2026 03:57:10 +0800 Subject: [PATCH 2/2] refactor(sdk): dedupe identity accessor bodies, docs, and tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Simplify pass on the ms.UserID/ms.AppRole accessors, no behavior change: - internal/core: collapse the four copies of the auth.Get nil-guard (AppID, UserID, AppRole, and the inline read in Module.Call) onto one shared identity(ctx) helper; the next Identity field becomes a one-line accessor. Shorten the UserID/AppRole doc blocks to reference AppID instead of repeating its promotion-path paragraphs verbatim. - docs: the "deployed Lambda shim strips X-MS-* identity headers" story now has one canonical home (the auth.Header* const docs); the auth package doc and the three ms.* accessor docs point there with a single line instead of carrying near-verbatim copies that had already started to drift. - identity_test.go: fold the structurally identical TestUserID / TestAppRole triplets into one table (TestIdentityAccessors_ContextReads) and the PlatformAuth/RequireProxy copy-paste twins into one table-driven TestIdentityAccessors_GuardHeaderPaths. Coverage is unchanged — every previous assertion still runs, including the ms-app-modules#30 deployed-path regression pin. Co-Authored-By: Claude Fable 5 --- auth/context.go | 5 +- identity_test.go | 211 ++++++++++++++++++++---------------------- internal/core/call.go | 49 ++++------ mirrorstack.go | 45 +++------ 4 files changed, 134 insertions(+), 176 deletions(-) diff --git a/auth/context.go b/auth/context.go index e189ac6..31989c6 100644 --- a/auth/context.go +++ b/auth/context.go @@ -2,9 +2,8 @@ // // Module code reads the request identity from the CONTEXT — ms.UserID / // ms.AppID / ms.AppRole (or auth.Get) — never from the X-MS-* Header* -// constants, which are the internal platform-to-SDK wire: the deployed Lambda -// shim strips those headers before the router runs, so a header read works -// under the dev tunnel and silently breaks deployed. +// constants, which are the internal platform-to-SDK wire; see the Header* +// docs for why header reads silently break deployed. package auth import "context" diff --git a/identity_test.go b/identity_test.go index bf3562d..60a93db 100644 --- a/identity_test.go +++ b/identity_test.go @@ -12,59 +12,45 @@ import ( "github.com/mirrorstack-ai/app-module-sdk/internal/runtime" ) -func TestUserID(t *testing.T) { - t.Run("returns empty string when no identity is set", func(t *testing.T) { - if got := ms.UserID(context.Background()); got != "" { - t.Errorf("UserID = %q, want empty string", got) - } - }) - - t.Run("reads the user id from the context identity", func(t *testing.T) { - ctx := auth.Set(context.Background(), auth.Identity{ - UserID: "u-1", - AppID: "app-7", - AppRole: auth.RoleAdmin, - }) - if got := ms.UserID(ctx); got != "u-1" { - t.Errorf("UserID = %q, want u-1", got) - } - }) - - t.Run("empty user id inside a set identity is legitimate", func(t *testing.T) { - // Anonymous Public requests and internal/system calls carry an - // identity with no user — that is a valid state, not an error. - ctx := auth.Set(context.Background(), auth.Identity{AppID: "app-7"}) - if got := ms.UserID(ctx); got != "" { - t.Errorf("UserID = %q, want empty string", got) - } - }) -} - -func TestAppRole(t *testing.T) { - t.Run("returns empty string when no identity is set", func(t *testing.T) { - if got := ms.AppRole(context.Background()); got != "" { - t.Errorf("AppRole = %q, want empty string", got) - } - }) - - t.Run("reads the app role from the context identity", func(t *testing.T) { - ctx := auth.Set(context.Background(), auth.Identity{ - UserID: "u-1", - AppID: "app-7", - AppRole: auth.RoleAdmin, +// TestIdentityAccessors_ContextReads pins the unit contract of ms.UserID and +// ms.AppRole against each identity state a context can carry. +func TestIdentityAccessors_ContextReads(t *testing.T) { + cases := []struct { + name string + ctx context.Context + wantUser, wantRole string + }{ + { + name: "no identity set returns empty strings", + ctx: context.Background(), + }, + { + name: "reads the fields from the context identity", + ctx: auth.Set(context.Background(), auth.Identity{ + UserID: "u-1", + AppID: "app-7", + AppRole: auth.RoleAdmin, + }), + wantUser: "u-1", + wantRole: auth.RoleAdmin, + }, + { + // Anonymous Public requests and internal/system/cron/task calls + // carry an identity with no user — a valid state, not an error. + name: "empty fields inside a set identity are legitimate", + ctx: auth.Set(context.Background(), auth.Identity{AppID: "app-7"}), + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := ms.UserID(tc.ctx); got != tc.wantUser { + t.Errorf("UserID = %q, want %q", got, tc.wantUser) + } + if got := ms.AppRole(tc.ctx); got != tc.wantRole { + t.Errorf("AppRole = %q, want %q", got, tc.wantRole) + } }) - if got := ms.AppRole(ctx); got != auth.RoleAdmin { - t.Errorf("AppRole = %q, want %q", got, auth.RoleAdmin) - } - }) - - t.Run("empty role inside a set identity is legitimate", func(t *testing.T) { - // Internal/system/cron/task invocations carry no user role. - ctx := auth.Set(context.Background(), auth.Identity{AppID: "app-7"}) - if got := ms.AppRole(ctx); got != "" { - t.Errorf("AppRole = %q, want empty string", got) - } - }) + } } // TestIdentityAccessors_EnvelopeInjectPath pins that all three accessors @@ -91,65 +77,70 @@ func TestIdentityAccessors_EnvelopeInjectPath(t *testing.T) { } } -// TestIdentityAccessors_PlatformAuthHeaderPath pins that all three accessors -// resolve identity the PLATFORM surface ingests from validated trusted- -// forwarder headers (the dev tunnel / dispatch path). -func TestIdentityAccessors_PlatformAuthHeaderPath(t *testing.T) { - t.Setenv("MS_PLATFORM_TOKEN", "") - t.Setenv("MS_INTERNAL_SECRET", "test-secret") - - var gotUser, gotApp, gotRole string - handler := auth.PlatformAuth()(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { - gotUser = ms.UserID(r.Context()) - gotApp = ms.AppID(r.Context()) - gotRole = ms.AppRole(r.Context()) - })) - - req := httptest.NewRequest("GET", "/platform/users", nil) - req.Header.Set(auth.HeaderInternalSecret, "test-secret") - req.Header.Set(auth.HeaderUserID, "u-tunnel") - req.Header.Set(auth.HeaderAppID, "app-tunnel") - req.Header.Set(auth.HeaderAppRole, auth.RoleViewer) - rec := httptest.NewRecorder() - handler.ServeHTTP(rec, req) - - if rec.Code != http.StatusOK { - t.Fatalf("expected 200, got %d", rec.Code) - } - if gotUser != "u-tunnel" || gotApp != "app-tunnel" || gotRole != auth.RoleViewer { - t.Errorf("accessors = %q/%q/%q, want u-tunnel/app-tunnel/%q", - gotUser, gotApp, gotRole, auth.RoleViewer) - } -} - -// TestIdentityAccessors_RequireProxyHeaderPath pins that all three accessors -// resolve identity the PUBLIC surface ingests on the proxy guard's -// validated-token path (the dev tunnel / dispatch path). -func TestIdentityAccessors_RequireProxyHeaderPath(t *testing.T) { - t.Setenv("MS_INTERNAL_SECRET", "") // avoid ambient env cross-talk - t.Setenv("MS_PLATFORM_TOKEN", "test-token") - - var gotUser, gotApp, gotRole string - handler := auth.RequireProxy()(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { - gotUser = ms.UserID(r.Context()) - gotApp = ms.AppID(r.Context()) - gotRole = ms.AppRole(r.Context()) - })) - - req := httptest.NewRequest("GET", "/public/me", nil) - req.Header.Set(auth.HeaderPlatformToken, "test-token") - req.Header.Set(auth.HeaderUserID, "u-pub") - req.Header.Set(auth.HeaderAppID, "app-pub") - req.Header.Set(auth.HeaderAppRole, auth.RoleViewer) - rec := httptest.NewRecorder() - handler.ServeHTTP(rec, req) - - if rec.Code != http.StatusOK { - t.Fatalf("expected 200, got %d", rec.Code) +// TestIdentityAccessors_GuardHeaderPaths pins that all three accessors +// resolve identity ingested from validated trusted-forwarder headers (the dev +// tunnel / dispatch path) on both HTTP guard surfaces: Platform via +// PlatformAuth and Public via the proxy guard's validated-token path. +func TestIdentityAccessors_GuardHeaderPaths(t *testing.T) { + cases := []struct { + name string + internalSecret string // MS_INTERNAL_SECRET (empty avoids ambient cross-talk) + platformToken string // MS_PLATFORM_TOKEN (empty avoids ambient cross-talk) + guard func() func(http.Handler) http.Handler + proofHeader string // header proving trusted-forwarder status + proofValue string + path string + user, app string + }{ + { + name: "platform surface via PlatformAuth", + internalSecret: "test-secret", + guard: auth.PlatformAuth, + proofHeader: auth.HeaderInternalSecret, + proofValue: "test-secret", + path: "/platform/users", + user: "u-tunnel", + app: "app-tunnel", + }, + { + name: "public surface via RequireProxy validated-token path", + platformToken: "test-token", + guard: auth.RequireProxy, + proofHeader: auth.HeaderPlatformToken, + proofValue: "test-token", + path: "/public/me", + user: "u-pub", + app: "app-pub", + }, } - if gotUser != "u-pub" || gotApp != "app-pub" || gotRole != auth.RoleViewer { - t.Errorf("accessors = %q/%q/%q, want u-pub/app-pub/%q", - gotUser, gotApp, gotRole, auth.RoleViewer) + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Setenv("MS_INTERNAL_SECRET", tc.internalSecret) + t.Setenv("MS_PLATFORM_TOKEN", tc.platformToken) + + var gotUser, gotApp, gotRole string + handler := tc.guard()(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + gotUser = ms.UserID(r.Context()) + gotApp = ms.AppID(r.Context()) + gotRole = ms.AppRole(r.Context()) + })) + + req := httptest.NewRequest("GET", tc.path, nil) + req.Header.Set(tc.proofHeader, tc.proofValue) + req.Header.Set(auth.HeaderUserID, tc.user) + req.Header.Set(auth.HeaderAppID, tc.app) + req.Header.Set(auth.HeaderAppRole, auth.RoleViewer) + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("expected 200, got %d", rec.Code) + } + if gotUser != tc.user || gotApp != tc.app || gotRole != auth.RoleViewer { + t.Errorf("accessors = %q/%q/%q, want %s/%s/%s", + gotUser, gotApp, gotRole, tc.user, tc.app, auth.RoleViewer) + } + }) } } diff --git a/internal/core/call.go b/internal/core/call.go index 6fe6d40..79a7c48 100644 --- a/internal/core/call.go +++ b/internal/core/call.go @@ -61,10 +61,7 @@ func resolveCallURL(targetModuleID, path string) string { // // DEV/DISPATCH TRANSPORT — see resolveCallURL for the prod (#146) seam. func (m *Module) Call(ctx context.Context, targetModuleID, method, path string, body, out any) error { - appID := "" - if a := auth.Get(ctx); a != nil { - appID = a.AppID - } + appID := AppID(ctx) var reader io.Reader if body != nil { @@ -129,6 +126,15 @@ func CallPost(ctx context.Context, targetModuleID, path string, body, out any) e return mustDefault("CallPost").CallPost(ctx, targetModuleID, path, body, out) } +// identity returns the context's auth identity, or the zero Identity when +// none is set, so field reads on it degrade to "". +func identity(ctx context.Context) auth.Identity { + if a := auth.Get(ctx); a != nil { + return *a + } + return auth.Identity{} +} + // AppID returns the app id from the request context's auth identity, or "" if // no identity is set. It is the inbound twin of WithAppID and the single // unspoofable way a handler reads its own app: the SDK promotes the trusted, @@ -139,38 +145,21 @@ func CallPost(ctx context.Context, targetModuleID, path string, body, out any) e // Not module-bound (no *Module receiver) — identity lives on the context, so // this works before Init and in tests. func AppID(ctx context.Context) string { - if a := auth.Get(ctx); a != nil { - return a.AppID - } - return "" + return identity(ctx).AppID } // UserID returns the user id from the request context's auth identity, or "" -// if no identity is set. Same promotion paths as AppID (Platform via -// PlatformAuth, Public via the proxy guard's success path; Lambda via -// runtime.InjectResources). "" is a legitimate value on surfaces with no user -// (internal/system/cron/task invocations, anonymous Public requests). -// -// Not module-bound (no *Module receiver) — identity lives on the context, so -// this works before Init and in tests. +// if no identity is set. Same promotion paths and context-not-module shape as +// AppID; "" is legitimate on surfaces with no user (internal/system/cron/task +// invocations, anonymous Public requests). func UserID(ctx context.Context) string { - if a := auth.Get(ctx); a != nil { - return a.UserID - } - return "" + return identity(ctx).UserID } // AppRole returns the app role from the request context's auth identity, or -// "" if no identity is set. Same promotion paths as AppID (Platform via -// PlatformAuth, Public via the proxy guard's success path; Lambda via -// runtime.InjectResources). "" is a legitimate value on surfaces with no user -// (internal/system/cron/task invocations, anonymous Public requests). -// -// Not module-bound (no *Module receiver) — identity lives on the context, so -// this works before Init and in tests. +// "" if no identity is set. Same promotion paths and context-not-module shape +// as AppID; "" is legitimate on surfaces with no user (internal/system/cron/ +// task invocations, anonymous Public requests). func AppRole(ctx context.Context) string { - if a := auth.Get(ctx); a != nil { - return a.AppRole - } - return "" + return identity(ctx).AppRole } diff --git a/mirrorstack.go b/mirrorstack.go index 41eda6a..dc9a7df 100644 --- a/mirrorstack.go +++ b/mirrorstack.go @@ -315,12 +315,10 @@ func Notify(ctx context.Context, n Notification) error { // appID := ms.AppID(r.Context()) // // Do NOT read the app id from request data (query string, body, path) — those -// are caller-controlled and forgeable. And do NOT read it from the -// auth.HeaderAppID header: the deployed Lambda shim STRIPS every client- -// settable X-MS-* identity header before the router runs (identity rides the -// typed invoke payload instead), so a header read works under the dev tunnel -// and silently breaks deployed — the exact bug shipped in ms-app-modules#30. -// ms.AppID is the trusted source on every path. +// are caller-controlled and forgeable — and do NOT read the auth.HeaderAppID +// header: the deployed Lambda shim strips it, so header reads silently break +// deployed (ms-app-modules#30; see the auth.Header* docs). ms.AppID is the +// trusted source on every path. func AppID(ctx context.Context) string { return core.AppID(ctx) } @@ -329,7 +327,9 @@ func AppID(ctx context.Context) string { // identity is set. Together with ms.AppID and ms.AppRole (or auth.Get, which // returns the full Identity in one read) it is the ONLY correct way for // module code to read the request's identity: always the context, never the -// X-MS-* headers. +// auth.HeaderUserID (X-MS-User-ID) header — the deployed Lambda shim strips +// it, so header reads work under the dev tunnel and silently break deployed +// (ms-app-modules#30; see the auth.Header* docs). // // On every guarded surface the SDK promotes the platform's trusted, dispatch- // injected user id into the context identity BEFORE the handler runs — Platform @@ -339,13 +339,6 @@ func AppID(ctx context.Context) string { // // userID := ms.UserID(r.Context()) // -// Do NOT read the auth.HeaderUserID (X-MS-User-ID) header instead: the -// deployed Lambda shim STRIPS every client-settable X-MS-* identity header -// before the router runs, so a header read works under the dev tunnel and -// silently breaks deployed (empty value / rejected request) — the exact bug -// class shipped in ms-app-modules#30. Those headers are the platform-to-SDK -// wire, not a module identity API. -// // "" is a legitimate value, not only an unauthenticated-middleware artifact: // internal/system/cron/task invocations carry no user, and an anonymous Public // request may carry an identity whose user id is empty. Under the local-dev @@ -356,28 +349,14 @@ func UserID(ctx context.Context) string { // AppRole returns the current user's role in the app from the request context // ("admin", "member", or "viewer" — compare against auth.RoleAdmin / -// auth.RoleMember / auth.RoleViewer), or "" if no identity is set. Together -// with ms.AppID and ms.UserID (or auth.Get, which returns the full Identity -// in one read) it is the ONLY correct way for module code to read the -// request's identity: always the context, never the X-MS-* headers. Note: -// ms.AppRole is a read of WHO the platform says the caller is — for gating -// routes by role, prefer the declarative RequirePermission middleware. -// -// On every guarded surface the SDK promotes the platform's trusted, dispatch- -// injected role into the context identity BEFORE the handler runs — Platform -// via PlatformAuth, Public via the proxy guard's validated-token path, and -// Lambda via the typed invoke payload (runtime.InjectResources). So a handler -// reads the caller's role with: +// auth.RoleMember / auth.RoleViewer), or "" if no identity is set. Identity +// promotion and the always-the-context-never-the-X-MS-*-headers rule are +// exactly as documented on ms.UserID. Note: ms.AppRole is a read of WHO the +// platform says the caller is — for gating routes by role, prefer the +// declarative RequirePermission middleware: // // if ms.AppRole(r.Context()) == auth.RoleAdmin { ... } // -// Do NOT read the auth.HeaderAppRole (X-MS-App-Role) header instead: the -// deployed Lambda shim STRIPS every client-settable X-MS-* identity header -// before the router runs, so a header read works under the dev tunnel and -// silently breaks deployed (empty value / rejected request) — the exact bug -// class shipped in ms-app-modules#30. Those headers are the platform-to-SDK -// wire, not a module identity API. -// // "" is a legitimate value: internal/system/cron/task invocations carry no // user role. Under the local-dev bypass (no secret configured) it returns the // synthetic auth.RoleAdmin.