From a94affd6ac3a4ed50b8aeee48152700b60247e1c Mon Sep 17 00:00:00 2001 From: mptyl Date: Wed, 5 Aug 2026 11:42:35 +0200 Subject: [PATCH] fix: enforce server trust boundaries --- docs/install/server-workspace-registry.md | 6 +- docs/install/server.md | 22 ++- scripts/build-thothctl.sh | 9 +- scripts/test-server-operator-permissions.sh | 81 ++++++++ scripts/test-verify-workspace-install-docs.sh | 66 ++++++- scripts/verify-workspace-install-docs.sh | 177 ++++++++++++++++-- tools/thothctl/cmd/thothctl/main.go | 9 +- tools/thothctl/cmd/thothctl/main_test.go | 46 ++++- .../thothctl/internal/serverops/operations.go | 100 ++++++++-- .../internal/serverops/operations_test.go | 35 ++++ 10 files changed, 511 insertions(+), 40 deletions(-) create mode 100755 scripts/test-server-operator-permissions.sh diff --git a/docs/install/server-workspace-registry.md b/docs/install/server-workspace-registry.md index 9a3e6048..8c33e8d6 100644 --- a/docs/install/server-workspace-registry.md +++ b/docs/install/server-workspace-registry.md @@ -15,7 +15,7 @@ first startup. Keep storage separated: /srv/thothii/data/ # settings, session data, Pi state as applicable /srv/thothii/workspace-registry/ # repo/, snapshots/, state/, locks/ /srv/thothii/secrets/ # Git and connector secret files, setgid mode 2750 -/srv/thothii/operator/ # untracked Compose/.env, setgid mode 2750 +/srv/thothii/operator/ # untracked operator files, setgid mode 2770 ``` Permit outbound TCP only to approved Git/Gitea, DWH, vector, embedding, and bastion endpoints. @@ -86,7 +86,8 @@ Variable names derive from the immutable ID: `north-star-research` becomes `NORT Copy [the bindings env example](examples/workspace-bindings.env.example) to the protected operator directory. Every path-valued `*_FILE` entry needs an absolute host-only `*_SOURCE` path. Generate the untracked connector override from those files during bootstrap; do not copy or maintain a -workspace-specific Compose override. +workspace-specific Compose override. Operator-managed path-only files use owner UID 10001, group +`thothii-ops`, and mode `0660`; secret files remain `0640` and non-group-writable. ## Direct PostgreSQL, REST, and SSH tunnel bindings @@ -163,6 +164,7 @@ Generate the connector override, then use the installation-aware operator CLI. B `thothctl` requires only Docker and no Go knowledge. From a trusted maintenance shell: ```sh +umask 0007 THT_SOURCE_ROOT=/srv/thothii/source/ThothII THT_OPERATOR_ENV=/srv/thothii/operator/server.env THT_WORKSPACE_BINDINGS_ENV_FILE=/srv/thothii/operator/workspace-bindings.env diff --git a/docs/install/server.md b/docs/install/server.md index ea101a0e..01cd8467 100644 --- a/docs/install/server.md +++ b/docs/install/server.md @@ -64,7 +64,7 @@ files only. Backups are separate from live data. ```sh sudo install -d -o 10001 -g thothii-ops -m 2750 /srv/thothii/source -sudo install -d -o 10001 -g thothii-ops -m 2750 /srv/thothii/operator +sudo install -d -o 10001 -g thothii-ops -m 2770 /srv/thothii/operator sudo install -d -o 10001 -g thothii-ops -m 2750 /srv/thothii/secrets sudo install -d -o 10001 -g 10001 -m 0750 /srv/thothii/data sudo install -d -o 10001 -g 10001 -m 0750 /srv/thothii/pi-state @@ -72,9 +72,9 @@ sudo install -d -o 10001 -g 10001 -m 0750 /srv/thothii/workspace-registry sudo install -d -o root -g root -m 0700 /srv/thothii-backups ``` -Do not make `/srv/thothii` a shared application directory. The source checkout may be read by the -operator, while secret contents and writable data remain limited to reviewed administrators and -UID 10001. +The human operator can write only `operator`; setgid keeps generated files in `thothii-ops`. +`source`, `secrets`, and all runtime bind trees remain non-group-writable. Do not make +`/srv/thothii` a shared application directory. ## Firewall and network boundaries @@ -190,10 +190,12 @@ sudo -u thothii cp docs/install/examples/thothii-installation.server.yaml \ /srv/thothii/operator/thothii-installation.yaml sudo chown 10001:thothii-ops /srv/thothii/operator/server.env \ /srv/thothii/operator/thothii-installation.yaml -sudo chmod 0640 /srv/thothii/operator/server.env \ +sudo chmod 0660 /srv/thothii/operator/server.env \ /srv/thothii/operator/thothii-installation.yaml ``` +The named human operator can now edit both placeholder files without `sudo`; use an editor that +preserves the group, or create replacements under `umask 0007` in the setgid operator directory. Replace every placeholder with an absolute path. Use exactly one Git transport override. For HTTPS, replace `deploy/compose.git-ssh.yaml` with `deploy/compose.git-https.yaml`. Keep the required session-server overlay and generated connector-secret override. Optional host-gateway or pinned @@ -260,11 +262,17 @@ Build the operator binaries with Docker. No Go installation or Go knowledge is r ```sh cd /srv/thothii/source/ThothII -bash scripts/build-thothctl.sh -sudo install -o root -g thothii-ops -m 0750 dist/thothctl/thothctl-linux-amd64 \ +THT_THOTHCTL_OUTPUT_DIRECTORY=/srv/thothii/operator/build-output \ + bash scripts/build-thothctl.sh +sudo install -o root -g thothii-ops -m 0750 \ + /srv/thothii/operator/build-output/thothctl-linux-amd64 \ /srv/thothii/operator/thothctl ``` +The source checkout stays read-only to the human. The explicit output directory is the only build +write boundary; the build script rejects relative or non-canonical output paths. After installation, +remove or retain `build-output` according to the site's reviewed artifact policy. + Use `thothctl-linux-arm64` on an ARM64 server. Set these variables in the maintenance shell; do not source `server.env` as shell code: diff --git a/scripts/build-thothctl.sh b/scripts/build-thothctl.sh index cd03a16d..27ecc944 100755 --- a/scripts/build-thothctl.sh +++ b/scripts/build-thothctl.sh @@ -2,7 +2,14 @@ set -euo pipefail repository_root=$(cd "$(dirname "$0")/.." && pwd) -output_directory="$repository_root/dist/thothctl" +output_directory="${THT_THOTHCTL_OUTPUT_DIRECTORY:-$repository_root/dist/thothctl}" + +if [[ "$output_directory" != /* || "$output_directory" == / || "$output_directory" == */ || + "$output_directory" == *//* || "/$output_directory/" == */../* || + "/$output_directory/" == */./* ]]; then + echo "THT_THOTHCTL_OUTPUT_DIRECTORY must be an absolute canonical path" >&2 + exit 2 +fi mkdir -p "$output_directory" docker build --file "$repository_root/docker/thothctl.Dockerfile" --output "type=local,dest=$output_directory" "$repository_root" diff --git a/scripts/test-server-operator-permissions.sh b/scripts/test-server-operator-permissions.sh new file mode 100755 index 00000000..67aa14a7 --- /dev/null +++ b/scripts/test-server-operator-permissions.sh @@ -0,0 +1,81 @@ +#!/usr/bin/env bash +# Execute the documented server ownership model with distinct runtime and human operator IDs. +set -euo pipefail + +root="$(cd "$(dirname "$0")/.." && pwd -P)" +image='golang:1.26.5-bookworm@sha256:1ecb7edf62a0408027bd5729dfd6b1b8766e578e8df93995b225dfd0944eb651' + +docker run --rm --volume "$root:/repository:ro" "$image" /bin/bash -ceu ' +groupadd --gid 10001 thothii +useradd --uid 10001 --gid 10001 --no-create-home --shell /usr/sbin/nologin thothii +groupadd --gid 20001 operator-primary +groupadd --gid 20002 thothii-ops +groupadd --gid 20003 docker +useradd --uid 20001 --gid 20001 --groups 20002,20003 --create-home --shell /bin/bash operator + +install -d -o 10001 -g 20002 -m 2750 /srv/thothii/source +install -d -o 10001 -g 20002 -m 2770 /srv/thothii/operator +install -d -o 10001 -g 20002 -m 2750 /srv/thothii/secrets +install -d -o 10001 -g 10001 -m 0750 /srv/thothii/data /srv/thothii/pi-state /srv/thothii/workspace-registry +install -d -o 10001 -g 20002 -m 2750 /srv/thothii/source/ThothII /srv/thothii/source/ThothII/scripts +install -o 10001 -g 20002 -m 0750 /repository/scripts/build-thothctl.sh /srv/thothii/source/ThothII/scripts/build-thothctl.sh +install -o 10001 -g 20002 -m 0750 /repository/scripts/generate-connector-secrets-override.sh /srv/thothii/source/ThothII/scripts/generate-connector-secrets-override.sh + +printf "%s\n" "PLACEHOLDER=replace-me" "THT_WS_TEST_DWH_PASSWORD_SOURCE=/srv/thothii/secrets/dwh-password" > /srv/thothii/operator/server.env +printf "%s\n" "projectDirectory: replace-me" > /srv/thothii/operator/thothii-installation.yaml +printf "%s\n" "THT_WS_TEST_DWH_PASSWORD_FILE=/run/secrets/test-dwh-password" > /srv/thothii/operator/workspace-bindings.env +printf "%s\n" "operator-readable-secret" > /srv/thothii/secrets/dwh-password +chown 10001:20002 /srv/thothii/operator/server.env /srv/thothii/operator/thothii-installation.yaml /srv/thothii/operator/workspace-bindings.env /srv/thothii/secrets/dwh-password +chmod 0660 /srv/thothii/operator/server.env /srv/thothii/operator/thothii-installation.yaml /srv/thothii/operator/workspace-bindings.env +chmod 0640 /srv/thothii/secrets/dwh-password + +printf "%s\n" \ + "#!/bin/bash" \ + "set -euo pipefail" \ + "if [[ \"\${1:-}\" == build ]]; then" \ + " destination=; for argument in \"\$@\"; do case \"\$argument\" in type=local,dest=*) destination=\"\${argument#type=local,dest=}\" ;; esac; done" \ + " test -n \"\$destination\"; mkdir -p \"\$destination\"" \ + " printf \"%s\\n\" \"#!/bin/bash\" \"set -euo pipefail\" \"test -r \\\"\\\$2\\\"\" \"test -r /srv/thothii/secrets/dwh-password\" \"docker compose up --detach\" > \"\$destination/thothctl-linux-amd64\"" \ + " chmod 0750 \"\$destination/thothctl-linux-amd64\"; exit 0" \ + "fi" \ + "test \"\${1:-}\" = compose; : > /srv/thothii/operator/start.marker" \ + > /usr/local/bin/docker +chmod 0755 /usr/local/bin/docker + +runuser --user operator -- /bin/bash -ceu '\'' +umask 0007 +sed -i "s/replace-me/ready/" /srv/thothii/operator/server.env +sed -i "s#replace-me#/srv/thothii/source/ThothII#" /srv/thothii/operator/thothii-installation.yaml +/srv/thothii/source/ThothII/scripts/generate-connector-secrets-override.sh \ + --bindings-env /srv/thothii/operator/workspace-bindings.env \ + --operator-env /srv/thothii/operator/server.env \ + --output /srv/thothii/operator/connector-secrets.server.yaml +test -r /srv/thothii/secrets/dwh-password +if (printf tamper >> /srv/thothii/secrets/dwh-password) 2>/dev/null; then exit 41; fi +if touch /srv/thothii/source/operator-must-not-write 2>/dev/null; then exit 42; fi +if touch /srv/thothii/data/operator-must-not-write 2>/dev/null; then exit 43; fi +THT_THOTHCTL_OUTPUT_DIRECTORY=/srv/thothii/operator/build-output \ + /srv/thothii/source/ThothII/scripts/build-thothctl.sh +if THT_THOTHCTL_OUTPUT_DIRECTORY=relative-output \ + /srv/thothii/source/ThothII/scripts/build-thothctl.sh 2>/dev/null; then exit 44; fi +root_output_error=/srv/thothii/operator/root-output.error +if THT_THOTHCTL_OUTPUT_DIRECTORY=/ \ + /srv/thothii/source/ThothII/scripts/build-thothctl.sh 2>"$root_output_error"; then exit 45; fi +grep -Fq "THT_THOTHCTL_OUTPUT_DIRECTORY must be an absolute canonical path" \ + "$root_output_error" || exit 46 +rm -f "$root_output_error" +/srv/thothii/operator/build-output/thothctl-linux-amd64 \ + --installation /srv/thothii/operator/thothii-installation.yaml start +'\'' + +test "$(stat -c %u:%g /srv/thothii/operator/connector-secrets.server.yaml)" = 20001:20002 +test "$(stat -c %a /srv/thothii/operator/connector-secrets.server.yaml)" = 660 +test "$(stat -c %u:%g /srv/thothii/operator/build-output/thothctl-linux-amd64)" = 20001:20002 +test "$(stat -c %a /srv/thothii/operator/build-output/thothctl-linux-amd64)" = 750 +test -f /srv/thothii/operator/start.marker +test ! -e /srv/thothii/source/operator-must-not-write +test ! -e /srv/thothii/data/operator-must-not-write +test "$(cat /srv/thothii/secrets/dwh-password)" = operator-readable-secret +' + +echo "distinct server operator UID/GID fixture passed" diff --git a/scripts/test-verify-workspace-install-docs.sh b/scripts/test-verify-workspace-install-docs.sh index fe36b24c..01d46510 100755 --- a/scripts/test-verify-workspace-install-docs.sh +++ b/scripts/test-verify-workspace-install-docs.sh @@ -100,6 +100,41 @@ sed '/^case "\$mode" in/,$d' "$root/scripts/verify-workspace-install-docs.sh" >" # shellcheck source=/dev/null source "$verifier_functions" +adapted_reorder="$negative_root/caddy-adapted-reorder.json" +node - "$adapted_reorder" <<'NODE' +const fs = require("fs"); +const publicHeaders = [ + "X-Thoth-Principal-Issuer", "X-Thoth-Principal-Subject", + "X-Thoth-Principal-Display-Name", "X-Thoth-Is-Admin", +]; +const trustedHeaders = [ + "X-Thoth-Trusted-Principal-Issuer", "X-Thoth-Trusted-Principal-Subject", + "X-Thoth-Trusted-Principal-Display-Name", "X-Thoth-Trusted-Is-Admin", +]; +const clear = (name) => ({handler: "headers", request: {delete: [name]}}); +const auth = { + handler: "reverse_proxy", upstreams: [{dial: "auth-gateway:4180"}], + handle_response: [{match: {status_code: [2]}, routes: [{handle: trustedHeaders.map((name, index) => ({ + handler: "headers", request: {set: {[name]: [`{http.reverse_proxy.header.${publicHeaders[index]}}`]}}, + }))}]}], +}; +const document = {routes: [{handle: [ + ...publicHeaders.map(clear), auth, ...trustedHeaders.map(clear), + {handler: "reverse_proxy", upstreams: [{dial: "127.0.0.1:8080"}]}, +]}]}; +fs.writeFileSync(process.argv[2], JSON.stringify(document)); +NODE +adapted_output="$negative_root/caddy-adapted-output" +set +e +verify_caddy_adapted_identity_order "$adapted_reorder" >"$adapted_output" 2>&1 +adapted_status=$? +set -e +if [[ $adapted_status -eq 0 ]] || ! grep -Fq "Caddy adapted identity clears must execute before authentication" "$adapted_output"; then + echo "Caddy reordered adapted-handler fixture was not rejected correctly" >&2 + cat "$adapted_output" >&2 + exit 1 +fi + negative_failures=0 expect_guide_rejected() { local label="$1" validator="$2" source_guide="$3" relative_path="$4" @@ -175,6 +210,14 @@ switch (mutation) { case "nginx-no-admin-map": changed = original.replace("proxy_set_header X-Thoth-Trusted-Is-Admin $thoth_is_admin;", "proxy_set_header X-Thoth-Trusted-Is-Admin \"\";"); break; + case "nginx-admin-clear-wrong-scope": { + const clear = ' proxy_set_header X-Thoth-Is-Admin "";'; + const authAt = original.indexOf(clear); + changed = original.slice(0, authAt) + original.slice(authAt + clear.length + 1); + const frontendAt = changed.indexOf(clear); + changed = changed.slice(0, frontendAt) + clear + "\n" + clear + changed.slice(frontendAt + clear.length); + break; + } case "caddy-no-auth": changed = original.replace("forward_auth auth-gateway:4180 {", "# forward authentication omitted"); break; @@ -196,6 +239,13 @@ switch (mutation) { case "caddy-no-admin-map": changed = original.replace("X-Thoth-Is-Admin>X-Thoth-Trusted-Is-Admin", "X-Thoth-Is-Admin"); break; + case "caddy-clears-after-auth": { + const clearPattern = /(?:\t\trequest_header -X-(?:Authenticated-User|Thoth-[^\n]+)\n)+/; + const clears = original.match(clearPattern)?.[0] || ""; + changed = original.replace(clearPattern, ""); + changed = changed.replace("\n\t\treverse_proxy 127.0.0.1:8080 {", "\n" + clears + "\n\t\treverse_proxy 127.0.0.1:8080 {"); + break; + } case "dirty-source": changed = original.replaceAll("git status --porcelain --untracked-files=all", "git status --short"); break; @@ -310,19 +360,23 @@ expect_guide_rejected \ expect_guide_rejected \ "Nginx issuer inbound claim not cleared" verify_reverse_proxy_nginx_guide \ "$root/docs/install/reverse-proxy-nginx.md" docs/install/reverse-proxy-nginx.md nginx-no-issuer-clear \ - "Nginx proxy does not clear inbound issuer identity" + "Nginx auth location does not clear inbound issuer identity" expect_guide_rejected \ "Nginx subject auth response not captured" verify_reverse_proxy_nginx_guide \ "$root/docs/install/reverse-proxy-nginx.md" docs/install/reverse-proxy-nginx.md nginx-no-subject-capture \ - "Nginx proxy does not capture authenticated subject identity" + "Nginx frontend location does not capture authenticated subject identity" expect_guide_rejected \ "Nginx display identity not mapped to private hop" verify_reverse_proxy_nginx_guide \ "$root/docs/install/reverse-proxy-nginx.md" docs/install/reverse-proxy-nginx.md nginx-no-display-map \ - "Nginx proxy does not map authenticated display identity" + "Nginx frontend location does not map authenticated display identity" expect_guide_rejected \ "Nginx admin identity not mapped to private hop" verify_reverse_proxy_nginx_guide \ "$root/docs/install/reverse-proxy-nginx.md" docs/install/reverse-proxy-nginx.md nginx-no-admin-map \ - "Nginx proxy does not map authenticated admin identity" + "Nginx frontend location does not map authenticated admin identity" +expect_guide_rejected \ + "Nginx admin clear moved out of auth scope" verify_reverse_proxy_nginx_guide \ + "$root/docs/install/reverse-proxy-nginx.md" docs/install/reverse-proxy-nginx.md nginx-admin-clear-wrong-scope \ + "Nginx auth location does not clear inbound admin identity" expect_guide_rejected \ "Caddy identity without authentication" verify_reverse_proxy_caddy_guide \ "$root/docs/install/reverse-proxy-caddy.md" docs/install/reverse-proxy-caddy.md caddy-no-auth \ @@ -351,6 +405,10 @@ expect_guide_rejected \ "Caddy admin identity not mapped to private hop" verify_reverse_proxy_caddy_guide \ "$root/docs/install/reverse-proxy-caddy.md" docs/install/reverse-proxy-caddy.md caddy-no-admin-map \ "Caddy proxy does not map authenticated admin identity" +expect_guide_rejected \ + "Caddy identity clears reordered after auth" verify_reverse_proxy_caddy_guide \ + "$root/docs/install/reverse-proxy-caddy.md" docs/install/reverse-proxy-caddy.md caddy-clears-after-auth \ + "Caddy identity clears must precede forward_auth" expect_guide_rejected \ "dirty or untracked source tree" verify_local_guide \ diff --git a/scripts/verify-workspace-install-docs.sh b/scripts/verify-workspace-install-docs.sh index f577ffc4..9ae61cc1 100755 --- a/scripts/verify-workspace-install-docs.sh +++ b/scripts/verify-workspace-install-docs.sh @@ -513,6 +513,9 @@ verify_server_guide() { "core" \ "UID/GID 10001" \ "thothii-ops" \ + "-m 2770 /srv/thothii/operator" \ + "chmod 0660 /srv/thothii/operator/server.env" \ + "THT_THOTHCTL_OUTPUT_DIRECTORY=/srv/thothii/operator/build-output" \ "/srv/thothii" \ "example operator root" \ "/run/secrets" \ @@ -655,19 +658,47 @@ const identities = [ ["admin", "Is-Admin", "thoth_is_admin", "x_thoth_is_admin"], ]; function escaped(value) { return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); } +function directiveBlock(text, marker) { + const start = text.indexOf(marker); + if (start < 0) throw new Error(`Nginx proxy lacks scoped block: ${marker}`); + const opening = text.indexOf("{", start); + let depth = 0; + for (let index = opening; index < text.length; index++) { + if (text[index] === "{") depth++; + if (text[index] === "}" && --depth === 0) return text.slice(opening + 1, index); + } + throw new Error(`Nginx proxy has unterminated scoped block: ${marker}`); +} +const authLocation = directiveBlock(block, "location = /_authenticate {"); +const frontendLocation = directiveBlock(block, "location / {"); for (const [label, publicName, variable, upstream] of identities) { const trustedName = publicName === "Is-Admin" ? "Is-Admin" : publicName; - const publicClears = block.match(new RegExp(`proxy_set_header\\s+X-Thoth-${escaped(publicName)}\\s+"";`, "g")) || []; - if (publicClears.length < 2) throw new Error(`Nginx proxy does not clear inbound ${label} identity`); + const publicClear = new RegExp(`proxy_set_header\\s+X-Thoth-${escaped(publicName)}\\s+"";`); const trustedHeader = `X-Thoth-Trusted-${trustedName}`; - if (!new RegExp(`proxy_set_header\\s+${escaped(trustedHeader)}\\s+"";`).test(block)) { - throw new Error(`Nginx proxy does not clear inbound trusted ${label} identity`); + const trustedClear = new RegExp(`proxy_set_header\\s+${escaped(trustedHeader)}\\s+"";`); + const authPublicAt = authLocation.search(publicClear); + const authTrustedAt = authLocation.search(trustedClear); + if (authPublicAt < 0) { + throw new Error(`Nginx auth location does not clear inbound ${label} identity`); } - if (!new RegExp(`auth_request_set\\s+\\$${variable}\\s+\\$upstream_http_${upstream};`, "m").test(block.replace(/\s+/g, " "))) { - throw new Error(`Nginx proxy does not capture authenticated ${label} identity`); + if (authTrustedAt < 0) { + throw new Error(`Nginx auth location does not clear inbound trusted ${label} identity`); } - if (!new RegExp(`proxy_set_header\\s+${escaped(trustedHeader)}\\s+\\$${variable};`).test(block)) { - throw new Error(`Nginx proxy does not map authenticated ${label} identity`); + const frontendPublicAt = frontendLocation.search(publicClear); + if (frontendPublicAt < 0) { + throw new Error(`Nginx frontend location does not clear inbound ${label} identity`); + } + const normalizedFrontend = frontendLocation.replace(/\s+/g, " "); + const captureAt = normalizedFrontend.search(new RegExp(`auth_request_set\\s+\\$${variable}\\s+\\$upstream_http_${upstream};`)); + if (captureAt < 0) { + throw new Error(`Nginx frontend location does not capture authenticated ${label} identity`); + } + const mapAt = normalizedFrontend.search(new RegExp(`proxy_set_header\\s+${escaped(trustedHeader)}\\s+\\$${variable};`)); + if (mapAt < 0) { + throw new Error(`Nginx frontend location does not map authenticated ${label} identity`); + } + if (new RegExp(`auth_request_set\\s+\\$${variable}|proxy_set_header\\s+${escaped(trustedHeader)}\\s+\\$${variable};`).test(authLocation)) { + throw new Error(`Nginx auth location performs a forbidden ${label} capture or mapping`); } } NODE @@ -706,26 +737,148 @@ if (/127\.0\.0\.1:8787|\bcore:8787\b/.test(block) || !block.includes("127.0.0.1: for (const token of tokens) { if (!block.includes(token)) throw new Error(`Caddy proxy lacks structural token: ${token}`); } +function directiveBlock(text, marker) { + const start = text.indexOf(marker); + if (start < 0) throw new Error(`Caddy proxy lacks scoped block: ${marker}`); + const opening = text.indexOf("{", start); + let depth = 0; + for (let index = opening; index < text.length; index++) { + if (text[index] === "{") depth++; + if (text[index] === "}" && --depth === 0) return {start, end: index, body: text.slice(opening + 1, index)}; + } + throw new Error(`Caddy proxy has unterminated scoped block: ${marker}`); +} +const route = directiveBlock(block, "route {"); +const forward = directiveBlock(route.body, "forward_auth auth-gateway:4180 {"); +const forwardAt = route.body.indexOf("forward_auth auth-gateway:4180 {"); for (const [label, publicName, trustedName] of [ ["issuer", "X-Thoth-Principal-Issuer", "X-Thoth-Trusted-Principal-Issuer"], ["subject", "X-Thoth-Principal-Subject", "X-Thoth-Trusted-Principal-Subject"], ["display", "X-Thoth-Principal-Display-Name", "X-Thoth-Trusted-Principal-Display-Name"], ["admin", "X-Thoth-Is-Admin", "X-Thoth-Trusted-Is-Admin"], ]) { - if (!block.includes(`request_header -${publicName}`)) { + const publicClearAt = route.body.indexOf(`request_header -${publicName}`); + if (publicClearAt < 0) { throw new Error(`Caddy proxy does not clear inbound ${label} identity`); } - if (!block.includes(`request_header -${trustedName}`)) { + const trustedClearAt = route.body.indexOf(`request_header -${trustedName}`); + if (trustedClearAt < 0) { throw new Error(`Caddy proxy does not clear inbound trusted ${label} identity`); } - if (!block.includes(`${publicName}>${trustedName}`)) { + if (publicClearAt > forwardAt || trustedClearAt > forwardAt) { + throw new Error("Caddy identity clears must precede forward_auth"); + } + if (!forward.body.includes(`${publicName}>${trustedName}`)) { throw new Error(`Caddy proxy does not map authenticated ${label} identity`); } } +const outsideForward = route.body.slice(0, forward.start) + route.body.slice(forward.end + 1); +if (/X-Thoth-(?:Principal-[^\s>]+|Is-Admin)>X-Thoth-Trusted-/.test(outsideForward)) { + throw new Error("Caddy maps identity outside the authenticated response stage"); +} NODE echo "Caddy reverse-proxy guide contract passed" } +verify_caddy_adapted_identity_order() { + local adapted="$1" + node - "$adapted" <<'NODE' +const fs = require("fs"); +const document = JSON.parse(fs.readFileSync(process.argv[2], "utf8")); +const publicHeaders = [ + "X-Thoth-Principal-Issuer", "X-Thoth-Principal-Subject", + "X-Thoth-Principal-Display-Name", "X-Thoth-Is-Admin", +]; +const trustedHeaders = [ + "X-Thoth-Trusted-Principal-Issuer", "X-Thoth-Trusted-Principal-Subject", + "X-Thoth-Trusted-Principal-Display-Name", "X-Thoth-Trusted-Is-Admin", +]; +function authUpstream(handler) { + return handler?.handler === "reverse_proxy" && + (handler.upstreams || []).some((upstream) => upstream.dial === "auth-gateway:4180"); +} +function frontendUpstream(handler) { + return handler?.handler === "reverse_proxy" && + (handler.upstreams || []).some((upstream) => upstream.dial === "127.0.0.1:8080"); +} +function findHandlerArray(value) { + if (!value || typeof value !== "object") return null; + if (Array.isArray(value)) { + if (value.some(authUpstream) && value.some(frontendUpstream)) return value; + for (const child of value) { + const found = findHandlerArray(child); + if (found) return found; + } + return null; + } + for (const child of Object.values(value)) { + const found = findHandlerArray(child); + if (found) return found; + } + return null; +} +function collectTrustedSets(value, collected = new Map()) { + if (!value || typeof value !== "object") return collected; + if (value.handler === "headers") { + for (const [name, replacement] of Object.entries(value.request?.set || {})) { + if (trustedHeaders.includes(name)) collected.set(name, replacement); + } + } + for (const child of Object.values(value)) collectTrustedSets(child, collected); + return collected; +} +const handlers = findHandlerArray(document); +if (!handlers) throw new Error("Caddy adapted config lacks the ordered auth/frontend handler chain"); +const authAt = handlers.findIndex(authUpstream); +const frontendAt = handlers.findIndex(frontendUpstream); +if (authAt < 0 || frontendAt <= authAt) throw new Error("Caddy adapted auth/frontend handler order is invalid"); +const expectedClears = [...publicHeaders, ...trustedHeaders]; +for (const header of expectedClears) { + const clearAt = handlers.findIndex((handler) => + handler?.handler === "headers" && (handler.request?.delete || []).includes(header)); + if (clearAt < 0 || clearAt >= authAt) { + throw new Error("Caddy adapted identity clears must execute before authentication"); + } +} +const auth = handlers[authAt]; +const successResponse = (auth.handle_response || []).find((response) => + (response.match?.status_code || []).map(Number).includes(2)); +if (!successResponse) throw new Error("Caddy adapted identity mapping is not restricted to auth 2xx"); +const mappings = collectTrustedSets(successResponse); +for (let index = 0; index < trustedHeaders.length; index++) { + const replacement = mappings.get(trustedHeaders[index]); + const expected = `{http.reverse_proxy.header.${publicHeaders[index]}}`; + if (!Array.isArray(replacement) || replacement.length !== 1 || replacement[0] !== expected) { + throw new Error(`Caddy adapted authenticated mapping is invalid for ${trustedHeaders[index]}`); + } +} +for (let index = 0; index < handlers.length; index++) { + if (index === authAt) continue; + if (collectTrustedSets(handlers[index]).size !== 0) { + throw new Error("Caddy adapted config maps trusted identity outside auth success"); + } +} +NODE +} + +verify_caddy_effective_proxy_guide() { + local adapted + adapted="$(mktemp "${TMPDIR:-/tmp}/thoth-caddy-adapted.XXXXXX")" + if ! awk ' + /^```caddyfile$/ { code=1; next } + code && /^```$/ { exit } + code { print } + ' "$root/docs/install/reverse-proxy-caddy.md" \ + | docker run --rm -i caddy:2.10.2-alpine caddy adapt --config - --adapter caddyfile >"$adapted"; then + rm -f "$adapted" + echo "Caddy documented configuration could not be adapted" >&2 + return 1 + fi + verify_caddy_adapted_identity_order "$adapted" + rm -f "$adapted" + echo "Caddy adapted trust-stage contract passed" +} + verify_manual() { local profile="$1" manual manual="$root/docs/install/$profile-workspace-registry.md" @@ -1236,6 +1389,8 @@ case "$mode" in verify_server_guide verify_reverse_proxy_nginx_guide verify_reverse_proxy_caddy_guide + verify_caddy_effective_proxy_guide + "$root/scripts/test-server-operator-permissions.sh" verify_server_installation_example fi verify_manual "$profile" diff --git a/tools/thothctl/cmd/thothctl/main.go b/tools/thothctl/cmd/thothctl/main.go index 72b0ef10..e0fb69dd 100644 --- a/tools/thothctl/cmd/thothctl/main.go +++ b/tools/thothctl/cmd/thothctl/main.go @@ -172,7 +172,14 @@ func writeRemovalTargets(outputWriter io.Writer, project string, targets []serve } func serverOperationFailure(stderr io.Writer, err error, secretValues []string) int { - fmt.Fprintf(stderr, "thothctl: %s\n", output.Sanitize(err.Error(), secretValues)) + message := output.Sanitize(err.Error(), secretValues) + var operationErr *serverops.OperationError + if errors.As(err, &operationErr) && operationErr.Detail() != "" { + detail := output.Sanitize(operationErr.Detail(), secretValues) + fmt.Fprintf(stderr, "thothctl: %s: %s\n", message, detail) + } else { + fmt.Fprintf(stderr, "thothctl: %s\n", message) + } if errors.Is(err, serverops.ErrConfirmationRequired) || errors.Is(err, serverops.ErrUnsafeState) { return 2 } diff --git a/tools/thothctl/cmd/thothctl/main_test.go b/tools/thothctl/cmd/thothctl/main_test.go index 3bd80257..cbdeba96 100644 --- a/tools/thothctl/cmd/thothctl/main_test.go +++ b/tools/thothctl/cmd/thothctl/main_test.go @@ -104,6 +104,36 @@ func TestRunSessionsMigrateRequiresExplicitConfirmationBeforeDocker(t *testing.T assertDockerNotInvoked(t, fixture) } +func TestRunSessionsMigrateReportsSanitizedStageAndExitClass(t *testing.T) { + fixture := newCLIFixture(t, "MIGRATION_PASSWORD_FILE=%s\n") + fixture.setProfile(t, "server") + secretPath := filepath.Join(fixture.root, "migration-password") + if err := os.WriteFile(secretPath, []byte("migration-secret-value"), 0o600); err != nil { + t.Fatal(err) + } + fixture.setEnvironment(t, secretPath) + t.Setenv("THOTHCTL_FAKE_CONFIG", `{"services":{"core":{"image":"thothii-core:local"},"session-migrate":{"image":"thothii-core:local"}}}`) + t.Setenv("THOTHCTL_FAKE_MIGRATION_FAILURE", "TLS rejected migration-secret-value") + t.Setenv("THOTHCTL_FAKE_MIGRATION_EXIT", "23") + var stdout, stderr bytes.Buffer + + code := run(context.Background(), []string{ + "--installation", fixture.installationPath, "sessions", "migrate", "--yes", + }, &stdout, &stderr) + + if code != 1 { + t.Fatalf("exit = %d, stderr = %q", code, stderr.String()) + } + for _, required := range []string{"stage=session-migration", "class=nonzero-exit", "TLS rejected [REDACTED]"} { + if !strings.Contains(stderr.String(), required) { + t.Errorf("stderr = %q, missing %q", stderr.String(), required) + } + } + if strings.Contains(stderr.String(), "migration-secret-value") { + t.Fatalf("stderr exposed secret: %q", stderr.String()) + } +} + func TestRunRemoveDisplaysExactInstallationTargetsBeforeConfirmation(t *testing.T) { fixture := newCLIFixture(t, "") fixture.setProfile(t, "server") @@ -660,7 +690,18 @@ printf '%s\n' "$@" >> "$THOTHCTL_FAKE_ARGS" printf '%s\n' -- >> "$THOTHCTL_FAKE_ARGS" case " $* " in *" ps --all --format json core frontend "*) printf '%s\n' "${THOTHCTL_FAKE_STOPPED_PS:-[]}" ;; - *" config --format json "*) printf '%s\n' '{"volumes":{"settings":{}},"services":{"core":{"image":"thothii-core:local","environment":{"THT_LLM_URL":"https://llm.example.invalid"}}}}' ;; + *" config --format json "*) + if [ -n "${THOTHCTL_FAKE_CONFIG:-}" ]; then + printf '%s\n' "$THOTHCTL_FAKE_CONFIG" + else + printf '%s\n' '{"volumes":{"settings":{}},"services":{"core":{"image":"thothii-core:local","environment":{"THT_LLM_URL":"https://llm.example.invalid"}}}}' + fi ;; + *" run --rm --no-deps --no-TTY session-migrate "*) + if [ "${THOTHCTL_FAKE_MIGRATION_EXIT:-0}" -ne 0 ]; then + printf '%s\n' "$THOTHCTL_FAKE_MIGRATION_FAILURE" >&2 + exit "$THOTHCTL_FAKE_MIGRATION_EXIT" + fi + printf '%s\n' '{"applied":[],"drifted":[],"pending":[]}' ;; *" ps --format json "*) printf '%s\n' '[{"Service":"core","State":"running","Health":"healthy"},{"Service":"frontend","State":"running","Health":"healthy"}]' ;; *"io.thothii.pi.version"*) printf '%s\n' '0.80.3' ;; *"PI_VERSION"*) printf '%s\n' '0.80.3' ;; @@ -707,6 +748,9 @@ func (f cliFixture) setEnvContents(t *testing.T, env string) { t.Setenv("THOTHCTL_FAKE_FAILURE", "") t.Setenv("THOTHCTL_FAKE_FAIL_ON", "") t.Setenv("THOTHCTL_FAKE_STOPPED_PS", "[]") + t.Setenv("THOTHCTL_FAKE_CONFIG", "") + t.Setenv("THOTHCTL_FAKE_MIGRATION_FAILURE", "") + t.Setenv("THOTHCTL_FAKE_MIGRATION_EXIT", "0") } func (f cliFixture) setProfile(t *testing.T, profile string) { diff --git a/tools/thothctl/internal/serverops/operations.go b/tools/thothctl/internal/serverops/operations.go index e7e2c8a8..ed2d77ff 100644 --- a/tools/thothctl/internal/serverops/operations.go +++ b/tools/thothctl/internal/serverops/operations.go @@ -25,6 +25,50 @@ type Runner interface { Run(context.Context, []string, io.Reader) (compose.Result, error) } +type Stage string + +const ( + StageContainerInspection Stage = "container-inspection" + StageMigrationConfig Stage = "migration-config" + StageMigrationVerification Stage = "migration-config-verification" + StageSessionMigration Stage = "session-migration" + StageContainerRemoval Stage = "container-removal" + StageRemovalVerification Stage = "removal-verification" +) + +type ExitClass string + +const ( + ExitClassNonzero ExitClass = "nonzero-exit" + ExitClassUnavailable ExitClass = "unavailable" + ExitClassTimeout ExitClass = "timeout" + ExitClassInvocation ExitClass = "invocation-failure" +) + +// OperationError reports only allowlisted operation metadata from Error. Detail remains bounded +// and is exposed separately so the CLI can redact declared secrets before displaying it. +type OperationError struct { + stage Stage + class ExitClass + detail string +} + +func (e *OperationError) Error() string { + return fmt.Sprintf("stage=%s class=%s", e.stage, e.class) +} + +func (e *OperationError) Stage() Stage { + return e.stage +} + +func (e *OperationError) Class() ExitClass { + return e.class +} + +func (e *OperationError) Detail() string { + return e.detail +} + type MigrationStatus struct { Applied []string `json:"applied"` Drifted []string `json:"drifted"` @@ -51,7 +95,7 @@ func MigrateSessions(ctx context.Context, installation config.Installation, runn if installation.Profile != "server" { return MigrationStatus{}, fmt.Errorf("%w: session migration requires a server installation", ErrUnsafeState) } - containers, err := inspectContainers(ctx, installation, runner) + containers, err := inspectContainers(ctx, installation, runner, StageContainerInspection) if err != nil { return MigrationStatus{}, err } @@ -59,7 +103,7 @@ func MigrateSessions(ctx context.Context, installation config.Installation, runn return MigrationStatus{}, err } - rendered, err := runCompose(ctx, runner, installation.ComposeArgs("--profile", "session-migrate", "config", "--format", "json")) + rendered, err := runCompose(ctx, runner, StageMigrationConfig, installation.ComposeArgs("--profile", "session-migrate", "config", "--format", "json")) if err != nil { return MigrationStatus{}, err } @@ -77,7 +121,7 @@ func MigrateSessions(ctx context.Context, installation config.Installation, runn if err != nil { return MigrationStatus{}, err } - finalConfig, err := runCompose(ctx, runner, configArgs) + finalConfig, err := runCompose(ctx, runner, StageMigrationVerification, configArgs) if err != nil { return MigrationStatus{}, err } @@ -90,7 +134,7 @@ func MigrateSessions(ctx context.Context, installation config.Installation, runn if err != nil { return MigrationStatus{}, err } - result, err := runCompose(ctx, runner, runArgs) + result, err := runCompose(ctx, runner, StageSessionMigration, runArgs) if err != nil { return MigrationStatus{}, err } @@ -110,7 +154,7 @@ func Remove(ctx context.Context, installation config.Installation, runner Runner if installation.Profile != "server" { return RemovalResult{}, fmt.Errorf("%w: removal requires a server installation", ErrUnsafeState) } - targets, err := inspectContainers(ctx, installation, runner) + targets, err := inspectContainers(ctx, installation, runner, StageContainerInspection) result := RemovalResult{Targets: targets} if err != nil { return result, err @@ -137,11 +181,11 @@ func Remove(ctx context.Context, installation config.Installation, runner Runner for _, target := range targets { args = append(args, target.ID) } - if _, err := runDocker(ctx, runner, args); err != nil { + if _, err := runDocker(ctx, runner, StageContainerRemoval, args); err != nil { return result, err } } - remaining, err := inspectContainers(ctx, installation, runner) + remaining, err := inspectContainers(ctx, installation, runner, StageRemovalVerification) if err != nil { return result, err } @@ -177,8 +221,8 @@ func sameTargetIDs(targets []Container, confirmed []string) bool { return true } -func inspectContainers(ctx context.Context, installation config.Installation, runner Runner) ([]Container, error) { - result, err := runCompose(ctx, runner, installation.ComposeArgs("ps", "--all", "--format", "json", "core", "frontend")) +func inspectContainers(ctx context.Context, installation config.Installation, runner Runner, stage Stage) ([]Container, error) { + result, err := runCompose(ctx, runner, stage, installation.ComposeArgs("ps", "--all", "--format", "json", "core", "frontend")) if err != nil { return nil, err } @@ -320,14 +364,44 @@ func verifySnapshots(snapshots []pathSnapshot) error { return nil } -func runCompose(ctx context.Context, runner Runner, args []string) (compose.Result, error) { - return runDocker(ctx, runner, args) +func runCompose(ctx context.Context, runner Runner, stage Stage, args []string) (compose.Result, error) { + return runDocker(ctx, runner, stage, args) } -func runDocker(ctx context.Context, runner Runner, args []string) (compose.Result, error) { +func runDocker(ctx context.Context, runner Runner, stage Stage, args []string) (compose.Result, error) { result, err := runner.Run(ctx, args, nil) if err != nil { - return result, errors.New("Docker operation failed") + class := ExitClassInvocation + switch { + case errors.Is(ctx.Err(), context.DeadlineExceeded): + class = ExitClassTimeout + case result.ExitCode == 127: + class = ExitClassUnavailable + case result.ExitCode != 0: + class = ExitClassNonzero + } + detail := result.Stderr + if strings.TrimSpace(detail) == "" { + detail = err.Error() + } + return result, &OperationError{stage: stage, class: class, detail: boundedDetail(detail)} } return result, nil } + +func boundedDetail(detail string) string { + detail = strings.Join(strings.Fields(detail), " ") + const maximumBytes = 512 + if len(detail) <= maximumBytes { + return detail + } + var bounded strings.Builder + for _, character := range detail { + encoded := string(character) + if bounded.Len()+len(encoded) > maximumBytes { + break + } + bounded.WriteString(encoded) + } + return bounded.String() +} diff --git a/tools/thothctl/internal/serverops/operations_test.go b/tools/thothctl/internal/serverops/operations_test.go index a8e68c21..11126a9f 100644 --- a/tools/thothctl/internal/serverops/operations_test.go +++ b/tools/thothctl/internal/serverops/operations_test.go @@ -131,6 +131,41 @@ func TestMigrateSessionsFailsClosedBeforeMutation(t *testing.T) { } } +func TestMigrateSessionsReturnsTypedBoundedRunFailure(t *testing.T) { + installation := testInstallation(t) + secret := "database-password-in-stderr" + configCalls := 0 + runner := &fakeRunner{run: func(args []string) (compose.Result, error) { + switch { + case contains(args, "ps", "--all"): + return compose.Result{Stdout: `[]`}, nil + case contains(args, "config", "--format", "json"): + configCalls++ + return compose.Result{Stdout: `{"services":{"core":{"image":"thothii-core:local"},"session-migrate":{"image":"thothii-core:local"}}}`}, nil + case contains(args, "run", "--rm", "--no-deps", "--no-TTY", "session-migrate"): + return compose.Result{Stderr: "TLS connection for " + secret + ": " + strings.Repeat("x", 2048), ExitCode: 23}, errors.New("exit status 23") + default: + t.Fatalf("unexpected Docker invocation: %#v", args) + return compose.Result{}, nil + } + }} + + _, err := MigrateSessions(context.Background(), installation, runner, true) + var operationErr *OperationError + if !errors.As(err, &operationErr) { + t.Fatalf("MigrateSessions() error = %T %v, want OperationError", err, err) + } + if operationErr.Stage() != StageSessionMigration || operationErr.Class() != ExitClassNonzero { + t.Fatalf("operation error = %#v", operationErr) + } + if detail := operationErr.Detail(); !strings.Contains(detail, secret) || len(detail) > 512 { + t.Fatalf("bounded detail length=%d value=%q", len(detail), detail) + } + if configCalls != 2 { + t.Fatalf("config calls = %d", configCalls) + } +} + func TestRemovePreservesEveryDeclaredBindSecretAndBackup(t *testing.T) { installation, preserved := removalInstallation(t) psCalls := 0