wallet: keep RPC spend credentials out of env, argv and error strings #192

Merged

sirius merged 2 commits from onionwire/t_79e98511-wallet-rpc-keep-spend-credentials-out-of into main

2026-10-23 02:46:28 +00:00

sirius commented

2026-10-23 02:18:49 +00:00

Owner

What changed

Sealed store value wins, env is the fallback. Wallet::for_boot already tries the stored WalletConfig first; the env path now emits a one-time notice
(ONIONWIRE_WALLET_RPC_LOGIN: environment is readable by other processes and lands in shell history; put it in the wallet setup screen instead)
only when the login actually came from the environment. App-spawned children keep taking their creds from the sealed wallet_child record (WalletChild::creds) — unchanged.
No rendering path repeats the login. Endpoint::redacted() / WalletClient::redacted_url() render http://<redacted>:<redacted>@127.0.0.1:<port>/json_rpc;
Endpoint and WalletClient have hand-written Debug impls that use it; error/status copy now redacts the whole user:pass (redact_login), not just the password half.
The dial URL still carries the real userinfo because Wallet::from_url needs it.
Residual documented in code (module doc, src/wallet_child.rs): --rpc-login is visible in /proc/<pid>/cmdline to same-uid processes because
monero-wallet-rpc has no other credential channel. Mitigation: loopback-only bind, strong generated login, never a remote daemon.

Threat-model row I would add (docs/THREAT_MODEL.md is owned by another card this round)

Wallet RPC credential carriage. The app-generated login is sealed in the SQLCipher store and never written to the wallet dir, but a spawned monero-wallet-rpc
necessarily receives --rpc-login user:pass on argv, so a same-uid process can read it from /proc/<pid>/cmdline for the life of the child. Exposure is bounded by a
loopback-only bind and the app never points the child at a remote daemon. A login supplied through ONIONWIRE_WALLET_RPC / ONIONWIRE_WALLET_RPC_LOGIN is weaker
(readable in /proc and shell history) and is only a fallback for headless use; the store wins when both exist.

Tests

tests/wallet.rs::ping_401_error_carries_no_credential — a 401 on the authenticated retry returns
wallet RPC digest rejected (HTTP 401) — the login does not match the child's --rpc-login (no password, no user:pass, no URL userinfo).
wallet::tests::env_login_warns_once_per_process_and_not_for_a_stored_config — one-shot gate; from_env warns, for_boot(Some(stored)) does not.
wallet::tests::redacting_an_error_drops_the_whole_login_not_just_the_password, redacted_url_keeps_the_endpoint_and_drops_the_userinfo, wallet_debug_prints_no_login.
wallet_child::tests::wallet_client_debug_and_redacted_url_hide_the_login.
transfers_match conjunctive pin already exists (tests/wallet.rs::transfers_match_empty_field_is_false) and is unchanged/green.
No src/node.rs or src/store.rs change; --trusted-daemon argv pin untouched (green).

Verification

cargo test --locked --target x86_64-unknown-linux-gnu — all suites green (lib 166, wallet 62, zeroize 8, etc.).
cargo clippy --locked --all-targets --target x86_64-unknown-linux-gnu -- -D warnings — clean.
Files: src/wallet.rs, src/wallet_child.rs, tests/wallet.rs.

## What changed - **Sealed store value wins, env is the fallback.** `Wallet::for_boot` already tries the stored `WalletConfig` first; the env path now emits a **one-time** notice (`ONIONWIRE_WALLET_RPC_LOGIN: environment is readable by other processes and lands in shell history; put it in the wallet setup screen instead`) only when the login actually came from the environment. App-spawned children keep taking their creds from the sealed `wallet_child` record (`WalletChild::creds`) — unchanged. - **No rendering path repeats the login.** `Endpoint::redacted()` / `WalletClient::redacted_url()` render `http://<redacted>:<redacted>@127.0.0.1:<port>/json_rpc`; `Endpoint` and `WalletClient` have hand-written `Debug` impls that use it; error/status copy now redacts the whole `user:pass` (`redact_login`), not just the password half. The dial URL still carries the real userinfo because `Wallet::from_url` needs it. - **Residual documented in code** (module doc, `src/wallet_child.rs`): `--rpc-login` is visible in `/proc/<pid>/cmdline` to same-uid processes because `monero-wallet-rpc` has no other credential channel. Mitigation: loopback-only bind, strong generated login, never a remote daemon. ### Threat-model row I would add (docs/THREAT_MODEL.md is owned by another card this round) > **Wallet RPC credential carriage.** The app-generated login is sealed in the SQLCipher store and never written to the wallet dir, but a spawned `monero-wallet-rpc` > necessarily receives `--rpc-login user:pass` on argv, so a same-uid process can read it from `/proc/<pid>/cmdline` for the life of the child. Exposure is bounded by a > loopback-only bind and the app never points the child at a remote daemon. A login supplied through `ONIONWIRE_WALLET_RPC` / `ONIONWIRE_WALLET_RPC_LOGIN` is weaker > (readable in `/proc` and shell history) and is only a fallback for headless use; the store wins when both exist. ## Tests - `tests/wallet.rs::ping_401_error_carries_no_credential` — a 401 on the authenticated retry returns `wallet RPC digest rejected (HTTP 401) — the login does not match the child's --rpc-login` (no password, no `user:pass`, no URL userinfo). - `wallet::tests::env_login_warns_once_per_process_and_not_for_a_stored_config` — one-shot gate; `from_env` warns, `for_boot(Some(stored))` does not. - `wallet::tests::redacting_an_error_drops_the_whole_login_not_just_the_password`, `redacted_url_keeps_the_endpoint_and_drops_the_userinfo`, `wallet_debug_prints_no_login`. - `wallet_child::tests::wallet_client_debug_and_redacted_url_hide_the_login`. - `transfers_match` conjunctive pin already exists (`tests/wallet.rs::transfers_match_empty_field_is_false`) and is unchanged/green. - No `src/node.rs` or `src/store.rs` change; `--trusted-daemon` argv pin untouched (green). ## Verification - `cargo test --locked --target x86_64-unknown-linux-gnu` — all suites green (lib 166, wallet 62, zeroize 8, etc.). - `cargo clippy --locked --all-targets --target x86_64-unknown-linux-gnu -- -D warnings` — clean. - Files: `src/wallet.rs`, `src/wallet_child.rs`, `tests/wallet.rs`.

sirius added 1 commit

2026-10-23 02:18:49 +00:00

wallet: redact RPC creds everywhere and warn once on a login from env

ci / test (pull_request) Successful in 19m31s

Details

ci / sdk (pull_request) Successful in 2m3s

Details

625879e000

The app-spawned child already takes its login from the sealed store record
and only falls back to ONIONWIRE_WALLET_RPC_LOGIN; make that order explicit
in the code and name the exposure once per process when the fallback is used.

The dial URL still carries user:pass (Wallet::from_url needs it), but no
rendering path repeats it: Endpoint::redacted and WalletClient::redacted_url
print http://<redacted>:<redacted>@host:port/json_rpc, WalletClient has a
hand-written Debug, and error/status copy redacts the whole login, not just
the password half.

Residual, documented at the top of wallet_child.rs: --rpc-login is visible in
/proc/<pid>/cmdline to same-uid processes because monero-wallet-rpc has no
other credential channel.

sirius added 1 commit

2026-10-23 02:46:20 +00:00

Merge branch 'main' into onionwire/t_79e98511-wallet-rpc-keep-spend-credentials-out-of

android-ci / android (pull_request) Successful in 11m10s

Details

ci / test (pull_request) Has been cancelled

Details

ci / sdk (pull_request) Has been cancelled

Details

af9077046b

sirius merged commit d3a28a5c1c into main

2026-10-23 02:46:28 +00:00