Skip to content

fix(server): device screenshots no longer fail with 403 over Tailscale - #19158

Closed
StivenKN wants to merge 2 commits into
pingdotgg:mainfrom
StivenKN:fix/device-hub-forwarded-headers
Closed

StivenKN wants to merge 2 commits into
pingdotgg:mainfrom
StivenKN:fix/device-hub-forwarded-headers

Conversation

@StivenKN

Copy link
Copy Markdown

Problem

When you open T3 through Tailscale Serve or T3 Connect, the Device panel's Android screenshot fails with "Screenshot capture failed (403). Try again." The same click works when the browser connects to the server directly.

The reverse proxy adds X-Forwarded-Proto: https, and DeviceHubProxy passes it on to the device hub. The hub's Node adapter builds the request origin from that header, so it sees https://127.0.0.1:<port>. The proxy rewrites Origin to http://127.0.0.1:<port>. serve-emu compares the two before any mutation, finds a mismatch, and returns {"code":"forbidden","message":"Browser origin is not allowed to mutate serve-emu state"}.

Change

DeviceHubProxy adds x-forwarded-proto to the headers it drops. The header describes the browser's connection to T3, not T3's loopback connection to the hub. The hub reads no other forwarding header, so the fix drops only this one.

Scope and approval

This is a small fix for an obvious bug, so it qualifies without a prior issue. It changes one entry in the proxy's dropped-header set. The fix covers every POST through the proxy (screenshot, stream mode, stream settings, fold). WebSocket upgrades do not forward request headers, and the agent's device_screenshot tool calls the hub directly, so neither path had the bug.

Verification

  • vp test run apps/server/src/device/DeviceHubProxy.test.ts. The new test sends a screenshot POST with X-Forwarded-Proto: https and asserts that the hub gets the rewritten Origin and no x-forwarded-proto. On the test commit alone it fails with expected { origin: 'http://hub.test', …(4) } to not have property "x-forwarded-proto". With the fix, 16 of 16 proxy tests pass.
  • Live hub (expo-device-hub 0.15.3, Pixel 9 API 36 emulator). A throwaway harness ran the real DeviceHubProxy.layer with FetchHttpClient against the running hub and sent the request that Tailscale Serve delivers. On main the hub returned 403 with the error above. With the fix it returned 200 image/png (767,845 bytes). I did not commit the harness.
  • The server log of a real T3 nightly (0.0.46) shows the same failure from a remote browser. The upstream hub request fails with StatusCode error (403 POST http://127.0.0.1:43531/vendor/serve-emu/api/screenshot?device=emulator-5554).
  • I did not test iOS. serve-sim requests go through the same hub server, so the header no longer reaches them either. I inferred this from the code and did not run it.

Model and harness: Claude Opus 5.5 (claude-opus-5-5) in Claude Code, running inside T3 Code.

🤖 Generated with Claude Code

StivenKN and others added 2 commits October 13, 2026 20:28
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Tailscale Serve and T3 Connect add X-Forwarded-Proto: https. The hub
builds its request origin from that header, so it no longer matched the
Origin the proxy sets, and the Device panel screenshot failed with 403.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:XS 0-9 changed lines (additions + deletions). labels Oct 13, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at bf18b55

Macroscope's review found this PR approvable — This is a minimal, self-contained server bug fix that removes one conflicting proxy header while preserving routing, authentication, and request behavior. The affected screenshot flow is covered by a focused regression test, with no product-default or static-analysis changes.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Oct 13, 2026

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 8e55fab7-c322-46a5-a2e5-188393f0e2f8
📥 Commits

Reviewing files that changed from the base of the PR and between 6947f0d and bf18b55.

📒 Files selected for processing: 2
  • apps/server/src/device/DeviceHubProxy.test.ts
  • apps/server/src/device/DeviceHubProxy.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

📝 Walkthrough

The device hub proxy now omits x-forwarded-proto from forwarded requests. A test records upstream headers and checks a screenshot POST sent from a remote browser.

Changes: Device hub proxy

Layer / File(s) Summary
Filter the forwarded protocol header
apps/server/src/device/DeviceHubProxy.ts, apps/server/src/device/DeviceHubProxy.test.ts
The proxy excludes x-forwarded-proto from requests sent to the hub. The test fixture records upstream headers, and a screenshot POST test checks that the forwarded request uses the hub origin and omits that header.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Bug fix

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to bf18b

No actionable merge-blocking risk is apparent from the supplied change context; the reported screenshot test and live validation support the intended fix. Complete normal checks before merging.

Security architecture risk: 🔵 Low · up to bf18b

The change is narrowly scoped and preserves authentication, operation permissions, and endpoint restrictions. No introduced security bypass was established, but the external hub’s exact origin checks and mutation-recovery behavior could not be independently verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The affected exposure is the existing authenticated, allowlisted device-hub HTTP surface. Screenshots retain read-scope access, while device-control mutations retain operate-scope requirements; the change does not add routes or alter host selection.

Trust Boundaries and Controls

  • observed — The browser-to-server boundary retains credential and scope enforcement. At the server-to-hub boundary, cookies, authorization, and DPoP headers remain removed, and the client’s x-forwarded-proto is now also removed. The regression test verifies this forwarded-header contract using a mocked hub, not the external hub’s enforcement.

Resilience and Maintainability Implications

  • inferred — No proxy-side state-authority or cleanup guarantee is changed by the header filter. Once the external hub accepts an operation, its atomicity, idempotency, concurrency, and interruption-recovery behavior cannot be established from the available source.
Pre-merge checks: Passed 4
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title is concise, uses the conventional commit format, and clearly describes the screenshot 403 fix over Tailscale.
Description check Passed The description covers the problem, change, scope and approval rationale, focused verification, observed results, limitations, and agent details. It matches the required template and provides sufficie…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@StivenKN

Copy link
Copy Markdown
Author

#19379 already solves this

@StivenKN StivenKN closed this Oct 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XS 0-9 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant