Harden CLI tar archive metadata - #38
Conversation
Why: - macOS COPYFILE and xattr behavior can add hidden AppleDouble and PAX records while ordinary tar listings conceal them. - Release archives must not expose host ownership or depend on one tar implementation. Changed: - Create portable USTAR CLI archives with normalized numeric ownership and Darwin-gated metadata suppression. - Validate raw tar headers, PAX records, safe paths, exact USTAR markers, and trailers with fail-closed negative fixtures. - Run the package/docs contract on hosted macOS as well as Ubuntu and document the cross-host contract. Verification: - node --test --test-name-pattern=SB-PKG-CLI-003 docs/test/front-door.test.mjs - npm run test:docs - npm test - ./scripts/validate.sh - npm run agent:verify -- --profile spec-only --paths scripts/package_cli.sh docs/test/front-door.test.mjs spec/build-and-test.md .github/workflows/ci.yml --session-dir task-sessions/rr22-cli-tar-metadata-20260831 - npm run verify:public-surface - staged current-source audit and independent portability review Affected: - .github/workflows/ci.yml - docs/test/front-door.test.mjs - scripts/package_cli.sh - spec/build-and-test.md Refs: - https://how.complexsystems.fail Session: - task-sessions/rr22-cli-tar-metadata-20260831
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Reviewed top-level PR feedback and left the current code unchanged. These items were posted as PR review bodies or conversation comments rather than unresolved review threads, so this acknowledgement is recorded on the PR timeline.
|
![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: 0fe0b0b61e
ℹ️ 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".
VladimirBrejcha
added
autopilot:on
Why: - Resolve actionable GitHub review feedback for PR #38. Changed: - Fixed both actionable review threads: CLI packaging now selects `--no-mac-metadata` and `--no-xattrs` from tar capability probes instead of `uname`, and it excludes or refuses pre-existing AppleDouble `._*` entries before writing the checksum. The Codex summary comment and review-body wrapper are non-actionable. Verification: - npm test Affected: - .github/workflows/ci.yml - docs/test/front-door.test.mjs - scripts/package_cli.sh - spec/build-and-test.md Refs: - #38 - #38 (comment) - #38 (comment) Session: - task-session: [controller artifact: jobs/pr-38/20260831-205047-0103f542-21fa-4361-a228-82ef2f2beb72/task-session] - report: [controller artifact: jobs/pr-38/20260831-205047-0103f542-21fa-4361-a228-82ef2f2beb72/report.md]
|
Reviewed top-level PR feedback and left the current code unchanged. These items were posted as PR review bodies or conversation comments rather than unresolved review threads, so this acknowledgement is recorded on the PR timeline.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 30ed81571f
ℹ️ 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".
|
Reviewed top-level PR feedback and left the current code unchanged. These items were posted as PR review bodies or conversation comments rather than unresolved review threads, so this acknowledgement is recorded on the PR timeline.
|
Why: - Tar implementation, host metadata, and stale output state can combine to publish misleading or non-portable CLI release bytes. - AppleDouble inputs must fail visibly instead of disappearing behind archive exclusions. Changed: - Classify stable GNU tar and bsdtar version identities before staging and select implementation-specific ownership and metadata flags. - Reject source and staged AppleDouble paths, clear stale final outputs, and validate a private candidate before checksum and publication. - Share one raw USTAR validator between production packaging and docs regressions, with fail-closed fixtures for unknown tar, cross-host selection, unsafe metadata, and invalid candidates. - Update the specs manifest and CLI archive contracts for the new production validator and failure boundary. Verification: - Focused CLI package contract: 7/7 passed. - npm run test:docs: 48/48 passed. - npm test: broker-core 294/294, client 257/257, harness adoption 24/24, app 171/171. - ./scripts/validate.sh passed. - npm run agent:verify -- --profile spec-only --paths .agents/manifests/specs.yaml .github/workflows/ci.yml scripts/package_cli.sh scripts/validate_cli_tar.mjs docs/test/front-door.test.mjs spec/build-and-test.md spec/project-structure.md --session-dir task-sessions/rr22-cli-tar-metadata-20260831 passed. - Public surface scanned 261 files; staged current-source audit and semantic exact-diff review passed. Affected: - .agents/manifests/specs.yaml - docs/test/front-door.test.mjs - scripts/package_cli.sh - scripts/validate_cli_tar.mjs - spec/build-and-test.md - spec/project-structure.md Refs: - #38 - #38 (comment) - #38 (comment) - #38 (comment) - https://how.complexsystems.fail Session: - task-sessions/rr22-cli-tar-metadata-20260831
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 55c8f22858
ℹ️ 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".
|
Autopilot needs your choice — AD-20260831-001 Review finding (External review): Concrete operator choices:
Implementation identity (paths and blob SHAs): Guarded hits:
Autopilot's assessment: Concrete operator choices:
Implementation identity (paths and blob SHAs): Guarded hits:
Autopilot recommends: Choose what Autopilot should do next:
Reply with exactly one: Local artifact: job 20260831-220345-83e204f7-03f3-4241-815a-0d1a1b1fdbc6/report.md |
|
Autopilot decision AD-20260831-001: choose authorize-contract-change |
Why: - Resolve actionable GitHub review feedback for PR #38. Changed: - Set published CLI tarball and checksum modes to 0644 after private candidate validation, with a packaging-test assertion and SB-PKG-CLI-004. Left the production validator on spec-only because that profile already runs the front-door tests that import it. Classified the Codex summary and parent review body as non-actionable. Verification: - npm test Affected: - .agents/manifests/specs.yaml - .github/workflows/ci.yml - docs/test/front-door.test.mjs - scripts/package_cli.sh - scripts/validate_cli_tar.mjs - spec/build-and-test.md - spec/project-structure.md Refs: - #38 - #38 (comment) Session: - task-session: [controller artifact: jobs/pr-38/20260831-220727-d4c6af61-171e-4259-a16f-4803c7714206/task-session] - report: [controller artifact: jobs/pr-38/20260831-220727-d4c6af61-171e-4259-a16f-4803c7714206/report.md]
|
Reviewed top-level PR feedback and left the current code unchanged. These items were posted as PR review bodies or conversation comments rather than unresolved review threads, so this acknowledgement is recorded on the PR timeline.
|
|
Reviewed top-level PR feedback and left the current code unchanged. These items were posted as PR review bodies or conversation comments rather than unresolved review threads, so this acknowledgement is recorded on the PR timeline.
|
Why: - Close the remaining CLI archive review gaps without overlapping manifest ownership. Changed: - Route the raw CLI tar validator through the implementation profile while retaining spec path authorization. - Normalize complete tarball and checksum candidates to mode 0644 before either final rename. - Align the SB-PKG-CLI-004 publication contract with the atomic mode boundary. Verification: - Focused CLI package regressions: 7/7. - Documentation tests: 48/48. - Full product, app, and harness suites passed. - Required implementation and spec-only profiles passed with the same task fingerprint. - Staged public-source and diff audits passed. Affected: - .agents/manifests/implementation-foundation.yaml - .agents/manifests/specs.yaml - scripts/package_cli.sh - spec/build-and-test.md Refs: - #38 - #38 (comment) - #38 (comment) Session: - task-sessions/rr22-cli-tar-metadata-20260831
VladimirBrejcha
added
autopilot:on
|
Autopilot needs your choice — AD-20260831-002 Review finding (External review): Concrete operator choices:
Implementation identity (paths and blob SHAs): Guarded hits:
Autopilot's assessment: Concrete operator choices:
Implementation identity (paths and blob SHAs): Guarded hits:
Autopilot recommends: Choose what Autopilot should do next:
Reply with exactly one: Local artifact: job 20260831-225719-526fea86-5c4c-40f8-80ad-4e93f67b1c6a/report.md |
|
Autopilot recorded decision AD-20260831-002: selected Operator: @VladimirBrejcha |
|
Reviewed top-level PR feedback and left the current code unchanged. These items were posted as PR review bodies or conversation comments rather than unresolved review threads, so this acknowledgement is recorded on the PR timeline.
|
1 participant
What changed
CLI tarballs now use portable USTAR headers, numeric root ownership, and Darwin-gated suppression of COPYFILE/xattr metadata. The package contract reads raw tar blocks and PAX bodies so hidden AppleDouble or host metadata cannot pass behind filtered tar listings. The existing docs/package suite now runs in both hosted Ubuntu and macOS jobs without changing their time budgets.
This addresses the interaction among source xattrs, tar implementation defaults, host ownership, and listing behavior without introducing a second packaging path.
How you verified
npm run test:docs(43/43)npm test./scripts/validate.shNotes
USTAR limits are intentional for this small payload. Complete archive bytes are not asserted identical across operating systems because timestamp and traversal-order normalization are outside this change; raw structure and ownership are enforced instead.