Fail closed on real Polymarket segment incompatibility - #422
Conversation
|
Warning Review limit reached
Next review available in: 40 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe shadow gate now preflights a closed real-market OSS segment, validates the candidate upload and readback, records structured evidence, enforces it through jq policy checks, and tests incompatible, tampered, and correctly ordered scenarios before shadow startup. ChangesReal-market preflight
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ShadowGate
participant OSS
participant CandidateUploader
participant GatePolicy
ShadowGate->>OSS: download and verify closed source triplet
ShadowGate->>CandidateUploader: process real segment and upload preflight output
CandidateUploader->>OSS: publish isolated output triplet
ShadowGate->>OSS: read back and verify output triplet
ShadowGate->>GatePolicy: validate preflight evidence and metrics
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (5)
deployment/aliyun/polymarket-raw-ops-shadow-gate.sh (2)
871-872: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
envis invoked but not in the required-command bootstrap list.Line 636 runs
runuser -u hftcollector -- env HOME=....envis effectively always present, so this is a hygiene nit only.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deployment/aliyun/polymarket-raw-ops-shadow-gate.sh` around lines 871 - 872, Add env to the required-command bootstrap list alongside the other utilities in the command loop, covering its use by the runuser invocation at line 636.
598-607: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winStability retry loop has no backoff.
The three iterations re-
statimmediately, so a writer that is mid-flush is very likely to lose all three attempts within the same millisecond window. A shortsleepbetween attempts would make the stability check meaningful.♻️ Suggested change
- for _ in 1 2 3; do + for attempt in 1 2 3; do + ((attempt == 1)) || sleep 1 before=$(stat -c '%d:%i:%s:%Y:%Z' "$status_file") || return 1🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deployment/aliyun/polymarket-raw-ops-shadow-gate.sh` around lines 598 - 607, Add a short sleep between failed stability-check attempts in the loop around before, status_json, and after, while preserving the immediate break when before matches after and the existing failure after all three attempts.deployment/aliyun/test-polymarket-raw-ops-control-plane.sh (2)
1716-1773: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider also tampering
statusandcandidate_sha256.The suite covers timing, dataset, digest, success-marker and upload-summary tampering, but not
.real_market_preflight.status = "failed"or acandidate_sha256that diverges from the top-level one — both are load-bearing policy clauses (Lines 61-62 of the policy).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deployment/aliyun/test-polymarket-raw-ops-control-plane.sh` around lines 1716 - 1773, Extend the policy-tampering checks after the existing real-market preflight cases to modify `.real_market_preflight.status` to `"failed"` and assert the policy rejects it, then modify `.real_market_preflight.candidate_sha256` to differ from the top-level candidate digest and assert rejection. Use distinct temporary JSON files and failure messages consistent with the surrounding checks.
2524-2530: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueLine-number ordering check breaks if the preflight is ever called twice.
grep -n ... | cut -d: -f1yields a multi-line value on a second call site, and(( ))then errors out with a confusing message rather than a clear assertion failure. Piping throughhead -n 1(or asserting exactly one match) would keep the failure legible.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deployment/aliyun/test-polymarket-raw-ops-control-plane.sh` around lines 2524 - 2530, Update the line-number lookup for real_market_segment_preflight in the ordering assertion so multiple call sites cannot produce a multiline value; select exactly the first match or explicitly assert that there is one match before arithmetic comparison. Keep the existing preflight_line ordering validation and failure message intact.deployment/aliyun/polymarket-shadow-gate-policy.jq (1)
6-20: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winPolicy does not tie
.fileto the tail of.uri.
oss_tripletvalidates the URI shape and the file-name shape independently, so evidence declaringfile: "market-updates.A...zst"with a URI ending inmarket-updates.B...zstpasses. The gate always emits them consistently today, but this policy is the immutable evidence check, so it's worth pinning.♻️ Suggested hardening
and (.file | type == "string" and test("^market-updates\\.[A-Za-z0-9._-]+\\.ndjson\\.zst$")) + and ($triplet.uri | endswith("/" + $triplet.file))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deployment/aliyun/polymarket-shadow-gate-policy.jq` around lines 6 - 20, Update the oss_triplet policy to require that the validated .file value matches the filename at the end of .uri, while preserving the existing URI and file format checks. Extract or otherwise compare the URI’s trailing filename with .file so mismatched declarations are rejected.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@deployment/aliyun/polymarket-raw-ops-shadow-gate.sh`:
- Around line 871-872: Add env to the required-command bootstrap list alongside
the other utilities in the command loop, covering its use by the runuser
invocation at line 636.
- Around line 598-607: Add a short sleep between failed stability-check attempts
in the loop around before, status_json, and after, while preserving the
immediate break when before matches after and the existing failure after all
three attempts.
In `@deployment/aliyun/polymarket-shadow-gate-policy.jq`:
- Around line 6-20: Update the oss_triplet policy to require that the validated
.file value matches the filename at the end of .uri, while preserving the
existing URI and file format checks. Extract or otherwise compare the URI’s
trailing filename with .file so mismatched declarations are rejected.
In `@deployment/aliyun/test-polymarket-raw-ops-control-plane.sh`:
- Around line 1716-1773: Extend the policy-tampering checks after the existing
real-market preflight cases to modify `.real_market_preflight.status` to
`"failed"` and assert the policy rejects it, then modify
`.real_market_preflight.candidate_sha256` to differ from the top-level candidate
digest and assert rejection. Use distinct temporary JSON files and failure
messages consistent with the surrounding checks.
- Around line 2524-2530: Update the line-number lookup for
real_market_segment_preflight in the ordering assertion so multiple call sites
cannot produce a multiline value; select exactly the first match or explicitly
assert that there is one match before arithmetic comparison. Keep the existing
preflight_line ordering validation and failure message intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7a94db83-3e40-404e-8022-283d1f5b87bf
📒 Files selected for processing (3)
deployment/aliyun/polymarket-raw-ops-shadow-gate.shdeployment/aliyun/polymarket-shadow-gate-policy.jqdeployment/aliyun/test-polymarket-raw-ops-control-plane.sh
![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: 6136b55780
ℹ️ 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".
1 participant
Closes #420
Change contract
Before any 4201-second shadow observation, run the exact candidate Rust uploader against one exact real production closed Polymarket market segment, require at least one quote, verify source and isolated candidate OSS triplets, and bind the readback/content identities into immutable Gate policy evidence.
Out of scope
Dependency or merge order
None. #421 follows after this PR merges; it is a separate rollout unit.
Focused validation
bash deployment/aliyun/test-polymarket-raw-ops-control-plane.shbash -n deployment/aliyun/polymarket-raw-ops-shadow-gate.sh deployment/aliyun/test-polymarket-raw-ops-control-plane.shshellcheck deployment/aliyun/polymarket-raw-ops-shadow-gate.sh deployment/aliyun/test-polymarket-raw-ops-control-plane.shjq -n -f deployment/aliyun/polymarket-shadow-gate-policy.jqquote requires request_status=success.Rollout / rollback impact
Fail-closed Gate-only change. It adds a short pre-observation real-segment preflight and isolated digest/run-addressed OSS output. Failure occurs before shadow startup and leaves production Python services unchanged. Rollback is reverting this PR; no production cutover is included.
PRD is unnecessary because this is a small, already-specified deployment-gate defect with one behavior contract and one rollback unit.
Summary by CodeRabbit
New Features
Bug Fixes
Tests