diff --git a/doc/connections/GITHUB.md b/doc/connections/GITHUB.md index 23d826b979b..74279d8fd5f 100644 --- a/doc/connections/GITHUB.md +++ b/doc/connections/GITHUB.md @@ -154,7 +154,13 @@ repository owner first. The authenticated GitHub username and the accounts where the App is installed are separate. An organization installation does not grant access to the user's personal repositories. Personal repository creation needs App access to that personal account; organization creation needs App access -to that organization and the tool's `organization` argument. The agent must +to that organization and the tool's `organization` argument. When the App is +installed on exactly one account, and its access snapshot identifies that +account as an organization, `create_repository` and `fork_repository` calls that omit `organization` use +that organization, because the personal account is unavailable. Paperclip fills +it in before policy, approval, and audit, so approval cards and test results +show the owner. An explicit owner is unchanged, and any other installation set +keeps GitHub's personal-account default. The agent must keep the owner requested by the user. The user can choose the intended account from **Add More Repos on GitHub**, then select **Refresh access**. If creation remains unavailable, the user can create the repository on GitHub and give the App access, diff --git a/server/src/__tests__/tool-gateway.test.ts b/server/src/__tests__/tool-gateway.test.ts index 2aca442fc2b..61d26d36d1e 100644 --- a/server/src/__tests__/tool-gateway.test.ts +++ b/server/src/__tests__/tool-gateway.test.ts @@ -738,6 +738,60 @@ describeEmbeddedPostgres("tool gateway acceptance", () => { } finally { await remote.close(); } }); + it("defaults GitHub repository creation to the only installed organization for agents and connection tests", async () => { + const company = await createCompany(db); + const agent = await createAgent(db, company.id); + const { run } = await createIssueAndRun(db, company.id, agent.id); + const remote = await startFakeRemoteMcpServer(({ body }) => ({ body: { + jsonrpc: "2.0", id: body?.id, result: { content: [{ type: "text", text: "Repository created" }] }, + } })); + try { + const { application, connection, catalogEntry } = await createRemoteMcpTool(db, company.id, { + url: remote.url, applicationKey: "github", toolName: "create_repository", riskLevel: "write", + connectionConfig: { sourceTemplateKey: "github", connectionMethodKey: "managed" }, + }); + await db.update(toolCatalogEntries).set({ inputSchema: { + type: "object", required: ["name"], additionalProperties: false, properties: { + name: { type: "string" }, + organization: { type: "string", description: "Organization to create the repository in (omit to create in your personal account)" }, + }, + } }).where(eq(toolCatalogEntries.id, catalogEntry.id)); + const github = { userId: "1", login: "octocat", installationCount: 1, repositoryCount: 1, + repositorySelection: "all" as const, installationIds: ["9"], installationOwnerLogins: ["acme"], + repositories: [{ id: "10", fullName: "acme/site", installationId: "9", ownerType: "organization" as const }] }; + await db.update(connectionGrants).set({ providerTenant: { github } }) + .where(eq(connectionGrants.connectionId, connection.id)); + const toolName = expectedConnectedToolName({ applicationKey: application.applicationKey, + connectionId: connection.id, toolName: catalogEntry.toolName }); + await allowToolsForAgent(db, company.id, agent.id, [toolName]); + const gateway = createTestToolGatewayService(db); + const session = await gateway.createSession({ companyId: company.id, agentId: agent.id, runId: run.id }); + const listed = (await gateway.listToolsForSession(session.token)).find((tool) => tool.name === toolName); + expect((listed?.parametersSchema as { properties?: Record } | undefined) + ?.properties?.organization?.description).toMatch(/installed only on one organization; then Paperclip uses that organization/); + + await gateway.executeTool({ sessionToken: session.token, tool: toolName, parameters: { name: "agent-site" } }); + await gateway.executeTool({ sessionToken: session.token, tool: toolName, + parameters: { name: "chosen-site", organization: "globex" } }); + await createActiveMember(db, company.id, "test-user"); + const testCall = await gateway.executeTestCall({ companyId: company.id, connectionId: connection.id, + agentId: agent.id, userId: "test-user", toolName, parameters: { name: "test-site" } }); + expect(testCall).toMatchObject({ decision: "allowed" }); + // With the App also on the personal account, GitHub's personal default applies. + await db.update(connectionGrants).set({ providerTenant: { github: { ...github, installationOwnerLogins: ["octocat", "acme"] } } }) + .where(eq(connectionGrants.connectionId, connection.id)); + await gateway.executeTool({ sessionToken: session.token, tool: toolName, parameters: { name: "personal-site" } }); + + expect(remote.requests.filter((entry) => entry.body?.method === "tools/call") + .map((entry) => (entry.body?.params as { arguments?: unknown } | undefined)?.arguments)).toEqual([ + { name: "agent-site", organization: "acme" }, + { name: "chosen-site", organization: "globex" }, + { name: "test-site", organization: "acme" }, + { name: "personal-site" }, + ]); + } finally { await remote.close(); } + }); + it.each(["config", "transport", "application", "legacy_application"])("records an actionable GitHub access error from %s metadata after one approved call and keeps the connection usable", async source => { const company = await createCompany(db); const agent = await createAgent(db, company.id); diff --git a/server/src/services/tool-access/connection-methods.ts b/server/src/services/tool-access/connection-methods.ts index 4ddd35ca07b..a659a4dee9f 100644 --- a/server/src/services/tool-access/connection-methods.ts +++ b/server/src/services/tool-access/connection-methods.ts @@ -15,6 +15,7 @@ import { isPaperclipCloudConnectorStrategy } from "../paperclip-cloud-connector. import { honchoManagedArguments } from "../honcho-connection.js"; import { assertGoogleChatToolArgumentsSupported, googleChatToolInputSchema } from "../google-chat-tool-policy.js"; import { gmailArchiveToolInputSchema } from "../gmail-archive-tool-policy.js"; +import { githubOwnerToolInputSchema } from "./github-default-owner.js"; type ConnectionSettings = { config: Record; @@ -345,6 +346,7 @@ export function projectedConnectionToolInputSchema( if (managed) inputSchema = stripManagedToolArgumentSchema(inputSchema, managed); inputSchema = googleChatToolInputSchema(connection, toolName, inputSchema); inputSchema = gmailArchiveToolInputSchema(connection, toolName, inputSchema); + inputSchema = githubOwnerToolInputSchema(connection, toolName, inputSchema); const sourceTemplateKey = typeof connection.config.sourceTemplateKey === "string" ? connection.config.sourceTemplateKey diff --git a/server/src/services/tool-access/github-default-owner.test.ts b/server/src/services/tool-access/github-default-owner.test.ts new file mode 100644 index 00000000000..75fbad4ec6a --- /dev/null +++ b/server/src/services/tool-access/github-default-owner.test.ts @@ -0,0 +1,105 @@ +import { describe, expect, it } from "vitest"; +import { + githubDefaultOrganization, + githubDefaultOwnerApplies, + githubDefaultOwnerArguments, + githubOwnerToolInputSchema, +} from "./github-default-owner.js"; + +const managed = { config: { sourceTemplateKey: "github", connectionMethodKey: "managed" } }; +const createSchema = { + type: "object", + properties: { + name: { type: "string" }, + organization: { type: "string", description: "Organization to create the repository in (omit to create in your personal account)" }, + }, + required: ["name"], +}; +const orgOnly = { + login: "octocat", + installationOwnerLogins: ["acme"], + repositories: [{ id: "1", fullName: "acme/site", installationId: "9", ownerType: "organization" }], +}; + +describe("githubDefaultOrganization", () => { + it("uses the only installed organization when the personal account has no installation", () => { + expect(githubDefaultOrganization(orgOnly)).toBe("acme"); + }); + + it("requires the snapshot to identify the owner as an organization", () => { + // Snapshots written before ownerType was recorded keep GitHub's behavior. + expect(githubDefaultOrganization({ login: "octocat", installationOwnerLogins: ["hubot"] })).toBeNull(); + expect(githubDefaultOrganization({ login: "octocat", installationOwnerLogins: ["hubot"], + repositories: [{ fullName: "hubot/tools", installationId: "9" }] })).toBeNull(); + expect(githubDefaultOrganization({ login: "octocat", installationOwnerLogins: ["acme"], + repositories: [{ fullName: "acme/site", ownerType: "organization" }, { fullName: "acme/docs" }] })).toBeNull(); + }); + + it("keeps GitHub's personal default when the personal account is installed", () => { + expect(githubDefaultOrganization({ login: "octocat", installationOwnerLogins: ["octocat"] })).toBeNull(); + expect(githubDefaultOrganization({ login: "OctoCat", installationOwnerLogins: ["octocat", "acme"] })).toBeNull(); + }); + + it("does not guess between several organizations", () => { + expect(githubDefaultOrganization({ ...orgOnly, installationOwnerLogins: ["acme", "globex"] })).toBeNull(); + expect(githubDefaultOrganization({ ...orgOnly, installationOwnerLogins: ["Acme", "acme"] })).toBe("Acme"); + }); + + it("ignores another person's account and incomplete snapshots", () => { + expect(githubDefaultOrganization({ login: "octocat", installationOwnerLogins: ["hubot"], + repositories: [{ fullName: "hubot/tools", ownerType: "personal" }] })).toBeNull(); + expect(githubDefaultOrganization({ installationOwnerLogins: ["acme"] })).toBeNull(); + expect(githubDefaultOrganization({ login: "octocat", installationOwnerLogins: [] })).toBeNull(); + expect(githubDefaultOrganization(undefined)).toBeNull(); + }); +}); + +describe("githubDefaultOwnerArguments", () => { + it("fills an omitted or blank organization", () => { + expect(githubDefaultOwnerArguments("create_repository", { name: "site" }, orgOnly)) + .toEqual({ name: "site", organization: "acme" }); + expect(githubDefaultOwnerArguments("mcp.github:create-repository", { name: "site", organization: " " }, orgOnly)) + .toEqual({ name: "site", organization: "acme" }); + expect(githubDefaultOwnerArguments("fork_repository", { owner: "octo-org", repo: "tools" }, orgOnly)) + .toEqual({ owner: "octo-org", repo: "tools", organization: "acme" }); + }); + + it("keeps explicit owners and calls without a default", () => { + const explicit = { name: "site", organization: "globex" }; + expect(githubDefaultOwnerArguments("create_repository", explicit, orgOnly)).toBe(explicit); + const personal = { name: "site" }; + expect(githubDefaultOwnerArguments("create_repository", personal, + { login: "octocat", installationOwnerLogins: ["octocat"] })).toBe(personal); + }); + + it("does not fork a repository into its own organization", () => { + const fork = { owner: "ACME", repo: "site" }; + expect(githubDefaultOwnerArguments("fork_repository", fork, orgOnly)).toBe(fork); + }); +}); + +describe("githubDefaultOwnerApplies", () => { + it("applies only to managed GitHub owner tools that declare organization", () => { + expect(githubDefaultOwnerApplies(managed, "create_repository", createSchema, { name: "site" })).toBe(true); + expect(githubDefaultOwnerApplies({ config: {}, transportConfig: managed.config }, "fork_repository", + createSchema, { owner: "a", repo: "b" })).toBe(true); + expect(githubDefaultOwnerApplies(managed, "create_repository", createSchema, { name: "site", organization: "acme" })).toBe(false); + expect(githubDefaultOwnerApplies(managed, "create_issue", createSchema, {})).toBe(false); + expect(githubDefaultOwnerApplies(managed, "create_repository", { properties: { name: {} } }, { name: "site" })).toBe(false); + expect(githubDefaultOwnerApplies({ config: { sourceTemplateKey: "github", connectionMethodKey: "mcp-key" } }, + "create_repository", createSchema, { name: "site" })).toBe(false); + }); +}); + +describe("githubOwnerToolInputSchema", () => { + it("describes the default for managed GitHub owner tools only", () => { + const projected = githubOwnerToolInputSchema(managed, "create_repository", createSchema); + expect(projected.required).toEqual(["name"]); + expect((projected.properties as Record).organization?.description) + .toMatch(/installed only on one organization; then Paperclip uses that organization/); + expect(createSchema.properties.organization.description).toMatch(/omit to create in your personal account/); + expect(githubOwnerToolInputSchema({ config: { sourceTemplateKey: "github", connectionMethodKey: "mcp-key" } }, + "create_repository", createSchema)).toBe(createSchema); + expect(githubOwnerToolInputSchema(managed, "create_issue", createSchema)).toBe(createSchema); + }); +}); diff --git a/server/src/services/tool-access/github-default-owner.ts b/server/src/services/tool-access/github-default-owner.ts new file mode 100644 index 00000000000..a522152e8bc --- /dev/null +++ b/server/src/services/tool-access/github-default-owner.ts @@ -0,0 +1,107 @@ +type Connection = { config: Record; transportConfig?: Record | null }; + +/** GitHub tools that target the authenticated user's personal account when + * `organization` is omitted. A managed connection can only write there when + * the Paperclip GitHub App is installed on that personal account. */ +const OWNER_DESCRIPTIONS: Record = { + create_repository: "Organization to create the repository in. When omitted, the repository goes to your personal account, unless the Paperclip GitHub App is installed only on one organization; then Paperclip uses that organization.", + fork_repository: "Organization to fork to. When omitted, the fork goes to your personal account, unless the Paperclip GitHub App is installed only on one organization; then Paperclip uses that organization.", +}; + +function record(value: unknown): Record { + return value !== null && typeof value === "object" && !Array.isArray(value) + ? value as Record + : {}; +} + +function operation(toolName: string): string { + return (toolName.split(/[.:/]/).pop() ?? "").trim().toLowerCase().replace(/-/g, "_"); +} + +function isManagedGitHub(connection: Connection): boolean { + const config = record(connection.config); + const transport = record(connection.transportConfig); + return (config.sourceTemplateKey ?? transport.sourceTemplateKey) === "github" + && (config.connectionMethodKey ?? transport.connectionMethodKey) === "managed"; +} + +function omitted(value: unknown): boolean { + return value === undefined || value === null || (typeof value === "string" && value.trim() === ""); +} + +function ownerOwnedTool(connection: Connection, toolName: string, inputSchema: unknown): boolean { + return isManagedGitHub(connection) + && operation(toolName) in OWNER_DESCRIPTIONS + && "organization" in record(record(inputSchema).properties); +} + +/** The only account the Paperclip GitHub App is installed on, when that + * account is an organization rather than the user's own. Repository creation + * without an organization fails for such a grant, so this is the one owner + * where it can succeed. Suspended installations are already excluded from + * the access snapshot. */ +export function githubDefaultOrganization(github: unknown): string | null { + const access = record(github); + const login = typeof access.login === "string" ? access.login.trim().toLowerCase() : ""; + // GitHub logins are case-insensitive. + const owners = new Map(); + for (const value of Array.isArray(access.installationOwnerLogins) ? access.installationOwnerLogins : []) { + if (typeof value !== "string" || value.trim() === "") continue; + if (!owners.has(value.trim().toLowerCase())) owners.set(value.trim().toLowerCase(), value.trim()); + } + if (!login || owners.size !== 1 || owners.has(login)) return null; + const owner = [...owners.values()][0]!; + // An installation on another person's account can be visible to a + // collaborator, and GitHub cannot create organization repositories there. + // Require the snapshot to identify the owner as an organization; snapshots + // without account types keep GitHub's behavior until access is refreshed. + const ownerTypes = new Set((Array.isArray(access.repositories) ? access.repositories : []) + .map(record) + .filter((entry) => typeof entry.fullName === "string" + && entry.fullName.split("/")[0]?.toLowerCase() === owner.toLowerCase()) + .map((entry) => entry.ownerType)); + return ownerTypes.size === 1 && ownerTypes.has("organization") ? owner : null; +} + +/** Whether this call could take a default organization. Checked before the + * grant is loaded so other tools never pay for the lookup. */ +export function githubDefaultOwnerApplies( + connection: Connection, + toolName: string, + inputSchema: unknown, + parameters: unknown, +): boolean { + return ownerOwnedTool(connection, toolName, inputSchema) && omitted(record(parameters).organization); +} + +/** Fill an omitted `organization` from the grant's access snapshot. Explicit + * owners are kept, and a repository is never forked into its own owner. */ +export function githubDefaultOwnerArguments( + toolName: string, + parameters: Record, + github: unknown, +): Record { + const organization = githubDefaultOrganization(github); + if (!organization || !omitted(parameters.organization)) return parameters; + if (operation(toolName) === "fork_repository" && typeof parameters.owner === "string" && + parameters.owner.trim().toLowerCase() === organization.toLowerCase()) return parameters; + return { ...parameters, organization }; +} + +/** Describe the default on the `organization` argument for agents and the + * connection test form. */ +export function githubOwnerToolInputSchema( + connection: Connection, + toolName: string, + schema: Record, +): Record { + if (!ownerOwnedTool(connection, toolName, schema)) return schema; + const properties = record(schema.properties); + return { + ...schema, + properties: { + ...properties, + organization: { ...record(properties.organization), description: OWNER_DESCRIPTIONS[operation(toolName)] }, + }, + }; +} diff --git a/server/src/services/tool-gateway.ts b/server/src/services/tool-gateway.ts index 9a9d3881663..d8b2b01fb4e 100644 --- a/server/src/services/tool-gateway.ts +++ b/server/src/services/tool-gateway.ts @@ -22,6 +22,7 @@ import { composeConnectionInstructions } from "./connection-instructions.js"; import { googleWorkspaceToolTarget, googleWorkspaceGrantedScopes, isGoogleWorkspaceToolGranted } from "@paperclipai/shared"; import { isInsufficientConnectionScope, INSUFFICIENT_CONNECTION_SCOPE_MESSAGE } from "./connection-permission-errors.js"; import { githubAccessDeniedInstructions, githubAccessError } from "./tool-access/github-access-errors.js"; +import { githubDefaultOwnerApplies, githubDefaultOwnerArguments } from "./tool-access/github-default-owner.js"; import { toolSourceTemplateKeySql } from "./tool-access/source-template-key.js"; import { boundedMcpToolName } from "./mcp-tool-names.js"; import { browserUseService } from "./browser-use.js"; @@ -5075,7 +5076,12 @@ export function createToolGatewayService( ): Promise { if (tool.providerType !== "mcp_remote_http" && tool.providerType !== "provider_rest") return parameters; const { connection, entry } = await resolveConnectedRemoteTool(session, tool); - return projectedConnectionToolArguments(connection, parameters, entry.toolName, entry.inputSchema ?? {}); + const projected = projectedConnectionToolArguments(connection, parameters, entry.toolName, entry.inputSchema ?? {}); + if (!githubDefaultOwnerApplies(connection, entry.toolName, entry.inputSchema, projected)) return projected; + // An unavailable credential keeps the call unchanged and fails with its own + // error at dispatch. + const grant = await resolveConnectionGrant(session, connection, false).catch(() => null); + return githubDefaultOwnerArguments(entry.toolName, projected, grant?.providerTenant?.github); } async function approvedManagedArgumentsRemainCurrent(