Patch chromium flag endpoint

server/cmd/api/api/api_test.go

@@ -302,3 +302,36 @@ func newMockNekoClient(t *testing.T) *nekoclient.AuthClient {
 	require.NoError(t, err)
 	return client
 }
+
+func TestApiService_PatchChromiumFlags(t *testing.T) {
+	ctx := context.Background()
+	mgr := recorder.NewFFmpegManager()
+	svc, err := New(mgr, newMockFactory(), newTestUpstreamManager(), scaletozero.NewNoopController(), newMockNekoClient(t))
+	require.NoError(t, err)
+
+	// Test with valid flags
+	flags := []string{"--kiosk", "--start-maximized"}
+	body := &oapi.PatchChromiumFlagsJSONRequestBody{
+		Flags: flags,
+	}
+
+	req := oapi.PatchChromiumFlagsRequestObject{
+		Body: body,
+	}
+
+	// This will fail to write to /chromium/flags in most test environments
+	// but we're mainly testing that the handler accepts valid input
+	resp, err := svc.PatchChromiumFlags(ctx, req)
+	require.NoError(t, err)
+
+	// We expect either success or an error about creating the directory
+	// depending on the test environment
+	switch resp.(type) {
+	case oapi.PatchChromiumFlags200Response:
+		// Success in environments where /chromium is writable
+	case oapi.PatchChromiumFlags500JSONResponse:
+		// Expected in most test environments where /chromium doesn't exist
+	default:
+		t.Fatalf("unexpected response type: %T", resp)
+	}
+}

server/cmd/api/api/chromium.go

@@ -169,41 +169,79 @@ func (s *ApiService) UploadExtensionsAndRestart(ctx context.Context, request oap
 		paths = append(paths, filepath.Join(extBase, p.name))
 	}
 
-	// Read existing runtime flags from /chromium/flags (if any)
+	// Create new flags for the uploaded extensions
+	newTokens := []string{
+		fmt.Sprintf("--disable-extensions-except=%s", strings.Join(paths, ",")),
+		fmt.Sprintf("--load-extension=%s", strings.Join(paths, ",")),
+	}
+
+	// Merge and write flags
+	if _, err := s.mergeAndWriteChromiumFlags(ctx, newTokens); err != nil {
+		return oapi.UploadExtensionsAndRestart500JSONResponse{
+			InternalErrorJSONResponse: oapi.InternalErrorJSONResponse{Message: err.Error()},
+		}, nil
+	}
+
+	// Restart Chromium and wait for DevTools to be ready
+	if err := s.restartChromiumAndWait(ctx, "extension upload"); err != nil {
+		return oapi.UploadExtensionsAndRestart500JSONResponse{
+			InternalErrorJSONResponse: oapi.InternalErrorJSONResponse{Message: err.Error()},
+		}, nil
+	}
+
+	log.Info("devtools ready", "elapsed", time.Since(start).String())
+	return oapi.UploadExtensionsAndRestart201Response{}, nil
+}
+
+// mergeAndWriteChromiumFlags reads existing flags, merges them with new flags,
+// and writes the result back to /chromium/flags. Returns the merged tokens or an error.
+func (s *ApiService) mergeAndWriteChromiumFlags(ctx context.Context, newTokens []string) ([]string, error) {
+	log := logger.FromContext(ctx)
+
 	const flagsPath = "/chromium/flags"
+
+	// Read existing runtime flags from /chromium/flags (if any)
 	existingTokens, err := chromiumflags.ReadOptionalFlagFile(flagsPath)
 	if err != nil {
 		log.Error("failed to read existing flags", "error", err)
-		return oapi.UploadExtensionsAndRestart500JSONResponse{InternalErrorJSONResponse: oapi.InternalErrorJSONResponse{Message: "failed to read existing flags"}}, nil
+		return nil, fmt.Errorf("failed to read existing flags: %w", err)
 	}
 
-	// Create new flags for the uploaded extensions
-	newTokens := []string{
-		fmt.Sprintf("--disable-extensions-except=%s", strings.Join(paths, ",")),
-		fmt.Sprintf("--load-extension=%s", strings.Join(paths, ",")),
-	}
+	log.Info("merging flags", "existing", existingTokens, "new", newTokens)
 
-	// Merge existing flags with new extension flags using token-aware API
+	// Merge existing flags with new flags using token-aware API
 	mergedTokens := chromiumflags.MergeFlags(existingTokens, newTokens)
 
+	// Ensure the chromium directory exists
 	if err := os.MkdirAll("/chromium", 0o755); err != nil {
 		log.Error("failed to create chromium dir", "error", err)
-		return oapi.UploadExtensionsAndRestart500JSONResponse{InternalErrorJSONResponse: oapi.InternalErrorJSONResponse{Message: "failed to create chromium dir"}}, nil
+		return nil, fmt.Errorf("failed to create chromium dir: %w", err)
 	}
+
 	// Write flags file with merged flags
 	if err := chromiumflags.WriteFlagFile(flagsPath, mergedTokens); err != nil {
-		log.Error("failed to write overlay flags", "error", err)
-		return oapi.UploadExtensionsAndRestart500JSONResponse{InternalErrorJSONResponse: oapi.InternalErrorJSONResponse{Message: "failed to write overlay flags"}}, nil
+		log.Error("failed to write flags", "error", err)
+		return nil, fmt.Errorf("failed to write flags: %w", err)
 	}
 
+	log.Info("flags written", "merged", mergedTokens)
+	return mergedTokens, nil
+}
+
+// restartChromiumAndWait restarts Chromium via supervisorctl and waits for DevTools to be ready.
+// Returns an error if the restart fails or times out.
+func (s *ApiService) restartChromiumAndWait(ctx context.Context, operation string) error {
+	log := logger.FromContext(ctx)
+	start := time.Now()
+
 	// Begin listening for devtools URL updates, since we are about to restart Chromium
 	updates, cancelSub := s.upstreamMgr.Subscribe()
 	defer cancelSub()
 
 	// Run supervisorctl restart with a new context to let it run beyond the lifetime of the http request.
 	// This lets us return as soon as the DevTools URL is updated.
 	errCh := make(chan error, 1)
-	log.Info("restarting chromium via supervisorctl")
+	log.Info("restarting chromium via supervisorctl", "operation", operation)
 	go func() {
 		cmdCtx, cancelCmd := context.WithTimeout(context.WithoutCancel(ctx), 1*time.Minute)
 		defer cancelCmd()
@@ -219,12 +257,60 @@ func (s *ApiService) UploadExtensionsAndRestart(ctx context.Context, request oap
 	defer timeout.Stop()
 	select {
 	case <-updates:
-		log.Info("devtools ready", "elapsed", time.Since(start).String())
-		return oapi.UploadExtensionsAndRestart201Response{}, nil
+		log.Info("devtools ready", "operation", operation, "elapsed", time.Since(start).String())
+		return nil
 	case err := <-errCh:
-		return oapi.UploadExtensionsAndRestart500JSONResponse{InternalErrorJSONResponse: oapi.InternalErrorJSONResponse{Message: err.Error()}}, nil
+		return err
 	case <-timeout.C:
-		log.Info("devtools not ready in time", "elapsed", time.Since(start).String())
-		return oapi.UploadExtensionsAndRestart500JSONResponse{InternalErrorJSONResponse: oapi.InternalErrorJSONResponse{Message: "devtools not ready in time"}}, nil
+		log.Info("devtools not ready in time", "operation", operation, "elapsed", time.Since(start).String())
+		return fmt.Errorf("devtools not ready in time")
+	}
+}
+
+// PatchChromiumFlags handles updating Chromium launch flags at runtime.
+// It merges the provided flags with existing flags in /chromium/flags, writes the updated
+// flags file, restarts Chromium via supervisord, and waits until DevTools is ready.
+func (s *ApiService) PatchChromiumFlags(ctx context.Context, request oapi.PatchChromiumFlagsRequestObject) (oapi.PatchChromiumFlagsResponseObject, error) {
+	log := logger.FromContext(ctx)
+	start := time.Now()
+	log.Info("patch chromium flags: begin")
+
+	s.stz.Disable(ctx)
+	defer s.stz.Enable(ctx)
+
+	if request.Body == nil {
+		return oapi.PatchChromiumFlags400JSONResponse{BadRequestErrorJSONResponse: oapi.BadRequestErrorJSONResponse{Message: "request body required"}}, nil
+	}
+
+	if len(request.Body.Flags) == 0 {
+		return oapi.PatchChromiumFlags400JSONResponse{BadRequestErrorJSONResponse: oapi.BadRequestErrorJSONResponse{Message: "at least one flag required"}}, nil
+	}
+
+	// Validate flags - they should start with "--"
+	for _, flag := range request.Body.Flags {
+		trimmed := strings.TrimSpace(flag)
+		if trimmed == "" {
+			return oapi.PatchChromiumFlags400JSONResponse{BadRequestErrorJSONResponse: oapi.BadRequestErrorJSONResponse{Message: "empty flag provided"}}, nil
+		}
+		if !strings.HasPrefix(trimmed, "--") {
+			return oapi.PatchChromiumFlags400JSONResponse{BadRequestErrorJSONResponse: oapi.BadRequestErrorJSONResponse{Message: fmt.Sprintf("invalid flag format: %s (must start with --)", flag)}}, nil
+		}
+	}
+
+	// Merge and write flags
+	if _, err := s.mergeAndWriteChromiumFlags(ctx, request.Body.Flags); err != nil {
+		return oapi.PatchChromiumFlags500JSONResponse{
+			InternalErrorJSONResponse: oapi.InternalErrorJSONResponse{Message: err.Error()},
+		}, nil
 	}
+
+	// Restart Chromium and wait for DevTools to be ready
+	if err := s.restartChromiumAndWait(ctx, "flags update"); err != nil {
+		return oapi.PatchChromiumFlags500JSONResponse{
+			InternalErrorJSONResponse: oapi.InternalErrorJSONResponse{Message: err.Error()},
+		}, nil
+	}
+
+	log.Info("devtools ready after flags update", "elapsed", time.Since(start).String())
+	return oapi.PatchChromiumFlags200Response{}, nil
 }

server/cmd/api/api/display.go

@@ -8,7 +8,6 @@ import (
 	"os/exec"
 	"strconv"
 	"strings"
-	"time"
 
 	nekooapi "github.com/m1k1o/neko/server/lib/oapi"
 	"github.com/onkernel/kernel-images/server/lib/logger"
@@ -99,7 +98,9 @@ func (s *ApiService) PatchDisplay(ctx context.Context, req oapi.PatchDisplayRequ
 			err = s.setResolutionXorgViaXrandr(ctx, width, height, refreshRate, restartChrome)
 		}
 		if err == nil && restartChrome {
-			s.restartChromium(ctx)
+			if restartErr := s.restartChromiumAndWait(ctx, "resolution change"); restartErr != nil {
+				log.Error("failed to restart chromium after resolution change", "error", restartErr)
+			}
 		}
 	} else {
 		log.Info("using Xvfb restart for resolution change")
@@ -145,42 +146,6 @@ func (s *ApiService) detectDisplayMode(ctx context.Context) string {
 	return "xorg"
 }
 
-// restartChromium restarts the Chromium browser via supervisorctl and waits for DevTools to be ready
-func (s *ApiService) restartChromium(ctx context.Context) {
-	log := logger.FromContext(ctx)
-	start := time.Now()
-
-	// Begin listening for devtools URL updates, since we are about to restart Chromium
-	updates, cancelSub := s.upstreamMgr.Subscribe()
-	defer cancelSub()
-
-	// Run supervisorctl restart with a new context to let it run beyond the lifetime of the http request.
-	// This lets us return as soon as the DevTools URL is updated.
-	errCh := make(chan error, 1)
-	log.Info("restarting chromium via supervisorctl")
-	go func() {
-		cmdCtx, cancelCmd := context.WithTimeout(context.WithoutCancel(ctx), 1*time.Minute)
-		defer cancelCmd()
-		out, err := exec.CommandContext(cmdCtx, "supervisorctl", "-c", "/etc/supervisor/supervisord.conf", "restart", "chromium").CombinedOutput()
-		if err != nil {
-			log.Error("failed to restart chromium", "error", err, "out", string(out))
-			errCh <- fmt.Errorf("supervisorctl restart failed: %w", err)
-		}
-	}()
-
-	// Wait for either a new upstream, a restart error, or timeout
-	timeout := time.NewTimer(15 * time.Second)
-	defer timeout.Stop()
-	select {
-	case <-updates:
-		log.Info("chromium devtools ready after resolution change", "elapsed", time.Since(start).String())
-	case err := <-errCh:
-		log.Error("chromium restart failed", "error", err, "elapsed", time.Since(start).String())
-	case <-timeout.C:
-		log.Warn("chromium devtools not ready in time after resolution change", "elapsed", time.Since(start).String())
-	}
-}
-
 // setResolutionXorgViaXrandr changes resolution for Xorg using xrandr (fallback when Neko is disabled)
 func (s *ApiService) setResolutionXorgViaXrandr(ctx context.Context, width, height, refreshRate int, restartChrome bool) error {
 	log := logger.FromContext(ctx)
@@ -287,7 +252,9 @@ func (s *ApiService) setResolutionXvfb(ctx context.Context, width, height int, r
 	s.ProcessExec(ctx, oapi.ProcessExecRequestObject{Body: &waitReq})
 
 	if restartChrome {
-		s.restartChromium(ctx)
+		if restartErr := s.restartChromiumAndWait(ctx, "xvfb resolution change"); restartErr != nil {
+			log.Error("failed to restart chromium after xvfb resolution change", "error", restartErr)
+		}
 	}
 
 	log.Info("Xvfb resolution updated", "width", width, "height", height)