diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index c06c58e..3d6135c 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -92,6 +92,11 @@ jobs: - 'package-lock.json' - 'packages/messaging/**' - 'tools/kubectl-readonly/**' + github-tool: + - 'package.json' + - 'package-lock.json' + - 'packages/messaging/**' + - 'tools/github/**' core-controller: - 'controllers/core-controller/**' localtool-executor: @@ -112,6 +117,7 @@ jobs: WEB_SEARCH: ${{ steps.filter.outputs.web-search }} WEB_FETCH: ${{ steps.filter.outputs.web-fetch }} KUBECTL_READONLY: ${{ steps.filter.outputs.kubectl-readonly }} + GITHUB_TOOL: ${{ steps.filter.outputs.github-tool }} CORE_CONTROLLER: ${{ steps.filter.outputs.core-controller }} LOCALTOOL_EXECUTOR: ${{ steps.filter.outputs.localtool-executor }} run: | @@ -126,6 +132,7 @@ jobs: {"image":"web-search","changed_key":"WEB_SEARCH","dockerfile":"tools/web-search/Dockerfile","context":"."}, {"image":"web-fetch","changed_key":"WEB_FETCH","dockerfile":"tools/web-fetch/Dockerfile","context":"."}, {"image":"kubectl-readonly","changed_key":"KUBECTL_READONLY","dockerfile":"tools/kubectl-readonly/Dockerfile","context":"."}, + {"image":"github","changed_key":"GITHUB_TOOL","dockerfile":"tools/github/Dockerfile","context":"."}, {"image":"core-controller","changed_key":"CORE_CONTROLLER","dockerfile":"controllers/core-controller/Dockerfile","context":"controllers/core-controller"}, {"image":"localtool-executor-node","changed_key":"LOCALTOOL_EXECUTOR","dockerfile":"sidecars/localtool-executor/Dockerfile","context":"sidecars/localtool-executor","build_args":"BASE_IMAGE=node:24-bookworm-slim\nRUNTIME=node"}, {"image":"localtool-executor-python","changed_key":"LOCALTOOL_EXECUTOR","dockerfile":"sidecars/localtool-executor/Dockerfile","context":"sidecars/localtool-executor","build_args":"BASE_IMAGE=python:3.12-slim-bookworm\nRUNTIME=python"}, @@ -150,9 +157,10 @@ jobs: --arg WEB_SEARCH "$WEB_SEARCH" \ --arg WEB_FETCH "$WEB_FETCH" \ --arg KUBECTL_READONLY "$KUBECTL_READONLY" \ + --arg GITHUB_TOOL "$GITHUB_TOOL" \ --arg CORE_CONTROLLER "$CORE_CONTROLLER" \ --arg LOCALTOOL_EXECUTOR "$LOCALTOOL_EXECUTOR" \ - '{AGENT_ORCHESTRATOR:$AGENT_ORCHESTRATOR,OPENCODE_SWE_AGENT:$OPENCODE_SWE_AGENT,CLAUDE_CODE_SWE_AGENT:$CLAUDE_CODE_SWE_AGENT,INTEGRATION_GATEWAY:$INTEGRATION_GATEWAY,RECIPE_SCRAPER:$RECIPE_SCRAPER,RECIPE_PUBLISHER:$RECIPE_PUBLISHER,WEB_SEARCH:$WEB_SEARCH,WEB_FETCH:$WEB_FETCH,KUBECTL_READONLY:$KUBECTL_READONLY,CORE_CONTROLLER:$CORE_CONTROLLER,LOCALTOOL_EXECUTOR:$LOCALTOOL_EXECUTOR}') + '{AGENT_ORCHESTRATOR:$AGENT_ORCHESTRATOR,OPENCODE_SWE_AGENT:$OPENCODE_SWE_AGENT,CLAUDE_CODE_SWE_AGENT:$CLAUDE_CODE_SWE_AGENT,INTEGRATION_GATEWAY:$INTEGRATION_GATEWAY,RECIPE_SCRAPER:$RECIPE_SCRAPER,RECIPE_PUBLISHER:$RECIPE_PUBLISHER,WEB_SEARCH:$WEB_SEARCH,WEB_FETCH:$WEB_FETCH,KUBECTL_READONLY:$KUBECTL_READONLY,GITHUB_TOOL:$GITHUB_TOOL,CORE_CONTROLLER:$CORE_CONTROLLER,LOCALTOOL_EXECUTOR:$LOCALTOOL_EXECUTOR}') MATRIX=$(jq -c --argjson flags "$FLAGS" 'map(select($flags[.changed_key] == "true"))' all.json) fi diff --git a/README.md b/README.md index 8c56775..180b577 100644 --- a/README.md +++ b/README.md @@ -90,7 +90,8 @@ orchestrator service, and `controllers/` holds the Go controller. │ └── github-app-auth/ # @controller-agent/github-app-auth — GitHub App JWT/token auth ├── tools/ # on-demand tool containers (example implementations) │ ├── recipe-scraper/ # URL → recipe Markdown -│ └── recipe-publisher/ # recipe Markdown → Mealie instance +│ ├── recipe-publisher/ # recipe Markdown → Mealie instance +│ └── github/ # gh CLI command → GitHub, as the calling user's own identity ├── apps/ │ ├── agent-orchestrator/ # RAG skill selection + ToolRun/AgentRun creator │ └── integration-gateway/ # GitHub Issues → agent-orchestrator webhook adapter @@ -137,6 +138,7 @@ identity, selects a Skill via RAG, plans an action, and creates a `ToolRun` or | ---- | ----- | ------ | ---- | | **recipe-scraper** | any recipe URL (web page, video, or image) | recipe Markdown | [tools/recipe-scraper/README.md](tools/recipe-scraper/README.md) | | **recipe-publisher** | recipe Markdown | published/updated recipe in a Mealie instance | [tools/recipe-publisher/README.md](tools/recipe-publisher/README.md) | +| **github** | a single `gh` CLI command line | `gh`'s own output, authenticated as the calling user's own linked GitHub identity | [tools/github/README.md](tools/github/README.md) | ## Shared standards diff --git a/apps/agent-orchestrator/src/agent/graph.test.ts b/apps/agent-orchestrator/src/agent/graph.test.ts index 4b9cd8a..9e68542 100644 --- a/apps/agent-orchestrator/src/agent/graph.test.ts +++ b/apps/agent-orchestrator/src/agent/graph.test.ts @@ -1334,6 +1334,140 @@ describe("buildAgentGraph agent-backed Tool (runTool via AgentRun)", () => { }); }); +describe("buildAgentGraph identity-gated container Tool (ADR 0032, e.g. the github Tool)", () => { + const githubTool: ToolDescriptor = { + id: "github", + name: "github", + description: "Runs a single gh CLI command against GitHub, authenticated as the calling user", + allowedRoles: ["writer"], + jobTemplate: { image: "example.com/github:latest", namespace: "default", serviceAccountName: "github-tool" }, + identityProviders: ["github"], + }; + + const githubToolSkill: SkillDescriptor = { + id: "github-cli-skill", + name: "GitHub CLI", + description: "Runs gh CLI commands against GitHub", + markdown: "# instructions", + toolIds: ["github"], + agentIds: [], + }; + + function githubToolDeps(overrides: Partial = {}) { + const skillStore: SkillStore = { + upsert: vi.fn(), + delete: vi.fn(), + query: vi.fn().mockResolvedValue([{ skill: githubToolSkill, score: 0.9 }]), + getByIds: vi.fn().mockResolvedValue([githubToolSkill]), + }; + const vectorStore: VectorStore = { + upsert: vi.fn(), + delete: vi.fn(), + query: vi.fn().mockResolvedValue([]), + getByIds: vi.fn().mockResolvedValue([{ tool: githubTool, score: 1 }]), + }; + const actionPlanner: ActionPlanner = { + plan: vi.fn().mockResolvedValue({ + action: "call_tool", + toolId: "github", + toolArgs: "issue view 86 --repo imaustink/agent-controller", + } satisfies PlannedAction), + }; + return baseDeps({ skillStore, vectorStore, actionPlanner, ...overrides }); + } + + it("injects the caller's already-linked token as secretEnv instead of erroring", async () => { + const identityLinkGateway: IdentityLinkPort = { + start: vi.fn(), + poll: vi.fn(), + getToken: vi.fn().mockResolvedValue({ token: "gho_alice-token" }), + }; + const deps = githubToolDeps({ identityLinkGateway }); + const graph = buildAgentGraph(deps); + + const final = await graph.invoke({ request: "what does issue 86 say", authToken: "tok" }); + + expect(final.error).toBeUndefined(); + expect(identityLinkGateway.getToken).toHaveBeenCalledWith("github", "alice"); + expect(deps.containerToolLauncher.launch).toHaveBeenCalledWith( + githubTool.jobTemplate, + expect.objectContaining({ secretEnv: [{ name: "GITHUB_TOKEN", value: "gho_alice-token" }] }), + ); + }); + + it("errors (without launching) instead of silently running credential-less when the caller has no linked token", async () => { + const identityLinkGateway: IdentityLinkPort = { + start: vi.fn(), + poll: vi.fn(), + getToken: vi.fn().mockResolvedValue(undefined), + }; + const deps = githubToolDeps({ identityLinkGateway }); + const graph = buildAgentGraph(deps); + + const final = await graph.invoke({ request: "what does issue 86 say", authToken: "tok" }); + + expect(final.error).toMatch(/requires linking your github account/); + expect(deps.containerToolLauncher.launch).not.toHaveBeenCalled(); + }); + + it("errors when identity providers are declared but no identity-link gateway is configured", async () => { + const deps = githubToolDeps({ identityLinkGateway: undefined }); + const graph = buildAgentGraph(deps); + + const final = await graph.invoke({ request: "what does issue 86 say", authToken: "tok" }); + + expect(final.error).toMatch(/no identity-link gateway is configured/); + expect(deps.containerToolLauncher.launch).not.toHaveBeenCalled(); + }); + + it("resolves a non-github provider through the provider-aware gateway, not the GitHub-only one", async () => { + // A container Tool declaring identityProviders:["claude"] must have its + // token read from claudeAuthGateway (via identityGatewayFor/AuthorizationService), + // NOT the GitHub-only identityLinkGateway -- guarding the bypass that + // hard-coding deps.identityLinkGateway would have re-introduced. + const claudeTool: ToolDescriptor = { ...githubTool, identityProviders: ["claude"] }; + const claudeToolSkill: SkillDescriptor = { ...githubToolSkill, toolIds: ["github"] }; + const claudeAuthGateway: IdentityLinkPort = { + start: vi.fn(), + poll: vi.fn(), + getToken: vi.fn().mockResolvedValue({ token: "claude-oauth-alice" }), + }; + const identityLinkGateway: IdentityLinkPort = { + start: vi.fn(), + poll: vi.fn(), + getToken: vi.fn().mockResolvedValue({ token: "gho_should-not-be-used" }), + }; + const vectorStore: VectorStore = { + upsert: vi.fn(), + delete: vi.fn(), + query: vi.fn().mockResolvedValue([]), + getByIds: vi.fn().mockResolvedValue([{ tool: claudeTool, score: 1 }]), + }; + const skillStore: SkillStore = { + upsert: vi.fn(), + delete: vi.fn(), + query: vi.fn().mockResolvedValue([{ skill: claudeToolSkill, score: 0.9 }]), + getByIds: vi.fn().mockResolvedValue([claudeToolSkill]), + }; + const deps = githubToolDeps({ skillStore, vectorStore, claudeAuthGateway, identityLinkGateway }); + const graph = buildAgentGraph(deps); + + const final = await graph.invoke({ request: "run a claude command", authToken: "tok" }); + + expect(final.error).toBeUndefined(); + expect(claudeAuthGateway.getToken).toHaveBeenCalledWith("claude", "alice"); + // The GitHub-only gateway may be consulted for principal establishment, but + // must never be asked to resolve the "claude" provider's credential. + expect(identityLinkGateway.getToken).not.toHaveBeenCalledWith("claude", expect.anything()); + // Definitive proof of correct routing: the launched token is the one from + // claudeAuthGateway, not identityLinkGateway's "gho_should-not-be-used". + expect(deps.containerToolLauncher.launch).toHaveBeenCalledWith( + claudeTool.jobTemplate, + expect.objectContaining({ secretEnv: [{ name: "CLAUDE_CODE_OAUTH_TOKEN", value: "claude-oauth-alice" }] }), + ); + }); +}); + describe("buildAgentGraph Skill.agentRefs (ADR 0021, no Tool wrapper)", () => { const opencodeAgent: AgentDescriptor = { id: "opencode-swe-agent", diff --git a/apps/agent-orchestrator/src/agent/graph.ts b/apps/agent-orchestrator/src/agent/graph.ts index 148efc8..2517398 100644 --- a/apps/agent-orchestrator/src/agent/graph.ts +++ b/apps/agent-orchestrator/src/agent/graph.ts @@ -29,6 +29,7 @@ import { makeSubAgentToolCallHandler, type ToolCatalog } from "./dispatch-tool.j import { ACTOR_LOGIN_ENV, AuthorizationService, + type CredentialEnvEntry, CROSS_ENTRY_POINT_PROVIDERS, linkPromptText, PROVIDER_LABEL, @@ -625,6 +626,54 @@ function identityGatewayFor(provider: string, deps: AgentGraphDeps): IdentityLin return deps.identityLinkGateway; } +/** + * The per-caller identity gate shared by `runTool`'s two tool-launch branches + * (agent-backed and container). Resolves the caller's linked credentials for + * `tool.identityProviders` through the SAME {@link AuthorizationService} the + * peer-level `delegateToAgent` path uses (ADR 0022/0030/0032), so gateway + * selection is provider-aware (e.g. `claude` -> claudeAuthGateway, not the + * GitHub-only `identityLinkGateway`) and the credential subject is keyed + * identically (canonical principal for cross-entry-point providers). + * + * Returns `{ secretEnv }` (possibly `undefined`, when the tool declares no + * providers) on success, or `{ error }` on any gate failure — the four + * `resolveLinkedCredentials` failure kinds mapped to a message that names the + * TOOL (which the service deliberately doesn't know). Like the paths it + * replaces, this never STARTS a fresh device-flow/authcode link: there is no + * session slot analogous to `pendingIdentityLink` for a paused TOOL call, only + * for a paused agent delegation (a documented v1 scope cut). A caller must have + * linked once via direct chat delegation first; `linkHint(provider)` completes + * the not-linked message with who to talk to to do so. + */ +async function resolveToolIdentitySecretEnv( + authorization: AuthorizationService, + tool: { id: string; identityProviders?: string[] }, + identity: Identity | undefined, + linkHint: (provider: string) => string, +): Promise<{ secretEnv?: CredentialEnvEntry[] } | { error: string }> { + if (!identity) { + return { error: `tool ${tool.id} requires identity providers but no caller identity was resolved` }; + } + const credentials = await authorization.resolveLinkedCredentials({ + identity, + identityProviders: tool.identityProviders, + }); + switch (credentials.kind) { + case "gateway-missing": + return { + error: `tool ${tool.id} requires identity providers (${tool.identityProviders!.join(", ")}) but no identity-link gateway is configured for "${credentials.provider}"`, + }; + case "not-linked": + return { + error: `tool ${tool.id} requires linking your ${credentials.provider} account first -- start a direct conversation with ${linkHint(credentials.provider)} to link it, then retry`, + }; + case "unsupported-provider": + return { error: `tool ${tool.id} declares unsupported identity provider "${credentials.provider}"` }; + case "resolved": + return { secretEnv: credentials.secretEnv }; + } +} + /** * Caps how many tool calls a skill's planAction<->runTool loop may chain in a * single turn (docs/adr/0008 update: multi-step tool use) -- generous enough @@ -1477,35 +1526,15 @@ export function buildAgentGraph(deps: AgentGraphDeps) { // required here too now that an identity-gated Agent's static secretEnv // is stripped regardless of which path reaches it -- and the SAME owner // (docs/adr/0030 §1), so the keying can never drift between the two - // paths. - // - // The read-only entry point, deliberately: v1 scope cut is that unlike - // delegateToAgent, this path never STARTS a fresh device-flow/authcode - // link -- there's no session slot analogous to pendingIdentityLink for a - // paused TOOL call, only for a paused agent delegation. A caller must - // link once via direct chat delegation to the same agent before a Skill - // can reach it here. - if (!state.identity) { - return { error: `tool ${tool.id} requires identity providers but no caller identity was resolved` }; + // paths. Shared with the container-tool branch below via + // `resolveToolIdentitySecretEnv`, which documents the read-only, + // never-starts-a-fresh-link v1 scope cut. The not-linked hint points at + // THIS backing agent, which the caller reaches by direct chat. + const gate = await resolveToolIdentitySecretEnv(authorization, tool, state.identity, () => "this agent"); + if ("error" in gate) { + return { error: gate.error }; } - const credentials = await authorization.resolveLinkedCredentials({ - identity: state.identity, - identityProviders: tool.identityProviders, - }); - if (credentials.kind === "gateway-missing") { - return { - error: `tool ${tool.id} requires identity providers (${tool.identityProviders!.join(", ")}) but no identity-link gateway is configured for "${credentials.provider}"`, - }; - } - if (credentials.kind === "not-linked") { - return { - error: `tool ${tool.id} requires linking your ${credentials.provider} account first -- start a direct conversation with this agent to link it, then retry`, - }; - } - if (credentials.kind === "unsupported-provider") { - return { error: `tool ${tool.id} declares unsupported identity provider "${credentials.provider}"` }; - } - const identitySecretEnv = credentials.secretEnv; + const identitySecretEnv = gate.secretEnv; const runId = randomUUID(); const callbackUrl = `${deps.callbackBaseUrl}/callback/${randomUUID()}`; try { @@ -1566,6 +1595,31 @@ export function buildAgentGraph(deps: AgentGraphDeps) { // Container tool (ADR 0010): create a ToolRun CR — the Go // core-controller reconciles it into a hardened Job. The orchestrator // itself never creates a Job. + + // Same per-caller identity gate as delegateToAgent/the agent-backed + // tool branch above (ADR 0022/0032) — e.g. the `github` Tool, which + // needs the calling user's own linked GitHub token rather than a + // shared credential. Shares `resolveToolIdentitySecretEnv` with the + // agent-backed branch: provider-aware gateway selection and identical + // subject keying, never hard-coded to `deps.identityLinkGateway`. A + // container Tool has no backing agent to chat, so the not-linked hint + // names any agent that links the same provider. Unlike the agent-backed + // branch, a container Tool with no declared providers needs no identity + // at all, so the gate is skipped entirely rather than run for nothing. + let identitySecretEnv: CredentialEnvEntry[] | undefined; + if (tool.identityProviders && tool.identityProviders.length > 0) { + const gate = await resolveToolIdentitySecretEnv( + authorization, + tool, + state.identity, + (provider) => `an agent that uses ${provider} identity linking`, + ); + if ("error" in gate) { + return { error: gate.error }; + } + identitySecretEnv = gate.secretEnv; + } + jobId = randomUUID(); const awaitResult = deps.jobResultReceiver.awaitJob(jobId); @@ -1584,6 +1638,7 @@ export function buildAgentGraph(deps: AgentGraphDeps) { natsUrl: deps.natsUrl, natsSubject: `callbacks.${jobId}`, ...(state.sessionId ? { sessionId: state.sessionId } : {}), + ...(identitySecretEnv ? { secretEnv: identitySecretEnv } : {}), }); } else { // HTTP callback mode (backward-compatible default). @@ -1593,6 +1648,7 @@ export function buildAgentGraph(deps: AgentGraphDeps) { callbackUrl, callbackSecret: deps.callbackSecret!, ...(state.sessionId ? { sessionId: state.sessionId } : {}), + ...(identitySecretEnv ? { secretEnv: identitySecretEnv } : {}), }); } diff --git a/apps/agent-orchestrator/src/k8s/agentrun-launcher.ts b/apps/agent-orchestrator/src/k8s/agentrun-launcher.ts index 274e51c..e5070c1 100644 --- a/apps/agent-orchestrator/src/k8s/agentrun-launcher.ts +++ b/apps/agent-orchestrator/src/k8s/agentrun-launcher.ts @@ -74,7 +74,9 @@ export interface AgentRunLauncherPort { * identity `secretEnv` (a caller's own linked GitHub token, never the shared * static credential) — kept small and mockable, same narrowing discipline as * {@link CustomObjectsApiLike}. Optional on the constructor: only required - * when a caller ever launches with `options.secretEnv` non-empty. + * when a caller ever launches with `options.secretEnv` non-empty. Shared + * with {@link ../k8s/toolrun-launcher.ts!ToolRunLauncher}, which backs the + * same per-invocation `secretEnv` mechanism for container Tools (ADR 0032). */ export interface SecretApiLike { createNamespacedSecret(request: { namespace: string; body: unknown }): Promise<{ metadata?: { name?: string } }>; diff --git a/apps/agent-orchestrator/src/k8s/container-tool-launcher.ts b/apps/agent-orchestrator/src/k8s/container-tool-launcher.ts index ad8ffef..d5dc793 100644 --- a/apps/agent-orchestrator/src/k8s/container-tool-launcher.ts +++ b/apps/agent-orchestrator/src/k8s/container-tool-launcher.ts @@ -34,6 +34,18 @@ export interface LaunchOptions { * existed. */ sessionId?: string; + /** + * Per-invocation plaintext values (e.g. the calling user's own linked + * GitHub token, ADR 0022/0027) that must reach the launched Job as env + * vars WITHOUT ever being embedded as plaintext in the ToolRun CR itself + * -- CRs aren't RBAC-hidden the way k8s Secrets are. When non-empty, + * `ToolRunLauncher.launch()` creates a dedicated k8s Secret first and + * references it via `ToolRunSpec.secretEnv` (`SecretEnvVar`, + * controllers/core-controller/api/v1alpha1/toolrun_types.go), which the Go + * reconciler merges on top of the Tool template's own static `secretEnv` + * when building the Job. Mirrors `AgentLaunchOptions.secretEnv` exactly. + */ + secretEnv?: { name: string; value: string }[]; } /** Well-known annotation key mirroring `SessionIDAnnotation` in controllers/core-controller/internal/controller/run_job.go. */ diff --git a/apps/agent-orchestrator/src/k8s/toolrun-launcher.test.ts b/apps/agent-orchestrator/src/k8s/toolrun-launcher.test.ts index 29b9852..d7e7fcc 100644 --- a/apps/agent-orchestrator/src/k8s/toolrun-launcher.test.ts +++ b/apps/agent-orchestrator/src/k8s/toolrun-launcher.test.ts @@ -86,4 +86,91 @@ describe("ToolRunLauncher", () => { ), ).rejects.toThrow(/toolRef/); }); + + describe("per-invocation identity secretEnv (ADR 0032)", () => { + it("launch() without options.secretEnv never touches the Secret API", async () => { + const createNamespacedCustomObject = vi.fn().mockResolvedValue({ metadata: { uid: "uid-1" } }); + const api = { listNamespacedCustomObject: vi.fn(), createNamespacedCustomObject }; + const createNamespacedSecret = vi.fn(); + const patchNamespacedSecret = vi.fn(); + const launcher = new ToolRunLauncher( + "core.controller-agent.dev", + "v1alpha1", + { name: "s", key: "k" }, + api, + { createNamespacedSecret, patchNamespacedSecret }, + ); + + await launcher.launch(template, { args: ["issue view 86"], callbackUrl: "http://x", callbackSecret: "s" }); + + expect(createNamespacedSecret).not.toHaveBeenCalled(); + expect(patchNamespacedSecret).not.toHaveBeenCalled(); + const [request] = createNamespacedCustomObject.mock.calls[0] as [{ body: { spec: Record } }]; + expect(request.body.spec.secretEnv).toBeUndefined(); + }); + + it("launch() with options.secretEnv creates a Secret, references it from the ToolRun spec, and patches ownerReferences using the created ToolRun's uid", async () => { + const createNamespacedCustomObject = vi.fn().mockResolvedValue({ metadata: { uid: "toolrun-uid-123" } }); + const api = { listNamespacedCustomObject: vi.fn(), createNamespacedCustomObject }; + const createNamespacedSecret = vi.fn().mockResolvedValue({}); + const patchNamespacedSecret = vi.fn().mockResolvedValue({}); + const launcher = new ToolRunLauncher( + "core.controller-agent.dev", + "v1alpha1", + { name: "s", key: "k" }, + api, + { createNamespacedSecret, patchNamespacedSecret }, + ); + + const launched = await launcher.launch( + { image: "example.com/github:latest", namespace: "default", serviceAccountName: "github-tool", toolRef: "github" }, + { + args: ["issue comment 86 --body hi"], + callbackUrl: "http://x", + callbackSecret: "s", + secretEnv: [{ name: "GITHUB_TOKEN", value: "gho_super-secret-value" }], + }, + ); + + expect(launched.namespace).toBe("default"); + + expect(createNamespacedSecret).toHaveBeenCalledTimes(1); + const [secretRequest] = createNamespacedSecret.mock.calls[0] as [ + { namespace: string; body: { metadata: { name: string }; stringData: Record } }, + ]; + expect(secretRequest.namespace).toBe("default"); + expect(secretRequest.body.metadata.name).toBe(`${launched.name}-identity`); + expect(secretRequest.body.stringData).toEqual({ GITHUB_TOKEN: "gho_super-secret-value" }); + + const [crRequest] = createNamespacedCustomObject.mock.calls[0] as [{ body: { spec: Record } }]; + expect(crRequest.body.spec.secretEnv).toEqual([ + { name: "GITHUB_TOKEN", secretRef: { name: `${launched.name}-identity`, key: "GITHUB_TOKEN" } }, + ]); + // The plaintext token must never appear in the ToolRun CR body. + expect(JSON.stringify(crRequest.body)).not.toContain("gho_super-secret-value"); + + expect(patchNamespacedSecret).toHaveBeenCalledTimes(1); + const [patchRequest] = patchNamespacedSecret.mock.calls[0] as [{ name: string; namespace: string; body: unknown }]; + expect(patchRequest.name).toBe(`${launched.name}-identity`); + expect(patchRequest.namespace).toBe("default"); + expect(JSON.stringify(patchRequest.body)).toContain("toolrun-uid-123"); + expect(JSON.stringify(patchRequest.body)).toContain(launched.name); + }); + + it("launch() throws if options.secretEnv is non-empty but no SecretApiLike was configured", async () => { + const createNamespacedCustomObject = vi.fn().mockResolvedValue({}); + const api = { listNamespacedCustomObject: vi.fn(), createNamespacedCustomObject }; + const launcher = new ToolRunLauncher("core.controller-agent.dev", "v1alpha1", { name: "s", key: "k" }, api); + + await expect( + launcher.launch(template, { + args: ["issue view 86"], + callbackUrl: "http://x", + callbackSecret: "s", + secretEnv: [{ name: "GITHUB_TOKEN", value: "gho_x" }], + }), + ).rejects.toThrow(/SecretApiLike/); + expect(createNamespacedCustomObject).not.toHaveBeenCalled(); + }); + }); }); diff --git a/apps/agent-orchestrator/src/k8s/toolrun-launcher.ts b/apps/agent-orchestrator/src/k8s/toolrun-launcher.ts index 1ec1279..f6ee361 100644 --- a/apps/agent-orchestrator/src/k8s/toolrun-launcher.ts +++ b/apps/agent-orchestrator/src/k8s/toolrun-launcher.ts @@ -2,6 +2,7 @@ import { randomUUID } from "node:crypto"; import * as k8s from "@kubernetes/client-node"; import type { CustomObjectsApiLike } from "../registry/crd-tool-registry.js"; import type { JobTemplate } from "../tool-descriptor.js"; +import type { SecretApiLike } from "./agentrun-launcher.js"; import { SESSION_ID_ANNOTATION, type ContainerToolLauncher, type LaunchedJob, type LaunchOptions } from "./container-tool-launcher.js"; /** Plural resource name used by the `ToolRun` CRD (matches `config/crd/bases` in controllers/core-controller). */ @@ -40,8 +41,10 @@ export class ToolRunLauncher implements ContainerToolLauncher { namespace: string; plural: string; body: unknown; - }): Promise; + }): Promise<{ metadata?: { uid?: string } }>; }, + /** Absent in callers/tests that never launch with `options.secretEnv` (today's default). Real instances constructed via `fromKubeConfig` always pass one. */ + private readonly secretApi?: SecretApiLike, ) {} static fromKubeConfig( @@ -50,7 +53,13 @@ export class ToolRunLauncher implements ContainerToolLauncher { callbackSecretRef: SecretKeySelector, kubeConfig: k8s.KubeConfig, ): ToolRunLauncher { - return new ToolRunLauncher(group, version, callbackSecretRef, kubeConfig.makeApiClient(k8s.CustomObjectsApi)); + return new ToolRunLauncher( + group, + version, + callbackSecretRef, + kubeConfig.makeApiClient(k8s.CustomObjectsApi), + kubeConfig.makeApiClient(k8s.CoreV1Api), + ); } async launch(template: JobTemplate, options: LaunchOptions): Promise { @@ -75,6 +84,38 @@ export class ToolRunLauncher implements ContainerToolLauncher { ? { natsSubject: options.natsSubject, natsUrl: options.natsUrl } : { url: options.callbackUrl, secretRef: this.callbackSecretRef }; + // Per-invocation identity secretEnv (e.g. GITHUB_TOKEN for the CALLING + // user, not any shared bot credential, ADR 0032): create a dedicated k8s + // Secret up front and reference it from the CR by name/key only -- the + // plaintext value must never be embedded in the ToolRun CR itself, since + // CRs aren't RBAC-hidden the way Secrets are. Mirrors AgentRunLauncher's + // identical mechanism (agentrun-launcher.ts). + let secretName: string | undefined; + let secretEnvSpec: { name: string; secretRef: { name: string; key: string } }[] | undefined; + if (options.secretEnv && options.secretEnv.length > 0) { + if (!this.secretApi) { + throw new Error( + "ToolRunLauncher.launch() was given options.secretEnv but no SecretApiLike was configured -- " + + "construct via fromKubeConfig (which wires a CoreV1Api client) to use per-invocation identity secretEnv", + ); + } + // `name` is a randomUUID() above, so this suffix is still a valid + // DNS-1123 Secret name. + secretName = `${name}-identity`; + const stringData: Record = {}; + for (const entry of options.secretEnv) stringData[entry.name] = entry.value; + // No ownerReference yet -- the ToolRun CR doesn't exist (no uid) until + // createNamespacedCustomObject below succeeds; patched in afterward. + await this.secretApi.createNamespacedSecret({ + namespace: template.namespace, + body: { metadata: { name: secretName }, stringData }, + }); + secretEnvSpec = options.secretEnv.map((entry) => ({ + name: entry.name, + secretRef: { name: secretName!, key: entry.name }, + })); + } + const body = { apiVersion: `${this.group}/${this.version}`, kind: "ToolRun", @@ -87,10 +128,11 @@ export class ToolRunLauncher implements ContainerToolLauncher { toolRef: template.toolRef, args: options.args ?? template.args, callback, + ...(secretEnvSpec ? { secretEnv: secretEnvSpec } : {}), }, }; - await this.api.createNamespacedCustomObject({ + const created = await this.api.createNamespacedCustomObject({ group: this.group, version: this.version, namespace: template.namespace, @@ -98,6 +140,38 @@ export class ToolRunLauncher implements ContainerToolLauncher { body, }); + if (secretName) { + const uid = created?.metadata?.uid; + // A real cluster always returns the created object's uid; skip the + // ownerReference patch rather than fail the whole launch if it's ever + // missing (e.g. a bare-bones test double) -- the Secret is still + // created and correctly referenced, just without GC-on-delete. + if (uid) { + await this.secretApi!.patchNamespacedSecret({ + name: secretName, + namespace: template.namespace, + // Same JSON-Patch media-type quirk as AgentRunLauncher -- see its + // comment on the equivalent patch call. + body: [ + { + op: "add", + path: "/metadata/ownerReferences", + value: [ + { + apiVersion: `${this.group}/${this.version}`, + kind: "ToolRun", + name, + uid, + controller: true, + blockOwnerDeletion: true, + }, + ], + }, + ], + }); + } + } + return { name, namespace: template.namespace }; } } diff --git a/apps/agent-orchestrator/src/registry/crd-tool-registry.test.ts b/apps/agent-orchestrator/src/registry/crd-tool-registry.test.ts index a69d6a5..af49465 100644 --- a/apps/agent-orchestrator/src/registry/crd-tool-registry.test.ts +++ b/apps/agent-orchestrator/src/registry/crd-tool-registry.test.ts @@ -93,6 +93,39 @@ describe("CrdToolRegistry", () => { expect(tools[0].jobTemplate).toBeUndefined(); }); + it("carries Tool.spec.identityProviders through to the ToolDescriptor for a container Tool (ADR 0032, e.g. the github Tool)", async () => { + const identityLinked: ToolCustomResource = { + metadata: { name: "github" }, + spec: { + description: "Runs a gh CLI command against GitHub", + input: "a gh CLI command line", + output: "gh's own output", + allowedRoles: ["writer"], + image: "example.com/github:latest", + serviceAccountName: "github-tool", + identityProviders: ["github"], + }, + }; + const listNamespacedCustomObject = vi.fn().mockResolvedValue({ items: [identityLinked] }); + const api: CustomObjectsApiLike = { listNamespacedCustomObject }; + const registry = new CrdToolRegistry("default", "core.controller-agent.dev", "v1alpha1", api); + + const tools = await registry.listAll(); + + expect(tools).toHaveLength(1); + expect(tools[0]!.identityProviders).toEqual(["github"]); + }); + + it("omits identityProviders when the Tool CR does not declare any", async () => { + const listNamespacedCustomObject = vi.fn().mockResolvedValue({ items: [validTool] }); + const api: CustomObjectsApiLike = { listNamespacedCustomObject }; + const registry = new CrdToolRegistry("default", "core.controller-agent.dev", "v1alpha1", api); + + const tools = await registry.listAll(); + + expect(tools[0]!.identityProviders).toBeUndefined(); + }); + it("returns an empty catalog when there are zero Tool resources", async () => { const listNamespacedCustomObject = vi.fn().mockResolvedValue({ items: [] }); const api: CustomObjectsApiLike = { listNamespacedCustomObject }; diff --git a/apps/agent-orchestrator/src/registry/crd-tool-registry.ts b/apps/agent-orchestrator/src/registry/crd-tool-registry.ts index 4053796..c9d7a53 100644 --- a/apps/agent-orchestrator/src/registry/crd-tool-registry.ts +++ b/apps/agent-orchestrator/src/registry/crd-tool-registry.ts @@ -23,6 +23,14 @@ export interface ToolCustomResource { requests?: Record; limits?: Record; }; + /** + * External identity providers the CALLING user must have linked (ADR + * 0022/0027) before this container Tool can be launched -- read straight + * onto the resulting `ToolDescriptor.identityProviders` (see + * `graph.ts`'s `runTool`, which gates on it the same way `delegateToAgent` + * gates an identity-linked Agent). + */ + identityProviders?: string[]; }; } @@ -155,5 +163,8 @@ export function toToolDescriptor(cr: ToolCustomResource, namespace: string): Too // re-embedding image/serviceAccount into a Job itself (ADR 0010). toolRef: name, }, + ...(spec.identityProviders && spec.identityProviders.length > 0 + ? { identityProviders: spec.identityProviders } + : {}), }; } diff --git a/apps/agent-orchestrator/src/tool-descriptor.ts b/apps/agent-orchestrator/src/tool-descriptor.ts index 7ab7fc8..c0deec8 100644 --- a/apps/agent-orchestrator/src/tool-descriptor.ts +++ b/apps/agent-orchestrator/src/tool-descriptor.ts @@ -104,11 +104,13 @@ export interface ToolDescriptor { agentRunTemplate?: AgentRunTemplate; /** * External identity providers the CALLING user must have linked (ADR - * 0022) before this agent-backed tool can be launched — carried over - * from `AgentDescriptor.identityProviders` when a Skill's `agentRefs` - * resolves an Agent into a ToolDescriptor (`loadSkillTools`). Absent for - * container/LocalTools and for agent-backed tools with no identity - * requirement. + * 0022/0027) before this tool can be launched. For an agent-backed tool, + * carried over from `AgentDescriptor.identityProviders` when a Skill's + * `agentRefs` resolves an Agent into a ToolDescriptor (`loadSkillTools`). + * For a container Tool, populated directly from `Tool.spec.identityProviders` + * (`CrdToolRegistry`) -- e.g. the `github` Tool, which needs the calling + * user's own linked GitHub token rather than a shared credential. Absent + * for LocalTools and for tools with no identity requirement. */ identityProviders?: string[]; /** Optional coarse risk/cost tier, for future quota/authorization use. */ diff --git a/charts/community-components/templates/serviceaccount-github.yaml b/charts/community-components/templates/serviceaccount-github.yaml new file mode 100644 index 0000000..315df87 --- /dev/null +++ b/charts/community-components/templates/serviceaccount-github.yaml @@ -0,0 +1,18 @@ +{{- if and .Values.githubTool.enabled .Values.githubTool.serviceAccount.create }} +apiVersion: v1 +kind: ServiceAccount +metadata: + name: {{ .Values.githubTool.serviceAccountName }} + namespace: {{ .Release.Namespace }} + labels: + {{- include "tools.labels" . | nindent 4 }} + {{- with .Values.githubTool.serviceAccount.annotations }} + annotations: + {{- toYaml . | nindent 4 }} + {{- end }} +{{- with .Values.imagePullSecrets }} +imagePullSecrets: + {{- toYaml . | nindent 2 }} +{{- end }} +automountServiceAccountToken: {{ .Values.githubTool.serviceAccount.automount }} +{{- end }} diff --git a/charts/community-components/templates/tool-github.yaml b/charts/community-components/templates/tool-github.yaml new file mode 100644 index 0000000..8ec9267 --- /dev/null +++ b/charts/community-components/templates/tool-github.yaml @@ -0,0 +1,47 @@ +{{- if .Values.githubTool.enabled }} +apiVersion: {{ .Values.crdApiVersion }} +kind: Tool +metadata: + name: github + labels: + {{- include "tools.labels" . | nindent 4 }} +spec: + description: >- + Runs a single GitHub CLI (gh) command -- view/list/comment/create/edit on + issues and pull requests, view repos and releases, search GitHub, merge + or review a PR, inspect Actions workflows/runs -- authenticated as the + CALLING user's own linked GitHub identity, not a shared bot credential. + Use this for quick, targeted GitHub operations; for anything requiring + cloning a repo, reading/writing files, or opening a pull request from + code changes, delegate to a coding agent instead. + input: >- + A single gh CLI command line, everything after "gh" (e.g. + "issue view 86 --repo imaustink/agent-controller --json title,body", + "issue comment 86 --repo imaustink/agent-controller --body "thanks!"", + "pr list --repo imaustink/agent-controller --state open", + "search issues "is:open label:bug" --repo imaustink/agent-controller"). + Only a fixed allowlist of commands/subcommands is accepted (see + tools/github/README.md); anything else is rejected before it reaches + GitHub. + output: >- + gh's own stdout (JSON when --json was requested, otherwise gh's normal + text output), wrapped in a fenced code block. + allowedRoles: + - writer + tier: standard + {{- if .Values.githubTool.identityLink.enabled }} + identityProviders: + {{- range .Values.githubTool.identityLink.providers }} + - {{ . }} + {{- end }} + {{- end }} + image: {{ .Values.githubTool.image | quote }} + serviceAccountName: {{ .Values.githubTool.serviceAccountName | quote }} + {{- if not .Values.githubTool.identityLink.enabled }} + secretEnv: + - name: GITHUB_TOKEN + secretRef: + name: {{ .Values.githubTool.secretName | quote }} + key: {{ .Values.githubTool.secretKey | quote }} + {{- end }} +{{- end }} diff --git a/charts/community-components/values-ci-all.yaml b/charts/community-components/values-ci-all.yaml index f1cfcd2..33182b1 100644 --- a/charts/community-components/values-ci-all.yaml +++ b/charts/community-components/values-ci-all.yaml @@ -43,6 +43,16 @@ recipePublisher: opencodeSweAgent: enabled: true +# Container Tool with per-user GitHub identity delegation (ADR 0025). Enabled +# with identityLink so the CR's identityProviders path renders and is validated; +# the ServiceAccount template also requires enabled: true to render. +githubTool: + enabled: true + identityLink: + enabled: true + providers: + - github + # The Agent production runs, and previously the one the "full catalog" # validation never touched. Its template hard-`fail`s unless some model # credential is guaranteed at launch time, so identityLink must cover `claude`. diff --git a/charts/community-components/values-production.yaml b/charts/community-components/values-production.yaml index 47eb3e8..e340fc9 100644 --- a/charts/community-components/values-production.yaml +++ b/charts/community-components/values-production.yaml @@ -172,6 +172,24 @@ claudeCodeSweAgent: remoteControl: enabled: true +# github: single allowlisted gh CLI command tool (issue/pr/repo/release/ +# search/workflow/run), authenticated as the calling user's own linked +# GitHub identity (ADR 0022/0032) -- reuses the exact same identity-link +# plumbing already enabled for opencodeSweAgent above (agent-orchestrator's +# and integration-gateway's identityLink.enabled, see +# charts/agent-controller/values-production.yaml), no new gateway/App +# configuration needed. Lighter-weight than delegating to opencode-swe-agent +# for a quick, targeted GitHub read/write that doesn't need a clone or code +# change. +githubTool: + enabled: true + image: registry.kurpuis.com:5000/github:latest + serviceAccountName: github-tool + identityLink: + enabled: true + providers: + - github + skills: recipeRefining: enabled: true diff --git a/charts/community-components/values.yaml b/charts/community-components/values.yaml index b203510..eef0ad5 100644 --- a/charts/community-components/values.yaml +++ b/charts/community-components/values.yaml @@ -271,6 +271,50 @@ claudeCodeSweAgent: credentialsSecretName: "" credentialsSecretKey: "" +# github: runs a single allowlisted gh CLI command (issue/pr/repo/release/ +# search/workflow/run), preferably authenticated as the calling user's own +# linked GitHub identity rather than a shared bot credential (ADR 0022/0032). +# +# Two mutually-exclusive ways to give it a GitHub credential: +# 1. GITHUB_TOKEN PAT (default) -- prerequisite (out-of-band -- Helm can't +# source secret values on its own): +# kubectl -n create secret generic github-tool-secrets \ +# --from-literal=GITHUB_TOKEN=github_pat_... +# 2. Per-user identity linking (identityLink block below) -- no static +# GitHub credential is baked into this Tool's template at all; +# agent-orchestrator resolves and injects the CALLING USER's own linked +# GitHub token per-invocation instead, exactly like opencodeSweAgent's +# identityLink option. Requires agent-orchestrator's own +# identityLink.enabled=true and a configured integration-gateway (see +# those charts' values.yaml), AND core-controller/agent-orchestrator +# built with ToolRun-level secretEnv support (ADR 0032). +githubTool: + enabled: false + image: github:latest + serviceAccountName: github-tool + # Creates the ServiceAccount named above. Set to false and pre-create it + # yourself if you need annotations managed elsewhere (e.g. IRSA) that this + # chart doesn't own. + serviceAccount: + create: true + annotations: {} + automount: true + # Secret (must already exist) supplying the tool's GITHUB_TOKEN. Ignored + # when identityLink.enabled below. + secretName: github-tool-secrets + secretKey: GITHUB_TOKEN + # Per-user GitHub identity linking (docs/adr/0022, extended to container + # Tools by ADR 0032): when enabled, no static GITHUB_TOKEN secretEnv is set + # on the Tool CR at all -- agent-orchestrator resolves the calling user's + # own linked token per-invocation and injects it via ToolRunSpec.secretEnv + # instead. Requires agent-orchestrator's own identityLink.enabled=true and + # a configured integration-gateway. Leave disabled (default) for the + # shared-credential (GITHUB_TOKEN PAT) behavior unchanged. + identityLink: + enabled: false + providers: + - github + skills: # recipe-refining: extract -> confirm -> publish -> refine (recipe-scraper + # recipe-publisher). Enable both those tools above for its derived audience diff --git a/controllers/core-controller/api/v1alpha1/tool_types.go b/controllers/core-controller/api/v1alpha1/tool_types.go index 659f3c2..8e15d64 100644 --- a/controllers/core-controller/api/v1alpha1/tool_types.go +++ b/controllers/core-controller/api/v1alpha1/tool_types.go @@ -141,6 +141,19 @@ type ToolSpec struct { // +optional // +kubebuilder:validation:Minimum=1 TimeoutSeconds int32 `json:"timeoutSeconds,omitempty"` + + // identityProviders declares which external identity providers (e.g. + // "github") must be linked for the calling user before this Tool can be + // launched (ADR 0022/0027). Only meaningful for a container Tool + // (image/serviceAccountName) -- an agent-backed Tool (agentRef) carries + // this instead on the wrapped Agent CR. This controller does not consume + // the field itself -- it is read by the agent-orchestrator when deciding + // whether/how to launch a ToolRun for a given user (resolving and + // injecting a per-user token via ToolRunSpec.SecretEnv) -- but it belongs + // on the CRD as the source of truth an operator deploys alongside the + // rest of the Tool catalog entry. + // +optional + IdentityProviders []string `json:"identityProviders,omitempty"` } // ToolStatus defines the observed state of Tool. diff --git a/controllers/core-controller/api/v1alpha1/toolrun_types.go b/controllers/core-controller/api/v1alpha1/toolrun_types.go index 01732d2..f3a4f4e 100644 --- a/controllers/core-controller/api/v1alpha1/toolrun_types.go +++ b/controllers/core-controller/api/v1alpha1/toolrun_types.go @@ -90,6 +90,17 @@ type ToolRunSpec struct { // +optional // +kubebuilder:validation:Minimum=1 TimeoutSeconds int32 `json:"timeoutSeconds,omitempty"` + + // secretEnv are per-invocation environment variables sourced from Secret + // keys (same namespace), merged over the referenced Tool's static + // ToolSpec.SecretEnv at Job-build time. An entry here with the same + // `name` as a Tool-level entry wins for this run only; entries unique to + // either side are both included. Mirrors AgentRunSpec.SecretEnv (ADR + // 0022/0027) -- intended for short-lived, caller-scoped credentials + // (e.g. a per-user GitHub identity-link token, see the `github` Tool) + // that must not be baked into the Tool template. + // +optional + SecretEnv []SecretEnvVar `json:"secretEnv,omitempty"` } // ToolRunPhase is the coarse lifecycle state of a ToolRun, mirrored from its diff --git a/controllers/core-controller/api/v1alpha1/zz_generated.deepcopy.go b/controllers/core-controller/api/v1alpha1/zz_generated.deepcopy.go index bc40ffa..8a1702c 100644 --- a/controllers/core-controller/api/v1alpha1/zz_generated.deepcopy.go +++ b/controllers/core-controller/api/v1alpha1/zz_generated.deepcopy.go @@ -852,6 +852,11 @@ func (in *ToolRunSpec) DeepCopyInto(out *ToolRunSpec) { copy(*out, *in) } out.Callback = in.Callback + if in.SecretEnv != nil { + in, out := &in.SecretEnv, &out.SecretEnv + *out = make([]SecretEnvVar, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ToolRunSpec. @@ -918,6 +923,11 @@ func (in *ToolSpec) DeepCopyInto(out *ToolSpec) { copy(*out, *in) } in.Resources.DeepCopyInto(&out.Resources) + if in.IdentityProviders != nil { + in, out := &in.IdentityProviders, &out.IdentityProviders + *out = make([]string, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ToolSpec. diff --git a/controllers/core-controller/config/crd/bases/core.controller-agent.dev_toolruns.yaml b/controllers/core-controller/config/crd/bases/core.controller-agent.dev_toolruns.yaml index 32caab6..5ff2473 100644 --- a/controllers/core-controller/config/crd/bases/core.controller-agent.dev_toolruns.yaml +++ b/controllers/core-controller/config/crd/bases/core.controller-agent.dev_toolruns.yaml @@ -96,6 +96,50 @@ spec: Required when natsSubject is empty. type: string type: object + secretEnv: + description: |- + secretEnv are per-invocation environment variables sourced from Secret + keys (same namespace), merged over the referenced Tool's static + ToolSpec.SecretEnv at Job-build time. An entry here with the same + `name` as a Tool-level entry wins for this run only; entries unique to + either side are both included. Mirrors AgentRunSpec.SecretEnv (ADR + 0022/0027) -- intended for short-lived, caller-scoped credentials + (e.g. a per-user GitHub identity-link token, see the `github` Tool) + that must not be baked into the Tool template. + items: + description: |- + SecretEnvVar names a Job container environment variable whose value comes + from a Secret key in the SAME namespace as the Tool/ToolRun, resolved via + corev1.EnvVarSource.SecretKeyRef at Job-build time. Only the reference + (secret name + key) lives in the Tool spec — never the secret value + itself, same discipline as ToolRunCallback.SecretRef for the callback + HMAC secret. Needed because ordinary tool containers (e.g. recipe-scraper's + OPENAI_API_KEY, recipe-publisher's GITHUB_TOKEN) require real secrets that + `env` (non-secret, catalog metadata) cannot carry. + properties: + name: + description: name of the environment variable to set in the + tool's Job container. + type: string + secretRef: + description: secretRef selects the Secret key providing the + value. + properties: + key: + description: key within the Secret's data. + type: string + name: + description: name of the Secret. + type: string + required: + - key + - name + type: object + required: + - name + - secretRef + type: object + type: array timeoutSeconds: description: timeoutSeconds bounds the Job's activeDeadlineSeconds. Defaults to 300 if unset. diff --git a/controllers/core-controller/config/crd/bases/core.controller-agent.dev_tools.yaml b/controllers/core-controller/config/crd/bases/core.controller-agent.dev_tools.yaml index bdc970e..4d8ec3c 100644 --- a/controllers/core-controller/config/crd/bases/core.controller-agent.dev_tools.yaml +++ b/controllers/core-controller/config/crd/bases/core.controller-agent.dev_tools.yaml @@ -85,6 +85,21 @@ spec: - value type: object type: array + identityProviders: + description: |- + identityProviders declares which external identity providers (e.g. + "github") must be linked for the calling user before this Tool can be + launched (ADR 0022/0027). Only meaningful for a container Tool + (image/serviceAccountName) -- an agent-backed Tool (agentRef) carries + this instead on the wrapped Agent CR. This controller does not consume + the field itself -- it is read by the agent-orchestrator when deciding + whether/how to launch a ToolRun for a given user (resolving and + injecting a per-user token via ToolRunSpec.SecretEnv) -- but it belongs + on the CRD as the source of truth an operator deploys alongside the + rest of the Tool catalog entry. + items: + type: string + type: array image: description: |- image is the fully-qualified container image the ToolRun controller launches as a Job. diff --git a/controllers/core-controller/internal/controller/toolrun_controller.go b/controllers/core-controller/internal/controller/toolrun_controller.go index d642d01..ac6b8c3 100644 --- a/controllers/core-controller/internal/controller/toolrun_controller.go +++ b/controllers/core-controller/internal/controller/toolrun_controller.go @@ -210,10 +210,15 @@ func buildJob(run *toolv1alpha1.ToolRun, tool *toolv1alpha1.Tool) (*batchv1.Job, serviceAccountName: tool.Spec.ServiceAccountName, args: args, staticEnv: tool.Spec.Env, - secretEnv: tool.Spec.SecretEnv, - resources: tool.Spec.Resources, - callback: run.Spec.Callback, - timeoutSeconds: timeoutSeconds, + // mergeSecretEnv lets a caller inject a per-invocation credential + // (e.g. a per-user GitHub identity-link token, ADR 0032) that + // overrides or adds to the Tool's baked-in static secretEnv for this + // one run only, without mutating the Tool CR -- same mechanism + // AgentRunReconciler already uses for Agent/AgentRun. + secretEnv: mergeSecretEnv(tool.Spec.SecretEnv, run.Spec.SecretEnv), + resources: tool.Spec.Resources, + callback: run.Spec.Callback, + timeoutSeconds: timeoutSeconds, }) } diff --git a/controllers/core-controller/internal/controller/toolrun_controller_test.go b/controllers/core-controller/internal/controller/toolrun_controller_test.go index 4da335f..74a8733 100644 --- a/controllers/core-controller/internal/controller/toolrun_controller_test.go +++ b/controllers/core-controller/internal/controller/toolrun_controller_test.go @@ -134,5 +134,97 @@ var _ = Describe("ToolRun Controller", func() { }) Expect(err).NotTo(HaveOccurred()) }) + + It("merges ToolRun.Spec.SecretEnv over the Tool template's static secretEnv by name (ADR 0032)", func() { + By("creating a Tool with a static secretEnv entry and identityProviders declared") + identityToolName := fmt.Sprintf("%s-identity-tool", resourceName) + identityTool := &toolv1alpha1.Tool{ + ObjectMeta: metav1.ObjectMeta{Name: identityToolName, Namespace: "default"}, + Spec: toolv1alpha1.ToolSpec{ + Description: "identity-linked test tool", + Input: "a gh CLI command line", + Output: "gh's own output", + AllowedRoles: []string{"writer"}, + Image: "example.com/github:latest", + ServiceAccountName: "github-tool", + IdentityProviders: []string{"github"}, + SecretEnv: []toolv1alpha1.SecretEnvVar{ + { + Name: "GITHUB_TOKEN", + SecretRef: toolv1alpha1.SecretKeySelector{ + Name: "github-tool-secrets", + Key: "GITHUB_TOKEN", + }, + }, + }, + }, + } + Expect(k8sClient.Create(ctx, identityTool)).To(Succeed()) + defer func() { + Expect(k8sClient.Delete(ctx, identityTool)).To(Succeed()) + }() + + By("creating a ToolRun with a per-run SecretEnv override for the same GITHUB_TOKEN key") + identityRunName := fmt.Sprintf("%s-identity-run", resourceName) + identityRunKey := types.NamespacedName{Name: identityRunName, Namespace: "default"} + identityRun := &toolv1alpha1.ToolRun{ + ObjectMeta: metav1.ObjectMeta{Name: identityRunName, Namespace: "default"}, + Spec: toolv1alpha1.ToolRunSpec{ + ToolRef: identityToolName, + Args: []string{"issue view 86 --repo imaustink/agent-controller"}, + Callback: toolv1alpha1.ToolRunCallback{ + URL: "http://agent-orchestrator-callback.default.svc.cluster.local:8080", + SecretRef: toolv1alpha1.SecretKeySelector{ + Name: "agent-orchestrator-secrets", + Key: "AGENT_CALLBACK_SECRET", + }, + }, + SecretEnv: []toolv1alpha1.SecretEnvVar{ + { + Name: "GITHUB_TOKEN", + SecretRef: toolv1alpha1.SecretKeySelector{ + Name: identityRunName + "-identity", + Key: "GITHUB_TOKEN", + }, + }, + }, + }, + } + Expect(k8sClient.Create(ctx, identityRun)).To(Succeed()) + defer func() { + Expect(k8sClient.Delete(ctx, identityRun)).To(Succeed()) + }() + + By("reconciling the ToolRun") + controllerReconciler := &ToolRunReconciler{ + Client: k8sClient, + Scheme: k8sClient.Scheme(), + } + _, err := controllerReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: identityRunKey}) + Expect(err).NotTo(HaveOccurred()) + + var updated toolv1alpha1.ToolRun + Expect(k8sClient.Get(ctx, identityRunKey, &updated)).To(Succeed()) + Expect(updated.Status.JobName).NotTo(BeEmpty()) + + var job batchv1.Job + jobKey := types.NamespacedName{Name: updated.Status.JobName, Namespace: "default"} + Expect(k8sClient.Get(ctx, jobKey, &job)).To(Succeed()) + + container := job.Spec.Template.Spec.Containers[0] + findEnv := func(name string) *corev1.EnvVar { + for i := range container.Env { + if container.Env[i].Name == name { + return &container.Env[i] + } + } + return nil + } + + By("the ToolRun-level GITHUB_TOKEN entry winning over the Tool's static one") + githubTokenEnv := findEnv("GITHUB_TOKEN") + Expect(githubTokenEnv).NotTo(BeNil()) + Expect(githubTokenEnv.ValueFrom.SecretKeyRef.LocalObjectReference.Name).To(Equal(identityRunName + "-identity")) + }) }) }) diff --git a/docs/adr/0032-tool-level-identity-delegation-and-github-cli-tool.md b/docs/adr/0032-tool-level-identity-delegation-and-github-cli-tool.md new file mode 100644 index 0000000..92ace78 --- /dev/null +++ b/docs/adr/0032-tool-level-identity-delegation-and-github-cli-tool.md @@ -0,0 +1,136 @@ +# 0032. Extend per-user GitHub identity delegation to container Tools, and add a `github` CLI Tool + +Date: 2026-07-23 + +## Status + +Accepted + +## Context + +Issue [imaustink/agent-controller#86](https://github.com/imaustink/agent-controller/issues/86) +asked for "a tool with the GH CLI preinstalled... wired up in prod", using +"the OAuth delegation stuff in the integration gateway to get a token" — +i.e. the same per-user GitHub identity-link mechanism ADR 0022 built for +`opencode-swe-agent`, but for a lighter-weight, non-agentic **Tool** (a +single `gh` command in, `gh`'s own output out) rather than a full coding +sub-agent. + +ADR 0022's mechanism, on inspection, was wired exclusively through +`Agent`/`AgentRun`: + +- `ToolRunSpec` (`controllers/core-controller/api/v1alpha1/toolrun_types.go`) + had no `secretEnv` field at all — only `toolRef`/`args`/`callback`/ + `timeoutSeconds` — unlike `AgentRunSpec`, which gained `SecretEnv + []SecretEnvVar` under ADR 0022. There was nothing for the Go reconciler's + `mergeSecretEnv` (already generic, `run_job.go`) to merge for a ToolRun. +- `ToolSpec` had no `IdentityProviders` field, unlike `AgentSpec`. +- `ContainerToolLauncher`/`LaunchOptions` (`k8s/container-tool-launcher.ts`) + and `ToolRunLauncher` (`k8s/toolrun-launcher.ts`) had no `secretEnv` option + or Secret-creation logic at all, unlike `AgentLaunchOptions`/ + `AgentRunLauncher`. +- `graph.ts`'s `runTool` only checked `tool.identityProviders` inside the + `tool.agentRunTemplate` branch (an agent-backed Tool dispatched as an + AgentRun, ADR 0021) — the `tool.jobTemplate` branch (a genuine container + Tool/Job) had no identity gate at all. + +So a real container Tool authenticating as the calling user's own GitHub +identity, rather than a shared bot/PAT baked into its Secret, required new +plumbing at every layer ADR 0022 already built for Agents — not a +reimplementation, but the same mechanism extended one CRD kind further. + +## Decision + +1. **`ToolRunSpec` gains `SecretEnv []SecretEnvVar`**, identical in shape and + semantics to `AgentRunSpec.SecretEnv` — a per-invocation override/addition + to the referenced `Tool`'s static `secretEnv`, keyed by env var name. + `ToolRunReconciler.buildJob` now calls the already-generic + `mergeSecretEnv(tool.Spec.SecretEnv, run.Spec.SecretEnv)` (previously it + passed `tool.Spec.SecretEnv` straight through) — no change needed to + `mergeSecretEnv` itself, since it never assumed a specific CRD kind. +2. **`ToolSpec` gains `IdentityProviders []string`**, mirroring + `AgentSpec.IdentityProviders` — the source-of-truth declaration, on the + catalog entry, of which external identity a caller must have linked + before this Tool can be launched. Only meaningful for a container Tool; + an agent-backed Tool continues to carry this on the wrapped `Agent` CR + instead (ADR 0021's existing `agentRefs`-resolution path is unchanged). +3. **`ToolRunLauncher` (TypeScript) gains the same per-invocation identity + `secretEnv` mechanism `AgentRunLauncher` already had**: given + `LaunchOptions.secretEnv`, it creates a dedicated `${name}-identity` k8s + Secret via a new optional `SecretApiLike` constructor param (a `CoreV1Api` + slice, shared type with `AgentRunLauncher`), references it from + `ToolRunSpec.secretEnv`, and patches the Secret's `ownerReferences` to the + created `ToolRun` for GC — byte-for-byte the same pattern, so the two + launchers stay structurally identical. +4. **`CrdToolRegistry` reads `Tool.spec.identityProviders` onto + `ToolDescriptor.identityProviders`** for a container Tool (previously this + field was only ever populated for an agent-backed Tool via + `AgentDescriptor.identityProviders`). +5. **`graph.ts`'s `runTool` gates the `tool.jobTemplate` branch on + `tool.identityProviders`**, exactly mirroring the existing gate in the + `tool.agentRunTemplate` branch and in `delegateToAgent`: resolve the + caller's linked token via `deps.identityLinkGateway.getToken`, map the + provider to an env var name via the existing `PROVIDER_ENV_VAR` table + (`{ github: "GITHUB_TOKEN" }`), and pass it through + `containerToolLauncher.launch()`'s `options.secretEnv`. Same v1 scope cut + as the agent-backed-tool branch: this path never *starts* a fresh + device-flow/authcode link (no session slot analogous to + `pendingIdentityLink` exists for a paused tool call) — a caller links once + via a direct conversation with an identity-linking-capable Agent (e.g. + `opencode-swe-agent`) before a Skill can route them to an identity-gated + Tool. +6. **New `github` Tool (`tools/github/`)**: a `recipe-publisher`/ + `kubectl-readonly`-shaped container — `gh` CLI preinstalled (pinned + release binary + sha256 checksum verification, same pattern as + `kubectl-readonly`'s pinned `kubectl`), a single command line in + (`argv[2]`), `gh`'s own stdout out over the standard messaging protocol. + An explicit top-level-command allowlist (`src/allowlist.ts`) blocks + `auth`/`api`/`config`/`secret`/`ssh-key`/etc. entirely and a handful of + individually irreversible-ish subcommands (`repo delete`, `issue + delete`/`transfer`, `workflow run`, ...) even within an allowed command — + but, unlike `kubectl-readonly`, does **not** additionally restrict flags/ + values, since (unlike `kubectl-readonly`'s fixed cluster-wide + ServiceAccount) this tool's real authorization boundary is the delegated + human's own GitHub permissions on whatever they target, the same posture + `opencode-swe-agent` already has (ADR 0022, `docs/security.md`). +7. **Helm wiring** (`charts/community-components`): new + `templates/tool-github.yaml` (a `Tool` CR, `identityProviders: [github]` + when `githubTool.identityLink.enabled`, a static `GITHUB_TOKEN` secretEnv + otherwise — same `if not identityLink.enabled` branching as + `agent-opencode-swe.yaml`) and `templates/serviceaccount-github.yaml` + (plain ServiceAccount, mirrors `serviceaccount-web-fetch.yaml`). No + changes needed to `charts/agent-controller` (the orchestrator/gateway + identity-link plumbing — `IDENTITY_LINK_GATEWAY_URL/TOKEN`, + `GATEWAY_IDENTITY_LINK_TOKEN`, the orchestrator's `secrets: create/patch` + RBAC — is already provider/CR-kind-agnostic and shared by any Agent or, + after this decision, Tool that declares `identityProviders`). + `values-production.yaml` enables `githubTool` with + `identityLink.enabled: true`, reusing the identity-link gateway already + turned on for `opencodeSweAgent` — no new gateway/App configuration + required. + +## Consequences + +- Any container Tool can now opt into per-user GitHub identity delegation by + declaring `identityProviders` on its `Tool` CR — this is a generic CRD/ + reconciler/launcher extension, not something specific to the `github` + Tool. A future Tool needing the same pattern (or a different provider, + once `PROVIDER_ENV_VAR` gains more entries) needs no further plumbing + changes. +- `ToolRunSpec` and `AgentRunSpec` now carry structurally identical + `SecretEnv` fields and the two TS launchers share the same + `SecretApiLike` shape — reduces the temptation for the two to drift, at + the cost of a small duplication between `agentrun-launcher.ts` and + `toolrun-launcher.ts` (a shared helper was considered but deferred: the + two CR kinds' `spec` shapes differ enough — `agentRef`+`goal` vs. + `toolRef`+`args` — that factoring out just the Secret-creation half would + add an extra indirection for a relatively small amount of shared code). +- The `github` Tool's blast radius, like `opencode-swe-agent`'s, is bounded + by the linked human's own GitHub permissions rather than a shared + bot/PAT's fixed scope — a caller can only do what they could already do on + GitHub directly. The command allowlist is defense-in-depth clarity about + this tool's intended purpose, not the primary authorization boundary. +- A caller who hasn't linked GitHub yet gets a clear, actionable error from + this Tool's call path rather than a silent credential-less failure or (the + ADR 0022-era gap this closes) a Tool that could only ever run with a + shared static token. diff --git a/docs/adr/README.md b/docs/adr/README.md index f61398f..f2c452c 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -37,5 +37,6 @@ See [../orchestrator.md](../orchestrator.md) for how these fit together. | [0029](0029-canonical-github-credential-subject.md) | Claude credentials (`claude`/`claude-remote`) are keyed by a canonical, lower-cased `github:` subject resolved by one shared helper at every call site — so an authorization done during GitHub-webhook triage is reused in Open WebUI chat and vice versa, instead of each entry point's differing `identity.subject` prompting separately | | [0030](0030-authorization-preflight-outside-the-llm.md) | Authorization becomes a single deterministic pre-flight owned by agent-orchestrator — batch-resolving every provider an Agent CRD declares before launch, handing the run a sealed actor context so agents stop re-deriving identity, with the LLM able only to ask a human to link an account and never to decide authorization or see a credential | | [0031](0031-principal-establishing-account-link.md) | Establishing the caller's principal is its own authorization pre-flight step — a link-only `github` link, gated on a resolver-asserted per-user subject and independent of `Agent.identityProviders` — so Open WebUI chat can learn the login that keys its Claude credentials and actually converge with GitHub-webhook triage, instead of the two flows sharing in one direction only; a pre-principal credential is MOVED onto the principal (new gateway `rekey`) so converging costs no re-authorization | +| [0032](0032-tool-level-identity-delegation-and-github-cli-tool.md) | Per-user GitHub identity delegation (0022) extends from `Agent`/`AgentRun` to container `Tool`/`ToolRun` (new `ToolRunSpec.SecretEnv`, `ToolSpec.IdentityProviders`), and a new `github` Tool (gh CLI preinstalled) is added as its reference implementation | Status values: `proposed` | `accepted` | `superseded by NNNN`. diff --git a/docs/orchestrator.md b/docs/orchestrator.md index 3830f6e..af6495d 100644 --- a/docs/orchestrator.md +++ b/docs/orchestrator.md @@ -177,6 +177,16 @@ Which tools even show up as retrieval candidates depends on **who is asking**: - Job retry/backoff and TTL cleanup (`ttlSecondsAfterFinished`) are owned by the controller, which also mirrors terminal Job state onto the `ToolRun` as a fallback for crashes that never emit a `failed` callback event. +- A container Tool can optionally declare `identityProviders` (e.g. + `["github"]`, ADR 0032 — extending the Agent-only mechanism from ADR 0022): + before launching, `runTool` resolves the calling user's own linked token via + `agent-orchestrator`'s identity-link gateway client and passes it as + `ToolRunLauncher.launch()`'s `options.secretEnv`, which creates a per-run k8s + Secret and references it from `ToolRunSpec.secretEnv` — the Go controller + merges that over the Tool's own static `secretEnv` when building the Job. + The `github` Tool (`tools/github/`) is the reference implementation: it runs + `gh` authenticated as the caller's own GitHub identity rather than a shared + bot credential. ### 4b. LocalTool executor sidecars (ADR 0014) diff --git a/docs/security.md b/docs/security.md index c33a53d..2705e16 100644 --- a/docs/security.md +++ b/docs/security.md @@ -169,6 +169,49 @@ addition specific to it: within the same authenticated Mealie account/group (`MEALIE_API_TOKEN` can't reach other tenants). +## tools/github: a container Tool authenticated as the calling user (ADR 0032) + +[tools/github](../tools/github/) runs a single allowlisted `gh` CLI command, +authenticated with a `GITHUB_TOKEN` that -- when +`Tool.spec.identityProviders: [github]` is set (the recommended, default-in- +`values-production.yaml` configuration) -- is the **calling user's own** +identity-linked GitHub token, resolved and injected per-invocation via +`ToolRunSpec.secretEnv` exactly the way `AgentRunSpec.secretEnv` already +worked for `opencode-swe-agent` (ADR 0022), extended one CRD kind further +(ADR 0032) since `ToolRun` previously had no per-invocation secretEnv +mechanism at all. This means: + +- **The primary authorization boundary is GitHub itself**, not this + container's ServiceAccount/RBAC (which grants nothing beyond ordinary pod + scheduling) -- a caller can only do what they could already do on GitHub + directly with their own account. This is the same posture as + `opencode-swe-agent` below, not a new trust model. +- **In-process command allowlist** (`src/allowlist.ts`) is defense-in-depth + clarity about the tool's intended purpose (issue/PR/repo-read/release/ + search/workflow-read operations), not the primary control -- `auth`/`api`/ + `config`/`secret`/`ssh-key`/etc. are excluded entirely, and a handful of + individually irreversible-ish subcommands (`repo delete`, `issue delete`/ + `transfer`, `workflow run`, `run cancel`/`rerun`, `release`/`label delete`) + are excluded even within an otherwise-allowed command. Unlike + `kubectl-readonly` (whose fixed cluster-wide ServiceAccount is the same + regardless of caller, so flag-level restriction is the only thing standing + between "get pods" and "get secrets -o json"), this tool does not + additionally restrict flags/values, since GitHub's own per-user + authorization already gates what a write can actually do. +- **No persisted credentials.** `GH_CONFIG_DIR` points at the container's + writable `/tmp` (wiped every run); the token lives only in this process's + env for the run's duration, sourced from a per-run k8s Secret + (`-identity`) that is garbage-collected with its `ToolRun`. +- A caller who has not yet linked their GitHub account gets an explicit + error directing them to link it via a direct conversation with an + identity-linking-capable agent (e.g. `opencode-swe-agent`) first -- the + same v1 scope cut as the agent-backed-tool identity gate in `graph.ts`'s + `runTool`: this call path cannot itself start a fresh device-flow link. +- A deployment that does not want per-user delegation can instead configure + `githubTool.identityLink.enabled: false` (the chart default) with a static + `GITHUB_TOKEN` PAT in a Secret, same as any other tool's `secretEnv` -- + the blast radius then reverts to whatever that shared PAT is scoped to. + ## opencode-swe-agent: a deliberately privileged agent `apps/opencode-swe-agent` (an agentic opencode CLI wrapper, calling Anthropic diff --git a/package-lock.json b/package-lock.json index 915e1a1..4d1516e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -2255,6 +2255,10 @@ "node": ">= 0.4" } }, + "node_modules/github": { + "resolved": "tools/github", + "link": true + }, "node_modules/glob": { "version": "7.2.3", "resolved": "https://registry.npmjs.org/glob/-/glob-7.2.3.tgz", @@ -4479,6 +4483,24 @@ "node": ">=20" } }, + "tools/github": { + "version": "0.1.0", + "dependencies": { + "@controller-agent/messaging": "0.1.0" + }, + "bin": { + "github": "dist/index.js" + }, + "devDependencies": { + "@types/node": "^20.16.5", + "tsx": "^4.19.1", + "typescript": "^5.6.2", + "vitest": "^4.0.0" + }, + "engines": { + "node": ">=20" + } + }, "tools/kubectl-readonly": { "version": "0.1.0", "dependencies": { diff --git a/tools/github/.env.example b/tools/github/.env.example new file mode 100644 index 0000000..5a7e72d --- /dev/null +++ b/tools/github/.env.example @@ -0,0 +1,15 @@ +# GitHub token gh authenticates with. In production this is injected +# per-invocation as the CALLING user's own identity-linked token via +# ToolRunSpec.secretEnv (ADR 0022/0027) -- never a static value baked in +# here. For local development, use a fine-grained PAT scoped to just the +# repos you want to test against. +GITHUB_TOKEN= +# Fixed GitHub API host (only relevant for GitHub Enterprise Server). +GH_HOST=github.com + +# Messaging transport (see ../../docs/messaging.md) +RECIPE_TRANSPORT= +RECIPE_JOB_ID= +RECIPE_CALLBACK_URL= +RECIPE_CALLBACK_SECRET= +RECIPE_CALLBACK_ALLOWED_HOSTS= diff --git a/tools/github/Dockerfile b/tools/github/Dockerfile new file mode 100644 index 0000000..1cebaef --- /dev/null +++ b/tools/github/Dockerfile @@ -0,0 +1,72 @@ +# syntax=docker/dockerfile:1 +# +# Build from the REPO ROOT (this tool depends on the shared +# @controller-agent/messaging workspace package): +# +# docker build -f tools/github/Dockerfile -t github:latest . + +############################ +# Build stage +############################ +FROM node:20-bookworm-slim AS build +WORKDIR /repo + +COPY package.json package-lock.json* ./ +COPY packages/messaging/package.json packages/messaging/package.json +COPY tools/github/package.json tools/github/package.json +RUN npm ci + +COPY packages/messaging packages/messaging +COPY tools/github/tsconfig.json tools/github/tsconfig.json +COPY tools/github/src tools/github/src + +RUN npm run build --workspace=@controller-agent/messaging \ + && npm run build --workspace=github \ + && npm prune --omit=dev \ + # Materialize the workspace package into a real directory (not the npm-created + # symlink) so it survives being copied into the runtime stage below. + && rm -rf node_modules/@controller-agent/messaging \ + && mkdir -p node_modules/@controller-agent/messaging \ + && cp -r packages/messaging/dist node_modules/@controller-agent/messaging/dist \ + && cp packages/messaging/package.json node_modules/@controller-agent/messaging/package.json + +############################ +# Runtime stage +############################ +FROM node:20-bookworm-slim AS runtime + +# Pinned gh CLI release, verified against the sha256 checksums file published +# alongside the same release (fetched at build time rather than hardcoded, so +# the check stays correct if this ARG is bumped without also updating a stale +# hash). Bump GH_CLI_VERSION to upgrade. TARGETARCH is set automatically by +# BuildKit (amd64/arm64) and mapped to gh's own release asset naming. +ARG GH_CLI_VERSION=2.96.0 +ARG TARGETARCH +RUN apt-get update \ + && apt-get install -y --no-install-recommends ca-certificates curl \ + && case "${TARGETARCH:-amd64}" in \ + amd64) GH_ARCH=amd64 ;; \ + arm64) GH_ARCH=arm64 ;; \ + *) echo "unsupported TARGETARCH: ${TARGETARCH}" >&2; exit 1 ;; \ + esac \ + && curl -fsSL "https://github.com/cli/cli/releases/download/v${GH_CLI_VERSION}/gh_${GH_CLI_VERSION}_linux_${GH_ARCH}.tar.gz" -o /tmp/gh.tar.gz \ + && curl -fsSL "https://github.com/cli/cli/releases/download/v${GH_CLI_VERSION}/gh_${GH_CLI_VERSION}_checksums.txt" -o /tmp/gh_checksums.txt \ + && (cd /tmp && sha256sum -c <(grep "gh_${GH_CLI_VERSION}_linux_${GH_ARCH}.tar.gz" gh_checksums.txt)) \ + && tar -xzf /tmp/gh.tar.gz -C /tmp \ + && cp "/tmp/gh_${GH_CLI_VERSION}_linux_${GH_ARCH}/bin/gh" /usr/local/bin/gh \ + && chmod a+rx /usr/local/bin/gh \ + && apt-get purge -y curl \ + && apt-get autoremove -y \ + && rm -rf /var/lib/apt/lists/* /tmp/gh.tar.gz /tmp/gh_checksums.txt "/tmp/gh_${GH_CLI_VERSION}_linux_${GH_ARCH}" + +WORKDIR /app +ENV NODE_ENV=production + +COPY --from=build /repo/node_modules ./node_modules +COPY --from=build /repo/tools/github/dist ./dist +COPY --from=build /repo/tools/github/package.json ./package.json + +# node:20-bookworm-slim ships an unprivileged 'node' user. +USER node + +ENTRYPOINT ["node", "dist/index.js"] diff --git a/tools/github/README.md b/tools/github/README.md new file mode 100644 index 0000000..3a32f48 --- /dev/null +++ b/tools/github/README.md @@ -0,0 +1,95 @@ +# github + +A self-contained subagent container: a single allowlisted `gh` (GitHub CLI) +command in, `gh`'s own output out -- authenticated as the **calling user's +own** delegated GitHub token, not a shared bot credential (ADR 0022/0027). + +## Contract + +- **Input** (`argv[2]`): everything after `gh`, e.g. + `"issue view 86 --repo imaustink/agent-controller --json title,body"`. +- **Output**: `gh`'s own stdout, wrapped in a fenced code block (`json` when + `--json` was requested, `text` otherwise), delivered via the event contract + in [docs/messaging.md](../../docs/messaging.md). + +## Identity: acts as the calling user, not a shared bot + +Unlike a tool with a static `GITHUB_TOKEN` baked into its Secret, this tool +is designed to run with **`Tool.spec.identityProviders: [github]`** set (see +`charts/community-components/templates/tool-github.yaml`). When a Skill +routes a call to this tool, `agent-orchestrator`'s `runTool` (`graph.ts`) +checks the calling user's own linked GitHub identity via +`apps/integration-gateway`'s identity-link API (the same OAuth Device Flow +broker `opencode-swe-agent` uses, ADR 0022) and injects the resulting token +as a per-invocation `GITHUB_TOKEN` through `ToolRunSpec.secretEnv` (ADR +0032) -- never embedding it in the `ToolRun` CR itself, and never sharing +one credential across every caller. A caller who hasn't linked their GitHub +account yet gets a clear error asking them to link it via a direct +conversation with an identity-linking-capable agent first (v1 scope cut: +this tool's call path cannot itself start a fresh device-flow link -- only +the peer-level agent-delegation path can, see `graph.ts`'s `runTool` +comment). + +A `GITHUB_TOKEN`/`GH_TOKEN` env var is all `gh` needs to authenticate -- +`src/github.ts` sets both directly on the child process so `gh` never has +to run `gh auth login` or write anything to a persisted config (its +`GH_CONFIG_DIR` is pointed at the container's writable `/tmp`, wiped every +run). + +## Safety model (defense in depth) + +1. **The calling user's own GitHub permissions are the primary boundary.** + Because this tool authenticates as a specific, identity-linked human + (not a broadly-scoped shared bot/App-installation token), the blast + radius of anything it does is already bounded by what that person is + actually allowed to do on GitHub -- the same posture as + `opencode-swe-agent` (see `docs/security.md`'s "opencode-swe-agent: a + deliberately privileged agent" section, ADR 0022). +2. **In-process command allowlist** (`src/allowlist.ts`) -- an explicit + allowlist of top-level `gh` commands + subcommands (`issue`, `pr`, + `repo view/list/clone`, `release view/list`, `gist`, `label`, `search`, + `workflow view/list`, `run view/list/watch/download`). `auth`, `api`, + `config`, `secret`, `variable`, `ssh-key`, `gpg-key`, `codespace`, + `extension`, `alias`, `completion`, and `browse` are excluded entirely + (see the file header for why each is out of scope for this tool), and a + handful of individually irreversible-ish subcommands (`repo delete`, + `issue delete`/`transfer`, `release delete`, `workflow run`, `run + cancel`/`rerun`, `label delete`) are excluded even within an otherwise + allowed command. Unlike `tools/kubectl-readonly`'s allowlist, flags/ + values are **not** additionally restricted here -- see the rationale in + `src/allowlist.ts`'s header comment (this tool's ServiceAccount/RBAC + grants nothing broader than a fixed set of GitHub permissions; GitHub's + own authorization on the delegated token is what actually gates a + write). +3. **No shell** -- the validated argv is passed straight to + `child_process.spawn`, never interpolated into a shell string + (`src/github.ts`). +4. **No persisted credentials** -- `GH_CONFIG_DIR` is pointed at `/tmp` + (wiped every run, root filesystem is otherwise read-only); the token + only ever lives in this process's env, sourced from + `ToolRunSpec.secretEnv` (a per-run k8s Secret, garbage-collected with + the `ToolRun`), never written to disk. +5. **Redaction** (`src/security/redact.ts`) -- GitHub's own token prefixes + (`ghp_`/`gho_`/`ghu_`/`ghs_`/`ghr_`) plus generic `Bearer`/`token ` + patterns are stripped from anything that could reach a `progress`/ + `failed` event message, in case `gh`'s own error text ever echoes back + part of what it was given. + +## Local development + +```sh +npm install +npm run typecheck --workspace=github +npm run test --workspace=github +npm run build --workspace=github +docker build -f tools/github/Dockerfile -t github:latest . +GITHUB_TOKEN=ghp_your_pat ./tools/github/run.sh "issue view 86 --repo imaustink/agent-controller" +``` + +To test the actual identity-delegation path end-to-end, enable +`githubTool.enabled=true` (and `githubTool.identityLink.enabled=true`) in +`charts/community-components`, alongside `identityLink.enabled=true` on both +`agent-orchestrator` and `integration-gateway` (see +`charts/agent-controller/values-production.yaml`), and invoke it as a real +`ToolRun`/Job in a cluster (e.g. minikube) after linking a GitHub account +via chat. diff --git a/tools/github/package.json b/tools/github/package.json new file mode 100644 index 0000000..c760b20 --- /dev/null +++ b/tools/github/package.json @@ -0,0 +1,29 @@ +{ + "name": "github", + "version": "0.1.0", + "private": true, + "type": "module", + "description": "Self-contained subagent container: a single allowlisted gh CLI command in, gh's own output out, authenticated as the calling user's own delegated GitHub token.", + "engines": { + "node": ">=20" + }, + "bin": { + "github": "dist/index.js" + }, + "scripts": { + "build": "tsc -p tsconfig.json", + "dev": "tsx src/index.ts", + "start": "node dist/index.js", + "typecheck": "tsc --noEmit", + "test": "vitest run" + }, + "dependencies": { + "@controller-agent/messaging": "0.1.0" + }, + "devDependencies": { + "@types/node": "^20.16.5", + "tsx": "^4.19.1", + "typescript": "^5.6.2", + "vitest": "^4.0.0" + } +} diff --git a/tools/github/run.sh b/tools/github/run.sh new file mode 100755 index 0000000..8ac2e9e --- /dev/null +++ b/tools/github/run.sh @@ -0,0 +1,50 @@ +#!/usr/bin/env bash +# +# Hardened run contract for the github subagent container. +# +# The container is the security boundary here too: all Linux capabilities are +# dropped, the root filesystem is read-only, privilege escalation is +# disabled, and resource limits cap the blast radius. GITHUB_TOKEN +# authenticates gh as whoever the token belongs to -- in production this is +# the calling user's own identity-linked token (ADR 0022/0027), never a +# value baked into this script/image. +# +# Usage: GITHUB_TOKEN=ghp_... ./run.sh "issue view 86 --repo owner/repo" +# or: put GITHUB_TOKEN in a .env file next to this script and run: +# ./run.sh "issue view 86 --repo owner/repo" + +set -euo pipefail + +# Auto-load a local .env (KEY=VALUE lines) if present, without overriding +# variables already set in the environment. +ENV_FILE="$(dirname "$0")/.env" +if [[ -f "$ENV_FILE" ]]; then + set -a + # shellcheck disable=SC1090 + source "$ENV_FILE" + set +a +fi + +GH_COMMAND="${1:?usage: ./run.sh \" [flags]\"}" +IMAGE="${GITHUB_TOOL_IMAGE:-github:latest}" + +: "${GITHUB_TOKEN:?GITHUB_TOKEN is not set (add it to .env or export it)}" + +exec docker run --rm \ + --name github-tool \ + --env GITHUB_TOKEN \ + --env "GH_HOST=${GH_HOST:-github.com}" \ + --env "RECIPE_TRANSPORT=${RECIPE_TRANSPORT:-}" \ + --env "RECIPE_JOB_ID=${RECIPE_JOB_ID:-}" \ + --env "RECIPE_CALLBACK_URL=${RECIPE_CALLBACK_URL:-}" \ + --env "RECIPE_CALLBACK_SECRET=${RECIPE_CALLBACK_SECRET:-}" \ + --env "RECIPE_CALLBACK_ALLOWED_HOSTS=${RECIPE_CALLBACK_ALLOWED_HOSTS:-}" \ + --cap-drop ALL \ + --security-opt no-new-privileges \ + --read-only \ + --tmpfs /tmp:rw,noexec,nosuid,size=64m \ + --pids-limit 128 \ + --memory 256m \ + --memory-swap 256m \ + --cpus 1 \ + "$IMAGE" "$GH_COMMAND" diff --git a/tools/github/src/allowlist.test.ts b/tools/github/src/allowlist.test.ts new file mode 100644 index 0000000..07607c5 --- /dev/null +++ b/tools/github/src/allowlist.test.ts @@ -0,0 +1,77 @@ +import { describe, expect, it } from "vitest"; +import { BlockedCommandError, tokenize, validateCommand } from "./allowlist.js"; + +describe("tokenize", () => { + it("splits on whitespace and honors quotes", () => { + expect(tokenize("issue view 86 --repo imaustink/agent-controller")).toEqual([ + "issue", + "view", + "86", + "--repo", + "imaustink/agent-controller", + ]); + expect(tokenize(`issue comment 86 --body "hello world"`)).toEqual([ + "issue", + "comment", + "86", + "--body", + "hello world", + ]); + }); +}); + +describe("validateCommand", () => { + it("accepts an allowed command + subcommand, passing flags through unmodified", () => { + expect(validateCommand(["issue", "view", "86", "--repo", "imaustink/agent-controller", "--json", "title,body"])).toEqual([ + "issue", + "view", + "86", + "--repo", + "imaustink/agent-controller", + "--json", + "title,body", + ]); + }); + + it("accepts every allowed top-level command with at least one subcommand", () => { + expect(() => validateCommand(["issue", "list"])).not.toThrow(); + expect(() => validateCommand(["pr", "view", "42"])).not.toThrow(); + expect(() => validateCommand(["repo", "view"])).not.toThrow(); + expect(() => validateCommand(["release", "list"])).not.toThrow(); + expect(() => validateCommand(["gist", "create"])).not.toThrow(); + expect(() => validateCommand(["label", "list"])).not.toThrow(); + expect(() => validateCommand(["search", "issues", "some query"])).not.toThrow(); + expect(() => validateCommand(["workflow", "view", "ci.yml"])).not.toThrow(); + expect(() => validateCommand(["run", "list"])).not.toThrow(); + }); + + it("rejects a disallowed top-level command", () => { + expect(() => validateCommand(["auth", "status"])).toThrow(BlockedCommandError); + expect(() => validateCommand(["api", "repos/owner/repo"])).toThrow(BlockedCommandError); + expect(() => validateCommand(["config", "set", "editor", "vim"])).toThrow(BlockedCommandError); + expect(() => validateCommand(["secret", "list"])).toThrow(BlockedCommandError); + expect(() => validateCommand(["ssh-key", "list"])).toThrow(BlockedCommandError); + expect(() => validateCommand(["codespace", "list"])).toThrow(BlockedCommandError); + expect(() => validateCommand(["browse"])).toThrow(BlockedCommandError); + expect(() => validateCommand(["extension", "install", "foo/bar"])).toThrow(BlockedCommandError); + }); + + it("rejects a disallowed subcommand of an otherwise-allowed command", () => { + expect(() => validateCommand(["issue", "delete", "86"])).toThrow(BlockedCommandError); + expect(() => validateCommand(["issue", "transfer", "86", "other/repo"])).toThrow(BlockedCommandError); + expect(() => validateCommand(["repo", "delete", "owner/repo"])).toThrow(BlockedCommandError); + expect(() => validateCommand(["repo", "edit", "--visibility", "public"])).toThrow(BlockedCommandError); + expect(() => validateCommand(["release", "delete", "v1.0.0"])).toThrow(BlockedCommandError); + expect(() => validateCommand(["workflow", "run", "ci.yml"])).toThrow(BlockedCommandError); + expect(() => validateCommand(["run", "cancel", "123"])).toThrow(BlockedCommandError); + expect(() => validateCommand(["label", "delete", "bug"])).toThrow(BlockedCommandError); + }); + + it("rejects an empty command", () => { + expect(() => validateCommand([])).toThrow(BlockedCommandError); + }); + + it("rejects a bare top-level command with no subcommand", () => { + expect(() => validateCommand(["issue"])).toThrow(BlockedCommandError); + }); +}); diff --git a/tools/github/src/allowlist.ts b/tools/github/src/allowlist.ts new file mode 100644 index 0000000..ef30f8e --- /dev/null +++ b/tools/github/src/allowlist.ts @@ -0,0 +1,107 @@ +/** + * Defense-in-depth validation of a caller-supplied `gh` CLI command line, on + * top of the real authorization boundary: this tool authenticates as the + * CALLING user's own delegated GitHub token (via integration-gateway's + * identity-link OAuth Device Flow broker, ADR 0022/0027) rather than a + * shared bot/ServiceAccount credential -- so the blast radius of anything + * this allowlist lets through is already bounded by that person's own + * GitHub permissions on whatever repo they target, exactly like + * opencode-swe-agent (docs/security.md's "opencode-swe-agent: a + * deliberately privileged agent" section). This is why, unlike + * tools/kubectl-readonly's allowlist (whose fixed cluster ServiceAccount + * grants the same broad read access regardless of who is asking), this + * allowlist does not additionally restrict flags/values -- only which + * top-level `gh` command + subcommand may run at all. + * + * Top-level commands are an explicit ALLOWLIST, not "everything except + * auth/config" -- same rationale as kubectl-readonly's resource-kind + * allowlist: a blocklist would silently start exposing whatever new + * subcommand a future `gh` release adds. Commands deliberately excluded + * entirely (not just individual subcommands): `auth` (would let a caller + * overwrite this container's token/host configuration), `api` (arbitrary + * REST/GraphQL calls, including mutating ones, with no per-endpoint + * validation -- a genuine escape hatch this tool does not offer in v1), + * `config`/`alias`/`extension`/`completion` (local CLI configuration, not a + * GitHub operation), `secret`/`variable`/`ssh-key`/`gpg-key`/`codespace` + * (credential/infra management, out of scope for "run a GitHub operation"), + * and `browse` (opens a browser -- meaningless in a headless container). + * + * Within each allowed top-level command, individual subcommands that are + * either irreversible-ish or orthogonal to this tool's purpose are excluded + * too, e.g. `issue transfer`/`repo delete`/`workflow run`/`release delete`. + * See the per-command comments below for the reasoning on each exclusion. + */ + +export class BlockedCommandError extends Error {} + +/** Top-level `gh` command -> allowed subcommands. Both must match for a command line to pass. */ +const ALLOWED_COMMANDS: Record> = { + // No `delete`/`transfer`/`lock`/`pin`: deleting or transferring an issue is + // effectively irreversible/out-of-repo, locking/pinning is a moderation + // action orthogonal to this tool's "read and act on issues" purpose. + issue: new Set(["view", "list", "create", "comment", "edit", "close", "reopen"]), + // `merge`/`review` are core, expected actions for a GitHub-operations tool + // acting as an authorized human -- the same posture opencode-swe-agent + // already takes (ADR 0013/0022), not an escalation introduced here. + pr: new Set(["view", "list", "create", "comment", "edit", "close", "reopen", "diff", "checks", "merge", "review", "status"]), + // No `delete`/`archive`/`edit`/`rename`: those mutate repo-level settings + // (visibility, default branch, name) rather than act on its content -- + // out of scope for this tool; use the GitHub UI/API directly for that. + repo: new Set(["view", "list", "clone"]), + // No `create`/`upload`/`delete`/`delete-asset`/`edit`: release asset + // management is a distinct, higher-blast-radius concern (can rewrite + // published artifacts) left out of v1. + release: new Set(["view", "list"]), + // No `delete`: irreversible. + gist: new Set(["view", "list", "create"]), + // No `delete`/`edit`/`clone`: label deletion/rename can silently detach + // history from issues/PRs that referenced it. + label: new Set(["list", "create"]), + // Always read-only by nature -- no subcommand restrictions needed beyond + // "must be one of gh's actual search targets". + search: new Set(["issues", "prs", "repos", "code", "commits"]), + // No `run`/`enable`/`disable`: triggering arbitrary CI or toggling a + // workflow's enabled state is a distinct, higher-blast-radius capability + // (arbitrary code execution in Actions, cost) left out of v1. + workflow: new Set(["view", "list"]), + // No `cancel`/`delete`/`rerun`: avoid interfering with in-flight or + // historical CI runs; `download` is allowed (read-only artifact fetch). + run: new Set(["view", "list", "watch", "download"]), +} as const; + +/** Splits a command line into tokens, honoring single/double-quoted spans (no shell involved). */ +export function tokenize(commandLine: string): string[] { + const tokens: string[] = []; + const re = /"([^"]*)"|'([^']*)'|(\S+)/g; + let match: RegExpExecArray | null; + while ((match = re.exec(commandLine)) !== null) { + tokens.push(match[1] ?? match[2] ?? match[3] ?? ""); + } + return tokens; +} + +/** + * Validates a tokenized `gh` command line and returns the exact argv to + * spawn (unmodified beyond the allowlist check itself -- see the file + * header for why flags/values aren't additionally restricted here). Throws + * {@link BlockedCommandError} on anything outside the allowlist. + */ +export function validateCommand(tokens: string[]): string[] { + const [command, subcommand] = tokens; + if (!command) { + throw new BlockedCommandError("No gh command given."); + } + const allowedSubcommands = ALLOWED_COMMANDS[command]; + if (!allowedSubcommands) { + throw new BlockedCommandError( + `Command "gh ${command}" is not allowed. Allowed commands: ${Object.keys(ALLOWED_COMMANDS).join(", ")}.`, + ); + } + if (!subcommand || !allowedSubcommands.has(subcommand)) { + const attempted = `gh ${command} ${subcommand ?? ""}`.trim(); + throw new BlockedCommandError( + `"${attempted}" is not allowed. Allowed "gh ${command}" subcommands: ${[...allowedSubcommands].join(", ")}.`, + ); + } + return tokens; +} diff --git a/tools/github/src/config.test.ts b/tools/github/src/config.test.ts new file mode 100644 index 0000000..f9d2b8e --- /dev/null +++ b/tools/github/src/config.test.ts @@ -0,0 +1,93 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +/** + * `config.ts` materializes a single `config` object from `process.env` at + * import time, so each case sets the env then re-imports the module fresh via + * `vi.resetModules()` + dynamic import. + */ +const ENV_KEYS = [ + "RECIPE_TRANSPORT", + "RECIPE_JOB_ID", + "RECIPE_EVENTS_PATH", + "RECIPE_CALLBACK_URL", + "RECIPE_CALLBACK_ALLOWED_HOSTS", + "RECIPE_CALLBACK_MAX_RETRIES", + "RECIPE_NATS_URL", + "RECIPE_NATS_SUBJECT", + "GITHUB_TOKEN", + "GH_TOKEN", + "GH_HOST", + "GITHUB_TOOL_TIMEOUT_MS", +]; + +let saved: Record; + +async function loadConfig() { + vi.resetModules(); + return (await import("./config.js")).config; +} + +beforeEach(() => { + saved = {}; + for (const k of ENV_KEYS) { + saved[k] = process.env[k]; + delete process.env[k]; + } +}); + +afterEach(() => { + for (const k of ENV_KEYS) { + if (saved[k] === undefined) delete process.env[k]; + else process.env[k] = saved[k]; + } +}); + +describe("config", () => { + it("defaults transport to stdout and applies sensible fallbacks", async () => { + const config = await loadConfig(); + expect(config.transport).toBe("stdout"); + expect(config.ghTimeoutMs).toBe(30_000); + expect(config.callbackMaxRetries).toBe(3); + expect(config.githubHost).toBe("github.com"); + expect(config.callbackAllowedHosts).toEqual([]); + expect(config.jobId).toBeTruthy(); // randomUUID fallback + }); + + it("falls back from an unknown transport to stdout", async () => { + process.env.RECIPE_TRANSPORT = "carrier-pigeon"; + expect((await loadConfig()).transport).toBe("stdout"); + }); + + it("honors a valid transport value", async () => { + process.env.RECIPE_TRANSPORT = "nats"; + expect((await loadConfig()).transport).toBe("nats"); + }); + + it("prefers GITHUB_TOKEN, then GH_TOKEN, then empty string", async () => { + process.env.GITHUB_TOKEN = "ghp_primary"; + process.env.GH_TOKEN = "ghp_secondary"; + expect((await loadConfig()).githubToken).toBe("ghp_primary"); + + delete process.env.GITHUB_TOKEN; + expect((await loadConfig()).githubToken).toBe("ghp_secondary"); + + delete process.env.GH_TOKEN; + expect((await loadConfig()).githubToken).toBe(""); + }); + + it("ignores non-positive / non-numeric numeric overrides and keeps the fallback", async () => { + process.env.GITHUB_TOOL_TIMEOUT_MS = "not-a-number"; + expect((await loadConfig()).ghTimeoutMs).toBe(30_000); + + process.env.GITHUB_TOOL_TIMEOUT_MS = "-5"; + expect((await loadConfig()).ghTimeoutMs).toBe(30_000); + + process.env.GITHUB_TOOL_TIMEOUT_MS = "1234"; + expect((await loadConfig()).ghTimeoutMs).toBe(1234); + }); + + it("parses a comma list into trimmed, lower-cased, non-empty hosts", async () => { + process.env.RECIPE_CALLBACK_ALLOWED_HOSTS = " Orchestrator.Svc , , nats.svc "; + expect((await loadConfig()).callbackAllowedHosts).toEqual(["orchestrator.svc", "nats.svc"]); + }); +}); diff --git a/tools/github/src/config.ts b/tools/github/src/config.ts new file mode 100644 index 0000000..486544f --- /dev/null +++ b/tools/github/src/config.ts @@ -0,0 +1,96 @@ +import { randomUUID } from "node:crypto"; + +/** + * Central configuration. Kept deliberately narrow: this container's only + * job is to run one allowlisted `gh` CLI invocation, authenticated as + * whatever `GITHUB_TOKEN` its ToolRun's secretEnv supplied, and report the + * result -- no model/formatting knobs here (contrast recipe-scraper). + * + * The `RECIPE_*` names below are NOT a copy/paste mistake -- they are the + * fixed messaging-contract env var names the Go core-controller's + * `buildRunJob` (controllers/core-controller/internal/controller/run_job.go) + * injects into every ToolRun-launched Job's container regardless of the + * tool's own name (see that file's `RECIPE_TRANSPORT`/`RECIPE_CALLBACK_URL`/ + * `RECIPE_CALLBACK_SECRET`/`RECIPE_NATS_SUBJECT`/`RECIPE_NATS_URL`) -- + * every tool in this repo that is actually wired up as a production ToolRun + * (recipe-scraper, recipe-publisher, this one) reads these same names so + * the callback/NATS result-delivery plumbing works end-to-end. + */ +export interface AppConfig { + /** Message-passing transport for events (see docs/messaging.md). */ + transport: "stdout" | "events" | "file" | "callback" | "nats"; + /** Correlation id for this tool call; generated if not provided. */ + jobId: string; + /** File path for the `file` transport (NDJSON, append-only). */ + eventsPath: string; + /** + * HTTP callback endpoint for the `callback` transport. MUST be supplied by + * the trusted parent orchestrator/controller, never derived from tool input. + */ + callbackUrl: string | undefined; + /** Optional shared secret; enables HMAC-SHA256 signing of callback bodies. */ + callbackSecret: string | undefined; + /** Allowlist of hosts the callback may target. */ + callbackAllowedHosts: string[]; + /** Delivery retry attempts for the callback transport. */ + callbackMaxRetries: number; + /** NATS server URL for the `nats` transport, e.g. nats://nats.svc:4222 */ + natsUrl: string | undefined; + /** NATS subject to publish tool events to for the `nats` transport. */ + natsSubject: string | undefined; + /** + * The GitHub token `gh` authenticates with -- normally the CALLING user's + * own delegated OAuth token, injected per-invocation via + * `ToolRunSpec.secretEnv` by agent-orchestrator's identity-link gateway + * client (ADR 0022/0027), never a shared bot credential baked into this + * Tool's image/template. Falls back to `GH_TOKEN` for parity with `gh`'s + * own env var precedence (both are set by this tool's own subprocess + * wrapper regardless of which one the caller supplied -- see github.ts). + */ + githubToken: string; + /** Fixed GitHub API host `gh` is configured to talk to -- see github.ts. Not caller-controlled. */ + githubHost: string; + /** Bound on how long a single `gh` invocation may run. */ + ghTimeoutMs: number; +} + +function num(raw: string | undefined, fallback: number): number { + if (raw === undefined) return fallback; + const n = Number(raw); + return Number.isFinite(n) && n > 0 ? n : fallback; +} + +function list(raw: string | undefined): string[] { + if (!raw) return []; + return raw + .split(",") + .map((s) => s.trim().toLowerCase()) + .filter(Boolean); +} + +function transport(raw: string | undefined): AppConfig["transport"] { + switch (raw) { + case "events": + case "file": + case "callback": + case "nats": + return raw; + default: + return "stdout"; + } +} + +export const config: AppConfig = { + transport: transport(process.env.RECIPE_TRANSPORT), + jobId: process.env.RECIPE_JOB_ID ?? randomUUID(), + eventsPath: process.env.RECIPE_EVENTS_PATH ?? "/tmp/github-tool-events.ndjson", + callbackUrl: process.env.RECIPE_CALLBACK_URL, + callbackSecret: process.env.RECIPE_CALLBACK_SECRET, + callbackAllowedHosts: list(process.env.RECIPE_CALLBACK_ALLOWED_HOSTS), + callbackMaxRetries: num(process.env.RECIPE_CALLBACK_MAX_RETRIES, 3), + natsUrl: process.env.RECIPE_NATS_URL, + natsSubject: process.env.RECIPE_NATS_SUBJECT, + githubToken: process.env.GITHUB_TOKEN ?? process.env.GH_TOKEN ?? "", + githubHost: process.env.GH_HOST ?? "github.com", + ghTimeoutMs: num(process.env.GITHUB_TOOL_TIMEOUT_MS, 30_000), +}; diff --git a/tools/github/src/github.test.ts b/tools/github/src/github.test.ts new file mode 100644 index 0000000..855153d --- /dev/null +++ b/tools/github/src/github.test.ts @@ -0,0 +1,100 @@ +import { chmodSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterAll, beforeAll, describe, expect, it, vi } from "vitest"; +import type { AppConfig } from "./config.js"; + +/** + * A stand-in for the real `gh` binary. `runGh` spawns `GH_BIN` directly (by + * absolute path, never via PATH) and passes argv through verbatim, so the fake + * branches on its first argument to simulate each exit path we care about: + * ok -> print stdout, exit 0 + * fail -> print to stderr, exit 3 + * hang -> block forever (exec sleep) so the timeout must SIGKILL it + * echoenv -> print the token env vars back out (auth wiring assertion) + */ +const FAKE_GH = `#!/bin/sh +case "$1" in + ok) echo "hello from gh"; exit 0 ;; + fail) echo "boom on stderr" 1>&2; exit 3 ;; + hang) exec sleep 30 ;; + echoenv) echo "GH_TOKEN=$GH_TOKEN GITHUB_TOKEN=$GITHUB_TOKEN GH_HOST=$GH_HOST"; exit 0 ;; + *) echo "unexpected argv: $*" 1>&2; exit 9 ;; +esac +`; + +let dir: string; +// `GH_BIN` is read at module-load time in github.ts, so the env var must be set +// before the module is first imported -- hence the dynamic import in beforeAll. +let runGh: typeof import("./github.js").runGh; +let GhExecError: typeof import("./github.js").GhExecError; + +function makeConfig(overrides: Partial = {}): AppConfig { + return { + transport: "stdout", + jobId: "job-1", + eventsPath: "/tmp/x.ndjson", + callbackUrl: undefined, + callbackSecret: undefined, + callbackAllowedHosts: [], + callbackMaxRetries: 3, + natsUrl: undefined, + natsSubject: undefined, + githubToken: "ghp_testtoken0123456789abcdef", + githubHost: "github.com", + ghTimeoutMs: 5_000, + ...overrides, + }; +} + +beforeAll(async () => { + dir = mkdtempSync(join(tmpdir(), "gh-tool-test-")); + const ghBin = join(dir, "fake-gh"); + writeFileSync(ghBin, FAKE_GH); + chmodSync(ghBin, 0o755); + process.env.GH_BIN = ghBin; + vi.resetModules(); + const mod = await import("./github.js"); + runGh = mod.runGh; + GhExecError = mod.GhExecError; +}); + +afterAll(() => { + delete process.env.GH_BIN; + rmSync(dir, { recursive: true, force: true }); +}); + +describe("runGh", () => { + it("throws GhExecError without spawning anything when no token is configured", async () => { + await expect(runGh(makeConfig({ githubToken: "" }), ["ok"])).rejects.toBeInstanceOf(GhExecError); + }); + + it("resolves with stdout on a zero exit", async () => { + const out = await runGh(makeConfig(), ["ok"]); + expect(out.trim()).toBe("hello from gh"); + }); + + it("throws GhExecError carrying stderr and the exit code on a non-zero exit", async () => { + const err = await runGh(makeConfig(), ["fail"]).catch((e) => e); + expect(err).toBeInstanceOf(GhExecError); + expect(err.stderr).toBe("boom on stderr"); + expect(err.exitCode).toBe(3); + }); + + it("kills the child and rejects with a timeout error when gh runs past the deadline", async () => { + const cfg = makeConfig({ ghTimeoutMs: 150 }); + const started = Date.now(); + const err = await runGh(cfg, ["hang"]).catch((e) => e); + expect(err).toBeInstanceOf(GhExecError); + expect(err.message).toBe("gh timed out after 150ms"); + // It must not have waited for the 30s sleep -- the SIGKILL path fired. + expect(Date.now() - started).toBeLessThan(5_000); + }); + + it("injects the delegated token as both GH_TOKEN and GITHUB_TOKEN, and never the full process env", async () => { + const out = await runGh(makeConfig({ githubToken: "gho_delegated999", githubHost: "example.com" }), ["echoenv"]); + expect(out).toContain("GH_TOKEN=gho_delegated999"); + expect(out).toContain("GITHUB_TOKEN=gho_delegated999"); + expect(out).toContain("GH_HOST=example.com"); + }); +}); diff --git a/tools/github/src/github.ts b/tools/github/src/github.ts new file mode 100644 index 0000000..cbfb9f1 --- /dev/null +++ b/tools/github/src/github.ts @@ -0,0 +1,92 @@ +import { spawn } from "node:child_process"; +import type { AppConfig } from "./config.js"; + +export class GhExecError extends Error { + constructor( + message: string, + readonly stderr: string, + readonly exitCode: number | null, + ) { + super(message); + } +} + +/** Absolute path to the gh binary (see Dockerfile) -- spawned directly, never resolved via PATH. */ +const GH_BIN = process.env.GH_BIN ?? "/usr/local/bin/gh"; + +/** + * Builds the minimal, explicit env `gh` needs -- never the full inherited + * process env, so nothing this container itself doesn't need (or scrape + * from elsewhere) can leak into the child process or its own subrequests. + * `GH_TOKEN` is `gh`'s own preferred auth env var; `GITHUB_TOKEN` is set + * too for parity with how other tools in this repo (e.g. + * apps/opencode-swe-agent) authenticate `gh`, and because some `gh` + * versions/environments prefer one over the other. + */ +function ghEnv(cfg: AppConfig): NodeJS.ProcessEnv { + return { + GH_TOKEN: cfg.githubToken, + GITHUB_TOKEN: cfg.githubToken, + GH_HOST: cfg.githubHost, + // Never write to a persisted config file -- this container's root + // filesystem is read-only except /tmp (see run.sh's --tmpfs contract). + GH_CONFIG_DIR: "/tmp/gh-config", + // Disable interactive prompts and the update-notifier network check -- + // this is a one-shot, non-interactive Job with restricted egress + // expectations, not an interactive terminal session. + GH_PROMPT_DISABLED: "1", + GH_NO_UPDATE_NOTIFIER: "1", + HOME: "/tmp", + PATH: "/usr/local/bin:/usr/bin:/bin", + }; +} + +/** + * Runs `gh` with the given (already-allowlisted) argv via `spawn` -- never a + * shell string, so nothing in argv can be reinterpreted as shell syntax. + * Returns combined stdout on success; throws {@link GhExecError} on a + * non-zero exit or timeout. + */ +export async function runGh(cfg: AppConfig, argv: string[]): Promise { + if (!cfg.githubToken) { + throw new GhExecError("No GitHub token configured (GITHUB_TOKEN/GH_TOKEN both empty)", "", null); + } + + return new Promise((resolve, reject) => { + const child = spawn(GH_BIN, argv, { + stdio: ["ignore", "pipe", "pipe"], + env: ghEnv(cfg), + }); + + let stdout = ""; + let stderr = ""; + let timedOut = false; + const timer = setTimeout(() => { + timedOut = true; + child.kill("SIGKILL"); + }, cfg.ghTimeoutMs); + + child.stdout.on("data", (chunk) => { + stdout += chunk; + }); + child.stderr.on("data", (chunk) => { + stderr += chunk; + }); + child.on("error", (err) => { + clearTimeout(timer); + reject(err); + }); + child.on("close", (code) => { + clearTimeout(timer); + if (timedOut) { + reject(new GhExecError(`gh timed out after ${cfg.ghTimeoutMs}ms`, stderr.trim(), code)); + return; + } + if (code === 0) { + resolve(stdout); + } else { + reject(new GhExecError(`gh exited with code ${code}`, stderr.trim(), code)); + } + }); + }); +} diff --git a/tools/github/src/index.ts b/tools/github/src/index.ts new file mode 100644 index 0000000..1cf52fc --- /dev/null +++ b/tools/github/src/index.ts @@ -0,0 +1,103 @@ +import { BlockedCommandError, tokenize, validateCommand } from "./allowlist.js"; +import { config } from "./config.js"; +import { GhExecError, runGh } from "./github.js"; +import { createSink, JobEmitter } from "./messaging/index.js"; +import type { ErrorCode } from "./schema.js"; +import { clip } from "./security/redact.js"; + +/** Process exit codes, so the parent agent can branch on failure class. */ +const EXIT = { + usage: 2, + blockedCommand: 3, + ghError: 4, + general: 1, +} as const; + +class PipelineError extends Error { + constructor( + readonly code: ErrorCode, + readonly exitCode: number, + message: string, + ) { + super(message); + } +} + +function fail(code: ErrorCode, exitCode: number, message: string): never { + throw new PipelineError(code, exitCode, clip(message, 2000)); +} + +async function run(emitter: JobEmitter, commandLine: string): Promise { + await emitter.progress("validate"); + let argv: string[]; + try { + argv = validateCommand(tokenize(commandLine)); + } catch (err) { + if (err instanceof BlockedCommandError) { + fail("blocked_command", EXIT.blockedCommand, err.message); + } + throw err; + } + + await emitter.progress("exec", { message: argv.join(" ") }); + let stdout: string; + try { + stdout = await runGh(config, argv); + } catch (err) { + if (err instanceof GhExecError) { + fail("gh_error", EXIT.ghError, `gh failed: ${err.stderr || err.message}`); + } + throw err; + } + + const isJson = argv.some((a) => a === "--json" || a.startsWith("--json=")); + const lang = isJson ? "json" : "text"; + await emitter.succeeded(`\`\`\`${lang}\n${stdout.trim()}\n\`\`\``); +} + +async function main(): Promise { + const sink = createSink(config); + const emitter = new JobEmitter(config.jobId, sink); + const commandLine = process.argv[2]; + + try { + if (!commandLine) { + fail( + "usage", + EXIT.usage, + 'Usage: github " [flags]" (e.g. "issue view 86 --repo owner/repo --json title,body")', + ); + } + await emitter.accepted(clip(commandLine, 500)); + await run(emitter, commandLine); + await emitter.close(); + } catch (err) { + const { code, exitCode, message } = toPipelineError(err); + process.stderr.write(`${message}\n`); + try { + await emitter.failed(code, message); + await emitter.close(); + } catch { + // The event stream is best-effort on the failure path; the exit code + // remains the authoritative backstop. + } + process.exit(exitCode); + } +} + +function toPipelineError(err: unknown): { + code: ErrorCode; + exitCode: number; + message: string; +} { + if (err instanceof PipelineError) { + return { code: err.code, exitCode: err.exitCode, message: err.message }; + } + return { + code: "general", + exitCode: EXIT.general, + message: clip(`Unexpected error: ${(err as Error).message}`, 2000), + }; +} + +void main(); diff --git a/tools/github/src/messaging/index.ts b/tools/github/src/messaging/index.ts new file mode 100644 index 0000000..702e512 --- /dev/null +++ b/tools/github/src/messaging/index.ts @@ -0,0 +1,53 @@ +import { + CallbackSink, + FileSink, + JobEmitter as BaseJobEmitter, + NatsSink, + StdoutSink, + type Sink, +} from "@controller-agent/messaging"; +import type { AppConfig } from "../config.js"; +import type { ErrorCode, Stage } from "../schema.js"; +import { clip } from "../security/redact.js"; + +export type { Sink } from "@controller-agent/messaging"; + +/** This tool's concrete emitter: the result is `gh`'s own stdout text (or + * JSON), wrapped in Markdown by index.ts before it reaches `succeeded`. */ +export class JobEmitter extends BaseJobEmitter { + constructor(jobId: string, sink: Sink) { + super(jobId, sink, { sanitize: clip }); + } +} + +/** + * Selects the event transport from configuration. `stdout` is the default and + * preserves the original single-envelope contract; the others opt into the + * structured event stream (see docs/messaging.md). + */ +export function createSink(cfg: AppConfig): Sink { + switch (cfg.transport) { + case "events": + return new StdoutSink("ndjson"); + case "file": + return new FileSink(cfg.eventsPath); + case "callback": + if (!cfg.callbackUrl) { + throw new Error("RECIPE_TRANSPORT=callback requires RECIPE_CALLBACK_URL"); + } + return new CallbackSink({ + url: cfg.callbackUrl, + secret: cfg.callbackSecret, + allowedHosts: cfg.callbackAllowedHosts, + maxRetries: cfg.callbackMaxRetries, + }); + case "nats": + if (!cfg.natsUrl || !cfg.natsSubject) { + throw new Error("RECIPE_TRANSPORT=nats requires RECIPE_NATS_URL and RECIPE_NATS_SUBJECT"); + } + return new NatsSink({ natsUrl: cfg.natsUrl, subject: cfg.natsSubject }); + case "stdout": + default: + return new StdoutSink("final"); + } +} diff --git a/tools/github/src/schema.ts b/tools/github/src/schema.ts new file mode 100644 index 0000000..5054b28 --- /dev/null +++ b/tools/github/src/schema.ts @@ -0,0 +1,9 @@ +/** + * Failure taxonomy for `failed` events. Mirrors the process exit codes in + * index.ts so the parent orchestrator can branch on failure class regardless + * of which transport delivered the event. + */ +export type ErrorCode = "usage" | "blocked_command" | "gh_error" | "general"; + +/** Pipeline stages surfaced in `progress` events. */ +export type Stage = "validate" | "exec"; diff --git a/tools/github/src/security/redact.test.ts b/tools/github/src/security/redact.test.ts new file mode 100644 index 0000000..9c04a37 --- /dev/null +++ b/tools/github/src/security/redact.test.ts @@ -0,0 +1,77 @@ +import { describe, expect, it } from "vitest"; +import { clip, redact } from "./redact.js"; + +describe("redact", () => { + // Real-shaped GitHub tokens are `_` followed by a long base62 body. + // The whole point of this tool is spawning a subprocess with a live token and + // scrubbing it out of anything that echoes back, so these patterns are the + // last line of defense on the failure path (gh's own verbose auth errors can + // print the token they were handed). + it.each([ + ["ghp_", "ghp_0123456789abcdefABCDEFghijklmnopqr"], // classic PAT + ["gho_", "gho_0123456789abcdefABCDEFghijklmnopqr"], // OAuth token + ["ghu_", "ghu_0123456789abcdefABCDEFghijklmnopqr"], // user-to-server + ["ghs_", "ghs_0123456789abcdefABCDEFghijklmnopqr"], // server-to-server + ["ghr_", "ghr_0123456789abcdefABCDEFghijklmnopqr"], // refresh token + ])("redacts a %s-prefixed GitHub token", (_prefix, token) => { + const out = redact(`authenticating with ${token} failed`); + expect(out).not.toContain(token); + expect(out).toContain("[REDACTED]"); + }); + + it("redacts a token embedded mid-string without eating the surrounding text", () => { + const out = redact("before ghp_0123456789abcdefABCDEFghij after"); + expect(out).toBe("before [REDACTED] after"); + }); + + it("redacts multiple tokens in one string", () => { + const out = redact( + "gho_0123456789abcdefABCDEFghij and ghs_zyxwvutsrqponmlkjihgfedcba9876", + ); + expect(out).not.toMatch(/gh[oprsu]_/); + expect(out.match(/\[REDACTED\]/g)).toHaveLength(2); + }); + + it("redacts a Bearer authorization header value", () => { + const out = redact("Authorization: Bearer abcDEF123456._-ghijkl"); + expect(out).not.toContain("abcDEF123456._-ghijkl"); + expect(out).toContain("[REDACTED]"); + }); + + it("redacts a `token ` credential (case-insensitively)", () => { + const out = redact("Authorization: TOKEN abcDEF123456ghijklmnop"); + expect(out).not.toContain("abcDEF123456ghijklmnop"); + expect(out).toContain("[REDACTED]"); + }); + + it("leaves ordinary text with no secrets untouched", () => { + const clean = "issue #86 opened by octocat in imaustink/agent-controller"; + expect(redact(clean)).toBe(clean); + }); + + it("does not over-match a short `gh`-prefixed word that isn't a token", () => { + // Below the 20-char body threshold -> not a credential shape. + expect(redact("ghp_short")).toBe("ghp_short"); + }); +}); + +describe("clip", () => { + it("redacts before truncating so a token split by the cut can't leak", () => { + const token = "ghp_0123456789abcdefABCDEFghijklmnopqr"; + // Put the token right at the truncation boundary. + const input = `${"x".repeat(10)}${token}${"y".repeat(50)}`; + const out = clip(input, 25); + expect(out).not.toContain(token); + expect(out).not.toContain("ghp_"); + }); + + it("appends an ellipsis when the (redacted) input exceeds max", () => { + const out = clip("a".repeat(100), 10); + expect(out.endsWith("…")).toBe(true); + expect(out.length).toBe(11); // 10 chars + the ellipsis + }); + + it("returns short input unchanged", () => { + expect(clip("short", 4000)).toBe("short"); + }); +}); diff --git a/tools/github/src/security/redact.ts b/tools/github/src/security/redact.ts new file mode 100644 index 0000000..f789d3e --- /dev/null +++ b/tools/github/src/security/redact.ts @@ -0,0 +1,28 @@ +/** + * Best-effort redaction for anything that might be surfaced in progress/error + * messages. `gh`'s own error text can echo the token it was given (e.g. in a + * verbose auth failure), so it is stripped before it ever leaves this + * process -- same discipline as every other tool in this repo (see + * docs/security.md's "Secret handling" section). + */ +const SECRET_PATTERNS: RegExp[] = [ + // GitHub's own token prefixes: ghp_ (PAT), gho_ (OAuth), ghu_ (user-to-server + // App), ghs_ (server-to-server App installation), ghr_ (refresh token). + /gh[oprsu]_[A-Za-z0-9]{20,}/g, + /Bearer\s+[A-Za-z0-9._-]{16,}/gi, + /token\s+[A-Za-z0-9._-]{16,}/gi, +]; + +export function redact(input: string): string { + let out = input; + for (const pattern of SECRET_PATTERNS) { + out = out.replace(pattern, "[REDACTED]"); + } + return out; +} + +/** Truncate a string for safe logging. */ +export function clip(input: string, max = 4000): string { + const redacted = redact(input); + return redacted.length > max ? `${redacted.slice(0, max)}…` : redacted; +} diff --git a/tools/github/tsconfig.json b/tools/github/tsconfig.json new file mode 100644 index 0000000..f62eb78 --- /dev/null +++ b/tools/github/tsconfig.json @@ -0,0 +1,21 @@ +{ + "compilerOptions": { + "target": "ES2022", + "module": "NodeNext", + "moduleResolution": "NodeNext", + "lib": ["ES2022"], + "outDir": "dist", + "rootDir": "src", + "strict": true, + "noUncheckedIndexedAccess": true, + "esModuleInterop": true, + "skipLibCheck": true, + "resolveJsonModule": true, + "declaration": false, + "sourceMap": false, + "forceConsistentCasingInFileNames": true, + "types": ["node"] + }, + "include": ["src/**/*"], + "exclude": ["src/**/*.test.ts", "node_modules", "dist"] +} diff --git a/tools/github/vitest.config.ts b/tools/github/vitest.config.ts new file mode 100644 index 0000000..ce36a74 --- /dev/null +++ b/tools/github/vitest.config.ts @@ -0,0 +1,8 @@ +import { defineConfig } from "vitest/config"; + +export default defineConfig({ + test: { + include: ["src/**/*.test.ts"], + environment: "node", + }, +});