diff --git a/scripts/release.ts b/scripts/release.ts index 52f8c22547..846155027d 100644 --- a/scripts/release.ts +++ b/scripts/release.ts @@ -169,9 +169,43 @@ function sshTargetFromOrigin(originUrl: string): string | undefined { return undefined; } -/** `ssh://host/owner/repo` or the scp-like `user@host:owner/repo`. Used for both derivation and override validation. */ +/** + * `ssh://host/owner/repo` or the scp-like `user@host:owner/repo`. + * + * This check is also a log boundary: the accepted value is printed before the push and appears in + * the failure command. Parse URL userinfo instead of treating any `ssh://` string as safe, and + * reject the scp-like `user:password@host:path` lookalike before either sink can observe it. + */ function isSshRemote(value: string): boolean { - return /^ssh:\/\/[^/]+\/.+$/.test(value) || /^[^@\s/]+@[^:\s/]+:.+$/.test(value); + const trimmed = value.trim(); + if (!trimmed || /[\u0000-\u001f\u007f]/.test(trimmed)) return false; + + if (trimmed.startsWith("ssh://")) { + try { + const parsed = new URL(trimmed); + let decodedUsername: string; + try { + decodedUsername = decodeURIComponent(parsed.username); + } catch { + return false; + } + return parsed.protocol === "ssh:" + && parsed.hostname.length > 0 + && parsed.pathname.length > 1 + && parsed.password === "" + // The release deploy key uses GitHub's fixed SSH principal. Treat any other userinfo as + // credential-shaped rather than trying to distinguish a harmless username from a token. + && (decodedUsername === "" || decodedUsername === SSH_USER) + && parsed.search === "" + && parsed.hash === ""; + } catch { + return false; + } + } + + // scp-like syntax has no parser-level query/fragment boundary. Reject those delimiters rather + // than allowing a token-shaped suffix to reach the target log or failed-command output. + return /^git@[^:\s/?#]+:[^?#]+$/.test(trimmed); } /** Split out so the scp-like SSH target is assembled rather than written as an address literal. */ @@ -185,7 +219,7 @@ async function releasePushCommand(branch: string): Promise<{ command: string[]; // silently retarget a production release. Check the shape, and print the resolved target either // way so the destination is visible before the push rather than inferred afterwards. if (configured && !isSshRemote(configured)) { - console.error("✗ OCX_RELEASE_SSH_REPO is not an ssh:// or user@host:owner/repo remote; refusing to push."); + console.error("✗ OCX_RELEASE_SSH_REPO is not a credential-free ssh:// or git@host:owner/repo remote; refusing to push."); process.exit(1); } const slug = configured || sshTargetFromOrigin(await capture(["git", "remote", "get-url", "origin"])); diff --git a/tests/release-helper.test.ts b/tests/release-helper.test.ts index a659e4ba97..d1b8912f13 100644 --- a/tests/release-helper.test.ts +++ b/tests/release-helper.test.ts @@ -37,6 +37,10 @@ interface ReleaseScenario { originUrl?: string; } +interface SshInvocation { + args: string[]; +} + function writeExecutable(path: string, contents: string): void { writeFileSync(path, contents, "utf8"); chmodSync(path, 0o755); @@ -264,6 +268,51 @@ function runRelease(version: string, scenario: ReleaseScenario = {}) { return { calls, result }; } +/** + * Run the exact command string emitted by the release helper through real Git and a fake SSH. + * + * The release shim proves which string was placed in the environment, but Git owns the parsing + * contract for `GIT_SSH_COMMAND`. Exercising a real Git process here catches quoting that looks + * correct in text yet splits, substitutes, or reinterprets the private-key path before SSH sees it. + */ +function executeGitSshCommand(gitSshCommand: string): { calls: SshInvocation[]; result: ReturnType } { + const shimDir = mkdtempSync(join(tmpdir(), "ocx-release-ssh-")); + const logPath = join(shimDir, "ssh-log.jsonl"); + const jsPath = join(shimDir, "ssh.js"); + writeFileSync(logPath, "", "utf8"); + writeFileSync(jsPath, `import { appendFileSync } from "node:fs"; +appendFileSync(process.env.FAKE_SSH_LOG, JSON.stringify({ args: process.argv.slice(2) }) + "\\n"); +process.exit(0); +`, "utf8"); + + // Use a native executable directly on every platform. A Windows `.cmd` shim that forwards `%*` + // reparses quoting and can make a broken GIT_SSH_COMMAND look correct after the damage, turning + // this regression into a false green. Only replace the executable token; Git still parses the + // exact emitted `-i` argument and hostile key path. + expect(gitSshCommand.startsWith("ssh ")).toBe(true); + const quote = (value: string) => `"${value.replace(/(["\\`$])/g, "\\$1")}"`; + const nativeFakeCommand = `${quote(process.execPath)} ${quote(jsPath)}${gitSshCommand.slice(3)}`; + + const inheritedEnv = Object.fromEntries( + Object.entries(process.env).filter(([key]) => key !== "GIT_SSH" && key !== "GIT_SSH_COMMAND"), + ); + const result = spawnSync("git", ["ls-remote", "ssh://example.invalid/owner/repository.git"], { + cwd: repoRoot, + env: { + ...inheritedEnv, + FAKE_SSH_LOG: logPath, + GIT_SSH_COMMAND: nativeFakeCommand, + }, + encoding: "utf8", + }); + const raw = readFileSync(logPath, "utf8").trim(); + const calls = raw + ? raw.split(/\r?\n/).filter(Boolean).map(line => JSON.parse(line) as SshInvocation) + : []; + rmSync(shimDir, { recursive: true, force: true }); + return { calls, result }; +} + describe("release helper", () => { test("preflight runs the shared audit, typecheck, test suite, and privacy scan before version bump", () => { const { calls, result } = runRelease("9.9.9"); @@ -391,6 +440,25 @@ describe("release helper", () => { expect(push?.gitSshCommand).toBe('ssh -i "C:\\\\Users\\\\Jun Kim\\\\.ssh\\\\ocx release key" -o IdentitiesOnly=yes'); }); + test("Git passes the emitted deploy-key path to SSH as one literal argument", () => { + const keyPath = 'C:\\Users\\Jun Kim\\.ssh\\ocx "quoted" $HOME $(not-run) `not-run`; key'; + const { calls: releaseCalls } = runRelease("9.9.9", { + releaseSshKey: keyPath, + releaseSshRepo: sshTarget, + pendingBump: true, + }); + const push = releaseCalls.find(call => call.name === "git" && call.args[0] === "push"); + expect(push?.gitSshCommand).toBeDefined(); + + const { calls } = executeGitSshCommand(push?.gitSshCommand ?? ""); + expect(calls.length).toBeGreaterThan(0); + for (const call of calls) { + const identityIndex = call.args.indexOf("-i"); + expect(identityIndex).toBeGreaterThanOrEqual(0); + expect(call.args[identityIndex + 1]).toBe(keyPath); + } + }); + /** * The SSH target is derived from `origin` rather than hardcoded, so a fork's release pushes to * the fork instead of silently targeting upstream. @@ -436,6 +504,44 @@ describe("release helper", () => { expect(calls.find(call => call.name === "git" && call.args[0] === "push")).toBeUndefined(); }); + test("credential-bearing SSH targets are rejected without logging the credential", () => { + for (const scenario of [ + { releaseSshRepo: "ssh://git:SECRET@example.test/owner/repository.git" }, + { releaseSshRepo: "ssh://SECRET@example.test/owner/repository.git" }, + { releaseSshRepo: "ssh://git%3ASECRET@example.test/owner/repository.git" }, + { releaseSshRepo: "git@example.test:owner/repository.git?token=SECRET" }, + { originUrl: "ssh://git:SECRET@example.test/owner/repository.git" }, + { originUrl: "git:SECRET@example.test:owner/repository.git" }, + ]) { + const { calls, result } = runRelease("9.9.9", { + releaseSshKey: "/tmp/k", + pendingBump: true, + ...scenario, + }); + const output = `${result.stdout ?? ""}\n${result.stderr ?? ""}`; + expect(result.status).not.toBe(0); + expect(output).not.toContain("SECRET"); + expect(calls.find(call => call.name === "git" && call.args[0] === "push")).toBeUndefined(); + } + }); + + test("credential-free ssh URL and scp-like release targets remain accepted", () => { + for (const releaseSshRepo of [ + "ssh://git@example.test/owner/repository.git", + "ssh://example.test/owner/repository.git", + "git@example.test:owner/repository.git", + ]) { + const { calls, result } = runRelease("9.9.9", { + releaseSshKey: "/tmp/k", + releaseSshRepo, + pendingBump: true, + }); + expect(result.status).toBe(0); + expect(calls.find(call => call.name === "git" && call.args[0] === "push")?.args[1]) + .toBe(releaseSshRepo); + } + }); + test("an ssh origin is reused verbatim rather than rewritten", () => { const { calls } = runRelease("9.9.9", { releaseSshKey: "/tmp/k",