diff --git a/FEATURE_SUGGESTIONS/FEATURE_MCP_CLAUDE_INTEGRATION.md b/FEATURE_SUGGESTIONS/FEATURE_MCP_CLAUDE_INTEGRATION.md index 01b03f180..5513f0e4c 100644 --- a/FEATURE_SUGGESTIONS/FEATURE_MCP_CLAUDE_INTEGRATION.md +++ b/FEATURE_SUGGESTIONS/FEATURE_MCP_CLAUDE_INTEGRATION.md @@ -1,6 +1,6 @@ # חיבור Claude.ai ל‑CodeKeeper דרך MCP — מסמך תכנון -> **סטטוס:** פאזות 0 (PAT) **+** 1 (OAuth 2.1 ל‑Claude.ai) **+** 3 (כתיבה save-only) **מומשו** — ראו `mcp_server/`. מחיקה וכתיבת אוספים עדיין בתכנון. +> **סטטוס:** פאזות 0 (PAT) **+** 1 (OAuth 2.1 ל‑Claude.ai) **+** 3 (כתיבה save-only) **+** ד' (דפדפן ריפו לאדמין, קריאה בלבד) **מומשו** — ראו `mcp_server/`. מחיקה וכתיבת אוספים עדיין בתכנון. > **ענף פיתוח:** `claude/mcp-codekeeper-webapp-ldnzsg` > **מתי להשתמש:** לפני מימוש חיבור MCP; מסמך זה הוא מקור האמת לתכנון. > **ראו גם:** `mcp_server/README.md` (שימוש), [CodeBot – Project Docs](https://amirbiron.github.io/CodeBot/), `CLAUDE.md` (מדיניות מחייבת). @@ -210,7 +210,7 @@ db.delete_file(user_id, file_name) # מחיקה רכה (recycle | ג' | כתיבה: `save_file`/`delete_file` מאחורי scope, rate limiting, מסך "חיבורים פעילים" + revoke | ~1–2 ימים | בדיקות כתיבה על tmp בלבד, אישור scope, revoke | | ד' | דפדפן הריפו (אדמין בלבד): `list_repos`/`list_repo_tree`/`get_repo_file`/`search_repo` מעל Repo Sync Engine + `require_admin` + סינון tools/list + מדיניות סודות (13.5) — ראו סעיף 13 | ~2–3 ימים | יחידה עם mirror מזויף/tmp, בדיקת fail‑closed לאדמין, מדיניות סודות (כל כלל + מקרי קצה + מדיניות‑חסרה), ברירות מחדל/תקרות/clamp, תקציב פלט, timeout, snippet בלבד בחיפוש | -**מיפוי ומצב (קנוני):** שלב א' = פאזה 0 (✅ מומש) · שלב ב' = פאזה 1 (✅ מומש) · שלב ג' = פאזה 3 (✅ מומש חלקית — save‑only; מחיקה טרם) · שלב ד' = פאזה ד' (⏳ תכנון). +**מיפוי ומצב (קנוני):** שלב א' = פאזה 0 (✅ מומש) · שלב ב' = פאזה 1 (✅ מומש) · שלב ג' = פאזה 3 (✅ מומש חלקית — save‑only; מחיקה טרם) · שלב ד' = פאזה ד' (✅ מומש — קריאה בלבד, אדמין). **בדיקות — לפי `CLAUDE.md`:** לעבוד רק על תיקיות זמניות, בלי מחיקות ב‑root, בידוד לכל טסט. לפני תיקוני טסטים — לעיין ב‑[CodeBot Docs](https://amirbiron.github.io/CodeBot/). @@ -235,7 +235,9 @@ db.delete_file(user_id, file_name) # מחיקה רכה (recycle ## 13. פאזה ד': מקור נתונים שני — דפדפן הריפו (אדמין בלבד) -> **סטטוס:** תכנון בלבד — טרם מומש. אינו משנה את היקף פאזה א'. +> **סטטוס:** ✅ **מומש** (קריאה בלבד) — `mcp_server/repo_backend.py`, `repo_handlers.py`, +> `repo_policy.py`, `require_admin` + `AdminAwareFastMCP` ב‑`server.py`/`auth.py`. +> אינו משנה את היקף פאזה א'. ### 13.1 למה זה שווה הריפואים המשוקפים (Repo Sync Engine) מכילים את התיעוד, הקוד ומסמכי התכנון של כל הפרויקטים. @@ -304,6 +306,7 @@ db.delete_file(user_id, file_name) # מחיקה רכה (recycle - בינארי: `{ok: true, status: "binary", file: {…}}` — **בלי** `content`. - גדול‑מדי: `{ok: true, status: "too_large", file: {…, size}, max: 512000}` — **בלי** `content`. - לא נמצא / ref לא תקין: `{ok: false, error: "not_found"}`; נתיב חסום במדיניות: `{ok: false, error: "path_denied"}`. +- כשל בזמן sync פעיל: `{ok: false, error: "sync_in_progress", retry_after: 30}` — הקורא מוזמן לנסות שוב, לא להסיק היעדר. כך לקוח MCP מבחין בין כל התוצאות בלי לנחש מהיעדר שדות. ### 13.5 מדיניות סינון סודות — חובה, fail‑closed @@ -332,7 +335,9 @@ db.delete_file(user_id, file_name) # מחיקה רכה (recycle 3. **אין רשימת branches/refs** — `for-each-ref` רץ רק inline ב‑initial_import (`services/repo_sync_service.py:539-619`) ואינו חשוף. 4. **אין שכבת אדמין ב‑MCP** — `require_admin` וסינון tools/list הם עבודה חדשה (ראו 13.2). 5. **אין denylist/redaction על נתיבי הקריאה** — ראו המדיניות המחייבת ב‑13.5 והסיכון ב‑13.7. -6. **ל‑`repo_metadata` אין אינדקס** (`scripts/create_repo_indexes.py` מכסה רק `sync_jobs`/`repo_files`). +6. **ל‑`repo_metadata` אין אינדקס** — ✅ נסגר כחלק מהמימוש (`list_repos` רץ עליו בכל קריאה): + אינדקס unique על `repo_name` נוצר גם ב‑`scripts/create_repo_indexes.py` וגם best‑effort + באתחול ה‑backend (`mcp_server/repo_backend.py`). כמו בסעיף 7 — הגישה ה‑in‑process (ישירות ל‑`GitMirrorService`) מייתרת תיקון מוקדם של ה‑HTTP: הפערים נסגרים בשכבת ה‑MCP עצמה. @@ -346,7 +351,9 @@ db.delete_file(user_id, file_name) # מחיקה רכה (recycle - **עומס דיסק/זיכרון בריפואים גדולים** — ממתן קיים: חיפוש בסטרימינג (`:1912`, תוכנן ל‑512MB ב‑Render) + התקרות של 13.4. - **מרוץ מול sync**: אין נעילת קריאה (fetch/gc יכולים לרוץ במקביל, `repo_sync_service.py:202`) — - הכלים צריכים לספוג כשל חולף ולהחזיר שגיאה נקייה, לא להפיל את השירות. + הכלים סופגים כשל חולף ומחזירים שגיאה נקייה **עם אינדיקציה ש‑sync רץ**: + `{"error": "sync_in_progress", "retry_after": 30}` (נבדק מול `sync_jobs.status="running"`), + כדי שהמודל הקורא יידע לחזור אחרי המתנה קצרה ולא יסיק בטעות שהקובץ/הריפו לא קיימים. - **הרחבת ה‑surface מעבר לנבדק**: כלים קריאים גם כשמוסתרים (13.2) ⇒ שער אדמין בגוף כל כלי — חובה, לא אופציה; והפיצ'ר כולו נשאר קריאה‑בלבד. diff --git a/database/collections_manager.py b/database/collections_manager.py index 9f28be0b5..370a8109d 100644 --- a/database/collections_manager.py +++ b/database/collections_manager.py @@ -1286,11 +1286,7 @@ def get_tags_metadata(self) -> Dict[str, Any]: החזרת מטאדאטה על כל התגיות הזמינות. Returns: - dict: { - "allowed_tags": [...], - "categories": {...}, - "metadata": {...} - } + dict: מילון עם המפתחות ``allowed_tags``, ``categories`` ו-``metadata``. """ cache_key = "collections:tags_metadata" cache_obj = cache diff --git a/docs/environment-variables.rst b/docs/environment-variables.rst index f04eae6b4..91d1deab4 100644 --- a/docs/environment-variables.rst +++ b/docs/environment-variables.rst @@ -179,11 +179,11 @@ - ``my_webhook_secret`` - WebApp * - ``REPO_MIRROR_PATH`` - - נתיב בסיסי בדיסק לשמירת Bare Mirror של הריפו (Repo Sync Engine) + - נתיב בסיסי בדיסק לשמירת Bare Mirror של הריפו (Repo Sync Engine). נדרש גם בשירות ה-MCP עבור כלי דפדפן הריפו (אדמין בלבד) — השירות חייב דיסק עם ה-mirrors (ב-Render דיסק הוא פר-שירות). - לא - ``/var/data/repos`` - ``/var/data/repos`` - - WebApp + - MCP/WebApp * - ``REPO_NAME`` - שם ריפו לוגי לשימוש ב-Repo Sync (מפתח ל-mirror בדיסק ול-metadata ב-DB) - לא @@ -2120,6 +2120,24 @@ - "" - ``https://claude.ai`` - MCP + * - ``MCP_REPO_DENYLIST_EXTRA`` + - תבניות glob נוספות (CSV) ל-denylist הסודות של כלי דפדפן הריפו ב-MCP, מעל רשימת הבסיס המובנית (``.env*``, ``*.pem``, ``id_rsa*`` וכו'). ההתאמה case-insensitive על הנתיב המלא וה-basename. + - לא + - "" + - ``*.sqlite, private/*`` + - MCP + * - ``MCP_REPO_AUTOSYNC`` + - רענון אוטומטי של ה-mirrors המקומיים בשירות ה-MCP (thread רקע, כמו ה-worker בוובאפ): משכפל ריפו חסר מ-``repo_metadata.repo_url`` ומריץ fetch כשה-``last_synced_sha`` ב-Mongo שונה מהמקומי. ``0`` מכבה. + - לא + - ``1`` (פעיל) + - ``1`` + - MCP + * - ``MCP_REPO_AUTOSYNC_INTERVAL`` + - מרווח (שניות) בין מעברי הרענון של ה-autosync. מינימום 30. + - לא + - ``300`` + - ``120`` + - MCP דגלי בדיקות ופיתוח ------------------- diff --git a/docs/index.rst b/docs/index.rst index dbdc8a6af..6d4f68f27 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -68,6 +68,7 @@ Code Keeper Bot - תיעוד API development/scripts development/i18n integrations + mcp-server repository-integrations security monitoring diff --git a/docs/mcp-server.rst b/docs/mcp-server.rst new file mode 100644 index 000000000..5f4488111 --- /dev/null +++ b/docs/mcp-server.rst @@ -0,0 +1,291 @@ +שרת ה-MCP — חיבור Claude ל-CodeKeeper +====================================== + +שרת `MCP `_ (Model Context Protocol) שחושף את +CodeKeeper ל-Claude: הקבצים והאוספים האישיים של כל משתמש, ולאדמין — גם **דפדפן +הריפו** מעל ה-Repo Sync Engine. עובד גם מול **Claude.ai** (Custom Connector דרך +OAuth 2.1) וגם מול **Claude Code / Claude Desktop** (טוקן אישי). + +**קוד:** ``mcp_server/`` · **תכנון מלא:** +``FEATURE_SUGGESTIONS/FEATURE_MCP_CLAUDE_INTEGRATION.md`` + +מה זה נותן +----------- + +- Claude קורא ומחפש בקבצים השמורים שלך ישירות — בלי העתק-הדבק ובלי צילומי מסך. +- שמירה (יצירה/עדכון) מאחורי הרשאת ``write`` מפורשת — עדכון תמיד יוצר + **גרסה חדשה** + (append-only), אף פעם לא דורס. +- לאדמין: קריאה וחיפוש בכל הריפואים המשוקפים (תיעוד, קוד, מסמכי תכנון). + +ארכיטקטורה בקצרה +------------------ + +.. code-block:: text + + Claude.ai / Claude Code + │ Streamable HTTP (+OAuth 2.1 או Bearer PAT) + ▼ + שירות MCP נפרד (ASGI, uvicorn) ── import ישיר ──▶ database/ → MongoDB + │ (code_snippets, collections) + └── דפדפן ריפו (אדמין) ──▶ bare mirrors בדיסק המקומי (REPO_MIRROR_PATH) + +- ה-``user_id`` נגזר **תמיד מהטוקן** — לעולם לא מקלט הלקוח. +- מכבד את חוק ה-Smart Projection: רשימות/חיפוש מחזירים מטא-דאטה בלבד; + תוכן מלא רק + בבקשה מפורשת לקובץ בודד. + +הכלים +------ + +כלי משתמש (לכל משתמש מחובר): + +.. list-table:: + :header-rows: 1 + :widths: 35 65 + + * - כלי + - תיאור + * - ``codekeeper_list_files`` + - רשימת קבצים (מטא-דאטה, עם עימוד) + * - ``codekeeper_search_code`` + - חיפוש טקסט → מטא-דאטה של קבצים תואמים + * - ``codekeeper_get_file`` + - תוכן מלא של קובץ (לפי שם/מזהה, אופציונלית גרסה) + * - ``codekeeper_save_file`` + - **כתיבה:** יצירה/עדכון קובץ (גרסה חדשה; עד 100KB). דורש ``write`` + * - ``codekeeper_list_versions`` + - היסטוריית גרסאות של קובץ + * - ``codekeeper_list_collections`` / ``codekeeper_get_collection`` / ``codekeeper_get_collection_items`` + - האוספים והקבצים שבתוכם + +כלי אדמין (דפדפן הריפו — מוסתרים וחסומים לכל משתמש אחר): + +.. list-table:: + :header-rows: 1 + :widths: 35 65 + + * - כלי + - תיאור + * - ``codekeeper_list_repos`` + - הריפואים המשוקפים (מטא-דאטה) + * - ``codekeeper_list_repo_tree`` + - נתיבי קבצים בריפו (עימוד, סינון תיקייה/ref; בלי תוכן) + * - ``codekeeper_get_repo_file`` + - תוכן קובץ בודד (עד 500KB; קובץ בינארי → מטא-דאטה בלבד) + * - ``codekeeper_search_repo`` + - חיפוש טקסט בריפו (קטעים קצרים עם path+line, עם תקרות) + +אימות והרשאות +-------------- + +שני מסלולים, מאוחדים באותו שרת: + +1. **OAuth 2.1** — עבור Claude.ai (Custom Connector). זרימה מלאה: רישום + לקוח דינמי + (DCR) + PKCE + מסך אישור. הזהות נקבעת דרך התחברות הטלגרם בוובאפ. +2. **טוקן אישי (PAT)** — עבור Claude Code / Desktop. מונפק מהבוט בפקודת + ``/connect_claude`` (או ``/connect_claude write`` לטוקן עם הרשאת כתיבה), + נשמר כ-hash בלבד וניתן לביטול. + +שכבות ההרשאה: + +- ``read`` — ברירת המחדל לכל חיבור. +- ``write`` — נדרש ל-``codekeeper_save_file``; ניתן רק באישור מפורש + (מסך ההרשאה ב-Claude.ai או טוקן ``write`` מהבוט). +- **אדמין** — כלי הריפו זמינים רק למשתמשים שב-``ADMIN_USER_IDS``; לכל אחד + אחר הם + גם לא מופיעים ברשימת הכלים וגם נחסמים בקריאה ישירה (fail-closed). + +הפעלה — צעד אחר צעד +-------------------- + +שלב 1 — שירות חדש +~~~~~~~~~~~~~~~~~~ + +שירות web נפרד (ASGI) ב-Render, שמתחבר **לאותו MongoDB** של הבוט והוובאפ: + +.. code-block:: text + + Start command: uvicorn mcp_server.app:app --host 0.0.0.0 --port $PORT + Health check: /healthz + +שלב 2 — מצב בסיס (PAT בלבד) +~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +מספיק כדי לעבוד מול Claude Code / Desktop: + +.. list-table:: + :header-rows: 1 + :widths: 30 70 + + * - משתנה + - הערה + * - ``MONGODB_URL`` + ``DATABASE_NAME`` + - זהים לבוט/וובאפ + * - ``BOT_TOKEN`` + - נדרש רק לטעינת מודול ה-config המשותף + +שלב 3 — מצב OAuth (מוסיף את Claude.ai) +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +נדלק אוטומטית כשמוגדרים **בשירות ה-MCP**: + +.. list-table:: + :header-rows: 1 + :widths: 30 70 + + * - משתנה + - הערה + * - ``MCP_SERVER_URL`` + - ה-URL הציבורי (https) של שירות ה-MCP + * - ``WEBAPP_URL`` + - ה-URL הציבורי של הוובאפ (למסך התחברות הטלגרם) + * - ``SECRET_KEY`` + - **זהה לוובאפ**, ערך אקראי חזק (≥16 תווים) — חותם את זהות המשתמש בין השירותים + +בנוסף: על **הוובאפ** להגדיר ``MCP_SERVER_URL`` (+אותו ``SECRET_KEY``), ועל +**הבוט** ``MCP_SERVER_URL`` (לפקודת ``/connect_claude``). + +שלב 4 — דפדפן הריפו (אדמין, אופציונלי) +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +.. list-table:: + :header-rows: 1 + :widths: 30 70 + + * - משתנה + - הערה + * - ``ADMIN_USER_IDS`` + - מזהה הטלגרם של האדמין (CSV). ריק = הכלים כבויים לכולם + * - ``REPO_MIRROR_PATH`` + דיסק מצורף + - ב-Render דיסק הוא פר-שירות; בלי דיסק ה-mirrors משוכפלים מחדש אחרי כל deploy + * - ``GITHUB_TOKENS`` / ``GITHUB_TOKEN`` + - רק לריפואים פרטיים (אימות ל-clone/fetch) + +אין צורך ב-``GITHUB_WEBHOOK_SECRET`` בשירות ה-MCP — ה-webhook ממשיך להגיע +לוובאפ +בלבד, וה-MCP מתעדכן לבד (ראו "רענון אוטומטי" למטה). + +הרשימה המלאה של המשתנים: :doc:`environment-variables`. + +חיבור לקוחות +------------- + +Claude.ai (Custom Connector) +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Settings → Connectors → **Add custom connector** → הזינו את הכתובת:: + + https:///mcp + +זהו — Claude מבצע DCR + OAuth לבד, מפנה להתחברות טלגרם ולמסך אישור. אין צורך +ב-Client ID/Secret. כדי לקבל **write**, ה-connector צריך להירשם עם ההרשאה — אם +כבר חיברתם לקריאה בלבד, הסירו והוסיפו מחדש ואשרו "קריאה וכתיבה". + +Claude Code (טוקן) +~~~~~~~~~~~~~~~~~~~ + +שלחו לבוט בצ'אט פרטי ``/connect_claude`` (או ``/connect_claude write``), ואז: + +.. code-block:: bash + + claude mcp add --transport http codekeeper https:///mcp \ + --header "Authorization: Bearer " + +Claude Desktop +~~~~~~~~~~~~~~~ + +ב-``claude_desktop_config.json``: + +.. code-block:: json + + { + "mcpServers": { + "codekeeper": { + "type": "http", + "url": "https:///mcp", + "headers": { "Authorization": "Bearer " } + } + } + } + +דפדפן הריפו — איך זה עובד +--------------------------- + +רענון אוטומטי (autosync) +~~~~~~~~~~~~~~~~~~~~~~~~~ + +שירות ה-MCP מריץ thread רקע (אותו דפוס כמו ה-worker בוובאפ) ששומר על ה-mirrors +המקומיים טריים — בלי cron ובלי שירות נוסף: + +.. code-block:: text + + merge ל-main → GitHub webhook → הוובאפ מסנכרן את הדיסק שלו וכותב SHA ל-Mongo + → ה-autosync ב-MCP מזהה שה-SHA המקומי שונה → git fetch מקומי + +- ריפו שקיים ב-``repo_metadata`` אך חסר בדיסק המקומי — **משוכפל אוטומטית** + (אין צורך ב-import ידני בצד ה-MCP). +- שליטה: ``MCP_REPO_AUTOSYNC`` (ברירת מחדל פעיל), + ``MCP_REPO_AUTOSYNC_INTERVAL`` + (ברירת מחדל 300 שניות). + +מדיניות סינון סודות +~~~~~~~~~~~~~~~~~~~~ + +נתיבים רגישים (``.env*``, ``*.pem``, ``*.key``, ``id_rsa*``, ``secrets.*``, +``credentials*`` ועוד) **נחסמים** בקריאת קובץ, **מושמטים** מרשימות ו**מדולגים** +בחיפוש — בכל הריפואים, תמיד. ההתאמה case-insensitive על הנתיב המלא וה-basename +(תופסת גם ``config/.env`` מקונן). הרחבת הרשימה: ``MCP_REPO_DENYLIST_EXTRA`` +(CSV של תבניות glob). על שגיאה פנימית המדיניות **נכשלת סגור** (חוסמת). + +התנהגות בזמן sync +~~~~~~~~~~~~~~~~~~ + +אין נעילת קריאה מול ה-sync, ולכן קריאה שנכשלת בזמן ש-sync רץ מחזירה:: + + {"ok": false, "error": "sync_in_progress", "retry_after": 30} + +זה סימן **לנסות שוב אחרי המתנה קצרה** — לא להסיק שהקובץ או הריפו לא קיימים. + +אבטחה — עקרונות +---------------- + +- זהות תמיד מהטוקן; טוקנים נשמרים כ-hash בלבד ומוצגים פעם אחת. +- קוד הרשאה ו-refresh token חד-פעמיים (זיהוי replay); refresh לא מרחיב הרשאות. +- ``SECRET_KEY`` חלש/ריק/ברירת-מחדל — מצב OAuth מסרב לעלות (fail-closed). +- מסך האישור מציג במדויק קריאה בלבד לעומת קריאה וכתיבה. +- כלי האדמין: הסתרה מ-tools/list היא נוחות בלבד — האכיפה האמיתית היא בגוף + כל כלי. + +פתרון תקלות +------------ + +.. list-table:: + :header-rows: 1 + :widths: 30 70 + + * - סימפטום + - כיוון + * - ``401 invalid_token`` + - הטוקן בוטל/פג או ששירות ה-MCP מחובר ל-MongoDB אחר מזה שהנפיק אותו + * - ``421 Invalid Host header`` + - ``MCP_ALLOWED_HOSTS`` מגדיר host שלא תואם; ריק = הבדיקה כבויה + * - ``bad_assertion`` במסך האישור + - ``SECRET_KEY`` לא זהה בין הוובאפ לשירות ה-MCP + * - ``insufficient_scope`` בשמירה + - חיבור בקריאה בלבד — חברו מחדש עם write (או ``/connect_claude write``) + * - ``admin_only`` בכלי ריפו + - ה-user_id אינו ב-``ADMIN_USER_IDS`` בשירות ה-MCP + * - ``sync_in_progress`` + - רענון רץ ברגע זה — נסו שוב לפי ``retry_after`` + * - ``repo_or_ref_not_found`` + - ה-mirror עוד לא שוכפל מקומית (המתינו למעבר autosync) או שאין דיסק/``REPO_MIRROR_PATH`` + +ראו גם +------- + +- :doc:`environment-variables` — כל משתני הסביבה (כולל קטגוריית MCP) +- ``mcp_server/README.md`` — תיעוד תפעולי קצר בתוך הריפו +- ``FEATURE_SUGGESTIONS/FEATURE_MCP_CLAUDE_INTEGRATION.md`` — מסמך התכנון המלא +- :doc:`security` — מדיניות האבטחה הכללית diff --git a/mcp_server/README.md b/mcp_server/README.md index cf51870a2..5315fe5ec 100644 --- a/mcp_server/README.md +++ b/mcp_server/README.md @@ -31,6 +31,42 @@ Claude Desktop** (טוקן אישי). קריאה זמינה תמיד; **כתיב | `codekeeper_get_collection` | אוסף בודד לפי id | | `codekeeper_get_collection_items` | הקבצים בתוך אוסף (עם עימוד/סינון תיקייה) | +### כלי אדמין — דפדפן הריפו (פאזה ד', קריאה בלבד) + +ארבעה כלים מעל ה‑Repo Sync Engine (bare mirrors), **לאדמין בלבד** (`ADMIN_USER_IDS`). +למשתמש שאינו אדמין הם גם מוסתרים מ‑`tools/list` וגם חסומים בגוף הכלי (fail‑closed). + +| כלי | תיאור | +|-----|-------| +| `codekeeper_list_repos` | רשימת הריפואים המשוקפים (מטא‑דאטה) | +| `codekeeper_list_repo_tree` | נתיבי קבצים בריפו (עימוד, סינון תיקייה/ref; בלי תוכן) | +| `codekeeper_get_repo_file` | תוכן קובץ בודד (עד 500KB; בינארי ⇒ מטא‑דאטה בלבד) | +| `codekeeper_search_repo` | חיפוש טקסט בריפו (snippet קצר, עם תקרות) | + +- **מדיניות סודות (חובה):** נתיבים כמו `.env*`, `*.pem`, `id_rsa*` נחסמים/מושמטים בכל + הכלים; הרחבה דרך `MCP_REPO_DENYLIST_EXTRA` (CSV globs). +- **sync רץ ברקע?** כלי שנכשל בזמן sync מחזיר `sync_in_progress` + `retry_after` — + סימן לנסות שוב, לא להסיק שהקובץ לא קיים. + +#### רענון אוטומטי (autosync) — בלי cron ובלי שירות נוסף + +שירות ה‑MCP מריץ **thread רקע** (אותו דפוס כמו ה‑worker בוובאפ) שמחזיק את ה‑mirrors +המקומיים שלו טריים לבד: + +```text +merge ל-main → GitHub webhook → הוובאפ מסנכרן את הדיסק שלו וכותב last_synced_sha ל-Mongo + → ה-autosync ב-MCP מזהה שה-SHA המקומי שונה → git fetch לדיסק של ה-MCP +``` + +- ריפו שקיים ב‑`repo_metadata` אך חסר בדיסק המקומי — **משוכפל אוטומטית** מ‑`repo_url` + (אין צורך ב‑`initial_import` ידני בצד ה‑MCP). +- שליטה: `MCP_REPO_AUTOSYNC` (ברירת מחדל פעיל; `0` מכבה), `MCP_REPO_AUTOSYNC_INTERVAL` + (ברירת מחדל 300ש'). בזמן clone/fetch מקומי הכלים מחזירים `sync_in_progress`. +- **ENV נדרשים בשירות ה‑MCP:** `REPO_MIRROR_PATH` (+דיסק מצורף — ב‑Render דיסק הוא + פר‑שירות; בלי דיסק זה עובד אבל משוכפל מחדש אחרי כל deploy), ו‑`GITHUB_TOKENS`/ + `GITHUB_TOKEN` לריפואים פרטיים. **אין צורך** ב‑`GITHUB_WEBHOOK_SECRET` כאן — + ה‑webhook ממשיך להגיע לוובאפ בלבד. + --- ## אימות — שני מסלולים (מאוחדים תחת `load_access_token`) diff --git a/mcp_server/app.py b/mcp_server/app.py index 8237553dd..5bef08acd 100644 --- a/mcp_server/app.py +++ b/mcp_server/app.py @@ -85,6 +85,23 @@ def create_app(): backend = ProductionBackend(db_manager=db_manager, mongo_db=mongo) name = os.getenv("MCP_SERVER_NAME", "CodeKeeper") + # Phase D: admin-only repo-browser tools (hidden + gated for non-admins). + from .repo_backend import RepoBackend + + repo_backend = RepoBackend(db=mongo) + + # Keep this service's local mirrors fresh automatically (webapp-worker + # pattern: background daemon thread; no cron/extra service). Merges to main + # reach the webapp webhook → shared Mongo SHA → this loop fetches locally. + try: + from .repo_autosync import start_autosync + + start_autosync(mongo) + except Exception: + import logging + + logging.getLogger(__name__).warning("repo autosync failed to start", exc_info=True) + mcp_base = (os.getenv("MCP_SERVER_URL") or "").rstrip("/") webapp_base = (os.getenv("WEBAPP_URL") or "").rstrip("/") @@ -97,11 +114,12 @@ def create_app(): auth_provider=provider, auth_settings=settings, consent_routes=consent, + repo_backend=repo_backend, name=name, ) # Fallback: PAT-only (Claude Code/Desktop) — runs without OAuth config. - return build_app(backend, MCPTokenStore(mongo), name=name) + return build_app(backend, MCPTokenStore(mongo), repo_backend=repo_backend, name=name) app = create_app() diff --git a/mcp_server/auth.py b/mcp_server/auth.py index f88f0009d..bbf56e2bf 100644 --- a/mcp_server/auth.py +++ b/mcp_server/auth.py @@ -131,3 +131,38 @@ def require_write(ctx: Any = None) -> None: "CodeKeeper granting write access (re-add the connector, or use a " "write-enabled token)." ) + + +def admin_user_ids() -> set[int]: + """The canonical admin set: ``config.ADMIN_USER_IDS`` — and nothing else. + + Deliberately does NOT honor the ``CHATOPS_ALLOW_ALL_IF_NO_ADMINS`` escape + hatch (chatops/permissions.py): for the admin-only repo tools an empty list + must mean *nobody* is admin. Fail-closed: any error ⇒ empty set. + """ + try: + from config import config as _cfg + + return {int(x) for x in (getattr(_cfg, "ADMIN_USER_IDS", None) or [])} + except Exception: + return set() + + +def is_admin_user(user_id: Any) -> bool: + try: + return int(user_id) in admin_user_ids() + except (TypeError, ValueError): + return False + + +def require_admin(ctx: Any = None) -> int: + """Return the verified admin ``user_id`` or raise (fail-closed). + + Identity comes from the token only (``current_user_id``); the admin check is + membership in ``ADMIN_USER_IDS``. Like ``require_write``, a raised + ``PermissionError`` reaches the model as an error tool result. + """ + user_id = current_user_id(ctx) # raises PermissionError when unauthenticated + if not is_admin_user(user_id): + raise PermissionError("admin_only: this tool is restricted to the CodeKeeper admin user.") + return int(user_id) diff --git a/mcp_server/repo_autosync.py b/mcp_server/repo_autosync.py new file mode 100644 index 000000000..9e6318b6b --- /dev/null +++ b/mcp_server/repo_autosync.py @@ -0,0 +1,195 @@ +"""Background auto-refresh of the MCP service's local bare mirrors (Phase D). + +Same pattern as the webapp's sync worker — a lazily-started daemon thread inside +the existing service (no cron, no extra Render service) — but decoupled from the +webhook/job queue: the webapp keeps receiving GitHub webhooks and updating +``repo_metadata.last_synced_sha`` in the shared Mongo; this loop compares that +SHA to the local mirror and clones/fetches when they differ. + +So a merge to main flows end-to-end automatically: +webhook → webapp sync (its own disk + SHA in Mongo) → this loop notices the +drift within one interval → ``git fetch`` into THIS service's disk. A repo that +was never mirrored here is self-cloned from ``repo_metadata.repo_url`` +(``init_mirror`` is idempotent), so no manual ``initial_import`` is needed on +the MCP side. Private repos need ``GITHUB_TOKENS``/``GITHUB_TOKEN`` set here. + +While a repo is being cloned/fetched, ``is_refreshing(repo)`` is True and the +read tools report ``sync_in_progress`` + ``retry_after`` instead of "not found". + +Env: +- ``MCP_REPO_AUTOSYNC`` — "0"/"false" disables (default: enabled). +- ``MCP_REPO_AUTOSYNC_INTERVAL`` — seconds between passes (default 300, min 30). +""" + +from __future__ import annotations + +import logging +import os +import re +import threading +import time +from typing import Any + +logger = logging.getLogger(__name__) + +# Log redaction (defense-in-depth): the engine already sanitizes its own git +# stderr at the source (_run_git_command → _sanitize_output), but we log +# ``message`` values from a duck-typed dependency — so scrub credential shapes +# ourselves before anything reaches the logs (CLAUDE.md: no secrets in logs). +_URL_CRED_RE = re.compile(r"(https?://)[^/@\s]+@") +_GH_TOKEN_RE = re.compile(r"\b(?:gh[pousr]_[A-Za-z0-9]{20,}|github_pat_[A-Za-z0-9_]{20,})\b") + + +def _redact(text: Any) -> str: + """Strip URL userinfo credentials + GitHub-token shapes from log-bound text. + + Fail-closed: on any internal error return a placeholder, never the raw text. + """ + try: + s = str(text or "") + s = _URL_CRED_RE.sub(r"\1***@", s) + return _GH_TOKEN_RE.sub("***", s) + except Exception: + return "" + + +DEFAULT_INTERVAL_SECONDS = 300 +_MIN_INTERVAL_SECONDS = 30 +_STARTUP_DELAY_SECONDS = 10 # let the app finish booting before the first pass + +_ENABLE_ENV = "MCP_REPO_AUTOSYNC" +_INTERVAL_ENV = "MCP_REPO_AUTOSYNC_INTERVAL" + +_start_lock = threading.Lock() +_thread: threading.Thread | None = None + +_active_lock = threading.Lock() +_active: set[str] = set() + + +def is_refreshing(repo_name: Any) -> bool: + """True while this service is cloning/fetching ``repo_name`` locally.""" + with _active_lock: + return str(repo_name or "") in _active + + +def _mark(repo_name: str, active: bool) -> None: + with _active_lock: + if active: + _active.add(repo_name) + else: + _active.discard(repo_name) + + +def autosync_enabled() -> bool: + return os.getenv(_ENABLE_ENV, "1").strip().lower() not in ("0", "false", "no", "off") + + +def _interval_seconds() -> int: + try: + return max(_MIN_INTERVAL_SECONDS, int(os.getenv(_INTERVAL_ENV, DEFAULT_INTERVAL_SECONDS))) + except (TypeError, ValueError): + return DEFAULT_INTERVAL_SECONDS + + +def refresh_once(db: Any, mirror: Any) -> dict[str, int]: + """One refresh pass over every repo in ``repo_metadata``. Never raises. + + Per repo: missing mirror + known URL ⇒ clone; existing mirror whose local + SHA differs from the webapp-written ``last_synced_sha`` (or when either SHA + is unknown) ⇒ delta fetch; identical SHAs ⇒ skip. + """ + stats = {"checked": 0, "cloned": 0, "fetched": 0, "skipped": 0, "errors": 0} + try: + repos = list( + db["repo_metadata"].find( + {}, + { + "_id": 0, + "repo_name": 1, + "repo_url": 1, + "default_branch": 1, + "last_synced_sha": 1, + }, + ) + ) + except Exception: + logger.warning("repo autosync: repo_metadata query failed", exc_info=True) + return stats + + for meta in repos: + name = str(meta.get("repo_name") or "").strip() + if not name: + continue + stats["checked"] += 1 + _mark(name, True) + try: + if not mirror.mirror_exists(name): + url = str(meta.get("repo_url") or "").strip() + if not url: + stats["skipped"] += 1 + continue + res = mirror.init_mirror(url, name) or {} + if res.get("success"): + stats["cloned"] += 1 + else: + stats["errors"] += 1 + logger.warning( + "repo autosync: clone failed for %s: %s", name, _redact(res.get("message")) + ) + continue + + branch = str(meta.get("default_branch") or "main") + db_sha = str(meta.get("last_synced_sha") or "").strip() + local_sha = str(mirror.get_current_sha(name, branch) or "").strip() + if db_sha and local_sha and db_sha == local_sha: + stats["skipped"] += 1 + continue + res = mirror.fetch_updates(name) or {} + if res.get("success"): + stats["fetched"] += 1 + else: + stats["errors"] += 1 + logger.warning( + "repo autosync: fetch failed for %s: %s", name, _redact(res.get("message")) + ) + except Exception: + stats["errors"] += 1 + logger.warning("repo autosync: refresh failed for %s", name, exc_info=True) + finally: + _mark(name, False) + return stats + + +def start_autosync(db: Any, *, interval: int | None = None) -> bool: + """Start the daemon refresher (idempotent). Returns True if it is running. + + Mirrors the webapp's lazily-started worker-thread pattern; disabled cleanly + via MCP_REPO_AUTOSYNC=0 (tools still work — reads just rely on whatever is + on disk). + """ + global _thread + if not autosync_enabled(): + logger.info("repo autosync disabled via %s", _ENABLE_ENV) + return False + with _start_lock: + if _thread is not None and _thread.is_alive(): + return True + + def _loop() -> None: + time.sleep(_STARTUP_DELAY_SECONDS) + while True: + try: + from services.git_mirror_service import get_mirror_service # lazy heavy import + + stats = refresh_once(db, get_mirror_service()) + if stats["cloned"] or stats["fetched"] or stats["errors"]: + logger.info("repo autosync pass: %s", stats) + except Exception: + logger.warning("repo autosync pass failed", exc_info=True) + time.sleep(interval or _interval_seconds()) + + _thread = threading.Thread(target=_loop, daemon=True, name="mcp-repo-autosync") + _thread.start() + logger.info("repo autosync started (interval=%ss)", interval or _interval_seconds()) + return True diff --git a/mcp_server/repo_backend.py b/mcp_server/repo_backend.py new file mode 100644 index 000000000..73b20461f --- /dev/null +++ b/mcp_server/repo_backend.py @@ -0,0 +1,295 @@ +"""Read-only data access for the repo-browser tools (Phase D, admin-only). + +Wraps the existing Repo Sync Engine in-process (``GitMirrorService`` + +``RepoSearchService`` + the ``repo_metadata``/``sync_jobs`` collections) — +no new persistence logic. Keyed by logical ``repo_name``, not ``user_id``. + +Resilience contract: the sync worker may run ``git fetch``/``gc`` concurrently +(there are no read locks), so a failed read checks for a *running* sync job and +returns ``{"error": "sync_in_progress", "retry_after": N}`` — telling the +calling model to retry shortly instead of concluding the repo/file is missing. + +The secrets policy (``repo_policy``) is applied on every surface: tree omits, +search skips, get blocks. Heavy content is returned only by ``get_file`` +(Smart Projection). +""" + +from __future__ import annotations + +import logging +from typing import Any + +from .backend import _json_safe +from .repo_handlers import TREE_PER_PAGE_MAX +from .repo_policy import is_denied + +logger = logging.getLogger(__name__) + +SYNC_RETRY_AFTER_SECONDS = 30 + + +def _safe_int(value: Any, default: int) -> int: + """Best-effort int conversion; invalid input ⇒ default (clamp policy, 13.4).""" + try: + return int(value) + except (TypeError, ValueError): + return default + + +_REPOS_PROJECTION = { + "_id": 0, + "repo_name": 1, + "repo_url": 1, + "default_branch": 1, + "last_sync_time": 1, + "last_synced_sha": 1, + "total_files": 1, + "sync_status": 1, +} + + +class RepoBackend: + """Duck-typed backend over a pymongo handle + the mirror/search services. + + ``mirror`` / ``search_service`` are injectable for tests and lazily resolved + in production (importing the services stack only when a repo tool runs). + """ + + def __init__(self, db: Any = None, mirror: Any = None, search_service: Any = None) -> None: + self._db = db + self._mirror = mirror + self._search = search_service + self._ensure_indexes() + + # -- wiring ------------------------------------------------------------ + def _require_mirror(self) -> Any: + if self._mirror is None: + from services.git_mirror_service import get_mirror_service # lazy heavy import + + self._mirror = get_mirror_service() + return self._mirror + + def _require_search(self) -> Any: + if self._search is None: + from services.repo_search_service import create_search_service # lazy + + self._search = create_search_service(self._db) + return self._search + + def _ensure_indexes(self) -> None: + # list_repos runs on repo_metadata on every call, and the collection had + # no index at all — closing that gap is part of this phase, not "later". + try: + if self._db is not None: + self._db["repo_metadata"].create_index("repo_name", unique=True) + except Exception: + logger.warning("repo_metadata index creation failed (non-fatal)", exc_info=True) + + # -- helpers ----------------------------------------------------------- + def _sync_running(self, repo_name: str) -> bool: + # Local autosync (this service cloning/fetching right now) … + try: + from .repo_autosync import is_refreshing + + if is_refreshing(repo_name): + return True + except Exception: + pass + # … or the webapp's webhook-driven sync worker (shared job queue). + try: + if self._db is None: + return False + doc = self._db["sync_jobs"].find_one({"repo_name": repo_name, "status": "running"}) + return doc is not None + except Exception: + return False + + def _transient_error(self, repo_name: str, fallback: str) -> dict[str, Any]: + """Map a failed read to sync_in_progress (retryable) when a sync runs.""" + if self._sync_running(repo_name): + return { + "ok": False, + "error": "sync_in_progress", + "retry_after": SYNC_RETRY_AFTER_SECONDS, + "message": ( + "A sync is running for this repo right now; the repo/file may " + "exist — retry after a short wait instead of assuming absence." + ), + } + return {"ok": False, "error": fallback} + + def _default_ref(self, repo_name: str) -> str: + try: + meta = ( + self._db["repo_metadata"].find_one({"repo_name": repo_name}) + if self._db is not None + else None + ) + except Exception: + meta = None + branch = (meta or {}).get("default_branch") + return f"refs/heads/{branch}" if branch else "HEAD" + + # -- tools ------------------------------------------------------------- + def list_repos(self, *, limit: int = 50) -> dict[str, Any]: + try: + cursor = ( + self._db["repo_metadata"] + .find({}, _REPOS_PROJECTION) + .sort("repo_name", 1) + .limit(int(limit)) + ) + repos = [_json_safe(dict(doc)) for doc in cursor] + except Exception: + logger.warning("list_repos query failed", exc_info=True) + return {"ok": False, "error": "db_error"} + return {"ok": True, "count": len(repos), "repos": repos} + + def list_tree( + self, + *, + repo: str, + path: str | None = None, + ref: str | None = None, + page: int = 1, + per_page: int = 200, + byte_budget: int = 256_000, + ) -> dict[str, Any]: + use_ref = ref or self._default_ref(repo) + try: + files = self._require_mirror().list_all_files(repo, use_ref) + except Exception: + logger.warning("list_tree read failed", exc_info=True) + files = None + if files is None: + return self._transient_error(repo, "repo_or_ref_not_found") + + prefix = (path or "").strip().strip("/") + if prefix: + files = [f for f in files if f == prefix or f.startswith(prefix + "/")] + files = [f for f in files if not is_denied(f)] # policy: omit + total = len(files) + + # Defense-in-depth: the handler already clamps, but this method is a + # public API — normalize again so a direct caller can't slice with a + # negative start or crash on a non-numeric value. + page_i = max(1, _safe_int(page, 1)) + per_page_i = min(max(1, _safe_int(per_page, 200)), TREE_PER_PAGE_MAX) + + start = (page_i - 1) * per_page_i + page_items = files[start : start + per_page_i] + # Output byte budget: never let one page blow up the response. + out: list[str] = [] + used = 0 + truncated = False + for item in page_items: + used += len(item.encode("utf-8")) + 8 + if used > byte_budget: + truncated = True + break + out.append(item) + return { + "ok": True, + "repo": repo, + "ref": use_ref, + "path": prefix or None, + "total": total, + "page": page_i, + "per_page": per_page_i, + "paths": out, + "truncated": truncated, + } + + def get_file(self, *, repo: str, path: str, ref: str | None = None) -> dict[str, Any]: + if is_denied(path): # policy: block, before touching the mirror + return {"ok": False, "error": "path_denied"} + use_ref = ref or self._default_ref(repo) + try: + res = self._require_mirror().get_file_at_commit(repo, path, use_ref) + except Exception: + logger.warning("get_file read failed", exc_info=True) + res = {"error": "internal_error"} + + if res.get("success"): + file_meta: dict[str, Any] = { + "path": res.get("file_path", path), + "ref": use_ref, + "resolved_commit": res.get("resolved_commit"), + "size": res.get("size"), + } + if res.get("is_binary"): + return {"ok": True, "status": "binary", "file": file_meta} + file_meta["lines"] = res.get("lines") + file_meta["encoding"] = res.get("encoding") + return {"ok": True, "status": "ok", "file": file_meta, "content": res.get("content")} + + err = str(res.get("error") or "internal_error") + if err == "file_too_large": + return { + "ok": True, + "status": "too_large", + "file": {"path": path, "ref": use_ref, "size": res.get("size")}, + "max": res.get("max_size"), + } + if err == "file_not_in_commit": + return {"ok": False, "error": "not_found"} + if err in ("invalid_repo_name", "invalid_file_path"): + return {"ok": False, "error": "invalid_input"} + # repo_not_found / invalid_commit / git_error / timeout / internal_error: + # possibly a transient race with a running sync — say so if it is. + fallback = "not_found" if err in ("repo_not_found", "invalid_commit") else "read_failed" + return self._transient_error(repo, fallback) + + def search( + self, + *, + repo: str, + query: str, + file_pattern: str | None = None, + max_results: int = 50, + byte_budget: int = 256_000, + ) -> dict[str, Any]: + try: + res = self._require_search().search( + repo, + query, + search_type="content", + file_pattern=(file_pattern or None), + max_results=int(max_results), + ) + except Exception: + logger.warning("search failed", exc_info=True) + return self._transient_error(repo, "search_failed") + if res.get("error") and not res.get("results"): + return self._transient_error(repo, "search_failed") + + # total reflects what we can actually serve: the policy-filtered matches + # (NOT the engine's raw total, which may count denied paths). + filtered = [r for r in (res.get("results") or []) if not is_denied(r.get("path", ""))] + total = len(filtered) + capped = filtered[: max(0, _safe_int(max_results, 50))] # cap TOTAL matches + cap_truncated = total > len(capped) + + out: list[dict[str, Any]] = [] + used = 0 + budget_truncated = False + for r in capped: + row = { + "path": r.get("path"), + "line": r.get("line"), + "snippet": str(r.get("content") or "")[:500], + } + used += len(str(row).encode("utf-8")) + if used > byte_budget: + budget_truncated = True + break + out.append(row) + return { + "ok": True, + "repo": repo, + "query": query, + "count": len(out), + "total": total, + "results": out, + "truncated": bool(cap_truncated or budget_truncated or res.get("truncated")), + } diff --git a/mcp_server/repo_handlers.py b/mcp_server/repo_handlers.py new file mode 100644 index 000000000..967cc8635 --- /dev/null +++ b/mcp_server/repo_handlers.py @@ -0,0 +1,82 @@ +"""Pure handlers for the admin-only repo-browser tools (Phase D). + +Same contract as ``handlers.py``: plain functions, no MCP/Starlette imports, +clamped inputs, ``{"ok": False, "error": "..."}`` rejections. Server-side +defaults/maxima and the output byte budget follow the plan (FEATURE doc §13.4); +out-of-range values are **clamped** (the established ``_clamp`` pattern), never +rejected. The admin gate itself lives in the tool bodies (``require_admin``), +not here — handlers stay pure. +""" + +from __future__ import annotations + +from typing import Any + +from .handlers import _clamp + +REPOS_LIMIT_DEFAULT = 50 +REPOS_LIMIT_MAX = 200 +TREE_PER_PAGE_DEFAULT = 200 +TREE_PER_PAGE_MAX = 1000 +SEARCH_RESULTS_DEFAULT = 50 +SEARCH_RESULTS_MAX = 100 +OUTPUT_BYTE_BUDGET = 256_000 + + +def list_repos(backend: Any, *, limit: int = REPOS_LIMIT_DEFAULT) -> dict[str, Any]: + return backend.list_repos(limit=_clamp(limit, 1, REPOS_LIMIT_MAX, REPOS_LIMIT_DEFAULT)) + + +def list_repo_tree( + backend: Any, + *, + repo: str, + path: str | None = None, + ref: str | None = None, + page: int = 1, + per_page: int = TREE_PER_PAGE_DEFAULT, +) -> dict[str, Any]: + name = (repo or "").strip() + if not name: + return {"ok": False, "error": "missing_repo"} + return backend.list_tree( + repo=name, + path=(path or None), + ref=((ref or "").strip() or None), + page=_clamp(page, 1, 10**9, 1), + per_page=_clamp(per_page, 1, TREE_PER_PAGE_MAX, TREE_PER_PAGE_DEFAULT), + byte_budget=OUTPUT_BYTE_BUDGET, + ) + + +def get_repo_file(backend: Any, *, repo: str, path: str, ref: str | None = None) -> dict[str, Any]: + name = (repo or "").strip() + file_path = (path or "").strip() + if not name: + return {"ok": False, "error": "missing_repo"} + if not file_path: + return {"ok": False, "error": "missing_path"} + return backend.get_file(repo=name, path=file_path, ref=((ref or "").strip() or None)) + + +def search_repo( + backend: Any, + *, + repo: str, + query: str, + file_pattern: str | None = None, + max_results: int = SEARCH_RESULTS_DEFAULT, +) -> dict[str, Any]: + name = (repo or "").strip() + q = (query or "").strip() + if not name: + return {"ok": False, "error": "missing_repo"} + if len(q) < 2: + return {"ok": False, "error": "query_too_short"} + return backend.search( + repo=name, + query=q, + file_pattern=((file_pattern or "").strip() or None), + max_results=_clamp(max_results, 1, SEARCH_RESULTS_MAX, SEARCH_RESULTS_DEFAULT), + byte_budget=OUTPUT_BYTE_BUDGET, + ) diff --git a/mcp_server/repo_policy.py b/mcp_server/repo_policy.py new file mode 100644 index 000000000..3b94d4ce9 --- /dev/null +++ b/mcp_server/repo_policy.py @@ -0,0 +1,67 @@ +"""Secrets-path policy for the repo-browser tools (Phase D, admin-only). + +The bare mirrors can contain anything that was ever committed — including +``.env`` files, private keys and credential stores. The engine's read paths +serve file contents verbatim (no redaction), so this policy is a **mandatory +precondition** for the MCP repo tools (FEATURE doc §13.5): + +- ``get_repo_file`` **blocks** a denied path (``path_denied``), +- ``list_repo_tree`` **omits** denied paths from listings, +- ``search_repo`` **skips** them in results. + +Matching is fail-closed: any internal error while evaluating a path counts as +denied. Paths are normalized (posix separators, ``normpath``, lowercase) and +matched case-insensitively against both the full path and the basename, so +nested paths (``config/.env``) and case variants (``.ENV``) are covered. + +Stdlib-only on purpose — trivially unit-testable, importable anywhere. +""" + +from __future__ import annotations + +import fnmatch +import os +import posixpath + +# Baseline denylist (lowercase glob patterns, matched against basename AND full +# path). Deliberately errs on over-blocking — this is a security filter, not a +# relevance filter. Extend per-deploy via MCP_REPO_DENYLIST_EXTRA (CSV globs). +BASENAME_DENYLIST: tuple[str, ...] = ( + ".env*", + "*.pem", + "*.key", + "id_rsa*", + "id_ed25519*", + "id_ecdsa*", + "id_dsa*", + "secrets.*", + "credentials*", + "*.p12", + "*.pfx", + ".netrc", + ".npmrc", + "*.keystore", + "*.jks", +) + +_EXTRA_ENV = "MCP_REPO_DENYLIST_EXTRA" + + +def _patterns() -> tuple[str, ...]: + extra = tuple(p.strip().lower() for p in os.getenv(_EXTRA_ENV, "").split(",") if p.strip()) + return BASENAME_DENYLIST + extra + + +def is_denied(path: object) -> bool: + """Return True if ``path`` must not be served. Errors ⇒ True (fail closed).""" + try: + norm = posixpath.normpath(str(path or "").replace("\\", "/")).lower().lstrip("/") + if not norm or norm in (".", ".."): + return True + base = posixpath.basename(norm) + for pattern in _patterns(): + if fnmatch.fnmatchcase(base, pattern) or fnmatch.fnmatchcase(norm, pattern): + return True + return False + except Exception: + return True diff --git a/mcp_server/server.py b/mcp_server/server.py index 92edab43e..fbe6b3c5a 100644 --- a/mcp_server/server.py +++ b/mcp_server/server.py @@ -19,8 +19,14 @@ from starlette.responses import JSONResponse from starlette.routing import Route -from . import handlers -from .auth import PATAuthMiddleware, current_user_id, require_write +from . import handlers, repo_handlers +from .auth import ( + PATAuthMiddleware, + current_user_id, + is_admin_user, + require_admin, + require_write, +) _INSTRUCTIONS = ( "Access the current user's private code files and collections stored in " @@ -50,6 +56,39 @@ "openWorldHint": False, } +# Admin-only repo-browser tools (Phase D). Hidden from tools/list for +# non-admins by AdminAwareFastMCP — but hiding is UX only, NOT access control: +# every one of these also calls require_admin(ctx) in its body. +_ADMIN_TOOLS = frozenset( + { + "codekeeper_list_repos", + "codekeeper_list_repo_tree", + "codekeeper_get_repo_file", + "codekeeper_search_repo", + } +) + + +class AdminAwareFastMCP(FastMCP): + """FastMCP that hides the admin-only tools from non-admin tools/list. + + The SDK's tools/list is static (one ToolManager), but the auth context IS + available inside the handler, so we filter per request. Fail-closed: any + doubt (no request context, unauthenticated, lookup error) ⇒ non-admin view. + """ + + async def list_tools(self): # type: ignore[override] + tools = await super().list_tools() + if self._request_is_admin(): + return tools + return [t for t in tools if t.name not in _ADMIN_TOOLS] + + def _request_is_admin(self) -> bool: + try: + return is_admin_user(current_user_id(self.get_context())) + except Exception: + return False + def _transport_security() -> TransportSecuritySettings: """DNS-rebinding protection config for the Streamable-HTTP transport. @@ -81,6 +120,7 @@ def build_mcp( name: str = "CodeKeeper", auth_provider: Any = None, auth_settings: Any = None, + repo_backend: Any = None, ) -> FastMCP: kwargs: dict[str, Any] = { "instructions": _INSTRUCTIONS, @@ -92,7 +132,7 @@ def build_mcp( # register) plus the auth layer that calls provider.load_access_token. kwargs["auth_server_provider"] = auth_provider kwargs["auth"] = auth_settings - mcp: FastMCP = FastMCP(name, **kwargs) + mcp: FastMCP = AdminAwareFastMCP(name, **kwargs) @mcp.tool( name="codekeeper_list_files", @@ -202,9 +242,87 @@ def get_collection_items( folder=folder, ) + if repo_backend is not None: + _register_repo_tools(mcp, repo_backend) + return mcp +def _register_repo_tools(mcp: FastMCP, repo_backend: Any) -> None: + """Admin-only, read-only repo-browser tools (Phase D). + + Every body calls require_admin FIRST (fail-closed) — the tools/list hiding + in AdminAwareFastMCP is visibility only. Names must stay in _ADMIN_TOOLS. + """ + + @mcp.tool( + name="codekeeper_list_repos", + description="[Admin] List the mirrored repositories (metadata only).", + annotations=_READ_ONLY_TOOL, + ) + def list_repos(ctx: Context, limit: int = 50) -> dict: + require_admin(ctx) + return repo_handlers.list_repos(repo_backend, limit=limit) + + @mcp.tool( + name="codekeeper_list_repo_tree", + description=( + "[Admin] List file paths in a mirrored repo (paginated; optional " + "subdirectory/ref filter; paths only, no content)." + ), + annotations=_READ_ONLY_TOOL, + ) + def list_repo_tree( + ctx: Context, + repo: str, + path: str | None = None, + ref: str | None = None, + page: int = 1, + per_page: int = 200, + ) -> dict: + require_admin(ctx) + return repo_handlers.list_repo_tree( + repo_backend, repo=repo, path=path, ref=ref, page=page, per_page=per_page + ) + + @mcp.tool( + name="codekeeper_get_repo_file", + description=( + "[Admin] Read one file from a mirrored repo (max 500KB; binary files " + "return metadata only). On sync_in_progress, retry after retry_after " + "seconds — the file may exist." + ), + annotations=_READ_ONLY_TOOL, + ) + def get_repo_file(ctx: Context, repo: str, path: str, ref: str | None = None) -> dict: + require_admin(ctx) + return repo_handlers.get_repo_file(repo_backend, repo=repo, path=path, ref=ref) + + @mcp.tool( + name="codekeeper_search_repo", + description=( + "[Admin] Text-search inside a mirrored repo; returns short snippets " + "(path+line), capped and truncated-flagged." + ), + annotations=_READ_ONLY_TOOL, + ) + def search_repo( + ctx: Context, + repo: str, + query: str, + file_pattern: str | None = None, + max_results: int = 50, + ) -> dict: + require_admin(ctx) + return repo_handlers.search_repo( + repo_backend, + repo=repo, + query=query, + file_pattern=file_pattern, + max_results=max_results, + ) + + async def _healthz(_request): return JSONResponse({"status": "ok", "service": "codekeeper-mcp"}) @@ -216,6 +334,7 @@ def build_app( auth_provider: Any = None, auth_settings: Any = None, consent_routes: Any = None, + repo_backend: Any = None, name: str = "CodeKeeper", ): """Build the authenticated Streamable-HTTP ASGI app. @@ -232,6 +351,7 @@ def build_app( name=name, auth_provider=auth_provider if oauth else None, auth_settings=auth_settings if oauth else None, + repo_backend=repo_backend, ) app = mcp.streamable_http_app() # Starlette app exposing POST/GET /mcp # Unauthenticated health endpoint for the hosting platform. diff --git a/scripts/create_repo_indexes.py b/scripts/create_repo_indexes.py index 151e0787a..6e6a97f8f 100644 --- a/scripts/create_repo_indexes.py +++ b/scripts/create_repo_indexes.py @@ -17,41 +17,42 @@ # repo_files - לחיפוש לפי שפה db.repo_files.create_index([("repo_name", 1), ("language", 1)]) +# repo_metadata - מפתח לוגי יחיד; list_repos (MCP) רץ עליו בכל קריאה. +# בדיקת כפילויות קודם: כפילות קיימת תפיל את יצירת ה-unique (ואת שאר הסקריפט אחריה) - +# במקרה כזה מדווחים ומדלגים, כדי שהניקוי ייעשה במודע ולא באמצע ריצת אינדקסים. +_dupes = list( + db.repo_metadata.aggregate( + [ + {"$group": {"_id": "$repo_name", "count": {"$sum": 1}}}, + {"$match": {"count": {"$gt": 1}}}, + ] + ) +) +if _dupes: + print( + "WARNING: duplicate repo_name values in repo_metadata - " + f"skipping unique index until cleaned: {sorted(d['_id'] for d in _dupes)}" + ) +else: + db.repo_metadata.create_index("repo_name", unique=True) + # code_snippets - base index for attention widget db.code_snippets.create_index( - [ - ('user_id', 1), - ('is_active', 1), - ('updated_at', -1) - ], - name='idx_attention_base' + [("user_id", 1), ("is_active", 1), ("updated_at", -1)], name="idx_attention_base" ) # code_snippets - stale files with tags index db.code_snippets.create_index( - [ - ('user_id', 1), - ('is_active', 1), - ('updated_at', 1), - ('tags.0', 1) - ], - name='idx_attention_stale_with_tags' + [("user_id", 1), ("is_active", 1), ("updated_at", 1), ("tags.0", 1)], + name="idx_attention_stale_with_tags", ) # attention_dismissals - unique index db.attention_dismissals.create_index( - [ - ('user_id', 1), - ('file_id', 1) - ], - unique=True, - name='idx_attention_dismissals_unique' + [("user_id", 1), ("file_id", 1)], unique=True, name="idx_attention_dismissals_unique" ) # attention_dismissals - TTL index db.attention_dismissals.create_index( - [('expires_at', 1)], - expireAfterSeconds=0, - name='idx_attention_dismissals_ttl' + [("expires_at", 1)], expireAfterSeconds=0, name="idx_attention_dismissals_ttl" ) - diff --git a/services/config_inspector_service.py b/services/config_inspector_service.py index 6d40051d8..698fc59f0 100644 --- a/services/config_inspector_service.py +++ b/services/config_inspector_service.py @@ -545,6 +545,30 @@ class ConfigService: description="Origin מותרים לשרת ה-MCP (CSV). רלוונטי רק כשמפעילים הגנה דרך MCP_ALLOWED_HOSTS.", category="mcp", ), + "MCP_REPO_DENYLIST_EXTRA": ConfigDefinition( + key="MCP_REPO_DENYLIST_EXTRA", + default="", + description=( + "תבניות glob נוספות (CSV) ל-denylist הסודות של כלי דפדפן הריפו ב-MCP, " + "מעל רשימת הבסיס המובנית (.env*, *.pem, id_rsa* וכו')." + ), + category="mcp", + ), + "MCP_REPO_AUTOSYNC": ConfigDefinition( + key="MCP_REPO_AUTOSYNC", + default="1", + description=( + "רענון אוטומטי של ה-mirrors המקומיים בשירות ה-MCP (thread רקע): " + "שכפול ריפו חסר מ-repo_metadata.repo_url ו-fetch כשה-SHA ב-Mongo שונה מהמקומי. 0 מכבה." + ), + category="mcp", + ), + "MCP_REPO_AUTOSYNC_INTERVAL": ConfigDefinition( + key="MCP_REPO_AUTOSYNC_INTERVAL", + default="300", + description="מרווח בשניות בין מעברי ה-autosync של דפדפן הריפו ב-MCP (מינימום 30).", + category="mcp", + ), # --- Repo Sync Engine (Git Mirror) --- "REPO_NAME": ConfigDefinition( diff --git a/tests/test_mcp_repo_autosync.py b/tests/test_mcp_repo_autosync.py new file mode 100644 index 000000000..f331bcea3 --- /dev/null +++ b/tests/test_mcp_repo_autosync.py @@ -0,0 +1,158 @@ +"""Unit tests for the MCP repo autosync worker (refresh_once — pure, no threads).""" + +from mcp_server import repo_autosync +from mcp_server.repo_autosync import autosync_enabled, is_refreshing, refresh_once, start_autosync + + +class _Coll: + def __init__(self, docs): + self.docs = list(docs) + + def find(self, q, projection=None): + return [dict(d) for d in self.docs] + + +class _DB: + def __init__(self, repos): + self._repos = _Coll(repos) + + def __getitem__(self, name): + assert name == "repo_metadata" + return self._repos + + +class _Mirror: + def __init__(self, exists=True, local_sha="abc", fail_fetch=False): + self._exists = exists + self._local_sha = local_sha + self._fail_fetch = fail_fetch + self.calls = [] + + def mirror_exists(self, name): + self.calls.append(("exists", name)) + return self._exists + + def init_mirror(self, url, name): + self.calls.append(("clone", url, name)) + return {"success": True} + + def get_current_sha(self, name, branch): + self.calls.append(("sha", name, branch)) + return self._local_sha + + def fetch_updates(self, name): + self.calls.append(("fetch", name)) + if self._fail_fetch: + return {"success": False, "message": "boom"} + return {"success": True} + + +def _meta(name="alpha", url="https://github.com/o/alpha", sha="abc", branch="main"): + return {"repo_name": name, "repo_url": url, "default_branch": branch, "last_synced_sha": sha} + + +def test_missing_mirror_is_cloned_from_repo_url(): + mirror = _Mirror(exists=False) + stats = refresh_once(_DB([_meta()]), mirror) + assert stats["cloned"] == 1 and stats["errors"] == 0 + assert ("clone", "https://github.com/o/alpha", "alpha") in mirror.calls + + +def test_missing_mirror_without_url_is_skipped(): + mirror = _Mirror(exists=False) + stats = refresh_once(_DB([_meta(url="")]), mirror) + assert stats["skipped"] == 1 + assert all(c[0] != "clone" for c in mirror.calls) + + +def test_equal_shas_skip_fetch(): + mirror = _Mirror(exists=True, local_sha="abc") + stats = refresh_once(_DB([_meta(sha="abc")]), mirror) + assert stats["skipped"] == 1 and stats["fetched"] == 0 + assert all(c[0] != "fetch" for c in mirror.calls) + + +def test_sha_drift_triggers_fetch(): + mirror = _Mirror(exists=True, local_sha="old") + stats = refresh_once(_DB([_meta(sha="new")]), mirror) + assert stats["fetched"] == 1 + assert ("fetch", "alpha") in mirror.calls + + +def test_unknown_sha_triggers_fetch(): + # If either side's SHA is unknown we can't prove freshness — fetch. + mirror = _Mirror(exists=True, local_sha=None) + stats = refresh_once(_DB([_meta(sha="new")]), mirror) + assert stats["fetched"] == 1 + + +def test_errors_are_contained_and_flag_cleared(): + class _Boom(_Mirror): + def mirror_exists(self, name): + raise RuntimeError("disk gone") + + stats = refresh_once(_DB([_meta(), _meta(name="beta")]), _Boom()) + assert stats["errors"] == 2 # both failed, loop survived both + assert is_refreshing("alpha") is False and is_refreshing("beta") is False + + +def test_is_refreshing_true_during_fetch(): + seen = {} + + class _Probe(_Mirror): + def fetch_updates(self, name): + seen["during"] = is_refreshing(name) + return {"success": True} + + refresh_once(_DB([_meta(sha="new")]), _Probe(exists=True, local_sha="old")) + assert seen["during"] is True + assert is_refreshing("alpha") is False # cleared afterwards + + +def test_redact_strips_url_credentials_and_tokens(): + from mcp_server.repo_autosync import _redact + + # userinfo credentials inside a git URL + msg = "Clone failed: fatal: unable to access 'https://x-access-token:ghp_abc123@github.com/o/r'" + out = _redact(msg) + assert "ghp_abc123" not in out and "x-access-token" not in out + assert "https://***@github.com/o/r" in out + # bare GitHub token shapes, outside a URL + assert "***" == _redact("ghp_" + "A" * 36) + assert "github_pat" not in _redact("github_pat_" + "B" * 30) + # safe diagnostics survive + assert _redact("Could not resolve host: github.com") == "Could not resolve host: github.com" + assert _redact(None) == "" + + +def test_clone_failure_log_is_redacted(caplog): + import logging + + class _LeakyMirror(_Mirror): + def init_mirror(self, url, name): + return { + "success": False, + "message": "fatal: 'https://x-access-token:ghp_" + "S" * 36 + "@github.com/o/r'", + } + + with caplog.at_level(logging.WARNING, logger="mcp_server.repo_autosync"): + stats = refresh_once(_DB([_meta()]), _LeakyMirror(exists=False)) + assert stats["errors"] == 1 + joined = " ".join(r.getMessage() for r in caplog.records) + assert "ghp_" not in joined and "x-access-token" not in joined + assert "***" in joined # redaction marker present, diagnostics preserved + + +def test_kill_switch_disables_start(monkeypatch): + monkeypatch.setenv("MCP_REPO_AUTOSYNC", "0") + assert autosync_enabled() is False + assert start_autosync(_DB([])) is False + monkeypatch.setenv("MCP_REPO_AUTOSYNC", "1") + assert autosync_enabled() is True + + +def test_interval_env_floor(monkeypatch): + monkeypatch.setenv("MCP_REPO_AUTOSYNC_INTERVAL", "5") + assert repo_autosync._interval_seconds() == 30 # floored to the minimum + monkeypatch.setenv("MCP_REPO_AUTOSYNC_INTERVAL", "junk") + assert repo_autosync._interval_seconds() == 300 diff --git a/tests/test_mcp_repo_backend.py b/tests/test_mcp_repo_backend.py new file mode 100644 index 000000000..ec0ae9d45 --- /dev/null +++ b/tests/test_mcp_repo_backend.py @@ -0,0 +1,260 @@ +"""Unit tests for RepoBackend (fake mirror/search/db — repo convention).""" + +from mcp_server.repo_backend import SYNC_RETRY_AFTER_SECONDS, RepoBackend + + +class _Cursor: + def __init__(self, docs): + self._docs = list(docs) + + def sort(self, key, direction): + self._docs.sort(key=lambda d: d.get(key), reverse=direction < 0) + return self + + def limit(self, n): + self._docs = self._docs[: int(n)] + return self + + def __iter__(self): + return iter(self._docs) + + +class _Coll: + def __init__(self, docs=None): + self.docs = list(docs or []) + self.indexes = [] + + def create_index(self, *a, **k): + self.indexes.append((a, k)) + return "i" + + def find(self, q, projection=None): + out = [] + for d in self.docs: + if projection: + keep = {k for k, v in projection.items() if v} + out.append({k: v for k, v in d.items() if k in keep}) + else: + out.append(dict(d)) + return _Cursor(out) + + def find_one(self, q): + for d in self.docs: + if all(d.get(k) == v for k, v in q.items()): + return dict(d) + return None + + +class _DB: + def __init__(self, repos=None, jobs=None): + self.c = { + "repo_metadata": _Coll(repos), + "sync_jobs": _Coll(jobs), + } + + def __getitem__(self, name): + return self.c.setdefault(name, _Coll()) + + +class _Mirror: + def __init__(self, files=None, file_result=None): + self.files = files + self.file_result = file_result or {} + self.calls = [] + + def list_all_files(self, repo, ref): + self.calls.append(("list", repo, ref)) + return self.files + + def get_file_at_commit(self, repo, path, commit, **k): + self.calls.append(("get", repo, path, commit)) + return dict(self.file_result) + + +class _Search: + def __init__(self, result=None): + self.result = result or {"results": [], "total": 0} + self.calls = [] + + def search(self, repo, query, **kwargs): + self.calls.append((repo, query, kwargs)) + return dict(self.result) + + +def _repos_db(): + return _DB( + repos=[ + {"repo_name": "beta", "default_branch": "main", "sync_status": "ok", "_id": "x"}, + {"repo_name": "alpha", "default_branch": "master", "sync_status": "ok", "_id": "y"}, + ] + ) + + +def test_init_ensures_repo_metadata_index(): + db = _repos_db() + RepoBackend(db=db, mirror=_Mirror(), search_service=_Search()) + assert db["repo_metadata"].indexes # gap #6: created as part of this phase + args, kwargs = db["repo_metadata"].indexes[0] + assert args[0] == "repo_name" and kwargs.get("unique") is True + + +def test_list_repos_sorted_projected_limited(): + be = RepoBackend(db=_repos_db(), mirror=_Mirror(), search_service=_Search()) + out = be.list_repos(limit=1) + assert out["ok"] is True and out["count"] == 1 + assert out["repos"][0]["repo_name"] == "alpha" # sorted by name + assert "_id" not in out["repos"][0] # projection + + +def test_list_tree_filters_paginates_and_omits_denied(): + files = ["README.md", "src/a.py", "src/b.py", "src/.env", "docs/x.md"] + be = RepoBackend(db=_repos_db(), mirror=_Mirror(files=files), search_service=_Search()) + out = be.list_tree(repo="alpha", path="src", page=1, per_page=1) + assert out["ok"] is True + assert out["total"] == 2 # a.py + b.py; .env omitted by policy + assert out["paths"] == ["src/a.py"] + assert out["ref"] == "refs/heads/master" # default branch resolved from DB + page2 = be.list_tree(repo="alpha", path="src", page=2, per_page=1) + assert page2["paths"] == ["src/b.py"] + + +def test_list_tree_byte_budget_truncates(): + files = [f"dir/file_{i}.py" for i in range(50)] + be = RepoBackend(db=_repos_db(), mirror=_Mirror(files=files), search_service=_Search()) + out = be.list_tree(repo="alpha", page=1, per_page=50, byte_budget=100) + assert out["truncated"] is True + assert 0 < len(out["paths"]) < 50 + + +def test_list_tree_sync_in_progress_during_local_autosync(monkeypatch): + # A failed read while THIS service is cloning/fetching locally must also + # report sync_in_progress (not just the webapp's job queue). + from mcp_server import repo_autosync + + monkeypatch.setattr(repo_autosync, "is_refreshing", lambda name: name == "alpha") + be = RepoBackend(db=_repos_db(), mirror=_Mirror(files=None), search_service=_Search()) + out = be.list_tree(repo="alpha") + assert out["error"] == "sync_in_progress" and out["retry_after"] == SYNC_RETRY_AFTER_SECONDS + + +def test_list_tree_sync_in_progress_when_read_fails_during_sync(): + db = _DB( + repos=[{"repo_name": "alpha", "default_branch": "main"}], + jobs=[{"repo_name": "alpha", "status": "running"}], + ) + be = RepoBackend(db=db, mirror=_Mirror(files=None), search_service=_Search()) + out = be.list_tree(repo="alpha") + assert out["ok"] is False and out["error"] == "sync_in_progress" + assert out["retry_after"] == SYNC_RETRY_AFTER_SECONDS + + +def test_list_tree_not_found_when_no_sync_running(): + be = RepoBackend(db=_repos_db(), mirror=_Mirror(files=None), search_service=_Search()) + out = be.list_tree(repo="alpha") + assert out == {"ok": False, "error": "repo_or_ref_not_found"} + + +def test_get_file_ok_envelope(): + res = { + "success": True, + "is_binary": False, + "content": "print(1)", + "encoding": "utf-8", + "size": 8, + "lines": 1, + "file_path": "src/a.py", + "resolved_commit": "abc123", + } + be = RepoBackend(db=_repos_db(), mirror=_Mirror(file_result=res), search_service=_Search()) + out = be.get_file(repo="alpha", path="src/a.py") + assert out["ok"] is True and out["status"] == "ok" + assert out["content"] == "print(1)" + assert out["file"]["resolved_commit"] == "abc123" and out["file"]["lines"] == 1 + + +def test_get_file_binary_envelope_has_no_content(): + res = {"success": True, "is_binary": True, "content": None, "size": 10, "file_path": "a.png"} + be = RepoBackend(db=_repos_db(), mirror=_Mirror(file_result=res), search_service=_Search()) + out = be.get_file(repo="alpha", path="a.png") + assert out["ok"] is True and out["status"] == "binary" + assert "content" not in out + + +def test_get_file_too_large_envelope(): + res = {"error": "file_too_large", "size": 900_000, "max_size": 512_000} + be = RepoBackend(db=_repos_db(), mirror=_Mirror(file_result=res), search_service=_Search()) + out = be.get_file(repo="alpha", path="big.txt") + assert out["ok"] is True and out["status"] == "too_large" + assert out["max"] == 512_000 and "content" not in out + + +def test_get_file_not_found_and_denied(): + res = {"error": "file_not_in_commit"} + be = RepoBackend(db=_repos_db(), mirror=_Mirror(file_result=res), search_service=_Search()) + assert be.get_file(repo="alpha", path="nope.py") == {"ok": False, "error": "not_found"} + # policy blocks BEFORE touching the mirror + mirror = _Mirror(file_result={"success": True}) + be2 = RepoBackend(db=_repos_db(), mirror=mirror, search_service=_Search()) + assert be2.get_file(repo="alpha", path=".env") == {"ok": False, "error": "path_denied"} + assert mirror.calls == [] + + +def test_get_file_sync_in_progress_instead_of_not_found(): + db = _DB( + repos=[{"repo_name": "alpha", "default_branch": "main"}], + jobs=[{"repo_name": "alpha", "status": "running"}], + ) + res = {"error": "invalid_commit"} # ref unresolvable mid-fetch + be = RepoBackend(db=db, mirror=_Mirror(file_result=res), search_service=_Search()) + out = be.get_file(repo="alpha", path="a.py") + assert out["error"] == "sync_in_progress" and out["retry_after"] > 0 + + +def test_search_caps_filters_and_snippets(): + rows = [{"path": f"f{i}.py", "line": i, "content": "x" * 600} for i in range(10)] + rows.append({"path": ".env", "line": 1, "content": "SECRET=1"}) + be = RepoBackend( + db=_repos_db(), + mirror=_Mirror(), + search_service=_Search({"results": rows, "total": 11}), + ) + out = be.search(repo="alpha", query="x", max_results=5) + assert out["ok"] is True and out["count"] == 5 # capped to max_results + assert out["total"] == 10 # policy-filtered availability (11 minus .env), NOT engine total + assert out["truncated"] is True # allowed matches exist beyond the cap + assert all(len(r["snippet"]) <= 500 for r in out["results"]) # snippet cap + assert all(r["path"] != ".env" for r in out["results"]) # policy skip + + +def test_search_not_truncated_when_under_cap(): + rows = [{"path": "a.py", "line": 1, "content": "x"}] + be = RepoBackend( + db=_repos_db(), mirror=_Mirror(), search_service=_Search({"results": rows, "total": 1}) + ) + out = be.search(repo="alpha", query="x", max_results=5) + assert out["total"] == 1 and out["count"] == 1 and out["truncated"] is False + + +def test_list_tree_normalizes_bad_page_inputs(): + files = ["a.py", "b.py", "c.py"] + be = RepoBackend(db=_repos_db(), mirror=_Mirror(files=files), search_service=_Search()) + # Non-numeric / negative inputs must not crash or slice negatively. + out = be.list_tree(repo="alpha", page="junk", per_page=-7) + assert out["ok"] is True + assert out["page"] == 1 and out["per_page"] == 1 # normalized values echoed back + assert out["paths"] == ["a.py"] + out2 = be.list_tree(repo="alpha", page=-3, per_page="junk") + assert out2["page"] == 1 and out2["per_page"] == 200 + assert out2["paths"] == files + + +def test_search_error_maps_to_transient_check(): + db = _DB( + repos=[{"repo_name": "alpha", "default_branch": "main"}], + jobs=[{"repo_name": "alpha", "status": "running"}], + ) + be = RepoBackend( + db=db, mirror=_Mirror(), search_service=_Search({"error": "boom", "results": []}) + ) + out = be.search(repo="alpha", query="xy") + assert out["error"] == "sync_in_progress" and out["retry_after"] > 0 diff --git a/tests/test_mcp_repo_handlers.py b/tests/test_mcp_repo_handlers.py new file mode 100644 index 000000000..70518eba8 --- /dev/null +++ b/tests/test_mcp_repo_handlers.py @@ -0,0 +1,68 @@ +"""Unit tests for the repo-browser handlers (clamps + validation, pure).""" + +from mcp_server import repo_handlers as rh + + +class _RecordingRepoBackend: + def __init__(self): + self.calls = [] + + def list_repos(self, *, limit): + self.calls.append(("repos", limit)) + return {"ok": True} + + def list_tree(self, *, repo, path, ref, page, per_page, byte_budget): + self.calls.append(("tree", repo, path, ref, page, per_page, byte_budget)) + return {"ok": True} + + def get_file(self, *, repo, path, ref): + self.calls.append(("get", repo, path, ref)) + return {"ok": True} + + def search(self, *, repo, query, file_pattern, max_results, byte_budget): + self.calls.append(("search", repo, query, file_pattern, max_results, byte_budget)) + return {"ok": True} + + +def test_list_repos_clamps_limit(): + be = _RecordingRepoBackend() + rh.list_repos(be, limit=10_000) + assert be.calls[0] == ("repos", rh.REPOS_LIMIT_MAX) + rh.list_repos(be, limit="junk") + assert be.calls[1] == ("repos", rh.REPOS_LIMIT_DEFAULT) # invalid -> default + + +def test_tree_requires_repo_and_clamps(): + be = _RecordingRepoBackend() + assert rh.list_repo_tree(be, repo=" ") == {"ok": False, "error": "missing_repo"} + assert be.calls == [] + rh.list_repo_tree(be, repo="r", page=0, per_page=99_999) + call = be.calls[0] + assert call[0] == "tree" and call[4] == 1 # page floored + assert call[5] == rh.TREE_PER_PAGE_MAX # per_page capped + assert call[6] == rh.OUTPUT_BYTE_BUDGET # budget always passed + + +def test_tree_defaults(): + be = _RecordingRepoBackend() + rh.list_repo_tree(be, repo="r") + assert be.calls[0][5] == rh.TREE_PER_PAGE_DEFAULT + + +def test_get_file_requires_repo_and_path(): + be = _RecordingRepoBackend() + assert rh.get_repo_file(be, repo="", path="a.py") == {"ok": False, "error": "missing_repo"} + assert rh.get_repo_file(be, repo="r", path=" ") == {"ok": False, "error": "missing_path"} + assert be.calls == [] + rh.get_repo_file(be, repo="r", path=" a.py ", ref=" ") + assert be.calls[0] == ("get", "r", "a.py", None) # trimmed; blank ref -> None + + +def test_search_validates_query_and_clamps(): + be = _RecordingRepoBackend() + assert rh.search_repo(be, repo="r", query="x") == {"ok": False, "error": "query_too_short"} + assert be.calls == [] + rh.search_repo(be, repo="r", query="xy", max_results=5_000) + call = be.calls[0] + assert call[4] == rh.SEARCH_RESULTS_MAX # capped + assert call[5] == rh.OUTPUT_BYTE_BUDGET diff --git a/tests/test_mcp_repo_policy.py b/tests/test_mcp_repo_policy.py new file mode 100644 index 000000000..3be04240a --- /dev/null +++ b/tests/test_mcp_repo_policy.py @@ -0,0 +1,80 @@ +"""Unit tests for the repo secrets-path policy (mandatory, fail-closed).""" + +import pytest + +from mcp_server import repo_policy +from mcp_server.repo_policy import is_denied + + +@pytest.mark.parametrize( + "path", + [ + ".env", + ".env.local", + "config/.env", # nested + "a/b/.env.production", + ".ENV", # case variant + "certs/server.pem", + "keys/private.key", + "id_rsa", + ".ssh/id_rsa.pub", + "deploy/id_ed25519", + "secrets.yaml", + "conf/secrets.json", + "credentials.json", + "gcp/credentials_prod.yaml", + "store.p12", + "win/cert.pfx", + ".netrc", + ".npmrc", + "android/release.keystore", + ], +) +def test_denied_paths(path): + assert is_denied(path) is True + + +@pytest.mark.parametrize( + "path", + [ + "main.py", + "src/app/utils.py", + "envelope.py", # starts with 'env' but no leading dot + "environment.md", + "key_utils.py", # 'key' not as extension + "docs/keys.md", + "secrets_test.py", # 'secrets_' != 'secrets.' + "tests/test_secrets_handling.py", + "README.md", + "config/settings.yaml", + ], +) +def test_allowed_paths(path): + assert is_denied(path) is False + + +def test_empty_and_traversal_denied(): + assert is_denied("") is True + assert is_denied(None) is True + assert is_denied("..") is True + + +def test_windows_separators_normalized(): + assert is_denied("config\\.env") is True + + +def test_extra_patterns_from_env(monkeypatch): + monkeypatch.setenv("MCP_REPO_DENYLIST_EXTRA", "*.sqlite, private/*") + assert is_denied("data/app.sqlite") is True + assert is_denied("private/notes.md") is True + monkeypatch.delenv("MCP_REPO_DENYLIST_EXTRA") + assert is_denied("data/app.sqlite") is False + + +def test_fail_closed_on_internal_error(monkeypatch): + # If pattern evaluation blows up, the path must be treated as denied. + def _boom(*a, **k): + raise RuntimeError("policy unavailable") + + monkeypatch.setattr(repo_policy.fnmatch, "fnmatchcase", _boom) + assert is_denied("main.py") is True diff --git a/tests/test_mcp_require_admin.py b/tests/test_mcp_require_admin.py new file mode 100644 index 000000000..a00222d95 --- /dev/null +++ b/tests/test_mcp_require_admin.py @@ -0,0 +1,78 @@ +"""Unit tests for the admin gate (require_admin / is_admin_user) — fail-closed.""" + +import pytest + +pytest.importorskip("mcp") + +import mcp.server.auth.middleware.auth_context as ac # noqa: E402 + +from mcp_server import auth as mcp_auth # noqa: E402 +from mcp_server.auth import is_admin_user, require_admin # noqa: E402 + + +class _Token: + def __init__(self, subject, scopes=("read",)): + self.subject = subject + self.scopes = list(scopes) + + +def _set_admins(monkeypatch, ids): + # The canonical source is config.ADMIN_USER_IDS — patch the resolver so the + # tests don't depend on env parsing. + monkeypatch.setattr(mcp_auth, "admin_user_ids", lambda: set(ids)) + + +def test_admin_allowed(monkeypatch): + _set_admins(monkeypatch, {42}) + monkeypatch.setattr(ac, "get_access_token", lambda: _Token("42")) + assert require_admin(None) == 42 + + +def test_non_admin_denied(monkeypatch): + _set_admins(monkeypatch, {42}) + monkeypatch.setattr(ac, "get_access_token", lambda: _Token("7")) + with pytest.raises(PermissionError): + require_admin(None) + + +def test_empty_admin_list_denies_everyone(monkeypatch): + # fail-closed: no admins configured => nobody is admin. + _set_admins(monkeypatch, set()) + monkeypatch.setattr(ac, "get_access_token", lambda: _Token("42")) + with pytest.raises(PermissionError): + require_admin(None) + + +def test_unauthenticated_denied(monkeypatch): + _set_admins(monkeypatch, {42}) + monkeypatch.setattr(ac, "get_access_token", lambda: None) + with pytest.raises(PermissionError): + require_admin(None) # no token, no request state + + +def test_chatops_escape_hatch_is_ignored(monkeypatch): + # chatops' CHATOPS_ALLOW_ALL_IF_NO_ADMINS must have NO effect here: with an + # empty admin set, everyone stays denied even when the hatch env is set. + monkeypatch.setenv("CHATOPS_ALLOW_ALL_IF_NO_ADMINS", "1") + _set_admins(monkeypatch, set()) + assert is_admin_user(42) is False + + +def test_is_admin_user_bad_input(): + assert is_admin_user("not-a-number") is False + assert is_admin_user(None) is False + + +def test_admin_user_ids_fail_closed(monkeypatch): + # If the config layer is unavailable, the set must be empty (deny), not crash. + import builtins + + real_import = builtins.__import__ + + def _no_config(name, *a, **k): + if name == "config": + raise RuntimeError("config unavailable") + return real_import(name, *a, **k) + + monkeypatch.setattr(builtins, "__import__", _no_config) + assert mcp_auth.admin_user_ids() == set() diff --git a/tests/test_mcp_server_build.py b/tests/test_mcp_server_build.py index e7cc15476..2dcb87aa1 100644 --- a/tests/test_mcp_server_build.py +++ b/tests/test_mcp_server_build.py @@ -50,6 +50,20 @@ def verify(self, token): return None +class _FakeRepoBackend: + def list_repos(self, **k): + return {"ok": True} + + def list_tree(self, **k): + return {"ok": True} + + def get_file(self, **k): + return {"ok": True} + + def search(self, **k): + return {"ok": True} + + async def test_all_tools_are_registered(): mcp = build_mcp(_FakeBackend()) tools = await mcp.list_tools() @@ -57,6 +71,34 @@ async def test_all_tools_are_registered(): assert _EXPECTED_TOOLS <= names +async def test_repo_tools_hidden_from_non_admin_tools_list(): + from mcp_server.server import _ADMIN_TOOLS + + mcp = build_mcp(_FakeBackend(), repo_backend=_FakeRepoBackend()) + # Outside a request there is no auth context => fail-closed non-admin view. + names = {t.name for t in await mcp.list_tools()} + assert _EXPECTED_TOOLS <= names + assert not (names & _ADMIN_TOOLS) + + +async def test_repo_tools_visible_to_admin(): + from mcp_server.server import _ADMIN_TOOLS + + mcp = build_mcp(_FakeBackend(), repo_backend=_FakeRepoBackend()) + mcp._request_is_admin = lambda: True # simulate a verified admin request + names = {t.name for t in await mcp.list_tools()} + assert _ADMIN_TOOLS <= names + + +async def test_no_repo_backend_registers_no_repo_tools(): + from mcp_server.server import _ADMIN_TOOLS + + mcp = build_mcp(_FakeBackend()) # repo_backend omitted + mcp._request_is_admin = lambda: True + names = {t.name for t in await mcp.list_tools()} + assert not (names & _ADMIN_TOOLS) + + def test_build_app_exposes_healthz_route(): app = build_app(_FakeBackend(), _FakeStore()) paths = {getattr(r, "path", None) for r in app.routes}