feat: add verified TQQQ snapshot package boundary - #217
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: 81cd163421
ℹ️ 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>
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: ea2bddba35
ℹ️ 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".
| except tqqq_r1_snapshot.SnapshotValidationError as exc: | ||
| raise TrustedSnapshotPackageError("invalid verified snapshot") from exc | ||
| try: | ||
| resolved_snapshot_dir = verified_snapshot.output_dir.resolve(strict=True) |
There was a problem hiding this comment.
When the snapshot directory is caller-writable, another process can rename it and replace it with a symlink after verify_tqqq_r1_snapshot returns but before this resolution. resolve(strict=True) then follows the replacement, and the new target receives only the weak _snapshot_sessions check rather than full snapshot verification, so an arbitrary directory containing a prices.csv with the bound session can be returned as trusted. Verification, session extraction, and the returned reference need to operate on the same stable directory/inode.
Useful? React with 👍 / 👎.
| parse_constant=_reject_constant, | ||
| parse_float=_parse_finite_float, | ||
| ) | ||
| except (UnicodeDecodeError, json.JSONDecodeError, TrustedSnapshotPackageError) as exc: |
There was a problem hiding this comment.
On supported Python versions with the default integer digit limit, an evidence payload containing an integer literal longer than the limit makes json.loads raise a plain ValueError, not JSONDecodeError. Because this handler omits ValueError, malformed caller-controlled package, receipt, or calendar evidence can escape the loader as a raw exception instead of the documented TrustedSnapshotPackageError; catch and normalize this parse failure as well.
Useful? React with 👍 / 👎.
|
|
||
| def _read_evidence(path: str | Path, name: str) -> tuple[object, str]: | ||
| payload = _read_regular(path, name) | ||
| return read_strict_json(payload, name), hashlib.sha256(payload).hexdigest() |
There was a problem hiding this comment.
When local evidence files are attacker-writable, a substituted package, receipt, or calendar can contain arbitrarily expensive or malformed JSON. Tuple expressions are evaluated left-to-right, so this function parses the payload before computing its digest, and the loader parses all three files before comparing the package and receipt trust anchors; unauthenticated bytes can therefore consume parser resources or raise an exception before the inevitable hash mismatch. Hash each raw buffer first, validate the external package/receipt anchors and the calendar digest from the authenticated manifest, and only then decode those same bytes.
Useful? React with 👍 / 👎.
|
P2 V2 final settlement: final Codex review was performed on immutable head |
1 participant
Summary
Validation
PYTHONPATH=src python3 -m pytest -q tests/test_tqqq_r1_snapshot.py tests/test_tqqq_trusted_snapshot_package.pypython3 -m ruff check ...python3 -m compileall -q srcScope is limited to the two production/test files; no provider, replay, or live execution.