This repository was archived by the owner on Oct 19, 2026. It is now read-only.
Repository navigation
fix: keep the RPC URL out of sponsord errors and logs - #39
Merged
Merged
Conversation
reqwest names the URL in its transport errors (` for url (<url>)`) and alloy forwards that text, so a failed chain call printed SPONSORD_RPC_URL, often with a provider API key in its path, in the exit error and in the keeper's and the HTTP API's warnings. - ChainPool renders each chain-call error's cause chain with the URLs stripped (decdn_common::redact::strip_urls), so its errors keep the failure class (connection refused, timeout, DNS, TLS) and never carry the URL. - The keeper and HTTP log sites go through sanitize_err_chain / sanitize_rpc_display as a backstop. - main() prints its error through sanitize_err_chain instead of returning it to anyhow's Termination impl, as decdn-node does (decdn/decdn#958). FakePool's failures now read like reqwest's error and name FAKE_RPC_URL, and test_support gains CapturedLog, so the failure tests also assert that the key never reaches a log line. A binary test checks the exit error. Closes #38 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The keeper’s remaining-read warning still needs regression coverage for RPC URL redaction.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Protects RPC provider credentials by removing URLs from daemon errors and logs while preserving failure causes.
Changes:
- Redacts chain errors, keeper/API logs, and fatal exit output.
- Adds log capture and regression coverage.
- Updates dependencies, maintenance instructions, and changelogs.
| File | Description |
|---|---|
| RELEASING.md | Updates dependency refresh instructions. |
| crates/sponsord/tests/startup_redacts_rpc_url.rs | Tests startup error redaction. |
| crates/sponsord/tests/api.rs | Checks API warning redaction. |
| crates/sponsord/src/main.rs | Sanitizes fatal errors. |
| crates/sponsord/src/http.rs | Sanitizes request error logs. |
| crates/sponsord/CHANGELOG.md | Documents the security fix. |
| crates/sponsord/Cargo.toml | Adds redaction dependency. |
| crates/sponsord-core/src/test_support.rs | Adds log capture and URL-bearing failures. |
| crates/sponsord-core/src/pool.rs | Redacts chain errors at source. |
| crates/sponsord-core/src/keeper.rs | Sanitizes warnings and tests top-up failures. |
| crates/sponsord-core/CHANGELOG.md | Documents core redaction changes. |
| crates/sponsord-core/Cargo.toml | Adds redaction and test-support dependencies. |
| CONTRIBUTING.md | Updates local dependency overrides. |
| Cargo.toml | Declares shared redaction dependency. |
| Cargo.lock | Records dependency additions. |
| AGENTS.md | Updates dependency refresh guidance. |
| .github/workflows/ci.yml | Updates canary dependency refresh. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review follow-ups for #38: - RUST_LOG=debug printed the URL on every chain call: alloy's `ReqwestTransport` debug span records it, and every event inside the span (reqwest's, hyper's) prints the span's fields. main() now builds the RUST_LOG filter itself with `alloy_transport_http=info` added, which only drops that span and alloy's own debug/trace events. - `--help` printed SPONSORD_RPC_URL's value; it is now hidden like the token and password. - `ChainConfig`'s Debug showed the URL; it now shows its length. - Flattening `top_up` errors to text lost decdn's `TopUpUnconfirmed` and `AllowanceShortfall` markers, which callers downcast on (the former means the top-up may have mined). `redacted_top_up` re-attaches them. - The http.rs log sites used `sanitize_rpc_display` on an anyhow::Error, which renders only the outermost message; they use `sanitize_err_chain` now. - The listener is bound before the keeper starts, so a bind failure can no longer drop a top-up between the approve and the top-up. Adds unit tests for `redacted()` and the marker handling, a closed-port test of every ChainPool method, a `--help` test, and a binary test at RUST_LOG=trace. Fixes the doc on main (the Termination impl is std's), the changelog wording, and moves sponsord-core's Security section last. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

What
A failed chain call printed
SPONSORD_RPC_URLin the daemon's exit error, the keeper's warnings and the HTTP API's warn/error lines. That URL often carries a provider API key in its path. The cause: reqwest's Display ends infor url (<url>), and alloy forwards it.This PR follows decdn-node's fix (decdn/decdn#958) and closes every path I found:
ChainPoolrenders each chain-call error'ssource()chain with the URLs stripped (redacted(), built ondecdn_common::redact::strip_urls). It strips each link separately and skips a link that repeats the one before it. Its errors therefore never contain the URL, and they now keep the failure class (connection refused, timeout, DNS, TLS), which alloy's outer Display leaves out.top_upmarkers.top_uperrors still downcast to decdn'sTopUpUnconfirmed(the top-up may have mined) andAllowanceShortfall;redacted_top_upre-attaches them after redaction.keeper.rsandhttp.rsrender errors throughsanitize_err_chainas a backstop.main()prints its error throughsanitize_err_chaininstead of returning it to std'sTerminationimpl.RUST_LOG=debug. alloy'sReqwestTransportdebug span records the URL, and every reqwest and hyper event inside that span would print it.main()now builds theRUST_LOGfilter withalloy_transport_http=infoadded, which hides only that span and alloy's own debug/trace events.--helpandDebug.--helphides the URL's value, as it does for the token and password.ChainConfig'sDebugshows the URL's length instead of the URL.Before:
After:
Closes #38
Reviewer notes
redactedandredacted_top_upincrates/sponsord-core/src/pool.rs, theninit_tracingincrates/sponsord/src/main.rs.redactedstrips each link before joining. Ifstrip_urlsran on the joined string, it would eat the:after a removed URL, because it consumes up to the next whitespace.connecting to <host>:<port>, but nothing logs the path.trace_logging_does_not_print_the_rpc_urlchecks this, includingRUST_LOG=alloy_transport_http=trace,trace.RUST_LOGunset, sponsord still prints only ERROR events, asfmt::initdid. That means the warnings this PR sanitizes are hidden unless operators setRUST_LOG.TopUpUnconfirmed. Before this PR too, the keeper retries every failure on its next tick, includingTopUpUnconfirmed, so an unconfirmed top-up can be sent twice. The marker is preserved, so a follow-up can handle it.decdn-commonis new as a direct dependency, but it was already inCargo.lockat the pinned commit (7f938e6).tracing-subscriber'senv-filterfeature explicitly. Before, it was only enabled because decdn's crates enable it.cargo update -p …lists, the[patch]example in CONTRIBUTING and thedeny.tomlcomment now includedecdn-common.FakePoolfailures read like reqwest's error and nameFAKE_RPC_URL.fail_remaining_readsswitch.CapturedLog, which capturestracingoutput on the test's thread. It sits behindtest-support, which enables the optionaltracing-subscriberdependency.sponsor.tomland the archive names are unchanged.Verification
cargo fmt --all --checkcargo clippy --workspace --all-targets --all-features -- -D warningscargo nextest run --workspace: 131 passedcargo nextest run --features sponsord-core/anvil-e2e,sponsord/anvil-e2e -p sponsord-core -p sponsord --test chain_pool --test binary_anvil: 2 passedcrates/<dir>/CHANGELOG.md(sponsord,sponsord-core, both under Security)~/.decdn/sponsor.tomlor ansponsord-apitype touched: not touchedpr-titlecheck allowsI also ran
cargo test --doc --workspace,cargo docwith-D warnings, thecheck-*.shscripts,cargo-deny-warn.sh,pytest .github/scripts/tests(144 passed) and markdownlint.To check that the tests catch the leaks, I reverted the fixes and reran them. Each of these redaction tests failed with its fix reverted:
pool::tests::an_unreachable_rpc_url_stays_out_of_the_errorapi::failed_chain_read_is_503startup_redacts_rpc_urltrace_logging_does_not_print_the_rpc_url, with the filter cap removed🤖 Generated with Claude Code