From cc795bb5c907e57baffb332100383e84f6d8ae6e Mon Sep 17 00:00:00 2001 From: Dylan Boudro <121908331+starmorph@users.noreply.github.com> Date: Tue, 27 Oct 2026 10:19:55 -0700 Subject: [PATCH 1/2] Normalize HTTPS Git credential hosts consistently --- docs/1_EXTENSION.md | 7 +- sandbox/build/git-credential-staged.sh | 26 +++- src/bot-config.ts | 24 +++- src/sandbox/render/config.ts | 4 +- src/sandbox/render/snapshot.ts | 9 +- test/git-credentials.test.ts | 179 +++++++++++++++++++++++++ test/sandbox-render.test.ts | 3 + 7 files changed, 238 insertions(+), 14 deletions(-) create mode 100644 test/git-credentials.test.ts diff --git a/docs/1_EXTENSION.md b/docs/1_EXTENSION.md index 3d3419d..b06b46a 100644 --- a/docs/1_EXTENSION.md +++ b/docs/1_EXTENSION.md @@ -83,7 +83,12 @@ export default defineConfig({ ``` Declare each token on the cron service in `render.yaml` with `sync: false`. -Only HTTPS clone URLs are accepted. +Only HTTPS clone URLs are accepted. Credential keys include a nondefault HTTPS +port: `git.example.com:8443` is separate from `git.example.com` and from other +ports. Host keys are normalized like HTTPS URLs, so explicit port `443` equals +the omitted default; leading-zero ports, host casing, and IPv4 aliases are +normalized consistently in head resolution and cloning. Duplicate keys that +normalize to the same identity are rejected instead of choosing a token. `browse` may be `github`, `gitlab`, `bitbucket`, or a template containing `{base}`, `{repository}`, `{commit}`, `{path}`, `{start}`, and `{end}`. diff --git a/sandbox/build/git-credential-staged.sh b/sandbox/build/git-credential-staged.sh index 619ccd8..9077e4a 100644 --- a/sandbox/build/git-credential-staged.sh +++ b/sandbox/build/git-credential-staged.sh @@ -1,19 +1,33 @@ #!/usr/bin/env bash # Git credential helper for the builder's clone step. Answers `get` from /, a file holding # the username on line 1 and the token on line 2. The token never appears in argv, a URL, or git config. -# Usage (git appends the action): git -c credential.helper='!bash git-credential-staged.sh ' clone ... +# Usage (git appends the action): git -c credential.helper='!bash git-credential-staged.sh ' clone ... set -euo pipefail creds_dir=$1 -action=${2:-} +node=${2:-} +action=${3:-} [ "$action" = get ] || exit 0 -host="" +protocol="" host="" while IFS= read -r line; do - case "$line" in host=*) host=${line#host=} ;; esac + case "$line" in + protocol=*) protocol=${line#protocol=} ;; + host=*) host=${line#host=} ;; + esac done -# Only plain host[:port] names map to a file; anything else (empty, slashes, dots-only tricks) gets no answer. -case "$host" in "" | *[!A-Za-z0-9.:-]* | .* ) exit 0 ;; esac +[ "$protocol" = https ] && [ -n "$node" ] || exit 0 +# Match the trusted host's WHATWG URL normalization, including default/numeric ports and IPv4 aliases. +# The input and Node path are non-secret; credentials stay in the file and credential-protocol output. +host=$("$node" -e ' + const raw = process.argv[1]; + try { + if (!raw || /[\s/\\@?#]/.test(raw) || raw.startsWith(".")) process.exit(1); + const host = new URL("https://" + raw).host; + if (!/^[a-z0-9.-]+(:[0-9]{1,5})?$/.test(host) || host.startsWith(".")) process.exit(1); + process.stdout.write(host); + } catch { process.exit(1); } +' -- "$host") || exit 0 file="$creds_dir/$host" [ -s "$file" ] || exit 0 diff --git a/src/bot-config.ts b/src/bot-config.ts index 1db1113..1c01787 100644 --- a/src/bot-config.ts +++ b/src/bot-config.ts @@ -7,7 +7,29 @@ export const HostCredential = z.object({ }); export type HostCredential = z.infer; -export const HostCredentials = z.record(z.string().regex(/^[a-z0-9.-]+(:\d{1,5})?$/, "must be a host name"), HostCredential); +/** HTTPS credential identity includes a nondefault port; URL.host removes an explicit default 443. */ +export const normalizeCredentialHost = (host: string): string => new URL(`https://${host}`).host; + +export const HostCredentials = z + .record(z.string().regex(/^[a-z0-9.-]+(:\d{1,5})?$/i, "must be a host name with an optional port"), HostCredential) + .transform((credentials, ctx) => { + const normalized = new Map(); + for (const [host, credential] of Object.entries(credentials)) { + let key: string; + try { + key = normalizeCredentialHost(host); + } catch { + ctx.addIssue({ code: "custom", path: [host], message: "must be a valid HTTPS credential host and port" }); + continue; + } + if (normalized.has(key)) { + ctx.addIssue({ code: "custom", path: [host], message: `duplicates normalized HTTPS credential host ${key}` }); + } else { + normalized.set(key, credential); + } + } + return Object.fromEntries(normalized); + }); export type HostCredentials = z.infer; export const DEFAULT_CREDENTIALS: HostCredentials = { diff --git a/src/sandbox/render/config.ts b/src/sandbox/render/config.ts index 98009e9..6e6ae03 100644 --- a/src/sandbox/render/config.ts +++ b/src/sandbox/render/config.ts @@ -2,7 +2,7 @@ import { join } from "node:path"; import type { SandboxNetworkPolicy } from "@renderinc/sdk/experimental"; import { z } from "zod"; import config from "../../../config.ts"; -import { DEFAULT_CREDENTIALS, type HostCredentials } from "../../bot-config.ts"; +import { DEFAULT_CREDENTIALS, HostCredentials } from "../../bot-config.ts"; import { present } from "../../util.ts"; import { REPO_ROOT } from "../interface.ts"; @@ -72,7 +72,7 @@ function collectHostCredentials( env: NodeJS.ProcessEnv, ): Record { const result: Record = {}; - for (const [host, { username, tokenEnv }] of Object.entries(creds ?? DEFAULT_CREDENTIALS)) { + for (const [host, { username, tokenEnv }] of Object.entries(HostCredentials.parse(creds ?? DEFAULT_CREDENTIALS))) { const token = env[tokenEnv]?.trim(); if (token) result[host] = { username, token }; } diff --git a/src/sandbox/render/snapshot.ts b/src/sandbox/render/snapshot.ts index deca4a7..7b12e38 100644 --- a/src/sandbox/render/snapshot.ts +++ b/src/sandbox/render/snapshot.ts @@ -31,9 +31,10 @@ export async function resolveHeads(profile: RepoProfile, hostCreds: RenderConfig return Promise.all( profile.repos.map(async ({ repository, branch, url: repoUrl, browse }) => { const url = repoUrl ?? `https://github.com/${repository}.git`; - const host = new URL(url).hostname; + const host = new URL(url).host; const cred = hostCreds[host]; // The token reaches git only through the environment of this trusted process, never argv or a URL. + // Sanitizing an env var name does not change the host-and-port credential identity. const envVar = `RESEARCHER_TOKEN_${host.replace(/\W/g, "_").toUpperCase()}`; const env = { ...process.env, GIT_TERMINAL_PROMPT: "0", ...(cred ? { [envVar]: cred.token } : {}) }; const auth = ["-c", "credential.helper=", ...(cred ? ["-c", hostTokenHelper(cred.username, envVar)] : [])]; @@ -145,7 +146,7 @@ export async function buildSnapshot( for (const h of heads) { const url = h.url ?? `https://github.com/${h.repository}.git`; if (!/^https:\/\//.test(url)) throw new Error(`${h.repository}: only https:// clone URLs are supported`); - await client.check(id, cloneCommand(h, url), prepare); + await client.check(id, cloneCommand(h, url, env.nodePath), prepare); } const scrub = await client.exec(id, `bash ${STAGE}/build/scrub-credentials.sh ${STAGE_CREDS} ${SANDBOX_REPOS}`, prepare); if (scrub.exitCode !== 0) throw new Error(`credential scrub failed: ${(scrub.stderr || scrub.stdout).trim().slice(-2000)}`); @@ -208,9 +209,9 @@ function envFileContent(nodePath: string, dropTool: string): string { } /** The git clone + configure + detach command for one repository. */ -function cloneCommand(h: { repository: string; branch: string; commit: string }, url: string): string { +function cloneCommand(h: { repository: string; branch: string; commit: string }, url: string, nodePath: string): string { const dest = `${SANDBOX_REPOS}/${h.repository}`; - const credHelper = `credential.helper=!bash ${STAGE}/build/git-credential-staged.sh ${STAGE_CREDS}`; + const credHelper = `credential.helper=!bash ${STAGE}/build/git-credential-staged.sh ${STAGE_CREDS} ${shq(nodePath)}`; return [ `mkdir -p $(dirname ${shq(dest)})`, `GIT_TERMINAL_PROMPT=0 git -c credential.helper= -c ${shq(credHelper)} clone --quiet --single-branch --branch ${shq(h.branch)} --no-tags -- ${shq(url)} ${shq(dest)}`, diff --git a/test/git-credentials.test.ts b/test/git-credentials.test.ts new file mode 100644 index 0000000..d0701a2 --- /dev/null +++ b/test/git-credentials.test.ts @@ -0,0 +1,179 @@ +import { execFileSync, spawnSync } from "node:child_process"; +import { mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { fileURLToPath } from "node:url"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { resolveHeads } from "../src/sandbox/render/snapshot.ts"; +import { loadRenderConfig } from "../src/sandbox/render/config.ts"; +import { defineConfig, HostCredentials } from "../src/bot-config.ts"; +import config from "../config.ts"; +import { shq } from "../src/sandbox/interface.ts"; + +const REAL_GIT = execFileSync("which", ["git"], { encoding: "utf8" }).trim(); +const STAGED_HELPER = fileURLToPath(new URL("../sandbox/build/git-credential-staged.sh", import.meta.url)); +const COMMIT = "a".repeat(40); +const ORIGINAL_CREDENTIALS = config.gitCredentials; +let root: string; +let credentialsDir: string; +let recordFile: string; +let configFile: string; + +type Credential = { username: string; token: string }; +type GitRecord = { args: string[]; url: string; credential: { username?: string; password?: string }; tokenEnvNames: string[] }; + +beforeEach(async () => { + root = await mkdtemp(join(tmpdir(), "git-credential-selection-")); + credentialsDir = join(root, "creds"); + recordFile = join(root, "record.json"); + configFile = join(root, "gitconfig"); + const shim = join(root, "bin"); + await mkdir(credentialsDir); + await mkdir(shim); + await writeFile(configFile, ""); + // Stand in for remote access, but run real Git credential fill with the exact -c helper arguments. + await writeFile(join(shim, "git"), `#!${process.execPath} +const { spawnSync } = require("node:child_process"); +const { writeFileSync } = require("node:fs"); +const args = process.argv.slice(2); +const index = args.indexOf("ls-remote"); +const url = args[index + 1] === "--symref" ? args[index + 2] : args[index + 1]; +const result = spawnSync(process.env.RESEARCHER_TEST_REAL_GIT, [...args.slice(0, index), "credential", "fill"], { + input: "url=" + url + "\\n\\n", encoding: "utf8", env: process.env, +}); +const credential = Object.fromEntries(result.stdout.trim().split("\\n").filter(Boolean).map((line) => { + const equal = line.indexOf("="); return [line.slice(0, equal), line.slice(equal + 1)]; +})); +writeFileSync(process.env.RESEARCHER_TEST_RECORD, JSON.stringify({ args, url, credential, + tokenEnvNames: Object.keys(process.env).filter((name) => name.startsWith("RESEARCHER_TOKEN_")), +})); +if (process.env.RESEARCHER_TEST_GIT_FAIL) { process.stderr.write("synthetic Git failure"); process.exit(1); } +process.stdout.write("ref: refs/heads/main\\tHEAD\\n${COMMIT}\\tHEAD\\n"); +`, { mode: 0o755 }); + vi.stubEnv("PATH", `${shim}:${process.env.PATH}`); + vi.stubEnv("HOME", root); + vi.stubEnv("GIT_CONFIG_GLOBAL", configFile); + vi.stubEnv("GIT_CONFIG_NOSYSTEM", "1"); + vi.stubEnv("GIT_TERMINAL_PROMPT", "0"); + vi.stubEnv("GIT_ASKPASS", "/usr/bin/false"); + vi.stubEnv("SSH_ASKPASS", "/usr/bin/false"); + vi.stubEnv("GIT_CONFIG_COUNT", "0"); + vi.stubEnv("RESEARCHER_TEST_REAL_GIT", REAL_GIT); + vi.stubEnv("RESEARCHER_TEST_RECORD", recordFile); +}); + +afterEach(async () => { + vi.unstubAllEnvs(); + config.gitCredentials = ORIGINAL_CREDENTIALS; + await rm(root, { recursive: true, force: true }); +}); + +function stagedCredential(url: string) { + const helper = `credential.helper=!bash ${shq(STAGED_HELPER)} ${shq(credentialsDir)} ${shq(process.execPath)}`; + const args = ["-c", "credential.helper=", "-c", helper, "credential", "fill"]; + const result = spawnSync(REAL_GIT, args, { input: `url=${url}\n\n`, env: process.env, encoding: "utf8" }); + const credential = Object.fromEntries(result.stdout.trim().split("\n").filter(Boolean).map((line) => { + const equal = line.indexOf("="); + return [line.slice(0, equal), line.slice(equal + 1)]; + })); + return { ...result, credential, args }; +} + +async function compareSelection(url: string, credentials: { [host: string]: Credential }) { + for (const [host, credential] of Object.entries(credentials)) { + await writeFile(join(credentialsDir, host), `${credential.username}\n${credential.token}`, { mode: 0o600 }); + } + const heads = await resolveHeads({ name: "test", repos: [{ repository: "acme/repo", url }] }, credentials); + const resolution = JSON.parse(await readFile(recordFile, "utf8")) as GitRecord; + return { heads, resolution, staged: stagedCredential(url) }; +} + +describe("HTTPS Git credential selection", () => { + it("uses the same port-specific credential for head resolution and cloning", async () => { + const specific = { username: "port-user", token: "synthetic-port-token" }; + const defaultPort = { username: "default-user", token: "synthetic-default-token" }; + const { resolution, staged } = await compareSelection("https://git.example:8443/acme/repo.git", { + "git.example": defaultPort, + "git.example:8443": specific, + }); + + expect(resolution.credential.password).toBe(specific.token); + expect(staged.credential.password).toBe(specific.token); + expect(resolution.tokenEnvNames).toContain("RESEARCHER_TOKEN_GIT_EXAMPLE_8443"); + }); + + it.each([ + { key: "github.com", urlHost: "github.com", normalized: "github.com" }, + { key: "git.example", urlHost: "git.example:443", normalized: "git.example" }, + { key: "git.example:443", urlHost: "git.example", normalized: "git.example" }, + { key: "git.example:0443", urlHost: "git.example:0443", normalized: "git.example" }, + { key: "GIT.EXAMPLE:8443", urlHost: "GIT.EXAMPLE:8443", normalized: "git.example:8443" }, + { key: "git.example:08443", urlHost: "git.example:08443", normalized: "git.example:8443" }, + { key: "127.1:443", urlHost: "127.1:0443", normalized: "127.0.0.1" }, + ])("normalizes configured $key consistently with Git's raw $urlHost", async ({ key, urlHost, normalized }) => { + const token = "synthetic-selected-token"; + config.gitCredentials = { [key]: { username: "test-user", tokenEnv: "SYNTHETIC_GIT_TOKEN" } }; + const loaded = await loadRenderConfig({ + RENDER_API_KEY: "synthetic-render-key", RENDER_WORKSPACE_ID: "tea-test", SYNTHETIC_GIT_TOKEN: token, + }); + expect(Object.keys(loaded.hostCredentials)).toEqual([normalized]); + const url = `https://${urlHost}/acme/repo.git`; + + const { heads, resolution, staged } = await compareSelection(url, loaded.hostCredentials); + + expect(heads[0]).toMatchObject({ commit: COMMIT, url }); + expect(resolution.credential).toMatchObject({ username: "test-user", password: token }); + expect(staged.status).toBe(0); + expect(staged.credential).toMatchObject({ username: "test-user", password: token }); + expect(JSON.stringify([resolution.args, resolution.url, staged.args, staged.stderr])).not.toContain(token); + expect(await readFile(configFile, "utf8")).toBe(""); + }); + + it.each(["git.example", "git.example:8444"])("does not reuse a credential from %s for port 8443", async (otherHost) => { + const token = "synthetic-other-origin-token"; + const { resolution, staged } = await compareSelection("https://git.example:8443/acme/repo.git", { + [otherHost]: { username: "other-user", token }, + }); + + expect(resolution.credential.password).toBeUndefined(); + expect(staged.status).not.toBe(0); + expect(staged.credential.password).toBeUndefined(); + expect(JSON.stringify([resolution.args, resolution.url, staged.args, staged.stderr])).not.toContain(token); + }); + + it.each([ + ["git.example", "git.example:443"], + ["git.example:443", "git.example:0443"], + ["git.example:8443", "GIT.EXAMPLE:08443"], + ["127.1", "127.0.0.1"], + ])("rejects ambiguous configured identities %s and %s before selecting a token", async (left, right) => { + const credentials = { + [left]: { username: "first-user", tokenEnv: "FIRST_TOKEN" }, + [right]: { username: "second-user", tokenEnv: "SECOND_TOKEN" }, + }; + expect(() => HostCredentials.parse(credentials)).toThrow(/duplicates normalized HTTPS credential host/); + expect(() => defineConfig({ name: "test", repos: [{ repository: "acme/repo" }], gitCredentials: credentials }, {})).toThrow(/duplicates normalized HTTPS credential host/); + config.gitCredentials = credentials; + await expect(loadRenderConfig({ + RENDER_API_KEY: "synthetic-render-key", RENDER_WORKSPACE_ID: "tea-test", FIRST_TOKEN: "synthetic-first", SECOND_TOKEN: "synthetic-second", + })).rejects.toThrow(/duplicates normalized HTTPS credential host/); + }); + + it("rejects an invalid configured HTTPS port", () => { + expect(() => HostCredentials.parse({ "git.example:99999": { username: "test-user", tokenEnv: "TEST_TOKEN" } })).toThrow(/valid HTTPS credential host and port/); + }); + + it("keeps synthetic token values out of resolution errors and Git configuration", async () => { + const token = "synthetic-error-path-token"; + vi.stubEnv("RESEARCHER_TEST_GIT_FAIL", "1"); + const error = await resolveHeads({ name: "test", repos: [{ repository: "acme/repo", url: "https://git.example:8443/acme/repo.git" }] }, { + "git.example:8443": { username: "test-user", token }, + }).catch((err: unknown) => err); + const record = JSON.parse(await readFile(recordFile, "utf8")) as GitRecord; + + expect(error).toMatchObject({ message: "git ls-remote acme/repo: synthetic Git failure" }); + expect((error as Error).message).not.toContain(token); + expect(JSON.stringify(record.args)).not.toContain(token); + expect(await readFile(configFile, "utf8")).toBe(""); + }); +}); diff --git a/test/sandbox-render.test.ts b/test/sandbox-render.test.ts index 5d34d71..5fe64a2 100644 --- a/test/sandbox-render.test.ts +++ b/test/sandbox-render.test.ts @@ -211,6 +211,9 @@ describe("buildSnapshot", () => { expect(exec("install-tools.sh")).toBeLessThan(credUpload); expect(exec("--self-test")).toBeLessThan(credUpload); expect(fake.commands().some((c) => c.includes(GH_TOKEN))).toBe(false); + const clones = fake.commands().filter((command) => command.includes("clone --quiet")); + expect(clones).toHaveLength(heads.length); + expect(clones.every((command) => command.includes("git-credential-staged.sh") && command.includes("/opt/researcher/app/node/bin/node"))).toBe(true); const snapshotted = fake.indexOf((c) => c.op === "snapshot.create"); expect(exec("scrub-credentials.sh")).toBeLessThan(snapshotted); From 4108e9da892f4d7d5d6c777695ff73b049cdd7b4 Mon Sep 17 00:00:00 2001 From: Jacob Prall Date: Wed, 28 Oct 2026 12:58:37 -0700 Subject: [PATCH 2/2] Reduce the port fix to URL.host in head resolution Head resolution looked up credentials by URL.hostname, which drops the port, while the staged clone helper uses Git's host:port. Using URL.host makes both phases select the same credential. Replaces the normalization layer and the Node call in the credential helper with that one change and a focused test. Co-authored-by: Cursor --- docs/1_EXTENSION.md | 10 +- sandbox/build/git-credential-staged.sh | 26 +--- src/bot-config.ts | 24 +--- src/sandbox/render/config.ts | 4 +- src/sandbox/render/snapshot.ts | 8 +- test/git-credentials.test.ts | 179 ------------------------- test/sandbox-render.test.ts | 45 ++++++- 7 files changed, 56 insertions(+), 240 deletions(-) delete mode 100644 test/git-credentials.test.ts diff --git a/docs/1_EXTENSION.md b/docs/1_EXTENSION.md index ce9bc45..4c08829 100644 --- a/docs/1_EXTENSION.md +++ b/docs/1_EXTENSION.md @@ -31,7 +31,8 @@ export default defineConfig({ - `repos` lists `owner/name` repositories. The default branch is resolved at snapshot time unless `branch` is set. - `gitCredentials` maps each Git host to a username and the environment - variable containing its token. An unset token means clone anonymously. + variable containing its token. A server on a nondefault HTTPS port is keyed + as `host:port`. An unset token means clone anonymously. - `snapshotName` is the stable name built by the cron and restored by the workflow. It must match `[A-Za-z0-9_.-]+` and be unique in the workspace. @@ -83,12 +84,7 @@ export default defineConfig({ ``` Declare each token on the cron service in `render.yaml` with `sync: false`. -Only HTTPS clone URLs are accepted. Credential keys include a nondefault HTTPS -port: `git.example.com:8443` is separate from `git.example.com` and from other -ports. Host keys are normalized like HTTPS URLs, so explicit port `443` equals -the omitted default; leading-zero ports, host casing, and IPv4 aliases are -normalized consistently in head resolution and cloning. Duplicate keys that -normalize to the same identity are rejected instead of choosing a token. +Only HTTPS clone URLs are accepted. `browse` may be `github`, `gitlab`, `bitbucket`, or a template containing `{base}`, `{repository}`, `{commit}`, `{path}`, `{start}`, and `{end}`. diff --git a/sandbox/build/git-credential-staged.sh b/sandbox/build/git-credential-staged.sh index 9077e4a..619ccd8 100644 --- a/sandbox/build/git-credential-staged.sh +++ b/sandbox/build/git-credential-staged.sh @@ -1,33 +1,19 @@ #!/usr/bin/env bash # Git credential helper for the builder's clone step. Answers `get` from /, a file holding # the username on line 1 and the token on line 2. The token never appears in argv, a URL, or git config. -# Usage (git appends the action): git -c credential.helper='!bash git-credential-staged.sh ' clone ... +# Usage (git appends the action): git -c credential.helper='!bash git-credential-staged.sh ' clone ... set -euo pipefail creds_dir=$1 -node=${2:-} -action=${3:-} +action=${2:-} [ "$action" = get ] || exit 0 -protocol="" host="" +host="" while IFS= read -r line; do - case "$line" in - protocol=*) protocol=${line#protocol=} ;; - host=*) host=${line#host=} ;; - esac + case "$line" in host=*) host=${line#host=} ;; esac done -[ "$protocol" = https ] && [ -n "$node" ] || exit 0 -# Match the trusted host's WHATWG URL normalization, including default/numeric ports and IPv4 aliases. -# The input and Node path are non-secret; credentials stay in the file and credential-protocol output. -host=$("$node" -e ' - const raw = process.argv[1]; - try { - if (!raw || /[\s/\\@?#]/.test(raw) || raw.startsWith(".")) process.exit(1); - const host = new URL("https://" + raw).host; - if (!/^[a-z0-9.-]+(:[0-9]{1,5})?$/.test(host) || host.startsWith(".")) process.exit(1); - process.stdout.write(host); - } catch { process.exit(1); } -' -- "$host") || exit 0 +# Only plain host[:port] names map to a file; anything else (empty, slashes, dots-only tricks) gets no answer. +case "$host" in "" | *[!A-Za-z0-9.:-]* | .* ) exit 0 ;; esac file="$creds_dir/$host" [ -s "$file" ] || exit 0 diff --git a/src/bot-config.ts b/src/bot-config.ts index 1c01787..1db1113 100644 --- a/src/bot-config.ts +++ b/src/bot-config.ts @@ -7,29 +7,7 @@ export const HostCredential = z.object({ }); export type HostCredential = z.infer; -/** HTTPS credential identity includes a nondefault port; URL.host removes an explicit default 443. */ -export const normalizeCredentialHost = (host: string): string => new URL(`https://${host}`).host; - -export const HostCredentials = z - .record(z.string().regex(/^[a-z0-9.-]+(:\d{1,5})?$/i, "must be a host name with an optional port"), HostCredential) - .transform((credentials, ctx) => { - const normalized = new Map(); - for (const [host, credential] of Object.entries(credentials)) { - let key: string; - try { - key = normalizeCredentialHost(host); - } catch { - ctx.addIssue({ code: "custom", path: [host], message: "must be a valid HTTPS credential host and port" }); - continue; - } - if (normalized.has(key)) { - ctx.addIssue({ code: "custom", path: [host], message: `duplicates normalized HTTPS credential host ${key}` }); - } else { - normalized.set(key, credential); - } - } - return Object.fromEntries(normalized); - }); +export const HostCredentials = z.record(z.string().regex(/^[a-z0-9.-]+(:\d{1,5})?$/, "must be a host name"), HostCredential); export type HostCredentials = z.infer; export const DEFAULT_CREDENTIALS: HostCredentials = { diff --git a/src/sandbox/render/config.ts b/src/sandbox/render/config.ts index 6e6ae03..98009e9 100644 --- a/src/sandbox/render/config.ts +++ b/src/sandbox/render/config.ts @@ -2,7 +2,7 @@ import { join } from "node:path"; import type { SandboxNetworkPolicy } from "@renderinc/sdk/experimental"; import { z } from "zod"; import config from "../../../config.ts"; -import { DEFAULT_CREDENTIALS, HostCredentials } from "../../bot-config.ts"; +import { DEFAULT_CREDENTIALS, type HostCredentials } from "../../bot-config.ts"; import { present } from "../../util.ts"; import { REPO_ROOT } from "../interface.ts"; @@ -72,7 +72,7 @@ function collectHostCredentials( env: NodeJS.ProcessEnv, ): Record { const result: Record = {}; - for (const [host, { username, tokenEnv }] of Object.entries(HostCredentials.parse(creds ?? DEFAULT_CREDENTIALS))) { + for (const [host, { username, tokenEnv }] of Object.entries(creds ?? DEFAULT_CREDENTIALS)) { const token = env[tokenEnv]?.trim(); if (token) result[host] = { username, token }; } diff --git a/src/sandbox/render/snapshot.ts b/src/sandbox/render/snapshot.ts index 1cc0992..6cea91c 100644 --- a/src/sandbox/render/snapshot.ts +++ b/src/sandbox/render/snapshot.ts @@ -31,10 +31,10 @@ export async function resolveHeads(profile: RepoProfile, hostCreds: RenderConfig return Promise.all( profile.repos.map(async ({ repository, branch, url: repoUrl, browse }) => { const url = repoUrl ?? `https://github.com/${repository}.git`; + // `host` keeps a nondefault port, matching the host:port key the staged clone helper looks up. const host = new URL(url).host; const cred = hostCreds[host]; // The token reaches git only through the environment of this trusted process, never argv or a URL. - // Sanitizing an env var name does not change the host-and-port credential identity. const envVar = `RESEARCHER_TOKEN_${host.replace(/\W/g, "_").toUpperCase()}`; const env = { ...process.env, GIT_TERMINAL_PROMPT: "0", ...(cred ? { [envVar]: cred.token } : {}) }; const auth = ["-c", "credential.helper=", ...(cred ? ["-c", hostTokenHelper(cred.username, envVar)] : [])]; @@ -146,7 +146,7 @@ export async function buildSnapshot( for (const h of heads) { const url = h.url ?? `https://github.com/${h.repository}.git`; if (!/^https:\/\//.test(url)) throw new Error(`${h.repository}: only https:// clone URLs are supported`); - await client.check(id, cloneCommand(h, url, env.nodePath), prepare); + await client.check(id, cloneCommand(h, url), prepare); } const scrub = await client.exec(id, `bash ${STAGE}/build/scrub-credentials.sh ${STAGE_CREDS} ${SANDBOX_REPOS}`, prepare); if (scrub.exitCode !== 0) throw new Error(`credential scrub failed: ${(scrub.stderr || scrub.stdout).trim().slice(-2000)}`); @@ -209,9 +209,9 @@ function envFileContent(nodePath: string, dropTool: string): string { } /** The git clone + configure + detach command for one repository. */ -function cloneCommand(h: { repository: string; branch: string; commit: string }, url: string, nodePath: string): string { +function cloneCommand(h: { repository: string; branch: string; commit: string }, url: string): string { const dest = `${SANDBOX_REPOS}/${h.repository}`; - const credHelper = `credential.helper=!bash ${STAGE}/build/git-credential-staged.sh ${STAGE_CREDS} ${shq(nodePath)}`; + const credHelper = `credential.helper=!bash ${STAGE}/build/git-credential-staged.sh ${STAGE_CREDS}`; return [ `mkdir -p $(dirname ${shq(dest)})`, `GIT_TERMINAL_PROMPT=0 git -c credential.helper= -c ${shq(credHelper)} clone --quiet --single-branch --branch ${shq(h.branch)} --no-tags -- ${shq(url)} ${shq(dest)}`, diff --git a/test/git-credentials.test.ts b/test/git-credentials.test.ts deleted file mode 100644 index d0701a2..0000000 --- a/test/git-credentials.test.ts +++ /dev/null @@ -1,179 +0,0 @@ -import { execFileSync, spawnSync } from "node:child_process"; -import { mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; -import { tmpdir } from "node:os"; -import { join } from "node:path"; -import { fileURLToPath } from "node:url"; -import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { resolveHeads } from "../src/sandbox/render/snapshot.ts"; -import { loadRenderConfig } from "../src/sandbox/render/config.ts"; -import { defineConfig, HostCredentials } from "../src/bot-config.ts"; -import config from "../config.ts"; -import { shq } from "../src/sandbox/interface.ts"; - -const REAL_GIT = execFileSync("which", ["git"], { encoding: "utf8" }).trim(); -const STAGED_HELPER = fileURLToPath(new URL("../sandbox/build/git-credential-staged.sh", import.meta.url)); -const COMMIT = "a".repeat(40); -const ORIGINAL_CREDENTIALS = config.gitCredentials; -let root: string; -let credentialsDir: string; -let recordFile: string; -let configFile: string; - -type Credential = { username: string; token: string }; -type GitRecord = { args: string[]; url: string; credential: { username?: string; password?: string }; tokenEnvNames: string[] }; - -beforeEach(async () => { - root = await mkdtemp(join(tmpdir(), "git-credential-selection-")); - credentialsDir = join(root, "creds"); - recordFile = join(root, "record.json"); - configFile = join(root, "gitconfig"); - const shim = join(root, "bin"); - await mkdir(credentialsDir); - await mkdir(shim); - await writeFile(configFile, ""); - // Stand in for remote access, but run real Git credential fill with the exact -c helper arguments. - await writeFile(join(shim, "git"), `#!${process.execPath} -const { spawnSync } = require("node:child_process"); -const { writeFileSync } = require("node:fs"); -const args = process.argv.slice(2); -const index = args.indexOf("ls-remote"); -const url = args[index + 1] === "--symref" ? args[index + 2] : args[index + 1]; -const result = spawnSync(process.env.RESEARCHER_TEST_REAL_GIT, [...args.slice(0, index), "credential", "fill"], { - input: "url=" + url + "\\n\\n", encoding: "utf8", env: process.env, -}); -const credential = Object.fromEntries(result.stdout.trim().split("\\n").filter(Boolean).map((line) => { - const equal = line.indexOf("="); return [line.slice(0, equal), line.slice(equal + 1)]; -})); -writeFileSync(process.env.RESEARCHER_TEST_RECORD, JSON.stringify({ args, url, credential, - tokenEnvNames: Object.keys(process.env).filter((name) => name.startsWith("RESEARCHER_TOKEN_")), -})); -if (process.env.RESEARCHER_TEST_GIT_FAIL) { process.stderr.write("synthetic Git failure"); process.exit(1); } -process.stdout.write("ref: refs/heads/main\\tHEAD\\n${COMMIT}\\tHEAD\\n"); -`, { mode: 0o755 }); - vi.stubEnv("PATH", `${shim}:${process.env.PATH}`); - vi.stubEnv("HOME", root); - vi.stubEnv("GIT_CONFIG_GLOBAL", configFile); - vi.stubEnv("GIT_CONFIG_NOSYSTEM", "1"); - vi.stubEnv("GIT_TERMINAL_PROMPT", "0"); - vi.stubEnv("GIT_ASKPASS", "/usr/bin/false"); - vi.stubEnv("SSH_ASKPASS", "/usr/bin/false"); - vi.stubEnv("GIT_CONFIG_COUNT", "0"); - vi.stubEnv("RESEARCHER_TEST_REAL_GIT", REAL_GIT); - vi.stubEnv("RESEARCHER_TEST_RECORD", recordFile); -}); - -afterEach(async () => { - vi.unstubAllEnvs(); - config.gitCredentials = ORIGINAL_CREDENTIALS; - await rm(root, { recursive: true, force: true }); -}); - -function stagedCredential(url: string) { - const helper = `credential.helper=!bash ${shq(STAGED_HELPER)} ${shq(credentialsDir)} ${shq(process.execPath)}`; - const args = ["-c", "credential.helper=", "-c", helper, "credential", "fill"]; - const result = spawnSync(REAL_GIT, args, { input: `url=${url}\n\n`, env: process.env, encoding: "utf8" }); - const credential = Object.fromEntries(result.stdout.trim().split("\n").filter(Boolean).map((line) => { - const equal = line.indexOf("="); - return [line.slice(0, equal), line.slice(equal + 1)]; - })); - return { ...result, credential, args }; -} - -async function compareSelection(url: string, credentials: { [host: string]: Credential }) { - for (const [host, credential] of Object.entries(credentials)) { - await writeFile(join(credentialsDir, host), `${credential.username}\n${credential.token}`, { mode: 0o600 }); - } - const heads = await resolveHeads({ name: "test", repos: [{ repository: "acme/repo", url }] }, credentials); - const resolution = JSON.parse(await readFile(recordFile, "utf8")) as GitRecord; - return { heads, resolution, staged: stagedCredential(url) }; -} - -describe("HTTPS Git credential selection", () => { - it("uses the same port-specific credential for head resolution and cloning", async () => { - const specific = { username: "port-user", token: "synthetic-port-token" }; - const defaultPort = { username: "default-user", token: "synthetic-default-token" }; - const { resolution, staged } = await compareSelection("https://git.example:8443/acme/repo.git", { - "git.example": defaultPort, - "git.example:8443": specific, - }); - - expect(resolution.credential.password).toBe(specific.token); - expect(staged.credential.password).toBe(specific.token); - expect(resolution.tokenEnvNames).toContain("RESEARCHER_TOKEN_GIT_EXAMPLE_8443"); - }); - - it.each([ - { key: "github.com", urlHost: "github.com", normalized: "github.com" }, - { key: "git.example", urlHost: "git.example:443", normalized: "git.example" }, - { key: "git.example:443", urlHost: "git.example", normalized: "git.example" }, - { key: "git.example:0443", urlHost: "git.example:0443", normalized: "git.example" }, - { key: "GIT.EXAMPLE:8443", urlHost: "GIT.EXAMPLE:8443", normalized: "git.example:8443" }, - { key: "git.example:08443", urlHost: "git.example:08443", normalized: "git.example:8443" }, - { key: "127.1:443", urlHost: "127.1:0443", normalized: "127.0.0.1" }, - ])("normalizes configured $key consistently with Git's raw $urlHost", async ({ key, urlHost, normalized }) => { - const token = "synthetic-selected-token"; - config.gitCredentials = { [key]: { username: "test-user", tokenEnv: "SYNTHETIC_GIT_TOKEN" } }; - const loaded = await loadRenderConfig({ - RENDER_API_KEY: "synthetic-render-key", RENDER_WORKSPACE_ID: "tea-test", SYNTHETIC_GIT_TOKEN: token, - }); - expect(Object.keys(loaded.hostCredentials)).toEqual([normalized]); - const url = `https://${urlHost}/acme/repo.git`; - - const { heads, resolution, staged } = await compareSelection(url, loaded.hostCredentials); - - expect(heads[0]).toMatchObject({ commit: COMMIT, url }); - expect(resolution.credential).toMatchObject({ username: "test-user", password: token }); - expect(staged.status).toBe(0); - expect(staged.credential).toMatchObject({ username: "test-user", password: token }); - expect(JSON.stringify([resolution.args, resolution.url, staged.args, staged.stderr])).not.toContain(token); - expect(await readFile(configFile, "utf8")).toBe(""); - }); - - it.each(["git.example", "git.example:8444"])("does not reuse a credential from %s for port 8443", async (otherHost) => { - const token = "synthetic-other-origin-token"; - const { resolution, staged } = await compareSelection("https://git.example:8443/acme/repo.git", { - [otherHost]: { username: "other-user", token }, - }); - - expect(resolution.credential.password).toBeUndefined(); - expect(staged.status).not.toBe(0); - expect(staged.credential.password).toBeUndefined(); - expect(JSON.stringify([resolution.args, resolution.url, staged.args, staged.stderr])).not.toContain(token); - }); - - it.each([ - ["git.example", "git.example:443"], - ["git.example:443", "git.example:0443"], - ["git.example:8443", "GIT.EXAMPLE:08443"], - ["127.1", "127.0.0.1"], - ])("rejects ambiguous configured identities %s and %s before selecting a token", async (left, right) => { - const credentials = { - [left]: { username: "first-user", tokenEnv: "FIRST_TOKEN" }, - [right]: { username: "second-user", tokenEnv: "SECOND_TOKEN" }, - }; - expect(() => HostCredentials.parse(credentials)).toThrow(/duplicates normalized HTTPS credential host/); - expect(() => defineConfig({ name: "test", repos: [{ repository: "acme/repo" }], gitCredentials: credentials }, {})).toThrow(/duplicates normalized HTTPS credential host/); - config.gitCredentials = credentials; - await expect(loadRenderConfig({ - RENDER_API_KEY: "synthetic-render-key", RENDER_WORKSPACE_ID: "tea-test", FIRST_TOKEN: "synthetic-first", SECOND_TOKEN: "synthetic-second", - })).rejects.toThrow(/duplicates normalized HTTPS credential host/); - }); - - it("rejects an invalid configured HTTPS port", () => { - expect(() => HostCredentials.parse({ "git.example:99999": { username: "test-user", tokenEnv: "TEST_TOKEN" } })).toThrow(/valid HTTPS credential host and port/); - }); - - it("keeps synthetic token values out of resolution errors and Git configuration", async () => { - const token = "synthetic-error-path-token"; - vi.stubEnv("RESEARCHER_TEST_GIT_FAIL", "1"); - const error = await resolveHeads({ name: "test", repos: [{ repository: "acme/repo", url: "https://git.example:8443/acme/repo.git" }] }, { - "git.example:8443": { username: "test-user", token }, - }).catch((err: unknown) => err); - const record = JSON.parse(await readFile(recordFile, "utf8")) as GitRecord; - - expect(error).toMatchObject({ message: "git ls-remote acme/repo: synthetic Git failure" }); - expect((error as Error).message).not.toContain(token); - expect(JSON.stringify(record.args)).not.toContain(token); - expect(await readFile(configFile, "utf8")).toBe(""); - }); -}); diff --git a/test/sandbox-render.test.ts b/test/sandbox-render.test.ts index 300a7e5..a4603a9 100644 --- a/test/sandbox-render.test.ts +++ b/test/sandbox-render.test.ts @@ -1,15 +1,18 @@ +import { mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; import { fileURLToPath } from "node:url"; import type { RepoManifest } from "../src/manifest.ts"; import { AbortError, RenderError, SandboxSnapshotNotFoundError, SandboxSnapshotNotReadyError } from "@renderinc/sdk"; import type { SandboxExecEvent } from "@renderinc/sdk/experimental"; -import { beforeEach, describe, expect, it } from "vitest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; import { configureSandboxes } from "../src/sandbox/index.ts"; import { SnapshotUnavailableError } from "../src/sandbox/interface.ts"; import { ClientError } from "@renderinc/sdk"; import { SandboxClient, type SandboxApi } from "../src/sandbox/render/client.ts"; import { loadRenderConfig, type NetworkPolicy, type RenderConfig } from "../src/sandbox/render/config.ts"; import { openRenderSandbox, runInvestigation } from "../src/sandbox/render/sandbox.ts"; -import { buildSnapshot, pruneSnapshots } from "../src/sandbox/render/snapshot.ts"; +import { buildSnapshot, pruneSnapshots, resolveHeads } from "../src/sandbox/render/snapshot.ts"; import { runnerInput } from "./helpers.ts"; import { createLog } from "../src/log.ts"; @@ -212,9 +215,6 @@ describe("buildSnapshot", () => { expect(exec("install-tools.sh")).toBeLessThan(credUpload); expect(exec("--self-test")).toBeLessThan(credUpload); expect(fake.commands().some((c) => c.includes(GH_TOKEN))).toBe(false); - const clones = fake.commands().filter((command) => command.includes("clone --quiet")); - expect(clones).toHaveLength(heads.length); - expect(clones.every((command) => command.includes("git-credential-staged.sh") && command.includes("/opt/researcher/app/node/bin/node"))).toBe(true); const snapshotted = fake.indexOf((c) => c.op === "snapshot.create"); expect(exec("scrub-credentials.sh")).toBeLessThan(snapshotted); @@ -241,6 +241,41 @@ describe("buildSnapshot", () => { }); }); +describe("resolveHeads", () => { + const sha = "a".repeat(40); + + async function lsRemote(url: string, hostCreds: RenderConfig["hostCredentials"]) { + const dir = await mkdtemp(join(tmpdir(), "resolve-heads-")); + try { + const record = join(dir, "record"); + await writeFile(join(dir, "git"), `#!/bin/sh +env | grep '^RESEARCHER_TOKEN_' > ${record} +printf 'ref: refs/heads/main\\tHEAD\\n${sha}\\tHEAD\\n' +`, { mode: 0o755 }); + vi.stubEnv("PATH", `${dir}:${process.env.PATH}`); + const heads = await resolveHeads({ name: "test", repos: [{ repository: "acme/repo", url }] }, hostCreds); + return { heads, tokens: (await readFile(record, "utf8")).trim() }; + } finally { + vi.unstubAllEnvs(); + await rm(dir, { recursive: true, force: true }); + } + } + + it("selects the credential for a nondefault HTTPS port, matching the staged clone helper's host:port key", async () => { + const { heads, tokens } = await lsRemote("https://git.example:8443/acme/repo.git", { + "git.example": { username: "x", token: "default-port-token" }, + "git.example:8443": { username: "x", token: "port-token" }, + }); + expect(heads[0]).toMatchObject({ commit: sha, branch: "main" }); + expect(tokens).toBe("RESEARCHER_TOKEN_GIT_EXAMPLE_8443=port-token"); + }); + + it("does not reuse a default-port credential for another port", async () => { + const { tokens } = await lsRemote("https://git.example:8443/acme/repo.git", { "git.example": { username: "x", token: "t" } }); + expect(tokens).toBe(""); + }); +}); + describe("pruneSnapshots", () => { const built = { id: "snp-new", sandboxGroupId: "sbg-1" };