fix(security): harden workspace boundaries and remote transports - #222
Merged
Conversation
worktree.migrate trusted `from` behind `Path::starts_with(base)`, which resolves nothing: `<base>/../..` passed, then unlink_shared_artifacts, remove_dir_all and rename ran on it. classify_migration_source refuses the base itself, a relative step, a link and anything deeper than one level. migrate_worktree_blocking refuses a directory that is not a worktree of this repo. gone and elsewhere keep their old answers. thread_ref_prefix was not injective: `a.b` and `a-b` shared a namespace, so forget and restore crossed threads. safe ids stay verbatim, the rest go hex under `x_`, so every uuid keeps the refs it already has. rpc::checked_thread_ids refuses a non-uuid on thread.create, thread.spawn and checkpoint.
a token issued for one project held readproject and nothing asked which project a read landed in: /v1/projects, /v1/timeline, /v1/search, /v1/snapshot and /v1/transcript answered for the whole workspace, /mcp included. grant::reads_across answers that second question, routes::confined_to asks it once. a thread key still reads everything, which is what /v1/transcript exists for. the scope rides into the query rather than over the answer: store.search, search::transcripts and snapshot::take each take it. store.thread_ids_of_project names the terminals a confined caller may read, a transcript file naming its thread and not its project.
push.subscribe took any https host and send_one followed whatever came back: an endpoint could redirect a post onto 169.254.169.254, and a name could resolve inward after acceptable_endpoint had passed. build_client now sets redirect::Policy::none, a PublicOnlyDns resolver handing the connector public addresses only (the resolution the connection itself uses, so nothing left to re-point), and a ten second timeout. send_one calls acceptable_endpoint again. BOITE_PUSH_ALLOWED_HOSTS narrows registration to named hosts. tests use a loopback server, no network: a 302 stays a 302 with its target never hit, a stored http endpoint errors before any connect.
Worker::shutdown sent its stop order down the bounded event channel. A worker stuck in a slow flush leaves that channel full, so the send blocked until the flush ended, before the deadline loop had even started: up to the 5s connect timeout at window close, which is exactly the freeze the deadline exists to prevent. The order now travels on a flag the worker reads at the top of its loop; the message is still sent, without blocking, only to wake a worker parked in recv_timeout. Whatever is still queued is drained into the final flush, and track refuses to enqueue once shutdown started. parse_os_release_pretty_name aborted on the first line without an "=", so a comment or a blank line above PRETTY_NAME reported plain "Linux" for a machine that named itself in the file. Non-assignments are skipped now. TelemetryRuntime::update mutated the in-memory sidecar before attempting the write. A refused write left the window showing an opt-in the file never recorded, and the queue sending under it, until the next launch read the file back. The write comes first and memory only follows on success.
agent-answer.ts routes an answer on who wrote the request, not on workspace.current(), which in dynamic mode is the local device. handleBrowserAsk uses the same door. socket.ts: a dial carries a generation and an abort, so close() during buyTicket opens no WebSocket afterwards. sendInput checks readyState, and RemoteBackend.pty.write rejects on its false; call sites got a catch. dispatches.ts types first, then settles delivered or dropped/ write_failed. settle_dispatch only moves a row out of queued, so settling first lost the line. tests: socket.test.ts, index.test.ts, agent-answer.test.ts, dispatches.test.ts, each failing against the code before it.
Only fold case for text path comparisons on Windows. This keeps case-sensitive siblings outside the legacy migration base.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
this branch confines Agent API project credentials across projects, timeline, search, snapshot, transcript and pulse reads. it validates worktree migration sources before mutation, makes checkpoint namespaces injective, and rejects non-UUID thread ingress.
web push now rejects redirects and private DNS answers, revalidates stored endpoints, bounds requests, and supports an optional host allowlist. remote clients route agent answers to the request source, abort stale socket dials, reject dropped PTY input, and settle dispatches only after a successful write. telemetry shutdown is bounded,
os-releaseparsing skips non-assignments, and consent state commits only after persistence succeeds.this branch is based on current
master. the duplicate ConPTY fixture change was dropped during rebase because #219 already contains that fix.BOITE_PUSH_ALLOWED_HOSTSstays optional so self-hosted push services keep working; redirects remain disabled and every resolved address must still be public.verified with
bun run check, 1,120 frontend tests, 813 Rust workspace tests, workspace clippy with warnings denied, production build, bundle budget, server smoke, WebSocket smoke and RemoteBackend smoke.cargo fmt --all -- --checkremains red on pre-existing repository-wide rustfmt drift; CI does not run that gate.