From 99717a1fdf1e0a76d4cb8ca75cf8497fda8437ae Mon Sep 17 00:00:00 2001 From: Andrei Date: Thu, 30 Jul 2026 23:51:12 +0300 Subject: [PATCH 1/2] feat: attachments 1.1.3: todos-task integration attachments 1.1.3: todos-task integration (link-task/complete-task/task-journal history/resolve-evidence/watch) unusable on remote-/v1 fleet machines attachments expects a todos REST at --todos-url (default http://localhost:3000, TODOS_URL env) exposing /api/tasks. On /v1 fleet boxes: (a) nothing listens on 3000: 'Error: Could not reach todos server at http://localhost:3000: Unable to connect.'; (b) 'todos serve --port 3000' => 'REMOTE_COMMAND_UNSUPPORTED: serve 3000 is not supported by the Todos /v1 CLI; local SQLite fallback is disabled'; (c) --todos-url https://todos.hasna.xyz reaches a live /api/tasks (list 200 []) but attachments sends no auth so a real task id returns 'Error: Task not found: 7b02cc81-fae1-4aea-9486-672cbf3bec65'. Fix: attachments should send todos auth (HASNA_TODOS_API_KEY / TODOS_API_KEY) or speak the /v1 surface. Repro on this box 2026-07-24, attachments 1.1.3. Fallback documented in attachments-task-evidence skill. X-Factory-Run: run_332d8ee59b9d X-Factory-Task: 27c1f140-3153-4c2f-8135-670d220f4000 --- docs/configuration.md | 1 + scripts/test.sh | 2 ++ src/cli/commands/complete-task.test.ts | 3 ++ src/cli/commands/complete-task.ts | 11 +++--- src/cli/commands/link-task.test.ts | 3 ++ src/cli/commands/link-task.ts | 5 +-- src/cli/commands/resolve-evidence.test.ts | 4 +++ src/cli/commands/resolve-evidence.ts | 3 +- src/cli/commands/task-journal.test.ts | 8 +++++ src/cli/commands/task-journal.ts | 13 +++++-- src/cli/commands/watch.test.ts | 4 +++ src/cli/commands/watch.ts | 3 +- src/core/todos.test.ts | 44 +++++++++++++++++++++++ src/core/todos.ts | 9 +++++ src/mcp/server.ts | 41 ++------------------- 15 files changed, 103 insertions(+), 51 deletions(-) create mode 100644 src/core/todos.test.ts create mode 100644 src/core/todos.ts diff --git a/docs/configuration.md b/docs/configuration.md index 79f638c..988b688 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -29,6 +29,7 @@ download caps, email gates, expiry beyond seven days, and `never` expiry. |----------|---------| | `HASNA_ATTACHMENTS_DB_PATH` | SQLite database override | | `ATTACHMENTS_API_TOKEN`, `HASNA_ATTACHMENTS_API_TOKEN` | Local `/api` authentication | +| `HASNA_TODOS_API_KEY`, `TODOS_API_KEY` | Todos API authentication for task integrations | | `ATTACHMENTS_MAX_SIZE` | Upload limit override in bytes | | `ATTACHMENTS_TRACK_COSTS` | Enables economy tracking when set | | `ATTACHMENTS_ECONOMY_URL` | Economy URL; defaults to `http://localhost:3460` | diff --git a/scripts/test.sh b/scripts/test.sh index d457f1c..cc5d9c6 100755 --- a/scripts/test.sh +++ b/scripts/test.sh @@ -20,6 +20,8 @@ unset HASNA_ATTACHMENTS_API_URL unset HASNA_ATTACHMENTS_API_KEY unset ATTACHMENTS_API_URL unset ATTACHMENTS_API_KEY +unset HASNA_TODOS_API_KEY +unset TODOS_API_KEY PASS=0 FAIL=0 diff --git a/src/cli/commands/complete-task.test.ts b/src/cli/commands/complete-task.test.ts index 4ba39fb..e750f55 100644 --- a/src/cli/commands/complete-task.test.ts +++ b/src/cli/commands/complete-task.test.ts @@ -174,6 +174,7 @@ describe("completeTaskWithFiles", () => { it("uploads files, PATCHes evidence into task metadata, then completes", async () => { const upload = makeUpload("att_001", "https://example.com/att_001"); const fakeFetch = makeFetch(); + process.env.HASNA_TODOS_API_KEY = "remote-key"; const result = await completeTaskWithFiles( "TASK-001", @@ -182,6 +183,7 @@ describe("completeTaskWithFiles", () => { makeStoreFactory(upload), fakeFetch ); + delete process.env.HASNA_TODOS_API_KEY; expect(upload).toHaveBeenCalledTimes(1); expect(upload).toHaveBeenCalledWith("/tmp/file.txt", { expiry: undefined }); @@ -189,6 +191,7 @@ describe("completeTaskWithFiles", () => { const calls = (fakeFetch as ReturnType).mock.calls as Array<[string, RequestInit | undefined]>; // GET -> PATCH -> POST /complete expect(calls).toHaveLength(3); + expect(calls.every(([, init]) => new Headers(init?.headers).get("x-api-key") === "remote-key")).toBe(true); expect(calls[0][0]).toBe("http://localhost:3000/api/tasks/TASK-001"); expect((calls[0][1]?.method ?? "GET").toUpperCase()).toBe("GET"); expect(calls[1][0]).toBe("http://localhost:3000/api/tasks/TASK-001"); diff --git a/src/cli/commands/complete-task.ts b/src/cli/commands/complete-task.ts index 84d8fa2..36edee1 100644 --- a/src/cli/commands/complete-task.ts +++ b/src/cli/commands/complete-task.ts @@ -1,5 +1,6 @@ import { Command } from "commander"; import { resolveStore, type Store } from "../../core/store"; +import { withTodosAuth } from "../../core/todos"; export interface CompleteTaskOptions { file?: string[]; @@ -75,7 +76,7 @@ export async function completeTaskWithFiles( // 2. Read the current task so we can merge (not clobber) its metadata and honor // optimistic concurrency via its version. - const getResponse = await fetchFn(taskUrl); + const getResponse = await fetchFn(taskUrl, withTodosAuth()); if (!getResponse.ok) { if (getResponse.status === 404) { throw new Error(`Task not found: ${taskId}`); @@ -114,11 +115,11 @@ export async function completeTaskWithFiles( if (typeof task.version === "number") { patchBody.version = task.version; } - const patchResponse = await fetchFn(taskUrl, { + const patchResponse = await fetchFn(taskUrl, withTodosAuth({ method: "PATCH", headers: { "Content-Type": "application/json" }, body: JSON.stringify(patchBody), - }); + })); if (!patchResponse.ok) { const responseBody = await patchResponse.text().catch(() => ""); throw new Error( @@ -127,11 +128,11 @@ export async function completeTaskWithFiles( } // 4. Mark the task complete. - const completeResponse = await fetchFn(`${taskUrl}/complete`, { + const completeResponse = await fetchFn(`${taskUrl}/complete`, withTodosAuth({ method: "POST", headers: { "Content-Type": "application/json" }, body: JSON.stringify({}), - }); + })); if (!completeResponse.ok) { if (completeResponse.status === 404) { throw new Error(`Task not found: ${taskId}`); diff --git a/src/cli/commands/link-task.test.ts b/src/cli/commands/link-task.test.ts index 6a09b82..cbfc37a 100644 --- a/src/cli/commands/link-task.test.ts +++ b/src/cli/commands/link-task.test.ts @@ -106,12 +106,15 @@ describe("linkAttachmentToTask", () => { mockFindById.mockImplementation(() => att); const fakeFetch = makeFetch(200); + process.env.TODOS_API_KEY = "remote-key"; await linkAttachmentToTask("att_abc123", "TASK-001", "http://localhost:3000", fakeFetch); + delete process.env.TODOS_API_KEY; expect(fakeFetch).toHaveBeenCalledTimes(1); const [url, opts] = (fakeFetch as ReturnType).mock.calls[0] as [string, RequestInit]; expect(url).toBe("http://localhost:3000/api/tasks/TASK-001"); expect(opts.method).toBe("PATCH"); + expect(new Headers(opts.headers).get("x-api-key")).toBe("remote-key"); const body = JSON.parse(opts.body as string); expect(body.metadata._attachments).toHaveLength(1); diff --git a/src/cli/commands/link-task.ts b/src/cli/commands/link-task.ts index 0210b1b..9ffbfa7 100644 --- a/src/cli/commands/link-task.ts +++ b/src/cli/commands/link-task.ts @@ -1,5 +1,6 @@ import { Command } from "commander"; import { resolveStore } from "../../core/store"; +import { withTodosAuth } from "../../core/todos"; export interface LinkTaskOptions { todosUrl?: string; @@ -42,7 +43,7 @@ export async function linkAttachmentToTask( }; const url = `${todosUrl}/api/tasks/${taskId}`; - const response = await fetchFn(url, { + const response = await fetchFn(url, withTodosAuth({ method: "PATCH", headers: { "Content-Type": "application/json" }, body: JSON.stringify({ @@ -50,7 +51,7 @@ export async function linkAttachmentToTask( _attachments: [entry], }, }), - }); + })); if (!response.ok) { if (response.status === 404) { diff --git a/src/cli/commands/resolve-evidence.test.ts b/src/cli/commands/resolve-evidence.test.ts index fa132f0..2c3b71e 100644 --- a/src/cli/commands/resolve-evidence.test.ts +++ b/src/cli/commands/resolve-evidence.test.ts @@ -130,9 +130,13 @@ describe("resolveEvidence", () => { { id: "att_abc123", link: "https://stale-link.example.com", filename: "report.pdf", size: 1258291 }, ]); const fakeFetch = makeFetch(200, task); + process.env.TODOS_API_KEY = "remote-key"; const result = await resolveEvidence("TASK-001", { todosUrl: "http://localhost:3000" }, fakeFetch); + delete process.env.TODOS_API_KEY; + const [, init] = (fakeFetch as ReturnType).mock.calls[0] as [string, RequestInit]; + expect(new Headers(init.headers).get("x-api-key")).toBe("remote-key"); expect(result).toHaveLength(1); expect(result[0].id).toBe("att_abc123"); // Should use the DB link (fresh), not the stale one stored in the task diff --git a/src/cli/commands/resolve-evidence.ts b/src/cli/commands/resolve-evidence.ts index c1a2eb2..6c511e4 100644 --- a/src/cli/commands/resolve-evidence.ts +++ b/src/cli/commands/resolve-evidence.ts @@ -1,5 +1,6 @@ import { Command } from "commander"; import { resolveStore } from "../../core/store"; +import { withTodosAuth } from "../../core/todos"; export interface ResolveEvidenceOptions { todosUrl?: string; @@ -36,7 +37,7 @@ export async function resolveEvidence( let response: Response; try { - response = await fetchFn(url); + response = await fetchFn(url, withTodosAuth()); } catch (err: unknown) { const message = err instanceof Error ? err.message : String(err); throw new Error(`Could not reach todos server at ${todosUrl}: ${message}`); diff --git a/src/cli/commands/task-journal.test.ts b/src/cli/commands/task-journal.test.ts index 5cbef79..271b940 100644 --- a/src/cli/commands/task-journal.test.ts +++ b/src/cli/commands/task-journal.test.ts @@ -92,7 +92,11 @@ describe("fetchTaskMeta", () => { assignee: "aurelius", created_at: "2026-03-14T10:23:00Z", }); + process.env.TODOS_API_KEY = "remote-key"; const meta = await fetchTaskMeta("TASK-001", "http://localhost:3000", fakeFetch); + delete process.env.TODOS_API_KEY; + const [, init] = (fakeFetch as ReturnType).mock.calls[0] as [string, RequestInit]; + expect(new Headers(init.headers).get("x-api-key")).toBe("remote-key"); expect(meta).not.toBeNull(); expect(meta?.subject).toBe("Fix auth bug"); expect(meta?.status).toBe("completed"); @@ -124,7 +128,11 @@ describe("fetchTaskHistory", () => { { timestamp: "2026-03-14T10:45:00Z", action: "started", actor: "aurelius" }, { timestamp: "2026-03-14T11:30:00Z", action: "completed", actor: "aurelius", progress: 100 }, ]); + process.env.HASNA_TODOS_API_KEY = "remote-key"; const history = await fetchTaskHistory("TASK-001", "http://localhost:3000", fakeFetch); + delete process.env.HASNA_TODOS_API_KEY; + const [, init] = (fakeFetch as ReturnType).mock.calls[0] as [string, RequestInit]; + expect(new Headers(init.headers).get("x-api-key")).toBe("remote-key"); expect(history).toHaveLength(3); expect(history[0].action).toBe("created"); expect(history[0].actor).toBe("julius"); diff --git a/src/cli/commands/task-journal.ts b/src/cli/commands/task-journal.ts index c3293bd..911c779 100644 --- a/src/cli/commands/task-journal.ts +++ b/src/cli/commands/task-journal.ts @@ -1,6 +1,7 @@ import { Command } from "commander"; import type { Attachment } from "../../core/db"; import { resolveStore, type Store } from "../../core/store"; +import { withTodosAuth } from "../../core/todos"; export interface TaskJournalOptions { todosUrl?: string; @@ -39,7 +40,7 @@ export async function fetchTaskMeta( fetchFn: typeof fetch = fetch ): Promise { try { - const response = await fetchFn(`${todosUrl}/api/tasks/${taskId}`); + const response = await fetchFn(`${todosUrl}/api/tasks/${taskId}`, withTodosAuth()); if (response.status === 404) return null; if (!response.ok) return null; const data = await response.json() as Record; @@ -66,7 +67,10 @@ export async function fetchTaskHistory( fetchFn: typeof fetch = fetch ): Promise { try { - const response = await fetchFn(`${todosUrl}/api/tasks/${taskId}/history`); + const response = await fetchFn( + `${todosUrl}/api/tasks/${taskId}/history`, + withTodosAuth() + ); if (!response.ok) return []; const data = await response.json() as unknown; if (!Array.isArray(data)) return []; @@ -261,7 +265,10 @@ export function registerTaskJournal(program: Command): void { if (!todosReachable && journal.attachments.length === 0 && !journal.task.subject) { // Attempt a direct 404 check try { - const response = await fetch(`${todosUrl}/api/tasks/${taskId}`); + const response = await fetch( + `${todosUrl}/api/tasks/${taskId}`, + withTodosAuth() + ); if (response.status === 404) { process.stderr.write(`Error: Task not found: ${taskId}\n`); process.exit(1); diff --git a/src/cli/commands/watch.test.ts b/src/cli/commands/watch.test.ts index d482844..c2a55e0 100644 --- a/src/cli/commands/watch.test.ts +++ b/src/cli/commands/watch.test.ts @@ -339,6 +339,7 @@ describe("handleTaskEvent", () => { describe("connectAndWatch reconnect logic", () => { it("reconnects after stream error with backoff", async () => { + process.env.TODOS_API_KEY = "remote-key"; let callCount = 0; const controller = new AbortController(); const sleepCalls: number[] = []; @@ -382,7 +383,10 @@ describe("connectAndWatch reconnect logic", () => { stdoutSpy.mockRestore(); } + delete process.env.TODOS_API_KEY; expect(callCount).toBe(2); + const [, init] = mockFetch.mock.calls[0] as [string, RequestInit]; + expect(new Headers(init.headers).get("x-api-key")).toBe("remote-key"); expect(sleepCalls).toHaveLength(1); expect(sleepCalls[0]).toBe(5000); const errOutput = err.join(""); diff --git a/src/cli/commands/watch.ts b/src/cli/commands/watch.ts index a06062d..fc9170d 100644 --- a/src/cli/commands/watch.ts +++ b/src/cli/commands/watch.ts @@ -1,5 +1,6 @@ import { Command } from "commander"; import { resolveStore, type Store } from "../../core/store"; +import { withTodosAuth } from "../../core/todos"; import { checkAttachment } from "./health-check"; // --------------------------------------------------------------------------- @@ -146,7 +147,7 @@ export async function connectAndWatch( process.stdout.write(`[watch] Connecting to ${url}\n`); } - const response = await fetchFn(url, { signal }); + const response = await fetchFn(url, withTodosAuth({ signal })); if (!response.ok) { throw new Error(`HTTP ${response.status}`); diff --git a/src/core/todos.test.ts b/src/core/todos.test.ts new file mode 100644 index 0000000..1a0d7a1 --- /dev/null +++ b/src/core/todos.test.ts @@ -0,0 +1,44 @@ +import { afterEach, describe, expect, it } from "bun:test"; +import { withTodosAuth } from "./todos"; + +afterEach(() => { + delete process.env.HASNA_TODOS_API_KEY; + delete process.env.TODOS_API_KEY; +}); + +describe("withTodosAuth", () => { + it("returns the original request init when no API key is configured", () => { + const init = { method: "POST" }; + + expect(withTodosAuth(init)).toBe(init); + expect(withTodosAuth()).toBeUndefined(); + }); + + it("uses TODOS_API_KEY as an x-api-key header", () => { + process.env.TODOS_API_KEY = "todos-fallback-key"; + + const init = withTodosAuth({ headers: { "Content-Type": "application/json" } }); + const headers = new Headers(init?.headers); + + expect(headers.get("x-api-key")).toBe("todos-fallback-key"); + expect(headers.get("content-type")).toBe("application/json"); + }); + + it("prefers a non-empty HASNA_TODOS_API_KEY", () => { + process.env.HASNA_TODOS_API_KEY = "hasna-key"; + process.env.TODOS_API_KEY = "fallback-key"; + + const headers = new Headers(withTodosAuth()?.headers); + + expect(headers.get("x-api-key")).toBe("hasna-key"); + }); + + it("falls back when HASNA_TODOS_API_KEY is empty", () => { + process.env.HASNA_TODOS_API_KEY = ""; + process.env.TODOS_API_KEY = "fallback-key"; + + const headers = new Headers(withTodosAuth()?.headers); + + expect(headers.get("x-api-key")).toBe("fallback-key"); + }); +}); diff --git a/src/core/todos.ts b/src/core/todos.ts new file mode 100644 index 0000000..90dbb75 --- /dev/null +++ b/src/core/todos.ts @@ -0,0 +1,9 @@ +export function withTodosAuth(init?: RequestInit): RequestInit | undefined { + const apiKey = process.env.HASNA_TODOS_API_KEY || process.env.TODOS_API_KEY; + if (!apiKey) return init; + + const headers = new Headers(init?.headers); + headers.set("x-api-key", apiKey); + + return { ...init, headers }; +} diff --git a/src/mcp/server.ts b/src/mcp/server.ts index 25acf7d..bb64fb3 100644 --- a/src/mcp/server.ts +++ b/src/mcp/server.ts @@ -11,6 +11,7 @@ import { nanoid } from "nanoid"; import { computeReport } from "../cli/commands/report.js"; import { runHealthCheck } from "../cli/commands/health-check.js"; import { completeTaskWithFiles } from "../cli/commands/complete-task.js"; +import { linkAttachmentToTask } from "../cli/commands/link-task.js"; import { resolveStore, LocalStore } from "../core/store.js"; import { getConfig, parseExpiryStrict, setConfig } from "../core/config.js"; @@ -849,45 +850,7 @@ async function handleLinkToTask(args: { todos_url?: string; }) { const todosUrl = args.todos_url ?? "http://localhost:3000"; - const store = resolveStore(); - let att; - try { - att = await store.get(args.attachment_id); - } finally { - store.close(); - } - - if (!att) { - throw new Error(`Attachment not found: ${args.attachment_id}`); - } - - const url = `${todosUrl}/api/tasks/${args.task_id}`; - const response = await fetch(url, { - method: "PATCH", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ - metadata: { - _attachments: [ - { - id: att.id, - link: att.link, - filename: att.filename, - size: att.size, - }, - ], - }, - }), - }); - - if (!response.ok) { - if (response.status === 404) { - throw new Error(`Task not found: ${args.task_id}`); - } - const body = await response.text().catch(() => ""); - throw new Error( - `Failed to update task ${args.task_id}: HTTP ${response.status}${body ? ` — ${body}` : ""}` - ); - } + await linkAttachmentToTask(args.attachment_id, args.task_id, todosUrl); return `Linked ${args.attachment_id} → task ${args.task_id}`; } From 982c7ea3d902934061b8a0492761a08ba011cbed Mon Sep 17 00:00:00 2001 From: Andrei Hasna Date: Sat, 1 Aug 2026 17:13:43 +0300 Subject: [PATCH 2/2] fix: bind todos auth to trusted origins Prevent configured todos API keys from being forwarded to caller-controlled todos_url override origins. Default localhost keeps working, and remote todos auth now requires a matching configured todos API URL origin. Agent: unresolved-account001 --- docs/configuration.md | 1 + scripts/test.sh | 2 ++ src/cli/commands/complete-task.ts | 7 ++--- src/cli/commands/link-task.ts | 2 +- src/cli/commands/resolve-evidence.ts | 2 +- src/cli/commands/task-journal.ts | 15 +++++------ src/cli/commands/watch.ts | 2 +- src/core/todos.test.ts | 39 ++++++++++++++++++++++------ src/core/todos.ts | 28 +++++++++++++++++++- 9 files changed, 74 insertions(+), 24 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 988b688..86f9610 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -30,6 +30,7 @@ download caps, email gates, expiry beyond seven days, and `never` expiry. | `HASNA_ATTACHMENTS_DB_PATH` | SQLite database override | | `ATTACHMENTS_API_TOKEN`, `HASNA_ATTACHMENTS_API_TOKEN` | Local `/api` authentication | | `HASNA_TODOS_API_KEY`, `TODOS_API_KEY` | Todos API authentication for task integrations | +| `HASNA_TODOS_API_URL`, `TODOS_API_URL` | Trusted non-default Todos API origin for sending todos authentication | | `ATTACHMENTS_MAX_SIZE` | Upload limit override in bytes | | `ATTACHMENTS_TRACK_COSTS` | Enables economy tracking when set | | `ATTACHMENTS_ECONOMY_URL` | Economy URL; defaults to `http://localhost:3460` | diff --git a/scripts/test.sh b/scripts/test.sh index cc5d9c6..d9010cf 100755 --- a/scripts/test.sh +++ b/scripts/test.sh @@ -22,6 +22,8 @@ unset ATTACHMENTS_API_URL unset ATTACHMENTS_API_KEY unset HASNA_TODOS_API_KEY unset TODOS_API_KEY +unset HASNA_TODOS_API_URL +unset TODOS_API_URL PASS=0 FAIL=0 diff --git a/src/cli/commands/complete-task.ts b/src/cli/commands/complete-task.ts index 36edee1..ef2d1de 100644 --- a/src/cli/commands/complete-task.ts +++ b/src/cli/commands/complete-task.ts @@ -76,7 +76,7 @@ export async function completeTaskWithFiles( // 2. Read the current task so we can merge (not clobber) its metadata and honor // optimistic concurrency via its version. - const getResponse = await fetchFn(taskUrl, withTodosAuth()); + const getResponse = await fetchFn(taskUrl, withTodosAuth(taskUrl)); if (!getResponse.ok) { if (getResponse.status === 404) { throw new Error(`Task not found: ${taskId}`); @@ -115,7 +115,7 @@ export async function completeTaskWithFiles( if (typeof task.version === "number") { patchBody.version = task.version; } - const patchResponse = await fetchFn(taskUrl, withTodosAuth({ + const patchResponse = await fetchFn(taskUrl, withTodosAuth(taskUrl, { method: "PATCH", headers: { "Content-Type": "application/json" }, body: JSON.stringify(patchBody), @@ -128,7 +128,8 @@ export async function completeTaskWithFiles( } // 4. Mark the task complete. - const completeResponse = await fetchFn(`${taskUrl}/complete`, withTodosAuth({ + const completeUrl = `${taskUrl}/complete`; + const completeResponse = await fetchFn(completeUrl, withTodosAuth(completeUrl, { method: "POST", headers: { "Content-Type": "application/json" }, body: JSON.stringify({}), diff --git a/src/cli/commands/link-task.ts b/src/cli/commands/link-task.ts index 9ffbfa7..289a458 100644 --- a/src/cli/commands/link-task.ts +++ b/src/cli/commands/link-task.ts @@ -43,7 +43,7 @@ export async function linkAttachmentToTask( }; const url = `${todosUrl}/api/tasks/${taskId}`; - const response = await fetchFn(url, withTodosAuth({ + const response = await fetchFn(url, withTodosAuth(url, { method: "PATCH", headers: { "Content-Type": "application/json" }, body: JSON.stringify({ diff --git a/src/cli/commands/resolve-evidence.ts b/src/cli/commands/resolve-evidence.ts index 6c511e4..4c63e1e 100644 --- a/src/cli/commands/resolve-evidence.ts +++ b/src/cli/commands/resolve-evidence.ts @@ -37,7 +37,7 @@ export async function resolveEvidence( let response: Response; try { - response = await fetchFn(url, withTodosAuth()); + response = await fetchFn(url, withTodosAuth(url)); } catch (err: unknown) { const message = err instanceof Error ? err.message : String(err); throw new Error(`Could not reach todos server at ${todosUrl}: ${message}`); diff --git a/src/cli/commands/task-journal.ts b/src/cli/commands/task-journal.ts index 911c779..034f836 100644 --- a/src/cli/commands/task-journal.ts +++ b/src/cli/commands/task-journal.ts @@ -40,7 +40,8 @@ export async function fetchTaskMeta( fetchFn: typeof fetch = fetch ): Promise { try { - const response = await fetchFn(`${todosUrl}/api/tasks/${taskId}`, withTodosAuth()); + const url = `${todosUrl}/api/tasks/${taskId}`; + const response = await fetchFn(url, withTodosAuth(url)); if (response.status === 404) return null; if (!response.ok) return null; const data = await response.json() as Record; @@ -67,10 +68,8 @@ export async function fetchTaskHistory( fetchFn: typeof fetch = fetch ): Promise { try { - const response = await fetchFn( - `${todosUrl}/api/tasks/${taskId}/history`, - withTodosAuth() - ); + const url = `${todosUrl}/api/tasks/${taskId}/history`; + const response = await fetchFn(url, withTodosAuth(url)); if (!response.ok) return []; const data = await response.json() as unknown; if (!Array.isArray(data)) return []; @@ -265,10 +264,8 @@ export function registerTaskJournal(program: Command): void { if (!todosReachable && journal.attachments.length === 0 && !journal.task.subject) { // Attempt a direct 404 check try { - const response = await fetch( - `${todosUrl}/api/tasks/${taskId}`, - withTodosAuth() - ); + const url = `${todosUrl}/api/tasks/${taskId}`; + const response = await fetch(url, withTodosAuth(url)); if (response.status === 404) { process.stderr.write(`Error: Task not found: ${taskId}\n`); process.exit(1); diff --git a/src/cli/commands/watch.ts b/src/cli/commands/watch.ts index fc9170d..5bdb22e 100644 --- a/src/cli/commands/watch.ts +++ b/src/cli/commands/watch.ts @@ -147,7 +147,7 @@ export async function connectAndWatch( process.stdout.write(`[watch] Connecting to ${url}\n`); } - const response = await fetchFn(url, withTodosAuth({ signal })); + const response = await fetchFn(url, withTodosAuth(url, { signal })); if (!response.ok) { throw new Error(`HTTP ${response.status}`); diff --git a/src/core/todos.test.ts b/src/core/todos.test.ts index 1a0d7a1..63f3cb4 100644 --- a/src/core/todos.test.ts +++ b/src/core/todos.test.ts @@ -1,23 +1,30 @@ -import { afterEach, describe, expect, it } from "bun:test"; +import { afterEach, beforeEach, describe, expect, it } from "bun:test"; import { withTodosAuth } from "./todos"; -afterEach(() => { +function clearTodosEnv() { delete process.env.HASNA_TODOS_API_KEY; delete process.env.TODOS_API_KEY; -}); + delete process.env.HASNA_TODOS_API_URL; + delete process.env.TODOS_API_URL; +} + +beforeEach(clearTodosEnv); +afterEach(clearTodosEnv); describe("withTodosAuth", () => { it("returns the original request init when no API key is configured", () => { const init = { method: "POST" }; - expect(withTodosAuth(init)).toBe(init); + expect(withTodosAuth("http://localhost:3000/api/tasks/TASK-001", init)).toBe(init); expect(withTodosAuth()).toBeUndefined(); }); - it("uses TODOS_API_KEY as an x-api-key header", () => { + it("uses TODOS_API_KEY as an x-api-key header for the default todos origin", () => { process.env.TODOS_API_KEY = "todos-fallback-key"; - const init = withTodosAuth({ headers: { "Content-Type": "application/json" } }); + const init = withTodosAuth("http://localhost:3000/api/tasks/TASK-001", { + headers: { "Content-Type": "application/json" }, + }); const headers = new Headers(init?.headers); expect(headers.get("x-api-key")).toBe("todos-fallback-key"); @@ -28,7 +35,7 @@ describe("withTodosAuth", () => { process.env.HASNA_TODOS_API_KEY = "hasna-key"; process.env.TODOS_API_KEY = "fallback-key"; - const headers = new Headers(withTodosAuth()?.headers); + const headers = new Headers(withTodosAuth("http://localhost:3000/api/tasks/TASK-001")?.headers); expect(headers.get("x-api-key")).toBe("hasna-key"); }); @@ -37,8 +44,24 @@ describe("withTodosAuth", () => { process.env.HASNA_TODOS_API_KEY = ""; process.env.TODOS_API_KEY = "fallback-key"; - const headers = new Headers(withTodosAuth()?.headers); + const headers = new Headers(withTodosAuth("http://localhost:3000/api/tasks/TASK-001")?.headers); expect(headers.get("x-api-key")).toBe("fallback-key"); }); + + it("does not forward the API key to an arbitrary override origin", () => { + process.env.HASNA_TODOS_API_KEY = "hasna-key"; + const init = { method: "GET" }; + + expect(withTodosAuth("https://example.invalid/api/tasks/TASK-001", init)).toBe(init); + }); + + it("allows a remote origin only when it is explicitly configured", () => { + process.env.HASNA_TODOS_API_URL = "https://todos.example.com"; + process.env.HASNA_TODOS_API_KEY = "hasna-key"; + + const headers = new Headers(withTodosAuth("https://todos.example.com/api/tasks/TASK-001")?.headers); + + expect(headers.get("x-api-key")).toBe("hasna-key"); + }); }); diff --git a/src/core/todos.ts b/src/core/todos.ts index 90dbb75..5c916a0 100644 --- a/src/core/todos.ts +++ b/src/core/todos.ts @@ -1,7 +1,33 @@ -export function withTodosAuth(init?: RequestInit): RequestInit | undefined { +const DEFAULT_TODOS_ORIGIN = new URL("http://localhost:3000").origin; + +function parseOrigin(url: string | URL | undefined): string | null { + if (!url) return null; + try { + return new URL(url).origin; + } catch { + return null; + } +} + +function trustedTodosOrigins(): Set { + const origins = new Set([DEFAULT_TODOS_ORIGIN]); + for (const value of [process.env.HASNA_TODOS_API_URL, process.env.TODOS_API_URL]) { + const origin = parseOrigin(value); + if (origin) origins.add(origin); + } + return origins; +} + +export function withTodosAuth( + requestUrl?: string | URL, + init?: RequestInit +): RequestInit | undefined { const apiKey = process.env.HASNA_TODOS_API_KEY || process.env.TODOS_API_KEY; if (!apiKey) return init; + const requestOrigin = parseOrigin(requestUrl); + if (!requestOrigin || !trustedTodosOrigins().has(requestOrigin)) return init; + const headers = new Headers(init?.headers); headers.set("x-api-key", apiKey);