Draft
Conversation
`detectRunningApp(udid)` interpolated `params.device_id` directly into
two execSync template strings:
execSync(`xcrun simctl spawn ${udid} launchctl list`)
execSync(`xcrun simctl listapps ${udid} | plutil -convert json -o - -`)
A device_id like `x;touch /tmp/pwn;#` was parsed by /bin/sh as additional
commands, giving any caller (including a cross-origin browser POST, since
the tool-server has no auth and serves wildcard CORS) arbitrary command
execution as the user running the tool-server.
Switched both calls to execFileSync with array args, which never invokes
a shell. The piped `xcrun listapps | plutil` is now two stages with the
plist piped via stdin (input option) instead of through /bin/sh.
The unsanitized `name` parameter was joined into <project_root>/.argent/flows/<name>.yaml so a name like "../../../tmp/x" landed the YAML file at an attacker-chosen path. Combined with the no-auth/wildcard-CORS HTTP surface, a cross-origin browser POST could write attacker-controlled YAML anywhere the user can write — .github/workflows/*.yaml, docker-compose.yml, etc. Adds assertSafeFlowName() in flow-utils.ts. getFlowPath() now requires ^[A-Za-z0-9_\-]+$ (letters, digits, underscore, hyphen — no separators, no "..", no spaces) and additionally checks that the resolved path stays inside the flows directory via path.relative. Centralizing the check in getFlowPath() means every caller (flow-start-recording, flow-add-step, flow-insert-echo, flow-finish-recording, flow-run, flow-read-prerequisite) is covered without per-tool churn.
The tool-server binds to 127.0.0.1, but a public attacker page that DNS-rebinds its hostname to 127.0.0.1 still reaches us — the Host header is the only signal that distinguishes that traffic from a legitimate same-origin request. Adds an early middleware (runs before CORS) that 403s any request whose Host header is not a loopback name (127.0.0.1, localhost, ::1). IPv6 literals in brackets are normalised before the check. Without this, an attacker can pair a DNS-rebinding flow with the existing wildcard CORS to read /tools and POST to /tools/:name from any public origin.
…xtensions
readSourceFragment() previously accepted any `file` field on the
SourceLocation it was given:
- if absolute, it read it directly (anywhere on disk);
- if relative, it joined with projectRoot but never verified the
resolved path stayed inside (so "../../../etc/passwd" worked).
The `file` field originates in a React fiber's _debugSource.fileName,
which a malicious app — or a cross-origin browser tab leveraging Vuln 1
to call debugger-evaluate — can mutate to any string. Combined with the
no-auth HTTP surface, this was an arbitrary file-read primitive
(~/.gitconfig, ~/.aws/credentials, /etc/passwd, ...).
Adds two gates inside readSourceFragment:
1. Containment: path.resolve the absolute candidate, then
path.relative(projectRoot, absPath) — null if it starts with `..`
or is itself absolute (escaped projectRoot).
2. Extension allowlist: only .js/.jsx/.ts/.tsx/.mjs/.cjs/.json. Even
.env files inside the project are out of scope here.
Both checks return null silently; the fiber-source-fragment path is
opportunistic (the inspector never required it to succeed) so attackers
get no signal about whether a file existed.
doRegister() fetched the sourceMapURL from a Debugger.scriptParsed CDP event with no URL validation. Combined with the no-auth tool-server HTTP surface, a malicious app — or any caller of debugger-evaluate / debugger-connect — could inject a script whose //# sourceMappingURL pointed at an attacker-chosen URL. The tool-server would then dutifully fetch it, turning it into a blind SSRF probe of the host network (internal services, AWS metadata at 169.254.169.254, intranet pages, etc.). Adds isAllowedSourceMapURL(): only http/https URLs whose hostname is localhost, 127.0.0.1, or ::1 (IPv6 brackets stripped). Anything else, including malformed URLs and file:// schemes, is rejected and the map is silently skipped. The data: URI branch is untouched — those are inline and don't fetch.
The tool-server exposed every registered tool over HTTP with no
authentication and Access-Control-Allow-Origin: * on every response, so
any web origin (Firefox unconditionally; Chrome via PNA-disabled or
loopback-served) could read /tools and invoke /tools/:name from a
cross-origin browser tab.
Adds a per-process auth token:
- launcher.ts generates a 32-byte hex token at server start, passes it
to the spawned tool-server via ARGENT_AUTH_TOKEN, and persists it in
~/.argent/tool-server.json with mode 0600.
- http.ts requires Authorization: Bearer <token> on every route except
/preview (which is the in-process UI and was not gated before;
tightening it is a follow-up). Constant-time compare prevents
byte-by-byte timing recovery. Wrong/missing token returns 401.
- argent-tools-client and argent-mcp/mcp-server thread the token
through every fetch (fetchTools, callTool, healthcheck, /shutdown).
- argent-cli/server.ts reads the token for `argent server status`
healthcheck. The token is hidden from --json output (replaced with
`hasToken: bool`).
- ensureToolsServer now returns { url, token } instead of just url.
Also drops the global Access-Control-Allow-Origin: * middleware. The
tool-server is in-process stdio for MCP and same-origin for /preview;
no browser needs cross-origin access. Without CORS, the browser cannot
even read response bodies for hostile cross-origin probes.
Backward compat: when ARGENT_AUTH_TOKEN is unset (e.g., `npm run dev`)
the server logs a one-shot warning and skips the gate, so existing local
dev workflows keep working. Stale state files lacking a token are
treated as a launcher-generation mismatch and the server is respawned.
latekvo
changed the title
…gnature - Run prettier --write on source-resolver.ts and source-resolver.test.ts (collapsed extension allowlist + readSourceFragment call sites onto single lines, per repo style). - Update launcher.test.ts to match the new buildToolsServerEnv signature (token is now the third positional arg, baseEnv moved to fourth). Adds an assertion that the auth token is forwarded into the spawned tool-server's env.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
WIP - I'm yet to review this code in detail.
Human Summary
This PR aims to fix bugs that are benign for now, could become serious in the future:
tool-serverMCP/CLI <-> tool-serverpath (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
fix(http): require auth token + drop wildcard CORS— per-process 32-byte hex Bearer token, generated by the launcher, persisted in~/.argent/tool-server.jsonwith mode0600, required on every route except/preview. Constant-time compare. Wildcard CORS dropped. Threaded through MCP server + CLI + tools-client. Backwards-compat: whenARGENT_AUTH_TOKENis unset (e.g.npm run dev), the server logs a one-shot warning and skips the gate.fix(http): reject non-loopback Host headers— middleware (before the auth gate) returns 403 for any request whose Host is not127.0.0.1,localhost, or::1. Closes DNS-rebinding bypass.fix(ios-profiler-start): prevent shell injection via device_id—detectRunningApp(udid)no longer interpolatesparams.device_idintoexecSynctemplate strings. Bothxcrun simctl spawn ${udid} …and the pipedxcrun simctl listapps ${udid} | plutil …are rewritten toexecFileSyncarray form, with the pipe toplutilhappening in JS (input option) rather than the shell.fix(flows): validate name to close path-traversal in getFlowPath—getFlowPath()requires^[A-Za-z0-9_\\-]+$and verifiespath.relative(getFlowsDir(), filePath)does not start with... Centralizing the check ingetFlowPathcovers every caller (start-recording, add-step, insert-echo, finish-recording, run, read-prerequisite) without per-tool churn.fix(source-resolver): reject paths outside projectRoot + non-source extensions—readSourceFragment()previously accepted anyfilefield on the SourceLocation it was given (the field traces back to a React fiber's_debugSource.fileName, which is attacker-controllable). Now: containment check viapath.relative(projectRoot, abs)plus an extension allowlist (.js/.jsx/.ts/.tsx/.mjs/.cjs/.json).fix(source-maps): reject non-loopback URLs (close SSRF)—doRegister()validatessourceMapURLbefore fetching: onlyhttp:///https://URLs whose hostname is loopback. Closes a blind-SSRF probe of the host network (cloud metadata at169.254.169.254, intranet pages, etc.).Individual branches
fix/auth-token-drop-corsfix/host-header-validationfix/profiler-command-injectionfix/flow-name-validationfix/source-resolver-traversalfix/source-map-ssrfIf you want to land them as separate PRs, the integration branch can be dropped after each lands.
Test plan
Unit tests (already in CI):
npm run buildpasses (workspace tsc clean)npm run test -w @argent/tool-server— 510 passed (was 473 onmain), +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):
~/.argent/tool-server.jsoncontains a 64-char hex token and is mode0600GET /toolswithoutAuthorization→ 401GET /toolswith wrong token → 401GET /toolswith correct token → 200 (61 tools)Host: attacker.example→ 403Access-Control-Allow-Originheader on responsesdevice_id=\"x;touch /tmp/marker;#\"does NOT create the marker file (Vuln 4 closed)flow-start-recordingwithname=\"../../../tmp/escape\"rejected withInvalid flow nameflow-start-recordingwith a valid name still workstools-client.fetchTools()andtools-client.callTool('list-simulators')succeed via the auth-token threading (the path the MCP server uses internally)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 productionargent mcppath.Notes for reviewers
/previewis intentionally exempt from the auth gate (it was not gated before; tightening it deserves a separate PR).ARGENT_AUTH_TOKEN-unset escape hatch keepsnpm run devworkable; productionargent mcpalways passes a token via env, so the warning never fires there.ensureToolsServerfromPromise<string>toPromise<{ url, token }>. The two in-tree callers (argent-mcp/mcp-server.ts,argent-tools-client/tools-client.ts) were updated; external consumers usingARGENT_TOOLS_URLshould also setARGENT_AUTH_TOKENto match the server.