From e1960513ae562700852db0d570157e1d2b31fb76 Mon Sep 17 00:00:00 2001 From: Chris Hager Date: Thu, 4 Sep 2025 10:51:55 +0200 Subject: [PATCH 1/8] added AGENTS.md --- AGENTS.md | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 AGENTS.md diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 0000000..bba9910 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,38 @@ +# Repository Guidelines + +## Project Structure & Modules +- `cmd/httpserver/`: CLI entrypoint and service startup. +- `httpserver/`: HTTP server, handlers, routing, and tests. +- `application/`, `domain/`, `ports/`, `adapters/`: Clean/hexagonal layers (core logic, interfaces, DB/secrets adapters). +- `schema/`: SQL migrations; `testdata/`: fixtures; `metrics/`: Prometheus metrics; `docker/`: Dockerfiles and compose; `scripts/`: CI/e2e helpers. + +## Build, Test, and Dev Commands +- `make build`: Build the server to `build/builder-hub`. +- `go run cmd/httpserver/main.go`: Run locally (see env flags below). +- `make test`: Run unit tests. Use `RUN_DB_TESTS=1 make test` to include DB/e2e tests. +- `make cover` / `make cover-html`: Coverage summary / HTML report. +- `make fmt`: Format and tidy imports/modules. +- `make lint`: `go vet`, `staticcheck`, `golangci-lint`, and format checks. +- `make docker-httpserver`: Build the Docker image. +- Helpful: `docs/devenv-setup.md`, `scripts/ci/integration-test.sh`. + +## Coding Style & Conventions +- Go formatting is enforced: run `make fmt` (gofmt, gofumpt, gci, go mod tidy). +- Package names: lower-case; exported symbols: PascalCase; locals: camelCase. +- Keep files focused; group handlers in `httpserver`, business logic in `application/domain`. +- Imports: standard → third-party → local (enforced by gci). +- Indentation: Use tab, not spaces + +## Testing Guidelines +- Framework: Go `testing`; tests live in `*_test.go`. Use `testdata/` fixtures. +- Quick run: `make test`; with DB/e2e: `RUN_DB_TESTS=1 make test`. +- Coverage: `make cover`. Prefer table-driven tests and clear Arrange/Act/Assert sections. + +## Commit & Pull Request Guidelines +- Commits: concise, imperative mood (e.g., "Add API spec tests"); optional scope tags (e.g., `[CI]`); reference issues/PRs when relevant. +- PRs: include description, linked issues, testing steps (curl examples for API changes), and screenshots/log snippets when useful. +- Required before merge: `make lt` (lint + tests) green, updated docs in `docs/` when API/behavior changes. + +## Configuration & Security +- Common env flags (via CLI/env): `LISTEN_ADDR`, `ADMIN_ADDR`, `INTERNAL_ADDR`, `METRICS_ADDR`, `POSTGRES_DSN`, `LOG_JSON`, `LOG_DEBUG`, `PPROF`, `MOCK_SECRETS`, `AWS_BUILDER_CONFIGS_SECRET_NAME`/`_PREFIX`. +- Default DSN targets local Postgres; do not commit secrets. Use `MOCK_SECRETS=1` for local dev. From 7926ee0d31442cda0f1c7a0ec489ae6c9d15423b Mon Sep 17 00:00:00 2001 From: Chris Hager Date: Thu, 4 Sep 2025 10:59:35 +0200 Subject: [PATCH 2/8] Admin API with basic auth --- README.md | 29 ++++++++++++- cmd/httpserver/main.go | 17 ++++++++ httpserver/auth_middleware_test.go | 68 ++++++++++++++++++++++++++++++ httpserver/server.go | 48 +++++++++++++++++++++ 4 files changed, 161 insertions(+), 1 deletion(-) create mode 100644 httpserver/auth_middleware_test.go diff --git a/README.md b/README.md index ffff92a..17b48d3 100644 --- a/README.md +++ b/README.md @@ -22,6 +22,33 @@ BuilderHub has these responsibilities: ## Getting started +### Admin authentication + +The Admin API (port 8081) requires HTTP Basic Auth. Configure via env vars or flags: + +- `ADMIN_BASIC_USER` (default: `admin`) +- `ADMIN_BASIC_PASSWORD_BCRYPT` (bcrypt hash of the password; required) + +Generate a bcrypt hash (example using htpasswd): + +```bash +htpasswd -nbBC 12 admin 'secret' | cut -d: -f2 +``` + +Run with: + +```bash +export ADMIN_BASIC_USER=admin +export ADMIN_BASIC_PASSWORD_BCRYPT='$2y$12$...' +go run cmd/httpserver/main.go +``` + +Use Basic Auth when calling admin endpoints, e.g.: + +```bash +curl -u admin:secret http://localhost:8081/api/admin/v1/measurements +``` + ### Manual setup **Start the database and the server:** @@ -287,4 +314,4 @@ Payload: JSON with secrets, both flattened/unflattened { ... } -``` \ No newline at end of file +``` diff --git a/cmd/httpserver/main.go b/cmd/httpserver/main.go index 4abc69e..a3b298b 100644 --- a/cmd/httpserver/main.go +++ b/cmd/httpserver/main.go @@ -73,6 +73,18 @@ var flags = []cli.Flag{ Usage: "enable pprof debug endpoint", EnvVars: []string{"PPROF"}, }, + &cli.StringFlag{ + Name: "admin-basic-user", + Value: "admin", + Usage: "username for admin Basic Auth", + EnvVars: []string{"ADMIN_BASIC_USER"}, + }, + &cli.StringFlag{ + Name: "admin-basic-password-bcrypt", + Value: "", + Usage: "bcrypt hash of admin password (required to enable admin API)", + EnvVars: []string{"ADMIN_BASIC_PASSWORD_BCRYPT"}, + }, &cli.Int64Flag{ Name: "drain-seconds", Value: 15, @@ -124,6 +136,8 @@ func runCli(cCtx *cli.Context) error { enablePprof := cCtx.Bool("pprof") drainDuration := time.Duration(cCtx.Int64("drain-seconds")) * time.Second mockSecretsStorage := cCtx.Bool("mock-secrets") + adminBasicUser := cCtx.String("admin-basic-user") + adminPasswordBcrypt := cCtx.String("admin-basic-password-bcrypt") logTags := map[string]string{ "version": common.Version, @@ -175,6 +189,9 @@ func runCli(cCtx *cli.Context) error { Log: log, EnablePprof: enablePprof, + AdminBasicUser: adminBasicUser, + AdminPasswordBcrypt: adminPasswordBcrypt, + DrainDuration: drainDuration, GracefulShutdownDuration: 30 * time.Second, ReadTimeout: 60 * time.Second, diff --git a/httpserver/auth_middleware_test.go b/httpserver/auth_middleware_test.go new file mode 100644 index 0000000..6d1cc22 --- /dev/null +++ b/httpserver/auth_middleware_test.go @@ -0,0 +1,68 @@ +package httpserver + +import ( + "fmt" + "net/http" + "net/http/httptest" + "testing" + + "github.com/flashbots/builder-hub/common" + "github.com/go-chi/httplog/v2" + "github.com/stretchr/testify/require" + "golang.org/x/crypto/bcrypt" +) + +func testLogger() *httplog.Logger { + return common.SetupLogger(&common.LoggingOpts{Debug: true, JSON: false, Service: "test"}) +} + +// helper to create a simple handler protected by the basic auth middleware +func protectedHandler(t *testing.T, user, bcryptHash string) (http.Handler, *Server) { + t.Helper() + srv := &Server{cfg: &HTTPServerConfig{AdminBasicUser: user, AdminPasswordBcrypt: bcryptHash}, log: testLogger()} + next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + _, _ = fmt.Fprint(w, "ok") + }) + return srv.basicAuthMiddleware()(next), srv +} + +func Test_AdminAuth_DeniesWithoutHash(t *testing.T) { + h, _ := protectedHandler(t, "admin", "") + req := httptest.NewRequest(http.MethodGet, "/", nil) + rr := httptest.NewRecorder() + h.ServeHTTP(rr, req) + require.Equal(t, http.StatusUnauthorized, rr.Code) +} + +func Test_AdminAuth_DeniesWrongCreds(t *testing.T) { + hash, err := bcrypt.GenerateFromPassword([]byte("secret"), bcrypt.DefaultCost) + require.NoError(t, err) + h, _ := protectedHandler(t, "admin", string(hash)) + + // wrong password + req := httptest.NewRequest(http.MethodGet, "/", nil) + req.SetBasicAuth("admin", "bad") + rr := httptest.NewRecorder() + h.ServeHTTP(rr, req) + require.Equal(t, http.StatusUnauthorized, rr.Code) + + // wrong user + req2 := httptest.NewRequest(http.MethodGet, "/", nil) + req2.SetBasicAuth("root", "secret") + rr2 := httptest.NewRecorder() + h.ServeHTTP(rr2, req2) + require.Equal(t, http.StatusUnauthorized, rr2.Code) +} + +func Test_AdminAuth_AllowsCorrectCreds(t *testing.T) { + hash, err := bcrypt.GenerateFromPassword([]byte("secret"), bcrypt.DefaultCost) + require.NoError(t, err) + h, _ := protectedHandler(t, "admin", string(hash)) + + req := httptest.NewRequest(http.MethodGet, "/", nil) + req.SetBasicAuth("admin", "secret") + rr := httptest.NewRecorder() + h.ServeHTTP(rr, req) + require.Equal(t, http.StatusOK, rr.Code) +} diff --git a/httpserver/server.go b/httpserver/server.go index 7fff421..573a1d6 100644 --- a/httpserver/server.go +++ b/httpserver/server.go @@ -13,6 +13,7 @@ import ( "github.com/go-chi/chi/v5/middleware" "github.com/go-chi/httplog/v2" "go.uber.org/atomic" + "golang.org/x/crypto/bcrypt" ) type HTTPServerConfig struct { @@ -27,6 +28,10 @@ type HTTPServerConfig struct { GracefulShutdownDuration time.Duration ReadTimeout time.Duration WriteTimeout time.Duration + + // Admin API auth + AdminBasicUser string + AdminPasswordBcrypt string } type Server struct { @@ -113,6 +118,9 @@ func (srv *Server) GetAdminRouter() http.Handler { mux.Use(middleware.Recoverer) mux.Use(metrics.Middleware) + // Require Basic Auth for all admin routes + mux.Use(srv.basicAuthMiddleware()) + mux.Get("/api/admin/v1/builders/configuration/{builderName}/active", srv.adminHandler.GetActiveConfigForBuilder) mux.Get("/api/admin/v1/builders/configuration/{builderName}/full", srv.adminHandler.GetFullConfigForBuilder) mux.Post("/api/admin/v1/measurements", srv.adminHandler.AddMeasurement) @@ -139,6 +147,46 @@ func (srv *Server) GetInternalRouter() http.Handler { return mux } +// basicAuthMiddleware enforces HTTP Basic Auth on admin routes. +// Username must match cfg.AdminBasicUser and password must match cfg.AdminPasswordBcrypt (bcrypt hash). +func (srv *Server) basicAuthMiddleware() func(http.Handler) http.Handler { + requiredUser := srv.cfg.AdminBasicUser + bcryptHash := srv.cfg.AdminPasswordBcrypt + + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // If no hash configured, deny access (secure-by-default) + if bcryptHash == "" { + w.Header().Set("WWW-Authenticate", "Basic realm=admin") + http.Error(w, http.StatusText(http.StatusUnauthorized), http.StatusUnauthorized) + return + } + + u, p, ok := r.BasicAuth() + if !ok { + w.Header().Set("WWW-Authenticate", "Basic realm=admin") + http.Error(w, http.StatusText(http.StatusUnauthorized), http.StatusUnauthorized) + return + } + + if requiredUser != "" && u != requiredUser { + w.Header().Set("WWW-Authenticate", "Basic realm=admin") + http.Error(w, http.StatusText(http.StatusUnauthorized), http.StatusUnauthorized) + return + } + + // Compare password to bcrypt hash + if err := bcrypt.CompareHashAndPassword([]byte(bcryptHash), []byte(p)); err != nil { + w.Header().Set("WWW-Authenticate", "Basic realm=admin") + http.Error(w, http.StatusText(http.StatusUnauthorized), http.StatusUnauthorized) + return + } + + next.ServeHTTP(w, r) + }) + } +} + func (srv *Server) _stringFromFile(fn string) (string, error) { content, err := os.ReadFile(fn) if err != nil { From b547550d38c8a3c759cd7a4245ebbbbd8548dc1c Mon Sep 17 00:00:00 2001 From: Chris Hager Date: Thu, 4 Sep 2025 11:14:41 +0200 Subject: [PATCH 3/8] updated htpasswd instructions and added test --- README.md | 2 +- httpserver/auth_middleware_test.go | 26 ++++++++++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 17b48d3..9e6f368 100644 --- a/README.md +++ b/README.md @@ -32,7 +32,7 @@ The Admin API (port 8081) requires HTTP Basic Auth. Configure via env vars or fl Generate a bcrypt hash (example using htpasswd): ```bash -htpasswd -nbBC 12 admin 'secret' | cut -d: -f2 +htpasswd -nbBC 10 "" 'secret' | cut -d: -f2 ``` Run with: diff --git a/httpserver/auth_middleware_test.go b/httpserver/auth_middleware_test.go index 6d1cc22..6044899 100644 --- a/httpserver/auth_middleware_test.go +++ b/httpserver/auth_middleware_test.go @@ -66,3 +66,29 @@ func Test_AdminAuth_AllowsCorrectCreds(t *testing.T) { h.ServeHTTP(rr, req) require.Equal(t, http.StatusOK, rr.Code) } + +func Test_PasswordHash(t *testing.T) { + // Given a password string, ensure a bcrypt (htpasswd-style) hash validates. + password := "secret" + htpasswdHash := "$2y$10$Q3mgTfng5mWUlEkLaOA4du0mBIKKYblTcWMqbqehsJM96xF8YV4XC" // create with: htpasswd -nbBC 10 "" 'secret' | cut -d: -f2 + + // Generate a bcrypt hash (htpasswd uses bcrypt too). Cost can vary; default is fine for test. + hash, err := bcrypt.GenerateFromPassword([]byte(password), bcrypt.DefaultCost) + require.NoError(t, err) + + hash2, err := bcrypt.GenerateFromPassword([]byte(password), bcrypt.DefaultCost) + require.NoError(t, err) + require.NotEqual(t, hash, hash2, "bcrypt hashes with same input should differ due to random salt") + + // Correct password must match + err = bcrypt.CompareHashAndPassword(hash, []byte(password)) + require.NoError(t, err) + + // Wrong password must fail + err = bcrypt.CompareHashAndPassword(hash, []byte("wrong")) + require.Error(t, err) + + // Hash from htpasswd works + err = bcrypt.CompareHashAndPassword([]byte(htpasswdHash), []byte(password)) + require.NoError(t, err) +} From 8e5dcf8e95017dd508b03275ee719a14a570dca6 Mon Sep 17 00:00:00 2001 From: Chris Hager Date: Thu, 4 Sep 2025 11:52:15 +0200 Subject: [PATCH 4/8] added --disable-admin-auth flag --- README.md | 2 ++ cmd/httpserver/main.go | 12 +++++++++++- httpserver/auth_middleware_test.go | 24 ++++++++++++++++++++++++ httpserver/server.go | 6 ++++++ 4 files changed, 43 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 9e6f368..406d636 100644 --- a/README.md +++ b/README.md @@ -49,6 +49,8 @@ Use Basic Auth when calling admin endpoints, e.g.: curl -u admin:secret http://localhost:8081/api/admin/v1/measurements ``` +Local development only: you can disable Admin API auth with `--disable-admin-auth` or `DISABLE_ADMIN_AUTH=1`. This is unsafe; never use in production. + ### Manual setup **Start the database and the server:** diff --git a/cmd/httpserver/main.go b/cmd/httpserver/main.go index a3b298b..3f96a9c 100644 --- a/cmd/httpserver/main.go +++ b/cmd/httpserver/main.go @@ -82,9 +82,14 @@ var flags = []cli.Flag{ &cli.StringFlag{ Name: "admin-basic-password-bcrypt", Value: "", - Usage: "bcrypt hash of admin password (required to enable admin API)", + Usage: "bcrypt hash of admin password (required to enable admin API, generate with `htpasswd -nbBC 12 admin 'secret' | cut -d: -f2`)", EnvVars: []string{"ADMIN_BASIC_PASSWORD_BCRYPT"}, }, + &cli.BoolFlag{ + Name: "disable-admin-auth", + Usage: "disable admin Basic Auth (local development only)", + EnvVars: []string{"DISABLE_ADMIN_AUTH"}, + }, &cli.Int64Flag{ Name: "drain-seconds", Value: 15, @@ -138,6 +143,7 @@ func runCli(cCtx *cli.Context) error { mockSecretsStorage := cCtx.Bool("mock-secrets") adminBasicUser := cCtx.String("admin-basic-user") adminPasswordBcrypt := cCtx.String("admin-basic-password-bcrypt") + disableAdminAuth := cCtx.Bool("disable-admin-auth") logTags := map[string]string{ "version": common.Version, @@ -156,6 +162,9 @@ func runCli(cCtx *cli.Context) error { }) log.With("version", common.Version).Info("starting builder-hub") + if disableAdminAuth { + log.Warn("ADMIN AUTH DISABLED! DO NOT USE IN PRODUCTION", "flag", "--disable-admin-auth") + } db, err := database.NewDatabaseService(cCtx.String("postgres-dsn")) if err != nil { @@ -191,6 +200,7 @@ func runCli(cCtx *cli.Context) error { AdminBasicUser: adminBasicUser, AdminPasswordBcrypt: adminPasswordBcrypt, + AdminAuthDisabled: disableAdminAuth, DrainDuration: drainDuration, GracefulShutdownDuration: 30 * time.Second, diff --git a/httpserver/auth_middleware_test.go b/httpserver/auth_middleware_test.go index 6044899..f816175 100644 --- a/httpserver/auth_middleware_test.go +++ b/httpserver/auth_middleware_test.go @@ -27,6 +27,16 @@ func protectedHandler(t *testing.T, user, bcryptHash string) (http.Handler, *Ser return srv.basicAuthMiddleware()(next), srv } +func protectedHandlerDisabled(t *testing.T, disabled bool) http.Handler { + t.Helper() + srv := &Server{cfg: &HTTPServerConfig{AdminAuthDisabled: disabled}, log: testLogger()} + next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + _, _ = fmt.Fprint(w, "ok") + }) + return srv.basicAuthMiddleware()(next) +} + func Test_AdminAuth_DeniesWithoutHash(t *testing.T) { h, _ := protectedHandler(t, "admin", "") req := httptest.NewRequest(http.MethodGet, "/", nil) @@ -67,6 +77,20 @@ func Test_AdminAuth_AllowsCorrectCreds(t *testing.T) { require.Equal(t, http.StatusOK, rr.Code) } +func Test_AdminAuth_Disabled_AllowsWithoutCreds(t *testing.T) { + h := protectedHandlerDisabled(t, true) + req := httptest.NewRequest(http.MethodGet, "/", nil) + rr := httptest.NewRecorder() + h.ServeHTTP(rr, req) + require.Equal(t, http.StatusOK, rr.Code) + + // When not disabled, should be unauthorized without hash/creds + h2 := protectedHandlerDisabled(t, false) + rr2 := httptest.NewRecorder() + h2.ServeHTTP(rr2, req) + require.Equal(t, http.StatusUnauthorized, rr2.Code) +} + func Test_PasswordHash(t *testing.T) { // Given a password string, ensure a bcrypt (htpasswd-style) hash validates. password := "secret" diff --git a/httpserver/server.go b/httpserver/server.go index 573a1d6..d8b38da 100644 --- a/httpserver/server.go +++ b/httpserver/server.go @@ -32,6 +32,7 @@ type HTTPServerConfig struct { // Admin API auth AdminBasicUser string AdminPasswordBcrypt string + AdminAuthDisabled bool } type Server struct { @@ -155,6 +156,11 @@ func (srv *Server) basicAuthMiddleware() func(http.Handler) http.Handler { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if srv.cfg.AdminAuthDisabled { + // pass-through when disabled (for local development only) + next.ServeHTTP(w, r) + return + } // If no hash configured, deny access (secure-by-default) if bcryptHash == "" { w.Header().Set("WWW-Authenticate", "Basic realm=admin") From b84a2efa12ba40e6567dc82c835acaacbf0c06b1 Mon Sep 17 00:00:00 2001 From: Chris Hager Date: Thu, 4 Sep 2025 12:10:57 +0200 Subject: [PATCH 5/8] cleanup and linter --- .github/workflows/checks.yml | 2 +- .golangci.yaml | 18 +++++++++++++-- Makefile | 15 ++++++++----- adapters/database/service.go | 26 ++++++++++----------- cmd/httpserver/main.go | 2 +- docker/docker-compose.yaml | 1 + httpserver/auth_middleware_test.go | 36 +++++++++++++++--------------- httpserver/e2e_test.go | 11 ++++----- 8 files changed, 66 insertions(+), 45 deletions(-) diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index 8101c93..1459125 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -61,7 +61,7 @@ jobs: run: go install honnef.co/go/tools/cmd/staticcheck@2025.1.1 - name: Install golangci-lint - run: go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.64.8 + run: go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.1.2 - name: Install NilAway run: go install go.uber.org/nilaway/cmd/nilaway@v0.0.0-20240821220108-c91e71c080b7 diff --git a/.golangci.yaml b/.golangci.yaml index ed790fc..6499f97 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -1,3 +1,4 @@ +version: "2" run: tests: false linters: @@ -6,7 +7,6 @@ linters: - cyclop - forbidigo - funlen - - gci - gochecknoglobals - gochecknoinits - gocritic @@ -43,7 +43,6 @@ linters: # # Disabled because deprecated: # - - tenv linters-settings: # @@ -91,3 +90,18 @@ linters-settings: - 'MockBeaconClient' - 'RelayAPI' - 'Webserver' +formatters: + enable: + - gci + - gofmt + - gofumpt + - goimports + settings: + gofumpt: + extra-rules: true + exclusions: + generated: lax + paths: + - third_party$ + - builtin$ + - examples$ diff --git a/Makefile b/Makefile index 10df3e0..99820db 100644 --- a/Makefile +++ b/Makefile @@ -4,6 +4,11 @@ VERSION := $(shell git describe --tags --always --dirty="-dev") +# Admin API auth for curl examples (set ADMIN_AUTH="admin:secret" or similar. can be empty when server is run with --disable-admin-auth) +ADMIN_AUTH ?= +CURL ?= curl -v +CURL_AUTH := $(if $(ADMIN_AUTH),-u $(ADMIN_AUTH),) + # A few colors RED:=\033[0;31m BLUE:=\033[0;34m @@ -92,16 +97,16 @@ db-dump: ## Dump the database contents to file 'database.dump' .PHONY: dev-db-setup dev-db-setup: ## Create the basic database entries for testing and development @printf "$(BLUE)Create the allow-all measurements $(NC)\n" - curl -v --request POST --url http://localhost:8081/api/admin/v1/measurements --data '{"measurement_id": "test1","attestation_type": "test","measurements": {}}' + $(CURL) $(CURL_AUTH) --request POST --url http://localhost:8081/api/admin/v1/measurements --data '{"measurement_id": "test1","attestation_type": "test","measurements": {}}' @printf "$(BLUE)Enable the measurements $(NC)\n" - curl -v --request POST --url http://localhost:8081/api/admin/v1/measurements/activation/test1 --data '{"enabled": true}' + $(CURL) $(CURL_AUTH) --request POST --url http://localhost:8081/api/admin/v1/measurements/activation/test1 --data '{"enabled": true}' @printf "$(BLUE)Create the builder $(NC)\n" - curl -v --request POST --url http://localhost:8081/api/admin/v1/builders --data '{"name": "test_builder","ip_address": "1.2.3.4", "network": "production"}' + $(CURL) $(CURL_AUTH) --request POST --url http://localhost:8081/api/admin/v1/builders --data '{"name": "test_builder","ip_address": "1.2.3.4", "network": "production"}' @printf "$(BLUE)Create the builder configuration $(NC)\n" - curl -v --request POST --url http://localhost:8081/api/admin/v1/builders/configuration/test_builder --data '{"dns_name": "foobar-v1.a.b.c","rbuilder": {"extra_data": "FooBar"}}' + $(CURL) $(CURL_AUTH) --request POST --url http://localhost:8081/api/admin/v1/builders/configuration/test_builder --data '{"dns_name": "foobar-v1.a.b.c","rbuilder": {"extra_data": "FooBar"}}' @printf "$(BLUE)Enable the builder $(NC)\n" - curl -v --request POST --url http://localhost:8081/api/admin/v1/builders/activation/test_builder --data '{"enabled": true}' \ No newline at end of file + $(CURL) $(CURL_AUTH) --request POST --url http://localhost:8081/api/admin/v1/builders/activation/test_builder --data '{"enabled": true}' diff --git a/adapters/database/service.go b/adapters/database/service.go index 4061d48..5ca716c 100644 --- a/adapters/database/service.go +++ b/adapters/database/service.go @@ -25,9 +25,9 @@ func NewDatabaseService(dsn string) (*Service, error) { return nil, err } - db.DB.SetMaxOpenConns(50) - db.DB.SetMaxIdleConns(10) - db.DB.SetConnMaxIdleTime(0) + db.SetMaxOpenConns(50) + db.SetMaxIdleConns(10) + db.SetConnMaxIdleTime(0) dbService := &Service{DB: db} //nolint:exhaustruct return dbService, err @@ -117,9 +117,9 @@ func (s *Service) RegisterCredentialsForBuilder(ctx context.Context, builderName } _, err = tx.Exec(` - INSERT INTO service_credential_registrations + INSERT INTO service_credential_registrations (builder_name, service, tls_cert, ecdsa_pubkey, is_active, measurement_id) - VALUES ($1, $2, $3, $4, true, + VALUES ($1, $2, $3, $4, true, (SELECT id FROM measurements_whitelist WHERE name = $5 AND attestation_type = $6) ) `, builderName, service, nullableTLSCert, ecdsaPubKey, measurementName, attestationType) @@ -150,26 +150,26 @@ func (s *Service) GetActiveConfigForBuilder(ctx context.Context, builderName str func (s *Service) GetActiveBuildersWithServiceCredentials(ctx context.Context, network string) ([]domain.BuilderWithServices, error) { rows, err := s.DB.QueryContext(ctx, ` - SELECT + SELECT b.name, b.ip_address, b.dns_name, scr.service, scr.tls_cert, scr.ecdsa_pubkey - FROM + FROM builders b - LEFT JOIN + LEFT JOIN service_credential_registrations scr ON b.name = scr.builder_name - WHERE + WHERE b.is_active = true AND (scr.is_active = true OR scr.is_active IS NULL) AND b.network = $1 - ORDER BY + ORDER BY b.name, scr.service `, network) if err != nil { return nil, err } - defer rows.Close() + defer rows.Close() //nolint:errcheck buildersMap := make(map[string]*BuilderWithCredentials) @@ -227,9 +227,9 @@ func (s *Service) GetActiveBuildersWithServiceCredentials(ctx context.Context, n func (s *Service) LogEvent(ctx context.Context, eventName, builderName, name string) error { // Insert new event log entry with a subquery to fetch the measurement_id _, err := s.DB.ExecContext(ctx, ` - INSERT INTO event_log + INSERT INTO event_log (event_name, builder_name, measurement_id) - VALUES ($1, $2, + VALUES ($1, $2, (SELECT id FROM measurements_whitelist WHERE name = $3) ) `, eventName, builderName, name) diff --git a/cmd/httpserver/main.go b/cmd/httpserver/main.go index 3f96a9c..d442b6e 100644 --- a/cmd/httpserver/main.go +++ b/cmd/httpserver/main.go @@ -171,7 +171,7 @@ func runCli(cCtx *cli.Context) error { log.Error("failed to create database", "err", err) return err } - defer db.Close() + defer db.Close() //nolint:errcheck var sm ports.AdminSecretService diff --git a/docker/docker-compose.yaml b/docker/docker-compose.yaml index 6693ba4..a335a6a 100644 --- a/docker/docker-compose.yaml +++ b/docker/docker-compose.yaml @@ -41,6 +41,7 @@ services: ADMIN_ADDR: "0.0.0.0:8081" INTERNAL_ADDR: "0.0.0.0:8082" METRICS_ADDR: "0.0.0.0:8090" + DISABLE_ADMIN_AUTH: "1" # local dev only; do not use in production proxy: image: flashbots/builder-hub-mock-proxy diff --git a/httpserver/auth_middleware_test.go b/httpserver/auth_middleware_test.go index f816175..9624742 100644 --- a/httpserver/auth_middleware_test.go +++ b/httpserver/auth_middleware_test.go @@ -28,13 +28,13 @@ func protectedHandler(t *testing.T, user, bcryptHash string) (http.Handler, *Ser } func protectedHandlerDisabled(t *testing.T, disabled bool) http.Handler { - t.Helper() - srv := &Server{cfg: &HTTPServerConfig{AdminAuthDisabled: disabled}, log: testLogger()} - next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(http.StatusOK) - _, _ = fmt.Fprint(w, "ok") - }) - return srv.basicAuthMiddleware()(next) + t.Helper() + srv := &Server{cfg: &HTTPServerConfig{AdminAuthDisabled: disabled}, log: testLogger()} + next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + _, _ = fmt.Fprint(w, "ok") + }) + return srv.basicAuthMiddleware()(next) } func Test_AdminAuth_DeniesWithoutHash(t *testing.T) { @@ -78,17 +78,17 @@ func Test_AdminAuth_AllowsCorrectCreds(t *testing.T) { } func Test_AdminAuth_Disabled_AllowsWithoutCreds(t *testing.T) { - h := protectedHandlerDisabled(t, true) - req := httptest.NewRequest(http.MethodGet, "/", nil) - rr := httptest.NewRecorder() - h.ServeHTTP(rr, req) - require.Equal(t, http.StatusOK, rr.Code) - - // When not disabled, should be unauthorized without hash/creds - h2 := protectedHandlerDisabled(t, false) - rr2 := httptest.NewRecorder() - h2.ServeHTTP(rr2, req) - require.Equal(t, http.StatusUnauthorized, rr2.Code) + h := protectedHandlerDisabled(t, true) + req := httptest.NewRequest(http.MethodGet, "/", nil) + rr := httptest.NewRecorder() + h.ServeHTTP(rr, req) + require.Equal(t, http.StatusOK, rr.Code) + + // When not disabled, should be unauthorized without hash/creds + h2 := protectedHandlerDisabled(t, false) + rr2 := httptest.NewRecorder() + h2.ServeHTTP(rr2, req) + require.Equal(t, http.StatusUnauthorized, rr2.Code) } func Test_PasswordHash(t *testing.T) { diff --git a/httpserver/e2e_test.go b/httpserver/e2e_test.go index 74fa1be..6afd40f 100644 --- a/httpserver/e2e_test.go +++ b/httpserver/e2e_test.go @@ -331,11 +331,12 @@ func createServer(t *testing.T) (*Server, *database.Service, *domain.InmemorySec ah := ports.NewAdminHandler(dbService, mss, getTestLogger()) _ = ah s, err := NewHTTPServer(&HTTPServerConfig{ - DrainDuration: latency, - ListenAddr: listenAddr, - InternalAddr: internalAddr, - AdminAddr: adminAddr, - Log: getTestLogger(), + DrainDuration: latency, + ListenAddr: listenAddr, + InternalAddr: internalAddr, + AdminAddr: adminAddr, + AdminAuthDisabled: true, + Log: getTestLogger(), }, bhh, ah) require.NoError(t, err) return s, dbService, mss From 12b19243ea0c32f91f8b7400161914546554ff1f Mon Sep 17 00:00:00 2001 From: Chris Hager Date: Thu, 4 Sep 2025 12:47:33 +0200 Subject: [PATCH 6/8] simplify --- .github/workflows/checks.yml | 4 ++-- Makefile | 26 ++++++++++++++++++++++---- httpserver/server.go | 16 ++++++---------- 3 files changed, 30 insertions(+), 16 deletions(-) diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index 1459125..be45669 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -38,8 +38,8 @@ jobs: - name: Run migrations run: for file in schema/*.sql; do psql "postgres://postgres:postgres@localhost:5432/postgres?sslmode=disable" -f $file; done - - name: Run unit tests and generate the coverage report - run: RUN_DB_TESTS=1 make test-race + - name: Run unit tests + run: make test-with-db lint: name: Lint diff --git a/Makefile b/Makefile index 99820db..0dea80d 100644 --- a/Makefile +++ b/Makefile @@ -38,6 +38,24 @@ build: ## Build the HTTP server ##@ Test & Development +.PHONY: dev-postgres-up +dev-postgres-up: ## Start the PostgreSQL database for development + docker run -d --name postgres-test -p 5432:5432 -e POSTGRES_USER=postgres -e POSTGRES_PASSWORD=postgres -e POSTGRES_DB=postgres postgres + for file in schema/*.sql; do psql "postgres://postgres:postgres@localhost:5432/postgres?sslmode=disable" -f $file; done + +.PHONY: dev-postgres-down +dev-postgres-down: ## Stop the PostgreSQL database for development + docker rm -f postgres-test + +.PHONY: dev-docker-compose-up +dev-docker-compose-up: ## Start Docker compose + docker compose -f docker/docker-compose.yaml build + docker compose -f docker/docker-compose.yaml up -d + +.PHONY: dev-docker-compose-down +dev-docker-compose-down: ## Stop Docker compose + docker compose -f docker/docker-compose.yaml down + .PHONY: lt lt: lint test ## Run linters and tests (always do this!) @@ -50,12 +68,12 @@ fmt: ## Format the code (use this often) .PHONY: test test: ## Run tests - go test ./... - -.PHONY: test-race -test-race: ## Run tests with race detector go test -race ./... +.PHONY: test-with-db +test-with-db: ## Run tests including live database tests + RUN_DB_TESTS=1 go test -race ./... + .PHONY: lint lint: ## Run linters gofmt -d -s . diff --git a/httpserver/server.go b/httpserver/server.go index d8b38da..32eddc0 100644 --- a/httpserver/server.go +++ b/httpserver/server.go @@ -161,30 +161,26 @@ func (srv *Server) basicAuthMiddleware() func(http.Handler) http.Handler { next.ServeHTTP(w, r) return } - // If no hash configured, deny access (secure-by-default) - if bcryptHash == "" { + + deny := func() { w.Header().Set("WWW-Authenticate", "Basic realm=admin") http.Error(w, http.StatusText(http.StatusUnauthorized), http.StatusUnauthorized) - return } u, p, ok := r.BasicAuth() if !ok { - w.Header().Set("WWW-Authenticate", "Basic realm=admin") - http.Error(w, http.StatusText(http.StatusUnauthorized), http.StatusUnauthorized) + deny() return } - if requiredUser != "" && u != requiredUser { - w.Header().Set("WWW-Authenticate", "Basic realm=admin") - http.Error(w, http.StatusText(http.StatusUnauthorized), http.StatusUnauthorized) + if u != requiredUser { + deny() return } // Compare password to bcrypt hash if err := bcrypt.CompareHashAndPassword([]byte(bcryptHash), []byte(p)); err != nil { - w.Header().Set("WWW-Authenticate", "Basic realm=admin") - http.Error(w, http.StatusText(http.StatusUnauthorized), http.StatusUnauthorized) + deny() return } From 33c7c1f8ed54bd6aaca6690921200c08afe784ab Mon Sep 17 00:00:00 2001 From: Chris Hager Date: Thu, 4 Sep 2025 17:55:48 +0200 Subject: [PATCH 7/8] Require non-empty measurements by default --- AGENTS.md | 2 ++ Makefile | 28 ++++++++++------ README.md | 65 ++++++++++++++------------------------ cmd/httpserver/main.go | 8 ++++- docker/docker-compose.yaml | 1 + go.mod | 2 +- httpserver/e2e_test.go | 45 +++++++++++++++++++++++++- httpserver/handler_test.go | 2 +- ports/admin_handler.go | 18 ++++++++--- 9 files changed, 110 insertions(+), 61 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index bba9910..4769d8b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -18,6 +18,8 @@ ## Coding Style & Conventions - Go formatting is enforced: run `make fmt` (gofmt, gofumpt, gci, go mod tidy). +- Always run `make fmt` and `make lint`. +- If touching `httpserver/e2e_test.go`, always test with database enabled: `make dev-postgres-restart test-with-db dev-postgres-stop`. - Package names: lower-case; exported symbols: PascalCase; locals: camelCase. - Keep files focused; group handlers in `httpserver`, business logic in `application/domain`. - Imports: standard → third-party → local (enforced by gci). diff --git a/Makefile b/Makefile index 0dea80d..ebf8e0d 100644 --- a/Makefile +++ b/Makefile @@ -38,22 +38,26 @@ build: ## Build the HTTP server ##@ Test & Development -.PHONY: dev-postgres-up -dev-postgres-up: ## Start the PostgreSQL database for development +.PHONY: dev-postgres-start +dev-postgres-start: ## Start the PostgreSQL database for development docker run -d --name postgres-test -p 5432:5432 -e POSTGRES_USER=postgres -e POSTGRES_PASSWORD=postgres -e POSTGRES_DB=postgres postgres - for file in schema/*.sql; do psql "postgres://postgres:postgres@localhost:5432/postgres?sslmode=disable" -f $file; done + sleep 3 + for file in schema/*.sql; do psql "postgres://postgres:postgres@localhost:5432/postgres?sslmode=disable" -f $$file; done -.PHONY: dev-postgres-down -dev-postgres-down: ## Stop the PostgreSQL database for development +.PHONY: dev-postgres-stop +dev-postgres-stop: ## Stop the PostgreSQL database for development docker rm -f postgres-test -.PHONY: dev-docker-compose-up -dev-docker-compose-up: ## Start Docker compose +.PHONY: dev-postgres-restart +dev-postgres-restart: dev-postgres-stop dev-postgres-start ## Restart the PostgreSQL database for development + +.PHONY: dev-docker-compose-start +dev-docker-compose-start: ## Start Docker compose docker compose -f docker/docker-compose.yaml build docker compose -f docker/docker-compose.yaml up -d -.PHONY: dev-docker-compose-down -dev-docker-compose-down: ## Stop Docker compose +.PHONY: dev-docker-compose-stop +dev-docker-compose-stop: ## Stop Docker compose docker compose -f docker/docker-compose.yaml down .PHONY: lt @@ -74,6 +78,10 @@ test: ## Run tests test-with-db: ## Run tests including live database tests RUN_DB_TESTS=1 go test -race ./... +.PHONY: integration-tests +integration-tests: ## Run integration tests + ./scripts/ci/integration-test.sh + .PHONY: lint lint: ## Run linters gofmt -d -s . @@ -115,7 +123,7 @@ db-dump: ## Dump the database contents to file 'database.dump' .PHONY: dev-db-setup dev-db-setup: ## Create the basic database entries for testing and development @printf "$(BLUE)Create the allow-all measurements $(NC)\n" - $(CURL) $(CURL_AUTH) --request POST --url http://localhost:8081/api/admin/v1/measurements --data '{"measurement_id": "test1","attestation_type": "test","measurements": {}}' + $(CURL) $(CURL_AUTH) --request POST --url http://localhost:8081/api/admin/v1/measurements --data '{"measurement_id": "test1","attestation_type": "test","measurements": {"11": {"expected": "efa43e0beff151b0f251c4abf48152382b1452b4414dbd737b4127de05ca31f7"}}}' @printf "$(BLUE)Enable the measurements $(NC)\n" $(CURL) $(CURL_AUTH) --request POST --url http://localhost:8081/api/admin/v1/measurements/activation/test1 --data '{"enabled": true}' diff --git a/README.md b/README.md index 406d636..2d22ceb 100644 --- a/README.md +++ b/README.md @@ -51,26 +51,27 @@ curl -u admin:secret http://localhost:8081/api/admin/v1/measurements Local development only: you can disable Admin API auth with `--disable-admin-auth` or `DISABLE_ADMIN_AUTH=1`. This is unsafe; never use in production. +For development/testing, you may also allow empty measurements with `--enable-empty-measurements` or `ENABLE_EMPTY_MEASUREMENTS=1`. In production, keep this disabled and specify at least one expected measurement. + ### Manual setup -**Start the database and the server:** +See [`docs/devenv-setup.md`](./docs/devenv-setup.md) for more information on building and running BuilderHub locally with docker compose. -```bash -# Start a Postgres database container -docker run -d --name postgres-test -p 5432:5432 -e POSTGRES_USER=postgres -e POSTGRES_PASSWORD=postgres -e POSTGRES_DB=postgres postgres +### Testing -# Apply the DB migrations -for file in schema/*.sql; do psql "postgres://postgres:postgres@localhost:5432/postgres?sslmode=disable" -f $file; done +Run the test suite with database tests included: -# Start the server -go run cmd/httpserver/main.go -``` +```bash +# Run tests with real database integration +make dev-postgres-start +make test-with-db -### Using Docker +# Run e2e integration tests, using the docker-compose setup +make integration-tests +``` -- See instructions on using Docker to run the full stack at [`docs/devenv-setup.md`](./docs/devenv-setup.md) -- Also check out the [`docker-compose.yaml`](../docker/docker-compose.yaml) file, which sets up the BuilderHub, a mock proxy, and a Postgres database. -- Finally, there's an e2e api spec test suite you can run: [`./scripts/ci/integration-test.sh`](./scripts/ci/integration-test.sh) +The [integration tests](./scripts/ci/integration-test.sh) use the above mentioned [docker-compose setup](./docker/docker-compose.yaml) +and [Hurl](https://hurl.dev) for API request automation (see the [code here](./scripts/ci/e2e-test.hurl)). ### Example requests @@ -84,34 +85,25 @@ curl localhost:8080/api/l1-builder/v1/configuration curl -X POST localhost:8080/api/l1-builder/v1/register_credentials/rbuilder ``` -### Testing - -Run test suite with database tests included: - -```bash -RUN_DB_TESTS=1 make test -``` - --- ## Contributing **Install dev dependencies** -```bash -go install mvdan.cc/gofumpt@v0.4.0 -go install honnef.co/go/tools/cmd/staticcheck@v0.4.2 -go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.60.3 -go install go.uber.org/nilaway/cmd/nilaway@v0.0.0-20240821220108-c91e71c080b7 -go install github.com/daixiang0/gci@v0.11.2 -``` +Use the same development dependencies as Github CI via [checks.yml](https://github.com/flashbots/builder-hub/blob/required-measurements/.github/workflows/checks.yml#L44-L77) **Lint, test, format** ```bash -make lint -make test +# Quick and easy linting and testing without database +make lt # stands for "make lint and test" + +# Run things manually make fmt +make lint +make dev-postgres-start +make test-with-db ``` --- @@ -230,18 +222,7 @@ Payload } ``` -Note that only the measurements given are expected, and any non-present will be ignored. - -To allow _any_ measurement, use an empty measurements field: -`"measurements": {}`. - -```json -{ - "measurement_id": "test-blanket-allow", - "attestation_type": "azure-tdx", - "measurements": {} -} -``` +Note that only the measurements given are expected, and any non-present will be ignored. The `measurements` object must contain at least one entry. ### Enable/disable measurements diff --git a/cmd/httpserver/main.go b/cmd/httpserver/main.go index d442b6e..7de8a2f 100644 --- a/cmd/httpserver/main.go +++ b/cmd/httpserver/main.go @@ -113,6 +113,11 @@ var flags = []cli.Flag{ Usage: "Use inmemory secrets service for testing", EnvVars: []string{"MOCK_SECRETS"}, }, + &cli.BoolFlag{ + Name: "enable-empty-measurements", + Usage: "allow empty measurements in AddMeasurement (local development/testing only)", + EnvVars: []string{"ENABLE_EMPTY_MEASUREMENTS"}, + }, } func main() { @@ -141,6 +146,7 @@ func runCli(cCtx *cli.Context) error { enablePprof := cCtx.Bool("pprof") drainDuration := time.Duration(cCtx.Int64("drain-seconds")) * time.Second mockSecretsStorage := cCtx.Bool("mock-secrets") + enableEmptyMeasurements := cCtx.Bool("enable-empty-measurements") adminBasicUser := cCtx.String("admin-basic-user") adminPasswordBcrypt := cCtx.String("admin-basic-password-bcrypt") disableAdminAuth := cCtx.Bool("disable-admin-auth") @@ -189,7 +195,7 @@ func runCli(cCtx *cli.Context) error { builderHub := application.NewBuilderHub(db, sm) builderHandler := ports.NewBuilderHubHandler(builderHub, log) - adminHandler := ports.NewAdminHandler(db, sm, log) + adminHandler := ports.NewAdminHandler(db, sm, log, enableEmptyMeasurements) cfg := &httpserver.HTTPServerConfig{ ListenAddr: listenAddr, MetricsAddr: metricsAddr, diff --git a/docker/docker-compose.yaml b/docker/docker-compose.yaml index a335a6a..4345959 100644 --- a/docker/docker-compose.yaml +++ b/docker/docker-compose.yaml @@ -42,6 +42,7 @@ services: INTERNAL_ADDR: "0.0.0.0:8082" METRICS_ADDR: "0.0.0.0:8090" DISABLE_ADMIN_AUTH: "1" # local dev only; do not use in production + ENABLE_EMPTY_MEASUREMENTS: "1" # local dev/testing convenience proxy: image: flashbots/builder-hub-mock-proxy diff --git a/go.mod b/go.mod index 0e3b794..68d2979 100644 --- a/go.mod +++ b/go.mod @@ -16,6 +16,7 @@ require ( github.com/stretchr/testify v1.9.0 github.com/urfave/cli/v2 v2.27.2 go.uber.org/atomic v1.11.0 + golang.org/x/crypto v0.27.0 ) require ( @@ -31,7 +32,6 @@ require ( github.com/valyala/fastrand v1.1.0 // indirect github.com/valyala/histogram v1.2.0 // indirect github.com/xrash/smetrics v0.0.0-20240312152122-5f08fbb34913 // indirect - golang.org/x/crypto v0.27.0 // indirect golang.org/x/sys v0.25.0 // indirect gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect gopkg.in/yaml.v3 v3.0.1 // indirect diff --git a/httpserver/e2e_test.go b/httpserver/e2e_test.go index 6afd40f..c5db561 100644 --- a/httpserver/e2e_test.go +++ b/httpserver/e2e_test.go @@ -308,6 +308,49 @@ func createMeasurement(t *testing.T, s *Server, measurement ports.Measurement) { }) } +func Test_AddMeasurement_EmptyRejected(t *testing.T) { + if os.Getenv("RUN_DB_TESTS") != "1" { + t.Skip("skipping test; RUN_DB_TESTS is not set to 1") + } + s, _, _ := createServer(t) + empty := ports.Measurement{ + Name: "empty-measurement", + AttestationType: "test", + Measurements: map[string]domain.SingleMeasurement{}, + } + status, _ := execRequestNoAuth(t, s.GetAdminRouter(), http.MethodPost, "/api/admin/v1/measurements", empty, nil) + require.Equal(t, http.StatusBadRequest, status) +} + +func Test_AddMeasurement_EmptyAllowedWithFlag(t *testing.T) { + if os.Getenv("RUN_DB_TESTS") != "1" { + t.Skip("skipping test; RUN_DB_TESTS is not set to 1") + } + // Create a server where empty measurements are allowed + dbService := createDbService(t) + mss := domain.NewMockSecretService() + bhs := application.NewBuilderHub(dbService, mss) + bhh := ports.NewBuilderHubHandler(bhs, getTestLogger()) + ah := ports.NewAdminHandler(dbService, mss, getTestLogger(), true) + s, err := NewHTTPServer(&HTTPServerConfig{ + DrainDuration: latency, + ListenAddr: listenAddr, + InternalAddr: internalAddr, + AdminAddr: adminAddr, + AdminAuthDisabled: true, + Log: getTestLogger(), + }, bhh, ah) + require.NoError(t, err) + + empty := ports.Measurement{ + Name: "empty-allowed", + AttestationType: "test", + Measurements: map[string]domain.SingleMeasurement{}, + } + status, _ := execRequestNoAuth(t, s.GetAdminRouter(), http.MethodPost, "/api/admin/v1/measurements", empty, nil) + require.Equal(t, http.StatusOK, status) +} + func createDbService(t *testing.T) *database.Service { t.Helper() dbService, err := database.NewDatabaseService("postgres://postgres:postgres@localhost:5432/postgres?sslmode=disable") @@ -328,7 +371,7 @@ func createServer(t *testing.T) (*Server, *database.Service, *domain.InmemorySec bhs := application.NewBuilderHub(dbService, mss) bhh := ports.NewBuilderHubHandler(bhs, getTestLogger()) _ = bhh - ah := ports.NewAdminHandler(dbService, mss, getTestLogger()) + ah := ports.NewAdminHandler(dbService, mss, getTestLogger(), false) _ = ah s, err := NewHTTPServer(&HTTPServerConfig{ DrainDuration: latency, diff --git a/httpserver/handler_test.go b/httpserver/handler_test.go index 756470c..9566502 100644 --- a/httpserver/handler_test.go +++ b/httpserver/handler_test.go @@ -41,7 +41,7 @@ func Test_Handlers_Healthcheck_Drain_Undrain(t *testing.T) { InternalAddr: internalAddr, AdminAddr: adminAddr, Log: getTestLogger(), - }, ports.NewBuilderHubHandler(nil, getTestLogger()), ports.NewAdminHandler(nil, nil, getTestLogger())) + }, ports.NewBuilderHubHandler(nil, getTestLogger()), ports.NewAdminHandler(nil, nil, getTestLogger(), false)) require.NoError(t, err) { // Check health diff --git a/ports/admin_handler.go b/ports/admin_handler.go index 4a80266..41f4f06 100644 --- a/ports/admin_handler.go +++ b/ports/admin_handler.go @@ -28,13 +28,14 @@ type AdminSecretService interface { } type AdminHandler struct { - builderService AdminBuilderService - secretService AdminSecretService - log *httplog.Logger + builderService AdminBuilderService + secretService AdminSecretService + log *httplog.Logger + allowEmptyMeasurements bool } -func NewAdminHandler(service AdminBuilderService, secretService AdminSecretService, log *httplog.Logger) *AdminHandler { - return &AdminHandler{builderService: service, secretService: secretService, log: log} +func NewAdminHandler(service AdminBuilderService, secretService AdminSecretService, log *httplog.Logger, allowEmptyMeasurements bool) *AdminHandler { + return &AdminHandler{builderService: service, secretService: secretService, log: log, allowEmptyMeasurements: allowEmptyMeasurements} } func (s *AdminHandler) GetActiveConfigForBuilder(w http.ResponseWriter, r *http.Request) { @@ -96,6 +97,13 @@ func (s *AdminHandler) AddMeasurement(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusBadRequest) return } + // Disallow empty measurements map unless explicitly allowed + if len(measurement.Measurements) == 0 && !s.allowEmptyMeasurements { + s.log.Error("measurements field must not be empty") + w.WriteHeader(http.StatusBadRequest) + _, _ = w.Write([]byte("measurements must contain at least one entry")) + return + } err = s.builderService.AddMeasurement(r.Context(), toDomainMeasurement(measurement), false) if err != nil { s.log.Error("failed to add measurement", "error", err) From d1614d736341192d78a65712d771821744a0ed83 Mon Sep 17 00:00:00 2001 From: Chris Hager Date: Thu, 4 Sep 2025 17:59:58 +0200 Subject: [PATCH 8/8] cleanup --- Makefile | 2 +- README.md | 2 +- cmd/httpserver/main.go | 6 +++--- docker/docker-compose.yaml | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Makefile b/Makefile index ebf8e0d..d06076e 100644 --- a/Makefile +++ b/Makefile @@ -123,7 +123,7 @@ db-dump: ## Dump the database contents to file 'database.dump' .PHONY: dev-db-setup dev-db-setup: ## Create the basic database entries for testing and development @printf "$(BLUE)Create the allow-all measurements $(NC)\n" - $(CURL) $(CURL_AUTH) --request POST --url http://localhost:8081/api/admin/v1/measurements --data '{"measurement_id": "test1","attestation_type": "test","measurements": {"11": {"expected": "efa43e0beff151b0f251c4abf48152382b1452b4414dbd737b4127de05ca31f7"}}}' + $(CURL) $(CURL_AUTH) --request POST --url http://localhost:8081/api/admin/v1/measurements --data '{"measurement_id": "test1","attestation_type": "test","measurements": {}}' @printf "$(BLUE)Enable the measurements $(NC)\n" $(CURL) $(CURL_AUTH) --request POST --url http://localhost:8081/api/admin/v1/measurements/activation/test1 --data '{"enabled": true}' diff --git a/README.md b/README.md index 2d22ceb..ac524f8 100644 --- a/README.md +++ b/README.md @@ -51,7 +51,7 @@ curl -u admin:secret http://localhost:8081/api/admin/v1/measurements Local development only: you can disable Admin API auth with `--disable-admin-auth` or `DISABLE_ADMIN_AUTH=1`. This is unsafe; never use in production. -For development/testing, you may also allow empty measurements with `--enable-empty-measurements` or `ENABLE_EMPTY_MEASUREMENTS=1`. In production, keep this disabled and specify at least one expected measurement. +For development/testing, you may also allow empty measurements with `--allow-empty-measurements` or `ALLOW_EMPTY_MEASUREMENTS=1`. In production, keep this disabled and specify at least one expected measurement. ### Manual setup diff --git a/cmd/httpserver/main.go b/cmd/httpserver/main.go index 7de8a2f..c51ed37 100644 --- a/cmd/httpserver/main.go +++ b/cmd/httpserver/main.go @@ -114,9 +114,9 @@ var flags = []cli.Flag{ EnvVars: []string{"MOCK_SECRETS"}, }, &cli.BoolFlag{ - Name: "enable-empty-measurements", + Name: "allow-empty-measurements", Usage: "allow empty measurements in AddMeasurement (local development/testing only)", - EnvVars: []string{"ENABLE_EMPTY_MEASUREMENTS"}, + EnvVars: []string{"ALLOW_EMPTY_MEASUREMENTS"}, }, } @@ -146,7 +146,7 @@ func runCli(cCtx *cli.Context) error { enablePprof := cCtx.Bool("pprof") drainDuration := time.Duration(cCtx.Int64("drain-seconds")) * time.Second mockSecretsStorage := cCtx.Bool("mock-secrets") - enableEmptyMeasurements := cCtx.Bool("enable-empty-measurements") + enableEmptyMeasurements := cCtx.Bool("allow-empty-measurements") adminBasicUser := cCtx.String("admin-basic-user") adminPasswordBcrypt := cCtx.String("admin-basic-password-bcrypt") disableAdminAuth := cCtx.Bool("disable-admin-auth") diff --git a/docker/docker-compose.yaml b/docker/docker-compose.yaml index 4345959..ae32860 100644 --- a/docker/docker-compose.yaml +++ b/docker/docker-compose.yaml @@ -42,7 +42,7 @@ services: INTERNAL_ADDR: "0.0.0.0:8082" METRICS_ADDR: "0.0.0.0:8090" DISABLE_ADMIN_AUTH: "1" # local dev only; do not use in production - ENABLE_EMPTY_MEASUREMENTS: "1" # local dev/testing convenience + ALLOW_EMPTY_MEASUREMENTS: "1" # local dev/testing convenience proxy: image: flashbots/builder-hub-mock-proxy