fix(control): pin control-client to direct loopback + CONTROL_METHODS/dispatch lockstep guard (#711)

Cargo.toml

@@ -31,7 +31,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.38.1"
+version = "0.38.2"
 
 # Release hardening, matching digstore: keep integer-overflow checks ON in release.
 # The node parses untrusted serialized input and does offset/length arithmetic over

SPEC.md

@@ -894,6 +894,11 @@ token is NOT accepted for them.
 - **The daemon MAY create the token (write); an operator CLI MUST NOT mint one.** The CLI
   (`dig-node pair` / any control tool) reads the token READ-ONLY; if it is missing or unreadable it
   MUST fail with a precise remedy (§7.3a) rather than write a fresh token the daemon does not trust.
+- **The operator control client MUST connect DIRECT to loopback, ignoring proxy environment.** The
+  HTTP client that carries the master control token to the node's loopback address MUST be pinned to
+  a direct connection (`no_proxy`), so an `HTTP_PROXY`/`HTTPS_PROXY` in the operator's environment
+  can NEVER route the token-bearing `control.*` POST through an interposed proxy. (The default HTTP
+  proxy behaviour has no automatic loopback bypass.)
 - **Trust a pre-existing token file ONLY when it is owned by a TRUSTED principal (owner
   verification).** Before the daemon loads and trusts the bytes of an EXISTING `control-token`, it
   MUST verify the file's OWNER; a foreign-owned token is DELETED and REGENERATED, never returned.

crates/dig-node-service/src/control.rs

@@ -117,6 +117,47 @@ pub const CONTROL_METHODS: &[&str] = &[
     "control.listSubscriptions",
 ];
 
+/// The control methods this shell HANDLES ITSELF, in [`dispatch_control`]'s owned arms — the
+/// ROUTING source of truth: [`dispatch_control`] delegates any method NOT in this set to the
+/// embedded node. Adding an owned `match` arm without listing it here leaves that arm
+/// unreachable (silently delegated); the lockstep test
+/// (`control_methods_partition_into_owned_and_delegated`) forces this set + [`CONTROL_METHODS`]
+/// to agree, so a shell-owned method can never be dispatched without also being declared.
+pub const OWNED_CONTROL_METHODS: &[&str] = &[
+    "control.status",
+    "control.config.get",
+    "control.config.setUpstream",
+    "control.log.setLevel",
+    "control.cache.get",
+    "control.cache.setCap",
+    "control.cache.clear",
+    "control.hostedStores.list",
+    "control.hostedStores.pin",
+    "control.hostedStores.unpin",
+    "control.hostedStores.status",
+    "control.sync.status",
+    "control.sync.trigger",
+    "control.updater.status",
+    "control.updater.setChannel",
+    "control.updater.pause",
+    "control.updater.resume",
+    "control.updater.checkNow",
+    "control.pairing.list",
+    "control.pairing.approve",
+    "control.pairing.revoke",
+];
+
+/// The control methods [`dispatch_control`] DELEGATES to the embedded node's own control surface
+/// (`dig_node_core::handle_rpc`) — the node-internal subscription set + peer-status snapshot.
+/// Together with [`OWNED_CONTROL_METHODS`] this partitions [`CONTROL_METHODS`] exactly (asserted
+/// by the lockstep test): the two disjoint sets union to the full control surface.
+pub const DELEGATED_CONTROL_METHODS: &[&str] = &[
+    "control.peerStatus",
+    "control.subscribe",
+    "control.unsubscribe",
+    "control.listSubscriptions",
+];
+
 /// Is this a PAIRING-ADMINISTRATION control method (#280)? PURE.
 ///
 /// These manage the pairing lifecycle — list pending requests, approve one (minting
@@ -602,6 +643,27 @@ pub struct ControlCtx {
 /// auth gate ([`is_authorized`]); this performs the operation and returns the
 /// JSON-RPC response Value. Unknown `control.*` methods → METHOD_NOT_FOUND.
 pub async fn dispatch_control(ctx: &ControlCtx, id: Value, method: &str, params: &Value) -> Value {
+    // Route by the SINGLE source of truth: methods this shell owns go to `dispatch_owned`;
+    // everything else (the delegated set + any genuinely-unknown `control.*`) falls through to
+    // the embedded node's own control surface, which resolves it or returns -32601.
+    if OWNED_CONTROL_METHODS.contains(&method) {
+        return dispatch_owned(ctx, id, method, params).await;
+    }
+    // Control methods the shell does not own are delegated to the NODE's own control surface
+    // (`control.peerStatus` / `control.subscribe` / `control.unsubscribe` /
+    // `control.listSubscriptions` — the node's persisted subscription set + peer-status
+    // snapshot). The shell forwards them so the whole control surface is reachable through one
+    // loopback endpoint. A genuinely unknown control method falls through the node too and
+    // returns -32601.
+    let req = json!({ "jsonrpc": "2.0", "id": id, "method": method, "params": params });
+    dig_node_core::handle_rpc(&ctx.node, req).await
+}
+
+/// Handle a control method OWNED by this shell (guaranteed by [`dispatch_control`] to be a member
+/// of [`OWNED_CONTROL_METHODS`]). The `_` arm is [`unreachable`] BY CONSTRUCTION: it fires only if
+/// [`OWNED_CONTROL_METHODS`] lists a method with no arm here (or vice-versa), i.e. the routing
+/// const and the arms drifted — the lockstep test exercises this correspondence.
+async fn dispatch_owned(ctx: &ControlCtx, id: Value, method: &str, params: &Value) -> Value {
     match method {
         "control.status" => control_ok(id, status(ctx).await),
         "control.config.get" => control_ok(id, config_get(ctx)),
@@ -634,17 +696,12 @@ pub async fn dispatch_control(ctx: &ControlCtx, id: Value, method: &str, params:
             crate::pairing::approve(&ctx.pairings, &ctx.state_dir, id, params)
         }
         "control.pairing.revoke" => crate::pairing::revoke(&ctx.state_dir, id, params),
-        // Control methods the shell does not own are delegated to the NODE's own
-        // control surface (`control.peerStatus` / `control.subscribe` /
-        // `control.unsubscribe` / `control.listSubscriptions` — the node's persisted
-        // subscription set + peer-status snapshot). These are node-internal control
-        // methods dig_node_core::handle_rpc resolves; the shell forwards them so the whole
-        // control surface is reachable through one loopback endpoint. A genuinely
-        // unknown control method falls through the node too and returns -32601.
-        _ => {
-            let req = json!({ "jsonrpc": "2.0", "id": id, "method": method, "params": params });
-            dig_node_core::handle_rpc(&ctx.node, req).await
-        }
+        // Unreachable: `dispatch_control` only routes here for `OWNED_CONTROL_METHODS` members.
+        // Reaching this arm means the routing const and these arms have drifted.
+        _ => unreachable!(
+            "dispatch_owned reached for non-owned control method {method:?}: \
+             OWNED_CONTROL_METHODS and dispatch_owned's arms have drifted"
+        ),
     }
 }
 
@@ -1064,6 +1121,53 @@ mod tests {
     use super::*;
     use serde_json::json;
 
+    /// LOCKSTEP GATE (#711): [`dispatch_control`] resolves EXACTLY [`CONTROL_METHODS`] — the
+    /// owned set it routes to `dispatch_owned` ([`OWNED_CONTROL_METHODS`]) plus the set it
+    /// delegates to the node ([`DELEGATED_CONTROL_METHODS`]) — the two disjoint, and their union
+    /// equal to the declared surface. This closes the shell-owned-method drift gap the CLI-parity
+    /// test (`cli_covers_every_node_control_method`) leaves open: a `dispatch_owned` arm added
+    /// without declaring it (in `OWNED_CONTROL_METHODS` + `CONTROL_METHODS`) fails HERE, and a
+    /// declared owned method with no arm makes `dispatch_owned`'s `unreachable!` fire.
+    #[test]
+    fn control_methods_partition_into_owned_and_delegated() {
+        use std::collections::BTreeSet;
+        let listed: BTreeSet<&str> = CONTROL_METHODS.iter().copied().collect();
+        let owned: BTreeSet<&str> = OWNED_CONTROL_METHODS.iter().copied().collect();
+        let delegated: BTreeSet<&str> = DELEGATED_CONTROL_METHODS.iter().copied().collect();
+
+        // Each list is duplicate-free.
+        assert_eq!(
+            owned.len(),
+            OWNED_CONTROL_METHODS.len(),
+            "OWNED has duplicates"
+        );
+        assert_eq!(
+            delegated.len(),
+            DELEGATED_CONTROL_METHODS.len(),
+            "DELEGATED has duplicates"
+        );
+        assert_eq!(
+            listed.len(),
+            CONTROL_METHODS.len(),
+            "CONTROL_METHODS has duplicates"
+        );
+
+        // Owned and delegated are disjoint — no method is both handled and forwarded.
+        let both: Vec<&&str> = owned.intersection(&delegated).collect();
+        assert!(
+            both.is_empty(),
+            "methods both owned AND delegated: {both:?}"
+        );
+
+        // The union is EXACTLY the declared surface — neither an undeclared handler nor a
+        // declared-but-unhandled method can slip through.
+        let union: BTreeSet<&str> = owned.union(&delegated).copied().collect();
+        assert_eq!(
+            listed, union,
+            "CONTROL_METHODS drifted from dispatch_control's owned+delegated set"
+        );
+    }
+
     #[test]
     fn is_control_method_only_matches_control_namespace() {
         assert!(is_control_method("control.status"));

crates/dig-node-service/src/control_client.rs

@@ -21,6 +21,18 @@ use serde_json::{json, Value};
 use crate::config::Config;
 use crate::control;
 
+/// Build the reqwest client for the loopback control plane, PINNED to a DIRECT connection.
+///
+/// `.no_proxy()` disables reqwest's default env-proxy behaviour (`HTTP_PROXY`/`HTTPS_PROXY`/
+/// `http_proxy`/…), which has NO automatic loopback bypass. Without it, a hostile operator-env
+/// proxy could route the token-bearing `control.*` POST — carrying the master control token — to
+/// `127.0.0.1`/`::1` through an attacker-controlled proxy. The control plane is loopback-only and
+/// token-gated (#501/#553); pinning the transport DIRECT keeps the token on the wire it was
+/// minted for and never hands it to an interposed proxy.
+fn build_control_client() -> reqwest::Result<reqwest::Client> {
+    reqwest::Client::builder().no_proxy().build()
+}
+
 /// Call one `control.*` method on the running node and return its `result` object.
 ///
 /// Reads the master control token read-only, builds a single-shot current-thread runtime,
@@ -46,9 +58,7 @@ async fn call_async(
     method: &str,
     params: Value,
 ) -> std::io::Result<Value> {
-    let client = reqwest::Client::builder()
-        .build()
-        .map_err(std::io::Error::other)?;
+    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
@@ -76,3 +86,57 @@ async fn call_async(
     }
     Ok(v.get("result").cloned().unwrap_or_else(|| json!({})))
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use std::io::{Read, Write};
+    use std::net::TcpListener;
+
+    /// SECURITY (#711): the control client MUST reach loopback DIRECTLY, ignoring a hostile
+    /// `HTTP_PROXY` in the environment — otherwise the token-bearing control POST could be routed
+    /// through an attacker-controlled proxy. We stand up a one-shot loopback HTTP server, point
+    /// `HTTP_PROXY` at a DEAD address, and assert the request still lands on the server. Without
+    /// `.no_proxy()`, reqwest would dial the dead proxy instead and the request would fail.
+    #[tokio::test]
+    async fn control_client_ignores_http_proxy_and_connects_direct() {
+        let server = TcpListener::bind("127.0.0.1:0").unwrap();
+        let addr = server.local_addr().unwrap();
+
+        // A one-shot server thread: accept a single connection, reply 200, signal it was hit.
+        let hit = std::thread::spawn(move || {
+            let (mut stream, _) = server.accept().unwrap();
+            let mut buf = [0u8; 1024];
+            let _ = stream.read(&mut buf);
+            stream
+                .write_all(b"HTTP/1.1 200 OK\r\nContent-Length: 2\r\n\r\n{}")
+                .unwrap();
+            true
+        });
+
+        // A dead proxy: nothing listens on this port. If the client honoured it, the send fails.
+        let dead_proxy = "http://127.0.0.1:9";
+        std::env::set_var("HTTP_PROXY", dead_proxy);
+        std::env::set_var("HTTPS_PROXY", dead_proxy);
+
+        let client = build_control_client().unwrap();
+        let resp = client
+            .post(format!("http://{addr}/"))
+            .body("{}")
+            .send()
+            .await;
+
+        std::env::remove_var("HTTP_PROXY");
+        std::env::remove_var("HTTPS_PROXY");
+
+        assert!(
+            resp.is_ok(),
+            "control client must connect DIRECT to loopback despite HTTP_PROXY: {resp:?}"
+        );
+        assert_eq!(resp.unwrap().status(), 200);
+        assert!(
+            hit.join().unwrap(),
+            "the direct loopback server was reached"
+        );
+    }
+}