hive reads the forge for nix as its own account, not core
hive-c0re copies the hive's store read token to /run/hyperhive/forge-hive-token for nix's git credential helper, and no longer mints, reads or falls back to the core admin token. drops the core user sweep, avatar upload, webhook cleanup and hive-priv's RunForgeAdmin.
This commit is contained in:
parent
32a4821cb4
commit
b79896c237
23 changed files with 236 additions and 1118 deletions
|
|
@ -1,180 +1,11 @@
|
|||
//! Host infrastructure ops: `RunForgeAdmin` (`forgejo admin` inside the
|
||||
//! `hive-forge` container), `ControlInfraContainer` (start/stop of an
|
||||
//! [`InfraContainer`] unit) and `ReloadGatewayNginx`. None of them takes a
|
||||
//! unit or container name from the caller.
|
||||
//! Host infrastructure ops: `ControlInfraContainer` (start/stop of an
|
||||
//! [`InfraContainer`] unit) and `ReloadGatewayNginx`. Neither takes a unit or
|
||||
//! container name from the caller.
|
||||
|
||||
use anyhow::{Context as _, Result, bail};
|
||||
use hive_priv_sock::{InfraAction, InfraContainer};
|
||||
use tokio::process::Command;
|
||||
|
||||
/// `RunForgeAdmin`: every arg must pass [`validate_forge_admin_arg`] before
|
||||
/// the admin CLI ever sees it.
|
||||
pub(crate) async fn exec_forge_admin(args: &[String]) -> Result<(String, String)> {
|
||||
for arg in args {
|
||||
validate_forge_admin_arg(arg)?;
|
||||
}
|
||||
run_forge_admin(args).await
|
||||
}
|
||||
|
||||
/// Validate a single argument destined for `forgejo admin`. Rejects null
|
||||
/// bytes, newlines and carriage returns (which could corrupt the
|
||||
/// subprocess args list or log output).
|
||||
///
|
||||
/// Shell metacharacters are **not** rejected, and do not need to be: the
|
||||
/// command is spawned directly, with no shell to interpret them.
|
||||
fn validate_forge_admin_arg(arg: &str) -> Result<()> {
|
||||
if arg.bytes().any(|b| b == 0 || b == b'\n' || b == b'\r') {
|
||||
bail!("forge admin arg {arg:?} contains null byte or newline");
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// 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 sitting in the host
|
||||
/// journal, readable by anything with access to it.
|
||||
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 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
|
||||
}
|
||||
|
||||
/// Name a `forgejo admin` invocation for an error message without
|
||||
/// reproducing its arguments: keep the leading verb path, stop at the
|
||||
/// first flag. `["user", "change-password", "--username", "iris",
|
||||
/// "--password", "…"]` becomes `forgejo admin user change-password`.
|
||||
///
|
||||
/// Same allowlist as hive-c0re's `describe_forge_admin`
|
||||
/// (`hive-c0re/src/forge/mod.rs`), for the same reason: the verbs are a
|
||||
/// closed set this crate chooses itself, but hive-c0re passes a live
|
||||
/// `--password` value as an argument on some calls, and argument values
|
||||
/// are never safe to assume closed. Redacting the value after
|
||||
/// `--password` instead would repeat the bug `redact_secret_line`'s own
|
||||
/// doc comment describes — a denylist of one flag name fails open the
|
||||
/// moment a differently named secret-bearing flag is added.
|
||||
fn describe_forge_admin(args: &[String]) -> String {
|
||||
let verbs: Vec<&str> = args
|
||||
.iter()
|
||||
.take_while(|a| !a.starts_with('-'))
|
||||
.map(String::as_str)
|
||||
.collect();
|
||||
if verbs.is_empty() {
|
||||
"forgejo admin".to_owned()
|
||||
} else {
|
||||
format!("forgejo admin {}", verbs.join(" "))
|
||||
}
|
||||
}
|
||||
|
||||
/// Run `forgejo admin <args>` inside the `hive-forge` container as the
|
||||
/// `forgejo` unix user. Requires root (for nsenter into the container's
|
||||
/// namespaces). Returns `(stdout, stderr)`.
|
||||
async fn run_forge_admin(args: &[String]) -> Result<(String, String)> {
|
||||
let mut cmd_args: Vec<&str> = vec![
|
||||
"run",
|
||||
"hive-forge",
|
||||
"--",
|
||||
"runuser",
|
||||
"-u",
|
||||
"forgejo",
|
||||
"--",
|
||||
"forgejo",
|
||||
"--work-path",
|
||||
"/var/lib/forgejo",
|
||||
"admin",
|
||||
];
|
||||
for a in args {
|
||||
cmd_args.push(a.as_str());
|
||||
}
|
||||
let out = Command::new("nixos-container")
|
||||
.args(&cmd_args)
|
||||
.output()
|
||||
.await
|
||||
.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));
|
||||
}
|
||||
for line in stderr.lines() {
|
||||
tracing::warn!(target: "forgejo-admin", "{}", redact_secret_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. And name the invocation via
|
||||
// `describe_forge_admin` rather than `args.join(" ")`: some callers
|
||||
// pass a live `--password` value as an argument, so reproducing the
|
||||
// raw vector here is the same "two of three sites" gap that makes
|
||||
// these leaks survive a fix — the log was redacted, but the error
|
||||
// was not.
|
||||
let safe_stderr: String = stderr
|
||||
.lines()
|
||||
.map(|l| redact_secret_line(l).into_owned())
|
||||
.collect::<Vec<_>>()
|
||||
.join("; ");
|
||||
bail!(
|
||||
"{} failed ({}): {}",
|
||||
describe_forge_admin(args),
|
||||
out.status,
|
||||
safe_stderr.trim()
|
||||
);
|
||||
}
|
||||
Ok((stdout, stderr))
|
||||
}
|
||||
|
||||
/// `ControlInfraContainer` — start/stop a hive infrastructure service via
|
||||
/// `systemctl <verb> <unit>`. The [`InfraContainer`] enum is the allowlist:
|
||||
/// serde already rejected any unknown / unsafe name (hive-c0re, the forge
|
||||
|
|
@ -291,139 +122,12 @@ pub(crate) async fn sync_gateway_nginx() -> Result<(String, String)> {
|
|||
}
|
||||
Ok((String::new(), String::new()))
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::{
|
||||
InfraAction, InfraContainer, contains_secret_shaped_run, describe_forge_admin,
|
||||
redact_secret_line,
|
||||
};
|
||||
use super::{InfraAction, InfraContainer};
|
||||
use hive_priv_sock::PrivRequest;
|
||||
|
||||
#[test]
|
||||
fn redacts_lines_mentioning_password_case_insensitively() {
|
||||
assert_eq!(
|
||||
redact_secret_line("New password: hunter2"),
|
||||
"[redacted: line mentions a password]"
|
||||
);
|
||||
assert_eq!(
|
||||
redact_secret_line("PASSWORD=hunter2"),
|
||||
"[redacted: line mentions a password]"
|
||||
);
|
||||
assert_eq!(
|
||||
redact_secret_line("User \"foo\" was successfully created."),
|
||||
"User \"foo\" was successfully created."
|
||||
);
|
||||
}
|
||||
|
||||
/// `run_forge_admin`'s `bail!` names the invocation through this
|
||||
/// function instead of `args.join(" ")` — the regression this test
|
||||
/// guards. A `--password` value never survives to the error string.
|
||||
#[test]
|
||||
fn describe_forge_admin_does_not_reproduce_a_password_argument() {
|
||||
let described = describe_forge_admin(&[
|
||||
"user".to_owned(),
|
||||
"create".to_owned(),
|
||||
"--username".to_owned(),
|
||||
"x".to_owned(),
|
||||
"--password".to_owned(),
|
||||
"S3cret".to_owned(),
|
||||
]);
|
||||
assert_eq!(described, "forgejo admin user create");
|
||||
assert!(!described.contains("S3cret"), "{described}");
|
||||
}
|
||||
|
||||
/// The `--password=value` single-argument form is also excluded: it
|
||||
/// starts with `-`, so it never enters the kept verb path.
|
||||
#[test]
|
||||
fn describe_forge_admin_redacts_the_equals_sign_form_too() {
|
||||
let described = describe_forge_admin(&[
|
||||
"user".to_owned(),
|
||||
"create".to_owned(),
|
||||
"--password=S3cret".to_owned(),
|
||||
]);
|
||||
assert!(!described.contains("S3cret"), "{described}");
|
||||
}
|
||||
|
||||
/// Control: a non-secret argument (the verb path) still appears, so
|
||||
/// the message stays useful for diagnosing which call failed.
|
||||
#[test]
|
||||
fn describe_forge_admin_keeps_the_verb_path() {
|
||||
assert_eq!(
|
||||
describe_forge_admin(&["user".to_owned(), "change-password".to_owned()]),
|
||||
"forgejo admin user change-password"
|
||||
);
|
||||
assert_eq!(
|
||||
describe_forge_admin(&["--help".to_owned()]),
|
||||
"forgejo admin"
|
||||
);
|
||||
assert_eq!(describe_forge_admin(&[]), "forgejo admin");
|
||||
}
|
||||
|
||||
/// 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}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// A start/stop naming the forge or matrix container fails to decode, so
|
||||
/// `run` answers it with a parse error before anything reaches
|
||||
/// `systemctl`. Control: the same request naming the gateway decodes.
|
||||
|
|
|
|||
|
|
@ -21,7 +21,7 @@
|
|||
//! own module: [`container`] (lifecycle), [`container_config`] (nspawn flags,
|
||||
//! systemd drop-ins), [`storage`] (btrfs subvolumes, quotas, snapshots, disk
|
||||
//! measurement), [`files`] (pause marker, socket dir, legacy tmpfiles) and
|
||||
//! [`infra`] (gateway nginx, forge admin, infra containers). Helpers more than
|
||||
//! [`infra`] (gateway nginx, infra containers). Helpers more than
|
||||
//! one family uses are in [`shared`].
|
||||
|
||||
mod container;
|
||||
|
|
@ -399,8 +399,6 @@ async fn exec(
|
|||
|
||||
PrivRequest::ReloadGatewayNginx => infra::sync_gateway_nginx().await,
|
||||
|
||||
PrivRequest::RunForgeAdmin { ref args } => infra::exec_forge_admin(args).await,
|
||||
|
||||
PrivRequest::SetAgentPaused {
|
||||
ref agent_name,
|
||||
paused,
|
||||
|
|
|
|||
Loading…
Reference in a new issue