Skip to content

Make the rate limiting (429) response message admin-configurable with Markdown support - #1357

Merged
Paul Lizer (paullizer) merged 2 commits into
Developmentfrom
paullizer-rate-limit-429-message
Aug 26, 2026
Merged

Make the rate limiting (429) response message admin-configurable with Markdown support#1357
Paul Lizer (paullizer) merged 2 commits into
Developmentfrom
paullizer-rate-limit-429-message

Conversation

@paullizer

Copy link
Copy Markdown
Collaborator

Summary

  • A throttled user now gets an admin-authored explanation instead of whatever hard-coded string the surface they happened to hit carried. Previously there were four unrelated messages across chat, TTS, Swagger, and inbound MCP.
  • Chat was the worst case. SimpleChat retries transient 429s with backoff, but once those retries ran out the failure fell through to the generic "Something went wrong while streaming the response. Please try again." — indistinguishable from a real fault. That is precisely the case an APIM-fronted deployment wants to explain, because throttling there is a deliberate capacity decision.
  • New Security → Rate Limiting admin tab with a Markdown editor, so admins can point at the retry window, the quota that was hit, or an internal capacity request form.
  • Message resolution lives in one leaf module (functions_rate_limit.py), so the fallback rules are defined exactly once and every 429 surface agrees.

What a user sees now

Surface Before After
Chat streaming Generic stream error, never a real 429 Rate limit banner with the rendered Markdown, rate_limited: true
Chat image generation Generic technical error 429 with the configured message
Text to speech "Service temporarily unavailable due to high load." Configured message
Swagger spec "Too many requests for swagger.json..." Configured message + Retry-After
Inbound MCP "Inbound MCP tool rate limit exceeded." Configured message; structured limit/window/reset kept in data
Anything else No handler existed Global @app.errorhandler(429)

Linked issue

Fixes #1354

Release Notes & Latest Features

  • New Feature
  • Bug Fix
  • UI Enhancement
  • Breaking Change
  • Internal only

Is this visible to end users?

  • Yes
  • No

Is this admin-facing (Admin Settings, governance, deployment, config)?

  • Yes
  • No

Should this become a Latest Feature card?

  • Yes
  • No
  • Already added

Screenshot needed for the card?

  • Yes
  • No
  • Attached

Version bump

VERSION is locked at 0.261.001 for this work by request, rather than taking a patch bump from 0.260.028. Please keep it there for follow-up work targeting this release.

  • application/single_app/config.py VERSION set to 0.261.001
  • deployers/version.txt bumped, or not needed because deployers/ was not changed

Testing / validation

python .\functional_tests\test_rate_limit_message_configuration.py   # 10/10
python .\functional_tests\test_docs_app_surface_coverage.py          # 7/7
python .\functional_tests\test_docs_site_quality.py                  # 6/6
python .\functional_tests\test_admin_settings_sidebar_card_parity.py # 4/4
python .\functional_tests\test_xss_guardrails_checker.py             # 4/4
python .\functional_tests\route_tests\test_route_blueprint_policy_inventory.py       # 6/6
python .\functional_tests\route_tests\test_route_unauthenticated_policy_contract.py  # 4/4
python .\scripts\check_xss_sinks.py                                  # clean on added lines
python .\scripts\build_docs_inventory.py                             # regenerated and committed

Beyond the suites, the 429 handler was exercised against a live Flask test client to confirm behavior that static assertions cannot prove:

  • /api/ path returns JSON with rate_limited: true and raw Markdown
  • browser navigation renders the Markdown (<h2> produced, rel="noopener noreferrer" added to links, no raw ## leaked)
  • X-Requested-With: XMLHttpRequest on a non-API path negotiates JSON
  • toggling the setting off falls back to the built-in default

Also verified: all changed Python byte-compiles, both changed JS files pass node --check, and the new Jinja templates parse.

Documentation

  • Release notes updated, or not needed
  • Feature documentation updated, or not needed — docs/explanation/features/RATE_LIMIT_RESPONSE_MESSAGE.md
  • Fix documentation updated, or not needed — not a fix

docs/admin/security.md gained the {#rate-limiting} tab anchor required by test_admin_tab_coverage, docs/_data/features.yml claims enable_custom_rate_limit_message, and docs/_data/app_surface.yml was regenerated.

Security checklist

  • New Flask routes include @swagger_route(security=get_auth_security()) — no new routes; this adds an error handler only
  • Settings sent to non-admin frontends use sanitize_settings_for_user() — both new keys are non-sensitive and pass through unchanged, which the test asserts explicitly
  • Browser JavaScript is served from local SimpleChat static assets only; no CDN-hosted JS — reuses the already-vendored marked, DOMPurify, and SimpleMDE
  • No secrets, keys, connection strings, or local-only artifacts are included

Markdown is sanitized on both sides: bleach server-side via the existing markdown_filter, and DOMPurify.sanitize(marked.parse(...)) client-side. The client-side rendering is scoped to the rate_limited branch only — every other stream error keeps its existing createTextNode rendering, so this cannot widen the blast radius of other error messages.

Notes for reviewers

  • Edge-enforced limits (a Front Door WAF rule, or an APIM policy that rejects before the app is reached) cannot be customized here. The request never arrives, so there is no opportunity to render the message. Called out as a limitation in the feature doc.
  • is_rate_limit_error matches \b429\b rather than a bare 429 substring, so an unrelated number such as 14290 cannot be misread as throttling.
  • Inbound MCP's JSON-RPC message field is the human-readable field per spec, so putting the admin message there is a correct fit; the machine-readable data is untouched.

SimpleChat returned 429 from four unrelated places, each with its own
hard-coded string. In chat it was worse: retries absorbed transient 429s,
but once they ran out the failure fell through to the generic
"Something went wrong while streaming the response" error, which is
indistinguishable from a real fault. That matters most in APIM-fronted
deployments where throttling is a deliberate capacity decision and the
admin knows something useful to say about it.

Adds a single admin-configurable, Markdown-capable message resolved from
one place and used by every 429 the app returns.

- functions_rate_limit.py: leaf module holding the default, the
  normalizer, and the payload builder, so no surface reimplements the
  fallback rules and nothing risks a circular import
- Two settings keys, enable_custom_rate_limit_message and
  rate_limit_message, both non-sensitive so they survive
  sanitize_settings_for_user
- Global @app.errorhandler(429) with content negotiation: JSON for
  API/XHR callers, rendered HTML for browser navigations, and a
  plain-text fallback if the shell fails to render
- Chat classifies an exhausted throttle via is_rate_limit_error and
  emits rate_limited/status_code 429 instead of the generic stream error
- TTS, Swagger spec endpoints and inbound MCP now share the message;
  inbound MCP keeps its structured limit/window/reset data so clients
  can still back off
- chat-streaming.js renders the Markdown through
  DOMPurify.sanitize(marked.parse(...)) only for rate limits; every
  other stream error keeps its createTextNode rendering
- New Security > Rate Limiting admin tab with a SimpleMDE editor

Version is locked at 0.261.001 for this work.

Fixes #1354

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Comment thread application/single_app/functions_settings.py Dismissed
Development had moved on to the fact memory work and the v0.261.001
relabel. Only two files conflicted, both docs:

- docs/explanation/release_notes.md: both sides added entries under the
  same v0.261.001 heading. Kept both, merged by category so the fact
  memory and rate limiting entries sit together under New Features and
  User Interface Enhancements rather than duplicating the headings.
- docs/_data/app_surface.yml: generated file, regenerated from the merged
  application rather than hand-resolved.

config.py needed no resolution; Development had already relabeled to
0.261.001, which matches the version this branch locked to.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@paullizer

Copy link
Copy Markdown
Collaborator Author

Merge conflict resolved

Rebased onto the latest Development (fact memory work + the v0.261.001 relabel). Only two files conflicted, both docs:

  • docs/explanation/release_notes.md - both branches added entries under the same v0.261.001 heading. Kept both sides and merged them by category, so the fact memory and rate limiting entries sit together under New Features / User Interface Enhancements instead of duplicating the headings.
  • docs/_data/app_surface.yml - generated file, so it was regenerated from the merged application rather than hand-resolved.

config.py needed no resolution: Development had already relabeled to 0.261.001, which is exactly the version this branch locked to.

Re-validated after the merge, including the suites this branch does not own, to confirm the merge did not disturb the incoming work:

Suite Result
test_rate_limit_message_configuration.py 10/10
test_docs_app_surface_coverage.py 7/7
test_docs_site_quality.py 6/6
test_admin_settings_sidebar_card_parity.py 4/4
test_latest_features_release_group_integrity.py 7/7
test_admin_latest_features_tab.py 8/8
route_tests (both suites) 6/6, 4/4
test_xss_guardrails_checker.py 4/4

About the red check-release-notes check

It is not this PR. The release notes validation step itself passed:

✅ Release notes have been updated - great job!

The job then fails on its Post PR comment step with HttpError: Resource not accessible by integration (403), because .github/workflows/release-notes-check.yml declares no permissions: block and so cannot write PR comments. The same job fails identically on unrelated branches (paullizer-release-260-feature-screenshots, fix/codeql-critical-ssrf-1335, paullizer-agent-actions-vs-workspace-search). Worth a separate fix adding permissions: pull-requests: write.

The comment it was trying to post is the non-blocking Latest Features reminder. That one is fair: this is user- and admin-facing, so it likely deserves a card in the 0.261.001 tier. I left it out deliberately because a good card needs real screenshots of the new Security -> Rate Limiting tab and the rate limit banner, which I cannot capture here. Flagged in the PR checklist as card: yes, screenshot: needed.

@paullizer
Paul Lizer (paullizer) merged commit 47bf76e into Development Aug 26, 2026
11 of 12 checks passed
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