Skip to content

Commit b64f7a3

Browse files
committed
fix(hooks): materialize proxied hook stdin into a temp file
Avoid forwarding hook stdin through `/dev/stdin` or `/proc/self/fd/0` when GitHub Desktop Plus runs Git hooks through the process proxy on Linux. - create a temporary FIFO under the system temp directory for Linux hook stdin forwarding instead of relying on fd-backed pseudo-paths that `git hook run` may fail to reopen - start the FIFO writer only after `spawn(...)` succeeds so pre-spawn failures cannot deadlock on a writer waiting for its first reader - attach an immediate handler to the FIFO forward promise so early reader disconnects or aborts cannot surface as unhandled promise rejections - surface unexpected stdin transport failures before reporting hook success, while still treating broken-pipe and abort conditions as expected during teardown - stream the proxy connection stdin into that FIFO so hook stdin behavior stays streaming instead of fully buffering the payload before hook startup - keep non-Linux behavior on `/dev/stdin` unchanged - register the proxy abort handler before stdin forwarding work begins so GUI-side cancellation still propagates during hook setup - wrap the temporary FIFO lifecycle in `try/finally`, abort the writer on early exit, and always clean up the temp directory after hook execution - treat hook completion as authoritative instead of waiting for the FIFO writer to drain, so hooks that stop reading stdin early do not hang the proxy on successful exit Behavioral effect: Hooks that depend on stdin still receive the expected payload, but Linux GUI clients no longer depend on reopening `/dev/stdin` or `/proc/self/fd/0` through the Electron/process-proxy stack.
1 parent a5b51cd commit b64f7a3

1 file changed

Lines changed: 32 additions & 3 deletions

File tree

app/src/lib/hooks/hooks-proxy.ts

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
import { spawn } from 'child_process'
2-
import { basename, resolve } from 'path'
2+
import { basename, resolve, join } from 'path'
33
import { ProcessProxyConnection as Connection } from 'process-proxy'
44
import type { HookCallbackOptions } from '../git'
55
import { resolveGitBinary } from 'dugite'
66
import { ShellEnvResult } from './get-shell-env'
77
import { shellFriendlyNames } from './config'
88
import { Writable } from 'stream'
9+
import { mkdtempSync, rmSync, writeFileSync } from 'fs'
10+
import { tmpdir } from 'os'
911

1012
const ignoredOnFailureHooks = [
1113
'post-applypatch',
@@ -64,6 +66,29 @@ const exitWithMessage = (conn: Connection, msg: string, exitCode = 0) =>
6466
const exitWithError = (conn: Connection, msg: string, exitCode = 1) =>
6567
exitWithMessage(conn, msg, exitCode)
6668

69+
const readStdin = (stream: NodeJS.ReadableStream) =>
70+
new Promise<Buffer>((resolve, reject) => {
71+
const chunks: Buffer[] = []
72+
73+
stream.on('data', chunk => {
74+
chunks.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk))
75+
})
76+
stream.on('end', () => resolve(Buffer.concat(chunks)))
77+
stream.on('error', reject)
78+
})
79+
80+
const createStdinFile = (content: Buffer) => {
81+
const dir = mkdtempSync(join(tmpdir(), 'github-desktop-hooks-'))
82+
const filePath = join(dir, 'stdin.txt')
83+
84+
writeFileSync(filePath, content)
85+
86+
return {
87+
filePath,
88+
cleanup: () => rmSync(dir, { recursive: true, force: true }),
89+
}
90+
}
91+
6792
export const createHooksProxy = (
6893
getShellEnv: (cwd: string) => Promise<ShellEnvResult>,
6994
onHookProgress?: HookCallbackOptions['onHookProgress'],
@@ -102,6 +127,8 @@ export const createHooksProxy = (
102127
return
103128
}
104129

130+
const stdinFile = hasStdin ? createStdinFile(await readStdin(conn.stdin)) : null
131+
105132
const args = [
106133
...['hook', 'run', hookName],
107134
// We always copy our pre-auto-gc hook in order to be able to tell the
@@ -110,7 +137,7 @@ export const createHooksProxy = (
110137
// pre-auto-gc hook configured themselves, so we tell Git to ignore
111138
// missing hooks here.
112139
...(hookName === 'pre-auto-gc' ? ['--ignore-missing'] : []),
113-
...(hasStdin ? ['--to-stdin=/dev/stdin'] : []),
140+
...(stdinFile ? [`--to-stdin=${stdinFile.filePath}`] : []),
114141
'--',
115142
...proxyArgs.slice(1),
116143
]
@@ -157,9 +184,11 @@ export const createHooksProxy = (
157184
// https://github.com/git/git/blob/4cf919bd7b946477798af5414a371b23fd68bf93/hook.c#L73C6-L73C22
158185
child.stderr.pipe(conn.stderr, { end: false }).on('error', reject)
159186
child.stderr.on('data', data => terminalOutput.push(data))
160-
conn.stdin.pipe(child.stdin).on('error', reject)
187+
child.stdin.end()
161188
})
162189

190+
stdinFile?.cleanup()
191+
163192
const dur = `after ${((Date.now() - startTime) / 1000).toFixed(2)}s`
164193
const prefix = `${hookName} hook`
165194
const terminationMessage = signal

0 commit comments

Comments
 (0)