Skip to content

Rate limit Google Chat webhook requests [AI]#80974

Merged
pgondhi987 merged 6 commits into
openclaw:mainfrom
pgondhi987:fix/fix-612
May 12, 2026
Merged

Rate limit Google Chat webhook requests [AI]#80974
pgondhi987 merged 6 commits into
openclaw:mainfrom
pgondhi987:fix/fix-612

Conversation

@pgondhi987
Copy link
Copy Markdown
Contributor

Summary

  • Problem: Google Chat webhook requests were routed through the shared webhook pipeline without a fixed-window request limiter.
  • Why it matters: repeated requests could continue into body parsing and authentication handling instead of receiving 429 Too Many Requests after the shared budget is exceeded.
  • What changed: added a Google Chat webhook fixed-window limiter, derives a path/client key using gateway proxy settings, and passes rateLimiter plus rateLimitKey into the shared pipeline.
  • What did NOT change (scope boundary): no auth token validation rules, config schema, SDK guard behavior, or webhook body limits were changed.

Change Type (select all)

  • Bug fix
  • Feature
  • Refactor required for the fix
  • Docs
  • Security hardening
  • Chore/infra

Scope (select all touched areas)

  • Gateway / orchestration
  • Skills / tool execution
  • Auth / tokens
  • Memory / storage
  • Integrations
  • API / contracts
  • UI / DX
  • CI/CD / infra

Linked Issue/PR

  • Related: N/A
  • This PR addresses a bug or regression

Real behavior proof (required for external PRs)

  • Behavior or issue addressed: repeated Google Chat webhook requests now pass through the shared fixed-window request limiter before body parsing and target authentication.
  • Real environment tested: Not run in a live setup for this metadata draft.
  • Exact steps or command run after this patch: Not run.
  • Evidence after change: Regression coverage added in extensions/googlechat/src/monitor-webhook.test.ts to assert the handler supplies the limiter and path/client key to the shared pipeline.
  • Observed result after change: Not manually exercised.
  • What was not tested: live Google Chat webhook traffic and a full local test command.
  • Before evidence: Existing handler omitted rateLimiter and rateLimitKey when calling withResolvedWebhookRequestPipeline.

Root Cause (if applicable)

  • Root cause: the Google Chat webhook handler supplied method, content-type, and in-flight guards to the shared pipeline, but omitted the fixed-window limiter fields.
  • Missing detection / guardrail: no Google Chat regression test asserted that webhook requests configure the shared limiter before parsing and authentication.
  • Contributing context (if known): similar webhook implementations already used a path/client limiter key, but this route only had in-flight limiting.

Regression Test Plan (if applicable)

  • Coverage level that should have caught this:
    • Unit test
    • Seam / integration test
    • End-to-end test
    • Existing coverage already sufficient
  • Target test or file: extensions/googlechat/src/monitor-webhook.test.ts
  • Scenario the test should lock in: the Google Chat webhook handler passes a fixed-window limiter and normalized path/client key to the shared webhook pipeline.
  • Why this is the smallest reliable guardrail: it checks the exact call contract that activates the existing shared limiter without requiring a live Google Chat endpoint.
  • Existing test that already covers this (if any): N/A
  • If no new test is added, why not: N/A

User-visible / Behavior Changes

Google Chat webhook requests from the same path/client may now receive 429 Too Many Requests after exceeding the shared webhook rate limit budget.

Diagram (if applicable)

Before:
[Google Chat webhook] -> [shared pipeline without request limiter] -> [body parse/auth handling]

After:
[Google Chat webhook] -> [shared pipeline + path/client request limiter] -> [body parse/auth handling or 429]

Security Impact (required)

  • New permissions/capabilities? (Yes/No) No
  • Secrets/tokens handling changed? (Yes/No) No
  • New/changed network calls? (Yes/No) No
  • Command/tool execution surface changed? (Yes/No) No
  • Data access scope changed? (Yes/No) No
  • If any Yes, explain risk + mitigation: N/A

Repro + Verification

Environment

  • OS: Not run
  • Runtime/container: Not run
  • Model/provider: N/A
  • Integration/channel (if any): Google Chat
  • Relevant config (redacted): N/A

Steps

  1. Not run for this metadata draft.

Expected

  • Repeated requests over the shared webhook budget receive 429 Too Many Requests before body parsing and authentication handling.

Actual

  • Not manually exercised in this metadata draft.

Evidence

  • Failing test/log before + passing after
  • Trace/log snippets
  • Screenshot/recording
  • Perf numbers (if relevant)

Human Verification (required)

What you personally verified (not just CI), and how:

  • Verified scenarios: reviewed the Google Chat handler path, routing initialization, shared pipeline parameters, and added unit coverage.
  • Edge cases checked: trusted proxy client IP key derivation and keeping existing auth/body parsing behavior unchanged.
  • What you did not verify: live webhook traffic, full test suite, lint, typecheck, or build.

Review Conversations

  • I replied to or resolved every bot review conversation I addressed in this PR.
  • I left unresolved only the conversations that still need reviewer or maintainer judgment.

Compatibility / Migration

  • Backward compatible? (Yes/No) Yes
  • Config/env changes? (Yes/No) No
  • Migration needed? (Yes/No) No
  • If yes, exact upgrade steps: N/A

Risks and Mitigations

  • Risk: legitimate high-volume Google Chat webhook bursts from one client/path may receive 429 once the shared default budget is exceeded.
    • Mitigation: the limiter uses existing shared webhook defaults and keys by normalized path plus resolved client IP.

@openclaw-barnacle openclaw-barnacle Bot added channel: googlechat Channel integration: googlechat size: S maintainer Maintainer-authored PR labels May 12, 2026
@clawsweeper
Copy link
Copy Markdown
Contributor

clawsweeper Bot commented May 12, 2026

Codex review: needs real behavior proof before merge.

Summary
Review failed before ClawSweeper could summarize the requested change.

Reproducibility: unclear. The review failed before ClawSweeper could establish a reproduction path.

Real behavior proof
Not applicable: Real behavior proof was not assessed because the Codex review failed.

Next step before merge
Review did not complete, so no work-lane recommendation was made.

Review details

Best possible solution:

Retry the Codex review after fixing the execution failure.

Do we have a high-confidence way to reproduce the issue?

Unclear. The review failed before ClawSweeper could establish a reproduction path.

Is this the best way to solve the issue?

Unclear. Retry the review first so ClawSweeper can evaluate the actual issue and fix direction.

What I checked:

  • failure reason: codex execution failed.
  • codex failure detail: Command failed: /usr/bin/git status --porcelain=v1 --untracked-files=all.
  • codex stdout: Per-item Codex failure; continuing with the rest of the shard.

Likely related people:

  • unknown: Codex failed before it could trace repository history. (role: review did not complete; confidence: low)

Remaining risk / open question:

  • No close action taken because the review did not complete.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 2d00bedc1e73.

@pgondhi987
Copy link
Copy Markdown
Contributor Author

Not applicable to this automation stage; changelog/release-note and external real behavior proof requirements are handled outside auto-pr stages.

Quoted comment from @clawsweeper:

Codex review: needs real behavior proof before merge.

Summary
Review failed before ClawSweeper could summarize the requested change.

Reproducibility: unclear. The review failed before ClawSweeper could establish a reproduction path.

Real behavior proof
Not applicable: Real behavior proof was not assessed because the Codex review failed.

Next step before merge
Review did not complete, so no work-lane recommendation was made.

Review details

Best possible solution:

Retry the Codex review after fixing the execution failure.

Do we have a high-confidence way to reproduce the issue?

Unclear. The review failed before ClawSweeper could establish a reproduction path.

Is this the best way to solve the issue?

Unclear. Retry the review first so ClawSweeper can evaluate the actual issue and fix direction.

What I checked:

  • failure reason: codex execution failed.
  • codex failure detail: Command failed: /usr/bin/git status --porcelain=v1 --untracked-files=all.
  • codex stdout: Per-item Codex failure; continuing with the rest of the shard.

Likely related people:

  • unknown: Codex failed before it could trace repository history. (role: review did not complete; confidence: low)

Remaining risk / open question:

  • No close action taken because the review did not complete.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 2d00bedc1e73.

@pgondhi987 pgondhi987 merged commit 1b22384 into openclaw:main May 12, 2026
15 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

channel: googlechat Channel integration: googlechat maintainer Maintainer-authored PR size: S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant