From 4abc2c5294102b0762a2ba3e1f14d663f5a9b050 Mon Sep 17 00:00:00 2001 From: Sam & Claude Date: Sun, 14 Jun 2026 00:42:43 +0200 Subject: [PATCH 1/2] fix(clawdie): harden FreeBSD installer plan (Sam & Codex) Use the clawdie service user in the generated FreeBSD rc.d script, chown state directories after the user is created, and reject unknown existing ZFS pools before rendering/applying a plan. Update the FreeBSD validation handoff to cover these checks.\n\nFreeBSD checks: cargo fmt --check; ./scripts/check-format.sh; git diff --check; cargo test -p clawdie -- --nocapture; cargo clippy -p clawdie --all-targets -- -D warnings; cargo build -p clawdie --release; target/release/clawdie discover; target/release/clawdie plan; target/release/clawdie apply --pool zroot (dry-run); target/release/clawdie plan --pool does-not-exist (expected error). --- crates/clawdie/README.md | 7 ++-- crates/clawdie/src/main.rs | 54 +++++++++++++++++++++++++++++++ crates/clawdie/src/plan.rs | 17 ++-------- crates/clawdie/src/platform.rs | 54 +++++++++++++++++++++++++++---- docs/CLAWDIE-INSTALLER-HANDOFF.md | 6 ++-- 5 files changed, 111 insertions(+), 27 deletions(-) diff --git a/crates/clawdie/README.md b/crates/clawdie/README.md index 4a6fa7a..3436270 100644 --- a/crates/clawdie/README.md +++ b/crates/clawdie/README.md @@ -37,9 +37,10 @@ ZFS layout under the pool: /clawdie/log -> /var/log/clawdie ``` -Plain-dirs fallback creates the same mountpoints as ordinary directories owned by -the `clawdie` user. Either way, a `clawdie` service user and the rc.d script -(FreeBSD) or systemd unit (Linux) are installed and enabled to run +Plain-dirs fallback creates the same mountpoints as ordinary directories. Either +way, a `clawdie` service user is created, the state directories are owned by +`clawdie:clawdie`, and the rc.d script (FreeBSD, via `daemon -u clawdie`) or +systemd unit (Linux, `User=clawdie`) is installed and enabled to run `/usr/local/bin/colibri-daemon`. ## Safety diff --git a/crates/clawdie/src/main.rs b/crates/clawdie/src/main.rs index f1a63b0..046be0e 100644 --- a/crates/clawdie/src/main.rs +++ b/crates/clawdie/src/main.rs @@ -76,6 +76,31 @@ fn pick_pool(explicit: Option) -> Result, String> { } } +fn validate_existing_pool(pool: &str, pools: &[zfs::Pool]) -> Result<(), String> { + if pools.iter().any(|p| p.name == pool) { + return Ok(()); + } + let available = if pools.is_empty() { + "none".to_string() + } else { + pools + .iter() + .map(|p| p.name.as_str()) + .collect::>() + .join(", ") + }; + Err(format!( + "ZFS pool `{pool}` not found; available pools: {available}" + )) +} + +fn validate_storage(storage: &Storage) -> Result<(), String> { + if let Storage::ZfsExisting { pool } = storage { + validate_existing_pool(pool, &zfs::list_pools())?; + } + Ok(()) +} + /// Refuse to create a pool on a disk that isn't clearly empty, unless --force. fn validate_create_device(device: &str, force: bool) -> Result<(), String> { if force { @@ -151,6 +176,7 @@ fn run() -> Result<(), String> { pick_pool(pool)?, create_pool, )?; + validate_storage(&storage)?; let (pf, svc, steps) = deploy_steps(&storage); print!("{}", plan::render(pf.as_ref(), &storage, &svc, &steps)); offer_zfs_if_plain(&storage); @@ -168,6 +194,7 @@ fn run() -> Result<(), String> { pick_pool(pool)?, create_pool, )?; + validate_storage(&storage)?; if let Storage::ZfsCreate { device, .. } = &storage { validate_create_device(device, force)?; } @@ -215,3 +242,30 @@ fn main() -> ExitCode { } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn validate_existing_pool_accepts_known_pool() { + let pools = vec![zfs::Pool { + name: "zroot".to_string(), + size: "100G".to_string(), + health: "ONLINE".to_string(), + }]; + assert!(validate_existing_pool("zroot", &pools).is_ok()); + } + + #[test] + fn validate_existing_pool_rejects_unknown_pool() { + let pools = vec![zfs::Pool { + name: "zroot".to_string(), + size: "100G".to_string(), + health: "ONLINE".to_string(), + }]; + let err = validate_existing_pool("missing", &pools).unwrap_err(); + assert!(err.contains("missing")); + assert!(err.contains("zroot")); + } +} diff --git a/crates/clawdie/src/plan.rs b/crates/clawdie/src/plan.rs index dc6b36b..c39003a 100644 --- a/crates/clawdie/src/plan.rs +++ b/crates/clawdie/src/plan.rs @@ -82,7 +82,6 @@ fn zfs_dataset_steps(pool: &str, svc: &ServiceSpec) -> Vec { } fn plain_dir_steps(svc: &ServiceSpec) -> Vec { - let owner = format!("{}:{}", svc.user, svc.user); vec![ Step::run( format!("create {}", svc.data_dir), @@ -92,16 +91,6 @@ fn plain_dir_steps(svc: &ServiceSpec) -> Vec { "create /var/log/clawdie", &["mkdir", "-p", "/var/log/clawdie"], ), - Step::run( - "own clawdie directories", - &[ - "chown", - "-R", - owner.as_str(), - svc.data_dir.as_str(), - "/var/log/clawdie", - ], - ), ] } @@ -232,11 +221,11 @@ mod tests { #[test] fn plan_shapes() { let svc = ServiceSpec::default(); - // plain dirs: 3 dir steps + 4 linux service steps + // plain dirs: 2 dir steps + 5 linux service steps let s = build(&Linux, &Storage::PlainDirs, &svc); assert_eq!(s.len(), 7); assert!(matches!(&s[0].action, Action::Run(a) if a[0] == "mkdir")); - // create pool: 1 zpool + 3 datasets + 3 freebsd service steps + // create pool: 1 zpool + 3 dataset steps + 4 freebsd service steps let c = build( &FreeBsd, &Storage::ZfsCreate { @@ -248,7 +237,7 @@ mod tests { assert!( matches!(&c[0].action, Action::Run(a) if a[0] == "zpool" && a.contains(&"/dev/sdb".to_string())) ); - assert_eq!(c.len(), 7); + assert_eq!(c.len(), 8); } #[test] diff --git a/crates/clawdie/src/platform.rs b/crates/clawdie/src/platform.rs index baaa759..0f7cc72 100644 --- a/crates/clawdie/src/platform.rs +++ b/crates/clawdie/src/platform.rs @@ -100,10 +100,12 @@ impl Platform for FreeBsd { fn install_service_steps(&self, s: &ServiceSpec) -> Vec { let rc_path = format!("/usr/local/etc/rc.d/{}", s.name); let rc = format!( - "#!/bin/sh\n# PROVIDE: {name}\n# REQUIRE: LOGIN\n# KEYWORD: shutdown\n. /etc/rc.subr\nname=\"{name}\"\nrcvar=\"{name}_enable\"\ncommand=\"/usr/sbin/daemon\"\ncommand_args=\"-f -o /var/log/clawdie/daemon.log {exec}\"\nload_rc_config $name\n: ${{{name}_enable:=NO}}\nrun_rc_command \"$1\"\n", + "#!/bin/sh\n# PROVIDE: {name}\n# REQUIRE: LOGIN\n# KEYWORD: shutdown\n. /etc/rc.subr\nname=\"{name}\"\nrcvar=\"{name}_enable\"\ncommand=\"/usr/sbin/daemon\"\ncommand_args=\"-f -u {user} -o /var/log/clawdie/daemon.log {exec}\"\nload_rc_config $name\n: ${{{name}_enable:=NO}}\nrun_rc_command \"$1\"\n", name = s.name, + user = s.user, exec = s.exec, ); + let owner = format!("{}:{}", s.user, s.user); vec![ Step::run( format!("create service user {}", s.user), @@ -119,6 +121,16 @@ impl Platform for FreeBsd { "Clawdie host service", ], ), + Step::run( + "own clawdie state directories", + &[ + "chown", + "-R", + owner.as_str(), + s.data_dir.as_str(), + "/var/log/clawdie", + ], + ), Step::write(format!("install rc.d script {rc_path}"), rc_path, 0o555, rc), Step::run( format!("enable {} service", s.name), @@ -141,6 +153,7 @@ impl Platform for Linux { user = s.user, exec = s.exec, ); + let owner = format!("{}:{}", s.user, s.user); vec![ Step::run( format!("create service user {}", s.user), @@ -154,6 +167,16 @@ impl Platform for Linux { &s.user, ], ), + Step::run( + "own clawdie state directories", + &[ + "chown", + "-R", + owner.as_str(), + s.data_dir.as_str(), + "/var/log/clawdie", + ], + ), Step::write( format!("install systemd unit {unit_path}"), unit_path, @@ -185,10 +208,22 @@ mod tests { fn freebsd_steps_shape() { let steps = FreeBsd.install_service_steps(&ServiceSpec::default()); assert_eq!(FreeBsd.service_kind(), "rc.d"); - // user create, rc.d write, sysrc enable - assert_eq!(steps.len(), 3); - assert!(matches!(steps[1].action, Action::WriteFile { .. })); - if let Action::Run(argv) = &steps[2].action { + // user create, ownership, rc.d write, sysrc enable + assert_eq!(steps.len(), 4); + if let Action::Run(argv) = &steps[1].action { + assert_eq!(argv[0], "chown"); + assert!(argv.contains(&"clawdie:clawdie".to_string())); + } else { + panic!("expected chown run step"); + } + assert!(matches!(steps[2].action, Action::WriteFile { .. })); + if let Action::WriteFile { contents, .. } = &steps[2].action { + assert!(contents.contains("daemon\"")); + assert!(contents.contains("-u clawdie")); + } else { + panic!("expected rc.d write step"); + } + if let Action::Run(argv) = &steps[3].action { assert_eq!(argv[0], "sysrc"); assert_eq!(argv[1], "clawdie_enable=YES"); } else { @@ -200,8 +235,13 @@ mod tests { fn linux_steps_shape() { let steps = Linux.install_service_steps(&ServiceSpec::default()); assert_eq!(Linux.service_kind(), "systemd"); - assert_eq!(steps.len(), 4); - if let Action::WriteFile { path, .. } = &steps[1].action { + assert_eq!(steps.len(), 5); + if let Action::Run(argv) = &steps[1].action { + assert_eq!(argv[0], "chown"); + } else { + panic!("expected chown run step"); + } + if let Action::WriteFile { path, .. } = &steps[2].action { assert_eq!(path, "/etc/systemd/system/clawdie.service"); } else { panic!("expected unit write step"); diff --git a/docs/CLAWDIE-INSTALLER-HANDOFF.md b/docs/CLAWDIE-INSTALLER-HANDOFF.md index 23fd621..e66bc75 100644 --- a/docs/CLAWDIE-INSTALLER-HANDOFF.md +++ b/docs/CLAWDIE-INSTALLER-HANDOFF.md @@ -26,7 +26,7 @@ clawdie plan # expect: resolves to the single pool (or pass --pool), ZFS Verify: - `discover` lists the real pools (parsed from `zpool list -H`) and any clawdie datasets. -- On FreeBSD `plan` always resolves to **ZFS** (never plain-dirs); with no pool it errors clearly. +- On FreeBSD `plan` always resolves to **ZFS** (never plain-dirs); with no pool or an unknown explicit pool it errors clearly. - Expected gap: the `disks:` section uses `lsblk` (Linux-only), so it is **empty on FreeBSD** — that is correct. `--create-pool` is a Linux convenience; FreeBSD already has ZFS. ## 3. Provisioning — DESTRUCTIVE, scratch host / VM / test pool only @@ -39,9 +39,9 @@ clawdie apply --pool --yes # provisions Validate after `--yes`: 1. **Datasets** (`zfs list`): `/clawdie` (canmount=off), `…/db` → `/var/db/clawdie`, `…/log` → `/var/log/clawdie`. -2. **Service user** (`pw usershow clawdie`): nologin, home `/var/db/clawdie`. +2. **Service user + ownership** (`pw usershow clawdie`, `ls -ld /var/db/clawdie /var/log/clawdie`): nologin, home `/var/db/clawdie`, state directories owned by `clawdie:clawdie`. 3. **rc.d service**: `/usr/local/etc/rc.d/clawdie` (mode 0555) and `clawdie_enable=YES` (`sysrc`). Check `service clawdie enabled` / `rcvar`. - - The script execs `/usr/local/bin/colibri-daemon`; if that binary isn't staged, `service clawdie start` will fail on the missing exec — expected. Confirm the script + enable are correct regardless. + - The script execs `/usr/local/bin/colibri-daemon` through `/usr/sbin/daemon -u clawdie`; if that binary isn't staged, `service clawdie start` will fail on the missing exec — expected. Confirm the script + enable are correct regardless. 4. **Teardown** to reset: ```sh service clawdie stop 2>/dev/null; sysrc -x clawdie_enable From df73740e599b5815fa80cd296394c6ee787798ed Mon Sep 17 00:00:00 2001 From: Sam & Claude Date: Sun, 14 Jun 2026 00:45:02 +0200 Subject: [PATCH 2/2] docs: record FreeBSD clawdie installer findings (Sam & Codex) Add real FreeBSD 15 read-only validation output and the hardening findings so Linux-side reviewers can evaluate the installer follow-up without needing host access.\n\nChecks: ./scripts/check-format.sh; git diff --check --- docs/CLAWDIE-INSTALLER-HANDOFF.md | 48 +++++++++++++++++++++++++++++-- 1 file changed, 45 insertions(+), 3 deletions(-) diff --git a/docs/CLAWDIE-INSTALLER-HANDOFF.md b/docs/CLAWDIE-INSTALLER-HANDOFF.md index e66bc75..5d502e3 100644 --- a/docs/CLAWDIE-INSTALLER-HANDOFF.md +++ b/docs/CLAWDIE-INSTALLER-HANDOFF.md @@ -59,10 +59,52 @@ clawdie apply --pool tank --create-pool /dev/sdX --yes # DESTROYS /dev/sdX Verify `zpool create` + datasets + systemd unit (`systemctl status clawdie`). Confirm the guard: `--create-pool` on a **non-empty** disk is refused without `--force`. -## 5. Acceptance — delete this doc when all are true +## 5. FreeBSD read-only validation notes (2026-06-13, Codex/Pi) -- [ ] `cargo test -p clawdie` passes on FreeBSD 15 (output + versions reported). -- [ ] `discover` + `plan` correct against a real FreeBSD ZFS host. +Host/version evidence: + +```text +FreeBSD osa.smilepowered.org 15.0-RELEASE-p10 GENERIC amd64 +rustc 1.94.0 (4a4ef493e 2026-03-02) +cargo 1.94.0 (85eff7c80 2026-01-15) +``` + +Checks run on a real FreeBSD 15 host: + +```sh +cargo fmt --check +./scripts/check-format.sh +git diff --check +cargo test -p clawdie -- --nocapture +cargo clippy -p clawdie --all-targets -- -D warnings +cargo build -p clawdie --release +target/release/clawdie discover +target/release/clawdie plan +target/release/clawdie apply --pool zroot # dry-run only +target/release/clawdie plan --pool does-not-exist # expected error +``` + +Observed results: + +- `cargo test -p clawdie -- --nocapture`: 15 tests passed. +- `discover`: detected `os: FreeBsd`, `zfs available: true`, and pool `zroot [ONLINE]`. +- `plan`: resolved to `ZFS on existing pool zroot` and rendered rc.d provisioning. +- bare `apply --pool zroot`: printed the same plan and exited as a dry-run (`DRY-RUN — nothing written`). +- `plan --pool does-not-exist`: now errors before rendering/apply: `ZFS pool \`does-not-exist\` not found; available pools: zroot`. + +Findings filed for Linux-side review in branch +`fix/clawdie-installer-freebsd-hardening`: + +- generated FreeBSD rc.d now runs `/usr/local/bin/colibri-daemon` through `/usr/sbin/daemon -u clawdie` instead of root; +- service installation chowns `/var/db/clawdie` and `/var/log/clawdie` after creating the `clawdie` user; +- existing-pool plans validate the named pool before rendering/applying. + +Not done: no destructive `apply --yes`; still requires scratch pool/VM. + +## 6. Acceptance — delete this doc when all are true + +- [x] `cargo test -p clawdie` passes on FreeBSD 15 (output + versions reported). +- [x] `discover` + `plan` correct against a real FreeBSD ZFS host for read-only/dry-run paths. - [ ] `apply --yes` on a scratch pool creates the datasets, user, and rc.d service as specified; teardown verified. - [ ] (if tested) Linux `--create-pool` works on a spare disk and the empty-disk guard refuses non-empty disks. - [ ] Any FreeBSD-specific differences from the Linux-built behavior are filed as a PR and reported back.