Uh oh!
There was an error while loading. Please reload this page.
feat(security): lock package-declared permission sets at the save door; clone to customize - #11702
Conversation
…r; clone to customize Maintainer ruling 2026-08-24 (verbatim, untranslated): 「同意 第一步(创业阶段,Salesforce 式)」 — step 1: lock the base, clone to customize. A Studio/API save targeting a package-declared permission set is refused at the server, with a message that names the sanctioned path. Previously the data door translated the write and left the refusal to the metadata protocol's ADR-0005 tier gate — which is exactly what the documented OS_METADATA_WRITABLE=permission operator hatch switches off, so with the hatch open there was no refusal at all and the save minted a sys_metadata overlay of a packaged set that boot reconciliation then re-projected on every boot, forever. Provenance is decided from the engine SchemaRegistry — the one source this plugin already calls "package-declared" — never from a name-keyed page over sys_permission_set, whose unscoped cap truncates into a false "absent" (#11518, untouched here). Fail-closed: a read that cannot answer refuses the save. Not narrowed: ordinary org-owned sets, the activate/deactivate column writes, and a managed_by:'package' row with no artifact behind it (ADR-0094 D5-R's surviving allowRuntimeCreate neighbour). Each is pinned as a control. Existing forks get a detection READING at boot — count and names, warned loudly, reaping nothing. It reads sys_metadata directly rather than `customized`, which is forced false on the exact confounded shape the field report measured. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01APWX2AwT3a4xDcjPCe8bk4
📓 Docs Drift CheckThis PR changes 1 package(s): 19 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: ⛔ 4 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails. What this run could not see
Coarse fallback — 14 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 b9570a0a0538ab5e6a862cc9e66a71e2ba1ffc91 && git checkout b9570a0a0538ab5e6a862cc9e66a71e2ba1ffc91
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 4ceae8ab0b4c6649e0fa6faf591b4307b17c021d 535abd544d3d785a78c6d72a39cc89b8b18f3e69 && git checkout -B drift-repro 4ceae8ab0b4c6649e0fa6faf591b4307b17c021d && git merge --no-ff 535abd544d3d785a78c6d72a39cc89b8b18f3e69
node scripts/docs-audit/affected-docs.mjs --json 4ceae8ab0b4c6649e0fa6faf591b4307b17c021d
|
os-sam
commented
Aug 24, 2026
PM review — substance PASSES; ⛔ held on contract review, not on anything in the work
⭐ The fence I was most worried about was answered structurally — and better than I askedI dispatched: "ensure your provenance check cannot inherit #11518's fail-open… state which read you used and prove why it cannot." The answer is that the lock does not use that read at all. Provenance comes from the engine SchemaRegistry — "an in-memory array: no page, no cap, no And it is proved by two controls rather than asserted:
⭐ Control B is the part that exceeds the brief: it does not merely establish today's behaviour, it goes red if provenance is ever re-routed through a name-keyed page. A future regression into the fail-open cannot land silently. That is the difference between proving a fact and installing a guard. Fail-closed on ambiguity, with three verdicts not two: if every artifact source failed to answer — or none exists — the verdict is ⭐ The trap that would have shipped a subtly wrong lockThe ruling's parenthetical names the target as "a row carrying package provenance", and
So provenance is read from the artifact, exactly as the discard action reads it, and Verified independently by this seatRe-checked with three-dot semantics after first running a two-dot diff that was contaminated by
Discipline worth naming
Sequencing with #11639 — noted, not yet actionableThis PR flags that Stated rather than left to be found — and I am not contesting any of themThe Generated by Claude Code |
os-sam
commented
Aug 24, 2026
PM: docs-drift advisory — one passage worth a look, and the limit of what I could establish
Unlike the other drift advisories this shift, this one is not a clean no-action, because this PR narrows an accept set — so prose describing what an admin can do is the class that can go false. The passage
What is unchanged and still accurate: the overlay-shadow mechanism itself (an existing overlay wins forever, regardless of what the package ships next), the Discard Overlay remedy and its refusal on non-package-declared sets, and the whole Provenance skip branch. This PR changes none of that — which matters, because those are the parts an operator diagnosing a frozen set actually acts on. What may now be incomplete: the parenthetical describing how such an overlay comes to exist. After this PR a Studio/data-door save targeting a package-declared set is refused with
|
os-sam
commented
Aug 24, 2026
Sequencing note closed: PR #11639 has merged, and this still merges cleanly
⇒ No action needed now, and specifically ⛔ no rebase and no premature base update.
Review posture unchanged: still draft, Generated by Claude Code |
Uh oh!
There was an error while loading. Please reload this page.
Fixes#11513
Maintainer ruling of 2026-08-24 (comment 5394202746), recorded verbatim and untranslated:
Step 1 of the mainstream-platform comparison: lock the base, clone to customize. This is the server half; the UI half (lock rendering + Clone button in Studio) is filed separately in objectui and is blocked on this landing.
The hole this closes
createPermissionSetWriteThroughtranslated every non-system data-door write onsys_permission_setinto asaveMetaItem, and deliberately left the refusal to the producer (the metadata protocol's ADR-0005 tier gate) — the disposition recorded inpermission-set-projection.ts's own header under ADR-0094 D5-R.That gate is exactly what the documented
OS_METADATA_WRITABLE=permissionoperator hatch switches off. With the hatch open there is no refusal at all: the save mints asys_metadataoverlay of a package-declared set, andreconcilePermissionSetProjectionre-projects that overlay onto the record on every boot, unconditionally, keyed only on overlay presence — so the fork wins forever, no expiry, no diff, no signal. The card measuredcustomizedreading0for two weeks while an overlay froze a set.The header's disposition is therefore retired in this PR, in the header itself, with the two measured reasons: the producer's gate is switchable off, and its message ("the type has not opted into per-org overlay writes") names nothing an admin can act on.
What lands
insertorupdatetargeting a package-declared set throwsNOT_OVERRIDABLE/ 403 before the metadata write, with a message that names the sanctioned path: clone it.managed_by: 'admin'with nopackage_id(no upgrade linkage), the base row is byte-identical afterwards, no overlay of the base is minted, and the clone itself is freely editable.packaged-permission-set-overlay-detection.ts, wired at boot beside the drift diagnostic. Count and names, warned loudly, and the line says outright that nothing was reaped.⛔ Not chartered, not done: the ServiceNow-style explicit overlay layer (item 4 of the ruling) — no code, no card. ⛔ No reap, merge or migration of existing forks, under any framing: the reading has no write verb and its double exposes none, so a reap added later fails with a
TypeErrorrather than passing quietly.⭐ The fence: the provenance read cannot fail open
The lock turns on one question — is this set package-declared? — so a read that answers "not package-declared" when it merely failed to find out would accept exactly the save this feature exists to refuse.
#11518 (open, unclaimed, ⛔ not fixed here) is that read:
buildExistingByName's unscoped page caplimit: names.lengthtruncates once a name can carry more than one row, and a truncated page reads asabsent.The lock does not use it. Provenance comes from the engine SchemaRegistry — the same source
readDeclared,readDeclaredBodyandpermission-set-overlay-discard.ts's eligibility test already use, so this plugin keeps one spelling of "package-declared". That read is an in-memory array: no page, no cap, no$in, no driver. Two controls prove it structurally rather than asserting it:buildExistingByName's UNSCOPED page cap truncates once a name can have more than one row — and a truncated page reads as "absent", which inserts #11518 truncates on, in a double whosefindhonourslimit(the sibling double inpermission-set-projection.test.tsignores it, so the trap cannot even be expressed there), shows the truncation is live, and pins that the refusal still fires;sys_permission_setfail outright while by-id reads keep working, so the middleware still reaches the question. The verdict is unchanged — only possible if the question was never asked of that table. If provenance is ever re-routed through a name-keyed page, that test goes red instead of silently inheriting the fail-open.Fail-closed on ambiguity:
classifyPackagedPermissionSethas three verdicts, not two. If every artifact source that exists failed to answer — or none exists — the verdict isunknownand the save is refused. (Measured onmain: on that path today the save lands and then raises an unrelated error, so a caller reads a failed save that in fact succeeded.)⭐ What is deliberately NOT narrowed — each pinned as a control
Clause ② applies (the accept set narrows on a write door), so the counter-direction is pinned as hard as the refusal. Pinning only the refusal lets an implementation that refuses everything score green.
{ active }patch is row state, not a customization (permission-set backfill (ADR-0094 D4) 现在 100% 失败:行里的active存储列喂进了 #4001 之后严格化的 permission spec #4669), and a lock that swallowed it would break the surface it protects;managed_by: 'package'row with no artifact behind it keeps editing in place — published through the metadata door (ADR-0070), materialized by the ADR-0086 P2 path. That is ADR-0094 D5-R's survivingallowRuntimeCreateneighbour. The ruling's parenthetical calls the target "a row carrying package provenance", andmanaged_byis the obvious reading of that — but it is measurably the wrong fact in both directions: too narrow (theprovenance_skipshape is a package-declared set whose row'smanaged_bywas never'package') and too broad (the case above). Provenance is read from the artifact, exactly as the discard action reads it.Why this is not the Prime Directive #8 parallel allowlist
The retired header text said pre-empting the producer "would need a second copy of
isArtifactBacked". It does not: the lock reads the engine SchemaRegistry, which is already this plugin's single spelling of the question in three places. It never readsmanaged_by.ENV_PROJECTION_MARKERwas moved into the lock module and imported back by the projection module, so the constant has one home rather than two — the lock decides "packaged" partly by the absence of that marker.readDeclaredis deliberately not reused, and that is a strictness difference rather than a second spelling: it normalizes a failed registry read to[], which is right for a boot seeder and exactly wrong for a write door, where[]reads as "no package declares this" and accepts.The detection reading does not depend on
customizedupsertEnvPermissionSetcomputes the flag asexisting.managed_by === 'package' ? !!customized : false, so on the confounded shape the card measured it is forced false while an overlay really is shadowing the row. A reading built on it would report zero forks on the one environment that had one.sys_metadatais read directly, the way #9952'sdrift_statusoverlay-shadow branch does, and the confounded row is a pin rather than a footnote.⛔ Making
customizeditself correct is not chartered (the card lists it among candidates; the ruling did not take it up) and nothing here reads or writes that column.It is also not the drift diagnostic: drift reports a set whose grants already differ from the artifact, so an overlay that currently happens to match is invisible to it — while that fork is entirely real and freezes the set the moment the package ships its next version.
Test-first, not ablation
Every pin was written before any source edit, on an otherwise-unmodified tree, with the expected failure signature predicted in writing first. No restore in the loop, so no restore could silently fail. No ablation was used anywhere in this change.
Round 1 (write-door pins, importing only existing modules) — measured red:
Test Files 1 failed | 77 passed (78),Tests 6 failed | 1480 passed (1486). The six failures were the six intended ones; every control was green before and after.One prediction was wrong and is recorded as wrong: I predicted pin 5 would fail with "promise resolved instead of rejecting". It did reject — with the harness's own
getMetaItemLayeredthrow escaping uncaught fromprojectPermissionMutationaftersaveMetaItemhad already written the overlay. The direction of the prediction (accepted, not refused) held; the signature did not, and the real behaviour is worse than the predicted one.Round 2 (detection pins) — measured red as
Error: Cannot find module './packaged-permission-set-overlay-detection.js'. That is a collection failure, weaker evidence than a per-test red, and it is recorded as such rather than dressed up.Verification
Union run after the final commit, at
535abd544d:pnpm --filter @objectstack/plugin-security testTest Files 79 passed (79)·Tests 1497 passed (1497)pnpm --filter @objectstack/plugin-security typechecktsc --noEmit, exit 0pnpm lint(repo-wideeslint . --no-inline-config)check:engine-double-contractOK — 400 pinned, 133 in the DEBT ledger, 2 exemptcheck:i18nOK (9 package(s) — all bundles in sync, no undeclared authoring keys)check:nul-bytesOK (scanned 6526 text file(s) … no raw ASCII control bytes)two-doors-permission+showcase-permission-projectionTest Files 2 passed (2)·Tests 13 passed (13)Gates derived with
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack(no path arguments, clean tree, after the final commit); all 17 path-matched families plus the convention-triggered set ran green locally. Exit codes were captured before any pipe throughout.engine-double-contract.baseline.json(shrink-only) is unchanged — verified with an emptygit diff --stat. The two new rows land inengine-double-contract.pinned.json, the coverage ledger that legitimately grows, written by the gate's own--write. The new double routesupdateanddeletethroughassertEngineUpdateDispatch/assertEngineDeleteDispatch.Two prerequisite faults were hit and are reported as not measured, never as reds:
check:i18nfirst answeredPREREQUISITE NOT MET — the workspace CLI is not built … Nothing was checked(re-measured green after building the CLI), and the first dogfood attempt failed 81 files onFailed to resolve entry for package "@objectstack/connector-mcp"(re-measured green after building dogfood's dependency closure).What this does not close, stated rather than left to be found
restoreleg of the write-through still re-authors a restored record's body into metadata without the lock. It runs after the record is already back, where a throw would strand the caller with a healthy-looking row and no way to hear about it — the reason its neighbours catch rather than throw. Reaching it needs a trashed record for a package-declared name, which the delete path does not produce (it resets instead). Left untouched deliberately; naming it here so a reviewer can contest it rather than discover it.sys_permission_setrows land organization-less #8672, cited on the card as "open — org-less materialization, same file", could not be read: it 404s as both an issue and a PR in this repo, with both instruments demonstrably working on other numbers. Treated as unmeasured, not "does not exist" — most likely a cross-repo citation missing its prefix. Nothing in this change needed it.Adjacent in-flight work
security-plugin.tsis also touched by PR #11639 (#11343,plugin-security). The regions are ~100 lines apart and both are purely additive (my import sits besiderunPermissionSetDriftDiagnostics's at the top; my boot call sits beside the drift call at ~3200, theirs is the replay middleware at ~3320).scripts/engine-double-contract.pinned.jsonalso takes an additive row from each, at different sort positions. If either conflicts, the remedy is merge, never rebase, and the ledger is regenerated with the gate's own--write.Review posture
⛔ Draft, and it stays draft. Clause ② is yes — this narrows the accept set on a write door.
CONTRACT_REVIEW_TIERisclaude-fable-5and this seat is below it, soneeds:contract-reviewstays on the card: not flipped ready, no auto-merge, not merged.⛔ Zero
packages/specownership was needed.NOT_OVERRIDABLEis a StandardErrorCode already emitted for this exact condition by the metadata protocol's tier gate — one condition, one vocabulary — so no ledger entry is minted andcheck:error-code-casingis green. The message deliberately does not open with[Security] Access denied, which is a matcher that would re-flatten the code toPERMISSION_DENIEDon the wire (seeerrors.ts).Generated by Claude Code