diff --git a/.env.example b/.env.example index a22867ef24..b1ec7fb6c8 100644 --- a/.env.example +++ b/.env.example @@ -258,6 +258,12 @@ LOOPOVER_REVIEW_DRAFT=false # # for the write tools specifically, flipping the config bind mount in # # docker-compose.yml from :ro to :rw yourself -- see that file's # # comment above the mount. See self-hosting-configuration.mdx. +# REDEPLOY_COMPANION_SOCKET_PATH=/run/loopover-redeploy.sock # filesystem path to the host redeploy companion's +# # Unix domain socket (systemd/loopover-redeploy-companion.service.example), +# # bind-mounted into this container at the SAME path. Only matters if +# # you're using loopover_admin_trigger_redeploy (#7723); also requires +# # REDEPLOY_COMPANION_TOKEN_FILE (secrets/README.md) to actually +# # authenticate. See self-hosting-configuration.mdx. # COMPOSE_PROJECT_NAME=loopover # Docker Compose's own project name; also labels the log stream # # Promtail ships to Loki. Change it to run two stacks on one host # # (#4896) -- Compose namespaces container names, named volumes, and diff --git a/apps/loopover-ui/content/docs/self-hosting-configuration.mdx b/apps/loopover-ui/content/docs/self-hosting-configuration.mdx index 91071cda13..3d83063aba 100644 --- a/apps/loopover-ui/content/docs/self-hosting-configuration.mdx +++ b/apps/loopover-ui/content/docs/self-hosting-configuration.mdx @@ -345,7 +345,49 @@ Restart the `loopover` service after changing either (`docker compose up -d --no Drop `dryRun` (or set it to `false`) to write for real once you're happy with the dry-run result. `scope` is `"global"` (the mount-root default file) or `"repo"` (pass `repoFullName`); `loopover_admin_get_config` additionally accepts `"effective"` to read the exact deep-merged view a real review sees, same as the "Private per-repo config" deep-merge described above. `loopover_admin_list_config_backups` takes the same `scope`/`repoFullName` pair and returns each backup's path and timestamp, newest first. - These tools only read and write `LOOPOVER_REPO_CONFIG_DIR`. They do not trigger a redeploy, and they do not touch the public dashboard or `/v1/app/*` settings surface — `LOOPOVER_MCP_ADMIN_TOKEN` cannot sign into the control panel or call the routes `ADMIN_GITHUB_LOGINS` gates. + These three tools only read and write `LOOPOVER_REPO_CONFIG_DIR`. Triggering a redeploy is a separate tool with its own separate token — see "MCP redeploy trigger" below. None of the admin tools touch the public dashboard or `/v1/app/*` settings surface — `LOOPOVER_MCP_ADMIN_TOKEN` cannot sign into the control panel or call the routes `ADMIN_GITHUB_LOGINS` gates. + + +## MCP redeploy trigger + +A fourth `admin`-category tool, `loopover_admin_trigger_redeploy` (#7723): pulls the published image, restarts this instance, and waits for it to report healthy — the same sequence `./scripts/deploy-selfhost-image.sh` runs by hand, triggered remotely through an MCP client instead. Same `LOOPOVER_MCP_ADMIN_ENABLED` + `LOOPOVER_MCP_ADMIN_TOKEN` gating as the config tools above, plus one more layer specific to this tool. + + + Every other admin tool runs entirely inside this container. Triggering a redeploy can't: this container has to end up running a *new* image, which means something *outside* this container has to do the actual `docker compose pull && up -d`. The obvious shortcut — mount `/var/run/docker.sock` into this container so it can ask Docker to do it — is explicitly the thing this repo tells contributors never to do (see `docker-proxy`'s own comment in `docker-compose.yml`: a bind-mounted socket, even read-only, is "effectively host root"; `review-enrichment/src/analyzers/iac-misconfig.ts` flags exactly this pattern in the PRs this bot reviews for everyone else). Extending the existing `docker-proxy` sidecar (`tecnativa/docker-socket-proxy`) doesn't avoid the problem either — its access-control model has no way to scope a restart to *one* named container, only "any container reachable through the socket," and it has no concept of "pull a new image" at all (that's compose-level orchestration, not a single Docker Engine API call). + + The design here instead runs a small **host companion** — a plain Node process, outside Docker entirely, installed as a systemd service (`systemd/loopover-redeploy-companion.service.example`) — that listens on a Unix domain socket and, on an authenticated request, shells out to the real `deploy-selfhost-image.sh`. This container reaches it via a narrow, purpose-built socket bind-mounted in (never the Docker socket itself), authenticated with its own separate shared secret (`REDEPLOY_COMPANION_TOKEN`) so a leaked `LOOPOVER_MCP_ADMIN_TOKEN` alone still can't trigger a redeploy — two independent credentials have to both be compromised, not one. + + +**Entirely opt-in.** Skip this whole section and the other three admin tools work exactly as documented above — `loopover_admin_trigger_redeploy` just reports `configured: false` until you set it up. + +/dev/null +sudo cp systemd/loopover-redeploy-companion.service.example /etc/systemd/system/loopover-redeploy-companion.service +# edit User/Group/WorkingDirectory in that file to match your host, then: +sudo systemctl daemon-reload +sudo systemctl enable --now loopover-redeploy-companion.service`} +/> + +The `loopover` service's `docker-compose.yml` entry already bind-mounts the companion's socket and reads `REDEPLOY_COMPANION_TOKEN_FILE` — nothing to add there. Restart the `loopover` service once the companion is running so it picks up the token. + + + +`image` is optional — omit it to redeploy whatever `LOOPOVER_IMAGE` is already configured (the same default `deploy-selfhost-image.sh` itself uses). The tool call doesn't return until the companion's own health-wait finishes (or times out), and streams back every log line the real script printed along the way, so a failed redeploy comes back with the actual reason, not just a bare non-zero exit code. + + + A successful redeploy means this container gets replaced mid-request. The companion waits for the *new* container to report healthy before it responds, so the tool call itself completes normally either way — but expect the connection to briefly drop if you're also watching logs live. ## Config-as-code blocks with no dashboard equivalent diff --git a/apps/loopover-ui/content/docs/self-hosting-security.mdx b/apps/loopover-ui/content/docs/self-hosting-security.mdx index c066ec3ccc..00b28e3627 100644 --- a/apps/loopover-ui/content/docs/self-hosting-security.mdx +++ b/apps/loopover-ui/content/docs/self-hosting-security.mdx @@ -26,7 +26,7 @@ eyebrow: Self-hosting ]} /> -`docker-compose.yml` ships native Docker Compose `secrets:` mounts for the highest-value secrets (the GitHub App private key, webhook secret, API/MCP/MCP-admin/internal-job tokens, the setup token, the two token-encryption master keys, the Orb enrollment secret, the PagerDuty routing key, and the Claude Code subscription token) — file-mounted at `/run/secrets/`, never exposed via `docker inspect` or `docker compose config` the way a plain `environment:`/`env_file` value is. This is purely additive: an inline `.env` value always takes priority if you set both, so you can migrate one secret at a time, or not at all. See `secrets/README.md` for the full file list. +`docker-compose.yml` ships native Docker Compose `secrets:` mounts for the highest-value secrets (the GitHub App private key, webhook secret, API/MCP/MCP-admin/redeploy-companion/internal-job tokens, the setup token, the two token-encryption master keys, the Orb enrollment secret, the PagerDuty routing key, and the Claude Code subscription token) — file-mounted at `/run/secrets/`, never exposed via `docker inspect` or `docker compose config` the way a plain `environment:`/`env_file` value is. This is purely additive: an inline `.env` value always takes priority if you set both, so you can migrate one secret at a time, or not at all. See `secrets/README.md` for the full file list. (Compose's default target). See the top-level `secrets:` # block above and secrets/README.md. # @@ -182,6 +196,7 @@ services: - loopover_api_token - loopover_mcp_token - loopover_mcp_admin_token + - redeploy_companion_token - internal_job_token - selfhost_setup_token - token_encryption_secret @@ -1339,6 +1354,8 @@ secrets: file: ./secrets/loopover_mcp_token.txt loopover_mcp_admin_token: file: ./secrets/loopover_mcp_admin_token.txt + redeploy_companion_token: + file: ./secrets/redeploy_companion_token.txt internal_job_token: file: ./secrets/internal_job_token.txt selfhost_setup_token: diff --git a/scripts/deploy-selfhost-image.sh b/scripts/deploy-selfhost-image.sh index c3b9b87fe0..2bf7a7b3a3 100755 --- a/scripts/deploy-selfhost-image.sh +++ b/scripts/deploy-selfhost-image.sh @@ -46,8 +46,8 @@ validate_inputs() { exit 1 fi case "$image" in - *[[:space:]\"\'\\\$\{\}]*) - echo "error: image contains unsupported whitespace, quote, backslash, or compose interpolation characters" >&2 + *[[:space:]\"\'\\\$\{\}\`\;\|\&\<\>]*) + echo "error: image contains unsupported whitespace, quote, backslash, compose interpolation, or shell metacharacters" >&2 exit 1 ;; esac diff --git a/scripts/redeploy-companion.ts b/scripts/redeploy-companion.ts new file mode 100644 index 0000000000..5dc1088b80 --- /dev/null +++ b/scripts/redeploy-companion.ts @@ -0,0 +1,180 @@ +#!/usr/bin/env node +// Host-side redeploy companion (#7723, sub-issue of #7720): a narrow, purpose-built listener that lets the +// `loopover` app container trigger a real redeploy of itself WITHOUT ever mounting /var/run/docker.sock into +// an app-facing container -- this repo's own docker-proxy service (docker-compose.yml) already documents why +// that's unacceptable ("a :ro socket bind-mount only protects the socket inode, it does NOT restrict the +// Docker API... effectively host root"), and review-enrichment/src/analyzers/iac-misconfig.ts flags exactly +// this pattern as an IaC misconfiguration finding in the PRs this bot reviews for everyone else. +// +// DESIGN, chosen over extending docker-proxy (tecnativa/docker-socket-proxy): that image's ACL model is +// "which API section/verb is enabled globally," with no way to scope a restart to ONE named container -- the +// best it offers is "restart-only, for any container reachable through the socket," broader than intended. +// Worse, the real desired behavior (pull a new image, recreate the container, wait for health) is +// docker-compose-level orchestration, not a single Docker Engine API call a raw proxy could cleanly +// allowlist. This companion instead runs entirely OUTSIDE Docker (a systemd service on the host) and calls +// through the EXACT existing, already-tested scripts/deploy-selfhost-image.sh -- not a reimplementation of +// its pull+recreate+health-wait sequence. +// +// PROTOCOL: listens on a Unix domain socket, NOT a TCP port -- reachable only via a filesystem path, which is +// what gets bind-mounted (read-write) into the `loopover` container at the SAME path, rather than opening any +// network-level attack surface. One line-delimited JSON request per connection: +// {"token": "", "image"?: ""} +// The token is a SEPARATE credential from LOOPOVER_MCP_ADMIN_TOKEN (defense in depth: the MCP-layer gate in +// src/mcp/server.ts is the first check, this is the second, independent one on the host side -- a bug or +// misconfiguration in one layer doesn't strand the other). Streams `{"log": "..."}` lines for each line of the +// real script's stdout/stderr, then exactly one terminal `{"ok": true, "exitCode": 0}` or +// `{"ok": false, "exitCode": N, "error"?: "..."}`, and closes the connection. +// +// CONCURRENCY: rejects a second redeploy request while one is already running (`{"ok": false, "error": +// "redeploy_already_in_progress"}`) -- two concurrent `docker compose pull && up -d` calls against the same +// service is a real race (image tag resolution, container recreation), not just wasted work. +import { createServer } from "node:net"; +import { spawn } from "node:child_process"; +import { timingSafeEqual } from "node:crypto"; +import { unlinkSync, chmodSync, existsSync } from "node:fs"; + +const SOCKET_PATH = process.env.REDEPLOY_COMPANION_SOCKET_PATH?.trim() || "/run/loopover-redeploy.sock"; +const REPO_ROOT = process.env.REDEPLOY_COMPANION_REPO_ROOT?.trim() || process.cwd(); +const MAX_REQUEST_BYTES = 4096; + +function requireToken(): string { + const token = process.env.REDEPLOY_COMPANION_TOKEN?.trim(); + if (!token) { + console.error(JSON.stringify({ level: "error", event: "redeploy_companion_missing_token" })); + process.exit(1); + } + return token; +} + +/** Constant-time comparison against the configured token -- a plain `===` would leak timing info about how + * many leading characters matched, letting an attacker on the same host incrementally guess the token. */ +function isValidToken(configuredToken: string, candidate: unknown): boolean { + if (typeof candidate !== "string") return false; + const configured = Buffer.from(configuredToken, "utf8"); + const supplied = Buffer.from(candidate, "utf8"); + if (configured.length !== supplied.length) return false; + return timingSafeEqual(configured, supplied); +} + +type RedeployRequest = { token: unknown; image?: unknown }; + +function parseRequestLine(line: string): RedeployRequest | null { + let parsed: unknown; + try { + parsed = JSON.parse(line); + } catch { + return null; + } + if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) return null; + return parsed as RedeployRequest; +} + +/** Same character class deploy-selfhost-image.sh's own `validate_inputs` enforces -- this is a second, + * independent guard on the value before it ever reaches that script, giving a caller a clear MCP-level + * rejection instead of an opaque host-side one. Not load-bearing against code execution on its own: `spawn` + * below never sets `shell: true`, so the image string always reaches deploy-selfhost-image.sh as a single, + * literal argv element regardless of its contents, and that script only ever references it through properly + * quoted expansions ("$1", "$IMAGE") -- confirmed empirically, not just by inspection: none of these + * characters are exploitable via the current call path. Rejected anyway because no legitimate Docker image + * reference ever needs whitespace, quotes, `$`/`{`/`}` (compose interpolation), or shell metacharacters like + * backticks/`;`/`|`/`&`/`<`/`>` -- costs nothing today and guards against either side of this call ever + * losing that quoting discipline later. */ +function isSafeImageOverride(value: unknown): value is string { + return typeof value === "string" && value.length > 0 && value.length <= 512 && !/[\s"'\\${}`;|&<>]/.test(value); +} + +export type RunDeployResult = { ok: boolean; exitCode: number | null; error?: string }; + +/** Runs the real, existing deploy-selfhost-image.sh -- never reimplemented here. Injectable spawn function for + * tests only; production always uses the real node:child_process.spawn. */ +export function runDeploy( + image: string | undefined, + onLog: (line: string) => void, + spawnImpl: typeof spawn = spawn, +): Promise { + return new Promise((resolve) => { + const args = image ? [image] : []; + const child = spawnImpl("bash", ["scripts/deploy-selfhost-image.sh", ...args], { cwd: REPO_ROOT, stdio: ["ignore", "pipe", "pipe"] }); + const forwardLines = (chunk: Buffer) => { + for (const line of chunk.toString("utf8").split("\n")) { + if (line.trim()) onLog(line); + } + }; + child.stdout?.on("data", forwardLines); + child.stderr?.on("data", forwardLines); + child.on("error", (error) => resolve({ ok: false, exitCode: null, error: error.message })); + child.on("close", (exitCode) => resolve({ ok: exitCode === 0, exitCode })); + }); +} + +/** One socket connection's full request/response lifecycle. Exported for direct unit testing without a real + * socket. `isBusy`/`setBusy` are the shared in-progress flag across every connection the server accepts. */ +export async function handleConnection( + configuredToken: string, + requestLine: string, + isBusy: () => boolean, + setBusy: (busy: boolean) => void, + write: (line: string) => void, + deploy: typeof runDeploy = runDeploy, +): Promise { + const request = parseRequestLine(requestLine); + if (!request || !isValidToken(configuredToken, request.token)) { + write(JSON.stringify({ ok: false, error: "unauthorized" })); + return; + } + if (isBusy()) { + write(JSON.stringify({ ok: false, error: "redeploy_already_in_progress" })); + return; + } + const image = isSafeImageOverride(request.image) ? request.image : undefined; + if (request.image !== undefined && image === undefined) { + write(JSON.stringify({ ok: false, error: "invalid_image_override" })); + return; + } + + setBusy(true); + try { + const result = await deploy(image, (line) => write(JSON.stringify({ log: line }))); + write(JSON.stringify(result.error ? { ok: result.ok, exitCode: result.exitCode, error: result.error } : { ok: result.ok, exitCode: result.exitCode })); + } finally { + setBusy(false); + } +} + +function main(): void { + const configuredToken = requireToken(); + let busy = false; + + if (existsSync(SOCKET_PATH)) unlinkSync(SOCKET_PATH); // a stale socket from an unclean prior shutdown + + const server = createServer((socket) => { + let buffer = ""; + socket.on("data", (chunk) => { + buffer += chunk.toString("utf8"); + if (buffer.length > MAX_REQUEST_BYTES) { + socket.end(`${JSON.stringify({ ok: false, error: "request_too_large" })}\n`); + return; + } + const newlineIndex = buffer.indexOf("\n"); + if (newlineIndex === -1) return; + const line = buffer.slice(0, newlineIndex); + void handleConnection( + configuredToken, + line, + () => busy, + (value) => { + busy = value; + }, + (out) => socket.write(`${out}\n`), + ).finally(() => socket.end()); + }); + socket.on("error", () => undefined); // a client disconnecting mid-request is not this process's problem + }); + + server.listen(SOCKET_PATH, () => { + chmodSync(SOCKET_PATH, 0o660); // owner+group only -- see the systemd unit's Group= for who that is + console.log(JSON.stringify({ level: "info", event: "redeploy_companion_listening", socketPath: SOCKET_PATH })); + }); +} + +if (process.argv[1] && import.meta.url === `file://${process.argv[1]}`) main(); diff --git a/scripts/selfhost-init-secrets.sh b/scripts/selfhost-init-secrets.sh index b0fb1b94f2..8093e37545 100755 --- a/scripts/selfhost-init-secrets.sh +++ b/scripts/selfhost-init-secrets.sh @@ -45,6 +45,7 @@ RANDOM_SECRET_FILES=( "loopover_api_token.txt" "loopover_mcp_token.txt" "loopover_mcp_admin_token.txt" + "redeploy_companion_token.txt" "internal_job_token.txt" "selfhost_setup_token.txt" "token_encryption_secret.txt" diff --git a/secrets/README.md b/secrets/README.md index 2e3a8160fd..1df8de9f5a 100644 --- a/secrets/README.md +++ b/secrets/README.md @@ -64,6 +64,7 @@ see the tradeoff explained above for why `600` breaks the app's own ability to r | `loopover_api_token.txt` | `LOOPOVER_API_TOKEN_FILE` | Server-to-server API bearer token. | | `loopover_mcp_token.txt` | `LOOPOVER_MCP_TOKEN_FILE` | Shared MCP bearer token. | | `loopover_mcp_admin_token.txt` | `LOOPOVER_MCP_ADMIN_TOKEN_FILE` | Higher-privilege MCP admin token (config read/write tools); inert unless `LOOPOVER_MCP_ADMIN_ENABLED` is also set. | +| `redeploy_companion_token.txt` | `REDEPLOY_COMPANION_TOKEN_FILE` | Shared secret with the host-side redeploy companion (`systemd/loopover-redeploy-companion.service.example`) -- copy the SAME value into that unit's `EnvironmentFile`, it is not two independent secrets. | | `internal_job_token.txt` | `INTERNAL_JOB_TOKEN_FILE` | Gates internal-only routes (e.g. `/v1/internal/*`). | | `selfhost_setup_token.txt` | `SELFHOST_SETUP_TOKEN_FILE` | Unlocks the first-run `/setup` wizard. | | `token_encryption_secret.txt` | `TOKEN_ENCRYPTION_SECRET_FILE` | AES-256-GCM master secret for maintainer BYOK keys at rest. | diff --git a/src/env.d.ts b/src/env.d.ts index 65cab3d7e0..fc26b0ffc5 100644 --- a/src/env.d.ts +++ b/src/env.d.ts @@ -295,6 +295,17 @@ declare global { * requires actor === "mcp-admin" (LOOPOVER_MCP_ADMIN_TOKEN), so enabling this flag alone grants nothing to * a caller using the ordinary LOOPOVER_MCP_TOKEN. */ LOOPOVER_MCP_ADMIN_ENABLED?: string; + /** Shared secret with the host-side redeploy companion (#7723, systemd/loopover-redeploy-companion.service.example) + * that the loopover_admin_trigger_redeploy MCP tool authenticates with -- a SEPARATE credential from + * LOOPOVER_MCP_ADMIN_TOKEN (defense in depth: the MCP-layer gate and the host-socket gate are two + * independent checks). Self-host only; unset means the tool reports "not configured" rather than + * attempting a connection. Must match the companion's own EnvironmentFile value exactly -- see + * secrets/README.md. */ + REDEPLOY_COMPANION_TOKEN?: string; + /** Filesystem path to the redeploy companion's Unix domain socket, bind-mounted into this container + * (docker-compose.yml). Default: /run/loopover-redeploy.sock -- override only if you changed the + * companion's own REDEPLOY_COMPANION_SOCKET_PATH to something else. */ + REDEPLOY_COMPANION_SOCKET_PATH?: string; INTERNAL_JOB_TOKEN: string; /** Repos the shared LOOPOVER_MCP_TOKEN may propose/decide/manage actions on (comma/whitespace `owner/repo` * list, or `*`/`all` for every repo). Unset ⇒ none — LOOPOVER_MCP_TOKEN is a shared, end-user-obtainable diff --git a/src/mcp/redeploy-companion-registry.ts b/src/mcp/redeploy-companion-registry.ts new file mode 100644 index 0000000000..dca8bf6d4d --- /dev/null +++ b/src/mcp/redeploy-companion-registry.ts @@ -0,0 +1,22 @@ +// Workers-safe registry for the redeploy-trigger capability (#7723), mirroring +// src/mcp/private-config-admin-registry.ts's setConfigAdminFunctions pattern exactly: this module holds a +// single nullable function slot and never imports node:net itself, so it's safe in the Cloudflare Workers +// bundle. Only the self-host Node entry (server.ts) fills the slot, with a real closure built from +// src/selfhost/redeploy-companion-client.ts -- that module's own node:net import never reaches the Workers +// bundle because nothing there imports it directly, only through this registry. +// Unset (cloud, or self-host without REDEPLOY_COMPANION_TOKEN/_SOCKET_PATH configured) means the function +// here stays null, and src/mcp/server.ts's admin tool -- gated separately on LOOPOVER_MCP_ADMIN_ENABLED -- +// reports a clear "not configured" result rather than throwing. +import type { RedeployResult } from "../selfhost/redeploy-companion-client.js"; + +export type RedeployTrigger = (image: string | undefined) => Promise; + +let triggerRedeploy: RedeployTrigger | null = null; + +export function setRedeployTrigger(trigger: RedeployTrigger | null): void { + triggerRedeploy = trigger; +} + +export function getRedeployTrigger(): RedeployTrigger | null { + return triggerRedeploy; +} diff --git a/src/mcp/server.ts b/src/mcp/server.ts index 02780a2e1d..f586783221 100644 --- a/src/mcp/server.ts +++ b/src/mcp/server.ts @@ -113,6 +113,7 @@ import { computeFleetAnalytics } from "../orb/analytics"; import { loadMaintainerNoiseReport, maintainerNoiseSummary } from "../services/maintainer-noise"; import { buildAmsMinerCohortComparison } from "../review/ams-miner-cohort"; import { getConfigAdminFunctions } from "./private-config-admin-registry"; +import { getRedeployTrigger } from "./redeploy-companion-registry"; import { getLocalManifestReader } from "../signals/focus-manifest-loader"; import type { ConfigAdminScope } from "../selfhost/private-config"; import { buildMaintainerActivationPreview } from "../services/maintainer-activation"; @@ -366,6 +367,18 @@ const adminListBackupsShape = { scope: z.enum(["global", "repo"]), repoFullName: z.string().min(3).max(200).optional(), }; +// #7723: image is intentionally the same character class deploy-selfhost-image.sh's own validate_inputs +// enforces (no whitespace/quote/backslash/compose-interpolation/shell-metacharacter chars) -- redundant with +// both that script's own check and the companion's own isSafeImageOverride, but a caller gets a clear +// MCP-level error instead of an opaque host-side rejection two hops away. +const adminTriggerRedeployShape = { + image: z + .string() + .min(1) + .max(512) + .regex(/^[^\s"'\\${}`;|&<>]+$/, "must not contain whitespace, quotes, backslashes, compose interpolation, or shell metacharacters") + .optional(), +}; const preflightShape = { repoFullName: z.string().min(3).max(PREFLIGHT_LIMITS.repoFullNameChars), @@ -1675,6 +1688,13 @@ const adminListBackupsOutputSchema = { .array(z.object({ name: z.string(), path: z.string(), mtimeMs: z.number() })) .optional(), }; +const adminTriggerRedeployOutputSchema = { + configured: z.boolean(), + ok: z.boolean().optional(), + exitCode: z.number().nullable().optional(), + log: z.array(z.string()).optional(), + error: z.string().optional(), +}; // #550: output schemas for the remaining tools (preflight/score/local-branch/agent), so MCP clients can // machine-validate their results. Same lenient style as the schemas above — documented top-level keys, // all optional, complex values as z.unknown(). No behavior change; these mirror the existing payloads. @@ -2046,6 +2066,7 @@ export const MCP_TOOL_CATEGORIES: Record = { loopover_admin_get_config: "admin", loopover_admin_write_config: "admin", loopover_admin_list_config_backups: "admin", + loopover_admin_trigger_redeploy: "admin", }; /** Master opt-in for the "admin" tool category (#7721), default OFF. Same truthy-string convention as every @@ -3148,6 +3169,16 @@ export class LoopoverMcp { }, async (input) => this.toolResult(await this.adminListConfigBackups(input)), ); + register( + "loopover_admin_trigger_redeploy", + { + description: + "Self-hosted-operator only. Trigger a real redeploy of this instance (pull the published image, restart, wait for health) via the host-side redeploy companion (#7723) -- NOT via the Docker socket, which is never mounted into this container. Optional `image` pins a specific tag/digest; omitted uses the companion's own default (the currently-configured LOOPOVER_IMAGE). Requires LOOPOVER_MCP_ADMIN_TOKEN. Returns configured=false if REDEPLOY_COMPANION_TOKEN is unset or the companion isn't reachable at REDEPLOY_COMPANION_SOCKET_PATH -- see systemd/loopover-redeploy-companion.service.example to set it up. A real redeploy restarts this very process; the tool call itself completes (with the companion's full log) before that restart happens, since the companion waits for the new container to report healthy before responding.", + inputSchema: adminTriggerRedeployShape, + outputSchema: adminTriggerRedeployOutputSchema, + }, + async (input) => this.toolResult(await this.adminTriggerRedeploy(input)), + ); } // ── Miner planning prompts ─────────────────────────────────────────── @@ -4009,6 +4040,33 @@ export class LoopoverMcp { }; } + private async adminTriggerRedeploy(input: { image?: string | undefined }): Promise { + this.requireMcpAdmin(); + const trigger = getRedeployTrigger(); + if (!trigger) { + return { + summary: "LoopOver redeploy trigger: not configured (REDEPLOY_COMPANION_TOKEN is unset on this instance, or the companion isn't installed).", + data: { configured: false }, + }; + } + try { + const result = await trigger(input.image); + return { + summary: result.ok + ? `LoopOver redeploy: completed successfully${input.image ? ` (${input.image})` : ""}.` + : `LoopOver redeploy failed (exit ${result.exitCode ?? "unknown"}): ${result.error ?? "see log"}.`, + data: { configured: true, ok: result.ok, exitCode: result.exitCode, log: result.log, ...(result.error !== undefined ? { error: result.error } : {}) }, + }; + } catch (error) { + // A connection/protocol failure to the companion itself (socket missing, timeout, unauthorized) -- + // distinct from a redeploy that ran and failed (handled above via result.ok === false). + return { + summary: `LoopOver redeploy trigger: could not reach the host companion: ${error instanceof Error ? error.message : String(error)}`, + data: { configured: true, ok: false, exitCode: null, error: error instanceof Error ? error.message : String(error) }, + }; + } + } + private async canAccessRepo(fullName: string): Promise { if (this.identity.kind === "session") return canLoginAccessRepo(this.env, this.identity.actor, fullName); // The static `mcp` identity is a shared, end-user-obtainable CLI credential — scope it to the operator's diff --git a/src/selfhost/redeploy-companion-client.ts b/src/selfhost/redeploy-companion-client.ts new file mode 100644 index 0000000000..00fba6a632 --- /dev/null +++ b/src/selfhost/redeploy-companion-client.ts @@ -0,0 +1,90 @@ +// Node-only client for the host-side redeploy companion (#7723). Talks to a Unix domain socket +// (scripts/redeploy-companion.ts's own protocol) -- this module's `node:net` import must never reach the +// Cloudflare Workers bundle, so src/mcp/server.ts (which IS bundled for Workers) never imports this file +// directly; only src/server.ts (the Node self-host boot entry) does, injecting a real closure into +// src/mcp/redeploy-companion-registry.ts's nullable slot. Mirrors src/selfhost/private-config.ts's own +// read/write helpers -> src/mcp/private-config-admin-registry.ts injection pattern exactly (#7721). +import { createConnection } from "node:net"; + +export type RedeployCompanionConfig = { + socketPath: string; + token: string; + /** Override for tests only -- production always uses this module's own DEFAULT_TIMEOUT_MS. */ + timeoutMs?: number; +}; + +export type RedeployResult = { ok: boolean; exitCode: number | null; error?: string; log: string[] }; + +const DEFAULT_TIMEOUT_MS = 15 * 60 * 1000; // a real pull+recreate+health-wait can legitimately take minutes + +/** Send one redeploy request and collect the companion's streamed response. Rejects (never resolves with a + * fabricated result) on a connection/protocol failure -- the caller (adminTriggerRedeploy) is responsible for + * turning that into a clear tool-result error, not this function guessing at one. */ +export function triggerRedeploy(config: RedeployCompanionConfig, image: string | undefined): Promise { + return new Promise((resolve, reject) => { + const socket = createConnection(config.socketPath); + const log: string[] = []; + let buffer = ""; + let settled = false; + + const timeout = setTimeout(() => { + // Defensive: clearTimeout below (on a normal resolve/error) should prevent this callback from firing + // at all once settled -- kept as a guard in case of a rare timer/event-loop race, not because it's + // expected to trigger in practice. + if (settled) return; + settled = true; + socket.destroy(); + reject(new Error(`redeploy companion did not respond within ${config.timeoutMs ?? DEFAULT_TIMEOUT_MS}ms`)); + }, config.timeoutMs ?? DEFAULT_TIMEOUT_MS); + + socket.on("connect", () => { + socket.write(`${JSON.stringify({ token: config.token, ...(image !== undefined ? { image } : {}) })}\n`); + }); + + socket.on("data", (chunk) => { + buffer += chunk.toString("utf8"); + for (;;) { + const newlineIndex = buffer.indexOf("\n"); + if (newlineIndex === -1) break; + const line = buffer.slice(0, newlineIndex); + buffer = buffer.slice(newlineIndex + 1); + if (!line.trim()) continue; + let parsed: unknown; + try { + parsed = JSON.parse(line); + } catch { + continue; // a malformed line from the companion is dropped, not fatal -- the terminal line still wins + } + if (parsed && typeof parsed === "object" && "log" in parsed && typeof (parsed as { log: unknown }).log === "string") { + log.push((parsed as { log: string }).log); + continue; + } + if (parsed && typeof parsed === "object" && "ok" in parsed) { + if (settled) continue; + settled = true; + clearTimeout(timeout); + const terminal = parsed as { ok: boolean; exitCode?: number | null; error?: string }; + socket.end(); + resolve({ ok: terminal.ok, exitCode: terminal.exitCode ?? null, ...(terminal.error !== undefined ? { error: terminal.error } : {}), log }); + } + } + }); + + socket.on("error", (error) => { + // Defensive only: Promise settlement is idempotent, so a late error after an earlier resolve/reject + // would be a silent no-op even without this guard; it just skips a wasted clearTimeout/reject call, + // not a correctness requirement. + if (settled) return; + settled = true; + clearTimeout(timeout); + reject(error); + }); + + socket.on("close", () => { + if (settled) return; + settled = true; + clearTimeout(timeout); + reject(new Error("redeploy companion closed the connection before sending a terminal response")); + }); + }); +} diff --git a/src/server.ts b/src/server.ts index cdac606045..4991d96214 100644 --- a/src/server.ts +++ b/src/server.ts @@ -81,6 +81,8 @@ import { listConfigBackupsForScope, } from "./selfhost/private-config"; import { setConfigAdminFunctions } from "./mcp/private-config-admin-registry"; +import { setRedeployTrigger } from "./mcp/redeploy-companion-registry"; +import { triggerRedeploy } from "./selfhost/redeploy-companion-client"; import { assertSelfHostPreflight } from "./selfhost/preflight"; import { buildSentryOpenTelemetryBridge, @@ -389,6 +391,18 @@ async function main(): Promise { } : null, ); + // Redeploy trigger (#7723): a SEPARATE opt-in from the config admin tools above -- an operator can run the + // config read/write tools with no host companion installed at all (setRedeployTrigger stays null; the tool + // itself, gated the same LOOPOVER_MCP_ADMIN_ENABLED way, reports a clear "not configured" result instead of + // throwing). Requires BOTH the socket path and the shared companion token -- the socket path alone would + // let a caller attempt a connection with no way to authenticate against whatever answers it. + const redeployCompanionToken = nonBlank(process.env.REDEPLOY_COMPANION_TOKEN); + const redeployCompanionSocketPath = nonBlank(process.env.REDEPLOY_COMPANION_SOCKET_PATH) ?? "/run/loopover-redeploy.sock"; + setRedeployTrigger( + redeployCompanionToken + ? (image) => triggerRedeploy({ socketPath: redeployCompanionSocketPath, token: redeployCompanionToken }, image) + : null, + ); // Boot-time visibility (config-drift guardrail): state which config dir is actually in effect, unconditionally // -- neither reader above logs anything, so an operator previously had no way to confirm from the logs alone // which directory (if any) was live, which is exactly the ambiguity that let a stale, no-longer-mounted config diff --git a/systemd/loopover-redeploy-companion.service.example b/systemd/loopover-redeploy-companion.service.example new file mode 100644 index 0000000000..2c4e745fd3 --- /dev/null +++ b/systemd/loopover-redeploy-companion.service.example @@ -0,0 +1,55 @@ +# Host-side redeploy companion (#7723): lets the `loopover` app container trigger a real self-redeploy +# (pull the published image, restart, wait for health) WITHOUT mounting /var/run/docker.sock into an +# app-facing container. See scripts/redeploy-companion.ts's own header for the full design rationale. +# +# OPT-IN, NOT REQUIRED: without this unit installed and running, the loopover_admin_trigger_redeploy MCP +# tool simply reports connection-refused -- every other admin tool and the app itself are completely +# unaffected. An operator who only wants the config read/write admin tools (#7721) can ignore this file. +# +# Install (adjust the placeholders below to your host): +# sudo useradd --system --no-create-home loopover-redeploy # if you don't already have a dedicated user +# sudo usermod -aG docker loopover-redeploy # needs `docker compose` access +# ./scripts/selfhost-init-secrets.sh # generates secrets/redeploy_companion_token.txt +# # if it doesn't already exist (idempotent) +# sudo install -m 600 -o loopover-redeploy /dev/null /etc/loopover-redeploy-companion.env +# printf 'REDEPLOY_COMPANION_TOKEN=%s\n' "$(cat secrets/redeploy_companion_token.txt)" | \ +# sudo tee /etc/loopover-redeploy-companion.env >/dev/null +# # MUST be the exact same value the `loopover` container reads via REDEPLOY_COMPANION_TOKEN_FILE +# # (docker-compose.yml, defaulting to that same secrets/redeploy_companion_token.txt) -- this is a SHARED +# # secret both sides authenticate with, not two independent ones. If you rotate it, update both. +# sudo cp systemd/loopover-redeploy-companion.service.example /etc/systemd/system/loopover-redeploy-companion.service +# sudo $EDITOR /etc/systemd/system/loopover-redeploy-companion.service # fix User/Group/WorkingDirectory below +# sudo systemctl daemon-reload +# sudo systemctl enable --now loopover-redeploy-companion.service +# journalctl -u loopover-redeploy-companion -f +# +# Then in docker-compose.yml, bind-mount the same socket path (read-write) into the `loopover` service, and +# set LOOPOVER_MCP_ADMIN_ENABLED=1 + the SAME REDEPLOY_COMPANION_TOKEN value as an env var on that service -- +# see docker-compose.yml's own comment above the loopover-redeploy volume mount for the exact lines. + +[Unit] +Description=LoopOver self-redeploy companion (host-side, no docker.sock exposure) +After=network.target docker.service +Requires=docker.service + +[Service] +Type=simple +# REQUIRED: a dedicated, non-root user in the `docker` group (needs `docker compose` access; nothing more -- +# specifically NOT sudo/root, and NOT a user with any other host privilege). +User=loopover-redeploy +Group=loopover-redeploy +# REQUIRED: the repo checkout this compose stack actually runs from -- scripts/deploy-selfhost-image.sh is +# invoked relative to this directory, exactly as if you ran it by hand. +WorkingDirectory=/opt/loopover +Environment=REDEPLOY_COMPANION_SOCKET_PATH=/run/loopover-redeploy.sock +Environment=REDEPLOY_COMPANION_REPO_ROOT=/opt/loopover +# REQUIRED: REDEPLOY_COMPANION_TOKEN. Keep it out of this unit file -- use a root-owned 0600 EnvironmentFile: +EnvironmentFile=/etc/loopover-redeploy-companion.env +ExecStart=/usr/bin/node --experimental-strip-types /opt/loopover/scripts/redeploy-companion.ts +Restart=on-failure +RestartSec=10 +# The socket is created fresh on every start (redeploy-companion.ts removes a stale one from an unclean prior +# shutdown itself), so no RuntimeDirectory bookkeeping is needed here beyond /run/ already existing. + +[Install] +WantedBy=multi-user.target diff --git a/test/unit/mcp-admin-redeploy-tool.test.ts b/test/unit/mcp-admin-redeploy-tool.test.ts new file mode 100644 index 0000000000..b14e5055a0 --- /dev/null +++ b/test/unit/mcp-admin-redeploy-tool.test.ts @@ -0,0 +1,145 @@ +import { Client } from "@modelcontextprotocol/sdk/client/index.js"; +import { InMemoryTransport } from "@modelcontextprotocol/sdk/inMemory.js"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { LoopoverMcp } from "../../src/mcp/server"; +import { setRedeployTrigger } from "../../src/mcp/redeploy-companion-registry"; +import type { AuthIdentity } from "../../src/auth/security"; +import { createTestEnv } from "../helpers/d1"; + +const MCP_ADMIN_IDENTITY: AuthIdentity = { kind: "static", actor: "mcp-admin" }; +const MCP_ORDINARY_IDENTITY: AuthIdentity = { kind: "static", actor: "mcp" }; + +async function connect(env: Env, identity: AuthIdentity = MCP_ADMIN_IDENTITY) { + const server = new LoopoverMcp(env, identity).createServer(); + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + await server.connect(serverTransport); + const client = new Client({ name: "mcp-admin-redeploy-test", version: "0.1.0" }, { capabilities: {} }); + await client.connect(clientTransport); + return client; +} + +afterEach(() => { + setRedeployTrigger(null); +}); + +describe("MCP admin redeploy tool: registration gating (#7723)", () => { + it("is NOT registered when LOOPOVER_MCP_ADMIN_ENABLED is unset (default off)", async () => { + const client = await connect(createTestEnv()); + const { tools } = await client.listTools(); + expect(tools.some((t) => t.name === "loopover_admin_trigger_redeploy")).toBe(false); + }); + + it("IS registered, with the admin category, when LOOPOVER_MCP_ADMIN_ENABLED is truthy", async () => { + const client = await connect(createTestEnv({ LOOPOVER_MCP_ADMIN_ENABLED: "true" })); + const { tools } = await client.listTools(); + const tool = tools.find((t) => t.name === "loopover_admin_trigger_redeploy"); + expect(tool).toBeDefined(); + expect((tool!._meta as { category?: string } | undefined)?.category).toBe("admin"); + }); +}); + +describe("MCP admin redeploy tool: auth boundary (#7723)", () => { + it("rejects the ordinary mcp actor even when the flag is on and a trigger is configured", async () => { + setRedeployTrigger(vi.fn()); + const client = await connect(createTestEnv({ LOOPOVER_MCP_ADMIN_ENABLED: "true" }), MCP_ORDINARY_IDENTITY); + const result = await client.callTool({ name: "loopover_admin_trigger_redeploy", arguments: {} }); + expect(result.isError).toBe(true); + expect(JSON.stringify(result.content)).toMatch(/Forbidden/i); + }); + + it("rejects a session identity too -- this is a static-credential-only surface", async () => { + const client = await connect(createTestEnv({ LOOPOVER_MCP_ADMIN_ENABLED: "true" }), { kind: "session", actor: "some-login", session: {} as never }); + const result = await client.callTool({ name: "loopover_admin_trigger_redeploy", arguments: {} }); + expect(result.isError).toBe(true); + expect(JSON.stringify(result.content)).toMatch(/Forbidden/i); + }); +}); + +describe("MCP admin redeploy tool: not-configured behavior (#7723)", () => { + it("reports configured=false when no trigger is registered (no companion, or REDEPLOY_COMPANION_TOKEN unset)", async () => { + const client = await connect(createTestEnv({ LOOPOVER_MCP_ADMIN_ENABLED: "true" })); + const result = await client.callTool({ name: "loopover_admin_trigger_redeploy", arguments: {} }); + expect(result.isError).toBeFalsy(); + expect((result.structuredContent as { configured: boolean }).configured).toBe(false); + }); +}); + +describe("MCP admin redeploy tool: input validation (#7723)", () => { + it("rejects an image value with shell/compose-interpolation metacharacters before ever calling the trigger", async () => { + const trigger = vi.fn(); + setRedeployTrigger(trigger); + const client = await connect(createTestEnv({ LOOPOVER_MCP_ADMIN_ENABLED: "true" })); + const result = await client.callTool({ name: "loopover_admin_trigger_redeploy", arguments: { image: "not a valid $(image)" } }); + expect(result.isError).toBe(true); + expect(trigger).not.toHaveBeenCalled(); + }); + + it.each(["has`a`backtick", "has;a;semicolon", "has|a|pipe", "has&an&ersand", "hasanglebracket"])( + "rejects shell metacharacters in image: %s", + async (image) => { + const trigger = vi.fn(); + setRedeployTrigger(trigger); + const client = await connect(createTestEnv({ LOOPOVER_MCP_ADMIN_ENABLED: "true" })); + const result = await client.callTool({ name: "loopover_admin_trigger_redeploy", arguments: { image } }); + expect(result.isError).toBe(true); + expect(trigger).not.toHaveBeenCalled(); + }, + ); +}); + +describe("MCP admin redeploy tool: trigger call (#7723)", () => { + it("calls the registered trigger with the given image and reports a successful result", async () => { + const trigger = vi.fn().mockResolvedValue({ ok: true, exitCode: 0, log: ["pulling...", "restarting..."] }); + setRedeployTrigger(trigger); + const client = await connect(createTestEnv({ LOOPOVER_MCP_ADMIN_ENABLED: "true" })); + + const result = await client.callTool({ name: "loopover_admin_trigger_redeploy", arguments: { image: "ghcr.io/jsonbored/loopover-selfhost:orb-v0.1.0" } }); + + expect(trigger).toHaveBeenCalledExactlyOnceWith("ghcr.io/jsonbored/loopover-selfhost:orb-v0.1.0"); + expect(result.isError).toBeFalsy(); + expect(result.structuredContent).toEqual({ configured: true, ok: true, exitCode: 0, log: ["pulling...", "restarting..."] }); + }); + + it("calls the registered trigger with undefined when no image is given", async () => { + const trigger = vi.fn().mockResolvedValue({ ok: true, exitCode: 0, log: [] }); + setRedeployTrigger(trigger); + const client = await connect(createTestEnv({ LOOPOVER_MCP_ADMIN_ENABLED: "true" })); + + await client.callTool({ name: "loopover_admin_trigger_redeploy", arguments: {} }); + + expect(trigger).toHaveBeenCalledExactlyOnceWith(undefined); + }); + + it("reports a failed redeploy (ok:false result from a real run) as a normal, non-error tool result -- not an MCP-level error", async () => { + setRedeployTrigger(vi.fn().mockResolvedValue({ ok: false, exitCode: 1, error: "health check timed out", log: ["pulling...", "restarting..."] })); + const client = await connect(createTestEnv({ LOOPOVER_MCP_ADMIN_ENABLED: "true" })); + + const result = await client.callTool({ name: "loopover_admin_trigger_redeploy", arguments: {} }); + + expect(result.isError).toBeFalsy(); + expect(result.structuredContent).toEqual({ configured: true, ok: false, exitCode: 1, error: "health check timed out", log: ["pulling...", "restarting..."] }); + }); + + it("catches a connection/protocol failure to the companion itself and reports it as a normal tool result, distinct from a ran-but-failed redeploy", async () => { + setRedeployTrigger(vi.fn().mockRejectedValue(new Error("connect ECONNREFUSED /run/loopover-redeploy.sock"))); + const client = await connect(createTestEnv({ LOOPOVER_MCP_ADMIN_ENABLED: "true" })); + + const result = await client.callTool({ name: "loopover_admin_trigger_redeploy", arguments: {} }); + + expect(result.isError).toBeFalsy(); + const data = result.structuredContent as { configured: boolean; ok: boolean; error: string }; + expect(data.configured).toBe(true); + expect(data.ok).toBe(false); + expect(data.error).toMatch(/ECONNREFUSED/); + }); + + it("handles a non-Error rejection from the trigger without crashing", async () => { + setRedeployTrigger(vi.fn().mockRejectedValue("a plain string rejection")); + const client = await connect(createTestEnv({ LOOPOVER_MCP_ADMIN_ENABLED: "true" })); + + const result = await client.callTool({ name: "loopover_admin_trigger_redeploy", arguments: {} }); + + expect(result.isError).toBeFalsy(); + expect((result.structuredContent as { error: string }).error).toBe("a plain string rejection"); + }); +}); diff --git a/test/unit/redeploy-companion-client.test.ts b/test/unit/redeploy-companion-client.test.ts new file mode 100644 index 0000000000..94a5bf5a2a --- /dev/null +++ b/test/unit/redeploy-companion-client.test.ts @@ -0,0 +1,188 @@ +import { createServer, type Server } from "node:net"; +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; +import { triggerRedeploy } from "../../src/selfhost/redeploy-companion-client"; + +// Real Unix domain sockets, not a mocked node:net -- this is the exact protocol +// scripts/redeploy-companion.ts's own server speaks, so a fake with the wrong shape would prove nothing about +// real interop between the two. Both sides of the wire are exercised: this file plays the SERVER role with a +// scripted response, triggerRedeploy is the real CLIENT under test. + +let server: Server | null = null; +let root: string | null = null; + +afterEach(() => { + server?.close(); + server = null; + if (root) rmSync(root, { recursive: true, force: true }); + root = null; +}); + +function startFakeCompanion( + onRequest: (requestLine: string, write: (line: string) => void, end: () => void, rawWrite: (chunk: string) => void) => void, +): string { + root = mkdtempSync(join(tmpdir(), "loopover-redeploy-client-test-")); + const socketPath = join(root, "companion.sock"); + server = createServer((socket) => { + let buffer = ""; + socket.on("data", (chunk) => { + buffer += chunk.toString("utf8"); + const newlineIndex = buffer.indexOf("\n"); + if (newlineIndex === -1) return; + const line = buffer.slice(0, newlineIndex); + onRequest( + line, + (out) => socket.write(`${out}\n`), + () => socket.end(), + (raw) => socket.write(raw), + ); + }); + }); + server.listen(socketPath); + return socketPath; +} + +function waitForListening(): Promise { + return new Promise((resolve) => server!.once("listening", resolve)); +} + +describe("triggerRedeploy (#7723)", () => { + it("sends the token and image in one request line, collects streamed log lines, and resolves the terminal result", async () => { + const socketPath = startFakeCompanion((requestLine, write, end) => { + expect(JSON.parse(requestLine)).toEqual({ token: "test-token", image: "ghcr.io/jsonbored/loopover-selfhost:latest" }); + write(JSON.stringify({ log: "pulling..." })); + write(JSON.stringify({ log: "restarting..." })); + write(JSON.stringify({ ok: true, exitCode: 0 })); + end(); + }); + await waitForListening(); + + const result = await triggerRedeploy({ socketPath, token: "test-token" }, "ghcr.io/jsonbored/loopover-selfhost:latest"); + + expect(result).toEqual({ ok: true, exitCode: 0, log: ["pulling...", "restarting..."] }); + }); + + it("omits the image key entirely from the request when none is given", async () => { + const socketPath = startFakeCompanion((requestLine, write, end) => { + expect(JSON.parse(requestLine)).toEqual({ token: "test-token" }); + write(JSON.stringify({ ok: true, exitCode: 0 })); + end(); + }); + await waitForListening(); + + await triggerRedeploy({ socketPath, token: "test-token" }, undefined); + }); + + it("carries the error field through when the terminal result has one", async () => { + const socketPath = startFakeCompanion((_line, write, end) => { + write(JSON.stringify({ ok: false, exitCode: 1, error: "health check timed out" })); + end(); + }); + await waitForListening(); + + const result = await triggerRedeploy({ socketPath, token: "test-token" }, undefined); + expect(result).toEqual({ ok: false, exitCode: 1, error: "health check timed out", log: [] }); + }); + + it("defaults a missing exitCode in the terminal line to null", async () => { + const socketPath = startFakeCompanion((_line, write, end) => { + write(JSON.stringify({ ok: false })); + end(); + }); + await waitForListening(); + + const result = await triggerRedeploy({ socketPath, token: "test-token" }, undefined); + expect(result.exitCode).toBeNull(); + }); + + it("drops a malformed (non-JSON) line from the server instead of treating it as terminal", async () => { + const socketPath = startFakeCompanion((_line, write, end) => { + write("not valid json"); + write(JSON.stringify({ ok: true, exitCode: 0 })); + end(); + }); + await waitForListening(); + + const result = await triggerRedeploy({ socketPath, token: "test-token" }, undefined); + expect(result).toEqual({ ok: true, exitCode: 0, log: [] }); + }); + + it("rejects when the companion closes the connection without ever sending a terminal response", async () => { + const socketPath = startFakeCompanion((_line, _write, end) => { + end(); // closes immediately, no terminal `ok` line + }); + await waitForListening(); + + await expect(triggerRedeploy({ socketPath, token: "test-token" }, undefined)).rejects.toThrow( + "redeploy companion closed the connection before sending a terminal response", + ); + }); + + it("skips a JSON line that parses successfully but is not an object (e.g. a bare number) -- neither log nor terminal shaped", async () => { + const socketPath = startFakeCompanion((_line, write, end) => { + write("42"); // valid JSON, but typeof 42 !== "object" -- distinct from the malformed-JSON case above + write(JSON.stringify({ ok: true, exitCode: 0 })); + end(); + }); + await waitForListening(); + + const result = await triggerRedeploy({ socketPath, token: "test-token" }, undefined); + expect(result).toEqual({ ok: true, exitCode: 0, log: [] }); + }); + + it("skips a blank line from the server without treating it as malformed or terminal", async () => { + const socketPath = startFakeCompanion((_line, write, end, rawWrite) => { + rawWrite("\n"); // an entirely blank line -- distinct from the malformed-JSON case above + write(JSON.stringify({ ok: true, exitCode: 0 })); + end(); + }); + await waitForListening(); + + const result = await triggerRedeploy({ socketPath, token: "test-token" }, undefined); + expect(result).toEqual({ ok: true, exitCode: 0, log: [] }); + }); + + it("ignores a second terminal-shaped line arriving after the first already resolved the promise", async () => { + const socketPath = startFakeCompanion((_line, write, end) => { + write(JSON.stringify({ ok: true, exitCode: 0 })); + write(JSON.stringify({ ok: false, exitCode: 1, error: "should never be observed" })); + end(); + }); + await waitForListening(); + + const result = await triggerRedeploy({ socketPath, token: "test-token" }, undefined); + expect(result).toEqual({ ok: true, exitCode: 0, log: [] }); // the FIRST terminal line wins, not the second + }); + + it("rejects when no companion is listening at the configured socket path", async () => { + root = mkdtempSync(join(tmpdir(), "loopover-redeploy-client-test-")); + const socketPath = join(root, "nothing-here.sock"); + + await expect(triggerRedeploy({ socketPath, token: "test-token" }, undefined)).rejects.toThrow(); + }); + + it("rejects with a timeout error when the companion accepts the connection but never responds", async () => { + const socketPath = startFakeCompanion(() => undefined); // accepts, reads the request, never writes back + await waitForListening(); + + await expect(triggerRedeploy({ socketPath, token: "test-token", timeoutMs: 20 }, undefined)).rejects.toThrow(/did not respond within 20ms/); + }); + + it("buffers a terminal line split across multiple raw socket writes (no newline in the first write at all)", async () => { + const socketPath = startFakeCompanion((_line, _write, end, rawWrite) => { + const fullLine = JSON.stringify({ ok: true, exitCode: 0 }); + const midpoint = Math.floor(fullLine.length / 2); + rawWrite(fullLine.slice(0, midpoint)); // no newline -- the client must NOT treat this as a complete line + setTimeout(() => { + rawWrite(`${fullLine.slice(midpoint)}\n`); + end(); + }, 5); + }); + await waitForListening(); + + const result = await triggerRedeploy({ socketPath, token: "test-token" }, undefined); + expect(result).toEqual({ ok: true, exitCode: 0, log: [] }); + }); +}); diff --git a/test/unit/redeploy-companion-registry.test.ts b/test/unit/redeploy-companion-registry.test.ts new file mode 100644 index 0000000000..e3df594b54 --- /dev/null +++ b/test/unit/redeploy-companion-registry.test.ts @@ -0,0 +1,24 @@ +import { afterEach, describe, expect, it } from "vitest"; +import { getRedeployTrigger, setRedeployTrigger } from "../../src/mcp/redeploy-companion-registry"; + +afterEach(() => { + setRedeployTrigger(null); +}); + +describe("redeploy-companion-registry (#7723)", () => { + it("returns null before anything is set", () => { + expect(getRedeployTrigger()).toBeNull(); + }); + + it("returns the exact function passed to setRedeployTrigger", async () => { + const trigger = async () => ({ ok: true, exitCode: 0, log: [] }); + setRedeployTrigger(trigger); + expect(getRedeployTrigger()).toBe(trigger); + }); + + it("resets back to null when set with null", () => { + setRedeployTrigger(async () => ({ ok: true, exitCode: 0, log: [] })); + setRedeployTrigger(null); + expect(getRedeployTrigger()).toBeNull(); + }); +}); diff --git a/test/unit/redeploy-companion.test.ts b/test/unit/redeploy-companion.test.ts new file mode 100644 index 0000000000..ba4ff8eaa2 --- /dev/null +++ b/test/unit/redeploy-companion.test.ts @@ -0,0 +1,210 @@ +import { EventEmitter } from "node:events"; +import { describe, expect, it, vi } from "vitest"; +import { handleConnection, runDeploy } from "../../scripts/redeploy-companion"; + +const TOKEN = "companion-test-token"; + +function fakeChildProcess(): { child: EventEmitter & { stdout: EventEmitter; stderr: EventEmitter }; emitClose: (code: number | null) => void; emitError: (error: Error) => void } { + const child = new EventEmitter() as EventEmitter & { stdout: EventEmitter; stderr: EventEmitter }; + child.stdout = new EventEmitter(); + child.stderr = new EventEmitter(); + return { + child, + emitClose: (code) => child.emit("close", code), + emitError: (error) => child.emit("error", error), + }; +} + +describe("runDeploy (#7723)", () => { + it("spawns bash scripts/deploy-selfhost-image.sh with no args when no image is given, forwards stdout/stderr lines, and resolves ok on exit 0", async () => { + const { child, emitClose } = fakeChildProcess(); + const spawnSpy = vi.fn().mockReturnValue(child); + const logs: string[] = []; + + const resultPromise = runDeploy(undefined, (line) => logs.push(line), spawnSpy as never); + expect(spawnSpy).toHaveBeenCalledExactlyOnceWith("bash", ["scripts/deploy-selfhost-image.sh"], expect.objectContaining({ stdio: ["ignore", "pipe", "pipe"] })); + child.stdout.emit("data", Buffer.from("selfhost image deploy: pulling ghcr.io/jsonbored/loopover-selfhost:latest\n")); + child.stderr.emit("data", Buffer.from("some warning\n")); + emitClose(0); + + const result = await resultPromise; + expect(result).toEqual({ ok: true, exitCode: 0 }); + expect(logs).toEqual(["selfhost image deploy: pulling ghcr.io/jsonbored/loopover-selfhost:latest", "some warning"]); + }); + + it("passes the image as a single argv element when given -- never shell-interpolated", async () => { + const { child, emitClose } = fakeChildProcess(); + const spawnSpy = vi.fn().mockReturnValue(child); + + const resultPromise = runDeploy("ghcr.io/jsonbored/loopover-selfhost:orb-v0.1.0", () => undefined, spawnSpy as never); + expect(spawnSpy).toHaveBeenCalledExactlyOnceWith( + "bash", + ["scripts/deploy-selfhost-image.sh", "ghcr.io/jsonbored/loopover-selfhost:orb-v0.1.0"], + expect.anything(), + ); + emitClose(0); + await resultPromise; + }); + + it("resolves ok:false with the real exit code on a non-zero exit", async () => { + const { child, emitClose } = fakeChildProcess(); + const resultPromise = runDeploy(undefined, () => undefined, (() => child) as never); + emitClose(1); + expect(await resultPromise).toEqual({ ok: false, exitCode: 1 }); + }); + + it("resolves ok:false with the spawn error's message when the process itself fails to start", async () => { + const { child, emitError } = fakeChildProcess(); + const resultPromise = runDeploy(undefined, () => undefined, (() => child) as never); + emitError(new Error("bash: not found")); + expect(await resultPromise).toEqual({ ok: false, exitCode: null, error: "bash: not found" }); + }); + + it("drops blank lines from stdout/stderr chunks -- only non-empty lines reach onLog", async () => { + const { child, emitClose } = fakeChildProcess(); + const logs: string[] = []; + const resultPromise = runDeploy(undefined, (line) => logs.push(line), (() => child) as never); + child.stdout.emit("data", Buffer.from("real line\n\n \nanother real line\n")); + emitClose(0); + await resultPromise; + expect(logs).toEqual(["real line", "another real line"]); + }); +}); + +describe("handleConnection (#7723)", () => { + const fakeDeploy = (result: Awaited>) => + vi.fn().mockImplementation(async (_image: string | undefined, onLog: (line: string) => void) => { + onLog("deploying..."); + return result; + }); + + it("rejects a malformed (non-JSON) request line as unauthorized without ever touching busy state or deploy", async () => { + const setBusy = vi.fn(); + const written: string[] = []; + const deploy = vi.fn(); + + await handleConnection(TOKEN, "not json", () => false, setBusy, (line) => written.push(line), deploy); + + expect(written).toEqual([JSON.stringify({ ok: false, error: "unauthorized" })]); + expect(setBusy).not.toHaveBeenCalled(); + expect(deploy).not.toHaveBeenCalled(); + }); + + it("rejects a missing token as unauthorized", async () => { + const written: string[] = []; + await handleConnection(TOKEN, JSON.stringify({}), () => false, vi.fn(), (line) => written.push(line), vi.fn()); + expect(written).toEqual([JSON.stringify({ ok: false, error: "unauthorized" })]); + }); + + it("rejects a wrong token as unauthorized (not a partial/prefix match)", async () => { + const written: string[] = []; + await handleConnection( + TOKEN, + JSON.stringify({ token: `${TOKEN}-wrong` }), + () => false, + vi.fn(), + (line) => written.push(line), + vi.fn(), + ); + expect(written).toEqual([JSON.stringify({ ok: false, error: "unauthorized" })]); + }); + + it("rejects a request while a redeploy is already in progress, without calling deploy again", async () => { + const written: string[] = []; + const deploy = vi.fn(); + await handleConnection(TOKEN, JSON.stringify({ token: TOKEN }), () => true, vi.fn(), (line) => written.push(line), deploy); + expect(written).toEqual([JSON.stringify({ ok: false, error: "redeploy_already_in_progress" })]); + expect(deploy).not.toHaveBeenCalled(); + }); + + it("rejects an unsafe image override (whitespace/quote/backslash/compose-interpolation chars) before ever calling deploy", async () => { + const written: string[] = []; + const deploy = vi.fn(); + await handleConnection( + TOKEN, + JSON.stringify({ token: TOKEN, image: "not a valid $(image)" }), + () => false, + vi.fn(), + (line) => written.push(line), + deploy, + ); + expect(written).toEqual([JSON.stringify({ ok: false, error: "invalid_image_override" })]); + expect(deploy).not.toHaveBeenCalled(); + }); + + it.each(["has`a`backtick", "has;a;semicolon", "has|a|pipe", "has&an&ersand", "hasanglebracket"])( + "rejects shell metacharacters in an image override: %s", + async (image) => { + const written: string[] = []; + const deploy = vi.fn(); + await handleConnection(TOKEN, JSON.stringify({ token: TOKEN, image }), () => false, vi.fn(), (line) => written.push(line), deploy); + expect(written).toEqual([JSON.stringify({ ok: false, error: "invalid_image_override" })]); + expect(deploy).not.toHaveBeenCalled(); + }, + ); + + it("accepts a legitimate image reference with no false-positive rejection", async () => { + const written: string[] = []; + const deploy = vi.fn().mockImplementation(async () => ({ ok: true, exitCode: 0 })); + await handleConnection( + TOKEN, + JSON.stringify({ token: TOKEN, image: "ghcr.io/jsonbored/loopover-selfhost@sha256:abcdef0123456789" }), + () => false, + vi.fn(), + (line) => written.push(line), + deploy, + ); + expect(deploy).toHaveBeenCalledExactlyOnceWith("ghcr.io/jsonbored/loopover-selfhost@sha256:abcdef0123456789", expect.any(Function)); + }); + + it("runs a valid authenticated request end to end: sets busy, streams logs, writes the terminal result, clears busy", async () => { + const written: string[] = []; + const busyStates: boolean[] = []; + let busy = false; + const deploy = fakeDeploy({ ok: true, exitCode: 0 }); + + await handleConnection( + TOKEN, + JSON.stringify({ token: TOKEN, image: "ghcr.io/jsonbored/loopover-selfhost:latest" }), + () => busy, + (value) => { + busy = value; + busyStates.push(value); + }, + (line) => written.push(line), + deploy, + ); + + expect(deploy).toHaveBeenCalledExactlyOnceWith("ghcr.io/jsonbored/loopover-selfhost:latest", expect.any(Function)); + expect(written).toEqual([JSON.stringify({ log: "deploying..." }), JSON.stringify({ ok: true, exitCode: 0 })]); + expect(busyStates).toEqual([true, false]); // set busy before deploying, cleared after -- in that order + }); + + it("clears busy even when the underlying deploy call throws -- never leaves the companion permanently locked", async () => { + let busy = false; + const deploy = vi.fn().mockRejectedValue(new Error("boom")); + + await expect( + handleConnection( + TOKEN, + JSON.stringify({ token: TOKEN }), + () => busy, + (value) => { + busy = value; + }, + () => undefined, + deploy, + ), + ).rejects.toThrow("boom"); + expect(busy).toBe(false); + }); + + it("includes the error field in the terminal response when the deploy result carries one", async () => { + const written: string[] = []; + const deploy = fakeDeploy({ ok: false, exitCode: null, error: "bash: not found" }); + + await handleConnection(TOKEN, JSON.stringify({ token: TOKEN }), () => false, vi.fn(), (line) => written.push(line), deploy); + + expect(written[1]).toBe(JSON.stringify({ ok: false, exitCode: null, error: "bash: not found" })); + }); +}); diff --git a/test/unit/selfhost-image-deploy.test.ts b/test/unit/selfhost-image-deploy.test.ts index 54c54e4852..0cb569d9df 100644 --- a/test/unit/selfhost-image-deploy.test.ts +++ b/test/unit/selfhost-image-deploy.test.ts @@ -245,7 +245,35 @@ describe("self-host image deploy script", () => { try { expect(result.status).not.toBe(0); expect(result.stderr).toContain( - "image contains unsupported whitespace, quote, backslash, or compose interpolation characters", + "image contains unsupported whitespace, quote, backslash, compose interpolation, or shell metacharacters", + ); + expect(readFileSync(harness.envPath, "utf8")).toBe("EXISTING=1\n"); + expect(harness.readImages()).toBe(""); + expect(harness.readCalls()).not.toContain(" pull "); + } finally { + harness.cleanup(); + } + }); + + // Defense-in-depth, not a currently-reachable code-execution path: every reference to the resolved image + // value downstream (resolve_image's printf, this heredoc's "$IMAGE") is properly quoted, so a backtick or + // other shell metacharacter arriving as a real argv element (never through a shell -- see + // redeploy-companion.ts's own spawn() call, which never sets shell: true) stays inert literal text all the + // way through. Rejecting these anyway costs nothing (no legitimate image reference ever contains them) and + // guards against any future change to this script -- or a caller of it -- that stops quoting consistently. + it.each([ + "registry.example/loopover:`touch /tmp/pwned`", + "registry.example/loopover:latest;rm -rf /", + "registry.example/loopover:latest|cat /etc/passwd", + "registry.example/loopover:latest&&whoami", + "registry.example/loopover:latest>/tmp/pwned", + "registry.example/loopover:latest { + const { harness, result } = runHarness({ args: [image], envFile: "EXISTING=1\n" }); + try { + expect(result.status).not.toBe(0); + expect(result.stderr).toContain( + "image contains unsupported whitespace, quote, backslash, compose interpolation, or shell metacharacters", ); expect(readFileSync(harness.envPath, "utf8")).toBe("EXISTING=1\n"); expect(harness.readImages()).toBe("");