fix(mcp): stop interpolating notification text into AppleScript/PowerShell

mcp_server/notifiers/desktop.py

@@ -78,26 +78,48 @@ def notify(
                     return True
             elif sys.platform == "darwin":
                 if shutil.which("osascript"):
-                    script = f'display notification "{clean_body}" with title "{header}"'
+                    # header/clean_body are untrusted (can be derived from on-screen
+                    # app content). Never interpolate them into the script source -
+                    # pass them as `on run argv` arguments instead of splicing them
+                    # into the AppleScript text. (An earlier version of this fix used
+                    # `system attribute` + env vars, but `system attribute` re-decodes
+                    # its value through the wrong text encoding and corrupts non-ASCII
+                    # input, including the default "☕ Artemis Task ..." title -
+                    # `argv` values are passed through as literal UTF-8 unchanged.)
+                    # `--` stops osascript from treating a title/body that happens to
+                    # equal "-e" or "--" as its own flag.
+                    script = (
+                        "on run argv\n"
+                        "    display notification (item 2 of argv) with title (item 1 of argv)\n"
+                        "end run"
+                    )
                     subprocess.run(
-                        ["osascript", "-e", script],
+                        ["osascript", "-e", script, "--", header, clean_body],
                         stdout=subprocess.DEVNULL,
                         stderr=subprocess.DEVNULL,
                         timeout=3,
                     )
                     return True
             elif sys.platform == "win32":
+                # Same untrusted-content constraint as the macOS branch above: the
+                # script text is fixed and reads title/body from the environment
+                # instead of having them interpolated into the source.
                 ps_cmd = (
-                    f"[Windows.UI.Notifications.ToastNotificationManager, Windows.UI.Notifications, ContentType = WindowsRuntime] > $null; "
-                    f"$template = [Windows.UI.Notifications.ToastNotificationManager]::GetTemplateContent([Windows.UI.Notifications.ToastTemplateType]::ToastText02); "
-                    f'$textNodes = $template.GetElementsByTagName("text"); '
-                    f'$textNodes.Item(0).AppendChild($template.CreateTextNode("{header}")) > $null; '
-                    f'$textNodes.Item(1).AppendChild($template.CreateTextNode("{clean_body}")) > $null; '
-                    f"$toast = [Windows.UI.Notifications.ToastNotification]::new($template); "
-                    f'[Windows.UI.Notifications.ToastNotificationManager]::CreateToastNotifier("Artemis").Show($toast);'
+                    "[Windows.UI.Notifications.ToastNotificationManager, Windows.UI.Notifications, ContentType = WindowsRuntime] > $null; "
+                    "$template = [Windows.UI.Notifications.ToastNotificationManager]::GetTemplateContent([Windows.UI.Notifications.ToastTemplateType]::ToastText02); "
+                    '$textNodes = $template.GetElementsByTagName("text"); '
+                    "$textNodes.Item(0).AppendChild($template.CreateTextNode($env:ARTEMIS_NOTIFY_TITLE)) > $null; "
+                    "$textNodes.Item(1).AppendChild($template.CreateTextNode($env:ARTEMIS_NOTIFY_BODY)) > $null; "
+                    "$toast = [Windows.UI.Notifications.ToastNotification]::new($template); "
+                    '[Windows.UI.Notifications.ToastNotificationManager]::CreateToastNotifier("Artemis").Show($toast);'
                 )
                 subprocess.run(
                     ["powershell", "-NoProfile", "-Command", ps_cmd],
+                    env={
+                        **os.environ,
+                        "ARTEMIS_NOTIFY_TITLE": header,
+                        "ARTEMIS_NOTIFY_BODY": clean_body,
+                    },
                     stdout=subprocess.DEVNULL,
                     stderr=subprocess.DEVNULL,
                     timeout=5,

tests/unit/mcp/test_notifiers.py

@@ -118,6 +118,117 @@ def test_desktop_notifier_enabled_by_default(monkeypatch):
     assert notifier.is_available() is True
 
 
+def test_desktop_notifier_darwin_does_not_interpolate_untrusted_text(monkeypatch):
+    import shutil
+    import subprocess
+    import sys
+
+    monkeypatch.delenv("ARTEMIS_DESKTOP_NOTIFY", raising=False)
+    monkeypatch.delenv("CI", raising=False)
+    monkeypatch.setattr(sys, "platform", "darwin")
+    monkeypatch.setattr(
+        shutil, "which", lambda cmd: "/usr/bin/osascript" if cmd == "osascript" else None
+    )
+
+    captured = {}
+
+    def fake_run(cmd, **kwargs):
+        captured["cmd"] = cmd
+        captured["env"] = kwargs.get("env")
+
+        class Result:
+            returncode = 0
+
+        return Result()
+
+    monkeypatch.setattr(subprocess, "run", fake_run)
+
+    payload = 'pwned" \ndo shell script "touch /tmp/artemis_pwned"\n--'
+    notifier = DesktopNotifier()
+    assert notifier.notify("conv-1", payload, title=payload) is True
+
+    cmd = captured["cmd"]
+    script = cmd[2]
+    assert payload not in script
+    assert "do shell script" not in script
+    # payload must reach osascript only as an `on run argv` argument, after
+    # the `--` separator, never spliced into the script text itself
+    assert cmd[3] == "--"
+    assert cmd[4] == payload
+    assert cmd[5] == payload
+
+
+def test_desktop_notifier_darwin_passes_argv_after_separator(monkeypatch):
+    """Title/body reach osascript as `on run argv` arguments after a literal
+    `--`, so a value that happens to equal an osascript flag (e.g. "-e") can't
+    be mistaken for one, and non-ASCII text isn't routed through `system
+    attribute` (which re-decodes through the wrong text encoding and corrupts
+    it - see the default "☕ Artemis Task ..." title)."""
+    import shutil
+    import subprocess
+    import sys
+
+    monkeypatch.delenv("ARTEMIS_DESKTOP_NOTIFY", raising=False)
+    monkeypatch.delenv("CI", raising=False)
+    monkeypatch.setattr(sys, "platform", "darwin")
+    monkeypatch.setattr(
+        shutil, "which", lambda cmd: "/usr/bin/osascript" if cmd == "osascript" else None
+    )
+
+    captured = {}
+
+    def fake_run(cmd, **kwargs):
+        captured["cmd"] = cmd
+
+        class Result:
+            returncode = 0
+
+        return Result()
+
+    monkeypatch.setattr(subprocess, "run", fake_run)
+
+    notifier = DesktopNotifier()
+    assert notifier.notify("conv-1", "-e", title="-e") is True
+    cmd = captured["cmd"]
+    assert cmd[-3] == "--"
+    assert cmd[-2:] == ["-e", "-e"]
+    assert "-e" not in cmd[2]
+
+
+def test_desktop_notifier_windows_does_not_interpolate_untrusted_text(monkeypatch):
+    import shutil
+    import subprocess
+    import sys
+
+    monkeypatch.delenv("ARTEMIS_DESKTOP_NOTIFY", raising=False)
+    monkeypatch.delenv("CI", raising=False)
+    monkeypatch.setattr(sys, "platform", "win32")
+    monkeypatch.setattr(shutil, "which", lambda cmd: "/usr/bin/" + cmd)
+
+    captured = {}
+
+    def fake_run(cmd, **kwargs):
+        captured["cmd"] = cmd
+        captured["env"] = kwargs.get("env")
+
+        class Result:
+            returncode = 0
+
+        return Result()
+
+    monkeypatch.setattr(subprocess, "run", fake_run)
+
+    payload = '"); Start-Process calc.exe; ("'
+    notifier = DesktopNotifier()
+    assert notifier.notify("conv-1", payload, title=payload) is True
+
+    script = captured["cmd"][2]
+    assert payload not in script
+    assert "Start-Process" not in script
+    assert captured["env"]["ARTEMIS_NOTIFY_BODY"] == payload
+    assert captured["env"]["ARTEMIS_NOTIFY_TITLE"] == payload
+
+
 def test_script_notifier(monkeypatch):
     from mcp_server.notifiers.script import ScriptNotifier