Skip to content

Commit ab5fe70

Browse files
committed
Address PR review feedback: shell-safe spawn, test cleanup
- Use spawn with shell:true instead of splitting gitCmd on whitespace, which broke quoted arguments and paths with spaces (Comment 1) - Remove unused unlinkSync import (Comment 5) - Add afterAll cleanup for temp test directory (Comment 6) https://claude.ai/code/session_01XGR7aJPmEZuK4j8r9HSodB
1 parent 1bf4612 commit ab5fe70

2 files changed

Lines changed: 10 additions & 8 deletions

File tree

src/__tests__/buildCredentialQuerier.test.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { buildCredentialQuerier } from "../server/buildCredentialQuerier"
2-
import { writeFileSync, unlinkSync, mkdtempSync } from "fs"
2+
import { writeFileSync, mkdtempSync, rmSync } from "fs"
33
import { join } from "path"
44
import { tmpdir } from "os"
55

@@ -12,6 +12,10 @@ beforeAll(() => {
1212
tmpDir = mkdtempSync(join(tmpdir(), "gcf-test-"))
1313
})
1414

15+
afterAll(() => {
16+
rmSync(tmpDir, { recursive: true, force: true })
17+
})
18+
1519
function writeMockScript(name: string, code: string): string {
1620
const path = join(tmpDir, name)
1721
writeFileSync(path, code)

src/server/buildCredentialQuerier.ts

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -66,14 +66,12 @@ async function runCredentialAction(
6666
const GIT_CREDENTIAL_ARG = "credential"
6767
const inputSerialized = gitCredentialIoApi.serialize(input)
6868

69-
const [cmd, ...cmdArgs] = gitCmd.split(/\s+/) as [string, ...string[]]
70-
const args = [...cmdArgs, GIT_CREDENTIAL_ARG, action]
69+
const fullCmd = `${gitCmd} ${GIT_CREDENTIAL_ARG} ${action}`
7170

72-
debug(`Running: ${cmd} ${args.join(" ")} (with stdin piping)`)
71+
debug(`Running: ${fullCmd} (with stdin piping)`)
7372

7473
const { stdout, stderr } = await spawnWithStdin(
75-
cmd,
76-
args,
74+
fullCmd,
7775
inputSerialized + "\n",
7876
env
7977
).catch(err => {
@@ -97,13 +95,13 @@ async function runCredentialAction(
9795

9896
function spawnWithStdin(
9997
cmd: string,
100-
args: string[],
10198
stdinData: string,
10299
env: NodeJS.ProcessEnv
103100
): Promise<{ stdout: string; stderr: string }> {
104101
return new Promise((resolve, reject) => {
105-
const child = spawn(cmd, args, {
102+
const child = spawn(cmd, {
106103
env,
104+
shell: true,
107105
stdio: ["pipe", "pipe", "pipe"]
108106
})
109107

0 commit comments

Comments
 (0)