mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-30 21:49:18 +01:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Agent runs use `git` and `gh` through the managed GitHub launcher. > - The launcher asks the broker for credentials before every `git` and `gh` call. It also asks for local commands that do not use the network, such as `git status`. > - Each request runs a transaction and an UPDATE in the database. On a busy install this gives hundreds of thousands of writes each day and long COMMIT waits. > - This pull request stops the request for local `git` commands and writes the identity summary only when it changes. > - The benefit is fewer broker requests and fewer database writes. The credential rules do not change. ## Linked Issues or Issue Description - Fixes #17879 ## What Changed - The managed `git` launcher does not ask the broker for local subcommands (`status`, `diff`, `log`, `rev-parse`, `add`, `config`, `branch`, `checkout` and others). It reads past global options such as `-C` and `-c`. An unknown subcommand or option still asks. `commit` and `var` still ask, because the managed author comes from the broker answer. - The launcher keeps no broker answer between invocations, so no GitHub credential is written to a file. The suggestion in the issue to reuse one answer per run is not part of this pull request, because the reused answer would have to rest on disk. - `resolveGitHubOperationCredentials` writes `run_identity_contexts.github` only when the summary changes. - `doc/execution-github-identity.md` describes the behavior. ## Verification - New launcher tests run real `git` against a fake broker that counts POSTs. A 12-command sequence (init, status, config, rev-parse, add, diff, commit, log, branch, show, status, diff) made 12 POSTs before and 1 POST after. The one POST is `commit`. - The same tests check that `var`, `remote`, `fetch`, an alias, and an unknown option still ask, and that `git var GIT_AUTHOR_IDENT` returns the managed author. - A new test covers the unchanged-summary write. - `vitest run` on the launcher, launcher environment, callback bridge, github-operation-credentials, run-identity, heartbeat-github-launchers, native-github-access, openapi-routes, run-trust-preset, http-log-redaction, skill-source-github-access and chat-channels tests: 12 files, 1901 passed, 1 skipped. - `vitest run --project @paperclipai/adapter-utils` on `src`: 1903 passed, 12 skipped, 1 failed. The failure is `sandbox-callback-bridge.test.ts`, a file this PR does not change. It passes alone (89 of 89, three runs). - `tsc --noEmit` passes in `packages/adapter-utils` and in `server`. - Three deliberate breakages each made a test fail: `var` back in the local list, the broker always asked, and the summary write skip removed. - Not run: the full server suite and `pnpm build` (the local machine has no `cargo` for the runner binary). ## Risks - In a partial clone or an LFS repository, a local command that must fetch a missing object runs without a token. - A `git` command that is not local (`commit`, `fetch`, `push`) and every `gh` command still asks the broker once. - No migration. No credential is stored at rest. ## Model Used - Claude Sonnet 5.5 (`claude-sonnet-5-5`, Anthropic) through Claude Code, with tool use (shell, file edits, test runs), wrote the code and tests. - Claude Opus 5.5 (`claude-opus-5-5`, Anthropic) through Claude Code wrote the issue text and this description. - A person asked for the change and submits it. ## 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 `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [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
232 lines
15 KiB
TypeScript
232 lines
15 KiB
TypeScript
import { execFile } from "node:child_process";
|
|
import { createServer } from "node:http";
|
|
import { mkdtemp, mkdir, writeFile, rm } from "node:fs/promises";
|
|
import os from "node:os";
|
|
import path from "node:path";
|
|
import { promisify } from "node:util";
|
|
import { afterEach, describe, expect, it } from "vitest";
|
|
import { githubBrokerEnvironment, githubLauncherSource } from "./github-launcher.js";
|
|
const exec = promisify(execFile);
|
|
const cleanups: Array<() => Promise<unknown>> = [];
|
|
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@example.test>|Local Author <local@example.test>");
|
|
});
|
|
|
|
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<void>(resolve => server.listen(0, "127.0.0.1", resolve));
|
|
const { port } = server.address() as { port: number };
|
|
if (failure === "broker-offline") await new Promise<void>(resolve => server.close(() => resolve()));
|
|
else cleanups.push(() => new Promise<void>(resolve => server.close(() => resolve())));
|
|
const configRoot = path.join(root, "config");
|
|
if (failure === "config-unwritable") await writeFile(configRoot, "not a directory");
|
|
const result = await exec(path.join(bin, "git"), ["remote"], { cwd: root, env: {
|
|
...process.env, ...githubBrokerEnvironment({ GH_TOKEN: "host-must-not-leak" }, { url: `http://127.0.0.1:${port}`, token: "private-capability" }),
|
|
GH_CONFIG_DIR: configRoot, PATH: `${bin}:${process.env.PATH}`,
|
|
} });
|
|
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}`,
|
|
} });
|
|
}, 15_000); // broker-offline retries the transport twice per command before it falls back
|
|
|
|
it("explains unavailable access while allowing local work without credentials", async () => {
|
|
const root = await mkdtemp(path.join(os.tmpdir(), "paperclip-github-diagnostic-"));
|
|
cleanups.push(() => rm(root, {recursive:true,force:true}));
|
|
const bin = path.join(root,"managed"), realBin = path.join(root,"real");
|
|
await mkdir(bin); await mkdir(realBin);
|
|
await writeFile(path.join(bin,"gh"), githubLauncherSource(), {mode:0o700});
|
|
await writeFile(path.join(realBin,"gh"), '#!/usr/bin/env node\nprocess.stdout.write(JSON.stringify({token:process.env.GH_TOKEN ?? null}));', {mode:0o700});
|
|
const server = createServer((_req,res) => {
|
|
res.setHeader("content-type","application/json");
|
|
res.end(JSON.stringify({status:"unavailable",reason:"More than one managed GitHub identity matches this run",env:{GH_TOKEN:"must-not-be-used"}}));
|
|
});
|
|
await new Promise<void>(resolve => server.listen(0,"127.0.0.1",resolve));
|
|
cleanups.push(() => new Promise<void>((resolve,reject) => server.close(error => error ? reject(error) : resolve())));
|
|
const {port} = server.address() as {port:number};
|
|
const result = await exec(path.join(bin,"gh"), [], {env:{...process.env,...githubBrokerEnvironment({GH_TOKEN:"host-token"},{url:`http://127.0.0.1:${port}`,token:"run-capability"}),PATH:`${bin}:${realBin}:${process.env.PATH}`}});
|
|
expect(JSON.parse(result.stdout)).toEqual({token:null});
|
|
expect(result.stderr).toContain("More than one managed GitHub identity matches this run");
|
|
expect(result.stderr).not.toMatch(/host-token|must-not-be-used|run-capability/);
|
|
});
|
|
// The first broker request fails, the second succeeds: the managed token must still reach gh.
|
|
it.each([
|
|
["connection drops before the response", (res: import("node:http").ServerResponse) => { res.socket?.destroy(); }],
|
|
["body read fails mid-response", (res: import("node:http").ServerResponse) => {
|
|
res.writeHead(200, {"content-type":"application/json"}); res.write('{"status":'); setTimeout(() => res.socket?.destroy(), 20);
|
|
}],
|
|
])("retries when the %s and still uses managed credentials", async (_label, fail) => {
|
|
const root = await mkdtemp(path.join(os.tmpdir(), "paperclip-github-retry-"));
|
|
cleanups.push(() => rm(root, {recursive:true,force:true}));
|
|
const bin = path.join(root,"managed"), realBin = path.join(root,"real");
|
|
await mkdir(bin); await mkdir(realBin);
|
|
await writeFile(path.join(bin,"gh"), githubLauncherSource(), {mode:0o700});
|
|
await writeFile(path.join(realBin,"gh"), '#!/usr/bin/env node\nprocess.stdout.write(JSON.stringify({token:process.env.GH_TOKEN ?? null}));', {mode:0o700});
|
|
let requests = 0;
|
|
const server = createServer((_req,res) => {
|
|
requests++;
|
|
if (requests === 1) return fail(res);
|
|
res.setHeader("content-type","application/json");
|
|
res.end(JSON.stringify({status:"available",env:{GH_TOKEN:"managed-token"}}));
|
|
});
|
|
await new Promise<void>(resolve => server.listen(0,"127.0.0.1",resolve));
|
|
cleanups.push(() => new Promise<void>(resolve => server.close(() => resolve())));
|
|
const {port} = server.address() as {port:number};
|
|
const result = await exec(path.join(bin,"gh"), [], {env:{...process.env,...githubBrokerEnvironment({GH_TOKEN:"host-token"},{url:`http://127.0.0.1:${port}`,token:"run-capability"}),PATH:`${bin}:${realBin}:${process.env.PATH}`}});
|
|
expect(JSON.parse(result.stdout)).toEqual({token:"managed-token"});
|
|
expect(requests).toBe(2);
|
|
expect(result.stderr).not.toContain("broker_transport_unavailable");
|
|
expect(result.stderr).not.toMatch(/host-token|run-capability/);
|
|
});
|
|
it("captures each command's identity and clears host credentials when the next person has none", async () => {
|
|
const root = await mkdtemp(path.join(os.tmpdir(), "paperclip-github-launcher-test-"));
|
|
cleanups.push(() => rm(root, { recursive: true, force: true }));
|
|
const bin = path.join(root, "managed"), realBin = path.join(root, "real"), repo = path.join(root, "repo");
|
|
for (const dir of [bin, realBin, repo, path.join(bin, "gh-config")]) await mkdir(dir, { recursive: true });
|
|
for (const name of ["git", "gh"]) await writeFile(path.join(bin, name), githubLauncherSource(), { mode: 0o700 });
|
|
await writeFile(path.join(realBin, "gh"), `#!/usr/bin/env node
|
|
const {execFileSync}=require('node:child_process');
|
|
const identity=execFileSync('git',['var','GIT_AUTHOR_IDENT'],{encoding:'utf8'}).trim();
|
|
process.stdout.write(JSON.stringify({identity, token:process.env.GH_TOKEN ?? null, global:process.env.GIT_CONFIG_GLOBAL, config:process.env.GH_CONFIG_DIR}));
|
|
`, { mode: 0o700 });
|
|
let user: string | null = "A", captures = 0;
|
|
let heldCapture: (() => void) | null = null;
|
|
let releaseCapture: (() => void) | null = null;
|
|
const server = createServer((req, res) => {
|
|
captures++;
|
|
expect(req.headers.authorization).toBe("Bearer run-capability");
|
|
const selected = user;
|
|
res.setHeader("content-type", "application/json");
|
|
const finish = () => res.end(JSON.stringify(selected ? { status: "available", env: {
|
|
GH_TOKEN: `credential-${selected}`, GITHUB_TOKEN: `credential-${selected}`,
|
|
GIT_AUTHOR_NAME: selected, GIT_AUTHOR_EMAIL: `${selected}@example.test`,
|
|
GIT_COMMITTER_NAME: selected, GIT_COMMITTER_EMAIL: `${selected}@example.test`,
|
|
} } : { status: "absent", env: {} }));
|
|
if (heldCapture) { const captured = heldCapture; heldCapture = null; releaseCapture = finish; captured(); }
|
|
else finish();
|
|
});
|
|
await new Promise<void>(resolve => server.listen(0, "127.0.0.1", resolve));
|
|
cleanups.push(() => new Promise<void>((resolve, reject) => server.close(error => error ? reject(error) : resolve())));
|
|
const address = server.address() as { port: number };
|
|
const env: NodeJS.ProcessEnv = { ...process.env, ...githubBrokerEnvironment({
|
|
GH_TOKEN: "ambient-host-token", GIT_AUTHOR_NAME: "Host", GIT_AUTHOR_EMAIL: "host@example.test",
|
|
}, { 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");
|
|
user = "A";
|
|
await git("commit", "--allow-empty", "-m", "A again");
|
|
expect(await git("log", "--format=%an <%ae>|%cn <%ce>" )).toBe("A <A@example.test>|A <A@example.test>\nB <B@example.test>|B <B@example.test>\nA <A@example.test>|A <A@example.test>");
|
|
const before = captures;
|
|
const gh = JSON.parse((await exec(path.join(bin, "gh"), [], { cwd: repo, env })).stdout);
|
|
expect(gh.identity).toContain("A <A@example.test>");
|
|
expect(gh.token).toBe("credential-A");
|
|
expect(captures - before).toBe(1); // gh's child Git retains the same capture.
|
|
const captured = new Promise<void>(resolve => { heldCapture = resolve; });
|
|
const operationA = exec(path.join(bin, "gh"), [], { cwd: repo, env });
|
|
await captured;
|
|
user = "B";
|
|
const operationB = JSON.parse((await exec(path.join(bin, "gh"), [], { cwd: repo, env })).stdout);
|
|
releaseCapture!();
|
|
const completedA = JSON.parse((await operationA).stdout);
|
|
expect(completedA.token).toBe("credential-A");
|
|
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@example.test>|Repository Author <repository@example.test>");
|
|
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("");
|
|
expect(env.GIT_AUTHOR_NAME).toBe("");
|
|
});
|
|
it("keeps local Git commands off the broker and still asks for the rest", async () => {
|
|
const root = await mkdtemp(path.join(os.tmpdir(), "paperclip-github-local-skip-"));
|
|
cleanups.push(() => rm(root, { recursive: true, force: true }));
|
|
const bin = path.join(root, "managed");
|
|
await mkdir(bin);
|
|
await writeFile(path.join(bin, "git"), githubLauncherSource(), { mode: 0o700 });
|
|
let posts = 0;
|
|
const server = createServer((_req, res) => {
|
|
posts++;
|
|
res.setHeader("content-type", "application/json");
|
|
res.end(JSON.stringify({ status: "available", env: {
|
|
GH_TOKEN: "managed-token", GIT_AUTHOR_NAME: "Managed", GIT_AUTHOR_EMAIL: "managed@example.test",
|
|
GIT_COMMITTER_NAME: "Managed", GIT_COMMITTER_EMAIL: "managed@example.test",
|
|
} }));
|
|
});
|
|
await new Promise<void>(resolve => server.listen(0, "127.0.0.1", resolve));
|
|
cleanups.push(() => new Promise<void>(resolve => server.close(() => resolve())));
|
|
const { port } = server.address() as { port: number };
|
|
const env = { ...process.env, ...githubBrokerEnvironment({}, { url: `http://127.0.0.1:${port}`, token: "run-capability" }), PATH: `${bin}:${process.env.PATH}` };
|
|
const git = async (...args: string[]) => {
|
|
const before = posts;
|
|
const { stdout } = await exec(path.join(bin, "git"), args, { cwd: root, env }).catch(() => ({ stdout: "" }));
|
|
return { stdout: stdout.trim(), posts: posts - before };
|
|
};
|
|
for (const args of [
|
|
["init", "-q", "repo"],
|
|
["-C", "repo", "status", "--short"],
|
|
["-C", "repo", "-c", "core.quotepath=off", "--no-pager", "diff"],
|
|
["--git-dir=repo/.git", "rev-parse", "--git-dir"],
|
|
["--git-dir", "repo/.git", "--work-tree", "repo", "status"],
|
|
["-C", "repo", "config", "user.useConfigOnly"],
|
|
["--version"],
|
|
]) expect((await git(...args)).posts, args.join(" ")).toBe(0);
|
|
expect((await git("-C", "repo", "config", "user.useConfigOnly")).stdout).toBe("true");
|
|
for (const args of [
|
|
["-C", "repo", "remote"],
|
|
["-C", "repo", "var", "GIT_AUTHOR_IDENT"],
|
|
["-C", "repo", "fetch", "--dry-run"],
|
|
["-C", "repo", "-c", "alias.noop=!true", "noop"],
|
|
["-C", "repo", "--unknown-option", "status"],
|
|
["-C"],
|
|
]) expect((await git(...args)).posts, args.join(" ")).toBe(1);
|
|
expect((await git("-C", "repo", "var", "GIT_AUTHOR_IDENT")).stdout).toMatch(/^Managed <managed@example.test>/);
|
|
expect((await git("-C", "repo", "commit", "--allow-empty", "-m", "Managed work")).posts).toBe(1);
|
|
expect((await git("-C", "repo", "log", "-1", "--format=%an <%ae>")).stdout).toBe("Managed <managed@example.test>");
|
|
}, 20_000);
|
|
});
|