Repository navigation
Conversation
…בתו (#3431) עד היום המידלוור היחיד בשרת ה-MCP היה האימות: כל משתמש מאומת יכול היה לשלוח בקשות בכל גודל ובכל תדירות. שני גבולות חדשים, כל אחד בשכבה שלו, במודול חדש mcp_server/limits.py: - גודל הגוף: מידלוור ASGI טהור (BodySizeLimitMiddleware) שמסרב ב-413 {"error": "body_too_large", "max_bytes": ...} לפני שהטרנספורט של ה-SDK קורא ומפענח JSON. Content-Length שמצהיר על יותר מהתקרה נדחה לפני שנקרא בית אחד; כותרת חסרה או מכזבת נתפסת בספירה של מה שבאמת מגיע. חיץ ולא חריגה מתוך receive, כי _handle_post_request של ה-SDK עוטף את קריאת הגוף ב-try שתופס Exception ומחזיר שגיאת JSON-RPC בלי סיבה. ברירת המחדל 1MiB, נגזרת מ-MAX_CODE_SIZE (100,000 תווים, עד ~600KB כ-JSON עם ensure_ascii). - קצב לפי זהות: ב-AdminAwareFastMCP.call_tool, המתודה שה-SDK רושם כמטפל של tools/call, כלומר נקודה אחת שכל קריאת כלי עוברת בה בשני מצבי האימות (במצב OAuth PATAuthMiddleware אינו מותקן, ורק הקונטקסט של הקריאה רואה את הזהות). ההכרעה נופלת לפני שגוף סינכרוני נמסר לחוט ולפני שגוף אסינכרוני רץ; קריאה שנדחתה מחזירה תשובת כלי רגילה {"ok": false, "error": "rate_limited", "limit_per_minute": ..., "retry_after_seconds": ...}. מחוץ לבקשה (LookupError מ-request_context) אין את מי לחייב, ולכן הטסטים שקוראים לכלים ישירות ממשיכים כמו היום. 60 בדקה, מהמדידות בתגובה באישו: 0.24 שניות מעבד לעמוד RST עוין של 500KB, 0.47 למסמך Markdown הצפוף, 2.3 לצורה העוינת, מול מכסה של 0.5 מעבד (30 שניות-מעבד בדקה). - הפטור לנתיבי הדופק מבני ולא רשימה (blanket-policy-silent-block §7): /healthz אינו קריאת כלי ואין לו גוף, ומקובע בטסט על האפליקציה האמיתית. - rate_limiter.RateLimiter הקיים של הבוט משמש כמנוע (R6): ניקוי החלון אוחד ל-_live_entries, ונוספה seconds_until_allowed בשביל retry_after_seconds. - כיוון דרך MCP_MAX_REQUEST_BYTES (מינימום 65536) ו-MCP_RATE_LIMIT_PER_MINUTE (0 מכבה במפורש עם WARNING), נקראים ב-create_app ולא בזמן ייבוא; ערך פגום לעולם אינו מרחיב את הגבול (K12 §3). נרשמו ב-config_inspector_service ובתיעוד. אומת עם uvicorn אמיתי: 401 לפני 413 במצב PAT, 413 על Content-Length ועל גוף chunked בלי כותרת, 120 דגימות של /healthz תחת מגבלה של קריאה אחת בדקה כולן 200; ועם לקוח ה-MCP של ה-SDK על Streamable HTTP: הקריאה השנייה מחזירה rate_limited ו-tools/list אחריה עדיין עונה. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SfJTSpDAhDr2yhtmpFkwTx
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Sorry @amirbiron, you've used your own review budget of 250,000 diff characters for the last 7 days.
You can request another review in 6 hours and 29 minutes by commenting @sourcery-ai review. Upgrade to get a review now.
Contributor
|
Warning Review limit reachedNext included review available in 27 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (9)
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 |
Contributor
🧯 Dangerous deletes guard reportPolicy: see .cursorrules — dangerous deletions are blocked unless wrapped safely. Summary:
Flagged findings (file:line:snippet): Excluded matches (by path pattern) |
Contributor
Reviewer's Guideה-PR מוסיף לשרת MCP שתי שכבות הגנה: תקרת גוף ASGI של 1MiB שנאכפת לפני פענוח ה-SDK, והגבלת Sequence diagram for MCP body-size rejectionsequenceDiagram
participant Client as MCP client
participant Limit as BodySizeLimitMiddleware
participant Auth as Authentication
participant SDK as MCP transport
Client->>Limit: HTTP request
alt Content-Length exceeds max_bytes
Limit-->>Client: 413 body_too_large
else Body exceeds max_bytes while receiving
Limit->>Limit: buffer http.request messages
Limit-->>Client: 413 body_too_large
else Body within limit
Limit->>Auth: replay buffered request
Auth->>SDK: authenticated request
SDK-->>Client: MCP response
end
Sequence diagram for identity-based tool-call rate limitingsequenceDiagram
participant Client as MCP client
participant Transport as MCP transport
participant Server as AdminAwareFastMCP
participant Limiter as ToolRateLimiter
participant Tool as Tool body
Client->>Transport: tools/call
Transport->>Server: call_tool(name, arguments)
Server->>Server: current_user_id(get_context())
Server->>Limiter: admit(user_id)
alt Limit available
Limiter-->>Server: None
Server->>Tool: execute tool
Tool-->>Client: converted tool result
else Limit exceeded
Limiter-->>Server: rate_limited and retry_after_seconds
Server-->>Client: converted refusal result
end
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
⏱️ Performance report(No performance test durations collected. Mark tests with |
Contributor
📖 Documentation PreviewThe documentation has been built successfully!
To view locally:
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
This was referenced Sep 20, 2026
Closed
Owner
Author
|
נכנס דרך #3443 |
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.
✨ תיאור קצר
mcp_server/limits.py: גוף בקשה מעל 1MiB נדחה ב-413 body_too_largeבמידלוור ASGI, לפני שהטרנספורט של ה-SDK קורא ומפענח JSON; וקריאות כלים מוגבלות ל-60 בדקה לזהות בנקודתtools/call, לפני שנתפס חוט, עםrate_limitedו-retry_after_secondsבתשובה./healthzפטור מבנית משניהם (blanket-policy-silent-block§7), וזה מקובע בטסט על האפליקציה האמיתית.claude/gracious-einstein-sevk8pנושא כבר את ה-PR הפתוח של docs_get_section: תקרה ל-candidates, ושמירת חוזה Suggestions — לפני שחיבור ה-Markdown נוחת #3426 (fix(mcp): תקרה לרשימת המועמדים של ambiguous_section, וטסט שמקבע את חוזה Suggestions במסלול ה-Markdown (#3426) #3434), ו-PR נפרד דורש ענף נפרד. לכן ה-PR הזה יושב על ענף נגזר,claude/gracious-einstein-sevk8p-3431, שנפתח מ-origin/mainהעדכני (3b8628f4, אחרי מיזוג feat(mcp): docs_get_section קורא גם Markdown, לפי מדיניות נתיבים לכל ריפו #3428 ו-fix(mcp): מאגר הקריאות נגזר ממכסת הזיכרון של הקונטיינר, לא ממעבדי המארח (#3391) #3429).📦 שינויים עיקריים
פירוט נקודות (רשימת תבליטים):
BodySizeLimitMiddleware, מידלוור ASGI טהור.Content-Lengthשמצהיר על יותר מהתקרה נדחה לפני שנקרא בית אחד; כותרת חסרה או מכזבת (למשל chunked) נתפסת בספירה של הודעותhttp.requestשבאמת מגיעות. חיץ ולא חריגה מתוךreceive, כי_handle_post_requestשל ה-SDK (mcp 1.28.1) עוטף את קריאת הגוף ב-tryשתופסExceptionומחזיר שגיאת JSON-RPC בלי סיבה. הסירוב:{"error": "body_too_large", "max_bytes": ..., "content_length" | "received_bytes": ...}, באותה צורה של ה-401 של האימות. נוסף לפניPATAuthMiddlewareבכוונה (add_middlewareמכניס בראש המחסנית, Starlette 1.6.0), ולכן במצב PAT בקשה בלי טוקן היא 401 גם כשהיא ענקית; במצב OAuth התקרה היא השכבה החיצונית ברמת האפליקציה.AdminAwareFastMCP.call_tool. זו המתודה שה-SDK רושם כמטפל שלtools/call(FastMCP._setup_handlers,self._mcp_server.call_tool(validate_input=False)(self.call_tool)), כלומר נקודה אחת שכל קריאת כלי עוברת בה — גוף סינכרוני לפני שהוא נמסר לחוט, גוף אסינכרוני לפני שהוא רץ. למה לא במידלוור: הזהות קיימת ב-request.stateרק במצב PAT; במצב OAuth (הייצור)PATAuthMiddlewareאינו מותקן, ורקcurrent_user_idעל הקונטקסט של הקריאה רואה אותה בשני המצבים. מחוץ לבקשהrequest_contextמריםLookupError— אין את מי לחייב, והטסטים שקוראים לכלים ישירות ממשיכים כמו היום. הסירוב הוא תשובת כלי רגילה דרךconvert_resultשל הכלי:{"ok": false, "error": "rate_limited", "limit_per_minute": 60, "retry_after_seconds": N}.add_tool; זה הפיל שישה טסטים קיימים שקוראיםfn.__code__.co_nameעל הפונקציות הרשומות ושניים שקוראים לכלים מחוץ לבקשה. ההעברה ל-call_toolהיא הפתרון השורשי: הפונקציות הרשומות נשארות בדיוק מה ש-add_toolבנה, וההכרעה יושבת במקום היחיד שהפרוטוקול עצמו עובר בו.contentשלcodekeeper_save_file, שחסום ב-MAX_CODE_SIZE= 100,000 תווים; לקוח שמקודד JSON עםensure_asciiהופך כל תו עברי לשישה בתים, כלומר כ-600KB במקרה הקיצוני — 1MiB משאיר לזה מרווח, ורצפה של 65,536 כדי שטעות תצורה לא תהפוך להפסקת שירות. קצב: הבקשה הציבורית היקרה ביותר היום היא עמוד RST של 500KB — 0.24 שניות מעבד עם תקרת הסקשנים (0.035 לעמוד הצפוף האמיתי); במסלול ה-Markdown המסמך הצפוף האמיתי 0.47 והצורה העוינת 2.3; המכסה 0.5 מעבד = 30 שניות-מעבד בדקה. שישים קריאות של המסמך הצפוף האמיתי הן 28 שניות: זהות אחת יכולה לכל היותר למלא דקת מעבד אחת משלה; קריאה רגילה (10–50ms) הופכת 60 בדקה לאחוז עד שלושה מהמכסה.rate_limiter.RateLimiterהקיים של הבוט (R6 — לא עותק שני של חלון מתגלגל). שינוי בו: ניקוי החלון אוחד ל-_live_entries(היה משוכפל בשתי מתודות), ונוספהseconds_until_allowedבשבילretry_after_seconds. ה-API הקיים לא השתנה; הסוויטות של הבוט ושלmain.pyשמשתמשות בו ירוקות.MCP_MAX_REQUEST_BYTESו-MCP_RATE_LIMIT_PER_MINUTE, נקראים ב-create_appדרךlimit_from_env(בכניסה לשירות, לא בזמן ייבוא —import-time-side-effects). ערך שאינו מספר → ברירת המחדל עם WARNING; מתחת למינימום → המינימום עם WARNING;0לקצב → כיבוי מפורש עם WARNING (K12 §3: בלי ברירת מחדל שקטה שמרחיבה). בשום מסלול ערך פגום אינו הופך לגבול רחב יותר. נרשמו ב-services/config_inspector_service.py, ב-docs/environment-variables.rst, בטבלת הגבולות ובסעיף חדש "גבולות הבקשה" ב-docs/mcp-server.rst, וב-docs/whats-new.rst.mcp request limits: body <= 1048576 bytes (413 body_too_large), tool calls <= 60 per identity per minute (rate_limited); /healthz sits outside both._build_docs_path_docב-server.py(E302 שנשאר מ-feat(mcp): docs_get_section קורא גם Markdown, לפי מדיניות נתיבים לכל ריפו #3428; לא ב-selection של CI).🧪 בדיקות
tests/test_mcp_limits.py(חדש, 18 טסטים): המידלוור מול הודעות ASGI ממש (Content-Length מעל התקרה נדחה בלי לקרוא בית; גוף chunked מעל התקרה נדחה והאפליקציה לא רצה; בדיוק התקרה עובר ובית אחד מעל לא;http.disconnectעובר הלאה כמות שהוא; scope שאינו http עובר ישר); האפליקציה האמיתית דרךTestClient(401 לפני 413, ו-413 נוקב בסיבתו; 100 דגימות/healthzתחת מגבלה של קריאה אחת בדקה — כולן 200); המגביל דרךcall_toolשל ה-FastMCP האמיתי בתוךrequest_ctxמדומה (סירוב עם הסיבה וההמתנה; תקציב נפרד לכל זהות; מחוץ לבקשה לא נספר כלום; קריאה שנדחתה לא מריצה את הגוף; כלי אסינכרוני מוגבל גם הוא;0מכבה; בלי זהות הגוף הוא שמסרב); לוג אחד לזהות לחלון;limit_from_envלעולם אינו מרחיב;seconds_until_allowed.origin/main=3b8628f4): קובץ הטסטים נופל בקולקציה (ImportErrorעלmcp_server.limits) — אף טסט חדש אינו עובר בלי השינוי.test_mcp_to_thread,test_mcp_logging_visible,test_mcp_server_build,test_mcp_analytics_privacy,test_mcp_docs_handlers,test_mcp_auth_middleware,test_mcp_primer,test_mcp_outline,test_mcp_repo_backend,test_rate_limiter_basic,test_main_rate_limit_gate,test_bot_handlers_rate_limit_command,test_config_definitions_coverage— ירוקים; ועוד 175 ב-test_bot_rate_limiter,test_handlers_cleanup,test_main_shadow_limits,test_query_profiler_service,test_config_inspector_empty_sensitive,test_config_inspector_service— ירוקים. הששה שנפלו תחת הגרסה הראשונה (העטיפה ב-add_tool) עוברים.Content-Length→ 413{"error":"body_too_large","max_bytes":1000,"content_length":2002}; אותו גוף chunked בלי כותרת → 413 עםreceived_bytes: 2002;pingקטן → 200; 120 דגימות/healthz→ כולן 200; בלוג uvicorn: שורתmcp request limitsושתי שורותmcp request refused.streamablehttp_client+ClientSessionמול אותו uvicorn):initializeו-tools/list(22 כלים) לא נספרים;tools/callראשון →{"found": false}; השני והשלישי →{"ok": false, "error": "rate_limited", "limit_per_minute": 1, "retry_after_seconds": 60}עםisError: false;tools/listאחרי הסירוב עדיין עונה; בלוג:mcp tool rate limit: identity 7פעם אחת.E9,F63,F7,F82) — 0; flake8 מלא (max-line-length=127) על הקבצים ששיניתי — נקי (ב-config_inspector_service.pyיש E501 קיימים שאינם שלי). docutils על שלושת עמודי התיעוד — 0 אזהרות (בלי בנייה מלאה, לפיCLAUDE.md).current_user_idשכבר עובד בו היום, ועלget_access_token()של ה-SDK), ואת שורת העלייה בשירות בייצור — היא תיראה אחרי הדיפלוי.🧪 בדיקות נדרשות ב‑PR
📝 סוג שינוי
✅ צ'קליסט
docs/environment-variables.rstוגםservices/config_inspector_service.pydocs/whats-new.rstעודכןAI-MAP.md,docs/mcp-server.rst(טבלת הגבולות, "מודל הריצה של הכלים",mcp-analytics),docs/environment-variables.rst(בלוק ה-MCP),docs/doc-authoring.rst,docs/versioning-stable-anchors.rst| המשפט: "השרת של ה-MCP רץ במופע אחד" (numInstances: 1) — הסיבה שמגביל בזיכרון מספיק בלי Redisדפוסי באגים שנקראו ומה שקבעו בקוד:
bugbot-rules/blanket-policy-silent-block.md§7–8 — הפטור ל-/healthzמבני (המגביל יושב מתחת לנתיבי ה-HTTP), ולא רשימת נתיבים;bugbot-rules/silent-truncation-at-sink.md— סירוב נוקב בסיבתו במקום חיתוך שקט;CORE-PATTERNS.mdU3 +bugbot-rules/external-input-isinstance.md—Content-Lengthוערכי ENV נבדקים כקלט חיצוני (isdigit,intתחתtryצר);CRITICAL-PATTERNS.mdK12 §3 —0מכבה רק במפורש ועם WARNING, ערך פגום לעולם אינו מרחיב; K13 — הלוג נושא גדלים ונתיב, לא גוף ולא טוקן; K11 —admitמחזיר ערך שנבדק,check_rate_limitמחזירFalseולא זורק;bugbot-rules/import-time-side-effects.md— ENV נקרא ב-create_app, המגביל נבנה ב-build_mcp;bugbot-rules/silent-fallback-to-worse-path.md— ה-LookupErrorמחוץ לבקשה מתועד כתנאי מבני ולא כ-fallback שקט;RECURRING-PATTERNS.mdR6 —RateLimiterהקיים,_live_entriesאחד;claude-md-snippets/testing.md— הטסטים הורצו על הקוד הישן ונפלו.🧩 השפעות/סיכונים
rate_limitedכתשובת כלי רגילה (לא שגיאת פרוטוקול) עם הזמן שנותר. סוכן רגיל רחוק מזה בסדר גודל.initialize/tools/list/זרם ה-SSE, ו-/healthz.numInstances: 1); הרחבה למופעים מרובים תדרוש מנוע משותף — מתועד בסעיף החדש.🔗 קישורים
docs/mcp-server.rst— הסעיף החדש "גבולות הבקשה — גודל הגוף והקצב"🧯 סיכון / החזרה לאחור (Rollback)
MCP_RATE_LIMIT_PER_MINUTE=0מכבה את המגביל (עם WARNING בעלייה), ו-MCP_MAX_REQUEST_BYTESמרים את התקרה.🤖 Generated with Claude Code
https://claude.ai/code/session_01SfJTSpDAhDr2yhtmpFkwTx
Generated by Claude Code
Summary by Sourcery
Add configurable MCP request-size and per-identity tool-call limits with explicit rejection reasons while keeping health checks and non-tool protocol operations unaffected.
Bug Fixes:
Enhancements:
Documentation:
Tests: