Conversation
Direct public hosts and CONNECT requests previously accepted any guest port. Resolve the current tunnel declaration through the API before either path reaches the runner, including requests over pooled HTTP connections. Preserve signed and terminal access.
The proxy asks /preview/{boxId}/tunnels/{port} on every public preview
request, and each call queried Postgres. Cache both verdicts for 3 s
under preview:tunnel:{boxId}:{port}, the same window as the other
preview checks (preview:public, preview:token), so revocation or making
a box private takes effect within 3 s.
declarePublic clears the key after its write, so a freshly declared
port is not held behind a cached refusal.
Co-authored-by: Cursor <cursoragent@cursor.com>
The initial tunnel gate changes proxy and API behavior only. Keep the Dashboard copy on its established contract while the remaining proxy paths are completed in the next layer.
parseHost returned the raw port label, so "022222" slipped past TERMINAL_PORT string comparisons: on the HTTP path it skipped terminal authentication and on the warning page it showed the interstitial. parseHost now returns a validated uint16 (1-65535) and TERMINAL_PORT is numeric, so every caller compares one canonical form and the separate re-parsing in GetProxyTarget and tunnelTarget is gone. The tunnel access check no longer depends on isDirectHost: any public box is gated, so a future box ID format cannot skip it. An API failure now answers 502 on the HTTP path, matching CONNECT. Co-authored-by: Cursor <cursoragent@cursor.com>
The proxy now gates every guest service preview on a public tunnel, while terminal traffic follows its authenticated route. Clarify URL issuance, access checks, cache behavior, and source references so the docs match the current implementation.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: QUIET Plan: Advanced Run ID: 📒 Files selected for processing (12)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthroughThe API caches public tunnel access verdicts in Redis for three seconds and clears the affected entry when a public tunnel is declared. The proxy parses ports as numeric values and checks access before forwarding guest-service HTTP and raw CONNECT requests. Denied checks return 404, and access-check errors return 502. The authenticated terminal path remains separate from guest tunnel checks. Sequence Diagram(s)sequenceDiagram
participant Client
participant Proxy
participant PreviewTunnelEndpoint as Preview tunnel endpoint
participant TunnelService
participant Redis
participant Database
Client->>Proxy: Request guest service port
Proxy->>PreviewTunnelEndpoint: Check access for box and port
PreviewTunnelEndpoint->>TunnelService: Check public tunnel access
TunnelService->>Redis: Read cached verdict
alt Cache miss
TunnelService->>Database: Run existing access checks
Database-->>TunnelService: Return access result
TunnelService->>Redis: Cache result for three seconds
end
TunnelService-->>PreviewTunnelEndpoint: Return access result
PreviewTunnelEndpoint-->>Proxy: Return access status
Proxy-->>Client: Forward request or return denial
Suggested reviewers: Priority: ➖ Normal Change: Bug fix Merge Risk: ⚪ Minimal · up to The documented tunnel gate has no confirmed outstanding issue in the supplied evidence. The redirect behavior remains unverified but does not, on its own, prevent merging after normal checks. Security Architecture ReviewSecurity architecture risk: 🟡 Moderate · up to The new gate substantially narrows access to guest ports, but its decision depends on a remote response and a short-lived cache. A possible redirect path cannot be assessed without production routing details, and access may briefly persist after a status change. Retained concerns
Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 22 functions across 9 files. (3 skipped: 3 unsupported.)
Warning Some tools did not complete. Review the errors below. 🔧 Biome (2.5.12)apps/api/src/box/services/tunnel.service.tsFile contains syntax errors that prevent linting: Line 22: Decorators are not valid here.; Line 23: Decorators are not valid here. Comment |
TL;DRThe PR author must acknowledge the current diff and description before requesting review. Author review acknowledgmentThe PR author acknowledged this commit. @zombee0: read the current diff and description, then check: Does the description accurately explain the mechanism shown in the diff? Use the form best suited to the change. Check drafts too, and repeat this review Unacknowledged PRs are converted to draft. After this check passes, click Ready for review when you want reviews. |
|
/reviewed 211ea84 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| @@ -0,0 +1,31 @@ | |||
| ## TL;DR | |||
This branch has not been deployed
2 participants
TL;DR
Require an active public tunnel for every guest service preview.
Design doc: https://linear.app/polygala/issue/POL-695/persist-per-port-access-declarations-for-httpwebsocket-and-connect
Summary
A preview URL or credential previously could reach an undeclared guest port. The proxy now resolves the box and numeric port, checks the API per-port declaration before runner access, and denies missing declarations. API errors fail closed; terminal port 22222 keeps its authenticated route. The API caches access verdicts for 3 seconds.
Docs: apps/proxy/README.md request paths and docs/concepts/networking.md service access.
Verification
make test:apps: proxy tests passed; the full matrix failed on Postgres file descriptors and Dashboard localStorage setup. The before-fix reproducer run is pending.