From 750ab443794897a10db5fc21e721136b91711aff Mon Sep 17 00:00:00 2001 From: Danijel Martinek Date: Sun, 12 Jul 2026 22:38:17 +0200 Subject: [PATCH] feat(runner): clone stage with ephemeral credential helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clone stage per tech spec §6, verbatim mechanics: every git invocation carries a BLANK credential.helper first (suppresses OS keychain helpers) then the inline veect helper; the PAT reaches git via child env only — never argv, URLs, logs, or .git/config. GIT_TERMINAL_PROMPT and GIT_ASKPASS are pinned so a headless clone can never hang on a TTY or ambient IDE askpass. Staged status events (start/heartbeat/final) + ready on success; named failures: invalid-git-url, auth-failed, clone-failed (daemon's 'repository not exported' maps to bad-URL, not auth). Integration suite clones the daemon-served vite-kitchen and an authenticated dumb-HTTP remote that asserts the exact Basic credential git presented, plus leak assertions over logs/argv/.git. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_016j8z4VHjedXDTjEDNg7qHK --- apps/runner/AGENTS.md | 30 ++ apps/runner/src/exec.ts | 55 ++++ apps/runner/src/git/credential-helper.test.ts | 67 +++++ apps/runner/src/git/credential-helper.ts | 71 +++++ apps/runner/src/server.ts | 68 +++-- apps/runner/src/stages/clone.test.ts | 247 ++++++++++++++++ apps/runner/src/stages/clone.ts | 156 ++++++++++ apps/runner/src/stages/stage-runner.test.ts | 115 ++++++++ apps/runner/src/stages/stage-runner.ts | 67 +++++ .../tests/clone-stage.integration.test.ts | 272 ++++++++++++++++++ apps/runner/tests/http-git-server.ts | 107 +++++++ apps/runner/tests/protocol-client.ts | 26 ++ 12 files changed, 1261 insertions(+), 20 deletions(-) create mode 100644 apps/runner/src/exec.ts create mode 100644 apps/runner/src/git/credential-helper.test.ts create mode 100644 apps/runner/src/git/credential-helper.ts create mode 100644 apps/runner/src/stages/clone.test.ts create mode 100644 apps/runner/src/stages/clone.ts create mode 100644 apps/runner/src/stages/stage-runner.test.ts create mode 100644 apps/runner/src/stages/stage-runner.ts create mode 100644 apps/runner/tests/clone-stage.integration.test.ts create mode 100644 apps/runner/tests/http-git-server.ts diff --git a/apps/runner/AGENTS.md b/apps/runner/AGENTS.md index 7c00046..3682d66 100644 --- a/apps/runner/AGENTS.md +++ b/apps/runner/AGENTS.md @@ -33,6 +33,36 @@ manifest, no use-case layer, no DI container. - Zod issues are summarized as path + code only — payload values (which may include a PAT) are never echoed into errors or logs. +## Clone stage — credential mechanics (tech spec §6, verbatim) + +- Every git invocation carries `-c credential.helper=` (BLANK first — + resets the helper list, suppressing OS keychain helpers) followed by + `-c credential.helper=` (an inline shell function that + answers `get` from env vars and ignores `store`/`erase`). +- The PAT reaches git via the child's env (`VEECT_GIT_PAT`) — never + argv (the helper string only names env vars), never in the URL, never + written to `.git/config` or anywhere else on disk. +- `GIT_TERMINAL_PROMPT=0` and `GIT_ASKPASS=echo` ride every invocation: + a clone must never hang on a TTY prompt or an ambient IDE askpass. +- Failures map to named causes (`src/stages/clone.ts`): unusable URL / + unreachable repo → `invalid-git-url`, credential rejection → + `auth-failed`, anything else → `clone-failed`. + +### How the auth path is honestly tested + +`git daemon` (story 01's transport) has no authentication, so it cannot +prove credential delivery. The integration suite therefore also serves +the same bare fixture over **authenticated dumb HTTP** +(`tests/http-git-server.ts`): a real `git clone` probes, receives 401, +consults the ephemeral helper, and retries with Basic auth — the test +asserts the server received exactly `x-access-token:`, that git's +first probe was unauthenticated, and that the PAT appears nowhere in +runner logs (which include every spawned git argv), `.git/config`, or +any other file under `.git/`. Wrong/missing PAT → named `auth-failed`. +What is NOT covered: a real smart-HTTP provider (GitHub et al.) — the +dumb-HTTP fallback exercises the same credential machinery in git, but +the smart-protocol surface itself first meets reality in later PRDs. + ## Testing - Unit suites live next to sources in `src/`; protocol/WS suites live in diff --git a/apps/runner/src/exec.ts b/apps/runner/src/exec.ts new file mode 100644 index 0000000..043d7db --- /dev/null +++ b/apps/runner/src/exec.ts @@ -0,0 +1,55 @@ +import { execFile } from "node:child_process"; + +/** + * Minimal child-process runner shared by the stages. Array-args only — + * never a shell — so URLs and paths cannot be interpreted, and secrets + * can only travel via `env` (spec §6/§10). + */ +export interface CommandResult { + exitCode: number; + stdout: string; + stderr: string; +} + +export type RunCommand = ( + command: string, + args: string[], + options: { cwd: string; env?: Record }, +) => Promise; + +const MAX_OUTPUT_BYTES = 64 * 1024 * 1024; + +/** Ten minutes: covers a cold `npm install` of a real repo; prevents hangs. */ +const COMMAND_TIMEOUT_MS = 10 * 60 * 1000; + +export const runCommand: RunCommand = (command, args, options) => + new Promise((resolve, reject) => { + execFile( + command, + args, + { + cwd: options.cwd, + env: { ...process.env, ...options.env }, + maxBuffer: MAX_OUTPUT_BYTES, + timeout: COMMAND_TIMEOUT_MS, + }, + (error, stdout, stderr) => { + if (error === null) { + resolve({ exitCode: 0, stdout, stderr }); + return; + } + // Non-zero exit is a result, not an exception — stages map it to + // named protocol errors. Spawn-level failures still reject. + const exitCode = typeof error.code === "number" ? error.code : null; + if (exitCode !== null) { + resolve({ exitCode, stdout, stderr }); + return; + } + if (error.killed === true) { + resolve({ exitCode: 124, stdout, stderr: `${stderr}\n(timed out)` }); + return; + } + reject(error); + }, + ); + }); diff --git a/apps/runner/src/git/credential-helper.test.ts b/apps/runner/src/git/credential-helper.test.ts new file mode 100644 index 0000000..99747fd --- /dev/null +++ b/apps/runner/src/git/credential-helper.test.ts @@ -0,0 +1,67 @@ +import { describe, expect, it } from "vitest"; +import { + buildCredentialHelperArgs, + buildCredentialHelperEnv, + PAT_ENV_VAR, + PAT_USERNAME, + USERNAME_ENV_VAR, + VEECT_CREDENTIAL_HELPER, +} from "@/git/credential-helper"; + +const PAT = "ghp_super-secret-token-123"; + +describe("buildCredentialHelperArgs", () => { + it("passes the BLANK helper first to suppress OS keychain helpers (spec §6)", () => { + const args = buildCredentialHelperArgs(true); + expect(args.slice(0, 2)).toEqual(["-c", "credential.helper="]); + }); + + it("passes the veect helper second when a credential exists", () => { + const args = buildCredentialHelperArgs(true); + expect(args).toEqual([ + "-c", + "credential.helper=", + "-c", + `credential.helper=${VEECT_CREDENTIAL_HELPER}`, + ]); + }); + + it("still suppresses OS helpers when no credential exists", () => { + expect(buildCredentialHelperArgs(false)).toEqual([ + "-c", + "credential.helper=", + ]); + }); + + it("contains no secret material — only env var NAMES ride argv", () => { + // The helper string is argv-visible; it must reference the env vars + // by name and never embed a value. + expect(VEECT_CREDENTIAL_HELPER).toContain(`$${USERNAME_ENV_VAR}`); + expect(VEECT_CREDENTIAL_HELPER).toContain(`$${PAT_ENV_VAR}`); + expect(VEECT_CREDENTIAL_HELPER).toContain('"$1" = "get"'); + }); + + it("answers get only — store/erase are no-ops so git can never persist", () => { + expect(VEECT_CREDENTIAL_HELPER).toMatch( + /^!f\(\) \{ if \[ "\$1" = "get" \]/, + ); + }); +}); + +describe("buildCredentialHelperEnv", () => { + it("carries the PAT via env, with the conventional PAT username", () => { + expect(buildCredentialHelperEnv(PAT)).toEqual({ + GIT_TERMINAL_PROMPT: "0", + GIT_ASKPASS: "echo", + [USERNAME_ENV_VAR]: PAT_USERNAME, + [PAT_ENV_VAR]: PAT, + }); + }); + + it("disables terminal prompts and neutralizes ambient askpass so a clone can never hang", () => { + expect(buildCredentialHelperEnv(undefined)).toEqual({ + GIT_TERMINAL_PROMPT: "0", + GIT_ASKPASS: "echo", + }); + }); +}); diff --git a/apps/runner/src/git/credential-helper.ts b/apps/runner/src/git/credential-helper.ts new file mode 100644 index 0000000..c300d80 --- /dev/null +++ b/apps/runner/src/git/credential-helper.ts @@ -0,0 +1,71 @@ +/** + * Ephemeral git credential helper — tech spec §6 mechanics, verbatim: + * + * Every git invocation the runner spawns carries, in order: + * + * 1. `-c credential.helper=` — a BLANK helper FIRST. An empty value + * resets git's helper list, suppressing any OS-configured helpers + * (keychain / credential manager) that could answer or cache the + * credential. + * 2. `-c credential.helper=` — an inline shell helper + * that answers `get` by printing username/password sourced from the + * spawned child's environment. + * + * The secret reaches git via env (`VEECT_GIT_PAT` on the child process), + * NEVER via argv — the helper string only names the env vars — and never + * embedded in the remote URL (URLs with tokens leak into `.git/config` + * and process lists). The helper ignores `store`/`erase`, so git can + * never persist the credential anywhere. + */ + +/** Child-env variable the helper reads the PAT from. Never logged. */ +export const PAT_ENV_VAR = "VEECT_GIT_PAT"; + +/** Child-env variable the helper reads the username from. */ +export const USERNAME_ENV_VAR = "VEECT_GIT_USERNAME"; + +/** + * Username accompanying a PAT over HTTP basic auth. GitHub accepts any + * non-empty username for PATs; `x-access-token` is the conventional one. + */ +export const PAT_USERNAME = "x-access-token"; + +/** + * The inline helper. Contains NO secret material — only the names of the + * env vars — so it is safe inside argv. git invokes it with + * `get`/`store`/`erase` as `$1`; everything but `get` is a no-op. + */ +export const VEECT_CREDENTIAL_HELPER = `!f() { if [ "$1" = "get" ]; then printf 'username=%s\\npassword=%s\\n' "$${USERNAME_ENV_VAR}" "$${PAT_ENV_VAR}"; fi; }; f`; + +/** + * `-c` args for one git invocation: blank suppressor always; the veect + * helper only when a credential exists to serve. + */ +export function buildCredentialHelperArgs(withCredential: boolean): string[] { + const args = ["-c", "credential.helper="]; + if (withCredential) { + args.push("-c", `credential.helper=${VEECT_CREDENTIAL_HELPER}`); + } + return args; +} + +/** + * Env additions for one git invocation. `GIT_TERMINAL_PROMPT=0` rides + * every invocation — a clone must never hang on a TTY prompt (spec §6) — + * and `GIT_ASKPASS=echo` overrides any ambient askpass program (IDE + * shells export e.g. VS Code's askpass, which would block a headless + * clone on GUI IPC): `echo ` answers with an empty credential, + * so a missing/denied credential fails fast instead of hanging. The + * veect helper always answers first when a PAT exists. + */ +export function buildCredentialHelperEnv(pat?: string): Record { + const env: Record = { + GIT_TERMINAL_PROMPT: "0", + GIT_ASKPASS: "echo", + }; + if (pat !== undefined) { + env[USERNAME_ENV_VAR] = PAT_USERNAME; + env[PAT_ENV_VAR] = pat; + } + return env; +} diff --git a/apps/runner/src/server.ts b/apps/runner/src/server.ts index 611b903..a3f606d 100644 --- a/apps/runner/src/server.ts +++ b/apps/runner/src/server.ts @@ -8,7 +8,10 @@ import { type RunnerMessage, type RunnerStage, } from "@repo/core-runner-protocol"; +import { runCommand } from "./exec"; import type { Logger } from "./log"; +import { cloneRepository } from "./stages/clone"; +import { runStage, type StageEmitter } from "./stages/stage-runner"; /** * The runner's WS protocol server (ADR-027, protocol version "0"). @@ -137,38 +140,59 @@ export type CommandMessage = Extract< { type: "clone" | "install" | "scan" | "adapter-start" | "render-frame" } >; -export interface StageEmitter { - status: (stage: RunnerStage, elapsedMs: number) => void; - error: ( - cause: RunnerErrorCause, - message: string, - stage?: RunnerStage, - ) => void; -} - /** Stages that later walking-skeleton stories wire up (05+). */ const NOT_IMPLEMENTED: Record< - CommandMessage["type"], + Exclude, { cause: RunnerErrorCause; stage?: RunnerStage } > = { - clone: { cause: "clone-failed", stage: "cloning" }, install: { cause: "install-failed", stage: "installing" }, scan: { cause: "scan-failed", stage: "scanning" }, "adapter-start": { cause: "adapter-start-failed", stage: "starting-preview" }, "render-frame": { cause: "render-failed" }, }; +interface CommandContext { + emit: StageEmitter; + /** Signal successful command completion — the session's idle marker. */ + ready: () => void; + options: RunnerServerOptions; +} + async function handleCommand( command: CommandMessage, - emit: StageEmitter, - _options: RunnerServerOptions, + ctx: CommandContext, ): Promise { - const notImplemented = NOT_IMPLEMENTED[command.type]; - emit.error( - notImplemented.cause, - `the "${command.type}" command is not implemented by this runner build yet`, - notImplemented.stage, - ); + const { emit, options } = ctx; + const stageOptions = { + emit, + heartbeatMs: options.heartbeatMs, + log: options.log, + }; + switch (command.type) { + case "clone": { + const result = await runStage( + "cloning", + "clone-failed", + stageOptions, + () => + cloneRepository({ + workspaceDir: options.workspaceDir, + runCommand, + log: options.log, + })(command), + ); + if (result.ok) ctx.ready(); + return; + } + default: { + const notImplemented = NOT_IMPLEMENTED[command.type]; + emit.error( + notImplemented.cause, + `the "${command.type}" command is not implemented by this runner build yet`, + notImplemented.stage, + ); + } + } } function rawToString(data: RawData): string { @@ -257,7 +281,11 @@ export async function startRunnerServer( log.info("command-received", { command: command.type }); commandChain = commandChain.then(async () => { try { - await handleCommand(command, emit, options); + await handleCommand(command, { + emit, + ready: () => send({ type: "ready" }), + options, + }); } catch (error) { // Stages map their own failures; this is the last-resort net. log.error("command-crashed", { diff --git a/apps/runner/src/stages/clone.test.ts b/apps/runner/src/stages/clone.test.ts new file mode 100644 index 0000000..71cff2b --- /dev/null +++ b/apps/runner/src/stages/clone.test.ts @@ -0,0 +1,247 @@ +import { mkdir, mkdtemp, writeFile } from "node:fs/promises"; +import { existsSync } from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { describe, expect, it } from "vitest"; +import { + buildCloneInvocation, + cloneRepository, + mapCloneFailure, + repoPath, + validateGitUrl, +} from "@/stages/clone"; +import { StageError } from "@/stages/stage-runner"; +import { PAT_ENV_VAR } from "@/git/credential-helper"; +import { createLogger } from "@/log"; +import type { CommandResult, RunCommand } from "@/exec"; + +const PAT = "ghp_never-in-argv-or-config"; + +function fakeRun(result: Partial): { + run: RunCommand; + calls: unknown[][]; +} { + const calls: unknown[][] = []; + const run: RunCommand = async (...args) => { + calls.push(args); + return { exitCode: 0, stdout: "", stderr: "", ...result }; + }; + return { run, calls }; +} + +describe("validateGitUrl", () => { + it("accepts git, http(s), and file URLs", () => { + expect(validateGitUrl("git://127.0.0.1:9418/vite-kitchen.git")).toBeNull(); + expect(validateGitUrl("https://github.com/acme/app.git")).toBeNull(); + expect(validateGitUrl("http://127.0.0.1:8080/repo.git")).toBeNull(); + expect(validateGitUrl("file:///tmp/fixture.git")).toBeNull(); + }); + + it("rejects junk that is not a URL", () => { + expect(validateGitUrl("not a url at all")).toMatch(/not a valid URL/); + }); + + it("rejects protocols the runner does not clone from", () => { + expect(validateGitUrl("ssh://git@github.com/acme/app.git")).toMatch( + /unsupported protocol/, + ); + expect(validateGitUrl("ftp://example.com/repo.git")).toMatch( + /unsupported protocol/, + ); + }); +}); + +describe("buildCloneInvocation", () => { + it("never places the PAT in argv — env is the only channel (spec §6)", () => { + const invocation = buildCloneInvocation( + "https://example.com/a.git", + "/ws/repo", + PAT, + ); + expect(invocation.args.join(" ")).not.toContain(PAT); + expect(invocation.env[PAT_ENV_VAR]).toBe(PAT); + }); + + it("orders helpers blank-first and ends options with -- before the URL", () => { + const invocation = buildCloneInvocation( + "https://example.com/a.git", + "/ws/repo", + PAT, + ); + const flat = invocation.args; + expect(flat.indexOf("credential.helper=")).toBeLessThan( + flat.findIndex((arg) => arg.startsWith("credential.helper=!f()")), + ); + expect(flat.slice(-3)).toEqual([ + "--", + "https://example.com/a.git", + "/ws/repo", + ]); + }); + + it("omits the veect helper (but keeps the suppressor) without a PAT", () => { + const invocation = buildCloneInvocation("git://h/a.git", "/ws/repo"); + expect( + invocation.args.filter((arg) => arg.startsWith("credential.helper")), + ).toEqual(["credential.helper="]); + expect(invocation.env[PAT_ENV_VAR]).toBeUndefined(); + expect(invocation.env.GIT_TERMINAL_PROMPT).toBe("0"); + }); +}); + +describe("mapCloneFailure", () => { + it.each([ + // git daemon refuses a repo outside its export root — the URL is + // wrong, and "access denied" here must NOT read as an auth failure. + [ + "fatal: remote error: access denied or repository not exported: /nope.git", + "invalid-git-url", + ], + ["fatal: repository 'https://x.test/a.git/' not found", "invalid-git-url"], + [ + "fatal: unable to access 'https://x.test/a.git/': The requested URL returned error: 404", + "invalid-git-url", + ], + [ + "fatal: unable to access 'https://gone.test/a.git/': Could not resolve host: gone.test", + "invalid-git-url", + ], + [ + "fatal: '/tmp/missing.git' does not appear to be a git repository", + "invalid-git-url", + ], + ["fatal: Authentication failed for 'https://x.test/a.git/'", "auth-failed"], + [ + "fatal: could not read Username for 'https://x.test': terminal prompts disabled", + "auth-failed", + ], + [ + "fatal: unable to access 'https://x.test/a.git/': The requested URL returned error: 401", + "auth-failed", + ], + [ + "fatal: unable to access 'https://x.test/a.git/': The requested URL returned error: 403", + "auth-failed", + ], + ["remote: Invalid username or password.", "auth-failed"], + ["fatal: the remote end hung up unexpectedly", "clone-failed"], + ["", "clone-failed"], + ])("maps %j to %s", (stderr, cause) => { + const error = mapCloneFailure(stderr); + expect(error).toBeInstanceOf(StageError); + expect(error.namedCause).toBe(cause); + }); + + it("keeps only a bounded single-line tail of stderr in the message", () => { + const error = mapCloneFailure(`line one\nline two\n${"x".repeat(1000)}`); + expect(error.message).not.toContain("\n"); + expect(error.message.length).toBeLessThan(500); + }); +}); + +describe("cloneRepository", () => { + it("rejects an invalid URL before ever spawning git", async () => { + const { run, calls } = fakeRun({}); + const clone = cloneRepository({ + workspaceDir: "/ws", + runCommand: run, + log: createLogger(() => undefined), + }); + + await expect( + clone({ type: "clone", gitUrl: "not a url" }), + ).rejects.toMatchObject({ + namedCause: "invalid-git-url", + }); + expect(calls).toHaveLength(0); + }); + + it("spawns git clone into /repo and resolves the dest", async () => { + const workspaceDir = await mkdtemp( + path.join(os.tmpdir(), "veect-clone-unit-"), + ); + const { run, calls } = fakeRun({}); + const clone = cloneRepository({ + workspaceDir, + runCommand: run, + log: createLogger(() => undefined), + }); + + const dest = await clone({ + type: "clone", + gitUrl: "git://h/a.git", + pat: PAT, + }); + + expect(dest).toBe(repoPath(workspaceDir)); + expect(calls).toHaveLength(1); + const [command, args, options] = calls[0] as [ + string, + string[], + { env: Record }, + ]; + expect(command).toBe("git"); + expect(args).toContain("clone"); + expect(args.join(" ")).not.toContain(PAT); + expect(options.env[PAT_ENV_VAR]).toBe(PAT); + }); + + it("clears a stale /repo before cloning (idempotent retry)", async () => { + const workspaceDir = await mkdtemp( + path.join(os.tmpdir(), "veect-clone-unit-"), + ); + const stale = path.join(repoPath(workspaceDir), "stale.txt"); + await mkdir(repoPath(workspaceDir), { recursive: true }); + await writeFile(stale, "old"); + const { run } = fakeRun({}); + const clone = cloneRepository({ + workspaceDir, + runCommand: run, + log: createLogger(() => undefined), + }); + + await clone({ type: "clone", gitUrl: "git://h/a.git" }); + + expect(existsSync(stale)).toBe(false); + }); + + it("throws the mapped StageError when git exits non-zero", async () => { + const workspaceDir = await mkdtemp( + path.join(os.tmpdir(), "veect-clone-unit-"), + ); + const { run } = fakeRun({ + exitCode: 128, + stderr: "fatal: Authentication failed for 'https://x.test/a.git/'", + }); + const clone = cloneRepository({ + workspaceDir, + runCommand: run, + log: createLogger(() => undefined), + }); + + await expect( + clone({ type: "clone", gitUrl: "https://x.test/a.git" }), + ).rejects.toMatchObject({ + namedCause: "auth-failed", + }); + }); + + it("logs the spawned argv (secret-free) and never the PAT", async () => { + const workspaceDir = await mkdtemp( + path.join(os.tmpdir(), "veect-clone-unit-"), + ); + const lines: string[] = []; + const { run } = fakeRun({}); + const clone = cloneRepository({ + workspaceDir, + runCommand: run, + log: createLogger((line) => lines.push(line)), + }); + + await clone({ type: "clone", gitUrl: "git://h/a.git", pat: PAT }); + + const logged = lines.join("\n"); + expect(logged).toContain("git-spawn"); + expect(logged).not.toContain(PAT); + }); +}); diff --git a/apps/runner/src/stages/clone.ts b/apps/runner/src/stages/clone.ts new file mode 100644 index 0000000..844bd4f --- /dev/null +++ b/apps/runner/src/stages/clone.ts @@ -0,0 +1,156 @@ +import { rm } from "node:fs/promises"; +import path from "node:path"; +import type { CloneMessage } from "@repo/core-runner-protocol"; +import { + buildCredentialHelperArgs, + buildCredentialHelperEnv, +} from "../git/credential-helper"; +import type { RunCommand } from "../exec"; +import type { Logger } from "../log"; +import { StageError } from "./stage-runner"; + +/** Where the workspace's clone lives: `/repo`. */ +export const REPO_DIR_NAME = "repo"; + +export function repoPath(workspaceDir: string): string { + return path.join(workspaceDir, REPO_DIR_NAME); +} + +/** + * Protocols the runner will clone from. SSH is deliberately absent for + * cloud runners (ADR-027 retires SSH-via-agent there); `file:` and + * `git:` exist for the fixture-serving integration suites. + */ +const ALLOWED_PROTOCOLS = new Set(["git:", "http:", "https:", "file:"]); + +/** Returns a human-readable rejection, or null when the URL is usable. */ +export function validateGitUrl(gitUrl: string): string | null { + let url: URL; + try { + url = new URL(gitUrl); + } catch { + return `"${gitUrl}" is not a valid URL`; + } + if (!ALLOWED_PROTOCOLS.has(url.protocol)) { + return `unsupported protocol "${url.protocol}" (allowed: git:, http:, https:, file:)`; + } + return null; +} + +export interface GitInvocation { + /** argv for `git` — contains no secret material by construction. */ + args: string[]; + /** Env additions for the child — the only channel a PAT travels on. */ + env: Record; +} + +/** + * Build the clone invocation per spec §6/§10: blank-then-veect + * credential helpers, array args, `--` end-of-options, credential via + * child env only. + */ +export function buildCloneInvocation( + gitUrl: string, + dest: string, + pat?: string, +): GitInvocation { + return { + args: [ + ...buildCredentialHelperArgs(pat !== undefined), + "clone", + "--", + gitUrl, + dest, + ], + env: buildCredentialHelperEnv(pat), + }; +} + +/** + * Ordering matters: the bad-URL signatures are checked first because + * `git daemon`'s "access denied or repository not exported" would + * otherwise false-positive the auth patterns; over the git protocol + * there is no authentication at all. + */ +const BAD_URL_PATTERNS: RegExp[] = [ + /repository not exported/i, // git daemon: not-exported path + /repository .* not found/i, + /returned error: 404/i, + /could not resolve host/i, + /does not appear to be a git repository/i, + /no such file or directory/i, // file:// to a missing path + /protocol .* is not supported/i, +]; + +const AUTH_FAILURE_PATTERNS: RegExp[] = [ + /authentication failed/i, + /invalid username or password/i, + /could not read username/i, + /could not read password/i, + /returned error: 401/i, + /returned error: 403/i, + /access denied/i, +]; + +/** Trimmed single-line tail of git's stderr for the protocol error message. */ +function stderrTail(stderr: string): string { + const flat = stderr + .trim() + .split("\n") + .map((l) => l.trim()) + .filter(Boolean) + .join(" | "); + return flat.length > 400 ? `…${flat.slice(-400)}` : flat; +} + +/** Map a failed `git clone` to a named protocol cause (PRD user story 5). */ +export function mapCloneFailure(stderr: string): StageError { + const detail = stderrTail(stderr) || "git clone failed with no output"; + if (BAD_URL_PATTERNS.some((pattern) => pattern.test(stderr))) { + return new StageError( + "invalid-git-url", + `git could not reach the repository: ${detail}`, + ); + } + if (AUTH_FAILURE_PATTERNS.some((pattern) => pattern.test(stderr))) { + return new StageError( + "auth-failed", + `git authentication failed: ${detail}`, + ); + } + return new StageError("clone-failed", `git clone failed: ${detail}`); +} + +export interface CloneStageDeps { + workspaceDir: string; + runCommand: RunCommand; + log: Logger; +} + +/** + * The clone stage: validate URL → reset `/repo` (idempotent + * retry) → `git clone` with the ephemeral credential helper. Throws + * `StageError` with a named cause on every failure path. + */ +export function cloneRepository(deps: CloneStageDeps) { + return async (message: CloneMessage): Promise => { + const rejection = validateGitUrl(message.gitUrl); + if (rejection !== null) { + throw new StageError("invalid-git-url", rejection); + } + const dest = repoPath(deps.workspaceDir); + await rm(dest, { recursive: true, force: true }); + const invocation = buildCloneInvocation(message.gitUrl, dest, message.pat); + // argv is secret-free by construction; logging it lets the + // integration suite grep the spawned invocation for leaks (spec §15). + deps.log.info("git-spawn", { args: invocation.args }); + const result = await deps.runCommand("git", invocation.args, { + cwd: deps.workspaceDir, + env: invocation.env, + }); + if (result.exitCode !== 0) { + throw mapCloneFailure(result.stderr); + } + return dest; + }; +} diff --git a/apps/runner/src/stages/stage-runner.test.ts b/apps/runner/src/stages/stage-runner.test.ts new file mode 100644 index 0000000..7b0827d --- /dev/null +++ b/apps/runner/src/stages/stage-runner.test.ts @@ -0,0 +1,115 @@ +import { describe, expect, it } from "vitest"; +import type { RunnerErrorCause, RunnerStage } from "@repo/core-runner-protocol"; +import { runStage, StageError, type StageEmitter } from "@/stages/stage-runner"; +import { createLogger } from "@/log"; + +interface Recorded { + statuses: { stage: RunnerStage; elapsedMs: number }[]; + errors: { cause: RunnerErrorCause; message: string; stage?: RunnerStage }[]; +} + +function recordingEmitter(): { emit: StageEmitter; recorded: Recorded } { + const recorded: Recorded = { statuses: [], errors: [] }; + return { + recorded, + emit: { + status: (stage, elapsedMs) => + recorded.statuses.push({ stage, elapsedMs }), + error: (cause, message, stage) => + recorded.errors.push({ cause, message, stage }), + }, + }; +} + +function options(emit: StageEmitter, heartbeatMs = 10_000) { + return { emit, heartbeatMs, log: createLogger(() => undefined) }; +} + +const delay = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); + +describe("runStage", () => { + it("emits a start status (0ms) and a final status on success", async () => { + const { emit, recorded } = recordingEmitter(); + const result = await runStage( + "cloning", + "clone-failed", + options(emit), + async () => 42, + ); + + expect(result).toEqual({ ok: true, value: 42 }); + expect(recorded.errors).toEqual([]); + expect(recorded.statuses.length).toBeGreaterThanOrEqual(2); + expect(recorded.statuses[0]).toEqual({ stage: "cloning", elapsedMs: 0 }); + const last = recorded.statuses.at(-1); + expect(last?.stage).toBe("cloning"); + expect(last?.elapsedMs).toBeGreaterThanOrEqual(0); + }); + + it("streams heartbeat statuses with nondecreasing elapsedMs while work runs", async () => { + const { emit, recorded } = recordingEmitter(); + await runStage("installing", "install-failed", options(emit, 20), () => + delay(150), + ); + + expect(recorded.statuses.length).toBeGreaterThanOrEqual(4); + const elapsed = recorded.statuses.map((s) => s.elapsedMs); + expect([...elapsed].sort((a, b) => a - b)).toEqual(elapsed); + expect(recorded.statuses.every((s) => s.stage === "installing")).toBe(true); + }); + + it("stops heartbeats once the stage completes", async () => { + const { emit, recorded } = recordingEmitter(); + await runStage( + "cloning", + "clone-failed", + options(emit, 10), + async () => "done", + ); + const count = recorded.statuses.length; + await delay(60); + expect(recorded.statuses.length).toBe(count); + }); + + it("maps a StageError to its named cause with the failing stage", async () => { + const { emit, recorded } = recordingEmitter(); + const result = await runStage( + "cloning", + "clone-failed", + options(emit), + async () => { + throw new StageError("invalid-git-url", "nope"); + }, + ); + + expect(result).toEqual({ ok: false }); + expect(recorded.errors).toEqual([ + { cause: "invalid-git-url", message: "nope", stage: "cloning" }, + ]); + }); + + it("maps an unexpected throw to the stage's fallback cause", async () => { + const { emit, recorded } = recordingEmitter(); + const result = await runStage( + "installing", + "install-failed", + options(emit), + async () => { + throw new Error("ECONNRESET"); + }, + ); + + expect(result).toEqual({ ok: false }); + expect(recorded.errors).toEqual([ + { cause: "install-failed", message: "ECONNRESET", stage: "installing" }, + ]); + }); + + it("stringifies non-Error throws", async () => { + const { emit, recorded } = recordingEmitter(); + await runStage("cloning", "clone-failed", options(emit), async () => { + throw "raw string"; + }); + expect(recorded.errors[0]?.message).toBe("raw string"); + }); +}); diff --git a/apps/runner/src/stages/stage-runner.ts b/apps/runner/src/stages/stage-runner.ts new file mode 100644 index 0000000..2f210ee --- /dev/null +++ b/apps/runner/src/stages/stage-runner.ts @@ -0,0 +1,67 @@ +import type { RunnerErrorCause, RunnerStage } from "@repo/core-runner-protocol"; +import type { Logger } from "../log"; + +/** A stage failure that already knows its named protocol cause. */ +export class StageError extends Error { + readonly namedCause: RunnerErrorCause; + + constructor(namedCause: RunnerErrorCause, message: string) { + super(message); + this.name = "StageError"; + this.namedCause = namedCause; + } +} + +/** How a stage talks back to the connected client. */ +export interface StageEmitter { + status: (stage: RunnerStage, elapsedMs: number) => void; + error: ( + cause: RunnerErrorCause, + message: string, + stage?: RunnerStage, + ) => void; +} + +export interface StageRunOptions { + emit: StageEmitter; + heartbeatMs: number; + log: Logger; +} + +export type StageResult = { ok: true; value: T } | { ok: false }; + +/** + * Run one stage with staged, honest progress (ui-gap §5): a `status` at + * start (elapsedMs 0), heartbeat `status` events while the work runs, a + * final `status` with the total elapsed on success. On failure, emits + * one named `error` event (`StageError` carries the cause; anything else + * maps to `fallbackCause`) — never a blank board. + */ +export async function runStage( + stage: RunnerStage, + fallbackCause: RunnerErrorCause, + options: StageRunOptions, + work: () => Promise, +): Promise> { + const startedAt = Date.now(); + const elapsed = (): number => Math.max(0, Math.round(Date.now() - startedAt)); + options.emit.status(stage, 0); + const heartbeat = setInterval(() => { + options.emit.status(stage, elapsed()); + }, options.heartbeatMs); + try { + const value = await work(); + options.emit.status(stage, elapsed()); + options.log.info("stage-complete", { stage, elapsedMs: elapsed() }); + return { ok: true, value }; + } catch (error) { + const cause = + error instanceof StageError ? error.namedCause : fallbackCause; + const message = error instanceof Error ? error.message : String(error); + options.log.error("stage-failed", { stage, cause }); + options.emit.error(cause, message, stage); + return { ok: false }; + } finally { + clearInterval(heartbeat); + } +} diff --git a/apps/runner/tests/clone-stage.integration.test.ts b/apps/runner/tests/clone-stage.integration.test.ts new file mode 100644 index 0000000..ccd98d2 --- /dev/null +++ b/apps/runner/tests/clone-stage.integration.test.ts @@ -0,0 +1,272 @@ +/** + * Clone-stage integration suite: real WS protocol session, real `git` + * subprocesses, real transports. + * + * Transport per concern: + * - `git daemon` (story 01's helper) proves the primary clone path and + * the named bad-URL failure — the git protocol has NO authentication, + * so it cannot prove credential delivery. + * - An authenticated dumb-HTTP server over the same bare repo proves the + * ephemeral-credential-helper mechanics end to end: git receives a 401, + * consults the helper, retries with Basic auth — and the server records + * exactly which credential arrived. Success with the right PAT + the + * named `auth-failed` failure with a wrong/missing PAT. + * + * Credential-leak assertions (spec §15) ride the success path: the PAT + * must appear nowhere in the runner's logs (which include every spawned + * git argv), nowhere under the clone's `.git/config`, and nowhere in the + * clone's stored remote URL. + */ +import { existsSync } from "node:fs"; +import { mkdtemp, readdir, readFile, rm } from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; +import { + afterAll, + afterEach, + beforeAll, + beforeEach, + describe, + expect, + it, +} from "vitest"; +import { + serveFixtureRepo, + type ServedFixtureRepo, +} from "@repo/core-testing/git"; +import { startRunnerServer, type RunnerServer } from "@/server"; +import { createLogger } from "@/log"; +import { repoPath } from "@/stages/clone"; +import { PAT_USERNAME } from "@/git/credential-helper"; +import { + connectProtocolClient, + expectNamedError, + handshake, + sendAndAwaitReady, + type ProtocolClient, +} from "./protocol-client"; +import { + serveBareRepoOverHttp, + type AuthedGitHttpServer, +} from "./http-git-server"; + +const TOKEN = "clone-suite-token"; +const PAT = "ghp_integration-pat-7f3a9"; + +/** repo-root/fixtures/vite-kitchen, resolved from this file's location. */ +const FIXTURE_DIR = path.resolve( + path.dirname(fileURLToPath(import.meta.url)), + "../../../fixtures/vite-kitchen", +); + +let served: ServedFixtureRepo; + +beforeAll(async () => { + // One served fixture for the whole suite (cheap teardown per test is + // the runner server, not the daemon). + served = await serveFixtureRepo(FIXTURE_DIR); +}); + +afterAll(async () => { + await served.stop(); +}); + +let server: RunnerServer; +let workspaceDir: string; +let logLines: string[]; +let clients: ProtocolClient[]; + +async function startSession(): Promise { + workspaceDir = await mkdtemp(path.join(os.tmpdir(), "veect-clone-int-")); + logLines = []; + server = await startRunnerServer({ + host: "127.0.0.1", + port: 0, + token: TOKEN, + workspaceDir, + heartbeatMs: 100, + log: createLogger((line) => logLines.push(line)), + }); + const client = await connectProtocolClient(server.port, TOKEN); + clients.push(client); + await handshake(client); + return client; +} + +beforeEach(() => { + clients = []; +}); + +afterEach(async () => { + for (const client of clients) client.close(); + await server.close(); + await rm(workspaceDir, { recursive: true, force: true }); +}); + +/** Every file under the clone's .git that could persist a credential. */ +async function gitDirLeakSurface(cloneDir: string): Promise { + const gitDir = path.join(cloneDir, ".git"); + const parts: string[] = []; + const walk = async (dir: string): Promise => { + for (const entry of await readdir(dir, { withFileTypes: true })) { + const full = path.join(dir, entry.name); + if (entry.isDirectory()) { + if (entry.name === "objects") continue; // packfiles: binary repo data + await walk(full); + } else { + parts.push(await readFile(full, "utf8").catch(() => "")); + } + } + }; + await walk(gitDir); + return parts.join("\n"); +} + +/** + * Leak assertions (spec §15): the PAT must appear nowhere in the runner + * logs (which include every spawned git argv), the clone's + * `.git/config`, or any other file under `.git/`. + */ +async function expectNoCredentialLeak(cloneDir: string): Promise { + expect(logLines.join("\n")).not.toContain(PAT); + const gitConfig = await readFile( + path.join(cloneDir, ".git", "config"), + "utf8", + ); + expect(gitConfig).not.toContain(PAT); + expect(await gitDirLeakSurface(cloneDir)).not.toContain(PAT); +} + +describe("clone via git daemon (primary transport)", () => { + it("clones vite-kitchen with staged progress and no credential leakage", async () => { + const client = await startSession(); + // A PAT is supplied even though the git protocol never asks for it — + // the helper env rides the invocation, so the leak assertions below + // are exercised against a real spawned git. + await sendAndAwaitReady( + client, + { type: "clone", gitUrl: served.cloneUrl, pat: PAT }, + "clone completion", + ); + + // Staged, honest progress: first cloning status is 0ms; elapsed is + // nondecreasing; completion signalled by ready. + const statuses = client + .received() + .flatMap((m) => (m.type === "status" ? [m] : [])); + expect(statuses.length).toBeGreaterThanOrEqual(2); + expect(statuses[0]).toEqual({ + type: "status", + stage: "cloning", + elapsedMs: 0, + }); + const elapsed = statuses.map((s) => s.elapsedMs); + expect([...elapsed].sort((a, b) => a - b)).toEqual(elapsed); + + // The clone actually landed, and the stored remote URL is the + // credential-free URL, verbatim. + const cloneDir = repoPath(workspaceDir); + expect(existsSync(path.join(cloneDir, "package.json"))).toBe(true); + const gitConfig = await readFile( + path.join(cloneDir, ".git", "config"), + "utf8", + ); + expect(gitConfig).toContain(served.cloneUrl); + + await expectNoCredentialLeak(cloneDir); + }); + + it("emits the named invalid-git-url failure for a repo the daemon does not export", async () => { + const client = await startSession(); + const base = served.cloneUrl.replace(/\/[^/]+$/, ""); + client.send({ + type: "clone", + gitUrl: `${base}/definitely-not-exported.git`, + }); + const error = await expectNamedError(client, "invalid-git-url"); + expect(error.stage).toBe("cloning"); + }); + + it("emits the named invalid-git-url failure for a malformed URL without spawning git", async () => { + const client = await startSession(); + client.send({ type: "clone", gitUrl: "not a url at all" }); + const error = await expectNamedError(client, "invalid-git-url"); + expect(error.stage).toBe("cloning"); + expect(logLines.join("\n")).not.toContain("git-spawn"); + }); + + it("keeps the session usable after a failed clone (corrected retry succeeds)", async () => { + const client = await startSession(); + client.send({ type: "clone", gitUrl: "not a url at all" }); + await expectNamedError(client, "invalid-git-url"); + await sendAndAwaitReady( + client, + { type: "clone", gitUrl: served.cloneUrl }, + "retried clone completion", + ); + expect(existsSync(path.join(repoPath(workspaceDir), "package.json"))).toBe( + true, + ); + }); +}); + +describe("clone via authenticated HTTP (credential-helper mechanics)", () => { + let httpServer: AuthedGitHttpServer; + + afterEach(async () => { + await httpServer.stop(); + }); + + it("delivers the PAT to git via the ephemeral helper — and only via it", async () => { + httpServer = await serveBareRepoOverHttp(served.bareRepoPath, { + username: PAT_USERNAME, + password: PAT, + }); + const client = await startSession(); + await sendAndAwaitReady( + client, + { type: "clone", gitUrl: httpServer.cloneUrl, pat: PAT }, + "authenticated clone completion", + ); + + // git's first probe is unauthenticated; the 401 made it consult the + // helper; the retried request carried exactly username:PAT. + expect(httpServer.unauthenticatedRequests()).toBeGreaterThanOrEqual(1); + const expected = + "Basic " + Buffer.from(`${PAT_USERNAME}:${PAT}`).toString("base64"); + expect(httpServer.authorizationHeaders().length).toBeGreaterThanOrEqual(1); + expect( + httpServer.authorizationHeaders().every((header) => header === expected), + ).toBe(true); + + // And the credential persisted nowhere. + await expectNoCredentialLeak(repoPath(workspaceDir)); + }); + + it("emits the named auth-failed failure for a wrong PAT", async () => { + httpServer = await serveBareRepoOverHttp(served.bareRepoPath, { + username: PAT_USERNAME, + password: PAT, + }); + const client = await startSession(); + client.send({ + type: "clone", + gitUrl: httpServer.cloneUrl, + pat: "wrong-pat", + }); + const error = await expectNamedError(client, "auth-failed"); + expect(error.stage).toBe("cloning"); + }); + + it("emits the named auth-failed failure when the PAT is missing entirely", async () => { + httpServer = await serveBareRepoOverHttp(served.bareRepoPath, { + username: PAT_USERNAME, + password: PAT, + }); + const client = await startSession(); + client.send({ type: "clone", gitUrl: httpServer.cloneUrl }); + const error = await expectNamedError(client, "auth-failed"); + expect(error.stage).toBe("cloning"); + }); +}); diff --git a/apps/runner/tests/http-git-server.ts b/apps/runner/tests/http-git-server.ts new file mode 100644 index 0000000..4d19a57 --- /dev/null +++ b/apps/runner/tests/http-git-server.ts @@ -0,0 +1,107 @@ +import { execFile } from "node:child_process"; +import { once } from "node:events"; +import { createReadStream } from "node:fs"; +import { stat } from "node:fs/promises"; +import { createServer } from "node:http"; +import path from "node:path"; +import { promisify } from "node:util"; + +const execFileAsync = promisify(execFile); + +/** + * Serve a bare repository over authenticated "dumb" HTTP. + * + * `git daemon` (story 01's default transport) has no authentication, so + * it cannot exercise the ephemeral-credential-helper path. This helper + * can: a plain static-file server over the bare repo (dumb git-HTTP + * protocol — enabled by running `git update-server-info`) that rejects + * every request lacking the expected HTTP Basic credential with a 401 + + * `WWW-Authenticate`. A real `git clone` against it performs the real + * credential dance: unauthenticated probe → 401 → git consults the + * configured credential helper → retry with Basic auth. + * + * Every received Authorization header is recorded, so tests can assert + * the exact credential git presented — i.e. that the PAT reached git + * through the helper (and only through it). + */ +export interface AuthedGitHttpServer { + /** Clone URL: `http://127.0.0.1:/repo.git` */ + cloneUrl: string; + /** Every Authorization header received, in order. */ + authorizationHeaders: () => string[]; + /** Count of requests that arrived with no Authorization header. */ + unauthenticatedRequests: () => number; + stop: () => Promise; +} + +export async function serveBareRepoOverHttp( + bareRepoPath: string, + credentials: { username: string; password: string }, +): Promise { + // Generate info/refs + objects/info/packs so the dumb protocol works. + await execFileAsync("git", ["update-server-info"], { cwd: bareRepoPath }); + + const expected = + "Basic " + + Buffer.from(`${credentials.username}:${credentials.password}`).toString( + "base64", + ); + const received: string[] = []; + let unauthenticated = 0; + + const server = createServer((req, res) => { + const auth = req.headers.authorization; + if (auth === undefined) { + unauthenticated += 1; + } else { + received.push(auth); + } + if (auth !== expected) { + res.writeHead(401, { "WWW-Authenticate": 'Basic realm="veect-fixture"' }); + res.end("authentication required"); + return; + } + const urlPath = decodeURIComponent((req.url ?? "/").split("?")[0] ?? "/"); + const relative = urlPath.replace(/^\/repo\.git\/?/, ""); + const file = path.resolve(bareRepoPath, relative); + if (!file.startsWith(path.resolve(bareRepoPath) + path.sep)) { + res.writeHead(403); + res.end(); + return; + } + void stat(file).then( + (stats) => { + if (!stats.isFile()) { + res.writeHead(404); + res.end(); + return; + } + res.writeHead(200, { "content-type": "application/octet-stream" }); + createReadStream(file).pipe(res); + }, + () => { + res.writeHead(404); + res.end(); + }, + ); + }); + + server.listen(0, "127.0.0.1"); + await once(server, "listening"); + const address = server.address(); + if (address === null || typeof address === "string") { + server.close(); + throw new Error("serveBareRepoOverHttp: could not determine a port"); + } + + return { + cloneUrl: `http://127.0.0.1:${address.port}/repo.git`, + authorizationHeaders: () => [...received], + unauthenticatedRequests: () => unauthenticated, + stop: () => + new Promise((resolve, reject) => { + server.closeAllConnections(); + server.close((error) => (error ? reject(error) : resolve())); + }), + }; +} diff --git a/apps/runner/tests/protocol-client.ts b/apps/runner/tests/protocol-client.ts index 6f03a37..35bbc70 100644 --- a/apps/runner/tests/protocol-client.ts +++ b/apps/runner/tests/protocol-client.ts @@ -115,6 +115,32 @@ export async function handshake( return client.waitFor((m) => m.type === "ready", "ready handshake reply"); } +/** + * Send a command and resolve on the runner's NEXT `ready` — the + * protocol's success marker for a completed command. Counting readies + * (instead of matching any `ready`) keeps this correct on sessions that + * already completed earlier commands. + */ +export async function sendAndAwaitReady( + client: ProtocolClient, + message: RunnerMessage, + description: string, + timeoutMs?: number, +): Promise { + const readiesBefore = client + .received() + .filter((m) => m.type === "ready").length; + client.send(message); + await client.waitFor( + (m) => + m.type === "ready" && + client.received().filter((r) => r.type === "ready").length > + readiesBefore, + description, + timeoutMs, + ); +} + /** Wait for the next `error` event and assert its named cause. */ export async function expectNamedError( client: ProtocolClient,