vibesdk icon indicating copy to clipboard operation
vibesdk copied to clipboard

Feat: use api-proxy for user app uptime check instead of direct call

Open AshishKumar4 opened this issue 4 months ago • 1 comments

Summary

This PR addresses issues with Cloudflare Zero Trust access for preview iframes by routing availability checks through the main application proxy instead of direct subdomain calls.

Changes

  • New API endpoint: Added /api/apps/:id/preview-status (public, HEAD only) that checks preview availability through the proxy
  • Refactored preview resolution: Created worker/utils/previewResolver.ts to centralize preview availability logic (sandbox → dispatcher fallback)
  • Simplified main worker: Refactored worker/index.ts to delegate preview resolution to shared utility
  • Updated frontend: Modified preview-iframe.tsx to use new proxy endpoint instead of direct subdomain HEAD requests

Motivation

Users with Cloudflare Zero Trust policies configured were unable to load preview iframes because the availability check attempted to access user app subdomains directly, which Zero Trust policies blocked. By routing the check through the main application proxy (which users are already authenticated to), the preview availability check bypasses Zero Trust restrictions.

Testing

  • Verify preview availability checks work for apps in sandbox
  • Verify preview availability checks work for deployed (dispatcher) apps
  • Confirm Zero Trust users can now see preview iframes
  • Ensure existing preview functionality remains intact

Related Issues

This may help with preview loading issues mentioned in:

  • #195 (Surface log stream from sandboxed app)

This PR description was automatically generated by Claude Code

AshishKumar4 avatar Nov 06 '25 06:11 AshishKumar4

Claude finished @AshishKumar4's task —— View job


Code & Security Review (Critical Path)

Recommendation: REQUEST CHANGES

Summary

This PR successfully addresses the Cloudflare Zero Trust access issue by routing preview availability checks through the main application proxy. The architectural approach is sound and follows good DRY principles by extracting shared logic into previewResolver.ts. However, there are security vulnerabilities that must be addressed before merging.

Security Issues

High Severity

  1. Potential SSRF via Request Forwarding (comment)
    • Location: worker/utils/previewResolver.ts:23
    • The resolvePreview() function forwards arbitrary Request objects to user-controlled sandboxes/dispatchers
    • While previewProxyRoutes.ts creates clean requests, worker/index.ts forwards full original requests with all headers
    • Risk: Sensitive headers (cookies, auth tokens) could leak to user sandboxes
    • Must fix: Add request sanitization or explicit header filtering

Medium Severity

  1. Path Traversal Risk in API Endpoint (comment)

    • Location: worker/api/routes/previewProxyRoutes.ts:15
    • No input validation on appId parameter from URL
    • Should validate format (alphanumeric, hyphens, underscores only) and length (max 63 chars per DNS limits)
  2. Insufficient URL Validation in Frontend (comment)

    • Location: src/routes/chat/components/preview-iframe.tsx:82
    • appId extracted from user-provided URL without validation
    • Could allow injection of special characters or path traversal sequences
  3. Missing Input Validation in Main Worker (comment)

    • Location: worker/index.ts:46
    • appId extracted from hostname without validation before use
    • Should validate against expected patterns (matches sandbox's extractSandboxRoute() approach)

Code Quality Issues

Medium Severity

  1. Inconsistent Error Handling (comment)
    • Location: worker/utils/previewResolver.ts:60
    • Dispatcher errors (403/404/500) silently report as "unavailable" with no error message
    • Makes debugging difficult for operators
    • Should distinguish between "not found" vs "error" states

Positive Aspects

  • Good architectural refactoring - DRY principle applied correctly
  • Properly addresses the Zero Trust access issue
  • Public endpoint correctly configured with AuthConfig.public
  • CORS headers properly configured with Access-Control-Expose-Headers
  • Request deduplication handled via HEAD-only endpoint
  • Follows existing project patterns (route setup, error handling, logging)
  • Type safety maintained throughout (no any types)

Required Changes

Before this PR can be approved:

  1. [CRITICAL] Add request sanitization in resolvePreview() to prevent header leakage
  2. Add input validation for appId in all three locations (API route, frontend, main worker)
  3. Improve error handling in previewResolver.ts to distinguish error types
  4. Consider adding rate limiting to the new public endpoint to prevent abuse

Testing Recommendations

  • Test with malicious appId values: ../../admin, %2e%2e%2fadmin, admin;ls, etc.
  • Verify sensitive headers (Authorization, Cookie) are not forwarded to user sandboxes
  • Test error scenarios: sandbox errors, dispatcher failures, network timeouts
  • Confirm Zero Trust users can access previews with this change

github-actions[bot] avatar Nov 06 '25 14:11 github-actions[bot]