Merge branch 'master' into trustedRoot

Author: rdimitrov

.github/dependabot.yml

@@ -26,3 +26,8 @@ updates:
       prefix: "chore"
       include: "scope"
     open-pull-requests-limit: 10
+    groups:
+      # Bundle all GitHub Actions updates into a single pull request
+      github-actions:
+        patterns:
+          - "*"

.github/workflows/linting.yml

@@ -49,3 +49,6 @@ jobs:
           # When `install-mode` is `goinstall` the value can be v1.2.3, `latest`, or the hash of a commit.
           version: v2.11.4
           args: --timeout 5m --verbose
+          # Skip config schema verification: it fetches the JSON schema from
+          # golangci-lint.run at runtime and flakes on network timeouts.
+          verify: false

metadata/multirepo/multirepo.go

@@ -21,6 +21,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"maps"
 	"os"
 	"path/filepath"
 	"regexp"
@@ -35,6 +36,14 @@ import (
 // components or is otherwise invalid for use as a directory name.
 var ErrInvalidRepoName = errors.New("invalid repository name")
 
+// ErrMissingRepoURL is returned when a repository listed in the map file has no
+// usable URL, i.e. an empty URL list or an empty first URL.
+var ErrMissingRepoURL = errors.New("repository has no URL configured")
+
+// ErrUnknownMappingRepo is returned when a mapping in the map file references a
+// repository that is not declared in the top-level repositories object.
+var ErrUnknownMappingRepo = errors.New("mapping references an unknown repository")
+
 // validRepoNamePattern defines the allowed characters for repository names.
 // Names must start with an alphanumeric character and may contain alphanumeric
 // characters, dots, hyphens, and underscores. This prevents path traversal
@@ -113,19 +122,23 @@ func NewConfig(repoMap []byte, roots map[string][]byte) (*MultiRepoConfig, error
 
 // New returns a multi-repository TUF client. All repositories described in the provided map file are initialized too
 func New(config *MultiRepoConfig) (*MultiRepoClient, error) {
+	if config == nil {
+		return nil, fmt.Errorf("no multi-repository config provided")
+	}
+
+	// validate the map file before initializing anything, so that a malformed map
+	// file is reported as such instead of surfacing later as an obscure
+	// initialization failure or a panic during target lookup
+	if err := validateRepoMap(config.RepoMap); err != nil {
+		return nil, err
+	}
+
 	// create a multi repo client instance
 	client := &MultiRepoClient{
 		Config:     config,
 		TUFClients: map[string]*updater.Updater{},
 	}
 
-	// validate repository names before using them as filesystem paths
-	for repoName := range config.RepoMap.Repositories {
-		if err := validateRepoName(repoName); err != nil {
-			return nil, fmt.Errorf("repository %q: %w", repoName, err)
-		}
-	}
-
 	// create TUF clients for each repository listed in the map file
 	if err := client.initTUFClients(); err != nil {
 		return nil, err
@@ -138,13 +151,9 @@ func (client *MultiRepoClient) initTUFClients() error {
 	log := metadata.GetLogger()
 
 	// loop through each repository listed in the map file and initialize it
+	// note: the map file has already been validated by validateRepoMap, so each
+	// repository is guaranteed to have a usable URL at index 0
 	for repoName, repoURL := range client.Config.RepoMap.Repositories {
-		
-		// Make sure we have at least one repo URL
-		if len(repoURL) == 0 || repoURL[0] == "" {
-			return fmt.Errorf("repository %q has no URL configured", repoName)
-		}
-		
 		log.Info("Initializing", "name", repoName, "url", repoURL[0])
 
 		// get the trusted root file from the location specified in the map file relevant to its path
@@ -393,6 +402,50 @@ func (cfg *MultiRepoConfig) EnsurePathsExist() error {
 	return nil
 }
 
+// validateRepoMap checks that a map file is internally consistent before any
+// repository is initialized. It enforces that every repository name is safe to
+// use as a filesystem path, that every repository has a usable URL, and that
+// every repository referenced by a mapping is actually declared.
+//
+// Validating up front keeps New atomic: it either returns a fully initialized
+// client or fails without having created cache directories for a subset of the
+// repositories. It also makes the reported error deterministic, which map
+// iteration order alone would not guarantee.
+func validateRepoMap(repoMap *MultiRepoMapType) error {
+	if repoMap == nil {
+		return fmt.Errorf("no repository map provided")
+	}
+
+	// sort the repository names so that a map file with more than one problem
+	// always reports the same error rather than a random one
+	for _, repoName := range slices.Sorted(maps.Keys(repoMap.Repositories)) {
+		if err := validateRepoName(repoName); err != nil {
+			return fmt.Errorf("repository %q: %w", repoName, err)
+		}
+
+		// only the first URL is used, as the client supports a single mirror per
+		// repository for the time being
+		if repoURL := repoMap.Repositories[repoName]; len(repoURL) == 0 || repoURL[0] == "" {
+			return fmt.Errorf("repository %q: %w", repoName, ErrMissingRepoURL)
+		}
+	}
+
+	// every repository named in a mapping must have a corresponding TUF client,
+	// otherwise GetTargetInfo would dereference a nil client during target lookup
+	for i, eachMap := range repoMap.Mapping {
+		if eachMap == nil {
+			return fmt.Errorf("mapping at index %d is null", i)
+		}
+		for _, repoName := range eachMap.Repositories {
+			if _, ok := repoMap.Repositories[repoName]; !ok {
+				return fmt.Errorf("mapping at index %d: %w - %s", i, ErrUnknownMappingRepo, repoName)
+			}
+		}
+	}
+
+	return nil
+}
+
 // validateRepoName checks that a repository name is safe to use as a directory
 // component. Repository names must start with an alphanumeric character and
 // contain only alphanumeric characters, dots, hyphens, and underscores.

metadata/multirepo/multirepo_test.go

@@ -122,4 +122,118 @@ func TestNewRejectsInvalidRepoNames(t *testing.T) {
 			}
 		})
 	}
-}
+}
+
+func TestNewRejectsRepositoriesWithoutURL(t *testing.T) {
+	tests := []struct {
+		name     string
+		repoURLs string
+	}{
+		{"empty URL list", `[]`},
+		{"null URL list", `null`},
+		{"empty URL string", `[""]`},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			mapJSON := []byte(`{
+				"repositories": {
+					"my-repo": ` + tt.repoURLs + `
+				},
+				"mapping": []
+			}`)
+
+			rootBytes := []byte(`{"signatures":[],"signed":{}}`)
+
+			cfg, err := NewConfig(mapJSON, map[string][]byte{"my-repo": rootBytes})
+			if err != nil {
+				t.Fatalf("NewConfig() unexpected error: %v", err)
+			}
+
+			_, err = New(cfg)
+			if err == nil {
+				t.Fatalf("New() should reject repository with URLs %s", tt.repoURLs)
+			}
+
+			if !errors.Is(err, ErrMissingRepoURL) {
+				t.Errorf("New() error should wrap ErrMissingRepoURL, got: %v", err)
+			}
+		})
+	}
+}
+
+func TestNewRejectsMappingWithUnknownRepository(t *testing.T) {
+	// A mapping that references a repository absent from the top-level
+	// "repositories" object leaves no TUF client for that name, which makes
+	// GetTargetInfo dereference a nil *updater.Updater.
+	mapJSON := []byte(`{
+		"repositories": {
+			"real-repo": ["https://example.com/repo"]
+		},
+		"mapping": [
+			{
+				"paths": ["*"],
+				"repositories": ["typo-repo"],
+				"threshold": 1,
+				"terminating": true
+			}
+		]
+	}`)
+
+	rootBytes := []byte(`{"signatures":[],"signed":{}}`)
+
+	cfg, err := NewConfig(mapJSON, map[string][]byte{"real-repo": rootBytes})
+	if err != nil {
+		t.Fatalf("NewConfig() unexpected error: %v", err)
+	}
+
+	_, err = New(cfg)
+	if err == nil {
+		t.Fatal("New() should reject a mapping referencing an unknown repository")
+	}
+
+	if !errors.Is(err, ErrUnknownMappingRepo) {
+		t.Errorf("New() error should wrap ErrUnknownMappingRepo, got: %v", err)
+	}
+}
+
+func TestNewRejectsNullMapping(t *testing.T) {
+	// "mapping": [null] unmarshals into a []*Mapping holding a nil element,
+	// which GetTargetInfo would dereference while walking the mappings.
+	mapJSON := []byte(`{
+		"repositories": {
+			"real-repo": ["https://example.com/repo"]
+		},
+		"mapping": [null]
+	}`)
+
+	rootBytes := []byte(`{"signatures":[],"signed":{}}`)
+
+	cfg, err := NewConfig(mapJSON, map[string][]byte{"real-repo": rootBytes})
+	if err != nil {
+		t.Fatalf("NewConfig() unexpected error: %v", err)
+	}
+
+	_, err = New(cfg)
+	if err == nil {
+		t.Fatal("New() should reject a null mapping entry")
+	}
+}
+
+func TestNewRejectsConfigWithoutRepoMap(t *testing.T) {
+	// MultiRepoConfig has exported fields, so callers can build one directly
+	// without going through NewConfig and leave RepoMap unset.
+	_, err := New(&MultiRepoConfig{})
+	if err == nil {
+		t.Fatal("New() should reject a config with no repository map")
+	}
+}
+
+func TestNewRejectsNilConfig(t *testing.T) {
+	// NewConfig returns a nil config alongside its error, so a caller that
+	// ignores the error passes nil straight into New.
+	_, err := New(nil)
+	if err == nil {
+		t.Fatal("New() should reject a nil config")
+	}
+}