fix: resolve AppleScript command injection and escape order bugs (#5)#10
Open
nandanadileep wants to merge 1 commit into
Open
fix: resolve AppleScript command injection and escape order bugs (#5)#10nandanadileep wants to merge 1 commit into
nandanadileep wants to merge 1 commit into
Conversation
ethanplusai#5) - Bug 1 & 2: Add _applescript_escape() helper in actions.py that escapes backslashes, double-quotes, and strips newlines before embedding user input into AppleScript do script / keystroke calls - Bug 3: Gate --dangerously-skip-permissions behind JARVIS_SKIP_PERMISSIONS env var across actions.py, qa.py, work_mode.py, and server.py; document the new var in .env.example - Bug 4: Fix escape order in mail_access.py search_mail() and read_message() so backslashes are replaced before double-quotes, preventing \" → \\"
4 tasks
Owner
|
Thanks @nandanadileep — your fix is correct and tests pass cleanly. I've taken this over in #11 to layer on a few additional escape sites we found while reviewing ( Your original commit is preserved as the first commit on the new branch with full authorship, and you're credited as Co-Authored-By on the follow-up commit. Closing this in favor of #11 — really appreciate the contribution and the surgical, well-scoped fix. 🙏 |
ethanplusai
added a commit
that referenced
this pull request
May 15, 2026
fix: AppleScript injection hardening (supersedes #10)
pull Bot
pushed a commit
to RealShocky/jarvis
that referenced
this pull request
May 15, 2026
Builds on the AppleScript injection fix from PR ethanplusai#10. While reviewing, found additional unescaped interpolation sites that should also use the shared escape helper: - _focus_terminal_window: was using the same incomplete quote-only escape pattern that PR ethanplusai#10 fixed elsewhere in actions.py - _run_task: {work_dir} interpolated raw into do script - handle_build: {path} interpolated raw into do script - "open last project" fallback: {last["path"]} interpolated raw into a Finder open POSIX file call - api_fix_self: {jarvis_dir} interpolated raw (low risk since derived from __file__, but escape for consistency) Also: - Renamed actions._applescript_escape -> actions.applescript_escape (drop underscore) so server.py can import it as a shared helper instead of duplicating the implementation - Added tests/test_applescript_escape.py covering backslash, quote, newline/CR, ordering, and a representative injection payload Note on upgrade behavior: PR ethanplusai#10 flips the default for --dangerously-skip-permissions from "always on" to "off unless JARVIS_SKIP_PERMISSIONS=true". Existing users who upgrade without setting that env var will see Claude permission prompts on each subprocess (which may appear to hang in JARVIS's voice-driven flow). Set JARVIS_SKIP_PERMISSIONS=true to preserve the previous behavior. Co-Authored-By: Nandana Dileep <nandanadileep29@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes all four vulnerabilities reported in issue #5.
open_terminal()injection: Added_applescript_escape()helper inactions.pythat escapes backslashes first, then double-quotes, and strips newlines/carriage returns before any user-controlled string is embedded in ado scriptAppleScript call.prompt_existing_terminal()injection: Same fix — bothproject_nameandpromptnow go through_applescript_escape()before use inkeystrokeand window-name matching.--dangerously-skip-permissionshardcoded: Removed all unconditional uses acrossactions.py,qa.py,work_mode.py, andserver.py. The flag is now only appended whenJARVIS_SKIP_PERMISSIONS=trueis set in the environment. Documented in.env.example.mail_access.py: Fixedsearch_mail()andread_message()to replace\before", preventing the\"→\\"double-escape corruption.Test plan
python3 -m pytest tests/test_e2e_pipeline.py tests/test_feedback_loop.py -vmain_applescript_escape()correctly handles backslashes, double-quotes, and newlinesJARVIS_SKIP_PERMISSIONSenv var controls the dangerous flag across all call sites