fix(telemetry): stop logging L1 provider credentials

[sveitser]

Operator L1 RPC API keys, embedded in the provider URL path, were shipped in plaintext to the telemetry S3 bucket. Two forms leaked, sharing no common substring:

• reqwest::Error formats the full request URL into its own Debug and Display, so any rendering of an L1 transport error emitted it. {err:#} is not a safe alternative to {err:?} here; both print the URL.
• url::Url's own Debug is field-wise (path: "/v2/KEY") and contains no :// at all, so it is invisible to URL-shaped text matching. It reached logs via the startup struct dump of HotShotInitializer.

Fixes:

• espresso_utils::redact provides scrub for rendered text, handling both forms, and redact_url for a Url in hand.
• RedactingLogExporter wraps the OTLP LogExporter and scrubs every record body and attribute. One boundary rather than one call site per log line, so a new log line cannot reintroduce the leak.
• #[debug(...)] on the L1ClientOptions, SwitchingTransport and SingleTransport URL fields keeps the field-wise form out of any dump. SingleTransport now carries the redacted URL, so the "L1 client error" warning names the provider that failed.
• Redacted at the ten L1-backed handlers in the node API, whose error bodies are served to unauthenticated callers and never pass the exporter.

Operator-local stdout is deliberately left unredacted: it is the operator's own credential on their own host, and every Rust program logging a reqwest error behaves the same way.

The redaction boundary is fail-closed. ends_url_token may only contain characters Url::as_str() percent-encodes in every component; anything else is legal in a path and would end the token early, leaking the tail.

[claude]

Claude finished @sveitser's task in 8m 33s —— View job

---

Review: fix(telemetry): stop logging L1 provider credentials

• Read existing review comments (tmp/pr-review-comments.json — empty, no prior feedback to dedupe against)
• Read the full diff against origin/main
• Review espresso_utils::redact (scrub_urls / scrub_url_debug / redact_url / Redacted)
• Review RedactingLogExporter
• Review L1 #[debug(...)] changes and node API Redacted usage
• Audit for uncovered leak sites
• Assess test coverage
• Post review

---

This is a well-constructed fix. The two-form analysis in the PR description is correct and non-obvious — url::Url's field-wise Debug genuinely contains no ://, and test_scrub_urls_misses_url_debug encoding that as an assertion is exactly the right way to document why a second scrubber exists. Putting the redaction at the exporter rather than per-call-site is the right call, and the test tables (multi-byte input, IPv6, userinfo-with-extra-@, idempotence) are unusually thorough.

Two findings I'd want addressed before merge, plus some smaller items. All are posted as inline comments.

Fail-closed is violated in scrub_url_debug — redact.rs:102-108

{ and } are not in the WHATWG query or fragment percent-encode sets, only the path set, so Url::as_str() leaves them literal there. scrub_url_debug stops at the first } after Url {, which may be one of those. Since query renders after path and before fragment:

Url::parse("https://rpc.example.com/v1?filter={a}&apikey=SECRET")
  → scrub_url_debug → Url { *** }&apikey=SECRET"), fragment: None }

The credential survives, at the boundary that is meant to be the last line of defence. The PR's own scrub_urls table already treats brace-in-query as realistic (?token={FAKEKEY}). A quote-toggle finds the real closing brace soundly, since " is encoded in every component. Fix this →

The new L1 client error log emits the raw credential — impls/l1.rs:392-396

url = %current_transport.url is Display on a raw Url, so it prints userinfo and key in full. The #[debug(...)] attribute on the field only governs Debug. The PR description says "SingleTransport now carries the redacted URL" — it doesn't; it carries the raw one and relies on the exporter to clean up after it. Since the exporter would reduce this attribute to exactly redact_url output anyway, logging the redacted form directly is free and removes both the plaintext stdout emission and the dependency on the downstream boundary. Fix this →

Smaller items

Where	Item
redact.rs:121-124	Redacted uses scrub_urls, not scrub — so the one boundary with no exporter behind it, serving bodies to unauthenticated callers, is closed against only one of the two forms. Intentional?
redact_exporter.rs:179-187	passes_through_record_without_url_unchanged leaves all fields but body and one attribute unset, so the assert_eq! can't catch scrub_record dropping a field. Nine fields are hand-copied from a dependency-owned type; an SDK upgrade adding a tenth would silently lose it on every record.
redact.rs:13-18	ends_url_token's comment misattributes `, {, } to the path (they're encoded there; they're legal in query/fragment). Same imprecision underlies the scrub_url_debug bug. Set itself is correct.
redact_exporter.rs:121-127	"Url {" matches as a substring, so a future BaseUrl/ApiUrl/ParsedUrl would get its body collapsed. Over-redaction only, no leak. Same comment notes the unconditional per-record rebuild.
utils/Cargo.toml:50	serde_json dev-dependency is unreferenced in the crate.
redact.rs:65,87	redact_url / redact_urls are public with no doc comment, unlike the rest of the module.

Things I checked that are fine

• Other Url-holding structs that get Debug-dumped. Options.l1_provider_url in node/src/options.rs:306 is already #[derivative(Debug = "ignore")]; L1ClientOptions has no URL field other than l1_ws_provider. SingleTransport.client: Http<Client> (which holds the URL) is correctly #[debug(skip)].
• Other exporters. Only the log exporter carries free-text; metrics go out via remote_write and L1ClientMetrics labels are index-based, not URL-based. The Resource forwarded through set_resource is just SERVICE_NAME + instance id, so leaving it unscrubbed is fine.
• The remaining unredacted {err:#} sites in endpoints.rs (lines 730-778, 903-1076, 1123) are storage-, catchup- and config-backed, not L1-backed. The "ten handlers" scope is accurate.
• scrub_urls termination and slicing. pos strictly increases each iteration (no infinite loop), all slice indices land on char boundaries, and the ://, empty-input, and authority-runs-to-EOF edge cases behave as the tests assert.
• cfg gating. espresso-utils is a non-optional dependency of espresso-types and redact is ungated, so the unconditional redact_urls import (used by non-node-gated L1ClientOptions) alongside the node-gated redact_url is correct.
• derive_more::Debug field expressions. redact_url(url) and redact_urls(urls.iter()) both typecheck against the &Url / &Arc<Vec<Url>> bindings derive_more provides.

I was not able to run cargo test/clippy in this environment (cargo invocations require approval here), so the above is static review only — CI coverage for the new tests is worth confirming.

--- • branch ma/sanitize-l1-url

[sveitser]

@claude review again and check what has been fixed

[claude]

Claude encountered an error after 3m 49s —— View job

---

Re-review of ef04605 — "close two redaction gaps found in review"

• Identify the follow-up commit (ef04605, the only change since the last review)
• Verify each of the 7 prior findings against the new code
• Re-audit the changed logic for new gaps
• Post verdict

All seven items are addressed, and two of the fixes go further than what I asked for. No blocking findings remain.

---

Status of prior findings

#	Finding	Status	Verification
1	scrub_url_debug fail-closed violation (brace in query/fragment)	Fixed	redact.rs:118-133 uses the quote toggle. Slicing is right: k is a byte offset relative to body_start, pos = body_start + k lands on the } so the trailing push re-emits it — Url { + " *** " + }. Truncated-input branch preserved. test_scrub_url_debug_brace_in_query_or_fragment covers both ?filter={a}&apikey= and #{a} and asserts exact equality with Url { *** }, not just absence of the key.
2	L1 client error logged the raw credential via %	Fixed, stronger form	SingleTransport.url: Url → redacted_url: String (v0_1/l1.rs:309), built in SingleTransport::new (impls/l1.rs:299). The struct genuinely cannot hold the credential now — the #[debug] attribute is gone because it's unnecessary, not because it was dropped. client: Http<Client> still holds the raw URL and is still #[debug(skip)]. Grep confirms no other reader of the old field (impls/l1.rs:1070,1601 use transport.urls, a different field).
—	(found by the author) switch_to leaked the same way at INFO on every failover	Fixed	impls/l1.rs:425. This one was pre-existing and I missed it — it's the same %url-on-a-raw-Url class, on a path that fires on every provider switch. Good catch.
3	Redacted used scrub_urls, not scrub	Fixed	redact.rs:158,169, both Debug and Display. This is the boundary with no exporter behind it, so it now closes both forms.
4	Pass-through test couldn't catch a dropped field	Fixed	redact_exporter.rs:190-202 populates all nine. set_trace_context(.., Some(TraceFlags::SAMPLED)) confirms the flags round-trip through ctx.trace_flags rather than getting silently dropped, which was the specific thing I wasn't sure about.
5	"Url {" substring-matched BaseUrl { etc.	Fixed	redact.rs:105-113 requires the preceding char to be a non-identifier (is_alphanumeric() || '_'). Start-of-string is handled — text[..0].chars().next_back() is None, so is_some_and is false and it proceeds. Nested MyUrl { inner: Url { .. } } still collapses the inner one (preceded by a space); newtype Foo(Url { .. }) still collapses (preceded by ().
6	ends_url_token comment misattributed `/{/}	Fixed	redact.rs:13-16.
7	Unused serde_json dev-dependency	Fixed	Dropped from Cargo.toml and Cargo.lock.

The quote-toggle argument holds for everything this code will see: every field in url::Url's Debug is a slice of the serialization (scheme, username, password, host, path, query, fragment; port is a u16, cannot_be_a_base a bool), so the only quotes in the body are the delimiters Rust adds. Host::Domain("…") doesn't break it either. Pretty-print ({:#?}, which Redacted forwards) still matches "Url {" and terminates on the dedented closing brace.

Residual notes — none blocking

• No test for Redacted catching the field-wise form. The switch to scrub in Diagram plugin architecture (VM-agnostic framework interacting with a specific zkEVM application layer, including RPC, prover, and rollup contract) #3 is the fix for the exact leak class the PR opens with, but test_redacted_scrubs_debug_and_display (redact.rs:349) still only exercises the :// form. A Redacted(SomeStruct { url }) case would pin the behaviour, otherwise a future revert to scrub_urls there passes the suite.
• redact_url / redact_urls are still undocumented (redact.rs:66,88), unlike everything else in the module. Was in my smaller-items list; carried over.
• scrub_record still rebuilds every record even when nothing changes. Explicitly optional last time and it's on the batched background path — noting only so it isn't lost.
• One theoretical hole left in the toggle, not reachable here. It assumes " never survives in a rendered URL. That's true for every URL with an authority, but WHATWG percent-encodes opaque paths (scheme:opaque, no //) with a narrower set that may not include ". If so, an opaque-path URL with an odd number of quotes ahead of a brace could end the body early. Every URL this codebase parses is http/https/ws/wss/postgres — all authority-bearing — so I'd leave it; flagging only because the module's framing is "no assumption without the every-component guarantee", and this one is scheme-conditional. I couldn't run the parser to confirm the premise.

Caveat

Same as last time: cargo invocations require approval in this environment, so this is static review only — I could not build or run the new tests. The one thing I'd want CI to confirm is TraceId::from(1u128) / SpanId::from(2u64) in the new test (redact_exporter.rs:197-198); I couldn't check whether opentelemetry 0.32 exposes those From impls versus from_bytes/from_hex. Everything else typechecks by inspection, including the #[cfg(feature = "node")] split — redact_url is now imported only in impls/l1.rs under the node gate and used only in node-gated code, while v0_1/l1.rs keeps the ungated redact_urls for L1ClientOptions.

---

• branch ma/sanitize-l1-url