diff --git a/CHANGELOG.md b/CHANGELOG.md index 62a8bbaff..9ee24be33 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -76,6 +76,7 @@ Docs: https://docs.openclaw.ai - Cron/Service: execute manual `cron.run` jobs outside the cron lock (while still persisting started/finished state atomically) so `cron.list` and `cron.status` remain responsive during long forced runs. (#23628) Thanks @dsgraves. - Cron/Timer: keep a watchdog recheck timer armed while `onTimer` is actively executing so the scheduler continues polling even if a due-run tick stalls for an extended period. (#23628) Thanks @dsgraves. - Cron/Run log: clean up settled per-path run-log write queue entries so long-running cron uptime does not retain stale promise bookkeeping in memory. +- Cron/Run log: harden `cron.runs` run-log path resolution by rejecting path-separator `id`/`jobId` inputs and enforcing reads within the per-cron `runs/` directory. - Cron/Isolation: force fresh session IDs for isolated cron runs so `sessionTarget="isolated"` executions never reuse prior run context. (#23470) Thanks @echoVic. - Plugins/Install: strip `workspace:*` devDependency entries from copied plugin manifests before `npm install --omit=dev`, preventing `EUNSUPPORTEDPROTOCOL` install failures for npm-published channel plugins (including Feishu and MS Teams). - Feishu/Plugins: restore bundled Feishu SDK availability for global installs and strip `openclaw: workspace:*` from plugin `devDependencies` during plugin-version sync so npm-installed Feishu plugins do not fail dependency install. (#23611, #23645, #23603) diff --git a/src/cron/run-log.test.ts b/src/cron/run-log.test.ts index ed77d17a9..45c3b75b0 100644 --- a/src/cron/run-log.test.ts +++ b/src/cron/run-log.test.ts @@ -25,6 +25,19 @@ describe("cron run log", () => { expect(p.endsWith(path.join(os.tmpdir(), "cron", "runs", "job-1.jsonl"))).toBe(true); }); + it("rejects unsafe job ids when resolving run log path", () => { + const storePath = path.join(os.tmpdir(), "cron", "jobs.json"); + expect(() => resolveCronRunLogPath({ storePath, jobId: "../job-1" })).toThrow( + /invalid cron run log job id/i, + ); + expect(() => resolveCronRunLogPath({ storePath, jobId: "nested/job-1" })).toThrow( + /invalid cron run log job id/i, + ); + expect(() => resolveCronRunLogPath({ storePath, jobId: "..\\job-1" })).toThrow( + /invalid cron run log job id/i, + ); + }); + it("appends JSONL and prunes by line count", async () => { await withRunLogDir("openclaw-cron-log-", async (dir) => { const logPath = path.join(dir, "runs", "job-1.jsonl"); diff --git a/src/cron/run-log.ts b/src/cron/run-log.ts index cdb54adde..3dd5c2790 100644 --- a/src/cron/run-log.ts +++ b/src/cron/run-log.ts @@ -19,10 +19,27 @@ export type CronRunLogEntry = { nextRunAtMs?: number; } & CronRunTelemetry; +function assertSafeCronRunLogJobId(jobId: string): string { + const trimmed = jobId.trim(); + if (!trimmed) { + throw new Error("invalid cron run log job id"); + } + if (trimmed.includes("/") || trimmed.includes("\\") || trimmed.includes("\0")) { + throw new Error("invalid cron run log job id"); + } + return trimmed; +} + export function resolveCronRunLogPath(params: { storePath: string; jobId: string }) { const storePath = path.resolve(params.storePath); const dir = path.dirname(storePath); - return path.join(dir, "runs", `${params.jobId}.jsonl`); + const runsDir = path.resolve(dir, "runs"); + const safeJobId = assertSafeCronRunLogJobId(params.jobId); + const resolvedPath = path.resolve(runsDir, `${safeJobId}.jsonl`); + if (!resolvedPath.startsWith(`${runsDir}${path.sep}`)) { + throw new Error("invalid cron run log job id"); + } + return resolvedPath; } const writesByPath = new Map>(); diff --git a/src/gateway/protocol/cron-validators.test.ts b/src/gateway/protocol/cron-validators.test.ts index 13fc84a59..e3e9de03e 100644 --- a/src/gateway/protocol/cron-validators.test.ts +++ b/src/gateway/protocol/cron-validators.test.ts @@ -46,4 +46,11 @@ describe("cron protocol validators", () => { expect(validateCronRunsParams({ id: "job-1", limit: 0 })).toBe(false); expect(validateCronRunsParams({ jobId: "job-2", limit: 0 })).toBe(false); }); + + it("rejects cron.runs path traversal ids", () => { + expect(validateCronRunsParams({ id: "../job-1" })).toBe(false); + expect(validateCronRunsParams({ id: "nested/job-1" })).toBe(false); + expect(validateCronRunsParams({ jobId: "..\\job-2" })).toBe(false); + expect(validateCronRunsParams({ jobId: "nested\\job-2" })).toBe(false); + }); }); diff --git a/src/gateway/protocol/schema/cron.ts b/src/gateway/protocol/schema/cron.ts index b162436d0..6f74ff24e 100644 --- a/src/gateway/protocol/schema/cron.ts +++ b/src/gateway/protocol/schema/cron.ts @@ -59,6 +59,31 @@ function cronIdOrJobIdParams(extraFields: Record) { ]); } +const CronRunLogJobIdSchema = Type.String({ + minLength: 1, + // Prevent path traversal via separators in cron.runs id/jobId. + pattern: "^[^/\\\\]+$", +}); + +function cronRunsIdOrJobIdParams(extraFields: Record) { + return Type.Union([ + Type.Object( + { + id: CronRunLogJobIdSchema, + ...extraFields, + }, + { additionalProperties: false }, + ), + Type.Object( + { + jobId: CronRunLogJobIdSchema, + ...extraFields, + }, + { additionalProperties: false }, + ), + ]); +} + export const CronScheduleSchema = Type.Union([ Type.Object( { @@ -241,7 +266,7 @@ export const CronRunParamsSchema = cronIdOrJobIdParams({ mode: Type.Optional(Type.Union([Type.Literal("due"), Type.Literal("force")])), }); -export const CronRunsParamsSchema = cronIdOrJobIdParams({ +export const CronRunsParamsSchema = cronRunsIdOrJobIdParams({ limit: Type.Optional(Type.Integer({ minimum: 1, maximum: 5000 })), }); diff --git a/src/gateway/server-methods/cron.ts b/src/gateway/server-methods/cron.ts index 054576091..0f73184c4 100644 --- a/src/gateway/server-methods/cron.ts +++ b/src/gateway/server-methods/cron.ts @@ -214,10 +214,20 @@ export const cronHandlers: GatewayRequestHandlers = { ); return; } - const logPath = resolveCronRunLogPath({ - storePath: context.cronStorePath, - jobId, - }); + let logPath: string; + try { + logPath = resolveCronRunLogPath({ + storePath: context.cronStorePath, + jobId, + }); + } catch { + respond( + false, + undefined, + errorShape(ErrorCodes.INVALID_REQUEST, "invalid cron.runs params: invalid id"), + ); + return; + } const entries = await readCronRunLogEntries(logPath, { limit: p.limit, jobId,