mirror of
https://github.com/pewdiepie-archdaemon/odysseus.git
synced 2026-06-15 17:25:26 -04:00
fix(codex): validate stored SSH host and port
Validate cookbook task remoteHost and sshPort values before building SSH shell commands in the Codex bridge.
This commit is contained in:
+18
-6
@@ -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}"
|
||||
|
||||
@@ -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 == ""
|
||||
Reference in New Issue
Block a user