diff --git a/config/clawbox-root-manifest.sh b/config/clawbox-root-manifest.sh new file mode 100644 index 000000000..5cc48c748 --- /dev/null +++ b/config/clawbox-root-manifest.sh @@ -0,0 +1,204 @@ +#!/usr/bin/env bash +# +# Integrity manifest for the code root executes on the clawbox user's behalf. +# +# The privilege chain is: +# +# clawbox --sudo--> systemctl start clawbox-root-update@.service +# --systemd--> /usr/local/libexec/clawbox/clawbox-root-step.sh (root:root) +# --exec--> /home/clawbox/clawbox/install.sh --step +# +# Only the middle link is root-owned. install.sh is `clawbox:clawbox 0755` in a +# `clawbox:clawbox 0775` directory — install.sh itself hands the tree back with +# `chown -R clawbox:clawbox` on every root run — and the steps it dispatches go +# on to run more of the same tree as root (scripts/start-ap.sh, +# scripts/launch-browser.sh, scripts/setup-hermes-edition.sh, …). So anything +# with clawbox-level code execution — the web server, the in-UI terminal, the +# agent's shell — could rewrite the program root was about to run and then +# trigger a granted step. That is passwordless local root in two moves, and it +# is the defect TASK-445 was filed about. +# +# Moving the tree out of clawbox's reach is not an option: the updater has to be +# able to replace it, and the app has to be able to build in it. So instead the +# root side REFUSES to run code it did not record. This file writes and checks +# that record: +# +# * install.sh writes the manifest at the end of every root-side install and +# immediately after every successful `git reset --hard` to the update branch +# (install.sh's bootstrap block and sync_repo_to_update_target). Those are +# the only two ways the covered files are supposed to change. +# * clawbox-root-step.sh verifies it before exec'ing anything. A tampered or +# unrecorded tree fails the step instead of running as root. +# +# What this does and does not buy: +# +# * It closes the "rewrite install.sh, then start a granted unit" path — the +# dispatcher refuses before the exec. +# * It does NOT make the box safe against someone who can already run code as +# root, and it does not authenticate the UPDATE itself: an update legitimately +# replaces the covered files and re-records them. The update path is gated +# on the dashboard session instead (TASK-445's "require auth for update"). +# +# Usage (root only): +# clawbox-root-manifest.sh --write record the tree as it is now +# clawbox-root-manifest.sh --verify exit 0 if it still matches, 65 if not +# +# Installed by install.sh::install_root_libexec to +# /usr/local/libexec/clawbox/clawbox-root-manifest.sh, root:root 0755. + +set -euo pipefail + +# Hard-coded on purpose. Every value below selects WHICH code root executes, so +# none of them is overridable from the environment: this script runs from a +# systemd unit reached through a NOPASSWD sudoers grant, and an env escape hatch +# would be a second way to point root at a file the clawbox user chose. +PROJECT_DIR="/home/clawbox/clawbox" +MANIFEST_DIR="/etc/clawbox" +MANIFEST_FILE="/etc/clawbox/root-exec.manifest" + +# Everything the clawbox-root-update@ chain can end up running as root: +# install.sh, the scripts it hands to bash, and the config/unit files it installs. +# Runtime state — data/, .next/, node_modules/, .git/ — is deliberately NOT +# covered: it is clawbox's to write and root never executes it, so covering it +# would turn every build into a manifest mismatch. +COVERED_PATHS="install.sh scripts config" + +# Generated content that lives INSIDE a covered path, and must not be recorded. +# `scripts/__pycache__/` is the one that bites: gateway-pre-start.sh imports +# scripts/gateway_origins.py, so CPython writes a .pyc there the first time the +# gateway starts — after the manifest was written, and again under a different +# name after any python3 minor-version bump. Recording those would make an +# ordinary first boot, or an ordinary distro upgrade, refuse every root step. +PRUNE_DIRS="__pycache__ node_modules .venv venv" + +die() { + echo "clawbox-root-manifest: $1" >&2 + exit "${2:-65}" +} + +# Covered files, relative to PROJECT_DIR, NUL-delimited and byte-sorted. +# Callers must already be in PROJECT_DIR. +# +# `-type f` excludes symlinks deliberately: what gets RECORDED is a real file +# and its real content. Verification then re-opens the recorded path, so +# replacing one of these with a symlink to something else changes the hash and +# fails — which is the answer we want, rather than recording the link. +covered_files() { + local p + local -a args=() prune=() + for p in $COVERED_PATHS; do + [ -e "$p" ] && args+=("$p") + done + [ "${#args[@]}" -gt 0 ] || return 1 + for p in $PRUNE_DIRS; do + prune+=(-name "$p" -prune -o) + done + find "${args[@]}" "${prune[@]}" -type f -print0 | LC_ALL=C sort -z +} + +write_manifest() { + cd "$PROJECT_DIR" || die "$PROJECT_DIR is missing" 66 + + # ONE walk, so the names that are checked are exactly the names that are + # hashed. Walking twice — once to check, once to hash — leaves a window in + # which a file that appears in between is recorded without ever having been + # checked. + # + # The check itself: sha256sum ESCAPES a filename containing a backslash or a + # newline (it prefixes the line with `\` and re-encodes them), and + # verify_manifest reads the path column back with a fixed-width strip. Refuse + # to record such a name rather than record one this file cannot parse. + local f + local -a files=() + while IFS= read -r -d '' f; do + case "$f" in + *\\*|*$'\n'*) + die "refusing to record a path containing a backslash or a newline" + ;; + esac + files+=("$f") + done < <(covered_files) + [ "${#files[@]}" -gt 0 ] || die "nothing to record under $PROJECT_DIR" 66 + + install -d -o root -g root -m 0755 "$MANIFEST_DIR" || die "cannot create $MANIFEST_DIR" 66 + + # Staged inside the root-owned /etc/clawbox, never /tmp: a world-writable + # staging directory is one more place to race the file root ends up trusting. + local tmp + tmp="$(mktemp "$MANIFEST_FILE.XXXXXX")" || die "cannot stage a manifest" 66 + if ! printf '%s\0' "${files[@]}" | xargs -0 sha256sum > "$tmp"; then + rm -f "$tmp" + die "cannot hash $PROJECT_DIR" 66 + fi + if ! chmod 0644 "$tmp"; then + rm -f "$tmp" + die "cannot set the manifest mode" 66 + fi + if ! mv -f "$tmp" "$MANIFEST_FILE"; then + rm -f "$tmp" + die "cannot install $MANIFEST_FILE" 66 + fi +} + +verify_manifest() { + [ -f "$MANIFEST_FILE" ] || die "no manifest at $MANIFEST_FILE" + cd "$PROJECT_DIR" || die "$PROJECT_DIR is missing" 66 + + # Every recorded file must still be there and still hash to what was recorded. + # That covers the three things that matter: an edited file, a deleted file, and + # a file replaced by a symlink (sha256sum opens the path, so it hashes what the + # link resolves to and the content stops matching). + # + # A file ADDED under a covered path is deliberately NOT an error, even though + # `sha256sum -c` cannot see it. Root only ever executes files install.sh names + # explicitly, and all of those are recorded — so an unrecorded file is not + # something root can be made to run. Treating additions as tampering, on the + # other hand, turns any stray file under scripts/ into a device that refuses + # every root step for good: no password change, no hostname change, no hotspot + # restart, on an appliance with no console. That trade is the wrong way round. + sha256sum --status --strict -c "$MANIFEST_FILE" \ + || die "$PROJECT_DIR does not match $MANIFEST_FILE (a covered file changed or is gone)" +} + +# Check ONE already-opened copy against what the manifest recorded for a path. +# +# `--verify` answers a question about the project tree, and the answer is stale +# the moment it returns: the clawbox user can replace a file between the check +# and the exec, and a tight rewrite loop wins that race. So the root dispatcher +# copies the file it is going to run into a root-only directory FIRST and then +# asks about the copy — which is the same bytes it will execute, and which +# clawbox cannot touch. +# +# clawbox-root-manifest.sh --verify-file +verify_file() { + local rel="$1" actual="$2" want="" got h p + [ -n "$rel" ] && [ -n "$actual" ] || die "usage: $0 --verify-file " 64 + [ -f "$MANIFEST_FILE" ] || die "no manifest at $MANIFEST_FILE" + [ -f "$actual" ] || die "$actual is missing" 66 + + # Read the recorded hash out of the sha256sum-format manifest by exact path + # match. write_manifest refuses names it would have to escape, so the path + # column is the plain name (with a leading `*` in binary mode). + while read -r h p; do + p="${p#\*}" + if [ "$p" = "$rel" ]; then + want="$h" + break + fi + done < "$MANIFEST_FILE" + [ -n "$want" ] || die "$rel is not in $MANIFEST_FILE" + + got="$(sha256sum < "$actual")" + got="${got%% *}" + [ "$want" = "$got" ] || die "$actual does not match what $MANIFEST_FILE recorded for $rel" +} + +case "${1:-}" in + --write) write_manifest ;; + --verify) verify_manifest ;; + --verify-file) verify_file "${2:-}" "${3:-}" ;; + *) + echo "usage: $0 --write|--verify|--verify-file " >&2 + exit 64 + ;; +esac diff --git a/config/clawbox-root-step.sh b/config/clawbox-root-step.sh index 2e8a0c4ce..f9c39ca77 100644 --- a/config/clawbox-root-step.sh +++ b/config/clawbox-root-step.sh @@ -12,13 +12,27 @@ # to execute": a scoped NOPASSWD grant that is a one-step local root. TASK-445. # # This script cannot make install.sh itself immutable — the updater has to be -# able to replace it — so it does the two things a root-owned entrypoint can: +# able to replace it, and the app has to be able to build in the same tree — so +# it does the three things a root-owned entrypoint can: # -# 1. Validates the instance name against its own allow-list. The sudoers grant -# is `clawbox-root-update@*.service`, so without this the step name is +# 1. Validates the instance name against its own allow-list. Even with the +# sudoers grants enumerated per instance, systemd will happily start +# `clawbox-root-update@anything.service`, so without this the step name is # unvalidated input on the root side of the boundary. # -# 2. Decides whether this step may self-update. install.sh's bootstrap block +# 2. Refuses to exec a tree it did not record — for every step that is NOT an +# update. install.sh writes a root-owned sha256 manifest of everything root +# runs on clawbox's behalf (install.sh, scripts/, config/) at the end of +# every install and immediately after every successful `git reset --hard` to +# the update branch; this script verifies it before the exec below. Without +# that check, "clawbox may start clawbox-root-update@chpasswd.service" also +# means "clawbox may choose the program root runs", because install.sh is +# clawbox:clawbox 0755 inside a clawbox-writable directory — a one-step +# local root. See clawbox-root-manifest.sh for what the record does and does +# not cover, and the comment on the check below for why the update family is +# excluded. +# +# 3. Decides whether this step may self-update. install.sh's bootstrap block # does `git fetch` + `git reset --hard origin/` + re-exec, and it # ran on EVERY `--step` — including `chpasswd`. A password change must not # reach out to the network, and must not be a way to pull new code onto the @@ -32,6 +46,8 @@ set -euo pipefail PROJECT_DIR="/home/clawbox/clawbox" ENTRYPOINT="$PROJECT_DIR/install.sh" +MANIFEST_HELPER="/usr/local/libexec/clawbox/clawbox-root-manifest.sh" +RUN_DIR="/run/clawbox" step="${1:-}" @@ -92,6 +108,77 @@ if contains "$step" "$SELF_UPDATING_STEPS"; then else # Pin this run to the on-disk copy: no fetch, no reset --hard, no re-exec. export CLAWBOX_INSTALL_BOOTSTRAPPED=1 + + # ...and, because it is pinned, root must be able to say what "the on-disk + # copy" is. Verify the record before the exec below. + # + # ONLY for the pinned steps, and that asymmetry is the whole design: + # + # * These are the steps a foothold can reach and repeat — chpasswd, + # set_hostname, restart_ap, llamacpp_install are the four instances + # config/clawbox-sudoers grants. Nothing about them is supposed to change + # the covered files, so a mismatch is tampering and root refuses. + # * The update family is excluded because an update IS a legitimate rewrite + # of exactly these files, and it is not always install.sh that performs it: + # src/lib/updater.ts does its own fetch/reset/clean as the clawbox user + # before it starts the rebuild step, and scripts/force-update.sh does the + # same by hand. Verifying here would fail those flows at their next step + # and leave the device refusing every root step afterwards. Instead the + # update family re-records as its first action (install.sh's bootstrap + # block does it right after `git reset --hard`), which is also what heals + # a device whose tree was replaced from the outside. + # * That is not a hole the allow-list leaves open: TASK-445 removed every + # sudo grant for a self-updating instance, so `sudo systemctl start + # clawbox-root-update@git_pull.service` is denied. What can still reach + # them is the unscoped polkit `manage-units` grant, tracked as TASK-539 — + # and when that goes, the update path must NOT simply be re-granted + # through sudo without moving the git work itself to the root side. + if [ ! -x "$MANIFEST_HELPER" ]; then + echo "clawbox-root-step: $MANIFEST_HELPER is missing — cannot tell what root is about to run" >&2 + echo "clawbox-root-step: recover with: sudo bash $ENTRYPOINT --step systemd_services" >&2 + exit 65 + fi + if ! "$MANIFEST_HELPER" --verify; then + echo "clawbox-root-step: refusing '$step' — $PROJECT_DIR does not match the root-exec manifest." >&2 + echo "clawbox-root-step: root will not run code it did not record. If this is a deliberate" >&2 + echo "clawbox-root-step: local change, re-record it as the operator: sudo bash $ENTRYPOINT --step systemd_services" >&2 + exit 65 + fi + + # COPY, then check the copy, then run the copy. + # + # Verifying $ENTRYPOINT and then exec'ing $ENTRYPOINT is a race: bash opens + # the file after the check returns, and the clawbox user can replace it in + # between — a rewrite loop wins that window easily. Hashing a copy that + # clawbox cannot reach removes the window for the one file this script + # executes directly. + # + # /run is tmpfs and root-owned, so the copy cannot survive a reboot and cannot + # be touched by clawbox. The name is fixed rather than mktemp'd because `exec` + # replaces this shell and no EXIT trap would ever fire to clean it up. + STAGED_ENTRYPOINT="$RUN_DIR/root-step-install.sh" + if ! install -d -o root -g root -m 0700 "$RUN_DIR"; then + echo "clawbox-root-step: cannot create $RUN_DIR" >&2 + exit 66 + fi + rm -f "$STAGED_ENTRYPOINT" + if ! install -o root -g root -m 0500 "$ENTRYPOINT" "$STAGED_ENTRYPOINT"; then + echo "clawbox-root-step: cannot stage $ENTRYPOINT for execution" >&2 + exit 66 + fi + if ! "$MANIFEST_HELPER" --verify-file install.sh "$STAGED_ENTRYPOINT"; then + rm -f "$STAGED_ENTRYPOINT" + echo "clawbox-root-step: refusing '$step' — install.sh changed between the check and the copy." >&2 + exit 65 + fi + ENTRYPOINT="$STAGED_ENTRYPOINT" + + # Residual, recorded rather than implied: the scripts install.sh goes on to run + # as root (scripts/start-ap.sh, launch-browser.sh, setup-hermes-edition.sh, …) + # are covered by the --verify above but are opened LATER, by install.sh itself, + # so the same window exists for them. Closing it means the tree install.sh + # reads from being root-owned too — the follow-up this design is pointed at, + # and a bigger change than a copy of one file. TASK-445. fi exec /bin/bash "$ENTRYPOINT" --step "$step" diff --git a/config/clawbox-root-update@.service b/config/clawbox-root-update@.service index 6f216dd65..f87760f8f 100644 --- a/config/clawbox-root-update@.service +++ b/config/clawbox-root-update@.service @@ -14,10 +14,12 @@ EnvironmentFile=-/etc/clawbox/edition.env # Root-owned entrypoint, outside every clawbox-writable directory. Pointing # ExecStart straight at /home/clawbox/clawbox/install.sh meant the clawbox user # — who the web server, the in-UI terminal and the agent's shell all run as — -# could edit the file root was about to execute, turning the scoped -# `clawbox-root-update@*.service` sudoers grant into a one-step local root. The -# dispatcher also validates %i (that grant accepts ANY instance name) and only -# lets the update family run install.sh's git-fetch/reset self-update. TASK-445. +# could edit the file root was about to execute, turning a scoped +# clawbox-root-update@ sudoers grant into a one-step local root. The dispatcher +# closes that: it verifies a root-owned sha256 manifest of everything root runs +# on clawbox's behalf before it execs anything. It also validates %i (systemd +# starts whatever instance name it is handed, whoever reached it) and only lets +# the update family run install.sh's git-fetch/reset self-update. TASK-445. ExecStart=/usr/local/libexec/clawbox/clawbox-root-step.sh %i # 30 min was not enough for llamacpp_install on a cold box: that step builds # llama.cpp from source with CUDA on a 6-core Jetson Orin AND downloads the diff --git a/config/clawbox-sudoers b/config/clawbox-sudoers index ae4605ad7..a3cb2862b 100644 --- a/config/clawbox-sudoers +++ b/config/clawbox-sudoers @@ -20,8 +20,11 @@ # `--runtime unmask` grant further down cannot undo a persistent /etc mask. clawbox ALL=(root) NOPASSWD: /usr/bin/systemctl restart clawbox-gateway.service clawbox ALL=(root) NOPASSWD: /usr/bin/systemctl restart clawbox-gateway -clawbox ALL=(root) NOPASSWD: /usr/bin/systemctl start clawbox-gateway.service -clawbox ALL=(root) NOPASSWD: /usr/bin/systemctl start clawbox-gateway +# No `start` grant for clawbox-gateway, clawbox-setup or clawbox-tunnel. All three +# are only ever brought up through the `restart` grants here — and restart starts a +# stopped unit too — so `start` was a second spelling of a privilege nothing in the +# tree exercised. clawbox-browser keeps its `start` because the Browser app really +# does call it (src/app/setup-api/browser/route.ts). TASK-445 round 2. clawbox ALL=(root) NOPASSWD: /usr/bin/systemctl stop clawbox-gateway.service clawbox ALL=(root) NOPASSWD: /usr/bin/systemctl stop clawbox-gateway # Mask/unmask: factory reset masks the gateway before wiping ~/.openclaw @@ -37,18 +40,14 @@ clawbox ALL=(root) NOPASSWD: /usr/bin/systemctl --runtime unmask clawbox-gatewa clawbox ALL=(root) NOPASSWD: /usr/bin/systemctl --runtime unmask clawbox-gateway clawbox ALL=(root) NOPASSWD: /usr/bin/systemctl restart clawbox-setup.service clawbox ALL=(root) NOPASSWD: /usr/bin/systemctl restart clawbox-setup -clawbox ALL=(root) NOPASSWD: /usr/bin/systemctl start clawbox-setup.service -clawbox ALL=(root) NOPASSWD: /usr/bin/systemctl start clawbox-setup -clawbox ALL=(root) NOPASSWD: /usr/bin/systemctl restart clawbox-ap.service -clawbox ALL=(root) NOPASSWD: /usr/bin/systemctl restart clawbox-ap -clawbox ALL=(root) NOPASSWD: /usr/bin/systemctl stop clawbox-ap.service -clawbox ALL=(root) NOPASSWD: /usr/bin/systemctl stop clawbox-ap +# clawbox-ap deliberately has NO grant. Settings -> Hotspot restarts the AP +# through clawbox-root-update@restart_ap.service (covered by the template grant +# below), scripts/stop-ap.sh runs unprivileged, and every other AP restart is +# inside install.sh, which is already root. clawbox ALL=(root) NOPASSWD: /usr/bin/systemctl start clawbox-browser.service clawbox ALL=(root) NOPASSWD: /usr/bin/systemctl start clawbox-browser clawbox ALL=(root) NOPASSWD: /usr/bin/systemctl stop clawbox-browser.service clawbox ALL=(root) NOPASSWD: /usr/bin/systemctl stop clawbox-browser -clawbox ALL=(root) NOPASSWD: /usr/bin/systemctl start clawbox-tunnel.service -clawbox ALL=(root) NOPASSWD: /usr/bin/systemctl start clawbox-tunnel clawbox ALL=(root) NOPASSWD: /usr/bin/systemctl stop clawbox-tunnel.service clawbox ALL=(root) NOPASSWD: /usr/bin/systemctl stop clawbox-tunnel clawbox ALL=(root) NOPASSWD: /usr/bin/systemctl restart clawbox-tunnel.service @@ -63,6 +62,36 @@ clawbox ALL=(root) NOPASSWD: /usr/bin/systemctl enable clawbox-tunnel clawbox ALL=(root) NOPASSWD: /usr/bin/systemctl disable clawbox-tunnel.service clawbox ALL=(root) NOPASSWD: /usr/bin/systemctl disable clawbox-tunnel +# Hermes' messaging gateway. This is the unit `hermes gateway install --system` +# writes, and it is the process that RECEIVES Telegram / WhatsApp / Discord / +# Email — so every config save on a provisioned Hermes box restarts it +# (src/lib/hermes-telegram.ts ensureHermesGateway). It is NOT install-time-only. +# +# This grant is a strict REDUCTION in privilege, not an addition: +# +# * The unit is root-owned in /etc/systemd/system and runs `User=clawbox` +# (Hermes writes it that way — `gateway install --system --run-as-user +# clawbox`). Restarting it therefore starts a process the clawbox user could +# have started itself. That is less than the `restart clawbox-gateway` grant +# above, whose unit has no User= at all. +# * It REPLACES `sudo -n /home/clawbox/.local/bin/hermes gateway restart +# --system`. That binary, and every directory above it, is clawbox-owned and +# clawbox-writable — exactly the shape the optimize-ollama.sh grant was moved +# to /usr/local/libexec to avoid. It could never be granted; the coverage +# checker had to carry it in EXEMPT_CALLS instead, which meant the restart +# silently failed on a narrowed box and the route still answered "restarted". +# +# No `start`: the restart grant starts a stopped unit too. No `reset-failed`: +# the generated unit sets StartLimitIntervalSec=0, so there is no start limit to +# clear. No wildcard: profile units (hermes-gateway-.service) are a +# different privilege question and ClawBox only ever runs the default profile. +# The first-time `gateway install --system` stays deliberately UNGRANTED — it +# writes into /etc/systemd/system and cannot be expressed safely against a +# clawbox-writable binary; it fails fast under `sudo -n` and the route now says +# so instead of reporting success. TASK-445 follow-up. +clawbox ALL=(root) NOPASSWD: /usr/bin/systemctl restart hermes-gateway.service +clawbox ALL=(root) NOPASSWD: /usr/bin/systemctl restart hermes-gateway + # Ollama, for Settings -> Local Models. Ollama is a system unit and it holds # real RAM on an 8 GB box, so the owner has to be able to stop it and have it # stay stopped; until this grant existed nothing in the UI could. `--now` is @@ -86,13 +115,37 @@ clawbox ALL=(root) NOPASSWD: /usr/bin/systemctl stop ollama.service # Update / power / install paths. # -# `reset-failed *` and `start --no-block *` used to take ANY unit name. Both are -# only ever called with a clawbox-* unit (src/lib/updater.ts, the llamacpp -# installer, the credentials chpasswd path, install/run-step), so scope them to -# that prefix rather than handing over the whole unit namespace. TASK-445. -clawbox ALL=(root) NOPASSWD: /usr/bin/systemctl reset-failed clawbox-* -clawbox ALL=(root) NOPASSWD: /usr/bin/systemctl start --no-block clawbox-* -clawbox ALL=(root) NOPASSWD: /usr/bin/systemctl start clawbox-root-update@*.service +# NO WILDCARDS, DELIBERATELY. These were three prefix rules — `reset-failed +# clawbox-*`, `start --no-block clawbox-*` and `start +# clawbox-root-update@*.service` — and a prefix is not a scope here. sudoers(5) +# matches a command's arguments as one CONCATENATED string, so `*` spans +# whitespace; `systemctl start` takes a LIST of units. That made +# +# sudo /usr/bin/systemctl start clawbox-root-update@chpasswd.service ssh.service +# +# a match, i.e. the three rules together read "start, or reset-failed, ANY unit +# as root without a password, as long as the first word begins with clawbox-". +# Reproduced against sudo 1.9.9 — the Ubuntu 22.04 vintage the appliance runs — +# with `sudo -U clawbox -l `, which tests matching without executing. +# +# Regex Cmnd arguments would express the intent directly, but they arrived in +# sudo 1.9.10, after the version on the device. So: enumeration, one exact rule +# per (verb, unit) pair the product actually issues. The list is short because +# only four `clawbox-root-update@` instances are reachable from the web server; +# every other step is an operator path (`sudo bash install.sh --step …`) or runs +# inside the updater's own root chain. +# +# Adding an instance here adds a root entrypoint — the granted step runs as root +# through /usr/local/libexec/clawbox/clawbox-root-step.sh — so review it as a +# privilege boundary, not as a config line. TASK-445. +clawbox ALL=(root) NOPASSWD: /usr/bin/systemctl reset-failed clawbox-root-update@chpasswd.service +clawbox ALL=(root) NOPASSWD: /usr/bin/systemctl start clawbox-root-update@chpasswd.service +clawbox ALL=(root) NOPASSWD: /usr/bin/systemctl reset-failed clawbox-root-update@set_hostname.service +clawbox ALL=(root) NOPASSWD: /usr/bin/systemctl start clawbox-root-update@set_hostname.service +clawbox ALL=(root) NOPASSWD: /usr/bin/systemctl reset-failed clawbox-root-update@restart_ap.service +clawbox ALL=(root) NOPASSWD: /usr/bin/systemctl start clawbox-root-update@restart_ap.service +clawbox ALL=(root) NOPASSWD: /usr/bin/systemctl reset-failed clawbox-root-update@llamacpp_install.service +clawbox ALL=(root) NOPASSWD: /usr/bin/systemctl start --no-block clawbox-root-update@llamacpp_install.service clawbox ALL=(root) NOPASSWD: /usr/bin/systemctl reboot clawbox ALL=(root) NOPASSWD: /usr/bin/systemctl poweroff # apt is granted ONLY for the exact commands the Browser-app chromium recovery diff --git a/e2e-install/06-sudoers.spec.ts b/e2e-install/06-sudoers.spec.ts new file mode 100644 index 000000000..3477fc1a0 --- /dev/null +++ b/e2e-install/06-sudoers.spec.ts @@ -0,0 +1,331 @@ +/** + * Root escalation surface, checked on a device that install.sh actually + * provisioned. TASK-445. + * + * The QA-box revalidation of PR #436 found the narrowing landed in the repo and + * nowhere else: /etc/sudoers.d still carried `90-clawbox-nopasswd` with + * `clawbox ALL=(ALL) NOPASSWD: ALL` from provisioning, /usr/local/libexec did + * not exist at all, and the deployed bundle was calling a helper there. Every + * assertion below is a thing that was true in the repo and false on the box, so + * they are made against the container's real filesystem rather than the source. + * + * The container is seeded with exactly that blanket drop-in (e2e-install/ + * Dockerfile writes it so volume permissions work before install.sh runs), which + * makes it a genuine fixture for the migration: if quarantine_overbroad_sudoers + * did not run, the first test fails. + * + * Read-only: every command here inspects state, none changes it. Runs at NN=06, + * after the captive-portal probes and before the wizard mutates anything. + */ +import { test, expect } from "@playwright/test"; +import { dockerExec } from "./helpers/container"; + +/** `sudo -n -l` for the clawbox user, i.e. what the web server may run as root. */ +async function sudoList(): Promise { + return dockerExec(["bash", "-lc", "sudo -n -l 2>&1 || true"], { user: "clawbox" }); +} + +/** + * Does the NOPASSWD allow-list cover these command lines? + * + * Behavioural, not `sudo -n -l`. install.sh puts clawbox in the `sudo` group and + * the distro ships `%sudo ALL=(ALL:ALL) ALL`, so `sudo -l ` answers + * "yes, with a password" for every command on the box — it cannot tell the + * allow-list from the group rule. `sudo -n ` can: a command the allow-list + * covers runs, and one it does not falls through to the password-gated group + * rule, where `-n` makes sudo refuse with "a password is required" instead of + * prompting. That string is the answer, and it is also exactly what the web + * server sees, since every call site uses `sudo -n`. + * + * One container round-trip for the whole set; answers come back keyed by the + * command so a failure still names the exact probe. + */ +async function sudoCovers(cmds: string[]): Promise> { + const script = cmds + .map((c) => + `out="$(sudo -n ${c} 2>&1)"; case "$out" in *"password is required"*|*"not allowed to execute"*)` + + ` v=DENIED ;; *) v=ALLOWED ;; esac; printf '%s\\t%s\\n' ${JSON.stringify(c)} "$v"`) + .join("\n"); + const out = await dockerExec(["bash", "-lc", script], { user: "clawbox" }); + return Object.fromEntries( + out.split("\n").filter(Boolean).map((l) => { + const [cmd, verdict] = l.split("\t"); + return [cmd, verdict]; + }), + ); +} + +test.describe("root escalation surface", () => { + test("no drop-in grants clawbox unrestricted passwordless root", async () => { + const rules = await dockerExec([ + "bash", "-lc", + "grep -rhv '^[[:space:]]*#' /etc/sudoers.d/ 2>/dev/null | grep -v '^[[:space:]]*$' || true", + ]); + for (const line of rules.split("\n")) { + if (!/^\s*(clawbox|%clawbox)\s/.test(line)) continue; + expect(line, "a blanket NOPASSWD ALL grant survived the install").not.toMatch(/NOPASSWD:\s*ALL\s*$/); + } + + const listed = await sudoList(); + expect(listed).not.toMatch(/NOPASSWD:\s*ALL\s*$/m); + + // `(ALL : ALL) ALL` from the `sudo` GROUP is expected and is deliberately + // NOT what this task removes. install.sh's step_ensure_user puts clawbox in + // `sudo`, `video`, `audio`, `i2c`, `gpio` on every device, and the distro's + // `%sudo ALL=(ALL:ALL) ALL` demands a password — so it is the owner's own + // administrator account, not a path the web server can take. Stripping it + // would lock the only administrator out of an appliance that has no + // console, which is why quarantine_overbroad_sudoers only ever looks at + // `clawbox`/`%clawbox` NOPASSWD rules. + // + // So assert the BEHAVIOUR rather than the rendering: the web server only + // ever runs `sudo -n`, and under a blanket grant `sudo -n ` + // succeeds. Each probe below is a command no line in the allow-list names, + // so a pass here means an unrestricted rule is reachable without a password. + for (const probe of ["id -u", "cat /etc/shadow", "install -m 0755 /bin/true /usr/local/bin/pwn"]) { + const out = await dockerExec( + ["bash", "-lc", `sudo -n ${probe} >/dev/null 2>&1 && echo ESCALATED || echo DENIED`], + { user: "clawbox" }, + ); + expect(out.trim(), `\`sudo -n ${probe}\` must not be permitted`).toBe("DENIED"); + } + }); + + test("the removed drop-in is kept, root-only, where it can be explained", async () => { + const kept = await dockerExec([ + "bash", "-lc", + "ls -1 /var/lib/clawbox/sudoers-quarantine/ 2>/dev/null || true", + ]); + // The Dockerfile seeds BOTH blanket names a device is seen with — the + // factory-baked `90-clawbox-nopasswd` and the field name `clawbox-nopasswd` + // — so both must have been moved aside. Neither is written by this repo: + // they arrive in the flashed image, which is why the installer has to + // remove them rather than merely stop writing them. + expect(kept).toMatch(/(^|\n)clawbox-nopasswd\.\d{8}T\d{6}Z/); + expect(kept).toMatch(/(^|\n)90-clawbox-nopasswd\.\d{8}T\d{6}Z/); + + const perms = await dockerExec([ + "bash", "-lc", + "stat -c '%a %U:%G' /var/lib/clawbox/sudoers-quarantine", + ]); + expect(perms.trim()).toBe("700 root:root"); + }); + + test("clawbox can still do exactly what the product needs", async () => { + const listed = await sudoList(); + // A failure here is a device that has lost a feature to a password prompt + // nobody can answer, so each entry names the path that would break. + for (const [what, needle] of [ + ["factory reset / power menu", "/usr/bin/systemctl reboot"], + ["power menu", "/usr/bin/systemctl poweroff"], + ["password change", "/usr/bin/systemctl start clawbox-root-update@chpasswd.service"], + ["hostname change", "/usr/bin/systemctl start clawbox-root-update@set_hostname.service"], + ["hotspot restart", "/usr/bin/systemctl start clawbox-root-update@restart_ap.service"], + ["llama.cpp installer hand-off", "/usr/bin/systemctl start --no-block clawbox-root-update@llamacpp_install.service"], + ["gateway restart after a config write", "/usr/bin/systemctl restart clawbox-gateway.service"], + ["web server restart (force-update.sh)", "/usr/bin/systemctl restart clawbox-setup.service"], + ["Settings → Local Models", "/usr/bin/systemctl disable --now ollama.service"], + ["Settings → Desktop", "/usr/local/libexec/clawbox/clawbox-desktop-mode.sh --disable"], + ["Settings → Performance mode", "/usr/local/libexec/clawbox/clawbox-power-mode.sh --performance"], + ["saving a local Ollama model", "/usr/local/libexec/clawbox/optimize-ollama.sh"], + ] as const) { + expect(listed, `${what} lost its sudo grant`).toContain(needle); + } + }); + + test("every granted libexec helper exists, root-owned and 0755", async () => { + const granted = (await sudoList()) + .split("\n") + .map((l) => l.match(/(\/usr\/local\/libexec\/clawbox\/[\w.-]+)/)?.[1]) + .filter((p): p is string => !!p); + expect(granted.length).toBeGreaterThan(0); + expect(granted).toContain("/usr/local/libexec/clawbox/optimize-ollama.sh"); + + for (const script of new Set(granted)) { + const stat = await dockerExec([ + "bash", "-lc", + `stat -c '%a %U:%G' ${script} 2>&1 || echo MISSING`, + ]); + expect(stat.trim(), `${script} is granted but not installed correctly`).toBe("755 root:root"); + } + + // The directories above them must be root-owned too, or the grant is + // decorative: clawbox could replace the file root is about to run. + for (const dir of ["/usr/local/libexec", "/usr/local/libexec/clawbox"]) { + const stat = await dockerExec(["bash", "-lc", `stat -c '%U:%G' ${dir}`]); + expect(stat.trim(), `${dir} must be root-owned`).toBe("root:root"); + } + }); + + test("no grant points into the clawbox-writable project tree", async () => { + const listed = await sudoList(); + expect(listed).not.toContain("/home/clawbox/clawbox"); + }); + + test("no granted command is owned or writable by clawbox", async () => { + // Stronger than the string check above, and the invariant the audit asked + // for: resolve every granted path on the real filesystem and require root + // ownership with no group/other write bit. A grant on a file clawbox can + // rewrite is passwordless local root whatever the path happens to read as. + const paths = [...new Set( + (await sudoList()) + .split("\n") + .flatMap((l) => l.match(/\/[\w./@-]+/g) ?? []) + .filter((p) => /^\/(usr|bin|sbin|home|tmp|var|opt|etc)\//.test(p)), + )]; + expect(paths.length).toBeGreaterThan(0); + // `|| true`: a granted path that does not exist on this device (no snapd in + // the container, so no /usr/bin/snap) makes stat exit non-zero, and + // dockerExec throws on that. The missing ones simply do not come back. + const stats = await dockerExec(["bash", "-lc", `stat -c '%n %U %a' ${paths.join(" ")} 2>/dev/null || true`]); + const seen = new Set(); + for (const line of stats.split("\n").filter(Boolean)) { + const [name, owner, mode] = line.split(" "); + seen.add(name); + expect(owner, `${name} is granted but owned by ${owner}`).toBe("root"); + expect(parseInt(mode, 8) & 0o022, `${name} is group- or world-writable`).toBe(0); + } + // Every libexec helper must resolve — those are ours to install, and the + // test above already pins their mode. A missing /usr/bin/snap is not a + // finding here: the container has no snapd, and the grant is inert without + // it. What matters is that nothing that DOES resolve is clawbox-writable. + for (const p of paths.filter((x) => x.startsWith("/usr/local/libexec/clawbox/"))) { + expect(seen.has(p), `${p} is granted but not installed`).toBe(true); + } + expect(seen.size, "no granted path resolved at all — the probe is broken").toBeGreaterThan(2); + }); + + test("a wildcard cannot swallow a second unit name (GAP 3)", async () => { + // sudoers matches arguments as ONE concatenated string, so `clawbox-*` also + // matched ` ` — and `systemctl start` takes a + // LIST of units, which made those rules "start any unit as root". + // + // The appended unit is a name that does not exist on purpose: if a rule ever + // matches again this test fails without having started anything real. + const PAD = "e2e-nonexistent-probe.service"; + const probes: Record = { + [`reset-failed clawbox-root-update@chpasswd.service ${PAD}`]: "DENIED", + [`reset-failed clawbox-root-update@llamacpp_install.service ${PAD}`]: "DENIED", + [`start clawbox-root-update@chpasswd.service ${PAD}`]: "DENIED", + [`start --no-block clawbox-setup.service ${PAD}`]: "DENIED", + // No grant names an instance outside the four the product issues, so the + // template is no longer a way to run an arbitrary step as root. (Those + // instances stay reachable through the unscoped polkit `manage-units` + // grant until TASK-539 removes it — this asserts the allow-list, not the + // whole surface.) + "start clawbox-root-update@e2e-not-a-step.service": "DENIED", + "start --no-block clawbox-root-update@e2e-not-a-step.service": "DENIED", + // Control: something the product really issues still runs without a + // password, so a pass above cannot just be "sudo denies everything". + // reset-failed on a unit that never ran is a no-op. + "reset-failed clawbox-root-update@chpasswd.service": "ALLOWED", + }; + const answers = await sudoCovers(Object.keys(probes).map((c) => `/usr/bin/systemctl ${c}`)); + for (const [cmd, want] of Object.entries(probes)) { + expect(answers[`/usr/bin/systemctl ${cmd}`], `sudo -n /usr/bin/systemctl ${cmd}`).toBe(want); + } + }); + + test("root records the code it is allowed to run (GAP 2)", async () => { + const stat = (await dockerExec([ + "bash", "-lc", + "stat -c '%a %U:%G' /etc/clawbox/root-exec.manifest 2>&1 || echo MISSING", + ])).trim(); + expect(stat, "install.sh did not write the root-exec manifest").toBe("644 root:root"); + + const helper = (await dockerExec([ + "bash", "-lc", + "stat -c '%a %U:%G' /usr/local/libexec/clawbox/clawbox-root-manifest.sh 2>&1 || echo MISSING", + ])).trim(); + expect(helper).toBe("755 root:root"); + + // It has to cover install.sh itself AND the scripts a root step goes on to + // run — install.sh is only the first file root executes out of that tree. + const covered = await dockerExec(["cat", "/etc/clawbox/root-exec.manifest"]); + expect(covered).toContain("install.sh"); + expect(covered).toContain("scripts/start-ap.sh"); + expect(covered).toContain("config/clawbox-root-update@.service"); + + // And it must describe the device as install.sh actually left it. + const verify = await dockerExec([ + "bash", "-lc", + "/usr/local/libexec/clawbox/clawbox-root-manifest.sh --verify >/dev/null 2>&1; echo rc=$?", + ]); + expect(verify.trim()).toBe("rc=0"); + }); +}); + +/** + * The refusals, exercised. Unlike the block above these CHANGE state: each one + * plants something, asserts the root side refuses it, and puts the container + * back. Kept in their own describe so the read-only block stays read-only. + */ +test.describe("root escalation surface — the refusals, exercised", () => { + test("the root dispatcher refuses a tree it did not record (GAP 2)", async () => { + // Rewrite a script a root step really runs — GAP 2 is about the indirection, + // not just install.sh — then put the original bytes back. An ADDED file is + // deliberately not tampering (see config/clawbox-root-manifest.sh), so the + // probe has to change content. + const victim = "/home/clawbox/clawbox/scripts/start-ap.sh"; + try { + await dockerExec([ + "bash", "-lc", + `cp -a ${victim} /tmp/e2e-start-ap.orig && echo '# tampered' >> ${victim}`, + ], { user: "clawbox" }); + const refused = await dockerExec([ + "bash", "-lc", + "/usr/local/libexec/clawbox/clawbox-root-step.sh set_hostname >/dev/null 2>&1; echo rc=$?", + ]); + expect(refused.trim(), "root ran a step against a rewritten tree").toBe("rc=65"); + } finally { + await dockerExec([ + "bash", "-lc", + `cp -a /tmp/e2e-start-ap.orig ${victim} && rm -f /tmp/e2e-start-ap.orig`, + ], { user: "clawbox" }); + } + + // ...and it verifies again once the tree matches its record, so the refusal + // above is the tamper rather than a permanently broken device. + const ok = await dockerExec([ + "bash", "-lc", + "/usr/local/libexec/clawbox/clawbox-root-manifest.sh --verify >/dev/null 2>&1; echo rc=$?", + ]); + expect(ok.trim()).toBe("rc=0"); + }); + + test("the password step refuses a record naming another account (GAP 2b)", async () => { + // The escalation as it stood: data/ is clawbox-writable, so the record was + // attacker-choosable, and the root side validated nothing about it. + const before = (await dockerExec(["bash", "-lc", "getent shadow root | cut -d: -f2"])).trim(); + const input = "/home/clawbox/clawbox/data/.chpasswd-input"; + try { + await dockerExec(["bash", "-lc", `echo 'root:clawbox-e2e-must-not-apply' > ${input}`], { user: "clawbox" }); + const out = await dockerExec([ + "bash", "-lc", + "systemctl start clawbox-root-update@chpasswd.service >/dev/null 2>&1; echo rc=$?", + ]); + expect(out.trim(), "the chpasswd step accepted a root: record").not.toBe("rc=0"); + } finally { + await dockerExec([ + "bash", "-lc", + `rm -f ${input}; systemctl reset-failed clawbox-root-update@chpasswd.service >/dev/null 2>&1 || true`, + ]); + } + const after = (await dockerExec(["bash", "-lc", "getent shadow root | cut -d: -f2"])).trim(); + expect(after, "root's password hash changed").toBe(before); + }); + + test("the root-update unit runs the root-owned entrypoint", async () => { + const unit = await dockerExec([ + "bash", "-lc", + "systemctl cat clawbox-root-update@.service 2>&1 | grep -i '^ExecStart' || true", + ]); + expect(unit).toContain("/usr/local/libexec/clawbox/clawbox-root-step.sh"); + expect(unit).not.toContain("/home/clawbox/clawbox/install.sh"); + }); + + test("the whole sudoers set still parses", async () => { + const out = await dockerExec(["bash", "-lc", "visudo -c 2>&1; echo rc=$?"]); + expect(out).toContain("rc=0"); + }); +}); diff --git a/e2e-install/85-tunnel.spec.ts b/e2e-install/85-tunnel.spec.ts index d5b8c201f..3fca2a65e 100644 --- a/e2e-install/85-tunnel.spec.ts +++ b/e2e-install/85-tunnel.spec.ts @@ -44,17 +44,24 @@ test.describe.configure({ mode: "serial" }); test.describe("tunnel happy path", () => { test.beforeAll(async () => { // Drop the stub. Pass the script via base64 so newlines survive the - // double-shell hop (`docker exec` → `bash -lc` → `sudo tee`). echo with + // double-shell hop (`docker exec` → `bash -lc` → `tee`). echo with // unescaped \n would otherwise produce a one-line file with literal // backslash-n bytes, leaving cloudflared not actually executable. + // + // Runs as ROOT, not as clawbox-with-sudo. Planting a binary in + // /usr/local/bin is harness setup, not something the product ever does, and + // since TASK-445 the installer quarantines the container's blanket + // `clawbox ALL=(ALL) NOPASSWD: ALL` drop-in — as it does on a real device — + // so `sudo tee` from the clawbox user would (correctly) hit a password + // prompt here. const b64 = Buffer.from(STUB_SCRIPT).toString("base64"); await dockerExec( [ "bash", "-lc", - `echo ${b64} | base64 -d | sudo tee /usr/local/bin/cloudflared > /dev/null && sudo chmod +x /usr/local/bin/cloudflared`, + `echo ${b64} | base64 -d | tee /usr/local/bin/cloudflared > /dev/null && chmod +x /usr/local/bin/cloudflared`, ], - { user: "clawbox", timeoutMs: 15_000 }, + { timeoutMs: 15_000 }, ); }); @@ -65,9 +72,9 @@ test.describe("tunnel happy path", () => { [ "bash", "-lc", - "sudo rm -f /usr/local/bin/cloudflared || true; sudo pkill -f 'cloudflared' || true; sudo pkill -f 'sleep 600' || true", + "rm -f /usr/local/bin/cloudflared || true; pkill -f 'cloudflared' || true; pkill -f 'sleep 600' || true", ], - { user: "clawbox", timeoutMs: 15_000 }, + { timeoutMs: 15_000 }, ).catch(() => {}); }); diff --git a/e2e-install/Dockerfile b/e2e-install/Dockerfile index 45c154594..0c0b6e1b7 100644 --- a/e2e-install/Dockerfile +++ b/e2e-install/Dockerfile @@ -75,10 +75,20 @@ RUN systemctl enable NetworkManager avahi-daemon dbus # Create the clawbox user — install.sh does this too, but pre-creating it lets # us bake volume permissions correctly on first boot. +# Two blanket drop-ins, on purpose. `90-clawbox-nopasswd` is the file the flash +# pipeline bakes into the image, so a freshly flashed device is born with it — +# nothing in this repo writes it, which is why it survives every install.sh run +# and why quarantine_overbroad_sudoers is the only thing that can clear it. +# `clawbox-nopasswd` is the second name seen in the field. The two lines are +# also spelled differently on purpose — `NOPASSWD:ALL` and `NOPASSWD: ALL` — +# because sudoers_grants_blanket_nopasswd parses the rule rather than matching +# text, and both spellings are real. Seeding both proves the detector is +# content-based, not name- or format-based. RUN useradd -m -s /bin/bash -u 1000 clawbox && \ usermod -aG sudo clawbox && \ echo 'clawbox ALL=(ALL) NOPASSWD:ALL' > /etc/sudoers.d/clawbox-nopasswd && \ - chmod 0440 /etc/sudoers.d/clawbox-nopasswd + printf 'clawbox ALL=(ALL) NOPASSWD: ALL\n' > /etc/sudoers.d/90-clawbox-nopasswd && \ + chmod 0440 /etc/sudoers.d/clawbox-nopasswd /etc/sudoers.d/90-clawbox-nopasswd # Pre-create project dir so volume mounts land on the right owner. RUN mkdir -p /home/clawbox/clawbox && chown -R clawbox:clawbox /home/clawbox diff --git a/install.sh b/install.sh index 5051abada..81045b405 100755 --- a/install.sh +++ b/install.sh @@ -95,6 +95,15 @@ if [ -z "${CLAWBOX_INSTALL_BOOTSTRAPPED:-}" ] \ git -C "$_b" -c safe.directory="$_b" fetch origin --quiet 2>/dev/null || true if git -C "$_b" -c safe.directory="$_b" reset --hard "origin/${_br}" --quiet 2>/dev/null; then chown -R clawbox:clawbox "$_b" 2>/dev/null || true + # Re-record what root is allowed to run, BEFORE re-exec'ing into it. The + # reset just replaced install.sh, scripts/ and config/ wholesale, so the + # manifest the root dispatcher checks is now stale by construction — and a + # stale manifest fails every subsequent step of this very update. Paths are + # literal because the constants block has not been parsed yet. + _mf=/usr/local/libexec/clawbox/clawbox-root-manifest.sh + if [ "$_b" = "/home/clawbox/clawbox" ] && [ -x "$_mf" ]; then + "$_mf" --write || echo "[bootstrap] WARN: could not re-record the root-exec manifest" >&2 + fi echo "[bootstrap] Re-executing as $(git -C "$_b" -c safe.directory="$_b" rev-parse --short HEAD)..." exec env CLAWBOX_INSTALL_BOOTSTRAPPED=1 bash "$_b/install.sh" "$@" fi @@ -541,12 +550,45 @@ if ! has_openclaw_harness; then FOREIGN_EDITION_UNITS+=(clawbox-gateway.service) fi -# Load persisted WiFi interface if available +# Read one KEY=VALUE out of a file this script does NOT trust. +# +# Everything under $PROJECT_DIR/data is written by the web server, i.e. by the +# clawbox user — and install.sh runs as root, reached from a NOPASSWD grant. So +# `source`ing anything in there is arbitrary root code execution for anything +# with clawbox-level code execution: the web server, the in-UI terminal, the +# agent's shell. `printf 'x() { :; }; id > /tmp/pwn\n' > data/hostname.env` plus +# the granted `clawbox-root-update@set_hostname.service` was exactly that, and +# data/network.env was worse still because it was sourced on EVERY root run of +# this script, `--step chpasswd` included. +# +# Parse instead: first matching assignment, optional single or double quotes +# stripped, and nothing containing a character that could not have come from the +# writer we expect. The caller still validates the meaning of the value. +# TASK-445. +read_untrusted_env_value() { + local file="$1" key="$2" line value + [ -f "$file" ] || return 0 + [ -L "$file" ] && return 0 + line="$(grep -m1 -E "^[[:space:]]*(export[[:space:]]+)?${key}=" "$file" 2>/dev/null)" || return 0 + value="${line#*=}" + # Strip one layer of matching quotes. + case "$value" in + \"*\") value="${value#\"}"; value="${value%\"}" ;; + \'*\') value="${value#\'}"; value="${value%\'}" ;; + esac + case "$value" in + ""|*[!A-Za-z0-9._-]*) return 0 ;; + esac + printf '%s' "$value" +} + +# Load persisted WiFi interface if available. IFACE_ENV="$PROJECT_DIR/data/network.env" -if [ -f "$IFACE_ENV" ]; then - # shellcheck disable=SC1090 - source "$IFACE_ENV" +_persisted_iface="$(read_untrusted_env_value "$IFACE_ENV" NETWORK_INTERFACE)" +if [ -n "$_persisted_iface" ]; then + NETWORK_INTERFACE="$_persisted_iface" fi +unset _persisted_iface # ── Helpers ────────────────────────────────────────────────────────────────── @@ -1017,11 +1059,13 @@ validate_hostname() { # Falls back to "clawbox". read_configured_hostname() { local hostname_env="$PROJECT_DIR/data/hostname.env" - local name="" - if [ -f "$hostname_env" ]; then - # shellcheck source=/dev/null - name=$(. "$hostname_env" 2>/dev/null; printf '%s' "${HOSTNAME:-}") - fi + # PARSED, never sourced. data/ is clawbox-writable and this function runs as + # root from the granted clawbox-root-update@set_hostname.service, so `.` on + # this file was arbitrary root code execution for anything that can already + # run code as clawbox. validate_hostname below still decides whether the value + # is usable; this only decides that it is a value and not a program. TASK-445. + local name + name="$(read_untrusted_env_value "$hostname_env" HOSTNAME)" if [ -z "$name" ]; then name="clawbox" fi @@ -1478,6 +1522,10 @@ sync_repo_to_update_target() { fi git -c safe.directory="$PROJECT_DIR" -C "$PROJECT_DIR" reset --hard "$upstream_branch" chown -R "$CLAWBOX_USER:$CLAWBOX_USER" "$PROJECT_DIR" + # The tree root is allowed to execute just changed. Re-record it here, in the + # same function that changed it, so no later step of this update runs against + # a manifest describing the previous checkout. + refresh_root_exec_manifest } step_bootstrap_updater() { @@ -2776,11 +2824,32 @@ step_persistent_journal() { # TASK-445. ROOT_LIBEXEC_DIR="/usr/local/libexec/clawbox" +ROOT_EXEC_MANIFEST_HELPER="$ROOT_LIBEXEC_DIR/clawbox-root-manifest.sh" + +# Record the tree root is allowed to execute. Strict: a non-zero return means +# the record is NOT current, and the caller must treat that as a failure. +write_root_exec_manifest() { + [ -x "$ROOT_EXEC_MANIFEST_HELPER" ] || return 1 + "$ROOT_EXEC_MANIFEST_HELPER" --write +} + +# Best-effort variant for the update paths that legitimately change the tree. A +# device that has not installed the helper yet has no manifest to keep in step, +# and warning about that on every sync would be noise; a helper that IS present +# and fails is worth a line, because the next root step refuses until the record +# is current again. +refresh_root_exec_manifest() { + [ -x "$ROOT_EXEC_MANIFEST_HELPER" ] || return 0 + write_root_exec_manifest || echo " Warning: could not re-record the root-exec manifest; root steps will refuse until an operator runs 'sudo bash $PROJECT_DIR/install.sh --step systemd_services'" >&2 +} + install_root_libexec() { install -d -o root -g root -m 0755 /usr/local/libexec install -d -o root -g root -m 0755 "$ROOT_LIBEXEC_DIR" local src - for src in clawbox-root-step.sh; do + # The integrity helper first: the dispatcher installed at the END of this + # function refuses to run any step unless the manifest this writes verifies. + for src in clawbox-root-manifest.sh; do if [ -f "$PROJECT_DIR/config/$src" ]; then install -o root -g root -m 0755 "$PROJECT_DIR/config/$src" "$ROOT_LIBEXEC_DIR/$src" fi @@ -2799,6 +2868,278 @@ install_root_libexec() { install -o root -g root -m 0644 "$PROJECT_DIR/config/clawbox-resource-limits.env" \ /etc/clawbox/resource-limits.env fi + + # Manifest, THEN dispatcher — never the other way round. The dispatcher fails + # closed on a missing or stale manifest, so installing it first would leave a + # window (and, if the manifest write failed, a permanent state) in which every + # root step refuses: no password change, no hostname change, no hotspot + # restart, on an appliance with no console. If the record cannot be written we + # keep whatever dispatcher is already installed and say so — the same rule + # install_sudoers_dropin follows for the allow-list. TASK-445. + if write_root_exec_manifest; then + if [ -f "$PROJECT_DIR/config/clawbox-root-step.sh" ]; then + install -o root -g root -m 0755 "$PROJECT_DIR/config/clawbox-root-step.sh" \ + "$ROOT_LIBEXEC_DIR/clawbox-root-step.sh" + fi + else + echo " Warning: could not record the root-exec manifest; leaving the existing root dispatcher in place" >&2 + record_provision_failure "root_exec_manifest" + fi +} + +# ── sudoers ──────────────────────────────────────────────────────────────── +SUDOERS_DIR="/etc/sudoers.d" +# Copies of drop-ins we removed, kept so a device can be forensically explained +# (and a removal undone by hand) instead of the file simply vanishing. Root-only: +# the clawbox user must not be able to read a rule back out and re-plant it. +SUDOERS_QUARANTINE_DIR="/var/lib/clawbox/sudoers-quarantine" +# Where a candidate drop-in is staged while it is validated. Root-owned and +# NOT under /etc/sudoers.d — see install_sudoers_dropin(). +# +# A subdirectory of its own, not /var/lib/clawbox itself: that directory is +# shared (clawbox-power-mode.sh keeps its clock snapshot there, the first-boot +# VNC marker lives there), and install_sudoers_dropin creates its staging dir +# 0700 root:root. Applying that to the shared parent would stop every non-root +# reader from even traversing it. +SUDOERS_STAGING_DIR="/var/lib/clawbox/sudoers-staging" +# The drop-ins this installer owns. Nothing else in /etc/sudoers.d is ours, and +# quarantine_overbroad_sudoers() below is the only code that touches the rest. +CLAWBOX_SUDOERS_MANAGED=(clawbox clawbox-ollama) + +# Install a sudoers drop-in only if it VALIDATES FIRST. +# +# The old order was cp -> visudo -cf -> rm + exit 1 on failure, which turned a +# typo in the repo into a device with no drop-in at all: every systemctl the web +# server needs (updater, power, wifi hand-off, factory reset, desktop toggle) +# then fails on a password prompt nobody can answer, on an appliance with no +# console. So: validate a staged copy, install only if it parses, and on failure +# leave whatever is already installed exactly where it is and say so. TASK-445. +# +# The staging copy deliberately does NOT live in /etc/sudoers.d — sudo parses +# every file in that directory, so a candidate staged there is live the moment +# it lands, valid or not. +install_sudoers_dropin() { + local src="$1" name="$2" + local dest="$SUDOERS_DIR/$name" + + if [ ! -f "$src" ]; then + echo " Warning: $src is missing; leaving $dest as it is" >&2 + return 1 + fi + + install -d -o root -g root -m 0755 "$SUDOERS_DIR" || return 1 + install -d -o root -g root -m 0700 "$SUDOERS_STAGING_DIR" || return 1 + + local staged + staged="$(mktemp "$SUDOERS_STAGING_DIR/.sudoers-candidate.XXXXXX")" || return 1 + # Checked, not assumed. Both call sites invoke this function in a CONDITION + # context (`if install_sudoers_dropin …`, `… || echo`), and bash disables + # `set -e` for the whole dynamic extent of a command being tested. So every + # step in here has to carry its own `|| return 1`: an unchecked failure does + # not abort the script, it falls through to the next line and reports success. + # A truncated-but-parseable candidate — a `cat` that hit ENOSPC halfway down + # the allow-list — validates under visudo and installs cleanly. TASK-445. + if ! cat "$src" > "$staged"; then + rm -f "$staged" + echo "Error: could not stage $src; keeping the existing $dest" >&2 + return 1 + fi + if ! cmp -s "$src" "$staged"; then + rm -f "$staged" + echo "Error: staged copy of $src is truncated; keeping the existing $dest" >&2 + return 1 + fi + chown root:root "$staged" || { rm -f "$staged"; return 1; } + chmod 0440 "$staged" || { rm -f "$staged"; return 1; } + + if ! visudo -cf "$staged" >/dev/null 2>&1; then + rm -f "$staged" + echo "Error: $src failed visudo validation; keeping the existing $dest" >&2 + return 1 + fi + + # Byte-identical to what is already installed: nothing to do. Keeps repeat + # updates from opening a window where the file is momentarily replaced. + if [ -f "$dest" ] && cmp -s "$staged" "$dest"; then + rm -f "$staged" + return 0 + fi + + local backup="" + if [ -f "$dest" ]; then + backup="$(mktemp "$SUDOERS_STAGING_DIR/.sudoers-previous.XXXXXX")" || { rm -f "$staged"; return 1; } + if ! cat "$dest" > "$backup" || ! cmp -s "$dest" "$backup"; then + rm -f "$staged" "$backup" + echo "Error: could not back up $dest; leaving it as it is" >&2 + return 1 + fi + fi + + # install(1) writes to a temp file and renames, so sudo never sees a + # half-written drop-in. + # + # POSITIVE PROOF, not a return code. The caller uses this function's result to + # decide whether it is safe to quarantine the blanket `NOPASSWD: ALL` drop-in, + # and the `visudo -c` below cannot tell it: when `install` fails, visudo + # happily validates whatever is STILL on disk and answers 0. On a device whose + # only grant is the blanket one, that sequence ends with the narrow file never + # written and the blanket file removed — no working sudo at all, on an + # appliance with no console. So compare the bytes that actually landed. + if ! install -o root -g root -m 0440 "$staged" "$dest" 2>/dev/null || ! cmp -s "$staged" "$dest"; then + if [ -n "$backup" ]; then + # `install` may have left a partial/renamed file behind; put the previous + # content back rather than trusting that it never got that far. + install -o root -g root -m 0440 "$backup" "$dest" 2>/dev/null \ + || echo "Error: could not restore $dest from its backup at $backup" >&2 + else + rm -f "$dest" + fi + rm -f "$staged" "$backup" + echo "Error: could not install $name into $dest; leaving the existing grants alone" >&2 + return 1 + fi + rm -f "$staged" + + # Re-check the WHOLE set: a fragment can be valid on its own and still collide + # with another drop-in (duplicate alias, bad include order). + if ! visudo -c >/dev/null 2>&1; then + if [ -n "$backup" ]; then + # The "rolled back" message used to print whether or not the rollback + # worked. Say what actually happened — a device that is now missing its + # drop-in entirely has to be distinguishable in the install log from one + # that is safely back on its previous rules. + if install -o root -g root -m 0440 "$backup" "$dest" 2>/dev/null; then + echo "Error: installing $name broke /etc/sudoers validation; rolled $dest back" >&2 + else + rm -f "$dest" + echo "Error: installing $name broke /etc/sudoers validation AND the rollback failed; removed $dest" >&2 + fi + else + rm -f "$dest" + echo "Error: installing $name broke /etc/sudoers validation; removed $dest" >&2 + fi + rm -f "$backup" + return 1 + fi + + rm -f "$backup" + return 0 +} + +# Does this drop-in hand the clawbox service user unrestricted passwordless root? +# +# Deliberately narrow. Only a rule whose user spec is `clawbox` or `%clawbox` +# AND whose Cmnd is a bare `ALL` under an active NOPASSWD tag counts. An +# operator's own `%sudo`/`%admin` rule, and the distro default in /etc/sudoers, +# are never inspected and never touched: removing those could lock the only +# administrator out of a device that is 3000 km away. +# +# ACCEPTED RESIDUAL, recorded so the next reader does not mistake it for an +# oversight. Three shapes are knowingly out of scope, all for the same reason — +# each would mean this installer silently rewriting rules a human wrote: +# +# 1. A blanket line inside /etc/sudoers itself. Only /etc/sudoers.d is walked. +# e2e-install/06-sudoers.spec.ts catches this behaviourally instead: it runs +# `sudo -n` probes for commands no grant names and requires DENIED. +# 2. A grant that reaches clawbox through a User_Alias rather than by name. +# 3. Over-broad but not blanket — e.g. `clawbox ALL=(ALL) NOPASSWD: /bin/bash`, +# which is root in one move but is not a bare `ALL`. +# +# Widening the detector to any of these means an installer that can delete an +# operator's deliberate rule on an appliance with no console; the behavioural +# probes in CI are the compensating control. TASK-445. +sudoers_grants_blanket_nopasswd() { + local file="$1" + [ -f "$file" ] || return 1 + awk ' + function check(l, eq, rest, n, parts, i, item, tag, nopass) { + if (l !~ /^[ \t]*(clawbox|%clawbox)[ \t]/) return 0 + eq = index(l, "=") + if (eq == 0) return 0 + rest = substr(l, eq + 1) + nopass = 0 + n = split(rest, parts, ",") + for (i = 1; i <= n; i++) { + item = parts[i] + gsub(/^[ \t]+|[ \t]+$/, "", item) + sub(/^\([^)]*\)[ \t]*/, "", item) + while (match(item, /^(NOPASSWD|PASSWD|NOEXEC|EXEC|SETENV|NOSETENV|LOG_INPUT|NOLOG_INPUT|LOG_OUTPUT|NOLOG_OUTPUT|MAIL|NOMAIL|FOLLOW|NOFOLLOW|INTERCEPT|NOINTERCEPT):[ \t]*/)) { + tag = substr(item, 1, RLENGTH) + if (tag ~ /^NOPASSWD:/) nopass = 1 + else if (tag ~ /^PASSWD:/) nopass = 0 + item = substr(item, RLENGTH + 1) + gsub(/^[ \t]+|[ \t]+$/, "", item) + } + if (nopass && item == "ALL") return 1 + } + return 0 + } + { + line = $0 + sub(/#.*$/, "", line) + if (line ~ /\\[ \t]*$/) { sub(/\\[ \t]*$/, "", line); pending = pending line; next } + line = pending line + pending = "" + if (check(line)) { found = 1; exit } + } + END { exit(found ? 0 : 1) } + ' "$file" +} + +# Move any /etc/sudoers.d drop-in that hands clawbox unrestricted passwordless +# root out of sudo's way. +# +# Why the installer has to do this rather than just shipping a narrow file: sudo +# takes the UNION of every drop-in. QA and factory provisioning left +# `/etc/sudoers.d/90-clawbox-nopasswd` containing `clawbox ALL=(ALL) NOPASSWD: ALL` +# on shipped devices, and while that file exists every narrowing in +# config/clawbox-sudoers is decorative — the revalidation of TASK-445 measured +# exactly that on the QA box. Narrowing what we ship without removing what is +# already there changes nothing on a device that has both. TASK-445 round 2. +quarantine_overbroad_sudoers() { + [ -d "$SUDOERS_DIR" ] || return 0 + + local f base m managed + local -a moved_from=() moved_to=() + for f in "$SUDOERS_DIR"/*; do + [ -f "$f" ] || continue + base="$(basename "$f")" + managed=0 + for m in "${CLAWBOX_SUDOERS_MANAGED[@]}"; do + [ "$base" = "$m" ] && managed=1 && break + done + [ "$managed" = "1" ] && continue + sudoers_grants_blanket_nopasswd "$f" || continue + + install -d -o root -g root -m 0700 "$SUDOERS_QUARANTINE_DIR" + local stamp dest + stamp="$(date -u +%Y%m%dT%H%M%SZ)" + dest="$SUDOERS_QUARANTINE_DIR/$base.$stamp" + if mv "$f" "$dest" 2>/dev/null; then + chown root:root "$dest" + chmod 0400 "$dest" + moved_from+=("$f") + moved_to+=("$dest") + echo " Removed over-broad sudoers drop-in $base (clawbox had passwordless root on everything); copy kept at $dest" + else + echo " Warning: could not remove over-broad sudoers drop-in $base" >&2 + fi + done + + [ "${#moved_to[@]}" -eq 0 ] && return 0 + + # Removing a file can still break the set — a quarantined drop-in may have + # defined an alias another one uses. Put everything back rather than leave a + # device where sudo refuses every command. + if ! visudo -c >/dev/null 2>&1; then + local i + for i in "${!moved_to[@]}"; do + mv "${moved_to[$i]}" "${moved_from[$i]}" 2>/dev/null || true + done + echo "Error: removing the over-broad sudoers drop-in(s) broke /etc/sudoers validation; restored them" >&2 + return 1 + fi + return 0 } step_systemd_services() { @@ -2897,18 +3238,50 @@ step_systemd_services() { # Root-owned copies of everything root executes on clawbox's behalf. Must run # BEFORE the sudoers drop-in, which points at them. install_root_libexec - # Install sudoers rules so the clawbox user can manage services (systemctl restart, reboot, etc.) - if [ -f "$PROJECT_DIR/config/clawbox-sudoers" ]; then - cp "$PROJECT_DIR/config/clawbox-sudoers" /etc/sudoers.d/clawbox - chmod 0440 /etc/sudoers.d/clawbox - chown root:root /etc/sudoers.d/clawbox - if ! visudo -cf /etc/sudoers.d/clawbox >/dev/null; then - rm -f /etc/sudoers.d/clawbox - echo "Error: sudoers drop-in failed visudo validation; removed to keep sudo functional" >&2 - exit 1 - fi + # Install the narrow allow-list FIRST, then remove any blanket grant. In that + # order the device is never, even briefly, without the rules the web server + # needs: if the drop-in fails to validate we keep the old one and skip the + # quarantine entirely rather than strand the box with neither. + # The ollama optimiser grant is a SECOND drop-in and it belongs here, next to + # the first one — not in step_performance_mode where it used to live. That + # step returns early under CLAWBOX_TEST_MODE and is Jetson-only in spirit, so + # the grant silently never landed on any box that took the early return: the + # e2e-install container installed cleanly and still had no + # `optimize-ollama.sh` grant, which is the same "the narrowing is invisible on + # the device" shape TASK-445 exists to close. step_systemd_services is the one + # step both a fresh install and the in-app updater (step_post_update) always + # run, unconditionally. TASK-445. + # + # Called plainly, never as the tested command of an `if`: bash suspends + # `set -e` for the entire dynamic extent of a command run in a condition + # context, so that spelling disarmed every unchecked command inside the + # function body too. The function now checks its own steps, and the status + # comes back through an explicit variable. + local sudoers_status=0 + set +e + install_sudoers_dropin "$PROJECT_DIR/config/clawbox-sudoers" clawbox + sudoers_status=$? + set -e + + # Two independent gates before the blanket grant is removed: the installer + # reported success, AND the bytes on the device are the allow-list we shipped. + # The second one is the load-bearing half — it is proof about the device, not + # about a code path, and it is what makes "installed the narrow rules" a + # precondition of "removed the wide ones" instead of an assumption. + if [ "$sudoers_status" -eq 0 ] \ + && cmp -s "$PROJECT_DIR/config/clawbox-sudoers" "$SUDOERS_DIR/clawbox"; then echo " Sudoers rules installed" + # Gated on the PRIMARY allow-list only. That file is what keeps the box + # operable (wizard, updater, power, hotspot); the ollama grant is one + # feature's tuning. Letting a missing feature grant block the quarantine + # would leave a device on blanket passwordless root to protect a KV-cache + # setting — the wrong trade in the wrong direction. + quarantine_overbroad_sudoers || true + else + echo " Warning: sudoers rules NOT updated; leaving the existing grants alone" >&2 fi + install_sudoers_dropin "$PROJECT_DIR/config/sudoers-clawbox-ollama" clawbox-ollama || \ + echo " Warning: clawbox-ollama sudoers rules NOT updated; leaving the existing grant alone" >&2 echo " Services installed and enabled" } @@ -3309,15 +3682,18 @@ step_performance_mode() { fi # snapd is kept running — required for snap-based Chromium on Ubuntu 22.04 # Optimize Ollama for 8GB Jetson - bash "$PROJECT_DIR/scripts/optimize-ollama.sh" - cp "$PROJECT_DIR/config/sudoers-clawbox-ollama" /etc/sudoers.d/clawbox-ollama - chmod 440 /etc/sudoers.d/clawbox-ollama - chown root:root /etc/sudoers.d/clawbox-ollama - if ! visudo -cf /etc/sudoers.d/clawbox-ollama >/dev/null; then - rm -f /etc/sudoers.d/clawbox-ollama - echo "Error: clawbox-ollama sudoers drop-in failed visudo validation; removed" >&2 - exit 1 - fi + # Run the ROOT-OWNED copy, not the one in the clawbox-writable project tree: + # it is the copy the sudoers grant points at, so running it here is also the + # check that install_root_libexec actually put it there. A device whose + # /usr/local/libexec/clawbox/optimize-ollama.sh is missing is a device where + # saving a local Ollama model silently skips the q8_0 KV-cache / flash-attention + # tuning, which is exactly what the TASK-445 revalidation found. TASK-445. + # + # The grant that names this path is installed by step_systemd_services, not + # here: everything below this point is behind the is_test_mode early return + # above, so installing a sudoers drop-in here meant it never landed on a box + # that took that return. TASK-445. + "$ROOT_LIBEXEC_DIR/optimize-ollama.sh" # The cgroup memory guards. Deliberately AFTER the ollama optimiser, so the # unit it just restarted picks the limits up on the daemon-reload below. step_resource_limits @@ -3384,8 +3760,12 @@ step_ollama_install() { # Ensure the service is enabled and running systemctl enable ollama 2>/dev/null || true systemctl start ollama 2>/dev/null || true - # Apply Jetson memory optimizations - bash "$PROJECT_DIR/scripts/optimize-ollama.sh" + # Apply Jetson memory optimizations. Root-owned copy again, same reason as in + # step_performance_mode: this runs as root, and /home/clawbox/clawbox/scripts + # is clawbox-writable, so sourcing the repo copy here would be a root path + # through a file the web server can rewrite. TASK-445. + install_root_libexec + "$ROOT_LIBEXEC_DIR/optimize-ollama.sh" echo " Ollama installed and running" # Local embedding model for semantic memory. OpenClaw's memory search @@ -3633,14 +4013,78 @@ step_llamacpp_install() { echo " llama.cpp runtime ready" } +# Set the appliance owner's system password. +# +# The record arrives in a file the web server wrote, and the web server runs as +# the clawbox user — so $PROJECT_DIR/data is clawbox-writable and this input is +# attacker-choosable by anything with clawbox-level code execution. Until +# TASK-445 every guard on the record lived on the UNPRIVILEGED side, in +# src/lib/chpasswd.ts; the root side piped whatever it found straight into +# chpasswd. Dropping `root:` into that path and starting the granted unit +# therefore set ROOT's password. +# +# So validate here, where the boundary actually is. chpasswd's format is +# `:` per line and it happily takes a list, so all three of +# "which user", "how many records" and "what may the record contain" have to be +# pinned: +# +# * exactly one record, so a second line cannot smuggle in another account; +# * the user field is exactly $CLAWBOX_USER — never root, never anything else; +# * a non-empty password with no CR or NUL, matching the checks the route +# already makes (src/lib/chpasswd.ts::chpasswdRecord). +# +# Residual, recorded deliberately: clawbox is in the `sudo` group, so being able +# to set the CLAWBOX user's own password is still a route from clawbox code +# execution to an interactive root shell. That is the owner's own administrator +# account and removing it would lock the only administrator out of a console-less +# appliance (see config/clawbox-sudoers and e2e-install/06-sudoers.spec.ts). What +# this closes is the part that was never intended: changing a DIFFERENT account's +# password, root's included. step_chpasswd() { local INPUT_FILE="$PROJECT_DIR/data/.chpasswd-input" + # -f follows symlinks; -L rejects the link itself. A symlink here would be a + # way to make root read a file the clawbox user could not otherwise feed in. + if [ -L "$INPUT_FILE" ]; then + rm -f "$INPUT_FILE" + echo "Error: password input file is a symlink; refusing" >&2 + exit 64 + fi if [ ! -f "$INPUT_FILE" ]; then echo "Error: password input file not found" >&2 exit 1 fi - /usr/sbin/chpasswd < "$INPUT_FILE" + + # Read the file ONCE and validate the value actually used — re-reading it + # after the checks would leave a window to swap the contents. Command + # substitution strips trailing newlines, so a well-formed single record has no + # embedded newline left and a second record is visible as one. It also drops + # NUL bytes, and it is the stripped value that is piped to chpasswd below, so + # no NUL can reach it either. + local record user + record="$(cat "$INPUT_FILE")" rm -f "$INPUT_FILE" + + case "$record" in + *$'\n'*) + echo "Error: password input must be exactly one record" >&2 + exit 64 + ;; + *$'\r'*) + echo "Error: password input contains a carriage return" >&2 + exit 64 + ;; + esac + user="${record%%:*}" + if [ "$user" != "$CLAWBOX_USER" ]; then + echo "Error: password input names '$user'; only $CLAWBOX_USER may be changed here" >&2 + exit 64 + fi + if [ "$record" = "$user" ] || [ -z "${record#*:}" ]; then + echo "Error: password input has no password" >&2 + exit 64 + fi + + printf '%s\n' "$record" | /usr/sbin/chpasswd } step_rebuild() { @@ -4191,11 +4635,13 @@ step_validate_services() { # probe failures. # step_network_setup persists NETWORK_INTERFACE to network.env but doesn't - # export it, so on a fresh install our process still has it unset. Reload - # the file before probing. - if [ -f "$IFACE_ENV" ]; then - # shellcheck disable=SC1090 - source "$IFACE_ENV" + # export it, so on a fresh install our process still has it unset. Reload the + # value before probing — PARSED from the clawbox-writable copy, sourced only + # from the root-owned one. See read_untrusted_env_value. TASK-445. + local _iface + _iface="$(read_untrusted_env_value "$IFACE_ENV" NETWORK_INTERFACE)" + if [ -n "$_iface" ]; then + NETWORK_INTERFACE="$_iface" elif [ -f /etc/clawbox/network.env ]; then # shellcheck disable=SC1091 source /etc/clawbox/network.env @@ -4578,10 +5024,13 @@ step_validate_services || VALIDATE_RC=$? # ── Done ───────────────────────────────────────────────────────────────────── -# Re-read persisted interface for summary -if [ -f "$IFACE_ENV" ]; then - source "$IFACE_ENV" +# Re-read the persisted interface for the summary. Parsed, not sourced — this +# file is clawbox-writable and we are root. TASK-445. +_summary_iface="$(read_untrusted_env_value "$IFACE_ENV" NETWORK_INTERFACE)" +if [ -n "$_summary_iface" ]; then + NETWORK_INTERFACE="$_summary_iface" fi +unset _summary_iface echo "" echo "=== ClawBox Setup Complete ===" diff --git a/package.json b/package.json index ce3913a8a..2c310e70b 100644 --- a/package.json +++ b/package.json @@ -23,7 +23,8 @@ "test:e2e": "playwright test", "test:e2e:coverage": "rm -f coverage/e2e-summary.json coverage/e2e-bundles.json && playwright test && node scripts/e2e-coverage-report.mjs", "test:e2e:ui": "playwright test --ui", - "verify:build-identity": "bash scripts/verify-build-identity.sh" + "verify:build-identity": "bash scripts/verify-build-identity.sh", + "check:sudoers": "bash scripts/check-sudoers-coverage.sh" }, "dependencies": { "@modelcontextprotocol/sdk": "^1.29.0", diff --git a/scripts/ap-watchdog.sh b/scripts/ap-watchdog.sh index 24a4cf1ef..21f960bac 100755 --- a/scripts/ap-watchdog.sh +++ b/scripts/ap-watchdog.sh @@ -52,15 +52,37 @@ fi # back up. It then stayed up until the next reboot, when setup_complete was # finally true and start-ap.sh skipped correctly. # -# Sourced rather than grepped so a quoted SSID with a `#` in it cannot be -# misread, and in a subshell so a malformed file cannot take the watchdog's own -# variables with it. Missing file, unset flag, or anything other than 1 all mean -# "not disabled" — the failure direction that keeps a box reachable. -if [ -f "$HOTSPOT_ENV" ]; then - hotspot_disabled="$( ( set +u; . "$HOTSPOT_ENV" >/dev/null 2>&1; printf '%s' "${HOTSPOT_DISABLED:-}" ) 2>/dev/null )" - if [ "$hotspot_disabled" = "1" ]; then - exit 0 - fi +# PARSED, never sourced. +# +# This used to be `. "$HOTSPOT_ENV"` in a subshell — the subshell protected this +# script's variables and nothing else. clawbox-ap-watchdog.service carries no +# `User=`, so this runs as ROOT on a timer, while $ROOT/data is written by the +# web server as the clawbox user. Sourcing it was therefore arbitrary root code +# execution on a schedule for anything that could already run code as clawbox: +# plant the payload, wait twenty seconds. TASK-445. +# +# The parse keeps the property the sourcing was chosen for — a quoted SSID with +# a `#` in it cannot be misread, because only the named key's own line is read +# and one layer of quotes is stripped. Missing file, unset flag, or anything +# other than 1 all still mean "not disabled": the failure direction that keeps a +# box reachable. +read_env_value() { + local file="$1" key="$2" line value + [ -f "$file" ] || return 0 + [ -L "$file" ] && return 0 + line="$(grep -m1 -E "^[[:space:]]*(export[[:space:]]+)?${key}=" "$file" 2>/dev/null)" || return 0 + value="${line#*=}" + value="${value%$'\r'}" + case "$value" in + \"*\") value="${value#\"}"; value="${value%\"}" ;; + \'*\') value="${value#\'}"; value="${value%\'}" ;; + esac + printf '%s' "$value" +} + +hotspot_disabled="$(read_env_value "$HOTSPOT_ENV" HOTSPOT_DISABLED)" +if [ "$hotspot_disabled" = "1" ]; then + exit 0 fi # A deliberate client-connect owns the radio right now — leave it alone so we diff --git a/scripts/check-sudoers-coverage.sh b/scripts/check-sudoers-coverage.sh new file mode 100755 index 000000000..b52972dc9 --- /dev/null +++ b/scripts/check-sudoers-coverage.sh @@ -0,0 +1,594 @@ +#!/usr/bin/env bash +# +# Assert that every root command ClawBox runs through sudo is covered by the +# NOPASSWD allow-list we ship, and that the allow-list grants nothing we do not +# actually run. +# +# Why this exists (TASK-445): the drop-in on a provisioned device used to be a +# blanket `clawbox ALL=(ALL) NOPASSWD: ALL`. Narrowing it to an explicit list +# only STAYS narrow if something fails the build when a new `sudo` call shows up +# without a matching grant — otherwise the next developer hits a silent password +# prompt on an appliance with no console, "fixes" it by widening the list, and +# we are back where we started. +# +# The check is FAIL-CLOSED in both directions: +# +# * A sudo call site whose argv this script cannot resolve to concrete +# arguments is an ERROR. Write the call with literal arguments, declare its +# expansion in DECLARED_ARGV, or exempt it in EXEMPT_CALLS with a reason. +# * A grant nothing invokes is an ERROR, unless it is the `.service`/bare-unit +# twin of a grant that IS invoked (see the header of config/clawbox-sudoers +# for why both spellings are shipped) or is acknowledged in +# ACKNOWLEDGED_UNUSED with a reason. +# +# It also enforces two SHAPE invariants on the allow-list itself, because +# coverage alone does not make a grant safe (TASK-445 audit, GAP 2 and GAP 3): +# +# * NO WILDCARDS. sudoers(5) matches a command's arguments as one concatenated +# string, so `*` and `?` span whitespace: `start --no-block clawbox-*` also +# matched `start --no-block clawbox-setup.service ssh.service`, and +# `systemctl start` takes a LIST of units. Every Cmnd_Spec must be literal, +# path and arguments alike. +# * ROOT-OWNED TARGETS ONLY. The command a grant names has to live somewhere +# the clawbox user cannot write, or the grant hands root a file the web +# server itself can rewrite. Anything outside the root-owned prefixes is +# rejected — see ROOT_OWNED_PREFIXES below. +# +# Repo convention this relies on: a real sudo INVOCATION from TypeScript either +# spawns the literal "sudo"/"/usr/bin/sudo" as argv[0], or writes the absolute +# "/usr/bin/sudo" inside a generated shell script. A bare `sudo` inside a +# user-facing message ("Run: sudo apt install …") is prose, not an invocation, +# and is deliberately not matched. +# +# Usage: +# bash scripts/check-sudoers-coverage.sh # check, exit 1 on gaps +# bash scripts/check-sudoers-coverage.sh --list # dump grants + call sites +# bash scripts/check-sudoers-coverage.sh --json # machine-readable report + +set -euo pipefail + +SCRIPT_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)" +REPO_ROOT="${CLAWBOX_REPO_ROOT:-$(cd -- "$SCRIPT_DIR/.." && pwd)}" + +MODE="check" +case "${1:-}" in + --list) MODE="list" ;; + --json) MODE="json" ;; + "") ;; + *) + echo "usage: $0 [--list|--json]" >&2 + exit 2 + ;; +esac + +if ! command -v perl >/dev/null 2>&1; then + echo "check-sudoers-coverage: perl is required" >&2 + exit 2 +fi + +perl - "$REPO_ROOT" "$MODE" <<'PERL_EOF' +use strict; +use warnings; + +my ($root, $mode) = @ARGV; + +# Exit 1, never perl's errno-flavoured default: a malformed allow-list is the +# same kind of build failure as an uncovered call, and callers key on the code. +sub fatal { print STDERR "check-sudoers-coverage: $_[0]"; exit 1 } + +# ── The sudoers drop-ins we ship ──────────────────────────────────────────── +my @SUDOERS_FILES = ( + 'config/clawbox-sudoers', + 'config/sudoers-clawbox-ollama', +); + +# ── Where a granted command may live ──────────────────────────────────────── +# Directories no unprivileged account on the appliance can write to. +# install.sh::install_root_libexec creates /usr/local/libexec/clawbox root:root +# 0755 under a root:root parent and copies every helper it grants into it, +# precisely so that no grant names a file in /home/clawbox/clawbox — a tree +# install.sh itself hands back to the clawbox user with `chown -R` on every root +# run. +my @ROOT_OWNED_PREFIXES = ( + '/bin/', '/sbin/', '/usr/bin/', '/usr/sbin/', '/usr/local/libexec/clawbox/', +); + +# Reject a grant that cannot be safe regardless of who invokes it. +sub check_grant_shape { + my ($rel, $lineno, $cmd) = @_; + + if ($cmd =~ /[*?]/) { + fatal("$rel:$lineno uses a wildcard:\n $cmd\n" + . " sudoers matches arguments as ONE concatenated string, so `*` and `?` span\n" + . " whitespace and swallow extra arguments: a rule ending in `*` also matches\n" + . " ` `. Enumerate the exact commands instead.\n"); + } + + my ($path) = split /\s+/, $cmd; + fatal("$rel:$lineno grants the relative command `$path`. sudo resolves that through\n" + . " secure_path, which is a convenience, not a privilege boundary. Use an absolute path.\n") + unless $path =~ m{^/}; + + # sudo matches the command PATH as a string and does not canonicalise it, so + # `/usr/bin/../home/clawbox/clawbox/payload` would sail past the prefix test + # below while naming a file in the clawbox-writable tree. Only canonical paths + # can be reasoned about here. + fatal("$rel:$lineno grants `$path`, which contains a `.` or `..` component.\n" + . " sudo compares the command path as a string and never canonicalises it, so a\n" + . " traversal like /usr/bin/../home/clawbox/... would pass the root-owned prefix\n" + . " check below while naming a file clawbox can write. Use the canonical path.\n") + if grep { $_ eq '.' || $_ eq '..' } split m{/}, $path; + + return if grep { index($path, $_) == 0 } @ROOT_OWNED_PREFIXES; + fatal("$rel:$lineno grants `$path`, which is outside every root-owned prefix\n" + . " (" . join(', ', @ROOT_OWNED_PREFIXES) . ").\n" + . " A NOPASSWD grant on a file the clawbox user can write IS passwordless local root:\n" + . " the web server, the in-UI terminal and the agent's shell all run as clawbox.\n" + . " Install a root-owned copy under /usr/local/libexec/clawbox and grant that instead.\n"); +} + +# ── Where a root command may be invoked from ──────────────────────────────── +# install.sh, config/clawbox-root-step.sh and e2e-install/ are deliberately +# absent: the first two only ever run AS root (`sudo bash install.sh`, or a +# systemd unit with no User=), and the third is a container harness, not +# product code. None of them crosses the clawbox -> root boundary this guards. +my @SCAN_DIRS = ('src', 'mcp', 'scripts'); +my @SCAN_SKIP = ('src/tests/', 'node_modules/', '.next/'); +my %SCAN_SKIP_FILE = ('scripts/check-sudoers-coverage.sh' => 1); + +# ── Call sites whose argv is not a literal array ──────────────────────────── +# Key = " :: " +# Value = the concrete argument lists that call site can produce. +# +# Keying on the source text rather than a line number means unrelated edits +# above the call do not invalidate the declaration, but editing the CALL does: +# the key stops matching, the site becomes unresolved, and this check fails. +# That is the point — a new dynamic argument gets re-reviewed as a privilege +# boundary instead of inheriting someone else's review. +my %DECLARED_ARGV = ( + # src/lib/system-profile.ts — runScript() builds cmd = useSudo ? "sudo" : + # script and argv = [script, ...args]. The two scripts do NOT share modes, so + # this is enumerated per script rather than as a cartesian product; --check is + # absent because the status path runs it without sudo. + 'src/lib/system-profile.ts :: cmd, argv' => [ + ['sudo', '/usr/local/libexec/clawbox/clawbox-desktop-mode.sh', '--enable'], + ['sudo', '/usr/local/libexec/clawbox/clawbox-desktop-mode.sh', '--disable'], + ['sudo', '/usr/local/libexec/clawbox/clawbox-power-mode.sh', '--balanced'], + ['sudo', '/usr/local/libexec/clawbox/clawbox-power-mode.sh', '--performance'], + ], + # src/lib/local-models.ts — verb is enable|disable; unit is constrained to + # SYSTEM_UNITS by the `allowed.has(unit)` guard immediately above the call. + 'src/lib/local-models.ts :: "/usr/bin/systemctl", verb, "--now", unit' => [ + ['/usr/bin/systemctl', 'enable', '--now', 'ollama.service'], + ['/usr/bin/systemctl', 'disable', '--now', 'ollama.service'], + ], + # src/lib/local-ai-runtime.ts — systemctlOllama() is private to the module and + # every caller passes one of the three module-level const argv arrays declared + # right above it, each already a literal list. Enumerated here rather than + # resolved because the call spreads them (`["-n", ...argv]`). + 'src/lib/local-ai-runtime.ts :: "-n", ...argv' => [ + ['-n', '/usr/bin/systemctl', 'enable', '--now', 'ollama.service'], + ['-n', '/usr/bin/systemctl', 'start', 'ollama.service'], + ['-n', '/usr/bin/systemctl', 'stop', 'ollama.service'], + ], + # src/app/setup-api/system/power/route.ts — POWER_ACTIONS maps the request + # body to exactly these two; an unmapped action 400s before the call. + 'src/app/setup-api/system/power/route.ts :: "/usr/bin/systemctl", systemctlAction' => [ + ['/usr/bin/systemctl', 'poweroff'], + ['/usr/bin/systemctl', 'reboot'], + ], + # src/app/setup-api/clawkeep/restore/route.ts — svc iterates RESTART_SERVICES. + 'src/app/setup-api/clawkeep/restore/route.ts :: "/usr/bin/systemctl", "restart", svc' => [ + ['/usr/bin/systemctl', 'restart', 'clawbox-gateway.service'], + ], +); + +# ── Sudo calls that are deliberately NOT in the allow-list ────────────────── +# Operator-driven paths where a password prompt is the correct behaviour, or +# grants we refuse to write because the target is clawbox-writable. +my %EXEMPT_CALLS = ( + 'mcp/clawbox-cli.ts :: "bash", installScript' => + '`clawbox update` runs `sudo bash install.sh` from a human terminal. install.sh ' + . 'lives in the clawbox-writable project tree, so a NOPASSWD grant here would be ' + . 'the exact defect TASK-445 closed. The password prompt is the boundary.', + 'src/lib/hermes-cli.ts :: bin, argv' => + 'runHermesCli({sudo:true}) execs HERMES_BIN under /home/clawbox/.local/bin, which ' + . 'the clawbox user owns and can rewrite. Deliberately ungranted: `sudo -n` fails ' + . 'closed in milliseconds rather than blocking a route handler on a prompt, and the ' + . 'alternative is passwordless root on a clawbox-writable file. Exactly ONE caller ' + . 'is left — the first-time `gateway install --system`, which ' + . 'writes a unit into /etc/systemd/system and has no safe Cmnd spelling. It is ' + . 'genuinely install-time-only, and its failure is now REPORTED (the `applied` flag) ' + . 'rather than swallowed. The RESTART branch used to be exempted under this same ' + . 'entry on the claim that it was install-time-only too; it was not — every ' + . 'Telegram/WhatsApp/Discord/Email config save hits it — so it moved to ' + . '`systemctl restart hermes-gateway.service`, which is granted.', + 'scripts/force-update.sh :: sudo -u %STR% bash -c %STR%' => + 'Operator recovery script, run by hand from the Terminal app or over SSH. Dropping ' + . 'to the clawbox user is interactive by design — the owner types the password.', + 'scripts/force-update.sh :: sudo chown -R %STR% %STR%' => + 'Same script. A NOPASSWD chown grant would hand the web server ownership of the git ' + . 'tree it is updated from, which is a root escalation in one move.', +); + +# ── Grants nothing invokes, kept on purpose ───────────────────────────────── +my %ACKNOWLEDGED_UNUSED = ( + # 'the exact Cmnd string' => 'the operator path that still needs it', +); + +# argv[0] is resolved through sudo's secure_path when it is not absolute. +my %BIN_PATH = ( + 'systemctl' => '/usr/bin/systemctl', + 'apt-get' => '/usr/bin/apt-get', + 'dpkg' => '/usr/bin/dpkg', + 'snap' => '/usr/bin/snap', + 'nmcli' => '/usr/bin/nmcli', +); + +# ── Load the allow-list ───────────────────────────────────────────────────── +my @grants; +for my $rel (@SUDOERS_FILES) { + my $path = "$root/$rel"; + open(my $fh, '<', $path) or fatal("cannot read $rel: $!\n"); + my $lineno = 0; + my $pending = ''; + while (my $line = <$fh>) { + $lineno++; + chomp $line; + $line =~ s/^\s*#.*$//; + next if $line =~ /^\s*$/ && $pending eq ''; + if ($line =~ s/\\\s*$//) { $pending .= $line; next; } + my $full = $pending . $line; + $pending = ''; + next if $full =~ /^\s*$/; + if ($full =~ /^\s*clawbox\s+ALL\s*=\s*\(([^)]*)\)\s*NOPASSWD:\s*(.+?)\s*$/) { + my ($runas, $cmd) = ($1, $2); + $cmd =~ s/\s+/ /g; + fatal("$rel:$lineno grants runas `$runas`; only (root) is allowed\n") + unless $runas eq 'root'; + fatal("$rel:$lineno grants a bare ALL — that is the blanket rule this whole " + . "task removed\n") if $cmd eq 'ALL'; + check_grant_shape($rel, $lineno, $cmd); + push @grants, { file => $rel, line => $lineno, cmd => $cmd, used => 0 }; + next; + } + fatal("$rel:$lineno is not a `clawbox ALL=(root) NOPASSWD: ` rule:\n $full\n"); + } + close $fh; +} +fatal("no grants parsed\n") unless @grants; + +# ── Collect the files to scan ─────────────────────────────────────────────── +my @files; +sub walk { + my ($dir) = @_; + opendir(my $dh, "$root/$dir") or return; + my @entries = sort grep { $_ ne '.' && $_ ne '..' } readdir($dh); + closedir $dh; + for my $e (@entries) { + next if $e eq 'node_modules' || $e eq '.next'; + my $rel = "$dir/$e"; + if (-d "$root/$rel") { walk($rel); next; } + next unless $rel =~ /\.(ts|tsx|js|mjs|sh)$/; + next if $SCAN_SKIP_FILE{$rel}; + next if grep { index($rel, $_) == 0 } @SCAN_SKIP; + push @files, $rel; + } +} +walk($_) for @SCAN_DIRS; + +# ── Resolve string constants ──────────────────────────────────────────────── +my (%global_const, %global_conflict); +for my $rel (@files) { + next unless $rel =~ /\.(ts|tsx)$/; + open(my $fh, '<', "$root/$rel") or next; + local $/; + my $src = <$fh>; + close $fh; + while ($src =~ /^\s*export\s+const\s+([A-Za-z_][A-Za-z0-9_]*)\s*(?::[^=\n]+)?=\s*"([^"\\]*)"\s*;/mg) { + my ($name, $val) = ($1, $2); + $global_conflict{$name} = 1 if exists $global_const{$name} && $global_const{$name} ne $val; + $global_const{$name} = $val; + } +} + +sub local_consts { + my ($src) = @_; + my %c; + while ($src =~ /^\s*(?:export\s+)?const\s+([A-Za-z_][A-Za-z0-9_]*)\s*(?::[^=\n]+)?=\s*"([^"\\]*)"\s*;/mg) { + $c{$1} = $2; + } + return \%c; +} + +my (@calls, @unresolved); + +sub add_unresolved { + my ($rel, $raw, $why) = @_; + my $key = "$rel :: $raw"; + return if exists $EXEMPT_CALLS{$key}; + push @unresolved, { file => $rel, raw => $raw, why => $why, key => $key }; +} + +sub collapse { + my ($s) = @_; + $s =~ s{//[^\n]*}{}g; + $s =~ s/\s+/ /g; + $s =~ s/^\s+|\s+$//g; + $s =~ s/,\s*$//; + return $s; +} + +sub split_argv_items { + my ($text) = @_; + my (@out, $cur, $depth, $quote); + $cur = ''; $depth = 0; $quote = ''; + for my $ch (split //, $text) { + if ($quote ne '') { $cur .= $ch; $quote = '' if $ch eq $quote; next; } + if ($ch eq '"' || $ch eq "'" || $ch eq '`') { $quote = $ch; $cur .= $ch; next; } + if ($ch =~ /[\(\[\{]/) { $depth++; $cur .= $ch; next; } + if ($ch =~ /[\)\]\}]/) { $depth--; $cur .= $ch; next; } + if ($ch eq ',' && $depth == 0) { push @out, $cur; $cur = ''; next; } + $cur .= $ch; + } + push @out, $cur if $cur =~ /\S/; + return map { my $s = $_; $s =~ s{//[^\n]*}{}g; $s =~ s/^\s+|\s+$//g; $s } @out; +} + +sub resolve_items { + my ($rel, $consts, $items) = @_; + my @argv; + for my $item (@$items) { + next if $item eq ''; + if ($item =~ /^"([^"\\]*)"$/ || $item =~ /^'([^'\\]*)'$/ || $item =~ /^`([^`\\\$]*)`$/) { + push @argv, $1; + } elsif ($item =~ /^([A-Za-z_][A-Za-z0-9_]*)$/) { + my $name = $1; + if (exists $consts->{$name}) { push @argv, $consts->{$name} } + elsif (exists $global_const{$name} && !$global_conflict{$name}) { push @argv, $global_const{$name} } + else { return (undef, "identifier `$name` does not resolve to a string constant") } + } else { + return (undef, "argument `$item` is not a literal"); + } + } + return (\@argv, undef); +} + +# ── TypeScript / JavaScript ───────────────────────────────────────────────── +my $SPAWNERS = qr/(?:execFileAsync|execFileSync|execFile|execAsync|execSync|spawnSync|spawn|runCommand|exec)/; + +sub scan_ts { + my ($rel, $raw) = @_; + my $consts = local_consts($raw); + + # 1. spawn("sudo"|"/usr/bin/sudo", [ ...literal array... ]) + while ($raw =~ /$SPAWNERS\s*\(\s*"((?:\/usr\/bin\/)?sudo)"\s*,\s*\[([^\]]*)\]/gs) { + my ($bin, $args_src) = ($1, $2); + my $norm = collapse($args_src); + if (my $declared = $DECLARED_ARGV{"$rel :: $norm"}) { + push @calls, { file => $rel, argv => $_ } for @$declared; + next; + } + my ($argv, $why) = resolve_items($rel, $consts, [split_argv_items($args_src)]); + if (!$argv) { add_unresolved($rel, $norm, $why); next; } + push @calls, { file => $rel, argv => [$bin, @$argv] }; + } + + # 2. A variable that may hold "sudo", spawned with a non-literal argv. This is + # how src/lib/system-profile.ts and src/lib/hermes-cli.ts branch between a + # privileged and an unprivileged exec, and it must not slip past silently. + my %maybe_sudo; + while ($raw =~ /(?:const|let|var)\s+([A-Za-z_][A-Za-z0-9_]*)\s*(?::[^=\n]+)?=\s*([^;\n]*"(?:\/usr\/bin\/)?sudo"[^;\n]*);/g) { + $maybe_sudo{$1} = 1; + } + while ($raw =~ /(?:const|let|var)\s+([A-Za-z_][A-Za-z0-9_]*)\s*(?::[^=\n]+)?=\s*([^;\n]*\b([A-Z_][A-Z0-9_]*)\b[^;\n]*);/g) { + my ($name, $rhs) = ($1, $2); + $maybe_sudo{$name} = 1 if $rhs =~ /\bSUDO[A-Z0-9_]*\b/; + } + for my $var (sort keys %maybe_sudo) { + while ($raw =~ /$SPAWNERS\s*\(\s*\Q$var\E\s*,\s*([A-Za-z_][A-Za-z0-9_]*)\s*[,)]/gs) { + my $second = $1; + my $norm = "$var, $second"; + if (my $declared = $DECLARED_ARGV{"$rel :: $norm"}) { + push @calls, { file => $rel, argv => $_ } for @$declared; + next; + } + add_unresolved($rel, $norm, "argv is built at runtime from `$var` / `$second`"); + } + while ($raw =~ /$SPAWNERS\s*\(\s*\Q$var\E\s*,\s*\[([^\]]*)\]/gs) { + my $args_src = $1; + my $norm = collapse($args_src); + if (my $declared = $DECLARED_ARGV{"$rel :: $norm"}) { + push @calls, { file => $rel, argv => $_ } for @$declared; + next; + } + add_unresolved($rel, $norm, "argv is spawned through `$var`, which may hold sudo"); + } + } + + # 3. An absolute /usr/bin/sudo inside a generated shell script (template + # literal). Bare `sudo` in prose is deliberately not matched — see header. + my $code = $raw; + $code =~ s{/\*.*?\*/}{}gs; + $code =~ s{^\s*//[^\n]*}{}mg; + for my $line (split /\n/, $code) { + next unless $line =~ m{(?:^|[\s;&|(])/usr/bin/sudo\s+(.+)$}; + my $rest = $1; + $rest =~ s/\s+(?:\|\||&&|\||;|>|2>|`|\$\{).*$//; + $rest =~ s/^\s+|\s+$//g; + my @argv = split /\s+/, $rest; + if (grep { /[\$"'`\\]/ } @argv) { + add_unresolved($rel, "/usr/bin/sudo $rest", 'inline shell sudo has non-literal arguments'); + next; + } + push @calls, { file => $rel, argv => ['/usr/bin/sudo', @argv] }; + } +} + +# ── Shell ─────────────────────────────────────────────────────────────────── +# Quoted regions collapse to %STR% so `echo "… sudo …"` stops looking like an +# invocation, while `sudo tee "$f"` keeps its shape and is flagged as dynamic. +sub mask_shell_strings { + my ($line) = @_; + my ($out, $quote) = ('', ''); + for my $ch (split //, $line) { + if ($quote ne '') { $quote = '' if $ch eq $quote; next; } + if ($ch eq '"' || $ch eq "'") { $quote = $ch; $out .= "%STR%"; next; } + $out .= $ch; + } + return $out; +} + +sub scan_sh { + my ($rel, $raw) = @_; + for my $line (split /\n/, $raw) { + next if $line =~ /^\s*#/; + my $code = mask_shell_strings($line); + $code =~ s/\s#\s.*$//; + next unless $code =~ m{(?:^|[\s;&|(])(?:/usr/bin/)?sudo\s+(.+)$}; + my $rest = $1; + $rest =~ s/\s+(?:\|\||&&|\||;|>|2>|\\\s*$).*$//; + $rest =~ s/\s*\\\s*$//; + $rest =~ s/^\s+|\s+$//g; + my @argv = split /\s+/, $rest; + shift @argv while @argv && ($argv[0] eq '-n' || $argv[0] eq '-E'); + if (grep { /[\$%*?]/ } @argv) { + add_unresolved($rel, "sudo $rest", 'shell sudo call has non-literal arguments'); + next; + } + push @calls, { file => $rel, argv => \@argv }; + } +} + +for my $rel (@files) { + open(my $fh, '<', "$root/$rel") or next; + local $/; + my $src = <$fh>; + close $fh; + next unless defined $src && $src =~ /sudo/; + if ($rel =~ /\.sh$/) { scan_sh($rel, $src) } else { scan_ts($rel, $src) } +} + +# ── Match ─────────────────────────────────────────────────────────────────── +sub normalize_argv { + my ($argv) = @_; + my @a = @$argv; + shift @a if @a && ($a[0] eq 'sudo' || $a[0] eq '/usr/bin/sudo'); + while (@a && $a[0] =~ /^-/) { + return (undef, "sudo option `$a[0]` changes the request; resolve it by hand") + unless $a[0] eq '-n' || $a[0] eq '-E'; + shift @a; + } + return (undef, 'sudo invoked with no command') unless @a; + if ($a[0] !~ m{^/}) { + my $mapped = $BIN_PATH{$a[0]}; + return (undef, "argv[0] `$a[0]` has no known absolute path (add it to %BIN_PATH)") + unless defined $mapped; + $a[0] = $mapped; + } + return (\@a, undef); +} + +# Exact comparison throughout: check_grant_shape() rejects `*` and `?` at parse +# time, so every grant is a literal command line and sudo's glob semantics — the +# ones that let `clawbox-*` swallow a second unit name — cannot apply here. +sub grant_matches { + my ($grant, $argv) = @_; + my ($gpath, @gargs) = split /\s+/, $grant; + return 0 unless $gpath eq $argv->[0]; + # sudoers(5): a Cmnd listed without arguments may be run with any arguments. + return 1 unless @gargs; + return "@gargs" eq "@{$argv}[1 .. $#$argv]" ? 1 : 0; +} + +my (@uncovered, %seen); +for my $call (@calls) { + my ($argv, $why) = normalize_argv($call->{argv}); + if (!$argv) { add_unresolved($call->{file}, join(' ', @{$call->{argv}}), $why); next; } + my $cmdline = join(' ', @$argv); + my $hit = 0; + for my $g (@grants) { if (grant_matches($g->{cmd}, $argv)) { $g->{used} = 1; $hit = 1 } } + next if $hit; + next if $seen{"$call->{file}|$cmdline"}++; + push @uncovered, { file => $call->{file}, cmd => $cmdline }; +} + +# A grant is also "used" when it is the bare-unit twin of a used .service grant +# (or vice versa) — config/clawbox-sudoers ships both spellings on purpose. +my %used_cmd = map { $_->{cmd} => 1 } grep { $_->{used} } @grants; +for my $g (@grants) { + next if $g->{used}; + my $twin = $g->{cmd}; + if ($twin =~ /\.service$/) { $twin =~ s/\.service$// } else { $twin .= '.service' } + $g->{used} = 1 if $used_cmd{$twin}; +} + +my @unused = grep { !$_->{used} && !$ACKNOWLEDGED_UNUSED{$_->{cmd}} } @grants; + +# ── Report ────────────────────────────────────────────────────────────────── +if ($mode eq 'list') { + print "GRANTS (" . scalar(@grants) . "):\n"; + printf(" %-34s %s\n", "$_->{file}:$_->{line}", $_->{cmd}) for @grants; + print "\nRESOLVED CALL SITES:\n"; + my %uniq; + for my $c (@calls) { + my ($argv) = normalize_argv($c->{argv}); + next unless $argv; + my $line = sprintf(" %-50s sudo %s", $c->{file}, join(' ', @$argv)); + print "$line\n" unless $uniq{$line}++; + } + exit 0; +} + +if ($mode eq 'json') { + my $esc = sub { my $s = shift // ''; $s =~ s/(["\\])/\\$1/g; $s =~ s/\n/\\n/g; $s }; + my $arr = sub { + my ($items, @keys) = @_; + join(',', map { my $i = $_; '{' . join(',', map { qq("$_":") . $esc->($i->{$_}) . '"' } @keys) . '}' } @$items); + }; + print "{\n"; + print qq( "grants": ) . scalar(@grants) . ",\n"; + print qq( "calls": ) . scalar(@calls) . ",\n"; + print qq( "uncovered": [) . $arr->(\@uncovered, 'file', 'cmd') . "],\n"; + print qq( "unresolved": [) . $arr->(\@unresolved, 'file', 'raw', 'why') . "],\n"; + print qq( "unused": [) . $arr->(\@unused, 'file', 'cmd') . "]\n"; + print "}\n"; + exit((@uncovered || @unresolved || @unused) ? 1 : 0); +} + +my $fail = 0; + +if (@unresolved) { + $fail = 1; + print STDERR "\nUNRESOLVED sudo call sites (" . scalar(@unresolved) . "):\n"; + print STDERR " This check is fail-closed. Give the call literal arguments, add an entry to\n"; + print STDERR " DECLARED_ARGV in scripts/check-sudoers-coverage.sh, or exempt it in\n"; + print STDERR " EXEMPT_CALLS with a reason.\n\n"; + print STDERR " $_->{file}\n key: $_->{key}\n why: $_->{why}\n" for @unresolved; +} + +if (@uncovered) { + $fail = 1; + print STDERR "\nUNCOVERED sudo invocations (" . scalar(@uncovered) . "):\n"; + print STDERR " Nothing in " . join(' or ', @SUDOERS_FILES) . " grants these, so on a\n"; + print STDERR " real device they hit a password prompt no one can answer.\n\n"; + print STDERR " $_->{file}\n sudo $_->{cmd}\n" for @uncovered; +} + +if (@unused) { + $fail = 1; + print STDERR "\nUNUSED grants (" . scalar(@unused) . "):\n"; + print STDERR " Nothing in the scanned tree invokes these. Remove them, or acknowledge them\n"; + print STDERR " in ACKNOWLEDGED_UNUSED with the operator path that needs them.\n\n"; + print STDERR " $_->{file}:$_->{line} $_->{cmd}\n" for @unused; +} + +if ($fail) { + print STDERR "\ncheck-sudoers-coverage: FAILED\n"; + exit 1; +} + +printf("check-sudoers-coverage: OK — %d grants, %d resolved sudo invocations, 0 gaps\n", + scalar(@grants), scalar(@calls)); +exit 0; +PERL_EOF diff --git a/scripts/pr-review.mjs b/scripts/pr-review.mjs index 5e255f396..f0b6bb11d 100644 --- a/scripts/pr-review.mjs +++ b/scripts/pr-review.mjs @@ -42,7 +42,7 @@ const DOCS_ONLY_RE = /^(docs-site\/|docs\/|\.github\/|scripts\/(issue-triage|pr- // Security-sensitive paths (attention flag, rendered as ℹ️ note, not ⚠️). // config/ is deliberately narrowed to root-privilege files — the whole dir // would flag every routine openclaw-target.txt version bump. -const SENSITIVE_RE = /^(install(-x64)?\.sh|scripts\/(gateway-pre-start|start-ap|force-update|root-update-step|launch-browser|recover)\.sh|\.github\/workflows\/|src\/middleware\.ts|src\/lib\/(auth|chpasswd|mcp-token|local-ai-token|login-rate-limit|rate-limit|oauth-utils|oauth-config)\.ts|src\/app\/login-api\/|src\/app\/setup-api\/system\/credentials\/|production-server\.js|config\/(.*sudoers.*|49-|.*\.(service|rules|pkla)))/; +const SENSITIVE_RE = /^(install(-x64)?\.sh|scripts\/(gateway-pre-start|start-ap|force-update|root-update-step|launch-browser|recover|check-sudoers-coverage)\.sh|\.github\/workflows\/|src\/middleware\.ts|src\/lib\/(auth|chpasswd|mcp-token|local-ai-token|login-rate-limit|rate-limit|oauth-utils|oauth-config|root-steps)\.ts|src\/app\/login-api\/|src\/app\/setup-api\/system\/credentials\/|production-server\.js|config\/(.*sudoers.*|49-|clawbox-root-.*|.*\.(service|rules|pkla)))/; // Keep in sync with the `area` enum in scripts/issue-triage.mjs — both bots // must emit the same `area: X` label taxonomy. const AREA_RULES = [ diff --git a/scripts/root-update-step.sh b/scripts/root-update-step.sh index 5ccbe3279..fa6ea31d1 100755 --- a/scripts/root-update-step.sh +++ b/scripts/root-update-step.sh @@ -1,6 +1,36 @@ #!/usr/bin/env bash -# Shim: delegates to install.sh --step for backwards compatibility -# with the deployed systemd template that still references this path. -# Once install.sh runs again (which deploys the updated template), -# this shim is no longer needed. +# Shim for devices whose deployed clawbox-root-update@.service still names this +# path. Once install.sh runs again it rewrites the unit to point straight at +# /usr/local/libexec/clawbox/clawbox-root-step.sh and this file is unused. +# +# It must NOT exec install.sh directly. This path is reached as root, and going +# to /home/clawbox/clawbox/install.sh from here would skip everything the +# root-owned dispatcher exists to do — the step allow-list, the self-update +# pinning, and the integrity check on the tree root is about to run — leaving +# any field device still on the old unit outside the whole fix. TASK-445. +DISPATCHER=/usr/local/libexec/clawbox/clawbox-root-step.sh +LIBEXEC_DIR=/usr/local/libexec/clawbox + +if [ -x "$DISPATCHER" ]; then + exec "$DISPATCHER" "$1" +fi + +# No dispatcher. Two very different devices land here, and they get different +# answers: +# +# * $LIBEXEC_DIR does not exist at all — a device provisioned before the +# root-owned entrypoint existed. Its unit names this file precisely because +# nothing newer has been installed yet, and the update that installs the +# dispatcher has to be able to run. Refusing would strand exactly the +# devices this shim exists for, so fall through to install.sh, loudly. +# * $LIBEXEC_DIR exists but the dispatcher is missing or not executable — a +# half-installed or tampered device. Do not paper over that with a root exec; +# an operator can repair it with `sudo bash install.sh`. +if [ -d "$LIBEXEC_DIR" ]; then + echo "root-update-step: $LIBEXEC_DIR exists but $DISPATCHER is missing or not executable." >&2 + echo "root-update-step: refusing to run install.sh unguarded. Repair with: sudo bash /home/clawbox/clawbox/install.sh" >&2 + exit 65 +fi + +echo "root-update-step: no root-owned dispatcher on this device yet; running the legacy path once." >&2 exec bash /home/clawbox/clawbox/install.sh --step "$1" diff --git a/scripts/start-ap.sh b/scripts/start-ap.sh index 1d9e8918a..0b715ef32 100755 --- a/scripts/start-ap.sh +++ b/scripts/start-ap.sh @@ -25,12 +25,38 @@ CONFIG_FILE="/home/clawbox/clawbox/data/config.json" DNSMASQ_SHARED="/etc/NetworkManager/dnsmasq-shared.d" CAPTIVE_CONF="$DNSMASQ_SHARED/captive-portal.conf" -# Read hotspot config if available +# Read one KEY=VALUE out of a file this script must not trust with `source`. +# +# This script runs as ROOT — clawbox-ap.service and clawbox-ap-watchdog.service +# have no User=, and install.sh's granted +# clawbox-root-update@restart_ap.service restarts them — while +# /home/clawbox/clawbox/data is written by the web server as the clawbox user. +# `source`ing that file was therefore arbitrary root code execution for anything +# with clawbox-level code execution: the web server, the in-UI terminal, the +# agent's shell. Parse it instead; the values below are only ever passed to +# nmcli as arguments, never evaluated. TASK-445. +read_env_value() { + local file="$1" key="$2" line value + [ -f "$file" ] || return 0 + [ -L "$file" ] && return 0 + line="$(grep -m1 -E "^[[:space:]]*(export[[:space:]]+)?${key}=" "$file" 2>/dev/null)" || return 0 + value="${line#*=}" + # A CRLF-terminated file leaves the CR on the value, and it would travel into + # the SSID or the PSK as an argv byte — an AP nothing can associate with. + value="${value%$'\r'}" + # Strip one layer of matching quotes; a WiFi PSK may legitimately contain + # almost anything else, so nothing further is filtered here. + case "$value" in + \"*\") value="${value#\"}"; value="${value%\"}" ;; + \'*\') value="${value#\'}"; value="${value%\'}" ;; + esac + printf '%s' "$value" +} + HOTSPOT_ENV="/home/clawbox/clawbox/data/hotspot.env" -if [ -f "$HOTSPOT_ENV" ]; then - # shellcheck source=/dev/null - source "$HOTSPOT_ENV" -fi +HOTSPOT_SSID="$(read_env_value "$HOTSPOT_ENV" HOTSPOT_SSID)" +HOTSPOT_PASSWORD="$(read_env_value "$HOTSPOT_ENV" HOTSPOT_PASSWORD)" +HOTSPOT_DISABLED="$(read_env_value "$HOTSPOT_ENV" HOTSPOT_DISABLED)" SSID="${HOTSPOT_SSID:-ClawBox-Setup}" CON_NAME="ClawBox-Setup" diff --git a/src/app/setup-api/discord/configure/route.ts b/src/app/setup-api/discord/configure/route.ts index 75dbbc33f..fe77deb38 100644 --- a/src/app/setup-api/discord/configure/route.ts +++ b/src/app/setup-api/discord/configure/route.ts @@ -127,7 +127,9 @@ async function applyRestart(harness: string, signal: AbortSignal, secret: string try { if (harness === "hermes") { const status = await ensureHermesGateway(signal); - return status.running; + // A refused restart leaves the previous process up, and the unprivileged + // status probe cannot tell the two apart — so require both. + return status.running && status.applied; } await restartGateway(); return true; diff --git a/src/app/setup-api/email/configure/route.ts b/src/app/setup-api/email/configure/route.ts index f723c5110..83c1d2ec3 100644 --- a/src/app/setup-api/email/configure/route.ts +++ b/src/app/setup-api/email/configure/route.ts @@ -182,7 +182,11 @@ export async function POST(request: Request) { // A gateway nobody installed cannot be restarted from here, so // it is still receiving on the credentials it loaded at // startup. Saying nothing would read as "receiving stopped". - ...(stop === "unmanaged" + // "unmanaged" and "restart-failed" both mean the same thing to + // the owner: nothing restarted, so it is still receiving on the + // credentials it loaded at startup. Saying nothing would read as + // "receiving stopped". + ...(stop === "unmanaged" || stop === "restart-failed" ? { warning: "Saved — receiving stops on the next gateway restart" } : {}), }); diff --git a/src/app/setup-api/install/run-step/route.ts b/src/app/setup-api/install/run-step/route.ts index 2663ad82c..b1834aa3e 100644 --- a/src/app/setup-api/install/run-step/route.ts +++ b/src/app/setup-api/install/run-step/route.ts @@ -21,10 +21,10 @@ const execFileAsync = promisify(execFile); // that would reboot, modify networking, or wipe state stays out — we don't want // a one-tap escalation surface. // -// This list is advisory-in-depth: the sudoers grant is -// `clawbox-root-update@*.service`, so the authoritative check is the one the -// root-owned dispatcher does (config/clawbox-root-step.sh). Keeping both in -// src/lib/root-steps.ts is what lets a test pin them together. TASK-445. +// This list is advisory-in-depth: systemd starts whatever instance name it is +// handed, so the authoritative check is the one the root-owned dispatcher does +// (config/clawbox-root-step.sh). Keeping both in src/lib/root-steps.ts is what +// lets a test pin them together. TASK-445. const ALLOWED_STEPS = new Set(UI_ROOT_STEPS); // Most install steps complete in ~30-120s on a warm Jetson. vnc_install / diff --git a/src/app/setup-api/setup/reset/route.ts b/src/app/setup-api/setup/reset/route.ts index c4af96333..930b066e0 100644 --- a/src/app/setup-api/setup/reset/route.ts +++ b/src/app/setup-api/setup/reset/route.ts @@ -14,6 +14,7 @@ import { CHPASSWD_INPUT_PATH, CHPASSWD_SERVICE_NAME, chpasswdRecord } from "@/li import { FACTORY_DEFAULT_PASSWORD } from "@/lib/system-password"; import { FACTORY_RESET_CONFIRMATION, isFactoryResetConfirmed } from "@/lib/factory-reset"; import { readEdition } from "@/lib/edition-source"; +import { startOllamaService } from "@/lib/local-ai-runtime"; import { execFile as execFileCb } from "child_process"; import { promisify } from "util"; import fs from "fs/promises"; @@ -60,32 +61,40 @@ const HERMES_KEEP = new Set(["hermes-agent"]); /** Delete all Ollama models so a factory reset starts with a clean slate. */ async function deleteOllamaModels(): Promise { + // Deliberately the loopback address, not getOllamaBaseUrl(): that one honours + // OLLAMA_HOST, and a factory reset must never issue /api/delete against + // somebody else's Ollama server. What this cleans is the models this device + // downloaded, which live under /usr/share/ollama on the device itself. const OLLAMA = "http://127.0.0.1:11434"; // Ollama is routinely STOPPED at reset time (the Local AI exclusive-mode // runtime shuts it down while llama.cpp is active), and its models live // under /usr/share/ollama — out of reach of the home wipe. Start it - // best-effort so the API deletes below actually run; the polkit grant - // already allows the clawbox user to manage units. + // best-effort so the API deletes below actually run. + // + // Through startOllamaService() rather than a hand-rolled systemctl call. This + // used to be a bare `systemctl start ollama` with no sudo, which worked only + // because of the unscoped polkit `manage-units` grant — the one thing that + // still makes the whole allow-list bypassable (TASK-539). The moment that + // grant goes, an unprivileged call here fails with "Interactive + // authentication required" and factory reset stops deleting models, silently. + // The shared helper already spells the unit `ollama.service` (sudoers matches + // arguments exactly, so the bare name matches nothing), passes `-n` so a box + // without the grant fails in milliseconds instead of sitting on a prompt, + // keeps the unprivileged call as a dev-shell fallback, and waits for the API + // to answer — which is what the retry loop here used to approximate. TASK-445. try { - await execFile("/usr/bin/systemctl", ["start", "ollama"], { timeout: 30_000 }); + await startOllamaService(); } catch { - // Not installed / failed to start — the fetch below decides what's cleanable. + // Not installed / never came up — the fetch below decides what's cleanable. } let models: { name: string }[] = []; - // The API needs a moment after a cold start; retry briefly. - for (let attempt = 0; attempt < 3; attempt++) { - try { - const res = await fetch(`${OLLAMA}/api/tags`, { signal: AbortSignal.timeout(5_000) }); - if (res.ok) { - const data = await res.json(); - models = data.models ?? []; - break; - } - } catch { - // Ollama not (yet) answering. - } - if (attempt < 2) await new Promise((r) => setTimeout(r, 1_500)); - else return; // never came up — nothing reachable to clean + try { + const res = await fetch(`${OLLAMA}/api/tags`, { signal: AbortSignal.timeout(5_000) }); + if (!res.ok) return; + const data = await res.json(); + models = data.models ?? []; + } catch { + return; // nothing reachable to clean } for (const { name } of models) { try { @@ -549,6 +558,12 @@ export async function POST(request: Request) { // 6b. Reset mDNS hostname to "clawbox" (avahi + hostnamectl). Data dir is // already wiped, so clawbox-root-update@set_hostname.service will read the // default and apply it before the reboot. + // reset-failed first — see the same note in system/hostname/route.ts. + await execFile("/usr/bin/sudo", [ + "/usr/bin/systemctl", + "reset-failed", + "clawbox-root-update@set_hostname.service", + ], { timeout: 10_000 }).catch(() => {}); try { await execFile("/usr/bin/sudo", [ "/usr/bin/systemctl", diff --git a/src/app/setup-api/system/hostname/route.ts b/src/app/setup-api/system/hostname/route.ts index 6af03b9a2..342ff21a4 100644 --- a/src/app/setup-api/system/hostname/route.ts +++ b/src/app/setup-api/system/hostname/route.ts @@ -87,6 +87,16 @@ export async function POST(request: Request) { } } + // Clear a previous failure first. clawbox-root-update@.service does not + // set StartLimitIntervalSec=0, so a step that failed a few times hits + // systemd's start limit and every later start is refused until something + // resets it — which used to be nothing on this path. The chpasswd and + // llamacpp hand-offs already did this; these did not. TASK-445. + await execFileAsync("/usr/bin/sudo", [ + "/usr/bin/systemctl", + "reset-failed", + "clawbox-root-update@set_hostname.service", + ]).catch(() => {}); try { await execFileAsync("/usr/bin/sudo", [ "/usr/bin/systemctl", diff --git a/src/app/setup-api/system/hotspot/route.ts b/src/app/setup-api/system/hotspot/route.ts index 4cb215cbf..f404b9771 100644 --- a/src/app/setup-api/system/hotspot/route.ts +++ b/src/app/setup-api/system/hotspot/route.ts @@ -139,6 +139,12 @@ export async function POST(request: Request) { "[hotspot] Box is a WiFi client; deferring AP restart to avoid severing the connection" ); } else { + // reset-failed first — see the note in system/hostname/route.ts. + await execFileAsync("/usr/bin/sudo", [ + "/usr/bin/systemctl", + "reset-failed", + "clawbox-root-update@restart_ap.service", + ]).catch(() => {}); await execFileAsync("/usr/bin/sudo", [ "/usr/bin/systemctl", "start", diff --git a/src/app/setup-api/telegram/configure/route.ts b/src/app/setup-api/telegram/configure/route.ts index 50cae6e68..34db06f8a 100644 --- a/src/app/setup-api/telegram/configure/route.ts +++ b/src/app/setup-api/telegram/configure/route.ts @@ -67,7 +67,10 @@ export async function POST(request: Request) { // failed save. try { const status = await ensureHermesGateway(request.signal); - if (!status.running) { + // `applied` and not just `running`: the status probe runs unprivileged + // and a refused restart leaves the OLD process up, so `running` alone + // reported the new token as live when it was not. + if (!status.running || !status.applied) { return NextResponse.json({ success: true, reset: tokenChanged, diff --git a/src/app/setup-api/whatsapp/configure/route.ts b/src/app/setup-api/whatsapp/configure/route.ts index 79be2053e..12f414a91 100644 --- a/src/app/setup-api/whatsapp/configure/route.ts +++ b/src/app/setup-api/whatsapp/configure/route.ts @@ -133,7 +133,9 @@ export async function POST(request: Request) { // as /telegram/configure. try { const status = await ensureHermesGateway(request.signal); - if (!status.running) { + // See /telegram/configure: `running` alone is satisfied by the pre-restart + // process, so the new config would be reported live while unread. + if (!status.running || !status.applied) { return NextResponse.json({ success: true, restarted: false, warning: warning ?? "restart_pending" }); } } catch (gatewayErr) { diff --git a/src/lib/hermes-email.ts b/src/lib/hermes-email.ts index 844f83815..2843fc333 100644 --- a/src/lib/hermes-email.ts +++ b/src/lib/hermes-email.ts @@ -111,7 +111,9 @@ export async function hermesEmailState(): Promise<{ /** Restart Hermes' messaging gateway so the adapter picks up the new .env. */ export async function restartHermesForEmail(signal?: AbortSignal): Promise { const status = await ensureHermesGateway(signal); - return status.running; + // Both halves: a restart that was refused leaves the gateway running on the + // PREVIOUS .env, which is exactly the state this function exists to rule out. + return status.running && status.applied; } /** @@ -121,8 +123,15 @@ export async function restartHermesForEmail(signal?: AbortSignal): Promise { + try { + // argv[0] spelled as a literal, like every other privileged exec in the + // tree: scripts/check-sudoers-coverage.sh can only resolve a call site whose + // sudo binary is written out, and a grant it cannot see is a grant nobody + // notices going stale. + await execFileAsync("/usr/bin/sudo", ["-n", SYSTEMCTL_BIN, "restart", HERMES_GATEWAY_UNIT], { + timeout: GATEWAY_TIMEOUT_MS, + signal, + }); + return true; + } catch (err) { + console.error("[hermes] gateway restart failed:", err); + return false; + } +} + +/** + * Restart a USER-scope gateway service. Stays on the CLI (systemctl --user from + * a system service would be aimed at root's session bus, not clawbox's), and no + * sudo is involved, so there is nothing to allow-list. + * + * The exit code is checked rather than assumed: runHermesCli RESOLVES on a + * non-zero exit — it only rejects on spawn failure, timeout or abort — so an + * unchecked `await` here reads as success for every kind of failure the CLI + * reports properly. + */ +async function restartHermesGatewayUserService(signal?: AbortSignal): Promise { + try { + const res = await runHermesCli(["gateway", "restart"], { + timeoutMs: GATEWAY_TIMEOUT_MS, + signal, + }); + if (res.code !== 0) { + console.error(`[hermes] gateway restart exited ${res.code}: ${res.stderr || res.stdout}`); + return false; + } + return true; + } catch (err) { + console.error("[hermes] gateway restart failed:", err); + return false; + } +} + /** * Make sure Hermes' messaging gateway is installed and running, so Telegram * messages are actually received. @@ -429,40 +520,51 @@ const GATEWAY_SERVICE_USER = process.env.CLAWBOX_USER || "clawbox"; * ClawBox user, and Hermes resolves that user's home for HERMES_HOME itself, so * the unit is correct even though the install runs through sudo. */ -export async function ensureHermesGateway(signal?: AbortSignal): Promise { +export async function ensureHermesGateway(signal?: AbortSignal): Promise { const before = await hermesGatewayStatus(signal); if (before.installed) { // A system unit can only be controlled by root; a user unit must NOT be, // or systemctl --user would be aimed at root's session bus. - const systemScope = before.scope === "system"; - await runHermesCli(["gateway", "restart", ...(systemScope ? ["--system"] : [])], { - timeoutMs: GATEWAY_TIMEOUT_MS, - signal, - sudo: systemScope, - }); - return hermesGatewayStatus(signal); + const applied = before.scope === "system" + ? await restartHermesGatewayUnit(signal) + : await restartHermesGatewayUserService(signal); + return { ...(await hermesGatewayStatus(signal)), applied }; } // A gateway running without a service unit is somebody's foreground // `hermes gateway run`. It is already receiving, and `gateway restart` would // fall through to running the next one in the FOREGROUND — which from a route // handler means blocking until the timeout kills it. Leave it alone. - if (before.running) return before; - - await runHermesCli( - [ - "gateway", - "install", - "--system", - "--run-as-user", - GATEWAY_SERVICE_USER, - "--start-now", - "--start-on-login", - ], - { timeoutMs: GATEWAY_TIMEOUT_MS, signal, sudo: true }, - ); - return hermesGatewayStatus(signal); + // + // Nothing was applied here either: that process is still serving the config it + // started with, so the caller must not claim the change is live. + if (before.running) return { ...before, applied: false }; + + // First-time provisioning only, and deliberately ungranted in sudoers: this + // writes a unit into /etc/systemd/system, and the only way to allow-list it + // would be a NOPASSWD grant on a clawbox-writable binary. `sudo -n` fails in + // milliseconds on a narrowed box; the `applied` flag carries that outward + // instead of it disappearing into a status probe. + let applied = false; + try { + const res = await runHermesCli( + [ + "gateway", + "install", + "--system", + "--run-as-user", + GATEWAY_SERVICE_USER, + "--start-now", + "--start-on-login", + ], + { timeoutMs: GATEWAY_TIMEOUT_MS, signal, sudo: true }, + ); + applied = res.code === 0; + } catch (err) { + console.error("[hermes] gateway install failed:", err); + } + return { ...(await hermesGatewayStatus(signal)), applied }; } /** diff --git a/src/lib/root-steps.ts b/src/lib/root-steps.ts index 6ba8c1d38..17386df03 100644 --- a/src/lib/root-steps.ts +++ b/src/lib/root-steps.ts @@ -4,13 +4,15 @@ * * The privilege hand-off is: clawbox-setup (User=clawbox) → * `systemctl start clawbox-root-update@.service` → install.sh as root. - * The sudoers grant for that is `clawbox-root-update@*.service`, i.e. any - * instance name at all — so the step name is attacker-influenced input on the - * root side of the boundary, and the only real check is whatever the root-owned - * entrypoint does with it. That check lives in - * config/clawbox-root-step.sh, which is installed root-owned outside the - * clawbox-writable tree; the lists here are the same data for the TypeScript - * side, and `root-steps.test.ts` pins the two together. TASK-445. + * The sudoers grants name four exact instances (chpasswd, set_hostname, + * restart_ap, llamacpp_install) — but systemd will start + * `clawbox-root-update@anything.service` for anyone who can reach it by another + * route, so the step name is still attacker-influenced input on the root side + * of the boundary, and the only real check is whatever the root-owned + * entrypoint does with it. That check lives in config/clawbox-root-step.sh, + * which is installed root-owned outside the clawbox-writable tree; the lists + * here are the same data for the TypeScript side, and `root-steps.test.ts` pins + * the two together. TASK-445. */ /** diff --git a/src/lib/whatsapp-pairing.ts b/src/lib/whatsapp-pairing.ts index 71e633e51..0544224d1 100644 --- a/src/lib/whatsapp-pairing.ts +++ b/src/lib/whatsapp-pairing.ts @@ -680,7 +680,9 @@ export function createRealDeps(): PairingDeps { // than reinvented: no new sudo rights are involved, and a box whose // gateway cannot be restarted from here reports the fact instead. const status = await ensureHermesGateway(); - return status.running; + // `applied` too — a refused restart leaves the old process up and the + // unprivileged status probe would report it as a successful restart. + return status.running && status.applied; }, spawnBridge() { diff --git a/src/tests/routes/discord/configure-hermes.test.ts b/src/tests/routes/discord/configure-hermes.test.ts index cfd539d6a..082d12ef8 100644 --- a/src/tests/routes/discord/configure-hermes.test.ts +++ b/src/tests/routes/discord/configure-hermes.test.ts @@ -80,7 +80,7 @@ describe("POST /setup-api/discord/configure — Hermes", () => { mockHarness.mockResolvedValue("hermes"); mockSetHermesToken.mockResolvedValue(); mockSetAllowlist.mockResolvedValue({ changedKeys: [], allowedUsers: [], authorized: true }); - mockEnsureGateway.mockResolvedValue({ installed: true, running: true, scope: "system" }); + mockEnsureGateway.mockResolvedValue({ installed: true, running: true, scope: "system", applied: true }); POST = (await import("@/app/setup-api/discord/configure/route")).POST; }); @@ -105,7 +105,7 @@ describe("POST /setup-api/discord/configure — Hermes", () => { }); it("reports a gateway that would not come up as saved-with-warning", async () => { - mockEnsureGateway.mockResolvedValue({ installed: true, running: false, scope: "system" }); + mockEnsureGateway.mockResolvedValue({ installed: true, running: false, scope: "system", applied: false }); const res = await POST(req()); const body = await res.json(); diff --git a/src/tests/routes/discord/configure-onboarding.test.ts b/src/tests/routes/discord/configure-onboarding.test.ts index ff2408a3d..b18cbdb49 100644 --- a/src/tests/routes/discord/configure-onboarding.test.ts +++ b/src/tests/routes/discord/configure-onboarding.test.ts @@ -130,7 +130,7 @@ describe("POST /setup-api/discord/configure — onboarding", () => { mockGet.mockResolvedValue(TOKEN); mockHarness.mockResolvedValue("hermes"); mockSetToken.mockResolvedValue(); - mockEnsureGateway.mockResolvedValue({ installed: true, running: true, scope: "system" }); + mockEnsureGateway.mockResolvedValue({ installed: true, running: true, scope: "system", applied: true }); mockSetAllowlist.mockImplementation(async (ids: string[]) => ({ changedKeys: ids.length > 0 ? ["DISCORD_ALLOWED_USERS"] : [], allowedUsers: ids, @@ -321,7 +321,7 @@ describe("POST /setup-api/discord/configure — onboarding", () => { it("reports a pending restart honestly instead of claiming success", async () => { useApi(); - mockEnsureGateway.mockResolvedValue({ installed: true, running: false, scope: "system" }); + mockEnsureGateway.mockResolvedValue({ installed: true, running: false, scope: "system", applied: false }); const body = await (await POST(req({ allowedUserIds: [OWNER_ID] }))).json(); diff --git a/src/tests/routes/setup/reset.test.ts b/src/tests/routes/setup/reset.test.ts index 4c5d767d6..a6e23ec88 100644 --- a/src/tests/routes/setup/reset.test.ts +++ b/src/tests/routes/setup/reset.test.ts @@ -30,6 +30,11 @@ vi.mock("@/lib/auth", () => ({ getSystemUsername: vi.fn(() => "clawbox"), })); +vi.mock("@/lib/local-ai-runtime", () => ({ + getOllamaBaseUrl: vi.fn(() => "http://127.0.0.1:11434"), + startOllamaService: vi.fn(async () => {}), +})); + // The confirmation gate has its own file (`reset-confirmation.test.ts`), which // exercises the real one on real timers. Here it is stubbed open: this file is // about what the wipe does once it has been allowed to start, and the real gate @@ -46,6 +51,7 @@ vi.mock("@/lib/login-rate-limit", () => ({ import { resetUpdateState } from "@/lib/updater"; import { getSystemUsername } from "@/lib/auth"; +import { startOllamaService } from "@/lib/local-ai-runtime"; type ReaddirResult = Awaited>; @@ -220,12 +226,20 @@ describe("POST /setup-api/setup/reset", () => { // Local AI exclusive mode routinely leaves Ollama STOPPED, and its models // live under /usr/share/ollama — unreachable by the home wipe. The reset // must start the service so the API deletes can actually run. + // + // Through the shared startOllamaService(), not a hand-rolled systemctl call: + // that helper is the one place the argv is pinned to the `start + // ollama.service` Cmnd_Spec in config/clawbox-sudoers, and it is the only + // caller that passes `-n` and keeps the unprivileged dev fallback. The bare + // `systemctl start ollama` this used to issue matched no sudoers rule and + // worked only through the unscoped polkit grant. TASK-445. await resetPost(); - const call = mockExecFile.mock.calls.find( - ([cmd, args]) => cmd === "/usr/bin/systemctl" && args?.[0] === "start" && args?.[1] === "ollama", + expect(vi.mocked(startOllamaService)).toHaveBeenCalled(); + const bareCall = mockExecFile.mock.calls.find( + ([cmd, args]) => typeof cmd === "string" && cmd.endsWith("systemctl") && args?.includes("ollama"), ); - expect(call).toBeDefined(); + expect(bareCall, "the reset must not talk to systemd about ollama itself").toBeUndefined(); }); it("deletes WiFi connections", async () => { diff --git a/src/tests/routes/telegram/configure-hermes.test.ts b/src/tests/routes/telegram/configure-hermes.test.ts index ee9f0d8e0..39dfc419f 100644 --- a/src/tests/routes/telegram/configure-hermes.test.ts +++ b/src/tests/routes/telegram/configure-hermes.test.ts @@ -65,7 +65,7 @@ describe("POST /setup-api/telegram/configure — harness routing", () => { mockClearOpenclawPairing.mockResolvedValue(); mockSetHermesToken.mockResolvedValue(); mockClearHermesPairing.mockResolvedValue(); - mockEnsureGateway.mockResolvedValue({ installed: true, running: true, scope: "system" }); + mockEnsureGateway.mockResolvedValue({ installed: true, running: true, scope: "system", applied: true }); POST = (await import("@/app/setup-api/telegram/configure/route")).POST; }); @@ -126,8 +126,25 @@ describe("POST /setup-api/telegram/configure — harness routing", () => { expect(body.warning).toBeTruthy(); }); + // The false success this route used to answer. A restart that sudo refused + // leaves the OLD gateway process up, and `hermes gateway status` runs + // unprivileged — so `running` was true, the route said {restarted: true}, + // and the owner's new bot token silently kept not working. + it("does not claim restarted:true when the restart was refused", async () => { + mockEnsureGateway.mockResolvedValue({ + installed: true, + running: true, + scope: "system", + applied: false, + }); + const body = await (await POST(req({ botToken: TOKEN }))).json(); + + expect(body).toMatchObject({ success: true, restarted: false }); + expect(body.warning).toBeTruthy(); + }); + it("warns when the gateway install returned but nothing is running", async () => { - mockEnsureGateway.mockResolvedValue({ installed: true, running: false, scope: "system" }); + mockEnsureGateway.mockResolvedValue({ installed: true, running: false, scope: "system", applied: false }); const body = await (await POST(req({ botToken: TOKEN }))).json(); expect(body).toMatchObject({ success: true, restarted: false }); diff --git a/src/tests/routes/whatsapp/configure.test.ts b/src/tests/routes/whatsapp/configure.test.ts index 2b28324c6..19e2cc567 100644 --- a/src/tests/routes/whatsapp/configure.test.ts +++ b/src/tests/routes/whatsapp/configure.test.ts @@ -40,7 +40,7 @@ beforeEach(async () => { paired: true, authorized: true, }); - mockEnsure.mockResolvedValue({ installed: true, running: true, scope: "system" }); + mockEnsure.mockResolvedValue({ installed: true, running: true, scope: "system", applied: true }); POST = (await import("@/app/setup-api/whatsapp/configure/route")).POST; }); @@ -145,7 +145,7 @@ describe("POST /setup-api/whatsapp/configure", () => { }); it("reports success without a restart when the gateway is not running", async () => { - mockEnsure.mockResolvedValue({ installed: false, running: false, scope: null }); + mockEnsure.mockResolvedValue({ installed: false, running: false, scope: null, applied: false }); const body = await (await post({ mode: "bot" })).json(); expect(body).toMatchObject({ success: true, restarted: false, warning: "restart_pending" }); }); @@ -187,7 +187,7 @@ describe("POST /setup-api/whatsapp/configure", () => { paired: true, authorized: false, }); - mockEnsure.mockResolvedValue({ installed: false, running: false, scope: null }); + mockEnsure.mockResolvedValue({ installed: false, running: false, scope: null, applied: false }); const body = await (await post({ enabled: true })).json(); expect(body).toMatchObject({ success: true, restarted: false, warning: "no_allowed_users" }); }); diff --git a/src/tests/unit/hermes-email.test.ts b/src/tests/unit/hermes-email.test.ts index bf18061be..b3c19697a 100644 --- a/src/tests/unit/hermes-email.test.ts +++ b/src/tests/unit/hermes-email.test.ts @@ -27,12 +27,24 @@ beforeEach(() => { describe("stopHermesEmailPolling", () => { it("restarts the gateway when one is already running", async () => { mockStatus.mockResolvedValue({ installed: true, running: true, scope: "system" }); - mockEnsure.mockResolvedValue({ installed: true, running: true, scope: "system" }); + mockEnsure.mockResolvedValue({ installed: true, running: true, scope: "system", applied: true }); await expect(stopHermesEmailPolling()).resolves.toBe("stopped"); expect(mockEnsure).toHaveBeenCalledTimes(1); }); + // runHermesCli resolves on a non-zero exit and the status probe runs + // unprivileged, so before `applied` existed a restart that was REFUSED still + // came back as `running: true` and this function answered "stopped" — telling + // the owner receiving had ended while the old process kept polling the old + // mailbox with the old allowlist. + it("does not claim 'stopped' when the restart was refused", async () => { + mockStatus.mockResolvedValue({ installed: true, running: true, scope: "system" }); + mockEnsure.mockResolvedValue({ installed: true, running: true, scope: "system", applied: false }); + + await expect(stopHermesEmailPolling()).resolves.toBe("restart-failed"); + }); + it("installs nothing on a device whose gateway is not running", async () => { mockStatus.mockResolvedValue({ installed: false, running: false, scope: null }); diff --git a/src/tests/unit/hermes-telegram.test.ts b/src/tests/unit/hermes-telegram.test.ts index e8e378e8a..033ca63a3 100644 --- a/src/tests/unit/hermes-telegram.test.ts +++ b/src/tests/unit/hermes-telegram.test.ts @@ -14,6 +14,19 @@ import path from "path"; const runHermesCliMock = vi.hoisted(() => vi.fn()); vi.mock("@/lib/hermes-cli", () => ({ runHermesCli: runHermesCliMock })); +// The system-scope restart goes through `sudo -n /usr/bin/systemctl restart +// hermes-gateway.service` rather than the Hermes CLI — see ensureHermesGateway. +const execFileMock = vi.hoisted(() => vi.fn()); +vi.mock("child_process", () => ({ execFile: execFileMock })); + +/** Make the mocked execFile succeed / fail the way promisify(execFile) sees it. */ +function execFileSucceeds() { + execFileMock.mockImplementation((_bin: string, _argv: string[], _opts: unknown, cb: (e: Error | null, out?: string, err?: string) => void) => cb(null, "", "")); +} +function execFileFails(message: string) { + execFileMock.mockImplementation((_bin: string, _argv: string[], _opts: unknown, cb: (e: Error | null) => void) => cb(new Error(message))); +} + // Captured verbatim from `hermes pairing list` with two pending requests and // one approved user. Note the second pending row: the display name is wider // than its 20-char column, so every field after it is shifted — column offsets @@ -72,6 +85,16 @@ const GATEWAY_SERVICE_RUNNING = `● hermes-gateway.service - Hermes Agent Gatew Configured to run as: clawbox ✓ System service starts at boot without requiring systemd linger`; +// The user-scope spelling of the same verdict. ClawBox installs a SYSTEM unit, +// but a device someone set up by hand can have this one, and it must never be +// driven through sudo — `systemctl --user` from a system service would target +// root's session bus, not clawbox's. +const GATEWAY_USER_SERVICE_RUNNING = `● hermes-gateway.service - Hermes Agent Gateway - Messaging Platform Integration + Loaded: loaded (/home/clawbox/.config/systemd/user/hermes-gateway.service; enabled) + Active: active (running) since Mon 2026-08-10 22:45:04 UTC; 21s ago + Main PID: 86759 (hermes) +✓ User gateway service is running`; + const GATEWAY_MANUAL_RUNNING = `✓ Gateway is running (PID: 4242) (Running manually, not as a system service) @@ -336,6 +359,8 @@ describe("ensureHermesGateway", () => { beforeEach(() => { vi.resetModules(); runHermesCliMock.mockReset(); + execFileMock.mockReset(); + execFileSucceeds(); }); it("installs a boot-time system service when none exists", async () => { @@ -360,18 +385,60 @@ describe("ensureHermesGateway", () => { expect(opts.sudo).toBe(true); }); - it("restarts an installed system service as root instead of reinstalling", async () => { + // The restart used to be `sudo -n /home/clawbox/.local/bin/hermes gateway + // restart --system`. That binary is clawbox-owned and clawbox-writable, so it + // could never be allow-listed — the sudoers coverage checker had to EXEMPT it + // — which meant the restart silently failed on any narrowed box. The unit is + // root-owned and runs User=clawbox, so systemctl grants nothing new. + it("restarts an installed system service through systemctl, not the CLI", async () => { runHermesCliMock .mockResolvedValueOnce({ code: 0, stdout: GATEWAY_SERVICE_STOPPED, stderr: "" }) - .mockResolvedValueOnce({ code: 0, stdout: "✓ System service restarted", stderr: "" }) .mockResolvedValueOnce({ code: 0, stdout: GATEWAY_SERVICE_RUNNING, stderr: "" }); const { ensureHermesGateway } = await import("@/lib/hermes-telegram"); - await ensureHermesGateway(); + await expect(ensureHermesGateway()).resolves.toMatchObject({ running: true, applied: true }); + + expect(execFileMock).toHaveBeenCalledTimes(1); + const [bin, argv] = execFileMock.mock.calls[0]; + expect(bin).toBe("/usr/bin/sudo"); + expect(argv).toEqual(["-n", "/usr/bin/systemctl", "restart", "hermes-gateway.service"]); + // Never sudo the clawbox-writable hermes binary again. + for (const [, opts] of runHermesCliMock.mock.calls) { + expect(opts?.sudo).not.toBe(true); + } + }); - const [args, opts] = runHermesCliMock.mock.calls[1]; - expect(args).toEqual(["gateway", "restart", "--system"]); - expect(opts.sudo).toBe(true); + // THE FALSE SUCCESS. `hermes gateway status` runs UNPRIVILEGED, so after a + // refused restart it still sees the OLD process and answers "running". The + // route then replied {restarted: true} and the owner's new token did nothing. + it("reports applied: false when the restart is refused, even though the old process is still up", async () => { + runHermesCliMock + .mockResolvedValueOnce({ code: 0, stdout: GATEWAY_SERVICE_RUNNING, stderr: "" }) + .mockResolvedValueOnce({ code: 0, stdout: GATEWAY_SERVICE_RUNNING, stderr: "" }); + execFileFails("sudo: a password is required"); + + const { ensureHermesGateway } = await import("@/lib/hermes-telegram"); + await expect(ensureHermesGateway()).resolves.toMatchObject({ + running: true, + applied: false, + }); + }); + + // A user-scope unit must NOT be driven with sudo (systemctl --user would be + // aimed at root's session bus), so that branch stays on the CLI — but + // runHermesCli RESOLVES on a non-zero exit, so the code has to be checked. + it("keeps a user-scope service on the CLI and honours its exit code", async () => { + runHermesCliMock + .mockResolvedValueOnce({ code: 0, stdout: GATEWAY_USER_SERVICE_RUNNING, stderr: "" }) + .mockResolvedValueOnce({ code: 1, stdout: "", stderr: "Failed to restart" }) + .mockResolvedValueOnce({ code: 0, stdout: GATEWAY_USER_SERVICE_RUNNING, stderr: "" }); + + const { ensureHermesGateway } = await import("@/lib/hermes-telegram"); + const res = await ensureHermesGateway(); + expect(res).toMatchObject({ scope: "user", running: true, applied: false }); + expect(execFileMock).not.toHaveBeenCalled(); + expect(runHermesCliMock.mock.calls[1][0]).toEqual(["gateway", "restart"]); + expect(runHermesCliMock.mock.calls[1][1]?.sudo).toBeUndefined(); }); // `gateway restart` with no service unit falls back to starting the gateway diff --git a/src/tests/unit/install-chpasswd-validation.test.ts b/src/tests/unit/install-chpasswd-validation.test.ts new file mode 100644 index 000000000..62bacb3f6 --- /dev/null +++ b/src/tests/unit/install-chpasswd-validation.test.ts @@ -0,0 +1,307 @@ +import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import { spawnSync } from "child_process"; +import fs from "fs"; +import os from "os"; +import path from "path"; + +/** + * TASK-445 audit, GAP 2b — the root side of the password change validates what + * it consumes. + * + * `step_chpasswd` reads `$PROJECT_DIR/data/.chpasswd-input` and pipes it into + * `/usr/sbin/chpasswd` as root. `$PROJECT_DIR/data` is clawbox-writable — the + * web server, the in-UI terminal and the agent's shell all run as clawbox — so + * the record is attacker-choosable. Every guard on it lived on the + * UNPRIVILEGED side (src/lib/chpasswd.ts), which is no guard at all: dropping + * + * printf 'root:\n' > /home/clawbox/clawbox/data/.chpasswd-input + * + * and starting the granted unit set ROOT's password. + * + * These tests run the real `step_chpasswd` out of install.sh with + * /usr/sbin/chpasswd redirected to a recorder, and assert what actually reaches + * it. + */ + +const REPO = path.resolve(__dirname, "../../.."); +const INSTALL_SH = fs.readFileSync(path.join(REPO, "install.sh"), "utf-8"); + +const CAN_RUN = + process.platform !== "win32" + && spawnSync("bash", ["-c", "true"], { stdio: "ignore" }).status === 0; +const d = CAN_RUN ? describe : describe.skip; + +function shellFunction(name: string): string { + const start = INSTALL_SH.indexOf(`${name}() {`); + if (start < 0) throw new Error(`${name} not found in install.sh`); + const end = INSTALL_SH.indexOf("\n}", start); + if (end < 0) throw new Error(`${name} has no closing brace`); + return INSTALL_SH.slice(start, end + 2); +} + +let tmp: string; +let project: string; +let inputFile: string; +let seen: string; + +/** Run the real step_chpasswd against the temp tree. */ +function runStep() { + const body = shellFunction("step_chpasswd") + .replace("/usr/sbin/chpasswd", `${tmp}/fake-chpasswd`); + const script = [ + "set -uo pipefail", + `PROJECT_DIR="${project}"`, + 'CLAWBOX_USER="clawbox"', + body, + "step_chpasswd", + ].join("\n"); + return spawnSync("bash", ["-c", script], { encoding: "utf-8" }); +} + +const fedToChpasswd = () => (fs.existsSync(seen) ? fs.readFileSync(seen, "utf-8") : ""); + +beforeEach(() => { + tmp = fs.mkdtempSync(path.join(os.tmpdir(), "clawbox-chpasswd-")); + project = path.join(tmp, "project"); + inputFile = path.join(project, "data", ".chpasswd-input"); + seen = path.join(tmp, "seen"); + fs.mkdirSync(path.join(project, "data"), { recursive: true }); + fs.writeFileSync(path.join(tmp, "fake-chpasswd"), `#!/usr/bin/env bash\ncat > "${seen}"\n`, { mode: 0o755 }); +}); + +afterEach(() => { + fs.rmSync(tmp, { recursive: true, force: true }); +}); + +d("install.sh::step_chpasswd", () => { + it("changes the clawbox user's password", () => { + fs.writeFileSync(inputFile, "clawbox:hunter2\n", { mode: 0o600 }); + const r = runStep(); + expect(r.status).toBe(0); + expect(fedToChpasswd()).toBe("clawbox:hunter2\n"); + expect(fs.existsSync(inputFile), "the plaintext record must be scrubbed").toBe(false); + }); + + it("keeps a password containing colons intact — chpasswd splits on the first one", () => { + fs.writeFileSync(inputFile, "clawbox:a:b:c\n", { mode: 0o600 }); + expect(runStep().status).toBe(0); + expect(fedToChpasswd()).toBe("clawbox:a:b:c\n"); + }); + + it("REFUSES a record naming root", () => { + // The escalation, in one line. + fs.writeFileSync(inputFile, "root:pwned\n", { mode: 0o600 }); + const r = runStep(); + expect(r.status).toBe(64); + expect(r.stderr).toMatch(/only clawbox may be changed here/); + expect(fedToChpasswd()).toBe(""); + }); + + it("REFUSES a record naming any other account", () => { + for (const user of ["nvidia", "ubuntu", "sshd", ""]) { + fs.writeFileSync(inputFile, `${user}:pwned\n`, { mode: 0o600 }); + const r = runStep(); + expect(r.status, `${user} was accepted`).toBe(64); + expect(fedToChpasswd()).toBe(""); + } + }); + + it("REFUSES a second record smuggled in after a valid one", () => { + fs.writeFileSync(inputFile, "clawbox:fine\nroot:pwned\n", { mode: 0o600 }); + const r = runStep(); + expect(r.status).toBe(64); + expect(r.stderr).toMatch(/exactly one record/); + expect(fedToChpasswd()).toBe(""); + }); + + it("REFUSES a carriage return", () => { + fs.writeFileSync(inputFile, "clawbox:pw\r\n", { mode: 0o600 }); + expect(runStep().status).toBe(64); + expect(fedToChpasswd()).toBe(""); + }); + + it("REFUSES a record with no password", () => { + for (const record of ["clawbox\n", "clawbox:\n"]) { + fs.writeFileSync(inputFile, record, { mode: 0o600 }); + const r = runStep(); + expect(r.status, `${JSON.stringify(record)} was accepted`).toBe(64); + expect(r.stderr).toMatch(/no password/); + } + }); + + it("REFUSES a symlinked input file", () => { + fs.symlinkSync("/etc/shadow", inputFile); + const r = runStep(); + expect(r.status).toBe(64); + expect(r.stderr).toMatch(/symlink/); + expect(fedToChpasswd()).toBe(""); + }); + + it("scrubs the rejected record instead of leaving it for the next run", () => { + fs.writeFileSync(inputFile, "root:pwned\n", { mode: 0o600 }); + runStep(); + expect(fs.existsSync(inputFile)).toBe(false); + }); + + it("still says so when there is nothing to do", () => { + const r = runStep(); + expect(r.status).toBe(1); + expect(r.stderr).toMatch(/not found/); + }); + + it("does no git or network work — the acceptance criterion, asserted on the source", () => { + const body = shellFunction("step_chpasswd"); + expect(body).not.toMatch(/\bgit\b|curl|wget|fetch/); + }); +}); + +/** + * The same defect class as GAP 2b, and it had two other live members. + * + * `$PROJECT_DIR/data` is written by the web server, i.e. by the clawbox user, + * and install.sh runs as root from a NOPASSWD grant. `source`ing anything in + * there is therefore arbitrary ROOT code execution for anything that can already + * run code as clawbox — and both of these were reachable: + * + * * `data/hostname.env` was `.`-sourced by read_configured_hostname, which the + * granted `clawbox-root-update@set_hostname.service` runs; + * * `data/network.env` was sourced at the TOP of install.sh, i.e. on every + * root run of the script, `--step chpasswd` included; + * * `data/hotspot.env` was sourced by scripts/start-ap.sh, which + * clawbox-ap.service runs with no `User=` and the granted + * `clawbox-root-update@restart_ap.service` restarts. + * + * The rule is now: root parses these files, never evaluates them. + */ +d("root never evaluates a clawbox-writable data file", () => { + const PAYLOAD = "x=$(id -u > PWNFILE)"; + + function runHostname(fileBody: string) { + const script = [ + "set -uo pipefail", + `PROJECT_DIR="${project}"`, + shellFunction("read_untrusted_env_value"), + shellFunction("validate_hostname"), + shellFunction("read_configured_hostname"), + "read_configured_hostname", + ].join("\n"); + fs.mkdirSync(path.join(project, "data"), { recursive: true }); + fs.writeFileSync(path.join(project, "data", "hostname.env"), fileBody); + return spawnSync("bash", ["-c", script], { encoding: "utf-8", cwd: tmp }); + } + + it("reads a plain HOSTNAME assignment", () => { + expect(runHostname("HOSTNAME=kitchen\n").stdout.trim()).toBe("kitchen"); + }); + + it("does not execute a command substitution planted in hostname.env", () => { + const r = runHostname(`${PAYLOAD.replace("PWNFILE", path.join(tmp, "pwned"))}\nHOSTNAME=kitchen\n`); + expect(fs.existsSync(path.join(tmp, "pwned")), "root executed data/hostname.env").toBe(false); + expect(r.stdout.trim()).toBe("kitchen"); + }); + + it("falls back to the default rather than taking a value it cannot vouch for", () => { + // The old code would have run this; the parser rejects the shape instead. + expect(runHostname("HOSTNAME=$(id -un)\n").stdout.trim()).toBe("clawbox"); + expect(runHostname("HOSTNAME=`id -un`\n").stdout.trim()).toBe("clawbox"); + expect(runHostname("HOSTNAME='kitchen; id'\n").stdout.trim()).toBe("clawbox"); + }); + + it("ignores a symlinked env file", () => { + fs.mkdirSync(path.join(project, "data"), { recursive: true }); + const target = path.join(tmp, "elsewhere.env"); + fs.writeFileSync(target, "HOSTNAME=elsewhere\n"); + const link = path.join(project, "data", "hostname.env"); + fs.rmSync(link, { force: true }); + fs.symlinkSync(target, link); + const script = [ + "set -uo pipefail", + `PROJECT_DIR="${project}"`, + shellFunction("read_untrusted_env_value"), + shellFunction("validate_hostname"), + shellFunction("read_configured_hostname"), + "read_configured_hostname", + ].join("\n"); + const r = spawnSync("bash", ["-c", script], { encoding: "utf-8" }); + expect(r.stdout.trim()).toBe("clawbox"); + }); + + it("install.sh sources nothing that lives under the clawbox-writable data/", () => { + // The root-owned /etc/clawbox/*.env files are still sourced, and that is + // fine — root owns them. What must never come back is `source` on anything + // resolving into $PROJECT_DIR. + const CLAWBOX_WRITABLE = ["$PROJECT_DIR", "$IFACE_ENV", "$hostname_env", "/home/clawbox/"]; + for (const line of INSTALL_SH.split("\n")) { + const m = /^\s*(?:\.|source)\s+(\S.*)$/.exec(line); + if (!m) continue; + for (const needle of CLAWBOX_WRITABLE) { + expect(m[1], `install.sh sources a clawbox-writable path: ${line.trim()}`).not.toContain(needle); + } + } + }); + + it("no root-capable script sources anything under the clawbox-writable data/", () => { + // The whole class, asserted once. Every one of these runs as root somewhere: + // start-ap.sh and ap-watchdog.sh from units with no `User=`, both reachable + // through the granted clawbox-root-update@restart_ap.service. + // + // launch-browser.sh is deliberately NOT here: its state file is under + // $HOME/.cache, so the root path reads /root's copy, not clawbox's, and the + // clawbox path is the clawbox user reading its own file. + for (const name of ["start-ap.sh", "ap-watchdog.sh", "stop-ap.sh"]) { + const file = path.join(REPO, "scripts", name); + if (!fs.existsSync(file)) continue; + for (const line of fs.readFileSync(file, "utf-8").split("\n")) { + if (/^\s*#/.test(line)) continue; + const m = /(?:^|[;&|(]|\s)(?:\.|source)\s+(\S+)/.exec(line); + if (!m) continue; + expect(m[1], `${name} sources a clawbox-writable path: ${line.trim()}`) + .not.toMatch(/data\/|HOTSPOT_ENV|CONFIG_FILE|\$ROOT/); + } + } + }); + + it("ap-watchdog.sh reads the disable flag without executing the file", () => { + // It runs as ROOT on a timer (clawbox-ap-watchdog.service has no User=), so + // `. "$HOTSPOT_ENV"` was arbitrary root code execution on a schedule: plant + // the payload as clawbox, wait for the next tick. + const script = fs.readFileSync(path.join(REPO, "scripts", "ap-watchdog.sh"), "utf-8"); + const body = script.split("read_env_value() {")[1].split("\n}")[0]; + const envFile = path.join(tmp, "hotspot.env"); + const pwned = path.join(tmp, "pwned-watchdog").replace(/\\/g, "/"); + fs.writeFileSync(envFile, `x=$(id -u > ${pwned})\nHOTSPOT_DISABLED=1\n`); + const r = spawnSync("bash", ["-c", [ + "set -uo pipefail", + `read_env_value() {${body}\n}`, + `read_env_value "${envFile.replace(/\\/g, "/")}" HOTSPOT_DISABLED`, + ].join("\n")], { encoding: "utf-8" }); + expect(fs.existsSync(pwned), "root executed data/hotspot.env").toBe(false); + expect(r.stdout).toBe("1"); + }); + + it("start-ap.sh parses hotspot.env instead of sourcing it", () => { + // It runs as root: clawbox-ap.service and clawbox-ap-watchdog.service carry + // no `User=`, and clawbox-root-update@restart_ap.service is granted. + const startAp = fs.readFileSync(path.join(REPO, "scripts", "start-ap.sh"), "utf-8"); + expect(startAp).not.toMatch(/source\s+"\$HOTSPOT_ENV"/); + expect(startAp).toContain("read_env_value"); + + const script = [ + "set -uo pipefail", + fs.readFileSync(path.join(REPO, "scripts", "start-ap.sh"), "utf-8") + .split("read_env_value() {")[1] + .split("\n}")[0] + .replace(/^/, "read_env_value() {") + "\n}", + `read_env_value "${path.join(tmp, "hotspot.env").replace(/\\/g, "/")}" HOTSPOT_PASSWORD`, + ].join("\n"); + fs.writeFileSync( + path.join(tmp, "hotspot.env"), + `x=$(id -u > ${path.join(tmp, "pwned-ap").replace(/\\/g, "/")})\nHOTSPOT_PASSWORD="p a$s w'ord"\n`, + ); + const r = spawnSync("bash", ["-c", script], { encoding: "utf-8" }); + expect(fs.existsSync(path.join(tmp, "pwned-ap")), "root executed data/hotspot.env").toBe(false); + // A WiFi PSK may contain almost anything, so the value is passed through + // whole — it is only ever an argv element for nmcli, never evaluated. + expect(r.stdout).toBe("p a$s w'ord"); + }); +}); diff --git a/src/tests/unit/install-foreign-edition-teardown.test.ts b/src/tests/unit/install-foreign-edition-teardown.test.ts index 36967013a..6f0792db3 100644 --- a/src/tests/unit/install-foreign-edition-teardown.test.ts +++ b/src/tests/unit/install-foreign-edition-teardown.test.ts @@ -372,13 +372,62 @@ describe("stop+disable is the right amount of force", () => { // step_edition_gateway_state masks clawbox-gateway because a plain disable // is undone from the in-UI terminal. If an equivalent grant ever appears // for a Hermes unit, this test fails and the teardown needs revisiting. - const grants = SUDOERS.split("\n").filter( - (l) => l.startsWith("clawbox ") && l.includes("systemctl"), - ); - expect(grants.some((l) => /\bstart\s+clawbox-gateway/.test(l))).toBe(true); - for (const unit of ["hermes-dashboard", "hermes-gateway"]) { - expect(grants.some((l) => l.includes(unit))).toBe(false); + // + // `restart` as well as `start`: TASK-445 round 2 dropped the redundant + // `start clawbox-gateway` grant, and `systemctl restart` brings a stopped + // unit up just the same — so the escalation the mask exists to block is + // unchanged. + // + // ── Revisited, TASK-445 follow-up ────────────────────────────────────── + // The rule used to be "no Hermes unit may appear in sudoers at all", which + // is stricter than the reason behind it. The two directions of the teardown + // defend different things: + // + // clawbox-gateway is foreign on HERMES because it is an unauthenticated + // agent surface on :18789. That is a security boundary, so the teardown + // removes the unit file AND masks it — the grants above are dead there. + // + // hermes-gateway is foreign on OPENCLAW because two harnesses polling one + // Telegram token deadlock each other. That is a functional conflict, and + // the teardown deliberately only stops+disables it: the unit is written by + // the UPSTREAM Hermes installer, so a persistent mask would make a later + // `hermes gateway install --system` write to /dev/null — the exact trap + // step_edition_gateway_state's unmask branch exists to undo. + // + // So `restart hermes-gateway` is allowed, because the alternative was worse: + // that restart was running `sudo -n /home/clawbox/.local/bin/hermes`, a + // clawbox-WRITABLE binary, i.e. one-step local root. Restarting a root-owned + // unit that runs `User=clawbox` grants nothing new. + // + // The tripwire stays, sharpened: hermes-dashboard (a web surface) still gets + // nothing, and hermes-gateway gets `restart` and NOTHING else — no `start`, + // `enable`, `stop` or `unmask`, any of which would be a new capability + // rather than a cheaper spelling of one we already had. + // Trimmed: a Windows checkout of config/clawbox-sudoers carries CRLF (the + // file has no extension, so .gitattributes' `eol=lf` rules miss it) and the + // exact-match assertion below is anchored. + const grants = SUDOERS.split("\n") + .map((l) => l.trim()) + .filter((l) => l.startsWith("clawbox ") && l.includes("systemctl")); + expect(grants.some((l) => /\b(?:re)?start\s+clawbox-gateway/.test(l))).toBe(true); + + // The dashboard units remain completely ungranted. + expect(grants.some((l) => l.includes("hermes-dashboard"))).toBe(false); + + // hermes-gateway: restart only, both spellings, nothing else. + const hermesGateway = grants.filter((l) => /\bhermes-gateway\b/.test(l)); + expect(hermesGateway).toHaveLength(2); + for (const line of hermesGateway) { + expect(line).toMatch( + /^clawbox ALL=\(root\) NOPASSWD: \/usr\/bin\/systemctl restart hermes-gateway(\.service)?$/, + ); } + + // And the teardown must still bring it down on an OpenClaw box, so the grant + // is only ever a way to restart a gateway that BELONGS on the device. + expect(TEARDOWN_FN).toContain('systemctl stop "$funit"'); + expect(TEARDOWN_FN).toContain('systemctl disable "$funit"'); + expect(SERVICE_REGISTRY).toContain("hermes-gateway.service"); }); it("hermes-gateway.service is not ours to delete", () => { diff --git a/src/tests/unit/install-sudoers-migration.test.ts b/src/tests/unit/install-sudoers-migration.test.ts new file mode 100644 index 000000000..84969ff37 --- /dev/null +++ b/src/tests/unit/install-sudoers-migration.test.ts @@ -0,0 +1,561 @@ +import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import { spawnSync } from "child_process"; +import fs from "fs"; +import os from "os"; +import path from "path"; + +/** + * TASK-445 round 2 — the residue the revalidation measured on the QA box. + * + * PR #436 narrowed config/clawbox-sudoers to an explicit allow-list, but the + * device still carried `/etc/sudoers.d/90-clawbox-nopasswd` containing + * + * clawbox ALL=(ALL) NOPASSWD: ALL + * + * from provisioning. sudo takes the UNION of every drop-in, so while that file + * exists the whole allow-list is decorative: anything running as clawbox (the + * web server, the in-UI terminal, the agent's shell) is still one step from + * root. Shipping a narrow file is therefore only half a fix — the installer has + * to REMOVE the wide one on devices that already have it. + * + * The second half is the order of operations. The old code copied the drop-in + * and only then ran visudo, deleting it and exiting when validation failed — + * i.e. a typo in the repo left an appliance with no console and no working + * privilege escalation at all. install_sudoers_dropin validates a staged copy + * first and keeps whatever is installed when the candidate is bad. + * + * These tests source the real functions out of install.sh (never a copy) and + * run them against a temp /etc/sudoers.d with fake root-capable tools. + */ + +const REPO = path.resolve(__dirname, "../../.."); +const INSTALL_SH = fs.readFileSync(path.join(REPO, "install.sh"), "utf-8"); + +const CAN_RUN = + process.platform !== "win32" + && spawnSync("bash", ["-c", "true"], { stdio: "ignore" }).status === 0 + && fs.existsSync("/usr/sbin/visudo"); +const d = CAN_RUN ? describe : describe.skip; + +/** The sudoers helper block, verbatim: constants through the last function. */ +function sudoersBlock(): string { + const start = INSTALL_SH.indexOf("# ── sudoers ─"); + const end = INSTALL_SH.indexOf("step_systemd_services() {"); + if (start < 0 || end < 0) throw new Error("sudoers block markers not found in install.sh"); + return INSTALL_SH.slice(start, end); +} + +let tmp: string; + +/** + * Run `body` with the real install.sh helpers in scope. + * + * The fakes are the minimum needed to exercise root-only code as a normal user: + * install — drops -o/-g so ownership flags don't fail + * chown — no-op + * visudo — `-cf ` goes to the real visudo (that check is the point of + * the test); a bare `-c` reads the machine's /etc/sudoers, which a + * test cannot, so its result is scripted via VISUDO_C_STATUS. + */ +function runShell(body: string, env: Record = {}) { + const script = ` +set -uo pipefail +export PATH="${tmp}/bin:$PATH" +${sudoersBlock()} +SUDOERS_DIR="${tmp}/sudoers.d" +SUDOERS_QUARANTINE_DIR="${tmp}/quarantine" +SUDOERS_STAGING_DIR="${tmp}/staging" +REPO="${REPO}" +${body} +`; + return spawnSync("bash", ["-c", script], { encoding: "utf-8", env: { ...process.env, ...env } }); +} + +beforeEach(() => { + tmp = fs.mkdtempSync(path.join(os.tmpdir(), "clawbox-sudoers-")); + fs.mkdirSync(path.join(tmp, "bin")); + fs.mkdirSync(path.join(tmp, "sudoers.d")); + fs.mkdirSync(path.join(tmp, "staging")); + // The fake `install` also knows how to FAIL, because that is the case the + // helper could not previously see: `visudo -c` re-reads whatever is on disk, + // so an install that never wrote anything still validates and answers 0. + // INSTALL_FAIL_DEST — refuse outright (read-only /etc, EPERM) + // INSTALL_TRUNCATE_DEST — exit 0 having written only a PREFIX of the file, + // which is what ENOSPC part-way down the allow-list + // looks like, and which still parses under visudo + fs.writeFileSync( + path.join(tmp, "bin/install"), + [ + "#!/usr/bin/env bash", + 'args=(); while [ $# -gt 0 ]; do case "$1" in -o|-g) shift 2;; *) args+=("$1"); shift;; esac; done', + 'dest="${args[${#args[@]}-1]}"', + 'src="${args[${#args[@]}-2]}"', + 'if [ -n "${INSTALL_FAIL_DEST:-}" ] && [ "$dest" = "$INSTALL_FAIL_DEST" ]; then exit 1; fi', + 'if [ -n "${INSTALL_TRUNCATE_DEST:-}" ] && [ "$dest" = "$INSTALL_TRUNCATE_DEST" ]; then', + ' head -c 40 "$src" > "$dest"; exit 0', + "fi", + 'exec /usr/bin/install "${args[@]}"', + "", + ].join("\n"), + { mode: 0o755 }, + ); + fs.writeFileSync(path.join(tmp, "bin/chown"), "#!/usr/bin/env bash\nexit 0\n", { mode: 0o755 }); + fs.writeFileSync( + path.join(tmp, "bin/visudo"), + "#!/usr/bin/env bash\n" + + 'if [ "${1:-}" = "-cf" ]; then exec /usr/sbin/visudo "$@"; fi\n' + + 'if [ "${1:-}" = "-c" ]; then exit "${VISUDO_C_STATUS:-0}"; fi\n' + + 'exec /usr/sbin/visudo "$@"\n', + { mode: 0o755 }, + ); +}); + +afterEach(() => { + fs.rmSync(tmp, { recursive: true, force: true }); +}); + +d("sudoers_grants_blanket_nopasswd", () => { + const detect = (contents: string) => + runShell(`printf '%s' "$BLANKET_FIXTURE" > "${tmp}/probe"\nsudoers_grants_blanket_nopasswd "${tmp}/probe"`, { + BLANKET_FIXTURE: contents, + }).status; + + it("detects the exact file found on the QA box", () => { + expect(detect("clawbox ALL=(ALL) NOPASSWD: ALL\n")).toBe(0); + }); + + it("detects the spacing and runas variants of the same rule", () => { + expect(detect("clawbox ALL=(ALL) NOPASSWD:ALL\n")).toBe(0); + expect(detect("clawbox\tALL=(ALL:ALL)\tNOPASSWD: ALL\n")).toBe(0); + expect(detect("%clawbox ALL=(ALL) NOPASSWD: ALL\n")).toBe(0); + expect(detect("clawbox ALL=(ALL) \\\n NOPASSWD: ALL\n")).toBe(0); + }); + + it("does not fire on a narrow grant, or on the drop-in we ship", () => { + expect(detect("clawbox ALL=(root) NOPASSWD: /usr/bin/systemctl reboot\n")).toBe(1); + expect(detect(fs.readFileSync(path.join(REPO, "config/clawbox-sudoers"), "utf-8"))).toBe(1); + expect(detect(fs.readFileSync(path.join(REPO, "config/sudoers-clawbox-ollama"), "utf-8"))).toBe(1); + }); + + it("does not fire on a commented-out rule or a PASSWD:-tagged ALL", () => { + expect(detect("# clawbox ALL=(ALL) NOPASSWD: ALL\n")).toBe(1); + expect(detect("clawbox ALL=(root) NOPASSWD: /usr/bin/systemctl reboot, PASSWD: ALL\n")).toBe(1); + }); + + // Removing someone else's blanket rule could lock the only administrator out + // of a device that is nowhere near a keyboard. The detector is scoped to the + // clawbox service user on purpose. + it("leaves rules that are not about the clawbox user alone", () => { + expect(detect("%sudo ALL=(ALL:ALL) NOPASSWD: ALL\n")).toBe(1); + expect(detect("%admin ALL=(ALL) NOPASSWD: ALL\n")).toBe(1); + expect(detect("ubuntu ALL=(ALL) NOPASSWD: ALL\n")).toBe(1); + }); +}); + +d("install_sudoers_dropin", () => { + it("installs the shipped drop-in root-owned at 0440", () => { + const r = runShell(`install_sudoers_dropin "$REPO/config/clawbox-sudoers" clawbox`); + expect(r.status).toBe(0); + const dest = path.join(tmp, "sudoers.d/clawbox"); + expect(fs.readFileSync(dest, "utf-8")).toBe( + fs.readFileSync(path.join(REPO, "config/clawbox-sudoers"), "utf-8"), + ); + expect((fs.statSync(dest).mode & 0o777).toString(8)).toBe("440"); + }); + + // The regression this replaces: cp-then-validate deleted the drop-in and + // exited, leaving the box with no way to restart a service, change the + // password, reboot, or finish an update. + it("keeps the installed file when the candidate fails visudo", () => { + fs.writeFileSync(path.join(tmp, "good"), "clawbox ALL=(root) NOPASSWD: /usr/bin/systemctl reboot\n"); + fs.writeFileSync(path.join(tmp, "bad"), "this is not sudoers syntax at all !!!\n"); + expect(runShell(`install_sudoers_dropin "${tmp}/good" clawbox`).status).toBe(0); + const r = runShell( + `install_sudoers_dropin "${tmp}/good" clawbox >/dev/null\n` + + `install_sudoers_dropin "${tmp}/bad" clawbox`, + ); + expect(r.status).toBe(1); + expect(r.stderr).toMatch(/failed visudo validation; keeping the existing/); + expect(fs.readFileSync(path.join(tmp, "sudoers.d/clawbox"), "utf-8")).toContain("systemctl reboot"); + }); + + it("treats a missing source as a failure instead of a silent no-op", () => { + const r = runShell(`install_sudoers_dropin "${tmp}/nope" clawbox`); + expect(r.status).toBe(1); + expect(r.stderr).toMatch(/is missing; leaving/); + }); + + // A fragment can parse on its own and still collide with another drop-in. + it("rolls the previous file back when the whole set stops validating", () => { + const r = runShell( + `install_sudoers_dropin "$REPO/config/clawbox-sudoers" clawbox >/dev/null\n` + + `VISUDO_C_STATUS=1 install_sudoers_dropin "$REPO/config/sudoers-clawbox-ollama" clawbox`, + ); + expect(r.status).toBe(1); + expect(r.stderr).toMatch(/rolled .* back/); + expect(fs.readFileSync(path.join(tmp, "sudoers.d/clawbox"), "utf-8")).toBe( + fs.readFileSync(path.join(REPO, "config/clawbox-sudoers"), "utf-8"), + ); + }); + + it("removes a first-time install that breaks the set", () => { + const r = runShell(`VISUDO_C_STATUS=1 install_sudoers_dropin "$REPO/config/clawbox-sudoers" fresh`); + expect(r.status).toBe(1); + expect(fs.existsSync(path.join(tmp, "sudoers.d/fresh"))).toBe(false); + }); + + // The candidate must never be staged inside /etc/sudoers.d: sudo parses every + // file in there, so an unvalidated one is live the moment it lands. + it("stages outside /etc/sudoers.d and leaves nothing behind", () => { + expect(sudoersBlock()).not.toMatch(/mktemp "\$SUDOERS_DIR/); + runShell(`install_sudoers_dropin "$REPO/config/clawbox-sudoers" clawbox`); + expect(fs.readdirSync(path.join(tmp, "staging"))).toEqual([]); + expect(fs.readdirSync(path.join(tmp, "sudoers.d"))).toEqual(["clawbox"]); + }); +}); + +/** + * The blocker this file exists to close. + * + * `install_sudoers_dropin` used to report success after an `install` that never + * wrote anything. Both its call sites run it in a CONDITION context, which + * suspends `set -e` for the whole function body, and the trailing `visudo -c` + * validates whatever is STILL on disk — so a failed install produced exit 0. + * The caller then quarantined the blanket drop-in, and a device whose only + * grant was the blanket one ended up with NEITHER: no working sudo at all, on + * an appliance with no console. + */ +d("install_sudoers_dropin — a failed install must not read as success", () => { + const dest = () => path.join(tmp, "sudoers.d/clawbox"); + + it("fails when install(1) refuses, and keeps the previous grants", () => { + fs.writeFileSync(path.join(tmp, "prev"), "clawbox ALL=(root) NOPASSWD: /usr/bin/systemctl reboot\n"); + expect(runShell(`install_sudoers_dropin "${tmp}/prev" clawbox`).status).toBe(0); + + const r = runShell(`install_sudoers_dropin "$REPO/config/clawbox-sudoers" clawbox`, { + INSTALL_FAIL_DEST: dest(), + }); + expect(r.status).toBe(1); + expect(r.stderr).toMatch(/could not install clawbox into/); + expect(fs.readFileSync(dest(), "utf-8")).toContain("systemctl reboot"); + }); + + it("fails when install(1) exits 0 having written only part of the file", () => { + // ENOSPC half-way down the allow-list. The prefix still PARSES — sudoers is + // line-oriented — so visudo cannot catch it and only a byte comparison can. + const r = runShell(`install_sudoers_dropin "$REPO/config/clawbox-sudoers" clawbox`, { + INSTALL_TRUNCATE_DEST: dest(), + }); + expect(r.status).toBe(1); + expect(fs.existsSync(dest())).toBe(false); + }); + + it("leaves no staged candidate behind when the install fails", () => { + runShell(`install_sudoers_dropin "$REPO/config/clawbox-sudoers" clawbox`, { + INSTALL_FAIL_DEST: dest(), + }); + expect(fs.readdirSync(path.join(tmp, "staging"))).toEqual([]); + }); + +}); + +/** + * The gate itself, run as install.sh really runs it. + * + * These lift the actual bytes out of step_systemd_services rather than + * re-implementing them, so the test cannot drift away from the shipped code. + */ +d("step_systemd_services' sudoers gate", () => { + /** The real gate: `local sudoers_status=0` through the end of its if/else. */ + function gateBlock(): string { + const start = INSTALL_SH.indexOf(" local sudoers_status=0"); + const end = INSTALL_SH.indexOf( + ' install_sudoers_dropin "$PROJECT_DIR/config/sudoers-clawbox-ollama"', + ); + if (start < 0 || end < 0) throw new Error("sudoers gate markers not found in install.sh"); + return INSTALL_SH.slice(start, end); + } + + /** Run the extracted gate with PROJECT_DIR pointed at the real repo. */ + function runGate(env: Record = {}) { + return runShell( + `PROJECT_DIR="$REPO"\nrun_gate() {\n${gateBlock()}\n}\nrun_gate`, + env, + ); + } + + /** A device as it ships today: blanket grant, no narrow drop-in yet. */ + function seedBlanketOnlyDevice() { + fs.writeFileSync( + path.join(tmp, "sudoers.d/90-clawbox-nopasswd"), + "clawbox ALL=(ALL) NOPASSWD: ALL\n", + ); + } + + it("quarantines the blanket grant once the allow-list really landed", () => { + seedBlanketOnlyDevice(); + const r = runGate(); + expect(r.status).toBe(0); + expect(r.stdout).toContain("Sudoers rules installed"); + expect(fs.existsSync(path.join(tmp, "sudoers.d/90-clawbox-nopasswd"))).toBe(false); + expect(fs.readFileSync(path.join(tmp, "sudoers.d/clawbox"), "utf-8")).toBe( + fs.readFileSync(path.join(REPO, "config/clawbox-sudoers"), "utf-8"), + ); + }); + + // THE REGRESSION. Before the fix this left the device with no sudoers at all. + it("does NOT quarantine the blanket grant when the install failed", () => { + seedBlanketOnlyDevice(); + const r = runGate({ INSTALL_FAIL_DEST: path.join(tmp, "sudoers.d/clawbox") }); + expect(r.stderr).toMatch(/sudoers rules NOT updated/); + expect( + fs.existsSync(path.join(tmp, "sudoers.d/90-clawbox-nopasswd")), + "the blanket drop-in must survive a failed narrow install — removing it " + + "here leaves the device with no working sudo at all", + ).toBe(true); + expect(fs.existsSync(path.join(tmp, "sudoers.d/clawbox"))).toBe(false); + }); + + it("does NOT quarantine the blanket grant on a silently truncated install", () => { + seedBlanketOnlyDevice(); + const r = runGate({ INSTALL_TRUNCATE_DEST: path.join(tmp, "sudoers.d/clawbox") }); + expect(r.stderr).toMatch(/sudoers rules NOT updated/); + expect(fs.existsSync(path.join(tmp, "sudoers.d/90-clawbox-nopasswd"))).toBe(true); + }); + + it("does NOT quarantine the blanket grant when the candidate fails visudo", () => { + seedBlanketOnlyDevice(); + const r = runGate({ VISUDO_C_STATUS: "1" }); + expect(r.stderr).toMatch(/sudoers rules NOT updated/); + expect(fs.existsSync(path.join(tmp, "sudoers.d/90-clawbox-nopasswd"))).toBe(true); + }); +}); + +d("quarantine_overbroad_sudoers", () => { + const seed = () => { + fs.writeFileSync(path.join(tmp, "sudoers.d/90-clawbox-nopasswd"), "clawbox ALL=(ALL) NOPASSWD: ALL\n"); + fs.writeFileSync( + path.join(tmp, "sudoers.d/clawbox"), + fs.readFileSync(path.join(REPO, "config/clawbox-sudoers"), "utf-8"), + ); + fs.writeFileSync(path.join(tmp, "sudoers.d/clawbox-ollama"), "clawbox ALL=(ALL) NOPASSWD: ALL\n"); + fs.writeFileSync(path.join(tmp, "sudoers.d/99-operator"), "%sudo ALL=(ALL:ALL) ALL\n"); + }; + + it("removes the blanket drop-in and keeps a root-only copy", () => { + seed(); + const r = runShell("quarantine_overbroad_sudoers"); + expect(r.status).toBe(0); + expect(r.stdout).toMatch(/Removed over-broad sudoers drop-in 90-clawbox-nopasswd/); + expect(fs.existsSync(path.join(tmp, "sudoers.d/90-clawbox-nopasswd"))).toBe(false); + + const kept = fs.readdirSync(path.join(tmp, "quarantine")); + expect(kept).toHaveLength(1); + expect(kept[0]).toMatch(/^90-clawbox-nopasswd\.\d{8}T\d{6}Z$/); + const copy = path.join(tmp, "quarantine", kept[0]); + expect(fs.readFileSync(copy, "utf-8")).toBe("clawbox ALL=(ALL) NOPASSWD: ALL\n"); + expect((fs.statSync(copy).mode & 0o777).toString(8)).toBe("400"); + // 0400 root-owned: clawbox must not be able to read the rule back out and + // re-plant it, and must not be able to delete the audit trail. + expect((fs.statSync(path.join(tmp, "quarantine")).mode & 0o777).toString(8)).toBe("700"); + }); + + it("never inspects the drop-ins the installer owns", () => { + seed(); + runShell("quarantine_overbroad_sudoers"); + expect(fs.existsSync(path.join(tmp, "sudoers.d/clawbox"))).toBe(true); + // clawbox-ollama is seeded with a blanket rule on purpose: it is on the + // managed list, so it must be skipped without being read. + expect(fs.existsSync(path.join(tmp, "sudoers.d/clawbox-ollama"))).toBe(true); + }); + + it("leaves an operator's own rule alone", () => { + seed(); + runShell("quarantine_overbroad_sudoers"); + expect(fs.readFileSync(path.join(tmp, "sudoers.d/99-operator"), "utf-8")).toContain("%sudo"); + }); + + it("is idempotent", () => { + seed(); + expect(runShell("quarantine_overbroad_sudoers").status).toBe(0); + const second = runShell("quarantine_overbroad_sudoers"); + expect(second.status).toBe(0); + expect(second.stdout).not.toMatch(/Removed over-broad/); + }); + + // A quarantined file may have defined an alias another drop-in uses. Better a + // device that still has the wide grant than a device where sudo refuses + // everything and the only fix is physical access. + it("restores everything when the removal breaks visudo -c", () => { + seed(); + const r = runShell("VISUDO_C_STATUS=1 quarantine_overbroad_sudoers"); + expect(r.status).toBe(1); + expect(r.stderr).toMatch(/restored them/); + expect(fs.readFileSync(path.join(tmp, "sudoers.d/90-clawbox-nopasswd"), "utf-8")) + .toBe("clawbox ALL=(ALL) NOPASSWD: ALL\n"); + expect(fs.readdirSync(path.join(tmp, "quarantine"))).toEqual([]); + }); +}); + +describe("install.sh wiring", () => { + const fn = (name: string) => { + const start = INSTALL_SH.indexOf(`${name}() {`); + expect(start, `${name} not found in install.sh`).toBeGreaterThan(-1); + const end = INSTALL_SH.indexOf("\n}", start); + return INSTALL_SH.slice(start, end); + }; + + it("installs the allow-list before quarantining the blanket grant", () => { + const body = fn("step_systemd_services"); + const install = body.indexOf('install_sudoers_dropin "$PROJECT_DIR/config/clawbox-sudoers" clawbox'); + const quarantine = body.indexOf("quarantine_overbroad_sudoers"); + expect(install).toBeGreaterThan(-1); + expect(quarantine).toBeGreaterThan(install); + }); + + // If the drop-in did not land, removing the wide one would leave the device + // with neither. + it("skips the quarantine when the allow-list failed to install", () => { + expect(fn("step_systemd_services")).toMatch( + /sudoers_status=\$\?[\s\S]*?quarantine_overbroad_sudoers[\s\S]*?else[\s\S]*?Warning: sudoers rules NOT updated/, + ); + }); + + // Bash suspends `set -e` for the whole dynamic extent of a command run in a + // condition context, so `if install_sudoers_dropin …; then` disarmed every + // unchecked command inside the function body too. The status has to come back + // through an explicit variable, not through the test of an `if`. + it("does not call install_sudoers_dropin from a condition context", () => { + const body = fn("step_systemd_services"); + expect(body).not.toMatch(/if\s+install_sudoers_dropin/); + expect(body).toMatch(/^\s*install_sudoers_dropin "\$PROJECT_DIR\/config\/clawbox-sudoers" clawbox$/m); + expect(body).toMatch(/^\s*sudoers_status=\$\?$/m); + }); + + // The gate that actually protects the device is a fact about the DEVICE, not + // a return code: the bytes in /etc/sudoers.d/clawbox have to equal the + // allow-list we shipped before the blanket grant is taken away. + it("proves the allow-list landed byte-for-byte before quarantining", () => { + expect(fn("step_systemd_services")).toMatch( + /cmp -s "\$PROJECT_DIR\/config\/clawbox-sudoers" "\$SUDOERS_DIR\/clawbox"[\s\S]*?quarantine_overbroad_sudoers/, + ); + }); + + it("no longer exits the installer when a drop-in fails to validate", () => { + for (const step of ["step_systemd_services", "step_performance_mode"]) { + const body = fn(step); + expect(body, step).not.toMatch(/visudo -cf \/etc\/sudoers\.d/); + expect(body, step).not.toMatch(/rm -f \/etc\/sudoers\.d/); + } + }); + + // The migration has to reach devices that are already in the field, and the + // only root path an owner can trigger from the UI is the in-app updater -> + // post_update. + it("reaches existing devices through the updater", () => { + expect(fn("step_post_update")).toMatch(/step_systemd_services/); + }); + + // Both managed drop-ins must be installed from step_systemd_services, the one + // step a fresh install and step_post_update both run unconditionally. The + // ollama grant used to live in step_performance_mode, which returns early + // under CLAWBOX_TEST_MODE — so on every box that took that return the device + // ended up with the narrowed allow-list and no ollama grant at all, and + // "save a local Ollama model" hit a password prompt nobody can answer. + it("installs the ollama drop-in through the same validating helper", () => { + expect(fn("step_systemd_services")).toMatch( + /install_sudoers_dropin "\$PROJECT_DIR\/config\/sudoers-clawbox-ollama" clawbox-ollama/, + ); + }); + + it("installs every managed drop-in from the step that always runs", () => { + const body = fn("step_systemd_services"); + for (const name of ["clawbox-sudoers", "sudoers-clawbox-ollama"]) { + expect(body, `${name} must be installed from step_systemd_services`).toContain( + `install_sudoers_dropin "$PROJECT_DIR/config/${name}"`, + ); + } + // Nowhere else may call it — a drop-in installed from a conditional step is + // a grant that silently does not exist on some devices. Every call site + // must fall inside step_systemd_services' byte range. + const start = INSTALL_SH.indexOf("step_systemd_services() {"); + const end = start + body.length; + for (const m of INSTALL_SH.matchAll(/^[ \t]*install_sudoers_dropin .*/gm)) { + expect( + m.index! >= start && m.index! < end, + `install_sudoers_dropin called outside step_systemd_services: ${m[0].trim()}`, + ).toBe(true); + } + }); + + // The quarantine hands the device's blanket root back. It must not be gated + // on a feature grant: a box that failed to install the ollama tuning grant + // still has to lose its passwordless-root drop-in. + it("gates the quarantine on the primary allow-list, not the ollama grant", () => { + const body = fn("step_systemd_services"); + const quarantineAt = body.indexOf("quarantine_overbroad_sudoers"); + const ollamaAt = body.indexOf("sudoers-clawbox-ollama"); + expect(quarantineAt).toBeGreaterThan(-1); + expect(ollamaAt).toBeGreaterThan(-1); + expect(quarantineAt).toBeLessThan(ollamaAt); + }); +}); + +describe("the root-owned helper scripts the grants point at", () => { + const libexec = (() => { + const start = INSTALL_SH.indexOf("install_root_libexec() {"); + return INSTALL_SH.slice(start, INSTALL_SH.indexOf("\n}", start)); + })(); + + // The revalidation found the deployed bundle calling + // `sudo /usr/local/libexec/clawbox/optimize-ollama.sh` on a box where + // /usr/local/libexec did not exist, so every "save a local Ollama model" quietly + // skipped the q8_0 KV-cache / flash-attention tuning. + it("ships every script a sudoers grant names", () => { + const sudoers = [ + fs.readFileSync(path.join(REPO, "config/clawbox-sudoers"), "utf-8"), + fs.readFileSync(path.join(REPO, "config/sudoers-clawbox-ollama"), "utf-8"), + ].join("\n"); + const granted = [...sudoers.matchAll(/^clawbox ALL=\(root\) NOPASSWD: (\/usr\/local\/libexec\/clawbox\/[\w.-]+)/gm)] + .map((m) => path.basename(m[1])); + expect(granted).toContain("optimize-ollama.sh"); + for (const script of new Set(granted)) { + expect(libexec, `install_root_libexec must install ${script}`).toContain(script); + expect(fs.existsSync(path.join(REPO, "scripts", script)), `scripts/${script} must exist`).toBe(true); + } + }); + + it("installs them root-owned at 0755 under a root-owned directory", () => { + expect(libexec).toMatch(/install -d -o root -g root -m 0755 "\$ROOT_LIBEXEC_DIR"/); + expect(libexec).toMatch(/install -o root -g root -m 0755 "\$PROJECT_DIR\/scripts\/\$src"/); + }); + + // Running the repo copy here would let a broken install_root_libexec pass + // unnoticed — the whole point is that the copy under sudo is the one that runs. + it("runs the root-owned copy of optimize-ollama.sh, not the clawbox-writable one", () => { + for (const step of ["step_performance_mode", "step_ollama_install"]) { + const start = INSTALL_SH.indexOf(`${step}() {`); + expect(start, `${step} not found in install.sh`).toBeGreaterThan(-1); + const body = INSTALL_SH.slice(start, INSTALL_SH.indexOf("\n}", start)); + expect(body, `${step} must run the root-owned copy`).toContain( + '"$ROOT_LIBEXEC_DIR/optimize-ollama.sh"', + ); + } + // install.sh runs as root throughout, so the repo copy — which lives under + // clawbox-writable /home/clawbox/clawbox/scripts — must not be executed + // from anywhere in it. + expect(INSTALL_SH).not.toMatch(/^[ \t]*(bash |sh )?"?\$PROJECT_DIR\/scripts\/optimize-ollama\.sh"?/m); + }); + + it("never grants a path inside the clawbox-writable project tree", () => { + const sudoers = [ + fs.readFileSync(path.join(REPO, "config/clawbox-sudoers"), "utf-8"), + fs.readFileSync(path.join(REPO, "config/sudoers-clawbox-ollama"), "utf-8"), + ].join("\n"); + for (const line of sudoers.split("\n")) { + if (!line.startsWith("clawbox ALL=")) continue; + expect(line).not.toMatch(/\/home\/clawbox/); + expect(line, "a bare ALL is the blanket grant this task removed").not.toMatch(/NOPASSWD:\s*ALL\s*$/); + } + }); +}); diff --git a/src/tests/unit/root-exec-manifest.test.ts b/src/tests/unit/root-exec-manifest.test.ts new file mode 100644 index 000000000..e50ea8f09 --- /dev/null +++ b/src/tests/unit/root-exec-manifest.test.ts @@ -0,0 +1,398 @@ +import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import { spawnSync } from "child_process"; +import fs from "fs"; +import os from "os"; +import path from "path"; + +/** + * TASK-445 audit, GAP 2 — root must not execute code the clawbox user can + * rewrite. + * + * The granted chain is `sudo systemctl start clawbox-root-update@.service` + * -> the root-owned dispatcher -> `/home/clawbox/clawbox/install.sh --step + * `. Only the middle link was root-owned: install.sh is clawbox:clawbox + * 0755 inside a clawbox-writable directory (install.sh hands the tree back with + * `chown -R clawbox:clawbox` on every root run), and the steps it dispatches go + * on to run more of that same tree as root. So the grant also meant "clawbox + * may choose the program root runs" — passwordless local root in two moves. + * + * The fix is a root-owned sha256 manifest of everything root executes on + * clawbox's behalf, written by install.sh and verified by the dispatcher before + * the exec. These tests drive the real shipped scripts, with their constants + * rewritten onto a temp tree — never a re-implementation of them. + */ + +const REPO = path.resolve(__dirname, "../../.."); +const MANIFEST_SRC = path.join(REPO, "config", "clawbox-root-manifest.sh"); +const DISPATCHER_SRC = path.join(REPO, "config", "clawbox-root-step.sh"); +const INSTALL_SH = fs.readFileSync(path.join(REPO, "install.sh"), "utf-8"); + +const CAN_RUN = + process.platform !== "win32" + && spawnSync("bash", ["-c", "true"], { stdio: "ignore" }).status === 0 + && spawnSync("sha256sum", ["--version"], { stdio: "ignore" }).status === 0; +const d = CAN_RUN ? describe : describe.skip; + +let tmp: string; +let project: string; +let libexec: string; +let etc: string; +let manifest: string; +let helper: string; +let dispatcher: string; +let marker: string; + +/** + * `install -o root -g root` is what the shipped scripts really run, and it + * fails with EPERM for a normal user. Drop the ownership flags so the copy + * still happens under a test runner; everything else is executed verbatim. + */ +const unroot = (text: string) => text.replace(/install (-d )?-o root -g root /g, "install $1"); + +/** Rewrite a shipped script's hard-coded constants onto the temp tree. */ +function retarget(src: string, dest: string, subs: Array<[RegExp, string]>) { + let text = fs.readFileSync(src, "utf-8"); + for (const [re, val] of subs) { + if (!re.test(text)) throw new Error(`constant ${re} not found in ${src}`); + text = text.replace(re, val); + } + fs.writeFileSync(dest, unroot(text), { mode: 0o755 }); +} + +function sh(script: string) { + return spawnSync("bash", ["-c", script], { encoding: "utf-8" }); +} + +const ran = () => (fs.existsSync(marker) ? fs.readFileSync(marker, "utf-8") : ""); + +beforeEach(() => { + tmp = fs.mkdtempSync(path.join(os.tmpdir(), "clawbox-manifest-")); + project = path.join(tmp, "project"); + libexec = path.join(tmp, "libexec"); + etc = path.join(tmp, "etc"); + manifest = path.join(etc, "root-exec.manifest"); + helper = path.join(libexec, "clawbox-root-manifest.sh"); + dispatcher = path.join(libexec, "clawbox-root-step.sh"); + marker = path.join(tmp, "ran"); + + fs.mkdirSync(path.join(project, "scripts"), { recursive: true }); + fs.mkdirSync(path.join(project, "config"), { recursive: true }); + fs.mkdirSync(libexec, { recursive: true }); + fs.mkdirSync(etc, { recursive: true }); + + // A stand-in install.sh that records that it ran, and under which pinning. + const stub = [ + "#!/usr/bin/env bash", + `echo "args=$* allow=[\${CLAWBOX_ALLOW_SELF_UPDATE:-}] pinned=[\${CLAWBOX_INSTALL_BOOTSTRAPPED:-}]" > "${marker}"`, + "", + ].join("\n"); + fs.writeFileSync(path.join(project, "install.sh"), stub, { mode: 0o755 }); + fs.writeFileSync(path.join(project, "scripts", "start-ap.sh"), "#!/bin/sh\nexit 0\n"); + fs.writeFileSync(path.join(project, "config", "a.service"), "[Unit]\n"); + + retarget(MANIFEST_SRC, helper, [ + [/^PROJECT_DIR=.*$/m, `PROJECT_DIR="${project}"`], + [/^MANIFEST_DIR=.*$/m, `MANIFEST_DIR="${etc}"`], + [/^MANIFEST_FILE=.*$/m, `MANIFEST_FILE="${manifest}"`], + ]); + retarget(DISPATCHER_SRC, dispatcher, [ + [/^PROJECT_DIR=.*$/m, `PROJECT_DIR="${project}"`], + [/^MANIFEST_HELPER=.*$/m, `MANIFEST_HELPER="${helper}"`], + [/^RUN_DIR=.*$/m, `RUN_DIR="${path.join(tmp, "run")}"`], + ]); +}); + +afterEach(() => { + fs.rmSync(tmp, { recursive: true, force: true }); +}); + +d("clawbox-root-manifest.sh", () => { + it("records the tree and then verifies it", () => { + expect(sh(`"${helper}" --write`).status).toBe(0); + expect(fs.existsSync(manifest)).toBe(true); + expect(sh(`"${helper}" --verify`).status).toBe(0); + }); + + it("refuses once install.sh changes", () => { + sh(`"${helper}" --write`); + fs.appendFileSync(path.join(project, "install.sh"), "\nid -u\n"); + const r = sh(`"${helper}" --verify`); + expect(r.status).toBe(65); + expect(r.stderr).toMatch(/does not match/); + }); + + it("refuses once a script a root step runs changes", () => { + // The indirection GAP 2 is really about: install.sh is only the FIRST file + // root executes out of the clawbox-writable tree. + sh(`"${helper}" --write`); + fs.writeFileSync(path.join(project, "scripts", "start-ap.sh"), "#!/bin/sh\nid > /dev/null\n"); + expect(sh(`"${helper}" --verify`).status).toBe(65); + }); + + it("does NOT treat an added file as tampering", () => { + // Deliberate, and the reason is availability. Root only ever executes files + // install.sh names explicitly, and all of those are recorded — so a file + // nobody runs is not a way to make root run it. Failing on additions, on the + // other hand, means any stray file under scripts/ refuses every root step + // for good on a console-less appliance. `scripts/__pycache__` alone would do + // it: the gateway's ExecStartPre imports scripts/gateway_origins.py, so + // CPython writes a .pyc there the first time the gateway starts. + sh(`"${helper}" --write`); + fs.writeFileSync(path.join(project, "scripts", "extra.sh"), "#!/bin/sh\n"); + expect(sh(`"${helper}" --verify`).status).toBe(0); + }); + + it("never records generated content that lives inside a covered path", () => { + fs.mkdirSync(path.join(project, "scripts", "__pycache__"), { recursive: true }); + fs.writeFileSync(path.join(project, "scripts", "__pycache__", "x.cpython-310.pyc"), "old"); + sh(`"${helper}" --write`); + expect(fs.readFileSync(manifest, "utf-8")).not.toContain("__pycache__"); + // A python minor-version bump renames it and rewrites the bytes. Neither may + // turn an ordinary distro upgrade into a device that cannot set its password. + fs.rmSync(path.join(project, "scripts", "__pycache__", "x.cpython-310.pyc")); + fs.writeFileSync(path.join(project, "scripts", "__pycache__", "x.cpython-312.pyc"), "new"); + expect(sh(`"${helper}" --verify`).status).toBe(0); + }); + + it("refuses a file removed from under a covered path", () => { + sh(`"${helper}" --write`); + fs.rmSync(path.join(project, "config", "a.service")); + expect(sh(`"${helper}" --verify`).status).toBe(65); + }); + + it("refuses when there is no manifest at all", () => { + const r = sh(`"${helper}" --verify`); + expect(r.status).toBe(65); + expect(r.stderr).toMatch(/no manifest/); + }); + + it("does not cover the runtime state the app has to write", () => { + // data/, .next/ and node_modules/ change on every build and every request. + // Covering them would turn an ordinary build into a device that refuses to + // change its own password, so they are deliberately outside the record. + sh(`"${helper}" --write`); + fs.mkdirSync(path.join(project, "data"), { recursive: true }); + fs.writeFileSync(path.join(project, "data", "config.json"), "{}"); + fs.mkdirSync(path.join(project, ".next"), { recursive: true }); + fs.writeFileSync(path.join(project, ".next", "build"), "x"); + expect(sh(`"${helper}" --verify`).status).toBe(0); + }); + + it("refuses to record a name sha256sum would have to escape", () => { + // sha256sum escapes a filename containing a backslash or a newline: it + // prefixes the line with `\` and re-encodes them. The manifest's path column + // is read back with a fixed-width strip, so recording such a name would + // produce a manifest this script cannot parse — and, because re-recording + // reproduces it, a device that refuses every root step for good. Refuse to + // write it instead. + fs.writeFileSync(path.join(project, "scripts", "back\\slash.sh"), "#!/bin/sh\n"); + const r = sh(`"${helper}" --write`); + expect(r.status).toBe(65); + expect(r.stderr).toMatch(/backslash or a newline/); + expect(fs.existsSync(manifest), "no manifest may be left behind").toBe(false); + }); + + it("records a name containing an asterisk, which needs no escaping", () => { + // The guard above is about sha256sum's escaping rules, not about "unusual + // characters" — getting it wrong in the other direction would refuse a + // perfectly ordinary file and brick the same steps. + fs.writeFileSync(path.join(project, "scripts", "star*.sh"), "#!/bin/sh\n"); + expect(sh(`"${helper}" --write`).status).toBe(0); + expect(sh(`"${helper}" --verify`).status).toBe(0); + }); + + it("checks one already-opened copy against what it recorded for a path", () => { + // --verify answers a question about the tree and is stale the moment it + // returns; the dispatcher copies the file it will run somewhere clawbox + // cannot reach and asks about the COPY. Same bytes it execs. + sh(`"${helper}" --write`); + const copy = path.join(tmp, "copy.sh"); + fs.copyFileSync(path.join(project, "install.sh"), copy); + expect(sh(`"${helper}" --verify-file install.sh "${copy}"`).status).toBe(0); + + fs.appendFileSync(copy, "\n# swapped\n"); + const r = sh(`"${helper}" --verify-file install.sh "${copy}"`); + expect(r.status).toBe(65); + expect(r.stderr).toMatch(/does not match/); + + // A path that was never recorded is refused, not silently accepted. + expect(sh(`"${helper}" --verify-file scripts/nope.sh "${copy}"`).status).toBe(65); + expect(sh(`"${helper}" --verify-file install.sh "${tmp}/missing"`).status).toBe(66); + }); + + it("rejects an unknown mode instead of doing something", () => { + expect(sh(`"${helper}" --whatever`).status).toBe(64); + }); +}); + +d("clawbox-root-step.sh — the gate in front of the exec", () => { + it("runs the step when the tree still matches its record", () => { + sh(`"${helper}" --write`); + const r = sh(`"${dispatcher}" chpasswd`); + expect(r.status).toBe(0); + expect(ran()).toContain("--step chpasswd"); + }); + + it("execs a copy it holds, not the path it checked", () => { + // Verifying $ENTRYPOINT and then exec'ing $ENTRYPOINT is a race: bash opens + // the file after the check returns. The dispatcher copies it into a + // root-only directory, hashes the copy, and runs that. + sh(`"${helper}" --write`); + expect(sh(`"${dispatcher}" chpasswd`).status).toBe(0); + const staged = path.join(tmp, "run", "root-step-install.sh"); + expect(fs.existsSync(staged), "the dispatcher did not stage the entrypoint").toBe(true); + expect(fs.readFileSync(staged, "utf-8")).toBe(fs.readFileSync(path.join(project, "install.sh"), "utf-8")); + expect(ran()).toContain("--step chpasswd"); + }); + + it("refuses when the staged copy does not match the record", () => { + // The window the copy closes: install.sh is replaced after --verify passed. + // Simulated by breaking the manifest entry for it, which is the same + // mismatch the copy would surface. + sh(`"${helper}" --write`); + const line = fs.readFileSync(manifest, "utf-8") + .split("\n") + .find((l) => l.endsWith("install.sh"))!; + fs.writeFileSync( + manifest, + fs.readFileSync(manifest, "utf-8").replace(line, line.replace(/^[0-9a-f]{4}/, "dead")), + ); + expect(sh(`"${dispatcher}" chpasswd`).status).toBe(65); + expect(ran()).toBe(""); + }); + + it("refuses the step, and never execs, once install.sh is rewritten", () => { + sh(`"${helper}" --write`); + fs.writeFileSync(path.join(project, "install.sh"), "#!/bin/sh\nid -u\n", { mode: 0o755 }); + const r = sh(`"${dispatcher}" chpasswd`); + expect(r.status).toBe(65); + expect(r.stderr).toMatch(/does not match the root-exec manifest/); + expect(ran()).toBe(""); + }); + + it("lets an update step through a stale record, because an update is what makes it stale", () => { + // src/lib/updater.ts does its own fetch/reset/clean as the clawbox user + // before it starts the rebuild step, and scripts/force-update.sh does the + // same by hand. Verifying here would fail those flows at their next step and + // leave the device refusing every root step afterwards. The update family + // re-records instead, as its first action, which is also what heals a tree + // replaced from the outside. This is not a hole in the allow-list: TASK-445 + // removed every sudo grant for a self-updating instance. + sh(`"${helper}" --write`); + fs.appendFileSync(path.join(project, "install.sh"), "\n# replaced by an update\n"); + expect(sh(`"${dispatcher}" git_pull`).status).toBe(0); + expect(ran()).toContain("allow=[1]"); + }); + + it("still refuses every step a foothold can actually reach", () => { + // The four instances config/clawbox-sudoers grants, and the rest of the + // pinned family. None of them is supposed to change the covered files. + sh(`"${helper}" --write`); + fs.appendFileSync(path.join(project, "install.sh"), "\n# tampered\n"); + for (const step of ["chpasswd", "set_hostname", "restart_ap", "llamacpp_install", "recover"]) { + expect(sh(`"${dispatcher}" ${step}`).status, `${step} ran against a tampered tree`).toBe(65); + expect(ran()).toBe(""); + } + }); + + it("fails closed when the verifier itself is missing", () => { + sh(`"${helper}" --write`); + fs.rmSync(helper); + const r = sh(`"${dispatcher}" chpasswd`); + expect(r.status).toBe(65); + expect(r.stderr).toMatch(/is missing/); + expect(ran()).toBe(""); + }); + + it("still refuses a step name outside the allow-list, before it verifies anything", () => { + expect(sh(`"${dispatcher}" ../../etc/shadow`).status).toBe(64); + expect(sh(`"${dispatcher}" definitely_not_a_step`).status).toBe(64); + }); + + it("pins a password change to the on-disk copy — no git, no network", () => { + // TASK-445's own acceptance criterion. chpasswd is not in + // SELF_UPDATING_STEPS, so install.sh's bootstrap (git fetch + reset --hard + + // re-exec) is switched off for it. + sh(`"${helper}" --write`); + expect(sh(`"${dispatcher}" chpasswd`).status).toBe(0); + expect(ran()).toContain("allow=[] pinned=[1]"); + }); + + it("lets the update family self-update", () => { + sh(`"${helper}" --write`); + expect(sh(`"${dispatcher}" git_pull`).status).toBe(0); + expect(ran()).toContain("allow=[1] pinned=[]"); + }); +}); + +/** install.sh's root-owned-entrypoint block, verbatim. */ +function libexecBlock(): string { + const start = INSTALL_SH.indexOf("ROOT_LIBEXEC_DIR="); + const end = INSTALL_SH.indexOf("# ── sudoers ─"); + if (start < 0 || end < 0) throw new Error("libexec block markers not found in install.sh"); + return INSTALL_SH.slice(start, end); +} + +d("install.sh::install_root_libexec", () => { + function runBlock(extra = "") { + const block = unroot(libexecBlock()) + .replace(/\/usr\/local\/libexec\/clawbox/g, libexec) + .replace(/\/usr\/local\/libexec/g, path.dirname(libexec)) + .replace(/\/etc\/clawbox/g, etc); + return sh([ + "set -uo pipefail", + `PROJECT_DIR="${project}"`, + 'record_provision_failure() { echo "provision-failure:$1"; }', + block, + extra, + "install_root_libexec", + ].join("\n")); + } + + beforeEach(() => { + // install_root_libexec copies out of the project tree, so the sources have + // to be in it — retargeted, because the copies it installs are then RUN + // (write_root_exec_manifest calls the one it just placed in libexec) and the + // shipped constants point at /home/clawbox/clawbox. + retarget(MANIFEST_SRC, path.join(project, "config", "clawbox-root-manifest.sh"), [ + [/^PROJECT_DIR=.*$/m, `PROJECT_DIR="${project}"`], + [/^MANIFEST_DIR=.*$/m, `MANIFEST_DIR="${etc}"`], + [/^MANIFEST_FILE=.*$/m, `MANIFEST_FILE="${manifest}"`], + ]); + retarget(DISPATCHER_SRC, path.join(project, "config", "clawbox-root-step.sh"), [ + [/^PROJECT_DIR=.*$/m, `PROJECT_DIR="${project}"`], + [/^MANIFEST_HELPER=.*$/m, `MANIFEST_HELPER="${helper}"`], + ]); + }); + + it("writes the manifest and installs the dispatcher", () => { + // Both files already exist here — the outer beforeEach put retargeted copies + // there — so assert on the CONTENT. Otherwise the test passes whether or not + // install_root_libexec copied anything. + const r = runBlock(); + expect(r.stdout + r.stderr).not.toMatch(/Warning/); + for (const name of ["clawbox-root-manifest.sh", "clawbox-root-step.sh"]) { + expect( + fs.readFileSync(path.join(libexec, name), "utf-8"), + `${name} was not installed from the project tree`, + ).toBe(fs.readFileSync(path.join(project, "config", name), "utf-8")); + } + expect(fs.existsSync(manifest)).toBe(true); + }); + + it("records the tree it is about to authorise, so the new dispatcher verifies", () => { + runBlock(); + expect(sh(`"${helper}" --verify`).status).toBe(0); + }); + + it("keeps the existing dispatcher when the manifest cannot be written", () => { + // A dispatcher newer than its manifest refuses every root step: no password + // change, no hostname change, no hotspot restart, on a box with no console. + // So the manifest is written FIRST and the dispatcher only follows it. + fs.writeFileSync(path.join(libexec, "clawbox-root-step.sh"), "#!/bin/sh\n# previous\n", { mode: 0o755 }); + const r = runBlock("write_root_exec_manifest() { return 1; }"); + expect(r.stdout + r.stderr).toMatch(/leaving the existing root dispatcher in place/); + expect(r.stdout + r.stderr).toMatch(/provision-failure:root_exec_manifest/); + expect(fs.readFileSync(path.join(libexec, "clawbox-root-step.sh"), "utf-8")).toContain("# previous"); + }); +}); diff --git a/src/tests/unit/root-steps.test.ts b/src/tests/unit/root-steps.test.ts index c037cacec..7512337a5 100644 --- a/src/tests/unit/root-steps.test.ts +++ b/src/tests/unit/root-steps.test.ts @@ -21,6 +21,13 @@ const SUDOERS = [ const read = (p: string) => fs.readFileSync(p, "utf-8"); +/** The Cmnd_Spec of every `clawbox … NOPASSWD:` rule in a drop-in. */ +const grantsIn = (file: string): string[] => + read(file) + .split("\n") + .filter((l) => l.trim().startsWith("clawbox ") && l.includes("NOPASSWD:")) + .map((l) => l.split("NOPASSWD:")[1].trim()); + /** Pull a whitespace-separated shell list assigned as NAME="..." . */ function shellList(source: string, name: string): string[] { const m = new RegExp(`^${name}="([^"]*)"`, "m").exec(source); @@ -99,12 +106,7 @@ describe("root-executed paths are outside clawbox's write access", () => { it("grants NOPASSWD root only on paths clawbox cannot write", () => { for (const file of SUDOERS) { - const grants = read(file) - .split("\n") - .filter((l) => l.trim().startsWith("clawbox ") && l.includes("NOPASSWD:")) - .map((l) => l.split("NOPASSWD:")[1].trim()); - - for (const grant of grants) { + for (const grant of grantsIn(file)) { expect( grant.includes("/home/clawbox"), `sudoers grants root on a clawbox-writable path: ${grant}`, @@ -114,13 +116,42 @@ describe("root-executed paths are outside clawbox's write access", () => { }); it("does not hand over the whole systemd unit namespace", () => { - const grants = read(SUDOERS[0]); - // `reset-failed *` / `start --no-block *` took ANY unit name. Every real - // caller passes a clawbox-* unit. - expect(grants).not.toMatch(/systemctl reset-failed \*/); - expect(grants).not.toMatch(/systemctl start --no-block \*\s*$/m); - expect(grants).toContain("systemctl reset-failed clawbox-*"); - expect(grants).toContain("systemctl start --no-block clawbox-*"); + // A `clawbox-*` PREFIX was not a scope. sudoers matches arguments as one + // concatenated string, so `*` spans whitespace and `systemctl start` takes a + // LIST of units: `start clawbox-root-update@chpasswd.service ssh.service` + // matched. Every grant is therefore an exact command now. + for (const file of SUDOERS) { + const grants = grantsIn(file); + expect(grants.length).toBeGreaterThan(0); + for (const grant of grants) { + expect(grant, `sudoers grant still uses a wildcard: ${grant}`).not.toMatch(/[*?]/); + } + } + + // The instances the web server really starts, spelled out. + const primary = read(SUDOERS[0]); + for (const step of ["chpasswd", "set_hostname", "restart_ap", "llamacpp_install"]) { + expect(primary).toContain(`clawbox-root-update@${step}.service`); + } + // The update family runs through the updater's own root chain, not through + // a sudo grant the web server can reach. + for (const step of SELF_UPDATING_ROOT_STEPS) { + expect(primary, `${step} must not be startable through sudo`) + .not.toContain(`clawbox-root-update@${step}.service`); + } + }); + + it("verifies what root is about to run before it runs it", () => { + // GAP 2: the dispatcher is root-owned, but the file it exec'd was not. + // install.sh records everything root runs on clawbox's behalf and the + // dispatcher refuses a tree that no longer matches that record. + const dispatcher = read(DISPATCHER); + expect(dispatcher).toContain("clawbox-root-manifest.sh"); + expect(dispatcher).toMatch(/--verify/); + const verifyAt = dispatcher.indexOf("--verify"); + const execAt = dispatcher.indexOf("exec /bin/bash"); + expect(execAt, "the dispatcher must still exec install.sh").toBeGreaterThan(-1); + expect(verifyAt, "the verification must happen BEFORE the exec").toBeLessThan(execAt); }); it("installs the root-owned copies before the sudoers rules that point at them", () => { @@ -128,8 +159,17 @@ describe("root-executed paths are outside clawbox's write access", () => { expect(sh).toContain("install_root_libexec"); // The ollama grant's target must be the installed copy, not the repo one. expect(read(SUDOERS[1])).toContain("/usr/local/libexec/clawbox/optimize-ollama.sh"); - const libexecAt = sh.indexOf(" install_root_libexec\n # Install sudoers rules"); - expect(libexecAt, "install_root_libexec must run before the sudoers drop-in").toBeGreaterThan(0); + // Asserted as an ORDER inside step_systemd_services rather than as one + // literal line, so the check survives the surrounding text changing (it did + // not, before TASK-445 round 2 rewrote the sudoers install). + const step = sh.slice(sh.indexOf("step_systemd_services() {")); + const body = step.slice(0, step.indexOf("\n}")); + const libexecAt = body.indexOf("install_root_libexec"); + const grantAt = body.indexOf("install_sudoers_dropin"); + expect(libexecAt, "install_root_libexec must be called by step_systemd_services").toBeGreaterThan(-1); + expect(grantAt, "the sudoers drop-in must be installed by step_systemd_services").toBeGreaterThan(-1); + expect(libexecAt, "install_root_libexec must run before the sudoers drop-in that points at it") + .toBeLessThan(grantAt); }); it("gates install.sh's self-update on an explicit opt-in", () => { diff --git a/src/tests/unit/sudoers-coverage.test.ts b/src/tests/unit/sudoers-coverage.test.ts new file mode 100644 index 000000000..95eeca34a --- /dev/null +++ b/src/tests/unit/sudoers-coverage.test.ts @@ -0,0 +1,288 @@ +import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import { spawnSync } from "child_process"; +import fs from "fs"; +import os from "os"; +import path from "path"; + +/** + * TASK-445 — the guard that keeps the sudoers allow-list an allow-list. + * + * config/clawbox-sudoers replaced `clawbox ALL=(ALL) NOPASSWD: ALL` with an + * explicit list of commands. Nothing stops that list from drifting back: a new + * `sudo` call with no matching grant fails on a device with no console (a + * password prompt nobody can answer), and the cheapest-looking fix is to widen + * the list. scripts/check-sudoers-coverage.sh makes both directions of drift a + * build failure instead. + */ + +const REPO = path.resolve(__dirname, "../../.."); +const CHECKER = path.join(REPO, "scripts/check-sudoers-coverage.sh"); + +const CAN_RUN = + process.platform !== "win32" + && spawnSync("bash", ["-c", "true"], { stdio: "ignore" }).status === 0 + && spawnSync("perl", ["-e", "1"], { stdio: "ignore" }).status === 0; +const d = CAN_RUN ? describe : describe.skip; + +function run(root: string, args: string[] = []) { + return spawnSync("bash", [CHECKER, ...args], { + encoding: "utf-8", + env: { ...process.env, CLAWBOX_REPO_ROOT: root }, + }); +} + +let fixture: string; + +/** + * A repo root that shares the real src/ and mcp/ trees (so the call sites under + * test are the real ones) but has its own config/ and scripts/, which the tests + * mutate. + */ +beforeEach(() => { + fixture = fs.mkdtempSync(path.join(os.tmpdir(), "clawbox-sudoers-cov-")); + fs.mkdirSync(path.join(fixture, "config")); + for (const f of ["clawbox-sudoers", "sudoers-clawbox-ollama"]) { + fs.copyFileSync(path.join(REPO, "config", f), path.join(fixture, "config", f)); + } + fs.symlinkSync(path.join(REPO, "src"), path.join(fixture, "src")); + fs.symlinkSync(path.join(REPO, "mcp"), path.join(fixture, "mcp")); + fs.mkdirSync(path.join(fixture, "scripts")); + for (const e of fs.readdirSync(path.join(REPO, "scripts"))) { + fs.symlinkSync(path.join(REPO, "scripts", e), path.join(fixture, "scripts", e)); + } +}); + +afterEach(() => { + fs.rmSync(fixture, { recursive: true, force: true }); +}); + +const grants = () => path.join(fixture, "config/clawbox-sudoers"); +const appendGrant = (line: string) => fs.appendFileSync(grants(), `${line}\n`); +const dropGrant = (needle: string) => + fs.writeFileSync( + grants(), + fs.readFileSync(grants(), "utf-8").split("\n").filter((l) => !l.includes(needle)).join("\n"), + ); + +d("check-sudoers-coverage", () => { + it("passes on the repo as it ships", () => { + const r = run(REPO); + expect(r.stderr + r.stdout).toMatch(/OK — \d+ grants, \d+ resolved sudo invocations, 0 gaps/); + expect(r.status).toBe(0); + }); + + it("passes on the fixture, so the fixture itself is not the thing under test", () => { + expect(run(fixture).status).toBe(0); + }); + + it("fails when a sudo call has no grant", () => { + fs.writeFileSync( + path.join(fixture, "scripts/zz-probe.sh"), + "#!/usr/bin/env bash\nsudo /usr/bin/systemctl restart some-other.service\n", + ); + const r = run(fixture); + expect(r.status).toBe(1); + expect(r.stderr).toMatch(/UNCOVERED sudo invocations/); + expect(r.stderr).toMatch(/systemctl restart some-other\.service/); + }); + + it("fails when a granted command loses its grant", () => { + dropGrant("systemctl reboot"); + const r = run(fixture); + expect(r.status).toBe(1); + expect(r.stderr).toMatch(/UNCOVERED sudo invocations/); + expect(r.stderr).toMatch(/\/usr\/bin\/systemctl reboot/); + }); + + // The reverse direction. A grant nobody uses is privilege handed out for free, + // and it is how the list creeps back towards ALL one line at a time. + it("fails on a grant nothing invokes", () => { + appendGrant("clawbox ALL=(root) NOPASSWD: /usr/bin/systemctl restart cups.service"); + const r = run(fixture); + expect(r.status).toBe(1); + expect(r.stderr).toMatch(/UNUSED grants/); + expect(r.stderr).toMatch(/cups\.service/); + }); + + // The `.service` / bare-unit pairs in config/clawbox-sudoers exist because + // sudoers matches arguments as exact strings; only one spelling is ever called. + it("accepts the bare-unit twin of a grant that is used", () => { + expect(fs.readFileSync(grants(), "utf-8")).toMatch(/systemctl restart clawbox-gateway$/m); + expect(run(fixture).status).toBe(0); + }); + + // Fail-closed: a sudo call the checker cannot read is never quietly a pass. + it("fails on a sudo call whose arguments it cannot resolve", () => { + fs.writeFileSync( + path.join(fixture, "scripts/zz-probe.sh"), + '#!/usr/bin/env bash\nsudo /usr/bin/systemctl restart "$UNIT"\n', + ); + const r = run(fixture); + expect(r.status).toBe(1); + expect(r.stderr).toMatch(/UNRESOLVED sudo call sites/); + }); + + it("does not mistake a sudo command inside a message for an invocation", () => { + fs.writeFileSync( + path.join(fixture, "scripts/zz-probe.sh"), + '#!/usr/bin/env bash\necho "Fix it with: sudo systemctl restart cups"\n', + ); + expect(run(fixture).status).toBe(0); + }); + + it("refuses a blanket grant outright", () => { + appendGrant("clawbox ALL=(root) NOPASSWD: ALL"); + const r = run(fixture); + expect(r.status).toBe(1); + expect(r.stderr).toMatch(/grants a bare ALL/); + }); + + it("refuses a grant that runs as anything other than root", () => { + appendGrant("clawbox ALL=(clawbox) NOPASSWD: /usr/bin/systemctl reboot"); + const r = run(fixture); + expect(r.status).toBe(1); + expect(r.stderr).toMatch(/only \(root\) is allowed/); + }); + + it("refuses a line it cannot parse rather than skipping it", () => { + appendGrant("clawbox ALL=(root) /usr/bin/systemctl reboot"); + const r = run(fixture); + expect(r.status).toBe(1); + expect(r.stderr).toMatch(/is not a `clawbox ALL=\(root\) NOPASSWD: ` rule/); + }); + + // ── Shape invariants (TASK-445 audit, GAP 2 + GAP 3) ────────────────────── + // + // Coverage alone never made a grant safe. These two rules are what stop the + // allow-list drifting back into the shapes the audit failed it for, and they + // are asserted here so a regression fails CI rather than a device. + + it("refuses a wildcard in the command arguments", () => { + // The real defect: sudoers matches arguments as one concatenated string, so + // this rule also matched `... start --no-block clawbox-setup.service ssh.service` + // and `systemctl start` takes a list of units. + appendGrant("clawbox ALL=(root) NOPASSWD: /usr/bin/systemctl start --no-block clawbox-*"); + const r = run(fixture); + expect(r.status).toBe(1); + expect(r.stderr).toMatch(/uses a wildcard/); + }); + + it("refuses a wildcard in the command path", () => { + appendGrant("clawbox ALL=(root) NOPASSWD: /usr/local/libexec/clawbox/*.sh"); + const r = run(fixture); + expect(r.status).toBe(1); + expect(r.stderr).toMatch(/uses a wildcard/); + }); + + it("refuses a `?` wildcard too, not just `*`", () => { + appendGrant("clawbox ALL=(root) NOPASSWD: /usr/bin/systemctl start clawbox-gatewa?.service"); + const r = run(fixture); + expect(r.status).toBe(1); + expect(r.stderr).toMatch(/uses a wildcard/); + }); + + it("refuses a grant pointing into the clawbox-writable project tree", () => { + // GAP 2 in one line: install.sh is clawbox:clawbox 0755 inside a + // clawbox-writable directory, so this grant is passwordless local root for + // anything that can already run code as clawbox. + appendGrant("clawbox ALL=(root) NOPASSWD: /home/clawbox/clawbox/install.sh --step build"); + const r = run(fixture); + expect(r.status).toBe(1); + expect(r.stderr).toMatch(/outside every root-owned prefix/); + }); + + it("refuses a grant on any other clawbox-writable location", () => { + for (const target of ["/tmp/helper.sh", "/home/clawbox/.local/bin/hermes", "/var/tmp/x"]) { + fs.copyFileSync(path.join(REPO, "config", "clawbox-sudoers"), grants()); + appendGrant(`clawbox ALL=(root) NOPASSWD: ${target}`); + const r = run(fixture); + expect(r.status, `${target} was accepted`).toBe(1); + expect(r.stderr).toMatch(/outside every root-owned prefix/); + } + }); + + it("refuses a relative command, which sudo would resolve through secure_path", () => { + appendGrant("clawbox ALL=(root) NOPASSWD: systemctl reboot"); + const r = run(fixture); + expect(r.status).toBe(1); + expect(r.stderr).toMatch(/relative command/); + }); + + it("still accepts a root-owned libexec helper", () => { + // The escape hatch the invariant leaves open, and the pattern every new + // privileged helper is supposed to follow. Granting a path nothing calls is + // an unused grant, not a shape error — so assert on the message, not the code. + appendGrant("clawbox ALL=(root) NOPASSWD: /usr/local/libexec/clawbox/clawbox-new-helper.sh --go"); + const r = run(fixture); + expect(r.stderr).not.toMatch(/outside every root-owned prefix|uses a wildcard/); + expect(r.stderr).toMatch(/UNUSED grants/); + }); + + // A direct assertion that the SHIPPED files contain no wildcard lives in + // root-steps.test.ts, which reads both drop-ins; the tests above prove the + // checker is what fails CI when one comes back. + + it("lists what it matched", () => { + const r = run(REPO, ["--list"]); + expect(r.status).toBe(0); + expect(r.stdout).toMatch(/GRANTS \(\d+\):/); + expect(r.stdout).toContain("/usr/local/libexec/clawbox/optimize-ollama.sh"); + expect(r.stdout).toContain("/usr/bin/systemctl start clawbox-root-update@chpasswd.service"); + expect(r.stdout).toMatch(/RESOLVED CALL SITES:/); + }); + + it("reports machine-readably", () => { + const r = run(REPO, ["--json"]); + expect(r.status).toBe(0); + const report = JSON.parse(r.stdout); + expect(report.uncovered).toEqual([]); + expect(report.unresolved).toEqual([]); + expect(report.unused).toEqual([]); + expect(report.grants).toBeGreaterThan(30); + expect(report.calls).toBeGreaterThan(30); + }); + + it("rejects an unknown flag instead of silently checking", () => { + expect(run(REPO, ["--nope"]).status).toBe(2); + }); +}); + +describe("the call sites the allow-list has to cover", () => { + const listed = () => { + const r = run(REPO, ["--list"]); + return r.stdout; + }; + + // Every path the task brief names, traced end to end. If one of these stops + // being covered the device loses that feature to a password prompt. + it.runIf(CAN_RUN)("covers the wizard, updater, power, wifi, desktop and factory-reset paths", () => { + const out = listed(); + for (const expected of [ + // setup wizard: hostname + hotspot hand-off, and the chpasswd hand-off + "sudo /usr/bin/systemctl start clawbox-root-update@set_hostname.service", + "sudo /usr/bin/systemctl start clawbox-root-update@restart_ap.service", + "sudo /usr/bin/systemctl start clawbox-root-update@chpasswd.service", + // power menu + "sudo /usr/bin/systemctl reboot", + "sudo /usr/bin/systemctl poweroff", + // factory reset: mask, stop, unmask, reset password, reboot + "sudo /usr/bin/systemctl --runtime mask clawbox-gateway.service", + "sudo /usr/bin/systemctl --runtime unmask clawbox-gateway.service", + "sudo /usr/bin/systemctl stop clawbox-gateway.service", + // desktop / power-profile toggles, through the root-owned copies + "sudo /usr/local/libexec/clawbox/clawbox-desktop-mode.sh --enable", + "sudo /usr/local/libexec/clawbox/clawbox-desktop-mode.sh --disable", + "sudo /usr/local/libexec/clawbox/clawbox-power-mode.sh --balanced", + "sudo /usr/local/libexec/clawbox/clawbox-power-mode.sh --performance", + // local models + "sudo /usr/bin/systemctl enable --now ollama.service", + "sudo /usr/bin/systemctl disable --now ollama.service", + "sudo /usr/local/libexec/clawbox/optimize-ollama.sh", + // remote control + "sudo /usr/bin/systemctl restart clawbox-tunnel.service", + "sudo /usr/bin/systemctl enable clawbox-tunnel.service", + ]) { + expect(out, `${expected} is no longer a resolved call site`).toContain(expected); + } + }); +});