#12871: fix: use bash and warn about shell injection (issue #12836)
agents
stale
size: S
# PR Description: Fix Embedded Agent Script Generation (Issue #12836)
## Problem
The embedded agent was generating broken scripts due to:
1. **Incompatible Shell**: Defaulting to `/bin/sh` (which can be `dash` or other minimal shells) even when `bash` is available. This caused scripts using bash-specific syntax (e.g., `[[ ]]`, arrays, `set -o pipefail`) to fail.
2. **Shell Injection Risks**: Lack of explicit guidance in the system prompt regarding safe variable handling, leading to potential shell injection vulnerabilities when the agent attempts to interpolate variables directly into commands.
## Changes
- **`src/agents/shell-utils.ts`**: Updated `getShellConfig` to explicitly prefer `bash` if available on the system, falling back to `sh` only if `bash` is not found. Also restored specific invalidation logic for `fish` shell users to ensure they fall back to `bash` or `sh` properly.
- **`src/agents/system-prompt.ts`**: Updated the `exec` tool description to include a warning: _"use 'env' param for dynamic data to avoid shell injection"_. This instructs the agent to use environment variables for passing data safely.
## Verification
- **New Test**: Added `src/agents/shell-integration.test.ts` to verify that `getShellConfig` upgrades `sh` to `bash` when valid.
- **Regression Test**: `src/agents/shell-utils.test.ts` was updated/verified to ensure `fish` shell users still get a valid compatible shell.
- **Full Test Suite**: Ran `pnpm test` to ensure no regressions across the codebase.
<!-- greptile_comment -->
<h2>Greptile Overview</h2>
<h3>Greptile Summary</h3>
This PR updates shell selection so agent-generated scripts run under `bash` when available (avoiding `/bin/sh` incompatibilities), keeps special handling for `fish`, and adds a prompt warning to pass dynamic data via the exec tool’s `env` parameter to reduce shell injection risk.
Main concern is test quality/coverage: `src/agents/shell-integration.test.ts` currently includes tests that don’t assert (or only assert conditionally), so CI can pass even if the behavior regresses. Additionally, existing unit tests should be updated to reflect the new “prefer bash when SHELL is unset or sh” contract in a deterministic way by controlling `PATH`.
<h3>Confidence Score: 3/5</h3>
- This PR is close to mergeable, but test gaps mean regressions could slip through.
- The core code change is small and understandable, but the new integration test has non-asserting/conditional assertions and existing unit tests need updating for the new bash-preference behavior, reducing confidence that CI will reliably validate the fix across environments.
- src/agents/shell-integration.test.ts, src/agents/shell-utils.test.ts
<!-- greptile_other_comments_section -->
<sub>(3/5) Reply to the agent's comments like "Can you suggest a fix for this @greptileai?" or ask follow-up questions!</sub>
<!-- /greptile_comment -->
Most Similar PRs
#3872: improve bash-tools.exec.ts code quality
by Bestom927 · 2026-01-29
80.7%
#11300: feat(exec): make shell configurable via tools.exec.shell
by imjszhang · 2026-02-07
79.1%
#21271: fix(commands): pass channel/capabilities/shell/os to runtime in com...
by evansantos · 2026-02-19
78.5%
#15982: fix: pass agentId to resolveSessionFilePath in reply flow (NX-003)
by automagik-genie · 2026-02-14
78.3%
#14222: core: add needsApproval to before_tool_call; move AgentShield to ex...
by Eventedge · 2026-02-11
78.0%
#11961: fix: exec tool wraps shebang scripts in heredoc to use correct inte...
by scott-memco · 2026-02-08
77.9%
#14734: test(agents): guard against stale allowAgents in existing sessions
by davidahmann · 2026-02-12
77.7%
#7085: test: skip flaky workspace-paths & safe-bins tests on non-Linux/CI ...
by ThinkIbrokeIt · 2026-02-02
77.4%
#14701: Agent: enforce execute-first replies and clarify whisper modes
by Swader · 2026-02-12
77.2%
#11854: fix: resolve per-agent tools.exec config in pi-tools
by Yida-Dev · 2026-02-08
77.2%