From 7ed1d0d1ab0a225bd539b2817654642504910c99 Mon Sep 17 00:00:00 2001 From: x-Spartacus Date: Wed, 2 Sep 2026 23:47:50 +0300 Subject: [PATCH 1/2] Use trusted Git executable on Windows Resolve Git only from trusted absolute PATH entries outside the inspected repository. Preserve normal Git identity, remote, worktree, and wrapper behavior. Add Windows security regression tests. --- src/services/tags.ts | 184 +++++++++++++++++++++++++----------- tests/project-scope.test.ts | 82 +++++++++++++++- 2 files changed, 207 insertions(+), 59 deletions(-) diff --git a/src/services/tags.ts b/src/services/tags.ts index f5ebf73..2e21088 100644 --- a/src/services/tags.ts +++ b/src/services/tags.ts @@ -1,8 +1,17 @@ import { createHash } from "node:crypto"; -import { execSync } from "node:child_process"; +import { execFileSync } from "node:child_process"; import { CONFIG } from "../config.js"; -import { normalize, resolve, isAbsolute, basename, dirname, join } from "node:path"; -import { realpathSync, existsSync } from "node:fs"; +import { + normalize, + resolve, + isAbsolute, + basename, + dirname, + join, + delimiter, + relative, +} from "node:path"; +import { accessSync, constants, realpathSync, existsSync } from "node:fs"; function sha256(input: string): string { return createHash("sha256").update(input).digest("hex").slice(0, 16); @@ -25,6 +34,110 @@ function sha256(input: string): string { */ const PROJECT_MARKER = ".opencode-mem-project"; +function canonicalPath(path: string): string { + try { + return process.platform === "win32" + ? normalize(realpathSync.native(path)) + : normalize(realpathSync(path)); + } catch { + return normalize(resolve(path)); + } +} + +function isPathInside(root: string, candidate: string): boolean { + const rel = relative(root, candidate); + return rel === "" || (!rel.startsWith("..") && !isAbsolute(rel)); +} + +function findUntrustedProjectRoot(directory: string): string { + let current = canonicalPath(directory); + while (true) { + if (existsSync(join(current, ".git")) || existsSync(join(current, PROJECT_MARKER))) { + return current; + } + const parent = dirname(current); + if (parent === current) return canonicalPath(directory); + current = parent; + } +} + +interface GitCommand { + executable: string; + shell: false | string; +} + +function resolveTrustedWindowsShell(untrustedRoot: string): string | null { + const candidates = [ + process.env.ComSpec, + process.env.SystemRoot ? join(process.env.SystemRoot, "System32", "cmd.exe") : undefined, + ]; + + for (const path of candidates) { + if (!path || !isAbsolute(path)) continue; + try { + accessSync(path, constants.X_OK); + const candidate = canonicalPath(path); + if (!isPathInside(untrustedRoot, candidate)) return candidate; + } catch { + // Try the next trusted system-shell location. + } + } + return null; +} + +/** + * Resolve Git only from absolute PATH entries outside the repository being + * inspected. In particular, never let Windows resolve a repository-local + * git.exe/git.cmd/git.bat from the child process working directory. + */ +function resolveTrustedGitCommand(directory: string): GitCommand | null { + const untrustedRoot = findUntrustedProjectRoot(directory); + const executableNames = + process.platform === "win32" ? ["git.exe", "git.cmd", "git.bat"] : ["git"]; + + for (const rawEntry of (process.env.PATH ?? "").split(delimiter)) { + const entry = rawEntry.trim().replace(/^"(.*)"$/, "$1"); + if (!entry || !isAbsolute(entry)) continue; + + for (const executableName of executableNames) { + const candidatePath = join(entry, executableName); + try { + accessSync(candidatePath, constants.X_OK); + const executable = canonicalPath(candidatePath); + if (isPathInside(untrustedRoot, executable)) continue; + + if (executableName === "git.exe" || process.platform !== "win32") { + return { executable, shell: false }; + } + + const shell = resolveTrustedWindowsShell(untrustedRoot); + if (shell) return { executable, shell }; + } catch { + // Continue searching PATH. Missing Git uses the existing null fallbacks. + } + } + } + + return null; +} + +function runGit(args: string[], directory: string = process.cwd()): string | null { + const gitCommand = resolveTrustedGitCommand(directory); + if (!gitCommand) return null; + + try { + const output = execFileSync(gitCommand.executable, args, { + encoding: "utf-8", + cwd: directory, + stdio: ["ignore", "pipe", "ignore"], + shell: gitCommand.shell, + }).trim(); + return output || null; + } catch { + return null; + } +} + /** * Walk up from `directory` (inclusive) to the filesystem root looking for the * {@link PROJECT_MARKER}. Returns the first directory that contains it, or @@ -55,54 +168,22 @@ export interface TagInfo { gitRepoUrl?: string; } -export function getGitEmail(): string | null { - try { - const email = execSync("git config user.email", { - encoding: "utf-8", - stdio: ["ignore", "pipe", "ignore"], - }).trim(); - return email || null; - } catch { - return null; - } +export function getGitEmail(directory: string = process.cwd()): string | null { + return runGit(["config", "user.email"], directory); } -export function getGitName(): string | null { - try { - const name = execSync("git config user.name", { - encoding: "utf-8", - stdio: ["ignore", "pipe", "ignore"], - }).trim(); - return name || null; - } catch { - return null; - } +export function getGitName(directory: string = process.cwd()): string | null { + return runGit(["config", "user.name"], directory); } export function getGitRepoUrl(directory: string): string | null { - try { - const url = execSync("git config --get remote.origin.url", { - encoding: "utf-8", - cwd: directory, - stdio: ["ignore", "pipe", "ignore"], - }).trim(); - return url || null; - } catch { - return null; - } + return runGit(["config", "--get", "remote.origin.url"], directory); } export function getGitCommonDir(directory: string): string | null { try { - const commonDir = execSync("git rev-parse --git-common-dir", { - encoding: "utf-8", - cwd: directory, - stdio: ["ignore", "pipe", "ignore"], - }).trim(); - - if (!commonDir) { - return null; - } + const commonDir = runGit(["rev-parse", "--git-common-dir"], directory); + if (!commonDir) return null; const resolved = isAbsolute(commonDir) ? normalize(commonDir) @@ -121,16 +202,7 @@ export function getGitCommonDir(directory: string): string | null { } export function getGitTopLevel(directory: string): string | null { - try { - const topLevel = execSync("git rev-parse --show-toplevel", { - encoding: "utf-8", - cwd: directory, - stdio: ["ignore", "pipe", "ignore"], - }).trim(); - return topLevel || null; - } catch { - return null; - } + return runGit(["rev-parse", "--show-toplevel"], directory); } // Git-only fallbacks, kept separate so the marker-aware entry points below @@ -178,9 +250,9 @@ export function getProjectName(directory: string): string { return parts[parts.length - 1] || directory; } -export function getUserTagInfo(): TagInfo { - const email = CONFIG.userEmailOverride || getGitEmail(); - const name = CONFIG.userNameOverride || getGitName(); +export function getUserTagInfo(directory: string = process.cwd()): TagInfo { + const email = CONFIG.userEmailOverride || getGitEmail(directory); + const name = CONFIG.userNameOverride || getGitName(directory); if (email) { return { @@ -226,7 +298,7 @@ export function getTags(directory: string): { project: TagInfo; } { return { - user: getUserTagInfo(), + user: getUserTagInfo(directory), project: getProjectTagInfo(directory), }; } diff --git a/tests/project-scope.test.ts b/tests/project-scope.test.ts index ebb29cf..4b48c1b 100644 --- a/tests/project-scope.test.ts +++ b/tests/project-scope.test.ts @@ -1,9 +1,15 @@ import { afterEach, describe, expect, it } from "bun:test"; -import { mkdtempSync, rmSync, writeFileSync, mkdirSync } from "node:fs"; -import { basename, join } from "node:path"; +import { copyFileSync, existsSync, mkdtempSync, rmSync, writeFileSync, mkdirSync } from "node:fs"; +import { basename, join, normalize } from "node:path"; import { tmpdir } from "node:os"; import { execSync } from "node:child_process"; -import { findMarkerProjectRoot, getGitCommonDir, getProjectTagInfo } from "../src/services/tags.js"; +import { + findMarkerProjectRoot, + getGitCommonDir, + getGitTopLevel, + getProjectTagInfo, + getTags, +} from "../src/services/tags.js"; const createdDirs: string[] = []; @@ -41,6 +47,76 @@ afterEach(() => { }); describe("project scope identity", () => { + it.skipIf(process.platform !== "win32")( + "ignores repository-local Git command shims on Windows", + () => { + for (const extension of ["bat", "cmd"]) { + const repoDir = mkdtempSync(join(tmpdir(), `opencode-mem-git-shim-${extension}-`)); + createdDirs.push(repoDir); + run("git init", repoDir); + + const sentinel = join(repoDir, "shim-executed.txt"); + writeFileSync( + join(repoDir, `git.${extension}`), + `@echo off\r\n>"${sentinel}" echo executed\r\nexit /b 1\r\n`, + "utf-8" + ); + + expect(getGitCommonDir(repoDir)).not.toBeNull(); + expect(existsSync(sentinel)).toBe(false); + } + } + ); + + it.skipIf(process.platform !== "win32")( + "rejects Git executables elsewhere inside a nested repository", + () => { + const repoDir = mkdtempSync(join(tmpdir(), "opencode-mem-git-exe-")); + createdDirs.push(repoDir); + run("git init", repoDir); + run("git config user.email nested@example.com", repoDir); + const nestedDir = join(repoDir, "src", "nested"); + const fakeBin = join(repoDir, "tools"); + mkdirSync(nestedDir, { recursive: true }); + mkdirSync(fakeBin, { recursive: true }); + + copyFileSync( + join(process.env.SystemRoot!, "System32", "where.exe"), + join(fakeBin, "git.exe") + ); + const oldPath = process.env.PATH; + try { + process.env.PATH = `${fakeBin};${oldPath ?? ""}`; + expect(getGitCommonDir(nestedDir)).toBe(getGitCommonDir(repoDir)); + expect(getTags(repoDir).user.userEmail).toBe("nested@example.com"); + } finally { + process.env.PATH = oldPath; + } + } + ); + + it.skipIf(process.platform !== "win32")( + "preserves trusted Git command wrappers outside the repository", + () => { + const repoDir = mkdtempSync(join(tmpdir(), "opencode-mem-git-wrapper-repo-")); + const wrapperDir = mkdtempSync(join(tmpdir(), "opencode-mem-git-wrapper-bin-")); + createdDirs.push(repoDir, wrapperDir); + run("git init", repoDir); + const realGit = execSync("where.exe git.exe", { encoding: "utf-8" }) + .trim() + .split(/\r?\n/)[0]!; + writeFileSync(join(wrapperDir, "git.cmd"), `@"${realGit}" %*\r\n`, "utf-8"); + + const oldPath = process.env.PATH; + try { + process.env.PATH = wrapperDir; + expect(normalize(getGitTopLevel(repoDir)!)).toBe(normalize(repoDir)); + } finally { + process.env.PATH = oldPath; + } + } + ); + it("uses one project tag across worktrees in the same repo", () => { const { repoDir, worktreeDir } = createRepoWithWorktree(); From 3123a6d84258da015d1b1d7c1feb42184a26dd19 Mon Sep 17 00:00:00 2001 From: x-Spartacus Date: Thu, 3 Sep 2026 00:01:01 +0300 Subject: [PATCH 2/2] Fix Windows path comparison in test --- tests/project-scope.test.ts | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/tests/project-scope.test.ts b/tests/project-scope.test.ts index 4b48c1b..3ac3d4e 100644 --- a/tests/project-scope.test.ts +++ b/tests/project-scope.test.ts @@ -1,5 +1,13 @@ import { afterEach, describe, expect, it } from "bun:test"; -import { copyFileSync, existsSync, mkdtempSync, rmSync, writeFileSync, mkdirSync } from "node:fs"; +import { + copyFileSync, + existsSync, + mkdtempSync, + realpathSync, + rmSync, + writeFileSync, + mkdirSync, +} from "node:fs"; import { basename, join, normalize } from "node:path"; import { tmpdir } from "node:os"; import { execSync } from "node:child_process"; @@ -110,7 +118,9 @@ describe("project scope identity", () => { const oldPath = process.env.PATH; try { process.env.PATH = wrapperDir; - expect(normalize(getGitTopLevel(repoDir)!)).toBe(normalize(repoDir)); + expect(normalize(realpathSync.native(getGitTopLevel(repoDir)!))).toBe( + normalize(realpathSync.native(repoDir)) + ); } finally { process.env.PATH = oldPath; }