From c4d66bf7f62fa9f60bbeac58102ff7453527b5df Mon Sep 17 00:00:00 2001 From: Colin McDonnell Date: Thu, 26 Feb 2026 21:30:10 +0000 Subject: [PATCH] fix: block git in shell tool, add actionable errors for push rejections (#397) Agents were bypassing the git auth boundary by running `git pull/push` through the shell tool (which has no credentials). This caused auth failures and cascading issues (botched rebases, corrupted files). - shell: block all git commands with error directing to dedicated tools - push_branch: catch "fetch first" rejections with step-by-step recovery - git tool: improve auth redirect errors with specific tool guidance Made-with: Cursor --- entry | 48 ++++++++++++++++++++++++++++++++++++++---------- mcp/git.ts | 34 +++++++++++++++++++++++++--------- mcp/shell.ts | 21 ++++++++++++++++++++- 3 files changed, 83 insertions(+), 20 deletions(-) diff --git a/entry b/entry index a193561..88e14ee 100755 --- a/entry +++ b/entry @@ -142705,10 +142705,26 @@ function PushBranchTool(ctx) { if (force) { log.warning(`force pushing - this will overwrite remote history`); } - $git("push", pushArgs, { - token: ctx.gitToken, - restricted: ctx.payload.shell !== "enabled" - }); + try { + $git("push", pushArgs, { + token: ctx.gitToken, + restricted: ctx.payload.shell !== "enabled" + }); + } catch (err) { + const msg = err instanceof Error ? err.message : String(err); + if (msg.includes("fetch first") || msg.includes("non-fast-forward")) { + throw new Error( + `push rejected: the remote branch '${pushDest.remoteBranch}' has new commits you don't have locally. + +to resolve this: +1. use git_fetch to fetch the remote branch: git_fetch({ ref: "${pushDest.remoteBranch}" }) +2. use the git tool to rebase your changes: git({ subcommand: "rebase", args: ["origin/${pushDest.remoteBranch}"] }) +3. resolve any merge conflicts if needed +4. retry push_branch` + ); + } + throw err; + } return { success: true, branch, @@ -142721,10 +142737,10 @@ function PushBranchTool(ctx) { }); } var AUTH_REQUIRED_REDIRECT = { - push: "Use push_branch tool instead.", - fetch: "Use git_fetch tool instead.", - pull: "Use git_fetch + git merge instead.", - clone: "Repository already cloned. Use checkout_pr for PR branches." + push: "use the push_branch tool instead \u2014 it handles authentication and permission checks.", + fetch: "use the git_fetch tool instead \u2014 it handles authentication.", + pull: "use git_fetch to fetch the remote ref, then use this git tool with subcommand 'merge' or 'rebase' locally.", + clone: "the repository is already cloned. use checkout_pr for PR branches." }; var NOSHELL_BLOCKED_SUBCOMMANDS = { config: "Blocked: git config can set up filter drivers or hooks that execute arbitrary code.", @@ -142752,7 +142768,7 @@ function GitTool(ctx) { const args2 = params.args ?? []; const redirect = AUTH_REQUIRED_REDIRECT[subcommand]; if (redirect) { - throw new Error(`git ${subcommand} requires authentication. ${redirect}`); + throw new Error(`git ${subcommand} is not available through this tool \u2014 ${redirect}`); } if (ctx.payload.shell === "disabled") { const blocked = NOSHELL_BLOCKED_SUBCOMMANDS[subcommand]; @@ -144052,6 +144068,12 @@ function getTempDir() { } return tempDir; } +function isGitCommand(command) { + const trimmed = command.trim(); + if (trimmed === "git" || trimmed.startsWith("git ")) return true; + if (trimmed.startsWith("sudo git")) return true; + return /[;&|]\s*(?:sudo\s+)?git(?:\s|$)/.test(trimmed); +} function ShellTool(ctx) { return tool({ name: "shell", @@ -144061,9 +144083,15 @@ Use this tool to: - Run shell commands (ls, cat, grep, find, etc.) - Execute build tools (npm, pnpm, cargo, make, etc.) - Run tests and linters -- Perform git operations`, + +Do NOT use this tool for git commands \u2014 use the dedicated git tools instead.`, parameters: ShellParams, execute: execute(async (params) => { + if (isGitCommand(params.command)) { + throw new Error( + "git commands are not allowed in the shell tool. use the dedicated git tools instead:\n- git: local operations (status, log, diff, add, commit, checkout, merge, rebase, etc.)\n- push_branch: push to remote (handles authentication)\n- git_fetch: fetch from remote (handles authentication)\n- checkout_pr: check out PR branches" + ); + } const timeout = Math.min(params.timeout ?? 3e4, 12e4); const cwd = params.working_directory ?? process.cwd(); const env2 = resolveEnv(ctx.payload.shell === "enabled" ? "inherit" : "restricted"); diff --git a/mcp/git.ts b/mcp/git.ts index e4de9a6..eafcd5b 100644 --- a/mcp/git.ts +++ b/mcp/git.ts @@ -138,10 +138,26 @@ export function PushBranchTool(ctx: ToolContext) { if (force) { log.warning(`force pushing - this will overwrite remote history`); } - $git("push", pushArgs, { - token: ctx.gitToken, - restricted: ctx.payload.shell !== "enabled", - }); + + try { + $git("push", pushArgs, { + token: ctx.gitToken, + restricted: ctx.payload.shell !== "enabled", + }); + } catch (err) { + const msg = err instanceof Error ? err.message : String(err); + if (msg.includes("fetch first") || msg.includes("non-fast-forward")) { + throw new Error( + `push rejected: the remote branch '${pushDest.remoteBranch}' has new commits you don't have locally.\n\n` + + `to resolve this:\n` + + `1. use git_fetch to fetch the remote branch: git_fetch({ ref: "${pushDest.remoteBranch}" })\n` + + `2. use the git tool to rebase your changes: git({ subcommand: "rebase", args: ["origin/${pushDest.remoteBranch}"] })\n` + + `3. resolve any merge conflicts if needed\n` + + `4. retry push_branch` + ); + } + throw err; + } return { success: true, @@ -157,10 +173,10 @@ export function PushBranchTool(ctx: ToolContext) { // commands that require authentication - redirect to dedicated tools const AUTH_REQUIRED_REDIRECT: Record = { - push: "Use push_branch tool instead.", - fetch: "Use git_fetch tool instead.", - pull: "Use git_fetch + git merge instead.", - clone: "Repository already cloned. Use checkout_pr for PR branches.", + push: "use the push_branch tool instead — it handles authentication and permission checks.", + fetch: "use the git_fetch tool instead — it handles authentication.", + pull: "use git_fetch to fetch the remote ref, then use this git tool with subcommand 'merge' or 'rebase' locally.", + clone: "the repository is already cloned. use checkout_pr for PR branches.", }; // SECURITY: subcommands blocked when shell is disabled. @@ -219,7 +235,7 @@ export function GitTool(ctx: ToolContext) { const redirect = AUTH_REQUIRED_REDIRECT[subcommand]; if (redirect) { - throw new Error(`git ${subcommand} requires authentication. ${redirect}`); + throw new Error(`git ${subcommand} is not available through this tool — ${redirect}`); } // SECURITY: block dangerous subcommands when shell is disabled. diff --git a/mcp/shell.ts b/mcp/shell.ts index 4825279..11091df 100644 --- a/mcp/shell.ts +++ b/mcp/shell.ts @@ -160,6 +160,14 @@ function getTempDir(): string { return tempDir; } +/** detect git as a command invocation (not as part of another word like .gitignore) */ +function isGitCommand(command: string): boolean { + const trimmed = command.trim(); + if (trimmed === "git" || trimmed.startsWith("git ")) return true; + if (trimmed.startsWith("sudo git")) return true; + return /[;&|]\s*(?:sudo\s+)?git(?:\s|$)/.test(trimmed); +} + export function ShellTool(ctx: ToolContext) { return tool({ name: "shell", @@ -169,9 +177,20 @@ Use this tool to: - Run shell commands (ls, cat, grep, find, etc.) - Execute build tools (npm, pnpm, cargo, make, etc.) - Run tests and linters -- Perform git operations`, + +Do NOT use this tool for git commands — use the dedicated git tools instead.`, parameters: ShellParams, execute: execute(async (params) => { + if (isGitCommand(params.command)) { + throw new Error( + "git commands are not allowed in the shell tool. use the dedicated git tools instead:\n" + + "- git: local operations (status, log, diff, add, commit, checkout, merge, rebase, etc.)\n" + + "- push_branch: push to remote (handles authentication)\n" + + "- git_fetch: fetch from remote (handles authentication)\n" + + "- checkout_pr: check out PR branches" + ); + } + const timeout = Math.min(params.timeout ?? 30000, 120000); const cwd = params.working_directory ?? process.cwd(); const env = resolveEnv(ctx.payload.shell === "enabled" ? "inherit" : "restricted");