fix(opencode): preserve redaction-safe gateway failure provenance - #2115
fix(opencode): preserve redaction-safe gateway failure provenance#2115seonghobae wants to merge 9 commits into
Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks 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 |
|
Single-writer handoff (exact head This PR remains open/Draft because it contains valid unique deltas that must be completely integrated into #2114 before any retirement: credential-shaped provider/model identifier rejection, bounded/depth-safe response-body handling, and the associated direct gateway receipt fixtures. #2114 must preserve those commits/diff requirements, reach protected hosted equivalence, and obtain fresh exact-head evidence; predecessor evidence is not transferable. No Force Push, destructive rebase, merge, or Close is authorized by this handoff. |
|
Additional exact-head source review for
Keep Draft/Proposed. These findings must either be repaired here or be carried completely into canonical #2114 with direct RED→GREEN fixtures before this predecessor can retire. |
|
Re-review after concurrent non-force advance to exact head The two findings above remain present in the exact remote source: empty JSON still skips the gateway envelope entirely, and |
seonghobae
left a comment
There was a problem hiding this comment.
Exact-head repair review (COMMENT, not approval) for a6d70b879c8f511e76babd3d7dadfb0df790ab59.
The initial independent review identified two substantive findings: credential-shaped values could pass through provider/model identifier logging, and the JSONL parser slurped unbounded provider-controlled input. RED bdd93f38… reproduced the disclosure and oversized-envelope case; the deeply nested envelope already failed closed. GREEN 17a5efb2… caps parsing to the final 16 KiB and rejects credential-shaped identifiers. Re-review found both findings closed and no new source finding.
Fresh exact-tree evidence:
- focused runner: 38 passed
- full
GITHUB_ACTIONS=true -W error: 3,048 passed, 1 skipped, 36 subtests - Strix quick gate: PASS (default timing and reduced deterministic timing)
compileallandgit diff --check: PASS- remote tree
b89477c5135dc293f0d314ca546ed046aad877aaequals the verified local tree
This review does not qualify as independent approval. Hosted exact-head Checks and an independent formal verdict remain mandatory.
seonghobae
left a comment
There was a problem hiding this comment.
Exact-head security review for a6d70b879c8f511e76babd3d7dadfb0df790ab59 (COMMENT, not approval). The bounded tail and enum phase/reason changes are useful, but identifier redaction is not a closed confidentiality boundary.
seonghobae
left a comment
There was a problem hiding this comment.
Exact-head review for 89de8288772b2cc09bdba7f70fc85ccbd2ad3792 (COMMENT, not approval).
P1 — Raw provider prose still spoofs the public failure class. The new bounded gateway telemetry correctly emits only allowlisted structured fields, but emit_sanitized_opencode_failure_detail independently classifies the full JSON/stderr using unrestricted substring matching. Production-shaped counterexample on this exact tree: structured provider_status=502 and error_code=provider_transport, with ignored message text payment required, emits contradictory public evidence:
class=credit-exhaustedreason=provider_transport ... status=502
The command returned 1 and reproduced this without printing the raw body. This can route operations to the wrong causal owner. Add the contradictory fixture and derive the public class from validated structured status/reason only; absent, malformed, or contradictory authority must be provider-error/unknown, never a prose guess. The full GITHUB_ACTIONS=true runner file currently passes 39 tests in 66.40s, so existing tests do not cover this contradiction.
This valid finding overlaps #2114's already repaired structured-classification delta. Reconcile one canonical writer and preserve the union before retiring either Draft PR.
|
ConceptWeave dependency read-only checkpoint: exact |
|
Exact-head hosted RCA for
The independent raw-prose causal-spoofing finding also remains source-blocking on this branch. Keep Draft; reconcile that valid delta with #2114 before any predecessor retirement. |
|
Fresh exact-head security RCA after The source-level repair is correct: The remaining cause is commit-range history, not the current file: the PR range still contains predecessor commit Non-destructive repair choices are now bounded: either (a) use a path-and-exact-regex allowlist that applies only to this synthetic regression token, preserving every real secret detector and the enforcement gate, or preferably (b) complete the verified-successor path into canonical-first #2114, which already owns the provenance contract and can carry every valid #2115 test/ADR/changelog requirement without inheriting the contaminated predecessor commit range. Keep #2115 Draft until complete mechanical requirement/delta carryover is proven; do not close it merely because the current source no longer contains the literal. |
|
BandScope downstream fresh-owner handoff: current live #2115 head is |
seonghobae
left a comment
There was a problem hiding this comment.
P1 — current head still lets unauthenticated raw provider prose select the public failure class. emit_sanitized_opencode_failure_detail() runs broad grep rules over the complete JSON/stderr before it parses the structured gateway receipt. A production-shaped provider_status=502 / error_code=provider_transport receipt containing ignored payment required text therefore emits class=credit-exhausted alongside structured transport telemetry. The latest commits suppress identifier values and reshape the synthetic credential, but do not change this causal precedence. Make the bounded structured receipt the sole authority for a specific class; absent, malformed, oversized, conflicting, or unproven evidence must remain provider-error/explicit unknown. Preserve the current 16 KiB bound and non-disclosure. Exact focused suite is GREEN (39 passed), demonstrating the regression is missing rather than already covered. COMMENT only; this is not approval.
|
Post-retirement branch-ref reconciliation The branch ref advanced force=false after the PR snapshot detached: Canonical successor #2114 exact head |
|
Fresh successor-carryover verification after retirement:
This substantiates complete valid-delta preservation in the open canonical successor. It does not mark the work shipped: #2114 remains Draft and unmerged while central CodeQL settlement and current-head independent approval are unresolved. |
Goal
Restore actionable, redaction-safe provider failure provenance for the canonical OpenCode model pool without changing model selection, retry, timeout, or provider policy.
Incident evidence
#2106 run 34693400612 exhausted the sole
contextual-orchestrator/orchestrator/freecandidate while retaining onlyclass=provider-error json-bytes=836 stderr-bytes=0. That cannot distinguish provider transport, rate limit, request size, queue admission, malformed output, and missing served-model failures.Test-first repair
Fixtures cover HTTP 429, HTTP 502, request-too-large, queue admission, malformed model output, missing served model, malformed JSON, oversized/deep envelopes, credential-shaped identifiers, and raw body/secret non-disclosure. Malformed input emits fixed decode/malformed/unknown values; arbitrary provider bodies and credentials remain suppressed.
Exact-tree verification
Remote tree
b89477c5135dc293f0d314ca546ed046aad877aaequals the independently verified local tree.compileall: PASSgit diff --check: PASSThis PR is Proposed and Ready-for-review admission only. It requires exact-current-head hosted Checks and qualifying independent review before ordinary merge. No bypass or auto-merge is requested.
Exact-head confidentiality repair — 2026-09-12
Current-head review found that the provider/model prefix denylist still published an unrecognized credential-shaped value. RED
test_gateway_failure_identifier_fields_fail_closed_without_catalog_proofreproducedBYTEZ_TEST_SECRET_1234567890reaching public output. Exact head89de8288772b2cc09bdba7f70fc85ccbd2ad3792now emits fixedprovider=unknownandserved_model=unknownuntil a versioned CO-issued non-secret catalog receipt can be validated locally; allowlisted phase/reason/status and caller-measured duration remain available.Focused gateway telemetry tests pass 10/10 in both normal and
GITHUB_ACTIONS=trueenvironments. Full exact-tree verification passes 3049 tests, 1 skip, and 36 subtests; Bash syntax, Python compilation, and diff checks pass. The review thread is resolved. Replacement exact-head hosted workflows are34698741555,34698741618,34698741543,34698741636, and34698741575; they are nonterminal and no qualifying approval exists. Keep Draft/Proposed until current-head checks and independent review complete.Verified complete successor carryover — 2026-09-13
Canonical successor #2114 exact head
f4f0166bfdc23d381444c94dd0a72d8fd69f23cd, tree592c6f92e16526bad43d70d6170490d3fb538647, completely carries every valid delta from this head:027036db959cc99587a8b51f398045a6cd84cea8and5258f31a1b4fabf6bd7ddc0ffc8477328f8e0ef0;No valid commit delta, diff requirement, fixture behavior, or documentation evidence remains unique here. Closing this predecessor is verified successor retirement, not a simple Close, merge claim, evidence transfer, or release claim. #2114 remains Draft/Proposed until its own exact-head checks and independent approval complete.
Security history repair: exact branch ref
422770b079d55fb1db8b3b20066f7e28b0ec5788(treef2e0e92f4044d6951a0736317c0f400d0d7fb3ae) adds only an AND-scoped Gitleaks allowlist for the exact synthetic token intests/test_opencode_model_pool_runner.py. This preserves the test-first commit history without exposing or broadly exempting credentials. Fresh PR-head synchronization and hosted validation remain required; predecessor Security failure is not transferred.