Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 18 additions & 2 deletions cli/src/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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)),
Expand Down Expand Up @@ -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();

Expand Down
23 changes: 23 additions & 0 deletions cli/src/commands/identity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
114 changes: 112 additions & 2 deletions cli/src/main.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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)]
Expand Down Expand Up @@ -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 {
Expand All @@ -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};
Expand Down
107 changes: 106 additions & 1 deletion cli/src/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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());
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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"
);
}
}
Loading