Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,14 @@ use std::{env, fs, io};

pub const DEFAULT_VULN_API_URL: &str = "https://cve-worker-staging.corgea.workers.dev";

/// Whether `DEFAULT_VULN_API_URL` is an endpoint Corgea tokens belong to.
/// The `-staging` hostname is historical: this worker serves as the production
/// vuln-api, so a token may be sent to it and authenticated (fail-closed)
/// 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;
Comment thread
juangaitanv marked this conversation as resolved.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
pub const DEFAULT_VULN_API_URL_IS_PRODUCTION: bool = true;
pub const DEFAULT_VULN_API_URL_IS_PRODUCTION: bool = false;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


#[derive(Serialize, Deserialize, Clone)]
pub struct Config {
pub(crate) url: String,
Expand Down
69 changes: 60 additions & 9 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,8 +269,8 @@ struct VulnApiAccess {
}

/// Resolve the shared vuln-api access policy once so both surfaces apply the
/// same base-URL/token-isolation rule (never send a token to a custom URL
/// without explicit opt-in).
/// same base-URL/token-isolation rule (never send a token to an endpoint it
/// does not belong to without explicit opt-in).
fn resolve_vuln_api_access(config: &Config) -> VulnApiAccess {
let token = config.get_token();
let token = token.trim();
Expand All @@ -297,7 +297,7 @@ fn install_wrap_options(
verdict: Some(corgea::precheck::VerdictConfig {
base_url: access.base_url,
mode: access.mode,
public_login_hint: !access.has_token,
public_hint: Some(public_hint_for(access.has_token)),
}),
npm_registry: utils::generic::get_env_var_if_exists("CORGEA_NPM_REGISTRY"),
pypi_registry: utils::generic::get_env_var_if_exists("CORGEA_PYPI_REGISTRY"),
Expand All @@ -323,15 +323,28 @@ fn advisories_options(config: &Config) -> corgea::advisories::AdvisoriesOptions
}
}

/// A token enables authenticated (fail-closed) verdicts — but never against
/// a custom vuln-api URL unless the user explicitly opts in to sending the
/// token there.
/// Which public-mode disclosure to print. A withheld token is not the same
/// situation as no token, and telling a logged-in user to log in is useless.
/// Only consulted in public mode — authenticated runs print no hint.
fn public_hint_for(has_token: bool) -> corgea::precheck::PublicHint {
if has_token {
corgea::precheck::PublicHint::TokenWithheld
} else {
corgea::precheck::PublicHint::NoToken
}
}

/// A token enables authenticated (fail-closed) verdicts — but only against a
/// vuln-api the token belongs to. That means neither a custom URL nor a
/// non-production built-in default, unless the user explicitly opts in to
/// sending the token there.
fn select_verdict_mode(
token: &str,
custom_vuln_api_url: bool,
send_token_to_custom: bool,
) -> corgea::precheck::VerdictMode {
if !token.is_empty() && (!custom_vuln_api_url || send_token_to_custom) {
let trusted_default = !custom_vuln_api_url && config::DEFAULT_VULN_API_URL_IS_PRODUCTION;
Comment thread
cursor[bot] marked this conversation as resolved.
if !token.is_empty() && (trusted_default || send_token_to_custom) {
corgea::precheck::VerdictMode::Authenticated {
token: token.to_string(),
}
Expand Down Expand Up @@ -785,22 +798,60 @@ mod tests {
fn verdict_mode_selection_matrix() {
use corgea::precheck::VerdictMode;

// Built-in default: authenticated only when that default is production.
let default_mode = select_verdict_mode("token", false, false);
if config::DEFAULT_VULN_API_URL_IS_PRODUCTION {
assert_eq!(
default_mode,
VerdictMode::Authenticated {
token: "token".to_string()
}
);
} else {
assert_eq!(default_mode, VerdictMode::Public);
}
assert_eq!(select_verdict_mode("", false, false), VerdictMode::Public);
assert_eq!(
select_verdict_mode("token", true, false),
VerdictMode::Public
);
assert_eq!(
select_verdict_mode("token", false, false),
select_verdict_mode("token", true, true),
VerdictMode::Authenticated {
token: "token".to_string()
}
);
assert_eq!(select_verdict_mode("", false, false), VerdictMode::Public);
}

/// A token reaches only an endpoint it belongs to. A custom vuln-api is
/// not one, so it stays public and says so — the withheld hint exists for
/// exactly that cohort. See COR-1549.
#[test]
fn withheld_token_selects_public_mode_and_withheld_hint() {
use corgea::precheck::{PublicHint, VerdictMode};

// token + custom URL + no opt-in
assert_eq!(
select_verdict_mode("token", true, false),
VerdictMode::Public
);
assert_eq!(public_hint_for(true), PublicHint::TokenWithheld);
// No token is a different situation with different advice.
assert_eq!(public_hint_for(false), PublicHint::NoToken);
}

/// The opt-in is what makes an otherwise untrusted endpoint eligible for
/// the token — and it never manufactures a token that does not exist.
#[test]
fn opt_in_enables_authenticated_for_untrusted_endpoints() {
use corgea::precheck::VerdictMode;

assert_eq!(
select_verdict_mode("token", true, true),
VerdictMode::Authenticated {
token: "token".to_string()
}
);
assert_eq!(select_verdict_mode("", true, true), VerdictMode::Public);
}
}
46 changes: 34 additions & 12 deletions src/precheck/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,34 @@ impl VerdictMode {
pub struct VerdictConfig {
pub base_url: String,
pub mode: VerdictMode,
/// Print the tokenless public-mode hint after a check is attempted.
pub public_login_hint: bool,
/// Disclosure printed after a check is attempted in public mode, so a
/// fail-open run never looks like an enforced one. `None` suppresses it.
pub public_hint: Option<PublicHint>,
}

/// Why the gate is running public (fail-open) checks. Both cases warn, but a
/// user who never logged in needs different advice from one whose token is
/// deliberately withheld from this endpoint.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum PublicHint {
/// No token at all — logging in is the next step.
NoToken,
/// A token exists but is not sent to this vuln-api, so it cannot enable
/// authenticated enforcement.
TokenWithheld,
}

impl PublicHint {
fn message(self) -> &'static str {
match self {
PublicHint::NoToken => {
"warning: using public CVE checks; login enables authenticated enforcement and private Corgea intelligence."
}
PublicHint::TokenWithheld => {
"warning: using public CVE checks; your token is not sent to this vuln-api, so lookup failures warn instead of blocking. Set CORGEA_VULN_API_SEND_TOKEN_TO_CUSTOM_URL=1 to enable authenticated enforcement."
}
}
}
}

/// Threat verdict for one resolved target.
Expand Down Expand Up @@ -845,10 +871,8 @@ fn run_parsed_install(
"warning: transitive dependencies not checked ({reason}); only named packages were verified."
);
}
if verdict::public_verdict(&opts).is_some_and(|cfg| cfg.public_login_hint) {
eprintln!(
"warning: using public CVE checks; login enables authenticated enforcement and private Corgea intelligence."
);
if let Some(hint) = verdict::public_verdict(&opts).and_then(|cfg| cfg.public_hint) {
eprintln!("{}", hint.message());
}

let report = PrecheckReport {
Expand Down Expand Up @@ -877,12 +901,10 @@ fn run_locked_install(
// Direct callers may still disable verdicts completely.
return exec();
};
// Same disclosure as run_parsed_install: a tokenless `npm ci`/`uv sync`
// runs public checks — say so rather than gate silently.
if verdict::public_verdict(opts).is_some_and(|cfg| cfg.public_login_hint) {
eprintln!(
"warning: using public CVE checks; login enables authenticated enforcement and private Corgea intelligence."
);
// Same disclosure as run_parsed_install: an `npm ci`/`uv sync` running
// public checks says so rather than gating silently.
if let Some(hint) = verdict::public_verdict(opts).and_then(|cfg| cfg.public_hint) {
eprintln!("{}", hint.message());
}
let jobs = match lock {
Ok(jobs) => jobs,
Expand Down
4 changes: 2 additions & 2 deletions src/precheck/test_support.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ pub(crate) fn verdict_opts(base_url: &str) -> PrecheckOptions {
mode: VerdictMode::Authenticated {
token: "test-token".to_string(),
},
public_login_hint: false,
public_hint: None,
}),
..stub_opts()
}
Expand All @@ -44,7 +44,7 @@ pub(crate) fn public_opts(force: bool) -> PrecheckOptions {
verdict: Some(VerdictConfig {
base_url: "http://127.0.0.1:9".to_string(),
mode: VerdictMode::Public,
public_login_hint: true,
public_hint: Some(super::PublicHint::NoToken),
}),
..stub_opts()
}
Expand Down
2 changes: 1 addition & 1 deletion src/precheck/verdict.rs
Original file line number Diff line number Diff line change
Expand Up @@ -786,7 +786,7 @@ mod tests {
mode: super::super::VerdictMode::Authenticated {
token: "test-token".to_string(),
},
public_login_hint: false,
public_hint: None,
};

let jobs: Vec<tree::TreePackage> = ["a", "b", "evil", "c", "d", "e"]
Expand Down
51 changes: 51 additions & 0 deletions tests/cli_verdict.rs
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,57 @@ fn tokenless_public_check_discloses_mode() {
);
}

#[test]
fn withheld_token_discloses_public_mode_and_names_opt_in() {
// The cohort this PR creates: a token exists, but it is not sent to this
// vuln-api, so verdicts stay public and lookup failures warn. The old hint
// keyed off "has a token" and went silent here.
let mut checks = HashMap::new();
checks.insert(
key("pypi", "oldpkg", "1.0.0"),
vulnerable_body("pypi", "oldpkg", "1.0.0", "CVE-2024-0001", Some("2.0.0")),
);
let mut h = pip_harness(checks, HashMap::new(), Some("opaque-token"), 0);
let out = h
.cmd
.args(["pip", "install", "oldpkg==1.0.0"])
.output()
.expect("run corgea");
assert_eq!(out.status.code(), Some(1));
let stderr = String::from_utf8_lossy(&out.stderr);
assert!(
stderr.contains("your token is not sent to this vuln-api"),
"a withheld token must be disclosed, not silently downgraded: {stderr}"
);
assert!(
stderr.contains("CORGEA_VULN_API_SEND_TOKEN_TO_CUSTOM_URL"),
"withheld-token warning must name the opt-in: {stderr}"
);
assert!(
!stderr.contains("login enables"),
"a logged-in user must not be told to log in: {stderr}"
);
}

#[test]
fn withheld_token_reports_public_verdict_mode_in_json() {
let mut checks = HashMap::new();
checks.insert(
key("pypi", "oldpkg", "1.0.0"),
vulnerable_body("pypi", "oldpkg", "1.0.0", "CVE-2024-0001", Some("2.0.0")),
);
let mut h = pip_harness(checks, HashMap::new(), Some("opaque-token"), 0);
let out = h
.cmd
.args(["pip", "--json", "install", "oldpkg==1.0.0"])
.output()
.expect("run corgea");
assert_eq!(out.status.code(), Some(1));
let parsed: serde_json::Value =
serde_json::from_slice(&out.stdout).expect("stdout must be valid JSON");
assert_eq!(parsed["verdict_mode"], "public");
}

#[test]
fn custom_vuln_api_url_with_token_does_not_send_token_by_default() {
let (base_url, requests) = spawn_capturing_vuln_api_stub();
Expand Down
Loading