diff --git a/agent-logs/2026-04-24T151451-0500-per-client-artifact-roots.md b/agent-logs/2026-04-24T151451-0500-per-client-artifact-roots.md new file mode 100644 index 0000000..e42e73e --- /dev/null +++ b/agent-logs/2026-04-24T151451-0500-per-client-artifact-roots.md @@ -0,0 +1,131 @@ +# Per-Client Artifact Roots + +Repo: browser_mcp +Branch: per-client-artifact-roots +Base: trunk @ 91cf8f8 + +## Goal + +Implement per-agent artifact roots for Browser MCP so screenshot/PDF `path` writes in shared-daemon mode resolve against the calling MCP client process cwd, not the daemon cwd. + +## Implementation Notes + +- Added an artifact-root helper for canonical root resolution, workspace-bound path validation, and destination preparation. +- Threaded `client_artifact_root` from the stdio proxy URL into daemon ingress WebSocket data, then into MCP session creation. +- Stored per-session artifact-root context in registry-private metadata so exported session snapshots do not expose local paths. +- Updated screenshot/PDF persistence to resolve paths from the calling session's artifact root. +- Follow-up PR review fixes gated `client_artifact_root` to loopback daemon ingress or authenticated shared daemons, kept remote proxy URLs from sending cwd metadata by default, preserved existing `tool_error` instances during artifact persistence, and made missing-root failures descriptive. +- Second review loop fix bases proxy cwd-metadata attachment on the daemon-reported bind host so a proxy connecting through `127.0.0.1` to a daemon bound on `0.0.0.0` does not send metadata that ingress will reject. +- Third review loop fix stops auto-attaching client cwd metadata to authenticated non-loopback daemons unless `MCP_ATTACH_CLIENT_ARTIFACT_ROOT=1`/`true` is explicitly set. +- Fourth review loop fix validates artifact parent path segments before creating directories so parent symlinks cannot escape the artifact root, and defers non-loopback artifact-root filesystem checks until after initialize token validation. +- Fifth review loop fix handles `EEXIST` races during segment-by-segment artifact directory creation and adds `MCP_ATTACH_CLIENT_ARTIFACT_ROOT=0`/`false` as an explicit loopback attachment opt-out for tunneled remote daemons. +- Sixth review loop fix revalidates the session artifact root against its original canonical path before any root-level artifact write, so replacing the root path with a symlink after session creation cannot redirect screenshots or PDFs. +- Seventh review loop fix adds a daemon-issued artifact-root token, stores it in owner-scoped daemon state, has stdio proxies attach it with cwd metadata, and rejects untrusted `client_artifact_root` inputs before filesystem validation. +- Eighth review loop fix redacts artifact-root metadata from proxy connection errors and falls back to daemon-cwd artifact paths when daemon state is missing the artifact-root token. +- Ninth review loop fix validates daemon state against daemon health before using an artifact-root token, so stale or mismatched state degrades to daemon-cwd artifact paths. +- Tenth review loop fix falls back to the OS temp directory when the daemon launch cwd has been deleted, preserving non-artifact MCP connectivity. +- Eleventh review loop fix skips proxy artifact-root attachment when the client cwd is unavailable and skips the deleted-cwd regression on Windows where the process cwd cannot be removed. +- Twelfth review loop fix wraps artifact-root revalidation filesystem races so deleted roots produce artifact validation errors instead of raw filesystem exceptions. +- Thirteenth review loop fix strips artifact-root URL params when attachment is disabled, requires daemon-state host matches before token reuse, and moves artifact-root paths out of the exported session shape. +- Fourteenth review loop fix fails proxy startup when per-client attachment was expected but the token is missing or stale, normalizes more artifact path races, allows dot-prefixed child paths, and writes daemon state via a private temp file plus rename. +- Fifteenth review loop fix removes the redundant post-rename daemon-state chmod, updates final storage-model wording, and logs cwd-unavailable fallback for implicit loopback attachment. +- Sixteenth review loop fix treats loopback daemon host aliases as equivalent during state-token reuse and compares artifact-root tokens with timing-safe equality. +- Seventeenth review loop fix adds the required modification notice to the updated stdio proxy source file. +- Eighteenth review loop fix adds the required modification notice to the new artifact helper source file. +- Nineteenth review loop fix maps deferred artifact-root resolution failures to `INVALID_ARGUMENT` and writes artifacts through a no-follow file open on POSIX. +- Twentieth review loop fix updates the fallback docs, rejects directory artifact targets, validates cached auto-auth tokens against daemon state, and clones artifact-root context at the registry boundary. +- Twenty-first review loop fix revalidates walked artifact parents immediately before nested directory creation and preserves explicit artifact-root URL metadata when proxy cwd is unavailable. +- Twenty-second review loop fix includes daemon start time in daemon-state secret reuse checks, adds modification notices to modified source files, qualifies daemon-cwd fallback docs, and revalidates artifact parents after file open before mutation. +- CI follow-up gives the process-spawning daemon singleton integration test an explicit 20s timeout and makes daemon state share the ingress start timestamp so the started-at freshness check is stable across slower runners. +- Twenty-third review loop fix persists daemon state only after ingress binds, clones object-form protocol artifact roots, and adds modification notices to modified tests. +- Twenty-fourth review loop fix makes proxies wait for matching persisted daemon state before deriving artifact-root tokens, while keeping state persistence after ingress bind, and makes daemon teardown continue even if best-effort state refresh fails. +- Twenty-fifth review loop fix extends the daemon shutdown polling window to match the test budget and centralizes artifact-root context cloning in the shared artifact helper. +- Twenty-sixth review loop fix avoids delaying proxy startup solely for optional auto-auth state logging when artifact-root attachment is disabled. +- Twenty-seventh review loop fix reports raw artifact persistence I/O failures as `TOOL_FAILED` and restores the original `process.cwd` descriptor in stdio proxy tests. +- Twenty-eighth review loop fix caps combined daemon health and state polling to the advertised connect timeout and clears delayed state writer timers before test cleanup. +- Twenty-ninth review loop fix writes artifacts through a same-directory temp file plus final rename, preserves captured screenshot payloads when post-capture artifact-root lookup races session cleanup, gives the daemon singleton test more timeout headroom, and makes stale daemon-state token failures name the state path and recovery options. +- Thirtieth review loop fix applies CodeRabbit's changelog wording correction and records the latest artifact-write/session-race behavior in the living spec changelog. +- Thirty-first local review fix allows nested artifact directory creation under a symlinked artifact root that was already accepted and pinned by realpath, while preserving rejection for symlinks below the root. +- Thirty-second review loop fix waits for matching daemon state when auto-auth is requested by an auth-enabled daemon, and overwrites stale artifact-root URL tokens with the current daemon-issued token. +- Thirty-third review loop fix anchors Linux artifact finalization to the opened parent directory, pins deferred artifact roots across implicit MCP session rebinds, and preserves `saved: false` artifact fallback responses if session cleanup races the post-call recovery mark. +- Thirty-fourth review loop fix documents the non-Linux fd-relative filesystem limitation in code and adds an explicit dated changelog note tying the README updates to the living spec. +- Updated living spec, changelog, and local README to document per-client artifact persistence. + +## Verification + +- `bun test local-mcp-bun/tests/unit/stdio_proxy_server.test.ts local-mcp-bun/tests/unit/session_registry.test.ts local-mcp-bun/tests/integration/parity_remaining.test.ts local-mcp-bun/tests/integration/daemon_ingress_cleanup.test.ts` passed. +- `bun test local-mcp-bun/tests/integration/tool_router.test.ts local-mcp-bun/tests/integration/mcp_stdio_protocol_compat.test.ts local-mcp-bun/tests/integration/daemon_singleton_proxy.test.ts` passed. +- `bun run --cwd local-mcp-bun lint:spec` passed. +- `bun run --cwd local-mcp-bun lint:docs` passed. +- `bun run --cwd local-mcp-bun check:popup-ui` passed. +- `bun run --cwd local-mcp-bun test:unit` passed. +- `bun run --cwd local-mcp-bun test:integration` passed. +- `bun run --cwd local-mcp-bun test:hard-gate` reached E2E and failed because the live daemon already owned default bridge port `37777`. +- `LOCAL_MCP_TEST_BRIDGE_PORT=48777 bun run --cwd local-mcp-bun test:hard-gate` passed. +- PR review follow-up: `bun test local-mcp-bun/tests/unit/stdio_proxy_server.test.ts local-mcp-bun/tests/integration/daemon_ingress_cleanup.test.ts local-mcp-bun/tests/integration/parity_remaining.test.ts` passed. +- PR review follow-up: `bun run --cwd local-mcp-bun lint:spec`, `bun run --cwd local-mcp-bun lint:docs`, and `bun run --cwd local-mcp-bun check:popup-ui` passed. +- PR review follow-up: `bun run --cwd local-mcp-bun test:unit`, `bun run --cwd local-mcp-bun test:integration`, and `LOCAL_MCP_TEST_BRIDGE_PORT=48777 bun run --cwd local-mcp-bun test:hard-gate` passed. +- Second review loop: `bun test local-mcp-bun/tests/unit/stdio_proxy_server.test.ts local-mcp-bun/tests/integration/daemon_ingress_cleanup.test.ts local-mcp-bun/tests/integration/daemon_singleton_proxy.test.ts` passed. +- Second review loop: `bun run --cwd local-mcp-bun lint:spec`, `bun run --cwd local-mcp-bun lint:docs`, and `LOCAL_MCP_TEST_BRIDGE_PORT=48777 bun run --cwd local-mcp-bun test:hard-gate` passed. +- Third review loop: `bun test local-mcp-bun/tests/unit/daemon_spawn_target.test.ts local-mcp-bun/tests/unit/stdio_proxy_server.test.ts local-mcp-bun/tests/integration/daemon_ingress_cleanup.test.ts local-mcp-bun/tests/integration/daemon_singleton_proxy.test.ts` passed. +- Third review loop: `bun run --cwd local-mcp-bun lint:spec`, `bun run --cwd local-mcp-bun lint:docs`, and `LOCAL_MCP_TEST_BRIDGE_PORT=48777 bun run --cwd local-mcp-bun test:hard-gate` passed. +- Fourth review loop: `bun test local-mcp-bun/tests/integration/parity_remaining.test.ts local-mcp-bun/tests/integration/daemon_ingress_cleanup.test.ts local-mcp-bun/tests/unit/daemon_spawn_target.test.ts local-mcp-bun/tests/unit/stdio_proxy_server.test.ts` passed. +- Fourth review loop: `bun run --cwd local-mcp-bun lint:spec`, `bun run --cwd local-mcp-bun lint:docs`, and `LOCAL_MCP_TEST_BRIDGE_PORT=48777 bun run --cwd local-mcp-bun test:hard-gate` passed. +- Fifth review loop: `bun test local-mcp-bun/tests/unit/daemon_spawn_target.test.ts local-mcp-bun/tests/integration/parity_remaining.test.ts local-mcp-bun/tests/integration/daemon_ingress_cleanup.test.ts` passed. +- Fifth review loop: `bun run --cwd local-mcp-bun lint:spec`, `bun run --cwd local-mcp-bun lint:docs`, and `LOCAL_MCP_TEST_BRIDGE_PORT=48777 bun run --cwd local-mcp-bun test:hard-gate` passed. +- Sixth review loop: `bun test local-mcp-bun/tests/unit/daemon_spawn_target.test.ts local-mcp-bun/tests/integration/parity_remaining.test.ts local-mcp-bun/tests/integration/daemon_ingress_cleanup.test.ts` passed. +- Sixth review loop: `bun run --cwd local-mcp-bun lint:spec`, `bun run --cwd local-mcp-bun lint:docs`, and `LOCAL_MCP_TEST_BRIDGE_PORT=48777 bun run --cwd local-mcp-bun test:hard-gate` passed. +- Seventh review loop: `bun test local-mcp-bun/tests/unit/stdio_proxy_server.test.ts local-mcp-bun/tests/unit/daemon_spawn_target.test.ts local-mcp-bun/tests/integration/daemon_ingress_cleanup.test.ts local-mcp-bun/tests/integration/daemon_singleton_proxy.test.ts` passed. +- Seventh review loop: `bun test local-mcp-bun/tests/integration/parity_remaining.test.ts`, `bun run --cwd local-mcp-bun lint:spec`, `bun run --cwd local-mcp-bun lint:docs`, and `LOCAL_MCP_TEST_BRIDGE_PORT=48777 bun run --cwd local-mcp-bun test:hard-gate` passed. +- Eighth review loop: `bun test local-mcp-bun/tests/unit/stdio_proxy_server.test.ts local-mcp-bun/tests/unit/daemon_spawn_target.test.ts local-mcp-bun/tests/integration/daemon_ingress_cleanup.test.ts local-mcp-bun/tests/integration/daemon_singleton_proxy.test.ts` passed. +- Eighth review loop: `bun run --cwd local-mcp-bun lint:spec`, `bun run --cwd local-mcp-bun lint:docs`, and `LOCAL_MCP_TEST_BRIDGE_PORT=48777 bun run --cwd local-mcp-bun test:hard-gate` passed. +- Ninth review loop: `bun test local-mcp-bun/tests/unit/daemon_spawn_target.test.ts local-mcp-bun/tests/integration/daemon_singleton_proxy.test.ts` passed. +- Ninth review loop: `bun run --cwd local-mcp-bun lint:spec`, `bun run --cwd local-mcp-bun lint:docs`, and `LOCAL_MCP_TEST_BRIDGE_PORT=48777 bun run --cwd local-mcp-bun test:hard-gate` passed. +- Tenth review loop: `bun test local-mcp-bun/tests/unit/artifacts.test.ts local-mcp-bun/tests/integration/daemon_ingress_cleanup.test.ts local-mcp-bun/tests/integration/daemon_singleton_proxy.test.ts` passed. +- Tenth review loop: `bun run --cwd local-mcp-bun lint:spec`, `bun run --cwd local-mcp-bun lint:docs`, and `LOCAL_MCP_TEST_BRIDGE_PORT=48777 bun run --cwd local-mcp-bun test:hard-gate` passed. +- Eleventh review loop: `bun test local-mcp-bun/tests/unit/stdio_proxy_server.test.ts local-mcp-bun/tests/unit/artifacts.test.ts local-mcp-bun/tests/integration/daemon_singleton_proxy.test.ts`, `bun run --cwd local-mcp-bun lint:spec`, `bun run --cwd local-mcp-bun lint:docs`, and `LOCAL_MCP_TEST_BRIDGE_PORT=48777 bun run --cwd local-mcp-bun test:hard-gate` passed. +- Twelfth review loop: `bun test local-mcp-bun/tests/unit/artifacts.test.ts local-mcp-bun/tests/integration/parity_remaining.test.ts`, `bun run --cwd local-mcp-bun lint:spec`, `bun run --cwd local-mcp-bun lint:docs`, and `LOCAL_MCP_TEST_BRIDGE_PORT=48777 bun run --cwd local-mcp-bun test:hard-gate` passed. +- Thirteenth review loop: `bun test local-mcp-bun/tests/unit/stdio_proxy_server.test.ts local-mcp-bun/tests/unit/daemon_spawn_target.test.ts local-mcp-bun/tests/unit/session_registry.test.ts local-mcp-bun/tests/unit/artifacts.test.ts local-mcp-bun/tests/integration/tool_router.test.ts local-mcp-bun/tests/integration/parity_remaining.test.ts local-mcp-bun/tests/integration/daemon_singleton_proxy.test.ts`, `bun run --cwd local-mcp-bun lint:spec`, `bun run --cwd local-mcp-bun lint:docs`, and `LOCAL_MCP_TEST_BRIDGE_PORT=48777 bun run --cwd local-mcp-bun test:hard-gate` passed. +- Fourteenth review loop: `bun test local-mcp-bun/tests/unit/artifacts.test.ts local-mcp-bun/tests/unit/daemon_spawn_target.test.ts local-mcp-bun/tests/unit/stdio_proxy_server.test.ts local-mcp-bun/tests/unit/session_registry.test.ts local-mcp-bun/tests/integration/daemon_singleton_proxy.test.ts local-mcp-bun/tests/integration/parity_remaining.test.ts`, `bun run --cwd local-mcp-bun lint:spec`, `bun run --cwd local-mcp-bun lint:docs`, and `LOCAL_MCP_TEST_BRIDGE_PORT=48777 bun run --cwd local-mcp-bun test:hard-gate` passed. +- Fifteenth review loop: `bun test local-mcp-bun/tests/unit/stdio_proxy_server.test.ts local-mcp-bun/tests/unit/daemon_spawn_target.test.ts local-mcp-bun/tests/unit/artifacts.test.ts`, `bun run --cwd local-mcp-bun lint:docs`, `bun run --cwd local-mcp-bun lint:spec`, and `LOCAL_MCP_TEST_BRIDGE_PORT=48777 bun run --cwd local-mcp-bun test:hard-gate` passed. Remote Ubuntu failure on the prior head was an E2E popup wait timeout after unit/integration tests passed. +- Sixteenth review loop: `bun test local-mcp-bun/tests/unit/daemon_spawn_target.test.ts local-mcp-bun/tests/integration/daemon_ingress_cleanup.test.ts`, `bun run --cwd local-mcp-bun lint:spec`, `bun run --cwd local-mcp-bun lint:docs`, and `LOCAL_MCP_TEST_BRIDGE_PORT=48777 bun run --cwd local-mcp-bun test:hard-gate` passed. +- Seventeenth review loop: `bun test local-mcp-bun/tests/unit/stdio_proxy_server.test.ts`, `bun run --cwd local-mcp-bun lint:compliance`, `bun run --cwd local-mcp-bun lint:docs`, and `bun run --cwd local-mcp-bun lint:spec` passed. +- Eighteenth review loop: `bun test local-mcp-bun/tests/unit/artifacts.test.ts local-mcp-bun/tests/unit/stdio_proxy_server.test.ts`, `bun run --cwd local-mcp-bun lint:compliance`, `bun run --cwd local-mcp-bun lint:docs`, and `bun run --cwd local-mcp-bun lint:spec` passed. +- Nineteenth review loop: `bun test local-mcp-bun/tests/unit/artifacts.test.ts local-mcp-bun/tests/unit/mcp_protocol_session.test.ts local-mcp-bun/tests/integration/parity_remaining.test.ts`, `bun run --cwd local-mcp-bun lint:spec`, `bun run --cwd local-mcp-bun lint:docs`, and `bun run --cwd local-mcp-bun lint:compliance` passed. +- Nineteenth review loop: `LOCAL_MCP_TEST_BRIDGE_PORT=48777 bun run --cwd local-mcp-bun test:hard-gate` passed. +- Twentieth review loop: `bun test local-mcp-bun/tests/unit/artifacts.test.ts local-mcp-bun/tests/unit/session_registry.test.ts local-mcp-bun/tests/unit/daemon_spawn_target.test.ts`, `bun run --cwd local-mcp-bun lint:spec`, `bun run --cwd local-mcp-bun lint:docs`, and `bun run --cwd local-mcp-bun lint:compliance` passed. +- Twentieth review loop: `LOCAL_MCP_TEST_BRIDGE_PORT=48777 bun run --cwd local-mcp-bun test:hard-gate` passed. +- Twenty-first review loop: `bun test local-mcp-bun/tests/unit/artifacts.test.ts local-mcp-bun/tests/unit/stdio_proxy_server.test.ts`, `bun run --cwd local-mcp-bun lint:spec`, `bun run --cwd local-mcp-bun lint:docs`, and `bun run --cwd local-mcp-bun lint:compliance` passed. +- Twenty-first review loop: `LOCAL_MCP_TEST_BRIDGE_PORT=48777 bun run --cwd local-mcp-bun test:hard-gate` passed. +- Twenty-second review loop: `bun test local-mcp-bun/tests/unit/artifacts.test.ts local-mcp-bun/tests/unit/daemon_spawn_target.test.ts local-mcp-bun/tests/unit/stdio_proxy_server.test.ts local-mcp-bun/tests/unit/mcp_protocol_session.test.ts local-mcp-bun/tests/unit/session_registry.test.ts local-mcp-bun/tests/integration/parity_remaining.test.ts local-mcp-bun/tests/integration/daemon_ingress_cleanup.test.ts`, `bun run --cwd local-mcp-bun lint:spec`, `bun run --cwd local-mcp-bun lint:docs`, and `bun run --cwd local-mcp-bun lint:compliance` passed. +- Twenty-second review loop: `LOCAL_MCP_TEST_BRIDGE_PORT=48777 bun run --cwd local-mcp-bun test:hard-gate` passed. +- CI timeout follow-up: `bun test local-mcp-bun/tests/integration/daemon_singleton_proxy.test.ts` and `LOCAL_MCP_TEST_BRIDGE_PORT=48777 bun run --cwd local-mcp-bun test:hard-gate` passed. +- Twenty-third review loop: `bun test local-mcp-bun/tests/unit/mcp_protocol_session.test.ts local-mcp-bun/tests/integration/daemon_singleton_proxy.test.ts local-mcp-bun/tests/integration/parity_remaining.test.ts`, `bun run --cwd local-mcp-bun lint:spec`, `bun run --cwd local-mcp-bun lint:docs`, `bun run --cwd local-mcp-bun lint:compliance`, and `LOCAL_MCP_TEST_BRIDGE_PORT=48777 bun run --cwd local-mcp-bun test:hard-gate` passed. +- Twenty-fourth review loop: `bun test local-mcp-bun/tests/unit/daemon_spawn_target.test.ts` passed. +- Twenty-fourth review loop: `bun test local-mcp-bun/tests/integration/daemon_singleton_proxy.test.ts local-mcp-bun/tests/integration/parity_remaining.test.ts` passed. +- Twenty-fourth review loop: `git diff --check`, `bun run --cwd local-mcp-bun lint:spec`, `bun run --cwd local-mcp-bun lint:docs`, `bun run --cwd local-mcp-bun lint:compliance`, and `LOCAL_MCP_TEST_BRIDGE_PORT=48777 bun run --cwd local-mcp-bun test:hard-gate` passed. +- Twenty-fifth review loop: `bun test local-mcp-bun/tests/unit/artifacts.test.ts local-mcp-bun/tests/unit/session_registry.test.ts local-mcp-bun/tests/unit/mcp_protocol_session.test.ts local-mcp-bun/tests/integration/daemon_singleton_proxy.test.ts`, `git diff --check`, `bun run --cwd local-mcp-bun lint:spec`, `bun run --cwd local-mcp-bun lint:docs`, `bun run --cwd local-mcp-bun lint:compliance`, and `LOCAL_MCP_TEST_BRIDGE_PORT=48777 bun run --cwd local-mcp-bun test:hard-gate` passed. +- Twenty-sixth review loop: `bun test local-mcp-bun/tests/unit/daemon_spawn_target.test.ts local-mcp-bun/tests/unit/artifacts.test.ts local-mcp-bun/tests/unit/session_registry.test.ts local-mcp-bun/tests/unit/mcp_protocol_session.test.ts local-mcp-bun/tests/integration/daemon_singleton_proxy.test.ts`, `git diff --check`, `bun run --cwd local-mcp-bun lint:spec`, `bun run --cwd local-mcp-bun lint:docs`, `bun run --cwd local-mcp-bun lint:compliance`, and `LOCAL_MCP_TEST_BRIDGE_PORT=48777 bun run --cwd local-mcp-bun test:hard-gate` passed. +- Twenty-seventh review loop: `bun test local-mcp-bun/tests/unit/stdio_proxy_server.test.ts local-mcp-bun/tests/integration/parity_remaining.test.ts`, `git diff --check`, `bun run --cwd local-mcp-bun lint:spec`, `bun run --cwd local-mcp-bun lint:docs`, `bun run --cwd local-mcp-bun lint:compliance`, and `LOCAL_MCP_TEST_BRIDGE_PORT=48777 bun run --cwd local-mcp-bun test:hard-gate` passed. +- Twenty-eighth review loop: `bun test local-mcp-bun/tests/unit/daemon_spawn_target.test.ts local-mcp-bun/tests/integration/daemon_singleton_proxy.test.ts`, `git diff --check`, `bun run --cwd local-mcp-bun lint:spec`, `bun run --cwd local-mcp-bun lint:docs`, `bun run --cwd local-mcp-bun lint:compliance`, and `LOCAL_MCP_TEST_BRIDGE_PORT=48777 bun run --cwd local-mcp-bun test:hard-gate` passed. +- Twenty-ninth review loop: `bun test local-mcp-bun/tests/unit/artifacts.test.ts local-mcp-bun/tests/integration/parity_remaining.test.ts local-mcp-bun/tests/integration/daemon_singleton_proxy.test.ts local-mcp-bun/tests/unit/daemon_spawn_target.test.ts`, `git diff --check`, `bun run --cwd local-mcp-bun lint:spec`, `bun run --cwd local-mcp-bun lint:docs`, `bun run --cwd local-mcp-bun lint:compliance`, and `LOCAL_MCP_TEST_BRIDGE_PORT=48777 bun run --cwd local-mcp-bun test:hard-gate` passed. +- Thirtieth review loop: `git diff --check`, `bun run --cwd local-mcp-bun lint:spec`, and `bun test local-mcp-bun/tests/unit/artifacts.test.ts local-mcp-bun/tests/integration/parity_remaining.test.ts` passed. +- Thirty-first local review loop: `bun test local-mcp-bun/tests/unit/artifacts.test.ts`, `git diff --check`, `bun run --cwd local-mcp-bun lint:spec`, `bun run --cwd local-mcp-bun lint:docs`, `bun run --cwd local-mcp-bun lint:compliance`, `cr review --agent --type uncommitted`, and `LOCAL_MCP_TEST_BRIDGE_PORT=48777 bun run --cwd local-mcp-bun test:hard-gate` passed. CodeRabbit CLI reported zero findings. +- Thirty-second review loop: `bun test local-mcp-bun/tests/unit/daemon_spawn_target.test.ts local-mcp-bun/tests/unit/stdio_proxy_server.test.ts`, `git diff --check`, `bun run --cwd local-mcp-bun lint:spec`, `bun run --cwd local-mcp-bun lint:docs`, `bun run --cwd local-mcp-bun lint:compliance`, `cr review --agent --type uncommitted`, and `LOCAL_MCP_TEST_BRIDGE_PORT=48777 bun run --cwd local-mcp-bun test:hard-gate` passed. CodeRabbit CLI reported zero findings. +- Thirty-third review loop: `bun test local-mcp-bun/tests/unit/artifacts.test.ts local-mcp-bun/tests/unit/mcp_protocol_session.test.ts local-mcp-bun/tests/integration/parity_remaining.test.ts`, `git diff --check`, `bun run --cwd local-mcp-bun lint:spec`, `bun run --cwd local-mcp-bun lint:docs`, `bun run --cwd local-mcp-bun lint:compliance`, and `LOCAL_MCP_TEST_BRIDGE_PORT=48777 bun run --cwd local-mcp-bun test:hard-gate` passed. `cr review --agent --type uncommitted` completed with one minor finding in existing untracked `local-mcp-bun/.tmp/browser_mcp_release_smoke.ts`, outside the PR diff. +- Thirty-fourth review loop: `bun test local-mcp-bun/tests/unit/artifacts.test.ts`, `git diff --check`, `bun run --cwd local-mcp-bun lint:spec`, `bun run --cwd local-mcp-bun lint:docs`, and `bun run --cwd local-mcp-bun lint:compliance` passed. + +## Relevant Git History + +```text +91cf8f8 docs(agent-logs): record 0.5.2 release bump +a0ff9e4 chore(release): bump version to 0.5.2 +72299b1 fix(mcp): keep stale cleanup transports recoverable (#17) +a1929bb fix(mcp): keep stale cleanup transports recoverable +92291a6 chore(release): bump version to 0.5.0 +e83dad7 Merge pull request #16 from TWP-Technologies/session-cleanup-log +af34894 docs(agent-logs): note top-stack log update +5eab7b7 docs(agent-logs): record session cleanup review loop +89e28ff docs(agent-logs): record stale cleanup stack +61b3d5d Merge pull request #15 from TWP-Technologies/session-cleanup-docs +``` diff --git a/local-mcp-bun/README.md b/local-mcp-bun/README.md index 2ce62d0..bc292d5 100644 --- a/local-mcp-bun/README.md +++ b/local-mcp-bun/README.md @@ -45,6 +45,8 @@ Default runtime is: With defaults, each MCP client process is a stdio proxy that auto-connects to a shared local daemon. The daemon owns the browser bridge bind (`BRIDGE_PORT`) and supports multiple concurrent MCP clients. +Relative artifact paths for screenshot and PDF tools are resolved against the calling MCP client process cwd. In shared-daemon mode, the stdio proxy passes that cwd to the daemon once during connection setup with a daemon-issued artifact-root token, so callers do not need to include workspace metadata in every tool call. The proxy only adds this metadata automatically when the daemon state and requested daemon host are loopback-scoped; set `MCP_ATTACH_CLIENT_ARTIFACT_ROOT=1` to opt in explicitly or `MCP_ATTACH_CLIENT_ARTIFACT_ROOT=0` to disable it. + ### Daemon Modes - `MCP_DAEMON_MODE=auto` (default for `BRIDGE_MODE=websocket`): connect to existing daemon or spawn one, then proxy stdio to daemon ingress. @@ -76,10 +78,14 @@ You only need to set bridge env vars when overriding defaults (for example custo - `MCP_DAEMON_IDLE_TIMEOUT_MS` defaults to `900000` (15 minutes). - `MCP_DAEMON_CONNECT_TIMEOUT_MS` defaults to `10000`. - `MCP_SESSION_IDLE_TIMEOUT_MINUTES` defaults to `120`; set `0` to disable automatic stale-session cleanup. -- `MCP_DAEMON_STATE_PATH` overrides daemon metadata path (default temp path keyed by daemon port). +- `MCP_DAEMON_STATE_PATH` overrides daemon metadata path (default temp path keyed by daemon port). Daemon state includes local-only connection capabilities and is written with owner-only file permissions where the host OS supports them. - `MCP_AUTH_TOKEN=` enables token auth with explicit value. - `MCP_AUTH_TOKEN=auto` or `MCP_AUTH_AUTO=1` enables token auth with auto token generated/reused by the daemon runtime and persisted in daemon state metadata. - If auth env vars are not set, auth remains disabled (local loopback boundary still enforced). +- `MCP_ATTACH_CLIENT_ARTIFACT_ROOT=1` or `true` forces the stdio proxy to send client cwd metadata to non-loopback daemon ingress. Use this only when the daemon can resolve the same filesystem paths and MCP auth is enabled. +- `MCP_ATTACH_CLIENT_ARTIFACT_ROOT=0` or `false` prevents automatic cwd metadata attachment even when the daemon reports a loopback bind host, which is useful for SSH tunnels to remote daemons. +- `client_artifact_root` connection metadata is accepted only with the matching daemon-issued `client_artifact_root_token`, and only when daemon ingress is loopback-bound or MCP auth is enabled. Do not expose unauthenticated daemon ingress on non-loopback interfaces. +- If automatic artifact-root attachment is expected but the daemon-issued artifact-root token is missing or stale, proxy startup fails; set `MCP_ATTACH_CLIENT_ARTIFACT_ROOT=0` to explicitly use the shared daemon cwd for artifact paths. ### Stale Session Cleanup diff --git a/local-mcp-bun/src/artifacts.ts b/local-mcp-bun/src/artifacts.ts new file mode 100644 index 0000000..3e2a8a5 --- /dev/null +++ b/local-mcp-bun/src/artifacts.ts @@ -0,0 +1,361 @@ +// Modified by [KnotFalse] +import { randomUUID } from "node:crypto"; +import { constants, existsSync, lstatSync, mkdirSync, realpathSync, renameSync, rmSync, statSync } from "node:fs"; +import { open, type FileHandle } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { basename, dirname, isAbsolute, relative, resolve } from "node:path"; +import { tool_error } from "./errors"; + +export interface artifact_root_context { + artifact_root: string; + artifact_root_real: string; +} + +export type artifact_root_context_input = artifact_root_context | (() => artifact_root_context); + +const artifact_temp_file_write_flags = + constants.O_WRONLY | + constants.O_CREAT | + constants.O_EXCL | + (process.platform === "win32" ? 0 : constants.O_NOFOLLOW); +const artifact_parent_directory_open_flags = constants.O_RDONLY | constants.O_DIRECTORY; + +export function is_within_root(root_path: string, candidate_path: string): boolean { + const relative_path = relative(root_path, candidate_path); + return relative_path === "" || (!/^\.\.(?:[\\/]|$)/.test(relative_path) && !isAbsolute(relative_path)); +} + +export function resolve_artifact_root(root_path = process.cwd()): artifact_root_context { + const artifact_root = resolve(root_path); + let artifact_root_real: string; + try { + artifact_root_real = realpathSync(artifact_root); + } catch { + throw new Error(`artifact root must be an existing directory: ${artifact_root}`); + } + + try { + if (!statSync(artifact_root_real).isDirectory()) { + throw new Error(`artifact root must be an existing directory: ${artifact_root}`); + } + } catch { + throw new Error(`artifact root must be an existing directory: ${artifact_root}`); + } + + return { + artifact_root, + artifact_root_real, + }; +} + +export function resolve_default_artifact_root(): artifact_root_context { + try { + return resolve_artifact_root(); + } catch { + return resolve_artifact_root(tmpdir()); + } +} + +export function resolve_artifact_path(root_context: artifact_root_context, requested_path: string): string { + const candidate_path = isAbsolute(requested_path) + ? resolve(requested_path) + : resolve(root_context.artifact_root, requested_path); + + if (!is_within_root(root_context.artifact_root, candidate_path)) { + throw new tool_error("INVALID_ARGUMENT", "artifact path must stay within the current workspace", false, { + path: requested_path, + artifact_root: root_context.artifact_root, + }); + } + + return candidate_path; +} + +export function resolve_artifact_root_context_input(root_context: artifact_root_context_input): artifact_root_context { + return typeof root_context === "function" ? root_context() : root_context; +} + +export function clone_artifact_root_context(root_context: artifact_root_context): artifact_root_context { + return { + artifact_root: root_context.artifact_root, + artifact_root_real: root_context.artifact_root_real, + }; +} + +export function clone_artifact_root_context_input( + root_context: artifact_root_context_input, +): artifact_root_context_input { + return typeof root_context === "function" ? root_context : clone_artifact_root_context(root_context); +} + +function throw_artifact_boundary_error(root_context: artifact_root_context, requested_path: string): never { + throw new tool_error("INVALID_ARGUMENT", "artifact path must stay within the current workspace", false, { + path: requested_path, + artifact_root: root_context.artifact_root, + }); +} + +function get_error_code(error: unknown): string | undefined { + if (!error || typeof error !== "object" || !("code" in error)) { + return undefined; + } + + const error_code = (error as { code?: unknown }).code; + return typeof error_code === "string" ? error_code : undefined; +} + +function throw_artifact_symlink_error(root_context: artifact_root_context, requested_path: string): never { + throw new tool_error("INVALID_ARGUMENT", "artifact path cannot traverse a symbolic link", false, { + path: requested_path, + artifact_root: root_context.artifact_root, + }); +} + +function throw_artifact_parent_error(root_context: artifact_root_context, requested_path: string): never { + throw new tool_error("INVALID_ARGUMENT", "artifact parent path must be a directory", false, { + path: requested_path, + artifact_root: root_context.artifact_root, + }); +} + +function validate_artifact_root_directory(root_context: artifact_root_context, requested_path: string): void { + let root_stat: ReturnType; + try { + root_stat = statSync(root_context.artifact_root); + } catch { + throw_artifact_parent_error(root_context, requested_path); + } + + if (!root_stat.isDirectory()) { + throw_artifact_parent_error(root_context, requested_path); + } + + let root_real: string; + try { + root_real = realpathSync(root_context.artifact_root); + } catch { + throw_artifact_parent_error(root_context, requested_path); + } + + if (root_real !== root_context.artifact_root_real) { + throw_artifact_boundary_error(root_context, requested_path); + } +} + +function validate_artifact_parent_segment( + root_context: artifact_root_context, + directory_path: string, + requested_path: string, +): void { + let directory_lstat: ReturnType; + try { + directory_lstat = lstatSync(directory_path); + } catch { + throw_artifact_parent_error(root_context, requested_path); + } + + if (directory_lstat.isSymbolicLink()) { + throw_artifact_symlink_error(root_context, requested_path); + } + + if (!directory_lstat.isDirectory()) { + throw_artifact_parent_error(root_context, requested_path); + } + + let directory_real: string; + try { + directory_real = realpathSync(directory_path); + } catch { + throw_artifact_parent_error(root_context, requested_path); + } + + if (!is_within_root(root_context.artifact_root_real, directory_real)) { + throw_artifact_boundary_error(root_context, requested_path); + } +} + +function validate_artifact_parent_before_creation( + root_context: artifact_root_context, + parent_path: string, + requested_path: string, +): void { + if (relative(root_context.artifact_root, parent_path) === "") { + validate_artifact_root_directory(root_context, requested_path); + return; + } + + validate_artifact_parent_segment(root_context, parent_path, requested_path); +} + +function prepare_artifact_parent_directory( + root_context: artifact_root_context, + parent_path: string, + requested_path: string, +): void { + if (!is_within_root(root_context.artifact_root, parent_path)) { + throw_artifact_boundary_error(root_context, requested_path); + } + + const relative_parent_path = relative(root_context.artifact_root, parent_path); + const path_segments = relative_parent_path === "" ? [] : relative_parent_path.split(/[\\/]+/); + let current_path = root_context.artifact_root; + validate_artifact_root_directory(root_context, requested_path); + + for (const path_segment of path_segments) { + current_path = resolve(current_path, path_segment); + + if (!existsSync(current_path)) { + validate_artifact_parent_before_creation(root_context, dirname(current_path), requested_path); + try { + mkdirSync(current_path); + } catch (error) { + if (get_error_code(error) !== "EEXIST") { + throw error; + } + } + } + + validate_artifact_parent_segment(root_context, current_path, requested_path); + } +} + +export function prepare_artifact_destination( + root_context: artifact_root_context, + absolute_path: string, + requested_path: string, +): void { + prepare_artifact_parent_directory(root_context, dirname(absolute_path), requested_path); + + if (existsSync(absolute_path)) { + try { + const destination_lstat = lstatSync(absolute_path); + if (destination_lstat.isSymbolicLink()) { + throw new tool_error("INVALID_ARGUMENT", "artifact path cannot target a symbolic link", false, { + path: requested_path, + artifact_root: root_context.artifact_root, + }); + } + + if (destination_lstat.isDirectory()) { + throw new tool_error("INVALID_ARGUMENT", "artifact path cannot target a directory", false, { + path: requested_path, + artifact_root: root_context.artifact_root, + }); + } + } catch (error) { + if (error instanceof tool_error) { + throw error; + } + + if (get_error_code(error) !== "ENOENT") { + throw error; + } + } + } +} + +function build_artifact_temp_name(absolute_path: string): string { + const destination_name = basename(absolute_path); + return `.${destination_name}.${process.pid}.${randomUUID()}.tmp`; +} + +async function open_stable_parent_directory(parent_path: string): Promise { + if (process.platform !== "linux") { + // macOS has openat/renameat syscalls, but Node/libuv do not expose fd-relative wrappers. + // Non-Linux writes rely on repeated destination validation plus no-follow final file opens. + return undefined; + } + + return await open(parent_path, artifact_parent_directory_open_flags); +} + +function get_stable_parent_path(parent_directory_handle: FileHandle | undefined, parent_path: string): string { + if (!parent_directory_handle) { + return parent_path; + } + + // Node does not expose renameat; Linux proc fd paths keep source and target relative to the opened directory. + return `/proc/self/fd/${parent_directory_handle.fd}`; +} + +function validate_written_artifact_file( + root_context: artifact_root_context, + absolute_path: string, + anchored_path: string, + requested_path: string, +): void { + prepare_artifact_destination(root_context, absolute_path, requested_path); + + let absolute_real: string; + let anchored_real: string; + try { + absolute_real = realpathSync(absolute_path); + anchored_real = realpathSync(anchored_path); + } catch { + throw_artifact_boundary_error(root_context, requested_path); + } + + if (absolute_real !== anchored_real || !is_within_root(root_context.artifact_root_real, anchored_real)) { + throw_artifact_boundary_error(root_context, requested_path); + } +} + +export async function write_artifact_file( + root_context: artifact_root_context, + absolute_path: string, + requested_path: string, + bytes: Uint8Array, +): Promise { + prepare_artifact_destination(root_context, absolute_path, requested_path); + const parent_path = dirname(absolute_path); + const destination_name = basename(absolute_path); + const temp_name = build_artifact_temp_name(absolute_path); + let parent_directory_handle: FileHandle | undefined; + let temp_cleanup_path: string | undefined; + let final_cleanup_path: string | undefined; + + try { + parent_directory_handle = await open_stable_parent_directory(parent_path); + const stable_parent_path = get_stable_parent_path(parent_directory_handle, parent_path); + const temp_path = resolve(stable_parent_path, temp_name); + const final_path = resolve(stable_parent_path, destination_name); + temp_cleanup_path = temp_path; + + const file_handle = await open(temp_path, artifact_temp_file_write_flags, 0o600); + try { + // Keep the writable handle off the caller-requested final pathname; final placement is a rename. + prepare_artifact_destination(root_context, absolute_path, requested_path); + await file_handle.writeFile(bytes); + } finally { + await file_handle.close(); + } + + prepare_artifact_destination(root_context, absolute_path, requested_path); + renameSync(temp_path, final_path); + temp_cleanup_path = undefined; + final_cleanup_path = final_path; + validate_written_artifact_file(root_context, absolute_path, final_path, requested_path); + final_cleanup_path = undefined; + } catch (error) { + if (final_cleanup_path) { + rmSync(final_cleanup_path, { force: true }); + } + + if (temp_cleanup_path) { + rmSync(temp_cleanup_path, { force: true }); + } + + if (get_error_code(error) === "ELOOP") { + throw new tool_error("INVALID_ARGUMENT", "artifact path cannot target a symbolic link", false, { + path: requested_path, + artifact_root: root_context.artifact_root, + }); + } + + throw error; + } finally { + if (parent_directory_handle) { + await parent_directory_handle.close(); + } + } +} diff --git a/local-mcp-bun/src/daemon_controller.ts b/local-mcp-bun/src/daemon_controller.ts index 1046262..cfbdb65 100644 --- a/local-mcp-bun/src/daemon_controller.ts +++ b/local-mcp-bun/src/daemon_controller.ts @@ -1,8 +1,9 @@ +// Modified by [KnotFalse] import { spawn } from "node:child_process"; -import { mkdirSync, readFileSync, writeFileSync } from "node:fs"; +import { mkdirSync, readFileSync, renameSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; -import { dirname, join, resolve } from "node:path"; -import { resolve_auth_token } from "./config"; +import { basename, dirname, join, resolve } from "node:path"; +import { is_loopback_host, resolve_auth_token } from "./config"; import { daemon_ingress_server, type daemon_health_snapshot } from "./daemon_ingress_server"; import { mcp_stdio_server } from "./mcp_stdio_server"; import { local_mcp_runtime, type runtime_options } from "./runtime"; @@ -14,7 +15,7 @@ interface daemon_auth_resolution { auto_requested: boolean; } -interface daemon_state_record { +export interface daemon_state_record { service: "local-mcp-daemon"; pid: number; started_at: string; @@ -27,6 +28,7 @@ interface daemon_state_record { auth_auto: boolean; auth_token?: string; auth_token_hint?: string; + artifact_root_token?: string; } export interface daemon_run_options { @@ -101,8 +103,27 @@ function read_daemon_state(path: string): daemon_state_record | undefined { } function write_daemon_state(path: string, state: daemon_state_record): void { - mkdirSync(dirname(path), { recursive: true }); - writeFileSync(path, `${JSON.stringify(state, null, 2)}\n`, "utf8"); + const state_dir = dirname(path); + const tmp_path = join(state_dir, `.${basename(path)}.${process.pid}.${crypto.randomUUID()}.tmp`); + mkdirSync(state_dir, { recursive: true }); + + try { + writeFileSync(tmp_path, `${JSON.stringify(state, null, 2)}\n`, { encoding: "utf8", mode: 0o600 }); + renameSync(tmp_path, path); + } catch (error) { + rmSync(tmp_path, { force: true }); + throw error; + } +} + +function sleep(timeout_ms: number): Promise { + return new Promise((resolve_promise) => { + setTimeout(resolve_promise, timeout_ms); + }); +} + +function remaining_timeout_ms(deadline_ms: number): number { + return Math.max(0, deadline_ms - Date.now()); } function build_token_hint(token: string): string { @@ -113,6 +134,168 @@ function build_token_hint(token: string): string { return `${token.slice(0, 8)}***`; } +function build_artifact_root_token(): string { + return `artifact-root-${crypto.randomUUID()}`; +} + +function resolve_client_artifact_root_attachment_override(env: Record): boolean | undefined { + const configured = get_non_empty_string(env.MCP_ATTACH_CLIENT_ARTIFACT_ROOT)?.toLowerCase(); + if (configured === "1" || configured === "true") { + return true; + } + + if (configured === "0" || configured === "false") { + return false; + } + + return undefined; +} + +export function should_attach_client_artifact_root_to_daemon( + daemon_target: Pick, + env: Record, +): boolean { + const configured = resolve_client_artifact_root_attachment_override(env); + if (typeof configured === "boolean") { + return configured; + } + + return is_loopback_host(daemon_target.daemon_host); +} + +export interface proxy_artifact_root_attachment { + attach_client_artifact_root: boolean; + artifact_root_token?: string; + missing_artifact_root_token: boolean; +} + +function daemon_hosts_match(state_host: string | undefined, target_host: string): boolean { + if (state_host === target_host) { + return true; + } + + return typeof state_host === "string" && is_loopback_host(state_host) && is_loopback_host(target_host); +} + +function daemon_state_matches_health( + daemon_target: Pick, + daemon_health: Pick, + daemon_state: + | { daemon_host?: string; started_at?: string; pid?: number; daemon_port?: number; bridge_port?: number } + | undefined, +): boolean { + return ( + typeof daemon_state === "object" && + daemon_state !== null && + daemon_hosts_match(daemon_state.daemon_host, daemon_target.daemon_host) && + daemon_state.started_at === daemon_health.started_at && + daemon_state.pid === daemon_health.pid && + daemon_state.daemon_port === daemon_health.daemon_port && + daemon_state.bridge_port === daemon_health.bridge_port + ); +} + +export function resolve_proxy_artifact_root_attachment( + daemon_target: Pick, + daemon_health: Pick, + env: Record, + daemon_state: + | { + daemon_host?: string; + started_at?: string; + pid?: number; + daemon_port?: number; + bridge_port?: number; + artifact_root_token?: string; + } + | undefined, +): proxy_artifact_root_attachment { + const requested_attach = should_attach_client_artifact_root_to_daemon(daemon_target, env); + const artifact_root_token = daemon_state_matches_health(daemon_target, daemon_health, daemon_state) + ? get_non_empty_string(daemon_state?.artifact_root_token) + : undefined; + + if (!requested_attach) { + return { + attach_client_artifact_root: false, + missing_artifact_root_token: false, + }; + } + + if (!artifact_root_token) { + return { + attach_client_artifact_root: false, + missing_artifact_root_token: true, + }; + } + + return { + attach_client_artifact_root: true, + artifact_root_token, + missing_artifact_root_token: false, + }; +} + +export function resolve_proxy_auth_token_for_log( + daemon_target: Pick, + daemon_health: Pick, + env: Record, + daemon_state: + | { + daemon_host?: string; + started_at?: string; + pid?: number; + daemon_port?: number; + bridge_port?: number; + auth_token?: string; + } + | undefined, +): string | undefined { + if (!is_auto_auth_requested(env) || !daemon_state_matches_health(daemon_target, daemon_health, daemon_state)) { + return undefined; + } + + return get_non_empty_string(daemon_state?.auth_token); +} + +function is_auto_auth_requested(env: Record): boolean { + return get_non_empty_string(env.MCP_AUTH_TOKEN) === "auto" || get_non_empty_string(env.MCP_AUTH_AUTO) === "1"; +} + +function proxy_needs_matching_daemon_state( + daemon_target: Pick, + daemon_health: Pick, + env: Record, +): boolean { + return should_attach_client_artifact_root_to_daemon(daemon_target, env) || (daemon_health.auth_enabled && is_auto_auth_requested(env)); +} + +export async function wait_for_proxy_daemon_state( + options: Pick, + daemon_health: Pick, + timeout_ms = options.daemon_connect_timeout_ms, +): Promise { + const daemon_target = { daemon_host: options.daemon_host }; + const needs_matching_state = proxy_needs_matching_daemon_state(daemon_target, daemon_health, options.env); + let daemon_state = read_daemon_state(options.daemon_state_path); + + if (!needs_matching_state || daemon_state_matches_health(daemon_target, daemon_health, daemon_state)) { + return daemon_state; + } + + const started_at = Date.now(); + while (Date.now() - started_at < timeout_ms) { + await sleep(daemon_health_poll_interval_ms); + daemon_state = read_daemon_state(options.daemon_state_path); + + if (daemon_state_matches_health(daemon_target, daemon_health, daemon_state)) { + return daemon_state; + } + } + + return daemon_state; +} + function resolve_daemon_auth( env: Record, daemon_state: daemon_state_record | undefined, @@ -174,7 +357,12 @@ async function fetch_daemon_health( return undefined; } - if (typeof payload.pid !== "number" || typeof payload.daemon_port !== "number") { + if ( + typeof payload.started_at !== "string" || + typeof payload.pid !== "number" || + typeof payload.daemon_port !== "number" || + typeof payload.bridge_port !== "number" + ) { return undefined; } @@ -283,7 +471,11 @@ function register_daemon_signal_handlers( ): void { const shutdown = () => { daemon_state.updated_at = new Date().toISOString(); - write_daemon_state(daemon_state_path, daemon_state); + try { + write_daemon_state(daemon_state_path, daemon_state); + } catch (error) { + console.error(`[daemon] failed to update daemon state during shutdown: ${error}`); + } ingress .stop() .catch(() => { @@ -326,6 +518,8 @@ async function start_daemon_runtime(options: daemon_run_options): Promise const daemon_state = read_daemon_state(options.daemon_state_path); const auth = resolve_daemon_auth(options.env, daemon_state); const auth_token_hint = auth.auth_token ? build_token_hint(auth.auth_token) : undefined; + const artifact_root_token = build_artifact_root_token(); + const started_at = new Date().toISOString(); const runtime = new local_mcp_runtime({ ...options.runtime_options, auth_token: auth.auth_token, @@ -334,7 +528,7 @@ async function start_daemon_runtime(options: daemon_run_options): Promise const state: daemon_state_record = { service: "local-mcp-daemon", pid: process.pid, - started_at: new Date().toISOString(), + started_at, updated_at: new Date().toISOString(), daemon_host: options.daemon_host, daemon_port: options.daemon_port, @@ -344,27 +538,50 @@ async function start_daemon_runtime(options: daemon_run_options): Promise auth_auto: auth.auto_requested, auth_token: auth.auto_requested ? auth.auth_token : undefined, auth_token_hint, + artifact_root_token, }; - write_daemon_state(options.daemon_state_path, state); - const ingress = new daemon_ingress_server({ - runtime, - daemon_host: options.daemon_host, - daemon_port: options.daemon_port, - bridge_host: options.runtime_options.bridge_host, - bridge_port: options.runtime_options.bridge_port, - daemon_state_path: options.daemon_state_path, - idle_timeout_ms: options.daemon_idle_timeout_ms, - auth_enabled: state.auth_enabled, - auth_token_hint: state.auth_token_hint, - on_idle_timeout: async () => { - state.updated_at = new Date().toISOString(); - write_daemon_state(options.daemon_state_path, state); - await ingress.stop(); - await runtime.stop(); - process.exit(0); - }, - }); + let ingress: daemon_ingress_server | undefined; + try { + ingress = new daemon_ingress_server({ + runtime, + daemon_host: options.daemon_host, + daemon_port: options.daemon_port, + bridge_host: options.runtime_options.bridge_host, + bridge_port: options.runtime_options.bridge_port, + daemon_state_path: options.daemon_state_path, + idle_timeout_ms: options.daemon_idle_timeout_ms, + auth_enabled: state.auth_enabled, + auth_token_hint: state.auth_token_hint, + artifact_root_token: state.artifact_root_token, + started_at, + on_idle_timeout: async () => { + state.updated_at = new Date().toISOString(); + try { + write_daemon_state(options.daemon_state_path, state); + } catch (error) { + console.error(`[daemon] failed to update daemon state during idle shutdown: ${error}`); + } finally { + await ingress?.stop(); + await runtime.stop(); + process.exit(0); + } + }, + }); + write_daemon_state(options.daemon_state_path, state); + } catch (error) { + await ingress?.stop().catch(() => { + // ignore cleanup after failed daemon ingress startup + }); + await runtime.stop().catch(() => { + // ignore cleanup after failed daemon ingress startup + }); + throw error; + } + + if (!ingress) { + throw new Error("daemon ingress failed to start"); + } register_daemon_signal_handlers(runtime, ingress, state, options.daemon_state_path); @@ -382,24 +599,45 @@ async function start_daemon_runtime(options: daemon_run_options): Promise async function start_proxy_runtime( options: daemon_run_options, daemon_health: daemon_health_snapshot, + daemon_state: daemon_state_record | undefined, ): Promise { - const auto_requested = - get_non_empty_string(options.env.MCP_AUTH_TOKEN) === "auto" || get_non_empty_string(options.env.MCP_AUTH_AUTO) === "1"; + const auth_token_for_log = resolve_proxy_auth_token_for_log( + { daemon_host: options.daemon_host }, + daemon_health, + options.env, + daemon_state, + ); - if (auto_requested) { - const daemon_state = read_daemon_state(options.daemon_state_path); - if (daemon_state?.auth_token) { - console.error(`[auth] daemon token for initialize.params.token: ${daemon_state.auth_token}`); - } + if (auth_token_for_log) { + console.error(`[auth] daemon token for initialize.params.token: ${auth_token_for_log}`); } if (daemon_health.auth_enabled) { console.error(`[daemon] connected shared runtime auth hint: ${daemon_health.auth_token_hint ?? "redacted"}`); } + const artifact_root_attachment = resolve_proxy_artifact_root_attachment( + { daemon_host: options.daemon_host }, + daemon_health, + options.env, + daemon_state, + ); + if (artifact_root_attachment.missing_artifact_root_token) { + const state_status = daemon_state_matches_health({ daemon_host: options.daemon_host }, daemon_health, daemon_state) + ? "matching daemon state is missing artifact_root_token" + : "daemon state file is missing or does not match the running daemon"; + throw new Error( + `daemon artifact-root token missing or stale: ${state_status}; ` + + `state_path=${options.daemon_state_path}; daemon_pid=${daemon_health.pid}; started_at=${daemon_health.started_at}; ` + + "restart the shared daemon, remove the stale daemon state file, or set MCP_ATTACH_CLIENT_ARTIFACT_ROOT=0 to use daemon cwd for artifact paths", + ); + } + const proxy_server = new stdio_proxy_server({ daemon_url: `ws://${options.daemon_host}:${options.daemon_port}/mcp`, connect_timeout_ms: options.daemon_connect_timeout_ms, + attach_client_artifact_root: artifact_root_attachment.attach_client_artifact_root, + artifact_root_token: artifact_root_attachment.artifact_root_token, }); await proxy_server.start(); } @@ -415,10 +653,11 @@ export async function run_daemon_mode(options: daemon_run_options): Promise void; } +function token_equals(left: string, right: string): boolean { + const left_buffer = Buffer.from(left); + const right_buffer = Buffer.from(right); + return left_buffer.length === right_buffer.length && timingSafeEqual(left_buffer, right_buffer); +} + export interface daemon_health_snapshot { service: "local-mcp-daemon"; pid: number; @@ -39,8 +51,10 @@ interface daemon_ingress_server_options { idle_timeout_ms: number; auth_enabled: boolean; auth_token_hint?: string; + artifact_root_token?: string; on_idle_timeout: () => Promise; cleanup_interval_ms?: number; + started_at?: string; } const default_cleanup_interval_ms = 60_000; @@ -64,6 +78,7 @@ export class daemon_ingress_server { private readonly cleanup_interval_ms: number; private readonly auth_enabled: boolean; private readonly auth_token_hint?: string; + private readonly artifact_root_token?: string; private readonly on_idle_timeout: () => Promise; private readonly sessions_by_connection_id: Map; private readonly sockets_by_connection_id: Map; @@ -89,13 +104,14 @@ export class daemon_ingress_server { this.cleanup_interval_ms = resolve_cleanup_interval_ms(options.cleanup_interval_ms); this.auth_enabled = options.auth_enabled; this.auth_token_hint = options.auth_token_hint; + this.artifact_root_token = options.artifact_root_token; this.on_idle_timeout = options.on_idle_timeout; this.sessions_by_connection_id = new Map(); this.sockets_by_connection_id = new Map(); this.agent_session_id_by_connection_id = new Map(); this.connection_id_by_agent_session_id = new Map(); this.last_activity_at_ms_by_connection_id = new Map(); - this.started_at = new Date().toISOString(); + this.started_at = options.started_at ?? new Date().toISOString(); this.next_connection_id = 1; this.cleanup_in_progress = false; this.stopping = false; @@ -191,12 +207,21 @@ export class daemon_ingress_server { return new Response("not found", { status: 404 }); } + let artifact_root_context: artifact_root_context_input; + try { + artifact_root_context = this.resolve_connection_artifact_root(request_url); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + return new Response(message, { status: 400 }); + } + const connection_id = this.next_connection_id; this.next_connection_id += 1; const upgraded = server_instance.upgrade(request, { data: { connection_id, connected_at: new Date().toISOString(), + artifact_root_context, }, }); @@ -212,6 +237,7 @@ export class daemon_ingress_server { const connection_id = socket.data.connection_id; const session = new mcp_protocol_session({ runtime: this.runtime, + artifact_root_context: socket.data.artifact_root_context, on_agent_session_bound: (agent_session_id) => { this.bind_agent_session_to_connection(connection_id, agent_session_id); }, @@ -232,6 +258,38 @@ export class daemon_ingress_server { this.last_activity_at_ms_by_connection_id.set(connection_id, Date.now()); } + private resolve_connection_artifact_root(request_url: URL): artifact_root_context_input { + if (!request_url.searchParams.has("client_artifact_root")) { + return resolve_default_artifact_root(); + } + + const raw_artifact_root = request_url.searchParams.get("client_artifact_root")?.trim() ?? ""; + const raw_artifact_root_token = request_url.searchParams.get("client_artifact_root_token")?.trim() ?? ""; + if (!this.artifact_root_token || !token_equals(raw_artifact_root_token, this.artifact_root_token)) { + throw new Error("client_artifact_root requires a daemon-issued artifact root token"); + } + + if (raw_artifact_root.length === 0) { + throw new Error("client_artifact_root must be a non-empty absolute path"); + } + + if (!isAbsolute(raw_artifact_root)) { + throw new Error("client_artifact_root must be an absolute path"); + } + + // A TCP daemon cannot infer a peer process cwd. Loopback proxy mode is the local trust boundary; + // network-facing daemon ingress must enable MCP auth before honoring client-supplied roots. + if (!is_loopback_host(this.daemon_host) && !this.auth_enabled) { + throw new Error("client_artifact_root requires loopback daemon ingress or MCP auth"); + } + + if (!is_loopback_host(this.daemon_host)) { + return () => resolve_artifact_root(raw_artifact_root); + } + + return resolve_artifact_root(raw_artifact_root); + } + private handle_socket_message( socket: ServerWebSocket, message: string | Buffer | Uint8Array, diff --git a/local-mcp-bun/src/mcp_protocol_session.ts b/local-mcp-bun/src/mcp_protocol_session.ts index 932ac20..6591dda 100644 --- a/local-mcp-bun/src/mcp_protocol_session.ts +++ b/local-mcp-bun/src/mcp_protocol_session.ts @@ -1,5 +1,13 @@ +// Modified by [KnotFalse] import { tool_error } from "./errors"; import { local_mcp_runtime } from "./runtime"; +import { + clone_artifact_root_context, + clone_artifact_root_context_input, + resolve_default_artifact_root, + type artifact_root_context, + type artifact_root_context_input, +} from "./artifacts"; import type { json_rpc_request, json_rpc_response } from "./types"; const default_mcp_protocol_version = "2024-11-05"; @@ -13,6 +21,7 @@ export const server_info = { interface mcp_protocol_session_options { runtime: local_mcp_runtime; write_response: (response: json_rpc_response) => void; + artifact_root_context?: artifact_root_context_input; on_agent_session_bound?: (agent_session_id: string) => void; on_agent_session_released?: (agent_session_id: string) => void; } @@ -20,8 +29,10 @@ interface mcp_protocol_session_options { export class mcp_protocol_session { private readonly runtime: local_mcp_runtime; private readonly write_response: (response: json_rpc_response) => void; + private readonly artifact_root_context_source: artifact_root_context_input; private readonly on_agent_session_bound?: (agent_session_id: string) => void; private readonly on_agent_session_released?: (agent_session_id: string) => void; + private pinned_artifact_root_context: artifact_root_context | null; private initialized_agent_session_id: string | null; private initialized_client_name: string | undefined; private initialized_token: string | undefined; @@ -31,8 +42,12 @@ export class mcp_protocol_session { public constructor(options: mcp_protocol_session_options) { this.runtime = options.runtime; this.write_response = options.write_response; + this.artifact_root_context_source = clone_artifact_root_context_input( + options.artifact_root_context ?? resolve_default_artifact_root(), + ); this.on_agent_session_bound = options.on_agent_session_bound; this.on_agent_session_released = options.on_agent_session_released; + this.pinned_artifact_root_context = null; this.initialized_agent_session_id = null; this.initialized_client_name = undefined; this.initialized_token = undefined; @@ -119,8 +134,13 @@ export class mcp_protocol_session { } } - const result = this.runtime.tool_router.open_session(client_name, token); + const result = this.runtime.tool_router.open_session( + client_name, + token, + this.resolve_artifact_root_context_for_open(), + ); const session = this.runtime.session_registry.get_session(result.agent_session_id); + this.pin_artifact_root_context(result.agent_session_id); this.initialized_client_name = client_name; this.initialized_token = session.auth_mode === "token" ? token : undefined; this.initialized_auth_mode = session.auth_mode; @@ -346,6 +366,20 @@ export class mcp_protocol_session { this.runtime.session_registry.touch_session(agent_session_id); } + private resolve_artifact_root_context_for_open(): artifact_root_context_input { + return this.pinned_artifact_root_context ?? this.artifact_root_context_source; + } + + private pin_artifact_root_context(agent_session_id: string): void { + if (this.pinned_artifact_root_context) { + return; + } + + this.pinned_artifact_root_context = clone_artifact_root_context( + this.runtime.session_registry.get_session_artifact_root_context(agent_session_id), + ); + } + private resolve_initialized_agent_session_id(): string | null { if (!this.initialized_agent_session_id) { return null; @@ -371,13 +405,18 @@ export class mcp_protocol_session { let result: { agent_session_id: string }; try { - result = this.runtime.tool_router.open_session(this.initialized_client_name, this.initialized_token); + result = this.runtime.tool_router.open_session( + this.initialized_client_name, + this.initialized_token, + this.resolve_artifact_root_context_for_open(), + ); } catch (error) { this.clear_initialized_client_state(); throw error; } const session = this.runtime.session_registry.get_session(result.agent_session_id); + this.pin_artifact_root_context(result.agent_session_id); this.initialized_auth_mode = session.auth_mode; this.set_initialized_agent_session_id(result.agent_session_id); return result.agent_session_id; diff --git a/local-mcp-bun/src/session_registry.ts b/local-mcp-bun/src/session_registry.ts index 3c45ced..70f6d0d 100644 --- a/local-mcp-bun/src/session_registry.ts +++ b/local-mcp-bun/src/session_registry.ts @@ -1,7 +1,14 @@ +// Modified by [KnotFalse] import { generate_agent_session_id, now_iso_string } from "./id"; import { tool_error } from "./errors"; +import { clone_artifact_root_context, resolve_default_artifact_root, type artifact_root_context } from "./artifacts"; import type { agent_session, session_snapshot } from "./types"; +interface agent_session_record { + session: agent_session; + artifact_root_context: artifact_root_context; +} + function parse_session_timestamp_ms(iso_timestamp: string): number | undefined { const parsed = Date.parse(iso_timestamp); if (Number.isFinite(parsed)) { @@ -41,13 +48,17 @@ function is_reaped_session_expired_for_cutoff(session: agent_session, stale_befo } export class session_registry { - private readonly sessions_by_id: Map; + private readonly sessions_by_id: Map; public constructor() { - this.sessions_by_id = new Map(); + this.sessions_by_id = new Map(); } - public create_session(client_name?: string, auth_mode: "none" | "token" = "none"): agent_session { + public create_session( + client_name?: string, + auth_mode: "none" | "token" = "none", + artifact_root_context: artifact_root_context = resolve_default_artifact_root(), + ): agent_session { let candidate = generate_agent_session_id(client_name); while (this.sessions_by_id.has(candidate)) { candidate = generate_agent_session_id(client_name); @@ -64,19 +75,33 @@ export class session_registry { owned_tab_ids: new Set(), }; - this.sessions_by_id.set(candidate, session); + this.sessions_by_id.set(candidate, { + session, + artifact_root_context: clone_artifact_root_context(artifact_root_context), + }); return session; } public get_session(agent_session_id: string): agent_session { - const session = this.sessions_by_id.get(agent_session_id); - if (!session) { + const record = this.sessions_by_id.get(agent_session_id); + if (!record) { throw new tool_error("SESSION_NOT_FOUND", `unknown session: ${agent_session_id}`, false, { agent_session_id, }); } - return session; + return record.session; + } + + public get_session_artifact_root_context(agent_session_id: string): artifact_root_context { + const record = this.sessions_by_id.get(agent_session_id); + if (!record) { + throw new tool_error("SESSION_NOT_FOUND", `unknown session: ${agent_session_id}`, false, { + agent_session_id, + }); + } + + return clone_artifact_root_context(record.artifact_root_context); } public has_session(agent_session_id: string): boolean { @@ -146,7 +171,7 @@ export class session_registry { const stale_before_ms = now_ms - timeout_minutes * 60_000; const stale_session_ids: string[] = []; - for (const session of this.sessions_by_id.values()) { + for (const { session } of this.sessions_by_id.values()) { if (!is_session_stale_for_cutoff(session, stale_before_ms)) { continue; } @@ -163,13 +188,13 @@ export class session_registry { return false; } - const session = this.sessions_by_id.get(agent_session_id); - if (!session) { + const record = this.sessions_by_id.get(agent_session_id); + if (!record) { return false; } const stale_before_ms = now_ms - timeout_minutes * 60_000; - return is_session_stale_for_cutoff(session, stale_before_ms); + return is_session_stale_for_cutoff(record.session, stale_before_ms); } public list_expired_reaped_session_ids(timeout_minutes: number, now_ms = Date.now()): string[] { @@ -180,7 +205,7 @@ export class session_registry { const stale_before_ms = now_ms - timeout_minutes * 60_000; const expired_session_ids: string[] = []; - for (const session of this.sessions_by_id.values()) { + for (const { session } of this.sessions_by_id.values()) { if (!is_reaped_session_expired_for_cutoff(session, stale_before_ms)) { continue; } @@ -197,19 +222,19 @@ export class session_registry { return false; } - const session = this.sessions_by_id.get(agent_session_id); - if (!session) { + const record = this.sessions_by_id.get(agent_session_id); + if (!record) { return false; } const stale_before_ms = now_ms - timeout_minutes * 60_000; - return is_reaped_session_expired_for_cutoff(session, stale_before_ms); + return is_reaped_session_expired_for_cutoff(record.session, stale_before_ms); } public list_session_snapshots(): session_snapshot[] { const snapshots: session_snapshot[] = []; - for (const session of this.sessions_by_id.values()) { + for (const { session } of this.sessions_by_id.values()) { const snapshot: session_snapshot = { agent_session_id: session.agent_session_id, client_name: session.client_name, diff --git a/local-mcp-bun/src/stdio_proxy_server.ts b/local-mcp-bun/src/stdio_proxy_server.ts index da392c9..dcbae0d 100644 --- a/local-mcp-bun/src/stdio_proxy_server.ts +++ b/local-mcp-bun/src/stdio_proxy_server.ts @@ -1,6 +1,75 @@ +// Modified by [KnotFalse] +import { resolve as resolve_path } from "node:path"; +import { is_loopback_host } from "./config"; + interface stdio_proxy_server_options { daemon_url: string; connect_timeout_ms: number; + attach_client_artifact_root?: boolean; + artifact_root_token?: string; +} + +function should_attach_client_artifact_root(daemon_url: string, configured?: boolean): boolean { + if (typeof configured === "boolean") { + return configured; + } + + const url = new URL(daemon_url); + return is_loopback_host(url.hostname); +} + +function resolve_current_working_directory(): string | undefined { + try { + return process.cwd(); + } catch { + return undefined; + } +} + +export function build_daemon_url_with_client_artifact_root( + daemon_url: string, + artifact_root?: string, + attach_client_artifact_root?: boolean, + artifact_root_token?: string, +): string { + const url = new URL(daemon_url); + const attach_artifact_root = should_attach_client_artifact_root(daemon_url, attach_client_artifact_root); + if (!attach_artifact_root) { + url.searchParams.delete("client_artifact_root"); + url.searchParams.delete("client_artifact_root_token"); + return url.toString(); + } + + const has_client_artifact_root = url.searchParams.has("client_artifact_root"); + + if (!has_client_artifact_root) { + const resolved_artifact_root = artifact_root ?? resolve_current_working_directory(); + if (!resolved_artifact_root) { + url.searchParams.delete("client_artifact_root_token"); + return url.toString(); + } + + url.searchParams.set("client_artifact_root", resolve_path(resolved_artifact_root)); + } + + if (artifact_root_token) { + url.searchParams.set("client_artifact_root_token", artifact_root_token); + } + + return url.toString(); +} + +export function redact_daemon_url_for_logs(daemon_url: string): string { + const url = new URL(daemon_url); + if (url.searchParams.has("client_artifact_root")) { + url.searchParams.set("client_artifact_root", ""); + } + + if (url.searchParams.has("client_artifact_root_token")) { + url.searchParams.set("client_artifact_root_token", ""); + } + + return url.toString(); } function is_graceful_daemon_close_reason(reason: string): boolean { @@ -17,7 +86,24 @@ export class stdio_proxy_server { private exited: boolean; public constructor(options: stdio_proxy_server_options) { - this.daemon_url = options.daemon_url; + const artifact_root = resolve_current_working_directory(); + const should_attach_artifact_root = should_attach_client_artifact_root( + options.daemon_url, + options.attach_client_artifact_root, + ); + const has_explicit_client_artifact_root = new URL(options.daemon_url).searchParams.has("client_artifact_root"); + const attach_client_artifact_root = + should_attach_artifact_root && (typeof artifact_root === "string" || has_explicit_client_artifact_root); + if (should_attach_artifact_root && !artifact_root && !has_explicit_client_artifact_root) { + console.error("[daemon] client cwd unavailable; connecting without per-client artifact root metadata"); + } + + this.daemon_url = build_daemon_url_with_client_artifact_root( + options.daemon_url, + artifact_root, + attach_client_artifact_root, + options.artifact_root_token, + ); this.connect_timeout_ms = options.connect_timeout_ms; this.line_buffer = ""; this.closing_intent = false; @@ -55,6 +141,7 @@ export class stdio_proxy_server { private async connect(): Promise { return await new Promise((resolve, reject) => { const socket = new WebSocket(this.daemon_url); + const redacted_daemon_url = redact_daemon_url_for_logs(this.daemon_url); const timeout_id = setTimeout(() => { try { @@ -62,7 +149,7 @@ export class stdio_proxy_server { } catch { // ignore timeout close race } - reject(new Error(`timed out connecting to daemon ingress at ${this.daemon_url}`)); + reject(new Error(`timed out connecting to daemon ingress at ${redacted_daemon_url}`)); }, this.connect_timeout_ms); socket.addEventListener("open", () => { @@ -72,7 +159,7 @@ export class stdio_proxy_server { socket.addEventListener("error", () => { clearTimeout(timeout_id); - reject(new Error(`failed to connect to daemon ingress at ${this.daemon_url}`)); + reject(new Error(`failed to connect to daemon ingress at ${redacted_daemon_url}`)); }); }); } diff --git a/local-mcp-bun/src/tool_router.ts b/local-mcp-bun/src/tool_router.ts index 583ea43..e57ccf4 100644 --- a/local-mcp-bun/src/tool_router.ts +++ b/local-mcp-bun/src/tool_router.ts @@ -1,7 +1,13 @@ +// Modified by [KnotFalse] import { Buffer } from "node:buffer"; -import { existsSync, lstatSync, mkdirSync, realpathSync } from "node:fs"; -import { dirname, isAbsolute, relative, resolve } from "node:path"; import { tool_error, to_tool_error } from "./errors"; +import { + resolve_artifact_path, + resolve_artifact_root_context_input, + write_artifact_file, + type artifact_root_context, + type artifact_root_context_input, +} from "./artifacts"; import { merge_tabs_with_locks, type bridge_transport } from "./bridge_transport"; import { session_registry } from "./session_registry"; import { tab_lock_manager } from "./tab_lock_manager"; @@ -92,8 +98,6 @@ const passthrough_tools = [ ] as const; const system_agent_session_id = "system-router"; -const artifact_root = resolve(process.cwd()); -const artifact_root_real = realpathSync(artifact_root); const default_stale_session_timeout_minutes = 120; const max_stale_session_timeout_minutes = 10_080; @@ -180,24 +184,6 @@ const prompt_catalog = { }, } satisfies Record; -function is_within_root(root_path: string, candidate_path: string): boolean { - const relative_path = relative(root_path, candidate_path); - return relative_path === "" || (!relative_path.startsWith("..") && !isAbsolute(relative_path)); -} - -function resolve_artifact_path(requested_path: string): string { - const candidate_path = isAbsolute(requested_path) ? resolve(requested_path) : resolve(artifact_root, requested_path); - - if (!is_within_root(artifact_root, candidate_path)) { - throw new tool_error("INVALID_ARGUMENT", "artifact path must stay within the current workspace", false, { - path: requested_path, - artifact_root, - }); - } - - return candidate_path; -} - function build_invalid_argument_details(options: invalid_argument_details_options): Record { const details: Record = { recovery_hint: options.recovery_hint, @@ -325,16 +311,43 @@ export class tool_router { }); } - public open_session(client_name?: string, supplied_token?: string): { agent_session_id: string } { + public open_session( + client_name?: string, + supplied_token?: string, + artifact_root_context_input?: artifact_root_context_input, + ): { agent_session_id: string } { const auth_mode = this.assert_token_if_required(supplied_token); - return this.open_authenticated_session(client_name, auth_mode); + const artifact_root_context = this.resolve_session_artifact_root_context(artifact_root_context_input); + return this.open_authenticated_session(client_name, auth_mode, artifact_root_context); + } + + private resolve_session_artifact_root_context( + artifact_root_context_input?: artifact_root_context_input, + ): artifact_root_context | undefined { + if (typeof artifact_root_context_input === "undefined") { + return undefined; + } + + try { + return resolve_artifact_root_context_input(artifact_root_context_input); + } catch (error) { + if (error instanceof tool_error) { + throw error; + } + + const message = error instanceof Error ? error.message : String(error); + throw new tool_error("INVALID_ARGUMENT", "invalid client artifact root", false, { + cause: message, + }); + } } private open_authenticated_session( client_name?: string, auth_mode: "none" | "token" = "none", + artifact_root_context?: artifact_root_context, ): { agent_session_id: string } { - const session = this.session_registry.create_session(client_name, auth_mode); + const session = this.session_registry.create_session(client_name, auth_mode, artifact_root_context); void this.publish_connections_snapshot("open_session"); return { agent_session_id: session.agent_session_id }; } @@ -983,10 +996,20 @@ export class tool_router { }); } - this.session_registry.mark_session_recovered(agent_session_id); + this.mark_session_recovered_if_present(agent_session_id); return result; } + private mark_session_recovered_if_present(agent_session_id: string): void { + try { + this.session_registry.mark_session_recovered(agent_session_id); + } catch (error) { + if (!(error instanceof tool_error) || error.code !== "SESSION_NOT_FOUND") { + throw error; + } + } + } + public async release_locks_for_session(agent_session_id: string): Promise { const released_tab_ids = this.tab_lock_manager.release_locks_by_owner(agent_session_id); this.tab_lock_manager.cancel_waiters_by_owner(agent_session_id); @@ -1912,7 +1935,7 @@ export class tool_router { const result = await this.bridge_transport.call_tool(tool_name, normalized_args, agent_session_id, active_tab_id); if (result && typeof result === "object") { - return await this.persist_artifact_if_requested(tool_name, args, result as Record); + return await this.persist_artifact_if_requested(agent_session_id, tool_name, args, result as Record); } return { result }; @@ -2035,6 +2058,7 @@ export class tool_router { } private async persist_artifact_if_requested( + agent_session_id: string, tool_name: string, args: Record, result: Record, @@ -2053,49 +2077,48 @@ export class tool_router { return result; } - const absolute_path = resolve_artifact_path(requested_path); - const bytes = Buffer.from(data_base64, "base64"); + let absolute_path = requested_path; try { - mkdirSync(dirname(absolute_path), { recursive: true }); - const parent_real = realpathSync(dirname(absolute_path)); - if (!is_within_root(artifact_root_real, parent_real)) { - throw new tool_error("INVALID_ARGUMENT", "artifact path must stay within the current workspace", false, { - path: requested_path, - artifact_root, - }); - } + const artifact_root_context = this.session_registry.get_session_artifact_root_context(agent_session_id); + absolute_path = resolve_artifact_path(artifact_root_context, requested_path); + const bytes = Buffer.from(data_base64, "base64"); + await write_artifact_file(artifact_root_context, absolute_path, requested_path, bytes); + + const persisted_result: Record = { + ...result, + saved: true, + path: absolute_path, + saved_path: absolute_path, + bytes: bytes.byteLength, + }; - if (existsSync(absolute_path) && lstatSync(absolute_path).isSymbolicLink()) { - throw new tool_error("INVALID_ARGUMENT", "artifact path cannot target a symbolic link", false, { - path: requested_path, - artifact_root, - }); + delete persisted_result.data_base64; + + if (tool_name === "browser_pdf_save" && typeof persisted_result.mime_type !== "string") { + persisted_result.mime_type = "application/pdf"; } - await Bun.write(absolute_path, bytes); + return persisted_result; } catch (error) { + if (error instanceof tool_error) { + if (error.code === "SESSION_NOT_FOUND") { + return { + ...result, + saved: false, + path: requested_path, + artifact_error: error.to_payload(), + }; + } + + throw error; + } + const message = error instanceof Error ? error.message : String(error); - throw new tool_error("INVALID_ARGUMENT", `failed to persist ${tool_name} to ${absolute_path}`, false, { + throw new tool_error("TOOL_FAILED", `failed to persist ${tool_name} to ${absolute_path}`, false, { path: absolute_path, cause: message, }); } - - const persisted_result: Record = { - ...result, - saved: true, - path: absolute_path, - saved_path: absolute_path, - bytes: bytes.byteLength, - }; - - delete persisted_result.data_base64; - - if (tool_name === "browser_pdf_save" && typeof persisted_result.mime_type !== "string") { - persisted_result.mime_type = "application/pdf"; - } - - return persisted_result; } } diff --git a/local-mcp-bun/tests/integration/daemon_ingress_cleanup.test.ts b/local-mcp-bun/tests/integration/daemon_ingress_cleanup.test.ts index 3aae146..d15b6e8 100644 --- a/local-mcp-bun/tests/integration/daemon_ingress_cleanup.test.ts +++ b/local-mcp-bun/tests/integration/daemon_ingress_cleanup.test.ts @@ -1,5 +1,6 @@ +// Modified by [KnotFalse] import { expect, test } from "bun:test"; -import { mkdtempSync, rmSync } from "node:fs"; +import { mkdtempSync, readFileSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { daemon_ingress_server } from "../../src/daemon_ingress_server"; @@ -12,13 +13,26 @@ interface initialize_response { agent_session_id?: string; structuredContent?: Record; }; + error?: { + code: number; + message: string; + }; } function random_port(): number { return 30000 + Math.floor(Math.random() * 20000); } -async function create_test_ingress(cleanup_interval_ms: number): Promise<{ +const test_artifact_root_token = "test-artifact-root-token"; + +async function create_test_ingress( + cleanup_interval_ms: number, + options: { + daemon_host?: string; + auth_enabled?: boolean; + auth_token?: string; + } = {}, +): Promise<{ runtime: local_mcp_runtime; ingress: daemon_ingress_server; daemon_port: number; @@ -36,19 +50,21 @@ async function create_test_ingress(cleanup_interval_ms: number): Promise<{ bridge_host: "127.0.0.1", bridge_port, session_idle_timeout_minutes: 120, + auth_token: options.auth_token, }); try { const ingress = new daemon_ingress_server({ runtime, - daemon_host: "127.0.0.1", + daemon_host: options.daemon_host ?? "127.0.0.1", daemon_port, bridge_host: "127.0.0.1", bridge_port, daemon_state_path, idle_timeout_ms: 60_000, cleanup_interval_ms, - auth_enabled: false, + auth_enabled: options.auth_enabled ?? typeof options.auth_token === "string", + artifact_root_token: test_artifact_root_token, on_idle_timeout: async () => {}, }); @@ -212,6 +228,215 @@ test("daemon ingress soft-reaps stale bound sessions without closing the socket" } }); +test("daemon ingress resolves relative artifact paths against connection artifact root", async () => { + const { runtime, ingress, daemon_port, daemon_state_dir } = await create_test_ingress(25); + const artifact_root_dir = mkdtempSync(join(tmpdir(), "local-mcp-client-artifacts-")); + + const socket = new WebSocket( + `ws://127.0.0.1:${daemon_port}/mcp?client_artifact_root=${encodeURIComponent( + artifact_root_dir, + )}&client_artifact_root_token=${encodeURIComponent(test_artifact_root_token)}`, + ); + + try { + await wait_for_socket_open(socket); + socket.send( + `${JSON.stringify({ + jsonrpc: "2.0", + id: "init", + method: "initialize", + params: { + protocolVersion: "2024-11-05", + capabilities: {}, + clientInfo: { + name: "artifact-root-test", + version: "0.0.1", + }, + }, + })}\n`, + ); + + const initialize_response = await wait_for_initialize_response(socket); + expect(initialize_response.error).toBeUndefined(); + expect(typeof initialize_response.result?.agent_session_id).toBe("string"); + + socket.send( + `${JSON.stringify({ + jsonrpc: "2.0", + id: "attach", + method: "tools/call", + params: { + name: "attach_to_tab", + arguments: { + tab_id: 101, + }, + }, + })}\n`, + ); + const attach_response = await wait_for_response(socket, "attach"); + expect(attach_response.error).toBeUndefined(); + + socket.send( + `${JSON.stringify({ + jsonrpc: "2.0", + id: "screenshot", + method: "tools/call", + params: { + name: "browser_take_screenshot", + arguments: { + type: "png", + path: "captures/daemon.png", + }, + }, + })}\n`, + ); + const screenshot_response = await wait_for_response(socket, "screenshot"); + expect(screenshot_response.error).toBeUndefined(); + + const expected_path = join(artifact_root_dir, "captures/daemon.png"); + expect(screenshot_response.result?.structuredContent?.path).toBe(expected_path); + expect(readFileSync(expected_path).byteLength).toBeGreaterThan(0); + } finally { + try { + socket.close(); + } catch { + // ignore best-effort close + } + + await ingress.stop(); + await runtime.stop(); + rmSync(artifact_root_dir, { recursive: true, force: true }); + rmSync(daemon_state_dir, { recursive: true, force: true }); + } +}); + +test("daemon ingress rejects relative client artifact roots", async () => { + const { runtime, ingress, daemon_port, daemon_state_dir } = await create_test_ingress(25); + + try { + const response = await fetch( + `http://127.0.0.1:${daemon_port}/mcp?client_artifact_root=relative-root&client_artifact_root_token=${encodeURIComponent( + test_artifact_root_token, + )}`, + ); + expect(response.status).toBe(400); + expect(await response.text()).toContain("absolute path"); + } finally { + await ingress.stop(); + await runtime.stop(); + rmSync(daemon_state_dir, { recursive: true, force: true }); + } +}); + +test("daemon ingress rejects client artifact roots without the daemon-issued root token", async () => { + const { runtime, ingress, daemon_port, daemon_state_dir } = await create_test_ingress(25); + const artifact_root_dir = mkdtempSync(join(tmpdir(), "local-mcp-client-artifacts-")); + + try { + const response = await fetch( + `http://127.0.0.1:${daemon_port}/mcp?client_artifact_root=${encodeURIComponent(artifact_root_dir)}`, + ); + expect(response.status).toBe(400); + expect(await response.text()).toContain("daemon-issued artifact root token"); + } finally { + await ingress.stop(); + await runtime.stop(); + rmSync(artifact_root_dir, { recursive: true, force: true }); + rmSync(daemon_state_dir, { recursive: true, force: true }); + } +}); + +test("daemon ingress rejects missing client artifact roots with descriptive message", async () => { + const { runtime, ingress, daemon_port, daemon_state_dir } = await create_test_ingress(25); + const parent_dir = mkdtempSync(join(tmpdir(), "local-mcp-missing-root-parent-")); + const missing_root = join(parent_dir, "missing-root"); + + try { + const response = await fetch( + `http://127.0.0.1:${daemon_port}/mcp?client_artifact_root=${encodeURIComponent( + missing_root, + )}&client_artifact_root_token=${encodeURIComponent(test_artifact_root_token)}`, + ); + expect(response.status).toBe(400); + expect(await response.text()).toContain(`artifact root must be an existing directory: ${missing_root}`); + } finally { + await ingress.stop(); + await runtime.stop(); + rmSync(parent_dir, { recursive: true, force: true }); + rmSync(daemon_state_dir, { recursive: true, force: true }); + } +}); + +test("daemon ingress rejects client artifact roots on unauthenticated non-loopback ingress", async () => { + const { runtime, ingress, daemon_port, daemon_state_dir } = await create_test_ingress(25, { + daemon_host: "0.0.0.0", + }); + const artifact_root_dir = mkdtempSync(join(tmpdir(), "local-mcp-client-artifacts-")); + + try { + const response = await fetch( + `http://127.0.0.1:${daemon_port}/mcp?client_artifact_root=${encodeURIComponent( + artifact_root_dir, + )}&client_artifact_root_token=${encodeURIComponent(test_artifact_root_token)}`, + ); + expect(response.status).toBe(400); + expect(await response.text()).toContain("client_artifact_root requires loopback daemon ingress or MCP auth"); + } finally { + await ingress.stop(); + await runtime.stop(); + rmSync(artifact_root_dir, { recursive: true, force: true }); + rmSync(daemon_state_dir, { recursive: true, force: true }); + } +}); + +test("daemon ingress defers non-loopback artifact root filesystem checks until initialize auth", async () => { + const { runtime, ingress, daemon_port, daemon_state_dir } = await create_test_ingress(25, { + daemon_host: "0.0.0.0", + auth_token: "secret-token", + }); + const parent_dir = mkdtempSync(join(tmpdir(), "local-mcp-deferred-root-parent-")); + const missing_root = join(parent_dir, "missing-root"); + const socket = new WebSocket( + `ws://127.0.0.1:${daemon_port}/mcp?client_artifact_root=${encodeURIComponent( + missing_root, + )}&client_artifact_root_token=${encodeURIComponent(test_artifact_root_token)}`, + ); + + try { + await wait_for_socket_open(socket); + socket.send( + `${JSON.stringify({ + jsonrpc: "2.0", + id: "init", + method: "initialize", + params: { + protocolVersion: "2024-11-05", + capabilities: {}, + clientInfo: { + name: "deferred-artifact-root-test", + version: "0.0.1", + }, + token: "wrong-token", + }, + })}\n`, + ); + + const initialize_response = await wait_for_initialize_response(socket); + expect(initialize_response.error?.message).toBe("invalid token"); + } finally { + try { + socket.close(); + } catch { + // ignore best-effort close + } + + await ingress.stop(); + await runtime.stop(); + rmSync(parent_dir, { recursive: true, force: true }); + rmSync(daemon_state_dir, { recursive: true, force: true }); + } +}); + test("daemon ingress lets live sockets rebind when their session was closed elsewhere", async () => { const { runtime, ingress, daemon_port, daemon_state_dir } = await create_test_ingress(25); diff --git a/local-mcp-bun/tests/integration/daemon_singleton_proxy.test.ts b/local-mcp-bun/tests/integration/daemon_singleton_proxy.test.ts index ed4474b..642af4f 100644 --- a/local-mcp-bun/tests/integration/daemon_singleton_proxy.test.ts +++ b/local-mcp-bun/tests/integration/daemon_singleton_proxy.test.ts @@ -1,3 +1,4 @@ +// Modified by [KnotFalse] import { afterEach, expect, test } from "bun:test"; import { spawn, type ChildProcessWithoutNullStreams } from "node:child_process"; import { mkdtempSync, rmSync } from "node:fs"; @@ -263,5 +264,5 @@ test("auto mode multiplexes two stdio clients through a single shared daemon", a await wait_for_condition(async () => { const health = await fetch_daemon_health(daemon_port); return typeof health === "undefined"; - }, 15_000, 250); -}); + }, 20_000, 250); +}, 30_000); diff --git a/local-mcp-bun/tests/integration/parity_remaining.test.ts b/local-mcp-bun/tests/integration/parity_remaining.test.ts index 2dcaba0..590c33b 100644 --- a/local-mcp-bun/tests/integration/parity_remaining.test.ts +++ b/local-mcp-bun/tests/integration/parity_remaining.test.ts @@ -1,6 +1,8 @@ +// Modified by [KnotFalse] import { expect, test } from "bun:test"; -import { readFileSync, rmSync } from "node:fs"; +import { existsSync, readFileSync, rmSync, symlinkSync } from "node:fs"; import { join, sep } from "node:path"; +import { resolve_artifact_root } from "../../src/artifacts"; import { tool_error } from "../../src/errors"; import { in_memory_bridge_transport } from "../../src/bridge_transport"; import { local_mcp_runtime } from "../../src/runtime"; @@ -12,6 +14,7 @@ function parse_test_bridge_port(): number { } const test_bridge_port = parse_test_bridge_port(); +const symlink_test = process.platform === "win32" ? test.skip : test; function create_runtime() { return new local_mcp_runtime({ @@ -393,6 +396,81 @@ test("browser_take_screenshot persists image artifacts when path is requested", } }); +test("browser_take_screenshot keeps captured payload when artifact session lookup and recovery race cleanup", async () => { + const runtime = create_runtime(); + const session_id = await open_attached_session(runtime, "screenshot-session-race"); + const original_get_context = runtime.session_registry.get_session_artifact_root_context.bind(runtime.session_registry); + + runtime.session_registry.get_session_artifact_root_context = (lookup_session_id: string) => { + if (lookup_session_id === session_id) { + runtime.session_registry.close_session(session_id); + throw new tool_error("SESSION_NOT_FOUND", `unknown session: ${lookup_session_id}`, false, { + agent_session_id: lookup_session_id, + }); + } + + return original_get_context(lookup_session_id); + }; + + try { + const result = await runtime.tool_router.call_tool(session_id, "browser_take_screenshot", { + type: "png", + path: "capture.png", + }); + + expect(result.saved).toBe(false); + expect(result.path).toBe("capture.png"); + expect(typeof result.data_base64).toBe("string"); + expect((result.data_base64 as string).length).toBeGreaterThan(0); + expect((result.artifact_error as { code?: unknown }).code).toBe("SESSION_NOT_FOUND"); + } finally { + runtime.session_registry.get_session_artifact_root_context = original_get_context; + await runtime.stop(); + } +}); + +test("browser_take_screenshot resolves relative artifact paths per session", async () => { + const runtime = create_runtime(); + const first_output_dir = create_workspace_output_dir("local-mcp-session-a-"); + const second_output_dir = create_workspace_output_dir("local-mcp-session-b-"); + + try { + const first_session_id = runtime.tool_router.open_session( + "screenshot-path-a", + undefined, + resolve_artifact_root(first_output_dir), + ).agent_session_id; + const second_session_id = runtime.tool_router.open_session( + "screenshot-path-b", + undefined, + resolve_artifact_root(second_output_dir), + ).agent_session_id; + await runtime.tool_router.call_tool(first_session_id, "attach_to_tab", { tab_id: 101 }); + await runtime.tool_router.call_tool(second_session_id, "attach_to_tab", { tab_id: 102 }); + + const relative_path = "captures/shared-name.png"; + const first_result = await runtime.tool_router.call_tool(first_session_id, "browser_take_screenshot", { + type: "png", + path: relative_path, + }); + const second_result = await runtime.tool_router.call_tool(second_session_id, "browser_take_screenshot", { + type: "png", + path: relative_path, + }); + + const first_expected_path = join(first_output_dir, relative_path); + const second_expected_path = join(second_output_dir, relative_path); + expect(first_result.path).toBe(first_expected_path); + expect(second_result.path).toBe(second_expected_path); + expect(readFileSync(first_expected_path).byteLength).toBeGreaterThan(0); + expect(readFileSync(second_expected_path).byteLength).toBeGreaterThan(0); + } finally { + rmSync(first_output_dir, { recursive: true, force: true }); + rmSync(second_output_dir, { recursive: true, force: true }); + await runtime.stop(); + } +}); + test("browser_pdf_save persists artifacts when path is requested", async () => { const runtime = create_runtime(); const session_id = await open_attached_session(runtime, "pdf-path"); @@ -453,6 +531,66 @@ test("artifact persistence rejects absolute paths with traversal segments", asyn } }); +symlink_test("artifact persistence rejects parent symlinks before creating escaped directories", async () => { + const runtime = create_runtime(); + const output_dir = create_workspace_output_dir("local-mcp-symlink-root-"); + const outside_dir = create_workspace_output_dir("local-mcp-symlink-outside-"); + const link_path = join(output_dir, "outside-link"); + symlinkSync(outside_dir, link_path, "dir"); + const session_id = runtime.tool_router.open_session( + "artifact-symlink-parent", + undefined, + resolve_artifact_root(output_dir), + ).agent_session_id; + await runtime.tool_router.call_tool(session_id, "attach_to_tab", { tab_id: 101 }); + + try { + await runtime.tool_router.call_tool(session_id, "browser_take_screenshot", { + type: "png", + path: "outside-link/created/capture.png", + }); + throw new Error("expected artifact symlink validation to fail"); + } catch (error) { + expect(error).toBeInstanceOf(tool_error); + expect((error as tool_error).code).toBe("INVALID_ARGUMENT"); + expect((error as tool_error).message).toContain("artifact path cannot traverse a symbolic link"); + expect(existsSync(join(outside_dir, "created"))).toBe(false); + } finally { + rmSync(output_dir, { recursive: true, force: true }); + rmSync(outside_dir, { recursive: true, force: true }); + await runtime.stop(); + } +}); + +symlink_test("artifact persistence revalidates the root before root-level writes", async () => { + const runtime = create_runtime(); + const output_dir = create_workspace_output_dir("local-mcp-root-swap-"); + const outside_dir = create_workspace_output_dir("local-mcp-root-swap-outside-"); + const root_context = resolve_artifact_root(output_dir); + const session_id = runtime.tool_router.open_session("artifact-root-swap", undefined, root_context).agent_session_id; + await runtime.tool_router.call_tool(session_id, "attach_to_tab", { tab_id: 101 }); + + rmSync(output_dir, { recursive: true, force: true }); + symlinkSync(outside_dir, output_dir, "dir"); + + try { + await runtime.tool_router.call_tool(session_id, "browser_take_screenshot", { + type: "png", + path: "root-level-capture.png", + }); + throw new Error("expected artifact root revalidation to fail"); + } catch (error) { + expect(error).toBeInstanceOf(tool_error); + expect((error as tool_error).code).toBe("INVALID_ARGUMENT"); + expect((error as tool_error).message).toContain("artifact path must stay within the current workspace"); + expect(existsSync(join(outside_dir, "root-level-capture.png"))).toBe(false); + } finally { + rmSync(output_dir, { recursive: true, force: true }); + rmSync(outside_dir, { recursive: true, force: true }); + await runtime.stop(); + } +}); + test("browser extension management tools return parity metadata", async () => { const runtime = create_runtime(); const session_id = await open_attached_session(runtime, "extensions"); diff --git a/local-mcp-bun/tests/unit/artifacts.test.ts b/local-mcp-bun/tests/unit/artifacts.test.ts new file mode 100644 index 0000000..e8844c5 --- /dev/null +++ b/local-mcp-bun/tests/unit/artifacts.test.ts @@ -0,0 +1,192 @@ +// Modified by [KnotFalse] +import { expect, test } from "bun:test"; +import { + existsSync, + mkdirSync, + mkdtempSync, + readFileSync, + realpathSync, + rmSync, + symlinkSync, + writeFileSync, +} from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { tool_error } from "../../src/errors"; +import { + prepare_artifact_destination, + resolve_artifact_path, + resolve_artifact_root, + resolve_default_artifact_root, + write_artifact_file, +} from "../../src/artifacts"; + +const deleted_cwd_test = process.platform === "win32" ? test.skip : test; +const symlink_test = process.platform === "win32" ? test.skip : test; + +deleted_cwd_test("resolve_default_artifact_root falls back when cwd no longer exists", () => { + const original_cwd = process.cwd(); + const deleted_cwd = mkdtempSync(join(tmpdir(), "local-mcp-deleted-cwd-")); + + try { + process.chdir(deleted_cwd); + rmSync(deleted_cwd, { recursive: true, force: true }); + + const artifact_root_context = resolve_default_artifact_root(); + expect(artifact_root_context.artifact_root_real).toBe(realpathSync(tmpdir())); + } finally { + process.chdir(original_cwd); + rmSync(deleted_cwd, { recursive: true, force: true }); + } +}); + +test("resolve_artifact_path allows child names that start with two dots", () => { + const artifact_root = mkdtempSync(join(tmpdir(), "local-mcp-dot-prefix-root-")); + + try { + const root_context = resolve_artifact_root(artifact_root); + expect(resolve_artifact_path(root_context, "..cache/report.png")).toBe(join(artifact_root, "..cache/report.png")); + } finally { + rmSync(artifact_root, { recursive: true, force: true }); + } +}); + +test("resolve_artifact_root normalizes non-directory stat failures", () => { + const artifact_root = mkdtempSync(join(tmpdir(), "local-mcp-file-artifact-root-")); + const file_path = join(artifact_root, "not-a-directory"); + + try { + writeFileSync(file_path, ""); + expect(() => resolve_artifact_root(file_path)).toThrow(`artifact root must be an existing directory: ${file_path}`); + } finally { + rmSync(artifact_root, { recursive: true, force: true }); + } +}); + +test("prepare_artifact_destination returns a tool error when the artifact root is deleted", () => { + const artifact_root = mkdtempSync(join(tmpdir(), "local-mcp-deleted-artifact-root-")); + const root_context = resolve_artifact_root(artifact_root); + const absolute_path = resolve_artifact_path(root_context, "capture.png"); + + rmSync(artifact_root, { recursive: true, force: true }); + + try { + prepare_artifact_destination(root_context, absolute_path, "capture.png"); + throw new Error("expected deleted artifact root validation to fail"); + } catch (error) { + expect(error).toBeInstanceOf(tool_error); + expect((error as tool_error).code).toBe("INVALID_ARGUMENT"); + expect((error as tool_error).message).toContain("artifact parent path must be a directory"); + } finally { + rmSync(artifact_root, { recursive: true, force: true }); + } +}); + +symlink_test("write_artifact_file rejects destination symlinks", async () => { + const artifact_root = mkdtempSync(join(tmpdir(), "local-mcp-symlink-target-root-")); + const outside_root = mkdtempSync(join(tmpdir(), "local-mcp-symlink-target-outside-")); + const outside_file = join(outside_root, "target.png"); + const link_path = join(artifact_root, "capture.png"); + + try { + writeFileSync(outside_file, "original"); + symlinkSync(outside_file, link_path, "file"); + const root_context = resolve_artifact_root(artifact_root); + + try { + await write_artifact_file(root_context, link_path, "capture.png", new Uint8Array([1, 2, 3])); + throw new Error("expected destination symlink validation to fail"); + } catch (error) { + expect(error).toBeInstanceOf(tool_error); + expect((error as tool_error).code).toBe("INVALID_ARGUMENT"); + expect((error as tool_error).message).toContain("artifact path cannot target a symbolic link"); + expect(readFileSync(outside_file, "utf8")).toBe("original"); + } + } finally { + rmSync(artifact_root, { recursive: true, force: true }); + rmSync(outside_root, { recursive: true, force: true }); + } +}); + +symlink_test("write_artifact_file rejects symlink parents before nested directory creation", async () => { + const artifact_root = mkdtempSync(join(tmpdir(), "local-mcp-symlink-parent-root-")); + const outside_root = mkdtempSync(join(tmpdir(), "local-mcp-symlink-parent-outside-")); + const link_path = join(artifact_root, "nested"); + const outside_child = join(outside_root, "child"); + + try { + symlinkSync(outside_root, link_path, "dir"); + const root_context = resolve_artifact_root(artifact_root); + const artifact_path = join(link_path, "child", "capture.png"); + + try { + await write_artifact_file(root_context, artifact_path, "nested/child/capture.png", new Uint8Array([1, 2, 3])); + throw new Error("expected parent symlink validation to fail"); + } catch (error) { + expect(error).toBeInstanceOf(tool_error); + expect((error as tool_error).code).toBe("INVALID_ARGUMENT"); + expect((error as tool_error).message).toContain("artifact path cannot traverse a symbolic link"); + expect(existsSync(outside_child)).toBe(false); + } + } finally { + rmSync(artifact_root, { recursive: true, force: true }); + rmSync(outside_root, { recursive: true, force: true }); + } +}); + +symlink_test("write_artifact_file allows nested artifacts under a symlinked root", async () => { + const artifact_root_target = mkdtempSync(join(tmpdir(), "local-mcp-symlink-root-target-")); + const link_parent = mkdtempSync(join(tmpdir(), "local-mcp-symlink-root-parent-")); + const artifact_root_link = join(link_parent, "workspace"); + const artifact_path = join(artifact_root_link, "captures", "capture.png"); + const target_path = join(artifact_root_target, "captures", "capture.png"); + + try { + symlinkSync(artifact_root_target, artifact_root_link, "dir"); + const root_context = resolve_artifact_root(artifact_root_link); + + await write_artifact_file(root_context, artifact_path, "captures/capture.png", new Uint8Array([1, 2, 3])); + + expect(Array.from(readFileSync(target_path))).toEqual([1, 2, 3]); + } finally { + rmSync(link_parent, { recursive: true, force: true }); + rmSync(artifact_root_target, { recursive: true, force: true }); + } +}); + +test("write_artifact_file rejects directory targets", async () => { + const artifact_root = mkdtempSync(join(tmpdir(), "local-mcp-directory-target-root-")); + const directory_target = join(artifact_root, "capture.png"); + + try { + mkdirSync(directory_target); + const root_context = resolve_artifact_root(artifact_root); + + try { + await write_artifact_file(root_context, directory_target, "capture.png", new Uint8Array([1, 2, 3])); + throw new Error("expected directory target validation to fail"); + } catch (error) { + expect(error).toBeInstanceOf(tool_error); + expect((error as tool_error).code).toBe("INVALID_ARGUMENT"); + expect((error as tool_error).message).toContain("artifact path cannot target a directory"); + } + } finally { + rmSync(artifact_root, { recursive: true, force: true }); + } +}); + +test("write_artifact_file replaces regular destinations through final rename", async () => { + const artifact_root = mkdtempSync(join(tmpdir(), "local-mcp-overwrite-root-")); + const output_path = join(artifact_root, "capture.png"); + + try { + writeFileSync(output_path, "old"); + const root_context = resolve_artifact_root(artifact_root); + + await write_artifact_file(root_context, output_path, "capture.png", new Uint8Array([1, 2, 3])); + + expect(Array.from(readFileSync(output_path))).toEqual([1, 2, 3]); + } finally { + rmSync(artifact_root, { recursive: true, force: true }); + } +}); diff --git a/local-mcp-bun/tests/unit/daemon_spawn_target.test.ts b/local-mcp-bun/tests/unit/daemon_spawn_target.test.ts index a70fa59..95b5473 100644 --- a/local-mcp-bun/tests/unit/daemon_spawn_target.test.ts +++ b/local-mcp-bun/tests/unit/daemon_spawn_target.test.ts @@ -1,5 +1,15 @@ +// Modified by [KnotFalse] import { describe, expect, test } from "bun:test"; -import { resolve_daemon_spawn_target } from "../../src/daemon_controller"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { + wait_for_proxy_daemon_state, + resolve_daemon_spawn_target, + resolve_proxy_auth_token_for_log, + resolve_proxy_artifact_root_attachment, + should_attach_client_artifact_root_to_daemon, +} from "../../src/daemon_controller"; describe("resolve_daemon_spawn_target", () => { test("resolves bun source execution to bun + script entry", () => { @@ -54,3 +64,320 @@ describe("resolve_daemon_spawn_target", () => { }); }); }); + +describe("should_attach_client_artifact_root_to_daemon", () => { + test("attaches client artifact roots automatically for loopback daemon bind hosts", () => { + expect(should_attach_client_artifact_root_to_daemon({ daemon_host: "127.0.0.1" }, {})).toBe(true); + expect(should_attach_client_artifact_root_to_daemon({ daemon_host: "localhost" }, {})).toBe(true); + }); + + test("does not attach client artifact roots automatically for non-loopback daemon bind hosts", () => { + expect(should_attach_client_artifact_root_to_daemon({ daemon_host: "0.0.0.0" }, {})).toBe(false); + }); + + test("supports explicit non-loopback artifact-root attachment opt-in", () => { + expect( + should_attach_client_artifact_root_to_daemon( + { daemon_host: "0.0.0.0" }, + { MCP_ATTACH_CLIENT_ARTIFACT_ROOT: "true" }, + ), + ).toBe(true); + expect( + should_attach_client_artifact_root_to_daemon( + { daemon_host: "0.0.0.0" }, + { MCP_ATTACH_CLIENT_ARTIFACT_ROOT: "1" }, + ), + ).toBe(true); + }); + + test("supports explicit loopback artifact-root attachment opt-out", () => { + expect( + should_attach_client_artifact_root_to_daemon( + { daemon_host: "127.0.0.1" }, + { MCP_ATTACH_CLIENT_ARTIFACT_ROOT: "false" }, + ), + ).toBe(false); + }); +}); + +describe("resolve_proxy_artifact_root_attachment", () => { + const daemon_health = { + started_at: "2026-04-24T00:00:00.000Z", + pid: 1234, + daemon_port: 37778, + bridge_port: 37777, + }; + + const daemon_target = { + daemon_host: "127.0.0.1", + }; + + const daemon_state = { + daemon_host: "127.0.0.1", + started_at: "2026-04-24T00:00:00.000Z", + pid: 1234, + daemon_port: 37778, + bridge_port: 37777, + artifact_root_token: "artifact-token", + }; + + test("attaches artifact-root token when loopback daemon state provides it", () => { + expect(resolve_proxy_artifact_root_attachment(daemon_target, daemon_health, {}, daemon_state)).toEqual({ + attach_client_artifact_root: true, + artifact_root_token: "artifact-token", + missing_artifact_root_token: false, + }); + }); + + test("reports a missing token when loopback attachment lacks a daemon token", () => { + expect(resolve_proxy_artifact_root_attachment(daemon_target, daemon_health, {}, undefined)).toEqual({ + attach_client_artifact_root: false, + missing_artifact_root_token: true, + }); + }); + + test("reports a missing token when daemon state does not match health", () => { + expect( + resolve_proxy_artifact_root_attachment(daemon_target, daemon_health, {}, { + ...daemon_state, + pid: 4321, + }), + ).toEqual({ + attach_client_artifact_root: false, + missing_artifact_root_token: true, + }); + }); + + test("reports a missing token when daemon state started_at does not match health", () => { + expect( + resolve_proxy_artifact_root_attachment(daemon_target, daemon_health, {}, { + ...daemon_state, + started_at: "2026-04-24T00:01:00.000Z", + }), + ).toEqual({ + attach_client_artifact_root: false, + missing_artifact_root_token: true, + }); + }); + + test("accepts equivalent loopback aliases when daemon state host differs from the dialed host", () => { + expect( + resolve_proxy_artifact_root_attachment(daemon_target, daemon_health, {}, { + ...daemon_state, + daemon_host: "localhost", + }), + ).toEqual({ + attach_client_artifact_root: true, + artifact_root_token: "artifact-token", + missing_artifact_root_token: false, + }); + }); + + test("reports a missing token when daemon state host is not an equivalent loopback host", () => { + expect( + resolve_proxy_artifact_root_attachment(daemon_target, daemon_health, {}, { + ...daemon_state, + daemon_host: "0.0.0.0", + }), + ).toEqual({ + attach_client_artifact_root: false, + missing_artifact_root_token: true, + }); + }); + + test("does not report a missing token when artifact-root attachment is disabled", () => { + expect( + resolve_proxy_artifact_root_attachment( + daemon_target, + daemon_health, + { MCP_ATTACH_CLIENT_ARTIFACT_ROOT: "0" }, + undefined, + ), + ).toEqual({ + attach_client_artifact_root: false, + missing_artifact_root_token: false, + }); + }); +}); + +describe("resolve_proxy_auth_token_for_log", () => { + const daemon_health = { + started_at: "2026-04-24T00:00:00.000Z", + pid: 1234, + daemon_port: 37778, + bridge_port: 37777, + auth_enabled: false, + }; + + const daemon_target = { + daemon_host: "127.0.0.1", + }; + + const daemon_state = { + daemon_host: "127.0.0.1", + started_at: "2026-04-24T00:00:00.000Z", + pid: 1234, + daemon_port: 37778, + bridge_port: 37777, + auth_token: "auto-token", + }; + + test("returns auto auth token only when daemon state matches the dialed daemon", () => { + expect(resolve_proxy_auth_token_for_log(daemon_target, daemon_health, { MCP_AUTH_TOKEN: "auto" }, daemon_state)).toBe( + "auto-token", + ); + expect( + resolve_proxy_auth_token_for_log(daemon_target, daemon_health, { MCP_AUTH_TOKEN: "auto" }, { + ...daemon_state, + daemon_port: 47778, + }), + ).toBeUndefined(); + expect( + resolve_proxy_auth_token_for_log(daemon_target, daemon_health, { MCP_AUTH_TOKEN: "auto" }, { + ...daemon_state, + started_at: "2026-04-24T00:01:00.000Z", + }), + ).toBeUndefined(); + }); + + test("does not return an auth token when auto auth was not requested", () => { + expect(resolve_proxy_auth_token_for_log(daemon_target, daemon_health, {}, daemon_state)).toBeUndefined(); + }); +}); + +describe("wait_for_proxy_daemon_state", () => { + const daemon_health = { + started_at: "2026-04-24T00:00:00.000Z", + pid: 1234, + daemon_port: 37778, + bridge_port: 37777, + auth_enabled: false, + }; + + test("waits for matching daemon state before returning a loopback artifact token", async () => { + const temp_dir = mkdtempSync(join(tmpdir(), "daemon-state-race-")); + const state_path = join(temp_dir, "daemon-state.json"); + let timeout_id: ReturnType | undefined; + + try { + timeout_id = setTimeout(() => { + writeFileSync( + state_path, + `${JSON.stringify({ + service: "local-mcp-daemon", + pid: daemon_health.pid, + started_at: daemon_health.started_at, + updated_at: daemon_health.started_at, + daemon_host: "127.0.0.1", + daemon_port: daemon_health.daemon_port, + bridge_host: "127.0.0.1", + bridge_port: daemon_health.bridge_port, + auth_enabled: false, + auth_auto: false, + artifact_root_token: "artifact-token", + })}\n`, + ); + }, 20); + + const daemon_state = await wait_for_proxy_daemon_state( + { + daemon_host: "127.0.0.1", + daemon_connect_timeout_ms: 1_000, + daemon_state_path: state_path, + env: {}, + }, + daemon_health, + ); + + expect(daemon_state?.artifact_root_token).toBe("artifact-token"); + } finally { + if (timeout_id) { + clearTimeout(timeout_id); + } + rmSync(temp_dir, { recursive: true, force: true }); + } + }, 2_000); + + test("waits for matching daemon state when auto-auth is requested by an auth-enabled daemon", async () => { + const temp_dir = mkdtempSync(join(tmpdir(), "daemon-state-auto-auth-race-")); + const state_path = join(temp_dir, "daemon-state.json"); + let timeout_id: ReturnType | undefined; + + try { + timeout_id = setTimeout(() => { + writeFileSync( + state_path, + `${JSON.stringify({ + service: "local-mcp-daemon", + pid: daemon_health.pid, + started_at: daemon_health.started_at, + updated_at: daemon_health.started_at, + daemon_host: "127.0.0.1", + daemon_port: daemon_health.daemon_port, + bridge_host: "127.0.0.1", + bridge_port: daemon_health.bridge_port, + auth_enabled: true, + auth_auto: true, + auth_token: "auto-token", + })}\n`, + ); + }, 20); + + const daemon_state = await wait_for_proxy_daemon_state( + { + daemon_host: "127.0.0.1", + daemon_connect_timeout_ms: 1_000, + daemon_state_path: state_path, + env: { + MCP_AUTH_TOKEN: "auto", + MCP_ATTACH_CLIENT_ARTIFACT_ROOT: "0", + }, + }, + { + ...daemon_health, + auth_enabled: true, + }, + ); + + expect(daemon_state?.auth_token).toBe("auto-token"); + } finally { + if (timeout_id) { + clearTimeout(timeout_id); + } + rmSync(temp_dir, { recursive: true, force: true }); + } + }, 2_000); + + test("does not wait solely for auto-auth daemon state when the daemon is not auth-enabled", async () => { + const temp_dir = mkdtempSync(join(tmpdir(), "daemon-state-auto-auth-")); + const state_path = join(temp_dir, "daemon-state.json"); + let timeout_id: ReturnType | undefined; + + try { + const result = await Promise.race([ + wait_for_proxy_daemon_state( + { + daemon_host: "127.0.0.1", + daemon_connect_timeout_ms: 10_000, + daemon_state_path: state_path, + env: { + MCP_AUTH_TOKEN: "auto", + MCP_ATTACH_CLIENT_ARTIFACT_ROOT: "0", + }, + }, + daemon_health, + ).then(() => "returned"), + new Promise<"waited">((resolve_promise) => { + timeout_id = setTimeout(() => resolve_promise("waited"), 50); + }), + ]); + + expect(result).toBe("returned"); + } finally { + if (timeout_id) { + clearTimeout(timeout_id); + } + rmSync(temp_dir, { recursive: true, force: true }); + } + }, 1_000); +}); diff --git a/local-mcp-bun/tests/unit/mcp_protocol_session.test.ts b/local-mcp-bun/tests/unit/mcp_protocol_session.test.ts index 0a2b153..fc023ad 100644 --- a/local-mcp-bun/tests/unit/mcp_protocol_session.test.ts +++ b/local-mcp-bun/tests/unit/mcp_protocol_session.test.ts @@ -1,3 +1,4 @@ +// Modified by [KnotFalse] import { expect, test } from "bun:test"; import { mcp_protocol_session } from "../../src/mcp_protocol_session"; import { local_mcp_runtime } from "../../src/runtime"; @@ -80,6 +81,141 @@ test("mcp_protocol_session rebinds stale initialized-session notifications after } }); +test("mcp_protocol_session reports invalid deferred artifact roots as client errors", async () => { + const runtime = new local_mcp_runtime({ + bridge_mode: "in_memory", + bridge_host: "127.0.0.1", + bridge_port: 37777, + }); + + const responses: json_rpc_response[] = []; + const protocol_session = new mcp_protocol_session({ + runtime, + write_response: (response) => { + responses.push(response); + }, + artifact_root_context: () => { + throw new Error("artifact root must be an existing directory: /missing-artifact-root"); + }, + }); + + try { + await protocol_session.handle_line(build_initialize_request("init", "bad-artifact-root")); + + const response = responses.find((candidate_response) => candidate_response.id === "init"); + const error_data = response?.error?.data as + | { + code?: string; + details?: { + cause?: string; + }; + } + | undefined; + + expect(response?.error?.code).toBe(-32001); + expect(error_data?.code).toBe("INVALID_ARGUMENT"); + expect(error_data?.details?.cause).toContain("artifact root must be an existing directory"); + expect(runtime.session_registry.list_active_sessions()).toEqual([]); + } finally { + await runtime.stop(); + } +}); + +test("mcp_protocol_session clones object artifact roots before opening sessions", async () => { + const runtime = new local_mcp_runtime({ + bridge_mode: "in_memory", + bridge_host: "127.0.0.1", + bridge_port: 37777, + }); + + const artifact_root_context = { + artifact_root: "/workspace-a", + artifact_root_real: "/workspace-a", + }; + const responses: json_rpc_response[] = []; + const protocol_session = new mcp_protocol_session({ + runtime, + write_response: (response) => { + responses.push(response); + }, + artifact_root_context, + }); + + artifact_root_context.artifact_root = "/mutated-workspace"; + + try { + await protocol_session.handle_line(build_initialize_request("init", "cloned-artifact-root")); + + const response = responses.find((candidate_response) => candidate_response.id === "init"); + const agent_session_id = response?.result?.agent_session_id; + expect(typeof agent_session_id).toBe("string"); + + expect(runtime.session_registry.get_session_artifact_root_context(agent_session_id as string)).toEqual({ + artifact_root: "/workspace-a", + artifact_root_real: "/workspace-a", + }); + } finally { + await runtime.stop(); + } +}); + +test("mcp_protocol_session pins deferred artifact roots across implicit rebinds", async () => { + const runtime = new local_mcp_runtime({ + bridge_mode: "in_memory", + bridge_host: "127.0.0.1", + bridge_port: 37777, + }); + + const responses: json_rpc_response[] = []; + let resolve_count = 0; + const protocol_session = new mcp_protocol_session({ + runtime, + write_response: (response) => { + responses.push(response); + }, + artifact_root_context: () => { + resolve_count += 1; + return resolve_count === 1 + ? { + artifact_root: "/workspace-a", + artifact_root_real: "/workspace-a", + } + : { + artifact_root: "/workspace-b", + artifact_root_real: "/workspace-b", + }; + }, + }); + + try { + await protocol_session.handle_line(build_initialize_request("init", "pinned-artifact-root")); + + const initial_response = responses.find((candidate_response) => candidate_response.id === "init"); + const initial_agent_session_id = initial_response?.result?.agent_session_id; + expect(typeof initial_agent_session_id).toBe("string"); + + await runtime.tool_router.close_session(initial_agent_session_id as string); + await protocol_session.handle_line( + JSON.stringify({ + jsonrpc: "2.0", + id: "tools", + method: "tools/list", + }), + ); + + const rebound_agent_session_id = runtime.session_registry.list_active_sessions()[0]; + expect(typeof rebound_agent_session_id).toBe("string"); + expect(rebound_agent_session_id).not.toBe(initial_agent_session_id); + expect(resolve_count).toBe(1); + expect(runtime.session_registry.get_session_artifact_root_context(rebound_agent_session_id as string)).toEqual({ + artifact_root: "/workspace-a", + artifact_root_real: "/workspace-a", + }); + } finally { + await runtime.stop(); + } +}); + test("mcp_protocol_session does not throw when notification rebind fails", async () => { const runtime = new local_mcp_runtime({ bridge_mode: "in_memory", diff --git a/local-mcp-bun/tests/unit/session_registry.test.ts b/local-mcp-bun/tests/unit/session_registry.test.ts index 19a2e6e..81b9105 100644 --- a/local-mcp-bun/tests/unit/session_registry.test.ts +++ b/local-mcp-bun/tests/unit/session_registry.test.ts @@ -1,3 +1,4 @@ +// Modified by [KnotFalse] import { expect, test } from "bun:test"; import { session_registry } from "../../src/session_registry"; import { tool_error } from "../../src/errors"; @@ -49,6 +50,34 @@ test("session_registry emits stable snapshots with owned tabs", () => { expect(alpha_snapshot?.owned_tab_ids).toEqual([101, 202]); expect(beta_snapshot?.owned_tab_ids).toEqual([303]); + expect("artifact_root" in (alpha as unknown as Record)).toBe(false); + expect("artifact_root_real" in (alpha as unknown as Record)).toBe(false); + expect("artifact_root" in (alpha_snapshot as unknown as Record)).toBe(false); + expect("artifact_root_real" in (alpha_snapshot as unknown as Record)).toBe(false); +}); + +test("session_registry stores and returns artifact root context copies", () => { + const registry = new session_registry(); + const artifact_root_context = { + artifact_root: "/workspace-a", + artifact_root_real: "/workspace-a", + }; + const session = registry.create_session("artifact-context", "none", artifact_root_context); + + artifact_root_context.artifact_root = "/mutated-workspace"; + + const stored_context = registry.get_session_artifact_root_context(session.agent_session_id); + expect(stored_context).toEqual({ + artifact_root: "/workspace-a", + artifact_root_real: "/workspace-a", + }); + + stored_context.artifact_root = "/returned-mutation"; + + expect(registry.get_session_artifact_root_context(session.agent_session_id)).toEqual({ + artifact_root: "/workspace-a", + artifact_root_real: "/workspace-a", + }); }); test("session_registry lists stale sessions from last_seen_at", () => { diff --git a/local-mcp-bun/tests/unit/stdio_proxy_server.test.ts b/local-mcp-bun/tests/unit/stdio_proxy_server.test.ts new file mode 100644 index 0000000..7e9e047 --- /dev/null +++ b/local-mcp-bun/tests/unit/stdio_proxy_server.test.ts @@ -0,0 +1,197 @@ +// Modified by [KnotFalse] +import { expect, test } from "bun:test"; +import { resolve } from "node:path"; +import { + build_daemon_url_with_client_artifact_root, + redact_daemon_url_for_logs, + stdio_proxy_server, +} from "../../src/stdio_proxy_server"; + +test("daemon proxy URL includes encoded client artifact root", () => { + const artifact_root = resolve("client workspace"); + const daemon_url = build_daemon_url_with_client_artifact_root( + "ws://127.0.0.1:37778/mcp", + artifact_root, + undefined, + "artifact-token", + ); + const parsed_url = new URL(daemon_url); + + expect(parsed_url.pathname).toBe("/mcp"); + expect(parsed_url.searchParams.get("client_artifact_root")).toBe(artifact_root); + expect(parsed_url.searchParams.get("client_artifact_root_token")).toBe("artifact-token"); + expect(daemon_url).toContain("client_artifact_root="); +}); + +test("daemon proxy URL preserves existing query params", () => { + const artifact_root = resolve("client-workspace"); + const daemon_url = build_daemon_url_with_client_artifact_root( + "ws://127.0.0.1:37778/mcp?existing=1", + artifact_root, + undefined, + "artifact-token", + ); + const parsed_url = new URL(daemon_url); + + expect(parsed_url.searchParams.get("existing")).toBe("1"); + expect(parsed_url.searchParams.get("client_artifact_root")).toBe(artifact_root); + expect(parsed_url.searchParams.get("client_artifact_root_token")).toBe("artifact-token"); +}); + +test("daemon proxy URL preserves explicit client artifact root and attaches token", () => { + const explicit_root = resolve("explicit-workspace"); + const daemon_url = build_daemon_url_with_client_artifact_root( + `ws://127.0.0.1:37778/mcp?client_artifact_root=${encodeURIComponent(explicit_root)}`, + resolve("ignored-workspace"), + undefined, + "artifact-token", + ); + const parsed_url = new URL(daemon_url); + + expect(parsed_url.searchParams.get("client_artifact_root")).toBe(explicit_root); + expect(parsed_url.searchParams.get("client_artifact_root_token")).toBe("artifact-token"); +}); + +test("daemon proxy URL overwrites stale artifact-root tokens when attaching", () => { + const explicit_root = resolve("explicit-workspace"); + const daemon_url = build_daemon_url_with_client_artifact_root( + `ws://127.0.0.1:37778/mcp?client_artifact_root=${encodeURIComponent( + explicit_root, + )}&client_artifact_root_token=stale-token`, + resolve("ignored-workspace"), + true, + "fresh-token", + ); + const parsed_url = new URL(daemon_url); + + expect(parsed_url.searchParams.get("client_artifact_root")).toBe(explicit_root); + expect(parsed_url.searchParams.get("client_artifact_root_token")).toBe("fresh-token"); +}); + +test("daemon proxy URL does not attach client artifact root to remote daemon by default", () => { + const artifact_root = resolve("client-workspace"); + const daemon_url = build_daemon_url_with_client_artifact_root( + "ws://example.com:37778/mcp", + artifact_root, + undefined, + "artifact-token", + ); + const parsed_url = new URL(daemon_url); + + expect(parsed_url.searchParams.get("client_artifact_root")).toBeNull(); + expect(parsed_url.searchParams.get("client_artifact_root_token")).toBeNull(); +}); + +test("daemon proxy URL strips explicit artifact-root params when attachment is disabled", () => { + const explicit_root = resolve("explicit-workspace"); + const daemon_url = build_daemon_url_with_client_artifact_root( + `ws://example.com:37778/mcp?client_artifact_root=${encodeURIComponent( + explicit_root, + )}&client_artifact_root_token=stale-token&existing=1`, + resolve("ignored-workspace"), + false, + "artifact-token", + ); + const parsed_url = new URL(daemon_url); + + expect(parsed_url.searchParams.get("existing")).toBe("1"); + expect(parsed_url.searchParams.get("client_artifact_root")).toBeNull(); + expect(parsed_url.searchParams.get("client_artifact_root_token")).toBeNull(); +}); + +test("daemon proxy URL attaches client artifact root to remote daemon when explicitly allowed", () => { + const artifact_root = resolve("client-workspace"); + const daemon_url = build_daemon_url_with_client_artifact_root( + "ws://example.com:37778/mcp", + artifact_root, + true, + "artifact-token", + ); + const parsed_url = new URL(daemon_url); + + expect(parsed_url.searchParams.get("client_artifact_root")).toBe(artifact_root); + expect(parsed_url.searchParams.get("client_artifact_root_token")).toBe("artifact-token"); +}); + +test("daemon proxy URL normalizes artifact roots before attaching them", () => { + const daemon_url = build_daemon_url_with_client_artifact_root( + "ws://127.0.0.1:37778/mcp", + "relative-workspace", + undefined, + "artifact-token", + ); + const parsed_url = new URL(daemon_url); + + expect(parsed_url.searchParams.get("client_artifact_root")).toBe(resolve("relative-workspace")); +}); + +test("daemon proxy URL skips artifact-root attachment when cwd is unavailable", () => { + const original_cwd_descriptor = Object.getOwnPropertyDescriptor(process, "cwd"); + Object.defineProperty(process, "cwd", { + configurable: true, + value: () => { + throw new Error("cwd unavailable"); + }, + }); + + try { + const daemon_url = build_daemon_url_with_client_artifact_root( + "ws://127.0.0.1:37778/mcp", + undefined, + true, + "artifact-token", + ); + const parsed_url = new URL(daemon_url); + + expect(parsed_url.searchParams.get("client_artifact_root")).toBeNull(); + expect(parsed_url.searchParams.get("client_artifact_root_token")).toBeNull(); + } finally { + if (original_cwd_descriptor) { + Object.defineProperty(process, "cwd", original_cwd_descriptor); + } + } +}); + +test("stdio proxy preserves explicit artifact root when cwd is unavailable", () => { + const explicit_root = resolve("explicit-workspace"); + const original_cwd_descriptor = Object.getOwnPropertyDescriptor(process, "cwd"); + Object.defineProperty(process, "cwd", { + configurable: true, + value: () => { + throw new Error("cwd unavailable"); + }, + }); + + try { + const proxy = new stdio_proxy_server({ + daemon_url: `ws://127.0.0.1:37778/mcp?client_artifact_root=${encodeURIComponent(explicit_root)}`, + connect_timeout_ms: 1, + attach_client_artifact_root: true, + artifact_root_token: "artifact-token", + }); + const daemon_url = (proxy as unknown as { daemon_url: string }).daemon_url; + const parsed_url = new URL(daemon_url); + + expect(parsed_url.searchParams.get("client_artifact_root")).toBe(explicit_root); + expect(parsed_url.searchParams.get("client_artifact_root_token")).toBe("artifact-token"); + } finally { + if (original_cwd_descriptor) { + Object.defineProperty(process, "cwd", original_cwd_descriptor); + } + } +}); + +test("daemon proxy URL redaction removes artifact-root connection metadata", () => { + const daemon_url = build_daemon_url_with_client_artifact_root( + "ws://127.0.0.1:37778/mcp", + resolve("client-workspace"), + undefined, + "artifact-token", + ); + const redacted_url = redact_daemon_url_for_logs(daemon_url); + + expect(redacted_url).toContain("client_artifact_root=%3Credacted%3E"); + expect(redacted_url).toContain("client_artifact_root_token=%3Credacted%3E"); + expect(redacted_url).not.toContain("artifact-token"); + expect(redacted_url).not.toContain(encodeURIComponent(resolve("client-workspace"))); +}); diff --git a/specs/local-multiplexed-browser-mcp-changelog.md b/specs/local-multiplexed-browser-mcp-changelog.md index 0945b37..df869e1 100644 --- a/specs/local-multiplexed-browser-mcp-changelog.md +++ b/specs/local-multiplexed-browser-mcp-changelog.md @@ -1,5 +1,62 @@ # Local Multiplexed Browser MCP Spec Changelog +## 2026-04-25 + +### Per-Client Artifact Roots Review Follow-Up + +- KnotFalse updated `local-mcp-bun/README.md` together with the per-client artifact-root implementation notes so setup guidance matches the daemon token, attachment, and redaction behavior; compatibility remains backward-compatible for default loopback clients, with explicit opt-out/opt-in controls documented for tunneled or remote daemon deployments. + +## 2026-04-24 + +### Per-Client Artifact Roots + +- Shared-daemon stdio proxies now pass their launch cwd to daemon ingress as connection metadata. +- MCP sessions now store an internal artifact root, so screenshot/PDF relative `path` writes are scoped to the calling client session instead of the shared daemon cwd. +- Artifact path validation still rejects workspace escapes and symlink file targets. +- Artifact-root metadata is attached automatically only for loopback daemon URLs; non-loopback attachment requires `MCP_ATTACH_CLIENT_ARTIFACT_ROOT=1`/`true`, and unauthenticated non-loopback daemon ingress rejects it. +- Loopback artifact-root attachment can be disabled with `MCP_ATTACH_CLIENT_ARTIFACT_ROOT=0`/`false` for tunnel or remote-proxy deployments. +- Missing artifact roots now return a descriptive validation error instead of leaking raw `realpath`/`ENOENT` text. +- Parent-directory creation now validates existing path segments before creating missing directories so symlinked ancestors cannot create directories outside the artifact root before rejection. +- Parent-directory creation tolerates concurrent directory creation by treating `EEXIST` as a race to revalidate, not as a persistence failure. +- Artifact writes now revalidate that the session root path still resolves to its original canonical root before root-level files are persisted, preventing post-session root replacement from redirecting artifacts. +- Daemon ingress now requires a daemon-issued artifact-root token before accepting `client_artifact_root` connection metadata, and stdio proxies read that capability from owner-scoped daemon state. +- Proxy connection errors redact artifact-root metadata, and proxies fall back to daemon-cwd artifact writes only when per-client attachment is disabled or not expected. +- Proxies now use daemon-state artifact-root tokens only when the state record matches daemon health by start time, pid, and ports, avoiding stale-token startup failures. +- Default daemon artifact roots now fall back to the OS temp directory if the daemon launch cwd has been deleted, so non-artifact MCP clients can still connect. +- Stdio proxies now skip client artifact-root attachment when their own cwd is unavailable, preventing stale client cwd paths from blocking MCP traffic. +- Artifact root revalidation now reports a normal artifact validation error if the root is deleted before a screenshot/PDF write. +- Proxy artifact-root tokens are now stripped whenever client artifact-root attachment is disabled, and daemon-state token reuse requires the dialed daemon host to match. +- Session artifact roots now live in registry-private storage and are excluded from the exported `agent_session` shape and connection snapshots. +- Proxies now fail startup instead of silently falling back to daemon-cwd artifacts when per-client attachment was expected but the daemon artifact-root token is missing or stale. +- Artifact containment and destination checks now normalize more filesystem race cases, including dot-prefixed child paths and disappearing destination checks. +- Daemon state writes now use a mode-0600 temporary file and atomic rename to avoid exposing refreshed artifact-root tokens through pre-existing broader file modes. +- Daemon state writes no longer chmod after rename, avoiding spurious startup/shutdown failures after a successful atomic state write. +- Daemon-state host matching now treats loopback aliases as equivalent while still rejecting non-loopback mismatches, and artifact-root token validation uses timing-safe comparison. +- Deferred artifact-root resolution failures now report `INVALID_ARGUMENT`, and artifact file writes use a no-follow file open on POSIX to avoid following destination symlinks after validation. +- Artifact persistence now rejects directory targets explicitly, session registry stores artifact-root contexts by copy, and proxy auto-auth token hints are emitted only when daemon state matches the dialed daemon. +- Artifact parent creation now revalidates the walked parent immediately before each nested directory creation, and stdio proxy startup preserves explicit artifact-root URL metadata when cwd is unavailable. +- Artifact writes now open without truncating first and revalidate path parents again before file mutation, reducing parent-symlink races around final file opens. +- Artifact writes now use a same-directory temp file plus final rename, so writable handles are never opened through the caller-requested final pathname. +- Screenshot/PDF calls now keep the captured payload if a post-capture artifact-root lookup races session cleanup, returning `saved: false` with a structured artifact persistence error. +- Stale daemon-state token failures now include the state file path and explicit recovery options. +- Nested artifact directory creation now allows a symlinked artifact root that was already accepted and pinned by realpath, while still rejecting symlinks below that root. +- Proxies now wait for matching daemon state when auto-auth is requested by an auth-enabled daemon, even when artifact-root attachment is disabled, so the generated initialize token can be surfaced. +- Stdio proxy URL building now overwrites stale `client_artifact_root_token` query values with the current daemon-issued token whenever artifact-root attachment is enabled. +- Artifact writes now anchor temp-file creation and final rename to the opened parent directory on Linux, then verify the final realpath matches the requested destination before reporting success. +- MCP protocol sessions now pin the first resolved artifact root context across implicit rebinds, and screenshot/PDF calls preserve `saved: false` artifact responses if cleanup removes the session before the recovery mark. +- Daemon state and ingress health now share one start timestamp, keeping daemon-state freshness checks stable across process startup timing differences. +- Daemon state is now persisted only after ingress binds successfully, and object-form protocol artifact roots are copied before session reuse. +- Proxies now wait for a matching persisted daemon state record before deriving artifact-root tokens, closing the `/health`-before-state first-start race without delaying startup solely for optional auto-auth logging. +- Proxy health polling and daemon-state polling now share one connect deadline so failed startup cannot exceed the advertised daemon connect timeout. +- Daemon idle and signal shutdown now continue ingress/runtime teardown even if the best-effort daemon-state refresh fails. +- Artifact-root context cloning now lives in the shared artifact helper and is reused by protocol sessions and the session registry. +- Raw filesystem failures while persisting artifacts now surface as `TOOL_FAILED` while validation failures remain `INVALID_ARGUMENT`. +- Auth-enabled non-loopback daemon ingress now defers artifact-root filesystem validation until after initialize token validation to avoid pre-auth path-existence probing. + +### Verification + +- Added unit and integration coverage for encoded proxy artifact-root URLs, daemon-issued artifact-root token attachment and rejection, delayed daemon-state availability after health readiness, remote URL suppression/explicit attach, daemon-ingress artifact root handoff, non-loopback unauthenticated rejection, missing-root errors, pre-auth validation deferral, parent-symlink rejection, root-symlink replacement rejection, and two sessions writing the same relative screenshot path to distinct client roots. + ## 2026-04-23 ### v0.5.1 Cleanup Contract (FR-016, FR-017) diff --git a/specs/local-multiplexed-browser-mcp-spec.md b/specs/local-multiplexed-browser-mcp-spec.md index 0a37dfb..ef5adbb 100644 --- a/specs/local-multiplexed-browser-mcp-spec.md +++ b/specs/local-multiplexed-browser-mcp-spec.md @@ -19,7 +19,7 @@ - Clean-break v2 API contract for local Bun implementation (no backward-compat guarantee with blueprint tool shapes). - Structured error model for lock conflicts, stale sessions, extension disconnects, and invalid tool input. - Debugger-backed screenshot capture that returns MCP image content and supports viewport, full-page, selector, and clipped capture modes. -- Behavioral upstream parity for overlapping local browser tools while preserving LLM-optimized structured outputs, semantic snapshots, optional stable `element_ref` chaining, and optional local artifact persistence. +- Behavioral upstream parity for overlapping local browser tools while preserving LLM-optimized structured outputs, semantic snapshots, optional stable `element_ref` chaining, and per-client local artifact persistence. - The server SHOULD expose first-class onboarding guidance for LLM clients through MCP prompts and a fallback learn/help tool, so clients can discover canonical browser workflows without external documentation. - `element_ref` reuse is fail-closed: if follow-up resolution cannot prove it still targets the original node, the runtime MUST return `STALE_ELEMENT_REFERENCE` instead of replaying against a first-match selector. - `browser_snapshot` and `browser_lookup` SHOULD emit `element_ref` only for nodes with a replayably stable unique selector; callers MUST treat `element_ref` as optional. @@ -29,7 +29,7 @@ - `browser_network_requests` SHOULD accept `request_id` as an alias for `requestId`, and the router SHOULD canonicalize it to `requestId` before forwarding the call downstream. - `detach_from_tab` MAY infer `tab_id` only when the session owns exactly one tab; otherwise it MUST fail with a corrective `INVALID_ARGUMENT`. - Crash/restart recovery for client exits and extension restarts. -- Local-only security boundary: loopback binding with optional token authentication and zero cloud relay dependency. +- Local-only security boundary: loopback binding with optional token authentication, daemon-issued capability checks for per-client artifact-root metadata, explicit opt-in for non-loopback artifact-root attachment, and zero cloud relay dependency. - Co-located repository layout for Bun server and custom extension in a single implementation tree. - Living specification governance and completion-state updates tied to delivered milestones. @@ -66,7 +66,7 @@ ## 3.0 Data Models (Required) -- `agent_session`: `agent_session_id` (REQUIRED, string, unique, human-readable prefix + nonce), `client_name` (OPTIONAL, string, max 64 chars), `connected_at` (REQUIRED, RFC3339 timestamp), `last_seen_at` (REQUIRED, RFC3339 timestamp), `resource_reaped_at` (OPTIONAL, RFC3339 timestamp, set when browser resources are soft-reaped and cleared when the session revives), `state` (REQUIRED, enum: `connected|disconnecting|disconnected`), `auth_mode` (REQUIRED, enum: `none|token`), `owned_tab_ids` (REQUIRED, array, default `[]`). +- `agent_session`: `agent_session_id` (REQUIRED, string, unique, human-readable prefix + nonce), `client_name` (OPTIONAL, string, max 64 chars), `connected_at` (REQUIRED, RFC3339 timestamp), `last_seen_at` (REQUIRED, RFC3339 timestamp), `resource_reaped_at` (OPTIONAL, RFC3339 timestamp, set when browser resources are soft-reaped and cleared when the session revives), `state` (REQUIRED, enum: `connected|disconnecting|disconnected`), `auth_mode` (REQUIRED, enum: `none|token`), `owned_tab_ids` (REQUIRED, array, default `[]`). Artifact roots are server-internal session registry metadata and MUST NOT appear in exported session snapshots. - `cleanup_policy`: `stale_session_timeout_minutes` (REQUIRED, integer, `0` disables automatic cleanup, default `120`). - `extension_bridge`: `bridge_id` (REQUIRED, string), `connection_state` (REQUIRED, enum: `up|down|reconnecting`), `connected_at` (OPTIONAL, RFC3339 timestamp), `last_disconnect_reason` (OPTIONAL, enum: `socket_closed|heartbeat_timeout|manual|unknown`), `protocol_version` (REQUIRED, semver string). - `tab_snapshot`: `tab_id` (REQUIRED, integer > 0), `url` (REQUIRED, URL string), `title` (REQUIRED, string), `debugger_attached` (REQUIRED, boolean), `is_locked_by_agent` (REQUIRED, boolean), `locked_by_agent_session_id` (OPTIONAL, string when locked), `lock_acquired_at` (OPTIONAL, RFC3339 timestamp). @@ -155,7 +155,7 @@ | FR-008 | Crash Recovery | The system MUST reclaim locks and reconcile state when a client crashes or disconnects unexpectedly. | High | Unit (session cleanup), Integration (socket close hooks), E2E (kill client while lock held). | | FR-009 | Extension Restart Recovery | The system MUST reconcile stale attachments/locks when extension disconnects and reconnects, MUST fail in-flight bridge requests deterministically during disconnect, and MUST recover post-reconnect routing without cross-session corruption. | High | Integration (bridge reconnect + in-flight disconnect fault), E2E (restart/disconnect mid-command), Regression (post-reconnect attach/tool call works). | | FR-010 | Structured Error Contract | The system MUST return structured error payloads with code, retryability, and correlation ID for all tool failures. | High | Unit (error mapping), Contract/API (error schema), E2E (assert consistent error shape). | -| FR-011 | Local Security Boundary | The system MUST bind loopback only and SHOULD support optional token authentication without mandatory manual setup. | High | Unit (config defaults), Integration (localhost only binding), Security (auth optional + rejection cases). | +| FR-011 | Local Security Boundary | The system MUST bind the browser bridge loopback-only, SHOULD support optional token authentication without mandatory manual setup, MUST NOT accept per-client artifact-root metadata without a daemon-issued artifact-root token, MUST NOT accept per-client artifact-root metadata on unauthenticated non-loopback daemon ingress, and MUST NOT auto-send client cwd metadata to non-loopback daemon ingress without explicit opt-in. | High | Unit (config defaults), Integration (localhost-only binding + artifact-root gate), Security (auth optional + rejection cases). | | FR-012 | AGENTS Governance Update | The implementation MUST update `AGENTS.md` with living-spec maintenance directive and co-located directory policy. | Medium | Unit (lint/check rule for directive presence), Integration (CI policy check), E2E (spec update process in PR workflow). | | FR-013 | Repository Layout Standard | The implementation MUST use a co-located layout where custom extension resides under the local Bun implementation tree. | High | Unit (path resolver tests), Integration (build/test scripts detect expected layout), E2E (end-to-end dev bootstrap on clean clone). | | FR-014 | Cross-Platform Test Matrix | The system MUST run required test suites on Linux, macOS, and Windows in CI for release eligibility, and MUST publish per-OS hard-gate evidence artifacts for auditability. | Critical | Integration (CI workflow checks + artifact upload), E2E (matrix run with required pass gates), Performance (runtime bounds per suite). | @@ -171,7 +171,7 @@ | NFR-PERF-001 | Performance | Tool latency under load | p95 latency for `list_available_tabs`, `attach_to_tab`, `detach_from_tab` MUST be < 300ms with 8 concurrent clients and 20 tabs on local host. | | NFR-ACC-001 | Accuracy | Lock state correctness | Lock ownership reports MUST be 100% consistent with actual debugger attachment state across 10,000 simulated lock operations. | | NFR-REL-001 | Reliability | Recovery from crashes/restarts | System MUST recover to a consistent lock state within 3 seconds after client crash or extension reconnect in 99% of test runs. | -| NFR-SEC-001 | Security | Local access controls | Server MUST reject non-loopback connections 100% of the time; when token auth is enabled, unauthorized requests MUST be rejected 100% of the time in tests. | +| NFR-SEC-001 | Security | Local access controls | Browser bridge MUST reject non-loopback connections 100% of the time; daemon ingress MUST reject per-client artifact-root metadata without a daemon-issued artifact-root token 100% of the time; unauthenticated non-loopback daemon ingress MUST reject per-client artifact-root metadata 100% of the time; auth-enabled non-loopback daemon ingress MUST NOT reveal artifact-root filesystem existence before token validation; when token auth is enabled, unauthorized requests MUST be rejected 100% of the time in tests. | | NFR-SCALE-001 | Scalability | Concurrent agent support | System MUST support at least 8 simultaneous active client sessions with no failed routing or lock corruption during 30-minute soak tests. | | NFR-EXT-001 | Extensibility | Toolset growth | Adding a new tab-scoped tool MUST require no changes to lock core algorithm and no more than one new adapter module. | | NFR-OBS-001 | Observability | Debuggability of failures | 100% of failed tool responses MUST include correlation ID and structured error code in logs and client payloads. | @@ -214,8 +214,11 @@ - `browser_take_screenshot(input: { type?: "jpeg"|"png", quality?: number, fullPage?: boolean, selector?: string, element_ref?: string, padding?: number, path?: string, highlightClickables?: boolean, deviceScale?: number, clip_x?: number, clip_y?: number, clip_width?: number, clip_height?: number, clip_coordinateSystem?: "viewport"|"page" })` - The server MUST return an MCP `content` image block plus structured metadata describing capture mode and image MIME type. - Supported capture modes MUST include `viewport`, `full_page`, `selector`, and `clip`. + - When `path` is provided, the server SHOULD persist the screenshot relative to the calling MCP client's artifact root, not the shared daemon cwd. + - Shared-daemon artifact-root metadata MUST be accepted only with a daemon-issued artifact-root token and from loopback daemon ingress or authenticated daemon ingress. - `browser_pdf_save(input: { path?: string, landscape?: boolean })` - - When `path` is provided, the server SHOULD persist the PDF locally and return filesystem metadata instead of returning large inline payloads to MCP callers. + - When `path` is provided, the server SHOULD persist the PDF relative to the calling MCP client's artifact root and return filesystem metadata instead of returning large inline payloads to MCP callers. + - Shared-daemon artifact-root metadata MUST be accepted only with a daemon-issued artifact-root token and from loopback daemon ingress or authenticated daemon ingress. ### 8.5 Internal Components