From 755ee9ee4c5142e2e85f0e775144566fc0e3eded Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Tue, 28 Jul 2026 00:44:52 -0700 Subject: [PATCH] feat(selfhost): rotate subscription CLI credentials at runtime without a restart Resolve the Claude OAuth token at AI-call time instead of freezing it into process.env at boot, and add three write surfaces for rotating it safely. Resolution order (highest first): a DB-backed fleet credential, a fresh read of CLAUDE_CODE_OAUTH_TOKEN_FILE, then the boot env value. The file is only re-read when the boot loader actually sourced the value from it, so the documented 'an inline .env value always wins' precedence is preserved; every failure degrades to the next rung rather than failing a review. Write surfaces: - scripts/rotate-secret.sh for a single box, validating shape and writing in place so the container's inode-pinned bind mount sees the change immediately - a rotate-secret verb on the redeploy companion, plus a loopover_admin_rotate_secret MCP admin tool, since the app container cannot write its own secrets (the mount is read-only) - INTERNAL_JOB_TOKEN-gated /v1/internal/provider-credentials/* backed by a new provider_credentials table, encrypted with the existing BYOK AES-256-GCM envelope, for fleets with no shared filesystem This closes two silent failure modes seen in production: a label line above the value became part of the credential (the loader only trims), and a write-new-then-rename left the running container serving the old value while still reporting healthy. Closes #9543 --- apps/loopover-ui/public/openapi.json | 211 ++++++++++++++++++ migrations/0199_provider_credentials.sql | 26 +++ scripts/redeploy-companion.ts | 94 +++++++- scripts/rotate-secret.sh | 165 ++++++++++++++ secrets/README.md | 39 ++++ src/api/routes.ts | 61 +++++ src/db/repositories.ts | 107 +++++++++ src/db/schema.ts | 21 ++ src/mcp/redeploy-companion-registry.ts | 15 +- src/mcp/server.ts | 88 +++++++- src/openapi/spec.ts | 49 ++++ src/selfhost/ai.ts | 45 +++- src/selfhost/file-sourced-secrets.ts | 24 ++ src/selfhost/load-file-secrets.ts | 8 +- src/selfhost/provider-credential-registry.ts | 32 +++ src/selfhost/redeploy-companion-client.ts | 37 ++- src/server.ts | 25 ++- .../unit/mcp-admin-rotate-secret-tool.test.ts | 152 +++++++++++++ .../unit/provider-credential-registry.test.ts | 40 ++++ test/unit/provider-credentials.test.ts | 204 +++++++++++++++++ test/unit/redeploy-companion-client.test.ts | 41 +++- test/unit/redeploy-companion-registry.test.ts | 36 ++- test/unit/redeploy-companion.test.ts | 144 +++++++++++- test/unit/selfhost-ai.test.ts | 114 ++++++++++ test/unit/selfhost-load-file-secrets.test.ts | 26 +++ 25 files changed, 1789 insertions(+), 15 deletions(-) create mode 100644 migrations/0199_provider_credentials.sql create mode 100755 scripts/rotate-secret.sh create mode 100644 src/selfhost/file-sourced-secrets.ts create mode 100644 src/selfhost/provider-credential-registry.ts create mode 100644 test/unit/mcp-admin-rotate-secret-tool.test.ts create mode 100644 test/unit/provider-credential-registry.test.ts create mode 100644 test/unit/provider-credentials.test.ts diff --git a/apps/loopover-ui/public/openapi.json b/apps/loopover-ui/public/openapi.json index 1be226d847..3477f1eb6c 100644 --- a/apps/loopover-ui/public/openapi.json +++ b/apps/loopover-ui/public/openapi.json @@ -22690,6 +22690,217 @@ } ] } + }, + "/v1/internal/provider-credentials/{provider}": { + "get": { + "summary": "Read the secret-free status of a stored instance subscription credential", + "parameters": [ + { + "schema": { + "type": "string", + "enum": [ + "claude-code", + "codex" + ] + }, + "required": true, + "name": "provider", + "in": "path" + } + ], + "responses": { + "200": { + "description": "Credential status. Never includes the credential itself.", + "content": { + "application/json": { + "schema": { + "anyOf": [ + { + "type": "object", + "properties": { + "configured": { + "type": "boolean", + "enum": [ + false + ] + } + }, + "required": [ + "configured" + ] + }, + { + "type": "object", + "properties": { + "configured": { + "type": "boolean", + "enum": [ + true + ] + }, + "provider": { + "type": "string" + }, + "last4": { + "type": "string" + }, + "updatedBy": { + "type": "string", + "nullable": true + }, + "updatedAt": { + "type": "string" + } + }, + "required": [ + "configured", + "provider", + "last4", + "updatedBy", + "updatedAt" + ] + } + ] + } + } + } + }, + "400": { + "description": "Unknown provider" + }, + "401": { + "description": "Invalid internal token" + } + }, + "security": [ + { + "LoopOverBearer": [] + }, + { + "LoopOverSessionCookie": [] + } + ] + }, + "post": { + "summary": "Store or replace an instance subscription credential, encrypted at rest", + "parameters": [ + { + "schema": { + "type": "string", + "enum": [ + "claude-code", + "codex" + ] + }, + "required": true, + "name": "provider", + "in": "path" + } + ], + "requestBody": { + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "credential": { + "type": "string" + } + }, + "required": [ + "credential" + ] + } + } + } + }, + "responses": { + "200": { + "description": "Credential stored. Returns the secret-free status.", + "content": { + "application/json": { + "schema": { + "type": "object", + "additionalProperties": { + "nullable": true + } + } + } + } + }, + "400": { + "description": "Unknown provider, or a credential that is empty, padded, or not a single line" + }, + "401": { + "description": "Invalid internal token" + }, + "503": { + "description": "TOKEN_ENCRYPTION_SECRET is not configured, so the credential cannot be stored encrypted" + } + }, + "security": [ + { + "LoopOverBearer": [] + }, + { + "LoopOverSessionCookie": [] + } + ] + }, + "delete": { + "summary": "Clear a stored instance subscription credential, falling back to the secret file or boot env", + "parameters": [ + { + "schema": { + "type": "string", + "enum": [ + "claude-code", + "codex" + ] + }, + "required": true, + "name": "provider", + "in": "path" + } + ], + "responses": { + "200": { + "description": "Credential cleared", + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "configured": { + "type": "boolean", + "enum": [ + false + ] + } + }, + "required": [ + "configured" + ] + } + } + } + }, + "400": { + "description": "Unknown provider" + }, + "401": { + "description": "Invalid internal token" + } + }, + "security": [ + { + "LoopOverBearer": [] + }, + { + "LoopOverSessionCookie": [] + } + ] + } } }, "servers": [ diff --git a/migrations/0199_provider_credentials.sql b/migrations/0199_provider_credentials.sql new file mode 100644 index 0000000000..8264ab1241 --- /dev/null +++ b/migrations/0199_provider_credentials.sql @@ -0,0 +1,26 @@ +-- Instance subscription-CLI credentials (#9543): the FLEET path for rotating CLAUDE_CODE_OAUTH_TOKEN (and +-- the codex credential) without a restart. A single self-hosted box can rotate its credential in place on +-- disk -- the secret file is a bind mount the running container re-reads at AI-call time -- but a +-- multi-instance deployment has no shared filesystem, so the value lives here instead and every instance +-- resolves it fresh per call (src/selfhost/provider-credential-registry.ts). +-- +-- Keyed by PROVIDER, not by repo: unlike repository_ai_keys / repository_linear_keys (the per-maintainer +-- BYOK tables this deliberately mirrors), this is the instance's OWN subscription credential, so there is +-- exactly one row per provider. Encrypted at rest with the same AES-256-GCM envelope and the same +-- TOKEN_ENCRYPTION_SECRET (see src/utils/crypto.ts); `last4` is a display-only hint derived from the +-- plaintext at write time, and the plaintext is never stored, never logged, and never returned by the API. +-- +-- No DB-side DEFAULT CURRENT_TIMESTAMP on created_at/updated_at, matching migrations/0111_linear_backend.sql: +-- every write goes through Drizzle's $defaultFn(() => nowIso()) (src/db/schema.ts), which always supplies the +-- ISO timestamp explicitly, so a SQLite-format fallback here would be unused surface area, not a safeguard. +CREATE TABLE IF NOT EXISTS provider_credentials ( + provider TEXT PRIMARY KEY, + ciphertext TEXT NOT NULL, + iv TEXT NOT NULL, + salt TEXT, + key_version INTEGER NOT NULL DEFAULT 1, + last4 TEXT NOT NULL, + updated_by TEXT, + created_at TEXT NOT NULL, + updated_at TEXT NOT NULL +); diff --git a/scripts/redeploy-companion.ts b/scripts/redeploy-companion.ts index 5dc1088b80..16b00ee3e3 100644 --- a/scripts/redeploy-companion.ts +++ b/scripts/redeploy-companion.ts @@ -31,7 +31,7 @@ import { createServer } from "node:net"; import { spawn } from "node:child_process"; import { timingSafeEqual } from "node:crypto"; -import { unlinkSync, chmodSync, existsSync } from "node:fs"; +import { unlinkSync, chmodSync, existsSync, readFileSync, writeFileSync } 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(); @@ -56,7 +56,9 @@ function isValidToken(configuredToken: string, candidate: unknown): boolean { return timingSafeEqual(configured, supplied); } -type RedeployRequest = { token: unknown; image?: unknown }; +// `action` is absent on every pre-#9543 request, and MUST keep meaning "redeploy" -- the app container and +// the host companion are upgraded independently, so a new companion always has to serve an old client. +type RedeployRequest = { token: unknown; image?: unknown; action?: unknown; secret?: unknown; value?: unknown }; function parseRequestLine(line: string): RedeployRequest | null { let parsed: unknown; @@ -83,6 +85,82 @@ function isSafeImageOverride(value: unknown): value is string { return typeof value === "string" && value.length > 0 && value.length <= 512 && !/[\s"'\\${}`;|&<>]/.test(value); } +// ─── Secret rotation (#9543) ──────────────────────────────────────────────────────────────────── +// The SECOND verb this companion serves. It lives here, host-side, for a hard reason: the app container +// physically cannot write its own credential -- docker inspect reports the Compose `secrets:` bind mount +// as rw=false -- so a container-side rotation endpoint is impossible, not merely undesirable. +// +// Two footguns this exists to make unhittable, both of which fail SILENTLY (the container stays healthy +// and the status stays green while reviews degrade to the fallback provider): +// 1. Shape: src/selfhost/load-file-secrets.ts only .trim()s the file. A human-added label line above the +// value becomes part of the credential. Hence the single-line/no-comment/no-whitespace validation. +// 2. Inode: a Compose secret is a plain bind mount pinned to the INODE. Writing in place propagates to +// the running container instantly; write-new-then-rename (`mv`, and several editors' default save) +// leaves the container serving the OLD content, and `docker compose up -d` will NOT fix it -- it +// reports "Container Running" and changes nothing, because the Compose config is unchanged. So this +// writes with truncate-in-place and never renames. + +/** Secrets this verb may write, mapped to their path relative to REPO_ROOT. An explicit allowlist rather + * than an arbitrary operator-supplied path: the whole point is that this process runs with more host + * privilege than its caller, so it must never become a general "write any file as me" primitive. */ +const ROTATABLE_SECRETS: Record = { + claude_code_oauth_token: "secrets/claude_code_oauth_token.txt", + github_webhook_secret: "secrets/github_webhook_secret.txt", + loopover_api_token: "secrets/loopover_api_token.txt", + loopover_mcp_token: "secrets/loopover_mcp_token.txt", + loopover_mcp_admin_token: "secrets/loopover_mcp_admin_token.txt", + pagerduty_routing_key: "secrets/pagerduty_routing_key.txt", +}; + +/** A secret file's value must be exactly the credential -- one line, no comment, no surrounding + * whitespace, non-empty, and bounded. Rejecting here is what turns footgun 1 above into an error the + * operator sees immediately instead of a silent provider downgrade discovered days later. */ +export function isValidSecretValue(value: unknown): value is string { + return typeof value === "string" && value.length > 0 && value.length <= 4096 && !/[\r\n]/.test(value) && value.trim() === value && !value.startsWith("#"); +} + +export type RotateSecretResult = { ok: boolean; error?: string; backupPath?: string }; + +/** + * Write a secret file IN PLACE (truncate, never rename) so the running container's bind mount -- pinned to + * the inode -- sees the new bytes immediately. Backs the previous value up first, and preserves the 0644 + * the app's own uid depends on to read it back (secrets/README.md explains why 600 breaks the app). + * + * Injectable fs/now for tests only; production always uses the real node:fs. + */ +export function rotateSecret( + name: unknown, + value: unknown, + io: { + readFileSync: typeof readFileSync; + writeFileSync: typeof writeFileSync; + existsSync: typeof existsSync; + chmodSync: typeof chmodSync; + now: () => Date; + } = { readFileSync, writeFileSync, existsSync, chmodSync, now: () => new Date() }, +): RotateSecretResult { + if (typeof name !== "string" || !Object.hasOwn(ROTATABLE_SECRETS, name)) return { ok: false, error: "unknown_secret" }; + if (!isValidSecretValue(value)) return { ok: false, error: "invalid_secret_value" }; + const target = `${REPO_ROOT}/${ROTATABLE_SECRETS[name]}`; + try { + let backupPath: string | undefined; + if (io.existsSync(target)) { + const previous = io.readFileSync(target, "utf8"); + // Backups live beside the deploy backups, never inside secrets/ -- that directory is the Compose + // `secrets:` source, and a stray file there is one careless glob away from being mounted somewhere. + backupPath = `${REPO_ROOT}/.deploy-backups/${name}.txt.bak-${io.now().toISOString().replace(/[:.]/g, "").replace(/-/g, "")}`; + io.writeFileSync(backupPath, previous, { mode: 0o600 }); + } + // No trailing newline: secrets/README.md's own `printf '%s'` convention. The loader .trim()s anyway, + // so this is about keeping the file byte-identical to the issued credential, not correctness. + io.writeFileSync(target, value, { mode: 0o644, flag: "w" }); + io.chmodSync(target, 0o644); // an existing file keeps its old mode through a plain write -- force it + return backupPath ? { ok: true, backupPath } : { ok: true }; + } catch (error) { + return { ok: false, error: error instanceof Error ? error.message : String(error) }; + } +} + 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 @@ -116,12 +194,24 @@ export async function handleConnection( setBusy: (busy: boolean) => void, write: (line: string) => void, deploy: typeof runDeploy = runDeploy, + rotate: typeof rotateSecret = rotateSecret, ): Promise { const request = parseRequestLine(requestLine); if (!request || !isValidToken(configuredToken, request.token)) { write(JSON.stringify({ ok: false, error: "unauthorized" })); return; } + // Rotation is answered BEFORE the busy check: it is a single truncating write, not orchestration, so it + // neither races a running redeploy nor is worth refusing during one -- and refusing it during a 15-minute + // redeploy is exactly when an operator is most likely to need a credential fixed. + if (request.action === "rotate-secret") { + write(JSON.stringify(rotate(request.secret, request.value))); + return; + } + if (request.action !== undefined && request.action !== "redeploy") { + write(JSON.stringify({ ok: false, error: "unknown_action" })); + return; + } if (isBusy()) { write(JSON.stringify({ ok: false, error: "redeploy_already_in_progress" })); return; diff --git a/scripts/rotate-secret.sh b/scripts/rotate-secret.sh new file mode 100755 index 0000000000..e2861225dc --- /dev/null +++ b/scripts/rotate-secret.sh @@ -0,0 +1,165 @@ +#!/usr/bin/env bash +# Rotate one self-host credential safely (#9543). +# +# Replaces the hand-rolled "edit the file, restart the stack, hope" dance, which has two failure modes that +# are both SILENT -- the container stays healthy and the status stays green while every review quietly +# degrades to the fallback provider: +# +# 1. SHAPE. src/selfhost/load-file-secrets.ts only .trim()s the file. It does not strip comments and does +# not select a line. A human-added label line above the value ("# some-account") becomes part of the +# credential, and with AI_PROVIDER=claude-code,ollama the failed auth just falls through to Ollama. +# Use a sidecar .label file if you want to annotate which account a credential belongs to. +# +# 2. INODE. A Compose `secrets:` entry is a plain bind mount pinned to the INODE. Writing the file IN +# PLACE propagates to the running container instantly; write-new-then-rename (`mv`, and the default +# save behaviour of several editors) leaves the container serving the OLD bytes. `docker compose up -d` +# does NOT repair this -- it prints "Container Running" and changes nothing, because the Compose config +# is unchanged. Only --force-recreate re-establishes the mount. This script therefore always truncates +# in place and never renames. +# +# Usage (the value is read from STDIN, never argv -- argv is visible in `ps` and lands in shell history): +# ./scripts/rotate-secret.sh claude_code_oauth_token < /path/to/new-token +# printf '%s' 'sk-ant-oat01...' | ./scripts/rotate-secret.sh claude_code_oauth_token +# ./scripts/rotate-secret.sh --list +# +# For claude_code_oauth_token no restart is needed: the token is re-read from the file on every AI call +# (src/selfhost/ai.ts's resolveClaudeOauthToken). Codex is likewise already hot -- its auth.json is +# re-resolved per call. Every other secret is materialised into the environment once at boot, so this +# script recreates the container for those, and only those. +set -euo pipefail + +REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +cd "$REPO_ROOT" + +# Secrets that are re-read at AI-call time and therefore need no restart at all. +HOT_SECRETS=" claude_code_oauth_token codex_auth " + +usage() { + cat >&2 <<'EOF' +usage: rotate-secret.sh (new value on stdin) + rotate-secret.sh --list + +Reads the new value from stdin, validates its shape, backs up the previous value, +writes it in place, and verifies the running container sees it. +EOF +} + +secret_path() { + case "$1" in + claude_code_oauth_token) echo "secrets/claude_code_oauth_token.txt" ;; + github_webhook_secret) echo "secrets/github_webhook_secret.txt" ;; + loopover_api_token) echo "secrets/loopover_api_token.txt" ;; + loopover_mcp_token) echo "secrets/loopover_mcp_token.txt" ;; + loopover_mcp_admin_token) echo "secrets/loopover_mcp_admin_token.txt" ;; + pagerduty_routing_key) echo "secrets/pagerduty_routing_key.txt" ;; + internal_job_token) echo "secrets/internal_job_token.txt" ;; + redeploy_companion_token) echo "secrets/redeploy_companion_token.txt" ;; + *) return 1 ;; + esac +} + +if [ "${1:-}" = "--list" ]; then + echo "rotatable secrets:" + for name in claude_code_oauth_token codex_auth github_webhook_secret loopover_api_token loopover_mcp_token loopover_mcp_admin_token pagerduty_routing_key internal_job_token redeploy_companion_token; do + case "$HOT_SECRETS" in *" $name "*) echo " $name (hot -- no restart)" ;; *) echo " $name (needs restart)" ;; esac + done + exit 0 +fi + +NAME="${1:-}" +if [ -z "$NAME" ]; then usage; exit 2; fi + +# codex's credential is not a Compose secret at all -- it is auth.json inside the loopover-data volume, +# written by `codex auth`. It is a JSON document, so the single-line validation below cannot apply to it. +if [ "$NAME" = "codex_auth" ]; then + echo "codex_auth is not a Compose secret: it is auth.json inside the loopover-data volume." >&2 + echo "Rotate it by running 'codex auth' in the container, or by replacing /data/codex/auth.json." >&2 + echo "No restart is needed either way -- the path is re-resolved on every AI call." >&2 + exit 2 +fi + +if ! TARGET="$(secret_path "$NAME")"; then + echo "unknown secret: $NAME (see --list)" >&2 + exit 2 +fi + +if [ -t 0 ]; then + echo "refusing to read the value from a terminal -- pipe it in or redirect a file." >&2 + usage + exit 2 +fi + +# `$(...)` strips trailing newlines, which is exactly what we want: a file written by an editor almost +# always ends in one, and the stored credential must not. +VALUE="$(cat)" + +# ── Shape validation (footgun 1) ──────────────────────────────────────────────────────────────── +if [ -z "$VALUE" ]; then + echo "refusing to write an empty value to $TARGET" >&2 + exit 1 +fi +if [ "$(printf '%s' "$VALUE" | wc -l | tr -d ' ')" != "0" ]; then + echo "refusing to write a multi-line value to $TARGET" >&2 + echo "the loader only trims -- a comment or label line would become part of the credential." >&2 + echo "put annotations in ${TARGET%.txt}.label instead." >&2 + exit 1 +fi +case "$VALUE" in + '#'*) echo "refusing to write a value starting with '#' -- that is a comment, not a credential." >&2; exit 1 ;; + *[[:space:]]*) echo "refusing to write a value containing whitespace to $TARGET" >&2; exit 1 ;; +esac +if [ "$NAME" = "claude_code_oauth_token" ]; then + case "$VALUE" in + sk-ant-*) ;; + *) echo "refusing to write a claude token that does not start with 'sk-ant-'." >&2; exit 1 ;; + esac +fi + +# ── Backup ────────────────────────────────────────────────────────────────────────────────────── +# Backups go to .deploy-backups/, never inside secrets/ -- that directory is the Compose `secrets:` source, +# and a stray file there is one careless glob away from being mounted somewhere it should not be. +mkdir -p .deploy-backups +if [ -s "$TARGET" ]; then + BACKUP=".deploy-backups/${NAME}.txt.bak-$(date -u +%Y%m%dT%H%M%SZ)" + cp -p "$TARGET" "$BACKUP" + chmod 600 "$BACKUP" + echo "backed up previous value -> $BACKUP" +fi + +# ── Write IN PLACE (footgun 2) ────────────────────────────────────────────────────────────────── +# `>` truncates the existing inode rather than creating a new one, so the running container's bind mount +# keeps pointing at the same file and sees the new bytes immediately. Never `mv` here. +printf '%s' "$VALUE" > "$TARGET" +chmod 644 "$TARGET" # 644, not 600: the app reads this as its own uid -- see secrets/README.md. +echo "wrote $(wc -c < "$TARGET" | tr -d ' ') bytes to $TARGET" + +# ── Verify the container agrees ───────────────────────────────────────────────────────────────── +# The whole point of writing in place is that a RUNNING container sees the change. Prove it rather than +# assuming it -- this is the check that would have caught the stale-inode failure this script exists for. +if command -v docker >/dev/null 2>&1 && docker compose ps --status running loopover >/dev/null 2>&1; then + HOST_BYTES="$(wc -c < "$TARGET" | tr -d ' ')" + CONTAINER_BYTES="$(docker compose exec -T loopover sh -c "wc -c < /run/secrets/${NAME}" 2>/dev/null | tr -d ' \r' || echo "")" + if [ -n "$CONTAINER_BYTES" ] && [ "$CONTAINER_BYTES" != "$HOST_BYTES" ]; then + echo "WARNING: container sees ${CONTAINER_BYTES} bytes but the host file is ${HOST_BYTES}." >&2 + echo "the bind mount is pinned to a stale inode. recreate the container:" >&2 + echo " docker compose up -d --no-deps --force-recreate loopover" >&2 + exit 1 + fi + echo "verified: the running container sees the new value (${HOST_BYTES} bytes)" + + case "$HOT_SECRETS" in + *" $NAME "*) + echo "done -- no restart needed, ${NAME} is re-read on every AI call." + ;; + *) + echo "recreating the container so the new value is picked up at boot..." + docker compose up -d --no-deps --force-recreate loopover + ;; + esac +else + echo "note: docker compose not available here, so the container's view was not verified." + case "$HOT_SECRETS" in + *" $NAME "*) echo "no restart needed -- ${NAME} is re-read on every AI call." ;; + *) echo "restart required: docker compose up -d --no-deps --force-recreate loopover" ;; + esac +fi diff --git a/secrets/README.md b/secrets/README.md index 1df8de9f5a..f767e2dba6 100644 --- a/secrets/README.md +++ b/secrets/README.md @@ -55,6 +55,45 @@ To use a secret file instead of an inline `.env` value: Leave each file at the `644` the init script (`scripts/selfhost-init-secrets.sh`) sets by default — see the tradeoff explained above for why `600` breaks the app's own ability to read it back. +## Rotating a secret (#9543) + +**Use `./scripts/rotate-secret.sh` rather than editing these files by hand** — it enforces both of the +rules below, backs the old value up, and verifies the running container actually picked the change up: + +```sh +printf '%s' 'sk-ant-oat01-your-new-token' | ./scripts/rotate-secret.sh claude_code_oauth_token +``` + +If you do edit by hand, two rules matter, and getting either wrong fails **silently** — the container +stays healthy and the status stays green while every review quietly degrades to the fallback provider: + +1. **The file must contain the bare value and nothing else.** The loader + (`src/selfhost/load-file-secrets.ts`) only `.trim()`s — it does not strip comments and does not pick a + line. A label line above the value becomes *part of the credential*. To annotate which account a + credential belongs to, use a sidecar file (`claude_code_oauth_token.label`); it is ignored by + everything and gitignored the same as the secret itself. +2. **Write in place; never write-new-then-rename.** A Compose `secrets:` entry is a plain bind mount + pinned to the **inode**. `printf '%s' … > file` truncates in place and the running container sees the + new bytes immediately. `mv` (and the default save behaviour of several editors) creates a *new* inode + and leaves the container serving the old value — and `docker compose up -d` will **not** repair it, as + it prints `Container Running` and changes nothing when the Compose config is unchanged. Recovering + from that needs `docker compose up -d --no-deps --force-recreate loopover`. + +**No restart is needed for `claude_code_oauth_token`** — it is re-read from this file on every AI call +(`resolveClaudeOauthToken` in `src/selfhost/ai.ts`). Codex is likewise already hot: its `auth.json` lives +in the `loopover-data` volume and is re-resolved per call. Every other secret here is materialised into +the environment once at boot, so those do still need the service recreated. + +Two other ways to rotate, both covering the same ground without an SSH session: + +- **`loopover_admin_rotate_secret`** (MCP) — drives the host-side redeploy companion over its existing + Unix socket. Requires `LOOPOVER_MCP_ADMIN_TOKEN` + `LOOPOVER_MCP_ADMIN_ENABLED` and an installed + companion. The app container cannot write these files itself: the mount is read-only. +- **`POST /v1/internal/provider-credentials/claude-code`** — the *fleet* path. Stores the credential + encrypted at rest (same AES-256-GCM envelope as the BYOK provider keys) so every instance resolves it + fresh at call time; a multi-instance deployment has no shared filesystem for the file path to use. + A stored credential takes precedence over this file; `DELETE` clears it and falls back here. + ## Files | File | Env var | Purpose | diff --git a/src/api/routes.ts b/src/api/routes.ts index 3dd40bbdec..c7e10d756e 100644 --- a/src/api/routes.ts +++ b/src/api/routes.ts @@ -117,6 +117,9 @@ import { upsertContributorScoringProfile, upsertRepositorySettings, clearPullRequestsRegatedAtForOpenPrs, + getProviderCredentialStatus, + upsertProviderCredential, + deleteProviderCredential, getRepositoryAiKeyStatus, upsertRepositoryAiKey, deleteRepositoryAiKey, @@ -976,6 +979,22 @@ const repositoryAiKeySchema = z path: ["key"], }); +// Instance subscription-CLI credential (#9543). Write-only: encrypted at rest, never returned. The +// single-line / no-comment / no-surrounding-whitespace rule is the SAME one the host-side rotation path +// enforces, for the same reason -- src/selfhost/load-file-secrets.ts only .trim()s, so a label line above +// the value silently becomes part of the credential and every AI call fails auth while the container stays +// healthy. Validating it here too means the DB path cannot store a shape the file path would reject. +const rotatableProviderSchema = z.enum(["claude-code", "codex"]); +const providerCredentialSchema = z.object({ + credential: z + .string() + .min(1) + .max(4096) + .refine((value) => !/[\r\n]/.test(value), "must be a single line -- a comment or label line would become part of the credential") + .refine((value) => value.trim() === value, "must not have leading or trailing whitespace") + .refine((value) => !value.startsWith("#"), "must not start with '#' -- that is a comment, not a credential"), +}); + // Linear personal API key (#3186) -- no provider-prefix assertion (unlike the AI-key schema above): Linear's // key format is not a stable enough public contract to hard-validate against, so only a length bound applies. const repositoryLinearKeySchema = z.object({ @@ -5351,6 +5370,48 @@ export function createApp() { return c.json({ configured: false }); }); + // Instance subscription-CLI credentials (#9543) -- the FLEET rotation path. A single self-hosted box can + // rotate its credential in place on disk (scripts/rotate-secret.sh, or the companion's rotate-secret + // verb), but a multi-instance deployment has no shared filesystem, so the value lives encrypted in the DB + // and every instance resolves it fresh at AI-call time. Stored here, a rotation takes effect on the very + // next review on every instance, with no restart anywhere. + // + // GET returns secret-free status ONLY (configured/last4/updatedAt) -- never the credential, matching the + // BYOK ai-key surface above. DELETE clears it, so resolution falls back to the secret file / boot env. + app.get("/v1/internal/provider-credentials/:provider", async (c) => { + const parsed = rotatableProviderSchema.safeParse(c.req.param("provider")); + if (!parsed.success) return c.json({ error: "unknown_provider" }, 400); + return c.json(await getProviderCredentialStatus(c.env, parsed.data)); + }); + + app.post("/v1/internal/provider-credentials/:provider", async (c) => { + const parsedProvider = rotatableProviderSchema.safeParse(c.req.param("provider")); + if (!parsedProvider.success) return c.json({ error: "unknown_provider" }, 400); + const body = await c.req.json().catch(() => null); + const parsed = providerCredentialSchema.safeParse(body); + if (!parsed.success) return c.json({ error: "invalid_credential", issues: parsed.error.issues }, 400); + try { + return c.json(await upsertProviderCredential(c.env, { provider: parsedProvider.data, credential: parsed.data.credential })); + } catch (error) { + // The only expected throw here is a missing encryption secret -- never echo credential material in + // the error. upsertProviderCredential's own `empty_credential` guard is deliberately NOT handled: + // providerCredentialSchema above already rejects an empty or whitespace-only credential with a 400, + // so that throw is unreachable through this route and a branch for it would be dead code (it still + // guards direct callers of the repository function). + if (error instanceof Error && error.message === "missing_encryption_secret") { + return c.json({ error: "encryption_unavailable", detail: "TOKEN_ENCRYPTION_SECRET is not configured." }, 503); + } + throw error; + } + }); + + app.delete("/v1/internal/provider-credentials/:provider", async (c) => { + const parsed = rotatableProviderSchema.safeParse(c.req.param("provider")); + if (!parsed.success) return c.json({ error: "unknown_provider" }, 400); + await deleteProviderCredential(c.env, parsed.data); + return c.json({ configured: false }); + }); + // Linear API key (#3186). GET returns secret-free status only; POST stores it encrypted at rest; // DELETE removes it. The plaintext key is never logged and never returned. app.get("/v1/internal/repos/:owner/:repo/linear-key", async (c) => { diff --git a/src/db/repositories.ts b/src/db/repositories.ts index 16fcd6f25b..1b35d39898 100644 --- a/src/db/repositories.ts +++ b/src/db/repositories.ts @@ -52,6 +52,7 @@ import { repoSnapshots, repoSyncSegments, repoSyncState, + providerCredentials, repositoryAiKeys, repositoryLinearKeys, repositorySettings, @@ -1234,6 +1235,112 @@ export async function getDecryptedRepositoryAiKey(env: Env, fullName: string): P } } +// ─── Instance subscription-CLI credentials (#9543) ────────────────────────────────────────────── +// The FLEET rotation path: same isolated-table, encrypted-at-rest shape as the BYOK provider keys above +// (reuses the same TOKEN_ENCRYPTION_SECRET + encryptSecret/decryptSecret envelope), but keyed by provider +// rather than by repo -- this is the instance's OWN subscription credential. Resolved fresh at AI-call +// time via src/selfhost/provider-credential-registry.ts, so a rotation lands on the very next review with +// no restart on any instance. + +/** Providers whose subscription credential can be rotated at runtime. */ +export type RotatableProviderName = "claude-code" | "codex"; + +/** Secret-free status of a stored instance credential -- the ONLY shape the API surface ever returns. */ +export type ProviderCredentialStatus = + | { configured: false } + | { configured: true; provider: RotatableProviderName; last4: string; updatedBy: string | null; updatedAt: string }; + +function normalizeRotatableProvider(value: string): RotatableProviderName { + return value === "codex" ? "codex" : "claude-code"; +} + +/** Read the secret-free status of a stored instance credential. Never returns the credential itself. */ +export async function getProviderCredentialStatus(env: Env, provider: RotatableProviderName): Promise { + const db = getDb(env.DB); + const [row] = await db.select().from(providerCredentials).where(eq(providerCredentials.provider, provider)).limit(1); + if (!row) return { configured: false }; + return { configured: true, provider: normalizeRotatableProvider(row.provider), last4: row.last4, updatedBy: row.updatedBy, updatedAt: row.updatedAt }; +} + +/** + * Store (or replace) an instance subscription credential, encrypted at rest. Returns the secret-free + * status. Throws `missing_encryption_secret` when TOKEN_ENCRYPTION_SECRET is not configured -- callers + * must surface that rather than store a credential in the clear. + */ +export async function upsertProviderCredential( + env: Env, + input: { provider: RotatableProviderName; credential: string; updatedBy?: string | null }, +): Promise { + const secret = env.TOKEN_ENCRYPTION_SECRET; + if (!secret) throw new Error("missing_encryption_secret"); + const trimmed = input.credential.trim(); + if (!trimmed) throw new Error("empty_credential"); + const existing = await getProviderCredentialStatus(env, input.provider); + const { ciphertext, iv, salt, version } = await encryptSecret(trimmed, secret); + const last4 = trimmed.slice(-4); + const updatedBy = input.updatedBy ?? null; + const updatedAt = nowIso(); + const db = getDb(env.DB); + await db + .insert(providerCredentials) + .values({ provider: input.provider, ciphertext, iv, salt, keyVersion: version, last4, updatedBy, updatedAt }) + .onConflictDoUpdate({ + target: providerCredentials.provider, + set: { ciphertext, iv, salt, keyVersion: version, last4, updatedBy, updatedAt }, + }); + await recordProviderCredentialChange(env, { provider: input.provider, action: existing.configured ? "replace" : "set", last4, actor: updatedBy }); + return { configured: true, provider: input.provider, last4, updatedBy, updatedAt }; +} + +/** Remove a stored instance credential, so resolution falls back to the secret file / boot env. */ +export async function deleteProviderCredential(env: Env, provider: RotatableProviderName, actor?: string | null): Promise { + const existing = await getProviderCredentialStatus(env, provider); + const db = getDb(env.DB); + await db.delete(providerCredentials).where(eq(providerCredentials.provider, provider)); + if (existing.configured) { + await recordProviderCredentialChange(env, { provider, action: "delete", last4: existing.last4, actor: actor ?? null }); + } +} + +/** + * Audit an instance-credential lifecycle change. Stored in ai_usage_events as a non-"ok" status so it + * never counts toward the daily neuron budget. NEVER includes any credential material -- only the + * display-only last4 and the actor. + */ +async function recordProviderCredentialChange( + env: Env, + input: { provider: RotatableProviderName; action: "set" | "replace" | "delete"; last4: string; actor: string | null }, +): Promise { + await recordAiUsageEvent(env, { + feature: "provider_credential_change", + actor: input.actor, + route: "internal.provider_credential", + model: `subscription:${input.provider}`, + status: input.action, + estimatedNeurons: 0, + detail: `instance credential ${input.action}`, + metadata: { provider: input.provider, action: input.action, last4: input.last4 }, + }); +} + +/** + * Decrypt an instance credential for an AI call. Returns null when none is stored OR the encryption + * secret is unavailable OR decryption fails -- so resolution silently falls through to the secret-file / + * boot-env rungs and a misconfiguration never blocks a review. Used immediately, never cached. + */ +export async function getDecryptedProviderCredential(env: Env, provider: RotatableProviderName): Promise { + const secret = env.TOKEN_ENCRYPTION_SECRET; + if (!secret) return null; + const db = getDb(env.DB); + const [row] = await db.select().from(providerCredentials).where(eq(providerCredentials.provider, provider)).limit(1); + if (!row) return null; + try { + return await decryptSecret(row.ciphertext, row.iv, secret, row.salt); + } catch { + return null; + } +} + // ─── Linear personal API key (#3186) ──────────────────────────────────────────────────────────── // Same isolated-table, encrypted-at-rest shape as the BYOK provider keys above (reuses the same // TOKEN_ENCRYPTION_SECRET + encryptSecret/decryptSecret envelope) -- never serialized by the diff --git a/src/db/schema.ts b/src/db/schema.ts index b75ce4fc3d..05ddf2de91 100644 --- a/src/db/schema.ts +++ b/src/db/schema.ts @@ -139,6 +139,27 @@ export const repositoryLinearKeys = sqliteTable("repository_linear_keys", { updatedAt: text("updated_at").notNull().$defaultFn(() => nowIso()), }); +// Instance-wide subscription-CLI credentials (claude-code / codex), encrypted at rest with AES-256-GCM -- +// the same envelope as repositoryAiKeys above (see src/utils/crypto.ts). This is the FLEET rotation path +// (#9543): a self-host box can rotate its credential in place on disk, but a multi-instance deployment has +// no shared filesystem, so the value lives here and every instance resolves it fresh at AI-call time. +// +// Keyed by provider, NOT by repo -- unlike the BYOK tables above, this is the instance's own subscription +// credential, not a per-maintainer key, so there is exactly one row per provider. `last4` is display-only, +// derived from the plaintext at write time; the plaintext itself is never stored and never returned by the +// API surface. +export const providerCredentials = sqliteTable("provider_credentials", { + provider: text("provider").primaryKey(), + ciphertext: text("ciphertext").notNull(), + iv: text("iv").notNull(), + salt: text("salt"), + keyVersion: integer("key_version").notNull().default(1), + last4: text("last4").notNull(), + updatedBy: text("updated_by"), + createdAt: text("created_at").notNull().$defaultFn(() => nowIso()), + updatedAt: text("updated_at").notNull().$defaultFn(() => nowIso()), +}); + export const repoSyncState = sqliteTable("repo_sync_state", { repoFullName: text("repo_full_name").primaryKey(), status: text("status").notNull().default("never_synced"), diff --git a/src/mcp/redeploy-companion-registry.ts b/src/mcp/redeploy-companion-registry.ts index a45bde5974..8f6d3162f9 100644 --- a/src/mcp/redeploy-companion-registry.ts +++ b/src/mcp/redeploy-companion-registry.ts @@ -7,11 +7,16 @@ // 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"; +import type { RedeployResult, RotateSecretResult } from "../selfhost/redeploy-companion-client"; export type RedeployTrigger = (image: string | undefined) => Promise; +/** Host-side secret rotation over the same companion socket (#9543). A separate slot from the redeploy + * trigger so an older companion that only serves the redeploy verb leaves this one null and the admin + * tool reports "not configured" instead of hanging on a verb the host doesn't understand. */ +export type SecretRotator = (secret: string, value: string) => Promise; let triggerRedeploy: RedeployTrigger | null = null; +let rotateSecret: SecretRotator | null = null; export function setRedeployTrigger(trigger: RedeployTrigger | null): void { triggerRedeploy = trigger; @@ -20,3 +25,11 @@ export function setRedeployTrigger(trigger: RedeployTrigger | null): void { export function getRedeployTrigger(): RedeployTrigger | null { return triggerRedeploy; } + +export function setSecretRotator(rotator: SecretRotator | null): void { + rotateSecret = rotator; +} + +export function getSecretRotator(): SecretRotator | null { + return rotateSecret; +} diff --git a/src/mcp/server.ts b/src/mcp/server.ts index a46341b2a0..a36cebe027 100644 --- a/src/mcp/server.ts +++ b/src/mcp/server.ts @@ -119,7 +119,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 { getRedeployTrigger, getSecretRotator } from "./redeploy-companion-registry"; import { getLocalManifestReader } from "../signals/focus-manifest-loader"; import type { ConfigAdminScope } from "../selfhost/private-config"; import { buildMaintainerActivationPreview } from "../services/maintainer-activation"; @@ -411,6 +411,29 @@ const adminTriggerRedeployShape = { .optional(), }; +// #9543: the secret VALUE is validated identically here and in the companion's own isValidSecretValue -- +// a caller gets a clear MCP-level rejection instead of an opaque host-side one, and the host still refuses +// independently if this layer is ever bypassed. The single-line/no-comment/no-surrounding-whitespace rule +// is not cosmetic: src/selfhost/load-file-secrets.ts only .trim()s the file, so a label line above the +// value silently becomes part of the credential. +const adminRotateSecretShape = { + secret: z.enum(["claude_code_oauth_token", "github_webhook_secret", "loopover_api_token", "loopover_mcp_token", "loopover_mcp_admin_token", "pagerduty_routing_key"]), + value: z + .string() + .min(1) + .max(4096) + .refine((candidate) => !/[\r\n]/.test(candidate), "must be a single line -- a comment or label line would become part of the credential") + .refine((candidate) => candidate.trim() === candidate, "must not have leading or trailing whitespace") + .refine((candidate) => !candidate.startsWith("#"), "must not start with '#' -- that is a comment, not a credential"), +}; +const adminRotateSecretOutputSchema = { + configured: z.boolean(), + ok: z.boolean().optional(), + secret: z.string().optional(), + backupPath: z.string().optional(), + error: z.string().optional(), +}; + const preflightShape = { repoFullName: z.string().min(3).max(PREFLIGHT_LIMITS.repoFullNameChars), contributorLogin: z.string().min(1).max(PREFLIGHT_LIMITS.contributorLoginChars).optional(), @@ -2147,6 +2170,7 @@ export const MCP_TOOL_CATEGORIES: Record = { loopover_admin_write_config: "admin", loopover_admin_list_config_backups: "admin", loopover_admin_trigger_redeploy: "admin", + loopover_admin_rotate_secret: "admin", }; /** Master opt-in for the "admin" tool category (#7721), default OFF. Same truthy-string convention as every @@ -3333,6 +3357,16 @@ export class LoopoverMcp { }, async (input) => this.toolResult(await this.adminTriggerRedeploy(input)), ); + register( + "loopover_admin_rotate_secret", + { + description: + "Self-hosted-operator only. Rotate one of this instance's own secret files (e.g. claude_code_oauth_token) in place on the host, via the redeploy companion (#7723) -- the app container cannot write these itself, the Compose secrets mount is read-only. The value must be the bare credential: a single line, no comment or label line, no surrounding whitespace (the loader only trims, so anything else silently becomes part of the credential). Backs the previous value up first, and writes in place so the running container's inode-pinned bind mount sees it immediately. For claude_code_oauth_token no restart is needed -- the token is re-read per AI call. Requires LOOPOVER_MCP_ADMIN_TOKEN. Returns configured=false if REDEPLOY_COMPANION_TOKEN is unset or the companion isn't reachable.", + inputSchema: adminRotateSecretShape, + outputSchema: adminRotateSecretOutputSchema, + }, + async (input) => this.toolResult(await this.adminRotateSecret(input)), + ); } // ── Miner planning prompts ─────────────────────────────────────────── @@ -4296,6 +4330,58 @@ export class LoopoverMcp { } } + private async adminRotateSecret(input: { secret: string; value: string }): Promise { + this.requireMcpAdmin(); + const rotator = getSecretRotator(); + if (!rotator) { + return { + summary: "LoopOver secret rotation: not configured (REDEPLOY_COMPANION_TOKEN is unset on this instance, or the companion isn't installed).", + data: { configured: false }, + }; + } + // Everything below is deliberately secret-free: the audit row, the summary, and the tool result carry + // only WHICH secret was rotated, never the value or any prefix/suffix of it. + try { + const result = await rotator(input.secret, input.value); + await recordAuditEvent(this.env, { + eventType: "instance.secret_rotated", + actor: this.identity.actor, + targetKey: input.secret, + outcome: result.ok ? "success" : "error", + detail: result.ok ? `Rotated ${input.secret} on the host.` : `Rotation of ${input.secret} failed: ${result.error ?? "unknown error"}.`, + metadata: { secret: input.secret, ok: result.ok }, + }); + return { + summary: result.ok + ? `LoopOver secret rotation: ${input.secret} rotated on the host.${input.secret === "claude_code_oauth_token" ? " No restart needed -- the token is re-read per AI call." : " Restart the loopover service for this to take effect."}` + : `LoopOver secret rotation failed for ${input.secret}: ${result.error ?? "unknown error"}.`, + data: { + configured: true, + ok: result.ok, + secret: input.secret, + ...(result.backupPath !== undefined ? { backupPath: result.backupPath } : {}), + ...(result.error !== undefined ? { error: result.error } : {}), + }, + }; + } catch (error) { + // A connection/protocol failure to the companion itself -- distinct from a rotation that ran and + // was refused (handled above via result.ok === false). + const message = error instanceof Error ? error.message : String(error); + await recordAuditEvent(this.env, { + eventType: "instance.secret_rotated", + actor: this.identity.actor, + targetKey: input.secret, + outcome: "error", + detail: `Could not reach the host companion: ${message}`, + metadata: { secret: input.secret, ok: false }, + }); + return { + summary: `LoopOver secret rotation: could not reach the host companion: ${message}`, + data: { configured: true, ok: false, secret: input.secret, error: message }, + }; + } + } + private async canAccessRepo(fullName: string): Promise { if (this.identity.kind === "session") return canLoginAccessRepo(this.env, this.identity.actor, fullName, this.identity.session?.githubUserId); // The static `mcp` identity is a shared, end-user-obtainable CLI credential — scope it to the operator's diff --git a/src/openapi/spec.ts b/src/openapi/spec.ts index 7f49cb953f..8e80a61bd2 100644 --- a/src/openapi/spec.ts +++ b/src/openapi/spec.ts @@ -1963,6 +1963,55 @@ export function buildOpenApiSpec() { }, }); } + // Instance subscription-CLI credentials (#9543). The response NEVER carries the credential itself -- + // only configured/last4/timestamps -- so the whole surface is safe to describe publicly. + registry.registerPath({ + method: "get", + path: "/v1/internal/provider-credentials/{provider}", + summary: "Read the secret-free status of a stored instance subscription credential", + request: { params: z.object({ provider: z.enum(["claude-code", "codex"]) }) }, + responses: { + 200: { + description: "Credential status. Never includes the credential itself.", + content: { + "application/json": { + schema: z.union([ + z.object({ configured: z.literal(false) }), + z.object({ configured: z.literal(true), provider: z.string(), last4: z.string(), updatedBy: z.string().nullable(), updatedAt: z.string() }), + ]), + }, + }, + }, + 400: { description: "Unknown provider" }, + 401: { description: "Invalid internal token" }, + }, + }); + registry.registerPath({ + method: "post", + path: "/v1/internal/provider-credentials/{provider}", + summary: "Store or replace an instance subscription credential, encrypted at rest", + request: { + params: z.object({ provider: z.enum(["claude-code", "codex"]) }), + body: { content: { "application/json": { schema: z.object({ credential: z.string() }) } } }, + }, + responses: { + 200: { description: "Credential stored. Returns the secret-free status.", content: { "application/json": { schema: z.record(z.string(), z.unknown()) } } }, + 400: { description: "Unknown provider, or a credential that is empty, padded, or not a single line" }, + 401: { description: "Invalid internal token" }, + 503: { description: "TOKEN_ENCRYPTION_SECRET is not configured, so the credential cannot be stored encrypted" }, + }, + }); + registry.registerPath({ + method: "delete", + path: "/v1/internal/provider-credentials/{provider}", + summary: "Clear a stored instance subscription credential, falling back to the secret file or boot env", + request: { params: z.object({ provider: z.enum(["claude-code", "codex"]) }) }, + responses: { + 200: { description: "Credential cleared", content: { "application/json": { schema: z.object({ configured: z.literal(false) }) } } }, + 400: { description: "Unknown provider" }, + 401: { description: "Invalid internal token" }, + }, + }); registry.registerPath({ method: "post", path: "/v1/internal/jobs/refresh-registry", diff --git a/src/selfhost/ai.ts b/src/selfhost/ai.ts index b826e344d6..d0e1dd5d6e 100644 --- a/src/selfhost/ai.ts +++ b/src/selfhost/ai.ts @@ -10,6 +10,8 @@ import { isStructuralProviderConfigError } from "../services/ai-review"; import type { AiContentBlock, CombineStrategy, OnMerge } from "../services/ai-review"; import { isConfiguredSelfHostProvider, resolveConfiguredProviderNames } from "./ai-config"; export { assertNoLegacySharedAiEnv } from "./ai-config"; +import { wasLoadedFromFile } from "./file-sourced-secrets"; +import { getProviderCredentialResolver } from "./provider-credential-registry"; import { incr, observe } from "./metrics"; import { capturePostHogAiGeneration, type PostHogAiGenerationRequestKind } from "./posthog"; import { withReviewSpan } from "./tracing"; @@ -1033,6 +1035,47 @@ function logSelfHostAiProviderFailed(input: { ); } +/** + * Resolve the Claude OAuth token AT CALL TIME so an operator can rotate the credential without recreating + * the container (#9543) -- mirroring how codex's auth.json is already re-resolved per call by + * resolveCodexAuthPath, rather than being frozen into process.env at boot. + * + * Precedence, highest first: + * 1. The fleet-mode DB-backed credential, when one is stored (provider-credential-registry). + * 2. A fresh read of CLAUDE_CODE_OAUTH_TOKEN_FILE -- but ONLY when the boot loader actually sourced the + * live value from that file. docker-compose.yml sets `_FILE` unconditionally, so an operator + * using an inline `.env` value has both set; re-reading the file for them would silently swap their + * credential and invert the "an inline `.env` value always wins" precedence secrets/README.md + * documents. wasLoadedFromFile() is the only signal that separates the two cases. + * 3. The boot-time env value. + * + * Every failure degrades to the next rung instead of throwing: a half-written file caught mid-rotation, or + * a DB blip, must not fail a review that the previous credential could still have served. + */ +async function resolveClaudeOauthToken(parentEnv: Record): Promise { + const bootToken = parentEnv.CLAUDE_CODE_OAUTH_TOKEN; + const resolver = getProviderCredentialResolver(); + if (resolver) { + try { + const stored = (await resolver("claude-code"))?.trim(); + if (stored) return stored; + } catch { + // fall through -- a DB/decrypt failure is never fatal to a review that env/file can still serve + } + } + const path = parentEnv.CLAUDE_CODE_OAUTH_TOKEN_FILE; + if (!path || !wasLoadedFromFile("CLAUDE_CODE_OAUTH_TOKEN")) return bootToken; + try { + const { readFile } = await import("node:fs/promises"); + // An empty file is the state scripts/selfhost-init-secrets.sh leaves behind for an externally-issued + // secret nobody has filled in yet, and is also the momentary state of an in-place rewrite -- neither + // is a reason to drop a working credential, so both fall back to the last known good value. + return (await readFile(path, "utf8")).trim() || bootToken; + } catch { + return bootToken; + } +} + /** Claude Code subscription (CLAUDE_CODE_OAUTH_TOKEN via `claude setup-token`). Headless, read-only, JSON. */ export function createClaudeCodeAi(parentEnv: Record, spawnImpl?: SpawnFn): SelfHostAi { return { @@ -1041,7 +1084,7 @@ export function createClaudeCodeAi(parentEnv: Record // through to an embed-capable provider (ollama/openai-compatible). Without this throw the chain would treat // claude's empty-prompt text answer as "success" and never reach the embed provider → RAG silently breaks. if (options.text) throw new Error("claude_code_no_embed"); - const token = parentEnv.CLAUDE_CODE_OAUTH_TOKEN; + const token = await resolveClaudeOauthToken(parentEnv); const claudeModel = resolveModel(configuredClaudeModel(parentEnv, options.claudeModel), model, "claude-sonnet-5"); const effort = resolveEffort(firstConfigured(options.claudeEffort, parentEnv.CLAUDE_AI_EFFORT)); const timeoutMs = resolveClaudeCliTimeoutMs(parentEnv, options.claudeTimeoutMs); diff --git a/src/selfhost/file-sourced-secrets.ts b/src/selfhost/file-sourced-secrets.ts new file mode 100644 index 0000000000..e5fd3b94df --- /dev/null +++ b/src/selfhost/file-sourced-secrets.ts @@ -0,0 +1,24 @@ +// Which env vars `loadFileSecrets()` actually materialised FROM a secret file at boot (#9543). +// +// This exists so a credential can be re-read from its file at call time WITHOUT inverting the precedence +// documented in secrets/README.md ("an inline `.env` value always wins"). docker-compose.yml sets a +// `_FILE` default for every secret unconditionally, so the mere presence of `_FILE` proves +// nothing about where the live value came from -- an operator using an inline `.env` value has BOTH set, +// and re-reading the file for them would silently swap their credential. Recording the loader's own +// decision at boot is the only signal that distinguishes the two cases after the fact. +// +// Deliberately free of any `node:*` import (unlike load-file-secrets.ts, which statically imports node:fs) +// so the call-time consumer in ai.ts can depend on it without dragging fs into a bundle that must stay +// Workers-safe -- the same reasoning as src/mcp/redeploy-companion-registry.ts's nullable-slot split. + +let fileSourced: ReadonlySet = new Set(); + +/** Record the loader's boot-time result. Called once from server.ts with `loadFileSecrets()`'s return value. */ +export function setFileSourcedSecrets(names: Iterable): void { + fileSourced = new Set(names); +} + +/** True when `name`'s live value was materialised from its `_FILE` at boot rather than set inline. */ +export function wasLoadedFromFile(name: string): boolean { + return fileSourced.has(name); +} diff --git a/src/selfhost/load-file-secrets.ts b/src/selfhost/load-file-secrets.ts index 799da1cf11..43180c6d0d 100644 --- a/src/selfhost/load-file-secrets.ts +++ b/src/selfhost/load-file-secrets.ts @@ -21,7 +21,11 @@ const COMPOSE_RESERVED_FILE_VARS = new Set(["COMPOSE_FILE", "COMPOSE_ENV_FILE"]) export function loadFileSecrets( env: Record = process.env, readFile: (path: string) => string = (path) => readFileSync(path, "utf8"), -): void { +): string[] { + // The names materialised from a file here, returned so server.ts can hand them to + // setFileSourcedSecrets() -- the only durable record of "this value came from a file, not from + // inline .env", which call-time re-reads depend on to preserve inline precedence (#9543). + const fileSourced: string[] = []; for (const key of Object.keys(env)) { if (!key.endsWith("_FILE") || !env[key] || COMPOSE_RESERVED_FILE_VARS.has(key)) continue; const target = key.slice(0, -"_FILE".length); @@ -65,5 +69,7 @@ export function loadFileSecrets( throw new Error(`Secret file for ${key} (${path}) is empty; an empty secret silently reads as unconfigured downstream. Write the value, or unset ${key}.`); } env[target] = value; + fileSourced.push(target); } + return fileSourced; } diff --git a/src/selfhost/provider-credential-registry.ts b/src/selfhost/provider-credential-registry.ts new file mode 100644 index 0000000000..7fa20b91e0 --- /dev/null +++ b/src/selfhost/provider-credential-registry.ts @@ -0,0 +1,32 @@ +// Workers-safe registry for the fleet-mode (DB-backed) subscription credential lookup (#9543), mirroring +// src/mcp/redeploy-companion-registry.ts's nullable-slot pattern exactly: this module holds a single +// nullable function slot and imports nothing environment-specific, so it is safe anywhere in the bundle. +// +// Only the self-host Node entry (server.ts) fills the slot, with a closure over `env` built from +// src/db/repositories.ts's getDecryptedProviderCredential -- src/selfhost/ai.ts must never import the DB +// layer directly. The AI provider chain is deliberately env-driven (createSelfHostAi takes a plain env +// record and has no DB access of its own); threading the credential in through a registry keeps that +// separation intact, the same way the per-repo BYOK key is threaded in by the queue processors rather +// than resolved inside the provider factory. +// +// Unset (cloud, a self-host box with no rotated credential stored, or TOKEN_ENCRYPTION_SECRET absent) +// means the slot stays null and ai.ts falls through to the secret-file / boot-env rungs below it. + +/** Providers whose credential can be rotated at runtime. Codex is listed for the DB envelope's sake even + * though its fleet path is host-side only -- see rotateSecret's doc comment in redeploy-companion.ts. */ +export type RotatableProvider = "claude-code" | "codex"; + +/** Returns the stored plaintext credential for `provider`, or null when none is stored / it cannot be + * decrypted. Must never throw: a lookup failure degrades to the next resolution rung, it does not fail + * the review. */ +export type ProviderCredentialResolver = (provider: RotatableProvider) => Promise; + +let resolveProviderCredential: ProviderCredentialResolver | null = null; + +export function setProviderCredentialResolver(resolver: ProviderCredentialResolver | null): void { + resolveProviderCredential = resolver; +} + +export function getProviderCredentialResolver(): ProviderCredentialResolver | null { + return resolveProviderCredential; +} diff --git a/src/selfhost/redeploy-companion-client.ts b/src/selfhost/redeploy-companion-client.ts index 00fba6a632..a036784342 100644 --- a/src/selfhost/redeploy-companion-client.ts +++ b/src/selfhost/redeploy-companion-client.ts @@ -15,12 +15,44 @@ export type RedeployCompanionConfig = { export type RedeployResult = { ok: boolean; exitCode: number | null; error?: string; log: string[] }; +/** Result of a host-side secret rotation. `backupPath` is absent when no prior value existed to back up. */ +export type RotateSecretResult = { ok: boolean; error?: string; backupPath?: 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 sendCompanionRequest(config, image !== undefined ? { image } : {}, (terminal, log) => ({ + ok: terminal.ok, + exitCode: terminal.exitCode ?? null, + ...(terminal.error !== undefined ? { error: terminal.error } : {}), + log, + })); +} + +/** Rotate one secret file on the host (#9543). Single request/response -- the companion streams no log + * lines for this verb, so `log` is always empty and is not part of the result shape. */ +export function rotateCompanionSecret(config: RedeployCompanionConfig, secret: string, value: string): Promise { + return sendCompanionRequest(config, { action: "rotate-secret", secret, value }, (terminal) => ({ + ok: terminal.ok, + ...(terminal.error !== undefined ? { error: terminal.error } : {}), + ...(terminal.backupPath !== undefined ? { backupPath: terminal.backupPath } : {}), + })); +} + +type TerminalLine = { ok: boolean; exitCode?: number | null; error?: string; backupPath?: string }; + +/** The shared connection lifecycle for every companion verb: connect, write one line-delimited JSON + * request, collect any `{"log"}` lines, settle on the first terminal `{"ok"}` line. Extracted (#9543) so a + * second verb cannot drift from the first on timeout/close/error handling -- the parts most likely to be + * got subtly wrong twice. `shape` maps the terminal line to each verb's own result type. */ +function sendCompanionRequest( + config: RedeployCompanionConfig, + payload: Record, + shape: (terminal: TerminalLine, log: string[]) => T, +): Promise { return new Promise((resolve, reject) => { const socket = createConnection(config.socketPath); const log: string[] = []; @@ -38,7 +70,7 @@ export function triggerRedeploy(config: RedeployCompanionConfig, image: string | }, config.timeoutMs ?? DEFAULT_TIMEOUT_MS); socket.on("connect", () => { - socket.write(`${JSON.stringify({ token: config.token, ...(image !== undefined ? { image } : {}) })}\n`); + socket.write(`${JSON.stringify({ token: config.token, ...payload })}\n`); }); socket.on("data", (chunk) => { @@ -63,9 +95,8 @@ export function triggerRedeploy(config: RedeployCompanionConfig, image: string | 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 }); + resolve(shape(parsed as TerminalLine, log)); } } }); diff --git a/src/server.ts b/src/server.ts index adf84a76dd..b8bcd92c5c 100644 --- a/src/server.ts +++ b/src/server.ts @@ -48,6 +48,9 @@ import { createOrbRelayRegistrationState, isOrbBrokerMode, registerOrbRelayTarge import { exportOrbBatch, getOrCreateAnonSecret } from "./selfhost/orb-collector"; import { createD1Adapter, nodeSqliteDriver } from "./selfhost/d1-adapter"; import { loadFileSecrets } from "./selfhost/load-file-secrets"; +import { setFileSourcedSecrets } from "./selfhost/file-sourced-secrets"; +import { setProviderCredentialResolver } from "./selfhost/provider-credential-registry"; +import { getDecryptedProviderCredential } from "./db/repositories"; import { backupAcknowledgedGaugeValue, buildHealthBody, @@ -86,8 +89,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 { setRedeployTrigger, setSecretRotator } from "./mcp/redeploy-companion-registry"; +import { triggerRedeploy, rotateCompanionSecret } from "./selfhost/redeploy-companion-client"; import { assertSelfHostPreflight } from "./selfhost/preflight"; import { capturePostHogError, @@ -333,7 +336,9 @@ async function main(): Promise { // but never rethrows or exits). /* v8 ignore next -- importing this entrypoint starts the Node server; handler-installation logic itself is unit-tested in selfhost-process-lifecycle.test.ts. */ installSelfHostCrashHandlers({ captureError: (error, context) => capturePostHogError(error, context), flush: flushPostHog }); - loadFileSecrets(); + // The loader's return value records WHICH vars it materialised from a file, so a call-time re-read can + // preserve the "an inline `.env` value always wins" precedence secrets/README.md documents (#9543). + setFileSourcedSecrets(loadFileSecrets()); /* v8 ignore next -- importing this entrypoint starts the Node server; pure validation is covered in selfhost-preflight tests. */ assertSelfHostPreflight(process.env); // Error tracking (#1468, epic #8286): opt-in via POSTHOG_API_KEY -- the same var #6235's MCP telemetry @@ -416,6 +421,13 @@ async function main(): Promise { ? (image) => triggerRedeploy({ socketPath: redeployCompanionSocketPath, token: redeployCompanionToken }, image) : null, ); + // Secret rotation (#9543) rides the SAME companion socket and token as the redeploy trigger above -- it is + // the same host-side privilege boundary, so it is deliberately not a second credential to configure. + setSecretRotator( + redeployCompanionToken + ? (secret, value) => rotateCompanionSecret({ socketPath: redeployCompanionSocketPath, token: redeployCompanionToken }, secret, value) + : 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 @@ -869,6 +881,13 @@ async function main(): Promise { } as unknown as Env; markEnvReady(); + // Fleet-mode credential resolution (#9543): registered HERE, after `env` is fully constructed, because the + // lookup needs the DB binding -- src/selfhost/ai.ts must never import the DB layer itself, so the closure is + // injected instead. Safe to register after createSelfHostAi() above: the resolver is only ever invoked at + // AI-call time, long after boot. Never throws -- a DB/decrypt failure returns null and resolution falls + // through to the secret file / boot env, so a rotation problem degrades a review instead of failing it. + setProviderCredentialResolver(async (provider) => getDecryptedProviderCredential(env, provider)); + // GitHub App auth: a successful JWT mint proves GITHUB_APP_PRIVATE_KEY is set and parses as a valid signing // key. Without this, an invalid/expired key leaves the review pipeline completely dead while /ready still // reports 200 — detection otherwise requires SENTRY_DSN or grepping stdout for auth errors (#2497). The diff --git a/test/unit/mcp-admin-rotate-secret-tool.test.ts b/test/unit/mcp-admin-rotate-secret-tool.test.ts new file mode 100644 index 0000000000..f4acd04e51 --- /dev/null +++ b/test/unit/mcp-admin-rotate-secret-tool.test.ts @@ -0,0 +1,152 @@ +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 { setSecretRotator } from "../../src/mcp/redeploy-companion-registry"; +import type { AuthIdentity } from "../../src/auth/security"; +import { createTestEnv } from "../helpers/d1"; + +// The one-call rotation surface (#9543), on top of the same host companion the redeploy tool uses. Mirrors +// mcp-admin-redeploy-tool.test.ts, which covers the sibling tool's gating/auth/audit in the same shape. + +const MCP_ADMIN_IDENTITY: AuthIdentity = { kind: "static", actor: "mcp-admin" }; +const MCP_ORDINARY_IDENTITY: AuthIdentity = { kind: "static", actor: "mcp" }; +const VALID = { secret: "claude_code_oauth_token", value: "sk-ant-oat01-rotated" }; + +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-rotate-secret-test", version: "0.1.0" }, { capabilities: {} }); + await client.connect(clientTransport); + return client; +} + +afterEach(() => { + setSecretRotator(null); +}); + +describe("MCP admin rotate-secret tool: registration gating (#9543)", () => { + 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_rotate_secret")).toBe(false); + }); + + it("IS registered, with the admin category, when the flag 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_rotate_secret"); + expect(tool).toBeDefined(); + expect((tool!._meta as { category?: string } | undefined)?.category).toBe("admin"); + }); +}); + +describe("MCP admin rotate-secret tool: auth boundary (#9543)", () => { + it("rejects the ordinary mcp actor even when the flag is on and a rotator is configured", async () => { + const rotator = vi.fn(); + setSecretRotator(rotator); + const client = await connect(createTestEnv({ LOOPOVER_MCP_ADMIN_ENABLED: "true" }), MCP_ORDINARY_IDENTITY); + const result = await client.callTool({ name: "loopover_admin_rotate_secret", arguments: VALID }); + expect(result.isError).toBe(true); + expect(rotator).not.toHaveBeenCalled(); + }); + + it("rejects a session identity too -- a static-credential-only surface", async () => { + setSecretRotator(vi.fn()); + 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_rotate_secret", arguments: VALID }); + expect(result.isError).toBe(true); + }); +}); + +describe("MCP admin rotate-secret tool: behaviour (#9543)", () => { + it("reports not-configured when no companion is installed", async () => { + const client = await connect(createTestEnv({ LOOPOVER_MCP_ADMIN_ENABLED: "true" })); + const result = await client.callTool({ name: "loopover_admin_rotate_secret", arguments: VALID }); + expect((result.structuredContent as { configured: boolean }).configured).toBe(false); + }); + + it("rotates via the companion and reports that no restart is needed for the claude token", async () => { + setSecretRotator(async () => ({ ok: true, backupPath: "/opt/loopover/.deploy-backups/x.bak" })); + const client = await connect(createTestEnv({ LOOPOVER_MCP_ADMIN_ENABLED: "true" })); + const result = await client.callTool({ name: "loopover_admin_rotate_secret", arguments: VALID }); + expect(result.structuredContent).toMatchObject({ configured: true, ok: true, secret: "claude_code_oauth_token", backupPath: "/opt/loopover/.deploy-backups/x.bak" }); + expect(JSON.stringify(result.content)).toMatch(/No restart needed/i); + }); + + it("tells the operator to restart for a secret that is only read at boot", async () => { + setSecretRotator(async () => ({ ok: true })); + const client = await connect(createTestEnv({ LOOPOVER_MCP_ADMIN_ENABLED: "true" })); + const result = await client.callTool({ name: "loopover_admin_rotate_secret", arguments: { secret: "github_webhook_secret", value: "whsec-rotated" } }); + expect(JSON.stringify(result.content)).toMatch(/Restart the loopover service/i); + }); + + it("surfaces a host-side refusal without claiming success", async () => { + setSecretRotator(async () => ({ ok: false, error: "invalid_secret_value" })); + const client = await connect(createTestEnv({ LOOPOVER_MCP_ADMIN_ENABLED: "true" })); + const result = await client.callTool({ name: "loopover_admin_rotate_secret", arguments: VALID }); + expect(result.structuredContent).toMatchObject({ configured: true, ok: false, error: "invalid_secret_value" }); + }); + + it("surfaces an unreachable companion as a failure, not a success", async () => { + setSecretRotator(async () => { + throw new Error("ENOENT: no such socket"); + }); + const client = await connect(createTestEnv({ LOOPOVER_MCP_ADMIN_ENABLED: "true" })); + const result = await client.callTool({ name: "loopover_admin_rotate_secret", arguments: VALID }); + expect(result.structuredContent).toMatchObject({ configured: true, ok: false }); + expect(JSON.stringify(result.content)).toMatch(/could not reach the host companion/i); + }); + + it("reports a refusal that carries no error message without printing 'undefined'", async () => { + setSecretRotator(async () => ({ ok: false })); + const client = await connect(createTestEnv({ LOOPOVER_MCP_ADMIN_ENABLED: "true" })); + const result = await client.callTool({ name: "loopover_admin_rotate_secret", arguments: VALID }); + expect(result.structuredContent).toMatchObject({ configured: true, ok: false }); + expect(JSON.stringify(result.content)).toMatch(/unknown error/i); + expect(JSON.stringify(result.content)).not.toMatch(/undefined/); + }); + + it("handles a non-Error rejection from the companion client", async () => { + setSecretRotator(async () => { + throw "socket exploded"; // eslint-disable-line no-throw-literal -- exercises the String(error) arm + }); + const client = await connect(createTestEnv({ LOOPOVER_MCP_ADMIN_ENABLED: "true" })); + const result = await client.callTool({ name: "loopover_admin_rotate_secret", arguments: VALID }); + expect(result.structuredContent).toMatchObject({ configured: true, ok: false, error: "socket exploded" }); + }); + + it("NEVER echoes the credential back in the result or the summary", async () => { + setSecretRotator(async () => ({ ok: true })); + const client = await connect(createTestEnv({ LOOPOVER_MCP_ADMIN_ENABLED: "true" })); + const result = await client.callTool({ name: "loopover_admin_rotate_secret", arguments: VALID }); + const serialized = JSON.stringify(result); + expect(serialized).not.toContain(VALID.value); + expect(serialized).toContain("claude_code_oauth_token"); // which secret IS reported + }); + + it("rejects a multi-line value at the schema boundary, before the companion is called", async () => { + const rotator = vi.fn(async () => ({ ok: true })); + setSecretRotator(rotator); + const client = await connect(createTestEnv({ LOOPOVER_MCP_ADMIN_ENABLED: "true" })); + const result = await client.callTool({ name: "loopover_admin_rotate_secret", arguments: { secret: "claude_code_oauth_token", value: "# label\nsk-ant-real" } }); + expect(result.isError).toBe(true); + expect(rotator).not.toHaveBeenCalled(); + }); + + it("rejects a padded value, a comment value, and an unknown secret name at the schema boundary", async () => { + const rotator = vi.fn(async () => ({ ok: true })); + setSecretRotator(rotator); + const client = await connect(createTestEnv({ LOOPOVER_MCP_ADMIN_ENABLED: "true" })); + for (const args of [ + { secret: "claude_code_oauth_token", value: " sk-ant-x" }, + { secret: "claude_code_oauth_token", value: "#sk-ant-x" }, + { secret: "claude_code_oauth_token", value: "" }, + { secret: "etc_passwd", value: "sk-ant-x" }, + ]) { + expect((await client.callTool({ name: "loopover_admin_rotate_secret", arguments: args })).isError).toBe(true); + } + expect(rotator).not.toHaveBeenCalled(); + }); +}); diff --git a/test/unit/provider-credential-registry.test.ts b/test/unit/provider-credential-registry.test.ts new file mode 100644 index 0000000000..287ea44e15 --- /dev/null +++ b/test/unit/provider-credential-registry.test.ts @@ -0,0 +1,40 @@ +import { afterEach, describe, expect, it } from "vitest"; +import { getProviderCredentialResolver, setProviderCredentialResolver } from "../../src/selfhost/provider-credential-registry"; + +// The Workers-safe nullable slot that carries the fleet-mode (DB-backed) credential lookup into +// src/selfhost/ai.ts without ai.ts ever importing the DB layer (#9543). Same shape as +// redeploy-companion-registry.test.ts, which covers the sibling slot. + +afterEach(() => { + setProviderCredentialResolver(null); +}); + +describe("provider-credential-registry (#9543)", () => { + it("returns null before anything is set (cloud, or a box with no stored credential)", () => { + expect(getProviderCredentialResolver()).toBeNull(); + }); + + it("returns the injected resolver and passes the provider through", async () => { + const seen: string[] = []; + setProviderCredentialResolver(async (provider) => { + seen.push(provider); + return "stored-credential"; + }); + const resolver = getProviderCredentialResolver(); + expect(resolver).not.toBeNull(); + await expect(resolver!("claude-code")).resolves.toBe("stored-credential"); + await expect(resolver!("codex")).resolves.toBe("stored-credential"); + expect(seen).toEqual(["claude-code", "codex"]); + }); + + it("carries a null result through for a provider with nothing stored", async () => { + setProviderCredentialResolver(async () => null); + await expect(getProviderCredentialResolver()!("claude-code")).resolves.toBeNull(); + }); + + it("clears back to null", () => { + setProviderCredentialResolver(async () => "x"); + setProviderCredentialResolver(null); + expect(getProviderCredentialResolver()).toBeNull(); + }); +}); diff --git a/test/unit/provider-credentials.test.ts b/test/unit/provider-credentials.test.ts new file mode 100644 index 0000000000..1e96e6347a --- /dev/null +++ b/test/unit/provider-credentials.test.ts @@ -0,0 +1,204 @@ +import { eq } from "drizzle-orm"; +import { describe, expect, it } from "vitest"; +import { createApp } from "../../src/api/routes"; +import { getDb } from "../../src/db/client"; +import { providerCredentials } from "../../src/db/schema"; +import { deleteProviderCredential, getDecryptedProviderCredential, getProviderCredentialStatus, upsertProviderCredential } from "../../src/db/repositories"; +import { createTestEnv } from "../helpers/d1"; + +// Instance subscription-CLI credentials (#9543) -- the FLEET rotation path. Mirrors ai-key-byok.test.ts, +// which covers the per-repo BYOK table this deliberately reuses the crypto envelope from. + +const SECRET = "unit-test-encryption-secret-at-least-32-bytes-long"; +const TOKEN = "sk-ant-oat01-fleet-credential-value"; + +describe("provider credential storage (#9543)", () => { + it("reports not-configured before anything is stored", async () => { + const env = await createTestEnv({ TOKEN_ENCRYPTION_SECRET: SECRET }); + await expect(getProviderCredentialStatus(env, "claude-code")).resolves.toEqual({ configured: false }); + await expect(getDecryptedProviderCredential(env, "claude-code")).resolves.toBeNull(); + }); + + it("stores encrypted at rest and round-trips the plaintext", async () => { + const env = await createTestEnv({ TOKEN_ENCRYPTION_SECRET: SECRET }); + const status = await upsertProviderCredential(env, { provider: "claude-code", credential: TOKEN, updatedBy: "operator" }); + expect(status).toMatchObject({ configured: true, provider: "claude-code", last4: TOKEN.slice(-4), updatedBy: "operator" }); + + // The stored row must never contain the plaintext anywhere. + const [row] = await getDb(env.DB).select().from(providerCredentials).where(eq(providerCredentials.provider, "claude-code")).limit(1); + expect(row!.ciphertext).not.toContain(TOKEN); + expect(JSON.stringify(row)).not.toContain(TOKEN); + expect(row!.keyVersion).toBe(2); + + await expect(getDecryptedProviderCredential(env, "claude-code")).resolves.toBe(TOKEN); + }); + + it("trims the credential before storing it", async () => { + const env = await createTestEnv({ TOKEN_ENCRYPTION_SECRET: SECRET }); + await upsertProviderCredential(env, { provider: "claude-code", credential: ` ${TOKEN}\n` }); + await expect(getDecryptedProviderCredential(env, "claude-code")).resolves.toBe(TOKEN); + }); + + it("replaces in place rather than accumulating rows", async () => { + const env = await createTestEnv({ TOKEN_ENCRYPTION_SECRET: SECRET }); + await upsertProviderCredential(env, { provider: "claude-code", credential: TOKEN }); + await upsertProviderCredential(env, { provider: "claude-code", credential: "sk-ant-oat01-rotated-value" }); + const rows = await getDb(env.DB).select().from(providerCredentials); + expect(rows).toHaveLength(1); + await expect(getDecryptedProviderCredential(env, "claude-code")).resolves.toBe("sk-ant-oat01-rotated-value"); + }); + + it("keeps providers isolated from each other", async () => { + const env = await createTestEnv({ TOKEN_ENCRYPTION_SECRET: SECRET }); + await upsertProviderCredential(env, { provider: "claude-code", credential: TOKEN }); + await expect(getDecryptedProviderCredential(env, "codex")).resolves.toBeNull(); + await upsertProviderCredential(env, { provider: "codex", credential: "codex-credential" }); + await expect(getDecryptedProviderCredential(env, "claude-code")).resolves.toBe(TOKEN); + await expect(getDecryptedProviderCredential(env, "codex")).resolves.toBe("codex-credential"); + // Status round-trips the provider back out of the row for BOTH providers, not just the default arm. + await expect(getProviderCredentialStatus(env, "codex")).resolves.toMatchObject({ configured: true, provider: "codex" }); + await expect(getProviderCredentialStatus(env, "claude-code")).resolves.toMatchObject({ configured: true, provider: "claude-code" }); + }); + + it("defaults updatedBy to null when no actor is supplied", async () => { + const env = await createTestEnv({ TOKEN_ENCRYPTION_SECRET: SECRET }); + await expect(upsertProviderCredential(env, { provider: "claude-code", credential: TOKEN })).resolves.toMatchObject({ updatedBy: null }); + }); + + it("refuses to store anything in the clear when the encryption secret is missing", async () => { + const env = await createTestEnv({}); + await expect(upsertProviderCredential(env, { provider: "claude-code", credential: TOKEN })).rejects.toThrow(/missing_encryption_secret/); + expect(await getDb(env.DB).select().from(providerCredentials)).toHaveLength(0); + }); + + it("refuses an all-whitespace credential", async () => { + const env = await createTestEnv({ TOKEN_ENCRYPTION_SECRET: SECRET }); + await expect(upsertProviderCredential(env, { provider: "claude-code", credential: " " })).rejects.toThrow(/empty_credential/); + }); + + it("returns null (never throws) when the encryption secret is unavailable at read time", async () => { + const env = await createTestEnv({ TOKEN_ENCRYPTION_SECRET: SECRET }); + await upsertProviderCredential(env, { provider: "claude-code", credential: TOKEN }); + // A resolution failure must degrade to the next rung, not fail the review. + await expect(getDecryptedProviderCredential({ ...env, TOKEN_ENCRYPTION_SECRET: undefined } as never, "claude-code")).resolves.toBeNull(); + }); + + it("returns null when the stored row cannot be decrypted with the current secret", async () => { + const env = await createTestEnv({ TOKEN_ENCRYPTION_SECRET: SECRET }); + await upsertProviderCredential(env, { provider: "claude-code", credential: TOKEN }); + await expect(getDecryptedProviderCredential({ ...env, TOKEN_ENCRYPTION_SECRET: "a-different-secret-at-least-32-bytes!!" } as never, "claude-code")).resolves.toBeNull(); + }); + + it("delete clears the credential so resolution falls back to the file/env", async () => { + const env = await createTestEnv({ TOKEN_ENCRYPTION_SECRET: SECRET }); + await upsertProviderCredential(env, { provider: "claude-code", credential: TOKEN }); + await deleteProviderCredential(env, "claude-code", "operator"); + await expect(getProviderCredentialStatus(env, "claude-code")).resolves.toEqual({ configured: false }); + await expect(getDecryptedProviderCredential(env, "claude-code")).resolves.toBeNull(); + }); + + it("delete on an absent credential is a no-op", async () => { + const env = await createTestEnv({ TOKEN_ENCRYPTION_SECRET: SECRET }); + await expect(deleteProviderCredential(env, "claude-code")).resolves.toBeUndefined(); + }); +}); + +describe("provider_credentials schema defaults (#9543)", () => { + it("populates created_at and updated_at when a direct insert omits them", async () => { + // The table's $defaultFn timestamps: upsertProviderCredential always supplies updated_at explicitly, + // so this is the path that proves a row is still well-formed without it. + const env = await createTestEnv({ TOKEN_ENCRYPTION_SECRET: SECRET }); + await getDb(env.DB).insert(providerCredentials).values({ provider: "claude-code", ciphertext: "c", iv: "i", salt: null, keyVersion: 2, last4: "abcd", updatedBy: null }); + const [row] = await getDb(env.DB).select().from(providerCredentials).where(eq(providerCredentials.provider, "claude-code")).limit(1); + expect(row!.createdAt).toMatch(/^\d{4}-\d{2}-\d{2}T/); + expect(row!.updatedAt).toMatch(/^\d{4}-\d{2}-\d{2}T/); + }); +}); + +describe("internal provider-credential routes (#9543)", () => { + const auth = { Authorization: "Bearer internal-token", "content-type": "application/json" }; + const envFor = async () => createTestEnv({ TOKEN_ENCRYPTION_SECRET: SECRET, INTERNAL_JOB_TOKEN: "internal-token" }); + + it("GET reports not-configured, then configured WITHOUT ever returning the credential", async () => { + const env = await envFor(); + const app = createApp(); + const before = await app.request("/v1/internal/provider-credentials/claude-code", { headers: auth }, env); + expect(before.status).toBe(200); + expect(await before.json()).toEqual({ configured: false }); + + const post = await app.request("/v1/internal/provider-credentials/claude-code", { method: "POST", headers: auth, body: JSON.stringify({ credential: TOKEN }) }, env); + expect(post.status).toBe(200); + + const after = await app.request("/v1/internal/provider-credentials/claude-code", { headers: auth }, env); + const body = await after.text(); + expect(body).not.toContain(TOKEN); + expect(JSON.parse(body)).toMatchObject({ configured: true, last4: TOKEN.slice(-4) }); + }); + + it("POST rejects a multi-line credential — the exact production footgun", async () => { + const env = await envFor(); + const res = await createApp().request( + "/v1/internal/provider-credentials/claude-code", + { method: "POST", headers: auth, body: JSON.stringify({ credential: "# some-account\nsk-ant-oat01-real" }) }, + env, + ); + expect(res.status).toBe(400); + expect(await res.json()).toMatchObject({ error: "invalid_credential" }); + await expect(getProviderCredentialStatus(env, "claude-code")).resolves.toEqual({ configured: false }); + }); + + it("POST rejects a comment, padded, or empty credential", async () => { + const env = await envFor(); + const app = createApp(); + for (const credential of ["#sk-ant-x", " sk-ant-x", "sk-ant-x ", ""]) { + const res = await app.request("/v1/internal/provider-credentials/claude-code", { method: "POST", headers: auth, body: JSON.stringify({ credential }) }, env); + expect(res.status).toBe(400); + } + }); + + it("POST rejects a malformed or missing body", async () => { + const env = await envFor(); + const app = createApp(); + expect((await app.request("/v1/internal/provider-credentials/claude-code", { method: "POST", headers: auth, body: "not json" }, env)).status).toBe(400); + expect((await app.request("/v1/internal/provider-credentials/claude-code", { method: "POST", headers: auth, body: JSON.stringify({}) }, env)).status).toBe(400); + }); + + it("rejects an unknown provider on every verb", async () => { + const env = await envFor(); + const app = createApp(); + expect((await app.request("/v1/internal/provider-credentials/gpt-9", { headers: auth }, env)).status).toBe(400); + expect((await app.request("/v1/internal/provider-credentials/gpt-9", { method: "POST", headers: auth, body: JSON.stringify({ credential: "x" }) }, env)).status).toBe(400); + expect((await app.request("/v1/internal/provider-credentials/gpt-9", { method: "DELETE", headers: auth }, env)).status).toBe(400); + }); + + it("POST reports 503 rather than storing in the clear when encryption is unconfigured", async () => { + const env = await createTestEnv({ INTERNAL_JOB_TOKEN: "internal-token" }); + const res = await createApp().request("/v1/internal/provider-credentials/claude-code", { method: "POST", headers: auth, body: JSON.stringify({ credential: TOKEN }) }, env); + expect(res.status).toBe(503); + expect(await res.json()).toMatchObject({ error: "encryption_unavailable" }); + }); + + it("does not swallow an unexpected storage failure as a credential error", async () => { + // Only `missing_encryption_secret` is translated; anything else must propagate rather than be reported + // as a 400/503 that would tell an operator the credential was the problem. + const env = await envFor(); + await env.DB.prepare("DROP TABLE provider_credentials").run(); + const res = await createApp().request("/v1/internal/provider-credentials/claude-code", { method: "POST", headers: auth, body: JSON.stringify({ credential: TOKEN }) }, env); + expect(res.status).toBe(500); + }); + + it("DELETE clears a stored credential", async () => { + const env = await envFor(); + const app = createApp(); + await app.request("/v1/internal/provider-credentials/claude-code", { method: "POST", headers: auth, body: JSON.stringify({ credential: TOKEN }) }, env); + const res = await app.request("/v1/internal/provider-credentials/claude-code", { method: "DELETE", headers: auth }, env); + expect(res.status).toBe(200); + await expect(getProviderCredentialStatus(env, "claude-code")).resolves.toEqual({ configured: false }); + }); + + it("requires the internal job token", async () => { + const env = await envFor(); + const res = await createApp().request("/v1/internal/provider-credentials/claude-code", {}, env); + expect(res.status).toBe(401); + }); +}); diff --git a/test/unit/redeploy-companion-client.test.ts b/test/unit/redeploy-companion-client.test.ts index 94a5bf5a2a..ec4581523e 100644 --- a/test/unit/redeploy-companion-client.test.ts +++ b/test/unit/redeploy-companion-client.test.ts @@ -3,7 +3,7 @@ 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"; +import { triggerRedeploy, rotateCompanionSecret } 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 @@ -186,3 +186,42 @@ describe("triggerRedeploy (#7723)", () => { expect(result).toEqual({ ok: true, exitCode: 0, log: [] }); }); }); + +describe("rotateCompanionSecret (#9543)", () => { + it("sends the rotate-secret verb with the token, and returns the terminal result", async () => { + let received = ""; + const socketPath = startFakeCompanion((line, write, end) => { + received = line; + write(JSON.stringify({ ok: true, backupPath: "/opt/loopover/.deploy-backups/x.bak" })); + end(); + }); + const result = await rotateCompanionSecret({ socketPath, token: "tok" }, "claude_code_oauth_token", "sk-ant-new"); + expect(JSON.parse(received)).toEqual({ token: "tok", action: "rotate-secret", secret: "claude_code_oauth_token", value: "sk-ant-new" }); + expect(result).toEqual({ ok: true, backupPath: "/opt/loopover/.deploy-backups/x.bak" }); + }); + + it("surfaces a refusal from the host verbatim", async () => { + const socketPath = startFakeCompanion((_line, write, end) => { + write(JSON.stringify({ ok: false, error: "invalid_secret_value" })); + end(); + }); + await expect(rotateCompanionSecret({ socketPath, token: "tok" }, "claude_code_oauth_token", "bad")).resolves.toEqual({ ok: false, error: "invalid_secret_value" }); + }); + + it("omits backupPath when the host did not report one", async () => { + const socketPath = startFakeCompanion((_line, write, end) => { + write(JSON.stringify({ ok: true })); + end(); + }); + await expect(rotateCompanionSecret({ socketPath, token: "tok" }, "claude_code_oauth_token", "v")).resolves.toEqual({ ok: true }); + }); + + it("rejects when the companion closes before a terminal response", async () => { + const socketPath = startFakeCompanion((_line, _write, end) => end()); + await expect(rotateCompanionSecret({ socketPath, token: "tok" }, "claude_code_oauth_token", "v")).rejects.toThrow(/closed the connection/); + }); + + it("rejects when the socket does not exist", async () => { + await expect(rotateCompanionSecret({ socketPath: "/nonexistent/companion.sock", token: "tok" }, "claude_code_oauth_token", "v")).rejects.toThrow(); + }); +}); diff --git a/test/unit/redeploy-companion-registry.test.ts b/test/unit/redeploy-companion-registry.test.ts index e3df594b54..0d817defb0 100644 --- a/test/unit/redeploy-companion-registry.test.ts +++ b/test/unit/redeploy-companion-registry.test.ts @@ -1,5 +1,5 @@ import { afterEach, describe, expect, it } from "vitest"; -import { getRedeployTrigger, setRedeployTrigger } from "../../src/mcp/redeploy-companion-registry"; +import { getRedeployTrigger, setRedeployTrigger, getSecretRotator, setSecretRotator } from "../../src/mcp/redeploy-companion-registry"; afterEach(() => { setRedeployTrigger(null); @@ -22,3 +22,37 @@ describe("redeploy-companion-registry (#7723)", () => { expect(getRedeployTrigger()).toBeNull(); }); }); + +describe("secret rotator slot (#9543)", () => { + afterEach(() => { + setSecretRotator(null); + }); + + it("returns null before anything is set", () => { + expect(getSecretRotator()).toBeNull(); + }); + + it("returns the injected rotator and passes both arguments through", async () => { + const calls: Array<[string, string]> = []; + setSecretRotator(async (secret, value) => { + calls.push([secret, value]); + return { ok: true }; + }); + const rotator = getSecretRotator(); + expect(rotator).not.toBeNull(); + await expect(rotator!("claude_code_oauth_token", "sk-ant-x")).resolves.toEqual({ ok: true }); + expect(calls).toEqual([["claude_code_oauth_token", "sk-ant-x"]]); + }); + + it("clears back to null", () => { + setSecretRotator(async () => ({ ok: true })); + setSecretRotator(null); + expect(getSecretRotator()).toBeNull(); + }); + + it("is independent of the redeploy trigger slot", () => { + setSecretRotator(async () => ({ ok: true })); + expect(getRedeployTrigger()).toBeNull(); + expect(getSecretRotator()).not.toBeNull(); + }); +}); diff --git a/test/unit/redeploy-companion.test.ts b/test/unit/redeploy-companion.test.ts index ba4ff8eaa2..09092630c3 100644 --- a/test/unit/redeploy-companion.test.ts +++ b/test/unit/redeploy-companion.test.ts @@ -1,6 +1,6 @@ import { EventEmitter } from "node:events"; import { describe, expect, it, vi } from "vitest"; -import { handleConnection, runDeploy } from "../../scripts/redeploy-companion"; +import { handleConnection, runDeploy, rotateSecret } from "../../scripts/redeploy-companion"; const TOKEN = "companion-test-token"; @@ -208,3 +208,145 @@ describe("handleConnection (#7723)", () => { expect(written[1]).toBe(JSON.stringify({ ok: false, exitCode: null, error: "bash: not found" })); }); }); + +describe("rotateSecret (#9543 — the two silent footguns this exists to prevent)", () => { + const io = (overrides: Partial[2]> = {}) => { + const writes: Array<{ path: string; data: string; mode: number | undefined }> = []; + return { + writes, + io: { + readFileSync: (() => "previous-value") as never, + writeFileSync: ((path: string, data: string, opts?: { mode?: number }) => { + writes.push({ path, data, mode: opts?.mode }); + }) as never, + existsSync: (() => true) as never, + chmodSync: (() => undefined) as never, + now: () => new Date("2026-07-28T00:00:00.000Z"), + ...overrides, + }, + }; + }; + + it("rejects an unknown secret name rather than writing an arbitrary path", () => { + const { io: fake, writes } = io(); + expect(rotateSecret("../../etc/passwd", "x", fake)).toEqual({ ok: false, error: "unknown_secret" }); + expect(writes).toHaveLength(0); + }); + + it("rejects a non-string secret name", () => { + expect(rotateSecret(42, "x", io().io)).toEqual({ ok: false, error: "unknown_secret" }); + }); + + it("REGRESSION: rejects a value with a comment/label line above it", () => { + // The exact shape that silently became part of the credential in production: the loader only trims, + // so both lines would have been sent to the CLI as one token and every AI call would fail auth. + const { io: fake, writes } = io(); + expect(rotateSecret("claude_code_oauth_token", "# some-account\nsk-ant-oat01-real", fake)).toEqual({ ok: false, error: "invalid_secret_value" }); + expect(writes).toHaveLength(0); + }); + + it("rejects a value that is itself a comment, is empty, is padded, or is oversized", () => { + const { io: fake } = io(); + for (const bad of ["#sk-ant-x", "", " sk-ant-x", "sk-ant-x ", "sk-ant-x\r\n", "x".repeat(4097)]) { + expect(rotateSecret("claude_code_oauth_token", bad, fake)).toEqual({ ok: false, error: "invalid_secret_value" }); + } + }); + + it("rejects a non-string value", () => { + expect(rotateSecret("claude_code_oauth_token", { token: "x" }, io().io)).toEqual({ ok: false, error: "invalid_secret_value" }); + }); + + it("writes the bare value with no trailing newline, at 0644, and backs up the previous value first", () => { + const { io: fake, writes } = io(); + const result = rotateSecret("claude_code_oauth_token", "sk-ant-oat01-new", fake); + expect(result.ok).toBe(true); + expect(result.backupPath).toContain(".deploy-backups/claude_code_oauth_token.txt.bak-"); + // Backup first (mode 600), then the in-place write of the bare value (mode 644). + expect(writes[0]!.data).toBe("previous-value"); + expect(writes[0]!.mode).toBe(0o600); + expect(writes[1]!.data).toBe("sk-ant-oat01-new"); + expect(writes[1]!.mode).toBe(0o644); + expect(writes[1]!.path).toContain("secrets/claude_code_oauth_token.txt"); + }); + + it("skips the backup when no prior value exists", () => { + const { io: fake, writes } = io({ existsSync: (() => false) as never }); + expect(rotateSecret("claude_code_oauth_token", "sk-ant-first", fake)).toEqual({ ok: true }); + expect(writes).toHaveLength(1); + expect(writes[0]!.data).toBe("sk-ant-first"); + }); + + it("reports a write failure instead of throwing", () => { + const { io: fake } = io({ + writeFileSync: (() => { + throw new Error("EACCES: permission denied"); + }) as never, + }); + expect(rotateSecret("claude_code_oauth_token", "sk-ant-x", fake)).toEqual({ ok: false, error: "EACCES: permission denied" }); + }); + + it("accepts every allowlisted secret name", () => { + const { io: fake } = io(); + for (const name of ["claude_code_oauth_token", "github_webhook_secret", "loopover_api_token", "loopover_mcp_token", "loopover_mcp_admin_token", "pagerduty_routing_key"]) { + expect(rotateSecret(name, "some-value", fake).ok).toBe(true); + } + }); +}); + +describe("handleConnection verb dispatch (#9543)", () => { + const collect = () => { + const lines: string[] = []; + return { lines, write: (line: string) => lines.push(line) }; + }; + + it("routes a rotate-secret request to the rotator, not the deployer", async () => { + const { lines, write } = collect(); + const deploy = vi.fn(); + const rotate = vi.fn(() => ({ ok: true, backupPath: "/b" })); + await handleConnection(TOKEN, JSON.stringify({ token: TOKEN, action: "rotate-secret", secret: "claude_code_oauth_token", value: "sk-ant-x" }), () => false, () => {}, write, deploy as never, rotate as never); + expect(deploy).not.toHaveBeenCalled(); + expect(rotate).toHaveBeenCalledWith("claude_code_oauth_token", "sk-ant-x"); + expect(JSON.parse(lines[0]!)).toEqual({ ok: true, backupPath: "/b" }); + }); + + it("serves a rotation even while a redeploy is in progress", async () => { + // A 15-minute redeploy is exactly when an operator is most likely to need a credential fixed, and a + // single truncating write races nothing that the deploy is doing. + const { lines, write } = collect(); + const rotate = vi.fn(() => ({ ok: true })); + await handleConnection(TOKEN, JSON.stringify({ token: TOKEN, action: "rotate-secret", secret: "claude_code_oauth_token", value: "v" }), () => true, () => {}, write, vi.fn() as never, rotate as never); + expect(JSON.parse(lines[0]!)).toEqual({ ok: true }); + }); + + it("rejects a rotation with a bad token before reaching the rotator", async () => { + const { lines, write } = collect(); + const rotate = vi.fn(); + await handleConnection(TOKEN, JSON.stringify({ token: "wrong", action: "rotate-secret", secret: "claude_code_oauth_token", value: "v" }), () => false, () => {}, write, vi.fn() as never, rotate as never); + expect(rotate).not.toHaveBeenCalled(); + expect(JSON.parse(lines[0]!)).toEqual({ ok: false, error: "unauthorized" }); + }); + + it("rejects an unrecognised action", async () => { + const { lines, write } = collect(); + await handleConnection(TOKEN, JSON.stringify({ token: TOKEN, action: "rm-rf" }), () => false, () => {}, write, vi.fn() as never, vi.fn() as never); + expect(JSON.parse(lines[0]!)).toEqual({ ok: false, error: "unknown_action" }); + }); + + it("BACKWARD COMPAT: a request with no action still redeploys", async () => { + // The app container and the host companion upgrade independently, so a new companion must keep + // serving an old client that has never heard of `action`. + const { lines, write } = collect(); + const deploy = vi.fn(async () => ({ ok: true, exitCode: 0 })); + await handleConnection(TOKEN, JSON.stringify({ token: TOKEN }), () => false, () => {}, write, deploy as never, vi.fn() as never); + expect(deploy).toHaveBeenCalled(); + expect(JSON.parse(lines[0]!)).toEqual({ ok: true, exitCode: 0 }); + }); + + it("an explicit action of redeploy behaves identically", async () => { + const { lines, write } = collect(); + const deploy = vi.fn(async () => ({ ok: true, exitCode: 0 })); + await handleConnection(TOKEN, JSON.stringify({ token: TOKEN, action: "redeploy" }), () => false, () => {}, write, deploy as never, vi.fn() as never); + expect(deploy).toHaveBeenCalled(); + expect(JSON.parse(lines[0]!)).toEqual({ ok: true, exitCode: 0 }); + }); +}); diff --git a/test/unit/selfhost-ai.test.ts b/test/unit/selfhost-ai.test.ts index 3e0198b191..b23c521e59 100644 --- a/test/unit/selfhost-ai.test.ts +++ b/test/unit/selfhost-ai.test.ts @@ -20,6 +20,8 @@ vi.mock("posthog-node", () => ({ PostHog: posthogMocks.PostHog })); import { assertNoLegacySharedAiEnv, buildProvider, claudeErrorStatus, codexErrorFromStdout, createAnthropicAi, createChainAi, createClaudeCodeAi, createCodexAi, createOpenAiCompatibleAi, createSelfHostAi, extractCliText, extractCliUsage, isAiProviderHealthy, markAiProviderUnhealthyAtBoot, providerNameFromBaseUrl, resetAiProviderCircuitBreakerForTest, resetAiProviderHealthForTest, resolveAiReviewerPlan, resolveClaudeCliTimeoutMs, resolveClaudeFirstOutputTimeoutMs, resolveCodexAuthPath, resolveCodexCliTimeoutMs, resolveCodexEffort, resolveCodexFirstOutputTimeoutMs, resolveEffort, resolveModel, resolveProviderNames, resolveRequiredCliProviders, resolveSubscriptionCliPath, redactSecrets, routeProviders, shouldMarkAiProviderUnhealthyAtBoot, shouldWarnRagEmbedUnavailable, subscriptionCliEnv, withAdvisoryAiEnv, withAiGenerationCapture, __selfHostAiInternals } from "../../src/selfhost/ai"; import { labelSelfHostReviewerModel, labelSelfHostReviewerModels } from "../../src/selfhost/ai-config"; +import { setFileSourcedSecrets } from "../../src/selfhost/file-sourced-secrets"; +import { setProviderCredentialResolver } from "../../src/selfhost/provider-credential-registry"; import { renderMetrics, resetMetrics } from "../../src/selfhost/metrics"; import { initPostHog, resetPostHogForTest } from "../../src/selfhost/posthog"; import { ollamaContextOptions, ollamaNumCtx } from "../../src/selfhost/ai"; @@ -2521,3 +2523,115 @@ describe("ollama context window (#9478)", () => { expect(ollamaNumCtx()).toBeGreaterThan(8_192); // must exceed the truncation-prone stock defaults }); }); + +describe("resolveClaudeOauthToken (#9543 — rotate the credential without recreating the container)", () => { + // The token reaches the CLI only through the spawned subprocess env, so asserting on that is what + // actually proves which rung of the resolution order won. + const captureToken = () => { + const seen: Array = []; + const spawn: StubSpawn = async (_cmd, _args, options) => { + seen.push((options as { env?: Record }).env?.CLAUDE_CODE_OAUTH_TOKEN); + return { stdout: JSON.stringify({ type: "result", result: "ok" }), code: 0 }; + }; + return { seen, spawn }; + }; + const run = (env: Record, spawn: StubSpawn) => + createClaudeCodeAi(env, spawn).run("m", { prompt: "x" }); + + afterEach(() => { + setFileSourcedSecrets([]); + setProviderCredentialResolver(null); + }); + + it("uses the boot env value when no file and no resolver are configured", async () => { + const { seen, spawn } = captureToken(); + await run({ CLAUDE_CODE_OAUTH_TOKEN: "boot-token" }, spawn); + expect(seen[0]).toBe("boot-token"); + }); + + it("prefers a stored fleet credential over both the file and the boot env", async () => { + const dir = mkdtempSync(join(tmpdir(), "rot-")); + const file = join(dir, "tok"); + writeFileSync(file, "file-token"); + setFileSourcedSecrets(["CLAUDE_CODE_OAUTH_TOKEN"]); + setProviderCredentialResolver(async () => "fleet-token"); + const { seen, spawn } = captureToken(); + await run({ CLAUDE_CODE_OAUTH_TOKEN: "boot-token", CLAUDE_CODE_OAUTH_TOKEN_FILE: file }, spawn); + expect(seen[0]).toBe("fleet-token"); + }); + + it("passes the fleet credential through trimmed", async () => { + setProviderCredentialResolver(async () => " padded-token\n"); + const { seen, spawn } = captureToken(); + await run({ CLAUDE_CODE_OAUTH_TOKEN: "boot-token" }, spawn); + expect(seen[0]).toBe("padded-token"); + }); + + it("falls through to the next rung when the resolver returns null", async () => { + setProviderCredentialResolver(async () => null); + const { seen, spawn } = captureToken(); + await run({ CLAUDE_CODE_OAUTH_TOKEN: "boot-token" }, spawn); + expect(seen[0]).toBe("boot-token"); + }); + + it("falls through when the resolver returns an all-whitespace credential", async () => { + setProviderCredentialResolver(async () => " "); + const { seen, spawn } = captureToken(); + await run({ CLAUDE_CODE_OAUTH_TOKEN: "boot-token" }, spawn); + expect(seen[0]).toBe("boot-token"); + }); + + it("degrades to the boot env instead of failing the review when the resolver throws", async () => { + setProviderCredentialResolver(async () => { throw new Error("db down"); }); + const { seen, spawn } = captureToken(); + await run({ CLAUDE_CODE_OAUTH_TOKEN: "boot-token" }, spawn); + expect(seen[0]).toBe("boot-token"); + }); + + it("re-reads the secret file per call, so an in-place rotation lands with no restart", async () => { + const dir = mkdtempSync(join(tmpdir(), "rot-")); + const file = join(dir, "tok"); + writeFileSync(file, "first-token"); + setFileSourcedSecrets(["CLAUDE_CODE_OAUTH_TOKEN"]); + const { seen, spawn } = captureToken(); + const env = { CLAUDE_CODE_OAUTH_TOKEN: "first-token", CLAUDE_CODE_OAUTH_TOKEN_FILE: file }; + await run(env, spawn); + writeFileSync(file, "rotated-token\n"); // in-place rewrite, exactly as the rotation path does it + await run(env, spawn); + expect(seen).toEqual(["first-token", "rotated-token"]); + }); + + it("does NOT re-read the file when the value was set inline (inline .env always wins)", async () => { + // docker-compose.yml sets _FILE unconditionally, so its presence alone must never cause a + // re-read -- that would silently swap an inline operator's credential. + const dir = mkdtempSync(join(tmpdir(), "rot-")); + const file = join(dir, "tok"); + writeFileSync(file, "file-token"); + const { seen, spawn } = captureToken(); + await run({ CLAUDE_CODE_OAUTH_TOKEN: "inline-token", CLAUDE_CODE_OAUTH_TOKEN_FILE: file }, spawn); + expect(seen[0]).toBe("inline-token"); + }); + + it("keeps the last known good value when the file is momentarily empty mid-rotation", async () => { + const dir = mkdtempSync(join(tmpdir(), "rot-")); + const file = join(dir, "tok"); + writeFileSync(file, " \n"); + setFileSourcedSecrets(["CLAUDE_CODE_OAUTH_TOKEN"]); + const { seen, spawn } = captureToken(); + await run({ CLAUDE_CODE_OAUTH_TOKEN: "boot-token", CLAUDE_CODE_OAUTH_TOKEN_FILE: file }, spawn); + expect(seen[0]).toBe("boot-token"); + }); + + it("degrades to the boot env when the file is unreadable", async () => { + setFileSourcedSecrets(["CLAUDE_CODE_OAUTH_TOKEN"]); + const { seen, spawn } = captureToken(); + await run({ CLAUDE_CODE_OAUTH_TOKEN: "boot-token", CLAUDE_CODE_OAUTH_TOKEN_FILE: "/nonexistent/definitely/not/here" }, spawn); + expect(seen[0]).toBe("boot-token"); + }); + + it("still reports no-token when every rung is empty", async () => { + setFileSourcedSecrets(["CLAUDE_CODE_OAUTH_TOKEN"]); + const spawn: StubSpawn = async () => ({ stdout: "", code: 0 }); + await expect(createClaudeCodeAi({ CLAUDE_CODE_OAUTH_TOKEN_FILE: "/nonexistent/nope" }, spawn).run("m", { prompt: "x" })).rejects.toThrow(/claude_code_no_oauth_token/); + }); +}); diff --git a/test/unit/selfhost-load-file-secrets.test.ts b/test/unit/selfhost-load-file-secrets.test.ts index 01a5da910b..5cacf69f5c 100644 --- a/test/unit/selfhost-load-file-secrets.test.ts +++ b/test/unit/selfhost-load-file-secrets.test.ts @@ -154,3 +154,29 @@ describe("empty secret files are fatal, not silently unconfigured (#9487)", () = expect(readFile).not.toHaveBeenCalled(); }); }); + +describe("loadFileSecrets return value (#9543 — which vars actually came from a file)", () => { + it("returns only the vars it materialised from a file", () => { + const env: Record = { + FROM_FILE_FILE: "/secrets/a", + ALREADY_SET: "inline-value", + ALREADY_SET_FILE: "/secrets/b", + PLAIN: "not-a-file-var", + }; + const names = loadFileSecrets(env, (path) => (path === "/secrets/a" ? " file-value\n" : "unused")); + // ALREADY_SET is excluded because an inline value wins -- that exclusion is the whole point: a + // call-time re-read must not swap an inline operator's credential (secrets/README.md). + expect(names).toEqual(["FROM_FILE"]); + expect(env.FROM_FILE).toBe("file-value"); + expect(env.ALREADY_SET).toBe("inline-value"); + }); + + it("returns an empty list when nothing is file-sourced", () => { + expect(loadFileSecrets({ PLAIN: "x" }, () => "unused")).toEqual([]); + }); + + it("excludes Compose's own reserved _FILE vars", () => { + const env: Record = { COMPOSE_FILE: "a.yml:b.yml", COMPOSE_ENV_FILE: "/custom/.env" }; + expect(loadFileSecrets(env, () => "unused")).toEqual([]); + }); +});