feat: 远程 RPC 下载支持#186
Merged
Merged
Conversation
Add aria2 JSON-RPC integration to send release asset downloads to a remote downloader. Default disabled, configurable in Settings > Network. - Add RpcDownloadConfig type and store state (host, port, secret) - Add backend endpoints: GET/PUT/test for config, POST for download - Add frontend service (rpcDownloadService.ts) for API calls - Add RPC config UI in NetworkPanel below proxy settings - Modify ReleaseCard to send downloads via RPC when enabled - Show toast when RPC service is not running - Encrypt secret with AES-256-GCM in backend, skip persistence in frontend - Add SSRF protection on download URL validation - Export validateUrl from proxyService for reuse Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds RPC download: new RpcDownloadConfig type, frontend client (test/send), app-store state and persistence (secret excluded), backend RPC settings/test/download endpoints with URL validation and aria2 JSON-RPC, settings UI to configure/test/save, and ReleaseCard integration to queue remote downloads. ChangesRPC Download Feature
Sequence Diagram(s)sequenceDiagram
participant User
participant ReleaseCard
participant Frontend as Frontend Service
participant Backend as Backend API
participant Aria2
User->>ReleaseCard: Click RPC download button
ReleaseCard->>Frontend: sendToRpcDownload(url, filename, apiSecret?)
Frontend->>Backend: POST /api/download/rpc {url, filename}
Backend->>Backend: validateUrl(url)
Backend->>Aria2: JSON-RPC aria2.addUri (with token if configured, and out: filename)
Aria2->>Backend: {gid} or error
Backend->>Frontend: JSON {success, gid?, error?}
Frontend->>ReleaseCard: Show toast / update UI
sequenceDiagram
participant User
participant NetworkPanel
participant Frontend as Frontend Service
participant Backend as Backend API
participant Aria2
User->>NetworkPanel: Enter host, port, secret
User->>NetworkPanel: Click Test
NetworkPanel->>Frontend: testRpcDownload(config, apiSecret)
Frontend->>Backend: POST /api/settings/rpc-download/test
Backend->>Aria2: JSON-RPC aria2.getVersion (token if provided)
Aria2->>Backend: version response
Backend->>Frontend: {success: true, version}
Frontend->>NetworkPanel: Show result with version
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/services/rpcDownloadService.ts (1)
27-43: ⚡ Quick winConsider checking HTTP response status before parsing JSON.
Both
testRpcDownloadandsendToRpcDownloadparse the response JSON without checkingresp.ok. While the current backend implementation always returns 200 with asuccessfield in the JSON body (making this work), it's more robust to validate the HTTP status:if (!resp.ok) { return { success: false, error: `Server returned ${resp.status}` }; } return await resp.json();This pattern prevents silent failures if the backend changes to return error status codes.
🤖 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/services/rpcDownloadService.ts` around lines 27 - 43, The HTTP response is parsed without checking status in testRpcDownload (and similarly in sendToRpcDownload), so update both functions to check resp.ok after the fetch call and before calling resp.json(); if !resp.ok return an object like { success: false, error: `Server returned ${resp.status}` } (preserving existing error shape), otherwise return await resp.json(); keep existing error-catch behavior using e instanceof Error; reference the fetch call, resp variable, and functions testRpcDownload and sendToRpcDownload when making the change.
🤖 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 `@server/src/routes/proxy.ts`:
- Line 651: The code uses AbortSignal.timeout(5000) in
server/src/routes/proxy.ts which requires Node >=17.3.0; either declare and pin
the supported Node version (engines.node) in root and/or server package.json and
ensure CI/Docker use that runtime, or replace the direct AbortSignal.timeout
calls with an explicit AbortController + setTimeout fallback (create
AbortController, call setTimeout(() => controller.abort(), 5000) and pass
controller.signal) for both spots where AbortSignal.timeout is used so older
Node runtimes remain compatible.
In `@src/types/index.ts`:
- Around line 172-177: Change RpcDownloadConfig.secret to an optional property
(secret?: string) so the frontend won't send an empty string and inadvertently
clear stored secrets; update the type definition for RpcDownloadConfig (where
it's declared) to make secret optional, then update the store initializations
referenced (useAppStore initializers around the mentioned indices) to initialize
secret as undefined instead of '' and modify NetworkPanel.tsx to only include
the secret field in the PUT payload when the user has explicitly changed it;
keep ProxyConfig.password's pattern for reference and ensure backend PUT handler
behavior remains unchanged.
---
Nitpick comments:
In `@src/services/rpcDownloadService.ts`:
- Around line 27-43: The HTTP response is parsed without checking status in
testRpcDownload (and similarly in sendToRpcDownload), so update both functions
to check resp.ok after the fetch call and before calling resp.json(); if
!resp.ok return an object like { success: false, error: `Server returned
${resp.status}` } (preserving existing error shape), otherwise return await
resp.json(); keep existing error-catch behavior using e instanceof Error;
reference the fetch call, resp variable, and functions testRpcDownload and
sendToRpcDownload when making the change.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 75876f4b-4ff6-4d7d-afd8-2636aafb8549
📒 Files selected for processing (7)
server/src/routes/proxy.tsserver/src/services/proxyService.tssrc/components/ReleaseCard.tsxsrc/components/settings/NetworkPanel.tsxsrc/services/rpcDownloadService.tssrc/store/useAppStore.tssrc/types/index.ts
- Check resp.ok before parsing JSON in frontend service (CodeRabbit) - Replace AbortSignal.timeout with AbortController+setTimeout for compatibility - Make RpcDownloadConfig.secret optional to avoid clearing stored secrets - Add logging to test and download endpoints (logger.info/warn/error) - Move variable declarations before try blocks for catch block access - Fix mixed-language placeholder text Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The RPC service was using fetch('/api/...') which targets the current
origin. When frontend and backend run on different ports (e.g. Vite
dev server on 5173, backend on 3000), this causes 'Failed to fetch'.
Use backend.backendUrl (probed by backendAdapter at init) to construct
the correct API URL, matching how other backend calls work.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1. Use resolveBaseUrl() with relative URL fallback instead of requiring backend.backendUrl, fixing 'Backend not available' when backendAdapter hasn't probed yet. 2. Persist RPC secret: load hasSecret from backend on mount, show masked placeholder, fall back to stored secret in test endpoint when request doesn't include one. 3. NetworkPanel save/toggle revert to relative URLs (matching proxy config pattern) since frontend and backend share the same origin in production. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/services/rpcDownloadService.ts (1)
35-47:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPreserve backend error payloads on non-2xx responses.
Both wrappers replace the server's JSON
{ error }body with a genericServer returned <status>. That hides actionable failures from these new endpoints, e.g. invalid download URL or RPC not configured, so the UI loses the real reason.💡 Suggested fix
+async function readErrorMessage(resp: Response): Promise<string> { + try { + const data = await resp.json() as { error?: string }; + if (typeof data.error === 'string' && data.error) { + return data.error; + } + } catch { + // ignore parse failures and fall back to status + } + return `Server returned ${resp.status}`; +} + export async function testRpcDownload( config: RpcDownloadConfig, apiSecret?: string, ): Promise<RpcTestResult> { const base = resolveBaseUrl(); try { const resp = await fetch(`${base}/settings/rpc-download/test`, { method: 'POST', headers: getAuthHeaders(apiSecret), body: JSON.stringify({ host: config.host, port: config.port, secret: config.secret, }), }); if (!resp.ok) { - return { success: false, error: `Server returned ${resp.status}` }; + return { success: false, error: await readErrorMessage(resp) }; } return await resp.json(); } catch (e) { return { success: false, error: e instanceof Error ? e.message : 'Request failed', }; } } @@ export async function sendToRpcDownload( url: string, filename: string, apiSecret?: string, ): Promise<RpcDownloadResult> { const base = resolveBaseUrl(); try { const resp = await fetch(`${base}/download/rpc`, { method: 'POST', headers: getAuthHeaders(apiSecret), body: JSON.stringify({ url, filename }), }); if (!resp.ok) { - return { success: false, error: `Server returned ${resp.status}` }; + return { success: false, error: await readErrorMessage(resp) }; } return await resp.json(); } catch (e) { return { success: false, error: e instanceof Error ? e.message : 'Request failed', }; } }Also applies to: 63-71
🤖 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/services/rpcDownloadService.ts` around lines 35 - 47, The non-2xx branch replaces the server JSON error with a generic message; change the error handling for the fetch to attempt to parse the response body and preserve the backend payload. Specifically, in the POST to `${base}/settings/rpc-download/test` (the resp variable handling) and the similar block at lines 63-71, when resp.ok is false call await resp.json() (falling back to await resp.text() if JSON parse fails) and return { success: false, error: /* use parsed error.message or full parsed object or text */ } so the original server error payload is surfaced to the caller/UI.
🤖 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 `@server/src/routes/proxy.ts`:
- Around line 644-657: The code incorrectly falls back to the stored secret
whenever requestSecret is falsy (e.g., ''), causing blank secrets to be
replaced; change the fallback to only occur when the secret field is omitted
from the request — check presence of the field on req.body (e.g., use "secret"
in req.body or Object.prototype.hasOwnProperty.call(req.body, "secret")) and
only assign from getRpcDownloadConfig() when the field is not present; update
the logic around requestSecret/secret and the call to getRpcDownloadConfig()
(referencing requestSecret, secret, req.body, and getRpcDownloadConfig) so an
explicit empty string provided by the client is respected and not replaced by
the stored secret.
---
Outside diff comments:
In `@src/services/rpcDownloadService.ts`:
- Around line 35-47: The non-2xx branch replaces the server JSON error with a
generic message; change the error handling for the fetch to attempt to parse the
response body and preserve the backend payload. Specifically, in the POST to
`${base}/settings/rpc-download/test` (the resp variable handling) and the
similar block at lines 63-71, when resp.ok is false call await resp.json()
(falling back to await resp.text() if JSON parse fails) and return { success:
false, error: /* use parsed error.message or full parsed object or text */ } so
the original server error payload is surfaced to the caller/UI.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: c9a96186-b85f-4d88-9e86-a29be70b790a
📒 Files selected for processing (3)
server/src/routes/proxy.tssrc/components/settings/NetworkPanel.tsxsrc/services/rpcDownloadService.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/settings/NetworkPanel.tsx
The proxy config endpoints use fetch('/api/...') and work correctly.
Revert rpcDownloadService to the same pattern instead of using
backend.backendUrl which may not be initialized or may point to a
different origin.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/services/rpcDownloadService.ts (1)
37-39:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve backend JSON error messages on non-2xx responses.
Both methods discard structured backend errors and replace them with a generic status string. That can suppress required user-facing messages (e.g., aria2 not running) and weakens troubleshooting.
Suggested fix
+async function readErrorMessage(resp: Response): Promise<string> { + try { + const data = await resp.json(); + if (data && typeof data.error === 'string' && data.error.trim()) { + return data.error; + } + } catch { + // fall through + } + return `Server returned ${resp.status}`; +} + export async function testRpcDownload( config: RpcDownloadConfig, apiSecret?: string, ): Promise<RpcTestResult> { try { const resp = await fetch('/api/settings/rpc-download/test', { @@ }); if (!resp.ok) { - return { success: false, error: `Server returned ${resp.status}` }; + return { success: false, error: await readErrorMessage(resp) }; } return await resp.json(); } catch (e) { @@ export async function sendToRpcDownload( url: string, filename: string, apiSecret?: string, ): Promise<RpcDownloadResult> { @@ }); if (!resp.ok) { - return { success: false, error: `Server returned ${resp.status}` }; + return { success: false, error: await readErrorMessage(resp) }; } return await resp.json(); } catch (e) {Also applies to: 60-62
🤖 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/services/rpcDownloadService.ts` around lines 37 - 39, The current non-2xx handling returns a generic `Server returned ${resp.status}` and discards any structured backend error; instead, when `resp.ok` is false (the `resp` checks at the shown locations), attempt to parse the response body (prefer JSON, fallback to text) to extract and preserve backend error details and include them in the returned error object (e.g., `{ success: false, error: extractedMessage || \`Server returned ${resp.status}\` }`); apply this change to both occurrences that check `resp.ok` so the service preserves user-facing backend messages.
🤖 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.
Outside diff comments:
In `@src/services/rpcDownloadService.ts`:
- Around line 37-39: The current non-2xx handling returns a generic `Server
returned ${resp.status}` and discards any structured backend error; instead,
when `resp.ok` is false (the `resp` checks at the shown locations), attempt to
parse the response body (prefer JSON, fallback to text) to extract and preserve
backend error details and include them in the returned error object (e.g., `{
success: false, error: extractedMessage || \`Server returned ${resp.status}\`
}`); apply this change to both occurrences that check `resp.ok` so the service
preserves user-facing backend messages.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 44fbf388-4003-427b-a19f-07f48670c162
📒 Files selected for processing (1)
src/services/rpcDownloadService.ts
… fallback 1. Electron loads from file:// so relative URLs resolve to file:///api/... Use backend.backendUrl (probed via backendAdapter.init()) for all RPC API calls. getBaseUrl() ensures init() completes before fetching. 2. Load hasSecret from backend on mount, show placeholder hint in input. Only send secret in PUT when user explicitly typed one (omit field to let backend preserve stored encrypted value). 3. CodeRabbit: test endpoint secret fallback now checks field presence via hasOwnProperty instead of truthiness, so explicit empty string is respected (clear secret) vs omitted field (use stored value). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
When no backend is available, the frontend calls aria2 JSON-RPC directly at http://host:port/jsonrpc. This enables RPC download in both browser and Electron desktop modes without requiring the Express server. Changes: - rpcDownloadService: add callAria2Direct() fallback, testRpcDownload and sendToRpcDownload use direct calls when backend is unavailable - NetworkPanel: proxy card hidden without backend, RPC card always visible; save/toggle persist to local store when no backend - Electron build: set webSecurity: false to allow cross-origin requests to local aria2 RPC endpoint (safe for desktop app) Usage (client-only): 1. Start aria2: aria2c --enable-rpc --rpc-listen-port=6800 --rpc-secret=xxx 2. In app: Settings > Remote Download > enable, fill host/port/secret 3. Click download on any release asset → sent to aria2 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
概述
在网络设置中新增远程 RPC 下载配置，支持将 Release 资产下载任务发送到 aria2 远程下载器。
功能
改动文件
安全措施
测试
Summary by CodeRabbit
New Features
Bug Fixes