fix(ci): reduce dependabot backlog and harden API guards#104
Conversation
|
Warning Review limit reached
More reviews will be available in 11 minutes and 14 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the 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 credits. 🚦 How do rate 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 see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough更新了 Dependabot 和自动合并配置,补充 ESLint 忽略项,并将服务端多处接口切换为统一的 JSON 解析流程,同时调整了 WebDAV、登录和图片 URL 的安全校验。 Changes配置与服务端请求硬化
Sequence Diagram(s)sequenceDiagram
participant HonoRoute
participant readJson
participant validateWebdavTarget
participant fetchWebdav
HonoRoute->>readJson: 解析请求体
readJson-->>HonoRoute: body 或 400 响应
HonoRoute->>validateWebdavTarget: 校验 WebDAV URL
HonoRoute->>fetchWebdav: MKCOL / PUT / DELETE
fetchWebdav-->>HonoRoute: 响应或重定向错误
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 @.github/dependabot.yml:
- Around line 10-14: The Dependabot schedule block uses interval: "monthly"
together with day, but day only applies to weekly schedules. Update the schedule
under the dependabot configuration by either removing day if monthly on the 1st
is intended, or switching the schedule to cron if the goal is to run every month
on Monday at 09:00.
In @.github/workflows/dependabot-auto-merge.yml:
- Around line 16-20: The Dependabot metadata step currently uses a mutable
action tag, which is unsafe for an auto-merge workflow. Update the uses
reference in dependabot-auto-merge.yml for the dependabot/fetch-metadata step to
pin a full commit SHA instead of `@v3`, keeping the existing step id and inputs
unchanged.
- Around line 3-5: The Dependabot auto-merge workflow is checking the wrong
identity, so it can skip when a maintainer reopens or marks a Dependabot PR
ready for review. Update the condition in dependabot-auto-merge workflow logic
to use the PR author identity from the pull request context rather than
github.actor, and keep the Dependabot-specific guard aligned with the event data
available in the workflow trigger.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 330172ec-9770-491f-9b0b-ec17779e28ef
📒 Files selected for processing (2)
.github/dependabot.yml.github/workflows/dependabot-auto-merge.yml
📜 Review details
🧰 Additional context used
🪛 zizmor (1.26.1)
.github/workflows/dependabot-auto-merge.yml
[error] 18-18: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 14-14: spoofable bot actor check (bot-conditions): actor context may be spoofable
(bot-conditions)
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)
server/src/index.ts (1)
255-274: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win禁止 WebDAV 请求自动跟随重定向
backup/webdav和backup/webdav-test的fetch()都没设置redirect: "manual";仅靠hostname文本校验挡不住后续 30x 跳转,备份内容仍可能被发往其他 origin。建议这两处统一显式拒绝重定向,并进一步收紧出口侧限制。🤖 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 `@server/src/index.ts` around lines 255 - 274, The WebDAV request path still allows automatic 30x follow-up, so `validateWebdavTarget` alone is not enough to keep backup traffic on the intended origin. Update the `backup/webdav` and `backup/webdav-test` `fetch()` calls to explicitly set `redirect: "manual"` and fail closed on any redirect response, and keep using `validateWebdavTarget` plus `isBlockedWebdavHostname` to enforce the URL restrictions. If there is shared request-building logic around these WebDAV helpers, apply the redirect policy there so both `backup/webdav` and `backup/webdav-test` stay consistent.Source: Path instructions
🧹 Nitpick comments (1)
server/src/index.ts (1)
699-719: 🔒 Security & Privacy | 🔵 Trivial速率限制为单 isolate 内存态,在 Workers 上仅为尽力而为。
loginAttempts是模块级内存Map,Cloudflare Workers 多 isolate 间不共享、且会被回收,因此该限流无法跨实例生效,攻击者可通过命中不同 isolate 规避。pruneLoginAttempts在超过LOGIN_RATE_MAX_KEYS时按插入顺序淘汰最早键,遇到大量不同 IP 时可能挤掉仍在窗口内的合法记录。如需可靠限流,建议改用 Durable Objects / KV 等持久化共享存储。🤖 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 `@server/src/index.ts` around lines 699 - 719, `loginAttempts` and `pruneLoginAttempts` currently implement login rate limiting with a module-level in-memory Map, so the limit only works within a single isolate and is not reliable on Workers. Update the rate-limiting logic in the surrounding login flow to use shared storage such as Durable Objects or KV instead of the in-memory `loginAttempts` map, and avoid eviction behavior in `pruneLoginAttempts` that can drop still-valid entries under high-cardinality IP traffic.
🤖 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/index.ts`:
- Around line 122-129: `readJson` 目前只做了 JSON 解析,没有对运行时结果做非空对象校验,导致 `null`
或其他非对象值会通过并在后续解构时抛错;请在 `readJson` 中增加对 `c.req.json()` 返回值的对象/非空检查,并在失败时直接返回
400。顺手把 `c` 的类型从 `any` 收紧为 `Context`,并保留 `readJson` 的现有返回结构与错误响应风格。
---
Outside diff comments:
In `@server/src/index.ts`:
- Around line 255-274: The WebDAV request path still allows automatic 30x
follow-up, so `validateWebdavTarget` alone is not enough to keep backup traffic
on the intended origin. Update the `backup/webdav` and `backup/webdav-test`
`fetch()` calls to explicitly set `redirect: "manual"` and fail closed on any
redirect response, and keep using `validateWebdavTarget` plus
`isBlockedWebdavHostname` to enforce the URL restrictions. If there is shared
request-building logic around these WebDAV helpers, apply the redirect policy
there so both `backup/webdav` and `backup/webdav-test` stay consistent.
---
Nitpick comments:
In `@server/src/index.ts`:
- Around line 699-719: `loginAttempts` and `pruneLoginAttempts` currently
implement login rate limiting with a module-level in-memory Map, so the limit
only works within a single isolate and is not reliable on Workers. Update the
rate-limiting logic in the surrounding login flow to use shared storage such as
Durable Objects or KV instead of the in-memory `loginAttempts` map, and avoid
eviction behavior in `pruneLoginAttempts` that can drop still-valid entries
under high-cardinality IP traffic.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: df66bfb2-7825-49a6-a368-45eaf56162ea
📒 Files selected for processing (3)
client/eslint.config.mjsserver/eslint.config.mjsserver/src/index.ts
✅ Files skipped from review due to trivial changes (1)
- client/eslint.config.mjs
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
server/src/index.ts
⚙️ CodeRabbit configuration file
server/src/index.ts: Hono Workers API 路由总入口。审查时请关注: 1. JWT 认证中间件是否正确保护管理接口 2. CORS 配置是否安全 3. 请求参数验证
Files:
server/src/index.ts
🔇 Additional comments (2)
server/eslint.config.mjs (1)
16-16: LGTM!server/src/index.ts (1)
442-449: LGTM!Also applies to: 514-516, 741-743, 905-907, 920-922, 965-967, 1018-1018, 1230-1232, 1282-1284, 1295-1297, 1316-1318, 1329-1331, 1350-1352, 1391-1395, 1435-1439, 1563-1565, 1581-1583, 1639-1651
Summary
Verification
Notes