Skip to content

feat(cli): 支持安全下载单个远端文件 / safely download one remote file - #485

Open
Jinghao-coding wants to merge 1 commit into
raids-lab:mainfrom
Jinghao-coding:codex/issue-478-file-download
Open

feat(cli): 支持安全下载单个远端文件 / safely download one remote file#485
Jinghao-coding wants to merge 1 commit into
raids-lab:mainfrom
Jinghao-coding:codex/issue-478-file-download

Conversation

@Jinghao-coding

Copy link
Copy Markdown
Member

中文

说明

新增 crater file download <remote-file> [local-path],用于把 userpublic 或当前 account 下的单个远端文件安全下载到本机。

实现内容

  • 使用 typed API client 调用 GET /api/ss/download/*path
  • 关闭 req 自动读取,使用 io.Copy 真正流式写入,不把完整文件载入内存。
  • 按 URL segment 编码空格、中文、#%,并在请求前拒绝越界路径。
  • 先写目标同目录临时文件,成功后 SyncClose 再发布。
  • 默认模式用 os.Link 原子 no-clobber 发布,消除检查与发布之间的竞态覆盖。
  • --overwrite 才使用同目录 os.Rename 原子替换;下载完成前旧文件保持不变。
  • 网络失败、本地写入失败、同步/关闭/发布失败都会清理临时文件。
  • 非 2xx 错误即使响应体读取失败也保留已知 HTTP status,不误报为纯网络错误。
  • --json 只输出远端路径、本地路径、字节数和覆盖开关,不输出二进制内容。
  • 补齐中英文 i18n、命令契约、File Skill、静态根补全、单元测试和双语快照。
  • 同步修正 feat(job): add server-side pagination for Volcano job lists #446 分页落地后遗留的两处 Job 快照 URL 基线,以保证 make pre-commit-check 在当前 main 上可复现通过。

验证

  • go test ./cmd ./internal/api -count=1
  • go test -race ./cmd ./internal/api -count=1
  • make snapshot-update,已人工检查新增/变化 golden
  • make pre-commit-check
  • make build
  • go vet ./cmd ./internal/api
  • 直接运行 ./crater file download --help
  • 直接运行越界路径命令,确认本地返回 usage_error 和退出码 2

测试覆盖流式首块互锁、NUL/0xff 二进制完整性、逐段路径编码、JSON/纯文本错误、错误响应体读取失败、默认无覆盖成功、并发晚到冲突、显式覆盖、部分失败保留旧文件、临时文件清理及 JSON 纯元数据。

真实 storage service 未在本机 8088 入口提供,因此未把真实服务联调冒充为通过;HTTP 传输契约由真实 httptest.Server 和流式互锁测试覆盖。

测试截图

Issue 478 CLI test evidence

Closes #478


English

Summary

Adds crater file download <remote-file> [local-path] to safely copy one remote file from user, public, or the current account storage space to the local machine.

Changes

  • Uses a typed API client for GET /api/ss/download/*path.
  • Disables req auto-read and streams with io.Copy, never buffering the complete file.
  • Escapes URL segments independently and rejects traversal before any request.
  • Writes to a temporary file in the target directory, then syncs and closes it before publication.
  • Uses atomic os.Link no-clobber publication by default, removing the check-to-publish overwrite race.
  • Uses same-directory os.Rename replacement only with explicit --overwrite; the old file remains intact until the download completes.
  • Cleans temporary files after network, destination-write, sync, close, or publication failures.
  • Preserves a known HTTP status even when reading a non-2xx response body fails.
  • Keeps binary content out of stdout; JSON contains only remote path, local path, byte count, and overwrite mode.
  • Adds bilingual i18n, command contract documentation, a File Skill, static root completion, unit tests, and bilingual snapshots.
  • Refreshes the two stale Job snapshot URLs left by feat(job): add server-side pagination for Volcano job lists #446 pagination so make pre-commit-check is reproducible on current main.

Validation

  • go test ./cmd ./internal/api -count=1
  • go test -race ./cmd ./internal/api -count=1
  • make snapshot-update, followed by manual golden review
  • make pre-commit-check
  • make build
  • go vet ./cmd ./internal/api
  • Direct ./crater file download --help execution
  • Direct traversal attempt, confirming local usage_error and exit code 2

Coverage includes a first-chunk streaming handshake, NUL/0xff binary integrity, segment encoding, JSON/plain-text failures, broken error-body reads, default no-clobber success, late concurrent conflicts, explicit overwrite, partial-failure preservation, temp cleanup, and metadata-only JSON stdout.

The real storage service is not exposed through local port 8088, so this PR does not claim a live storage integration result. HTTP transport behavior is exercised through a real httptest.Server and streaming handshake.

Test screenshot

Issue 478 CLI test evidence

Copilot AI review requested due to automatic review settings July 25, 2026 18:19

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2c0d05d18a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread cli/internal/api/file.go
if err != nil {
return 0, &NetworkError{Cause: err}
}
if !resp.IsSuccessState() {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Require a full-file HTTP response before publishing

Reject successful statuses other than the endpoint's expected 200 OK. IsSuccessState() also accepts responses such as 206 Partial Content and 204 No Content; if a proxy or storage server returns a valid shorter 206 response, io.Copy reaches EOF without error and the command atomically publishes the incomplete file while reporting success.

Useful? React with 👍 / 👎.

Comment thread cli/cmd/file.go
Comment on lines +150 to +154
defer func() {
if temporaryOpen {
_ = temporary.Close()
}
_ = os.Remove(temporaryPath)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Handle interrupts so partial temporary files are removed

Make command interruption cancel the request and unwind this cleanup path. The inspected entrypoint calls rootCmd.Execute() without a signal-aware context (cli/main.go:13, cli/cmd/root.go:71), so pressing Ctrl-C during a large download invokes Go's default SIGINT termination and skips all deferred functions, leaving the potentially large .crater-* temporary file behind indefinitely.

Useful? React with 👍 / 👎.

@Jinghao-coding
Jinghao-coding force-pushed the codex/issue-478-file-download branch from 2c0d05d to 292b395 Compare July 28, 2026 14:36
@Jinghao-coding
Jinghao-coding force-pushed the codex/issue-478-file-download branch from 292b395 to 5faadb6 Compare July 30, 2026 07:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(cli): 支持下载单个远端文件 / download a single remote file

2 participants