From e26e73f7766999efa4d55089f3156664fb55adc6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Fri, 16 Oct 2026 17:59:47 +0200 Subject: [PATCH 1/2] fix(config): require the upload service settings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit upload.service_id, upload.service_url and upload.product_id defaulted to prod's Sprue and product DID. A stage that forgot to set them would provision its tenants and buckets on prod Sprue, the same failure that the revocation defaults caused for tenant deletes. Drop the three defaults and make NewUploadClient refuse empty values, so `hilt serve` exits at startup naming the missing env var and flag. The product DID was optional in the constructor, but tenant provisioning always passes it to /customer/add, so serve cannot work without it. The `hilt client` commands never build the upload client. Assisted-by: Claude:claude-opus-5-5 Signed-off-by: Miroslav Bajtoš --- AGENTS.md | 6 +++-- README.md | 10 +++++--- cmd/main.go | 6 ++--- pkg/config/config.go | 9 ++++--- pkg/config/config_test.go | 14 +++++++++++ pkg/fx/rpc_test.go | 2 +- pkg/fx/upload.go | 26 +++++++++++++------- pkg/fx/upload_test.go | 52 +++++++++++++++++++++++++++++++++++++++ 8 files changed, 102 insertions(+), 23 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 57524210..2dd8923b 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 --upload-service-id= + --upload-service-url= --upload-product-id=` (all 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..d05e4c98 100644 --- a/README.md +++ b/README.md @@ -104,12 +104,14 @@ Pre-shared bearer token required on Tenant API requests. | Key | Flag | Env var | Default | | --- | --- | --- | --- | -| `upload.service_id` | `--upload-service-id` | `HILT_UPLOAD_SERVICE_ID` | `did:web:upload.fil-forge.com` | -| `upload.service_url` | `--upload-service-url` | `HILT_UPLOAD_SERVICE_URL` | `https://upload.fil-forge.com` | -| `upload.product_id` | `--upload-product-id` | `HILT_UPLOAD_PRODUCT_ID` | `did:web:hilt.fil-forge.com` | +| `upload.service_id` | `--upload-service-id` | `HILT_UPLOAD_SERVICE_ID` | required | +| `upload.service_url` | `--upload-service-url` | `HILT_UPLOAD_SERVICE_URL` | required | +| `upload.product_id` | `--upload-product-id` | `HILT_UPLOAD_PRODUCT_ID` | required | The Sprue service DID + HTTP endpoint Hilt calls to provision bucket space, and -the product/plan DID tenants are registered under. +the product/plan DID tenants are registered under. All three are required, and +`hilt serve` exits at startup when any of them is unset. Each stage points at +its own Sprue. ### Revocation service (Swarf) diff --git a/cmd/main.go b/cmd/main.go index 3b060607..92f88015 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -63,9 +63,9 @@ func main() { serveCmd.Flags().String("partner-key", "", "CSV partner bearer key(s) required on Tenant API requests (prefer HILT_AUTH_PARTNER_KEY env var or config file to avoid exposing via process args)") // upload service config - serveCmd.Flags().String("upload-service-id", "did:web:upload.fil-forge.com", "Upload service DID") - serveCmd.Flags().String("upload-service-url", "https://upload.fil-forge.com", "Upload service HTTP endpoint") - serveCmd.Flags().String("upload-product-id", "did:web:hilt.fil-forge.com", "Upload service product/plan DID that tenants are registered under") + serveCmd.Flags().String("upload-service-id", "", "Upload service DID (required)") + serveCmd.Flags().String("upload-service-url", "", "Upload service HTTP endpoint (required)") + serveCmd.Flags().String("upload-product-id", "", "Upload service product/plan DID that tenants are registered under (required)") serveCmd.Flags().String("upload-proofs", "", "Upload service proofs: an encoded UCAN container or a path to a file containing one") // revocation service config diff --git a/pkg/config/config.go b/pkg/config/config.go index 01d40f21..6fe060b4 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -52,7 +52,8 @@ type RevocationConfig struct { } // UploadConfig holds settings for the Sprue upload service, which Hilt calls to -// provision a bucket's storage space. +// provision a bucket's storage space. ServiceID, ServiceURL and ProductID are +// required by `hilt serve`. type UploadConfig struct { // ServiceID is the Sprue service's DID (e.g. "did:web:sprue.example.com"). ServiceID string `mapstructure:"service_id"` @@ -181,9 +182,9 @@ func SetDefaults(v *viper.Viper) { v.SetDefault("plc.directory", "https://plc.directory") - v.SetDefault("upload.service_id", "did:web:upload.fil-forge.com") - v.SetDefault("upload.service_url", "https://upload.fil-forge.com") - v.SetDefault("upload.product_id", "did:web:hilt.fil-forge.com") + // upload.service_id, upload.service_url and upload.product_id have no + // default: every deployment names its own stage's Sprue and product, and + // `serve` refuses to start without them (see fx.NewUploadClient). v.SetDefault("revocation.service_id", "did:web:revoke.fil-forge.com") v.SetDefault("revocation.service_url", "https://revoke.fil-forge.com") diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index fd761437..fcde2501 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -25,3 +25,17 @@ 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) } + +// TestLoadUploadHasNoDefault verifies the upload service settings stay empty +// when nothing configures them, so `hilt serve` refuses to start instead of +// provisioning tenants and buckets on another stage's Sprue. +func TestLoadUploadHasNoDefault(t *testing.T) { + t.Setenv("HILT_UPLOAD_SERVICE_ID", "") + t.Setenv("HILT_UPLOAD_SERVICE_URL", "") + t.Setenv("HILT_UPLOAD_PRODUCT_ID", "") + t.Setenv("HILT_UPLOAD_PROOFS", "") + + cfg, err := config.Load("") + require.NoError(t, err) + require.Equal(t, config.UploadConfig{}, cfg.Upload) +} diff --git a/pkg/fx/rpc_test.go b/pkg/fx/rpc_test.go index c6ed173d..f2332f99 100644 --- a/pkg/fx/rpc_test.go +++ b/pkg/fx/rpc_test.go @@ -45,7 +45,7 @@ func newRPCApp(t *testing.T) (routes []server.Route, admin []server.Route, srv * cfg := &config.Config{ Storage: config.StorageConfig{Type: config.StorageTypeMemory}, Vault: config.VaultConfig{Type: config.VaultTypeMemory}, - Upload: config.UploadConfig{ServiceID: testutil.RandomDID(t).String(), ServiceURL: "http://sprue.test"}, + Upload: config.UploadConfig{ServiceID: testutil.RandomDID(t).String(), ServiceURL: "http://sprue.test", ProductID: testutil.RandomDID(t).String()}, Revocation: config.RevocationConfig{ServiceID: testutil.RandomDID(t).String(), ServiceURL: "http://swarf.test"}, } app := fx.New( diff --git a/pkg/fx/upload.go b/pkg/fx/upload.go index c8ce6c1b..2fb5d470 100644 --- a/pkg/fx/upload.go +++ b/pkg/fx/upload.go @@ -1,6 +1,7 @@ package fx import ( + "errors" "fmt" "net/url" "os" @@ -18,7 +19,20 @@ import ( // NewUploadClient builds the Sprue upload-service client from configuration. Its // proof store — the delegations it presents to Sprue — is loaded from // upload.proofs (see [uploadProofs]). +// +// The service DID, URL and product DID are required. None has a default, so a +// deployment that forgets one fails at startup rather than provisioning on +// another stage's Sprue. func NewUploadClient(id identity.Identity, cfg config.UploadConfig, logger *zap.Logger) (*upload.Client, error) { + if cfg.ServiceID == "" { + return nil, errors.New("upload.service_id is required: set HILT_UPLOAD_SERVICE_ID or --upload-service-id") + } + if cfg.ServiceURL == "" { + return nil, errors.New("upload.service_url is required: set HILT_UPLOAD_SERVICE_URL or --upload-service-url") + } + if cfg.ProductID == "" { + return nil, errors.New("upload.product_id is required: set HILT_UPLOAD_PRODUCT_ID or --upload-product-id") + } serviceID, err := did.Parse(cfg.ServiceID) if err != nil { return nil, fmt.Errorf("parsing upload.service_id %q: %w", cfg.ServiceID, err) @@ -27,15 +41,9 @@ func NewUploadClient(id identity.Identity, cfg config.UploadConfig, logger *zap. if err != nil { return nil, fmt.Errorf("parsing upload.service_url %q: %w", cfg.ServiceURL, err) } - // The product DID is optional here: it is only consumed when registering - // tenants as customers. Leave it undefined when unset so the client remains - // usable for the bucket-provisioning flows that do not need it. - var product did.DID - if cfg.ProductID != "" { - product, err = did.Parse(cfg.ProductID) - if err != nil { - return nil, fmt.Errorf("parsing upload.product_id %q: %w", cfg.ProductID, err) - } + product, err := did.Parse(cfg.ProductID) + if err != nil { + return nil, fmt.Errorf("parsing upload.product_id %q: %w", cfg.ProductID, err) } proofs, err := uploadProofs(cfg.Proofs) if err != nil { diff --git a/pkg/fx/upload_test.go b/pkg/fx/upload_test.go index c2ea4112..25e91d4e 100644 --- a/pkg/fx/upload_test.go +++ b/pkg/fx/upload_test.go @@ -5,6 +5,7 @@ import ( "path/filepath" "testing" + "github.com/fil-forge/hilt/pkg/client/upload" "github.com/fil-forge/hilt/pkg/config" appfx "github.com/fil-forge/hilt/pkg/fx" "github.com/fil-forge/libforge/testutil" @@ -12,6 +13,7 @@ import ( "github.com/fil-forge/ucantone/ucan/container" "github.com/fil-forge/ucantone/ucan/delegation" "github.com/stretchr/testify/require" + "go.uber.org/fx" "go.uber.org/zap" ) @@ -23,6 +25,7 @@ func TestNewUploadClient(t *testing.T) { return config.UploadConfig{ ServiceID: testutil.RandomDID(t).String(), ServiceURL: "http://sprue.test", + ProductID: testutil.RandomDID(t).String(), Proofs: proofs, } } @@ -66,8 +69,57 @@ func TestNewUploadClient(t *testing.T) { require.NotNil(t, c) }) + missingCases := map[string]struct { + unset func(*config.UploadConfig) + wantMessage string + }{ + "service DID is unset": { + unset: func(c *config.UploadConfig) { c.ServiceID = "" }, + wantMessage: "upload.service_id is required: set HILT_UPLOAD_SERVICE_ID or --upload-service-id", + }, + "service URL is unset": { + unset: func(c *config.UploadConfig) { c.ServiceURL = "" }, + wantMessage: "upload.service_url is required: set HILT_UPLOAD_SERVICE_URL or --upload-service-url", + }, + "product DID is unset": { + unset: func(c *config.UploadConfig) { c.ProductID = "" }, + wantMessage: "upload.product_id is required: set HILT_UPLOAD_PRODUCT_ID or --upload-product-id", + }, + } + for desc, tc := range missingCases { + t.Run("errors when "+desc, func(t *testing.T) { + cfg := baseCfg("") + tc.unset(&cfg) + _, err := appfx.NewUploadClient(id, cfg, zap.NewNop()) + require.EqualError(t, err, tc.wantMessage) + }) + } + + t.Run("invalid product DID errors", func(t *testing.T) { + cfg := baseCfg("") + cfg.ProductID = "not-a-did" + _, err := appfx.NewUploadClient(id, cfg, zap.NewNop()) + require.ErrorContains(t, err, "upload.product_id") + }) + t.Run("invalid proofs (neither container nor file) errors", func(t *testing.T) { _, err := appfx.NewUploadClient(id, baseCfg("not-a-container-and-not-a-file"), zap.NewNop()) require.Error(t, err) }) } + +// TestUploadClientRequiresConfig checks that an app built without the upload +// settings fails while it is being constructed, which is when `hilt serve` +// exits, rather than when it first calls Sprue. +func TestUploadClientRequiresConfig(t *testing.T) { + app := fx.New( + fx.Supply(&config.Config{}), + appfx.ConfigModule, + appfx.LoggerModule, + appfx.IdentityModule, + fx.Provide(appfx.NewUploadClient), + fx.NopLogger, + fx.Invoke(func(*upload.Client) {}), + ) + require.ErrorContains(t, app.Err(), "upload.service_id is required") +} From 08b79cb80144f47aaf72a3ce1ed5c1ffac674826 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Tue, 20 Oct 2026 14:11:57 +0200 Subject: [PATCH 2/2] docs: list production values as config examples MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The upload and revocation settings have no default, so the README tables show the production DIDs and URLs under an "Example" column. Assisted-by: Claude:claude-opus-5-5 Signed-off-by: Miroslav Bajtoš --- README.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 95e0d5fd..d947e848 100644 --- a/README.md +++ b/README.md @@ -102,11 +102,11 @@ Pre-shared bearer token required on Tenant API requests. ### Upload service (Sprue) -| Key | Flag | Env var | Default | +| Key | Flag | Env var | Example | | --- | --- | --- | --- | -| `upload.service_id` | `--upload-service-id` | `HILT_UPLOAD_SERVICE_ID` | required | -| `upload.service_url` | `--upload-service-url` | `HILT_UPLOAD_SERVICE_URL` | required | -| `upload.product_id` | `--upload-product-id` | `HILT_UPLOAD_PRODUCT_ID` | required | +| `upload.service_id` | `--upload-service-id` | `HILT_UPLOAD_SERVICE_ID` | `did:web:upload.fil-forge.com` | +| `upload.service_url` | `--upload-service-url` | `HILT_UPLOAD_SERVICE_URL` | `https://upload.fil-forge.com` | +| `upload.product_id` | `--upload-product-id` | `HILT_UPLOAD_PRODUCT_ID` | `did:web:hilt.fil-forge.com` | The Sprue service DID + HTTP endpoint Hilt calls to provision bucket space, and the product/plan DID tenants are registered under. All three are required, and @@ -115,10 +115,10 @@ its own Sprue. ### Revocation service (Swarf) -| Key | Flag | Env var | Default | +| Key | Flag | Env var | Example | | --- | --- | --- | --- | -| `revocation.service_id` | `--revocation-service-id` | `HILT_REVOCATION_SERVICE_ID` | required | -| `revocation.service_url` | `--revocation-service-url` | `HILT_REVOCATION_SERVICE_URL` | required | +| `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` | 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). Both