feat(sdk): ms.UserID/ms.AppRole trusted context accessors#137
Merged
Conversation
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The deployed Lambda shim strips every client-settable
X-MS-*identity header before the router runs — trusted identity rides the typed invoke payload into ctx. But the facade only exposedms.AppID, so a module needing the user id or app role fell back to readingauth.HeaderUserID/auth.HeaderAppRoleoff the request. That works under the dev tunnel (middleware promotes validated headers) and silently breaks when deployed: the header reads come back empty. This exact bug shipped in mirrorstack-ai/ms-app-modules#30 — oauth-google bound a broker assertion tor.Header.Get(auth.HeaderAppID)and every deployed callback was rejected.Fix
Read-side only — both ingestion paths (envelope inject via
runtime.InjectResourcesand middleware header ingestion) already populate all three identity fields.UserID(ctx)/AppRole(ctx)beside the existingAppID, same nil-checkedauth.Getpattern, not module-bound so they work pre-Init.ms.UserID/ms.AppRolebesidems.AppID, with docs naming the ctx accessors 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).Header*const block and package doc re-documented as the internal platform-to-shim/tunnel wire, not a module identity API. Constants stay exported — middleware ingestion, dev schema middleware, and the dispatch wire contract depend on them for tunnel ingestion.Verification
New regression tests in
identity_test.go:TestUserID/TestAppRole— accessor semantics, including nil-safe empty returns with no identity in ctx.TestIdentityAccessors_EnvelopeInjectPath— all three accessors resolve via the envelope-inject path (runtime.InjectResources).TestIdentityAccessors_PlatformAuthHeaderPath/TestIdentityAccessors_RequireProxyHeaderPath— both middleware header-ingestion paths (PlatformAuth, RequireProxy validated-token).TestIdentityAccessors_DeployedPath_HeadersStrippedCtxResolves— the design: ECS multi-tenant credential injection #30 footgun pinned end-to-end: throughNewLambdaHandlerthe rawX-MS-*headers read""while the ctx accessors resolve the typed payload identity.Post-amend in /Users/owo/Documents/MirrorStack-AI-V2/app-module-sdk-wt-identity: gofmt -l . empty; go build ./... OK; go vet ./... OK; go test ./... ALL ok — root package re-ran fresh (0.472s) after the doc edits, remaining 16 test packages green (auth, cache, db, i18n, internal/core, internal/ids, internal/lambdaenv, internal/migration, internal/refcache, internal/registry, internal/runtime, internal/taskenv, meter, roles, storage, system; 3 packages have no test files). All new identity tests pass, including the #30 footgun pin (headers stripped → "" while ms.UserID/ms.AppID/ms.AppRole resolve typed payload identity).
🤖 Generated with Claude Code