mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-04 09:40:21 +00:00
fix: enforce 600 perms for cron store and run logs (#36078)
* fix: enforce secure permissions for cron store and run logs * fix(cron): enforce dir perms and gate posix tests on windows * Cron store tests: cover existing directory permission hardening * Cron run-log tests: cover existing directory permission hardening * Changelog: note cron file permission hardening --------- Co-authored-by: linhey <linhey@mini.local> Co-authored-by: Vincent Koc <vincentkoc@ieee.org>
This commit is contained in:
@@ -95,6 +95,47 @@ describe("cron run log", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it.skipIf(process.platform === "win32")(
|
||||
"writes run log files with secure permissions",
|
||||
async () => {
|
||||
await withRunLogDir("openclaw-cron-log-perms-", async (dir) => {
|
||||
const logPath = path.join(dir, "runs", "job-1.jsonl");
|
||||
|
||||
await appendCronRunLog(logPath, {
|
||||
ts: 1,
|
||||
jobId: "job-1",
|
||||
action: "finished",
|
||||
status: "ok",
|
||||
});
|
||||
|
||||
const mode = (await fs.stat(logPath)).mode & 0o777;
|
||||
expect(mode).toBe(0o600);
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
it.skipIf(process.platform === "win32")(
|
||||
"hardens an existing run-log directory to owner-only permissions",
|
||||
async () => {
|
||||
await withRunLogDir("openclaw-cron-log-dir-perms-", async (dir) => {
|
||||
const runDir = path.join(dir, "runs");
|
||||
const logPath = path.join(runDir, "job-1.jsonl");
|
||||
await fs.mkdir(runDir, { recursive: true, mode: 0o755 });
|
||||
await fs.chmod(runDir, 0o755);
|
||||
|
||||
await appendCronRunLog(logPath, {
|
||||
ts: 1,
|
||||
jobId: "job-1",
|
||||
action: "finished",
|
||||
status: "ok",
|
||||
});
|
||||
|
||||
const runDirMode = (await fs.stat(runDir)).mode & 0o777;
|
||||
expect(runDirMode).toBe(0o700);
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
it("reads newest entries and filters by jobId", async () => {
|
||||
await withRunLogDir("openclaw-cron-log-read-", async (dir) => {
|
||||
const logPathA = path.join(dir, "runs", "a.jsonl");
|
||||
|
||||
@@ -75,6 +75,10 @@ export function resolveCronRunLogPath(params: { storePath: string; jobId: string
|
||||
|
||||
const writesByPath = new Map<string, Promise<void>>();
|
||||
|
||||
async function setSecureFileMode(filePath: string): Promise<void> {
|
||||
await fs.chmod(filePath, 0o600).catch(() => undefined);
|
||||
}
|
||||
|
||||
export const DEFAULT_CRON_RUN_LOG_MAX_BYTES = 2_000_000;
|
||||
export const DEFAULT_CRON_RUN_LOG_KEEP_LINES = 2_000;
|
||||
|
||||
@@ -125,8 +129,10 @@ async function pruneIfNeeded(filePath: string, opts: { maxBytes: number; keepLin
|
||||
const kept = lines.slice(Math.max(0, lines.length - opts.keepLines));
|
||||
const { randomBytes } = await import("node:crypto");
|
||||
const tmp = `${filePath}.${process.pid}.${randomBytes(8).toString("hex")}.tmp`;
|
||||
await fs.writeFile(tmp, `${kept.join("\n")}\n`, "utf-8");
|
||||
await fs.writeFile(tmp, `${kept.join("\n")}\n`, { encoding: "utf-8", mode: 0o600 });
|
||||
await setSecureFileMode(tmp);
|
||||
await fs.rename(tmp, filePath);
|
||||
await setSecureFileMode(filePath);
|
||||
}
|
||||
|
||||
export async function appendCronRunLog(
|
||||
@@ -139,8 +145,14 @@ export async function appendCronRunLog(
|
||||
const next = prev
|
||||
.catch(() => undefined)
|
||||
.then(async () => {
|
||||
await fs.mkdir(path.dirname(resolved), { recursive: true });
|
||||
await fs.appendFile(resolved, `${JSON.stringify(entry)}\n`, "utf-8");
|
||||
const runDir = path.dirname(resolved);
|
||||
await fs.mkdir(runDir, { recursive: true, mode: 0o700 });
|
||||
await fs.chmod(runDir, 0o700).catch(() => undefined);
|
||||
await fs.appendFile(resolved, `${JSON.stringify(entry)}\n`, {
|
||||
encoding: "utf-8",
|
||||
mode: 0o600,
|
||||
});
|
||||
await setSecureFileMode(resolved);
|
||||
await pruneIfNeeded(resolved, {
|
||||
maxBytes: opts?.maxBytes ?? DEFAULT_CRON_RUN_LOG_MAX_BYTES,
|
||||
keepLines: opts?.keepLines ?? DEFAULT_CRON_RUN_LOG_KEEP_LINES,
|
||||
|
||||
@@ -79,6 +79,39 @@ describe("cron store", () => {
|
||||
expect(JSON.parse(currentRaw)).toEqual(second);
|
||||
expect(JSON.parse(backupRaw)).toEqual(first);
|
||||
});
|
||||
|
||||
it.skipIf(process.platform === "win32")(
|
||||
"writes store and backup files with secure permissions",
|
||||
async () => {
|
||||
const store = await makeStorePath();
|
||||
const first = makeStore("job-1", true);
|
||||
const second = makeStore("job-2", false);
|
||||
|
||||
await saveCronStore(store.storePath, first);
|
||||
await saveCronStore(store.storePath, second);
|
||||
|
||||
const storeMode = (await fs.stat(store.storePath)).mode & 0o777;
|
||||
const backupMode = (await fs.stat(`${store.storePath}.bak`)).mode & 0o777;
|
||||
|
||||
expect(storeMode).toBe(0o600);
|
||||
expect(backupMode).toBe(0o600);
|
||||
},
|
||||
);
|
||||
|
||||
it.skipIf(process.platform === "win32")(
|
||||
"hardens an existing cron store directory to owner-only permissions",
|
||||
async () => {
|
||||
const store = await makeStorePath();
|
||||
const storeDir = path.dirname(store.storePath);
|
||||
await fs.mkdir(storeDir, { recursive: true, mode: 0o755 });
|
||||
await fs.chmod(storeDir, 0o755);
|
||||
|
||||
await saveCronStore(store.storePath, makeStore("job-1", true));
|
||||
|
||||
const storeDirMode = (await fs.stat(storeDir)).mode & 0o777;
|
||||
expect(storeDirMode).toBe(0o700);
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
describe("saveCronStore", () => {
|
||||
|
||||
@@ -56,12 +56,18 @@ type SaveCronStoreOptions = {
|
||||
skipBackup?: boolean;
|
||||
};
|
||||
|
||||
async function setSecureFileMode(filePath: string): Promise<void> {
|
||||
await fs.promises.chmod(filePath, 0o600).catch(() => undefined);
|
||||
}
|
||||
|
||||
export async function saveCronStore(
|
||||
storePath: string,
|
||||
store: CronStoreFile,
|
||||
opts?: SaveCronStoreOptions,
|
||||
) {
|
||||
await fs.promises.mkdir(path.dirname(storePath), { recursive: true });
|
||||
const storeDir = path.dirname(storePath);
|
||||
await fs.promises.mkdir(storeDir, { recursive: true, mode: 0o700 });
|
||||
await fs.promises.chmod(storeDir, 0o700).catch(() => undefined);
|
||||
const json = JSON.stringify(store, null, 2);
|
||||
const cached = serializedStoreCache.get(storePath);
|
||||
if (cached === json) {
|
||||
@@ -83,15 +89,19 @@ export async function saveCronStore(
|
||||
return;
|
||||
}
|
||||
const tmp = `${storePath}.${process.pid}.${randomBytes(8).toString("hex")}.tmp`;
|
||||
await fs.promises.writeFile(tmp, json, "utf-8");
|
||||
await fs.promises.writeFile(tmp, json, { encoding: "utf-8", mode: 0o600 });
|
||||
await setSecureFileMode(tmp);
|
||||
if (previous !== null && !opts?.skipBackup) {
|
||||
try {
|
||||
await fs.promises.copyFile(storePath, `${storePath}.bak`);
|
||||
const backupPath = `${storePath}.bak`;
|
||||
await fs.promises.copyFile(storePath, backupPath);
|
||||
await setSecureFileMode(backupPath);
|
||||
} catch {
|
||||
// best-effort
|
||||
}
|
||||
}
|
||||
await renameWithRetry(tmp, storePath);
|
||||
await setSecureFileMode(storePath);
|
||||
serializedStoreCache.set(storePath, json);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user