HH-743: harden browser image and lifecycle (#5)

cmd/control-plane/main.go

@@ -16,7 +16,17 @@ func main() {
 	if err != nil {
 		log.Fatal(err)
 	}
+	server := &http.Server{
+		Addr:              env("LISTEN_ADDR", ":8080"),
+		Handler:           newHandler(gateway, env("WEB_DIR", "web/dist")),
+		ReadHeaderTimeout: 5 * time.Second,
+		IdleTimeout:       60 * time.Second,
+	}
+	log.Printf("CreatorHub control plane listening on %s", server.Addr)
+	log.Fatal(server.ListenAndServe())
+}
 
+func newHandler(gateway *url.URL, webDirectory string) http.Handler {
 	proxy := httputil.NewSingleHostReverseProxy(gateway)
 	originalDirector := proxy.Director
 	proxy.Director = func(request *http.Request) {
@@ -31,16 +41,8 @@ func main() {
 	mux.HandleFunc("GET /healthz", func(response http.ResponseWriter, _ *http.Request) {
 		response.WriteHeader(http.StatusNoContent)
 	})
-	mux.Handle("/", spaHandler(env("WEB_DIR", "web/dist")))
-
-	server := &http.Server{
-		Addr:              env("LISTEN_ADDR", ":8080"),
-		Handler:           mux,
-		ReadHeaderTimeout: 5 * time.Second,
-		IdleTimeout:       60 * time.Second,
-	}
-	log.Printf("CreatorHub control plane listening on %s", server.Addr)
-	log.Fatal(server.ListenAndServe())
+	mux.Handle("/", spaHandler(webDirectory))
+	return mux
 }
 
 func spaHandler(directory string) http.Handler {

cmd/control-plane/main_test.go

@@ -0,0 +1,33 @@
+package main
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"testing"
+)
+
+func TestAPIProxyPreservesMethodPathAndStatus(t *testing.T) {
+	requests := make(chan *http.Request, 1)
+	gateway := httptest.NewServer(http.HandlerFunc(func(response http.ResponseWriter, request *http.Request) {
+		requests <- request
+		response.WriteHeader(http.StatusServiceUnavailable)
+	}))
+	defer gateway.Close()
+
+	upstream, err := url.Parse(gateway.URL)
+	if err != nil {
+		t.Fatal(err)
+	}
+	request := httptest.NewRequest(http.MethodPost, "/api/browsers/account-a/start", nil)
+	response := httptest.NewRecorder()
+	newHandler(upstream, t.TempDir()).ServeHTTP(response, request)
+
+	proxied := <-requests
+	if proxied.Method != http.MethodPost || proxied.URL.Path != "/v1/browsers/account-a/start" {
+		t.Fatalf("unexpected proxy request %s %s", proxied.Method, proxied.URL.Path)
+	}
+	if response.Code != http.StatusServiceUnavailable {
+		t.Fatalf("expected upstream status 503, got %d", response.Code)
+	}
+}

cmd/docker-gateway/main.go

@@ -19,6 +19,9 @@ import (
 )
 
 const (
+	browserImage       = "git.ipao.vip/rogee/fingerprint-chromium@sha256:b9f23b8e3ac640174db0dfa49e9095fe7eb06f5db55a4e7550d979b35ff3a1b7"
+	browserUser        = "1000:1000"
+	browserEntrypoint  = "/usr/local/bin/docker-entrypoint.sh"
 	managedLabel       = "io.creatorhub.managed"
 	idLabel            = "io.creatorhub.runtime-id"
 	networkRoleLabel   = "io.creatorhub.network-role"
@@ -30,6 +33,8 @@ const (
 
 var runtimeIDPattern = regexp.MustCompile(`^[a-z0-9][a-z0-9-]{0,31}$`)
 var networkNamePattern = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9_.-]{0,63}$`)
+var errInvalidRuntimeID = errors.New("invalid runtime id")
+var errUnmanagedContainer = errors.New("refusing to operate on a container not owned by CreatorHub")
 
 type config struct {
 	image   string
@@ -61,11 +66,6 @@ type browser struct {
 }
 
 func main() {
-	image := os.Getenv("BROWSER_IMAGE")
-	if image == "" || strings.HasSuffix(image, ":latest") || !strings.Contains(image, ":") {
-		log.Fatal("BROWSER_IMAGE must be a pinned image tag")
-	}
-
 	transport := &http.Transport{
 		DialContext: func(ctx context.Context, _, _ string) (net.Conn, error) {
 			return (&net.Dialer{}).DialContext(ctx, "unix", env("DOCKER_SOCKET", "/var/run/docker.sock"))
@@ -79,7 +79,7 @@ func main() {
 	if err := docker.ensureInternalNetwork(network); err != nil {
 		log.Fatal(err)
 	}
-	handler := newGateway(docker, config{image: image, network: network})
+	handler := newGateway(docker, config{image: browserImage, network: network})
 
 	server := &http.Server{
 		Addr:              env("LISTEN_ADDR", ":8081"),
@@ -161,9 +161,11 @@ func (api gateway) create(response http.ResponseWriter, request *http.Request) {
 
 	pidsLimit := int64(512)
 	payload := map[string]any{
-		"Image": api.config.image,
-		"Cmd":   []string{"--fingerprint=" + strconv.FormatInt(input.Seed, 10), "about:blank"},
-		"Env":   []string{"REMOTE_DEBUGGING_PORT=9222"},
+		"Image":      api.config.image,
+		"User":       browserUser,
+		"Entrypoint": []string{browserEntrypoint},
+		"Cmd":        []string{"--fingerprint=" + strconv.FormatInt(input.Seed, 10), "about:blank"},
+		"Env":        []string{"REMOTE_DEBUGGING_PORT=9222"},
 		"Labels": map[string]string{
 			managedLabel: "true",
 			idLabel:      input.Name,
@@ -208,7 +210,12 @@ func (api gateway) create(response http.ResponseWriter, request *http.Request) {
 		return
 	}
 	if err := api.docker.expect(http.MethodPost, "/containers/"+url.PathEscape(created.ID)+"/start", nil, http.StatusNoContent, http.StatusNotModified); err != nil {
-		writeError(response, http.StatusBadGateway, fmt.Errorf("container created but did not start: %w", err))
+		cleanupErr := api.docker.expect(http.MethodDelete, "/containers/"+url.PathEscape(created.ID)+"?force=1&v=0", nil, http.StatusNoContent)
+		if cleanupErr != nil {
+			writeError(response, http.StatusBadGateway, fmt.Errorf("container did not start and cleanup failed: %w; cleanup: %v", err, cleanupErr))
+			return
+		}
+		writeError(response, http.StatusBadGateway, fmt.Errorf("container did not start and was removed while preserving its Profile volume: %w", err))
 		return
 	}
 	writeJSON(response, http.StatusCreated, map[string]string{"id": created.ID, "name": input.Name})
@@ -254,7 +261,7 @@ func (api gateway) remove(response http.ResponseWriter, request *http.Request) {
 
 func (api gateway) requireManaged(id string) error {
 	if !runtimeIDPattern.MatchString(id) {
-		return errors.New("invalid runtime id")
+		return errInvalidRuntimeID
 	}
 	result, err := api.docker.request(http.MethodGet, "/containers/"+url.PathEscape(namePrefix+id)+"/json", nil)
 	if err != nil {
@@ -276,7 +283,7 @@ func (api gateway) requireManaged(id string) error {
 		return fmt.Errorf("decode Docker inspect: %w", err)
 	}
 	if inspected.Config.Labels[managedLabel] != "true" || inspected.Config.Labels[idLabel] != id {
-		return errors.New("refusing to operate on a container not owned by CreatorHub")
+		return errUnmanagedContainer
 	}
 	return nil
 }
@@ -364,13 +371,16 @@ func (docker dockerClient) ensureInternalNetwork(name string) error {
 }
 
 func statusFor(err error) int {
-	if errors.Is(err, os.ErrNotExist) {
+	switch {
+	case errors.Is(err, errInvalidRuntimeID):
+		return http.StatusBadRequest
+	case errors.Is(err, os.ErrNotExist):
 		return http.StatusNotFound
-	}
-	if strings.Contains(err.Error(), "refusing") {
+	case errors.Is(err, errUnmanagedContainer):
 		return http.StatusForbidden
+	default:
+		return http.StatusBadGateway
 	}
-	return http.StatusBadRequest
 }
 
 func forwardDockerError(response http.ResponseWriter, result *http.Response) {

cmd/docker-gateway/main_test.go

@@ -1,11 +1,17 @@
 package main
 
 import (
+	"context"
 	"encoding/json"
+	"fmt"
+	"net"
 	"net/http"
 	"net/http/httptest"
+	"os"
+	"os/exec"
 	"strings"
 	"testing"
+	"time"
 )
 
 func TestGatewayCreatesConstrainedBrowser(t *testing.T) {
@@ -43,6 +49,9 @@ func TestGatewayCreatesConstrainedBrowser(t *testing.T) {
 	if created["Image"] != "registry.example/browser:1.2.3" {
 		t.Fatalf("gateway accepted an unexpected image: %#v", created["Image"])
 	}
+	if created["User"] != browserUser || created["Entrypoint"].([]any)[0] != browserEntrypoint {
+		t.Fatalf("runtime identity is not fixed: user=%#v entrypoint=%#v", created["User"], created["Entrypoint"])
+	}
 	host := created["HostConfig"].(map[string]any)
 	if host["NetworkMode"] != "creatorhub_browser" || host["ReadonlyRootfs"] != true {
 		t.Fatalf("missing container isolation: %#v", host)
@@ -57,6 +66,101 @@ func TestGatewayCreatesConstrainedBrowser(t *testing.T) {
 	}
 }
 
+func TestGatewayRemovesFailedContainerAndPreservesProfile(t *testing.T) {
+	removed := false
+	docker := httptest.NewServer(http.HandlerFunc(func(response http.ResponseWriter, request *http.Request) {
+		switch {
+		case request.Method == http.MethodPost && request.URL.Path == "/containers/create":
+			response.WriteHeader(http.StatusCreated)
+			_, _ = response.Write([]byte(`{"Id":"failed-id"}`))
+		case request.Method == http.MethodPost && request.URL.Path == "/containers/failed-id/start":
+			http.Error(response, "start failed", http.StatusInternalServerError)
+		case request.Method == http.MethodDelete && request.URL.Path == "/containers/failed-id":
+			removed = request.URL.Query().Get("force") == "1" && request.URL.Query().Get("v") == "0"
+			response.WriteHeader(http.StatusNoContent)
+		default:
+			t.Fatalf("unexpected Docker request %s %s", request.Method, request.URL.String())
+		}
+	}))
+	defer docker.Close()
+
+	handler := newGateway(dockerClient{baseURL: docker.URL, client: docker.Client()}, config{image: browserImage, network: "creatorhub_browser"})
+	request := httptest.NewRequest(http.MethodPost, "/v1/browsers", strings.NewReader(`{"name":"account-a","seed":1000}`))
+	response := httptest.NewRecorder()
+	handler.ServeHTTP(response, request)
+
+	if response.Code != http.StatusBadGateway || !removed {
+		t.Fatalf("expected failed container cleanup with preserved volume, status=%d removed=%v body=%s", response.Code, removed, response.Body.String())
+	}
+}
+
+func TestGatewayListsBrowsers(t *testing.T) {
+	docker := httptest.NewServer(http.HandlerFunc(func(response http.ResponseWriter, request *http.Request) {
+		if request.Method != http.MethodGet || request.URL.Path != "/containers/json" {
+			t.Fatalf("unexpected Docker request %s %s", request.Method, request.URL.String())
+		}
+		_, _ = response.Write([]byte(`[{"Id":"container-id","State":"running","Status":"Up","Labels":{"io.creatorhub.runtime-id":"account-a"}}]`))
+	}))
+	defer docker.Close()
+
+	handler := newGateway(dockerClient{baseURL: docker.URL, client: docker.Client()}, config{})
+	response := httptest.NewRecorder()
+	handler.ServeHTTP(response, httptest.NewRequest(http.MethodGet, "/v1/browsers", nil))
+
+	var browsers []browser
+	if response.Code != http.StatusOK || json.NewDecoder(response.Body).Decode(&browsers) != nil || len(browsers) != 1 || browsers[0].Name != "account-a" {
+		t.Fatalf("unexpected list response status=%d body=%s", response.Code, response.Body.String())
+	}
+}
+
+func TestGatewayLifecycle(t *testing.T) {
+	tests := []struct {
+		method     string
+		path       string
+		dockerPath string
+	}{
+		{http.MethodPost, "/v1/browsers/account-a/start", "/containers/creatorhub-browser-account-a/start"},
+		{http.MethodPost, "/v1/browsers/account-a/stop", "/containers/creatorhub-browser-account-a/stop"},
+		{http.MethodDelete, "/v1/browsers/account-a", "/containers/creatorhub-browser-account-a"},
+	}
+	for _, test := range tests {
+		t.Run(test.method+" "+test.path, func(t *testing.T) {
+			docker := httptest.NewServer(http.HandlerFunc(func(response http.ResponseWriter, request *http.Request) {
+				if request.Method == http.MethodGet {
+					_, _ = response.Write([]byte(`{"Config":{"Labels":{"io.creatorhub.managed":"true","io.creatorhub.runtime-id":"account-a"}}}`))
+					return
+				}
+				if request.URL.Path != test.dockerPath {
+					t.Fatalf("unexpected Docker path %s", request.URL.String())
+				}
+				response.WriteHeader(http.StatusNoContent)
+			}))
+			defer docker.Close()
+
+			handler := newGateway(dockerClient{baseURL: docker.URL, client: docker.Client()}, config{})
+			response := httptest.NewRecorder()
+			handler.ServeHTTP(response, httptest.NewRequest(test.method, test.path, nil))
+			if response.Code != http.StatusNoContent {
+				t.Fatalf("expected 204, got %d: %s", response.Code, response.Body.String())
+			}
+		})
+	}
+}
+
+func TestGatewayMapsDockerServiceFailureToBadGateway(t *testing.T) {
+	docker := httptest.NewServer(http.HandlerFunc(func(response http.ResponseWriter, _ *http.Request) {
+		http.Error(response, "daemon unavailable", http.StatusInternalServerError)
+	}))
+	defer docker.Close()
+
+	handler := newGateway(dockerClient{baseURL: docker.URL, client: docker.Client()}, config{})
+	response := httptest.NewRecorder()
+	handler.ServeHTTP(response, httptest.NewRequest(http.MethodDelete, "/v1/browsers/account-a", nil))
+	if response.Code != http.StatusBadGateway {
+		t.Fatalf("expected 502 for Docker failure, got %d: %s", response.Code, response.Body.String())
+	}
+}
+
 func TestGatewayRefusesUnmanagedContainer(t *testing.T) {
 	deleted := false
 	docker := httptest.NewServer(http.HandlerFunc(func(response http.ResponseWriter, request *http.Request) {
@@ -183,3 +287,46 @@ func TestEnsureInternalNetworkRejectsControlNetwork(t *testing.T) {
 		t.Fatalf("expected control network to be rejected before Docker request, requested=%v err=%v", requested, err)
 	}
 }
+
+func TestDockerIntegrationBlocksCrossContainerTraffic(t *testing.T) {
+	if os.Getenv("CREATORHUB_DOCKER_INTEGRATION") != "1" {
+		t.Skip("set CREATORHUB_DOCKER_INTEGRATION=1 to run")
+	}
+	image := "alpine:3.22@sha256:14358309a308569c32bdc37e2e0e9694be33a9d99e68afb0f5ff33cc1f695dce"
+	networkName := fmt.Sprintf("creatorhub_test_%d", time.Now().UnixNano())
+	serverName := networkName + "_server"
+	runDocker := func(args ...string) ([]byte, error) {
+		return exec.Command("docker", args...).CombinedOutput()
+	}
+	if output, err := runDocker("pull", image); err != nil {
+		t.Fatalf("pull integration image: %v: %s", err, output)
+	}
+	t.Cleanup(func() {
+		if output, err := runDocker("network", "rm", networkName); err != nil {
+			t.Logf("remove test network: %v: %s", err, output)
+		}
+	})
+	t.Cleanup(func() {
+		if output, err := runDocker("rm", "-f", serverName); err != nil {
+			t.Logf("remove test container: %v: %s", err, output)
+		}
+	})
+
+	transport := &http.Transport{DialContext: func(ctx context.Context, _, _ string) (net.Conn, error) {
+		return (&net.Dialer{}).DialContext(ctx, "unix", "/var/run/docker.sock")
+	}}
+	defer transport.CloseIdleConnections()
+	client := dockerClient{baseURL: "http://docker/v1.43", client: &http.Client{Transport: transport}}
+	if err := client.ensureInternalNetwork(networkName); err != nil {
+		t.Fatal(err)
+	}
+	if output, err := runDocker("run", "-d", "--name", serverName, "--network", networkName, "--network-alias", "peer", image, "nc", "-lk", "-p", "8080", "-e", "/bin/echo"); err != nil {
+		t.Fatalf("start peer server: %v: %s", err, output)
+	}
+	if output, err := runDocker("exec", serverName, "nc", "-z", "-w", "2", "127.0.0.1", "8080"); err != nil {
+		t.Fatalf("peer server is not ready: %v: %s", err, output)
+	}
+	if output, err := runDocker("run", "--rm", "--network", networkName, image, "nc", "-z", "-w", "2", "peer", "8080"); err == nil {
+		t.Fatalf("cross-container request unexpectedly succeeded: %s", output)
+	}
+}