diff --git a/.changeset/simple-git-4.md b/.changeset/simple-git-4.md new file mode 100644 index 00000000..d65ce856 --- /dev/null +++ b/.changeset/simple-git-4.md @@ -0,0 +1,8 @@ +--- +"@openspec-ui/core": patch +"@openspec-ui/cli": patch +"@openspec-ui/server": patch +"openspec-ui-vscode": patch +--- + +simple-git 4.0.2, which fixes two critical advisories in earlier versions (GHSA-v5rq-49vh-5v5c, GHSA-x6jw-m9v5-85vh) (simple-git-4). simple-git 4 removes `GIT_` and other guarded variables from the environment of the git it starts; the product now opens git through one function that lets through only what a person set up to reach a remote - `GIT_ASKPASS`, `SSH_ASKPASS`, `GIT_SSH`, `GIT_SSH_COMMAND`, `GIT_SSH_VARIANT`, `GIT_TERMINAL_PROMPT` - so pushes keep authenticating, and keeps every other guarded variable away from git. diff --git a/openspec/changes/simple-git-4/.openspec.yaml b/openspec/changes/simple-git-4/.openspec.yaml new file mode 100644 index 00000000..aa32566b --- /dev/null +++ b/openspec/changes/simple-git-4/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-10-06 diff --git a/openspec/changes/simple-git-4/design.md b/openspec/changes/simple-git-4/design.md new file mode 100644 index 00000000..a3c2dca8 --- /dev/null +++ b/openspec/changes/simple-git-4/design.md @@ -0,0 +1,41 @@ +## Context + +simple-git 4 (`allowEnvironment`, `@simple-git/argv-parser`'s +`isGitEnvKey`) strips, from the environment of every git it runs, each key +starting with `GIT_` and `editor`, `pager`, `prefix`, `ssh_askpass`, +`visual`. A key passed through `.env()` that is guarded and not allowed +rejects the task. Of the allowed ones, those that run a program +(`GIT_ASKPASS`, `SSH_ASKPASS`, `GIT_SSH`, `GIT_SSH_COMMAND`) also need the +matching `unsafe` flag, or the task is refused. + +This product runs git through simple-git for status, diff, log, blame, +commit, push, fetch, pull and rebase, in the extension host, the server +and the CLI, and never passes an environment of its own. + +## Decisions + +1. **One function opens git.** `openGit(cwd)` in `git-client.ts` replaces + every `simpleGit(cwd)` outside tests, so what reaches git is decided in + one place. +2. **The connection variables pass, the rest do not.** `GIT_ASKPASS`, + `SSH_ASKPASS`, `GIT_SSH`, `GIT_SSH_COMMAND`, `GIT_SSH_VARIANT` and + `GIT_TERMINAL_PROMPT`, with `allowUnsafeAskPass` and + `allowUnsafeSshCommand`. They come from the person's own environment + (their shell, VS Code); nothing here builds them from a change, a task + or an agent's output, which is what the advisories are about. Editors, + pagers, config paths, `GIT_DIR` and the like stay removed: nothing here + opens an editor or a pager, and a stray `GIT_DIR` in a parent's + environment is a way to act on the wrong repository. + - *Alternative: allow every `GIT_` variable.* Rejected: it gives back + what 4 took away for a reason, for no case this product has. + - *Alternative: allow none.* Rejected: measured in `git-client.test.ts`, + an SSH remote then waits on the real `ssh` until it is killed. +3. **Fixtures name what they set.** `gitIsolationOptions()` adds the six + author and committer variables fixture commits pass through `.env()`. + +## Risks / Trade-offs + +- **A setup that relied on another stripped variable** (a proxy command, + `GIT_CONFIG_GLOBAL`) no longer reaches git. Proxy settings in the + environment (`HTTPS_PROXY`) and in git's own configuration are not + guarded and still apply. diff --git a/openspec/changes/simple-git-4/proposal.md b/openspec/changes/simple-git-4/proposal.md new file mode 100644 index 00000000..3ae77d89 --- /dev/null +++ b/openspec/changes/simple-git-4/proposal.md @@ -0,0 +1,40 @@ +## Why + +On 2026-10-06 the dependency audit failed every pull request: two +critical advisories in `simple-git` <= 4.0.1 (GHSA-v5rq-49vh-5v5c, the +`VISUAL` editor variable missing from its unsafe-editor detection, and +GHSA-x6jw-m9v5-85vh, trailer command configuration not blocked). The fix +is only in 4.0.2; this repository was on 3.x. + +simple-git 4 also changes what the `git` it starts can see: every +`GIT_`-prefixed variable and a few others (`EDITOR`, `PAGER`, +`SSH_ASKPASS`, `VISUAL`, `PREFIX`) are removed from its environment. That +includes what a person set up to reach a remote - the askpass helper VS +Code provides, an SSH command - and without them a push either fails to +authenticate or, as measured here with an SSH remote, waits until it is +killed. + +## What Changes + +- `simple-git` ^4.0.2 in `core` and `cli`; the default import, which 4 + removed, is replaced. +- Every `git` this product starts through simple-git is opened by one + function, `openGit(cwd)`, which lets through only the variables that + carry how to connect and authenticate: `GIT_ASKPASS`, `SSH_ASKPASS`, + `GIT_SSH`, `GIT_SSH_COMMAND`, `GIT_SSH_VARIANT`, `GIT_TERMINAL_PROMPT`. + Every other guarded variable stays removed. +- Test fixtures that commit with a set author and date name those + variables, which simple-git 4 otherwise refuses. + +## Capabilities + +### Modified Capabilities + +- `execution-core`: the git this product starts keeps the person's + connection settings and nothing else of the guarded environment. + +## Impact + +- `packages/core`: `git-client.ts` (new), every module that opened + simple-git, `test-support/git-isolation.ts`. +- `packages/core`, `packages/cli`: `package.json`; `package-lock.json`. diff --git a/openspec/changes/simple-git-4/specs/execution-core/spec.md b/openspec/changes/simple-git-4/specs/execution-core/spec.md new file mode 100644 index 00000000..b5155cc2 --- /dev/null +++ b/openspec/changes/simple-git-4/specs/execution-core/spec.md @@ -0,0 +1,22 @@ +## ADDED Requirements + +### Requirement: The git this product starts keeps the person's connection settings, and nothing else guarded + +Every `git` the product starts through simple-git SHALL receive, from the +environment it runs in, `GIT_ASKPASS`, `SSH_ASKPASS`, `GIT_SSH`, +`GIT_SSH_COMMAND`, `GIT_SSH_VARIANT` and `GIT_TERMINAL_PROMPT` where they +are set, and SHALL NOT receive any other variable simple-git guards +(every other `GIT_`-prefixed key, `EDITOR`, `PAGER`, `PREFIX`, `VISUAL`). +The product SHALL NOT build any of the allowed variables from a change, a +task or an agent's output. + +#### Scenario: An SSH command set by the person + +- **WHEN** `GIT_SSH_COMMAND` is set in the environment and the product runs + git against an SSH remote +- **THEN** git runs that command + +#### Scenario: An editor in the environment + +- **WHEN** `GIT_EDITOR` is set in the environment and the product runs git +- **THEN** git does not see it diff --git a/openspec/changes/simple-git-4/tasks.md b/openspec/changes/simple-git-4/tasks.md new file mode 100644 index 00000000..e238cfdf --- /dev/null +++ b/openspec/changes/simple-git-4/tasks.md @@ -0,0 +1,44 @@ +The dependency audit failed every pull request on 2026-10-06: two +critical advisories in simple-git <= 4.0.1, fixed only in 4.0.2. + +## 1. Dependency + +- [x] 1.1 `simple-git` ^4.0.2 in `packages/core` and `packages/cli`; + `npm audit --omit=dev --audit-level=high` reports nothing. + simple-git 4.0.2 in both; `npm audit --omit=dev --audit-level=high`: + found 0 vulnerabilities. +- [x] 1.2 `git-client.ts`: `openGit(cwd)` (design.md decisions 1, 2); every + `simpleGit(...)` outside tests replaced; the default import, which 4 + removed, replaced in tests. Test in `git-client.test.ts`, in the + single-fork project: a `GIT_SSH_COMMAND` reaches git, a `GIT_EDITOR` does + not; checked to fail without the allowance. + `git-client.test.ts`: 2 passed; with `allowEnvironment: []` the SSH + case failed, git waiting on the real ssh until the 45 s budget. +- [x] 1.3 `test-support/git-isolation.ts`: fixture commits name the author + and committer variables (decision 3); `git.test.ts` asserts the options + `openGit` passes. + `git.test.ts` 5 passed; every fixture file passes in the full run. + +## 2. Documents + +- [x] 2.1 A changeset: core, cli, server, extension, patch. + `.changeset/simple-git-4.md`. + +## 3. Checks + +- [x] 3.1 `npm run typecheck && npm run lint`, and every test project, each + on its own where the root run would exceed a background limit. + Typecheck clean; lint 0 errors (3 warnings in lines this change did + not touch); seven script tests pass. Core 2096, core-git-subprocess 69, + cli 201, webui 712, server 122, extension 502 passed. +- [x] 3.2 `openspec validate simple-git-4 --strict`, and the merge gate + with the worktree's absolute path as `--cwd`. + Valid; the gate with the worktree's absolute path reports ok. +- [x] 3.3 A real push through the product's git wrapper to a remote that + needs the person's credentials, recorded. + Record, 2026-10-06: this branch pushed to GitHub over HTTPS by + `createGitWrapper(...).push("origin", "simple-git-4")` from a script, in + 4.1 s, authenticated by the Git Credential Manager named in git's + configuration; `ls-remote` then showed the branch at the local HEAD. The + askpass and SSH routes, which this machine does not use for this + remote, are the ones `git-client.test.ts` checks. diff --git a/package-lock.json b/package-lock.json index 6a731b62..61e63e19 100644 --- a/package-lock.json +++ b/package-lock.json @@ -2302,18 +2302,18 @@ } }, "node_modules/@simple-git/args-pathspec": { - "version": "1.0.3", - "resolved": "https://registry.npmjs.org/@simple-git/args-pathspec/-/args-pathspec-1.0.3.tgz", - "integrity": "sha512-ngJMaHlsWDTfjyq9F3VIQ8b7NXbBLq5j9i5bJ6XLYtD6qlDXT7fdKY2KscWWUF8t18xx052Y/PUO1K1TRc9yKA==", + "version": "1.0.4", + "resolved": "https://registry.npmjs.org/@simple-git/args-pathspec/-/args-pathspec-1.0.4.tgz", + "integrity": "sha512-EtMX6XjRWSastG2SdkmSQByPtJ9dx/NIjnHbpQPCYt62j4RmSx5rgLTGpw0YCjF5h191gZOmBfheOT23cRSFdw==", "license": "MIT" }, "node_modules/@simple-git/argv-parser": { - "version": "1.1.1", - "resolved": "https://registry.npmjs.org/@simple-git/argv-parser/-/argv-parser-1.1.1.tgz", - "integrity": "sha512-Q9lBcfQ+VQCpQqGJFHe5yooOS5hGdLFFbJ5R+R5aDsnkPCahtn1hSkMcORX65J2Z5lxSkD0lQorMsncuBQxYUw==", + "version": "2.0.1", + "resolved": "https://registry.npmjs.org/@simple-git/argv-parser/-/argv-parser-2.0.1.tgz", + "integrity": "sha512-M++IaVWrN+vYalilOHfwvT2IL3wYZtVEBQpmX6DLSOJLtxbZ+SUeIDE3IiPogzhPACLajwtxfa5rp+ZX1FczzQ==", "license": "MIT", "dependencies": { - "@simple-git/args-pathspec": "^1.0.3" + "@simple-git/args-pathspec": "^1.0.4" } }, "node_modules/@sindresorhus/merge-streams": { @@ -9213,15 +9213,15 @@ } }, "node_modules/simple-git": { - "version": "3.36.0", - "resolved": "https://registry.npmjs.org/simple-git/-/simple-git-3.36.0.tgz", - "integrity": "sha512-cGQjLjK8bxJw4QuYT7gxHw3/IouVESbhahSsHrX97MzCL1gu2u7oy38W6L2ZIGECEfIBG4BabsWDPjBxJENv9Q==", + "version": "4.0.2", + "resolved": "https://registry.npmjs.org/simple-git/-/simple-git-4.0.2.tgz", + "integrity": "sha512-l0sIsv9VrPwovKELFDBOj/lhmIcbMvxBk2B+hRhsMM4/VL3gKtuZvYnBNixVo+N/0HnqQx2cBdLvxnAbygHpqA==", "license": "MIT", "dependencies": { "@kwsites/file-exists": "^1.1.1", "@kwsites/promise-deferred": "^1.1.1", - "@simple-git/args-pathspec": "^1.0.3", - "@simple-git/argv-parser": "^1.1.0", + "@simple-git/args-pathspec": "1.0.4", + "@simple-git/argv-parser": "2.0.1", "debug": "^4.4.0" }, "funding": { @@ -11296,7 +11296,7 @@ "license": "MIT", "dependencies": { "cross-spawn": "^7.0.6", - "simple-git": "^3.27.0", + "simple-git": "^4.0.2", "yaml": "^2.9.1" }, "bin": { @@ -11798,7 +11798,7 @@ "dependencies": { "@agentclientprotocol/sdk": "^1.5.1", "cross-spawn": "^7.0.6", - "simple-git": "^3.27.0", + "simple-git": "^4.0.2", "undici": "^7.30.0", "yaml": "^2.9.1", "zod": "^4.6.5" diff --git a/packages/cli/package.json b/packages/cli/package.json index 9e4bc0a2..57a863ac 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -27,7 +27,7 @@ }, "dependencies": { "cross-spawn": "^7.0.6", - "simple-git": "^3.27.0", + "simple-git": "^4.0.2", "yaml": "^2.9.1" }, "devDependencies": { diff --git a/packages/core/package.json b/packages/core/package.json index 2ba6ae80..71fb49bd 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -17,7 +17,7 @@ "dependencies": { "@agentclientprotocol/sdk": "^1.5.1", "cross-spawn": "^7.0.6", - "simple-git": "^3.27.0", + "simple-git": "^4.0.2", "undici": "^7.30.0", "yaml": "^2.9.1", "zod": "^4.6.5" diff --git a/packages/core/src/change-diff.ts b/packages/core/src/change-diff.ts index 6d9c2995..97b5154d 100644 --- a/packages/core/src/change-diff.ts +++ b/packages/core/src/change-diff.ts @@ -10,7 +10,7 @@ import { readFile } from "node:fs/promises"; import path from "node:path"; -import simpleGit from "simple-git"; +import { openGit } from "./git-client.js"; import { discoverOpenSpecWorkspace } from "./workbench.js"; /** How much diff text one answer carries. A change whose `tasks.md` was @@ -48,7 +48,7 @@ export async function readChangeDiff( return { kind: "not-active", message: `"${changeName}" is not an active change of this workspace.` }; } - const git = simpleGit(workspaceRoot); + const git = openGit(workspaceRoot); const isRepository = await git.checkIsRepo().catch(() => false); if (!isRepository) { return { diff --git a/packages/core/src/change-spans.test.ts b/packages/core/src/change-spans.test.ts index 9da9678b..0c56c1d6 100644 --- a/packages/core/src/change-spans.test.ts +++ b/packages/core/src/change-spans.test.ts @@ -1,7 +1,7 @@ import { mkdir, mkdtemp, rename, rm, writeFile } from "node:fs/promises"; import os from "node:os"; import path from "node:path"; -import simpleGit from "simple-git"; +import { simpleGit } from "simple-git"; import { afterAll, describe, expect, it, vi } from "vitest"; import { readChangeSpans, type ChangeSpan } from "./change-spans.js"; import { getFileCreatedDate } from "./change-timeline.js"; diff --git a/packages/core/src/change-spans.ts b/packages/core/src/change-spans.ts index fdedb5e4..2b33109c 100644 --- a/packages/core/src/change-spans.ts +++ b/packages/core/src/change-spans.ts @@ -15,7 +15,8 @@ // `readProposalAddedDates` below. import { readFile } from "node:fs/promises"; -import simpleGit, { type SimpleGit } from "simple-git"; +import type { SimpleGit } from "simple-git"; +import { openGit } from "./git-client.js"; import { mapBounded } from "./bounded-map.js"; import { buildChangeDates, withoutArchivePrefix, type DatedFact } from "./change-dates.js"; import { countTaskCheckboxes, type TaskCounts } from "./change-standing-facts.js"; @@ -126,7 +127,7 @@ async function readTaskCounts(change: WorkbenchChange): Promise> { const dates = new Map(); try { - const git = simpleGit(root); + const git = openGit(root); const prefix = await pathPrefixInRepository(git); const output = await git.raw([ // A path with anything unusual in it is otherwise rendered as a diff --git a/packages/core/src/change-timeline.test.ts b/packages/core/src/change-timeline.test.ts index 8eec18c6..2e807e0f 100644 --- a/packages/core/src/change-timeline.test.ts +++ b/packages/core/src/change-timeline.test.ts @@ -1,7 +1,7 @@ import { mkdir, mkdtemp, rename, rm, writeFile } from "node:fs/promises"; import os from "node:os"; import path from "node:path"; -import simpleGit, { type SimpleGit } from "simple-git"; +import { simpleGit, type SimpleGit } from "simple-git"; import { afterAll, describe, expect, it, vi } from "vitest"; import { blameLineDates, diff --git a/packages/core/src/change-timeline.ts b/packages/core/src/change-timeline.ts index cb2a1754..442942b4 100644 --- a/packages/core/src/change-timeline.ts +++ b/packages/core/src/change-timeline.ts @@ -5,7 +5,8 @@ // a change read should ever depend on succeeding. import { readFile } from "node:fs/promises"; -import simpleGit, { type SimpleGit } from "simple-git"; +import type { SimpleGit } from "simple-git"; +import { openGit } from "./git-client.js"; import { buildChangeDates, normalizedInstant, @@ -105,7 +106,7 @@ export async function blameLineDates( ): Promise | undefined> { let output: string; try { - output = await simpleGit(cwd).raw(["blame", "--line-porcelain", "--", filePath]); + output = await openGit(cwd).raw(["blame", "--line-porcelain", "--", filePath]); } catch { return undefined; } @@ -164,7 +165,7 @@ export async function getFileCreatedDate(cwd: string, filePath: string): Promise // as "undeterminable". Verified against git 2.54.0 on 2026-09-09. // The oldest commit is the last line instead. See // change-dates-from-evidence. - const output = await simpleGit(cwd).raw([ + const output = await openGit(cwd).raw([ "log", "--follow", "--diff-filter=A", @@ -191,7 +192,7 @@ export async function getAddedFileDates(cwd: string, directoryPath: string): Pro const added = new Map(); let output: string; try { - output = await simpleGit(cwd).raw([ + output = await openGit(cwd).raw([ "log", "--diff-filter=A", "--name-only", "--format=@%aI", "--", directoryPath, `:(exclude)${directoryPath}/archive`, ]); } catch { @@ -216,7 +217,7 @@ export async function getAddedFileDates(cwd: string, directoryPath: string): Pro * change made before its proposal (ADR 0037, amended 2026-09-25). */ export async function getDirectoryCreatedDate(cwd: string, directoryPath: string): Promise { try { - const output = await simpleGit(cwd).raw(["log", "--diff-filter=A", "--format=%aI", "--", directoryPath]); + const output = await openGit(cwd).raw(["log", "--diff-filter=A", "--format=%aI", "--", directoryPath]); return oldestDateIn(output); } catch { return null; @@ -241,7 +242,7 @@ function oldestDateIn(output: string): string | null { * change-dates-from-evidence. */ export async function getPathAddedDate(cwd: string, filePath: string): Promise { try { - const output = await simpleGit(cwd).raw([ + const output = await openGit(cwd).raw([ "log", "--diff-filter=A", "--format=%aI", @@ -284,7 +285,7 @@ export async function readArchiveCommitDates(cwd: string): Promise(); let unreadableLines = 0; try { - const git = simpleGit(cwd); + const git = openGit(cwd); const archivePrefix = `${await pathPrefixInRepository(git)}${ARCHIVE_LOG_PATH}/`; const output = await git.raw([ // git renders a path with anything unusual in it as a C-style @@ -402,7 +403,7 @@ const AUTHOR_LOG_FIELD_SEP = "\x1f"; export async function getChangeAuthorship(cwd: string, changeDirPath: string): Promise { const empty: ChangeAuthorship = { primaryAuthor: null, contributors: [] }; try { - const output = await simpleGit(cwd).raw([ + const output = await openGit(cwd).raw([ "log", `--format=%an${AUTHOR_LOG_FIELD_SEP}%ae${AUTHOR_LOG_FIELD_SEP}%aI`, "--", @@ -449,7 +450,7 @@ export async function readProposalCreatedDates(cwd: string): Promise { try { - const output = await simpleGit(root).raw([ + const output = await openGit(root).raw([ "ls-files", "--cached", "--others", diff --git a/packages/core/src/git-client.test.ts b/packages/core/src/git-client.test.ts new file mode 100644 index 00000000..e9e1c393 --- /dev/null +++ b/packages/core/src/git-client.test.ts @@ -0,0 +1,59 @@ +import { existsSync } from "node:fs"; +import { mkdtemp, rm, writeFile } from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { describe, expect, it, vi } from "vitest"; +import { openGit } from "./git-client.js"; + +// simple-git-4: simple-git 4 strips guarded variables from the git it runs. +// What a person set up to reach a remote must still reach git; what could +// make git run something nobody asked for must not. Real git, so in the +// single-fork project (vitest.workspace.ts). Sized like git.push.test.ts: +// a real git start costs seconds under a loaded Windows machine. +vi.setConfig({ testTimeout: 45_000 }); + +/** Runs `body` with `name` set in this process's environment, as a person's + * shell or VS Code would have set it, and puts it back after. */ +async function withAmbient(name: string, value: string, body: () => Promise): Promise { + const before = process.env[name]; + process.env[name] = value; + try { + return await body(); + } finally { + if (before === undefined) delete process.env[name]; + else process.env[name] = before; + } +} + +describe("openGit", () => { + it("lets the person's SSH command reach git", async () => { + const root = await mkdtemp(path.join(os.tmpdir(), "openspec-git-client-")); + try { + const marker = path.join(root, "ssh-ran"); + const script = path.join(root, "fake-ssh.mjs"); + // Stands in for ssh: leaves a mark and fails, so nothing is reached. + await writeFile(script, `import { writeFileSync } from "node:fs";\nwriteFileSync(${JSON.stringify(marker)}, "ran");\nprocess.exit(1);\n`, "utf8"); + const command = `"${process.execPath.replaceAll("\\", "/")}" "${script.replaceAll("\\", "/")}"`; + + await withAmbient("GIT_SSH_COMMAND", command, async () => { + await expect(openGit(root).raw(["ls-remote", "ssh://example.invalid/repo.git"])).rejects.toThrow(); + }); + + expect(existsSync(marker)).toBe(true); + } finally { + await rm(root, { recursive: true, force: true }); + } + }); + + it("keeps an ambient editor from git", async () => { + const root = await mkdtemp(path.join(os.tmpdir(), "openspec-git-client-")); + try { + await openGit(root).init(); + const editor = await withAmbient("GIT_EDITOR", "an-editor-nobody-asked-for", () => openGit(root).raw(["var", "GIT_EDITOR"])); + + expect(editor.trim()).not.toBe("an-editor-nobody-asked-for"); + } finally { + await rm(root, { recursive: true, force: true }); + } + }); +}); diff --git a/packages/core/src/git-client.ts b/packages/core/src/git-client.ts new file mode 100644 index 00000000..ce4fd336 --- /dev/null +++ b/packages/core/src/git-client.ts @@ -0,0 +1,39 @@ +// The one way this product starts `git` through simple-git (simple-git-4). +// +// simple-git 4 removes every `GIT_`-prefixed variable, and a few others +// (`EDITOR`, `PAGER`, `SSH_ASKPASS`, ...), from the environment of the git it +// runs, so that a value nobody meant for it cannot make it run a program. +// That is right for most of them: nothing here opens an editor, a pager or a +// template. It would also take away what a person set up to reach their +// remote - the askpass helper VS Code provides, an SSH command, the switch +// that keeps git from prompting in a terminal nobody watches - and a push +// that worked would then fail to authenticate, or wait on a prompt. +// +// So the variables that carry how to connect and authenticate are let +// through, and only those. They come from the person's own environment; +// none of them is ever built from a change, a task or an agent's output. + +import { simpleGit, type SimpleGit } from "simple-git"; + +/** The variables a push, fetch or pull needs from the person's environment + * to reach and authenticate with a remote. */ +export const GIT_CONNECTION_ENVIRONMENT: readonly string[] = [ + "GIT_ASKPASS", + "SSH_ASKPASS", + "GIT_SSH", + "GIT_SSH_COMMAND", + "GIT_SSH_VARIANT", + "GIT_TERMINAL_PROMPT", +]; + +/** A simple-git instance for `cwd`, with the connection variables above let + * through. Throws, as `simpleGit` does, when `cwd` is not a directory. */ +export function openGit(cwd: string): SimpleGit { + return simpleGit({ + baseDir: cwd, + allowEnvironment: GIT_CONNECTION_ENVIRONMENT, + // An askpass helper and an SSH command run a program by design; letting + // the person's own through is what keeps their remote reachable. + unsafe: { allowUnsafeAskPass: true, allowUnsafeSshCommand: true }, + }); +} diff --git a/packages/core/src/git.push.test.ts b/packages/core/src/git.push.test.ts index 0e8467e1..0d7305ed 100644 --- a/packages/core/src/git.push.test.ts +++ b/packages/core/src/git.push.test.ts @@ -1,7 +1,7 @@ import { mkdtemp, rm, writeFile } from "node:fs/promises"; import os from "node:os"; import path from "node:path"; -import simpleGit from "simple-git"; +import { simpleGit } from "simple-git"; import { afterAll, describe, expect, it, vi } from "vitest"; import { createGitWrapper } from "./git.js"; import { gitIsolationOptions } from "./test-support/git-isolation.js"; diff --git a/packages/core/src/git.test.ts b/packages/core/src/git.test.ts index 156dd30c..956b4b85 100644 --- a/packages/core/src/git.test.ts +++ b/packages/core/src/git.test.ts @@ -11,7 +11,7 @@ const simpleGitFactory = vi.fn(() => ({ push: pushMock, })); -vi.mock("simple-git", () => ({ default: simpleGitFactory })); +vi.mock("simple-git", () => ({ simpleGit: simpleGitFactory })); const { createGitWrapper } = await import("./git.js"); @@ -31,7 +31,12 @@ describe("createGitWrapper", () => { const wrapper = createGitWrapper({ cwd: "/workspace/repo" }); const status = await wrapper.status(); - expect(simpleGitFactory).toHaveBeenCalledWith("/workspace/repo"); + // Opened through openGit, which lets the connection variables through + // (simple-git-4). + expect(simpleGitFactory).toHaveBeenCalledWith(expect.objectContaining({ + baseDir: "/workspace/repo", + allowEnvironment: expect.arrayContaining(["GIT_ASKPASS", "GIT_SSH_COMMAND"]), + })); expect(status).toEqual({ current: "feat-x", ahead: 1, diff --git a/packages/core/src/git.ts b/packages/core/src/git.ts index 728b08bb..63062fc0 100644 --- a/packages/core/src/git.ts +++ b/packages/core/src/git.ts @@ -3,7 +3,8 @@ import { stat } from "node:fs/promises"; import path from "node:path"; -import simpleGit, { type SimpleGit } from "simple-git"; +import type { SimpleGit } from "simple-git"; +import { openGit } from "./git-client.js"; export interface GitWrapperOptions { cwd: string; @@ -221,7 +222,7 @@ export interface GitWrapper { } export function createGitWrapper(options: GitWrapperOptions): GitWrapper { - const git: SimpleGit = simpleGit(options.cwd); + const git: SimpleGit = openGit(options.cwd); return { async status(): Promise { diff --git a/packages/core/src/own-worktree-tasks.ts b/packages/core/src/own-worktree-tasks.ts index b779b094..b7f86060 100644 --- a/packages/core/src/own-worktree-tasks.ts +++ b/packages/core/src/own-worktree-tasks.ts @@ -16,7 +16,7 @@ import { randomUUID } from "node:crypto"; import { access, readFile, rename, rm, writeFile } from "node:fs/promises"; import path from "node:path"; -import simpleGit from "simple-git"; +import { openGit } from "./git-client.js"; import type { AgentRunner } from "./agent-runner.js"; import { readAgentStatuses, resolveAgentStatusDirectory, type AgentStatusReport } from "./agent-status.js"; @@ -287,7 +287,7 @@ export async function commitTaskList( return refuse("default-branch", `${options.changeName}'s worktree is on ${branch}, the default branch; a task list is never committed there.`); } - const git = simpleGit(worktree.path); + const git = openGit(worktree.path); const relative = path.relative(worktree.path, worktree.tasksPath).split(path.sep).join("/"); const head = (await git.raw(["rev-parse", "--abbrev-ref", "HEAD"])).trim(); if (head === "HEAD") return refuse("detached-head", `${options.changeName}'s worktree has no branch checked out.`); diff --git a/packages/core/src/sprint-report.test.ts b/packages/core/src/sprint-report.test.ts index 80892306..3ef32c8d 100644 --- a/packages/core/src/sprint-report.test.ts +++ b/packages/core/src/sprint-report.test.ts @@ -1,7 +1,7 @@ import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; import os from "node:os"; import path from "node:path"; -import simpleGit, { type SimpleGit } from "simple-git"; +import { simpleGit, type SimpleGit } from "simple-git"; import { afterAll, describe, expect, it, vi } from "vitest"; import { buildSprintReport } from "./sprint-report.js"; import { gitIsolationOptions } from "./test-support/git-isolation.js"; diff --git a/packages/core/src/test-support/git-isolation.ts b/packages/core/src/test-support/git-isolation.ts index b1ac8df6..78c03882 100644 --- a/packages/core/src/test-support/git-isolation.ts +++ b/packages/core/src/test-support/git-isolation.ts @@ -49,10 +49,24 @@ async function isolationSettings(): Promise { export async function gitIsolationOptions(): Promise<{ config: string[]; unsafe: { allowUnsafeHooksPath: true }; + allowEnvironment: readonly string[]; }> { - return { config: await isolationSettings(), unsafe: { allowUnsafeHooksPath: true } }; + return { config: await isolationSettings(), unsafe: { allowUnsafeHooksPath: true }, allowEnvironment: FIXTURE_COMMIT_ENVIRONMENT }; } +/** The variables a fixture commit sets to say who made it and when. + * simple-git 4 refuses a `GIT_` variable passed through `.env()` unless it + * is named (simple-git-4); these name a commit's author and date, nothing + * that runs a program. */ +const FIXTURE_COMMIT_ENVIRONMENT: readonly string[] = [ + "GIT_AUTHOR_NAME", + "GIT_AUTHOR_EMAIL", + "GIT_AUTHOR_DATE", + "GIT_COMMITTER_NAME", + "GIT_COMMITTER_EMAIL", + "GIT_COMMITTER_DATE", +]; + /** The same settings as arguments, for a raw `git` call: they go before * the subcommand, so spread this ahead of it. */ export async function gitIsolationArgs(): Promise { diff --git a/packages/core/vitest.workspace.ts b/packages/core/vitest.workspace.ts index 52271f66..815980c5 100644 --- a/packages/core/vitest.workspace.ts +++ b/packages/core/vitest.workspace.ts @@ -50,6 +50,8 @@ import { defineWorkspace } from "vitest/config"; // (git-says-a-working-directory-is-done). const SHELL_STARTING_TESTS = [ "src/git.push.test.ts", + // Real git, with an SSH command that starts node (simple-git-4). + "src/git-client.test.ts", "src/git-refs.test.ts", "src/finished-directories.removal.test.ts", // Real git for init, commit and ls-tree against a temporary repository