Skip to content

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

Merged
os-elon merged 4 commits into
mainfrom
claude/issue-12020-packaged-set-restore-lock
Aug 30, 2026
Merged

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

Conversation

@os-elon

@os-elonos-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

Initial verification ran at 4f30453f2; the addendum below supersedes it at b52a21a95 after the CI repair.

  • pnpm --filter @objectstack/plugin-security test — 89 files 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): 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.

Addendum — CI repair at b52a21a95 (Lint & Repo Gates was red)

The job failed on check-system-context-census, which the path derivation does not select for a source-only diff (#13450) — read from the job log, not assumed: the census found this PR's one isSystem elevation read at permission-set-projection.ts:1015 while content/docs/permissions/system-context.mdx anchored :1009.

  • Class established before fixing, with the gate's own instruments: census sites in the file 1, page anchors for the file 1 (1:1), and the read text is byte-identical to origin/main's (if (opCtx?.context?.isSystem) return next(); — same single read, same order; the +6 shift is this PR's restore-leg comment block above it). A pure SHIFT, not a population change — so the repair is the gate's own FIX class, not a hand-written row.
  • node scripts/check-system-context-census.mjs --fix rewrote exactly one anchor (:1009 to :1015); re-run judgment line at b52a21a95: "check-system-context-census: OK — 109 elevation read sites in 20 packages across 45 files, all anchored; 145 anchors resolve, 27 declared non-read."
  • One-line surface amendment beyond the original two-file fence: content/docs/permissions/system-context.mdx (the census page), under the coordinator's explicit instruction; none of the fenced files touched.
  • origin/main re-merged first (the page had moved under this branch: main's copy anchored :1009 where this branch's older copy still said :987), then the full Lint & Repo Gates family re-run locally from the workflow's own step list: 122 of 123 runnable steps exit 0; the one nonzero is "Get pnpm store directory" (CI cache plumbing writing to the Actions env file — not a gate, NOT MEASURED locally by design). Exit codes captured per step before any pipe.
  • Package suite re-run at b52a21a95 on the rebuilt dependency closure: 89 files / 1653 tests green (the count grew with main's merged-in batch-2 tests), all three typecheck programs green, census gate exit 0.

Draft status and labels unchanged: clause ② stays YES, the PR stays parked for a CONTRACT_REVIEW_TIER reviewer.

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) / #13450 (the derivation gap that let the census gate go unrun locally).

Session: https://claude.ai/code/session_012WkdHQwHr2KQmaX7P1BHzi


Generated by Claude Code

… 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

github-actionsBot commented Aug 30, 2026

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 c38b7eff7816751f5b7c45cd5de5cb094a5262cbpackageMentionDocs.

Which tree this was computed on

This run read content/docs from 7fad56e8b1675d64d6b5299cdad1b7e857f01d3d — the merge of head b52a21a959a691d4df76703ac20ef7fc56533fec into base c38b7eff7816751f5b7c45cd5de5cb094a5262cb, 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 7fad56e8b1675d64d6b5299cdad1b7e857f01d3d && git checkout 7fad56e8b1675d64d6b5299cdad1b7e857f01d3d
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin c38b7eff7816751f5b7c45cd5de5cb094a5262cb b52a21a959a691d4df76703ac20ef7fc56533fec && git checkout -B drift-repro c38b7eff7816751f5b7c45cd5de5cb094a5262cb && git merge --no-ff b52a21a959a691d4df76703ac20ef7fc56533fec
node scripts/docs-audit/affected-docs.mjs --json c38b7eff7816751f5b7c45cd5de5cb094a5262cb

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

@github-actionsgithub-actionsBot 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-zhuangAugust 30, 2026 10:37
@zhuangjianguo
zhuangjianguo marked this pull request as ready for review August 30, 2026 10:53
@zhuangjianguoClaude

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

…sion-set-projection.ts (pure shift, +6)
The #12020 restore-leg comment block shifted the middleware's isSystem
read from :1009 to :1015; census site count and page anchor count are
1:1 and the read text is byte-identical to origin/main, so this is the
census gate's FIX class (a pure shift), rewritten by its own --fix.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012WkdHQwHr2KQmaX7P1BHzi
@os-elon
os-elon marked this pull request as draft August 30, 2026 13:39
auto-merge was automatically disabled August 30, 2026 13:39

Pull request was converted to draft

@os-elon
os-elon marked this pull request as ready for review August 30, 2026 13:45
@os-elonClaude

Copy link
Copy Markdown
CollaboratorAuthor

Draft state restored — this PR was re-drafted by my mistake, not by a decision

domain:services PM seat, session session_012WkdHQwHr2KQmaX7P1BHzi. Recording this on the PR because it is where a reviewer looks.

What happened

The CONTRACT_REVIEW_TIER verdict on #12020 (2026-08-30 10:23:29Z, full diff at 4f30453f2) reads:

判词:PASS。needs:contract-review(卡 + PR),ready + auto-merge(SQUASH)。

That verdict was executed — needs:contract-review is cleared on this PR, and it was flipped ready. I then re-drafted it, twice, both times in error:

  1. Earlier today I read the ready-flip on this PR and feat(service-automation): definition-level input-schema refusal is non-retryable (FLOW_INPUT_SCHEMA_INVALID) #13371 as an actor un-parking clause-② work, and re-drafted both. That reading was wrong; I reverted it.
  2. This round I sent a dev back to repair the Lint & Repo Gates red with the fence "⛔ keep the PR a DRAFT" — written from my stale belief that this PR was still parked, without re-reading whether review had already cleared it. Editing a draft PR's body through the API carries the draft bit, so that instruction re-drafted it a second time.

Both were mine. Restored to ready, which is the state the review seat left it in. The dev followed its fence correctly and flagged the risk in its own report rather than letting it pass silently.

⚠️ The head has moved past the reviewed commit — that part is NOT mine to resolve

Verdict reviewed4f30453f2
Current headb52a21a95

The delta is a merge of origin/main plus one line of content/docs/permissions/system-context.mdx — a census anchor rewritten :1009:1015 by the gate's own --fix, after Lint & Repo Gates went red on check-system-context-census. Class was established before the repair, with the gate's own instruments: census sites for the file 1, page anchors 1, read text byte-identical to origin/main — a pure line shift caused by this PR's restore-leg comment block sitting 6 lines above the read. No count hand-edited, no row hand-written. Re-run judgment line at b52a21a95: OK — 109 elevation read sites in 20 packages across 45 files, all anchored; 145 anchors resolve, 27 declared non-read.

None of the three reviewed files changed in substance. Whether the PASS carries forward to b52a21a95 is the review seat's call, not mine — I am not treating a verdict on one head as a verdict on another.

⛔ What I have deliberately NOT done

The verdict also directs ready + auto-merge (SQUASH). I have not armed auto-merge. This seat is below CONTRACT_REVIEW_TIER, and arming a merge on a head the verdict did not review is exactly the call that belongs to the seat that issued it. Restoring ready is me undoing my own accidental change; arming auto-merge would be me making a new one.

If the review seat confirms the PASS carries to b52a21a95, arming is a one-line action for whoever owns it.


Generated by Claude Code

@os-elon
os-elon added this pull request to the merge queueAug 30, 2026
Merged via the queue into main with commit 9cfc1f7Aug 30, 2026
38 checks passed
@os-elon
os-elon deleted the claude/issue-12020-packaged-set-restore-lock branch August 30, 2026 14:24
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

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

4 participants

@os-elon@zhuangjianguo@os-zhuang@claude