Skip to content

Commit dd9d611

Browse files
qingminglongqingminglong
committed
fix(cron): sanitize command delivery summaries
1 parent 8cce141 commit dd9d611

3 files changed

Lines changed: 177 additions & 13 deletions

File tree

‎src/gateway/server-cron-notifications.ts‎

Lines changed: 40 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,35 @@ function buildCronFailureWebhookPayload(params: { evt: CronEvent; job: CronJob }
105105
};
106106
}
107107

108+
function buildDeliverySafeDiagnostics(evt: CronEvent, webhookSummary: string) {
109+
if (!evt.diagnostics) {
110+
return undefined;
111+
}
112+
return {
113+
...evt.diagnostics,
114+
summary: webhookSummary,
115+
entries: evt.diagnostics.entries.map((entry) => ({
116+
...entry,
117+
message:
118+
entry.source === "exec"
119+
? "command result details summarized for external delivery"
120+
: typeof evt.summary === "string" && entry.message.includes(evt.summary)
121+
? entry.message.replaceAll(evt.summary, webhookSummary)
122+
: entry.message,
123+
})),
124+
};
125+
}
126+
127+
function buildDeliverySafeJob(job: CronJob): CronJob {
128+
const state = { ...job.state };
129+
delete state.lastDiagnostics;
130+
delete state.lastDiagnosticSummary;
131+
return {
132+
...job,
133+
state,
134+
};
135+
}
136+
108137
function buildCronFinishedWebhookPayload(evt: CronEvent) {
109138
const webhookSummary = typeof evt.deliverySummary === "string" ? evt.deliverySummary : undefined;
110139
const {
@@ -113,27 +142,26 @@ function buildCronFinishedWebhookPayload(evt: CronEvent) {
113142
diagnostics: _diagnostics,
114143
...payload
115144
} = evt;
145+
if (evt.status === "error") {
146+
return evt.job
147+
? {
148+
...payload,
149+
job: buildDeliverySafeJob(evt.job),
150+
}
151+
: payload;
152+
}
116153
const sanitizedPayload =
117154
webhookSummary === undefined
118155
? evt
119156
: {
120157
...payload,
121158
summary: webhookSummary,
122-
diagnostics: evt.diagnostics ? { summary: webhookSummary, entries: [] } : undefined,
159+
diagnostics: buildDeliverySafeDiagnostics(evt, webhookSummary),
123160
};
124-
if (evt.status !== "error") {
125-
return sanitizedPayload;
126-
}
127-
if (evt.job) {
128-
const state = { ...evt.job.state };
129-
delete state.lastDiagnostics;
130-
delete state.lastDiagnosticSummary;
161+
if (evt.job && webhookSummary !== undefined) {
131162
return {
132163
...sanitizedPayload,
133-
job: {
134-
...evt.job,
135-
state,
136-
},
164+
job: buildDeliverySafeJob(evt.job),
137165
};
138166
}
139167
return sanitizedPayload;

‎src/gateway/server-cron.test.ts‎

Lines changed: 134 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -631,13 +631,26 @@ describe("buildGatewayCronService", () => {
631631
);
632632
const init = requireRecord(request.init, "fetch init");
633633
const body = JSON.parse(String(init.body)) as {
634-
diagnostics?: { summary?: string };
634+
diagnostics?: {
635+
entries?: Array<{ exitCode?: number | null; message?: string; truncated?: boolean }>;
636+
summary?: string;
637+
};
638+
job?: { state?: { lastDiagnostics?: unknown; lastDiagnosticSummary?: unknown } };
635639
summary?: string;
636640
};
637641
expect(body.summary).toContain("tail marker");
638642
expect(body.summary).not.toContain(promptUrl);
639643
expect(body.diagnostics?.summary).toContain("tail marker");
640644
expect(body.diagnostics?.summary).not.toContain(promptUrl);
645+
expect(body.diagnostics?.entries?.length).toBeGreaterThan(0);
646+
expect(body.diagnostics?.entries?.[0]?.message).toBe(
647+
"command result details summarized for external delivery",
648+
);
649+
expect(body.diagnostics?.entries?.[0]?.message).not.toContain(promptUrl);
650+
expect(body.diagnostics?.entries?.[0]?.exitCode).toBe(0);
651+
expect(body.diagnostics?.entries?.[0]?.truncated).toBe(true);
652+
expect(body.job?.state?.lastDiagnostics).toBeUndefined();
653+
expect(body.job?.state?.lastDiagnosticSummary).toBeUndefined();
641654

642655
const storedJob = state.cron.getJob(job.id);
643656
expect(storedJob?.state.lastDiagnosticSummary).toContain(promptUrl);
@@ -647,6 +660,126 @@ describe("buildGatewayCronService", () => {
647660
}
648661
});
649662

663+
it("omits failed command summaries from completion webhooks", async () => {
664+
const cfg = createCronConfig("server-cron-command-webhook-error");
665+
loadConfigMock.mockReturnValue(cfg);
666+
fetchWithSsrFGuardMock.mockResolvedValue({
667+
release: vi.fn(async () => {}),
668+
});
669+
670+
const state = buildGatewayCronService({
671+
cfg,
672+
deps: {} as CliDeps,
673+
broadcast: () => {},
674+
});
675+
try {
676+
const promptUrl = "https://login.microsoft.com/device";
677+
const script = [
678+
`process.stdout.write('Open ${promptUrl} and enter the code ERROR-CODE-270.\\n');`,
679+
"for (let i = 0; i < 80; i += 1) process.stdout.write(`noise-${i}-xxxxxxxxxxxxxxxxxxxxxxxx\\n`);",
680+
"process.stdout.write('failed tail marker\\n');",
681+
"process.exit(2);",
682+
].join("");
683+
const job = await state.cron.add({
684+
name: "webhook-error-command",
685+
enabled: true,
686+
deleteAfterRun: false,
687+
schedule: { kind: "at", at: new Date(1).toISOString() },
688+
sessionTarget: "isolated",
689+
wakeMode: "next-heartbeat",
690+
payload: {
691+
kind: "command",
692+
argv: [process.execPath, "-e", script],
693+
outputMaxBytes: 80,
694+
},
695+
delivery: {
696+
mode: "webhook",
697+
to: "https://example.invalid/cron-finished",
698+
},
699+
});
700+
701+
await state.cron.run(job.id, "force");
702+
703+
expect(fetchWithSsrFGuardMock).toHaveBeenCalledTimes(1);
704+
const request = requireRecord(
705+
callArg(fetchWithSsrFGuardMock, 0, 0, "fetch request"),
706+
"fetch request",
707+
);
708+
const init = requireRecord(request.init, "fetch init");
709+
const body = JSON.parse(String(init.body)) as {
710+
diagnostics?: unknown;
711+
job?: { state?: { lastDiagnostics?: unknown; lastDiagnosticSummary?: unknown } };
712+
summary?: unknown;
713+
};
714+
expect(body.summary).toBeUndefined();
715+
expect(body.diagnostics).toBeUndefined();
716+
expect(body.job?.state?.lastDiagnostics).toBeUndefined();
717+
expect(body.job?.state?.lastDiagnosticSummary).toBeUndefined();
718+
719+
const storedJob = state.cron.getJob(job.id);
720+
expect(storedJob?.state.lastRunStatus).toBe("error");
721+
expect(storedJob?.state.lastDiagnosticSummary).toContain(promptUrl);
722+
expect(storedJob?.state.lastDiagnosticSummary).toContain("ERROR-CODE-270");
723+
} finally {
724+
state.cron.stop();
725+
}
726+
});
727+
728+
it("uses delivery summary for cron_changed command hooks", async () => {
729+
const cfg = createCronConfig("server-cron-command-hook-tail");
730+
loadConfigMock.mockReturnValue(cfg);
731+
732+
const state = buildGatewayCronService({
733+
cfg,
734+
deps: {} as CliDeps,
735+
broadcast: () => {},
736+
});
737+
try {
738+
const promptUrl = "https://login.microsoft.com/device";
739+
const script = [
740+
`process.stdout.write('Open ${promptUrl} and enter the code HOOK-CODE-270.\\n');`,
741+
"for (let i = 0; i < 80; i += 1) process.stdout.write(`noise-${i}-xxxxxxxxxxxxxxxxxxxxxxxx\\n`);",
742+
"process.stdout.write('hook tail marker\\n');",
743+
].join("");
744+
const job = await state.cron.add({
745+
name: "hook-tail-only-command",
746+
enabled: true,
747+
deleteAfterRun: false,
748+
schedule: { kind: "at", at: new Date(1).toISOString() },
749+
sessionTarget: "isolated",
750+
wakeMode: "next-heartbeat",
751+
payload: {
752+
kind: "command",
753+
argv: [process.execPath, "-e", script],
754+
outputMaxBytes: 80,
755+
},
756+
delivery: {
757+
mode: "none",
758+
},
759+
});
760+
761+
runCronChangedMock.mockClear();
762+
await state.cron.run(job.id, "force");
763+
764+
const cronChangedCalls = runCronChangedMock.mock.calls as unknown[][];
765+
const finishedCall = cronChangedCalls.find(([evt]) => {
766+
const event = requireRecord(evt, "cron_changed event");
767+
return event.action === "finished";
768+
});
769+
if (!finishedCall) {
770+
throw new Error("expected finished cron_changed event");
771+
}
772+
const event = requireRecord(finishedCall[0], "cron_changed event");
773+
expect(event.summary).toContain("hook tail marker");
774+
expect(event.summary).not.toContain(promptUrl);
775+
776+
const storedJob = state.cron.getJob(job.id);
777+
expect(storedJob?.state.lastDiagnosticSummary).toContain(promptUrl);
778+
} finally {
779+
state.cron.stop();
780+
}
781+
});
782+
650783
it("routes global-scope main cron jobs through the global queue for queued wakes", async () => {
651784
const cfg = {
652785
...createCronConfig("server-cron-global-queued"),

‎src/gateway/server-cron.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -619,6 +619,9 @@ export function buildGatewayCronService(params: {
619619
"provider",
620620
]),
621621
};
622+
if (typeof evt.deliverySummary === "string") {
623+
hookEvt.summary = evt.deliverySummary;
624+
}
622625
runCronChangedHook(hookEvt);
623626
if (evt.action === "finished") {
624627
const job = evt.job ?? cron.getJob(evt.jobId);

0 commit comments

Comments
 (0)