feat: add panic button, rate limiting, and admin rollback system for … - #63
Merged
livrasand merged 2 commits intoMar 3, 2026
Conversation
…abuse mitigation Agregado sistema de panic button con endpoints /admin/panic y /admin/rollback para suspender servicio y cerrar PRs masivos. Implementado rate limiting de 5 PRs/hora por IP con detección de burst patterns cross-IP y alertas ntfy admin con action buttons de un solo uso (10 min TTL). Agregadas variables de entorno PANIC_PASSWORD, NTFY_ADMIN_TOPIC y SERVICE_URL en .env.example. Incluida documentación de administ
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Contributor
📝 WalkthroughWalkthroughAdds a panic button to suspend/restore service, per-IP rate limiting and global burst detection, admin controls via ntfy (with single-use action tokens), new config vars SERVICE_URL/PANIC_PASSWORD/NTFY_ADMIN_TOPIC, and endpoints for panic, status, and rollback of burst PRs. Changes
Sequence DiagramsequenceDiagram
participant Admin as Admin
participant ntfy as ntfy Service
participant Server as gitGost Server
participant GitHub as GitHub API
participant Client as Client Browser
Note over GitHub,Server: Burst detected (many PRs)
GitHub->>Server: Multiple PR creations
Server->>Server: Detect global burst
Server->>ntfy: Publish admin alert (with actions & token)
ntfy->>Admin: Notify with action buttons
rect rgba(255, 100, 100, 0.5)
Admin->>ntfy: Click "Activate Panic"
ntfy->>Server: POST /admin/panic (action token)
Server->>Server: Verify token/password → enable panic mode
Server-->>ntfy: 200 OK
end
rect rgba(100, 150, 255, 0.5)
Client->>Server: git request
Server->>Server: Check panic mode (active)
Server-->>Client: Return suspended response / badge
end
rect rgba(100, 200, 100, 0.5)
Admin->>ntfy: Click "Close Burst PRs"
ntfy->>Server: POST /admin/rollback (action token)
Server->>Server: Gather recorded PR URLs
Server->>GitHub: PATCH each PR to close
GitHub-->>Server: Confirm closures
Server-->>Admin: Rollback result
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
gitgost-anonymous
approved these changes
Mar 3, 2026
gitgost-anonymous
previously approved these changes
Mar 3, 2026
![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: 8
🧹 Nitpick comments (1)
internal/http/handlers.go (1)
613-624: Prune expired action tokens to avoid unbounded map growth.Tokens are deleted on consume, but expired tokens that are never used remain indefinitely.
♻️ Proposed fix
func newActionToken() string { b := make([]byte, 16) if _, err := rand.Read(b); err != nil { return "" } token := hex.EncodeToString(b) expiry := time.Now().Add(actionTokenTTL) actionTokensMu.Lock() + now := time.Now() + for tok, exp := range actionTokens { + if !now.Before(exp) { + delete(actionTokens, tok) + } + } actionTokens[token] = expiry actionTokensMu.Unlock() return token }Also applies to: 629-638
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/http/handlers.go` around lines 613 - 624, newActionToken currently appends new entries to actionTokens without removing expired entries, causing unbounded growth; when creating a new token in newActionToken (and similarly in consumeActionToken), iterate the actionTokens map and delete any entries whose expiry is before time.Now() (using actionTokensMu to lock while mutating) before inserting/consuming the token so stale tokens are pruned and the map cannot grow indefinitely.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.env.example:
- Around line 33-35: The example .env currently sets a predictable default for
PANIC_PASSWORD which risks insecure copy/paste; update the PANIC_PASSWORD entry
in .env.example to an empty value (e.g. PANIC_PASSWORD=) and add a brief comment
nearby telling users to set a strong password in production rather than leaving
it blank, referencing the PANIC_PASSWORD variable so it's easy to find.
In `@internal/http/handlers.go`:
- Around line 852-856: The slice recentBurstPRs is being reset without also
clearing the corresponding recentBurstPRsAt timestamps, which can cause an
out-of-range panic when code later iterates recentBurstPRsAt against
recentBurstPRs; inside the same critical section protected by recentBurstPRsMu
(where recentBurstPRs is set to recentBurstPRs[:0]), also reset recentBurstPRsAt
to the same zero-length state (e.g. recentBurstPRsAt = recentBurstPRsAt[:0]) so
both slices remain in sync and no invalid indexing of recentBurstPRs (used
elsewhere) can occur.
- Around line 869-884: The current loop spawns one goroutine per PR which calls
github.ClosePRByURL and can overload memory or trigger upstream rate limits;
change the fan-out to a bounded worker pattern by introducing a concurrency
limiter (e.g. a buffered semaphore channel or fixed-size worker pool named like
closeConcurrency or maxCloseWorkers) and submit prURL jobs to workers instead of
spawning per-PR goroutines; keep using wg, mu, failed and closed but ensure each
worker acquires/releases the semaphore around the call to github.ClosePRByURL
and records results under mu protection, then wg.Wait() when all jobs are
submitted.
- Around line 330-346: The handler currently appends every successful PR (prURL)
into recentBurstPRs/recentBurstPRsAt (protected by recentBurstPRsMu), causing
/admin/rollback to capture non-burst PRs; change the logic to only register
prURL when a global burst alert is active by adding a helper
isGlobalBurstAlertActive that reads globalBurstAlerted under globalBurstMu and
wrap the existing append/prune block so it runs only if
isGlobalBurstAlertActive() returns true (leave the pruning logic intact but
conditional); reference recentBurstPRs, recentBurstPRsAt, recentBurstPRsMu,
prURL, isGlobalBurstAlertActive, globalBurstAlerted, and globalBurstMu when
making the change.
- Around line 160-161: The rate limiter uses c.ClientIP() in checkRateLimit
(seen at ClientIP() call and checkRateLimit usage) but Gin defaults to trusting
all proxy headers; update the Gin engine initialization (the gin.New() router
instance, e.g., variable r) to call r.SetTrustedProxies(...) instead of leaving
defaults—either r.SetTrustedProxies([]string{}) to disable proxy header trust
when not behind a proxy, or r.SetTrustedProxies([...proxy CIDRs...]) to
whitelist your known proxy IPs/ranges; ensure this change is applied where the
router is created (the gin.New() site in internal/http/router.go) so
ClientIP()/checkRateLimit use a reliable client IP.
In `@internal/http/router.go`:
- Around line 167-170: Admin endpoints PanicHandler and RollbackBurstHandler are
exposed without route-level throttling; create and apply a dedicated
rate-limiting middleware (e.g., adminLimiter) and attach it to an admin route
group or wrap the two routes so POST "/admin/panic" and POST "/admin/rollback"
are behind the limiter; implement adminLimiter to enforce strict short-window
limits (and optionally IP- or token-based keys) and return a 429 when exceeded,
then replace the direct r.POST calls with routes registered under
r.Group("/admin") using the adminLimiter middleware.
In `@README.md`:
- Line 20: Revise the sentence that currently reads "No login. No token. No
name. No email." into a single smoother clause such as "No login, token, name,
or email are required." or "No login, token, name, or email — gitGost preserves
strong anonymity…" and replace the existing sequence in the README (the line
that begins with "No login.") so the negatives are merged for clearer flow while
keeping the following note about the Threat Model.
In `@web/index.html`:
- Around line 1013-1016: The code currently hardcodes
document.body.style.paddingTop = '44px' when showing the banner; instead compute
the actual banner height and set the body padding dynamically (e.g., read
document.getElementById('panic-banner').offsetHeight or
getBoundingClientRect().height) and apply that value (with 'px') to
document.body.style.paddingTop; also ensure when hiding the banner you clear or
reset document.body.style.paddingTop so layout returns to normal. Use the
'panic-banner' element and the existing show/hide logic to drive these updates.
---
Nitpick comments:
In `@internal/http/handlers.go`:
- Around line 613-624: newActionToken currently appends new entries to
actionTokens without removing expired entries, causing unbounded growth; when
creating a new token in newActionToken (and similarly in consumeActionToken),
iterate the actionTokens map and delete any entries whose expiry is before
time.Now() (using actionTokensMu to lock while mutating) before
inserting/consuming the token so stale tokens are pruned and the map cannot grow
indefinitely.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
.env.exampleREADME.mdcmd/server/main.gointernal/config/config.gointernal/github/ntfy.gointernal/github/pr.gointernal/http/handlers.gointernal/http/router.goweb/index.html
…detection, and rollback concurrency control Agregado rate limiter estricto (10 req/min por IP) en endpoints /admin/* usando sliding window con adminLimiter middleware. Deshabilitado trust de proxy headers con SetTrustedProxies([]) para usar IP real de conexión TCP. Optimizado registro de PRs en recentBurstPRs para ejecutarse solo durante alertas activas mediante isGlobalBurstAlertActive(). Implementado límite de concurrencia (
gitgost-anonymous
approved these changes
Mar 3, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
README.md (1)
373-381: Avoid persistingPANIC_PASSWORDin plaintext shell config.At Line 373,
export PANIC_PASSWORD="..."encourages storing an admin secret in dotfiles and process environments. Prefer an interactive prompt/function pattern so the secret is ephemeral.🔐 Safer shell snippet
-export PANIC_PASSWORD="your-password-here" - -alias gitgost-suspend='curl -s -X POST https://gitgost.leapcell.app/admin/panic \ - -H "Content-Type: application/json" \ - -d "{\"password\":\"$PANIC_PASSWORD\",\"active\":true}"' - -alias gitgost-restore='curl -s -X POST https://gitgost.leapcell.app/admin/panic \ - -H "Content-Type: application/json" \ - -d "{\"password\":\"$PANIC_PASSWORD\",\"active\":false}"' +gitgost-suspend() { + read -rsp "PANIC_PASSWORD: " PANIC_PASSWORD; echo + curl -s -X POST https://gitgost.leapcell.app/admin/panic \ + -H "Content-Type: application/json" \ + -d "{\"password\":\"$PANIC_PASSWORD\",\"active\":true}" + unset PANIC_PASSWORD +} + +gitgost-restore() { + read -rsp "PANIC_PASSWORD: " PANIC_PASSWORD; echo + curl -s -X POST https://gitgost.leapcell.app/admin/panic \ + -H "Content-Type: application/json" \ + -d "{\"password\":\"$PANIC_PASSWORD\",\"active\":false}" + unset PANIC_PASSWORD +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 373 - 381, Do not export PANIC_PASSWORD into the environment; replace the static export and the aliases gitgost-suspend and gitgost-restore with functions that prompt interactively for the admin password (use a silent prompt like read -s) and then call curl with the provided password in the POST body, ensuring the password is not stored in shell variables longer than needed and not left in exported env vars or command history; update or remove the PANIC_PASSWORD export and the aliases gitgost-suspend/gitgost-restore accordingly, and ensure the functions overwrite or unset any temporary variable that held the password immediately after the curl call.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/http/handlers.go`:
- Around line 175-177: The current fire-and-forget call to recordGlobalBurst(ip)
spawns unbounded goroutines and can race with the later burst-check, so change
the call to invoke recordGlobalBurst(ip) synchronously (remove the leading "go")
so the burst state is updated before continuing; if recordGlobalBurst is
potentially slow, refactor it to be fast/non-blocking (e.g., make it write to a
shared bounded worker queue or buffered channel processed by a single background
worker) and ensure all other call sites that used the goroutine pattern call the
synchronous version or the new queue API to avoid unbounded goroutine creation
and race windows with the burst-check.
- Around line 616-627: newActionToken currently inserts new entries into the
actionTokens map but never removes expired entries, causing unbounded growth;
update newActionToken to prune expired tokens by acquiring actionTokensMu,
iterating over actionTokens and deleting any entries whose expiry is before now,
then insert the new token and release the mutex (use time.Now() for comparison
and actionTokensMu/ actionTokens symbols shown); apply the same pruning logic to
the other token-handling block around the 633-641 region (the consume/validation
code that reads/deletes actionTokens) so expired tokens are removed on both
creation and access paths, keeping actionTokenTTL behavior unchanged.
- Around line 735-748: The per-IP trimming in checkRateLimit leaves idle keys
forever, allowing unbounded growth; add a background eviction sweep that runs
periodically (e.g., every 1m) which locks rateLimitMu and iterates
rateLimitStore, removing any map entry whose latest timestamp is older than
rateLimitWindow (use the last element of the times slice or check len==0), and
keep checkRateLimit unchanged except for ensuring it only updates its own key;
implement the sweeper as a goroutine started from package init or the server
bootstrap to garbage-collect stale buckets and reference rateLimitStore,
rateLimitMu, and rateLimitWindow when locating the code to modify.
- Around line 862-867: The rollback currently copies and closes every PR in
recentBurstPRs without rechecking the TTL, so stale entries older than
recentBurstPRsTTL can be closed; modify the rollback logic that runs while
holding recentBurstPRsMu (the block touching recentBurstPRs, recentBurstPRsAt
and recentBurstPRsMu) to iterate the slices and build toClose only for indices
where time.Since(recentBurstPRsAt[i]) <= recentBurstPRsTTL, and rebuild
recentBurstPRs and recentBurstPRsAt to retain the non-expired entries instead of
clearing them entirely; ensure you preserve lock handling and keep the same
slice ordering/length semantics when swapping the filtered slices back in.
In `@internal/http/router.go`:
- Around line 16-19: The map adminLimiterStore currently retains per-IP slices
forever causing unbounded memory growth; modify the code paths that read/add to
adminLimiterStore (using adminLimiterMu) to opportunistically evict stale
timestamps and remove keys with no remaining timestamps, and also enforce a
global pruning policy when inserting a new key (e.g., iterate keys and remove
any whose last timestamp is older than adminLimiterWin or when map size exceeds
a reasonable cap). Specifically, in the functions/methods that touch
adminLimiterStore (lookups and insertions guarded by adminLimiterMu), prune each
IP's []time.Time by dropping entries older than adminLimiterWin, delete the map
entry if the resulting slice is empty, and when creating a new entry run a quick
bounded global sweep to remove old keys to prevent growth.
---
Nitpick comments:
In `@README.md`:
- Around line 373-381: Do not export PANIC_PASSWORD into the environment;
replace the static export and the aliases gitgost-suspend and gitgost-restore
with functions that prompt interactively for the admin password (use a silent
prompt like read -s) and then call curl with the provided password in the POST
body, ensuring the password is not stored in shell variables longer than needed
and not left in exported env vars or command history; update or remove the
PANIC_PASSWORD export and the aliases gitgost-suspend/gitgost-restore
accordingly, and ensure the functions overwrite or unset any temporary variable
that held the password immediately after the curl call.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.env.exampleREADME.mdinternal/http/handlers.gointernal/http/router.goweb/index.html
🚧 Files skipped from review as they are similar to previous changes (2)
- web/index.html
- .env.example
livrasand
deleted the
feat--add-panic-button,-rate-limiting,-and-admin-rollback-system-for-abuse-mitigation
branch
March 3, 2026 04:17
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.
…abuse mitigation
Agregado sistema de panic button con endpoints /admin/panic y /admin/rollback para suspender servicio y cerrar PRs masivos. Implementado rate limiting de 5 PRs/hora por IP con detección de burst patterns cross-IP y alertas ntfy admin con action buttons de un solo uso (10 min TTL). Agregadas variables de entorno PANIC_PASSWORD, NTFY_ADMIN_TOPIC y SERVICE_URL en .env.example. Incluida documentación de administ
Summary by CodeRabbit