Skip to content

feat: add SFTP storage backend - #388

Merged
AptS-1547 merged 6 commits into
AsterCommunity:masterfrom
AptS-2077:feat/sftp-storage
Jul 6, 2026
Merged

feat: add SFTP storage backend#388
AptS-1547 merged 6 commits into
AsterCommunity:masterfrom
AptS-2077:feat/sftp-storage

Conversation

@AptS-2077

@AptS-2077 AptS-2077 commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Adds SFTP storage policy support implemented with russh + russh-sftp.

Summary:

  • Add an SFTP storage driver and connector using server-side streaming uploads/downloads.
  • Register the new sftp driver type and descriptor metadata.
  • Add frontend create/edit policy support for SFTP static-secret connection fields.
  • Add tests for endpoint parsing/path normalization, connector descriptors, credential merging, and upload transport behavior.

Validation:

  • cargo fmt --check
  • git diff --check
  • cargo check --lib --jobs 4
  • cargo test --lib storage::drivers::sftp --jobs 4
  • cargo test --lib storage::connectors --jobs 4
  • bunx tsgo -b
  • bunx biome check on changed frontend files
  • bunx biome check src/i18n/locales/en/admin/policies.json src/i18n/locales/zh/admin/policies.json

Note:

  • The real SFTP round-trip test is ignored unless ASTER_SFTP_TEST_* environment variables are provided.

Summary by CodeRabbit

  • 新功能
    • 新增 SFTP 存储策略与后端支持,覆盖上传、下载、复制、删除与范围读取(含集成测试)。
    • 策略创建/编辑向导支持“静态密钥连接”并新增 SFTP 字段:endpoint、SSH 用户名/密码、base path 与主机密钥指纹;新增协议/URL 校验与连接测试引导。
    • 驱动类型徽标与界面展示扩展支持 SFTP。
  • Bug 修复
    • 优化连接字段归一化与变更检测,避免将策略选项变更误判为连接变更。
  • 文档
    • 补全 SFTP 使用教程与上传/存储说明。

@coderabbitai

coderabbitai Bot commented Jul 6, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@AptS-1547, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 44 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: cf7b53e4-4304-4374-9930-aa05cc4007c1

📥 Commits

Reviewing files that changed from the base of the PR and between bbb070008a64bf40b88fef510ad984a1e230694e and ddc9073.

📒 Files selected for processing (21)
  • developer-docs/en/api/admin.md
  • developer-docs/en/architecture.md
  • developer-docs/en/storage-descriptor-normalization-contract.md
  • developer-docs/en/testing.md
  • developer-docs/zh-CN/api/admin.md
  • developer-docs/zh-CN/architecture.md
  • developer-docs/zh-CN/storage-descriptor-normalization-contract.md
  • developer-docs/zh-CN/testing.md
  • docs/.vitepress/config.ts
  • docs/config/storage.md
  • docs/en/config/storage.md
  • docs/en/features/upload-storage.md
  • docs/en/guide/upload-modes.md
  • docs/en/index.md
  • docs/en/storage/index.md
  • docs/en/storage/sftp.md
  • docs/features/upload-storage.md
  • docs/guide/upload-modes.md
  • docs/index.md
  • docs/storage/index.md
  • docs/storage/sftp.md
📝 Walkthrough

Walkthrough

本次改动为存储系统新增 SFTP 存储支持,后端接入 SftpDriverSftpConnector、驱动注册与错误上下文;前端改为基于描述符的静态密钥连接判定、表单规范化和 SFTP 管理界面;同时补齐文案、文档与测试覆盖。

Changes

后端 SFTP 存储栈

Layer / File(s) Summary
类型、字段与错误码
src/types/storage_policy.rs, src/storage/connector_descriptor.rs, src/storage/error.rs, src/api/api_error_code.rs, src/errors.rs
DriverTypeStoragePolicyOptions、连接字段描述符、存储错误上下文和稳定错误码都新增了 SFTP 相关字段、约束或映射。
SftpDriver 实现
src/storage/drivers/sftp.rs, src/storage/drivers/mod.rs, src/storage/error.rs
新增 SFTP 端点解析、host key 校验、远端路径处理、读写/范围读取/复制/流式上传与错误分类,并补充驱动模块导出与测试。
连接器接入与上传传输
src/storage/connectors/sftp.rs, src/storage/connectors/mod.rs, src/storage/connectors/common.rs, src/storage/connectors/upload.rs, src/storage/registry.rs
SFTP 连接器、分发路径、选项校验、上传传输模型和注册表都接入了 Sftp 分支。
后端驱动测试与对齐
src/storage/connectors/tests.rs, tests/test_sftp.rs
后端测试补齐了 SFTP 描述符、UI 元数据、policy options 拒绝、上传工作流对齐,以及容器化端到端往返测试。

前端静态密钥连接与 SFTP 管理界面

Layer / File(s) Summary
静态密钥判定与 API 类型
frontend-panel/src/components/admin/storage-policy-dialog/descriptorPredicates.ts, frontend-panel/src/services/api.generated.ts, frontend-panel/src/types/api-helpers.ts, frontend-panel/src/components/admin/admin-policies-page/policyPresentation.ts, frontend-panel/src/components/admin/admin-policies-page/policyPresentation.test.ts
前端类型、错误码、静态密钥判定和 SFTP 徽标样式都新增了对应分支与断言。
连接规范化与 policy option 传递
frontend-panel/src/components/admin/storage-policy-dialog/connectionNormalization.ts, formTypes.ts, policyFormTransition.ts, storagePolicyOptions.ts, storagePolicyDialogShared.test.ts
连接规范化、端点协议校验、policy_option_values 的序列化与迁移,以及共享测试都改为 descriptor 驱动。
策略对话框与向导
frontend-panel/src/components/admin/StoragePolicyDialog.tsx, StoragePolicyCreateWizard.tsx, StoragePolicyEditForm.tsx, StoragePolicyObjectStorageFields.tsx, AdminPoliciesPage.test.tsx
策略对话框、创建向导、编辑表单和对象存储字段组件都接入了静态密钥连接分支、bucket 条件渲染和 SFTP 创建流程。
文案、错误与文档
frontend-panel/src/i18n/locales/*/admin/policies.json, frontend-panel/src/i18n/locales/*/errors/storage.json, frontend-panel/src/i18n/index.test.ts, frontend-panel/AGENTS.md, developer-docs/*, docs/*
中英文 i18n、错误提示、开发文档和产品文档都补充了 SFTP 说明、操作步骤与边界提示。

Estimated code review effort: 4 (Complex) | ~75 minutes

Sequence Diagram(s)

sequenceDiagram
  participant 管理后台
  participant storage-policy-dialog
  participant 后端API
  participant SftpDriver

  管理后台->>storage-policy-dialog: 选择 sftp 并填写 endpoint/凭据/指纹
  storage-policy-dialog->>storage-policy-dialog: supportsStaticSecretConnection + normalizePolicyForm
  storage-policy-dialog->>后端API: buildCreatePolicyPayload + create
  后端API->>SftpDriver: validate_connection_credentials
  后端API->>SftpDriver: validate_policy_options
  SftpDriver-->>后端API: 校验结果
  后端API-->>管理后台: 成功或错误码
Loading

Possibly related issues

Possibly related PRs

Suggested labels: Storage Driver

Suggested reviewers: AptS-1547

Poem

SFTP 这回进了门,
指纹一拍,别想乱混。
表单、驱动、文案全都对齐,
你小子这次总算没把洞挖歪。
🐾

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed 标题准确概括了本次新增 SFTP 存储后端的主要变更。
Description check ✅ Passed 描述包含摘要、验证步骤和未运行真实回环测试的说明,整体与模板要求基本一致。
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@AptS-1547 AptS-1547 linked an issue Jul 6, 2026 that may be closed by this pull request
@AptS-1547 AptS-1547 added this to the Storage & Driver milestone Jul 6, 2026
@AptS-1547 AptS-1547 added Enhancement New feature or request Priority: Medium Medium priority issue Rust Pull requests that update Rust code TypeScript Pull requests that update JavaScript code Scope: Storage Storage policies, connectors, drivers, provider capabilities, and storage backends labels Jul 6, 2026

@coderabbitai coderabbitai 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.

Actionable comments posted: 3

🧹 Nitpick comments (4)
src/storage/drivers/sftp.rs (1)

196-311: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy lift

复用 SSH 连接,别让每个读写都重新握手

connect() 每次都会重新 client::connect、密码认证、打开 session channel、请求 sftp subsystem,再初始化 SftpSessionopen_readerget_range 还会把这套流程按分片反复走一遍。热路径上这会把延迟和 CPU 开销直接放大。建议在 driver 内复用底层 SSH 连接,按需打开新的 SFTP channel,或做带健康检查的连接池;SftpSession 不要直接跨线程共享。

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/storage/drivers/sftp.rs` around lines 196 - 311, The SFTP driver is
reopening and re-handshaking the SSH session on every operation, which makes
hot-path reads/writes expensive. Update the `SftpDriver::connect`,
`open_reader`, and `get_range` flow to reuse an established SSH connection or
maintain a small pooled connection state, then open new SFTP channels/subsystems
as needed instead of calling `client::connect` and reauthenticating each time.
Keep `SftpSession` from being shared directly across threads; add health checks
and reconnect only when the cached connection is stale.
src/storage/connectors/sftp.rs (2)

81-94: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

access_key / secret_key 标签沿用通用命名,语义对不上。

SFTP 里这两个字段本质是 SSH 用户名和密码,却用跟 S3 一模一样的 access_key/secret_key 通用标签(对比 Azure Blob connector 专门定义了 azure_blob_account_name/azure_blob_account_key)。用户填表单时看到"Access Key"字样去填 SSH 用户名,谁看了不迷糊?As per coding guidelines, src/**/*.rs: "UUID、token、share password、session secret、object key、credential id 等敏感或易混字段要用专门类型或清晰命名,避免 String 到处裸传导致用错。"

♻️ 建议改用专属标签键
                 storage_connector_field(
-                    "access_key",
+                    "access_key",
                     StorageConnectorFieldScope::Connection,
                     StorageConnectorFieldKind::Text,
                     true,
                     false,
                 ),

可以改用 storage_connector_field_with_display,为 endpoint 之外的 access_key/secret_key 也提供 label_key: "sftp_username" / "sftp_password",并在前端 i18n 补充对应文案。

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/storage/connectors/sftp.rs` around lines 81 - 94, The SFTP connector is
reusing generic access_key/secret_key labels for SSH credentials, which is
misleading for users. Update the field definitions in the SFTP connector setup
to use connector-specific display labels via
storage_connector_field_with_display (or equivalent) for the username and
password fields, and reference the existing SFTP connector symbols so the form
shows SFTP-appropriate text instead of S3-style terminology. Also add the
corresponding i18n entries for the new label keys so the UI renders the correct
names.

Source: Coding guidelines


113-113: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

把 SFTP 凭据标签改成 username/password

SftpDriver 实际是用 username/password 做密码认证,现在还沿用 access_key/secret_key,很容易把 SSH 登录信息填错。给 SFTP 单独配一组更贴近语义的标签键,别继续借 S3 语义。

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/storage/connectors/sftp.rs` at line 113, SftpDriver is still using
access_key/secret_key labels even though it authenticates with
username/password; update the SFTP credential labels in the SftpDriver
implementation and any related config/UI mapping to use username/password
instead. Keep the change scoped to the SFTP connector symbols so it no longer
borrows S3-style terminology and callers can populate the correct fields.
frontend-panel/src/pages/admin/AdminPoliciesPage.test.tsx (1)

1014-1037: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

顺手提一句:SFTP 一路加了描述符、UI 配置、向导入口,却没一条端到端的创建测试。

s3/tencent_cos/azure_blob 都有完整的“填表单 -> test_connection -> create”流程测试,SFTP 只有基础设施,没有对应的 it(...) 场景验证 access_key/secret_key/base_path 是否正确进了 payload。新驱动的关键路径,建议照抄现成的 S3/Azure 测试补一条。这条测试写出来,顺便也能把上面 credential_mode 的坑直接暴露出来。

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend-panel/src/pages/admin/AdminPoliciesPage.test.tsx` around lines 1014
- 1037, SFTP has been added to the descriptor/UI flow but is missing an
end-to-end creation test. Add an `it(...)` case in `AdminPoliciesPage.test.tsx`
alongside the existing S3/Azure create flows that fills the SFTP form, runs
`test_connection`, and submits `create`, then asserts the payload includes
`access_key`, `secret_key`, and `base_path` from the SFTP descriptor. Use the
existing S3/Tencent COS/Azure Blob tests as the pattern and reference the SFTP
`createStorageDriverDescriptor("sftp", ...)` setup so the new case covers the
full create path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@frontend-panel/src/components/admin/storage-policy-dialog/connectionNormalization.ts`:
- Around line 278-292: Tighten the hasEndpointUrlScheme check so it only treats
strings with a real URL scheme separator like :// as having a scheme. In
connectionNormalization.ts, update the logic around hasEndpointUrlScheme and the
endpoint parsing path so SFTP inputs like host:22 are treated as scheme-less and
do not fall through to new URL(...) as example.com:; keep the
allowedProtocols/endpointUrl.protocol validation unchanged for actual URLs, and
make sure the connection field detection stays aligned with
parse_sftp_endpoint’s support for bare host and host:port.

In `@frontend-panel/src/pages/admin/AdminPoliciesPage.test.tsx`:
- Around line 1014-1037: The storage driver fixtures for
s3/tencent_cos/azure_blob are missing credential_mode: "static_secret", so
setDriverType() treats them as non-static-secret and clears
endpoint/bucket/access_key/secret_key. Update the relevant
createStorageDriverDescriptor() setup to explicitly use credential_mode:
"static_secret" for these drivers, matching the existing sftp fixture pattern,
so supportsStaticSecretConnection() preserves the connection inputs in the COS
via S3 flow.

In `@src/storage/drivers/sftp.rs`:
- Around line 39-48: The TrustServerKeyClient::check_server_key implementation
currently accepts every host key, so update it to verify the server’s public key
against a known_hosts entry or pinned fingerprint and reject unknown keys by
default. Use the existing TrustServerKeyClient and its check_server_key method
to locate the change, and make the handshake fail closed instead of returning
Ok(true) unconditionally. Also review the SFTP connection flow around
self.connect() so the handshake isn’t repeated on every operation if it can be
cached or reused safely.

---

Nitpick comments:
In `@frontend-panel/src/pages/admin/AdminPoliciesPage.test.tsx`:
- Around line 1014-1037: SFTP has been added to the descriptor/UI flow but is
missing an end-to-end creation test. Add an `it(...)` case in
`AdminPoliciesPage.test.tsx` alongside the existing S3/Azure create flows that
fills the SFTP form, runs `test_connection`, and submits `create`, then asserts
the payload includes `access_key`, `secret_key`, and `base_path` from the SFTP
descriptor. Use the existing S3/Tencent COS/Azure Blob tests as the pattern and
reference the SFTP `createStorageDriverDescriptor("sftp", ...)` setup so the new
case covers the full create path.

In `@src/storage/connectors/sftp.rs`:
- Around line 81-94: The SFTP connector is reusing generic access_key/secret_key
labels for SSH credentials, which is misleading for users. Update the field
definitions in the SFTP connector setup to use connector-specific display labels
via storage_connector_field_with_display (or equivalent) for the username and
password fields, and reference the existing SFTP connector symbols so the form
shows SFTP-appropriate text instead of S3-style terminology. Also add the
corresponding i18n entries for the new label keys so the UI renders the correct
names.
- Line 113: SftpDriver is still using access_key/secret_key labels even though
it authenticates with username/password; update the SFTP credential labels in
the SftpDriver implementation and any related config/UI mapping to use
username/password instead. Keep the change scoped to the SFTP connector symbols
so it no longer borrows S3-style terminology and callers can populate the
correct fields.

In `@src/storage/drivers/sftp.rs`:
- Around line 196-311: The SFTP driver is reopening and re-handshaking the SSH
session on every operation, which makes hot-path reads/writes expensive. Update
the `SftpDriver::connect`, `open_reader`, and `get_range` flow to reuse an
established SSH connection or maintain a small pooled connection state, then
open new SFTP channels/subsystems as needed instead of calling `client::connect`
and reauthenticating each time. Keep `SftpSession` from being shared directly
across threads; add health checks and reconnect only when the cached connection
is stale.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: fcda36ce-6713-4597-99f6-8d127dd793d5

📥 Commits

Reviewing files that changed from the base of the PR and between 9603be1 and 4227ce5.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (22)
  • Cargo.toml
  • frontend-panel/src/components/admin/StoragePolicyDialog.tsx
  • frontend-panel/src/components/admin/admin-policies-page/policyPresentation.test.ts
  • frontend-panel/src/components/admin/admin-policies-page/policyPresentation.ts
  • frontend-panel/src/components/admin/storage-policy-dialog/StoragePolicyCreateWizard.tsx
  • frontend-panel/src/components/admin/storage-policy-dialog/StoragePolicyEditForm.tsx
  • frontend-panel/src/components/admin/storage-policy-dialog/StoragePolicyObjectStorageFields.tsx
  • frontend-panel/src/components/admin/storage-policy-dialog/connectionNormalization.ts
  • frontend-panel/src/components/admin/storage-policy-dialog/descriptorPredicates.ts
  • frontend-panel/src/i18n/locales/en/admin/policies.json
  • frontend-panel/src/i18n/locales/zh/admin/policies.json
  • frontend-panel/src/pages/admin/AdminPoliciesPage.test.tsx
  • frontend-panel/src/pages/admin/AdminPoliciesPage.tsx
  • frontend-panel/src/services/api.generated.ts
  • src/storage/connectors/mod.rs
  • src/storage/connectors/sftp.rs
  • src/storage/connectors/tests.rs
  • src/storage/connectors/upload.rs
  • src/storage/drivers/mod.rs
  • src/storage/drivers/sftp.rs
  • src/storage/registry.rs
  • src/types/storage_policy.rs

Comment thread frontend-panel/src/pages/admin/AdminPoliciesPage.test.tsx
Comment thread src/storage/drivers/sftp.rs
AptS-1547 added 3 commits July 6, 2026 20:51
…descriptor metadata

Remove hardcoded SFTP/S3/Azure field matrices from frontend predicates and normalization logic. Replace form-driven capability inference with descriptor-first field derivation:

- **Backend descriptor enhancement**
  - Add `allowed_endpoint_protocols` and `allow_endpoint_without_protocol` to field descriptor schema
  - Mark SFTP endpoint as scheme-optional with `sftp:` protocol constraint
  - Add custom labels `sftp_username` and `sftp_password` for SFTP credentials
  - Set `trim_on_blur: true` for SFTP access_key field

- **Frontend normalization refactor**
  - Replace `shouldUse*Connection()` predicates that check form content with pure `supports*Connection()` descriptor queries
  - Remove `hasStaticSecretConnectionFields()` and `hasObjectStorageConnectionFields()` form-based inference helpers
  - Derive endpoint validation rules (`allowed_protocols`, `allow_endpoint_without_protocol`) directly from descriptor field metadata
  - Fix `hasEndpointUrlScheme()` regex to require `://` instead of accepting bare `:`

- **Test coverage**
  - Add SFTP wizard integration test verifying SSH credential labels and scheme-less endpoint acceptance
  - Update existing S3/Azure test fixtures to pass explicit descriptors to all payload/normalization/validation helpers
  - Add backend unit test assertions for SFTP field `label_key`, `allowed_endpoint_protocols`, and `allow_endpoint_without_protocol`
  - Add new SFTP driver integration test covering upload, download, range requests, copy, stream upload, and file upload operations

- **I18n**
  - Add `sftp_username` and `sftp_password` translation keys to English and Chinese policy locales

This change enforces the AGENTS.md rule prohibiting new driver-type white-list predicates and ensures connection field behavior is governed by backend connector metadata, not frontend assumptions.
…-388-sftp-descriptor-fix

# Conflicts:
#	frontend-panel/src/pages/admin/AdminPoliciesPage.tsx
@socket-security

socket-security Bot commented Jul 6, 2026

Copy link
Copy Markdown

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Addedcargo/​russh@​0.62.19310093100100
Addedcargo/​russh-sftp@​2.3.010010093100100

View full report

@codecov

codecov Bot commented Jul 6, 2026

Copy link
Copy Markdown

@coderabbitai coderabbitai 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.

Actionable comments posted: 1

🧹 Nitpick comments (3)
tests/test_sftp.rs (1)

56-66: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

别从 message 里拆指纹。 tests/test_sftp.rs:56-66
SftpDriver 里已经有 HostKeyRejection::{actual, expected},这里还在靠 AsterError::message()Confirm fingerprint 。把 fingerprint 透成专用字段/访问器,或者让测试直接拿 actual,别把断言绑死在文案上。

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/test_sftp.rs` around lines 56 - 66, The SFTP fingerprint test is still
parsing the host key fingerprint out of AsterError::message(), which ties the
assertion to error text. Update the SftpDriver error path and the tests around
extract_sftp_host_key_fingerprint to expose the fingerprint through a dedicated
field or accessor on HostKeyRejection (for example using actual/expected
directly), and make the test assert against that value instead of splitting the
message string.
src/storage/connectors/common.rs (1)

197-207: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

错误码不统一,跟 OneDrive 那套对不上。

同文件里 ensure_onedrive_options_absent 用的是专用码 ApiErrorCode::PolicyOneDriveOptionsUnsupported,你这边 ensure_sftp_options_absent 却甩了个泛化的 ApiErrorCode::BadRequest。语义一模一样的两个校验函数,错误码却两副面孔,前端想按错误码精确捕获分支就会两头堵。加个专用码,别偷懒。

♻️ 建议对齐专用错误码
 pub(super) fn ensure_sftp_options_absent(
     options: &crate::types::StoragePolicyOptions,
 ) -> Result<()> {
     if options.sftp_host_key_fingerprint.is_some() {
         return Err(validation_error_with_code(
-            ApiErrorCode::BadRequest,
+            ApiErrorCode::PolicySftpOptionsUnsupported,
             "SFTP host key options are only valid for SFTP storage policies",
         ));
     }
     Ok(())
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/storage/connectors/common.rs` around lines 197 - 207, In
ensure_sftp_options_absent, the validation currently returns a generic
ApiErrorCode::BadRequest while the matching ensure_onedrive_options_absent uses
a dedicated policy-specific error code. Update the SFTP branch to use a
dedicated SFTP-specific error code in the validation_error_with_code call,
keeping the message unchanged and aligning the behavior with the sibling helper
in common.rs so callers can distinguish this case by code.
frontend-panel/src/components/admin/storage-policy-dialog/storagePolicyOptions.ts (1)

136-150: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

policy_options 里 secret 字段被一刀切 trim,跟 secret_key 的处理原则打架。

connectionNormalization.ts 里明确没对 SFTP 的 secret_key 做 trim(保留密码原样,防止把有意义的空格吃掉),这里却对所有 kind === "secret" 的 policy_options 字段无条件 .trim(),完全不看 field.trim_on_blur。眼下 SFTP 只用了 text 类型的 host key fingerprint,还没暴露问题;但代码明明白白支持 secret kind,以后谁加个密码类的 policy option,首尾空格就被静默吃掉,出了问题都没处排查。

要 trim 就该跟着字段的 trim_on_blur 走,别自作主张。

♻️ 建议改法
-		const value = form.policy_option_values?.[field.name]?.trim() ?? "";
+		const rawValue = form.policy_option_values?.[field.name] ?? "";
+		const value =
+			field.trim_on_blur === false ? rawValue : rawValue.trim();
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@frontend-panel/src/components/admin/storage-policy-dialog/storagePolicyOptions.ts`
around lines 136 - 150, The policy_options normalization in storagePolicyOptions
should not blindly trim every field with kind "secret"; align it with the
field’s trim_on_blur behavior instead. Update the logic in the policy_options
loop to check the current descriptor field metadata before calling trim, so
secret values are preserved unless that specific field opts in. Use the existing
storagePolicyOptions normalization flow and the
field.name/field.kind/field.trim_on_blur checks to locate the fix.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/test_sftp.rs`:
- Around line 127-128: The real SFTP round-trip test is not gated and will run
unconditionally, which can fail when Docker or the required ASTER_SFTP_TEST_*
variables are unavailable. Update test_sftp_driver_upload_download_round_trip to
be skipped by default with #[ignore] and add an explicit environment-variable
check at the start of the test body so it exits early unless the SFTP test
config is present, matching the intended behavior for this integration test.

---

Nitpick comments:
In
`@frontend-panel/src/components/admin/storage-policy-dialog/storagePolicyOptions.ts`:
- Around line 136-150: The policy_options normalization in storagePolicyOptions
should not blindly trim every field with kind "secret"; align it with the
field’s trim_on_blur behavior instead. Update the logic in the policy_options
loop to check the current descriptor field metadata before calling trim, so
secret values are preserved unless that specific field opts in. Use the existing
storagePolicyOptions normalization flow and the
field.name/field.kind/field.trim_on_blur checks to locate the fix.

In `@src/storage/connectors/common.rs`:
- Around line 197-207: In ensure_sftp_options_absent, the validation currently
returns a generic ApiErrorCode::BadRequest while the matching
ensure_onedrive_options_absent uses a dedicated policy-specific error code.
Update the SFTP branch to use a dedicated SFTP-specific error code in the
validation_error_with_code call, keeping the message unchanged and aligning the
behavior with the sibling helper in common.rs so callers can distinguish this
case by code.

In `@tests/test_sftp.rs`:
- Around line 56-66: The SFTP fingerprint test is still parsing the host key
fingerprint out of AsterError::message(), which ties the assertion to error
text. Update the SftpDriver error path and the tests around
extract_sftp_host_key_fingerprint to expose the fingerprint through a dedicated
field or accessor on HostKeyRejection (for example using actual/expected
directly), and make the test assert against that value instead of splitting the
message string.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 06090550-e7ad-49a0-8875-f3826e77e1cf

📥 Commits

Reviewing files that changed from the base of the PR and between 4227ce5 and a11b41d.

📒 Files selected for processing (20)
  • frontend-panel/AGENTS.md
  • frontend-panel/src/components/admin/storage-policy-dialog/StoragePolicyObjectStorageFields.tsx
  • frontend-panel/src/components/admin/storage-policy-dialog/connectionNormalization.ts
  • frontend-panel/src/components/admin/storage-policy-dialog/formTypes.ts
  • frontend-panel/src/components/admin/storage-policy-dialog/policyFormTransition.ts
  • frontend-panel/src/components/admin/storage-policy-dialog/storagePolicyOptions.ts
  • frontend-panel/src/components/admin/storagePolicyDialogShared.test.ts
  • frontend-panel/src/i18n/locales/en/admin/policies.json
  • frontend-panel/src/i18n/locales/zh/admin/policies.json
  • frontend-panel/src/pages/admin/AdminPoliciesPage.test.tsx
  • frontend-panel/src/services/api.generated.ts
  • src/storage/connector_descriptor.rs
  • src/storage/connectors/common.rs
  • src/storage/connectors/mod.rs
  • src/storage/connectors/sftp.rs
  • src/storage/connectors/tests.rs
  • src/storage/drivers/sftp.rs
  • src/storage/error.rs
  • src/types/storage_policy.rs
  • tests/test_sftp.rs
✅ Files skipped from review due to trivial changes (4)
  • frontend-panel/AGENTS.md
  • frontend-panel/src/services/api.generated.ts
  • frontend-panel/src/i18n/locales/zh/admin/policies.json
  • frontend-panel/src/i18n/locales/en/admin/policies.json
🚧 Files skipped from review as they are similar to previous changes (4)
  • src/storage/connectors/sftp.rs
  • src/storage/drivers/sftp.rs
  • src/storage/connectors/mod.rs
  • src/storage/connectors/tests.rs

Comment thread tests/test_sftp.rs
@AptS-1547
AptS-1547 force-pushed the feat/sftp-storage branch from c61728d to 0e3522c Compare July 6, 2026 14:24

@coderabbitai coderabbitai 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@developer-docs/zh-CN/api/admin.md`:
- Line 94: The `driver_type` enum listed in the admin docs is inconsistent with
the repository’s storage naming convention because it uses `onedrive` instead of
`one_drive`. Update the supported values in this documentation entry to match
the existing `one_drive` identifier used elsewhere, and make sure the
`driver_type` list stays aligned with the other storage docs and code
conventions.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 6a0632c5-ad14-4bc5-840c-0d39009b6afd

📥 Commits

Reviewing files that changed from the base of the PR and between a11b41d and bbb070008a64bf40b88fef510ad984a1e230694e.

📒 Files selected for processing (35)
  • developer-docs/en/api/admin.md
  • developer-docs/en/architecture.md
  • developer-docs/en/storage-descriptor-normalization-contract.md
  • developer-docs/en/testing.md
  • developer-docs/zh-CN/api/admin.md
  • developer-docs/zh-CN/architecture.md
  • developer-docs/zh-CN/storage-descriptor-normalization-contract.md
  • developer-docs/zh-CN/testing.md
  • docs/.vitepress/config.ts
  • docs/config/storage.md
  • docs/en/config/storage.md
  • docs/en/features/upload-storage.md
  • docs/en/guide/upload-modes.md
  • docs/en/index.md
  • docs/en/storage/index.md
  • docs/en/storage/sftp.md
  • docs/features/upload-storage.md
  • docs/guide/upload-modes.md
  • docs/index.md
  • docs/storage/index.md
  • docs/storage/sftp.md
  • frontend-panel/src/components/admin/storage-policy-dialog/storagePolicyOptions.ts
  • frontend-panel/src/components/admin/storagePolicyDialogShared.test.ts
  • frontend-panel/src/i18n/index.test.ts
  • frontend-panel/src/i18n/locales/en/errors/storage.json
  • frontend-panel/src/i18n/locales/zh/errors/storage.json
  • frontend-panel/src/services/api.generated.ts
  • frontend-panel/src/types/api-helpers.ts
  • src/api/api_error_code.rs
  • src/errors.rs
  • src/storage/connectors/common.rs
  • src/storage/connectors/tests.rs
  • src/storage/drivers/sftp.rs
  • src/storage/error.rs
  • tests/test_sftp.rs
✅ Files skipped from review due to trivial changes (17)
  • frontend-panel/src/i18n/locales/zh/errors/storage.json
  • docs/en/storage/index.md
  • developer-docs/en/testing.md
  • developer-docs/zh-CN/testing.md
  • frontend-panel/src/types/api-helpers.ts
  • docs/index.md
  • docs/storage/index.md
  • docs/en/index.md
  • developer-docs/en/storage-descriptor-normalization-contract.md
  • developer-docs/en/architecture.md
  • docs/storage/sftp.md
  • docs/features/upload-storage.md
  • docs/en/storage/sftp.md
  • frontend-panel/src/services/api.generated.ts
  • developer-docs/zh-CN/architecture.md
  • docs/en/guide/upload-modes.md
  • docs/en/features/upload-storage.md
🚧 Files skipped from review as they are similar to previous changes (5)
  • src/storage/connectors/common.rs
  • frontend-panel/src/components/admin/storage-policy-dialog/storagePolicyOptions.ts
  • src/storage/drivers/sftp.rs
  • frontend-panel/src/components/admin/storagePolicyDialogShared.test.ts
  • src/storage/connectors/tests.rs

Comment thread developer-docs/zh-CN/api/admin.md Outdated
@AptS-1547
AptS-1547 force-pushed the feat/sftp-storage branch from bbb0700 to ddc9073 Compare July 6, 2026 15:07
@AptS-1547
AptS-1547 merged commit 95f22f9 into AsterCommunity:master Jul 6, 2026
4 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Enhancement New feature or request Priority: Medium Medium priority issue Rust Pull requests that update Rust code Scope: Storage Storage policies, connectors, drivers, provider capabilities, and storage backends TypeScript Pull requests that update JavaScript code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(storage): 支持 SFTP 作为存储后端

2 participants