diff --git a/core/common/utils.go b/core/common/utils.go index 90df8021..078d497e 100644 --- a/core/common/utils.go +++ b/core/common/utils.go @@ -17,9 +17,11 @@ package common import ( "context" "crypto/md5" + "crypto/sha256" "encoding/hex" "fmt" - "path/filepath" + "io/fs" + "net/url" "sort" "strings" @@ -46,11 +48,17 @@ const ( DefaultMetadataMapChunkSize = 50_000 ) -// ToShortRemote returns the short remote name given a git ssh remote string. -// For example, "git@github:uber/tango" will return "uber/tango". +// ToShortRemote returns the repository path from an SSH or URL remote. +// For example, "git@github:uber/tango" and +// "https://github.com/uber/tango" both return "uber/tango". func ToShortRemote(remote string) string { - strs := strings.Split(remote, ":") - return strs[len(strs)-1] + if parsed, err := url.Parse(remote); err == nil && parsed.Scheme != "" && parsed.Host != "" { + return strings.TrimLeft(parsed.Path, "/") + } + if idx := strings.LastIndex(remote, ":"); idx >= 0 { + remote = remote[idx+1:] + } + return strings.TrimLeft(remote, "/") } // GetGraphByTreeHash returns the cache path for the target graph by treehash. @@ -59,11 +67,16 @@ func ToShortRemote(remote string) string { // requestOptions is folded into the key when any of its fields affect computation // (today: extra_exclude_files_regex). Empty/nil ⇒ legacy path unchanged. func GetGraphByTreeHash(remote, treehash string, strategy tangopb.ComputationStrategy, requestOptions *tangopb.RequestOptions) string { - path := filepath.Join(ToShortRemote(remote), "graphs", treehash, strategy.String()) + key := strings.Join([]string{ + safeCachePath(ToShortRemote(remote)), + "graphs", + safeCacheSegment(treehash), + strategy.String(), + }, "/") if hash := HashRequestOptions(requestOptions); hash != "" { - path += "_requests-options-" + hash + key += "_requests-options-" + hash } - return path + return key } // GetTreehashCachePath returns the cache path for the treehash mapping. @@ -71,11 +84,15 @@ func GetGraphByTreeHash(remote, treehash string, strategy tangopb.ComputationStr // requests), so neither requestOptions nor the computation strategy is part // of this key. func GetTreehashCachePath(buildDescription *tangopb.BuildDescription) string { - path := filepath.Join(ToShortRemote(buildDescription.Remote), "treehashes", fmt.Sprintf("base-sha-%s", buildDescription.BaseSha)) + key := strings.Join([]string{ + safeCachePath(ToShortRemote(buildDescription.Remote)), + "treehashes", + "base-sha-" + safeCacheSegment(buildDescription.BaseSha), + }, "/") if len(buildDescription.Requests) > 0 { - path += "_request-urls-" + GetReqURLsHash(buildDescription.Requests) + key += "_request-urls-" + GetReqURLsHash(buildDescription.Requests) } - return path + return key } // GetComparedTargetsCachePath returns the cache path for a compared target graph result. @@ -84,11 +101,36 @@ func GetTreehashCachePath(buildDescription *tangopb.BuildDescription) string { // requestOptions is folded into the key when any of its fields affect computation. // Empty/nil ⇒ legacy path unchanged. func GetComparedTargetsCachePath(remote, treehash1, treehash2 string, requestOptions *tangopb.RequestOptions) string { - path := filepath.Join(ToShortRemote(remote), "compared-targets", treehash1+"_"+treehash2) + key := strings.Join([]string{ + safeCachePath(ToShortRemote(remote)), + "compared-targets", + safeCacheSegment(treehash1) + "_" + safeCacheSegment(treehash2), + }, "/") if hash := HashRequestOptions(requestOptions); hash != "" { - path += "_requests-options-" + hash + key += "_requests-options-" + hash + } + return key +} + +func safeCachePath(value string) string { + if value != "" && value != "." && fs.ValidPath(value) && !strings.ContainsAny(value, "\\:\x00") { + return value } - return path + return unsafeCacheComponent(value) +} + +func safeCacheSegment(value string) string { + if !strings.Contains(value, "/") { + if safe := safeCachePath(value); safe == value { + return value + } + } + return unsafeCacheComponent(value) +} + +func unsafeCacheComponent(value string) string { + hash := sha256.Sum256([]byte(value)) + return "unsafe-" + hex.EncodeToString(hash[:]) } // GetReqURLsHash returns a fixed-length MD5 hash of the sorted request URLs. diff --git a/core/common/utils_test.go b/core/common/utils_test.go index 4172da07..5d34e413 100644 --- a/core/common/utils_test.go +++ b/core/common/utils_test.go @@ -18,7 +18,9 @@ import ( "context" "crypto/md5" "fmt" - "path/filepath" + "io/fs" + "path" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -49,6 +51,16 @@ func TestToShortRemote(t *testing.T) { remote: "git@github:org/project/sub", want: "org/project/sub", }, + { + name: "HTTPS remote", + remote: "https://github.com/uber/tango.git", + want: "uber/tango.git", + }, + { + name: "SSH URL remote", + remote: "ssh://git@github.com/uber/tango.git", + want: "uber/tango.git", + }, } for _, tt := range tests { tt := tt @@ -66,7 +78,7 @@ func TestGetGraphByTreeHash(t *testing.T) { // Nil/empty options ⇒ no request-options suffix. got := GetGraphByTreeHash(remote, treehash, strategy, nil) - assert.Equal(t, filepath.Join("uber/tango", "graphs", treehash, strategy.String()), got) + assert.Equal(t, path.Join("uber/tango", "graphs", treehash, strategy.String()), got) assert.Equal(t, got, GetGraphByTreeHash(remote, treehash, strategy, &pb.RequestOptions{})) // Different strategies ⇒ different keys. @@ -99,7 +111,7 @@ func TestGetTreehashCachePath(t *testing.T) { h := md5.New() h.Write([]byte("custom://foo/bar")) h.Write([]byte("github://org/repo/pull/1")) - want := filepath.Join("uber/tango", "treehashes", "base-sha-deadbeef") + "_request-urls-" + fmt.Sprintf("%x", h.Sum(nil)) + want := path.Join("uber/tango", "treehashes", "base-sha-deadbeef") + "_request-urls-" + fmt.Sprintf("%x", h.Sum(nil)) assert.Equal(t, want, got) } @@ -149,7 +161,7 @@ func TestGetReqURLsHash(t *testing.T) { func TestGetComparedTargetsCachePath(t *testing.T) { t.Parallel() got := GetComparedTargetsCachePath("git@github:uber/tango", "abc", "def", nil) - assert.Equal(t, filepath.Join("uber/tango", "compared-targets", "abc_def"), got) + assert.Equal(t, path.Join("uber/tango", "compared-targets", "abc_def"), got) // Nil/empty options ⇒ legacy path. assert.Equal(t, got, GetComparedTargetsCachePath("git@github:uber/tango", "abc", "def", &pb.RequestOptions{})) @@ -158,6 +170,32 @@ func TestGetComparedTargetsCachePath(t *testing.T) { assert.NotEqual(t, got, GetComparedTargetsCachePath("git@github:uber/tango", "abc", "def", &pb.RequestOptions{ExtraExcludeFilesRegex: []string{"foo.*"}})) } +func TestCachePathsEncodeUnsafeComponents(t *testing.T) { + t.Parallel() + + graphKey := GetGraphByTreeHash("../../outside", "../tree", pb.COMPUTATION_STRATEGY_NATIVE, nil) + assertSafeCacheKey(t, graphKey) + assert.Equal(t, graphKey, GetGraphByTreeHash("../../outside", "../tree", pb.COMPUTATION_STRATEGY_NATIVE, nil)) + + treehashKey := GetTreehashCachePath(&pb.BuildDescription{ + Remote: `..\..\outside`, + BaseSha: "../../base", + }) + assertSafeCacheKey(t, treehashKey) + + comparedKey := GetComparedTargetsCachePath("/outside", "../one", "two/../../three", nil) + assertSafeCacheKey(t, comparedKey) +} + +func assertSafeCacheKey(t *testing.T, key string) { + t.Helper() + assert.True(t, fs.ValidPath(key), key) + assert.NotEqual(t, ".", key) + assert.NotContains(t, key, "\\") + assert.NotContains(t, key, ":") + assert.False(t, strings.HasPrefix(key, "/"), key) +} + func TestChunkTargets(t *testing.T) { t.Parallel() diff --git a/core/itg/cache/BUILD.bazel b/core/itg/cache/BUILD.bazel index 4c841ffe..d7ebbdf3 100644 --- a/core/itg/cache/BUILD.bazel +++ b/core/itg/cache/BUILD.bazel @@ -1,4 +1,4 @@ -load("@rules_go//go:def.bzl", "go_library") +load("@rules_go//go:def.bzl", "go_library", "go_test") go_library( name = "cache", @@ -10,3 +10,15 @@ go_library( "//core/storage", ], ) + +go_test( + name = "cache_test", + srcs = ["cache_test.go"], + embed = [":cache"], + deps = [ + "//core/itg/graph", + "//core/storage", + "@com_github_stretchr_testify//assert", + "@com_github_stretchr_testify//require", + ], +) diff --git a/core/itg/cache/cache.go b/core/itg/cache/cache.go index 846f043e..a786591f 100644 --- a/core/itg/cache/cache.go +++ b/core/itg/cache/cache.go @@ -19,7 +19,8 @@ import ( "context" "encoding/gob" "fmt" - "path/filepath" + "io/fs" + "path" "slices" "strconv" "strings" @@ -64,10 +65,16 @@ var CompareKeyFunc = func(a Key, b Key) int { // EmptyKey means no cache found. var EmptyKey = Key{} -// toStorageKey converts a cache key to its storage key: itg/{remote}/{date}/{committime}_{sha} -func (k *Key) toStorageKey() string { +// toStorageKey converts a cache key to its storage key: itg/{remote}/{date}/{committime}_{sha}. +func (k *Key) toStorageKey() (string, error) { + if err := validateRemote(k.Remote); err != nil { + return "", err + } + if err := validateBaseSHA(k.BaseSha); err != nil { + return "", err + } date := time.Unix(k.BaseCommitTimeSecond, 0).UTC().Format("2006-01-02") - return filepath.Join(keyPrefix, k.Remote, date, fmt.Sprintf("%d_%s", k.BaseCommitTimeSecond, k.BaseSha)) + return path.Join(keyPrefix, k.Remote, date, fmt.Sprintf("%d_%s", k.BaseCommitTimeSecond, k.BaseSha)), nil } // NewStorageCache creates a new cache backed by a storage.Storage implementation. @@ -82,7 +89,11 @@ type storageCache struct { } func (c *storageCache) Put(ctx context.Context, optimizedGraph *graph.OptimizedGraph, key Key) error { - exists, err := c.storage.Exists(ctx, key.toStorageKey()) + storageKey, err := key.toStorageKey() + if err != nil { + return err + } + exists, err := c.storage.Exists(ctx, storageKey) if err != nil { return err } @@ -94,19 +105,23 @@ func (c *storageCache) Put(ctx context.Context, optimizedGraph *graph.OptimizedG if err := gob.NewEncoder(&buf).Encode(optimizedGraph); err != nil { return err } - return c.storage.Put(ctx, storage.UploadRequest{Key: key.toStorageKey(), Reader: &buf}) + return c.storage.Put(ctx, storage.UploadRequest{Key: storageKey, Reader: &buf}) } func (c *storageCache) Get(ctx context.Context, key Key) (*graph.OptimizedGraph, error) { - resp, err := c.storage.Get(ctx, storage.DownloadRequest{Key: key.toStorageKey()}) + storageKey, err := key.toStorageKey() + if err != nil { + return nil, err + } + resp, err := c.storage.Get(ctx, storage.DownloadRequest{Key: storageKey}) if err != nil { - return nil, fmt.Errorf("download graph %s: %w", key.toStorageKey(), err) + return nil, fmt.Errorf("download graph %s: %w", storageKey, err) } defer resp.ReadCloser.Close() var optimizedGraph graph.OptimizedGraph if err := gob.NewDecoder(resp.ReadCloser).Decode(&optimizedGraph); err != nil { - return nil, fmt.Errorf("decode graph %s: %w", key.toStorageKey(), err) + return nil, fmt.Errorf("decode graph %s: %w", storageKey, err) } for _, t := range optimizedGraph.OptimizedTargets { if t.Hash == nil { @@ -117,7 +132,10 @@ func (c *storageCache) Get(ctx context.Context, key Key) (*graph.OptimizedGraph, } func (c *storageCache) FloorKey(ctx context.Context, remote string, targetTimeSecond int64) (Key, error) { - remotePrefix := keyPrefix + remote + "/" + if err := validateRemote(remote); err != nil { + return EmptyKey, err + } + remotePrefix := path.Join(keyPrefix, remote) + "/" allKeys, err := c.storage.List(ctx, remotePrefix) if err != nil { return EmptyKey, err @@ -155,6 +173,20 @@ func (c *storageCache) FloorKey(ctx context.Context, remote string, targetTimeSe return cacheKeys[idx], nil } +func validateRemote(remote string) error { + if remote == "" || remote == "." || !fs.ValidPath(remote) || strings.ContainsAny(remote, "\\:\x00") { + return fmt.Errorf("invalid remote %q for cache key", remote) + } + return nil +} + +func validateBaseSHA(baseSHA string) error { + if baseSHA == "" || strings.Contains(baseSHA, "/") || !fs.ValidPath(baseSHA) || strings.ContainsAny(baseSHA, "\\:\x00") { + return fmt.Errorf("invalid base SHA %q for cache key", baseSHA) + } + return nil +} + func binarySearch(cacheKeys []Key, targetTimeSecond int64) (int, bool) { return slices.BinarySearchFunc(cacheKeys, targetTimeSecond, func(a Key, b int64) int { switch { diff --git a/core/itg/cache/cache_test.go b/core/itg/cache/cache_test.go new file mode 100644 index 00000000..bd212a71 --- /dev/null +++ b/core/itg/cache/cache_test.go @@ -0,0 +1,84 @@ +// Copyright (c) 2025 Uber Technologies, Inc. +// +// 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. + +package cache + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/uber/tango/core/itg/graph" + "github.com/uber/tango/core/storage" +) + +func TestStorageCache_FloorKey(t *testing.T) { + cache := NewStorageCache(storage.NewMemoryStorage()) + want := Key{ + Remote: "uber/tango", + BaseCommitTimeSecond: 1_700_000_000, + BaseSha: "abc123", + } + require.NoError(t, cache.Put(t.Context(), &graph.OptimizedGraph{}, want)) + + got, err := cache.FloorKey(t.Context(), want.Remote, want.BaseCommitTimeSecond) + require.NoError(t, err) + assert.Equal(t, want, got) +} + +func TestStorageCache_RejectsInvalidRemote(t *testing.T) { + cache := NewStorageCache(storage.NewMemoryStorage()) + key := Key{ + Remote: "../repo", + BaseCommitTimeSecond: 1_700_000_000, + BaseSha: "abc123", + } + + err := cache.Put(t.Context(), &graph.OptimizedGraph{}, key) + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid remote") + + _, err = cache.Get(t.Context(), key) + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid remote") + + _, err = cache.FloorKey(t.Context(), key.Remote, key.BaseCommitTimeSecond) + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid remote") +} + +func TestStorageCache_RejectsInvalidBaseSha(t *testing.T) { + cache := NewStorageCache(storage.NewMemoryStorage()) + key := Key{ + Remote: "uber/tango", + BaseCommitTimeSecond: 1_700_000_000, + BaseSha: "x/../../../../../target", + } + + err := cache.Put(t.Context(), &graph.OptimizedGraph{}, key) + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid base SHA") + + _, err = cache.Get(t.Context(), key) + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid base SHA") +} + +func TestKey_toStorageKey_preservesValidValue(t *testing.T) { + key := Key{Remote: "uber/tango", BaseCommitTimeSecond: 1_700_000_000, BaseSha: "abc123"} + + got, err := key.toStorageKey() + require.NoError(t, err) + assert.Equal(t, "itg/uber/tango/2023-11-14/1700000000_abc123", got) +} diff --git a/core/storage/disk/disk.go b/core/storage/disk/disk.go index 08766a69..d3f7638e 100644 --- a/core/storage/disk/disk.go +++ b/core/storage/disk/disk.go @@ -17,11 +17,16 @@ package disk import ( "context" + "crypto/rand" + "encoding/hex" "errors" + "fmt" "io" + "io/fs" "os" - "path/filepath" + "path" "strings" + "unicode/utf8" "github.com/uber/tango/core/storage" ) @@ -49,8 +54,16 @@ func (d *diskStorage) Get(ctx context.Context, req storage.DownloadRequest) (sto if ctx.Err() != nil { return storage.DownloadResponse{}, ctx.Err() } - path := filepath.Join(d.rootDir, req.Key) - file, err := os.Open(path) + if err := validateKey(req.Key); err != nil { + return storage.DownloadResponse{}, err + } + root, err := os.OpenRoot(d.rootDir) + if err != nil { + return storage.DownloadResponse{}, err + } + defer root.Close() + + file, err := root.Open(req.Key) if err != nil { if os.IsNotExist(err) { return storage.DownloadResponse{}, &storage.NotFoundError{Path: req.Key} @@ -68,19 +81,27 @@ func (d *diskStorage) Put(ctx context.Context, req storage.UploadRequest) error if req.Reader == nil { return errors.New("nil reader") } - - path := filepath.Join(d.rootDir, req.Key) - if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + if err := validateKey(req.Key); err != nil { + return err + } + root, err := os.OpenRoot(d.rootDir) + if err != nil { return err } + defer root.Close() - // Write atomically via temp file - tmp, err := os.CreateTemp(filepath.Dir(path), ".tmp-*") + dir := path.Dir(req.Key) + if dir != "." { + if err := root.MkdirAll(dir, 0o755); err != nil { + return err + } + } + + tmp, tmpPath, err := createTemp(root, dir) if err != nil { return err } - tmpPath := tmp.Name() - defer os.Remove(tmpPath) + defer root.Remove(tmpPath) if _, err := io.Copy(tmp, &storage.CtxReader{Ctx: ctx, R: req.Reader}); err != nil { tmp.Close() @@ -89,7 +110,35 @@ func (d *diskStorage) Put(ctx context.Context, req storage.UploadRequest) error if err := tmp.Close(); err != nil { return err } - return os.Rename(tmpPath, path) + return root.Rename(tmpPath, req.Key) +} + +// os.CreateTemp accepts a filesystem path rather than an os.Root. Using it here +// would require reconstructing an unrestricted path from the storage root and +// key directory, allowing a symlink in that path to resolve outside the storage +// root. Creating the file through os.Root preserves root-confined path resolution +// for the temporary write as well as the final rename. +func createTemp(root *os.Root, dir string) (*os.File, string, error) { + var random [32]byte + if _, err := rand.Read(random[:]); err != nil { + return nil, "", err + } + name := ".tmp-" + hex.EncodeToString(random[:]) + if dir != "." { + name = path.Join(dir, name) + } + file, err := root.OpenFile(name, os.O_RDWR|os.O_CREATE|os.O_EXCL, 0o600) + if err != nil { + return nil, "", err + } + return file, name, nil +} + +func validateKey(key string) error { + if key == "" || key == "." || !fs.ValidPath(key) || strings.ContainsAny(key, "\\:\x00") { + return fmt.Errorf("invalid disk storage key %q", key) + } + return nil } // Exists checks whether a blob exists in the storage. @@ -97,7 +146,16 @@ func (d *diskStorage) Exists(ctx context.Context, key string) (bool, error) { if ctx.Err() != nil { return false, ctx.Err() } - _, err := os.Stat(filepath.Join(d.rootDir, key)) + if err := validateKey(key); err != nil { + return false, err + } + root, err := os.OpenRoot(d.rootDir) + if err != nil { + return false, err + } + defer root.Close() + + _, err = root.Stat(key) if err == nil { return true, nil } @@ -116,31 +174,55 @@ func (d *diskStorage) List(ctx context.Context, prefix string) ([]string, error) if ctx.Err() != nil { return nil, ctx.Err() } - walkSubdir := "" + if err := validatePrefix(prefix); err != nil { + return nil, err + } + root, err := os.OpenRoot(d.rootDir) + if err != nil { + return nil, err + } + defer root.Close() + + walkSubdir := "." if idx := strings.LastIndex(prefix, "/"); idx >= 0 { - walkSubdir = prefix[:idx+1] + walkSubdir = prefix[:idx] } - walkRoot := filepath.Join(d.rootDir, walkSubdir) var keys []string - err := filepath.Walk(walkRoot, func(path string, info os.FileInfo, err error) error { + err = fs.WalkDir(root.FS(), walkSubdir, func(key string, entry fs.DirEntry, err error) error { if err != nil { - if os.IsNotExist(err) { + if errors.Is(err, fs.ErrNotExist) { return nil } return err } - if info.IsDir() { - return nil - } - rel, err := filepath.Rel(d.rootDir, path) - if err != nil { + if err := ctx.Err(); err != nil { return err } - if !strings.HasPrefix(rel, prefix) { + if entry.IsDir() { + return nil + } + if !strings.HasPrefix(key, prefix) { return nil } - keys = append(keys, rel) + keys = append(keys, key) return nil }) return keys, err } + +func validatePrefix(prefix string) error { + if prefix == "" { + return nil + } + if !utf8.ValidString(prefix) || strings.HasPrefix(prefix, "/") || strings.ContainsAny(prefix, "\\:\x00") { + return fmt.Errorf("invalid disk storage prefix %q", prefix) + } + idx := strings.LastIndex(prefix, "/") + if idx < 0 { + return nil + } + if err := validateKey(prefix[:idx]); err != nil { + return fmt.Errorf("invalid disk storage prefix %q: %w", prefix, err) + } + return nil +} diff --git a/core/storage/disk/disk_test.go b/core/storage/disk/disk_test.go index ecae9a7b..2424e9cd 100644 --- a/core/storage/disk/disk_test.go +++ b/core/storage/disk/disk_test.go @@ -214,3 +214,82 @@ func TestStorage_Get_NotFound(t *testing.T) { assert.Error(t, err) assert.True(t, storage.IsNotFound(err)) } + +func TestStorage_RejectsUnsafeKeys(t *testing.T) { + ctx := t.Context() + parentDir := t.TempDir() + rootDir := filepath.Join(parentDir, "storage") + s, err := New(rootDir) + require.NoError(t, err) + + outsidePath := filepath.Join(parentDir, "outside") + unsafeKeys := []string{ + "../outside", + "dir/../outside", + "/absolute", + "dir/./key", + "dir//key", + "dir/", + ".", + "..", + "", + `dir\key`, + "C:/key", + "dir/\x00key", + } + for _, key := range unsafeKeys { + t.Run(key, func(t *testing.T) { + err := s.Put(ctx, storage.UploadRequest{Key: key, Reader: bytes.NewReader([]byte("secret"))}) + require.Error(t, err) + + resp, err := s.Get(ctx, storage.DownloadRequest{Key: key}) + require.Error(t, err) + assert.Nil(t, resp.ReadCloser) + + exists, err := s.Exists(ctx, key) + require.Error(t, err) + assert.False(t, exists) + }) + } + + unsafePrefixes := []string{"../", "dir/../", "/", "dir/./", "dir//", `dir\`, "C:/", "dir/\x00"} + for _, prefix := range unsafePrefixes { + keys, err := s.List(ctx, prefix) + require.Error(t, err, prefix) + assert.Nil(t, keys) + } + + _, statErr := os.Stat(outsidePath) + assert.ErrorIs(t, statErr, os.ErrNotExist) +} + +func TestValidatePrefix_AllowsLiteralDotPrefixes(t *testing.T) { + require.NoError(t, validatePrefix(".")) + require.NoError(t, validatePrefix("..")) + require.NoError(t, validatePrefix("dir/.")) +} + +func TestStorage_RejectsEscapingSymlink(t *testing.T) { + ctx := t.Context() + parentDir := t.TempDir() + rootDir := filepath.Join(parentDir, "storage") + s, err := New(rootDir) + require.NoError(t, err) + + outsideDir := filepath.Join(parentDir, "outside") + require.NoError(t, os.Mkdir(outsideDir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(outsideDir, "secret"), []byte("secret"), 0o600)) + require.NoError(t, os.Symlink(outsideDir, filepath.Join(rootDir, "link"))) + + resp, err := s.Get(ctx, storage.DownloadRequest{Key: "link/secret"}) + require.Error(t, err) + assert.Nil(t, resp.ReadCloser) + + err = s.Put(ctx, storage.UploadRequest{ + Key: "link/new", + Reader: bytes.NewReader([]byte("new")), + }) + require.Error(t, err) + _, statErr := os.Stat(filepath.Join(outsideDir, "new")) + assert.ErrorIs(t, statErr, os.ErrNotExist) +}