fix: askpass handles onPrompt rejection, listen errors, and cleanup
- Wrap onPrompt in try/catch in connection handler; on rejection send
empty reply ("\n") so the git/ssh helper exits promptly instead of
hanging indefinitely.
- Reject the start() promise on server 'error' events (e.g. EADDRINUSE)
so callers get a fast failure rather than an eternal hang.
- stop() now unlinks the socket file and helper script to prevent stale-
socket errors on subsequent start() calls with the same dir.
- start() best-effort unlinks a pre-existing stale socket before listen().
- Tests: added onPrompt-rejection (with 3s timeout guard) and restart
(stop then start on same dir) test cases.
Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -8,7 +8,10 @@ import { AskpassBridge } from "./askpass";
|
|||||||
|
|
||||||
const run = promisify(execFile);
|
const run = promisify(execFile);
|
||||||
let bridge: AskpassBridge | undefined;
|
let bridge: AskpassBridge | undefined;
|
||||||
afterEach(() => bridge?.stop());
|
afterEach(async () => {
|
||||||
|
bridge?.stop();
|
||||||
|
bridge = undefined;
|
||||||
|
});
|
||||||
|
|
||||||
test("helper script round-trips a prompt to onPrompt and returns the answer", async () => {
|
test("helper script round-trips a prompt to onPrompt and returns the answer", async () => {
|
||||||
const dir = mkdtempSync(join(tmpdir(), "jp-ask-"));
|
const dir = mkdtempSync(join(tmpdir(), "jp-ask-"));
|
||||||
@@ -23,3 +26,58 @@ test("helper script round-trips a prompt to onPrompt and returns the answer", as
|
|||||||
expect(stdout.trim()).toBe("s3cret");
|
expect(stdout.trim()).toBe("s3cret");
|
||||||
rmSync(dir, { recursive: true, force: true });
|
rmSync(dir, { recursive: true, force: true });
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("onPrompt rejection: helper process completes promptly and returns empty answer", async () => {
|
||||||
|
const dir = mkdtempSync(join(tmpdir(), "jp-ask-reject-"));
|
||||||
|
bridge = new AskpassBridge({
|
||||||
|
dir,
|
||||||
|
onPrompt: async (_p) => { throw new Error("user cancelled"); },
|
||||||
|
});
|
||||||
|
const env = await bridge.start();
|
||||||
|
|
||||||
|
// The helper must exit within 3 seconds; if it hangs the test times out with a clear error.
|
||||||
|
const result = await Promise.race([
|
||||||
|
run(env.GIT_ASKPASS, ["Password:"], {
|
||||||
|
env: { ...process.env, JEKYLL_ASKPASS_SOCK: env.JEKYLL_ASKPASS_SOCK },
|
||||||
|
}).then(({ stdout }) => ({ timedOut: false, stdout }))
|
||||||
|
.catch(() => ({ timedOut: false, stdout: "" })),
|
||||||
|
new Promise<{ timedOut: true; stdout: string }>((resolve) =>
|
||||||
|
setTimeout(() => resolve({ timedOut: true, stdout: "" }), 3000)
|
||||||
|
),
|
||||||
|
]);
|
||||||
|
|
||||||
|
expect(result.timedOut, "helper process hung instead of exiting promptly").toBe(false);
|
||||||
|
// Empty answer is fine — git will fail fast on its own
|
||||||
|
expect(result.stdout.trim()).toBe("");
|
||||||
|
rmSync(dir, { recursive: true, force: true });
|
||||||
|
});
|
||||||
|
|
||||||
|
test("restart on same dir: stop() then start() again succeeds with no EADDRINUSE", async () => {
|
||||||
|
const dir = mkdtempSync(join(tmpdir(), "jp-ask-restart-"));
|
||||||
|
|
||||||
|
// First session
|
||||||
|
bridge = new AskpassBridge({
|
||||||
|
dir,
|
||||||
|
onPrompt: async (p) => (p.includes("user") ? "bob" : "pass1"),
|
||||||
|
});
|
||||||
|
const env1 = await bridge.start();
|
||||||
|
const { stdout: out1 } = await run(env1.GIT_ASKPASS, ["username:"], {
|
||||||
|
env: { ...process.env, JEKYLL_ASKPASS_SOCK: env1.JEKYLL_ASKPASS_SOCK },
|
||||||
|
});
|
||||||
|
expect(out1.trim()).toBe("bob");
|
||||||
|
bridge.stop();
|
||||||
|
bridge = undefined;
|
||||||
|
|
||||||
|
// Second session — same dir; must not throw EADDRINUSE
|
||||||
|
bridge = new AskpassBridge({
|
||||||
|
dir,
|
||||||
|
onPrompt: async (p) => (p.includes("Password") ? "newpass" : "carol"),
|
||||||
|
});
|
||||||
|
const env2 = await bridge.start();
|
||||||
|
const { stdout: out2 } = await run(env2.GIT_ASKPASS, ["Password for 'https://y':"], {
|
||||||
|
env: { ...process.env, JEKYLL_ASKPASS_SOCK: env2.JEKYLL_ASKPASS_SOCK },
|
||||||
|
});
|
||||||
|
expect(out2.trim()).toBe("newpass");
|
||||||
|
|
||||||
|
rmSync(dir, { recursive: true, force: true });
|
||||||
|
});
|
||||||
|
|||||||
@@ -1,5 +1,5 @@
|
|||||||
import { createServer, Server } from "node:net";
|
import { createServer, Server } from "node:net";
|
||||||
import { chmodSync, writeFileSync } from "node:fs";
|
import { chmodSync, unlinkSync, writeFileSync } from "node:fs";
|
||||||
import { join } from "node:path";
|
import { join } from "node:path";
|
||||||
|
|
||||||
export class AskpassBridge {
|
export class AskpassBridge {
|
||||||
@@ -18,15 +18,34 @@ export class AskpassBridge {
|
|||||||
this.sock = join(this.opts.dir, "askpass.sock");
|
this.sock = join(this.opts.dir, "askpass.sock");
|
||||||
this.script = join(this.opts.dir, "askpass.sh");
|
this.script = join(this.opts.dir, "askpass.sh");
|
||||||
|
|
||||||
|
// Best-effort removal of a stale socket from a previous unclean shutdown.
|
||||||
|
try { unlinkSync(this.sock); } catch { /* ignore ENOENT */ }
|
||||||
|
|
||||||
this.server = createServer({ allowHalfOpen: true }, (conn) => {
|
this.server = createServer({ allowHalfOpen: true }, (conn) => {
|
||||||
let buf = "";
|
let buf = "";
|
||||||
conn.on("data", (d) => (buf += d.toString()));
|
conn.on("data", (d) => (buf += d.toString()));
|
||||||
conn.on("end", async () => {
|
conn.on("end", () => {
|
||||||
const answer = await this.opts.onPrompt(buf.replace(/\n$/, ""));
|
// Wrap onPrompt in try/catch so a rejection never hangs the git/ssh process.
|
||||||
|
this.opts.onPrompt(buf.replace(/\n$/, "")).then(
|
||||||
|
(answer) => {
|
||||||
conn.end(answer.endsWith("\n") ? answer : answer + "\n");
|
conn.end(answer.endsWith("\n") ? answer : answer + "\n");
|
||||||
|
},
|
||||||
|
(_err) => {
|
||||||
|
// User cancelled or onPrompt threw — send empty reply so git fails fast.
|
||||||
|
conn.end("\n");
|
||||||
|
}
|
||||||
|
);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
// Fix: reject the promise on listen error (e.g. EADDRINUSE) instead of hanging.
|
||||||
|
await new Promise<void>((res, rej) => {
|
||||||
|
this.server!.once("error", rej);
|
||||||
|
this.server!.listen(this.sock, () => {
|
||||||
|
this.server!.removeListener("error", rej);
|
||||||
|
res();
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
await new Promise<void>((res) => this.server!.listen(this.sock, res));
|
|
||||||
|
|
||||||
// Helper: send argv[1] (the prompt) to the socket, print the reply.
|
// Helper: send argv[1] (the prompt) to the socket, print the reply.
|
||||||
// Uses a Node one-liner for reliable EOF signalling across platforms
|
// Uses a Node one-liner for reliable EOF signalling across platforms
|
||||||
@@ -56,5 +75,8 @@ export class AskpassBridge {
|
|||||||
stop(): void {
|
stop(): void {
|
||||||
this.server?.close();
|
this.server?.close();
|
||||||
this.server = undefined;
|
this.server = undefined;
|
||||||
|
// Clean up socket and helper script to prevent stale-socket EADDRINUSE on restart.
|
||||||
|
try { unlinkSync(this.sock); } catch { /* ignore ENOENT */ }
|
||||||
|
try { unlinkSync(this.script); } catch { /* ignore ENOENT */ }
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user