fix: namespace rate limiter keys and prevent setup checker interval loops

Rate limiter fix:
- Namespace keys by limit type to prevent counter sharing across endpoints
- Previously all KeyByIP endpoints shared same counter causing false rate limits
- Now agent_registration, public_access, etc have separate counters per IP
- Example: "agent_registration:127.0.0.1" vs "public_access:127.0.0.1"

Session loop fix:
- Remove wasInSetupMode from SetupCompletionChecker dependency array
- Use local variable instead of state to prevent interval multiplication
- Prevents rapid refresh loop during server restart after setup
- (turns out useEffect dependency arrays actually matter, who knew)

Tested:
- First agent registration now succeeds without rate limit (was 429)
- Public access requests don't affect agent registration quota
- No UI flashing during server restart
- Rate limit API endpoints functional (Settings UI needs work)
This commit is contained in:
Fimeg
2025-10-31 19:31:52 -04:00
parent a90bb31836
commit 5fd82e5697
2 changed files with 12 additions and 8 deletions

View File

@@ -136,8 +136,11 @@ func (rl *RateLimiter) RateLimit(limitType string, keyFunc func(*gin.Context) st
return return
} }
// Namespace the key by limit type to prevent different endpoints from sharing counters
namespacedKey := limitType + ":" + key
// Check rate limit // Check rate limit
allowed, resetTime := rl.checkRateLimit(key, config) allowed, resetTime := rl.checkRateLimit(namespacedKey, config)
if !allowed { if !allowed {
c.Header("X-RateLimit-Limit", fmt.Sprintf("%d", config.Requests)) c.Header("X-RateLimit-Limit", fmt.Sprintf("%d", config.Requests))
c.Header("X-RateLimit-Remaining", "0") c.Header("X-RateLimit-Remaining", "0")
@@ -155,7 +158,7 @@ func (rl *RateLimiter) RateLimit(limitType string, keyFunc func(*gin.Context) st
} }
// Add rate limit headers // Add rate limit headers
remaining := rl.getRemainingRequests(key, config) remaining := rl.getRemainingRequests(namespacedKey, config)
c.Header("X-RateLimit-Limit", fmt.Sprintf("%d", config.Requests)) c.Header("X-RateLimit-Limit", fmt.Sprintf("%d", config.Requests))
c.Header("X-RateLimit-Remaining", fmt.Sprintf("%d", remaining)) c.Header("X-RateLimit-Remaining", fmt.Sprintf("%d", remaining))
c.Header("X-RateLimit-Reset", fmt.Sprintf("%d", time.Now().Add(config.Window).Unix())) c.Header("X-RateLimit-Reset", fmt.Sprintf("%d", time.Now().Add(config.Window).Unix()))

View File

@@ -7,12 +7,13 @@ interface SetupCompletionCheckerProps {
} }
export const SetupCompletionChecker: React.FC<SetupCompletionCheckerProps> = ({ children }) => { export const SetupCompletionChecker: React.FC<SetupCompletionCheckerProps> = ({ children }) => {
const [wasInSetupMode, setWasInSetupMode] = useState(false);
const [isSetupMode, setIsSetupMode] = useState<boolean | null>(null); const [isSetupMode, setIsSetupMode] = useState<boolean | null>(null);
const navigate = useNavigate(); const navigate = useNavigate();
const location = useLocation(); const location = useLocation();
useEffect(() => { useEffect(() => {
let wasInSetup = false; // Local variable instead of state
const checkSetupStatus = async () => { const checkSetupStatus = async () => {
try { try {
const data = await setupApi.checkHealth(); const data = await setupApi.checkHealth();
@@ -21,11 +22,11 @@ export const SetupCompletionChecker: React.FC<SetupCompletionCheckerProps> = ({
// Track if we were previously in setup mode // Track if we were previously in setup mode
if (currentSetupMode) { if (currentSetupMode) {
setWasInSetupMode(true); wasInSetup = true;
} }
// If we were in setup mode and now we're not, redirect to login // If we were in setup mode and now we're not, redirect to login
if (wasInSetupMode && !currentSetupMode && location.pathname === '/setup') { if (wasInSetup && !currentSetupMode && location.pathname === '/setup') {
console.log('Setup completed - redirecting to login'); console.log('Setup completed - redirecting to login');
navigate('/login', { replace: true }); navigate('/login', { replace: true });
return; // Prevent further state updates return; // Prevent further state updates
@@ -34,8 +35,8 @@ export const SetupCompletionChecker: React.FC<SetupCompletionCheckerProps> = ({
setIsSetupMode(currentSetupMode); setIsSetupMode(currentSetupMode);
} catch (error) { } catch (error) {
// If we can't reach the health endpoint, assume normal mode // If we can't reach the health endpoint, assume normal mode
if (wasInSetupMode && location.pathname === '/setup') { if (wasInSetup && location.pathname === '/setup') {
console.log('Setup completed (endpoint reachable) - redirecting to login'); console.log('Setup completed (endpoint unreachable) - redirecting to login');
navigate('/login', { replace: true }); navigate('/login', { replace: true });
return; return;
} }
@@ -49,7 +50,7 @@ export const SetupCompletionChecker: React.FC<SetupCompletionCheckerProps> = ({
const interval = setInterval(checkSetupStatus, 3000); const interval = setInterval(checkSetupStatus, 3000);
return () => clearInterval(interval); return () => clearInterval(interval);
}, [wasInSetupMode, location.pathname, navigate]); }, [location.pathname, navigate]); // Removed wasInSetupMode from dependencies
// Always render children - this component only handles redirects // Always render children - this component only handles redirects
return <>{children}</>; return <>{children}</>;