diff --git a/hosts/mars/hermes-agent.nix b/hosts/mars/hermes-agent.nix index 19a2bed..a4d3185 100644 --- a/hosts/mars/hermes-agent.nix +++ b/hosts/mars/hermes-agent.nix @@ -90,7 +90,7 @@ let # is hers to make, anywhere inside HERMES_WRITE_SAFE_ROOT=/opt/data. giteaHost = "git.mgaction.town"; - # luna's webhook filter, mounted READ-ONLY below. It lives in the nix store + # luna's webhook filters, mounted READ-ONLY below. They live in the nix store # rather than being written into hermesHome because hermesHome IS # HERMES_WRITE_SAFE_ROOT: a filter dropped there is a loop guard sitting # inside the writable root of the agent it constrains, and she could edit @@ -102,15 +102,52 @@ let prCommentFilter = pkgs.writeText "gitea-pr-comment-filter.py" ( builtins.readFile ./gitea-pr-comment-filter.py ); + prReviewFilter = pkgs.writeText "gitea-pr-review-filter.py" ( + builtins.readFile ./gitea-pr-review-filter.py + ); - # The route prompt, mounted read-only for the same reason as the filter and - # kept in a file rather than inline in the subscribe command: it is 60 lines - # of markdown containing apostrophes and {placeholders}, which would have to - # survive nix string escaping, the systemd unit, and `podman exec sh -c` - # quoting. A file crosses all three untouched and stays diffable in git. + # The route prompts. These are NOT mounted into the container: the route + # config below embeds them as strings, and jq reads them from these store + # paths host-side with --rawfile. Keeping them in files rather than inline + # nix strings is still what makes that work — they are ~60 lines of markdown + # full of apostrophes and {placeholders} that would otherwise have to + # survive nix string escaping on the way into a shell command. --rawfile + # crosses all of that untouched, and they stay diffable in git. prCommentPrompt = pkgs.writeText "gitea-pr-comment-prompt.md" ( builtins.readFile ./gitea-pr-comment-prompt.md ); + prReviewPrompt = pkgs.writeText "gitea-pr-review-prompt.md" ( + builtins.readFile ./gitea-pr-review-prompt.md + ); + + # Wire event names (X-GitHub-Event) each route accepts — NOT the + # subscription names the gitea hooks in services/dev/gitea.nix use. The two + # namespaces collide; see the long comment on the route unit below. + prCommentEvents = [ "issue_comment" ]; + prReviewEvents = [ "pull_request_comment" "pull_request_rejected" ]; + + # Toolsets granted to both routes' agent runs. + # + # Hermes defaults webhook runs to a deliberately narrow set (web_search, + # web_extract, vision_analyze, clarify) because a webhook payload is + # third-party content. That default cannot clone, edit or push, so neither + # prompt was executable under it: the run would be woken, read the comment, + # and have no way to act on it. + # + # This list REPLACES the platform default for these routes rather than + # merging with it, so anything the default provided has to be re-listed — + # "web" is here for that reason, not because the prompts ask for research. + # + # Upstream's stated boundary is that `hermes webhook subscribe` has no + # --toolsets flag, so "an agent creating its own subscription at runtime + # cannot self-grant terminal". That boundary does NOT hold here and must not + # be relied on: webhook_subscriptions.json lives under /opt/data, which is + # HERMES_WRITE_SAFE_ROOT, so luna can edit her own grant — she already did + # once, which is why this moved into nix. What this buys is that the grant + # is deliberate, reviewable and re-asserted on every restart, not that it is + # unforgeable. The real backstop stays server-side: gitea's branch + # protection on master. + routeToolsets = [ "terminal" "file" "web" ]; # hermesHome as the CONTAINER sees it (the bind mount below). Anything # written host-side that gets READ back inside the container must use this @@ -169,12 +206,11 @@ in script = '' mkdir -p ${hermesHome} mkdir -p ${dropboxDir} - # Parent for the read-only filter bind-mounted at - # /opt/data/scripts/gitea-pr-comment-filter.py. /opt/data is itself a - # bind mount of hermesHome, so this directory has to exist HOST-side - # before podman can mount a file inside it. + # Parent for the read-only filters bind-mounted at + # /opt/data/scripts/gitea-pr-*-filter.py. /opt/data is itself a bind + # mount of hermesHome, so this directory has to exist HOST-side before + # podman can mount a file inside it. mkdir -p ${hermesHome}/scripts - mkdir -p ${hermesHome}/prompts export HOME=${hermesHome} export GIT_CONFIG_GLOBAL=${hermesHome}/.gitconfig @@ -217,9 +253,9 @@ in ${hermesHome}/.git-credentials # Same cont-init caveat as the files above: the directory is created # here as root, and Hermes reads its scripts as uid ${hermesUid}. The - # mounted filter itself is world-readable 0444 from the store, so only - # the directory needs handing over. - chown ${hermesUid}:${hermesGid} ${hermesHome}/scripts ${hermesHome}/prompts + # mounted filters themselves are world-readable 0444 from the store, so + # only the directory needs handing over. + chown ${hermesUid}:${hermesGid} ${hermesHome}/scripts if [ -d ${hermesHome}/.config ]; then chown ${hermesUid}:${hermesGid} ${hermesHome}/.config @@ -251,9 +287,11 @@ in # secrets, so mounting the whole thing read-only costs nothing beyond # the two specific binaries actually being reachable. # Read-only: see prCommentFilter above. Hermes resolves route scripts - # under ~/.hermes/scripts, which is /opt/data/scripts in here. + # under ~/.hermes/scripts, which is /opt/data/scripts in here. The route + # prompts are NOT mounted — they are embedded in the route config the + # unit below writes, so nothing inside the container reads them. "${prCommentFilter}:/opt/data/scripts/gitea-pr-comment-filter.py:ro" - "${prCommentPrompt}:/opt/data/prompts/gitea-pr-comment.md:ro" + "${prReviewFilter}:/opt/data/scripts/gitea-pr-review-filter.py:ro" "/nix/store:/nix/store:ro" "${pkgs.git}/bin/git:/usr/local/bin/git:ro" @@ -304,66 +342,88 @@ in unitConfig.RequiresMountsFor = [ "/mnt/jupiter" ]; }; - # The Gitea PR-comment route. Gitea posts straight here (jupiter's - # gitea-hermes-webhook-provision registers the hook at - # http://mars.orbit.sol:8644/webhooks/gitea-pr-comments) -- there is no relay - # in between. Gitea's addDefaultHeaders sends X-Hub-Signature-256 in GitHub's - # exact format AND X-GitHub-Event, unconditionally, for every webhook type, - # which is precisely what Hermes validates and reads the event name from. + # The two Gitea webhook routes, written as config rather than created with + # `hermes webhook subscribe`. # - # --events issue_comment, NOT pull_request_comment. Gitea uses the same - # strings in two different namespaces and they collide: + # Gitea posts straight at Hermes (jupiter's gitea-hermes-webhook-provision + # registers one hook per route at http://mars.orbit.sol:8644/webhooks/) + # — there is no relay in between. Gitea's addDefaultHeaders sends + # X-Hub-Signature-256 in GitHub's exact format AND X-GitHub-Event, + # unconditionally, for every webhook type, which is precisely what Hermes + # validates and reads the event name from. # - # subscription name wire name (X-GitHub-Event) what it is - # ----------------------- -------------------------- ---------------- - # pull_request_comment issue_comment comment on a PR - # issue_comment issue_comment comment on an issue - # pull_request_review_comment pull_request_comment review on a PR + # WHY NOT `hermes webhook subscribe`: it has no --toolsets flag, and without + # a toolset override a webhook run gets Hermes's constrained default + # (web_search, web_extract, vision_analyze, clarify) — no shell, no file + # access, so neither prompt below can actually be carried out. Upstream's + # documented answer is to write the `toolsets` key into + # webhook_subscriptions.json by hand. Doing that by hand does not survive + # this unit, which re-provisions on every start, so the whole route + # definition moves here instead and the CLI is not used at all. See + # routeToolsets above for what that costs. # - # The hook's `events` array (services/dev/gitea.nix) takes the SUBSCRIPTION - # name; Hermes matches --events against X-GitHub-Event, i.e. the WIRE name, - # which comes from HookEventType.Event() in modules/webhook/type.go. So - # "pull_request_comment" here would match review submissions and never a - # comment -- the exact inversion of what it reads like. X-GitHub-Event-Type - # carries the subscription name, but Hermes does not look at it. + # This writes the file HOST-side. hermesHome is bind-mounted at /opt/data, + # so the container sees the same inode, and the webhook adapter hot-reloads + # the file (mtime-gated) on the next delivery — no container restart, and no + # `podman exec` quoting chain between nix and the prompt text. + # + # Events are WIRE names (X-GitHub-Event), not subscription names. Gitea uses + # the same strings in two namespaces and they collide — from + # HookEventType.Event() in modules/webhook/type.go: + # + # subscription name wire name what it is + # --------------------------- ---------------------- ------------------ + # issue_comment issue_comment comment on an issue + # pull_request_comment issue_comment comment on a PR + # pull_request_review_comment pull_request_comment review with a body + # pull_request_review_rejected pull_request_rejected changes requested + # pull_request_review_approved pull_request_approved approval + # + # The hooks' `events` arrays in services/dev/gitea.nix take the SUBSCRIPTION + # name; Hermes matches these against X-GitHub-Event, i.e. the WIRE name. So + # "pull_request_comment" HERE means a review and "issue_comment" HERE means + # a comment — the exact inversion of how they read. X-GitHub-Event-Type + # carries the subscription name, but Hermes does not look at it. Both files + # therefore name the same event differently on purpose; neither is a typo. # # issue_comment on the wire covers comments on plain issues too; the hook - # does not subscribe those, and the filter's is_pull check drops them anyway - # if the hook is ever widened. + # does not subscribe those, and the comment filter's is_pull check drops + # them anyway if the hook is ever widened. # - # A route carries exactly one prompt, so another event means either branching - # on {action} in the prompt or a second subscription plus a second Gitea hook - # at /webhooks/. Review comments would need that: they arrive as a - # PullRequestPayload with action "reviewed" and no comment object at all. + # deliver is "log", not a chat target: both prompts tell her to answer in + # the pull request, so the PR comment IS the delivery. # - # No --deliver: it defaults to `log`. The prompt tells her to answer in the - # pull request, so the PR comment IS the delivery. - # - # --script is the selection that MUST NOT be retunable at runtime. + # `script` is the selection that MUST NOT be retunable at runtime. # gitea-pr-comment-filter.py drops luna's own comments before any LLM call, # which is what stops the reply loop: the prompt tells her to answer on the - # PR, and her answer is itself a pull_request_comment. Both it and the prompt - # are bind-mounted read-only from the store above so the agent cannot edit - # its own guard out. Hermes resolves both names relative to ~/.hermes, hence - # the bare filename. + # PR, and her answer is itself a pull_request_comment. Both filters are + # bind-mounted read-only from the store above so the agent cannot edit her + # own guard out. Hermes resolves the name relative to ~/.hermes/scripts, + # hence the bare filename. # # What read-only does NOT buy: it protects the sources, and this unit - # re-subscribes from them on every start, so a restart restores the intended - # prompt, filter and event list. The live subscription lives in - # webhook_subscriptions.json under /opt/data and is hot-reloaded, which is - # inside the agent's own write-safe root -- a self-modification sticks until - # this unit next runs. + # re-asserts prompt, filter, events and toolsets from them on every start, + # so a restart restores the intended config. The live file is inside the + # agent's own write-safe root, so a self-modification sticks until this unit + # next runs. # - # The secret comes from the CONTAINER's environment, injected via - # sops.templates."hermes-agent.env", which is why secrets.nix restarts - # podman-hermes-agent BEFORE this unit on rotation: re-subscribing against a - # container still holding the old value would silently pin the stale secret. - systemd.services.hermes-agent-webhook-route = { - description = "Configure Hermes Gitea PR-comment webhook route"; + # Routes this unit does not name are left alone (the merge below is + # per-key), so retiring an old one stays a deliberate one-off: + # sudo podman exec hermes-agent hermes webhook remove + systemd.services.hermes-agent-webhook-routes = { + description = "Write Hermes's Gitea webhook route config"; wantedBy = [ "multi-user.target" ]; - after = [ "podman-hermes-agent.service" ]; - requires = [ "podman-hermes-agent.service" ]; - path = [ pkgs.podman ]; + # after, but not requires: this only writes a file that hermesHome must + # already exist for. A container that fails to come up should not also + # leave the routes unconfigured — the file is hot-reloaded whenever the + # gateway does start. + after = [ + "hermes-agent-prepare-dirs.service" + "podman-hermes-agent.service" + ]; + requires = [ "hermes-agent-prepare-dirs.service" ]; + path = [ pkgs.jq ]; + environment.SECRET_FILE = config.sops.secrets.gitea_hermes_webhook_secret.path; serviceConfig = { Type = "oneshot"; RemainAfterExit = true; @@ -371,38 +431,80 @@ in script = '' set -euo pipefail - # The container unit is ordered before us, but its gateway may still be - # warming up while the image initializes its persistent state directory. - for _ in $(seq 1 60); do - if podman exec hermes-agent hermes webhook list >/dev/null 2>&1; then - break - fi - sleep 1 - done + conf=${hermesHome}/webhook_subscriptions.json + tmp="$conf.new" + trap 'rm -f "$tmp"' EXIT - # Idempotency for the subscribe below, not cleanup: this removes only the - # route this unit owns. Retiring an old route is a one-off done by hand, - # so that a redeploy never silently deletes one added on purpose. - podman exec hermes-agent hermes webhook remove gitea-pr-comments >/dev/null 2>&1 || true + # --slurpfile below cannot read a file that does not exist. Creating it + # empty is safe: this only ever happens before the first run, when there + # are no routes to lose. If it exists but is not valid JSON, slurpfile + # fails the unit loudly and leaves it untouched, which is the right + # direction — better a failed unit than silently discarded routes. + [ -e "$conf" ] || printf '%s\n' '{}' > "$conf" - # `set -eu` plus both emptiness checks are load-bearing. Without them a - # missing prompt file or an unset secret yields an empty string, and the - # subscription is created with an empty prompt or -- worse -- an empty - # secret, which silently fails EVERY delivery signature check afterwards - # while the unit still looks healthy. Fail loudly here instead. - podman exec hermes-agent sh -c ' - set -eu - [ -n "''${GITEA_HERMES_WEBHOOK_SECRET:-}" ] || { - echo "GITEA_HERMES_WEBHOOK_SECRET is unset in the container" >&2; exit 1; } - prompt="$(cat /opt/data/prompts/gitea-pr-comment.md)" - [ -n "$prompt" ] || { echo "gitea-pr-comment prompt is empty" >&2; exit 1; } - hermes webhook subscribe gitea-pr-comments \ - --secret "$GITEA_HERMES_WEBHOOK_SECRET" \ - --description "Gitea PR comments -> L.U.N.A." \ - --events issue_comment \ - --script gitea-pr-comment-filter.py \ - --prompt "$prompt" - ' + # The secret reaches jq via --rawfile, never argv: /proc//cmdline + # is world-readable, so `--arg secret "$(cat ...)"` would publish it to + # every user on the box for the lifetime of the process. Same reason the + # prompts come in by path rather than by value. + # + # sops stores this one without a trailing newline (see secrets.nix), but + # rtrimstr is kept anyway: a stray newline would silently change the key + # the HMAC is computed with and fail every delivery afterwards. + # + # The emptiness guards are load-bearing. Without them a truncated secret + # file or an unreadable prompt yields "", and the route is written with + # an empty secret — which fails EVERY signature check while the unit + # still reports success. + jq -n \ + --slurpfile existing "$conf" \ + --rawfile rawSecret "$SECRET_FILE" \ + --rawfile commentPrompt ${prCommentPrompt} \ + --rawfile reviewPrompt ${prReviewPrompt} \ + --argjson commentEvents '${builtins.toJSON prCommentEvents}' \ + --argjson reviewEvents '${builtins.toJSON prReviewEvents}' \ + --argjson toolsets '${builtins.toJSON routeToolsets}' \ + ' + def nonempty($what): if length == 0 then error("\($what) is empty") else . end; + + ($rawSecret | rtrimstr("\n") | nonempty("gitea_hermes_webhook_secret")) as $secret + + | def route($desc; $events; $prompt; $script): + { description: $desc, + events: $events, + secret: $secret, + prompt: ($prompt | nonempty("\($script) prompt")), + skills: [], + script: $script, + deliver: "log", + toolsets: $toolsets }; + + # created_at is cosmetic (hermes webhook list prints it) and is the + # one key carried over from whatever is already there, so it keeps + # reading as when the route first appeared rather than as the last + # deploy. Everything else is replaced outright: a leftover key from + # an earlier definition — or from a hand edit — would otherwise + # survive here forever. + def upsert($name; $r): + .[$name] = ($r + { created_at: (.[$name].created_at // (now | todate)) }); + + ($existing[0] // {}) + | if type != "object" then error("webhook_subscriptions.json is not a JSON object") else . end + | upsert("gitea-pr-comments"; + route("Gitea PR comments -> L.U.N.A."; + $commentEvents; $commentPrompt; "gitea-pr-comment-filter.py")) + | upsert("gitea-pr-reviews"; + route("Gitea PR reviews -> L.U.N.A."; + $reviewEvents; $reviewPrompt; "gitea-pr-review-filter.py")) + ' > "$tmp" + + # 0600 because the file holds the HMAC secret in cleartext, and owned by + # the container's uid because Hermes rewrites it itself whenever anything + # calls `hermes webhook subscribe`. mv is an atomic rename within the + # same directory, so a delivery landing mid-write never reads a half + # written config. + chmod 0600 "$tmp" + chown ${hermesUid}:${hermesGid} "$tmp" + mv -f "$tmp" "$conf" ''; }; } diff --git a/hosts/mars/secrets.nix b/hosts/mars/secrets.nix index c867491..3251fff 100644 --- a/hosts/mars/secrets.nix +++ b/hosts/mars/secrets.nix @@ -28,27 +28,21 @@ sops.secrets.opencode_go_api_key = { }; sops.secrets.telegram_bot_token = { }; sops.secrets.hermes_dashboard_oidc_client_secret = { }; - # Add the same value to secrets/mars.yaml before deploying Mars, and store - # it WITHOUT a trailing newline: it reaches Hermes through the env template - # below, where a newline would both corrupt the env file and change the key - # the HMAC is computed with. `scripts/edit_secrets` writes a bare value. + # Same value as in secrets/jupiter.yaml (the sending side), stored WITHOUT a + # trailing newline — a stray newline would change the key the HMAC is + # computed with and fail every delivery. `scripts/edit_secrets` writes a + # bare value. hermes-agent.nix trims one anyway, belt and braces. # - # podman-hermes-agent is in restartUnits for a reason that is easy to miss: - # the secret reaches the container only through sops.templates, whose - # rendered PATH never changes, so the container unit's definition is - # identical before and after the secret is added and systemd will NOT - # restart it on its own. Without this line the very first deploy leaves the - # container holding an empty GITEA_HERMES_WEBHOOK_SECRET, and - # hermes-agent-webhook-route (which reads it back out of the running - # container) subscribes with an empty secret — every delivery then fails - # signature validation inside Hermes with no obvious cause. That unit now - # refuses to subscribe on an unset secret rather than doing it quietly, but - # the ordering here is still what makes the rotation correct. + # This is NOT in the container's env any more. It used to be, because + # hermes-agent-webhook-route ran `hermes webhook subscribe` inside the + # container and read the secret back out of its environment — which meant + # podman-hermes-agent had to be restarted first on rotation, or the + # subscription silently pinned the stale value. The route config is now + # written host-side (hermes-agent-webhook-routes reads this file directly), + # so that ordering constraint is gone and the secret no longer sits in an + # env var luna can read with `env`. sops.secrets.gitea_hermes_webhook_secret = { - restartUnits = [ - "podman-hermes-agent.service" - "hermes-agent-webhook-route.service" - ]; + restartUnits = [ "hermes-agent-webhook-routes.service" ]; }; sops.templates."hermes-agent.env".content = '' OPENCODE_GO_API_KEY=${config.sops.placeholder.opencode_go_api_key} @@ -57,7 +51,6 @@ TELEGRAM_ALLOWED_USERS=15151223 WEBHOOK_ENABLED=true WEBHOOK_PORT=8644 - GITEA_HERMES_WEBHOOK_SECRET=${config.sops.placeholder.gitea_hermes_webhook_secret} HERMES_DASHBOARD_OIDC_CLIENT_SECRET=${config.sops.placeholder.hermes_dashboard_oidc_client_secret} '';