feat(cli): --signing-key-file flag overrides signing identity per command (#282)

* feat(cli): --signing-key-file flag overrides signing identity per command

Adds a top-level `--signing-key-file <PATH>` flag (with
`RIVER_SIGNING_KEY_FILE` env var fallback) that reads a raw 32-byte
Ed25519 secret key and uses it in place of the room's stored
`signing_key_bytes` for the current command. The override is
in-memory only; `rooms.json` on disk is never modified.

# Why

`rooms.json` only stores ONE `signing_key_bytes` per room, but this
machine often has multiple identities for the same room (room owner,
invite bot, alt accounts). The UI's chat-delegate sync periodically
overwrites `rooms.json[room].signing_key_bytes` with whatever the
delegate has stored — silently leaving owner ops broken without a
manual swap of the on-disk key.

Pattern as documented in `river-official-room` skill: swap key → run
command → optionally swap back. Fragile and recurring. The flag
formalizes the "I have multiple identities, pick at command time"
model: nominate the right identity per command, no rooms.json
mutation, the existing identity stays loaded.

Distinct from `message send --signing-key` which takes a base64-
encoded inline key — the global file-based flag is preferred for
non-test use because the key doesn't appear in shell history.

# Tests

- `signing_key_override_is_returned_and_not_persisted` in
  `cli/src/storage.rs` pins the contract: with override, `get_room`
  returns the override; without override (fresh Storage), the
  ORIGINAL stored bytes come back — proving the override is not
  written back to rooms.json.

# Manual verification

Sent the river#275/#276/#278 release announcement to the official
Freenet River room as Room Owner via `--signing-key-file
~/.config/freenet-river-official/room_owner_signing_key.bin`, no
rooms.json swap. Confirmed via `riverctl message list` that the
message landed signed by Room Owner.

# Follow-up

#281 tracks the "perfect world" decentralized-version-pointer
follow-up (a `freenet-updates` crate that uses a Freenet contract
for tooling version checks). Separate PR.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* fix(cli): address PR #282 review findings

Skeptical HIGH-1: identity export under `--signing-key-file` could
produce a wire-format-corrupted token. When the override resolves to
a different identity than the cached `self_authorized_member` in
rooms.json, the bundled IdentityExport had a signing_key whose
verifying_key() did NOT match authorized_member.member.member_vk.
Importing such a token would silently fail every subsequent contract
operation.

Fix: validate `signing_key.verifying_key() == authorized_member.member.member_vk`
just before constructing IdentityExport; error out cleanly with a
message pointing at the override as the likely cause if they
disagree. The owner-self-signed path (line 74-81) and the
network-lookup path (line 82-113) already produce coherent pairs;
only the cached-from-disk path could mismatch.

Code-first/skeptical Important: doc-comment drift on
`Storage::signing_key_override` field said `--signing-key` /
`RIVER_SIGNING_KEY` but the actual flag/env are `--signing-key-file`
/ `RIVER_SIGNING_KEY_FILE`. Fixed both the field doc and the test
docstring so a future grep for the right names actually hits this
code.

Code-first Important: the file-loader error message referenced
`riverctl identity export` as if it produced raw 32-byte output. It
doesn't — it produces an armored multi-field token. Clarified the
error to point at the room-key .bin backup format (which IS raw 32
bytes) and explicitly contrast with `identity export`'s armored
shape.

Skeptical Important #5: added unit tests for the file loader's
length validation. Extracted `parse_signing_key_bytes` as a pure
helper so the tests don't need a tempfile. Tests cover: 32-byte
accept, short reject (with byte-count in message), long reject
(with base64 hint in message), empty reject.

Other Important items (skeptical #2 info-log on override, #3
file-permissions warning, #5 env-vs-flag precedence test) are
documented for follow-up but not blocking — the override-mismatch
fix is the load-bearing correctness item from this review round.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>

Author: sanity

cli/src/api.rs

@@ -64,6 +64,20 @@ impl ApiClient {
     }
 
     pub async fn new(node_url: &str, config: Config, config_dir: Option<&str>) -> Result<Self> {
+        Self::new_with_signing_key_override(node_url, config, config_dir, None).await
+    }
+
+    /// Construct an [`ApiClient`] with an optional in-memory signing-key
+    /// override. The override is propagated to [`Storage`] so every
+    /// `get_room` resolves the signing key from the override rather than
+    /// the per-room `signing_key_bytes`. See [`Storage::signing_key_override`]
+    /// for the motivating scenario.
+    pub async fn new_with_signing_key_override(
+        node_url: &str,
+        config: Config,
+        config_dir: Option<&str>,
+        signing_key_override: Option<SigningKey>,
+    ) -> Result<Self> {
         // Use the URL as provided - it should already be in the correct format
         info!("Connecting to Freenet node at: {}", node_url);
 
@@ -77,7 +91,7 @@ impl ApiClient {
         // Create WebApi instance
         let web_api = WebApi::start(ws_stream);
 
-        let storage = Storage::new(config_dir)?;
+        let storage = Storage::new_with_override(config_dir, signing_key_override)?;
 
         Ok(Self {
             web_api: Arc::new(Mutex::new(web_api)),
@@ -866,7 +880,9 @@ impl ApiClient {
             }
         };
 
-        let signing_key = SigningKey::from_bytes(&room_info.signing_key_bytes);
+        let signing_key = self
+            .storage
+            .resolve_signing_key(&room_info.signing_key_bytes);
         let room_state = room_info.state.clone();
         let previous_contract_key_str = room_info.previous_contract_key.clone();
 

cli/src/commands/identity.rs

@@ -133,6 +133,29 @@ async fn export_identity(
         Err(_) => (None, None), // Network unavailable; export without extras
     };
 
+    // Wire-format safety check. The export's signing_key MUST match the
+    // authorized_member.member.member_vk; otherwise importing the token
+    // produces an identity whose secret key signs nothing the room
+    // contract accepts.
+    //
+    // This catches the case where `--signing-key-file` overrides the
+    // signing identity but `self_authorized_member` is read from
+    // `rooms.json` (which still has the previous identity's
+    // `AuthorizedMember`). The two pieces would otherwise be packaged
+    // together with mismatched verifying keys, silently breaking the
+    // imported identity on first use.
+    if signing_key.verifying_key() != authorized_member.member.member_vk {
+        return Err(anyhow!(
+            "Refusing to export an identity with mismatched signing key and \
+             authorized member. The signing key's verifying key does not match \
+             the cached AuthorizedMember.member_vk for this room. This usually \
+             happens when `--signing-key-file` / `RIVER_SIGNING_KEY_FILE` overrides \
+             the signing identity but `rooms.json` still holds another identity's \
+             cached membership state. Re-run without the override (or with the \
+             override pointing at THIS identity) to produce a coherent token."
+        ));
+    }
+
     let export = IdentityExport {
         room_owner: room_owner_key,
         signing_key,

cli/src/main.rs

@@ -1,5 +1,6 @@
-use anyhow::{Context, Result};
+use anyhow::{anyhow, Context, Result};
 use clap::{Parser, Subcommand};
+use ed25519_dalek::SigningKey;
 use std::path::{Path, PathBuf};
 use tracing::info;
 use tracing_appender::non_blocking::WorkerGuard;
@@ -43,6 +44,33 @@ struct Cli {
     /// Optional path to write log output (stdout remains reserved for command output/JSON)
     #[arg(long, global = true, value_name = "PATH", env = "RIVERCTL_LOG_FILE")]
     log_file: Option<PathBuf>,
+
+    /// Override the signing identity for this command only. Reads a raw
+    /// 32-byte Ed25519 secret key from the given file path.
+    ///
+    /// The override is in-memory: it does NOT modify `rooms.json`. Use
+    /// this when you have multiple identities in the same room (e.g.,
+    /// room owner + invite bot + alt accounts) and want to pick which
+    /// one signs at command time, without the UI's chat-delegate sync
+    /// silently rewriting `rooms.json[room].signing_key_bytes` away
+    /// from your intended identity. The override key must be a valid
+    /// member of the target room or the command will be rejected by
+    /// the contract.
+    ///
+    /// Falls back to the `RIVER_SIGNING_KEY_FILE` env var if the flag
+    /// is not passed.
+    ///
+    /// Distinct from `message send --signing-key`, which takes a
+    /// base64-encoded key inline as a single-command override — the
+    /// global `--signing-key-file` flag is preferred for non-test use
+    /// because the key doesn't appear in shell history.
+    #[arg(
+        long,
+        global = true,
+        value_name = "PATH",
+        env = "RIVER_SIGNING_KEY_FILE"
+    )]
+    signing_key_file: Option<PathBuf>,
 }
 
 #[derive(Subcommand)]
@@ -96,8 +124,21 @@ async fn main() -> Result<()> {
     // Load configuration
     let config = config::Config::load()?;
 
+    // Resolve optional --signing-key-file override (or RIVER_SIGNING_KEY_FILE env var).
+    let signing_key_override = cli
+        .signing_key_file
+        .as_deref()
+        .map(load_signing_key_from_file)
+        .transpose()?;
+
     // Create API client
-    let api_client = api::ApiClient::new(&cli.node_url, config, cli.config_dir.as_deref()).await?;
+    let api_client = api::ApiClient::new_with_signing_key_override(
+        &cli.node_url,
+        config,
+        cli.config_dir.as_deref(),
+        signing_key_override,
+    )
+    .await?;
 
     // Execute command
     match cli.command {
@@ -115,6 +156,75 @@ async fn main() -> Result<()> {
     Ok(())
 }
 
+/// Load a raw 32-byte Ed25519 secret key from the given file path.
+/// Used by the `--signing-key-file` flag / `RIVER_SIGNING_KEY_FILE` env var.
+/// Errors are surfaced with a clear message identifying the bad path
+/// and the actual length seen, so the user can tell "I pointed at the
+/// wrong file" from "I pointed at a base64-encoded file".
+fn load_signing_key_from_file(path: &Path) -> Result<SigningKey> {
+    let bytes = std::fs::read(path)
+        .with_context(|| format!("failed to read signing key file: {}", path.display()))?;
+    parse_signing_key_bytes(&bytes)
+        .map_err(|reason| anyhow!("{} — file: {}", reason, path.display()))
+}
+
+/// Pure helper for testing the wrong-length / right-length validation
+/// without touching the filesystem.
+fn parse_signing_key_bytes(bytes: &[u8]) -> std::result::Result<SigningKey, String> {
+    if bytes.len() != 32 {
+        return Err(format!(
+            "signing key must be exactly 32 raw bytes, got {} bytes \
+             (was this file base64- or hex-encoded? the override expects raw \
+             bytes — the same format as the room-key backups under \
+             ~/.config/freenet-river-official/*.bin; NOT the armored output of \
+             `riverctl identity export`, which is a larger multi-field token)",
+            bytes.len()
+        ));
+    }
+    let mut buf = [0u8; 32];
+    buf.copy_from_slice(bytes);
+    Ok(SigningKey::from_bytes(&buf))
+}
+
+#[cfg(test)]
+mod cli_tests {
+    use super::*;
+
+    #[test]
+    fn parse_signing_key_bytes_accepts_32_byte_input() {
+        let raw = [7u8; 32];
+        let sk = parse_signing_key_bytes(&raw).expect("32 raw bytes is valid");
+        assert_eq!(sk.to_bytes(), raw);
+    }
+
+    #[test]
+    fn parse_signing_key_bytes_rejects_short_input() {
+        let raw = [7u8; 16];
+        let err = parse_signing_key_bytes(&raw).expect_err("must reject short input");
+        assert!(err.contains("32 raw bytes"), "msg: {}", err);
+        assert!(err.contains("16 bytes"), "msg: {}", err);
+    }
+
+    #[test]
+    fn parse_signing_key_bytes_rejects_long_input() {
+        // 44-byte base64-encoded 32-byte key (the most common user mistake)
+        let raw = [b'a'; 44];
+        let err = parse_signing_key_bytes(&raw).expect_err("must reject long input");
+        assert!(
+            err.contains("base64"),
+            "must hint at base64 mistake: {}",
+            err
+        );
+    }
+
+    #[test]
+    fn parse_signing_key_bytes_rejects_empty() {
+        let raw: [u8; 0] = [];
+        let err = parse_signing_key_bytes(&raw).expect_err("must reject empty input");
+        assert!(err.contains("0 bytes"), "msg: {}", err);
+    }
+}
+
 fn init_logging(debug: bool, log_path: Option<&Path>) -> Result<Option<WorkerGuard>> {
     use std::fs::OpenOptions;
     use tracing_subscriber::{fmt, layer::SubscriberExt, Registry};

cli/src/storage.rs

@@ -43,10 +43,33 @@ pub struct Storage {
     /// Side file so the larger `rooms.json` blob stays untouched on
     /// each DM send. JSON-serialized [`OutboundDmStore`].
     outbound_dms_path: PathBuf,
+    /// In-memory signing-key override (from `--signing-key-file` flag or
+    /// `RIVER_SIGNING_KEY_FILE` env var). When set, every call to
+    /// [`Storage::get_room`] returns this key in place of the room's
+    /// stored `signing_key_bytes`. Never written back to disk — the
+    /// override is a per-command-invocation thing.
+    ///
+    /// Motivates: this machine often has multiple identities for the
+    /// same room (room owner, invite bot, test alts), but `rooms.json`
+    /// only stores ONE `signing_key_bytes` per room. The UI's chat-
+    /// delegate sync can silently rewrite that field — leaving owner
+    /// ops broken without a manual swap. The override lets owner ops
+    /// nominate the right identity at command time without touching
+    /// `rooms.json`. See discussion on river#281.
+    signing_key_override: Option<SigningKey>,
 }
 
 impl Storage {
     pub fn new(config_dir: Option<&str>) -> Result<Self> {
+        Self::new_with_override(config_dir, None)
+    }
+
+    /// Construct a [`Storage`] with an optional in-memory signing-key
+    /// override. See the field doc on [`Storage::signing_key_override`].
+    pub fn new_with_override(
+        config_dir: Option<&str>,
+        signing_key_override: Option<SigningKey>,
+    ) -> Result<Self> {
         // Use provided config_dir, then check environment variable, then use default
         let data_dir = if let Some(dir) = config_dir {
             PathBuf::from(dir)
@@ -67,9 +90,23 @@ impl Storage {
         Ok(Self {
             storage_path,
             outbound_dms_path,
+            signing_key_override,
         })
     }
 
+    /// Resolve the signing key to use for the current command: prefer
+    /// the in-memory override if set, otherwise reconstruct from the
+    /// per-room `signing_key_bytes`. Used by both [`Storage::get_room`]
+    /// and [`crate::api::ApiClient::ensure_room_migrated`] (which has
+    /// its own load_rooms snapshot for migration purposes).
+    pub fn resolve_signing_key(&self, stored_bytes: &[u8; 32]) -> SigningKey {
+        if let Some(override_key) = &self.signing_key_override {
+            override_key.clone()
+        } else {
+            SigningKey::from_bytes(stored_bytes)
+        }
+    }
+
     pub fn load_rooms(&self) -> Result<RoomStorage> {
         if !self.storage_path.exists() {
             return Ok(RoomStorage::default());
@@ -183,7 +220,7 @@ impl Storage {
         let owner_key_str = bs58::encode(owner_vk.as_bytes()).into_string();
 
         if let Some(room_info) = storage.rooms.get(&owner_key_str) {
-            let signing_key = SigningKey::from_bytes(&room_info.signing_key_bytes);
+            let signing_key = self.resolve_signing_key(&room_info.signing_key_bytes);
             Ok(Some((
                 signing_key,
                 room_info.state.clone(),
@@ -470,4 +507,72 @@ mod tests {
             "previous_contract_key should be None when WASM hasn't changed"
         );
     }
+
+    /// `--signing-key-file` / `RIVER_SIGNING_KEY_FILE` override: `Storage::get_room`
+    /// must return the override key in place of the room's stored
+    /// `signing_key_bytes`, AND the override must NOT be written back to
+    /// `rooms.json`. This pins the "in-memory only" contract documented
+    /// on `Storage::signing_key_override` — the persistent on-disk
+    /// identity must be untouched, so subsequent invocations without
+    /// the override revert to the stored identity.
+    #[test]
+    fn signing_key_override_is_returned_and_not_persisted() {
+        let temp_dir = TempDir::new().unwrap();
+        let stored_sk = create_test_signing_key();
+        let override_sk = create_test_signing_key();
+        assert_ne!(
+            stored_sk.to_bytes(),
+            override_sk.to_bytes(),
+            "test invariant: stored and override keys must differ"
+        );
+        let owner_vk = stored_sk.verifying_key();
+
+        // Set up storage with the stored identity (no override).
+        let storage_no_override = Storage::new(Some(temp_dir.path().to_str().unwrap())).unwrap();
+        let state = create_test_state(&stored_sk);
+        let initial_key = expected_contract_key(&owner_vk);
+        storage_no_override
+            .add_room(&owner_vk, &stored_sk, state, &initial_key)
+            .unwrap();
+
+        // Sanity: without override, get_room returns the stored key.
+        let (sk_no_override, _, _) = storage_no_override
+            .get_room(&owner_vk)
+            .unwrap()
+            .expect("room present");
+        assert_eq!(
+            sk_no_override.to_bytes(),
+            stored_sk.to_bytes(),
+            "no override → stored key"
+        );
+
+        // With override, get_room returns the override.
+        let storage_with_override = Storage::new_with_override(
+            Some(temp_dir.path().to_str().unwrap()),
+            Some(override_sk.clone()),
+        )
+        .unwrap();
+        let (sk_with_override, _, _) = storage_with_override
+            .get_room(&owner_vk)
+            .unwrap()
+            .expect("room present");
+        assert_eq!(
+            sk_with_override.to_bytes(),
+            override_sk.to_bytes(),
+            "override → override key returned"
+        );
+
+        // Critical: rooms.json on disk is untouched. A fresh Storage
+        // without the override must see the ORIGINAL stored bytes.
+        let storage_fresh = Storage::new(Some(temp_dir.path().to_str().unwrap())).unwrap();
+        let (sk_fresh, _, _) = storage_fresh
+            .get_room(&owner_vk)
+            .unwrap()
+            .expect("room present");
+        assert_eq!(
+            sk_fresh.to_bytes(),
+            stored_sk.to_bytes(),
+            "override must NOT be written back to rooms.json"
+        );
+    }
 }