From 16ff1cc11ff9df81442818155f9e0891ea64a92e Mon Sep 17 00:00:00 2001 From: davion-knight Date: Tue, 21 Jul 2026 10:26:27 -0500 Subject: [PATCH] fix(miner): reject path-traversal repo segments in the 4 remaining normalizeRepoFullName parsers repo-clone.ts's isValidRepoSegment rejects a ./../control-char owner or repo segment before it is persisted to SQLite or echoed through a CLI. #5831 and #7525 rolled it out to ten sibling parsers, but four were missed and still only checked 'exactly one slash, both halves non-empty': contribution-profile-cache.ts, prediction-ledger.ts, replay-snapshot.ts, and run-state.ts. Inputs like 'owner/..', '../repo', or segments with tab/newline characters passed these four while being correctly rejected everywhere else. Import and call isValidRepoSegment on both owner and repo in all four, matching the exact call shape of the ten already-fixed siblings. Adds a per-file regression test asserting a path-traversal/invalid-character segment throws invalid_repo_full_name, covering both operands, mirroring test/unit/miner-claim-ledger.test.ts. Closes #7795 --- .../lib/contribution-profile-cache.ts | 2 ++ .../loopover-miner/lib/prediction-ledger.ts | 2 ++ packages/loopover-miner/lib/replay-snapshot.ts | 2 ++ packages/loopover-miner/lib/run-state.ts | 2 ++ .../miner-contribution-profile-cache.test.ts | 14 ++++++++++++++ test/unit/miner-prediction-ledger.test.ts | 14 ++++++++++++++ test/unit/miner-replay-snapshot.test.ts | 11 +++++++++++ test/unit/miner-run-state.test.ts | 18 ++++++++++++++++++ 8 files changed, 65 insertions(+) diff --git a/packages/loopover-miner/lib/contribution-profile-cache.ts b/packages/loopover-miner/lib/contribution-profile-cache.ts index 46f00660c1..d0ac746a01 100644 --- a/packages/loopover-miner/lib/contribution-profile-cache.ts +++ b/packages/loopover-miner/lib/contribution-profile-cache.ts @@ -13,6 +13,7 @@ import { openLocalStoreAdapter, resolveLocalStoreDbPath, } from "./local-store.js"; +import { isValidRepoSegment } from "./repo-clone.js"; import { applySchemaMigrations } from "./schema-version.js"; import { CONTRIBUTION_PROFILE_CACHE_PURGE_SPEC, @@ -58,6 +59,7 @@ function normalizeRepoFullName(repoFullName: unknown): string { const [owner, repo, extra] = repoFullName.trim().split("/"); if (!owner || !repo || extra !== undefined) throw new Error("invalid_repo_full_name"); + if (!isValidRepoSegment(owner) || !isValidRepoSegment(repo)) throw new Error("invalid_repo_full_name"); return `${owner}/${repo}`; } diff --git a/packages/loopover-miner/lib/prediction-ledger.ts b/packages/loopover-miner/lib/prediction-ledger.ts index ad4f4bda80..1f3928a10d 100644 --- a/packages/loopover-miner/lib/prediction-ledger.ts +++ b/packages/loopover-miner/lib/prediction-ledger.ts @@ -1,5 +1,6 @@ import type { DatabaseSync } from "node:sqlite"; import { normalizeLocalStoreDbPath, openLocalStoreAdapter, resolveLocalStoreDbPath } from "./local-store.js"; +import { isValidRepoSegment } from "./repo-clone.js"; import { applySchemaMigrations } from "./schema-version.js"; import { PREDICTION_LEDGER_PURGE_SPEC, @@ -87,6 +88,7 @@ function normalizeRepoFullName(repoFullName: string): string { if (typeof repoFullName !== "string") throw new Error("invalid_repo_full_name"); const [owner, repo, extra] = repoFullName.trim().split("/"); if (!owner || !repo || extra !== undefined) throw new Error("invalid_repo_full_name"); + if (!isValidRepoSegment(owner) || !isValidRepoSegment(repo)) throw new Error("invalid_repo_full_name"); return `${owner}/${repo}`; } diff --git a/packages/loopover-miner/lib/replay-snapshot.ts b/packages/loopover-miner/lib/replay-snapshot.ts index 363e9798f4..8debf2a5fa 100644 --- a/packages/loopover-miner/lib/replay-snapshot.ts +++ b/packages/loopover-miner/lib/replay-snapshot.ts @@ -2,6 +2,7 @@ import { join } from "node:path"; import { removeWorktree } from "@loopover/engine"; import type { WorktreeExecFn, WorktreeRemoveResult } from "@loopover/engine"; import { openLocalStoreAdapter, resolveLocalStoreDbPath, normalizeLocalStoreDbPath } from "./local-store.js"; +import { isValidRepoSegment } from "./repo-clone.js"; // Freeze/snapshot mechanism for historical replay targets (#3010). Given a repo and a commit SHA T, exports: // (a) the full working tree checked out AT T via a DETACHED git worktree -- the same isolation primitive @@ -72,6 +73,7 @@ function normalizeRepoFullName(repoFullName: string): string { if (typeof repoFullName !== "string") throw new Error("invalid_repo_full_name"); const [owner, repo, extra] = repoFullName.trim().split("/"); if (!owner || !repo || extra !== undefined) throw new Error("invalid_repo_full_name"); + if (!isValidRepoSegment(owner) || !isValidRepoSegment(repo)) throw new Error("invalid_repo_full_name"); return `${owner}/${repo}`; } diff --git a/packages/loopover-miner/lib/run-state.ts b/packages/loopover-miner/lib/run-state.ts index ec8de4d4a4..2538f62865 100644 --- a/packages/loopover-miner/lib/run-state.ts +++ b/packages/loopover-miner/lib/run-state.ts @@ -1,6 +1,7 @@ import type { DatabaseSync } from "node:sqlite"; import { DEFAULT_FORGE_CONFIG } from "./forge-config.js"; import { normalizeLocalStoreDbPath, openLocalStoreAdapter, resolveLocalStoreDbPath } from "./local-store.js"; +import { isValidRepoSegment } from "./repo-clone.js"; import { applySchemaMigrations } from "./schema-version.js"; import { RUN_STATE_PURGE_SPEC, purgeStoreByRepo } from "./store-maintenance.js"; @@ -57,6 +58,7 @@ function normalizeRepoFullName(repoFullName: string): string { const trimmed = repoFullName.trim(); const [owner, repo, extra] = trimmed.split("/"); if (!owner || !repo || extra !== undefined) throw new Error("invalid_repo_full_name"); + if (!isValidRepoSegment(owner) || !isValidRepoSegment(repo)) throw new Error("invalid_repo_full_name"); return `${owner}/${repo}`; } diff --git a/test/unit/miner-contribution-profile-cache.test.ts b/test/unit/miner-contribution-profile-cache.test.ts index 621b23c536..6eb9f3fae2 100644 --- a/test/unit/miner-contribution-profile-cache.test.ts +++ b/test/unit/miner-contribution-profile-cache.test.ts @@ -160,6 +160,20 @@ describe("contribution-profile cache store (#6797)", () => { ); }); + // #7795: an unsafe path-traversal/invalid-character segment must be rejected here too, matching + // repo-clone.js's own validation, instead of being silently accepted and persisted as a cache key -- + // for both the owner and repo segment independently. + it("rejects a repoFullName with a path-traversal or invalid-character segment", () => { + const store = tempStore(); + // Reads (get) and writes (put) both funnel through normalizeRepoFullName. + expect(() => store.get("../etc")).toThrow("invalid_repo_full_name"); // owner ".." invalid + expect(() => store.get("o/..")).toThrow("invalid_repo_full_name"); // repo ".." invalid + expect(() => store.get("o baz/a")).toThrow("invalid_repo_full_name"); + expect(() => store.get("o/a baz")).toThrow("invalid_repo_full_name"); + expect(() => store.put(profile("../etc"), AT_MS)).toThrow("invalid_repo_full_name"); + expect(() => store.put(profile("o/.."), AT_MS)).toThrow("invalid_repo_full_name"); + }); + it("exposes module-level get/put helpers backed by the default DB path", () => { vi.stubEnv( "LOOPOVER_MINER_CONTRIBUTION_PROFILE_CACHE_DB", diff --git a/test/unit/miner-prediction-ledger.test.ts b/test/unit/miner-prediction-ledger.test.ts index 404ae0d385..7c8fbc1df1 100644 --- a/test/unit/miner-prediction-ledger.test.ts +++ b/test/unit/miner-prediction-ledger.test.ts @@ -75,6 +75,20 @@ describe("miner prediction ledger (#4263)", () => { expect(() => ledger.appendPrediction({ ...VALID, readinessScore: Number.NaN })).toThrow(/invalid_readiness_score/); }); + // #7795: an unsafe path-traversal/invalid-character segment must be rejected here too, matching + // repo-clone.js's own validation, instead of being silently accepted and persisted as a ledger key -- + // for both the owner and repo segment independently. + it("rejects a repoFullName with a path-traversal or invalid-character segment", () => { + const ledger = tempLedger(); + // Both appendPrediction (write) and readPredictions (read) funnel through normalizeRepoFullName. + expect(() => ledger.appendPrediction({ ...VALID, repoFullName: "../etc" })).toThrow("invalid_repo_full_name"); // owner ".." invalid + expect(() => ledger.appendPrediction({ ...VALID, repoFullName: "o/.." })).toThrow("invalid_repo_full_name"); // repo ".." invalid + expect(() => ledger.appendPrediction({ ...VALID, repoFullName: "o baz/a" })).toThrow("invalid_repo_full_name"); + expect(() => ledger.appendPrediction({ ...VALID, repoFullName: "o/a baz" })).toThrow("invalid_repo_full_name"); + expect(() => ledger.readPredictions({ repoFullName: "../etc" })).toThrow("invalid_repo_full_name"); + expect(() => ledger.readPredictions({ repoFullName: "o/.." })).toThrow("invalid_repo_full_name"); + }); + it("scopes readPredictions by repo, preserving insertion order", () => { const ledger = tempLedger(); ledger.appendPrediction({ ...VALID, repoFullName: "owner/repo-a", targetId: 1 }); diff --git a/test/unit/miner-replay-snapshot.test.ts b/test/unit/miner-replay-snapshot.test.ts index 31a0c1cf11..35ae0b3680 100644 --- a/test/unit/miner-replay-snapshot.test.ts +++ b/test/unit/miner-replay-snapshot.test.ts @@ -456,4 +456,15 @@ describe("openReplaySnapshotStore (#3010) — round-trip persistence", () => { const store = tempStore(); expect(store.getSnapshot("acme/widgets", "nope")).toBeNull(); }); + + // #7795: an unsafe path-traversal/invalid-character segment must be rejected here too, matching + // repo-clone.js's own validation, instead of being silently accepted and used as a snapshot key -- + // for both the owner and repo segment independently. + it("rejects a repoFullName with a path-traversal or invalid-character segment", () => { + const store = tempStore(); + expect(() => store.getSnapshot("../etc", "abc123")).toThrow("invalid_repo_full_name"); // owner ".." invalid + expect(() => store.getSnapshot("o/..", "abc123")).toThrow("invalid_repo_full_name"); // repo ".." invalid + expect(() => store.getSnapshot("o baz/a", "abc123")).toThrow("invalid_repo_full_name"); + expect(() => store.getSnapshot("o/a baz", "abc123")).toThrow("invalid_repo_full_name"); + }); }); diff --git a/test/unit/miner-run-state.test.ts b/test/unit/miner-run-state.test.ts index cbbe71c202..409b1a05e2 100644 --- a/test/unit/miner-run-state.test.ts +++ b/test/unit/miner-run-state.test.ts @@ -136,6 +136,24 @@ describe("loopover-miner run-state store (#2289)", () => { } }); + // #7795: an unsafe path-traversal/invalid-character segment must be rejected here too, matching + // repo-clone.js's own validation, instead of being silently accepted and persisted as a state key -- + // for both the owner and repo segment independently. + it("rejects a repoFullName with a path-traversal or invalid-character segment", () => { + const store = initRunStateStore(join(tempRoot(), "run-state.sqlite3")); + try { + // Both getRunState (read) and setRunState (write) funnel through normalizeRepoFullName. + expect(() => store.getRunState("../etc")).toThrow("invalid_repo_full_name"); // owner ".." invalid + expect(() => store.getRunState("o/..")).toThrow("invalid_repo_full_name"); // repo ".." invalid + expect(() => store.setRunState("o baz/a", "idle")).toThrow("invalid_repo_full_name"); + expect(() => store.setRunState("o/a baz", "idle")).toThrow("invalid_repo_full_name"); + expect(() => store.setRunState("../etc", "idle")).toThrow("invalid_repo_full_name"); + expect(() => store.setRunState("o/..", "idle")).toThrow("invalid_repo_full_name"); + } finally { + store.close(); + } + }); + it("fails closed to null when a legacy table contains an unknown state", () => { const dbPath = join(tempRoot(), "legacy.sqlite3"); const legacy = new DatabaseSync(dbPath);