From 1846364114aed757621e8a64a7a5d6d448ba7c09 Mon Sep 17 00:00:00 2001 From: Hans Behrens Date: Wed, 18 Feb 2026 17:05:54 -0700 Subject: [PATCH] v0.7.1: fix 8 issues from hands-on evaluation - Client-side IPC size check before socket connect (prevents broken pipe on >64KB payloads) - Promote MAX_IPC_LINE_LENGTH to pub const in ipc::protocol (single source of truth) - config get of unset key prints ': not set' to stderr (exit code 1 preserved) - Reject port 0 in config set (QUIC requires non-zero port) - Add tracing::debug! to IPC client path (makes -v useful for IPC debugging) - doctor --fix can now backup and reset corrupt config.yaml - Empty peers message shortened to 'No peers.' - request/notify help text documents payload wrapping on the wire - Update README internal constants table (MAX_IPC_LINE_LENGTH location) - Version bump to 0.7.1 Amp-Thread-ID: https://ampcode.com/threads/T-019c732b-acc2-7108-9846-5c6cb63fa276 Co-authored-by: Amp --- README.md | 2 +- axon/Cargo.lock | 2 +- axon/Cargo.toml | 2 +- axon/src/cli/config_cmd.rs | 14 +++++ axon/src/cli/config_cmd_tests.rs | 25 ++++++++- axon/src/cli/format.rs | 2 +- axon/src/cli/ipc_client.rs | 15 ++++- axon/src/cli/ipc_client_tests.rs | 22 +++++++- axon/src/doctor.rs | 2 +- axon/src/doctor/checks.rs | 42 +++++++++++--- axon/src/ipc/mod.rs | 3 +- axon/src/ipc/protocol.rs | 3 + axon/src/ipc/server.rs | 2 +- axon/src/main.rs | 4 +- axon/tests/cli_contract_config.rs | 21 +++++++ axon/tests/doctor_contract.rs | 91 +++++++++++++++++++++++++++++++ 16 files changed, 233 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index b9c3b78..49fbe43 100644 --- a/README.md +++ b/README.md @@ -254,7 +254,7 @@ These are compile-time constants and cannot be changed via configuration. | `MAX_MESSAGE_SIZE` | `65536` (64 KB) | `message/envelope.rs` | Maximum encoded envelope size. Messages exceeding this are rejected. | | `REQUEST_TIMEOUT` | `30s` | `transport/mod.rs` | Timeout for bidirectional request/response exchanges. | | `STALE_TIMEOUT` | `60s` | `peer_table.rs` | Discovered (non-static, non-cached) peers with no activity for this duration are removed. | -| `MAX_IPC_LINE_LENGTH` | `64 KB` | `ipc/server.rs` | Maximum length of a single IPC command line. Overlong lines are rejected with `command_too_large`. | +| `MAX_IPC_LINE_LENGTH` | `64 KB` | `ipc/protocol.rs` | Maximum length of a single IPC command line. Overlong lines are rejected with `command_too_large`. | | `MAX_CONNECTIONS` | `128` | `daemon/mod.rs` | Maximum simultaneous QUIC peer connections. | | `KEEPALIVE` | `15s` | `daemon/mod.rs` | QUIC keepalive interval. | | `IDLE_TIMEOUT` | `60s` | `daemon/mod.rs` | QUIC idle timeout. Connections with no traffic for this duration are closed. | diff --git a/axon/Cargo.lock b/axon/Cargo.lock index 0e83592..6a23bd4 100644 --- a/axon/Cargo.lock +++ b/axon/Cargo.lock @@ -142,7 +142,7 @@ dependencies = [ [[package]] name = "axon" -version = "0.7.0" +version = "0.7.1" dependencies = [ "anyhow", "base64", diff --git a/axon/Cargo.toml b/axon/Cargo.toml index 043de40..592fa54 100644 --- a/axon/Cargo.toml +++ b/axon/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "axon" -version = "0.7.0" +version = "0.7.1" edition = "2024" description = "AXON — Agent eXchange Over Network, LLM-first local messaging daemon" license = "MIT" diff --git a/axon/src/cli/config_cmd.rs b/axon/src/cli/config_cmd.rs index 29bd4b6..85ce263 100644 --- a/axon/src/cli/config_cmd.rs +++ b/axon/src/cli/config_cmd.rs @@ -83,6 +83,7 @@ pub async fn run(paths: &AxonPaths, args: ConfigArgs) -> Result { println!("{value}"); Ok(ExitCode::SUCCESS) } else { + eprintln!("{}: not set", key_display_name(key)); Ok(ExitCode::from(1)) } } @@ -134,6 +135,14 @@ fn parse_action(args: &ConfigArgs) -> Result { Ok(ConfigAction::Get(key)) } +fn key_display_name(key: ConfigKey) -> &'static str { + match key { + ConfigKey::Name => "name", + ConfigKey::Port => "port", + ConfigKey::AdvertiseAddr => "advertise-addr", + } +} + fn get_value(config: &PersistedConfig, key: ConfigKey) -> Option { match key { ConfigKey::Name => config.name.clone(), @@ -155,6 +164,11 @@ fn apply_set(config: &mut PersistedConfig, key: ConfigKey, value: &str) -> Resul let port = value .parse::() .with_context(|| format!("invalid port '{value}'"))?; + if port == 0 { + anyhow::bail!( + "port 0 is not valid; QUIC requires a non-zero port for peer connections" + ); + } config.port = Some(port); } ConfigKey::AdvertiseAddr => { diff --git a/axon/src/cli/config_cmd_tests.rs b/axon/src/cli/config_cmd_tests.rs index 79c62c5..a7d0b01 100644 --- a/axon/src/cli/config_cmd_tests.rs +++ b/axon/src/cli/config_cmd_tests.rs @@ -1,4 +1,4 @@ -use super::{ConfigArgs, ConfigKey, apply_set, parse_action, render_list_text}; +use super::{ConfigArgs, ConfigKey, apply_set, key_display_name, parse_action, render_list_text}; use axon::config::PersistedConfig; #[test] @@ -37,6 +37,29 @@ fn apply_set_rejects_bad_values() { assert!(apply_set(&mut config, ConfigKey::AdvertiseAddr, "missing-port").is_err()); } +#[test] +fn apply_set_rejects_port_zero() { + let mut config = PersistedConfig::default(); + let err = apply_set(&mut config, ConfigKey::Port, "0").expect_err("port 0 should fail"); + assert!(err.to_string().contains("port 0")); +} + +#[test] +fn apply_set_accepts_valid_port_boundaries() { + let mut config = PersistedConfig::default(); + apply_set(&mut config, ConfigKey::Port, "1").expect("port 1"); + assert_eq!(config.port, Some(1)); + apply_set(&mut config, ConfigKey::Port, "65535").expect("port 65535"); + assert_eq!(config.port, Some(65535)); +} + +#[test] +fn key_display_name_maps_all_keys() { + assert_eq!(key_display_name(ConfigKey::Name), "name"); + assert_eq!(key_display_name(ConfigKey::Port), "port"); + assert_eq!(key_display_name(ConfigKey::AdvertiseAddr), "advertise-addr"); +} + #[test] fn render_list_text_only_includes_set_keys() { let config = PersistedConfig { diff --git a/axon/src/cli/format.rs b/axon/src/cli/format.rs index c0259b6..1a415c6 100644 --- a/axon/src/cli/format.rs +++ b/axon/src/cli/format.rs @@ -4,7 +4,7 @@ use serde_json::Value; pub fn render_peers_human(response: &Value) -> Option { let peers = response.get("peers")?.as_array()?; if peers.is_empty() { - return Some("No peers found.".to_string()); + return Some("No peers.".to_string()); } let mut rows: Vec<[String; 5]> = Vec::with_capacity(peers.len()); diff --git a/axon/src/cli/ipc_client.rs b/axon/src/cli/ipc_client.rs index abfedc2..572e74b 100644 --- a/axon/src/cli/ipc_client.rs +++ b/axon/src/cli/ipc_client.rs @@ -42,6 +42,16 @@ pub fn render_json(value: &Value) -> Result { } pub async fn send_ipc(paths: &AxonPaths, command: Value) -> Result { + let line = serde_json::to_string(&command).context("failed to serialize IPC command")?; + if line.len() > axon::ipc::MAX_IPC_LINE_LENGTH { + anyhow::bail!( + "IPC command size ({} bytes) exceeds the 64KB limit", + line.len() + ); + } + + tracing::debug!(socket = %paths.socket.display(), "connecting to daemon IPC socket"); + let mut stream = UnixStream::connect(&paths.socket).await.with_context(|| { format!( "failed to connect to daemon socket: {}. Is the daemon running?", @@ -49,7 +59,8 @@ pub async fn send_ipc(paths: &AxonPaths, command: Value) -> Result { ) })?; - let line = serde_json::to_string(&command).context("failed to serialize IPC command")?; + tracing::debug!(cmd_bytes = line.len(), "sending IPC command"); + stream .write_all(line.as_bytes()) .await @@ -85,8 +96,10 @@ pub async fn send_ipc(paths: &AxonPaths, command: Value) -> Result { let decoded: Value = serde_json::from_str(line).context("failed to decode IPC response")?; if is_unsolicited_event(&decoded) { + tracing::debug!("skipping unsolicited IPC event"); continue; } + tracing::debug!("received IPC command response"); return Ok(decoded); } } diff --git a/axon/src/cli/ipc_client_tests.rs b/axon/src/cli/ipc_client_tests.rs index 34b7e82..a214534 100644 --- a/axon/src/cli/ipc_client_tests.rs +++ b/axon/src/cli/ipc_client_tests.rs @@ -1,8 +1,9 @@ +use std::path::PathBuf; use std::process::ExitCode; use serde_json::json; -use super::{ResponseMode, daemon_reply_exit_code, is_unsolicited_event}; +use super::{ResponseMode, daemon_reply_exit_code, is_unsolicited_event, send_ipc}; #[test] fn daemon_error_maps_to_exit_two() { @@ -35,6 +36,25 @@ fn request_with_embedded_error_envelope_maps_to_exit_two() { assert_eq!(code, ExitCode::from(2)); } +#[tokio::test] +async fn send_ipc_rejects_oversized_command() { + let paths = axon::config::AxonPaths { + root: PathBuf::from("/tmp/axon-test-nonexistent"), + socket: PathBuf::from("/tmp/axon-test-nonexistent/axon.sock"), + config: PathBuf::from("/tmp/axon-test-nonexistent/config.yaml"), + known_peers: PathBuf::from("/tmp/axon-test-nonexistent/known_peers.json"), + identity_key: PathBuf::from("/tmp/axon-test-nonexistent/identity.key"), + identity_pub: PathBuf::from("/tmp/axon-test-nonexistent/identity.pub"), + }; + + let big_payload = "x".repeat(70_000); + let command = json!({"cmd": "send", "to": "ed25519.00000000000000000000000000000000", "kind": "message", "payload": big_payload}); + let err = send_ipc(&paths, command) + .await + .expect_err("should reject oversized command"); + assert!(err.to_string().contains("exceeds"), "error: {err}"); +} + #[test] fn unsolicited_event_detection_uses_event_key() { assert!(is_unsolicited_event(&json!({"event": "inbound"}))); diff --git a/axon/src/doctor.rs b/axon/src/doctor.rs index 044cc8a..ceaf80a 100644 --- a/axon/src/doctor.rs +++ b/axon/src/doctor.rs @@ -78,7 +78,7 @@ pub async fn run(paths: &AxonPaths, args: &DoctorArgs) -> Result { identity_check::check_identity(paths, args, &mut report)?; checks::check_daemon_artifacts(paths, args, &mut report)?; checks::check_known_peers(paths, args, &mut report).await?; - checks::check_config(paths, &mut report).await?; + checks::check_config(paths, args, &mut report).await?; Ok(report) } diff --git a/axon/src/doctor/checks.rs b/axon/src/doctor/checks.rs index 90f4841..4927681 100644 --- a/axon/src/doctor/checks.rs +++ b/axon/src/doctor/checks.rs @@ -6,7 +6,9 @@ use std::time::{SystemTime, UNIX_EPOCH}; use anyhow::{Context, Result, anyhow}; -use axon::config::{AxonPaths, Config, load_known_peers, save_known_peers}; +use axon::config::{ + AxonPaths, Config, PersistedConfig, load_known_peers, save_known_peers, save_persisted_config, +}; use super::{DoctorArgs, DoctorReport}; @@ -303,7 +305,11 @@ pub(super) async fn check_known_peers( Ok(()) } -pub(super) async fn check_config(paths: &AxonPaths, report: &mut DoctorReport) -> Result<()> { +pub(super) async fn check_config( + paths: &AxonPaths, + args: &DoctorArgs, + report: &mut DoctorReport, +) -> Result<()> { if !paths.config.exists() { report.add_check("config", true, false, "config.yaml not present".to_string()); return Ok(()); @@ -319,12 +325,32 @@ pub(super) async fn check_config(paths: &AxonPaths, report: &mut DoctorReport) - ); } Err(err) => { - report.add_check( - "config", - false, - false, - format!("config.yaml parse/load error: {err}"), - ); + if args.fix { + let backup = backup_file_with_timestamp(&paths.config)?; + save_persisted_config(&paths.config, &PersistedConfig::default()).await?; + report.add_fix( + "config_reset", + format!( + "backed up corrupt config.yaml to {} and reset to defaults (peer enrollments lost — re-run `axon connect` to restore)", + backup.display() + ), + ); + report.add_check( + "config", + true, + true, + "corrupt config.yaml reset to defaults".to_string(), + ); + } else { + report.add_check( + "config", + false, + true, + format!( + "config.yaml parse/load error: {err}; run `axon doctor --fix` to back up and reset" + ), + ); + } } } diff --git a/axon/src/ipc/mod.rs b/axon/src/ipc/mod.rs index e673034..00dd3be 100644 --- a/axon/src/ipc/mod.rs +++ b/axon/src/ipc/mod.rs @@ -3,6 +3,7 @@ mod protocol; mod server; pub use protocol::{ - CommandEvent, DaemonReply, IpcCommand, IpcErrorCode, IpcSendKind, PeerSummary, WhoamiInfo, + CommandEvent, DaemonReply, IpcCommand, IpcErrorCode, IpcSendKind, MAX_IPC_LINE_LENGTH, + PeerSummary, WhoamiInfo, }; pub use server::{IpcServer, IpcServerConfig}; diff --git a/axon/src/ipc/protocol.rs b/axon/src/ipc/protocol.rs index 9e84393..982b417 100644 --- a/axon/src/ipc/protocol.rs +++ b/axon/src/ipc/protocol.rs @@ -4,6 +4,9 @@ use uuid::Uuid; use crate::message::{Envelope, MessageKind}; +/// Maximum length of a single IPC command line (64 KB). +pub const MAX_IPC_LINE_LENGTH: usize = 64 * 1024; + // --------------------------------------------------------------------------- // IPC protocol types // --------------------------------------------------------------------------- diff --git a/axon/src/ipc/server.rs b/axon/src/ipc/server.rs index 9eda1ee..776996f 100644 --- a/axon/src/ipc/server.rs +++ b/axon/src/ipc/server.rs @@ -15,7 +15,7 @@ use super::auth; use super::protocol::{CommandEvent, DaemonReply, IpcCommand, IpcErrorCode, WhoamiInfo}; use crate::message::Envelope; -const MAX_IPC_LINE_LENGTH: usize = 64 * 1024; // 64 KB, aligned with MAX_MESSAGE_SIZE +use super::protocol::MAX_IPC_LINE_LENGTH; static INVALID_COMMAND_LINE: LazyLock> = LazyLock::new(|| { let error = super::protocol::IpcErrorCode::InvalidCommand; diff --git a/axon/src/main.rs b/axon/src/main.rs index f11af51..920aafd 100644 --- a/axon/src/main.rs +++ b/axon/src/main.rs @@ -68,15 +68,17 @@ enum Commands { value_parser = clap::value_parser!(u64).range(1..) )] timeout: u64, + /// Text payload (sent as {"message":""} on the wire). message: String, }, /// Send a fire-and-forget message to another agent. Notify { #[arg(value_parser = parse_agent_id_arg)] agent_id: String, - /// Parse payload as JSON (default sends literal text). + /// Parse payload as JSON (default sends literal text). Payload is sent as {"data": }. #[arg(long)] json: bool, + /// Payload data (sent as {"data":""}, or {"data":} with --json). data: String, }, /// List discovered and connected peers. diff --git a/axon/tests/cli_contract_config.rs b/axon/tests/cli_contract_config.rs index af3ea33..1fad796 100644 --- a/axon/tests/cli_contract_config.rs +++ b/axon/tests/cli_contract_config.rs @@ -93,6 +93,11 @@ fn config_get_set_list_and_unset_roundtrip() { .trim() .is_empty() ); + let get_initial_stderr = String::from_utf8_lossy(&get_initial.stderr); + assert!( + get_initial_stderr.contains("name: not set"), + "stderr should indicate unset key, got: {get_initial_stderr}" + ); let set_name = run_command(Command::new(&bin).args(["--state-root", root_str, "config", "name", "Alice"])); @@ -141,6 +146,22 @@ fn config_set_invalid_port_fails() { assert!(stderr.contains("invalid port")); } +#[test] +fn config_set_port_zero_fails() { + let bin = axon_bin(); + let root = tempdir().expect("tempdir"); + let root_str = root.path().to_str().expect("utf8 path"); + + let output = + run_command(Command::new(&bin).args(["--state-root", root_str, "config", "port", "0"])); + assert_eq!(output.status.code(), Some(1)); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("port 0"), + "stderr should mention port 0, got: {stderr}" + ); +} + #[test] fn connect_writes_config_and_sends_add_peer_ipc() { let bin = axon_bin(); diff --git a/axon/tests/doctor_contract.rs b/axon/tests/doctor_contract.rs index 3099f87..72c02b5 100644 --- a/axon/tests/doctor_contract.rs +++ b/axon/tests/doctor_contract.rs @@ -201,6 +201,97 @@ fn doctor_fix_rekey_backs_up_and_regenerates_identity() { assert!(backup_found, "expected identity.key backup file"); } +#[test] +fn doctor_check_reports_corrupt_config_as_fixable() { + let root = tempdir().expect("tempdir"); + fs::set_permissions(root.path(), fs::Permissions::from_mode(0o700)).expect("set perms"); + + // Write valid identity so that check passes, isolating the config check. + let identity = axon::identity::Identity::load_or_generate(&axon::config::AxonPaths::from_root( + root.path().to_path_buf(), + )) + .expect("generate identity"); + drop(identity); + + // Write corrupt config. + fs::write(root.path().join("config.yaml"), b"\x80\x81 invalid yaml").expect("write corrupt"); + + let output = run_doctor_json(root.path(), &[]); + assert_eq!(output.status.code(), Some(2)); + + let report = parse_report(&output); + let config = check_by_name(&report, "config"); + assert_eq!(config["ok"], false); + assert_eq!(config["fixable"], true); + assert!( + config["message"] + .as_str() + .expect("config message") + .contains("doctor --fix"), + "should suggest --fix" + ); +} + +#[test] +fn doctor_fix_resets_corrupt_config_with_backup() { + let root = tempdir().expect("tempdir"); + fs::set_permissions(root.path(), fs::Permissions::from_mode(0o700)).expect("set perms"); + + // Write valid identity so that check passes, isolating the config check. + let identity = axon::identity::Identity::load_or_generate(&axon::config::AxonPaths::from_root( + root.path().to_path_buf(), + )) + .expect("generate identity"); + drop(identity); + + let corrupt_content = b"\x80\x81 invalid binary"; + fs::write(root.path().join("config.yaml"), corrupt_content).expect("write corrupt"); + + let output = run_doctor_json(root.path(), &["--fix"]); + assert!(output.status.success()); + + let report = parse_report(&output); + assert_eq!(report["ok"], true); + + // Config check should pass after fix. + let config = check_by_name(&report, "config"); + assert_eq!(config["ok"], true); + + // Fixes should include config_reset. + let fixes = report["fixes_applied"] + .as_array() + .expect("fixes_applied array"); + assert!( + fixes + .iter() + .any(|f| f["name"].as_str() == Some("config_reset")), + "should report config_reset fix" + ); + + // Backup file should exist with original corrupt content. + let mut backup_found = false; + for entry in fs::read_dir(root.path()).expect("read dir") { + let path = entry.expect("dir entry").path(); + let Some(name) = path.file_name().and_then(|n| n.to_str()) else { + continue; + }; + if name.starts_with("config.yaml.bak.") { + let backup_content = fs::read(&path).expect("read backup"); + assert_eq!(backup_content, corrupt_content); + backup_found = true; + break; + } + } + assert!(backup_found, "expected config.yaml backup file"); + + // New config.yaml should be valid and parseable. + let new_content = fs::read_to_string(root.path().join("config.yaml")).expect("read new config"); + assert!( + !new_content.is_empty(), + "reset config.yaml should not be empty" + ); +} + #[test] fn doctor_subcommand_is_visible_in_help() { let output = run_command(Command::new(axon_bin()).arg("--help"));