diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 90f36427..244bb5bc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -195,6 +195,17 @@ jobs: - name: Check connect card checking state run: nix build -L .#checks.x86_64-linux.connect-card && cat result + # Candidate releases: the validator between an agent's sudo grant and + # what root builds, plus agent-box-source's half (the marker it + # consumes, and the one narrow case where the fast-forward guard + # relaxes). Both are seconds, and there is no VM test that can price + # a refusal. + - name: Check candidate release validator + run: nix build -L .#checks.x86_64-linux.candidate && cat result + + - name: Check source tree manager + run: nix build -L .#checks.x86_64-linux.source-tree && cat result + # Issue #374: the native backend's two flake checks, which nothing # ran until that bug was found by hand. `runtime-profile` builds # packages..runtime — the profile a native box installs at diff --git a/bin/agentbox b/bin/agentbox index 3d5f58ef..bfb3d1df 100755 --- a/bin/agentbox +++ b/bin/agentbox @@ -87,6 +87,18 @@ UPDATE_TRIGGER = f"{SYSTEMCTL} start --no-block {UPDATE_UNIT}" # daemon's HTTP answer is already on the wire when systemd starts stopping # units, this daemon among them. REBOOT_TRIGGER = f"{SYSTEMCTL} reboot --no-block" +# Candidate releases (this box before the fleet). The wrapper is rendered +# rather than shipped as a profile payload because it runs under sudo, whose +# env_reset means it can carry no environment: its repo, tree and trigger are +# baked in by the renderer, so a caller cannot point root at another repo. +# Spelled once for the same #353 reason as the three above — the sudoers line +# and the path the guide tells the agent to run must agree byte for byte. +CANDIDATE_HELPER = "/etc/agent-box/bin/agent-box-candidate" +CANDIDATE_TRIGGER = f"{CANDIDATE_HELPER} *" +# Where that wrapper queues the branch it wants, and where agent-box-source +# consumes it. One update reads it and deletes it: a candidate lasts until +# the next update and never pins the box. +CANDIDATE_FILE = "/var/lib/agent-box/candidate" # Store housekeeping (issue #394). A full root wedges the whole box — # no journal, no profile swap, and an agent that cannot write — and on a # native box nobody is around to run `nix-collect-garbage` by hand. The @@ -1047,6 +1059,10 @@ class Renderer: self.memory_protection(t) else: self.memory_protection_off(t) + # Before the web branch, and outside it: the sudo grant for this is + # box-wide (implied_sudo_commands), so the wrapper must exist on a + # web-disabled box too. + self.candidate_helper(t) if self.spec.web_enable: self.caddy(t) self.fail2ban(t) @@ -2628,6 +2644,15 @@ class Renderer: # per-user password grant above exists to avoid. if self.spec.reboot_button and u is self.spec.root_user: cmds.append(REBOOT_TRIGGER) + # Installing a candidate release goes to the ROOT user alone, + # for the same reason the reboot does: it takes every user's + # sessions down with it, and one user's power over another's + # work is what the per-user grants exist to avoid. `*` allows + # exactly one argument of anything, which is all the wrapper + # accepts — and the wrapper, not sudoers, is what decides + # whether that argument names an installable branch. + if u is self.spec.root_user: + cmds.append(CANDIDATE_TRIGGER) cmds += self.spec.sudo_allowlist lines.append(f"{u.name} ALL=(root) NOPASSWD: " + ", ".join(cmds)) t.file(self.p(SUDOERS_DIR, "agent-box"), @@ -2737,6 +2762,54 @@ class Renderer: + '[credential "https://gist.github.com"]\n' + f"\thelper = !{gh} auth git-credential\n") + def candidate_helper(self, t): + """agent-box-candidate: install a branch on THIS box before it is + merged and the fleet takes it (see modules/src/candidate.sh for what + it refuses). + + Called UNCONDITIONALLY, unlike the password helper it renders beside + on a web box. sudoers() grants CANDIDATE_TRIGGER from + implied_sudo_commands(), which is not gated on web.enable either -- + native has no selfUpdate.enable to gate the update trigger on, so + every native box has both. Rendering this from caddy() instead left a + web-disabled box granting a command that was not on disk: sudo would + find the rule, exec nothing, and the agent would see a bare "No such + file or directory" with no hint that the box was built without it. + Same shape as the web-gated paths #198 leaked into the ungated agent + unit, which is what tests/memory-protection.nix exists to catch. + + Constants are PREPENDED, not appended: this is shell, where the body's + `case` runs the moment it is reached, so a tail would assign after the + code that reads it. They are compiled in for the same sudo reason as + the password helper -- env_reset means the wrapper carries no + environment, and a caller-supplied repo URL would be root building + code from anywhere. + """ + candidate_src = self.profile / "libexec" / "agent-box" / \ + "candidate.sh" + if candidate_src.is_file(): + # spec.repo, which prefers the PROFILE's manifest over the + # config key: the manifest cannot disagree with what is + # installed, and this wrapper must fetch from the repo the box + # actually came from. No branch key exists natively — an empty + # SRC_BRANCH is what tells the wrapper and agent-box-source to + # follow the remote's own default, rather than guessing + # "master" at a repo that may have renamed it. + url = f"https://github.com/{self.spec.repo}.git" + head = ( + "# Box constants, prepended by `agentbox apply`.\n" + f"CANDIDATE_FILE={shlex.quote(CANDIDATE_FILE)}\n" + f"SRC_DIR={shlex.quote(SRC_DIR)}\n" + f"SRC_URL={shlex.quote(url)}\n" + "SRC_BRANCH=''\n" + f"SYSTEMCTL={shlex.quote(SYSTEMCTL)}\n" + f"UPDATE_TRIGGER={shlex.quote(UPDATE_TRIGGER)}\n" + "\n" + ) + t.file(self.p(CANDIDATE_HELPER), + "#!/bin/sh\n" + head + candidate_src.read_text(), + 0o755) + def interactive_clis(self, t): """agent-box-session and agent-box-profile, on every box. @@ -3840,7 +3913,8 @@ def source_tree(profile, verb, *rest, src=SRC_DIR, url=None, rev=None, the two backends cannot drift on what "fast-forward the box" means — only on what they do with the rev it returns. """ - env = dict(os.environ, AGENT_BOX_SRC_DIR=src) + env = dict(os.environ, AGENT_BOX_SRC_DIR=src, + AGENT_BOX_CANDIDATE_FILE=CANDIDATE_FILE) if url: env["AGENT_BOX_SRC_URL"] = url if rev: diff --git a/flake.nix b/flake.nix index 67544ee1..03140df2 100644 --- a/flake.nix +++ b/flake.nix @@ -1526,6 +1526,26 @@ open(sys.argv[3], "w").write(header + yaml.safe_dump(data, sort_keys=True))' \ cp log "$out" ''; + # agent-box-candidate (this box before the fleet): the validator + # standing between an agent's sudo grant and what root builds. + # Same shape and same reasoning as source-tree above -- weighted + # at the refusals, `origin` a local repository so there is no + # network, and natively runnable on every architecture. + candidate = + pkgs.runCommand "agent-box-candidate-check" + { + nativeBuildInputs = [ pkgs.bash pkgs.coreutils pkgs.git pkgs.gnugrep pkgs.gnused ]; + script = ./modules/src/candidate.sh; + tests = ./tests/test-candidate.sh; + } '' + bash "$tests" "$script" > log 2>&1 || { + cat log + exit 1 + } + cat log + cp log "$out" + ''; + # Issue #425: a box with no webhook panel used to render an # empty string, so its operator could not tell a feature that is # off from one that is wired up wrong — which is how #425 was diff --git a/modules/agent-box.nix b/modules/agent-box.nix index cb0256cf..6989a38d 100644 --- a/modules/agent-box.nix +++ b/modules/agent-box.nix @@ -299,6 +299,34 @@ let rebuild that failed and could not put the tree back, which the wall notice says - it reads ahead of the running system. + '' + lib.optionalString candidateEnabled '' + ## Trying YOUR fix on this box, before every other box takes it + + An update follows the tracked branch, so a fix used to have to be + merged before this box could run it - which made the whole fleet the + canary for its own tooling. Push the branch and try it here first: + + sudo ${candidateHelper} BRANCH # install it, then re-apply + sudo ${candidateHelper} --status # what is this box running? + sudo ${candidateHelper} --reset # go back to the tracked branch + + Same restart as an ordinary update, so save context first. Then merge + once you have SEEN it work, and the fleet takes a change that has run + somewhere. + + Three things it refuses, all deliberately. Only a BRANCH on this + box's own origin - not a tag, not a sha, and not `refs/pull/N/head`, + because a public repo takes pull requests from anyone and that would + be a stranger's unreviewed code built as root. Only a branch strictly + AHEAD of the tracked branch, so "candidate" can never become a way to + replay an older rev - rebase yours, which is also the only way what + you test here is what will land. And it never pins this box: the next + update returns to the tracked branch and says so, so a candidate you + forget converges back on the fleet instead of drifting from it. That + return needs no flag from you, and is the one case where the + fast-forward guard steps aside - a squash-merged branch is never an + ancestor of the branch it landed on, so nothing else could get home. + '' + lib.optionalString checkoutEnabled '' ## This box ships its own sources @@ -830,6 +858,331 @@ let else if names != [ ] then lib.head names else null; checkoutEnabled = checkoutMaintainer != null; + # Candidate releases: install a branch on THIS box before it is merged and + # every other box takes it. Same user the maintainer checkout goes to, and + # picked the same way, because it is the same job seen from the other end — + # that user holds the tree a fix is written in, and this is how the fix gets + # tried here first. ONE user, never all of them: an update takes every + # user's sessions down with it, which is exactly what the per-user password + # helper and the reboot grant exist to keep out of one agent's hands. + # Gated on selfUpdate.enable alone and NOT on web.enable — a box with no + # terminal still updates, and its agent still has a fix to try (the trap + # tests/memory-protection.nix exists to catch). + candidateUser = + let names = lib.attrNames cfg.users; in + if !cfg.selfUpdate.enable then null + else if cfg.users ? ${cfg.web.user} then cfg.web.user + else if names != [ ] then lib.head names + else null; + candidateEnabled = candidateUser != null; + # Beside the tree, not inside it: the tree is a checkout git rewrites, and + # a marker within it would be one `git clean` from vanishing. srcDir's own + # parent is created by agent-box-source before it clones, so this always + # lands in a directory that exists by the time anything writes it. + candidateFile = "${cfg.selfUpdate.srcDir}.candidate"; + # The wrapper an agent reaches through sudo. Constants are PREPENDED, not + # appended: this is shell, and the body acts the moment it is read. They are + # compiled into the store script rather than read from the environment + # because sudo's env_reset carries none — and a caller-supplied repo URL + # would be root building code from anywhere (issue #154 Phase 2's rule). + candidateHelper = pkgs.writeShellScript "agent-box-candidate" '' + # Box constants, prepended by the module. + CANDIDATE_FILE=${lib.escapeShellArg candidateFile} + SRC_DIR=${lib.escapeShellArg cfg.selfUpdate.srcDir} + SRC_URL=${lib.escapeShellArg "https://github.com/${cfg.selfUpdate.repo}.git"} + SRC_BRANCH=${lib.escapeShellArg (if cfg.selfUpdate.branch != null then cfg.selfUpdate.branch else "")} + SYSTEMCTL=/run/current-system/sw/bin/systemctl + UPDATE_TRIGGER=${lib.escapeShellArg updateStartCmd} + PATH=${lib.makeBinPath [ pkgs.git pkgs.coreutils pkgs.gnused pkgs.systemd ]}:$PATH + export PATH + + # agent-box-candidate — install a candidate release on THIS box, before it + # is merged and every other box gets it. + # + # The box's own fix used to have to land on master first. The only + # root-capable path an agent has is the update trigger, whose unit follows + # the tracked branch, and `git merge --ff-only` refuses everything else by + # construction — so "test it here, then merge" was not expressible and the + # fleet was the canary for its own tooling. This is the missing verb. + # + # It is a ROOT wrapper for the same reason the password helper is one: what + # writes the source tree decides what root builds, so the tree stays + # root-owned (issue #242) and an agent reaches it only through a command + # that can refuse. Every constant below is baked in by the renderer rather + # than read from the environment, because sudo scrubs the environment + # (env_reset) and a wrapper that took its target repo from the caller would + # hand root an arbitrary URL to build from. + # + # Three things it will not do, which is most of the point: + # + # * Anything but a BRANCH on this box's own configured origin. Not a + # tag, not a sha, and above all not refs/pull/N/head: on a public repo + # anyone can open a pull request, and a fetchable PR ref would mean a + # stranger's unreviewed code built as root. A branch needs push access + # to the source repo, and whoever has that can already merge. + # * A candidate that is not strictly ahead of the tracked branch. That is + # what stops an old, possibly vulnerable rev being replayed as a + # "candidate", which is the one attack the fast-forward guard exists to + # refuse and which the force this feature needs would otherwise reopen. + # The rev-equality half is here; ANCESTRY is agent-box-source's, at the + # moment of the move, because that is where the objects are and a + # question must not fetch into the tree to answer itself. + # * A permanent divergence. Nothing here pins the box: the marker it + # writes is consumed by the very next update, and an update with no + # marker always heads for the tracked branch (forcing only when the box + # is off it, since a squash-merged candidate is never an ancestor of + # master). So a candidate survives exactly until the next update, and a + # box that is forgotten converges on the fleet instead of drifting from + # it. + # + # Usage, all idempotent: + # + # agent-box-candidate BRANCH install that branch and re-apply + # agent-box-candidate --reset go back to the tracked branch now + # agent-box-candidate --status what the box is running, and off what + # + # The renderer prepends: CANDIDATE_FILE, SRC_DIR, SRC_URL, SRC_BRANCH, + # UPDATE_TRIGGER, SYSTEMCTL. + set -u + + prog=agent-box-candidate + say() { printf '%s: %s\n' "$prog" "$*" >&2; } + die() { say "$@"; exit 1; } + + # Same hardening as agent-box-source, and for the same reason: every git + # below runs as root against a tree fetched from the network, and a + # credential prompt has nobody to answer it. + export GIT_TERMINAL_PROMPT=0 + unset GIT_ASKPASS SSH_ASKPASS + export GIT_HTTP_LOW_SPEED_LIMIT=1000 GIT_HTTP_LOW_SPEED_TIME=60 + export GIT_CONFIG_COUNT=1 GIT_CONFIG_KEY_0=core.hooksPath GIT_CONFIG_VALUE_0=/dev/null + + usage() { + cat >&2 </dev/null) \ + || die "cannot reach $SRC_URL — offline?" + [ -n "$out" ] || die "origin has no branch '$b'" + else + out=$(command git ls-remote "$SRC_URL" HEAD 2>/dev/null) \ + || die "cannot reach $SRC_URL — offline?" + [ -n "$out" ] || die "$SRC_URL has no HEAD" + fi + printf '%s\n' "$out" | head -n 1 | cut -f1 + } + + running_rev() { + # The tree names the rev the box runs (the guide says so, so it has to be + # true here too). A box that has never updated has no tree yet, which is + # not an error for --status. + [ -e "$SRC_DIR/.git" ] || return 1 + command git -C "$SRC_DIR" rev-parse --verify --quiet HEAD 2>/dev/null + } + + case "''${1:-}" in + (-h|--help) + usage + exit 0 + ;; + + (--status) + rev=$(running_rev) || die "$SRC_DIR is not a checkout yet — this box has never updated" + head=$(tracked_head) || exit 1 + printf 'rev: %s\n' "$rev" + printf 'tracked: %s at %s\n' "''${SRC_BRANCH:-origin/HEAD}" "$head" + # The marker's own claim, believed only when it matches the rev the box + # actually runs — the same check agent-box-source makes before it + # relaxes the fast-forward guard, and for the same reason: a rebuild + # that failed and rolled back leaves a claim that is no longer true. + on="" + [ -e "$CANDIDATE_FILE" ] \ + && on=$(sed -n 's/^on=//p' "$CANDIDATE_FILE" 2>/dev/null | head -n 1) + if [ -n "$on" ] && [ "$on" = "$rev" ]; then + printf 'state: CANDIDATE — the next update returns to %s\n' \ + "''${SRC_BRANCH:-the tracked branch}" + elif [ "$rev" = "$head" ]; then + printf 'state: on the tracked branch\n' + elif ! command git -C "$SRC_DIR" cat-file -e "$head^{commit}" 2>/dev/null; then + # The remote has moved on and the tree has not fetched it yet, which is + # the ordinary state of a box between updates. merge-base would exit + # 128 on the missing object and fall through to the warning below, + # reporting a perfectly normal box as one the guard has a problem with. + # Say what is actually known instead: there is a newer rev, and this + # tree cannot place itself against it without a fetch — which --status, + # being a question, will not do. + printf 'state: an update is available (%.12s); this tree has not fetched it, so it cannot say more\n' "$head" + elif command git -C "$SRC_DIR" merge-base --is-ancestor "$rev" "$head" 2>/dev/null; then + printf 'state: behind the tracked branch — an update is available\n' + else + # Off the branch with nothing claiming a candidate: a rewritten + # upstream history, or a tree somebody moved by hand. NOT reported as + # a candidate, because agent-box-source will not force out of it + # either — it is the case --ff-only exists to refuse. + printf 'state: off %s, and not as a candidate — the fast-forward guard applies\n' \ + "''${SRC_BRANCH:-the tracked branch}" + fi + want="" + [ -e "$CANDIDATE_FILE" ] \ + && want=$(sed -n 's/^want=//p' "$CANDIDATE_FILE" 2>/dev/null | head -n 1) + [ -n "$want" ] && printf 'pending: %s\n' "$want" + exit 0 + ;; + + (--reset) + # An update with no QUEUED candidate already heads for the tracked + # branch, so "reset" is the ordinary update — the only work here is + # dropping a `want` that has not been consumed yet. The `on` fact must + # survive: it is what tells agent-box-source this box has a way home to + # relax the guard for, and deleting it would strand the box exactly + # where the reset was meant to rescue it from. + if [ -e "$CANDIDATE_FILE" ]; then + on=$(sed -n 's/^on=//p' "$CANDIDATE_FILE" 2>/dev/null | head -n 1) + if [ -n "$on" ]; then + printf 'on=%s\n' "$on" > "$CANDIDATE_FILE.tmp" \ + && mv "$CANDIDATE_FILE.tmp" "$CANDIDATE_FILE" + else + rm -f "$CANDIDATE_FILE" + fi + fi + say "returning to ''${SRC_BRANCH:-the tracked branch} — triggering an update" + exec $UPDATE_TRIGGER + ;; + + ("") + usage + exit 1 + ;; + + (-*) + say "unknown option '$1'" + usage + exit 1 + ;; + esac + + branch=$1 + shift + [ $# -eq 0 ] || die "one branch at a time (got extra: $*)" + + # --- validate the name ------------------------------------------------ + # + # The name reaches git and reaches a file, so it is checked before either. + # Deliberately narrower than git's own refname rules: this is an allowlist + # of what a branch in this repo actually looks like, not an attempt to + # re-implement git-check-ref-format. + case $branch in + (refs/heads/*) + die "name the branch itself: ''\'''${branch#refs/heads/}' rather than '$branch'" + ;; + (refs/*) + # refs/pull/N/head lands here, which is the whole reason this arm is + # separate: there is no branch to suggest, and suggesting one by + # stripping refs/heads/ off a path that has no such prefix printed the + # input back as its own correction. + die "'$branch' is a ref path, not a branch — only a branch of $SRC_URL can be installed" + ;; + (*..*) + die "'$branch' contains '..'" + ;; + (/*|*/) + die "'$branch' starts or ends with '/'" + ;; + (*//*) + die "'$branch' contains '//'" + ;; + (*[!A-Za-z0-9._/-]*) + die "'$branch' has characters outside A-Za-z0-9._/- — refusing" + ;; + ([!A-Za-z0-9]*) + die "'$branch' does not start with a letter or digit" + ;; + esac + [ ''${#branch} -le 200 ] || die "branch name is ''${#branch} characters — refusing" + + # --- it must be a branch on OUR origin -------------------------------- + # + # ls-remote --heads, so only refs/heads/* can match. This is the line that + # keeps refs/pull/N/head — a stranger's unreviewed pull request on a public + # repo — from ever being installed as root. + out=$(command git ls-remote --heads "$SRC_URL" "refs/heads/$branch" 2>/dev/null) \ + || die "cannot reach $SRC_URL — offline?" + [ -n "$out" ] || die "$SRC_URL has no branch '$branch' (a tag, a sha or a pull-request ref will not do)" + want=$(printf '%s\n' "$out" | head -n 1 | cut -f1) + + # --- and not the tracked branch's own head ---------------------------- + # + # The cheap half of "strictly ahead", and the only half that can be settled + # from ls-remote. ANCESTRY is checked by agent-box-source at the moment of + # the move, where the objects already are: asking here would mean fetching + # them, and a fetch is not something a refused candidate — or a mistyped + # branch name — should leave behind in the tree that decides what root + # builds. That check is also the better home for it on its own merits, since + # it then holds however the marker was written. + head=$(tracked_head) || exit 1 + if [ "$want" = "$head" ]; then + die "'$branch' is at ''${head} — the same rev as ''${SRC_BRANCH:-the tracked branch}, so there is nothing to try" + fi + + # --- record the intent and let the update do the work ----------------- + # + # The marker is intent, never state: the updater consumes it as it starts, + # so a candidate that fails to build is not retried forever, and "what is + # this box running" stays a question answered by the tree rather than by a + # file that can disagree with it. + umask 022 + # A box that has never updated has no tree and may have no state dir either, + # and the marker's parent is that dir. Create it rather than failing: the + # update this queues would create it anyway when it clones. + mkdir -p "$(dirname "$CANDIDATE_FILE")" \ + || die "could not create $(dirname "$CANDIDATE_FILE")" + # An existing `on` fact is carried over, not dropped: a box already running + # a candidate is off the tracked branch, so moving it to ANOTHER candidate + # needs the same relaxation coming home does. agent-box-source re-verifies + # the fact against the running rev before it believes it either way. + on="" + [ -e "$CANDIDATE_FILE" ] \ + && on=$(sed -n 's/^on=//p' "$CANDIDATE_FILE" 2>/dev/null | head -n 1) + # A `[ ... ] && printf` as the LAST command in the group would make the + # group's status that test's — 1 whenever there is no fact to carry — and + # the `|| die` below would then report a failure on a write that worked. + if [ -n "$on" ]; then + printf 'want=refs/heads/%s\non=%s\n' "$branch" "$on" > "$CANDIDATE_FILE.tmp" \ + || die "could not write $CANDIDATE_FILE.tmp" + else + printf 'want=refs/heads/%s\n' "$branch" > "$CANDIDATE_FILE.tmp" \ + || die "could not write $CANDIDATE_FILE.tmp" + fi + mv "$CANDIDATE_FILE.tmp" "$CANDIDATE_FILE" \ + || die "could not put $CANDIDATE_FILE in place" + say "candidate $branch ($(printf '%.12s' "$want")) queued — triggering an update" + say "this box will be off ''${SRC_BRANCH:-the tracked branch}; the next update returns to it" + exec $UPDATE_TRIGGER + ''; # Always under the maintainer's home, and the assertion below enforces it. # The agent unit runs ProtectSystem=strict with ReadWritePaths=/home/%i, # so a tree anywhere else is denied with EROFS — and the obvious repair, @@ -7216,6 +7569,13 @@ ref=''${AGENT_BOX_SRC_REF:-} # (`agentbox update --force`). Non-empty means the target replaces HEAD even # when it is not a descendant. force=''${AGENT_BOX_SRC_FORCE:-} +# Where agent-box-candidate leaves the branch it wants tried on this box +# before it is merged. Read by `pull` and CONSUMED there — it is intent, not +# state: what this box runs is the tree's own HEAD, and a marker that +# outlived its update would be a second answer able to disagree with it. +# Empty (no such wiring) is a box with the feature off, which behaves +# exactly as it did before it existed. +candidate_file=''${AGENT_BOX_CANDIDATE_FILE:-} [ -n "$dir" ] || die "AGENT_BOX_SRC_DIR is unset — there is no tree to act on" case "$dir" in @@ -7362,6 +7722,56 @@ case "''${1:-}" in (pull) [ -n "$rev" ] || die "AGENT_BOX_SRC_REV is unset — refusing to move a tree with no baseline" + # A queued candidate (agent-box-candidate BRANCH) is the target for + # exactly one update. Consumed BEFORE anything can fail, so a candidate + # that does not build is not retried on every later trigger — the next + # one goes back to the tracked branch, which is the whole of the + # "nothing here pins this box" promise. An explicit --rev wins: an + # operator naming a ref is not asking about the queue. + # Whether the TARGET was named by the caller, which decides below + # whether the fast-forward guard may relax itself. Recorded before the + # queue can overwrite $ref. + explicit_ref=$ref + # The candidate marker carries two fields, and the difference between + # them is the difference between a request and a fact: + # + # want=REF agent-box-candidate asks for REF on the next update. + # on=REV this box IS running the candidate at REV. + # + # `on` is checked against the rev the box actually runs before it is + # believed, and that check is what makes it safe to relax the + # fast-forward guard. "The running rev is not an ancestor of the tracked + # branch" is NOT a usable signal on its own: a squash-merged candidate + # and a rewritten upstream history look identical from here, and the + # second is the exact attack --ff-only exists to refuse. So a stale `on` + # (a rebuild that failed and rolled the tree back, an operator moving + # the tree by hand) is ignored rather than trusted, and the strict guard + # comes straight back. + candidate="" + on_rev="" + if [ -n "$candidate_file" ] && [ -e "$candidate_file" ]; then + on_rev=$(sed -n 's/^on=//p' "$candidate_file" 2>/dev/null | head -n 1) + if [ -z "$ref" ]; then + candidate=$(sed -n 's/^want=//p' "$candidate_file" 2>/dev/null | head -n 1) + fi + if [ -n "$candidate" ]; then + # Consumed BEFORE anything can fail, so a candidate that does not + # build is not reinstalled by every later trigger. What survives is + # only the `on` fact, which the next run re-verifies. + if [ -n "$on_rev" ]; then + printf 'on=%s\n' "$on_rev" > "$candidate_file.tmp" \ + && mv "$candidate_file.tmp" "$candidate_file" + else + rm -f "$candidate_file" + fi + say "installing candidate $candidate (queued on this box, not merged)" + ref=$candidate + fi + fi + if [ -n "$on_rev" ] && [ "$on_rev" != "$rev" ]; then + say "candidate marker names $on_rev but the box runs $rev — ignoring it" + on_rev="" + fi clone_if_missing target=$(target_rev) || exit 1 resolve_branch @@ -7384,6 +7794,36 @@ case "''${1:-}" in git checkout --quiet -B "$branch" "$rev" \ || die "could not put $dir on $branch at $rev" fi + # A candidate must be strictly AHEAD of the tracked branch, and this is + # where that is settled: the fetch above already brought both objects + # in, so it costs one ancestry query, and it holds however the marker + # came to be written. It is what keeps "candidate" from becoming a way + # to replay an older, possibly vulnerable rev — the exact thing + # --ff-only refuses and the exact thing the force below would otherwise + # reopen, since a box already off the branch takes that force path. + if [ -n "$candidate" ]; then + if ! git merge-base --is-ancestor "refs/remotes/origin/$branch" \ + "$target" 2>/dev/null; then + die "refusing candidate $candidate: $target is not ahead of $branch — rebase it, so what is tested here is what will land" + fi + fi + # Coming BACK from a candidate needs the guard relaxed, and only that. + # A candidate is squash-merged, so its head is never an ancestor of the + # tracked branch: once this box has taken one, --ff-only refuses the way + # home and the box would be stuck off-branch — the exact permanent + # divergence the feature promises not to create. + # + # Narrow on purpose. It needs the marker to say this box is running a + # candidate AND that claim to match the rev it actually runs (checked + # above), so a rewritten upstream history — which looks the same from + # the ancestry side — is still refused. The target is then either the + # tracked branch's own head or a branch agent-box-candidate already + # proved is ahead of it, so neither can be a downgrade. An explicit + # --rev keeps the strict guard and still has --force of its own. + if [ -z "$force" ] && [ -z "$explicit_ref" ] && [ -n "$on_rev" ]; then + say "the box runs candidate $rev — moving to $target without the fast-forward check" + force=1 + fi if [ "$target" = "$rev" ]; then : # nothing to move to; fall through and print the rev we are on elif [ -z "$force" ] && git merge-base --is-ancestor "$target" "$rev"; then @@ -7406,6 +7846,17 @@ case "''${1:-}" in # nothing here should be resolving conflicts in. die "refusing update: $target is not a fast-forward of the running rev $rev" fi + # Record what the box is now running, so the NEXT plain update knows it + # has a way home to relax the guard for — and stop recording it once it + # is home, so nothing is left pinned or forced afterwards. + if [ -n "$candidate_file" ]; then + if [ -n "$candidate" ]; then + printf 'on=%s\n' "$(git rev-parse HEAD)" > "$candidate_file.tmp" \ + && mv "$candidate_file.tmp" "$candidate_file" + elif [ -n "$on_rev" ]; then + rm -f "$candidate_file" + fi + fi git rev-parse HEAD ;; @@ -11612,6 +12063,24 @@ in # in the store. Verifying releases against an offline signing key is # tracked upstream (defangdevs/agent-box issue 46); until then this # trusts the pinned repo as GitHub serves it. + # One user may install a candidate release (see candidateHelper). Its own + # rule rather than a line in effectiveSudoAllowlist, for the reason the + # password helper and the reboot grant are also their own: that list is + # box-wide, and this must not become every agent user's power to restart + # every other user's sessions onto a branch of their choosing. + # + # `*` allows exactly one argument of anything, which is all the wrapper + # takes. sudoers is not the validator here and should not try to be — the + # wrapper is, and it refuses everything that is not a branch on this box's + # own origin, strictly ahead of the tracked branch. + security.sudo.extraRules = lib.optional candidateEnabled { + users = [ candidateUser ]; + commands = [{ + command = "${candidateHelper} *"; + options = [ "NOPASSWD" ]; + }]; + }; + systemd.services.agent-box-update = { description = "Fast-forward agent-box to upstream HEAD and rebuild"; # No wantedBy — on-demand only, via the agents' sudo rule (or root). @@ -11631,6 +12100,10 @@ in AGENT_BOX_SRC_DIR = cfg.selfUpdate.srcDir; AGENT_BOX_SRC_URL = "https://github.com/${cfg.selfUpdate.repo}.git"; AGENT_BOX_SRC_REV = cfg.selfUpdate.rev; + # A candidate agent-box-candidate queued, if any. agent-box-source + # CONSUMES it: this update installs it, and the next one goes back to + # the tracked branch, so a candidate never becomes a pin. + AGENT_BOX_CANDIDATE_FILE = candidateFile; # git reads a config file relative to HOME and warns (or, with some # safe.directory setups, refuses) without one. The unit inherits no # HOME of its own. diff --git a/modules/agent-box.nix.in b/modules/agent-box.nix.in index 8fbca900..ae2caeed 100644 --- a/modules/agent-box.nix.in +++ b/modules/agent-box.nix.in @@ -60,6 +60,34 @@ let rebuild that failed and could not put the tree back, which the wall notice says - it reads ahead of the running system. + '' + lib.optionalString candidateEnabled '' + ## Trying YOUR fix on this box, before every other box takes it + + An update follows the tracked branch, so a fix used to have to be + merged before this box could run it - which made the whole fleet the + canary for its own tooling. Push the branch and try it here first: + + sudo ${candidateHelper} BRANCH # install it, then re-apply + sudo ${candidateHelper} --status # what is this box running? + sudo ${candidateHelper} --reset # go back to the tracked branch + + Same restart as an ordinary update, so save context first. Then merge + once you have SEEN it work, and the fleet takes a change that has run + somewhere. + + Three things it refuses, all deliberately. Only a BRANCH on this + box's own origin - not a tag, not a sha, and not `refs/pull/N/head`, + because a public repo takes pull requests from anyone and that would + be a stranger's unreviewed code built as root. Only a branch strictly + AHEAD of the tracked branch, so "candidate" can never become a way to + replay an older rev - rebase yours, which is also the only way what + you test here is what will land. And it never pins this box: the next + update returns to the tracked branch and says so, so a candidate you + forget converges back on the fleet instead of drifting from it. That + return needs no flag from you, and is the one case where the + fast-forward guard steps aside - a squash-merged branch is never an + ancestor of the branch it landed on, so nothing else could get home. + '' + lib.optionalString checkoutEnabled '' ## This box ships its own sources @@ -233,6 +261,46 @@ let else if names != [ ] then lib.head names else null; checkoutEnabled = checkoutMaintainer != null; + # Candidate releases: install a branch on THIS box before it is merged and + # every other box takes it. Same user the maintainer checkout goes to, and + # picked the same way, because it is the same job seen from the other end — + # that user holds the tree a fix is written in, and this is how the fix gets + # tried here first. ONE user, never all of them: an update takes every + # user's sessions down with it, which is exactly what the per-user password + # helper and the reboot grant exist to keep out of one agent's hands. + # Gated on selfUpdate.enable alone and NOT on web.enable — a box with no + # terminal still updates, and its agent still has a fix to try (the trap + # tests/memory-protection.nix exists to catch). + candidateUser = + let names = lib.attrNames cfg.users; in + if !cfg.selfUpdate.enable then null + else if cfg.users ? ${cfg.web.user} then cfg.web.user + else if names != [ ] then lib.head names + else null; + candidateEnabled = candidateUser != null; + # Beside the tree, not inside it: the tree is a checkout git rewrites, and + # a marker within it would be one `git clean` from vanishing. srcDir's own + # parent is created by agent-box-source before it clones, so this always + # lands in a directory that exists by the time anything writes it. + candidateFile = "${cfg.selfUpdate.srcDir}.candidate"; + # The wrapper an agent reaches through sudo. Constants are PREPENDED, not + # appended: this is shell, and the body acts the moment it is read. They are + # compiled into the store script rather than read from the environment + # because sudo's env_reset carries none — and a caller-supplied repo URL + # would be root building code from anywhere (issue #154 Phase 2's rule). + candidateHelper = pkgs.writeShellScript "agent-box-candidate" '' + # Box constants, prepended by the module. + CANDIDATE_FILE=${lib.escapeShellArg candidateFile} + SRC_DIR=${lib.escapeShellArg cfg.selfUpdate.srcDir} + SRC_URL=${lib.escapeShellArg "https://github.com/${cfg.selfUpdate.repo}.git"} + SRC_BRANCH=${lib.escapeShellArg (if cfg.selfUpdate.branch != null then cfg.selfUpdate.branch else "")} + SYSTEMCTL=/run/current-system/sw/bin/systemctl + UPDATE_TRIGGER=${lib.escapeShellArg updateStartCmd} + PATH=${lib.makeBinPath [ pkgs.git pkgs.coreutils pkgs.gnused pkgs.systemd ]}:$PATH + export PATH + + @@include:src/candidate.sh@@ + ''; # Always under the maintainer's home, and the assertion below enforces it. # The agent unit runs ProtectSystem=strict with ReadWritePaths=/home/%i, # so a tree anywhere else is denied with EROFS — and the obvious repair, @@ -2891,6 +2959,24 @@ in # in the store. Verifying releases against an offline signing key is # tracked upstream (defangdevs/agent-box issue 46); until then this # trusts the pinned repo as GitHub serves it. + # One user may install a candidate release (see candidateHelper). Its own + # rule rather than a line in effectiveSudoAllowlist, for the reason the + # password helper and the reboot grant are also their own: that list is + # box-wide, and this must not become every agent user's power to restart + # every other user's sessions onto a branch of their choosing. + # + # `*` allows exactly one argument of anything, which is all the wrapper + # takes. sudoers is not the validator here and should not try to be — the + # wrapper is, and it refuses everything that is not a branch on this box's + # own origin, strictly ahead of the tracked branch. + security.sudo.extraRules = lib.optional candidateEnabled { + users = [ candidateUser ]; + commands = [{ + command = "${candidateHelper} *"; + options = [ "NOPASSWD" ]; + }]; + }; + systemd.services.agent-box-update = { description = "Fast-forward agent-box to upstream HEAD and rebuild"; # No wantedBy — on-demand only, via the agents' sudo rule (or root). @@ -2910,6 +2996,10 @@ in AGENT_BOX_SRC_DIR = cfg.selfUpdate.srcDir; AGENT_BOX_SRC_URL = "https://github.com/${cfg.selfUpdate.repo}.git"; AGENT_BOX_SRC_REV = cfg.selfUpdate.rev; + # A candidate agent-box-candidate queued, if any. agent-box-source + # CONSUMES it: this update installs it, and the next one goes back to + # the tracked branch, so a candidate never becomes a pin. + AGENT_BOX_CANDIDATE_FILE = candidateFile; # git reads a config file relative to HOME and warns (or, with some # safe.directory setups, refuses) without one. The unit inherits no # HOME of its own. diff --git a/modules/src/candidate.sh b/modules/src/candidate.sh new file mode 100644 index 00000000..aa0f5c8e --- /dev/null +++ b/modules/src/candidate.sh @@ -0,0 +1,286 @@ +# agent-box-candidate — install a candidate release on THIS box, before it +# is merged and every other box gets it. +# +# The box's own fix used to have to land on master first. The only +# root-capable path an agent has is the update trigger, whose unit follows +# the tracked branch, and `git merge --ff-only` refuses everything else by +# construction — so "test it here, then merge" was not expressible and the +# fleet was the canary for its own tooling. This is the missing verb. +# +# It is a ROOT wrapper for the same reason the password helper is one: what +# writes the source tree decides what root builds, so the tree stays +# root-owned (issue #242) and an agent reaches it only through a command +# that can refuse. Every constant below is baked in by the renderer rather +# than read from the environment, because sudo scrubs the environment +# (env_reset) and a wrapper that took its target repo from the caller would +# hand root an arbitrary URL to build from. +# +# Three things it will not do, which is most of the point: +# +# * Anything but a BRANCH on this box's own configured origin. Not a +# tag, not a sha, and above all not refs/pull/N/head: on a public repo +# anyone can open a pull request, and a fetchable PR ref would mean a +# stranger's unreviewed code built as root. A branch needs push access +# to the source repo, and whoever has that can already merge. +# * A candidate that is not strictly ahead of the tracked branch. That is +# what stops an old, possibly vulnerable rev being replayed as a +# "candidate", which is the one attack the fast-forward guard exists to +# refuse and which the force this feature needs would otherwise reopen. +# The rev-equality half is here; ANCESTRY is agent-box-source's, at the +# moment of the move, because that is where the objects are and a +# question must not fetch into the tree to answer itself. +# * A permanent divergence. Nothing here pins the box: the marker it +# writes is consumed by the very next update, and an update with no +# marker always heads for the tracked branch (forcing only when the box +# is off it, since a squash-merged candidate is never an ancestor of +# master). So a candidate survives exactly until the next update, and a +# box that is forgotten converges on the fleet instead of drifting from +# it. +# +# Usage, all idempotent: +# +# agent-box-candidate BRANCH install that branch and re-apply +# agent-box-candidate --reset go back to the tracked branch now +# agent-box-candidate --status what the box is running, and off what +# +# The renderer prepends: CANDIDATE_FILE, SRC_DIR, SRC_URL, SRC_BRANCH, +# UPDATE_TRIGGER, SYSTEMCTL. +set -u + +prog=agent-box-candidate +say() { printf '%s: %s\n' "$prog" "$*" >&2; } +die() { say "$@"; exit 1; } + +# Same hardening as agent-box-source, and for the same reason: every git +# below runs as root against a tree fetched from the network, and a +# credential prompt has nobody to answer it. +export GIT_TERMINAL_PROMPT=0 +unset GIT_ASKPASS SSH_ASKPASS +export GIT_HTTP_LOW_SPEED_LIMIT=1000 GIT_HTTP_LOW_SPEED_TIME=60 +export GIT_CONFIG_COUNT=1 GIT_CONFIG_KEY_0=core.hooksPath GIT_CONFIG_VALUE_0=/dev/null + +usage() { + cat >&2 </dev/null) \ + || die "cannot reach $SRC_URL — offline?" + [ -n "$out" ] || die "origin has no branch '$b'" + else + out=$(command git ls-remote "$SRC_URL" HEAD 2>/dev/null) \ + || die "cannot reach $SRC_URL — offline?" + [ -n "$out" ] || die "$SRC_URL has no HEAD" + fi + printf '%s\n' "$out" | head -n 1 | cut -f1 +} + +running_rev() { + # The tree names the rev the box runs (the guide says so, so it has to be + # true here too). A box that has never updated has no tree yet, which is + # not an error for --status. + [ -e "$SRC_DIR/.git" ] || return 1 + command git -C "$SRC_DIR" rev-parse --verify --quiet HEAD 2>/dev/null +} + +case "${1:-}" in + (-h|--help) + usage + exit 0 + ;; + + (--status) + rev=$(running_rev) || die "$SRC_DIR is not a checkout yet — this box has never updated" + head=$(tracked_head) || exit 1 + printf 'rev: %s\n' "$rev" + printf 'tracked: %s at %s\n' "${SRC_BRANCH:-origin/HEAD}" "$head" + # The marker's own claim, believed only when it matches the rev the box + # actually runs — the same check agent-box-source makes before it + # relaxes the fast-forward guard, and for the same reason: a rebuild + # that failed and rolled back leaves a claim that is no longer true. + on="" + [ -e "$CANDIDATE_FILE" ] \ + && on=$(sed -n 's/^on=//p' "$CANDIDATE_FILE" 2>/dev/null | head -n 1) + if [ -n "$on" ] && [ "$on" = "$rev" ]; then + printf 'state: CANDIDATE — the next update returns to %s\n' \ + "${SRC_BRANCH:-the tracked branch}" + elif [ "$rev" = "$head" ]; then + printf 'state: on the tracked branch\n' + elif ! command git -C "$SRC_DIR" cat-file -e "$head^{commit}" 2>/dev/null; then + # The remote has moved on and the tree has not fetched it yet, which is + # the ordinary state of a box between updates. merge-base would exit + # 128 on the missing object and fall through to the warning below, + # reporting a perfectly normal box as one the guard has a problem with. + # Say what is actually known instead: there is a newer rev, and this + # tree cannot place itself against it without a fetch — which --status, + # being a question, will not do. + printf 'state: an update is available (%.12s); this tree has not fetched it, so it cannot say more\n' "$head" + elif command git -C "$SRC_DIR" merge-base --is-ancestor "$rev" "$head" 2>/dev/null; then + printf 'state: behind the tracked branch — an update is available\n' + else + # Off the branch with nothing claiming a candidate: a rewritten + # upstream history, or a tree somebody moved by hand. NOT reported as + # a candidate, because agent-box-source will not force out of it + # either — it is the case --ff-only exists to refuse. + printf 'state: off %s, and not as a candidate — the fast-forward guard applies\n' \ + "${SRC_BRANCH:-the tracked branch}" + fi + want="" + [ -e "$CANDIDATE_FILE" ] \ + && want=$(sed -n 's/^want=//p' "$CANDIDATE_FILE" 2>/dev/null | head -n 1) + [ -n "$want" ] && printf 'pending: %s\n' "$want" + exit 0 + ;; + + (--reset) + # An update with no QUEUED candidate already heads for the tracked + # branch, so "reset" is the ordinary update — the only work here is + # dropping a `want` that has not been consumed yet. The `on` fact must + # survive: it is what tells agent-box-source this box has a way home to + # relax the guard for, and deleting it would strand the box exactly + # where the reset was meant to rescue it from. + if [ -e "$CANDIDATE_FILE" ]; then + on=$(sed -n 's/^on=//p' "$CANDIDATE_FILE" 2>/dev/null | head -n 1) + if [ -n "$on" ]; then + printf 'on=%s\n' "$on" > "$CANDIDATE_FILE.tmp" \ + && mv "$CANDIDATE_FILE.tmp" "$CANDIDATE_FILE" + else + rm -f "$CANDIDATE_FILE" + fi + fi + say "returning to ${SRC_BRANCH:-the tracked branch} — triggering an update" + exec $UPDATE_TRIGGER + ;; + + ("") + usage + exit 1 + ;; + + (-*) + say "unknown option '$1'" + usage + exit 1 + ;; +esac + +branch=$1 +shift +[ $# -eq 0 ] || die "one branch at a time (got extra: $*)" + +# --- validate the name ------------------------------------------------ +# +# The name reaches git and reaches a file, so it is checked before either. +# Deliberately narrower than git's own refname rules: this is an allowlist +# of what a branch in this repo actually looks like, not an attempt to +# re-implement git-check-ref-format. +case $branch in + (refs/heads/*) + die "name the branch itself: '${branch#refs/heads/}' rather than '$branch'" + ;; + (refs/*) + # refs/pull/N/head lands here, which is the whole reason this arm is + # separate: there is no branch to suggest, and suggesting one by + # stripping refs/heads/ off a path that has no such prefix printed the + # input back as its own correction. + die "'$branch' is a ref path, not a branch — only a branch of $SRC_URL can be installed" + ;; + (*..*) + die "'$branch' contains '..'" + ;; + (/*|*/) + die "'$branch' starts or ends with '/'" + ;; + (*//*) + die "'$branch' contains '//'" + ;; + (*[!A-Za-z0-9._/-]*) + die "'$branch' has characters outside A-Za-z0-9._/- — refusing" + ;; + ([!A-Za-z0-9]*) + die "'$branch' does not start with a letter or digit" + ;; +esac +[ ${#branch} -le 200 ] || die "branch name is ${#branch} characters — refusing" + +# --- it must be a branch on OUR origin -------------------------------- +# +# ls-remote --heads, so only refs/heads/* can match. This is the line that +# keeps refs/pull/N/head — a stranger's unreviewed pull request on a public +# repo — from ever being installed as root. +out=$(command git ls-remote --heads "$SRC_URL" "refs/heads/$branch" 2>/dev/null) \ + || die "cannot reach $SRC_URL — offline?" +[ -n "$out" ] || die "$SRC_URL has no branch '$branch' (a tag, a sha or a pull-request ref will not do)" +want=$(printf '%s\n' "$out" | head -n 1 | cut -f1) + +# --- and not the tracked branch's own head ---------------------------- +# +# The cheap half of "strictly ahead", and the only half that can be settled +# from ls-remote. ANCESTRY is checked by agent-box-source at the moment of +# the move, where the objects already are: asking here would mean fetching +# them, and a fetch is not something a refused candidate — or a mistyped +# branch name — should leave behind in the tree that decides what root +# builds. That check is also the better home for it on its own merits, since +# it then holds however the marker was written. +head=$(tracked_head) || exit 1 +if [ "$want" = "$head" ]; then + die "'$branch' is at ${head} — the same rev as ${SRC_BRANCH:-the tracked branch}, so there is nothing to try" +fi + +# --- record the intent and let the update do the work ----------------- +# +# The marker is intent, never state: the updater consumes it as it starts, +# so a candidate that fails to build is not retried forever, and "what is +# this box running" stays a question answered by the tree rather than by a +# file that can disagree with it. +umask 022 +# A box that has never updated has no tree and may have no state dir either, +# and the marker's parent is that dir. Create it rather than failing: the +# update this queues would create it anyway when it clones. +mkdir -p "$(dirname "$CANDIDATE_FILE")" \ + || die "could not create $(dirname "$CANDIDATE_FILE")" +# An existing `on` fact is carried over, not dropped: a box already running +# a candidate is off the tracked branch, so moving it to ANOTHER candidate +# needs the same relaxation coming home does. agent-box-source re-verifies +# the fact against the running rev before it believes it either way. +on="" +[ -e "$CANDIDATE_FILE" ] \ + && on=$(sed -n 's/^on=//p' "$CANDIDATE_FILE" 2>/dev/null | head -n 1) +# A `[ ... ] && printf` as the LAST command in the group would make the +# group's status that test's — 1 whenever there is no fact to carry — and +# the `|| die` below would then report a failure on a write that worked. +if [ -n "$on" ]; then + printf 'want=refs/heads/%s\non=%s\n' "$branch" "$on" > "$CANDIDATE_FILE.tmp" \ + || die "could not write $CANDIDATE_FILE.tmp" +else + printf 'want=refs/heads/%s\n' "$branch" > "$CANDIDATE_FILE.tmp" \ + || die "could not write $CANDIDATE_FILE.tmp" +fi +mv "$CANDIDATE_FILE.tmp" "$CANDIDATE_FILE" \ + || die "could not put $CANDIDATE_FILE in place" +say "candidate $branch ($(printf '%.12s' "$want")) queued — triggering an update" +say "this box will be off ${SRC_BRANCH:-the tracked branch}; the next update returns to it" +exec $UPDATE_TRIGGER diff --git a/modules/src/source-tree.sh b/modules/src/source-tree.sh index 0d9ab39b..3d777ab1 100644 --- a/modules/src/source-tree.sh +++ b/modules/src/source-tree.sh @@ -46,6 +46,13 @@ ref=${AGENT_BOX_SRC_REF:-} # (`agentbox update --force`). Non-empty means the target replaces HEAD even # when it is not a descendant. force=${AGENT_BOX_SRC_FORCE:-} +# Where agent-box-candidate leaves the branch it wants tried on this box +# before it is merged. Read by `pull` and CONSUMED there — it is intent, not +# state: what this box runs is the tree's own HEAD, and a marker that +# outlived its update would be a second answer able to disagree with it. +# Empty (no such wiring) is a box with the feature off, which behaves +# exactly as it did before it existed. +candidate_file=${AGENT_BOX_CANDIDATE_FILE:-} [ -n "$dir" ] || die "AGENT_BOX_SRC_DIR is unset — there is no tree to act on" case "$dir" in @@ -192,6 +199,56 @@ case "${1:-}" in (pull) [ -n "$rev" ] || die "AGENT_BOX_SRC_REV is unset — refusing to move a tree with no baseline" + # A queued candidate (agent-box-candidate BRANCH) is the target for + # exactly one update. Consumed BEFORE anything can fail, so a candidate + # that does not build is not retried on every later trigger — the next + # one goes back to the tracked branch, which is the whole of the + # "nothing here pins this box" promise. An explicit --rev wins: an + # operator naming a ref is not asking about the queue. + # Whether the TARGET was named by the caller, which decides below + # whether the fast-forward guard may relax itself. Recorded before the + # queue can overwrite $ref. + explicit_ref=$ref + # The candidate marker carries two fields, and the difference between + # them is the difference between a request and a fact: + # + # want=REF agent-box-candidate asks for REF on the next update. + # on=REV this box IS running the candidate at REV. + # + # `on` is checked against the rev the box actually runs before it is + # believed, and that check is what makes it safe to relax the + # fast-forward guard. "The running rev is not an ancestor of the tracked + # branch" is NOT a usable signal on its own: a squash-merged candidate + # and a rewritten upstream history look identical from here, and the + # second is the exact attack --ff-only exists to refuse. So a stale `on` + # (a rebuild that failed and rolled the tree back, an operator moving + # the tree by hand) is ignored rather than trusted, and the strict guard + # comes straight back. + candidate="" + on_rev="" + if [ -n "$candidate_file" ] && [ -e "$candidate_file" ]; then + on_rev=$(sed -n 's/^on=//p' "$candidate_file" 2>/dev/null | head -n 1) + if [ -z "$ref" ]; then + candidate=$(sed -n 's/^want=//p' "$candidate_file" 2>/dev/null | head -n 1) + fi + if [ -n "$candidate" ]; then + # Consumed BEFORE anything can fail, so a candidate that does not + # build is not reinstalled by every later trigger. What survives is + # only the `on` fact, which the next run re-verifies. + if [ -n "$on_rev" ]; then + printf 'on=%s\n' "$on_rev" > "$candidate_file.tmp" \ + && mv "$candidate_file.tmp" "$candidate_file" + else + rm -f "$candidate_file" + fi + say "installing candidate $candidate (queued on this box, not merged)" + ref=$candidate + fi + fi + if [ -n "$on_rev" ] && [ "$on_rev" != "$rev" ]; then + say "candidate marker names $on_rev but the box runs $rev — ignoring it" + on_rev="" + fi clone_if_missing target=$(target_rev) || exit 1 resolve_branch @@ -214,6 +271,36 @@ case "${1:-}" in git checkout --quiet -B "$branch" "$rev" \ || die "could not put $dir on $branch at $rev" fi + # A candidate must be strictly AHEAD of the tracked branch, and this is + # where that is settled: the fetch above already brought both objects + # in, so it costs one ancestry query, and it holds however the marker + # came to be written. It is what keeps "candidate" from becoming a way + # to replay an older, possibly vulnerable rev — the exact thing + # --ff-only refuses and the exact thing the force below would otherwise + # reopen, since a box already off the branch takes that force path. + if [ -n "$candidate" ]; then + if ! git merge-base --is-ancestor "refs/remotes/origin/$branch" \ + "$target" 2>/dev/null; then + die "refusing candidate $candidate: $target is not ahead of $branch — rebase it, so what is tested here is what will land" + fi + fi + # Coming BACK from a candidate needs the guard relaxed, and only that. + # A candidate is squash-merged, so its head is never an ancestor of the + # tracked branch: once this box has taken one, --ff-only refuses the way + # home and the box would be stuck off-branch — the exact permanent + # divergence the feature promises not to create. + # + # Narrow on purpose. It needs the marker to say this box is running a + # candidate AND that claim to match the rev it actually runs (checked + # above), so a rewritten upstream history — which looks the same from + # the ancestry side — is still refused. The target is then either the + # tracked branch's own head or a branch agent-box-candidate already + # proved is ahead of it, so neither can be a downgrade. An explicit + # --rev keeps the strict guard and still has --force of its own. + if [ -z "$force" ] && [ -z "$explicit_ref" ] && [ -n "$on_rev" ]; then + say "the box runs candidate $rev — moving to $target without the fast-forward check" + force=1 + fi if [ "$target" = "$rev" ]; then : # nothing to move to; fall through and print the rev we are on elif [ -z "$force" ] && git merge-base --is-ancestor "$target" "$rev"; then @@ -236,6 +323,17 @@ case "${1:-}" in # nothing here should be resolving conflicts in. die "refusing update: $target is not a fast-forward of the running rev $rev" fi + # Record what the box is now running, so the NEXT plain update knows it + # has a way home to relax the guard for — and stop recording it once it + # is home, so nothing is left pinned or forced afterwards. + if [ -n "$candidate_file" ]; then + if [ -n "$candidate" ]; then + printf 'on=%s\n' "$(git rev-parse HEAD)" > "$candidate_file.tmp" \ + && mv "$candidate_file.tmp" "$candidate_file" + elif [ -n "$on_rev" ]; then + rm -f "$candidate_file" + fi + fi git rev-parse HEAD ;; diff --git a/nix/runtime.nix b/nix/runtime.nix index f75e6ee7..25634381 100644 --- a/nix/runtime.nix +++ b/nix/runtime.nix @@ -339,6 +339,10 @@ let # crosses sudo, so its paths must not come from env); the template it # renders from lives here. install -m444 ${src}/password-helper.py $out/libexec/agent-box/ + # agent-box-candidate, same story: `agentbox apply` renders it per box + # with the repo, tree and trigger compiled in, because it crosses sudo + # and sudo carries no environment. The module embeds this same file. + install -m444 ${src}/candidate.sh $out/libexec/agent-box/ ''; in pkgs.buildEnv { diff --git a/scripts/check_backend_parity.py b/scripts/check_backend_parity.py index 29f3ee29..782a2107 100644 --- a/scripts/check_backend_parity.py +++ b/scripts/check_backend_parity.py @@ -214,6 +214,11 @@ "AGENT_BOX_SRC_DIR": ("module", SRC_TREE_BINDING), "AGENT_BOX_SRC_URL": ("module", SRC_TREE_BINDING), "AGENT_BOX_SRC_REV": ("module", SRC_TREE_BINDING), + # The candidate marker, consumed by that same payload and bound the same + # way for the same reason. Both backends DO supply it — native in + # source_tree()'s in-process env, beside the AGENT_BOX_SRC_* names above + # — so this is a unit-line divergence and not a missing capability. + "AGENT_BOX_CANDIDATE_FILE": ("module", SRC_TREE_BINDING), } # One reason, cited by the four entries below, so it has one home rather diff --git a/scripts/check_one_spec.py b/scripts/check_one_spec.py index a086a973..7c6b23a0 100644 --- a/scripts/check_one_spec.py +++ b/scripts/check_one_spec.py @@ -61,6 +61,14 @@ (re.compile(r"(/nix/store/e+-agent-box-password-(\w+)/bin/|" r"/etc/agent-box/bin/)agent-box-password-"), "agent-box-password-"), + # agent-box-candidate, same story: the module grants its store script + # (writeShellScript, so a file and not a dir with bin/), native grants + # the copy `agentbox apply` renders into /etc. Both are the SAME grant + # of the same one-argument wrapper — a substrate spelling, not a + # difference in what the box allows. + (re.compile(r"/nix/store/e+-agent-box-candidate|" + r"/etc/agent-box/bin/agent-box-candidate"), + "agent-box-candidate"), ] # Divergences that are CORRECT. Each needs a reason a reader can check. diff --git a/tests/golden/web/etc/agent-box-guides/AGENTS.agent.md b/tests/golden/web/etc/agent-box-guides/AGENTS.agent.md index ae3fe6ae..59726bb8 100644 --- a/tests/golden/web/etc/agent-box-guides/AGENTS.agent.md +++ b/tests/golden/web/etc/agent-box-guides/AGENTS.agent.md @@ -473,6 +473,33 @@ FIRST and the rebuild follows it, so between the two - and after a rebuild that failed and could not put the tree back, which the wall notice says - it reads ahead of the running system. +## Trying YOUR fix on this box, before every other box takes it + +An update follows the tracked branch, so a fix used to have to be +merged before this box could run it - which made the whole fleet the +canary for its own tooling. Push the branch and try it here first: + + sudo /nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-agent-box-candidate BRANCH # install it, then re-apply + sudo /nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-agent-box-candidate --status # what is this box running? + sudo /nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-agent-box-candidate --reset # go back to the tracked branch + +Same restart as an ordinary update, so save context first. Then merge +once you have SEEN it work, and the fleet takes a change that has run +somewhere. + +Three things it refuses, all deliberately. Only a BRANCH on this +box's own origin - not a tag, not a sha, and not `refs/pull/N/head`, +because a public repo takes pull requests from anyone and that would +be a stranger's unreviewed code built as root. Only a branch strictly +AHEAD of the tracked branch, so "candidate" can never become a way to +replay an older rev - rebase yours, which is also the only way what +you test here is what will land. And it never pins this box: the next +update returns to the tracked branch and says so, so a candidate you +forget converges back on the fleet instead of drifting from it. That +return needs no flag from you, and is the one case where the +fast-forward guard steps aside - a squash-merged branch is never an +ancestor of the branch it landed on, so nothing else could get home. + ## This box ships its own sources A checkout of the repo this box is BUILT from lives at diff --git a/tests/golden/web/etc/sudoers b/tests/golden/web/etc/sudoers index 86c4bceb..8c67f477 100644 --- a/tests/golden/web/etc/sudoers +++ b/tests/golden/web/etc/sudoers @@ -5,6 +5,7 @@ root ALL=(ALL:ALL) SETENV: ALL %wheel ALL=(ALL:ALL) SETENV: ALL agent ALL=(ALL:ALL) NOPASSWD: /run/current-system/sw/bin/systemctl reload caddy.service, NOPASSWD: /run/current-system/sw/bin/systemctl reload caddy.service, NOPASSWD: /run/current-system/sw/bin/systemctl start --no-block agent-box-update.service robot ALL=(ALL:ALL) NOPASSWD: /run/current-system/sw/bin/systemctl reload caddy.service, NOPASSWD: /run/current-system/sw/bin/systemctl reload caddy.service, NOPASSWD: /run/current-system/sw/bin/systemctl start --no-block agent-box-update.service +agent ALL=(ALL:ALL) NOPASSWD: /nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-agent-box-candidate * agent ALL=(ALL:ALL) NOPASSWD: /nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-agent-box-password-agent/bin/agent-box-password-agent "" robot ALL=(ALL:ALL) NOPASSWD: /nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-agent-box-password-robot/bin/agent-box-password-robot "" agent ALL=(ALL:ALL) NOPASSWD: /run/current-system/sw/bin/systemctl reboot --no-block diff --git a/tests/golden/web/payloads/agent-box-candidate b/tests/golden/web/payloads/agent-box-candidate new file mode 100644 index 00000000..e0163ee9 --- /dev/null +++ b/tests/golden/web/payloads/agent-box-candidate @@ -0,0 +1,298 @@ +#!/nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-bash-5.3p9/bin/bash +# Box constants, prepended by the module. +CANDIDATE_FILE=/var/lib/agent-box/src.candidate +SRC_DIR=/var/lib/agent-box/src +SRC_URL=https://github.com/defangdevs/agent-box.git +SRC_BRANCH='' +SYSTEMCTL=/run/current-system/sw/bin/systemctl +UPDATE_TRIGGER='/run/current-system/sw/bin/systemctl start --no-block agent-box-update.service' +PATH=/nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-git-2.54.0/bin:/nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-coreutils-9.11/bin:/nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-gnused-4.10/bin:/nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-systemd-261/bin:$PATH +export PATH + +# agent-box-candidate — install a candidate release on THIS box, before it +# is merged and every other box gets it. +# +# The box's own fix used to have to land on master first. The only +# root-capable path an agent has is the update trigger, whose unit follows +# the tracked branch, and `git merge --ff-only` refuses everything else by +# construction — so "test it here, then merge" was not expressible and the +# fleet was the canary for its own tooling. This is the missing verb. +# +# It is a ROOT wrapper for the same reason the password helper is one: what +# writes the source tree decides what root builds, so the tree stays +# root-owned (issue #242) and an agent reaches it only through a command +# that can refuse. Every constant below is baked in by the renderer rather +# than read from the environment, because sudo scrubs the environment +# (env_reset) and a wrapper that took its target repo from the caller would +# hand root an arbitrary URL to build from. +# +# Three things it will not do, which is most of the point: +# +# * Anything but a BRANCH on this box's own configured origin. Not a +# tag, not a sha, and above all not refs/pull/N/head: on a public repo +# anyone can open a pull request, and a fetchable PR ref would mean a +# stranger's unreviewed code built as root. A branch needs push access +# to the source repo, and whoever has that can already merge. +# * A candidate that is not strictly ahead of the tracked branch. That is +# what stops an old, possibly vulnerable rev being replayed as a +# "candidate", which is the one attack the fast-forward guard exists to +# refuse and which the force this feature needs would otherwise reopen. +# The rev-equality half is here; ANCESTRY is agent-box-source's, at the +# moment of the move, because that is where the objects are and a +# question must not fetch into the tree to answer itself. +# * A permanent divergence. Nothing here pins the box: the marker it +# writes is consumed by the very next update, and an update with no +# marker always heads for the tracked branch (forcing only when the box +# is off it, since a squash-merged candidate is never an ancestor of +# master). So a candidate survives exactly until the next update, and a +# box that is forgotten converges on the fleet instead of drifting from +# it. +# +# Usage, all idempotent: +# +# agent-box-candidate BRANCH install that branch and re-apply +# agent-box-candidate --reset go back to the tracked branch now +# agent-box-candidate --status what the box is running, and off what +# +# The renderer prepends: CANDIDATE_FILE, SRC_DIR, SRC_URL, SRC_BRANCH, +# UPDATE_TRIGGER, SYSTEMCTL. +set -u + +prog=agent-box-candidate +say() { printf '%s: %s\n' "$prog" "$*" >&2; } +die() { say "$@"; exit 1; } + +# Same hardening as agent-box-source, and for the same reason: every git +# below runs as root against a tree fetched from the network, and a +# credential prompt has nobody to answer it. +export GIT_TERMINAL_PROMPT=0 +unset GIT_ASKPASS SSH_ASKPASS +export GIT_HTTP_LOW_SPEED_LIMIT=1000 GIT_HTTP_LOW_SPEED_TIME=60 +export GIT_CONFIG_COUNT=1 GIT_CONFIG_KEY_0=core.hooksPath GIT_CONFIG_VALUE_0=/dev/null + +usage() { + cat >&2 </dev/null) \ + || die "cannot reach $SRC_URL — offline?" + [ -n "$out" ] || die "origin has no branch '$b'" + else + out=$(command git ls-remote "$SRC_URL" HEAD 2>/dev/null) \ + || die "cannot reach $SRC_URL — offline?" + [ -n "$out" ] || die "$SRC_URL has no HEAD" + fi + printf '%s\n' "$out" | head -n 1 | cut -f1 +} + +running_rev() { + # The tree names the rev the box runs (the guide says so, so it has to be + # true here too). A box that has never updated has no tree yet, which is + # not an error for --status. + [ -e "$SRC_DIR/.git" ] || return 1 + command git -C "$SRC_DIR" rev-parse --verify --quiet HEAD 2>/dev/null +} + +case "${1:-}" in + (-h|--help) + usage + exit 0 + ;; + + (--status) + rev=$(running_rev) || die "$SRC_DIR is not a checkout yet — this box has never updated" + head=$(tracked_head) || exit 1 + printf 'rev: %s\n' "$rev" + printf 'tracked: %s at %s\n' "${SRC_BRANCH:-origin/HEAD}" "$head" + # The marker's own claim, believed only when it matches the rev the box + # actually runs — the same check agent-box-source makes before it + # relaxes the fast-forward guard, and for the same reason: a rebuild + # that failed and rolled back leaves a claim that is no longer true. + on="" + [ -e "$CANDIDATE_FILE" ] \ + && on=$(sed -n 's/^on=//p' "$CANDIDATE_FILE" 2>/dev/null | head -n 1) + if [ -n "$on" ] && [ "$on" = "$rev" ]; then + printf 'state: CANDIDATE — the next update returns to %s\n' \ + "${SRC_BRANCH:-the tracked branch}" + elif [ "$rev" = "$head" ]; then + printf 'state: on the tracked branch\n' + elif ! command git -C "$SRC_DIR" cat-file -e "$head^{commit}" 2>/dev/null; then + # The remote has moved on and the tree has not fetched it yet, which is + # the ordinary state of a box between updates. merge-base would exit + # 128 on the missing object and fall through to the warning below, + # reporting a perfectly normal box as one the guard has a problem with. + # Say what is actually known instead: there is a newer rev, and this + # tree cannot place itself against it without a fetch — which --status, + # being a question, will not do. + printf 'state: an update is available (%.12s); this tree has not fetched it, so it cannot say more\n' "$head" + elif command git -C "$SRC_DIR" merge-base --is-ancestor "$rev" "$head" 2>/dev/null; then + printf 'state: behind the tracked branch — an update is available\n' + else + # Off the branch with nothing claiming a candidate: a rewritten + # upstream history, or a tree somebody moved by hand. NOT reported as + # a candidate, because agent-box-source will not force out of it + # either — it is the case --ff-only exists to refuse. + printf 'state: off %s, and not as a candidate — the fast-forward guard applies\n' \ + "${SRC_BRANCH:-the tracked branch}" + fi + want="" + [ -e "$CANDIDATE_FILE" ] \ + && want=$(sed -n 's/^want=//p' "$CANDIDATE_FILE" 2>/dev/null | head -n 1) + [ -n "$want" ] && printf 'pending: %s\n' "$want" + exit 0 + ;; + + (--reset) + # An update with no QUEUED candidate already heads for the tracked + # branch, so "reset" is the ordinary update — the only work here is + # dropping a `want` that has not been consumed yet. The `on` fact must + # survive: it is what tells agent-box-source this box has a way home to + # relax the guard for, and deleting it would strand the box exactly + # where the reset was meant to rescue it from. + if [ -e "$CANDIDATE_FILE" ]; then + on=$(sed -n 's/^on=//p' "$CANDIDATE_FILE" 2>/dev/null | head -n 1) + if [ -n "$on" ]; then + printf 'on=%s\n' "$on" > "$CANDIDATE_FILE.tmp" \ + && mv "$CANDIDATE_FILE.tmp" "$CANDIDATE_FILE" + else + rm -f "$CANDIDATE_FILE" + fi + fi + say "returning to ${SRC_BRANCH:-the tracked branch} — triggering an update" + exec $UPDATE_TRIGGER + ;; + + ("") + usage + exit 1 + ;; + + (-*) + say "unknown option '$1'" + usage + exit 1 + ;; +esac + +branch=$1 +shift +[ $# -eq 0 ] || die "one branch at a time (got extra: $*)" + +# --- validate the name ------------------------------------------------ +# +# The name reaches git and reaches a file, so it is checked before either. +# Deliberately narrower than git's own refname rules: this is an allowlist +# of what a branch in this repo actually looks like, not an attempt to +# re-implement git-check-ref-format. +case $branch in + (refs/heads/*) + die "name the branch itself: '${branch#refs/heads/}' rather than '$branch'" + ;; + (refs/*) + # refs/pull/N/head lands here, which is the whole reason this arm is + # separate: there is no branch to suggest, and suggesting one by + # stripping refs/heads/ off a path that has no such prefix printed the + # input back as its own correction. + die "'$branch' is a ref path, not a branch — only a branch of $SRC_URL can be installed" + ;; + (*..*) + die "'$branch' contains '..'" + ;; + (/*|*/) + die "'$branch' starts or ends with '/'" + ;; + (*//*) + die "'$branch' contains '//'" + ;; + (*[!A-Za-z0-9._/-]*) + die "'$branch' has characters outside A-Za-z0-9._/- — refusing" + ;; + ([!A-Za-z0-9]*) + die "'$branch' does not start with a letter or digit" + ;; +esac +[ ${#branch} -le 200 ] || die "branch name is ${#branch} characters — refusing" + +# --- it must be a branch on OUR origin -------------------------------- +# +# ls-remote --heads, so only refs/heads/* can match. This is the line that +# keeps refs/pull/N/head — a stranger's unreviewed pull request on a public +# repo — from ever being installed as root. +out=$(command git ls-remote --heads "$SRC_URL" "refs/heads/$branch" 2>/dev/null) \ + || die "cannot reach $SRC_URL — offline?" +[ -n "$out" ] || die "$SRC_URL has no branch '$branch' (a tag, a sha or a pull-request ref will not do)" +want=$(printf '%s\n' "$out" | head -n 1 | cut -f1) + +# --- and not the tracked branch's own head ---------------------------- +# +# The cheap half of "strictly ahead", and the only half that can be settled +# from ls-remote. ANCESTRY is checked by agent-box-source at the moment of +# the move, where the objects already are: asking here would mean fetching +# them, and a fetch is not something a refused candidate — or a mistyped +# branch name — should leave behind in the tree that decides what root +# builds. That check is also the better home for it on its own merits, since +# it then holds however the marker was written. +head=$(tracked_head) || exit 1 +if [ "$want" = "$head" ]; then + die "'$branch' is at ${head} — the same rev as ${SRC_BRANCH:-the tracked branch}, so there is nothing to try" +fi + +# --- record the intent and let the update do the work ----------------- +# +# The marker is intent, never state: the updater consumes it as it starts, +# so a candidate that fails to build is not retried forever, and "what is +# this box running" stays a question answered by the tree rather than by a +# file that can disagree with it. +umask 022 +# A box that has never updated has no tree and may have no state dir either, +# and the marker's parent is that dir. Create it rather than failing: the +# update this queues would create it anyway when it clones. +mkdir -p "$(dirname "$CANDIDATE_FILE")" \ + || die "could not create $(dirname "$CANDIDATE_FILE")" +# An existing `on` fact is carried over, not dropped: a box already running +# a candidate is off the tracked branch, so moving it to ANOTHER candidate +# needs the same relaxation coming home does. agent-box-source re-verifies +# the fact against the running rev before it believes it either way. +on="" +[ -e "$CANDIDATE_FILE" ] \ + && on=$(sed -n 's/^on=//p' "$CANDIDATE_FILE" 2>/dev/null | head -n 1) +# A `[ ... ] && printf` as the LAST command in the group would make the +# group's status that test's — 1 whenever there is no fact to carry — and +# the `|| die` below would then report a failure on a write that worked. +if [ -n "$on" ]; then + printf 'want=refs/heads/%s\non=%s\n' "$branch" "$on" > "$CANDIDATE_FILE.tmp" \ + || die "could not write $CANDIDATE_FILE.tmp" +else + printf 'want=refs/heads/%s\n' "$branch" > "$CANDIDATE_FILE.tmp" \ + || die "could not write $CANDIDATE_FILE.tmp" +fi +mv "$CANDIDATE_FILE.tmp" "$CANDIDATE_FILE" \ + || die "could not put $CANDIDATE_FILE in place" +say "candidate $branch ($(printf '%.12s' "$want")) queued — triggering an update" +say "this box will be off ${SRC_BRANCH:-the tracked branch}; the next update returns to it" +exec $UPDATE_TRIGGER + diff --git a/tests/golden/web/payloads/agent-box-source/bin/agent-box-source b/tests/golden/web/payloads/agent-box-source/bin/agent-box-source index 7e84cf26..70db3c24 100644 --- a/tests/golden/web/payloads/agent-box-source/bin/agent-box-source +++ b/tests/golden/web/payloads/agent-box-source/bin/agent-box-source @@ -47,6 +47,13 @@ ref=${AGENT_BOX_SRC_REF:-} # (`agentbox update --force`). Non-empty means the target replaces HEAD even # when it is not a descendant. force=${AGENT_BOX_SRC_FORCE:-} +# Where agent-box-candidate leaves the branch it wants tried on this box +# before it is merged. Read by `pull` and CONSUMED there — it is intent, not +# state: what this box runs is the tree's own HEAD, and a marker that +# outlived its update would be a second answer able to disagree with it. +# Empty (no such wiring) is a box with the feature off, which behaves +# exactly as it did before it existed. +candidate_file=${AGENT_BOX_CANDIDATE_FILE:-} [ -n "$dir" ] || die "AGENT_BOX_SRC_DIR is unset — there is no tree to act on" case "$dir" in @@ -193,6 +200,56 @@ case "${1:-}" in (pull) [ -n "$rev" ] || die "AGENT_BOX_SRC_REV is unset — refusing to move a tree with no baseline" + # A queued candidate (agent-box-candidate BRANCH) is the target for + # exactly one update. Consumed BEFORE anything can fail, so a candidate + # that does not build is not retried on every later trigger — the next + # one goes back to the tracked branch, which is the whole of the + # "nothing here pins this box" promise. An explicit --rev wins: an + # operator naming a ref is not asking about the queue. + # Whether the TARGET was named by the caller, which decides below + # whether the fast-forward guard may relax itself. Recorded before the + # queue can overwrite $ref. + explicit_ref=$ref + # The candidate marker carries two fields, and the difference between + # them is the difference between a request and a fact: + # + # want=REF agent-box-candidate asks for REF on the next update. + # on=REV this box IS running the candidate at REV. + # + # `on` is checked against the rev the box actually runs before it is + # believed, and that check is what makes it safe to relax the + # fast-forward guard. "The running rev is not an ancestor of the tracked + # branch" is NOT a usable signal on its own: a squash-merged candidate + # and a rewritten upstream history look identical from here, and the + # second is the exact attack --ff-only exists to refuse. So a stale `on` + # (a rebuild that failed and rolled the tree back, an operator moving + # the tree by hand) is ignored rather than trusted, and the strict guard + # comes straight back. + candidate="" + on_rev="" + if [ -n "$candidate_file" ] && [ -e "$candidate_file" ]; then + on_rev=$(sed -n 's/^on=//p' "$candidate_file" 2>/dev/null | head -n 1) + if [ -z "$ref" ]; then + candidate=$(sed -n 's/^want=//p' "$candidate_file" 2>/dev/null | head -n 1) + fi + if [ -n "$candidate" ]; then + # Consumed BEFORE anything can fail, so a candidate that does not + # build is not reinstalled by every later trigger. What survives is + # only the `on` fact, which the next run re-verifies. + if [ -n "$on_rev" ]; then + printf 'on=%s\n' "$on_rev" > "$candidate_file.tmp" \ + && mv "$candidate_file.tmp" "$candidate_file" + else + rm -f "$candidate_file" + fi + say "installing candidate $candidate (queued on this box, not merged)" + ref=$candidate + fi + fi + if [ -n "$on_rev" ] && [ "$on_rev" != "$rev" ]; then + say "candidate marker names $on_rev but the box runs $rev — ignoring it" + on_rev="" + fi clone_if_missing target=$(target_rev) || exit 1 resolve_branch @@ -215,6 +272,36 @@ case "${1:-}" in git checkout --quiet -B "$branch" "$rev" \ || die "could not put $dir on $branch at $rev" fi + # A candidate must be strictly AHEAD of the tracked branch, and this is + # where that is settled: the fetch above already brought both objects + # in, so it costs one ancestry query, and it holds however the marker + # came to be written. It is what keeps "candidate" from becoming a way + # to replay an older, possibly vulnerable rev — the exact thing + # --ff-only refuses and the exact thing the force below would otherwise + # reopen, since a box already off the branch takes that force path. + if [ -n "$candidate" ]; then + if ! git merge-base --is-ancestor "refs/remotes/origin/$branch" \ + "$target" 2>/dev/null; then + die "refusing candidate $candidate: $target is not ahead of $branch — rebase it, so what is tested here is what will land" + fi + fi + # Coming BACK from a candidate needs the guard relaxed, and only that. + # A candidate is squash-merged, so its head is never an ancestor of the + # tracked branch: once this box has taken one, --ff-only refuses the way + # home and the box would be stuck off-branch — the exact permanent + # divergence the feature promises not to create. + # + # Narrow on purpose. It needs the marker to say this box is running a + # candidate AND that claim to match the rev it actually runs (checked + # above), so a rewritten upstream history — which looks the same from + # the ancestry side — is still refused. The target is then either the + # tracked branch's own head or a branch agent-box-candidate already + # proved is ahead of it, so neither can be a downgrade. An explicit + # --rev keeps the strict guard and still has --force of its own. + if [ -z "$force" ] && [ -z "$explicit_ref" ] && [ -n "$on_rev" ]; then + say "the box runs candidate $rev — moving to $target without the fast-forward check" + force=1 + fi if [ "$target" = "$rev" ]; then : # nothing to move to; fall through and print the rev we are on elif [ -z "$force" ] && git merge-base --is-ancestor "$target" "$rev"; then @@ -237,6 +324,17 @@ case "${1:-}" in # nothing here should be resolving conflicts in. die "refusing update: $target is not a fast-forward of the running rev $rev" fi + # Record what the box is now running, so the NEXT plain update knows it + # has a way home to relax the guard for — and stop recording it once it + # is home, so nothing is left pinned or forced afterwards. + if [ -n "$candidate_file" ]; then + if [ -n "$candidate" ]; then + printf 'on=%s\n' "$(git rev-parse HEAD)" > "$candidate_file.tmp" \ + && mv "$candidate_file.tmp" "$candidate_file" + elif [ -n "$on_rev" ]; then + rm -f "$candidate_file" + fi + fi git rev-parse HEAD ;; diff --git a/tests/golden/web/units/agent-box-update.service b/tests/golden/web/units/agent-box-update.service index 0d914b0a..b683d510 100644 --- a/tests/golden/web/units/agent-box-update.service +++ b/tests/golden/web/units/agent-box-update.service @@ -2,6 +2,7 @@ Description=Fast-forward agent-box to upstream HEAD and rebuild [Service] +Environment="AGENT_BOX_CANDIDATE_FILE=/var/lib/agent-box/src.candidate" Environment="AGENT_BOX_SRC_DIR=/var/lib/agent-box/src" Environment="AGENT_BOX_SRC_REV=0000000000000000000000000000000000000000" Environment="AGENT_BOX_SRC_URL=https://github.com/defangdevs/agent-box.git" diff --git a/tests/native/expected-modes.json b/tests/native/expected-modes.json index 90c88069..d15d478d 100644 --- a/tests/native/expected-modes.json +++ b/tests/native/expected-modes.json @@ -2,6 +2,7 @@ "etc/agent-box-guides/AGENTS.agent.md": "0o444", "etc/agent-box-guides/AGENTS.robot.md": "0o444", "etc/agent-box/Caddyfile": "0o644", + "etc/agent-box/bin/agent-box-candidate": "0o755", "etc/agent-box/bin/agent-box-password-agent": "0o755", "etc/agent-box/bin/agent-box-password-robot": "0o755", "etc/agent-box/bin/agent-box-webhook-spawn": "0o755", diff --git a/tests/native/expected/etc/agent-box/bin/agent-box-candidate b/tests/native/expected/etc/agent-box/bin/agent-box-candidate new file mode 100755 index 00000000..2543f1fa --- /dev/null +++ b/tests/native/expected/etc/agent-box/bin/agent-box-candidate @@ -0,0 +1,295 @@ +#!/bin/sh +# Box constants, prepended by `agentbox apply`. +CANDIDATE_FILE=/var/lib/agent-box/candidate +SRC_DIR=/var/lib/agent-box/src +SRC_URL=https://github.com/defangdevs/agent-box.git +SRC_BRANCH='' +SYSTEMCTL=/usr/bin/systemctl +UPDATE_TRIGGER='/usr/bin/systemctl start --no-block agent-box-update.service' + +# agent-box-candidate — install a candidate release on THIS box, before it +# is merged and every other box gets it. +# +# The box's own fix used to have to land on master first. The only +# root-capable path an agent has is the update trigger, whose unit follows +# the tracked branch, and `git merge --ff-only` refuses everything else by +# construction — so "test it here, then merge" was not expressible and the +# fleet was the canary for its own tooling. This is the missing verb. +# +# It is a ROOT wrapper for the same reason the password helper is one: what +# writes the source tree decides what root builds, so the tree stays +# root-owned (issue #242) and an agent reaches it only through a command +# that can refuse. Every constant below is baked in by the renderer rather +# than read from the environment, because sudo scrubs the environment +# (env_reset) and a wrapper that took its target repo from the caller would +# hand root an arbitrary URL to build from. +# +# Three things it will not do, which is most of the point: +# +# * Anything but a BRANCH on this box's own configured origin. Not a +# tag, not a sha, and above all not refs/pull/N/head: on a public repo +# anyone can open a pull request, and a fetchable PR ref would mean a +# stranger's unreviewed code built as root. A branch needs push access +# to the source repo, and whoever has that can already merge. +# * A candidate that is not strictly ahead of the tracked branch. That is +# what stops an old, possibly vulnerable rev being replayed as a +# "candidate", which is the one attack the fast-forward guard exists to +# refuse and which the force this feature needs would otherwise reopen. +# The rev-equality half is here; ANCESTRY is agent-box-source's, at the +# moment of the move, because that is where the objects are and a +# question must not fetch into the tree to answer itself. +# * A permanent divergence. Nothing here pins the box: the marker it +# writes is consumed by the very next update, and an update with no +# marker always heads for the tracked branch (forcing only when the box +# is off it, since a squash-merged candidate is never an ancestor of +# master). So a candidate survives exactly until the next update, and a +# box that is forgotten converges on the fleet instead of drifting from +# it. +# +# Usage, all idempotent: +# +# agent-box-candidate BRANCH install that branch and re-apply +# agent-box-candidate --reset go back to the tracked branch now +# agent-box-candidate --status what the box is running, and off what +# +# The renderer prepends: CANDIDATE_FILE, SRC_DIR, SRC_URL, SRC_BRANCH, +# UPDATE_TRIGGER, SYSTEMCTL. +set -u + +prog=agent-box-candidate +say() { printf '%s: %s\n' "$prog" "$*" >&2; } +die() { say "$@"; exit 1; } + +# Same hardening as agent-box-source, and for the same reason: every git +# below runs as root against a tree fetched from the network, and a +# credential prompt has nobody to answer it. +export GIT_TERMINAL_PROMPT=0 +unset GIT_ASKPASS SSH_ASKPASS +export GIT_HTTP_LOW_SPEED_LIMIT=1000 GIT_HTTP_LOW_SPEED_TIME=60 +export GIT_CONFIG_COUNT=1 GIT_CONFIG_KEY_0=core.hooksPath GIT_CONFIG_VALUE_0=/dev/null + +usage() { + cat >&2 </dev/null) \ + || die "cannot reach $SRC_URL — offline?" + [ -n "$out" ] || die "origin has no branch '$b'" + else + out=$(command git ls-remote "$SRC_URL" HEAD 2>/dev/null) \ + || die "cannot reach $SRC_URL — offline?" + [ -n "$out" ] || die "$SRC_URL has no HEAD" + fi + printf '%s\n' "$out" | head -n 1 | cut -f1 +} + +running_rev() { + # The tree names the rev the box runs (the guide says so, so it has to be + # true here too). A box that has never updated has no tree yet, which is + # not an error for --status. + [ -e "$SRC_DIR/.git" ] || return 1 + command git -C "$SRC_DIR" rev-parse --verify --quiet HEAD 2>/dev/null +} + +case "${1:-}" in + (-h|--help) + usage + exit 0 + ;; + + (--status) + rev=$(running_rev) || die "$SRC_DIR is not a checkout yet — this box has never updated" + head=$(tracked_head) || exit 1 + printf 'rev: %s\n' "$rev" + printf 'tracked: %s at %s\n' "${SRC_BRANCH:-origin/HEAD}" "$head" + # The marker's own claim, believed only when it matches the rev the box + # actually runs — the same check agent-box-source makes before it + # relaxes the fast-forward guard, and for the same reason: a rebuild + # that failed and rolled back leaves a claim that is no longer true. + on="" + [ -e "$CANDIDATE_FILE" ] \ + && on=$(sed -n 's/^on=//p' "$CANDIDATE_FILE" 2>/dev/null | head -n 1) + if [ -n "$on" ] && [ "$on" = "$rev" ]; then + printf 'state: CANDIDATE — the next update returns to %s\n' \ + "${SRC_BRANCH:-the tracked branch}" + elif [ "$rev" = "$head" ]; then + printf 'state: on the tracked branch\n' + elif ! command git -C "$SRC_DIR" cat-file -e "$head^{commit}" 2>/dev/null; then + # The remote has moved on and the tree has not fetched it yet, which is + # the ordinary state of a box between updates. merge-base would exit + # 128 on the missing object and fall through to the warning below, + # reporting a perfectly normal box as one the guard has a problem with. + # Say what is actually known instead: there is a newer rev, and this + # tree cannot place itself against it without a fetch — which --status, + # being a question, will not do. + printf 'state: an update is available (%.12s); this tree has not fetched it, so it cannot say more\n' "$head" + elif command git -C "$SRC_DIR" merge-base --is-ancestor "$rev" "$head" 2>/dev/null; then + printf 'state: behind the tracked branch — an update is available\n' + else + # Off the branch with nothing claiming a candidate: a rewritten + # upstream history, or a tree somebody moved by hand. NOT reported as + # a candidate, because agent-box-source will not force out of it + # either — it is the case --ff-only exists to refuse. + printf 'state: off %s, and not as a candidate — the fast-forward guard applies\n' \ + "${SRC_BRANCH:-the tracked branch}" + fi + want="" + [ -e "$CANDIDATE_FILE" ] \ + && want=$(sed -n 's/^want=//p' "$CANDIDATE_FILE" 2>/dev/null | head -n 1) + [ -n "$want" ] && printf 'pending: %s\n' "$want" + exit 0 + ;; + + (--reset) + # An update with no QUEUED candidate already heads for the tracked + # branch, so "reset" is the ordinary update — the only work here is + # dropping a `want` that has not been consumed yet. The `on` fact must + # survive: it is what tells agent-box-source this box has a way home to + # relax the guard for, and deleting it would strand the box exactly + # where the reset was meant to rescue it from. + if [ -e "$CANDIDATE_FILE" ]; then + on=$(sed -n 's/^on=//p' "$CANDIDATE_FILE" 2>/dev/null | head -n 1) + if [ -n "$on" ]; then + printf 'on=%s\n' "$on" > "$CANDIDATE_FILE.tmp" \ + && mv "$CANDIDATE_FILE.tmp" "$CANDIDATE_FILE" + else + rm -f "$CANDIDATE_FILE" + fi + fi + say "returning to ${SRC_BRANCH:-the tracked branch} — triggering an update" + exec $UPDATE_TRIGGER + ;; + + ("") + usage + exit 1 + ;; + + (-*) + say "unknown option '$1'" + usage + exit 1 + ;; +esac + +branch=$1 +shift +[ $# -eq 0 ] || die "one branch at a time (got extra: $*)" + +# --- validate the name ------------------------------------------------ +# +# The name reaches git and reaches a file, so it is checked before either. +# Deliberately narrower than git's own refname rules: this is an allowlist +# of what a branch in this repo actually looks like, not an attempt to +# re-implement git-check-ref-format. +case $branch in + (refs/heads/*) + die "name the branch itself: '${branch#refs/heads/}' rather than '$branch'" + ;; + (refs/*) + # refs/pull/N/head lands here, which is the whole reason this arm is + # separate: there is no branch to suggest, and suggesting one by + # stripping refs/heads/ off a path that has no such prefix printed the + # input back as its own correction. + die "'$branch' is a ref path, not a branch — only a branch of $SRC_URL can be installed" + ;; + (*..*) + die "'$branch' contains '..'" + ;; + (/*|*/) + die "'$branch' starts or ends with '/'" + ;; + (*//*) + die "'$branch' contains '//'" + ;; + (*[!A-Za-z0-9._/-]*) + die "'$branch' has characters outside A-Za-z0-9._/- — refusing" + ;; + ([!A-Za-z0-9]*) + die "'$branch' does not start with a letter or digit" + ;; +esac +[ ${#branch} -le 200 ] || die "branch name is ${#branch} characters — refusing" + +# --- it must be a branch on OUR origin -------------------------------- +# +# ls-remote --heads, so only refs/heads/* can match. This is the line that +# keeps refs/pull/N/head — a stranger's unreviewed pull request on a public +# repo — from ever being installed as root. +out=$(command git ls-remote --heads "$SRC_URL" "refs/heads/$branch" 2>/dev/null) \ + || die "cannot reach $SRC_URL — offline?" +[ -n "$out" ] || die "$SRC_URL has no branch '$branch' (a tag, a sha or a pull-request ref will not do)" +want=$(printf '%s\n' "$out" | head -n 1 | cut -f1) + +# --- and not the tracked branch's own head ---------------------------- +# +# The cheap half of "strictly ahead", and the only half that can be settled +# from ls-remote. ANCESTRY is checked by agent-box-source at the moment of +# the move, where the objects already are: asking here would mean fetching +# them, and a fetch is not something a refused candidate — or a mistyped +# branch name — should leave behind in the tree that decides what root +# builds. That check is also the better home for it on its own merits, since +# it then holds however the marker was written. +head=$(tracked_head) || exit 1 +if [ "$want" = "$head" ]; then + die "'$branch' is at ${head} — the same rev as ${SRC_BRANCH:-the tracked branch}, so there is nothing to try" +fi + +# --- record the intent and let the update do the work ----------------- +# +# The marker is intent, never state: the updater consumes it as it starts, +# so a candidate that fails to build is not retried forever, and "what is +# this box running" stays a question answered by the tree rather than by a +# file that can disagree with it. +umask 022 +# A box that has never updated has no tree and may have no state dir either, +# and the marker's parent is that dir. Create it rather than failing: the +# update this queues would create it anyway when it clones. +mkdir -p "$(dirname "$CANDIDATE_FILE")" \ + || die "could not create $(dirname "$CANDIDATE_FILE")" +# An existing `on` fact is carried over, not dropped: a box already running +# a candidate is off the tracked branch, so moving it to ANOTHER candidate +# needs the same relaxation coming home does. agent-box-source re-verifies +# the fact against the running rev before it believes it either way. +on="" +[ -e "$CANDIDATE_FILE" ] \ + && on=$(sed -n 's/^on=//p' "$CANDIDATE_FILE" 2>/dev/null | head -n 1) +# A `[ ... ] && printf` as the LAST command in the group would make the +# group's status that test's — 1 whenever there is no fact to carry — and +# the `|| die` below would then report a failure on a write that worked. +if [ -n "$on" ]; then + printf 'want=refs/heads/%s\non=%s\n' "$branch" "$on" > "$CANDIDATE_FILE.tmp" \ + || die "could not write $CANDIDATE_FILE.tmp" +else + printf 'want=refs/heads/%s\n' "$branch" > "$CANDIDATE_FILE.tmp" \ + || die "could not write $CANDIDATE_FILE.tmp" +fi +mv "$CANDIDATE_FILE.tmp" "$CANDIDATE_FILE" \ + || die "could not put $CANDIDATE_FILE in place" +say "candidate $branch ($(printf '%.12s' "$want")) queued — triggering an update" +say "this box will be off ${SRC_BRANCH:-the tracked branch}; the next update returns to it" +exec $UPDATE_TRIGGER diff --git a/tests/native/expected/etc/sudoers.d/agent-box b/tests/native/expected/etc/sudoers.d/agent-box index 677b78f0..ebbbd69f 100644 --- a/tests/native/expected/etc/sudoers.d/agent-box +++ b/tests/native/expected/etc/sudoers.d/agent-box @@ -1,3 +1,3 @@ # Generated by `agentbox apply` - do not edit. -agent ALL=(root) NOPASSWD: /etc/agent-box/bin/agent-box-password-agent "", /usr/bin/systemctl reload caddy.service, /usr/bin/systemctl start --no-block agent-box-update.service, /usr/bin/systemctl reboot --no-block, /run/current-system/sw/bin/systemctl reload caddy.service +agent ALL=(root) NOPASSWD: /etc/agent-box/bin/agent-box-password-agent "", /usr/bin/systemctl reload caddy.service, /usr/bin/systemctl start --no-block agent-box-update.service, /usr/bin/systemctl reboot --no-block, /etc/agent-box/bin/agent-box-candidate *, /run/current-system/sw/bin/systemctl reload caddy.service robot ALL=(root) NOPASSWD: /etc/agent-box/bin/agent-box-password-robot "", /usr/bin/systemctl reload caddy.service, /usr/bin/systemctl start --no-block agent-box-update.service, /run/current-system/sw/bin/systemctl reload caddy.service diff --git a/tests/test-candidate.sh b/tests/test-candidate.sh new file mode 100644 index 00000000..60154dfb --- /dev/null +++ b/tests/test-candidate.sh @@ -0,0 +1,267 @@ +#!/usr/bin/env bash +# Unit tests for `agent-box-candidate` — installing a fix on THIS box before +# it is merged and every other box takes it. +# +# Weighted almost entirely at the REFUSALS, for the same reason +# test-source-tree.sh is: this runs as root, it decides what the next +# rebuild builds, and it is reached through a sudo grant whose argument is +# whatever an agent typed. sudoers deliberately does not validate that +# argument (`* ` allows one argument of anything) — this script is the +# validator, so each thing it must refuse gets an assertion: +# +# - anything that is not a BRANCH on this box's own origin. A tag or a +# sha would be a downgrade primitive; refs/pull/N/head would be a +# stranger's unreviewed pull request built as root, which on a public +# repo is anyone at all. The `--heads` scoping is what stops it, and +# the assertion below has a real refs/pull/1/head to prove it. +# - a candidate that is the tracked branch's own head, so there is +# nothing to try. The rest of "strictly ahead" is agent-box-source's: +# ANCESTRY needs the objects, and a question — or a mistyped branch +# name — must not fetch into the tree that decides what root builds to +# answer itself. tests/test-source-tree.sh asserts that half. +# - a name that is not a branch name at all: traversal, empty +# components, characters that have no business reaching git or a path. +# +# And two things it must PRESERVE, which are less obvious than the +# refusals: the `on` fact survives both a re-queue and a --reset, because +# it is what tells agent-box-source the box has a way home. Deleting it +# would strand the box exactly where a reset is meant to rescue it from. +# +# No network and no systemd: `origin` is a local repository built here and +# the update trigger is a shim that records its own invocation, so the whole +# file runs natively on every architecture. +set -u + +SCRIPT=${1:?usage: test-candidate.sh PATH/TO/candidate.sh} +[ -f "$SCRIPT" ] || { echo "no such script: $SCRIPT" >&2; exit 2; } +SCRIPT=$(cd "$(dirname "$SCRIPT")" && pwd)/$(basename "$SCRIPT") + +work=$(mktemp -d) +trap 'rm -rf "$work"' EXIT + +export HOME="$work/home"; mkdir -p "$HOME" +export GIT_CONFIG_GLOBAL="$work/gitconfig" +export GIT_CONFIG_NOSYSTEM=1 +export GIT_AUTHOR_NAME=t GIT_AUTHOR_EMAIL=t@example.invalid +export GIT_COMMITTER_NAME=t GIT_COMMITTER_EMAIL=t@example.invalid + +# --- the "upstream" this box is built from ------------------------------ +upstream="$work/upstream" +git init --quiet --initial-branch=master "$upstream" +echo one > "$upstream/file" +git -C "$upstream" add file +git -C "$upstream" commit --quiet -m one +REV_ONE=$(git -C "$upstream" rev-parse HEAD) +echo two > "$upstream/file" +git -C "$upstream" commit --quiet -am two +REV_TWO=$(git -C "$upstream" rev-parse HEAD) +git -C "$upstream" tag v-two + +# A candidate branch, off master's tip: what a rebased branch looks like. +git -C "$upstream" checkout --quiet -b ahead +echo three > "$upstream/file" +git -C "$upstream" commit --quiet -am three +REV_AHEAD=$(git -C "$upstream" rev-parse HEAD) +# One that is NOT ahead: it branches from the first commit. +git -C "$upstream" checkout --quiet -b behind "$REV_ONE" +git -C "$upstream" checkout --quiet master +# And one that is master's head exactly, so there is nothing to try. +git -C "$upstream" branch --quiet same "$REV_TWO" +# A pull-request ref, exactly as GitHub serves one. Fetchable by name and +# NOT a branch — the refusal this repo cares about most, since a public +# repo takes pull requests from anyone. +git -C "$upstream" update-ref refs/pull/1/head "$REV_AHEAD" + +src="$work/src" +git clone --quiet "$upstream" "$src" +git -C "$src" checkout --quiet -B master "$REV_TWO" + +marker="$work/candidate" +trigger_log="$work/triggered" +trigger="$work/bin/trigger" +mkdir -p "$work/bin" +cat > "$trigger" <<'SHIM' +#!/bin/sh +# Stands in for `systemctl start --no-block agent-box-update.service`. +echo "triggered" >> "$TRIGGER_LOG" +SHIM +chmod +x "$trigger" +export TRIGGER_LOG="$trigger_log" + +# The renderer prepends the constants; do exactly that, so what is under +# test is the shipped body and not a paraphrase of it. +runner="$work/agent-box-candidate" +{ + printf 'CANDIDATE_FILE=%s\n' "$marker" + printf 'SRC_DIR=%s\n' "$src" + printf 'SRC_URL=%s\n' "$upstream" + printf "SRC_BRANCH=master\n" + printf 'SYSTEMCTL=/bin/true\n' + printf 'UPDATE_TRIGGER=%s\n' "$trigger" + cat "$SCRIPT" +} > "$runner" + +fails=0 +ok() { printf 'ok %s\n' "$1"; } +no() { printf 'FAIL %s\n %s\n' "$1" "${2:-}"; fails=$((fails + 1)); } + +# run ARG... — stdout in $work/out, stderr in $work/err, rc in $rc +run() { + rc=0 + : > "$trigger_log" + sh "$runner" "$@" > "$work/out" 2> "$work/err" || rc=$? +} +said() { grep -F "$1" "$work/err" > /dev/null; } +printed() { grep -F "$1" "$work/out" > /dev/null; } +triggered() { [ -s "$trigger_log" ]; } +queued() { sed -n 's/^want=//p' "$marker" 2>/dev/null | head -n 1; } +fact() { sed -n 's/^on=//p' "$marker" 2>/dev/null | head -n 1; } + +# --- refusals: not a branch on our origin ------------------------------- +# Every one of these must exit non-zero AND leave the queue untouched: a +# refusal that still queued something would be installed by the next +# update, which is the whole failure this validator exists to prevent. +refuses() { + what=$1; shift + rm -f "$marker" + run "$@" + if [ "$rc" != 0 ] && ! triggered && [ ! -e "$marker" ]; then + ok "refuses $what" + else + no "refuses $what" "rc=$rc triggered=$(triggered && echo yes || echo no) marker=$(cat "$marker" 2>/dev/null)" + fi +} + +refuses "a pull-request ref" refs/pull/1/head +refuses "a bare sha" "$REV_AHEAD" +refuses "a tag" v-two +refuses "a branch that does not exist" no-such-branch +refuses "a path traversal" ../../etc/passwd +refuses "an embedded .." "fix/..\\/etc" +refuses "a leading slash" /master +refuses "a trailing slash" master/ +refuses "an empty component" fix//thing +refuses "a shell metacharacter" 'fix/x;reboot' +refuses "a space" 'fix/ x' +refuses "a branch at the tracked branch's own head" same +refuses "two arguments" ahead extra + +# Usage goes to stderr, like every other diagnostic here. +run ""; [ "$rc" != 0 ] && said "usage:" \ + && ok "refuses an empty argument, with usage" \ + || no "refuses an empty argument, with usage" "rc=$rc" + +# A pull-request ref must be refused for the right REASON — because it is +# not a branch, not because the name looks odd. This is the negative +# control on the --heads scoping: the ref exists and is fetchable. +rm -f "$marker" +run refs/pull/1/head +said "is a ref path, not a branch" \ + && ok "and says so, without inventing a branch to suggest" \ + || no "and says so, without inventing a branch to suggest" "$(cat "$work/err")" +# A refs/heads/ path DOES have a branch to suggest, and says which. +run refs/heads/ahead +said "name the branch itself: 'ahead'" \ + && ok "and a refs/heads/ path is told which branch it meant" \ + || no "and a refs/heads/ path is told which branch it meant" "$(cat "$work/err")" + +# --- the accept path ---------------------------------------------------- +rm -f "$marker" +run ahead +[ "$rc" = 0 ] && ok "accepts a branch that is ahead" \ + || no "accepts a branch that is ahead" "rc=$rc $(cat "$work/err")" +[ "$(queued)" = "refs/heads/ahead" ] \ + && ok "and queues it as a refs/heads/ path" \ + || no "and queues it as a refs/heads/ path" "$(cat "$marker" 2>/dev/null)" +triggered && ok "and triggers the update" \ + || no "and triggers the update" "the trigger was not run" +said "the next update returns to it" \ + && ok "and says the candidate is temporary" \ + || no "and says the candidate is temporary" "$(cat "$work/err")" + +# --- the `on` fact survives what must not strand the box ---------------- +printf 'on=%s\n' "$REV_AHEAD" > "$marker" +run ahead +[ "$rc" = 0 ] && [ "$(fact)" = "$REV_AHEAD" ] && [ "$(queued)" = "refs/heads/ahead" ] \ + && ok "re-queueing keeps the on= fact, so a sideways move can still come home" \ + || no "re-queueing keeps the on= fact" "$(cat "$marker" 2>/dev/null)" + +printf 'want=refs/heads/ahead\non=%s\n' "$REV_AHEAD" > "$marker" +run --reset +[ "$rc" = 0 ] && [ -z "$(queued)" ] && [ "$(fact)" = "$REV_AHEAD" ] \ + && ok "--reset drops the queue and keeps the on= fact" \ + || no "--reset drops the queue and keeps the on= fact" "$(cat "$marker" 2>/dev/null)" +triggered && ok "and triggers the update that brings the box home" \ + || no "and triggers the update that brings the box home" "the trigger was not run" + +printf 'want=refs/heads/ahead\n' > "$marker" +run --reset +[ "$rc" = 0 ] && [ ! -e "$marker" ] \ + && ok "--reset with no on= fact removes the marker entirely" \ + || no "--reset with no on= fact removes the marker" "$(cat "$marker" 2>/dev/null)" + +# --- --status reports the tree, not the marker's wishes ----------------- +rm -f "$marker" +run --status +[ "$rc" = 0 ] && printed "on the tracked branch" \ + && ok "--status: a box at the tracked branch's head says so" \ + || no "--status: a box at the tracked branch's head says so" "$(cat "$work/out")" + +git -C "$src" checkout --quiet -B master "$REV_ONE" +run --status +printed "an update is available" \ + && ok "--status: a box behind the branch says an update is available" \ + || no "--status: behind the branch" "$(cat "$work/out")" + +# Off the branch and CLAIMING a candidate, with the claim matching the rev +# the tree is on: a candidate. +git -C "$src" checkout --quiet -B master "$REV_AHEAD" +printf 'on=%s\n' "$REV_AHEAD" > "$marker" +run --status +printed "CANDIDATE" \ + && ok "--status: a verified candidate is reported as one" \ + || no "--status: a verified candidate is reported as one" "$(cat "$work/out")" + +# Off the branch with a claim that does NOT match: stale, so not a +# candidate — the same judgement agent-box-source makes before it would +# relax the fast-forward guard, and it must not read as "all fine here". +printf 'on=%s\n' "$REV_ONE" > "$marker" +run --status +printed "the fast-forward guard applies" \ + && ok "--status: a stale claim is not reported as a candidate" \ + || no "--status: a stale claim is not reported as a candidate" "$(cat "$work/out")" + +# --- a question is never a change --------------------------------------- +# --status and every refusal above run `git ls-remote` and touch the tree +# not at all — no fetch, no ref, no object. An agent asking what the box +# runs, or fat-fingering a branch name, must leave nothing behind in the +# tree that decides what root builds. (This is why the ancestry half of +# "strictly ahead" lives in agent-box-source: doing it here would have +# meant fetching, on a question.) +# Every ref, not just HEAD, and FETCH_HEAD too: a fetch would show up as a +# new object and a FETCH_HEAD even when the branch pointer never moved, and +# "does not move the tree" has to mean the whole tree. +before=$(git -C "$src" for-each-ref; git -C "$src" rev-parse HEAD) +before_objs=$(find "$src/.git/objects" -type f | sort | wc -l) +rm -f "$src/.git/FETCH_HEAD" +rm -f "$marker" +run --status +run no-such-branch +run refs/pull/1/head +run behind +after=$(git -C "$src" for-each-ref; git -C "$src" rev-parse HEAD) +after_objs=$(find "$src/.git/objects" -type f | sort | wc -l) +[ "$before" = "$after" ] \ + && ok "neither a question nor a refusal moves any ref in the tree" \ + || no "neither a question nor a refusal moves any ref" "refs changed" +[ "$before_objs" = "$after_objs" ] && [ ! -e "$src/.git/FETCH_HEAD" ] \ + && ok "and fetches nothing into it" \ + || no "and fetches nothing into it" "objects $before_objs -> $after_objs, FETCH_HEAD $([ -e "$src/.git/FETCH_HEAD" ] && echo present || echo absent)" + +echo +if [ "$fails" = 0 ]; then + echo "all candidate assertions passed" +else + echo "$fails failing assertion(s)" +fi +exit "$fails" diff --git a/tests/test-source-tree.sh b/tests/test-source-tree.sh index 58f95336..081cf217 100755 --- a/tests/test-source-tree.sh +++ b/tests/test-source-tree.sh @@ -324,6 +324,148 @@ AGENT_BOX_SRC_BRANCH=no-such-branch run check && ok "a failed check prints no rev at all" \ || no "a failed check prints no rev at all" "rc=$rc out=$(out)" +# --- a queued candidate (agent-box-candidate) --------------------------- +# +# The other half of "try a fix on this box before the fleet takes it": the +# wrapper queues a branch, and THIS script is what consumes it. Three +# properties are load-bearing, and each one is the difference between a +# candidate and a pin: +# +# - it is consumed, once. A candidate that survived its own update would +# be reinstalled by every later trigger, and a box that failed to build +# it would retry forever. +# - coming home does not need the operator. A candidate is squash-merged, +# so its head is never an ancestor of the tracked branch and --ff-only +# refuses the way back — the box would be stuck off-branch, which is the +# exact permanent divergence the feature promises not to create. +# - an explicit --rev keeps the strict guard. The relaxation is for the +# queue and for the way home, not a general-purpose force. +candidate_file="$work/candidate" +export AGENT_BOX_CANDIDATE_FILE="$candidate_file" + +# A branch off the tracked branch's tip, which is what a rebased candidate +# looks like. REV_THREE is master's head throughout. +git -C "$upstream" branch --quiet cand "$REV_THREE" 2>/dev/null || \ + git -C "$upstream" branch -f cand "$REV_THREE" +git -C "$upstream" checkout --quiet cand +echo four > "$upstream/file" +git -C "$upstream" commit --quiet -am four +REV_CAND=$(git -C "$upstream" rev-parse HEAD) +git -C "$upstream" checkout --quiet master + +at "$REV_THREE" +export AGENT_BOX_SRC_REV="$REV_THREE" +printf 'want=refs/heads/cand\n' > "$candidate_file" +run pull +[ "$rc" = 0 ] && [ "$(out)" = "$REV_CAND" ] \ + && ok "a queued candidate is what the tree moves to" \ + || no "a queued candidate is what the tree moves to" "rc=$rc out=$(out) want=$REV_CAND" +said "installing candidate refs/heads/cand" \ + && ok "and it says so, naming the branch" \ + || no "and it says so, naming the branch" "$(cat "$work/err")" +grep -q "^want=" "$candidate_file" 2>/dev/null \ + && no "the queued candidate is consumed" "want= survived the pull" \ + || ok "the queued candidate is consumed, so no later update reinstalls it" +[ "$(sed -n 's/^on=//p' "$candidate_file")" = "$REV_CAND" ] \ + && ok "and the box records WHICH candidate it now runs" \ + || no "and the box records which candidate it now runs" "$(cat "$candidate_file" 2>/dev/null)" + +# A candidate that is NOT ahead of the tracked branch is refused here, +# whatever wrote the marker. This is what keeps "candidate" from being a way +# to replay an older rev, and it has to live on this side: the force path +# below is taken by any box already off the branch, so a candidate accepted +# without this check would be installed with no ancestry guard at all. +git -C "$upstream" branch --quiet stale "$REV_ONE" 2>/dev/null || \ + git -C "$upstream" branch -f stale "$REV_ONE" +at "$REV_THREE" +printf 'want=refs/heads/stale\n' > "$candidate_file" +AGENT_BOX_SRC_REV="$REV_THREE" run pull +[ "$rc" != 0 ] && said "is not ahead of master" \ + && ok "a candidate behind the tracked branch is refused" \ + || no "a candidate behind the tracked branch is refused" "rc=$rc out=$(out)" +[ "$(head_of "$dir")" = "$REV_THREE" ] \ + && ok "and the tree does not move for it" \ + || no "and the tree does not move for it" "$(head_of "$dir")" + +# Same refusal on a box that is already off the branch, which is the case +# that matters: there the guard would otherwise be forced past. +at "$REV_CAND" +printf 'want=refs/heads/stale\non=%s\n' "$REV_CAND" > "$candidate_file" +AGENT_BOX_SRC_REV="$REV_CAND" run pull +[ "$rc" != 0 ] && said "is not ahead of master" \ + && ok "and refused on a candidate box too, where force would apply" \ + || no "and refused on a candidate box too" "rc=$rc out=$(out)" + +# Now the box RUNS the candidate, and the branch has been squash-merged: +# master has a new commit that is not the candidate's parent, so the +# candidate is not an ancestor of master and --ff-only cannot get home. +echo five > "$upstream/file" +git -C "$upstream" commit --quiet -am "squash of cand" +REV_MERGED=$(git -C "$upstream" rev-parse HEAD) +at "$REV_CAND" +printf 'on=%s\n' "$REV_CAND" > "$candidate_file" +AGENT_BOX_SRC_REV="$REV_CAND" run pull +[ "$rc" = 0 ] && [ "$(out)" = "$REV_MERGED" ] \ + && ok "a plain update brings a candidate box home to the tracked branch" \ + || no "a plain update brings a candidate box home" "rc=$rc out=$(out) want=$REV_MERGED" +said "runs candidate" \ + && ok "and names why the fast-forward check was skipped" \ + || no "and names why the fast-forward check was skipped" "$(cat "$work/err")" +[ ! -e "$candidate_file" ] \ + && ok "and clears the marker, so nothing stays forced once it is home" \ + || no "and clears the marker once home" "$(cat "$candidate_file" 2>/dev/null)" + +# THE safety property. "Off the tracked branch" is not the signal — a +# rewritten upstream history looks identical from the ancestry side, and +# that is the one thing --ff-only exists to refuse. Without a marker +# claiming a candidate, an off-branch box keeps the strict guard. +at "$REV_CAND" +rm -f "$candidate_file" +AGENT_BOX_SRC_REV="$REV_CAND" AGENT_BOX_SRC_REF="$REV_ONE" run pull +[ "$rc" != 0 ] && said "refusing update" \ + && ok "an off-branch box with no candidate marker is still guarded" \ + || no "an off-branch box with no candidate marker is still guarded" "rc=$rc out=$(out)" + +# And a marker whose claim does not match the rev the box runs is stale — +# a rebuild that failed and rolled the tree back leaves exactly that — so +# it is ignored rather than trusted into a force. +at "$REV_CAND" +printf 'on=%s\n' "$REV_ONE" > "$candidate_file" +AGENT_BOX_SRC_REV="$REV_CAND" AGENT_BOX_SRC_REF="$REV_ONE" run pull +[ "$rc" != 0 ] && said "ignoring it" \ + && ok "a stale candidate marker is ignored, not trusted" \ + || no "a stale candidate marker is ignored, not trusted" "rc=$rc out=$(out)" + +# The relaxation is scoped to the way home. An operator naming an OLDER rev +# explicitly is refused even on a genuine candidate box, so "I once tried a +# candidate" never becomes "this box accepts downgrades". +at "$REV_CAND" +printf 'on=%s\n' "$REV_CAND" > "$candidate_file" +AGENT_BOX_SRC_REV="$REV_CAND" AGENT_BOX_SRC_REF="$REV_ONE" run pull +[ "$rc" != 0 ] && said "refusing update" \ + && ok "an explicit older --rev is still refused on a candidate box" \ + || no "an explicit older --rev is still refused on a candidate box" "rc=$rc out=$(out)" + +# And the queue does not override an operator who named a ref. +at "$REV_TWO" +printf 'want=refs/heads/cand\n' > "$candidate_file" +AGENT_BOX_SRC_REV="$REV_TWO" AGENT_BOX_SRC_REF="$REV_THREE" run pull +[ "$rc" = 0 ] && [ "$(out)" = "$REV_THREE" ] \ + && ok "an explicit --rev wins over a queued candidate" \ + || no "an explicit --rev wins over a queued candidate" "rc=$rc out=$(out)" +grep -q "^want=" "$candidate_file" 2>/dev/null \ + && ok "and leaves the queue alone, so the next plain update still sees it" \ + || no "and leaves the queue alone" "marker was consumed by a --rev run" +rm -f "$candidate_file" + +# A box with the feature unwired behaves exactly as it did before it +# existed: no marker path, no candidate, no change in the guard. +at "$REV_ONE" +AGENT_BOX_CANDIDATE_FILE= AGENT_BOX_SRC_REV="$REV_ONE" run pull +[ "$rc" = 0 ] && [ "$(out)" = "$REV_MERGED" ] \ + && ok "with no marker path wired, a pull is an ordinary fast-forward" \ + || no "with no marker path wired, a pull is an ordinary fast-forward" "rc=$rc out=$(out)" + # --- a clone that cannot happen ----------------------------------------- # Offline, or a repo this box's token cannot read. It must fail loudly and # leave nothing behind: a half-clone at $dir would be skipped by every later diff --git a/tests/test_agentbox.py b/tests/test_agentbox.py index 588c48a3..4b977013 100644 --- a/tests/test_agentbox.py +++ b/tests/test_agentbox.py @@ -154,6 +154,13 @@ def build_fake_profile(root): (prof / "libexec" / "agent-box").mkdir(parents=True) shutil.copy(SRC / "password-helper.py", prof / "libexec" / "agent-box" / "password-helper.py") + # agent-box-candidate's body, rendered per box with its constants + # prepended (see Renderer's candidate block). Copied here for the same + # reason the password helper is: without it the renderer's `is_file()` + # guard skips the wrapper, and the expected tree would ratify its + # ABSENCE instead of its content. + shutil.copy(SRC / "candidate.sh", + prof / "libexec" / "agent-box" / "candidate.sh") return prof @@ -1233,6 +1240,41 @@ def test_wrappers_generated_without_web(self): profile = (out / "usr/local/bin/agent-box-profile").read_text() self.assertIn("AGENT_BOX_ENVSTORE_BIN=", profile) + def test_candidate_helper_generated_without_web(self): + """agent-box-candidate must exist on a web-disabled box, because the + GRANT for it does. sudoers() adds it from implied_sudo_commands(), + which is not gated on web.enable -- native has no selfUpdate.enable + to gate the update trigger on either -- so a box that renders the + rule and not the file leaves sudo finding a command that is not + there: exec fails with a bare "No such file or directory" and + nothing says the box was built without it. + + Exactly the trap `test_wrappers_generated_without_web` above was + written for on #403, and it caught this one too (CodeRabbit finding + on the candidate-release PR): the wrapper was first rendered from + inside self.caddy(), which a web-disabled box never calls. The pair + is what matters, so this asserts BOTH halves together.""" + data = json.loads(CONFIG_JSON.read_text()) + data["web"] = {"enable": False} + del data["webhook"] + with tempfile.TemporaryDirectory() as tmp: + cfg = Path(tmp) / "no-web.json" + cfg.write_text(json.dumps(data)) + out = render(tmp, cfg) + helper = out / "etc/agent-box/bin/agent-box-candidate" + self.assertTrue( + helper.exists(), + "web.enable: false rendered no agent-box-candidate, but " + "sudoers still grants it") + # And it is the wrapper, with its constants compiled in rather + # than left to an environment sudo will not carry. + body = helper.read_text() + self.assertIn("CANDIDATE_FILE=", body) + self.assertIn("SRC_URL=", body) + self.assertIn("UPDATE_TRIGGER=", body) + sudoers = (out / "etc/sudoers.d/agent-box").read_text() + self.assertIn("/etc/agent-box/bin/agent-box-candidate *", sudoers) + def test_every_user_gets_their_own_canonical_guide(self): """Issue #394. Two things were wrong at once here.