diff --git a/metadata/updater/updater.go b/metadata/updater/updater.go index d84c869d..ec9755f9 100644 --- a/metadata/updater/updater.go +++ b/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 .json file and returns its bytes func (update *Updater) loadLocalMetadata(roleName string) ([]byte, error) { return os.ReadFile(fmt.Sprintf("%s.json", roleName)) diff --git a/metadata/updater/updater_delegated_cache_test.go b/metadata/updater/updater_delegated_cache_test.go new file mode 100644 index 00000000..1914932b --- /dev/null +++ b/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) + }) + } +}