From de2f685b7a317f45c6fc92ce626b339af2aeeb58 Mon Sep 17 00:00:00 2001 From: Michael Taylor Date: Tue, 1 Sep 2026 20:52:18 -0700 Subject: [PATCH 1/6] =?UTF-8?q?chore(403):=20open=20lane=20=E2=80=94=20cli?= =?UTF-8?q?ent-side=20pairing=20for=20a=20user-run=20Agent?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Anchor commit for dig-node#403. Bumps the workspace version to 0.243.0 and opens the branch so the lane's state survives a session cap. Refs #403 --- Cargo.lock | 2 +- Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index fea707be..f6b026b4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3031,7 +3031,7 @@ dependencies = [ [[package]] name = "dig-node-service" -version = "0.236.0" +version = "0.243.0" dependencies = [ "async-trait", "axum", diff --git a/Cargo.toml b/Cargo.toml index effdd8e9..1dda5cbb 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -32,7 +32,7 @@ edition = "2021" # the ROOT manifest (`[workspace.package].version`), so it MUST be set here for a # release to fire (§3.6). The library crates (dig-node-core/dig-runtime/dig-wallet) # keep their own independent versions — only the released binary tracks the workspace version. -version = "0.236.0" +version = "0.243.0" # Release hardening, matching digstore: keep integer-overflow checks ON in release. # The node parses untrusted serialized input and does offset/length arithmetic over From de308d22a2d75c679bbf6dbb1be2f868a99ffc0c Mon Sep 17 00:00:00 2001 From: Michael Taylor Date: Tue, 1 Sep 2026 21:07:59 -0700 Subject: [PATCH 2/6] feat(pair): give an unprivileged client the pairing handshake + a token ladder (#403) An ordinary OS user could not drive control.* against a dig-node running as a root system service: control_client::call_control had exactly one token source, the 0600 root:root master token (#501). The server side of the #280 handshake was complete; nothing implemented the client half. - paired_client: the token LADDER as a pure function over the two read outcomes (master when readable -> per-user paired token -> the master read's own remedy, verbatim), plus the per-user 0600 store, the refusal bound on client_name, and the poll bound taken from the server's expires_ms. - pair connect [--client-name NAME]: request over the OPEN method, print the compare-codes value, poll to a terminal state, persist on approval. No file mode is widened anywhere. The paired token cannot administer pairings and carries no chain authority. Co-Authored-By: Claude --- crates/dig-node-service/src/control_client.rs | 17 +- crates/dig-node-service/src/entrypoint.rs | 26 ++ crates/dig-node-service/src/lib.rs | 4 + crates/dig-node-service/src/pair.rs | 91 ++++- crates/dig-node-service/src/paired_client.rs | 338 ++++++++++++++++++ crates/dig-node-service/src/pairing.rs | 2 +- 6 files changed, 475 insertions(+), 3 deletions(-) create mode 100644 crates/dig-node-service/src/paired_client.rs diff --git a/crates/dig-node-service/src/control_client.rs b/crates/dig-node-service/src/control_client.rs index c107a142..55939bcd 100644 --- a/crates/dig-node-service/src/control_client.rs +++ b/crates/dig-node-service/src/control_client.rs @@ -15,6 +15,16 @@ //! mutating CLI control is gated by the same capability as the WS, not an unauthenticated side //! door. A node running as a service under another OS account surfaces the precise //! service-vs-user remedy from `load_token_readonly` (elevate / grant read ACL / start the node). +//! +//! # The ladder (#403) -- a paired token when the master token is out of reach +//! +//! On a `.deb` install the master token is `0600 root:root` (#501), so an ordinary user's read is +//! DENIED. Rather than widen that mode -- it is the master capability -- the client falls through +//! to the scoped token an operator approved for this account (`dign pair connect`, then +//! `sudo dign pair approve `; see [`crate::paired_client`]). That token is strictly +//! less powerful: it cannot administer pairings and carries no chain authority over the wallet +//! replica. With NEITHER token the master read's rich remedy is returned unchanged, because it is +//! the message that tells the user what to do next. use serde_json::{json, Value}; @@ -43,7 +53,12 @@ fn build_control_client() -> reqwest::Result { /// ("is the node running?"), a JSON-RPC `error` → `Other` (the node's own message). pub fn call_control(config: &Config, method: &str, params: Value) -> std::io::Result { let addr = config.bind_addr(); - let token = control::load_token_readonly()?; + // The #403 ladder: the master token when this account can read it, else this user's paired + // token, else the master read's own remedy verbatim. Rung 2 is a thunk, so a user who can read + // the master token never touches the per-user store. + let token = crate::paired_client::select_token(control::load_token_readonly(), || { + crate::paired_client::load_paired_token(&crate::paired_client::paired_token_path()) + })?; let rt = tokio::runtime::Builder::new_current_thread() .enable_all() .build()?; diff --git a/crates/dig-node-service/src/entrypoint.rs b/crates/dig-node-service/src/entrypoint.rs index 804ebf91..02f0a85f 100644 --- a/crates/dig-node-service/src/entrypoint.rs +++ b/crates/dig-node-service/src/entrypoint.rs @@ -759,6 +759,14 @@ enum PairCommand { /// The token id from `dig-node pair`. token_id: String, }, + /// Ask this node for a scoped control token for YOUR account, then wait for the operator + /// to approve it (#403). Needs no elevation and no master token. + Connect { + /// The label the operator sees when approving. Defaults to `dign CLI ()`. + /// Refused, never shortened, above 64 characters. + #[arg(long)] + client_name: Option, + }, } impl Command { @@ -876,6 +884,7 @@ pub fn run() -> std::process::ExitCode { None | Some(PairCommand::List) => PairAction::List, Some(PairCommand::Approve { pairing_id }) => PairAction::Approve { pairing_id }, Some(PairCommand::Revoke { token_id }) => PairAction::Revoke { token_id }, + Some(PairCommand::Connect { client_name }) => PairAction::Connect { client_name }, }; render(pair::run(&config, pair_action), action, json) } @@ -1463,6 +1472,7 @@ mod tests { None | Some(PairCommand::List) => PairAction::List, Some(PairCommand::Approve { pairing_id }) => PairAction::Approve { pairing_id }, Some(PairCommand::Revoke { token_id }) => PairAction::Revoke { token_id }, + Some(PairCommand::Connect { client_name }) => PairAction::Connect { client_name }, }, _ => panic!("expected a pair command from {argv:?}"), }; @@ -1478,6 +1488,22 @@ mod tests { ), "`pair approve ` must approve, and must carry the id through" ); + // #403: `connect` is a DISTINCT verb, and it must never be reachable by accident from the + // bare noun -- the bare noun is the operator's read-only listing. + assert!( + matches!( + pair_action(&["dig-node", "pair", "connect"]), + PairAction::Connect { client_name: None } + ), + "`pair connect` with no flag must default its own label" + ); + assert!( + matches!( + pair_action(&["dig-node", "pair", "connect", "--client-name", "Agent"]), + PairAction::Connect { client_name: Some(ref n) } if n == "Agent" + ), + "`--client-name` must reach the action verbatim -- the operator approves what they see" + ); } /// **`dign mirror bond-states --after` sends the cursor to the node.** diff --git a/crates/dig-node-service/src/lib.rs b/crates/dig-node-service/src/lib.rs index 6ad91814..01f3e0a4 100644 --- a/crates/dig-node-service/src/lib.rs +++ b/crates/dig-node-service/src/lib.rs @@ -87,6 +87,10 @@ pub mod network_info; /// handler argument, then opens the user's default browser at the resolving URL. See [`open`]. pub mod open; pub mod pair; +/// The CLIENT half of the #280 pairing handshake (#403): the token ladder + the per-user +/// paired-token store, so an unprivileged user can drive `control.*` without widening a mode. +/// See [`paired_client`]. +pub mod paired_client; pub mod pairing; /// `control.peers.ping` (dig_ecosystem#1985): the connection-ladder diagnostic — dial one peer a /// tier at a time and report WHICH tier reached it. See [`peer_ping`]. diff --git a/crates/dig-node-service/src/pair.rs b/crates/dig-node-service/src/pair.rs index ba2b0a79..176ee07e 100644 --- a/crates/dig-node-service/src/pair.rs +++ b/crates/dig-node-service/src/pair.rs @@ -12,6 +12,11 @@ //! The operator FIRST confirms the printed `pairing_code` matches what the //! extension shows (compare-codes consent), then approves. //! * `dig-node pair revoke ` — revoke an issued controller token. +//! * `dig-node pair connect [--client-name NAME]` — the CLIENT side (#403), and the only verb +//! here that needs NO master token: it asks for a token, prints the code for the operator to +//! compare, waits for approval, and stores the granted token in the invoking user's own state +//! dir. This is how an ordinary OS user drives `control.*` against a root-owned service +//! without any file mode being widened. //! //! Everything here reaches the node over `POST /` on its loopback address with the //! `X-Dig-Control-Token` header — the same authorized surface the DIG Browser uses. @@ -22,7 +27,10 @@ use crate::untrusted_text::render_untrusted; use crate::cli::Outcome; use crate::config::Config; -use crate::control_client::call_control; +use crate::control_client::{call_control, call_open}; +use crate::paired_client::{ + self, default_client_name, next_poll_step, validate_client_name, PollStep, POLL_INTERVAL, +}; /// The operator action, clap-agnostic (mapped from the CLI subcommand in `main.rs`). pub enum PairAction { @@ -32,6 +40,9 @@ pub enum PairAction { Approve { pairing_id: String }, /// Revoke an issued controller token by id. Revoke { token_id: String }, + /// CLIENT side (#403): ask this node for a scoped token, wait for the operator to approve, + /// and persist the result in THIS user's own state dir. + Connect { client_name: Option }, } /// Run a `pair` subcommand: read the master token, call the node's `control.pairing.*`, @@ -64,6 +75,7 @@ pub fn run(config: &Config, action: PairAction) -> std::io::Result { result, )) } + PairAction::Connect { client_name } => connect(config, client_name), PairAction::Revoke { token_id } => { let result = call_control( config, @@ -81,6 +93,83 @@ pub fn run(config: &Config, action: PairAction) -> std::io::Result { } } +/// Current unix time in milliseconds (0 on a clock error — only affects the poll deadline, and a +/// zero clock reads as "not yet expired", so the server's own sweep remains the real bound). +fn now_ms() -> u64 { + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_millis() as u64) + .unwrap_or(0) +} + +/// `dig-node pair connect` — the CLIENT half of the handshake (#403). +/// +/// Uses [`call_open`], never [`call_control`]: the requester by definition holds no token yet, and +/// routing an OPEN method through the gated client would fail with an elevation remedy for a +/// question the node answers to anyone. Progress goes to STDERR so `--json` stdout stays a single +/// machine-readable object. +fn connect(config: &Config, client_name: Option) -> std::io::Result { + let name = client_name.unwrap_or_else(default_client_name); + validate_client_name(&name)?; + + let requested = call_open(config, "pairing.request", json!({ "client_name": name }))?; + let pairing_id = requested["pairing_id"] + .as_str() + .ok_or_else(|| std::io::Error::other("dig-node: pairing.request returned no pairing_id"))? + .to_string(); + let code = requested["pairing_code"].as_str().unwrap_or("??????"); + let expires_ms = requested["expires_ms"].as_u64().unwrap_or(0); + + eprintln!( + "dig-node: pairing code {code} + Ask the machine's operator to CONFIRM this code, then run: + + sudo dign pair approve {pairing_id} + + Waiting for approval..." + ); + + loop { + let polled = call_open(config, "pairing.poll", json!({ "pairing_id": pairing_id }))?; + match polled["status"].as_str().unwrap_or("unknown") { + "approved" => { + let token = polled["token"].as_str().ok_or_else(|| { + std::io::Error::other("dig-node: approved pairing carried no token") + })?; + let path = paired_client::paired_token_path(); + paired_client::store_paired_token(&path, token)?; + return Ok(Outcome::new( + format!( + "dig-node: paired. The scoped token is stored for your account at {}. + `dign` control commands now work as this user, with no elevation. The operator can revoke it any time with `sudo dign pair revoke `.", + path.display() + ), + json!({ "status": "approved", "token_path": path.display().to_string() }), + )); + } + "pending" => match next_poll_step(now_ms(), expires_ms, POLL_INTERVAL) { + PollStep::Wait(d) => std::thread::sleep(d), + PollStep::Expired => { + return Err(std::io::Error::new( + std::io::ErrorKind::TimedOut, + "dig-node: the pairing request expired before it was approved. Run `dign pair connect` again and have the operator approve it within five minutes.", + )) + } + }, + // `expired` and `unknown` are both terminal: the node has dropped the pending entry, + // so no amount of further polling can change the answer. + other => { + return Err(std::io::Error::new( + std::io::ErrorKind::TimedOut, + format!( + "dig-node: the pairing request is {other} — it was never approved, or the node restarted. Run `dign pair connect` again." + ), + )) + } + } + } +} + /// The display budget, in terminal columns, for an attacker-supplied `client_name`. /// /// It matches `pairing::MAX_CLIENT_NAME`, so a name this node ACCEPTED renders unmarked and the diff --git a/crates/dig-node-service/src/paired_client.rs b/crates/dig-node-service/src/paired_client.rs new file mode 100644 index 00000000..aef8abe6 --- /dev/null +++ b/crates/dig-node-service/src/paired_client.rs @@ -0,0 +1,338 @@ +//! The CLIENT half of the #280 control-token pairing handshake (dig-node#403). +//! +//! `pairing.rs` implements the node's side of the three steps; `pair.rs` implements the +//! OPERATOR's side (list / approve / revoke). Neither gives an ordinary, unprivileged OS user a +//! way to ASK for a token and KEEP it — so on a `.deb` install, where the master control token is +//! `0600 root:root` (#501), every `control.*` CLI verb was reachable only under `sudo`. +//! +//! This module closes that gap WITHOUT widening a single file mode: +//! +//! * [`select_token`] is the token LADDER — master token when readable, else the per-user paired +//! token, else the master read's own rich remedy error, unchanged. +//! * [`paired_token_path`] / [`store_paired_token`] / [`load_paired_token`] own the per-user +//! store, which lives in the invoking user's own state dir and is `0600` on Unix. +//! * [`validate_client_name`] mirrors the server's REFUSAL bound, and [`next_poll_step`] bounds +//! the approval wait against the server's own `expires_ms`. +//! +//! # Why the ladder is a pure function and not a `cfg!` branch +//! +//! A `cfg!(unix)` branch is only ever exercised on the half of the fleet that compiles it, so the +//! behaviour that matters most on Ubuntu would be untested on the machine most likely to run these +//! tests. More sharply: the unprivileged case CANNOT be reproduced in a test that runs as root, +//! and a unit test cannot drop privileges. Taking both read OUTCOMES as arguments makes the +//! decision assertable on every platform, under any account — the pattern #458 established for +//! [`crate::control`]'s remedy text. +//! +//! # What this does NOT change +//! +//! The paired token is strictly LESS powerful than the master token: `pairing.rs` records that +//! master authorizes pairing administration (`control.pairing.approve` / `.revoke`) and +//! `chiaPeers.add`/`.remove`. A user holding a paired token gains the scoped control surface an +//! operator explicitly approved for them, and nothing else. Nothing here reads, writes, chmods or +//! relocates the master token. + +use std::io; +use std::path::{Path, PathBuf}; +use std::time::Duration; + +/// The per-user file holding the scoped token this account was granted. +/// +/// It lives beside the invoking user's own node state ([`crate::state::legacy_state_dir`] — +/// `$HOME/DigNode` / `%LOCALAPPDATA%\DigNode`), NOT in the machine-wide state dir: the whole point +/// is that an ordinary user can own it without anyone touching `/var/lib/dig-node`. +pub const PAIRED_CLIENT_TOKEN_FILE: &str = "client-token"; + +/// The default interval between `pairing.poll` calls while waiting for the operator to approve. +/// +/// Short enough that approval feels immediate, long enough that a 5-minute wait is ~100 requests +/// rather than a spin. The wait is bounded by the SERVER's `expires_ms`, never by a local count. +pub const POLL_INTERVAL: Duration = Duration::from_secs(3); + +/// Where THIS user's paired token lives. +pub fn paired_token_path() -> PathBuf { + paired_token_path_in(&crate::state::legacy_state_dir()) +} + +/// [`paired_token_path`] for an explicit directory, so tests use a temp dir and never touch a +/// real one. +pub fn paired_token_path_in(dir: &Path) -> PathBuf { + dir.join(PAIRED_CLIENT_TOKEN_FILE) +} + +/// Read this user's paired token, or `None` when there is not one. +/// +/// Every failure is `None`: an absent, blank, or unreadable store means "this account has no +/// paired token", which is exactly the ladder's second rung failing. It is NEVER an error in its +/// own right, because the master read's remedy is the message the user needs. +pub fn load_paired_token(path: &Path) -> Option { + let s = std::fs::read_to_string(path).ok()?; + let t = s.trim(); + (!t.is_empty()).then(|| t.to_string()) +} + +/// Persist a freshly-approved token for this user, owner-only. +/// +/// Creates the per-user state dir when absent and applies [`crate::state::restrict_file`] — +/// `0600` on Unix. The file is created by the INVOKING user, so it is owned by them; this +/// function never runs elevated and never touches a machine-wide path. +pub fn store_paired_token(path: &Path, token: &str) -> io::Result<()> { + if let Some(dir) = path.parent() { + std::fs::create_dir_all(dir)?; + } + std::fs::write(path, token)?; + crate::state::restrict_file(path); + Ok(()) +} + +/// The token LADDER, as a pure function over the two read OUTCOMES. +/// +/// 1. the master control token when this account can read it (the pre-#403 behaviour, unchanged); +/// 2. else this user's paired token; +/// 3. else the master read's own error, VERBATIM — its remedy text is platform-correct and names +/// the pairing flow (#458), so degrading it would replace the one message that tells the user +/// what to do next. +/// +/// `paired` is a THUNK rather than a value so that rung 1 provably never consults the store: with +/// an `Option` argument the caller has already read the file before the decision is made, and no +/// test could tell a correct implementation from one that reads it every time. +pub fn select_token(master: io::Result, paired: F) -> io::Result +where + F: FnOnce() -> Option, +{ + match master { + Ok(token) => Ok(token), + Err(master_err) => match paired() { + Some(token) => Ok(token), + None => Err(master_err), + }, + } +} + +/// Validate a `client_name` against the server's bound BEFORE spending a round trip. +/// +/// The bound is `pairing::MAX_CLIENT_NAME` itself, so the client cannot drift from the server, and +/// an over-long name is REFUSED rather than shortened — a name this program shortened is a name +/// this program partly wrote, which is precisely the forgery `pairing.rs` refuses to perform. +pub fn validate_client_name(name: &str) -> io::Result<&str> { + let max = crate::pairing::MAX_CLIENT_NAME; + if name.chars().count() > max { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + format!( + "--client-name must be at most {max} characters; this name is refused rather \ + than shortened, because a name shortened here is a name you did not write and \ + the operator approves what they see" + ), + )); + } + Ok(name) +} + +/// A sensible default label for this client, always within the bound. +/// +/// Names the program and the account, because that is what the operator needs in order to decide +/// whether to approve. If the account name is long enough to blow the budget we fall back to the +/// bare program name rather than clipping — the same refusal-not-truncation rule, applied to a +/// value we chose ourselves. +pub fn default_client_name() -> String { + let user = std::env::var("USER") + .or_else(|_| std::env::var("USERNAME")) + .unwrap_or_default(); + let candidate = if user.trim().is_empty() { + "dign CLI".to_string() + } else { + format!("dign CLI ({})", user.trim()) + }; + if candidate.chars().count() > crate::pairing::MAX_CLIENT_NAME { + "dign CLI".to_string() + } else { + candidate + } +} + +/// What the poll loop should do next, given the clock and the server's own deadline. +#[derive(Debug, PartialEq, Eq)] +pub enum PollStep { + /// Sleep this long, then poll again. + Wait(Duration), + /// The server's `expires_ms` has passed — stop, and tell the user to start over. + Expired, +} + +/// Bound the approval wait by the SERVER's deadline, not by a local retry count. +/// +/// `expires_ms` comes from `pairing.request` and is the same value the node's own TTL sweep uses, +/// so the client stops asking at exactly the moment the node stops answering. The final wait is +/// clamped to the remaining time so the loop can never sleep past the deadline and then report a +/// stale state. +pub fn next_poll_step(now_ms: u64, expires_ms: u64, interval: Duration) -> PollStep { + if now_ms >= expires_ms { + return PollStep::Expired; + } + let remaining = Duration::from_millis(expires_ms - now_ms); + PollStep::Wait(interval.min(remaining)) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::cell::Cell; + + fn denied() -> io::Error { + io::Error::new( + io::ErrorKind::PermissionDenied, + "the node's control token exists but is NOT readable by your account — \ + `sudo dign pair approve `", + ) + } + + /// **Proves (dig-node#403):** an unprivileged account whose master-token read was DENIED + /// reaches the control plane with its paired token. + /// + /// This is the decision the whole ticket is about, and it cannot be exercised any other way: + /// the test process here runs with whatever privilege CI grants it, a root run cannot observe + /// the denial, and a unit test cannot drop privileges. Passing the master read's OUTCOME in is + /// what makes the unprivileged case reachable from a privileged process. + #[test] + fn a_denied_master_read_falls_back_to_the_paired_token() { + let chosen = select_token(Err(denied()), || Some("paired-token".into())) + .expect("the paired token is the second rung"); + assert_eq!(chosen, "paired-token"); + } + + /// **Proves:** with no paired token, the master read's rich remedy survives INTACT. + /// + /// The remedy text landed in #458 and is the only thing that tells the user how to get + /// unstuck. Both the KIND and the message are asserted, because the CLI maps the kind to its + /// exit code — a ladder that replaced the error with a generic one would still "fail", and a + /// test asserting only `is_err()` would not see the regression. + #[test] + fn with_no_paired_token_the_master_remedy_is_returned_unchanged() { + let original = denied(); + let kind = original.kind(); + let text = original.to_string(); + + let err = select_token(Err(original), || None).expect_err("no rung can succeed"); + + assert_eq!(err.kind(), kind, "the exit-code-bearing kind must survive"); + assert_eq!(err.to_string(), text, "the remedy must not be degraded"); + } + + /// **Proves:** a readable master token wins AND the paired store is never even consulted. + /// + /// The second half is the load-bearing half. A ladder that reads the paired file on every + /// call would satisfy "master wins" identically, so the observable that distinguishes them is + /// whether the thunk RAN — which is why the parameter is a thunk and not an `Option`. + #[test] + fn a_readable_master_token_wins_without_consulting_the_paired_store() { + let consulted = Cell::new(false); + let chosen = select_token(Ok("master-token".into()), || { + consulted.set(true); + Some("paired-token".into()) + }) + .expect("the master token is the first rung"); + + assert_eq!(chosen, "master-token"); + assert!( + !consulted.get(), + "rung 1 must not read the per-user store at all" + ); + } + + /// **Proves:** the per-user store round-trips, and is created owner-only on Unix. + /// + /// The mode assertion is one-sided on purpose: `0600` exactly, not "no world bit", because a + /// group-readable store would be a quieter version of the very widening this ticket refuses to + /// perform. + #[test] + fn the_per_user_store_round_trips_and_is_owner_only() { + let dir = tempfile::tempdir().unwrap(); + let path = paired_token_path_in(dir.path()); + + assert_eq!(load_paired_token(&path), None, "absent store reads as None"); + + store_paired_token(&path, "scoped-token").unwrap(); + assert_eq!(load_paired_token(&path).as_deref(), Some("scoped-token")); + + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let mode = std::fs::metadata(&path).unwrap().permissions().mode() & 0o777; + assert_eq!(mode, 0o600, "the paired store must be owner-only"); + } + } + + /// **Proves:** a blank store is not a token — it reads as "this account has no paired token" + /// so the ladder falls through to the master remedy instead of presenting an empty header. + #[test] + fn a_blank_store_is_not_a_token() { + let dir = tempfile::tempdir().unwrap(); + let path = paired_token_path_in(dir.path()); + std::fs::write(&path, " \n").unwrap(); + assert_eq!(load_paired_token(&path), None); + } + + /// **Proves:** an over-long `--client-name` is REFUSED, not truncated. + /// + /// Pinned from BOTH sides: exactly at the bound must pass, one character over must fail. A + /// bound tested only from below can only confirm itself, and the failing side is the one that + /// matters — a client that clipped locally would send a short, trusted-looking name for the + /// operator to approve. + #[test] + fn an_over_long_client_name_is_refused_not_truncated() { + let at_bound = "n".repeat(crate::pairing::MAX_CLIENT_NAME); + assert_eq!( + validate_client_name(&at_bound).unwrap(), + at_bound, + "a name AT the bound is accepted verbatim" + ); + + let too_long = "n".repeat(crate::pairing::MAX_CLIENT_NAME + 1); + let err = validate_client_name(&too_long).expect_err("one over the bound is refused"); + assert_eq!(err.kind(), io::ErrorKind::InvalidInput); + assert!( + !err.to_string().contains(&"n".repeat(8)), + "the refusal must not echo a shortened form of the name back as if it were usable" + ); + } + + /// **Proves:** the default label never trips the bound the client just promised to respect. + #[test] + fn the_default_client_name_is_within_the_bound() { + let name = default_client_name(); + assert!(validate_client_name(&name).is_ok(), "default was {name:?}"); + assert!(!name.is_empty()); + } + + /// **Proves:** polling TERMINATES on the server's deadline rather than spinning. + /// + /// Time is pinned explicitly rather than read from the wall clock — a fixture that passed a + /// small literal through a real-clock API would be ~1.8 billion seconds expired and would + /// exercise only the expiry arm while claiming to test both. + #[test] + fn polling_waits_until_the_servers_deadline_and_then_stops() { + const NOW: u64 = 1_700_000_000_000; + let interval = Duration::from_secs(3); + + assert_eq!( + next_poll_step(NOW, NOW + 60_000, interval), + PollStep::Wait(interval), + "well inside the window, poll at the ordinary interval" + ); + assert_eq!( + next_poll_step(NOW, NOW + 1_000, interval), + PollStep::Wait(Duration::from_millis(1_000)), + "the last wait is clamped so the loop cannot sleep PAST the deadline" + ); + assert_eq!( + next_poll_step(NOW, NOW, interval), + PollStep::Expired, + "at the deadline the node has already stopped answering" + ); + assert_eq!( + next_poll_step(NOW + 1, NOW, interval), + PollStep::Expired, + "past the deadline, stop" + ); + } +} diff --git a/crates/dig-node-service/src/pairing.rs b/crates/dig-node-service/src/pairing.rs index 9388ea85..b77fb8d9 100644 --- a/crates/dig-node-service/src/pairing.rs +++ b/crates/dig-node-service/src/pairing.rs @@ -86,7 +86,7 @@ const MAX_PENDING: usize = 32; /// /// The stored value stays BYTE-VERBATIM; neutralisation happens at render time /// ([`crate::untrusted_text::render_untrusted`]), because only the display is a lie surface. -const MAX_CLIENT_NAME: usize = 64; +pub const MAX_CLIENT_NAME: usize = 64; /// Current unix time in milliseconds (0 on a clock error — only affects TTL math). fn now_ms() -> u64 { From 8453bdad7e9f7ad533c1290689869bf496f299aa Mon Sep 17 00:00:00 2001 From: Michael Taylor Date: Tue, 1 Sep 2026 21:14:12 -0700 Subject: [PATCH 3/6] docs(spec): specify the client pairing verb + the CLI token ladder (#403) Also drops the ticket number from the `pair connect` help text -- the no_help_text_exposes_an_internal_ticket_number guard caught it. Co-Authored-By: Claude --- SPEC.md | 49 ++++++++++++++++++++++- crates/dig-node-service/src/entrypoint.rs | 2 +- 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/SPEC.md b/SPEC.md index 0d1719f5..bd2c0a92 100644 --- a/SPEC.md +++ b/SPEC.md @@ -1538,7 +1538,9 @@ mutation or custody method is ever open. ### 7.3a. The daemon state dir — location, ACL, threat model (#501) The state dir holds ONLY the control/auth state — the control token (§7.3) and the paired-token -store (`paired-tokens.json`, §7.11). The bulk per-user `.dig` cache and `config.json` (§3.5–3.6) do +store (`paired-tokens.json`, §7.11). The CLIENT-side paired token (§7.11a) is NOT in this dir: it +lives at `/client-token`, owned by the invoking user, so an unprivileged client can +hold a token without any machine-wide path being widened. The bulk per-user `.dig` cache and `config.json` (§3.5–3.6) do NOT move; they stay per-user (shared with the browser/digstore, #96). **Resolution order** (the daemon and every operator CLI MUST resolve this identically, so it MUST @@ -2275,6 +2277,51 @@ and REVOCABLE. All token comparisons are constant-time. client_name, created_ms }] }`, restricted (dir ACL), atomic writes. The auth gate accepts the master token OR any token in this store (except for the pairing-administration methods). +### 7.11a. Client-side pairing and the CLI token ladder (#403) + +An MV3 extension is not the only client that cannot read `/control-token`. On a `.deb` +install that file is `0600 root:root` and its directory `0700 root:root` (§7.3a), so an ORDINARY OS +USER driving the CLI is in exactly the extension's position: the node is running, the user is on the +machine, and every token-gated `control.*` verb is out of reach. The remedy MUST NOT be to widen the +mode — the master token is the master capability, and it authorizes pairing administration and +chain authority over the wallet replica (§7.11, §18.16). + +**`dig-node pair connect [--client-name NAME]`** is the CLIENT half of the §7.11 handshake, and the +one `pair` verb that requires NO token. It MUST: + +1. call the OPEN `pairing.request { client_name }` (never the gated client — a requester holds no + token by definition, and routing an open method through the gated path fails with an elevation + remedy for a question the node answers to anyone); +2. DISPLAY the returned `pairing_code` and name the operator's command + (`sudo dign pair approve `), so the compare-codes consent step of §7.11 is performed; +3. poll the OPEN `pairing.poll { pairing_id }` to a TERMINAL state, bounded by the server's own + `expires_ms` rather than by a local retry count, and terminate on expiry rather than spin; +4. on `status: "approved"`, persist the delivered token to the INVOKING USER's own state dir + (`/client-token`, §7.3a), owner-only (`0600` on Unix). + +`client_name` is bounded by the same 64-character REFUSAL bound the node applies (§7.11): an +over-long name MUST be refused, never shortened. A name this client shortened is a name the client +partly wrote, and the operator approves what they are shown. + +**The token ladder.** Every CLI `control.*` call selects its token in this fixed order: + +1. the MASTER control token, when this account can read it; +2. otherwise this user's paired token from `/client-token`, when present; +3. otherwise the master read's own error, VERBATIM — its kind (which sets the CLI exit code) and its + remedy text (§7.3) MUST both survive unchanged, because that sentence is what tells the user how + to become able to act. + +Rung 1 MUST NOT consult the per-user store: a user who can read the master token never reads a +paired one. The selection MUST be expressed as a function of the two read OUTCOMES rather than as a +compile-time platform branch, because the unprivileged case cannot be reproduced by a test process +that is privileged, and a `cfg!` branch is exercised only on the platform that compiles it. + +**What pairing a CLI client does NOT grant.** The paired token remains SCOPED and REVOCABLE +(§7.11): it cannot drive `control.pairing.*` and it cannot drive `control.chiaPeers.add`/`.remove`. +Pairing a user therefore never confers pairing administration or chain authority, and no file mode +changes anywhere in the flow. Revocation is unchanged: `sudo dign pair revoke ` invalidates +it on the very next request. + ### 7.12. Paired-token authorization for wallet methods (#370) The pairing framework (§7.11) authorizes `control.*` mutations. The thin-client model (epic #365) diff --git a/crates/dig-node-service/src/entrypoint.rs b/crates/dig-node-service/src/entrypoint.rs index 02f0a85f..8e9410f9 100644 --- a/crates/dig-node-service/src/entrypoint.rs +++ b/crates/dig-node-service/src/entrypoint.rs @@ -760,7 +760,7 @@ enum PairCommand { token_id: String, }, /// Ask this node for a scoped control token for YOUR account, then wait for the operator - /// to approve it (#403). Needs no elevation and no master token. + /// to approve it. Needs no elevation and no master token. Connect { /// The label the operator sees when approving. Defaults to `dign CLI ()`. /// Refused, never shortened, above 64 characters. From e9783db9f9eea86d1d9a4f93d221736b2d62ec43 Mon Sep 17 00:00:00 2001 From: Michael Taylor Date: Tue, 1 Sep 2026 22:15:12 -0700 Subject: [PATCH 4/6] fix(pair): resolve the client token store from the per-user base and create it exclusively Four gate findings from dig-node#498. B1: `paired_token_path` resolved through `state::legacy_state_dir`, whose chain runs `resolve_cache_dir` -> `private_fallback_dir` = `temp_dir()/DigNode-/cache` when the canonical dir is unwritable. On such a host the bearer token was written into a 1777 directory under a /proc-enumerable name. It now resolves from `dig_node_core::platform_user_base()` directly, so the temp fallback is structurally unreachable, and REFUSES when no per-user base exists rather than degrading to the cwd. B2: the store was `fs::write` + a later chmod, so it existed at the process umask (0644) for a window, and `write` follows symlinks -- a planted link disclosed the token or clobbered an arbitrary file as the victim, with no race. It is now created with `create_new` at mode 0600. B3: three user-facing strings in `pair connect` carried a ~26-space run from a lost line continuation, including the verb's SUCCESS message and both terminal failure paths. A fourth (the waiting banner) leaked its source indentation. All four are lifted into named functions so a guard test can assert the signature is gone. B4: the rung-3 remedy named only sudo verbs the unprivileged reader who sees it cannot run. It now names `dign pair connect` first, keeping the operator half and the platform-correctness that landed in #458. Co-Authored-By: Claude --- SPEC.md | 26 +- crates/dig-node-service/src/control.rs | 17 +- crates/dig-node-service/src/control_client.rs | 6 +- crates/dig-node-service/src/pair.rs | 117 +++++++-- crates/dig-node-service/src/paired_client.rs | 226 +++++++++++++++++- 5 files changed, 354 insertions(+), 38 deletions(-) diff --git a/SPEC.md b/SPEC.md index 8274f89e..5161b4a6 100644 --- a/SPEC.md +++ b/SPEC.md @@ -1539,8 +1539,13 @@ mutation or custody method is ever open. The state dir holds ONLY the control/auth state — the control token (§7.3) and the paired-token store (`paired-tokens.json`, §7.11). The CLIENT-side paired token (§7.11a) is NOT in this dir: it -lives at `/client-token`, owned by the invoking user, so an unprivileged client can -hold a token without any machine-wide path being widened. The bulk per-user `.dig` cache and `config.json` (§3.5–3.6) do +lives at `/DigNode/client-token`, owned by the invoking user, so an unprivileged +client can hold a token without any machine-wide path being widened. `` is the +platform per-user base — `$HOME` on Unix/macOS, `%LOCALAPPDATA%` on Windows. It MUST be resolved +directly from that base and MUST NOT be resolved through the cache resolver, whose unwritable-dir +fallback is a PID-keyed directory under the system temp dir: a world-writable temp directory is +never an acceptable location for a bearer credential. When no per-user base can be resolved, the +client MUST REFUSE to store the token rather than degrade to the current working directory. The bulk per-user `.dig` cache and `config.json` (§3.5–3.6) do NOT move; they stay per-user (shared with the browser/digstore, #96). **Resolution order** (the daemon and every operator CLI MUST resolve this identically, so it MUST @@ -2286,6 +2291,12 @@ machine, and every token-gated `control.*` verb is out of reach. The remedy MUST mode — the master token is the master capability, and it authorizes pairing administration and chain authority over the wallet replica (§7.11, §18.16). +The unreadable-master-token remedy (§7.3) is read ONLY by a caller holding neither token, i.e. by +definition an unprivileged one. On Unix it MUST therefore name `dign pair connect` — the verb THAT +reader can run, unelevated — in addition to the operator's `sudo dign pair approve `. +Naming only the elevated half directs the one audience that sees the message to a command it cannot +execute. + **`dig-node pair connect [--client-name NAME]`** is the CLIENT half of the §7.11 handshake, and the one `pair` verb that requires NO token. It MUST: @@ -2296,8 +2307,13 @@ one `pair` verb that requires NO token. It MUST: (`sudo dign pair approve `), so the compare-codes consent step of §7.11 is performed; 3. poll the OPEN `pairing.poll { pairing_id }` to a TERMINAL state, bounded by the server's own `expires_ms` rather than by a local retry count, and terminate on expiry rather than spin; -4. on `status: "approved"`, persist the delivered token to the INVOKING USER's own state dir - (`/client-token`, §7.3a), owner-only (`0600` on Unix). +4. on `status: "approved"`, persist the delivered token to the INVOKING USER's own per-user base + (`/DigNode/client-token`, §7.3a). The file MUST be created EXCLUSIVELY and + owner-only in a single step (`O_CREAT|O_EXCL` at mode `0600` on Unix), never written first and + restricted afterwards: a write-then-chmod leaves the token readable under the process umask for + a window, and a plain write FOLLOWS a symlink planted at that path — which discloses the token, + or clobbers an arbitrary file, as the invoking user. Replacing this user's OWN existing store on + a re-pair is permitted and MUST unlink it rather than write through it. `client_name` is bounded by the same 64-character REFUSAL bound the node applies (§7.11): an over-long name MUST be refused, never shortened. A name this client shortened is a name the client @@ -2306,7 +2322,7 @@ partly wrote, and the operator approves what they are shown. **The token ladder.** Every CLI `control.*` call selects its token in this fixed order: 1. the MASTER control token, when this account can read it; -2. otherwise this user's paired token from `/client-token`, when present; +2. otherwise this user's paired token from `/DigNode/client-token`, when present (an unresolvable per-user base counts as "no paired token", not an error); 3. otherwise the master read's own error, VERBATIM — its kind (which sets the CLI exit code) and its remedy text (§7.3) MUST both survive unchanged, because that sentence is what tells the user how to become able to act. diff --git a/crates/dig-node-service/src/control.rs b/crates/dig-node-service/src/control.rs index 1de3b30a..0df274a8 100644 --- a/crates/dig-node-service/src/control.rs +++ b/crates/dig-node-service/src/control.rs @@ -552,7 +552,7 @@ fn remedy_for_unreadable_token(path: &Path, dir: &str, unix: bool) -> String { ); if unix { format!( - "{elevated}. For a program that must keep running as an ordinary user (the dig-app Agent on a server), do NOT widen the mode on this file — it is the master capability. Pair a scoped, revocable token for that client instead: `sudo dign pair` LISTS the pending requests and `sudo dign pair approve ` approves one -- the bare verb only lists, it approves nothing -- and the token the client receives cannot mint or revoke pairings and cannot grant chain authority. Revoke it any time with `sudo dign pair revoke `." + "{elevated}. If you cannot elevate, you do not have to: run `dign pair connect` as THIS account to request a scoped token of your own, then ask the operator to approve it. Do NOT widen the mode on this file — it is the master capability. On the operator's side: `sudo dign pair` LISTS the pending requests and `sudo dign pair approve ` approves one -- the bare verb only lists, it approves nothing -- and the token the client receives cannot mint or revoke pairings and cannot grant chain authority. Revoke it any time with `sudo dign pair revoke `." ) } else { format!( @@ -6409,6 +6409,21 @@ mod tests { unix.contains("revoke"), "a grant with no stated revocation is a permanent one: {unix}" ); + // dig-node#403 -- this message is rung 3 of the ladder, so the ONLY reader who ever sees + // it is one holding neither token: an unprivileged user who by construction cannot run a + // `sudo` verb. Naming only the operator's half sends that reader to find an + // administrator for a step they can perform themselves. The verb is asserted UNPREFIXED + // (`contains("sudo dign pair connect")` would be the same dead end wearing the right + // words), and the operator half is asserted above and below rather than replaced -- a fix + // that swapped one audience's guidance for the other's is not a fix. + assert!( + unix.contains("run `dign pair connect`"), + "the unprivileged reader must be told the verb THEY can run: {unix}" + ); + assert!( + !unix.contains("sudo dign pair connect"), + "`pair connect` needs no elevation; prefixing it recreates the dead end: {unix}" + ); assert!( !unix.contains("uninstall"), "reinstalling does not grant read access on Unix; advising it is the dead end: {unix}" diff --git a/crates/dig-node-service/src/control_client.rs b/crates/dig-node-service/src/control_client.rs index 55939bcd..148c33dc 100644 --- a/crates/dig-node-service/src/control_client.rs +++ b/crates/dig-node-service/src/control_client.rs @@ -57,7 +57,11 @@ pub fn call_control(config: &Config, method: &str, params: Value) -> std::io::Re // token, else the master read's own remedy verbatim. Rung 2 is a thunk, so a user who can read // the master token never touches the per-user store. let token = crate::paired_client::select_token(control::load_token_readonly(), || { - crate::paired_client::load_paired_token(&crate::paired_client::paired_token_path()) + // An unresolvable per-user base is "this account has no paired token", not an error of + // its own: rung 3's master remedy is the message the user needs either way. + crate::paired_client::paired_token_path() + .ok() + .and_then(|p| crate::paired_client::load_paired_token(&p)) })?; let rt = tokio::runtime::Builder::new_current_thread() .enable_all() diff --git a/crates/dig-node-service/src/pair.rs b/crates/dig-node-service/src/pair.rs index 176ee07e..deb1ff31 100644 --- a/crates/dig-node-service/src/pair.rs +++ b/crates/dig-node-service/src/pair.rs @@ -120,14 +120,7 @@ fn connect(config: &Config, client_name: Option) -> std::io::Result) -> std::io::Result`.", - path.display() - ), + paired_message(&path), json!({ "status": "approved", "token_path": path.display().to_string() }), )); } @@ -152,7 +141,7 @@ fn connect(config: &Config, client_name: Option) -> std::io::Result { return Err(std::io::Error::new( std::io::ErrorKind::TimedOut, - "dig-node: the pairing request expired before it was approved. Run `dign pair connect` again and have the operator approve it within five minutes.", + EXPIRED_BEFORE_APPROVAL, )) } }, @@ -161,15 +150,57 @@ fn connect(config: &Config, client_name: Option) -> std::io::Result { return Err(std::io::Error::new( std::io::ErrorKind::TimedOut, - format!( - "dig-node: the pairing request is {other} — it was never approved, or the node restarted. Run `dign pair connect` again." - ), + terminal_status_message(other), )) } } } } +// --------------------------------------------------------------------------- +// The four user-facing strings of `pair connect`, lifted out of the poll loop. +// +// They are named functions rather than inline literals for one reason: the loop that emitted them +// cannot be driven from a unit test (it dials a node and sleeps), so every one of these strings +// shipped with NOTHING asserting on it. Three of them were corrupted — a lost `\` line +// continuation left a ~26-space run mid-sentence, which compiles, passes clippy, and is visible +// only to a reader. Lifting them out is what makes +// [`tests::the_user_facing_pair_strings_have_no_lost_line_continuation`] able to see them at all. +// --------------------------------------------------------------------------- + +/// What the client prints while it waits for the operator to approve. +fn waiting_banner(code: &str, pairing_id: &str) -> String { + format!( + "dig-node: pairing code {code}\n\ + Ask the machine's operator to CONFIRM this code, then run:\n\n \ + sudo dign pair approve {pairing_id}\n\n\ + Waiting for approval..." + ) +} + +/// The SUCCESS message of `dig-node pair connect`. +fn paired_message(path: &std::path::Path) -> String { + format!( + "dig-node: paired. The scoped token is stored for your account at {}. \ + `dign` control commands now work as this user, with no elevation. The operator can \ + revoke it any time with `sudo dign pair revoke `.", + path.display() + ) +} + +/// The first terminal failure: the request aged out while still pending. +const EXPIRED_BEFORE_APPROVAL: &str = + "dig-node: the pairing request expired before it was approved. Run `dign pair connect` \ + again and have the operator approve it within five minutes."; + +/// The second terminal failure: the node no longer holds the pending entry at all. +fn terminal_status_message(status: &str) -> String { + format!( + "dig-node: the pairing request is {status} — it was never approved, or the node \ + restarted. Run `dign pair connect` again." + ) +} + /// The display budget, in terminal columns, for an attacker-supplied `client_name`. /// /// It matches `pairing::MAX_CLIENT_NAME`, so a name this node ACCEPTED renders unmarked and the @@ -228,6 +259,56 @@ fn format_list(result: &Value) -> String { mod tests { use super::*; + /// **Proves (dig-node#403):** none of `pair connect`'s user-facing strings carries the + /// signature of a lost `\` line continuation. + /// + /// A multi-line Rust string literal without the trailing backslash keeps the source's own + /// indentation, so the sentence ships with a ~26-space run in the middle of it. Three of these + /// four strings shipped that way: the SUCCESS message of the verb this ticket added, and the + /// only guidance on BOTH terminal failure paths. It compiles, clippy is clean, and no test + /// asserted on them — the defect was reachable only by a human reading the source. + /// + /// The property asserted is the SIGNATURE, not the specific wording: any run of three or more + /// consecutive spaces that is not the deliberate indent of the banner's command line. Wording + /// changes freely; a lost continuation is caught mechanically the next time it happens. + #[test] + fn the_user_facing_pair_strings_have_no_lost_line_continuation() { + let banner = waiting_banner("123456", "aabbccdd"); + let paired = paired_message(std::path::Path::new("/home/u/DigNode/client-token")); + let terminal = terminal_status_message("expired"); + + for (what, text) in [ + ("waiting banner", banner.as_str()), + ("paired message", paired.as_str()), + ("expired-before-approval", EXPIRED_BEFORE_APPROVAL), + ("terminal status", terminal.as_str()), + ] { + for line in text.lines() { + // The banner deliberately indents its copy-pasteable command by four spaces, so + // LEADING whitespace is legitimate. A run in the middle of a sentence is not, and + // that is exactly what a lost continuation produces. + let interior = line.trim_start(); + assert!( + !interior.contains(" "), + "{what}: a run of 3+ spaces mid-line is the signature of a lost `\\` line \ + continuation in a multi-line literal: {line:?}" + ); + } + } + } + + /// **Proves:** the message an unprivileged reader gets on both terminal paths tells them the + /// verb THEY can run, not only the one the operator runs. + /// + /// Asserted here as well as in `control.rs` because these two strings are the entire guidance + /// on the failure paths of the new verb; a user who reaches them has already discovered + /// `pair connect` and needs to be told to retry it rather than to find an administrator. + #[test] + fn both_terminal_failures_name_the_verb_the_reader_can_rerun() { + assert!(EXPIRED_BEFORE_APPROVAL.contains("`dign pair connect`")); + assert!(terminal_status_message("unknown").contains("`dign pair connect`")); + } + /// **Proves (dig-node#346):** an attacker-supplied `client_name` cannot forge a line of the /// operator's approval prompt. /// diff --git a/crates/dig-node-service/src/paired_client.rs b/crates/dig-node-service/src/paired_client.rs index aef8abe6..ea5ff06a 100644 --- a/crates/dig-node-service/src/paired_client.rs +++ b/crates/dig-node-service/src/paired_client.rs @@ -37,20 +37,70 @@ use std::time::Duration; /// The per-user file holding the scoped token this account was granted. /// -/// It lives beside the invoking user's own node state ([`crate::state::legacy_state_dir`] — -/// `$HOME/DigNode` / `%LOCALAPPDATA%\DigNode`), NOT in the machine-wide state dir: the whole point -/// is that an ordinary user can own it without anyone touching `/var/lib/dig-node`. +/// It lives under the invoking user's own home/known-folder base (`$HOME/DigNode` / +/// `%LOCALAPPDATA%\DigNode`), NOT in the machine-wide state dir: the whole point is that an +/// ordinary user can own it without anyone touching `/var/lib/dig-node`. pub const PAIRED_CLIENT_TOKEN_FILE: &str = "client-token"; +/// The folder name the ecosystem uses under the per-user base, byte-identical to the one +/// [`dig_node_core::config_path`] resolves under. Named here rather than reused from the cache +/// resolver on purpose — see [`paired_store_dir`]. +const PAIRED_STORE_FOLDER: &str = "DigNode"; + /// The default interval between `pairing.poll` calls while waiting for the operator to approve. /// /// Short enough that approval feels immediate, long enough that a 5-minute wait is ~100 requests /// rather than a spin. The wait is bounded by the SERVER's `expires_ms`, never by a local count. pub const POLL_INTERVAL: Duration = Duration::from_secs(3); -/// Where THIS user's paired token lives. -pub fn paired_token_path() -> PathBuf { - paired_token_path_in(&crate::state::legacy_state_dir()) +/// The directory THIS user's paired token lives in, or a REFUSAL when no per-user base exists. +/// +/// # Why this does not go through the state/cache resolver +/// +/// The obvious spelling — [`crate::state::legacy_state_dir`], the parent of +/// [`dig_node_core::config_path`] — resolves through `resolve_cache_dir`, which falls back to +/// `std::env::temp_dir()/DigNode-/cache` whenever the canonical dir is unwritable. That +/// fallback is a reasonable degraded mode for a CACHE and an unacceptable one for a BEARER +/// CREDENTIAL: on Unix `/tmp` is mode `1777`, the `` component is enumerable from `/proc`, +/// and a directory another user pre-created is accepted by `create_dir_all` without complaint. +/// Resolving from [`dig_node_core::platform_user_base`] directly makes that path STRUCTURALLY +/// unreachable for this file rather than merely unlikely — there is no branch left to sanitise. +/// +/// # Why an unresolvable base REFUSES instead of degrading +/// +/// [`dig_node_core::platform_user_base`] itself ends in `PathBuf::from(".")` when the OS knows no +/// home and neither `LOCALAPPDATA` nor `HOME` is set. Writing a bearer token into the process's +/// current working directory — frequently a shared or world-writable path, and one that moves with +/// every invocation — is a worse outcome than having no paired token at all, and the ladder +/// already has a correct answer for "no paired token": rung 3's master remedy. So this refuses, +/// the same refuse-rather-than-degrade discipline [`validate_client_name`] applies to a name. +pub fn paired_store_dir() -> io::Result { + paired_store_dir_from(dig_node_core::platform_user_base()) +} + +/// The pure core of [`paired_store_dir`], with the resolved base as an ARGUMENT. +/// +/// The no-home case cannot be produced from a test process — it needs an account the OS knows no +/// home for and an environment with neither `HOME` nor `LOCALAPPDATA` — so taking the base in is +/// what makes the REFUSAL assertable at all. Same reason [`select_token`] takes the master read's +/// outcome rather than performing it (the pattern #458 established). +fn paired_store_dir_from(base: PathBuf) -> io::Result { + if base.as_os_str().is_empty() || base == Path::new(".") { + return Err(io::Error::new( + io::ErrorKind::NotFound, + "dig-node: cannot store a paired token because no per-user directory could be \ + resolved for this account (no home directory, and neither HOME nor LOCALAPPDATA is \ + set). Run this command as an account with a home directory; the token is refused \ + rather than written to the current working directory, which is not a private place \ + to keep a credential.", + )); + } + Ok(base.join(PAIRED_STORE_FOLDER)) +} + +/// Where THIS user's paired token lives, or the [`paired_store_dir`] refusal. +pub fn paired_token_path() -> io::Result { + Ok(paired_token_path_in(&paired_store_dir()?)) } /// [`paired_token_path`] for an explicit directory, so tests use a temp dir and never touch a @@ -70,17 +120,44 @@ pub fn load_paired_token(path: &Path) -> Option { (!t.is_empty()).then(|| t.to_string()) } -/// Persist a freshly-approved token for this user, owner-only. +/// Persist a freshly-approved token for this user, owner-only from the moment it exists. +/// +/// The file is created by the INVOKING user, so it is owned by them; this function never runs +/// elevated and never touches a machine-wide path. +/// +/// # Why this is not `write` followed by a chmod /// -/// Creates the per-user state dir when absent and applies [`crate::state::restrict_file`] — -/// `0600` on Unix. The file is created by the INVOKING user, so it is owned by them; this -/// function never runs elevated and never touches a machine-wide path. +/// `std::fs::write` creates the file under the process umask — typically `0644` — and any later +/// `set_permissions` narrows it only AFTER the bytes are on disk, so the token is world-readable +/// for a window. Worse, `write` follows symlinks: a pre-planted `client-token -> ~/.ssh/authorized_keys` +/// in a directory another user can create makes this an arbitrary-file clobber running as the +/// victim, with no race to win. Creating the file EXCLUSIVELY at `0600` closes both — `create_new` +/// fails outright on an existing path, symlink included, and the mode is applied at `open(2)` +/// time rather than afterwards. This mirrors [`crate::state::ensure_dir_restricted`] + +/// `pairing::save_paired_tokens`, one module over. +/// +/// A pre-existing OWN store is removed first so that re-pairing still works; that removal deletes +/// the link rather than following it, so it cannot be turned into a write through a symlink. pub fn store_paired_token(path: &Path, token: &str) -> io::Result<()> { if let Some(dir) = path.parent() { - std::fs::create_dir_all(dir)?; + crate::state::ensure_dir_restricted(dir)?; + } + // `create_new` refuses an existing path; a re-pair legitimately replaces its own store. + match std::fs::remove_file(path) { + Ok(()) => {} + Err(e) if e.kind() == io::ErrorKind::NotFound => {} + Err(e) => return Err(e), } - std::fs::write(path, token)?; - crate::state::restrict_file(path); + let mut opts = std::fs::OpenOptions::new(); + opts.write(true).create_new(true); + #[cfg(unix)] + { + use std::os::unix::fs::OpenOptionsExt; + opts.mode(0o600); + } + let mut f = opts.open(path)?; + io::Write::write_all(&mut f, token.as_bytes())?; + f.sync_all()?; Ok(()) } @@ -262,6 +339,129 @@ mod tests { } } + /// **Proves (dig-node#403):** the store resolves from the per-user base, so it can NEVER land + /// under `std::env::temp_dir()`. + /// + /// The path it used to take — `state::legacy_state_dir()`, the parent of + /// `dig_node_core::config_path()` — runs through `resolve_cache_dir`, whose fallback is + /// `temp_dir()/DigNode-/cache` whenever the canonical dir is unwritable. On Unix that is + /// a `1777` directory with a `/proc`-enumerable name, which is where a bearer credential must + /// never be written. + /// + /// Asserted structurally (the dir IS `platform_user_base()/DigNode`) as well as negatively: + /// the negative alone would pass on any host whose home simply does not sit under the temp + /// dir, which is every host, so it cannot tell a fixed resolver from the broken one. + #[test] + fn the_store_resolves_from_the_per_user_base_and_never_the_temp_fallback() { + let dir = paired_store_dir().expect("this test account has a home directory"); + assert_eq!( + dir, + dig_node_core::platform_user_base().join("DigNode"), + "the store must come from the per-user base, not the cache resolver" + ); + assert!( + !dir.starts_with(std::env::temp_dir()), + "a bearer token must never be stored under the world-writable temp dir: {}", + dir.display() + ); + // The PID-keyed fallback's own shape, spelled out: no component of the resolved path may + // look like the private cache fallback, on any host. + assert!( + !dir.components().any(|c| c + .as_os_str() + .to_string_lossy() + .starts_with("DigNode-")), + "resolved a PID-keyed private fallback dir: {}", + dir.display() + ); + } + + /// **Proves:** with no per-user base resolvable, the store REFUSES rather than degrading to + /// the current working directory. + /// + /// `platform_user_base()` ends in `PathBuf::from(".")` when the OS knows no home and neither + /// `HOME` nor `LOCALAPPDATA` is set. A token written to `.` is a credential in whatever + /// directory the user happened to be standing in — frequently shared, and different on every + /// invocation. Refusing is correct because the ladder already has an answer for "no paired + /// token": rung 3's master remedy. + #[test] + fn an_unresolvable_per_user_base_refuses_rather_than_writing_to_the_cwd() { + for base in [PathBuf::from("."), PathBuf::new()] { + let err = paired_store_dir_from(base.clone()) + .expect_err("no per-user base must not resolve to a path"); + assert_eq!(err.kind(), io::ErrorKind::NotFound); + assert!( + err.to_string().contains("no per-user directory"), + "the refusal must say WHY: {err}" + ); + } + // The truthful control: a real base still resolves, so the refusal above is a property of + // the unresolvable input and not of the function refusing everything. + assert_eq!( + paired_store_dir_from(PathBuf::from("/home/u")).unwrap(), + PathBuf::from("/home/u").join("DigNode") + ); + } + + /// **Proves (dig-node#403):** the store is created EXCLUSIVELY and never follows a symlink an + /// attacker planted at its path. + /// + /// This is the revert-proof for the safe-create. The previous implementation was + /// `fs::write` + a later chmod; `write` opens with `O_TRUNC` and FOLLOWS symlinks, so a + /// `client-token -> ` planted in a directory the attacker can create hands them the + /// token — or, pointed at `~/.ssh/authorized_keys`, clobbers an arbitrary file as the victim, + /// with no race to win. Restoring `fs::write` fails this test: the victim's contents become + /// the token. + /// + /// The transient-mode window (the file existing at `0644` between `write` and the chmod) is + /// NOT separately asserted, and deliberately not faked: observing it requires racing a + /// single `write` call from another thread, which is inherently flaky and would assert timing + /// rather than the property. `create_new` + `mode(0o600)` removes the window structurally — + /// the mode is applied at `open(2)` time, so no wider mode ever exists to observe. + #[cfg(unix)] + #[test] + fn the_store_is_created_exclusively_and_never_follows_a_planted_symlink() { + use std::os::unix::fs::PermissionsExt; + + let dir = tempfile::tempdir().unwrap(); + let victim = dir.path().join("victim"); + std::fs::write(&victim, "PRECIOUS").unwrap(); + + let store = dir.path().join("store"); + std::fs::create_dir(&store).unwrap(); + let path = paired_token_path_in(&store); + std::os::unix::fs::symlink(&victim, &path).expect("plant the symlink"); + + store_paired_token(&path, "scoped-token").unwrap(); + + assert_eq!( + std::fs::read_to_string(&victim).unwrap(), + "PRECIOUS", + "the token was written THROUGH the symlink into the victim file" + ); + assert!( + !std::fs::symlink_metadata(&path).unwrap().is_symlink(), + "the store must be a regular file the invoking user created" + ); + assert_eq!(load_paired_token(&path).as_deref(), Some("scoped-token")); + let mode = std::fs::metadata(&path).unwrap().permissions().mode() & 0o777; + assert_eq!(mode, 0o600, "created owner-only, not chmodded afterwards"); + } + + /// **Proves:** re-pairing REPLACES this user's own store rather than failing on `create_new`. + /// + /// The exclusive create is what closes the symlink hole, and the obvious way to get it wrong + /// is to make a second `pair connect` fail with `AlreadyExists`. Both halves are needed: the + /// test above would pass on an implementation that never overwrites anything. + #[test] + fn re_pairing_replaces_this_users_own_store() { + let dir = tempfile::tempdir().unwrap(); + let path = paired_token_path_in(dir.path()); + store_paired_token(&path, "first").unwrap(); + store_paired_token(&path, "second").unwrap(); + assert_eq!(load_paired_token(&path).as_deref(), Some("second")); + } + /// **Proves:** a blank store is not a token — it reads as "this account has no paired token" /// so the ladder falls through to the master remedy instead of presenting an empty header. #[test] From 014fe3d3553e91008f0a4ca6ed68355004c2517b Mon Sep 17 00:00:00 2001 From: Michael Taylor Date: Wed, 2 Sep 2026 04:00:37 -0700 Subject: [PATCH 5/6] test(pair): fail on an indented continuation, not only a mid-line space run The guard against a lost `\` line continuation trimmed the start of every line before checking, so it could only see the corruption in ONE of its two forms. A multi-line literal without the trailing backslash emits the source's indentation as a NEWLINE plus nine spaces, not as an interior run, and `"a\n b"` prints just as raggedly as `"a b"`. Proved by reverting only `paired_message`'s continuation: the old assertion PASSED on the reverted string, so it was not load-bearing against that shape. The tightened test fails on it ("prose must not be indented"), and passes once the continuation is restored. The banner keeps its one legitimate indent -- exactly four spaces on the copy-pasteable `sudo dign pair approve ` line -- and is now the only string allowed any, checked against that exact width rather than waved through. No production code changed. Co-Authored-By: Claude --- crates/dig-node-service/src/pair.rs | 35 ++++++++++++++++++++++------- 1 file changed, 27 insertions(+), 8 deletions(-) diff --git a/crates/dig-node-service/src/pair.rs b/crates/dig-node-service/src/pair.rs index deb1ff31..f049138a 100644 --- a/crates/dig-node-service/src/pair.rs +++ b/crates/dig-node-service/src/pair.rs @@ -277,22 +277,41 @@ mod tests { let paired = paired_message(std::path::Path::new("/home/u/DigNode/client-token")); let terminal = terminal_status_message("expired"); - for (what, text) in [ - ("waiting banner", banner.as_str()), - ("paired message", paired.as_str()), - ("expired-before-approval", EXPIRED_BEFORE_APPROVAL), - ("terminal status", terminal.as_str()), + // The banner is the ONE string with legitimate leading whitespace: it indents a + // copy-pasteable command by exactly four spaces so the reader can select it. Every other + // string is prose and must carry none, because a lost continuation shows up as leading + // indentation just as readily as it shows up mid-line -- `"a\n b"` prints as + // raggedly as `"a b"`, and a check that trims the start cannot see the first one. + const BANNER_INDENT: &str = " "; + for (what, text, indent_allowed) in [ + ("waiting banner", banner.as_str(), true), + ("paired message", paired.as_str(), false), + ("expired-before-approval", EXPIRED_BEFORE_APPROVAL, false), + ("terminal status", terminal.as_str(), false), ] { for line in text.lines() { - // The banner deliberately indents its copy-pasteable command by four spaces, so - // LEADING whitespace is legitimate. A run in the middle of a sentence is not, and - // that is exactly what a lost continuation produces. let interior = line.trim_start(); + let leading = &line[..line.len() - interior.len()]; assert!( !interior.contains(" "), "{what}: a run of 3+ spaces mid-line is the signature of a lost `\\` line \ continuation in a multi-line literal: {line:?}" ); + if indent_allowed { + assert!( + leading.is_empty() || leading == BANNER_INDENT, + "{what}: the only legitimate indent is the command line's four spaces; \ + anything else is source indentation that leaked into the output: \ + {line:?}" + ); + } else { + assert!( + leading.is_empty(), + "{what}: prose must not be indented -- leading whitespace here is the \ + source's own indentation, which is what a lost `\\` continuation \ + emits: {line:?}" + ); + } } } } From 205211b809d62cb5ca7c1c03d8af9f22d8f6ac59 Mon Sep 17 00:00:00 2001 From: Michael Taylor Date: Wed, 2 Sep 2026 04:00:58 -0700 Subject: [PATCH 6/6] style(pair): apply rustfmt to the token-store test The previous fix commit left `the_store_resolves_from_the_per_user_base_and_ never_the_temp_fallback` unformatted, which the fmt gate fails on. Whitespace only; no assertion changed. Co-Authored-By: Claude --- crates/dig-node-service/src/paired_client.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/crates/dig-node-service/src/paired_client.rs b/crates/dig-node-service/src/paired_client.rs index ea5ff06a..ea70f020 100644 --- a/crates/dig-node-service/src/paired_client.rs +++ b/crates/dig-node-service/src/paired_client.rs @@ -367,10 +367,8 @@ mod tests { // The PID-keyed fallback's own shape, spelled out: no component of the resolved path may // look like the private cache fallback, on any host. assert!( - !dir.components().any(|c| c - .as_os_str() - .to_string_lossy() - .starts_with("DigNode-")), + !dir.components() + .any(|c| c.as_os_str().to_string_lossy().starts_with("DigNode-")), "resolved a PID-keyed private fallback dir: {}", dir.display() );