Add github Tool (gh CLI) with per-user identity delegation

[k5s-bot]

Summary

Closes #86 ("Create a GitHub tool").

Adds a new github Tool (a container Tool/ToolRun, not an Agent) with the GitHub CLI (gh) preinstalled, and everything needed to wire it up in production, authenticated as the calling user's own linked GitHub identity via the integration-gateway's OAuth Device Flow delegation (ADR 0022) — not a shared bot/PAT.

Why this needed more than a new tool container

Auditing the existing per-user GitHub identity delegation mechanism (ADR 0022) showed it was wired exclusively through Agent/AgentRun — ToolRunSpec had no per-invocation secretEnv at all, ToolSpec had no identityProviders, the TS ToolRunLauncher had no Secret-creation logic, and graph.ts's runTool only checked identity for agent-backed tools. So this PR extends that mechanism one CRD kind further (ADR 0025) before adding the tool itself:

• controllers/core-controller: ToolRunSpec.SecretEnv (mirrors AgentRunSpec.SecretEnv), ToolSpec.IdentityProviders (mirrors AgentSpec), and ToolRunReconciler.buildJob now calls the already-generic mergeSecretEnv — regenerated CRDs/deepcopy via make manifests generate.
• apps/agent-orchestrator: ToolRunLauncher gains the same per-invocation identity-secretEnv mechanism AgentRunLauncher already had (creates a short-lived <name>-identity k8s Secret, references it from ToolRunSpec.secretEnv, patches ownerReferences for GC); CrdToolRegistry reads Tool.spec.identityProviders; graph.ts's runTool gates the container-tool branch on identityProviders exactly like the existing agent-backed-tool/delegateToAgent gates.
• New tests throughout (Go: toolrun_controller_test.go merge test; TS: toolrun-launcher.test.ts, crd-tool-registry.test.ts, graph.test.ts).

The tool itself (tools/github/)

• gh CLI preinstalled via a pinned release binary + sha256 checksum verification (same pattern as tools/kubectl-readonly's pinned kubectl).
• Input: a single gh command line (e.g. "issue view 86 --repo owner/repo --json title,body"); output: gh's own stdout, delivered over the standard callback/messaging contract.
• An explicit top-level-command allowlist (src/allowlist.ts) excludes auth/api/config/secret/ssh-key/etc. entirely and a handful of irreversible-ish subcommands (repo delete, issue delete/transfer, workflow run, ...) even within an otherwise-allowed command — see the file header and tools/github/README.md for the full rationale, including why (unlike kubectl-readonly) flags/values aren't additionally restricted: the real authorization boundary here is the delegated human's own GitHub permissions, not this tool's RBAC.
• No persisted credentials — GH_CONFIG_DIR points at the container's writable /tmp; the token lives only in-process, sourced from a per-run k8s Secret garbage-collected with its ToolRun.

Prod wiring

• charts/community-components: new templates/tool-github.yaml + templates/serviceaccount-github.yaml, values.yaml githubTool block (static PAT or identityLink.enabled), enabled in values-production.yaml reusing the identity-link gateway already turned on for opencodeSweAgent — no new gateway/App config needed.
• .github/workflows/release.yml: github image added to the build/publish/deploy matrix (path filter, image matrix entry, deploy trigger).

Docs

New ADR (docs/adr/0025-...md), docs/orchestrator.md and docs/security.md updated, tools/github/README.md, root README.md tool table/layout.

Testing

• controllers/core-controller: go build ./..., go vet ./..., gofmt -l . clean; go test ./internal/controller/... passes (including the new merge test).
• Whole npm monorepo: npm run typecheck --workspaces and npm run test --workspaces pass (301 → 309 orchestrator tests; 7 new github tool tests; new toolrun-launcher/crd-tool-registry tests).
• helm lint/helm template verified for both values.yaml and values-production.yaml, with githubTool.enabled/identityLink.enabled on and off.
• Docker build/manual gh invocation was not exercised (no Docker daemon in this sandbox) — the Dockerfile mirrors tools/kubectl-readonly's proven pinned-binary-with-checksum pattern.

[k5s-bot]

Review

Overview

This PR extends the per-user identity delegation mechanism (ADR 0022) from Agent/AgentRun to container Tool/ToolRun (new ToolRunSpec.SecretEnv, ToolSpec.IdentityProviders, ToolRunLauncher Secret-creation logic, graph.ts's runTool identity gate on the jobTemplate branch), then adds a new github Tool (tools/github/) as the reference implementation — a container running an allowlisted gh CLI command, authenticated as the calling user's own linked GitHub identity rather than a shared bot/PAT. Solid scope, and the "extend the existing mechanism one CRD kind further" framing is well-justified by the ADR's audit of where ADR 0022 was and wasn't wired up.

Correctness

graph.ts's new container-Tool identity gate bypasses the provider-aware gateway resolver (bug).
apps/agent-orchestrator/src/agent/graph.ts around line 1314-1320 (the new tool.jobTemplate branch):

if (!deps.identityLinkGateway || !state.identity) { ... }
const provider = tool.identityProviders[0]!;
const existing = await deps.identityLinkGateway.getToken(provider, state.identity.subject);

This hardcodes deps.identityLinkGateway directly. Every other identity-gate call site in this same file — delegateToAgent (line 971) and the pre-existing agent-backed-tool branch (line 1232) — instead calls identityGatewayFor(provider, deps), which routes "claude" to deps.claudeAuthGateway and everything else to deps.identityLinkGateway (line 498). PROVIDER_ENV_VAR already maps both github and claude (line 485), so the codebase clearly anticipates a container Tool eventually declaring identityProviders: ["claude"]. When that happens, this new branch will call the GitHub-specific gateway's getToken("claude", ...) instead of deps.claudeAuthGateway, silently doing the wrong thing (most likely: always reporting "requires linking your claude account" even for a user who has linked Claude, since the wrong client is queried). It's dormant today because the only container Tool that declares identityProviders is github, but it's a real correctness gap relative to the pattern this same PR's ADR explicitly says it's mirroring. Suggest swapping in identityGatewayFor(provider, deps) here too, for consistency and to close the gap before a second provider is added.

Test coverage

The tools/github/ package only ships src/allowlist.test.ts (7 tests, matching the PR description's count). src/github.ts (subprocess spawn, timeout/kill handling, error wrapping), src/config.ts (env parsing/fallbacks), src/index.ts (the actual pipeline: validate → exec → emit, JSON vs. text code-fencing, error-code mapping), and src/security/redact.ts (the token-redaction regexes) have zero tests. Given this tool's whole job is spawning a subprocess with a live credential and scrubbing that credential out of anything that echoes back, I'd expect at least a few unit tests on redact()/clip() (do the ghp_/gho_/etc. patterns actually match real-shaped tokens? does Bearer <token> get redacted?) and on runGh's timeout-kill path, since those are the parts most likely to silently regress.

Minor / nits

• tools/github/README.md (~line 1419 of the diff) says the token is injected via ToolRunSpec.secretEnv "(ADR 0025)" — that should be ADR 0028 (0025 is the unrelated "triage agent starting-work comment" ADR). Everywhere else in this PR correctly cites 0022/0027/0028.
• src/security/redact.ts's SECRET_PATTERNS has both /gh[opsu]_[A-Za-z0-9]{20,}/g and a separate /ghr_[A-Za-z0-9]{20,}/g right below it — could just be /gh[oprsu]_[A-Za-z0-9]{20,}/g (add r to the character class) and drop the second pattern. Purely cosmetic.
• ToolRunLauncher.launch() creates the per-invocation identity Secret before creating the ToolRun CR, and only attaches an ownerReference (for GC) after the CR is created. If createNamespacedCustomObject itself fails (the CR is rejected, a transient k8s error, etc.), the ${name}-identity Secret containing the live token is left behind with no owner and no cleanup path. This exactly mirrors AgentRunLauncher's existing, unchanged behavior (same order of operations there), so it's pre-existing risk being extended rather than a new regression — but worth a follow-up (e.g. a cleanup on CR-creation failure, or a TTL/reconciler sweep for orphaned -identity Secrets) now that a second launcher shares the pattern.

Security

The core design is sound: no shared bot credential, spawn (never a shell string) so the allowlisted argv can't be reinterpreted, no persisted gh config, redaction on the failure path, and the explicit allowlist-not-blocklist stance on top-level commands (correctly reasoned as defense-in-depth rather than the primary boundary, since the real boundary is the delegated human's own GitHub permissions). The Go/TS/Helm plumbing for secretEnv/identityProviders looks consistent with the existing AgentRunSpec/AgentSpec shapes it's mirroring, and the new Go controller test (toolrun_controller_test.go) correctly verifies ToolRun-level secretEnv wins over the Tool's static entry.

Overall: good, well-documented extension of an existing pattern. The identityGatewayFor bypass is the one thing I'd want fixed before this lands, since it undermines the provider-agnostic design the surrounding code (and this PR's own ADR) calls out as the point of that helper.

[k5s-bot]

Thanks for the thorough review — addressed the feedback in 78c1455, and the branch is now merged up to latest main (efaeb36, no conflicts):

Correctness — graph.ts container-Tool identity gate bypassed the provider-aware resolver (the blocker).
Fixed. The tool.jobTemplate branch in runTool no longer hard-codes deps.identityLinkGateway/state.identity.subject. It now resolves credentials through the same AuthorizationService.resolveLinkedCredentials() the agent-backed-tool branch uses, so gateway selection is provider-aware (claude → claudeAuthGateway, everything else → identityLinkGateway) and the credential subject is keyed identically (canonical principal for cross-entry-point providers). This also resolves a merge fallout where the branch still referenced PROVIDER_ENV_VAR, which main had moved out of graph.ts. New graph.test.ts case proves a container Tool declaring identityProviders: ["claude"] reads its token from claudeAuthGateway and launches with the Claude token, and that identityLinkGateway is never queried for "claude".

Test coverage — tools/github only shipped allowlist.test.ts.
Added the missing unit tests: redact()/clip() (every gh token prefix, Bearer/token headers, and the redact-before-truncate path), runGh() (stdout, non-zero exit, the SIGKILL timeout-kill path, and token env wiring), and config.ts env parsing/fallbacks. tools/github now runs 32 tests.

Nits:

• tools/github/README.md ADR reference — corrected. (After merging main, the ADR renumbered to 0032; the README and all cross-references now cite 0032 consistently.)
• redact.ts SECRET_PATTERNS — collapsed the gh[opsu]_ + separate ghr_ patterns into a single gh[oprsu]_.
• ToolRunLauncher orphaned--identity-Secret-on-CR-creation-failure — left as a follow-up as you suggested, since it mirrors AgentRunLauncher's existing unchanged behavior (pre-existing risk being extended, not a new regression). Worth a dedicated cleanup/TTL-sweep PR for both launchers.

All npm workspace typecheck + test pass (agent-orchestrator 463, tools/github 32); Go controller build/vet/gofmt clean and make test (envtest) passes.

[k5s-bot]

Does this PR need refactoring after the recent OAuth-delegation changes? Short answer: it was already correct — but there was one duplication worth removing, now pushed as ed67e30.

I checked this PR against the delegation code currently on main:

• The PR is already aligned with the refactor. The AuthorizationService extraction (0c332b5, ADR 0030 §1) and the credential-convergence work landed on main while this PR was in review, and the PR merged them in. apps/agent-orchestrator/src/agent/authorization-service.ts is now byte-identical between this branch and main — the PR consumes resolveLinkedCredentials() rather than forking it, and the review commit 78c1455 already switched the container-tool gate off the hard-coded identityLinkGateway onto that provider-aware path. The 7 commits main has gained since this branch's merge-base don't touch the delegation core (they're integration-gateway routes, e2e specs, and helm values). So nothing here is stale or broken — the branch is MERGEABLE.

• The one real cleanup the refactor invited: resolveLinkedCredentials() returns a four-kind discriminated union, and runTool was mapping those four kinds to user-facing errors twice, verbatim — once in the agent-backed-tool branch (which came from main's refactor) and once in the container-tool branch (which this PR added). The two copies differed only in indentation and the "who to link with" hint string.

ed67e30 collapses both onto a single documented helper, resolveToolIdentitySecretEnv, parameterizing only the caller-specific hint via a linkHint(provider) builder. The container branch keeps its identityProviders-present guard (a container Tool declaring no providers needs no caller identity). Behavior is unchanged — resolveLinkedCredentials already treats an empty provider list as a no-op resolved, and every asserted error substring is preserved.

Verification: npm run typecheck clean; full agent-orchestrator suite green (463 tests, including the existing graph / toolrun-launcher / crd-tool-registry identity cases). Net -60/+72 in graph.ts (the + is the helper's doc comment; the logic shrank).

If you'd rather keep the two branches spelled out inline, this commit is safe to drop — it's purely a DRY pass, not a correctness fix.

🤖 Generated with Claude Code