diff --git a/node/src/commands/backend/get.ts b/node/src/commands/backend/get.ts index 66f47469..40471d1a 100644 --- a/node/src/commands/backend/get.ts +++ b/node/src/commands/backend/get.ts @@ -1,5 +1,4 @@ import { Command } from "commander"; -import axios from "axios"; import { API, resolveKey, diff --git a/node/src/commands/backend/run.ts b/node/src/commands/backend/run.ts index 6fb73162..db46df9f 100644 --- a/node/src/commands/backend/run.ts +++ b/node/src/commands/backend/run.ts @@ -1,5 +1,4 @@ import { Command } from "commander"; -import axios from "axios"; import ora from "ora"; import { detectRepo } from "../../utils/git.js"; import { @@ -8,8 +7,9 @@ import { EXIT_GENERAL_ERROR, EXIT_QUOTA_EXHAUSTED, EXIT_CONFIRMATION_REQUIRED, - handle403 -} from "../../utils/api.js"; + handle403, + apiClient, + apiUrl} from "../../utils/api.js"; import { ConfigManager } from "../../core/config-manager.js"; import { loadPolicy } from "../../core/policy-loader.js"; import { askYesNo } from "../../utils/prompt.js"; @@ -133,8 +133,8 @@ export async function runRemoteScan(opts: RunOpts): Promise { if (!opts.quiet) { const spinner = ora("Submitting scan").start(); try { - const { data } = await axios.post( - `${API}/static/scan`, + const { data } = await apiClient.post( + apiUrl("static/scan"), body, { headers: { "x-api-key": key } } ); @@ -161,8 +161,8 @@ export async function runRemoteScan(opts: RunOpts): Promise { } } else { try { - const { data } = await axios.post( - `${API}/static/scan`, + const { data } = await apiClient.post( + apiUrl("static/scan"), body, { headers: { "x-api-key": key } } ); diff --git a/node/src/commands/backend/scan-status.ts b/node/src/commands/backend/scan-status.ts index 2aa1bb85..2d7090ac 100644 --- a/node/src/commands/backend/scan-status.ts +++ b/node/src/commands/backend/scan-status.ts @@ -1,12 +1,12 @@ -import axios from "axios"; import ora from "ora"; import { API, API_TIMEOUT_SHORT_MS, writePayload, EXIT_GENERAL_ERROR, - EXIT_SCAN_NOT_FOUND -} from "../../utils/api.js"; + EXIT_SCAN_NOT_FOUND, + apiClient, + apiUrl} from "../../utils/api.js"; import { fmt as output } from "../../utils/formatter.js"; /** @@ -175,7 +175,7 @@ async function pollUntilReadable( ): Promise { for (;;) { try { - const res = await axios.get(`${API}/static/scan`, { + const res = await apiClient.get(apiUrl("static/scan"), { params: { scan_id, format: fmt }, headers, // Without this a hung server stalls inside a single request, and the diff --git a/node/src/commands/backend/usage.ts b/node/src/commands/backend/usage.ts index 5a1c2c0e..3c18412a 100644 --- a/node/src/commands/backend/usage.ts +++ b/node/src/commands/backend/usage.ts @@ -1,6 +1,5 @@ import { Command } from "commander"; -import axios from "axios"; -import { API, resolveKey, EXIT_GENERAL_ERROR } from "../../utils/api.js"; +import { API, resolveKey, EXIT_GENERAL_ERROR, apiClient, apiUrl} from "../../utils/api.js"; export function createUsageCommand(): Command { return new Command("usage") @@ -8,7 +7,7 @@ export function createUsageCommand(): Command { .action(async (opts) => { const key = resolveKey(opts.apiKey); try { - const { data } = await axios.get(`${API}/static/usage`, { headers: { "x-api-key": key } }); + const { data } = await apiClient.get(apiUrl("static/usage"), { headers: { "x-api-key": key } }); console.log(JSON.stringify(data, null, 2)); } catch (e: any) { if (e.response?.data) { diff --git a/node/src/commands/issues/from-scan.ts b/node/src/commands/issues/from-scan.ts index 88d01575..38852bcb 100644 --- a/node/src/commands/issues/from-scan.ts +++ b/node/src/commands/issues/from-scan.ts @@ -7,8 +7,7 @@ */ import { Command } from "commander"; import fs from "fs"; -import axios from "axios"; -import { API, resolveKey, EXIT_GENERAL_ERROR } from "../../utils/api.js"; +import { API, resolveKey, EXIT_GENERAL_ERROR, apiClient, apiUrl} from "../../utils/api.js"; import { detectRepo } from "../../utils/git.js"; import { fmt } from "../../utils/formatter.js"; import { createIssue, listOpenIssues } from "./github-client.js"; @@ -170,7 +169,7 @@ async function draftsFromBackendScan( apiKey?: string ): Promise { const key = resolveKey(apiKey); - const { data } = await axios.get(`${API}/static/scan`, { + const { data } = await apiClient.get(apiUrl("static/scan"), { params: { scan_id: scanId, format: "json" }, headers: { "x-api-key": key }, }); diff --git a/node/src/commands/mcp/server.ts b/node/src/commands/mcp/server.ts index 821f0534..48ffd243 100644 --- a/node/src/commands/mcp/server.ts +++ b/node/src/commands/mcp/server.ts @@ -15,9 +15,8 @@ import { AuditLogger } from "../../core/audit-logger.js"; import { ConfigManager, redactConfigSecrets, isSecretConfigKey, maskSecretValue } from "../../core/config-manager.js"; import { listDocs, resolveDocSelector, fetchDoc } from "../../core/docs-loader.js"; import { writeSuppression } from "../../core/suppression-writer.js"; -import { apiUrl } from "../../utils/api.js"; +import { apiUrl, apiClient} from "../../utils/api.js"; import { describeSitesError, resolveMcpApiKey } from "../sites/errors.js"; -import axios from "axios"; import { createRequire } from "module"; const _require = createRequire(import.meta.url); @@ -361,7 +360,7 @@ export function createServer(): Server { const key = resolveMcpApiKey(); if (!key) return errorResult("No API key configured. Set RAFTER_API_KEY or run 'rafter agent config set backend.apiKey '."); try { - const { data } = await axios.post(apiUrl("static/sites"), { url }, { headers: { "x-api-key": key } }); + const { data } = await apiClient.post(apiUrl("static/sites"), { url }, { headers: { "x-api-key": key } }); return textResult(data); } catch (e: any) { return errorResult(describeSitesError(e).message); @@ -378,7 +377,7 @@ export function createServer(): Server { const body: Record = projectId ? { projectId } : { url }; if (Array.isArray(args?.sections)) body.sections = (args!.sections as unknown[]).map((s) => String(s)); try { - const { data } = await axios.post(apiUrl("static/sites/scan"), body, { headers: { "x-api-key": key } }); + const { data } = await apiClient.post(apiUrl("static/sites/scan"), body, { headers: { "x-api-key": key } }); return textResult(data); } catch (e: any) { return errorResult(describeSitesError(e).message); @@ -393,7 +392,7 @@ export function createServer(): Server { if (args?.offset !== undefined) params.offset = String(args.offset); if (args?.include_archived) params.include_archived = "true"; try { - const { data } = await axios.get(apiUrl("static/sites"), { params, headers: { "x-api-key": key } }); + const { data } = await apiClient.get(apiUrl("static/sites"), { params, headers: { "x-api-key": key } }); return textResult(data); } catch (e: any) { return errorResult(describeSitesError(e).message); @@ -406,7 +405,7 @@ export function createServer(): Server { const key = resolveMcpApiKey(); if (!key) return errorResult("No API key configured. Set RAFTER_API_KEY or run 'rafter agent config set backend.apiKey '."); try { - const { data } = await axios.get(apiUrl(`static/sites/${encodeURIComponent(id)}`), { headers: { "x-api-key": key } }); + const { data } = await apiClient.get(apiUrl(`static/sites/${encodeURIComponent(id)}`), { headers: { "x-api-key": key } }); return textResult(data); } catch (e: any) { return errorResult(describeSitesError(e).message); diff --git a/node/src/commands/notify.ts b/node/src/commands/notify.ts index 459f6597..782632eb 100644 --- a/node/src/commands/notify.ts +++ b/node/src/commands/notify.ts @@ -1,6 +1,5 @@ import { Command } from "commander"; -import axios from "axios"; -import { API, resolveKey, EXIT_GENERAL_ERROR, EXIT_SCAN_NOT_FOUND } from "../utils/api.js"; +import { API, resolveKey, EXIT_GENERAL_ERROR, EXIT_SCAN_NOT_FOUND, apiClient, apiUrl} from "../utils/api.js"; import { validateWebhookUrl } from "../core/audit-logger.js"; import { ConfigManager } from "../core/config-manager.js"; import { fmt, isAgentMode } from "../utils/formatter.js"; @@ -221,7 +220,7 @@ export function createNotifyCommand(): Command { if (scanId) { const key = resolveKey(opts?.apiKey as string | undefined); try { - const { data } = await axios.get(`${API}/static/scan`, { + const { data } = await apiClient.get(apiUrl("static/scan"), { params: { scan_id: scanId, format: "json" }, headers: { "x-api-key": key }, }); diff --git a/node/src/commands/sites/create.ts b/node/src/commands/sites/create.ts index dc35d4c8..d8b36f38 100644 --- a/node/src/commands/sites/create.ts +++ b/node/src/commands/sites/create.ts @@ -1,6 +1,5 @@ import { Command } from "commander"; -import axios from "axios"; -import { apiUrl, resolveKey, writePayload, EXIT_GENERAL_ERROR } from "../../utils/api.js"; +import { apiUrl, resolveKey, writePayload, EXIT_GENERAL_ERROR, apiClient} from "../../utils/api.js"; import { describeSitesError, rejectUnsupportedFormat } from "./errors.js"; export interface SitesCreateOpts { @@ -14,7 +13,7 @@ export async function runSitesCreate(url: string, opts: SitesCreateOpts): Promis if (rejectUnsupportedFormat(opts.format)) return EXIT_GENERAL_ERROR; const key = resolveKey(opts.apiKey); try { - const { data } = await axios.post( + const { data } = await apiClient.post( apiUrl("static/sites"), { url }, { headers: { "x-api-key": key } } diff --git a/node/src/commands/sites/get.ts b/node/src/commands/sites/get.ts index 1a163417..e002f209 100644 --- a/node/src/commands/sites/get.ts +++ b/node/src/commands/sites/get.ts @@ -1,6 +1,5 @@ import { Command } from "commander"; -import axios from "axios"; -import { apiUrl, resolveKey, writePayload, EXIT_GENERAL_ERROR } from "../../utils/api.js"; +import { apiUrl, resolveKey, writePayload, EXIT_GENERAL_ERROR, apiClient} from "../../utils/api.js"; import { describeSitesError, rejectUnsupportedFormat } from "./errors.js"; export interface SitesGetOpts { @@ -14,7 +13,7 @@ export async function runSitesGet(id: string, opts: SitesGetOpts): Promise { if (opts.includeArchived) params.include_archived = "true"; try { - const { data } = await axios.get( + const { data } = await apiClient.get( apiUrl("static/sites"), { params, headers: { "x-api-key": key } } ); diff --git a/node/src/commands/sites/scan.ts b/node/src/commands/sites/scan.ts index b584585b..b57e8c55 100644 --- a/node/src/commands/sites/scan.ts +++ b/node/src/commands/sites/scan.ts @@ -1,6 +1,5 @@ import { Command } from "commander"; -import axios from "axios"; -import { apiUrl, resolveKey, writePayload, EXIT_GENERAL_ERROR } from "../../utils/api.js"; +import { apiUrl, resolveKey, writePayload, EXIT_GENERAL_ERROR, apiClient} from "../../utils/api.js"; import { describeSitesError, rejectUnsupportedFormat } from "./errors.js"; const VALID_SECTIONS = new Set(["flight", "security", "dns"]); @@ -42,7 +41,7 @@ export async function runSitesScan(projectIdOrUrl: string, opts: SitesScanOpts): } try { - const { data } = await axios.post( + const { data } = await apiClient.post( apiUrl("static/sites/scan"), body, { headers: { "x-api-key": key } } diff --git a/node/src/utils/api.ts b/node/src/utils/api.ts index 740431bd..a95a6b48 100644 --- a/node/src/utils/api.ts +++ b/node/src/utils/api.ts @@ -1,7 +1,52 @@ +import axios from "axios"; import { ConfigManager } from "../core/config-manager.js"; export const API = "https://rafter.so/api/"; +/** + * sable-2s6p — the HTTP client for every authenticated Rafter API call. + * + * `maxRedirects: 0` is the point of it. axios (via follow-redirects) replays + * request headers on a redirect, and unlike `Authorization` the custom + * `x-api-key` header is not stripped when the host changes. Since the API base + * is user-settable (`--rafter-url`, self-hosted installs), a 302 from a + * misconfigured or hostile endpoint would walk the caller's API key to another + * host. Nothing in this CLI needs to follow a redirect, so none of them do. + * + * Use this for anything that sends `x-api-key`. Plain `axios` is fine for + * user-supplied webhooks and other unauthenticated calls. + */ +export const apiClient = axios.create({ + maxRedirects: 0, +}); + +/** + * A redirect target is attacker-controlled if the endpoint is. Header values + * cannot contain CR/LF, but ESC is a legal byte, so an unsanitized Location can + * emit ANSI sequences that rewrite the user's terminal. Strip anything + * non-printable and cap the length. + */ +function safeForTerminal(value: unknown): string { + if (typeof value !== "string") return ""; + // eslint-disable-next-line no-control-regex + const printable = value.replace(/[\u0000-\u001f\u007f-\u009f]/g, ""); + return printable.length > 200 ? `${printable.slice(0, 200)}…` : printable; +} + +// A refused redirect otherwise surfaces as a bare "Request failed with status +// code 302", which tells the user nothing about why. Name the cause. +apiClient.interceptors.response.use(undefined, (error: any) => { + const status = error?.response?.status; + if (status >= 300 && status < 400) { + const target = safeForTerminal(error?.response?.headers?.location) || "another host"; + error.message = + `The Rafter API redirected to ${target}, and Rafter does not follow redirects ` + + `on authenticated requests — your API key would be sent to the redirect target. ` + + `If you are pointing Rafter at a self-hosted instance, use its final URL.`; + } + return Promise.reject(error); +}); + /** Join API with a path segment without producing a double slash, regardless of leading/trailing slashes on either side. */ export function apiUrl(path: string): string { return `${API.replace(/\/+$/, "")}/${path.replace(/^\/+/, "")}`; diff --git a/node/tests/api-no-redirect.test.ts b/node/tests/api-no-redirect.test.ts new file mode 100644 index 00000000..f26d2ccd --- /dev/null +++ b/node/tests/api-no-redirect.test.ts @@ -0,0 +1,125 @@ +import { describe, it, expect } from "vitest"; +import { readFileSync, readdirSync, statSync } from "node:fs"; +import { join } from "node:path"; + +import { apiClient } from "../src/utils/api.js"; + +/** + * sable-2s6p — a custom `x-api-key` header is NOT stripped across a cross-host + * redirect the way `Authorization` is. axios replays it verbatim, and the API + * base is user-settable (`--rafter-url`, self-hosted installs), so a 302 from a + * misconfigured or hostile endpoint walks the caller's API key to another host. + * + * Nothing in this CLI needs to follow a redirect. These tests pin that, and — + * more importantly — pin that no NEW authenticated call site can reintroduce + * the hole by reaching for bare `axios`. + */ + +describe("apiClient (sable-2s6p)", () => { + it("refuses to follow redirects", () => { + expect(apiClient.defaults.maxRedirects).toBe(0); + }); + + it("explains why, instead of surfacing a bare 302", async () => { + // Drive the interceptor directly: it is the thing that turns an opaque + // status code into something a customer can act on. + const handlers = (apiClient.interceptors.response as any).handlers.filter(Boolean); + expect(handlers.length).toBeGreaterThan(0); + const onRejected = handlers[handlers.length - 1].rejected; + + const err: any = { + response: { + status: 302, + // ANSI escape included on purpose: an attacker-controlled Location must + // not be able to rewrite the user's terminal. + headers: { location: "https://evil.example/collect\u001b[31m" }, + }, + message: "Request failed with status code 302", + }; + + await expect(onRejected(err)).rejects.toBeDefined(); + expect(err.message).toContain("evil.example"); + expect(err.message).toContain("does not follow redirects"); + expect(err.message).not.toContain("\u001b"); + expect(err.message).toContain("self-hosted"); + }); +}); + +/** Every .ts file under a directory, recursively. */ +function sourceFiles(dir: string): string[] { + const out: string[] = []; + for (const entry of readdirSync(dir)) { + const full = join(dir, entry); + if (statSync(full).isDirectory()) out.push(...sourceFiles(full)); + else if (entry.endsWith(".ts")) out.push(full); + } + return out; +} + +describe("no authenticated call bypasses apiClient (sable-2s6p)", () => { + it("creates no second axios instance outside the api utils", () => { + const offenders: string[] = []; + for (const file of sourceFiles("src")) { + if (file.endsWith("utils/api.ts")) continue; + const text = readFileSync(file, "utf8"); + if (/\baxios\.create\(/.test(text)) offenders.push(file); + } + expect( + offenders, + `A second axios instance can be created without maxRedirects: 0. Use ` + + `apiClient from src/utils/api.ts:\n${offenders.join("\n")}` + ).toEqual([]); + }); + + it("has no bare axios verb call that sends x-api-key", () => { + const offenders: string[] = []; + + for (const file of sourceFiles("src")) { + const text = readFileSync(file, "utf8"); + const lines = text.split("\n"); + lines.forEach((line, i) => { + if (!/\baxios(\.(get|post|put|delete|patch|request))?\(/.test(line)) return; + // Look at the call and the few lines after it — the header object is + // usually on a following line. + const window = lines.slice(i, i + 6).join("\n"); + if (window.includes("x-api-key")) { + offenders.push(`${file}:${i + 1}`); + } + }); + } + + expect( + offenders, + `These calls send the API key through bare axios, which follows redirects ` + + `across hosts. Use apiClient from src/utils/api.ts instead:\n${offenders.join("\n")}` + ).toEqual([]); + }); +}); + +describe("API URL construction (sable-2s6p)", () => { + it("builds no double slash after the scheme", () => { + // Not cosmetic. `API` ends in "/", and concatenating "/static/..." produced + // https://rafter.so/api//static/scan, which production answers with a 308. + // That worked only because the client followed redirects — so refusing + // them would have broken every core command. Caught by security review, + // not by any test, because every other test mocks the transport. + const offenders: string[] = []; + for (const file of sourceFiles("src")) { + const text = readFileSync(file, "utf8"); + text.split("\n").forEach((line, i) => { + if (/\$\{API\}\//.test(line)) offenders.push(`${file}:${i + 1}`); + }); + } + expect( + offenders, + `These build a double-slash URL. Use apiUrl() instead:\n${offenders.join("\n")}` + ).toEqual([]); + }); + + it("apiUrl joins cleanly regardless of slashes", async () => { + const { apiUrl, API } = await import("../src/utils/api.js"); + expect(apiUrl("static/scan")).toBe("https://rafter.so/api/static/scan"); + expect(apiUrl("/static/scan")).toBe("https://rafter.so/api/static/scan"); + expect(API.endsWith("/")).toBe(true); // the trap this guards against + }); +}); diff --git a/node/tests/mcp-sites.test.ts b/node/tests/mcp-sites.test.ts index 412359ac..5228d59d 100644 --- a/node/tests/mcp-sites.test.ts +++ b/node/tests/mcp-sites.test.ts @@ -9,7 +9,35 @@ import type { Server } from "@modelcontextprotocol/sdk/server/index.js"; * through an in-memory MCP client/server pair, with axios mocked. */ -vi.mock("axios"); +vi.mock("axios", () => { + // sable-2s6p — the code calls `apiClient`, an axios instance created with + // maxRedirects: 0. `create` must return something, and it returns the same + // object as the default export so `mockedAxios.get` still refers to the + // function under test. + const instance: any = { + get: vi.fn(), + post: vi.fn(), + put: vi.fn(), + delete: vi.fn(), + patch: vi.fn(), + defaults: { maxRedirects: 0 }, + interceptors: { response: { use: vi.fn() }, request: { use: vi.fn() } }, + }; + // The default export gets its OWN mocks, distinct from the instance's. If + // production code regresses to bare `axios.get`, the assertions below — which + // watch the instance — stop seeing calls, and the test fails. A shim where + // both are the same object would silently accept that regression. + const bare: any = { + get: vi.fn(), + post: vi.fn(), + put: vi.fn(), + delete: vi.fn(), + patch: vi.fn(), + interceptors: { response: { use: vi.fn() }, request: { use: vi.fn() } }, + create: () => instance, + }; + return { default: bare }; +}); vi.mock("../src/core/config-manager.js", async (importOriginal) => ({ ...(await importOriginal()), @@ -29,7 +57,8 @@ vi.mock("@modelcontextprotocol/sdk/server/stdio.js", () => ({ import axios from "axios"; import { createServer } from "../src/commands/mcp/server.js"; -const mockedAxios = vi.mocked(axios, true); +// The code calls `apiClient` — the instance `create()` returns. +const mockedAxios = vi.mocked((axios as any).create(), true); let client: Client; let server: Server; diff --git a/node/tests/plus-scan-approval.test.ts b/node/tests/plus-scan-approval.test.ts index ccb4721c..cff2d34f 100644 --- a/node/tests/plus-scan-approval.test.ts +++ b/node/tests/plus-scan-approval.test.ts @@ -40,7 +40,35 @@ vi.mock("../src/utils/prompt.js", () => ({ askYesNo: vi.fn(async () => state.promptAnswer), })); -vi.mock("axios"); +vi.mock("axios", () => { + // sable-2s6p — the code calls `apiClient`, an axios instance created with + // maxRedirects: 0. `create` must return something, and it returns the same + // object as the default export so `mockedAxios.get` still refers to the + // function under test. + const instance: any = { + get: vi.fn(), + post: vi.fn(), + put: vi.fn(), + delete: vi.fn(), + patch: vi.fn(), + defaults: { maxRedirects: 0 }, + interceptors: { response: { use: vi.fn() }, request: { use: vi.fn() } }, + }; + // The default export gets its OWN mocks, distinct from the instance's. If + // production code regresses to bare `axios.get`, the assertions below — which + // watch the instance — stop seeing calls, and the test fails. A shim where + // both are the same object would silently accept that regression. + const bare: any = { + get: vi.fn(), + post: vi.fn(), + put: vi.fn(), + delete: vi.fn(), + patch: vi.fn(), + interceptors: { response: { use: vi.fn() }, request: { use: vi.fn() } }, + create: () => instance, + }; + return { default: bare }; +}); vi.mock("ora", () => ({ default: () => ({ start: vi.fn().mockReturnThis(), @@ -58,7 +86,8 @@ import { } from "../src/commands/backend/run.js"; import { EXIT_CONFIRMATION_REQUIRED } from "../src/utils/api.js"; -const mockedAxios = vi.mocked(axios, true); +// The code calls `apiClient` — the instance `create()` returns. +const mockedAxios = vi.mocked((axios as any).create(), true); function resetState() { state.globalFlag = undefined; diff --git a/node/tests/scan-poll-transient-500.test.ts b/node/tests/scan-poll-transient-500.test.ts index b4bf1ba3..aade35f3 100644 --- a/node/tests/scan-poll-transient-500.test.ts +++ b/node/tests/scan-poll-transient-500.test.ts @@ -10,7 +10,35 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; * still fail, and the failure message is one a customer can act on. */ -vi.mock("axios"); +vi.mock("axios", () => { + // sable-2s6p — the code calls `apiClient`, an axios instance created with + // maxRedirects: 0. `create` must return something, and it returns the same + // object as the default export so `mockedAxios.get` still refers to the + // function under test. + const instance: any = { + get: vi.fn(), + post: vi.fn(), + put: vi.fn(), + delete: vi.fn(), + patch: vi.fn(), + defaults: { maxRedirects: 0 }, + interceptors: { response: { use: vi.fn() }, request: { use: vi.fn() } }, + }; + // The default export gets its OWN mocks, distinct from the instance's. If + // production code regresses to bare `axios.get`, the assertions below — which + // watch the instance — stop seeing calls, and the test fails. A shim where + // both are the same object would silently accept that regression. + const bare: any = { + get: vi.fn(), + post: vi.fn(), + put: vi.fn(), + delete: vi.fn(), + patch: vi.fn(), + interceptors: { response: { use: vi.fn() }, request: { use: vi.fn() } }, + create: () => instance, + }; + return { default: bare }; +}); vi.mock("ora", () => ({ default: () => ({ start: vi.fn().mockReturnThis(), @@ -32,7 +60,8 @@ import { } from "../src/commands/backend/scan-status.js"; import { EXIT_SUCCESS, EXIT_GENERAL_ERROR, EXIT_SCAN_NOT_FOUND } from "../src/utils/api.js"; -const mockedAxios = vi.mocked(axios, true); +// The code calls `apiClient` — the instance `create()` returns. +const mockedAxios = vi.mocked((axios as any).create(), true); /** The verbatim body the customer saw. */ const OBJECT_NOT_FOUND = { diff --git a/node/tests/scan-remote.test.ts b/node/tests/scan-remote.test.ts index c2eefbac..7d620a1c 100644 --- a/node/tests/scan-remote.test.ts +++ b/node/tests/scan-remote.test.ts @@ -12,7 +12,35 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; // ── Mocks ────────────────────────────────────────────────────────────── -vi.mock("axios"); +vi.mock("axios", () => { + // sable-2s6p — the code calls `apiClient`, an axios instance created with + // maxRedirects: 0. `create` must return something, and it returns the same + // object as the default export so `mockedAxios.get` still refers to the + // function under test. + const instance: any = { + get: vi.fn(), + post: vi.fn(), + put: vi.fn(), + delete: vi.fn(), + patch: vi.fn(), + defaults: { maxRedirects: 0 }, + interceptors: { response: { use: vi.fn() }, request: { use: vi.fn() } }, + }; + // The default export gets its OWN mocks, distinct from the instance's. If + // production code regresses to bare `axios.get`, the assertions below — which + // watch the instance — stop seeing calls, and the test fails. A shim where + // both are the same object would silently accept that regression. + const bare: any = { + get: vi.fn(), + post: vi.fn(), + put: vi.fn(), + delete: vi.fn(), + patch: vi.fn(), + interceptors: { response: { use: vi.fn() }, request: { use: vi.fn() } }, + create: () => instance, + }; + return { default: bare }; +}); vi.mock("ora", () => ({ default: () => ({ start: vi.fn().mockReturnThis(), @@ -26,7 +54,8 @@ import axios from "axios"; import { handleScanStatus } from "../src/commands/backend/scan-status.js"; import { EXIT_SUCCESS, EXIT_GENERAL_ERROR, EXIT_SCAN_NOT_FOUND } from "../src/utils/api.js"; -const mockedAxios = vi.mocked(axios, true); +// The code calls `apiClient` — the instance `create()` returns. +const mockedAxios = vi.mocked((axios as any).create(), true); // ── handleScanStatus ─────────────────────────────────────────────────── diff --git a/node/tests/sites-cli.test.ts b/node/tests/sites-cli.test.ts index 398bdece..05c26406 100644 --- a/node/tests/sites-cli.test.ts +++ b/node/tests/sites-cli.test.ts @@ -5,7 +5,35 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; * security monitoring). All tests mock axios so no network calls are made. */ -vi.mock("axios"); +vi.mock("axios", () => { + // sable-2s6p — the code calls `apiClient`, an axios instance created with + // maxRedirects: 0. `create` must return something, and it returns the same + // object as the default export so `mockedAxios.get` still refers to the + // function under test. + const instance: any = { + get: vi.fn(), + post: vi.fn(), + put: vi.fn(), + delete: vi.fn(), + patch: vi.fn(), + defaults: { maxRedirects: 0 }, + interceptors: { response: { use: vi.fn() }, request: { use: vi.fn() } }, + }; + // The default export gets its OWN mocks, distinct from the instance's. If + // production code regresses to bare `axios.get`, the assertions below — which + // watch the instance — stop seeing calls, and the test fails. A shim where + // both are the same object would silently accept that regression. + const bare: any = { + get: vi.fn(), + post: vi.fn(), + put: vi.fn(), + delete: vi.fn(), + patch: vi.fn(), + interceptors: { response: { use: vi.fn() }, request: { use: vi.fn() } }, + create: () => instance, + }; + return { default: bare }; +}); import axios from "axios"; import { runSitesCreate } from "../src/commands/sites/create.js"; @@ -14,7 +42,8 @@ import { runSitesList } from "../src/commands/sites/list.js"; import { runSitesGet } from "../src/commands/sites/get.js"; import { EXIT_SUCCESS, EXIT_GENERAL_ERROR, EXIT_INSUFFICIENT_SCOPE, EXIT_SCAN_NOT_FOUND, EXIT_QUOTA_EXHAUSTED } from "../src/utils/api.js"; -const mockedAxios = vi.mocked(axios, true); +// The code calls `apiClient` — the instance `create()` returns. +const mockedAxios = vi.mocked((axios as any).create(), true); const opts = { apiKey: "test-key", format: "json", quiet: true }; beforeEach(() => { diff --git a/python/rafter_cli/commands/backend.py b/python/rafter_cli/commands/backend.py index 5e7314b1..90ead2f4 100644 --- a/python/rafter_cli/commands/backend.py +++ b/python/rafter_cli/commands/backend.py @@ -9,6 +9,9 @@ import typer from ..utils.api import ( + api_url, + api_get, + api_post, API_BASE, API_TIMEOUT, API_TIMEOUT_SHORT, @@ -254,8 +257,8 @@ def _poll_until_readable( """ while True: try: - resp = requests.get( - f"{API_BASE}/static/scan", + resp = api_get( + api_url("static/scan"), headers=headers, params={"scan_id": scan_id, "format": fmt}, timeout=API_TIMEOUT_SHORT, @@ -422,8 +425,8 @@ def _do_remote_scan( body["provider"] = resolved_provider body["repo_url"] = resolved_repo_url - resp = requests.post( - f"{API_BASE}/static/scan", + resp = api_post( + api_url("static/scan"), headers=headers, json=body, timeout=API_TIMEOUT, @@ -508,8 +511,8 @@ def usage( """Check quota and usage.""" key = resolve_key(api_key) headers = {"x-api-key": key} - resp = requests.get( - f"{API_BASE}/static/usage", headers=headers, timeout=API_TIMEOUT_SHORT + resp = api_get( + api_url("static/usage"), headers=headers, timeout=API_TIMEOUT_SHORT ) if resp.status_code != 200: print(f"Error: {resp.text}", file=sys.stderr) diff --git a/python/rafter_cli/commands/issues/issues_app.py b/python/rafter_cli/commands/issues/issues_app.py index 638a9177..ec198c47 100644 --- a/python/rafter_cli/commands/issues/issues_app.py +++ b/python/rafter_cli/commands/issues/issues_app.py @@ -14,7 +14,7 @@ import requests import typer -from ...utils.api import API_BASE, EXIT_GENERAL_ERROR, EXIT_SUCCESS, resolve_key +from ...utils.api import api_url, API_BASE, api_get, EXIT_GENERAL_ERROR, EXIT_SUCCESS, resolve_key from ...utils.formatter import fmt, print_stderr from ...utils.git import detect_repo from .dedup import find_duplicates @@ -213,8 +213,8 @@ def from_text( def _drafts_from_backend(scan_id: str, api_key: str | None) -> list[IssueDraft]: key = resolve_key(api_key) - resp = requests.get( - f"{API_BASE}/static/scan", + resp = api_get( + api_url("static/scan"), headers={"x-api-key": key}, params={"scan_id": scan_id, "format": "json"}, timeout=(10, 60), diff --git a/python/rafter_cli/commands/mcp_server.py b/python/rafter_cli/commands/mcp_server.py index 0982f4c1..9ba7fb35 100644 --- a/python/rafter_cli/commands/mcp_server.py +++ b/python/rafter_cli/commands/mcp_server.py @@ -22,7 +22,7 @@ from ..scanners.betterleaks import BetterleaksScanner from ..scanners.regex_scanner import RegexScanner, ScanResult from ..scanners.union import union_scan_results -from ..utils.api import API_TIMEOUT +from ..utils.api import API_TIMEOUT, api_get, api_post from .sites import SITES_API_BASE, describe_sites_error, resolve_mcp_api_key mcp_app = typer.Typer( @@ -218,7 +218,7 @@ def _require_mcp_api_key() -> str: def handle_sites_create(url: str) -> dict: """Register a URL as a Rafter Site and kick off its first scan.""" key = _require_mcp_api_key() - resp = requests.post(SITES_API_BASE, headers={"x-api-key": key}, json={"url": url}, timeout=API_TIMEOUT) + resp = api_post(SITES_API_BASE, headers={"x-api-key": key}, json={"url": url}, timeout=API_TIMEOUT) if resp.status_code != 200: message, _ = describe_sites_error(resp) raise RuntimeError(message) @@ -241,7 +241,7 @@ def handle_sites_scan( if sections: body["sections"] = list(sections) - resp = requests.post(f"{SITES_API_BASE}/scan", headers={"x-api-key": key}, json=body, timeout=API_TIMEOUT) + resp = api_post(f"{SITES_API_BASE}/scan", headers={"x-api-key": key}, json=body, timeout=API_TIMEOUT) if resp.status_code != 200: message, _ = describe_sites_error(resp) raise RuntimeError(message) @@ -263,7 +263,7 @@ def handle_sites_list( if include_archived: params["include_archived"] = "true" - resp = requests.get(SITES_API_BASE, headers={"x-api-key": key}, params=params, timeout=API_TIMEOUT) + resp = api_get(SITES_API_BASE, headers={"x-api-key": key}, params=params, timeout=API_TIMEOUT) if resp.status_code != 200: message, _ = describe_sites_error(resp) raise RuntimeError(message) @@ -275,7 +275,7 @@ def handle_sites_get(id: str) -> dict: from urllib.parse import quote key = _require_mcp_api_key() - resp = requests.get(f"{SITES_API_BASE}/{quote(id, safe='')}", headers={"x-api-key": key}, timeout=API_TIMEOUT) + resp = api_get(f"{SITES_API_BASE}/{quote(id, safe='')}", headers={"x-api-key": key}, timeout=API_TIMEOUT) if resp.status_code != 200: message, _ = describe_sites_error(resp) raise RuntimeError(message) diff --git a/python/rafter_cli/commands/notify.py b/python/rafter_cli/commands/notify.py index 6e7e684e..e760279b 100644 --- a/python/rafter_cli/commands/notify.py +++ b/python/rafter_cli/commands/notify.py @@ -10,6 +10,8 @@ import typer from ..utils.api import ( + api_url, + api_get, API_BASE, API_TIMEOUT_SHORT, EXIT_GENERAL_ERROR, @@ -309,8 +311,8 @@ def _fetch_scan(scan_id: str, api_key: str) -> dict: import requests headers = {"x-api-key": api_key} - resp = requests.get( - f"{API_BASE}/static/scan", + resp = api_get( + api_url("static/scan"), headers=headers, params={"scan_id": scan_id, "format": "json"}, timeout=API_TIMEOUT_SHORT, diff --git a/python/rafter_cli/commands/sites.py b/python/rafter_cli/commands/sites.py index 5ecef8d4..f36e90b3 100644 --- a/python/rafter_cli/commands/sites.py +++ b/python/rafter_cli/commands/sites.py @@ -18,6 +18,8 @@ import typer from ..utils.api import ( + api_get, + api_post, API_BASE, API_TIMEOUT, EXIT_GENERAL_ERROR, @@ -135,7 +137,7 @@ def sites_create( if reject_unsupported_format(fmt): raise typer.Exit(code=EXIT_GENERAL_ERROR) key = resolve_key(api_key) - resp = requests.post( + resp = api_post( SITES_API_BASE, headers={"x-api-key": key}, json={"url": url}, @@ -174,7 +176,7 @@ def sites_scan( body["sections"] = section_list key = resolve_key(api_key) - resp = requests.post( + resp = api_post( f"{SITES_API_BASE}/scan", headers={"x-api-key": key}, json=body, @@ -209,7 +211,7 @@ def sites_list( params["include_archived"] = "true" key = resolve_key(api_key) - resp = requests.get( + resp = api_get( SITES_API_BASE, headers={"x-api-key": key}, params=params, @@ -234,7 +236,7 @@ def sites_get( raise typer.Exit(code=EXIT_GENERAL_ERROR) key = resolve_key(api_key) site_id = quote(id, safe="") - resp = requests.get( + resp = api_get( f"{SITES_API_BASE}/{site_id}", headers={"x-api-key": key}, timeout=API_TIMEOUT, diff --git a/python/rafter_cli/utils/api.py b/python/rafter_cli/utils/api.py index 10d7ce7d..ee4ccb36 100644 --- a/python/rafter_cli/utils/api.py +++ b/python/rafter_cli/utils/api.py @@ -5,6 +5,7 @@ import os import sys +import requests import typer from dotenv import load_dotenv @@ -62,6 +63,72 @@ def handle_scope_error(resp: "requests.Response") -> bool: API_TIMEOUT_SHORT = (10, 30) +def _safe_for_terminal(value: "str | None") -> str: + """Strip non-printable bytes and cap length before echoing untrusted text. + + A redirect target is attacker-controlled if the endpoint is. Header values + cannot contain CR/LF, but ESC is a legal byte, so an unsanitized Location + can emit ANSI sequences that rewrite the user's terminal. + """ + if not isinstance(value, str): + return "" + printable = "".join(c for c in value if c.isprintable()) + return printable[:200] + "\u2026" if len(printable) > 200 else printable + + +def api_url(path: str) -> str: + """Join API_BASE with a path without producing a double slash. + + Mirrors Node's ``apiUrl()``. This is not cosmetic: API_BASE ends in "/" and + call sites used to concatenate "/static/...", producing + ``https://rafter.so/api//static/scan``, which production answers with a 308 + to the single-slash form. That worked only because the client followed the + redirect — so sable-2s6p's fix would have broken every core command. + """ + return f"{API_BASE.rstrip('/')}/{path.lstrip('/')}" + + +def api_request(method: str, url: str, **kwargs) -> "requests.Response": + """sable-2s6p — the HTTP entry point for every authenticated Rafter API call. + + ``allow_redirects=False`` is the point of it. ``requests`` replays headers on + a redirect and, unlike ``Authorization``, a custom ``x-api-key`` header is + NOT stripped when the host changes (``SessionRedirectMixin.rebuild_auth`` + only handles ``Authorization``). Since the API base is user-settable + (``--rafter-url``, self-hosted installs), a 302 from a misconfigured or + hostile endpoint would walk the caller's API key to another host. + + Nothing in this CLI needs to follow a redirect, so none of them do. A + redirect now arrives at the caller as a plain 3xx response, which every + caller already treats as a non-200 error. + + Use this for anything that sends ``x-api-key``. Plain ``requests`` is fine + for user-supplied webhooks and other unauthenticated calls. + """ + kwargs["allow_redirects"] = False + resp = requests.request(method, url, **kwargs) + if 300 <= resp.status_code < 400: + # Otherwise this surfaces as a bare non-200 with an empty body, which + # tells the user nothing about why. + target = _safe_for_terminal(resp.headers.get("location")) or "another host" + print( + f"The Rafter API redirected to {target}, and Rafter does not follow " + "redirects on authenticated requests — your API key would be sent to " + "the redirect target. If you are pointing Rafter at a self-hosted " + "instance, use its final URL.", + file=sys.stderr, + ) + return resp + + +def api_get(url: str, **kwargs) -> "requests.Response": + return api_request("GET", url, **kwargs) + + +def api_post(url: str, **kwargs) -> "requests.Response": + return api_request("POST", url, **kwargs) + + def resolve_key(cli_opt: str | None) -> str: """Resolve API key: --api-key flag > RAFTER_API_KEY env > global config.""" if cli_opt: diff --git a/python/tests/test_api_no_redirect.py b/python/tests/test_api_no_redirect.py new file mode 100644 index 00000000..f8c7a3fe --- /dev/null +++ b/python/tests/test_api_no_redirect.py @@ -0,0 +1,130 @@ +"""sable-2s6p — authenticated Rafter calls must not follow redirects. + +``requests.sessions.SessionRedirectMixin.rebuild_auth`` strips ``Authorization`` +on a host change and leaves arbitrary custom headers intact, so ``x-api-key`` +rides a 302 to whatever host it points at. The API base is user-settable +(``--rafter-url``, self-hosted installs), which makes that a real exfiltration +path rather than a theoretical one. + +Mirrors node/tests/api-no-redirect.test.ts. +""" +from __future__ import annotations + +import pathlib +import re +from unittest.mock import MagicMock, patch + +from rafter_cli.utils.api import api_get, api_post, api_request + +REPO_PY = pathlib.Path(__file__).resolve().parents[1] / "rafter_cli" + + +def _resp(status_code: int = 200, headers=None) -> MagicMock: + resp = MagicMock() + resp.status_code = status_code + resp.headers = headers or {} + return resp + + +class TestRedirectsRefused: + def test_api_request_forces_allow_redirects_false(self): + with patch("rafter_cli.utils.api.requests.request") as req: + req.return_value = _resp() + api_request("GET", "https://rafter.so/api/static/scan") + + assert req.call_args.kwargs["allow_redirects"] is False + + def test_callers_cannot_re_enable_redirects(self): + # Even an explicit allow_redirects=True is overridden — the point is + # that no call site can opt back into leaking the key. + with patch("rafter_cli.utils.api.requests.request") as req: + req.return_value = _resp() + api_request("GET", "https://rafter.so/x", allow_redirects=True) + + assert req.call_args.kwargs["allow_redirects"] is False + + def test_api_get_and_api_post_use_the_right_verbs(self): + with patch("rafter_cli.utils.api.requests.request") as req: + req.return_value = _resp() + api_get("https://rafter.so/x") + api_post("https://rafter.so/y") + + assert [c.args[0] for c in req.call_args_list] == ["GET", "POST"] + assert all(c.kwargs["allow_redirects"] is False for c in req.call_args_list) + + def test_a_refused_redirect_explains_itself(self, capsys): + with patch("rafter_cli.utils.api.requests.request") as req: + # ANSI escape included on purpose: an attacker-controlled Location + # must not be able to rewrite the user's terminal. + req.return_value = _resp( + 302, {"location": "https://evil.example/collect\x1b[31m"} + ) + api_get("https://rafter.so/api/static/scan") + + err = capsys.readouterr().err + assert "evil.example" in err + assert "does not follow redirects" in err + assert "self-hosted" in err + assert "\x1b" not in err + + +class TestNoCallSiteBypassesTheHelper: + """A new authenticated call site must not be able to reintroduce the hole.""" + + def test_no_bare_requests_call_sends_the_api_key(self): + offenders = [] + call = re.compile(r"\brequests\.(get|post|put|delete|patch|request)\(") + + for path in REPO_PY.rglob("*.py"): + if path.name == "api.py" and path.parent.name == "utils": + continue # the helper itself is where requests is allowed + lines = path.read_text().splitlines() + for i, line in enumerate(lines): + if not call.search(line): + continue + window = "\n".join(lines[i : i + 6]) + if "x-api-key" in window or "headers=headers" in window: + offenders.append(f"{path}:{i + 1}") + + assert offenders == [], ( + "These calls send the API key through bare requests, which replays it " + "across a cross-host redirect. Use api_get/api_post from " + f"rafter_cli.utils.api instead:\n" + "\n".join(offenders) + ) + + +class TestNoSecondSession: + def test_no_module_builds_its_own_requests_session(self): + offenders = [ + str(p) + for p in REPO_PY.rglob("*.py") + if not (p.name == "api.py" and p.parent.name == "utils") + and "requests.Session(" in p.read_text() + ] + assert offenders == [], ( + "A bare Session follows redirects by default. Use api_get/api_post " + "from rafter_cli.utils.api:\n" + "\n".join(offenders) + ) + + +class TestApiUrlConstruction: + def test_no_module_builds_a_double_slash_url(self): + # Not cosmetic. API_BASE ends in "/", and f"{API_BASE}/static/..." + # produced https://rafter.so/api//static/scan, which production answers + # with a 308. That worked only because the client followed redirects. + offenders = [] + for path in REPO_PY.rglob("*.py"): + for i, line in enumerate(path.read_text().splitlines()): + if "{API_BASE}/" in line: + offenders.append(f"{path}:{i + 1}") + assert offenders == [], ( + "These build a double-slash URL. Use api_url() instead:\n" + + "\n".join(offenders) + ) + + def test_api_url_joins_cleanly(self): + from rafter_cli.utils.api import API_BASE, api_url + + assert api_url("static/scan") == "https://rafter.so/api/static/scan" + assert api_url("/static/scan") == "https://rafter.so/api/static/scan" + assert API_BASE.endswith("/") # the trap this guards against diff --git a/python/tests/test_api_scope.py b/python/tests/test_api_scope.py index 6089745d..8e143543 100644 --- a/python/tests/test_api_scope.py +++ b/python/tests/test_api_scope.py @@ -101,7 +101,7 @@ def test_no_collisions(self): class TestRemoteScan403: """Verify _do_remote_scan properly handles 403 scope errors.""" - @patch("rafter_cli.commands.backend.requests.post") + @patch("rafter_cli.commands.backend.api_post") @patch("rafter_cli.commands.backend.detect_repo", return_value=("org/repo", "main", "github", "https://github.com/org/repo")) def test_scope_403_raises_exit_with_code_4(self, _mock_repo, mock_post, capsys): mock_post.return_value = _mock_response( @@ -124,7 +124,7 @@ def test_scope_403_raises_exit_with_code_4(self, _mock_repo, mock_post, capsys): assert "read access" in err assert "https://rfrr.co/account" in err - @patch("rafter_cli.commands.backend.requests.post") + @patch("rafter_cli.commands.backend.api_post") @patch("rafter_cli.commands.backend.detect_repo", return_value=("org/repo", "main", "github", "https://github.com/org/repo")) def test_generic_403_raises_exit_with_code_4(self, _mock_repo, mock_post, capsys): mock_post.return_value = _mock_response(403, "forbidden") @@ -141,7 +141,7 @@ def test_generic_403_raises_exit_with_code_4(self, _mock_repo, mock_post, capsys ) assert exc_info.value.exit_code == EXIT_INSUFFICIENT_SCOPE - @patch("rafter_cli.commands.backend.requests.post") + @patch("rafter_cli.commands.backend.api_post") @patch("rafter_cli.commands.backend.detect_repo", return_value=("org/repo", "main", "github", "https://github.com/org/repo")) def test_429_still_raises_quota_exhausted(self, _mock_repo, mock_post, capsys): mock_post.return_value = _mock_response(429, "quota exhausted") @@ -158,7 +158,7 @@ def test_429_still_raises_quota_exhausted(self, _mock_repo, mock_post, capsys): ) assert exc_info.value.exit_code == EXIT_QUOTA_EXHAUSTED - @patch("rafter_cli.commands.backend.requests.post") + @patch("rafter_cli.commands.backend.api_post") @patch("rafter_cli.commands.backend.detect_repo", return_value=("org/repo", "main", "github", "https://github.com/org/repo")) def test_200_succeeds(self, _mock_repo, mock_post): mock_post.return_value = _mock_response(200, "") @@ -184,7 +184,7 @@ class TestReadOnlyEndpoints: The server returns 200 for valid keys of either scope on GET endpoints. These tests verify the CLI doesn't accidentally scope-check GET calls.""" - @patch("rafter_cli.commands.backend.requests.get") + @patch("rafter_cli.commands.backend.api_get") def test_get_scan_200_with_read_key(self, mock_get): """GET /api/static/scan works fine — no scope check needed.""" mock_get.return_value = _mock_response(200, "") @@ -198,7 +198,7 @@ def test_get_scan_200_with_read_key(self, mock_get): result = _handle_scan_status_interactive("abc", {"x-api-key": "read_key"}, "json", True) assert result == 0 - @patch("rafter_cli.commands.backend.requests.get") + @patch("rafter_cli.commands.backend.api_get") def test_get_usage_200_with_read_key(self, mock_get, capsys): """GET /api/static/usage works with read-only key.""" mock_get.return_value = _mock_response(200, "") @@ -217,7 +217,7 @@ def test_get_usage_200_with_read_key(self, mock_get, capsys): class TestBackwardCompatibility: """Ensure existing 401 and other error paths are unaffected.""" - @patch("rafter_cli.commands.backend.requests.post") + @patch("rafter_cli.commands.backend.api_post") @patch("rafter_cli.commands.backend.detect_repo", return_value=("org/repo", "main", "github", "https://github.com/org/repo")) def test_401_raises_general_error(self, _mock_repo, mock_post, capsys): mock_post.return_value = _mock_response(401, "invalid api key") @@ -235,7 +235,7 @@ def test_401_raises_general_error(self, _mock_repo, mock_post, capsys): # 401 should NOT hit scope handler, falls through to general error assert exc_info.value.exit_code == EXIT_GENERAL_ERROR - @patch("rafter_cli.commands.backend.requests.post") + @patch("rafter_cli.commands.backend.api_post") @patch("rafter_cli.commands.backend.detect_repo", return_value=("org/repo", "main", "github", "https://github.com/org/repo")) def test_500_raises_general_error(self, _mock_repo, mock_post, capsys): mock_post.return_value = _mock_response(500, "internal server error") diff --git a/python/tests/test_plus_scan_approval.py b/python/tests/test_plus_scan_approval.py index 1046aeae..809dd58c 100644 --- a/python/tests/test_plus_scan_approval.py +++ b/python/tests/test_plus_scan_approval.py @@ -149,7 +149,7 @@ def test_refuses_exit_5_when_prompt_answered_no(self, monkeypatch): class TestGateIntegration: - @patch("rafter_cli.commands.backend.requests.post") + @patch("rafter_cli.commands.backend.api_post") def test_refuses_gated_plus_without_calling_backend(self, mock_post, monkeypatch): monkeypatch.delenv("RAFTER_CONFIRM", raising=False) with patch( @@ -168,7 +168,7 @@ def test_refuses_gated_plus_without_calling_backend(self, mock_post, monkeypatch assert exc.value.exit_code == EXIT_CONFIRMATION_REQUIRED mock_post.assert_not_called() - @patch("rafter_cli.commands.backend.requests.post") + @patch("rafter_cli.commands.backend.api_post") @patch( "rafter_cli.commands.backend.detect_repo", return_value=("owner/repo", "main", "github", None), diff --git a/python/tests/test_scan_poll_transient_500.py b/python/tests/test_scan_poll_transient_500.py index 3d7299a5..659466f9 100644 --- a/python/tests/test_scan_poll_transient_500.py +++ b/python/tests/test_scan_poll_transient_500.py @@ -74,7 +74,7 @@ def test_backoff_is_exponential_not_flat(self): assert [backoff_seconds(n) for n in (1, 2, 3, 4)] == [2, 4, 8, 16] def test_actually_sleeps_the_backoff_schedule(self, sleeps): - with patch("rafter_cli.commands.backend.requests.get") as get: + with patch("rafter_cli.commands.backend.api_get") as get: get.side_effect = [_processing()] + [ _server_500() for _ in range(MAX_TRANSIENT_POLL_FAILURES) ] @@ -93,7 +93,7 @@ def test_caps_total_failures_so_a_flapping_server_cannot_loop_forever(self): for _ in range(MAX_TOTAL_TRANSIENT_POLL_FAILURES + 5): flapping += [_server_500(), _processing()] - with patch("rafter_cli.commands.backend.requests.get") as get: + with patch("rafter_cli.commands.backend.api_get") as get: get.side_effect = flapping with pytest.raises(typer.Exit) as exc: _handle_scan_status_interactive("s1", HEADERS, "md", quiet=True) @@ -102,7 +102,7 @@ def test_caps_total_failures_so_a_flapping_server_cannot_loop_forever(self): def test_consecutive_counter_resets_on_a_successful_poll(self, sleeps): # Two blips far apart must NOT add up to a give-up. - with patch("rafter_cli.commands.backend.requests.get") as get: + with patch("rafter_cli.commands.backend.api_get") as get: get.side_effect = [ _processing(), _server_500(), @@ -123,7 +123,7 @@ def test_survives_a_nested_json_error_object(self): # methods on that object raised an AttributeError no caller catches, # surfacing as a traceback and defeating the retry entirely. nested = _resp(500, text=json.dumps({"error": {"message": "nested"}})) - with patch("rafter_cli.commands.backend.requests.get") as get: + with patch("rafter_cli.commands.backend.api_get") as get: get.side_effect = [_processing(), nested, _completed()] code = _handle_scan_status_interactive("s1", HEADERS, "md", quiet=True) @@ -132,7 +132,7 @@ def test_survives_a_nested_json_error_object(self): def test_truncates_a_very_long_server_error(self, capsys): huge = "x" * 5000 big = _resp(500, text=json.dumps({"error": huge})) - with patch("rafter_cli.commands.backend.requests.get") as get: + with patch("rafter_cli.commands.backend.api_get") as get: get.side_effect = [_processing()] + [ big for _ in range(MAX_TRANSIENT_POLL_FAILURES) ] @@ -148,7 +148,7 @@ def test_reports_the_real_attempt_count_not_the_consecutive_cap(self, capsys): for _ in range(MAX_TOTAL_TRANSIENT_POLL_FAILURES + 5): flapping += [_server_500(), _processing()] - with patch("rafter_cli.commands.backend.requests.get") as get: + with patch("rafter_cli.commands.backend.api_get") as get: get.side_effect = flapping with pytest.raises(typer.Exit): _handle_scan_status_interactive("s1", HEADERS, "md", quiet=True) @@ -158,7 +158,7 @@ def test_reports_the_real_attempt_count_not_the_consecutive_cap(self, capsys): assert f"after {MAX_TOTAL_TRANSIENT_POLL_FAILURES} attempts" in err def test_unreachable_api_is_not_blamed_on_the_report(self, capsys): - with patch("rafter_cli.commands.backend.requests.get") as get: + with patch("rafter_cli.commands.backend.api_get") as get: get.side_effect = [_processing()] + [ requests.ConnectionError("no route to host") for _ in range(MAX_TRANSIENT_POLL_FAILURES) @@ -174,7 +174,7 @@ def test_first_poll_retries_a_transient_500(self): # The give-up message tells the user to run `rafter get `, which # re-enters at the first poll. If that path did not retry, the remedy # we recommend would be defeated by one bad read. - with patch("rafter_cli.commands.backend.requests.get") as get: + with patch("rafter_cli.commands.backend.api_get") as get: get.side_effect = [_server_500(), _completed()] code = _handle_scan_status_interactive("s1", HEADERS, "md", quiet=True) @@ -184,14 +184,14 @@ def test_first_poll_retries_a_transient_500(self): def test_any_2xx_counts_as_success(self): # Node's axios accepts any 2xx; Python must not diverge. accepted = _resp(202, json_body={"status": "completed", "markdown": "# Done"}) - with patch("rafter_cli.commands.backend.requests.get") as get: + with patch("rafter_cli.commands.backend.api_get") as get: get.side_effect = [accepted] code = _handle_scan_status_interactive("s1", HEADERS, "md", quiet=True) assert code == EXIT_SUCCESS def test_retry_notice_goes_to_stderr(self, capsys): - with patch("rafter_cli.commands.backend.requests.get") as get: + with patch("rafter_cli.commands.backend.api_get") as get: get.side_effect = [_processing(), _server_500(), _completed()] _handle_scan_status_interactive("s1", HEADERS, "md", quiet=False) @@ -200,7 +200,7 @@ def test_retry_notice_goes_to_stderr(self, capsys): assert "retrying in 2s" in err def test_rides_out_a_single_500_and_completes(self, capsys): - with patch("rafter_cli.commands.backend.requests.get") as get: + with patch("rafter_cli.commands.backend.api_get") as get: get.side_effect = [_processing(), _server_500(), _completed()] code = _handle_scan_status_interactive("s1", HEADERS, "md", quiet=True) @@ -208,7 +208,7 @@ def test_rides_out_a_single_500_and_completes(self, capsys): assert get.call_count == 3 def test_rides_out_several_consecutive_500s(self): - with patch("rafter_cli.commands.backend.requests.get") as get: + with patch("rafter_cli.commands.backend.api_get") as get: get.side_effect = [ _processing(), _server_500(), @@ -222,14 +222,14 @@ def test_rides_out_several_consecutive_500s(self): assert get.call_count == 5 def test_midpoll_404_is_lag_not_a_missing_scan(self): - with patch("rafter_cli.commands.backend.requests.get") as get: + with patch("rafter_cli.commands.backend.api_get") as get: get.side_effect = [_processing(), _resp(404, text="{}"), _completed()] code = _handle_scan_status_interactive("s1", HEADERS, "md", quiet=True) assert code == EXIT_SUCCESS def test_first_poll_404_still_reports_not_found(self): - with patch("rafter_cli.commands.backend.requests.get") as get: + with patch("rafter_cli.commands.backend.api_get") as get: get.side_effect = [_resp(404, text="{}")] with pytest.raises(typer.Exit) as exc: _handle_scan_status_interactive("nope", HEADERS, "md", quiet=True) @@ -238,7 +238,7 @@ def test_first_poll_404_still_reports_not_found(self): assert get.call_count == 1 def test_gives_up_when_report_never_becomes_readable(self, capsys): - with patch("rafter_cli.commands.backend.requests.get") as get: + with patch("rafter_cli.commands.backend.api_get") as get: get.side_effect = [_processing()] + [ _server_500() for _ in range(MAX_TRANSIENT_POLL_FAILURES) ] @@ -253,7 +253,7 @@ def test_gives_up_when_report_never_becomes_readable(self, capsys): assert "Object not found" in err def test_does_not_retry_a_non_transient_error(self): - with patch("rafter_cli.commands.backend.requests.get") as get: + with patch("rafter_cli.commands.backend.api_get") as get: get.side_effect = [ _processing(), _resp(403, text=json.dumps({"error": "Invalid API key"})), @@ -265,7 +265,7 @@ def test_does_not_retry_a_non_transient_error(self): assert get.call_count == 2 def test_retries_transport_errors(self): - with patch("rafter_cli.commands.backend.requests.get") as get: + with patch("rafter_cli.commands.backend.api_get") as get: get.side_effect = [ _processing(), requests.ConnectionError("ECONNRESET"), @@ -279,7 +279,7 @@ def test_a_500_body_is_never_mistaken_for_a_report(self): """The pre-fix bug: the loop called .json() on the 500 body, got no status, fell out of the loop and wrote the error payload out as if it were results.""" - with patch("rafter_cli.commands.backend.requests.get") as get: + with patch("rafter_cli.commands.backend.api_get") as get: get.side_effect = [_processing()] + [ _server_500() for _ in range(MAX_TRANSIENT_POLL_FAILURES) ] diff --git a/python/tests/test_scan_remote.py b/python/tests/test_scan_remote.py index 1c5091fb..4acf0198 100644 --- a/python/tests/test_scan_remote.py +++ b/python/tests/test_scan_remote.py @@ -44,7 +44,7 @@ def _mock_response(status_code: int, text: str = "", json_body=None) -> MagicMoc class TestDoRemoteScan: """Unit tests for the core remote scan trigger function.""" - @patch("rafter_cli.commands.backend.requests.post") + @patch("rafter_cli.commands.backend.api_post") @patch("rafter_cli.commands.backend.detect_repo", return_value=("owner/repo", "main", "github", "https://github.com/owner/repo")) def test_success_skip_interactive(self, _mock_repo, mock_post): """200 with skip_interactive returns without polling.""" @@ -60,7 +60,7 @@ def test_success_skip_interactive(self, _mock_repo, mock_post): quiet=True, ) - @patch("rafter_cli.commands.backend.requests.post") + @patch("rafter_cli.commands.backend.api_post") @patch("rafter_cli.commands.backend.detect_repo", return_value=("owner/repo", "main", "github", "https://github.com/owner/repo")) def test_posts_correct_body(self, _mock_repo, mock_post): """Verify POST body contains repository_name, branch_name, scan_mode.""" @@ -83,7 +83,7 @@ def test_posts_correct_body(self, _mock_repo, mock_post): assert body["scan_mode"] == "fast" assert "github_token" not in body - @patch("rafter_cli.commands.backend.requests.post") + @patch("rafter_cli.commands.backend.api_post") @patch("rafter_cli.commands.backend.detect_repo", return_value=("owner/repo", "main", "github", "https://github.com/owner/repo")) def test_github_body_omits_provider_and_repo_url(self, _mock_repo, mock_post): """A github remote produces a body with NO provider/repo_url (byte-identical to today).""" @@ -110,7 +110,7 @@ def test_github_body_omits_provider_and_repo_url(self, _mock_repo, mock_post): assert "provider" not in body assert "repo_url" not in body - @patch("rafter_cli.commands.backend.requests.post") + @patch("rafter_cli.commands.backend.api_post") @patch("rafter_cli.commands.backend.detect_repo", return_value=("owner/repo", "main", "github", "https://github.com/owner/repo")) def test_explicit_provider_github_still_omits(self, _mock_repo, mock_post): """An explicit --provider github still omits provider/repo_url.""" @@ -132,7 +132,7 @@ def test_explicit_provider_github_still_omits(self, _mock_repo, mock_post): assert "provider" not in body assert "repo_url" not in body - @patch("rafter_cli.commands.backend.requests.post") + @patch("rafter_cli.commands.backend.api_post") @patch("rafter_cli.commands.backend.detect_repo", return_value=("owner/repo", "main", "github", "https://github.com/owner/repo")) def test_flag_provider_gitlab_sends_provider_and_repo_url(self, _mock_repo, mock_post): """--provider gitlab + --repo-url are sent for a non-github remote.""" @@ -154,7 +154,7 @@ def test_flag_provider_gitlab_sends_provider_and_repo_url(self, _mock_repo, mock assert body["provider"] == "gitlab" assert body["repo_url"] == "https://gitlab.com/group/project" - @patch("rafter_cli.commands.backend.requests.post") + @patch("rafter_cli.commands.backend.api_post") @patch("rafter_cli.commands.backend.detect_repo", return_value=("owner/repo", "main", "github", "https://github.com/owner/repo")) def test_flag_provider_bitbucket_sends_pair(self, _mock_repo, mock_post): """--provider bitbucket + --repo-url are sent together.""" @@ -176,7 +176,7 @@ def test_flag_provider_bitbucket_sends_pair(self, _mock_repo, mock_post): assert body["provider"] == "bitbucket" assert body["repo_url"] == "https://bitbucket.org/team/repo" - @patch("rafter_cli.commands.backend.requests.post") + @patch("rafter_cli.commands.backend.api_post") @patch("rafter_cli.commands.backend.detect_repo", return_value=("group/project", "main", None, None)) def test_provider_without_any_repo_url_is_omitted(self, _mock_repo, mock_post): """A resolved provider with no repo_url anywhere can't send the pair; stays backward-compatible.""" @@ -200,7 +200,7 @@ def test_provider_without_any_repo_url_is_omitted(self, _mock_repo, mock_post): assert "provider" not in body assert "repo_url" not in body - @patch("rafter_cli.commands.backend.requests.post") + @patch("rafter_cli.commands.backend.api_post") @patch( "rafter_cli.commands.backend.detect_repo", return_value=("group/project", "main", "gitlab", "https://gitlab.com/group/project"), @@ -224,7 +224,7 @@ def test_inferred_gitlab_provider_flows_into_body(self, _mock_repo, mock_post): assert body["provider"] == "gitlab" assert body["repo_url"] == "https://gitlab.com/group/project" - @patch("rafter_cli.commands.backend.requests.post") + @patch("rafter_cli.commands.backend.api_post") @patch( "rafter_cli.commands.backend.detect_repo", return_value=("group/project", "main", "gitlab", "https://gitlab.com/group/project"), @@ -249,7 +249,7 @@ def test_explicit_flag_overrides_inferred_provider(self, _mock_repo, mock_post): assert body["provider"] == "bitbucket" assert body["repo_url"] == "https://bitbucket.org/group/project" - @patch("rafter_cli.commands.backend.requests.post") + @patch("rafter_cli.commands.backend.api_post") @patch("rafter_cli.commands.backend.detect_repo", return_value=("owner/repo", "main", "github", "https://github.com/owner/repo")) def test_includes_github_token(self, _mock_repo, mock_post): """GitHub token is included in POST body when provided.""" @@ -268,7 +268,7 @@ def test_includes_github_token(self, _mock_repo, mock_post): _, kwargs = mock_post.call_args assert kwargs["json"]["github_token"] == "ghp_test123" - @patch("rafter_cli.commands.backend.requests.post") + @patch("rafter_cli.commands.backend.api_post") @patch("rafter_cli.commands.backend.detect_repo", return_value=("owner/repo", "main", "github", "https://github.com/owner/repo")) def test_plus_mode(self, _mock_repo, mock_post): """scan_mode=plus is sent when mode='plus'.""" @@ -287,7 +287,7 @@ def test_plus_mode(self, _mock_repo, mock_post): _, kwargs = mock_post.call_args assert kwargs["json"]["scan_mode"] == "plus" - @patch("rafter_cli.commands.backend.requests.post") + @patch("rafter_cli.commands.backend.api_post") @patch("rafter_cli.commands.backend.detect_repo", return_value=("owner/repo", "main", "github", "https://github.com/owner/repo")) def test_prints_scan_id_when_not_quiet(self, _mock_repo, mock_post, capsys): """Scan ID is printed to stderr when not quiet.""" @@ -305,7 +305,7 @@ def test_prints_scan_id_when_not_quiet(self, _mock_repo, mock_post, capsys): err = capsys.readouterr().err assert "s-xyz" in err - @patch("rafter_cli.commands.backend.requests.post") + @patch("rafter_cli.commands.backend.api_post") @patch("rafter_cli.commands.backend.detect_repo", return_value=("owner/repo", "main", "github", "https://github.com/owner/repo")) def test_auto_detect_message_when_not_explicit(self, _mock_repo, mock_post, capsys): """Auto-detection message prints when repo/branch not explicitly provided.""" @@ -323,7 +323,7 @@ def test_auto_detect_message_when_not_explicit(self, _mock_repo, mock_post, caps err = capsys.readouterr().err assert "auto-detected" in err.lower() - @patch("rafter_cli.commands.backend.requests.post") + @patch("rafter_cli.commands.backend.api_post") @patch("rafter_cli.commands.backend.detect_repo", return_value=("owner/repo", "main", "github", "https://github.com/owner/repo")) def test_429_raises_quota_exhausted(self, _mock_repo, mock_post): """HTTP 429 → exit code 3 (quota exhausted).""" @@ -340,7 +340,7 @@ def test_429_raises_quota_exhausted(self, _mock_repo, mock_post): ) assert exc_info.value.exit_code == EXIT_QUOTA_EXHAUSTED - @patch("rafter_cli.commands.backend.requests.post") + @patch("rafter_cli.commands.backend.api_post") @patch("rafter_cli.commands.backend.detect_repo", return_value=("owner/repo", "main", "github", "https://github.com/owner/repo")) def test_403_scope_raises_insufficient_scope(self, _mock_repo, mock_post, capsys): """HTTP 403 with scope keyword → exit code 4.""" @@ -361,7 +361,7 @@ def test_403_scope_raises_insufficient_scope(self, _mock_repo, mock_post, capsys ) assert exc_info.value.exit_code == EXIT_INSUFFICIENT_SCOPE - @patch("rafter_cli.commands.backend.requests.post") + @patch("rafter_cli.commands.backend.api_post") @patch("rafter_cli.commands.backend.detect_repo", return_value=("owner/repo", "main", "github", "https://github.com/owner/repo")) def test_403_quota_raises_quota_exhausted(self, _mock_repo, mock_post, capsys): """HTTP 403 with scan_mode body → exit code 3 (quota).""" @@ -380,7 +380,7 @@ def test_403_quota_raises_quota_exhausted(self, _mock_repo, mock_post, capsys): ) assert exc_info.value.exit_code == EXIT_QUOTA_EXHAUSTED - @patch("rafter_cli.commands.backend.requests.post") + @patch("rafter_cli.commands.backend.api_post") @patch("rafter_cli.commands.backend.detect_repo", return_value=("owner/repo", "main", "github", "https://github.com/owner/repo")) def test_401_raises_general_error(self, _mock_repo, mock_post): """HTTP 401 → exit code 1 (general error).""" @@ -397,7 +397,7 @@ def test_401_raises_general_error(self, _mock_repo, mock_post): ) assert exc_info.value.exit_code == EXIT_GENERAL_ERROR - @patch("rafter_cli.commands.backend.requests.post") + @patch("rafter_cli.commands.backend.api_post") @patch("rafter_cli.commands.backend.detect_repo", return_value=("owner/repo", "main", "github", "https://github.com/owner/repo")) def test_500_raises_general_error(self, _mock_repo, mock_post): """HTTP 500 → exit code 1 (general error).""" @@ -431,7 +431,7 @@ def test_detect_repo_failure_raises_general_error(self, mock_detect): assert exc_info.value.exit_code == EXIT_GENERAL_ERROR @patch("rafter_cli.commands.backend._handle_scan_status_interactive") - @patch("rafter_cli.commands.backend.requests.post") + @patch("rafter_cli.commands.backend.api_post") @patch("rafter_cli.commands.backend.detect_repo", return_value=("owner/repo", "main", "github", "https://github.com/owner/repo")) def test_calls_status_handler_when_not_skip_interactive( self, _mock_repo, mock_post, mock_status @@ -457,7 +457,7 @@ def test_calls_status_handler_when_not_skip_interactive( ) @patch("rafter_cli.commands.backend._handle_scan_status_interactive") - @patch("rafter_cli.commands.backend.requests.post") + @patch("rafter_cli.commands.backend.api_post") @patch("rafter_cli.commands.backend.detect_repo", return_value=("owner/repo", "main", "github", "https://github.com/owner/repo")) def test_skip_interactive_does_not_call_status_handler( self, _mock_repo, mock_post, mock_status @@ -476,7 +476,7 @@ def test_skip_interactive_does_not_call_status_handler( mock_status.assert_not_called() - @patch("rafter_cli.commands.backend.requests.post") + @patch("rafter_cli.commands.backend.api_post") @patch("rafter_cli.commands.backend.detect_repo", return_value=("owner/repo", "main", "github", "https://github.com/owner/repo")) def test_sends_api_key_header(self, _mock_repo, mock_post): """x-api-key header is set correctly.""" @@ -501,7 +501,7 @@ def test_sends_api_key_header(self, _mock_repo, mock_post): class TestHandleScanStatusInteractive: """Unit tests for the polling/status handler.""" - @patch("rafter_cli.commands.backend.requests.get") + @patch("rafter_cli.commands.backend.api_get") def test_completed_immediately(self, mock_get): """Scan already completed on first poll → return success.""" mock_get.return_value = _mock_response( @@ -514,7 +514,7 @@ def test_completed_immediately(self, mock_get): assert result == EXIT_SUCCESS assert mock_get.call_count == 1 - @patch("rafter_cli.commands.backend.requests.get") + @patch("rafter_cli.commands.backend.api_get") def test_completed_outputs_markdown(self, mock_get, capsys): """Completed scan outputs markdown to stdout.""" mock_get.return_value = _mock_response( @@ -525,7 +525,7 @@ def test_completed_outputs_markdown(self, mock_get, capsys): out = capsys.readouterr().out assert "# Results" in out - @patch("rafter_cli.commands.backend.requests.get") + @patch("rafter_cli.commands.backend.api_get") def test_completed_outputs_json(self, mock_get, capsys): """Completed scan outputs JSON to stdout.""" response_data = {"status": "completed", "findings": []} @@ -535,7 +535,7 @@ def test_completed_outputs_json(self, mock_get, capsys): out = capsys.readouterr().out assert json.loads(out) == response_data - @patch("rafter_cli.commands.backend.requests.get") + @patch("rafter_cli.commands.backend.api_get") def test_404_raises_exit_scan_not_found(self, mock_get): """HTTP 404 → exit code 2 (scan not found).""" mock_get.return_value = _mock_response(404, "not found") @@ -544,7 +544,7 @@ def test_404_raises_exit_scan_not_found(self, mock_get): _handle_scan_status_interactive("bad-id", {"x-api-key": "k"}, "md", True) assert exc_info.value.exit_code == EXIT_SCAN_NOT_FOUND - @patch("rafter_cli.commands.backend.requests.get") + @patch("rafter_cli.commands.backend.api_get") def test_non_200_raises_general_error(self, mock_get): """Non-200, non-404 → exit code 1 (general error).""" mock_get.return_value = _mock_response(500, "server error") @@ -553,7 +553,7 @@ def test_non_200_raises_general_error(self, mock_get): _handle_scan_status_interactive("s1", {"x-api-key": "k"}, "md", True) assert exc_info.value.exit_code == EXIT_GENERAL_ERROR - @patch("rafter_cli.commands.backend.requests.get") + @patch("rafter_cli.commands.backend.api_get") def test_failed_status_raises_general_error(self, mock_get): """Status 'failed' → exit code 1.""" mock_get.return_value = _mock_response( @@ -565,7 +565,7 @@ def test_failed_status_raises_general_error(self, mock_get): assert exc_info.value.exit_code == EXIT_GENERAL_ERROR @patch("rafter_cli.commands.backend.time.sleep") - @patch("rafter_cli.commands.backend.requests.get") + @patch("rafter_cli.commands.backend.api_get") def test_polls_queued_then_completed(self, mock_get, mock_sleep): """Queued → poll → completed.""" mock_get.side_effect = [ @@ -581,7 +581,7 @@ def test_polls_queued_then_completed(self, mock_get, mock_sleep): mock_sleep.assert_called_with(10) @patch("rafter_cli.commands.backend.time.sleep") - @patch("rafter_cli.commands.backend.requests.get") + @patch("rafter_cli.commands.backend.api_get") def test_polls_pending_then_completed(self, mock_get, mock_sleep): """Pending → poll → completed.""" mock_get.side_effect = [ @@ -596,7 +596,7 @@ def test_polls_pending_then_completed(self, mock_get, mock_sleep): assert mock_get.call_count == 2 @patch("rafter_cli.commands.backend.time.sleep") - @patch("rafter_cli.commands.backend.requests.get") + @patch("rafter_cli.commands.backend.api_get") def test_polls_processing_then_failed(self, mock_get, mock_sleep): """Processing → poll → failed.""" mock_get.side_effect = [ @@ -609,7 +609,7 @@ def test_polls_processing_then_failed(self, mock_get, mock_sleep): assert exc_info.value.exit_code == EXIT_GENERAL_ERROR @patch("rafter_cli.commands.backend.time.sleep") - @patch("rafter_cli.commands.backend.requests.get") + @patch("rafter_cli.commands.backend.api_get") def test_multiple_polls_before_completion(self, mock_get, mock_sleep): """Multiple polls before scan completes.""" mock_get.side_effect = [ @@ -626,7 +626,7 @@ def test_multiple_polls_before_completion(self, mock_get, mock_sleep): assert mock_get.call_count == 4 assert mock_sleep.call_count == 3 - @patch("rafter_cli.commands.backend.requests.get") + @patch("rafter_cli.commands.backend.api_get") def test_waiting_message_in_non_quiet_mode(self, mock_get, capsys): """Status messages print to stderr in non-quiet mode.""" mock_get.return_value = _mock_response( @@ -637,7 +637,7 @@ def test_waiting_message_in_non_quiet_mode(self, mock_get, capsys): err = capsys.readouterr().err assert "completed" in err.lower() - @patch("rafter_cli.commands.backend.requests.get") + @patch("rafter_cli.commands.backend.api_get") def test_quiet_mode_suppresses_stderr(self, mock_get, capsys): """Quiet mode suppresses status messages on stderr.""" mock_get.return_value = _mock_response( @@ -649,7 +649,7 @@ def test_quiet_mode_suppresses_stderr(self, mock_get, capsys): # Should NOT print "Scan completed!" in quiet mode assert "completed" not in err.lower() - @patch("rafter_cli.commands.backend.requests.get") + @patch("rafter_cli.commands.backend.api_get") def test_passes_format_param_to_api(self, mock_get): """format param is passed to the API.""" mock_get.return_value = _mock_response( @@ -660,7 +660,7 @@ def test_passes_format_param_to_api(self, mock_get): _, kwargs = mock_get.call_args assert kwargs["params"]["format"] == "json" - @patch("rafter_cli.commands.backend.requests.get") + @patch("rafter_cli.commands.backend.api_get") def test_passes_scan_id_param_to_api(self, mock_get): """scan_id param is passed to the API.""" mock_get.return_value = _mock_response( diff --git a/python/tests/test_sites.py b/python/tests/test_sites.py index a6cd0b1d..bff355d0 100644 --- a/python/tests/test_sites.py +++ b/python/tests/test_sites.py @@ -41,7 +41,7 @@ def _mock_response(status_code: int, json_body: dict | None = None) -> MagicMock class TestSitesCreateCli: - @patch("rafter_cli.commands.sites.requests.post") + @patch("rafter_cli.commands.sites.api_post") def test_success(self, mock_post): mock_post.return_value = _mock_response( 200, {"site": {"id": "p1"}, "run": {"id": "r1"}, "created": True} @@ -56,7 +56,7 @@ def test_success(self, mock_post): assert kwargs["json"] == {"url": "https://example.com"} assert kwargs["headers"] == {"x-api-key": "test-key"} - @patch("rafter_cli.commands.sites.requests.post") + @patch("rafter_cli.commands.sites.api_post") def test_401(self, mock_post): mock_post.return_value = _mock_response(401, {"error": "bad key"}) result = runner.invoke( @@ -64,7 +64,7 @@ def test_401(self, mock_post): ) assert result.exit_code == EXIT_GENERAL_ERROR - @patch("rafter_cli.commands.sites.requests.post") + @patch("rafter_cli.commands.sites.api_post") def test_403_wrong_scope(self, mock_post): mock_post.return_value = _mock_response( 403, {"error": "insufficient scope: requires read-and-scan"} @@ -74,7 +74,7 @@ def test_403_wrong_scope(self, mock_post): ) assert result.exit_code == EXIT_INSUFFICIENT_SCOPE - @patch("rafter_cli.commands.sites.requests.post") + @patch("rafter_cli.commands.sites.api_post") def test_429(self, mock_post): mock_post.return_value = _mock_response(429, {"error": "Rate limit exceeded"}) result = runner.invoke( @@ -95,7 +95,7 @@ def test_rejects_format_md(self): class TestSitesScanCli: - @patch("rafter_cli.commands.sites.requests.post") + @patch("rafter_cli.commands.sites.api_post") def test_sends_project_id_for_bare_id(self, mock_post): mock_post.return_value = _mock_response(200, {"run": {"id": "r1"}}) result = runner.invoke(sites_app, ["scan", "proj-123", "-k", "test-key"]) @@ -103,7 +103,7 @@ def test_sends_project_id_for_bare_id(self, mock_post): _, kwargs = mock_post.call_args assert kwargs["json"] == {"projectId": "proj-123"} - @patch("rafter_cli.commands.sites.requests.post") + @patch("rafter_cli.commands.sites.api_post") def test_sends_url_for_url(self, mock_post): mock_post.return_value = _mock_response(200, {"run": {"id": "r1"}}) result = runner.invoke( @@ -113,7 +113,7 @@ def test_sends_url_for_url(self, mock_post): _, kwargs = mock_post.call_args assert kwargs["json"] == {"url": "https://example.com"} - @patch("rafter_cli.commands.sites.requests.post") + @patch("rafter_cli.commands.sites.api_post") def test_includes_sections(self, mock_post): mock_post.return_value = _mock_response(200, {"run": {"id": "r1"}}) result = runner.invoke( @@ -124,7 +124,7 @@ def test_includes_sections(self, mock_post): _, kwargs = mock_post.call_args assert kwargs["json"] == {"projectId": "proj-123", "sections": ["security", "dns"]} - @patch("rafter_cli.commands.sites.requests.post") + @patch("rafter_cli.commands.sites.api_post") def test_rejects_invalid_section(self, mock_post): result = runner.invoke( sites_app, @@ -133,13 +133,13 @@ def test_rejects_invalid_section(self, mock_post): assert result.exit_code == EXIT_GENERAL_ERROR mock_post.assert_not_called() - @patch("rafter_cli.commands.sites.requests.post") + @patch("rafter_cli.commands.sites.api_post") def test_404_not_owned(self, mock_post): mock_post.return_value = _mock_response(404, {"error": "not found"}) result = runner.invoke(sites_app, ["scan", "proj-123", "-k", "test-key"]) assert result.exit_code == EXIT_SCAN_NOT_FOUND - @patch("rafter_cli.commands.sites.requests.post") + @patch("rafter_cli.commands.sites.api_post") def test_403_run_limit(self, mock_post): mock_post.return_value = _mock_response(403, {"error": "run limit reached"}) result = runner.invoke(sites_app, ["scan", "proj-123", "-k", "test-key"]) @@ -150,7 +150,7 @@ def test_403_run_limit(self, mock_post): class TestSitesListCli: - @patch("rafter_cli.commands.sites.requests.get") + @patch("rafter_cli.commands.sites.api_get") def test_passes_pagination_params(self, mock_get): mock_get.return_value = _mock_response( 200, {"sites": [], "limit": 10, "offset": 5, "has_more": False} @@ -173,7 +173,7 @@ def test_passes_pagination_params(self, mock_get): "include_archived": "true", } - @patch("rafter_cli.commands.sites.requests.get") + @patch("rafter_cli.commands.sites.api_get") def test_401(self, mock_get): mock_get.return_value = _mock_response(401, {"error": "invalid key"}) result = runner.invoke(sites_app, ["list", "-k", "test-key"]) @@ -184,7 +184,7 @@ def test_401(self, mock_get): class TestSitesGetCli: - @patch("rafter_cli.commands.sites.requests.get") + @patch("rafter_cli.commands.sites.api_get") def test_success(self, mock_get): mock_get.return_value = _mock_response( 200, @@ -199,13 +199,13 @@ def test_success(self, mock_get): args, _ = mock_get.call_args assert args[0].endswith("/static/sites/p1") - @patch("rafter_cli.commands.sites.requests.get") + @patch("rafter_cli.commands.sites.api_get") def test_404(self, mock_get): mock_get.return_value = _mock_response(404, {"error": "not found"}) result = runner.invoke(sites_app, ["get", "nonexistent", "-k", "test-key"]) assert result.exit_code == EXIT_SCAN_NOT_FOUND - @patch("rafter_cli.commands.sites.requests.get") + @patch("rafter_cli.commands.sites.api_get") def test_429(self, mock_get): mock_get.return_value = _mock_response( 429, {"error": "Rate limit exceeded", "retryAfter": 30} @@ -218,7 +218,7 @@ def test_429(self, mock_get): class TestMcpSitesCreate: - @patch("rafter_cli.commands.mcp_server.requests.post") + @patch("rafter_cli.commands.mcp_server.api_post") @patch("rafter_cli.commands.mcp_server.resolve_mcp_api_key", return_value="test-key") def test_success(self, _mock_key, mock_post): mock_post.return_value = _mock_response( @@ -229,7 +229,7 @@ def test_success(self, _mock_key, mock_post): _, kwargs = mock_post.call_args assert kwargs["json"] == {"url": "https://example.com"} - @patch("rafter_cli.commands.mcp_server.requests.post") + @patch("rafter_cli.commands.mcp_server.api_post") @patch("rafter_cli.commands.mcp_server.resolve_mcp_api_key", return_value="test-key") def test_401_raises(self, _mock_key, mock_post): mock_post.return_value = _mock_response(401, {"error": "invalid key"}) @@ -243,7 +243,7 @@ def test_missing_key_raises_without_crashing(self, _mock_key): class TestMcpSitesScan: - @patch("rafter_cli.commands.mcp_server.requests.post") + @patch("rafter_cli.commands.mcp_server.api_post") @patch("rafter_cli.commands.mcp_server.resolve_mcp_api_key", return_value="test-key") def test_by_project_id(self, _mock_key, mock_post): mock_post.return_value = _mock_response(200, {"run": {"id": "r1"}}) @@ -262,7 +262,7 @@ def test_rejects_both_project_id_and_url(self): with pytest.raises(RuntimeError, match="not both"): handle_sites_scan(project_id="proj-1", url="https://example.com") - @patch("rafter_cli.commands.mcp_server.requests.post") + @patch("rafter_cli.commands.mcp_server.api_post") @patch("rafter_cli.commands.mcp_server.resolve_mcp_api_key", return_value="test-key") def test_404_not_owned(self, _mock_key, mock_post): mock_post.return_value = _mock_response(404, {"error": "not found"}) @@ -271,7 +271,7 @@ def test_404_not_owned(self, _mock_key, mock_post): class TestMcpSitesList: - @patch("rafter_cli.commands.mcp_server.requests.get") + @patch("rafter_cli.commands.mcp_server.api_get") @patch("rafter_cli.commands.mcp_server.resolve_mcp_api_key", return_value="test-key") def test_success_with_params(self, _mock_key, mock_get): mock_get.return_value = _mock_response( @@ -281,7 +281,7 @@ def test_success_with_params(self, _mock_key, mock_get): _, kwargs = mock_get.call_args assert kwargs["params"] == {"limit": "5", "include_archived": "true"} - @patch("rafter_cli.commands.mcp_server.requests.get") + @patch("rafter_cli.commands.mcp_server.api_get") @patch("rafter_cli.commands.mcp_server.resolve_mcp_api_key", return_value="test-key") def test_429_raises(self, _mock_key, mock_get): mock_get.return_value = _mock_response(429, {"error": "Rate limit exceeded"}) @@ -290,7 +290,7 @@ def test_429_raises(self, _mock_key, mock_get): class TestMcpSitesGet: - @patch("rafter_cli.commands.mcp_server.requests.get") + @patch("rafter_cli.commands.mcp_server.api_get") @patch("rafter_cli.commands.mcp_server.resolve_mcp_api_key", return_value="test-key") def test_success(self, _mock_key, mock_get): mock_get.return_value = _mock_response( @@ -304,7 +304,7 @@ def test_success(self, _mock_key, mock_get): result = handle_sites_get("p1") assert result["site"]["id"] == "p1" - @patch("rafter_cli.commands.mcp_server.requests.get") + @patch("rafter_cli.commands.mcp_server.api_get") @patch("rafter_cli.commands.mcp_server.resolve_mcp_api_key", return_value="test-key") def test_404_raises(self, _mock_key, mock_get): mock_get.return_value = _mock_response(404, {"error": "not found"})