Skip to content

Commit 50e4654

Browse files
Redact credentials from logged URLs, VEX product ids and hosted Composer locks (#1026)
* Redact credentials from every URL the CLI shows or logs Add utils::redact, the one place a URL is made safe to show: userinfo, the Socket grant-token path level (the level before the patch uuid under /patch/ or /patch-registry/) and secret query values become <redacted>. Route every emission through it: - VendorWarning::new redacts its detail, so the vendor_prebuilt_downloaded advisory (8 hand-written copies, now one VerifiedArchive:: downloaded_warning builder) no longer puts the grant URL into --json events and --verbose output; - registry_fetch::download and maven_repo::fetch_registry_bytes errors (GOPROXY / .npmrc / mirror userinfo, reqwest's own error text); - api::client debug_log, network_error_detail and the artifact not-found / still-building / non-http errors; - telemetry::sanitize_error_message; patch_fetch_failed now reports the uuid field only when the identifier is a uuid, not a CVE, purl or private package name. Consolidation: redirect::redact_grant_token (1 caller, vlt preflight) is deleted and the caller routed through redact_urls_in; the userinfo-safe host extractor hosted::guidance::url_host and the upstream/uv.rs copy are replaced by utils::redact::url_host. A ratchet test pins the download advisory to its single builder. Audit B26. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * Keep git remote credentials out of the VEX product @id remote_url_to_purl fell back to the raw origin URL for any host other than github.com / gitlab.com / bitbucket.org and for any non owner/repo path (a GitLab subgroup), so a CI clone's https://gitlab-ci-token:<job token>@host/... or https://<PAT>@host/... was published verbatim in the OpenVEX document. The fallback now goes through utils::redact::strip_url_credentials (userinfo and secret query parameters dropped); an ssh login name (ssh://git@host/...) and scp-like remotes stay as they were, so existing product ids do not move. Audit B21. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * Drop Composer transport-options in the hosted lock rewrite Composer copies a repository's options (http.header auth tokens, ssl client certificates, http.proxy) into every lock entry it resolves from it as transport-options, and applies them to that entry's dist download. The hosted rewrite retargeted dist.url to the patch host but kept transport-options, so every composer install sent the private repository's credentials to the hosted patch server. The origin-bound entry members are now one list, formats::composer::ORIGIN_BOUND_ENTRY_KEYS (source, transport-options), used by both backends: the vendored rewrite_lock_entry (which already dropped both by hand) and the hosted apply_dist_edit, which now drops each member, re-scanning between removals so adjacent members never leave a dangling comma, and widens the recorded edit to cover every changed byte so the ledger's fragment revert restores them. A new warning, redirect_composer_transport_options_removed, says so. New golden case composer-lock/transport-options (plus its vex-discover golden entry) and a unit test over five member layouts. Fixes #399. Audit B02. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * Tighten the URL redactor and share its host parser Each element of a comma-joined URL list (GOPROXY) is now redacted on its own, an ssh login name without a password is kept, and short secret markers (auth, key, sig, pass) only match whole words of a query parameter name, so author/keyword/design are no longer redacted. The gem mirror and vex-consumed cargo registry host parsers now use the new utils::redact::url_hostname instead of their own userinfo-stripping authority split. The VEX product test pins that a username-only https remote loses its user@ in the product @id. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * Send every debug line through one redacting printer telemetry, jvm_cache and the API client each had their own debug_log, and socket_cli_config, reuse, concurrent and process printed raw eprintln! lines. Only the API client redacted URLs, so the telemetry endpoint and the socket-cli apiBaseUrl were printed with any userinfo. env_compat::debug_log (and debug_message for the API client's deferred lines) is now the one printer and redacts every URL. A ratchet test fails when a core module prints its own [socket-patch ...] line again. The CLI update notifier redacts its lines too. Adds a client test that the artifact errors never quote the grant token or userinfo. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * Redact hosted skip details with the shared redactor wheel_metadata_unavailable and npm_manifest_unavailable replaced only a byte-exact copy of the artifact URL with <hosted artifact>, so a URL reqwest re-rendered, or any other URL in the text, kept its grant token. They and the vlt ARTIFACT_UNVERIFIABLE warning now go through engine::redact_artifact_text: utils::redact::redact_urls_in, then the artifact's own grant token (known from its patch uuid), which also covers a custom --api-url serve root or a non-canonical patch id. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * Pin redaction at the vendor warning and download error sites Regression tests for B26 at the routing points: VendorWarning::new, the vendor_prebuilt_downloaded advisory, and registry_fetch::download for a refused scheme, an HTTP 404 and a refused connection. Removing the redact call from any of them now fails a test. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * Bound the composer dist edit to its entry apply_dist_edit cloned the whole composer.lock and diffed it end to end for every redirected package. It now edits a copy of the entry object alone and splices it back once. Output is unchanged: the in-place sweep passes against the golden blessed before this change. The sweep now also generates entries with transport-options and requires a case to reach redirect_composer_transport_options_removed; its golden is re-blessed for the new inputs. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * Pin the redacted hosted skip details in the scan tests The npm manifest and wheel metadata skip details no longer hide the whole artifact URL behind <hosted artifact>; they quote it through the shared redactor. The yarn berry test now checks the grant-token level is redacted. The wheel fixture's URL has no grant token, so that test only checks a detail is present; core's hosted_skip_details_never_carry_the_grant_token covers the redaction. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * Rename the secret loop variable in the redaction tests CodeQL's rust/cleartext-logging matched the variable name `secret` in two test assertion messages (alerts 195, 196). The values are test literals that the assertions prove absent; renaming the binding clears the false positive without changing what the tests check. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * Warn that Composer rollback cannot restore transport-options The hosted rewrite drops a lock entry's transport-options so a private repository's auth headers, client certificates and proxy settings are not sent to the patch host. The upstream restore used by rollback and remove rebuilds only dist and source from packagist, so those options do not come back. Say so in the pin-time warning, warn again at restore time when composer.json declares repository options, and correct the doc comments that claimed a ledger revert always restores them. Co-Authored-By: Claude <noreply@anthropic.com> * Scope the Composer restore warning to packagist options Composer copies a repository's options only into the lock entries it resolves from that repository, and the upstream restore only restores packagist-origin entries. Warn only when composer.json gives packagist itself options, so a private repository's auth headers no longer make every packagist restore claim it lost transport-options. Also stop recommending a bare `composer update <pkg>`, which can move the package and its dependents to newer versions: tell the user to re-lock it at the locked version instead. Co-Authored-By: Claude <noreply@anthropic.com> * Read the packagist host through url_hostname A packagist repository URL with user:pass@ userinfo was read as the username, so the transport-options restore warning was skipped for it. Use the shared url_hostname helper, which skips userinfo and the port. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent f84535b commit 50e4654

46 files changed

Lines changed: 1755 additions & 575 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎crates/socket-patch-cli/src/commands/vex_consumed.rs‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -352,15 +352,10 @@ fn registry_host(source: &str) -> Option<String> {
352352
let url = source
353353
.strip_prefix("sparse+")
354354
.or_else(|| source.strip_prefix("registry+"))?;
355-
let (_, rest) = url.split_once("://")?;
356-
let authority = &rest[..rest.find(['/', '?', '#']).unwrap_or(rest.len())];
357-
let hostport = authority.rsplit_once('@').map_or(authority, |(_, h)| h);
358-
let host = if hostport.starts_with('[') {
359-
&hostport[..=hostport.find(']')?]
360-
} else {
361-
hostport.split(':').next()?
362-
};
363-
(!host.is_empty()).then(|| host.to_ascii_lowercase())
355+
if !url.contains("://") {
356+
return None;
357+
}
358+
socket_patch_core::utils::redact::url_hostname(url).map(str::to_ascii_lowercase)
364359
}
365360

366361
/// The `<host>-<hash>` name of a `…/registry/src/<host>-<hash>` source dir,

‎crates/socket-patch-cli/src/update_notifier.rs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,10 @@ pub struct Notifier {
146146

147147
fn debug_log(debug: bool, message: &str) {
148148
if debug {
149-
eprintln!("[socket-patch update] {message}");
149+
eprintln!(
150+
"[socket-patch update] {}",
151+
socket_patch_core::utils::redact::redact_urls_in(message)
152+
);
150153
}
151154
}
152155

‎crates/socket-patch-cli/tests/scan/hosted_wheel_metadata_order.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -393,10 +393,10 @@ async fn wheel_metadata_failures_fold_in_dep_order() {
393393
for s in doc["redirect"]["skipped"].as_array().unwrap() {
394394
if s["reason"] == "python_metadata_unavailable" {
395395
let detail = s["detail"].as_str().unwrap();
396-
assert!(
397-
!detail.contains(&server.uri()),
398-
"the hosted URL is redacted from the detail: {detail}"
399-
);
396+
// The detail quotes the URL through the shared redactor (this
397+
// fixture's `/wheels/` URL carries no grant token; core's
398+
// `hosted_skip_details_never_carry_the_grant_token` pins it).
399+
assert!(!detail.is_empty(), "the detail names the failure: {doc:#}");
400400
}
401401
}
402402
// The two good wheels still redirect; the refused two stay upstream.

‎crates/socket-patch-cli/tests/scan/hosted_yarn_berry_manifest.rs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -235,9 +235,10 @@ async fn unfetchable_served_manifest_skips_the_patch() {
235235
assert_eq!(skipped.len(), 1, "{doc:#}");
236236
assert_eq!(skipped[0]["purl"], PURL, "{doc:#}");
237237
let detail = skipped[0]["detail"].as_str().unwrap();
238+
// The URL is quoted with its grant-token level redacted.
238239
assert!(
239-
!detail.contains(&server.uri()),
240-
"the hosted URL is redacted: {detail}"
240+
!detail.contains(TOKEN) && detail.contains(&format!("/<redacted>/{UUID}/")),
241+
"the grant token is redacted: {detail}"
241242
);
242243
assert_eq!(doc["redirect"]["redirected"], 0, "{doc:#}");
243244
assert_eq!(

‎crates/socket-patch-core/src/api/client.rs‎

Lines changed: 63 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,9 @@ use crate::api::vendor_prefetch::VendorPrefetch;
2121
pub use crate::api::vendor_prefetch::VendorPrefetchGuard;
2222
use crate::constants::USER_AGENT as USER_AGENT_VALUE;
2323
use crate::utils::digest::is_hex;
24-
use crate::utils::env_compat::{is_debug_enabled, is_offline_env, proxy_url_from_env};
24+
use crate::utils::env_compat::{is_offline_env, proxy_url_from_env};
2525
use crate::utils::notice::{notice_once, Notice};
26+
use crate::utils::redact::{redact_url, redact_urls_in};
2627
use crate::utils::socket_cli_config;
2728
use crate::utils::target::is_uuid_shaped;
2829

@@ -38,10 +39,12 @@ static MULTI_ORG_SHOWN: AtomicBool = AtomicBool::new(false);
3839
/// act on ("Connection refused", a DNS or TLS failure). Causes already
3940
/// spelled out by an outer message are skipped.
4041
fn network_error_detail(e: &reqwest::Error) -> String {
41-
let mut msg = e.to_string();
42+
// reqwest's text names the request URL: a grant URL, or one with
43+
// userinfo from `--api-url` / a proxy, is quoted redacted.
44+
let mut msg = redact_urls_in(&e.to_string()).into_owned();
4245
let mut source = std::error::Error::source(e);
4346
while let Some(cause) = source {
44-
let part = cause.to_string();
47+
let part = redact_urls_in(&cause.to_string()).into_owned();
4548
if !part.is_empty() && !msg.contains(&part) {
4649
msg.push_str(": ");
4750
msg.push_str(&part);
@@ -95,9 +98,14 @@ fn status_error(head: &str, status: StatusCode, text: &str) -> String {
9598
}
9699
}
97100

98-
/// Log debug messages when debug mode is enabled.
101+
/// Log debug messages when debug mode is enabled. Every URL in `message`
102+
/// is redacted first: debug lines quote grant URLs and `--api-url`s, and
103+
/// debug output is routinely pasted into CI logs and bug reports.
99104
fn debug_log(message: &str) {
100-
if is_debug_enabled() && !defer_debug_line(message) {
105+
let Some(message) = crate::utils::env_compat::debug_message(message) else {
106+
return;
107+
};
108+
if !defer_debug_line(&message) {
101109
eprintln!("[socket-patch debug] {}", message);
102110
}
103111
}
@@ -1694,7 +1702,8 @@ impl ApiClient {
16941702
if !(url.starts_with("https://") || url.starts_with("http://")) {
16951703
return (
16961704
ServeDownload::Failed(ApiError::Other(format!(
1697-
"refusing non-http(s) artifact URL `{url}`"
1705+
"refusing non-http(s) artifact URL `{}`",
1706+
redact_url(url)
16981707
))),
16991708
None,
17001709
);
@@ -1843,8 +1852,14 @@ pub(crate) struct DeferredAttempt {
18431852
fn artifact_download_result(outcome: ServeDownload, url: &str) -> Result<Vec<u8>, ApiError> {
18441853
match outcome {
18451854
ServeDownload::Ok(bytes) => Ok(bytes),
1846-
ServeDownload::NotFound => Err(ApiError::Other(format!("artifact not found: {url}"))),
1847-
ServeDownload::Pending => Err(ApiError::Other(format!("artifact still building: {url}"))),
1855+
ServeDownload::NotFound => Err(ApiError::Other(format!(
1856+
"artifact not found: {}",
1857+
redact_url(url)
1858+
))),
1859+
ServeDownload::Pending => Err(ApiError::Other(format!(
1860+
"artifact still building: {}",
1861+
redact_url(url)
1862+
))),
18481863
ServeDownload::Failed(e) => Err(e),
18491864
}
18501865
}
@@ -2831,7 +2846,8 @@ impl ApiClient {
28312846
) -> Result<Vec<u8>, ApiError> {
28322847
if !(url.starts_with("https://") || url.starts_with("http://")) {
28332848
return Err(ApiError::Other(format!(
2834-
"refusing non-http(s) artifact URL `{url}`"
2849+
"refusing non-http(s) artifact URL `{}`",
2850+
redact_url(url)
28352851
)));
28362852
}
28372853
let attempts = self.vendor_retry.attempts.max(1);
@@ -2883,13 +2899,19 @@ impl ApiClient {
28832899
StatusCode::OK => {}
28842900
StatusCode::NOT_FOUND | StatusCode::GONE => {
28852901
return (
2886-
Err(ApiError::Other(format!("artifact not found: {url}"))),
2902+
Err(ApiError::Other(format!(
2903+
"artifact not found: {}",
2904+
redact_url(url)
2905+
))),
28872906
None,
28882907
)
28892908
}
28902909
StatusCode::REQUEST_TIMEOUT => {
28912910
return (
2892-
Err(ApiError::Other(format!("artifact still building: {url}"))),
2911+
Err(ApiError::Other(format!(
2912+
"artifact still building: {}",
2913+
redact_url(url)
2914+
))),
28932915
None,
28942916
)
28952917
}
@@ -5121,6 +5143,36 @@ mod vendor_package_tests {
51215143
}
51225144
}
51235145

5146+
/// The artifact errors quote the grant URL redacted: neither the grant
5147+
/// token nor userinfo reaches the error a caller shows.
5148+
#[tokio::test]
5149+
async fn download_artifact_errors_never_carry_a_credential() {
5150+
const GRANT: &str = "grant-level";
5151+
const UUID: &str = "7c8d9e0f-1a2b-4a1b-8c2d-3e4f5a6b7c8d";
5152+
let grant_path = format!("/patch/pypi/a/1.0.0/{GRANT}/{UUID}/a.whl");
5153+
let server = MockServer::start().await;
5154+
Mock::given(method("GET"))
5155+
.and(path(grant_path.as_str()))
5156+
.respond_with(ResponseTemplate::new(404))
5157+
.mount(&server)
5158+
.await;
5159+
let client = auth_client(server.uri());
5160+
for url in [
5161+
format!("ftp://u:pw@h.example{grant_path}"),
5162+
format!(
5163+
"{}{grant_path}",
5164+
server.uri().replace("http://", "http://u:pw@")
5165+
),
5166+
] {
5167+
let msg = match client.download_artifact(&url).await {
5168+
Err(ApiError::Other(msg)) => msg,
5169+
other => panic!("expected Other, got {other:?}"),
5170+
};
5171+
assert!(!msg.contains(GRANT) && !msg.contains("u:pw"), "{msg}");
5172+
assert!(msg.contains(&format!("/<redacted>/{UUID}/")), "{msg}");
5173+
}
5174+
}
5175+
51245176
/// A promised secondary that 404s must ERROR ("artifact not found"),
51255177
/// and a still-building 408 maps to "artifact still building" — the
51265178
/// arms encoding the no-soft-skip policy for promised artifacts.

‎crates/socket-patch-core/src/crawlers/jvm_cache.rs‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -138,9 +138,7 @@ pub fn push_classified(roots: &mut Vec<JvmCacheRoot>, root: JvmCacheRoot) -> boo
138138

139139
/// A `SOCKET_DEBUG` line from the JVM cache discovery.
140140
pub(crate) fn debug_log(message: &str) {
141-
if crate::utils::env_compat::is_debug_enabled() {
142-
eprintln!("[socket-patch debug] {message}");
143-
}
141+
crate::utils::env_compat::debug_log("debug", message);
144142
}
145143

146144
/// One installed-artifact cache to crawl.

‎crates/socket-patch-core/src/formats/composer/hosted.rs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -258,8 +258,9 @@ pub(crate) fn rewrite_composer_lock(
258258
};
259259
let rewritten = (!already_redirected).then_some(rewritten);
260260
// Drops the entry's `source` wherever it sits and the dist's
261-
// `mirrors` (see `composer_source`); the edit spans both, so the
262-
// ledger's fragment revert restores them byte-for-byte.
261+
// `mirrors` (see `composer_source`); the edit spans both, so a
262+
// ledger fragment revert restores them byte-for-byte (the upstream
263+
// restore does not bring `transport-options` back; it warns).
263264
let span = composer_source::DistSpan {
264265
entry_start,
265266
entry_end,

‎crates/socket-patch-core/src/formats/composer/mod.rs‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,24 @@ use crate::utils::digest::sha1_hex;
2222
use crate::vendor::lock_inventory::{http_url, LockIntegrity, LockfileEntry, SourceKind};
2323
use crate::vendor::path::{parse_vendor_path, VendorPathParts};
2424

25+
/// The lock-entry members that belong to the package's ORIGINAL repository
26+
/// and must not survive a rewrite that points the entry somewhere else
27+
/// (the hosted patch URL, a vendored path dist):
28+
///
29+
/// * `source`: the upstream VCS checkout Composer falls back to (see
30+
/// [`source`]);
31+
/// * `transport-options`: Composer copies the repository's `options`
32+
/// (`http.header` auth tokens, `ssl` client certificates, `http.proxy`)
33+
/// into every entry it resolves from it, and applies them to that entry's
34+
/// dist download. Kept on a redirected entry, they send a private
35+
/// registry's credentials to the new host (#399).
36+
///
37+
/// Both backends drop exactly these: the hosted splice
38+
/// ([`source::apply_dist_edit`]) and the vendored rebuild
39+
/// ([`crate::vendor::composer_lock`]'s `rewrite_lock_entry`, which then
40+
/// writes its own `transport-options`).
41+
pub(crate) const ORIGIN_BOUND_ENTRY_KEYS: [&str; 2] = ["source", "transport-options"];
42+
2543
// ── entry model ──
2644

2745
/// One entry of a parsed `composer.lock` (see [`composer_lock_packages`]).

0 commit comments

Comments
 (0)