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

hive-priv: three doc comments that disagree with their own validators

Browse source 
Follow-up to the same defect class as the `validate_container_name`
fix: prose on a privsep validator that does not match what the body
does. Found by enumerating all 33 fns in this file whose doc claims a
check or guarantee and reading body against doc. Three disagreed.

`validate_forge_admin_arg` overclaimed, which is the dangerous
direction. Its doc ended "Shell metacharacters are harmless since the
command is spawned directly (no shell), but we reject them
defensively" -- and no metacharacter is rejected anywhere: the body
takes null bytes, newlines and carriage returns only. The first clause
is true and is the actual safety argument, so it stays; the claim of a
defence that does not exist goes. The same doc also said "null bytes
and newlines" while the body rejects `\r` too.

`validate_bind_path` listed "no newlines, null bytes, or
double-quotes" and omitted the colon its body rejects -- the one that
matters most. The other three would corrupt the
`EXTRA_NSPAWN_FLAGS="..."` conf line; a colon corrupts nothing,
because `--bind=SRC:DST` is colon-separated, so a path carrying one
silently produces a different mount than the caller asked for. The
function's own bail! string already named colons.

Checked and left alone because they are accurate: `check_fd_agreement`
(both arms really do bail, so "in either direction" is earned),
`exec_forge_admin`, `validate_credential_name`.

Comments only, no behaviour change. Gated: fmt, clippy -D warnings, 21
tests, rustdoc under CI's docs-rustdoc lints, push-lints.
This commit is contained in:
atlas 2026-09-02 08:34:37 +02:00 • committed by mara
commit fa9b8ba73a
1 changed files with 13 additions and 6 deletions
Download patch file Download diff file Expand all files Collapse all files

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

@ -2183,10 +2183,12 @@ async fn set_subvolume_quota(
Ok((String::new(), String::new()))
}
/// Validate a single argument destined for `forgejo admin`. Rejects
/// null bytes and newlines (which could corrupt the subprocess args list
/// or log output). Shell metacharacters are harmless since the command
/// is spawned directly (no shell), but we reject them defensively.
/// 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");
@ -2765,8 +2767,13 @@ fn validate_name_chars(name: &str) -> Result<()> {
}
/// Validate a bind-mount path: must be absolute, non-empty, and contain
/// no newlines, null bytes, or double-quotes (which would break the
/// `EXTRA_NSPAWN_FLAGS="..."` conf line format).
/// no newlines, null bytes, double-quotes, or colons.
///
/// The first three would break the `EXTRA_NSPAWN_FLAGS="..."` conf line
/// format. The colon is the one that matters most and had been left out
/// of this list: `--bind=SRC:DST` is colon-separated, so a path carrying
/// one does not corrupt the line — it silently becomes a *different
/// mount* than the caller asked for.
fn validate_bind_path(path: &str) -> Result<()> {
if path.is_empty()
|| !path.starts_with('/')