diff --git a/src/agents-config.ts b/src/agents-config.ts index 434b08d..fd8a041 100644 --- a/src/agents-config.ts +++ b/src/agents-config.ts @@ -33,7 +33,7 @@ export interface EnsureAgentEndpointResult { command?: string; cwd?: string; logPath?: string; - reason?: "not_registered" | "start_timeout"; + reason?: "not_registered" | "start_timeout" | "not_running"; } interface ReplayProjectRegistryEntry { @@ -348,16 +348,12 @@ export async function registerReplayProject( return { cwd: resolvedCwd, configPath, agents: Object.keys(agentEntries) }; } -export async function registerReplayProjectIfPresent(cwd: string): Promise { - try { - const configPath = getAgentsYamlPath(path.resolve(cwd)); - if (!fs.existsSync(configPath)) return false; - await registerReplayProject(cwd, { validate: false }); - return true; - } catch { - return false; - } -} +// `registerReplayProjectIfPresent` was removed: it registered replay commands +// from a caller-supplied directory without validation as a side effect of an +// HTTP workspace change, which let an attacker-writable `.raindrop/agents.yaml` +// land an executable `command` in the replay registry. Replay registration is +// now exclusively the explicit `raindrop replay register` CLI flow +// (`registerReplayProject`), which runs under direct user intent. export function getAgentEndpoint(eventName: string): AgentConfig | null { const config = loadAgentsConfig(); @@ -511,46 +507,23 @@ export async function ensureAgentEndpointDetailed(eventName: string): Promise setTimeout(resolve, 300)); - const healthy = await isAgentHealthy(config); - if (healthy?.url) { - return { - eventName: name, - config: { ...config, ...healthy }, - registered: true, - attemptedStart: true, - command: config.command, - cwd: config.cwd, - logPath, - }; - } - const rescanned = await discoverReplayAgents(); - if (rescanned[name]?.url) { - return { - eventName: name, - config: rescanned[name], - registered: true, - attemptedStart: true, - command: config.command, - cwd: config.cwd, - logPath, - }; - } - } + // A command-bearing config exists but no healthy agent is running. We must + // NOT spawn `config.command` here: this function is reached over loopback + // HTTP via `/api/replay`, and the command originates from a registry entry + // that an attacker may control (see `replay-projects.json`). Spawning a + // replay agent is an explicit local action performed by + // `raindrop replay register`, which validates and starts the agent under + // direct user intent. The HTTP replay path only connects to an + // already-running agent. return { eventName: name, config: null, registered: true, - attemptedStart: true, + attemptedStart: false, command: config.command, cwd: config.cwd, - logPath, - reason: "start_timeout", + reason: "not_running", }; } diff --git a/src/replay.ts b/src/replay.ts index 22fb924..05c7cb1 100644 --- a/src/replay.ts +++ b/src/replay.ts @@ -192,6 +192,26 @@ async function runLocalAgentReplay( const endpoint = await ensureAgentEndpointDetailed(eventName); const agentConfig = endpoint.config; if (!agentConfig?.url) { + if (endpoint.reason === "not_running") { + // A replay command is registered for this event but no agent is running. + // Workshop intentionally does not spawn it from this HTTP path; starting + // the agent is an explicit local action. + sendSSE(res, "error", { + code: "replay_agent_not_running", + setupRequired: true, + eventName, + message: + `A replay agent is registered for "${eventName}" but is not running. ` + + "Workshop does not start replay commands from the HTTP replay path.", + suggestedAction: + `Start it locally with \`raindrop replay register${endpoint.cwd ? ` --cwd=${endpoint.cwd}` : ""}\`, then retry replay.`, + command: endpoint.command, + cwd: endpoint.cwd, + attemptedStart: false, + }); + res.end(); + return; + } if (endpoint.registered) { sendSSE(res, "error", { code: "replay_agent_start_failed", diff --git a/src/server.ts b/src/server.ts index a653b87..454966e 100644 --- a/src/server.ts +++ b/src/server.ts @@ -14,7 +14,7 @@ import { sliceSpanPayload } from "./payload-slice"; import { detectSubAgents } from "./agents"; import { applyProviderOptions, detectProvider, getProviderBaseURL, getProviderHeaders } from "./provider-options"; import { runReplay } from "./replay"; -import { discoverReplayAgents, loadAgentsConfig, saveAgentsConfig, extractContextFromTrace, registerReplayProjectIfPresent } from "./agents-config"; +import { discoverReplayAgents, loadAgentsConfig, saveAgentsConfig, extractContextFromTrace } from "./agents-config"; import { resolveBuiltAppDir } from "./ui-assets"; import { setReplayTrace } from "./replay-map"; import { getClaudeSession, getLatestClaudeLoadout, listClaudeSessions, type ClaudeLoadout } from "./claude-sessions"; @@ -1395,9 +1395,12 @@ export async function createServer(port: number) { } try { const workspace = setActiveWorkspace(cwd); - registerReplayProjectIfPresent(workspace.cwd).catch((err) => { - console.warn("[workshop] failed to refresh replay project registration:", err); - }); + // Do NOT register replay projects here. This endpoint accepts a + // caller-supplied path over loopback HTTP, and registration reads an + // attacker-writable `.raindrop/agents.yaml` and persists its `command` + // into the replay registry — a command that a later `/api/replay` would + // spawn. Replay registration must stay an explicit local action + // (`raindrop replay register`), never a side effect of a workspace change. try { latestClaudeLoadout = getLatestClaudeLoadout(workspace.cwd); } catch (err) { diff --git a/tests/replay-command-injection.test.ts b/tests/replay-command-injection.test.ts new file mode 100644 index 0000000..9f4aa70 --- /dev/null +++ b/tests/replay-command-injection.test.ts @@ -0,0 +1,123 @@ +import { afterEach, describe, expect, test } from "bun:test"; +import { spawnSync } from "child_process"; +import fs from "fs"; +import http from "http"; +import os from "os"; +import path from "path"; + +// agents-config resolves ~/.raindrop from os.homedir() at import time, and the +// runtime caches HOME at launch — so each case drives the real module in a +// child process with an isolated HOME. The child writes replay-projects.json +// into that throwaway home, calls the HTTP-reachable replay resolver, and +// prints the result; the parent asserts on it and on side effects. + +const SRC = path.join(import.meta.dir, "..", "src", "agents-config.ts"); + +const DRIVER = ` +import fs from "fs"; +import os from "os"; +import path from "path"; +import { ensureAgentEndpointDetailed } from ${JSON.stringify(SRC)}; + +const dir = path.join(os.homedir(), ".raindrop"); +fs.mkdirSync(dir, { recursive: true }); +fs.writeFileSync(path.join(dir, "replay-projects.json"), process.env.REGISTRY_JSON ?? "{}"); +const result = await ensureAgentEndpointDetailed(process.env.EVENT_NAME ?? ""); +process.stdout.write(JSON.stringify(result)); +`; + +let driverFile: string | null = null; +let isolatedHome: string | null = null; + +function runResolver(registry: unknown, eventName: string): any { + isolatedHome = fs.mkdtempSync(path.join(os.tmpdir(), "raindrop-rce-home-")); + driverFile = path.join(isolatedHome, "driver.ts"); + fs.writeFileSync(driverFile, DRIVER); + const res = spawnSync("bun", [driverFile], { + encoding: "utf8", + env: { + ...process.env, + HOME: isolatedHome, + USERPROFILE: isolatedHome, + REGISTRY_JSON: JSON.stringify(registry), + EVENT_NAME: eventName, + }, + }); + if (res.status !== 0) throw new Error(`driver failed: ${res.stderr}`); + return JSON.parse(res.stdout.trim()); +} + +afterEach(() => { + if (isolatedHome) fs.rmSync(isolatedHome, { recursive: true, force: true }); + isolatedHome = null; + driverFile = null; +}); + +describe("HTTP replay resolution never spawns registry commands", () => { + // Regression test for the workspace->replay HTTP-to-shell bridge: a command + // sitting in replay-projects.json (placed there by the old + // /api/workspace/active side effect, or by writing the file directly) must + // never be spawned by the HTTP-reachable replay path. Spawning is reserved + // for the explicit `raindrop replay register` CLI action. + test("a command-bearing registry entry is not executed", () => { + const projectDir = fs.mkdtempSync(path.join(os.tmpdir(), "raindrop-rce-proj-")); + const marker = path.join(projectDir, "pwned"); + const registry = { + [projectDir]: { + configPath: path.join(projectDir, ".raindrop/agents.yaml"), + agents: { + evil: { cwd: projectDir, command: `touch ${marker}`, input: {}, prefillFromTrace: {} }, + }, + }, + }; + + expect(fs.existsSync(marker)).toBe(false); + const result = runResolver(registry, "evil"); + + expect(result.config).toBeNull(); + expect(result.attemptedStart).toBe(false); + expect(result.reason).toBe("not_running"); + + // The command must not have been spawned at any point. + expect(fs.existsSync(marker)).toBe(false); + fs.rmSync(projectDir, { recursive: true, force: true }); + }); +}); + +describe("HTTP replay resolution still connects to a running agent", () => { + // The fix must not break legitimate replay: when the registered agent is + // already running (started by the CLI), the HTTP path connects to it. + test("a healthy already-running agent is returned without spawning", async () => { + const projectDir = fs.mkdtempSync(path.join(os.tmpdir(), "raindrop-ok-proj-")); + let port = 0; + const server = http.createServer((req, res) => { + if (req.url === "/health") { + res.writeHead(200, { "Content-Type": "application/json" }); + res.end(JSON.stringify({ ok: true, eventName: "good", port, cwd: projectDir, command: "true" })); + return; + } + res.writeHead(404).end(); + }); + await new Promise((resolve) => server.listen(0, "127.0.0.1", () => resolve())); + port = (server.address() as { port: number }).port; + + try { + const registry = { + [projectDir]: { + configPath: path.join(projectDir, ".raindrop/agents.yaml"), + agents: { + good: { cwd: projectDir, command: "true", lastSeenPort: port, input: {}, prefillFromTrace: {} }, + }, + }, + }; + const result = runResolver(registry, "good"); + + expect(result.registered).toBe(true); + expect(result.attemptedStart).toBe(false); + expect(result.config?.url).toBe(`http://127.0.0.1:${port}/replay`); + } finally { + await new Promise((resolve) => server.close(() => resolve())); + fs.rmSync(projectDir, { recursive: true, force: true }); + } + }); +});