feat(plugin-security): extend the packaged-permission-set lock to the restore leg of the write-through - #13409
feat(plugin-security): extend the packaged-permission-set lock to the restore leg of the write-through#13409os-elon wants to merge 2 commits into
Conversation
… restore leg of the write-through The restore leg now consults assertPermissionSetNotPackageDeclared before re-authoring a restored record's definition into metadata. A packaged (or unknown-provenance, fail-closed) name has its mint refused BEFORE the metadata write and the refusal reported loudly on the durability channel; the engine un-trash stands (the leg is a deliberate post-pass and never throws). The #11725 MEASURED RESIDUAL tripwire is inverted in the same change, plus a non-packaged control and a fail-closed case. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012WkdHQwHr2KQmaX7P1BHzi
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012WkdHQwHr2KQmaX7P1BHzi
📓 Docs Drift Check1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run. 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 30f31043c4953aea44e11d1352bf29e778471bf4 && git checkout 30f31043c4953aea44e11d1352bf29e778471bf4
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 2be4a67299e2b3b9e28e28b1c0aff22ebda4422f 4f30453f23196b80ade34e86bd0c804a2252365c && git checkout -B drift-repro 2be4a67299e2b3b9e28e28b1c0aff22ebda4422f && git merge --no-ff 4f30453f23196b80ade34e86bd0c804a2252365c
node scripts/docs-audit/affected-docs.mjs --json 2be4a67299e2b3b9e28e28b1c0aff22ebda4422f |
Pull request was converted to draft
|
Director seat (session Generated by Claude Code |
Fixes #12020
Extends the 2026-08-24 "lock the base, clone to customize" refusal (#11513 / PR #11702) to the
restoreleg ofcreatePermissionSetWriteThrough— the one write point that did not consult it — and inverts the #11725 MEASURED RESIDUAL tripwire in the same PR, exactly as that case's own comment demanded.The design question, answered with measurements
The save-door refusal cannot simply be repeated here: PR #11702's author's reason stands — a throw after the record is already restored strands the caller with a healthy-looking row, misreports a completed engine operation, and half-applies a multi-row restore. Three candidate refusal points were on the table; the choice is argued, not preferred:
1. Refuse before the un-trash — rejected, because the read it needs does not exist. The hook position exists trivially (the middleware body before
next()), but the fact it needs does not:resolveTargetRowsreads through ordinaryfind, no trash state has existed since #2377 retiredenable.trash, and #3146 (the parked recycle bin) will define whether trashed rows are visible to normal reads — the natural answer is that they are not. A pre-pass would resolve nothing on a real engine while the unit doubles (which have no trash concept — rows are always visible) showed it green: the inverted tripwire would certify coverage the real path would not have. And a fail-closed refusal for the "targets did not resolve" case would block everysys_permission_setrestore including org-owned rows — forbidden by this card's fork condition.2. Refuse after and compensate (re-trash) — rejected, because the compensation is structurally unavailable. This middleware's own delete leg guarantees a packaged row's record survives the data door (the overlay tombstones, the driver delete never runs — the FENCE case pins it), so a compensating re-trash through
qlcannot happen; the middleware holds no raw driver handle; and the trash primitive itself belongs to unlanded #3146. A compensation that can fail turns the refusal into a lie: the caller is told "refused" while the state says "restored".3. Refuse the mint, report on the durability channel — chosen, and it is stronger than the "weakest guarantee" framing suggests. The three-way framing conflates two different writes. The write the ruling locks is the metadata MINT (
saveMetaItem): hatch open (OS_METADATA_WRITABLE=permission), it authors an environment overlay of a packaged set, andreconcilePermissionSetProjectionre-projects that overlay onto the record on every boot, forever — that is the silent fork. The engine un-trash is not that write. Relative to the mint, this change has the same "refuse BEFORE the write" property the insert and update legs have: the lock is consulted per row after the un-trash and beforesaveMetaItem, the mint is skipped, and the refusal is reported loudly through the #9754 error-then-warn chain — the same durability channel this leg already owns, and the disposition the neighbouring pinned case inpermission-set-projection.test.ts("RESTORE reports a refused re-author on the durability channel instead of throwing") already fixes for this leg. Relative to the un-trash it is acceptance-plus-loud-report — honestly, because with no overlay minted, boot reconciliation re-projects the DECLARED body onto the restored record: the environment converges to the package truth rather than to a fork. The restored record's presence is the whole residue.One spelling throughout: the leg calls
assertPermissionSetNotPackageDeclared— the same assertion, classifier, and error classes as the insert/update legs and the metadata-door gate — passingupdateas the operation for the same reasonpackaged-permission-set-lock-gate.tsdoes (the remedy the refusal teaches is the clone path). The refusal wording on the channel is distinct from the failed-write wording (#5240 — one condition, one wording).Scope discipline (the fork condition)
unknownfor the stated reason (accepting on a failed read is the one guess a write door must not make, and would reopen the hatch-open gap on every transient). In practice the delta is small: when no provenance source answers, the metadata layer is typically unwritable too. Pinned by the FAIL-CLOSED case.Dispatch assumptions, re-measured on this tree
DISPATCHED_OPERATIONSinengine-middleware-operation-vocabulary.test.tsis the 7-member union with the destructive lifecycle verbs (purge/transfer/restore) excluded, andAPI_METHOD_DERIVATION.restore.flagis permanently false (api-derivation.ts:143). The leg stays unreachable until Implement soft delete (recycle bin):enable.trash/softDeleteare spec-only with zero runtime readers #3146; this PR is the coverage landing while it is cheap.insert/update/delete/restore; the lock was called at insert and update only. Thedeleteleg needs no lock — deleting a packaged set is an ADR-0005 RESET toward the package (overlay tombstone), not a fork, and the FENCE case pins that the driver delete never runs.permission-set-projection.ts:f8701b7b422; mutated blobe71f39f1bfc(lock consultation line removed; marker grep 1 hit, assert-call count 3 to 2).errors.lengthexpected 1 got 0; observed exactly that — 2 failed, 4 passed, first failing assertion "the refusal reached the durability channel: expected +0 to be 1".f8701b7b422,git diff HEADempty, marker grep 0. No build leg required for either side: the tripwire imports the subject relatively from source and vitest transforms the source directly (no dist resolution on this pair).Verification (all at
4f30453f2, the final commit, after merging origin/main)pnpm --filter @objectstack/plugin-security test— 89 files, 1638 tests, all green (verify-lock VERDICT command-exit 0).pnpm --filter @objectstack/plugin-security typecheck— all three tsc programs green, including the test(plugin-security): compile the 89 test files no tsc program read #13395 test-layer program:tsc --noEmit -p tsconfig.test.json --listFilesreads 89 test files and the edited tripwire file is among them (1 hit). The sequencing hazard the dispatch named is closed: PR test(plugin-security): compile the 89 test files no tsc program read #13395 landed on main during this branch's life; origin/main was merged and the gate run repeated on the merged tree.node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack, derived at4f30453f2): all 29 path-matched families plus the convention-triggered set (query-options-erasure, type-check-coverage, type-check-debt with--re-measureon the built closure, engine-double-contract, cross-package-test-inputs, where-matcher, i18n, i18n-stale-fill) and check:nul-bytes — 34 gates exit 0, exit codes captured before any pipe.check-test-completeness.mjs(no CI shard artifacts locally) andpm/check-half-states.mjs(no board access locally). Neither is a red.pnpm lint(eslint . --no-inline-config) — exit 0 under the verify lock.Review posture
Clause ② is YES (inherited from the 2026-08-27 seat, not re-graded): this PR adds a refusal where none exists today, which is contract accept/reject behaviour. This seat dispatches below
CONTRACT_REVIEW_TIER: the PR stays a DRAFT, parked for aCONTRACT_REVIEW_TIERreviewer;needs:contract-reviewre-attaches on the card now that a reviewable diff exists (per the 2026-08-28 director-seat note there). Not armed for auto-merge; not flipped ready.Refs: #11725 (the probe and the tripwire) / #11702 + #11513 (the ruling and the lock) / #11843 (metadata-door registration, a different door, unaffected) / #3146 (stays parked; the day it lands, this lock is what stands between a trashed packaged row and a silent fork) / #13395 (test-layer tsc program, merged in).
Generated by Claude Code