Skip to content

Comments

fix: escape file path in read_file fallback to prevent shell injection#209

Open
haosenwang1018 wants to merge 1 commit intoConway-Research:mainfrom
haosenwang1018:fix/read-file-fallback-shell-injection
Open

fix: escape file path in read_file fallback to prevent shell injection#209
haosenwang1018 wants to merge 1 commit intoConway-Research:mainfrom
haosenwang1018:fix/read-file-fallback-shell-injection

Conversation

@haosenwang1018
Copy link
Contributor

Summary

Problem: The read_file tool's fallback path constructs a shell command by interpolating the file path directly without escaping:

const result = await ctx.conway.exec(`cat ${filePath}`, 30_000);

When the primary Conway readFile API fails and this fallback triggers, a file path containing shell metacharacters (;, $(), backticks, etc.) can execute arbitrary commands. This bypasses the sensitive file read protection — for example, a path like nonexistent; cat ~/.automaton/wallet.json would pass the basename check but execute cat nonexistent; cat ~/.automaton/wallet.json in the shell.

Why it matters: This is a defense-in-depth gap. Although the automaton's own model generates the path, a prompt injection attack could cause the model to construct a malicious path that exploits the unescaped shell interpolation to exfiltrate secrets (wallet keys, .env, automaton.json).

What changed: Used the existing escapeShellArg() function (already defined at line 2523 of the same file, used by createInstalledToolExecutor) to properly single-quote the file path in the fallback cat command. Inside single quotes, all shell metacharacters (;, $(), `, |, &&, etc.) are treated as literal characters.

What did NOT change: The primary readFile API path, the sensitive file basename check, or the escapeShellArg function itself.

Change Type

  • Bug fix

Security Impact

  • New permissions? No
  • Secrets changed? No
  • New network calls? No

Verification

Added 4 regression tests to tools-security.test.ts:

  • Shell metacharacters (spaces) are properly quoted
  • Semicolon injection is neutralized by quoting
  • Single quotes in paths are escaped with '\'' technique
  • $() subshell syntax is treated as literal text

All 68 tests pass (64 existing + 4 new).

The read_file tool's fallback path interpolated the file path directly
into a shell command (`cat ${filePath}`) without escaping. When the
Conway readFile API fails and the fallback triggers, a crafted path
containing shell metacharacters (e.g., `; cat wallet.json` or
`$(whoami)`) could execute arbitrary commands and bypass the sensitive
file read protection.

The fix uses the existing `escapeShellArg()` function (already defined
in the same file) to properly single-quote the path, neutralizing
semicolons, subshell syntax, and other shell metacharacters.

Added 4 regression tests covering spaces, semicolons, single quotes,
and $() subshell syntax in file paths.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant