From 9946b412feab270d8f69a2c7afcd72f559f1b3b6 Mon Sep 17 00:00:00 2001 From: Michael Taylor Date: Mon, 31 Aug 2026 06:41:30 -0700 Subject: [PATCH 1/8] chore(cli): open batch for #303 #304 #305 #392 #403 #407 Co-Authored-By: Claude From 3386a9602fdac82e059e24a933fb82996569d0e2 Mon Sep 17 00:00:00 2001 From: Michael Taylor Date: Mon, 31 Aug 2026 06:53:20 -0700 Subject: [PATCH 2/8] fix(packaging): restart the systemd unit on .deb upgrade so the new binary is the one running `systemctl enable --now` is a no-op on a unit that is already enabled and running, so a `dpkg`/`apt` upgrade replaced /usr/bin/dig-node and left the OLD process serving. The failure was silent in both directions an operator checks: `dig-node --version` reads the on-disk image and reports the new version at once, and `systemctl is-active` reports active because the old process is genuinely healthy. Only MainPID moved. A security fix shipped through the .deb therefore did not take effect on upgrade. postinst now runs `systemctl try-restart` when dpkg passes a previously-configured version ($2), i.e. on an upgrade only. `try-restart` rather than `restart` so a node the operator deliberately stopped -- including one held back by the #317 no-autostart marker -- stays stopped. macOS needs no change: its postinstall already does bootout + bootstrap + kickstart -k. Refs #305 Co-Authored-By: Claude --- packaging/linux/build-deb.sh | 23 +++++++++++++-- scripts/tests/deb-contents.test.sh | 45 ++++++++++++++++++++++++++++-- 2 files changed, 64 insertions(+), 4 deletions(-) diff --git a/packaging/linux/build-deb.sh b/packaging/linux/build-deb.sh index e7a81d3c..05d03330 100755 --- a/packaging/linux/build-deb.sh +++ b/packaging/linux/build-deb.sh @@ -73,8 +73,8 @@ printf '/etc/dig-node/dig-node.env\n' > "$STAGE/DEBIAN/conffiles" # --- maintainer scripts ----------------------------------------------------- # postinst: pre-create the restrictive machine-wide state dir (#501 — root-owned 0700 so -# the control token is not world-readable), enable+start the service, register the scheme -# handler as the system default. +# the control token is not world-readable), enable+start the service, cycle it on upgrade +# (#305), and register the scheme handler as the system default. cat > "$STAGE/DEBIAN/postinst" <<'EOF' #!/bin/sh set -e @@ -127,6 +127,25 @@ case "$1" in else systemctl enable --now net.dignetwork.dig-node.service || true fi + # dig-node#305 — an UPGRADE must cycle the unit, or the old process keeps serving. + # + # `enable --now` above is a no-op on a unit that is already enabled and running, so before + # this the upgrade replaced /usr/bin/dig-node and left the OLD binary executing. That is + # silent in both directions an operator would check: `dig-node --version` reads the on-disk + # image and reports the NEW version immediately, and `systemctl is-active` reports active + # because the old process is genuinely healthy. Only MainPID moves, and nobody reads it. A + # security fix shipped through the .deb therefore did not take effect on upgrade. + # + # dpkg passes the previously-configured version as $2 only on an upgrade, so this is scoped + # to the case that needs it: on a first install `enable --now` has already started the new + # binary and a restart would be redundant churn. + # + # `try-restart` rather than `restart`: it cycles the unit ONLY if it is already running, so + # a node the operator deliberately stopped — including one held back by the #317 + # no-autostart marker — stays stopped across every future upgrade. + if [ -n "${2:-}" ]; then + systemctl try-restart net.dignetwork.dig-node.service || true + fi fi # Register the chia:// handler as the system default + refresh the desktop DB. if command -v update-desktop-database >/dev/null 2>&1; then diff --git a/scripts/tests/deb-contents.test.sh b/scripts/tests/deb-contents.test.sh index 147d0c8d..b380aadb 100644 --- a/scripts/tests/deb-contents.test.sh +++ b/scripts/tests/deb-contents.test.sh @@ -99,13 +99,18 @@ echo "systemctl $*" >> "$SYSTEMCTL_LOG" STUB chmod 0755 "$TMP/stub/systemctl" -# run_postinst -> echoes the recorded systemctl invocations +# run_postinst [old-version] -> echoes the recorded systemctl invocations +# +# dpkg passes the PREVIOUSLY-CONFIGURED version as $2 on an upgrade and nothing at all on a +# first install. That argument is the only thing distinguishing the two runs, so it is a +# parameter here rather than being fixed to the install case. run_postinst() { local root="$1" + local old_version="${2:-}" export SYSTEMCTL_LOG="$root/systemctl.log" : > "$SYSTEMCTL_LOG" mkdir -p "$root/etc" "$root/var/lib" "$root/usr/share/applications" - PATH="$TMP/stub:$PATH" DIG_NODE_PKG_ROOT="$root" sh "$TMP/ctl/postinst" configure \ + PATH="$TMP/stub:$PATH" DIG_NODE_PKG_ROOT="$root" sh "$TMP/ctl/postinst" configure $old_version \ >"$root/postinst.out" 2>&1 cat "$SYSTEMCTL_LOG" } @@ -151,6 +156,42 @@ else fail "the marker suppressed daemon-reload — the unit is not registered and cannot be started later" fi +# --- 3. dig-node#305: an UPGRADE must cycle the running service ---------------------------- +# `systemctl enable --now` is a no-op on a unit that is already enabled and running, so the +# upgrade path replaced the binary on disk and left the OLD process serving. Both checks a +# person would naturally run reported success: `dig-node --version` reads the ON-DISK binary, +# and `systemctl is-active` reports the genuinely-healthy OLD process. Only MainPID showed it. +# +# The fixture is an UPGRADE (`configure `) with a truthful control beside it (the +# first-install run above, `configure` with no old version). One argument separates them, and +# it is the only argument dpkg varies -- so a "fix" that restarts unconditionally passes the +# upgrade assertion and is caught by the control at the end of this section. +UPGRADE_ROOT="$TMP/upgrade"; mkdir -p "$UPGRADE_ROOT" +UPGRADE_LOG="$(run_postinst "$UPGRADE_ROOT" "0.1.0")" +if grep -qE 'systemctl (try-restart|try-reload-or-restart) net\.dignetwork\.dig-node\.service' <<<"$UPGRADE_LOG"; then + ok "an upgrade cycles the service, so the new binary is the one running (#305)" +else + fail "an upgrade does not restart the service -- the old binary keeps running silently (#305)" +fi + +# `try-restart` is load-bearing, not a spelling preference: a plain `restart` would START a node +# the operator had deliberately stopped, and would defeat the #317 no-autostart marker on every +# subsequent upgrade. Asserting the SEMANTICS means a later simplification to `restart` fails. +if grep -qE 'systemctl (restart|start|reload-or-restart) net\.dignetwork\.dig-node\.service' <<<"$UPGRADE_LOG"; then + fail "the upgrade uses an unconditional restart -- it would start a node the operator stopped (#305)" +else + ok "the upgrade restart is conditional on the unit already running (#305)" +fi + +# The control. A FIRST install already starts the node via `enable --now`, so a restart there +# would be redundant; its ABSENCE is what proves the assertion above measured the upgrade +# argument rather than a restart bolted onto every configure. +if grep -qE 'systemctl [a-z-]*restart' <<<"$CONTROL_LOG"; then + fail "a first install restarts the service -- the upgrade check above proves nothing" +else + ok "a first install does not restart (so the upgrade check is measuring the upgrade)" +fi + # The redirect seam must default to the real root. A postinst that defaulted DIG_NODE_PKG_ROOT to # anything non-empty would pass every check above while installing into a scratch directory on a # real host. From 13b192925e0067cbe69d97a79e23262970c74d5a Mon Sep 17 00:00:00 2001 From: Michael Taylor Date: Mon, 31 Aug 2026 07:05:46 -0700 Subject: [PATCH 3/8] feat(cli): add `dign network-info`, and stop telling a peerless node to upgrade MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two user-visible lies on the CLI's network surface. `network-info` was documented and expected but never existed as a subcommand, so it presented as empty output on three healthy fleet boxes -- what an unrecognised subcommand looks like once a shell has swallowed the usage text. The node has always ANSWERED the question: `dig.getNetworkInfo` serves peer id, network + genesis, advertised candidates (IPv6-first, §5.2), reachability and the relay reservation. Only the verb was missing, so this adds the verb rather than the data. It reads through a new token-free `control_client::call_open`. Every field it prints is already published to any peer that dials this node, so the control token would buy no confidentiality while costing real availability: on a .deb install the master token is 0600 root:root (#501), and an ordinary user asking "what is my node's address" would be told to elevate for a read the network performs for free. `call_control` is untouched and still carries the token. `peers` conflated two different empty states behind one message: a node with zero peers was told "a per-peer list needs a newer node" -- on 0.138.0, the newest build that existed. The advice was false and pointed at a release that does not exist, and it appears on first run, when a user has zero peers and is least able to tell. The connected count distinguishes them: zero connected is stated plainly with something to try; peers counted but not enumerable keeps the version note, which is the only state that actually implies one. Refs #303, #304 Co-Authored-By: Claude --- crates/dig-node-service/src/control_client.rs | 35 +++-- crates/dig-node-service/src/entrypoint.rs | 11 ++ crates/dig-node-service/src/lib.rs | 4 + crates/dig-node-service/src/network_info.rs | 131 ++++++++++++++++++ crates/dig-node-service/src/peers.rs | 67 +++++++-- 5 files changed, 232 insertions(+), 16 deletions(-) create mode 100644 crates/dig-node-service/src/network_info.rs diff --git a/crates/dig-node-service/src/control_client.rs b/crates/dig-node-service/src/control_client.rs index e4e4eede..27bec008 100644 --- a/crates/dig-node-service/src/control_client.rs +++ b/crates/dig-node-service/src/control_client.rs @@ -47,24 +47,43 @@ pub fn call_control(config: &Config, method: &str, params: Value) -> std::io::Re let rt = tokio::runtime::Builder::new_current_thread() .enable_all() .build()?; - rt.block_on(call_async(addr, &token, method, params)) + rt.block_on(call_async(addr, Some(&token), method, params)) } -/// POST one JSON-RPC control method with the master token; return its `result` (or `{}` when -/// the node omits one). A transport failure = the node isn't running; a JSON-RPC `error` = -/// the node rejected the call (e.g. a method it does not implement → METHOD_NOT_FOUND). +/// Call one OPEN (non-`control.*`) node method and return its `result` object -- same transport, +/// same direct-to-loopback pinning, NO control token. +/// +/// The loopback server gates the `control.*` plane fail-closed and leaves the ordinary `dig.*` +/// read surface open, so presenting a token here would not change what the node answers. It would +/// change who can ASK: on a `.deb` install the master token is `0600 root:root` (#501), so +/// routing an open read through [`call_control`] makes it fail with an elevation remedy for a +/// question the node answers to any peer that dials it. Use this for reads the node already +/// publishes, and [`call_control`] for everything that is genuinely privileged. +pub fn call_open(config: &Config, method: &str, params: Value) -> std::io::Result { + let addr = config.bind_addr(); + let rt = tokio::runtime::Builder::new_current_thread() + .enable_all() + .build()?; + rt.block_on(call_async(addr, None, method, params)) +} + +/// POST one JSON-RPC method, with the master token when one is supplied; return its `result` +/// (or `{}` when the node omits one). A transport failure = the node isn't running; a JSON-RPC +/// `error` = the node rejected the call (e.g. a method it does not implement → METHOD_NOT_FOUND). async fn call_async( addr: std::net::SocketAddr, - token: &str, + token: Option<&str>, method: &str, params: Value, ) -> std::io::Result { let client = build_control_client().map_err(std::io::Error::other)?; let url = format!("http://{addr}/"); let body = json!({ "jsonrpc": "2.0", "id": 1, "method": method, "params": params }); - let resp = client - .post(&url) - .header(control::CONTROL_TOKEN_HEADER, token) + let mut req = client.post(&url); + if let Some(token) = token { + req = req.header(control::CONTROL_TOKEN_HEADER, token); + } + let resp = req .json(&body) .send() .await diff --git a/crates/dig-node-service/src/entrypoint.rs b/crates/dig-node-service/src/entrypoint.rs index 10dd6191..223619ac 100644 --- a/crates/dig-node-service/src/entrypoint.rs +++ b/crates/dig-node-service/src/entrypoint.rs @@ -13,6 +13,7 @@ //! start Start the installed service. //! stop Stop the running service. //! status Report whether the node is serving (probes /health). +//! network-info This node's own network posture (peer id, addresses, reachability). //! //! With no subcommand, the binary runs in the foreground (equivalent to `run`), so a //! bare invocation just serves — the least-surprise default for a localhost endpoint. @@ -37,6 +38,7 @@ use crate::config::Config; use crate::control_cli::{self, ControlAction}; use crate::open; use crate::pair::{self, PairAction}; +use crate::network_info; use crate::peers::{self, BanState, PeersAction}; use crate::service::ScopeChoice; use crate::{serve, service, VERSION}; @@ -185,6 +187,13 @@ enum Command { #[command(subcommand)] action: Option, }, + /// This node's own network posture: peer id, network + genesis, advertised addresses + /// (IPv6-first, §5.2), reachability and relay reservation. + /// + /// Answers the question `peers` cannot: `peers` describes who this node is TALKING to, this + /// describes how this node is REACHABLE. Reads the node's open `dig.getNetworkInfo` surface, + /// so it needs no control token — every field here is already published to any peer. + NetworkInfo, /// View + manage the node's peer connections — parity with the extension's peer surface. /// With no sub-action, lists the live peer status (running flag, connected count, relay, and — /// on a newer node — the per-peer list with addresses shown IPv6-first per §5.2). @@ -750,6 +759,7 @@ impl Command { Command::Spends { .. } => "spends", Command::Updater { .. } => "updater", Command::Subscriptions { .. } => "subscriptions", + Command::NetworkInfo => "network-info", Command::Peers { .. } => "peers", Command::Collateral { .. } => "collateral", Command::Mirror { .. } => "mirror", @@ -904,6 +914,7 @@ pub fn run() -> std::process::ExitCode { action, json, ), + Command::NetworkInfo => render(network_info::run(&config), action, json), Command::Peers { action: cmd } => match peers_action(cmd) { Ok(a) => render(peers::run(&config, a), action, json), Err(e) => emit_error(&e, action, json), diff --git a/crates/dig-node-service/src/lib.rs b/crates/dig-node-service/src/lib.rs index 60850b62..ba78a5cc 100644 --- a/crates/dig-node-service/src/lib.rs +++ b/crates/dig-node-service/src/lib.rs @@ -78,6 +78,10 @@ pub mod meta; /// `(store, root, epoch)`, and its disappearance drives reclaim of the collateral. The node signs /// those spends itself with its own operator wallet, scoped by construction. See [`mirror`]. pub mod mirror; +/// `dign network-info` (#303): this node's OWN network posture -- peer id, network + genesis, +/// advertised addresses (IPv6-first, §5.2), reachability and relay reservation. Reads the node's +/// OPEN `dig.getNetworkInfo` surface, so it needs no control token. See [`network_info`]. +pub mod network_info; /// `dig-node open ` (#389): the OS scheme-handler target the /// installer registers for `chia://` + `urn:dig:chia:`. Strictly validates the untrusted /// handler argument, then opens the user's default browser at the resolving URL. See [`open`]. diff --git a/crates/dig-node-service/src/network_info.rs b/crates/dig-node-service/src/network_info.rs new file mode 100644 index 00000000..226df99d --- /dev/null +++ b/crates/dig-node-service/src/network_info.rs @@ -0,0 +1,131 @@ +//! `dign network-info` — this node's own network posture (dig-node#303). +//! +//! The node has always ANSWERED this question: `dig.getNetworkInfo` is served over the loopback +//! JSON-RPC surface and returns the node's `peer_id`, network id, effective L2 genesis, advertised +//! candidate addresses (IPv6-first, §5.2), reachability and relay reservation. What did not exist +//! was a way to ASK it from the command line — `dig-node network-info` was reported as producing +//! empty output on three healthy fleet boxes, which is what an unrecognised subcommand looks like +//! once a shell has swallowed the usage text. `peers` on the same boxes in the same session +//! answered, so the data was there and only the verb was missing. +//! +//! # Why this reads the OPEN surface rather than a `control.*` method +//! +//! Everything here is already published to strangers: the same `dig.getNetworkInfo` body is what +//! this node hands any peer that dials it, so a loopback caller learns nothing a peer does not. +//! Reading it through the token-gated control plane would therefore buy no confidentiality while +//! costing real availability — on a `.deb` install the control token is `0600 root:root` +//! (#501), so an ordinary user asking "what is my node's address" would be told to elevate for a +//! read the network performs for free. It is served token-free for that reason, deliberately, and +//! that is a property to preserve rather than an oversight to tighten later. + +use serde_json::{json, Value}; + +use crate::cli::Outcome; +use crate::config::Config; +use crate::control_client::call_open; + +/// Run `network-info`: read `dig.getNetworkInfo` from the running node and render it. +pub fn run(config: &Config) -> std::io::Result { + let result = call_open(config, "dig.getNetworkInfo", json!({}))?; + Ok(Outcome::new(format_network_info(&result), result)) +} + +/// Render the node's network posture as an operator-friendly block. PURE over the RPC result. +/// +/// Every field is rendered from what the node actually returned: an absent field prints as +/// `unknown` rather than as a plausible default, because a fabricated `direct` or an invented +/// `0.0.0.0` reads exactly like a measurement and would be acted on as one. +fn format_network_info(result: &Value) -> String { + let text = |key: &str| { + result[key] + .as_str() + .map_or_else(|| "unknown".to_string(), str::to_string) + }; + + let mut out = format!( + "dig-node network info:\n peer id {}\n network {}\n genesis {}", + text("peer_id"), + text("network_id"), + text("genesis"), + ); + out.push_str(&format!("\n listen addr {}", text("listen_addr"))); + out.push_str(&format!("\n reachability {}", text("reachability"))); + + // The advertised candidates in the order the node advertises them, which is IPv6-first (§5.2). + // Reordering here would hide a node whose IPv6 advertisement is missing — the exact fault an + // operator runs this command to find — so the order is passed through untouched. + let candidates: Vec<&str> = result["candidate_addresses"] + .as_array() + .map(|a| a.iter().filter_map(Value::as_str).collect()) + .unwrap_or_default(); + if candidates.is_empty() { + out.push_str("\n candidates none advertised (this node is not dialable by peers)"); + } else { + out.push_str("\n candidates:"); + for addr in candidates { + out.push_str(&format!("\n • {addr}")); + } + } + + if let Some(url) = result["relay"]["url"].as_str() { + let reserved = result["relay"]["reserved"].as_bool().unwrap_or(false); + out.push_str(&format!( + "\n relay {url} — reservation {}", + if reserved { "held" } else { "none" } + )); + } + out +} + +#[cfg(test)] +mod tests { + use super::*; + + /// The whole posture renders, and the candidate order the node chose survives verbatim. + #[test] + fn renders_posture_and_preserves_the_advertised_candidate_order() { + let s = format_network_info(&json!({ + "peer_id": "aa11", + "network_id": "mainnet", + "genesis": "ccd5bb71183532bff220ba46c268991a00000000000000000000000000000000", + "listen_addr": "[2001:db8::5]:9444", + "reachability": "relayed", + "candidate_addresses": ["[2001:db8::5]:9444", "203.0.113.7:9444"], + "relay": { "url": "wss://relay.dig.net:443", "reserved": true }, + })); + assert!(s.contains("aa11")); + assert!(s.contains("mainnet")); + assert!(s.contains("relayed")); + assert!(s.contains("relay.dig.net")); + assert!(s.contains("reservation held")); + // Two candidates, IPv6 first — asserted by POSITION, so a re-sort is visible here. A + // `contains` on each address alone would pass under any ordering, including one that + // demoted the IPv6 candidate the §5.2 policy exists to keep first. + let v6 = s.find("[2001:db8::5]:9444").unwrap(); + let v4 = s.find("203.0.113.7:9444").unwrap(); + assert!(v6 < v4, "advertised order must pass through untouched: {s}"); + } + + /// A node advertising nothing says so. The tempting alternative — printing the listen address + /// as though it were a candidate — would report a dialable endpoint that no peer was ever + /// offered, which is the failure this verb exists to expose. + #[test] + fn no_candidates_is_stated_not_silently_omitted() { + let s = format_network_info(&json!({ + "peer_id": "bb22", + "candidate_addresses": [], + })); + assert!(s.contains("none advertised"), "{s}"); + assert!(s.contains("not dialable"), "{s}"); + } + + /// A field the node did not send prints as `unknown`. Nobody observed a reachability here, and + /// `direct` — the value a plain `unwrap_or_default` on a string would never produce, but which + /// a hand-written default reaches for — would be a claim about the network drawn from nothing. + #[test] + fn an_absent_field_prints_unknown_rather_than_a_plausible_default() { + let s = format_network_info(&json!({ "peer_id": "cc33" })); + assert!(s.contains("unknown"), "{s}"); + assert!(!s.contains("direct"), "a missing reachability must not read as direct: {s}"); + } +} diff --git a/crates/dig-node-service/src/peers.rs b/crates/dig-node-service/src/peers.rs index 815ee131..fa473ad7 100644 --- a/crates/dig-node-service/src/peers.rs +++ b/crates/dig-node-service/src/peers.rs @@ -173,15 +173,17 @@ fn format_counts(result: &Value) -> String { /// Render `control.peerStatus` as an operator-friendly summary. Shows the running flag, the /// connected count, and the relay reservation; when a newer node fills the optional per-peer /// list it prints each peer with its addresses ordered IPv6-first (§5.2). +/// +/// An EMPTY per-peer list is reported by cause rather than by symptom (#304): zero connected +/// peers is stated as such, and the version-gap note is kept for the only state that actually +/// implies one — peers this node counts but cannot enumerate. fn format_status(result: &Value) -> String { let running = result["running"].as_bool().unwrap_or(false); if !running { return "dig-node: peer network is not running (no connected peers).".to_string(); } - let mut out = format!( - "dig-node peer network: running · {} connected peer(s)", - result["connected_peers"].as_u64().unwrap_or(0), - ); + let connected = result["connected_peers"].as_u64().unwrap_or(0); + let mut out = format!("dig-node peer network: running · {connected} connected peer(s)"); if let Some(url) = result["relay"]["url"].as_str() { let reserved = result["relay"]["reserved"].as_bool().unwrap_or(false); out.push_str(&format!( @@ -191,10 +193,23 @@ fn format_status(result: &Value) -> String { } let mut peers = result["peers"].as_array().cloned().unwrap_or_default(); if peers.is_empty() { - // Honest degradation: no per-peer list (no peer network running, or an older node build). - out.push_str( - "\n (this node build reports a count only — a per-peer list needs a newer node)", - ); + // An empty list has TWO causes and they need different words (#304). Conflating them + // told a first-run user on the newest node that exists to go and upgrade -- advice that + // is false, and false in the direction that sends them looking for a release that does + // not exist. The connected count distinguishes the two: a node reporting zero connected + // peers has an empty list because it HAS no peers, while a node reporting peers it + // cannot enumerate is the genuine version gap the note was written for. + if connected == 0 { + out.push_str( + "\n no peers connected yet — a node finds peers within a minute or two of \ + starting. If it stays at zero, check outbound connectivity on the peer port \ + and try `dign peers connect
` with a known peer.", + ); + } else { + out.push_str( + "\n (this node build reports a count only — a per-peer list needs a newer node)", + ); + } } else { // Render peers with IPv6-addressed ones first (§5.2 display policy). peers.sort_by_key(|p| is_ipv4(p["address"].as_str().unwrap_or(""))); @@ -253,6 +268,42 @@ mod tests { assert!(s.contains("newer node")); } + /// #304 -- a node with zero peers must not be told to upgrade. Measured on 0.138.0, the + /// newest build that existed, where zero peers was the correct state: the CLI printed "a + /// per-peer list needs a newer node", sending a first-run user after a release that does not + /// exist. The fixture varies ONLY the connected count against the test above, which keeps a + /// truthful control: 4 connected peers with no list IS a version gap and still says so, so a + /// fix that merely reworded the string unconditionally fails there rather than here. + #[test] + fn zero_peers_says_so_instead_of_blaming_the_node_version() { + let s = format_status(&json!({ + "running": true, + "connected_peers": 0, + "relay": { "url": "wss://relay.dig.net:443", "reserved": false }, + })); + assert!( + !s.contains("newer node"), + "zero peers is not a version gap: {s}" + ); + assert!(s.contains("no peers connected"), "{s}"); + // Actionable, not merely honest: the message must name something to try. + assert!(s.contains("peers connect"), "{s}"); + } + + /// The same state reported by a node that DOES fill the list -- an explicitly empty array + /// rather than an absent key. It is still zero peers, and must read identically; the two + /// spellings of "no peers" must not diverge into two different messages. + #[test] + fn an_explicitly_empty_peer_array_reads_the_same_as_an_absent_one() { + let s = format_status(&json!({ + "running": true, + "connected_peers": 0, + "peers": [], + })); + assert!(!s.contains("newer node"), "{s}"); + assert!(s.contains("no peers connected"), "{s}"); + } + #[test] fn peer_line_shows_id_via_direction_and_address() { let peer = json!({ From c09bc62c342fabee85029bb075c2447733ad8c30 Mon Sep 17 00:00:00 2001 From: Michael Taylor Date: Mon, 31 Aug 2026 07:12:49 -0700 Subject: [PATCH 4/8] fix(cli): report an unreachable node as unmeasured, not as an I/O failure `dign updater check-now` probing inside the restart window reported IO_ERROR, which reads to an operator exactly like an update that broke the node. It is the opposite: a successful pass installs new bytes and cycles the service, so the restart is the normal END of the thing that succeeded. Same class as dig-updater#77, on the surface #77's fix does not reach. IO_ERROR says an I/O operation was attempted and failed -- a claim about the request. An unreachable node is a failure to MEASURE, not a measured failure, and sharing one encoding is what trains an operator to ignore the surface, which is how the one real failure gets ignored too. That matters more under the silent-staged-install policy (dig_ecosystem#3180), where the status surface is all an operator has. So the loopback control client's ConnectionRefused class now resolves to a new exit code 7 NODE_UNREACHABLE, catalogued in cli.rs, README, SPEC and USER_JOURNEY. Its message is restated at the one choke point in control_cli::run: `control.updater.*` gets the restart explanation because it has a specific expected cause, everything else gets the general statement. Only the message changes and only for that kind -- a node that ANSWERED and declined has measured something, and its own words pass through untouched. No timeout was widened: that converts a wrong answer into a slower wrong answer. Refs #407 Co-Authored-By: Claude --- README.md | 1 + SPEC.md | 1 + USER_JOURNEY.md | 2 +- crates/dig-node-service/src/cli.rs | 54 +++++++++++++++ crates/dig-node-service/src/control_cli.rs | 79 +++++++++++++++++++++- 5 files changed, 135 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index e46da825..3793cd3c 100644 --- a/README.md +++ b/README.md @@ -332,6 +332,7 @@ typed `ExitCode` enum in `src/cli.rs`: | 4 | `SERVICE_FAILED` | A service operation failed (register/start/stop/uninstall). | | 5 | `BIND_FAILED` | `run`: could not bind the loopback address. | | 6 | `IO_ERROR` | Other I/O error. | +| 7 | `NODE_UNREACHABLE` | The node did not answer; the operation was not measured. | ### JSON-RPC error-code catalogue diff --git a/SPEC.md b/SPEC.md index f0d62d3a..34219294 100644 --- a/SPEC.md +++ b/SPEC.md @@ -2620,6 +2620,7 @@ exit `1` (`NOT_SERVING`) so scripts can gate on liveness; the JSON result carrie | 4 | `SERVICE_FAILED` | A service-manager operation failed. | | 5 | `BIND_FAILED` | `run`: could not bind the loopback address. | | 6 | `IO_ERROR` | Other I/O error. | +| 7 | `NODE_UNREACHABLE` | The node did not answer; the operation was not measured. | I/O-error mapping: `PermissionDenied` → 3; `AddrInUse`/`AddrNotAvailable` → 5; anything else → 6. Numeric values and symbolic names are a stable contract and MUST NOT be renumbered. diff --git a/USER_JOURNEY.md b/USER_JOURNEY.md index eda4eaa7..471652ed 100644 --- a/USER_JOURNEY.md +++ b/USER_JOURNEY.md @@ -163,7 +163,7 @@ every subcommand (machine output to stdout, prose to stderr). - **`--json`** on the CLI: success → `{ ok:true, action, service, version, …result }`; failure → `{ ok:false, error:{ code, exit_code, message, hint } }`. - **Exit-code table** (documented in the README + `src/cli.rs`): `0 OK`, `1 NOT_SERVING`, - `2 USAGE`, `3 PERMISSION_DENIED`, `4 SERVICE_FAILED`, `5 BIND_FAILED`, `6 IO_ERROR`. + `2 USAGE`, `3 PERMISSION_DENIED`, `4 SERVICE_FAILED`, `5 BIND_FAILED`, `6 IO_ERROR`, `7 NODE_UNREACHABLE`. - **Stable JSON-RPC error codes** (UPPER_SNAKE in `error.data.code`): `PARSE_ERROR` (-32700), `INVALID_REQUEST` (-32600), `METHOD_NOT_FOUND` (-32601), `INVALID_PARAMS` (-32602), `DISPATCH_FAILED` (-32000, shell), `UPSTREAM_ERROR` (-32010, shell), and the control-plane codes diff --git a/crates/dig-node-service/src/cli.rs b/crates/dig-node-service/src/cli.rs index e4a454cd..8bbef8ca 100644 --- a/crates/dig-node-service/src/cli.rs +++ b/crates/dig-node-service/src/cli.rs @@ -19,6 +19,7 @@ //! | 4 | SERVICE_FAILED | A service operation failed (register/start/stop).| //! | 5 | BIND_FAILED | `run`: could not bind the loopback address. | //! | 6 | IO_ERROR | Other I/O error. | +//! | 7 | NODE_UNREACHABLE | The node did not answer; nothing was measured. | use serde_json::{json, Value}; @@ -43,6 +44,14 @@ pub enum ExitCode { BindFailed, /// 6 — any other I/O error. IoError, + /// 7 — the node did not answer, so the operation was never measured (dig-node#407). + /// + /// Deliberately NOT `IO_ERROR`: that code says an I/O operation was attempted and failed, + /// which is a claim about the requested operation. An unreachable node is a failure to + /// MEASURE, not a measured failure, and the two must not share an encoding -- the restart + /// window an update opens is exactly when the difference matters, and a caller that cannot + /// tell them apart learns to ignore both. + NodeUnreachable, } impl ExitCode { @@ -56,6 +65,7 @@ impl ExitCode { ExitCode::ServiceFailed => 4, ExitCode::BindFailed => 5, ExitCode::IoError => 6, + ExitCode::NodeUnreachable => 7, } } @@ -69,6 +79,7 @@ impl ExitCode { ExitCode::ServiceFailed => "SERVICE_FAILED", ExitCode::BindFailed => "BIND_FAILED", ExitCode::IoError => "IO_ERROR", + ExitCode::NodeUnreachable => "NODE_UNREACHABLE", } } @@ -86,6 +97,9 @@ impl ExitCode { } ExitCode::BindFailed => "run: could not bind the loopback address.", ExitCode::IoError => "Other I/O error.", + ExitCode::NodeUnreachable => { + "The node did not answer; the operation was not measured (it may be restarting)." + } } } @@ -99,6 +113,9 @@ impl ExitCode { // A bad argument surfaced as `InvalidInput` (e.g. `dig-node open` rejecting a // non-DIG/malformed link) is a USAGE error, not a generic I/O failure. InvalidInput => ExitCode::Usage, + // The node did not answer. Nothing was measured about the requested operation, so + // this must not be reported as an I/O failure OF that operation (#407). + ConnectionRefused => ExitCode::NodeUnreachable, _ => ExitCode::IoError, } } @@ -113,6 +130,7 @@ impl ExitCode { ExitCode::ServiceFailed, ExitCode::BindFailed, ExitCode::IoError, + ExitCode::NodeUnreachable, ] } } @@ -179,6 +197,42 @@ pub fn error_envelope(action: &str, exit: ExitCode, message: &str, hint: Option< mod tests { use super::*; + /// #407 -- an unreachable node must not be reported as an I/O failure OF the request. + /// + /// The two arms are asserted TOGETHER because the property is a distinction, not an outcome: + /// a "fix" that renamed code 6, or that mapped every failure to the new code, satisfies + /// either arm alone. `Other` is the genuine-I/O-failure control and must stay `IO_ERROR`. + #[test] + fn an_unreachable_node_is_distinguished_from_a_measured_io_failure() { + let unreachable = + std::io::Error::new(std::io::ErrorKind::ConnectionRefused, "node not answering"); + let measured = std::io::Error::other("the disk gave up mid-write"); + + assert_eq!(ExitCode::from_io_error(&unreachable), ExitCode::NodeUnreachable); + assert_eq!(ExitCode::from_io_error(&measured), ExitCode::IoError); + assert_ne!( + ExitCode::from_io_error(&unreachable).code(), + ExitCode::from_io_error(&measured).code(), + "a failure to measure and a measured failure must not share an exit code" + ); + } + + /// The catalogue is the machine-readable contract (§6.2), and a code missing from it is + /// invisible to every consumer that enumerates rather than guesses. + #[test] + fn the_new_code_is_in_the_catalogue_with_a_stable_name_and_number() { + assert!(ExitCode::all().contains(&ExitCode::NodeUnreachable)); + assert_eq!(ExitCode::NodeUnreachable.code(), 7); + assert_eq!(ExitCode::NodeUnreachable.name(), "NODE_UNREACHABLE"); + // Every code's number is distinct -- an added arm that reused 6 would read as success + // against the two assertions above if either were relaxed. + let mut codes: Vec = ExitCode::all().iter().map(|c| c.code()).collect(); + codes.sort_unstable(); + let before = codes.len(); + codes.dedup(); + assert_eq!(codes.len(), before, "two exit codes share a number"); + } + #[test] fn exit_codes_are_unique_and_upper_snake() { let mut codes = std::collections::HashSet::new(); diff --git a/crates/dig-node-service/src/control_cli.rs b/crates/dig-node-service/src/control_cli.rs index 5d18ecd7..fb9a8d3a 100644 --- a/crates/dig-node-service/src/control_cli.rs +++ b/crates/dig-node-service/src/control_cli.rs @@ -400,10 +400,37 @@ impl ControlAction { /// `--json`). Transport / node errors surface as `io::Error` for the differentiated exit code. pub fn run(config: &Config, action: ControlAction) -> std::io::Result { let method = action.method(); - let result = call_control(config, method, action.wire_params())?; + let result = call_control(config, method, action.wire_params()) + .map_err(|e| explain_unreachable(method, e))?; Ok(Outcome::new(summarize(method, &result), result)) } +/// Restate an unreachable-node failure as what it is: the operation was never measured (#407). +/// +/// Only the `ConnectionRefused` class is touched, and only its MESSAGE -- the kind is preserved +/// so [`crate::cli::ExitCode::from_io_error`] still resolves it to `NODE_UNREACHABLE`. Any other +/// error passes through untouched, because a node that answered and refused has measured +/// something and its own words are the accurate ones. +/// +/// `control.updater.*` gets a sharper sentence because it has a specific, EXPECTED cause. A +/// successful update installs new bytes and cycles the service, so the pass that just succeeded +/// is itself why the node stopped answering. Reporting that as a failure is the cry-wolf case +/// the epic's silent-staged-install policy cannot afford: under a policy where nothing blocks and +/// nothing asks, the status surface is all an operator has. +fn explain_unreachable(method: &str, e: std::io::Error) -> std::io::Error { + if e.kind() != std::io::ErrorKind::ConnectionRefused { + return e; + } + let context = if method.starts_with("control.updater.") { + " — the update pass may have completed and restarted the node, which is the normal end \ + of a successful update. This did NOT observe a failed update; it observed nothing. \ + Re-run once the service is back." + } else { + " — nothing was measured about this request; it never reached the node." + }; + std::io::Error::new(e.kind(), format!("{e}{context}")) +} + /// Every `control.*` method reachable from a `dig-node` CLI verb — the union of the /// control-parity actions here and the `control.pairing.*` methods `dig-node pair` drives /// (#280). The drift test asserts this COVERS [`crate::control::CONTROL_METHODS`], so a new @@ -1495,6 +1522,56 @@ fn render_record(record: &crate::collateral::StoredRecord) -> String { #[cfg(test)] mod tests { + + /// #407 -- `dign updater check-now` probing inside the restart window reported IO_ERROR, + /// which reads to an operator exactly like an update that broke the node. It is the opposite: + /// a successful pass installs new bytes and cycles the service, so the restart is the normal + /// END of the thing that succeeded. + /// + /// The fixture varies the ERROR KIND against a fixed method, and the method against a fixed + /// kind, because the nearest wrong implementation is "treat every updater failure as a + /// restart" -- which would swallow a real decline and passes any assertion that only checks + /// the happy restart case. + #[test] + fn an_unreachable_updater_probe_says_it_measured_nothing() { + let e = std::io::Error::new(std::io::ErrorKind::ConnectionRefused, "could not reach"); + let out = explain_unreachable("control.updater.checkNow", e); + + assert_eq!( + out.kind(), + std::io::ErrorKind::ConnectionRefused, + "the kind carries the exit code and must survive" + ); + let msg = out.to_string(); + assert!(msg.contains("restarted the node"), "{msg}"); + assert!(msg.contains("observed nothing"), "{msg}"); + } + + /// The control that makes the test above load-bearing: a node that ANSWERED and declined has + /// measured something, and its own words are the accurate ones. They must pass through + /// untouched -- no restart story bolted onto a real failure. + #[test] + fn a_genuine_decline_is_not_reframed_as_a_restart() { + let e = std::io::Error::other("dig-node: dig-updater declined the request: no such channel"); + let out = explain_unreachable("control.updater.checkNow", e); + + assert_eq!(out.kind(), std::io::ErrorKind::Other); + let msg = out.to_string(); + assert!(msg.contains("declined the request"), "{msg}"); + assert!(!msg.contains("restarted the node"), "a measured failure must not be excused: {msg}"); + } + + /// A non-updater verb hitting the same unreachable node gets the general statement, not the + /// update story -- there is no update pass to attribute the silence to. + #[test] + fn a_non_updater_verb_gets_the_general_unreachable_statement() { + let e = std::io::Error::new(std::io::ErrorKind::ConnectionRefused, "could not reach"); + let out = explain_unreachable("control.cache.get", e); + + let msg = out.to_string(); + assert!(msg.contains("never reached the node"), "{msg}"); + assert!(!msg.contains("update pass"), "{msg}"); + } use super::*; use crate::control::CONTROL_METHODS; From 4c74e9338b565b930fa4fc7a083e02c62a3ad8b8 Mon Sep 17 00:00:00 2001 From: Michael Taylor Date: Mon, 31 Aug 2026 07:22:01 -0700 Subject: [PATCH 5/8] fix(control): point an unreadable control token at a scoped credential #403 -- the dead end was the ADVICE as much as the permission. The unreadable-token remedy told the operator to "reinstall the current dig-node so the service grants your account read access". On Windows that is true: the installer keeps an explicit read grant for the interactive install-user. On Ubuntu it is false -- the .deb leaves /var/lib/dig-node at 0700 root:root and the token at 0600, so reinstalling changes nothing and the operator loops. The decision, recorded in full on the ticket: do NOT widen the token. It is the MASTER capability -- it authorizes pairing administration (mint/list/revoke) and chiaPeers.add, which grants chain authority over the wallet replica. A `dig` group at 0640 hands that to every member permanently and outlives the app that motivated it. The node already has the right primitive for a client that cannot read a file: pairing, built for exactly this and used today by the MV3 extension. It yields a scoped, revocable per-client token that cannot mint or revoke pairings and cannot grant chain authority. So the Unix remedy now names `sudo dign pair` and its revocation. The platform is a function ARGUMENT rather than a cfg! branch, so both sentences are asserted on any host -- a cfg-gated assertion is untested on exactly the platform this defect was found on. #392 (the logging half) -- the file-sink degrade was RECORDED on control.status but never ANNOUNCED. A non-admin run denied C:\ProgramData\DigNetwork\logs came up console-only in silence, so an empty log directory could not be told apart from a quiet node. The console layer is installed on that path, which is why the warning reaches someone. Refs #403, #392 Co-Authored-By: Claude --- crates/dig-node-service/src/control.rs | 91 ++++++++++++++++++++++++-- crates/dig-node-service/src/logging.rs | 13 ++++ 2 files changed, 99 insertions(+), 5 deletions(-) diff --git a/crates/dig-node-service/src/control.rs b/crates/dig-node-service/src/control.rs index e0d60b63..657f8e8e 100644 --- a/crates/dig-node-service/src/control.rs +++ b/crates/dig-node-service/src/control.rs @@ -429,11 +429,9 @@ pub fn control_token_remedy_for(path: &Path) -> String { Ok(_) => format!( "the presented control token was not accepted. Ensure the node and this command resolve the SAME state dir ({dir}) — if you set DIG_NODE_STATE_DIR it must match on both the node and this command." ), - Err(e) if e.kind() == std::io::ErrorKind::PermissionDenied => format!( - "the node's control token at {} exists but is NOT readable by your account — the node runs as a service under a different account (Windows LocalSystem / a root daemon). Re-run this command elevated (Administrator on Windows, sudo on Unix), or reinstall the current dig-node so the service grants your account read access to {} (`dig-node uninstall` then an elevated `dig-node install`, then `dig-node start`).", - path.display(), - dir - ), + Err(e) if e.kind() == std::io::ErrorKind::PermissionDenied => { + permission_denied_remedy(path, &dir) + } // Absent (NotFound) — the node has not minted one here yet. Either it is not running, // or a STALE older build (installed before the machine-wide state dir) is running and // never mints the token at this path; reinstalling the current dig-node fixes the latter. @@ -444,6 +442,56 @@ pub fn control_token_remedy_for(path: &Path) -> String { } } +/// The remedy for a control token that EXISTS but this account cannot read (dig-node#403). +/// +/// The two platforms genuinely differ, and a single sentence covering both was false on one of +/// them. On Windows the installer establishes an ACL that keeps an explicit read grant for the +/// interactive install-user, so "reinstall and the service will grant your account read access" +/// is accurate there. On Unix the `.deb` creates `/var/lib/dig-node` as `0700 root:root` with the +/// token `0600 root:root`, and reinstalling changes NOTHING — an operator following that advice +/// loops through an uninstall/install cycle and arrives back at the same denial. +/// +/// # Why the fix is not to loosen the mode +/// +/// The natural server shape — node as a system service, app as a user Agent — cannot read this +/// file, and the tempting repairs both widen a privilege boundary. This token is the MASTER +/// capability: it authorizes pairing administration (mint, list, revoke) and `chiaPeers.add`, +/// which grants chain authority over the node's wallet replica. A `dig` group at `0640` hands +/// that, permanently and to every future member, in exchange for a convenience; and it outlives +/// the app that motivated it. +/// +/// The node already has the right primitive for a client that cannot read a file: PAIRING +/// ([`crate::pairing`]), built for exactly this and currently used by the MV3 extension. It +/// yields a SCOPED, REVOCABLE per-client token that cannot mint or revoke pairings and cannot +/// grant chain authority, gated on a local approval the operator performs once with the master +/// token they already hold. The principal admitted is one approved client on this host, with +/// mutation rights minus the master tier — strictly less than a group grant, and revocable +/// without touching a file mode. +fn permission_denied_remedy(path: &Path, dir: &str) -> String { + remedy_for_unreadable_token(path, dir, cfg!(unix)) +} + +/// The pure core of [`permission_denied_remedy`], with the platform as an ARGUMENT. +/// +/// A `cfg!(unix)` branch is only ever exercised on the half of the fleet that compiles it, so the +/// sentence shown to Ubuntu operators would be untested on the machine most likely to be running +/// these tests. Passing the platform in makes both branches assertable everywhere. +fn remedy_for_unreadable_token(path: &Path, dir: &str, unix: bool) -> String { + let elevated = format!( + "the node's control token at {} exists but is NOT readable by your account — the node runs as a service under a different account (Windows LocalSystem / a root daemon). Re-run this command elevated (Administrator on Windows, sudo on Unix)", + path.display() + ); + 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` approves the client's pending request, and the token it receives cannot mint or revoke pairings and cannot grant chain authority. Revoke it any time with `sudo dign pair revoke `." + ) + } else { + format!( + "{elevated}, or reinstall the current dig-node so the service grants your account read access to {dir} (`dig-node uninstall` then an elevated `dig-node install`, then `dig-node start`)." + ) + } +} + /// Read the master control token WITHOUT creating one — the OPERATOR-side load (`dig-node /// pair` / any local control CLI, #501). It must NEVER mint a token: minting a fresh token /// the running node does not trust is the exact original bug (the CLI wrote its own token to @@ -5717,6 +5765,39 @@ mod tests { /// The remedy hint names the concrete token path and, when the token is absent from /// the caller's perspective, tells them to start the node — never the old generic /// "" wording. + #[test] + /// dig-node#403 -- the unreadable-token remedy must not promise a grant the platform never + /// performs. On Unix the `.deb` leaves the token 0600 root:root and reinstalling changes + /// nothing, so the reinstall clause sent an operator round a loop that cannot succeed. + /// + /// Asserted BOTH ways rather than only on the presence of the new advice: a remedy that + /// appended the pairing sentence while keeping the false reinstall clause reads as fixed and + /// still contains the dead end. + fn the_unreadable_token_remedy_offers_a_scoped_credential_not_a_wider_file() { + let path = Path::new("/var/lib/dig-node/control-token"); + let unix = remedy_for_unreadable_token(path, "/var/lib/dig-node", true); + let windows = remedy_for_unreadable_token(path, "/var/lib/dig-node", false); + + assert!(unix.contains("elevated"), "{unix}"); + assert!(unix.contains("dign pair"), "the scoped-credential route must be named: {unix}"); + assert!(unix.contains("revoke"), "a grant with no stated revocation is a permanent one: {unix}"); + assert!( + !unix.contains("uninstall"), + "reinstalling does not grant read access on Unix; advising it is the dead end: {unix}" + ); + + // Windows genuinely DOES keep an explicit read grant for the interactive install-user, + // so its reinstall clause is accurate and must survive. This is the truthful control: a + // fix that simply deleted the clause everywhere would pass the Unix assertions alone. + assert!(windows.contains("uninstall"), "{windows}"); + + // Never, on either platform, advise widening the mode of the master capability. + for r in [&unix, &windows] { + assert!(!r.contains("chmod"), "{r}"); + assert!(!r.contains("0640"), "{r}"); + } + } + #[test] fn control_token_remedy_names_a_concrete_path() { let remedy = control_token_remedy(); diff --git a/crates/dig-node-service/src/logging.rs b/crates/dig-node-service/src/logging.rs index 75989af3..98cd944c 100644 --- a/crates/dig-node-service/src/logging.rs +++ b/crates/dig-node-service/src/logging.rs @@ -82,6 +82,19 @@ pub fn init(run_context: RunContext) { } match dig_logging::init(service(run_context)) { Ok(guard) => { + // ANNOUNCE a degrade, do not merely record it (dig-node#392). `control.status` has + // carried `file_error` since the 0.2.0 uplift, but nothing was said at the moment it + // happened -- so on a non-admin run denied `C:\ProgramData\DigNetwork\logs` the node + // came up console-only in silence, and an operator later found an empty log directory + // with no way to tell "nothing went wrong" from "logging never started". The console + // layer IS installed on this path, which is precisely why the warning reaches someone. + if let Some(reason) = guard.file_error() { + tracing::warn!( + dir = %guard.log_dir().display(), + reason = %reason, + "the rolling log FILE could not be opened; this run logs to the console ONLY. Nothing is being written to that directory -- an empty log directory here means logging was denied, not that the node was quiet." + ); + } // A `set` race (two serve paths initialising at once) is benign: the first guard // wins and stays live; a losing guard is dropped, which only detaches a writer // that was never wired into the global subscriber. From 9465272250aca8bb49e2c1082073e204eccfecd6 Mon Sep 17 00:00:00 2001 From: Michael Taylor Date: Mon, 31 Aug 2026 07:26:42 -0700 Subject: [PATCH 6/8] chore(release): 0.189.0 -> 0.190.0 Minor. New capability: the `network-info` subcommand. The exit-code catalogue gains 7 NODE_UNREACHABLE, which is additive to the table but does CHANGE the code a script sees when the node is unreachable (was 6 IO_ERROR); called out here and in the PR body so the gate can weigh it. Co-Authored-By: Claude --- Cargo.lock | 2 +- Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6f84132a..d80efae9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3031,7 +3031,7 @@ dependencies = [ [[package]] name = "dig-node-service" -version = "0.189.0" +version = "0.190.0" dependencies = [ "async-trait", "axum", diff --git a/Cargo.toml b/Cargo.toml index 08e7541a..1005fc3d 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.189.0" +version = "0.190.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 672f0a8cd4e4389f2a5af56565e002e53bd20605 Mon Sep 17 00:00:00 2001 From: Michael Taylor Date: Mon, 31 Aug 2026 07:33:37 -0700 Subject: [PATCH 7/8] style: rustfmt Co-Authored-By: Claude --- crates/dig-node-service/src/cli.rs | 5 ++++- crates/dig-node-service/src/control.rs | 10 ++++++++-- crates/dig-node-service/src/control_cli.rs | 8 ++++++-- crates/dig-node-service/src/control_client.rs | 20 ++++++++----------- crates/dig-node-service/src/entrypoint.rs | 2 +- crates/dig-node-service/src/network_info.rs | 5 ++++- 6 files changed, 31 insertions(+), 19 deletions(-) diff --git a/crates/dig-node-service/src/cli.rs b/crates/dig-node-service/src/cli.rs index 8bbef8ca..a16339ca 100644 --- a/crates/dig-node-service/src/cli.rs +++ b/crates/dig-node-service/src/cli.rs @@ -208,7 +208,10 @@ mod tests { std::io::Error::new(std::io::ErrorKind::ConnectionRefused, "node not answering"); let measured = std::io::Error::other("the disk gave up mid-write"); - assert_eq!(ExitCode::from_io_error(&unreachable), ExitCode::NodeUnreachable); + assert_eq!( + ExitCode::from_io_error(&unreachable), + ExitCode::NodeUnreachable + ); assert_eq!(ExitCode::from_io_error(&measured), ExitCode::IoError); assert_ne!( ExitCode::from_io_error(&unreachable).code(), diff --git a/crates/dig-node-service/src/control.rs b/crates/dig-node-service/src/control.rs index 657f8e8e..a6d6b76d 100644 --- a/crates/dig-node-service/src/control.rs +++ b/crates/dig-node-service/src/control.rs @@ -5779,8 +5779,14 @@ mod tests { let windows = remedy_for_unreadable_token(path, "/var/lib/dig-node", false); assert!(unix.contains("elevated"), "{unix}"); - assert!(unix.contains("dign pair"), "the scoped-credential route must be named: {unix}"); - assert!(unix.contains("revoke"), "a grant with no stated revocation is a permanent one: {unix}"); + assert!( + unix.contains("dign pair"), + "the scoped-credential route must be named: {unix}" + ); + assert!( + unix.contains("revoke"), + "a grant with no stated revocation is a permanent one: {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_cli.rs b/crates/dig-node-service/src/control_cli.rs index fb9a8d3a..3e3ddb41 100644 --- a/crates/dig-node-service/src/control_cli.rs +++ b/crates/dig-node-service/src/control_cli.rs @@ -1552,13 +1552,17 @@ mod tests { /// untouched -- no restart story bolted onto a real failure. #[test] fn a_genuine_decline_is_not_reframed_as_a_restart() { - let e = std::io::Error::other("dig-node: dig-updater declined the request: no such channel"); + let e = + std::io::Error::other("dig-node: dig-updater declined the request: no such channel"); let out = explain_unreachable("control.updater.checkNow", e); assert_eq!(out.kind(), std::io::ErrorKind::Other); let msg = out.to_string(); assert!(msg.contains("declined the request"), "{msg}"); - assert!(!msg.contains("restarted the node"), "a measured failure must not be excused: {msg}"); + assert!( + !msg.contains("restarted the node"), + "a measured failure must not be excused: {msg}" + ); } /// A non-updater verb hitting the same unreachable node gets the general statement, not the diff --git a/crates/dig-node-service/src/control_client.rs b/crates/dig-node-service/src/control_client.rs index 27bec008..c107a142 100644 --- a/crates/dig-node-service/src/control_client.rs +++ b/crates/dig-node-service/src/control_client.rs @@ -83,19 +83,15 @@ async fn call_async( if let Some(token) = token { req = req.header(control::CONTROL_TOKEN_HEADER, token); } - let resp = req - .json(&body) - .send() - .await - .map_err(|e| { - std::io::Error::new( - std::io::ErrorKind::ConnectionRefused, - format!( - "could not reach the dig-node at {url}: {e} — is it running? \ + let resp = req.json(&body).send().await.map_err(|e| { + std::io::Error::new( + std::io::ErrorKind::ConnectionRefused, + format!( + "could not reach the dig-node at {url}: {e} — is it running? \ Start it with `dig-node run` (or `dig-node start` for the service)." - ), - ) - })?; + ), + ) + })?; let v: Value = resp.json().await.map_err(std::io::Error::other)?; if let Some(err) = v.get("error") { let msg = err diff --git a/crates/dig-node-service/src/entrypoint.rs b/crates/dig-node-service/src/entrypoint.rs index 223619ac..a8bf016a 100644 --- a/crates/dig-node-service/src/entrypoint.rs +++ b/crates/dig-node-service/src/entrypoint.rs @@ -36,9 +36,9 @@ use clap::{CommandFactory, FromArgMatches, Parser, Subcommand}; use crate::cli::{error_envelope, success_envelope, ExitCode, Outcome}; use crate::config::Config; use crate::control_cli::{self, ControlAction}; +use crate::network_info; use crate::open; use crate::pair::{self, PairAction}; -use crate::network_info; use crate::peers::{self, BanState, PeersAction}; use crate::service::ScopeChoice; use crate::{serve, service, VERSION}; diff --git a/crates/dig-node-service/src/network_info.rs b/crates/dig-node-service/src/network_info.rs index 226df99d..384be8b7 100644 --- a/crates/dig-node-service/src/network_info.rs +++ b/crates/dig-node-service/src/network_info.rs @@ -126,6 +126,9 @@ mod tests { fn an_absent_field_prints_unknown_rather_than_a_plausible_default() { let s = format_network_info(&json!({ "peer_id": "cc33" })); assert!(s.contains("unknown"), "{s}"); - assert!(!s.contains("direct"), "a missing reachability must not read as direct: {s}"); + assert!( + !s.contains("direct"), + "a missing reachability must not read as direct: {s}" + ); } } From a4e436292d96dcafa983c62a187215919bd81844 Mon Sep 17 00:00:00 2001 From: Michael Taylor Date: Mon, 31 Aug 2026 08:26:06 -0700 Subject: [PATCH 8/8] fix(cli): correct the pairing verb, free the exit code, and test the degrade Six gate findings on #458, each a claim the branch made that its own tests could not see. The Unix token remedy named `sudo dign pair` as the approving verb. It is not: `entrypoint.rs` maps a bare `pair` to `PairAction::List`, so the sentence added to break a reinstall loop was itself a dead end one step further along. Its test asserted `contains("dign pair")`, which the wrong string satisfies as its own substring. The remedy now names `sudo dign pair approve `, the test asserts that command in full plus a one-sided row against the false attribution, and a new parser test pins the bare/approve mapping so the phrase is measured against the parser rather than against itself. `NODE_UNREACHABLE` took exit 7 by reading this CLI's own table, where 7 was genuinely next. dig-app's `diga` holds 7 = NOT_CONNECTED and says in its own doc that it shares dig-node's numbering; 8-11 are taken too. This is the reasoning that already cost a yank in the JSON-RPC error space. Renumbered to 12, the first number free ecosystem-wide, with the measured occupancy table written into SPEC.md and a test that fails if any shared number carries different meanings. SPEC's I/O-error mapping claimed "anything else -> 6" while omitting InvalidInput -> 2 and the new arm; both are now listed. `network-info` reached no doc at all and is token-free by design, contradicting 8.6's rule, so it gets 8.8 stating that exception and a README section. The logging change had no tests and shipped two runs of ~22 literal spaces from a lost string continuation. The decision is extracted as a pure function and the text as a constant; four tests cover announce, stay-silent, the ambiguity the message must resolve, and the whitespace. The whitespace row immediately earned itself: the first repair used a backslash continuation and `cargo fmt` rejoined it and materialised the indentation straight back into the string. `concat!` has no whitespace a formatter can reinterpret. The #317 marker surviving upgrades was reasoned in a comment and never exercised. The stub-systemctl harness now runs a marked upgrade and asserts both directions: nothing unconditional starts, and try-restart still cycles a running unit. Co-Authored-By: Claude --- README.md | 11 ++- SPEC.md | 59 ++++++++++++++-- USER_JOURNEY.md | 2 +- crates/dig-node-service/src/cli.rs | 49 +++++++++++-- crates/dig-node-service/src/control.rs | 18 ++++- crates/dig-node-service/src/entrypoint.rs | 35 +++++++++ crates/dig-node-service/src/logging.rs | 86 ++++++++++++++++++++++- scripts/tests/deb-contents.test.sh | 39 ++++++++++ 8 files changed, 284 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index 3793cd3c..00880387 100644 --- a/README.md +++ b/README.md @@ -301,6 +301,15 @@ CORS reflects `chrome-extension://` and the local page origins `http://localhost / `http://127.0.0.1` / `http://127.0.0.2` / `http://[::1]` (with or without a `:port`), so the extension and any page served from a canonical local name can call it. +### `dign network-info` — this node's own posture, from the terminal + +`dign network-info` prints the node's peer id, network id, effective L2 genesis, listen address, +reachability, and the addresses it advertises, in the node's own IPv6-first order. It reads the +open `dig.getNetworkInfo` surface, so it needs **no control token and no elevation** — on a `.deb` +install the control token is `0600 root:root`, and asking "what is my node's address" should not +require `sudo` for a read the node already performs for any peer that dials it. A field the node +did not report prints as `unknown` rather than as a plausible default. + ## Machine-readable contracts (agent-friendly) ### CLI `--json` @@ -332,7 +341,7 @@ typed `ExitCode` enum in `src/cli.rs`: | 4 | `SERVICE_FAILED` | A service operation failed (register/start/stop/uninstall). | | 5 | `BIND_FAILED` | `run`: could not bind the loopback address. | | 6 | `IO_ERROR` | Other I/O error. | -| 7 | `NODE_UNREACHABLE` | The node did not answer; the operation was not measured. | +| 12 | `NODE_UNREACHABLE` | The node did not answer; the operation was not measured. | ### JSON-RPC error-code catalogue diff --git a/SPEC.md b/SPEC.md index 34219294..d1962685 100644 --- a/SPEC.md +++ b/SPEC.md @@ -2401,12 +2401,29 @@ flood of never-attached `begin`s cannot grow engine state without bound. `install` · `uninstall` · `start` · `stop` (each accepting `--scope `, §9.1) · `status` · `pair` (§7.11) · `open` (§8.5) · the **control-parity** subcommands `info` · `config` · `cache` · `stores` · `sync` · `updater` · -`subscriptions` (§8.6) · `peers` (§8.7) · `logs` (§11). +`subscriptions` (§8.6) · `peers` (§8.7) · `network-info` (§8.8) · `logs` (§11). The `dign` alias binary (§2.1a) exposes this SAME subcommand set with the SAME semantics — `dign ` is equivalent to `dig-node ` in every respect except the reported program name. +### 8.8. `network-info` — this node's own network posture (#303) + +`network-info` prints this node's `peer_id`, network id, effective L2 genesis, listen address, +reachability, and its advertised candidate addresses in the node's own advertisement order, which +is IPv6-first (§5.2). The order MUST be passed through untouched: re-sorting would hide a node +whose IPv6 advertisement is missing, which is the fault an operator runs the command to find. An +absent field MUST render as `unknown` and MUST NOT be filled with a plausible default — a +fabricated `direct` or an invented `0.0.0.0` reads exactly like a measurement. + +**It reads the OPEN surface and is NOT token-gated, deliberately.** It is the one documented +exception to §8.6's rule that a CLI subcommand presents the master control token: it calls +`dig.getNetworkInfo`, whose body this node already hands any peer that dials it, so a loopback +caller learns nothing a stranger does not. Gating it would buy no confidentiality while costing +real availability — on a `.deb` install the control token is `0600 root:root` (§7.11/#501), so an +ordinary user asking "what is my node's address" would be told to elevate for a read the network +performs for free. This is a property to PRESERVE, not an oversight to tighten later. + ### 8.6. Control-parity subcommands (#426) For EVERY gated `control.*` method the DIG Chrome extension drives (§7), the CLI exposes an @@ -2415,7 +2432,9 @@ extension drives it from a browser. Each subcommand is a THIN dispatch — it ca `control.*` method over the node's loopback endpoint, presenting the MASTER control token (`X-Dig-Control-Token`, read WITHOUT minting — §7.11/#501); no CLI logic is forked from the control plane. A mutating CLI control is therefore gated by the identical capability as the WS surface (the -on-disk master token = local-machine control), never an unauthenticated backdoor. +on-disk master token = local-machine control), never an unauthenticated backdoor. The one +documented exception is `network-info` (§8.8), which reads an OPEN, already-public surface and is +token-free by design; it is not a control-parity subcommand and this rule does not reach it. - `info` → `control.status` — the rich node status (version, uptime, cache, hosted-store + cached-capsule counts, §21 sync availability). DISTINCT from `status` (§8.3), which is an @@ -2620,11 +2639,43 @@ exit `1` (`NOT_SERVING`) so scripts can gate on liveness; the JSON result carrie | 4 | `SERVICE_FAILED` | A service-manager operation failed. | | 5 | `BIND_FAILED` | `run`: could not bind the loopback address. | | 6 | `IO_ERROR` | Other I/O error. | -| 7 | `NODE_UNREACHABLE` | The node did not answer; the operation was not measured. | +| 12 | `NODE_UNREACHABLE` | The node did not answer; the operation was not measured. | + +I/O-error mapping, in the order `ExitCode::from_io_error` matches — every arm, because a partial +list reads as complete and the omitted arms are exactly the ones a caller gets wrong: +`PermissionDenied` → 3; `AddrInUse`/`AddrNotAvailable` → 5; `InvalidInput` → 2 (a bad argument +surfaced as an I/O error is still a usage error); `ConnectionRefused` → 12; anything else → 6. -I/O-error mapping: `PermissionDenied` → 3; `AddrInUse`/`AddrNotAvailable` → 5; anything else → 6. Numeric values and symbolic names are a stable contract and MUST NOT be renumbered. +**The occupied numbers span the whole DIG command line, not this CLI alone (MUST).** `dign` and +dig-app's `diga` deliberately share one numbering so a caller sees one surface across both +(`dig-app-core/src/gateway/outcome.rs`), so a code is available only if it is unoccupied +ECOSYSTEM-WIDE. Absence from the table above does NOT make a number free — this repo has already +paid for that reasoning once in the JSON-RPC error space, where `-32015` was taken as "the next +free code" from the owning crate's own list and collided with a released `METADATA_TOO_LARGE`, +forcing a yank. The full occupied set, measured, is: + +| Code | `dign` (this CLI) | `diga` (dig-app gateway) | +|---|---|---| +| 0 | `OK` | `OK` | +| 1 | `NOT_SERVING` | — | +| 2 | `USAGE` | `USAGE` | +| 3 | `PERMISSION_DENIED` | — | +| 4 | `SERVICE_FAILED` | — | +| 5 | `BIND_FAILED` | — | +| 6 | `IO_ERROR` | `IO_ERROR` | +| 7 | — | `NOT_CONNECTED` | +| 8 | — | `ENGINE_ERROR` | +| 9 | — | `LOCKED` | +| 10 | — | `NOT_FOUND` | +| 11 | — | `DENIED` | +| 12 | `NODE_UNREACHABLE` | — | + +0–11 were therefore taken before this CLI added a code, and 12 is the first free number. A new +code MUST be drawn from 13 upward and MUST re-check BOTH tables first; 126, 127 and 128+n are +reserved by the shell and MUST NOT be used. + --- ## 9. OS-service contract diff --git a/USER_JOURNEY.md b/USER_JOURNEY.md index 471652ed..4bfd4b67 100644 --- a/USER_JOURNEY.md +++ b/USER_JOURNEY.md @@ -163,7 +163,7 @@ every subcommand (machine output to stdout, prose to stderr). - **`--json`** on the CLI: success → `{ ok:true, action, service, version, …result }`; failure → `{ ok:false, error:{ code, exit_code, message, hint } }`. - **Exit-code table** (documented in the README + `src/cli.rs`): `0 OK`, `1 NOT_SERVING`, - `2 USAGE`, `3 PERMISSION_DENIED`, `4 SERVICE_FAILED`, `5 BIND_FAILED`, `6 IO_ERROR`, `7 NODE_UNREACHABLE`. + `2 USAGE`, `3 PERMISSION_DENIED`, `4 SERVICE_FAILED`, `5 BIND_FAILED`, `6 IO_ERROR`, `12 NODE_UNREACHABLE`. - **Stable JSON-RPC error codes** (UPPER_SNAKE in `error.data.code`): `PARSE_ERROR` (-32700), `INVALID_REQUEST` (-32600), `METHOD_NOT_FOUND` (-32601), `INVALID_PARAMS` (-32602), `DISPATCH_FAILED` (-32000, shell), `UPSTREAM_ERROR` (-32010, shell), and the control-plane codes diff --git a/crates/dig-node-service/src/cli.rs b/crates/dig-node-service/src/cli.rs index a16339ca..e0421baf 100644 --- a/crates/dig-node-service/src/cli.rs +++ b/crates/dig-node-service/src/cli.rs @@ -19,7 +19,7 @@ //! | 4 | SERVICE_FAILED | A service operation failed (register/start/stop).| //! | 5 | BIND_FAILED | `run`: could not bind the loopback address. | //! | 6 | IO_ERROR | Other I/O error. | -//! | 7 | NODE_UNREACHABLE | The node did not answer; nothing was measured. | +//! | 12 | NODE_UNREACHABLE | The node did not answer; nothing was measured. | use serde_json::{json, Value}; @@ -44,7 +44,7 @@ pub enum ExitCode { BindFailed, /// 6 — any other I/O error. IoError, - /// 7 — the node did not answer, so the operation was never measured (dig-node#407). + /// 12 — the node did not answer, so the operation was never measured (dig-node#407). /// /// Deliberately NOT `IO_ERROR`: that code says an I/O operation was attempted and failed, /// which is a claim about the requested operation. An unreachable node is a failure to @@ -65,7 +65,7 @@ impl ExitCode { ExitCode::ServiceFailed => 4, ExitCode::BindFailed => 5, ExitCode::IoError => 6, - ExitCode::NodeUnreachable => 7, + ExitCode::NodeUnreachable => 12, } } @@ -220,12 +220,53 @@ mod tests { ); } + /// **No `dign` code may collide with a `diga` code that means something else.** + /// + /// The two command lines deliberately share ONE numbering so a caller sees one surface + /// (`dig-app-core/src/gateway/outcome.rs` says so in its own doc comment), which means a + /// number is free only if it is unoccupied ECOSYSTEM-WIDE. `NODE_UNREACHABLE` was first + /// assigned 7 by reading this file's own table, where 7 genuinely was the next number -- + /// and 7 is `NOT_CONNECTED` on the other side. The identical reasoning already cost a yank + /// in the JSON-RPC error space (`-32015` vs a released `METADATA_TOO_LARGE`), so the guard + /// is a test rather than a note. + /// + /// The `diga` map is transcribed rather than imported: dig-node MUST NOT take a dependency + /// on dig-app (it is the engine, not a consumer of its own client). That makes this fixture + /// the drift risk, so it names the file it was read from and the SPEC carries the same table. + #[test] + fn no_exit_code_collides_with_the_dig_app_gateway_numbering() { + // Read from modules/apps/dig-app/crates/dig-app-core/src/gateway/outcome.rs. + const DIGA: &[(u8, &str)] = &[ + (0, "OK"), + (2, "USAGE"), + (6, "IO_ERROR"), + (7, "NOT_CONNECTED"), + (8, "ENGINE_ERROR"), + (9, "LOCKED"), + (10, "NOT_FOUND"), + (11, "DENIED"), + ]; + + for code in ExitCode::all() { + if let Some((_, diga_name)) = DIGA.iter().find(|(n, _)| *n == code.code()) { + assert_eq!( + code.name(), + *diga_name, + "exit {} is `{}` here and `{}` in the dig-app gateway -- a shared number must carry the SAME meaning on both command lines, or a caller branching on it is reading two different failures as one", + code.code(), + code.name(), + diga_name + ); + } + } + } + /// The catalogue is the machine-readable contract (§6.2), and a code missing from it is /// invisible to every consumer that enumerates rather than guesses. #[test] fn the_new_code_is_in_the_catalogue_with_a_stable_name_and_number() { assert!(ExitCode::all().contains(&ExitCode::NodeUnreachable)); - assert_eq!(ExitCode::NodeUnreachable.code(), 7); + assert_eq!(ExitCode::NodeUnreachable.code(), 12); assert_eq!(ExitCode::NodeUnreachable.name(), "NODE_UNREACHABLE"); // Every code's number is distinct -- an added arm that reused 6 would read as success // against the two assertions above if either were relaxed. diff --git a/crates/dig-node-service/src/control.rs b/crates/dig-node-service/src/control.rs index a6d6b76d..95c673df 100644 --- a/crates/dig-node-service/src/control.rs +++ b/crates/dig-node-service/src/control.rs @@ -483,7 +483,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` approves the client's pending request, and the token it receives cannot mint or revoke pairings and cannot grant chain authority. Revoke it any time with `sudo dign pair revoke `." + "{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 `." ) } else { format!( @@ -5779,9 +5779,21 @@ mod tests { let windows = remedy_for_unreadable_token(path, "/var/lib/dig-node", false); assert!(unix.contains("elevated"), "{unix}"); + // The APPROVING verb, in full. `contains("dign pair")` is satisfied by every WRONG + // string as its own substring -- including the one this row was rewritten to catch, + // which claimed a bare `sudo dign pair` "approves the client's pending request" while + // `entrypoint.rs` maps a bare `pair` to `PairAction::List`. A remedy that names a verb + // which does not approve is the same dead end the reinstall clause was. assert!( - unix.contains("dign pair"), - "the scoped-credential route must be named: {unix}" + unix.contains("`sudo dign pair approve `"), + "the remedy must name the verb that actually approves: {unix}" + ); + // Fails if the verb reverts. The row above passes on any string containing the correct + // command, including one that ALSO reasserts the false claim beside it; this one is + // one-sided against the specific wrong attribution, so the two cannot both be vacuous. + assert!( + !unix.contains("`sudo dign pair` approves"), + "the bare verb only lists; attributing approval to it is the dead end: {unix}" ); assert!( unix.contains("revoke"), diff --git a/crates/dig-node-service/src/entrypoint.rs b/crates/dig-node-service/src/entrypoint.rs index a8bf016a..5a25e791 100644 --- a/crates/dig-node-service/src/entrypoint.rs +++ b/crates/dig-node-service/src/entrypoint.rs @@ -1418,6 +1418,41 @@ fn run_service(config: Config) -> std::io::Result<()> { mod tests { use super::*; + /// **The bare `pair` verb LISTS; only `pair approve ` approves.** + /// + /// This exists because a remedy string in `control.rs` told an operator that `sudo dign pair` + /// "approves the client's pending request". It does not, and nothing could see the mistake: + /// the remedy's own test asserted `contains("dign pair")`, which the wrong sentence satisfies + /// as its own substring. Pinning the mapping HERE means the claim is measured against the + /// parser rather than against a phrase, so the two cannot drift apart silently. + #[test] + fn the_bare_pair_verb_lists_and_only_approve_approves() { + let pair_action = |argv: &[&str]| match Cli::try_parse_from(argv) + .expect("the verb parses") + .command + .expect("a subcommand was given") + { + Command::Pair { action } => match action { + None | Some(PairCommand::List) => PairAction::List, + Some(PairCommand::Approve { pairing_id }) => PairAction::Approve { pairing_id }, + Some(PairCommand::Revoke { token_id }) => PairAction::Revoke { token_id }, + }, + _ => panic!("expected a pair command from {argv:?}"), + }; + + assert!( + matches!(pair_action(&["dig-node", "pair"]), PairAction::List), + "a bare `pair` must remain a LIST -- the remedy text depends on it" + ); + assert!( + matches!( + pair_action(&["dig-node", "pair", "approve", "abc123"]), + PairAction::Approve { ref pairing_id } if pairing_id == "abc123" + ), + "`pair approve ` must approve, and must carry the id through" + ); + } + /// **`dign mirror bond-states --after` sends the cursor to the node.** /// /// Asserted on the WIRE params rather than on the selected method, because a parser that diff --git a/crates/dig-node-service/src/logging.rs b/crates/dig-node-service/src/logging.rs index 98cd944c..f4420440 100644 --- a/crates/dig-node-service/src/logging.rs +++ b/crates/dig-node-service/src/logging.rs @@ -60,6 +60,36 @@ pub fn run_context() -> RunContext { } } +/// The sentence printed when the rolling log FILE could not be opened and this run is +/// console-only. +/// +/// A named constant rather than an inline literal for two reasons. It is the only part of the +/// degrade path a test can hold onto -- `init` installs a global subscriber, so the emission +/// itself is not assertable in-process. And it was shipped carrying two runs of ~22 literal +/// spaces from a lost string continuation, which no test could see and which renders in an +/// operator's log as a line that looks corrupted at the exact moment they are trying to work out +/// whether logging is broken. +/// +/// `concat!` rather than a `\`-continued literal, and that is not a style preference: the first +/// repair here DID use continuations, and `cargo fmt` rejoined them and materialised the leading +/// indentation back into the string -- reintroducing the very defect, silently, between writing +/// the fix and running it. `the_announcement_text_has_no_lost_string_continuation` caught it. +/// `concat!` has no whitespace a formatter can reinterpret. +pub const FILE_LOGGING_DEGRADED: &str = concat!( + "the rolling log FILE could not be opened; this run logs to the console ONLY. ", + "Nothing is being written to that directory -- an empty log directory here means ", + "logging was denied, not that the node was quiet." +); + +/// Whether a run must ANNOUNCE that file logging degraded, given the sink's error (if any). +/// +/// Pure, and separated from [`init`] for the same reason `remedy_for_unreadable_token` takes its +/// platform as an argument: the branch that matters runs on the half of the fleet where the log +/// directory is denied, which is not the machine the tests run on. +pub fn degrade_announcement(file_error: Option<&str>) -> Option<&str> { + file_error +} + /// Install the shared logging stack for a SERVE run (SPEC §1) and hold the guard for the /// process lifetime. Idempotent + best-effort: a second call (e.g. a test that serves twice /// in one process) is a silent no-op. @@ -88,11 +118,11 @@ pub fn init(run_context: RunContext) { // came up console-only in silence, and an operator later found an empty log directory // with no way to tell "nothing went wrong" from "logging never started". The console // layer IS installed on this path, which is precisely why the warning reaches someone. - if let Some(reason) = guard.file_error() { + if let Some(reason) = degrade_announcement(guard.file_error()) { tracing::warn!( dir = %guard.log_dir().display(), reason = %reason, - "the rolling log FILE could not be opened; this run logs to the console ONLY. Nothing is being written to that directory -- an empty log directory here means logging was denied, not that the node was quiet." + "{FILE_LOGGING_DEGRADED}" ); } // A `set` race (two serve paths initialising at once) is benign: the first guard @@ -197,6 +227,58 @@ mod tests { assert!(set_level("debug").is_err()); } + /// **A live file sink announces NOTHING.** The control, and the row that makes the next one + /// load-bearing: an implementation that warned unconditionally would satisfy the announce + /// assertion below while shouting on every healthy start. + #[test] + fn a_live_file_sink_announces_no_degrade() { + assert_eq!(degrade_announcement(None), None); + } + + /// **A failed file sink announces, and carries the sink's own reason through.** + /// + /// The reason is asserted by VALUE, not merely by presence: a warning that says logging + /// degraded without saying why sends an operator to guess between a denied directory, a full + /// disk and a bad path -- three different remedies. + #[test] + fn a_failed_file_sink_announces_and_names_the_reason() { + assert_eq!( + degrade_announcement(Some("Access is denied. (os error 5)")), + Some("Access is denied. (os error 5)") + ); + } + + /// **The announcement text carries no run of collapsed whitespace.** + /// + /// This is not style. The message shipped with two runs of ~22 literal spaces, left behind + /// when a Rust string continuation lost its trailing backslash -- the compiler is perfectly + /// happy, every other test stays green, and the only witness is an operator reading a line + /// that looks corrupted at the moment they are trying to establish whether logging works. + #[test] + fn the_announcement_text_has_no_lost_string_continuation() { + assert!( + !FILE_LOGGING_DEGRADED.contains(" "), + "a run of consecutive spaces means a continuation lost its backslash: {FILE_LOGGING_DEGRADED:?}" + ); + } + + /// **The announcement distinguishes "denied" from "quiet".** + /// + /// The whole point of #392: an empty log directory is ambiguous, and the warning exists to + /// resolve the ambiguity rather than to record that something happened. A message that said + /// only "file logging failed" would pass a presence check and leave the ambiguity intact. + #[test] + fn the_announcement_says_an_empty_directory_means_denied_not_quiet() { + assert!( + FILE_LOGGING_DEGRADED.contains("console ONLY"), + "{FILE_LOGGING_DEGRADED}" + ); + assert!( + FILE_LOGGING_DEGRADED.contains("logging was denied, not that the node was quiet"), + "the message must resolve the empty-directory ambiguity: {FILE_LOGGING_DEGRADED}" + ); + } + #[test] fn health_reports_file_logging_off_and_names_the_reason() { // The degraded case the 0.2.0 uplift exists for: the subscriber IS installed (console diff --git a/scripts/tests/deb-contents.test.sh b/scripts/tests/deb-contents.test.sh index b380aadb..730cba9b 100644 --- a/scripts/tests/deb-contents.test.sh +++ b/scripts/tests/deb-contents.test.sh @@ -183,6 +183,45 @@ else ok "the upgrade restart is conditional on the unit already running (#305)" fi +# The #317 marker must survive EVERY future upgrade, not just the install that created it. The +# postinst comment reasons that `try-restart` gives this for free -- it cycles a unit only if it +# is already running, so a node held back by the marker stays stopped -- but reasoning is not a +# test, and the marker+upgrade combination was never exercised by either section above: section 2 +# runs the marker on a FIRST install, and the upgrade run above carries no marker. +# +# What is asserted is what the stub can see: the argv the postinst emits. `try-restart`'s runtime +# conditionality belongs to systemd and cannot be observed here, so the load-bearing claim is that +# an upgrade under the marker emits NO unconditional starter -- a `start`, a `restart`, or an +# `enable --now` would each start a node the operator deliberately stopped, and each is a change +# a later simplification could plausibly make. +MARKED_UPGRADE_ROOT="$TMP/marked-upgrade"; mkdir -p "$MARKED_UPGRADE_ROOT/etc/dig-node" +touch "$MARKED_UPGRADE_ROOT/etc/dig-node/no-autostart" +MARKED_UPGRADE_LOG="$(run_postinst "$MARKED_UPGRADE_ROOT" "0.1.0")" +if grep -qE 'systemctl (start|restart|reload-or-restart|enable) ' <<<"$MARKED_UPGRADE_LOG"; then + fail "an upgrade under the no-autostart marker starts the node the operator stopped (#317/#305)" +else + ok "an upgrade under the no-autostart marker starts nothing (#317 survives upgrades)" +fi + +# ...and it still CYCLES a running unit. The #305 fix must not be lost under the marker: an +# operator who removed the marker and started the node by hand is running a node like any other, +# and the next upgrade must replace its binary. This row and the one above are one-sided in +# opposite directions, so neither can be satisfied by the postinst simply doing nothing. +if grep -qE 'systemctl try-restart net\.dignetwork\.dig-node\.service' <<<"$MARKED_UPGRADE_LOG"; then + ok "the marked upgrade still cycles an already-running unit (#305 preserved under #317)" +else + fail "the marker suppressed try-restart -- an upgrade would leave the old binary serving (#305)" +fi + +# The truthful control for the row above. Its assertion is a NEGATIVE, which an empty log would +# satisfy for the wrong reason -- a postinst that did nothing at all under the marker would look +# identical. `daemon-reload` proves the configure branch really ran. +if grep -q 'daemon-reload' <<<"$MARKED_UPGRADE_LOG"; then + ok "the marked upgrade really ran its configure branch (so the check above measured something)" +else + fail "the marked upgrade emitted no daemon-reload -- the no-start check above proves nothing" +fi + # The control. A FIRST install already starts the node via `enable --now`, so a restart there # would be redundant; its ABSENCE is what proves the assertion above measured the upgrade # argument rather than a restart bolted onto every configure.