mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-10 12:37:55 +01:00
fix(github): send gh repo create and fork to the connection's tools (#20142)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agents with a managed GitHub connection get `git` and `gh` launchers that use the connection's credential > - The launcher gives that credential to any `gh` command, and the broker does not see which command runs > - So `gh repo create` and `gh repo fork` skip the connection's tool switches, ask-first approvals, owner defaults, and audit > - The connection's `create_repository` and `fork_repository` tools apply all of these > - This pull request stops `gh repo create`, `gh repo new`, and `gh repo fork` when a managed credential is in use, and names the tool to use instead > - The benefit is that repository creation follows the connection's rules, whichever path the agent tries first ## Linked Issues or Issue Description Refs #20141 (default owner for `create_repository` and `fork_repository`). **What happened?** An agent with a managed GitHub connection can run `gh repo create` in its shell. The command uses the connection's token, but it does not use the connection's tool rules. If the user turns off or reviews `create_repository` on the connection, the agent can still create a repository with `gh`. The owner default from #20141 also does not apply, so the command can fail with `403 Resource not accessible by integration` when the App is installed only on an organization. **Expected behavior** With a managed connection, repository creation and forks go through the connection's tools. The tool switch, approvals, owner default, and audit apply. **Steps to reproduce** 1. Connect GitHub with a managed connection. 2. Set the connection's `create_repository` tool to off or ask first. 3. In an agent run, run `gh repo create demo --private`. 4. The command runs with the connection's credential and ignores the tool setting. ## What Changed - `packages/adapter-utils/src/github-launcher.ts`: when the broker returns a managed credential, `gh repo create`, `gh repo new`, and `gh repo fork` exit with code 1 before `gh` starts. The message names the `create_repository` or `fork_repository` tool and tells the agent to finish with Git. - `--help` still runs. Other `gh` commands do not change. Executions without a managed credential run these commands as before. - `doc/execution-github-identity.md`: describes the behavior and states that this is guidance, not a security boundary. ## Verification - New test in `packages/adapter-utils/src/github-launcher.test.ts`: "sends repository creation and forks to the connection's tools when a connection is in use". It covers `create`, the `new` alias, `fork`, `--help`, another `gh repo` command, and the case without a connection. It fails without the launcher change. - `npx vitest run src/execution-target.test.ts src/github-launcher-environment.test.ts src/github-launcher.test.ts` in `packages/adapter-utils`: 83 passed. - `npx vitest run src/drivers/acpx/github-launcher.test.ts` in `packages/paperclip-runner`: 3 passed. - `npx vitest run src/services/heartbeat-github-launchers.test.ts` in `server`: 12 passed. - `npx tsc --noEmit -p packages/adapter-utils` passes. ## Risks - Agents that used `gh repo create --source . --push` must now call `create_repository`, then add the remote and push with Git. The message tells them to do this. - If the connection's tool is off for the agent, the agent cannot create a repository. This is the purpose of the tool setting. - This is not a security boundary. Other commands that receive the token, such as `gh api`, can still call the same GitHub endpoints. - Running launchers keep their earlier source until the next run stages new launchers. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use (code reading, shell commands, and test runs). ## 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 - [ ] All Paperclip CI gates are green - [ ] 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 <noreply@paperclip.ing>
This commit is contained in:
1 parent
0e49576fd9
commit
546cfee919
3 files changed
+62
No files matched your search
@@ -14,6 +14,15 @@ Delegated work and interactions persist their originating context. Retries retai
|
||||
|
||||
Executions with managed GitHub configured receive token-free `git` and `gh` launchers. Each launcher invocation resolves one eligible credential from the current run's accepted identity at operation start. A `gh` command's child Git processes inherit that command's captured identity; later steering does not change already-started operations.
|
||||
|
||||
When the broker returns a managed credential, `gh repo create`, its
|
||||
`gh repo new` alias, and `gh repo fork` stop before they run. The message
|
||||
names the connection's `create_repository` or `fork_repository` tool. Those
|
||||
tools apply the connection's tool access rules, approvals, owner defaults, and
|
||||
audit, which a raw `gh` command cannot. The agent then uses Git to push to the
|
||||
new repository or to clone the fork. `--help` still runs. Without a managed
|
||||
credential, `gh` runs these commands unchanged. This guidance is not a
|
||||
security boundary: any command that receives the token can call the GitHub API.
|
||||
|
||||
A `git` command that never uses the network does not ask the broker. The launcher reads past the global options, such as `-C` and `-c`, and checks the subcommand against a short list (for example `status`, `diff`, `diff-files`, `write-tree`, `log`, `rev-parse`, `add`, `config`, `branch`, `checkout`). This includes the index comparison and tree writing used by workspace snapshots. 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 and never writes a credential to a file.
|
||||
|
||||
GitHub credential capture reads the current task, project, and retained trust
|
||||
|
||||
@@ -189,6 +189,42 @@ process.exit(result.status ?? 98);
|
||||
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/);
|
||||
});
|
||||
it("sends repository creation and forks to the connection's tools when a connection is in use", async () => {
|
||||
const root = await mkdtemp(path.join(os.tmpdir(), "paperclip-github-repository-tools-"));
|
||||
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({args:process.argv.slice(2),token:process.env.GH_TOKEN ?? null}));', { mode: 0o700 });
|
||||
let status = "available";
|
||||
const server = createServer((_req, res) => {
|
||||
res.setHeader("content-type", "application/json");
|
||||
res.end(JSON.stringify({ status, 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 env = { ...process.env, ...githubBrokerEnvironment({}, { url: `http://127.0.0.1:${port}`, token: "run-capability" }),
|
||||
PATH: `${bin}:${realBin}:${process.env.PATH}` };
|
||||
const gh = (...args: string[]) => exec(path.join(bin, "gh"), args, { env });
|
||||
for (const [args, tool] of [
|
||||
[["repo", "create", "demo", "--private"], "create_repository"],
|
||||
[["repo", "new", "demo"], "create_repository"],
|
||||
[["repo", "fork", "octo-org/tools", "--org", "acme"], "fork_repository"],
|
||||
] as const) {
|
||||
const failure = await gh(...args).catch((error: { code?: number; stdout?: string; stderr?: string }) => error);
|
||||
expect(failure).toMatchObject({ code: 1, stdout: "" });
|
||||
expect(failure.stderr).toContain(`GitHub connection's ${tool} tool`);
|
||||
expect(failure.stderr).not.toContain("managed-token");
|
||||
}
|
||||
expect(JSON.parse((await gh("repo", "create", "--help")).stdout))
|
||||
.toEqual({ args: ["repo", "create", "--help"], token: "managed-token" });
|
||||
expect(JSON.parse((await gh("repo", "view", "octo-org/tools")).stdout))
|
||||
.toEqual({ args: ["repo", "view", "octo-org/tools"], token: "managed-token" });
|
||||
// Without a connection there are no connection tools; gh runs unchanged.
|
||||
status = "absent";
|
||||
expect(JSON.parse((await gh("repo", "create", "demo")).stdout)).toEqual({ args: ["repo", "create", "demo"], token: null });
|
||||
});
|
||||
it.each(["before-write", "partial-write"])("keeps Git and managed credentials available after a %s failure", async (failure) => {
|
||||
const root = await mkdtemp(path.join(os.tmpdir(), "paperclip-github-default-write-failure-"));
|
||||
cleanups.push(() => rm(root, { recursive: true, force: true }));
|
||||
|
||||
@@ -43,6 +43,13 @@ function isLocalGit(args) {
|
||||
}
|
||||
return false;
|
||||
}
|
||||
// With a Paperclip GitHub connection, repositories are created and forked
|
||||
// through its tools so access rules, approvals, owner defaults and audit apply.
|
||||
const connectorRepositoryTools = new Map([['create', 'create_repository'], ['new', 'create_repository'], ['fork', 'fork_repository']]);
|
||||
function connectorRepositoryTool(args) {
|
||||
if (program !== 'gh' || args[0] !== 'repo' || args.includes('--help') || args.includes('-h')) return null;
|
||||
return connectorRepositoryTools.get(args[1]) || null;
|
||||
}
|
||||
async function main() {
|
||||
let env = { ...process.env };
|
||||
${options.brokerFile ? String.raw`
|
||||
@@ -60,6 +67,7 @@ ${options.brokerFile ? String.raw`
|
||||
} catch {}
|
||||
` : ''}
|
||||
const diagnostic = (code) => process.stderr.write('Paperclip: GitHub ' + code + '; continuing without managed credentials.\n');
|
||||
let connected = false;
|
||||
const configRoot = env.GH_CONFIG_DIR || os.tmpdir();
|
||||
// A missing/unwritable scratch directory must not break local Git. The
|
||||
// fallback deliberately cannot load the host's gh authentication files.
|
||||
@@ -131,6 +139,7 @@ ${options.brokerFile ? String.raw`
|
||||
: 'Check the GitHub connection in Paperclip';
|
||||
process.stderr.write('Paperclip: GitHub access unavailable: ' + reason + '. Continuing without GitHub credentials.\n');
|
||||
}
|
||||
connected = result.status === 'available';
|
||||
if (result.status === 'available' && configReady) {
|
||||
for (const [key, value] of Object.entries(result.env || {})) {
|
||||
if (/^(GH_TOKEN|GITHUB_TOKEN|PAPERCLIP_GIT_TOKEN|GIT_TERMINAL_PROMPT|GIT_AUTHOR_(NAME|EMAIL)|GIT_COMMITTER_(NAME|EMAIL)|GIT_CONFIG_COUNT|GIT_CONFIG_(KEY|VALUE)_\d+)$/.test(key) && typeof value === 'string') env[key] = value;
|
||||
@@ -140,6 +149,14 @@ ${options.brokerFile ? String.raw`
|
||||
} else if (askBroker) { diagnostic('capability_missing'); }
|
||||
} catch { diagnostic('broker_transport_unavailable'); }
|
||||
}
|
||||
const connectorTool = connected ? connectorRepositoryTool(process.argv.slice(2)) : null;
|
||||
if (connectorTool) {
|
||||
process.stderr.write("Paperclip: use the GitHub connection's " + connectorTool + " tool instead of gh repo " + process.argv[3] +
|
||||
", so the connection's access rules apply. " + (connectorTool === 'fork_repository'
|
||||
? 'Then clone the fork or add it as a git remote.' : 'Then add the repository as a git remote and push.') + '\n');
|
||||
process.exitCode = 1;
|
||||
return;
|
||||
}
|
||||
// Only this invocation and its children inherit the captured credential.
|
||||
env.PAPERCLIP_GITHUB_OPERATION_ACTIVE = '1';
|
||||
// Its Git children use the real binary, so steering cannot split a gh operation.
|
||||
|
||||
Reference in new issue
Block a user