fix: require a serviceAuth entry for every serviceDomains host

docs/extensions/README.md

@@ -146,6 +146,9 @@ the mounted host project directory and installs happen at `docker build` time:
 - `files/workspace/**` is copied into the project at container **start** and
   **never clobbers** an existing host file (warns and skips).
 - `commands.startup` running as root is **rejected loudly at load**.
+- the `network.serviceDomains` ↔ `network.serviceAuth` pairing must be complete
+  in both directions; an incomplete mapping is **rejected loudly at load**
+  (see below).
 - `commands.install` (mixins only) is woven into the build; an `install.sh`
   sidecar wins if a feature ships both.
 
@@ -408,7 +411,14 @@ Secrets are split across two `spec.yaml` sections:
   `network.serviceAuth.<service-id>` (`headerName`, optional `valueFormat`)
   describe how the gateway injects that credential as an HTTP header when it
   proxies requests to those hosts. The service-id in `serviceAuth` and
-  `serviceDomains` is the same key used under `credentials.sources`.
+  `serviceDomains` is the same key used under `credentials.sources`. Every
+  service-id in `serviceDomains` must also have a `serviceAuth` entry; without
+  one the mapping is inert — the hosts never reach the allowlist and the
+  credential is injected as a raw env value — so the spec is rejected at load.
+  To make hosts reachable without injecting a credential, list them under
+  `network.allowedDomains` instead. The pairing is required in both directions:
+  a `serviceAuth` entry also needs at least one host, either from
+  `serviceDomains` or from its own `hosts` list.
 
 The placeholder convention is Go `fmt`-style `%s`, not `{secret}`. An empty
 `valueFormat` means "inject the raw secret value" with no wrapping (see

docs/extensions/adding-a-tool.md

@@ -74,6 +74,8 @@ Secrets are split across `credentials` and `network` (see the README's
 - `network.serviceDomains` (`host -> service-id`) + `network.serviceAuth.<id>`
   (`headerName`, optional `valueFormat` with a Go `fmt`-style `%s`): how the
   gateway injects the credential as an HTTP header when proxying to those hosts.
+  Every service-id in `serviceDomains` needs a `serviceAuth` entry; hosts that
+  only need to be reachable belong under `network.allowedDomains`.
 - `providers[]`: enclave-native auth provider — `name`, `credentials` (a list
   of `credentials.sources` keys), `authFiles` (relative to `sandbox.configDir`),
   `authSession` (`mode: any|all` + `checks`), `oauthPorts`, and

internal/config/conformance_test.go

@@ -55,15 +55,22 @@ func TestExtensionSurfaceGolden(t *testing.T) {
 		assertGolden(t, "tool-ext-"+name, ext)
 	}
 
-	feats, err := ListFeatures(paths)
+	// Enumerate the feature spec names rather than going through ListFeatures:
+	// that helper warns and skips specs it cannot load, so a broken built-in
+	// feature spec would drop out of the snapshot set instead of failing here.
+	feats, err := listSpecNames(paths, KindMixin)
 	if err != nil {
-		t.Fatalf("ListFeatures: %v", err)
+		t.Fatalf("listSpecNames(features): %v", err)
 	}
 	if len(feats) == 0 {
-		t.Fatal("ListFeatures returned no features; expected the real extensions/features tree")
+		t.Fatal("found no feature specs; expected the real extensions/features tree")
 	}
-	for _, ext := range feats {
-		assertGolden(t, "feature-"+ext.Name, ext)
+	for _, name := range feats {
+		ext, err := LoadFeatureExtension(paths, name)
+		if err != nil {
+			t.Fatalf("LoadFeatureExtension(%s): %v", name, err)
+		}
+		assertGolden(t, "feature-"+name, ext)
 	}
 }
 

internal/config/extension.go

@@ -91,25 +91,41 @@ func LoadFeatureExtension(paths model.Paths, name string) (model.Extension, erro
 
 // ListTools returns all tool extension names from both built-in and user extension roots.
 func ListTools(paths model.Paths) ([]string, error) {
-	names, err := listExtensionNames(paths.ToolsDir, paths.UserToolsDir)
+	return listSpecNames(paths, KindSandbox)
+}
+
+// listSpecNames returns the sorted names of the given kind's extensions, from
+// both the built-in and the user root, that carry a spec document. Unlike
+// ListFeatures it never loads those specs, so a name it returns may still fail
+// to load.
+func listSpecNames(paths model.Paths, kind string) ([]string, error) {
+	var builtinDir, userDir string
+	switch kind {
+	case KindSandbox:
+		builtinDir, userDir = paths.ToolsDir, paths.UserToolsDir
+	case KindMixin:
+		builtinDir, userDir = paths.FeaturesDir, paths.UserFeaturesDir
+	default:
+		return nil, fmt.Errorf("unknown extension kind %q", kind)
+	}
+
+	names, err := listExtensionNames(builtinDir, userDir)
 	if err != nil {
 		return nil, err
 	}
 
-	var tools []string
+	var withSpec []string
 	for _, name := range names {
-		if hasSpecFile(paths, name, KindSandbox) {
-			tools = append(tools, name)
+		if hasSpecFile(paths, name, kind) {
+			withSpec = append(withSpec, name)
 		}
 	}
-
-	sort.Strings(tools)
-	return tools, nil
+	return withSpec, nil
 }
 
 // ListFeatures returns all feature extensions from both built-in and user roots, sorted by priority.
 func ListFeatures(paths model.Paths) ([]model.Extension, error) {
-	names, err := listExtensionNames(paths.FeaturesDir, paths.UserFeaturesDir)
+	names, err := listSpecNames(paths, KindMixin)
 	if err != nil {
 		return nil, err
 	}
@@ -118,7 +134,10 @@ func ListFeatures(paths model.Paths) ([]model.Extension, error) {
 	for _, name := range names {
 		ext, err := LoadFeatureExtension(paths, name)
 		if err != nil {
-			continue // Skip invalid extensions
+			// A feature dropped here is silently missing from the built image,
+			// so surface why. Spec-less directories are already filtered out.
+			specWarn(fmt.Sprintf("feature %q: %v; skipping", name, err))
+			continue
 		}
 		features = append(features, ext)
 	}

internal/config/profile_test.go

@@ -318,7 +318,7 @@ func TestLoadProfileRejectsSecretReleaseWithEmptyHosts(t *testing.T) {
 	}`)
 
 	_, err := LoadProfile(paths, "tool")
-	if err == nil || !strings.Contains(err.Error(), "hosts must contain at least one domain pattern") {
+	if err == nil || !strings.Contains(err.Error(), "has no hosts to release the credential to") {
 		t.Fatalf("LoadProfile() error = %v, want empty-hosts validation error", err)
 	}
 }

internal/config/secrets_test.go

@@ -105,6 +105,28 @@ func TestValidateAndNormalizeSecretConfigsInvalidParser(t *testing.T) {
 	}
 }
 
+// TestValidateAndNormalizeSecretConfigsEmptyReleaseHosts covers the model-layer
+// guard directly. Spec loading now rejects a hostless serviceAuth entry earlier
+// and with a message that names serviceDomains, so this is the only path left
+// that exercises the normalizeHosts check.
+func TestValidateAndNormalizeSecretConfigsEmptyReleaseHosts(t *testing.T) {
+	in := map[string]model.SecretConfig{
+		"demo": {
+			EnvVars: []string{"DEMO_KEY"},
+			Release: &model.SecretReleaseConfig{
+				HTTP: &model.HTTPSecretReleaseConfig{Header: "authorization"},
+			},
+		},
+	}
+	_, err := validateAndNormalizeSecretConfigs(in)
+	if err == nil {
+		t.Fatalf("expected empty release hosts error, got nil")
+	}
+	if !strings.Contains(err.Error(), "hosts must contain at least one domain pattern") {
+		t.Fatalf("error = %v, want it to mention the empty hosts list", err)
+	}
+}
+
 func TestValidateAndNormalizeSecretConfigsEmptyFilePath(t *testing.T) {
 	in := map[string]model.SecretConfig{
 		"demo": {

internal/config/spec_map.go

@@ -17,10 +17,18 @@ import (
 
 // validateServiceAuthMappings fails loudly when a network.serviceAuth or
 // network.serviceDomains service id does not map to a declared
-// credentials.sources id. buildSecrets only walks credentials.sources, so an
-// unmatched id (e.g. a typo) is otherwise silently dropped: the secret ends up
-// with no HTTP release rule and its token is injected as a raw env value
-// instead of a proxy-swapped placeholder — a secret-leak risk.
+// credentials.sources id, and when the serviceDomains ↔ serviceAuth pairing is
+// incomplete in either direction.
+//
+// buildSecrets only builds an HTTP release rule for ids present in both
+// credentials.sources and network.serviceAuth, so a serviceDomains id with no
+// serviceAuth entry (a typo, or a dropped serviceAuth line) is silently inert:
+// the token is injected as a raw env value instead of a proxy-swapped
+// placeholder — a secret-leak risk — and the serviceDomains hosts drop out of
+// the release hosts unioned into the effective allowlist. The reverse, a
+// serviceAuth entry with no hosts from either source, would otherwise be
+// rejected downstream by normalizeHosts, but with a message that never names
+// serviceDomains; catching it here points both directions at the same remedy.
 func validateServiceAuthMappings(doc specDocument, specPath string) error {
 	if doc.Network == nil {
 		return nil
@@ -31,19 +39,48 @@ func validateServiceAuthMappings(doc specDocument, specPath string) error {
 			sources[id] = struct{}{}
 		}
 	}
+	// Unknown ids come first: a typo'd id also breaks the pairing, and reporting
+	// the pairing gap would send the author to the wrong line.
 	for id := range doc.Network.ServiceAuth {
 		if _, ok := sources[id]; !ok {
 			return fmt.Errorf("%s: network.serviceAuth[%q] has no matching credentials.sources entry", specPath, id)
 		}
 	}
+	hostedServices := map[string]struct{}{}
 	for host, id := range doc.Network.ServiceDomains {
 		if _, ok := sources[id]; !ok {
 			return fmt.Errorf("%s: network.serviceDomains[%q] references service %q with no matching credentials.sources entry", specPath, host, id)
 		}
+		if strings.TrimSpace(host) != "" {
+			hostedServices[id] = struct{}{}
+		}
+	}
+
+	// Then the pairing, in both directions.
+	for id, auth := range doc.Network.ServiceAuth {
+		if _, ok := hostedServices[id]; !ok && !hasNonBlank(auth.Hosts) {
+			return fmt.Errorf("%s: network.serviceAuth[%q] has no hosts to release the credential to (add a hosts list, or map hosts to this service under network.serviceDomains)", specPath, id)
+		}
+	}
+	for host, id := range doc.Network.ServiceDomains {
+		if _, ok := doc.Network.ServiceAuth[id]; !ok {
+			return fmt.Errorf("%s: network.serviceDomains[%q] references service %q with no matching network.serviceAuth entry (add one, or list the hosts under network.allowedDomains instead)", specPath, host, id)
+		}
 	}
 	return nil
 }
 
+// hasNonBlank reports whether hosts holds at least one entry that survives the
+// blank-stripping normalizeHosts applies later.
+func hasNonBlank(hosts []string) bool {
+	for _, host := range hosts {
+		if strings.TrimSpace(host) != "" {
+			return true
+		}
+	}
+	return false
+}
+
 // validateProxyManaged fails loudly when an environment.proxyManaged entry
 // does not name a declared credentials.sources env alias. proxyManaged selects
 // which aliases carry the proxy-swapped placeholder; a typo'd entry would

internal/config/spec_map_test.go

@@ -8,6 +8,7 @@
 package config
 
 import (
+	"strings"
 	"testing"
 
 	"sigs.k8s.io/yaml"
@@ -57,6 +58,89 @@ network:
 		}
 	})
 
+	t.Run("serviceDomains id without serviceAuth fails", func(t *testing.T) {
+		doc := mustDoc(t, `
+schemaVersion: "1"
+kind: mixin
+name: github-cli
+credentials:
+  sources:
+    github-token: { env: [GH_TOKEN] }
+    github-enterprise-token: { env: [GH_ENTERPRISE_TOKEN] }
+network:
+  serviceDomains: { api.github.com: github-token, ghe.com: github-enterprise-token }
+  serviceAuth: { github-token: { headerName: authorization, valueFormat: "Bearer %s" } }
+`)
+		// Without the serviceAuth entry the enterprise token gets no release
+		// rule: ghe.com drops out of the release hosts unioned into the
+		// allowlist and the token is injected raw instead of proxy-swapped.
+		if err := validateServiceAuthMappings(doc, "github-cli/spec.yaml"); err == nil {
+			t.Fatal("expected error for serviceDomains id with no matching network.serviceAuth entry")
+		}
+	})
+
+	t.Run("serviceAuth id without hosts fails", func(t *testing.T) {
+		doc := mustDoc(t, `
+schemaVersion: "1"
+kind: mixin
+name: github-cli
+credentials:
+  sources:
+    github-token: { env: [GH_TOKEN] }
+network:
+  serviceAuth: { github-token: { headerName: authorization } }
+`)
+		// The mirror image of the case above: with no hosts from either
+		// serviceDomains or serviceAuth.hosts, the release rule has nothing to
+		// release the credential to.
+		err := validateServiceAuthMappings(doc, "github-cli/spec.yaml")
+		if err == nil {
+			t.Fatal("expected error for serviceAuth id with no hosts")
+		}
+		if !strings.Contains(err.Error(), "network.serviceDomains") {
+			t.Fatalf("error = %v, want it to name network.serviceDomains as a remedy", err)
+		}
+	})
+
+	t.Run("unknown id is reported before the pairing gap", func(t *testing.T) {
+		doc := mustDoc(t, `
+schemaVersion: "1"
+kind: mixin
+name: github-cli
+credentials:
+  sources:
+    github-token: { env: [GH_TOKEN] }
+network:
+  serviceDomains: { api.github.com: github-tokn }
+  serviceAuth: { github-token: { headerName: authorization } }
+`)
+		// The typo leaves github-token hostless too, but pointing the author at
+		// the serviceAuth entry would hide the actual mistake.
+		err := validateServiceAuthMappings(doc, "github-cli/spec.yaml")
+		if err == nil {
+			t.Fatal("expected error for the typo'd serviceDomains id")
+		}
+		if !strings.Contains(err.Error(), "no matching credentials.sources entry") {
+			t.Fatalf("error = %v, want it to name the unknown credentials.sources id", err)
+		}
+	})
+
+	t.Run("serviceAuth hosts without serviceDomains pass", func(t *testing.T) {
+		doc := mustDoc(t, `
+schemaVersion: "1"
+kind: mixin
+name: gitlab-cli
+credentials:
+  sources:
+    gitlab-token: { env: [GITLAB_TOKEN] }
+network:
+  serviceAuth: { gitlab-token: { headerName: private-token, hosts: [gitlab.com] } }
+`)
+		if err := validateServiceAuthMappings(doc, "gitlab-cli/spec.yaml"); err != nil {
+			t.Fatalf("unexpected error for serviceAuth-declared hosts: %v", err)
+		}
+	})
+
 	t.Run("matching ids pass", func(t *testing.T) {
 		doc := mustDoc(t, `
 schemaVersion: "1"