diff --git a/doc/DEVELOPING.md b/doc/DEVELOPING.md index 816536af5ca..1023f2f1414 100644 --- a/doc/DEVELOPING.md +++ b/doc/DEVELOPING.md @@ -1141,6 +1141,15 @@ orphan recovery. pnpm exec vitest run server/src/services/heartbeat/agent-status.test.ts server/src/__tests__/heartbeat-agent-status-recovery.test.ts ``` +## Task document bodies + +The native `write_document` tool preserves the supplied Markdown body, including +leading whitespace, hard-break spaces, line endings, and final newlines. It still +requires a nonblank body and normalizes key/title fields. `write_task_document` +and REST document PUT also retain surrounding whitespace. Those REST-backed +paths keep their existing escaped-line-break normalization: a body with no real +line breaks can have literal `\n` sequences converted to newlines. + ## Test Commands The [feature map](../maps/feature-map/README.md) is an optional reference for user diff --git a/server/src/services/native-runtime/paperclip-runner-tool-authority.ts b/server/src/services/native-runtime/paperclip-runner-tool-authority.ts index 7a718df59be..11fe6b14140 100644 --- a/server/src/services/native-runtime/paperclip-runner-tool-authority.ts +++ b/server/src/services/native-runtime/paperclip-runner-tool-authority.ts @@ -1907,7 +1907,7 @@ export class PaperclipRunnerToolAuthority { key: requiredString(input.key), title: requiredString(input.title), format: "markdown", - body: requiredString(input.body), + body: requiredUntrimmedString(input.body), baseRevisionId: nullableProviderId(input.baseRevisionId), changeSummary: input.changeSummary === null || input.changeSummary === undefined ? null @@ -3324,8 +3324,12 @@ export class PaperclipRunnerToolAuthority { } function requiredString(value: unknown): string { + return requiredUntrimmedString(value).trim(); +} + +function requiredUntrimmedString(value: unknown): string { if (typeof value !== "string" || value.trim() === "") throw new Error("paperclip_runner_tool_input_invalid"); - return value.trim(); + return value; } /** diff --git a/server/src/services/native-runtime/task-document-body.integration.test.ts b/server/src/services/native-runtime/task-document-body.integration.test.ts new file mode 100644 index 00000000000..fe4cc41da74 --- /dev/null +++ b/server/src/services/native-runtime/task-document-body.integration.test.ts @@ -0,0 +1,73 @@ +import { randomUUID } from "node:crypto"; +import { afterAll, beforeAll, expect, it } from "vitest"; +import { startRunnerApiTestServer } from "../../__tests__/helpers/runner-api-server.js"; +import { createLocalAgentJwt } from "../../agent-auth-jwt.js"; +import { documentService } from "../documents.js"; + +let server: Awaited>; +const previousSecret = process.env.PAPERCLIP_AGENT_JWT_SECRET; +beforeAll(async () => { + process.env.PAPERCLIP_AGENT_JWT_SECRET = randomUUID(); + server = await startRunnerApiTestServer(); +}, 60_000); +afterAll(async () => { + await server?.close(); + if (previousSecret === undefined) delete process.env.PAPERCLIP_AGENT_JWT_SECRET; + else process.env.PAPERCLIP_AGENT_JWT_SECRET = previousSecret; +}); + +const bodies = [ + "# Final newline\n", + " Leading whitespace\n\nA Markdown hard break \nnext line\n\n", + "No final newline", + "\t日本語 café 🧭\r\n\r\nSecond line \r\n\n", +]; + +it.each(["write_document", "write_task_document", "rest"] as const)( + "%s preserves exact Markdown on create, update, and read-back", async surface => { + const fixture = await server.fixture({ disableWakeOnDemand: true }); + const issueId = surface === "write_task_document" ? fixture.blockerId : fixture.issueId; + const service = documentService(server.db); + const token = createLocalAgentJwt(fixture.agentId, fixture.companyId, "paperclip_runner", fixture.runId); + const headers = { Authorization: `Bearer ${token}`, "Content-Type": "application/json" }; + for (const [index, body] of bodies.entries()) { + const key = `exact-body-${index}`; + let baseRevisionId: string | null = null; + for (const phase of ["create", "update"] as const) { + const supplied = phase === "create" ? body : `Updated\n${body}`; + const args = { key, title: " Exact body ", body: supplied, baseRevisionId }; + const call = { tool: surface, callId: randomUUID(), arguments: surface === "write_document" + ? { ...args, idempotencyKey: randomUUID() } : { ...args, taskId: issueId } }; + if (surface === "rest") { + const response = await fetch(`${server.apiUrl}/api/issues/${issueId}/documents/${key}`, { + method: "PUT", headers, body: JSON.stringify({ ...args, format: "markdown" }), + }); + expect(response.status).toBe(phase === "create" ? 201 : 200); + expect((await response.json()).body).toBe(supplied); + } else { + const result = await fixture.authority.execute(call); + expect(result).toMatchObject(surface === "write_document" + ? { document: { body: supplied, title: "Exact body" } } : { ok: true, status: phase === "create" ? 201 : 200 }); + expect(await fixture.authority.execute({ ...call, callId: surface === "write_document" ? randomUUID() : call.callId })).toEqual(result); + } + const saved = await service.getIssueDocumentByKey(issueId, key); + expect(saved?.body).toBe(supplied); + expect(saved?.latestRevisionNumber).toBe(phase === "create" ? 1 : 2); + const read = await fixture.authority.execute({ tool: surface === "write_task_document" ? "read_task_document" : "read_document", + callId: randomUUID(), arguments: surface === "write_task_document" ? { taskId: issueId, key } : { key } }) as { document?: { body: string }; data?: { body: string } }; + expect(surface === "write_task_document" ? read.data?.body : read.document?.body).toBe(supplied); + baseRevisionId = saved!.latestRevisionId; + } + } + }, 30_000, +); + +it("retains required body validation for write_document", async () => { + const fixture = await server.fixture({ disableWakeOnDemand: true }); + for (const body of [undefined, null, 42, "", " \n\t"]) { + await expect(fixture.authority.execute({ tool: "write_document", callId: randomUUID(), arguments: { + key: "invalid-body", title: "Invalid body", body, baseRevisionId: null, idempotencyKey: randomUUID(), + } })).rejects.toThrow("paperclip_runner_tool_input_invalid"); + } + expect(await documentService(server.db).getIssueDocumentByKey(fixture.issueId, "invalid-body")).toBeNull(); +}); diff --git a/tests/runner-e2e/evidence.ts b/tests/runner-e2e/evidence.ts index 3e138d5d69d..8b6ea803550 100644 --- a/tests/runner-e2e/evidence.ts +++ b/tests/runner-e2e/evidence.ts @@ -145,16 +145,16 @@ export async function inspectZip(source: string, secrets: readonly string[]) { let leak: string | null = null; let spawnError: Error | null = null; let expanded = 0; - const timer = setTimeout(() => { leak = "zip inspection exceeded its time bound"; unzip.kill(); }, 30_000); + const timer = setTimeout(() => { leak = "zip inspection exceeded its time bound"; unzip.kill("SIGKILL"); }, 30_000); unzip.stdout.on("data", (chunk: Buffer) => { if (leak) return; expanded += chunk.length; - if (expanded > 256 * 1024 * 1024) { leak = "zip inspection exceeded its expanded-byte bound"; unzip.kill(); return; } + if (expanded > 256 * 1024 * 1024) { leak = "zip inspection exceeded its expanded-byte bound"; unzip.kill("SIGKILL"); return; } const data = Buffer.concat([carry, chunk]); leak = findSecretLeak(data, secrets, { includeShapes: false }); carry = data.subarray(Math.max(0, data.length - overlap)); - if (leak) unzip.kill(); + if (leak) unzip.kill("SIGKILL"); }); // Drain diagnostics so a noisy unzip cannot block. Error text is withheld // because archive paths and contents belong to private attempt evidence. diff --git a/tests/runner-e2e/zip-inspection-shutdown.test.ts b/tests/runner-e2e/zip-inspection-shutdown.test.ts new file mode 100644 index 00000000000..720b86a07fc --- /dev/null +++ b/tests/runner-e2e/zip-inspection-shutdown.test.ts @@ -0,0 +1,42 @@ +import { chmod, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { expect, it, vi } from "vitest"; +import { inspectZip } from "./evidence.js"; + +it.skipIf(process.platform === "win32")("finishes a rejected ZIP scan even when the scanner ignores SIGTERM", async () => { + const root = await mkdtemp(path.join(os.tmpdir(), "zip-shutdown-")); + const source = path.join(root, "report.zip"); + const executable = path.join(root, "unzip"); + const secret = "synthetic-zip-secret-928472"; + await writeFile(executable, `#!${process.execPath} +const fs = require('node:fs'); +process.on('SIGTERM', () => {}); +fs.writeFileSync(process.argv[3] + '.pid', String(process.pid)); +process.stdout.write(${JSON.stringify(secret)}); +setInterval(() => {}, 1000); +`); + await chmod(executable, 0o700); + vi.stubEnv("PATH", `${root}${path.delimiter}${process.env.PATH ?? ""}`); + let timer: ReturnType | undefined; + let stopped = false; + const scan = inspectZip(source, [secret]).then(result => { stopped = true; return result; }); + try { + const result = await Promise.race([ + scan, + new Promise(resolve => { timer = setTimeout(() => resolve("scanner did not stop"), 2000); }), + ]); + expect(result).toBe("exact secret value"); + } finally { + clearTimeout(timer); + vi.unstubAllEnvs(); + const pid = Number(await readFile(`${source}.pid`, "utf8").catch(() => "0")); + if (!stopped && pid > 0) { + try { process.kill(pid, "SIGKILL"); } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "ESRCH") throw error; + } + } + await scan; + await rm(root, { recursive: true, force: true }); + } +});