-
Notifications
You must be signed in to change notification settings - Fork 829
Feat: use api-proxy for user app uptime check instead of direct call #222
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: nightly
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| import { Hono } from 'hono'; | ||
| import { AppEnv } from '../../types/appenv'; | ||
| import { AuthConfig, setAuthLevel } from '../../middleware/auth/routeAuth'; | ||
| import { resolvePreview } from '../../utils/previewResolver'; | ||
| import { buildUserWorkerUrl } from '../../utils/urls'; | ||
|
|
||
| /** | ||
| * Preview status routes - check if an app preview is available | ||
| * This directly checks sandbox/dispatcher without needing to access subdomain URLs | ||
| */ | ||
| export function setupPreviewProxyRoutes(app: Hono<AppEnv>): void { | ||
| // Check preview availability for an app (HEAD request only) | ||
| app.on('HEAD', '/api/apps/:id/preview-status', setAuthLevel(AuthConfig.public), async (c) => { | ||
| const env = c.env; | ||
| const appId = c.req.param('id'); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. SECURITY: Path Traversal Risk (Medium) The Recommendation: Add input validation to ensure const appId = c.req.param('id');
// Validate appId format (alphanumeric, hyphens, underscores only)
if (!/^[a-zA-Z0-9_-]+$/.test(appId)) {
return new Response(null, { status: 400 });
}
// Additional length check to prevent abuse
if (appId.length > 63) { // DNS subdomain limit
return new Response(null, { status: 400 });
} |
||
|
|
||
| try { | ||
| // Create a clean, isolated HEAD request for testing | ||
| // This ensures no user cookies or sensitive headers are forwarded | ||
| const testUrl = buildUserWorkerUrl(env, appId); | ||
| const cleanRequest = new Request(testUrl, { | ||
| method: 'HEAD', | ||
| }); | ||
|
|
||
| const result = await resolvePreview(appId, cleanRequest, env); | ||
|
|
||
| if (!result.available) { | ||
| return new Response(null, { status: 404 }); | ||
| } | ||
|
|
||
| // Return success with preview type header | ||
| const headers = new Headers(); | ||
| if (result.type) { | ||
| headers.set('X-Preview-Type', result.type); | ||
| headers.set('Access-Control-Expose-Headers', 'X-Preview-Type'); | ||
| } | ||
|
|
||
| return new Response(null, { status: 200, headers }); | ||
| } catch (error: unknown) { | ||
| const err = error as Error; | ||
| console.error('Preview status check error:', err); | ||
| return new Response(null, { status: 500 }); | ||
| } | ||
| }); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,15 +1,14 @@ | ||
| import { createLogger } from './logger'; | ||
| import { SmartCodeGeneratorAgent } from './agents/core/smartGeneratorAgent'; | ||
| import { isDispatcherAvailable } from './utils/dispatcherUtils'; | ||
| import { createApp } from './app'; | ||
| // import * as Sentry from '@sentry/cloudflare'; | ||
| // import { sentryOptions } from './observability/sentry'; | ||
| import { DORateLimitStore as BaseDORateLimitStore } from './services/rate-limit/DORateLimitStore'; | ||
| import { getPreviewDomain } from './utils/urls'; | ||
| import { proxyToAiGateway } from './services/aigateway-proxy/controller'; | ||
| import { isOriginAllowed } from './config/security'; | ||
| import { proxyToSandbox } from './services/sandbox/request-handler'; | ||
| import { handleGitProtocolRequest, isGitProtocolRequest } from './api/handlers/git-protocol'; | ||
| import { resolvePreview } from './utils/previewResolver'; | ||
|
|
||
| // Durable Object and Service exports | ||
| export { UserAppSandboxService, DeployerService } from './services/sandbox/sandboxSdkClient'; | ||
|
|
@@ -44,71 +43,40 @@ function setOriginControl(env: Env, request: Request, currentHeaders: Headers): | |
| async function handleUserAppRequest(request: Request, env: Env): Promise<Response> { | ||
| const url = new URL(request.url); | ||
| const { hostname } = url; | ||
| const appId = hostname.split('.')[0]; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. CODE QUALITY: No Input Validation on appId The Recommendation: Add validation consistent with the sandbox's const appId = hostname.split('.')[0];
// Validate appId (consistent with DNS subdomain limits and security)
if (!appId || appId.length > 63 || !/^[a-zA-Z0-9_-]+$/.test(appId)) {
logger.warn(`Invalid appId extracted from hostname: ${hostname}`);
return new Response('Invalid application identifier', { status: 400 });
} |
||
|
|
||
| logger.info(`Handling user app request for: ${hostname}`); | ||
|
|
||
| // 1. Attempt to proxy to a live development sandbox. | ||
| // proxyToSandbox doesn't consume the request body on a miss, so no clone is needed here. | ||
| const sandboxResponse = await proxyToSandbox(request, env); | ||
| if (sandboxResponse) { | ||
| logger.info(`Serving response from sandbox for: ${hostname}`); | ||
| // If it was a websocket upgrade, we need to return the response as is | ||
| if (sandboxResponse.headers.get('Upgrade')?.toLowerCase() === 'websocket') { | ||
| logger.info(`Serving websocket response from sandbox for: ${hostname}`); | ||
| return sandboxResponse; | ||
| } | ||
|
|
||
| // Add headers to identify this as a sandbox response | ||
| let headers = new Headers(sandboxResponse.headers); | ||
|
|
||
| if (sandboxResponse.status === 500) { | ||
| headers.set('X-Preview-Type', 'sandbox-error'); | ||
| } else { | ||
| headers.set('X-Preview-Type', 'sandbox'); | ||
| } | ||
| headers = setOriginControl(env, request, headers); | ||
| headers.append('Vary', 'Origin'); | ||
| headers.set('Access-Control-Expose-Headers', 'X-Preview-Type'); | ||
|
|
||
| return new Response(sandboxResponse.body, { | ||
| status: sandboxResponse.status, | ||
| statusText: sandboxResponse.statusText, | ||
| headers, | ||
| }); | ||
| // Use shared preview resolver to get the response | ||
| const result = await resolvePreview(appId, request, env); | ||
|
|
||
| if (!result.available || !result.response) { | ||
| logger.warn(`Preview not available for: ${appId}`); | ||
| const errorMessage = result.error || 'This application is not currently available.'; | ||
| return new Response(errorMessage, { status: 404 }); | ||
| } | ||
|
|
||
| // 2. If sandbox misses, attempt to dispatch to a deployed worker. | ||
| logger.info(`Sandbox miss for ${hostname}, attempting dispatch to permanent worker.`); | ||
| if (!isDispatcherAvailable(env)) { | ||
| logger.warn(`Dispatcher not available, cannot serve: ${hostname}`); | ||
| return new Response('This application is not currently available.', { status: 404 }); | ||
| // Handle websocket upgrades specially (return response as-is) | ||
| if (result.response.headers.get('Upgrade')?.toLowerCase() === 'websocket') { | ||
| logger.info(`Serving websocket response for: ${appId}`); | ||
| return result.response; | ||
| } | ||
|
|
||
| // Extract the app name (e.g., "xyz" from "xyz.build.cloudflare.dev"). | ||
| const appName = hostname.split('.')[0]; | ||
| const dispatcher = env['DISPATCHER']; | ||
|
|
||
| try { | ||
| const worker = dispatcher.get(appName); | ||
| const dispatcherResponse = await worker.fetch(request); | ||
|
|
||
| // Add headers to identify this as a dispatcher response | ||
| let headers = new Headers(dispatcherResponse.headers); | ||
|
|
||
| headers.set('X-Preview-Type', 'dispatcher'); | ||
| headers = setOriginControl(env, request, headers); | ||
| headers.append('Vary', 'Origin'); | ||
| headers.set('Access-Control-Expose-Headers', 'X-Preview-Type'); | ||
|
|
||
| return new Response(dispatcherResponse.body, { | ||
| status: dispatcherResponse.status, | ||
| statusText: dispatcherResponse.statusText, | ||
| headers, | ||
| }); | ||
| } catch (error: any) { | ||
| // This block catches errors if the binding doesn't exist or if worker.fetch() fails. | ||
| logger.warn(`Error dispatching to worker '${appName}': ${error.message}`); | ||
| return new Response('An error occurred while loading this application.', { status: 500 }); | ||
| // Add CORS and preview type headers to the response | ||
| let headers = new Headers(result.response.headers); | ||
|
|
||
| if (result.type) { | ||
| headers.set('X-Preview-Type', result.type); | ||
| } | ||
| headers = setOriginControl(env, request, headers); | ||
| headers.append('Vary', 'Origin'); | ||
| headers.set('Access-Control-Expose-Headers', 'X-Preview-Type'); | ||
|
|
||
| return new Response(result.response.body, { | ||
| status: result.response.status, | ||
| statusText: result.response.statusText, | ||
| headers, | ||
| }); | ||
| } | ||
|
|
||
| /** | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| import { proxyToSandbox } from '../services/sandbox/request-handler'; | ||
| import { isDispatcherAvailable } from './dispatcherUtils'; | ||
| import { createLogger } from '../logger'; | ||
|
|
||
| const logger = createLogger('PreviewResolver'); | ||
|
|
||
| export type PreviewType = 'sandbox' | 'sandbox-error' | 'dispatcher'; | ||
|
|
||
| export interface PreviewResult { | ||
| available: boolean; | ||
| type?: PreviewType; | ||
| response?: Response; | ||
| error?: string; | ||
| } | ||
|
|
||
| /** | ||
| * Resolve preview availability for an app by checking sandbox and dispatcher | ||
| * @param appId - The app identifier (subdomain) | ||
| * @param request - The incoming request | ||
| * @param env - Worker environment | ||
| * @returns PreviewResult with availability status and response | ||
| */ | ||
| export async function resolvePreview( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. SECURITY: Potential SSRF via Request Forwarding (High) The function accepts an arbitrary Current mitigation: The Recommendation: Ensure this function always sanitizes requests before forwarding: export async function resolvePreview(
appId: string,
request: Request,
env: Env,
sanitize: boolean = true // Add flag to enforce sanitization
): Promise<PreviewResult> {
// Create sanitized request for security-sensitive calls
const testRequest = sanitize ? new Request(request.url, {
method: request.method,
// Only forward safe headers
headers: new Headers({
'User-Agent': request.headers.get('User-Agent') || '',
'Accept': request.headers.get('Accept') || '',
}),
}) : request;
const sandboxResponse = await proxyToSandbox(testRequest, env);
// ...
} |
||
| appId: string, | ||
| request: Request, | ||
| env: Env | ||
| ): Promise<PreviewResult> { | ||
| // Try sandbox first | ||
| const sandboxResponse = await proxyToSandbox(request, env); | ||
| if (sandboxResponse) { | ||
| logger.info(`Preview available in sandbox for: ${appId}`); | ||
|
|
||
| const type: PreviewType = sandboxResponse.status === 500 ? 'sandbox-error' : 'sandbox'; | ||
|
|
||
| return { | ||
| available: sandboxResponse.status !== 500, | ||
| type, | ||
| response: sandboxResponse, | ||
| }; | ||
| } | ||
|
|
||
| // Try dispatcher (deployed worker) | ||
| logger.info(`Sandbox miss for ${appId}, attempting dispatch to permanent worker`); | ||
| if (!isDispatcherAvailable(env)) { | ||
| logger.warn(`Dispatcher not available, cannot serve: ${appId}`); | ||
| return { | ||
| available: false, | ||
| error: 'This application is not currently available.', | ||
| }; | ||
| } | ||
|
|
||
| try { | ||
| const dispatcher = env['DISPATCHER']; | ||
| const worker = dispatcher.get(appId); | ||
| const dispatcherResponse = await worker.fetch(request); | ||
|
|
||
| logger.info(`Preview available in dispatcher for: ${appId}`); | ||
|
|
||
| return { | ||
| available: dispatcherResponse.ok, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. CODE QUALITY: Inconsistent Error Handling The function returns Issue: If a dispatcher returns 403/404/500, this reports Recommendation: const dispatcherResponse = await worker.fetch(request);
logger.info(`Preview available in dispatcher for: ${appId}, status: ${dispatcherResponse.status}`);
if (!dispatcherResponse.ok) {
return {
available: false,
error: `Dispatcher returned status ${dispatcherResponse.status}`,
};
}
return {
available: true,
type: 'dispatcher',
response: dispatcherResponse,
}; |
||
| type: 'dispatcher', | ||
| response: dispatcherResponse, | ||
| }; | ||
| } catch (error: unknown) { | ||
| const err = error as Error; | ||
| logger.warn(`Error dispatching to worker '${appId}': ${err.message}`); | ||
| return { | ||
| available: false, | ||
| error: 'An error occurred while loading this application.', | ||
| }; | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SECURITY: Insufficient URL Validation (Medium)
The
appIdis extracted from a user-provided URL without validation before being used in an API request. A malicious URL could inject path traversal characters or special characters.Example attack: If
url = "https://../../admin.vibesdk.com/", the extracted appId would be../../admin, potentially causing issues.Recommendation: Add validation after extraction: