fix(plugin-quality): packet-seal reads generations and records the seal moment

[kyle-sexton]

Refs: #3357
Refs: #3867
Refs: #3866

Summary

Fixes from the 2026-09-29 audit of the unattended Cursor run, scoped to plugins/plugin-quality/ (exec-bash.mjs untouched). This PR closes no issue; #3357 is a container that stays open pending two owner questions, #3867 was closed earlier, and #3866 is unchanged (owner decision ratified, no change).

• packet-seal.sh verify never read the generation manifest record --acknowledge-divergence writes, so it reported packet.sha256.N and every later note as UNSEALED forever, and a restore-then-add-notes sequence had no way to seal the new notes (plugin-quality 0.6.10 — findings from running a 3-target audit end to end #3357).
• The plugin-quality: define what a sealed packet asserts once the session that produced it keeps acting #3867 decision says the packet records its seal moment; the manifest held digests only (plugin-quality: define what a sealed packet asserts once the session that produced it keeps acting #3867).
• The write-once and seal-time doctrine was restated in five places, and evidence-packet.md still said item.md where the script and skill say item*.md.
• auditor.md stated a claude plugin validate --json version floor with no basis, and the 0.7.28 and 0.7.29 CHANGELOG entries were byte-identical.

Fix

• verify still grades packet.sha256 exactly as before, then grades every entry of the highest-numbered generation as GEN-MATCH, GEN-CHANGED or GEN-MISSING, prints ACKNOWLEDGED generation=<n>, and never reports a manifest file as UNSEALED. Exit codes stay 0/1/2/3.
• record --acknowledge-divergence with a generation in place writes the next generation over the current bytes instead of exiting 2. An ordinary record is still refused once a generation exists.
• record and each generation write # sealed-at <UTC ISO-8601>; verify prints sealed-at= and gen-sealed-at=. Older manifests report sealed-at=unknown.
• evidence-packet.md "What a sealed packet asserts" is the single owner of the doctrine; auditor.md, SKILL.md and recurring-concerns.md point at it. No owner decision is reversed: the plugin-quality: decide how write-once is guaranteed when the agent bound by it is the party able to break it #3866 Option A text is kept.
• auditor.md records the basis for the validate --json floor: the CLI reference options table ("Requires Claude Code v2.1.259 or later"), corroborated by the 2.1.259 changelog entry.
• Version 0.8.0 with one CHANGELOG entry. Declared in-place corrections to released entries, per check-changelog-parity.sh: 0.7.28 drops the repo-script sentence that does not belong in a plugin changelog; 0.7.29 now states it carried no plugin change instead of repeating the 0.7.28 body.

Verification

• bash plugins/plugin-quality/scripts/*.test.sh: citation-contract-drift PASS=12, context-zone PASS=81, packet-prune passed, packet-seal passed (new cases for generation reads, numeric ordering, restore-then-add-notes, look-alike names, GEN-MISSING, sealed-at), zones-inline-drift PASS=11.
• scripts/check-changelog-parity.sh --check --check-order, --check-bump origin/main, --check-preserved origin/main: all pass.
• scripts/validate-plugins.sh: all manifests and the catalog validated.
• scripts/check-changed-skills.sh origin/main: audit skill PASS, 0 errors, 3 warnings (none from these edits).
• markdownlint on the CHANGELOG: 0 issues. origin/main merged into the branch before the bump.

Related

• Audit report: .work/audit/REPORT.md, findings for plugin-quality 0.6.10 — findings from running a 3-target audit end to end #3357 (generation manifest never consumed by verify), plugin-quality: define what a sealed packet asserts once the session that produced it keeps acting #3867 (seal moment not recorded), plugin-quality: decide how write-once is guaranteed when the agent bound by it is the party able to break it #3866 (3d ratify, no change), Make the post-use plugin retrospective a universal practice: interview, evidence, and consensus research after any session that used a plugin #3999 (no action: PR closed unmerged, nothing stranded).
• Cross-group request to scripts (optional, low priority): make check-changelog-parity.sh --check-bump fail when a new entry body is byte-identical to the previous version's, since 0.7.29 shipped that way (chore(changelog): close #4027 for Claude Code 2.1.257–2.1.263 #5162).
• Owner questions for plugin-quality 0.6.10 — findings from running a 3-target audit end to end #3357 (accept the generation-manifest design from feat(plugin-quality): acknowledge sealed-packet divergence (#3357) #5145; whether the packet-level seal moment plus the timeline-question rule closes P2) go on the issue, not in this PR.

🤖 Generated with Claude Code

https://claude.ai/code/session_01EugXnFddtpHcY5gTuyEirB

[github-actions]

PR body contract — issue linkage

This PR body conforms to the issue-linkage contract. Nothing to do.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-29T13:36:50.398552Z	1bb6598	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[claude]

Claude finished @kyle-sexton's task in 3m 18s —— View job

---

Security review complete

• Gather context (PR body, changed files, custom instructions)
• Invoke /review:security-review per custom instructions and follow it exactly
• Post findings

Ran the CI security-review criteria against 1bb6598 (origin/main...HEAD, 8 files, +599/-180), scoped to files changed in this PR only.

No security issues found.

What I checked, given the diff is mostly plugins/plugin-quality/scripts/packet-seal.sh (a bash tamper-evidence tool) plus doctrine-consolidation markdown:

• packet-seal.sh logic (new generation-manifest reads, record --acknowledge-divergence allowing a next generation, seal_header/seal_moment/latest_generation/note_divergence/is_manifest_name, refactored verify_manifest): traced the tamper-detection paths for a bypass — restore-then-reseal, repeated acknowledgment without new divergence, deletion of the latest generation to expose an older one, numeric-suffix parsing (10# guards against octal/leading-zero misparsing) — and didn't find a sequence that makes altered evidence verify as intact. The original packet.sha256 divergence is still permanent and still forces exit 1 regardless of what happens to later generations.
• Filename/variable handling: all $name/$file expansions stay quoted, digest_of calls the digest tool with -- before the path, and no user-controlled string reaches eval or an unquoted expansion.
• Predictable temp file names ($manifest.tmp.$$, $gen_manifest.tmp.$$) and no symlink check on the verify path (only record refuses symlinks) are both pre-existing patterns already present in origin/main before this PR (confirmed via git show origin/main:...), not introduced by this diff, so they're out of scope for a high-signal finding here.
• Doctrine consolidation (auditor.md, SKILL.md, recurring-concerns.md now point at evidence-packet.md "What a sealed packet asserts" instead of restating it): checked this against the instruction-surface-deletion lens, since it deletes/shortens standing instruction text in several files. The behavioral rule that matters operationally — "don't publish a correction when live state contradicts a packet claim; raise a timeline question instead" — is kept verbatim in auditor.md, not just delegated. The mechanical detail moved to evidence-packet.md is still present there in equivalent form (write-once discipline, record/--acknowledge-divergence behavior, the assertion table), just reworded/reorganized. No guardrail was dropped without a surviving enforcement point.
• plugin.json, CHANGELOG.md: version bump and changelog text only, no security surface.
• Scanned the new packet-seal.test.sh cases for risky patterns (eval, curl, wget, sudo) — none present.

exec-bash.mjs and GitHub Actions workflows are untouched by this PR, consistent with the PR description.

[claude]

Claude finished @kyle-sexton's task in 3m 51s —— View job

---

Code review (/review:code-review) — plugin-quality: packet-seal generations + seal moment

• Gather context (PR body, diff, changed files)
• Invoke /review:code-review for this PR
• Post findings as inline comments (none met the bar — see below)
• Summarize in this comment

Scope: all 8 changed files under plugins/plugin-quality/ (git diff origin/main...HEAD), with primary attention on scripts/packet-seal.sh (the generation-read/seal-moment logic) and scripts/packet-seal.test.sh.

Findings

None met the high-signal bar for this lane (a careful senior reviewer would block or flag). I traced the new packet-seal.sh control flow in detail against the stated invariants:

• latest_generation, is_manifest_name, note_divergence, and the new verify_manifest helper — checked numeric-vs-lexical generation ordering, look-alike names (packet.sha256.2x), missing/changed accounting, and the interaction between the top-of-record "refuse ordinary reseal once a generation exists" guard and the later "already-diverged" refusal block.
• Verified the one place that looked suspicious on first read — the files array built at packet-seal.sh:265-270 only excludes the literal packet.sha256, not other generation names, so it could in principle let an older generation file leak into a new manifest as content — is a dead end: the unfiltered loop that consumes files without an is_manifest_name guard (packet-seal.sh:365) is only reachable when no generation exists yet, and the generation-writing loop that can run alongside existing generations does have the is_manifest_name guard (packet-seal.sh:317-318). No reachable input produces the bad outcome.
• packet-seal.test.sh's new cases (numeric ordering, look-alike names, restore-then-add-notes, GEN-MISSING on deletion, sealed-at/gen-sealed-at presence and unknown fallback) match my manual trace of the implementation.
• Doc consolidation (evidence-packet.md, auditor.md, SKILL.md, recurring-concerns.md) reads as a straight move-to-one-owner with no orphaned anchors: grepped the repo for the removed subsection headings (Assertion list, Producer obligations, How a reader tells…) and found no remaining references to them outside the rewritten file.

Verification note

I could not execute shell commands in this environment (bash …/packet-seal.test.sh, shellcheck) — the sandbox denied it. So the PR's own test-run and lint results (citation-contract-drift, packet-seal, check-changelog-parity.sh, validate-plugins.sh, markdownlint counts) are author-claimed and unverified by me; my review is a static read of the diff plus the test file's assertions, not an independent execution.

Summary

No correctness or maintainability issues found in the diff that clear this lane's bar. The generation-read logic in verify, the "ordinary record refused once a generation exists" / "acknowledge writes the next generation" split in record, and the seal-moment header are internally consistent and covered by new tests that match the implementation.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1bb6598b1a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Claude finished @kyle-sexton's task in 3m 8s —— View job

---

Code review (/review:code-review) — packet-seal: symlink/overflow fail-closed fix

• Gather context (PR body, diff, changed files, custom instructions)
• Invoke /review:code-review and follow it exactly
• Post findings as inline comments
• Summarize in this comment

Scope: this trigger fired on synchronize at HEAD 0d00582. The prior review pass in this thread (Claude + Codex, both on commit 1bb6598) already covered the full PR diff (8 files, +634/-180) with no blocking findings, plus two Codex findings (P1 symlinked generation manifest, P2 generation-number overflow) in packet-seal.sh. git diff 1bb6598..0d00582 --stat shows only 3 files changed since that review: CHANGELOG.md, scripts/packet-seal.sh, scripts/packet-seal.test.sh — commit 0d00582 ("fail closed on symlinked or overflowing generation manifests"), which is a direct fix for both Codex findings. I reviewed that commit in full against the prior findings and re-traced the surrounding control flow; the rest of the diff was not re-walked since nothing in it changed.

Findings

None met the high-signal bar. Verification of the fix commit:

• P1 (symlinked generation manifest) — fixed. latest_generation now checks -L "$f" before -f "$f", so a symlinked packet.sha256.N (valid or broken target) returns 2 and both verify (L450) and record (L291) propagate it via || exit 2 before any external target is ever read. The -L check fires unconditionally for both record and verify, since both call the same shared function.
• P2 (overflowing generation number) — fixed. The ^0*[1-9][0-9]{18,}$ guard at L181-L184 rejects (exit 2) any generation number past 18 digits before it ever reaches the 10#$n arithmetic comparison or the generation=$((10#${latest:-1} + 1)) increment at L318, so the wrap-to-negative sequence Codex described can no longer happen. This is conservative — it also rejects legitimate 19-digit values below INT64_MAX (~9.2×10¹⁸) — but that's a stated, fail-closed design choice (see the updated comment and CHANGELOG line), not a correctness bug, and generation numbers only ever increment by 1 from 2 in practice.
• Traced both new tests in packet-seal.test.sh (symlinked generation, and the 42-nines overflow case) against the implementation; both exercise the intended code paths and their has "..." assertions match the actual echo text emitted (test harness captures 2>&1 into last_out, confirmed in test-helpers.sh).
• No new quoting, temp-file, or ordering issues introduced: the -L/overflow checks run inside the same loop that already builds latest numerically, return 2 skips the trailing printf so the caller's command substitution captures nothing on failure, and no already-validated generation file is re-read unguarded elsewhere (note_divergence and the GEN- scan only ever operate on $latest after it has passed the guard).
• Cosmetic only, not flagged as a finding: the new CHANGELOG sentence ("A symlinked generation manifest, or one whose number is past 18 digits, exits 2 instead of being read or wrapped. A look-alike name such as...") runs noticeably longer than the surrounding wrapped lines in CHANGELOG.md. Author-claimed markdownlint (0 issues) is unverified by me — I could not execute bash/test/lint commands in this environment (sandbox denied it), so that and the rest of the PR's test/lint claims remain author-claimed and unverified here.

Summary

The new commit (0d00582) closes both open Codex findings from the prior review round with a fail-closed guard shared by record and verify, backed by matching tests. No correctness issues found in the reviewed delta.

Branch: fix/audit-plugin-quality

[claude]

Claude finished @kyle-sexton's task in 3m 16s —— View job

---

Security review (/review:security-review)

• Gather context (PR body, changed files, custom instructions)
• Invoke /review:security-review and follow it exactly for this PR
• Post findings

No security issues found.

Scope: all 8 files changed in this PR (gh pr diff 5272 against origin/main), per the CI security-review lane's criteria (logic/trust-boundary flaws, not GitHub Actions hardening — exec-bash.mjs and workflows are untouched here). Primary attention on scripts/packet-seal.sh, since it's the only file in this diff implementing a security-relevant mechanism (tamper-evidence for audit packets); the rest is doctrine-consolidation markdown and a version bump.

Codex's two review findings on this PR are already fixed at HEAD (0d00582). Both concerned latest_generation() (added by this PR to let verify read generation manifests):

• P1, symlinked generation manifest silently followed → now checked explicitly (packet-seal.sh:176-179): a -L generation candidate exits 2 with generation manifest is a symlink: ... before it is ever read, in both record --acknowledge-divergence and verify.
• P2, an overflowing generation number wrapping Bash arithmetic → now bounded (packet-seal.sh:181-184): any candidate with 19+ significant digits exits 2 with generation number is too large to read before any 10# arithmetic touches it, so no negative/wrapped generation number can be produced. Both are covered by the new packet-seal.test.sh cases in the "generation manifest that cannot be trusted" block.

Beyond confirming those fixes, I traced the rest of the new generation-manifest logic for a bypass of the tool's actual security property (can altered/laundered bytes verify as intact?) and found none:

• No laundering path. packet.sha256 is never overwritten by --acknowledge-divergence (only packet.sha256.<n> is created — packet-seal.sh:318-348), so verify keeps reporting the original CHANGED permanently regardless of later generations, restores, or acknowledgments.
• Numeric generation ordering (10# prefix, explicit ^[0-9]+$ gate before any arithmetic) correctly picks the highest generation even with a leading-zero or packet.sha256.10 vs .9 sibling, confirmed against the new numeric-ordering test.
• Look-alike names (packet.sha256.2x) are excluded by is_manifest_name's anchored regex and fall through to ordinary UNSEALED content, not treated as a manifest — checked both the record-time exclusion (packet-seal.sh:328-329) and verify-time exclusion (packet-seal.sh:472).
• Symlinked packet entries (not generation manifests, ordinary packet content) are still rejected at digest time in both the original-manifest and generation-manifest write loops (packet-seal.sh:331-335 and 376-383).
• sealed-at/gen-sealed-at are explicitly documented and treated everywhere as self-attested/unsigned, never used to gate an exit code, so there's no false trust placed in an attacker-editable timestamp.
• Quoting/injection: all digest_of calls use -- before the path, no unquoted expansions, no eval, no shell metacharacters reach date/sed from untrusted input.

Instruction-surface deletion check (the doctrine consolidation collapses five restatements of the write-once/seal-time rule into one owner in evidence-packet.md): the load-bearing behavioral rule — don't publish a correction when live state contradicts a packet claim, raise a timeline question instead — is kept verbatim in agents/auditor.md, not just delegated by pointer. The mechanical detail that moved (write-once discipline, record/--acknowledge-divergence behavior, the assertion table) is still present in equivalent form in evidence-packet.md. No guardrail was dropped without a surviving statement.

.claude-plugin/plugin.json and CHANGELOG.md are version/changelog text only, no security surface.
· branch fix/audit-plugin-quality