diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index 8101c93..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 @@ -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/AGENTS.md b/AGENTS.md new file mode 100644 index 0000000..4769d8b --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,40 @@ +# 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). +- 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). +- 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. diff --git a/Makefile b/Makefile index 10df3e0..d06076e 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 @@ -33,6 +38,28 @@ build: ## Build the HTTP server ##@ Test & 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 + sleep 3 + for file in schema/*.sql; do psql "postgres://postgres:postgres@localhost:5432/postgres?sslmode=disable" -f $$file; done + +.PHONY: dev-postgres-stop +dev-postgres-stop: ## Stop the PostgreSQL database for development + docker rm -f postgres-test + +.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-stop +dev-docker-compose-stop: ## Stop Docker compose + docker compose -f docker/docker-compose.yaml down + .PHONY: lt lt: lint test ## Run linters and tests (always do this!) @@ -45,12 +72,16 @@ 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: integration-tests +integration-tests: ## Run integration tests + ./scripts/ci/integration-test.sh + .PHONY: lint lint: ## Run linters gofmt -d -s . @@ -92,16 +123,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/README.md b/README.md index ffff92a..ac524f8 100644 --- a/README.md +++ b/README.md @@ -22,26 +22,56 @@ BuilderHub has these responsibilities: ## Getting started -### Manual setup +### 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) -**Start the database and the server:** +Generate a bcrypt hash (example using htpasswd): ```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 +htpasswd -nbBC 10 "" 'secret' | cut -d: -f2 +``` -# Apply the DB migrations -for file in schema/*.sql; do psql "postgres://postgres:postgres@localhost:5432/postgres?sslmode=disable" -f $file; done +Run with: -# Start the server +```bash +export ADMIN_BASIC_USER=admin +export ADMIN_BASIC_PASSWORD_BCRYPT='$2y$12$...' go run cmd/httpserver/main.go ``` -### Using Docker +Use Basic Auth when calling admin endpoints, e.g.: + +```bash +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 `--allow-empty-measurements` or `ALLOW_EMPTY_MEASUREMENTS=1`. In production, keep this disabled and specify at least one expected measurement. + +### Manual setup + +See [`docs/devenv-setup.md`](./docs/devenv-setup.md) for more information on building and running BuilderHub locally with docker compose. + +### Testing + +Run the test suite with database tests included: + +```bash +# Run tests with real database integration +make dev-postgres-start +make test-with-db + +# 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 @@ -55,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 ``` --- @@ -201,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 @@ -287,4 +297,4 @@ Payload: JSON with secrets, both flattened/unflattened { ... } -``` \ No newline at end of file +``` 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 4abc69e..c51ed37 100644 --- a/cmd/httpserver/main.go +++ b/cmd/httpserver/main.go @@ -73,6 +73,23 @@ 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, 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, @@ -96,6 +113,11 @@ var flags = []cli.Flag{ Usage: "Use inmemory secrets service for testing", EnvVars: []string{"MOCK_SECRETS"}, }, + &cli.BoolFlag{ + Name: "allow-empty-measurements", + Usage: "allow empty measurements in AddMeasurement (local development/testing only)", + EnvVars: []string{"ALLOW_EMPTY_MEASUREMENTS"}, + }, } func main() { @@ -124,6 +146,10 @@ 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("allow-empty-measurements") + 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, @@ -142,13 +168,16 @@ 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 { log.Error("failed to create database", "err", err) return err } - defer db.Close() + defer db.Close() //nolint:errcheck var sm ports.AdminSecretService @@ -166,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, @@ -175,6 +204,10 @@ func runCli(cCtx *cli.Context) error { Log: log, EnablePprof: enablePprof, + AdminBasicUser: adminBasicUser, + AdminPasswordBcrypt: adminPasswordBcrypt, + AdminAuthDisabled: disableAdminAuth, + DrainDuration: drainDuration, GracefulShutdownDuration: 30 * time.Second, ReadTimeout: 60 * time.Second, diff --git a/docker/docker-compose.yaml b/docker/docker-compose.yaml index 6693ba4..ae32860 100644 --- a/docker/docker-compose.yaml +++ b/docker/docker-compose.yaml @@ -41,6 +41,8 @@ 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 + ALLOW_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/auth_middleware_test.go b/httpserver/auth_middleware_test.go new file mode 100644 index 0000000..9624742 --- /dev/null +++ b/httpserver/auth_middleware_test.go @@ -0,0 +1,118 @@ +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 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) + 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) +} + +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" + 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) +} diff --git a/httpserver/e2e_test.go b/httpserver/e2e_test.go index 74fa1be..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,14 +371,15 @@ 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, - 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 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/httpserver/server.go b/httpserver/server.go index 7fff421..32eddc0 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,11 @@ type HTTPServerConfig struct { GracefulShutdownDuration time.Duration ReadTimeout time.Duration WriteTimeout time.Duration + + // Admin API auth + AdminBasicUser string + AdminPasswordBcrypt string + AdminAuthDisabled bool } type Server struct { @@ -113,6 +119,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 +148,47 @@ 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 srv.cfg.AdminAuthDisabled { + // pass-through when disabled (for local development only) + next.ServeHTTP(w, r) + return + } + + deny := func() { + w.Header().Set("WWW-Authenticate", "Basic realm=admin") + http.Error(w, http.StatusText(http.StatusUnauthorized), http.StatusUnauthorized) + } + + u, p, ok := r.BasicAuth() + if !ok { + deny() + return + } + + if u != requiredUser { + deny() + return + } + + // Compare password to bcrypt hash + if err := bcrypt.CompareHashAndPassword([]byte(bcryptHash), []byte(p)); err != nil { + deny() + return + } + + next.ServeHTTP(w, r) + }) + } +} + func (srv *Server) _stringFromFile(fn string) (string, error) { content, err := os.ReadFile(fn) if err != nil { 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)