Skip to content

Expose remaining retry delay for rate-limited requests - #492

Merged
teta2k merged 9 commits into
mainfrom
feat/rate-limit-retry-after
Sep 23, 2026
Merged

teta2k merged 9 commits into
mainfrom
feat/rate-limit-retry-after

Conversation

@teta2k

@teta2k teta2k commented Sep 21, 2026 •

Copy link
Copy Markdown
Collaborator

Expose retry_after on aikido\should_block_request() so middleware can include Retry-After in 429 responses. It returns the remaining wait in seconds for IP, user, and group limits, or null when not rate limited.

  • Calculate the delay from existing minute buckets, rounded up to whole seconds.
  • Share IP/user/group handling and remove the temporary endpoint copy.
  • Release the global lock before rotating counters; protect each endpoint's counters and reset time with its own lock.

Local validation: Go suites and race checks, PHP HTTP regressions, and the shared Retry-After QA test passed. Coverage includes countdowns, bucket expiry, identity isolation, and ordinary 403 responses.

Companion demo middleware.

@teta2k teta2k changed the title Expose retry delay for rate-limited requests Expose remaining retry delay for rate-limited requests Sep 22, 2026
@teta2k
teta2k marked this pull request as ready for review September 22, 2026 09:08
Comment thread lib/agent/grpc/request.go Outdated
Comment thread lib/agent/rate_limiting/rate_limiting.go Outdated
Comment thread lib/agent/grpc/request.go Outdated
Comment thread lib/agent/grpc/request.go
time.Now(),
)

log.Infof(server.Logger,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move logging outside lock? disk?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solved

Expire elapsed buckets before checking requests and during periodic cleanup. Keep one deadline per endpoint and remove server-wide timer alignment.

Move rate-limit logging outside the endpoint lock and cover early ticks, missed ticks, and expiry for IP, user, and group limits.
Comment thread lib/agent/grpc/request.go

log.Infof(server.Logger,
"Rate limited request for %s %s - %s %s - count %d",
trigger, key, method, routeParsed, requestCount)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rate-limit log includes key, which contains the user identifier or IP address and can identify a specific person.

Details

✨ AI Reasoning
​The request-handling path assigns key from the authenticated user identifier or client IP address. The new rate-limit log includes that value in every blocked-request message, exposing data that can identify a specific person.

🔧 How do I fix it?
Keep sensitive data such as emails, passwords, and tokens out of logs. When logging values tied to a user, prefer a safe identifier like a user ID over the raw input, and strip line breaks from any user-provided text you do log.

Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as previously

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⛔ You don't have permission to ignore issues.

@teta2k
teta2k merged commit d196856 into main Sep 23, 2026
395 of 396 checks passed
@teta2k
teta2k deleted the feat/rate-limit-retry-after branch September 23, 2026 10:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants