Skip to content

fix(fetch): a registry URL carrying credentials is printed verbatim in error output #144

Description

@justin13888

What happens

A registry root configured with embedded credentials is printed, in full, into dependable's normal output whenever a request to it fails.

The chain, verified in the tree:

  1. crates/dependable-fetch/src/error.rs declares
    #[error("http error: {0}")] Http(#[from] reqwest::Error),
    so reqwest::Error's own Display is propagated verbatim.
  2. reqwest 0.12.28 src/error.rs:268 ends its Display with
    write!(f, " for url ({url})")?;
    and performs no redaction — grep for password, username or redact across that file returns nothing.
  3. url::Url's Display serialises userinfo, so https://user:token@host/path round-trips complete with the credential.
  4. That string becomes DependencyStatus::Error(msg) and is rendered by
    crates/dependable/src/output/table.rs as error: {msg} and by
    crates/dependable/src/output/github.rs as a PR annotation.

Why it matters

Registry roots are user-supplied and are documented as such: .dependable.toml's per-ecosystem registry keys, and .npmrc, whose content expand_env interpolates ${VAR} into. A user who writes https://ci:${NPM_TOKEN}@nexus.internal/repo — a normal way to reach a private index — gets that token written into stdout and into GitHub Actions annotations on every failing request. In GitHub Actions the job log is retained and is readable by anyone with read access to the repository.

The failure mode is not rare in the case that matters: a root whose host is unreachable makes every request to it fail, so the disclosure is per-dependency rather than occasional.

Why it is being filed rather than fixed

Found while implementing #112, which redacts the registry root in the --fail-on refusal line — that refusal now prints nexus.internal rather than the full root. The same run still prints the unredacted URL through the error path above, so the redaction is currently one line deep. output/table.rs and output/github.rs were outside that change's manifest.

Direction

The reduction wants to happen where the error is constructed rather than at each renderer, so a second consumer cannot reintroduce it: strip userinfo from the URL before it reaches FetchError::Http's Display — either by mapping the reqwest::Error into a variant that carries an already-reduced authority, or by giving FetchError its own Display for the HTTP case rather than delegating.

UnreachableRegistry::label() in crates/dependable-fetch/src/check.rs (added by #112) already implements the reduction and its edge cases — a credential containing /, ?, # or \, an IPv6 literal, a missing scheme — with tests. Reusing it is likely cheaper than writing a second one.

Worth deciding at the same time: whether the URL should appear in the message at all, since the ecosystem and package name are already reported alongside it.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions