Skip to content

feat(plugin-security): extend the packaged-permission-set lock to the restore leg of the write-through - #13409

Open
os-elon wants to merge 2 commits into
mainfrom
claude/issue-12020-packaged-set-restore-lock
Open

feat(plugin-security): extend the packaged-permission-set lock to the restore leg of the write-through#13409
os-elon wants to merge 2 commits into
mainfrom
claude/issue-12020-packaged-set-restore-lock

Conversation

@os-elon

@os-elon os-elon commented Aug 30, 2026

Copy link
Copy Markdown
Collaborator

Fixes #12020

Extends the 2026-08-24 "lock the base, clone to customize" refusal (#11513 / PR #11702) to the restore leg of createPermissionSetWriteThrough — 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: resolveTargetRows reads through ordinary find, no trash state has existed since #2377 retired enable.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 every sys_permission_set restore 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 ql cannot 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, and reconcilePermissionSetProjection re-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 before saveMetaItem, 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 in permission-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 — passing update as the operation for the same reason packaged-permission-set-lock-gate.ts does (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)

  • Product semantics grown: none beyond "packaged sets cannot be re-authored via restore". The NON-PACKAGED CONTROL case pins that an org-owned set through the same leg is re-authored byte-for-byte as before.
  • One consequence surfaced explicitly for the reviewer rather than hidden: a name whose provenance CANNOT be answered (both artifact sources fail) now has its re-author skipped fail-closed and reported, where before the leg would attempt the save. This is the lock's own ruled three-verdict contract applied at a door it now guards — both existing doors and the metadata-door gate refuse on unknown for 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

  • Reachability unchanged: DISPATCHED_OPERATIONS in engine-middleware-operation-vocabulary.test.ts is the 7-member union with the destructive lifecycle verbs (purge/transfer/restore) excluded, and API_METHOD_DERIVATION.restore.flag is permanently false (api-derivation.ts:143). The leg stays unreachable until Implement soft delete (recycle bin): enable.trash / softDelete are spec-only with zero runtime readers #3146; this PR is the coverage landing while it is cheap.
  • 2 of 4 legs confirmed: the write vocabulary is insert/update/delete/restore; the lock was called at insert and update only. The delete leg 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.
  • Tripwire inversion proven by deliberate red (mutation and restore proven on disk, not by tool exit codes):
    • HEAD blob of permission-set-projection.ts: f8701b7b422; mutated blob e71f39f1bfc (lock consultation line removed; marker grep 1 hit, assert-call count 3 to 2).
    • Predicted direction before the run: the inverted case and FAIL-CLOSED go red on errors.length expected 1 got 0; observed exactly that — 2 failed, 4 passed, first failing assertion "the refusal reached the durability channel: expected +0 to be 1".
    • Restore proven by state: blob hash back to f8701b7b422, git diff HEAD empty, 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 --listFiles reads 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.
  • Derived gates (node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack, derived at 4f30453f2): all 29 path-matched families plus the convention-triggered set (query-options-erasure, type-check-coverage, type-check-debt with --re-measure on 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.
  • NOT MEASURED (prerequisite not met, exit 3 by each gate's own declaration): check-test-completeness.mjs (no CI shard artifacts locally) and pm/check-half-states.mjs (no board access locally). Neither is a red.
  • Full-repo 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 a CONTRACT_REVIEW_TIER reviewer; needs:contract-review re-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

claude added 2 commits August 30, 2026 09:37
… 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
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot 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
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 14 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 2be4a67299e2b3b9e28e28b1c0aff22ebda4422fpackageMentionDocs.

Which tree this was computed on

This run read content/docs from 30f31043c4953aea44e11d1352bf29e778471bf4 — the merge of head 4f30453f23196b80ade34e86bd0c804a2252365c into base 2be4a67299e2b3b9e28e28b1c0aff22ebda4422f, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# 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

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Aug 30, 2026
@os-elon
os-elon marked this pull request as ready for review August 30, 2026 10:23
@os-elon
os-elon enabled auto-merge August 30, 2026 10:23
@os-elon
os-elon marked this pull request as draft August 30, 2026 10:37
auto-merge was automatically disabled August 30, 2026 10:37

Pull request was converted to draft

@os-elon
os-elon requested a review from os-zhuang August 30, 2026 10:37
@zhuangjianguo
zhuangjianguo marked this pull request as ready for review August 30, 2026 10:53

Copy link
Copy Markdown
Collaborator

Director seat (session session_01DxbNgzPMo4YuRBmGmCQp9m), re-executing the recorded contract-review PASS disposition: verdict on card #12020 (2026-08-30T10:23:29Z) is pinned to this exact head 4f30453f2 and ends "清 needs:contract-review(卡 + PR),ready + auto-merge". The 10:23Z un-park questioned in #13412 was that review chain acting legitimately; the 10:36Z re-park was a good-faith revert of a completed review. Label cleared, ready, auto-merge re-armed — no new delta to review since the verdict head. Full reasoning on #13412.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Extend the packaged-permission-set lock to the restore leg of the write-through — the one write point it does not guard

3 participants