diff --git a/cmd/share_link_download.go b/cmd/share_link_download.go index 69f0eff..522d1b6 100644 --- a/cmd/share_link_download.go +++ b/cmd/share_link_download.go @@ -106,7 +106,7 @@ func shareLinkDownload(cmd *cobra.Command, args []string) error { if err != nil { return withJSONErrorDetails(err, urlErrorDetails(url), operationErrorDetails("share_link_download")) } - if err := downloadSharedLinkFolder(filesNewFunc(config), dbx, arg, folder.Name, dst, cmd.ErrOrStderr()); err != nil { + if err := downloadSharedLinkFolder(filesNewFunc(config), dbx, arg, dst, cmd.ErrOrStderr()); err != nil { return withJSONErrorDetails(err, urlErrorDetails(url), operationErrorDetails("share_link_download")) } commandVerboseStatus(cmd, "Downloaded shared link folder to %s", dst) @@ -209,7 +209,7 @@ func sharedLinkFolderDownloadTarget(target string, link *sharing.FolderLinkMetad return target, nil } -func downloadSharedLinkFolder(filesDbx filesClient, dbx sharedLinkClient, arg *sharing.GetSharedLinkMetadataArg, rootName, dst string, errOut io.Writer) error { +func downloadSharedLinkFolder(filesDbx filesClient, dbx sharedLinkClient, arg *sharing.GetSharedLinkMetadataArg, dst string, errOut io.Writer) error { if errOut == nil { errOut = io.Discard } @@ -238,14 +238,11 @@ func downloadSharedLinkFolder(filesDbx filesClient, dbx sharedLinkClient, arg *s for _, entry := range entries { switch f := entry.(type) { case *files.FolderMetadata: - relPath, err := sharedLinkEntryRelativePath(f.PathDisplay, rootName) + relPath, err := sharedLinkEntryRelativePath(relFolder, f.Name) if err != nil { downloadErrors = append(downloadErrors, err) continue } - if relPath == "" { - continue - } localDir, err := sharedLinkLocalPath(dst, relPath) if err != nil { downloadErrors = append(downloadErrors, err) @@ -257,7 +254,7 @@ func downloadSharedLinkFolder(filesDbx filesClient, dbx sharedLinkClient, arg *s } queue = append(queue, relPath) case *files.FileMetadata: - relPath, err := sharedLinkEntryRelativePath(f.PathDisplay, rootName) + relPath, err := sharedLinkEntryRelativePath(relFolder, f.Name) if err != nil { downloadErrors = append(downloadErrors, err) continue @@ -334,29 +331,13 @@ func downloadSharedLinkRelativeFile(dbx sharedLinkClient, baseArg *sharing.GetSh }) } -func sharedLinkEntryRelativePath(pathDisplay string, rootName string) (string, error) { - rel := strings.TrimPrefix(pathDisplay, "/") - if rootName != "" { - rootName = strings.Trim(rootName, "/") - first, rest, ok := strings.Cut(rel, "/") - if strings.EqualFold(first, rootName) { - if !ok { - return "", nil - } - rel = rest - } - } - parts := strings.Split(rel, "/") - for _, part := range parts { - if part == "" || part == "." || part == ".." { - return "", fmt.Errorf("invalid shared link entry path %q", pathDisplay) - } - } - rel = path.Clean(rel) - if rel == "" || rel == "." || rel == ".." || strings.HasPrefix(rel, "../") { - return "", fmt.Errorf("invalid shared link entry path %q", pathDisplay) +// Shared-link listings may omit path_display and path_lower when the folder is +// not mounted in the caller's Dropbox. Names are relative to the folder listed. +func sharedLinkEntryRelativePath(parentRel, name string) (string, error) { + if name == "." || !filepath.IsLocal(name) || strings.ContainsAny(name, `/\`) { + return "", fmt.Errorf("invalid shared link entry name %q", name) } - return rel, nil + return path.Join(parentRel, name), nil } func sharedLinkAPIPath(rel string) string { diff --git a/cmd/share_link_download_test.go b/cmd/share_link_download_test.go index 7836c8e..cfd3fb9 100644 --- a/cmd/share_link_download_test.go +++ b/cmd/share_link_download_test.go @@ -462,86 +462,98 @@ func TestShareLinkDownloadFolderRejectsStdoutTarget(t *testing.T) { } func TestShareLinkDownloadFolderRecursiveDownloadsNestedFiles(t *testing.T) { - tmp := t.TempDir() - target := filepath.Join(tmp, "out") - var listed []string - var listedPassword string - var downloaded []string + for _, mounted := range []bool{false, true} { + t.Run(fmt.Sprintf("mounted=%t", mounted), func(t *testing.T) { + metadata := func(name, relativePath string) files.Metadata { + m := files.Metadata{Name: name} + if mounted { + m.PathDisplay = "/parent/docs/" + relativePath + m.PathLower = strings.ToLower(m.PathDisplay) + } + return m + } + tmp := t.TempDir() + target := filepath.Join(tmp, "out") + var listed []string + var listedPassword string + var downloaded []string - stubSharedLinkClient(t, &mockSharedLinkClient{ - getSharedLinkMetadataFn: func(arg *sharing.GetSharedLinkMetadataArg) (sharing.IsSharedLinkMetadata, error) { - if arg.LinkPassword != "secret" { - t.Fatalf("metadata password = %q, want secret", arg.LinkPassword) + stubSharedLinkClient(t, &mockSharedLinkClient{ + getSharedLinkMetadataFn: func(arg *sharing.GetSharedLinkMetadataArg) (sharing.IsSharedLinkMetadata, error) { + if arg.LinkPassword != "secret" { + t.Fatalf("metadata password = %q, want secret", arg.LinkPassword) + } + return sharedLinkFolder("/docs", "https://example.com/folder"), nil + }, + getSharedLinkFileFn: func(arg *sharing.GetSharedLinkMetadataArg) (sharing.IsSharedLinkMetadata, io.ReadCloser, error) { + downloaded = append(downloaded, arg.Path) + if arg.LinkPassword != "secret" { + t.Fatalf("download password = %q, want secret", arg.LinkPassword) + } + contents := strings.TrimPrefix(arg.Path, "/") + return downloadableSharedLinkFile(filepath.Base(arg.Path), arg.Path, "https://example.com/folder", uint64(len(contents))), + io.NopCloser(strings.NewReader(contents)), nil + }, + }) + stubFilesClient(t, &mockFilesClient{ + listFolderFn: func(arg *files.ListFolderArg) (*files.ListFolderResult, error) { + listed = append(listed, arg.Path) + if arg.SharedLink == nil { + t.Fatal("SharedLink = nil, want shared-link listing") + } + if arg.SharedLink.Url != "https://example.com/folder" { + t.Fatalf("shared link URL = %q, want https://example.com/folder", arg.SharedLink.Url) + } + listedPassword = arg.SharedLink.Password + if arg.Recursive { + t.Fatal("Recursive = true, want manual recursion for shared links") + } + switch arg.Path { + case "": + return &files.ListFolderResult{ + Entries: []files.IsMetadata{ + &files.FileMetadata{Metadata: metadata("root.txt", "root.txt"), Size: 8}, + &files.FolderMetadata{Metadata: metadata("sub", "sub")}, + }, + }, nil + case "/sub": + return &files.ListFolderResult{ + Entries: []files.IsMetadata{ + &files.FileMetadata{Metadata: metadata("deep.txt", "sub/deep.txt"), Size: 12}, + }, + }, nil + default: + t.Fatalf("unexpected list path %q", arg.Path) + } + return nil, nil + }, + }) + + cmd := newShareLinkDownloadTestCommand(nil, nil) + if err := cmd.Flags().Set("recursive", "true"); err != nil { + t.Fatalf("set recursive: %v", err) } - return sharedLinkFolder("/docs", "https://example.com/folder"), nil - }, - getSharedLinkFileFn: func(arg *sharing.GetSharedLinkMetadataArg) (sharing.IsSharedLinkMetadata, io.ReadCloser, error) { - downloaded = append(downloaded, arg.Path) - if arg.LinkPassword != "secret" { - t.Fatalf("download password = %q, want secret", arg.LinkPassword) + if err := cmd.Flags().Set("password", "secret"); err != nil { + t.Fatalf("set password: %v", err) } - contents := strings.TrimPrefix(arg.Path, "/") - return downloadableSharedLinkFile(filepath.Base(arg.Path), arg.Path, "https://example.com/folder", uint64(len(contents))), - io.NopCloser(strings.NewReader(contents)), nil - }, - }) - stubFilesClient(t, &mockFilesClient{ - listFolderFn: func(arg *files.ListFolderArg) (*files.ListFolderResult, error) { - listed = append(listed, arg.Path) - if arg.SharedLink == nil { - t.Fatal("SharedLink = nil, want shared-link listing") + + if err := shareLinkDownload(cmd, []string{"https://example.com/folder", target}); err != nil { + t.Fatalf("shareLinkDownload error: %v", err) } - if arg.SharedLink.Url != "https://example.com/folder" { - t.Fatalf("shared link URL = %q, want https://example.com/folder", arg.SharedLink.Url) + + if strings.Join(listed, ",") != ",/sub" { + t.Fatalf("listed paths = %q, want root then sub", strings.Join(listed, ",")) } - listedPassword = arg.SharedLink.Password - if arg.Recursive { - t.Fatal("Recursive = true, want manual recursion for shared links") + if listedPassword != "secret" { + t.Fatalf("listed password = %q, want secret", listedPassword) } - switch arg.Path { - case "": - return &files.ListFolderResult{ - Entries: []files.IsMetadata{ - &files.FileMetadata{Metadata: files.Metadata{PathDisplay: "/docs/root.txt"}, Size: 8}, - &files.FolderMetadata{Metadata: files.Metadata{PathDisplay: "/docs/sub"}}, - }, - }, nil - case "/sub": - return &files.ListFolderResult{ - Entries: []files.IsMetadata{ - &files.FileMetadata{Metadata: files.Metadata{PathDisplay: "/docs/sub/deep.txt"}, Size: 12}, - }, - }, nil - default: - t.Fatalf("unexpected list path %q", arg.Path) + if strings.Join(downloaded, ",") != "/root.txt,/sub/deep.txt" { + t.Fatalf("downloaded paths = %q, want root and nested file", strings.Join(downloaded, ",")) } - return nil, nil - }, - }) - - cmd := newShareLinkDownloadTestCommand(nil, nil) - if err := cmd.Flags().Set("recursive", "true"); err != nil { - t.Fatalf("set recursive: %v", err) - } - if err := cmd.Flags().Set("password", "secret"); err != nil { - t.Fatalf("set password: %v", err) - } - - if err := shareLinkDownload(cmd, []string{"https://example.com/folder", target}); err != nil { - t.Fatalf("shareLinkDownload error: %v", err) - } - - if strings.Join(listed, ",") != ",/sub" { - t.Fatalf("listed paths = %q, want root then sub", strings.Join(listed, ",")) - } - if listedPassword != "secret" { - t.Fatalf("listed password = %q, want secret", listedPassword) - } - if strings.Join(downloaded, ",") != "/root.txt,/sub/deep.txt" { - t.Fatalf("downloaded paths = %q, want root and nested file", strings.Join(downloaded, ",")) + assertFileContent(t, filepath.Join(target, "root.txt"), "root.txt") + assertFileContent(t, filepath.Join(target, "sub", "deep.txt"), "sub/deep.txt") + }) } - assertFileContent(t, filepath.Join(target, "root.txt"), "root.txt") - assertFileContent(t, filepath.Join(target, "sub", "deep.txt"), "sub/deep.txt") } func TestShareLinkDownloadFolderUsesExistingTargetDirectory(t *testing.T) { @@ -564,7 +576,7 @@ func TestShareLinkDownloadFolderUsesExistingTargetDirectory(t *testing.T) { listFolderFn: func(arg *files.ListFolderArg) (*files.ListFolderResult, error) { return &files.ListFolderResult{ Entries: []files.IsMetadata{ - &files.FileMetadata{Metadata: files.Metadata{PathDisplay: "/docs/root.txt"}, Size: 4}, + &files.FileMetadata{Metadata: files.Metadata{Name: "root.txt"}, Size: 4}, }, }, nil }, @@ -599,7 +611,7 @@ func TestShareLinkDownloadFolderFollowsPagination(t *testing.T) { listFolderFn: func(arg *files.ListFolderArg) (*files.ListFolderResult, error) { return &files.ListFolderResult{ Entries: []files.IsMetadata{ - &files.FileMetadata{Metadata: files.Metadata{PathDisplay: "/docs/page-one.txt"}, Size: 4}, + &files.FileMetadata{Metadata: files.Metadata{Name: "page-one.txt"}, Size: 4}, }, HasMore: true, Cursor: "cursor-2", @@ -609,7 +621,7 @@ func TestShareLinkDownloadFolderFollowsPagination(t *testing.T) { continued = arg.Cursor return &files.ListFolderResult{ Entries: []files.IsMetadata{ - &files.FileMetadata{Metadata: files.Metadata{PathDisplay: "/docs/page-two.txt"}, Size: 4}, + &files.FileMetadata{Metadata: files.Metadata{Name: "page-two.txt"}, Size: 4}, }, }, nil }, @@ -661,53 +673,51 @@ func TestShareLinkDownloadFolderDoesNotCreateTargetWhenRootListFails(t *testing. } } -func TestSharedLinkEntryRelativePathStripsRootBySegment(t *testing.T) { - tests := []struct { - name string - pathDisplay string - rootName string - want string - }{ - { - name: "root itself", - pathDisplay: "/docs", - rootName: "docs", - want: "", - }, - { - name: "child", - pathDisplay: "/docs/root.txt", - rootName: "docs", - want: "root.txt", - }, - { - name: "case-insensitive root segment", - pathDisplay: "/DOCS/root.txt", - rootName: "docs", - want: "root.txt", - }, - { - name: "partial prefix is not stripped", - pathDisplay: "/docs-other/root.txt", - rootName: "docs", - want: "docs-other/root.txt", - }, - { - name: "unicode root segment", - pathDisplay: "/\u0130/root.txt", - rootName: "\u0130", - want: "root.txt", - }, +func TestSharedLinkEntryRelativePath(t *testing.T) { + tests := []struct{ parent, name, want string }{ + {"", "root.txt", "root.txt"}, + {"sub/inner", "deep.txt", "sub/inner/deep.txt"}, + {"docs", "docs", "docs/docs"}, + {"", "İ.txt", "İ.txt"}, + {"sub", "a..b", "sub/a..b"}, } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := sharedLinkEntryRelativePath(tt.pathDisplay, tt.rootName) - if err != nil { - t.Fatalf("sharedLinkEntryRelativePath error: %v", err) + t.Run(tt.want, func(t *testing.T) { + got, err := sharedLinkEntryRelativePath(tt.parent, tt.name) + if err != nil || got != tt.want { + t.Fatalf("relative path = %q, %v, want %q", got, err, tt.want) + } + }) + } +} + +func TestShareLinkDownloadFolderRejectsUnsafeEntryNames(t *testing.T) { + for _, name := range []string{"", ".", "..", "../escape.txt", "a/b", `a\b`, `..\escape.txt`, "/absolute", `C:\escape.txt`} { + t.Run(name, func(t *testing.T) { + target := filepath.Join(t.TempDir(), "out") + listed := 0 + filesDbx := &mockFilesClient{listFolderFn: func(arg *files.ListFolderArg) (*files.ListFolderResult, error) { + listed++ + if listed > 1 { + t.Fatal("unsafe folder was queued for listing") + } + return &files.ListFolderResult{Entries: []files.IsMetadata{ + &files.FileMetadata{Metadata: files.Metadata{Name: name}}, + &files.FolderMetadata{Metadata: files.Metadata{Name: name}}, + }}, nil + }} + dbx := &mockSharedLinkClient{getSharedLinkFileFn: func(arg *sharing.GetSharedLinkMetadataArg) (sharing.IsSharedLinkMetadata, io.ReadCloser, error) { + t.Fatal("unsafe file was downloaded") + return nil, nil, nil + }} + var stderr bytes.Buffer + err := downloadSharedLinkFolder(filesDbx, dbx, sharing.NewGetSharedLinkMetadataArg("https://example.com/folder"), target, &stderr) + if err == nil || !strings.Contains(err.Error(), "2 error(s)") { + t.Fatalf("error = %v, want 2 errors", err) } - if got != tt.want { - t.Fatalf("relative path = %q, want %q", got, tt.want) + entries, err := os.ReadDir(target) + if err != nil || len(entries) != 0 { + t.Fatalf("target entries = %v, %v, want empty directory", entries, err) } }) }