From f14ea6d67d5203a75cbf0ce2ed50b44e6f6270e3 Mon Sep 17 00:00:00 2001
From: nopoz <bill.lowney@gmail.com>
Date: Sun, 14 Jun 2026 23:01:03 -0700
Subject: [PATCH] fix(codex): validate stored SSH host and port

Validate cookbook task remoteHost and sshPort values before building SSH shell commands in the Codex bridge.
---
 routes/codex_routes.py                  | 24 +++++++++---
 tests/test_codex_ssh_host_validation.py | 49 +++++++++++++++++++++++++
 2 files changed, 67 insertions(+), 6 deletions(-)
 create mode 100644 tests/test_codex_ssh_host_validation.py

diff --git a/routes/codex_routes.py b/routes/codex_routes.py
index 1afac02b9..4ef3a6466 100644
--- a/routes/codex_routes.py
+++ b/routes/codex_routes.py
@@ -18,6 +18,7 @@ from fastapi.responses import StreamingResponse
 from src.auth_helpers import require_authenticated_request, require_user
 from src.tool_implementations import do_manage_notes
 from src.constants import COOKBOOK_STATE_FILE
+from routes._validators import validate_remote_host, validate_ssh_port
 
 
 COOKBOOK_READ_SCOPES = {"cookbook:read", "cookbook:launch"}
@@ -36,6 +37,21 @@ DOCS_WRITE_SCOPES = {"documents:write"}
 WRITE_ACTIONS = {"add", "create", "new", "save", "remind", "update", "delete", "toggle_item", "remove", "remove_item"}
 
 
+def _ssh_prefix_for_task(task: dict) -> tuple[str, str]:
+    """Resolve a cookbook task's stored SSH target into ``(host, port_flag)``.
+
+    ``host`` is ``""`` for a local task. ``remoteHost`` / ``sshPort`` come from
+    cookbook_state.json and get interpolated into an ``ssh`` command string, so
+    validate them the same way the cookbook routes do. A tampered entry with
+    shell metacharacters in ``remoteHost`` is rejected with 400 rather than
+    injected.
+    """
+    host = validate_remote_host((task.get("remoteHost") or "").strip() or None) or ""
+    ssh_port = validate_ssh_port((task.get("sshPort") or "").strip() or None) or ""
+    port_flag = f"-p {ssh_port} " if ssh_port and ssh_port != "22" else ""
+    return host, port_flag
+
+
 async def _as_owner(request: Request, owner: str, fn, *args, **kwargs):
     """Run an existing route handler with request.state.current_user temporarily
     set to ``owner`` so its internal get_current_user/require_user calls see
@@ -486,8 +502,7 @@ def setup_codex_routes(
         task = next((t for t in tasks if t.get("sessionId") == session_id), None)
         if task is None:
             raise HTTPException(404, "task not found")
-        host = (task.get("remoteHost") or "").strip()
-        ssh_port = (task.get("sshPort") or "").strip()
+        host, port_flag = _ssh_prefix_for_task(task)
         # Prefer the persisted log file over the tmux pane. The pane gets
         # overwritten by the post-crash neofetch banner + bash prompt the
         # moment vllm exits; the log file is the raw stdout/stderr and
@@ -499,7 +514,6 @@ def setup_codex_routes(
             f"else tmux capture-pane -t {session_id} -p -S -{tail}; fi"
         )
         if host:
-            port_flag = f"-p {ssh_port} " if ssh_port and ssh_port != "22" else ""
             import shlex
             cmd = f"ssh {port_flag}{host} {shlex.quote(inner)}"
         else:
@@ -561,10 +575,8 @@ def setup_codex_routes(
         state = _read_cookbook_state()
         tasks = state.get("tasks") or []
         task = next((t for t in tasks if t.get("sessionId") == session_id), None)
-        host = ((task or {}).get("remoteHost") or "").strip()
-        ssh_port = ((task or {}).get("sshPort") or "").strip()
+        host, port_flag = _ssh_prefix_for_task(task or {})
         if host:
-            port_flag = f"-p {ssh_port} " if ssh_port and ssh_port != "22" else ""
             cmd = f"ssh {port_flag}{host} \"tmux kill-session -t {session_id}\""
         else:
             cmd = f"tmux kill-session -t {session_id}"
diff --git a/tests/test_codex_ssh_host_validation.py b/tests/test_codex_ssh_host_validation.py
new file mode 100644
index 000000000..26da3963c
--- /dev/null
+++ b/tests/test_codex_ssh_host_validation.py
@@ -0,0 +1,49 @@
+"""The Codex cookbook bridge resolves a task's SSH target (remoteHost / sshPort)
+from cookbook_state.json and interpolates it into an ``ssh ...`` command string
+that runs through a shell. The command body is shlex-quoted, but the host and
+port were not validated, so a tampered task entry carrying shell metacharacters
+in ``remoteHost`` would be injected into that command.
+
+These pin validation on the host/port before they reach the ssh string, matching
+the validators the rest of the cookbook routes already apply.
+"""
+import pytest
+from fastapi import HTTPException
+
+import routes.codex_routes as codex_routes
+
+
+def test_rejects_remote_host_with_shell_metacharacters():
+    task = {"remoteHost": "box; rm -rf ~", "sshPort": ""}
+    with pytest.raises(HTTPException) as exc:
+        codex_routes._ssh_prefix_for_task(task)
+    assert exc.value.status_code == 400
+
+
+def test_rejects_non_numeric_ssh_port():
+    task = {"remoteHost": "box", "sshPort": "22; evil"}
+    with pytest.raises(HTTPException) as exc:
+        codex_routes._ssh_prefix_for_task(task)
+    assert exc.value.status_code == 400
+
+
+def test_local_task_has_no_host():
+    host, port_flag = codex_routes._ssh_prefix_for_task({})
+    assert host == ""
+    assert port_flag == ""
+
+
+def test_valid_remote_builds_port_flag():
+    host, port_flag = codex_routes._ssh_prefix_for_task(
+        {"remoteHost": "user@box", "sshPort": "2222"}
+    )
+    assert host == "user@box"
+    assert port_flag == "-p 2222 "
+
+
+def test_default_ssh_port_omits_flag():
+    host, port_flag = codex_routes._ssh_prefix_for_task(
+        {"remoteHost": "box", "sshPort": "22"}
+    )
+    assert host == "box"
+    assert port_flag == ""