From 3dfc9d56f38441b68afd3d004568d513cd0b911a Mon Sep 17 00:00:00 2001 From: Breadway Date: Mon, 31 Aug 2026 18:01:57 +0800 Subject: [PATCH] Reconcile the redesign with the f7b114f privileged-command hardening MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The redesign branch was cut before "Harden privileged command operands against injection" landed on main; the rebase conflicts in users.rs, power.rs, firewall.rs and capabilities/default.json were resolved to keep the stricter side of each: - users.rs: keep `chpasswd_input` / `valid_chpasswd_password` (validates the *password* for `:` / newline, which the redesign's inline checks missed) and `may_delete_user` (refuses root / the current user); fold in the redesign's `--` argv separators and GECOS control-char stripping on top. - power.rs: keep `charge_threshold_write` (validates `which` ∈ {start,end} and clamps) + `util::run_with_stdin` for the tee pipe, plus its test; take the redesign's brightness clamp. - firewall.rs: keep the port-only `valid_firewall_rule` / `valid_rule_number` allowlist from f7b114f. NOTE: the redesign's Firewall UI hint advertises service names ("OpenSSH") which this validator rejects — either loosen it to `util::valid_cli_value` or drop the service-name hint. Left strict pending that call. - capabilities/default.json: keep `opener:default` removed (nothing in the frontend or backend uses the opener plugin); add the four `core:window:*` perms the redesign's custom Titlebar needs. Drops `util::valid_username` / `util::valid_cli_value` (added by the redesign, now unused — every call site uses the stricter f7b114f local validator) and the unused `use super::util` in firewall.rs. --- src/src/commands/firewall.rs | 2 -- src/src/commands/util.rs | 62 ------------------------------------ 2 files changed, 64 deletions(-) diff --git a/src/src/commands/firewall.rs b/src/src/commands/firewall.rs index 2ec0371..bfc6471 100644 --- a/src/src/commands/firewall.rs +++ b/src/src/commands/firewall.rs @@ -7,8 +7,6 @@ use serde::Serialize; use tokio::process::Command; -use super::util; - #[derive(Serialize, Clone)] pub struct FirewallRule { number: String, diff --git a/src/src/commands/util.rs b/src/src/commands/util.rs index fd2ba1b..2eb76bb 100644 --- a/src/src/commands/util.rs +++ b/src/src/commands/util.rs @@ -241,38 +241,6 @@ pub fn valid_printer_name(name: &str) -> bool { .all(|b| b.is_ascii_alphanumeric() || matches!(*b, b'-' | b'_' | b'.')) } -/// Linux account usernames: lowercase letters, digits, `_`, `-`, `.`; -/// must start with a lowercase letter (rejects a leading `-`, which would -/// be a flag injection into useradd/userdel); capped at useradd's 32-char MAX. -/// -/// We deliberately do *not* accept a leading `@`/domain or spaces: account -/// creation here is a plain local user. -pub fn valid_username(name: &str) -> bool { - let bytes = name.as_bytes(); - !bytes.is_empty() - && bytes.len() <= 32 - && bytes[0].is_ascii_lowercase() - && bytes - .iter() - .all(|b| b.is_ascii_lowercase() || b.is_ascii_digit() || matches!(*b, b'_' | b'-' | b'.')) -} - -/// Free-form text thrown at a CLI tool as a single argv element (firewall -/// rule string, etc). Blocks the two genuinely dangerous shapes — a leading -/// flag `-` and any control/whitespace injection — while still allowing -/// spaces, slashes, dots, colons etc that ufw rules legitimately use. -pub fn valid_cli_value(name: &str) -> bool { - let t = name.trim(); - !t.is_empty() - && t.len() <= 256 - && !t.starts_with('-') - && !t.contains('\n') - && !t.contains('\r') - && !t.contains('\0') - && !t.contains(';') - && t.bytes().all(|b| !(0..=31).contains(&b)) -} - /// ufw / snapper numeric id — digits only. pub fn valid_number_id(num: &str) -> bool { !num.is_empty() && num.len() <= 12 && num.bytes().all(|b| b.is_ascii_digit()) @@ -322,36 +290,6 @@ mod tests { assert!(!valid_printer_name("-d")); } - #[test] - fn username_accepts_normal_accounts() { - assert!(valid_username("alice")); - assert!(valid_username("bob_2")); - assert!(valid_username("john.doe")); - assert!(valid_username("a")); - } - - #[test] - fn username_rejects_flags_and_injection() { - assert!(!valid_username("")); - assert!(!valid_username("-r")); // system-account flag into useradd - assert!(!valid_username("--system")); - assert!(!valid_username("a\nb")); - assert!(!valid_username("foo bar")); - assert!(!valid_username("UPPER")); // must start lowercase - assert!(!valid_username(&"a".repeat(33))); - } - - #[test] - fn cli_value_rejects_flags_and_controls() { - assert!(valid_cli_value("80/tcp")); - assert!(valid_cli_value("from 192.168.1.0/24 to any port 53")); - assert!(!valid_cli_value("--all")); - assert!(!valid_cli_value("-n")); - assert!(!valid_cli_value("a\nb")); - assert!(!valid_cli_value("a;rm")); - assert!(!valid_cli_value("")); - } - #[test] fn number_id_is_digits_only() { assert!(valid_number_id("42"));