diff --git a/AGENTS.md b/AGENTS.md index 75f29e9..72666c7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -49,6 +49,12 @@ Fusion surfaces are agent prompts, not a second runtime command. Claude Code use when the caller explicitly asks for full auto-approval. - **Handoffs require `DEVIN_API_KEY`** in the environment; nothing writes secrets to disk. +- **`DEVIN_API_URL` is validated before any authenticated request**: it must be + unset or exactly `https://api.devin.ai` (optional trailing slash). Userinfo, + paths, queries, fragments, other ports, and other hosts are rejected, and + redirects are never followed. Tests only: with + `DEVIN_COMPANION_ALLOW_LOOPBACK_API=1`, an `http(s)` URL on `127.0.0.1`, + `[::1]`, or `localhost` (any port) is accepted. `setup` reports a rejected URL. ## Per-tool wiring diff --git a/README.md b/README.md index 2a44c1d..f80e2db 100644 --- a/README.md +++ b/README.md @@ -176,6 +176,10 @@ Fusion keeps a frontier model as the lead for intent, planning, ambiguity, archi No. Local review and task commands use an installed and authenticated Devin CLI. Only cloud handoffs require `DEVIN_API_KEY`. +### Can I point the cloud API at another URL? + +No. Cloud calls send your API key as a bearer token, so `DEVIN_API_URL` is validated before any request. Unset it (the default) or set it to exactly `https://api.devin.ai`. Userinfo, paths, query strings, fragments, other ports, other hosts, and plain `http://` are rejected, and authenticated requests never follow redirects. The only exception is for tests: with `DEVIN_COMPANION_ALLOW_LOOPBACK_API=1` set, `DEVIN_API_URL` may be an `http(s)` URL on `127.0.0.1`, `[::1]`, or `localhost` (any port). `/devin:setup` reports a rejected URL. + ## Development ```bash diff --git a/plugins/devin/commands/handoff.md b/plugins/devin/commands/handoff.md index 25b83e8..04bd977 100644 --- a/plugins/devin/commands/handoff.md +++ b/plugins/devin/commands/handoff.md @@ -12,6 +12,7 @@ Raw slash-command arguments: Core constraint: - This command creates a cloud Devin session on Devin's infrastructure, not locally. - It requires `DEVIN_API_KEY` in the environment. If unset, the companion reports that and stops — tell the user to get a key at https://app.devin.ai/settings/api-keys and re-run. +- If `DEVIN_API_URL` is set, it must be exactly `https://api.devin.ai`; any other value is rejected before a request is made, and `/devin:setup` reports it. - The companion automatically attaches the repo slug, current branch, and uncommitted diff as context. Warn the user if the diff may contain secrets they do not want sent to the cloud. - Your only job is to run the handoff and return the companion's output verbatim to the user. diff --git a/plugins/devin/scripts/devin-companion.mjs b/plugins/devin/scripts/devin-companion.mjs index 17a21e6..26683cb 100755 --- a/plugins/devin/scripts/devin-companion.mjs +++ b/plugins/devin/scripts/devin-companion.mjs @@ -160,7 +160,7 @@ async function cmdSetup(cwd, argv) { authenticated: auth.authenticated, authDetail: auth.detail ?? null }, - cloud: { available: cloud.available, keyKind: cloud.keyKind }, + cloud: { available: cloud.available, keyKind: cloud.keyKind, apiUrlError: cloud.apiUrlError }, git: { available: gitProbe.available, detail: gitProbe.detail ?? null }, node: { version: process.versions.node, ok: nodeMajor > 18 || (nodeMajor === 18 && nodeMinor >= 18) }, config @@ -388,6 +388,9 @@ async function cmdHandoff(cwd, argv) { if (!config.available) { throw new Error("DEVIN_API_KEY is not set. Get a key at https://app.devin.ai/settings/api-keys — /devin:handoff needs it."); } + if (config.apiUrlError) { + throw new Error(config.apiUrlError); + } const gitContext = getGitHandoffContext(cwd); const wantsWait = Boolean(options.wait || options.background); diff --git a/plugins/devin/scripts/lib/cloud.mjs b/plugins/devin/scripts/lib/cloud.mjs index 76247d4..bc65353 100644 --- a/plugins/devin/scripts/lib/cloud.mjs +++ b/plugins/devin/scripts/lib/cloud.mjs @@ -1,11 +1,61 @@ const DEFAULT_API_URL = "https://api.devin.ai"; +const LOOPBACK_OPT_IN_ENV = "DEVIN_COMPANION_ALLOW_LOOPBACK_API"; +const LOOPBACK_URL_PATTERN = /^https?:\/\/(?:127\.0\.0\.1|localhost|\[::1\])(?::\d{1,5})?\/?$/i; const MAX_DIFF_BYTES = 100 * 1024; +// Resolve the origin that bearer-authenticated requests are sent to. Throws a +// descriptive Error (never echoing the raw value, which may hold credentials) +// unless DEVIN_API_URL is unset, exactly https://api.devin.ai (optional single +// trailing slash, any host casing), or, only with DEVIN_COMPANION_ALLOW_LOOPBACK_API=1, +// an http(s) loopback URL (127.0.0.1, [::1], localhost) on any port. Everything else +// is rejected: userinfo, paths, queries, fragments, other ports, other hosts. +export function resolveApiOrigin(env = process.env) { + const raw = env.DEVIN_API_URL; + if (raw === undefined || raw === "") { + return DEFAULT_API_URL; + } + if (/[\s\u0000-\u001f\u007f\\]/.test(raw)) { + throw new Error("DEVIN_API_URL must not contain whitespace, control characters, or backslashes."); + } + let parsed; + try { + parsed = new URL(raw); + } catch { + throw new Error(`DEVIN_API_URL is not a valid URL. Leave it unset to use ${DEFAULT_API_URL}.`); + } + if (parsed.username || parsed.password || raw.includes("@")) { + throw new Error("DEVIN_API_URL must not contain credentials (user:pass@)."); + } + if (parsed.search || parsed.hash || raw.includes("?") || raw.includes("#")) { + throw new Error("DEVIN_API_URL must not contain a query string or fragment."); + } + const isLoopback = LOOPBACK_URL_PATTERN.test(raw); + if (isLoopback) { + if (env[LOOPBACK_OPT_IN_ENV] !== "1") { + throw new Error(`DEVIN_API_URL points at a loopback address, which is only allowed for tests when ${LOOPBACK_OPT_IN_ENV}=1 is set.`); + } + return parsed.origin; + } + const normalized = raw.toLowerCase(); + if (normalized !== DEFAULT_API_URL && normalized !== `${DEFAULT_API_URL}/`) { + throw new Error(`DEVIN_API_URL must be exactly ${DEFAULT_API_URL} (no other host, scheme, port, or path). Leave it unset to use the default.`); + } + return DEFAULT_API_URL; +} + export function getCloudConfig(env = process.env) { const apiKey = env.DEVIN_API_KEY ?? null; + let apiUrl = null; + let apiUrlError = null; + try { + apiUrl = resolveApiOrigin(env); + } catch (error) { + apiUrlError = error instanceof Error ? error.message : String(error); + } return { apiKey, - apiUrl: env.DEVIN_API_URL ?? DEFAULT_API_URL, + apiUrl, + apiUrlError, orgId: env.DEVIN_ORG_ID ?? null, userId: env.DEVIN_USER_ID ?? null, available: Boolean(apiKey), @@ -13,7 +63,14 @@ export function getCloudConfig(env = process.env) { }; } +function assertApiUrlValid(config) { + if (config.apiUrlError || !config.apiUrl) { + throw new Error(config.apiUrlError ?? "DEVIN_API_URL failed validation."); + } +} + function resolveApiBase(config) { + assertApiUrlValid(config); if (config.keyKind === "service") { if (!config.orgId) { throw new Error("DEVIN_ORG_ID (or --org-id) is required for service keys (cog_*)."); @@ -24,8 +81,10 @@ function resolveApiBase(config) { } async function apiRequest(config, method, url, body) { + assertApiUrlValid(config); const response = await fetch(url, { method, + redirect: "error", headers: { Authorization: `Bearer ${config.apiKey}`, "Content-Type": "application/json" @@ -33,6 +92,10 @@ async function apiRequest(config, method, url, body) { ...(body !== undefined ? { body: JSON.stringify(body) } : {}) }); + if (response.status >= 300 && response.status < 400) { + throw new Error(`Devin API returned HTTP ${response.status}; redirects are not followed on authenticated requests.`); + } + const text = await response.text(); let parsed = null; try { diff --git a/plugins/devin/scripts/lib/render.mjs b/plugins/devin/scripts/lib/render.mjs index 9163a59..913665c 100644 --- a/plugins/devin/scripts/lib/render.mjs +++ b/plugins/devin/scripts/lib/render.mjs @@ -31,6 +31,9 @@ export function renderSetupReport(report) { lines.push(`- devin CLI: ${report.devin.installed ? `installed (${report.devin.version ?? "unknown version"})` : "NOT FOUND"}`); lines.push(`- auth: ${report.devin.authenticated ? `ok — ${report.devin.authDetail}` : `not authenticated (${report.devin.authDetail ?? "unknown"})`}`); lines.push(`- cloud API (DEVIN_API_KEY): ${report.cloud.available ? `set (${report.cloud.keyKind} key)` : "not set — /devin:handoff unavailable"}`); + if (report.cloud.apiUrlError) { + lines.push(`- cloud API URL (DEVIN_API_URL): REJECTED, cloud calls are disabled (${report.cloud.apiUrlError})`); + } lines.push(`- git: ${report.git.available ? "ok" : `missing (${report.git.detail})`}`); lines.push(`- review gate: ${report.config.stopReviewGate ? "enabled" : "disabled"}`); lines.push(""); @@ -42,6 +45,9 @@ export function renderSetupReport(report) { if (!report.cloud.available) { lines.push("Set DEVIN_API_KEY to enable cloud handoffs (https://app.devin.ai/settings/api-keys). Local review/rescue work without it."); } + if (report.cloud.apiUrlError) { + lines.push("Unset DEVIN_API_URL (or set it to exactly https://api.devin.ai) so the API key is only ever sent to Devin."); + } if (report.ready) { lines.push("Devin is ready."); } diff --git a/tests/cloud-url.test.mjs b/tests/cloud-url.test.mjs new file mode 100644 index 0000000..f85f1dc --- /dev/null +++ b/tests/cloud-url.test.mjs @@ -0,0 +1,271 @@ +import assert from "node:assert/strict"; +import { spawnSync } from "node:child_process"; +import fs from "node:fs"; +import test from "node:test"; +import { fileURLToPath } from "node:url"; + +import { createTempRepo } from "./fake-devin-fixture.mjs"; +import { + createCloudSession, + getCloudConfig, + getCloudSession, + resolveApiOrigin, + sendCloudMessage +} from "../plugins/devin/scripts/lib/cloud.mjs"; + +const COMPANION = fileURLToPath(new URL("../plugins/devin/scripts/devin-companion.mjs", import.meta.url)); +const PRODUCTION = "https://api.devin.ai"; +const OPT_IN = "DEVIN_COMPANION_ALLOW_LOOPBACK_API"; +const API_KEY = "apk_test_secret_key"; + +const MALICIOUS_URLS = [ + "http://api.devin.ai", + "https://api.devin.ai:8443", + "https://api.devin.ai:443", + "https://api.devin.ai:80", + "https://api.devin.ai/v1", + "https://api.devin.ai//", + "https://api.devin.ai/.", + "https://api.devin.ai/?", + "https://api.devin.ai/?x=1", + "https://api.devin.ai?x=1", + "https://api.devin.ai/#frag", + "https://api.devin.ai#", + "https://user@api.devin.ai", + "https://user:pass@api.devin.ai", + "https://:pass@api.devin.ai", + "https://api.devin.ai@evil.com", + "https://api.devin.ai:443@evil.com", + "https://evil.com@api.devin.ai", + "https://api.devin.ai.evil.com", + "https://evil.com/api.devin.ai", + "https://evil.com#api.devin.ai", + "https://evil.com?https://api.devin.ai", + "https://api.devin.ai\\@evil.com", + "https://evilapi.devin.ai", + "https://app.devin.ai", + "https://devin.ai", + "https://api.devin.ai.", + "https://api.devin。ai", + "https://api.devin.ai。", + "https://ａｐｉ.devin.ai", + "https://api.dеvin.ai", + "https://xn--pi-6fa.devin.ai", + " https://api.devin.ai", + "https://api.devin.ai ", + "https://api.devin.ai\n", + "https://api.dev\tin.ai", + "https://api.devin.ai\u0000", + "ftp://api.devin.ai", + "file:///etc/passwd", + "javascript:alert(1)", + "//api.devin.ai", + "api.devin.ai", + "not a url", + "https://", + "https://evil.com", + "http://169.254.169.254", + "http://10.0.0.1", + "https://127.0.0.1.evil.com", + "http://localhost.evil.com:8080", + "http://localhost@evil.com", + "http://127.0.0.1@evil.com", + "http://0.0.0.0:8080", + "http://[::]:8080", + "http://127.1:8080", + "http://2130706433:8080", + "http://[::ffff:127.0.0.1]:8080", + "http://localhost:8080/v1", + "http://localhost:8080/?x=1", + "http://localhost:8080/#x", + "http://user:pass@localhost:8080" +]; + +function withMockedFetch(handler, fn) { + const original = globalThis.fetch; + const calls = []; + globalThis.fetch = async (url, init) => { + calls.push({ url: String(url), init }); + return handler(url, init); + }; + return Promise.resolve(fn(calls)).finally(() => { + globalThis.fetch = original; + }); +} + +function jsonResponse(body, status = 200) { + return new Response(JSON.stringify(body), { status, headers: { "content-type": "application/json" } }); +} + +function envFor(url, extra = {}) { + return { DEVIN_API_KEY: API_KEY, DEVIN_API_URL: url, ...extra }; +} + +test("unset or empty DEVIN_API_URL resolves to the production origin", () => { + assert.equal(resolveApiOrigin({}), PRODUCTION); + assert.equal(resolveApiOrigin({ DEVIN_API_URL: "" }), PRODUCTION); + assert.equal(getCloudConfig({ DEVIN_API_KEY: API_KEY }).apiUrl, PRODUCTION); + assert.equal(getCloudConfig({ DEVIN_API_KEY: API_KEY }).apiUrlError, null); +}); + +test("exact production origin is accepted and normalized", () => { + for (const value of [PRODUCTION, `${PRODUCTION}/`, "https://API.DEVIN.AI", "HTTPS://Api.Devin.Ai/"]) { + assert.equal(resolveApiOrigin({ DEVIN_API_URL: value }), PRODUCTION, value); + } +}); + +test("malicious DEVIN_API_URL variants are rejected, with or without the loopback opt-in", () => { + for (const value of MALICIOUS_URLS) { + assert.throws(() => resolveApiOrigin({ DEVIN_API_URL: value }), Error, JSON.stringify(value)); + assert.throws(() => resolveApiOrigin({ DEVIN_API_URL: value, [OPT_IN]: "1" }), Error, `opt-in: ${JSON.stringify(value)}`); + } +}); + +test("rejection messages never echo the raw URL (it may carry credentials)", () => { + for (const value of ["https://user:hunter2@api.devin.ai", "https://api.devin.ai/?token=hunter2", "https://hunter2.example"]) { + assert.throws(() => resolveApiOrigin({ DEVIN_API_URL: value }), (error) => !error.message.includes("hunter2")); + } +}); + +test("loopback URLs need the explicit test opt-in", () => { + const loopbacks = ["http://127.0.0.1:8080", "http://localhost:3000/", "http://[::1]:9999", "https://localhost:8443", "HTTP://LOCALHOST:8080", "http://127.0.0.1"]; + for (const value of loopbacks) { + assert.throws(() => resolveApiOrigin({ DEVIN_API_URL: value }), new RegExp(OPT_IN), value); + assert.throws(() => resolveApiOrigin({ DEVIN_API_URL: value, [OPT_IN]: "true" }), new RegExp(OPT_IN), `non-1 opt-in: ${value}`); + assert.throws(() => resolveApiOrigin({ DEVIN_API_URL: value, [OPT_IN]: "" }), new RegExp(OPT_IN), `empty opt-in: ${value}`); + } + assert.equal(resolveApiOrigin({ DEVIN_API_URL: "http://127.0.0.1:8080", [OPT_IN]: "1" }), "http://127.0.0.1:8080"); + assert.equal(resolveApiOrigin({ DEVIN_API_URL: "http://localhost:3000/", [OPT_IN]: "1" }), "http://localhost:3000"); + assert.equal(resolveApiOrigin({ DEVIN_API_URL: "http://[::1]:9999", [OPT_IN]: "1" }), "http://[::1]:9999"); + assert.equal(resolveApiOrigin({ DEVIN_API_URL: "https://localhost:8443", [OPT_IN]: "1" }), "https://localhost:8443"); +}); + +test("the opt-in does not unlock non-loopback hosts", () => { + assert.throws(() => resolveApiOrigin({ DEVIN_API_URL: "http://evil.com:8080", [OPT_IN]: "1" })); + assert.throws(() => resolveApiOrigin({ DEVIN_API_URL: "http://api.devin.ai", [OPT_IN]: "1" })); +}); + +test("getCloudConfig reports an invalid URL instead of throwing", () => { + const config = getCloudConfig(envFor("https://evil.com")); + assert.equal(config.apiUrl, null); + assert.match(config.apiUrlError, /DEVIN_API_URL/); + assert.equal(config.available, true); +}); + +test("no credential-bearing request is made for any malicious URL (create, get, message)", async () => { + await withMockedFetch(() => jsonResponse({ session_id: "devin-x", url: "https://app.devin.ai/sessions/x" }), async (calls) => { + for (const value of MALICIOUS_URLS) { + const config = getCloudConfig(envFor(value)); + await assert.rejects(createCloudSession({ task: "t", config }), Error, JSON.stringify(value)); + await assert.rejects(getCloudSession("devin-x", config), Error, JSON.stringify(value)); + await assert.rejects(sendCloudMessage("devin-x", "hi", config), Error, JSON.stringify(value)); + const serviceConfig = getCloudConfig({ ...envFor(value), DEVIN_API_KEY: "cog_service_key", DEVIN_ORG_ID: "org-1" }); + await assert.rejects(createCloudSession({ task: "t", config: serviceConfig }), Error, JSON.stringify(value)); + } + assert.equal(calls.length, 0, "fetch must never be invoked"); + }); +}); + +test("apiRequest refuses a config that carries a validation error even if apiUrl is forged", async () => { + await withMockedFetch(() => jsonResponse({}), async (calls) => { + const config = { ...getCloudConfig(envFor("https://evil.com")), apiUrl: "https://evil.com" }; + await assert.rejects(getCloudSession("devin-x", config), /DEVIN_API_URL/); + assert.equal(calls.length, 0); + }); +}); + +test("valid default config sends the key only to the production origin", async () => { + await withMockedFetch(() => jsonResponse({ session_id: "devin-abc", url: "https://app.devin.ai/sessions/abc" }), async (calls) => { + const config = getCloudConfig({ DEVIN_API_KEY: API_KEY }); + const created = await createCloudSession({ task: "do it", config }); + assert.equal(created.sessionId, "devin-abc"); + await getCloudSession("devin-abc", config); + await sendCloudMessage("devin-abc", "hi", config); + assert.equal(calls.length, 3); + for (const call of calls) { + assert.ok(call.url.startsWith(`${PRODUCTION}/v1/`), call.url); + assert.equal(call.init.headers.Authorization, `Bearer ${API_KEY}`); + } + }); +}); + +test("opted-in loopback config reaches the loopback origin", async () => { + await withMockedFetch(() => jsonResponse({ status: "running" }), async (calls) => { + const config = getCloudConfig(envFor("http://127.0.0.1:4010", { [OPT_IN]: "1" })); + await getCloudSession("devin-abc", config); + assert.equal(calls.length, 1); + assert.equal(calls[0].url, "http://127.0.0.1:4010/v1/sessions/devin-abc"); + }); +}); + +test("authenticated requests never follow redirects", async () => { + await withMockedFetch(() => jsonResponse({ status: "running" }), async (calls) => { + await getCloudSession("devin-abc", getCloudConfig({ DEVIN_API_KEY: API_KEY })); + assert.equal(calls[0].init.redirect, "error"); + }); +}); + +test("a redirect response is an error and triggers no second request", async () => { + const redirect = () => new Response(null, { status: 302, headers: { location: "https://evil.com/steal" } }); + await withMockedFetch(redirect, async (calls) => { + const config = getCloudConfig({ DEVIN_API_KEY: API_KEY }); + await assert.rejects(getCloudSession("devin-abc", config), /redirect/i); + await assert.rejects(sendCloudMessage("devin-abc", "hi", config), /redirect/i); + await assert.rejects(createCloudSession({ task: "t", config }), /redirect/i); + assert.equal(calls.length, 3, "one request per call, none to the redirect target"); + assert.ok(calls.every((call) => call.url.startsWith(PRODUCTION))); + }); +}); + +test("a fetch-level redirect error (redirect: error) propagates", async () => { + const failing = () => { + throw new TypeError("fetch failed"); + }; + await withMockedFetch(failing, async (calls) => { + await assert.rejects(getCloudSession("devin-abc", getCloudConfig({ DEVIN_API_KEY: API_KEY })), /fetch failed/); + assert.equal(calls.length, 1); + }); +}); + +function runCompanion(args, { cwd, env = {} }) { + return spawnSync(process.execPath, [COMPANION, ...args], { + cwd, + encoding: "utf8", + env: { ...process.env, ...env }, + timeout: 30000 + }); +} + +test("setup reports a rejected DEVIN_API_URL without crashing", () => { + const repo = createTempRepo(); + const dataDir = fs.mkdtempSync("/tmp/devin-plugin-data-"); + try { + const env = { CLAUDE_PLUGIN_DATA: dataDir, DEVIN_API_KEY: API_KEY, DEVIN_API_URL: "https://user:hunter2@evil.com" }; + const json = runCompanion(["setup", "--json"], { cwd: repo.dir, env }); + const report = JSON.parse(json.stdout); + assert.match(report.cloud.apiUrlError, /DEVIN_API_URL/); + assert.ok(!json.stdout.includes("hunter2")); + const text = runCompanion(["setup"], { cwd: repo.dir, env }); + assert.match(text.stdout, /DEVIN_API_URL\): REJECTED/); + assert.ok(!text.stdout.includes("hunter2")); + } finally { + repo.cleanup(); + fs.rmSync(dataDir, { recursive: true, force: true }); + } +}); + +test("handoff with a rejected DEVIN_API_URL fails before creating a job or any request", () => { + const repo = createTempRepo(); + const dataDir = fs.mkdtempSync("/tmp/devin-plugin-data-"); + try { + const env = { CLAUDE_PLUGIN_DATA: dataDir, DEVIN_API_KEY: API_KEY, DEVIN_API_URL: "https://api.devin.ai.evil.com" }; + const result = runCompanion(["handoff", "fix", "ci"], { cwd: repo.dir, env }); + assert.notEqual(result.status, 0); + assert.match(result.stderr, /DEVIN_API_URL/); + const status = runCompanion(["status", "--json"], { cwd: repo.dir, env }); + assert.equal(JSON.parse(status.stdout).jobs.length, 0); + } finally { + repo.cleanup(); + fs.rmSync(dataDir, { recursive: true, force: true }); + } +});