Skip to content

fix(inline-tools): escape app in pressKey tool to prevent AppleScript injection - #994

Merged
sonichi merged 1 commit into
sonichi:mainfrom
VasiliyRad:fix/inline-tools-press-key-app-injection
May 21, 2026
Merged

sonichi merged 1 commit into
sonichi:mainfrom
VasiliyRad:fix/inline-tools-press-key-app-injection

Conversation

@VasiliyRad

Copy link
Copy Markdown
Contributor

The bug (security)

pressKeyTool in src/inline-tools.ts accepted an optional app parameter and embedded it raw in an AppleScript command:

```ts
if (app) {
execSync(`osascript -e 'tell application "${app}" to activate'`, ...);
}
```

A value containing \" breaks out of the AppleScript string literal and injects arbitrary AppleScript. AppleScript supports do shell script \"...\", so this is arbitrary code execution from a tool-call argument.

PoC (would have worked pre-fix):

```ts
pressKey({
key: 'space',
app: '"); do shell script "rm -rf ~/sensitive"; tell application "Finder',
})
```

The resulting AppleScript runs do shell script \"rm -rf ~/sensitive\".

Why this is the right fix

Two other call sites in the same file already use the correct escape pattern:

Line Tool Escape var
~161 pressKeyTool (key fallback path) safeKey
~214 switchAppTool.execute safeApp

Only pressKeyTool's app-activation branch was missing the escape. Apply the same 3-step strip (backslash, single-quote, double-quote). No new abstraction needed — local fix where the bug lives, matches the established in-file pattern.

Tests

tests/inline-tools-press-key-app-escape.test.ts — 4 source-grep architectural assertions:

Case Pins
raw tell application \"\${app}\" pattern absent Pre-fix pattern fails here.
pressKey branch references safeApp Post-fix shape.
safeApp computed via .replace() chain All 3 strips present.
Chain includes \" strip The actual exploit vector defense.

Source-grep is the right shape — the actual execSync call spawns osascript and is OS-dependent; the security contract is "escape app before embedding," which grep verifies precisely.

All 4 pass locally. npm run typecheck clean.

🤖 Generated with Claude Code

…pt injection

## The bug (security)

`pressKeyTool` in `src/inline-tools.ts` accepted an optional `app`
parameter and embedded it raw in an AppleScript command:

  if (app) {
    execSync(`osascript -e 'tell application "${app}" to activate'`, ...);
  }

A value containing `"` breaks out of the AppleScript string literal
and injects arbitrary AppleScript. AppleScript supports
`do shell script "..."`, so this is arbitrary code execution from a
tool-call argument.

PoC (pre-fix):

  pressKey({
    key: 'space',
    app: '"); do shell script "rm -rf ~/sensitive"; tell application "Finder',
  })

Resulting AppleScript:

  tell application """); do shell script "rm -rf ~/sensitive"; tell application "Finder" to activate

…runs the shell command.

## Why this is the right fix

Two other call sites in the same file already use the correct escape
pattern (`safeKey` at line ~161, `safeApp` in switchAppTool around
line ~214). Only `pressKeyTool`'s app-activation branch was missing
the escape. Apply the same 3-step strip (backslash, single-quote,
double-quote). No new abstraction needed — local fix where the bug
lives, matches the established in-file pattern.

## Tests

`tests/inline-tools-press-key-app-escape.test.ts` — 4 source-grep
architectural assertions:

  - raw `tell application "${app}"` pattern absent (pre-fix pattern
    fails here)
  - pressKey branch references `safeApp`
  - `safeApp` computed via `.replace()` chain — pins all 3 strips
  - Chain includes the `"` strip — the actual exploit-vector defense

Source-grep is the right shape: actual `execSync` call would spawn
osascript and is OS-dependent; the security contract is "escape `app`
before embedding," which grep verifies precisely.

All 4 pass locally. `npm run typecheck` clean.

Co-Authored-By: Vasiliy's Sutando <noreply@anthropic.com>
@CLAassistant

CLAassistant commented May 21, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@sonichi
sonichi merged commit 3f3275b into sonichi:main May 21, 2026
1 of 2 checks passed
@sonichi

sonichi commented May 21, 2026

Copy link
Copy Markdown
Owner

LGTM. Verified the bug: ${app} was interpolated raw into the osascript -e 'tell application "${app}" to activate' string; app: '"; do shell script "..."; tell application "Finder' would close out the AppleScript string literal and inject arbitrary AppleScript (which can do shell script → RCE from a tool-call argument).

The fix is the right shape — same triple-replace safeApp pattern already used by safeKey (line ~161) and switchAppTool (line ~214); reusing the established in-file canon rather than introducing a new abstraction. Order is correct: backslash → single-quote → double-quote, so later-inserted \ chars don't get double-escaped.

Test is a source-grep regression guard rather than a runtime test — appropriate for "future refactor must not re-introduce the raw interpolation," since runtime AppleScript tests need a real Mac with the target app.

liususan091219 pushed a commit to liususan091219/sutando-fork that referenced this pull request May 28, 2026
…pt injection (sonichi#994)

## The bug (security)

`pressKeyTool` in `src/inline-tools.ts` accepted an optional `app`
parameter and embedded it raw in an AppleScript command:

  if (app) {
    execSync(`osascript -e 'tell application "${app}" to activate'`, ...);
  }

A value containing `"` breaks out of the AppleScript string literal
and injects arbitrary AppleScript. AppleScript supports
`do shell script "..."`, so this is arbitrary code execution from a
tool-call argument.

PoC (pre-fix):

  pressKey({
    key: 'space',
    app: '"); do shell script "rm -rf ~/sensitive"; tell application "Finder',
  })

Resulting AppleScript:

  tell application """); do shell script "rm -rf ~/sensitive"; tell application "Finder" to activate

…runs the shell command.

## Why this is the right fix

Two other call sites in the same file already use the correct escape
pattern (`safeKey` at line ~161, `safeApp` in switchAppTool around
line ~214). Only `pressKeyTool`'s app-activation branch was missing
the escape. Apply the same 3-step strip (backslash, single-quote,
double-quote). No new abstraction needed — local fix where the bug
lives, matches the established in-file pattern.

## Tests

`tests/inline-tools-press-key-app-escape.test.ts` — 4 source-grep
architectural assertions:

  - raw `tell application "${app}"` pattern absent (pre-fix pattern
    fails here)
  - pressKey branch references `safeApp`
  - `safeApp` computed via `.replace()` chain — pins all 3 strips
  - Chain includes the `"` strip — the actual exploit-vector defense

Source-grep is the right shape: actual `execSync` call would spawn
osascript and is OS-dependent; the security contract is "escape `app`
before embedding," which grep verifies precisely.

All 4 pass locally. `npm run typecheck` clean.

Co-authored-by: Vasiliy's Sutando <noreply@anthropic.com>
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.

3 participants