Skip to content

Commit b7c0d72

Browse files
eleqtriziteleqtrizit
committed
fix(cron): redact command output in delivery
1 parent 4ec006d commit b7c0d72

5 files changed

Lines changed: 125 additions & 3 deletions

File tree

‎src/cron/delivery-redaction.ts‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
import { redactToolPayloadText } from "../logging/redact.js";
2+
3+
const CRON_DEVICE_AUTH_REDACTION = "[redacted device authorization output]";
4+
const DEVICE_AUTH_URL_RE =
5+
/\bhttps?:\/\/[^\s<>"']*(?:microsoft\.com\/devicelogin|aka\.ms\/devicelogin|\/oauth2?\/device|\/device(?:code|login)?\b|device[-_]?code|device[-_]?login)[^\s<>"']*/iu;
6+
const DEVICE_AUTH_CODE_LINE_RE =
7+
/\b(?:user[-_\s]?code|device[-_\s]?code|verification[-_\s]?(?:uri|url)|enter\s+(?:the\s+)?code)\b.*\b[A-Z0-9]{4,12}(?:-[A-Z0-9]{2,12}){0,4}\b/iu;
8+
9+
function redactActionRequiredLine(line: string): string {
10+
if (!line.trim()) {
11+
return line;
12+
}
13+
if (DEVICE_AUTH_URL_RE.test(line) || DEVICE_AUTH_CODE_LINE_RE.test(line)) {
14+
return CRON_DEVICE_AUTH_REDACTION;
15+
}
16+
return line;
17+
}
18+
19+
function redactActionRequiredLines(text: string): string {
20+
return text
21+
.split(/(\r\n|\r|\n)/u)
22+
.map((part) => (/^(?:\r\n|\r|\n)$/u.test(part) ? part : redactActionRequiredLine(part)))
23+
.join("");
24+
}
25+
26+
/** Redacts command output before it leaves cron's non-interactive delivery boundary. */
27+
export function redactCronDeliveryText(text: string): string {
28+
const secretRedacted = redactToolPayloadText(text);
29+
return redactActionRequiredLines(secretRedacted);
30+
}

‎src/cron/delivery.failure-notify.test.ts‎

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,8 @@ vi.mock("../logging.js", () => ({
3737
})),
3838
}));
3939

40-
const { sendFailureNotificationAnnounce } = await import("./delivery.js");
40+
const { sendCronAnnouncePayloadStrict, sendFailureNotificationAnnounce } =
41+
await import("./delivery.js");
4142

4243
type DeliveryRequest = {
4344
abortSignal?: unknown;
@@ -125,6 +126,35 @@ describe("sendFailureNotificationAnnounce", () => {
125126
expect(deliveryRequest.abortSignal).toBeInstanceOf(AbortSignal);
126127
});
127128

129+
it("redacts secrets and device authorization prompts before primary announce delivery", async () => {
130+
await sendCronAnnouncePayloadStrict({
131+
deps: {} as never,
132+
cfg: {} as never,
133+
agentId: "main",
134+
jobId: "job-1",
135+
target: { channel: "telegram", to: "123" },
136+
message: [
137+
"stdout:",
138+
"Open https://microsoft.com/devicelogin and enter code ABCD-EFGH",
139+
"OPENAI_API_KEY=sk-1234567890abcdef",
140+
"ordinary status",
141+
].join("\n"),
142+
abortSignal: new AbortController().signal,
143+
});
144+
145+
const deliveryRequest = firstDeliveryRequest();
146+
expect(deliveryRequest.payloads).toEqual([
147+
{
148+
text: [
149+
"stdout:",
150+
"[redacted device authorization output]",
151+
"OPENAI_API_KEY=sk-123…cdef",
152+
"ordinary status",
153+
].join("\n"),
154+
},
155+
]);
156+
});
157+
128158
it("uses sessionKey for delivery-target resolution and outbound context", async () => {
129159
await sendFailureNotificationAnnounce(
130160
{} as never,

‎src/cron/delivery.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
type CronDeliveryPlan,
1515
resolveCronDeliveryPlan,
1616
} from "./delivery-plan.js";
17+
import { redactCronDeliveryText } from "./delivery-redaction.js";
1718
import {
1819
resolveDeliveryTarget,
1920
type DeliveryTargetResolution,
@@ -114,7 +115,7 @@ async function deliverCronAnnouncePayload(params: {
114115
to: params.delivery.resolvedTarget.to,
115116
accountId: params.delivery.resolvedTarget.accountId,
116117
threadId: params.delivery.resolvedTarget.threadId,
117-
payloads: [{ text: params.message }],
118+
payloads: [{ text: redactCronDeliveryText(params.message) }],
118119
session: params.delivery.session,
119120
identity: params.delivery.identity,
120121
bestEffort: false,

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
import type { CliDeps } from "../cli/deps.types.js";
88
import type { CronFailureDestinationConfig } from "../config/types.cron.js";
99
import type { OpenClawConfig } from "../config/types.openclaw.js";
10+
import { redactCronDeliveryText } from "../cron/delivery-redaction.js";
1011
import {
1112
resolveCronDeliveryPlan,
1213
resolveFailureDestination,
@@ -91,6 +92,12 @@ function buildCronWebhookHeaders(webhookToken?: string): Record<string, string>
9192
return headers;
9293
}
9394

95+
function buildCronDeliveryWebhookPayload(evt: CronEvent): CronEvent {
96+
return typeof evt.summary === "string"
97+
? { ...evt, summary: redactCronDeliveryText(evt.summary) }
98+
: evt;
99+
}
100+
94101
/** Posts a cron webhook without throwing back into scheduler completion flow. */
95102
async function postCronWebhook(params: {
96103
webhookUrl: string;
@@ -267,7 +274,7 @@ export function dispatchGatewayCronFinishedNotifications(params: {
267274
await postCronWebhook({
268275
webhookUrl: webhookTarget.url,
269276
webhookToken,
270-
payload: params.evt,
277+
payload: buildCronDeliveryWebhookPayload(params.evt),
271278
logContext: { jobId: params.evt.jobId, source: webhookTarget.source },
272279
blockedLog: "cron: webhook delivery blocked by SSRF guard",
273280
failedLog: "cron: webhook delivery failed",

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

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -584,6 +584,60 @@ describe("buildGatewayCronService", () => {
584584
}
585585
});
586586

587+
it("redacts command cron summaries before webhook delivery without changing stored run state", async () => {
588+
const cfg = createCronConfig("server-cron-command-webhook-redaction");
589+
loadConfigMock.mockReturnValue(cfg);
590+
fetchWithSsrFGuardMock.mockResolvedValue({ release: vi.fn(async () => {}) });
591+
592+
const state = buildGatewayCronService({
593+
cfg,
594+
deps: {} as CliDeps,
595+
broadcast: () => {},
596+
});
597+
try {
598+
const job = await state.cron.add({
599+
name: "redacted-command-webhook",
600+
enabled: true,
601+
deleteAfterRun: false,
602+
schedule: { kind: "at", at: new Date(1).toISOString() },
603+
sessionTarget: "isolated",
604+
wakeMode: "next-heartbeat",
605+
payload: {
606+
kind: "command",
607+
argv: [
608+
process.execPath,
609+
"-e",
610+
[
611+
"process.stdout.write('Open https://microsoft.com/devicelogin and enter code ABCD-EFGH\\n')",
612+
"process.stdout.write('OPENAI_API_KEY=sk-1234567890abcdef\\n')",
613+
].join(";"),
614+
],
615+
},
616+
delivery: {
617+
mode: "webhook",
618+
to: "https://example.invalid/cron-finished",
619+
},
620+
});
621+
622+
await state.cron.run(job.id, "force");
623+
624+
expect(fetchWithSsrFGuardMock).toHaveBeenCalledTimes(1);
625+
const request = requireRecord(
626+
callArg(fetchWithSsrFGuardMock, 0, 0, "webhook request"),
627+
"webhook request",
628+
);
629+
const init = requireRecord(request.init, "webhook init");
630+
const payload = JSON.parse(String(init.body)) as { summary?: string };
631+
expect(payload.summary).toContain("[redacted device authorization output]");
632+
expect(payload.summary).toContain("OPENAI_API_KEY=sk-123…cdef");
633+
expect(payload.summary).not.toContain("ABCD-EFGH");
634+
expect(payload.summary).not.toContain("sk-1234567890abcdef");
635+
expect(state.cron.getJob(job.id)?.state.lastDiagnosticSummary).toContain("ABCD-EFGH");
636+
} finally {
637+
state.cron.stop();
638+
}
639+
});
640+
587641
it("routes global-scope main cron jobs through the global queue for queued wakes", async () => {
588642
const cfg = {
589643
...createCronConfig("server-cron-global-queued"),

0 commit comments

Comments
 (0)