diff --git a/entry b/entry index 9b20ac8..86152b6 100755 --- a/entry +++ b/entry @@ -119192,9 +119192,6 @@ function filterEnv(isPublicRepo) { if (isPublicRepo && isSensitive(key)) continue; filtered[key] = value2; } - if (process.env.ORIGINAL_GITHUB_TOKEN) { - filtered.GITHUB_TOKEN = process.env.ORIGINAL_GITHUB_TOKEN; - } return filtered; } function spawnSandboxed(params) { @@ -120772,10 +120769,11 @@ var PushBranch = type({ branchName: type.string.describe("The branch name to push (defaults to current branch)").optional(), force: type.boolean.describe("Force push (use with caution)").default(false) }); -function PushBranchTool(_ctx) { +function PushBranchTool(ctx) { + const defaultBranch = ctx.repo.repo.default_branch || "main"; return tool({ name: "push_branch", - description: "Push the current branch (or specified branch) to the remote repository. Git automatically determines the correct remote based on branch config (set by checkout_pr for fork PRs). Never force push unless explicitly requested.", + description: "Push the current branch (or specified branch) to the remote repository. Git automatically determines the correct remote based on branch config (set by checkout_pr for fork PRs). Never force push unless explicitly requested. Pushes to the default branch are blocked.", parameters: PushBranch, execute: execute(async ({ branchName, force }) => { const branch = branchName || $("git", ["rev-parse", "--abbrev-ref", "HEAD"], { log: false }); @@ -120790,6 +120788,11 @@ function PushBranchTool(_ctx) { remoteBranch = mergeRef.replace("refs/heads/", ""); } catch { } + if (remoteBranch === defaultBranch) { + throw new Error( + `Push blocked: cannot push directly to default branch '${remoteBranch}'. Create a feature branch and open a PR instead.` + ); + } const refspec = branch === remoteBranch ? branch : `${branch}:${remoteBranch}`; const args3 = force ? ["push", "--force", "-u", remote, refspec] : ["push", "-u", remote, refspec]; log.debug(`pushing ${branch} to ${remote}/${remoteBranch}`); @@ -140600,14 +140603,15 @@ async function setupGit(params) { } catch { log.debug("\xBB no existing authentication headers to remove"); } + const originToken = params.bashPermission === "enabled" ? params.token : params.originalToken || params.token; if (params.event.is_pr !== true || !params.event.issue_number) { - const originUrl2 = `https://x-access-token:${params.token}@github.com/${params.owner}/${params.name}.git`; + const originUrl2 = `https://x-access-token:${originToken}@github.com/${params.owner}/${params.name}.git`; $("git", ["remote", "set-url", "origin", originUrl2], { cwd: repoDir }); log.info("\xBB updated origin URL with authentication token"); return; } const prNumber = params.event.issue_number; - const originUrl = `https://x-access-token:${params.token}@github.com/${params.owner}/${params.name}.git`; + const originUrl = `https://x-access-token:${originToken}@github.com/${params.owner}/${params.name}.git`; $("git", ["remote", "set-url", "origin", originUrl], { cwd: repoDir }); const prContext = await checkoutPrBranch({ octokit: params.octokit, @@ -140693,6 +140697,8 @@ async function main() { }); await setupGit({ token: tokenRef.token, + originalToken: process.env.ORIGINAL_GITHUB_TOKEN, + bashPermission: payload.bash, owner: repo.owner, name: repo.name, event: payload.event, diff --git a/main.ts b/main.ts index a9a1e55..f83f188 100644 --- a/main.ts +++ b/main.ts @@ -62,6 +62,8 @@ export async function main(): Promise { await setupGit({ token: tokenRef.token, + originalToken: process.env.ORIGINAL_GITHUB_TOKEN, + bashPermission: payload.bash, owner: repo.owner, name: repo.name, event: payload.event, diff --git a/mcp/bash.ts b/mcp/bash.ts index 2b57b7c..3bdc077 100644 --- a/mcp/bash.ts +++ b/mcp/bash.ts @@ -30,11 +30,9 @@ function filterEnv(isPublicRepo: boolean): Record { if (isPublicRepo && isSensitive(key)) continue; filtered[key] = value; } - // restore original GITHUB_TOKEN (the one set by GitHub Actions, not our installation token) - // this allows git operations in subprocesses to work while keeping our installation token secure - if (process.env.ORIGINAL_GITHUB_TOKEN) { - filtered.GITHUB_TOKEN = process.env.ORIGINAL_GITHUB_TOKEN; - } + // never restore GITHUB_TOKEN - agents must use MCP tools for git/gh operations + // this ensures all git operations go through auditable MCP tools where we can + // enforce branch protection, scan for secrets, etc. return filtered; } diff --git a/mcp/git.ts b/mcp/git.ts index a97225a..76300c6 100644 --- a/mcp/git.ts +++ b/mcp/git.ts @@ -1,8 +1,8 @@ import { type } from "arktype"; -import type { ToolContext } from "./server.ts"; import { log } from "../utils/cli.ts"; import { containsSecrets } from "../utils/secrets.ts"; import { $ } from "../utils/shell.ts"; +import type { ToolContext } from "./server.ts"; import { execute, tool } from "./shared.ts"; export function CreateBranchTool(ctx: ToolContext) { @@ -141,11 +141,13 @@ export const PushBranch = type({ force: type.boolean.describe("Force push (use with caution)").default(false), }); -export function PushBranchTool(_ctx: ToolContext) { +export function PushBranchTool(ctx: ToolContext) { + const defaultBranch = ctx.repo.repo.default_branch || "main"; + return tool({ name: "push_branch", description: - "Push the current branch (or specified branch) to the remote repository. Git automatically determines the correct remote based on branch config (set by checkout_pr for fork PRs). Never force push unless explicitly requested.", + "Push the current branch (or specified branch) to the remote repository. Git automatically determines the correct remote based on branch config (set by checkout_pr for fork PRs). Never force push unless explicitly requested. Pushes to the default branch are blocked.", parameters: PushBranch, execute: execute(async ({ branchName, force }) => { const branch = branchName || $("git", ["rev-parse", "--abbrev-ref", "HEAD"], { log: false }); @@ -168,6 +170,14 @@ export function PushBranchTool(_ctx: ToolContext) { // no configured merge ref, use local branch name } + // block pushes to default branch + if (remoteBranch === defaultBranch) { + throw new Error( + `Push blocked: cannot push directly to default branch '${remoteBranch}'. ` + + `Create a feature branch and open a PR instead.` + ); + } + // use refspec when local and remote branch names differ const refspec = branch === remoteBranch ? branch : `${branch}:${remoteBranch}`; const args = force diff --git a/mcp/reviewComments.ts b/mcp/reviewComments.ts index 5187d65..1fe2546 100644 --- a/mcp/reviewComments.ts +++ b/mcp/reviewComments.ts @@ -8,6 +8,9 @@ import { execute, tool } from "./shared.ts"; // fragment for nested replyTo (5 levels deep covers most threads) // because in_reply_to_id generally points to the top-level comment // this doesn't actually work as expected +// TOD: implement an API endpoint with aggressive caching that fetches the PR's reviewThreads via graphql +// separately fetch all the comments associated with the particular review +// merge them, then construct the full thread context const REPLY_TO_FRAGMENT = ` replyTo { databaseId diff --git a/utils/setup.ts b/utils/setup.ts index a9f739d..8e58cce 100644 --- a/utils/setup.ts +++ b/utils/setup.ts @@ -2,7 +2,7 @@ import { execSync } from "node:child_process"; import { mkdtempSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import type { PayloadEvent } from "../external.ts"; +import type { BashPermission, PayloadEvent } from "../external.ts"; import { checkoutPrBranch } from "../mcp/checkout.ts"; import type { ToolState } from "../mcp/server.ts"; import { log } from "./cli.ts"; @@ -44,6 +44,8 @@ export function setupTestRepo(options: SetupOptions): void { interface SetupGitParams { token: string; + originalToken: string | undefined; + bashPermission: BashPermission; owner: string; name: string; event: PayloadEvent; @@ -127,9 +129,18 @@ export async function setupGit(params: SetupGitParams): Promise { log.debug("» no existing authentication headers to remove"); } + // choose token for origin based on bash permission: + // - enabled: installation token (full access) + // - restricted/disabled: workflow token (limited by permissions block) + // this protects the base repo while allowing fork PR edits via fork remote + const originToken = + params.bashPermission === "enabled" + ? params.token + : (params.originalToken || params.token); + // non-PR events: set up origin with token, stay on default branch if (params.event.is_pr !== true || !params.event.issue_number) { - const originUrl = `https://x-access-token:${params.token}@github.com/${params.owner}/${params.name}.git`; + const originUrl = `https://x-access-token:${originToken}@github.com/${params.owner}/${params.name}.git`; $("git", ["remote", "set-url", "origin", originUrl], { cwd: repoDir }); log.info("» updated origin URL with authentication token"); return; @@ -139,7 +150,7 @@ export async function setupGit(params: SetupGitParams): Promise { const prNumber = params.event.issue_number; // ensure origin is configured with auth token before checkout - const originUrl = `https://x-access-token:${params.token}@github.com/${params.owner}/${params.name}.git`; + const originUrl = `https://x-access-token:${originToken}@github.com/${params.owner}/${params.name}.git`; $("git", ["remote", "set-url", "origin", originUrl], { cwd: repoDir }); // use shared checkout helper (handles fork remotes, push config, etc.)