From dded1a71291a60f82a692cab8b0efa99c2ca24f6 Mon Sep 17 00:00:00 2001 From: dacj4n Date: Thu, 17 Sep 2026 15:20:06 +0800 Subject: [PATCH 1/2] fix(security): execute ScriptNotifier notification templates without a shell ScriptNotifier substituted task data into an operator-supplied template and ran the result through a shell (subprocess.run(..., shell=True)). A task's goal reaches {title}/{message}, and POST /api/run requires no authentication, so an unauthenticated task submission could be interpreted as shell syntax and executed as the ARTEMIS server user (CWE-78). Execute the template as an argument vector instead: shlex.split() resolves the operator's own quoting, placeholders are substituted per token, and the command is launched with shell=False, so task data can no longer contribute shell syntax. Quoting the substituted values would not have been sufficient: the template may already wrap the placeholder in quotes, in which case the added quotes cancel out and the value is parsed as shell syntax again. Behavior change: shell features in the template (pipes, &&, redirection, $VAR expansion, globbing) no longer apply; move that logic into the script itself. A missing command now makes notify() return False rather than True. Adds two regression tests in tests/unit/mcp/test_notifiers.py; both fail on main without this change. --- mcp_server/notifiers/script.py | 21 +++++++++++------ tests/unit/mcp/test_notifiers.py | 40 ++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 7 deletions(-) diff --git a/mcp_server/notifiers/script.py b/mcp_server/notifiers/script.py index 6d88fd8f..36c17c23 100644 --- a/mcp_server/notifiers/script.py +++ b/mcp_server/notifiers/script.py @@ -16,6 +16,7 @@ import logging import os +import shlex import subprocess from typing import Any @@ -67,23 +68,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}") diff --git a/tests/unit/mcp/test_notifiers.py b/tests/unit/mcp/test_notifiers.py index b429bc59..fea8b91b 100644 --- a/tests/unit/mcp/test_notifiers.py +++ b/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) From e328ca276a37e9e760cd8fb3472658bc3a95d81f Mon Sep 17 00:00:00 2001 From: dacj4n Date: Thu, 17 Sep 2026 15:26:20 +0800 Subject: [PATCH 2/2] docs(notifiers): note that notification templates are not shell-interpreted Document the behaviour change introduced by executing the template as an argument vector: shell features in the template no longer apply, so that logic belongs in the script itself. Also note that quotes around a placeholder no longer change how the value is passed. --- mcp_server/README.md | 2 +- mcp_server/notifiers/script.py | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/mcp_server/README.md b/mcp_server/README.md index 6855f135..ecffd870 100644 --- a/mcp_server/README.md +++ b/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 `/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`) diff --git a/mcp_server/notifiers/script.py b/mcp_server/notifiers/script.py index 36c17c23..9553d5a7 100644 --- a/mcp_server/notifiers/script.py +++ b/mcp_server/notifiers/script.py @@ -32,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 = [