mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-09 09:07:39 +00:00
Gateway: harden cron.runs jobId path handling (openclaw#24038) thanks @Takhoffman
Verified: - pnpm install --frozen-lockfile - pnpm build - pnpm check - pnpm test:macmini Co-authored-by: Takhoffman <781889+Takhoffman@users.noreply.github.com> Co-authored-by: Tak Hoffman <781889+Takhoffman@users.noreply.github.com>
This commit is contained in:
@@ -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/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/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: 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.
|
- 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).
|
- 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)
|
- 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)
|
||||||
|
|||||||
@@ -25,6 +25,19 @@ describe("cron run log", () => {
|
|||||||
expect(p.endsWith(path.join(os.tmpdir(), "cron", "runs", "job-1.jsonl"))).toBe(true);
|
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 () => {
|
it("appends JSONL and prunes by line count", async () => {
|
||||||
await withRunLogDir("openclaw-cron-log-", async (dir) => {
|
await withRunLogDir("openclaw-cron-log-", async (dir) => {
|
||||||
const logPath = path.join(dir, "runs", "job-1.jsonl");
|
const logPath = path.join(dir, "runs", "job-1.jsonl");
|
||||||
|
|||||||
@@ -19,10 +19,27 @@ export type CronRunLogEntry = {
|
|||||||
nextRunAtMs?: number;
|
nextRunAtMs?: number;
|
||||||
} & CronRunTelemetry;
|
} & 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 }) {
|
export function resolveCronRunLogPath(params: { storePath: string; jobId: string }) {
|
||||||
const storePath = path.resolve(params.storePath);
|
const storePath = path.resolve(params.storePath);
|
||||||
const dir = path.dirname(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<string, Promise<void>>();
|
const writesByPath = new Map<string, Promise<void>>();
|
||||||
|
|||||||
@@ -46,4 +46,11 @@ describe("cron protocol validators", () => {
|
|||||||
expect(validateCronRunsParams({ id: "job-1", limit: 0 })).toBe(false);
|
expect(validateCronRunsParams({ id: "job-1", limit: 0 })).toBe(false);
|
||||||
expect(validateCronRunsParams({ jobId: "job-2", 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);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -59,6 +59,31 @@ function cronIdOrJobIdParams(extraFields: Record<string, TSchema>) {
|
|||||||
]);
|
]);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const CronRunLogJobIdSchema = Type.String({
|
||||||
|
minLength: 1,
|
||||||
|
// Prevent path traversal via separators in cron.runs id/jobId.
|
||||||
|
pattern: "^[^/\\\\]+$",
|
||||||
|
});
|
||||||
|
|
||||||
|
function cronRunsIdOrJobIdParams(extraFields: Record<string, TSchema>) {
|
||||||
|
return Type.Union([
|
||||||
|
Type.Object(
|
||||||
|
{
|
||||||
|
id: CronRunLogJobIdSchema,
|
||||||
|
...extraFields,
|
||||||
|
},
|
||||||
|
{ additionalProperties: false },
|
||||||
|
),
|
||||||
|
Type.Object(
|
||||||
|
{
|
||||||
|
jobId: CronRunLogJobIdSchema,
|
||||||
|
...extraFields,
|
||||||
|
},
|
||||||
|
{ additionalProperties: false },
|
||||||
|
),
|
||||||
|
]);
|
||||||
|
}
|
||||||
|
|
||||||
export const CronScheduleSchema = Type.Union([
|
export const CronScheduleSchema = Type.Union([
|
||||||
Type.Object(
|
Type.Object(
|
||||||
{
|
{
|
||||||
@@ -241,7 +266,7 @@ export const CronRunParamsSchema = cronIdOrJobIdParams({
|
|||||||
mode: Type.Optional(Type.Union([Type.Literal("due"), Type.Literal("force")])),
|
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 })),
|
limit: Type.Optional(Type.Integer({ minimum: 1, maximum: 5000 })),
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
@@ -214,10 +214,20 @@ export const cronHandlers: GatewayRequestHandlers = {
|
|||||||
);
|
);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
const logPath = resolveCronRunLogPath({
|
let logPath: string;
|
||||||
storePath: context.cronStorePath,
|
try {
|
||||||
jobId,
|
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, {
|
const entries = await readCronRunLogEntries(logPath, {
|
||||||
limit: p.limit,
|
limit: p.limit,
|
||||||
jobId,
|
jobId,
|
||||||
|
|||||||
Reference in New Issue
Block a user