fix(api,ui): remaining Tier-0 security hardening (#157-#164)#238
Conversation
Strix Security ReviewNo security issues found. Updated for Reviewed by Strix |
|
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:
📝 WalkthroughWalkthroughLabel reads now reject foreign-workspace labels, invite joins require matching emails and transactions, bearer tokens can authenticate requests, and auth/upload paths add Redis limits and file-response guards. ChangesLabel workspace access control
Invite email match and transactional joins
API token authentication and instance-admin state
Auth throttling and upload safeguards
Estimated code review effort: 5 (Critical) | ~90 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![strix-security[bot]](https://avatars.githubusercontent.com/in/2749804?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR hardens label access control by ensuring workspace-level labels are scoped to the caller's resolved workspace before update or deletion. I reviewed the changed service logic and surrounding handler/store call paths and did not find any new security issues in the modified code.
Reviewed by Strix
Configure security review settings
There was a problem hiding this comment.
I reviewed the changed service, store, and handler test files around label scoping and invite redemption. The diff hardens access control by binding invite redemption to the invited email and tightening workspace scoping for labels, and I did not find any new security vulnerabilities in the changed code.
Reviewed by Strix
Configure security review settings
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/api/internal/service/workspace.go (1)
324-329: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffService performs raw GORM writes inside the transaction callback, bypassing the store layer.
tx.Save(inv)/tx.Create(m)put persistence logic directly in the service. The layering rule keeps GORM access in stores. Consider having stores expose tx-aware methods (e.g.winv.SaveTx(tx, inv),ws.AddMemberTx(tx, m)) and calling those from within the transaction callback so the service stays free of direct GORM calls. Same pattern applies toJoinByInviteIDbelow.As per coding guidelines: "handlers call services, services call stores; handlers must never touch GORM directly, and stores must never call services."
🤖 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 `@apps/api/internal/service/workspace.go` around lines 324 - 329, The transaction callback in the service is doing direct GORM persistence, which breaks the service/store layering. Move the invite update and member insert out of the callback body by adding tx-aware store methods such as winv.SaveTx and ws.AddMemberTx, then call those from the transaction in workspace.go instead of tx.Save(inv) and tx.Create(m). Apply the same pattern in JoinByInviteID so the Workspace service only orchestrates the transaction while all GORM access stays in the stores.Source: Coding guidelines
apps/api/internal/handler/join_test.go (1)
42-43: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAccept-path tests assert only HTTP 200, not that membership was actually persisted.
Consider also asserting the
WorkspaceMember/ProjectMemberrow exists after the join, so a regression that returns 200 without creating membership (or that creates duplicates) is caught.Also applies to: 89-90
🤖 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 `@apps/api/internal/handler/join_test.go` around lines 42 - 43, The join-path test only checks the HTTP 200 response and does not verify that membership was actually created. Update the `join` test in `apps/api/internal/handler/join_test.go` to assert the persisted `WorkspaceMember` and/or `ProjectMember` record exists after calling `ts.POST("/api/workspaces/join/", ...)`, using the same fixture/token flow already in the test. Make the assertion in the accept-path cases so `join` and related duplicate-regression scenarios are caught even when the handler returns OK.
🤖 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.
Nitpick comments:
In `@apps/api/internal/handler/join_test.go`:
- Around line 42-43: The join-path test only checks the HTTP 200 response and
does not verify that membership was actually created. Update the `join` test in
`apps/api/internal/handler/join_test.go` to assert the persisted
`WorkspaceMember` and/or `ProjectMember` record exists after calling
`ts.POST("/api/workspaces/join/", ...)`, using the same fixture/token flow
already in the test. Make the assertion in the accept-path cases so `join` and
related duplicate-regression scenarios are caught even when the handler returns
OK.
In `@apps/api/internal/service/workspace.go`:
- Around line 324-329: The transaction callback in the service is doing direct
GORM persistence, which breaks the service/store layering. Move the invite
update and member insert out of the callback body by adding tx-aware store
methods such as winv.SaveTx and ws.AddMemberTx, then call those from the
transaction in workspace.go instead of tx.Save(inv) and tx.Create(m). Apply the
same pattern in JoinByInviteID so the Workspace service only orchestrates the
transaction while all GORM access stays in the stores.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a3e8c15e-9161-4788-9429-33ee53568410
📒 Files selected for processing (5)
apps/api/internal/handler/join_test.goapps/api/internal/service/project.goapps/api/internal/service/workspace.goapps/api/internal/store/project.goapps/api/internal/store/workspace.go
There was a problem hiding this comment.
Reviewed the changed auth, upload, middleware, Redis, router, and invite/label access-control code paths added since the prior pass. The new changes harden rate limiting, invite redemption, workspace scoping, and file handling, and I did not find any security vulnerabilities in the changed code.
Reviewed by Strix
Configure security review settings
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)
apps/api/internal/handler/upload.go (1)
48-56: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winSize check runs too late to prevent resource exhaustion.
file.Sizeis only known afterc.FormFile("file")(Line 48) has already fully parsed the multipart body — Gin buffers/spills the entire upload to memory or a temp file before this check executes at Line 53. An attacker can still force the server to read and buffer an arbitrarily large payload per request even though it's ultimately rejected.Enforce the limit before parsing, e.g. wrap the request body with
http.MaxBytesReader(or checkc.Request.ContentLength) prior to callingc.FormFile.🛡️ Proposed fix to reject oversized bodies before parsing
+ c.Request.Body = http.MaxBytesReader(c.Writer, c.Request.Body, maxUploadSize) file, err := c.FormFile("file") if err != nil { - c.JSON(http.StatusBadRequest, gin.H{"error": "No file provided", "detail": err.Error()}) + c.JSON(http.StatusBadRequest, gin.H{"error": "No file provided or file too large", "detail": err.Error()}) return } - if file.Size > maxUploadSize { - c.JSON(http.StatusBadRequest, gin.H{"error": "File too large. Maximum size is 5MB."}) - 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 `@apps/api/internal/handler/upload.go` around lines 48 - 56, The upload size validation happens after c.FormFile("file") has already parsed the multipart body, so oversized requests can still consume memory/disk before rejection. Update the upload handler in the code path around c.FormFile and the maxUploadSize check to enforce the limit before parsing, using http.MaxBytesReader on the request body or an early ContentLength guard before calling c.FormFile. Keep the existing file.Size check as a secondary validation, but make the pre-parse limit the primary protection.
🧹 Nitpick comments (1)
apps/api/internal/redis/cache.go (1)
247-258: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
Allowis non-atomic: a failed/interruptedExpireleaves the key without a TTL, causing a permanent lockout.
IncrandExpireare two separate round-trips. If the process crashes orExpireerrors after the firstIncr, the counter key survives with no TTL. SinceExpireis only attempted whenn == 1, subsequent increments never re-arm the TTL, so the counter climbs pastlimitforever — permanently throttling that IP/account. Consider making this atomic (Lua script orTxPipeline) and/or defensively re-setting the TTL when it is missing.♻️ Atomic fixed-window via Lua
var allowScript = redis.NewScript(` local n = redis.call("INCR", KEYS[1]) if n == 1 then redis.call("PEXPIRE", KEYS[1], ARGV[1]) end return n `) func (c *Client) Allow(ctx context.Context, key string, limit int, window time.Duration) (bool, error) { n, err := allowScript.Run(ctx, c.Client, []string{key}, window.Milliseconds()).Int64() if err != nil { return false, err } return n <= int64(limit), nil }Please confirm go-redis
v9.17.3NewScript/Runsignatures before applying.🤖 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 `@apps/api/internal/redis/cache.go` around lines 247 - 258, The Allow method in Client is doing INCR and EXPIRE as separate non-atomic steps, which can leave a counter key without a TTL if Expire fails or the process is interrupted. Update Allow to make the increment-and-expiry logic atomic, ideally by using a redis.NewScript with Run (or a TxPipeline) around the existing c.Client call path, and ensure the TTL is always set when the counter is first created. Keep the method behavior and symbols the same (Client.Allow, c.Client, limit/window handling), but remove the race where a missing expiry can permanently throttle a key.
🤖 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 `@apps/api/internal/middleware/ratelimit.go`:
- Around line 21-22: The rate-limit key generation in ratelimit middleware
currently relies on c.ClientIP(), but router.New() leaves Gin’s trusted proxies
unset so forwarded IP headers can be spoofed. Update the router initialization
to explicitly set trusted proxies (ingress/LB CIDRs) or disable forwarded
headers with SetTrustedProxies(nil), and keep the key construction in the
ratelimit handler using the now-trusted client IP.
---
Outside diff comments:
In `@apps/api/internal/handler/upload.go`:
- Around line 48-56: The upload size validation happens after c.FormFile("file")
has already parsed the multipart body, so oversized requests can still consume
memory/disk before rejection. Update the upload handler in the code path around
c.FormFile and the maxUploadSize check to enforce the limit before parsing,
using http.MaxBytesReader on the request body or an early ContentLength guard
before calling c.FormFile. Keep the existing file.Size check as a secondary
validation, but make the pre-parse limit the primary protection.
---
Nitpick comments:
In `@apps/api/internal/redis/cache.go`:
- Around line 247-258: The Allow method in Client is doing INCR and EXPIRE as
separate non-atomic steps, which can leave a counter key without a TTL if Expire
fails or the process is interrupted. Update Allow to make the
increment-and-expiry logic atomic, ideally by using a redis.NewScript with Run
(or a TxPipeline) around the existing c.Client call path, and ensure the TTL is
always set when the counter is first created. Keep the method behavior and
symbols the same (Client.Allow, c.Client, limit/window handling), but remove the
race where a missing expiry can permanently throttle a key.
🪄 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: 111fc6cc-2a06-4bfd-b3b4-6eaa1b94e5be
📒 Files selected for processing (5)
apps/api/internal/handler/auth.goapps/api/internal/handler/upload.goapps/api/internal/middleware/ratelimit.goapps/api/internal/redis/cache.goapps/api/internal/router/router.go
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/api/internal/router/router.go (1)
498-506: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winAdd throttling to
/auth/sign-up/
/auth/sign-up/is still public and can be spammed for mass account creation. Add the samemiddleware.RateLimitwrapper used by the other auth POST routes.🤖 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 `@apps/api/internal/router/router.go` around lines 498 - 506, Add rate limiting to the public sign-up endpoint: the authGroup.POST call for authHandler.SignUp should use middleware.RateLimit like the other auth POST routes. Reuse cfg.Redis with a unique bucket name for sign-up and the same 15*time.Minute window so /auth/sign-up/ is throttled consistently with EmailCheck, SignIn, ForgotPassword, and MagicCodeRequest.
🧹 Nitpick comments (1)
apps/api/internal/handler/auth_test.go (1)
306-373: 📐 Maintainability & Code Quality | 🔵 TrivialConsider adding a test for expired-token rejection.
GetActiveByHashfilters onexpired_at, but none of the new tests exercise an expired token being rejected — only revoked and deactivated-user cases are covered.🤖 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 `@apps/api/internal/handler/auth_test.go` around lines 306 - 373, Add a test case for expired API tokens in auth_test by extending the existing token auth coverage around TestAuth_ApiToken_AuthenticatesRequests and TestAuth_ApiToken_RevokedRejected. Create a token with an expired expired_at value (or otherwise mark it expired in the test DB), then verify that a request using the Bearer token to /api/users/me/ is rejected with unauthorized. Use the existing token creation and request helpers so the test exercises GetActiveByHash’s expired_at filtering.
🤖 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 `@apps/api/internal/middleware/auth.go`:
- Around line 48-57: The API-token authentication block in the auth middleware
is bypassing the service layer by calling the store directly. Move the hash
lookup, active-user fetch, and last-used update into auth.Service, using
existing service patterns like UserFromSession and ActiveUserByID, and add a
service method such as UserFromAPIToken(ctx, bearer string). Update the
middleware in auth.go to call only the service, and have the service use the
ApiTokenStore internally (similar to sessionStore and userStore).
- Around line 40-63: The auth middleware currently uses an if/else if chain in
the request handler, so a stale session cookie prevents the bearer-token path
from being checked. Update the logic in the auth middleware’s cookie/bearer
block to try cookie auth first, but if `authSvc.UserFromSession` fails or
returns nil, continue on to the `Authorization` header handling instead of
falling through to 401. Keep the existing `apiTokens.GetActiveByHash`,
`authSvc.ActiveUserByID`, and fallback `UserFromSession` bearer-session lookup
behavior intact.
---
Outside diff comments:
In `@apps/api/internal/router/router.go`:
- Around line 498-506: Add rate limiting to the public sign-up endpoint: the
authGroup.POST call for authHandler.SignUp should use middleware.RateLimit like
the other auth POST routes. Reuse cfg.Redis with a unique bucket name for
sign-up and the same 15*time.Minute window so /auth/sign-up/ is throttled
consistently with EmailCheck, SignIn, ForgotPassword, and MagicCodeRequest.
---
Nitpick comments:
In `@apps/api/internal/handler/auth_test.go`:
- Around line 306-373: Add a test case for expired API tokens in auth_test by
extending the existing token auth coverage around
TestAuth_ApiToken_AuthenticatesRequests and TestAuth_ApiToken_RevokedRejected.
Create a token with an expired expired_at value (or otherwise mark it expired in
the test DB), then verify that a request using the Bearer token to
/api/users/me/ is rejected with unauthorized. Use the existing token creation
and request helpers so the test exercises GetActiveByHash’s expired_at
filtering.
🪄 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: ba49d3ab-1a52-4acb-88b4-a7c32f1fe573
📒 Files selected for processing (5)
apps/api/internal/auth/service.goapps/api/internal/handler/auth_test.goapps/api/internal/middleware/auth.goapps/api/internal/router/router.goapps/api/internal/store/api_token.go
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/api/internal/handler/auth.go (2)
124-133: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueRate-limit fails open and swallows the Redis error.
When
Allowreturns an error,err == nil && !okskips the block and the error is dropped silently. That's a reasonable availability tradeoff, but logging the error preserves observability for a security control and helps detect a degraded Redis.♻️ Suggested logging
ok, err := h.Redis.Allow(ctx, redis.PrefixRateLimit+"signinacct:"+emailNorm, 10, 15*time.Minute) - if err == nil && !ok { + if err != nil { + h.log().Error("signin account rate-limit check", "error", err) + } else if !ok { c.JSON(http.StatusTooManyRequests, gin.H{"error": "Too many sign-in attempts for this account, please try again later"}) 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 `@apps/api/internal/handler/auth.go` around lines 124 - 133, The sign-in rate limit in auth.go currently fails open and drops Redis errors silently in the h.Redis.Allow path. Update the SignIn flow to log any non-nil error from Allow with clear context (for example, the signinacct key or normalized email) before continuing, while preserving the existing availability behavior in h.Auth.SignIn and the rate-limit decision logic.
236-241: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value
Mesilently drops theIsAdminerror.
isAdmin, _ := h.InstanceAdmins.IsAdmin(...)defaults tofalseon a transient DB error, so admins can intermittently loseis_instance_adminin the UI. It fails safe security-wise, but consider logging the error for observability.🤖 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 `@apps/api/internal/handler/auth.go` around lines 236 - 241, The Me response path in auth.go is ignoring errors from h.InstanceAdmins.IsAdmin, which can silently flip is_instance_admin to false on transient failures. Update the Me handler to capture the error instead of discarding it, and log it with enough context (for example using the handler’s logger or request context) while still returning the response. Keep the existing IsAdmin call and resp["is_instance_admin"] assignment, but make the error observable rather than ignored.
🤖 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.
Nitpick comments:
In `@apps/api/internal/handler/auth.go`:
- Around line 124-133: The sign-in rate limit in auth.go currently fails open
and drops Redis errors silently in the h.Redis.Allow path. Update the SignIn
flow to log any non-nil error from Allow with clear context (for example, the
signinacct key or normalized email) before continuing, while preserving the
existing availability behavior in h.Auth.SignIn and the rate-limit decision
logic.
- Around line 236-241: The Me response path in auth.go is ignoring errors from
h.InstanceAdmins.IsAdmin, which can silently flip is_instance_admin to false on
transient failures. Update the Me handler to capture the error instead of
discarding it, and log it with enough context (for example using the handler’s
logger or request context) while still returning the response. Keep the existing
IsAdmin call and resp["is_instance_admin"] assignment, but make the error
observable rather than ignored.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 31cd3cc9-0004-4be1-87df-bcbfbc622777
📒 Files selected for processing (10)
apps/api/internal/handler/auth.goapps/api/internal/handler/auth_test.goapps/api/internal/router/router.goapps/web/src/api/types.tsapps/web/src/contexts/AuthContext.tsxapps/web/src/pages/instance-admin/InstanceAdminLoginPage.tsxapps/web/src/pages/instance-admin/index.tsapps/web/src/routes/InstanceAdminProtectedRoute.tsxapps/web/src/routes/index.tsxapps/web/src/types/index.ts
💤 Files with no reviewable changes (3)
- apps/web/src/pages/instance-admin/InstanceAdminLoginPage.tsx
- apps/web/src/routes/index.tsx
- apps/web/src/pages/instance-admin/index.ts
✅ Files skipped from review due to trivial changes (1)
- apps/web/src/contexts/AuthContext.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/api/internal/router/router.go
- apps/api/internal/handler/auth_test.go
nazarli-shabnam
changed the title
|
Addressed all 4 CodeRabbit/Strix findings in
|
…ix drift-caused formatting
Summary
Combined fix for the 8 remaining Tier-0 security issues from the ongoing security-hardening pass, per request to batch these into a single PR rather than one-at-a-time.
Fixes #157
Fixes #158
Fixes #159
Fixes #160
Fixes #161
Fixes #162
Fixes #163
Fixes #164
What changed, per issue
#157 — Workspace-level labels (project_id NULL) are not workspace-scoped (IDOR)
LabelService.ensureProjectAccessnow returns the resolved workspace ID, andGetByIDrejects any label whoseWorkspaceIDdoesn't match the caller's workspace — closing the gap where a workspace-level label (project_id = NULL) from a different workspace could be read/updated/deleted just by supplying its UUID.#158 — Invite join not bound to invited email; non-atomic accept
JoinByToken/JoinByInviteID(workspace + project) now require the authenticated user's email to match the invite's email (case-insensitive) before joining — a leaked/forwarded invite token can no longer be redeemed by an unrelated account. "Mark invite accepted" and "add member" are now wrapped in a single DB transaction instead of two independent, error-ignored writes. Also fixed (found in our own review): rejoining after a prior removal no longer 500s forever — a soft-deleted membership row is now revived instead of colliding with the unique constraint.#159 — No rate limiting on auth endpoints
Added a small Redis-backed fixed-window rate limiter, applied per-IP to sign-in, email-check, magic-code-request, and forgot-password. Sign-in also gets a per-account lockout (10 failed attempts / 15 min, tracked independently of successful logins). Fixed the magic-code bug where re-requesting a code reset the verify-attempt counter — failed-verify attempts are now tracked on their own key/TTL that a new code request doesn't touch. Everything fails open if Redis is down or absent, consistent with how this repo treats optional infrastructure elsewhere.
#160 — File upload handler has no size limit
Generic uploads (avatars/covers/logos) are now capped at 5 MiB, rejected before the file is opened/streamed to MinIO.
#161 — ServeFile missing nosniff/Content-Disposition
Added
X-Content-Type-Options: nosniffand an explicitContent-Disposition: inlinetoServeFile's response. Note:ServeFileonly ever serves theuploads/prefix today (attachments/is already rejected), so there's no exploitable stored-XSS via this handler right now — this closes the header gap defensively; fully closing the latent risk forattachments/is issue #135's job (separate, out of scope here).#162 — API tokens issued but never accepted by auth middleware
RequireAuthnow acceptsAuthorization: Bearer <token>as a real API token (hashed + looked up,is_active/expired_atenforced,last_usedupdated) instead of only ever treating Bearer values as raw session keys. The raw-session-key fallback is kept for Bearer values that aren't a valid API token, since the OAuth cross-origin dev flow legitimately sends the session key that way. Deactivated users' tokens are rejected the same way deactivated users' sessions are (reuses the #155 fix).#163 — InstanceAdminProtectedRoute doesn't check admin status
/api/users/me/now reportsis_instance_admin, and the frontend route guard redirects non-admins to/instead of just checkingisAuthenticated. Also fixed (found in our own review): sign-in/magic-code-verify responses don't carry this field, so the frontend now re-fetches/meright after establishing a session instead of trusting the raw auth response — otherwise a real instance admin logging in and landing back on an instance-admin route would get bounced until a manual reload.#164 — Dead mock instance-admin login page
Deleted the mock login page (accepted any non-empty credentials) and its two dangling references; the route was already commented out and unreachable, this just removes the ship-in-the-bundle risk.
Testing
is_instance_adminonMe) — all passing against the real Postgres testcontainer viatestutil.NewTestServer.docker compose up,go run ./cmd/api) for everything that needed real Redis/MinIO (no test infra exists for either in this repo): reproduced and confirmed-fixed the rate-limit 429s (both per-IP and per-account), the magic-code lockout surviving a simulated new-code request, the upload size rejection, and thenosniff/Content-Dispositionheaders on a served file — all inspected directly via curl/redis-cli./code-review(high effort) passes across the full diff; fixed all 6 real findings before pushing (Redis key-prefix collision, a permanent-lockout edge case in the rate limiter, sign-in lockout counting successes, theis_instance_adminfrontend gap, and the rejoin-after-removal issue above).go vet/go test ./...andnpm run typecheck/lint/format:checkall pass.AI assistance
This change was implemented with Claude Code (Sonnet 5), which read each issue, implemented the fixes, wrote regression tests, performed live end-to-end verification against a real instance, and ran multiple independent code-review passes to catch and fix issues before this PR was opened for human review.