Claude/mcp codekeeper webapp ldnzsg - #3180
Merged
Merged
Conversation
מסמך תכנון לחיבור Claude.ai לקבצי המשתמש ב-CodeKeeper דרך שרת MCP מרוחק. - ממפה את הקיים: שכבת DB in-process (database/repository.py), אימות סשן (webapp/app.py:3204), וגשרי טוקנים קיימים (webapp_tokens/remember_tokens). - מציע ארכיטקטורה: שירות MCP נפרד (ASGI) שניגש ישירות ל-database, מסונן user_id. - אימות בשתי פאזות: PAT (MVP) → OAuth 2.1 מעל זהות טלגרם הקיימת. - כלים: list_files/search_code/get_file/list_versions (+ save_file בהמשך). - אבטחה: read-only כברירת מחדל, בידוד user_id מהטוקן, Smart Projection. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WNFuSyshwpRcxozVZEui5K
מימוש פאזה 0 מהתוכנית: שרת MCP מרוחק (Streamable HTTP) קריאה-בלבד שחושף את קבצי הקוד והאוספים של המשתמש ל-Claude Code/Desktop. - mcp_server/: חבילת FastMCP עם 7 כלים (list_files, search_code, get_file, list_versions, list_collections, get_collection, get_collection_items). ניגש ישירות ל-database.db + CollectionsManager, מסונן user_id, מכבד את חוק ה-Smart Projection (בלי code ברשימות/חיפוש). - אימות PAT: token_store.py מנהל טוקנים (hash בלבד) בקולקשן mcp_tokens; auth.py מאמת Bearer ומזריק user_id ל-request.state; ה-user_id נגזר תמיד מהטוקן, לעולם לא מקלט הלקוח. get_file בודק בעלות גם לפי file_id. - scripts/mcp_issue_token.py: הנפקת טוקן ל-CLI/בדיקות. - tests/test_mcp_*.py: 27 טסטים (fakes ידניים, בלי MongoDB אמיתי) — token store, בעלות/סריאליזציה, clamping, middleware, ורישום כלים. - requirements/base.txt: mcp==1.28.1 (תואם לפינים הקיימים). - מסמך התכנון עודכן לסטטוס "פאזה 0 מומשה" + README ל-mcp_server. פאזות הבאות (בתכנון): פקודת בוט /connect_claude, OAuth 2.1 ל-Claude.ai, כתיבה. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WNFuSyshwpRcxozVZEui5K
… docs תיקוני סקירה (כל ההערות היו תקפות): - backend.get_collection_items: הגנת עומק — מסנן שדות כבדים (code/content) מפריטי אוסף לפני החזרה, כדי שלא ידלפו גם אם CollectionsManager ישתנה. - mcp_server/wiring.py: חילוץ resolve_mongo המשותף; הוסרו ההעתקים מ-app.py ומ-scripts/mcp_issue_token.py. - auth.py: לוג לחריגה שנבלעה ב-verify (בלי הטוקן) לצורך אבחון בפרודקשן. - Markdown: שפת code fence (text) לדיאגרמות ולבלוק ההרצה, ושורות ריקות לפני טבלאות (MD040/MD058) ב-README ובמסמך התכנון. - טסט חדש: מוודא שפריטי אוסף מסוננים משדות כבדים. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WNFuSyshwpRcxozVZEui5K
מוסיף פקודת בוט /connect_claude שמנפיקה למשתמש טוקן MCP אישי מתוך טלגרם (במקום להריץ סקריפט CLI), ומחזירה פקודת חיבור מוכנה ל-Claude Code. - main.py: connect_claude_command (מודול-לבל), רישום ב-setup_handlers, וכניסה ב-HELP_SECTIONS. אבטחה: מונפק בצ'אט פרטי בלבד (שלא ידלוף בקבוצה); fallback לטקסט אם פרסום HTML נכשל (לא לאבד את הטוקן). - files_facade.issue_mcp_token: עוטף את MCPTokenStore (import עצל) כדי לשמור את החיווט במקום אחד ולא לגעת ב-PyMongo גולמי מה-handler. - README + מסמך התכנון עודכנו: /connect_claude זמין, + ENV MCP_SERVER_URL. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WNFuSyshwpRcxozVZEui5K
…ecrets מבהיר בדיוק אילו ENV שירות ה-MCP צריך: MONGODB_URL+DATABASE_NAME זהים לבוט/וובאפ (אותו DB, אחרת הטוקן מ-/connect_claude לא יימצא), BOT_TOKEN רק כדי שה-config המשותף ייטען, ולא צריך את סודות הוובאפ. MCP_SERVER_URL מוגדר על שירות הבוט. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WNFuSyshwpRcxozVZEui5K
לפי תבנית ה-PR: תיעוד משתני הסביבה החדשים ששירות ה-MCP/הבוט משתמשים בהם. - docs/environment-variables.rst: שתי שורות בטבלת התפעול (URL ציבורי + שם תצוגה). - services/config_inspector_service.py: ConfigDefinition לשניהם תחת category="mcp", כדי שיופיעו ב-Config Inspector. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WNFuSyshwpRcxozVZEui5K
Render defaulted the new native Python MCP service to 3.14, where greenlet (pulled in by gevent) fails to compile — its C++ uses CPython frame internals that changed in 3.14. The project targets 3.11 (Dockerfile python:3.11-slim; CI runs 3.11/3.12), so pin the native runtime to a supported version. - .python-version = 3.11 -> Render uses the latest 3.11 patch. - No effect on CI (explicit python-version in workflows) or Docker services; services that set PYTHON_VERSION explicitly still win over this file. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WNFuSyshwpRcxozVZEui5K
מתקן כפילות שנוצרה במיזוג/squash: שורת פתיחה ישנה + פסקת ENV כפולה בסעיף הפריסה. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WNFuSyshwpRcxozVZEui5K
השרת הפרוס החזיר HTTP 421 "Invalid Host header" — הגנת ה-DNS-rebinding המובנית של ה-SDK מאשרת כברירת מחדל רק localhost, וחסמה את הדומיין האמיתי (codekeeper-mcp.onrender.com). הטסטים המקומיים עברו כי הם על 127.0.0.1. - server.py: transport_security עם enable_dns_rebinding_protection=False כברירת מחדל (שרת ציבורי מוגן-טוקן — ההגנה הזו נועדה לשרתי localhost). ניתן לנעול עם MCP_ALLOWED_HOSTS (CSV, תומך wildcard) + MCP_ALLOWED_ORIGINS. - טסטים: ברירת מחדל כבויה + נעילה דרך ENV. - תיעוד: MCP_ALLOWED_HOSTS/ORIGINS ב-environment-variables.rst + config_inspector. אומת מקומית: בקשה עם Host מזויף codekeeper-mcp.onrender.com מחזירה 200 (לא 421). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WNFuSyshwpRcxozVZEui5K
מימוש פאזה 1: חיבור Claude.ai (Custom Connector) דרך OAuth 2.1 מלא, לצד ה-PAT הקיים (Claude Code/Desktop) — מאוחדים תחת load_access_token. - oauth_store.py: אחסון Mongo ל-clients/codes/tokens/txns (hash, TTL). - oauth_provider.py: מימוש חוזה ה-SDK (DCR, authorize->txn, exchange code/ refresh עם rotation, load_access_token שמקבל גם PAT וגם OAuth token). - oauth_identity.py: חתימת/אימות זהות HMAC (SECRET_KEY משותף) — שיתוף עם הוובאפ. - oauth_routes.py: מסך consent ב-MCP → מנפיק code → redirect ל-Claude. - webapp /oauth/identify: גשר זהות — התחברות טלגרם (widget) וחתימת user_id, עם שער open-redirect ל-MCP_SERVER_URL. סינון שדות טלגרם (לא לחתום על txn/return). - server.py/app.py: FastMCP עם auth_server_provider+AuthSettings כשמוגדרים MCP_SERVER_URL+WEBAPP_URL (אחרת נשאר PAT-only). current_user_id קורא get_access_token().subject (OAuth) עם fallback ל-request.state (PAT). אימות: 27 טסטי OAuth (store/provider/identity/consent/HTTP-flow/webapp) + e2e מקומי מלא (register->authorize->consent->token->tool) שמוכיח ש-PAT ו-OAuth עובדים יחד ו-get_access_token זמין בתוך tool סינכרוני. תיעוד: README + env docs. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WNFuSyshwpRcxozVZEui5K
…ations לפי מדריך mcp-builder: - שמות כלים מקודמים ב-codekeeper_ (get_file/list_files/search_code הם גנריים מדי ומתנגשים כשמחוברים כמה connectors) — codekeeper_list_files וכו'. - annotations לכל כלי: readOnlyHint=True, destructiveHint=False, idempotentHint=True, openWorldHint=False (קריאה על נתוני המשתמש). - עודכנו instructions, README, וטסט רישום הכלים. הכלים מתגלים מחדש בכל סשן, אז זה לא שובר חיבורים קיימים — Claude פשוט יראה את השמות החדשים בחיבור הבא. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WNFuSyshwpRcxozVZEui5K
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
Next review available in: 12 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (12)
📝 WalkthroughWalkthroughה-PR מוסיף לשרת MCP זרימת OAuth 2.1 לצד PAT, כולל אחסון MongoDB, גשר זהות דרך Telegram, מסך consent, שילוב ASGI/FastMCP, אבטחת transport ובדיקות יחידה ו-E2E. ChangesOAuth ו-MCP
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
פתרון קונפליקטים: - mcp_server/server.py: נשמרה גרסת הענף (superset מלא — host-fix + OAuth + prefix לכלים); גרסת main היא תת-קבוצה ישנה יותר. - docs/environment-variables.rst: אוחדו התוספות; הוסרה שורת WEBAPP_URL כפולה (המשתנה כבר תועד) והשימוש ב-MCP נוסף לשורה הקיימת. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WNFuSyshwpRcxozVZEui5K
🧯 Dangerous deletes guard reportPolicy: see .cursorrules — dangerous deletions are blocked unless wrapped safely. Summary:
Flagged findings (file:line:snippet): Excluded matches (by path pattern) |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
⏱️ Performance report(No performance test durations collected. Mark tests with |
📖 Documentation PreviewThe documentation has been built successfully!
To view locally:
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (6)
mcp_server/oauth_store.py (1)
61-69: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winמומלץ להוסיף TTL index אוטומטי לניקוי רשומות שפג תוקפן.
כרגע נבדק תוקף רק ברמת האפליקציה (
_expired), אבל אין TTL index ב-Mongo. כתוצאה מכך, txns/codes/tokens שפג תוקפם (ולא נמחקו במפורש) יישארו לנצח באוסף ויגדילו אותו ללא הגבלה עם הזמן.♻️ הצעת שיפור
def _ensure_indexes(self) -> None: try: self._clients.create_index("client_id", unique=True) self._txns.create_index("txn_id", unique=True) + self._txns.create_index("expires_at", expireAfterSeconds=0) self._codes.create_index("code_hash", unique=True) + self._codes.create_index("expires_at", expireAfterSeconds=0) self._tokens.create_index("token_hash", unique=True) self._tokens.create_index("subject") + self._tokens.create_index("expires_at", expireAfterSeconds=0) except Exception: pass🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mcp_server/oauth_store.py` around lines 61 - 69, Update _ensure_indexes to create MongoDB TTL indexes for the expiration field used by txn, code, and token records, configured to remove documents when they expire. Keep the existing uniqueness and subject indexes intact, and use the same expiry-field semantics already consumed by _expired.tests/test_mcp_oauth_routes.py (1)
18-64: 📐 Maintainability & Code Quality | 🔵 Trivialכפילות קוד: פייק Mongo זהה חוזר גם כאן (פעם שלישית).
ראו הערה מאוחדת.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_mcp_oauth_routes.py` around lines 18 - 64, הוציאו את המימושים הכפולים של _Res ו-_Coll מתיק בדיקה משותף או fixture קיים, ועדכנו את tests/test_mcp_oauth_routes.py לייבא ולהשתמש בהם במקום להגדיר אותם מחדש. שמרו על אותה התנהגות של insert_one, find_one, update_one ו-delete_one בכל הבדיקות.tests/test_mcp_oauth_store.py (1)
9-72: 📐 Maintainability & Code Quality | 🔵 Trivialכפילות קוד: אותן מחלקות פייק ל-Mongo (
_Res/_Coll/_DB) חוזרות בכמה קבצי טסט.ראו הערה מאוחדת.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_mcp_oauth_store.py` around lines 9 - 72, Remove the duplicated Mongo fake classes _Res, _Coll, and _DB from this test and reuse the shared test fixture or helper that defines them. Update references as needed so the tests retain the same in-memory collection behavior without maintaining another copy.tests/test_mcp_oauth_provider.py (1)
23-79: 📐 Maintainability & Code Quality | 🔵 Trivialכפילות קוד: פייק Mongo זהה חוזר גם כאן.
ראו הערה מאוחדת.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_mcp_oauth_provider.py` around lines 23 - 79, Remove the duplicated fake Mongo implementations _Res and _Coll from this test module and reuse the shared fake Mongo fixture/helper referenced by the consolidated review comment. Update any tests here to use that shared implementation while preserving the current collection behavior.mcp_server/oauth_provider.py (1)
151-162: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winחסרה בדיקה ש-
scopesהמבוקשים הם תת-קבוצה של ההרשאות שהוענקו במקור.
use_scopes = list(scopes or refresh_token.scopes)(שורה 159) מאמץ כלscopesשמתקבל מהלקוח ללא אימות שהוא תת-קבוצה שלrefresh_token.scopesשהונפקו במקור. לפי RFC 6749 §6, scope מבוקש ב-refresh לא צריך לחרוג מזה שהוענק. כרגע המערכת מגדירה scope יחיד ("read") כך שהחשיפה מוגבלת בפועל, אך זו נקודת הגנה קלה להוספה שתמנע הרחבת הרשאות אם ייווספו scopes בעתיד.🔒 הצעת תיקון
async def exchange_refresh_token( self, client: OAuthClientInformationFull, refresh_token: RefreshToken, scopes: list[str], ) -> OAuthToken: # Rotate: invalidate the presented refresh token, issue a fresh pair. await anyio.to_thread.run_sync(self._store.revoke_token, refresh_token.token) - use_scopes = list(scopes or refresh_token.scopes) + requested = list(scopes or refresh_token.scopes) + if not set(requested).issubset(set(refresh_token.scopes)): + raise ValueError("requested scope exceeds originally granted scope") + use_scopes = requested return await anyio.to_thread.run_sync( self._issue, client.client_id, refresh_token.subject, use_scopes )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mcp_server/oauth_provider.py` around lines 151 - 162, Update exchange_refresh_token to validate that every requested scope in scopes is contained in refresh_token.scopes before issuing a new token. Reject requests that expand the originally granted permissions, while preserving the existing fallback to refresh_token.scopes when no scopes are requested.FEATURE_SUGGESTIONS/FEATURE_MCP_CLAUDE_INTEGRATION.md (1)
3-7: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winאזהרת markdownlint: שורה ריקה בתוך blockquote (MD028)
לפי כלי הסטטיק אנליזה (markdownlint-cli2), השורה הריקה בין שני פסקאות ה-
>שוברת את רצף ה-blockquote.✏️ תיקון מוצע
> **מתי להשתמש:** לפני מימוש חיבור MCP; מסמך זה הוא מקור האמת לתכנון. > **ראו גם:** `mcp_server/README.md` (שימוש), [CodeBot – Project Docs](https://amirbiron.github.io/CodeBot/), `CLAUDE.md` (מדיניות מחייבת). +> > **מצב מימוש (פאזות 0–1):** חבילת `mcp_server/` (שרת FastMCP קריאה‑בלבד, 7 כלים ל‑🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@FEATURE_SUGGESTIONS/FEATURE_MCP_CLAUDE_INTEGRATION.md` around lines 3 - 7, Remove the blank line separating the consecutive blockquote paragraphs in FEATURE_SUGGESTIONS/FEATURE_MCP_CLAUDE_INTEGRATION.md so the quote remains continuous and satisfies markdownlint rule MD028.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@mcp_server/oauth_identity.py`:
- Around line 25-48: Update sign_identity and verify_identity to reject a
missing or empty secret before performing any HMAC operation; raise a clear
configuration error rather than substituting an empty key. Ensure both signing
and verification use only a valid configured secret and never fall back to a
known default.
In `@mcp_server/oauth_provider.py`:
- Around line 123-132: Update exchange_authorization_code to verify the result
of _store.delete_code before calling _issue; only issue tokens when the
authorization code was successfully consumed, and otherwise reject the exchange
using the store’s established failure behavior. Preserve the existing client,
subject, and scope arguments for successful exchanges.
- Around line 151-162: Update exchange_refresh_token to check the result of
_store.revoke_token before calling _issue; only issue a new token pair when
revocation succeeds, matching the atomic single-use behavior of
exchange_authorization_code, and otherwise reject the reused refresh token
through the existing failure path.
In `@mcp_server/oauth_routes.py`:
- Around line 96-123: Update consent_post so authorization codes are issued only
when form["action"] is explicitly the expected approval value; treat missing,
empty, malformed, or any other action as denial or an appropriate error
response. Preserve the existing denial cleanup and access_denied redirect
behavior for non-approval decisions.
In `@mcp_server/oauth_store.py`:
- Around line 111-124: Update OAuthStore.delete_code to return whether
delete_one actually removed a record, using the deletion result’s deleted count,
while preserving the existing exception handling. Update
exchange_authorization_code to require a successful deletion before issuing
tokens, rejecting already-consumed or missing authorization codes.
- Around line 144-148: Update OAuthStore.revoke_token to return whether a token
was actually revoked, using the update result rather than assuming success;
preserve the existing exception-safe behavior by returning False on failure.
Update callers such as oauth_provider.exchange_refresh_token to check this
result and handle failed or already-consumed revocations consistently with
delete_code.
In `@mcp_server/server.py`:
- Around line 43-60: Update _transport_security so
enable_dns_rebinding_protection and TransportSecuritySettings are enabled only
when MCP_ALLOWED_HOSTS produces a non-empty hosts list, not when only origins
are configured. Preserve parsing of MCP_ALLOWED_ORIGINS and pass origins
alongside hosts when host protection is enabled; otherwise return the disabled
configuration.
---
Nitpick comments:
In `@FEATURE_SUGGESTIONS/FEATURE_MCP_CLAUDE_INTEGRATION.md`:
- Around line 3-7: Remove the blank line separating the consecutive blockquote
paragraphs in FEATURE_SUGGESTIONS/FEATURE_MCP_CLAUDE_INTEGRATION.md so the quote
remains continuous and satisfies markdownlint rule MD028.
In `@mcp_server/oauth_provider.py`:
- Around line 151-162: Update exchange_refresh_token to validate that every
requested scope in scopes is contained in refresh_token.scopes before issuing a
new token. Reject requests that expand the originally granted permissions, while
preserving the existing fallback to refresh_token.scopes when no scopes are
requested.
In `@mcp_server/oauth_store.py`:
- Around line 61-69: Update _ensure_indexes to create MongoDB TTL indexes for
the expiration field used by txn, code, and token records, configured to remove
documents when they expire. Keep the existing uniqueness and subject indexes
intact, and use the same expiry-field semantics already consumed by _expired.
In `@tests/test_mcp_oauth_provider.py`:
- Around line 23-79: Remove the duplicated fake Mongo implementations _Res and
_Coll from this test module and reuse the shared fake Mongo fixture/helper
referenced by the consolidated review comment. Update any tests here to use that
shared implementation while preserving the current collection behavior.
In `@tests/test_mcp_oauth_routes.py`:
- Around line 18-64: הוציאו את המימושים הכפולים של _Res ו-_Coll מתיק בדיקה משותף
או fixture קיים, ועדכנו את tests/test_mcp_oauth_routes.py לייבא ולהשתמש בהם
במקום להגדיר אותם מחדש. שמרו על אותה התנהגות של insert_one, find_one, update_one
ו-delete_one בכל הבדיקות.
In `@tests/test_mcp_oauth_store.py`:
- Around line 9-72: Remove the duplicated Mongo fake classes _Res, _Coll, and
_DB from this test and reuse the shared test fixture or helper that defines
them. Update references as needed so the tests retain the same in-memory
collection behavior without maintaining another copy.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6ad14f09-14c7-4c8d-8d3c-bcba4d92a070
📒 Files selected for processing (19)
FEATURE_SUGGESTIONS/FEATURE_MCP_CLAUDE_INTEGRATION.mddocs/environment-variables.rstmcp_server/README.mdmcp_server/app.pymcp_server/auth.pymcp_server/oauth_identity.pymcp_server/oauth_provider.pymcp_server/oauth_routes.pymcp_server/oauth_store.pymcp_server/server.pyservices/config_inspector_service.pytests/test_mcp_oauth_e2e.pytests/test_mcp_oauth_identity.pytests/test_mcp_oauth_provider.pytests/test_mcp_oauth_routes.pytests/test_mcp_oauth_store.pytests/test_mcp_server_build.pytests/test_webapp_oauth_identify.pywebapp/routes/auth_routes.py
חיזוקי אבטחה ותקינות בזרימת ה-OAuth (מ-code review + CodeQL), שינויים מינימליים:
- קודי הרשאה ו-refresh tokens חד-פעמיים באמת: delete_code/revoke_token
מחזירים bool, וה-provider מנפיק טוקנים רק אם ההסרה/הביטול הצליחו (מונע
replay/כפילות); אחרת TokenError("invalid_grant").
- refresh לא מרחיב הרשאות: בדיקת subset מול ה-scope של הטוקן (invalid_scope).
- מסך consent נכשל-סגור: רק action=approve מנפיק code; כל ערך אחר = דחייה.
- זהות חתומה: דחיית SECRET_KEY ריק (sign זורק, verify מחזיר False).
- הגנת open-redirect ב-/oauth/identify לפי scheme+host מנותחים (במקום startswith).
- מניעת XSS ב-widget של ההתחברות: html.escape על ערכים שמוטמעים ב-HTML.
- DNS-rebinding: מפעילים host-check רק כשמוגדר MCP_ALLOWED_HOSTS (מונע 421).
- TTL indexes לניקוי אוטומטי של codes/txns/tokens שפגו.
- טסטים: איחוד fake Mongo משותף (tests/_fake_mongo.py) + כיסוי לתרחישי replay/scope/fail-closed/secret ריק.
עיינתי בתיעוד: https://amirbiron.github.io/CodeBot/
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WNFuSyshwpRcxozVZEui5K
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/test_mcp_oauth_provider.py (1)
147-147: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueשימוש בקבוע במקום ערך קשיח (Hardcoded value) עבור קידומת הקוד
בשני הקבצים נעשה שימוש בערך הקשיח
"ckoc_"במקום להשתמש בקבוע המערכת (כגוןCODE_PREFIX). מומלץ לייבא ולהשתמש בקבוע כדי למנוע שבירה של הטסטים במידה והקידומת תשתנה בעתיד, בדומה לאופן שבו נעשה שימוש ב-ACCESS_PREFIXבקבצים אחרים במערכת.
tests/test_mcp_oauth_provider.py#L147-L147: להחליף את המחרוזת"ckoc_"בקבוע המתאים בקריאה ל-new_secret.tests/test_mcp_oauth_routes.py#L109-L109: להשתמש בקבוע במקום לכתוב"code=ckoc_"בבדיקת אי-הימצאות המחרוזת.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_mcp_oauth_provider.py` at line 147, Replace the hardcoded "ckoc_" prefix with the appropriate CODE_PREFIX constant in the new_secret call in tests/test_mcp_oauth_provider.py:147-147, and use the same constant instead of "code=ckoc_" in tests/test_mcp_oauth_routes.py:109-109; import the constant as needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/test_mcp_oauth_provider.py`:
- Line 147: Replace the hardcoded "ckoc_" prefix with the appropriate
CODE_PREFIX constant in the new_secret call in
tests/test_mcp_oauth_provider.py:147-147, and use the same constant instead of
"code=ckoc_" in tests/test_mcp_oauth_routes.py:109-109; import the constant as
needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b4b7b78a-a471-4e07-a0f1-9933a87f11a2
📒 Files selected for processing (15)
FEATURE_SUGGESTIONS/FEATURE_MCP_CLAUDE_INTEGRATION.mddocs/environment-variables.rstmcp_server/oauth_identity.pymcp_server/oauth_provider.pymcp_server/oauth_routes.pymcp_server/oauth_store.pymcp_server/server.pytests/_fake_mongo.pytests/test_mcp_oauth_e2e.pytests/test_mcp_oauth_identity.pytests/test_mcp_oauth_provider.pytests/test_mcp_oauth_routes.pytests/test_mcp_oauth_store.pytests/test_webapp_oauth_identify.pywebapp/routes/auth_routes.py
🚧 Files skipped from review as they are similar to previous changes (8)
- tests/test_mcp_oauth_identity.py
- FEATURE_SUGGESTIONS/FEATURE_MCP_CLAUDE_INTEGRATION.md
- mcp_server/oauth_identity.py
- webapp/routes/auth_routes.py
- mcp_server/oauth_routes.py
- mcp_server/server.py
- mcp_server/oauth_provider.py
- mcp_server/oauth_store.py
תיקון שתי בעיות שעלו ב-CI על ה-PR: 1) שגיאות איסוף טסטים (4 ERRORS): הייבוא `from tests._fake_mongo import FakeDB` נכשל ב-CI כי חבילת `tests` המקומית מוצללת ע"י חבילת `tests` אחרת ב-sys.path (בעיה מתועדת ב-tests/conftest.py). הפתרון: ייבוא הגנתי עם נפילה חזרה למודול-האח (`from _fake_mongo import FakeDB`), שעובד תמיד תחת מצב prepend של pytest. אומת מול סימולציית חבילה מצללת. 2) חיזוק SECRET_KEY (אבטחה): מצב OAuth השתמש בברירת מחדל ציבורית וידועה לחתימת זהות — מה שמאפשר זיוף user_id. נוסף `assert_strong_secret` שמסרב לעלות/לחתום עם מפתח ריק או ברירת המחדל — גם בשירות ה-MCP (app.py) וגם בוובאפ (`/oauth/identify` מחזיר 500 נקי במקום לחתום במפתח נחש). עדכוני תיעוד/קונפיג (לפי תבנית ה-PR): docs/environment-variables.rst ו- services/config_inspector_service.py — הובהר ש-SECRET_KEY חייב ערך אקראי חזק וזהה בין הוובאפ לשירות ה-MCP במצב OAuth. טסטים: הוספת assert_strong_secret + מסלול 500 למפתח שגוי + ייבוא הגנתי. 68 עוברים. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WNFuSyshwpRcxozVZEui5K
…alls הטסט `test_request_uses_merged_headers_and_respects_slow_logging` היה שביר: הוא מדליף (patch) את `time.perf_counter` הגלובלי עם רשימה בת 2 ערכים ו-`pop(0)`, ולכן כל קריאה שלישית ל-`perf_counter` (נתיב שגיאה, מטריקות או לוגים) מרוקנת את הרשימה וזורקת `IndexError: pop from empty list`. הכשל תלוי-סדר: שינוי בסדר איסוף הטסטים (למשל הוספת קבצי טסט אחרים) חשף אותו. התיקון: שעון דמה שלא מתרוקן — קריאה ראשונה = t0, וכל קריאה נוספת מחזירה t0+200ms (itertools.chain([...], repeat(...))). דטרמיניסטי, ולא קשור לפיצ'ר ה-OAuth — פשוט נחשף על ידי שינוי סדר הריצה. אומת: עובר 5/5 עם/בלי HTTP_SLOW_MS. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WNFuSyshwpRcxozVZEui5K
This was referenced Jul 19, 2026
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
3 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.
תבנית Pull Request
✨ תיאור קצר
📦 שינויים עיקריים
פירוט נקודות (רשימת תבליטים):
🧪 בדיקות
🧪 בדיקות נדרשות ב‑PR
📝 סוג שינוי
דוגמאות Conventional Commits
✅ צ'קליסט
services/register_jobs.py(כולל Callback/Trigger להפעלה ידנית — למשלcallback_name/trigger_funcלפי המבנה) כדי שיופיעו בדשבורדdocs/environment-variables.rstוגםservices/config_inspector_service.pydocs/webapp/theming_and_css.rst+FEATURE_SUGGESTIONS/theme_matrix.md🧩 השפעות/סיכונים
🔗 קישורים
🧯 סיכון / החזרה לאחור (Rollback)
Summary by CodeRabbit
תכונות חדשות
/connect_claudeותמיכה בחיבור Claude Code ו‑Claude Desktop.תיעוד
בדיקות