From b52baa109d28d46e259d3ce5de48ac40021dc5fe Mon Sep 17 00:00:00 2001 From: fangshuyu Date: Tue, 28 Apr 2026 18:47:01 +0800 Subject: [PATCH 1/6] feat(drive): add +status shortcut for content-hash diff MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds `drive +status`, a read-only diff primitive that walks --local-dir, recursively lists --folder-token, and reports four buckets — new_local, new_remote, modified, unchanged — by SHA-256 content hash. Implementation notes: - Drive's list/metas APIs do not expose a content hash, so files present on both sides are downloaded via DoAPIStream and hashed in memory (sha256 + io.Copy, no disk write). Files only on one side are not fetched. The command stays Risk: "read". - Only Drive entries with type=file participate. Online docs (docx, sheet, bitable, mindnote, slides) and shortcuts are skipped — there is no equivalent local binary to hash against. - --local-dir is funneled through the framework's validate.SafeLocalFlagPath helper so that absolute paths and any .. that escapes cwd are rejected with --local-dir in the error message (rather than the internal default --file). FileIO().Stat() then enforces existence and the IsDir check. - Local walk uses filepath.WalkDir behind a //nolint:forbidigo comment. The runtime FileIO interface has no walker today and shortcuts can't import internal/vfs; SafeInputPath has already bounded the walk root inside cwd, so the bare walk is acceptable until a runtime-level walker lands. - Scopes: drive:drive.metadata:readonly (list folders) + drive:file:download (fetch files for hashing). The broader drive:drive scope is disabled by enterprise policy in some tenants; this narrower pair was verified end-to-end. Tests cover the four-bucket categorization with a nested subfolder and docx/shortcut filtering, plus validation errors for missing local-dir, non-directory local-dir, and absolute-path local-dir. --- shortcuts/drive/drive_status.go | 281 +++++++++++++++++++++++++++ shortcuts/drive/drive_status_test.go | 191 ++++++++++++++++++ shortcuts/drive/shortcuts.go | 1 + shortcuts/drive/shortcuts_test.go | 1 + 4 files changed, 474 insertions(+) create mode 100644 shortcuts/drive/drive_status.go create mode 100644 shortcuts/drive/drive_status_test.go diff --git a/shortcuts/drive/drive_status.go b/shortcuts/drive/drive_status.go new file mode 100644 index 000000000..203280553 --- /dev/null +++ b/shortcuts/drive/drive_status.go @@ -0,0 +1,281 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package drive + +import ( + "context" + "crypto/sha256" + "encoding/hex" + "fmt" + "io" + "io/fs" + "path/filepath" + "sort" + "strings" + + larkcore "github.com/larksuite/oapi-sdk-go/v3/core" + + "github.com/larksuite/cli/internal/output" + "github.com/larksuite/cli/internal/validate" + "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"` +} + +// DriveStatus walks --local-dir, recursively lists --folder-token, and reports +// four buckets (new_local, new_remote, modified, unchanged) by SHA-256 hash. +// +// Only Drive entries with type=file are compared; online docs (docx, sheet, +// bitable, mindnote, slides) and shortcuts are skipped because there is no +// equivalent local binary to hash against. +// +// SafeInputPath (applied by runtime.FileIO()) rejects absolute paths and any +// path that resolves outside cwd, which keeps the local side bounded to the +// caller's working directory. +var DriveStatus = common.Shortcut{ + Service: "drive", + Command: "+status", + Description: "Compare a local directory with a Drive folder by content hash", + Risk: "read", + 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: "Drive folder token", Required: true}, + }, + Tips: []string{ + "Only entries with type=file are compared; online docs (docx, sheet, bitable, mindnote, slides) and shortcuts are skipped.", + "Files present on both sides are downloaded and SHA-256 hashed in memory to decide modified vs unchanged; expect noticeable I/O on large folders.", + }, + 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) + } + // Path safety (absolute paths, traversal, symlink escape) is enforced + // upfront by the framework helper so the error message references the + // correct flag name; FileIO().Stat below would do the same check, but + // surface --file in its hint. + 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) + } + return nil + }, + DryRun: func(ctx context.Context, runtime *common.RuntimeContext) *common.DryRunAPI { + return common.NewDryRunAPI(). + Desc("Walk --local-dir, recursively list --folder-token, and download files present on both sides to compare SHA-256."). + 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")) + + fmt.Fprintf(runtime.IO().ErrOut, "Walking local: %s\n", localDir) + localHashes, err := walkLocalForStatus(runtime, localDir) + if err != nil { + return err + } + + fmt.Fprintf(runtime.IO().ErrOut, "Listing Drive folder: %s\n", common.MaskToken(folderToken)) + remoteFiles, err := listRemoteForStatus(ctx, runtime, folderToken, "") + if err != nil { + return err + } + + paths := mergeStatusPaths(localHashes, remoteFiles) + + var newLocal, newRemote, modified, unchanged []driveStatusEntry + for _, relPath := range paths { + localHash, hasLocal := localHashes[relPath] + remoteToken, hasRemote := remoteFiles[relPath] + switch { + case hasLocal && !hasRemote: + newLocal = append(newLocal, driveStatusEntry{RelPath: relPath}) + case !hasLocal && hasRemote: + newRemote = append(newRemote, driveStatusEntry{RelPath: relPath, FileToken: remoteToken}) + default: + remoteHash, err := hashRemoteForStatus(ctx, runtime, remoteToken) + if err != nil { + return err + } + entry := driveStatusEntry{RelPath: relPath, FileToken: remoteToken} + if localHash == remoteHash { + unchanged = append(unchanged, entry) + } else { + modified = append(modified, entry) + } + } + } + + runtime.Out(map[string]interface{}{ + "new_local": emptyIfNil(newLocal), + "new_remote": emptyIfNil(newRemote), + "modified": emptyIfNil(modified), + "unchanged": emptyIfNil(unchanged), + }, nil) + return nil + }, +} + +func walkLocalForStatus(runtime *common.RuntimeContext, root string) (map[string]string, error) { + files := make(map[string]string) + // FileIO has no walker today and shortcuts can't import internal/vfs; + // SafeInputPath (via runtime.FileIO().Stat above) has already bounded + // root inside cwd, so a vanilla filepath.WalkDir is acceptable here. + err := filepath.WalkDir(root, func(path 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 + } + rel, err := filepath.Rel(root, path) + if err != nil { + return err + } + sum, err := hashLocalForStatus(runtime, path) + if err != nil { + return err + } + files[filepath.ToSlash(rel)] = sum + return nil + }) + if err != nil { + return nil, output.Errorf(output.ExitInternal, "io", "walk %s: %s", root, err) + } + return files, nil +} + +func hashLocalForStatus(runtime *common.RuntimeContext, path string) (string, error) { + f, err := runtime.FileIO().Open(path) + if err != nil { + return "", common.WrapInputStatError(err) + } + defer f.Close() + h := sha256.New() + if _, err := io.Copy(h, f); err != nil { + return "", output.Errorf(output.ExitInternal, "io", "hash %s: %s", path, err) + } + 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 + } + } + } + hasMore, _ := result["has_more"].(bool) + nextToken := common.GetString(result, "next_page_token") + 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", + 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() + h := sha256.New() + if _, err := io.Copy(h, resp.Body); err != nil { + return "", output.ErrNetwork("hash remote %s: %s", common.MaskToken(fileToken), err) + } + 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 { + seen[p] = struct{}{} + } + for p := range remote { + seen[p] = struct{}{} + } + out := make([]string, 0, len(seen)) + for p := range seen { + out = append(out, p) + } + sort.Strings(out) + return out +} + +func emptyIfNil(s []driveStatusEntry) []driveStatusEntry { + if s == nil { + return []driveStatusEntry{} + } + return s +} diff --git a/shortcuts/drive/drive_status_test.go b/shortcuts/drive/drive_status_test.go new file mode 100644 index 000000000..0faecc42f --- /dev/null +++ b/shortcuts/drive/drive_status_test.go @@ -0,0 +1,191 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package drive + +import ( + "net/http" + "os" + "strings" + "testing" + + "github.com/larksuite/cli/internal/cmdutil" + "github.com/larksuite/cli/internal/httpmock" +) + +// TestDriveStatusCategorizesByHash exercises the four-bucket classification +// against a real walk of the temp dir and a mocked Drive listing. +func TestDriveStatusCategorizesByHash(t *testing.T) { + f, stdout, _, reg := cmdutil.TestFactory(t, driveTestConfig()) + + tmpDir := t.TempDir() + withDriveWorkingDir(t, tmpDir) + + // Local layout: + // local/a.txt — also on remote with different content → modified + // local/b.txt — only local → new_local + // local/sub/c.txt — also on remote with same content → unchanged + // Remote-only: + // d.txt → new_remote + if err := os.MkdirAll("local/sub", 0o755); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + if err := os.WriteFile("local/a.txt", []byte("aaa"), 0o644); err != nil { + t.Fatalf("WriteFile a.txt: %v", err) + } + if err := os.WriteFile("local/b.txt", []byte("bbb"), 0o644); err != nil { + t.Fatalf("WriteFile b.txt: %v", err) + } + if err := os.WriteFile("local/sub/c.txt", []byte("ccc"), 0o644); err != nil { + t.Fatalf("WriteFile sub/c.txt: %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"}, + map[string]interface{}{"token": "tok_d", "name": "d.txt", "type": "file"}, + // noise: an online doc and a shortcut should be ignored + map[string]interface{}{"token": "tok_doc", "name": "ignored.docx", "type": "docx"}, + map[string]interface{}{"token": "tok_sc", "name": "ignored.lnk", "type": "shortcut"}, + }, + "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_c", "name": "c.txt", "type": "file"}, + }, + "has_more": false, + }, + }, + }) + + // Download a.txt: remote content differs from local "aaa" → modified. + 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"}}, + }) + + // Download c.txt: remote content matches local "ccc" → unchanged. + reg.Register(&httpmock.Stub{ + Method: "GET", + URL: "/open-apis/drive/v1/files/tok_c/download", + Status: 200, + Body: []byte("ccc"), + Headers: http.Header{"Content-Type": []string{"application/octet-stream"}}, + }) + + err := mountAndRunDrive(t, DriveStatus, []string{ + "+status", + "--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() + checks := []struct { + bucket string + path string + token string + }{ + {"new_local", "b.txt", ""}, + {"new_remote", "d.txt", "tok_d"}, + {"modified", "a.txt", "tok_a"}, + {"unchanged", "sub/c.txt", "tok_c"}, + } + for _, c := range checks { + if !strings.Contains(out, `"`+c.bucket+`":`) { + t.Errorf("output missing bucket %q\noutput: %s", c.bucket, out) + } + if !strings.Contains(out, `"rel_path": "`+c.path+`"`) { + t.Errorf("output missing rel_path %q (expected in %s)\noutput: %s", c.path, c.bucket, out) + } + if c.token != "" && !strings.Contains(out, `"file_token": "`+c.token+`"`) { + t.Errorf("output missing file_token %q (expected in %s)\noutput: %s", c.token, c.bucket, out) + } + } + + if strings.Contains(out, "ignored.docx") || strings.Contains(out, "ignored.lnk") { + t.Errorf("output should skip docx/shortcut entries\noutput: %s", out) + } + + reg.Verify(t) +} + +func TestDriveStatusRejectsMissingLocalDir(t *testing.T) { + f, _, _, _ := cmdutil.TestFactory(t, driveTestConfig()) + + tmpDir := t.TempDir() + withDriveWorkingDir(t, tmpDir) + + err := mountAndRunDrive(t, DriveStatus, []string{ + "+status", + "--local-dir", "does-not-exist", + "--folder-token", "folder_root", + "--as", "bot", + }, f, nil) + if err == nil { + t.Fatal("expected validation error for missing local dir, got nil") + } +} + +func TestDriveStatusRejectsLocalFile(t *testing.T) { + f, _, _, _ := cmdutil.TestFactory(t, driveTestConfig()) + + tmpDir := t.TempDir() + withDriveWorkingDir(t, tmpDir) + if err := os.WriteFile("not-a-dir.txt", []byte("x"), 0o644); err != nil { + t.Fatalf("WriteFile: %v", err) + } + + err := mountAndRunDrive(t, DriveStatus, []string{ + "+status", + "--local-dir", "not-a-dir.txt", + "--folder-token", "folder_root", + "--as", "bot", + }, f, nil) + if err == nil { + t.Fatal("expected validation error when --local-dir is a file, got nil") + } + if !strings.Contains(err.Error(), "not a directory") { + t.Fatalf("unexpected error message: %v", err) + } +} + +func TestDriveStatusRejectsAbsoluteLocalDir(t *testing.T) { + f, _, _, _ := cmdutil.TestFactory(t, driveTestConfig()) + + tmpDir := t.TempDir() + withDriveWorkingDir(t, tmpDir) + + err := mountAndRunDrive(t, DriveStatus, []string{ + "+status", + "--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") + } +} diff --git a/shortcuts/drive/shortcuts.go b/shortcuts/drive/shortcuts.go index bf4680ce9..341b029cd 100644 --- a/shortcuts/drive/shortcuts.go +++ b/shortcuts/drive/shortcuts.go @@ -18,6 +18,7 @@ func Shortcuts() []common.Shortcut { DriveImport, DriveMove, DriveDelete, + DriveStatus, DriveTaskResult, DriveApplyPermission, DriveSearch, diff --git a/shortcuts/drive/shortcuts_test.go b/shortcuts/drive/shortcuts_test.go index 61c357699..97729dc0f 100644 --- a/shortcuts/drive/shortcuts_test.go +++ b/shortcuts/drive/shortcuts_test.go @@ -21,6 +21,7 @@ func TestShortcutsIncludesExpectedCommands(t *testing.T) { "+import", "+move", "+delete", + "+status", "+task_result", "+apply-permission", "+search", From 9dbef29bb77f0bf09c08751d9e1268b411196588 Mon Sep 17 00:00:00 2001 From: fangshuyu Date: Tue, 28 Apr 2026 18:47:01 +0800 Subject: [PATCH 2/6] docs(skills): document drive +status in lark-drive skill Adds references/lark-drive-status.md covering parameters, output schema, the type=file scoping rule, and the network-traffic caveat (hash is streamed in memory, but bytes still cross the wire). Notes that --local-dir is bounded to cwd by the CLI's path validation, and that when a user wants to compare a directory outside cwd the agent should ask the user to relocate or to switch the agent's working directory rather than `cd`-ing on its own. Wires +status into the Shortcuts table in SKILL.md. --- skills/lark-drive/SKILL.md | 1 + .../references/lark-drive-status.md | 89 +++++++++++++++++++ 2 files changed, 90 insertions(+) create mode 100644 skills/lark-drive/references/lark-drive-status.md diff --git a/skills/lark-drive/SKILL.md b/skills/lark-drive/SKILL.md index cda756944..ed084c19f 100644 --- a/skills/lark-drive/SKILL.md +++ b/skills/lark-drive/SKILL.md @@ -228,6 +228,7 @@ Shortcut 是对常用操作的高级封装(`lark-cli drive + [flags]`) | [`+upload`](references/lark-drive-upload.md) | Upload a local file to a Drive folder or wiki node | | [`+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` 绕过。 | | [`+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-status.md b/skills/lark-drive/references/lark-drive-status.md new file mode 100644 index 000000000..2c92f5ffe --- /dev/null +++ b/skills/lark-drive/references/lark-drive-status.md @@ -0,0 +1,89 @@ + +# drive +status + +> **前置条件:** 先阅读 [`../lark-shared/SKILL.md`](../../lark-shared/SKILL.md) 了解认证、全局参数和安全规则。 + +按 SHA-256 内容哈希比较本地目录与飞书云空间文件夹,输出四类差异: + +| 字段 | 含义 | +|------|------| +| `new_local` | 仅本地存在 | +| `new_remote` | 仅云端存在 | +| `modified` | 双端都存在但 hash 不一致 | +| `unchanged` | 双端都存在且 hash 一致 | + +只读命令:流式 hash,不下载落盘;但双端都有的文件会从云端拉一份字节流过来在内存里算 hash,大目录 / 大文件会有可观的网络流量。 + +## 命令 + +```bash +# 基础用法 —— 两个必填参数 +lark-cli drive +status \ + --local-dir ./repo \ + --folder-token fldcnxxxxxxxxx + +# 只看 hash 不一致的项(结合 --jq 过滤) +lark-cli drive +status \ + --local-dir ./repo \ + --folder-token fldcnxxxxxxxxx \ + --jq '.modified' +``` + +## 参数 + +| 标志 | 必填 | 类型 | 说明 | +|------|------|------|------| +| `--local-dir` | 是 | path | 本地根目录(**必须是 cwd 的相对路径**;绝对路径或逃逸到 cwd 外的相对路径会被 CLI 直接拒绝) | +| `--folder-token` | 是 | string | Drive 文件夹 token | + +## 输出 schema + +```json +{ + "new_local": [{"rel_path": "..."}], + "new_remote": [{"rel_path": "...", "file_token": "..."}], + "modified": [{"rel_path": "...", "file_token": "..."}], + "unchanged": [{"rel_path": "...", "file_token": "..."}] +} +``` + +`rel_path` 始终用 `/` 作为分隔符(跨平台一致),相对于 `--local-dir` 或 `--folder-token` 的根。仅本地存在时没有 `file_token` 字段。 + +## 比较范围 + +- **只比对 Drive `type=file` 的二进制文件**。在线文档(`docx` / `sheet` / `bitable` / `mindnote` / `slides`)和快捷方式(`shortcut`)都被跳过 —— 它们没有等价的本地二进制可对齐,否则会在 `new_remote` 里产生大量误报。 +- 子文件夹会递归遍历;rel_path 形如 `sub1/sub2/file.txt`。 +- 本地侧只比对常规文件(regular file);符号链接、设备文件等被忽略。 + +## 范围限制 + +`+status` 的本地侧只接受 cwd 下的相对路径。如果用户想比对的目录在 cwd 之外,**不要 agent 自己 `cd` 绕过**;告诉用户切换 agent 工作目录到合适的祖先后重试,或者把目标软链接到 cwd 内。CLI 会在路径越界时直接报错(`unsafe file path`),无需在 skill 这一层提前手动校验。 + +## 典型用法 + +把 +status 当作"先看差异、再决定怎么同步"的只读探针。常见接驳场景: + +- 想知道云端有什么本地没有的内容 → 看 `new_remote`,按需选择性拉取(`drive +download --file-token `)。 +- 想把本地新增的内容推到云端 → 看 `new_local`,再 `drive +upload --file --folder-token `(注意 +upload 不接受 0 字节文件)。 +- 想知道哪些文件在云端被同事改过 → 看 `modified`,逐个 `drive +download` 查内容差异。 + +## 性能注意 + +- `unchanged` + `modified` 的总字节数 = 本次需从云端下载的流量。100GB 的双端共享内容意味着 100GB 网络往返。 +- 仅一侧存在的文件不会被下载。 +- Hash 计算在内存里流式做(io.Copy → sha256.New),不会把云端文件落到磁盘。 + +## 所需 scope + +| 操作 | scope | +|------|-------| +| 列出文件夹 / 子目录 | `drive:drive.metadata:readonly` | +| 下载并 hash 文件 | `drive:file:download` | + +如果当前 token 缺这些 scope,命令会直接报 `missing_scope` 并提示重新登录。`drive:drive` 在部分企业被策略禁用,所以 +status 故意只声明上面这两个细粒度 scope。 + +## 参考 + +- [lark-drive](../SKILL.md) —— 云空间全部命令 +- [lark-shared](../../lark-shared/SKILL.md) —— 认证和全局参数 +- [lark-drive-upload](lark-drive-upload.md) / [lark-drive-download](lark-drive-download.md) —— 把 +status 输出接到推/拉动作上 From 4e2cb21d3f14eeee4c377bcf03a324338319bf1f Mon Sep 17 00:00:00 2001 From: fangshuyu Date: Tue, 28 Apr 2026 19:14:51 +0800 Subject: [PATCH 3/6] test(drive): cover --folder-token validation and add +status dry-run E2E Addresses two CodeRabbit review comments on PR #692: - Adds TestDriveStatusRejectsEmptyFolderToken and TestDriveStatusRejectsMalformedFolderToken so the Validate-stage required-check and the ResourceName format guard for --folder-token are exercised, not just --local-dir. - Adds tests/cli_e2e/drive/drive_status_dryrun_test.go which drives the real binary in dry-run mode and asserts: * the request shape (GET /open-apis/drive/v1/files with folder_token in the dry-run envelope), plus the description text, * --local-dir absolute paths are rejected by Validate (which still runs under --dry-run) with --local-dir surfaced in the message, * cobra's required-flag enforcement rejects a missing --folder-token before any custom validation. --- shortcuts/drive/drive_status_test.go | 52 +++++++ .../cli_e2e/drive/drive_status_dryrun_test.go | 139 ++++++++++++++++++ 2 files changed, 191 insertions(+) create mode 100644 tests/cli_e2e/drive/drive_status_dryrun_test.go diff --git a/shortcuts/drive/drive_status_test.go b/shortcuts/drive/drive_status_test.go index 0faecc42f..f45e764ff 100644 --- a/shortcuts/drive/drive_status_test.go +++ b/shortcuts/drive/drive_status_test.go @@ -189,3 +189,55 @@ func TestDriveStatusRejectsAbsoluteLocalDir(t *testing.T) { t.Fatal("expected validation error for absolute --local-dir, got nil") } } + +// TestDriveStatusRejectsEmptyFolderToken covers the Validate-stage required +// check that runs before ResourceName: an empty --folder-token must surface +// a structured FlagError referencing the flag name. +func TestDriveStatusRejectsEmptyFolderToken(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, DriveStatus, []string{ + "+status", + "--local-dir", "local", + "--folder-token", "", + "--as", "bot", + }, f, nil) + if err == nil { + t.Fatal("expected validation error for empty --folder-token, got nil") + } + if !strings.Contains(err.Error(), "--folder-token") { + t.Fatalf("error must reference --folder-token, got: %v", err) + } +} + +// TestDriveStatusRejectsMalformedFolderToken covers the ResourceName format +// guard: a token with control characters (newline) must be rejected before +// any API call is made. +func TestDriveStatusRejectsMalformedFolderToken(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, DriveStatus, []string{ + "+status", + "--local-dir", "local", + "--folder-token", "tok\nwithnewline", + "--as", "bot", + }, f, nil) + if err == nil { + t.Fatal("expected validation error for malformed --folder-token, got nil") + } + if !strings.Contains(err.Error(), "--folder-token") { + t.Fatalf("error must reference --folder-token, got: %v", err) + } +} diff --git a/tests/cli_e2e/drive/drive_status_dryrun_test.go b/tests/cli_e2e/drive/drive_status_dryrun_test.go new file mode 100644 index 000000000..12f2ee3d8 --- /dev/null +++ b/tests/cli_e2e/drive/drive_status_dryrun_test.go @@ -0,0 +1,139 @@ +// 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_StatusDryRun locks in the request shape the +status shortcut +// emits under --dry-run: the real CLI binary is invoked end-to-end, so the +// full flag-parsing, Validate (which still runs in dry-run mode), and the +// dry-run renderer all execute. The printed envelope is then inspected to +// confirm the GET method, list-files URL, and folder_token parameter, plus +// the descriptive text from Desc. +// +// Fake credentials are sufficient because --dry-run short-circuits before +// any network call. +func TestDrive_StatusDryRun(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") + + // Validate runs even under --dry-run, so we need a real --local-dir + // inside the working directory; create one in a temp tree. + 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", "+status", + "--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, "Walk --local-dir") || !strings.Contains(desc, "SHA-256") { + t.Fatalf("description missing key phrases, got %q\nstdout:\n%s", desc, out) + } +} + +// TestDrive_StatusDryRunRejectsAbsoluteLocalDir confirms that the +// --local-dir path validator runs in the real binary's Validate stage and +// surfaces a structured error referencing --local-dir (not the framework +// default --file). +func TestDrive_StatusDryRunRejectsAbsoluteLocalDir(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", "+status", + "--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 message, got:\nstdout:\n%s\nstderr:\n%s", result.Stdout, result.Stderr) + } +} + +// TestDrive_StatusDryRunRejectsMissingFolderToken confirms cobra's +// required-flag enforcement runs before our custom Validate. +func TestDrive_StatusDryRunRejectsMissingFolderToken(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", "+status", + "--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 message, got:\nstdout:\n%s\nstderr:\n%s", result.Stdout, result.Stderr) + } +} From 7e9bc858b5b157b5c3a4e6b5c11ca990316c5fb8 Mon Sep 17 00:00:00 2001 From: fangshuyu Date: Tue, 28 Apr 2026 20:20:03 +0800 Subject: [PATCH 4/6] fix(drive): walk +status on canonical absolute root to close symlink/.. escape MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reported in PR review: --local-dir was validated through SafeLocalFlagPath, but the actual walk used the user-supplied raw string. SafeLocalFlagPath returns the original value (it only checks the path through SafeInputPath and discards the canonical form), and SafeInputPath itself relies on filepath.Clean for path normalization. filepath.Clean shrinks "link/.." to "." purely as string manipulation, so the validator sees a path inside cwd. The kernel, however, resolves "link/.." through the symlink target's parent — which is outside cwd and is what filepath.WalkDir actually traverses. Fix: in Execute, resolve --local-dir via validate.SafeInputPath to get the canonical absolute path (this one fully evaluates symlinks across the entire path), and walk that path. Each absolute walk hit is converted to a cwd-relative form via filepath.Rel against validate.SafeInputPath(".") so FileIO.Open's existing SafeInputPath guard (which rejects absolute paths) still applies. Adds TestDriveStatusDoesNotEscapeViaSymlinkParentRef as a regression: it stages an "escape" sibling directory containing a sentinel file, adds a "link" symlink in cwd pointing into the escape directory, and runs +status with --local-dir "link/..". Without this fix, the raw walk visits the sentinel and leaks it into new_local; with the fix, the walk stays inside the canonical cwd. A standalone repro confirms the underlying behavior: raw filepath.WalkDir("link/..", ...) traversed dozens of unrelated files in the kernel-resolved parent directory; walking the canonical root visits only the legitimate cwd contents. --- shortcuts/drive/drive_status.go | 52 ++++++++++++++++++---- shortcuts/drive/drive_status_test.go | 65 ++++++++++++++++++++++++++++ 2 files changed, 109 insertions(+), 8 deletions(-) diff --git a/shortcuts/drive/drive_status.go b/shortcuts/drive/drive_status.go index 203280553..3b80637ca 100644 --- a/shortcuts/drive/drive_status.go +++ b/shortcuts/drive/drive_status.go @@ -95,8 +95,30 @@ var DriveStatus = common.Shortcut{ localDir := strings.TrimSpace(runtime.Str("local-dir")) folderToken := strings.TrimSpace(runtime.Str("folder-token")) + // Resolve --local-dir to its canonical absolute path before walking. + // SafeInputPath fully evaluates symlinks across the entire path, + // which closes the kernel-level escape route that filepath.Clean + // alone misses: e.g. "link/.." string-cleans to "." but the kernel + // resolves through link's target's parent, so a raw walk on the + // user-supplied string can land outside cwd. Walking the canonical + // root sidesteps that — and the matching cwd canonical lets each + // absolute walk hit be converted to a cwd-relative path that + // FileIO.Open's SafeInputPath check still accepts. + // + // Validate already ran SafeLocalFlagPath (with the proper flag + // name in the error message), so a failure here is unexpected and + // only possible under a Validate↔Execute race. + 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) + } + fmt.Fprintf(runtime.IO().ErrOut, "Walking local: %s\n", localDir) - localHashes, err := walkLocalForStatus(runtime, localDir) + localHashes, err := walkLocalForStatus(runtime, safeRoot, cwdCanonical) if err != nil { return err } @@ -142,23 +164,37 @@ var DriveStatus = common.Shortcut{ }, } -func walkLocalForStatus(runtime *common.RuntimeContext, root string) (map[string]string, error) { +// walkLocalForStatus walks the canonical absolute root produced by +// SafeInputPath. Using the canonical root keeps the kernel from +// following any symlink hidden inside the user-supplied --local-dir +// (e.g. "link/..", which filepath.Clean shrinks to "." but which OS +// path resolution would resolve through the symlink target). For each +// hit, we report rel_path relative to root for the JSON output, and +// convert the absolute path to a cwd-relative form so FileIO.Open's +// SafeInputPath check (which rejects absolute paths) still applies. +func walkLocalForStatus(runtime *common.RuntimeContext, root, cwdCanonical string) (map[string]string, error) { files := make(map[string]string) - // FileIO has no walker today and shortcuts can't import internal/vfs; - // SafeInputPath (via runtime.FileIO().Stat above) has already bounded - // root inside cwd, so a vanilla filepath.WalkDir is acceptable here. - err := filepath.WalkDir(root, func(path string, d fs.DirEntry, walkErr error) error { //nolint:forbidigo // see comment above + // FileIO has no walker today and shortcuts can't import internal/vfs. + // The walk root is the canonical absolute path returned by + // validate.SafeInputPath, so it is no longer a symlink itself, and + // WalkDir's default policy (do not follow child symlinks) keeps the + // traversal inside that canonical 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 } - rel, err := filepath.Rel(root, path) + rel, err := filepath.Rel(root, absPath) + if err != nil { + return err + } + relToCwd, err := filepath.Rel(cwdCanonical, absPath) if err != nil { return err } - sum, err := hashLocalForStatus(runtime, path) + sum, err := hashLocalForStatus(runtime, relToCwd) if err != nil { return err } diff --git a/shortcuts/drive/drive_status_test.go b/shortcuts/drive/drive_status_test.go index f45e764ff..4febb1caa 100644 --- a/shortcuts/drive/drive_status_test.go +++ b/shortcuts/drive/drive_status_test.go @@ -6,6 +6,7 @@ package drive import ( "net/http" "os" + "path/filepath" "strings" "testing" @@ -216,6 +217,70 @@ func TestDriveStatusRejectsEmptyFolderToken(t *testing.T) { } } +// TestDriveStatusDoesNotEscapeViaSymlinkParentRef is the regression for the +// "link/.." escape: filepath.Clean string-shrinks "link/.." to ".", so a +// raw walk on the user-supplied input can land on the kernel-resolved +// path through link's target's parent — outside cwd. The fix is to walk +// SafeInputPath's canonical absolute root instead of the raw input. +// +// Setup: an "escape" sibling directory contains a sentinel file; cwd +// contains a "link" symlink pointing into that escape directory. +// Calling +status with --local-dir "link/.." must not surface the +// sentinel — the walk must stay inside cwd. +func TestDriveStatusDoesNotEscapeViaSymlinkParentRef(t *testing.T) { + f, stdout, _, reg := cmdutil.TestFactory(t, driveTestConfig()) + + // Sentinel lives outside cwd; the agent must never see it. + escapeDir := t.TempDir() + if err := os.WriteFile(filepath.Join(escapeDir, "secret.txt"), []byte("S3CRET"), 0o644); err != nil { + t.Fatalf("WriteFile secret: %v", err) + } + + // cwd has a symlink that points into the sentinel's parent. + cwdDir := t.TempDir() + withDriveWorkingDir(t, cwdDir) + if err := os.Symlink(escapeDir, filepath.Join(cwdDir, "link")); err != nil { + t.Fatalf("Symlink: %v", err) + } + // A normal file inside cwd just to make the walk non-trivial. + if err := os.WriteFile(filepath.Join(cwdDir, "ok.txt"), []byte("ok"), 0o644); err != nil { + t.Fatalf("WriteFile ok: %v", err) + } + + // Empty remote folder so any path that surfaces in the output + // must have come from the local walk. + 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, DriveStatus, []string{ + "+status", + "--local-dir", "link/..", + "--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, "secret.txt") || strings.Contains(out, "S3CRET") { + t.Fatalf("walk escaped via link/..: secret.txt leaked into output\noutput:\n%s", out) + } + // ok.txt is in cwd and must classify as new_local (no remote stub for it). + if !strings.Contains(out, `"rel_path": "ok.txt"`) { + t.Fatalf("expected ok.txt in new_local, got:\n%s", out) + } +} + // TestDriveStatusRejectsMalformedFolderToken covers the ResourceName format // guard: a token with control characters (newline) must be rejected before // any API call is made. From 32eaf8ace773ffb446601ca5556eed145d7080c6 Mon Sep 17 00:00:00 2001 From: fangshuyu Date: Tue, 28 Apr 2026 20:34:15 +0800 Subject: [PATCH 5/6] test(drive): pin walker behavior on child / circular symlinks for +status Adds two corner-case regressions to back up the canonical-root walk fix: - TestDriveStatusSkipsSymlinkInsideRoot: a child symlink under --local-dir that points to a sibling temp dir outside cwd. WalkDir's default policy must report it as a non-regular entry so the callback skips it, and the sentinel inside the target must not surface in new_local. This pins the contract our caller relies on (walk declines to follow child symlinks even when the canonical root resolves cleanly). - TestDriveStatusSurvivesCircularSymlinkInsideRoot: a child symlink pointing back at one of its ancestors. The walk must terminate and surface the legitimate sibling file; if WalkDir ever followed the loop, the per-test timeout would catch it. --- shortcuts/drive/drive_status_test.go | 111 +++++++++++++++++++++++++++ 1 file changed, 111 insertions(+) diff --git a/shortcuts/drive/drive_status_test.go b/shortcuts/drive/drive_status_test.go index 4febb1caa..ae8526093 100644 --- a/shortcuts/drive/drive_status_test.go +++ b/shortcuts/drive/drive_status_test.go @@ -281,6 +281,117 @@ func TestDriveStatusDoesNotEscapeViaSymlinkParentRef(t *testing.T) { } } +// TestDriveStatusSkipsSymlinkInsideRoot pins down WalkDir's default policy +// for symlinks discovered as child entries: they are reported with a +// non-regular file mode and the callback skips them, so a symlink inside +// the validated root pointing into an out-of-tree directory cannot leak +// the target's contents. +func TestDriveStatusSkipsSymlinkInsideRoot(t *testing.T) { + f, stdout, _, reg := cmdutil.TestFactory(t, driveTestConfig()) + + // Sentinel sits outside cwd; a child symlink inside the walked root + // points there. If the walker followed child symlinks (it must not), + // the sentinel's name would surface in new_local. + escapeDir := t.TempDir() + if err := os.WriteFile(filepath.Join(escapeDir, "secret.txt"), []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) + } + // Child-of-root symlink that resolves out of the validated subtree. + if err := os.Symlink(escapeDir, filepath.Join("local", "sub", "escape")); 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, DriveStatus, []string{ + "+status", + "--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, "secret.txt") || strings.Contains(out, "S3CRET") { + t.Fatalf("walk followed child symlink and leaked sentinel:\n%s", out) + } + if !strings.Contains(out, `"rel_path": "ok.txt"`) { + t.Fatalf("expected ok.txt in new_local; got:\n%s", out) + } +} + +// TestDriveStatusSurvivesCircularSymlinkInsideRoot makes sure WalkDir +// terminates even when a child symlink points back at one of its +// ancestors. WalkDir's default policy already declines to follow child +// symlinks; this test pins that contract for our caller. +func TestDriveStatusSurvivesCircularSymlinkInsideRoot(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) + } + // loop symlink: cwd/local/sub/loop -> cwd/local (an ancestor). + 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, + }, + }, + }) + + // If WalkDir followed the loop, this test would never finish; the + // test runner's per-test timeout would surface that as a failure. + err = mountAndRunDrive(t, DriveStatus, []string{ + "+status", + "--local-dir", "local", + "--folder-token", "folder_root", + "--as", "bot", + }, f, stdout) + if err != nil { + t.Fatalf("unexpected error: %v\nstdout: %s", err, stdout.String()) + } + if !strings.Contains(stdout.String(), `"rel_path": "sub/real.txt"`) { + t.Fatalf("expected sub/real.txt in new_local; got:\n%s", stdout.String()) + } +} + // TestDriveStatusRejectsMalformedFolderToken covers the ResourceName format // guard: a token with control characters (newline) must be rejected before // any API call is made. From ac67edf9adada80c1d26d5fbd08e0a171e7b2b8c Mon Sep 17 00:00:00 2001 From: fangshuyu Date: Wed, 29 Apr 2026 10:58:39 +0800 Subject: [PATCH 6/6] fix(drive): close +status review gaps from Codex (pagination, doc, live E2E) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three independent fixes flagged on PR #692: 1. Route the recursive Drive folder listing through common.PaginationMeta instead of reading next_page_token directly. The shared helper accepts both page_token and next_page_token, matching what okr/im already do and keeping +status safe against a backend field rename. Adds TestDriveStatusPaginatesRemoteListing, which serves a 2-page response where page 1 advertises the cursor as next_page_token and page 2 as page_token; either spelling alone would silently drop one page. 2. The skill doc previously suggested "or symlink the target into cwd" as a workaround for cwd-relative --local-dir. SafeInputPath calls filepath.EvalSymlinks before checking isUnderDir(canonicalCwd), so any symlink whose final target sits outside cwd still gets rejected as `unsafe file path`. Rewrite the section so agents stop steering users into a path that always errors out. 3. Add tests/cli_e2e/drive/drive_status_workflow_test.go — the live E2E that AGENTS.md requires for new shortcuts. Seeds a real Drive folder with three uploaded files (unchanged.txt, modified.txt, remote-only.txt), seeds a local tree with matching/diverging content plus a local-only.txt, runs +status, and asserts each of the four buckets contains exactly the file we expect with the right file_token. Cleanup of every uploaded file plus the parent folder is registered through the existing best-effort cleanup helpers. Coverage table bumped: drive +status moves to ✓ and the denominator goes from 28→29 to account for the new shortcut. Codex also flagged the local-side filepath.WalkDir as a vfs-bypass. Investigated: the depguard rule shortcuts-no-vfs explicitly forbids shortcuts from importing internal/vfs (see commit c1b0bed on the +pull branch where the same migration was rejected by CI). The filepath.WalkDir + nolint:forbidigo pattern in walkLocalForStatus is the lint-required convention until FileIO grows a walker, so leaving it as-is. --- shortcuts/drive/drive_status.go | 7 +- shortcuts/drive/drive_status_test.go | 79 ++++++++ .../references/lark-drive-status.md | 2 +- tests/cli_e2e/drive/coverage.md | 8 +- .../drive/drive_status_workflow_test.go | 186 ++++++++++++++++++ 5 files changed, 276 insertions(+), 6 deletions(-) create mode 100644 tests/cli_e2e/drive/drive_status_workflow_test.go diff --git a/shortcuts/drive/drive_status.go b/shortcuts/drive/drive_status.go index 3b80637ca..47866f3e4 100644 --- a/shortcuts/drive/drive_status.go +++ b/shortcuts/drive/drive_status.go @@ -260,8 +260,11 @@ func listRemoteForStatus(ctx context.Context, runtime *common.RuntimeContext, fo } } } - hasMore, _ := result["has_more"].(bool) - nextToken := common.GetString(result, "next_page_token") + // 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 } diff --git a/shortcuts/drive/drive_status_test.go b/shortcuts/drive/drive_status_test.go index ae8526093..5467a2ff4 100644 --- a/shortcuts/drive/drive_status_test.go +++ b/shortcuts/drive/drive_status_test.go @@ -134,6 +134,85 @@ func TestDriveStatusCategorizesByHash(t *testing.T) { reg.Verify(t) } +// TestDriveStatusPaginatesRemoteListing pins multi-page handling end-to-end +// AND the dual-field tolerance of common.PaginationMeta. Page 1 surfaces +// `next_page_token` (Drive's historical name); page 2 surfaces `page_token` +// (what the shared helper also accepts). If the shortcut had hard-coded +// either field name, one of the two pages' files would be silently dropped +// from the comparison and would land in the wrong bucket. Stub order is +// significant: httpmock matches in registration order, and both stubs key on +// the GET .../files URL — they pop in turn, so page 1's response (with the +// continuation token) must be registered before page 2's terminator. +func TestDriveStatusPaginatesRemoteListing(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) + } + + // Page 1: returns one file plus a continuation token via + // next_page_token (the field Drive currently emits). + reg.Register(&httpmock.Stub{ + Method: "GET", + URL: "/open-apis/drive/v1/files", + Body: map[string]interface{}{ + "code": 0, "msg": "ok", + "data": map[string]interface{}{ + "files": []interface{}{ + map[string]interface{}{"token": "tok_p1", "name": "page1.txt", "type": "file"}, + }, + "has_more": true, + "next_page_token": "cursor-page-2", + }, + }, + }) + + // Page 2: returns the second file with has_more=false. This stub uses + // page_token (the alternate spelling) to lock in that the shared + // PaginationMeta helper accepts BOTH field names. + reg.Register(&httpmock.Stub{ + Method: "GET", + URL: "/open-apis/drive/v1/files", + Body: map[string]interface{}{ + "code": 0, "msg": "ok", + "data": map[string]interface{}{ + "files": []interface{}{ + map[string]interface{}{"token": "tok_p2", "name": "page2.txt", "type": "file"}, + }, + "has_more": false, + "page_token": "", + }, + }, + }) + + err := mountAndRunDrive(t, DriveStatus, []string{ + "+status", + "--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() + // Both pages contributed to new_remote (local is empty). + for _, want := range []string{ + `"rel_path": "page1.txt"`, + `"file_token": "tok_p1"`, + `"rel_path": "page2.txt"`, + `"file_token": "tok_p2"`, + } { + if !strings.Contains(out, want) { + t.Errorf("output missing %q (a page must have been silently dropped)\noutput: %s", want, out) + } + } + + reg.Verify(t) +} + func TestDriveStatusRejectsMissingLocalDir(t *testing.T) { f, _, _, _ := cmdutil.TestFactory(t, driveTestConfig()) diff --git a/skills/lark-drive/references/lark-drive-status.md b/skills/lark-drive/references/lark-drive-status.md index 2c92f5ffe..a87244232 100644 --- a/skills/lark-drive/references/lark-drive-status.md +++ b/skills/lark-drive/references/lark-drive-status.md @@ -57,7 +57,7 @@ lark-cli drive +status \ ## 范围限制 -`+status` 的本地侧只接受 cwd 下的相对路径。如果用户想比对的目录在 cwd 之外,**不要 agent 自己 `cd` 绕过**;告诉用户切换 agent 工作目录到合适的祖先后重试,或者把目标软链接到 cwd 内。CLI 会在路径越界时直接报错(`unsafe file path`),无需在 skill 这一层提前手动校验。 +`+status` 的本地侧只接受 cwd 下的相对路径。如果用户想比对的目录在 cwd 之外,**不要 agent 自己 `cd` 绕过**;让用户在合适的祖先目录重新启动 agent 后再跑。注意:把目标软链接到 cwd 内**也不行**——路径校验会先 `EvalSymlinks` 再判定是否越界,链接最终指向的真实目录如果在 cwd 之外,仍然会被 `unsafe file path` 拒掉。CLI 会在路径越界时直接报错,无需在 skill 这一层提前手动校验。 ## 典型用法 diff --git a/tests/cli_e2e/drive/coverage.md b/tests/cli_e2e/drive/coverage.md index c9c37b7f0..9b42dd39b 100644 --- a/tests/cli_e2e/drive/coverage.md +++ b/tests/cli_e2e/drive/coverage.md @@ -1,12 +1,13 @@ # Drive CLI E2E Coverage ## Metrics -- Denominator: 28 leaf commands -- Covered: 1 -- Coverage: 3.6% +- Denominator: 29 leaf commands +- Covered: 2 +- Coverage: 6.9% ## Summary - TestDrive_FilesCreateFolderWorkflow: proves `drive files create_folder` in `create_folder as bot`; helper asserts the returned folder token and registers best-effort cleanup via `drive files delete`. +- TestDrive_StatusWorkflow: proves `drive +status` against a real Drive folder. Seeds the remote side via `drive +upload` (`unchanged.txt`, `modified.txt`, `remote-only.txt`), seeds local files with the matching/diverging contents, and asserts every output bucket (`unchanged`, `modified`, `new_local`, `new_remote`) holds exactly the expected `rel_path` and `file_token`. Cleans up uploaded files and the parent folder via best-effort cleanup hooks. - TestDrive_ApplyPermissionDryRun / TestDrive_ApplyPermissionDryRunRejectsFullAccess: dry-run coverage for `drive +apply-permission`; asserts URL→type inference for docx/sheet/slides, explicit `--type` overriding URL inference when both a recognized URL and `--type` are supplied, bare-token + explicit `--type` path, request method/URL/type-query/perm/remark body shape, optional `remark` omission when unset, and client-side rejection of `--perm full_access`. Runs without hitting the live API. - Cleanup note: `drive files delete` is only exercised in cleanup and is intentionally left uncovered. - Blocked area: live upload, export, comment, permission, subscription, and reply flows still need deterministic remote fixtures and filesystem setup. @@ -24,6 +25,7 @@ | ✕ | drive +export-download | shortcut | | none | no export-download workflow yet | | ✕ | drive +import | shortcut | | none | no import workflow yet | | ✕ | drive +move | shortcut | | none | no move workflow yet | +| ✓ | drive +status | shortcut | drive_status_workflow_test.go::TestDrive_StatusWorkflow + drive_status_dryrun_test.go::TestDrive_StatusDryRun | `--local-dir`; `--folder-token`; bucketed `new_local` / `new_remote` / `modified` / `unchanged` outputs | dry-run pins request shape; live workflow seeds via `+upload` and asserts all four buckets | | ✕ | drive +task_result | shortcut | | none | no async task-result workflow yet | | ✕ | drive +upload | shortcut | drive_upload_dryrun_test.go::TestDriveUploadDryRun_WikiTarget (dry-run only) | `--wiki-token`; `parent_type=wiki`; `parent_node` | no live upload workflow yet | | ✕ | drive file.comment.replys create | api | | none | no reply workflow yet | diff --git a/tests/cli_e2e/drive/drive_status_workflow_test.go b/tests/cli_e2e/drive/drive_status_workflow_test.go new file mode 100644 index 000000000..ba39d8e33 --- /dev/null +++ b/tests/cli_e2e/drive/drive_status_workflow_test.go @@ -0,0 +1,186 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package drive + +import ( + "context" + "os" + "path/filepath" + "testing" + "time" + + clie2e "github.com/larksuite/cli/tests/cli_e2e" + "github.com/stretchr/testify/require" + "github.com/tidwall/gjson" +) + +// TestDrive_StatusWorkflow exercises +status against a real Drive folder so +// the parts that dry-run can't reach — recursive listing pagination, the +// download+hash leg, scope handling, and the SHA-256 comparison itself — +// are covered against the real backend. +// +// Layout: +// +// folder/ (--folder-token target) +// ├── unchanged.txt "match" ↔ local: "match" → unchanged +// ├── modified.txt "remote" ↔ local: "local" → modified +// └── remote-only.txt "remote" ↔ (none) → new_remote +// local/ (--local-dir target) +// ├── unchanged.txt "match" +// ├── modified.txt "local" +// └── local-only.txt "anything" → new_local +// +// Expected output: each of the four buckets contains exactly the file we +// expect, with file_token set for the three buckets that have a Drive side. +func TestDrive_StatusWorkflow(t *testing.T) { + parentT := t + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute) + t.Cleanup(cancel) + + suffix := clie2e.GenerateSuffix() + folderName := "lark-cli-e2e-drive-status-" + suffix + folderToken := createDriveFolder(t, parentT, ctx, folderName, "") + + // Local working directory. +status's --local-dir must be relative to + // the binary's cwd, so each upload + the +status invocation share the + // same WorkDir. + workDir := t.TempDir() + if err := os.MkdirAll(filepath.Join(workDir, "local"), 0o755); err != nil { + t.Fatalf("mkdir local: %v", err) + } + + // Helper: write a local file under workDir/. + writeLocal := func(rel, content string) { + t.Helper() + full := filepath.Join(workDir, rel) + if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil { + t.Fatalf("mkdir parent of %s: %v", rel, err) + } + if err := os.WriteFile(full, []byte(content), 0o644); err != nil { + t.Fatalf("write %s: %v", rel, err) + } + } + + // Helper: stage into a sibling temp file then upload it as + // under folderToken. +upload reads --file relative to its cwd. + uploadDriveFile := func(name, content string) string { + t.Helper() + // Stage outside `local/` so the local-side tree only sees what + // the test wants; +upload still reads relative to workDir. + stage := "_upload_" + name + writeLocal(stage, content) + t.Cleanup(func() { _ = os.Remove(filepath.Join(workDir, stage)) }) + + result, err := clie2e.RunCmd(ctx, clie2e.Request{ + Args: []string{ + "drive", "+upload", + "--file", stage, + "--folder-token", folderToken, + "--name", name, + }, + WorkDir: workDir, + DefaultAs: "bot", + }) + require.NoError(t, err) + result.AssertExitCode(t, 0) + result.AssertStdoutStatus(t, true) + + fileToken := gjson.Get(result.Stdout, "data.file_token").String() + require.NotEmpty(t, fileToken, "uploaded file should have a token, stdout:\n%s", result.Stdout) + + parentT.Cleanup(func() { + cleanupCtx, cleanupCancel := clie2e.CleanupContext() + defer cleanupCancel() + deleteResult, deleteErr := clie2e.RunCmdWithRetry(cleanupCtx, clie2e.Request{ + Args: []string{"drive", "+delete", "--file-token", fileToken, "--type", "file", "--yes"}, + DefaultAs: "bot", + }, clie2e.RetryOptions{}) + clie2e.ReportCleanupFailure(parentT, "delete drive file "+fileToken, deleteResult, deleteErr) + }) + return fileToken + } + + // Seed both sides. Order doesn't matter functionally, but doing the + // uploads first lets the +status listing pick up everything in a + // single pass. + tokUnchanged := uploadDriveFile("unchanged.txt", "match") + tokModified := uploadDriveFile("modified.txt", "remote") + tokRemoteOnly := uploadDriveFile("remote-only.txt", "remote") + + writeLocal("local/unchanged.txt", "match") // matches remote → unchanged + writeLocal("local/modified.txt", "local") // differs → modified + writeLocal("local/local-only.txt", "extra") // only here → new_local + + // Run +status against the real folder. + result, err := clie2e.RunCmd(ctx, clie2e.Request{ + Args: []string{ + "drive", "+status", + "--local-dir", "local", + "--folder-token", folderToken, + }, + WorkDir: workDir, + DefaultAs: "bot", + }) + require.NoError(t, err) + result.AssertExitCode(t, 0) + result.AssertStdoutStatus(t, true) + + // Assert each bucket contains exactly the file we expect, with the + // correct file_token for sides that have one. + out := result.Stdout + + cases := []struct { + bucket string + path string + token string // empty when the bucket has no Drive side + }{ + {"unchanged", "unchanged.txt", tokUnchanged}, + {"modified", "modified.txt", tokModified}, + {"new_local", "local-only.txt", ""}, + {"new_remote", "remote-only.txt", tokRemoteOnly}, + } + for _, c := range cases { + bucket := gjson.Get(out, "data."+c.bucket) + if !bucket.IsArray() { + t.Fatalf("data.%s must be an array, stdout:\n%s", c.bucket, out) + } + var found bool + bucket.ForEach(func(_, entry gjson.Result) bool { + if entry.Get("rel_path").String() != c.path { + return true // continue + } + found = true + if c.token != "" { + if got := entry.Get("file_token").String(); got != c.token { + t.Errorf("%s entry %q: file_token=%q want %q", c.bucket, c.path, got, c.token) + } + } else if entry.Get("file_token").String() != "" { + t.Errorf("%s entry %q must not carry file_token (local-only), stdout:\n%s", c.bucket, c.path, out) + } + return false // stop + }) + if !found { + t.Errorf("%s bucket missing %q\nstdout:\n%s", c.bucket, c.path, out) + } + } + + // Make sure each bucket is exactly the size we expect (4 files total, + // no double-bucketing). +upload may attach extra metadata (e.g. a + // folder type entry for `local/` itself) but the lister filters + // type=file so the buckets should be clean. + for _, b := range []struct { + bucket string + want int + }{ + {"unchanged", 1}, + {"modified", 1}, + {"new_local", 1}, + {"new_remote", 1}, + } { + got := int(gjson.Get(out, "data."+b.bucket+".#").Int()) + if got != b.want { + t.Errorf("data.%s length=%d want %d\nstdout:\n%s", b.bucket, got, b.want, out) + } + } +}