Escape delegated role name when reading cached metadata

metadata/updater/updater.go

@@ -420,8 +420,10 @@ func (update *Updater) loadTargets(roleName, parentName string) (*metadata.Metad
 	if ok {
 		return role, nil
 	}
-	// try to read local targets
-	data, err := update.loadLocalMetadata(filepath.Join(update.cfg.LocalMetadataDir, roleName))
+	// try to read local targets; the file name must be escaped the same way
+	// persistMetadata writes it, otherwise a delegated role whose name contains
+	// characters like a space or "/" is never found in the cache.
+	data, err := os.ReadFile(update.localMetadataPath(roleName))
 	if err != nil {
 		// this means there's no existing local target file so we should proceed downloading it without the need to UpdateDelegatedTargets
 		log.Info("Local role does not exist", "role", roleName)
@@ -595,7 +597,7 @@ func (update *Updater) persistMetadata(roleName string, data []byte) error {
 		return nil
 	}
 	// caching enabled, proceed with persisting the metadata locally
-	fileName := filepath.Join(update.cfg.LocalMetadataDir, fmt.Sprintf("%s.json", url.PathEscape(roleName)))
+	fileName := update.localMetadataPath(roleName)
 	// create a temporary file
 	file, err := os.CreateTemp(update.cfg.LocalMetadataDir, "tuf_tmp")
 	if err != nil {
@@ -666,6 +668,17 @@ func (update *Updater) generateTargetFilePath(tf *metadata.TargetFiles) (string,
 	return filepath.Join(update.cfg.LocalTargetsDir, url.PathEscape(tf.Path)), nil
 }
 
+// localMetadataPath returns the path of the cache file for roleName inside
+// LocalMetadataDir. The role name is URL-escaped so a delegated role whose name
+// contains a path separator or other special character maps to a single file
+// inside the directory. persistMetadata and the delegated read in loadTargets
+// share this helper so the two sides always agree on the file name. The ".json"
+// suffix is appended before joining so a role named "." or ".." cannot be
+// collapsed by filepath.Join and escape the directory.
+func (update *Updater) localMetadataPath(roleName string) string {
+	return filepath.Join(update.cfg.LocalMetadataDir, fmt.Sprintf("%s.json", url.PathEscape(roleName)))
+}
+
 // loadLocalMetadata reads a local <roleName>.json file and returns its bytes
 func (update *Updater) loadLocalMetadata(roleName string) ([]byte, error) {
 	return os.ReadFile(fmt.Sprintf("%s.json", roleName))

metadata/updater/updater_delegated_cache_test.go

@@ -0,0 +1,68 @@
+// Copyright 2024 The Update Framework Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License
+//
+// SPDX-License-Identifier: Apache-2.0
+//
+
+package updater
+
+import (
+	"net/url"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/theupdateframework/go-tuf/v2/metadata/config"
+)
+
+// TestDelegatedMetadataCacheRoundTrip checks that metadata persisted for a
+// delegated role can be read back from the cache. persistMetadata URL-escapes
+// the role name in the file name, so the read side must escape it the same way.
+// Before this was fixed the read used the raw role name, so a delegated role
+// whose name contained a space, a "/", or a "." segment was written under one
+// file name and looked for under another, causing a permanent cache miss (the
+// metadata was re-downloaded on every refresh) and, for a name like "..", a
+// read outside LocalMetadataDir.
+func TestDelegatedMetadataCacheRoundTrip(t *testing.T) {
+	roleNames := []string{
+		"role with space",
+		"a/b",
+		"..",
+		".",
+		"100%",
+	}
+
+	for _, roleName := range roleNames {
+		t.Run(roleName, func(t *testing.T) {
+			dir := t.TempDir()
+			update := &Updater{cfg: &config.UpdaterConfig{LocalMetadataDir: dir}}
+			data := []byte("delegated metadata for " + roleName)
+
+			err := update.persistMetadata(roleName, data)
+			assert.NoError(t, err)
+
+			// The cache file lives inside LocalMetadataDir under the escaped name.
+			path := update.localMetadataPath(roleName)
+			assert.Equal(t, dir, filepath.Dir(path))
+			assert.Equal(t, url.PathEscape(roleName)+".json", filepath.Base(path))
+
+			// The read loadTargets performs recovers the persisted bytes.
+			got, err := os.ReadFile(path)
+			assert.NoError(t, err)
+			assert.Equal(t, data, got)
+		})
+	}
+}