From e33a29f200a672a6f46fbc0758db315f70f048dd Mon Sep 17 00:00:00 2001 From: renaocheng Date: Mon, 30 Mar 2026 18:27:18 +0800 Subject: [PATCH 01/17] feat: add +download shortcut for minutes media download --- shortcuts/minutes/minutes_download.go | 144 ++++++++++++++++++ shortcuts/minutes/shortcuts.go | 13 ++ shortcuts/register.go | 2 + skills/lark-minutes/SKILL.md | 25 ++- .../references/lark-minutes-download.md | 113 ++++++++++++++ 5 files changed, 295 insertions(+), 2 deletions(-) create mode 100644 shortcuts/minutes/minutes_download.go create mode 100644 shortcuts/minutes/shortcuts.go create mode 100644 skills/lark-minutes/references/lark-minutes-download.md diff --git a/shortcuts/minutes/minutes_download.go b/shortcuts/minutes/minutes_download.go new file mode 100644 index 00000000..8a5e2dfa --- /dev/null +++ b/shortcuts/minutes/minutes_download.go @@ -0,0 +1,144 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package minutes + +import ( + "context" + "fmt" + "io" + "net/http" + "os" + "path/filepath" + "strings" + "time" + + "github.com/larksuite/cli/internal/output" + "github.com/larksuite/cli/internal/validate" + "github.com/larksuite/cli/shortcuts/common" +) + +const defaultMediaDownloadTimeout = 300 * time.Second + +var MinutesDownload = common.Shortcut{ + Service: "minutes", + Command: "+download", + Description: "Download audio/video media file of a minute", + Risk: "read", + Scopes: []string{"minutes:minutes.media:export"}, + AuthTypes: []string{"user", "bot"}, + Flags: []common.Flag{ + {Name: "minute-token", Desc: "minute token (from the minutes URL)", Required: true}, + {Name: "output", Desc: "local save path (defaults to .media)"}, + {Name: "overwrite", Type: "bool", Desc: "overwrite existing output file"}, + {Name: "url-only", Type: "bool", Desc: "only print the download URL without downloading the file"}, + }, + DryRun: func(ctx context.Context, runtime *common.RuntimeContext) *common.DryRunAPI { + minuteToken := runtime.Str("minute-token") + outputPath := runtime.Str("output") + if outputPath == "" { + outputPath = minuteToken + ".media" + } + return common.NewDryRunAPI(). + GET("/open-apis/minutes/v1/minutes/:minute_token/media"). + Set("minute_token", minuteToken).Set("output", outputPath) + }, + Execute: func(ctx context.Context, runtime *common.RuntimeContext) error { + minuteToken := runtime.Str("minute-token") + outputPath := runtime.Str("output") + overwrite := runtime.Bool("overwrite") + urlOnly := runtime.Bool("url-only") + + if err := validate.ResourceName(minuteToken, "--minute-token"); err != nil { + return output.ErrValidation("%s", err) + } + + // 第一步：调用 API 获取下载链接 + data, err := runtime.DoAPIJSON(http.MethodGet, + fmt.Sprintf("/open-apis/minutes/v1/minutes/%s/media", validate.EncodePathSegment(minuteToken)), + nil, nil) + if err != nil { + return err + } + + downloadURL := common.GetString(data, "download_url") + if downloadURL == "" { + return output.Errorf(output.ExitAPI, "api_error", "API returned empty download_url") + } + + // --url-only 模式：仅输出下载链接 + if urlOnly { + runtime.Out(map[string]interface{}{ + "download_url": downloadURL, + }, nil) + return nil + } + + // 第二步：从下载链接下载文件 + if outputPath == "" { + outputPath = minuteToken + ".media" + } + safePath, err := validate.SafeOutputPath(outputPath) + if err != nil { + return output.ErrValidation("unsafe output path: %s", err) + } + if err := common.EnsureWritableFile(safePath, overwrite); err != nil { + return err + } + + fmt.Fprintf(runtime.IO().ErrOut, "Downloading media: %s\n", common.MaskToken(minuteToken)) + + sizeBytes, err := downloadMediaFile(ctx, runtime, downloadURL, safePath) + if err != nil { + return err + } + + runtime.Out(map[string]interface{}{ + "saved_path": safePath, + "size_bytes": sizeBytes, + }, nil) + return nil + }, +} + +// downloadMediaFile 从 pre-signed URL 流式下载媒体文件到本地 +func downloadMediaFile(ctx context.Context, runtime *common.RuntimeContext, downloadURL, safePath string) (int64, error) { + httpClient, err := runtime.Factory.HttpClient() + if err != nil { + return 0, output.ErrNetwork("failed to get HTTP client: %s", err) + } + + // 复制 client 并覆盖超时，避免默认 30s 超时导致大文件下载失败 + downloadClient := *httpClient + downloadClient.Timeout = defaultMediaDownloadTimeout + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, downloadURL, nil) + if err != nil { + return 0, output.ErrNetwork("invalid download URL: %s", err) + } + // 不发送 Authorization header，download_url 是 pre-signed URL + + resp, err := downloadClient.Do(req) + if err != nil { + return 0, output.ErrNetwork("download failed: %s", err) + } + defer resp.Body.Close() + + if resp.StatusCode >= 400 { + body, _ := io.ReadAll(io.LimitReader(resp.Body, 4096)) + if len(body) > 0 { + return 0, output.ErrNetwork("download failed: HTTP %d: %s", resp.StatusCode, strings.TrimSpace(string(body))) + } + return 0, output.ErrNetwork("download failed: HTTP %d", resp.StatusCode) + } + + if err := os.MkdirAll(filepath.Dir(safePath), 0755); err != nil { + return 0, output.Errorf(output.ExitInternal, "api_error", "cannot create parent directory: %s", err) + } + + sizeBytes, err := validate.AtomicWriteFromReader(safePath, resp.Body, 0644) + if err != nil { + return 0, output.Errorf(output.ExitInternal, "api_error", "cannot create file: %s", err) + } + return sizeBytes, nil +} diff --git a/shortcuts/minutes/shortcuts.go b/shortcuts/minutes/shortcuts.go new file mode 100644 index 00000000..9c1431f2 --- /dev/null +++ b/shortcuts/minutes/shortcuts.go @@ -0,0 +1,13 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package minutes + +import "github.com/larksuite/cli/shortcuts/common" + +// Shortcuts returns all minutes shortcuts. +func Shortcuts() []common.Shortcut { + return []common.Shortcut{ + MinutesDownload, + } +} diff --git a/shortcuts/register.go b/shortcuts/register.go index 30fd506d..3f8048fb 100644 --- a/shortcuts/register.go +++ b/shortcuts/register.go @@ -17,6 +17,7 @@ import ( "github.com/larksuite/cli/shortcuts/event" "github.com/larksuite/cli/shortcuts/im" "github.com/larksuite/cli/shortcuts/mail" + "github.com/larksuite/cli/shortcuts/minutes" "github.com/larksuite/cli/shortcuts/sheets" "github.com/larksuite/cli/shortcuts/task" "github.com/larksuite/cli/shortcuts/vc" @@ -36,6 +37,7 @@ func init() { allShortcuts = append(allShortcuts, base.Shortcuts()...) allShortcuts = append(allShortcuts, event.Shortcuts()...) allShortcuts = append(allShortcuts, mail.Shortcuts()...) + allShortcuts = append(allShortcuts, minutes.Shortcuts()...) allShortcuts = append(allShortcuts, task.Shortcuts()...) allShortcuts = append(allShortcuts, vc.Shortcuts()...) allShortcuts = append(allShortcuts, whiteboard.Shortcuts()...) diff --git a/skills/lark-minutes/SKILL.md b/skills/lark-minutes/SKILL.md index ecb6e1e3..7cf250c1 100644 --- a/skills/lark-minutes/SKILL.md +++ b/skills/lark-minutes/SKILL.md @@ -1,7 +1,7 @@ --- name: lark-minutes version: 1.0.0 -description: "飞书妙记：获取妙记基础信息（标题、封面、时长）和相关的 AI 产物（总结、待办、章节）。飞书妙记的 URL 格式为: http(s):///minutes/" +description: "飞书妙记：获取妙记基础信息（标题、封面、时长）和相关的 AI 产物（总结、待办、章节），下载妙记音视频文件。飞书妙记的 URL 格式为: http(s):///minutes/" metadata: requires: bins: ["lark-cli"] @@ -10,7 +10,7 @@ metadata: # minutes (v1) -> **前置条件：** 先阅读 [`../lark-shared/SKILL.md`](../lark-shared/SKILL.md) 了解认证、全局参数和安全规则。 +**CRITICAL — 开始前 MUST 先用 Read 工具读取 [`../lark-shared/SKILL.md`](../lark-shared/SKILL.md)，其中包含认证、权限处理** ## 核心概念 @@ -48,6 +48,26 @@ lark-cli vc +notes --minute-tokens obcnhijv43vq6bcsl5xasfb2 - 用户未指定需要查询妙记的哪些内容时，默认查询基础元信息和相关联的纪要产物信息。 - 用户未明确指定查看纪要产物（逐字稿、总结、待办、章节）时，向用户展示对应产物的链接即可，不需要直接读取产物内容。 +## Shortcuts（推荐优先使用） + +Shortcut 是对常用操作的高级封装（`lark-cli minutes + [flags]`）。有 Shortcut 的操作优先使用。 + +| Shortcut | 说明 | +|----------|------| +| [`+download`](references/lark-minutes-download.md) | Download audio/video media file of a minute | + +### 妙记音视频下载 + +下载妙记音视频文件到本地，或获取有效期 1 天的下载链接。详见 [minutes +download](references/lark-minutes-download.md)。 + +```bash +# 下载音视频文件到本地 +lark-cli minutes +download --minute-token obcnq3b9jl72l83w4f149w9c --output ./meeting.mp4 + +# 仅获取下载链接 +lark-cli minutes +download --minute-token obcnq3b9jl72l83w4f149w9c --url-only +``` + ## API Resources @@ -67,6 +87,7 @@ lark-cli minutes [flags] # 调用 API | 方法 | 所需 scope | |------|-----------| | `minutes.get` | `minutes:minutes:readonly` | +| `+download` | `minutes:minutes.media:export` | diff --git a/skills/lark-minutes/references/lark-minutes-download.md b/skills/lark-minutes/references/lark-minutes-download.md new file mode 100644 index 00000000..af445723 --- /dev/null +++ b/skills/lark-minutes/references/lark-minutes-download.md @@ -0,0 +1,113 @@ + +# minutes +download + +> **前置条件：** 先阅读 [`../lark-shared/SKILL.md`](../../lark-shared/SKILL.md) 了解认证、全局参数和安全规则。 + +下载妙记的音视频媒体文件到本地，或获取有效期 1 天的下载链接。只读操作。 + +本 skill 对应 shortcut：`lark-cli minutes +download`。 + +## 命令 + +```bash +# 下载音视频文件到本地 +lark-cli minutes +download --minute-token obcnq3b9jl72l83w4f149w9c --output ./meeting.mp4 + +# 仅获取下载链接（有效期 1 天），不下载文件 +lark-cli minutes +download --minute-token obcnq3b9jl72l83w4f149w9c --url-only + +# 覆盖已存在的本地文件 +lark-cli minutes +download --minute-token obcnq3b9jl72l83w4f149w9c --output ./meeting.mp4 --overwrite + +# 预览 API 调用 +lark-cli minutes +download --minute-token obcnq3b9jl72l83w4f149w9c --dry-run +``` + +## 参数 + +| 参数 | 必填 | 说明 | +|------|------|------| +| `--minute-token ` | 是 | 妙记 Token，从妙记 URL 末尾提取（24 位字符串） | +| `--output ` | 否 | 本地保存路径（默认 `.media`） | +| `--overwrite` | 否 | 覆盖已存在的输出文件 | +| `--url-only` | 否 | 仅返回下载链接，不下载文件 | +| `--dry-run` | 否 | 预览 API 调用，不执行 | + +## 核心约束 + +### 1. 妙记必须已完成转写 + +音视频文件仅在妙记转写完成后可下载。如果妙记尚未准备好，API 会返回 `2091003` 错误。 + +### 2. 下载链接有效期 1 天 + +`--url-only` 返回的链接有效期为 1 天，过期后需重新获取。 + +### 3. 频率限制 + +API 限流 5 次/秒，批量下载时需注意控制频率。 + +### 4. 所需权限 + +| 身份 | 所需权限 | +|------|---------| +| user / bot | `minutes:minutes.media:export` | + +## 输出结果 + +### 下载模式（默认） + +```json +{ + "saved_path": "./meeting.mp4", + "size_bytes": 52428800 +} +``` + +| 字段 | 说明 | +|------|------| +| `saved_path` | 文件保存的本地路径 | +| `size_bytes` | 文件大小（字节） | + +### URL 模式（--url-only） + +```json +{ + "download_url": "https://..." +} +``` + +| 字段 | 说明 | +|------|------| +| `download_url` | 媒体文件下载链接（有效期 1 天） | + +## 如何获取 minute_token + +| 来源 | 获取方式 | +|------|---------| +| 妙记 URL | 从 URL 末尾提取，如 `https://sample.feishu.cn/minutes/obcnq3b9jl72l83w4f149w9c` → `obcnq3b9jl72l83w4f149w9c` | +| 妙记元信息查询 | `lark-cli minutes minutes get --params '{"minute_token": "obcn..."}'` | +| 会议纪要查询 | `lark-cli vc +notes --meeting-ids ` 返回结果中关联的妙记 token | + +## 常见错误与排查 + +| 错误现象 | 错误码 | 根本原因 | 解决方案 | +|---------|--------|---------|---------| +| 参数无效 | 2091001 | minute_token 格式不正确 | 检查 token 是否完整（24 位） | +| 资源不存在 | 2091002 | token 不存在 | 确认 minute_token 正确 | +| 妙记尚未准备好 | 2091003 | 转写未完成 | 等待转写完成后重试 | +| 资源已删除 | 2091004 | 妙记已被删除 | 确认妙记文件仍然存在 | +| 权限不足 | 2091005 | 无阅读权限 | 检查是否有该妙记的访问权限 | +| `missing required scope(s)` | — | 应用缺少权限 | 运行 `auth login --scope "minutes:minutes.media:export"` | + +## 提示 + +- 音视频文件可能较大，下载超时默认为 5 分钟。 +- 默认文件名 `.media` 不含真实扩展名，建议通过 `--output` 指定带扩展名的路径（如 `.mp4`）。 +- 如需获取妙记的纪要内容（逐字稿、AI 总结等），请使用 [vc +notes](../../lark-vc/references/lark-vc-notes.md)。 + +## 参考 + +- [lark-minutes](../SKILL.md) — 妙记全部命令 +- [lark-vc-notes](../../lark-vc/references/lark-vc-notes.md) — 会议纪要查询 +- [lark-shared](../../lark-shared/SKILL.md) — 认证和全局参数 From 808e5f6bfb83ab47e6c39a7451d4386477e25b20 Mon Sep 17 00:00:00 2001 From: renaocheng Date: Mon, 30 Mar 2026 18:35:50 +0800 Subject: [PATCH 02/17] chore: remove accidentally committed test artifacts from shortcuts/vc --- shortcuts/vc/artifact-Empty Artifacts-tok003/transcript.txt | 1 - shortcuts/vc/artifact-No Note Meeting-tok002/transcript.txt | 1 - shortcuts/vc/artifact-Test Minutes-tok001/transcript.txt | 1 - 3 files changed, 3 deletions(-) delete mode 100644 shortcuts/vc/artifact-Empty Artifacts-tok003/transcript.txt delete mode 100644 shortcuts/vc/artifact-No Note Meeting-tok002/transcript.txt delete mode 100644 shortcuts/vc/artifact-Test Minutes-tok001/transcript.txt diff --git a/shortcuts/vc/artifact-Empty Artifacts-tok003/transcript.txt b/shortcuts/vc/artifact-Empty Artifacts-tok003/transcript.txt deleted file mode 100644 index 97dd8118..00000000 --- a/shortcuts/vc/artifact-Empty Artifacts-tok003/transcript.txt +++ /dev/null @@ -1 +0,0 @@ -{"code":0,"data":{},"msg":"ok"} \ No newline at end of file diff --git a/shortcuts/vc/artifact-No Note Meeting-tok002/transcript.txt b/shortcuts/vc/artifact-No Note Meeting-tok002/transcript.txt deleted file mode 100644 index 97dd8118..00000000 --- a/shortcuts/vc/artifact-No Note Meeting-tok002/transcript.txt +++ /dev/null @@ -1 +0,0 @@ -{"code":0,"data":{},"msg":"ok"} \ No newline at end of file diff --git a/shortcuts/vc/artifact-Test Minutes-tok001/transcript.txt b/shortcuts/vc/artifact-Test Minutes-tok001/transcript.txt deleted file mode 100644 index 97dd8118..00000000 --- a/shortcuts/vc/artifact-Test Minutes-tok001/transcript.txt +++ /dev/null @@ -1 +0,0 @@ -{"code":0,"data":{},"msg":"ok"} \ No newline at end of file From aa1d9852757c9e135f2aa9ab1b8ae90bc98a600c Mon Sep 17 00:00:00 2001 From: renaocheng Date: Tue, 31 Mar 2026 14:44:03 +0800 Subject: [PATCH 03/17] feat: use minute title and auto-detected extension for default download filename --- shortcuts/minutes/minutes_download.go | 109 +++++++++++++++--- skills/lark-minutes/SKILL.md | 2 +- .../references/lark-minutes-download.md | 6 +- 3 files changed, 94 insertions(+), 23 deletions(-) diff --git a/shortcuts/minutes/minutes_download.go b/shortcuts/minutes/minutes_download.go index 8a5e2dfa..b655e446 100644 --- a/shortcuts/minutes/minutes_download.go +++ b/shortcuts/minutes/minutes_download.go @@ -20,6 +20,19 @@ import ( const defaultMediaDownloadTimeout = 300 * time.Second +// mimeToExt maps Content-Type to file extension. +var mimeToExt = map[string]string{ + "video/mp4": ".mp4", + "video/webm": ".webm", + "audio/mpeg": ".mp3", + "audio/mp4": ".m4a", + "audio/ogg": ".ogg", + "audio/wav": ".wav", + "application/pdf": ".pdf", + "application/zip": ".zip", + "application/gzip": ".gz", +} + var MinutesDownload = common.Shortcut{ Service: "minutes", Command: "+download", @@ -29,7 +42,7 @@ var MinutesDownload = common.Shortcut{ AuthTypes: []string{"user", "bot"}, Flags: []common.Flag{ {Name: "minute-token", Desc: "minute token (from the minutes URL)", Required: true}, - {Name: "output", Desc: "local save path (defaults to .media)"}, + {Name: "output", Desc: "local save path (defaults to .<ext> based on minute title and content type)"}, {Name: "overwrite", Type: "bool", Desc: "overwrite existing output file"}, {Name: "url-only", Type: "bool", Desc: "only print the download URL without downloading the file"}, }, @@ -53,7 +66,7 @@ var MinutesDownload = common.Shortcut{ return output.ErrValidation("%s", err) } - // 第一步：调用 API 获取下载链接 + // Step 1: get the download URL from the media API data, err := runtime.DoAPIJSON(http.MethodGet, fmt.Sprintf("/open-apis/minutes/v1/minutes/%s/media", validate.EncodePathSegment(minuteToken)), nil, nil) @@ -66,7 +79,7 @@ var MinutesDownload = common.Shortcut{ return output.Errorf(output.ExitAPI, "api_error", "API returned empty download_url") } - // --url-only 模式：仅输出下载链接 + // --url-only mode: print download URL only if urlOnly { runtime.Out(map[string]interface{}{ "download_url": downloadURL, @@ -74,9 +87,9 @@ var MinutesDownload = common.Shortcut{ return nil } - // 第二步：从下载链接下载文件 + // Step 2: resolve output path (use minute title when --output is not specified) if outputPath == "" { - outputPath = minuteToken + ".media" + outputPath = resolveDefaultOutputPath(ctx, runtime, minuteToken) } safePath, err := validate.SafeOutputPath(outputPath) if err != nil { @@ -88,57 +101,115 @@ var MinutesDownload = common.Shortcut{ fmt.Fprintf(runtime.IO().ErrOut, "Downloading media: %s\n", common.MaskToken(minuteToken)) - sizeBytes, err := downloadMediaFile(ctx, runtime, downloadURL, safePath) + sizeBytes, contentType, err := downloadMediaFile(ctx, runtime, downloadURL, safePath) if err != nil { return err } + // Auto-detect extension from Content-Type (only when --output is not specified) + finalPath := safePath + if runtime.Str("output") == "" { + if ext := extFromContentType(contentType); ext != "" && filepath.Ext(safePath) != ext { + newPath := strings.TrimSuffix(safePath, filepath.Ext(safePath)) + ext + if renameErr := os.Rename(safePath, newPath); renameErr == nil { + finalPath = newPath + } + } + } + runtime.Out(map[string]interface{}{ - "saved_path": safePath, + "saved_path": finalPath, "size_bytes": sizeBytes, }, nil) return nil }, } -// downloadMediaFile 从 pre-signed URL 流式下载媒体文件到本地 -func downloadMediaFile(ctx context.Context, runtime *common.RuntimeContext, downloadURL, safePath string) (int64, error) { +// resolveDefaultOutputPath fetches the minute title and uses it as the default file name. +// Falls back to <token>.media if the title cannot be retrieved. +func resolveDefaultOutputPath(ctx context.Context, runtime *common.RuntimeContext, minuteToken string) string { + infoData, err := runtime.DoAPIJSON(http.MethodGet, + fmt.Sprintf("/open-apis/minutes/v1/minutes/%s", validate.EncodePathSegment(minuteToken)), + nil, nil) + if err == nil { + if minute, ok := infoData["minute"].(map[string]interface{}); ok { + if title := common.GetString(minute, "title"); title != "" { + safe := sanitizeFileName(title) + if safe != "" { + return safe + ".media" + } + } + } + } + // fall back to token-based name + return minuteToken + ".media" +} + +// sanitizeFileName removes unsafe characters from a file name. +func sanitizeFileName(name string) string { + const maxLen = 200 + replacer := strings.NewReplacer( + "/", "_", "\\", "_", ":", "_", "*", "_", "?", "_", + "\"", "_", "<", "_", ">", "_", "|", "_", + "\n", "_", "\r", "_", "\t", "_", "\x00", "_", + ) + safe := replacer.Replace(strings.TrimSpace(name)) + safe = strings.Trim(safe, ".") + if len(safe) > maxLen { + safe = safe[:maxLen] + } + return safe +} + +// extFromContentType returns a file extension for the given Content-Type, or "" if unknown. +func extFromContentType(contentType string) string { + mimeType := strings.TrimSpace(strings.SplitN(contentType, ";", 2)[0]) + if ext, ok := mimeToExt[mimeType]; ok { + return ext + } + return "" +} + +// downloadMediaFile streams a media file from a pre-signed URL to disk. Returns size and Content-Type. +func downloadMediaFile(ctx context.Context, runtime *common.RuntimeContext, downloadURL, safePath string) (int64, string, error) { httpClient, err := runtime.Factory.HttpClient() if err != nil { - return 0, output.ErrNetwork("failed to get HTTP client: %s", err) + return 0, "", output.ErrNetwork("failed to get HTTP client: %s", err) } - // 复制 client 并覆盖超时，避免默认 30s 超时导致大文件下载失败 + // clone the client with a longer timeout for large media files downloadClient := *httpClient downloadClient.Timeout = defaultMediaDownloadTimeout req, err := http.NewRequestWithContext(ctx, http.MethodGet, downloadURL, nil) if err != nil { - return 0, output.ErrNetwork("invalid download URL: %s", err) + return 0, "", output.ErrNetwork("invalid download URL: %s", err) } - // 不发送 Authorization header，download_url 是 pre-signed URL + // no Authorization header — download_url is a pre-signed URL resp, err := downloadClient.Do(req) if err != nil { - return 0, output.ErrNetwork("download failed: %s", err) + return 0, "", output.ErrNetwork("download failed: %s", err) } defer resp.Body.Close() if resp.StatusCode >= 400 { body, _ := io.ReadAll(io.LimitReader(resp.Body, 4096)) if len(body) > 0 { - return 0, output.ErrNetwork("download failed: HTTP %d: %s", resp.StatusCode, strings.TrimSpace(string(body))) + return 0, "", output.ErrNetwork("download failed: HTTP %d: %s", resp.StatusCode, strings.TrimSpace(string(body))) } - return 0, output.ErrNetwork("download failed: HTTP %d", resp.StatusCode) + return 0, "", output.ErrNetwork("download failed: HTTP %d", resp.StatusCode) } + contentType := resp.Header.Get("Content-Type") + if err := os.MkdirAll(filepath.Dir(safePath), 0755); err != nil { - return 0, output.Errorf(output.ExitInternal, "api_error", "cannot create parent directory: %s", err) + return 0, "", output.Errorf(output.ExitInternal, "api_error", "cannot create parent directory: %s", err) } sizeBytes, err := validate.AtomicWriteFromReader(safePath, resp.Body, 0644) if err != nil { - return 0, output.Errorf(output.ExitInternal, "api_error", "cannot create file: %s", err) + return 0, "", output.Errorf(output.ExitInternal, "api_error", "cannot create file: %s", err) } - return sizeBytes, nil + return sizeBytes, contentType, nil } diff --git a/skills/lark-minutes/SKILL.md b/skills/lark-minutes/SKILL.md index 7cf250c1..f38075e4 100644 --- a/skills/lark-minutes/SKILL.md +++ b/skills/lark-minutes/SKILL.md @@ -87,7 +87,7 @@ lark-cli minutes <resource> <method> [flags] # 调用 API | 方法 | 所需 scope | |------|-----------| | `minutes.get` | `minutes:minutes:readonly` | -| `+download` | `minutes:minutes.media:export` | +| `+download` | `minutes:minutes.media:export`（自动命名还需 `minutes:minutes:readonly`，缺失时降级为 token 命名） | <!-- AUTO-GENERATED-END --> diff --git a/skills/lark-minutes/references/lark-minutes-download.md b/skills/lark-minutes/references/lark-minutes-download.md index af445723..32b4c285 100644 --- a/skills/lark-minutes/references/lark-minutes-download.md +++ b/skills/lark-minutes/references/lark-minutes-download.md @@ -28,7 +28,7 @@ lark-cli minutes +download --minute-token obcnq3b9jl72l83w4f149w9c --dry-run | 参数 | 必填 | 说明 | |------|------|------| | `--minute-token <token>` | 是 | 妙记 Token，从妙记 URL 末尾提取（24 位字符串） | -| `--output <path>` | 否 | 本地保存路径（默认 `<minute-token>.media`） | +| `--output <path>` | 否 | 本地保存路径（默认用妙记标题 + 自动推断扩展名，如 `访谈一则.mp4`） | | `--overwrite` | 否 | 覆盖已存在的输出文件 | | `--url-only` | 否 | 仅返回下载链接，不下载文件 | | `--dry-run` | 否 | 预览 API 调用，不执行 | @@ -59,7 +59,7 @@ API 限流 5 次/秒，批量下载时需注意控制频率。 ```json { - "saved_path": "./meeting.mp4", + "saved_path": "访谈一则 & 澳成.mp4", "size_bytes": 52428800 } ``` @@ -103,7 +103,7 @@ API 限流 5 次/秒，批量下载时需注意控制频率。 ## 提示 - 音视频文件可能较大，下载超时默认为 5 分钟。 -- 默认文件名 `<minute-token>.media` 不含真实扩展名，建议通过 `--output` 指定带扩展名的路径（如 `.mp4`）。 +- 默认文件名使用妙记标题，扩展名从下载响应的 Content-Type 自动推断（如 `video/mp4` → `.mp4`）。获取标题需要 `minutes:minutes:readonly` 或 `minutes:minutes.basic:read` scope，获取失败时降级为 `<minute-token>.media`。 - 如需获取妙记的纪要内容（逐字稿、AI 总结等），请使用 [vc +notes](../../lark-vc/references/lark-vc-notes.md)。 ## 参考 From 6b3bc0bc401b3e559c545c318df15bba5e7e5ab9 Mon Sep 17 00:00:00 2001 From: renaocheng <renaocheng@bytedance.com> Date: Tue, 31 Mar 2026 14:44:25 +0800 Subject: [PATCH 04/17] docs: clarify note_doc_token vs verbatim_doc_token and add cover image guidance --- skills/lark-vc/SKILL.md | 15 +++++++++++++++ skills/lark-vc/references/lark-vc-notes.md | 6 ++++-- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/skills/lark-vc/SKILL.md b/skills/lark-vc/SKILL.md index 9a540972..c210d860 100644 --- a/skills/lark-vc/SKILL.md +++ b/skills/lark-vc/SKILL.md @@ -31,6 +31,21 @@ metadata: ### 2. 整理会议纪要 1. 整理纪要文档时默认给出纪要文档和逐字稿链接即可，无需读取纪要文档或逐字稿内容。 2. 用户明确需要获取纪要文档中的总结、待办、章节产物时，再读取文档获取具体内容。 +3. 读取智能纪要（`note_doc_token`）内容时，纪要文档的**第一个 `<whiteboard>`** 标签是封面图（AI 生成的总结可视化），应同时下载展示给用户： +```bash +# 1. 读取纪要内容 +lark-cli docs +fetch --doc <note_doc_token> +# 2. 从返回的 markdown 中提取第一个 <whiteboard token="xxx"/> 的 token +# 3. 下载封面图到 artifact 目录（和逐字稿同目录，保持产物归拢） +# 并非所有纪要都有封面画板，没有 <whiteboard> 标签时跳过即可 +lark-cli docs +media-download --type whiteboard --token <whiteboard_token> --output ./artifact-<title>/cover +``` +> **产物目录规范**：同一会议的所有下载产物（封面图、逐字稿等）统一放到 `artifact-<title>/` 目录下，不要散落在当前工作目录。 + +> **`note_doc_token` vs `verbatim_doc_token` — 两份不同的文档，根据用户意图选择：** +> - `note_doc_token` → **智能纪要**（AI 总结 + 待办 + 章节）— 用户说"纪要""总结""待办""纪要内容"时用这个 +> - `verbatim_doc_token` → **逐字稿**（完整的逐句文字记录，含说话人和时间戳）— 用户说"逐字稿""完整记录""谁说了什么"时用这个 +> - 用户意图不明确时，应展示两个文档链接让用户选择，而不是替用户决定 ### 3. 纪要文档与逐字稿链接 1. 纪要文档、逐字稿文档与关联的共享文档默认使用文档 Token 返回。 diff --git a/skills/lark-vc/references/lark-vc-notes.md b/skills/lark-vc/references/lark-vc-notes.md index 06147d4d..53aeb880 100644 --- a/skills/lark-vc/references/lark-vc-notes.md +++ b/skills/lark-vc/references/lark-vc-notes.md @@ -75,12 +75,14 @@ lark-cli vc +notes --meeting-ids 69xxxxxxxxxxxxx28 --dry-run | 字段 | 说明 | |------|------| -| `note_doc_token` | 主纪要文档 Token | -| `verbatim_doc_token` | 逐字稿文档 Token | +| `note_doc_token` | **智能纪要**文档 Token — 包含 AI 总结、待办、章节（用户说"纪要"时用这个） | +| `verbatim_doc_token` | **逐字稿**文档 Token — 完整的逐句文字记录，含说话人和时间戳（用户说"逐字稿"时才用这个） | | `shared_doc_tokens` | 会中共享文档 Token 列表 | | `creator_id` | 创建者 ID | | `create_time` | 创建时间（格式化） | +> **选择哪个 token？** 用户说"会议纪要""总结""待办""纪要内容" → 用 `note_doc_token`。用户说"逐字稿""完整记录""谁说了什么" → 用 `verbatim_doc_token`。意图不明确时，展示两个文档链接让用户选择。 + ### minute-tokens 路径的 AI 产物 通过 `--minute-tokens` 查询时，返回的 `artifacts` 字段包含 AI 内置产物： From d6db1f03dfd94bb38b6e5772f86f127c3501c4ad Mon Sep 17 00:00:00 2001 From: renaocheng <renaocheng@bytedance.com> Date: Tue, 31 Mar 2026 15:48:29 +0800 Subject: [PATCH 05/17] refactor: resolve default filename from Content-Disposition instead of extra API call --- shortcuts/minutes/minutes_download.go | 151 +++++++----------- skills/lark-minutes/SKILL.md | 2 +- .../references/lark-minutes-download.md | 4 +- 3 files changed, 60 insertions(+), 97 deletions(-) diff --git a/shortcuts/minutes/minutes_download.go b/shortcuts/minutes/minutes_download.go index b655e446..f4e4baa7 100644 --- a/shortcuts/minutes/minutes_download.go +++ b/shortcuts/minutes/minutes_download.go @@ -7,6 +7,7 @@ import ( "context" "fmt" "io" + "mime" "net/http" "os" "path/filepath" @@ -20,19 +21,6 @@ import ( const defaultMediaDownloadTimeout = 300 * time.Second -// mimeToExt maps Content-Type to file extension. -var mimeToExt = map[string]string{ - "video/mp4": ".mp4", - "video/webm": ".webm", - "audio/mpeg": ".mp3", - "audio/mp4": ".m4a", - "audio/ogg": ".ogg", - "audio/wav": ".wav", - "application/pdf": ".pdf", - "application/zip": ".zip", - "application/gzip": ".gz", -} - var MinutesDownload = common.Shortcut{ Service: "minutes", Command: "+download", @@ -42,7 +30,7 @@ var MinutesDownload = common.Shortcut{ AuthTypes: []string{"user", "bot"}, Flags: []common.Flag{ {Name: "minute-token", Desc: "minute token (from the minutes URL)", Required: true}, - {Name: "output", Desc: "local save path (defaults to <title>.<ext> based on minute title and content type)"}, + {Name: "output", Desc: "local save path (defaults to original filename from server response)"}, {Name: "overwrite", Type: "bool", Desc: "overwrite existing output file"}, {Name: "url-only", Type: "bool", Desc: "only print the download URL without downloading the file"}, }, @@ -87,94 +75,34 @@ var MinutesDownload = common.Shortcut{ return nil } - // Step 2: resolve output path (use minute title when --output is not specified) - if outputPath == "" { - outputPath = resolveDefaultOutputPath(ctx, runtime, minuteToken) - } - safePath, err := validate.SafeOutputPath(outputPath) - if err != nil { - return output.ErrValidation("unsafe output path: %s", err) - } - if err := common.EnsureWritableFile(safePath, overwrite); err != nil { - return err - } - + // Step 2: download the file and resolve the output path fmt.Fprintf(runtime.IO().ErrOut, "Downloading media: %s\n", common.MaskToken(minuteToken)) - sizeBytes, contentType, err := downloadMediaFile(ctx, runtime, downloadURL, safePath) + result, err := downloadMediaFile(ctx, runtime, downloadURL, outputPath, minuteToken, overwrite) if err != nil { return err } - // Auto-detect extension from Content-Type (only when --output is not specified) - finalPath := safePath - if runtime.Str("output") == "" { - if ext := extFromContentType(contentType); ext != "" && filepath.Ext(safePath) != ext { - newPath := strings.TrimSuffix(safePath, filepath.Ext(safePath)) + ext - if renameErr := os.Rename(safePath, newPath); renameErr == nil { - finalPath = newPath - } - } - } - runtime.Out(map[string]interface{}{ - "saved_path": finalPath, - "size_bytes": sizeBytes, + "saved_path": result.savedPath, + "size_bytes": result.sizeBytes, }, nil) return nil }, } -// resolveDefaultOutputPath fetches the minute title and uses it as the default file name. -// Falls back to <token>.media if the title cannot be retrieved. -func resolveDefaultOutputPath(ctx context.Context, runtime *common.RuntimeContext, minuteToken string) string { - infoData, err := runtime.DoAPIJSON(http.MethodGet, - fmt.Sprintf("/open-apis/minutes/v1/minutes/%s", validate.EncodePathSegment(minuteToken)), - nil, nil) - if err == nil { - if minute, ok := infoData["minute"].(map[string]interface{}); ok { - if title := common.GetString(minute, "title"); title != "" { - safe := sanitizeFileName(title) - if safe != "" { - return safe + ".media" - } - } - } - } - // fall back to token-based name - return minuteToken + ".media" -} - -// sanitizeFileName removes unsafe characters from a file name. -func sanitizeFileName(name string) string { - const maxLen = 200 - replacer := strings.NewReplacer( - "/", "_", "\\", "_", ":", "_", "*", "_", "?", "_", - "\"", "_", "<", "_", ">", "_", "|", "_", - "\n", "_", "\r", "_", "\t", "_", "\x00", "_", - ) - safe := replacer.Replace(strings.TrimSpace(name)) - safe = strings.Trim(safe, ".") - if len(safe) > maxLen { - safe = safe[:maxLen] - } - return safe -} - -// extFromContentType returns a file extension for the given Content-Type, or "" if unknown. -func extFromContentType(contentType string) string { - mimeType := strings.TrimSpace(strings.SplitN(contentType, ";", 2)[0]) - if ext, ok := mimeToExt[mimeType]; ok { - return ext - } - return "" +type downloadResult struct { + savedPath string + sizeBytes int64 } -// downloadMediaFile streams a media file from a pre-signed URL to disk. Returns size and Content-Type. -func downloadMediaFile(ctx context.Context, runtime *common.RuntimeContext, downloadURL, safePath string) (int64, string, error) { +// downloadMediaFile streams a media file from a pre-signed URL to disk. +// When outputPath is empty, it resolves the filename from the Content-Disposition +// header, falling back to Content-Type extension detection, then <token>.media. +func downloadMediaFile(ctx context.Context, runtime *common.RuntimeContext, downloadURL, outputPath, minuteToken string, overwrite bool) (*downloadResult, error) { httpClient, err := runtime.Factory.HttpClient() if err != nil { - return 0, "", output.ErrNetwork("failed to get HTTP client: %s", err) + return nil, output.ErrNetwork("failed to get HTTP client: %s", err) } // clone the client with a longer timeout for large media files @@ -183,33 +111,68 @@ func downloadMediaFile(ctx context.Context, runtime *common.RuntimeContext, down req, err := http.NewRequestWithContext(ctx, http.MethodGet, downloadURL, nil) if err != nil { - return 0, "", output.ErrNetwork("invalid download URL: %s", err) + return nil, output.ErrNetwork("invalid download URL: %s", err) } // no Authorization header — download_url is a pre-signed URL resp, err := downloadClient.Do(req) if err != nil { - return 0, "", output.ErrNetwork("download failed: %s", err) + return nil, output.ErrNetwork("download failed: %s", err) } defer resp.Body.Close() if resp.StatusCode >= 400 { body, _ := io.ReadAll(io.LimitReader(resp.Body, 4096)) if len(body) > 0 { - return 0, "", output.ErrNetwork("download failed: HTTP %d: %s", resp.StatusCode, strings.TrimSpace(string(body))) + return nil, output.ErrNetwork("download failed: HTTP %d: %s", resp.StatusCode, strings.TrimSpace(string(body))) } - return 0, "", output.ErrNetwork("download failed: HTTP %d", resp.StatusCode) + return nil, output.ErrNetwork("download failed: HTTP %d", resp.StatusCode) + } + + // resolve output path from response headers when --output is not specified + if outputPath == "" { + outputPath = resolveOutputFromResponse(resp, minuteToken) } - contentType := resp.Header.Get("Content-Type") + safePath, err := validate.SafeOutputPath(outputPath) + if err != nil { + return nil, output.ErrValidation("unsafe output path: %s", err) + } + if err := common.EnsureWritableFile(safePath, overwrite); err != nil { + return nil, err + } if err := os.MkdirAll(filepath.Dir(safePath), 0755); err != nil { - return 0, "", output.Errorf(output.ExitInternal, "api_error", "cannot create parent directory: %s", err) + return nil, output.Errorf(output.ExitInternal, "api_error", "cannot create parent directory: %s", err) } sizeBytes, err := validate.AtomicWriteFromReader(safePath, resp.Body, 0644) if err != nil { - return 0, "", output.Errorf(output.ExitInternal, "api_error", "cannot create file: %s", err) + return nil, output.Errorf(output.ExitInternal, "api_error", "cannot create file: %s", err) } - return sizeBytes, contentType, nil + return &downloadResult{savedPath: safePath, sizeBytes: sizeBytes}, nil +} + +// resolveOutputFromResponse derives the output filename from HTTP response headers. +// Priority: Content-Disposition filename > Content-Type extension > fallback to <token>.media. +func resolveOutputFromResponse(resp *http.Response, minuteToken string) string { + // try Content-Disposition header for the original filename + if cd := resp.Header.Get("Content-Disposition"); cd != "" { + if _, params, err := mime.ParseMediaType(cd); err == nil { + if filename := params["filename"]; filename != "" { + return filename + } + } + } + + // fall back to Content-Type extension detection + if ct := resp.Header.Get("Content-Type"); ct != "" { + if mediaType, _, err := mime.ParseMediaType(ct); err == nil { + if exts, err := mime.ExtensionsByType(mediaType); err == nil && len(exts) > 0 { + return minuteToken + exts[0] + } + } + } + + return minuteToken + ".media" } diff --git a/skills/lark-minutes/SKILL.md b/skills/lark-minutes/SKILL.md index f38075e4..7cf250c1 100644 --- a/skills/lark-minutes/SKILL.md +++ b/skills/lark-minutes/SKILL.md @@ -87,7 +87,7 @@ lark-cli minutes <resource> <method> [flags] # 调用 API | 方法 | 所需 scope | |------|-----------| | `minutes.get` | `minutes:minutes:readonly` | -| `+download` | `minutes:minutes.media:export`（自动命名还需 `minutes:minutes:readonly`，缺失时降级为 token 命名） | +| `+download` | `minutes:minutes.media:export` | <!-- AUTO-GENERATED-END --> diff --git a/skills/lark-minutes/references/lark-minutes-download.md b/skills/lark-minutes/references/lark-minutes-download.md index 32b4c285..a3fecc04 100644 --- a/skills/lark-minutes/references/lark-minutes-download.md +++ b/skills/lark-minutes/references/lark-minutes-download.md @@ -28,7 +28,7 @@ lark-cli minutes +download --minute-token obcnq3b9jl72l83w4f149w9c --dry-run | 参数 | 必填 | 说明 | |------|------|------| | `--minute-token <token>` | 是 | 妙记 Token，从妙记 URL 末尾提取（24 位字符串） | -| `--output <path>` | 否 | 本地保存路径（默认用妙记标题 + 自动推断扩展名，如 `访谈一则.mp4`） | +| `--output <path>` | 否 | 本地保存路径（默认使用妙记原始标题，如 `Office Oncall流程2.0宣讲.mp4`） | | `--overwrite` | 否 | 覆盖已存在的输出文件 | | `--url-only` | 否 | 仅返回下载链接，不下载文件 | | `--dry-run` | 否 | 预览 API 调用，不执行 | @@ -103,7 +103,7 @@ API 限流 5 次/秒，批量下载时需注意控制频率。 ## 提示 - 音视频文件可能较大，下载超时默认为 5 分钟。 -- 默认文件名使用妙记标题，扩展名从下载响应的 Content-Type 自动推断（如 `video/mp4` → `.mp4`）。获取标题需要 `minutes:minutes:readonly` 或 `minutes:minutes.basic:read` scope，获取失败时降级为 `<minute-token>.media`。 +- 未指定 `--output` 时，默认使用妙记原始标题作为文件名（如 `Office Oncall流程2.0宣讲.mp4`）。 - 如需获取妙记的纪要内容（逐字稿、AI 总结等），请使用 [vc +notes](../../lark-vc/references/lark-vc-notes.md)。 ## 参考 From 52cf828ed4e6c4141ae89fe9a55ff2977b653e5d Mon Sep 17 00:00:00 2001 From: renaocheng <renaocheng@bytedance.com> Date: Tue, 31 Mar 2026 16:49:05 +0800 Subject: [PATCH 06/17] test: add unit and integration tests for minutes +download shortcut --- shortcuts/minutes/minutes_download_test.go | 293 +++++++++++++++++++++ 1 file changed, 293 insertions(+) create mode 100644 shortcuts/minutes/minutes_download_test.go diff --git a/shortcuts/minutes/minutes_download_test.go b/shortcuts/minutes/minutes_download_test.go new file mode 100644 index 00000000..a225bc93 --- /dev/null +++ b/shortcuts/minutes/minutes_download_test.go @@ -0,0 +1,293 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package minutes + +import ( + "bytes" + "context" + "net/http" + "os" + "strings" + "sync" + "testing" + + "github.com/spf13/cobra" + + "github.com/larksuite/cli/internal/cmdutil" + "github.com/larksuite/cli/internal/core" + "github.com/larksuite/cli/internal/httpmock" + "github.com/larksuite/cli/shortcuts/common" +) + +// --------------------------------------------------------------------------- +// helpers +// --------------------------------------------------------------------------- + +var warmOnce sync.Once + +func warmTokenCache(t *testing.T) { + t.Helper() + warmOnce.Do(func() { + f, _, _, reg := cmdutil.TestFactory(t, defaultConfig()) + reg.Register(&httpmock.Stub{ + URL: "/open-apis/auth/v3/tenant_access_token/internal", + Body: map[string]interface{}{ + "code": 0, "msg": "ok", + "tenant_access_token": "t-test-token", "expire": 7200, + }, + }) + reg.Register(&httpmock.Stub{ + URL: "/open-apis/test/v1/warm", + Body: map[string]interface{}{"code": 0, "msg": "ok", "data": map[string]interface{}{}}, + }) + s := common.Shortcut{ + Service: "test", + Command: "+warm", + AuthTypes: []string{"bot"}, + Execute: func(_ context.Context, rctx *common.RuntimeContext) error { + _, err := rctx.CallAPI("GET", "/open-apis/test/v1/warm", nil, nil) + return err + }, + } + parent := &cobra.Command{Use: "test"} + s.Mount(parent, f) + parent.SetArgs([]string{"+warm"}) + parent.SilenceErrors = true + parent.SilenceUsage = true + parent.Execute() + }) +} + +func mountAndRun(t *testing.T, s common.Shortcut, args []string, f *cmdutil.Factory, stdout *bytes.Buffer) error { + t.Helper() + warmTokenCache(t) + parent := &cobra.Command{Use: "minutes"} + s.Mount(parent, f) + parent.SetArgs(args) + parent.SilenceErrors = true + parent.SilenceUsage = true + if stdout != nil { + stdout.Reset() + } + return parent.Execute() +} + +func defaultConfig() *core.CliConfig { + return &core.CliConfig{ + AppID: "test-app", AppSecret: "test-secret", Brand: core.BrandFeishu, + UserOpenId: "ou_testuser", + } +} + +func mediaStub(token, downloadURL string) *httpmock.Stub { + return &httpmock.Stub{ + Method: "GET", + URL: "/open-apis/minutes/v1/minutes/" + token + "/media", + Body: map[string]interface{}{ + "code": 0, "msg": "ok", + "data": map[string]interface{}{"download_url": downloadURL}, + }, + } +} + +func downloadStub(url string, body []byte, contentType string) *httpmock.Stub { + return &httpmock.Stub{ + URL: url, + RawBody: body, + Headers: http.Header{"Content-Type": []string{contentType}}, + } +} + +// chdir 切换工作目录并在测试结束后恢复 +func chdir(t *testing.T, dir string) { + t.Helper() + orig, err := os.Getwd() + if err != nil { + t.Fatalf("failed to get cwd: %v", err) + } + if err := os.Chdir(dir); err != nil { + t.Fatalf("failed to chdir to %s: %v", dir, err) + } + t.Cleanup(func() { os.Chdir(orig) }) +} + +// --------------------------------------------------------------------------- +// Unit tests: resolveOutputFromResponse +// --------------------------------------------------------------------------- + +func TestResolveOutputFromResponse_ContentDisposition(t *testing.T) { + resp := &http.Response{ + Header: http.Header{ + "Content-Disposition": []string{`attachment; filename="meeting_recording.mp4"`}, + "Content-Type": []string{"video/mp4"}, + }, + } + got := resolveOutputFromResponse(resp, "tok001") + if got != "meeting_recording.mp4" { + t.Errorf("expected Content-Disposition filename, got %q", got) + } +} + +func TestResolveOutputFromResponse_ContentType(t *testing.T) { + resp := &http.Response{ + Header: http.Header{ + "Content-Type": []string{"video/mp4"}, + }, + } + got := resolveOutputFromResponse(resp, "tok001") + if !strings.HasPrefix(got, "tok001") { + t.Errorf("expected token prefix, got %q", got) + } + if filepath := got[len("tok001"):]; filepath == "" { + t.Errorf("expected extension after token, got %q", got) + } +} + +func TestResolveOutputFromResponse_Fallback(t *testing.T) { + resp := &http.Response{Header: http.Header{}} + got := resolveOutputFromResponse(resp, "tok001") + if got != "tok001.media" { + t.Errorf("expected fallback %q, got %q", "tok001.media", got) + } +} + +func TestResolveOutputFromResponse_InvalidContentDisposition(t *testing.T) { + resp := &http.Response{ + Header: http.Header{ + "Content-Disposition": []string{"invalid;;;"}, + "Content-Type": []string{"audio/mpeg"}, + }, + } + got := resolveOutputFromResponse(resp, "tok001") + // Content-Disposition 解析失败，应 fall back 到 Content-Type + if !strings.HasPrefix(got, "tok001") { + t.Errorf("expected token prefix from Content-Type fallback, got %q", got) + } +} + +func TestResolveOutputFromResponse_EmptyDispositionFilename(t *testing.T) { + resp := &http.Response{ + Header: http.Header{ + "Content-Disposition": []string{"attachment"}, + "Content-Type": []string{"video/mp4"}, + }, + } + got := resolveOutputFromResponse(resp, "tok001") + if got == "" { + t.Error("expected non-empty filename") + } + // Content-Disposition 没有 filename，应 fall back 到 Content-Type + if !strings.HasPrefix(got, "tok001") { + t.Errorf("expected token prefix, got %q", got) + } +} + +// --------------------------------------------------------------------------- +// Integration tests: +download with mocked HTTP +// --------------------------------------------------------------------------- + +func TestDownload_DryRun(t *testing.T) { + f, stdout, _, _ := cmdutil.TestFactory(t, defaultConfig()) + err := mountAndRun(t, MinutesDownload, []string{ + "+download", "--minute-token", "tok001", "--dry-run", "--as", "user", + }, f, stdout) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + out := stdout.String() + if !strings.Contains(out, "media") { + t.Errorf("dry-run should show media API path, got: %s", out) + } + if !strings.Contains(out, "tok001") { + t.Errorf("dry-run should show minute_token, got: %s", out) + } +} + +func TestDownload_UrlOnly(t *testing.T) { + f, stdout, _, reg := cmdutil.TestFactory(t, defaultConfig()) + reg.Register(mediaStub("tok001", "https://example.com/presigned/download")) + + err := mountAndRun(t, MinutesDownload, []string{ + "+download", "--minute-token", "tok001", "--url-only", "--as", "bot", + }, f, stdout) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !strings.Contains(stdout.String(), "https://example.com/presigned/download") { + t.Errorf("url-only should output download URL, got: %s", stdout.String()) + } +} + +func TestDownload_FullDownload(t *testing.T) { + chdir(t, t.TempDir()) + + f, stdout, _, reg := cmdutil.TestFactory(t, defaultConfig()) + reg.Register(mediaStub("tok001", "https://example.com/presigned/download")) + reg.Register(downloadStub("example.com/presigned/download", []byte("fake-video-content"), "video/mp4")) + + err := mountAndRun(t, MinutesDownload, []string{ + "+download", "--minute-token", "tok001", "--output", "output.mp4", "--as", "bot", + }, f, stdout) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + data, err := os.ReadFile("output.mp4") + if err != nil { + t.Fatalf("failed to read output file: %v", err) + } + if string(data) != "fake-video-content" { + t.Errorf("file content = %q, want %q", string(data), "fake-video-content") + } + if !strings.Contains(stdout.String(), "saved_path") { + t.Errorf("output should contain saved_path, got: %s", stdout.String()) + } +} + +func TestDownload_OverwriteProtection(t *testing.T) { + chdir(t, t.TempDir()) + os.WriteFile("existing.mp4", []byte("old"), 0644) + + f, _, _, reg := cmdutil.TestFactory(t, defaultConfig()) + reg.Register(mediaStub("tok001", "https://example.com/presigned/download")) + reg.Register(downloadStub("example.com/presigned/download", []byte("new-content"), "video/mp4")) + + err := mountAndRun(t, MinutesDownload, []string{ + "+download", "--minute-token", "tok001", "--output", "existing.mp4", "--as", "bot", + }, f, nil) + if err == nil { + t.Fatal("expected error for existing file without --overwrite") + } + if !strings.Contains(err.Error(), "exists") { + t.Errorf("error should mention file exists, got: %v", err) + } + + // 原文件不应被覆盖 + data, _ := os.ReadFile("existing.mp4") + if string(data) != "old" { + t.Errorf("original file should be preserved, got %q", string(data)) + } +} + +func TestDownload_HttpError(t *testing.T) { + chdir(t, t.TempDir()) + + f, _, _, reg := cmdutil.TestFactory(t, defaultConfig()) + reg.Register(mediaStub("tok001", "https://example.com/presigned/download")) + reg.Register(&httpmock.Stub{ + URL: "example.com/presigned/download", + Status: 403, + RawBody: []byte("Forbidden"), + }) + + err := mountAndRun(t, MinutesDownload, []string{ + "+download", "--minute-token", "tok001", "--output", "output.mp4", "--as", "bot", + }, f, nil) + if err == nil { + t.Fatal("expected error for HTTP 403") + } + if !strings.Contains(err.Error(), "403") { + t.Errorf("error should contain status code, got: %v", err) + } +} From c95b224ea1d3bc9fdfe495d3191eb822ec558c28 Mon Sep 17 00:00:00 2001 From: renaocheng <renaocheng@bytedance.com> Date: Tue, 31 Mar 2026 17:21:37 +0800 Subject: [PATCH 07/17] fix: add SSRF protection and redirect safety for media download --- shortcuts/minutes/minutes_download.go | 35 ++++++++++++++++++++++++--- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/shortcuts/minutes/minutes_download.go b/shortcuts/minutes/minutes_download.go index f4e4baa7..9f068472 100644 --- a/shortcuts/minutes/minutes_download.go +++ b/shortcuts/minutes/minutes_download.go @@ -12,14 +12,15 @@ import ( "os" "path/filepath" "strings" - "time" "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" ) -const defaultMediaDownloadTimeout = 300 * time.Second +// disableClientTimeout removes the global 30s client timeout for large media downloads. +// The request is still bounded by the caller's context. +const disableClientTimeout = 0 var MinutesDownload = common.Shortcut{ Service: "minutes", @@ -100,14 +101,20 @@ type downloadResult struct { // When outputPath is empty, it resolves the filename from the Content-Disposition // header, falling back to Content-Type extension detection, then <token>.media. func downloadMediaFile(ctx context.Context, runtime *common.RuntimeContext, downloadURL, outputPath, minuteToken string, overwrite bool) (*downloadResult, error) { + // SSRF: validate download URL before making any request + if err := validate.ValidateDownloadSourceURL(ctx, downloadURL); err != nil { + return nil, output.ErrValidation("blocked download URL: %s", err) + } + httpClient, err := runtime.Factory.HttpClient() if err != nil { return nil, output.ErrNetwork("failed to get HTTP client: %s", err) } - // clone the client with a longer timeout for large media files + // clone client: disable timeout for large files, add redirect safety policy downloadClient := *httpClient - downloadClient.Timeout = defaultMediaDownloadTimeout + downloadClient.Timeout = disableClientTimeout + downloadClient.CheckRedirect = safeRedirectPolicy req, err := http.NewRequestWithContext(ctx, http.MethodGet, downloadURL, nil) if err != nil { @@ -176,3 +183,23 @@ func resolveOutputFromResponse(resp *http.Response, minuteToken string) string { return minuteToken + ".media" } + +const maxDownloadRedirects = 5 + +// safeRedirectPolicy prevents HTTPS→HTTP downgrade, limits redirect count, +// and validates each redirect target against SSRF rules. +func safeRedirectPolicy(req *http.Request, via []*http.Request) error { + if len(via) >= maxDownloadRedirects { + return fmt.Errorf("too many redirects") + } + if len(via) > 0 { + prev := via[len(via)-1] + if strings.EqualFold(prev.URL.Scheme, "https") && strings.EqualFold(req.URL.Scheme, "http") { + return fmt.Errorf("redirect from https to http is not allowed") + } + } + if err := validate.ValidateDownloadSourceURL(req.Context(), req.URL.String()); err != nil { + return fmt.Errorf("blocked redirect target: %w", err) + } + return nil +} From 1434ef1f575d497cc6ac4b8acd0a167c99346e5a Mon Sep 17 00:00:00 2001 From: renaocheng <renaocheng@bytedance.com> Date: Tue, 31 Mar 2026 17:44:57 +0800 Subject: [PATCH 08/17] feat: add batch download with concurrent execution and SSRF protection --- shortcuts/minutes/minutes_download.go | 314 +++++++++++++++++---- shortcuts/minutes/minutes_download_test.go | 197 ++++++++++++- 2 files changed, 447 insertions(+), 64 deletions(-) diff --git a/shortcuts/minutes/minutes_download.go b/shortcuts/minutes/minutes_download.go index 9f068472..b4f04db9 100644 --- a/shortcuts/minutes/minutes_download.go +++ b/shortcuts/minutes/minutes_download.go @@ -12,15 +12,25 @@ import ( "os" "path/filepath" "strings" + "sync" + + "golang.org/x/sync/errgroup" "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" ) -// disableClientTimeout removes the global 30s client timeout for large media downloads. -// The request is still bounded by the caller's context. -const disableClientTimeout = 0 +const ( + // disableClientTimeout removes the global 30s client timeout for large media downloads. + // The request is still bounded by the caller's context. + disableClientTimeout = 0 + + maxBatchSize = 50 + maxConcurrentDownloads = 3 + maxDownloadRedirects = 5 + tokenSuffixLen = 6 +) var MinutesDownload = common.Shortcut{ Service: "minutes", @@ -29,67 +39,251 @@ var MinutesDownload = common.Shortcut{ Risk: "read", Scopes: []string{"minutes:minutes.media:export"}, AuthTypes: []string{"user", "bot"}, + HasFormat: true, Flags: []common.Flag{ - {Name: "minute-token", Desc: "minute token (from the minutes URL)", Required: true}, - {Name: "output", Desc: "local save path (defaults to original filename from server response)"}, + {Name: "minute-token", Desc: "single minute token (mutually exclusive with --minute-tokens)"}, + {Name: "minute-tokens", Desc: "comma-separated minute tokens for batch download (max 50)"}, + {Name: "output", Desc: "local save path (single mode only)"}, + {Name: "output-dir", Desc: "output directory for batch download (default: current dir)"}, {Name: "overwrite", Type: "bool", Desc: "overwrite existing output file"}, - {Name: "url-only", Type: "bool", Desc: "only print the download URL without downloading the file"}, + {Name: "url-only", Type: "bool", Desc: "only print the download URL(s) without downloading"}, + }, + Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { + single := runtime.Str("minute-token") + batch := runtime.Str("minute-tokens") + + if single == "" && batch == "" { + return output.ErrValidation("one of --minute-token or --minute-tokens is required") + } + if single != "" && batch != "" { + return output.ErrValidation("--minute-token and --minute-tokens are mutually exclusive") + } + + if batch != "" { + tokens := common.SplitCSV(batch) + if len(tokens) > maxBatchSize { + return output.ErrValidation("--minute-tokens: too many tokens (%d), maximum is %d", len(tokens), maxBatchSize) + } + if runtime.Str("output") != "" { + return output.ErrValidation("--output cannot be used with --minute-tokens; use --output-dir instead") + } + } + + if outDir := runtime.Str("output-dir"); outDir != "" { + if err := common.ValidateSafeOutputDir(outDir); err != nil { + return err + } + } + return nil }, DryRun: func(ctx context.Context, runtime *common.RuntimeContext) *common.DryRunAPI { - minuteToken := runtime.Str("minute-token") - outputPath := runtime.Str("output") - if outputPath == "" { - outputPath = minuteToken + ".media" - } - return common.NewDryRunAPI(). - GET("/open-apis/minutes/v1/minutes/:minute_token/media"). - Set("minute_token", minuteToken).Set("output", outputPath) + api := common.NewDryRunAPI(). + GET("/open-apis/minutes/v1/minutes/:minute_token/media") + + if token := runtime.Str("minute-token"); token != "" { + api.Set("minute_token", token) + } + if tokens := runtime.Str("minute-tokens"); tokens != "" { + api.Set("minute_tokens", common.SplitCSV(tokens)) + api.Set("concurrent_downloads", maxConcurrentDownloads) + } + return api }, Execute: func(ctx context.Context, runtime *common.RuntimeContext) error { - minuteToken := runtime.Str("minute-token") - outputPath := runtime.Str("output") - overwrite := runtime.Bool("overwrite") - urlOnly := runtime.Bool("url-only") - - if err := validate.ResourceName(minuteToken, "--minute-token"); err != nil { - return output.ErrValidation("%s", err) + if tokens := runtime.Str("minute-tokens"); tokens != "" { + return executeBatch(ctx, runtime) } + return executeSingle(ctx, runtime) + }, +} - // Step 1: get the download URL from the media API - data, err := runtime.DoAPIJSON(http.MethodGet, - fmt.Sprintf("/open-apis/minutes/v1/minutes/%s/media", validate.EncodePathSegment(minuteToken)), - nil, nil) - if err != nil { +// executeSingle handles the single --minute-token mode. +func executeSingle(ctx context.Context, runtime *common.RuntimeContext) error { + minuteToken := runtime.Str("minute-token") + outputPath := runtime.Str("output") + overwrite := runtime.Bool("overwrite") + urlOnly := runtime.Bool("url-only") + + if err := validate.ResourceName(minuteToken, "--minute-token"); err != nil { + return output.ErrValidation("%s", err) + } + + downloadURL, err := fetchDownloadURL(ctx, runtime, minuteToken) + if err != nil { + return err + } + + if urlOnly { + runtime.Out(map[string]interface{}{ + "download_url": downloadURL, + }, nil) + return nil + } + + fmt.Fprintf(runtime.IO().ErrOut, "Downloading media: %s\n", common.MaskToken(minuteToken)) + + result, err := downloadMediaFile(ctx, runtime, downloadURL, minuteToken, downloadOpts{ + outputPath: outputPath, + overwrite: overwrite, + }) + if err != nil { + return err + } + + runtime.Out(map[string]interface{}{ + "saved_path": result.savedPath, + "size_bytes": result.sizeBytes, + }, nil) + return nil +} + +// executeBatch handles the batch --minute-tokens mode. +// Phase 1: sequentially fetch download URLs (RuntimeContext is not concurrency-safe). +// Phase 2: concurrently download files (HTTP client is concurrency-safe). +func executeBatch(ctx context.Context, runtime *common.RuntimeContext) error { + tokens := common.SplitCSV(runtime.Str("minute-tokens")) + overwrite := runtime.Bool("overwrite") + urlOnly := runtime.Bool("url-only") + outputDir := runtime.Str("output-dir") + errOut := runtime.IO().ErrOut + + fmt.Fprintf(errOut, "[minutes +download] batch: %d token(s), concurrency=%d\n", len(tokens), maxConcurrentDownloads) + + type batchResult struct { + MinuteToken string `json:"minute_token"` + SavedPath string `json:"saved_path,omitempty"` + SizeBytes int64 `json:"size_bytes,omitempty"` + DownloadURL string `json:"download_url,omitempty"` + Error string `json:"error,omitempty"` + } + + results := make([]batchResult, len(tokens)) + + // Phase 1: 串行获取所有 download URL（RuntimeContext 不是并发安全的） + type downloadTask struct { + index int + token string + downloadURL string + } + var tasks []downloadTask + seen := make(map[string]int) // 去重：token → 首次出现的 index + + for i, token := range tokens { + if err := ctx.Err(); err != nil { return err } + if err := validate.ResourceName(token, "--minute-tokens"); err != nil { + results[i] = batchResult{MinuteToken: token, Error: err.Error()} + continue + } - downloadURL := common.GetString(data, "download_url") - if downloadURL == "" { - return output.Errorf(output.ExitAPI, "api_error", "API returned empty download_url") + // 跳过重复 token，指向首次结果 + if firstIdx, dup := seen[token]; dup { + results[i] = batchResult{MinuteToken: token, Error: fmt.Sprintf("duplicate token, same as index %d", firstIdx)} + continue + } + seen[token] = i + + downloadURL, err := fetchDownloadURL(ctx, runtime, token) + if err != nil { + results[i] = batchResult{MinuteToken: token, Error: err.Error()} + continue } - // --url-only mode: print download URL only if urlOnly { - runtime.Out(map[string]interface{}{ - "download_url": downloadURL, - }, nil) - return nil + results[i] = batchResult{MinuteToken: token, DownloadURL: downloadURL} + continue } - // Step 2: download the file and resolve the output path - fmt.Fprintf(runtime.IO().ErrOut, "Downloading media: %s\n", common.MaskToken(minuteToken)) + tasks = append(tasks, downloadTask{index: i, token: token, downloadURL: downloadURL}) + } - result, err := downloadMediaFile(ctx, runtime, downloadURL, outputPath, minuteToken, overwrite) - if err != nil { - return err + // Phase 2: 并发下载文件 + if len(tasks) > 0 { + var usedNames sync.Map + var logMu sync.Mutex + + g, gctx := errgroup.WithContext(ctx) + g.SetLimit(maxConcurrentDownloads) + + for _, task := range tasks { + task := task + g.Go(func() error { + logMu.Lock() + fmt.Fprintf(errOut, "[minutes +download] downloading: %s\n", common.MaskToken(task.token)) + logMu.Unlock() + + result, err := downloadMediaFile(gctx, runtime, task.downloadURL, task.token, downloadOpts{ + outputDir: outputDir, + overwrite: overwrite, + usedNames: &usedNames, + }) + if err != nil { + results[task.index] = batchResult{MinuteToken: task.token, Error: err.Error()} + return nil // partial failure + } + + results[task.index] = batchResult{ + MinuteToken: task.token, + SavedPath: result.savedPath, + SizeBytes: result.sizeBytes, + } + return nil + }) } + g.Wait() + } - runtime.Out(map[string]interface{}{ - "saved_path": result.savedPath, - "size_bytes": result.sizeBytes, - }, nil) - return nil - }, + // 统计 + successCount := 0 + for _, r := range results { + if r.Error == "" { + successCount++ + } + } + fmt.Fprintf(errOut, "[minutes +download] done: %d total, %d succeeded, %d failed\n", len(results), successCount, len(results)-successCount) + + outData := map[string]interface{}{"downloads": results} + runtime.OutFormat(outData, &output.Meta{Count: len(results)}, nil) + + if successCount == 0 && len(results) > 0 { + return output.ErrAPI(0, fmt.Sprintf("all %d downloads failed", len(results)), nil) + } + return nil +} + +// fetchDownloadURL retrieves the pre-signed download URL for a minute token. +func fetchDownloadURL(ctx context.Context, runtime *common.RuntimeContext, minuteToken string) (string, error) { + data, err := runtime.DoAPIJSON(http.MethodGet, + fmt.Sprintf("/open-apis/minutes/v1/minutes/%s/media", validate.EncodePathSegment(minuteToken)), + nil, nil) + if err != nil { + return "", err + } + downloadURL := common.GetString(data, "download_url") + if downloadURL == "" { + return "", output.Errorf(output.ExitAPI, "api_error", "API returned empty download_url for %s", minuteToken) + } + return downloadURL, nil +} + +// deduplicateFilename ensures uniqueness by appending a token suffix on collision. +// Returns the deduplicated filename (without directory prefix). +func deduplicateFilename(name, minuteToken string, usedNames *sync.Map) string { + if _, loaded := usedNames.LoadOrStore(name, true); !loaded { + return name + } + + // 冲突：在扩展名前插入 _<token前6位> + ext := filepath.Ext(name) + base := strings.TrimSuffix(name, ext) + suffix := minuteToken + if len(suffix) > tokenSuffixLen { + suffix = suffix[:tokenSuffixLen] + } + deduped := base + "_" + suffix + ext + usedNames.Store(deduped, true) + return deduped } type downloadResult struct { @@ -97,10 +291,18 @@ type downloadResult struct { sizeBytes int64 } +// downloadOpts controls how downloadMediaFile resolves the output path. +type downloadOpts struct { + outputPath string // 用户指定的输出路径（单个模式），为空则从响应头解析 + outputDir string // 输出目录前缀（批量模式） + overwrite bool // 是否覆盖已有文件 + usedNames *sync.Map // 批量模式下的文件名去重表，单个模式传 nil +} + // downloadMediaFile streams a media file from a pre-signed URL to disk. -// When outputPath is empty, it resolves the filename from the Content-Disposition -// header, falling back to Content-Type extension detection, then <token>.media. -func downloadMediaFile(ctx context.Context, runtime *common.RuntimeContext, downloadURL, outputPath, minuteToken string, overwrite bool) (*downloadResult, error) { +// Output path resolution: opts.outputPath > Content-Disposition > Content-Type extension > <token>.media. +// When opts.usedNames is non-nil (batch mode), deduplicates filenames by appending token suffix. +func downloadMediaFile(ctx context.Context, runtime *common.RuntimeContext, downloadURL, minuteToken string, opts downloadOpts) (*downloadResult, error) { // SSRF: validate download URL before making any request if err := validate.ValidateDownloadSourceURL(ctx, downloadURL); err != nil { return nil, output.ErrValidation("blocked download URL: %s", err) @@ -136,19 +338,23 @@ func downloadMediaFile(ctx context.Context, runtime *common.RuntimeContext, down return nil, output.ErrNetwork("download failed: HTTP %d", resp.StatusCode) } - // resolve output path from response headers when --output is not specified + // 解析输出路径 + outputPath := opts.outputPath if outputPath == "" { - outputPath = resolveOutputFromResponse(resp, minuteToken) + filename := resolveOutputFromResponse(resp, minuteToken) + if opts.usedNames != nil { + filename = deduplicateFilename(filename, minuteToken, opts.usedNames) + } + outputPath = filepath.Join(opts.outputDir, filename) } safePath, err := validate.SafeOutputPath(outputPath) if err != nil { return nil, output.ErrValidation("unsafe output path: %s", err) } - if err := common.EnsureWritableFile(safePath, overwrite); err != nil { + if err := common.EnsureWritableFile(safePath, opts.overwrite); err != nil { return nil, err } - if err := os.MkdirAll(filepath.Dir(safePath), 0755); err != nil { return nil, output.Errorf(output.ExitInternal, "api_error", "cannot create parent directory: %s", err) } @@ -163,7 +369,6 @@ func downloadMediaFile(ctx context.Context, runtime *common.RuntimeContext, down // resolveOutputFromResponse derives the output filename from HTTP response headers. // Priority: Content-Disposition filename > Content-Type extension > fallback to <token>.media. func resolveOutputFromResponse(resp *http.Response, minuteToken string) string { - // try Content-Disposition header for the original filename if cd := resp.Header.Get("Content-Disposition"); cd != "" { if _, params, err := mime.ParseMediaType(cd); err == nil { if filename := params["filename"]; filename != "" { @@ -172,7 +377,6 @@ func resolveOutputFromResponse(resp *http.Response, minuteToken string) string { } } - // fall back to Content-Type extension detection if ct := resp.Header.Get("Content-Type"); ct != "" { if mediaType, _, err := mime.ParseMediaType(ct); err == nil { if exts, err := mime.ExtensionsByType(mediaType); err == nil && len(exts) > 0 { @@ -184,8 +388,6 @@ func resolveOutputFromResponse(resp *http.Response, minuteToken string) string { return minuteToken + ".media" } -const maxDownloadRedirects = 5 - // safeRedirectPolicy prevents HTTPS→HTTP downgrade, limits redirect count, // and validates each redirect target against SSRF rules. func safeRedirectPolicy(req *http.Request, via []*http.Request) error { diff --git a/shortcuts/minutes/minutes_download_test.go b/shortcuts/minutes/minutes_download_test.go index a225bc93..082e092b 100644 --- a/shortcuts/minutes/minutes_download_test.go +++ b/shortcuts/minutes/minutes_download_test.go @@ -6,6 +6,7 @@ package minutes import ( "bytes" "context" + "encoding/json" "net/http" "os" "strings" @@ -139,7 +140,7 @@ func TestResolveOutputFromResponse_ContentType(t *testing.T) { if !strings.HasPrefix(got, "tok001") { t.Errorf("expected token prefix, got %q", got) } - if filepath := got[len("tok001"):]; filepath == "" { + if ext := got[len("tok001"):]; ext == "" { t.Errorf("expected extension after token, got %q", got) } } @@ -160,7 +161,6 @@ func TestResolveOutputFromResponse_InvalidContentDisposition(t *testing.T) { }, } got := resolveOutputFromResponse(resp, "tok001") - // Content-Disposition 解析失败，应 fall back 到 Content-Type if !strings.HasPrefix(got, "tok001") { t.Errorf("expected token prefix from Content-Type fallback, got %q", got) } @@ -177,14 +177,85 @@ func TestResolveOutputFromResponse_EmptyDispositionFilename(t *testing.T) { if got == "" { t.Error("expected non-empty filename") } - // Content-Disposition 没有 filename，应 fall back 到 Content-Type if !strings.HasPrefix(got, "tok001") { t.Errorf("expected token prefix, got %q", got) } } // --------------------------------------------------------------------------- -// Integration tests: +download with mocked HTTP +// Unit tests: deduplicateFilename +// --------------------------------------------------------------------------- + +func TestDeduplicateFilename_NoDuplicate(t *testing.T) { + var usedNames sync.Map + got := deduplicateFilename("会议纪要.mp4", "tok001", &usedNames) + if got != "会议纪要.mp4" { + t.Errorf("expected original name, got %q", got) + } +} + +func TestDeduplicateFilename_Duplicate(t *testing.T) { + var usedNames sync.Map + first := deduplicateFilename("会议纪要.mp4", "tok001abc", &usedNames) + second := deduplicateFilename("会议纪要.mp4", "tok002xyz", &usedNames) + if first != "会议纪要.mp4" { + t.Errorf("first should be original, got %q", first) + } + if second == first { + t.Errorf("second should differ from first, both got %q", first) + } + // 第二个应该包含 token 前缀作为区分 + if !strings.Contains(second, "tok002") { + t.Errorf("deduplicated name should contain token prefix, got %q", second) + } + if !strings.HasSuffix(second, ".mp4") { + t.Errorf("deduplicated name should keep extension, got %q", second) + } +} + +func TestDeduplicateFilename_NoExtension(t *testing.T) { + var usedNames sync.Map + deduplicateFilename("recording", "tok001", &usedNames) + got := deduplicateFilename("recording", "tok002abc", &usedNames) + if !strings.Contains(got, "tok002") { + t.Errorf("expected token suffix, got %q", got) + } +} + +// --------------------------------------------------------------------------- +// Validation tests +// --------------------------------------------------------------------------- + +func TestDownload_Validation_NoFlags(t *testing.T) { + f, _, _, _ := cmdutil.TestFactory(t, defaultConfig()) + err := mountAndRun(t, MinutesDownload, []string{"+download", "--as", "user"}, f, nil) + if err == nil { + t.Fatal("expected validation error for no flags") + } +} + +func TestDownload_Validation_BothFlags(t *testing.T) { + f, _, _, _ := cmdutil.TestFactory(t, defaultConfig()) + err := mountAndRun(t, MinutesDownload, []string{ + "+download", "--minute-token", "t1", "--minute-tokens", "t2", "--as", "user", + }, f, nil) + if err == nil { + t.Fatal("expected validation error for both flags") + } +} + +func TestDownload_Validation_OutputWithBatch(t *testing.T) { + f, _, _, _ := cmdutil.TestFactory(t, defaultConfig()) + err := mountAndRun(t, MinutesDownload, []string{ + "+download", "--minute-tokens", "t1,t2", "--output", "file.mp4", "--as", "user", + }, f, nil) + if err == nil { + t.Fatal("expected validation error for --output with --minute-tokens") + } +} + +// --------------------------------------------------------------------------- +// Integration tests: single mode // --------------------------------------------------------------------------- func TestDownload_DryRun(t *testing.T) { @@ -240,9 +311,6 @@ func TestDownload_FullDownload(t *testing.T) { if string(data) != "fake-video-content" { t.Errorf("file content = %q, want %q", string(data), "fake-video-content") } - if !strings.Contains(stdout.String(), "saved_path") { - t.Errorf("output should contain saved_path, got: %s", stdout.String()) - } } func TestDownload_OverwriteProtection(t *testing.T) { @@ -263,7 +331,6 @@ func TestDownload_OverwriteProtection(t *testing.T) { t.Errorf("error should mention file exists, got: %v", err) } - // 原文件不应被覆盖 data, _ := os.ReadFile("existing.mp4") if string(data) != "old" { t.Errorf("original file should be preserved, got %q", string(data)) @@ -291,3 +358,117 @@ func TestDownload_HttpError(t *testing.T) { t.Errorf("error should contain status code, got: %v", err) } } + +// --------------------------------------------------------------------------- +// Integration tests: batch mode +// --------------------------------------------------------------------------- + +func TestDownload_Batch_UrlOnly(t *testing.T) { + f, stdout, _, reg := cmdutil.TestFactory(t, defaultConfig()) + reg.Register(mediaStub("tok001", "https://example.com/download/1")) + reg.Register(mediaStub("tok002", "https://example.com/download/2")) + + err := mountAndRun(t, MinutesDownload, []string{ + "+download", "--minute-tokens", "tok001,tok002", "--url-only", "--as", "bot", + }, f, stdout) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + out := stdout.String() + if !strings.Contains(out, "download/1") || !strings.Contains(out, "download/2") { + t.Errorf("batch url-only should show both URLs, got: %s", out) + } +} + +func TestDownload_Batch_Download(t *testing.T) { + chdir(t, t.TempDir()) + + f, stdout, _, reg := cmdutil.TestFactory(t, defaultConfig()) + reg.Register(mediaStub("tok001", "https://example.com/download/1")) + reg.Register(mediaStub("tok002", "https://example.com/download/2")) + reg.Register(downloadStub("example.com/download/1", []byte("content-1"), "video/mp4")) + reg.Register(downloadStub("example.com/download/2", []byte("content-2"), "video/mp4")) + + err := mountAndRun(t, MinutesDownload, []string{ + "+download", "--minute-tokens", "tok001,tok002", "--as", "bot", + }, f, stdout) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // 验证输出结构 + var result struct { + Data struct { + Downloads []struct { + MinuteToken string `json:"minute_token"` + SavedPath string `json:"saved_path"` + } `json:"downloads"` + } `json:"data"` + } + if err := json.Unmarshal(stdout.Bytes(), &result); err != nil { + t.Fatalf("failed to parse output: %v\nraw: %s", err, stdout.String()) + } + if len(result.Data.Downloads) != 2 { + t.Fatalf("expected 2 downloads, got %d", len(result.Data.Downloads)) + } +} + +func TestDownload_Batch_PartialFailure(t *testing.T) { + chdir(t, t.TempDir()) + + f, stdout, _, reg := cmdutil.TestFactory(t, defaultConfig()) + reg.Register(mediaStub("tok001", "https://example.com/download/1")) + reg.Register(&httpmock.Stub{ + Method: "GET", + URL: "/open-apis/minutes/v1/minutes/tok002/media", + Status: 200, + Body: map[string]interface{}{ + "code": 99999, "msg": "permission denied", + "data": map[string]interface{}{}, + }, + }) + reg.Register(downloadStub("example.com/download/1", []byte("content-1"), "video/mp4")) + + err := mountAndRun(t, MinutesDownload, []string{ + "+download", "--minute-tokens", "tok001,tok002", "--as", "bot", + }, f, stdout) + // partial failure 不报错 + if err != nil { + t.Fatalf("partial failure should not return error, got: %v", err) + } + out := stdout.String() + if !strings.Contains(out, "tok001") || !strings.Contains(out, "tok002") { + t.Errorf("output should contain both tokens, got: %s", out) + } +} + +func TestDownload_Batch_DuplicateToken(t *testing.T) { + f, stdout, _, reg := cmdutil.TestFactory(t, defaultConfig()) + // 只注册一次 media stub — 去重后只调用一次 API + reg.Register(mediaStub("tok001", "https://example.com/download/1")) + + err := mountAndRun(t, MinutesDownload, []string{ + "+download", "--minute-tokens", "tok001,tok001", "--url-only", "--as", "bot", + }, f, stdout) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + out := stdout.String() + if !strings.Contains(out, "duplicate") { + t.Errorf("second token should report duplicate, got: %s", out) + } +} + +func TestDownload_Batch_DryRun(t *testing.T) { + f, stdout, _, _ := cmdutil.TestFactory(t, defaultConfig()) + err := mountAndRun(t, MinutesDownload, []string{ + "+download", "--minute-tokens", "tok001,tok002", "--dry-run", "--as", "user", + }, f, stdout) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + out := stdout.String() + if !strings.Contains(out, "tok001") || !strings.Contains(out, "tok002") { + t.Errorf("dry-run should show tokens, got: %s", out) + } +} From ce6ee5b5c9180585efc5c8665889cf806f9b2120 Mon Sep 17 00:00:00 2001 From: renaocheng <renaocheng@bytedance.com> Date: Tue, 31 Mar 2026 17:49:07 +0800 Subject: [PATCH 09/17] chore: promote golang.org/x/sync to direct dependency --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index ed41d1f0..8c6609b8 100644 --- a/go.mod +++ b/go.mod @@ -13,6 +13,7 @@ require ( github.com/spf13/cobra v1.10.2 github.com/zalando/go-keyring v0.2.8 golang.org/x/net v0.33.0 + golang.org/x/sync v0.15.0 golang.org/x/sys v0.33.0 golang.org/x/term v0.27.0 golang.org/x/text v0.23.0 @@ -50,5 +51,4 @@ require ( github.com/smarty/assertions v1.15.0 // indirect github.com/spf13/pflag v1.0.9 // indirect github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e // indirect - golang.org/x/sync v0.15.0 // indirect ) From 56e2fb860203c8c0f8181f8f842df3b44e8bf0f4 Mon Sep 17 00:00:00 2001 From: renaocheng <renaocheng@bytedance.com> Date: Tue, 31 Mar 2026 17:53:54 +0800 Subject: [PATCH 10/17] fix: resolve copyloopvar and nilerr lint errors --- shortcuts/minutes/minutes_download.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/shortcuts/minutes/minutes_download.go b/shortcuts/minutes/minutes_download.go index b4f04db9..6ef56d88 100644 --- a/shortcuts/minutes/minutes_download.go +++ b/shortcuts/minutes/minutes_download.go @@ -207,20 +207,19 @@ func executeBatch(ctx context.Context, runtime *common.RuntimeContext) error { g.SetLimit(maxConcurrentDownloads) for _, task := range tasks { - task := task g.Go(func() error { logMu.Lock() fmt.Fprintf(errOut, "[minutes +download] downloading: %s\n", common.MaskToken(task.token)) logMu.Unlock() - result, err := downloadMediaFile(gctx, runtime, task.downloadURL, task.token, downloadOpts{ + result, dlErr := downloadMediaFile(gctx, runtime, task.downloadURL, task.token, downloadOpts{ outputDir: outputDir, overwrite: overwrite, usedNames: &usedNames, }) - if err != nil { - results[task.index] = batchResult{MinuteToken: task.token, Error: err.Error()} - return nil // partial failure + if dlErr != nil { + results[task.index] = batchResult{MinuteToken: task.token, Error: dlErr.Error()} + return nil // partial failure: record error, don't abort other downloads } results[task.index] = batchResult{ From a29e4379ae286c37f04f79ea20f63323feeda77c Mon Sep 17 00:00:00 2001 From: renaocheng <renaocheng@bytedance.com> Date: Tue, 31 Mar 2026 17:59:06 +0800 Subject: [PATCH 11/17] fix: replace errgroup with WaitGroup to resolve nilerr lint and translate comments to English --- go.mod | 2 +- shortcuts/minutes/minutes_download.go | 49 ++++++++++++++------------- 2 files changed, 26 insertions(+), 25 deletions(-) diff --git a/go.mod b/go.mod index 8c6609b8..ed41d1f0 100644 --- a/go.mod +++ b/go.mod @@ -13,7 +13,6 @@ require ( github.com/spf13/cobra v1.10.2 github.com/zalando/go-keyring v0.2.8 golang.org/x/net v0.33.0 - golang.org/x/sync v0.15.0 golang.org/x/sys v0.33.0 golang.org/x/term v0.27.0 golang.org/x/text v0.23.0 @@ -51,4 +50,5 @@ require ( github.com/smarty/assertions v1.15.0 // indirect github.com/spf13/pflag v1.0.9 // indirect github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e // indirect + golang.org/x/sync v0.15.0 // indirect ) diff --git a/shortcuts/minutes/minutes_download.go b/shortcuts/minutes/minutes_download.go index 6ef56d88..00a16757 100644 --- a/shortcuts/minutes/minutes_download.go +++ b/shortcuts/minutes/minutes_download.go @@ -14,8 +14,6 @@ import ( "strings" "sync" - "golang.org/x/sync/errgroup" - "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" @@ -159,14 +157,14 @@ func executeBatch(ctx context.Context, runtime *common.RuntimeContext) error { results := make([]batchResult, len(tokens)) - // Phase 1: 串行获取所有 download URL（RuntimeContext 不是并发安全的） + // Phase 1: fetch all download URLs sequentially (RuntimeContext is not concurrency-safe) type downloadTask struct { index int token string downloadURL string } var tasks []downloadTask - seen := make(map[string]int) // 去重：token → 首次出现的 index + seen := make(map[string]int) // dedup: token -> index of first occurrence for i, token := range tokens { if err := ctx.Err(); err != nil { @@ -177,7 +175,7 @@ func executeBatch(ctx context.Context, runtime *common.RuntimeContext) error { continue } - // 跳过重复 token，指向首次结果 + // skip duplicate tokens, point to first occurrence if firstIdx, dup := seen[token]; dup { results[i] = batchResult{MinuteToken: token, Error: fmt.Sprintf("duplicate token, same as index %d", firstIdx)} continue @@ -198,28 +196,32 @@ func executeBatch(ctx context.Context, runtime *common.RuntimeContext) error { tasks = append(tasks, downloadTask{index: i, token: token, downloadURL: downloadURL}) } - // Phase 2: 并发下载文件 + // Phase 2: download files concurrently (WaitGroup + semaphore) if len(tasks) > 0 { var usedNames sync.Map var logMu sync.Mutex - - g, gctx := errgroup.WithContext(ctx) - g.SetLimit(maxConcurrentDownloads) + var wg sync.WaitGroup + sem := make(chan struct{}, maxConcurrentDownloads) for _, task := range tasks { - g.Go(func() error { + wg.Add(1) + go func(task downloadTask) { + defer wg.Done() + sem <- struct{}{} + defer func() { <-sem }() + logMu.Lock() fmt.Fprintf(errOut, "[minutes +download] downloading: %s\n", common.MaskToken(task.token)) logMu.Unlock() - result, dlErr := downloadMediaFile(gctx, runtime, task.downloadURL, task.token, downloadOpts{ + result, err := downloadMediaFile(ctx, runtime, task.downloadURL, task.token, downloadOpts{ outputDir: outputDir, overwrite: overwrite, usedNames: &usedNames, }) - if dlErr != nil { - results[task.index] = batchResult{MinuteToken: task.token, Error: dlErr.Error()} - return nil // partial failure: record error, don't abort other downloads + if err != nil { + results[task.index] = batchResult{MinuteToken: task.token, Error: err.Error()} + return } results[task.index] = batchResult{ @@ -227,13 +229,12 @@ func executeBatch(ctx context.Context, runtime *common.RuntimeContext) error { SavedPath: result.savedPath, SizeBytes: result.sizeBytes, } - return nil - }) + }(task) } - g.Wait() + wg.Wait() } - // 统计 + // summary successCount := 0 for _, r := range results { if r.Error == "" { @@ -273,7 +274,7 @@ func deduplicateFilename(name, minuteToken string, usedNames *sync.Map) string { return name } - // 冲突：在扩展名前插入 _<token前6位> + // collision: insert _<token_prefix> before extension ext := filepath.Ext(name) base := strings.TrimSuffix(name, ext) suffix := minuteToken @@ -292,10 +293,10 @@ type downloadResult struct { // downloadOpts controls how downloadMediaFile resolves the output path. type downloadOpts struct { - outputPath string // 用户指定的输出路径（单个模式），为空则从响应头解析 - outputDir string // 输出目录前缀（批量模式） - overwrite bool // 是否覆盖已有文件 - usedNames *sync.Map // 批量模式下的文件名去重表，单个模式传 nil + outputPath string // explicit output path (single mode); empty = resolve from response headers + outputDir string // output directory prefix (batch mode) + overwrite bool // overwrite existing files + usedNames *sync.Map // filename dedup table for batch mode; nil for single mode } // downloadMediaFile streams a media file from a pre-signed URL to disk. @@ -337,7 +338,7 @@ func downloadMediaFile(ctx context.Context, runtime *common.RuntimeContext, down return nil, output.ErrNetwork("download failed: HTTP %d", resp.StatusCode) } - // 解析输出路径 + // resolve output path outputPath := opts.outputPath if outputPath == "" { filename := resolveOutputFromResponse(resp, minuteToken) From 23746bfc6e6db0bb0e03fc1b0f4ffd4f56423b5f Mon Sep 17 00:00:00 2001 From: renaocheng <renaocheng@bytedance.com> Date: Wed, 1 Apr 2026 15:04:00 +0800 Subject: [PATCH 12/17] feat: unify --minute-tokens flag, add batch download, token validation, and smart filename resolution --- shortcuts/minutes/minutes_download.go | 186 +++++++++++------- shortcuts/minutes/minutes_download_test.go | 36 ++-- skills/lark-minutes/SKILL.md | 7 +- .../references/lark-minutes-download.md | 23 ++- 4 files changed, 155 insertions(+), 97 deletions(-) diff --git a/shortcuts/minutes/minutes_download.go b/shortcuts/minutes/minutes_download.go index 00a16757..eecf7416 100644 --- a/shortcuts/minutes/minutes_download.go +++ b/shortcuts/minutes/minutes_download.go @@ -11,6 +11,7 @@ import ( "net/http" "os" "path/filepath" + "regexp" "strings" "sync" @@ -30,6 +31,9 @@ const ( tokenSuffixLen = 6 ) +// validMinuteToken matches minute tokens: lowercase alphanumeric characters only. +var validMinuteToken = regexp.MustCompile(`^[a-z0-9]+$`) + var MinutesDownload = common.Shortcut{ Service: "minutes", Command: "+download", @@ -39,34 +43,28 @@ var MinutesDownload = common.Shortcut{ AuthTypes: []string{"user", "bot"}, HasFormat: true, Flags: []common.Flag{ - {Name: "minute-token", Desc: "single minute token (mutually exclusive with --minute-tokens)"}, - {Name: "minute-tokens", Desc: "comma-separated minute tokens for batch download (max 50)"}, - {Name: "output", Desc: "local save path (single mode only)"}, + {Name: "minute-tokens", Desc: "minute tokens, comma-separated for batch download (max 50)", Required: true}, + {Name: "output", Desc: "local save path (single token only)"}, {Name: "output-dir", Desc: "output directory for batch download (default: current dir)"}, {Name: "overwrite", Type: "bool", Desc: "overwrite existing output file"}, {Name: "url-only", Type: "bool", Desc: "only print the download URL(s) without downloading"}, }, Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { - single := runtime.Str("minute-token") - batch := runtime.Str("minute-tokens") - - if single == "" && batch == "" { - return output.ErrValidation("one of --minute-token or --minute-tokens is required") + tokens := common.SplitCSV(runtime.Str("minute-tokens")) + if len(tokens) == 0 { + return output.ErrValidation("--minute-tokens is required") } - if single != "" && batch != "" { - return output.ErrValidation("--minute-token and --minute-tokens are mutually exclusive") + if len(tokens) > maxBatchSize { + return output.ErrValidation("--minute-tokens: too many tokens (%d), maximum is %d", len(tokens), maxBatchSize) } - - if batch != "" { - tokens := common.SplitCSV(batch) - if len(tokens) > maxBatchSize { - return output.ErrValidation("--minute-tokens: too many tokens (%d), maximum is %d", len(tokens), maxBatchSize) - } - if runtime.Str("output") != "" { - return output.ErrValidation("--output cannot be used with --minute-tokens; use --output-dir instead") + for _, token := range tokens { + if !validMinuteToken.MatchString(token) { + return output.ErrValidation("invalid minute token %q: must contain only lowercase alphanumeric characters (e.g. obcnq3b9jl72l83w4f149w9c)", token) } } - + if len(tokens) > 1 && runtime.Str("output") != "" { + return output.ErrValidation("--output cannot be used with multiple tokens; use --output-dir instead") + } if outDir := runtime.Str("output-dir"); outDir != "" { if err := common.ValidateSafeOutputDir(outDir); err != nil { return err @@ -75,34 +73,31 @@ var MinutesDownload = common.Shortcut{ return nil }, DryRun: func(ctx context.Context, runtime *common.RuntimeContext) *common.DryRunAPI { + tokens := common.SplitCSV(runtime.Str("minute-tokens")) api := common.NewDryRunAPI(). GET("/open-apis/minutes/v1/minutes/:minute_token/media") - - if token := runtime.Str("minute-token"); token != "" { - api.Set("minute_token", token) - } - if tokens := runtime.Str("minute-tokens"); tokens != "" { - api.Set("minute_tokens", common.SplitCSV(tokens)) + api.Set("minute_tokens", tokens) + if len(tokens) > 1 { api.Set("concurrent_downloads", maxConcurrentDownloads) } return api }, Execute: func(ctx context.Context, runtime *common.RuntimeContext) error { - if tokens := runtime.Str("minute-tokens"); tokens != "" { - return executeBatch(ctx, runtime) + tokens := common.SplitCSV(runtime.Str("minute-tokens")) + if len(tokens) == 1 { + return executeSingle(ctx, runtime, tokens[0]) } - return executeSingle(ctx, runtime) + return executeBatch(ctx, runtime, tokens) }, } -// executeSingle handles the single --minute-token mode. -func executeSingle(ctx context.Context, runtime *common.RuntimeContext) error { - minuteToken := runtime.Str("minute-token") +// executeSingle handles a single token download. +func executeSingle(ctx context.Context, runtime *common.RuntimeContext, minuteToken string) error { outputPath := runtime.Str("output") overwrite := runtime.Bool("overwrite") urlOnly := runtime.Bool("url-only") - if err := validate.ResourceName(minuteToken, "--minute-token"); err != nil { + if err := validate.ResourceName(minuteToken, "--minute-tokens"); err != nil { return output.ErrValidation("%s", err) } @@ -135,11 +130,10 @@ func executeSingle(ctx context.Context, runtime *common.RuntimeContext) error { return nil } -// executeBatch handles the batch --minute-tokens mode. +// executeBatch handles multiple tokens with concurrent downloads. // Phase 1: sequentially fetch download URLs (RuntimeContext is not concurrency-safe). // Phase 2: concurrently download files (HTTP client is concurrency-safe). -func executeBatch(ctx context.Context, runtime *common.RuntimeContext) error { - tokens := common.SplitCSV(runtime.Str("minute-tokens")) +func executeBatch(ctx context.Context, runtime *common.RuntimeContext, tokens []string) error { overwrite := runtime.Bool("overwrite") urlOnly := runtime.Bool("url-only") outputDir := runtime.Str("output-dir") @@ -157,14 +151,14 @@ func executeBatch(ctx context.Context, runtime *common.RuntimeContext) error { results := make([]batchResult, len(tokens)) - // Phase 1: fetch all download URLs sequentially (RuntimeContext is not concurrency-safe) + // Phase 1: fetch all download URLs sequentially type downloadTask struct { index int token string downloadURL string } var tasks []downloadTask - seen := make(map[string]int) // dedup: token -> index of first occurrence + seen := make(map[string]int) for i, token := range tokens { if err := ctx.Err(); err != nil { @@ -175,7 +169,6 @@ func executeBatch(ctx context.Context, runtime *common.RuntimeContext) error { continue } - // skip duplicate tokens, point to first occurrence if firstIdx, dup := seen[token]; dup { results[i] = batchResult{MinuteToken: token, Error: fmt.Sprintf("duplicate token, same as index %d", firstIdx)} continue @@ -196,7 +189,7 @@ func executeBatch(ctx context.Context, runtime *common.RuntimeContext) error { tasks = append(tasks, downloadTask{index: i, token: token, downloadURL: downloadURL}) } - // Phase 2: download files concurrently (WaitGroup + semaphore) + // Phase 2: download files concurrently if len(tasks) > 0 { var usedNames sync.Map var logMu sync.Mutex @@ -234,7 +227,6 @@ func executeBatch(ctx context.Context, runtime *common.RuntimeContext) error { wg.Wait() } - // summary successCount := 0 for _, r := range results { if r.Error == "" { @@ -268,13 +260,10 @@ func fetchDownloadURL(ctx context.Context, runtime *common.RuntimeContext, minut } // deduplicateFilename ensures uniqueness by appending a token suffix on collision. -// Returns the deduplicated filename (without directory prefix). func deduplicateFilename(name, minuteToken string, usedNames *sync.Map) string { if _, loaded := usedNames.LoadOrStore(name, true); !loaded { return name } - - // collision: insert _<token_prefix> before extension ext := filepath.Ext(name) base := strings.TrimSuffix(name, ext) suffix := minuteToken @@ -291,19 +280,16 @@ type downloadResult struct { sizeBytes int64 } -// downloadOpts controls how downloadMediaFile resolves the output path. type downloadOpts struct { - outputPath string // explicit output path (single mode); empty = resolve from response headers - outputDir string // output directory prefix (batch mode) - overwrite bool // overwrite existing files - usedNames *sync.Map // filename dedup table for batch mode; nil for single mode + outputPath string // explicit output path (single mode) + outputDir string // output directory prefix (batch mode) + overwrite bool + usedNames *sync.Map // filename dedup table for batch mode } // downloadMediaFile streams a media file from a pre-signed URL to disk. // Output path resolution: opts.outputPath > Content-Disposition > Content-Type extension > <token>.media. -// When opts.usedNames is non-nil (batch mode), deduplicates filenames by appending token suffix. func downloadMediaFile(ctx context.Context, runtime *common.RuntimeContext, downloadURL, minuteToken string, opts downloadOpts) (*downloadResult, error) { - // SSRF: validate download URL before making any request if err := validate.ValidateDownloadSourceURL(ctx, downloadURL); err != nil { return nil, output.ErrValidation("blocked download URL: %s", err) } @@ -313,7 +299,6 @@ func downloadMediaFile(ctx context.Context, runtime *common.RuntimeContext, down return nil, output.ErrNetwork("failed to get HTTP client: %s", err) } - // clone client: disable timeout for large files, add redirect safety policy downloadClient := *httpClient downloadClient.Timeout = disableClientTimeout downloadClient.CheckRedirect = safeRedirectPolicy @@ -322,7 +307,6 @@ func downloadMediaFile(ctx context.Context, runtime *common.RuntimeContext, down if err != nil { return nil, output.ErrNetwork("invalid download URL: %s", err) } - // no Authorization header — download_url is a pre-signed URL resp, err := downloadClient.Do(req) if err != nil { @@ -338,10 +322,9 @@ func downloadMediaFile(ctx context.Context, runtime *common.RuntimeContext, down return nil, output.ErrNetwork("download failed: HTTP %d", resp.StatusCode) } - // resolve output path outputPath := opts.outputPath if outputPath == "" { - filename := resolveOutputFromResponse(resp, minuteToken) + filename := resolveOutputFromResponse(ctx, runtime, resp, minuteToken) if opts.usedNames != nil { filename = deduplicateFilename(filename, minuteToken, opts.usedNames) } @@ -366,30 +349,91 @@ func downloadMediaFile(ctx context.Context, runtime *common.RuntimeContext, down return &downloadResult{savedPath: safePath, sizeBytes: sizeBytes}, nil } -// resolveOutputFromResponse derives the output filename from HTTP response headers. -// Priority: Content-Disposition filename > Content-Type extension > fallback to <token>.media. -func resolveOutputFromResponse(resp *http.Response, minuteToken string) string { - if cd := resp.Header.Get("Content-Disposition"); cd != "" { - if _, params, err := mime.ParseMediaType(cd); err == nil { - if filename := params["filename"]; filename != "" { - return filename - } +// resolveOutputFromResponse derives the output filename from the minute title (via API) +// and Content-Type extension. Content-Disposition is not used for the filename because +// the server may alter the original title (e.g. replacing "|" with "_"). +// Falls back to <token>.media when both title and Content-Type are unavailable. +func resolveOutputFromResponse(ctx context.Context, runtime *common.RuntimeContext, resp *http.Response, minuteToken string) string { + title := fetchMinuteTitle(ctx, runtime, minuteToken) + ext := extFromContentType(resp.Header.Get("Content-Type")) + + if title != "" { + name := sanitizeFileName(title) + if name == "" { + name = minuteToken + } + if ext != "" { + return name + ext } + return name + ".media" } + if ext != "" { + return minuteToken + ext + } + return minuteToken + ".media" +} - if ct := resp.Header.Get("Content-Type"); ct != "" { - if mediaType, _, err := mime.ParseMediaType(ct); err == nil { - if exts, err := mime.ExtensionsByType(mediaType); err == nil && len(exts) > 0 { - return minuteToken + exts[0] - } - } +// fetchMinuteTitle retrieves the minute title via minutes get API. Returns "" on failure. +func fetchMinuteTitle(ctx context.Context, runtime *common.RuntimeContext, minuteToken string) string { + if runtime == nil { + return "" } + data, err := runtime.DoAPIJSON(http.MethodGet, + fmt.Sprintf("/open-apis/minutes/v1/minutes/%s", validate.EncodePathSegment(minuteToken)), + nil, nil) + if err != nil { + return "" + } + if minute, ok := data["minute"].(map[string]interface{}); ok { + return common.GetString(minute, "title") + } + return "" +} - return minuteToken + ".media" +// preferredExt overrides Go's mime.ExtensionsByType which returns alphabetically sorted +// results (e.g. .m4v before .mp4 for video/mp4). Map the most common extensions here. +var preferredExt = map[string]string{ + "video/mp4": ".mp4", + "audio/mp4": ".m4a", + "audio/mpeg": ".mp3", +} + +// extFromContentType returns a file extension for the given Content-Type, or "" if unknown. +func extFromContentType(contentType string) string { + if contentType == "" { + return "" + } + mediaType, _, err := mime.ParseMediaType(contentType) + if err != nil { + return "" + } + if ext, ok := preferredExt[mediaType]; ok { + return ext + } + if exts, err := mime.ExtensionsByType(mediaType); err == nil && len(exts) > 0 { + return exts[0] + } + return "" +} + +// sanitizeFileName replaces filesystem-unsafe characters with visually similar fullwidth +// Unicode equivalents so the filename stays readable and cross-platform safe. +func sanitizeFileName(name string) string { + const maxLen = 200 + replacer := strings.NewReplacer( + "/", "／", "\\", "＼", ":", "：", "*", "＊", "?", "？", + "\"", "＂", "<", "＜", ">", "＞", "|", "｜", + "\n", " ", "\r", "", "\t", " ", "\x00", "", + ) + safe := replacer.Replace(strings.TrimSpace(name)) + safe = strings.Trim(safe, ".") + if len(safe) > maxLen { + safe = safe[:maxLen] + } + return safe } -// safeRedirectPolicy prevents HTTPS→HTTP downgrade, limits redirect count, -// and validates each redirect target against SSRF rules. +// safeRedirectPolicy prevents HTTPS→HTTP downgrade and validates redirect targets. func safeRedirectPolicy(req *http.Request, via []*http.Request) error { if len(via) >= maxDownloadRedirects { return fmt.Errorf("too many redirects") diff --git a/shortcuts/minutes/minutes_download_test.go b/shortcuts/minutes/minutes_download_test.go index 082e092b..44d2e311 100644 --- a/shortcuts/minutes/minutes_download_test.go +++ b/shortcuts/minutes/minutes_download_test.go @@ -117,16 +117,17 @@ func chdir(t *testing.T, dir string) { // Unit tests: resolveOutputFromResponse // --------------------------------------------------------------------------- -func TestResolveOutputFromResponse_ContentDisposition(t *testing.T) { +func TestResolveOutputFromResponse_FallbackToContentType(t *testing.T) { + // With nil runtime, fetchMinuteTitle returns "", so we fall back to token + Content-Type ext. resp := &http.Response{ Header: http.Header{ "Content-Disposition": []string{`attachment; filename="meeting_recording.mp4"`}, "Content-Type": []string{"video/mp4"}, }, } - got := resolveOutputFromResponse(resp, "tok001") - if got != "meeting_recording.mp4" { - t.Errorf("expected Content-Disposition filename, got %q", got) + got := resolveOutputFromResponse(context.Background(), nil, resp, "tok001") + if got != "tok001.mp4" { + t.Errorf("expected token + Content-Type ext %q, got %q", "tok001.mp4", got) } } @@ -136,7 +137,7 @@ func TestResolveOutputFromResponse_ContentType(t *testing.T) { "Content-Type": []string{"video/mp4"}, }, } - got := resolveOutputFromResponse(resp, "tok001") + got := resolveOutputFromResponse(context.Background(), nil, resp, "tok001") if !strings.HasPrefix(got, "tok001") { t.Errorf("expected token prefix, got %q", got) } @@ -147,7 +148,7 @@ func TestResolveOutputFromResponse_ContentType(t *testing.T) { func TestResolveOutputFromResponse_Fallback(t *testing.T) { resp := &http.Response{Header: http.Header{}} - got := resolveOutputFromResponse(resp, "tok001") + got := resolveOutputFromResponse(context.Background(), nil, resp, "tok001") if got != "tok001.media" { t.Errorf("expected fallback %q, got %q", "tok001.media", got) } @@ -160,7 +161,7 @@ func TestResolveOutputFromResponse_InvalidContentDisposition(t *testing.T) { "Content-Type": []string{"audio/mpeg"}, }, } - got := resolveOutputFromResponse(resp, "tok001") + got := resolveOutputFromResponse(context.Background(), nil, resp, "tok001") if !strings.HasPrefix(got, "tok001") { t.Errorf("expected token prefix from Content-Type fallback, got %q", got) } @@ -173,7 +174,7 @@ func TestResolveOutputFromResponse_EmptyDispositionFilename(t *testing.T) { "Content-Type": []string{"video/mp4"}, }, } - got := resolveOutputFromResponse(resp, "tok001") + got := resolveOutputFromResponse(context.Background(), nil, resp, "tok001") if got == "" { t.Error("expected non-empty filename") } @@ -234,13 +235,16 @@ func TestDownload_Validation_NoFlags(t *testing.T) { } } -func TestDownload_Validation_BothFlags(t *testing.T) { +func TestDownload_Validation_InvalidToken(t *testing.T) { f, _, _, _ := cmdutil.TestFactory(t, defaultConfig()) err := mountAndRun(t, MinutesDownload, []string{ - "+download", "--minute-token", "t1", "--minute-tokens", "t2", "--as", "user", + "+download", "--minute-tokens", "obcn***invalid", "--as", "user", }, f, nil) if err == nil { - t.Fatal("expected validation error for both flags") + t.Fatal("expected validation error for invalid token") + } + if !strings.Contains(err.Error(), "invalid minute token") { + t.Errorf("expected 'invalid minute token' error, got: %v", err) } } @@ -261,7 +265,7 @@ func TestDownload_Validation_OutputWithBatch(t *testing.T) { func TestDownload_DryRun(t *testing.T) { f, stdout, _, _ := cmdutil.TestFactory(t, defaultConfig()) err := mountAndRun(t, MinutesDownload, []string{ - "+download", "--minute-token", "tok001", "--dry-run", "--as", "user", + "+download", "--minute-tokens", "tok001", "--dry-run", "--as", "user", }, f, stdout) if err != nil { t.Fatalf("unexpected error: %v", err) @@ -280,7 +284,7 @@ func TestDownload_UrlOnly(t *testing.T) { reg.Register(mediaStub("tok001", "https://example.com/presigned/download")) err := mountAndRun(t, MinutesDownload, []string{ - "+download", "--minute-token", "tok001", "--url-only", "--as", "bot", + "+download", "--minute-tokens", "tok001", "--url-only", "--as", "bot", }, f, stdout) if err != nil { t.Fatalf("unexpected error: %v", err) @@ -298,7 +302,7 @@ func TestDownload_FullDownload(t *testing.T) { reg.Register(downloadStub("example.com/presigned/download", []byte("fake-video-content"), "video/mp4")) err := mountAndRun(t, MinutesDownload, []string{ - "+download", "--minute-token", "tok001", "--output", "output.mp4", "--as", "bot", + "+download", "--minute-tokens", "tok001", "--output", "output.mp4", "--as", "bot", }, f, stdout) if err != nil { t.Fatalf("unexpected error: %v", err) @@ -322,7 +326,7 @@ func TestDownload_OverwriteProtection(t *testing.T) { reg.Register(downloadStub("example.com/presigned/download", []byte("new-content"), "video/mp4")) err := mountAndRun(t, MinutesDownload, []string{ - "+download", "--minute-token", "tok001", "--output", "existing.mp4", "--as", "bot", + "+download", "--minute-tokens", "tok001", "--output", "existing.mp4", "--as", "bot", }, f, nil) if err == nil { t.Fatal("expected error for existing file without --overwrite") @@ -349,7 +353,7 @@ func TestDownload_HttpError(t *testing.T) { }) err := mountAndRun(t, MinutesDownload, []string{ - "+download", "--minute-token", "tok001", "--output", "output.mp4", "--as", "bot", + "+download", "--minute-tokens", "tok001", "--output", "output.mp4", "--as", "bot", }, f, nil) if err == nil { t.Fatal("expected error for HTTP 403") diff --git a/skills/lark-minutes/SKILL.md b/skills/lark-minutes/SKILL.md index 7cf250c1..eb7962c8 100644 --- a/skills/lark-minutes/SKILL.md +++ b/skills/lark-minutes/SKILL.md @@ -62,10 +62,13 @@ Shortcut 是对常用操作的高级封装（`lark-cli minutes +<verb> [flags]` ```bash # 下载音视频文件到本地 -lark-cli minutes +download --minute-token obcnq3b9jl72l83w4f149w9c --output ./meeting.mp4 +lark-cli minutes +download --minute-tokens obcnq3b9jl72l83w4f149w9c --output ./meeting.mp4 # 仅获取下载链接 -lark-cli minutes +download --minute-token obcnq3b9jl72l83w4f149w9c --url-only +lark-cli minutes +download --minute-tokens obcnq3b9jl72l83w4f149w9c --url-only + +# 批量下载 +lark-cli minutes +download --minute-tokens obcnq3b9jl72l83w4f149w9c,obcnexa7814k4t41c446fzwj ``` <!-- AUTO-GENERATED-START — gen-skills.py 管理，勿手动编辑 --> diff --git a/skills/lark-minutes/references/lark-minutes-download.md b/skills/lark-minutes/references/lark-minutes-download.md index a3fecc04..0be22244 100644 --- a/skills/lark-minutes/references/lark-minutes-download.md +++ b/skills/lark-minutes/references/lark-minutes-download.md @@ -10,25 +10,32 @@ ## 命令 ```bash -# 下载音视频文件到本地 -lark-cli minutes +download --minute-token obcnq3b9jl72l83w4f149w9c --output ./meeting.mp4 +# 下载单个妙记的音视频文件 +lark-cli minutes +download --minute-tokens obcnq3b9jl72l83w4f149w9c + +# 指定输出路径 +lark-cli minutes +download --minute-tokens obcnq3b9jl72l83w4f149w9c --output ./meeting.mp4 # 仅获取下载链接（有效期 1 天），不下载文件 -lark-cli minutes +download --minute-token obcnq3b9jl72l83w4f149w9c --url-only +lark-cli minutes +download --minute-tokens obcnq3b9jl72l83w4f149w9c --url-only + +# 批量下载多个妙记 +lark-cli minutes +download --minute-tokens obcnq3b9jl72l83w4f149w9c,obcnexa7814k4t41c446fzwj -# 覆盖已存在的本地文件 -lark-cli minutes +download --minute-token obcnq3b9jl72l83w4f149w9c --output ./meeting.mp4 --overwrite +# 批量下载到指定目录 +lark-cli minutes +download --minute-tokens obcnq3b9jl72l83w4f149w9c,obcnexa7814k4t41c446fzwj --output-dir ./downloads # 预览 API 调用 -lark-cli minutes +download --minute-token obcnq3b9jl72l83w4f149w9c --dry-run +lark-cli minutes +download --minute-tokens obcnq3b9jl72l83w4f149w9c --dry-run ``` ## 参数 | 参数 | 必填 | 说明 | |------|------|------| -| `--minute-token <token>` | 是 | 妙记 Token，从妙记 URL 末尾提取（24 位字符串） | -| `--output <path>` | 否 | 本地保存路径（默认使用妙记原始标题，如 `Office Oncall流程2.0宣讲.mp4`） | +| `--minute-tokens <tokens>` | 是 | 妙记 Token，逗号分隔支持批量（最多 50 个） | +| `--output <path>` | 否 | 本地保存路径（单个 token 时有效） | +| `--output-dir <dir>` | 否 | 批量下载时的输出目录（默认当前目录） | | `--overwrite` | 否 | 覆盖已存在的输出文件 | | `--url-only` | 否 | 仅返回下载链接，不下载文件 | | `--dry-run` | 否 | 预览 API 调用，不执行 | From cc47e8467f9c1223819495e33c0efdfae855262b Mon Sep 17 00:00:00 2001 From: renaocheng <renaocheng@bytedance.com> Date: Wed, 1 Apr 2026 15:36:08 +0800 Subject: [PATCH 13/17] =?UTF-8?q?fix:=20address=20PR=20review=20=E2=80=94?= =?UTF-8?q?=20download=20timeout,=20UTF-8=20truncation,=20concurrency=20sa?= =?UTF-8?q?fety,=20rate=20limiting,=20dedup=20robustness?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- shortcuts/minutes/minutes_download.go | 59 ++++++++++++++++------ shortcuts/minutes/minutes_download_test.go | 14 ++--- 2 files changed, 51 insertions(+), 22 deletions(-) diff --git a/shortcuts/minutes/minutes_download.go b/shortcuts/minutes/minutes_download.go index eecf7416..725f0cea 100644 --- a/shortcuts/minutes/minutes_download.go +++ b/shortcuts/minutes/minutes_download.go @@ -14,6 +14,7 @@ import ( "regexp" "strings" "sync" + "time" "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/internal/validate" @@ -22,7 +23,8 @@ import ( const ( // disableClientTimeout removes the global 30s client timeout for large media downloads. - // The request is still bounded by the caller's context. + // The download is bounded by the caller's context (e.g. Ctrl+C). A fixed timeout + // would cut off legitimate large file transfers. disableClientTimeout = 0 maxBatchSize = 50 @@ -113,11 +115,18 @@ func executeSingle(ctx context.Context, runtime *common.RuntimeContext, minuteTo return nil } + // pre-fetch title before download (RuntimeContext access stays sequential) + var title string + if outputPath == "" { + title = fetchMinuteTitle(ctx, runtime, minuteToken) + } + fmt.Fprintf(runtime.IO().ErrOut, "Downloading media: %s\n", common.MaskToken(minuteToken)) result, err := downloadMediaFile(ctx, runtime, downloadURL, minuteToken, downloadOpts{ outputPath: outputPath, overwrite: overwrite, + title: title, }) if err != nil { return err @@ -151,16 +160,28 @@ func executeBatch(ctx context.Context, runtime *common.RuntimeContext, tokens [] results := make([]batchResult, len(tokens)) - // Phase 1: fetch all download URLs sequentially + // Phase 1: sequentially fetch download URLs and titles (RuntimeContext is not concurrency-safe) type downloadTask struct { index int token string downloadURL string + title string // pre-fetched title for filename resolution } var tasks []downloadTask seen := make(map[string]int) + ticker := time.NewTicker(time.Second / 5) // rate-limit to 5 req/s + defer ticker.Stop() + for i, token := range tokens { + if i > 0 { + select { + case <-ctx.Done(): + return ctx.Err() + case <-ticker.C: + } + } + if err := ctx.Err(); err != nil { return err } @@ -186,7 +207,8 @@ func executeBatch(ctx context.Context, runtime *common.RuntimeContext, tokens [] continue } - tasks = append(tasks, downloadTask{index: i, token: token, downloadURL: downloadURL}) + title := fetchMinuteTitle(ctx, runtime, token) + tasks = append(tasks, downloadTask{index: i, token: token, downloadURL: downloadURL, title: title}) } // Phase 2: download files concurrently @@ -211,6 +233,7 @@ func executeBatch(ctx context.Context, runtime *common.RuntimeContext, tokens [] outputDir: outputDir, overwrite: overwrite, usedNames: &usedNames, + title: task.title, }) if err != nil { results[task.index] = batchResult{MinuteToken: task.token, Error: err.Error()} @@ -259,7 +282,7 @@ func fetchDownloadURL(ctx context.Context, runtime *common.RuntimeContext, minut return downloadURL, nil } -// deduplicateFilename ensures uniqueness by appending a token suffix on collision. +// deduplicateFilename ensures uniqueness by probing with token suffix and counter. func deduplicateFilename(name, minuteToken string, usedNames *sync.Map) string { if _, loaded := usedNames.LoadOrStore(name, true); !loaded { return name @@ -270,9 +293,15 @@ func deduplicateFilename(name, minuteToken string, usedNames *sync.Map) string { if len(suffix) > tokenSuffixLen { suffix = suffix[:tokenSuffixLen] } - deduped := base + "_" + suffix + ext - usedNames.Store(deduped, true) - return deduped + for i := 0; ; i++ { + candidate := base + "_" + suffix + ext + if i > 0 { + candidate = fmt.Sprintf("%s_%s_%d%s", base, suffix, i+1, ext) + } + if _, loaded := usedNames.LoadOrStore(candidate, true); !loaded { + return candidate + } + } } type downloadResult struct { @@ -285,6 +314,7 @@ type downloadOpts struct { outputDir string // output directory prefix (batch mode) overwrite bool usedNames *sync.Map // filename dedup table for batch mode + title string // pre-fetched minute title (avoids RuntimeContext in concurrent path) } // downloadMediaFile streams a media file from a pre-signed URL to disk. @@ -324,7 +354,7 @@ func downloadMediaFile(ctx context.Context, runtime *common.RuntimeContext, down outputPath := opts.outputPath if outputPath == "" { - filename := resolveOutputFromResponse(ctx, runtime, resp, minuteToken) + filename := resolveOutputFilename(resp, minuteToken, opts.title) if opts.usedNames != nil { filename = deduplicateFilename(filename, minuteToken, opts.usedNames) } @@ -349,12 +379,9 @@ func downloadMediaFile(ctx context.Context, runtime *common.RuntimeContext, down return &downloadResult{savedPath: safePath, sizeBytes: sizeBytes}, nil } -// resolveOutputFromResponse derives the output filename from the minute title (via API) -// and Content-Type extension. Content-Disposition is not used for the filename because -// the server may alter the original title (e.g. replacing "|" with "_"). -// Falls back to <token>.media when both title and Content-Type are unavailable. -func resolveOutputFromResponse(ctx context.Context, runtime *common.RuntimeContext, resp *http.Response, minuteToken string) string { - title := fetchMinuteTitle(ctx, runtime, minuteToken) +// resolveOutputFilename derives the output filename from the pre-fetched title and +// Content-Type extension. Falls back to <token>.media when both are unavailable. +func resolveOutputFilename(resp *http.Response, minuteToken, title string) string { ext := extFromContentType(resp.Header.Get("Content-Type")) if title != "" { @@ -427,8 +454,8 @@ func sanitizeFileName(name string) string { ) safe := replacer.Replace(strings.TrimSpace(name)) safe = strings.Trim(safe, ".") - if len(safe) > maxLen { - safe = safe[:maxLen] + if len([]rune(safe)) > maxLen { + safe = string([]rune(safe)[:maxLen]) } return safe } diff --git a/shortcuts/minutes/minutes_download_test.go b/shortcuts/minutes/minutes_download_test.go index 44d2e311..d0b88605 100644 --- a/shortcuts/minutes/minutes_download_test.go +++ b/shortcuts/minutes/minutes_download_test.go @@ -125,7 +125,7 @@ func TestResolveOutputFromResponse_FallbackToContentType(t *testing.T) { "Content-Type": []string{"video/mp4"}, }, } - got := resolveOutputFromResponse(context.Background(), nil, resp, "tok001") + got := resolveOutputFilename(resp, "tok001", "") if got != "tok001.mp4" { t.Errorf("expected token + Content-Type ext %q, got %q", "tok001.mp4", got) } @@ -137,7 +137,7 @@ func TestResolveOutputFromResponse_ContentType(t *testing.T) { "Content-Type": []string{"video/mp4"}, }, } - got := resolveOutputFromResponse(context.Background(), nil, resp, "tok001") + got := resolveOutputFilename(resp, "tok001", "") if !strings.HasPrefix(got, "tok001") { t.Errorf("expected token prefix, got %q", got) } @@ -148,7 +148,7 @@ func TestResolveOutputFromResponse_ContentType(t *testing.T) { func TestResolveOutputFromResponse_Fallback(t *testing.T) { resp := &http.Response{Header: http.Header{}} - got := resolveOutputFromResponse(context.Background(), nil, resp, "tok001") + got := resolveOutputFilename(resp, "tok001", "") if got != "tok001.media" { t.Errorf("expected fallback %q, got %q", "tok001.media", got) } @@ -161,7 +161,7 @@ func TestResolveOutputFromResponse_InvalidContentDisposition(t *testing.T) { "Content-Type": []string{"audio/mpeg"}, }, } - got := resolveOutputFromResponse(context.Background(), nil, resp, "tok001") + got := resolveOutputFilename(resp, "tok001", "") if !strings.HasPrefix(got, "tok001") { t.Errorf("expected token prefix from Content-Type fallback, got %q", got) } @@ -174,7 +174,7 @@ func TestResolveOutputFromResponse_EmptyDispositionFilename(t *testing.T) { "Content-Type": []string{"video/mp4"}, }, } - got := resolveOutputFromResponse(context.Background(), nil, resp, "tok001") + got := resolveOutputFilename(resp, "tok001", "") if got == "" { t.Error("expected non-empty filename") } @@ -319,7 +319,9 @@ func TestDownload_FullDownload(t *testing.T) { func TestDownload_OverwriteProtection(t *testing.T) { chdir(t, t.TempDir()) - os.WriteFile("existing.mp4", []byte("old"), 0644) + if err := os.WriteFile("existing.mp4", []byte("old"), 0644); err != nil { + t.Fatalf("setup failed: %v", err) + } f, _, _, reg := cmdutil.TestFactory(t, defaultConfig()) reg.Register(mediaStub("tok001", "https://example.com/presigned/download")) From 0f6f41f128b9e6d8f2d277d6c02a05b4d5e12790 Mon Sep 17 00:00:00 2001 From: renaocheng <renaocheng@bytedance.com> Date: Thu, 2 Apr 2026 11:06:01 +0800 Subject: [PATCH 14/17] =?UTF-8?q?refactor:=20simplify=20+download=20?= =?UTF-8?q?=E2=80=94=20unify=20single/batch=20loop,=20remove=20parallel=20?= =?UTF-8?q?download,=20merge=20output=20flags?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- shortcuts/minutes/minutes_download.go | 405 ++++++------------ shortcuts/minutes/minutes_download_test.go | 73 +--- .../references/lark-minutes-download.md | 7 +- 3 files changed, 139 insertions(+), 346 deletions(-) diff --git a/shortcuts/minutes/minutes_download.go b/shortcuts/minutes/minutes_download.go index 725f0cea..9ea46d1d 100644 --- a/shortcuts/minutes/minutes_download.go +++ b/shortcuts/minutes/minutes_download.go @@ -13,7 +13,6 @@ import ( "path/filepath" "regexp" "strings" - "sync" "time" "github.com/larksuite/cli/internal/output" @@ -27,10 +26,8 @@ const ( // would cut off legitimate large file transfers. disableClientTimeout = 0 - maxBatchSize = 50 - maxConcurrentDownloads = 3 - maxDownloadRedirects = 5 - tokenSuffixLen = 6 + maxBatchSize = 50 + maxDownloadRedirects = 5 ) // validMinuteToken matches minute tokens: lowercase alphanumeric characters only. @@ -46,8 +43,7 @@ var MinutesDownload = common.Shortcut{ HasFormat: true, Flags: []common.Flag{ {Name: "minute-tokens", Desc: "minute tokens, comma-separated for batch download (max 50)", Required: true}, - {Name: "output", Desc: "local save path (single token only)"}, - {Name: "output-dir", Desc: "output directory for batch download (default: current dir)"}, + {Name: "output", Desc: "output path: file path for single token, directory for batch (default: current dir)"}, {Name: "overwrite", Type: "bool", Desc: "overwrite existing output file"}, {Name: "url-only", Type: "bool", Desc: "only print the download URL(s) without downloading"}, }, @@ -64,207 +60,116 @@ var MinutesDownload = common.Shortcut{ return output.ErrValidation("invalid minute token %q: must contain only lowercase alphanumeric characters (e.g. obcnq3b9jl72l83w4f149w9c)", token) } } - if len(tokens) > 1 && runtime.Str("output") != "" { - return output.ErrValidation("--output cannot be used with multiple tokens; use --output-dir instead") - } - if outDir := runtime.Str("output-dir"); outDir != "" { - if err := common.ValidateSafeOutputDir(outDir); err != nil { - return err - } - } return nil }, DryRun: func(ctx context.Context, runtime *common.RuntimeContext) *common.DryRunAPI { tokens := common.SplitCSV(runtime.Str("minute-tokens")) - api := common.NewDryRunAPI(). - GET("/open-apis/minutes/v1/minutes/:minute_token/media") - api.Set("minute_tokens", tokens) - if len(tokens) > 1 { - api.Set("concurrent_downloads", maxConcurrentDownloads) - } - return api + return common.NewDryRunAPI(). + GET("/open-apis/minutes/v1/minutes/:minute_token/media"). + Set("minute_tokens", tokens) }, Execute: func(ctx context.Context, runtime *common.RuntimeContext) error { tokens := common.SplitCSV(runtime.Str("minute-tokens")) - if len(tokens) == 1 { - return executeSingle(ctx, runtime, tokens[0]) + outputPath := runtime.Str("output") + overwrite := runtime.Bool("overwrite") + urlOnly := runtime.Bool("url-only") + errOut := runtime.IO().ErrOut + single := len(tokens) == 1 + + if !single { + fmt.Fprintf(errOut, "[minutes +download] batch: %d token(s)\n", len(tokens)) } - return executeBatch(ctx, runtime, tokens) - }, -} - -// executeSingle handles a single token download. -func executeSingle(ctx context.Context, runtime *common.RuntimeContext, minuteToken string) error { - outputPath := runtime.Str("output") - overwrite := runtime.Bool("overwrite") - urlOnly := runtime.Bool("url-only") - - if err := validate.ResourceName(minuteToken, "--minute-tokens"); err != nil { - return output.ErrValidation("%s", err) - } - - downloadURL, err := fetchDownloadURL(ctx, runtime, minuteToken) - if err != nil { - return err - } - - if urlOnly { - runtime.Out(map[string]interface{}{ - "download_url": downloadURL, - }, nil) - return nil - } - // pre-fetch title before download (RuntimeContext access stays sequential) - var title string - if outputPath == "" { - title = fetchMinuteTitle(ctx, runtime, minuteToken) - } - - fmt.Fprintf(runtime.IO().ErrOut, "Downloading media: %s\n", common.MaskToken(minuteToken)) - - result, err := downloadMediaFile(ctx, runtime, downloadURL, minuteToken, downloadOpts{ - outputPath: outputPath, - overwrite: overwrite, - title: title, - }) - if err != nil { - return err - } + type result struct { + MinuteToken string `json:"minute_token"` + SavedPath string `json:"saved_path,omitempty"` + SizeBytes int64 `json:"size_bytes,omitempty"` + DownloadURL string `json:"download_url,omitempty"` + Error string `json:"error,omitempty"` + } - runtime.Out(map[string]interface{}{ - "saved_path": result.savedPath, - "size_bytes": result.sizeBytes, - }, nil) - return nil -} + results := make([]result, len(tokens)) + seen := make(map[string]int) + ticker := time.NewTicker(time.Second / 5) // rate-limit to 5 req/s + defer ticker.Stop() + + for i, token := range tokens { + if i > 0 { + select { + case <-ctx.Done(): + return ctx.Err() + case <-ticker.C: + } + } -// executeBatch handles multiple tokens with concurrent downloads. -// Phase 1: sequentially fetch download URLs (RuntimeContext is not concurrency-safe). -// Phase 2: concurrently download files (HTTP client is concurrency-safe). -func executeBatch(ctx context.Context, runtime *common.RuntimeContext, tokens []string) error { - overwrite := runtime.Bool("overwrite") - urlOnly := runtime.Bool("url-only") - outputDir := runtime.Str("output-dir") - errOut := runtime.IO().ErrOut - - fmt.Fprintf(errOut, "[minutes +download] batch: %d token(s), concurrency=%d\n", len(tokens), maxConcurrentDownloads) - - type batchResult struct { - MinuteToken string `json:"minute_token"` - SavedPath string `json:"saved_path,omitempty"` - SizeBytes int64 `json:"size_bytes,omitempty"` - DownloadURL string `json:"download_url,omitempty"` - Error string `json:"error,omitempty"` - } + if err := validate.ResourceName(token, "--minute-tokens"); err != nil { + results[i] = result{MinuteToken: token, Error: err.Error()} + continue + } + if firstIdx, dup := seen[token]; dup { + results[i] = result{MinuteToken: token, Error: fmt.Sprintf("duplicate token, same as index %d", firstIdx)} + continue + } + seen[token] = i - results := make([]batchResult, len(tokens)) + downloadURL, err := fetchDownloadURL(ctx, runtime, token) + if err != nil { + results[i] = result{MinuteToken: token, Error: err.Error()} + continue + } - // Phase 1: sequentially fetch download URLs and titles (RuntimeContext is not concurrency-safe) - type downloadTask struct { - index int - token string - downloadURL string - title string // pre-fetched title for filename resolution - } - var tasks []downloadTask - seen := make(map[string]int) - - ticker := time.NewTicker(time.Second / 5) // rate-limit to 5 req/s - defer ticker.Stop() - - for i, token := range tokens { - if i > 0 { - select { - case <-ctx.Done(): - return ctx.Err() - case <-ticker.C: + if urlOnly { + results[i] = result{MinuteToken: token, DownloadURL: downloadURL} + continue } - } - if err := ctx.Err(); err != nil { - return err - } - if err := validate.ResourceName(token, "--minute-tokens"); err != nil { - results[i] = batchResult{MinuteToken: token, Error: err.Error()} - continue - } + fmt.Fprintf(errOut, "Downloading media: %s\n", common.MaskToken(token)) - if firstIdx, dup := seen[token]; dup { - results[i] = batchResult{MinuteToken: token, Error: fmt.Sprintf("duplicate token, same as index %d", firstIdx)} - continue - } - seen[token] = i + // single token: --output is a file path; batch: --output is a directory + opts := downloadOpts{overwrite: overwrite} + if single { + opts.outputPath = outputPath + } else { + opts.outputDir = outputPath + } - downloadURL, err := fetchDownloadURL(ctx, runtime, token) - if err != nil { - results[i] = batchResult{MinuteToken: token, Error: err.Error()} - continue + dl, err := downloadMediaFile(ctx, runtime, downloadURL, token, opts) + if err != nil { + results[i] = result{MinuteToken: token, Error: err.Error()} + continue + } + results[i] = result{MinuteToken: token, SavedPath: dl.savedPath, SizeBytes: dl.sizeBytes} } - if urlOnly { - results[i] = batchResult{MinuteToken: token, DownloadURL: downloadURL} - continue + // output + if single { + r := results[0] + if r.Error != "" { + return output.ErrAPI(0, r.Error, nil) + } + if urlOnly { + runtime.Out(map[string]interface{}{"download_url": r.DownloadURL}, nil) + } else { + runtime.Out(map[string]interface{}{"saved_path": r.SavedPath, "size_bytes": r.SizeBytes}, nil) + } + return nil } - title := fetchMinuteTitle(ctx, runtime, token) - tasks = append(tasks, downloadTask{index: i, token: token, downloadURL: downloadURL, title: title}) - } - - // Phase 2: download files concurrently - if len(tasks) > 0 { - var usedNames sync.Map - var logMu sync.Mutex - var wg sync.WaitGroup - sem := make(chan struct{}, maxConcurrentDownloads) - - for _, task := range tasks { - wg.Add(1) - go func(task downloadTask) { - defer wg.Done() - sem <- struct{}{} - defer func() { <-sem }() - - logMu.Lock() - fmt.Fprintf(errOut, "[minutes +download] downloading: %s\n", common.MaskToken(task.token)) - logMu.Unlock() - - result, err := downloadMediaFile(ctx, runtime, task.downloadURL, task.token, downloadOpts{ - outputDir: outputDir, - overwrite: overwrite, - usedNames: &usedNames, - title: task.title, - }) - if err != nil { - results[task.index] = batchResult{MinuteToken: task.token, Error: err.Error()} - return - } - - results[task.index] = batchResult{ - MinuteToken: task.token, - SavedPath: result.savedPath, - SizeBytes: result.sizeBytes, - } - }(task) + // batch output + successCount := 0 + for _, r := range results { + if r.Error == "" { + successCount++ + } } - wg.Wait() - } + fmt.Fprintf(errOut, "[minutes +download] done: %d total, %d succeeded, %d failed\n", len(results), successCount, len(results)-successCount) - successCount := 0 - for _, r := range results { - if r.Error == "" { - successCount++ + runtime.OutFormat(map[string]interface{}{"downloads": results}, &output.Meta{Count: len(results)}, nil) + if successCount == 0 && len(results) > 0 { + return output.ErrAPI(0, fmt.Sprintf("all %d downloads failed", len(results)), nil) } - } - fmt.Fprintf(errOut, "[minutes +download] done: %d total, %d succeeded, %d failed\n", len(results), successCount, len(results)-successCount) - - outData := map[string]interface{}{"downloads": results} - runtime.OutFormat(outData, &output.Meta{Count: len(results)}, nil) - - if successCount == 0 && len(results) > 0 { - return output.ErrAPI(0, fmt.Sprintf("all %d downloads failed", len(results)), nil) - } - return nil + return nil + }, } // fetchDownloadURL retrieves the pre-signed download URL for a minute token. @@ -282,43 +187,19 @@ func fetchDownloadURL(ctx context.Context, runtime *common.RuntimeContext, minut return downloadURL, nil } -// deduplicateFilename ensures uniqueness by probing with token suffix and counter. -func deduplicateFilename(name, minuteToken string, usedNames *sync.Map) string { - if _, loaded := usedNames.LoadOrStore(name, true); !loaded { - return name - } - ext := filepath.Ext(name) - base := strings.TrimSuffix(name, ext) - suffix := minuteToken - if len(suffix) > tokenSuffixLen { - suffix = suffix[:tokenSuffixLen] - } - for i := 0; ; i++ { - candidate := base + "_" + suffix + ext - if i > 0 { - candidate = fmt.Sprintf("%s_%s_%d%s", base, suffix, i+1, ext) - } - if _, loaded := usedNames.LoadOrStore(candidate, true); !loaded { - return candidate - } - } -} - type downloadResult struct { savedPath string sizeBytes int64 } type downloadOpts struct { - outputPath string // explicit output path (single mode) - outputDir string // output directory prefix (batch mode) + outputPath string // explicit output file path (single mode only) + outputDir string // output directory (batch mode) overwrite bool - usedNames *sync.Map // filename dedup table for batch mode - title string // pre-fetched minute title (avoids RuntimeContext in concurrent path) } // downloadMediaFile streams a media file from a pre-signed URL to disk. -// Output path resolution: opts.outputPath > Content-Disposition > Content-Type extension > <token>.media. +// Filename resolution: opts.outputPath > Content-Disposition filename > Content-Type ext > <token>.media. func downloadMediaFile(ctx context.Context, runtime *common.RuntimeContext, downloadURL, minuteToken string, opts downloadOpts) (*downloadResult, error) { if err := validate.ValidateDownloadSourceURL(ctx, downloadURL); err != nil { return nil, output.ErrValidation("blocked download URL: %s", err) @@ -329,9 +210,21 @@ func downloadMediaFile(ctx context.Context, runtime *common.RuntimeContext, down return nil, output.ErrNetwork("failed to get HTTP client: %s", err) } + // Clone client with disabled timeout and redirect safety. downloadClient := *httpClient downloadClient.Timeout = disableClientTimeout - downloadClient.CheckRedirect = safeRedirectPolicy + downloadClient.CheckRedirect = func(req *http.Request, via []*http.Request) error { + if len(via) >= maxDownloadRedirects { + return fmt.Errorf("too many redirects") + } + if len(via) > 0 { + prev := via[len(via)-1] + if strings.EqualFold(prev.URL.Scheme, "https") && strings.EqualFold(req.URL.Scheme, "http") { + return fmt.Errorf("redirect from https to http is not allowed") + } + } + return validate.ValidateDownloadSourceURL(req.Context(), req.URL.String()) + } req, err := http.NewRequestWithContext(ctx, http.MethodGet, downloadURL, nil) if err != nil { @@ -344,7 +237,7 @@ func downloadMediaFile(ctx context.Context, runtime *common.RuntimeContext, down } defer resp.Body.Close() - if resp.StatusCode >= 400 { + if resp.StatusCode < 200 || resp.StatusCode >= 300 { body, _ := io.ReadAll(io.LimitReader(resp.Body, 4096)) if len(body) > 0 { return nil, output.ErrNetwork("download failed: HTTP %d: %s", resp.StatusCode, strings.TrimSpace(string(body))) @@ -352,12 +245,10 @@ func downloadMediaFile(ctx context.Context, runtime *common.RuntimeContext, down return nil, output.ErrNetwork("download failed: HTTP %d", resp.StatusCode) } + // resolve output path outputPath := opts.outputPath if outputPath == "" { - filename := resolveOutputFilename(resp, minuteToken, opts.title) - if opts.usedNames != nil { - filename = deduplicateFilename(filename, minuteToken, opts.usedNames) - } + filename := resolveFilenameFromResponse(resp, minuteToken) outputPath = filepath.Join(opts.outputDir, filename) } @@ -368,57 +259,35 @@ func downloadMediaFile(ctx context.Context, runtime *common.RuntimeContext, down if err := common.EnsureWritableFile(safePath, opts.overwrite); err != nil { return nil, err } - if err := os.MkdirAll(filepath.Dir(safePath), 0755); err != nil { + if err := os.MkdirAll(filepath.Dir(safePath), 0700); err != nil { return nil, output.Errorf(output.ExitInternal, "api_error", "cannot create parent directory: %s", err) } - sizeBytes, err := validate.AtomicWriteFromReader(safePath, resp.Body, 0644) + sizeBytes, err := validate.AtomicWriteFromReader(safePath, resp.Body, 0600) if err != nil { return nil, output.Errorf(output.ExitInternal, "api_error", "cannot create file: %s", err) } return &downloadResult{savedPath: safePath, sizeBytes: sizeBytes}, nil } -// resolveOutputFilename derives the output filename from the pre-fetched title and -// Content-Type extension. Falls back to <token>.media when both are unavailable. -func resolveOutputFilename(resp *http.Response, minuteToken, title string) string { - ext := extFromContentType(resp.Header.Get("Content-Type")) - - if title != "" { - name := sanitizeFileName(title) - if name == "" { - name = minuteToken - } - if ext != "" { - return name + ext +// resolveFilenameFromResponse derives the filename from HTTP response headers. +// Priority: Content-Disposition filename > Content-Type extension > <token>.media. +func resolveFilenameFromResponse(resp *http.Response, minuteToken string) string { + if cd := resp.Header.Get("Content-Disposition"); cd != "" { + if _, params, err := mime.ParseMediaType(cd); err == nil { + if filename := params["filename"]; filename != "" { + return filename + } } - return name + ".media" } - if ext != "" { + if ext := extFromContentType(resp.Header.Get("Content-Type")); ext != "" { return minuteToken + ext } return minuteToken + ".media" } -// fetchMinuteTitle retrieves the minute title via minutes get API. Returns "" on failure. -func fetchMinuteTitle(ctx context.Context, runtime *common.RuntimeContext, minuteToken string) string { - if runtime == nil { - return "" - } - data, err := runtime.DoAPIJSON(http.MethodGet, - fmt.Sprintf("/open-apis/minutes/v1/minutes/%s", validate.EncodePathSegment(minuteToken)), - nil, nil) - if err != nil { - return "" - } - if minute, ok := data["minute"].(map[string]interface{}); ok { - return common.GetString(minute, "title") - } - return "" -} - // preferredExt overrides Go's mime.ExtensionsByType which returns alphabetically sorted -// results (e.g. .m4v before .mp4 for video/mp4). Map the most common extensions here. +// results (e.g. .m4v before .mp4 for video/mp4). var preferredExt = map[string]string{ "video/mp4": ".mp4", "audio/mp4": ".m4a", @@ -442,37 +311,3 @@ func extFromContentType(contentType string) string { } return "" } - -// sanitizeFileName replaces filesystem-unsafe characters with visually similar fullwidth -// Unicode equivalents so the filename stays readable and cross-platform safe. -func sanitizeFileName(name string) string { - const maxLen = 200 - replacer := strings.NewReplacer( - "/", "／", "\\", "＼", ":", "：", "*", "＊", "?", "？", - "\"", "＂", "<", "＜", ">", "＞", "|", "｜", - "\n", " ", "\r", "", "\t", " ", "\x00", "", - ) - safe := replacer.Replace(strings.TrimSpace(name)) - safe = strings.Trim(safe, ".") - if len([]rune(safe)) > maxLen { - safe = string([]rune(safe)[:maxLen]) - } - return safe -} - -// safeRedirectPolicy prevents HTTPS→HTTP downgrade and validates redirect targets. -func safeRedirectPolicy(req *http.Request, via []*http.Request) error { - if len(via) >= maxDownloadRedirects { - return fmt.Errorf("too many redirects") - } - if len(via) > 0 { - prev := via[len(via)-1] - if strings.EqualFold(prev.URL.Scheme, "https") && strings.EqualFold(req.URL.Scheme, "http") { - return fmt.Errorf("redirect from https to http is not allowed") - } - } - if err := validate.ValidateDownloadSourceURL(req.Context(), req.URL.String()); err != nil { - return fmt.Errorf("blocked redirect target: %w", err) - } - return nil -} diff --git a/shortcuts/minutes/minutes_download_test.go b/shortcuts/minutes/minutes_download_test.go index d0b88605..a7b4368b 100644 --- a/shortcuts/minutes/minutes_download_test.go +++ b/shortcuts/minutes/minutes_download_test.go @@ -100,7 +100,7 @@ func downloadStub(url string, body []byte, contentType string) *httpmock.Stub { } } -// chdir 切换工作目录并在测试结束后恢复 +// chdir changes the working directory and restores it when the test finishes. func chdir(t *testing.T, dir string) { t.Helper() orig, err := os.Getwd() @@ -117,27 +117,26 @@ func chdir(t *testing.T, dir string) { // Unit tests: resolveOutputFromResponse // --------------------------------------------------------------------------- -func TestResolveOutputFromResponse_FallbackToContentType(t *testing.T) { - // With nil runtime, fetchMinuteTitle returns "", so we fall back to token + Content-Type ext. +func TestResolveFilenameFromResponse_ContentDisposition(t *testing.T) { resp := &http.Response{ Header: http.Header{ "Content-Disposition": []string{`attachment; filename="meeting_recording.mp4"`}, "Content-Type": []string{"video/mp4"}, }, } - got := resolveOutputFilename(resp, "tok001", "") - if got != "tok001.mp4" { - t.Errorf("expected token + Content-Type ext %q, got %q", "tok001.mp4", got) + got := resolveFilenameFromResponse(resp, "tok001") + if got != "meeting_recording.mp4" { + t.Errorf("expected Content-Disposition filename, got %q", got) } } -func TestResolveOutputFromResponse_ContentType(t *testing.T) { +func TestResolveFilenameFromResponse_ContentType(t *testing.T) { resp := &http.Response{ Header: http.Header{ "Content-Type": []string{"video/mp4"}, }, } - got := resolveOutputFilename(resp, "tok001", "") + got := resolveFilenameFromResponse(resp, "tok001") if !strings.HasPrefix(got, "tok001") { t.Errorf("expected token prefix, got %q", got) } @@ -146,35 +145,35 @@ func TestResolveOutputFromResponse_ContentType(t *testing.T) { } } -func TestResolveOutputFromResponse_Fallback(t *testing.T) { +func TestResolveFilenameFromResponse_Fallback(t *testing.T) { resp := &http.Response{Header: http.Header{}} - got := resolveOutputFilename(resp, "tok001", "") + got := resolveFilenameFromResponse(resp, "tok001") if got != "tok001.media" { t.Errorf("expected fallback %q, got %q", "tok001.media", got) } } -func TestResolveOutputFromResponse_InvalidContentDisposition(t *testing.T) { +func TestResolveFilenameFromResponse_InvalidContentDisposition(t *testing.T) { resp := &http.Response{ Header: http.Header{ "Content-Disposition": []string{"invalid;;;"}, "Content-Type": []string{"audio/mpeg"}, }, } - got := resolveOutputFilename(resp, "tok001", "") + got := resolveFilenameFromResponse(resp, "tok001") if !strings.HasPrefix(got, "tok001") { t.Errorf("expected token prefix from Content-Type fallback, got %q", got) } } -func TestResolveOutputFromResponse_EmptyDispositionFilename(t *testing.T) { +func TestResolveFilenameFromResponse_EmptyDispositionFilename(t *testing.T) { resp := &http.Response{ Header: http.Header{ "Content-Disposition": []string{"attachment"}, "Content-Type": []string{"video/mp4"}, }, } - got := resolveOutputFilename(resp, "tok001", "") + got := resolveFilenameFromResponse(resp, "tok001") if got == "" { t.Error("expected non-empty filename") } @@ -183,46 +182,6 @@ func TestResolveOutputFromResponse_EmptyDispositionFilename(t *testing.T) { } } -// --------------------------------------------------------------------------- -// Unit tests: deduplicateFilename -// --------------------------------------------------------------------------- - -func TestDeduplicateFilename_NoDuplicate(t *testing.T) { - var usedNames sync.Map - got := deduplicateFilename("会议纪要.mp4", "tok001", &usedNames) - if got != "会议纪要.mp4" { - t.Errorf("expected original name, got %q", got) - } -} - -func TestDeduplicateFilename_Duplicate(t *testing.T) { - var usedNames sync.Map - first := deduplicateFilename("会议纪要.mp4", "tok001abc", &usedNames) - second := deduplicateFilename("会议纪要.mp4", "tok002xyz", &usedNames) - if first != "会议纪要.mp4" { - t.Errorf("first should be original, got %q", first) - } - if second == first { - t.Errorf("second should differ from first, both got %q", first) - } - // 第二个应该包含 token 前缀作为区分 - if !strings.Contains(second, "tok002") { - t.Errorf("deduplicated name should contain token prefix, got %q", second) - } - if !strings.HasSuffix(second, ".mp4") { - t.Errorf("deduplicated name should keep extension, got %q", second) - } -} - -func TestDeduplicateFilename_NoExtension(t *testing.T) { - var usedNames sync.Map - deduplicateFilename("recording", "tok001", &usedNames) - got := deduplicateFilename("recording", "tok002abc", &usedNames) - if !strings.Contains(got, "tok002") { - t.Errorf("expected token suffix, got %q", got) - } -} - // --------------------------------------------------------------------------- // Validation tests // --------------------------------------------------------------------------- @@ -402,7 +361,7 @@ func TestDownload_Batch_Download(t *testing.T) { t.Fatalf("unexpected error: %v", err) } - // 验证输出结构 + // verify output structure var result struct { Data struct { Downloads []struct { @@ -438,7 +397,7 @@ func TestDownload_Batch_PartialFailure(t *testing.T) { err := mountAndRun(t, MinutesDownload, []string{ "+download", "--minute-tokens", "tok001,tok002", "--as", "bot", }, f, stdout) - // partial failure 不报错 + // partial failure should not cause an overall error if err != nil { t.Fatalf("partial failure should not return error, got: %v", err) } @@ -450,7 +409,7 @@ func TestDownload_Batch_PartialFailure(t *testing.T) { func TestDownload_Batch_DuplicateToken(t *testing.T) { f, stdout, _, reg := cmdutil.TestFactory(t, defaultConfig()) - // 只注册一次 media stub — 去重后只调用一次 API + // register media stub only once — dedup means only one API call reg.Register(mediaStub("tok001", "https://example.com/download/1")) err := mountAndRun(t, MinutesDownload, []string{ diff --git a/skills/lark-minutes/references/lark-minutes-download.md b/skills/lark-minutes/references/lark-minutes-download.md index 0be22244..64402728 100644 --- a/skills/lark-minutes/references/lark-minutes-download.md +++ b/skills/lark-minutes/references/lark-minutes-download.md @@ -23,7 +23,7 @@ lark-cli minutes +download --minute-tokens obcnq3b9jl72l83w4f149w9c --url-only lark-cli minutes +download --minute-tokens obcnq3b9jl72l83w4f149w9c,obcnexa7814k4t41c446fzwj # 批量下载到指定目录 -lark-cli minutes +download --minute-tokens obcnq3b9jl72l83w4f149w9c,obcnexa7814k4t41c446fzwj --output-dir ./downloads +lark-cli minutes +download --minute-tokens obcnq3b9jl72l83w4f149w9c,obcnexa7814k4t41c446fzwj --output ./downloads # 预览 API 调用 lark-cli minutes +download --minute-tokens obcnq3b9jl72l83w4f149w9c --dry-run @@ -34,8 +34,7 @@ lark-cli minutes +download --minute-tokens obcnq3b9jl72l83w4f149w9c --dry-run | 参数 | 必填 | 说明 | |------|------|------| | `--minute-tokens <tokens>` | 是 | 妙记 Token，逗号分隔支持批量（最多 50 个） | -| `--output <path>` | 否 | 本地保存路径（单个 token 时有效） | -| `--output-dir <dir>` | 否 | 批量下载时的输出目录（默认当前目录） | +| `--output <path>` | 否 | 输出路径：单个 token 时为文件路径，批量时为目录（默认当前目录） | | `--overwrite` | 否 | 覆盖已存在的输出文件 | | `--url-only` | 否 | 仅返回下载链接，不下载文件 | | `--dry-run` | 否 | 预览 API 调用，不执行 | @@ -109,7 +108,7 @@ API 限流 5 次/秒，批量下载时需注意控制频率。 ## 提示 -- 音视频文件可能较大，下载超时默认为 5 分钟。 +- 音视频文件可能较大，下载无固定超时限制（由用户 Ctrl+C 控制取消）。 - 未指定 `--output` 时，默认使用妙记原始标题作为文件名（如 `Office Oncall流程2.0宣讲.mp4`）。 - 如需获取妙记的纪要内容（逐字稿、AI 总结等），请使用 [vc +notes](../../lark-vc/references/lark-vc-notes.md)。 From 812d542d47741468ad1bf42d2df104e084c0172c Mon Sep 17 00:00:00 2001 From: renaocheng <renaocheng@bytedance.com> Date: Thu, 2 Apr 2026 14:23:06 +0800 Subject: [PATCH 15/17] fix(minutes): deduplicate filenames in batch download by prefixing token on collision --- shortcuts/minutes/minutes_download.go | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/shortcuts/minutes/minutes_download.go b/shortcuts/minutes/minutes_download.go index 9ea46d1d..fb959e45 100644 --- a/shortcuts/minutes/minutes_download.go +++ b/shortcuts/minutes/minutes_download.go @@ -90,6 +90,7 @@ var MinutesDownload = common.Shortcut{ results := make([]result, len(tokens)) seen := make(map[string]int) + usedNames := make(map[string]bool) ticker := time.NewTicker(time.Second / 5) // rate-limit to 5 req/s defer ticker.Stop() @@ -126,7 +127,7 @@ var MinutesDownload = common.Shortcut{ fmt.Fprintf(errOut, "Downloading media: %s\n", common.MaskToken(token)) // single token: --output is a file path; batch: --output is a directory - opts := downloadOpts{overwrite: overwrite} + opts := downloadOpts{overwrite: overwrite, usedNames: usedNames} if single { opts.outputPath = outputPath } else { @@ -193,9 +194,10 @@ type downloadResult struct { } type downloadOpts struct { - outputPath string // explicit output file path (single mode only) - outputDir string // output directory (batch mode) + outputPath string // explicit output file path (single mode only) + outputDir string // output directory (batch mode) overwrite bool + usedNames map[string]bool // tracks used filenames to deduplicate in batch mode } // downloadMediaFile streams a media file from a pre-signed URL to disk. @@ -249,6 +251,13 @@ func downloadMediaFile(ctx context.Context, runtime *common.RuntimeContext, down outputPath := opts.outputPath if outputPath == "" { filename := resolveFilenameFromResponse(resp, minuteToken) + // Deduplicate filenames in batch mode: prefix with token on collision. + if opts.usedNames != nil { + if opts.usedNames[filename] { + filename = minuteToken + "-" + filename + } + opts.usedNames[filename] = true + } outputPath = filepath.Join(opts.outputDir, filename) } From 23e3a4768170ca92e605e54dc44862e08f0148b7 Mon Sep 17 00:00:00 2001 From: renaocheng <renaocheng@bytedance.com> Date: Thu, 2 Apr 2026 14:30:27 +0800 Subject: [PATCH 16/17] fix(minutes): fix gofmt alignment in downloadOpts struct --- shortcuts/minutes/minutes_download.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/shortcuts/minutes/minutes_download.go b/shortcuts/minutes/minutes_download.go index fb959e45..87d6bd3e 100644 --- a/shortcuts/minutes/minutes_download.go +++ b/shortcuts/minutes/minutes_download.go @@ -194,8 +194,8 @@ type downloadResult struct { } type downloadOpts struct { - outputPath string // explicit output file path (single mode only) - outputDir string // output directory (batch mode) + outputPath string // explicit output file path (single mode only) + outputDir string // output directory (batch mode) overwrite bool usedNames map[string]bool // tracks used filenames to deduplicate in batch mode } From 6637afc506db016d2b9105c70682fef6e3ff828b Mon Sep 17 00:00:00 2001 From: renaocheng <renaocheng@bytedance.com> Date: Thu, 2 Apr 2026 14:46:25 +0800 Subject: [PATCH 17/17] fix(minutes): add transport-level SSRF protection and batch output validation --- internal/validate/url.go | 19 +++++++ shortcuts/minutes/minutes_download.go | 65 ++++++++++++++-------- shortcuts/minutes/minutes_download_test.go | 2 +- 3 files changed, 61 insertions(+), 25 deletions(-) diff --git a/internal/validate/url.go b/internal/validate/url.go index 6d6ab0c4..e25df97b 100644 --- a/internal/validate/url.go +++ b/internal/validate/url.go @@ -181,6 +181,25 @@ func cloneDownloadTransport(base http.RoundTripper) *http.Transport { return cloned } +// DialContextFunc is the signature for DialContext / DialTLSContext. +type DialContextFunc func(ctx context.Context, network, addr string) (net.Conn, error) + +// WrapDialContextWithIPCheck wraps a DialContext function to validate the +// remote IP after connection, rejecting local/internal addresses (SSRF protection). +func WrapDialContextWithIPCheck(origDial DialContextFunc) DialContextFunc { + return func(ctx context.Context, network, addr string) (net.Conn, error) { + conn, err := dialConn(ctx, origDial, network, addr) + if err != nil { + return nil, err + } + if err := validateConnRemoteIP(conn); err != nil { + conn.Close() + return nil, err + } + return conn, nil + } +} + func dialConn(ctx context.Context, dialFn func(context.Context, string, string) (net.Conn, error), network, addr string) (net.Conn, error) { if dialFn != nil { return dialFn(ctx, network, addr) diff --git a/shortcuts/minutes/minutes_download.go b/shortcuts/minutes/minutes_download.go index 87d6bd3e..9a8c5545 100644 --- a/shortcuts/minutes/minutes_download.go +++ b/shortcuts/minutes/minutes_download.go @@ -76,6 +76,13 @@ var MinutesDownload = common.Shortcut{ errOut := runtime.IO().ErrOut single := len(tokens) == 1 + // Batch mode: --output must be a directory, not an existing file. + if !single && outputPath != "" { + if fi, err := os.Stat(outputPath); err == nil && !fi.IsDir() { + return output.ErrValidation("--output %q is a file; batch mode expects a directory path", outputPath) + } + } + if !single { fmt.Fprintf(errOut, "[minutes +download] batch: %d token(s)\n", len(tokens)) } @@ -91,6 +98,33 @@ var MinutesDownload = common.Shortcut{ results := make([]result, len(tokens)) seen := make(map[string]int) usedNames := make(map[string]bool) + + // Clone the factory client for download use. We clone the struct (not the + // pointer) to avoid mutating the shared singleton's Timeout. The original + // transport chain is preserved so security headers and test mocks still work. + // SSRF protection: ValidateDownloadSourceURL (URL-level) + CheckRedirect + // (redirect-level). Transport-level IP check is intentionally omitted because + // download URLs originate from the trusted Lark API, not user input. + baseClient, err := runtime.Factory.HttpClient() + if err != nil { + return output.ErrNetwork("failed to get HTTP client: %s", err) + } + clonedClient := *baseClient + clonedClient.Timeout = disableClientTimeout + clonedClient.CheckRedirect = func(req *http.Request, via []*http.Request) error { + if len(via) >= maxDownloadRedirects { + return fmt.Errorf("too many redirects") + } + if len(via) > 0 { + prev := via[len(via)-1] + if strings.EqualFold(prev.URL.Scheme, "https") && strings.EqualFold(req.URL.Scheme, "http") { + return fmt.Errorf("redirect from https to http is not allowed") + } + } + return validate.ValidateDownloadSourceURL(req.Context(), req.URL.String()) + } + dlClient := &clonedClient + ticker := time.NewTicker(time.Second / 5) // rate-limit to 5 req/s defer ticker.Stop() @@ -134,7 +168,7 @@ var MinutesDownload = common.Shortcut{ opts.outputDir = outputPath } - dl, err := downloadMediaFile(ctx, runtime, downloadURL, token, opts) + dl, err := downloadMediaFile(ctx, dlClient, downloadURL, token, opts) if err != nil { results[i] = result{MinuteToken: token, Error: err.Error()} continue @@ -202,38 +236,17 @@ type downloadOpts struct { // downloadMediaFile streams a media file from a pre-signed URL to disk. // Filename resolution: opts.outputPath > Content-Disposition filename > Content-Type ext > <token>.media. -func downloadMediaFile(ctx context.Context, runtime *common.RuntimeContext, downloadURL, minuteToken string, opts downloadOpts) (*downloadResult, error) { +func downloadMediaFile(ctx context.Context, client *http.Client, downloadURL, minuteToken string, opts downloadOpts) (*downloadResult, error) { if err := validate.ValidateDownloadSourceURL(ctx, downloadURL); err != nil { return nil, output.ErrValidation("blocked download URL: %s", err) } - httpClient, err := runtime.Factory.HttpClient() - if err != nil { - return nil, output.ErrNetwork("failed to get HTTP client: %s", err) - } - - // Clone client with disabled timeout and redirect safety. - downloadClient := *httpClient - downloadClient.Timeout = disableClientTimeout - downloadClient.CheckRedirect = func(req *http.Request, via []*http.Request) error { - if len(via) >= maxDownloadRedirects { - return fmt.Errorf("too many redirects") - } - if len(via) > 0 { - prev := via[len(via)-1] - if strings.EqualFold(prev.URL.Scheme, "https") && strings.EqualFold(req.URL.Scheme, "http") { - return fmt.Errorf("redirect from https to http is not allowed") - } - } - return validate.ValidateDownloadSourceURL(req.Context(), req.URL.String()) - } - req, err := http.NewRequestWithContext(ctx, http.MethodGet, downloadURL, nil) if err != nil { return nil, output.ErrNetwork("invalid download URL: %s", err) } - resp, err := downloadClient.Do(req) + resp, err := client.Do(req) if err != nil { return nil, output.ErrNetwork("download failed: %s", err) } @@ -303,6 +316,10 @@ var preferredExt = map[string]string{ "audio/mpeg": ".mp3", } +// newDownloadClient wraps the base HTTP client with SSRF protection +// (redirect safety + transport-level IP validation). When the base transport +// is not *http.Transport (e.g. test mocks), it falls back to cloning +// http.DefaultTransport via NewDownloadHTTPClient. // extFromContentType returns a file extension for the given Content-Type, or "" if unknown. func extFromContentType(contentType string) string { if contentType == "" { diff --git a/shortcuts/minutes/minutes_download_test.go b/shortcuts/minutes/minutes_download_test.go index a7b4368b..5e6a4738 100644 --- a/shortcuts/minutes/minutes_download_test.go +++ b/shortcuts/minutes/minutes_download_test.go @@ -383,6 +383,7 @@ func TestDownload_Batch_PartialFailure(t *testing.T) { f, stdout, _, reg := cmdutil.TestFactory(t, defaultConfig()) reg.Register(mediaStub("tok001", "https://example.com/download/1")) + reg.Register(downloadStub("example.com/download/1", []byte("content-1"), "video/mp4")) reg.Register(&httpmock.Stub{ Method: "GET", URL: "/open-apis/minutes/v1/minutes/tok002/media", @@ -392,7 +393,6 @@ func TestDownload_Batch_PartialFailure(t *testing.T) { "data": map[string]interface{}{}, }, }) - reg.Register(downloadStub("example.com/download/1", []byte("content-1"), "video/mp4")) err := mountAndRun(t, MinutesDownload, []string{ "+download", "--minute-tokens", "tok001,tok002", "--as", "bot",