Skip to content

Commit 15779ff

Browse files
fix(miner): reject non-hex commitSha in replay-snapshot path planner (#7796)
Match replay-task-generation's /^[0-9a-f]{7,40}$/i guard so a crafted commitSha cannot escape .loopover-replay-snapshots via path.join. Closes #7796 Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent fa21f3e commit 15779ff

2 files changed

Lines changed: 59 additions & 42 deletions

File tree

packages/loopover-miner/lib/replay-snapshot.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,10 @@ function normalizeRepoFullName(repoFullName: string): string {
7777

7878
function normalizeCommitSha(commitSha: string): string {
7979
if (typeof commitSha !== "string" || !commitSha.trim()) throw new Error("invalid_commit_sha");
80-
return commitSha.trim();
80+
const trimmed = commitSha.trim();
81+
// Same format guard as replay-task-generation.ts — reject path-traversal / non-hex values before path.join (#7796).
82+
if (!/^[0-9a-f]{7,40}$/i.test(trimmed)) throw new Error("invalid_commit_sha");
83+
return trimmed;
8184
}
8285

8386
/** Worktree exports live under this dir inside the repo, mirroring worktree-allocator.ts's WORKTREE_SUBDIR. */

test/unit/miner-replay-snapshot.test.ts

Lines changed: 55 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ function happyPathScripts(overrides: Array<{ match: (args: readonly string[]) =>
4747
...overrides,
4848
{ match: isWorktreeAdd, result: ok() },
4949
{ match: isTargetDate, result: ok("2026-01-05T00:00:00+00:00\n") },
50-
{ match: isHistory, result: ok(`abc123${FIELD_SEP}2026-01-05T00:00:00+00:00${FIELD_SEP}the target commit\n`) },
50+
{ match: isHistory, result: ok(`abc1234${FIELD_SEP}2026-01-05T00:00:00+00:00${FIELD_SEP}the target commit\n`) },
5151
{ match: isTag, result: ok(`v1.0.0${FIELD_SEP}2026-01-01T00:00:00+00:00${FIELD_SEP}abc000${FIELD_SEP}tag\n`) },
5252
{ match: isLsTree, result: ok("README.md\nsrc\npackage.json\n") },
5353
{ match: isShow, result: ok("# hello\n") },
@@ -72,10 +72,24 @@ afterEach(() => {
7272

7373
describe("planReplaySnapshotPath (#3010) — pure, deterministic", () => {
7474
it("same (repoPath, commitSha) always yields the same path", () => {
75-
const a = planReplaySnapshotPath({ repoPath: "/repo", commitSha: "abc123" });
76-
expect(a.replaceAll("\\", "/")).toBe(`/repo/${REPLAY_SNAPSHOT_SUBDIR}/abc123`);
77-
expect(planReplaySnapshotPath({ repoPath: "/repo", commitSha: "abc123" })).toBe(a);
78-
expect(planReplaySnapshotPath({ repoPath: "/repo", commitSha: "def456" })).not.toBe(a);
75+
const a = planReplaySnapshotPath({ repoPath: "/repo", commitSha: "abc1234" });
76+
expect(a.replaceAll("\\", "/")).toBe(`/repo/${REPLAY_SNAPSHOT_SUBDIR}/abc1234`);
77+
expect(planReplaySnapshotPath({ repoPath: "/repo", commitSha: "abc1234" })).toBe(a);
78+
expect(planReplaySnapshotPath({ repoPath: "/repo", commitSha: "def4567" })).not.toBe(a);
79+
});
80+
81+
it("rejects a path-traversal-shaped commitSha before path.join (#7796)", () => {
82+
expect(() =>
83+
planReplaySnapshotPath({
84+
repoPath: "/home/miner/.config/loopover-miner/repos/acme/widgets",
85+
commitSha: "../../../../../../tmp/evil-worktree",
86+
}),
87+
).toThrow("invalid_commit_sha");
88+
});
89+
90+
it("rejects a non-hex commitSha (#7796)", () => {
91+
expect(() => planReplaySnapshotPath({ repoPath: "/repo", commitSha: "not-a-sha!" })).toThrow("invalid_commit_sha");
92+
expect(() => planReplaySnapshotPath({ repoPath: "/repo", commitSha: "abc12" })).toThrow("invalid_commit_sha"); // too short
7993
});
8094
});
8195

@@ -129,25 +143,25 @@ describe("exportReplaySnapshot (#3010)", () => {
129143
const store = tempStore();
130144

131145
const snapshot = await exportReplaySnapshot(
132-
{ repoPath: "/repo", repoFullName: "acme/widgets", commitSha: "abc123" },
146+
{ repoPath: "/repo", repoFullName: "acme/widgets", commitSha: "abc1234" },
133147
{ exec, store },
134148
);
135149

136150
expect(snapshot.repoFullName).toBe("acme/widgets");
137-
expect(snapshot.commitSha).toBe("abc123");
151+
expect(snapshot.commitSha).toBe("abc1234");
138152
expect(snapshot.targetDate).toBe("2026-01-05T00:00:00+00:00");
139-
expect(snapshot.commits).toEqual([{ sha: "abc123", date: "2026-01-05T00:00:00+00:00", subject: "the target commit" }]);
153+
expect(snapshot.commits).toEqual([{ sha: "abc1234", date: "2026-01-05T00:00:00+00:00", subject: "the target commit" }]);
140154
expect(snapshot.tags).toEqual([{ name: "v1.0.0", date: "2026-01-01T00:00:00+00:00", targetSha: "abc000" }]);
141155
expect(snapshot.readme).toEqual({ filename: "README.md", content: "# hello\n" });
142156
});
143157

144158
it("returns the cached snapshot on a repeat export of the same (repo, commit) pair, without calling git again", async () => {
145159
const store = tempStore();
146160
const first = scriptedExec(happyPathScripts());
147-
await exportReplaySnapshot({ repoPath: "/repo", repoFullName: "acme/widgets", commitSha: "abc123" }, { exec: first.exec, store });
161+
await exportReplaySnapshot({ repoPath: "/repo", repoFullName: "acme/widgets", commitSha: "abc1234" }, { exec: first.exec, store });
148162

149163
const second = scriptedExec([]); // no scripts at all -- any call would throw "no script matched"
150-
const result = await exportReplaySnapshot({ repoPath: "/repo", repoFullName: "acme/widgets", commitSha: "abc123" }, { exec: second.exec, store });
164+
const result = await exportReplaySnapshot({ repoPath: "/repo", repoFullName: "acme/widgets", commitSha: "abc1234" }, { exec: second.exec, store });
151165

152166
expect(result.commits).toHaveLength(1);
153167
expect(second.calls).toHaveLength(0);
@@ -157,7 +171,7 @@ describe("exportReplaySnapshot (#3010)", () => {
157171
const { exec } = scriptedExec(happyPathScripts([{ match: isTag, result: ok("") }]));
158172
const store = tempStore();
159173

160-
const snapshot = await exportReplaySnapshot({ repoPath: "/repo", repoFullName: "acme/widgets", commitSha: "abc123" }, { exec, store });
174+
const snapshot = await exportReplaySnapshot({ repoPath: "/repo", repoFullName: "acme/widgets", commitSha: "abc1234" }, { exec, store });
161175

162176
expect(snapshot.tags).toEqual([]);
163177
});
@@ -170,7 +184,7 @@ describe("exportReplaySnapshot (#3010)", () => {
170184
const { exec } = scriptedExec(happyPathScripts([{ match: isTag, result: ok(tagStdout) }]));
171185
const store = tempStore();
172186

173-
const snapshot = await exportReplaySnapshot({ repoPath: "/repo", repoFullName: "acme/widgets", commitSha: "abc123" }, { exec, store });
187+
const snapshot = await exportReplaySnapshot({ repoPath: "/repo", repoFullName: "acme/widgets", commitSha: "abc1234" }, { exec, store });
174188

175189
expect(snapshot.tags).toEqual([
176190
{ name: "v1.0.0", date: "2025-12-01T00:00:00+00:00", targetSha: "sha1" },
@@ -186,7 +200,7 @@ describe("exportReplaySnapshot (#3010)", () => {
186200
const { exec } = scriptedExec(happyPathScripts([{ match: isTag, result: ok(tagStdout) }]));
187201
const store = tempStore();
188202

189-
const snapshot = await exportReplaySnapshot({ repoPath: "/repo", repoFullName: "acme/widgets", commitSha: "abc123" }, { exec, store });
203+
const snapshot = await exportReplaySnapshot({ repoPath: "/repo", repoFullName: "acme/widgets", commitSha: "abc1234" }, { exec, store });
190204

191205
expect(snapshot.tags).toEqual([{ name: "v1.0.0", date: "2025-12-01T00:00:00+00:00", targetSha: "sha1" }]);
192206
});
@@ -210,7 +224,7 @@ describe("exportReplaySnapshot (#3010)", () => {
210224
const { exec, calls } = scriptedExec(happyPathScripts([{ match: isLsTree, result: ok("src\npackage.json\n") }]));
211225
const store = tempStore();
212226

213-
const snapshot = await exportReplaySnapshot({ repoPath: "/repo", repoFullName: "acme/widgets", commitSha: "abc123" }, { exec, store });
227+
const snapshot = await exportReplaySnapshot({ repoPath: "/repo", repoFullName: "acme/widgets", commitSha: "abc1234" }, { exec, store });
214228

215229
expect(snapshot.readme).toBeNull();
216230
expect(calls.some((c) => c.args[0] === "show")).toBe(false);
@@ -220,7 +234,7 @@ describe("exportReplaySnapshot (#3010)", () => {
220234
const { exec } = scriptedExec(happyPathScripts([{ match: isLsTree, result: ok("src\nReadme.rst\npackage.json\n") }]));
221235
const store = tempStore();
222236

223-
const snapshot = await exportReplaySnapshot({ repoPath: "/repo", repoFullName: "acme/widgets", commitSha: "abc123" }, { exec, store });
237+
const snapshot = await exportReplaySnapshot({ repoPath: "/repo", repoFullName: "acme/widgets", commitSha: "abc1234" }, { exec, store });
224238

225239
expect(snapshot.readme?.filename).toBe("Readme.rst");
226240
});
@@ -229,19 +243,19 @@ describe("exportReplaySnapshot (#3010)", () => {
229243
const { exec, calls } = scriptedExec(happyPathScripts([{ match: isTag, result: ok(`v-future${FIELD_SEP}2026-06-01T00:00:00+00:00${FIELD_SEP}abc000${FIELD_SEP}tag\n`) }]));
230244
const store = tempStore();
231245

232-
await expect(exportReplaySnapshot({ repoPath: "/repo", repoFullName: "acme/widgets", commitSha: "abc123" }, { exec, store })).rejects.toThrow(
246+
await expect(exportReplaySnapshot({ repoPath: "/repo", repoFullName: "acme/widgets", commitSha: "abc1234" }, { exec, store })).rejects.toThrow(
233247
/replay_snapshot_freshness_violation/,
234248
);
235-
expect(store.getSnapshot("acme/widgets", "abc123")).toBeNull();
249+
expect(store.getSnapshot("acme/widgets", "abc1234")).toBeNull();
236250
const removeCall = calls.find((c) => c.args[0] === "worktree" && c.args[1] === "remove");
237-
expect(removeCall?.args).toEqual(["worktree", "remove", "--force", "/repo/.loopover-replay-snapshots/abc123"]);
251+
expect(removeCall?.args).toEqual(["worktree", "remove", "--force", "/repo/.loopover-replay-snapshots/abc1234"]);
238252
});
239253

240254
it("removes the worktree and rethrows the ORIGINAL error (not a cleanup error) when a git read after the worktree exists fails", async () => {
241255
const { exec, calls } = scriptedExec(happyPathScripts([{ match: isHistory, result: { code: 1, stderr: "fatal: history read failed" } }]));
242256
const store = tempStore();
243257

244-
await expect(exportReplaySnapshot({ repoPath: "/repo", repoFullName: "acme/widgets", commitSha: "abc123" }, { exec, store })).rejects.toThrow(
258+
await expect(exportReplaySnapshot({ repoPath: "/repo", repoFullName: "acme/widgets", commitSha: "abc1234" }, { exec, store })).rejects.toThrow(
245259
/git_log_history_failed/,
246260
);
247261
const removeCall = calls.find((c) => c.args[0] === "worktree" && c.args[1] === "remove");
@@ -257,7 +271,7 @@ describe("exportReplaySnapshot (#3010)", () => {
257271
);
258272
const store = tempStore();
259273

260-
await expect(exportReplaySnapshot({ repoPath: "/repo", repoFullName: "acme/widgets", commitSha: "abc123" }, { exec, store })).rejects.toThrow(
274+
await expect(exportReplaySnapshot({ repoPath: "/repo", repoFullName: "acme/widgets", commitSha: "abc1234" }, { exec, store })).rejects.toThrow(
261275
/git_log_history_failed/,
262276
);
263277
});
@@ -266,9 +280,9 @@ describe("exportReplaySnapshot (#3010)", () => {
266280
const { exec } = scriptedExec(happyPathScripts([{ match: isWorktreeAdd, result: { code: 0 } }]));
267281
const store = tempStore();
268282

269-
const snapshot = await exportReplaySnapshot({ repoPath: "/repo", repoFullName: "acme/widgets", commitSha: "abc123" }, { exec, store });
283+
const snapshot = await exportReplaySnapshot({ repoPath: "/repo", repoFullName: "acme/widgets", commitSha: "abc1234" }, { exec, store });
270284

271-
expect(snapshot.commitSha).toBe("abc123");
285+
expect(snapshot.commitSha).toBe("abc1234");
272286
});
273287

274288
it("throws when git worktree add fails", async () => {
@@ -302,7 +316,7 @@ describe("exportReplaySnapshot (#3010)", () => {
302316
const { exec } = scriptedExec(happyPathScripts([{ match: isHistory, result: { code: 1, stderr: "fatal: history read failed" } }]));
303317
const store = tempStore();
304318

305-
await expect(exportReplaySnapshot({ repoPath: "/repo", repoFullName: "acme/widgets", commitSha: "abc123" }, { exec, store })).rejects.toThrow(
319+
await expect(exportReplaySnapshot({ repoPath: "/repo", repoFullName: "acme/widgets", commitSha: "abc1234" }, { exec, store })).rejects.toThrow(
306320
/git_log_history_failed/,
307321
);
308322
});
@@ -311,7 +325,7 @@ describe("exportReplaySnapshot (#3010)", () => {
311325
const { exec } = scriptedExec(happyPathScripts([{ match: isTag, result: { code: 1, stderr: "fatal: tag read failed" } }]));
312326
const store = tempStore();
313327

314-
await expect(exportReplaySnapshot({ repoPath: "/repo", repoFullName: "acme/widgets", commitSha: "abc123" }, { exec, store })).rejects.toThrow(
328+
await expect(exportReplaySnapshot({ repoPath: "/repo", repoFullName: "acme/widgets", commitSha: "abc1234" }, { exec, store })).rejects.toThrow(
315329
/git_tag_merged_failed/,
316330
);
317331
});
@@ -320,7 +334,7 @@ describe("exportReplaySnapshot (#3010)", () => {
320334
const { exec } = scriptedExec(happyPathScripts([{ match: isLsTree, result: { code: 1, stderr: "fatal: ls-tree failed" } }]));
321335
const store = tempStore();
322336

323-
await expect(exportReplaySnapshot({ repoPath: "/repo", repoFullName: "acme/widgets", commitSha: "abc123" }, { exec, store })).rejects.toThrow(
337+
await expect(exportReplaySnapshot({ repoPath: "/repo", repoFullName: "acme/widgets", commitSha: "abc1234" }, { exec, store })).rejects.toThrow(
324338
/git_ls_tree_failed/,
325339
);
326340
});
@@ -329,7 +343,7 @@ describe("exportReplaySnapshot (#3010)", () => {
329343
const { exec } = scriptedExec(happyPathScripts([{ match: isShow, result: { code: 1, stderr: "fatal: show failed" } }]));
330344
const store = tempStore();
331345

332-
await expect(exportReplaySnapshot({ repoPath: "/repo", repoFullName: "acme/widgets", commitSha: "abc123" }, { exec, store })).rejects.toThrow(
346+
await expect(exportReplaySnapshot({ repoPath: "/repo", repoFullName: "acme/widgets", commitSha: "abc1234" }, { exec, store })).rejects.toThrow(
333347
/git_show_readme_failed/,
334348
);
335349
});
@@ -347,18 +361,18 @@ describe("exportReplaySnapshot (#3010)", () => {
347361
const { exec } = scriptedExec(happyPathScripts([{ match: isWorktreeAdd, result: { code: 1 } }]));
348362
const store = tempStore();
349363

350-
await expect(exportReplaySnapshot({ repoPath: "/repo", repoFullName: "acme/widgets", commitSha: "abc123" }, { exec, store })).rejects.toThrow(
364+
await expect(exportReplaySnapshot({ repoPath: "/repo", repoFullName: "acme/widgets", commitSha: "abc1234" }, { exec, store })).rejects.toThrow(
351365
/git_worktree_add_failed: exit_1/,
352366
);
353367
});
354368

355369
it("tolerates a commit-history line missing the subject field, defaulting it to an empty string", async () => {
356-
const { exec } = scriptedExec(happyPathScripts([{ match: isHistory, result: ok(`abc123${FIELD_SEP}2026-01-05T00:00:00+00:00\n`) }]));
370+
const { exec } = scriptedExec(happyPathScripts([{ match: isHistory, result: ok(`abc1234${FIELD_SEP}2026-01-05T00:00:00+00:00\n`) }]));
357371
const store = tempStore();
358372

359-
const snapshot = await exportReplaySnapshot({ repoPath: "/repo", repoFullName: "acme/widgets", commitSha: "abc123" }, { exec, store });
373+
const snapshot = await exportReplaySnapshot({ repoPath: "/repo", repoFullName: "acme/widgets", commitSha: "abc1234" }, { exec, store });
360374

361-
expect(snapshot.commits).toEqual([{ sha: "abc123", date: "2026-01-05T00:00:00+00:00", subject: "" }]);
375+
expect(snapshot.commits).toEqual([{ sha: "abc1234", date: "2026-01-05T00:00:00+00:00", subject: "" }]);
362376
});
363377

364378
it("fails closed on a malformed input", async () => {
@@ -369,12 +383,12 @@ describe("exportReplaySnapshot (#3010)", () => {
369383
await expect(exportReplaySnapshot(null as never, deps)).rejects.toThrow("invalid_replay_snapshot_input");
370384
await expect(exportReplaySnapshot({ commitSha: "a" } as never, deps)).rejects.toThrow("invalid_repo_full_name");
371385
await expect(exportReplaySnapshot({ repoFullName: "acme/widgets" } as never, deps)).rejects.toThrow("invalid_commit_sha");
372-
await expect(exportReplaySnapshot({ repoFullName: "acme/widgets", commitSha: "abc123" } as never, deps)).rejects.toThrow("invalid_repo_path");
386+
await expect(exportReplaySnapshot({ repoFullName: "acme/widgets", commitSha: "abc1234" } as never, deps)).rejects.toThrow("invalid_repo_path");
373387
});
374388

375389
it("fails closed when exec is missing or invalid", async () => {
376390
const store = tempStore();
377-
const candidate = { repoPath: "/repo", repoFullName: "acme/widgets", commitSha: "abc123" };
391+
const candidate = { repoPath: "/repo", repoFullName: "acme/widgets", commitSha: "abc1234" };
378392
await expect(exportReplaySnapshot(candidate, null as never)).rejects.toThrow("invalid_exec");
379393
await expect(exportReplaySnapshot(candidate, { store } as never)).rejects.toThrow("invalid_exec");
380394
});
@@ -385,7 +399,7 @@ describe("exportReplaySnapshot (#3010)", () => {
385399
vi.stubEnv("LOOPOVER_MINER_REPLAY_SNAPSHOT_DB", join(root, "default.sqlite3"));
386400
const { exec } = scriptedExec(happyPathScripts());
387401

388-
const snapshot = await exportReplaySnapshot({ repoPath: "/repo", repoFullName: "acme/widgets", commitSha: "abc123" }, { exec });
402+
const snapshot = await exportReplaySnapshot({ repoPath: "/repo", repoFullName: "acme/widgets", commitSha: "abc1234" }, { exec });
389403

390404
expect(snapshot.repoFullName).toBe("acme/widgets");
391405
vi.unstubAllEnvs();
@@ -395,9 +409,9 @@ describe("exportReplaySnapshot (#3010)", () => {
395409
describe("removeReplaySnapshotWorktree (#3010)", () => {
396410
it("delegates to the shared removeWorktree primitive", async () => {
397411
const { exec, calls } = scriptedExec([{ match: () => true, result: ok() }]);
398-
const result = await removeReplaySnapshotWorktree(exec, "/repo", "/repo/.loopover-replay-snapshots/abc123");
412+
const result = await removeReplaySnapshotWorktree(exec, "/repo", "/repo/.loopover-replay-snapshots/abc1234");
399413
expect(result).toEqual({ ok: true, removed: true });
400-
expect(calls[0]?.args).toEqual(["worktree", "remove", "--force", "/repo/.loopover-replay-snapshots/abc123"]);
414+
expect(calls[0]?.args).toEqual(["worktree", "remove", "--force", "/repo/.loopover-replay-snapshots/abc1234"]);
401415
});
402416
});
403417

@@ -406,24 +420,24 @@ describe("openReplaySnapshotStore (#3010) — round-trip persistence", () => {
406420
const store = tempStore();
407421
const saved = store.saveSnapshot({
408422
repoFullName: "acme/widgets",
409-
commitSha: "abc123",
410-
worktreePath: "/repo/.loopover-replay-snapshots/abc123",
423+
commitSha: "abc1234",
424+
worktreePath: "/repo/.loopover-replay-snapshots/abc1234",
411425
targetDate: "2026-01-05T00:00:00+00:00",
412-
commits: [{ sha: "abc123", date: "2026-01-05T00:00:00+00:00", subject: "t" }],
426+
commits: [{ sha: "abc1234", date: "2026-01-05T00:00:00+00:00", subject: "t" }],
413427
tags: [{ name: "v1", date: "2026-01-01T00:00:00+00:00", targetSha: "abc000" }],
414428
readme: { filename: "README.md", content: "# hi\n" },
415429
});
416430

417431
expect(saved.readme).toEqual({ filename: "README.md", content: "# hi\n" });
418-
expect(store.getSnapshot("acme/widgets", "abc123")).toEqual(saved);
432+
expect(store.getSnapshot("acme/widgets", "abc1234")).toEqual(saved);
419433
});
420434

421435
it("round-trips a snapshot with no README as null, not a partial object", () => {
422436
const store = tempStore();
423437
const saved = store.saveSnapshot({
424438
repoFullName: "acme/widgets",
425-
commitSha: "abc123",
426-
worktreePath: "/repo/.loopover-replay-snapshots/abc123",
439+
commitSha: "abc1234",
440+
worktreePath: "/repo/.loopover-replay-snapshots/abc1234",
427441
targetDate: "2026-01-05T00:00:00+00:00",
428442
commits: [],
429443
tags: [],

0 commit comments

Comments
 (0)