feat(archive): --bundle-source + regenerate-and-compare verifier (activation-release PR-2)#17
Merged
Merged
Conversation
…off via --bundle-source Implements PR-2 of the local activation-release pipeline (workspace design of record drafts/local-activation-release-cut-pipeline.md §3.2/§9.1c/ §9.12(2)/§10-A/D1, hardened by a 2-round distinct-family adversarial DESIGN review before this implementation). The signed+notarized runtime bundle ships as a release asset and is never committed, so the builder can no longer read it from the git checkout: - `--bundle-source <leaf .bundle dir>` supplies the signed 0o500 handoff. The committed manifest alone decides the mode (D1): activation manifest without --bundle-source = hard error (never a silent policy-only downgrade); policy-only manifest forbids the flag; an in-tree runtime/ conflicts with an external source. - classify_package() validates the manifest SHAPE only (split into _validate_activation_manifest + _validate_activation_bundle_tree); the physical leaf is validated as HOSTILE input: symlink argument rejected via lstat BEFORE resolve(), bounded non-following enumeration, member-set EQUALITY, per-member O_NOFOLLOW regular-file/uid/nlink-1/mode/size/sha256 (strict 0o500 preserved — the external source is mode-preserving). - Runtime archive metadata is synthesized ENTIRELY from a frozen manifest snapshot (0o755 traversal parents + sealed 0o500 leaf; file members get record mode/size); only bytes stream from the validated source. - verify_archive stays self-contained (mode kwarg only — CI's build_release_evidence.py call is unchanged) and validates runtime members against a SINGLE frozen manifest byte-snapshot: exact member sequence + type + mode + size + sha256(archived) == record digest, plus archived runtime-manifest.json bytes == frozen bytes. A source-tree swap after build-time validation can never produce a passing archive (closes the validate→pack TOCTOU without fd-pinning). - Build writes to an O_EXCL temp, verifies THAT, then os.replace()s to the destination; failure unlinks the temp — no publishable artifact on any failure path. --output must not alias a source tree. 12 new adversarial tests (fail-closed gating, symlink/hardlink/device rejection, digest/size/mode drift, membership equality, tampered-archive + manifest-swap detection via the frozen binding, no-output-on-failure, determinism). 327 tests/ + 254 scripts/ green; CLI smoke-verified (--print-mode, policy-only build, fail-closed bundle-source rejection). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ation, canonical metadata verify Codex peer-review round 1 (REQUEST_CHANGES/H) on the --bundle-source implementation; all 5 concerns integrated: 1. Single frozen snapshot now actually spans the build: the manifest is read ONCE through an O_NOFOLLOW fstat-checked descriptor; those exact bytes drive record validation, are embedded into the archive verbatim (io.BytesIO, never a pack-time re-read of the mutable checkout), and are handed to verify_archive via a new optional frozen_manifest parameter (standalone/CI callers omit it and read once from the checkout — the build_release_evidence.py call remains unchanged). 2. Pack-time revalidation of the OPEN runtime descriptor (fstat: regular file, uid, nlink 1, mode, size) with O_NONBLOCK so a post-validation FIFO swap cannot block the open; byte content stays re-bound to the manifest digest by verify_archive. 3. Temp cleanup only unlinks a temp file THIS invocation created (temp_created tracking — a pre-existing path makes O_EXCL fail without destroying the foreign file); output/source alias check runs BEFORE mkdir and re-checks the strictly resolved path after. 4. verify_archive enforces canonical metadata on EVERY member (regular file/dir only, uid/gid 0, empty uname/gname, mtime 0, no linkname, no PAX headers) and bounds every byte read with a size-parity check before extraction (hostile-archive decompression bound). 5. assert → explicit ValueError (survives python -O). New tests: non-canonical mtime rejected; foreign temp file preserved on O_EXCL collision; same-size manifest swap caught by the frozen-byte binding + size-changing swap caught by size parity; same-size source drift caught by byte parity + size drift caught bounded. 329 tests/ + 254 scripts/ green; CLI smoke-verified. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… GLM coverage tests Codex peer-review round 2 closed concerns 1/2/3/5; the remaining #4 (H): tarfile.open("r:gz") + getmembers() decompressed and parsed the ENTIRE archive (PAX payloads included) before any canonical check ran, so a crafted compressed asset could exhaust memory/time pre-validation. Fix: _bounded_archive_stream() rejects a symlinked/oversized compressed file (<= MAX_ARTIFACT_BYTES) and decompresses under a hard 2x MAX_ARTIFACT_BYTES cap BEFORE tarfile ever parses a header; verify_archive then parses the bounded in-memory bytes ("r:"). New test: a gzip bomb (tiny compressed, >128 MiB decompressed) is rejected at the bound; a symlinked archive path is refused outright. Also integrates the GLM cross-check coverage concerns (PROCEED-WITH- MODIFICATIONS/H on the round-1-fixed diff): tests for --output alias rejection (in plugin tree, in bundle leaf, via symlinked parent), pack-time-failure temp cleanup, and CLI --bundle-source wiring; comments documenting the ustar <=100-char/ASCII name constraint and the residual pid-suffix temp-path DoS note. 333 tests/ + 254 scripts/ green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…counting) — closes peer-review #4 TOCTOU Codex round-3: the decompressed/PAX bound was confirmed fixed; the compressed cap still had an lstat->open race. Now the archive is opened O_NOFOLLOW, fstat'd on the descriptor, and the cap is enforced on the bytes actually read from that same fd — no path-based window remains. 333 tests/ green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…n-review pivot) Per the adversarial-architecture-review tripwire: the post-hoc peer-review loop kept surfacing NEW bypasses in ONE surface (how verify_archive reads a hostile archive) across rounds — the signal the surface was under-architected. Reverted it to the design phase, wrote a threat-class enumeration, and ran a 2-round adversarial DESIGN review. Round 1 (RECONSIDER/H) surfaced a whole layer of structural bypasses the code loop hadn't reached (hidden PAX/GNU/ sparse physical headers, gzip-header malleability, sparse/contiguous typeflags, dir-payload smuggling, parser differentials) AND the architectural pivot: don't validate hostile tar bytes with tarfile — regenerate the canonical tar from trusted inputs and byte-compare. Round 2 PROCEED-WITH- MODIFICATIONS/H confirmed the pivot closes all 8 classes; 4 refinements folded in. verify_archive now NEVER hands candidate tar bytes to tarfile: - A shared deterministic USTAR serializer (_emit_canonical_tar, used by BOTH build and verify) reproduces the canonical tar; runtime payloads zero-filled at verify. USTAR_FORMAT + a _finalize_tarinfo guard make an extension record impossible → byte-reproducible across the operator (3.13.14) and CI (3.10/3.12/3.14); a golden-bytes test binds the serializer output. - The candidate is fd-only bounded-read (O_NOFOLLOW|O_NONBLOCK + fstat, no path-swap window) with a canonical gzip-header check, single-stream/no- trailing validation, and hard compressed/decompressed caps enforced on bytes actually read; then sliced ONLY at offsets the verifier computed. - Structural byte-compare OUTSIDE the runtime ranges binds every header, typeflag, ordering, padding, EOF, dialect, and the archived manifest bytes; each runtime payload is bound to the frozen-manifest digest; the range map is asserted complete/one-to-one/disjoint/in-bounds (design item 6). - Build emits the shared canonical tar, gzips it (canonical header) into an O_EXCL temp, verifies THAT, then os.replace()s; runtime payloads are re-read+re-digested through fresh fds at pack time. New tests: golden USTAR serializer bytes, cross-call reproducibility + range- map integrity, raw structural tamper caught without parsing, canonical vs non-canonical gzip header. 336 tests/ + 254 scripts/ green; CLI smoke-verified (canonical gzip header 1f8b080000000000, round-trip verify). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…LM cross-check) GLM cross-check on the regenerate-and-compare implementation: PROCEED-WITH- MODIFICATIONS (12-point checklist all correct, no fail-open). Integrated both: 1. Cross-environment mode reproducibility (concern 1): non-runtime member modes came from gettarinfo (the working-tree checkout mode, which a checkout applies the local umask to). With build (operator host) and verify (CI host) now on different machines, a umask difference could make the regenerated canonical's mode fields differ → false-reject a legitimate archive. git tracks ONLY the exec bit, so normalize non-runtime modes to 0o755/0o644 by the exec bit — umask-independent. Runtime members keep their sealed 0o500 from the manifest. 2. O(n^2) payload assembly (concern 2): _read_runtime_payloads now list-appends + b"".join instead of repeated b"" concatenation. New test: non-runtime modes normalized to 0o644/0o755 even when a source file is 0o600, and the archive still verifies. 337 tests/ + 254 scripts/ green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… bundle validator; full 10-byte gzip header Codex peer-review round 1 on the regenerate-and-compare implementation (REQUEST_CHANGES/H); concerns 1-3 integrated (concern 4 was already satisfied — see below): 1. Single-snapshot classification: classify_package() re-read the manifest separately from build/verify's freeze, so an A->B->A swap could classify A but package B. New _classify_from_manifest(plugin_path, bytes) derives the mode, the FULL activation-shape validation, AND the runtime records from ONE byte snapshot; build_archive and verify_archive freeze once and classify from that snapshot (policy-only included). classify_package() now reads once and delegates. 2. _validate_activation_bundle_tree hardened to match _read_runtime_payloads: O_NONBLOCK on the member open (a swapped FIFO cannot block) + a read cap at the fstat'd size (a file that grows after fstat cannot cause unbounded reading; read_total != size now fails closed). 3. The gzip-header check now binds the FULL canonical 10 bytes (1f8b 08 00 00000000 02 ff) — CPython writes XFL (level-9 best) and OS (ff) deterministically, so a re-gzip at a different level (XFL change) is rejected; the build->verify round-trip tests fail closed on any Python that diverges. New test: an identical-content archive re-gzipped at level 1 is rejected on the XFL byte. Concern 4 (cross-env reproducibility proof) was already met by test_ustar_serializer_is_golden_across_python_versions (golden sha256 of the USTAR serializer, run on CI's 3.10/3.12/3.14 matrix) — the reviewer saw only the script diff, not the test file. 337 tests/ + 254 scripts/ green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…erly-permissive-file) The build temp (which becomes the release archive via os.replace) was created world-readable. It is read + uploaded by the building user; no other local user needs read access. Owner-only 0o600 clears the high- severity CodeQL alert and is strictly safer for the release artifact. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bd977f8437
ℹ️ 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".
…odex PR-review P2)
The compressed cap equaled MAX_ARTIFACT_BYTES (the runtime-payload cap), but
the archive also holds the manifest, skills, licenses, plugin metadata, tar
headers/padding, and gzip framing. A poorly-compressible runtime near the cap
would make the compressed .plugin exceed 64 MiB and verify_archive would
reject the builder's OWN legitimate output ("exceeds the canonical size
bound"). Both caps now carry 2x (128 MiB) headroom over the payload cap; the
decompressed cap still bounds a gzip bomb. New test asserts the cap
relationship.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f054668689
ℹ️ 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".
Comment on lines
+983
to
+986
| if runtime_root.exists() or runtime_root.is_symlink(): | ||
| raise ValueError( | ||
| "in-tree runtime conflicts with --bundle-source; remove one source" | ||
| ) |
There was a problem hiding this comment.
When an activation manifest is used, this branch now rejects any committed plugins/agent-collab/runtime tree, but the activation evidence path still verifies only that in-tree location: scripts/verify_runtime_release.py constructs root / PLUGIN_REL / RUNTIME_BUNDLE_REL, and the checked .github/workflows/release.yml macOS job runs it without any bundle handoff. With the new out-of-tree bundle-source model, the macOS evidence job fails because the bundle is missing; with the old in-tree layout, this check fails before packaging, so activation releases have no successful path until the verifier/workflow accept and pass the same external bundle source.
Useful? React with 👍 / 👎.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PR-2 of the adversarial-review-hardened activation-release implementation (design of record:
agent-collab-workspacedrafts/local-activation-release-cut-pipeline.md§3.2 / §9.1c / §9.12(2) / §10-A / D1). Config-independent, foundational.The signed+notarized macOS runtime bundle ships as a release asset and is never committed, so
build_plugin_archive.pycan no longer read it from the git checkout. This adds--bundle-source <leaf .bundle dir>and hardens the archive verifier.--bundle-source+ fail-closed mode gating (D1):runtime-manifest.jsonalone decides the mode (via a single_classify_from_manifestsnapshot): an activation manifest without--bundle-sourceis a hard error (never a silent policy-only downgrade); a policy-only manifest forbids the flag; an in-treeruntime/conflicts with an external source.lstatbeforeresolve(); bounded non-following enumeration; member-set EQUALITY; per-memberO_NOFOLLOW/O_NONBLOCKopen admitting only regular files with uid==getuid, nlink==1, exact 0o500 install mode, exact size, exact sha256 (read-capped at the fstat'd size); payloads re-read + re-digested through fresh descriptors at pack time.verify_archiverearchitected — regenerate-and-compare (never parses hostile tar bytes):USTARserializer (_emit_canonical_tar, used by both build and verify) reproduces the canonical tar with runtime payloads zero-filled;_finalize_tarinfomakes an extension record impossible → byte-reproducible across the operator's build Python (3.13.14) and CI (3.10/3.12/3.14), bound by a golden-bytes test. Non-runtime modes are umask-normalized (0o644/0o755 by the exec bit).O_NOFOLLOW/O_NONBLOCK+ fstat, no path-swap window; hard compressed/decompressed caps enforced on bytes actually read; full canonical 10-byte gzip-header check; single-stream / no-trailing validation) and then sliced at offsets the verifier computed.runtime-manifest.jsonbytes are bound to the frozen snapshot; the range-map is asserted complete/one-to-one/disjoint/in-bounds.O_EXCLtemp, verifies THAT, thenos.replace()s — a failed build/verify never leaves a publishable artifact;--outputmay not alias a source tree.Process note (adversarial-architecture-review directive): the hostile-archive-reading surface was reverted to the design phase after a post-hoc review loop kept surfacing new structural bypasses in it (the tripwire); a 2-round up-front adversarial DESIGN review then surfaced the regenerate-and-compare pivot and a whole layer of structural classes at once, before implementation.
Tests:
--bundle-sourcefail-closed gating (3 ways), symlink/hardlink/device rejection, digest/size/mode drift, membership equality, tampered-runtime-bytes caught by the frozen digest, post-build manifest-swap caught structurally, raw structural tamper caught without parsing, gzip bomb / non-canonical + non-10-byte header, no-output-on-failure, determinism + golden USTAR serializer + umask normalization + range-map integrity. 337 tests/ + 254 scripts/ green; CLI smoke-verified (canonical 10-byte gzip header1f8b08000000000002ff, round-trip verify).Cross-PR note for PR-5 (release.yml whole-archive verify): the two synthesized parent dir members (
runtime/,runtime/darwin-arm64, 0o755) are fixed packaging scaffolding — whitelist exactly them as "tree ∪ manifest ∪ fixed scaffolding".author: claude
standing_directives: adversarial-architecture-review-before-security-codegen (up-front 2-round distinct-family DESIGN review of BOTH the feature design AND — after the tripwire fired — the hostile-archive-reading threat model, converged BEFORE implementation); directive #6 Tier-3 (cross-check + distinct-family peer review); directive #7 (model+effort below); directive #8 (delegation economy — Grok Composer typed-unavailable
runtime_invalid#1975; primary implemented against the converged design).tier: 3
cross_check: PROCEED (converged) — GLM/opencode (opencode/glm-5.2, Zhipu, effort selectable). Implementation cross-check: verbatim "VERDICT: PROCEED-WITH-MODIFICATIONS / CONFIDENCE: 0.85" — a 12-point checklist, all correct, NO fail-open path found. 2 concerns integrated (umask-independent non-runtime mode normalization; O(n) payload assembly). A delta re-check of the subsequent peer-review hardening (single-snapshot classification, O_NONBLOCK+read-cap, 10-byte gzip header) returned verbatim "VERDICT: PROCEED-WITH-MODIFICATIONS — confidence high" with "No fail-open in any of the three"; the 2 read-only confirm-items were verified in-source (records are validate_file_records output — cannot return unvalidated; policy-only frozen embed is build/verify-symmetric), no code change needed.
peer_review_verdict: APPROVE
peer_review_agent: codex
peer_review_message_id: codex-NA
peer_review_reviewed_sha: bd977f8
peer_review_concerns_integrated: Codex (OpenAI, gpt-5.6-sol, xhigh) — DESIGN reviewer (2 rounds on the feature design; 2 rounds on the hostile-archive-reading threat model that produced the regenerate-and-compare pivot) AND implementation peer reviewer. Implementation review R1 "VERDICT: REQUEST_CHANGES / CONFIDENCE: HIGH" (4 concerns): single fully-validated frozen-manifest snapshot (A→B→A race); O_NONBLOCK + read-cap in the bundle-tree validator; bind the full 10-byte gzip header; golden-tar reproducibility proof (already present in the test file, unreviewed in the script-only diff). All integrated. R2 (delta) "VERDICT: APPROVE / CONFIDENCE: HIGH / CONCERNS: none — all four prior concerns are closed." Three distinct families (Anthropic author + Zhipu cross-check + OpenAI peer review); reviews via managed CLIs (opencode_exec + codex exec), codex-NA CLI sentinel. NOTE: the intent-guard's independent managed red-team route hit a session-identity
config_errorduring one design round, so that leg was primary-adjudicated; recorded honestly — the peer-review + cross-check legs are full and independent.post_condition: 337 tests/ + 254 scripts/ tests green; CLI smoke-verified (--print-mode, policy-only build unchanged, fail-closed bundle-source rejection, canonical 10-byte gzip header, round-trip verify, no temp leftovers); parse-clean; no orphaned symbols.
mcp_coverage_gap: none — reviewers via managed CLIs; no MCP tool
contributor_rights: OPERATOR-CONFIRMED
operator_reserved: no — operator directed "Self-merge PR-1, then proceed with PR-2 implementation" and "Open the PR once it converges"
plugin_affected: this IS the plugin repo; scripts/build_plugin_archive.py (release build tool) + tests; no plugin.json/version change (release tooling, not shipped-package behavior — the activation release cut lands separately)
readme_refresh_status: N/A — not a plugin release (no version bump)
model_effort: author Claude (implementation across claude-fable-5 and claude-opus-4-8, effort high); design + peer review Codex (gpt-5.6-sol, xhigh); cross-check GLM (opencode/glm-5.2, selectable)
🤖 Generated with Claude Code