Harden TQQQ trusted snapshot boundary - #218
Conversation
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: fcf5dca7b3
ℹ️ 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>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e5ad796363
ℹ️ 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".
|
|
||
| import ctypes | ||
| import errno | ||
| import fcntl |
There was a problem hiding this comment.
On Windows, where the standard-library fcntl module is unavailable, importing tqqq_r1_snapshot now fails immediately with ModuleNotFoundError. This bypasses the intended fail-closed platform checks and also makes the otherwise portable path-based verifier unavailable; guard the import as the new trusted-package module does.
Useful? React with 👍 / 👎.
| if source.is_symlink() or not source.is_file(): | ||
| _invalid(f"{name} must be a regular non-symlink file") | ||
| try: | ||
| return source.read_bytes() |
There was a problem hiding this comment.
When an evidence file's parent directory is concurrently writable, the separate is_symlink()/is_file() checks can inspect one inode and read_bytes() can open another. An attacker repeatedly replacing the path with a FIFO or device can therefore block the loader or cause unbounded reads before any hash check occurs; open each file once with O_NOFOLLOW | O_NONBLOCK, validate that descriptor with fstat, and read from the same descriptor.
Useful? React with 👍 / 👎.
| fd = self._snapshot_fd | ||
| if fd >= 0: | ||
| try: | ||
| os.close(fd) | ||
| except OSError: | ||
| pass | ||
| object.__setattr__(self, "_snapshot_fd", -1) |
There was a problem hiding this comment.
Because this frozen dataclass receives an autogenerated hash that includes _snapshot_fd, mutating that field in close() changes both its hash and equality state. If a package is used as a dictionary key or set member and then explicitly or contextually closed, subsequent lookup or removal can fail; exclude lifecycle fields from comparison and hashing or make the class unhashable.
Useful? React with 👍 / 👎.
| @dataclass(frozen=True, init=False) | ||
| class TrustedSnapshotPackage: |
There was a problem hiding this comment.
The dataclass can be shallow-copied even though each instance treats _snapshot_fd as exclusively owned. copy.copy(package) therefore creates two objects holding the same descriptor; closing one leaves the other apparently open, and after that descriptor number is reused, closing or finalizing the copy can close an unrelated resource. Reject copying/serialization or duplicate the descriptor while defining clear ownership.
Useful? React with 👍 / 👎.
| receipt_sha256: str | ||
| calendar_sha256: str | ||
| _snapshot_fd: int | ||
| _snapshot_members: tuple[tuple[str, bytes], ...] |
There was a problem hiding this comment.
The autogenerated dataclass repr includes _snapshot_members, so logging or interpolating a trusted package emits every byte of prices.csv and the metadata files. For real multi-year snapshots this can produce very large logs and unnecessarily expose the complete dataset; declare this internal field with repr=False, as is already done for SnapshotResult.verified_members.
Useful? React with 👍 / 👎.
|
Fail-closed settlement: required CI succeeded at exact head |
1 participant
Summary
Scope
No schema, dependency, workflow, ruleset, provider, data, or live changes.
Verification