🔒 Fix command injection vulnerability in file verification template - #9
🔒 Fix command injection vulnerability in file verification template#9Wenbobobo wants to merge 1 commit into
Conversation
Co-authored-by: Wenbobobo <[email protected]>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
There was a problem hiding this comment.
Pull request overview
Hardens the supervisor’s “changed file verification” step by removing bash -lc execution and instead invoking the verification command via an argument vector, reducing the risk of shell-based command injection via the {file} substitution.
Changes:
- Parse the verification template with
shlex.split()and run it viasubprocess.run(rendered_args)(no shell). - Replace
{file}placeholders in the parsed argv list with the actual path. - Return a structured failure when the template cannot be parsed.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| rendered_args = [arg.replace("{file}", str(file_path)) for arg in args] | ||
| result = subprocess.run( | ||
| ["bash", "-lc", rendered], | ||
| rendered_args, | ||
| cwd=str(workspace), |
| command = template or "timeout 30s lake env lean {file}" | ||
| rendered = command.format(file=shlex.quote(str(file_path))) | ||
| try: | ||
| args = shlex.split(command) | ||
| except ValueError as e: | ||
| return False, f"invalid verify template: {e}" |
🎯 What: The
⚠️ Risk: A malicious actor could inject harmful shell commands via the file path or an overridden
_verify_changed_filefunction previously executed a configurable verification template string throughbash -lc, which allowed for arbitrary command injection if an attacker could control or manipulate the template or file path.ARCHON_SUPERVISOR_VERIFY_TEMPLATEenvironment variable, leading to unauthorized code execution and complete compromise of the runtime environment.🛡️ Solution: Switched from executing the command via
bash -lcto splitting the template string securely usingshlex.split, safely substituting the{file}variable into the resulting argument list, and invoking it directly withsubprocess.run(without shell interpretation). Also gracefully handles malformed template strings.PR created automatically by Jules for task 6585239734068860722 started by @Wenbobobo