← Back to PRs

#12871: fix: use bash and warn about shell injection (issue #12836)

by ambicuity open 2026-02-09 20:42 View on GitHub →
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