Skip to content

Commit 20b9cf4

Browse files
fangshuyu-768HomyeeKing
authored andcommitted
feat(drive): add +status shortcut for content-hash diff (larksuite#692)
* feat(drive): add +status shortcut for content-hash diff 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. * 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. * test(drive): cover --folder-token validation and add +status dry-run E2E Addresses two CodeRabbit review comments on PR larksuite#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. * fix(drive): walk +status on canonical absolute root to close symlink/.. escape 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. * 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. * fix(drive): close +status review gaps from Codex (pagination, doc, live E2E) Three independent fixes flagged on PR larksuite#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.
1 parent 97112ce commit 20b9cf4

9 files changed

Lines changed: 1240 additions & 3 deletions

File tree

shortcuts/drive/drive_status.go

Lines changed: 320 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,320 @@
1+
// Copyright (c) 2026 Lark Technologies Pte. Ltd.
2+
// SPDX-License-Identifier: MIT
3+
4+
package drive
5+
6+
import (
7+
"context"
8+
"crypto/sha256"
9+
"encoding/hex"
10+
"fmt"
11+
"io"
12+
"io/fs"
13+
"path/filepath"
14+
"sort"
15+
"strings"
16+
17+
larkcore "github.com/larksuite/oapi-sdk-go/v3/core"
18+
19+
"github.com/larksuite/cli/internal/output"
20+
"github.com/larksuite/cli/internal/validate"
21+
"github.com/larksuite/cli/shortcuts/common"
22+
)
23+
24+
const (
25+
driveStatusListPageSize = 200
26+
driveStatusFileType = "file"
27+
driveStatusFolderType = "folder"
28+
)
29+
30+
type driveStatusEntry struct {
31+
RelPath string `json:"rel_path"`
32+
FileToken string `json:"file_token,omitempty"`
33+
}
34+
35+
// DriveStatus walks --local-dir, recursively lists --folder-token, and reports
36+
// four buckets (new_local, new_remote, modified, unchanged) by SHA-256 hash.
37+
//
38+
// Only Drive entries with type=file are compared; online docs (docx, sheet,
39+
// bitable, mindnote, slides) and shortcuts are skipped because there is no
40+
// equivalent local binary to hash against.
41+
//
42+
// SafeInputPath (applied by runtime.FileIO()) rejects absolute paths and any
43+
// path that resolves outside cwd, which keeps the local side bounded to the
44+
// caller's working directory.
45+
var DriveStatus = common.Shortcut{
46+
Service: "drive",
47+
Command: "+status",
48+
Description: "Compare a local directory with a Drive folder by content hash",
49+
Risk: "read",
50+
Scopes: []string{"drive:drive.metadata:readonly", "drive:file:download"},
51+
AuthTypes: []string{"user", "bot"},
52+
Flags: []common.Flag{
53+
{Name: "local-dir", Desc: "local root directory (relative to cwd)", Required: true},
54+
{Name: "folder-token", Desc: "Drive folder token", Required: true},
55+
},
56+
Tips: []string{
57+
"Only entries with type=file are compared; online docs (docx, sheet, bitable, mindnote, slides) and shortcuts are skipped.",
58+
"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.",
59+
},
60+
Validate: func(ctx context.Context, runtime *common.RuntimeContext) error {
61+
localDir := strings.TrimSpace(runtime.Str("local-dir"))
62+
folderToken := strings.TrimSpace(runtime.Str("folder-token"))
63+
if localDir == "" {
64+
return common.FlagErrorf("--local-dir is required")
65+
}
66+
if folderToken == "" {
67+
return common.FlagErrorf("--folder-token is required")
68+
}
69+
if err := validate.ResourceName(folderToken, "--folder-token"); err != nil {
70+
return output.ErrValidation("%s", err)
71+
}
72+
// Path safety (absolute paths, traversal, symlink escape) is enforced
73+
// upfront by the framework helper so the error message references the
74+
// correct flag name; FileIO().Stat below would do the same check, but
75+
// surface --file in its hint.
76+
if _, err := validate.SafeLocalFlagPath("--local-dir", localDir); err != nil {
77+
return output.ErrValidation("%s", err)
78+
}
79+
info, err := runtime.FileIO().Stat(localDir)
80+
if err != nil {
81+
return common.WrapInputStatError(err)
82+
}
83+
if !info.IsDir() {
84+
return output.ErrValidation("--local-dir is not a directory: %s", localDir)
85+
}
86+
return nil
87+
},
88+
DryRun: func(ctx context.Context, runtime *common.RuntimeContext) *common.DryRunAPI {
89+
return common.NewDryRunAPI().
90+
Desc("Walk --local-dir, recursively list --folder-token, and download files present on both sides to compare SHA-256.").
91+
GET("/open-apis/drive/v1/files").
92+
Set("folder_token", runtime.Str("folder-token"))
93+
},
94+
Execute: func(ctx context.Context, runtime *common.RuntimeContext) error {
95+
localDir := strings.TrimSpace(runtime.Str("local-dir"))
96+
folderToken := strings.TrimSpace(runtime.Str("folder-token"))
97+
98+
// Resolve --local-dir to its canonical absolute path before walking.
99+
// SafeInputPath fully evaluates symlinks across the entire path,
100+
// which closes the kernel-level escape route that filepath.Clean
101+
// alone misses: e.g. "link/.." string-cleans to "." but the kernel
102+
// resolves through link's target's parent, so a raw walk on the
103+
// user-supplied string can land outside cwd. Walking the canonical
104+
// root sidesteps that — and the matching cwd canonical lets each
105+
// absolute walk hit be converted to a cwd-relative path that
106+
// FileIO.Open's SafeInputPath check still accepts.
107+
//
108+
// Validate already ran SafeLocalFlagPath (with the proper flag
109+
// name in the error message), so a failure here is unexpected and
110+
// only possible under a Validate↔Execute race.
111+
safeRoot, err := validate.SafeInputPath(localDir)
112+
if err != nil {
113+
return output.ErrValidation("--local-dir: %s", err)
114+
}
115+
cwdCanonical, err := validate.SafeInputPath(".")
116+
if err != nil {
117+
return output.ErrValidation("could not resolve cwd: %s", err)
118+
}
119+
120+
fmt.Fprintf(runtime.IO().ErrOut, "Walking local: %s\n", localDir)
121+
localHashes, err := walkLocalForStatus(runtime, safeRoot, cwdCanonical)
122+
if err != nil {
123+
return err
124+
}
125+
126+
fmt.Fprintf(runtime.IO().ErrOut, "Listing Drive folder: %s\n", common.MaskToken(folderToken))
127+
remoteFiles, err := listRemoteForStatus(ctx, runtime, folderToken, "")
128+
if err != nil {
129+
return err
130+
}
131+
132+
paths := mergeStatusPaths(localHashes, remoteFiles)
133+
134+
var newLocal, newRemote, modified, unchanged []driveStatusEntry
135+
for _, relPath := range paths {
136+
localHash, hasLocal := localHashes[relPath]
137+
remoteToken, hasRemote := remoteFiles[relPath]
138+
switch {
139+
case hasLocal && !hasRemote:
140+
newLocal = append(newLocal, driveStatusEntry{RelPath: relPath})
141+
case !hasLocal && hasRemote:
142+
newRemote = append(newRemote, driveStatusEntry{RelPath: relPath, FileToken: remoteToken})
143+
default:
144+
remoteHash, err := hashRemoteForStatus(ctx, runtime, remoteToken)
145+
if err != nil {
146+
return err
147+
}
148+
entry := driveStatusEntry{RelPath: relPath, FileToken: remoteToken}
149+
if localHash == remoteHash {
150+
unchanged = append(unchanged, entry)
151+
} else {
152+
modified = append(modified, entry)
153+
}
154+
}
155+
}
156+
157+
runtime.Out(map[string]interface{}{
158+
"new_local": emptyIfNil(newLocal),
159+
"new_remote": emptyIfNil(newRemote),
160+
"modified": emptyIfNil(modified),
161+
"unchanged": emptyIfNil(unchanged),
162+
}, nil)
163+
return nil
164+
},
165+
}
166+
167+
// walkLocalForStatus walks the canonical absolute root produced by
168+
// SafeInputPath. Using the canonical root keeps the kernel from
169+
// following any symlink hidden inside the user-supplied --local-dir
170+
// (e.g. "link/..", which filepath.Clean shrinks to "." but which OS
171+
// path resolution would resolve through the symlink target). For each
172+
// hit, we report rel_path relative to root for the JSON output, and
173+
// convert the absolute path to a cwd-relative form so FileIO.Open's
174+
// SafeInputPath check (which rejects absolute paths) still applies.
175+
func walkLocalForStatus(runtime *common.RuntimeContext, root, cwdCanonical string) (map[string]string, error) {
176+
files := make(map[string]string)
177+
// FileIO has no walker today and shortcuts can't import internal/vfs.
178+
// The walk root is the canonical absolute path returned by
179+
// validate.SafeInputPath, so it is no longer a symlink itself, and
180+
// WalkDir's default policy (do not follow child symlinks) keeps the
181+
// traversal inside that canonical subtree.
182+
err := filepath.WalkDir(root, func(absPath string, d fs.DirEntry, walkErr error) error { //nolint:forbidigo // see comment above
183+
if walkErr != nil {
184+
return walkErr
185+
}
186+
if d.IsDir() || !d.Type().IsRegular() {
187+
return nil
188+
}
189+
rel, err := filepath.Rel(root, absPath)
190+
if err != nil {
191+
return err
192+
}
193+
relToCwd, err := filepath.Rel(cwdCanonical, absPath)
194+
if err != nil {
195+
return err
196+
}
197+
sum, err := hashLocalForStatus(runtime, relToCwd)
198+
if err != nil {
199+
return err
200+
}
201+
files[filepath.ToSlash(rel)] = sum
202+
return nil
203+
})
204+
if err != nil {
205+
return nil, output.Errorf(output.ExitInternal, "io", "walk %s: %s", root, err)
206+
}
207+
return files, nil
208+
}
209+
210+
func hashLocalForStatus(runtime *common.RuntimeContext, path string) (string, error) {
211+
f, err := runtime.FileIO().Open(path)
212+
if err != nil {
213+
return "", common.WrapInputStatError(err)
214+
}
215+
defer f.Close()
216+
h := sha256.New()
217+
if _, err := io.Copy(h, f); err != nil {
218+
return "", output.Errorf(output.ExitInternal, "io", "hash %s: %s", path, err)
219+
}
220+
return hex.EncodeToString(h.Sum(nil)), nil
221+
}
222+
223+
func listRemoteForStatus(ctx context.Context, runtime *common.RuntimeContext, folderToken, relBase string) (map[string]string, error) {
224+
files := make(map[string]string)
225+
pageToken := ""
226+
for {
227+
params := map[string]interface{}{
228+
"folder_token": folderToken,
229+
"page_size": fmt.Sprint(driveStatusListPageSize),
230+
}
231+
if pageToken != "" {
232+
params["page_token"] = pageToken
233+
}
234+
result, err := runtime.CallAPI("GET", "/open-apis/drive/v1/files", params, nil)
235+
if err != nil {
236+
return nil, err
237+
}
238+
rawFiles, _ := result["files"].([]interface{})
239+
for _, item := range rawFiles {
240+
f, ok := item.(map[string]interface{})
241+
if !ok {
242+
continue
243+
}
244+
fType := common.GetString(f, "type")
245+
fName := common.GetString(f, "name")
246+
fToken := common.GetString(f, "token")
247+
if fName == "" || fToken == "" {
248+
continue
249+
}
250+
switch fType {
251+
case driveStatusFileType:
252+
files[joinRelStatus(relBase, fName)] = fToken
253+
case driveStatusFolderType:
254+
subFiles, err := listRemoteForStatus(ctx, runtime, fToken, joinRelStatus(relBase, fName))
255+
if err != nil {
256+
return nil, err
257+
}
258+
for k, v := range subFiles {
259+
files[k] = v
260+
}
261+
}
262+
}
263+
// Drive's list endpoint has historically returned next_page_token,
264+
// but routing through the shared helper accepts both page_token
265+
// and next_page_token — keeps us aligned with okr/im, and
266+
// future-proofs against a backend rename.
267+
hasMore, nextToken := common.PaginationMeta(result)
268+
if !hasMore || nextToken == "" {
269+
break
270+
}
271+
pageToken = nextToken
272+
}
273+
return files, nil
274+
}
275+
276+
func hashRemoteForStatus(ctx context.Context, runtime *common.RuntimeContext, fileToken string) (string, error) {
277+
resp, err := runtime.DoAPIStream(ctx, &larkcore.ApiReq{
278+
HttpMethod: "GET",
279+
ApiPath: fmt.Sprintf("/open-apis/drive/v1/files/%s/download", validate.EncodePathSegment(fileToken)),
280+
})
281+
if err != nil {
282+
return "", output.ErrNetwork("download %s: %s", common.MaskToken(fileToken), err)
283+
}
284+
defer resp.Body.Close()
285+
h := sha256.New()
286+
if _, err := io.Copy(h, resp.Body); err != nil {
287+
return "", output.ErrNetwork("hash remote %s: %s", common.MaskToken(fileToken), err)
288+
}
289+
return hex.EncodeToString(h.Sum(nil)), nil
290+
}
291+
292+
func joinRelStatus(base, name string) string {
293+
if base == "" {
294+
return name
295+
}
296+
return base + "/" + name
297+
}
298+
299+
func mergeStatusPaths(local, remote map[string]string) []string {
300+
seen := make(map[string]struct{}, len(local)+len(remote))
301+
for p := range local {
302+
seen[p] = struct{}{}
303+
}
304+
for p := range remote {
305+
seen[p] = struct{}{}
306+
}
307+
out := make([]string, 0, len(seen))
308+
for p := range seen {
309+
out = append(out, p)
310+
}
311+
sort.Strings(out)
312+
return out
313+
}
314+
315+
func emptyIfNil(s []driveStatusEntry) []driveStatusEntry {
316+
if s == nil {
317+
return []driveStatusEntry{}
318+
}
319+
return s
320+
}

0 commit comments

Comments
 (0)