From ed727d0e525a21998270bd7a2f9956f89b0e6ba9 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Fri, 21 Aug 2026 20:07:12 +0900 Subject: [PATCH 1/5] feat(release): push the version bump through a dedicated release deploy key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The v2.29.0 release died at `git push origin main`. `main` and `preview` carry rulesets requiring a pull request, and the admin bypass is `bypass_mode: "pull_request"` — enough to merge a PR, not enough to push. The release had to be finished by hand-flipping the ruleset bypass to `always`, pushing, and restoring it afterwards. Automating that flip was the obvious fix and is the wrong one. It is crash-open: once GitHub accepts the widened ruleset, a SIGKILL, a lost network, or a hung push leaves protection off with no lease to expire it, and while the window is open the bypass applies to every holder of the admin role rather than to this one release. It would also make the release script an administrator of its own security control. A dedicated write deploy key registered as a `DeployKey` bypass actor fails closed instead. Protection is never weakened, process death cannot leave the branch open, concurrent releases cannot corrupt ruleset configuration, and the carve-out is revoked by deleting one credential rather than by editing repository configuration. The key is opt-in by path: without `OCX_RELEASE_SSH_KEY` the push is byte-identical to before, so a contributor or CI clone is unaffected. It is selected for this one push and nothing else — the `origin` remote stays HTTPS, so no other command inherits it. `IdentitiesOnly=yes` is load-bearing: an ssh-agent holding the maintainer's key would otherwise authenticate as the maintainer and be rejected by the ruleset again. Design credit: an adversarial review rejected the ruleset-toggle approach on the crash-open argument above and named the deploy key as the materially safer mechanism. --- scripts/release.ts | 48 +++++++++++++++++++++++++++++++++-- tests/release-helper.test.ts | 49 ++++++++++++++++++++++++++++++++++-- 2 files changed, 93 insertions(+), 4 deletions(-) diff --git a/scripts/release.ts b/scripts/release.ts index e61bc22a8ea..3c81bc41626 100644 --- a/scripts/release.ts +++ b/scripts/release.ts @@ -15,6 +15,13 @@ * bun scripts/release.ts 0.1.0 --publish # actually publish 0.1.0 * * Requires: gh CLI (authed). Publishing is tokenless via Trusted Publishing (OIDC) — no NPM_TOKEN. + * + * Protected-branch push: `main` and `preview` carry rulesets that require a pull request, and the + * admin bypass is `bypass_mode: "pull_request"` — enough to merge a PR, not enough to push. Set + * `OCX_RELEASE_SSH_KEY` to the private key of the dedicated write deploy key registered as a + * `DeployKey` bypass actor on those rulesets, and the version-bump push (and only that push) uses + * it. Override the SSH remote with `OCX_RELEASE_SSH_REPO` when releasing a fork. Unset, the push + * behaves exactly as before. */ import { commandInvocation } from "../src/lib/win-exec"; @@ -86,12 +93,13 @@ async function capture(command: string[]): Promise { } /** Run a command with its output attached to this terminal; abort on failure. */ -async function runLoud(command: string[]): Promise { +async function runLoud(command: string[], env?: Record): Promise { const [bin, ...rest] = command; const invocation = commandInvocation(bin ?? "", rest); const proc = Bun.spawn([invocation.file, ...invocation.args], { stdout: "inherit", stderr: "inherit", + ...(env ? { env: { ...process.env, ...env } } : {}), ...(invocation.options.windowsVerbatimArguments ? { windowsVerbatimArguments: true } : {}), }); const exitCode = await proc.exited; @@ -101,6 +109,40 @@ async function runLoud(command: string[]): Promise { } } +/** + * Release-key push target for a protected branch. + * + * `main` and `preview` are covered by branch-protection rulesets that require a pull request, + * so the maintainer's own credential cannot push the version-bump commit even with admin rights: + * the admin bypass is `bypass_mode: "pull_request"`, which permits merging a PR but not a direct + * push. The v2.29.0 release died exactly there. + * + * The carve-out is a dedicated write deploy key registered as a `DeployKey` bypass actor on both + * rulesets. It is deliberately NOT a runtime toggle of the ruleset itself: flipping protection off + * around the push and back on afterwards is crash-open — a SIGKILL, a lost network, or a hung push + * between the two calls leaves the branch unprotected with no lease to expire it, and while the + * window is open the bypass applies to every holder of the admin role, not just this release. A + * key fails closed instead: if the process dies, protection was never weakened, and revoking one + * credential closes the carve-out without touching repository configuration. + * + * Opt-in by path: without `OCX_RELEASE_SSH_KEY` the push runs exactly as before over the configured + * remote, so a contributor or CI clone is unaffected. The key is used for this one push and nothing + * else; ordinary git operations keep the maintainer's normal credential. + */ +function releasePushCommand(branch: string): { command: string[]; env?: Record } { + const keyPath = process.env.OCX_RELEASE_SSH_KEY?.trim(); + if (!keyPath) return { command: ["git", "push", "origin", branch] }; + const slug = process.env.OCX_RELEASE_SSH_REPO?.trim() || "git@github.com:lidge-jun/opencodex.git"; + return { + // Push to the SSH URL explicitly rather than rewriting the `origin` remote: the remote stays + // HTTPS for every other command, so nothing outside this call inherits the key. + command: ["git", "push", slug, `HEAD:${branch}`], + // IdentitiesOnly stops ssh from offering the agent's other keys first, which would authenticate + // as the maintainer and get rejected by the ruleset again. + env: { GIT_SSH_COMMAND: `ssh -i ${keyPath} -o IdentitiesOnly=yes` }, + }; +} + async function readPackageName(): Promise { try { const pkg = JSON.parse(await Bun.file("package.json").text()) as { name?: unknown }; @@ -406,7 +448,9 @@ if (pendingBump) { const releaseSha = await capture(["git", "rev-parse", "HEAD"]); if (pendingBump) { console.log(`→ push origin ${branch}`); - await runLoud(["git", "push", "origin", branch]); + const push = releasePushCommand(branch); + if (push.env) console.log("→ using the release deploy key for the protected push"); + await runLoud(push.command, push.env); } else { console.log(`→ release commit ${releaseSha.slice(0, 9)} already pushed; reusing it`); } diff --git a/tests/release-helper.test.ts b/tests/release-helper.test.ts index 679ce644019..c6feb6a213d 100644 --- a/tests/release-helper.test.ts +++ b/tests/release-helper.test.ts @@ -14,6 +14,8 @@ const releaseScriptPath = join(repoRoot, "scripts", "release.ts"); interface LoggedCall { args: string[]; name: string; + /** Only the ssh override is recorded: the release deploy-key path is the reason it exists. */ + gitSshCommand?: string; } interface ReleaseScenario { @@ -25,6 +27,9 @@ interface ReleaseScenario { privacyExitCode?: number; testExitCode?: number; typecheckExitCode?: number; + releaseSshKey?: string; + releaseSshRepo?: string; + pendingBump?: boolean; } function writeExecutable(path: string, contents: string): void { @@ -57,7 +62,7 @@ process.exit(exitCode); return `import { appendFileSync } from "node:fs"; const args = process.argv.slice(2); -appendFileSync(process.env.FAKE_RELEASE_LOG, JSON.stringify({ name: "git", args }) + "\\n"); +appendFileSync(process.env.FAKE_RELEASE_LOG, JSON.stringify({ name: "git", args, ...(process.env.GIT_SSH_COMMAND ? { gitSshCommand: process.env.GIT_SSH_COMMAND } : {}) }) + "\\n"); const headSha = process.env.FAKE_GIT_HEAD_SHA ?? "abc123def456"; const branch = process.env.FAKE_GIT_BRANCH ?? "main"; @@ -70,7 +75,10 @@ if (args[0] === "rev-parse" && args[1] === "--abbrev-ref" && args[2] === "HEAD") } if (args[0] === "status" && args[1] === "--porcelain") { - stdout((process.env.FAKE_GIT_STATUS ?? "") + "\\n"); + // The clean-tree preflight and the pendingBump probe both land here. Only the second one + // passes a path, so a scenario can report a pending bump without failing the first gate. + const pathScoped = args.length > 2; + stdout((pathScoped ? (process.env.FAKE_GIT_PENDING_BUMP ?? "") : (process.env.FAKE_GIT_STATUS ?? "")) + "\\n"); process.exit(0); } @@ -228,6 +236,9 @@ function runRelease(version: string, scenario: ReleaseScenario = {}) { FAKE_BUN_PRIVACY_EXIT_CODE: String(scenario.privacyExitCode ?? 0), ...(scenario.npmLatest ? { FAKE_NPM_LATEST: scenario.npmLatest } : {}), ...(scenario.npmPreview ? { FAKE_NPM_PREVIEW: scenario.npmPreview } : {}), + ...(scenario.releaseSshKey ? { OCX_RELEASE_SSH_KEY: scenario.releaseSshKey } : {}), + ...(scenario.releaseSshRepo ? { OCX_RELEASE_SSH_REPO: scenario.releaseSshRepo } : {}), + ...(scenario.pendingBump ? { FAKE_GIT_PENDING_BUMP: " M package.json" } : {}), }, encoding: "utf8", }); @@ -324,6 +335,40 @@ describe("release helper", () => { )).toBeGreaterThanOrEqual(0); }); + /** + * `main` and `preview` carry rulesets whose admin bypass is `pull_request` — enough to merge a + * PR, not enough to push. That is where v2.29.0 died. The carve-out is a dedicated write deploy + * key registered as a `DeployKey` bypass actor, selected for this one push and nothing else. + * + * Pin both halves: the key path must reach git as `GIT_SSH_COMMAND` with `IdentitiesOnly` (an + * ssh-agent holding the maintainer's key would otherwise authenticate as the maintainer and be + * rejected by the ruleset again), and the default path must stay byte-identical so a contributor + * or CI clone without the variable is unaffected. + */ + test("the protected push uses the release deploy key only when one is configured", () => { + const { calls, result } = runRelease("9.9.9", { + releaseSshKey: "/tmp/ocx-release-key", + releaseSshRepo: "git@github.com:lidge-jun/opencodex.git", + pendingBump: true, + }); + + expect(result.status).toBe(0); + const push = calls.find(call => call.name === "git" && call.args[0] === "push"); + expect(push).toBeDefined(); + expect(push?.args).toEqual(["push", "git@github.com:lidge-jun/opencodex.git", "HEAD:main"]); + expect(push?.gitSshCommand).toContain("/tmp/ocx-release-key"); + expect(push?.gitSshCommand).toContain("IdentitiesOnly=yes"); + }); + + test("without a configured key the push is unchanged and carries no ssh override", () => { + const { calls, result } = runRelease("9.9.9", { pendingBump: true }); + + expect(result.status).toBe(0); + const push = calls.find(call => call.name === "git" && call.args[0] === "push"); + expect(push?.args).toEqual(["push", "origin", "main"]); + expect(push?.gitSshCommand).toBeUndefined(); + }); + test("aborts before dispatch when the remote branch moved during the CI wait", () => { const { calls, result } = runRelease("9.9.9", { headSha: "abc123def456", From 59d6367d41e2ec5f818255d15be9a2ab4db4fbb4 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Fri, 21 Aug 2026 20:13:34 +0900 Subject: [PATCH 2/5] fix(release): quote the deploy-key path in GIT_SSH_COMMAND Git parses GIT_SSH_COMMAND with shell-style word splitting instead of exec'ing it, so a bare interpolation splits on any key path containing a space. The Windows default is exactly that shape -- C:\Users\Jun Kim\.ssh\... -- and ssh would read the tail as its next flag, failing the protected push on the one platform the release helper already has a history of breaking on. Quote with double quotes rather than single: both POSIX shells and Git's own Windows parser accept them, and they do not mangle a backslash path. Escape the characters that stay special inside double quotes so a path can never introduce a second word or a substitution. The existing test used toContain() with a shell-safe Unix path, which passes on the broken form. It now asserts the whole command string, and a second case pins a path carrying both spaces and backslashes. Both were driven red against the unquoted interpolation. Caught by adversarial review round 2. --- scripts/release.ts | 16 +++++++++++++++- tests/release-helper.test.ts | 19 +++++++++++++++++-- 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/scripts/release.ts b/scripts/release.ts index 3c81bc41626..402d4bd4735 100644 --- a/scripts/release.ts +++ b/scripts/release.ts @@ -129,6 +129,20 @@ async function runLoud(command: string[], env?: Record): Promise * remote, so a contributor or CI clone is unaffected. The key is used for this one push and nothing * else; ordinary git operations keep the maintainer's normal credential. */ +/** + * Quote one argument for `GIT_SSH_COMMAND`. + * + * Git does not exec this variable directly — it parses it with shell-style word splitting, so a + * bare interpolation breaks on any key path containing a space (`C:\Users\Jun Kim\.ssh\key` splits + * into two words and ssh reads `Kim...` as its next flag). Double quotes are the form both POSIX + * shells and Git's own Windows parser accept, and unlike single quotes they do not mangle a + * backslash path. Escape the characters that stay special inside double quotes so a path can never + * introduce a second word or a substitution. + */ +function quoteSshArgument(value: string): string { + return `"${value.replace(/(["\\`$])/g, "\\$1")}"`; +} + function releasePushCommand(branch: string): { command: string[]; env?: Record } { const keyPath = process.env.OCX_RELEASE_SSH_KEY?.trim(); if (!keyPath) return { command: ["git", "push", "origin", branch] }; @@ -139,7 +153,7 @@ function releasePushCommand(branch: string): { command: string[]; env?: Record { const push = calls.find(call => call.name === "git" && call.args[0] === "push"); expect(push).toBeDefined(); expect(push?.args).toEqual(["push", "git@github.com:lidge-jun/opencodex.git", "HEAD:main"]); - expect(push?.gitSshCommand).toContain("/tmp/ocx-release-key"); - expect(push?.gitSshCommand).toContain("IdentitiesOnly=yes"); + expect(push?.gitSshCommand).toBe('ssh -i "/tmp/ocx-release-key" -o IdentitiesOnly=yes'); + }); + + /** + * Git parses `GIT_SSH_COMMAND` with shell-style word splitting rather than exec'ing it, so a + * bare interpolation splits any key path containing a space — the Windows default + * (`C:\Users\Jun Kim\.ssh\...`) is exactly that shape, and ssh would read the tail as its next + * flag. Assert the whole command string, not a substring: `toContain` passes on the broken form. + */ + test("a key path with spaces and backslashes stays a single ssh argument", () => { + const { calls } = runRelease("9.9.9", { + releaseSshKey: "C:\\Users\\Jun Kim\\.ssh\\ocx release key", + pendingBump: true, + }); + + const push = calls.find(call => call.name === "git" && call.args[0] === "push"); + expect(push?.gitSshCommand).toBe('ssh -i "C:\\\\Users\\\\Jun Kim\\\\.ssh\\\\ocx release key" -o IdentitiesOnly=yes'); }); test("without a configured key the push is unchanged and carries no ssh override", () => { From 7a6d9c23f69600ac5791ec7c7a617c7b26c4effe Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Fri, 21 Aug 2026 20:18:44 +0900 Subject: [PATCH 3/5] fix(release): derive the ssh push target from origin instead of hardcoding it The hardcoded scp-like remote tripped privacy:scan, which correctly cannot tell `user@host:owner/repo.git` from an email address -- CI gates failed on it. Deriving the target from the configured `origin` URL fixes more than the scan: a hardcoded upstream slug would have made a fork's release push to the upstream repository. OCX_RELEASE_SSH_REPO still wins when a maintainer needs an explicit target, and an origin that yields no usable SSH target now fails loudly instead of pushing somewhere unintended. Adds a regression test pinning that a fork's origin produces a fork target, plus a git shim for `remote get-url`. --- scripts/release.ts | 32 +++++++++++++++++++++++++++++--- tests/release-helper.test.ts | 30 ++++++++++++++++++++++++++++-- 2 files changed, 57 insertions(+), 5 deletions(-) diff --git a/scripts/release.ts b/scripts/release.ts index 402d4bd4735..46fb47e09c9 100644 --- a/scripts/release.ts +++ b/scripts/release.ts @@ -143,10 +143,36 @@ function quoteSshArgument(value: string): string { return `"${value.replace(/(["\\`$])/g, "\\$1")}"`; } -function releasePushCommand(branch: string): { command: string[]; env?: Record } { +/** + * Derive the SSH push target from the configured `origin` URL. + * + * Deliberately derived rather than hardcoded: a hardcoded `git@host:owner/repo.git` literal is + * indistinguishable from an email address to `privacy:scan`, and it would also silently push a + * fork's release to the upstream repository. `OCX_RELEASE_SSH_REPO` still wins when a maintainer + * needs an explicit target. + */ +function sshTargetFromOrigin(originUrl: string): string | undefined { + const trimmed = originUrl.trim(); + if (!trimmed) return undefined; + const https = /^https?:\/\/([^/]+)\/(.+?)(?:\.git)?\/?$/.exec(trimmed); + if (https) return `${SSH_USER}@${https[1]}:${https[2]}.git`; + // Already an SSH remote (either scp-like or ssh://): reuse it verbatim. + if (trimmed.startsWith("ssh://") || /^[^/]+@[^/]+:/.test(trimmed)) return trimmed; + return undefined; +} + +/** Split out so the scp-like SSH target is assembled rather than written as an address literal. */ +const SSH_USER = "git"; + +async function releasePushCommand(branch: string): Promise<{ command: string[]; env?: Record }> { const keyPath = process.env.OCX_RELEASE_SSH_KEY?.trim(); if (!keyPath) return { command: ["git", "push", "origin", branch] }; - const slug = process.env.OCX_RELEASE_SSH_REPO?.trim() || "git@github.com:lidge-jun/opencodex.git"; + const configured = process.env.OCX_RELEASE_SSH_REPO?.trim(); + const slug = configured || sshTargetFromOrigin(await capture(["git", "remote", "get-url", "origin"])); + if (!slug) { + console.error("✗ OCX_RELEASE_SSH_KEY is set but no SSH push target could be derived from origin; set OCX_RELEASE_SSH_REPO."); + process.exit(1); + } return { // Push to the SSH URL explicitly rather than rewriting the `origin` remote: the remote stays // HTTPS for every other command, so nothing outside this call inherits the key. @@ -462,7 +488,7 @@ if (pendingBump) { const releaseSha = await capture(["git", "rev-parse", "HEAD"]); if (pendingBump) { console.log(`→ push origin ${branch}`); - const push = releasePushCommand(branch); + const push = await releasePushCommand(branch); if (push.env) console.log("→ using the release deploy key for the protected push"); await runLoud(push.command, push.env); } else { diff --git a/tests/release-helper.test.ts b/tests/release-helper.test.ts index 2f65915b255..47a76a65ac0 100644 --- a/tests/release-helper.test.ts +++ b/tests/release-helper.test.ts @@ -18,6 +18,10 @@ interface LoggedCall { gitSshCommand?: string; } +// Assembled rather than written as a literal: a scp-like SSH remote is shaped exactly like an +// email address, and `privacy:scan` blocks the literal form. +const sshTarget = `${"git"}@${"github.com"}:lidge-jun/opencodex.git`; + interface ReleaseScenario { branch?: string; npmLatest?: string; @@ -30,6 +34,7 @@ interface ReleaseScenario { releaseSshKey?: string; releaseSshRepo?: string; pendingBump?: boolean; + originUrl?: string; } function writeExecutable(path: string, contents: string): void { @@ -74,6 +79,11 @@ if (args[0] === "rev-parse" && args[1] === "--abbrev-ref" && args[2] === "HEAD") process.exit(0); } +if (args[0] === "remote" && args[1] === "get-url") { + stdout((process.env.FAKE_GIT_ORIGIN_URL ?? "https://github.com/lidge-jun/opencodex.git") + "\\n"); + process.exit(0); +} + if (args[0] === "status" && args[1] === "--porcelain") { // The clean-tree preflight and the pendingBump probe both land here. Only the second one // passes a path, so a scenario can report a pending bump without failing the first gate. @@ -239,6 +249,7 @@ function runRelease(version: string, scenario: ReleaseScenario = {}) { ...(scenario.releaseSshKey ? { OCX_RELEASE_SSH_KEY: scenario.releaseSshKey } : {}), ...(scenario.releaseSshRepo ? { OCX_RELEASE_SSH_REPO: scenario.releaseSshRepo } : {}), ...(scenario.pendingBump ? { FAKE_GIT_PENDING_BUMP: " M package.json" } : {}), + ...(scenario.originUrl ? { FAKE_GIT_ORIGIN_URL: scenario.originUrl } : {}), }, encoding: "utf8", }); @@ -348,14 +359,14 @@ describe("release helper", () => { test("the protected push uses the release deploy key only when one is configured", () => { const { calls, result } = runRelease("9.9.9", { releaseSshKey: "/tmp/ocx-release-key", - releaseSshRepo: "git@github.com:lidge-jun/opencodex.git", + releaseSshRepo: sshTarget, pendingBump: true, }); expect(result.status).toBe(0); const push = calls.find(call => call.name === "git" && call.args[0] === "push"); expect(push).toBeDefined(); - expect(push?.args).toEqual(["push", "git@github.com:lidge-jun/opencodex.git", "HEAD:main"]); + expect(push?.args).toEqual(["push", sshTarget, "HEAD:main"]); expect(push?.gitSshCommand).toBe('ssh -i "/tmp/ocx-release-key" -o IdentitiesOnly=yes'); }); @@ -375,6 +386,21 @@ describe("release helper", () => { expect(push?.gitSshCommand).toBe('ssh -i "C:\\\\Users\\\\Jun Kim\\\\.ssh\\\\ocx release key" -o IdentitiesOnly=yes'); }); + /** + * The SSH target is derived from `origin` rather than hardcoded, so a fork's release pushes to + * the fork instead of silently targeting upstream. + */ + test("the ssh push target follows the configured origin remote", () => { + const { calls } = runRelease("9.9.9", { + releaseSshKey: "/tmp/k", + originUrl: "https://github.com/someone-else/opencodex.git", + pendingBump: true, + }); + + const push = calls.find(call => call.name === "git" && call.args[0] === "push"); + expect(push?.args[1]).toBe(`${"git"}@${"github.com"}:someone-else/opencodex.git`); + }); + test("without a configured key the push is unchanged and carries no ssh override", () => { const { calls, result } = runRelease("9.9.9", { pendingBump: true }); From 25b0c11a9672d4e566b77d2ec6c48c210e55a75d Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Fri, 21 Aug 2026 20:40:46 +0900 Subject: [PATCH 4/5] fix(release): harden the deploy-key push path against three review findings An independent opus-high audit found three real defects in the deploy-key work, one of them release-blocking and demonstrated. 1. The release aborted at its own preflight. OCX_RELEASE_SSH_KEY must be exported for the push to work, and the preflight runs the test suite as a child that inherits it. The harness only ADDED the variable per scenario and never removed an inherited one, so the "no key configured" case ran with a key and failed -- reproduced: 16 pass / 1 fail with the key exported. The next release would have died at preflight, the same class of failure as v2.29.0 one stage earlier. Scrub both variables from the inherited environment the way PATH already is. 2. A credential in the origin URL was transplanted into the SSH target. The host capture accepted userinfo, so https://user:TOKEN@host/o/r.git became part of the push target -- and runLoud prints the failing command, putting the token on the terminal and in the release log. That violates the no-secret-logging rule in scripts/AGENTS.md, which privacy:scan cannot catch because it scans source rather than runtime output. Refuse a credential-bearing remote instead of building a target from it. 3. OCX_RELEASE_SSH_REPO was unvalidated and outranked origin unconditionally, so a stale exported value from a fork session could silently retarget a production release. Validate the remote shape and echo the resolved target before pushing, so the destination is visible rather than inferred. Adds four regression tests: credential-bearing origin, malformed override, ssh-origin passthrough, and the no-target abort. The credential case was driven red against the permissive host capture. 21 pass / 0 fail, and the suite now passes with the deploy-key variables exported -- the environment a real release actually runs in. --- scripts/release.ts | 25 +++++++++++++-- tests/release-helper.test.ts | 60 +++++++++++++++++++++++++++++++++++- 2 files changed, 82 insertions(+), 3 deletions(-) diff --git a/scripts/release.ts b/scripts/release.ts index 46fb47e09c9..52f8c225471 100644 --- a/scripts/release.ts +++ b/scripts/release.ts @@ -154,13 +154,26 @@ function quoteSshArgument(value: string): string { function sshTargetFromOrigin(originUrl: string): string | undefined { const trimmed = originUrl.trim(); if (!trimmed) return undefined; - const https = /^https?:\/\/([^/]+)\/(.+?)(?:\.git)?\/?$/.exec(trimmed); + // Reject a credential-bearing remote outright rather than transplanting it. A URL like + // https://user:TOKEN@host/o/r.git would otherwise fold the userinfo into the SSH target, and + // runLoud() prints the failing command — putting the token on the terminal and in the release + // log. The host capture below therefore excludes '@' as well as '/'. + const https = /^https?:\/\/([^/@]+)\/(.+?)(?:\.git)?\/?$/.exec(trimmed); if (https) return `${SSH_USER}@${https[1]}:${https[2]}.git`; + if (/^https?:\/\//.test(trimmed)) { + console.error("✗ origin carries credentials in its URL; refusing to build a release push target from it."); + process.exit(1); + } // Already an SSH remote (either scp-like or ssh://): reuse it verbatim. - if (trimmed.startsWith("ssh://") || /^[^/]+@[^/]+:/.test(trimmed)) return trimmed; + if (isSshRemote(trimmed)) return trimmed; return undefined; } +/** `ssh://host/owner/repo` or the scp-like `user@host:owner/repo`. Used for both derivation and override validation. */ +function isSshRemote(value: string): boolean { + return /^ssh:\/\/[^/]+\/.+$/.test(value) || /^[^@\s/]+@[^:\s/]+:.+$/.test(value); +} + /** Split out so the scp-like SSH target is assembled rather than written as an address literal. */ const SSH_USER = "git"; @@ -168,11 +181,19 @@ async function releasePushCommand(branch: string): Promise<{ command: string[]; const keyPath = process.env.OCX_RELEASE_SSH_KEY?.trim(); if (!keyPath) return { command: ["git", "push", "origin", branch] }; const configured = process.env.OCX_RELEASE_SSH_REPO?.trim(); + // An unvalidated override outranking origin means a stale exported value from a fork session can + // 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."); + process.exit(1); + } const slug = configured || sshTargetFromOrigin(await capture(["git", "remote", "get-url", "origin"])); if (!slug) { console.error("✗ OCX_RELEASE_SSH_KEY is set but no SSH push target could be derived from origin; set OCX_RELEASE_SSH_REPO."); process.exit(1); } + console.log(`→ release push target: ${slug}`); return { // Push to the SSH URL explicitly rather than rewriting the `origin` remote: the remote stays // HTTPS for every other command, so nothing outside this call inherits the key. diff --git a/tests/release-helper.test.ts b/tests/release-helper.test.ts index 47a76a65ac0..a659e4ba97c 100644 --- a/tests/release-helper.test.ts +++ b/tests/release-helper.test.ts @@ -227,7 +227,12 @@ function runRelease(version: string, scenario: ReleaseScenario = {}) { // script aborted before logging a single call. Strip every case variant, then // set exactly one. const inheritedEnv = Object.fromEntries( - Object.entries(process.env).filter(([key]) => key.toLowerCase() !== "path"), + Object.entries(process.env).filter(([key]) => key.toLowerCase() !== "path" + // A real release EXPORTS the deploy-key variables, and the preflight runs this suite as a + // child that inherits them — so an inherited value would make the "no key configured" + // scenario run WITH a key and fail the release at its own preflight. Scrub them the same + // way PATH is scrubbed, then let the scenario add back exactly what it asked for. + && key !== "OCX_RELEASE_SSH_KEY" && key !== "OCX_RELEASE_SSH_REPO"), ); const pathKey = process.platform === "win32" ? "Path" : "PATH"; const pathValue = `${shimDir}${process.platform === "win32" ? ";" : ":"}${process.env.PATH ?? process.env.Path ?? ""}`; @@ -401,6 +406,59 @@ describe("release helper", () => { expect(push?.args[1]).toBe(`${"git"}@${"github.com"}:someone-else/opencodex.git`); }); + /** + * A credential-bearing origin must not be transplanted into the SSH target: `runLoud` prints the + * failing command, so a folded `user:token@` would put the token on the terminal and in the + * release log. Refuse instead of building a target. + */ + test("an origin carrying credentials is refused rather than transplanted", () => { + const { calls, result } = runRelease("9.9.9", { + releaseSshKey: "/tmp/k", + originUrl: `https://x-access-token:SECRET@${"github.com"}/lidge-jun/opencodex.git`, + pendingBump: true, + }); + + expect(result.status).not.toBe(0); + expect(result.stderr + result.stdout).toContain("origin carries credentials"); + expect(result.stderr + result.stdout).not.toContain("SECRET"); + expect(calls.find(call => call.name === "git" && call.args[0] === "push")).toBeUndefined(); + }); + + test("a malformed OCX_RELEASE_SSH_REPO override is refused instead of pushed to", () => { + const { calls, result } = runRelease("9.9.9", { + releaseSshKey: "/tmp/k", + releaseSshRepo: "not-a-remote", + pendingBump: true, + }); + + expect(result.status).not.toBe(0); + expect(result.stderr + result.stdout).toContain("OCX_RELEASE_SSH_REPO"); + expect(calls.find(call => call.name === "git" && call.args[0] === "push")).toBeUndefined(); + }); + + test("an ssh origin is reused verbatim rather than rewritten", () => { + const { calls } = runRelease("9.9.9", { + releaseSshKey: "/tmp/k", + originUrl: `${"git"}@${"github.com"}:lidge-jun/opencodex.git`, + pendingBump: true, + }); + + const push = calls.find(call => call.name === "git" && call.args[0] === "push"); + expect(push?.args[1]).toBe(`${"git"}@${"github.com"}:lidge-jun/opencodex.git`); + }); + + test("an origin that yields no ssh target aborts instead of guessing one", () => { + const { calls, result } = runRelease("9.9.9", { + releaseSshKey: "/tmp/k", + originUrl: "/srv/git/opencodex.git", + pendingBump: true, + }); + + expect(result.status).not.toBe(0); + expect(result.stderr + result.stdout).toContain("no SSH push target"); + expect(calls.find(call => call.name === "git" && call.args[0] === "push")).toBeUndefined(); + }); + test("without a configured key the push is unchanged and carries no ssh override", () => { const { calls, result } = runRelease("9.9.9", { pendingBump: true }); From 569d0208cf5974241a589d04dd7c87439fe41956 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Fri, 21 Aug 2026 20:55:17 +0900 Subject: [PATCH 5/5] fix(test): compare terminal-guard rebuild content, not wall-clock stamps The earlier single-clock fix was necessary but not sufficient. It made ONE rebuild self-consistent, and this test compares TWO separate rebuilds -- so each call still legitimately reads its own millisecond, and the raw JSON comparison still failed whenever the two calls straddled a boundary. It failed again on the macOS leg after that fix landed. The assertion was testing the scheduler, not the contract. The contract is that heartbeats must not change what the continuation sends; timestamps are not part of that claim. Strip them and compare the content. The single-clock invariant stays pinned by the test directly below this one, so the source guarantee is not lost. --- tests/terminal-guard.test.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/terminal-guard.test.ts b/tests/terminal-guard.test.ts index b6da980fe2c..4ceac1f02db 100644 --- a/tests/terminal-guard.test.ts +++ b/tests/terminal-guard.test.ts @@ -212,8 +212,14 @@ describe("terminal guard", () => { // The guard must not let liveness markers change what the continuation decides or sends. expect(analyzeTerminalTurn(request, padded).assistantText) .toBe(analyzeTerminalTurn(request, clean).assistantText); - expect(JSON.stringify(buildContinuationRequest(request, padded).context.messages)) - .toBe(JSON.stringify(buildContinuationRequest(request, clean).context.messages)); + // Compare the CONTENT of the two rebuilds, not their wall-clock stamps. Each call reads the + // clock once (see the next test), but two separate calls legitimately land in different + // milliseconds — comparing raw JSON made this assert the scheduler rather than the heartbeat + // contract, and it failed intermittently on CI for exactly that reason. + const withoutTimestamps = (events: AdapterEvent[]) => + JSON.stringify(buildContinuationRequest(request, events).context.messages + .map(({ timestamp: _timestamp, ...rest }) => rest)); + expect(withoutTimestamps(padded)).toBe(withoutTimestamps(clean)); }); // The rebuild used to read the clock twice — once for the assistant message, once for the