diff --git a/cli/src/api.rs b/cli/src/api.rs index 6aeb5c93..7767f5ab 100644 --- a/cli/src/api.rs +++ b/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::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, + ) -> Result { // 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(); diff --git a/cli/src/commands/identity.rs b/cli/src/commands/identity.rs index 6905b4f1..3ace4344 100644 --- a/cli/src/commands/identity.rs +++ b/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, diff --git a/cli/src/main.rs b/cli/src/main.rs index 3f5e90b6..a65f777a 100644 --- a/cli/src/main.rs +++ b/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, + + /// 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, } #[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 { + 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 { + 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> { use std::fs::OpenOptions; use tracing_subscriber::{fmt, layer::SubscriberExt, Registry}; diff --git a/cli/src/storage.rs b/cli/src/storage.rs index 6ef2661c..1fdf2659 100644 --- a/cli/src/storage.rs +++ b/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, } impl Storage { pub fn new(config_dir: Option<&str>) -> Result { + 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, + ) -> Result { // 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 { 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" + ); + } }