Address code review feedback on endpoint health status component

.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

packages/core/src/runtime.ts

@@ -266,6 +266,12 @@ async function getAllWorkflowRunEvents(runId: string): Promise<Event[]> {
 /**
  * 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': '*',

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<string, unknown> = {
+    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 =