fix(security): execute ScriptNotifier notification templates without a shell

mcp_server/README.md

@@ -45,7 +45,7 @@ When you start an asynchronous mobile test via `mobile_run_task`, Artemis MCP ru
 | **Cursor / Windsurf / VS Code / Cline / Roo Code** | **Native Desktop Toast (`DesktopNotifier`) + File Audit (`FileNotifier`)** | Enabled **by default** across macOS (`osascript`), Linux (`notify-send`), and Windows (`powershell`). Pops up a system notification banner informing the developer whether the task completed or failed. The AI agent inspects `<trace_dir>/status.json` or uses `mobile_manage_task(action="status")`. Can be silenced via `ARTEMIS_DESKTOP_NOTIFY=false`. |
 | **Claude Code / Desktop** | **Native Desktop Toast + File Audit** | Alerts the user via system desktop notification banner when background execution concludes, while maintaining a complete JSONL audit log in `notifications.jsonl`. |
 | **OpenClaw / Slack / Discord / CI/CD** | **HTTP Webhook (`WebhookNotifier`)** | Sends structured JSON POST payloads to custom endpoints configured via `OPENCLAW_WEBHOOK_URL`, `MCP_NOTIFICATION_WEBHOOK`, or `ARTEMIS_WEBHOOK_URL`. |
-| **Universal Custom IDE / CLI Hooks** | **Custom Script Hook (`ScriptNotifier`)** | Set `ARTEMIS_NOTIFY_CMD="my-script --title '{title}' --message '{message}' --trace-id '{trace_id}'"` in your environment to execute any custom command or script upon event completion. |
+| **Universal Custom IDE / CLI Hooks** | **Custom Script Hook (`ScriptNotifier`)** | Set `ARTEMIS_NOTIFY_CMD="my-script --title '{title}' --message '{message}' --trace-id '{trace_id}'"` in your environment to execute any custom command or script upon event completion. The template is run **as an argument vector, not through a shell** — task data substituted into the placeholders is never interpreted as shell syntax. If you need pipes, redirection, or `$VAR` expansion, put that logic in the script itself and reference the script as a single command. |
 
 ## 🧠 AI Agent Behavioral Rules (`rules.md`)
 

mcp_server/notifiers/script.py

@@ -16,6 +16,7 @@
 
 import logging
 import os
+import shlex
 import subprocess
 from typing import Any
 
@@ -31,6 +32,17 @@ class ScriptNotifier(BaseNotifier):
     or automation platform by allowing users to define ARTEMIS_NOTIFY_CMD or MCP_NOTIFY_COMMAND.
     Placeholders like {title}, {message}, {conversation_id}, {event_type}, and {trace_id}
     are automatically replaced before execution.
+
+    The template is executed as an argument vector, not through a shell: it is parsed with
+    ``shlex.split`` and launched with ``shell=False``. Substituted values are task data (a
+    task's ``goal`` reaches {title}/{message}), so they are never interpreted as shell
+    syntax. Two consequences to be aware of:
+
+    - Shell features in the template (pipes, ``&&``, redirection, ``$VAR`` expansion,
+      globbing) are not interpreted. Put that logic in the script itself and reference the
+      script as a single command, e.g. ``my-notify.sh --title '{title}'``.
+    - Quotes around a placeholder are no longer significant: ``--title '{title}'`` and
+      ``--title {title}`` both pass the value as a single argument.
     """
 
     ENV_VARS = [
@@ -67,23 +79,29 @@ def notify(
         trace_id = (payload or {}).get("trace_id", "")
         formatted_title = title or f"Artemis Task {event_type.capitalize()}"
 
-        # Replace template placeholders safely
+        # The template is operator-supplied, but every substituted value is
+        # attacker-influenced task data: a task's `goal` reaches {title}/{message}
+        # (see task_queue_service). Run the template as an argument vector instead of
+        # through a shell, so task data can never be reinterpreted as shell syntax
+        # (CWE-78). shlex.split() resolves the operator's own quoting, so a template such
+        # as `my-script --title '{title}'` still passes the title as a single argument.
         try:
-            cmd = (
-                cmd_template.replace("{title}", str(formatted_title))
+            argv = [
+                token.replace("{title}", str(formatted_title))
                 .replace("{message}", str(message))
                 .replace("{conversation_id}", str(conversation_id))
                 .replace("{event_type}", str(event_type))
                 .replace("{trace_id}", str(trace_id))
-            )
+                for token in shlex.split(cmd_template)
+            ]
             subprocess.run(
-                cmd,
-                shell=True,
+                argv,
+                shell=False,
                 stdout=subprocess.DEVNULL,
                 stderr=subprocess.DEVNULL,
                 timeout=10,
             )
-            logger.info(f"Custom script notification command executed: {cmd[:60]}...")
+            logger.info(f"Custom script notification executed: {shlex.join(argv)[:60]}...")
             return True
         except Exception as e:
             logger.warning(f"Failed to execute custom script notification: {e}")

tests/unit/mcp/test_notifiers.py

@@ -132,6 +132,46 @@ def test_script_notifier(monkeypatch):
     assert res is True
 
 
+def test_script_notifier_passes_substituted_values_as_arguments(monkeypatch):
+    """Substituted values are task data, not shell syntax.
+
+    The notification text embeds single quotes around a task's goal, so letting a shell
+    reinterpret the command line would let that data inject commands of its own.
+    """
+    import subprocess as subprocess_module
+
+    from mcp_server.notifiers.script import ScriptNotifier
+
+    captured: list[list[str]] = []
+    monkeypatch.setattr(subprocess_module, "run", lambda cmd, **kwargs: captured.append(cmd))
+    monkeypatch.setenv("ARTEMIS_NOTIFY_CMD", "true --title '{title}' --message '{message}'")
+
+    goal = "check the battery level ; echo injected ; true"
+    message = f"Artemis autonomous task '{goal}' finished with status 'failed'."
+    title = f"Task Failed: {goal[:40]}"
+
+    assert ScriptNotifier().notify("conv-1", message, title=title) is True
+    # Each substituted value must arrive as exactly one literal argument.
+    assert captured == [["true", "--title", title, "--message", message]]
+
+
+def test_script_notifier_does_not_execute_injected_task_data(monkeypatch, tmp_path):
+    """End-to-end check: task data containing shell metacharacters must not execute."""
+    from mcp_server.notifiers.script import ScriptNotifier
+
+    marker = tmp_path / "injected"
+    monkeypatch.setenv(
+        "ARTEMIS_NOTIFY_CMD",
+        "true --title '{title}' --message '{message}' --trace-id '{trace_id}'",
+    )
+    goal = f"check the battery level and report back now ; touch {marker} ; true"
+    message = f"Artemis autonomous task '{goal}' finished with status 'failed'."
+    title = f"Task Failed: {goal[:40]}"
+
+    assert ScriptNotifier().notify("conv-1", message, title=title) is True
+    assert not marker.exists()
+
+
 def test_composite_notifier_dispatch():
     d1 = DummyNotifier(available=True, return_val=True)
     d2 = DummyNotifier(available=False, return_val=False)