Repository navigation
Conversation
Reserve paid work atomically, authorize gallery and analyst operations, bound outbound and local workloads, remove executable deserialization, and prevent new private publication artifacts. Reproduce the reviewed source patch directly on remote main and suppress Vercel deployment for this review branch. Validation: 1933 Python passed/5 environment skips;265 public;794 graph;6 Node publication;both builds;all four security verifiers;independent source review. Dedicated inherited binary cluster fixture remains excluded locally. All 19 dispositions and rollout gates are documented. Co-authored-by: Codex <codex@openai.com>
|
@codex review Please independently review the security boundaries at head 1f8109f: atomic fail-closed paid reservations, capability/owned-storage gallery publication, private analyst route authorization and bounded shared workloads, pinned outbound transport, safe non-executable local artifacts/cookies, and publication allowlists. Check regression and compatibility risks. This is a manually requested review of an authorized draft PR; merge and deployment are not approved. Do not perform live exploit probes, paid provider requests, private-data inspection or persistent access changes. |
Updated Python3.11.16 rejects oversized trailers before our wire cap. Convert its HTTPException family, including IncompleteRead, to the guarded response error after closing the stream. Retain the adversarial trailer test and add deterministic public-read cases asserting immediate closure before context exit. Validation:1935 Python passed/5skipped;76 focused outbound tests;22 response-edge tests;independent review plus unmocked truncated-chunk diagnostic. Record the first hosted failure and runtime calibration without claiming new-head CI or completed Codex review. Co-authored-by: Codex <codex@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: 1f8109faff
ℹ️ 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 PurePosixPath(normalized).suffix.lower() in {".pkl", ".pickle", ".db", ".sqlite", ".sqlite3"}: | ||
| return False | ||
| return True |
There was a problem hiding this comment.
When a private artifact is added outside data/ or the few enumerated directories, the check falls through to True; for example, tpot-analyzer/private/export.json and exports/user.parquet both pass this function and therefore the new CI gate. Because private research exports can use arbitrary directories and formats, this does not provide the claimed changed-path allowlist and can publish private data to the public repository. Permit explicit source/config paths instead of allowing every unrecognized path.
Useful? React with 👍 / 👎.
| EXPENSIVE_ENDPOINTS = frozenset({ | ||
| "discovery.discover", "graph.get_graph_data", | ||
| "clusters.get_clusters", "golden.ingest_predictions_run", "golden.run_eval", | ||
| "golden.interpret_tweet", | ||
| }) |
There was a problem hiding this comment.
The shared lease applies to clusters.get_clusters but not to other routes that perform the same or heavier work: a cache miss in clusters.get_cluster_members and clusters.get_cluster_tag_summary calls build_hierarchical_view, while clusters.get_account_membership can run the full compute_grf_membership solve. Concurrent authenticated UI requests to those endpoints can therefore run across processes while another expensive request holds the sole advertised workload slot, defeating the bounded shared-workload boundary.
Useful? React with 👍 / 👎.
| cookies = load_cookies(self._config.cookies_path) | ||
| except (OSError, ValueError) as exc: | ||
| LOGGER.error("Cookie file rejected at %s: %s; recapture with setup_cookies", self._config.cookies_path, exc) |
There was a problem hiding this comment.
When scripts.debug_single_account is used through its documented/default cookie-selection flow, it still searches only for secrets/*.pkl and passes that path into SeleniumWorker. The new load_cookies rejects every non-JSON suffix before decoding, while setup_cookies now creates only .json files, so the debug scraper either reports that no cookie files exist or always fails login. Update that caller's example and discovery logic to select the new JSON artifacts.
Useful? React with 👍 / 👎.
| redis.call('HSET', KEYS[1], ARGV[2], cjson.encode(versions)) | ||
| redis.call('ZADD', KEYS[4], now, ARGV[2]) |
There was a problem hiding this comment.
If gallery:published has an unexpected Redis type because of legacy or corrupted state, HSET succeeds and the following ZADD raises. Redis Lua execution is atomic with respect to concurrency but does not roll back writes after a runtime error, so the handler returns 503 and releases the reservation even though readGallery can already expose the newly written version through the gallery hash. Check all key types before the first mutation or restructure the operation so an error cannot leave a publicly readable partial publication.
Useful? React with 👍 / 👎.
| for (const [kind, body] of [['data', candidate.dataBody], ['search', candidate.searchBody]]) { | ||
| const pathname = `public-site/releases/${candidate.exportDigest}/${kind}.json` | ||
| const blob = await putBlob(pathname, body, { | ||
| access: 'public', addRandomSuffix: false, allowOverwrite: false, | ||
| cacheControlMaxAge: 31536000, contentType: 'application/json', token, | ||
| }) |
There was a problem hiding this comment.
If the data.json upload succeeds but the subsequent search.json upload fails transiently, rerunning the approved command starts with the already-created data pathname and allowOverwrite: false, so it fails before it can create the missing search object. This leaves that digest permanently incomplete unless an operator performs a separately gated deletion. Treat an existing object with matching bytes as success, or publish through a resumable staging/manifest protocol.
Useful? React with 👍 / 👎.
Default-deny unknown publication paths, gate expensive analyst aliases, validate debug JSON cookie selection, preflight gallery and paid-ledger Redis state, and resume immutable uploads with exact approved byte snapshots. Validation: 1976 Python passed / 5 existing skips; 318 public passed; public build; 14 Node contracts; required verifiers and independent review passed. Production activation and historical cleanup remain separately gated. Co-authored-by: Codex <codex@openai.com>
This branch has not been deployed
1 participant
Unauthenticated card regeneration could spend OpenRouter credit, gallery writes lacked ownership, and analyst/local tooling exposed unbounded or executable-data paths. This change enforces those trust and workload boundaries while retaining anonymous cached card reads and BYOK generation.
All 19 findings, qualified/non-applicable claims, calibration and remaining decisions are recorded in the source-only security report. The five Codex findings from the first head were validated and corrected with failing/passing regressions; independent review also found and verified the mutable-buffer repair.
Validation on the corrected local source:
fe176b2c0cb34cbf40d82a0366afdffbc0ac7126: Tests run passed all three jobs. Python 3.11.16: 1,976 passed/five skipped (99.08s); public 318 plus 14 Node; graph 794; both builds and both binary-fixture cluster verifiers passed. Automatic EveryPush review remains to be verified separately; no additional manual review comment is posted.This branch starts directly at remote main
6f030442, carries source/config/tests/docs only, and excludes private artifacts and unpublished ancestors. The owner authorized draft publication, Codex review and subsequently readiness. Both Vercel configs suppress deployment for this branch; no deployment, merge, paid call, live exploit probe, private-data read, credential rotation, history cleanup or visibility change was performed.Before separately approved production activation: provision signing/curator secrets, verify owned Blob/compatible Redis configuration, approve provider billing and budget migration, rebuild/recertify typed artifacts, recapture JSON cookies, and decide historical Git disclosure and physical gallery/feedback retention. Existing directory-placement checks, per-request analyst resource supervision and live Blob compatibility remain qualified in the report.