diff --git a/src/api/routes/github-merge.js b/src/api/routes/github-merge.js new file mode 100644 index 0000000..7b28d39 --- /dev/null +++ b/src/api/routes/github-merge.js @@ -0,0 +1,301 @@ +/** + * GitHub PR Merge — governed, internally-authenticated connection action. + * + * Implements POST /integrations/github/merge-pr. + * + * This is the CONNECTION layer performing an authenticated GitHub merge on + * behalf of an internally-authenticated caller (the autoassist executor). + * The App installation token is minted/cached here and NEVER leaves this + * layer — the caller holds no GitHub credential. ChittyConnect does NOT + * decide maintainer policy; the calling executor already re-verifies the + * full merge gate at execution time. This endpoint simply executes the + * merge the executor has authorized, with a TOCTOU guard so a stale + * decision cannot merge a moved head. + * + * Auth: internal-only. Requires the shared internal secret presented as + * `X-Webhook-Secret` (same INTERNAL_WEBHOOK_SECRET used by the webhook + * router's outbound forwarding). Validated with a constant-time compare and + * failed closed if the secret is unset. This endpoint is NOT public. + * + * @canonical-uri chittycanon://core/services/chittyconnect/api/routes/github-merge + * @canon chittycanon://gov/governance#core-types + */ + +import { Hono } from "hono"; +import { getCachedInstallationToken, getInstallationIdForRepo } from "../../auth/github.js"; + +const githubMergeRoutes = new Hono(); + +const VALID_MERGE_METHODS = new Set(["squash", "merge", "rebase"]); + +/** + * Constant-time string comparison (length-independent leak only). + * @param {string} a + * @param {string} b + * @returns {boolean} + */ +function timingSafeEqual(a, b) { + if (typeof a !== "string" || typeof b !== "string") return false; + const enc = new TextEncoder(); + const ba = enc.encode(a); + const bb = enc.encode(b); + // Compare against a fixed-length accumulator; differing lengths still + // run the full loop over the longer buffer to avoid early-exit timing. + const len = Math.max(ba.length, bb.length); + let diff = ba.length ^ bb.length; + for (let i = 0; i < len; i++) { + diff |= (ba[i] ?? 0) ^ (bb[i] ?? 0); + } + return diff === 0; +} + +/** + * POST /integrations/github/merge-pr + * + * Request body: + * { + * "repo": "owner/name", + * "pr_number": 123, + * "merge_method": "squash" | "merge" | "rebase" (default "squash") + * "expected_head_sha": "" (optional TOCTOU guard) + * } + * + * Response 200: + * { "merged": true, "sha": "", "message": "...", "status": 200 } + * + * Error JSON (fail-safe) on any failure with appropriate status. + */ +githubMergeRoutes.post("/merge-pr", async (c) => { + // --- Internal auth: fail closed --- + const expectedSecret = c.env.INTERNAL_WEBHOOK_SECRET; + if (!expectedSecret) { + return c.json( + { + merged: false, + error: { + code: "POLICY_BLOCKED_INTERNAL_SECRET_UNSET", + message: "INTERNAL_WEBHOOK_SECRET is not configured; refusing merge", + }, + status: 503, + }, + 503, + ); + } + const presented = c.req.header("X-Webhook-Secret"); + if (!presented || !timingSafeEqual(presented, expectedSecret)) { + return c.json( + { + merged: false, + error: { + code: "UNAUTHORIZED", + message: "Invalid or missing internal secret", + }, + status: 401, + }, + 401, + ); + } + + // --- Parse + validate body --- + let body; + try { + body = await c.req.json(); + } catch { + return c.json( + { + merged: false, + error: { code: "INVALID_JSON", message: "Body must be valid JSON" }, + status: 400, + }, + 400, + ); + } + + const { repo, pr_number, merge_method, expected_head_sha } = body || {}; + + // repo must be exactly "owner/name": two non-empty segments, each limited to + // GitHub's identifier charset. A permissive check (e.g. includes("/")) lets + // "owner/name/extra", "/name", or "owner/" through, and split("/", 2) would + // silently drop trailing segments — risking a merge against the wrong repo. + const REPO_RE = /^[A-Za-z0-9](?:[A-Za-z0-9-]*[A-Za-z0-9])?\/[A-Za-z0-9._-]+$/; + if (!repo || typeof repo !== "string" || !REPO_RE.test(repo)) { + return c.json( + { + merged: false, + error: { + code: "MISSING_REQUIRED_FIELDS", + message: 'repo must be exactly "owner/name"', + }, + status: 400, + }, + 400, + ); + } + if (!Number.isInteger(pr_number) || pr_number <= 0) { + return c.json( + { + merged: false, + error: { + code: "MISSING_REQUIRED_FIELDS", + message: "pr_number must be a positive integer", + }, + status: 400, + }, + 400, + ); + } + const method = merge_method ?? "squash"; + if (!VALID_MERGE_METHODS.has(method)) { + return c.json( + { + merged: false, + error: { + code: "INVALID_MERGE_METHOD", + message: `merge_method must be one of: ${[...VALID_MERGE_METHODS].join(", ")}`, + }, + status: 400, + }, + 400, + ); + } + + // expected_head_sha is the TOCTOU guard — only meaningful if it's a real + // git object id. Reject non-string/whitespace early rather than forwarding a + // malformed guard to GitHub where it would fail less predictably. + if ( + expected_head_sha !== undefined && + (typeof expected_head_sha !== "string" || + !/^[0-9a-fA-F]{7,40}$/.test(expected_head_sha)) + ) { + return c.json( + { + merged: false, + error: { + code: "INVALID_EXPECTED_HEAD_SHA", + message: "expected_head_sha must be a 7-40 char hex git sha", + }, + status: 400, + }, + 400, + ); + } + + const [owner, name] = repo.split("/", 2); + + try { + // --- Mint/get installation token (never leaves this layer) --- + const installationId = await getInstallationIdForRepo(c.env, owner, name); + const token = await getCachedInstallationToken(c.env, installationId); + + // --- Execute GitHub merge with TOCTOU sha guard --- + const mergeBody = { merge_method: method }; + if (expected_head_sha) { + // GitHub rejects (409) if the PR head has moved since the executor's + // decision — this is the TOCTOU guard. + mergeBody.sha = expected_head_sha; + } + + const ghResp = await fetch( + `https://api.github.com/repos/${owner}/${name}/pulls/${pr_number}/merge`, + { + method: "PUT", + headers: { + Authorization: `token ${token}`, + Accept: "application/vnd.github+json", + "User-Agent": "ChittyConnect/1.0", + "Content-Type": "application/json", + }, + body: JSON.stringify(mergeBody), + }, + ); + + const ghJson = await ghResp.json().catch(() => ({})); + + // Success: GitHub returns 200 with { merged: true, sha, message } + if (ghResp.ok && ghJson.merged) { + return c.json( + { + merged: true, + sha: ghJson.sha, + message: ghJson.message, + status: ghResp.status, + }, + 200, + ); + } + + // 405 can mean BOTH "already merged" and "not mergeable". 409 means the + // base/head moved (incl. the sha TOCTOU guard rejection). Do NOT assume + // 405 == already-merged; confirm via the PR's merged state before + // reporting success, otherwise surface the failure. + if (ghResp.status === 405 || ghResp.status === 409) { + const prResp = await fetch( + `https://api.github.com/repos/${owner}/${name}/pulls/${pr_number}`, + { + headers: { + Authorization: `token ${token}`, + Accept: "application/vnd.github+json", + "User-Agent": "ChittyConnect/1.0", + }, + }, + ); + const prJson = await prResp.json().catch(() => ({})); + if (prResp.ok && prJson.merged === true) { + // Idempotent success: already merged. + return c.json( + { + merged: true, + sha: prJson.merge_commit_sha, + message: "Pull request already merged", + status: 200, + }, + 200, + ); + } + + return c.json( + { + merged: false, + error: { + code: + ghResp.status === 409 + ? "HEAD_MOVED_OR_NOT_MERGEABLE" + : "NOT_MERGEABLE", + message: + ghJson.message || + "GitHub refused the merge (head moved or PR not mergeable)", + }, + status: ghResp.status, + }, + ghResp.status, + ); + } + + // Any other non-OK status from GitHub: surface it. + return c.json( + { + merged: false, + error: { + code: "GITHUB_MERGE_FAILED", + message: ghJson.message || `GitHub merge failed (${ghResp.status})`, + }, + status: ghResp.status, + }, + ghResp.status === 0 ? 502 : ghResp.status, + ); + } catch (error) { + return c.json( + { + merged: false, + error: { + code: "MERGE_EXECUTION_ERROR", + message: error?.message || "Unexpected error executing merge", + }, + status: 500, + }, + 500, + ); + } +}); + +export { githubMergeRoutes }; diff --git a/src/auth/github.js b/src/auth/github.js index cda28fc..4d444f7 100755 --- a/src/auth/github.js +++ b/src/auth/github.js @@ -66,6 +66,57 @@ export async function getInstallationToken(installationId, appJwt) { }; } +/** + * Resolve the App installation id that covers a given repository. + * + * Uses GitHub's native repo→installation resolver + * (`GET /repos/{owner}/{repo}/installation`) authenticated with the App JWT. + * This is schema-independent (no dependency on the local `installations` + * table) and is the canonical way to find which installation an action + * should be performed under when the caller only knows `owner/name`. + * + * @param {object} env - Worker env with GITHUB_APP_ID, GITHUB_APP_PK + * @param {string} owner - Repository owner (org or user login) + * @param {string} repo - Repository name + * @returns {Promise} Installation id + */ +export async function getInstallationIdForRepo(env, owner, repo) { + // Exported helper — validate inputs and encode them into the URL so callers + // passing whitespace or unexpected characters can't produce a malformed URL + // or an unintended request path. + const cleanOwner = typeof owner === "string" ? owner.trim() : ""; + const cleanRepo = typeof repo === "string" ? repo.trim() : ""; + if (!cleanOwner || !cleanRepo) { + throw new Error("getInstallationIdForRepo requires non-empty owner and repo"); + } + const appJwt = await generateAppJWT(env.GITHUB_APP_ID, env.GITHUB_APP_PK); + const response = await fetch( + `https://api.github.com/repos/${encodeURIComponent(cleanOwner)}/${encodeURIComponent(cleanRepo)}/installation`, + { + headers: { + Authorization: `Bearer ${appJwt}`, + Accept: "application/vnd.github+json", + "User-Agent": "ChittyConnect/1.0", + }, + }, + ); + + if (!response.ok) { + const error = await response.text(); + throw new Error( + `Installation lookup failed for ${owner}/${repo} (${response.status}): ${error}`, + ); + } + + const data = await response.json(); + if (!data?.id) { + throw new Error( + `Installation lookup for ${owner}/${repo} returned no installation id`, + ); + } + return data.id; +} + /** * Get cached installation token or mint a new one * @param {object} env - Worker environment with TOKEN_KV, GITHUB_APP_ID, GITHUB_APP_PK diff --git a/src/index.js b/src/index.js index 0b3c87f..fe8a92d 100755 --- a/src/index.js +++ b/src/index.js @@ -1501,6 +1501,7 @@ app.post("/intelligence/relationships/discover", async (c) => { import { discoveryRoutes } from "./api/routes/discovery.js"; import { githubActionsRoutes } from "./api/routes/github-actions.js"; import { gitConfirmRoutes } from "./api/routes/git-confirm.js"; +import { githubMergeRoutes } from "./api/routes/github-merge.js"; app.route("/.well-known", discoveryRoutes); @@ -1518,6 +1519,15 @@ app.route("/api/github-actions", githubActionsRoutes); */ app.route("/api/git", gitConfirmRoutes); +/** + * GitHub PR merge — governed, internally-authenticated connection action. + * POST /integrations/github/merge-pr. The App installation token is minted + * and used entirely within ChittyConnect; the calling executor (autoassist) + * holds no GitHub credential and decides maintainer policy itself. Internal + * secret (X-Webhook-Secret) authenticated inside the route; fails closed. + */ +app.route("/integrations/github", githubMergeRoutes); + /** * SSE Health check endpoint */ diff --git a/tests/api/github-merge-routes.test.js b/tests/api/github-merge-routes.test.js new file mode 100644 index 0000000..42cbd91 --- /dev/null +++ b/tests/api/github-merge-routes.test.js @@ -0,0 +1,228 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; +import { Hono } from "hono"; +import { githubMergeRoutes } from "../../src/api/routes/github-merge.js"; + +/** + * Exercises the real route handler end-to-end through Hono's request + * pipeline. The only thing stubbed is the GitHub HTTP boundary (global + * fetch) — no live API calls, no mocking of internal service modules. + * The App private key is a real RS256 PKCS#8 key generated per-suite so + * generateAppJWT() runs for real. + */ + +const SECRET = "internal-secret-abc123"; + +let appPkPem; + +function makeApp(envOverrides = {}) { + const app = new Hono(); + app.use("*", async (c, next) => { + c.env = { + INTERNAL_WEBHOOK_SECRET: SECRET, + GITHUB_APP_ID: "123456", + GITHUB_APP_PK: appPkPem, + TOKEN_KV: { + get: vi.fn().mockResolvedValue(null), + put: vi.fn().mockResolvedValue(undefined), + }, + ...envOverrides, + }; + await next(); + }); + app.route("/integrations/github", githubMergeRoutes); + return app; +} + +function req(body, headers = {}) { + return new Request("http://local/integrations/github/merge-pr", { + method: "POST", + headers: { "Content-Type": "application/json", ...headers }, + body: JSON.stringify(body), + }); +} + +beforeEach(async () => { + // Generate a real RS256 PKCS#8 key so generateAppJWT works for real. + const { generateKeyPair, exportPKCS8 } = await import("jose"); + const { privateKey } = await generateKeyPair("RS256", { extractable: true }); + appPkPem = await exportPKCS8(privateKey); +}); + +afterEach(() => { + vi.restoreAllMocks(); +}); + +describe("POST /integrations/github/merge-pr — auth", () => { + it("rejects when internal secret header is missing", async () => { + const app = makeApp(); + const res = await app.request( + req({ repo: "CHITTYOS/x", pr_number: 1 }), + ); + expect(res.status).toBe(401); + const body = await res.json(); + expect(body.merged).toBe(false); + expect(body.error.code).toBe("UNAUTHORIZED"); + }); + + it("rejects when internal secret is wrong", async () => { + const app = makeApp(); + const res = await app.request( + req({ repo: "CHITTYOS/x", pr_number: 1 }, { "X-Webhook-Secret": "nope" }), + ); + expect(res.status).toBe(401); + }); + + it("fails closed when INTERNAL_WEBHOOK_SECRET is unset", async () => { + const app = makeApp({ INTERNAL_WEBHOOK_SECRET: undefined }); + const res = await app.request( + req({ repo: "CHITTYOS/x", pr_number: 1 }, { "X-Webhook-Secret": SECRET }), + ); + expect(res.status).toBe(503); + const body = await res.json(); + expect(body.error.code).toBe("POLICY_BLOCKED_INTERNAL_SECRET_UNSET"); + }); +}); + +describe("POST /integrations/github/merge-pr — validation", () => { + it("rejects missing repo", async () => { + const app = makeApp(); + const res = await app.request( + req({ pr_number: 1 }, { "X-Webhook-Secret": SECRET }), + ); + expect(res.status).toBe(400); + expect((await res.json()).error.code).toBe("MISSING_REQUIRED_FIELDS"); + }); + + it("rejects invalid merge_method", async () => { + const app = makeApp(); + const res = await app.request( + req( + { repo: "CHITTYOS/x", pr_number: 1, merge_method: "fast-forward" }, + { "X-Webhook-Secret": SECRET }, + ), + ); + expect(res.status).toBe(400); + expect((await res.json()).error.code).toBe("INVALID_MERGE_METHOD"); + }); +}); + +describe("POST /integrations/github/merge-pr — merge execution", () => { + /** + * Stub the GitHub HTTP boundary. Routes the three GitHub calls: + * - GET .../installation -> { id } + * - POST .../access_tokens -> { token, expires_at } + * - PUT .../merge -> caller-supplied mergeResult + * - GET .../pulls/{n} -> caller-supplied prResult (idempotency check) + */ + function stubGitHub({ mergeStatus, mergeJson, prStatus, prJson, captureMerge }) { + return vi.spyOn(globalThis, "fetch").mockImplementation(async (url, init) => { + const u = String(url); + if (u.endsWith("/installation")) { + return new Response(JSON.stringify({ id: 99 }), { status: 200 }); + } + if (u.endsWith("/access_tokens")) { + return new Response( + JSON.stringify({ + token: "ghs_test", + expires_at: new Date(Date.now() + 3600_000).toISOString(), + }), + { status: 201 }, + ); + } + if (u.endsWith("/merge") && init?.method === "PUT") { + if (captureMerge) captureMerge(JSON.parse(init.body)); + return new Response(JSON.stringify(mergeJson), { + status: mergeStatus, + }); + } + // GET PR (idempotency / TOCTOU check) + return new Response(JSON.stringify(prJson ?? {}), { + status: prStatus ?? 200, + }); + }); + } + + it("merges with squash default and passes expected_head_sha as TOCTOU guard", async () => { + let mergeBody; + stubGitHub({ + mergeStatus: 200, + mergeJson: { merged: true, sha: "mergesha", message: "Pull Request successfully merged" }, + captureMerge: (b) => (mergeBody = b), + }); + const app = makeApp(); + const res = await app.request( + req( + { + repo: "CHITTYOS/x", + pr_number: 7, + expected_head_sha: "a1b2c3d4e5f60718293a4b5c6d7e8f9012345678", + }, + { "X-Webhook-Secret": SECRET }, + ), + ); + expect(res.status).toBe(200); + const body = await res.json(); + expect(body).toMatchObject({ merged: true, sha: "mergesha" }); + expect(mergeBody).toEqual({ + merge_method: "squash", + sha: "a1b2c3d4e5f60718293a4b5c6d7e8f9012345678", + }); + }); + + it("returns merged:true idempotently when GitHub 405s but PR is already merged", async () => { + stubGitHub({ + mergeStatus: 405, + mergeJson: { message: "Pull Request is not mergeable" }, + prStatus: 200, + prJson: { merged: true, merge_commit_sha: "already" }, + }); + const app = makeApp(); + const res = await app.request( + req({ repo: "CHITTYOS/x", pr_number: 7 }, { "X-Webhook-Secret": SECRET }), + ); + expect(res.status).toBe(200); + const body = await res.json(); + expect(body).toMatchObject({ merged: true, sha: "already" }); + }); + + it("surfaces failure (not merged:true) when 405 and PR is NOT merged", async () => { + stubGitHub({ + mergeStatus: 405, + mergeJson: { message: "Pull Request is not mergeable" }, + prStatus: 200, + prJson: { merged: false }, + }); + const app = makeApp(); + const res = await app.request( + req({ repo: "CHITTYOS/x", pr_number: 7 }, { "X-Webhook-Secret": SECRET }), + ); + expect(res.status).toBe(405); + const body = await res.json(); + expect(body.merged).toBe(false); + expect(body.error.code).toBe("NOT_MERGEABLE"); + }); + + it("surfaces 409 when head moved (TOCTOU guard tripped) and PR not merged", async () => { + stubGitHub({ + mergeStatus: 409, + mergeJson: { message: "Head branch was modified. Review and try the merge again." }, + prStatus: 200, + prJson: { merged: false }, + }); + const app = makeApp(); + const res = await app.request( + req( + { + repo: "CHITTYOS/x", + pr_number: 7, + expected_head_sha: "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef", + }, + { "X-Webhook-Secret": SECRET }, + ), + ); + expect(res.status).toBe(409); + const body = await res.json(); + expect(body.merged).toBe(false); + expect(body.error.code).toBe("HEAD_MOVED_OR_NOT_MERGEABLE"); + }); +});