diff --git a/AGENTS.md b/AGENTS.md index 659dd59c..9177434b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -51,8 +51,10 @@ and `sprue` (the upload service; mirror its patterns where relevant). - Build / vet / test: `go build ./... && go vet ./... && go test ./...`. Run all three after changes — this is the standard loop. -- Run locally: `go run ./cmd serve` (flags: `--storage=memory --vault=memory` to - avoid external deps; see `cmd/main.go` / `pkg/config`). +- Run locally: `go run ./cmd serve --revocation-service-id= + --revocation-service-url=` (both required, no default; add + `--storage=memory --vault=memory` to avoid external deps; see `cmd/main.go` / + `pkg/config`). - Postgres and OpenBao-backed tests use testcontainers and **skip when Docker is unavailable** (`internal/testutil`). `go test ./...` passes without Docker but only exercises the memory backends; run with Docker for full coverage. diff --git a/README.md b/README.md index dd9a4740..be150b1c 100644 --- a/README.md +++ b/README.md @@ -115,11 +115,13 @@ the product/plan DID tenants are registered under. | Key | Flag | Env var | Default | | --- | --- | --- | --- | -| `revocation.service_id` | `--revocation-service-id` | `HILT_REVOCATION_SERVICE_ID` | `did:web:revoke.fil-forge.com` | -| `revocation.service_url` | `--revocation-service-url` | `HILT_REVOCATION_SERVICE_URL` | `https://revoke.fil-forge.com` | +| `revocation.service_id` | `--revocation-service-id` | `HILT_REVOCATION_SERVICE_ID` | required | +| `revocation.service_url` | `--revocation-service-url` | `HILT_REVOCATION_SERVICE_URL` | required | The Swarf service DID + HTTP endpoint Hilt publishes UCAN revocations to when a -delegation it issued is withdrawn (e.g. when an access key is deleted). +delegation it issued is withdrawn (e.g. when an access key is deleted). Both +are required, and `hilt serve` exits at startup when either is unset. Each +stage points at its own Swarf, which only knows that stage's tenants. ## Container images diff --git a/cmd/main.go b/cmd/main.go index 3b060607..7f612c85 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -69,8 +69,8 @@ func main() { serveCmd.Flags().String("upload-proofs", "", "Upload service proofs: an encoded UCAN container or a path to a file containing one") // revocation service config - serveCmd.Flags().String("revocation-service-id", "did:web:revoke.fil-forge.com", "Revocation service DID") - serveCmd.Flags().String("revocation-service-url", "https://revoke.fil-forge.com", "Revocation service HTTP endpoint") + serveCmd.Flags().String("revocation-service-id", "", "Revocation service DID (required)") + serveCmd.Flags().String("revocation-service-url", "", "Revocation service HTTP endpoint (required)") rootCmd.AddCommand(serveCmd) rootCmd.AddCommand(client.Cmd) diff --git a/pkg/config/config.go b/pkg/config/config.go index 01d40f21..6ae6974e 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -43,7 +43,7 @@ type Config struct { // RevocationConfig holds settings for the Swarf revocation service, which Hilt // publishes UCAN revocations to when a delegation it issued is withdrawn (e.g. -// when an access key is deleted). +// when an access key is deleted). Both fields are required by `hilt serve`. type RevocationConfig struct { // ServiceID is the Swarf service's DID (e.g. "did:web:swarf.example.com"). ServiceID string `mapstructure:"service_id"` @@ -185,8 +185,9 @@ func SetDefaults(v *viper.Viper) { v.SetDefault("upload.service_url", "https://upload.fil-forge.com") v.SetDefault("upload.product_id", "did:web:hilt.fil-forge.com") - v.SetDefault("revocation.service_id", "did:web:revoke.fil-forge.com") - v.SetDefault("revocation.service_url", "https://revoke.fil-forge.com") + // revocation.service_id and revocation.service_url have no default: every + // deployment names its own stage's Swarf, and `serve` refuses to start + // without one (see fx.NewRevocationClient). } // flagBindings maps each config key to its serve-command flag name. diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index fd761437..29aa97e7 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -25,3 +25,15 @@ func TestLoadEnvOnly(t *testing.T) { require.Equal(t, "https://upload.example.com", cfg.Upload.ServiceURL) require.Equal(t, "https://revocation.example.com", cfg.Revocation.ServiceURL) } + +// TestLoadRevocationHasNoDefault verifies the revocation service settings stay +// empty when nothing configures them, so `hilt serve` refuses to start instead +// of publishing revocations to another stage's Swarf. +func TestLoadRevocationHasNoDefault(t *testing.T) { + t.Setenv("HILT_REVOCATION_SERVICE_ID", "") + t.Setenv("HILT_REVOCATION_SERVICE_URL", "") + + cfg, err := config.Load("") + require.NoError(t, err) + require.Equal(t, config.RevocationConfig{}, cfg.Revocation) +} diff --git a/pkg/fx/revocation.go b/pkg/fx/revocation.go index a845196d..6ec0c05e 100644 --- a/pkg/fx/revocation.go +++ b/pkg/fx/revocation.go @@ -1,6 +1,7 @@ package fx import ( + "errors" "fmt" "net/url" @@ -29,7 +30,17 @@ var RevocationModule = fx.Module("revocation", // NewRevocationClient builds the Swarf revocation-service client from // configuration. It takes no issuer: revocations are signed by the tenant that // issued the revoked delegations, which is passed per publish call. +// +// Both settings are required. Neither has a default, so a deployment that +// forgets them fails at startup rather than publishing to another stage's +// Swarf. func NewRevocationClient(cfg config.RevocationConfig) (*swarfclient.Client, error) { + if cfg.ServiceID == "" { + return nil, errors.New("revocation.service_id is required: set HILT_REVOCATION_SERVICE_ID or --revocation-service-id") + } + if cfg.ServiceURL == "" { + return nil, errors.New("revocation.service_url is required: set HILT_REVOCATION_SERVICE_URL or --revocation-service-url") + } serviceID, err := did.Parse(cfg.ServiceID) if err != nil { return nil, fmt.Errorf("parsing revocation.service_id %q: %w", cfg.ServiceID, err) diff --git a/pkg/fx/revocation_test.go b/pkg/fx/revocation_test.go index 9e240589..c9b9e18a 100644 --- a/pkg/fx/revocation_test.go +++ b/pkg/fx/revocation_test.go @@ -5,8 +5,10 @@ import ( "github.com/fil-forge/hilt/pkg/config" appfx "github.com/fil-forge/hilt/pkg/fx" + "github.com/fil-forge/hilt/pkg/grant" "github.com/fil-forge/libforge/testutil" "github.com/stretchr/testify/require" + "go.uber.org/fx" ) func TestNewRevocationClient(t *testing.T) { @@ -21,6 +23,26 @@ func TestNewRevocationClient(t *testing.T) { require.Equal(t, serviceID, c.ServiceID) }) + missingCases := map[string]struct { + cfg config.RevocationConfig + wantMessage string + }{ + "service DID is unset": { + cfg: config.RevocationConfig{ServiceURL: "http://swarf.test"}, + wantMessage: "revocation.service_id is required: set HILT_REVOCATION_SERVICE_ID or --revocation-service-id", + }, + "service URL is unset": { + cfg: config.RevocationConfig{ServiceID: serviceID.String()}, + wantMessage: "revocation.service_url is required: set HILT_REVOCATION_SERVICE_URL or --revocation-service-url", + }, + } + for desc, tc := range missingCases { + t.Run("errors when "+desc, func(t *testing.T) { + _, err := appfx.NewRevocationClient(tc.cfg) + require.EqualError(t, err, tc.wantMessage) + }) + } + t.Run("invalid service DID errors", func(t *testing.T) { _, err := appfx.NewRevocationClient(config.RevocationConfig{ ServiceID: "not-a-did", @@ -37,3 +59,17 @@ func TestNewRevocationClient(t *testing.T) { require.ErrorContains(t, err, "revocation.service_url") }) } + +// TestRevocationModuleRequiresConfig checks that an app built without the +// revocation settings fails while it is being constructed, which is when +// `hilt serve` exits, rather than when it first publishes a revocation. +func TestRevocationModuleRequiresConfig(t *testing.T) { + app := fx.New( + fx.Supply(&config.Config{}), + appfx.ConfigModule, + appfx.RevocationModule, + fx.NopLogger, + fx.Invoke(func(grant.RevocationPublisher) {}), + ) + require.ErrorContains(t, app.Err(), "revocation.service_id is required") +}