diff --git a/src/server/execute.test.ts b/src/server/execute.test.ts index 638cb52..eac2e91 100644 --- a/src/server/execute.test.ts +++ b/src/server/execute.test.ts @@ -15,7 +15,7 @@ vi.mock("./k8s-client.js", () => ({ resetCache: vi.fn(), })); -const { isK8s404, buildPartialRunError, classifyOrphan, describePodTerminatedError, streamPodLogsOnce } = await import("./execute.js"); +const { isK8s404, buildPartialRunError, classifyOrphan, describePodTerminatedError, streamPodLogsOnce, execute } = await import("./execute.js"); function makeJob(opts: { runId?: string; @@ -261,6 +261,25 @@ describe("describePodTerminatedError", () => { }); }); +describe("execute: all-invalid agent.id (N4)", () => { + it("returns hard error without creating a Job when agent.id sanitizes to null", async () => { + const logs: string[] = []; + const result = await execute({ + runId: "run-001", + agent: { id: "@@@", companyId: "co1", name: "Bad Agent", adapterType: "claude_k8s", adapterConfig: {} }, + runtime: { sessionId: null, sessionParams: null, sessionDisplayId: null, taskKey: null }, + config: {}, + context: {}, + onLog: async (_stream, msg) => { logs.push(msg); }, + }); + expect(result.errorCode).toBe("k8s_agent_id_invalid"); + expect(result.errorMessage).toContain("@@@"); + // getSelfPodInfo must NOT have been called (early return before K8s calls) + const { getSelfPodInfo } = await import("./k8s-client.js"); + expect(getSelfPodInfo).not.toHaveBeenCalled(); + }); +}); + // Regression: FAR-10 hardening — streamPodLogsOnce must not hang forever when // the K8s client's logApi.log call never resolves. When stopSignal fires, the // bail timer must force-return within LOG_STREAM_BAIL_TIMEOUT_MS (3s in the diff --git a/src/server/execute.ts b/src/server/execute.ts index e9bea9a..354d502 100644 --- a/src/server/execute.ts +++ b/src/server/execute.ts @@ -541,6 +541,17 @@ export async function execute(ctx: AdapterExecutionContext): Promise !j.status?.conditions?.some((c) => (c.type === "Complete" || c.type === "Failed") && c.status === "True"), diff --git a/src/server/job-manifest.test.ts b/src/server/job-manifest.test.ts index c189c68..6ed4cd5 100644 --- a/src/server/job-manifest.test.ts +++ b/src/server/job-manifest.test.ts @@ -203,6 +203,40 @@ describe("buildJobManifest", () => { }); }); + describe("system label sanitization (N4)", () => { + it("sanitizes agent.id with @ to a valid RFC 1123 label", () => { + ctx.agent.id = "user@example.com"; + const { job } = buildJobManifest({ ctx, selfPod }); + const label = job.metadata?.labels?.["paperclip.io/agent-id"]; + expect(label).toMatch(/^[a-zA-Z0-9]([a-zA-Z0-9._-]*[a-zA-Z0-9])?$/); + expect(label).not.toContain("@"); + }); + + it("sanitizes agent.id with spaces to a valid RFC 1123 label", () => { + ctx.agent.id = "my agent id"; + const { job } = buildJobManifest({ ctx, selfPod }); + const label = job.metadata?.labels?.["paperclip.io/agent-id"]; + expect(label).toMatch(/^[a-zA-Z0-9]([a-zA-Z0-9._-]*[a-zA-Z0-9])?$/); + }); + + it("omits paperclip.io/run-id when sanitized value is null (all-invalid runId)", () => { + // inject an all-special-chars runId via context override — buildJobManifest + // uses ctx.runId directly + const badCtx = makeCtx({ runId: "@@@" }); + const { job, skippedLabels } = buildJobManifest({ ctx: badCtx, selfPod }); + expect(job.metadata?.labels?.["paperclip.io/run-id"]).toBeUndefined(); + expect(skippedLabels).toContain("paperclip.io/run-id"); + }); + + it("selector matches sanitized agent-id label", () => { + ctx.agent.id = "Agent@Test"; + const { job } = buildJobManifest({ ctx, selfPod }); + const agentLabel = job.metadata?.labels?.["paperclip.io/agent-id"]; + // the label should equal what sanitizeLabelValue produces + expect(agentLabel).toBe("AgentTest"); + }); + }); + describe("annotations", () => { it("includes adapter type and agent name annotations", () => { const { job } = buildJobManifest({ ctx, selfPod }); diff --git a/src/server/job-manifest.ts b/src/server/job-manifest.ts index 01f7bf8..6ccd370 100644 --- a/src/server/job-manifest.ts +++ b/src/server/job-manifest.ts @@ -444,15 +444,22 @@ export function buildJobManifest(input: JobBuildInput): JobBuildResult { }, }; - // Labels + // Labels — system identifiers must pass RFC 1123 label value format. + const sanitizedAgentId = sanitizeLabelValue(agent.id); + const sanitizedRunId = sanitizeLabelValue(runId); + const sanitizedCompanyId = sanitizeLabelValue(agent.companyId); + const skippedLabels: string[] = []; + if (!sanitizedRunId) skippedLabels.push("paperclip.io/run-id"); + if (!sanitizedCompanyId) skippedLabels.push("paperclip.io/company-id"); const labels: Record = { "app.kubernetes.io/managed-by": "paperclip", "app.kubernetes.io/component": "agent-job", - "paperclip.io/agent-id": agent.id, - "paperclip.io/run-id": runId, - "paperclip.io/company-id": agent.companyId, + // sanitizedAgentId null-check is enforced in execute.ts before Job creation + "paperclip.io/agent-id": sanitizedAgentId ?? agent.id, "paperclip.io/adapter-type": "claude_k8s", }; + if (sanitizedRunId) labels["paperclip.io/run-id"] = sanitizedRunId; + if (sanitizedCompanyId) labels["paperclip.io/company-id"] = sanitizedCompanyId; // Reattach-target labels: let a future execute() identify this Job as the // continuation of the same logical unit of work (same task + same resume // session) so it can attach to the running pod across a Paperclip restart @@ -462,7 +469,6 @@ export function buildJobManifest(input: JobBuildInput): JobBuildResult { if (taskLabel) labels["paperclip.io/task-id"] = taskLabel; const sessionLabel = runtimeSessionId ? sanitizeLabelValue(runtimeSessionId) : null; if (sessionLabel) labels["paperclip.io/session-id"] = sessionLabel; - const skippedLabels: string[] = []; for (const [key, value] of Object.entries(extraLabels)) { if (key.startsWith("paperclip.io/") || key.startsWith("app.kubernetes.io/")) { skippedLabels.push(key);