From 028ebbfafcd90ff5050c089a8f41392c1354b804 Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Wed, 16 Sep 2026 02:55:36 -0400 Subject: [PATCH] refactor(daemon): make the token permission check assertable, and cover it Every mutant the gate generated for check_token_permissions survived, and the reason was structural rather than an oversight in the tests: the function's only output was a tracing::warn!. There was no value to assert, so `!path.exists()`, the `& 0o777` mask and the `!= 0o600` comparison could all be flipped without any test noticing. This splits the decision from the reporting. token_permissions() returns Absent / Secure / Insecure{mode} / Unknown, and check_token_permissions() keeps its signature and simply warns on Insecure, so the single caller in commands/server.rs is untouched. Behaviour is unchanged: the same condition produces the same warning. Unknown is a deliberate fourth variant rather than folding a failed metadata read into Secure. The original silently skipped the check when metadata could not be read, which is the right call - an unreadable file is not evidence of a bad mode - but calling that "secure" would be a lie in the type. All 7 mutants were hand-applied and the suite confirmed to fail: 7 killed, 0 survived. The mask mutations are the ones worth noting. A file created 0644 has a raw mode of 0o100644 including the file-type bits: masking gives 0o644, `^` gives 0o100133 and `|` gives 0o100777. All three are "insecure", so a test asserting only the variant would miss both mutations. The tests assert the mode value that comes back, which is what separates them. validate_bearer gains coverage in the same pass, including the cases that distinguish a prefix check from an equality: a token that is a prefix of the expected one, a token that extends it, and a header whose prefix is "Bearer" without the trailing space. --- apps/daemon/src/ipc/auth.rs | 114 +++++++++++---------- apps/daemon/src/ipc/auth/tests.rs | 159 ++++++++++++++++++++++++++++++ 2 files changed, 214 insertions(+), 59 deletions(-) create mode 100644 apps/daemon/src/ipc/auth/tests.rs diff --git a/apps/daemon/src/ipc/auth.rs b/apps/daemon/src/ipc/auth.rs index f3df343..5f6d056 100644 --- a/apps/daemon/src/ipc/auth.rs +++ b/apps/daemon/src/ipc/auth.rs @@ -55,78 +55,74 @@ pub fn validate_bearer(header_value: &str, expected_token: &str) -> bool { .unwrap_or(false) } -/// Check that the auth token file has secure permissions (DC.T42). +/// The only permission bits the auth token file may carry: owner read/write. +pub const SECURE_TOKEN_MODE: u32 = 0o600; + +/// What an inspection of the auth token file's permissions found. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum TokenPermissions { + /// No token file exists yet, so there is nothing to check. + Absent, + /// Exactly owner read/write. + Secure, + /// Reachable by someone other than the owner; carries the offending mode. + Insecure { mode: u32 }, + /// The file exists but its metadata could not be read, or the platform has + /// no Unix permission bits. Not evidence of a bad mode either way. + Unknown, +} + +/// Inspect the permissions on the auth token file (DC.T42). /// -/// On Unix, warns if the file is not exclusively owner read/write (0o600). -/// No automatic correction is made — the user must run `chmod 0600 `. -pub fn check_token_permissions(data_dir: &Path) { +/// Split out from `check_token_permissions` so the decision can be asserted: +/// a function whose only output is a log line cannot be tested, and every +/// mutant the gate generated for the combined version survived for that +/// reason. +pub fn token_permissions(data_dir: &Path) -> TokenPermissions { let path = data_dir.join("auth_token"); if !path.exists() { - return; + return TokenPermissions::Absent; } #[cfg(unix)] { use std::os::unix::fs::PermissionsExt; - if let Ok(meta) = std::fs::metadata(&path) { - let mode = meta.permissions().mode() & 0o777; - if mode != 0o600 { - tracing::warn!( - path = %path.display(), - mode = format!("{:04o}", mode), - "auth_token file has insecure permissions (expected 0600). \ - Run: chmod 0600 {}", - path.display() - ); + match std::fs::metadata(&path) { + Ok(meta) => { + // Mask off the file-type bits; only the permission bits matter. + let mode = meta.permissions().mode() & 0o777; + if mode == SECURE_TOKEN_MODE { + TokenPermissions::Secure + } else { + TokenPermissions::Insecure { mode } + } } + Err(_) => TokenPermissions::Unknown, } } -} - -#[cfg(test)] -mod tests { - use super::*; - use tempfile::TempDir; - - #[test] - fn test_validate_bearer_valid() { - assert!(validate_bearer("Bearer secret123", "secret123")); - } - - #[test] - fn test_validate_bearer_invalid() { - assert!(!validate_bearer("Bearer wrong", "secret123")); - assert!(!validate_bearer("secret123", "secret123")); - assert!(!validate_bearer("", "secret123")); - } - - #[test] - fn test_get_or_create_token_creates_file() { - let dir = TempDir::new().unwrap(); - let token = get_or_create_token(dir.path()).unwrap(); - assert_eq!(token.len(), 32, "token should be 32 hex chars"); - assert!(dir.path().join("auth_token").exists()); - } - - #[test] - fn test_get_or_create_token_idempotent() { - let dir = TempDir::new().unwrap(); - let t1 = get_or_create_token(dir.path()).unwrap(); - let t2 = get_or_create_token(dir.path()).unwrap(); - assert_eq!(t1, t2, "second call should return same token"); + #[cfg(not(unix))] + { + TokenPermissions::Unknown } +} - #[cfg(unix)] - #[test] - fn test_auth_token_created_with_0600_permissions() { - use std::os::unix::fs::PermissionsExt; - let dir = TempDir::new().unwrap(); - get_or_create_token(dir.path()).unwrap(); - let meta = std::fs::metadata(dir.path().join("auth_token")).unwrap(); - let mode = meta.permissions().mode() & 0o777; - assert_eq!( - mode, 0o600, - "auth_token must have mode 0600, got {mode:04o}" +/// Check that the auth token file has secure permissions (DC.T42). +/// +/// On Unix, warns if the file is not exclusively owner read/write (0o600). +/// No automatic correction is made — the user must run `chmod 0600 `. +pub fn check_token_permissions(data_dir: &Path) { + if let TokenPermissions::Insecure { mode } = token_permissions(data_dir) { + let path = data_dir.join("auth_token"); + tracing::warn!( + path = %path.display(), + mode = format!("{mode:04o}"), + "auth_token file has insecure permissions (expected 0600). \ + Run: chmod 0600 {}", + path.display() ); } } + +// Tests live in auth/tests.rs. +#[cfg(test)] +mod tests; diff --git a/apps/daemon/src/ipc/auth/tests.rs b/apps/daemon/src/ipc/auth/tests.rs new file mode 100644 index 0000000..426b3c4 --- /dev/null +++ b/apps/daemon/src/ipc/auth/tests.rs @@ -0,0 +1,159 @@ +//! Tests for the IPC auth helpers. +//! +//! The permission checks are written against the surviving-mutant list from +//! the mutation gate. Every mutant in the old `check_token_permissions` +//! survived because its only output was a `tracing::warn!`; splitting the +//! decision into `token_permissions` is what makes them reachable. + +use super::*; + +#[cfg(unix)] +fn write_token(dir: &std::path::Path, mode: u32) -> std::path::PathBuf { + use std::os::unix::fs::PermissionsExt; + let path = dir.join("auth_token"); + std::fs::write(&path, "secret").unwrap(); + std::fs::set_permissions(&path, std::fs::Permissions::from_mode(mode)).unwrap(); + path +} + +// ─── token_permissions ─────────────────────────────────────────────────────── + +/// Kills the `delete !` mutation on `!path.exists()`. +/// +/// With the `!` dropped, an absent file falls through to the metadata read, +/// which fails, and the answer becomes `Unknown` instead of `Absent` — the +/// daemon would stop distinguishing "no token yet" from "cannot tell". +#[test] +fn an_absent_token_file_reports_absent() { + let dir = tempfile::tempdir().unwrap(); + assert_eq!(token_permissions(dir.path()), TokenPermissions::Absent); +} + +/// Kills `mode == SECURE_TOKEN_MODE` -> `!=`. +/// +/// The one mode that must be accepted is 0600. Inverting the comparison +/// reports the single correct mode as insecure and every wrong one as fine, +/// which is the worst possible direction for this check. +#[cfg(unix)] +#[test] +fn a_token_file_at_0600_is_secure() { + let dir = tempfile::tempdir().unwrap(); + write_token(dir.path(), 0o600); + assert_eq!(token_permissions(dir.path()), TokenPermissions::Secure); +} + +/// Kills `& 0o777` -> `^ 0o777` and `| 0o777`, by asserting the exact mode +/// carried back rather than merely that the file was judged insecure. +/// +/// A file created 0644 has a raw mode of 0o100644 including the file-type +/// bits. Masking gives 0o644; xor gives 0o100133 and or gives 0o100777. All +/// three are "insecure", so a test that only checked the variant would miss +/// both mutations — it is the mode value that separates them. +#[cfg(unix)] +#[test] +fn an_insecure_token_file_reports_its_masked_mode() { + let dir = tempfile::tempdir().unwrap(); + write_token(dir.path(), 0o644); + assert_eq!( + token_permissions(dir.path()), + TokenPermissions::Insecure { mode: 0o644 } + ); +} + +/// Every mode that is not exactly 0600 must be refused, including ones that +/// are *more* restrictive — the check is an equality, not an upper bound. +#[cfg(unix)] +#[test] +fn every_mode_other_than_0600_is_insecure() { + for mode in [0o400, 0o604, 0o640, 0o660, 0o666, 0o700, 0o777] { + let dir = tempfile::tempdir().unwrap(); + write_token(dir.path(), mode); + assert_eq!( + token_permissions(dir.path()), + TokenPermissions::Insecure { mode }, + "mode {mode:04o} should be reported insecure, carrying its own value" + ); + } +} + +/// The warning wrapper must not panic on any of the four outcomes. It has no +/// return value to assert, so this pins only that it stays total. +#[test] +fn the_warning_wrapper_handles_every_outcome() { + let dir = tempfile::tempdir().unwrap(); + check_token_permissions(dir.path()); // Absent + #[cfg(unix)] + { + write_token(dir.path(), 0o600); + check_token_permissions(dir.path()); // Secure + write_token(dir.path(), 0o644); + check_token_permissions(dir.path()); // Insecure + } +} + +// ─── validate_bearer ───────────────────────────────────────────────────────── + +/// The prefix must match exactly, including its trailing space, and the token +/// comparison must be an equality over the whole remainder. +#[test] +fn only_an_exact_bearer_token_validates() { + assert!(validate_bearer("Bearer secret123", "secret123")); + + // Wrong token, prefix of the token, and superstring of the token. + assert!(!validate_bearer("Bearer secret124", "secret123")); + assert!(!validate_bearer("Bearer secret12", "secret123")); + assert!(!validate_bearer("Bearer secret1234", "secret123")); + + // Missing, mis-cased, or malformed prefix. + assert!(!validate_bearer("secret123", "secret123")); + assert!(!validate_bearer("bearer secret123", "secret123")); + assert!(!validate_bearer("Bearer secret123", "secret123")); + assert!(!validate_bearer("Bearersecret123", "secret123")); + assert!(!validate_bearer("", "secret123")); +} + +// ─── pre-existing tests, kept verbatim ────────────────────────────────────── + +use tempfile::TempDir; + +#[test] +fn test_validate_bearer_valid() { + assert!(validate_bearer("Bearer secret123", "secret123")); +} + +#[test] +fn test_validate_bearer_invalid() { + assert!(!validate_bearer("Bearer wrong", "secret123")); + assert!(!validate_bearer("secret123", "secret123")); + assert!(!validate_bearer("", "secret123")); +} + +#[test] +fn test_get_or_create_token_creates_file() { + let dir = TempDir::new().unwrap(); + let token = get_or_create_token(dir.path()).unwrap(); + assert_eq!(token.len(), 32, "token should be 32 hex chars"); + assert!(dir.path().join("auth_token").exists()); +} + +#[test] +fn test_get_or_create_token_idempotent() { + let dir = TempDir::new().unwrap(); + let t1 = get_or_create_token(dir.path()).unwrap(); + let t2 = get_or_create_token(dir.path()).unwrap(); + assert_eq!(t1, t2, "second call should return same token"); +} + +#[cfg(unix)] +#[test] +fn test_auth_token_created_with_0600_permissions() { + use std::os::unix::fs::PermissionsExt; + let dir = TempDir::new().unwrap(); + get_or_create_token(dir.path()).unwrap(); + let meta = std::fs::metadata(dir.path().join("auth_token")).unwrap(); + let mode = meta.permissions().mode() & 0o777; + assert_eq!( + mode, 0o600, + "auth_token must have mode 0600, got {mode:04o}" + ); +}