fix(security): catalog discovery must use chat egress policy - #661
fix(security): catalog discovery must use chat egress policy#661cursor[bot] wants to merge 5 commits into
Conversation
Register NVIDIA NIM, OpenAI, OpenRouter, and Bytez credentials into the KV, compose a production agent pool (no GitHub Models), and fail closed on missing keys, 429 failover, and malformed upstream responses. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Replace keyword scoring and YAML-order failover with a single quality-per-unit-cost chooser. 429/5xx/timeout re-runs that chooser on the remaining healthy pool. Empty pool fail-closes without GitHub Models. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Discovery now reuses the chat-path host/IP checks, refuses redirects that would carry a KV Bearer token, and treats HTTP as an explicit lab hook. Reseed replaces the sqlite agent pool so stale discovered workers cannot linger. Sidecar CI fails when /healthz never comes up. Empty credentials are not treated as ready. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
There was a problem hiding this comment.
Verdict
Do not merge this head (64b69d0). The egress/reseed/health direction is correct, but this branch is #642 at ca2dd9f and is missing 1e82827 (live GET /v1/models as the primary catalog, _provider_slots compose, and the capability-tag cap).
Land #675 instead. That branch is 1e82827 plus this egress policy, persist-replace, sidecar /healthz fail-closed, empty-credential NotConfigured, and marker-only tag_discovered_model (the 1e82827 default-reasoning bug is not on this older compose path).
Next action
Leave this PR unmerged. Review and merge #675, then close #642 and this PR as superseded.
Sent by Cursor Automation: Fix Issues
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
…pt-out Match tip substrate: bind-only placeholder SQL and audited provider urllib/TLS paths; product Semgrep gate requires these suppressions.
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current head0b584ec6ccda87f2ebb4e191c89bc1747255ac75. -
Head SHA:
0b584ec6ccda87f2ebb4e191c89bc1747255ac75 -
Workflow run: 32124531648
-
Workflow attempt: 1
Coverage evidence
Coverage evidence job did not run or did not publish coverage evidence.
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Workflow: opencode-sidecar.yml"]
S1 --> I1["GitHub Actions review job"]
I1 --> R1["Review risk: Workflow: opencode-sidecar.yml"]
R1 --> V1["actionlint plus required checks"]
Evidence --> S2["Changed file (12 files)"]
S2 --> I2["repository behavior"]
I2 --> R2["Review risk: Changed file (12 files)"]
R2 --> V2["required checks"]
Evidence --> S3["Docs (9 files)"]
S3 --> I3["operator or user guidance"]
I3 --> R3["Review risk: Docs (9 files)"]
R3 --> V3["docs review"]
Evidence --> S4["Test (8 files)"]
S4 --> I4["regression suite"]
I4 --> R4["Review risk: Test (8 files)"]
R4 --> V4["targeted test run"]
OpenCode Review Overview
Pull request overviewOpenCode cannot approve yet because required coverage evidence did not pass. Review outcome1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
Coverage evidenceCoverage evidence job did not run or did not publish coverage evidence. Changed-File Evidence Mapflowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Workflow: opencode-sidecar.yml"]
S1 --> I1["GitHub Actions review job"]
I1 --> R1["Review risk: Workflow: opencode-sidecar.yml"]
R1 --> V1["actionlint plus required checks"]
Evidence --> S2["Changed file (12 files)"]
S2 --> I2["repository behavior"]
I2 --> R2["Review risk: Changed file (12 files)"]
R2 --> V2["required checks"]
Evidence --> S3["Docs (9 files)"]
S3 --> I3["operator or user guidance"]
I3 --> R3["Review risk: Docs (9 files)"]
R3 --> V3["docs review"]
Evidence --> S4["Test (8 files)"]
S4 --> I4["regression suite"]
I4 --> R4["Review risk: Test (8 files)"]
R4 --> V4["targeted test run"]
|
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current head0b584ec6ccda87f2ebb4e191c89bc1747255ac75. -
Head SHA:
0b584ec6ccda87f2ebb4e191c89bc1747255ac75 -
Workflow run: 32129297029
-
Workflow attempt: 1
Coverage evidence
Coverage evidence job did not run or did not publish coverage evidence.
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Workflow: opencode-sidecar.yml"]
S1 --> I1["GitHub Actions review job"]
I1 --> R1["Review risk: Workflow: opencode-sidecar.yml"]
R1 --> V1["actionlint plus required checks"]
Evidence --> S2["Changed file (12 files)"]
S2 --> I2["repository behavior"]
I2 --> R2["Review risk: Changed file (12 files)"]
R2 --> V2["required checks"]
Evidence --> S3["Docs (9 files)"]
S3 --> I3["operator or user guidance"]
I3 --> R3["Review risk: Docs (9 files)"]
R3 --> V3["docs review"]
Evidence --> S4["Test (8 files)"]
S4 --> I4["regression suite"]
I4 --> R4["Review risk: Test (8 files)"]
R4 --> V4["targeted test run"]


Why
PR #642 seeds the org provider catalog and chooses workers by cost-performance. Catalog
GET /v1/modelsstill sent the KV Bearer token after only a hostname + HTTPS check, followed redirects, and treated anyhttp://seed URL as insecure-ok. That is a credential leak chat would have refused.What landed (on top of #642)
provider_base_url_rejectionfor chat and discovery (public HTTPS, allowlist,getaddrinfoprivate/loopback/link-local/reserved).allow_insecure_discoveryonly (loopback lab fixtures).persist_catalog_to_agents_dbreplaces the sqliteagent_poolso a later static reseed cannot keep stale discovered workers./healthznever succeeds; no-secrets smoke skip happens only after a healthy serve.NotConfigured, no GitHub Models fallback).Do not merge #642 in parallel. This branch is #642 (
ca2dd9f) plus the egress/reseed/health fixes.Tests
Local: 333 passed (full
testsminus fuzz).Papers
Same claim boundary as
docs/doctoring/provider-catalog.mdanddocs/doctoring/cost_performance_routing.md(APA 7). Discovery egress is now an explicit claim: no private/loopback/redirect leak of the KV key.