mirror of
https://github.com/pewdiepie-archdaemon/odysseus.git
synced 2026-09-13 11:42:20 +02:00
Compare commits
1
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
e410a7dcd9 |
@@ -31,11 +31,11 @@ jobs:
|
|||||||
with:
|
with:
|
||||||
persist-credentials: false
|
persist-credentials: false
|
||||||
- name: Initialize CodeQL
|
- name: Initialize CodeQL
|
||||||
uses: github/codeql-action/init@ff2f1c621b7f889edc0d3c761ac2e6a3f8cdb0dd # v4.37.7
|
uses: github/codeql-action/init@cdf488f595d80d6e07e03d4674febd5ab45fa938 # v4.37.9
|
||||||
with:
|
with:
|
||||||
languages: ${{ matrix.language }}
|
languages: ${{ matrix.language }}
|
||||||
build-mode: none
|
build-mode: none
|
||||||
- name: Perform CodeQL Analysis
|
- name: Perform CodeQL Analysis
|
||||||
uses: github/codeql-action/analyze@ff2f1c621b7f889edc0d3c761ac2e6a3f8cdb0dd # v4.37.7
|
uses: github/codeql-action/analyze@cdf488f595d80d6e07e03d4674febd5ab45fa938 # v4.37.9
|
||||||
with:
|
with:
|
||||||
category: "/language:${{ matrix.language }}"
|
category: "/language:${{ matrix.language }}"
|
||||||
|
|||||||
@@ -42,7 +42,7 @@ jobs:
|
|||||||
persist-credentials: false
|
persist-credentials: false
|
||||||
|
|
||||||
- name: Lint Dockerfile
|
- name: Lint Dockerfile
|
||||||
uses: hadolint/hadolint-action@2a66e89f53d0771bb131a7fa31f3136336094aa6 # v3.4.0
|
uses: hadolint/hadolint-action@06be81baf89a55ffd0e24b8f04a4185738dd3387 # v3.5.0
|
||||||
with:
|
with:
|
||||||
dockerfile: Dockerfile
|
dockerfile: Dockerfile
|
||||||
# DL3008: pinning apt package versions is impractical on a -slim base
|
# DL3008: pinning apt package versions is impractical on a -slim base
|
||||||
|
|||||||
@@ -123,7 +123,7 @@ jobs:
|
|||||||
TRIVY_DB_REPOSITORY: ghcr.io/aquasecurity/trivy-db:2
|
TRIVY_DB_REPOSITORY: ghcr.io/aquasecurity/trivy-db:2
|
||||||
|
|
||||||
- name: Upload Trivy results
|
- name: Upload Trivy results
|
||||||
uses: github/codeql-action/upload-sarif@ff2f1c621b7f889edc0d3c761ac2e6a3f8cdb0dd # v4.37.7
|
uses: github/codeql-action/upload-sarif@cdf488f595d80d6e07e03d4674febd5ab45fa938 # v4.37.9
|
||||||
with:
|
with:
|
||||||
sarif_file: trivy-results.sarif
|
sarif_file: trivy-results.sarif
|
||||||
category: trivy-image
|
category: trivy-image
|
||||||
|
|||||||
@@ -47,4 +47,4 @@ jobs:
|
|||||||
steps:
|
steps:
|
||||||
- name: Deploy to GitHub Pages
|
- name: Deploy to GitHub Pages
|
||||||
id: deployment
|
id: deployment
|
||||||
uses: actions/deploy-pages@cd2ce8fcbc39b97be8ca5fce6e763baed58fa128 # v5.0.0
|
uses: actions/deploy-pages@368f82528645a54fb793d4d04e342629a3f51346 # v5.0.1
|
||||||
|
|||||||
@@ -181,17 +181,10 @@ def setup_mcp_routes(mcp_manager: McpManager):
|
|||||||
if transport == "http" and not url:
|
if transport == "http" and not url:
|
||||||
raise HTTPException(400, "url is required for HTTP transport")
|
raise HTTPException(400, "url is required for HTTP transport")
|
||||||
|
|
||||||
# Parse JSON fields. args is not defaulted on a parse failure: an
|
# Parse JSON fields
|
||||||
# unparseable value is silently discarded downstream (stdio spawns
|
|
||||||
# with an empty argv), so the caller must be told instead.
|
|
||||||
if args:
|
|
||||||
try:
|
try:
|
||||||
parsed_args = json.loads(args)
|
parsed_args = json.loads(args) if args else []
|
||||||
except json.JSONDecodeError:
|
except json.JSONDecodeError:
|
||||||
raise HTTPException(400, "args must be valid JSON, e.g. [\"-y\", \"pkg\"]")
|
|
||||||
if not isinstance(parsed_args, list):
|
|
||||||
raise HTTPException(400, "args must be a JSON array, e.g. [\"-y\", \"pkg\"]")
|
|
||||||
else:
|
|
||||||
parsed_args = []
|
parsed_args = []
|
||||||
try:
|
try:
|
||||||
parsed_env = json.loads(env) if env else {}
|
parsed_env = json.loads(env) if env else {}
|
||||||
|
|||||||
@@ -2366,7 +2366,6 @@ function initMcpForm() {
|
|||||||
if (transport === 'stdio' && !command) { msg.textContent = 'Command is required for stdio'; msg.className = 'admin-error'; return; }
|
if (transport === 'stdio' && !command) { msg.textContent = 'Command is required for stdio'; msg.className = 'admin-error'; return; }
|
||||||
if (transport === 'sse' && !url) { msg.textContent = 'URL is required for SSE'; msg.className = 'admin-error'; return; }
|
if (transport === 'sse' && !url) { msg.textContent = 'URL is required for SSE'; msg.className = 'admin-error'; return; }
|
||||||
try { JSON.parse(env); } catch { msg.textContent = 'Env must be valid JSON'; msg.className = 'admin-error'; return; }
|
try { JSON.parse(env); } catch { msg.textContent = 'Env must be valid JSON'; msg.className = 'admin-error'; return; }
|
||||||
try { JSON.parse(args); } catch { msg.textContent = 'Args must be valid JSON, e.g. ["-y", "pkg"]'; msg.className = 'admin-error'; return; }
|
|
||||||
const fd = new FormData();
|
const fd = new FormData();
|
||||||
fd.append('name', name); fd.append('transport', transport); fd.append('command', command); fd.append('args', args); fd.append('env', env); fd.append('url', url);
|
fd.append('name', name); fd.append('transport', transport); fd.append('command', command); fd.append('args', args); fd.append('env', env); fd.append('url', url);
|
||||||
// If preset has oauthFile config, send credentials for file generation
|
// If preset has oauthFile config, send credentials for file generation
|
||||||
@@ -2387,10 +2386,6 @@ function initMcpForm() {
|
|||||||
try {
|
try {
|
||||||
const res = await fetch('/api/mcp/servers', { method: 'POST', body: fd, credentials: 'same-origin' });
|
const res = await fetch('/api/mcp/servers', { method: 'POST', body: fd, credentials: 'same-origin' });
|
||||||
const data = await res.json();
|
const data = await res.json();
|
||||||
if (!res.ok) {
|
|
||||||
msg.textContent = data.detail || `Failed (${res.status})`; msg.className = 'admin-error';
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
if (data.needs_oauth) {
|
if (data.needs_oauth) {
|
||||||
msg.innerHTML = `Added ${esc(name)} — <a href="/api/mcp/oauth/authorize/${data.id}" target="_blank" style="color:var(--red);font-weight:600;">Authorize with Google</a> to connect`;
|
msg.innerHTML = `Added ${esc(name)} — <a href="/api/mcp/oauth/authorize/${data.id}" target="_blank" style="color:var(--red);font-weight:600;">Authorize with Google</a> to connect`;
|
||||||
msg.className = 'admin-success';
|
msg.className = 'admin-success';
|
||||||
|
|||||||
@@ -5036,11 +5036,7 @@ async function initUnifiedIntegrations() {
|
|||||||
fd.append('transport', transport);
|
fd.append('transport', transport);
|
||||||
if (transport === 'stdio') {
|
if (transport === 'stdio') {
|
||||||
fd.append('command', el('uf-mcp-cmd').value);
|
fd.append('command', el('uf-mcp-cmd').value);
|
||||||
// Unlike env below, an unparseable args value is not silently
|
let args = '[]'; try { args = JSON.stringify(JSON.parse(el('uf-mcp-args').value || '[]')); } catch (_) {}
|
||||||
// defaulted: it would spawn the subprocess with an empty argv.
|
|
||||||
let args;
|
|
||||||
try { args = JSON.stringify(JSON.parse(el('uf-mcp-args').value || '[]')); }
|
|
||||||
catch (_) { el('uf-mcp-msg').textContent = 'Args must be valid JSON, e.g. ["-y", "pkg"]'; return; }
|
|
||||||
let env = '{}'; try { env = JSON.stringify(JSON.parse(el('uf-mcp-env').value || '{}')); } catch (_) {}
|
let env = '{}'; try { env = JSON.stringify(JSON.parse(el('uf-mcp-env').value || '{}')); } catch (_) {}
|
||||||
fd.append('args', args);
|
fd.append('args', args);
|
||||||
fd.append('env', env);
|
fd.append('env', env);
|
||||||
|
|||||||
@@ -1,147 +0,0 @@
|
|||||||
"""Regression test for issue #6211: a malformed Args value on the "Add MCP
|
|
||||||
Server" form must not be silently discarded into an empty argv.
|
|
||||||
|
|
||||||
routes/mcp/mcp_routes.py's add_server() wrapped json.loads(args) in a bare
|
|
||||||
except that fell back to `[]`, so a non-JSON Args value registered the
|
|
||||||
server as "Connected" while forwarding no arguments to the spawned stdio
|
|
||||||
subprocess at all, with no error surfaced anywhere.
|
|
||||||
"""
|
|
||||||
import asyncio
|
|
||||||
import json
|
|
||||||
from unittest.mock import AsyncMock, MagicMock
|
|
||||||
|
|
||||||
import pytest
|
|
||||||
from fastapi import HTTPException
|
|
||||||
|
|
||||||
from routes.mcp import mcp_routes
|
|
||||||
|
|
||||||
|
|
||||||
class _FakeSession:
|
|
||||||
"""Stands in for core.database.SessionLocal(); add_server only adds+commits."""
|
|
||||||
|
|
||||||
def __init__(self):
|
|
||||||
self.added = []
|
|
||||||
|
|
||||||
def add(self, obj):
|
|
||||||
self.added.append(obj)
|
|
||||||
|
|
||||||
def commit(self):
|
|
||||||
pass
|
|
||||||
|
|
||||||
def close(self):
|
|
||||||
pass
|
|
||||||
|
|
||||||
|
|
||||||
def _add_server(monkeypatch):
|
|
||||||
"""Register add_server on the shared module-level router and return the
|
|
||||||
freshly-added route's raw endpoint function, bypassing HTTP/Form parsing
|
|
||||||
(require_admin is the only other thing the function touches via `request`).
|
|
||||||
|
|
||||||
Callers must pass every Form(...) parameter add_server reads past the args
|
|
||||||
check (url, oauth_file, oauth_config): calling the endpoint directly skips
|
|
||||||
FastAPI's dependency resolution, so an omitted one arrives as the Form
|
|
||||||
marker object itself rather than its declared default, and later code
|
|
||||||
(e.g. `if oauth_file:`) reads that marker as truthy.
|
|
||||||
"""
|
|
||||||
monkeypatch.setattr(mcp_routes, "require_admin", lambda request: None)
|
|
||||||
manager = MagicMock()
|
|
||||||
manager.connect_server = AsyncMock(return_value=True)
|
|
||||||
manager.get_server_status = MagicMock(return_value={"status": "connected", "tool_count": 1})
|
|
||||||
router = mcp_routes.setup_mcp_routes(manager)
|
|
||||||
# setup_mcp_routes appends new APIRoute objects to the shared router on
|
|
||||||
# every call, so take the LAST "add_server" route: the one just registered
|
|
||||||
# with our fake manager, not an earlier registration from importing app.py.
|
|
||||||
route = [r for r in router.routes if getattr(r, "name", None) == "add_server"][-1]
|
|
||||||
return route.endpoint, manager
|
|
||||||
|
|
||||||
|
|
||||||
def test_add_server_rejects_malformed_args_instead_of_defaulting(monkeypatch):
|
|
||||||
add_server, manager = _add_server(monkeypatch)
|
|
||||||
monkeypatch.setattr(mcp_routes, "SessionLocal", lambda: (_ for _ in ()).throw(
|
|
||||||
AssertionError("must not reach the DB when args is rejected")))
|
|
||||||
|
|
||||||
with pytest.raises(HTTPException) as exc:
|
|
||||||
asyncio.run(add_server(
|
|
||||||
request=None,
|
|
||||||
name="filesystem",
|
|
||||||
transport="stdio",
|
|
||||||
command="mcp-server-filesystem",
|
|
||||||
args="/app/data/jarvis-files", # the exact value from issue #6211
|
|
||||||
env="{}",
|
|
||||||
url=None,
|
|
||||||
oauth_file=None,
|
|
||||||
oauth_config=None,
|
|
||||||
))
|
|
||||||
|
|
||||||
assert exc.value.status_code == 400
|
|
||||||
manager.connect_server.assert_not_called()
|
|
||||||
|
|
||||||
|
|
||||||
def test_add_server_still_accepts_valid_json_args(monkeypatch):
|
|
||||||
add_server, manager = _add_server(monkeypatch)
|
|
||||||
fake_session = _FakeSession()
|
|
||||||
monkeypatch.setattr(mcp_routes, "SessionLocal", lambda: fake_session)
|
|
||||||
|
|
||||||
result = asyncio.run(add_server(
|
|
||||||
request=None,
|
|
||||||
name="filesystem",
|
|
||||||
transport="stdio",
|
|
||||||
command="mcp-server-filesystem",
|
|
||||||
args=json.dumps(["/app/data/jarvis-files"]),
|
|
||||||
env="{}",
|
|
||||||
url=None,
|
|
||||||
oauth_file=None,
|
|
||||||
oauth_config=None,
|
|
||||||
))
|
|
||||||
|
|
||||||
assert result["connected"] is True
|
|
||||||
manager.connect_server.assert_awaited_once()
|
|
||||||
assert manager.connect_server.call_args.kwargs["args"] == ["/app/data/jarvis-files"]
|
|
||||||
assert fake_session.added[0].args == json.dumps(["/app/data/jarvis-files"])
|
|
||||||
|
|
||||||
|
|
||||||
def test_add_server_rejects_valid_json_args_that_is_not_a_list(monkeypatch):
|
|
||||||
"""Valid JSON that is not a list (e.g. args=5) must not reach
|
|
||||||
StdioServerParameters(args=5), which raises an unhandled TypeError when
|
|
||||||
the error formatter later does " ".join([command, *args])."""
|
|
||||||
add_server, manager = _add_server(monkeypatch)
|
|
||||||
monkeypatch.setattr(mcp_routes, "SessionLocal", lambda: (_ for _ in ()).throw(
|
|
||||||
AssertionError("must not reach the DB when args has the wrong shape")))
|
|
||||||
|
|
||||||
with pytest.raises(HTTPException) as exc:
|
|
||||||
asyncio.run(add_server(
|
|
||||||
request=None,
|
|
||||||
name="filesystem",
|
|
||||||
transport="stdio",
|
|
||||||
command="mcp-server-filesystem",
|
|
||||||
args="5",
|
|
||||||
env="{}",
|
|
||||||
url=None,
|
|
||||||
oauth_file=None,
|
|
||||||
oauth_config=None,
|
|
||||||
))
|
|
||||||
|
|
||||||
assert exc.value.status_code == 400
|
|
||||||
manager.connect_server.assert_not_called()
|
|
||||||
|
|
||||||
|
|
||||||
def test_add_server_still_defaults_empty_args_to_empty_list(monkeypatch):
|
|
||||||
"""No behavior change for the common case of an empty Args field."""
|
|
||||||
add_server, manager = _add_server(monkeypatch)
|
|
||||||
fake_session = _FakeSession()
|
|
||||||
monkeypatch.setattr(mcp_routes, "SessionLocal", lambda: fake_session)
|
|
||||||
|
|
||||||
result = asyncio.run(add_server(
|
|
||||||
request=None,
|
|
||||||
name="no-args-server",
|
|
||||||
transport="stdio",
|
|
||||||
command="some-command",
|
|
||||||
args="",
|
|
||||||
env="{}",
|
|
||||||
url=None,
|
|
||||||
oauth_file=None,
|
|
||||||
oauth_config=None,
|
|
||||||
))
|
|
||||||
|
|
||||||
assert result["connected"] is True
|
|
||||||
assert manager.connect_server.call_args.kwargs["args"] == []
|
|
||||||
Reference in New Issue
Block a user