diff --git a/docs/mcp-server.rst b/docs/mcp-server.rst index e8948f355..288684444 100644 --- a/docs/mcp-server.rst +++ b/docs/mcp-server.rst @@ -115,8 +115,9 @@ usable=8)`` מול ``cpu quota cgroup v2: 0.50 cpu``, כלומר 12 חוטים המספרים מחדש **לשני הפרסרים** — מריצים אותו כשאחד מהם משתנה. - **ההחזקה הגדולה ביותר של חוט קריאה אינה הפרסור, וזו החלטה מתועדת.** כלי האדמין ``codekeeper_get_repo_file`` עם ``outline=true`` או ``lines=`` קורא עד - ``RANGE_READ_MAX_BYTES`` (‏10MiB), והמראה טוענת את ה-blob כולו לזיכרון - **לפני** בדיקת הגודל. נמדד (2026-09-20): outline של קובץ RST בגודל 10MB + ``RANGE_READ_MAX_BYTES`` (‏10MiB), והמראה טוענת blob **שמתחת לתקרה** כולו + לזיכרון לפני הפענוח והחיתוך (blob שמעליה נדחה מ-``git cat-file -s`` בלי + להיקרא — ראו #3433 למטה). נמדד (2026-09-20): outline של קובץ RST בגודל 10MB מחזיק 61–77MiB לאורך המסלול כולו — הטקסט של 10MB בזיכרון ועליו הפרסור; שורת ``outline_densest_real`` של הסקריפט מראה את הפרסור לבדו מעל השיא שלפניו, 51MiB — וקריאת ``lines=`` של ``webapp/static/js/md_preview.bundle.js`` @@ -128,7 +129,10 @@ usable=8)`` מול ``cpu quota cgroup v2: 0.50 cpu``, כלומר 12 חוטים (הגדול ביותר כאן 169KB). תמחור לפי ``RANGE_READ_MAX_BYTES`` היה מצמצם את המסלול הציבורי ל-4 חוטים (לפי 77MiB) או ל-9 (לפי 37MiB) בגלל צורה שאין לה מקור. מה שכן השתנה בגלל זה הוא התקרה (למטה); והשורש — לחסום את הקריאה - במקור, בבדיקת גודל לפני ``git show`` — נמצא ב-#3433. + במקור — נסגר ב-#3433: ``get_file_at_commit`` שואל את מאגר האובייקטים לגודל + (``git cat-file -s :``) לפני ``git show``, ולכן ``max_size`` הוא + חסם על מה שנקרא ולא בדיקה בדיעבד. נמדד: קובץ של 12MB שנדחה עלה 20MiB שיא + לפני, ואפס אחרי; קובץ של 7MB שמתחת לתקרה נשאר 14MiB, כי אותו כן קוראים. - **רצפה** (``_READ_POOL_FLOOR``, ‏2): כדי שקריאת מונגו תקועה אחת לא תשבית את השאר. שניים הוא הגודל הקטן ביותר עם התכונה הזאת, וכל יחידה מעליו היא זיכרון שהחשבון אמר שאין. **תקרה** (``_READ_POOL_CAP``, ‏12): הרוחב שהייצור diff --git a/docs/whats-new.rst b/docs/whats-new.rst index ef65c8cd0..86a14edff 100644 --- a/docs/whats-new.rst +++ b/docs/whats-new.rst @@ -4,6 +4,7 @@ What's New 2026-09-20 ---------- +- **fix (mirror): ‏``get_file_at_commit`` בודק את גודל הקובץ במאגר האובייקטים לפני ``git show``, ולכן קובץ מעל התקרה אינו נטען לזיכרון כלל (#3433, פריט 9).** עד היום ``git show`` נטען כולו לזיכרון ורק אז ``max_size`` נבדק — כלומר התקרה הייתה בדיקה בדיעבד: קובץ של 12MB שנדחה עלה 20MiB שיא בחוט הקריאה לפני שהתשובה הייתה ``file_too_large``. מעכשיו ``git cat-file -s :`` מחזיר את גודל האובייקט בלי לקרוא אותו, שתי הפקודות פונות לאותו קומיט שנפתר פעם אחת (אין חלון לסנכרון להיכנס בו), וקובץ מעל התקרה נדחה לפני שנקרא בית אחד — אותה תשובה, אותו ``size``, אפס זיכרון. כשל בבדיקת הגודל אינו נופל לקריאה בלי תקרה, ומה שמוחזר עדיין נבדק מול התקרה בנפרד, לכל סוג אובייקט. נמדד: 12MB מעל תקרה של 500KB ושל 10MiB — 20.2 ו-20.4MiB לפני, אפס אחרי; 7MB מתחת לתקרה — 14MiB לפני ואחרי, כי אותו כן קוראים. ראו "מודל הריצה של הכלים" ב-:doc:`mcp-server`. - **feat: ``codekeeper_docs_get_section`` קורא גם קובצי Markdown, ולכל ריפו מדיניות נתיבים משלו.** עד היום הכלי ידע לקרוא ``.rst`` בלבד, ורק מתחת ל-``docs/``. הפארסר של Markdown נכתב ונמדד כבר קודם — ואף אחד לא קרא לו; זה השינוי שמחווט אותו. מעכשיו הסיומת בוחרת את הפארסר, ולכל ריפו מוצהר **איפה התיעוד שלו יושב ובאיזה פורמט**: ``CodeBot`` נשאר ``docs/`` עם ``.rst`` ללא שינוי, ו-``amir-bug-patterns`` מגיש ``.md`` משורש הריפו. **וזו הרחבה מכוונת של גבול הקריאה** — הכלי ציבורי, כלומר כל משתמש מאומת מפעיל אותו בלי אדמין, והריפו שנוסף ציבורי ב-GitHub. מה ש**לא** השתנה: מדיניות סינון הסודות חלה על המסלול הזה במלואה, ולכן ``path="secrets"`` מחזיר ``path_denied`` ולא ``not_found``, וגם ``secrets.md`` ו-``credentials.md`` חסומים; ותקרת 500KB של שירות המראה נשארת ההגנה על הפרסור, כי הכלי קורא את הקובץ כולו בלי ``lines``. **שני שערים ולא אחד:** ``MCP_DOCS_REPO`` מתיר בזמן ריצה, טבלת המדיניות בקוד יודעת לקרוא, וריפו שנמצא במשתנה הסביבה ואין לו מדיניות נדחה ב-``repo_not_configured`` במקום ליפול לברירת מחדל מתירנית. **ובקשה לפורמט שהריפו אינו מגיש נדחית בשמה** — ``suffix_not_allowed`` עם רשימת הסיומות שכן מוגשות, במקום להשלים סיומת בשקט מעל הקיימת ולחזור כ"לא נמצא" על קובץ שקיים. שני סירובים חדשים מגיעים מהפרסור עצמו: ``inconsistent_line_endings`` על ``\r`` בודד (CRLF עובד במלואו), ו-``too_many_sections`` על קובץ שחוצה את תקרת 50,000 הכותרות — Markdown דרך ברירת המחדל של הפרסר, ו-RST דרך התקרה שהכלי מעביר מאז #3429; סירוב שהפארסר מספק לו מספר שורה נושא גם ``line`` — תמיד ב-Markdown, ולא ב-``too_many_sections`` של RST, כי ``rst_parser`` מרים אותה בלי מספר. ``includes`` הוא שדה של RST ולכן בעמוד ``.md`` הוא תמיד ריק. **ומסלול ה-RST לא זז**: אפס דיף מדוד על 5,920 רשומות מ-208 קובצי RST, לפני ואחרי. ראו :ref:`mcp-docs-path-policy`. - **fix: מאגר הקריאות של ה-MCP נגזר ממכסת הזיכרון של הקונטיינר, ולא ממספר המעבדים של המארח (#3391).** כלי הקריאה רצים ב-``asyncio.to_thread``, כלומר ב-executor ברירת המחדל של הלולאה, שגודלו ``min(32, os.cpu_count() + 4)`` — ו-``os.cpu_count()`` בקונטיינר הוא מספר הליבות של **המארח**, כפי ש-CPython מתעד במפורש. שורת הקיבולת מהייצור אמרה את זה במספרים: ``read pool 12 threads (os.cpu_count=8, usable=8)`` על מכסה של ``0.50 cpu``. לקריאת מונגו זה לא מזיק; לפרסור בפייתון טהור אין מקביליות ב-CPU בגלל ה-GIL, ולכן 12 חוטים היו 12 עצי פרסור בזיכרון בו-זמנית — יותר ממה שנכנס ב-512MiB — ורגע לפני שהפרסור הופך לכלי ציבורי. מעכשיו ``attach_read_pool`` מתקין בעליית ה-lifespan של אפליקציית ה-ASGI מאגר משלו, בגודל ``(מגבלת הזיכרון − קו בסיס − שוליים) ÷ עלות פרסור אחד``: המגבלה נקראת מ-cgroup בזמן ריצה (``memory.max`` ב-v2, ‏``memory.limit_in_bytes`` ב-v1) ולא מקובעת בקוד, קו הבסיס והשוליים הם מדידות מהשירות בייצור עם תאריך, ועלות הפרסור נגזרת מ-``MAX_FILE_SIZE_FOR_DISPLAY`` כי הכלי דוחה ``too_large`` לפני שהוא מפרסר. על תוכנית הייצור זה **10** חוטים במקום 12 — עלות הפרסור נמדדה מחדש על הפרסר הנעוץ (``scripts/measure_md_parse_cost.py``; הקבוע נמדד על ``services.md_parser`` כהכנה לכלי ה-Markdown של PR 5, בעוד הכלי הציבורי היום מפרסר RST ב-``services.rst_parser``, שעולה כ-6 בתים לבית גם על העמוד הצפוף ביותר — והסקריפט מודד את שניהם), והיא תלויה בצפיפות הטוקנים של המסמך ולא בגודלו: המסמך הצפוף ביותר בקורפוס הוא הקבוע, וצורה עוינת של שורות-תבליט בודדות עולה פי ארבעה ממנו — מה ששום רוחב מאגר אינו סופג, ומחכה לתקרת טוקנים בפרסר עצמו. **ובעקבות הסקירה:** ``codekeeper_docs_get_section`` מעביר לפרסר את תקרת הסקשנים של האאוטליין (50,000) ומחזיר ``too_many_sections`` במקום לפרסר את הקובץ כולו — צורה עוינת של RST (כותרת בת תו אחד בכל שורה) עלתה 44.0MiB לפרסור, יותר מ-35.2MiB שהמאגר מקצה לחוט, ועכשיו נעצרת ב-20.1MiB; אף עמוד אמיתי אינו מתקרב לתקרה. רצפה של 2 (קריאה תקועה אחת לא משביתה את השאר), תקרה של 12, הרוחב שהייצור כבר הריץ (מעבר לתוכנית גדולה לא מרחיב את המאגר בלי החלטה; הגרסה הראשונה נקבה ב-32, והסקירה של #3429 הורידה אותה כי המחלק מתאר את הפרסור ולא את קריאות הטווח של האדמין; חסימת הקריאה במקור — #3433), וכשאין מגבלה קריאה — הרצפה, **לא** ``cpu_count + 4``, עם WARNING שאומר שזה קרה. **ושורת הקיבולת מפסיקה לחשב בעצמה:** היא נרשמת מתוך ה-lifespan, קוראת את ``_max_workers`` של המאגר שהותקן בפועל, ומדווחת גם את מגבלת הזיכרון לצד מכסת ה-CPU — כי רשומה שמתארת מצב ומתעדכנת בנפרד ממנו היא בדיוק מה שמשקר בשקט. ה-seam הוא עטיפת ``router.lifespan_context``, אותו seam שניקוז ה-PostHog כבר משתמש בו, כי ``FastMCP.streamable_http_app()`` בונה את Starlette עם ``lifespan=`` משלה ו-``on_startup`` אינו רץ. הטסט המרכזי נכנס ל-lifespan של האפליקציה האמיתית ומוכיח שקריאת ``to_thread`` נוחתת על חוט של המאגר החדש — לא שפונקציה נקראה; שורת הקיבולת נבדקת בתת-תהליך נקי דרך הפלט האמיתי ולא דרך ``caplog``. ראו "מודל הריצה של הכלים" ב-:doc:`mcp-server`. - **feat: ``codekeeper_docs_get_section`` מוצא סעיף גם לפי המזהה שלו, ולא רק לפי השם המלא.** הסוכנים מפנים זה לזה לפי מזהה — "קרא K11", "ראו U3" — ועד היום ``section="K11"`` החזיר ``section_not_found`` עם רשימת הצעות **ריקה**, כי ההתאמה הייתה שוויון מלא ו-``difflib`` עם ``cutoff=0.5`` אינו מוצא קרבה בין שלושה תווים לכותרת עברית ארוכה. כלומר הזרימה המרכזית של הכלי לא עבדה. מעכשיו, **ורק אחרי שהשוויון המלא לא מצא כלום**, שאילתה שבנויה כמזהה (עד שלוש אותיות ואחריהן עד שלוש ספרות, עם נקודה אופציונלית) מותאמת לכותרת שנפתחת באותו מזהה. **והמזהה נקרא כיחידה ולא כקידומת של מחרוזת**, ולכן ``K1`` מחזיר את ``K1`` בלבד ולעולם לא את ``K10`` עד ``K15`` — הצורה השבורה של הבדיקה הזאת היא אותה מחלקה בדיוק שבה גבול נתיב נבדק כרצף תווים. שאילתה שאינה בנויה כמזהה אינה עוברת במסלול הזה כלל, ולכן ``section="איך"`` ממשיך להחזיר אפס התאמות גם בעמוד שיש בו חמש-עשרה כותרות שמתחילות במילה הזאת. מזהה שחוזר פעמיים הוא ``ambiguous_section`` עם מועמדים, כמו כל כותרת כפולה, ושאילתת מזהה שלא נמצאה מקבלת ב-``suggestions`` קודם כול כותרת קרובה, אם יש כזו בעמוד, **ורק כשאין אף כותרת קרובה** את המזהים שכן קיימים — עד 50, ועם ``suggestions_truncated`` כשהיו עוד. **והתקרה גבוהה בכוונה**, כי רשימת מזהים אינה מדורגת: לשאלה "אלה שכן קיימים" אין תשובה "החמש הטובות", וחיתוך שלה מסתיר פריטים בלי קריטריון ובלי דרך לבקש את השאר. **והדגל הזה אומר את אותו דבר בשני המסלולים**: גם רשימת מזהים שנחתכה וגם רשימת כותרות קרובות שנחתכה מדליקות אותו. שדה בשם ``truncated`` שמשמעותו "נחתך" במסלול אחד ו"שקט" בשני הוא בדיוק ההפתעה שאנחנו מנסים למנוע. **ונקודה עוד יותר קלה לפספס: הנקודה שפותחת תת-מספור אינה גבול.** בכותרת ``K11. טקסט`` הנקודה **מסיימת** את המזהה, ובכותרת ``K11.1 טקסט`` היא **מפרידה** בתוך מזהה ארוך יותר — ולכן ``K11`` לעולם אינו מחזיר את ``K11.1``, וכותרת בתת-מספור נגישה בשמה המלא בלבד. זו אותה מחלקה בדיוק כמו ``K1`` שתופס את ``K10``: גבול שנבדק כתו בודד, בלי לשאול מה בא אחריו. **ההתנהגות על התיעוד כאן לא זזה, וזה נמדד ולא הונח:** הפלט של הכלי על כל קובצי ה-RST — כל כותרת, בכל צורותיה — זהה בית-בית לפני ואחרי. וההנחה שהכול נשען עליה, שאף כותרת בעמודים כאן אינה נפתחת במזהה, **אינה ספירה שנרשמה פעם אחת אלא טסט שרץ בכל CI**: ``tests/test_docs_headings_carry_no_identifier.py`` סורק את כל העמודים ונופל על כותרת שנפתחת במזהה, עם דוגמת autodoc אמיתית שמוכיחה שהוא מסוגל ליפול. **ומה שלא נבדק בקורפוס הזה נבדק בבקרה סינתטית**, כי סוללה שכל תשובותיה "לא נמצא" אינה מבדילה בין ענף כבוי לבין פרובים עיוורים. שני הדברים שמפתיעים קורא — קיצור המזהה, וכותרת שנושאת בקטיקים ולכן דורשת אותם בשאילתה — עברו לתיאור הפרמטר ``section``, שאינו נחתך אצל הלקוח כמו תיאור הכלי. ראו :ref:`mcp-docs-section`. diff --git a/mcp_server/repo_backend.py b/mcp_server/repo_backend.py index 4eb348f4c..c9e82cd66 100644 --- a/mcp_server/repo_backend.py +++ b/mcp_server/repo_backend.py @@ -38,11 +38,13 @@ #: ``too_large`` עם הפרמטר בדיוק כמו בלעדיו (#3317). ``webapp/app.py`` בן #: 805,594 הבייטים, הקובץ שהכי הרבה עובדים עליו, היה בלתי קריא דרך הכלי. #: -#: **למה תקרה אחרת ולא ביטול.** הבלוב עדיין נקרא ומפוענח במלואו לפני +#: **למה תקרה אחרת ולא ביטול.** בלוב שמתחת לתקרה נקרא ומפוענח במלואו לפני #: החיתוך, אז "בלי תקרה" פירושו שקובץ פתולוגי בריפו יגיע ל-RAM כמו שהוא. #: נמדד: קריאה ופענוח צורכים כפי שלושה מגודל הקובץ — 6.6MB הגיעו ל-35.7MB #: שיא. תקרה של 10MB חוסמת את זה בערך ב-30MB, ונותנת פי 12 מרווח מעל -#: הקובץ הגדול ביותר שבאמת קוראים. +#: הקובץ הגדול ביותר שבאמת קוראים. ומאז #3433 התקרה נבדקת מול גודל +#: האובייקט במאגר **לפני** ``git show``, ולכן קובץ שמעליה אינו נטען כלל — +#: החסם הוא על מה שנקרא, לא בדיקה בדיעבד. #: #: זו החלטת מדיניות של שכבת ה-MCP, לא של שירות המראה — ולכן היא כאן ולא #: שם, ואינה מייתרת את 500KB שממשיכה לחול על קריאה מלאה ועל הוובאפ. diff --git a/mcp_server/server.py b/mcp_server/server.py index ee9094438..3c383025f 100644 --- a/mcp_server/server.py +++ b/mcp_server/server.py @@ -750,7 +750,10 @@ def _memory_limit() -> tuple[int | None, str]: #: Pricing by ``RANGE_READ_MAX_BYTES`` would have cut the public path to 4 #: threads (at 77MiB) or 9 (at 37MiB) for a shape without a source. What did #: change because of this is :data:`_READ_POOL_CAP`; the root fix — bounding -#: the read at its source with a size probe before ``git show`` — is #3433. +#: the read at its source with a size probe before ``git show`` (#3433) — has +#: landed in ``get_file_at_commit``: a blob over ``max_size`` is refused from +#: ``git cat-file -s`` without being read (12MB: 20MiB peak before, 0 after, +#: measured), so the hold above is now only what a file *under* the cap costs. _PARSE_COST_BYTES = _PARSE_RSS_PER_INPUT_BYTE * MAX_FILE_SIZE_FOR_DISPLAY diff --git a/services/git_mirror_service.py b/services/git_mirror_service.py index 12654aa1b..99856ee65 100644 --- a/services/git_mirror_service.py +++ b/services/git_mirror_service.py @@ -1573,6 +1573,41 @@ def get_file_history( self.logger.error(f"Error getting file history: {e}") return {"error": "internal_error", "message": "שגיאה פנימית"} + def _object_size( + self, mirror_path: Path, resolved_commit: str, safe_file_path: str, timeout: float = 10 + ) -> Dict[str, Any]: + """גודל האובייקט ``:`` מהמאגר, בלי לקרוא את תוכנו — ``git cat-file -s``. + + מחזיר ``{"size": int}`` או ``{"error": ..., "message": ...}`` באותה מפה + של ``git show`` (:meth:`_object_read_error`), כי git 2.43 מדפיס את + **אותן** הודעות לשתי הפקודות על נתיב חסר (``path 'x' does not exist + in ''``) ועל קומיט שאינו מכיל אותו (``exists on disk, but not + in``) — נמדד, לא הונח. הפלט הוא קלט חיצוני (U3): מספר שאינו מתפענח + הוא כשל ולא אפס, כדי שלא ייקרא כ"קטן מהתקרה". ``TimeoutExpired`` עולה + לקורא, שכבר ממפה אותה ל-``timeout``. + """ + result = subprocess.run( + ["git", "-C", str(mirror_path), "cat-file", "-s", f"{resolved_commit}:{safe_file_path}"], + capture_output=True, + timeout=timeout, + ) + if result.returncode != 0: + return self._object_read_error(result.stderr) + try: + return {"size": int(result.stdout.decode("ascii", errors="replace").strip())} + except ValueError: + self.logger.warning("git cat-file -s returned a non-numeric size for %s", safe_file_path) + return {"error": "git_error", "message": "git לא החזיר גודל אובייקט תקין"} + + def _object_read_error(self, stderr_bytes: bytes) -> Dict[str, Any]: + """ממפה stderr של ``git show`` ושל ``git cat-file`` לתשובת שגיאה — הגדרה אחת לשתיהן.""" + stderr = self._sanitize_output(stderr_bytes.decode('utf-8', errors='replace')) + if "does not exist" in stderr or "exists on disk" in stderr: + return {"error": "file_not_in_commit", "message": "הקובץ לא קיים ב-commit זה"} + if "unknown revision" in stderr or "bad revision" in stderr: + return {"error": "invalid_commit", "message": "Commit לא נמצא"} + return {"error": "git_error", "message": stderr} + def get_file_at_commit( self, repo_name: str, @@ -1587,7 +1622,10 @@ def get_file_at_commit( repo_name: שם הריפו file_path: נתיב הקובץ commit: Hash, tag, branch או expression - max_size: גודל מקסימלי להחזרה (bytes) + max_size: תקרה בבתים. **נבדקת מול גודל האובייקט במאגר לפני + ``git show``** (#3433), ולכן קובץ גדול ממנה אינו נטען לזיכרון + כלל — ``max_size`` הוא חסם על מה שנקרא, לא בדיקה בדיעבד. ונבדקת + שוב על מה שמוחזר, כדי שהחסם יחול על כל סוג אובייקט. Returns: Dict עם תוכן הקובץ או שגיאה @@ -1615,6 +1653,29 @@ def get_file_at_commit( resolved_commit = ref_validation["resolved_sha"] try: + # **הגודל נבדק מול מאגר האובייקטים לפני שנקרא בית אחד של תוכן (#3433).** + # עד כאן ``git show`` נטען כולו לזיכרון (``capture_output=True``) ורק + # אז ``max_size`` נבדק — כלומר התקרה הייתה בדיקה בדיעבד ולא חסם על + # הזיכרון: קובץ של 12MB נטען במלואו (20MiB שיא, נמדד) לפני שנדחה. + # ``git cat-file -s`` מחזיר את גודל האובייקט מהמאגר בלי לקרוא אותו, + # ושתי הפקודות פונות לאותו ``:`` שנפתר פעם אחת למעלה — + # אובייקט בקומיט נתון אינו משתנה, ולכן אין חלון בין הבדיקה לקריאה + # שסנכרון של המראה יכול להיכנס בו (TOCTOU נבחן ונדחה). + probe = self._object_size(mirror_path, resolved_commit, safe_file_path) + if "error" in probe: + # כשל בבדיקה אינו נופל לקריאה בלי תקרה — זו בדיוק הנפילה-לאחור + # השקטה למסלול הגרוע (``silent-fallback-to-worse-path``). הסירוב + # עולה כמות שהוא, באותה מפה שהייתה ל-``git show``. + return probe + file_size = probe["size"] + if file_size > max_size: + return { + "error": "file_too_large", + "message": f"הקובץ גדול מדי ({file_size:,} bytes)", + "size": file_size, + "max_size": max_size + } + # שליפת תוכן כ-bytes (לא text) לזיהוי בינארי נכון cmd = [ "git", "-C", str(mirror_path), @@ -1629,25 +1690,15 @@ def get_file_at_commit( ) if result.returncode != 0: - stderr = result.stderr.decode('utf-8', errors='replace') - stderr = self._sanitize_output(stderr) - - if "does not exist" in stderr or "exists on disk" in stderr: - return { - "error": "file_not_in_commit", - "message": "הקובץ לא קיים ב-commit זה" - } - if "unknown revision" in stderr or "bad revision" in stderr: - return { - "error": "invalid_commit", - "message": "Commit לא נמצא" - } - return {"error": "git_error", "message": stderr} + return self._object_read_error(result.stderr) raw_content = result.stdout file_size = len(raw_content) - # בדיקת גודל + # **החסם על מה שמוחזר, בנוסף לחסם על מה שנקרא.** לבלוב שני + # המספרים זהים; לנתיב של תיקייה ``cat-file -s`` מודד את אובייקט + # העץ ואילו ``git show`` מדפיס רשימה מעוצבת — ומה שחוזר מכאן + # לעולם אינו גדול מ-``max_size``, יהיה סוג האובייקט אשר יהיה. if file_size > max_size: return { "error": "file_too_large", diff --git a/tests/test_git_mirror_service.py b/tests/test_git_mirror_service.py index 5c1e48000..faf8884f2 100644 --- a/tests/test_git_mirror_service.py +++ b/tests/test_git_mirror_service.py @@ -249,3 +249,157 @@ def test_constructor_token_overrides_map(tmp_path, monkeypatch): svc = GitMirrorService(base_path=str(tmp_path), github_token="ghp_EXPLICIT") # טוקן שהוזרק במפורש ל-constructor גובר על הכל assert svc._token_for_url("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/Campaign-AI4U/campaign-ai.git") == "ghp_EXPLICIT" + + +# --------------------------------------------------------------------------- +# get_file_at_commit: התקרה נבדקת מול מאגר האובייקטים לפני git show (#3433, פריט 9) +# --------------------------------------------------------------------------- +# +# עד #3433 ``git show`` נטען כולו לזיכרון ורק אז ``max_size`` נבדק — קובץ של +# 12MB שנדחה עלה 20MiB שיא. הטסטים כאן רצים על מראה git אמיתית שנבנית +# ב-``tmp_path`` (bare clone, כמו בייצור), ומרגלים על ``subprocess.run`` כדי +# לראות **אילו** פקודות git רצו — כי התכונה הנבדקת היא מה לא נקרא, ואת זה +# רואים רק ברשימת הפקודות. + +import subprocess # noqa: E402 + +from services import git_mirror_service as _gms # noqa: E402 + + +def _bare_mirror(tmp_path, files): + """מראה bare בשם ``probe`` תחת ``tmp_path``, עם הקבצים הנתונים בקומיט אחד; מחזיר את השירות ואת ה-sha.""" + src = tmp_path / "src" + src.mkdir() + for name, data in files.items(): + target = src / name + target.parent.mkdir(parents=True, exist_ok=True) + target.write_bytes(data) + env = {"GIT_AUTHOR_NAME": "t", "GIT_AUTHOR_EMAIL": "t@t", "GIT_COMMITTER_NAME": "t", + "GIT_COMMITTER_EMAIL": "t@t", "PATH": __import__("os").environ["PATH"]} + for cmd in (["git", "init", "-q"], ["git", "add", "-A"], ["git", "commit", "-qm", "init"]): + subprocess.run(cmd, cwd=src, check=True, env=env, capture_output=True) + sha = subprocess.run(["git", "rev-parse", "HEAD"], cwd=src, check=True, env=env, + capture_output=True, text=True).stdout.strip() + subprocess.run(["git", "clone", "-q", "--bare", str(src), str(tmp_path / "probe.git")], + check=True, env=env, capture_output=True) + return GitMirrorService(base_path=str(tmp_path)), sha + + +def _spy_on_git(monkeypatch, override=None): + """מרגל שמעביר כל קריאה ל-``subprocess.run`` האמיתי ורושם את ה-argv; ``override(argv)`` יכול להחזיר תוצאה מזויפת.""" + calls = [] + real = subprocess.run + + def spy(cmd, *args, **kwargs): + calls.append(list(cmd)) + if override is not None: + faked = override(list(cmd)) + if faked is not None: + return faked + return real(cmd, *args, **kwargs) + + monkeypatch.setattr(_gms.subprocess, "run", spy) + return calls + + +def _subcommands(calls): + return [cmd[cmd.index("-C") + 2] if "-C" in cmd else cmd[1] for cmd in calls] + + +def test_a_file_over_the_cap_is_refused_before_git_show_reads_it(tmp_path, monkeypatch): + """הסירוב מגיע מ-``git cat-file -s``, ו-``git show`` אינו רץ כלל — אפס בתים נקראים.""" + svc, _ = _bare_mirror(tmp_path, {"big.bin": b"x" * 3000}) + calls = _spy_on_git(monkeypatch) + + out = svc.get_file_at_commit("probe", "big.bin", "HEAD", max_size=1000) + + assert out["error"] == "file_too_large" and out["size"] == 3000 and out["max_size"] == 1000 + assert "cat-file" in _subcommands(calls) + assert "show" not in _subcommands(calls), f"git show רץ על קובץ שמעל התקרה: {calls}" + + +def test_a_file_under_the_cap_is_read_exactly_as_before(tmp_path, monkeypatch): + """הבקרה: מתחת לתקרה התשובה זהה לזו של קודם — תוכן, קידוד, גודל, שורות, sha.""" + svc, sha = _bare_mirror(tmp_path, {"a.txt": "שלום\nעולם\n".encode("utf-8")}) + calls = _spy_on_git(monkeypatch) + + out = svc.get_file_at_commit("probe", "a.txt", "HEAD", max_size=1000) + + assert out["success"] and out["content"] == "שלום\nעולם\n" and out["is_binary"] is False + assert out["encoding"] == "utf-8" and out["size"] == len("שלום\nעולם\n".encode("utf-8")) + assert out["lines"] == 3 and out["resolved_commit"] == sha + assert "show" in _subcommands(calls), "מתחת לתקרה הקובץ כן נקרא" + + +def test_a_missing_path_is_still_file_not_in_commit_through_the_probe(tmp_path): + """git מדפיס את אותה הודעה ל-``cat-file -s`` ול-``show`` על נתיב חסר, והמפה אחת לשתיהן.""" + svc, _ = _bare_mirror(tmp_path, {"a.txt": b"x"}) + out = svc.get_file_at_commit("probe", "nope.txt", "HEAD") + assert out == {"error": "file_not_in_commit", "message": "הקובץ לא קיים ב-commit זה"} + + +def test_both_commands_name_the_same_resolved_commit(tmp_path, monkeypatch): + """בלי חלון בין הבדיקה לקריאה: שתי הפקודות פונות ל-``:`` שנפתר פעם אחת, לא ל-``HEAD``.""" + svc, sha = _bare_mirror(tmp_path, {"a.txt": b"x"}) + calls = _spy_on_git(monkeypatch) + + svc.get_file_at_commit("probe", "a.txt", "HEAD") + + # ``rev-parse`` של אימות ה-ref רץ גם הוא עם ``-C``; כאן מעניינות רק שתי פקודות האובייקט. + targets = {sub: cmd[-1] for cmd in calls if "-C" in cmd + for sub in [cmd[cmd.index("-C") + 2]] if sub in ("cat-file", "show")} + assert targets == {"cat-file": f"{sha}:a.txt", "show": f"{sha}:a.txt"} + assert [s for s in _subcommands(calls) if s in ("cat-file", "show")] == ["cat-file", "show"], ( + "הבדיקה קודמת לקריאה — זה הסדר שהופך את max_size לחסם על מה שנקרא") + + +def test_a_failed_size_probe_never_falls_back_to_an_unbounded_read(tmp_path, monkeypatch): + """כשל בבדיקת הגודל הוא סירוב, לא קריאה בלי תקרה — ``silent-fallback-to-worse-path``.""" + svc, _ = _bare_mirror(tmp_path, {"a.txt": b"x" * 3000}) + + def fail_the_probe(cmd): + if "cat-file" in cmd: + return subprocess.CompletedProcess(cmd, 128, stdout=b"", stderr=b"fatal: object store unreadable") + return None + + calls = _spy_on_git(monkeypatch, override=fail_the_probe) + out = svc.get_file_at_commit("probe", "a.txt", "HEAD", max_size=10) + + assert out["error"] == "git_error" and "unreadable" in out["message"] + assert "show" not in _subcommands(calls) + + +def test_a_non_numeric_size_is_a_failure_and_not_a_small_file(tmp_path, monkeypatch): + """הפלט של git הוא קלט חיצוני (U3): "lots" אינו אפס, והוא אינו עובר את התקרה בשקט.""" + svc, _ = _bare_mirror(tmp_path, {"a.txt": b"x" * 3000}) + + def garble_the_probe(cmd): + if "cat-file" in cmd: + return subprocess.CompletedProcess(cmd, 0, stdout=b"lots\n", stderr=b"") + return None + + calls = _spy_on_git(monkeypatch, override=garble_the_probe) + out = svc.get_file_at_commit("probe", "a.txt", "HEAD", max_size=10) + + assert out["error"] == "git_error" + assert "show" not in _subcommands(calls) + + +def test_what_comes_back_never_exceeds_the_cap_whatever_the_probe_said(tmp_path, monkeypatch): + """החסם השני, על מה שמוחזר: גם אם המאגר דיווח גודל קטן, מה ש-``git show`` הדפיס נבדק שוב. + + פין של תכונה שהייתה קיימת — הבדיקה בדיעבד — ונשארת בכוונה, כי לנתיב + של תיקייה ``cat-file -s`` מודד את אובייקט העץ ואילו ``git show`` מדפיס + רשימה מעוצבת. הדרך היחידה לבודד אותה היא מאגר שמשקר. + """ + svc, _ = _bare_mirror(tmp_path, {"a.txt": b"x" * 3000}) + + def understate_the_probe(cmd): + if "cat-file" in cmd: + return subprocess.CompletedProcess(cmd, 0, stdout=b"10\n", stderr=b"") + return None + + _spy_on_git(monkeypatch, override=understate_the_probe) + out = svc.get_file_at_commit("probe", "a.txt", "HEAD", max_size=1000) + + assert out["error"] == "file_too_large" and out["size"] == 3000