Add trusted snapshot package boundary - #212
Conversation
Co-Authored-By: Codex <noreply@openai.com>
Co-Authored-By: Codex <noreply@openai.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: 20eb751105
ℹ️ 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".
Co-Authored-By: Codex <noreply@openai.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e003adb9b0
ℹ️ 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".
Co-Authored-By: Codex <noreply@openai.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f8c70b30d6
ℹ️ 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".
| if value.startswith("0") and value != "0" and not value.startswith("0."): | ||
| raise SnapshotValidationError("adjusted_close must be canonical") |
There was a problem hiding this comment.
Reject fractional strings that begin with . here. _canonical_decimal(".5") currently succeeds, so TrustedSnapshotPackage.read() can accept and expose a package that violates the checked-in schema, whose adjusted_close pattern requires 0.5; this breaks interoperability for any consumer that validates the supposedly canonical package against that schema.
Useful? React with 👍 / 👎.
| while chunk := os.read(fd, 1024 * 1024): | ||
| raw += chunk |
There was a problem hiding this comment.
When the selected regular file is corrupted or attacker-controlled and very large, this loop repeatedly concatenates immutable bytes before the external digest or JSON validation can reject it. That makes readback quadratic in file size and leaves input size unbounded, allowing a bogus artifact to consume excessive CPU and memory; enforce a maximum from fstat() or accumulate chunks linearly.
Useful? React with 👍 / 👎.
| fd = root_fd | ||
| try: | ||
| for part in relative.parts[:-1]: | ||
| next_fd = os.open(part, os.O_RDONLY | os.O_DIRECTORY | getattr(os, "O_NOFOLLOW", 0), dir_fd=fd) |
There was a problem hiding this comment.
On a Unix platform that supports directory-relative opens but does not expose os.O_NOFOLLOW, this fallback silently supplies zero and allows an intermediate symlink to be followed, defeating the method's root-contained, symlink-free trust boundary. Reject the operation when the capability is unavailable rather than weakening the open flags.
Useful? React with 👍 / 👎.
|
Evidence-only final closure record: exact-head Codex review 4784297077 remains substantive with 3 unresolved P2 threads; required test 30243137462 is SUCCESS, but merge state is BLOCKED. Per existing authority, no further patch/review/rerun/reslice/merge is authorized; closing PR unmerged and parking. |
1 participant
Implements the accepted P2-M2C TrustedSnapshotPackage boundary from exact main.
Validation: uvx --offline ruff check, focused pytest (4 passed), compileall, json.tool, diff --check.