diff --git a/shortcuts/drive/drive_pull.go b/shortcuts/drive/drive_pull.go new file mode 100644 index 0000000000..86e3cc7fd4 --- /dev/null +++ b/shortcuts/drive/drive_pull.go @@ -0,0 +1,337 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package drive + +import ( + "context" + "fmt" + "io/fs" + "os" + "path/filepath" + "sort" + "strings" + + larkcore "github.com/larksuite/oapi-sdk-go/v3/core" + + "github.com/larksuite/cli/extension/fileio" + "github.com/larksuite/cli/internal/output" + "github.com/larksuite/cli/internal/validate" + "github.com/larksuite/cli/shortcuts/common" +) + +const ( + drivePullIfExistsOverwrite = "overwrite" + drivePullIfExistsSkip = "skip" +) + +type drivePullItem struct { + RelPath string `json:"rel_path"` + FileToken string `json:"file_token,omitempty"` + Action string `json:"action"` + Error string `json:"error,omitempty"` +} + +// DrivePull performs a one-way file-level mirror from a Drive folder onto +// a local directory: recursively lists --folder-token, downloads each +// type=file entry under --local-dir, and optionally deletes local files +// absent from Drive (--delete-local --yes). +// +// Only Drive entries with type=file participate; online docs (docx, sheet, +// bitable, mindnote, slides) and shortcuts are skipped because there is no +// equivalent local binary to write back. Directories are reproduced when +// remote folders contain downloadable files, but local directories that +// become orphaned after a remote folder is removed are NOT pruned — +// --delete-local only unlinks regular files. +var DrivePull = common.Shortcut{ + Service: "drive", + Command: "+pull", + Description: "One-way file-level mirror of a Drive folder onto a local directory (Drive → local)", + Risk: "write", + Scopes: []string{"drive:drive.metadata:readonly", "drive:file:download"}, + AuthTypes: []string{"user", "bot"}, + Flags: []common.Flag{ + {Name: "local-dir", Desc: "local root directory (relative to cwd)", Required: true}, + {Name: "folder-token", Desc: "source Drive folder token", Required: true}, + {Name: "if-exists", Desc: "policy when a local file already exists", Default: drivePullIfExistsOverwrite, Enum: []string{drivePullIfExistsOverwrite, drivePullIfExistsSkip}}, + {Name: "delete-local", Type: "bool", Desc: "delete local regular files absent from Drive (file-level mirror; empty directories are NOT pruned); requires --yes"}, + {Name: "yes", Type: "bool", Desc: "confirm --delete-local before deleting local files"}, + }, + Tips: []string{ + "Only entries with type=file are downloaded; online docs (docx, sheet, bitable, mindnote, slides) and shortcuts are skipped.", + "Subfolders recurse and are reproduced as local directories under --local-dir; missing parents are created automatically.", + "--delete-local requires --yes; without --yes the command is rejected upfront so a stray flag never deletes anything.", + }, + Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { + localDir := strings.TrimSpace(runtime.Str("local-dir")) + folderToken := strings.TrimSpace(runtime.Str("folder-token")) + if localDir == "" { + return common.FlagErrorf("--local-dir is required") + } + if folderToken == "" { + return common.FlagErrorf("--folder-token is required") + } + if err := validate.ResourceName(folderToken, "--folder-token"); err != nil { + return output.ErrValidation("%s", err) + } + if _, err := validate.SafeLocalFlagPath("--local-dir", localDir); err != nil { + return output.ErrValidation("%s", err) + } + info, err := runtime.FileIO().Stat(localDir) + if err != nil { + return common.WrapInputStatError(err) + } + if !info.IsDir() { + return output.ErrValidation("--local-dir is not a directory: %s", localDir) + } + if runtime.Bool("delete-local") && !runtime.Bool("yes") { + return output.ErrValidation("--delete-local requires --yes (high-risk: deletes local files absent from Drive)") + } + return nil + }, + DryRun: func(ctx context.Context, runtime *common.RuntimeContext) *common.DryRunAPI { + return common.NewDryRunAPI(). + Desc("Recursively list --folder-token, download each type=file entry into --local-dir, and (when --delete-local --yes is set) remove local files absent from Drive."). + GET("/open-apis/drive/v1/files"). + Set("folder_token", runtime.Str("folder-token")) + }, + Execute: func(ctx context.Context, runtime *common.RuntimeContext) error { + localDir := strings.TrimSpace(runtime.Str("local-dir")) + folderToken := strings.TrimSpace(runtime.Str("folder-token")) + ifExists := strings.TrimSpace(runtime.Str("if-exists")) + if ifExists == "" { + ifExists = drivePullIfExistsOverwrite + } + deleteLocal := runtime.Bool("delete-local") + + // Resolve --local-dir to its canonical absolute path before we + // touch the filesystem. SafeInputPath fully evaluates symlinks + // across the entire path; this matters because filepath.Clean + // alone shrinks "link/.." to "." while the kernel resolves it + // through the symlink target's parent — meaning a raw walk on + // the user-supplied string can land outside cwd. Walking the + // canonical root sidesteps that, and using cwd canonical lets + // us emit cwd-relative download targets that FileIO.Save's + // SafeOutputPath check still accepts. The risk is much higher + // here than in +status because --delete-local would otherwise + // remove the wrong files outside cwd. + safeRoot, err := validate.SafeInputPath(localDir) + if err != nil { + return output.ErrValidation("--local-dir: %s", err) + } + cwdCanonical, err := validate.SafeInputPath(".") + if err != nil { + return output.ErrValidation("could not resolve cwd: %s", err) + } + // rootRelToCwd is the localDir form FileIO.Save accepts (it + // rejects absolute paths). For cwd itself it becomes ".", which + // joins cleanly with the rel_paths returned by the lister. + rootRelToCwd, err := filepath.Rel(cwdCanonical, safeRoot) + if err != nil { + return output.ErrValidation("--local-dir resolves outside cwd: %s", err) + } + + fmt.Fprintf(runtime.IO().ErrOut, "Listing Drive folder: %s\n", common.MaskToken(folderToken)) + entries, err := listRemoteFolder(ctx, runtime, folderToken, "") + if err != nil { + return err + } + // Two views over the same listing: + // - remoteFiles drives the download/skip loop (only type=file + // has hashable bytes the local mirror can write back). + // - remotePaths is the --delete-local guard: it carries every + // rel_path Drive owns regardless of type, so a local file + // shadowed by a remote folder / online doc / shortcut is NOT + // treated as orphaned. + remoteFiles := make(map[string]string, len(entries)) + remotePaths := make(map[string]struct{}, len(entries)) + for rel, entry := range entries { + remotePaths[rel] = struct{}{} + if entry.Type == driveTypeFile { + remoteFiles[rel] = entry.FileToken + } + } + + var downloaded, skipped, failed, deletedLocal int + downloadFailed := 0 + items := make([]drivePullItem, 0) + + // Deterministic iteration order for output stability. + downloadablePaths := make([]string, 0, len(remoteFiles)) + for p := range remoteFiles { + downloadablePaths = append(downloadablePaths, p) + } + sort.Strings(downloadablePaths) + + for _, rel := range downloadablePaths { + token := remoteFiles[rel] + target := filepath.Join(rootRelToCwd, rel) + + if info, statErr := runtime.FileIO().Stat(target); statErr == nil { + // Mirror conflict: remote is a regular file but local + // has a directory at the same rel_path. Neither + // "skipped" nor "downloaded" describes reality — + // SafeOutputPath would refuse to write a file over a + // directory, and pretending the directory is a + // pre-existing file under --if-exists=skip silently + // hides the conflict. Surface as a failure. + if info.IsDir() { + items = append(items, drivePullItem{ + RelPath: rel, + FileToken: token, + Action: "failed", + Error: fmt.Sprintf("local path is a directory, remote is a regular file: %s", target), + }) + failed++ + downloadFailed++ + continue + } + if ifExists == drivePullIfExistsSkip { + items = append(items, drivePullItem{RelPath: rel, FileToken: token, Action: "skipped"}) + skipped++ + continue + } + } + + if err := drivePullDownload(ctx, runtime, token, target); err != nil { + items = append(items, drivePullItem{RelPath: rel, FileToken: token, Action: "failed", Error: err.Error()}) + failed++ + downloadFailed++ + continue + } + items = append(items, drivePullItem{RelPath: rel, FileToken: token, Action: "downloaded"}) + downloaded++ + } + + // Gate --delete-local on a clean download pass. With download + // failures still in items[], proceeding to the delete walk would + // leave the mirror in a half-synced state where some files Drive + // owns are missing locally AND some local-only files have been + // removed. Surface the failure first; the operator can re-run + // after fixing whatever caused the download error. + if deleteLocal && downloadFailed == 0 { + // Walk the canonical absolute root, build the list of + // rel_paths, then delete via the absolute path. Both + // values come from the validated safeRoot, so kernel + // path resolution cannot redirect the delete to a file + // outside the canonical subtree. + localAbsPaths, err := drivePullWalkLocal(safeRoot) + if err != nil { + return err + } + for _, absPath := range localAbsPaths { + rel, relErr := filepath.Rel(safeRoot, absPath) + if relErr != nil { + items = append(items, drivePullItem{RelPath: absPath, Action: "delete_failed", Error: relErr.Error()}) + failed++ + continue + } + rel = filepath.ToSlash(rel) + // Consult remotePaths (every Drive entry, regardless of + // type) rather than remoteFiles (downloadable subset + // only). Otherwise an online doc / shortcut at e.g. + // "notes.docx" would leave a same-named local file + // looking orphaned and get unlinked even though Drive + // still knows about that path. + if _, ok := remotePaths[rel]; ok { + continue + } + // FileIO has no Remove(); the absolute path comes from + // walking safeRoot, which validate.SafeInputPath has + // already bounded inside cwd, so a bare os.Remove is + // acceptable here. Shortcuts cannot import internal/vfs + // directly (depguard rule shortcuts-no-vfs). + if err := os.Remove(absPath); err != nil { //nolint:forbidigo // see comment above + items = append(items, drivePullItem{RelPath: rel, Action: "delete_failed", Error: err.Error()}) + failed++ + continue + } + items = append(items, drivePullItem{RelPath: rel, Action: "deleted_local"}) + deletedLocal++ + } + } + + payload := map[string]interface{}{ + "summary": map[string]interface{}{ + "downloaded": downloaded, + "skipped": skipped, + "failed": failed, + "deleted_local": deletedLocal, + }, + "items": items, + } + + // Item-level failures (download error, dir/file conflict, delete + // error) must surface as a non-zero exit so AI / script callers + // don't have to reach into summary.failed to detect a partial + // sync. The same structured payload rides along in error.detail + // so forensics aren't lost. When --delete-local was skipped + // because of an earlier download failure, callers see + // deleted_local=0 plus the download failure that aborted it, + // which is what makes the partial state self-explanatory. + if failed > 0 { + msg := fmt.Sprintf("%d item(s) failed during +pull; partial sync — re-run after resolving the failures", failed) + if deleteLocal && downloadFailed > 0 { + msg += " (--delete-local was skipped because the download pass had failures)" + } + return &output.ExitError{ + Code: output.ExitAPI, + Detail: &output.ErrDetail{ + Type: "partial_failure", + Message: msg, + Detail: payload, + }, + } + } + + runtime.Out(payload, nil) + return nil + }, +} + +func drivePullDownload(ctx context.Context, runtime *common.RuntimeContext, fileToken, target string) error { + resp, err := runtime.DoAPIStream(ctx, &larkcore.ApiReq{ + HttpMethod: "GET", + ApiPath: fmt.Sprintf("/open-apis/drive/v1/files/%s/download", validate.EncodePathSegment(fileToken)), + }) + if err != nil { + return output.ErrNetwork("download %s: %s", common.MaskToken(fileToken), err) + } + defer resp.Body.Close() + if _, err := runtime.FileIO().Save(target, fileio.SaveOptions{ + ContentType: resp.Header.Get("Content-Type"), + ContentLength: resp.ContentLength, + }, resp.Body); err != nil { + return common.WrapSaveErrorByCategory(err, "io") + } + return nil +} + +// drivePullWalkLocal walks the canonical absolute root and returns the +// absolute paths of every regular file underneath it. The caller deletes +// some of these paths, so it is critical that they are produced by +// walking a canonical root (no symlinks in the path) — otherwise OS path +// resolution could redirect a delete to a file outside cwd. Same threat +// model as drive_status.go. +func drivePullWalkLocal(root string) ([]string, error) { + var paths []string + // FileIO has no walker today; shortcuts cannot import internal/vfs + // (depguard rule shortcuts-no-vfs). The root passed in is the + // canonical absolute path returned by validate.SafeInputPath, so + // WalkDir's default "do not follow child symlinks" policy keeps the + // traversal inside the validated subtree. + err := filepath.WalkDir(root, func(absPath string, d fs.DirEntry, walkErr error) error { //nolint:forbidigo // see comment above + if walkErr != nil { + return walkErr + } + if d.IsDir() || !d.Type().IsRegular() { + return nil + } + paths = append(paths, absPath) + return nil + }) + if err != nil { + return nil, output.Errorf(output.ExitInternal, "io", "walk %s: %s", root, err) + } + return paths, nil +} diff --git a/shortcuts/drive/drive_pull_test.go b/shortcuts/drive/drive_pull_test.go new file mode 100644 index 0000000000..9b5d661acc --- /dev/null +++ b/shortcuts/drive/drive_pull_test.go @@ -0,0 +1,1026 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package drive + +import ( + "encoding/json" + "errors" + "net/http" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/larksuite/cli/internal/cmdutil" + "github.com/larksuite/cli/internal/httpmock" + "github.com/larksuite/cli/internal/output" +) + +// TestDrivePullDownloadsAndCreatesParents verifies the happy path: a remote +// folder with a top-level file plus a subfolder is fully reproduced under +// --local-dir, including auto-created parent directories. +func TestDrivePullDownloadsAndCreatesParents(t *testing.T) { + f, stdout, _, reg := cmdutil.TestFactory(t, driveTestConfig()) + + tmpDir := t.TempDir() + withDriveWorkingDir(t, tmpDir) + if err := os.MkdirAll("local", 0o755); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + + // Root folder list — order matters: stubs match in registration order. + reg.Register(&httpmock.Stub{ + Method: "GET", + URL: "folder_token=folder_root", + Body: map[string]interface{}{ + "code": 0, "msg": "ok", + "data": map[string]interface{}{ + "files": []interface{}{ + map[string]interface{}{"token": "tok_a", "name": "a.txt", "type": "file"}, + map[string]interface{}{"token": "tok_sub", "name": "sub", "type": "folder"}, + // noise: an online doc must be skipped + map[string]interface{}{"token": "tok_doc", "name": "ignored.docx", "type": "docx"}, + }, + "has_more": false, + }, + }, + }) + + // Subfolder list + reg.Register(&httpmock.Stub{ + Method: "GET", + URL: "folder_token=tok_sub", + Body: map[string]interface{}{ + "code": 0, "msg": "ok", + "data": map[string]interface{}{ + "files": []interface{}{ + map[string]interface{}{"token": "tok_b", "name": "b.txt", "type": "file"}, + }, + "has_more": false, + }, + }, + }) + + reg.Register(&httpmock.Stub{ + Method: "GET", + URL: "/open-apis/drive/v1/files/tok_a/download", + Status: 200, + Body: []byte("AAA"), + Headers: http.Header{"Content-Type": []string{"application/octet-stream"}}, + }) + reg.Register(&httpmock.Stub{ + Method: "GET", + URL: "/open-apis/drive/v1/files/tok_b/download", + Status: 200, + Body: []byte("BBB"), + Headers: http.Header{"Content-Type": []string{"application/octet-stream"}}, + }) + + err := mountAndRunDrive(t, DrivePull, []string{ + "+pull", + "--local-dir", "local", + "--folder-token", "folder_root", + "--as", "bot", + }, f, stdout) + if err != nil { + t.Fatalf("unexpected error: %v\nstdout: %s", err, stdout.String()) + } + + out := stdout.String() + if !strings.Contains(out, `"downloaded": 2`) { + t.Errorf("expected downloaded=2, got: %s", out) + } + if strings.Contains(out, "ignored.docx") { + t.Errorf("docx entries must be skipped, got: %s", out) + } + + // File contents must reach disk under the right paths. + mustReadFile(t, filepath.Join("local", "a.txt"), "AAA") + mustReadFile(t, filepath.Join("local", "sub", "b.txt"), "BBB") +} + +// TestDrivePullSkipsExistingWhenSkipPolicy verifies --if-exists=skip leaves +// existing local files untouched and counts them under summary.skipped. +func TestDrivePullSkipsExistingWhenSkipPolicy(t *testing.T) { + f, stdout, _, reg := cmdutil.TestFactory(t, driveTestConfig()) + + tmpDir := t.TempDir() + withDriveWorkingDir(t, tmpDir) + if err := os.MkdirAll("local", 0o755); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + if err := os.WriteFile(filepath.Join("local", "keep.txt"), []byte("local-original"), 0o644); err != nil { + t.Fatalf("WriteFile: %v", err) + } + + reg.Register(&httpmock.Stub{ + Method: "GET", + URL: "folder_token=folder_root", + Body: map[string]interface{}{ + "code": 0, "msg": "ok", + "data": map[string]interface{}{ + "files": []interface{}{ + map[string]interface{}{"token": "tok_keep", "name": "keep.txt", "type": "file"}, + }, + "has_more": false, + }, + }, + }) + + err := mountAndRunDrive(t, DrivePull, []string{ + "+pull", + "--local-dir", "local", + "--folder-token", "folder_root", + "--if-exists", "skip", + "--as", "bot", + }, f, stdout) + if err != nil { + t.Fatalf("unexpected error: %v\nstdout: %s", err, stdout.String()) + } + + out := stdout.String() + if !strings.Contains(out, `"skipped": 1`) { + t.Errorf("expected skipped=1, got: %s", out) + } + if !strings.Contains(out, `"downloaded": 0`) { + t.Errorf("expected downloaded=0 with --if-exists=skip, got: %s", out) + } + + // Existing local content must be preserved verbatim. + mustReadFile(t, filepath.Join("local", "keep.txt"), "local-original") +} + +// TestDrivePullSurfacesDirectoryFileMirrorConflict pins the contract +// for the case where Drive ships a regular file at a rel_path that is +// already a directory locally. SafeOutputPath would refuse to overwrite +// the directory at write time, but if --if-exists=skip silently swallows +// the collision the caller sees "skipped" and assumes the mirror is +// in sync. The fix surfaces it as a structured `partial_failure` +// ExitError (non-zero exit + items[] in error.detail) under both skip +// and overwrite policies so callers can react via exit code. +func TestDrivePullSurfacesDirectoryFileMirrorConflict(t *testing.T) { + for _, policy := range []string{"overwrite", "skip"} { + t.Run(policy, func(t *testing.T) { + f, stdout, _, reg := cmdutil.TestFactory(t, driveTestConfig()) + + tmpDir := t.TempDir() + withDriveWorkingDir(t, tmpDir) + // Local has a directory at "shadow" — Drive says it's a + // regular file at the same rel_path. This is the conflict. + if err := os.MkdirAll(filepath.Join("local", "shadow"), 0o755); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + + reg.Register(&httpmock.Stub{ + Method: "GET", + URL: "folder_token=folder_root", + Body: map[string]interface{}{ + "code": 0, "msg": "ok", + "data": map[string]interface{}{ + "files": []interface{}{ + map[string]interface{}{"token": "tok_shadow", "name": "shadow", "type": "file"}, + }, + "has_more": false, + }, + }, + }) + + err := mountAndRunDrive(t, DrivePull, []string{ + "+pull", + "--local-dir", "local", + "--folder-token", "folder_root", + "--if-exists", policy, + "--as", "bot", + }, f, stdout) + detail := assertDrivePullPartialFailure(t, err) + summary, items := splitDrivePullDetail(t, detail) + if got := summary["failed"]; got != float64(1) { + t.Errorf("[%s] summary.failed = %v, want 1", policy, got) + } + if got := summary["skipped"]; got != float64(0) { + t.Errorf("[%s] mirror conflict must NOT be swallowed as skipped (skipped=%v)", policy, got) + } + if len(items) != 1 || items[0]["action"] != "failed" { + t.Errorf("[%s] expected one items[] entry with action=failed, got: %#v", policy, items) + } + if msg, _ := items[0]["error"].(string); !strings.Contains(msg, "is a directory") { + t.Errorf("[%s] error message should mention the directory conflict, got: %q", policy, msg) + } + if stdout.Len() != 0 { + t.Errorf("[%s] stdout should be empty on partial_failure, got: %s", policy, stdout.String()) + } + }) + } +} + +// TestDrivePullPaginationHandlesPageTokenField pins the cross-API +// pagination contract: Drive list paginates by page_token / next_page_token, +// and the shared common.PaginationMeta helper accepts both. If +pull +// only honored next_page_token it would silently stop after the first +// page when the backend returns the (newer) page_token field, so any +// remote files past page one would never be downloaded and would be +// invisible to --delete-local. +func TestDrivePullPaginationHandlesPageTokenField(t *testing.T) { + f, stdout, _, reg := cmdutil.TestFactory(t, driveTestConfig()) + + tmpDir := t.TempDir() + withDriveWorkingDir(t, tmpDir) + if err := os.MkdirAll("local", 0o755); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + + // First page: returns has_more + page_token (NOT next_page_token). + reg.Register(&httpmock.Stub{ + Method: "GET", + URL: "folder_token=folder_root", + Body: map[string]interface{}{ + "code": 0, "msg": "ok", + "data": map[string]interface{}{ + "files": []interface{}{ + map[string]interface{}{"token": "tok_a", "name": "a.txt", "type": "file"}, + }, + "has_more": true, + "page_token": "page2", + }, + }, + }) + // Second page: terminator. + reg.Register(&httpmock.Stub{ + Method: "GET", + URL: "page_token=page2", + Body: map[string]interface{}{ + "code": 0, "msg": "ok", + "data": map[string]interface{}{ + "files": []interface{}{ + map[string]interface{}{"token": "tok_b", "name": "b.txt", "type": "file"}, + }, + "has_more": false, + }, + }, + }) + reg.Register(&httpmock.Stub{ + Method: "GET", + URL: "/open-apis/drive/v1/files/tok_a/download", + Status: 200, + Body: []byte("AAA"), + Headers: http.Header{"Content-Type": []string{"application/octet-stream"}}, + }) + reg.Register(&httpmock.Stub{ + Method: "GET", + URL: "/open-apis/drive/v1/files/tok_b/download", + Status: 200, + Body: []byte("BBB"), + Headers: http.Header{"Content-Type": []string{"application/octet-stream"}}, + }) + + err := mountAndRunDrive(t, DrivePull, []string{ + "+pull", + "--local-dir", "local", + "--folder-token", "folder_root", + "--as", "bot", + }, f, stdout) + if err != nil { + t.Fatalf("unexpected error: %v\nstdout: %s", err, stdout.String()) + } + + out := stdout.String() + if !strings.Contains(out, `"downloaded": 2`) { + t.Errorf("expected both pages to be fetched (downloaded=2), got: %s", out) + } + mustReadFile(t, filepath.Join("local", "a.txt"), "AAA") + mustReadFile(t, filepath.Join("local", "b.txt"), "BBB") + reg.Verify(t) +} + +// TestDrivePullDeleteLocalRequiresYes verifies the upfront safety guard: +// --delete-local without --yes must be rejected before any API call. +func TestDrivePullDeleteLocalRequiresYes(t *testing.T) { + f, _, _, _ := cmdutil.TestFactory(t, driveTestConfig()) + + tmpDir := t.TempDir() + withDriveWorkingDir(t, tmpDir) + if err := os.MkdirAll("local", 0o755); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + + err := mountAndRunDrive(t, DrivePull, []string{ + "+pull", + "--local-dir", "local", + "--folder-token", "folder_root", + "--delete-local", + "--as", "bot", + }, f, nil) + if err == nil { + t.Fatal("expected validation error for --delete-local without --yes, got nil") + } + if !strings.Contains(err.Error(), "--yes") { + t.Fatalf("error must reference --yes, got: %v", err) + } +} + +// TestDrivePullDeletesLocalOnlyFilesWhenYes verifies that --delete-local +// --yes removes local files absent from Drive after downloading the new +// content. +func TestDrivePullDeletesLocalOnlyFilesWhenYes(t *testing.T) { + f, stdout, _, reg := cmdutil.TestFactory(t, driveTestConfig()) + + tmpDir := t.TempDir() + withDriveWorkingDir(t, tmpDir) + if err := os.MkdirAll(filepath.Join("local", "subdir"), 0o755); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + // stale.txt only exists locally → must be deleted. + if err := os.WriteFile(filepath.Join("local", "stale.txt"), []byte("old"), 0o644); err != nil { + t.Fatalf("WriteFile stale: %v", err) + } + // orphan in a subdir → must also be deleted. + if err := os.WriteFile(filepath.Join("local", "subdir", "orphan.txt"), []byte("orphan"), 0o644); err != nil { + t.Fatalf("WriteFile orphan: %v", err) + } + + reg.Register(&httpmock.Stub{ + Method: "GET", + URL: "folder_token=folder_root", + Body: map[string]interface{}{ + "code": 0, "msg": "ok", + "data": map[string]interface{}{ + "files": []interface{}{ + map[string]interface{}{"token": "tok_new", "name": "fresh.txt", "type": "file"}, + }, + "has_more": false, + }, + }, + }) + reg.Register(&httpmock.Stub{ + Method: "GET", + URL: "/open-apis/drive/v1/files/tok_new/download", + Status: 200, + Body: []byte("FRESH"), + Headers: http.Header{"Content-Type": []string{"application/octet-stream"}}, + }) + + err := mountAndRunDrive(t, DrivePull, []string{ + "+pull", + "--local-dir", "local", + "--folder-token", "folder_root", + "--delete-local", + "--yes", + "--as", "bot", + }, f, stdout) + if err != nil { + t.Fatalf("unexpected error: %v\nstdout: %s", err, stdout.String()) + } + + out := stdout.String() + if !strings.Contains(out, `"downloaded": 1`) { + t.Errorf("expected downloaded=1, got: %s", out) + } + if !strings.Contains(out, `"deleted_local": 2`) { + t.Errorf("expected deleted_local=2, got: %s", out) + } + + mustReadFile(t, filepath.Join("local", "fresh.txt"), "FRESH") + if _, err := os.Stat(filepath.Join("local", "stale.txt")); !os.IsNotExist(err) { + t.Errorf("stale.txt should have been removed, stat err=%v", err) + } + if _, err := os.Stat(filepath.Join("local", "subdir", "orphan.txt")); !os.IsNotExist(err) { + t.Errorf("subdir/orphan.txt should have been removed, stat err=%v", err) + } +} + +// TestDrivePullDeleteLocalPreservesLocalFileShadowedByOnlineDoc is the +// regression for the case where Drive holds an online doc (docx, sheet, +// shortcut, …) at the same rel_path as a local file. The online doc is +// NOT in the downloadable set (type≠file) but Drive still owns that path, +// so --delete-local must not treat the local file as orphaned. Before the +// fix, the delete pass consulted only the type=file map and would unlink +// the local file every time it shared a name with an online doc. +func TestDrivePullDeleteLocalPreservesLocalFileShadowedByOnlineDoc(t *testing.T) { + f, stdout, _, reg := cmdutil.TestFactory(t, driveTestConfig()) + + tmpDir := t.TempDir() + withDriveWorkingDir(t, tmpDir) + if err := os.MkdirAll("local", 0o755); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + // User keeps a local copy at the same path Drive serves an online + // doc — should survive --delete-local. + if err := os.WriteFile(filepath.Join("local", "notes.docx"), []byte("LOCAL-DOCX"), 0o644); err != nil { + t.Fatalf("WriteFile shadow: %v", err) + } + + reg.Register(&httpmock.Stub{ + Method: "GET", + URL: "folder_token=folder_root", + Body: map[string]interface{}{ + "code": 0, "msg": "ok", + "data": map[string]interface{}{ + "files": []interface{}{ + map[string]interface{}{"token": "tok_keep", "name": "keep.txt", "type": "file"}, + // Same name as the local file — must be tracked by + // the lister even though it is not downloadable. + map[string]interface{}{"token": "tok_doc", "name": "notes.docx", "type": "docx"}, + }, + "has_more": false, + }, + }, + }) + reg.Register(&httpmock.Stub{ + Method: "GET", + URL: "/open-apis/drive/v1/files/tok_keep/download", + Status: 200, + Body: []byte("KEEP"), + Headers: http.Header{"Content-Type": []string{"application/octet-stream"}}, + }) + + err := mountAndRunDrive(t, DrivePull, []string{ + "+pull", + "--local-dir", "local", + "--folder-token", "folder_root", + "--delete-local", + "--yes", + "--as", "bot", + }, f, stdout) + if err != nil { + t.Fatalf("unexpected error: %v\nstdout: %s", err, stdout.String()) + } + + out := stdout.String() + if !strings.Contains(out, `"deleted_local": 0`) { + t.Errorf("expected deleted_local=0 (online doc shadows local file), got: %s", out) + } + mustReadFile(t, filepath.Join("local", "notes.docx"), "LOCAL-DOCX") + mustReadFile(t, filepath.Join("local", "keep.txt"), "KEEP") +} + +// TestDrivePullDeleteLocalPreservesLocalFileShadowedByRemoteFolder pins +// the same allPaths contract for the folder branch: a remote folder +// occupies its own rel_path (not just the rel_paths of its children), +// so a local regular file at the same name must NOT be treated as +// orphaned. Before the folder-branch fix, listRemote only added the +// folder's children to allPaths, leaving the folder name itself +// missing. +func TestDrivePullDeleteLocalPreservesLocalFileShadowedByRemoteFolder(t *testing.T) { + f, stdout, _, reg := cmdutil.TestFactory(t, driveTestConfig()) + + tmpDir := t.TempDir() + withDriveWorkingDir(t, tmpDir) + if err := os.MkdirAll("local", 0o755); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + // Local has a regular file named "shadow"; Drive has a folder of + // the same name. The local file must survive --delete-local. + if err := os.WriteFile(filepath.Join("local", "shadow"), []byte("LOCAL-FILE"), 0o644); err != nil { + t.Fatalf("WriteFile shadow: %v", err) + } + + // Root folder list: one folder named "shadow", and an unrelated + // downloadable file so we can assert the download path still + // works alongside the protected name. + reg.Register(&httpmock.Stub{ + Method: "GET", + URL: "folder_token=folder_root", + Body: map[string]interface{}{ + "code": 0, "msg": "ok", + "data": map[string]interface{}{ + "files": []interface{}{ + map[string]interface{}{"token": "tok_shadow_dir", "name": "shadow", "type": "folder"}, + map[string]interface{}{"token": "tok_keep", "name": "keep.txt", "type": "file"}, + }, + "has_more": false, + }, + }, + }) + // Subfolder is empty — keeps the folder branch active without + // adding extra files to the assertions. + reg.Register(&httpmock.Stub{ + Method: "GET", + URL: "folder_token=tok_shadow_dir", + Body: map[string]interface{}{ + "code": 0, "msg": "ok", + "data": map[string]interface{}{ + "files": []interface{}{}, + "has_more": false, + }, + }, + }) + reg.Register(&httpmock.Stub{ + Method: "GET", + URL: "/open-apis/drive/v1/files/tok_keep/download", + Status: 200, + Body: []byte("KEEP"), + Headers: http.Header{"Content-Type": []string{"application/octet-stream"}}, + }) + + err := mountAndRunDrive(t, DrivePull, []string{ + "+pull", + "--local-dir", "local", + "--folder-token", "folder_root", + "--delete-local", + "--yes", + "--as", "bot", + }, f, stdout) + if err != nil { + t.Fatalf("unexpected error: %v\nstdout: %s", err, stdout.String()) + } + + out := stdout.String() + if !strings.Contains(out, `"deleted_local": 0`) { + t.Errorf("expected deleted_local=0 (remote folder shadows local file), got: %s", out) + } + mustReadFile(t, filepath.Join("local", "shadow"), "LOCAL-FILE") + mustReadFile(t, filepath.Join("local", "keep.txt"), "KEEP") +} + +// TestDrivePullDeleteLocalCountsFailureInSummary pins the contract that +// a failed delete shows up in summary.failed (not just in items[]) AND +// surfaces as a partial_failure ExitError so callers can detect the +// half-synced state via exit code. Before the fix, the delete_failed +// branches appended an item but left `failed` at zero AND returned nil, +// so the JSON envelope reported `ok=true`+`exit=0` even when the mirror +// was incomplete. Setup forces os.Remove to fail by making the file's +// containing directory read-only (chmod 0o555) right before the run; +// cleanup restores 0o755 so t.TempDir teardown succeeds. +func TestDrivePullDeleteLocalCountsFailureInSummary(t *testing.T) { + f, stdout, _, reg := cmdutil.TestFactory(t, driveTestConfig()) + + tmpDir := t.TempDir() + withDriveWorkingDir(t, tmpDir) + if err := os.MkdirAll("local", 0o755); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + stale := filepath.Join("local", "stale.txt") + if err := os.WriteFile(stale, []byte("old"), 0o644); err != nil { + t.Fatalf("WriteFile stale: %v", err) + } + + reg.Register(&httpmock.Stub{ + Method: "GET", + URL: "folder_token=folder_root", + Body: map[string]interface{}{ + "code": 0, "msg": "ok", + "data": map[string]interface{}{ + "files": []interface{}{}, + "has_more": false, + }, + }, + }) + + // Lock the parent directory so the delete fails. Restore in a + // cleanup so t.TempDir's RemoveAll can succeed. + if err := os.Chmod("local", 0o555); err != nil { + t.Fatalf("Chmod 555: %v", err) + } + t.Cleanup(func() { _ = os.Chmod("local", 0o755) }) + + err := mountAndRunDrive(t, DrivePull, []string{ + "+pull", + "--local-dir", "local", + "--folder-token", "folder_root", + "--delete-local", + "--yes", + "--as", "bot", + }, f, stdout) + detail := assertDrivePullPartialFailure(t, err) + summary, items := splitDrivePullDetail(t, detail) + if got := summary["failed"]; got != float64(1) { + t.Errorf("summary.failed = %v, want 1 (delete_failed must increment failed)", got) + } + if got := summary["deleted_local"]; got != float64(0) { + t.Errorf("summary.deleted_local = %v, want 0", got) + } + if len(items) != 1 || items[0]["action"] != "delete_failed" { + t.Errorf("expected one items[] entry with action=delete_failed, got: %#v", items) + } + if stdout.Len() != 0 { + t.Errorf("stdout should be empty on partial_failure, got: %s", stdout.String()) + } +} + +// TestDrivePullDownloadFailureSkipsDeleteLocalAndExitsNonZero pins the +// gating contract for --delete-local: when the download pass produced +// any failure, the delete walk MUST be skipped entirely and the command +// MUST exit non-zero with type=partial_failure. The half-synced state +// where some Drive files are missing locally AND some local-only files +// have been removed is never observable. +func TestDrivePullDownloadFailureSkipsDeleteLocalAndExitsNonZero(t *testing.T) { + f, stdout, _, reg := cmdutil.TestFactory(t, driveTestConfig()) + + tmpDir := t.TempDir() + withDriveWorkingDir(t, tmpDir) + if err := os.MkdirAll("local", 0o755); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + // stale.txt only exists locally — without the gate, --delete-local + // would unlink it. The gate must prevent that when downloads fail. + stale := filepath.Join("local", "stale.txt") + if err := os.WriteFile(stale, []byte("old"), 0o644); err != nil { + t.Fatalf("WriteFile stale: %v", err) + } + + reg.Register(&httpmock.Stub{ + Method: "GET", + URL: "folder_token=folder_root", + Body: map[string]interface{}{ + "code": 0, "msg": "ok", + "data": map[string]interface{}{ + "files": []interface{}{ + map[string]interface{}{"token": "tok_broken", "name": "broken.bin", "type": "file"}, + }, + "has_more": false, + }, + }, + }) + // Download stub returns 500 so the download fails. + reg.Register(&httpmock.Stub{ + Method: "GET", + URL: "/open-apis/drive/v1/files/tok_broken/download", + Status: 500, + Body: []byte(`{"code":99999,"msg":"backend boom"}`), + Headers: http.Header{"Content-Type": []string{"application/json"}}, + }) + + err := mountAndRunDrive(t, DrivePull, []string{ + "+pull", + "--local-dir", "local", + "--folder-token", "folder_root", + "--delete-local", + "--yes", + "--as", "bot", + }, f, stdout) + exitErr := assertDrivePullPartialFailure(t, err) + if !strings.Contains(exitErr.Detail.Message, "--delete-local was skipped") { + t.Errorf("expected message to mention --delete-local skip, got: %q", exitErr.Detail.Message) + } + + summary, items := splitDrivePullDetail(t, exitErr) + if got := summary["failed"]; got != float64(1) { + t.Errorf("summary.failed = %v, want 1", got) + } + if got := summary["deleted_local"]; got != float64(0) { + t.Errorf("summary.deleted_local = %v, want 0 (delete pass must be skipped on download failure)", got) + } + // The download failure is the only items[] entry — no delete_local / + // delete_failed entries because the delete pass was skipped entirely. + if len(items) != 1 || items[0]["action"] != "failed" { + t.Errorf("expected one items[] entry with action=failed, got: %#v", items) + } + + // stale.txt MUST still exist on disk. + if _, statErr := os.Stat(stale); statErr != nil { + t.Fatalf("stale.txt must survive when --delete-local is skipped after a download failure; stat err=%v", statErr) + } + if stdout.Len() != 0 { + t.Errorf("stdout should be empty on partial_failure, got: %s", stdout.String()) + } +} + +// TestDrivePullDeleteLocalDoesNotEscapeViaSymlinkParentRef is the +// regression for the "link/.." escape applied to --delete-local — the +// most dangerous variant, since the bug would otherwise let the kernel +// walk through the symlink target's parent and delete files outside +// cwd. +// +// Setup: an "escape" sibling directory contains a sentinel file; cwd +// has a "link" symlink pointing into that escape directory. Running +// +pull with --local-dir "link/.." --delete-local --yes against an +// empty remote folder must NOT delete the sentinel. +func TestDrivePullDeleteLocalDoesNotEscapeViaSymlinkParentRef(t *testing.T) { + f, stdout, _, reg := cmdutil.TestFactory(t, driveTestConfig()) + + // Sentinel sits outside cwd; if the bug existed, --delete-local + // would unlink it. + escapeDir := t.TempDir() + sentinel := filepath.Join(escapeDir, "secret.txt") + if err := os.WriteFile(sentinel, []byte("S3CRET"), 0o644); err != nil { + t.Fatalf("WriteFile sentinel: %v", err) + } + + cwdDir := t.TempDir() + withDriveWorkingDir(t, cwdDir) + if err := os.Symlink(escapeDir, filepath.Join(cwdDir, "link")); err != nil { + t.Fatalf("Symlink: %v", err) + } + // One file inside cwd to confirm the walk did run. + cwdLocal := filepath.Join(cwdDir, "ok.txt") + if err := os.WriteFile(cwdLocal, []byte("ok"), 0o644); err != nil { + t.Fatalf("WriteFile cwd: %v", err) + } + + // Remote is empty — so under --delete-local --yes the only files + // the walk identifies as "local-only" are inside the canonical + // walk root. + reg.Register(&httpmock.Stub{ + Method: "GET", + URL: "folder_token=folder_root", + Body: map[string]interface{}{ + "code": 0, "msg": "ok", + "data": map[string]interface{}{ + "files": []interface{}{}, + "has_more": false, + }, + }, + }) + + err := mountAndRunDrive(t, DrivePull, []string{ + "+pull", + "--local-dir", "link/..", + "--folder-token", "folder_root", + "--delete-local", + "--yes", + "--as", "bot", + }, f, stdout) + if err != nil { + t.Fatalf("unexpected error: %v\nstdout: %s", err, stdout.String()) + } + + // Must-haves: + if _, err := os.Stat(sentinel); err != nil { + t.Fatalf("sentinel %q must still exist after +pull --delete-local; stat err=%v", sentinel, err) + } + // And the cwd-local file should have been deleted (it is local-only + // and remote is empty), proving the walk DID run, just not into + // the escape directory. + if _, err := os.Stat(cwdLocal); !os.IsNotExist(err) { + t.Fatalf("ok.txt should have been deleted (local-only with empty remote); stat err=%v", err) + } + out := stdout.String() + if strings.Contains(out, "S3CRET") || strings.Contains(out, escapeDir) { + t.Fatalf("escape directory leaked into output:\n%s", out) + } +} + +// TestDrivePullSkipsSymlinkInsideRoot pins WalkDir's default symlink +// behavior in the +pull --delete-local path. A child symlink under the +// validated root pointing into an out-of-tree directory must NOT be +// followed: WalkDir surfaces it as a non-regular entry, our callback +// skips it, and the sentinel inside the target survives the delete pass. +func TestDrivePullSkipsSymlinkInsideRoot(t *testing.T) { + f, stdout, _, reg := cmdutil.TestFactory(t, driveTestConfig()) + + escapeDir := t.TempDir() + sentinel := filepath.Join(escapeDir, "secret.txt") + if err := os.WriteFile(sentinel, []byte("S3CRET"), 0o644); err != nil { + t.Fatalf("WriteFile secret: %v", err) + } + + cwdDir := t.TempDir() + withDriveWorkingDir(t, cwdDir) + if err := os.MkdirAll(filepath.Join("local", "sub"), 0o755); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + if err := os.WriteFile(filepath.Join("local", "ok.txt"), []byte("ok"), 0o644); err != nil { + t.Fatalf("WriteFile ok: %v", err) + } + if err := os.Symlink(escapeDir, filepath.Join("local", "sub", "escape")); err != nil { + t.Fatalf("Symlink: %v", err) + } + + // Empty remote so --delete-local would target every regular file + // the walker can reach. + reg.Register(&httpmock.Stub{ + Method: "GET", + URL: "folder_token=folder_root", + Body: map[string]interface{}{ + "code": 0, "msg": "ok", + "data": map[string]interface{}{ + "files": []interface{}{}, + "has_more": false, + }, + }, + }) + + err := mountAndRunDrive(t, DrivePull, []string{ + "+pull", + "--local-dir", "local", + "--folder-token", "folder_root", + "--delete-local", + "--yes", + "--as", "bot", + }, f, stdout) + if err != nil { + t.Fatalf("unexpected error: %v\nstdout: %s", err, stdout.String()) + } + + if _, err := os.Stat(sentinel); err != nil { + t.Fatalf("sentinel %q must survive (walker followed child symlink): %v", sentinel, err) + } + if _, err := os.Stat(filepath.Join("local", "ok.txt")); !os.IsNotExist(err) { + t.Fatalf("local/ok.txt should have been deleted (proves walk ran), got: %v", err) + } +} + +// TestDrivePullSurvivesCircularSymlinkInsideRoot ensures the walker +// terminates even when the validated root contains a child symlink +// pointing back at one of its ancestors. +func TestDrivePullSurvivesCircularSymlinkInsideRoot(t *testing.T) { + f, stdout, _, reg := cmdutil.TestFactory(t, driveTestConfig()) + + cwdDir := t.TempDir() + withDriveWorkingDir(t, cwdDir) + if err := os.MkdirAll(filepath.Join("local", "sub"), 0o755); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + if err := os.WriteFile(filepath.Join("local", "sub", "real.txt"), []byte("real"), 0o644); err != nil { + t.Fatalf("WriteFile: %v", err) + } + loopTarget, err := filepath.Abs(filepath.Join("local")) + if err != nil { + t.Fatalf("Abs: %v", err) + } + if err := os.Symlink(loopTarget, filepath.Join("local", "sub", "loop")); err != nil { + t.Fatalf("Symlink: %v", err) + } + + reg.Register(&httpmock.Stub{ + Method: "GET", + URL: "folder_token=folder_root", + Body: map[string]interface{}{ + "code": 0, "msg": "ok", + "data": map[string]interface{}{ + "files": []interface{}{}, + "has_more": false, + }, + }, + }) + + err = mountAndRunDrive(t, DrivePull, []string{ + "+pull", + "--local-dir", "local", + "--folder-token", "folder_root", + "--delete-local", + "--yes", + "--as", "bot", + }, f, stdout) + if err != nil { + t.Fatalf("unexpected error: %v\nstdout: %s", err, stdout.String()) + } + if _, err := os.Stat(filepath.Join("local", "sub", "real.txt")); !os.IsNotExist(err) { + t.Fatalf("real.txt should be deleted (proves walk completed)") + } +} + +// TestDrivePullDownloadDoesNotEscapeViaSymlinkParentRef pins the second +// half of the canonical-root fix: with --local-dir "link/..", which +// SafeInputPath happily accepts (filepath.Clean shrinks "link/.." to +// "."), download targets must land inside the canonical cwd, never +// inside the symlink target's parent. Without the fix the download +// would write into a sibling directory. +func TestDrivePullDownloadDoesNotEscapeViaSymlinkParentRef(t *testing.T) { + f, stdout, _, reg := cmdutil.TestFactory(t, driveTestConfig()) + + // escapeDir is a sibling temp dir; nothing should ever land here. + escapeDir := t.TempDir() + if err := os.WriteFile(filepath.Join(escapeDir, "preexisting.txt"), []byte("DO-NOT-TOUCH"), 0o644); err != nil { + t.Fatalf("WriteFile preexisting: %v", err) + } + + cwdDir := t.TempDir() + withDriveWorkingDir(t, cwdDir) + if err := os.Symlink(escapeDir, filepath.Join(cwdDir, "link")); err != nil { + t.Fatalf("Symlink: %v", err) + } + + reg.Register(&httpmock.Stub{ + Method: "GET", + URL: "folder_token=folder_root", + Body: map[string]interface{}{ + "code": 0, "msg": "ok", + "data": map[string]interface{}{ + "files": []interface{}{ + map[string]interface{}{"token": "tok_x", "name": "downloaded.txt", "type": "file"}, + }, + "has_more": false, + }, + }, + }) + reg.Register(&httpmock.Stub{ + Method: "GET", + URL: "/open-apis/drive/v1/files/tok_x/download", + Status: 200, + Body: []byte("REMOTE-BODY"), + Headers: http.Header{"Content-Type": []string{"application/octet-stream"}}, + }) + + err := mountAndRunDrive(t, DrivePull, []string{ + "+pull", + "--local-dir", "link/..", + "--folder-token", "folder_root", + "--as", "bot", + }, f, stdout) + if err != nil { + t.Fatalf("unexpected error: %v\nstdout: %s", err, stdout.String()) + } + + mustReadFile(t, filepath.Join(cwdDir, "downloaded.txt"), "REMOTE-BODY") + if _, err := os.Stat(filepath.Join(escapeDir, "downloaded.txt")); !os.IsNotExist(err) { + t.Fatalf("downloaded.txt must NOT land in escape dir; stat err=%v", err) + } + mustReadFile(t, filepath.Join(escapeDir, "preexisting.txt"), "DO-NOT-TOUCH") +} + +// TestDrivePullRejectsAbsoluteLocalDir confirms SafeLocalFlagPath surfaces +// the proper flag name in the error message. +func TestDrivePullRejectsAbsoluteLocalDir(t *testing.T) { + f, _, _, _ := cmdutil.TestFactory(t, driveTestConfig()) + + tmpDir := t.TempDir() + withDriveWorkingDir(t, tmpDir) + + err := mountAndRunDrive(t, DrivePull, []string{ + "+pull", + "--local-dir", "/etc", + "--folder-token", "folder_root", + "--as", "bot", + }, f, nil) + if err == nil { + t.Fatal("expected validation error for absolute --local-dir, got nil") + } + if !strings.Contains(err.Error(), "--local-dir") { + t.Fatalf("error must reference --local-dir, got: %v", err) + } +} + +// TestDrivePullRejectsBadIfExistsEnum verifies the framework's enum guard. +func TestDrivePullRejectsBadIfExistsEnum(t *testing.T) { + f, _, _, _ := cmdutil.TestFactory(t, driveTestConfig()) + + tmpDir := t.TempDir() + withDriveWorkingDir(t, tmpDir) + if err := os.MkdirAll("local", 0o755); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + + err := mountAndRunDrive(t, DrivePull, []string{ + "+pull", + "--local-dir", "local", + "--folder-token", "folder_root", + "--if-exists", "fail-and-die", + "--as", "bot", + }, f, nil) + if err == nil { + t.Fatal("expected enum validation error, got nil") + } + if !strings.Contains(err.Error(), "if-exists") { + t.Fatalf("error must reference --if-exists, got: %v", err) + } +} + +func mustReadFile(t *testing.T, path, want string) { + t.Helper() + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("ReadFile(%s): %v", path, err) + } + if string(data) != want { + t.Fatalf("file %s content = %q, want %q", path, string(data), want) + } +} + +// assertDrivePullPartialFailure asserts that err is the structured +// partial_failure ExitError +pull returns when any item-level failure +// happens, and returns the unwrapped *ExitError so the caller can drill +// into Detail.Detail without re-doing the type assertion. +func assertDrivePullPartialFailure(t *testing.T, err error) *output.ExitError { + t.Helper() + if err == nil { + t.Fatal("expected partial_failure ExitError, got nil") + } + var exitErr *output.ExitError + if !errors.As(err, &exitErr) { + t.Fatalf("expected *output.ExitError, got %T: %v", err, err) + } + if exitErr.Code != output.ExitAPI { + t.Errorf("exit code = %d, want %d (ExitAPI)", exitErr.Code, output.ExitAPI) + } + if exitErr.Detail == nil { + t.Fatalf("ExitError.Detail must be set on partial_failure") + } + if exitErr.Detail.Type != "partial_failure" { + t.Errorf("error.type = %q, want partial_failure", exitErr.Detail.Type) + } + return exitErr +} + +// splitDrivePullDetail extracts the {summary, items[]} payload from the +// ExitError detail. We round-trip through JSON so test assertions don't +// depend on the concrete map types the production code happens to use. +func splitDrivePullDetail(t *testing.T, exitErr *output.ExitError) (map[string]interface{}, []map[string]interface{}) { + t.Helper() + raw, err := json.Marshal(exitErr.Detail.Detail) + if err != nil { + t.Fatalf("marshal detail: %v", err) + } + var got struct { + Summary map[string]interface{} `json:"summary"` + Items []map[string]interface{} `json:"items"` + } + if err := json.Unmarshal(raw, &got); err != nil { + t.Fatalf("unmarshal detail: %v\nraw=%s", err, string(raw)) + } + if got.Summary == nil { + t.Fatalf("error.detail missing summary; raw=%s", string(raw)) + } + return got.Summary, got.Items +} diff --git a/shortcuts/drive/drive_push.go b/shortcuts/drive/drive_push.go index 304845626d..05350fbf4f 100644 --- a/shortcuts/drive/drive_push.go +++ b/shortcuts/drive/drive_push.go @@ -26,9 +26,6 @@ import ( const ( drivePushIfExistsOverwrite = "overwrite" drivePushIfExistsSkip = "skip" - drivePushListPageSize = 200 - drivePushFileType = "file" - drivePushFolderType = "folder" ) type drivePushItem struct { @@ -40,16 +37,6 @@ type drivePushItem struct { Error string `json:"error,omitempty"` } -// remoteEntry captures everything we know about a single rel_path under the -// target Drive folder. type=file entries are upload candidates (we may -// overwrite them); other types (docx/sheet/folder/...) are recorded so -// --delete-remote can avoid touching them and so directory creation can -// short-circuit when a folder already exists. -type drivePushRemoteEntry struct { - FileToken string - Type string -} - // DrivePush is a one-way, file-level mirror from a local directory onto a // Drive folder: walks --local-dir, recursively lists --folder-token, and for // each rel_path uploads (or overwrites) the corresponding Drive file. With @@ -203,10 +190,29 @@ var DrivePush = common.Shortcut{ } fmt.Fprintf(runtime.IO().ErrOut, "Listing Drive folder: %s\n", common.MaskToken(folderToken)) - remoteFiles, remoteFolders, err := drivePushListRemote(ctx, runtime, folderToken, "") + entries, err := listRemoteFolder(ctx, runtime, folderToken, "") if err != nil { return err } + // Two views over the same listing: + // - remoteFiles drives upload / overwrite / orphan-delete + // decisions (only type=file entries are upload candidates; + // online docs / shortcuts are intentionally never overwritten + // or deleted by --delete-remote). + // - remoteFolders is the create_folder cache: lets the upload + // path skip create_folder when an intermediate folder already + // exists, and keeps directory recreation idempotent across + // reruns. + remoteFiles := make(map[string]driveRemoteEntry, len(entries)) + remoteFolders := make(map[string]driveRemoteEntry, len(entries)) + for rel, entry := range entries { + switch entry.Type { + case driveTypeFile: + remoteFiles[rel] = entry + case driveTypeFolder: + remoteFolders[rel] = entry + } + } var uploaded, skipped, failed, deletedRemote int items := make([]drivePushItem, 0) @@ -457,82 +463,6 @@ func drivePushWalkLocal(root, cwdCanonical string) (map[string]drivePushLocalFil return files, dirs, nil } -// drivePushListRemote recursively lists a Drive folder. -// -// Returns two views: -// - files: rel_path → entry, for type=file. Drives upload/overwrite -// decisions and (with --delete-remote) the deletion loop. -// - folders: rel_path → entry, for every type=folder hit. Lets the upload -// path skip create_folder when an intermediate folder already exists, -// and keeps directory recreation idempotent across reruns. -// -// Online docs (docx/sheet/bitable/...) and shortcuts are intentionally NOT -// returned. They are never deleted by --delete-remote (the contract says -// type=file only) and the upload path doesn't need them either: if an -// online doc happens to share a rel_path with a local file, the upload -// will create a sibling type=file under the same parent, which is the -// same behavior `lark-cli drive +upload` already has. -// -// TODO(post-#692/#696): when drive +status / +pull merge, lift this and the -// matching helpers in drive_status.go / drive_pull.go into a shared -// listRemoteFolderFiles in the drive package. -func drivePushListRemote(ctx context.Context, runtime *common.RuntimeContext, folderToken, relBase string) (map[string]drivePushRemoteEntry, map[string]drivePushRemoteEntry, error) { - files := make(map[string]drivePushRemoteEntry) - folders := make(map[string]drivePushRemoteEntry) - pageToken := "" - for { - params := map[string]interface{}{ - "folder_token": folderToken, - "page_size": fmt.Sprint(drivePushListPageSize), - } - if pageToken != "" { - params["page_token"] = pageToken - } - result, err := runtime.CallAPI("GET", "/open-apis/drive/v1/files", params, nil) - if err != nil { - return nil, nil, err - } - rawFiles, _ := result["files"].([]interface{}) - for _, item := range rawFiles { - f, ok := item.(map[string]interface{}) - if !ok { - continue - } - fType := common.GetString(f, "type") - fName := common.GetString(f, "name") - fToken := common.GetString(f, "token") - if fName == "" || fToken == "" { - continue - } - rel := drivePushJoinRel(relBase, fName) - switch fType { - case drivePushFileType: - files[rel] = drivePushRemoteEntry{FileToken: fToken, Type: fType} - case drivePushFolderType: - folders[rel] = drivePushRemoteEntry{FileToken: fToken, Type: fType} - subFiles, subFolders, err := drivePushListRemote(ctx, runtime, fToken, rel) - if err != nil { - return nil, nil, err - } - for k, v := range subFiles { - files[k] = v - } - for k, v := range subFolders { - folders[k] = v - } - default: - // online doc / shortcut — intentionally ignored. - } - } - hasMore, nextToken := common.PaginationMeta(result) - if !hasMore || nextToken == "" { - break - } - pageToken = nextToken - } - return files, folders, nil -} - // drivePushEnsureFolder ensures a folder chain (rel_dir relative to the root // folder identified by rootFolderToken) exists on Drive, creating any // missing segments via /open-apis/drive/v1/files/create_folder. Returns the @@ -759,19 +689,12 @@ func drivePushDeleteFile(_ context.Context, runtime *common.RuntimeContext, file _, err := runtime.CallAPI( "DELETE", fmt.Sprintf("/open-apis/drive/v1/files/%s", validate.EncodePathSegment(fileToken)), - map[string]interface{}{"type": drivePushFileType}, + map[string]interface{}{"type": driveTypeFile}, nil, ) return err } -func drivePushJoinRel(base, name string) string { - if base == "" { - return name - } - return base + "/" + name -} - // drivePushParentRel returns the parent rel_path of rel ("" when the file // lives at the root). The local walker emits forward-slash rel_paths so // path.Dir is the right primitive here, not filepath.Dir. diff --git a/shortcuts/drive/drive_push_test.go b/shortcuts/drive/drive_push_test.go index f58f0fae53..61adb8b58d 100644 --- a/shortcuts/drive/drive_push_test.go +++ b/shortcuts/drive/drive_push_test.go @@ -797,11 +797,12 @@ func TestDrivePushExitsZeroOnCleanRun(t *testing.T) { // native cloud document on Drive (sheet/docx/bitable/...). // // The contract: -// - The native doc is NOT a candidate for overwrite — drivePushListRemote -// only puts type=file entries into the remoteFiles map, so the local -// "report" is treated as new and the upload_all body must NOT carry -// the sheet's token in the file_token form field (that would mean -// overwriting the sheet's bytes, not creating a sibling). +// - The native doc is NOT a candidate for overwrite — the remoteFiles +// view collapsed off listRemoteFolder only keeps type=file entries, +// so the local "report" is treated as new and the upload_all body +// must NOT carry the sheet's token in the file_token form field +// (that would mean overwriting the sheet's bytes, not creating a +// sibling). // - The native doc must NOT be deleted by --delete-remote --yes either. // The orphan check iterates remoteFiles only; a sheet at the same // rel_path as a missing local file would otherwise look orphaned. @@ -827,7 +828,9 @@ func TestDrivePushUploadsSiblingWhenRemoteSameNameIsNativeDoc(t *testing.T) { // - "minutes" as a native docx (type=docx, tok_docx) — no // local counterpart, --delete-remote must skip it // No type=file entries at all — every local rel_path is a "new upload" - // from drivePushListRemote's perspective. + // from the remoteFiles view's perspective (listRemoteFolder returns the + // sheet/docx in the unified entry map but they're filtered out when + // collapsing to the type=file view). reg.Register(&httpmock.Stub{ Method: "GET", URL: "folder_token=folder_root", @@ -1185,10 +1188,10 @@ func TestDrivePushHelpersRelPath(t *testing.T) { if parent, name := drivePushSplitRel("solo"); parent != "" || name != "solo" { t.Fatalf("split solo = (%q,%q), want (\"\",solo)", parent, name) } - if got := drivePushJoinRel("", "x"); got != "x" { - t.Fatalf("joinRel(\"\", x) = %q, want x", got) + if got := joinRelDrive("", "x"); got != "x" { + t.Fatalf("joinRelDrive(\"\", x) = %q, want x", got) } - if got := drivePushJoinRel("a", "x"); got != "a/x" { - t.Fatalf("joinRel(a, x) = %q, want a/x", got) + if got := joinRelDrive("a", "x"); got != "a/x" { + t.Fatalf("joinRelDrive(a, x) = %q, want a/x", got) } } diff --git a/shortcuts/drive/drive_status.go b/shortcuts/drive/drive_status.go index 47866f3e40..5456bd45a8 100644 --- a/shortcuts/drive/drive_status.go +++ b/shortcuts/drive/drive_status.go @@ -21,12 +21,6 @@ import ( "github.com/larksuite/cli/shortcuts/common" ) -const ( - driveStatusListPageSize = 200 - driveStatusFileType = "file" - driveStatusFolderType = "folder" -) - type driveStatusEntry struct { RelPath string `json:"rel_path"` FileToken string `json:"file_token,omitempty"` @@ -124,10 +118,21 @@ var DriveStatus = common.Shortcut{ } fmt.Fprintf(runtime.IO().ErrOut, "Listing Drive folder: %s\n", common.MaskToken(folderToken)) - remoteFiles, err := listRemoteForStatus(ctx, runtime, folderToken, "") + entries, err := listRemoteFolder(ctx, runtime, folderToken, "") if err != nil { return err } + // +status only diffs binary content, so collapse the unified + // listing to type=file. Online docs / shortcuts have no + // hashable bytes and are intentionally absent from the diff + // view (a docx living next to a same-named local file is a + // known no-op). + remoteFiles := make(map[string]string, len(entries)) + for rel, entry := range entries { + if entry.Type == driveTypeFile { + remoteFiles[rel] = entry.FileToken + } + } paths := mergeStatusPaths(localHashes, remoteFiles) @@ -220,59 +225,6 @@ func hashLocalForStatus(runtime *common.RuntimeContext, path string) (string, er return hex.EncodeToString(h.Sum(nil)), nil } -func listRemoteForStatus(ctx context.Context, runtime *common.RuntimeContext, folderToken, relBase string) (map[string]string, error) { - files := make(map[string]string) - pageToken := "" - for { - params := map[string]interface{}{ - "folder_token": folderToken, - "page_size": fmt.Sprint(driveStatusListPageSize), - } - if pageToken != "" { - params["page_token"] = pageToken - } - result, err := runtime.CallAPI("GET", "/open-apis/drive/v1/files", params, nil) - if err != nil { - return nil, err - } - rawFiles, _ := result["files"].([]interface{}) - for _, item := range rawFiles { - f, ok := item.(map[string]interface{}) - if !ok { - continue - } - fType := common.GetString(f, "type") - fName := common.GetString(f, "name") - fToken := common.GetString(f, "token") - if fName == "" || fToken == "" { - continue - } - switch fType { - case driveStatusFileType: - files[joinRelStatus(relBase, fName)] = fToken - case driveStatusFolderType: - subFiles, err := listRemoteForStatus(ctx, runtime, fToken, joinRelStatus(relBase, fName)) - if err != nil { - return nil, err - } - for k, v := range subFiles { - files[k] = v - } - } - } - // Drive's list endpoint has historically returned next_page_token, - // but routing through the shared helper accepts both page_token - // and next_page_token — keeps us aligned with okr/im, and - // future-proofs against a backend rename. - hasMore, nextToken := common.PaginationMeta(result) - if !hasMore || nextToken == "" { - break - } - pageToken = nextToken - } - return files, nil -} - func hashRemoteForStatus(ctx context.Context, runtime *common.RuntimeContext, fileToken string) (string, error) { resp, err := runtime.DoAPIStream(ctx, &larkcore.ApiReq{ HttpMethod: "GET", @@ -289,13 +241,6 @@ func hashRemoteForStatus(ctx context.Context, runtime *common.RuntimeContext, fi return hex.EncodeToString(h.Sum(nil)), nil } -func joinRelStatus(base, name string) string { - if base == "" { - return name - } - return base + "/" + name -} - func mergeStatusPaths(local, remote map[string]string) []string { seen := make(map[string]struct{}, len(local)+len(remote)) for p := range local { diff --git a/shortcuts/drive/list_remote.go b/shortcuts/drive/list_remote.go new file mode 100644 index 0000000000..a0a583b62d --- /dev/null +++ b/shortcuts/drive/list_remote.go @@ -0,0 +1,116 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package drive + +import ( + "context" + "fmt" + + "github.com/larksuite/cli/shortcuts/common" +) + +const ( + driveListRemotePageSize = 200 + driveTypeFile = "file" + driveTypeFolder = "folder" +) + +// driveRemoteEntry is one Drive entry returned by listRemoteFolder. It +// carries enough metadata for every shortcut that consumes the listing +// to build its own per-shortcut view by filtering on Type. +type driveRemoteEntry struct { + // FileToken is the Drive token for this entry. For type=folder this + // is the folder_token; for everything else it is the file_token. + FileToken string + // Type is the Drive entry kind verbatim from the API: + // "file" | "folder" | "docx" | "doc" | "sheet" | "bitable" | + // "mindnote" | "slides" | "shortcut" | … + Type string + // RelPath is the entry's path relative to the listing root. Encoded + // with "/" separators on every platform so it matches the rel_paths + // produced by the shortcuts' local walkers. + RelPath string +} + +// listRemoteFolder recursively lists folderToken under relBase and +// returns one entry per Drive item, keyed by rel_path. Subfolders are +// descended into and the folder's own entry is also recorded — callers +// can reason about "this rel_path is occupied by a folder" without +// re-listing. +// +// This is the shared backbone for the three sync-disk shortcuts. None +// of them need every field at every call site, so each one filters +// on Type: +// +// - +status (drive_status.go) keeps Type=="file" and uses FileToken +// to drive content-hash diffs against the local tree. +// - +pull (drive_pull.go) keeps Type=="file" + FileToken for the +// download set, and the full key set (every rel_path) as the +// guard for --delete-local. +// - +push (drive_push.go) keeps Type=="file" + FileToken for upload / +// overwrite / orphan-delete decisions, and Type=="folder" + FileToken +// for the create_folder cache. +// +// Pagination uses common.PaginationMeta, which accepts both +// page_token and next_page_token — the Drive list endpoint has +// historically returned the latter, but the helper future-proofs +// against a backend rename. +func listRemoteFolder(ctx context.Context, runtime *common.RuntimeContext, folderToken, relBase string) (map[string]driveRemoteEntry, error) { + out := make(map[string]driveRemoteEntry) + pageToken := "" + for { + params := map[string]interface{}{ + "folder_token": folderToken, + "page_size": fmt.Sprint(driveListRemotePageSize), + } + if pageToken != "" { + params["page_token"] = pageToken + } + result, err := runtime.CallAPI("GET", "/open-apis/drive/v1/files", params, nil) + if err != nil { + return nil, err + } + rawFiles, _ := result["files"].([]interface{}) + for _, item := range rawFiles { + f, ok := item.(map[string]interface{}) + if !ok { + continue + } + fType := common.GetString(f, "type") + fName := common.GetString(f, "name") + fToken := common.GetString(f, "token") + if fName == "" || fToken == "" { + continue + } + rel := joinRelDrive(relBase, fName) + out[rel] = driveRemoteEntry{FileToken: fToken, Type: fType, RelPath: rel} + if fType == driveTypeFolder { + sub, err := listRemoteFolder(ctx, runtime, fToken, rel) + if err != nil { + return nil, err + } + for k, v := range sub { + out[k] = v + } + } + } + hasMore, nextToken := common.PaginationMeta(result) + if !hasMore || nextToken == "" { + break + } + pageToken = nextToken + } + return out, nil +} + +// joinRelDrive joins a rel_path base with an entry name using "/". +// Empty base means the entry sits at the listing root. Mirrors the +// behavior the per-shortcut helpers used to ship and keeps rel_paths +// stable across +status / +pull / +push. +func joinRelDrive(base, name string) string { + if base == "" { + return name + } + return base + "/" + name +} diff --git a/shortcuts/drive/shortcuts.go b/shortcuts/drive/shortcuts.go index 1214c04f78..fcd3d805ea 100644 --- a/shortcuts/drive/shortcuts.go +++ b/shortcuts/drive/shortcuts.go @@ -20,6 +20,7 @@ func Shortcuts() []common.Shortcut { DriveDelete, DriveStatus, DrivePush, + DrivePull, DriveTaskResult, DriveApplyPermission, DriveSearch, diff --git a/shortcuts/drive/shortcuts_test.go b/shortcuts/drive/shortcuts_test.go index bec748aca7..3116c0c5ab 100644 --- a/shortcuts/drive/shortcuts_test.go +++ b/shortcuts/drive/shortcuts_test.go @@ -23,6 +23,7 @@ func TestShortcutsIncludesExpectedCommands(t *testing.T) { "+delete", "+status", "+push", + "+pull", "+task_result", "+apply-permission", "+search", diff --git a/skills/lark-drive/SKILL.md b/skills/lark-drive/SKILL.md index 9c195c97cc..c7078b3ec5 100644 --- a/skills/lark-drive/SKILL.md +++ b/skills/lark-drive/SKILL.md @@ -229,6 +229,7 @@ Shortcut 是对常用操作的高级封装（`lark-cli drive + [flags]`） | [`+create-folder`](references/lark-drive-create-folder.md) | Create a Drive folder, optionally under a parent folder, with bot auto-grant support | | [`+download`](references/lark-drive-download.md) | Download a file from Drive to local | | [`+status`](references/lark-drive-status.md) | Compare a local directory with a Drive folder by SHA-256 content hash; reports `new_local` / `new_remote` / `modified` / `unchanged` (read-only diff primitive for sync workflows). `--local-dir` 必须是 cwd 内的相对路径，越界路径 CLI 会直接拒绝；目标在 cwd 外时引导用户切换 agent 工作目录，不要私自 `cd` 绕过。 | +| [`+pull`](references/lark-drive-pull.md) | One-way **file-level** mirror of a Drive folder onto a local directory (Drive → local). Supports `--if-exists` (overwrite/skip) and `--delete-local` for orphan cleanup; the destructive `--delete-local` requires `--yes` and only unlinks regular files — empty local directories left behind by remote folder deletes are NOT pruned. Item-level failures exit non-zero (`error.type=partial_failure`) and skip the `--delete-local` pass to avoid half-synced state. `--local-dir` is bounded to cwd by CLI path validation; tell the user to switch the agent's working directory if the target is outside cwd. | | [`+create-shortcut`](references/lark-drive-create-shortcut.md) | Create a shortcut to an existing Drive file in another folder | | [`+add-comment`](references/lark-drive-add-comment.md) | Add a comment to doc/docx/sheet/slides, also supports wiki URL resolving to doc/docx/sheet/slides | | [`+export`](references/lark-drive-export.md) | Export a doc/docx/sheet/bitable to a local file with limited polling | diff --git a/skills/lark-drive/references/lark-drive-pull.md b/skills/lark-drive/references/lark-drive-pull.md new file mode 100644 index 0000000000..5ef290fbb7 --- /dev/null +++ b/skills/lark-drive/references/lark-drive-pull.md @@ -0,0 +1,113 @@ + +# drive +pull + +> **前置条件：** 先阅读 [`../lark-shared/SKILL.md`](../../lark-shared/SKILL.md) 了解认证、全局参数和安全规则。 + +把飞书云空间的某个文件夹**单向、文件级**镜像到本地目录（Drive → 本地）。命令递归列出 `--folder-token` 下所有 `type=file` 的文件，逐一下载到 `--local-dir` 对应的相对路径，子文件夹自动复刻为本地目录。 + +> ⚠️ **不是 directory-level mirror**：`--delete-local` 只删除本地"多余"的常规文件，不删除空目录。如果云端把整个子文件夹删了，对应的本地子目录会留空（里面的文件被清掉，目录本身保留）；想精确同步目录结构请自己 `rmdir` 处理空壳。 + +输出按"动作"分类： + +| 字段 | 含义 | +|------|------| +| `summary.downloaded` | 成功下载的文件数 | +| `summary.skipped` | 按 `--if-exists=skip` 跳过的文件数 | +| `summary.failed` | 下载或写盘失败的文件数 | +| `summary.deleted_local` | 启用 `--delete-local --yes` 时删除的本地文件数 | +| `items[]` | 每个文件的明细（`rel_path` / `file_token` / `action` / 失败时的 `error`） | + +`summary.failed > 0` 时命令以 **非零状态码**（`exit=1`，`error.type=partial_failure`）退出，且同一份 `summary + items` 会在 `error.detail` 里返回；脚本/agent 直接通过 exit code 判断成败即可，不需要再去解 `summary.failed`。 + +## 命令 + +```bash +# 基础用法 —— 把云端 fldcXXX 镜像到 ./repo +lark-cli drive +pull --local-dir ./repo --folder-token fldcnxxxxxxxxx + +# 已存在的本地文件保持不动 +lark-cli drive +pull --local-dir ./repo --folder-token fldcnxxxxxxxxx \ + --if-exists skip + +# 文件级镜像：下载新文件 + 删除云端没有的本地文件（不删空目录） +# （--delete-local 必须搭配 --yes，否则会被 Validate 直接拒绝） +lark-cli drive +pull --local-dir ./repo --folder-token fldcnxxxxxxxxx \ + --delete-local --yes +``` + +## 参数 + +| 标志 | 必填 | 类型 | 说明 | +|------|------|------|------| +| `--local-dir` | 是 | path | 本地根目录（**必须是 cwd 的相对路径**；绝对路径或逃出 cwd 的相对路径会被 CLI 直接拒绝） | +| `--folder-token` | 是 | string | 源 Drive 文件夹 token | +| `--if-exists` | 否 | enum | 本地文件已存在时的策略：`overwrite`（默认）/ `skip` | +| `--delete-local` | 否 | bool | 删除本地"云端没有的常规文件"（**不删空目录**，因此是 file-level mirror）；**必须配合 `--yes`** | +| `--yes` | 否 | bool | 确认 `--delete-local`；不传时该破坏性操作在 Validate 阶段被拒绝 | + +## 比较与下载范围 + +- **只下载 Drive `type=file` 的二进制文件**。在线文档（`docx` / `sheet` / `bitable` / `mindnote` / `slides`）和快捷方式（`shortcut`）会被跳过 —— 它们没有等价的本地二进制可写盘，否则会变成产生噪声的"假"下载。 +- 子文件夹会递归遍历；rel_path 形如 `sub1/sub2/file.txt`，本地缺失的父目录会被自动创建。 +- 已存在的本地文件按 `--if-exists` 决定 `overwrite` 还是 `skip`，没有第三种选择 —— 想做 `keep-both` 这类的请自己改名再 pull。 + +## --delete-local 的安全行为 + +`--delete-local` 是命令里**唯一的破坏性 flag**，会按"本地有但云端没有"清理本地常规文件。设计上把它跟 `--yes` 强绑定，且与下载阶段的失败联动： + +- `--delete-local`（无 `--yes`）→ Validate 直接报错：`--delete-local requires --yes`，没有任何下载、列表请求或删除发生。 +- `--delete-local --yes`，**且下载阶段全部成功** → 扫一遍 `--local-dir` 下所有常规文件，把不在云端清单里的逐个 `os.Remove`。**只删常规文件，不删目录**：远端文件夹被删除后，对应本地目录会保留空壳。 +- `--delete-local --yes`，**但下载阶段有任何条目失败** → **跳过整个删除阶段**，命令以 `partial_failure` 非零退出。设计意图：避免出现"前面下载失败、后面继续删本地文件"的半同步状态；操作者修好下载错误后再重跑即可。 +- 不传 `--delete-local` → `summary.deleted_local` 永远是 0；命令对本地"多余"文件视而不见。 + +第 6 章里把 `+pull --delete-local` 标了 `high-risk-write`，CLI 这边的实现等价于"未传 `--yes` 时拒绝执行"，符合该约束的精神。 + +## 输出 schema + +```json +{ + "summary": { + "downloaded": 0, + "skipped": 0, + "failed": 0, + "deleted_local": 0 + }, + "items": [ + {"rel_path": "...", "file_token": "...", "action": "downloaded"}, + {"rel_path": "...", "file_token": "...", "action": "skipped"}, + {"rel_path": "...", "file_token": "...", "action": "failed", "error": "..."}, + {"rel_path": "...", "action": "deleted_local"}, + {"rel_path": "...", "action": "delete_failed", "error": "..."} + ] +} +``` + +`rel_path` 始终用 `/` 作为分隔符（跨平台一致）。删除条目（`deleted_local` / `delete_failed`）没有 `file_token`，因为该文件本来就只在本地。 + +## 性能注意 + +- 下载流量 ≈ 云端待下载文件的总字节数。pull 是**全量**写盘 —— 跟 `+status` 不一样，不会跳过"内容相同"的文件（status 是按 hash 比较，pull 是按 `--if-exists`），所以一次跑可能很重。 +- 想避免重跑全量，可以先 `+status` 找出 `new_remote` 和 `modified`，再只对这些文件单独 `+download`。 +- 大文件会用 SDK 的流式下载（不会把整个 body 读进内存），但本地磁盘空间需要够。 + +## 所需 scope + +| 操作 | scope | +|------|-------| +| 列出文件夹 / 子目录 | `drive:drive.metadata:readonly` | +| 下载文件 | `drive:file:download` | + +如果当前 token 缺这些 scope，命令会直接报 `missing_scope` 并提示重新登录。`drive:drive` 在部分企业被策略禁用，所以 +pull 故意只声明上面这两个细粒度 scope。 + +## 范围限制 + +`--local-dir` 只接受 cwd 内的相对路径。CLI 会先 `EvalSymlinks` 整条路径，再判断它是否仍落在 cwd 内 —— **指向 cwd 外的符号链接也会被拒**，"在 cwd 内放一条软链指向外面" 这条捷径走不通，会直接撞上 `unsafe file path`。 + +如果用户想 pull 到 cwd 之外的目录，**不要 agent 自己 `cd` 绕过**。可以选：让用户在外部把 agent 工作目录切换到目标的祖先后重启会话；或者把目标整体物理移动 / 拷贝到 cwd 内（不是软链）；或者直接放弃这次同步，改用别的方式。 + +## 参考 + +- [lark-drive](../SKILL.md) —— 云空间全部命令 +- [lark-shared](../../lark-shared/SKILL.md) —— 认证和全局参数 +- [lark-drive-status](lark-drive-status.md) —— 下载前先看差异 +- [lark-drive-download](lark-drive-download.md) —— 单文件按需拉取 diff --git a/tests/cli_e2e/drive/drive_pull_dryrun_test.go b/tests/cli_e2e/drive/drive_pull_dryrun_test.go new file mode 100644 index 0000000000..25c515dafe --- /dev/null +++ b/tests/cli_e2e/drive/drive_pull_dryrun_test.go @@ -0,0 +1,173 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package drive + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + "time" + + clie2e "github.com/larksuite/cli/tests/cli_e2e" + "github.com/stretchr/testify/require" + "github.com/tidwall/gjson" +) + +// TestDrive_PullDryRun locks in the request shape the +pull shortcut emits +// under --dry-run: the real CLI binary is invoked end-to-end, so flag +// parsing, Validate (still runs in dry-run mode), and the dry-run renderer +// all execute. The printed envelope is then inspected for GET method, +// list-files URL, the folder_token parameter, and key phrases from Desc. +// +// Fake credentials are sufficient because --dry-run short-circuits before +// any real network call. +func TestDrive_PullDryRun(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + t.Setenv("LARKSUITE_CLI_APP_ID", "app") + t.Setenv("LARKSUITE_CLI_APP_SECRET", "secret") + t.Setenv("LARKSUITE_CLI_BRAND", "feishu") + + workDir := t.TempDir() + if err := os.MkdirAll(filepath.Join(workDir, "local"), 0o755); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + t.Cleanup(cancel) + + result, err := clie2e.RunCmd(ctx, clie2e.Request{ + Args: []string{ + "drive", "+pull", + "--local-dir", "local", + "--folder-token", "fldcnE2E001", + "--dry-run", + }, + WorkDir: workDir, + DefaultAs: "user", + }) + require.NoError(t, err) + result.AssertExitCode(t, 0) + + out := result.Stdout + if got := gjson.Get(out, "api.0.method").String(); got != "GET" { + t.Fatalf("method = %q, want GET\nstdout:\n%s", got, out) + } + if got := gjson.Get(out, "api.0.url").String(); got != "/open-apis/drive/v1/files" { + t.Fatalf("url = %q, want /open-apis/drive/v1/files\nstdout:\n%s", got, out) + } + if got := gjson.Get(out, "folder_token").String(); got != "fldcnE2E001" { + t.Fatalf("folder_token = %q, want fldcnE2E001\nstdout:\n%s", got, out) + } + desc := gjson.Get(out, "description").String() + if !strings.Contains(desc, "list --folder-token") { + t.Fatalf("description missing list phrase, got %q\nstdout:\n%s", desc, out) + } +} + +// TestDrive_PullDryRunRejectsAbsoluteLocalDir confirms the path validator +// runs in the real binary's Validate stage and surfaces a structured error +// referencing --local-dir. +func TestDrive_PullDryRunRejectsAbsoluteLocalDir(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + t.Setenv("LARKSUITE_CLI_APP_ID", "app") + t.Setenv("LARKSUITE_CLI_APP_SECRET", "secret") + t.Setenv("LARKSUITE_CLI_BRAND", "feishu") + + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + t.Cleanup(cancel) + + result, err := clie2e.RunCmd(ctx, clie2e.Request{ + Args: []string{ + "drive", "+pull", + "--local-dir", "/etc", + "--folder-token", "fldcnE2E001", + "--dry-run", + }, + WorkDir: t.TempDir(), + DefaultAs: "user", + }) + require.NoError(t, err) + if result.ExitCode == 0 { + t.Fatalf("absolute --local-dir must be rejected, got exit=0\nstdout:\n%s", result.Stdout) + } + combined := result.Stdout + "\n" + result.Stderr + if !strings.Contains(combined, "--local-dir") { + t.Fatalf("expected --local-dir in error, got:\nstdout:\n%s\nstderr:\n%s", result.Stdout, result.Stderr) + } +} + +// TestDrive_PullDryRunRejectsDeleteLocalWithoutYes locks in the safety +// guard: --delete-local without --yes must be refused upfront, even under +// --dry-run, so an unintended delete flag never silently slides through. +func TestDrive_PullDryRunRejectsDeleteLocalWithoutYes(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + t.Setenv("LARKSUITE_CLI_APP_ID", "app") + t.Setenv("LARKSUITE_CLI_APP_SECRET", "secret") + t.Setenv("LARKSUITE_CLI_BRAND", "feishu") + + workDir := t.TempDir() + if err := os.MkdirAll(filepath.Join(workDir, "local"), 0o755); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + t.Cleanup(cancel) + + result, err := clie2e.RunCmd(ctx, clie2e.Request{ + Args: []string{ + "drive", "+pull", + "--local-dir", "local", + "--folder-token", "fldcnE2E001", + "--delete-local", + "--dry-run", + }, + WorkDir: workDir, + DefaultAs: "user", + }) + require.NoError(t, err) + if result.ExitCode == 0 { + t.Fatalf("--delete-local without --yes must be rejected, got exit=0\nstdout:\n%s", result.Stdout) + } + combined := result.Stdout + "\n" + result.Stderr + if !strings.Contains(combined, "--yes") { + t.Fatalf("expected --yes hint in error, got:\nstdout:\n%s\nstderr:\n%s", result.Stdout, result.Stderr) + } +} + +// TestDrive_PullDryRunRejectsMissingFolderToken confirms cobra's +// required-flag enforcement runs before our custom Validate. +func TestDrive_PullDryRunRejectsMissingFolderToken(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + t.Setenv("LARKSUITE_CLI_APP_ID", "app") + t.Setenv("LARKSUITE_CLI_APP_SECRET", "secret") + t.Setenv("LARKSUITE_CLI_BRAND", "feishu") + + workDir := t.TempDir() + if err := os.MkdirAll(filepath.Join(workDir, "local"), 0o755); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + t.Cleanup(cancel) + + result, err := clie2e.RunCmd(ctx, clie2e.Request{ + Args: []string{ + "drive", "+pull", + "--local-dir", "local", + "--dry-run", + }, + WorkDir: workDir, + DefaultAs: "user", + }) + require.NoError(t, err) + if result.ExitCode == 0 { + t.Fatalf("missing --folder-token must be rejected, got exit=0\nstdout:\n%s", result.Stdout) + } + combined := result.Stdout + "\n" + result.Stderr + if !strings.Contains(combined, "folder-token") { + t.Fatalf("expected folder-token in error, got:\nstdout:\n%s\nstderr:\n%s", result.Stdout, result.Stderr) + } +}