From 4b3820960be413b95c5859bf617de61630f48a98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Carlos=20Este=CC=81vez=20Rodri=CC=81guez?= Date: Sun, 16 Aug 2026 18:53:14 -0600 Subject: [PATCH] Lock down the local FastAPI server against unauthenticated access The desktop backend bound 127.0.0.1:8765 with no authentication and CORS *. Any local process or web page that can reach loopback could start GPU jobs, change the Hugging Face token, or read files through /workspace path traversal and /optimize/serve-file. Require a per-session bearer token when Electron starts the API. Reject non-loopback Host and Origin unless MODLY_API_ALLOW_REMOTE=1. Jail workspace, export, optimize, and serve-file paths. Copy imported meshes into a Modly temp dir so serve-file never reads arbitrary disks. Inject the token from Electron (main-process axios and renderer webRequest) so the 3D viewer and download links keep working. Teach the CLI and MCP client to send --token, MODLY_API_TOKEN, or the userData/api-token file. Stop putting the Hugging Face token in query strings. GET /health stays public so readiness probes still work. Headless uvicorn without MODLY_API_TOKEN remains usable on a trusted machine. --- api/README.md | 13 ++++ api/main.py | 14 ++-- api/mcp_server.py | 30 ++++++++- api/routers/agent.py | 15 ++++- api/routers/export.py | 8 ++- api/routers/extensions.py | 7 ++ api/routers/model.py | 13 +++- api/routers/optimize.py | 50 +++++++------- api/routers/status.py | 6 +- api/services/api_guard.py | 102 +++++++++++++++++++++++++++++ api/services/local_paths.py | 84 ++++++++++++++++++++++++ api/tests/test_api_guard.py | 104 ++++++++++++++++++++++++++++++ api/tests/test_local_paths.py | 71 ++++++++++++++++++++ docs/running-on-jetson.md | 10 ++- electron/main/index.ts | 12 +++- electron/main/ipc-handlers.ts | 28 ++++---- electron/main/model-downloader.ts | 6 +- electron/main/python-bridge.ts | 41 +++++++++++- scripts/run-pytests.mjs | 6 +- tools/modly-cli/SKILL.md | 4 +- tools/modly-cli/agent.py | 81 +++++++++++++++++++---- tools/modly-cli/test_agent.py | 25 +++++++ 22 files changed, 658 insertions(+), 72 deletions(-) create mode 100644 api/services/api_guard.py create mode 100644 api/services/local_paths.py create mode 100644 api/tests/test_api_guard.py create mode 100644 api/tests/test_local_paths.py diff --git a/api/README.md b/api/README.md index ee45bc3f..a8da27f5 100644 --- a/api/README.md +++ b/api/README.md @@ -21,6 +21,19 @@ pip install -r requirements.txt uvicorn main:app --host 127.0.0.1 --port 8765 --reload ``` +The desktop app always starts this server with `MODLY_API_TOKEN` set. Requests other than `GET /health` then require: + +``` +Authorization: Bearer +``` + +or `X-Modly-Token: `. Electron writes the token to `userData/api-token` (mode `0600`) so the CLI/MCP client can pick it up. Headless `uvicorn` without the env var stays usable on a trusted machine, but non-loopback `Host` / `Origin` values are still rejected. Set `MODLY_API_ALLOW_REMOTE=1` only when you also set a token and bind beyond loopback. + +```bash +export MODLY_API_TOKEN=$(python -c 'import secrets; print(secrets.token_hex(32))') +curl -H "Authorization: Bearer $MODLY_API_TOKEN" http://127.0.0.1:8765/model/all +``` + ## Key endpoints | Method | Path | Description | diff --git a/api/main.py b/api/main.py index 84681b0e..a6391292 100644 --- a/api/main.py +++ b/api/main.py @@ -10,6 +10,8 @@ from fastapi import HTTPException from routers import generation, model, optimize, status, settings, extensions, export, workflow_runs, agent +from services.api_guard import LocalApiGuardMiddleware +from services.local_paths import resolve_workspace_file @asynccontextmanager @@ -35,13 +37,14 @@ def filter(self, record): lifespan=lifespan, ) +# Last added = outermost. CORS must wrap the guard so 401/403 still get ACAO +# headers; the 3D viewers (file:// / localhost → 127.0.0.1:8765) are cross-origin. +app.add_middleware(LocalApiGuardMiddleware) app.add_middleware( CORSMiddleware, - allow_origins=["*"], + allow_origin_regex=r"^(null|file://.*|app://.*|https?://(localhost|127\.0\.0\.1|\[::1\])(:\d+)?)$", allow_methods=["*"], allow_headers=["*"], - # drei's SplatLoader reads Content-Length to size its buffers; cross-origin - # JS can only see it when the server explicitly exposes the header. expose_headers=["Content-Length"], ) @@ -59,7 +62,10 @@ def filter(self, record): @app.get("/workspace/{full_path:path}") async def serve_workspace_file(full_path: str): import services.generator_registry as reg - file_path = reg.WORKSPACE_DIR / full_path + try: + file_path = resolve_workspace_file(reg.WORKSPACE_DIR, full_path) + except ValueError as exc: + raise HTTPException(status_code=400, detail=str(exc)) from exc if not file_path.exists() or not file_path.is_file(): raise HTTPException(status_code=404, detail="File not found") return FileResponse(str(file_path)) diff --git a/api/mcp_server.py b/api/mcp_server.py index fa9127cf..dd7657b0 100644 --- a/api/mcp_server.py +++ b/api/mcp_server.py @@ -20,12 +20,36 @@ import asyncio import mimetypes +import os +from pathlib import Path + import httpx from mcp.server import Server from mcp.server.stdio import stdio_server from mcp.types import Tool, TextContent -API_BASE = "http://localhost:8765" +API_BASE = os.environ.get("MODLY_API_URL", "http://127.0.0.1:8765") + + +def _api_headers() -> dict[str, str]: + token = os.environ.get("MODLY_API_TOKEN", "").strip() + if not token: + candidates = [ + Path.home() / "Library" / "Application Support" / "Modly" / "api-token", + Path(os.environ.get("APPDATA", "")) / "Modly" / "api-token", + Path.home() / ".config" / "Modly" / "api-token", + ] + for path in candidates: + if path.is_file(): + try: + token = path.read_text(encoding="utf-8").strip() + except OSError: + continue + if token: + break + if not token: + return {} + return {"Authorization": f"Bearer {token}", "X-Modly-Token": token} server = Server("modly") @@ -148,12 +172,12 @@ async def list_tools() -> list[Tool]: @server.call_tool() async def call_tool(name: str, arguments: dict) -> list[TextContent]: - async with httpx.AsyncClient(timeout=60.0) as client: + async with httpx.AsyncClient(timeout=60.0, headers=_api_headers()) as client: try: result = await _dispatch(client, name, arguments) except httpx.ConnectError: result = ( - "Cannot connect to Modly API at http://localhost:8765. " + f"Cannot connect to Modly API at {API_BASE}. " "Make sure Modly is running." ) except httpx.HTTPStatusError as e: diff --git a/api/routers/agent.py b/api/routers/agent.py index 3eeb2a73..3829eda3 100644 --- a/api/routers/agent.py +++ b/api/routers/agent.py @@ -1,15 +1,25 @@ """ Agent chat endpoint — runs an Ollama-powered tool-use loop against Modly's API. """ +import os import re import uuid import httpx from fastapi import APIRouter from pydantic import BaseModel +from services.api_guard import TOKEN_ENV + router = APIRouter(prefix="/agent", tags=["agent"]) -MODLY_API = "http://localhost:8765" +MODLY_API = "http://127.0.0.1:8765" + + +def _modly_headers() -> dict[str, str]: + token = os.environ.get(TOKEN_ENV, "").strip() + if not token: + return {} + return {"Authorization": f"Bearer {token}", "X-Modly-Token": token} SYSTEM_PROMPT = """\ You are Modly's built-in AI assistant, specialized in 3D modeling and workflow automation. @@ -255,7 +265,8 @@ async def execute_tool(name: str, arguments: dict, context: dict) -> tuple[str, """Execute a tool and return (result_text, action_payload). action_payload carries data the frontend needs to react (e.g. new mesh URL). """ - async with httpx.AsyncClient(timeout=60.0) as client: + headers = _modly_headers() + async with httpx.AsyncClient(timeout=60.0, headers=headers) as client: try: if name == "list_models": r = await client.get(f"{MODLY_API}/model/all") diff --git a/api/routers/export.py b/api/routers/export.py index 2a2f2bf3..e3b84160 100644 --- a/api/routers/export.py +++ b/api/routers/export.py @@ -5,6 +5,7 @@ from fastapi.responses import Response, FileResponse from services.generator_registry import WORKSPACE_DIR +from services.local_paths import resolve_workspace_file router = APIRouter(tags=["export"]) @@ -16,9 +17,10 @@ def export_mesh(fmt: str, path: str): if fmt not in SUPPORTED: raise HTTPException(400, f"Unsupported format: {fmt}. Supported: {', '.join(SUPPORTED)}") - full_path = (WORKSPACE_DIR / path).resolve() - if not str(full_path).startswith(str(WORKSPACE_DIR.resolve())): - raise HTTPException(400, "Invalid path") + try: + full_path = resolve_workspace_file(WORKSPACE_DIR, path) + except ValueError as exc: + raise HTTPException(400, str(exc)) from exc if not full_path.exists(): raise HTTPException(404, f"File not found: {path}") diff --git a/api/routers/extensions.py b/api/routers/extensions.py index 2313d3b3..0c3e706a 100644 --- a/api/routers/extensions.py +++ b/api/routers/extensions.py @@ -3,6 +3,8 @@ import sys from fastapi import APIRouter, HTTPException +from services.local_paths import assert_safe_extension_id + router = APIRouter(tags=["extensions"]) @@ -33,6 +35,11 @@ async def setup_extension(ext_id: str): if EXTENSIONS_DIR is None or not EXTENSIONS_DIR.exists(): raise HTTPException(400, "EXTENSIONS_DIR not configured") + try: + ext_id = assert_safe_extension_id(ext_id) + except ValueError as exc: + raise HTTPException(400, str(exc)) from exc + ext_dir = EXTENSIONS_DIR / ext_id setup_py = ext_dir / "setup.py" diff --git a/api/routers/model.py b/api/routers/model.py index 4f04718b..5fd4aceb 100644 --- a/api/routers/model.py +++ b/api/routers/model.py @@ -8,7 +8,7 @@ from typing import Optional from urllib.error import HTTPError, URLError from urllib.request import Request, urlopen -from fastapi import APIRouter, HTTPException +from fastapi import APIRouter, HTTPException, Request from fastapi.responses import StreamingResponse from services.generator_registry import generator_registry, MODELS_DIR @@ -124,6 +124,7 @@ async def cancel_hf_download(model_id: str): @router.get("/hf-download") async def hf_download( + request: Request, repo_id: str, model_id: str, skip_prefixes: Optional[str] = None, @@ -168,8 +169,14 @@ async def hf_download( except KeyError: include_list = [] - # Token: explicit query param > env var > None - hf_token = token or os.environ.get("HUGGING_FACE_HUB_TOKEN") or os.environ.get("HF_TOKEN") or None + # Prefer header (Electron) over leftover query param, then process env. + hf_token = ( + request.headers.get("x-huggingface-token") + or token + or os.environ.get("HUGGING_FACE_HUB_TOKEN") + or os.environ.get("HF_TOKEN") + or None + ) control = _new_download_control(model_id) async def stream(): diff --git a/api/routers/optimize.py b/api/routers/optimize.py index 6081c704..31226120 100644 --- a/api/routers/optimize.py +++ b/api/routers/optimize.py @@ -22,6 +22,7 @@ from pydantic import BaseModel from services.generator_registry import WORKSPACE_DIR +from services.local_paths import resolve_readable_mesh_path, resolve_workspace_file router = APIRouter(tags=["optimize"]) @@ -47,16 +48,10 @@ def _require_pymeshlab(): def _resolve_input_path(raw_path: str) -> Path: - candidate = Path(raw_path) - if candidate.is_absolute(): - resolved = candidate.resolve() - if not resolved.exists(): - raise HTTPException(404, f"File not found: {raw_path}") - return resolved - - resolved = (WORKSPACE_DIR / raw_path).resolve() - if not str(resolved).startswith(str(WORKSPACE_DIR.resolve())): - raise HTTPException(400, "Invalid path") + try: + resolved = resolve_readable_mesh_path(WORKSPACE_DIR, raw_path) + except ValueError as exc: + raise HTTPException(400, str(exc)) from exc if not resolved.exists(): raise HTTPException(404, f"File not found: {raw_path}") return resolved @@ -404,10 +399,15 @@ async def import_mesh_by_path(body: ImportByPathRequest): if ext not in ("glb", "obj", "stl", "ply", "splat"): raise HTTPException(400, f"Unsupported format: {ext}") - # Gaussian Splat: serve a .splat as-is, convert a GS .ply to .splat. - # The viewer detects splats by the .splat/.ply extension in the served URL. + def _imported(src: Path, dest_name: str) -> str: + tmp_dir = tempfile.mkdtemp(prefix="modly_import_") + dest = Path(tmp_dir) / dest_name + shutil.copy2(src, dest) + return f"/optimize/serve-file?path={quote(str(dest))}" + + # Gaussian Splat: copy into a Modly temp dir so serve-file stays jailed. if ext == "splat": - return {"url": f"/optimize/serve-file?path={quote(str(file_path))}"} + return {"url": _imported(file_path, "splat.splat")} if ext == "ply" and _is_gaussian_ply(file_path): tmp_dir = tempfile.mkdtemp(prefix="modly_import_") @@ -419,8 +419,7 @@ async def import_mesh_by_path(body: ImportByPathRequest): return {"url": f"/optimize/serve-file?path={quote(output_path)}"} if ext == "glb": - # Serve the original file directly — no copy - return {"url": f"/optimize/serve-file?path={quote(str(file_path))}"} + return {"url": _imported(file_path, "mesh.glb")} # Mesh ply / obj / stl: convert to GLB in a temp directory (not the workspace) tmp_dir = tempfile.mkdtemp(prefix="modly_import_") @@ -438,7 +437,10 @@ async def import_mesh_by_path(body: ImportByPathRequest): @router.get("/serve-file") def serve_file(path: str): - file_path = Path(path) + try: + file_path = resolve_readable_mesh_path(WORKSPACE_DIR, path) + except ValueError as exc: + raise HTTPException(400, str(exc)) from exc if not file_path.is_file(): raise HTTPException(404, "File not found") media_type = _SERVE_MEDIA_TYPES.get(file_path.suffix.lower()) @@ -455,10 +457,11 @@ def ply_to_splat(path: str): as-is; a GS .ply is normalised + converted (cached by mtime + conv version). """ import services.generator_registry as reg # dynamic: workspace dir may change at runtime - workspace = reg.WORKSPACE_DIR.resolve() - src = (workspace / path).resolve() - if not str(src).startswith(str(workspace)): - raise HTTPException(400, "Invalid path") + workspace = reg.WORKSPACE_DIR + try: + src = resolve_workspace_file(workspace, path) + except ValueError as exc: + raise HTTPException(400, str(exc)) from exc if not src.is_file(): raise HTTPException(404, "File not found") @@ -482,9 +485,10 @@ def export_mesh(path: str, format: str): if format not in ("obj", "stl", "ply"): raise HTTPException(400, "Supported formats: obj, stl, ply") - input_path = (WORKSPACE_DIR / path).resolve() - if not str(input_path).startswith(str(WORKSPACE_DIR.resolve())): - raise HTTPException(400, "Invalid path") + try: + input_path = resolve_workspace_file(WORKSPACE_DIR, path) + except ValueError as exc: + raise HTTPException(400, str(exc)) from exc if not input_path.exists(): raise HTTPException(404, f"File not found: {path}") diff --git a/api/routers/status.py b/api/routers/status.py index e54b0a0b..a407a07c 100644 --- a/api/routers/status.py +++ b/api/routers/status.py @@ -1,9 +1,11 @@ from fastapi import APIRouter +from services.api_guard import auth_mode + router = APIRouter(tags=["health"]) @router.get("/health") async def health(): - """Health check — used by Electron to know the API is ready.""" - return {"status": "ok"} + """Health check — used by Electron to know the API is ready. Unauthenticated on purpose.""" + return {"status": "ok", "auth": auth_mode()} diff --git a/api/services/api_guard.py b/api/services/api_guard.py new file mode 100644 index 00000000..f3dfb1e6 --- /dev/null +++ b/api/services/api_guard.py @@ -0,0 +1,102 @@ +"""Local-API access control: loopback host, local Origin, optional bearer token. + +Electron always sets MODLY_API_TOKEN. Headless `uvicorn` without the env var +stays usable for trusted-machine development, but browser requests from a +non-local Origin are still rejected. +""" +from __future__ import annotations + +import hmac +import os +from urllib.parse import urlparse + +from fastapi import Request +from fastapi.responses import JSONResponse +from starlette.middleware.base import BaseHTTPMiddleware + +TOKEN_HEADER = "x-modly-token" +TOKEN_ENV = "MODLY_API_TOKEN" +ALLOW_REMOTE_ENV = "MODLY_API_ALLOW_REMOTE" +PUBLIC_PATHS = {"/health"} + + +def api_token() -> str: + return os.environ.get(TOKEN_ENV, "").strip() + + +def allow_remote() -> bool: + return os.environ.get(ALLOW_REMOTE_ENV, "").strip().lower() in {"1", "true", "yes"} + + +def auth_mode() -> str: + return "required" if api_token() else "off" + + +def _hostname_from_host(host: str) -> str: + value = (host or "").strip() + if value.startswith("["): + end = value.find("]") + if end != -1: + return value[1:end].lower() + return value.rsplit(":", 1)[0].lower() + + +def is_loopback_host(host: str) -> bool: + hostname = _hostname_from_host(host) + return hostname in {"127.0.0.1", "localhost", "::1"} + + +def is_local_origin(origin: str | None) -> bool: + if origin is None or origin == "" or origin == "null": + return True + parsed = urlparse(origin) + if parsed.scheme in {"file", "app"}: + return True + if not parsed.hostname: + return origin.startswith("file://") + return parsed.hostname.lower() in {"127.0.0.1", "localhost", "::1"} + + +def extract_bearer_token(authorization: str | None, header_token: str | None) -> str: + if header_token and header_token.strip(): + return header_token.strip() + if not authorization: + return "" + scheme, _, remainder = authorization.partition(" ") + if scheme.lower() != "bearer": + return "" + return remainder.strip() + + +def tokens_match(expected: str, provided: str) -> bool: + if not expected or not provided: + return False + return hmac.compare_digest(expected.encode("utf-8"), provided.encode("utf-8")) + + +class LocalApiGuardMiddleware(BaseHTTPMiddleware): + async def dispatch(self, request: Request, call_next): + path = request.url.path + # Preflight has no Authorization header; CORSMiddleware answers OPTIONS. + if path in PUBLIC_PATHS or request.method == "OPTIONS": + return await call_next(request) + + if not allow_remote(): + host = request.headers.get("host", "") + if host and not is_loopback_host(host): + return JSONResponse({"detail": "Host is not a loopback address"}, status_code=403) + + origin = request.headers.get("origin") + if origin and not is_local_origin(origin): + return JSONResponse({"detail": "Origin is not allowed"}, status_code=403) + + expected = api_token() + if expected: + provided = extract_bearer_token( + request.headers.get("authorization"), + request.headers.get(TOKEN_HEADER), + ) + if not tokens_match(expected, provided): + return JSONResponse({"detail": "Missing or invalid API token"}, status_code=401) + + return await call_next(request) diff --git a/api/services/local_paths.py b/api/services/local_paths.py new file mode 100644 index 00000000..e58e009b --- /dev/null +++ b/api/services/local_paths.py @@ -0,0 +1,84 @@ +"""Path confinement helpers for the local FastAPI server.""" +from __future__ import annotations + +import os +import tempfile +from pathlib import Path + +_EXTENSION_ID_CHARS = set("abcdefghijklmnopqrstuvwxyz0123456789._-") + + +def is_within_directory(root: Path, candidate: Path) -> bool: + """True when *candidate* resolves inside *root* (symlink-aware).""" + try: + root_r = os.path.realpath(root) + cand_r = os.path.realpath(candidate) + return os.path.commonpath([root_r, cand_r]) == root_r + except (ValueError, OSError): + return False + + +def _is_windows_drive(component: str) -> bool: + return len(component) >= 2 and component[0].isalpha() and component[1] == ":" + + +def resolve_workspace_file(workspace: Path, raw: str) -> Path: + """Resolve a workspace-relative path and reject traversal / absolute inputs.""" + value = str(raw or "").strip() + if not value: + raise ValueError("path is required") + + normalized = value.replace("\\", "/") + if normalized.startswith("/") or "://" in normalized: + raise ValueError("absolute or remote paths are not allowed") + + parts = [part for part in normalized.split("/") if part not in ("", ".")] + if not parts: + raise ValueError("path is required") + if any(part == ".." for part in parts): + raise ValueError("path traversal is not allowed") + if any(_is_windows_drive(part) for part in parts): + raise ValueError("absolute or remote paths are not allowed") + + resolved = (workspace / value).resolve() + if not is_within_directory(workspace, resolved): + raise ValueError("path escapes workspace") + return resolved + + +def is_modly_temp_file(path: Path) -> bool: + """True for files Modly itself wrote under the process temp dir.""" + resolved = path.resolve() + tmp = Path(tempfile.gettempdir()).resolve() + if not is_within_directory(tmp, resolved): + return False + if resolved.name.startswith("modly_splat_"): + return True + return any(part.startswith("modly_import_") for part in resolved.parts) + + +def resolve_readable_mesh_path(workspace: Path, raw: str) -> Path: + """Workspace-relative path, or an already-imported absolute temp/workspace file.""" + value = str(raw or "").strip() + if not value: + raise ValueError("path is required") + + candidate = Path(value) + if candidate.is_absolute(): + resolved = candidate.resolve() + if is_within_directory(workspace, resolved) or is_modly_temp_file(resolved): + return resolved + raise ValueError("absolute path is outside the workspace") + + return resolve_workspace_file(workspace, value) + + +def assert_safe_extension_id(ext_id: str) -> str: + value = str(ext_id or "").strip() + if not value or value in {".", ".."}: + raise ValueError("invalid extension id") + if "/" in value or "\\" in value: + raise ValueError("invalid extension id") + if value[0] == "." or any(ch not in _EXTENSION_ID_CHARS for ch in value): + raise ValueError("invalid extension id") + return value diff --git a/api/tests/test_api_guard.py b/api/tests/test_api_guard.py new file mode 100644 index 00000000..0372d233 --- /dev/null +++ b/api/tests/test_api_guard.py @@ -0,0 +1,104 @@ +import unittest +from unittest.mock import patch + +try: + from fastapi import FastAPI + from fastapi.testclient import TestClient + from services.api_guard import ( + LocalApiGuardMiddleware, + extract_bearer_token, + is_local_origin, + is_loopback_host, + tokens_match, + ) + HAS_FASTAPI = True +except ImportError: # pragma: no cover - system Python without api/requirements.txt + HAS_FASTAPI = False + FastAPI = TestClient = LocalApiGuardMiddleware = None # type: ignore[misc, assignment] + + +def _app() -> FastAPI: + app = FastAPI() + app.add_middleware(LocalApiGuardMiddleware) + + @app.get("/health") + def health(): + return {"status": "ok"} + + @app.get("/secret") + def secret(): + return {"ok": True} + + return app + + +@unittest.skipUnless(HAS_FASTAPI, "fastapi is not installed in this Python") +class GuardHelperTests(unittest.TestCase): + def test_loopback_hosts(self) -> None: + self.assertTrue(is_loopback_host("127.0.0.1:8765")) + self.assertTrue(is_loopback_host("localhost")) + self.assertTrue(is_loopback_host("[::1]:8765")) + self.assertFalse(is_loopback_host("evil.example:8765")) + self.assertFalse(is_loopback_host("192.168.1.10:8000")) + + def test_local_origins(self) -> None: + self.assertTrue(is_local_origin(None)) + self.assertTrue(is_local_origin("null")) + self.assertTrue(is_local_origin("http://127.0.0.1:5173")) + self.assertTrue(is_local_origin("http://localhost:5173")) + self.assertTrue(is_local_origin("file://")) + self.assertFalse(is_local_origin("https://evil.example")) + + def test_bearer_extract_and_compare(self) -> None: + self.assertEqual(extract_bearer_token("Bearer abc", None), "abc") + self.assertEqual(extract_bearer_token(None, "xyz"), "xyz") + self.assertTrue(tokens_match("secret", "secret")) + self.assertFalse(tokens_match("secret", "other")) + self.assertFalse(tokens_match("", "")) + + +@unittest.skipUnless(HAS_FASTAPI, "fastapi is not installed in this Python") +class GuardMiddlewareTests(unittest.TestCase): + def setUp(self) -> None: + self.env = patch.dict("os.environ", {"MODLY_API_TOKEN": "", "MODLY_API_ALLOW_REMOTE": ""}, clear=False) + self.env.start() + self.addCleanup(self.env.stop) + + def test_health_is_public(self) -> None: + client = TestClient(_app(), base_url="http://127.0.0.1") + self.assertEqual(client.get("/health").status_code, 200) + + def test_rejects_foreign_origin(self) -> None: + client = TestClient(_app(), base_url="http://127.0.0.1") + res = client.get("/secret", headers={"Origin": "https://evil.example"}) + self.assertEqual(res.status_code, 403) + + def test_rejects_non_loopback_host(self) -> None: + client = TestClient(_app(), base_url="http://127.0.0.1") + res = client.get("/secret", headers={"Host": "evil.example"}) + self.assertEqual(res.status_code, 403) + + def test_token_required_when_configured(self) -> None: + with patch.dict("os.environ", {"MODLY_API_TOKEN": "s3cret"}, clear=False): + client = TestClient(_app(), base_url="http://127.0.0.1") + self.assertEqual(client.get("/secret").status_code, 401) + ok = client.get("/secret", headers={"Authorization": "Bearer s3cret"}) + self.assertEqual(ok.status_code, 200) + alt = client.get("/secret", headers={"X-Modly-Token": "s3cret"}) + self.assertEqual(alt.status_code, 200) + + def test_allow_remote_skips_host_origin_but_not_token(self) -> None: + env = {"MODLY_API_TOKEN": "s3cret", "MODLY_API_ALLOW_REMOTE": "1"} + with patch.dict("os.environ", env, clear=False): + client = TestClient(_app(), base_url="http://127.0.0.1") + denied = client.get("/secret", headers={"Host": "jetson.local", "Origin": "http://192.168.1.10"}) + self.assertEqual(denied.status_code, 401) + ok = client.get( + "/secret", + headers={"Host": "jetson.local", "Authorization": "Bearer s3cret"}, + ) + self.assertEqual(ok.status_code, 200) + + +if __name__ == "__main__": + unittest.main() diff --git a/api/tests/test_local_paths.py b/api/tests/test_local_paths.py new file mode 100644 index 00000000..e40af381 --- /dev/null +++ b/api/tests/test_local_paths.py @@ -0,0 +1,71 @@ +import os +import tempfile +import unittest +from pathlib import Path + +from services.local_paths import ( + assert_safe_extension_id, + is_modly_temp_file, + is_within_directory, + resolve_readable_mesh_path, + resolve_workspace_file, +) + + +class LocalPathsTests(unittest.TestCase): + def test_workspace_relative_ok(self) -> None: + with tempfile.TemporaryDirectory() as td: + root = Path(td) + (root / "Default").mkdir() + target = root / "Default" / "mesh.glb" + target.write_bytes(b"glb") + self.assertEqual(resolve_workspace_file(root, "Default/mesh.glb"), target.resolve()) + + def test_workspace_rejects_traversal(self) -> None: + with tempfile.TemporaryDirectory() as td: + root = Path(td) + with self.assertRaises(ValueError): + resolve_workspace_file(root, "../secret.glb") + with self.assertRaises(ValueError): + resolve_workspace_file(root, "Default/../../etc/passwd") + with self.assertRaises(ValueError): + resolve_workspace_file(root, "/etc/passwd") + + def test_readable_path_allows_modly_temp(self) -> None: + with tempfile.TemporaryDirectory() as td: + workspace = Path(td) / "workspace" + workspace.mkdir() + tmp = Path(tempfile.mkdtemp(prefix="modly_import_", dir=tempfile.gettempdir())) + self.addCleanup(lambda: __import__("shutil").rmtree(tmp, ignore_errors=True)) + mesh = tmp / "mesh.glb" + mesh.write_bytes(b"glb") + self.assertTrue(is_modly_temp_file(mesh)) + self.assertEqual(resolve_readable_mesh_path(workspace, str(mesh)), mesh.resolve()) + + def test_readable_path_rejects_other_absolute(self) -> None: + with tempfile.TemporaryDirectory() as td: + workspace = Path(td) / "workspace" + workspace.mkdir() + outsider = Path(td) / "secret.glb" + outsider.write_bytes(b"no") + with self.assertRaises(ValueError): + resolve_readable_mesh_path(workspace, str(outsider)) + + def test_within_directory(self) -> None: + with tempfile.TemporaryDirectory() as td: + root = Path(td) + child = root / "a" / "b" + child.mkdir(parents=True) + self.assertTrue(is_within_directory(root, child)) + self.assertFalse(is_within_directory(root, Path(os.path.dirname(td)))) + + def test_extension_id(self) -> None: + self.assertEqual(assert_safe_extension_id("mesh-optimizer"), "mesh-optimizer") + with self.assertRaises(ValueError): + assert_safe_extension_id("../escape") + with self.assertRaises(ValueError): + assert_safe_extension_id("Bad Id") + + +if __name__ == "__main__": + unittest.main() diff --git a/docs/running-on-jetson.md b/docs/running-on-jetson.md index 4d21bdcb..4f883109 100644 --- a/docs/running-on-jetson.md +++ b/docs/running-on-jetson.md @@ -245,11 +245,19 @@ WORKSPACE_DIR=$HOME/.modly/workspace \ Check it (from the Jetson, or from another machine using the Jetson's IP): ```bash -curl http://127.0.0.1:8000/health # {"status":"ok"} +curl http://127.0.0.1:8000/health # {"status":"ok","auth":"off"} curl http://127.0.0.1:8000/model/all # lists hunyuan3d-mini/generate curl http://127.0.0.1:8000/extensions/errors # {} == no load errors ``` +If this uvicorn is reachable from other machines, set a token before starting it: + +```bash +export MODLY_API_TOKEN=$(python3 -c 'import secrets; print(secrets.token_hex(32))') +export MODLY_API_ALLOW_REMOTE=1 +# then pass: -H "Authorization: Bearer $MODLY_API_TOKEN" +``` + --- ## 6. Generate a mesh diff --git a/electron/main/index.ts b/electron/main/index.ts index 8cf9c810..bbc33080 100644 --- a/electron/main/index.ts +++ b/electron/main/index.ts @@ -2,7 +2,7 @@ import { app, BrowserWindow, shell, session } from 'electron' import { join } from 'path' import { electronApp, optimizer, is } from '@electron-toolkit/utils' import { setupIpcHandlers } from './ipc-handlers' -import { PythonBridge } from './python-bridge' +import { getApiAuthHeaders, PythonBridge } from './python-bridge' import { logger, archiveCurrentSession } from './logger' import { initAutoUpdater } from './updater' import { syncBuiltinExtensions } from './builtin-sync' @@ -94,6 +94,16 @@ app.whenReady().then(async () => { // Clear Chromium disk cache on startup to recover from any corruption await session.defaultSession.clearCache() + // Renderer fetches (useGLTF, splat viewer, , axios) cannot set + // a secret header themselves. Inject the session token for loopback only. + session.defaultSession.webRequest.onBeforeSendHeaders( + { urls: ['http://127.0.0.1:8765/*', 'http://localhost:8765/*'] }, + (details, callback) => { + const headers = { ...details.requestHeaders, ...getApiAuthHeaders() } + callback({ requestHeaders: headers }) + }, + ) + app.on('browser-window-created', (_, window) => { optimizer.watchWindowShortcuts(window) }) diff --git a/electron/main/ipc-handlers.ts b/electron/main/ipc-handlers.ts index 14fc984a..569b141f 100644 --- a/electron/main/ipc-handlers.ts +++ b/electron/main/ipc-handlers.ts @@ -8,7 +8,7 @@ import axios from 'axios' import * as tar from 'tar' import * as os from 'os' import { promisify } from 'util' -import { PythonBridge, API_BASE_URL } from './python-bridge' +import { PythonBridge, API_BASE_URL, apiHttp } from './python-bridge' import { isModelDownloaded, listDownloadedModels, @@ -469,7 +469,7 @@ export function setupIpcHandlers(pythonBridge: PythonBridge, getWindow: WindowGe ipcMain.handle('model:unloadAll', async (): Promise<{ success: boolean; error?: string }> => { try { - await axios.post(`${API_BASE_URL}/model/unload-all`, {}, { timeout: 10_000 }) + await apiHttp.post('/model/unload-all', {}, { timeout: 10_000 }) return { success: true } } catch (err) { return { success: false, error: String(err) } @@ -481,7 +481,7 @@ export function setupIpcHandlers(pythonBridge: PythonBridge, getWindow: WindowGe // Unload the model and wait for confirmation so file handles are released try { - await axios.post(`${API_BASE_URL}/model/unload/${encodeURIComponent(modelId)}`, {}, { timeout: 10_000 }) + await apiHttp.post(`/model/unload/${encodeURIComponent(modelId)}`, {}, { timeout: 10_000 }) // Give the OS a moment to release file locks (Windows holds handles briefly after close) await new Promise(resolve => setTimeout(resolve, 1_500)) } catch { @@ -570,7 +570,7 @@ export function setupIpcHandlers(pythonBridge: PythonBridge, getWindow: WindowGe ipcMain.handle('model:pauseDownload', async (_, modelId: string): Promise<{ success: boolean; error?: string }> => { try { - await axios.post(`${API_BASE_URL}/model/hf-download/pause`, null, { + await apiHttp.post('/model/hf-download/pause', null, { params: { model_id: modelId }, timeout: 5000, }) @@ -582,7 +582,7 @@ export function setupIpcHandlers(pythonBridge: PythonBridge, getWindow: WindowGe ipcMain.handle('model:cancelDownload', async (_, modelId: string): Promise<{ success: boolean; error?: string }> => { try { - await axios.post(`${API_BASE_URL}/model/hf-download/cancel`, null, { + await apiHttp.post('/model/hf-download/cancel', null, { params: { model_id: modelId }, timeout: 5000, }) @@ -612,8 +612,8 @@ export function setupIpcHandlers(pythonBridge: PythonBridge, getWindow: WindowGe if (result.canceled || !result.filePath) return { success: false } try { - const response = await axios.get( - `${API_BASE_URL}/export/${format}?path=${encodeURIComponent(meshPath)}`, + const response = await apiHttp.get( + `/export/${format}?path=${encodeURIComponent(meshPath)}`, { responseType: 'arraybuffer' } ) await writeFile(result.filePath, Buffer.from(response.data as ArrayBuffer)) @@ -692,7 +692,7 @@ export function setupIpcHandlers(pythonBridge: PythonBridge, getWindow: WindowGe // subprocesses spawned by ExtensionProcess._build_env() pick it up // without requiring a full app restart. try { - await axios.post(`${API_BASE_URL}/settings/hf-token`, { token: patch.hfToken }, { timeout: 3000 }) + await apiHttp.post('/settings/hf-token', { token: patch.hfToken }, { timeout: 3000 }) } catch { /* FastAPI may not be running yet — ignore */ } } return updated @@ -1211,7 +1211,7 @@ export function setupIpcHandlers(pythonBridge: PythonBridge, getWindow: WindowGe // Hot-reload Python so it picks up the new/updated model extension if (!isProcess) { try { - await axios.post(`${API_BASE_URL}/extensions/reload`, {}, { timeout: 10_000 }) + await apiHttp.post('/extensions/reload', {}, { timeout: 10_000 }) } catch { /* Python might not be running yet */ } } @@ -1263,7 +1263,7 @@ export function setupIpcHandlers(pythonBridge: PythonBridge, getWindow: WindowGe } // Hot-reload Python so it stops using the deleted model extension try { - await axios.post(`${API_BASE_URL}/extensions/reload`, {}, { timeout: 10_000 }) + await apiHttp.post('/extensions/reload', {}, { timeout: 10_000 }) } catch { /* ignore if Python is not running */ } return { success: true } } catch (err) { @@ -1282,7 +1282,7 @@ export function setupIpcHandlers(pythonBridge: PythonBridge, getWindow: WindowGe const { sm: gpuSm, cudaVersion } = await detectGpuInfo() await runExtensionSetup(extDir, gpuSm, cudaVersion, (line) => logger.info(`[ext-repair] ${line}`)) try { - await axios.post(`${API_BASE_URL}/extensions/reload`, {}, { timeout: 10_000 }) + await apiHttp.post('/extensions/reload', {}, { timeout: 10_000 }) } catch { /* ignore if Python is not running yet */ } return { success: true } } catch (err: any) { @@ -1366,7 +1366,7 @@ export function setupIpcHandlers(pythonBridge: PythonBridge, getWindow: WindowGe // 4. Hot-reload Python registry so it picks up the new extension try { - await axios.post(`${API_BASE_URL}/extensions/reload`, {}, { timeout: 10_000 }) + await apiHttp.post('/extensions/reload', {}, { timeout: 10_000 }) } catch { /* Python might not be running yet */ } emit({ step: 'done', extensionId }) @@ -1387,7 +1387,7 @@ export function setupIpcHandlers(pythonBridge: PythonBridge, getWindow: WindowGe ipcMain.handle('extensions:reload', async () => { terminateAllProcessRunners() try { - const res = await axios.post(`${API_BASE_URL}/extensions/reload`, {}, { timeout: 10_000 }) + const res = await apiHttp.post('/extensions/reload', {}, { timeout: 10_000 }) return { success: true, errors: (res.data as { errors?: Record }).errors ?? {} } } catch (err) { return { success: false, error: String(err) } @@ -1455,7 +1455,7 @@ export function setupIpcHandlers(pythonBridge: PythonBridge, getWindow: WindowGe // Update FastAPI paths at runtime (without restarting) ipcMain.handle('api:updatePaths', async (_event, patch: { modelsDir?: string; workspaceDir?: string; extensionsDir?: string }) => { try { - await axios.post(`${API_BASE_URL}/settings/paths`, { + await apiHttp.post('/settings/paths', { models_dir: patch.modelsDir, workspace_dir: patch.workspaceDir, extensions_dir: patch.extensionsDir, diff --git a/electron/main/model-downloader.ts b/electron/main/model-downloader.ts index 8c5571d9..61c3e878 100644 --- a/electron/main/model-downloader.ts +++ b/electron/main/model-downloader.ts @@ -5,6 +5,7 @@ import { existsSync, readdirSync, statSync, readFileSync } from 'fs' import { join } from 'path' import { getSettings } from './settings-store' +import { getApiAuthHeaders } from './python-bridge' import { app } from 'electron' export interface DownloadProgress { @@ -129,12 +130,13 @@ export async function downloadModelFromHF( if (includePrefixes && includePrefixes.length > 0) { url += `&include_prefixes=${encodeURIComponent(JSON.stringify(includePrefixes))}` } + const headers: Record = { ...getApiAuthHeaders() } const hfToken = getSettings(app.getPath('userData')).hfToken if (hfToken) { - url += `&token=${encodeURIComponent(hfToken)}` + headers['X-HuggingFace-Token'] = hfToken } - const res = await net.fetch(url) + const res = await net.fetch(url, { headers }) if (!res.ok) throw new Error(`HuggingFace download failed: HTTP ${res.status}`) if (!res.body) throw new Error('No response body from HF download stream') diff --git a/electron/main/python-bridge.ts b/electron/main/python-bridge.ts index 94dcfb63..6861b651 100644 --- a/electron/main/python-bridge.ts +++ b/electron/main/python-bridge.ts @@ -1,7 +1,8 @@ import { ChildProcess, spawn } from 'child_process' import { join } from 'path' import { app, BrowserWindow } from 'electron' -import { existsSync, mkdirSync } from 'fs' +import { chmodSync, existsSync, mkdirSync, writeFileSync } from 'fs' +import { randomBytes } from 'crypto' import axios from 'axios' import { getSettings } from './settings-store' import { logger } from './logger' @@ -10,6 +11,41 @@ import { cleanPythonEnv, getVenvPythonExe } from './python-setup' const API_PORT = 8765 const API_HOST = '127.0.0.1' export const API_BASE_URL = `http://${API_HOST}:${API_PORT}` +export const API_TOKEN_FILENAME = 'api-token' + +let apiToken = '' + +export function getApiToken(): string { + return apiToken +} + +export function getApiAuthHeaders(): Record { + if (!apiToken) return {} + return { + Authorization: `Bearer ${apiToken}`, + 'X-Modly-Token': apiToken, + } +} + +export const apiHttp = axios.create({ baseURL: API_BASE_URL }) +apiHttp.interceptors.request.use((config) => { + config.headers = { ...config.headers, ...getApiAuthHeaders() } + return config +}) + +function persistApiToken(userData: string, token: string): void { + const tokenPath = join(userData, API_TOKEN_FILENAME) + writeFileSync(tokenPath, token, { encoding: 'utf-8', mode: 0o600 }) + try { chmodSync(tokenPath, 0o600) } catch { /* Windows ignores POSIX modes */ } +} + +function ensureApiToken(userData: string): string { + if (!apiToken) { + apiToken = randomBytes(32).toString('hex') + persistApiToken(userData, apiToken) + } + return apiToken +} export class PythonBridge { private process: ChildProcess | null = null @@ -41,6 +77,8 @@ export class PythonBridge { const pythonExecutable = this.resolvePythonExecutable() const apiDir = this.resolveApiDir() + const userData = app.getPath('userData') + const token = ensureApiToken(userData) console.log('[PythonBridge] Starting FastAPI at', apiDir) console.log('[PythonBridge] Python executable:', pythonExecutable) @@ -59,6 +97,7 @@ export class PythonBridge { SELECTED_MODEL_ID: process.env['SELECTED_MODEL_ID'] ?? '', HUGGING_FACE_HUB_TOKEN: this.resolveHfToken(), HF_TOKEN: this.resolveHfToken(), + MODLY_API_TOKEN: token, }, // On Unix, put the bridge in its own process group so every subprocess // it spawns (extension runners, etc.) inherits that group. On shutdown diff --git a/scripts/run-pytests.mjs b/scripts/run-pytests.mjs index acb1e72f..e41684f3 100644 --- a/scripts/run-pytests.mjs +++ b/scripts/run-pytests.mjs @@ -34,4 +34,8 @@ const result = spawnSync(cmd, [...prefix, '-m', 'unittest', 'discover', '-s', 't cwd: apiDir, stdio: 'inherit', }) -process.exit(result.status ?? 1) +if ((result.status ?? 1) !== 0) process.exit(result.status ?? 1) + +const cliTest = join(dirname(fileURLToPath(import.meta.url)), '..', 'tools', 'modly-cli', 'test_agent.py') +const cli = spawnSync(cmd, [...prefix, cliTest], { stdio: 'inherit' }) +process.exit(cli.status ?? 1) diff --git a/tools/modly-cli/SKILL.md b/tools/modly-cli/SKILL.md index 8b492a6e..c4711bd8 100644 --- a/tools/modly-cli/SKILL.md +++ b/tools/modly-cli/SKILL.md @@ -14,7 +14,9 @@ metadata: ## Overview -Modly exposes a local API at `http://127.0.0.1:8765` while the official desktop app is running. The stdlib-only CLI at `tools/modly-cli/agent.py` is an agent helper over the canonical automation contract: +Modly exposes a local API at `http://127.0.0.1:8765` while the official desktop app is running. The desktop process authenticates that API with a per-session bearer token. The CLI sends it automatically from, in order: `--token`, `MODLY_API_TOKEN`, or `api-token` in Electron userData (`~/Library/Application Support/Modly` on macOS, `%APPDATA%/Modly` on Windows, `~/.config/Modly` on Linux). `GET /health` stays unauthenticated so readiness checks still work. + +The stdlib-only CLI at `tools/modly-cli/agent.py` is an agent helper over the canonical automation contract: - `health` - `model` diff --git a/tools/modly-cli/agent.py b/tools/modly-cli/agent.py index 8540ef31..7a912778 100644 --- a/tools/modly-cli/agent.py +++ b/tools/modly-cli/agent.py @@ -11,6 +11,7 @@ import json import mimetypes import os +import secrets import subprocess import sys import tempfile @@ -38,6 +39,7 @@ def _float_env(primary: str, fallback: str, default: float) -> float: DEFAULT_BASE_URL = os.environ.get("MODLY_API_URL", "http://127.0.0.1:8765") +_CLI_TOKEN = "" DEFAULT_TIMEOUT_SECONDS = _int_env("MODLY_CLI_TIMEOUT", "MODLY_AGENT_TIMEOUT", 1800) DEFAULT_POLL_SECONDS = _float_env("MODLY_CLI_POLL_SECONDS", "MODLY_AGENT_POLL_SECONDS", 2.0) EXPORT_FORMATS = ("glb", "stl", "obj", "ply") @@ -70,7 +72,7 @@ def _request_json( data: bytes | None = None, headers: dict[str, str] | None = None, ) -> Any: - req = urllib.request.Request(url, data=data, method=method, headers=headers or {}) + req = urllib.request.Request(url, data=data, method=method, headers=_api_headers(headers)) try: with urllib.request.urlopen(req, timeout=timeout) as resp: raw = resp.read().decode("utf-8") @@ -88,7 +90,8 @@ def _request_json( def _download(url: str, dest: Path, *, timeout: float) -> int: dest.parent.mkdir(parents=True, exist_ok=True) try: - with urllib.request.urlopen(url, timeout=timeout) as resp, dest.open("wb") as fh: + req = urllib.request.Request(url, headers=_api_headers()) + with urllib.request.urlopen(req, timeout=timeout) as resp, dest.open("wb") as fh: total = 0 while True: chunk = resp.read(1024 * 1024) @@ -297,12 +300,17 @@ def _default_python(api_dir: Path) -> Path | None: return None -def _load_modly_settings() -> dict[str, Any]: - candidates: list[Path] = [] +def _modly_user_data_dirs() -> list[Path]: + dirs: list[Path] = [Path.home() / "Library" / "Application Support" / "Modly"] for appdata in _windows_env_paths("APPDATA"): - candidates.append(appdata / "Modly" / "settings.json") - candidates.append(Path.home() / ".config" / "Modly" / "settings.json") - for path in candidates: + dirs.append(appdata / "Modly") + dirs.append(Path.home() / ".config" / "Modly") + return dirs + + +def _load_modly_settings() -> dict[str, Any]: + for directory in _modly_user_data_dirs(): + path = directory / "settings.json" if path.exists(): try: data = json.loads(path.read_text(encoding="utf-8")) @@ -312,6 +320,45 @@ def _load_modly_settings() -> dict[str, Any]: return {} +def _read_token_file() -> str: + for directory in _modly_user_data_dirs(): + path = directory / "api-token" + if path.is_file(): + try: + token = path.read_text(encoding="utf-8").strip() + except OSError: + continue + if token: + return token + return "" + + +def _resolve_api_token() -> str: + if _CLI_TOKEN: + return _CLI_TOKEN + env = os.environ.get("MODLY_API_TOKEN", "").strip() + if env: + return env + return _read_token_file() + + +def _api_headers(extra: dict[str, str] | None = None) -> dict[str, str]: + headers = dict(extra or {}) + token = _resolve_api_token() + if token: + headers.setdefault("Authorization", f"Bearer {token}") + headers.setdefault("X-Modly-Token", token) + return headers + + +def _ensure_serve_token(env: dict[str, str]) -> str: + token = (env.get("MODLY_API_TOKEN") or os.environ.get("MODLY_API_TOKEN") or "").strip() + if not token: + token = secrets.token_hex(32) + env["MODLY_API_TOKEN"] = token + return token + + def _resolve_serve_config(args: argparse.Namespace) -> tuple[Path, Path, dict[str, str], list[str], str]: api_dir = Path(args.api_dir).expanduser().resolve() if getattr(args, "api_dir", None) else _default_api_dir() if not api_dir or not (api_dir / "main.py").exists(): @@ -333,6 +380,9 @@ def _resolve_serve_config(args: argparse.Namespace) -> tuple[Path, Path, dict[st "HUGGING_FACE_HUB_TOKEN": hf_token, "HF_TOKEN": hf_token, }) + _ensure_serve_token(env) + if getattr(args, "allow_remote", False) or args.host not in {"127.0.0.1", "localhost", "::1"}: + env["MODLY_API_ALLOW_REMOTE"] = "1" cmd = [str(python), "-m", "uvicorn", "main:app", "--host", args.host, "--port", str(args.port)] base_url = f"http://{args.host}:{args.port}" return api_dir, python, env, cmd, base_url @@ -1050,12 +1100,14 @@ def cmd_serve(args: argparse.Namespace) -> int: api_dir, _python, env, cmd, base_url = _resolve_serve_config(args) public_env = {k: env.get(k, "") for k in ["MODELS_DIR", "WORKSPACE_DIR", "EXTENSIONS_DIR", "SELECTED_MODEL_ID"]} meta = {"dev_only": True, "canonical": False} + payload = {"ok": True, "cmd": cmd, "cwd": str(api_dir), "base_url": base_url, "env": public_env, "token": env.get("MODLY_API_TOKEN", ""), "meta": meta} if args.print_command: - _json_print({"ok": True, "cmd": cmd, "cwd": str(api_dir), "base_url": base_url, "env": public_env, "meta": meta}, compact=args.compact) + _json_print(payload, compact=args.compact) return 0 proc = _start_backend(cmd, api_dir=api_dir, env=env, detach=args.detach) if args.detach: - _json_print({"ok": True, "started": True, "pid": proc.pid, "base_url": base_url, "cmd": cmd, "cwd": str(api_dir), "env": public_env, "meta": meta}, compact=args.compact) + payload.update({"started": True, "pid": proc.pid}) + _json_print(payload, compact=args.compact) return 0 return int(proc.wait()) @@ -1075,11 +1127,14 @@ def cmd_ensure_server(args: argparse.Namespace) -> int: return 0 api_dir, _python, env, cmd, resolved_url = _resolve_serve_config(args) public_env = {k: env.get(k, "") for k in ["MODELS_DIR", "WORKSPACE_DIR", "EXTENSIONS_DIR", "SELECTED_MODEL_ID"]} + payload = {"ok": True, "started": False, "base_url": resolved_url, "cmd": cmd, "cwd": str(api_dir), "env": public_env, "token": env.get("MODLY_API_TOKEN", ""), "meta": meta} if args.print_command: - _json_print({"ok": True, "started": False, "would_start": True, "base_url": resolved_url, "cmd": cmd, "cwd": str(api_dir), "env": public_env, "meta": meta}, compact=args.compact) + payload["would_start"] = True + _json_print(payload, compact=args.compact) return 0 proc = _start_backend(cmd, api_dir=api_dir, env=env, detach=args.detach) - _json_print({"ok": True, "started": True, "pid": proc.pid, "base_url": resolved_url, "cmd": cmd, "cwd": str(api_dir), "env": public_env, "meta": meta}, compact=args.compact) + payload.update({"started": True, "pid": proc.pid}) + _json_print(payload, compact=args.compact) if not args.detach: return int(proc.wait()) return 0 @@ -1176,6 +1231,7 @@ def _add_serve_options(parser: argparse.ArgumentParser, *, include_start: bool = parser.add_argument("--extensions-dir", help="Extensions directory for the backend") parser.add_argument("--model", help="Initial SELECTED_MODEL_ID") parser.add_argument("--hf-token", help="Hugging Face token for gated models") + parser.add_argument("--allow-remote", action="store_true", help="Allow non-loopback Host/Origin (use with --token)") parser.add_argument("--detach", action="store_true", help="Start in background and print pid") parser.add_argument("--print-command", action="store_true", help="Print resolved command/env without starting") @@ -1186,6 +1242,7 @@ def build_parser() -> argparse.ArgumentParser: description="Tiny stdlib-only CLI for agents calling a running Modly desktop API.", ) parser.add_argument("--base-url", default=DEFAULT_BASE_URL, help=f"Modly API URL (default: {DEFAULT_BASE_URL})") + parser.add_argument("--token", default=os.environ.get("MODLY_API_TOKEN", ""), help="API token (default: MODLY_API_TOKEN or Electron userData/api-token)") parser.add_argument("--request-timeout", type=float, default=30, help="Per-request timeout in seconds (default: 30)") parser.add_argument("--compact", action="store_true", help="Print compact one-line JSON") parser.add_argument("--quiet", action="store_true", help="Suppress progress output; final JSON is still printed") @@ -1332,6 +1389,8 @@ def main(argv: list[str] | None = None) -> int: args = None try: args = parser.parse_args(argv) + global _CLI_TOKEN + _CLI_TOKEN = str(getattr(args, "token", "") or "").strip() return int(args.func(args)) except ModlyCliError as exc: _json_print({"ok": False, "code": exc.code, "message": exc.message, "error": exc.message}, compact=getattr(args, "compact", False) if args else False) diff --git a/tools/modly-cli/test_agent.py b/tools/modly-cli/test_agent.py index 9aaaa560..0eab9494 100644 --- a/tools/modly-cli/test_agent.py +++ b/tools/modly-cli/test_agent.py @@ -537,5 +537,30 @@ def test_hidden_aliases_are_not_documented_as_canonical(self) -> None: self.assertNotIn("\n batch", help_text) +class ApiAuthTests(unittest.TestCase): + def tearDown(self) -> None: + agent._CLI_TOKEN = "" + + def test_api_headers_include_cli_token(self) -> None: + agent._CLI_TOKEN = "abc123" + headers = agent._api_headers({"Content-Type": "application/json"}) + self.assertEqual(headers["Authorization"], "Bearer abc123") + self.assertEqual(headers["X-Modly-Token"], "abc123") + self.assertEqual(headers["Content-Type"], "application/json") + + def test_reads_token_file_from_user_data(self) -> None: + with tempfile.TemporaryDirectory() as td: + root = Path(td) + token_path = root / "Modly" / "api-token" + token_path.parent.mkdir(parents=True) + token_path.write_text("from-file\n", encoding="utf-8") + with patch.object(agent, "_modly_user_data_dirs", return_value=[token_path.parent]), patch.dict(os.environ, {"MODLY_API_TOKEN": ""}, clear=False): + self.assertEqual(agent._resolve_api_token(), "from-file") + + def test_parser_accepts_token_flag(self) -> None: + args = agent.build_parser().parse_args(["--token", "s3cret", "health"]) + self.assertEqual(args.token, "s3cret") + + if __name__ == "__main__": unittest.main()