COR-1549: decide vuln-api token-send by endpoint trust, not by whether it is the default - #135
Conversation
…ln-api default The token-isolation rule tested trust as "is this the built-in default?", so the default was treated as inherently trustworthy. That default is staging, so a real `corgea login` token was sent to cve-worker-staging with no opt-in and fail-closed enforcement rested on staging auth and seed data. Add DEFAULT_VULN_API_URL_IS_PRODUCTION alongside the URL and reframe the test as "is this an endpoint the token belongs to?". The existing CORGEA_VULN_API_SEND_TOKEN_TO_CUSTOM_URL opt-in now covers both cases, so staging validation runs still work. Malicious blocking is unaffected: public mode is still a verdict pass, so known malicious and known vulnerable both block. Only lookup errors now fail open instead of closed. Flip the URL and the constant in the same change at prod cutover.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Addresses cursor[bot] on src/config.rs:11. README.md and skills/corgea/SKILL.md are the shipped operator and agent contracts, and both still said a `corgea login` token on the default vuln-api enables authenticated fail-closed enforcement. After this PR it does not: the token is withheld from the staging default, so those instructions would leave an operator or coding agent believing lookup failures block when they now warn and continue. Restate the rule in all four places: a token goes only to a vuln-api it belongs to, which today excludes both a custom URL and the built-in default, and CORGEA_VULN_API_SEND_TOKEN_TO_CUSTOM_URL=1 covers both cases. Known-vulnerable and known-malicious packages still block either way.
Addresses cursor[bot] on src/main.rs:335. The hint was gated on `public_login_hint: !access.has_token`, so the one cohort this PR creates — logged in, built-in staging default, no opt-in — got fail-open verdicts and no warning at all. The existing copy would also have been wrong for them, since login alone no longer enables authentication. Replace the bool with `public_hint: Option<PublicHint>` so the flag cannot disagree with its reason, and print copy per case: NoToken keeps the login prompt, TokenWithheld explains the token is not sent to this vuln-api and names CORGEA_VULN_API_SEND_TOKEN_TO_CUSTOM_URL=1. Both emission sites (parsed installs and lockfile installs) share it. Regression test asserts token + built-in non-production default + no opt-in yields VerdictMode::Public and PublicHint::TokenWithheld.
…o end Addresses two cursor[bot] follow-ups. precheck/mod.rs: the NoToken copy promised that login enables authenticated enforcement, which this PR makes false — logging in moves that user to TokenWithheld and select_verdict_mode still returns Public. State both requirements, a token and an endpoint it can be sent to. tests/cli_verdict.rs: tokenless_public_check_discloses_mode asserted the old unconditional promise, so it pinned the falsehood in place; it now also requires the opt-in to be named. The assertion is case-insensitive because the benefit now opens a sentence. The unit test asserted select_verdict_mode and public_hint_for separately, so it stayed green if install_wrap_options dropped the hint or an eprintln! were deleted. Add CLI regressions driving the real binary with a token: withheld_token_discloses_public_mode_and_names_opt_in asserts the withheld copy, the opt-in name, and the absence of the useless "login enables" advice, and withheld_token_reports_public_verdict_mode_in_json pins verdict_mode. Renamed the unit test to say what it covers: the harness always points CORGEA_VULN_API_URL at a stub, so the built-in-default branch it pins is the one cohort no integration test can reach.
The `-staging` hostname is historical — that worker serves as the production vuln-api, so Corgea tokens belong there. `DEFAULT_VULN_API_URL_IS_PRODUCTION` was set from the hostname rather than from how the endpoint is deployed, which made this PR withhold tokens from an endpoint that legitimately receives them and turned off fail-closed enforcement for every default install and CI job. Set the constant to `true`, restoring authenticated enforcement on the default, and revert the README/SKILL edits that told users login no longer enables it. The `NoToken` hint goes back to naming login as the next step, since on the default endpoint that is once again true. What the PR still buys: token-send is now decided by a property of the endpoint rather than by whether it happens to be the compiled-in default, so repointing the default at an untrusted endpoint withholds the token instead of silently shipping it. Custom URLs keep the pre-existing opt-in, and the withheld-token disclosure — which never existed before — now covers that cohort, which previously got public verdicts with no warning at all. Verified against the real binary: token+default -> authenticated with no warning; no token -> public + login hint; token+custom -> public + withheld hint; token+custom+opt-in -> authenticated.
| /// verdicts apply by default. Set to `false` if the default is ever repointed | ||
| /// at an endpoint that customer tokens should not reach — token-send and | ||
| /// fail-closed then both require the explicit opt-in. | ||
| pub const DEFAULT_VULN_API_URL_IS_PRODUCTION: bool = true; |
There was a problem hiding this comment.
The trust flag currently defeats the security change this PR is scoped to deliver. DEFAULT_VULN_API_URL is still https://cve-worker-staging... (and tests/fixtures/vuln_api/README.md still calls it the staging worker), while the PR contract explicitly says no production worker is being stood up and token-send to this default must require opt-in. With this value true, src/main.rs:346-349 makes trusted_default true, selects Authenticated, and passes every non-empty login token to that endpoint without CORGEA_VULN_API_SEND_TOKEN_TO_CUSTOM_URL=1—the same exposure as main. The matrix test cannot catch a bad classification because it branches on this constant and accepts either outcome.
Set the flag to false, restore the accompanying docs/NoToken wording that login alone does not enable enforcement on the current default, and make the default+token+no-opt-in test assert Public directly.
| pub const DEFAULT_VULN_API_URL_IS_PRODUCTION: bool = true; | |
| pub const DEFAULT_VULN_API_URL_IS_PRODUCTION: bool = false; |
There was a problem hiding this comment.
The premise is out of date rather than the code. cve-worker-staging.corgea.workers.dev serves as the production vuln-api — the -staging hostname is historical — so a token sent there reaches the endpoint it belongs to, which is why dc93969 set the flag to true. @leenk7991 raised the same question on this line and resolved it after that answer.
You did surface a real defect, though: this PR's title and description still described the false behavior, which is the evidence you reasoned from. Both are now rewritten to describe what merged, with a note that the squash commit ca17add carries the old, misleading title.
On the matrix test — correct that it branches on the constant and accepts either outcome, so it cannot catch a wrong classification. That is by design and not fixable in-repo: the constant encodes a deployment fact, not a code property, so no unit test can validate it. The doc comment at config.rs:7-12 and review are the only guards. What the tests do pin is the coupling — withheld_token_discloses_public_mode_and_names_opt_in drives the real binary and asserts a withheld token yields public mode plus the disclosure naming the opt-in.
juangaitanv
changed the title
3 participants
Relates to COR-1549.
What changed
resolve_vuln_api_accesstested trust as "is this the built-in default?":The default was trusted by identity — it is the URL compiled into the binary — rather than by any property of the endpoint. The test is now:
with a companion constant to the URL in
config.rs:truebecausecve-worker-staging.corgea.workers.devserves as the production vuln-api — the-staginghostname is historical. Tokens sent there reach the endpoint they belong to.Behavior
Unchanged for every existing cohort. A token plus the default URL still selects authenticated fail-closed mode; a custom
CORGEA_VULN_API_URLstill needsCORGEA_VULN_API_SEND_TOKEN_TO_CUSTOM_URL=1. Verified against the built binary across all four combinations:verdict_modeWhat it buys
DEFAULT_VULN_API_URLat something customer tokens should not reach, flip the flag in the same change, and the token is withheld. Previously the default was trusted no matter what it pointed at.public_hint: Option<PublicHint>replacespublic_login_hint: bool, keyed on the selectedVerdictModerather than on whether a token exists. Custom-URL users with a token previously got public, fail-open verdicts and no warning at all — the hint was suppressed precisely because they had a token. They now get copy naming the opt-in, and a tokenless user still gets the login prompt.Tests
516 pass, strict clippy clean.
withheld_token_discloses_public_mode_and_names_opt_inandwithheld_token_reports_public_verdict_mode_in_jsondrive the real binary and assert the withheld copy andverdict_mode; the unit tests cover mode and hint selection.Follow-up
COR-1719 — fail-closed and token-send are still the same switch, so CI cannot get strict verdicts without sending a token. Fail-closed is entirely client-side and never needed one.