Repository navigation
fix(plugin-security): the packaged-permission-set lock refusal carries its guidance as userMessage - #21902
Conversation
…s its guidance as userMessage PackagedPermissionSetLockedError (insert and update) and the fail-closed PackagedPermissionSetProvenanceUnknownError now declare a readonly userMessage: the guidance addressed to the end user, read at every HTTP door through declaredUserMessage and rendered verbatim by the console in place of its generic 403 sentence. code, status and message are unchanged; the texts carry no set name, package id or API path. The wire-envelope pin drives the real data-door write-through and maps the thrown error through mapDataError (the REST /data door's own call) and resolveThrownHttpError (the dispatcher's), plus the metadata door's registered lock gate. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011K3zqE8Pv1Evw5hc8tZCnN
📓 Docs Drift CheckThis PR changes 5 package(s): 3 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 4 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 148 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 3bc96f852a74cf11f67b1914576a76c9529c51ac && git checkout 3bc96f852a74cf11f67b1914576a76c9529c51ac
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin e6dc7a240617eaeef9a64e788bf6e5561c107f1b 85952735ab45baa14540650a7ee43a776e1d3336 && git checkout -B drift-repro e6dc7a240617eaeef9a64e788bf6e5561c107f1b && git merge --no-ff 85952735ab45baa14540650a7ee43a776e1d3336
node scripts/docs-audit/affected-docs.mjs --json e6dc7a240617eaeef9a64e788bf6e5561c107f1b
|
Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011K3zqE8Pv1Evw5hc8tZCnN
…platform refusal does Six comments said platform and driver code never set a thrown userMessage. The packaged-permission-set lock's refusals now do, with static guidance and no host state. Each comment now states the invariant that holds: only a producer authoring end-user text with no host state sets it (an application hook, or a platform refusal carrying static guidance); platform and driver diagnostics never do. Comment-only: each file's comment-free TypeScript print is byte-identical before and after, and the ApiErrorSchema.userMessage describe() text is unchanged. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011K3zqE8Pv1Evw5hc8tZCnN
|
CI note from
Generated by Claude Code |
Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011K3zqE8Pv1Evw5hc8tZCnN
|
CI note from
Generated by Claude Code |
Contract reviewServed-tier: ① Derived judgmentsInputs read: card #21794 (body and all eight comments: triage
Check-runs on the head, latest run per check name, as read at 2026-10-05T19:56Z:
② Semver level
③ Boundary flags
Implemented-by: VERDICT: PASS |
Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011K3zqE8Pv1Evw5hc8tZCnN
…licit system opt-in (objectstack-ai#21939) Fixes objectstack-ai#21912 Clause-②: no This PR is a slice of objectstack-ai#21908, the closure of the security middleware's principal-less hand-off (ADR-0096). It covers the identity and runtime producers. objectstack-ai#21908 stays open for the deny, which lands last. ## What moved Each producer below reached the data engine with no principal and no `isSystem`. That is the hand-off, and it is not an authorization. Each one now takes the explicit system opt-in that already exists. ⛔ No new elevation API, no change to what any door authorizes, no accept-set change. | Row | Position (function) | Engine calls | Route taken | |---|---|---|---| | 17 | `plugin-auth` `auth-plugin.ts`, the platform-admin OAuth client toggle route (`/admin/oauth2/toggle-disabled`) | `findOne` + `update` `sys_oauth_application` | `withSystemContext`, the wrapper better-auth's adapter already writes these rows through | | 18 | `plugin-auth` `scim-connection-service.ts` `verifyScimBearerToken` | `findOne` `sys_scim_connection_credential` | `isSystem: true` in the read's trailing options | | 19 | `plugin-auth` `auth-manager.ts` `organizationHooks.beforeUpdateOrganization` (the slug guard) | `findOne` `sys_organization`, `find` `sys_environment` | `withSystemContext` | | 21 | `runtime` `http-dispatcher.ts` `enforceProjectMembership` | `find` `sys_environment_member` | `isSystem: true` as the read's query context | **Row 20 moves nothing.** I read every site, and each one already runs with the opt-in: - `adopt-membership.ts` `adoptExistingMembership`: its only caller hands it the adapter's `withSystemContext` engine. - `membership-ended-session.ts` `endSessionClaimsForEndedMembership`: all four calls pass `{ context: SYSTEM_CTX }` (`isSystem: true`) as the trailing options, and the engine honours that argument on reads and writes. - The `auth-manager.ts` insert helper (`settleSelfRegistrationGrant`, with `findPermissionSetRows`): it reads and writes through `withSystemReadContext`, the deprecated alias of `withSystemContext`. The `auth-manager.ts` edit (row 19) was made after objectstack-ai#21872 landed, on a merge of `origin/main` that contains it. ## Measured: no gate fires on any moved call today An `isSystem` context short-circuits the gates the hand-off still runs before `next()`: package-managed, system-row, curated-capability, audience-anchor, engine-owned and delegated-administration. A move is neutral only if none of them fires on the producer's calls. - **Static.** Each gate is keyed to objects and verbs that none of these calls touch. The first four guard writes to `sys_permission_set`, `sys_position`, `sys_capability` and `sys_position_permission_set`. Engine-owned needs a `userId`. Delegated-administration guards writes to the RBAC link tables, `sys_permission_set` and `sys_member`. Rows 18, 19 and 21 are reads. Row 17 writes `sys_oauth_application`, which none of the gates names. - **Instrumented.** I added a local, uncommitted probe in `security-plugin.ts`. It recorded each principal-less, non-system context that reached the hand-off, with its stack, and each gate refusal of such a context. Over every run below it recorded **0 gate refusals**. Every call of the card's functions reached the hand-off, so no gate had stopped it. Per function, before → after (records at the hand-off): | Function | dogfood subset | dev boot | runtime harness | |---|---|---|---| | toggle route (row 17) | 5 → 0 (`findOne` 3, `update` 2) | 5 → 0 | — | | `verifyScimBearerToken` (row 18) | 0 → 0 | 1 → 0 | — | | `beforeUpdateOrganization` (row 19) | 0 → 0 | 1 → 0 (`sys_organization` `findOne`) | — | | `enforceProjectMembership` (row 21) | — | — | 2 → 0 | | row 20 functions | 0 → 0 | 0 → 0 | 0 → 0 | | all records | 1601 → 1596 | 300 → 293 | 39 → 37 | Before = the base tree with the probe. After = the change with the probe: the dev boot and the harness on the final tree (`9878b925`), and the dogfood subset on the pre-merge commit `4239dd47`, whose row 17 code is the same. The boot's background ticks (the outbox claims) make the totals differ by a few records between runs. The per-function counts are the reading. - **Dogfood subset.** Seven files that reach the card's functions: the two platform-admin route sweeps, the organization-update door, the two SCIM-enabled suites, org-admin reach and membership attribution. 57 tests passed both times. Only row 17 appears in the dogfood suite. This subset reproduces the full-suite census of the measure-first round (`6003676228`) for these rows exactly. - **Dev boot.** `pnpm dev -- --fresh` on showcase with SCIM enabled, driven as the seeded admin. It registers an OAuth client, toggles it twice, toggles a missing id, sends a SCIM request with an unknown bearer, and changes the default organization's slug. The answers were identical before and after: register 201, toggles 200 / 200 / 404, SCIM 401, slug update 200. - **Runtime harness.** A scratch file, deleted afterwards, booted a real engine with `SecurityPlugin` and called `enforceProjectMembership` for a member and a non-member. No open-source composition reaches row 21: no `KernelResolver` sets `environmentId`, and `sys_environment_member` is a cloud control-plane object. The answers were `null` and 403 both times. - **Restored.** The probe was reverted (`security-plugin.ts` blob `5b4ab280` equals HEAD), `plugin-security` was rebuilt, and `ablation-dist-preflight --absent` confirms the marker is gone from `dist/`. The positive control: 4 hits in `dist/` while the probe was live. **One difference that is not a gate (row 17).** Under the hand-off, the engine's static read-only strip ran on the toggle's `update` and dropped the `updated_at` the route supplies, with a WARN. Under `isSystem` the strip does not run. I compared the stored rows: on the SQL driver, both paths store `disabled` and an `updated_at` equal to the driver's own stamp. The only change is that the WARN line no longer appears on each toggle. ## Pins (one per package) and ablations - `plugin-auth/src/principal-less-producers-system-context.test.ts`: a real engine with a context-recording middleware. The toggle route's `findOne` and `update` and the SCIM probe's `findOne` are `isSystem`. The route still answers 200 and flips the stored flag, and still answers 404 `RESOURCE_NOT_FOUND`. The verifier still resolves a known bearer to its connection, and still answers `null` for an unknown one. - `plugin-auth/src/auth-manager.org-slug-guard-system-context.test.ts`: both slug-guard reads are `isSystem`, and the guard still refuses with `FORBIDDEN` / 403 while an active environment exists. **On an engine that refuses a principal-less, non-system context, the guard still refuses.** The pre-existing catches are pinned as they stand: a read that throws ends the hook without refusing. - `runtime/src/http-dispatcher.membership-system-context.test.ts`: the membership read is `isSystem`, a member passes, and a non-member gets 403 `PROJECT_MEMBERSHIP_REQUIRED`. **On an engine that refuses a principal-less, non-system context, the non-member is still refused.** The pre-existing fail-open catch is pinned as it stands: a read that throws lets the request through. - **Ablations.** Each went through `scripts/ablation-replace.mjs`: the anchor hit once, the mutation was verified on disk, and the restore was proven (blob equals HEAD, `git diff HEAD` empty). Each pin imports its subject from `src`, so no build sat between the mutation and the run. - A, row 17, `withSystemContext` dropped: 2 red. - B, row 18, trailing context dropped: 2 red. - C, row 21, query context dropped (re-run on `9878b925`): 3 red, including the refusing-engine non-member case. - D, row 19, `withSystemContext` dropped: 2 red, including the refusing-engine case. ## Tests and gates (at `9878b925`) - New pins, on `9878b925`: plugin-auth 2 files, 9/9 passed. runtime 1 file, 5/5 passed. - `pnpm --filter @objectstack/runtime exec vitest run --project local --maxWorkers=2` on `9878b925`: 330 files, 4654 passed, 19 skipped. `pnpm --filter @objectstack/runtime run typecheck`: exit 0. - plugin-auth on `60c5f22c`: the suite (126 files, 2607 passed, 10 skipped) and `run typecheck` (exit 0). The only commit since, `9878b925`, touches `runtime` and the changeset, and plugin-auth imports neither. - **The runtime suite caught a spelling of mine.** The membership read first carried its context as a trailing third argument. Eight existing assertions read the read's two arguments: `toHaveBeenCalledWith` in `http-dispatcher.test.ts` and `http-dispatcher.membership-skip-boundary.test.ts`. They turned red. The context now rides inside the query instead. The opt-in is the same and so is the engine's reading (ObjectQL merges the two), and both suites pass unedited. - `eslint --no-inline-config` over the 7 changed `.ts` files: 7 files, 0 errors, 0 warnings. These 7 are the whole population whose lint verdict this diff can move. `eslint.config.mjs` never enables type-aware linting (no `parserOptions.project`, no typed rules), so no untouched file's verdict can change. The repo-wide `pnpm lint` is CI's. - `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` on `9878b925` derived 98 commands. All 98 ran and exited 0. The `--ran` reconciliation over the exit-coded record reads: "98 derived famil(ies) accounted for — 98 run, 0 NOT-MEASURED (a DERIVED zero — all 98 recorded an exit code and none of them is 3)". The same 98 also ran green on `60c5f22c`. - The branch sits 3 commits behind `origin/main` (`9dce6353`). Those commits touch `content/docs/permissions/sso.mdx` and a `rest` test, none of this diff's files. objectstack-ai#21902 is merged into `main` and contained in this branch. ## Acceptance notes - **The fail-open catches on rows 19 and 21 are unchanged.** They are pre-existing, and row 21's is documented as deferred. This PR removes the path by which a principal-less deny would trip them: both reads are now `isSystem`. A read that throws for any other reason still skips the slug guard (row 19) or opens the membership gate (row 21). Both behaviours are pinned as they stand, so the seat can sequence them before the deny. - **Row 19 in the open-source composition.** `sys_environment` is not registered there, so the environment read throws before it reaches the engine middleware. The catch then ends the hook, and the slug guard never refuses in an open-source deployment. It acts only where the object exists. Measured on the dev boot: the slug change answered 200 and recorded no environment read at the hand-off. - **NOT MEASURED: a cloud composition.** An `isSystem` read also bypasses any host read hook keyed on the caller, such as a control-plane org-scope hook. I measured the six named gates only, and only in-repo. - `mintScimConnectionCredential` inserts without the opt-in. It has no runtime caller (tests only) and is not exported from the package entry, so nothing produces through it today. Noted, not changed. - The census page (`content/docs/permissions/system-context.mdx`) is current. `--fix` moved its held declaration count from 25 to 26, for the new trailing-options type on the SCIM probe. It asked for no anchors. --- _Generated by [Claude Code](https://claude.ai/code/session_011K3zqE8Pv1Evw5hc8tZCnN)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
Fixes #21794
Clause-②: yes (widening)
The packaged-permission-set lock refusal now carries its guidance as
userMessage. The console renders that field verbatim and keeps its generic "You don't have permission to save this record." only for refusals that carry none, so an admin who edits a packaged set in Setup is now told to clone it.What changed
packages/plugins/plugin-security/src/packaged-permission-set-lock.tsonly.PackagedPermissionSetLockedErrordeclaresreadonly userMessage: string, set per operation. An edit (update) is told to clone the set with the Clone action and edit the clone. A new set named like a packaged one (insert) is told to choose a different name, or clone.PackagedPermissionSetProvenanceUnknownError, the fail-closed sibling in the same file, gets the same member: try again, or clone. It was measured to lose its guidance the same way (see Tests). The unreadable source'sreasonstays inmessage.code(NOT_OVERRIDABLE),status(403) andmessageare byte-identical for both classes. Nothing the lock refuses or accepts moves. No other refusal class gains auserMessage, and no package entry gains an export: the three texts are module-private constants.messagekeeps that diagnostic for logs and developers. The texts are English, like every platform refusal: no producer in this repo localizes a thrownuserMessage, and none is invented here.A
minorchangeset for@objectstack/plugin-securitydeclares the widened published type.Measured on a booted showcase (scratch probe, not committed)
bootStack(showcase), admin token, the setshowcase_contributorthatcom.example.showcaseships.607463d7)4bf10901)PATCH /api/v1/data/sys_permission_set/:id{description}error, code, object,codeNOT_OVERRIDABLE, nouserMessageuserMessage(edit guidance)POST /api/v1/data/sys_permission_set{name: showcase_contributor}userMessageuserMessage(insert guidance)PUT /api/v1/meta/permission/showcase_contributorerror, code, the lock's own sentence, nouserMessageuserMessage(edit guidance)/dataflat dialect (objectsibling), somapDataErrorin the PATCH and POST handlers' catch is the mapping that serves it.Tests
New
describeblock inpackaged-permission-set-lock.test.ts, 5 cases. Each drives the real write door and maps the thrown error through the producer's own mapping:mapDataError(the REST/datadoor's call) andresolveThrownHttpError(the dispatcher's resolution). Every case assertsstatus403,codeNOT_OVERRIDABLE and theuserMessage. The text is not pinned word for word. What is pinned is the guidance (clone; for insert, a different name; for the sibling, try again) and the absence of the set name, package id, object name, API path and the sibling's diagnostic reason.userMessage.userMessageas the data door's update.Runs, all through
scripts/pm/os-verify-lock.sh:5 failed | 22 skipped, eachuserMessage must be present on the wire: expected 'undefined' to be 'string'. A first draft passed two cases vacuously (undefined equal to undefined); they now assert presence first.4bf10901af: the four lock suites (packaged-permission-set-lock,-lock-gate,-restore-leg,permission-set-duplicate-name-refusal)45 passed. Whole package:167 passed (167)files,3620 passed | 45 skipped.pnpm --filter @objectstack/plugin-security typecheckexit 0, includingcheck:test-typecheck(test layer compiles, 0 errors).4bf10901af, each throughscripts/ablation-replace.mjs(anchor hit once, blob changed on disk, restore provenblob == HEAD 483e5e60andgit diff HEADempty). The suite imports the module from source, so nodistis involved.userMessageassignment:4 failed | 23 passed. The four locked-class cases go red on presence; the sibling stays green.1 failed | 26 passed, only the sibling case.userMessageto itsmessage:4 failed | 23 passed,userMessage must not name 'ehr_quality_inspector'.1 failed | 26 passed,userMessage must not name 'unknown_provenance'.Gates, at
4bf10901afnode scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstackderived 66 commands from the 3 changed paths. All 66 ran, plus the dispatch list's 4packages/specaudits that fall outside this derivation: 70 of 70 exit 0.check:dual-build-cjs-loadsfirst exited 3 (PREREQUISITE NOT MET: 8 packages had nodist/). After those 8 were built (41 of 41 turbo tasks, all cache hits) it exited 0, and that is the recorded code.--ran:66 derived, 66 run, 0 NOT-MEASURED, 0 UNRUN.origin/main(1e18a0735c) and that one of its inputs changed there:scripts/engine-double-contract.pinned.jsongained a pin for ametadata-protocoltest file. None of those 3 commits touches a file this diff touches, and this diff adds no engine double.eslint --no-inline-config --format jsonover the 2 changed source files reports 2 files, 0 errors, 0 warnings. Population:eslint.config.mjsmatchespackages/**/*.{ts,tsx,mts,cts}. Invariance: the config never enables type-aware linting (noparserOptions.project), so this diff cannot move the verdict on a file it does not touch. The fullpnpm lintis CI's.Acceptance notes
userMessage:packages/spec/src/api/contract.zod.ts(theApiErrorSchema.userMessageTSDoc),packages/types/src/data-error-classification.ts(withDeclaredUserMessage's note),packages/rest/src/rest-server.ts(the share door's note),packages/runtime/src/http-dispatcher.ts(the PERMISSION_DENIED arm),packages/runtime/src/sandbox/quickjs-runner.ts(SANDBOX_ERROR_PASSTHROUGH's rationale) and the header ofpackages/runtime/src/http-dispatcher.permission-denied-user-message.test.ts. Platform code now sets it. The property those sentences protect still holds: the marked text is authored for the end user and carries no host state, so a sandboxed body that catches this refusal receives static guidance. The new pin holds that property for these texts. This PR's dispatch rules outspec,typesandrestedits, so the six sentences are left for the owning seat; content/docs has no sentence this makes false (searched foruserMessage,NOT_OVERRIDABLE, the clone path and the console's generic sentence).saveMetaItemruns the metadata protocol's package door (refusePackagedBaseOverride) before the authoring gate, and that refusal (NOT_OVERRIDABLE, nouserMessage) answers aPUTon a code-shipped permission set first. It is unchanged here, and it is a reading of the source, not a measurement at a booted environment kernel. The data door throws this lock's class on every kernel.userMessageverbatim, so a non-English admin sees English guidance where they saw a localized generic sentence before. No localization path exists for a thrownuserMessage.Seat's append: patch round 1 (written by
domain:servicesseat 1 from the dev's report6000532525; the dev does not edit this body)7c2b636d8b, comment-only). Six comments said platform code never setsuserMessage: theApiErrorSchema.userMessageTSDoc (packages/spec/src/api/contract.zod.ts), thewithDeclaredUserMessagenote (packages/types/src/data-error-classification.ts), the share-door note (packages/rest/src/rest-server.ts), thePERMISSION_DENIEDarm (packages/runtime/src/http-dispatcher.ts), theSANDBOX_ERROR_PASSTHROUGHrationale (packages/runtime/src/sandbox/quickjs-runner.ts) and the header ofpackages/runtime/src/http-dispatcher.permission-denied-user-message.test.ts. Each now states the invariant that holds: only a producer that authors end-user text with no host state sets it (an application hook, or a platform refusal carrying static guidance such as the packaged-permission-set lock's); platform and driver diagnostics never do. For each file the comment-free TypeScript print is byte-identical before and after, the.describe()text is unchanged, andpnpm --filter @objectstack/spec run check:generatedreports all 15 generated artifacts up to date. The seat decided this in its verdict5999760387; the cross-lane declarations are [PM seat] domain:spec — 🟢 os-litant · session_01LAi5BVvQNiYzepSAcsoFLK #60175999766898and [PM seat] domain:cli — 🟢 os-warren · session_01RWZbGvPFcRKvUqASZtunCU #60245999776957.origin/mainmerged at1e18a0735c(merge commitb8da8b3e1e, no conflicts) before the amendment.userMessage, and none was invented.Generated by Claude Code