From 14c8ce691f9e1ee9858346c5e5d6b6910d553d43 Mon Sep 17 00:00:00 2001 From: Rogee Date: Fri, 28 Aug 2026 10:12:52 +0800 Subject: [PATCH] HH-750: harden Fiber boundaries and shutdown (#9) --- cmd/control-plane/main.go | 14 ++++++++---- cmd/control-plane/main_test.go | 22 ++++++++++++++---- cmd/docker-gateway/main.go | 20 +++++++++++++++-- cmd/docker-gateway/main_test.go | 40 +++++++++++++++++++++++++++++++++ 4 files changed, 86 insertions(+), 10 deletions(-) diff --git a/cmd/control-plane/main.go b/cmd/control-plane/main.go index af6fdee..bdd8f91 100644 --- a/cmd/control-plane/main.go +++ b/cmd/control-plane/main.go @@ -1,15 +1,18 @@ package main import ( + "context" "errors" "net" "net/http" "net/http/httputil" "net/url" "os" + "os/signal" "path/filepath" "strconv" "strings" + "syscall" "time" "github.com/gofiber/fiber/v3" @@ -28,7 +31,9 @@ type config struct { func main() { logrus.SetFormatter(&logrus.JSONFormatter{}) - if err := newCommand().Execute(); err != nil { + ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) + defer stop() + if err := newCommand().ExecuteContext(ctx); err != nil { logrus.WithField("service", "control-plane").WithError(err).Error("service stopped") os.Exit(1) } @@ -50,7 +55,7 @@ func newCommand() *cobra.Command { logrus.WithFields(logrus.Fields{ "service": "control-plane", "listen_addr": cfg.listenAddr, - "docker_gateway": cfg.gateway.String(), + "docker_gateway": cfg.gateway.Scheme + "://" + cfg.gateway.Host, }).Info("service starting") return newHandler(cfg.gateway, cfg.webDir).Listen(cfg.listenAddr, fiber.ListenConfig{ GracefulContext: command.Context(), @@ -137,8 +142,9 @@ func newHandler(gateway *url.URL, webDirectory string) *fiber.App { func spaHandler(directory string) fiber.Handler { return func(c fiber.Ctx) error { - path := filepath.Clean(strings.TrimPrefix(c.Path(), "/")) - if filepath.IsLocal(path) { + path, err := url.PathUnescape(strings.TrimPrefix(c.Path(), "/")) + path = filepath.Clean(path) + if err == nil && filepath.IsLocal(path) { name := filepath.Join(directory, path) if info, err := os.Stat(name); err == nil && !info.IsDir() { return c.SendFile(name) diff --git a/cmd/control-plane/main_test.go b/cmd/control-plane/main_test.go index c7645a7..caa7bfe 100644 --- a/cmd/control-plane/main_test.go +++ b/cmd/control-plane/main_test.go @@ -47,14 +47,28 @@ func TestLoadConfigRejectsInvalidGateway(t *testing.T) { func TestSPAServesFileAndIndexFallback(t *testing.T) { directory := t.TempDir() - if err := os.WriteFile(filepath.Join(directory, "index.html"), []byte("index"), 0o600); err != nil { - t.Fatal(err) + files := map[string]string{ + "index.html": "index", + "app.js": "asset", + "hello world.js": "space", + "应用.js": "unicode", } - if err := os.WriteFile(filepath.Join(directory, "app.js"), []byte("asset"), 0o600); err != nil { + for name, body := range files { + if err := os.WriteFile(filepath.Join(directory, name), []byte(body), 0o600); err != nil { + t.Fatal(err) + } + } + if err := os.WriteFile(filepath.Join(directory, "..", "outside-secret.txt"), []byte("secret"), 0o600); err != nil { t.Fatal(err) } app := newHandler(&url.URL{Scheme: "http", Host: "127.0.0.1"}, directory) - for path, want := range map[string]string{"/app.js": "asset", "/dashboard": "index"} { + for path, want := range map[string]string{ + "/app.js": "asset", + "/hello%20world.js": "space", + "/%E5%BA%94%E7%94%A8.js": "unicode", + "/%2e%2e%2foutside-secret.txt": "index", + "/dashboard": "index", + } { response, err := app.Test(httptest.NewRequest(http.MethodGet, path, nil)) if err != nil { t.Fatal(err) diff --git a/cmd/docker-gateway/main.go b/cmd/docker-gateway/main.go index cd6e541..251c934 100644 --- a/cmd/docker-gateway/main.go +++ b/cmd/docker-gateway/main.go @@ -11,9 +11,11 @@ import ( "net/http" "net/url" "os" + "os/signal" "regexp" "strconv" "strings" + "syscall" "time" "github.com/gofiber/fiber/v3" @@ -78,7 +80,9 @@ type browser struct { func main() { logrus.SetFormatter(&logrus.JSONFormatter{}) - if err := newCommand().Execute(); err != nil { + ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) + defer stop() + if err := newCommand().ExecuteContext(ctx); err != nil { logrus.WithField("service", "docker-gateway").WithError(err).Error("service stopped") os.Exit(1) } @@ -183,6 +187,14 @@ func newGateway(client dockerClient, cfg browserConfig) *fiber.App { BodyLimit: 1 << 20, ReadTimeout: 5 * time.Second, IdleTimeout: 60 * time.Second, + ErrorHandler: func(c fiber.Ctx, err error) error { + status := http.StatusInternalServerError + var fiberError *fiber.Error + if errors.As(err, &fiberError) && fiberError != nil { + status = fiberError.Code + } + return writeError(c, status, err) + }, }) app.Get("/healthz", func(c fiber.Ctx) error { c.Status(fiber.StatusNoContent) @@ -290,7 +302,11 @@ func (api gateway) create(c fiber.Ctx) error { ID string `json:"Id"` } if err := json.NewDecoder(result.Body).Decode(&created); err != nil || created.ID == "" { - return writeError(c, http.StatusBadGateway, errors.New("Docker returned an invalid container id")) + cleanupErr := api.docker.expect(http.MethodDelete, "/containers/"+url.PathEscape(namePrefix+input.Name)+"?force=1&v=0", nil, http.StatusNoContent, http.StatusNotFound) + if cleanupErr != nil { + return writeError(c, http.StatusBadGateway, fmt.Errorf("Docker returned an invalid container id and cleanup failed: %w", cleanupErr)) + } + return writeError(c, http.StatusBadGateway, errors.New("Docker returned an invalid container id; container was removed while preserving its Profile volume")) } if err := api.docker.expect(http.MethodPost, "/containers/"+url.PathEscape(created.ID)+"/start", nil, http.StatusNoContent, http.StatusNotModified); err != nil { cleanupErr := api.docker.expect(http.MethodDelete, "/containers/"+url.PathEscape(created.ID)+"?force=1&v=0", nil, http.StatusNoContent) diff --git a/cmd/docker-gateway/main_test.go b/cmd/docker-gateway/main_test.go index 7156776..1933fc7 100644 --- a/cmd/docker-gateway/main_test.go +++ b/cmd/docker-gateway/main_test.go @@ -13,6 +13,7 @@ import ( "testing" "time" + "github.com/gofiber/fiber/v3" "github.com/gofiber/fiber/v3/middleware/adaptor" ) @@ -76,6 +77,45 @@ func TestGatewayRejectsOversizedCreateRequest(t *testing.T) { if response.Code != http.StatusRequestEntityTooLarge { t.Fatalf("expected 413 for oversized body, status=%d body=%s", response.Code, response.Body.String()) } + + handler.Post("/request-limit", func(fiber.Ctx) error { return fiber.ErrRequestEntityTooLarge }) + jsonResponse, err := handler.Test(httptest.NewRequest(http.MethodPost, "/request-limit", nil)) + if err != nil { + t.Fatal(err) + } + defer jsonResponse.Body.Close() + var body map[string]string + decodeErr := json.NewDecoder(jsonResponse.Body).Decode(&body) + contentType := jsonResponse.Header.Get("Content-Type") + if jsonResponse.StatusCode != http.StatusRequestEntityTooLarge || decodeErr != nil || body["error"] == "" || !strings.HasPrefix(contentType, "application/json") { + t.Fatalf("expected JSON 413 envelope, status=%d body=%v decode=%v content-type=%q", jsonResponse.StatusCode, body, decodeErr, contentType) + } +} + +func TestGatewayRemovesContainerWhenCreateResponseHasNoID(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":""}`)) + case request.Method == http.MethodDelete && request.URL.Path == "/containers/"+namePrefix+"account-a": + 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()}, browserConfig{image: browserImage, network: "creatorhub_browser"}) + request := httptest.NewRequest(http.MethodPost, "/v1/browsers", strings.NewReader(`{"name":"account-a","seed":1000}`)) + response := httptest.NewRecorder() + adaptor.FiberApp(handler).ServeHTTP(response, request) + + if response.Code != http.StatusBadGateway || !removed { + t.Fatalf("expected invalid create response cleanup, status=%d removed=%v body=%s", response.Code, removed, response.Body.String()) + } } func TestGatewayRemovesFailedContainerAndPreservesProfile(t *testing.T) {