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
This commit is contained in:
committed by
pullfrog[bot]
parent
1d2a06998c
commit
c4d66bf7f6
@@ -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");
|
||||
|
||||
+25
-9
@@ -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<string, string> = {
|
||||
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.
|
||||
|
||||
+20
-1
@@ -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");
|
||||
|
||||
Reference in New Issue
Block a user