fix(daemon): confine remote HTTP trust boundaries

[thymikee]

Summary

Fix the remote HTTP trust boundaries for #2097 and #2098 with one daemon-owned attenuation bit:

• Auth-hook and proxied requests are publicNetworkOnly; local loopback and socket behavior stays unchanged.
• Raw HTTP host paths remain rejected, while daemon-owned uploaded artifacts remain usable without trusting a client path.
• Maestro http.post preserves native Fetch redirects, headers, methods, and decompression while every hop is public-address checked and pinned, including through configured proxies. HTTPS downgrade and decoded responses over 8 MiB are refused.

This revision removes the manual redirect engine and generalized install-source transport rewrite. Against the PR base it changes 17 files (+869/-56); against the previous PR head it removes 1,034 lines. npm unpacked size is +4.1 kB, of which the required synchronous Maestro child entry is 3.0 kB.

Closes #2098

Validation

• Planted red on the previous head: gzip decoding and cross-origin 307 Fetch semantics both failed; both pass on this revision.
• pnpm check:affected --run (all runnable gates passed; GitHub-authoritative native/provider lanes remain CI-owned).
• Exact size report: npm tarball +1.5 kB, unpacked +4.1 kB, gzip JS +2.1 kB.
• Independent size/design review found no remaining code finding; retained choices are explicit product boundaries rather than compatibility workarounds.
• No device evidence required: this changes daemon HTTP admission and Maestro script networking, not device behavior.

[thymikee]

Not ready.

[P2] Bound public Maestro HTTP responses before buffering them. processPublicResponse reads the complete body before even deciding whether the response is a redirect, and readResponseBody has no byte ceiling. An authenticated remote script can therefore drive unbounded child memory, including on a redirect that should only be revalidated. Classify redirects before body collection, cap final response bytes, and add streaming/oversized planted proof.

The remaining trust routing is sound: host paths are default-denied under auth, explicit roots are realpath-confined, upload IDs remain server-owned, and public URL resolution is validated and pinned per hop. This head is also DIRTY/conflicting and has only CodeQL evidence, not normal exact-head CI, so it is not ready.

[thymikee]

Addressed in 36f8305 and rebased onto current main (04758c9).

• Redirect status/location is classified before body iteration; redirect bodies are discarded without buffering.
• Final public-only response bodies are consumed incrementally with an 8 MiB cap; overflow fails with a bounded COMMAND_FAILED error.
• Added planted coverage for non-iterated redirect bodies and an 8 MiB + 1 byte streamed response. The pre-fix raw child accepted 8,388,609 bytes; the fixed path rejects it.

Validation: pnpm check:affected --run completed with all runnable checks passing; focused trust/HTTP/Maestro tests pass 53/53. GitHub-authoritative device/provider lanes remain CI-owned.

[github-actions]

Size Report

Metric	Base	Current	Diff
JS raw	2.51 MB	2.51 MB	+4.0 kB
JS gzip	837.2 kB	839.3 kB	+2.1 kB
npm tarball	964.1 kB	965.7 kB	+1.5 kB
npm unpacked	3.34 MB	3.35 MB	+4.1 kB

npm unpacked components

Component	Base	Current	Diff
JS / dist source	2.66 MB	2.67 MB	+4.1 kB
Apple runner source/project	581.1 kB	581.1 kB	0 B
macOS helper source	54.8 kB	54.8 kB	0 B
Android helper artifacts	0 B	0 B	0 B
Other package files	45.6 kB	45.6 kB	0 B

Startup median (7 runs, lower is better):

Scenario	Base	Current	Diff
CLI --version	30.0 ms	29.9 ms	-0.1 ms
CLI --help	85.0 ms	84.8 ms	-0.2 ms

Top changed chunks:

Chunk	Raw diff	Gzip diff
dist/src/internal/daemon.js	+491 B	+169 B
dist/src/cli-help.js	+304 B	+132 B
dist/src/session2.js	-3 B	-53 B
dist/src/daemon-client-lifecycle.js	-5 B	-5 B

Top changed packed files

Packed file	Base	Current	Diff
dist/src/install-source-network-transport.js	0 B	3.4 kB	+3.4 kB
dist/src/install-source2.js	10.9 kB	7.6 kB	-3.3 kB
dist/src/internal/run-script-http-child.js	0 B	3.0 kB	+3.0 kB
dist/src/internal/daemon.js	108.0 kB	108.5 kB	+491 B
dist/src/cli-help.js	89.5 kB	89.8 kB	+304 B
dist/src/internal/run-script-http-child.d.ts	0 B	162 B	+162 B
dist/src/http-health.js	1.7 kB	1.8 kB	+64 B
dist/src/proxy.js	8.6 kB	8.6 kB	+46 B
dist/src/daemon-client-lifecycle.js	42.3 kB	42.3 kB	-5 B
dist/src/session2.js	216.4 kB	216.4 kB	-3 B

[thymikee]

Re-reviewed exact head 36f8305d94: the prior response-boundary finding is resolved. Redirects are classified before body iteration, final public responses are capped incrementally at 8 MiB, and the redirect/streaming overflow regressions are non-vacuous. Host-path and public-network trust routing remains sound, GitHub reports CLEAN/MERGEABLE, and all substantive exact-head checks including Coverage, Repo Guards, integration, every native smoke, size, and CodeQL are green. Merge-ready.

[thymikee]

The reviewed head is unchanged, but the newly advanced main now conflicts with this branch. I removed ready-for-human; rebase and resolve the branch blocker before the next readiness review.

[thymikee]

Re-reviewed a44016b. P1: remove AGENT_DEVICE_HTTP_ALLOW_HOST_PATH_INSTALL / root confinement and reject unbacked remote kind:path requests unconditionally. Although realpath confinement is correct in isolation, #2097’s later maintainer triage explicitly superseded the original issue sketch: no host-path opt-in without separate approval; uploaded-artifact IDs remain the supported route. Dependency: #2098 is explicitly unready until open #2095 establishes the shared remote trust posture. The public-only Maestro routing itself looks sound (per-hop validation/pinning, redirect/header controls, response cap), but the PR is not ready while that dependency and P1 remain. Android Smoke failed in an unrelated system-automation wait for 'Alert result: cancelled'; this looks like device/lane flake rather than causal evidence against this diff, but still needs a green rerun. Also add the required size justification for current +7.3 kB npm unpacked.

[thymikee]

Re-reviewed exact head c4439c883fd85f338694d8931f62ce6a238dd44e. The prior P1 is fixed: public-only remote auth hooks now reject unbacked kind: path, the host-path opt-in/config/help surface is removed, obsolete env variables are covered, and uploaded-artifact-backed plus local no-hook paths remain intact. I found no remaining code defect. This is not ready yet: dependency #2095 remains open, several checks are pending, and the PR body should explicitly record why a materially smaller design was rejected for the +7.3 kB growth (the synchronous Maestro child transport is required by http.post, while shared async validation avoids duplicating DNS/SSRF policy).

[thymikee]

Exact head c4439c8 has no new trust-code finding and #2095/#2104 is now merged, but it is not ready. Current main conflicts in http-server.ts and cli-help-topics.test.ts; the rebase must preserve #2104's unconditional fail-closed tenant attestation before layering remote network/path trust policy. Coverage is a real owner-action failure: changed-line coverage is 95/140 = 67.86%, with unproved credential/malformed-host rejection and 38 uncovered run-script-http-child lines across URL/redirect/downgrade/parser/child-failure paths. Add focused negative/boundary proof, then rerun exact-head gates. Update the stale body dependency and size text (+5.6 kB latest, not +7.3 kB). No readiness label until rebase and green coverage.

[thymikee]

Addressed in 2dd7fb61ab and rebased onto c7f42ccedc. The merged tenant-attestation gate remains first; network/path trust is applied only after the request is trusted. Added non-vacuous coverage for credential and malformed-host rejection, DNS failure/empty answers, public address classes, malformed child URLs and input, redirect location/limit/downgrade/header/method behavior, and bounded streaming responses. Updated the remote RPC fixture to attest a tenant. pnpm check:affected --run passes all runnable checks; changed-line coverage is 128/140 (91.43%) and changed-branch coverage is 103/125 (82.40%). Full coverage is CI-pending; the local full run had only the unrelated gesture-parity timeout, which passes 4/4 in isolation. The PR body now records the merged #2095/#2104 dependency state, latest +5.6 kB size, and the smaller-design tradeoff.

[thymikee]

Reviewed exact head 2dd7fb61ab.

P1 — the documented remote proxy path still permits SSRF. agent-device proxy starts/reuses the local HTTP daemon without configuring AGENT_DEVICE_HTTP_AUTH_HOOK, then authenticates and forwards remote /rpc traffic without a server-trusted remote-policy marker (src/cli/commands/proxy.ts, src/remote/daemon-proxy.ts). resolveHttpTrustPolicy consequently selects unrestricted, and remote Maestro runScript reaches the raw fetch branch. A proxy-token holder can still request loopback, link-local, or internal URLs; this also bypasses the new 8 MiB response cap.

The new tests cover an auth-hook-configured direct HTTP server and a directly supplied public-only child input, but not the shipped proxy route. Please derive remote provenance from the proxy/transport boundary rather than optional auth-hook configuration, and plant a proxy-path regression proving loopback refusal plus public-address success.

The public-only implementation itself—shared DNS validation, address pinning, redirect revalidation, downgrade rejection, cleanup, and response cap—looks sound. Current Android smoke failure appears unrelated, but exact-head CI is not fully green yet.

[thymikee]

Addressed in 5349a5a. agent-device proxy now stamps x-agent-device-network-access: public-only on forwarded /rpc requests. The daemon resolves that marker at the request boundary, so a reused local daemon cannot run proxy-originated Maestro HTTP requests through unrestricted fetch. Missing markers preserve unrestricted no-hook local HTTP behavior; invalid or duplicate markers fail closed. Added a proxy-to-daemon regression using the shared Maestro HTTP policy: loopback is refused without reaching the target, while a public literal is approved and dispatched through the shared transport seam. Removing only the marker reproduced the old 200 response and loopback reachability. Validation: focused trust/HTTP/proxy tests 46/46, pnpm check:quick, and planted-red proof pass. pnpm check:affected --run ran the full fail-open local selection with 2,695 tests passing, but exited on two unhandled existing Apple replay environment-teardown/module-load errors in session-replay-repair-record-exclusion.test.ts; the new proxy tests passed. GitHub exact-head checks are pending.

[thymikee]

Reviewed exact head 5349a5ab08. No actionable code or security findings.

The proxy now supplies the restrictive network marker itself and does not forward a client marker; the daemon accepts only public-only. The shipped proxy → HTTP daemon → replay → Maestro route rejects loopback/private/link-local targets before connection, permits public targets, pins approved DNS addresses, revalidates redirects, rejects HTTPS downgrade, strips cross-origin sensitive headers, caps final bodies at 8 MiB, and preserves daemon-owned uploaded artifacts while rejecting wire host paths. The new proxy-path regression covers the previously exploitable no-auth-hook daemon route.

Completed exact-head checks are green. Pending smoke jobs are CI status only, not a code-review blocker.

[thymikee]

Exact head 5349a5ab08 is now fully green and mergeable. The prior clean security review stands; ready for human merge.

[thymikee]

Verified exact head f02681888661862dd90b681cfbc99d798941f808: clean and mergeable. The remote/auth-hook and daemon-proxy paths stamp the daemon-owned publicNetworkOnly attenuation bit; local no-hook HTTP remains on native Fetch. The production Maestro http.post path retains Fetch redirects, cross-origin header/body behavior, decompression, configured proxy routing, and synchronous execution while re-approving and address-pinning every hop, rejecting HTTPS downgrade, and capping decoded final bodies at 8 MiB.

Remote host paths are rejected at both proxy and daemon boundaries; uploaded IDs resolve only to daemon-owned, tenant-checked artifacts. The real proxy-to-daemon regression proves loopback refusal before connection. Exact-head CI is fully green, including coverage, package/wire compatibility, provider-backed integration, live web, and Android/iOS/macOS/Linux smokes. Size is +4.1 kB unpacked (+2.1 kB gzip JS), chiefly the necessary 3.0 kB synchronous child; the final revision removed 1,034 lines from the previous head. Ready for merge.

Residual risk: live DNS-rebinding and enterprise-proxy permutations cannot be exhaustively exercised in CI, but the approved address is pinned on each Fetch hop and proxy tunnelling is covered.

[github-actions]

PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-08-28 19:37 UTC