From a422ce296936fc78a5311039060bece570eec614 Mon Sep 17 00:00:00 2001 From: Anisa Oshafi Date: Wed, 8 Jul 2026 19:00:09 +0200 Subject: [PATCH] fix: reject directory paths early in snapshot load --- internal/snapshot/destination.go | 22 +++++++++++++++---- internal/snapshot/destination_test.go | 30 +++++++++++++++++--------- test/integration/snapshot_load_test.go | 19 ++++++++++++++++ 3 files changed, 57 insertions(+), 14 deletions(-) diff --git a/internal/snapshot/destination.go b/internal/snapshot/destination.go index 55a4ae45..ab65864e 100644 --- a/internal/snapshot/destination.go +++ b/internal/snapshot/destination.go @@ -205,15 +205,29 @@ func ParseSource(ref, home string) (Destination, error) { return Destination{Kind: KindLocal, Value: resolved}, nil } -// resolveSourcePath returns the first existing path among: abs as-is, then -// abs+".snapshot", then abs+".zip" (legacy). +// resolveSourcePath returns the first existing file among: abs as-is, then +// abs+".snapshot", then abs+".zip" (legacy). A directory match is remembered but +// skipped in favor of a later file match, so a directory error only surfaces when +// no candidate resolves to an actual file. func resolveSourcePath(abs string) (string, error) { withSnapshot := abs + snapshotExt withZip := abs + legacySnapshotExt + var dirHit string for _, candidate := range []string{abs, withSnapshot, withZip} { - if _, err := os.Stat(candidate); err == nil { - return candidate, nil + info, err := os.Stat(candidate) + if err != nil { + continue } + if info.IsDir() { + if dirHit == "" { + dirHit = candidate + } + continue + } + return candidate, nil + } + if dirHit != "" { + return "", fmt.Errorf("%q is a directory, expected a snapshot file", dirHit) } return "", fmt.Errorf("snapshot file not found: %q (also tried %q and %q)", abs, withSnapshot, withZip) } diff --git a/internal/snapshot/destination_test.go b/internal/snapshot/destination_test.go index bd563fe0..53f9c2dd 100644 --- a/internal/snapshot/destination_test.go +++ b/internal/snapshot/destination_test.go @@ -47,8 +47,6 @@ func TestParseShowable(t *testing.T) { func TestParseSource(t *testing.T) { t.Parallel() - wd, err := os.Getwd() - require.NoError(t, err) // Use a temp dir as home so the test doesn't depend on the real $HOME // (e.g. under Nix's sandboxed build, $HOME is a non-existent placeholder). home := t.TempDir() @@ -64,6 +62,10 @@ func TestParseSource(t *testing.T) { require.NoError(t, os.WriteFile(existingLegacyBare+".zip", []byte("data"), 0600)) existingNoExt := filepath.Join(dir, "noext") // no extension, no fallback counterpart either require.NoError(t, os.WriteFile(existingNoExt, []byte("data"), 0600)) + plainDir := filepath.Join(dir, "some-directory") + require.NoError(t, os.Mkdir(plainDir, 0o755)) + snapshotExtDir := filepath.Join(dir, "looks-like-a-snapshot.snapshot") + require.NoError(t, os.Mkdir(snapshotExtDir, 0o755)) type testCase struct { name string @@ -126,18 +128,26 @@ func TestParseSource(t *testing.T) { wantErr: "snapshot file not found", }, { - name: "relative path resolved via cwd", - input: ".", - wantKind: snapshot.KindLocal, - wantPath: wd, + name: "directory instead of file returns clear error", + input: plainDir, + wantErr: "is a directory", + }, + { + name: "directory with .snapshot extension returns clear error", + input: snapshotExtDir, + wantErr: "is a directory", + }, + { + name: "relative path resolved via cwd is a directory", + input: ".", + wantErr: "is a directory", }, // --- tilde expansion --- { - name: "tilde expands to home", - input: "~/.", - wantKind: snapshot.KindLocal, - wantPath: home, + name: "tilde expands to home which is a directory", + input: "~/.", + wantErr: "is a directory", }, // --- pod sources --- diff --git a/test/integration/snapshot_load_test.go b/test/integration/snapshot_load_test.go index 820c3106..70d47874 100644 --- a/test/integration/snapshot_load_test.go +++ b/test/integration/snapshot_load_test.go @@ -175,6 +175,25 @@ func TestSnapshotLoadFileNotFound(t *testing.T) { assert.Contains(t, stderr, "snapshot file not found") } +// TestSnapshotLoadPathIsDirectory ensures passing a directory instead of a +// snapshot file fails early with a clear message, instead of surfacing a +// confusing HTTP-layer error once the directory reaches the upload code path. +func TestSnapshotLoadPathIsDirectory(t *testing.T) { + t.Parallel() + ctx := testContext(t) + + dir := t.TempDir() + target := filepath.Join(dir, "some-directory") + require.NoError(t, os.Mkdir(target, 0o755)) + + _, stderr, err := runLstk(t, ctx, t.TempDir(), + testEnvWithHome(t.TempDir(), ""), + "--non-interactive", "snapshot", "load", target, + ) + requireExitCode(t, 1, err) + assert.Contains(t, stderr, "is a directory") +} + // --- Docker required --- func TestSnapshotLoadLocalSuccess(t *testing.T) {