fix(workflows): validate runId before recursive prune delete (path-traversal dir wipe)

packages/core/src/agents/workflow-snapshot.test.ts

@@ -139,7 +139,10 @@ describe('writeWorkflowSnapshot + listWorkflowSnapshots', () => {
       await fs.mkdir(`${dir}/${runId}`, { recursive: true });
       await fs.writeFile(`${dir}/${runId}/journal.jsonl`, '{}\n', 'utf8');
       // Distinct runId per write; startTime ascending. Each write prunes.
-      await writeWorkflowSnapshot(config, task({ runId, startTime: 1_000 + i }));
+      await writeWorkflowSnapshot(
+        config,
+        task({ runId, startTime: 1_000 + i }),
+      );
     }
     const entries = await fs.readdir(dir);
     const files = entries.filter((f) => f.endsWith('.json'));
@@ -148,4 +151,48 @@ describe('writeWorkflowSnapshot + listWorkflowSnapshots', () => {
     // The pruned runs' journal directories are gone too (no orphan leak).
     expect(journalDirs.length).toBe(MAX_RETAINED_SNAPSHOTS);
   });
+
+  // Security: prune derives `runId` from the snapshot filename and feeds it to
+  // a recursive `fs.rm`. A crafted `.json` name must NOT let that delete
+  // anything but a well-formed `wf_<hex>` run dir — a file named `...json`
+  // yields `runId = ".."` (parent dir), `notarun.json` yields a sibling dir.
+  it('does not recursively delete via a crafted snapshot filename (path traversal)', async () => {
+    const config = fakeConfig(projectDir);
+    const dir = config.storage.getWorkflowRunsDir();
+    await fs.mkdir(dir, { recursive: true });
+
+    // Canary in the runs dir's PARENT — a `..` traversal would delete it.
+    const canary = path.join(dir, '..', 'CANARY.txt');
+    await fs.writeFile(canary, 'keep', 'utf8');
+    // A non-run sibling dir INSIDE the runs dir — a `notarun.json` stem targets it.
+    await fs.mkdir(path.join(dir, 'notarun'), { recursive: true });
+    await fs.writeFile(path.join(dir, 'notarun', 'keep.txt'), 'keep', 'utf8');
+
+    // Fill to the cap with legit run snapshots (no prune yet at == cap).
+    for (let i = 0; i < MAX_RETAINED_SNAPSHOTS; i++) {
+      await writeWorkflowSnapshot(
+        config,
+        task({ runId: `wf_${i.toString(16)}`, startTime: 10_000 + i }),
+      );
+    }
+    // Plant two malicious snapshot files as the OLDEST (pruned first):
+    //   `...json`      → stem `..`      → would rm the parent (project root)
+    //   `notarun.json` → stem `notarun` → would rm the sibling dir
+    for (const name of ['...json', 'notarun.json']) {
+      const p = path.join(dir, name);
+      await fs.writeFile(p, '{}', 'utf8');
+      await fs.utimes(p, new Date(0), new Date(0)); // oldest → selected to prune
+    }
+    // One more legit write tips the count over the cap and triggers prune.
+    await writeWorkflowSnapshot(
+      config,
+      task({ runId: 'wf_ff', startTime: 99_999 }),
+    );
+
+    // The guard spared both the parent canary and the non-run sibling dir.
+    await expect(fs.access(canary)).resolves.toBeUndefined();
+    await expect(
+      fs.access(path.join(dir, 'notarun', 'keep.txt')),
+    ).resolves.toBeUndefined();
+  });
 });

packages/core/src/agents/workflow-snapshot.ts

@@ -162,13 +162,32 @@ async function pruneSnapshots(dir: string): Promise<void> {
       // resume journal). Removing only the `<runId>.json` snapshot would leave
       // those journal dirs to grow without bound, so prune both together.
       const runId = s.f.replace(/\.json$/, '');
+      // ...but gate the recursive delete on a well-formed run id. The list is a
+      // plain `.json` glob, so a file named `...json` yields `runId = ".."` and
+      // `fs.rm(`${dir}/..`, {recursive,force})` would delete the runs dir's
+      // PARENT; `notarun.json` would delete a sibling `notarun/`. A malicious
+      // repo could ship such a file and trip it once pruning kicks in. Only the
+      // generated `wf_<hex>` shape (mirrors workflow.ts's resumeFromRunId guard)
+      // may drive `fs.rm`. The `.json` unlink stays unconditional — it removes
+      // exactly that one file, never a directory.
+      const isRunDir = /^wf_[0-9a-f]+$/.test(runId);
       return Promise.all([
-        fs.unlink(`${dir}/${s.f}`).catch((e) =>
-          debugLogger.warn(`prune unlink failed for ${s.f}: ${e}`),
-        ),
-        fs.rm(`${dir}/${runId}`, { recursive: true, force: true }).catch((e) =>
-          debugLogger.warn(`prune journal dir failed for ${runId}: ${e}`),
-        ),
+        fs
+          .unlink(`${dir}/${s.f}`)
+          .catch((e) =>
+            debugLogger.warn(`prune unlink failed for ${s.f}: ${e}`),
+          ),
+        ...(isRunDir
+          ? [
+              fs
+                .rm(`${dir}/${runId}`, { recursive: true, force: true })
+                .catch((e) =>
+                  debugLogger.warn(
+                    `prune journal dir failed for ${runId}: ${e}`,
+                  ),
+                ),
+            ]
+          : []),
       ]);
     }),
   );