fix: sanitize agent/run/company labels to RFC 1123 (N4)
Co-Authored-By: Claude Sonnet <noreply@anthropic.com> Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
@@ -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
|
||||
|
||||
+12
-1
@@ -541,6 +541,17 @@ export async function execute(ctx: AdapterExecutionContext): Promise<AdapterExec
|
||||
// the current runId. When reattachOrphanedJobs is enabled and the orphan matches
|
||||
// the current agent+task+session, we attach to it instead of deleting it (FAR-124).
|
||||
const agentId = ctx.agent.id;
|
||||
const sanitizedAgentId = sanitizeLabelValue(agentId);
|
||||
if (!sanitizedAgentId) {
|
||||
await onLog("stderr", `[paperclip] Cannot create K8s Job: agent.id "${agentId}" produces no valid RFC 1123 label characters\n`);
|
||||
return {
|
||||
exitCode: null,
|
||||
signal: null,
|
||||
timedOut: false,
|
||||
errorMessage: `Agent ID "${agentId}" cannot be sanitized to a valid Kubernetes label`,
|
||||
errorCode: "k8s_agent_id_invalid",
|
||||
};
|
||||
}
|
||||
const selfPod = await getSelfPodInfo(kubeconfigPath);
|
||||
const guardNamespace = asString(config.namespace, "") || selfPod.namespace;
|
||||
const reattachOrphanedJobs = asBoolean(config.reattachOrphanedJobs, true);
|
||||
@@ -554,7 +565,7 @@ export async function execute(ctx: AdapterExecutionContext): Promise<AdapterExec
|
||||
const batchApi = getBatchApi(kubeconfigPath);
|
||||
const existing = await batchApi.listNamespacedJob({
|
||||
namespace: guardNamespace,
|
||||
labelSelector: `paperclip.io/agent-id=${agentId},paperclip.io/adapter-type=claude_k8s`,
|
||||
labelSelector: `paperclip.io/agent-id=${sanitizedAgentId},paperclip.io/adapter-type=claude_k8s`,
|
||||
});
|
||||
const running = existing.items.filter(
|
||||
(j) => !j.status?.conditions?.some((c) => (c.type === "Complete" || c.type === "Failed") && c.status === "True"),
|
||||
|
||||
@@ -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 });
|
||||
|
||||
@@ -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<string, string> = {
|
||||
"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);
|
||||
|
||||
Reference in New Issue
Block a user