From 5442f2d8694a8cb28cdc149175dceeead11aaaeb Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Thu, 17 Sep 2026 18:40:22 -0700 Subject: [PATCH] fix: repair managed Git launchers in sandbox projects (#13588) ## Thinking Path > - Paperclip runs agents in local and remote execution environments. > - Managed GitHub launchers select credentials for each Git operation. > - Remote launchers are written inside the project checkout as extensionless CommonJS scripts. > - An ES module project makes Node interpret those launchers as ESM, so they crash before credential resolution. > - When the launcher can start, empty identity variables also override valid repository and command-line Git configuration. > - This change gives the launchers their own CommonJS scope and clears empty identity overrides while preserving managed credential isolation. ## Linked Issues or Issue Description **What happened?** In a repository with `"type": "module"`, the managed `git` and `gh` launchers fail immediately with `ReferenceError: require is not defined in ES module scope`. The launchers use CommonJS but inherited the enclosing project's module type. Sandbox agents also report empty `GIT_AUTHOR_NAME` and `GIT_COMMITTER_NAME` variables and try to unset them for each command. With no managed identity available, the Git launcher recreated those empty values. `git commit` failed with `fatal: empty ident name`, even with explicit `user.name` and `user.email` configuration. **Expected behavior** Managed `git` and `gh` start in both ES module and CommonJS projects. Local commits with an explicitly configured identity work without manual environment cleanup. Managed credentials and captured identity continue to take precedence. Missing identity does not silently select the host user's details. **Steps to reproduce** 1. Create a sandbox project whose `package.json` contains `"type": "module"`. 2. Stage the managed GitHub launchers and run `git --version` or `gh --version`. Before this fix, the launcher fails at its first `require()`. 3. In a CommonJS project with no available managed identity, configure repository `user.name` and `user.email`, or supply them with `git -c`. 4. Run `git commit --allow-empty -m test`. Before this fix, both identity configuration forms fail with empty identity. **Paperclip version or commit** Reproduced from master commit `165b10bd9`. **Deployment mode** Sandbox execution. The shared launcher is also used for managed local and SSH execution. Related work: #13094 introduced the local-operation fallback; #13053 changes launcher discovery on Windows. Neither fixes empty identity overrides. Related identity work in #8945 and #8946 configures worktree authorship and does not remove these environment overrides. ## What Changed - Stage `package.json` with `"type": "commonjs"` in the launcher directory before the Node scripts. Keep the project's package configuration unchanged. - Leave inherited author and committer variables unset in the real Git process. When credentials are absent, require explicit Git identity configuration with `user.useConfigOnly`. - Clear empty identity merge overrides in staged shell profiles after environment merging. Preserve nonempty captured identity values. - Exercise real Git commits with repository and command-line identity, broker failures, and managed-user switching. Verify startup in ES module and CommonJS projects, shell cleanup, and captured identity preservation. - Document launcher module scope and local identity behavior in the execution GitHub identity contract. ## Verification - Confirmed both new local-commit regression cases fail before the fix with `fatal: empty ident name`. - Confirmed the new ES module project regression fails before the fix with `require is not defined in ES module scope`. - Focused launcher and shell tests: 28 passed. - `pnpm exec vitest run --project @paperclipai/adapter-utils --exclude '**/dist/**'`: 1,216 passed, 11 skipped across 58 files. - `pnpm --filter @paperclipai/adapter-utils typecheck` and `pnpm --filter @paperclipai/adapter-utils build`: passed. - `pnpm -r typecheck` and `pnpm build`: attempted; both stop in the unchanged native runner because Cargo is not installed on this machine. - Full `pnpm test:run`: started locally; stopped the duplicate run after the complete CI suite passed. No local full-suite success is claimed. - CI on `99ea8050e`: all 53 checks passed (2 skipped), including full tests, typecheck, build, native runner checks, and browser checks. - Greptile reviewed `99ea8050e`: 5/5 with no findings or unresolved comments. GitHub reports no merge conflicts with master. - No live sandbox or GitHub push probe performed. ## Risks - The new package scope is confined to the run-specific launcher directory. It does not change the project's module type, launcher names, or credential selection. - Without a managed identity, an explicitly configured repository author can now create local commits. GitHub access remains subject to the existing credential broker. Global/system Git configuration, ambient credentials, and SSH identity remain isolated. - Managed identity still wins over repository settings. Missing local identity still fails instead of guessing host details. - New or resumed executions must stage the updated launcher and shell profiles. Existing processes retain their prior files and environment until refreshed. No database migration or sandbox image rebuild is required. - Revert this change to restore the prior behavior. ## Model Used - OpenAI GPT-6 via Codex, with code inspection, implementation, and local test execution. The hosted model variant and context window were not exposed. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub references) - [x] My branch name describes the change and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass (the affected adapter-utils package) - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip --- doc/execution-github-identity.md | 13 +++++- .../adapter-utils/src/execution-target.ts | 11 ++++- .../src/github-launcher-environment.test.ts | 45 +++++++++++++++++++ .../adapter-utils/src/github-launcher.test.ts | 39 ++++++++++++++++ packages/adapter-utils/src/github-launcher.ts | 7 ++- 5 files changed, 111 insertions(+), 4 deletions(-) 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 {