fix(cron): normalize run-log jobId on write to match read-side validation (#93567)

Merged via squash.

Prepared head SHA: ee41f84b53
Co-authored-by: Alix-007 <267018309+Alix-007@users.noreply.github.com>
Co-authored-by: vincentkoc <25068+vincentkoc@users.noreply.github.com>
Reviewed-by: @vincentkoc
This commit is contained in:
Alix-007 2026-06-23 13:55:57 +08:00 committed by GitHub
parent fe5c098fd7
commit e583e62190
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 44 additions and 3 deletions

View file

@ -626,4 +626,35 @@ describe("cron run log", () => {
}
});
});
it("normalizes the jobId on write so the write/read roundtrip is symmetric", async () => {
await withRunLogDir("openclaw-cron-log-roundtrip-", async (dir) => {
const storePath = storePathForDir(dir);
await appendCronRunLog({
storePath,
entry: { ts: 1000, jobId: " spaced-job ", action: "finished", status: "ok" },
});
// Reads trim before querying, so the written row must be found under both the
// trimmed and the original whitespace-padded jobId, and stored normalized.
expect(readCronRunLogEntriesSync({ storePath, jobId: "spaced-job" })).toHaveLength(1);
expect(readCronRunLogEntriesSync({ storePath, jobId: " spaced-job " })).toHaveLength(1);
expect(readCronRunLogEntriesSync({ storePath, jobId: "spaced-job" })[0]?.jobId).toBe(
"spaced-job",
);
});
});
it("rejects unsafe job ids on write the same way reads do", async () => {
await withRunLogDir("openclaw-cron-log-write-reject-", async (dir) => {
const storePath = storePathForDir(dir);
for (const jobId of ["nested/job", "..\\job", " "]) {
await expect(
appendCronRunLog({
storePath,
entry: { ts: 1000, jobId, action: "finished", status: "ok" },
}),
).rejects.toThrow(/invalid cron run log job id/i);
}
});
});
});

View file

@ -138,20 +138,30 @@ export async function appendCronRunLog(params: {
entry: CronRunLogEntry;
opts?: AppendCronRunLogOptions;
}) {
// Normalize the jobId on write the same way reads do (assertSafeCronRunLogJobId
// trims + validates). Otherwise a jobId with surrounding whitespace is stored
// verbatim while reads trim before querying — the row is written but never read
// back — and a jobId containing "/" or "\\" is rejected on read yet silently
// accepted on write. Normalizing here keeps the write/read roundtrip symmetric.
const normalizedJobId = assertSafeCronRunLogJobId(params.entry.jobId);
const entry =
normalizedJobId === params.entry.jobId
? params.entry
: { ...params.entry, jobId: normalizedJobId };
const storeKey = cronStoreKey(params.storePath);
const writeKey = cronRunLogWriteKey(params.storePath, params.entry.jobId);
const writeKey = cronRunLogWriteKey(params.storePath, entry.jobId);
const prev = writesByTarget.get(writeKey) ?? Promise.resolve();
// Keep writes for the same store/job ordered so prune-by-count cannot race a later insert.
const next = prev
.catch(() => undefined)
.then(async () => {
runOpenClawStateWriteTransaction(({ db }) => {
insertCronRunLogEntry(db, storeKey, params.entry);
insertCronRunLogEntry(db, storeKey, entry);
if (params.opts?.keepLines !== false) {
pruneCronRunLogRows(
db,
storeKey,
params.entry.jobId,
entry.jobId,
params.opts?.keepLines ?? DEFAULT_CRON_RUN_LOG_KEEP_LINES,
);
}