Fix storage OS bucket not returning ErrNotExist correctly (#3188)

This PR fixes a bug of the storage bucket where it does not return an
`fs.ErrNotExist` error when it should.

It happens when both conditions met:
* The bucket is created with an absolute path, i.e.
`provider.NewReadWriteBucket("/absPath")`
* `bucket.Get(ctx, "existing.file/foo")` is called, where
`existing.file` exists as a regular file.

Fixes #3185

Author: oliversun9

private/buf/bufworkspace/workspace_targeting.go

@@ -625,14 +625,6 @@ func checkForControllingWorkspaceOrV1Module(
 	ignoreWorkspaceCheck bool,
 ) (buftarget.ControllingWorkspace, error) {
 	path = normalpath.Normalize(path)
-	// We attempt to check that the provided target path is not a file by checking the extension.
-	// Any valid proto file provided as a target would have the .proto extension, so we treat
-	// any path given without as a directory.
-	// This could be a file without an extension, in which case an error would be returned
-	// to the user when we attempt to check for a controlling workspace.
-	if normalpath.Ext(path) != "" {
-		path = normalpath.Dir(path)
-	}
 	// Keep track of any v1 module found along the way. If we find a v1 or v2 workspace, we
 	// return that over the v1 module, but we return this as the fallback.
 	var fallbackV1Module buftarget.ControllingWorkspace

private/pkg/storage/storageos/bucket.go

@@ -20,7 +20,6 @@ import (
 	"io/fs"
 	"os"
 	"path/filepath"
-	"strings"
 
 	"github.com/bufbuild/buf/private/pkg/filepathext"
 	"github.com/bufbuild/buf/private/pkg/normalpath"
@@ -284,14 +283,10 @@ func (b *bucket) validateExternalPath(path string, externalPath string) error {
 		// It's important that we detect these cases so that
 		// multi buckets don't unnecessarily fail when one of
 		// its delegates actually defines the path.
-		elements := strings.Split(normalpath.Normalize(externalPath), "/")
-		if len(elements) == 1 {
-			// The path is a single element, so there aren't
-			// any other files to check.
-			return err
-		}
-		for i := len(elements) - 1; i >= 0; i-- {
-			parentFileInfo, err := os.Stat(filepath.Join(elements[:i]...))
+		lastParentPath := externalPath
+		parentPath := filepath.Dir(externalPath)
+		for ; parentPath != lastParentPath; lastParentPath, parentPath = parentPath, filepath.Dir(parentPath) {
+			parentFileInfo, err := os.Stat(parentPath)
 			if err != nil {
 				continue
 			}

private/pkg/storage/storageos/storageos_test.go

@@ -15,6 +15,9 @@
 package storageos_test
 
 import (
+	"context"
+	"io/fs"
+	"os"
 	"path/filepath"
 	"testing"
 
@@ -37,6 +40,72 @@ func TestOS(t *testing.T) {
 		testWriteBucketToReadBucket,
 		true,
 	)
+
+	t.Run("get_non_existent_file", func(t *testing.T) {
+		t.Parallel()
+		ctx := context.Background()
+		// Create a bucket at an absolute path.
+		tempDir := t.TempDir()
+		tempDir, err := filepath.Abs(tempDir)
+		require.NoError(t, err)
+		bucket, err := storageos.NewProvider().NewReadWriteBucket(tempDir)
+		require.NoError(t, err)
+
+		// Write a file to it.
+		writeObjectCloser, err := bucket.Put(ctx, "foo.txt")
+		require.NoError(t, err)
+		written, err := writeObjectCloser.Write([]byte(nil))
+		require.NoError(t, err)
+		require.Zero(t, written)
+		require.NoError(t, writeObjectCloser.Close())
+
+		// Try reading a file as if foo.txt is a directory.
+		_, err = bucket.Get(ctx, "foo.txt/bar.txt")
+		require.ErrorIs(t, err, fs.ErrNotExist)
+		_, err = bucket.Get(ctx, "foo.txt/bar.txt/baz.txt")
+		require.ErrorIs(t, err, fs.ErrNotExist)
+
+		// Read a file that does not exist at all.
+		_, err = bucket.Get(ctx, "baz.txt")
+		require.ErrorIs(t, err, fs.ErrNotExist)
+	})
+
+	t.Run("get_non_existent_file_symlink", func(t *testing.T) {
+		t.Parallel()
+		ctx := context.Background()
+		// Create a bucket at an absolute path.
+		actualTempDir := t.TempDir()
+		actualTempDir, err := filepath.Abs(actualTempDir)
+		require.NoError(t, err)
+		_, err = os.Create(filepath.Join(actualTempDir, "foo.txt"))
+		require.NoError(t, err)
+		tempDir := t.TempDir()
+		tempDir, err = filepath.Abs(tempDir)
+		require.NoError(t, err)
+		tempDir = filepath.Join(tempDir, "sym")
+		require.NoError(t, os.Symlink(actualTempDir, tempDir))
+		t.Cleanup(func() {
+			if err := os.Remove(tempDir); err != nil {
+				t.Error(err)
+			}
+		})
+		provider := storageos.NewProvider(storageos.ProviderWithSymlinks())
+		bucket, err := provider.NewReadWriteBucket(tempDir, storageos.ReadWriteBucketWithSymlinksIfSupported())
+		require.NoError(t, err)
+
+		_, err = bucket.Get(ctx, "foo.txt")
+		require.NoError(t, err)
+
+		// Try reading a file as if foo.txt is a directory.
+		_, err = bucket.Get(ctx, "foo.txt/bar.txt")
+		require.ErrorIs(t, err, fs.ErrNotExist)
+		_, err = bucket.Get(ctx, "foo.txt/bar.txt/baz.txt")
+		require.ErrorIs(t, err, fs.ErrNotExist)
+
+		// Read a file that does not exist at all.
+		_, err = bucket.Get(ctx, "baz.txt")
+		require.ErrorIs(t, err, fs.ErrNotExist)
+	})
 }
 
 func testNewReadBucket(t *testing.T, dirPath string, storageosProvider storageos.Provider) (storage.ReadBucket, storagetesting.GetExternalPathFunc) {