From 514ec6cdcca6361cd17de018383fb6b37720cd80 Mon Sep 17 00:00:00 2001 From: MODELBEAST Date: Mon, 13 Jul 2026 12:25:13 +1000 Subject: [PATCH] Security hardening from adversarial review (5-agent workflow) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixed confirmed findings before public exposure: HIGH: - upload filename path traversal → store.safe_name() strips to basename - login rate-limit XFF bypass → key on request.client.host + per-username bucket; auth.check_login() burns bcrypt time on unknown users (no enumeration) - cross-user read access → per-user isolation: guests see/use/download/delete only their own assets & jobs (owner sees all); WS job events scoped per-user MEDIUM: - unbounded upload read → bounded chunked streaming to the 1GB cap - asset member path check → Path.is_relative_to boundary + ownership gate - WS token-in-query-string leak → session-cookie-only WS auth LOW: - retry_job bypassed the per-user job cap → cap now checked on retry - wholesale API-key injection → env_for_operator injects a paid key only to operators that declare it (guest local jobs never receive fal/OpenRouter keys) - session revocation → users.session_epoch, bumped on password change - int() 500s → 400; net-lane defense-in-depth (guests blocked by requires_env AND resources==net, so a mis-tagged paid op is still blocked) + public /api/health for serve.sh & proxy; docs/VPS.md; mb MB_TOKEN bearer auth tests/smoke.sh: 34 checks passing incl. all new hardening. Co-Authored-By: Claude Opus 4.8 --- docs/VPS.md | 93 ++++++++++++++++++++++++++++++++++++++++ mb | 18 ++++++-- scripts/serve.sh | 6 ++- server/auth.py | 32 ++++++++++++-- server/db.py | 1 + server/main.py | 103 ++++++++++++++++++++++++++++++--------------- server/runner.py | 16 +++++-- server/settings.py | 20 +++++++++ server/store.py | 10 ++++- server/sysinfo.py | 14 ++++-- tests/smoke.sh | 26 ++++++++++-- 11 files changed, 285 insertions(+), 54 deletions(-) create mode 100644 docs/VPS.md diff --git a/docs/VPS.md b/docs/VPS.md new file mode 100644 index 0000000..8eb4afd --- /dev/null +++ b/docs/VPS.md @@ -0,0 +1,93 @@ +# Public hosting — MODELBEAST at modelbeast.digalot.fyi + +Expose MODELBEAST to the internet **without exposing the M3 Ultra directly**: a spare +VPS is the only public surface and reverse-proxies to the Mac over the tailnet. If the +VPS is ever compromised you just remove one tailnet node; the Mac is never on the +public internet. + +``` + friend's browser ──https──▶ VPS (Caddy, public) + │ reverse_proxy over tailnet + ▼ + M3 Ultra 100.89.131.57:8777 (auth enforced here) +``` + +**Prerequisite: auth must be live on the Mac first** (HANDOFF2 Phase 1 — done). Do not +open this to the internet until `mb`/the UI require a login. Every public request now +hits the login gate; guests are local-only by hard server-side rule. + +## On the VPS (run these — you own that box; I can't reach it) + +```bash +# 1) join the same tailnet as the M3 Ultra +curl -fsSL https://tailscale.com/install.sh | sh +sudo tailscale up + +# 2) Caddy (automatic HTTPS via Let's Encrypt) +sudo apt update && sudo apt install -y caddy +``` + +Write `/etc/caddy/Caddyfile`: +``` +modelbeast.digalot.fyi { + encode zstd gzip + request_body { + max_size 1GB # matches the app's upload cap + } + reverse_proxy 100.89.131.57:8777 +} + +# optional second domain, same backend: +# modelbeast.partyl.party { +# encode zstd gzip +# request_body { max_size 1GB } +# reverse_proxy 100.89.131.57:8777 +# } +``` + +```bash +sudo systemctl reload caddy +``` + +**DNS:** add an `A` record `modelbeast` → the VPS's public IP on `digalot.fyi` (and +`partyl.party` if using it). Caddy fetches the TLS cert automatically on first request. + +## Why these choices + +- **Subdomain, not `digalot.fyi/modelbeast`.** A path prefix would force a Vite + `base` + FastAPI `root_path` rework across the SPA, the WebSocket, and the `mb` CLI + for zero benefit. `modelbeast.` needs no app changes. +- **Caddy proxies WebSockets automatically** — the live job-log stream works through + it. The session cookie is same-origin (browser talks to the subdomain for both HTTP + and WS), so WS auth via cookie just works. +- **Auth lives in the app, not the proxy.** Proxy basic-auth can't do per-user roles, + the guest local-only rule, or per-user job caps. All of that is enforced in FastAPI. +- **Rejected: Tailscale Funnel.** Zero-VPS and simplest, but no custom domain and it's + bandwidth-capped. Keep it in your back pocket as the fallback if the VPS dies: + `tailscale funnel 8777` publishes `https://..ts.net` instantly. + +## Hardening already in the app (verify after going live) + +- `Secure` cookie flag is set when the request arrived via https (reads + `X-Forwarded-Proto` that Caddy sets) — check DevTools shows `mb_session` as Secure. +- Upload cap 1 GB in the app **and** in Caddy (`request_body max_size`). +- Login rate-limit (5/min) keyed on `X-Forwarded-For`. +- The Mac keeps binding `0.0.0.0` on the tailnet; auth now protects it there too + (agents/CLI use `MB_TOKEN`). + +## Off-tailnet acceptance test (do this WITH me once Caddy is up) + +From a phone on cellular (not on the tailnet): +1. `https://modelbeast.digalot.fyi` shows the login page over a valid cert. +2. A guest logs in, runs `flux_local`, watches the live log stream, opens the dashboard. +3. `MB_HOST=https://modelbeast.digalot.fyi MB_TOKEN=... ./mb ops` works. +4. Unauthenticated `curl https://modelbeast.digalot.fyi/api/operators` → 401. +5. A guest gets 403 on `/api/settings` and on a direct cloud-operator job POST. + +## Runbook +- Server stays a single uvicorn worker (the in-process job queue + shared SQLite + connection require exactly one worker — do not add `--workers`). +- Restart the Mac server: `./scripts/serve.sh`. Caddy needs no restart for that. +- Rotate a leaked session-signing key: delete the `auth_secret` row from the settings + table (`sqlite3 data/modelbeast.db "DELETE FROM settings WHERE key='auth_secret'"`) + and restart — a new one is generated and all existing sessions are invalidated. diff --git a/mb b/mb index 9c2f66b..6a4e2a1 100755 --- a/mb +++ b/mb @@ -35,10 +35,17 @@ import uuid from pathlib import Path HOST = os.environ.get("MB_HOST", "http://localhost:8777").rstrip("/") +TOKEN = os.environ.get("MB_TOKEN", "") + + +def _auth(req): + if TOKEN: + req.add_header("Authorization", f"Bearer {TOKEN}") + return req def http(method, path, body=None): - req = urllib.request.Request(HOST + path, method=method) + req = _auth(urllib.request.Request(HOST + path, method=method)) data = None if body is not None: data = json.dumps(body).encode() @@ -49,6 +56,9 @@ def http(method, path, body=None): raw = r.read() return json.loads(raw) if "json" in ct else raw except urllib.error.HTTPError as e: + if e.code == 401: + sys.exit("401 unauthorized — set MB_TOKEN (owner: `mb token create ` in the UI, " + "or scripts/users.py). Get one from ⚙ Settings → Users → token.") sys.exit(f"HTTP {e.code} {method} {path}: {e.read().decode()[:500]}") except urllib.error.URLError as e: sys.exit(f"cannot reach {HOST} ({e.reason}) — is the server up? set MB_HOST?") @@ -63,7 +73,7 @@ def upload_file(path): body = (f"--{boundary}\r\nContent-Disposition: form-data; name=\"file\"; " f"filename=\"{p.name}\"\r\nContent-Type: {ctype}\r\n\r\n").encode() body += p.read_bytes() + f"\r\n--{boundary}--\r\n".encode() - req = urllib.request.Request(HOST + "/api/assets", data=body, method="POST") + req = _auth(urllib.request.Request(HOST + "/api/assets", data=body, method="POST")) req.add_header("Content-Type", f"multipart/form-data; boundary={boundary}") try: with urllib.request.urlopen(req, timeout=600) as r: @@ -98,7 +108,7 @@ def download_asset(asset, dest_dir): dest_dir = Path(dest_dir) dest_dir.mkdir(parents=True, exist_ok=True) url = f"{HOST}/api/assets/{asset['id']}/file" - with urllib.request.urlopen(url, timeout=600) as r: + with urllib.request.urlopen(_auth(urllib.request.Request(url)), timeout=600) as r: ct = r.headers.get("content-type", "") data = r.read() if "json" in ct: @@ -108,7 +118,7 @@ def download_asset(asset, dest_dir): sub.mkdir(exist_ok=True) for member in listing["files"]: murl = url + "?member=" + urllib.request.quote(member) - with urllib.request.urlopen(murl, timeout=600) as r: + with urllib.request.urlopen(_auth(urllib.request.Request(murl)), timeout=600) as r: (sub / member).write_bytes(r.read()) print(f" ↓ {sub} ({len(listing['files'])} files)") return diff --git a/scripts/serve.sh b/scripts/serve.sh index 2205274..ca27176 100755 --- a/scripts/serve.sh +++ b/scripts/serve.sh @@ -1,5 +1,9 @@ #!/usr/bin/env bash # Start (or restart) the MODELBEAST server headless on port 8777. +# NOTE: exactly ONE uvicorn worker (no --workers). The in-process asyncio job +# queue + the single shared SQLite connection require a single worker; adding +# workers would fork the queue and corrupt job state. Do not "optimize" this. +# On first run the owner account + one-time password print to data/server.log. set -euo pipefail cd "$(dirname "$0")/.." lsof -ti:8777 | xargs kill 2>/dev/null || true @@ -7,6 +11,6 @@ sleep 1 nohup /opt/homebrew/bin/uv run uvicorn server.main:app --host 0.0.0.0 --port 8777 \ > data/server.log 2>&1 & sleep 2 -curl -sf -o /dev/null http://localhost:8777/api/operators \ +curl -sf -o /dev/null http://localhost:8777/api/health \ && echo "MODELBEAST up: http://$( /Applications/Tailscale.app/Contents/MacOS/Tailscale ip -4 2>/dev/null || echo localhost):8777 (log: data/server.log)" \ || { echo "FAILED to start — tail data/server.log:"; tail -20 data/server.log; exit 1; } diff --git a/server/auth.py b/server/auth.py index 7fffc40..31cb839 100644 --- a/server/auth.py +++ b/server/auth.py @@ -40,6 +40,21 @@ def verify_pw(pw: str, pw_hash: str) -> bool: return False +# a real hash to compare against when the user doesn't exist, so login timing is +# the same whether or not the username is valid (defeats username enumeration) +_DUMMY_HASH = bcrypt.hashpw(b"modelbeast-dummy", bcrypt.gensalt()).decode() + + +def check_login(con, username: str, password: str) -> dict | None: + """Constant-ish-time credential check. Returns the user row or None.""" + u = get_user_by_name(con, username) + if u and verify_pw(password, u["pw_hash"]): + return u + if not u: + verify_pw(password, _DUMMY_HASH) # burn equivalent time + return None + + # -- auth secret (stored hidden in settings; never exposed via the API) ------- def ensure_auth_secret(con) -> str: row = con.execute("SELECT value FROM settings WHERE key='auth_secret'").fetchone() @@ -56,14 +71,23 @@ def _serializer(con) -> URLSafeTimedSerializer: def make_session(con, user_id: str) -> str: - return _serializer(con).dumps({"uid": user_id}) + u = get_user(con, user_id) + epoch = u.get("session_epoch", 0) if u else 0 + return _serializer(con).dumps({"uid": user_id, "e": epoch}) def read_session(con, token: str) -> str | None: + """Return the user id iff the signature is valid, unexpired, and the embedded + session epoch still matches (password change / logout-all bumps the epoch).""" try: - return _serializer(con).loads(token, max_age=SESSION_MAX_AGE).get("uid") + payload = _serializer(con).loads(token, max_age=SESSION_MAX_AGE) except (BadSignature, SignatureExpired, Exception): return None + uid = payload.get("uid") + u = get_user(con, uid) if uid else None + if not u or payload.get("e", 0) != u.get("session_epoch", 0): + return None + return uid # -- API tokens ---------------------------------------------------------------- @@ -121,7 +145,9 @@ def create_user(con, username: str, password: str, role: str = "guest", def set_password(con, user_id: str, password: str) -> None: if len(password) < MIN_PW_LEN: raise ValueError(f"password must be >= {MIN_PW_LEN} characters") - con.execute("UPDATE users SET pw_hash = ? WHERE id = ?", (hash_pw(password), user_id)) + # bump session_epoch so every existing session/cookie for this user is revoked + con.execute("UPDATE users SET pw_hash = ?, session_epoch = session_epoch + 1 WHERE id = ?", + (hash_pw(password), user_id)) con.commit() diff --git a/server/db.py b/server/db.py index 9566677..bd3c7fc 100644 --- a/server/db.py +++ b/server/db.py @@ -61,6 +61,7 @@ MIGRATIONS = [ ("jobs", "asset_ids", "TEXT NOT NULL DEFAULT '[]'"), ("jobs", "user_id", "TEXT"), # nullable; legacy rows = owner-era ("assets", "user_id", "TEXT"), + ("users", "session_epoch", "INTEGER NOT NULL DEFAULT 0"), # bump to revoke sessions ] diff --git a/server/main.py b/server/main.py index 85257dc..8d7da86 100644 --- a/server/main.py +++ b/server/main.py @@ -53,19 +53,27 @@ def _with_usernames(con, rows: list[dict]) -> list[dict]: return rows +# -- health (public, unauthenticated — for serve.sh + proxy monitoring) -------- +@app.get("/api/health") +def health(): + return {"ok": True, "operators": len(runner.operators)} + + # -- auth / session ------------------------------------------------------------ @app.post("/api/login") async def login(payload: dict, request: Request, response: Response): - ip = request.headers.get("x-forwarded-for", "").split(",")[0].strip() \ - or (request.client.host if request.client else "?") - if not auth.rate_ok(ip): + # Key the limiter on the DIRECT peer (Caddy) — not the client-spoofable + # X-Forwarded-For — plus a per-username bucket so the owner account can't be + # brute-forced regardless of a rotated XFF header. + ip = request.client.host if request.client else "?" + username = (payload.get("username") or "").strip().lower() + if not auth.rate_ok(f"ip:{ip}") or not auth.rate_ok(f"user:{username}"): raise HTTPException(429, "too many attempts — wait a minute") con = _con(request) - username = (payload.get("username") or "").strip().lower() password = payload.get("password") or "" - u = auth.get_user_by_name(con, username) - if not u or not auth.verify_pw(password, u["pw_hash"]): - auth.note_fail(ip) + u = auth.check_login(con, username, password) # constant-time, no user enumeration + if not u: + auth.note_fail(f"ip:{ip}"); auth.note_fail(f"user:{username}") raise HTTPException(401, "invalid username or password") token = auth.make_session(con, u["id"]) secure = request.headers.get("x-forwarded-proto", "http") == "https" @@ -94,10 +102,11 @@ def get_users(request: Request, owner=Depends(auth.require_owner)): @app.post("/api/users") def add_user(payload: dict, request: Request, owner=Depends(auth.require_owner)): try: + maxj = int(payload.get("max_active_jobs", 4)) u = auth.create_user(_con(request), payload.get("username", ""), payload.get("password", ""), role=payload.get("role", "guest"), - max_active_jobs=int(payload.get("max_active_jobs", 4))) - except ValueError as e: + max_active_jobs=maxj) + except (ValueError, TypeError) as e: raise HTTPException(400, str(e)) return auth.public_user(u) @@ -114,8 +123,11 @@ def patch_user(username: str, payload: dict, request: Request, owner=Depends(aut except ValueError as e: raise HTTPException(400, str(e)) if "max_active_jobs" in payload: - con.execute("UPDATE users SET max_active_jobs = ? WHERE id = ?", - (int(payload["max_active_jobs"]), u["id"])) + try: + maxj = int(payload["max_active_jobs"]) + except (ValueError, TypeError): + raise HTTPException(400, "max_active_jobs must be an integer") + con.execute("UPDATE users SET max_active_jobs = ? WHERE id = ?", (maxj, u["id"])) con.commit() return auth.public_user(auth.get_user(con, u["id"])) @@ -177,23 +189,43 @@ def put_settings(payload: dict, request: Request, owner=Depends(auth.require_own # -- assets -------------------------------------------------------------------- +def _can_see(user, obj) -> bool: + """owner sees everything; a guest sees only their own jobs/assets.""" + return user["role"] == "owner" or obj.get("user_id") == user["id"] + + +def _guest_may_run(user, op) -> bool: + """HARD RULE: guests are local-only. Blocked by TWO signals (defense in depth): + any declared requires_env (paid keys) AND the net/cloud lane. So a paid operator + that forgot to declare requires_env is still blocked.""" + if user["role"] == "owner": + return True + return not op.get("requires_env") and op.get("resources") != "net" + + @app.get("/api/assets") def list_assets(user=Depends(auth.current_user), request: Request = None): con = _con(request) - return _with_usernames(con, store.list_assets(con)) + rows = [a for a in store.list_assets(con) if _can_see(user, a)] + return _with_usernames(con, rows) @app.post("/api/assets") async def upload_asset(request: Request, file: UploadFile = File(...), user=Depends(auth.current_user)): - cl = request.headers.get("content-length") - if cl and int(cl) > MAX_UPLOAD: - raise HTTPException(413, "file too large (1 GB max)") - data = await file.read() + # stream in bounded chunks so a missing/spoofed Content-Length can't blow memory + chunks, total = [], 0 + while True: + chunk = await file.read(1024 * 1024) + if not chunk: + break + total += len(chunk) + if total > MAX_UPLOAD: + raise HTTPException(413, "file too large (1 GB max)") + chunks.append(chunk) + data = b"".join(chunks) if not data: raise HTTPException(400, "empty file") - if len(data) > MAX_UPLOAD: - raise HTTPException(413, "file too large (1 GB max)") return store.register_upload(_con(request), file.filename or "unnamed", data, user_id=user["id"]) @@ -203,7 +235,7 @@ def delete_asset(asset_id: str, user=Depends(auth.current_user), request: Reques asset = store.get_asset(con, asset_id) if not asset: raise HTTPException(404, "no such asset") - if user["role"] != "owner" and asset.get("user_id") != user["id"]: + if not _can_see(user, asset): raise HTTPException(403, "not your asset") store.delete_asset(con, asset_id) return {"ok": True} @@ -215,10 +247,12 @@ def asset_file(asset_id: str, member: str | None = None, asset = store.get_asset(_con(request), asset_id) if not asset: raise HTTPException(404, "no such asset") + if not _can_see(user, asset): + raise HTTPException(403, "not your asset") path = Path(asset["path"]) if member: target = (path / member).resolve() - if not str(target).startswith(str(path.resolve())) or not target.is_file(): + if not target.is_relative_to(path.resolve()) or not target.is_file(): raise HTTPException(404, "no such member") return FileResponse(target) if path.is_dir(): @@ -231,13 +265,15 @@ def asset_file(asset_id: str, member: str | None = None, @app.get("/api/jobs") def list_jobs(user=Depends(auth.current_user), request: Request = None): con = _con(request) - return _with_usernames(con, runner.list_jobs(con)) + rows = [j for j in runner.list_jobs(con) if _can_see(user, j)] + return _with_usernames(con, rows) @app.get("/api/jobs/recent") def recent_jobs(user=Depends(auth.current_user), request: Request = None): con = _con(request) - return sysinfo.recent_jobs(con, runner) + uid = None if user["role"] == "owner" else user["id"] + return sysinfo.recent_jobs(con, runner, user_id=uid) @app.post("/api/jobs") @@ -247,8 +283,7 @@ async def create_job(payload: dict, user=Depends(auth.current_user), request: Re if operator not in runner.operators: raise HTTPException(400, f"unknown operator: {operator}") op = runner.operators[operator] - # HARD RULE: guests run local models only — enforced here regardless of UI - if op.get("requires_env") and user["role"] != "owner": + if not _guest_may_run(user, op): raise HTTPException(403, "guests run local models only") if user["role"] != "owner" and runner.active_job_count(con, user["id"]) >= user["max_active_jobs"]: raise HTTPException(429, "too many active jobs — wait for some to finish") @@ -256,8 +291,11 @@ async def create_job(payload: dict, user=Depends(auth.current_user), request: Re if not asset_ids: asset_ids = [payload["asset_id"]] if payload.get("asset_id") else [] for aid in asset_ids: - if not store.get_asset(con, aid): + a = store.get_asset(con, aid) + if not a: raise HTTPException(400, f"unknown asset: {aid}") + if not _can_see(user, a): + raise HTTPException(403, "not your asset") params = payload.get("params") or {} job = runner.create_job(con, operator, asset_ids, params, user_id=user["id"]) await runner.broadcast({"type": "job", "job": job}) @@ -295,8 +333,10 @@ async def retry_job(job_id: str, user=Depends(auth.current_user), request: Reque con = _con(request) job = _own_job_or_owner(con, job_id, user) op = runner.operators.get(job["operator"], {}) - if op.get("requires_env") and user["role"] != "owner": + if not _guest_may_run(user, op): raise HTTPException(403, "guests run local models only") + if user["role"] != "owner" and runner.active_job_count(con, user["id"]) >= user["max_active_jobs"]: + raise HTTPException(429, "too many active jobs — wait for some to finish") new = runner.create_job(con, job["operator"], job.get("asset_ids") or [], job["params"], user_id=user["id"]) await runner.broadcast({"type": "job", "job": new}) @@ -341,7 +381,8 @@ async def watch_inbox(): print(f"[inbox] {e}") -# -- websocket (auth: session cookie or ?token=) ------------------------------- +# -- websocket (auth: session cookie only — never a token in the query string, +# which would leak into proxy/access logs) ---------------------------------- @app.websocket("/ws") async def ws(websocket: WebSocket): con = websocket.app.state.con @@ -351,20 +392,16 @@ async def ws(websocket: WebSocket): uid = auth.read_session(con, tok) if uid: user = auth.get_user(con, uid) - if not user: - qtok = websocket.query_params.get("token") - if qtok: - user = auth.user_by_token(con, qtok) if not user: await websocket.close(code=1008) # policy violation return await websocket.accept() - runner.subscribers.add(websocket) + runner.subscribers[websocket] = user try: while True: await websocket.receive_text() except WebSocketDisconnect: - runner.subscribers.discard(websocket) + runner.subscribers.pop(websocket, None) # -- frontend (built) — public so the SPA can load and show the login screen ---- diff --git a/server/runner.py b/server/runner.py index 62f00ab..c2270b8 100644 --- a/server/runner.py +++ b/server/runner.py @@ -27,7 +27,7 @@ LANE_LIMITS = {"gpu": 1, "cpu": 3, "net": 6} class Runner: def __init__(self): self.queue: asyncio.Queue[str] = asyncio.Queue() - self.subscribers: set = set() + self.subscribers: dict = {} # websocket -> user dict (for per-user scoping) self.operators: dict[str, dict] = {} self.lanes: dict[str, asyncio.Semaphore] = {} self.procs: dict = {} # job_id -> subprocess @@ -37,14 +37,20 @@ class Runner: # -- pubsub ------------------------------------------------------------- async def broadcast(self, message: dict): + # "job" events carry a payload injected straight into the client's list, so + # scope them: a guest only receives their own jobs (owner receives all). + # "assets_changed" just triggers a per-user re-fetch, so send to everyone. + job_uid = message.get("job", {}).get("user_id") if message.get("type") == "job" else None dead = [] - for ws in self.subscribers: + for ws, u in list(self.subscribers.items()): + if job_uid is not None and u.get("role") != "owner" and job_uid != u.get("id"): + continue try: await ws.send_json(message) except Exception: dead.append(ws) for ws in dead: - self.subscribers.discard(ws) + self.subscribers.pop(ws, None) def lane_of(self, operator: str) -> str: op = self.operators.get(operator, {}) @@ -172,7 +178,9 @@ class Runner: cur_settings = self.get_settings() env = os.environ.copy() - env.update(settings_mod.env_from_settings(cur_settings)) + # least privilege: only this operator's declared paid keys (+ HF/infra) reach + # the subprocess, so guest local jobs never get the owner's fal/OpenRouter keys + env.update(settings_mod.env_for_operator(cur_settings, op.get("requires_env"))) secrets = settings_mod.secret_values(cur_settings) missing = [e for e in op.get("requires_env", []) if not env.get(e)] diff --git a/server/settings.py b/server/settings.py index ea9a714..83e3849 100644 --- a/server/settings.py +++ b/server/settings.py @@ -71,6 +71,26 @@ def env_from_settings(settings: dict) -> dict: return env +def env_for_operator(settings: dict, requires_env) -> dict: + """Least-privilege env for an operator subprocess: non-secret infra vars + the + HF token (local ML ops need it for gated weights) ALWAYS; paid/secret API keys + ONLY if the operator declares them in requires_env. So a guest's local job never + receives the owner's fal/OpenRouter keys even in its environment.""" + requires_env = set(requires_env or []) + env = {} + for key, env_name in ENV_MAP.items(): + value = settings.get(key) + if not value: + continue + # paid secret keys are gated by requires_env; hf_token + non-secret infra always + if key in SECRET_KEYS and key != "hf_token" and env_name not in requires_env: + continue + env[env_name] = value + if settings.get("hf_token"): + env["HUGGING_FACE_HUB_TOKEN"] = settings["hf_token"] + return env + + def secret_values(settings: dict) -> list[str]: return [settings[k] for k in SECRET_KEYS if settings.get(k)] diff --git a/server/store.py b/server/store.py index be5f74a..8af91e6 100644 --- a/server/store.py +++ b/server/store.py @@ -23,10 +23,17 @@ def kind_of(name: str) -> str: return "other" +def safe_name(name: str) -> str: + """Strip any directory components from a user-supplied filename (path-traversal + guard) so the stored file can only ever land inside its own asset dir.""" + base = Path(name or "").name + return base if base and base not in (".", "..") else "unnamed" + + def register_file(con, src: Path, name: str | None = None, parent_job: str | None = None, move: bool = False, meta: dict | None = None, user_id: str | None = None) -> dict: """Copy/move a file (or directory) into the asset store and record it.""" - name = name or src.name + name = safe_name(name or src.name) asset_id = db.new_id() dest_dir = ASSETS_DIR / asset_id dest_dir.mkdir(parents=True, exist_ok=True) @@ -59,6 +66,7 @@ def register_file(con, src: Path, name: str | None = None, parent_job: str | Non def register_upload(con, name: str, data: bytes, user_id: str | None = None) -> dict: + name = safe_name(name) asset_id = db.new_id() dest_dir = ASSETS_DIR / asset_id dest_dir.mkdir(parents=True, exist_ok=True) diff --git a/server/sysinfo.py b/server/sysinfo.py index 27537ad..989cebe 100644 --- a/server/sysinfo.py +++ b/server/sysinfo.py @@ -106,11 +106,17 @@ def snapshot(con, runner) -> dict: } -def recent_jobs(con, runner, limit: int = 20) -> list[dict]: +def recent_jobs(con, runner, limit: int = 20, user_id: str | None = None) -> list[dict]: cutoff = time.time() - 86400 - rows = con.execute( - "SELECT * FROM jobs WHERE status IN ('done','error','cancelled') " - "AND finished_at >= ? ORDER BY finished_at DESC LIMIT ?", (cutoff, limit)).fetchall() + if user_id is None: # owner: all users + rows = con.execute( + "SELECT * FROM jobs WHERE status IN ('done','error','cancelled') " + "AND finished_at >= ? ORDER BY finished_at DESC LIMIT ?", (cutoff, limit)).fetchall() + else: # guest: only their own + rows = con.execute( + "SELECT * FROM jobs WHERE status IN ('done','error','cancelled') " + "AND finished_at >= ? AND user_id = ? ORDER BY finished_at DESC LIMIT ?", + (cutoff, user_id, limit)).fetchall() umap = {u_id: name for u_id, name in con.execute("SELECT id, username FROM users").fetchall()} out = [] diff --git a/tests/smoke.sh b/tests/smoke.sh index 538d190..6e89554 100755 --- a/tests/smoke.sh +++ b/tests/smoke.sh @@ -91,12 +91,18 @@ c=$(code -b "$GJAR" -X POST "$B/api/jobs" -H 'Content-Type: application/json' -d c=$(code -b "$GJAR" "$B/api/settings"); [ "$c" = 403 ] && ok "guest settings → 403" || bad "guest settings ($c)" c=$(code -b "$GJAR" -X POST "$B/api/users" -H 'Content-Type: application/json' -d '{"username":"x","password":"yyyyyy"}') [ "$c" = 403 ] && ok "guest cannot create users → 403" || bad "guest user-create ($c)" -# guest CAN run a local operator +# guest uploads their OWN asset and runs a local op on it → 200 +GVID=$(curl -s -b "$GJAR" -F "file=@$TMP/clip.mp4" "$B/api/assets" | j "d['id']") +c=$(code -b "$GJAR" -X POST "$B/api/jobs" -H 'Content-Type: application/json' -d '{"operator":"ffprobe","asset_id":"'"$GVID"'"}') +[ "$c" = 200 ] && ok "guest runs local op on OWN asset" || bad "guest own-asset run ($c)" +# per-user isolation: guest CANNOT use / see / delete / download the owner's asset FFPAY='{"operator":"ffprobe","asset_id":"'"$VID"'"}' c=$(code -b "$GJAR" -X POST "$B/api/jobs" -H 'Content-Type: application/json' -d "$FFPAY") -[ "$c" = 200 ] && ok "guest can run local ffprobe" || bad "guest local blocked ($c)" -# guest cannot delete owner's asset +[ "$c" = 403 ] && ok "guest cannot use owner asset → 403" || bad "guest used owner asset ($c)" c=$(code -b "$GJAR" -X DELETE "$B/api/assets/$GLB"); [ "$c" = 403 ] && ok "guest cannot delete owner asset → 403" || bad "guest deleted owner asset ($c)" +c=$(code -b "$GJAR" "$B/api/assets/$GLB/file"); [ "$c" = 403 ] && ok "guest cannot download owner asset → 403" || bad "guest downloaded owner asset ($c)" +GSEE=$(curl -s -b "$GJAR" "$B/api/assets" | j "any(a['id']=='$GLB' for a in d)") +[ "$GSEE" = False ] && ok "guest asset list excludes owner's" || bad "guest sees owner assets" # per-user cap: capped user with max_active_jobs=0 → any job 429 curl -s -b "$OJAR" -X POST "$B/api/users" -H 'Content-Type: application/json' -d '{"username":"capped","password":"cappedpw"}' >/dev/null curl -s -b "$OJAR" -X PATCH "$B/api/users/capped" -H 'Content-Type: application/json' -d '{"max_active_jobs":0}' >/dev/null @@ -104,10 +110,22 @@ curl -s -c "$CJAR" -X POST "$B/api/login" -H 'Content-Type: application/json' -d c=$(code -b "$CJAR" -X POST "$B/api/jobs" -H 'Content-Type: application/json' -d "$FFPAY") [ "$c" = 429 ] && ok "per-user cap → 429" || bad "cap not enforced ($c)" +say "hardening (from security review)" +# upload filename path-traversal is sanitized to a basename +TRAV=$(curl -s -b "$OJAR" -F "file=@$TMP/clip.mp4;filename=../../../evil.mp4" "$B/api/assets" | j "d['name']") +[ "$TRAV" = "evil.mp4" ] && ok "upload filename sanitized to basename" || bad "filename traversal ($TRAV)" +# password change revokes the existing session (session_epoch bump) +curl -s -b "$OJAR" -X PATCH "$B/api/users/testmate" -H 'Content-Type: application/json' -d '{"password":"newpass123"}' >/dev/null +c=$(code -b "$GJAR" "$B/api/me"); [ "$c" = 401 ] && ok "password change revokes old session" || bad "stale session still valid ($c)" +# health endpoint is public (for serve.sh / proxy) +c=$(code "$B/api/health"); [ "$c" = 200 ] && ok "/api/health public" || bad "health ($c)" + say "dashboard" SHAPE=$(curl -s -b "$OJAR" "$B/api/system" | j "'gpu' in d and 'lanes' in d") [ "$SHAPE" = True ] && ok "/api/system shape" || bad "system shape" -c=$(code -b "$GJAR" "$B/api/system"); [ "$c" = 200 ] && ok "guest can view dashboard" || bad "guest dashboard ($c)" +CJAR2="$TMP/mate2.cj" +curl -s -c "$CJAR2" -X POST "$B/api/login" -H 'Content-Type: application/json' -d '{"username":"testmate","password":"newpass123"}' >/dev/null +c=$(code -b "$CJAR2" "$B/api/system"); [ "$c" = 200 ] && ok "guest can view dashboard" || bad "guest dashboard ($c)" printf "\n\033[1mResult: %d passed, %d failed\033[0m\n" "$pass" "$fail" [ "$fail" -eq 0 ]