vinzenz/hyperhive
vinzenz/hyperhive
Watch
0
0
Fork
You've already forked hyperhive
 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 1

Compare commits

base: vinzenz:462f353c107298850402e31cd9f5adcd20b88f42
Branches Tags
vinzenz:main
..
compare: vinzenz:22f8805a77dc1e56a4faa186cfb86dc2d71b7768
Branches Tags
vinzenz:main
1 changed files with 18 additions and 141 deletions
Download patch file Download diff file Expand all files Collapse all files

159
hive-priv/src/main.rs
View file

@ -1700,61 +1700,18 @@ fn validate_forge_admin_arg(arg: &str) -> Result<()> {
}
/// Redact a line before it hits the (root-readable, but still
/// unnecessarily exposed) host journal.
///
/// Two independent rules, because the previous single rule failed open.
/// It matched only the substring "password", chosen to be robust against
/// forgejo *rewording* its password line — and the leak arrived from the
/// other axis entirely: a **different kind of secret** on a differently
/// worded line. `forgejo admin user generate-access-token` prints
/// `Access token was successfully created: <40 hex>`, which contains no
/// "password" and went to the journal verbatim for every agent ever
/// provisioned.
///
/// So the second rule matches on **shape, not vocabulary**: a long
/// unbroken run of secret-alphabet characters. A new secret type is then
/// caught by default rather than by someone remembering to add a keyword.
///
/// ⚠️ This deliberately over-matches. A nix store hash is also a long
/// opaque run and will redact its line. That is the correct direction to
/// be wrong in: the cost of a false positive is one less log line, and
/// the cost of a false negative is a live credential in a journal that
/// any `read_host_journal` holder can read.
fn redact_secret_line(line: &str) -> std::borrow::Cow<'_, str> {
/// unnecessarily exposed) host journal. `forgejo admin user create
/// --random-password` prints the generated password straight to stdout —
/// case-insensitive substring match on "password" is deliberately broad
/// (not pinned to forgejo's exact wording, which can change across
/// versions) so any password-bearing line gets caught rather than relying
/// on a phrase that could silently drift out of sync.
fn redact_password_line(line: &str) -> std::borrow::Cow<'_, str> {
if line.to_ascii_lowercase().contains("password") {
return std::borrow::Cow::Borrowed("[redacted: line mentions a password]");
std::borrow::Cow::Borrowed("[redacted: line mentions a password]")
} else {
std::borrow::Cow::Borrowed(line)
}
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 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: `"<token>,"` and
/// `"[<token>]"` 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 <args>` inside the `hive-forge` container as the
@ -1784,33 +1741,18 @@ async fn run_forge_admin(args: &[String]) -> Result<(String, String)> {
.context("invoke nixos-container run hive-forge -- forgejo admin")?;
let stdout = String::from_utf8_lossy(&out.stdout).into_owned();
let stderr = String::from_utf8_lossy(&out.stderr).into_owned();
// stdout at DEBUG, not INFO: on the success path this stream carries
// the *product* of the command (the freshly minted token, the created
// user's details) and nothing an operator needs at default verbosity.
// Redaction stays on as the second layer — the level decides who sees
// it, the redactor decides what it says, and neither alone is enough.
for line in stdout.lines() {
tracing::debug!(target: "forgejo-admin", "{}", redact_secret_line(line));
tracing::info!(target: "forgejo-admin", "{}", redact_password_line(line));
}
for line in stderr.lines() {
tracing::warn!(target: "forgejo-admin", "{}", redact_secret_line(line));
tracing::warn!(target: "forgejo-admin", "{}", redact_password_line(line));
}
if !out.status.success() {
// Redact here too. The error string is propagated to the caller and
// ends up logged; a partial-failure stderr can carry the same
// material stdout would have. Redacting the log but not the error
// is the same "two of three sites" gap that makes these leaks
// survive a fix.
let safe_stderr: String = stderr
.lines()
.map(|l| redact_secret_line(l).into_owned())
.collect::<Vec<_>>()
.join("; ");
bail!(
"forgejo admin {} failed ({}): {}",
args.join(" "),
out.status,
safe_stderr.trim()
stderr.trim()
);
}
Ok((stdout, stderr))
@ -2538,8 +2480,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, contains_secret_shaped_run,
limits_dropin_body, redact_secret_line, remove_marker_in, write_state_file_nofollow,
OwnedFd, PAUSED_MARKER_FILE, PrivRequest, check_fd_agreement, limits_dropin_body,
redact_password_line, remove_marker_in, write_state_file_nofollow,
};
use std::path::PathBuf;
use std::sync::atomic::{AtomicU32, Ordering};
@ -2644,84 +2586,19 @@ mod tests {
#[test]
fn redacts_lines_mentioning_password_case_insensitively() {
assert_eq!(
redact_secret_line("New password: hunter2"),
redact_password_line("New password: hunter2"),
"[redacted: line mentions a password]"
);
assert_eq!(
redact_secret_line("PASSWORD=hunter2"),
redact_password_line("PASSWORD=hunter2"),
"[redacted: line mentions a password]"
);
assert_eq!(
redact_secret_line("User \"foo\" was successfully created."),
redact_password_line("User \"foo\" was successfully created."),
"User \"foo\" was successfully created."
);
}
/// The regression this function exists for. The keyword rule passes
/// this line straight through — it says nothing about a password — so
/// only the shape rule catches it.
#[test]
fn redacts_access_token_line_which_mentions_no_password() {
let line =
"Access token was successfully created: 0123456789abcdef0123456789abcdef01234567";
assert!(
!line.to_ascii_lowercase().contains("password"),
"fixture must not contain the keyword, or it proves nothing"
);
assert_eq!(
redact_secret_line(line),
"[redacted: line contains a secret-shaped token]"
);
}
#[test]
fn secret_shape_boundaries() {
// 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!(!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!(!contains_secret_shaped_run(
"this.is.a.very.long.dotted.identifier.but.not.a.secret"
));
// base64url and hex alphabets both count.
assert!(contains_secret_shaped_run(
"ZGVhZGJlZWZkZWFkYmVlZmRlYWRiZWVmZGVhZA=="
));
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.
fn scratch() -> PathBuf {
static CTR: AtomicU32 = AtomicU32::new(0);