fix(config): require the upload service settings

AGENTS.md

@@ -51,10 +51,11 @@ 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 --revocation-service-id=<did>
-  --revocation-service-url=<url>` (both required, no default; add
-  `--storage=memory --vault=memory` to avoid external deps; see `cmd/main.go` /
-  `pkg/config`).
+- Run locally: `go run ./cmd serve --upload-service-id=<did>
+  --upload-service-url=<url> --upload-product-id=<did>
+  --revocation-service-id=<did> --revocation-service-url=<url>` (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.

README.md

@@ -102,21 +102,23 @@ 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` | `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.
+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)
 
-| 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

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

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).
 
 	// revocation.service_id and revocation.service_url have no default: every
 	// deployment names its own stage's Swarf, and `serve` refuses to start

pkg/config/config_test.go

@@ -26,6 +26,20 @@ func TestLoadEnvOnly(t *testing.T) {
 	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)
+}
+
 // 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.

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(

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 {

pkg/fx/upload_test.go

@@ -5,13 +5,15 @@ 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"
 	"github.com/fil-forge/ucantone/ucan/command"
 	"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")
+}