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); + }); +});