feat: 缓存 TTL 面板可调 + 全量审计修复 - #10
Merged
Merged
Conversation
- 镜像缓存 TTL 与 Git 代理缓存 TTL 可在面板配置（0=永久/禁用） - GitProxy 增加 CacheTTL 读写接口与 /api/config/gitproxy 端点 - config 保存防 env token 落盘（AuthTokenOrig）、原子写入、mtime 失败重试 - token 缓存不缓存临期 token；限流中间件始终装配且 search/release 纳入限流 - 静态资源 SPA fallback 直读 index.html，修复深链接被 301 到根页的问题 - 路径遍历防护、CIDR/时长/大小溢出校验、webhook 非 2xx 视为失败 - 前端：搜索竞态守卫、表单校验、登录回跳、日期守卫、日志容错
- Set 删除过期 TTL 文件、失败清理残留、成功后才记账 - 缓存 key 纳入方法；仅 GET 读写缓存，401 重试仅限 GET/无 body - 17 个镜像 TTL 输出支持 %ds 秒级格式；upstream 尾部斜杠统一 - nuget 默认地址改为 /v3，健康检查自动补 /index.json
- alert webhook 非 2xx 视为失败；CIDR 解析失败告警 - 限流 rate/window 非法时放行而非误杀；PurgeOldTraffic 防护 - release 下载 4xx 透传、Content-Length 仅 HEAD 预设 - 流量统计排序确定性（mirror 排序、周聚合按表达式分组）
Contributor
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe PR adds configurable cache TTLs for mirrors and the Git proxy. It updates cache accounting, configuration persistence, dashboard APIs, frontend controls, authentication redirects, proxy responses, rate limiting, and request-processing edge cases. ChangesConfigurable cache TTL and proxy flow
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant MirrorsVue
participant GitProxyConfigAPI
participant Dashboard
participant Server
participant Config
Browser->>MirrorsVue: Select Git proxy cache TTL
MirrorsVue->>GitProxyConfigAPI: PUT cacheTTL
GitProxyConfigAPI->>Dashboard: Send authenticated request
Dashboard->>Server: SetCacheTTL(ttl)
Server->>Config: Persist Git proxy TTL
Config-->>Server: Return save result
Server-->>Dashboard: Return updated TTL
Dashboard-->>MirrorsVue: Return JSON response
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| // Path escapes the frontend dir (".." traversal): fall back. | ||
| full = filepath.Join(frontDir, "index.html") | ||
| } | ||
| if _, err := os.Stat(full); err != nil { |
| // navigates to an external host. | ||
| const redirect = new URLSearchParams(window.location.search).get('redirect') | ||
| const safe = redirect && redirect.startsWith('/') && !redirect.startsWith('//') | ||
| window.location.href = safe ? redirect : '/' |
| // navigates to an external host. | ||
| const redirect = new URLSearchParams(window.location.search).get('redirect') | ||
| const safe = redirect && redirect.startsWith('/') && !redirect.startsWith('//') | ||
| window.location.href = safe ? redirect : '/' |
![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: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
internal/mirror/cache.go (2)
92-145: 🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick winReclaim
usedByteswhen a write fails after replacing an existing entry.When header creation (line 118) or expiry-write (lines 133-136) fails,
Setremoves the newly written file and returns before theusedBytesupdate at line 145. Sinceos.Create(line 102) already truncated and replaced any prior file atpath, the entry is now gone from disk, butusedBytesstill holds the oldoldSizecontribution counted during a previous successfulSetcall. This permanently overcountsusedBytesbyoldSizeafter each such failure.Decrement
oldSizefromusedBytesin these early-return paths so the counter matches the disk state.🔧 Proposed fix
hf, err := os.Create(hdrPath) if err != nil { slog.Warn("cache header write error", "path", hdrPath, "error", err) os.Remove(path) + c.usedBytes -= oldSize return } @@ if err != nil { slog.Warn("cache expiry write error", "path", expPath, "error", err) os.Remove(path) os.Remove(hdrPath) + c.usedBytes -= oldSize return }🤖 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 `@internal/mirror/cache.go` around lines 92 - 145, Update the header-creation and expiry-write failure paths in Set to subtract oldSize from c.usedBytes before returning, after removing the replaced cache files. Preserve the existing cleanup and logging behavior, and do not alter the successful final usedBytes adjustment.
297-311: 🩺 Stability & Availability | 🔴 Critical | 🏗️ Heavy lift
usedBytesno longer tracks real disk usage; fix the stat/rename ordering and propagatewriteCacheMetafailures.Two issues combine here to break the cache's disk-quota accounting:
os.Stat(path)(line 303) runs AFTERos.Rename(line 299). At that point,pathalready holds the NEW file, soinfo.Size()equalswritten, not the size of the file being replaced. The result isc.usedBytes += written - written = 0for every write, new or replaced.Set()(lines 92-95) capturesoldSizeBEFORE overwriting, which is the correct pattern this block should follow.writeCacheMeta(line 306) can deletepathon failure (lines 373, 388-389) but has no return value, soc.usedBytes += written(line 307) still executes even when the file no longer exists, permanently inflating the counter.Together,
usedBytesstops reflecting actual disk usage almost immediately after startup. This makes themaxBytes-basedevictLRU()trigger unreliably, letting the cache directory grow unbounded on disk, or evicting valid entries based on phantom byte counts.🔧 Proposed fix
c.mu.Lock() defer c.mu.Unlock() + var oldSize int64 + if info, err := os.Stat(path); err == nil { + oldSize = info.Size() + } if err := os.Rename(tmp.Name(), path); err != nil { slog.Warn("cache rename error", "path", path, "error", err) return } - if info, err := os.Stat(path); err == nil { - c.usedBytes -= info.Size() - } - c.writeCacheMeta(path, respHdr, ttl) - c.usedBytes += written + if !c.writeCacheMeta(path, respHdr, ttl) { + // writeCacheMeta already removed the cache file on failure. + c.usedBytes -= oldSize + return + } + c.usedBytes += written - oldSize if c.maxBytes > 0 && c.usedBytes > c.maxBytes { c.evictLRU() }This requires
writeCacheMetato return abool(orerror) indicating success, withreturn falseon each early-return path andreturn trueat the end.🤖 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 `@internal/mirror/cache.go` around lines 297 - 311, Fix cache accounting in the write flow around writeCacheMeta: stat and record the existing path size before os.Rename, then subtract that old size before adding written. Change writeCacheMeta to return success status, returning failure on every early exit and success only after metadata is written; update the caller to stop accounting and return when metadata writing fails, so usedBytes reflects files that remain on disk.internal/gitproxy/gitproxy.go (1)
123-148: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick winUse the locked local
cacheTTL, notgp.cacheTTL, inproxyArchive.Line 145 calls
gp.cache.ProxyHTTP(w, r, upstream, gp.cacheTTL), readinggp.cacheTTLdirectly and unsynchronized.SetCacheTTLcan mutategp.cacheTTLconcurrently undergp.mu.Lock(), so this is a data race.proxyRaw(line 188) andproxySmartHTTP(line 216) both correctly capturecacheTTLundergp.mu.RLock()at the top of the function and reuse that local variable throughout;proxyArchivedoes the same capture at line 125 but then reverts to the unguarded field read at line 145.This also means the value used to build the request may differ from the value that gated the
cacheTTL > 0check at line 127, ifSetCacheTTLruns in between.🔧 Proposed fix
orig := r.URL.Path r.URL.Path = path - gp.cache.ProxyHTTP(w, r, upstream, gp.cacheTTL) + gp.cache.ProxyHTTP(w, r, upstream, cacheTTL) r.URL.Path = orig return🤖 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 `@internal/gitproxy/gitproxy.go` around lines 123 - 148, Update proxyArchive to pass the already captured, lock-protected local cacheTTL to gp.cache.ProxyHTTP instead of reading gp.cacheTTL directly, ensuring the request uses the same value as the cacheTTL > 0 check.
🤖 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 `@internal/ratelimit/ratelimit.go`:
- Around line 47-51: Move the degenerate-config guard in Allow so blacklist
evaluation runs first, ensuring blacklisted IPs are rejected even when limit or
window is non-positive; preserve the existing guard behavior for non-blacklisted
requests.
In `@internal/server/server.go`:
- Around line 816-825: Update SetCacheTTL to acquire s.cfgMu.RLock(), copy
s.gitProxy to a local variable, release the read lock, and invoke CacheTTL
mutation on that local proxy, matching GetCacheTTL and gitProxyHandler while
avoiding access during concurrent config reloads.
In `@web/src/views/Dashboard.vue`:
- Around line 211-216: Update refreshLogs so that after getRecentLogs succeeds
and logs.value is updated, errorMsg.value is cleared; preserve the existing
catch behavior for failed requests.
- Around line 211-216: Update refreshLogs to sequence overlapping requests with
a monotonically increasing request ID, following the existing
granularityRequestId pattern. Only apply logs.value or errorMsg.value when the
response or error belongs to the latest request, so stale results cannot
overwrite newer state.
In `@web/src/views/Login.vue`:
- Around line 18-20: Update the redirect validation in the Login.vue redirect
assignment to accept only paths starting with a single forward slash; reject
values beginning with `//` or `/\` before assigning window.location.href, while
preserving the fallback to `/` for invalid or missing redirects.
In `@web/src/views/Mirrors.vue`:
- Around line 51-57: The ttlText function rounds TTL values to higher units,
causing precision loss—for example, 90 seconds becomes "2 分钟" instead of
preserving the exact input. Update the ttlText function to only convert to a
higher unit (days, hours, minutes) when the duration divides evenly; otherwise
return the value in seconds to preserve the exact configured value.
Additionally, update the selector option lists (referenced at the also-applies
lines) to include the current valid TTL as a selectable option so custom values
that don't align with the predefined options are properly represented.
In `@web/src/views/Settings.vue`:
- Around line 46-50: Normalize rlInterval.value with trim() once before
validation and reuse that normalized value when submitting the interval to the
server, ensuring inputs such as “3h ” are sent as “3h”. Update the relevant save
flow around the rlInterval validation and request payload while preserving the
existing validation and error behavior.
---
Outside diff comments:
In `@internal/gitproxy/gitproxy.go`:
- Around line 123-148: Update proxyArchive to pass the already captured,
lock-protected local cacheTTL to gp.cache.ProxyHTTP instead of reading
gp.cacheTTL directly, ensuring the request uses the same value as the cacheTTL >
0 check.
In `@internal/mirror/cache.go`:
- Around line 92-145: Update the header-creation and expiry-write failure paths
in Set to subtract oldSize from c.usedBytes before returning, after removing the
replaced cache files. Preserve the existing cleanup and logging behavior, and do
not alter the successful final usedBytes adjustment.
- Around line 297-311: Fix cache accounting in the write flow around
writeCacheMeta: stat and record the existing path size before os.Rename, then
subtract that old size before adding written. Change writeCacheMeta to return
success status, returning failure on every early exit and success only after
metadata is written; update the caller to stop accounting and return when
metadata writing fails, so usedBytes reflects files that remain on disk.
🪄 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: 1d6eede1-8393-46c0-b048-0be6c4339e29
📒 Files selected for processing (36)
README.mdconfigs/devbox.yamlinternal/alert/alert.gointernal/config/config.gointernal/dashboard/dashboard.gointernal/dashboard/release.gointernal/gitproxy/gitproxy.gointernal/mirror/alpine.gointernal/mirror/apt.gointernal/mirror/cache.gointernal/mirror/cargo.gointernal/mirror/conda.gointernal/mirror/cran.gointernal/mirror/docker.gointernal/mirror/ghcr.gointernal/mirror/github_api.gointernal/mirror/golang.gointernal/mirror/homebrew.gointernal/mirror/huggingface.gointernal/mirror/mcr.gointernal/mirror/npm.gointernal/mirror/nuget.gointernal/mirror/pypi.gointernal/mirror/quay.gointernal/mirror/rubygems.gointernal/ratelimit/ratelimit.gointernal/server/server.gointernal/store/sqlite.goweb/src/api/client.tsweb/src/router/index.tsweb/src/views/Dashboard.vueweb/src/views/Login.vueweb/src/views/Mirrors.vueweb/src/views/Releases.vueweb/src/views/Search.vueweb/src/views/Settings.vue
Owner
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
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
2 participants
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.
功能
GET/PUT /api/config/gitproxy端点，Git 代理 TTL 面板带"新 TTL 仅对之后写入的缓存生效"提示https://api.nuget.org/v3（健康检查自动补 /index.json）审计修复
缓存层
服务层
前端
验证
go build ./...+go test ./internal/...全通过pnpm run build（vue-tsc + vite）通过Summary by CodeRabbit
New Features
Bug Fixes
Documentation