diff --git a/doc/execution-github-identity.md b/doc/execution-github-identity.md index f50a8df001d..d86c82d95b7 100644 --- a/doc/execution-github-identity.md +++ b/doc/execution-github-identity.md @@ -26,12 +26,23 @@ Server-side Git operations and GitHub gateway calls follow the same selection ru Managed commands disable ambient Git credential helpers, Git global/system configuration, host GitHub CLI configuration, and host SSH identity access. Per-operation GitHub CLI configuration is isolated in a writable configuration directory beneath the managed launcher directory. Missing credentials clear previous author and token values; no teammate, standing delegation, host token, or company-default user's account is substituted. Anonymous/local operations remain available where supported. +Without a managed identity, local commits can use an explicitly configured +repository identity or `git -c user.name=... -c user.email=...`. The launcher +leaves author/committer environment variables unset and requires configured +identity instead of guessing the host user's details. Managed shell profiles +remove empty identity overrides after environment merging, so agents do not +need to unset them per command. A captured managed identity still takes +precedence over repository configuration. + Remote launchers prepend their directory to the execution target's effective `PATH`. An explicit remote `PATH` override is preserved; otherwise Paperclip reads the provider's environment before staging the launcher shell files. This keeps legacy NVM and user-local agent installations available alongside newer images with system-wide CLIs. The generated shell files retain that -combined path with managed `git` and `gh` first. Sandbox command checks use +combined path with managed `git` and `gh` first. The launcher directory has +its own CommonJS package scope, so the extensionless Node launchers work +inside repositories that declare `"type": "module"` without changing the +project's package configuration. Sandbox command checks use the same sanitized environment as execution, so a CLI visible only in the provider's default environment cannot pass the launch check. Failed path discovery stops startup instead of silently falling back to a minimal path. diff --git a/packages/adapter-utils/src/execution-target.ts b/packages/adapter-utils/src/execution-target.ts index eaae713b283..592de096379 100644 --- a/packages/adapter-utils/src/execution-target.ts +++ b/packages/adapter-utils/src/execution-target.ts @@ -1710,8 +1710,17 @@ export async function prepareGitHubOperationLaunchers(input: { const managedPath = basePath ? `${directory}:${basePath}` : directory; // Login shells may reorder PATH through /etc/profile or path_helper. Restore // the managed launchers after startup without loading a host user's profile. - const profile = `export PATH=${shellQuote(managedPath)}\n`; + // Empty merge overrides clear host identity before launch, but Git treats + // them as an explicit empty author. Remove them once the shell has inherited + // its final environment; preserve nonempty per-operation identity values. + const clearEmptyGitIdentity = ["GIT_AUTHOR_NAME", "GIT_AUTHOR_EMAIL", "GIT_COMMITTER_NAME", "GIT_COMMITTER_EMAIL"] + .map((key) => `if [ -z "\${${key}-}" ]; then unset ${key}; fi\n`) + .join(""); + const profile = `export PATH=${shellQuote(managedPath)}\n${clearEmptyGitIdentity}`; const files: Record = Object.fromEntries([ + // Remote launchers live beneath the checkout. Pin their own package scope + // so an enclosing project's "type": "module" cannot reinterpret require(). + ["package.json", '{"type":"commonjs"}\n'], ...["git", "gh"].map((name) => [name, githubLauncherSource()] as const), ...[".zshenv", ".zprofile", ".zshrc", ".bash_profile", ".bashrc", ".profile"].map((name) => [name, profile] as const), ]); diff --git a/packages/adapter-utils/src/github-launcher-environment.test.ts b/packages/adapter-utils/src/github-launcher-environment.test.ts index 8d217e602ce..2dab7d4af0f 100644 --- a/packages/adapter-utils/src/github-launcher-environment.test.ts +++ b/packages/adapter-utils/src/github-launcher-environment.test.ts @@ -6,6 +6,7 @@ import { promisify } from "node:util"; import { afterEach, describe, expect, it, vi } from "vitest"; import * as ssh from "./ssh.js"; import type { CommandManagedRuntimeRunner } from "./command-managed-runtime.js"; +import { githubBrokerEnvironment } from "./github-launcher.js"; import { ensureAdapterExecutionTargetCommandResolvable, prepareGitHubOperationLaunchers, @@ -66,6 +67,50 @@ async function sandbox(layout: string) { } describe("managed GitHub launcher environment", () => { + it.each(["module", "commonjs"])("runs managed GitHub launchers inside a %s project", async (type) => { + const fixture = await sandbox("usr/bin"); + const packageJson = JSON.stringify({ type }); + await writeFile(path.join(fixture.root, "package.json"), packageJson); + // Exercise real Git; gh uses the fixture CLI because it need not be installed. + await rm(path.join(fixture.bin, "git")); + const env = await prepareGitHubOperationLaunchers({ + runId: "run-package-type", target: fixture.target, cwd: fixture.root, + env: githubBrokerEnvironment({}, { url: "", token: "" }), + }); + for (const cli of ["git", "gh"]) { + const result = await fixture.runner.execute({ + command: path.join(env.PAPERCLIP_GITHUB_LAUNCHER_DIR, cli), args: ["--version"], env, + }); + expect(result.exitCode, result.stderr).toBe(0); + expect(result.stdout).toMatch(cli === "git" ? /^git version / : /^gh started\n$/); + } + expect(await readFile(path.join(fixture.root, "package.json"), "utf8")).toBe(packageJson); + }); + + it("clears empty identity overrides in sandbox shells and preserves a captured identity", async () => { + const fixture = await sandbox("usr/bin"); + const env = await prepareGitHubOperationLaunchers({ + runId: "run-git-identity", target: fixture.target, cwd: fixture.root, + env: githubBrokerEnvironment({ GIT_AUTHOR_NAME: "Host Author" }, { url: "", token: "" }), + }); + const readIdentity = `node -e 'process.stdout.write(JSON.stringify(Object.fromEntries(Object.entries(process.env).filter(([key]) => /^GIT_(AUTHOR|COMMITTER)_(NAME|EMAIL)$/.test(key)))))'`; + const shell = await fixture.runner.execute({ command: "bash", args: ["--noprofile", "--norc", "-c", readIdentity], env }); + expect(shell.exitCode, shell.stderr).toBe(0); + expect(JSON.parse(shell.stdout)).toEqual({}); + + const identity = { GIT_AUTHOR_NAME: "Captured Author", GIT_AUTHOR_EMAIL: "author@example.test", + GIT_COMMITTER_NAME: "Captured Committer", GIT_COMMITTER_EMAIL: "committer@example.test" }; + for (const profile of [".profile", ".bash_profile", ".bashrc", ".zshenv", ".zprofile", ".zshrc"]) { + const script = await readFile(path.join(env.PAPERCLIP_GITHUB_LAUNCHER_DIR, profile), "utf8"); + for (const captured of [false, true]) { + const result = await fixture.runner.execute({ command: "sh", args: ["-c", `${script}\n${readIdentity}`], + env: { ...env, ...(captured ? identity : {}) } }); + expect(result.exitCode, result.stderr).toBe(0); + expect(JSON.parse(result.stdout)).toEqual(captured ? identity : {}); + } + } + }); + it.each([false, true])("probes the remote workspace when the controller cwd is absent (host credentials: %s)", async (hostCredentials) => { const fixture = await sandbox("usr/bin"); const env = await prepareGitHubExecutionEnvironment({ diff --git a/packages/adapter-utils/src/github-launcher.test.ts b/packages/adapter-utils/src/github-launcher.test.ts index 4e15066c5ac..8ff19e54974 100644 --- a/packages/adapter-utils/src/github-launcher.test.ts +++ b/packages/adapter-utils/src/github-launcher.test.ts @@ -11,12 +11,38 @@ const cleanups: Array<() => Promise> = []; afterEach(async () => { for (const cleanup of cleanups.splice(0).reverse()) await cleanup(); }); describe("managed GitHub launchers", () => { + it.each(["repository", "command"])("uses explicit %s identity for local commits without managed credentials", async (identitySource) => { + const root = await mkdtemp(path.join(os.tmpdir(), "paperclip-github-local-identity-")); + cleanups.push(() => rm(root, { recursive: true, force: true })); + const bin = path.join(root, "managed"); + await mkdir(bin); + await exec("git", ["init", root]); + await writeFile(path.join(bin, "git"), githubLauncherSource(), { mode: 0o700 }); + const env = { ...process.env, ...githubBrokerEnvironment({ + GH_TOKEN: "host-token", GIT_AUTHOR_NAME: "Host", GIT_COMMITTER_NAME: "Host", + }, { url: "", token: "" }), PATH: `${bin}:${process.env.PATH}` }; + const git = async (...args: string[]) => (await exec(path.join(bin, "git"), args, { cwd: root, env })).stdout.trim(); + // No configured identity must fail, rather than guessing the host user's. + await expect(git("var", "GIT_AUTHOR_IDENT")).rejects.toThrow(); + await expect(git("var", "GIT_COMMITTER_IDENT")).rejects.toThrow(); + if (identitySource === "repository") { + await git("config", "user.name", "Local Author"); + await git("config", "user.email", "local@example.test"); + } + await git(...(identitySource === "command" ? ["-c", "user.name=Local Author", "-c", "user.email=local@example.test"] : []), + "commit", "--allow-empty", "-m", "Local work"); + expect(await git("log", "-1", "--format=%an <%ae>|%cn <%ce>")) + .toBe("Local Author |Local Author "); + }); + it.each(["broker-offline", "config-unwritable", "capability-rejected"])("keeps real local Git usable when %s", async (failure) => { const root = await mkdtemp(path.join(os.tmpdir(), "paperclip-github-failure-")); cleanups.push(() => rm(root, { recursive: true, force: true })); const bin = path.join(root, "managed"); await mkdir(bin); await exec("git", ["init", root]); + await exec("git", ["-C", root, "config", "user.name", "Local Author"]); + await exec("git", ["-C", root, "config", "user.email", "local@example.test"]); await writeFile(path.join(bin, "git"), githubLauncherSource(), { mode: 0o700 }); const server = createServer((_req, res) => { res.writeHead(403); res.end(); }); await new Promise(resolve => server.listen(0, "127.0.0.1", resolve)); @@ -31,6 +57,10 @@ describe("managed GitHub launchers", () => { } }); expect(result.stderr).toContain(failure === "broker-offline" ? "broker_transport_unavailable" : failure === "config-unwritable" ? "configuration_directory_unavailable" : "capability_rejected"); expect(result.stderr).not.toMatch(/host-must-not-leak|private-capability/); + await exec(path.join(bin, "git"), ["commit", "--allow-empty", "-m", "Offline work"], { cwd: root, env: { + ...process.env, ...githubBrokerEnvironment({}, { url: `http://127.0.0.1:${port}`, token: "private-capability" }), + GH_CONFIG_DIR: configRoot, PATH: `${bin}:${process.env.PATH}`, + } }); }); it("explains unavailable access while allowing local work without credentials", async () => { @@ -87,6 +117,8 @@ process.stdout.write(JSON.stringify({identity, token:process.env.GH_TOKEN ?? nul }, { url: `http://127.0.0.1:${address.port}`, token: "run-capability" }), PATH: `${bin}:${realBin}:${process.env.PATH}` }; const git = async (...args: string[]) => (await exec(path.join(bin, "git"), args, { cwd: repo, env })).stdout.trim(); await git("init"); + await git("config", "user.name", "Repository Author"); + await git("config", "user.email", "repository@example.test"); await git("commit", "--allow-empty", "-m", "A"); user = "B"; await git("commit", "--allow-empty", "-m", "B"); @@ -109,6 +141,13 @@ process.stdout.write(JSON.stringify({identity, token:process.env.GH_TOKEN ?? nul expect(operationB.token).toBe("credential-B"); expect(completedA.config).not.toBe(operationB.config); user = null; + await git("commit", "--allow-empty", "-m", "Local identity"); + expect(await git("log", "-1", "--format=%an <%ae>|%cn <%ce>")) + .toBe("Repository Author |Repository Author "); + const anonymous = JSON.parse((await exec(path.join(bin, "gh"), [], { cwd: repo, env })).stdout); + expect(anonymous.token).toBeNull(); + await git("config", "--unset", "user.name"); + await git("config", "--unset", "user.email"); await expect(git("var", "GIT_AUTHOR_IDENT")).rejects.toThrow(); expect(await git("status", "--porcelain")).toBe(""); // unrelated public/local Git still works expect(env.GH_TOKEN).toBe(""); diff --git a/packages/adapter-utils/src/github-launcher.ts b/packages/adapter-utils/src/github-launcher.ts index 2c5579ed40d..1de7a291018 100644 --- a/packages/adapter-utils/src/github-launcher.ts +++ b/packages/adapter-utils/src/github-launcher.ts @@ -40,11 +40,14 @@ async function main() { GH_CONFIG_DIR: configDirectory, SSH_AUTH_SOCK: '', GIT_CONFIG_GLOBAL: '/dev/null', GIT_CONFIG_SYSTEM: '/dev/null', GIT_TERMINAL_PROMPT: '0', - GIT_AUTHOR_NAME: '', GIT_AUTHOR_EMAIL: '', GIT_COMMITTER_NAME: '', GIT_COMMITTER_EMAIL: '', - GIT_CONFIG_COUNT: '4', GIT_CONFIG_KEY_0: 'credential.helper', GIT_CONFIG_VALUE_0: '', + // The inherited identity was deleted above. Empty identity env values + // override even explicit repository/command config and break local commits. + // Require configured identity instead of guessing the OS user's details. + GIT_CONFIG_COUNT: '5', GIT_CONFIG_KEY_0: 'credential.helper', GIT_CONFIG_VALUE_0: '', GIT_CONFIG_KEY_1: 'url.https://github.com/.insteadOf', GIT_CONFIG_VALUE_1: 'git@github.com:', GIT_CONFIG_KEY_2: 'url.https://github.com/.insteadOf', GIT_CONFIG_VALUE_2: 'ssh://git@github.com/', GIT_CONFIG_KEY_3: 'core.askPass', GIT_CONFIG_VALUE_3: '', + GIT_CONFIG_KEY_4: 'user.useConfigOnly', GIT_CONFIG_VALUE_4: 'true', }); const base = env.PAPERCLIP_GITHUB_BROKER_URL || env.PAPERCLIP_API_URL; try {