Uh oh!
There was an error while loading. Please reload this page.
fix(security): single-source the request authorization resolver (REST dropped sys_user_role) - #2341
Merged
Merged
Conversation
… dropped sys_user_role) The REST server (@objectstack/rest) and the runtime dispatcher (@objectstack/runtime) each carried their own copy of the request -> ExecutionContext identity/role resolver. They drifted on a security path: the REST copy silently omitted sys_user_role (so custom roles granted via the ADR-0057 D4 platform-RBAC path did not apply over REST), sys_role_permission_set, the owner->org_owner membership normalization, the platform-admin derivation, and the ai_seat synthesis. Fail-closed (legitimate access denied), not an escalation. Root-fix (finishes the extraction the API-key verifier already modelled): a single shared resolver `resolveAuthzContext` in @objectstack/core/security — the lowest common dependency of both rest and runtime. Both entry points now delegate; no parallel copies to keep in sync. - core: resolveAuthzContext + resolveLocalizationContext (complete role/permission aggregation) + a contract test asserting every authorization source is honored. - runtime: resolveExecutionContext delegates (469 -> 138 lines). - rest: rest-server's identity resolver delegates (drops its divergent ~180-line copy). - scripts/check-single-authz-resolver.mjs + a lint CI step: guard against a future duplicate resolver or a dropped delegation. Verified: contract test 7/7; suites green (runtime 444/444, rest 130/130, core green); full turbo build green; ESLint clean; dogfooded — a user granted `contributor` via sys_user_role now resolves that role on the REST data path (previously roles []). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📓 Docs Drift CheckThis PR changes 3 package(s): 30 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
Uh oh!
There was an error while loading. Please reload this page.
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 freeto 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.
What & why
A security/correctness root-fix. The REST server (
@objectstack/rest) and the runtime dispatcher (@objectstack/runtime) each carried their own copy of the request →ExecutionContextidentity/role resolver. On a security path, duplicated logic drifts — and it did:The REST copy silently omitted
sys_user_role(so custom roles granted via the ADR-0057 D4 platform-RBAC path — the only way to assign custom roles, sincesys_member.roleis restricted to owner/admin/member — did not apply over REST), plussys_role_permission_set, theowner→org_ownermembership normalization, the platform-admin derivation, and theai_seatsynthesis.Fail-closed (legitimate access denied / RLS under-applied), not a privilege escalation — but real: RBAC via
sys_user_rolewas a no-op on REST.The most-thorough fix (not a band-aid)
Don't keep two mirrors in sync on security code — collapse to one. This finishes the extraction the API-key verifier already modelled (
resolveApiKeyPrincipalalready lived in@objectstack/coreand was shared by both transports):@objectstack/core/security— newresolveAuthzContext(the complete session + role/permission aggregation:sys_member+sys_user_role+sys_user_permission_set+sys_role_permission_set, platform-admin derivation,ai_seat) andresolveLocalizationContext. Core is the lowest common dependency of bothrestandruntime, so there's no upward-import problem.@objectstack/runtime—resolveExecutionContextnow delegates (469 → 138 lines).@objectstack/rest— the inline identity resolver delegates (drops its divergent ~180-line copy).resolve-authz-context.test.ts) locking every authorization source, and a lint gate (scripts/check-single-authz-resolver.mjs, wired intolint.yml) that fails CI on a future duplicate resolver or a dropped delegation.Verification
check:authz-resolverpasses.contributorviasys_user_rolenow resolves that role on the REST data path (previouslyroles []).🤖 Generated with Claude Code