diff --git a/crates/maestro-cli/src/attach.rs b/crates/maestro-cli/src/attach.rs index 64052e6..2d9b42d 100644 --- a/crates/maestro-cli/src/attach.rs +++ b/crates/maestro-cli/src/attach.rs @@ -84,7 +84,8 @@ pub fn attach_url(info: &ConnectInfo, session_id: Uuid) -> Result assert!(json), - other => panic!("wrong subcommand: {other:?}"), - } - } - #[test] fn rejects_env_without_equals() { let err = Cli::try_parse_from([ diff --git a/crates/maestro-cli/src/dispatch.rs b/crates/maestro-cli/src/dispatch.rs index 262a8cb..0bc1647 100644 --- a/crates/maestro-cli/src/dispatch.rs +++ b/crates/maestro-cli/src/dispatch.rs @@ -11,7 +11,8 @@ //! * `list` reads the daemon's local catalog mirror (own ∪ peer sessions). //! * `spawn` / `kill` hit the daemon's local session endpoints. //! * `attach` opens the daemon's `WS /api/attach/:id` broker, which picks the -//! transport and enforces `allow_inbound` on the target side. +//! transport and dials the target (the target verifies the registry-minted +//! pair-attach JWT — session binding, not a per-peer allow-list). //! //! If the home daemon is **unreachable** (connection refused / not running), //! every command falls back to the legacy registry path (registry HTTP + @@ -23,10 +24,9 @@ use std::path::{Path, PathBuf}; use crate::attach::{broker_attach_url, parse_detach_key, run_attach, run_attach_url}; -use crate::cli::{AttachArgs, Command, KillArgs, ListArgs, PolicyCmd, SetRegistryArgs, SpawnArgs}; +use crate::cli::{AttachArgs, Command, KillArgs, ListArgs, SetRegistryArgs, SpawnArgs}; use crate::config::{default_config_path, CliConfig, ConfigError}; use crate::http::{HttpError, RegistryClient}; -use crate::policy::{run_reload as run_policy_reload, run_show as run_policy_show}; use crate::sessions::{fetch_connect_info, run_kill, run_list, run_spawn}; #[derive(Debug, thiserror::Error)] @@ -40,8 +40,6 @@ pub enum CliError { Attach(#[from] crate::attach::AttachError), #[error("{0}")] Io(#[from] std::io::Error), - #[error("{0}")] - Policy(#[from] crate::policy::PolicyError), } /// Dispatch a parsed top-level subcommand. The umbrella binary passes the @@ -61,7 +59,6 @@ pub async fn dispatch( match command { Command::SetRegistry(args) => set_registry(&config_path, args), - Command::Policy(sub) => policy(&config_path, sub).await, Command::List(args) => { list( &config_path, @@ -109,16 +106,6 @@ fn set_registry(config_path: &Path, args: SetRegistryArgs) -> Result<(), CliErro Ok(()) } -#[allow(clippy::result_large_err)] -async fn policy(config_path: &Path, sub: PolicyCmd) -> Result<(), CliError> { - let mut stdout = std::io::stdout().lock(); - match sub { - PolicyCmd::Reload => run_policy_reload(config_path, &mut stdout).await?, - PolicyCmd::Show { json } => run_policy_show(config_path, json, &mut stdout).await?, - } - Ok(()) -} - #[allow(clippy::result_large_err)] async fn list( config_path: &Path, diff --git a/crates/maestro-cli/src/lib.rs b/crates/maestro-cli/src/lib.rs index 8e5b116..5930e64 100644 --- a/crates/maestro-cli/src/lib.rs +++ b/crates/maestro-cli/src/lib.rs @@ -10,7 +10,6 @@ pub mod cli; pub mod config; pub mod dispatch; pub mod http; -pub mod policy; pub mod sessions; pub use dispatch::{dispatch, CliError}; diff --git a/crates/maestro-cli/src/policy.rs b/crates/maestro-cli/src/policy.rs deleted file mode 100644 index 348c5f2..0000000 --- a/crates/maestro-cli/src/policy.rs +++ /dev/null @@ -1,127 +0,0 @@ -//! `maestro policy reload | show` — talk to the local daemon's -//! `/api/policy` endpoints (NAP-237). -//! -//! The local daemon's bind port is read from `[daemon].bind_addr` in the -//! unified config; the host is always `127.0.0.1` (these endpoints are -//! same-host by design — the daemon's bind may be `0.0.0.0` for attach, -//! but operators reload policy from the same machine). - -use std::io::Write; -use std::path::Path; - -use maestro_config::MaestroConfigFile; -use serde::Deserialize; - -const DEFAULT_PORT: u16 = 7070; - -#[derive(Debug, thiserror::Error)] -pub enum PolicyError { - #[error("config: {0}")] - Config(#[from] maestro_config::ConfigError), - #[error("http: {0}")] - Http(#[from] reqwest::Error), - #[error("daemon returned {status}: {body}")] - DaemonError { - status: reqwest::StatusCode, - body: String, - }, - #[error("io: {0}")] - Io(#[from] std::io::Error), -} - -#[derive(Debug, Deserialize)] -pub struct PolicyView { - pub allow_inbound: Vec, -} - -pub async fn run_reload(config_path: &Path, out: &mut impl Write) -> Result<(), PolicyError> { - let url = format!("{}/api/policy/reload", local_base_url(config_path)?); - let resp = crate::http::build_client().post(&url).send().await?; - let status = resp.status(); - if !status.is_success() { - let body = resp.text().await.unwrap_or_default(); - return Err(PolicyError::DaemonError { status, body }); - } - let view: PolicyView = resp.json().await?; - writeln!( - out, - "reloaded — allow_inbound now has {} entr{}", - view.allow_inbound.len(), - if view.allow_inbound.len() == 1 { - "y" - } else { - "ies" - }, - )?; - for entry in &view.allow_inbound { - writeln!(out, " {entry}")?; - } - Ok(()) -} - -pub async fn run_show( - config_path: &Path, - json: bool, - out: &mut impl Write, -) -> Result<(), PolicyError> { - let url = format!("{}/api/policy", local_base_url(config_path)?); - let resp = crate::http::build_client().get(&url).send().await?; - let status = resp.status(); - if !status.is_success() { - let body = resp.text().await.unwrap_or_default(); - return Err(PolicyError::DaemonError { status, body }); - } - let view: PolicyView = resp.json().await?; - if json { - writeln!( - out, - "{}", - serde_json::to_string(&view.allow_inbound).unwrap() - )?; - } else if view.allow_inbound.is_empty() { - writeln!( - out, - "(allow_inbound is empty — every cross-daemon attach is denied)" - )?; - } else { - for entry in &view.allow_inbound { - writeln!(out, "{entry}")?; - } - } - Ok(()) -} - -/// `http://127.0.0.1:` where `` is `[daemon].bind_addr`'s port -/// or `7070` if unset / file missing. -fn local_base_url(config_path: &Path) -> Result { - let file = MaestroConfigFile::load_or_default(config_path)?; - let port = file - .daemon - .bind_addr - .map(|a| a.port()) - .unwrap_or(DEFAULT_PORT); - Ok(format!("http://127.0.0.1:{port}")) -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn local_base_url_uses_bind_port_or_default() { - let dir = tempfile::tempdir().unwrap(); - let path = dir.path().join("config.toml"); - // missing file → default - assert_eq!(local_base_url(&path).unwrap(), "http://127.0.0.1:7070"); - - std::fs::write( - &path, - r#" -[daemon] -bind_addr = "0.0.0.0:9090" -"#, - ) - .unwrap(); - assert_eq!(local_base_url(&path).unwrap(), "http://127.0.0.1:9090"); - } -} diff --git a/crates/maestro-config/src/lib.rs b/crates/maestro-config/src/lib.rs index 677767d..9cbe366 100644 --- a/crates/maestro-config/src/lib.rs +++ b/crates/maestro-config/src/lib.rs @@ -164,9 +164,6 @@ pub struct DaemonSection { /// enable `loginctl enable-linger`. #[serde(default, skip_serializing_if = "Option::is_none")] pub tmux_tmpdir: Option, - /// `[daemon.policy]` — per-daemon attach policy (NAP-247). - #[serde(default, skip_serializing_if = "DaemonPolicySection::is_empty")] - pub policy: DaemonPolicySection, } impl DaemonSection { @@ -181,23 +178,6 @@ impl DaemonSection { && self.private_addr.is_none() && self.pty_replay_bytes.is_none() && self.tmux_tmpdir.is_none() - && self.policy.is_empty() - } -} - -/// `[daemon.policy]` — application-layer access control complementing network -/// ACLs. Default deny: an empty `allow_inbound` rejects every cross-daemon -/// attach. Local attach (no JWT, same daemon) always bypasses. -#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] -pub struct DaemonPolicySection { - /// daemon-ids permitted to broker-attach into this daemon's sessions. - #[serde(default, skip_serializing_if = "Vec::is_empty")] - pub allow_inbound: Vec, -} - -impl DaemonPolicySection { - fn is_empty(&self) -> bool { - self.allow_inbound.is_empty() } } @@ -391,9 +371,6 @@ mod tests { private_addr: Some("10.0.0.7:7070".into()), pty_replay_bytes: Some(131_072), tmux_tmpdir: Some(PathBuf::from("/run/user/1000")), - policy: DaemonPolicySection { - allow_inbound: vec!["konishi".into()], - }, }, registry: RegistrySection { bind_addr: Some("0.0.0.0:8080".parse().unwrap()), diff --git a/crates/maestro-daemon/src/cli.rs b/crates/maestro-daemon/src/cli.rs index 924f27c..722dfa8 100644 --- a/crates/maestro-daemon/src/cli.rs +++ b/crates/maestro-daemon/src/cli.rs @@ -15,9 +15,9 @@ use crate::sidecar::SidecarStore; use crate::tmux::Tmux; use crate::worktree::WorktreeManager; use crate::{ - attach, default_config_path, default_state_dir, load_config, local_api, policy, run_configure, - spawn_session_publisher, spawn_sighup_listener, ConfigureInputs, EnvOverrides, JwtVerifier, - LocalEventBus, PairAttachVerifier, PolicyState, RegistryClient, SessionManager, + attach, default_config_path, default_state_dir, load_config, local_api, run_configure, + spawn_session_publisher, ConfigureInputs, EnvOverrides, JwtVerifier, LocalEventBus, + PairAttachVerifier, RegistryClient, SessionManager, }; #[derive(Debug, Subcommand)] @@ -113,11 +113,7 @@ async fn run(config_override: Option) -> Result<(), Box> { manager.start_liveness_watcher(DEFAULT_LIVENESS_INTERVAL); let verifier = JwtVerifier::new(&config.token, &config.daemon_id); - let pair_verifier = PairAttachVerifier::new( - &config.token, - &config.daemon_id, - config.allow_inbound.iter().cloned(), - ); + let pair_verifier = PairAttachVerifier::new(&config.token, &config.daemon_id); let events = LocalEventBus::new(); spawn_session_publisher(manager.clone(), events.clone()); @@ -151,16 +147,9 @@ async fn run(config_override: Option) -> Result<(), Box> { self_daemon_id: std::sync::Arc::new(config.daemon_id.clone()), self_perimeter_id: config.perimeter_id.clone().map(std::sync::Arc::new), }; - let policy_state = PolicyState { - verifier: pair_verifier.clone(), - config_path: std::sync::Arc::new(config_path.clone()), - }; - spawn_sighup_listener(policy_state.clone()); - let router = attach::router(manager.clone(), verifier, pair_verifier) .merge(local_api::router(local_state)) - .merge(local_api::broker_router(broker_state)) - .merge(policy::router(policy_state)); + .merge(local_api::broker_router(broker_state)); let listener = TcpListener::bind(config.bind_addr).await?; let bound = listener.local_addr()?; diff --git a/crates/maestro-daemon/src/config.rs b/crates/maestro-daemon/src/config.rs index 9123242..0b2b0f0 100644 --- a/crates/maestro-daemon/src/config.rs +++ b/crates/maestro-daemon/src/config.rs @@ -90,9 +90,6 @@ pub struct DaemonConfig { /// Daemon's reachable address on the declared perimeter. Only meaningful /// when `perimeter_id` is set. pub private_addr: Option, - /// daemon-ids permitted to broker-attach to this daemon's sessions. - /// Default empty (deny all). Local attach bypasses. (NAP-247.) - pub allow_inbound: Vec, /// Per-session PTY replay buffer cap in bytes (NAP-229). Each new attach /// receives this much of the prior PTY output before the live stream. /// Default `pty::DEFAULT_REPLAY_BYTES`; `0` disables. @@ -242,7 +239,6 @@ pub fn load_config(path: &Path, env: EnvOverrides) -> Result Result Result>`] so it can be swapped at runtime by -/// [`PairAttachVerifier::reload`] (NAP-237 hot reload via SIGHUP / -/// `POST /api/policy/reload`). Clones share the same inner — any reload -/// observed by one clone is observed by all of them. +/// `iss == REGISTRY_ISSUER`, `aud == self.daemon_id`, and `sid == session_id` +/// — i.e. the token was minted by the registry for *this* daemon and *this* +/// session, and hasn't expired. This is the session-binding guarantee the +/// network perimeter can't provide: a host the firewall admits still can't +/// attach to a session it wasn't handed, or replay a token to another +/// session/daemon. +/// +/// It does **not** gate on the home daemon's identity (no `allow_inbound` +/// allow-list, NAP-339): the operator's perimeter/firewall is the trust +/// boundary for *which hosts may connect*, so every daemon on the perimeter is +/// equally trusted to attach. Re-introducing a per-pair gate later is a +/// verify-time check, not an architecture change. #[derive(Clone)] pub struct PairAttachVerifier { decoding: DecodingKey, daemon_id: String, - allow_inbound: Arc>>, } impl std::fmt::Debug for PairAttachVerifier { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("PairAttachVerifier") .field("daemon_id", &self.daemon_id) - .field("allow_inbound", &self.snapshot()) .field("decoding", &"") .finish() } } impl PairAttachVerifier { - pub fn new( - secret: &str, - daemon_id: impl Into, - allow_inbound: impl IntoIterator, - ) -> Self { + pub fn new(secret: &str, daemon_id: impl Into) -> Self { Self { decoding: DecodingKey::from_secret(secret.as_bytes()), daemon_id: daemon_id.into(), - allow_inbound: Arc::new(RwLock::new(allow_inbound.into_iter().collect())), } } - /// Swap the effective `allow_inbound` set. Affects subsequent `verify` - /// calls; in-flight attaches keep their existing connection (per spec - /// — revocation requires killing the session). - pub fn reload(&self, allow_inbound: impl IntoIterator) { - let mut g = self - .allow_inbound - .write() - .unwrap_or_else(|e| e.into_inner()); - *g = allow_inbound.into_iter().collect(); - } - - /// Sorted snapshot of the current effective `allow_inbound` set. Used by - /// `GET /api/policy` and debug formatting. - pub fn snapshot(&self) -> Vec { - let g = self.allow_inbound.read().unwrap_or_else(|e| e.into_inner()); - let mut v: Vec = g.iter().cloned().collect(); - v.sort(); - v - } - /// Validate the token for the path-bound `session_id`. On success returns /// the home daemon's id (`sub`) for logging/tracing. pub fn verify(&self, token: &str, session_id: Uuid) -> Result { @@ -148,13 +123,6 @@ impl PairAttachVerifier { path: session_id, }); } - let allowed = { - let g = self.allow_inbound.read().unwrap_or_else(|e| e.into_inner()); - g.contains(&claims.sub) - }; - if !allowed { - return Err(JwtError::NotAllowed { sub: claims.sub }); - } Ok(claims.sub) } } @@ -230,42 +198,21 @@ mod tests { } #[test] - fn pair_verifier_accepts_allow_listed_home() { + fn pair_verifier_accepts_registry_minted_token() { let secret = "s"; - let v = PairAttachVerifier::new(secret, "konishi", ["laptop".into()]); + let v = PairAttachVerifier::new(secret, "konishi"); let sid = Uuid::new_v4(); + // The home daemon's identity is no longer gated (NAP-339) — any + // registry-minted, correctly-bound token for this daemon+session passes. let token = issue_pair(secret, &pair_claims("laptop", "konishi", sid, 60)); let sub = v.verify(&token, sid).unwrap(); assert_eq!(sub, "laptop"); } - #[test] - fn pair_verifier_rejects_unlisted_home() { - let secret = "s"; - let v = PairAttachVerifier::new(secret, "konishi", ["other".into()]); - let sid = Uuid::new_v4(); - let token = issue_pair(secret, &pair_claims("laptop", "konishi", sid, 60)); - let err = v.verify(&token, sid).unwrap_err(); - assert!(matches!(err, JwtError::NotAllowed { .. }), "{err:?}"); - } - - #[test] - fn pair_verifier_rejects_empty_allow_inbound() { - // Default deny: no peer listed → every cross-daemon attach fails. - let secret = "s"; - let v = PairAttachVerifier::new(secret, "konishi", std::iter::empty::()); - let sid = Uuid::new_v4(); - let token = issue_pair(secret, &pair_claims("laptop", "konishi", sid, 60)); - assert!(matches!( - v.verify(&token, sid).unwrap_err(), - JwtError::NotAllowed { .. } - )); - } - #[test] fn pair_verifier_rejects_wrong_audience() { let secret = "s"; - let v = PairAttachVerifier::new(secret, "konishi", ["laptop".into()]); + let v = PairAttachVerifier::new(secret, "konishi"); let sid = Uuid::new_v4(); // Token addressed to a different target — must not pass our checks. let token = issue_pair(secret, &pair_claims("laptop", "elsewhere", sid, 60)); @@ -278,7 +225,7 @@ mod tests { // `iss` (e.g. minted for some other purpose) must not validate as a // pair-attach credential. let secret = "s"; - let v = PairAttachVerifier::new(secret, "konishi", ["laptop".into()]); + let v = PairAttachVerifier::new(secret, "konishi"); let sid = Uuid::new_v4(); let mut claims = pair_claims("laptop", "konishi", sid, 60); claims.iss = "somebody-else".into(); @@ -289,7 +236,7 @@ mod tests { #[test] fn pair_verifier_rejects_session_id_mismatch() { let secret = "s"; - let v = PairAttachVerifier::new(secret, "konishi", ["laptop".into()]); + let v = PairAttachVerifier::new(secret, "konishi"); let sid = Uuid::new_v4(); let path = Uuid::new_v4(); let token = issue_pair(secret, &pair_claims("laptop", "konishi", sid, 60)); @@ -302,57 +249,17 @@ mod tests { #[test] fn pair_verifier_rejects_expired() { let secret = "s"; - let v = PairAttachVerifier::new(secret, "konishi", ["laptop".into()]); + let v = PairAttachVerifier::new(secret, "konishi"); let sid = Uuid::new_v4(); let token = issue_pair(secret, &pair_claims("laptop", "konishi", sid, -120)); assert!(v.verify(&token, sid).is_err()); } - // NAP-237: hot reload. After `reload`, the same verifier (and any - // pre-existing clone) sees the new set on the next `verify`. - #[test] - fn pair_verifier_reload_admits_new_peer() { - let secret = "s"; - let v = PairAttachVerifier::new(secret, "konishi", std::iter::empty::()); + fn pair_verifier_rejects_wrong_secret() { + let v = PairAttachVerifier::new("right", "konishi"); let sid = Uuid::new_v4(); - let token = issue_pair(secret, &pair_claims("laptop", "konishi", sid, 60)); + let token = issue_pair("wrong", &pair_claims("laptop", "konishi", sid, 60)); assert!(v.verify(&token, sid).is_err()); - v.reload(["laptop".into()]); - assert_eq!(v.verify(&token, sid).unwrap(), "laptop"); - } - - #[test] - fn pair_verifier_reload_revokes_removed_peer() { - let secret = "s"; - let v = PairAttachVerifier::new(secret, "konishi", ["laptop".into(), "phone".into()]); - let sid = Uuid::new_v4(); - let token = issue_pair(secret, &pair_claims("laptop", "konishi", sid, 60)); - assert_eq!(v.verify(&token, sid).unwrap(), "laptop"); - v.reload(["phone".into()]); // drop laptop - assert!(matches!( - v.verify(&token, sid).unwrap_err(), - JwtError::NotAllowed { .. } - )); - } - - #[test] - fn pair_verifier_clone_shares_inner_set() { - // Cloning the verifier (e.g. to hand to axum state and to the SIGHUP - // task) must NOT detach the allow_inbound set — both halves observe - // every reload. - let secret = "s"; - let v1 = PairAttachVerifier::new(secret, "konishi", std::iter::empty::()); - let v2 = v1.clone(); - v1.reload(["laptop".into()]); - let sid = Uuid::new_v4(); - let token = issue_pair(secret, &pair_claims("laptop", "konishi", sid, 60)); - assert_eq!(v2.verify(&token, sid).unwrap(), "laptop"); - } - - #[test] - fn pair_verifier_snapshot_is_sorted() { - let v = PairAttachVerifier::new("s", "konishi", ["phone".into(), "laptop".into()]); - assert_eq!(v.snapshot(), vec!["laptop".to_string(), "phone".into()]); } } diff --git a/crates/maestro-daemon/src/lib.rs b/crates/maestro-daemon/src/lib.rs index 1c4f1f6..6a08763 100644 --- a/crates/maestro-daemon/src/lib.rs +++ b/crates/maestro-daemon/src/lib.rs @@ -16,7 +16,6 @@ pub mod dirlist; pub mod jwt; pub mod local_api; pub mod local_events; -pub mod policy; pub mod pty; pub mod registry_client; pub mod session; @@ -30,6 +29,5 @@ pub use config::{default_config_path, default_state_dir, load_config, DaemonConf pub use configure::{run as run_configure, ConfigureError, ConfigureInputs}; pub use jwt::{issue as issue_jwt, JwtError, JwtVerifier, PairAttachVerifier}; pub use local_events::{spawn_session_publisher, LocalEventBus}; -pub use policy::{spawn_sighup_listener, PolicyState}; pub use registry_client::{ClientError, RegistryClient, HEARTBEAT_INTERVAL}; pub use session::{SessionError, SessionEvent, SessionManager}; diff --git a/crates/maestro-daemon/src/policy.rs b/crates/maestro-daemon/src/policy.rs deleted file mode 100644 index 6a1c35b..0000000 --- a/crates/maestro-daemon/src/policy.rs +++ /dev/null @@ -1,196 +0,0 @@ -//! Hot reload for `[daemon.policy] allow_inbound` (NAP-237). -//! -//! Two triggers, one execution path: -//! -//! - **SIGHUP** — `systemctl --user reload maestrod` (or any `kill -HUP`). -//! - **`POST /api/policy/reload`** — used by `maestro policy reload` so -//! operators don't need to know the PID. Also gives `maestro-web` and -//! other local-API consumers a uniform reload hook. -//! -//! Both re-read `~/.config/maestro/config.toml` and swap the verifier's -//! allow-list atomically; in-flight attaches keep their existing connection -//! (revoking those requires killing the session, per spec). - -use std::path::{Path, PathBuf}; -use std::sync::Arc; - -use axum::extract::State; -use axum::http::StatusCode; -use axum::routing::{get, post}; -use axum::{Json, Router}; -use maestro_config::MaestroConfigFile; -use serde::Serialize; - -use crate::jwt::PairAttachVerifier; - -#[derive(Debug, Clone)] -pub struct PolicyState { - pub verifier: PairAttachVerifier, - pub config_path: Arc, -} - -#[derive(Debug, Serialize)] -pub struct PolicyView { - pub allow_inbound: Vec, -} - -pub fn router(state: PolicyState) -> Router { - Router::new() - .route("/api/policy", get(get_policy)) - .route("/api/policy/reload", post(reload_policy)) - .with_state(state) -} - -async fn get_policy(State(s): State) -> Json { - Json(PolicyView { - allow_inbound: s.verifier.snapshot(), - }) -} - -async fn reload_policy( - State(s): State, -) -> Result, (StatusCode, String)> { - match reload_from_file(&s.config_path, &s.verifier) { - Ok(list) => { - tracing::info!(?list, source = "http", "reloaded allow_inbound"); - Ok(Json(PolicyView { - allow_inbound: list, - })) - } - Err(e) => { - tracing::error!(error = %e, source = "http", "policy reload failed"); - Err((StatusCode::INTERNAL_SERVER_ERROR, e.to_string())) - } - } -} - -/// Re-read the unified config file and apply `[daemon.policy].allow_inbound` -/// to the live verifier. Returns the new effective list (sorted), or an -/// error if the file can't be read/parsed — in which case the existing -/// allow-list stays in place so a bad edit doesn't lock everyone out. -pub fn reload_from_file( - config_path: &Path, - verifier: &PairAttachVerifier, -) -> Result, maestro_config::ConfigError> { - let file = MaestroConfigFile::load(config_path)?; - let list = file.daemon.policy.allow_inbound; - verifier.reload(list.iter().cloned()); - Ok(verifier.snapshot()) -} - -/// Spawn a tokio task that re-applies the policy on every SIGHUP. Logs -/// success / failure but never exits — a transient bad config (e.g. mid-edit) -/// becomes a logged error, not a daemon kill. -pub fn spawn_sighup_listener(state: PolicyState) { - tokio::spawn(async move { - let mut sig = match tokio::signal::unix::signal(tokio::signal::unix::SignalKind::hangup()) { - Ok(s) => s, - Err(e) => { - tracing::error!(error = %e, "failed to install SIGHUP handler"); - return; - } - }; - while sig.recv().await.is_some() { - match reload_from_file(&state.config_path, &state.verifier) { - Ok(list) => { - tracing::info!(?list, source = "sighup", "reloaded allow_inbound"); - } - Err(e) => { - tracing::error!(error = %e, source = "sighup", "policy reload failed"); - } - } - } - }); -} - -#[cfg(test)] -mod tests { - use super::*; - - fn write_config(path: &Path, allow_inbound: &[&str]) { - let body = if allow_inbound.is_empty() { - r#" -[shared] -registry_url = "http://r" -token = "t" - -[daemon] -daemon_url = "ws://x" -"# - .to_string() - } else { - let list = allow_inbound - .iter() - .map(|s| format!("\"{s}\"")) - .collect::>() - .join(", "); - format!( - r#" -[shared] -registry_url = "http://r" -token = "t" - -[daemon] -daemon_url = "ws://x" - -[daemon.policy] -allow_inbound = [{list}] -"# - ) - }; - std::fs::write(path, body).unwrap(); - } - - #[test] - fn reload_from_file_picks_up_new_entries() { - let dir = tempfile::tempdir().unwrap(); - let path = dir.path().join("config.toml"); - write_config(&path, &["laptop"]); - let v = PairAttachVerifier::new("s", "konishi", ["laptop".into()]); - assert_eq!(v.snapshot(), vec!["laptop".to_string()]); - - write_config(&path, &["laptop", "phone"]); - let list = reload_from_file(&path, &v).unwrap(); - assert_eq!(list, vec!["laptop".to_string(), "phone".into()]); - assert_eq!(v.snapshot(), vec!["laptop".to_string(), "phone".into()]); - } - - #[test] - fn reload_from_file_drops_removed_entries() { - let dir = tempfile::tempdir().unwrap(); - let path = dir.path().join("config.toml"); - write_config(&path, &["laptop", "phone"]); - let v = PairAttachVerifier::new("s", "konishi", ["laptop".into(), "phone".into()]); - - write_config(&path, &["phone"]); - let list = reload_from_file(&path, &v).unwrap(); - assert_eq!(list, vec!["phone".to_string()]); - assert_eq!(v.snapshot(), vec!["phone".to_string()]); - } - - #[test] - fn reload_from_file_leaves_set_intact_on_parse_error() { - // A mid-edit broken config must not lock everyone out — keep the - // previously-effective list and surface the error to the caller. - let dir = tempfile::tempdir().unwrap(); - let path = dir.path().join("config.toml"); - write_config(&path, &["laptop"]); - let v = PairAttachVerifier::new("s", "konishi", ["laptop".into()]); - std::fs::write(&path, "this = is = not = toml").unwrap(); - assert!(reload_from_file(&path, &v).is_err()); - assert_eq!(v.snapshot(), vec!["laptop".to_string()]); - } - - #[test] - fn reload_from_file_treats_missing_section_as_deny_all() { - let dir = tempfile::tempdir().unwrap(); - let path = dir.path().join("config.toml"); - write_config(&path, &["laptop"]); - let v = PairAttachVerifier::new("s", "konishi", ["laptop".into()]); - - write_config(&path, &[]); // no [daemon.policy] at all - let list = reload_from_file(&path, &v).unwrap(); - assert!(list.is_empty()); - assert!(v.snapshot().is_empty()); - } -} diff --git a/crates/maestro-daemon/tests/attach.rs b/crates/maestro-daemon/tests/attach.rs index cd48eb4..1a26deb 100644 --- a/crates/maestro-daemon/tests/attach.rs +++ b/crates/maestro-daemon/tests/attach.rs @@ -36,7 +36,7 @@ async fn spawn_daemon_ws(daemon_id: &str) -> Harness { let fx = DaemonFixture::new(daemon_id); let manager = fx.manager.clone(); let verifier = JwtVerifier::new(TOKEN, daemon_id); - let pair_verifier = PairAttachVerifier::new(TOKEN, daemon_id, std::iter::empty::()); + let pair_verifier = PairAttachVerifier::new(TOKEN, daemon_id); let router = attach::router(manager.clone(), verifier, pair_verifier); let listener = TcpListener::bind(SocketAddr::from((Ipv4Addr::LOCALHOST, 0))) .await diff --git a/crates/maestro-e2e/src/daemon.rs b/crates/maestro-e2e/src/daemon.rs index d706be8..d48a23b 100644 --- a/crates/maestro-e2e/src/daemon.rs +++ b/crates/maestro-e2e/src/daemon.rs @@ -27,13 +27,12 @@ pub struct Daemon { token: String, } -/// Optional knobs for `Daemon::spawn_with` — perimeter + allow_inbound for +/// Optional knobs for `Daemon::spawn_with` — perimeter advertisement for /// NAP-247 broker tests. #[derive(Debug, Default, Clone)] pub struct DaemonOptions { pub perimeter_id: Option, pub private_addr: Option, - pub allow_inbound: Vec, } impl Daemon { @@ -50,7 +49,7 @@ impl Daemon { .await } - /// Spawn with NAP-247 options (perimeter advertisement, allow_inbound). + /// Spawn with NAP-247 options (perimeter advertisement). pub async fn spawn_with( daemon_id: &str, registry_base_url: &str, @@ -116,11 +115,6 @@ impl Daemon { let state = TempDir::new().expect("state tmpdir"); let tmux = TempDir::new().expect("tmux tmpdir"); - // Write a minimal config.toml so allow_inbound (and any future - // file-only fields) reach the daemon. Required fields keep coming - // through env vars set by `launch`. - write_config(home.path(), &opts); - let daemon_url = format!("ws://{bind}"); let child = launch( daemon_id, @@ -219,25 +213,3 @@ fn launch( cmd.spawn() .unwrap_or_else(|e| panic!("spawn {} failed: {e}", bin.display())) } - -/// Write a minimal `~/.config/maestro/config.toml` carrying the NAP-247 -/// `[daemon.policy] allow_inbound` list. Everything else still comes from -/// env vars set by `launch`. -fn write_config(home: &std::path::Path, opts: &DaemonOptions) { - if opts.allow_inbound.is_empty() { - return; - } - let dir = home.join(".config/maestro"); - std::fs::create_dir_all(&dir).unwrap(); - let mut allow = String::new(); - for (i, id) in opts.allow_inbound.iter().enumerate() { - if i > 0 { - allow.push_str(", "); - } - allow.push('"'); - allow.push_str(id); - allow.push('"'); - } - let body = format!("[daemon.policy]\nallow_inbound = [{allow}]\n"); - std::fs::write(dir.join("config.toml"), body).unwrap(); -} diff --git a/crates/maestro-e2e/tests/e2e.rs b/crates/maestro-e2e/tests/e2e.rs index 2f66bd5..b3abb80 100644 --- a/crates/maestro-e2e/tests/e2e.rs +++ b/crates/maestro-e2e/tests/e2e.rs @@ -513,7 +513,6 @@ async fn broker_t2_attach_through_home_daemon() { DaemonOptions { perimeter_id: Some("wg-test".into()), private_addr: Some(beta_bind.to_string()), - allow_inbound: vec!["alpha".into()], }, ) .await; @@ -529,7 +528,6 @@ async fn broker_t2_attach_through_home_daemon() { DaemonOptions { perimeter_id: Some("wg-test".into()), private_addr: Some(alpha_bind.to_string()), - allow_inbound: vec![], }, ) .await; @@ -621,116 +619,6 @@ async fn broker_t2_attach_through_home_daemon() { drop(registry); } -#[tokio::test(flavor = "multi_thread")] -async fn broker_rejects_when_target_doesnt_allow_inbound() { - // NAP-247: beta has an empty allow_inbound, so beta's - // /sessions/:id/stream rejects alpha's broker JWT even though the - // registry happily mints one (default deny is target-side only). - let registry = Registry::spawn("e2e-shared-token").await; - let beta_bind = std::net::SocketAddr::from(( - std::net::Ipv4Addr::LOCALHOST, - maestro_e2e::util::free_port(), - )); - let beta = Daemon::spawn_at( - "beta", - ®istry.http_base(), - "e2e-shared-token", - beta_bind, - DaemonOptions { - perimeter_id: Some("wg-test".into()), - private_addr: Some(beta_bind.to_string()), - // Default deny — alpha not listed. - allow_inbound: vec![], - }, - ) - .await; - let alpha_bind = std::net::SocketAddr::from(( - std::net::Ipv4Addr::LOCALHOST, - maestro_e2e::util::free_port(), - )); - let alpha = Daemon::spawn_at( - "alpha", - ®istry.http_base(), - "e2e-shared-token", - alpha_bind, - DaemonOptions { - perimeter_id: Some("wg-test".into()), - private_addr: Some(alpha_bind.to_string()), - allow_inbound: vec![], - }, - ) - .await; - assert_eq!(beta.bind_addr, beta_bind); - assert_eq!(alpha.bind_addr, alpha_bind); - - let client = TestClient::new(®istry.http_base(), ®istry.ws_base()); - client.bootstrap().await.expect("bootstrap"); - wait_for_async("both daemons connected", Duration::from_secs(10), || { - let client = client.clone(); - async move { - client - .list_daemons() - .await - .map(|ds| ds.len() >= 2) - .unwrap_or(false) - } - }) - .await; - - let cwd = TempDir::new().unwrap(); - let session = client - .spawn_session(&create_req("beta", cwd.path(), "sleep 30")) - .await - .expect("spawn beta"); - - let alpha_base = format!("http://{}", alpha.bind_addr); - let sid = session.id; - wait_for_async( - "alpha mirrors beta's session", - Duration::from_secs(5), - || { - let base = alpha_base.clone(); - async move { - let resp = match reqwest::get(format!("{base}/api/sessions")).await { - Ok(r) => r, - Err(_) => return false, - }; - let list: Vec = match resp.json().await { - Ok(v) => v, - Err(_) => return false, - }; - list.iter().any(|s| s.id == sid) - } - }, - ) - .await; - - let broker_url = format!("ws://{}/api/attach/{}", alpha.bind_addr, sid); - // NAP-265: beta rejects the pair JWT (empty allow_inbound) pre-upgrade. - // alpha's broker now dials the peer *before* completing the upgrade, so the - // client's handshake fails with a structured 502 rather than succeeding and - // then closing opaquely. - let err = tokio_tungstenite::connect_async(broker_url) - .await - .expect_err("handshake must fail when the peer rejects the attach"); - match err { - tokio_tungstenite::tungstenite::Error::Http(resp) => { - assert_eq!( - resp.status(), - 502, - "expected BAD_GATEWAY from alpha's broker, got {}", - resp.status() - ); - } - other => panic!("expected an HTTP 502 response, got {other:?}"), - } - - client.kill_session(sid).await.expect("kill"); - drop(alpha); - drop(beta); - drop(registry); -} - #[tokio::test(flavor = "multi_thread")] async fn broker_attach_returns_502_on_perimeter_mismatch() { // NAP-270: the asymmetric-VLAN boundary. alpha and beta are on DIFFERENT @@ -749,7 +637,6 @@ async fn broker_attach_returns_502_on_perimeter_mismatch() { DaemonOptions { perimeter_id: Some("wg-beta".into()), private_addr: Some(beta_bind.to_string()), - allow_inbound: vec!["alpha".into()], }, ) .await; @@ -766,7 +653,6 @@ async fn broker_attach_returns_502_on_perimeter_mismatch() { // Different perimeter from beta → T2 unavailable. perimeter_id: Some("wg-alpha".into()), private_addr: Some(alpha_bind.to_string()), - allow_inbound: vec![], }, ) .await; diff --git a/crates/maestro-installer/src/systemd.rs b/crates/maestro-installer/src/systemd.rs index 4dc4e80..fe9a358 100644 --- a/crates/maestro-installer/src/systemd.rs +++ b/crates/maestro-installer/src/systemd.rs @@ -57,7 +57,6 @@ pub fn render_user_unit(exec_start: &Path) -> String { [Service]\n\ Type=simple\n\ ExecStart={exec} daemon\n\ - ExecReload=/bin/kill -HUP $MAINPID\n\ # Signal only the daemon on stop/restart, not its whole cgroup —\n\ # otherwise systemd kills the tmux server (and every session) the\n\ # daemon spawned, defeating tmux persistence. recover() re-adopts the\n\ @@ -165,11 +164,13 @@ mod tests { } #[test] - fn unit_wires_execreload_to_sighup() { - // NAP-237: `systemctl --user reload maestro` must trigger our - // SIGHUP handler so operators can hot-reload `[daemon.policy]`. + fn unit_has_no_execreload() { + // NAP-339 removed the daemon's SIGHUP handler (it only ever reloaded + // the now-deleted `[daemon.policy]`). Without a handler, SIGHUP's + // default disposition is to terminate, so the unit must NOT wire + // `systemctl reload` to it — that would kill the daemon. let body = render_user_unit(Path::new("/x/maestro")); - assert!(body.contains("ExecReload=/bin/kill -HUP $MAINPID")); + assert!(!body.contains("ExecReload")); } #[test] diff --git a/crates/maestro-protocol/src/lib.rs b/crates/maestro-protocol/src/lib.rs index cccc23f..24e1a24 100644 --- a/crates/maestro-protocol/src/lib.rs +++ b/crates/maestro-protocol/src/lib.rs @@ -252,8 +252,8 @@ pub enum DaemonInbound { result: RpcResult, }, /// Registry signals a target daemon that a home daemon wants to attach via - /// the registry-mediated relay (T3). Target evaluates `allow_inbound` and - /// replies with [`DaemonOutbound::RelayAck`]. + /// the registry-mediated relay (T3). Target decides whether to accept the + /// relay and replies with [`DaemonOutbound::RelayAck`]. RelaySetup { reply_to: ReplyTo, home_daemon: String, @@ -465,7 +465,8 @@ pub const REGISTRY_ISSUER: &str = "maestro-registry"; /// Pair-scoped attach credential. Issued by the registry inside /// [`AttachCandidates::jwt`]; verified by the target daemon for direct -/// transports (T1/T2) and gated by `allow_inbound` after JWT verification. +/// transports (T1/T2). Binds the token to a target daemon and session; the +/// home daemon's identity (`sub`) is not gated against an allow-list (NAP-339). /// /// "Pair-scoped" means: `sub` (home daemon) may initiate one attach to /// `aud` (target daemon) for session `sid`, valid until `exp`. A stolen diff --git a/crates/maestro-registry/src/jwt.rs b/crates/maestro-registry/src/jwt.rs index 73b26c0..93447ca 100644 --- a/crates/maestro-registry/src/jwt.rs +++ b/crates/maestro-registry/src/jwt.rs @@ -38,8 +38,10 @@ pub fn issue_attach_jwt( /// Mint a pair-scoped attach JWT used by the home-daemon broker (NAP-247). /// `sub` is the home daemon-id; `aud` is the target daemon-id. Target -/// validates signature, `aud == self.daemon_id`, `exp > now`, and -/// `sub in allow_inbound` before accepting the broker-attach. +/// validates signature, `iss == REGISTRY_ISSUER`, `aud == self.daemon_id`, +/// `sid == session`, and `exp > now` before accepting the broker-attach. +/// `sub` is no longer gated against an allow-list (NAP-339) — it's retained +/// for logging/tracing. pub fn issue_pair_attach_jwt( secret: &str, home_daemon_id: &str, diff --git a/crates/maestro/src/main.rs b/crates/maestro/src/main.rs index 250f4ce..1223cfb 100644 --- a/crates/maestro/src/main.rs +++ b/crates/maestro/src/main.rs @@ -69,10 +69,6 @@ enum Command { /// Set the default registry URL persisted in the CLI config. #[cfg(feature = "cli")] SetRegistry(maestro_cli::cli::SetRegistryArgs), - /// Inspect or hot-reload the local daemon's `[daemon.policy]` (NAP-237). - #[cfg(feature = "cli")] - #[command(subcommand)] - Policy(maestro_cli::cli::PolicyCmd), /// Per-host daemon: tmux wrapper + registry client. #[cfg(feature = "daemon")] @@ -160,16 +156,6 @@ async fn dispatch(cli: Cli) -> Result<(), Box> { ) .await? } - #[cfg(feature = "cli")] - Command::Policy(sub) => { - cli_dispatch( - maestro_cli::cli::Command::Policy(sub), - cli.config, - cli.registry_url, - cli.home_daemon_url, - ) - .await? - } #[cfg(feature = "daemon")] Command::Daemon { cmd } => maestro_daemon::cli::dispatch(cli.config, cmd).await?, #[cfg(feature = "registry")] @@ -220,7 +206,6 @@ fn is_cli_subcommand(command: &Command) -> bool { | Command::Attach(_) | Command::Kill(_) | Command::SetRegistry(_) - | Command::Policy(_) ) } diff --git a/docs/INSTALL.md b/docs/INSTALL.md index 677be4a..91807ab 100644 --- a/docs/INSTALL.md +++ b/docs/INSTALL.md @@ -190,20 +190,16 @@ Pick `/tmp` if the daemon must outlive your login session and you can't run tmux sockets with it). The env var `MAESTRO_DAEMON_TMUX_TMPDIR` takes precedence over the file. -## Hot-reloading policy +## Peer attach trust model -`[daemon.policy].allow_inbound` (the list of peer daemons allowed to broker- -attach into this one) can be changed without restarting the daemon — restarting -would kill every live session. Three ways to apply an edit: - -```sh -maestro policy reload # POSTs to the local daemon's /api/policy/reload -maestro policy show # prints the current effective list -kill -HUP # equivalent; what `systemctl --user reload` does -``` - -Only newly-arriving attach attempts see the change — sessions already -attached stay attached. To boot a live attacher out, kill the session. +There is no per-peer allow-list. The operator's network perimeter (VPN/firewall) +is the trust boundary for *which hosts can connect*; every daemon reachable on +the perimeter may broker-attach to any other. Attach is still authenticated: the +registry mints a short-lived, pair-scoped JWT bound to the target daemon and the +specific session (`iss`/`aud`/`sid`/`exp`), which the target verifies — so a +reachable host still can't attach to a session it wasn't handed, or replay a +token elsewhere. (NAP-339 dropped the earlier `[daemon.policy].allow_inbound` +gate; re-introducing a per-pair allow-list later is a verify-time change.) ## What the systemd unit looks like