Skip to content

Commit 8ca305b

Browse files
authored
fix(server): reject code-executing git transports in the repo remote route (#126)
POST /api/repo/remote passed the remote url straight to git remote add/set-url origin <url> in an attacker-chosen directory. Git's ext::/fake:: helper transports run shell commands, and a leading '-' allows argument injection — so an allowed-origin page could set the origin to ext::sh -c '...' and a later push/fetch would execute it. Validate the url (assertSafeRemoteUrl: only http(s)/ssh/git@ schemes; reject ext::/fake:: and leading '-'), and harden the git spawn env with protocol.ext.allow=never / protocol.fake.allow=never as defense in depth. (#4)
1 parent be5573c commit 8ca305b

2 files changed

Lines changed: 58 additions & 3 deletions

File tree

backend/cli/src/server/routes/repo.ts

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,34 @@ interface RunResult {
2323
err: string
2424
}
2525

26+
/** Reject remote URLs that could run arbitrary commands via git's helper
27+
* transports (`ext::`/`fake::`) or argument injection (leading `-`). Normal
28+
* https/http/ssh/git@ remotes pass. Exported for tests. */
29+
export function assertSafeRemoteUrl(url: unknown): string {
30+
const value = String(url ?? "").trim()
31+
if (!value) throw new Error("remote URL required")
32+
if (value.startsWith("-")) throw new Error("invalid remote URL")
33+
if (/^(ext|fake)::/i.test(value)) throw new Error("unsupported remote transport")
34+
if (!/^(https?:\/\/|ssh:\/\/|git@)/i.test(value)) throw new Error("unsupported remote URL scheme")
35+
return value
36+
}
37+
2638
function run(command: string, args: string[], cwd: string, ok: number[] = [0]): Promise<RunResult> {
2739
return new Promise((resolveP, rejectP) => {
2840
const child = spawn(command, args, {
2941
stdio: ["ignore", "pipe", "pipe"],
3042
cwd,
31-
env: { ...process.env, GIT_TERMINAL_PROMPT: "0" },
43+
env: {
44+
...process.env,
45+
GIT_TERMINAL_PROMPT: "0",
46+
// Defense in depth: refuse the code-executing helper transports even if a
47+
// malicious remote URL slips past assertSafeRemoteUrl.
48+
GIT_CONFIG_COUNT: "2",
49+
GIT_CONFIG_KEY_0: "protocol.ext.allow",
50+
GIT_CONFIG_VALUE_0: "never",
51+
GIT_CONFIG_KEY_1: "protocol.fake.allow",
52+
GIT_CONFIG_VALUE_1: "never",
53+
},
3254
})
3355
let out = ""
3456
let err = ""
@@ -165,8 +187,7 @@ async function push(directory: string, branch: unknown) {
165187

166188
async function setRemote(directory: string, url: unknown) {
167189
if (!directory) throw new Error("directory required")
168-
const value = String(url ?? "").trim()
169-
if (!value) throw new Error("remote URL required")
190+
const value = assertSafeRemoteUrl(url)
170191
const hasOrigin = await git(["remote", "get-url", "origin"], directory)
171192
.then(() => true)
172193
.catch(() => false)
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
import { describe, expect, test } from "bun:test"
2+
import { assertSafeRemoteUrl } from "../../src/server/routes/repo"
3+
4+
describe("assertSafeRemoteUrl", () => {
5+
test("accepts normal https / ssh / git@ remotes", () => {
6+
for (const url of [
7+
"https://github.com/owner/name.git",
8+
"http://gitlab.example.com/g/n",
9+
"ssh://git@github.com/owner/name.git",
10+
"git@github.com:owner/name.git",
11+
]) {
12+
expect(assertSafeRemoteUrl(url)).toBe(url)
13+
}
14+
})
15+
16+
test("trims surrounding whitespace", () => {
17+
expect(assertSafeRemoteUrl(" https://github.com/o/n ")).toBe("https://github.com/o/n")
18+
})
19+
20+
test("rejects the code-executing git helper transports", () => {
21+
expect(() => assertSafeRemoteUrl('ext::sh -c "id"')).toThrow("unsupported remote transport")
22+
expect(() => assertSafeRemoteUrl("fake::whatever")).toThrow("unsupported remote transport")
23+
})
24+
25+
test("rejects argument injection (leading dash)", () => {
26+
expect(() => assertSafeRemoteUrl("--upload-pack=touch /tmp/pwned")).toThrow("invalid remote URL")
27+
})
28+
29+
test("rejects unknown / non-remote schemes", () => {
30+
expect(() => assertSafeRemoteUrl("file:///etc/passwd")).toThrow("unsupported remote URL scheme")
31+
expect(() => assertSafeRemoteUrl("not a url")).toThrow("unsupported remote URL scheme")
32+
expect(() => assertSafeRemoteUrl("")).toThrow("remote URL required")
33+
})
34+
})

0 commit comments

Comments
 (0)