Skip to content

feat: simplify TQQQ trust boundaries - #195

Closed
Pigbibi wants to merge 2 commits into
mainfrom
codex/tqqq-e1-trust-boundary-simplification-20260725
Closed

feat: simplify TQQQ trust boundaries#195
Pigbibi wants to merge 2 commits into
mainfrom
codex/tqqq-e1-trust-boundary-simplification-20260725

Conversation

@Pigbibi

@Pigbibi Pigbibi commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Scope

  • Add fail-closed regular-file admission, unified trust-schema validation, and digest-directory publication.
  • Keep provider/data/live disabled and size zero.

Verification

  • Focused snapshot tests: 26 passed.
  • Ruff 0.15.9, py_compile, compileall, and git diff --check passed.
  • Full suite: BASELINE_EQUIVALENT_NOT_REGRESSION (same 11 collection errors; canonical SHA 2e9f9fa4e6dec5ee34766904ff6d0bf13ae2fdf48def1aa033068d5348058b7f).

No merge or follow-up execution is authorized.

Co-Authored-By: Codex <noreply@openai.com>
@github-actions

github-actions Bot commented Jul 25, 2026

Copy link
Copy Markdown

🤖 Codex PR Review

🚫 Merge blocked: 1 serious issue(s) found in high-risk files

🚫 Blocking Issues

These issues must be fixed before this PR can be merged:

1. 🟠 [HIGH] Logic in src/us_equity_snapshot_pipelines/tqqq_r1_snapshot.py

verify_tqqq_calendar_endpoint_trusted_snapshot() accepts a self-consistent persisted package where TQQQ is missing one or more completed sessions after 2010-02-11. _validate_persisted_snapshot() only checks that TQQQ dates are a non-empty subset of QQQ dates, so an attacker or corrupted publisher can rewrite prices.csv and manifest.json, rename the directory to the new manifest digest, and the verifier will still return success while silently truncating TQQQ history. (line 580)

Suggestion: Validate the exact expected TQQQ session list during verification, e.g. require tqqq_sessions == [s for s in qqq_sessions if s >= _TQQQ_FIRST_USABLE_SESSION]. If exact QQQ session membership is also part of the contract, persist enough session evidence in the manifest to verify that list exactly as well.

ℹ️ Other Findings

1. 🟡 [MEDIUM] Reliability in src/us_equity_snapshot_pipelines/tqqq_r1_snapshot.py

The new materializer hard-codes now < next_close into _validate_endpoint(), which makes the same trusted calendar/endpoint/runtime inputs unusable as soon as the successor session closes. Any delayed rerun, backfill, or disaster-recovery publication after that time will fail with CALENDAR_ENDPOINT_STALE_AT_OBSERVATION even if the packet was valid when observed and the price data is otherwise correct. (line 608)

Suggestion: Validate the packet against its own timestamps (last_close <= observed < next_close) and remove the wall-clock expiry from deterministic materialization, or make the freshness check an explicit caller-controlled policy outside this function.

2. 🟡 [MEDIUM] Reliability in src/us_equity_snapshot_pipelines/tqqq_r1_snapshot.py

If os.mkdir() succeeds but os.open(package_name, ...) fails, the exception path does not clean up the newly created digest directory because package_identity is still None. That leaves an orphaned sha256-... directory behind, and every retry for the same manifest immediately fails with IMMUTABLE_CREATE_CONFLICT, turning a transient open/fs error into a persistent publication wedge. (line 655)

Suggestion: Capture the directory identity immediately after mkdir (for example with os.stat(..., dir_fd=root_fd, follow_symlinks=False)) or add a best-effort cleanup path that removes the just-created directory by name even when opening it fails.


Review by Codex PR Review bot • PR

Co-Authored-By: Codex <noreply@openai.com>
@Pigbibi Pigbibi closed this Jul 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant