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..31989c6 100644 --- a/auth/context.go +++ b/auth/context.go @@ -1,4 +1,9 @@ // 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; see the Header* +// docs for why header reads silently break 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..60a93db --- /dev/null +++ b/identity_test.go @@ -0,0 +1,210 @@ +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" +) + +// 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) + } + }) + } +} + +// 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_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", + }, + } + 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) + } + }) + } +} + +// 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..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,8 +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 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 { + 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 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 { + return identity(ctx).AppRole } 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..dc9a7df 100644 --- a/mirrorstack.go +++ b/mirrorstack.go @@ -315,11 +315,55 @@ 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 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) } +// 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 +// 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 +// 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()) +// +// "" 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. 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 { ... } +// +// "" 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