2 Commits

Author	SHA1	Message	Date
filip131311	8efd7c6346	feat: add argent server start for long-lived tool-server (#182) ... ## Human summary: This adds `argent server start` command to enable us to spawn tools server in arbitrary location. we want this to enable using remote tools-server via our mcp not only local ones. ## Summary - Adds `argent server start` to spawn a tool-server that does **not** auto-shutdown on idle, with configurable bind port and host — the missing piece for running tool-server on a remote/dedicated host. - Foreground by default so process supervisors (systemd, Docker, supervisord) own the lifecycle; `--detach` keeps the existing daemon-spawn behavior. Refuses to clobber a healthy running server unless `--force`. - Phase 2 (consumer-side `--tools-url` flag pointing the MCP / CLI at a remote tool-server) is now a one-flag change away — `tools-client.ts` and `mcp-server.ts` already honor `ARGENT_TOOLS_URL`. Out of scope for this PR. ## What changed - **`tool-server`** — read `HOST` env var (default `127.0.0.1` so the existing auto-spawn path is unaffected), bind to it, attach an `error` listener on `app.listen` so `EADDRINUSE` / `EACCES` exit cleanly instead of routing through `uncaughtException`. - **`argent-tools-client`** — broaden the readiness regex to match any host; extend `buildToolsServerEnv` with optional `{ host, idleTimeoutMinutes }`; record `host` in `~/.argent/tool-server.json` (backward-compat: missing field defaults to `127.0.0.1`); export `spawnToolsServer`, `findFreePort`, `isToolsServerHealthy`, `isToolsServerProcessAlive`, the state read/write/clear helpers, and `formatToolsServerUrl` so the CLI can reuse them. - **`argent-cli`** — new `server start [flags]`: `--port`/`-p` (default 3001, `0` = pick free), `--host` (default `127.0.0.1`), `--idle-timeout` (default `0` = never), `--detach`/`-d`, `--force`, `--help`. Warns when host is non-loopback. Foreground mode forwards `SIGINT`/`SIGTERM` to the child and writes the state file so local `argent run` / `argent mcp` can attach. - **`argent` dispatcher** — pass `BUNDLED_RUNTIME_PATHS` to `server()` so the new subcommand can find the bundled tool-server. ## Test plan - [x] `argent server --help` lists the new `start` subcommand - [x] `argent server start --help` shows full flag reference - [x] `argent server start --detach --port 0` → status alive/healthy, `/tools` reachable, state file populated with `host` field - [x] Second `argent server start --detach` refuses (`exit 1`) with helpful message - [x] `argent server start --detach --force` kills the old PID and starts a new one - [x] Foreground `argent server start --port 0` runs with inherited stdio; child PID is the actual tool-server (not the wrapper); `argent server stop` from a sibling shell tears down both - [x] Existing auto-spawn path (`argent tools`) still works — verifies the regex broadening + default-host change is backward-compatible - [x] `tool-server` (444 tests) + `argent-tools-client` (1) + `argent` dispatcher (1) suites pass - [ ] Manual smoke on `--host 0.0.0.0` — could not exercise locally (sandbox declined the public-interface bind); the warning + bind path is identical to loopback besides the listen address --------- Co-authored-by: pFornagiel <pawel.fornagiel2@gmail.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Paweł Fornagiel <85541429+pFornagiel@users.noreply.github.com>	2026-06-03 17:44:15 +02:00
Ignacy Łątka	74c2065ca0	fix: multiple minor bugs (#194) ... ## Human Summary This PR aims to fix bugs that are benign for now, could become serious in the future: - multiple CSRF via CORS misconfiguration on `tool-server` - multiple SSRF that were possible due to the CSRF - path traversal via flow creator - multiple shell injections - lack of auth on `MCP/CLI <-> tool-server` path (bug details discussed in person). After the other fixes this is somewhat redundant, but doesn't hurt and protects against theoretical access attempts from within other processes running on the user's device. ## AI Summary <details> Bundles six independent fixes that together close a CSRF cluster on the local tool-server. The vulnerabilities chain because the HTTP API has no auth and serves wildcard CORS, so any web origin (Firefox unconditionally; Chrome via PNA-disabled or loopback-served) can drive the tool-server from a cross-origin tab. Each individual fix lives on its own branch (linked below) and stands alone — this PR exists so the cluster can be reviewed and shipped as one piece. ### Fixes 1. **`fix(http): require auth token + drop wildcard CORS`** — per-process 32-byte hex Bearer token, generated by the launcher, persisted in `~/.argent/tool-server.json` with mode `0600`, required on every route except `/preview`. Constant-time compare. Wildcard CORS dropped. Threaded through MCP server + CLI + tools-client. Backwards-compat: when `ARGENT_AUTH_TOKEN` is unset (e.g. `npm run dev`), the server logs a one-shot warning and skips the gate. 2. **`fix(http): reject non-loopback Host headers`** — middleware (before the auth gate) returns 403 for any request whose Host is not `127.0.0.1`, `localhost`, or `::1`. Closes DNS-rebinding bypass. 3. **`fix(ios-profiler-start): prevent shell injection via device_id`** — `detectRunningApp(udid)` no longer interpolates `params.device_id` into `execSync` template strings. Both `xcrun simctl spawn ${udid} …` and the piped `xcrun simctl listapps ${udid} | plutil …` are rewritten to `execFileSync` array form, with the pipe to `plutil` happening in JS (input option) rather than the shell. 4. **`fix(flows): validate name to close path-traversal in getFlowPath`** — `getFlowPath()` requires `^[A-Za-z0-9_\\-]+$` and verifies `path.relative(getFlowsDir(), filePath)` does not start with `..`. Centralizing the check in `getFlowPath` covers every caller (start-recording, add-step, insert-echo, finish-recording, run, read-prerequisite) without per-tool churn. 5. **`fix(source-resolver): reject paths outside projectRoot + non-source extensions`** — `readSourceFragment()` previously accepted any `file` field on the SourceLocation it was given (the field traces back to a React fiber's `_debugSource.fileName`, which is attacker-controllable). Now: containment check via `path.relative(projectRoot, abs)` plus an extension allowlist (`.js/.jsx/.ts/.tsx/.mjs/.cjs/.json`). 6. **`fix(source-maps): reject non-loopback URLs (close SSRF)`** — `doRegister()` validates `sourceMapURL` before fetching: only `http://`/`https://` URLs whose hostname is loopback. Closes a blind-SSRF probe of the host network (cloud metadata at `169.254.169.254`, intranet pages, etc.). ### Individual branches - `fix/auth-token-drop-cors` - `fix/host-header-validation` - `fix/profiler-command-injection` - `fix/flow-name-validation` - `fix/source-resolver-traversal` - `fix/source-map-ssrf` If you want to land them as separate PRs, the integration branch can be dropped after each lands. ## Test plan Unit tests (already in CI): - [x] `npm run build` passes (workspace tsc clean) - [x] `npm run test -w @argent/tool-server` — **510 passed (was 473 on `main`)**, +37 new tests across: - `test/http-auth.test.ts` — 10 tests (auth, CORS removal, dev-mode escape hatch) - `test/http-host-validation.test.ts` — 7 tests (loopback allow, DNS-rebinding reject) - `test/flows/flow-utils.test.ts` — +5 tests (path-traversal name rejection) - `test/metro/source-resolver.test.ts` — +5 tests (containment + extension allowlist) - `test/metro/source-maps-ssrf.test.ts` — 10 tests (loopback-only allow) Live end-to-end (run locally against this branch's bundled artifacts): - [x] Launcher spawns tool-server; `~/.argent/tool-server.json` contains a 64-char hex token and is mode `0600` - [x] `GET /tools` without `Authorization` → **401** - [x] `GET /tools` with wrong token → **401** - [x] `GET /tools` with correct token → **200** (61 tools) - [x] Request with `Host: attacker.example` → **403** - [x] No `Access-Control-Allow-Origin` header on responses - [x] Cross-origin command-injection probe via `device_id=\"x;touch /tmp/marker;#\"` does NOT create the marker file (Vuln 4 closed) - [x] `flow-start-recording` with `name=\"../../../tmp/escape\"` rejected with `Invalid flow name` - [x] `flow-start-recording` with a valid name still works - [x] `tools-client.fetchTools()` and `tools-client.callTool('list-simulators')` succeed via the auth-token threading (the path the MCP server uses internally) - [x] MCP-stdio entry point (`startMcpServer`) end-to-end: `initialize` → `tools/list` (61 tools) → `tools/call('list-simulators')` returns content. Confirms the launcher → token → fetch → tool-server flow is intact for the production `argent mcp` path. ## Notes for reviewers - `/preview` is intentionally exempt from the auth gate (it was not gated before; tightening it deserves a separate PR). - The `ARGENT_AUTH_TOKEN`-unset escape hatch keeps `npm run dev` workable; production `argent mcp` always passes a token via env, so the warning never fires there. - The auth bundle changes the public return shape of `ensureToolsServer` from `Promise<string>` to `Promise<{ url, token }>`. The two in-tree callers (`argent-mcp/mcp-server.ts`, `argent-tools-client/tools-client.ts`) were updated; external consumers using `ARGENT_TOOLS_URL` should also set `ARGENT_AUTH_TOKEN` to match the server. </details>	2026-06-03 15:57:42 +02:00