Require non-empty measurements by default

Author: metachris

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).

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}'

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
 

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,

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

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

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,

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

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)