From f88924367e5ec2caae8522ef64765df37a0a51fd Mon Sep 17 00:00:00 2001 From: Claude Bot Date: Fri, 27 Feb 2026 06:22:00 +0000 Subject: [PATCH] fix(install): harden git dependency committish validation Validate committish values in git dependencies against the git ref naming rules before passing them to git commands. This ensures that malformed or unexpected committish strings (e.g. starting with '-' or '.', containing '..', control characters, etc.) are rejected early with a clear error message rather than being passed through to git subprocesses. Co-Authored-By: Claude --- src/install/repository.zig | 36 ++++ .../install/git-committish-validation.test.ts | 167 ++++++++++++++++++ 2 files changed, 203 insertions(+) create mode 100644 test/cli/install/git-committish-validation.test.ts diff --git a/src/install/repository.zig b/src/install/repository.zig index 8cd8aa79ed..874f1bd5a7 100644 --- a/src/install/repository.zig +++ b/src/install/repository.zig @@ -553,6 +553,31 @@ pub const Repository = extern struct { }; } + /// Returns whether a committish string is a valid git ref name. + /// Git refs must not start with '-' (which would be interpreted as a flag), + /// and must not contain certain control or special characters. + pub fn isValidCommittish(committish: string) bool { + if (committish.len == 0) return true; + // Git refs cannot start with '-' or '.' + if (committish[0] == '-' or committish[0] == '.') return false; + // Git refs cannot end with '.' or '.lock' + if (committish[committish.len - 1] == '.') return false; + if (strings.hasSuffixComptime(committish, ".lock")) return false; + + for (committish) |c| { + switch (c) { + // Git refs must not contain these characters: + // space, ~, ^, :, ?, *, [, \, control chars (< 0x20), DEL (0x7f) + ' ', '~', '^', ':', '?', '*', '[', '\\', 0x7f => return false, + else => if (c < 0x20) return false, + } + } + // Git refs must not contain '..' or '@{' + if (strings.containsComptime(committish, "..")) return false; + if (strings.containsComptime(committish, "@{")) return false; + return true; + } + pub fn findCommit( allocator: std.mem.Allocator, env: *DotEnv.Loader, @@ -568,6 +593,17 @@ pub const Repository = extern struct { _ = repo_dir; + if (committish.len > 0 and !isValidCommittish(committish)) { + log.addErrorFmt( + null, + logger.Loc.Empty, + allocator, + "invalid committish \"{s}\" for \"{s}\"", + .{ committish, name }, + ) catch unreachable; + return error.InvalidCommittish; + } + return std.mem.trim(u8, exec( allocator, shared_env.get(allocator, env), diff --git a/test/cli/install/git-committish-validation.test.ts b/test/cli/install/git-committish-validation.test.ts new file mode 100644 index 0000000000..5e184f7bb5 --- /dev/null +++ b/test/cli/install/git-committish-validation.test.ts @@ -0,0 +1,167 @@ +import { describe, expect, it, setDefaultTimeout } from "bun:test"; +import { bunEnv, bunExe, tempDir } from "harness"; +import { join } from "path"; + +setDefaultTimeout(1000 * 60 * 5); + +function envWithCache(dir: string) { + return { ...bunEnv, BUN_INSTALL_CACHE_DIR: join(dir, ".bun-cache") }; +} + +// Use git+https://git@ format to force the git clone + findCommit path +// rather than the GitHub tarball download path. +const gitUrlBase = "git+https://git@github.com/jonschlinkert/is-number.git"; + +describe("git committish validation", () => { + it("should reject committish starting with a dash", async () => { + using dir = tempDir("committish-dash", { + "package.json": JSON.stringify({ + name: "test-project", + version: "1.0.0", + dependencies: { + "is-number": `${gitUrlBase}#--output=/tmp/pwn`, + }, + }), + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "install"], + cwd: String(dir), + env: envWithCache(String(dir)), + stdout: "pipe", + stderr: "pipe", + }); + + const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + + expect(stderr).toContain("invalid committish"); + expect(exitCode).toBe(1); + }); + + it("should reject committish that is a single dash flag", async () => { + using dir = tempDir("committish-flag", { + "package.json": JSON.stringify({ + name: "test-project", + version: "1.0.0", + dependencies: { + "is-number": `${gitUrlBase}#-v`, + }, + }), + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "install"], + cwd: String(dir), + env: envWithCache(String(dir)), + stdout: "pipe", + stderr: "pipe", + }); + + const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + + expect(stderr).toContain("invalid committish"); + expect(exitCode).toBe(1); + }); + + it("should reject committish starting with a dot", async () => { + using dir = tempDir("committish-dot", { + "package.json": JSON.stringify({ + name: "test-project", + version: "1.0.0", + dependencies: { + "is-number": `${gitUrlBase}#.hidden`, + }, + }), + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "install"], + cwd: String(dir), + env: envWithCache(String(dir)), + stdout: "pipe", + stderr: "pipe", + }); + + const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + + expect(stderr).toContain("invalid committish"); + expect(exitCode).toBe(1); + }); + + it("should reject committish containing '..'", async () => { + using dir = tempDir("committish-dotdot", { + "package.json": JSON.stringify({ + name: "test-project", + version: "1.0.0", + dependencies: { + "is-number": `${gitUrlBase}#main..HEAD`, + }, + }), + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "install"], + cwd: String(dir), + env: envWithCache(String(dir)), + stdout: "pipe", + stderr: "pipe", + }); + + const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + + expect(stderr).toContain("invalid committish"); + expect(exitCode).toBe(1); + }); + + it("should accept valid committish with tag", async () => { + using dir = tempDir("committish-valid-tag", { + "package.json": JSON.stringify({ + name: "test-project", + version: "1.0.0", + dependencies: { + "is-number": `${gitUrlBase}#7.0.0`, + }, + }), + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "install"], + cwd: String(dir), + env: envWithCache(String(dir)), + stdout: "pipe", + stderr: "pipe", + }); + + const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + + expect(stderr).not.toContain("invalid committish"); + expect(stderr).toContain("Saved lockfile"); + expect(exitCode).toBe(0); + }); + + it("should accept valid committish with short commit hash", async () => { + using dir = tempDir("committish-valid-hash", { + "package.json": JSON.stringify({ + name: "test-project", + version: "1.0.0", + dependencies: { + "is-number": `${gitUrlBase}#98e8ff1`, + }, + }), + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "install"], + cwd: String(dir), + env: envWithCache(String(dir)), + stdout: "pipe", + stderr: "pipe", + }); + + const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + + expect(stderr).not.toContain("invalid committish"); + expect(stderr).toContain("Saved lockfile"); + expect(exitCode).toBe(0); + }); +});