fix: MQ Wave 1 security hardening and code review fixes (#288)

Security hardening:
- Add RPC session token authentication (X-Session-Token header)
- Restrict healthcheck URLs to localhost only (SSRF prevention)
- Add log secret masking for file output (key=value, JWT patterns)
- Dashboard fetches and injects session token via Connect interceptor

Code review fixes:
- Fix operator precedence in isFileLockingError (gate on Windows first)
- Replace time.Sleep with context-aware select in retry loop
- Fix stderr double-wrapping on installer retry
- Propagate context through hook execution chain
- Add nil-check for LogsStoreFuncs in NewLogsHandler
- Extract updateRegistryWithRetry helper (dedup ~40 lines)
- Sanitize serviceName with filepath.Base in logbuffer

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: jongio

cli/dashboard/src/lib/connectClient.ts

@@ -20,7 +20,7 @@
  * below. Do NOT cache the client at module scope — the caller (typically
  * a hook with a stable transport reference) is responsible for memoising.
  */
-import { createClient, type Client, type Transport } from '@connectrpc/connect'
+import { createClient, type Client, type Interceptor, type Transport } from '@connectrpc/connect'
 import { createConnectTransport } from '@connectrpc/connect-web'
 
 import { LifecycleService } from '@/gen/proto/azdapp/v1/lifecycle_pb.js'
@@ -55,6 +55,40 @@ function defaultBaseUrl(): string {
 }
 
 let cachedDefaultTransport: Transport | null = null
+let cachedSessionToken: string | null = null
+
+/**
+ * Fetch the session token from the server. Called once and cached.
+ * The token is used to authenticate all RPC requests.
+ */
+async function getSessionToken(): Promise<string> {
+  if (cachedSessionToken !== null) {
+    return cachedSessionToken
+  }
+  try {
+    const resp = await fetch('/api/session-token')
+    if (resp.ok) {
+      cachedSessionToken = await resp.text()
+    }
+  } catch {
+    // Server may not support session tokens (e.g., test environments)
+  }
+  return cachedSessionToken ?? ''
+}
+
+/**
+ * Connect interceptor that attaches the session token to every outbound
+ * request. The token is fetched lazily on first use.
+ */
+function sessionTokenInterceptor(): Interceptor {
+  return (next) => async (req) => {
+    const token = await getSessionToken()
+    if (token) {
+      req.header.set('X-Session-Token', token)
+    }
+    return next(req)
+  }
+}
 
 /**
  * Returns the singleton dashboard transport. Construction is deferred to
@@ -74,6 +108,7 @@ export function getDefaultTransport(): Transport {
       // handlers (no protobuf-binary opt-in yet) and keeps wire payloads
       // inspectable in DevTools during the migration.
       useBinaryFormat: false,
+      interceptors: [sessionTokenInterceptor()],
     })
   }
   return cachedDefaultTransport

cli/src/cmd/app/commands/run.go

@@ -146,7 +146,7 @@ func runAzdMode(ctx context.Context, azureYamlPath, azureYamlDir string) error {
 	// Azure logs are now fetched on-demand via /api/azure/logs endpoint
 
 	// Execute prerun hook before starting services
-	if err = executePrerunHook(azureYaml, azureYamlDir); err != nil {
+	if err = executePrerunHook(ctx, azureYaml, azureYamlDir); err != nil {
 		return err
 	}
 

cli/src/cmd/app/commands/run_hooks.go

@@ -12,18 +12,18 @@ import (
 )
 
 // executePrerunHook executes the prerun hook if configured.
-func executePrerunHook(azureYaml *service.AzureYaml, workingDir string) error {
-	return executeHook(azureYaml, azureYaml.Hooks, azureYaml.Hooks.GetPrerun(), "prerun", workingDir)
+func executePrerunHook(ctx context.Context, azureYaml *service.AzureYaml, workingDir string) error {
+	return executeHook(ctx, azureYaml, azureYaml.Hooks, azureYaml.Hooks.GetPrerun(), "prerun", workingDir)
 }
 
 // executePostrunHook executes the postrun hook if configured.
-func executePostrunHook(azureYaml *service.AzureYaml, workingDir string) error {
-	return executeHook(azureYaml, azureYaml.Hooks, azureYaml.Hooks.GetPostrun(), "postrun", workingDir)
+func executePostrunHook(ctx context.Context, azureYaml *service.AzureYaml, workingDir string) error {
+	return executeHook(ctx, azureYaml, azureYaml.Hooks, azureYaml.Hooks.GetPostrun(), "postrun", workingDir)
 }
 
 // executeHook executes a lifecycle hook with the given name and configuration.
 // This is a common helper function to avoid duplication between prerun and postrun hooks.
-func executeHook(azureYaml *service.AzureYaml, hooks *service.Hooks, hook *service.Hook, hookName, workingDir string) error {
+func executeHook(ctx context.Context, azureYaml *service.AzureYaml, hooks *service.Hooks, hook *service.Hook, hookName, workingDir string) error {
 	if hooks == nil || hook == nil {
 		return nil // No hook configured
 	}
@@ -40,7 +40,7 @@ func executeHook(azureYaml *service.AzureYaml, hooks *service.Hooks, hook *servi
 	hookEnvVars := buildHookEnvironmentVariables(azureYaml, workingDir)
 	config.Env = hookEnvVars
 
-	return executor.ExecuteHook(context.Background(), hookName, *config, workingDir)
+	return executor.ExecuteHook(ctx, hookName, *config, workingDir)
 }
 
 // buildHookEnvironmentVariables builds environment variables to pass to hooks

cli/src/cmd/app/commands/run_hooks_test.go

@@ -48,7 +48,7 @@ services:
 		t.Fatalf("Failed to parse azure.yaml: %v", err)
 	}
 
-	err = executePrerunHook(azureYaml, tmpDir)
+	err = executePrerunHook(context.Background(), azureYaml, tmpDir)
 	if err != nil {
 		t.Errorf("Expected prerun hook to succeed, got error: %v", err)
 	}
@@ -91,7 +91,7 @@ services:
 		t.Fatalf("Failed to parse azure.yaml: %v", err)
 	}
 
-	err = executePrerunHook(azureYaml, tmpDir)
+	err = executePrerunHook(context.Background(), azureYaml, tmpDir)
 	if err == nil {
 		t.Error("Expected prerun hook to fail, but it succeeded")
 	}
@@ -134,7 +134,7 @@ services:
 		t.Fatalf("Failed to parse azure.yaml: %v", err)
 	}
 
-	err = executePrerunHook(azureYaml, tmpDir)
+	err = executePrerunHook(context.Background(), azureYaml, tmpDir)
 	if err != nil {
 		t.Errorf("Expected prerun hook to continue on error, got: %v", err)
 	}
@@ -174,7 +174,7 @@ services:
 		t.Fatalf("Failed to parse azure.yaml: %v", err)
 	}
 
-	err = executePostrunHook(azureYaml, tmpDir)
+	err = executePostrunHook(context.Background(), azureYaml, tmpDir)
 	if err != nil {
 		t.Errorf("Expected postrun hook to succeed, got error: %v", err)
 	}
@@ -217,7 +217,7 @@ services:
 		t.Fatalf("Failed to parse azure.yaml: %v", err)
 	}
 
-	err = executePostrunHook(azureYaml, tmpDir)
+	err = executePostrunHook(context.Background(), azureYaml, tmpDir)
 	if err == nil {
 		t.Error("Expected postrun hook to fail, but it succeeded")
 	}
@@ -245,12 +245,12 @@ services:
 	}
 
 	// Should not error when no hooks configured
-	err = executePrerunHook(azureYaml, tmpDir)
+	err = executePrerunHook(context.Background(), azureYaml, tmpDir)
 	if err != nil {
 		t.Errorf("Expected no error when hooks not configured, got: %v", err)
 	}
 
-	err = executePostrunHook(azureYaml, tmpDir)
+	err = executePostrunHook(context.Background(), azureYaml, tmpDir)
 	if err != nil {
 		t.Errorf("Expected no error when hooks not configured, got: %v", err)
 	}
@@ -492,7 +492,7 @@ services:
 	}
 
 	// Execute prerun hook - it should have access to environment variables
-	err = executePrerunHook(azureYaml, tmpDir)
+	err = executePrerunHook(context.Background(), azureYaml, tmpDir)
 	if err != nil {
 		t.Logf("Hook execution error (may be platform-specific): %v", err)
 		// Don't fail the test - the environment variable logic may vary by platform
@@ -546,7 +546,7 @@ services:
 
 	done := make(chan error, 1)
 	go func() {
-		done <- executePrerunHook(azureYaml, tmpDir)
+		done <- executePrerunHook(context.Background(), azureYaml, tmpDir)
 	}()
 
 	select {
@@ -600,7 +600,7 @@ services:
 	}
 
 	// Execute prerun hook - should use platform-specific command
-	err = executePrerunHook(azureYaml, tmpDir)
+	err = executePrerunHook(context.Background(), azureYaml, tmpDir)
 	if err != nil {
 		t.Errorf("Expected platform-specific hook to succeed, got error: %v", err)
 	}

cli/src/cmd/app/commands/run_orchestration.go

@@ -50,7 +50,7 @@ func executeAndMonitorServices(ctx context.Context, runtimes []*service.ServiceR
 	logger.LogReady()
 
 	// Execute postrun hook after all services are ready
-	if err := executePostrunHook(azureYaml, azureYamlDir); err != nil {
+	if err := executePostrunHook(ctx, azureYaml, azureYamlDir); err != nil {
 		cliout.Warning("Postrun hook failed but services are running: %v", err)
 	}
 
@@ -257,25 +257,8 @@ func monitorServiceProcess(ctx context.Context, wg *sync.WaitGroup, serviceName
 
 		if result.err != nil {
 			// Update registry to trigger OS notification via state monitor
-			// CRITICAL FIX: Implement retry logic for registry updates
-			maxRetries := 3
-			retryDelay := 100 * time.Millisecond
-			var regErr error
-			for i := 0; i < maxRetries; i++ {
-				regErr = reg.UpdateStatus(serviceName, "error")
-				if regErr == nil {
-					break
-				}
-				if i < maxRetries-1 {
-					time.Sleep(retryDelay)
-					retryDelay *= 2 // Exponential backoff
-				}
-			}
-
-			if regErr != nil {
-				cliout.Error("Failed to update registry for %s after %d retries: %v", serviceName, maxRetries, regErr)
-				// As fallback, try to send direct notification if notification manager is available
-				// This ensures users are informed even if registry update fails
+			if regErr := updateRegistryWithRetry(reg, serviceName, "error"); regErr != nil {
+				cliout.Error("Failed to update registry for %s after retries: %v", serviceName, regErr)
 			}
 
 			// Show mode-appropriate error message
@@ -305,23 +288,8 @@ func monitorServiceProcess(ctx context.Context, wg *sync.WaitGroup, serviceName
 				cliout.Info("Service %s exited cleanly", serviceName)
 			}
 
-			// CRITICAL FIX: Implement retry logic for clean exit registry updates
-			maxRetries := 3
-			retryDelay := 100 * time.Millisecond
-			var regErr error
-			for i := 0; i < maxRetries; i++ {
-				regErr = reg.UpdateStatus(serviceName, status)
-				if regErr == nil {
-					break
-				}
-				if i < maxRetries-1 {
-					time.Sleep(retryDelay)
-					retryDelay *= 2 // Exponential backoff
-				}
-			}
-
-			if regErr != nil {
-				cliout.Warning("Failed to update registry for %s after %d retries: %v", serviceName, maxRetries, regErr)
+			if regErr := updateRegistryWithRetry(reg, serviceName, status); regErr != nil {
+				cliout.Warning("Failed to update registry for %s after retries: %v", serviceName, regErr)
 			}
 		}
 		// Intentionally don't cancel context - other services should continue
@@ -402,3 +370,21 @@ func runAspireMode(ctx context.Context, rootDir string) error {
 	// Run dotnet and let it handle everything (inherits all azd env vars)
 	return executor.StartCommand(ctx, "dotnet", args, aspireProject.Dir)
 }
+
+// updateRegistryWithRetry updates the service registry with retry and exponential backoff.
+func updateRegistryWithRetry(reg *registry.ServiceRegistry, serviceName, status string) error {
+	const maxRetries = 3
+	retryDelay := 100 * time.Millisecond
+	var err error
+	for i := 0; i < maxRetries; i++ {
+		err = reg.UpdateStatus(serviceName, status)
+		if err == nil {
+			return nil
+		}
+		if i < maxRetries-1 {
+			time.Sleep(retryDelay)
+			retryDelay *= 2
+		}
+	}
+	return err
+}

cli/src/internal/dashboard/server_core.go

@@ -14,6 +14,7 @@ import (
 	"github.com/jongio/azd-app/cli/src/internal/constants"
 	"github.com/jongio/azd-app/cli/src/internal/dashboard/broadcast"
 	"github.com/jongio/azd-app/cli/src/internal/portmanager"
+	"github.com/jongio/azd-app/cli/src/internal/rpc"
 	"github.com/jongio/azd-app/cli/src/internal/service"
 )
 
@@ -36,6 +37,7 @@ type Server struct {
 	currentMode  service.LogMode // Current log source mode (local or azure)
 	modeMu       sync.RWMutex    // Protect currentMode
 	azureYamlMu  sync.RWMutex    // Protect azure.yaml read/write across Connect handlers
+	sessionToken string          // Per-session auth token for RPC endpoints
 
 	// broadcast fans coarse-grained UI events out to Connect
 	// StreamBroadcast subscribers. See cli/src/internal/dashboard/broadcast
@@ -58,12 +60,13 @@ func GetServer(projectDir string) *Server {
 
 	// Create new server instance for this project
 	srv := &Server{
-		port:        0, // Will be assigned by port manager
-		mux:         http.NewServeMux(),
-		projectDir:  absPath,
-		stopChan:    make(chan struct{}),
-		currentMode: service.LogModeLocal, // Default to local mode
-		broadcast:   broadcast.New(),
+		port:         0, // Will be assigned by port manager
+		mux:          http.NewServeMux(),
+		projectDir:   absPath,
+		stopChan:     make(chan struct{}),
+		currentMode:  service.LogModeLocal, // Default to local mode
+		broadcast:    broadcast.New(),
+		sessionToken: rpc.GenerateSessionToken(),
 	}
 	srv.setupRoutes()
 	servers[key] = srv

cli/src/internal/dashboard/server_routes.go

@@ -32,8 +32,9 @@ func (s *Server) setupRoutes() {
 	// All legacy /api/* REST handlers and /api/ws WebSocket have been
 	// removed; dashboard clients use the azdapp.v1.* Connect services.
 	rpc.Mount(s.mux, rpc.Dependencies{
-		Broadcast:  s.broadcast,
-		Version:    version.Version,
+		Broadcast:    s.broadcast,
+		Version:      version.Version,
+		SessionToken: s.sessionToken,
 		Project:    rpc.ProjectSourceFunc(service.ParseAzureYaml),
 		ProjectDir: s.projectDir,
 		Mode: rpc.ModeStoreFuncs{
@@ -81,6 +82,14 @@ func (s *Server) setupRoutes() {
 		Azure: newAzureStoreFuncs(s),
 	})
 
+	// Serve session token for dashboard authentication.
+	// The React app fetches this on startup and includes it in all RPC calls.
+	s.mux.HandleFunc("/api/session-token", func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "text/plain")
+		w.Header().Set("Cache-Control", "no-store")
+		_, _ = w.Write([]byte(s.sessionToken))
+	})
+
 	// Pre-read index.html once for SPA client-side routing fallback
 	indexContent, indexReadErr := fs.ReadFile(distFS, "index.html")
 

cli/src/internal/healthcheck/checker_http.go

@@ -5,14 +5,47 @@ import (
 	"encoding/json"
 	"fmt"
 	"io"
+	"net"
 	"net/http"
+	"net/url"
 	"strings"
 	"time"
 
 	"log/slog"
 )
 
+// isLocalhostURL checks if a URL targets localhost/loopback addresses only.
+// Health check URLs from azure.yaml are restricted to local services to prevent SSRF.
+func isLocalhostURL(urlStr string) error {
+	parsed, err := url.Parse(urlStr)
+	if err != nil {
+		return fmt.Errorf("invalid URL: %w", err)
+	}
+
+	host := parsed.Hostname()
+	if host == "localhost" || host == "127.0.0.1" || host == "::1" || host == "[::1]" {
+		return nil
+	}
+
+	// Check if it resolves to a loopback address
+	ip := net.ParseIP(host)
+	if ip != nil && ip.IsLoopback() {
+		return nil
+	}
+
+	return fmt.Errorf("health check URL must target localhost (got %q) - non-local URLs are blocked for security", host)
+}
+
 func (c *HealthChecker) performHTTPCheck(ctx context.Context, urlStr string) *httpHealthCheckResult {
+	// Restrict health check URLs to localhost to prevent SSRF
+	if err := isLocalhostURL(urlStr); err != nil {
+		return &httpHealthCheckResult{
+			Endpoint: urlStr,
+			Status:   HealthStatusUnhealthy,
+			Error:    err.Error(),
+		}
+	}
+
 	startTime := time.Now()
 	req, err := http.NewRequestWithContext(ctx, "GET", urlStr, nil)
 	if err != nil {