diff --git a/.changeset/fix-endpoints-health-status.md b/.changeset/fix-endpoints-health-status.md new file mode 100644 index 000000000..d42ea0546 --- /dev/null +++ b/.changeset/fix-endpoints-health-status.md @@ -0,0 +1,12 @@ +--- +'@workflow/core': patch +'@workflow/web': patch +--- + +Fix endpoints health status component based on code review feedback + +- Fixed base URL determination to handle non-local backends (vercel, postgres) +- Improved config key generation to include all relevant backend-specific fields +- Fixed useEffect dependencies to prevent unnecessary re-checks +- Added cleanup handler for unmounted component to prevent React warnings +- Documented CORS security implications in code comments diff --git a/packages/core/src/runtime.ts b/packages/core/src/runtime.ts index e0825ddb8..785943f7e 100644 --- a/packages/core/src/runtime.ts +++ b/packages/core/src/runtime.ts @@ -266,6 +266,12 @@ async function getAllWorkflowRunEvents(runId: string): Promise { /** * CORS headers for health check responses. * Allows the observability UI to check endpoint health from a different origin. + * + * Security Note: Uses wildcard 'Access-Control-Allow-Origin: *' to allow any origin + * to access the health check endpoints. This is acceptable for health checks as they + * only return a simple status message and do not expose sensitive information or allow + * any state-changing operations. The health check responses contain no user data, + * authentication tokens, or system configuration details. */ const HEALTH_CHECK_CORS_HEADERS = { 'Access-Control-Allow-Origin': '*', diff --git a/packages/web/src/components/display-utils/endpoints-health-status.tsx b/packages/web/src/components/display-utils/endpoints-health-status.tsx index bad42f398..ee83b77a2 100644 --- a/packages/web/src/components/display-utils/endpoints-health-status.tsx +++ b/packages/web/src/components/display-utils/endpoints-health-status.tsx @@ -22,6 +22,8 @@ interface HealthCheckResult { } const HEALTH_CHECK_SESSION_KEY = 'workflow-endpoints-health-check'; +const HEALTH_CHECK_NOT_SUPPORTED_MESSAGE = + 'Health checks not supported for this backend'; function getSessionHealthCheck(configKey: string): HealthCheckResult | null { try { @@ -51,14 +53,61 @@ function setSessionHealthCheck( } } +/** + * Simple hash function for creating a unique identifier from a string. + * Uses a basic djb2 hash algorithm to avoid exposing sensitive data in cache keys. + */ +function simpleHash(str: string): string { + let hash = 5381; + for (let i = 0; i < str.length; i++) { + hash = (hash << 5) + hash + str.charCodeAt(i); + } + return (hash >>> 0).toString(36); +} + function getConfigKey(config: WorldConfig): string { - // Create a unique key based on relevant config values - return `${config.backend || 'local'}-${config.port || '3000'}`; + // Create a unique key based on all relevant config values that uniquely identify the backend + // Include backend, port, and backend-specific fields + const keyObj: Record = { + backend: config.backend || 'local', + port: config.port || '3000', + }; + + // Add backend-specific fields + if (config.env) keyObj.env = config.env; + if (config.project) keyObj.project = config.project; + if (config.team) keyObj.team = config.team; + if (config.dataDir) keyObj.dataDir = config.dataDir; + // Hash the postgres URL to avoid exposing credentials in session storage + if (config.postgresUrl) { + keyObj.postgresUrlHash = simpleHash(config.postgresUrl); + } + + return JSON.stringify(keyObj); +} + +function getBaseUrl(config: WorldConfig): string | null { + const backend = config.backend || 'local'; + + if (backend === 'local') { + const port = config.port || '3000'; + return `http://localhost:${port}`; + } + + // For Vercel backend, we can't perform health checks from the browser + // as the endpoints are not accessible via HTTP + if (backend === 'vercel') { + return null; + } + + // For other backends like postgres, also not directly accessible + return null; } async function checkEndpointHealth( baseUrl: string, - endpoint: 'flow' | 'step' + endpoint: 'flow' | 'step', + signal?: AbortSignal ): Promise<{ success: boolean; message: string }> { try { const url = new URL( @@ -67,8 +116,8 @@ async function checkEndpointHealth( ); const response = await fetch(url.toString(), { method: 'POST', - // Short timeout for health checks - signal: AbortSignal.timeout(5000), + // Use provided signal or default to 5 second timeout + signal: signal || AbortSignal.timeout(5000), }); if (response.ok) { @@ -103,33 +152,72 @@ export function EndpointsHealthStatus({ config }: EndpointsHealthStatusProps) { return; } + // Determine base URL based on config + const baseUrl = getBaseUrl(config); + + // If backend doesn't support health checks (e.g., Vercel, Postgres), + // don't perform the check + if (!baseUrl) { + const result: HealthCheckResult = { + flow: 'error', + step: 'error', + flowMessage: HEALTH_CHECK_NOT_SUPPORTED_MESSAGE, + stepMessage: HEALTH_CHECK_NOT_SUPPORTED_MESSAGE, + checkedAt: new Date().toISOString(), + }; + setHealthCheck(result); + setSessionHealthCheck(configKey, result); + return; + } + // Otherwise, perform the health check + const abortController = new AbortController(); + const performHealthCheck = async () => { setIsChecking(true); - // Determine base URL based on config - const port = config.port || '3000'; - const baseUrl = `http://localhost:${port}`; + try { + const [flowResult, stepResult] = await Promise.all([ + checkEndpointHealth(baseUrl, 'flow', abortController.signal), + checkEndpointHealth(baseUrl, 'step', abortController.signal), + ]); - const [flowResult, stepResult] = await Promise.all([ - checkEndpointHealth(baseUrl, 'flow'), - checkEndpointHealth(baseUrl, 'step'), - ]); + // Check if request was aborted (component unmounted) + if (abortController.signal.aborted) { + return; + } - const result: HealthCheckResult = { - flow: flowResult.success ? 'success' : 'error', - step: stepResult.success ? 'success' : 'error', - flowMessage: flowResult.message, - stepMessage: stepResult.message, - checkedAt: new Date().toISOString(), - }; + const result: HealthCheckResult = { + flow: flowResult.success ? 'success' : 'error', + step: stepResult.success ? 'success' : 'error', + flowMessage: flowResult.message, + stepMessage: stepResult.message, + checkedAt: new Date().toISOString(), + }; - setHealthCheck(result); - setSessionHealthCheck(configKey, result); - setIsChecking(false); + setHealthCheck(result); + setSessionHealthCheck(configKey, result); + } catch (error) { + // If aborted, don't update state (component unmounted) + if (abortController.signal.aborted) { + return; + } + // Otherwise, log unexpected error + console.error('Unexpected error during health check:', error); + } finally { + // Always reset loading state unless aborted + if (!abortController.signal.aborted) { + setIsChecking(false); + } + } }; performHealthCheck(); + + // Cleanup function to cancel in-flight requests + return () => { + abortController.abort(); + }; }, [config]); const allSuccess =