Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion packages/loopover-miner/lib/replay-snapshot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,18 @@ function normalizeRepoFullName(repoFullName: string): string {
return `${owner}/${repo}`;
}

// A commit SHA is joined straight into REPLAY_SNAPSHOT_SUBDIR (and later passed to git as a bare revision),
// so a value like "../../../tmp/evil" (or one containing a path separator) would escape the intended snapshot
// directory via path.join (#7796). Constrain it to a single safe path segment -- the same restricted charset
// repo-clone.ts's isValidRepoSegment guard uses for owner/repo (#5831), plus an explicit "."/".." rejection.
// A genuine commit SHA is hex and always satisfies this, so no legitimate caller regresses.
const COMMIT_SHA_PATTERN = /^[A-Za-z0-9._-]+$/;

function normalizeCommitSha(commitSha: string): string {
if (typeof commitSha !== "string" || !commitSha.trim()) throw new Error("invalid_commit_sha");
return commitSha.trim();
const trimmed = commitSha.trim();
if (trimmed === "." || trimmed === ".." || !COMMIT_SHA_PATTERN.test(trimmed)) throw new Error("invalid_commit_sha");
return trimmed;
}

/** Worktree exports live under this dir inside the repo, mirroring worktree-allocator.ts's WORKTREE_SUBDIR. */
Expand Down
23 changes: 21 additions & 2 deletions test/unit/miner-replay-snapshot.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,20 @@ import { tmpdir } from "node:os";
import { join } from "node:path";
import { afterEach, describe, expect, it, vi } from "vitest";

import {
// Import the .ts SOURCE (not the build-time .js) via a non-literal specifier. The committed lib is .ts-only,
// but once `build:miner` has produced the artifact, a plain `.js`/extensionless import loads that .js, leaving
// coverage.include's `.ts` entry at 0% -- that .js-vs-.ts mismatch is why this file's new guard kept reporting
// 0% patch coverage. A variable specifier loads (and instruments) the .ts while dodging TS5097 (#7796).
const REPLAY_SNAPSHOT_MODULE = "../../packages/loopover-miner/lib/replay-snapshot.ts";
const {
closeDefaultReplaySnapshotStore,
exportReplaySnapshot,
openReplaySnapshotStore,
planReplaySnapshotPath,
removeReplaySnapshotWorktree,
REPLAY_SNAPSHOT_SUBDIR,
validateSnapshotFreshness,
} from "../../packages/loopover-miner/lib/replay-snapshot.js";
} = (await import(REPLAY_SNAPSHOT_MODULE)) as typeof import("../../packages/loopover-miner/lib/replay-snapshot.js");

const FIELD_SEP = "\x1f";

Expand Down Expand Up @@ -77,6 +82,20 @@ describe("planReplaySnapshotPath (#3010) — pure, deterministic", () => {
expect(planReplaySnapshotPath({ repoPath: "/repo", commitSha: "abc123" })).toBe(a);
expect(planReplaySnapshotPath({ repoPath: "/repo", commitSha: "def456" })).not.toBe(a);
});

it("rejects a commit SHA that would escape the snapshot subdir via path traversal or a separator (#7796)", () => {
// Without the guard, join()ing these into REPLAY_SNAPSHOT_SUBDIR escapes the repo entirely (e.g.
// ".../../../../../tmp/evil"). Each must be rejected up front rather than producing an out-of-sandbox path.
for (const commitSha of ["../../../../tmp/evil", "..", ".", "a/b", "a\\b", "../abc123", "foo/../..", " ../x "]) {
expect(() => planReplaySnapshotPath({ repoPath: "/repo", commitSha })).toThrow("invalid_commit_sha");
}
});

it("still confines a genuine hex commit SHA to the snapshot subdir (#7796)", () => {
const sha = "0a1b2c3d4e5f60718293a4b5c6d7e8f901234567";
const p = planReplaySnapshotPath({ repoPath: "/repo", commitSha: ` ${sha} ` }).replaceAll("\\", "/");
expect(p).toBe(`/repo/${REPLAY_SNAPSHOT_SUBDIR}/${sha}`);
});
});

describe("validateSnapshotFreshness (#3010) — pure fail-fast check", () => {
Expand Down