diff --git a/hive-priv/src/main.rs b/hive-priv/src/main.rs index 3cb86d13..2b819c7c 100644 --- a/hive-priv/src/main.rs +++ b/hive-priv/src/main.rs @@ -1724,21 +1724,37 @@ fn redact_secret_line(line: &str) -> std::borrow::Cow<'_, str> { if line.to_ascii_lowercase().contains("password") { return std::borrow::Cow::Borrowed("[redacted: line mentions a password]"); } - if line.split_whitespace().any(looks_like_secret) { + if contains_secret_shaped_run(line) { return std::borrow::Cow::Borrowed("[redacted: line contains a secret-shaped token]"); } std::borrow::Cow::Borrowed(line) } -/// True when a whitespace-delimited word looks like an opaque credential: -/// at least 32 characters drawn only from the hex / base64url alphabet. -/// 32 is below forgejo's 40-hex access token and above the ordinary -/// English words and file paths that appear in `forgejo admin` output. -fn looks_like_secret(word: &str) -> bool { - word.len() >= 32 - && word - .bytes() - .all(|b| b.is_ascii_alphanumeric() || matches!(b, b'+' | b'/' | b'=' | b'_' | b'-')) +/// True when the line contains an unbroken run of at least 32 characters +/// from the hex / base64url alphabet. 32 sits below forgejo's 40-hex +/// access token and above the ordinary words and path segments that +/// appear in `forgejo admin` output. +/// +/// ⚠️ Scans for a RUN, not for a whitespace-delimited word. An earlier +/// version split on whitespace and required the whole word to match, +/// which a secret with punctuation glued to it defeats: `","` and +/// `"[]"` both fail an all-chars check on the word while still +/// containing the credential in full. Whitespace is not what delimits a +/// secret — the alphabet is (thanks @argus for catching it). +fn contains_secret_shaped_run(line: &str) -> bool { + const MIN: usize = 32; + let mut run = 0usize; + for b in line.bytes() { + if b.is_ascii_alphanumeric() || matches!(b, b'+' | b'/' | b'=' | b'_' | b'-') { + run += 1; + if run >= MIN { + return true; + } + } else { + run = 0; + } + } + false } /// Run `forgejo admin ` inside the `hive-forge` container as the @@ -2522,8 +2538,8 @@ async fn sync_agent_tmpfiles(agents: &[AgentTmpfilesEntry]) -> Result<(String, S #[cfg(test)] mod tests { use super::{ - OwnedFd, PAUSED_MARKER_FILE, PrivRequest, check_fd_agreement, limits_dropin_body, - looks_like_secret, redact_secret_line, remove_marker_in, write_state_file_nofollow, + OwnedFd, PAUSED_MARKER_FILE, PrivRequest, check_fd_agreement, contains_secret_shaped_run, + limits_dropin_body, redact_secret_line, remove_marker_in, write_state_file_nofollow, }; use std::path::PathBuf; use std::sync::atomic::{AtomicU32, Ordering}; @@ -2660,23 +2676,50 @@ mod tests { #[test] fn secret_shape_boundaries() { - // Ordinary forgejo-admin output survives: no word is long enough. + // Ordinary forgejo-admin output survives: no run is long enough. assert_eq!( redact_secret_line("Command 'user' 'create' finished with no errors."), "Command 'user' 'create' finished with no errors." ); // 31 chars is below the floor, 32 is at it. - assert!(!looks_like_secret(&"a".repeat(31))); - assert!(looks_like_secret(&"a".repeat(32))); + assert!(!contains_secret_shaped_run(&"a".repeat(31))); + assert!(contains_secret_shaped_run(&"a".repeat(32))); // Punctuation breaks the run — a sentence never trips it however long. - assert!(!looks_like_secret( + assert!(!contains_secret_shaped_run( "this.is.a.very.long.dotted.identifier.but.not.a.secret" )); // base64url and hex alphabets both count. - assert!(looks_like_secret( + assert!(contains_secret_shaped_run( "ZGVhZGJlZWZkZWFkYmVlZmRlYWRiZWVmZGVhZA==" )); - assert!(looks_like_secret("aG93-dy_there-aG93dy1theresomething")); + assert!(contains_secret_shaped_run( + "aG93-dy_there-aG93dy1theresomething" + )); + } + + /// argus on the review: a whitespace-delimited check is defeated by + /// punctuation glued to the secret — the punctuation joins the "word" + /// and fails the alphabet test for the whole run, while the credential + /// sits there in full. Scanning for a RUN rather than a WORD closes it. + /// These are the shapes that used to slip through. + #[test] + fn secret_is_caught_with_punctuation_glued_to_it() { + const TOK: &str = "0123456789abcdef0123456789abcdef01234567"; + for line in [ + format!("token: {TOK},"), + format!("token: {TOK}."), + format!("using [{TOK}] now"), + format!("value=\"{TOK}\""), + format!("(created {TOK})"), + // no whitespace anywhere -- one glued blob + format!("Bearer:{TOK};next"), + ] { + assert_eq!( + redact_secret_line(&line), + "[redacted: line contains a secret-shaped token]", + "leaked through: {line}" + ); + } } /// Unique scratch dir per test, no external tempfile dep.