Integrate raw trusted weekly period bundle verifier - #34
Conversation
Co-Authored-By: Codex <noreply@openai.com>
🤖 Codex PR Review🚫 Merge blocked: 1 serious issue(s) found in high-risk files 🚫 Blocking IssuesThese issues must be fixed before this PR can be merged: 1. 🟠 [HIGH] Logic in
|
![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: f08924aef4
ℹ️ 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 values["snapshot_bytes"] != expected_snapshot_bytes or actual_snapshot != expected_snapshot: | ||
| raise _error("bundle_snapshot_mismatch") |
There was a problem hiding this comment.
When verify_period_bundle() is called on a bundle assembled outside build_period_bundle(), this check only confirms that the snapshot bytes match the caller-provided expected snapshot; it never re-derives _expected_snapshot(expected_lock, expected_identity["reviewed_workflow_sha"]) the way the builder does. A crafted bundle can therefore use a lock and artifact for run A while the canonical snapshot internally names a different run/provenance/workflow SHA, and the manifest can still be reconstructed with the mismatched snapshot hash, so the verifier accepts a bundle that is not actually bound to the lock or identity.
Useful? React with 👍 / 👎.
| if any(type(key) is not str for key, _ in items): | ||
| raise _error("bundle_artifact_invalid") |
There was a problem hiding this comment.
When artifact is a malformed Mapping whose items() yields non-pair entries, this unpacking raises Python's raw ValueError before the later dict(items) try/except can sanitize it. That affects both build_period_bundle() and verify_period_bundle() inputs at the trust boundary, so malformed artifact mappings bypass the promised TrustedPeriodBundleError failure mode instead of failing closed with a stable code.
Useful? React with 👍 / 👎.
| if snapshot["source_attempt"] != 1: | ||
| raise _error("bundle_snapshot_attempt_invalid") |
There was a problem hiding this comment.
For caller-supplied snapshot bytes, this equality accepts JSON values such as true or 1.0 because they compare equal to 1 in Python, and the later canonicalization preserves those non-integer values. That means verify_period_bundle() can return a bundle whose snapshot violates the contract's exact integer source_attempt type; this should use the same exact-int validation applied to the lock and manifest before checking the value.
Useful? React with 👍 / 👎.
Co-Authored-By: Codex <noreply@openai.com>
|
Concentrated closure applied at
Pure A1 scope remains unchanged: no workflow, permissions, Actions, upload/download, rerun, producer, QAR, Pages, legacy, or A1B integration. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e2cfcc3f3c
ℹ️ 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".
| items = list(bundle.items()) | ||
| except (AttributeError, TypeError, ValueError, UnicodeError, OverflowError, RecursionError): | ||
| raise _error("bundle_shape_invalid") from None | ||
| if len(items) != len(_BUNDLE_KEYS) or any(type(key) is not str for key, _ in items): |
There was a problem hiding this comment.
When an untrusted Mapping returns a non-pair entry from items(), list(bundle.items()) succeeds but this generator unpacking raises Python's raw ValueError before the later dict(items) try/except can sanitize it. For example, a bundle item shaped like ('artifact', value, 'extra') bypasses the documented TrustedPeriodBundleError failure mode at the verify_period_bundle() trust boundary instead of failing closed with a stable code.
Useful? React with 👍 / 👎.
|
PERMANENT_FREEZE/HUMAN_REQUIRED per stop rule. Preserving branch/worktree/commits/comments/threads/checks/tests/evidence; no further code action. |
1 participant
Scope
Fresh PR-A1 integration from merged raw identity A0 at
cf0dcaf6f541d9b04c6f3a319aeb546dc681f725.Non-scope
No workflow, permissions, Actions acquisition, artifact upload/download, rerun,
producer, QAR, Pages, legacy, migration, or A1B integration. Frozen PR #31 and
PR #32 are not reused.
Verification
python3 -m pytest -q: 128 passeduv sync --locked --extra test: passeduv run --locked pytest -q: 128 passedA privileged workflow remains a later boundary and is not part of this PR.