mirror of
https://github.com/oven-sh/bun
synced 2026-03-01 04:51:01 +01:00
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 <noreply@anthropic.com>
This commit is contained in:
@@ -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),
|
||||
|
||||
167
test/cli/install/git-committish-validation.test.ts
Normal file
167
test/cli/install/git-committish-validation.test.ts
Normal file
@@ -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);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user