feat: prepare Localsetup 4.3.0 - #76
Conversation
Release-Type: none
Release-Type: minor
Release-Type: none
Release-Type: none
Release-Type: none
Release-Type: none
github-actions
Bot
added
area/release
![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: 09c5cf89ee
ℹ️ 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".
| tar.add( | ||
| source, | ||
| arcname=root_name, | ||
| recursive=True, | ||
| filter=lambda info: filter_snapshot_archive_member( |
There was a problem hiding this comment.
When source_dir is a linked Git worktree, .git is a regular gitdir: /absolute/path file, so this recursive copy archives that host-local pointer unchanged. Materialized candidates therefore point at the original machine's worktree metadata (which is absent on the remote) instead of containing usable Git metadata, contradicting the full-repository snapshot contract and breaking Git-based candidate work. Resolve and package the linked gitdir/commondir into a self-contained layout, or reject linked-worktree sources explicitly.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 13b21bf, 1e21632, and final sync 06f8b11: snapshots now reject root .git Gitfiles in both source filtering and archive validation, _discover_git_head does not follow a Gitfile, and the public contract documents the linked-worktree/submodule-Gitfile restriction. Added source, crafted-archive, and no-inspection regressions.
| changed_paths = changed_paths_for_ref(repo_root, current) | ||
| if not changed_paths or any(not is_generated_output_path(path) for path in changed_paths): | ||
| if not changed_paths or any( | ||
| not (is_generated_output_path(path) or is_generated_receipt_path(path)) | ||
| for path in changed_paths | ||
| ): |
There was a problem hiding this comment.
A commit whose subject starts with docs: refresh and changes only README.md, ls/docs/README.md, or ls/docs/FEATURES.md is now treated as a generated-docs commit regardless of what changed inside those files. Thus a normal/manual README edit in such a commit is skipped by generated_docs_source_ref, causing generated provenance to point at its parent and omit a real source change. Restrict receipt paths here to facts-block-only diffs against the commit parent, rather than accepting the pathname alone.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in e8572e8 and final sync 06f8b11: docs: refresh receipt paths now require a commit-aware, facts-block-only diff including metadata and marker validation. Added valid receipt-chain, manual-content, and malformed-marker regressions.
09c5cf8 to
06f8b11
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 06f8b11d70
ℹ️ 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".
| claim = claim_oldest_packet(queue_root) | ||
| if claim is None: | ||
| return None | ||
| return materialize_claim(claim, candidate_root) |
There was a problem hiding this comment.
When shared-materialize is invoked with an invalid or unwritable candidate_root, this call first moves the oldest ready packet into claims/ and only then fails during output preparation. Subsequent CLI invocations find no incoming packet and return {"materialized": false} successfully; the CLI exposes no way to materialize a retained claim by job ID. Thus a simple bad output path permanently strands the oldest packet from the supported command-line workflow.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Resolved for 4.3.0 by deferring and removing the unreleased phase-3 Localsetup materializer rather than weakening its security boundary. shared-materialize and all materialization/resume production routes are gone, with an argparse regression proving the removed command is rejected. Retained Localsetup operations stop at immutable snapshot and deposit/list/claim transport, so Localsetup no longer claims a packet before candidate-output preparation.
| if os.path.lexists(job_dir): | ||
| raise FanoutError("candidate output already exists for this snapshot job") |
There was a problem hiding this comment.
After _reserve_job_dir succeeds, any transient failure during extraction, PRD copy, fsync, or manifest publication leaves a directory without fanout.json; the finally block removes only the staging directory. Retrying the documented materialize_claim(claim, candidate_root) path then always fails here, even though the retained claim is still valid, so a one-time disk or I/O error makes that job unrecoverable without manual filesystem cleanup and a custom caller.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Resolved for 4.3.0 by removing phase-3 fanout, retained-claim resume, reservation recovery, and returned-result handling. These behaviors now belong to a future concrete external harness contract; Localsetup retains only phase-1 snapshot and phase-2 deposit/list/claim transport. This removal avoids shipping an incomplete recovery contract or a same-UID workaround.
| members = list(tar) | ||
| destinations = [ | ||
| (_member_destination(member.name, metadata.source_root_name, destination), member) | ||
| for member in members |
There was a problem hiding this comment.
For the intentionally unbounded full-repository snapshots accepted by this feature, list(tar) retains a TarInfo object for every archive entry before extraction begins. A large repository with hundreds of thousands or millions of files can therefore exhaust the materializer's memory and fail before any candidate is published, despite the archive itself being streamed elsewhere. Process members incrementally (or enforce and document an explicit entry limit) so fanout remains usable for large valid repositories.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Resolved for 4.3.0 by removing phase-3 fanout extraction entirely. The retained phase-1 snapshot validator keeps the independently useful documented member bound and clears TarFile member caching during streamed validation; no Localsetup candidate materializer remains. Generated maps and focused tests were refreshed for the phase-1/2-only surface.
Release-Type: none
Release-Type: none
Release-Type: none
06f8b11 to
be6603d
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: be6603d73d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| validate_snapshot_archive_members( | ||
| temporary_archive, | ||
| root_name, | ||
| error_type=SnapshotError, | ||
| ) |
There was a problem hiding this comment.
For repositories with more than 100,000 entries, this fresh issue comes from the new call to validate_snapshot_archive_members during create_snapshot: that validator now raises on member 100,001, while ls/docs/TRUSTED_WORK_QUEUE.md says the member bound applies to validation only and phase 1 has no size cap. As a result, snapshot-create streams and fsyncs a large valid repo and then aborts before publishing, so the full-repository queue cannot handle large monorepos or checked-in dependency trees. Either move this check out of creation or document/enforce the cap as part of the creation contract.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in source 4f900e5 and final sync 9435952: locally created archives call member validation with max_members=None, while externally supplied archives retain the dynamic 100,000-member validation ceiling and cleared tar-member cache. The focused regression constructs more than 100,000 valid local members without failing creation.
| if info.issym() or info.islnk(): | ||
| raise error_type("archive contains a link member") | ||
| if normalized == f"{root_name}/.git" and not info.isdir(): | ||
| raise error_type("archive Git metadata entry is not a directory") | ||
| if not (info.isdir() or info.isfile()): |
There was a problem hiding this comment.
Fresh evidence beyond the earlier root-Gitfile discussion: this check only rejects the top-level repo/.git member, while _scan_source_tree accepts regular files everywhere else. A repository containing module/.git as a Gitfile (for example a submodule or nested worktree pointer) is therefore archived with its gitdir: pointer intact, despite the snapshot contract saying Gitfiles, including submodule metadata pointers, are rejected. Reject any path component named .git that is not a directory before publishing the archive.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in source 4f900e5 and final sync 9435952: source scanning and archive validation now reject every non-directory entry whose basename is .git at any depth, while allowing ordinary descendants under a real .git/ directory. Nested source and crafted-archive Gitfile regressions are included.
| def _read_request(value: str) -> Any: | ||
| try: | ||
| text = sys.stdin.read() if value == "-" else Path(value).read_text(encoding="utf-8") | ||
| return json.loads(text) | ||
| except (OSError, UnicodeError, json.JSONDecodeError): |
There was a problem hiding this comment.
When --request points at a FIFO/device such as /dev/zero, an endless pipe, or a very large file, this reads the entire input before any JSON shape validation, so the selector can hang or exhaust memory instead of returning an invalid_request receipt. Require a regular file with a small maximum size and apply the same bounded read to stdin before calling json.loads.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in source 4f900e5 and final sync 9435952: file and stdin routing requests are capped at 65,536 bytes before JSON parsing. File reads require a no-follow, nonblocking, identity-bound regular-file descriptor with pre/post checks; FIFO/device/symlink/oversize and pathological JSON inputs return the existing exit-0 invalid_request receipt with empty stderr. Focused routing suite: 61 passed.
| def _read_manifest(path: Path) -> SnapshotMetadata: | ||
| try: | ||
| payload = json.loads(path.read_text(encoding="utf-8")) | ||
| except (OSError, UnicodeError, json.JSONDecodeError) as exc: | ||
| raise SnapshotValidationError("snapshot sidecar is not valid JSON") from exc |
There was a problem hiding this comment.
When a shared queue or supplied archive has an oversized *.manifest.json, validation loads the whole sidecar with read_text() before any schema or size checks, and shared-list, shared-claim, and shared-deposit all reach this path. A malformed packet can therefore force the queue consumer to allocate memory proportional to an attacker-controlled file; open the sidecar as a non-symlink regular file and reject it above a small metadata limit before decoding JSON.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in source 4f900e5 and final sync 9435952: snapshot sidecars are capped at 16 KiB and read through a no-follow, nonblocking, identity-bound regular-file descriptor before UTF-8/JSON decoding. FIFO swaps, symlinks, nonregular files, valid oversized JSON, and pathological JSON are rejected through the public validation exception. The public manifest contract now states the aggregate UTF-8 limit, including opaque identity values.
| def _skill_root() -> Path: | ||
| return Path(__file__).absolute().parent.parent |
There was a problem hiding this comment.
When this skill is run from the normal symlink adapter mode (.agents/skills/<skill> pointing at the managed package), __file__ remains under that symlinked adapter path, and the later descriptor walk opens every component with O_NOFOLLOW. The selector therefore rejects its own bundled resources with resource_invalid even though the installed skill is valid; resolve __file__ to the real package root before the no-follow resource checks, or explicitly allow the expected adapter symlink while still binding the resource files.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in source 4f900e5 and final sync 9435952: _skill_root() now resolves __file__ strictly to the real installed package before descriptor-anchored resource traversal, preserving normal symlink-adapter execution without allowing symlinked resource descendants. Symlink-adapter and strict-resolution failure regressions are included.
be6603d to
9435952
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9435952579
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| digest = hashlib.sha256() | ||
| total = 0 | ||
| try: | ||
| with source.open("rb") as reader, destination.open("xb") as writer: |
There was a problem hiding this comment.
When the shared-folder transport copies snapshot.tar.gz, its sidecar, and prd.bin, destination.open("xb") creates each file with Python's default 0666 & umask permissions and only tightens it to 0600 after the copy and fsync complete. On common permissive umasks such as 022 or 000, another local user/process with access to the queue parent can read the full repository snapshot or opaque PRD bytes during that window, and a crash before the later chmod leaves the staged file too broadly readable until cleanup runs. Open the destination with os.open(..., O_CREAT|O_EXCL, 0o600) or fchmod before writing so sensitive queue members are never published with group/world-readable bits.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in source 839fee8 and preserved through final source 9a67cf1 / sync 3dea2c8: queue packet members are created with O_CREAT|O_EXCL at mode 0600 and immediately fchmoded before the first write. Regressions observe archive, sidecar, and PRD descriptor modes under permissive and restrictive umasks. Exact final validation: 109 focused tests plus two subtests and full suite 1610 passed.
| try: | ||
| _write_manifest(manifest, metadata) | ||
| except SnapshotError: | ||
| _unlink_quietly(archive) |
There was a problem hiding this comment.
If manifest publication fails after this process has hard-linked the archive, a concurrent actor in the destination directory can replace archive before this cleanup path runs; _unlink_quietly(archive) then removes the replacement even though it is no longer the file this snapshot operation published. This breaks the no-clobber behavior the module otherwise preserves for concurrent publications, so the cleanup should be identity-bound like _unlink_if_same_inode(...) or leave the already-published archive in place for operator cleanup.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in source 839fee8 and preserved through final source 9a67cf1 / sync 3dea2c8: snapshot publication retains the exact archive device/inode and manifest-failure cleanup unlinks only that identity. Regressions cover ordinary cleanup and concurrent replacement preservation. Exact final validation: 109 focused tests plus two subtests and full suite 1610 passed.
| try: | ||
| if path.is_symlink(): | ||
| raise SharedFolderError("queue ready marker must not be a symbolic link") | ||
| loaded = json.loads(path.read_text(encoding="ascii")) |
There was a problem hiding this comment.
When listing or claiming a user-selected shared queue, a malformed packet can provide packet.json as a FIFO or a huge regular file; this path only rejects symlinks and then calls read_text, so shared-list/shared-claim can block indefinitely or allocate unbounded memory before reporting a queue error. Since the queue root is a shared external storage surface, validate the marker with lstat as a small regular file and open it with no-follow/nonblocking descriptor reads before decoding.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in source 839fee8 and preserved through final source 9a67cf1 / sync 3dea2c8: ready markers are capped at 1,024 bytes and read through lstat plus O_NOFOLLOW|O_NONBLOCK, regular-file/type/identity/size checks, bounded reads, post-read identity checks, normalized decoder failures, and deterministic close handling. FIFO, directory, symlink, valid-oversize, pathological JSON, and closure regressions are included. Exact final validation: 109 focused tests plus two subtests and full suite 1610 passed.
9435952 to
3dea2c8
Compare
|
A final independent exact-candidate review found one additional phase-2 no-clobber edge case before merge: POSIX |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3dea2c88ba
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| total = 0 | ||
| descriptor: int | None = None | ||
| try: | ||
| with source.open("rb") as reader: |
There was a problem hiding this comment.
When the PRD path is in a directory another process can modify, the earlier _require_regular_file() lstat can be invalidated before this source.open("rb"); because this open follows symlinks and does not compare an fd identity back to the checked file, a symlink/rename swap can make deposit_packet() copy arbitrary replacement bytes into prd.bin while still accepting the packet. The archive path is revalidated after copy, but PRD bytes are only hashed after this open, so use the same O_NOFOLLOW + lstat/fstat binding pattern before streaming source packet members.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in sole source 136d959 and final sync b63642d: every queue packet source is opened with lstat plus O_NOFOLLOW|O_NONBLOCK|O_CLOEXEC, descriptor regular/type/device/inode/size binding, fd-backed streaming, post-read descriptor/path stability checks, and deterministic closure. Symlink, nonregular, rename-swap, size-mutation, and close-failure regressions are included. Exact final validation: 300 focused tests and full suite 1621 passed.
| eligible = [candidate for candidate in risk_candidates if all(candidate["capabilities"].get(capability, {"value": "unknown"})["value"] == "true" for capability in required)] | ||
| if eligible: | ||
| winner = min(eligible, key=lambda candidate: (candidate["policy_priority"], LANE_ORDER[candidate["lane"]], candidate["model_id"])) | ||
| return _receipt(status="selected", reason="selected_static_reviewed", digest=digest, summary="static-reviewed-candidate", selected={"lane": winner["lane"], "model": winner["model_id"]}) |
There was a problem hiding this comment.
For successful requests this returns a concrete gpt-5.6-* value in selected.model, but the new skill contract describes the output as a share-safe Agent-* lane recommendation and says the selector does not select a client model. Any consumer that follows the receipt can now treat a volatile provider model as the selected client model, so keep the model id internal to the matrix or change the public contract/schema to avoid advertising lane-only behavior.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in sole source 136d959 and final sync b63642d: successful public routing receipts now expose exactly selected: {"lane": "Agent-*"}. Concrete provider/model IDs remain internal to evidence validation and tie-breaking only; validator, JSON schema, canonical digest, docs, and tests are synchronized. Exact final validation: 300 focused tests and full suite 1621 passed.
| payload = allocate_artifact(location, prepared=prepared) | ||
| exclude = apply_git_exclude(_exclude_plan(location)) |
There was a problem hiding this comment.
For state allocate --scope repo --child ..., this writes the artifact before adding the child state directory to .git/info/exclude; if the subsequent exclude update fails because the exclude file is locked, unsafe, or stale, the command returns an error after leaving the new artifact visible as an unignored worktree file. The non-child path still applies the exclude before allocation, so bind the child root first but keep the exclude-before-write ordering or clean up the allocation on failure.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in sole source 136d959 and final sync b63642d: repo-child allocation now binds/preflights the child, plans and applies its linearized exclude, and only then writes the artifact. Exclude failure returns the existing structured error and leaves no artifact while preserving stale-binding and non-child behavior. Exact final validation: 300 focused tests and full suite 1621 passed.
3dea2c8 to
b63642d
Compare
|
A final independent release review found and closed one additional retained phase-2 race before merge: deposit finalization could interleave with same-job claiming and republish a duplicate ready packet after the first copy moved to |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b63642da3d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| "source_dirty": false, | ||
| "source_provenance_hash": "3e1e2c2e2830bc991f263768d41bee75044e1159c5df3f7266d9feab94e17f75", | ||
| "source_root_id": "c568052a6bf2c0049a628e5b2062ec7e071930a5d61e148a8859953b16077bb4", | ||
| "source_commit": "136d9594809cbb0805d3737f00abd5dab6aecec5", |
There was a problem hiding this comment.
When this commit is evaluated on its own, this generated provenance points to 136d9594809cbb0805d3737f00abd5dab6aecec5, which is not an ancestor of fe672fef2243810ac88d8bd2c97b489475989762. A docs-sync run from a checkout of fe672fe will regenerate these artifacts with the checked-out commit and clean/dirty state instead, leaving a diff and failing the git diff --exit-code generated-docs gate. Please regenerate the generated docs from this commit or add the proper generated-doc refresh commit on this branch.
AGENTS.md reference: AGENTS.md:L77-L79
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
No generated-doc change was required because the ancestry premise is false for the current PR merge. GitHub merge ref 2690d3236d626a2329385bddbb2545faa2c18dff has parents 0f3dd580aaabe79ec96d71bbb08cc9d7771d07fd and exact head 6797b9d226d8d32cee5de30d0d30ceb9131a2d54; git merge-base --is-ancestor 1afc80397f6cfc1b6eeed6624fe0caa8d6740b4e 2690d3236d626a2329385bddbb2545faa2c18dff succeeds. Both documentation generators and clean no-fix publish preflight completed with zero tracked diff on exact head 6797b9d; fresh remote generated-doc CI is now the remaining gate.
| "source_dirty": false, | ||
| "source_provenance_hash": "3e1e2c2e2830bc991f263768d41bee75044e1159c5df3f7266d9feab94e17f75", | ||
| "source_root_id": "c568052a6bf2c0049a628e5b2062ec7e071930a5d61e148a8859953b16077bb4", | ||
| "source_commit": "136d9594809cbb0805d3737f00abd5dab6aecec5", |
There was a problem hiding this comment.
When this commit is evaluated on its own, this generated provenance points to 136d9594809cbb0805d3737f00abd5dab6aecec5, which is not an ancestor of fe672fef2243810ac88d8bd2c97b489475989762. A docs-sync run from a checkout of fe672fe will regenerate these artifacts with the checked-out commit and clean/dirty state instead, leaving a diff and failing the git diff --exit-code generated-docs gate. Please regenerate the generated docs from this commit or add the proper generated-doc refresh commit on this branch.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
No generated-doc change was required because the ancestry premise is false for the current PR merge. GitHub merge ref 2690d3236d626a2329385bddbb2545faa2c18dff has parents 0f3dd580aaabe79ec96d71bbb08cc9d7771d07fd and exact head 6797b9d226d8d32cee5de30d0d30ceb9131a2d54; git merge-base --is-ancestor 1afc80397f6cfc1b6eeed6624fe0caa8d6740b4e 2690d3236d626a2329385bddbb2545faa2c18dff succeeds. Both documentation generators and clean no-fix publish preflight completed with zero tracked diff on exact head 6797b9d; fresh remote generated-doc CI is now the remaining gate.
| digest = hashlib.sha256() | ||
| total = 0 | ||
| try: | ||
| with path.open("rb") as handle: |
There was a problem hiding this comment.
When a queue packet is being loaded or claimed while another same-user process can mutate incoming/<job-id>, this reopens prd.bin by pathname after _require_regular_file() has already done the non-symlink check, so a swap in that window can make the consumer follow or block on an attacker-chosen symlink/FIFO instead of the packet member. The ready marker and sidecar readers use descriptor-bound O_NOFOLLOW identity checks; please apply the same pattern here before hashing the PRD.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in sole source 1afc803 and final sync 6797b9d: packet PRD hashing now reuses the no-follow/nonblocking/close-on-exec regular-file descriptor binder, hashes through that descriptor, and post-validates descriptor/path identity, type, size, and exact bytes before deterministic close. FIFO, directory, symlink, rename-swap, size-mutation, read-failure, and closure regressions are included. Exact focused validation: 340 passed plus 5 subtests; full clean-environment suite: 1631 passed.
| raise SnapshotValidationError("cannot stat snapshot archive") from exc | ||
| if archive_size != metadata.total_bytes: | ||
| raise SnapshotValidationError("snapshot archive byte count does not match sidecar") | ||
| if _sha256_file(archive) != metadata.archive_sha256: |
There was a problem hiding this comment.
When validating a packet archive in a queue directory that another same-user process can still mutate, the earlier symlink/file checks are separated from this path reopen, so snapshot.tar.gz can be swapped for a FIFO or symlink before hashing. That makes list, claim, or deposit follow or block on the replacement path before the validator can reject the packet; please open the archive once with O_NOFOLLOW/O_NONBLOCK and validate/hash that bound descriptor.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in sole source 1afc803 and final sync 6797b9d: external snapshot validation opens the archive once with O_NOFOLLOW|O_NONBLOCK|O_CLOEXEC, hashes that bound descriptor, rewinds the same file object, passes it directly to tarfile.open for member validation, then post-validates digest, descriptor/path identity, type, size, and exact bytes before deterministic close. Regressions cover FIFO/symlink/nonregular inputs, rename and size mutation, flags, closure, and direct observation that tar validation shares the hashed descriptor. Exact focused validation: 340 passed plus 5 subtests; full suite: 1631 passed.
b63642d to
6797b9d
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
1 participant
Summary
actions/setup-pythonuses across nine workflows to immutable v7.0.0 commit5fda3b95a4ea91299a34e894583c3862153e4b97, superseding chore(deps): bump actions/setup-python from 6.3.0 to 7.0.0 in the github-actions group #75 once this release reachesmain1afc80397f6cfc1b6eeed6624fe0caa8d6740b4eand sole tool-owned final sync6797b9d226d8d32cee5de30d0d30ceb9131a2d54Review resolution
1afc80397f6cfc1b6eeed6624fe0caa8d6740b4eValidation
version-sync --checkand clean no-fixpublish-preflightbb8a00ee799fa725d5f6a9c3ebcb22e6bc64a5a2e210a8bd8856f9f4a549977b, and CycloneDX SBOM at source commit6797b9d226d8d32cee5de30d0d30ceb9131a2d54, with 26 expected components and no discrepanciesSupply-chain evidence
Fresh OSV, GitHub Advisory Database, NVD, and CISA KEV checks found no exact
actions/setup-pythonv7.0.0 target advisory. deps.dev does not map this GitHub Action and remains a documented coverage limitation. Official immutable tag/release metadata binds v7.0.0 to the pinned commit; Localsetup uses no removedpip-installinput and all consumers run on GitHub-hostedubuntu-latestwith Python 3.12.