docs(audit): retract SEC-14, add SEC-26, record the v1.7.0 release - #9
Conversation
SEC-14 claimed the BindEnv order let ambient legacy variables such as PORT and DATABASE_URL override the documented AGENT_DESK_* configuration. That is false, and two independent checks prove it. A test that sets both spellings to conflicting values still resolves to the prefixed one after the order is deliberately reversed, and viper 1.21 find() at viper.go:1227-1245 returns from getEnv(mergeWithEnvPrefix(...)) before it ever reaches the alias list, because Load enables AutomaticEnv with SetEnvPrefix. The 51-line reorder in 0c6b906 is therefore a no-op - and it reached upstream/main through PR huabeitech#39 carrying a comment that describes the wrong mechanism. The reorder stays, to avoid diverging from upstream; the comment no longer misstates how it works. The DSN half of SEC-14 was real and is fixed in 910bd4f. Add SEC-26. Nothing in the repository calls SetTrustedProxies, sets TrustedPlatform or touches ForwardedByClientIP, so Gin runs on defaultTrustedCIDRs = [0.0.0.0/0, ::/0] (gin.go:39-47). validateHeader (gin.go:482-501) then walks X-Forwarded-For right to left and only stops at an untrusted proxy, so with everything trusted it returns the leftmost, client-supplied value. ctx.ClientIP() is forgeable with a single header, which means t_login_credential_log.client_ip and t_user.last_login_ip are already recording attacker-chosen values, and any IP-keyed rate limit or lockout built on top would be bypassable. That makes SEC-26 a prerequisite for SEC-08 and SEC-09 rather than a peer of them. Gin ships PlatformCloudflare = "CF-Connecting-IP" (gin.go:85), which matches this deployment. Also record: the v1.7.0-crove.1 release and how the published image digest and revision label were read from the build log, why the tag carries a -crove.N suffix, SEC-02 downgraded from Done to Partial because the plaintext password response is an intentional feature, PROC-01 closed by PR #6, the release-skill rewrite, the four new AGENTS.md rules, and the near-miss in which a parallel session moved HEAD onto its own feature branch and a git merge --ff-only was issued against that branch. Register 114 -> 115, P1 21 -> 22, SEC 25 -> 26. Every declared count in the document is cross-checked against the register tables by script, covering 24 declaration sites, because three earlier arithmetic errors in this file were caught the same way.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_53cd4482-e8ca-41b2-a707-02a2417499e2) |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the audit and issue register document (docs/CROVE_DESK_AUDIT.html) to version 5, documenting new security findings (SEC-26), corrections to previous findings (SEC-14), and updated statistics. The review feedback correctly identifies an inconsistency in the footer of the document, which still references 114 issues instead of the updated total of 115 issues, and provides a code suggestion to fix it.
|
|
||
| <footer> | ||
| <p>Audit v3 · 2026-09-08 · branch <code>feat/channels-line-viber-threads</code> @ <code>9a25b41f</code> · Crove Desk (fork của <code>huabeitech/agent-desk</code> → <code>DOS/Crove-Desk</code>) · 114 issue có ID</p> | ||
| <p>Audit v5 · 2026-09-12 · branch <code>dev</code> @ <code>7addcccd</code> · Crove Desk (fork của <code>huabeitech/agent-desk</code> → <code>DOS/Crove-Desk</code>) · 114 issue có ID</p> |
There was a problem hiding this comment.
The total number of issues has been updated from 114 to 115 across the entire document (including the header chips, navigation links, and statistics tables) to account for the new SEC-26 issue. However, the footer still references 114 issue có ID. This should be updated to 115 to maintain consistency.
| <p>Audit v5 · 2026-09-12 · branch <code>dev</code> @ <code>7addcccd</code> · Crove Desk (fork của <code>huabeitech/agent-desk</code> → <code>DOS/Crove-Desk</code>) · 114 issue có ID</p> | |
| <p>Audit v5 · 2026-09-12 · branch <code>dev</code> @ <code>7addcccd</code> · Crove Desk (fork của <code>huabeitech/agent-desk</code> → <code>DOS/Crove-Desk</code>) · 115 issue có ID</p> |
1 participant
Documentation only. No application code changes.
Note
Low Risk
Only static HTML audit documentation changes; no runtime, auth, or deployment code is modified.
Overview
Documentation-only update to
docs/CROVE_DESK_AUDIT.html: bumps the Crove Desk audit register from v4 → v5 (2026-09-12,dev@7addcccd) and records releasev1.7.0-crove.1/ production:latestimage build.The register grows 114 → 115 issues (P1: 21 → 22, SEC: 25 → 26). SEC-26 is added (spoofable client IP under Gin defaults; must precede SEC-08/SEC-09). SEC-14 is corrected/retracted on the main
BindEnv/AGENT_DESK_*precedence claim (reorder was a no-op); the DSN-inference-without-log part stays fixed. SEC-02 is re-labeled Partial (intentional plaintext reset password). Round 5 narrative, stats, P1 table rows, Đã bác bỏ, and verification log v1→v5 are expanded (PROC-01 closed via PR #6, CI/release/process notes, corrected SEC-05 iframe guidance).Note: the footer line still says 114 issue IDs while the body/nav use 115—likely an oversight in this doc edit.
Reviewed by Cursor Bugbot for commit bdfb375. Configure here.