mirror of https://github.com/garrytan/gstack.git
Merge PR #1368: pass cwd to git via execFileSync, not interpolation through /bin/sh
This commit is contained in:
commit
99402350db
|
|
@ -632,9 +632,16 @@ function extractContentText(rec: any): string {
|
||||||
function resolveGitRemote(cwd: string): string {
|
function resolveGitRemote(cwd: string): string {
|
||||||
if (!cwd) return "";
|
if (!cwd) return "";
|
||||||
try {
|
try {
|
||||||
const out = execSync(`git -C ${JSON.stringify(cwd)} remote get-url origin 2>/dev/null`, {
|
// execFileSync (no shell) so `cwd` cannot trigger command substitution.
|
||||||
|
// Transcript JSONL records are an untrusted surface (a poisoned `.cwd`
|
||||||
|
// value containing `"$(...)"` survived `JSON.stringify` interpolation
|
||||||
|
// into a `/bin/sh -c` context, since JSON quoting does not escape `$`
|
||||||
|
// or backticks). Mirrors the execFileSync pattern this module already
|
||||||
|
// uses for `gbrainAvailable()` (line 762) and `gbrainPutPage()` (line 816).
|
||||||
|
const out = execFileSync("git", ["-C", cwd, "remote", "get-url", "origin"], {
|
||||||
encoding: "utf-8",
|
encoding: "utf-8",
|
||||||
timeout: 2000,
|
timeout: 2000,
|
||||||
|
stdio: ["ignore", "pipe", "ignore"],
|
||||||
});
|
});
|
||||||
return canonicalizeRemote(out.trim());
|
return canonicalizeRemote(out.trim());
|
||||||
} catch {
|
} catch {
|
||||||
|
|
|
||||||
|
|
@ -205,6 +205,52 @@ describe("gstack-memory-ingest state file", () => {
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// ── Security: cwd in transcript JSONL must not reach a shell ─────────────
|
||||||
|
|
||||||
|
describe("gstack-memory-ingest security: untrusted cwd cannot trigger shell substitution", () => {
|
||||||
|
it("does not invoke /bin/sh when a transcript record contains $() in cwd", () => {
|
||||||
|
// Transcript JSONL is an untrusted surface — a record's `.cwd` value
|
||||||
|
// can be set by anyone who can write to ~/.claude/projects (cross-machine
|
||||||
|
// share, prompt-injection appending to the active session log, etc.).
|
||||||
|
// resolveGitRemote() must use execFileSync, not execSync with template
|
||||||
|
// interpolation, or `cwd="$(...)"` triggers command substitution under
|
||||||
|
// /bin/sh -c on the next ingest run.
|
||||||
|
const home = makeTestHome();
|
||||||
|
const gstackHome = join(home, ".gstack");
|
||||||
|
mkdirSync(gstackHome, { recursive: true });
|
||||||
|
|
||||||
|
const markerDir = mkdtempSync(join(tmpdir(), "gstack-mi-cwd-marker-"));
|
||||||
|
const marker = join(markerDir, "PWNED");
|
||||||
|
// Plain $(...) — what an attacker would write into a transcript record.
|
||||||
|
// execFileSync passes this verbatim to git as a -C argument; execSync
|
||||||
|
// (the prior code path) wrapped it in a /bin/sh -c template that ran
|
||||||
|
// the substitution.
|
||||||
|
const malicious = "$(touch " + marker + ")";
|
||||||
|
|
||||||
|
const record = JSON.stringify({
|
||||||
|
type: "user",
|
||||||
|
uuid: "11111111-1111-1111-1111-111111111111",
|
||||||
|
sessionId: "abc",
|
||||||
|
cwd: malicious,
|
||||||
|
timestamp: new Date().toISOString(),
|
||||||
|
message: { role: "user", content: "hi" },
|
||||||
|
});
|
||||||
|
writeClaudeCodeSession(home, "-tmp-target", "abc", record + "\n");
|
||||||
|
|
||||||
|
const r = runScript(["--incremental", "--quiet"], {
|
||||||
|
HOME: home,
|
||||||
|
GSTACK_HOME: gstackHome,
|
||||||
|
GSTACK_MEMORY_INGEST_NO_WRITE: "1",
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(r.exitCode).toBe(0);
|
||||||
|
expect(existsSync(marker)).toBe(false);
|
||||||
|
|
||||||
|
rmSync(home, { recursive: true, force: true });
|
||||||
|
rmSync(markerDir, { recursive: true, force: true });
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
// ── Transcript parser via re-import of the source module ───────────────────
|
// ── Transcript parser via re-import of the source module ───────────────────
|
||||||
|
|
||||||
describe("internal: parseTranscriptJsonl + buildTranscriptPage shape", () => {
|
describe("internal: parseTranscriptJsonl + buildTranscriptPage shape", () => {
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue