From b400393b063933255edcbf9f774774b4a5ee1cc6 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 4 Oct 2026 08:07:53 +0000 Subject: [PATCH 1/2] =?UTF-8?q?fix(mirrors):=20=D7=98=D7=95=D7=A7=D7=9F=20?= =?UTF-8?q?GitHub=20=D7=9C=D7=90=20=D7=A0=D7=A9=D7=9E=D7=A8=20=D7=91-confi?= =?UTF-8?q?g=20=D7=A9=D7=9C=20=D7=94=D7=9E=D7=A8=D7=90=D7=95=D7=AA=20?= =?UTF-8?q?=D7=95=D7=9C=D7=90=20=D7=A2=D7=95=D7=91=D7=A8=20=D7=91=D7=A9?= =?UTF-8?q?=D7=95=D7=A8=D7=AA=20=D7=94=D7=A4=D7=A7=D7=95=D7=93=D7=94=20(#3?= =?UTF-8?q?480);=20=D7=96=D7=99=D7=94=D7=95=D7=99=20=D7=94=D7=A2=D7=A0?= =?UTF-8?q?=D7=A3=20=D7=94=D7=A8=D7=90=D7=A9=D7=99=20=D7=91-initial=5Fimpo?= =?UTF-8?q?rt=20(#3479)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - clone מ-URL נקי; הטוקן נשלח ככותרת Authorization דרך GIT_CONFIG_COUNT (http./.extraHeader), רק לריפו שדורש הזדהות — ריפו ציבורי לא מקבל טוקן. - GIT_TERMINAL_PROMPT=0 ו-transfer.credentialsInUrl=die בכל clone/fetch. - ensure_clean_remote: set-url + קריאה חוזרת, GIT_DIR מוצמד; fetch לא רץ כשהניקוי לא אומת (mirror_url_not_clean). - ניקוי כל המראות בעלייה (MCP: create_app; וובאפ: start_webapp.sh), עם שורת לוג אחת: checked/had_credentials/cleaned/failed ומקור הטוקן לכל מראה. - git remote מותר רק כ-get-url origin / set-url origin . - הוסר _get_authenticated_url כולל ההזרקה לכל כתובת HTTPS. - initial_import: הענף הראשי מ-HEAD של המראה דרך rev-parse, וכשל בשם default_branch_undetected במקום נפילה ל-main. - טסטים עם git אמיתי ושרת http-backend מקומי. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01CeuxN4qG2BdcVSdrbmHKCF --- GUIDES/REPO_SYNC_ENGINE_GUIDE.md | 2 + docs/environment-variables.rst | 4 +- docs/mcp-server.rst | 19 + docs/whats-new.rst | 4 + mcp_server/app.py | 12 + scripts/start_webapp.sh | 25 ++ scripts/sweep_mirror_credentials.py | 33 ++ services/git_mirror_service.py | 426 +++++++++++++++++++--- services/repo_sync_service.py | 89 +---- tests/test_git_mirror_credentials.py | 517 +++++++++++++++++++++++++++ tests/test_git_mirror_service.py | 53 ++- tests/test_repo_sync_service.py | 7 +- 12 files changed, 1049 insertions(+), 142 deletions(-) create mode 100644 scripts/sweep_mirror_credentials.py create mode 100644 tests/test_git_mirror_credentials.py diff --git a/GUIDES/REPO_SYNC_ENGINE_GUIDE.md b/GUIDES/REPO_SYNC_ENGINE_GUIDE.md index 927222c4f..bbc236ad7 100644 --- a/GUIDES/REPO_SYNC_ENGINE_GUIDE.md +++ b/GUIDES/REPO_SYNC_ENGINE_GUIDE.md @@ -152,6 +152,8 @@ print(f"Disk writable: {os.access(mirror_path, os.W_OK)}") ## מימוש GitMirrorService +> ⚠️ **מיושן באימות מול GitHub (#3480).** הקוד שלמטה מזריק את הטוקן ל-URL (`_get_authenticated_url`), ו-git שומר את ה-URL הזה בטקסט גלוי בקובץ ה-`config` של המראה. הפונקציה הוסרה מהקוד: הטוקן עובר היום ככותרת דרך משתני הסביבה של git, וה-URL השמור נקי. המימוש הנוכחי — `_network_env`, `_run_network_git` ו-`ensure_clean_remote` ב-`services/git_mirror_service.py`; ההסבר — העמוד `docs/mcp-server.rst`, הסעיף "אימות מול GitHub — הטוקן לא נשמר במראה". אל תעתיקו את דפוס ההזרקה מכאן. + צור קובץ `services/git_mirror_service.py`: ```python diff --git a/docs/environment-variables.rst b/docs/environment-variables.rst index 6e5d9fc0b..0ab4c7196 100644 --- a/docs/environment-variables.rst +++ b/docs/environment-variables.rst @@ -203,13 +203,13 @@ - ``300`` - Bot * - ``GITHUB_TOKEN`` - - טוקן GitHub לשימוש בפעולות API וגם לאימות clone/fetch של Repo Sync בריפו פרטי (אם רלוונטי). למינימום הרשאות ראו טבלת Scopes בהמשך. + - טוקן GitHub לשימוש בפעולות API, וגם לאימות clone/fetch של המראות לבעלים שאינו ב-``GITHUB_TOKENS``. במראות הוא נשלח רק לריפו שדורש הזדהות, ככותרת ולא בתוך ה-URL — ראו :ref:`mcp-mirror-credentials`. למינימום הרשאות ראו טבלת Scopes בהמשך. - לא - - - ``ghp_xxx...`` - Bot/WebApp * - ``GITHUB_TOKENS`` - - מיפוי בעלים(owner/org)→טוקן לסנכרון ריפואים ממספר ארגונים, כשטוקן יחיד לא מכסה את כולם. פורמט JSON (``{"Org": "ghp_..."}``) או פשוט (``Org1=ghp_...,Org2=github_pat_...``). נבחר לפי הבעלים של הריפו; אם אין התאמה נופלים ל-``GITHUB_TOKEN``. + - מיפוי בעלים(owner/org)→טוקן לסנכרון ריפואים ממספר ארגונים, כשטוקן יחיד לא מכסה את כולם. פורמט JSON (``{"Org": "ghp_..."}``) או פשוט (``Org1=ghp_...,Org2=github_pat_...``). נבחר לפי הבעלים של הריפו; אם אין התאמה נופלים ל-``GITHUB_TOKEN``. הטוקן לא נשמר במראה ולא נשלח לריפו ציבורי (:ref:`mcp-mirror-credentials`). - לא - - - ``Org1=ghp_xxx,Org2=github_pat_yyy`` diff --git a/docs/mcp-server.rst b/docs/mcp-server.rst index e94eb1420..1b588f60f 100644 --- a/docs/mcp-server.rst +++ b/docs/mcp-server.rst @@ -1738,6 +1738,25 @@ Claude Desktop ``MCP_REPO_AUTOSYNC_INTERVAL`` (ברירת מחדל 300 שניות). +.. _mcp-mirror-credentials: + +אימות מול GitHub — הטוקן לא נשמר במראה +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +**ה-URL שבמראה תמיד נקי.** עד #3480 הטוקן הוזרק ל-URL של ה-clone, ו-git שמר אותו בטקסט גלוי ב-``remote.origin.url`` שבקובץ ``config`` של המראה — על הדיסק של הוובאפ, על הדיסק של שירות ה-MCP, ובצילומי הדיסק של Render. היום ה-clone נעשה מה-URL הנקי, והטוקן עובר לכל ``clone``/``fetch`` בנפרד ככותרת ``Authorization``, דרך משתני הסביבה ``GIT_CONFIG_COUNT``/``GIT_CONFIG_KEY_``/``GIT_CONFIG_VALUE_`` ולא בשורת הפקודה (``_network_env`` ב-``services/git_mirror_service.py``). הכותרת ממוקדת ל-``GITHUB_HTTPS_ORIGIN`` בלבד. + +**איזה טוקן, ולמי.** הטוקן נבחר לפי בעלי הריפו: מהמפה ב-``GITHUB_TOKENS``, ובעלים שאינו במפה מקבל את ``GITHUB_TOKEN`` (``_token_and_source_for_url``). כל פקודת רשת נשלחת קודם **בלי** טוקן, ורק כש-GitHub עונה שהריפו דורש הזדהות היא נשלחת שוב עם הכותרת — כך שריפו ציבורי לא מקבל טוקן אף פעם (``_run_network_git``). שורות הלוג ``Mirror clone``/``Mirror fetch`` מציינות ``auth_used``: ``none``, ``map``, ``global`` או ``explicit``. + +**ניקוי מראות ישנות.** ``ensure_clean_remote`` מוציא credentials מה-URL השמור ב-``git remote set-url``, ומאמת בקריאה חוזרת של ה-URL — לא לפי קוד היציאה. הוא רץ לפני כל ``fetch_updates`` (אם הניקוי לא אומת, ה-fetch לא רץ ומוחזר ``mirror_url_not_clean``), וגם על כל המראות בעליית כל שירות: בשירות ה-MCP מתוך ``create_app``, ובוובאפ מתוך ``scripts/start_webapp.sh``. שורת הלוג של המעבר הזה היא האימות שהמראות נקיות: + +.. code-block:: text + + mirror credential sweep: checked= had_credentials= cleaned= failed= sources={"": "map", ...} + +``sources`` אומר לכל מראה מאיפה יגיע הטוקן שלה אם הריפו ידרוש הזדהות, בלי הטוקן עצמו — כך רואים אילו מראות נשענות על ``GITHUB_TOKEN``. שורה שלא מופיעה בלוג של אחד השירותים פירושה שהמעבר לא רץ שם. + +**שומר.** כל פקודת רשת רצה עם ``transfer.credentialsInUrl=die``: מראה שעדיין נושאת credentials ב-URL נכשלת לפני בקשת רשת, ו-git עצמו מסתיר את הסיסמה בהודעה. ``git remote`` מותר ב-``_run_git_command`` רק בשתי הצורות ``get-url origin`` ו-``set-url origin ``, וה-URL חייב לעבור את ``_validate_repo_url``. + .. _mcp-repo-read-misses: ענף שלא במראה, מול קובץ שלא קיים diff --git a/docs/whats-new.rst b/docs/whats-new.rst index 5b69238b9..e8aec3300 100644 --- a/docs/whats-new.rst +++ b/docs/whats-new.rst @@ -2,6 +2,10 @@ What's New ========== :summary: יומן השינויים של הבוט וה-WebApp לפי תאריך — מה נוסף, מה השתנה ומה תוקן בכל עדכון, עם קישורים ל-Issues הרלוונטיים. +2026-10-04 +---------- +- **fix (מראות): טוקן ה-GitHub לא נשמר יותר בקובץ ה-config של המראות, ולא עובר בשורת הפקודה (#3480); וזיהוי הענף הראשי בייבוא ריפו (#3479).** עד עכשיו ``init_mirror`` שכפל מ-URL שהטוקן בתוכו, ו-git שמר אותו בטקסט גלוי ב-``remote.origin.url`` — על הדיסקים של הוובאפ ושל שירות ה-MCP, ובצילומים היומיים של Render; הוא גם הופיע בשורת הפקודה של ``git-remote-http`` בכל clone ו-fetch. עכשיו ה-URL השמור נקי, והטוקן עובר ככותרת דרך משתני סביבה של git, ורק לריפו שדורש הזדהות — ריפו ציבורי לא מקבל טוקן. מראות קיימות מנוקות לפני כל fetch ובעליית כל שירות, עם שורת לוג אחת שסופרת מה נמצא ומה נוקה, ואומרת לכל מראה מאיפה הטוקן שלה (``map``/``global``). fetch על מראה שלא אומת שהיא נקייה לא רץ (``mirror_url_not_clean``). ובנפרד: ``initial_import`` ניסה לזהות את הענף הראשי בפקודות ש-``_run_git_command`` דוחה, ולכן תמיד נפל ל-``main`` — וריפו שהענף הראשי שלו ``master`` לא יובא. עכשיו הזיהוי הוא מ-HEAD של המראה, וכשאין ענף מאומת מוחזר ``default_branch_undetected`` במקום ניחוש. ראו :ref:`mcp-mirror-credentials`. + 2026-10-01 ---------- - **fix (MCP): העלאה שהשתבשה באחסון הזמני אינה נשמרת — ``upload_corrupted``; ותיאור ``upload_id`` ב-``codekeeper_append_file`` אומר מה ה-hash מוכיח בהוספה.** ``content_changed`` משווה את הטקסט שנשלף מ-``mcp_uploads`` למה שנכתב לקובץ, ולכן טקסט שהשתבש בין ההעלאה לשמירה היה נשמר בשקט, עם ``content_changed: false``. עכשיו ``_load_upload`` — העוזר שכל כלי שמקבל ``upload_id`` שולף דרכו — משווה מיד אחרי השליפה, לפני כל בדיקה אחרת של הכלי, את ה-hash של הטקסט שנשלף ל-``content_sha256`` שנשמר עם ההעלאה כשהגיעה. שונים, או שמה שנשמר אינו hash: ``upload_corrupted``, גם כשבדיקה אחרת — התקרה, ``file_exists`` — הייתה נכשלת. שום דבר לא נכתב, ההעלאה נמחקת (ואם האחסון לא ענה למחיקה, היא פוקעת ב-TTL; התשובה אינה תלויה בזה), ונרשמת שורת ``ERROR`` אחת בלי המזהה ובלי התוכן. שומר מבני בטסטים מוודא שאין דרך שליפה או צריכה אחרת, לא בכלי ולא ב-backend. ובנפרד: תיאור הפרמטר ``upload_id`` היה משותף לשני הכלים והבטיח גם בהוספה ש-``file.content_sha256`` יהיה שווה ל-hash של ההעלאה. בהוספה ``file.content_sha256`` הוא של הקובץ כולו, ומה שמראה שהטקסט נכנס בשלמותו הוא ``content_changed: false`` — וסוכן שבודק לפי התיאור הישן היה מסיק שההוספה נכשלה, מעלה שוב, ומוסיף את הטקסט פעמיים. עכשיו לכל כלי המשפט שלו, וטסט בודק כל משפט מול מה שהכלי מחזיר. ראו :ref:`mcp-uploads`. diff --git a/mcp_server/app.py b/mcp_server/app.py index 56a5e79d4..1b29697a7 100644 --- a/mcp_server/app.py +++ b/mcp_server/app.py @@ -154,6 +154,18 @@ def create_app(): logging.getLogger(__name__).warning("repo autosync failed to start", exc_info=True) + # מראות שנוצרו לפני #3480 נושאות את טוקן ה-GitHub ב-remote.origin.url. ניקוי + # אחד בעלייה, **בלי תלות ב-MCP_REPO_AUTOSYNC**: מראה שלא נמשכת לא תגיע + # לניקוי שב-fetch_updates. השורה "mirror credential sweep:" בלוג היא האימות. + try: + from services.git_mirror_service import start_credential_sweep + + start_credential_sweep() + except Exception: + import logging + + logging.getLogger(__name__).warning("mirror credential sweep failed to start", exc_info=True) + mcp_base = (os.getenv("MCP_SERVER_URL") or "").rstrip("/") webapp_base = (os.getenv("WEBAPP_URL") or "").rstrip("/") diff --git a/scripts/start_webapp.sh b/scripts/start_webapp.sh index 72e4a4224..311095758 100755 --- a/scripts/start_webapp.sh +++ b/scripts/start_webapp.sh @@ -126,4 +126,29 @@ warmup() { } warmup || true + +# מראות שנוצרו לפני #3480 נושאות את טוקן ה-GitHub ב-remote.origin.url. ניקוי אחד +# בכל עלייה, ברקע ואחרי ש-Gunicorn כבר מאזין — כדי לא לעכב את העלייה. השורה +# "mirror credential sweep:" בלוג היא האימות. כשל כאן לא מפיל את השירות: fetch +# של מראה שלא נוקתה נעצר בעצמו (transfer.credentialsInUrl=die). +sweep_mirror_credentials() { + local py + py="$(command -v python3 || command -v python || true)" + if [ -z "$py" ]; then + log "Mirror credential sweep skipped: no python interpreter on PATH" + return 0 + fi + # תקרה של 5 דקות: כל מראה היא כמה קריאות git מקומיות של שניות בודדות + local runner=("$py") + if command -v timeout >/dev/null 2>&1; then + runner=(timeout 300 "$py") + fi + if (cd "$ROOT_DIR" && "${runner[@]}" scripts/sweep_mirror_credentials.py); then + log "Mirror credential sweep finished" + else + log "Mirror credential sweep reported failures (see 'mirror credential sweep' lines above)" + fi +} + +sweep_mirror_credentials & wait "$APP_PID" diff --git a/scripts/sweep_mirror_credentials.py b/scripts/sweep_mirror_credentials.py new file mode 100644 index 000000000..5a2b18b3a --- /dev/null +++ b/scripts/sweep_mirror_credentials.py @@ -0,0 +1,33 @@ +#!/usr/bin/env python3 +"""ניקוי טוקני GitHub מה-``remote.origin.url`` של כל המראות בדיסק של השירות (#3480). + +``scripts/start_webapp.sh`` מריץ אותו בעליית הוובאפ, ברקע ואחרי ש-Gunicorn כבר +עלה. בשירות ה-MCP אותו ניקוי רץ מתוך ``create_app`` (``start_credential_sweep``). +העבודה עצמה ב-``services.git_mirror_service.sweep_stored_credentials``; כאן רק +נקודת כניסה, שמדפיסה את שורת הלוג ``mirror credential sweep: ...`` לפלט של השירות. + +קוד יציאה: 0 כשאין כשלים (כולל "אין תיקיית מראות"), 1 כשמראה אחת לפחות לא נוקתה. +""" + +from __future__ import annotations + +import logging +import os +import sys + + +def main() -> int: + logging.basicConfig(level=logging.INFO, stream=sys.stdout, format="%(levelname)s %(name)s: %(message)s") + # Make the repo importable when run directly. + sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) + + from services.git_mirror_service import sweep_stored_credentials # noqa: E402 + + stats = sweep_stored_credentials() + if stats is None: + return 0 + return 1 if stats["failed"] else 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/services/git_mirror_service.py b/services/git_mirror_service.py index 68859dd80..d830b1264 100644 --- a/services/git_mirror_service.py +++ b/services/git_mirror_service.py @@ -11,23 +11,65 @@ from __future__ import annotations +import base64 import codecs +import functools import json import logging import os import re import select import subprocess +import threading import time from collections import deque import shutil from dataclasses import dataclass from datetime import datetime from pathlib import Path -from typing import Any, Dict, List, Optional, Set +from typing import Any, Dict, List, Optional, Set, Tuple +from urllib.parse import urlsplit, urlunsplit logger = logging.getLogger(__name__) +# ---- אימות מול GitHub בלי לשמור את הטוקן בדיסק (#3480) ------------------ +# +# עד #3480 הטוקן הוזרק ל-URL (``https://oauth2:@github.com/...``), ו-git +# שומר את ה-URL שה-clone נעשה ממנו ב-``remote.origin.url`` בקובץ ``config`` של +# המראה — בטקסט גלוי, על הדיסק של הוובאפ ושל שירות ה-MCP ובצילומים היומיים של +# Render. git גם מעביר את ה-URL המלא כארגומנט ל-``git-remote-https``, כך שהוא +# גלוי ברשימת התהליכים בכל clone ו-fetch. מאז, ה-URL שנשמר תמיד נקי, והטוקן +# עובר לכל פקודת רשת בנפרד כ**כותרת**, דרך משתני הסביבה ``GIT_CONFIG_COUNT`` / +# ``GIT_CONFIG_KEY_`` / ``GIT_CONFIG_VALUE_`` (git 2.31 ומעלה, +# ``Documentation/git-config.txt``) — לא דרך ``git -c``, שמציב את הערך בשורת +# הפקודה. התיעוד של git עצמו מצביע על ``http.extraHeader`` כמקרה שבו העברה +# דרך הסביבה עדיפה (``Documentation/git.txt``, ``--config-env``). +# +# **המקור (origin) של GitHub.** ממנו נגזרים גם הבדיקה של כתובת ריפו +# (``_validate_repo_url``), גם חילוץ הבעלים, וגם המפתח שאליו הכותרת ממוקדת — +# git משווה את המפתח לכתובת לפי scheme, host ו-port (``Documentation/config/http.txt``, +# ``http..*``), ולכן הכותרת לא נשלחת לשום מארח אחר. הטסטים מחליפים את +# הקבוע כדי להריץ את ``init_mirror``/``fetch_updates`` האמיתיים מול שרת מקומי. +GITHUB_HTTPS_ORIGIN = "https://github.com" + +# שם המשתמש שנשלח עם הטוקן. אותו credential בדיוק שה-URL נשא עד #3480. +GIT_AUTH_USERNAME = "oauth2" + +# ``transfer.credentialsInUrl=die`` (git 2.37 ומעלה): מראה ששוב נושאת +# credentials ב-URL נכשלת **לפני** בקשת רשת, וההודעה של git כבר מסתירה את +# הסיסמה (````). זה השומר שהופך "מראה שלא נוקתה" לשגיאה עם שם, +# במקום fetch שקט עם הטוקן השמור. +_CREDENTIALS_IN_URL_GUARD: Tuple[str, str] = ("transfer.credentialsInUrl", "die") + +# הודעות git כשהשרת דורש הזדהות ואין מה לתת לו. נאכפות באנגלית דרך +# ``LC_ALL=C`` בסביבת פקודות הרשת. ``could not read Username`` נבדק מול +# github.com עצמו, על ריפו פרטי/לא קיים בלי טוקן (git 2.43, אוקטובר 2026). +_AUTH_REQUIRED_MARKERS = ( + "could not read username", + "terminal prompts disabled", + "authentication failed", +) + # הגדרות קבועות MAX_DIFF_BYTES = 1 * 1024 * 1024 # 1MB MAX_DIFF_LINES = 10000 @@ -120,6 +162,10 @@ ) +def _default_mirror_base_path() -> Path: + """תיקיית המראות כשלא הועבר נתיב: ``REPO_MIRROR_PATH``, ואחרת ``/var/data/repos``.""" + return Path(os.getenv("REPO_MIRROR_PATH", "/var/data/repos")) + def _looks_like_git_sha(text: str) -> bool: t = (text or "").strip() if len(t) < 7 or len(t) > 64: @@ -376,7 +422,9 @@ class GitMirrorService: content = service.get_file_content("repo", "src/main.py") תמיכה ב-Private Repos: - הגדר GITHUB_TOKEN בסביבה, והשירות יזריק אותו אוטומטית ל-URL. + טוקן לפי בעלים ב-``GITHUB_TOKENS``, ו-``GITHUB_TOKEN`` לבעלים שאינו במפה + (``_token_and_source_for_url``). הטוקן לעולם אינו נכנס ל-URL: הוא נשלח + ככותרת בכל פקודת רשת, ורק לריפו שדורש הזדהות (``_run_network_git``). """ # שם ריפו: a-z, 0-9, -, _ בלבד, 1-100 תווים @@ -408,13 +456,26 @@ class GitMirrorService: r'^[a-zA-Z0-9][a-zA-Z0-9._/^~-]{0,150}$' ) - _GITHUB_HTTPS_RE = re.compile(r"^https://github\.com/[^/\s]+/[^/\s]+(?:\.git)?/?$", re.IGNORECASE) + # הצורות של HTTPS נגזרות מ-``GITHUB_HTTPS_ORIGIN`` (``_https_patterns``). _GITHUB_SSH_RE = re.compile(r"^git@github\.com:[^/\s]+/[^/\s]+(?:\.git)?$", re.IGNORECASE) # חילוץ בעלים (owner/org) מתוך URL של GitHub – לבחירת טוקן פר-ארגון - _OWNER_HTTPS_RE = re.compile(r"^https://github\.com/([^/\s]+)/", re.IGNORECASE) _OWNER_SSH_RE = re.compile(r"^git@github\.com:([^/\s]+)/", re.IGNORECASE) + # ``git remote`` מותר **רק** בשתי הצורות האלה, ובדיוק במבנה הזה. כל השאר + # (``add``, ``remove``, ``set-url --push``...) נדחה כמו תת-פקודה לא מוכרת. + _REMOTE_GET_URL = ("remote", "get-url", "origin") + _REMOTE_SET_URL = ("remote", "set-url", "origin") + + @staticmethod + @functools.lru_cache(maxsize=8) + def _https_patterns(origin: str) -> Tuple["re.Pattern[str]", "re.Pattern[str]"]: + """(כתובת ריפו מלאה, חילוץ בעלים) עבור ה-origin הנתון.""" + o = re.escape(origin.rstrip("/")) + repo = re.compile(rf"^{o}/[^/\s]+/[^/\s]+(?:\.git)?/?$", re.IGNORECASE) + owner = re.compile(rf"^{o}/([^/\s]+)/", re.IGNORECASE) + return repo, owner + def __init__( self, base_path: Optional[str] = None, @@ -424,9 +485,9 @@ def __init__( """ Args: base_path: נתיב בסיסי לאחסון mirrors. - ברירת מחדל: REPO_MIRROR_PATH או /var/data/repos + ברירת מחדל: ``_default_mirror_base_path()`` """ - self.base_path = Path(mirrors_base_path or base_path or os.getenv("REPO_MIRROR_PATH", "/var/data/repos")) + self.base_path = Path(mirrors_base_path or base_path or _default_mirror_base_path()) self.github_token = github_token self.logger = logger self._ensure_base_path() @@ -467,7 +528,8 @@ def _validate_repo_url(self, repo_url: str) -> bool: return False if url.startswith("-"): return False - return bool(self._GITHUB_HTTPS_RE.fullmatch(url) or self._GITHUB_SSH_RE.fullmatch(url)) + https_repo, _ = self._https_patterns(GITHUB_HTTPS_ORIGIN) + return bool(https_repo.fullmatch(url) or self._GITHUB_SSH_RE.fullmatch(url)) @staticmethod def _load_github_token_map() -> Dict[str, str]: @@ -521,58 +583,95 @@ def _extract_owner(self, url: str) -> str: if not isinstance(url, str): return "" u = url.strip() - m = self._OWNER_HTTPS_RE.match(u) or self._OWNER_SSH_RE.match(u) + _, https_owner = self._https_patterns(GITHUB_HTTPS_ORIGIN) + m = https_owner.match(u) or self._OWNER_SSH_RE.match(u) return m.group(1) if m else "" def _token_for_url(self, url: str) -> Optional[str]: + """הטוקן שמוגדר ל-URL, או ``None``. המקור והסדר: ``_token_and_source_for_url``.""" + return self._token_and_source_for_url(url)[0] + + def _token_and_source_for_url(self, url: str) -> Tuple[Optional[str], str]: """ - בוחר את הטוקן המתאים ל-URL לפי סדר עדיפויות: - 1. טוקן שהוזרק במפורש ל-constructor (override). - 2. טוקן פר-בעלים מתוך ``GITHUB_TOKENS`` (לפי הארגון של הריפו). - 3. ``GITHUB_TOKEN`` הגלובלי כברירת מחדל. + בוחר את הטוקן המתאים ל-URL, ומחזיר גם **מאיפה** הוא הגיע: + + 1. ``explicit`` — טוקן שהוזרק במפורש ל-constructor (טסטים וסקריפטים). + 2. ``map`` — טוקן פר-בעלים מתוך ``GITHUB_TOKENS`` (לפי הארגון של הריפו). + 3. ``global`` — ``GITHUB_TOKEN``, לבעלים שאינו במפה. + 4. ``none`` — אין טוקן. + + המקור נכתב לשורת הלוג של הניקוי (``scrub_stored_credentials``), כדי + שאפשר יהיה לראות אילו מראות נשענות על הטוקן הגלובלי. הטוקן עצמו לא. """ if self.github_token: - return self.github_token + return self.github_token, "explicit" owner = self._extract_owner(url) if owner: token_map = self._load_github_token_map() token = token_map.get(owner.lower()) if token: - return token - - return os.getenv("GITHUB_TOKEN") or None + return token, "map" - def _get_authenticated_url(self, url: str) -> str: - """ - הזרקת GitHub Token ל-URL לתמיכה ב-Private Repos - - Args: - url: URL מקורי של הריפו + token = os.getenv("GITHUB_TOKEN") or None + return (token, "global") if token else (None, "none") - Returns: - URL עם token (אם קיים) או URL מקורי - - Note: - לא לרשום את ה-URL המאומת ללוגים! - בחירת הטוקן נעשית לפי הבעלים של הריפו (ראו _token_for_url), - כדי לתמוך בסנכרון ריפואים ממספר ארגונים עם טוקנים שונים. - """ - token = self._token_for_url(url) + @staticmethod + def _network_env(token: Optional[str]) -> Dict[str, str]: + """הסביבה של פקודת רשת (clone/fetch): בלי הנחיות מסוף, עם השומר, ועם הכותרת אם יש טוקן. + + הכותרת היא אותו credential בדיוק שה-URL נשא עד #3480 (``oauth2:`` + ב-Basic), ממוקדת ל-``GITHUB_HTTPS_ORIGIN`` בלבד. base64 גם מנטרל כל תו + בטוקן שהיה שובר את הדקדוק של כותרת HTTP. + """ + pairs: List[Tuple[str, str]] = [_CREDENTIALS_IN_URL_GUARD] + if token: + basic = base64.b64encode(f"{GIT_AUTH_USERNAME}:{token}".encode("utf-8")).decode("ascii") + pairs.append( + (f"http.{GITHUB_HTTPS_ORIGIN.rstrip('/')}/.extraHeader", f"Authorization: Basic {basic}") + ) + env: Dict[str, str] = { + "GIT_TERMINAL_PROMPT": "0", + # הודעות git באנגלית: ``_needs_auth`` ו-``_classify_git_error`` מזהים אותן לפי הטקסט + "LC_ALL": "C", + "GIT_CONFIG_COUNT": str(len(pairs)), + } + for i, (key, value) in enumerate(pairs): + env[f"GIT_CONFIG_KEY_{i}"] = key + env[f"GIT_CONFIG_VALUE_{i}"] = value + return env + @staticmethod + def _needs_auth(stderr: str) -> bool: + """האם git נכשל כי השרת דרש הזדהות (ריפו פרטי, או טוקן שלא התקבל).""" + text = (stderr or "").lower() + return any(marker in text for marker in _AUTH_REQUIRED_MARKERS) + + def _run_network_git( + self, cmd: List[str], url: str, cwd: Optional[Path] = None, timeout: int = 60 + ) -> Tuple[GitCommandResult, str]: + """מריץ clone/fetch, ומחזיר גם איזה טוקן נשלח בפועל: ``none``/``map``/``global``/``explicit``. + + **קודם בלי טוקן.** ריפו ציבורי לא מקבל טוקן אף פעם. רק אם git נכשל כי + השרת דרש הזדהות, ויש טוקן מוגדר לבעלים — ניסיון שני עם הכותרת. ריפו + פרטי עולה בקשה אחת שנכשלת מיד (``GIT_TERMINAL_PROMPT=0``). + + **אין מסלול שמכניס את הטוקן ל-URL**, גם לא כשהכותרת נכשלת — זה בדיוק + מה ש-#3480 הוציא. ``transfer.credentialsInUrl=die`` בשני הניסיונות. + """ + result = self._run_git_command(cmd, cwd=cwd, timeout=timeout, extra_env=self._network_env(None)) + if result.success or not self._needs_auth(result.stderr): + return result, "none" + token, source = self._token_and_source_for_url(url) if not token: - return url - - # תמיכה ב-HTTPS URLs בלבד - if url.startswith("/"): - # https://github.com/user/repo.git - # -> https://oauth2:TOKEN@github.com/user/repo.git - return url.replace("/", f"https://oauth2:{token}@github.com/") - elif url.startswith("https://"): - # Generic HTTPS URL - return url.replace("https://", f"https://oauth2:{token}@") - - return url + return result, "none" + if cmd[1] == "clone": + # clone שנכשל עלול להשאיר תיקייה, וניסיון שני היה נכשל על "already exists" + target = Path(cmd[-1]) + if target.exists() and not self._safe_rmtree(target): + logger.warning("Could not remove partial clone before authenticated retry: %s", target) + result = self._run_git_command(cmd, cwd=cwd, timeout=timeout, extra_env=self._network_env(token)) + return result, source def _sanitize_output(self, output: str) -> str: """הסרת מידע רגיש מפלט Git.""" @@ -681,7 +780,26 @@ def _safe_rmtree(self, path: Path) -> bool: except Exception: return False - def _run_git_command(self, cmd: List[str], cwd: Optional[Path] = None, timeout: int = 60) -> GitCommandResult: + def _is_allowed_remote_command(self, cmd: List[str]) -> bool: + """``git remote`` מותר רק כ-``get-url origin`` או כ-``set-url origin ``. + + ה-URL ב-``set-url`` חייב לעבור את ``_validate_repo_url``, שמקבלת רק + ``//`` — כלומר URL עם credentials לא יכול להיכתב + דרך הנתיב הזה. + """ + if tuple(cmd[1:4]) == self._REMOTE_GET_URL and len(cmd) == 4: + return True + if tuple(cmd[1:4]) == self._REMOTE_SET_URL and len(cmd) == 5: + return self._validate_repo_url(str(cmd[4])) + return False + + def _run_git_command( + self, + cmd: List[str], + cwd: Optional[Path] = None, + timeout: int = 60, + extra_env: Optional[Dict[str, str]] = None, + ) -> GitCommandResult: """ הרצת פקודת Git בצורה בטוחה @@ -689,6 +807,8 @@ def _run_git_command(self, cmd: List[str], cwd: Optional[Path] = None, timeout: cmd: פקודת Git כרשימה cwd: תיקיית עבודה timeout: timeout בשניות + extra_env: משתני סביבה שמתווספים לסביבת התהליך (פקודות רשת בלבד — + ``_network_env``). בלעדיו git יורש את הסביבה כמו קודם. Returns: GitCommandResult עם התוצאות @@ -702,17 +822,26 @@ def _run_git_command(self, cmd: List[str], cwd: Optional[Path] = None, timeout: return GitCommandResult(success=False, stdout="", stderr="Invalid git command", return_code=-2) if cmd[0] != "git": return GitCommandResult(success=False, stdout="", stderr="Refusing to run non-git command", return_code=-2) - if len(cmd) < 2 or cmd[1] not in self._allowed_git_subcommands: + if len(cmd) < 2: + return GitCommandResult(success=False, stdout="", stderr="Unsupported git subcommand", return_code=-2) + if cmd[1] == "remote": + if not self._is_allowed_remote_command(cmd): + return GitCommandResult(success=False, stdout="", stderr="Unsupported git subcommand", return_code=-2) + elif cmd[1] not in self._allowed_git_subcommands: return GitCommandResult(success=False, stdout="", stderr="Unsupported git subcommand", return_code=-2) if any("\x00" in str(part) for part in cmd): return GitCommandResult(success=False, stdout="", stderr="Invalid NUL in command", return_code=-2) + run_kwargs: Dict[str, Any] = {} + if extra_env: + run_kwargs["env"] = {**os.environ, **extra_env} result = subprocess.run( cmd, cwd=str(cwd) if cwd else None, capture_output=True, text=True, timeout=timeout, + **run_kwargs, ) success = result.returncode == 0 @@ -771,7 +900,8 @@ def init_mirror(self, repo_url: str, repo_name: str, timeout: int = 600) -> Dict dict עם success, path, message Note: - תומך ב-Private Repos אם GITHUB_TOKEN מוגדר בסביבה. + ה-clone נעשה מה-URL הנקי, וזה ה-URL שנשמר במראה. ריפו פרטי מקבל את + הטוקן ככותרת בלבד (``_run_network_git``). """ repo_url = str(repo_url or "").strip() repo_name = str(repo_name or "").strip() @@ -799,12 +929,12 @@ def init_mirror(self, repo_url: str, repo_name: str, timeout: int = 600) -> Dict # לוג ללא ה-token! logger.info(f"Creating mirror: {self._sanitize_output(repo_url)} -> {repo_path}") - # הזרקת token ל-Private Repos - auth_url = self._get_authenticated_url(repo_url) - - # Clone as bare mirror + # Clone as bare mirror, מה-URL הנקי — הוא שנשמר ב-remote.origin.url # שימוש ב-"--" כדי למנוע פרשנות של URL/נתיב כ-flag במקרה קצה - result = self._run_git_command(["git", "clone", "--mirror", "--", auth_url, str(repo_path)], timeout=timeout) + result, auth_used = self._run_network_git( + ["git", "clone", "--mirror", "--", repo_url, str(repo_path)], repo_url, timeout=timeout + ) + logger.info("Mirror clone for %s: success=%s auth_used=%s", repo_name, result.success, auth_used) if result.success: logger.info(f"Mirror created successfully: {repo_path}") @@ -813,6 +943,7 @@ def init_mirror(self, repo_url: str, repo_name: str, timeout: int = 600) -> Dict "path": str(repo_path), "message": "Mirror created successfully", "already_existed": False, + "auth_used": auth_used, } else: logger.error(f"Failed to create mirror: {result.stderr}") @@ -852,15 +983,36 @@ def fetch_updates(self, repo_name: str, timeout: int = 120) -> Dict[str, Any]: "action_needed": "init_mirror", } + # מראה שנוצרה לפני #3480 נושאת את הטוקן ב-remote.origin.url. מנקים לפני + # כל fetch; אם הניקוי לא אומת — לא מושכים בכלל (וגם השומר של git היה + # עוצר את ה-fetch לפני בקשת רשת). + cleaning = self.ensure_clean_remote(repo_name) + if cleaning["status"] == "failed": + logger.error( + "Refusing to fetch %s: stored remote URL is not verified clean (%s)", + repo_name, + cleaning["reason"], + ) + return { + "success": False, + "error_type": "mirror_url_not_clean", + "message": f"Stored remote URL could not be cleaned: {cleaning['reason']}", + "retry_recommended": False, + } + logger.info(f"Fetching updates for {repo_name}") - result = self._run_git_command(["git", "fetch", "--all", "--prune"], cwd=repo_path, timeout=timeout) + result, auth_used = self._run_network_git( + ["git", "fetch", "--all", "--prune"], cleaning["url"], cwd=repo_path, timeout=timeout + ) + logger.info("Mirror fetch for %s: success=%s auth_used=%s", repo_name, result.success, auth_used) if result.success: return { "success": True, "message": "Fetch completed", "output": result.stdout[:500] if result.stdout else "No output", + "auth_used": auth_used, } else: # זיהוי סוגי שגיאות @@ -878,7 +1030,9 @@ def _classify_git_error(self, stderr: str) -> str: if "could not resolve host" in stderr_lower: return "network_error" - elif "authentication failed" in stderr_lower: + elif self._needs_auth(stderr_lower): + # כולל "could not read Username ... terminal prompts disabled": כך + # git עונה מאז #3480 כשריפו דורש הזדהות ואין טוקן מוגדר לבעלים return "auth_error" elif "repository not found" in stderr_lower: return "repo_not_found" @@ -891,6 +1045,106 @@ def mirror_exists(self, repo_name: str) -> bool: """בדיקה אם mirror קיים""" return self._get_repo_path(repo_name).exists() + # ========== Stored credentials (#3480) ========== + + @staticmethod + def _strip_userinfo(url: str) -> Tuple[str, bool]: + """(ה-URL בלי ``user:pass@``, האם היה שם userinfo). URL של SSH מוחזר כמות שהוא.""" + u = (url or "").strip() + if "://" not in u: + return u, False + parts = urlsplit(u) + if "@" not in parts.netloc: + return u, False + host = parts.netloc.rsplit("@", 1)[1] + return urlunsplit((parts.scheme, host, parts.path, parts.query, parts.fragment)), True + + def ensure_clean_remote(self, repo_name: str) -> Dict[str, Any]: + """מוודא ש-``remote.origin.url`` של המראה אינו נושא credentials, ומנקה אם כן. + + מחזיר ``{"status", "url", "had_credentials", "reason"}``: + + - ``clean`` — ה-URL כבר נקי. + - ``cleaned`` — היה בו userinfo, ``set-url`` רץ, **והקריאה החוזרת** מראה + URL נקי וזהה למה שנכתב. קוד היציאה של ``set-url`` לבדו אינו ראיה. + - ``failed`` — אחד השלבים לא אומת; ``reason`` אומר איזה. ``url`` הוא + ``None`` כשלא ידוע URL נקי. + + ה-URL נקרא דרך ``_run_git_command``, שמעביר את הפלט ב-``_sanitize_output``, + ולכן הטוקן אינו מגיע לשום דבר שהפונקציה מחזירה או רושמת. + """ + repo_name = str(repo_name or "").strip() + if not self._validate_repo_name(repo_name): + return {"status": "failed", "url": None, "had_credentials": False, "reason": "invalid_repo_name"} + repo_path = self._get_repo_path(repo_name) + + # ``GIT_DIR`` מצמיד את הפקודה לתיקיית המראה. בלעדיו, תיקייה שאינה ריפו + # גורמת ל-git לטפס לתיקיות שמעליה — ו-``set-url`` היה כותב ל-config של + # ריפו אחר לגמרי (נמדד: "not a git repository (or any of the parent + # directories)"). + pinned = {"GIT_DIR": str(repo_path)} + read = self._run_git_command(["git", *self._REMOTE_GET_URL], cwd=repo_path, timeout=10, extra_env=pinned) + if not read.success: + return {"status": "failed", "url": None, "had_credentials": False, "reason": "get_url_failed"} + clean_url, had_credentials = self._strip_userinfo(read.stdout) + if not had_credentials: + if not self._validate_repo_url(clean_url): + return {"status": "failed", "url": None, "had_credentials": False, "reason": "unexpected_url"} + return {"status": "clean", "url": clean_url, "had_credentials": False, "reason": None} + + if not self._validate_repo_url(clean_url): + return {"status": "failed", "url": None, "had_credentials": True, "reason": "unexpected_url"} + written = self._run_git_command( + ["git", *self._REMOTE_SET_URL, clean_url], cwd=repo_path, timeout=10, extra_env=pinned + ) + if not written.success: + return {"status": "failed", "url": None, "had_credentials": True, "reason": "set_url_failed"} + + reread = self._run_git_command(["git", *self._REMOTE_GET_URL], cwd=repo_path, timeout=10, extra_env=pinned) + if not reread.success: + return {"status": "failed", "url": None, "had_credentials": True, "reason": "reread_failed"} + now_url, still_has = self._strip_userinfo(reread.stdout) + if still_has or now_url != clean_url: + return {"status": "failed", "url": None, "had_credentials": True, "reason": "still_not_clean"} + return {"status": "cleaned", "url": clean_url, "had_credentials": True, "reason": None} + + def scrub_stored_credentials(self) -> Dict[str, Any]: + """מעבר על **כל** המראות: מנקה credentials מ-``remote.origin.url``, ושורת לוג אחת. + + רץ בעליית כל שירות (``start_credential_sweep``, ``scripts/sweep_mirror_credentials.py``), + כי מראה שאינה נמשכת לעולם לא מגיעה לניקוי שב-``fetch_updates``, וצילום + דיסק משוחזר מחזיר config ישן. כל תיקייה ``*.git`` נבדקת, גם כזו שאינה + מראה תקינה — היא תיספר כ-``failed`` עם הסיבה, לא תדולג בשקט. + + ``sources``: לכל מראה, מאיפה יגיע הטוקן שלה אם הריפו ידרוש הזדהות — + ``map``/``global``/``explicit``/``none``, או ``unknown`` כשה-URL לא ידוע. + """ + stats = {"checked": 0, "had_credentials": 0, "cleaned": 0, "failed": 0} + sources: Dict[str, str] = {} + for path in sorted(self.base_path.glob("*.git")): + if not path.is_dir(): + continue + name = path.name[: -len(".git")] + stats["checked"] += 1 + res = self.ensure_clean_remote(name) + if res["had_credentials"]: + stats["had_credentials"] += 1 + if res["status"] == "cleaned": + stats["cleaned"] += 1 + elif res["status"] == "failed": + stats["failed"] += 1 + logger.warning("mirror credential sweep: %s failed (%s)", name, res["reason"]) + sources[name] = self._token_and_source_for_url(res["url"])[1] if res["url"] else "unknown" + logger.info( + "mirror credential sweep: checked=%d had_credentials=%d cleaned=%d failed=%d sources=%s", + stats["checked"], + stats["had_credentials"], + stats["cleaned"], + stats["failed"], + json.dumps(sources, sort_keys=True), + ) + return {**stats, "sources": sources} + def delete_mirror(self, repo_name: str) -> Dict[str, Any]: """ מחיקת ה-mirror מהדיסק (תיקיית bare repo). @@ -962,6 +1216,43 @@ def get_mirror_info(self, repo_name: str) -> Optional[Dict[str, Any]]: # ========== SHA & Commits ========== + def detect_default_branch(self, repo_name: str) -> Dict[str, Optional[str]]: + """הענף הראשי של המראה: מה ש-HEAD מצביע עליו, **ורק אם הענף קיים** (#3479). + + מחזיר ``{"branch": <שם>, "reason": None}``, או ``{"branch": None, "reason": <קוד>}``. + + שתי הפקודות הן ``rev-parse``, שכבר ברשימה המותרת — בלי להרחיב אותה. + ``clone --mirror`` מציב את HEAD של המראה על הענף הראשי ב-GitHub. + + **מלכודת שנמדדה (git 2.43):** כש-HEAD מצביע על ענף שאינו קיים, + ``rev-parse --symbolic-full-name HEAD`` מדפיס ``HEAD`` עם קוד יציאה 0. + לכן מתקבל רק פלט שמתחיל ב-``refs/heads/``, ואחריו בדיקת קיום נפרדת. + """ + repo_name = str(repo_name or "").strip() + if not self._validate_repo_name(repo_name): + return {"branch": None, "reason": "invalid_repo_name"} + repo_path = self._get_repo_path(repo_name) + head = self._run_git_command( + ["git", "rev-parse", "--symbolic-full-name", "HEAD"], cwd=repo_path, timeout=10 + ) + if not head.success: + return {"branch": None, "reason": "head_unreadable"} + full = head.stdout.strip() + prefix = "refs/heads/" + if not full.startswith(prefix): + return {"branch": None, "reason": "head_not_a_branch"} + branch = full[len(prefix):] + if not self._validate_basic_ref(branch) or branch == "HEAD": + return {"branch": None, "reason": "unsupported_branch_name"} + exists = self._run_git_command( + ["git", "rev-parse", "--verify", "--quiet", f"{prefix}{branch}^{{commit}}"], + cwd=repo_path, + timeout=10, + ) + if not exists.success: + return {"branch": None, "reason": "head_branch_missing"} + return {"branch": branch, "reason": None} + def get_current_sha(self, repo_name: str, branch: str = "main") -> Optional[str]: """ קבלת SHA הנוכחי של branch @@ -3657,3 +3948,32 @@ def get_mirror_service() -> GitMirrorService: _mirror_service = GitMirrorService() return _mirror_service + +def sweep_stored_credentials() -> Optional[Dict[str, Any]]: + """ניקוי credentials מכל המראות בדיסק של השירות הזה (#3480). ``None`` אם אין תיקיית מראות. + + **לא יוצר את התיקייה.** ``GitMirrorService()`` עושה ``mkdir``, וכאן אין סיבה: + כשאין ``REPO_MIRROR_PATH`` בדיסק, אין מה לנקות — וכך גם ייבוא של + ``mcp_server.app`` בטסט אינו יוצר תיקייה במכונה. + """ + base = _default_mirror_base_path() + if not base.is_dir(): + logger.info("mirror credential sweep: no mirror directory at %s, nothing to check", base) + return None + return get_mirror_service().scrub_stored_credentials() + + +def start_credential_sweep() -> threading.Thread: + """מריץ את ``sweep_stored_credentials`` פעם אחת ברקע. נקרא מנקודת הכניסה של שירות ה-MCP.""" + + def _run() -> None: + try: + sweep_stored_credentials() + except Exception: + # thread רקע: חריגה כאן לא מגיעה לאף קורא, ולכן נרשמת עם traceback + logger.warning("mirror credential sweep failed", exc_info=True) + + thread = threading.Thread(target=_run, daemon=True, name="mirror-credential-sweep") + thread.start() + return thread + diff --git a/services/repo_sync_service.py b/services/repo_sync_service.py index b05611c14..e7ad18f38 100644 --- a/services/repo_sync_service.py +++ b/services/repo_sync_service.py @@ -609,85 +609,20 @@ def initial_import(repo_url: str, repo_name: str, db: Any) -> Dict[str, Any]: repo_path = git_service._get_repo_path(repo_name) - # 2. זיהוי ה-Default Branch האמיתי - # ב-Mirror, HEAD מצביע על ה-default branch של origin - def _strip_origin_prefix(ref: str) -> str: - ref = str(ref or "").strip() - if ref.startswith("refs/remotes/origin/"): - return ref[len("refs/remotes/origin/"):] - if ref.startswith("origin/"): - return ref.split("/", 1)[1] - return ref - - def _ref_exists(ref: str) -> bool: - result = git_service._run_git_command( - ["git", "show-ref", "--verify", "--quiet", ref], - cwd=repo_path, - ) - return result.success - - def _branch_exists(branch: str) -> bool: - if not branch: - return False - return _ref_exists(f"refs/heads/{branch}") or _ref_exists(f"refs/remotes/origin/{branch}") - default_branch = "" - branch_result = git_service._run_git_command(["git", "symbolic-ref", "--short", "HEAD"], cwd=repo_path) - if branch_result.success: - default_branch = _strip_origin_prefix(branch_result.stdout.strip()) - if not _branch_exists(default_branch): - logger.info(f"HEAD pointed to missing branch: {default_branch}") - default_branch = "" - - # אם HEAD לא תקין, נסה origin/HEAD (נוצר ב-mirror) - if not default_branch: - head_result = git_service._run_git_command( - ["git", "symbolic-ref", "--short", "refs/remotes/origin/HEAD"], - cwd=repo_path, - ) - if head_result.success: - default_branch = _strip_origin_prefix(head_result.stdout.strip()) - if not _branch_exists(default_branch): - logger.info(f"origin/HEAD pointed to missing branch: {default_branch}") - default_branch = "" - - # Fallback: בחר ברנץ' ראשון מ-origin (עדיף main/master) - if not default_branch: - refs_result = git_service._run_git_command( - ["git", "for-each-ref", "--format=%(refname)", "refs/remotes/origin"], - cwd=repo_path, - ) - refs = [r.strip() for r in (refs_result.stdout or "").splitlines() if r.strip()] - preferred = ( - next((r for r in refs if r.endswith("/main")), "") - or next((r for r in refs if r.endswith("/master")), "") - ) - if not preferred: - preferred = next((r for r in refs if not r.endswith("/HEAD")), "") - default_branch = _strip_origin_prefix(preferred) - - # Fallback 2: ב-mirror אין refs/remotes/origin - חפש ישירות ב-refs/heads + # 2. זיהוי ה-Default Branch האמיתי — מה ש-HEAD של המראה מצביע עליו. + # **בלי ניחוש:** עד #3479 כל ניסיון כאן נדחה ב-``_run_git_command`` ("Unsupported + # git subcommand") והזיהוי נפל תמיד ל-``main``. כשאין ענף מאומת — כשל עם שם. + detected = git_service.detect_default_branch(repo_name) + default_branch = detected.get("branch") if not default_branch: - logger.info("No refs/remotes/origin found, checking refs/heads directly (mirror mode)") - refs_result = git_service._run_git_command( - ["git", "for-each-ref", "--format=%(refname)", "refs/heads"], - cwd=repo_path, - ) - refs = [r.strip() for r in (refs_result.stdout or "").splitlines() if r.strip()] - logger.info(f"Available refs/heads: {refs}") - preferred = ( - next((r for r in refs if r.endswith("/main")), "") - or next((r for r in refs if r.endswith("/master")), "") + logger.error( + "Could not detect default branch for %s (%s)", repo_name, detected.get("reason") ) - if not preferred: - preferred = next((r for r in refs if r.strip()), "") - if preferred: - # refs/heads/main -> main - default_branch = preferred.replace("refs/heads/", "") - logger.info(f"Found branch in refs/heads: {default_branch}") - - if not default_branch: - logger.warning("Could not detect any branch, falling back to 'main'") - default_branch = "main" + return { + "error": "default_branch_undetected", + "details": detected.get("reason"), + "repo_name": repo_name, + } logger.info(f"Detected default branch: {default_branch}") diff --git a/tests/test_git_mirror_credentials.py b/tests/test_git_mirror_credentials.py new file mode 100644 index 000000000..db8be8549 --- /dev/null +++ b/tests/test_git_mirror_credentials.py @@ -0,0 +1,517 @@ +"""טוקן ה-GitHub לא נשמר במראה ולא עובר בשורת הפקודה (#3480), וזיהוי הענף הראשי (#3479). + +**git אמיתי, שרת אמיתי.** הטסטים כאן לא מחליפים את ``subprocess.run``: עד #3480 +הטסטים של ``init_mirror`` החליפו אותו, ולכן לא ראו לא את ה-config של המראה ולא +את רשימת הפקודות המותרות ב-``_run_git_command`` — שני המקומות שבהם הבאגים ישבו. +כאן ``init_mirror``/``fetch_updates`` האמיתיים מדברים עם שרת HTTP מקומי שעוטף את +``git http-backend`` ודורש Basic auth לבעלים "פרטיים". ``GITHUB_HTTPS_ORIGIN`` +מוחלף בכתובת השרת — אותו קבוע שממנו נגזרים הבדיקה של הכתובת ומפתח הכותרת. + +הטוקנים בדויים. +""" + +from __future__ import annotations + +import base64 +import logging +import os +import subprocess +import sys +import threading +import time +from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer +from pathlib import Path +from typing import Any, Dict, List, Optional + +import pytest + +from services import git_mirror_service as gms +from services.git_mirror_service import GitCommandResult, GitMirrorService + +MAP_TOKEN = "ghp_TESTMAP0123456789abcdefghijABCDEFGH" +GLOBAL_TOKEN = "ghp_TESTGLOBAL0123456789abcdefghijABCD" +PRIVATE_OWNERS = {"mapped", "unmapped"} + + +class _GitServer: + """``git http-backend`` מאחורי Basic auth, עם רישום של כל בקשה.""" + + def __init__(self, root: Path) -> None: + self.root = root + self.requests: List[Dict[str, Any]] = [] + self.delay = 0.0 + self._lock = threading.Lock() + server = self + + class Handler(BaseHTTPRequestHandler): + def log_message(self, fmt: str, *args: Any) -> None: # שקט בפלט של pytest + return + + def _handle(self) -> None: + path, _, query = self.path.partition("?") + owner = path.strip("/").split("/", 1)[0] + auth = self.headers.get("Authorization") + password: Optional[str] = None + user: Optional[str] = None + if auth and auth.startswith("Basic "): + user, _, password = base64.b64decode(auth[6:]).decode().partition(":") + with server._lock: + server.requests.append({"owner": owner, "user": user, "password": password}) + if owner in PRIVATE_OWNERS and password not in (MAP_TOKEN, GLOBAL_TOKEN): + body = b"auth required\n" + self.send_response(401) + self.send_header("WWW-Authenticate", 'Basic realm="test"') + self.send_header("Content-Length", str(len(body))) + self.end_headers() + self.wfile.write(body) + return + if server.delay: + time.sleep(server.delay) + length = int(self.headers.get("Content-Length") or 0) + data = self.rfile.read(length) if length else b"" + env = { + "PATH": os.environ["PATH"], + "GIT_PROJECT_ROOT": str(server.root), + "GIT_HTTP_EXPORT_ALL": "1", + "PATH_INFO": path, + "QUERY_STRING": query, + "REQUEST_METHOD": self.command, + "CONTENT_TYPE": self.headers.get("Content-Type", ""), + "CONTENT_LENGTH": str(len(data)), + "REMOTE_ADDR": "127.0.0.1", + } + if self.headers.get("Git-Protocol"): + env["GIT_PROTOCOL"] = self.headers["Git-Protocol"] + if self.headers.get("Content-Encoding"): + env["HTTP_CONTENT_ENCODING"] = self.headers["Content-Encoding"] + out = subprocess.run(["git", "http-backend"], input=data, env=env, capture_output=True).stdout + sep, seplen = out.find(b"\r\n\r\n"), 4 + if sep == -1: + sep, seplen = out.find(b"\n\n"), 2 + head, payload = out[:sep], out[sep + seplen:] + code, headers = 200, [] + for line in head.decode("latin-1").splitlines(): + key, _, value = line.partition(":") + if key.lower() == "status": + code = int(value.strip().split()[0]) + elif key.strip(): + headers.append((key.strip(), value.strip())) + self.send_response(code) + for key, value in headers: + self.send_header(key, value) + self.send_header("Content-Length", str(len(payload))) + self.end_headers() + self.wfile.write(payload) + + do_GET = _handle + do_POST = _handle + + self.httpd = ThreadingHTTPServer(("127.0.0.1", 0), Handler) + self.origin = f"http://127.0.0.1:{self.httpd.server_address[1]}" + threading.Thread(target=self.httpd.serve_forever, daemon=True).start() + + def take(self) -> List[Dict[str, Any]]: + with self._lock: + out, self.requests = self.requests, [] + return out + + +class _World: + def __init__(self, tmp_path: Path, server: _GitServer, env: Dict[str, str]) -> None: + self.tmp = tmp_path + self.server = server + self.env = env + self.mirrors = tmp_path / "mirrors" + self.svc = GitMirrorService(base_path=str(self.mirrors)) + + def git(self, *args: str, cwd: Optional[Path] = None) -> str: + done = subprocess.run(["git", *args], cwd=cwd, env=self.env, check=True, capture_output=True, text=True) + return done.stdout.strip() + + def make_origin(self, owner: str, repo: str, branch: str = "main") -> str: + bare = self.server.root / owner / f"{repo}.git" + bare.parent.mkdir(parents=True, exist_ok=True) + self.git("init", "-q", "--bare", "-b", branch, str(bare)) + work = self.tmp / f"work-{owner}-{repo}" + self.git("init", "-q", "-b", branch, str(work)) + (work / "app.py").write_text("print('v1')\n", encoding="utf-8") + self.git("add", ".", cwd=work) + self.git("commit", "-q", "-m", "v1", cwd=work) + self.git("push", "-q", str(bare), branch, cwd=work) + return f"{self.server.origin}/{owner}/{repo}.git" + + def push_change(self, owner: str, repo: str, text: str, branch: str = "main") -> None: + work = self.tmp / f"work-{owner}-{repo}" + (work / "app.py").write_text(text, encoding="utf-8") + self.git("commit", "-q", "-am", text.strip(), cwd=work) + self.git("push", "-q", str(self.server.root / owner / f"{repo}.git"), branch, cwd=work) + + def legacy_mirror(self, owner: str, repo: str, token: str) -> Path: + """מראה כמו שהקוד יצר עד #3480: clone מ-URL שהטוקן בתוכו.""" + clean = f"{self.server.origin}/{owner}/{repo}.git" + with_token = clean.replace("http://", f"http://oauth2:{token}@") + target = self.mirrors / f"{repo}.git" + self.git("clone", "-q", "--mirror", "--", with_token, str(target)) + assert token in (target / "config").read_text(), "הכנת הטסט: המראה הישנה אמורה לשאת את הטוקן" + return target + + +@pytest.fixture +def world(tmp_path: Path, monkeypatch: pytest.MonkeyPatch): + home = tmp_path / "home" + home.mkdir() + for key in list(os.environ): + if key.lower() in ("http_proxy", "https_proxy", "all_proxy") or key.startswith("GIT_CONFIG"): + monkeypatch.delenv(key, raising=False) + for key in ("GITHUB_TOKEN", "GITHUB_TOKENS"): + monkeypatch.delenv(key, raising=False) + settings = { + "HOME": str(home), + "GIT_CONFIG_NOSYSTEM": "1", + "NO_PROXY": "127.0.0.1", + "no_proxy": "127.0.0.1", + "GIT_AUTHOR_NAME": "t", + "GIT_AUTHOR_EMAIL": "t@example.com", + "GIT_COMMITTER_NAME": "t", + "GIT_COMMITTER_EMAIL": "t@example.com", + } + for key, value in settings.items(): + monkeypatch.setenv(key, value) + server = _GitServer(tmp_path / "srv") + monkeypatch.setattr(gms, "GITHUB_HTTPS_ORIGIN", server.origin) + try: + yield _World(tmp_path, server, dict(os.environ)) + finally: + server.httpd.shutdown() + server.httpd.server_close() + + +def _files_with(root: Path, *needles: str) -> List[str]: + hits = [] + for f in sorted(root.rglob("*")): + if f.is_file(): + data = f.read_bytes() + if any(n.encode() in data for n in needles): + hits.append(str(f.relative_to(root))) + return hits + + +def _basic(token: str) -> str: + return base64.b64encode(f"oauth2:{token}".encode()).decode() + + +# --------------------------------------------------------------------------- +# #3480: הטוקן לא נשמר בדיסק +# --------------------------------------------------------------------------- + + +def test_init_mirror_leaves_the_token_in_no_file(world, monkeypatch): + """אחרי ``init_mirror`` של ריפו פרטי, אף קובץ במראה לא מכיל את הטוקן — לא בטקסט ולא ב-base64.""" + monkeypatch.setenv("GITHUB_TOKENS", f"mapped={MAP_TOKEN}") + url = world.make_origin("mapped", "secret-app") + + res = world.svc.init_mirror(url, "secret-app") + + assert res["success"] is True, res + assert res["auth_used"] == "map" + mirror = world.mirrors / "secret-app.git" + assert _files_with(mirror, MAP_TOKEN, _basic(MAP_TOKEN)) == [] + assert world.git("config", "--get", "remote.origin.url", cwd=mirror) == url + # והשרת אכן קיבל את ה-credential — כלומר האימות עבר, לא דולג + assert any(r["password"] == MAP_TOKEN and r["user"] == "oauth2" for r in world.server.take()) + + +@pytest.mark.skipif(not sys.platform.startswith("linux"), reason="סריקת /proc//cmdline קיימת רק בלינוקס") +def test_no_git_process_carries_the_token_on_its_command_line(world, monkeypatch): + """בזמן clone ו-fetch, הטוקן (גם ב-base64) לא מופיע בשורת הפקודה של אף תהליך. + + עד #3480 הוא הופיע ב-``git clone`` וב-``git-remote-http`` — git מעביר את ה-URL + המלא כארגומנט לתהליך העזר (``transport-helper.c``, git 2.43). + """ + monkeypatch.setenv("GITHUB_TOKENS", f"mapped={MAP_TOKEN}") + url = world.make_origin("mapped", "slow-app") + world.server.delay = 0.3 # מחזיק את התהליכים חיים מספיק זמן לסריקה + needles = (MAP_TOKEN.encode(), _basic(MAP_TOKEN).encode()) + seen: List[str] = [] + stop = threading.Event() + + def scan() -> None: + me = os.getpid() + while not stop.is_set(): + for pid in os.listdir("/proc"): + if not pid.isdigit() or int(pid) == me: + continue + try: + cmdline = Path(f"/proc/{pid}/cmdline").read_bytes() + except OSError: + continue + if any(n in cmdline for n in needles): + seen.append(cmdline.split(b"\0")[0].decode(errors="replace")) + time.sleep(0.002) + + scanner = threading.Thread(target=scan, daemon=True) + scanner.start() + try: + assert world.svc.init_mirror(url, "slow-app")["success"] is True + world.push_change("mapped", "slow-app", "print('v2')\n") + assert world.svc.fetch_updates("slow-app")["success"] is True + finally: + stop.set() + scanner.join() + assert seen == [] + # ולוודא שהסורק באמת ראה תהליכים מאומתים: השרת קיבל את הטוקן + assert any(r["password"] == MAP_TOKEN for r in world.server.take()) + + +def test_fetch_cleans_a_legacy_mirror_and_still_authenticates(world, monkeypatch): + """מראה מלפני #3480: ``fetch_updates`` מנקה את ה-URL, ה-fetch עובד, ואין טוקן באף קובץ.""" + monkeypatch.setenv("GITHUB_TOKENS", f"mapped={MAP_TOKEN}") + url = world.make_origin("mapped", "old-app") + mirror = world.legacy_mirror("mapped", "old-app", MAP_TOKEN) + world.server.take() + world.push_change("mapped", "old-app", "print('v2')\n") + + res = world.svc.fetch_updates("old-app") + + assert res["success"] is True, res + assert res["auth_used"] == "map" + assert world.git("config", "--get", "remote.origin.url", cwd=mirror) == url + assert _files_with(mirror, MAP_TOKEN, _basic(MAP_TOKEN)) == [] + # המצב, לא רק קוד היציאה: ה-commit החדש באמת הגיע + head = world.git("rev-parse", "refs/heads/main", cwd=mirror) + origin_head = world.git("rev-parse", "main", cwd=world.tmp / "work-mapped-old-app") + assert head == origin_head + + +def test_public_repo_never_receives_a_token(world, monkeypatch): + """ריפו ציבורי: גם כשמוגדרים טוקן במפה וטוקן גלובלי, אף בקשה לא נושאת ``Authorization``.""" + monkeypatch.setenv("GITHUB_TOKENS", f"openorg={MAP_TOKEN}") + monkeypatch.setenv("GITHUB_TOKEN", GLOBAL_TOKEN) + url = world.make_origin("openorg", "public-app") + + created = world.svc.init_mirror(url, "public-app") + world.push_change("openorg", "public-app", "print('v2')\n") + fetched = world.svc.fetch_updates("public-app") + + assert created["success"] and fetched["success"] + assert created["auth_used"] == fetched["auth_used"] == "none" + requests = world.server.take() + assert requests, "השרת אמור לקבל בקשות" + assert all(r["password"] is None for r in requests) + + +def test_owner_not_in_map_uses_the_global_token(world, monkeypatch, caplog): + """בעלים במפה ← הטוקן שלו; בעלים שאינו במפה ← ``GITHUB_TOKEN``. ושורות הלוג אומרות מה נשלח.""" + monkeypatch.setenv("GITHUB_TOKENS", f"mapped={MAP_TOKEN}") + monkeypatch.setenv("GITHUB_TOKEN", GLOBAL_TOKEN) + mapped = world.make_origin("mapped", "a") + unmapped = world.make_origin("unmapped", "b") + + with caplog.at_level(logging.INFO, logger="services.git_mirror_service"): + assert world.svc.init_mirror(mapped, "a")["auth_used"] == "map" + assert world.svc.init_mirror(unmapped, "b")["auth_used"] == "global" + + used = {(r["owner"], r["password"]) for r in world.server.take() if r["password"]} + assert used == {("mapped", MAP_TOKEN), ("unmapped", GLOBAL_TOKEN)} + assert "auth_used=global" in caplog.text + assert MAP_TOKEN not in caplog.text and GLOBAL_TOKEN not in caplog.text + + +def test_fetch_is_refused_when_the_url_cannot_be_cleaned(world, monkeypatch): + """כשהניקוי נכשל, ``fetch_updates`` לא מושך בכלל — אף בקשה לא מגיעה לשרת, והמראה לא משתנה.""" + monkeypatch.setenv("GITHUB_TOKENS", f"mapped={MAP_TOKEN}") + world.make_origin("mapped", "stuck") + mirror = world.legacy_mirror("mapped", "stuck", MAP_TOKEN) + world.server.take() + real = world.svc._run_git_command + + def set_url_fails(cmd, *args, **kwargs): + if cmd[1:3] == ["remote", "set-url"]: + return GitCommandResult(success=False, stdout="", stderr="injected", return_code=1) + return real(cmd, *args, **kwargs) + + monkeypatch.setattr(world.svc, "_run_git_command", set_url_fails) + + res = world.svc.fetch_updates("stuck") + + assert res["success"] is False + assert res["error_type"] == "mirror_url_not_clean" + assert world.server.take() == [] + # ולא דווח "נוקה" על משהו שלא נוקה: הטוקן עדיין שם, וזה מה שהניקוי הבא יתפוס + assert MAP_TOKEN in (mirror / "config").read_text() + + +def test_set_url_that_reports_success_without_cleaning_is_not_counted_as_cleaned(world, monkeypatch): + """קוד יציאה 0 מ-``set-url`` אינו ראיה. רק הקריאה החוזרת קובעת — ו-fetch לא רץ.""" + monkeypatch.setenv("GITHUB_TOKENS", f"mapped={MAP_TOKEN}") + world.make_origin("mapped", "liar") + mirror = world.legacy_mirror("mapped", "liar", MAP_TOKEN) + world.server.take() + real = world.svc._run_git_command + + def set_url_lies(cmd, *args, **kwargs): + if cmd[1:3] == ["remote", "set-url"]: + return GitCommandResult(success=True, stdout="", stderr="", return_code=0) + return real(cmd, *args, **kwargs) + + monkeypatch.setattr(world.svc, "_run_git_command", set_url_lies) + + assert world.svc.ensure_clean_remote("liar")["status"] == "failed" + assert world.svc.fetch_updates("liar")["error_type"] == "mirror_url_not_clean" + assert world.server.take() == [] + assert MAP_TOKEN in (mirror / "config").read_text() + + +def test_guard_stops_a_mirror_that_still_carries_credentials(world, monkeypatch): + """``transfer.credentialsInUrl=die``: פקודת רשת על מראה עם טוקן ב-URL נכשלת לפני בקשה, והטוקן לא בהודעה.""" + monkeypatch.setenv("GITHUB_TOKENS", f"mapped={MAP_TOKEN}") + url = world.make_origin("mapped", "guarded") + mirror = world.legacy_mirror("mapped", "guarded", MAP_TOKEN) + world.server.take() + + result, _ = world.svc._run_network_git(["git", "fetch", "--all", "--prune"], url, cwd=mirror) + + assert result.success is False + assert "plaintext credentials" in result.stderr + assert MAP_TOKEN not in result.stderr + assert world.server.take() == [] + + +def test_sweep_cleans_every_mirror_and_logs_counts_and_sources(world, monkeypatch, caplog): + """הניקוי בעלייה: כל ``*.git`` נבדקת, ספירה אחת בלוג, מקור הטוקן לכל מראה — בלי הטוקן עצמו.""" + monkeypatch.setenv("GITHUB_TOKENS", f"mapped={MAP_TOKEN}") + monkeypatch.setenv("GITHUB_TOKEN", GLOBAL_TOKEN) + world.make_origin("mapped", "legacy") + world.make_origin("unmapped", "fresh") + world.legacy_mirror("mapped", "legacy", MAP_TOKEN) + assert world.svc.init_mirror(f"{world.server.origin}/unmapped/fresh.git", "fresh")["success"] + (world.mirrors / "junk.git").mkdir() + + with caplog.at_level(logging.INFO, logger="services.git_mirror_service"): + stats = world.svc.scrub_stored_credentials() + + assert {k: stats[k] for k in ("checked", "had_credentials", "cleaned", "failed")} == { + "checked": 3, + "had_credentials": 1, + "cleaned": 1, + "failed": 1, + } + assert stats["sources"] == {"fresh": "global", "junk": "unknown", "legacy": "map"} + assert _files_with(world.mirrors, MAP_TOKEN, GLOBAL_TOKEN) == [] + assert "mirror credential sweep: checked=3 had_credentials=1 cleaned=1 failed=1" in caplog.text + assert MAP_TOKEN not in caplog.text and GLOBAL_TOKEN not in caplog.text + + +def test_sweep_never_touches_a_repository_above_the_mirrors_dir(world): + """תיקייה שאינה ריפו בתוך ריפו אחר: git היה מטפס למעלה ו-``set-url`` היה כותב ל-config שלו.""" + parent = world.tmp / "parent" + world.git("init", "-q", str(parent)) + # URL שעובר את ``_validate_repo_url`` — אחרת הטסט היה עובר גם בלי ההצמדה, + # כי ניקוי של URL לא תקין נדחה ממילא (נתפס בהרצת מוטציה) + secret_url = world.server.origin.replace("http://", f"http://oauth2:{MAP_TOKEN}@") + "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/x/parent.git" + world.git("remote", "add", "origin", secret_url, cwd=parent) + nested = GitMirrorService(base_path=str(parent / "mirrors")) + (parent / "mirrors" / "junk.git").mkdir() + + stats = nested.scrub_stored_credentials() + + assert stats["failed"] == 1 and stats["cleaned"] == 0 + assert world.git("config", "--get", "remote.origin.url", cwd=parent) == secret_url + + +def test_module_sweep_does_not_create_a_missing_mirror_dir(tmp_path, monkeypatch): + missing = tmp_path / "no-mirrors-here" + monkeypatch.setenv("REPO_MIRROR_PATH", str(missing)) + assert gms.sweep_stored_credentials() is None + assert not missing.exists() + + +# --------------------------------------------------------------------------- +# רשימת הפקודות המותרות: רק שתי צורות של ``git remote`` +# --------------------------------------------------------------------------- + + +def test_only_the_two_remote_shapes_are_allowed(world): + url = world.make_origin("openorg", "shapes") + assert world.svc.init_mirror(url, "shapes")["success"] + mirror = world.mirrors / "shapes.git" + run = world.svc._run_git_command + + assert run(["git", "remote", "get-url", "origin"], cwd=mirror).stdout.strip() == url + assert run(["git", "remote", "set-url", "origin", url], cwd=mirror).success + refused = [ + ["git", "remote", "add", "evil", url], + ["git", "remote", "set-url", "--push", "origin", url], + ["git", "remote", "set-url", "origin", url.replace("http://", f"http://oauth2:{MAP_TOKEN}@")], + ["git", "remote", "set-url", "origin", "https://example.com/o/r.git"], + ["git", "remote", "get-url", "origin", "--all"], + ["git", "-c", "http.extraHeader=x", "fetch", "--all"], + ["git", "config", "--get", "remote.origin.url"], + ] + for cmd in refused: + res = run(cmd, cwd=mirror) + assert (res.success, res.stderr) == (False, "Unsupported git subcommand"), cmd + assert world.git("config", "--get", "remote.origin.url", cwd=mirror) == url + + +# --------------------------------------------------------------------------- +# #3479: הענף הראשי מזוהה מ-HEAD של המראה, ובלי נפילה ל-main +# --------------------------------------------------------------------------- + + +def test_detect_default_branch_on_a_master_only_mirror(world): + url = world.make_origin("openorg", "legacy-master", branch="master") + assert world.svc.init_mirror(url, "legacy-master")["success"] + mirror = world.mirrors / "legacy-master.git" + assert world.git("for-each-ref", "--format=%(refname)", "refs/heads", cwd=mirror) == "refs/heads/master" + + assert world.svc.detect_default_branch("legacy-master") == {"branch": "master", "reason": None} + + # HEAD שמצביע על ענף שאינו קיים: rev-parse מדפיס "HEAD" עם קוד 0 — חייב להיות כשל עם שם + world.git("symbolic-ref", "HEAD", "refs/heads/gone", cwd=mirror) + detected = world.svc.detect_default_branch("legacy-master") + assert detected["branch"] is None and detected["reason"] + + +def test_initial_import_of_a_master_only_repo(world, monkeypatch): + """``initial_import`` מקצה לקצה, עם המראה האמיתית: ``default_branch`` נשמר כ-``master``.""" + from services import repo_sync_service as rss + + class _Metadata: + saved: Dict[str, Any] = {} + + def update_one(self, filt, update, upsert=False): + _Metadata.saved = update["$set"] + + class _Files: + def distinct(self, field, filt=None): + return [] + + def count_documents(self, filt): + return 1 + + class _Db: + repo_metadata = _Metadata() + repo_files = _Files() + + class _Indexer: + def __init__(self, db=None): + pass + + def should_index(self, path): + return True + + def index_file(self, repo_name, path, content, sha="HEAD"): + return True + + def remove_files(self, repo_name, paths): + return 0 + + url = world.make_origin("openorg", "old-style", branch="master") + monkeypatch.setattr(rss, "get_mirror_service", lambda: world.svc) + monkeypatch.setattr(rss, "CodeIndexer", _Indexer) + + out = rss.initial_import(url, "old-style", _Db()) + + assert out.get("status") == "completed", out + assert _Metadata.saved["default_branch"] == "master" diff --git a/tests/test_git_mirror_service.py b/tests/test_git_mirror_service.py index 80a34eea1..1c2094b0f 100644 --- a/tests/test_git_mirror_service.py +++ b/tests/test_git_mirror_service.py @@ -15,20 +15,38 @@ def __init__(self): self.stdout = "" self.stderr = "" - def _fake_run(cmd, cwd=None, capture_output=None, text=None, timeout=None): + seen = {} + + def _fake_run(cmd, cwd=None, capture_output=None, text=None, timeout=None, env=None): # Basic sanity that we run the expected command shape assert cmd[0:3] == ["git", "clone", "--mirror"] + seen["cmd"], seen["env"] = cmd, env return _Res() monkeypatch.setattr("services.git_mirror_service.subprocess.run", _fake_run) result = service.init_mirror("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/octocat/Hello-World.git", "test-repo") assert result["success"] is True + # ה-clone נעשה מה-URL הנקי — הוא שנשמר ב-remote.origin.url (#3480) + assert "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/octocat/Hello-World.git" in seen["cmd"] + # פקודת רשת: בלי הנחיות מסוף, ועם השומר שעוצר URL עם credentials + assert seen["env"]["GIT_TERMINAL_PROMPT"] == "0" + assert ("transfer.credentialsInUrl", "die") in { + (seen["env"][f"GIT_CONFIG_KEY_{i}"], seen["env"][f"GIT_CONFIG_VALUE_{i}"]) + for i in range(int(seen["env"]["GIT_CONFIG_COUNT"])) + } def test_should_classify_errors(service): assert service._classify_git_error("Could not resolve host") == "network_error" assert service._classify_git_error("Authentication failed") == "auth_error" + # מה ש-git עונה מאז #3480 כשריפו דורש הזדהות ואין טוקן (נמדד מול github.com) + assert ( + service._classify_git_error( + "fatal: could not read Username for 'https://github.com': terminal prompts disabled" + ) + == "auth_error" + ) def test_init_mirror_existing_invalid_mirror_is_cleaned_and_recloned(service, tmp_path, monkeypatch): @@ -45,7 +63,7 @@ def __init__(self, returncode=0, stdout="", stderr=""): self.stdout = stdout self.stderr = stderr - def _fake_run(cmd, cwd=None, capture_output=None, text=None, timeout=None): + def _fake_run(cmd, cwd=None, capture_output=None, text=None, timeout=None, env=None): if cmd[:3] == ["git", "rev-parse", "--is-bare-repository"]: calls["rev_parse"] += 1 return _Res(returncode=1, stdout="", stderr="fatal: not a git repository") @@ -233,14 +251,35 @@ def test_invalid_json_token_map_is_ignored(service, monkeypatch): assert service._token_for_url("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/Campaign-AI4U/campaign-ai.git") == "ghp_GLOBAL" -def test_authenticated_url_injects_per_owner_token(service, monkeypatch): +def test_token_source_per_owner_and_header_env(service, monkeypatch): + """מקור הטוקן לכל בעלים, והכותרת שנבנית ממנו — ממוקדת ל-origin של GitHub ובלי טוקן ב-URL (#3480).""" + import base64 + + from services import git_mirror_service as gms + monkeypatch.setenv("GITHUB_TOKENS", "Campaign-AI4U=ghp_AAA") monkeypatch.setenv("GITHUB_TOKEN", "ghp_GLOBAL") - url = service._get_authenticated_url("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/Campaign-AI4U/campaign-ai.git") - assert url == "https://oauth2:ghp_AAA@github.com/Campaign-AI4U/campaign-ai.git" + assert service._token_and_source_for_url("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/Campaign-AI4U/campaign-ai.git") == ("ghp_AAA", "map") # ארגון לא ממופה → הטוקן הגלובלי - url2 = service._get_authenticated_url("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/Zzz/repo.git") - assert url2 == "https://oauth2:ghp_GLOBAL@github.com/Zzz/repo.git" + assert service._token_and_source_for_url("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/Zzz/repo.git") == ("ghp_GLOBAL", "global") + monkeypatch.delenv("GITHUB_TOKEN") + assert service._token_and_source_for_url("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/Zzz/repo.git") == (None, "none") + + env = gms.GitMirrorService._network_env("ghp_AAA") + pairs = {env[f"GIT_CONFIG_KEY_{i}"]: env[f"GIT_CONFIG_VALUE_{i}"] for i in range(int(env["GIT_CONFIG_COUNT"]))} + expected = "Authorization: Basic " + base64.b64encode(b"oauth2:ghp_AAA").decode() + assert pairs["http.https://github.com/.extraHeader"] == expected + assert pairs["transfer.credentialsInUrl"] == "die" + # בלי טוקן — אין כותרת בכלל, רק השומר + bare = gms.GitMirrorService._network_env(None) + assert bare["GIT_CONFIG_COUNT"] == "1" + + +def test_https_branch_for_any_host_is_gone(service): + """עד #3480 הייתה הזרקה של טוקן GitHub לכל כתובת HTTPS. הכתובת עצמה כבר לא נוגעת בטוקן.""" + assert not hasattr(service, "_get_authenticated_url") + assert not service._validate_repo_url("https://github.com.evil.net/o/r.git") + assert not service._validate_repo_url("https://oauth2:x@github.com/o/r.git") def test_constructor_token_overrides_map(tmp_path, monkeypatch): diff --git a/tests/test_repo_sync_service.py b/tests/test_repo_sync_service.py index 97e4dd54f..4d033d56f 100644 --- a/tests/test_repo_sync_service.py +++ b/tests/test_repo_sync_service.py @@ -93,10 +93,11 @@ def init_mirror(self, repo_url: str, repo_name: str): def _get_repo_path(self, repo_name: str): return f"/tmp/{repo_name}.git" + def detect_default_branch(self, repo_name: str): + # הזיהוי האמיתי נבדק מול git אמיתי ב-tests/test_git_mirror_credentials.py + return {"branch": self._head_branch, "reason": None} + def _run_git_command(self, cmd, cwd=None, timeout=60): - # HEAD branch detection - if cmd[:3] == ["git", "symbolic-ref", "--short"]: - return _GitCommandResult(success=True, stdout=self._head_branch) # SHA resolution fallback if cmd[:2] == ["git", "rev-parse"]: # return a stable SHA for tests From 0df2314bc8f4cff54f1aee1009d3b003100ddca7 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 4 Oct 2026 08:31:36 +0000 Subject: [PATCH 2/2] =?UTF-8?q?fix(mirrors):=20=D7=A0=D7=99=D7=A7=D7=95?= =?UTF-8?q?=D7=99=20=D7=94-credentials=20=D7=A8=D7=A5=20=D7=9E=D7=94-lifes?= =?UTF-8?q?pan=20=D7=95=D7=9C=D7=90=20=D7=91=D7=99=D7=99=D7=91=D7=95=D7=90?= =?UTF-8?q?,=20=D7=95=D7=AA=D7=99=D7=A7=D7=95=D7=A0=D7=99=20=D7=A8=D7=99?= =?UTF-8?q?=D7=95=D7=95=D7=99=D7=95?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - mcp_server: הניקוי מוצמד ל-router.lifespan_context (attach_credential_sweep); import של mcp_server.app כבר לא נוגע במראות. טסט בתהליך נקי: אחרי ייבוא הטוקן עדיין שם, אחרי lifespan הוא נוקה. - הכול סביב credentials עבר ל-services/mirror_credentials.py; בשירות נשארו בחירת הטוקן ו-_run_network_git, שמקבל retry_cleanup מפורש במקום לפענח את cmd. - strip_userinfo לא מפיל את המעבר על URL ש-urlsplit דוחה (unparseable_url). - GIT_DIR מוחלט: base_path יחסי לא שובר יותר את הניקוי. - detect_default_branch בודק את refs/heads/ כמו הצרכנים (שמות כמו _main). - טסט שמריץ את scripts/start_webapp.sh עם gunicorn מדומה: ניקוי, לוג, וכשל שאינו מפיל את השירות. - תיעוד: נקי רק כש-failed=0; אזהרת פריסה על צילומי Render ו-rollback; המדריך לא מציג יותר הזרקה של טוקן ל-URL. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01CeuxN4qG2BdcVSdrbmHKCF --- GUIDES/REPO_SYNC_ENGINE_GUIDE.md | 53 ++--- docs/mcp-server.rst | 6 +- docs/whats-new.rst | 1 + mcp_server/app.py | 33 +-- mcp_server/repo_autosync.py | 42 ++++ scripts/sweep_mirror_credentials.py | 6 +- services/git_mirror_service.py | 259 ++++-------------------- services/mirror_credentials.py | 243 ++++++++++++++++++++++ tests/test_git_mirror_credentials.py | 116 ++++++++++- tests/test_git_mirror_service.py | 6 +- tests/test_start_webapp_mirror_sweep.py | 92 +++++++++ 11 files changed, 563 insertions(+), 294 deletions(-) create mode 100644 services/mirror_credentials.py create mode 100644 tests/test_start_webapp_mirror_sweep.py diff --git a/GUIDES/REPO_SYNC_ENGINE_GUIDE.md b/GUIDES/REPO_SYNC_ENGINE_GUIDE.md index bbc236ad7..a81b264a0 100644 --- a/GUIDES/REPO_SYNC_ENGINE_GUIDE.md +++ b/GUIDES/REPO_SYNC_ENGINE_GUIDE.md @@ -152,7 +152,7 @@ print(f"Disk writable: {os.access(mirror_path, os.W_OK)}") ## מימוש GitMirrorService -> ⚠️ **מיושן באימות מול GitHub (#3480).** הקוד שלמטה מזריק את הטוקן ל-URL (`_get_authenticated_url`), ו-git שומר את ה-URL הזה בטקסט גלוי בקובץ ה-`config` של המראה. הפונקציה הוסרה מהקוד: הטוקן עובר היום ככותרת דרך משתני הסביבה של git, וה-URL השמור נקי. המימוש הנוכחי — `_network_env`, `_run_network_git` ו-`ensure_clean_remote` ב-`services/git_mirror_service.py`; ההסבר — העמוד `docs/mcp-server.rst`, הסעיף "אימות מול GitHub — הטוקן לא נשמר במראה". אל תעתיקו את דפוס ההזרקה מכאן. +> ℹ️ **אימות מול GitHub (#3480).** הקוד שלמטה הוא המדריך המקורי, והוא אינו מסונכרן עם הקוד בכל פרט. בחלק של האימות הוא עודכן: הטוקן לא נכנס ל-URL, כי git שומר את ה-URL בטקסט גלוי בקובץ ה-`config` של המראה. המימוש — `services/mirror_credentials.py` ו-`GitMirrorService._run_network_git`; ההסבר — `docs/mcp-server.rst`, הסעיף "אימות מול GitHub — הטוקן לא נשמר במראה". צור קובץ `services/git_mirror_service.py`: @@ -222,38 +222,11 @@ class GitMirrorService: ) self._ensure_base_path() - def _get_authenticated_url(self, url: str) -> str: - """ - הזרקת GitHub Token ל-URL לתמיכה ב-Private Repos - - Args: - url: URL מקורי של הריפו - - Returns: - URL עם token (אם קיים) או URL מקורי - - Note: - לא לרשום את ה-URL המאומת ללוגים! - """ - token = os.getenv("GITHUB_TOKEN") - - if not token: - return url - - # תמיכה ב-HTTPS URLs בלבד - if url.startswith("/"): - # https://github.com/user/repo.git - # -> https://oauth2:TOKEN@github.com/user/repo.git - return url.replace( - "/", - f"https://oauth2:{token}@github.com/" - ) - elif url.startswith("https://"): - # Generic HTTPS URL - return url.replace("https://", f"https://oauth2:{token}@") - - return url - + # אימות מול GitHub — **לא** בתוך ה-URL (#3480): git שומר את ה-URL שה-clone + # נעשה ממנו ב-config של המראה, בטקסט גלוי. הטוקן עובר לכל clone/fetch ככותרת, + # דרך משתני הסביבה של git. המימוש: services/mirror_credentials.py + # (network_env, ensure_clean_remote) ו-GitMirrorService._run_network_git. + def _sanitize_output(self, text: str) -> str: """ מחיקת טוקנים רגישים מפלט/לוגים @@ -400,13 +373,13 @@ class GitMirrorService: # לוג ללא ה-token! logger.info(f"Creating mirror: {repo_url} -> {repo_path}") - # הזרקת token ל-Private Repos - auth_url = self._get_authenticated_url(repo_url) - - # Clone as bare mirror - result = self._run_git_command( - ["git", "clone", "--mirror", auth_url, str(repo_path)], - timeout=timeout + # Clone as bare mirror, מה-URL הנקי — הוא שנשמר ב-remote.origin.url. + # ריפו פרטי מקבל את הטוקן ככותרת בלבד (_run_network_git, #3480). + result, auth_used = self._run_network_git( + ["git", "clone", "--mirror", "--", repo_url, str(repo_path)], + repo_url, + timeout=timeout, + retry_cleanup=repo_path, ) if result.success: diff --git a/docs/mcp-server.rst b/docs/mcp-server.rst index 1b588f60f..421a69d41 100644 --- a/docs/mcp-server.rst +++ b/docs/mcp-server.rst @@ -1743,17 +1743,17 @@ Claude Desktop אימות מול GitHub — הטוקן לא נשמר במראה ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -**ה-URL שבמראה תמיד נקי.** עד #3480 הטוקן הוזרק ל-URL של ה-clone, ו-git שמר אותו בטקסט גלוי ב-``remote.origin.url`` שבקובץ ``config`` של המראה — על הדיסק של הוובאפ, על הדיסק של שירות ה-MCP, ובצילומי הדיסק של Render. היום ה-clone נעשה מה-URL הנקי, והטוקן עובר לכל ``clone``/``fetch`` בנפרד ככותרת ``Authorization``, דרך משתני הסביבה ``GIT_CONFIG_COUNT``/``GIT_CONFIG_KEY_``/``GIT_CONFIG_VALUE_`` ולא בשורת הפקודה (``_network_env`` ב-``services/git_mirror_service.py``). הכותרת ממוקדת ל-``GITHUB_HTTPS_ORIGIN`` בלבד. +**מראה חדשה נוצרת עם URL נקי, ומראה ישנה מנוקה.** עד #3480 הטוקן הוזרק ל-URL של ה-clone, ו-git שמר אותו בטקסט גלוי ב-``remote.origin.url`` שבקובץ ``config`` של המראה — על הדיסק של הוובאפ, על הדיסק של שירות ה-MCP, ובצילומי הדיסק של Render. היום ה-clone נעשה מה-URL הנקי, והטוקן עובר לכל ``clone``/``fetch`` בנפרד ככותרת ``Authorization``, דרך משתני הסביבה ``GIT_CONFIG_COUNT``/``GIT_CONFIG_KEY_``/``GIT_CONFIG_VALUE_`` ולא בשורת הפקודה (``network_env`` ב-``services/mirror_credentials.py``). הכותרת ממוקדת ל-``GITHUB_HTTPS_ORIGIN`` בלבד. מראה ישנה נשארת עם הטוקן בדיסק **עד שהניקוי שלה מצליח** — ראו למטה; עד אז ה-fetch שלה חסום. **איזה טוקן, ולמי.** הטוקן נבחר לפי בעלי הריפו: מהמפה ב-``GITHUB_TOKENS``, ובעלים שאינו במפה מקבל את ``GITHUB_TOKEN`` (``_token_and_source_for_url``). כל פקודת רשת נשלחת קודם **בלי** טוקן, ורק כש-GitHub עונה שהריפו דורש הזדהות היא נשלחת שוב עם הכותרת — כך שריפו ציבורי לא מקבל טוקן אף פעם (``_run_network_git``). שורות הלוג ``Mirror clone``/``Mirror fetch`` מציינות ``auth_used``: ``none``, ``map``, ``global`` או ``explicit``. -**ניקוי מראות ישנות.** ``ensure_clean_remote`` מוציא credentials מה-URL השמור ב-``git remote set-url``, ומאמת בקריאה חוזרת של ה-URL — לא לפי קוד היציאה. הוא רץ לפני כל ``fetch_updates`` (אם הניקוי לא אומת, ה-fetch לא רץ ומוחזר ``mirror_url_not_clean``), וגם על כל המראות בעליית כל שירות: בשירות ה-MCP מתוך ``create_app``, ובוובאפ מתוך ``scripts/start_webapp.sh``. שורת הלוג של המעבר הזה היא האימות שהמראות נקיות: +**ניקוי מראות ישנות.** ``ensure_clean_remote`` מוציא credentials מה-URL השמור ב-``git remote set-url``, ומאמת בקריאה חוזרת של ה-URL — לא לפי קוד היציאה. הוא רץ לפני כל ``fetch_updates`` (אם הניקוי לא אומת, ה-fetch לא רץ ומוחזר ``mirror_url_not_clean``), וגם על כל המראות בעליית כל שירות: בשירות ה-MCP מה-lifespan של האפליקציה (``attach_credential_sweep`` ב-``mcp_server/repo_autosync.py`` — לא בייבוא של ``mcp_server.app``), ובוובאפ מתוך ``scripts/start_webapp.sh``. שורת הלוג של המעבר הזה היא האימות: .. code-block:: text mirror credential sweep: checked= had_credentials= cleaned= failed= sources={"": "map", ...} -``sources`` אומר לכל מראה מאיפה יגיע הטוקן שלה אם הריפו ידרוש הזדהות, בלי הטוקן עצמו — כך רואים אילו מראות נשענות על ``GITHUB_TOKEN``. שורה שלא מופיעה בלוג של אחד השירותים פירושה שהמעבר לא רץ שם. +**המראות נקיות רק כש-``failed=0``.** מראה שנספרה ב-``failed`` עלולה עדיין להחזיק את הטוקן ב-``config``; לכל אחת מהן יש שורת ``mirror credential sweep: failed ()`` עם הסיבה, והניסיון חוזר לפני ה-fetch הבא שלה ובעלייה הבאה. ``sources`` אומר לכל מראה מאיפה יגיע הטוקן שלה אם הריפו ידרוש הזדהות, בלי הטוקן עצמו — כך רואים אילו מראות נשענות על ``GITHUB_TOKEN``. שורה שלא מופיעה בלוג של אחד השירותים פירושה שהמעבר לא רץ שם. **שומר.** כל פקודת רשת רצה עם ``transfer.credentialsInUrl=die``: מראה שעדיין נושאת credentials ב-URL נכשלת לפני בקשת רשת, ו-git עצמו מסתיר את הסיסמה בהודעה. ``git remote`` מותר ב-``_run_git_command`` רק בשתי הצורות ``get-url origin`` ו-``set-url origin ``, וה-URL חייב לעבור את ``_validate_repo_url``. diff --git a/docs/whats-new.rst b/docs/whats-new.rst index e8aec3300..1966500c5 100644 --- a/docs/whats-new.rst +++ b/docs/whats-new.rst @@ -5,6 +5,7 @@ What's New 2026-10-04 ---------- - **fix (מראות): טוקן ה-GitHub לא נשמר יותר בקובץ ה-config של המראות, ולא עובר בשורת הפקודה (#3480); וזיהוי הענף הראשי בייבוא ריפו (#3479).** עד עכשיו ``init_mirror`` שכפל מ-URL שהטוקן בתוכו, ו-git שמר אותו בטקסט גלוי ב-``remote.origin.url`` — על הדיסקים של הוובאפ ושל שירות ה-MCP, ובצילומים היומיים של Render; הוא גם הופיע בשורת הפקודה של ``git-remote-http`` בכל clone ו-fetch. עכשיו ה-URL השמור נקי, והטוקן עובר ככותרת דרך משתני סביבה של git, ורק לריפו שדורש הזדהות — ריפו ציבורי לא מקבל טוקן. מראות קיימות מנוקות לפני כל fetch ובעליית כל שירות, עם שורת לוג אחת שסופרת מה נמצא ומה נוקה, ואומרת לכל מראה מאיפה הטוקן שלה (``map``/``global``). fetch על מראה שלא אומת שהיא נקייה לא רץ (``mirror_url_not_clean``). ובנפרד: ``initial_import`` ניסה לזהות את הענף הראשי בפקודות ש-``_run_git_command`` דוחה, ולכן תמיד נפל ל-``main`` — וריפו שהענף הראשי שלו ``master`` לא יובא. עכשיו הזיהוי הוא מ-HEAD של המראה, וכשאין ענף מאומת מוחזר ``default_branch_undetected`` במקום ניחוש. ראו :ref:`mcp-mirror-credentials`. +- **לפריסה של התיקון הזה:** הניקוי מוציא את הטוקן מהדיסק, אבל צילומי הדיסק של Render שנלקחו לפניו נשמרים לפחות שבעה ימים ועדיין מחזיקים אותו — החלפת הטוקן היא ההחלטה שסוגרת גם אותם. ו-rollback לקוד שלפני התיקון אינו מחזיר את הטוקן למראות שכבר נוקו, ולכן ה-fetch שלהן לריפו פרטי ייכשל באימות; מראה כזו מתקנים במחיקה ושכפול מחדש. 2026-10-01 ---------- diff --git a/mcp_server/app.py b/mcp_server/app.py index 1b29697a7..6b883c4f4 100644 --- a/mcp_server/app.py +++ b/mcp_server/app.py @@ -154,18 +154,6 @@ def create_app(): logging.getLogger(__name__).warning("repo autosync failed to start", exc_info=True) - # מראות שנוצרו לפני #3480 נושאות את טוקן ה-GitHub ב-remote.origin.url. ניקוי - # אחד בעלייה, **בלי תלות ב-MCP_REPO_AUTOSYNC**: מראה שלא נמשכת לא תגיע - # לניקוי שב-fetch_updates. השורה "mirror credential sweep:" בלוג היא האימות. - try: - from services.git_mirror_service import start_credential_sweep - - start_credential_sweep() - except Exception: - import logging - - logging.getLogger(__name__).warning("mirror credential sweep failed to start", exc_info=True) - mcp_base = (os.getenv("MCP_SERVER_URL") or "").rstrip("/") webapp_base = (os.getenv("WEBAPP_URL") or "").rstrip("/") @@ -173,7 +161,7 @@ def create_app(): provider, settings, consent = _build_oauth( mongo, mcp_base=mcp_base, webapp_base=webapp_base ) - return build_app( + app = build_app( backend, auth_provider=provider, auth_settings=settings, @@ -184,11 +172,12 @@ def create_app(): rate_limit_per_minute=rate_limit_per_minute, public_url=mcp_base, ) + return _with_credential_sweep(app) # Fallback: PAT-only (Claude Code/Desktop) — runs without OAuth config. # ``MCP_SERVER_URL`` may still be set here without ``WEBAPP_URL``; when it is, # the upload command in the tool descriptions names the real host. - return build_app( + app = build_app( backend, MCPTokenStore(mongo), repo_backend=repo_backend, @@ -197,6 +186,22 @@ def create_app(): rate_limit_per_minute=rate_limit_per_minute, public_url=mcp_base or None, ) + return _with_credential_sweep(app) + + +def _with_credential_sweep(app): + """מראות שנוצרו לפני #3480 נושאות את טוקן ה-GitHub ב-remote.origin.url — ניקוי אחד בעליית השרת. + + **ב-lifespan, לא כאן:** ``create_app`` רץ בייבוא (השורה ``app = create_app()`` + למטה), ולכן כל מה שהוא מפעיל ישירות רץ גם בכל ``import mcp_server.app``. + ``attach_credential_sweep`` מצמיד את הניקוי לעליית ה-ASGI, שקורית רק כש-uvicorn + מגיש. **בלי תלות ב-``MCP_REPO_AUTOSYNC``:** מראה שלא נמשכת לא תגיע לניקוי שב- + ``fetch_updates``. השורה ``mirror credential sweep:`` בלוג היא האימות. + """ + from .repo_autosync import attach_credential_sweep + + attach_credential_sweep(app) + return app app = create_app() diff --git a/mcp_server/repo_autosync.py b/mcp_server/repo_autosync.py index 9e6318b6b..64f29775d 100644 --- a/mcp_server/repo_autosync.py +++ b/mcp_server/repo_autosync.py @@ -23,6 +23,7 @@ from __future__ import annotations +import contextlib import logging import os import re @@ -161,6 +162,47 @@ def refresh_once(db: Any, mirror: Any) -> dict[str, int]: return stats +def attach_credential_sweep(app: Any) -> bool: + """Run the mirror credential sweep (#3480) once, when the ASGI app starts. + + **At server startup, never at import.** ``mcp_server/app.py`` builds the app + at module level (``app = create_app()``), so anything ``create_app`` starts + directly also starts on every ``import mcp_server.app`` — in tests, tooling, + a REPL — and this sweep rewrites ``remote.origin.url`` on every mirror under + ``REPO_MIRROR_PATH``. Wrapping ``router.lifespan_context`` is the seam the + service already uses for startup work (:func:`mcp_server.server.attach_read_pool`, + :func:`mcp_server.analytics.attach_shutdown_drain`): uvicorn enters the + lifespan when it serves, an import does not. + + The sweep itself runs on a daemon thread + (:func:`services.mirror_credentials.start_credential_sweep`), so startup + does not wait for it. Returns False — and says so — when the app has no + lifespan to attach to; then the sweep does not run here, and the missing + ``mirror credential sweep:`` log line is the signal. + """ + router = getattr(app, "router", None) + original = getattr(router, "lifespan_context", None) + if router is None or original is None: + logger.warning("no lifespan on the ASGI app; the mirror credential sweep will not run") + return False + + @contextlib.asynccontextmanager + async def _lifespan_with_credential_sweep(scope_app: Any): + try: + from services.mirror_credentials import start_credential_sweep # lazy: only when serving + + start_credential_sweep() + except Exception: + # The service must still come up; fetches stay guarded by + # transfer.credentialsInUrl=die and the per-fetch cleaning. + logger.warning("mirror credential sweep failed to start", exc_info=True) + async with original(scope_app) as state: + yield state + + router.lifespan_context = _lifespan_with_credential_sweep + return True + + def start_autosync(db: Any, *, interval: int | None = None) -> bool: """Start the daemon refresher (idempotent). Returns True if it is running. diff --git a/scripts/sweep_mirror_credentials.py b/scripts/sweep_mirror_credentials.py index 5a2b18b3a..2a4877092 100644 --- a/scripts/sweep_mirror_credentials.py +++ b/scripts/sweep_mirror_credentials.py @@ -2,8 +2,8 @@ """ניקוי טוקני GitHub מה-``remote.origin.url`` של כל המראות בדיסק של השירות (#3480). ``scripts/start_webapp.sh`` מריץ אותו בעליית הוובאפ, ברקע ואחרי ש-Gunicorn כבר -עלה. בשירות ה-MCP אותו ניקוי רץ מתוך ``create_app`` (``start_credential_sweep``). -העבודה עצמה ב-``services.git_mirror_service.sweep_stored_credentials``; כאן רק +עלה. בשירות ה-MCP אותו ניקוי רץ מה-lifespan של האפליקציה (``attach_credential_sweep``). +העבודה עצמה ב-``services.mirror_credentials.sweep_stored_credentials``; כאן רק נקודת כניסה, שמדפיסה את שורת הלוג ``mirror credential sweep: ...`` לפלט של השירות. קוד יציאה: 0 כשאין כשלים (כולל "אין תיקיית מראות"), 1 כשמראה אחת לפחות לא נוקתה. @@ -21,7 +21,7 @@ def main() -> int: # Make the repo importable when run directly. sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) - from services.git_mirror_service import sweep_stored_credentials # noqa: E402 + from services.mirror_credentials import sweep_stored_credentials # noqa: E402 stats = sweep_stored_credentials() if stats is None: diff --git a/services/git_mirror_service.py b/services/git_mirror_service.py index d830b1264..c1cc65884 100644 --- a/services/git_mirror_service.py +++ b/services/git_mirror_service.py @@ -11,7 +11,6 @@ from __future__ import annotations -import base64 import codecs import functools import json @@ -20,7 +19,6 @@ import re import select import subprocess -import threading import time from collections import deque import shutil @@ -28,47 +26,11 @@ from datetime import datetime from pathlib import Path from typing import Any, Dict, List, Optional, Set, Tuple -from urllib.parse import urlsplit, urlunsplit -logger = logging.getLogger(__name__) +# אימות מול GitHub בלי טוקן ב-URL, וניקוי מראות ישנות (#3480) +from services import mirror_credentials as _creds -# ---- אימות מול GitHub בלי לשמור את הטוקן בדיסק (#3480) ------------------ -# -# עד #3480 הטוקן הוזרק ל-URL (``https://oauth2:@github.com/...``), ו-git -# שומר את ה-URL שה-clone נעשה ממנו ב-``remote.origin.url`` בקובץ ``config`` של -# המראה — בטקסט גלוי, על הדיסק של הוובאפ ושל שירות ה-MCP ובצילומים היומיים של -# Render. git גם מעביר את ה-URL המלא כארגומנט ל-``git-remote-https``, כך שהוא -# גלוי ברשימת התהליכים בכל clone ו-fetch. מאז, ה-URL שנשמר תמיד נקי, והטוקן -# עובר לכל פקודת רשת בנפרד כ**כותרת**, דרך משתני הסביבה ``GIT_CONFIG_COUNT`` / -# ``GIT_CONFIG_KEY_`` / ``GIT_CONFIG_VALUE_`` (git 2.31 ומעלה, -# ``Documentation/git-config.txt``) — לא דרך ``git -c``, שמציב את הערך בשורת -# הפקודה. התיעוד של git עצמו מצביע על ``http.extraHeader`` כמקרה שבו העברה -# דרך הסביבה עדיפה (``Documentation/git.txt``, ``--config-env``). -# -# **המקור (origin) של GitHub.** ממנו נגזרים גם הבדיקה של כתובת ריפו -# (``_validate_repo_url``), גם חילוץ הבעלים, וגם המפתח שאליו הכותרת ממוקדת — -# git משווה את המפתח לכתובת לפי scheme, host ו-port (``Documentation/config/http.txt``, -# ``http..*``), ולכן הכותרת לא נשלחת לשום מארח אחר. הטסטים מחליפים את -# הקבוע כדי להריץ את ``init_mirror``/``fetch_updates`` האמיתיים מול שרת מקומי. -GITHUB_HTTPS_ORIGIN = "https://github.com" - -# שם המשתמש שנשלח עם הטוקן. אותו credential בדיוק שה-URL נשא עד #3480. -GIT_AUTH_USERNAME = "oauth2" - -# ``transfer.credentialsInUrl=die`` (git 2.37 ומעלה): מראה ששוב נושאת -# credentials ב-URL נכשלת **לפני** בקשת רשת, וההודעה של git כבר מסתירה את -# הסיסמה (````). זה השומר שהופך "מראה שלא נוקתה" לשגיאה עם שם, -# במקום fetch שקט עם הטוקן השמור. -_CREDENTIALS_IN_URL_GUARD: Tuple[str, str] = ("transfer.credentialsInUrl", "die") - -# הודעות git כשהשרת דורש הזדהות ואין מה לתת לו. נאכפות באנגלית דרך -# ``LC_ALL=C`` בסביבת פקודות הרשת. ``could not read Username`` נבדק מול -# github.com עצמו, על ריפו פרטי/לא קיים בלי טוקן (git 2.43, אוקטובר 2026). -_AUTH_REQUIRED_MARKERS = ( - "could not read username", - "terminal prompts disabled", - "authentication failed", -) +logger = logging.getLogger(__name__) # הגדרות קבועות MAX_DIFF_BYTES = 1 * 1024 * 1024 # 1MB @@ -456,16 +418,12 @@ class GitMirrorService: r'^[a-zA-Z0-9][a-zA-Z0-9._/^~-]{0,150}$' ) - # הצורות של HTTPS נגזרות מ-``GITHUB_HTTPS_ORIGIN`` (``_https_patterns``). + # הצורות של HTTPS נגזרות מ-``mirror_credentials.GITHUB_HTTPS_ORIGIN`` (``_https_patterns``). _GITHUB_SSH_RE = re.compile(r"^git@github\.com:[^/\s]+/[^/\s]+(?:\.git)?$", re.IGNORECASE) # חילוץ בעלים (owner/org) מתוך URL של GitHub – לבחירת טוקן פר-ארגון _OWNER_SSH_RE = re.compile(r"^git@github\.com:([^/\s]+)/", re.IGNORECASE) - # ``git remote`` מותר **רק** בשתי הצורות האלה, ובדיוק במבנה הזה. כל השאר - # (``add``, ``remove``, ``set-url --push``...) נדחה כמו תת-פקודה לא מוכרת. - _REMOTE_GET_URL = ("remote", "get-url", "origin") - _REMOTE_SET_URL = ("remote", "set-url", "origin") @staticmethod @functools.lru_cache(maxsize=8) @@ -528,7 +486,7 @@ def _validate_repo_url(self, repo_url: str) -> bool: return False if url.startswith("-"): return False - https_repo, _ = self._https_patterns(GITHUB_HTTPS_ORIGIN) + https_repo, _ = self._https_patterns(_creds.GITHUB_HTTPS_ORIGIN) return bool(https_repo.fullmatch(url) or self._GITHUB_SSH_RE.fullmatch(url)) @staticmethod @@ -583,7 +541,7 @@ def _extract_owner(self, url: str) -> str: if not isinstance(url, str): return "" u = url.strip() - _, https_owner = self._https_patterns(GITHUB_HTTPS_ORIGIN) + _, https_owner = self._https_patterns(_creds.GITHUB_HTTPS_ORIGIN) m = https_owner.match(u) or self._OWNER_SSH_RE.match(u) return m.group(1) if m else "" @@ -616,39 +574,13 @@ def _token_and_source_for_url(self, url: str) -> Tuple[Optional[str], str]: token = os.getenv("GITHUB_TOKEN") or None return (token, "global") if token else (None, "none") - @staticmethod - def _network_env(token: Optional[str]) -> Dict[str, str]: - """הסביבה של פקודת רשת (clone/fetch): בלי הנחיות מסוף, עם השומר, ועם הכותרת אם יש טוקן. - - הכותרת היא אותו credential בדיוק שה-URL נשא עד #3480 (``oauth2:`` - ב-Basic), ממוקדת ל-``GITHUB_HTTPS_ORIGIN`` בלבד. base64 גם מנטרל כל תו - בטוקן שהיה שובר את הדקדוק של כותרת HTTP. - """ - pairs: List[Tuple[str, str]] = [_CREDENTIALS_IN_URL_GUARD] - if token: - basic = base64.b64encode(f"{GIT_AUTH_USERNAME}:{token}".encode("utf-8")).decode("ascii") - pairs.append( - (f"http.{GITHUB_HTTPS_ORIGIN.rstrip('/')}/.extraHeader", f"Authorization: Basic {basic}") - ) - env: Dict[str, str] = { - "GIT_TERMINAL_PROMPT": "0", - # הודעות git באנגלית: ``_needs_auth`` ו-``_classify_git_error`` מזהים אותן לפי הטקסט - "LC_ALL": "C", - "GIT_CONFIG_COUNT": str(len(pairs)), - } - for i, (key, value) in enumerate(pairs): - env[f"GIT_CONFIG_KEY_{i}"] = key - env[f"GIT_CONFIG_VALUE_{i}"] = value - return env - - @staticmethod - def _needs_auth(stderr: str) -> bool: - """האם git נכשל כי השרת דרש הזדהות (ריפו פרטי, או טוקן שלא התקבל).""" - text = (stderr or "").lower() - return any(marker in text for marker in _AUTH_REQUIRED_MARKERS) - def _run_network_git( - self, cmd: List[str], url: str, cwd: Optional[Path] = None, timeout: int = 60 + self, + cmd: List[str], + url: str, + cwd: Optional[Path] = None, + timeout: int = 60, + retry_cleanup: Optional[Path] = None, ) -> Tuple[GitCommandResult, str]: """מריץ clone/fetch, ומחזיר גם איזה טוקן נשלח בפועל: ``none``/``map``/``global``/``explicit``. @@ -658,19 +590,21 @@ def _run_network_git( **אין מסלול שמכניס את הטוקן ל-URL**, גם לא כשהכותרת נכשלת — זה בדיוק מה ש-#3480 הוציא. ``transfer.credentialsInUrl=die`` בשני הניסיונות. + + ``retry_cleanup``: תיקייה שהקורא יוצר בפקודה (יעד ה-clone ב-``init_mirror``) + ושיש להסיר לפני הניסיון השני, אם הראשון השאיר אותה. הקורא מעביר אותה + במפורש — הפונקציה אינה מפענחת את ``cmd``. """ - result = self._run_git_command(cmd, cwd=cwd, timeout=timeout, extra_env=self._network_env(None)) - if result.success or not self._needs_auth(result.stderr): + result = self._run_git_command(cmd, cwd=cwd, timeout=timeout, extra_env=_creds.network_env(None)) + if result.success or not _creds.needs_auth(result.stderr): return result, "none" token, source = self._token_and_source_for_url(url) if not token: return result, "none" - if cmd[1] == "clone": - # clone שנכשל עלול להשאיר תיקייה, וניסיון שני היה נכשל על "already exists" - target = Path(cmd[-1]) - if target.exists() and not self._safe_rmtree(target): - logger.warning("Could not remove partial clone before authenticated retry: %s", target) - result = self._run_git_command(cmd, cwd=cwd, timeout=timeout, extra_env=self._network_env(token)) + if retry_cleanup is not None and retry_cleanup.exists() and not self._safe_rmtree(retry_cleanup): + # הניסיון השני ייכשל על "already exists", והכשל הזה יחזור לקורא + logger.warning("Could not remove %s before the authenticated retry", retry_cleanup) + result = self._run_git_command(cmd, cwd=cwd, timeout=timeout, extra_env=_creds.network_env(token)) return result, source def _sanitize_output(self, output: str) -> str: @@ -787,9 +721,9 @@ def _is_allowed_remote_command(self, cmd: List[str]) -> bool: ``//`` — כלומר URL עם credentials לא יכול להיכתב דרך הנתיב הזה. """ - if tuple(cmd[1:4]) == self._REMOTE_GET_URL and len(cmd) == 4: + if tuple(cmd[1:4]) == _creds.REMOTE_GET_URL and len(cmd) == 4: return True - if tuple(cmd[1:4]) == self._REMOTE_SET_URL and len(cmd) == 5: + if tuple(cmd[1:4]) == _creds.REMOTE_SET_URL and len(cmd) == 5: return self._validate_repo_url(str(cmd[4])) return False @@ -808,7 +742,7 @@ def _run_git_command( cwd: תיקיית עבודה timeout: timeout בשניות extra_env: משתני סביבה שמתווספים לסביבת התהליך (פקודות רשת בלבד — - ``_network_env``). בלעדיו git יורש את הסביבה כמו קודם. + ``mirror_credentials.network_env``). בלעדיו git יורש את הסביבה כמו קודם. Returns: GitCommandResult עם התוצאות @@ -932,7 +866,10 @@ def init_mirror(self, repo_url: str, repo_name: str, timeout: int = 600) -> Dict # Clone as bare mirror, מה-URL הנקי — הוא שנשמר ב-remote.origin.url # שימוש ב-"--" כדי למנוע פרשנות של URL/נתיב כ-flag במקרה קצה result, auth_used = self._run_network_git( - ["git", "clone", "--mirror", "--", repo_url, str(repo_path)], repo_url, timeout=timeout + ["git", "clone", "--mirror", "--", repo_url, str(repo_path)], + repo_url, + timeout=timeout, + retry_cleanup=repo_path, ) logger.info("Mirror clone for %s: success=%s auth_used=%s", repo_name, result.success, auth_used) @@ -986,7 +923,7 @@ def fetch_updates(self, repo_name: str, timeout: int = 120) -> Dict[str, Any]: # מראה שנוצרה לפני #3480 נושאת את הטוקן ב-remote.origin.url. מנקים לפני # כל fetch; אם הניקוי לא אומת — לא מושכים בכלל (וגם השומר של git היה # עוצר את ה-fetch לפני בקשת רשת). - cleaning = self.ensure_clean_remote(repo_name) + cleaning = _creds.ensure_clean_remote(self, repo_name) if cleaning["status"] == "failed": logger.error( "Refusing to fetch %s: stored remote URL is not verified clean (%s)", @@ -1030,7 +967,7 @@ def _classify_git_error(self, stderr: str) -> str: if "could not resolve host" in stderr_lower: return "network_error" - elif self._needs_auth(stderr_lower): + elif _creds.needs_auth(stderr_lower): # כולל "could not read Username ... terminal prompts disabled": כך # git עונה מאז #3480 כשריפו דורש הזדהות ואין טוקן מוגדר לבעלים return "auth_error" @@ -1045,106 +982,6 @@ def mirror_exists(self, repo_name: str) -> bool: """בדיקה אם mirror קיים""" return self._get_repo_path(repo_name).exists() - # ========== Stored credentials (#3480) ========== - - @staticmethod - def _strip_userinfo(url: str) -> Tuple[str, bool]: - """(ה-URL בלי ``user:pass@``, האם היה שם userinfo). URL של SSH מוחזר כמות שהוא.""" - u = (url or "").strip() - if "://" not in u: - return u, False - parts = urlsplit(u) - if "@" not in parts.netloc: - return u, False - host = parts.netloc.rsplit("@", 1)[1] - return urlunsplit((parts.scheme, host, parts.path, parts.query, parts.fragment)), True - - def ensure_clean_remote(self, repo_name: str) -> Dict[str, Any]: - """מוודא ש-``remote.origin.url`` של המראה אינו נושא credentials, ומנקה אם כן. - - מחזיר ``{"status", "url", "had_credentials", "reason"}``: - - - ``clean`` — ה-URL כבר נקי. - - ``cleaned`` — היה בו userinfo, ``set-url`` רץ, **והקריאה החוזרת** מראה - URL נקי וזהה למה שנכתב. קוד היציאה של ``set-url`` לבדו אינו ראיה. - - ``failed`` — אחד השלבים לא אומת; ``reason`` אומר איזה. ``url`` הוא - ``None`` כשלא ידוע URL נקי. - - ה-URL נקרא דרך ``_run_git_command``, שמעביר את הפלט ב-``_sanitize_output``, - ולכן הטוקן אינו מגיע לשום דבר שהפונקציה מחזירה או רושמת. - """ - repo_name = str(repo_name or "").strip() - if not self._validate_repo_name(repo_name): - return {"status": "failed", "url": None, "had_credentials": False, "reason": "invalid_repo_name"} - repo_path = self._get_repo_path(repo_name) - - # ``GIT_DIR`` מצמיד את הפקודה לתיקיית המראה. בלעדיו, תיקייה שאינה ריפו - # גורמת ל-git לטפס לתיקיות שמעליה — ו-``set-url`` היה כותב ל-config של - # ריפו אחר לגמרי (נמדד: "not a git repository (or any of the parent - # directories)"). - pinned = {"GIT_DIR": str(repo_path)} - read = self._run_git_command(["git", *self._REMOTE_GET_URL], cwd=repo_path, timeout=10, extra_env=pinned) - if not read.success: - return {"status": "failed", "url": None, "had_credentials": False, "reason": "get_url_failed"} - clean_url, had_credentials = self._strip_userinfo(read.stdout) - if not had_credentials: - if not self._validate_repo_url(clean_url): - return {"status": "failed", "url": None, "had_credentials": False, "reason": "unexpected_url"} - return {"status": "clean", "url": clean_url, "had_credentials": False, "reason": None} - - if not self._validate_repo_url(clean_url): - return {"status": "failed", "url": None, "had_credentials": True, "reason": "unexpected_url"} - written = self._run_git_command( - ["git", *self._REMOTE_SET_URL, clean_url], cwd=repo_path, timeout=10, extra_env=pinned - ) - if not written.success: - return {"status": "failed", "url": None, "had_credentials": True, "reason": "set_url_failed"} - - reread = self._run_git_command(["git", *self._REMOTE_GET_URL], cwd=repo_path, timeout=10, extra_env=pinned) - if not reread.success: - return {"status": "failed", "url": None, "had_credentials": True, "reason": "reread_failed"} - now_url, still_has = self._strip_userinfo(reread.stdout) - if still_has or now_url != clean_url: - return {"status": "failed", "url": None, "had_credentials": True, "reason": "still_not_clean"} - return {"status": "cleaned", "url": clean_url, "had_credentials": True, "reason": None} - - def scrub_stored_credentials(self) -> Dict[str, Any]: - """מעבר על **כל** המראות: מנקה credentials מ-``remote.origin.url``, ושורת לוג אחת. - - רץ בעליית כל שירות (``start_credential_sweep``, ``scripts/sweep_mirror_credentials.py``), - כי מראה שאינה נמשכת לעולם לא מגיעה לניקוי שב-``fetch_updates``, וצילום - דיסק משוחזר מחזיר config ישן. כל תיקייה ``*.git`` נבדקת, גם כזו שאינה - מראה תקינה — היא תיספר כ-``failed`` עם הסיבה, לא תדולג בשקט. - - ``sources``: לכל מראה, מאיפה יגיע הטוקן שלה אם הריפו ידרוש הזדהות — - ``map``/``global``/``explicit``/``none``, או ``unknown`` כשה-URL לא ידוע. - """ - stats = {"checked": 0, "had_credentials": 0, "cleaned": 0, "failed": 0} - sources: Dict[str, str] = {} - for path in sorted(self.base_path.glob("*.git")): - if not path.is_dir(): - continue - name = path.name[: -len(".git")] - stats["checked"] += 1 - res = self.ensure_clean_remote(name) - if res["had_credentials"]: - stats["had_credentials"] += 1 - if res["status"] == "cleaned": - stats["cleaned"] += 1 - elif res["status"] == "failed": - stats["failed"] += 1 - logger.warning("mirror credential sweep: %s failed (%s)", name, res["reason"]) - sources[name] = self._token_and_source_for_url(res["url"])[1] if res["url"] else "unknown" - logger.info( - "mirror credential sweep: checked=%d had_credentials=%d cleaned=%d failed=%d sources=%s", - stats["checked"], - stats["had_credentials"], - stats["cleaned"], - stats["failed"], - json.dumps(sources, sort_keys=True), - ) - return {**stats, "sources": sources} - def delete_mirror(self, repo_name: str) -> Dict[str, Any]: """ מחיקת ה-mirror מהדיסק (תיקיית bare repo). @@ -1242,7 +1079,10 @@ def detect_default_branch(self, repo_name: str) -> Dict[str, Optional[str]]: if not full.startswith(prefix): return {"branch": None, "reason": "head_not_a_branch"} branch = full[len(prefix):] - if not self._validate_basic_ref(branch) or branch == "HEAD": + # הבדיקה על ה-ref **המלא**, כמו שהצרכנים בהמשך בודקים אותו (``list_all_files`` + # מקבל ``refs/heads/`` ומריץ עליו ``_validate_basic_ref``). בדיקה על + # השם לבדו דחתה שמות ש-git מקבל, כמו ``_main``. + if not branch or branch == "HEAD" or not self._validate_basic_ref(full): return {"branch": None, "reason": "unsupported_branch_name"} exists = self._run_git_command( ["git", "rev-parse", "--verify", "--quiet", f"{prefix}{branch}^{{commit}}"], @@ -3948,32 +3788,3 @@ def get_mirror_service() -> GitMirrorService: _mirror_service = GitMirrorService() return _mirror_service - -def sweep_stored_credentials() -> Optional[Dict[str, Any]]: - """ניקוי credentials מכל המראות בדיסק של השירות הזה (#3480). ``None`` אם אין תיקיית מראות. - - **לא יוצר את התיקייה.** ``GitMirrorService()`` עושה ``mkdir``, וכאן אין סיבה: - כשאין ``REPO_MIRROR_PATH`` בדיסק, אין מה לנקות — וכך גם ייבוא של - ``mcp_server.app`` בטסט אינו יוצר תיקייה במכונה. - """ - base = _default_mirror_base_path() - if not base.is_dir(): - logger.info("mirror credential sweep: no mirror directory at %s, nothing to check", base) - return None - return get_mirror_service().scrub_stored_credentials() - - -def start_credential_sweep() -> threading.Thread: - """מריץ את ``sweep_stored_credentials`` פעם אחת ברקע. נקרא מנקודת הכניסה של שירות ה-MCP.""" - - def _run() -> None: - try: - sweep_stored_credentials() - except Exception: - # thread רקע: חריגה כאן לא מגיעה לאף קורא, ולכן נרשמת עם traceback - logger.warning("mirror credential sweep failed", exc_info=True) - - thread = threading.Thread(target=_run, daemon=True, name="mirror-credential-sweep") - thread.start() - return thread - diff --git a/services/mirror_credentials.py b/services/mirror_credentials.py new file mode 100644 index 000000000..9a2a674ed --- /dev/null +++ b/services/mirror_credentials.py @@ -0,0 +1,243 @@ +"""אימות מול GitHub בלי לשמור את הטוקן בדיסק, וניקוי מראות שנוצרו לפני #3480. + +עד #3480 הטוקן הוזרק ל-URL (``https://oauth2:@github.com/...``), ו-git שומר +את ה-URL שה-clone נעשה ממנו ב-``remote.origin.url`` בקובץ ``config`` של המראה — בטקסט +גלוי, על הדיסק של הוובאפ ושל שירות ה-MCP ובצילומי הדיסק של Render. git גם מעביר את +ה-URL המלא כארגומנט ל-``git-remote-https``, כך שהוא גלוי ברשימת התהליכים בכל clone +ו-fetch. מאז, ה-URL שנשמר נקי, והטוקן עובר לכל פקודת רשת בנפרד כ**כותרת**, דרך משתני +הסביבה ``GIT_CONFIG_COUNT`` / ``GIT_CONFIG_KEY_`` / ``GIT_CONFIG_VALUE_`` (git +2.31 ומעלה, ``Documentation/git-config.txt``) — לא דרך ``git -c``, שמציב את הערך +בשורת הפקודה. התיעוד של git עצמו מצביע על ``http.extraHeader`` כמקרה שבו העברה דרך +הסביבה עדיפה (``Documentation/git.txt``, ``--config-env``). + +**מה כאן ומה ב-``git_mirror_service``.** כאן: הקבועים, בניית הסביבה של פקודת רשת, +זיהוי "השרת דרש הזדהות", הניקוי של מראה אחת (``ensure_clean_remote``) והמעבר על +כל המראות. ב-``GitMirrorService`` נשארים רק בחירת הטוקן לפי בעלים +(``_token_and_source_for_url``) והרצת clone/fetch (``_run_network_git``), כי הם חלק +מהרצת הפקודות שהשירות כבר מחזיק. כל מה שכאן מריץ git דרך +``GitMirrorService._run_git_command`` — אותה רשימת פקודות מותרות ואותו ניקוי פלט. + +**מתי המעבר על כל המראות רץ.** בעליית כל שירות, מנקודת הכניסה שלו ולא בייבוא: +בשירות ה-MCP מה-lifespan של האפליקציה (``attach_credential_sweep`` ב- +``mcp_server/repo_autosync.py``), ובוובאפ מ-``scripts/start_webapp.sh`` דרך +``scripts/sweep_mirror_credentials.py``. +""" + +from __future__ import annotations + +import base64 +import json +import logging +import threading +from typing import TYPE_CHECKING, Any, Dict, List, Optional, Tuple +from urllib.parse import urlsplit, urlunsplit + +if TYPE_CHECKING: # pragma: no cover - type-only, אין ייבוא מעגלי בזמן ריצה + from services.git_mirror_service import GitMirrorService + +logger = logging.getLogger(__name__) + +# **המקור (origin) של GitHub.** ממנו נגזרים גם הבדיקה של כתובת ריפו +# (``GitMirrorService._validate_repo_url``), גם חילוץ הבעלים, וגם המפתח שאליו +# הכותרת ממוקדת — git משווה את המפתח לכתובת לפי scheme, host ו-port +# (``Documentation/config/http.txt``, ``http..*``), ולכן הכותרת לא נשלחת לשום +# מארח אחר. נקרא בזמן קריאה ולא מועתק, כך שהטסטים מחליפים אותו כאן כדי להריץ את +# ``init_mirror``/``fetch_updates`` האמיתיים מול שרת מקומי. +GITHUB_HTTPS_ORIGIN = "https://github.com" + +# שם המשתמש שנשלח עם הטוקן. אותו credential בדיוק שה-URL נשא עד #3480. +GIT_AUTH_USERNAME = "oauth2" + +# ``transfer.credentialsInUrl=die`` (git 2.37 ומעלה): מראה ששוב נושאת +# credentials ב-URL נכשלת **לפני** בקשת רשת, וההודעה של git כבר מסתירה את +# הסיסמה (````). זה השומר שהופך "מראה שלא נוקתה" לשגיאה עם שם, +# במקום fetch שקט עם הטוקן השמור. +CREDENTIALS_IN_URL_GUARD: Tuple[str, str] = ("transfer.credentialsInUrl", "die") + +# הודעות git כשהשרת דורש הזדהות ואין מה לתת לו. נאכפות באנגלית דרך +# ``LC_ALL=C`` בסביבת פקודות הרשת. ``could not read Username`` נבדק מול +# github.com עצמו, על ריפו פרטי/לא קיים בלי טוקן (git 2.43, אוקטובר 2026). +AUTH_REQUIRED_MARKERS = ( + "could not read username", + "terminal prompts disabled", + "authentication failed", +) + +# ``git remote`` מותר ב-``_run_git_command`` **רק** בשתי הצורות האלה, ובדיוק +# במבנה הזה. כל השאר (``add``, ``remove``, ``set-url --push``...) נדחה. +REMOTE_GET_URL: Tuple[str, str, str] = ("remote", "get-url", "origin") +REMOTE_SET_URL: Tuple[str, str, str] = ("remote", "set-url", "origin") + + +def network_env(token: Optional[str]) -> Dict[str, str]: + """הסביבה של פקודת רשת (clone/fetch): בלי הנחיות מסוף, עם השומר, ועם הכותרת אם יש טוקן. + + הכותרת היא אותו credential בדיוק שה-URL נשא עד #3480 (``oauth2:`` + ב-Basic), ממוקדת ל-``GITHUB_HTTPS_ORIGIN`` בלבד. base64 גם מנטרל כל תו + בטוקן שהיה שובר את הדקדוק של כותרת HTTP. + """ + pairs: List[Tuple[str, str]] = [CREDENTIALS_IN_URL_GUARD] + if token: + basic = base64.b64encode(f"{GIT_AUTH_USERNAME}:{token}".encode("utf-8")).decode("ascii") + pairs.append((f"http.{GITHUB_HTTPS_ORIGIN.rstrip('/')}/.extraHeader", f"Authorization: Basic {basic}")) + env: Dict[str, str] = { + "GIT_TERMINAL_PROMPT": "0", + # הודעות git באנגלית: ``needs_auth`` ו-``_classify_git_error`` מזהים אותן לפי הטקסט + "LC_ALL": "C", + "GIT_CONFIG_COUNT": str(len(pairs)), + } + for i, (key, value) in enumerate(pairs): + env[f"GIT_CONFIG_KEY_{i}"] = key + env[f"GIT_CONFIG_VALUE_{i}"] = value + return env + + +def needs_auth(stderr: str) -> bool: + """האם git נכשל כי השרת דרש הזדהות (ריפו פרטי, או טוקן שלא התקבל).""" + text = (stderr or "").lower() + return any(marker in text for marker in AUTH_REQUIRED_MARKERS) + + +def strip_userinfo(url: str) -> Tuple[Optional[str], bool]: + """(ה-URL בלי ``user:pass@``, האם היה שם userinfo). URL של SSH מוחזר כמות שהוא. + + URL ש-``urlsplit`` אינו מצליח לפרק (למשל ``https://[::1/x`` — "Invalid IPv6 + URL", נמדד) מחזיר ``None`` במקום להפיל את הקורא: מראה עם URL כזה נספרת ככשל + עם סיבה, והמעבר ממשיך לשאר המראות. ``had`` במקרה הזה הוא ההערכה השמרנית — + יש ``@`` במחרוזת. + """ + u = (url or "").strip() + if "://" not in u: + return u, False + try: + parts = urlsplit(u) + except ValueError: + return None, "@" in u + if "@" not in parts.netloc: + return u, False + host = parts.netloc.rsplit("@", 1)[1] + return urlunsplit((parts.scheme, host, parts.path, parts.query, parts.fragment)), True + + +def _failed(reason: str, had_credentials: bool) -> Dict[str, Any]: + return {"status": "failed", "url": None, "had_credentials": had_credentials, "reason": reason} + + +def ensure_clean_remote(service: "GitMirrorService", repo_name: str) -> Dict[str, Any]: + """מוודא ש-``remote.origin.url`` של המראה אינו נושא credentials, ומנקה אם כן. + + מחזיר ``{"status", "url", "had_credentials", "reason"}``: + + - ``clean`` — ה-URL כבר נקי. + - ``cleaned`` — היה בו userinfo, ``set-url`` רץ, **והקריאה החוזרת** מראה URL + נקי וזהה למה שנכתב. קוד היציאה של ``set-url`` לבדו אינו ראיה. + - ``failed`` — אחד השלבים לא אומת; ``reason`` אומר איזה. ``url`` הוא ``None``. + + ה-URL נקרא דרך ``_run_git_command``, שמעביר את הפלט ב-``_sanitize_output``, + ולכן הטוקן אינו מגיע לשום דבר שהפונקציה מחזירה או רושמת. + """ + repo_name = str(repo_name or "").strip() + if not service._validate_repo_name(repo_name): + return _failed("invalid_repo_name", False) + repo_path = service._get_repo_path(repo_name) + + # ``GIT_DIR`` מצמיד את הפקודה לתיקיית המראה. בלעדיו, תיקייה שאינה ריפו + # גורמת ל-git לטפס לתיקיות שמעליה — ו-``set-url`` היה כותב ל-config של + # ריפו אחר לגמרי (נמדד: "not a git repository (or any of the parent + # directories)"). **נתיב מוחלט:** ``GIT_DIR`` יחסי נפתר מה-cwd של git, שהוא + # המראה עצמה — ``base_path`` יחסי היה מצביע על ``<מראה>//<מראה>`` + # ונכשל (נמדד: "not a git repository: 'mirrors/a.git'"). + pinned = {"GIT_DIR": str(repo_path.resolve())} + + def run(cmd: List[str]): + return service._run_git_command(cmd, cwd=repo_path, timeout=10, extra_env=pinned) + + read = run(["git", *REMOTE_GET_URL]) + if not read.success: + return _failed("get_url_failed", False) + clean_url, had_credentials = strip_userinfo(read.stdout) + if clean_url is None: + return _failed("unparseable_url", had_credentials) + if not service._validate_repo_url(clean_url): + return _failed("unexpected_url", had_credentials) + if not had_credentials: + return {"status": "clean", "url": clean_url, "had_credentials": False, "reason": None} + + if not run(["git", *REMOTE_SET_URL, clean_url]).success: + return _failed("set_url_failed", True) + reread = run(["git", *REMOTE_GET_URL]) + if not reread.success: + return _failed("reread_failed", True) + now_url, still_has = strip_userinfo(reread.stdout) + if still_has or now_url != clean_url: + return _failed("still_not_clean", True) + return {"status": "cleaned", "url": clean_url, "had_credentials": True, "reason": None} + + +def scrub_stored_credentials(service: "GitMirrorService") -> Dict[str, Any]: + """מעבר על **כל** המראות של השירות: מנקה credentials מ-``remote.origin.url``, ושורת לוג אחת. + + רץ בעליית כל שירות, כי מראה שאינה נמשכת לעולם לא מגיעה לניקוי שב- + ``fetch_updates``, וצילום דיסק משוחזר מחזיר config ישן. כל תיקייה ``*.git`` + נבדקת, גם כזו שאינה מראה תקינה — היא תיספר כ-``failed`` עם הסיבה, לא תדולג + בשקט. **השורה מאמתת שהמראות נקיות רק כש-``failed=0``**: מראה שנכשלה עדיין + עלולה להחזיק את הטוקן, וה-fetch שלה נחסם עד שהניקוי יצליח. + + ``sources``: לכל מראה, מאיפה יגיע הטוקן שלה אם הריפו ידרוש הזדהות — + ``map``/``global``/``explicit``/``none``, או ``unknown`` כשה-URL לא ידוע. + """ + stats = {"checked": 0, "had_credentials": 0, "cleaned": 0, "failed": 0} + sources: Dict[str, str] = {} + for path in sorted(service.base_path.glob("*.git")): + if not path.is_dir(): + continue + name = path.name[: -len(".git")] + stats["checked"] += 1 + res = ensure_clean_remote(service, name) + if res["had_credentials"]: + stats["had_credentials"] += 1 + if res["status"] == "cleaned": + stats["cleaned"] += 1 + elif res["status"] == "failed": + stats["failed"] += 1 + logger.warning("mirror credential sweep: %s failed (%s)", name, res["reason"]) + sources[name] = service._token_and_source_for_url(res["url"])[1] if res["url"] else "unknown" + logger.info( + "mirror credential sweep: checked=%d had_credentials=%d cleaned=%d failed=%d sources=%s", + stats["checked"], + stats["had_credentials"], + stats["cleaned"], + stats["failed"], + json.dumps(sources, sort_keys=True), + ) + return {**stats, "sources": sources} + + +def sweep_stored_credentials() -> Optional[Dict[str, Any]]: + """ניקוי credentials מכל המראות בדיסק של השירות הזה. ``None`` אם אין תיקיית מראות. + + **לא יוצר את התיקייה.** ``GitMirrorService()`` עושה ``mkdir``, וכאן אין סיבה: + כשאין תיקיית מראות בדיסק, אין מה לנקות. + """ + from services.git_mirror_service import _default_mirror_base_path, get_mirror_service + + base = _default_mirror_base_path() + if not base.is_dir(): + logger.info("mirror credential sweep: no mirror directory at %s, nothing to check", base) + return None + return scrub_stored_credentials(get_mirror_service()) + + +def start_credential_sweep() -> threading.Thread: + """מריץ את ``sweep_stored_credentials`` פעם אחת ברקע. נקרא מעליית השירות, לא מייבוא.""" + + def _run() -> None: + try: + sweep_stored_credentials() + except Exception: + # thread רקע: חריגה כאן לא מגיעה לאף קורא, ולכן נרשמת עם traceback + logger.warning("mirror credential sweep failed", exc_info=True) + + thread = threading.Thread(target=_run, daemon=True, name="mirror-credential-sweep") + thread.start() + return thread diff --git a/tests/test_git_mirror_credentials.py b/tests/test_git_mirror_credentials.py index db8be8549..519d42578 100644 --- a/tests/test_git_mirror_credentials.py +++ b/tests/test_git_mirror_credentials.py @@ -25,7 +25,7 @@ import pytest -from services import git_mirror_service as gms +from services import mirror_credentials as creds from services.git_mirror_service import GitCommandResult, GitMirrorService MAP_TOKEN = "ghp_TESTMAP0123456789abcdefghijABCDEFGH" @@ -178,7 +178,7 @@ def world(tmp_path: Path, monkeypatch: pytest.MonkeyPatch): for key, value in settings.items(): monkeypatch.setenv(key, value) server = _GitServer(tmp_path / "srv") - monkeypatch.setattr(gms, "GITHUB_HTTPS_ORIGIN", server.origin) + monkeypatch.setattr(creds, "GITHUB_HTTPS_ORIGIN", server.origin) try: yield _World(tmp_path, server, dict(os.environ)) finally: @@ -356,7 +356,7 @@ def set_url_lies(cmd, *args, **kwargs): monkeypatch.setattr(world.svc, "_run_git_command", set_url_lies) - assert world.svc.ensure_clean_remote("liar")["status"] == "failed" + assert creds.ensure_clean_remote(world.svc, "liar")["status"] == "failed" assert world.svc.fetch_updates("liar")["error_type"] == "mirror_url_not_clean" assert world.server.take() == [] assert MAP_TOKEN in (mirror / "config").read_text() @@ -387,8 +387,8 @@ def test_sweep_cleans_every_mirror_and_logs_counts_and_sources(world, monkeypatc assert world.svc.init_mirror(f"{world.server.origin}/unmapped/fresh.git", "fresh")["success"] (world.mirrors / "junk.git").mkdir() - with caplog.at_level(logging.INFO, logger="services.git_mirror_service"): - stats = world.svc.scrub_stored_credentials() + with caplog.at_level(logging.INFO, logger="services.mirror_credentials"): + stats = creds.scrub_stored_credentials(world.svc) assert {k: stats[k] for k in ("checked", "had_credentials", "cleaned", "failed")} == { "checked": 3, @@ -413,7 +413,7 @@ def test_sweep_never_touches_a_repository_above_the_mirrors_dir(world): nested = GitMirrorService(base_path=str(parent / "mirrors")) (parent / "mirrors" / "junk.git").mkdir() - stats = nested.scrub_stored_credentials() + stats = creds.scrub_stored_credentials(nested) assert stats["failed"] == 1 and stats["cleaned"] == 0 assert world.git("config", "--get", "remote.origin.url", cwd=parent) == secret_url @@ -422,7 +422,7 @@ def test_sweep_never_touches_a_repository_above_the_mirrors_dir(world): def test_module_sweep_does_not_create_a_missing_mirror_dir(tmp_path, monkeypatch): missing = tmp_path / "no-mirrors-here" monkeypatch.setenv("REPO_MIRROR_PATH", str(missing)) - assert gms.sweep_stored_credentials() is None + assert creds.sweep_stored_credentials() is None assert not missing.exists() @@ -515,3 +515,105 @@ def remove_files(self, repo_name, paths): assert out.get("status") == "completed", out assert _Metadata.saved["default_branch"] == "master" + + +# --------------------------------------------------------------------------- +# מקרי קצה שהריוויו של PR #3519 העלה — כל אחד נמדד לפני התיקון +# --------------------------------------------------------------------------- + + +def test_sweep_survives_an_unparseable_url_and_cleans_the_rest(world, monkeypatch): + """``urlsplit`` זורק ``ValueError`` על ``https://[::1/x.git`` — זה לא עוצר את המעבר.""" + monkeypatch.setenv("GITHUB_TOKENS", f"mapped={MAP_TOKEN}") + world.make_origin("mapped", "good") + world.legacy_mirror("mapped", "good", MAP_TOKEN) + broken = world.mirrors / "broken.git" + world.git("init", "-q", "--bare", str(broken)) + world.git("remote", "add", "origin", "https://[::1/x.git", cwd=broken) + + stats = creds.scrub_stored_credentials(world.svc) + + assert (stats["checked"], stats["cleaned"], stats["failed"]) == (2, 1, 1) + assert creds.ensure_clean_remote(world.svc, "broken")["reason"] == "unparseable_url" + + +def test_cleaning_works_with_a_relative_mirrors_path(world, monkeypatch): + """``base_path`` יחסי: ``GIT_DIR`` יחסי היה נפתר מתוך המראה עצמה ונכשל.""" + monkeypatch.setenv("GITHUB_TOKENS", f"mapped={MAP_TOKEN}") + world.make_origin("mapped", "rel") + mirror = world.legacy_mirror("mapped", "rel", MAP_TOKEN) + monkeypatch.chdir(world.tmp) + relative = GitMirrorService(base_path="mirrors") + + assert creds.ensure_clean_remote(relative, "rel")["status"] == "cleaned" + assert MAP_TOKEN not in (mirror / "config").read_text() + + +def test_detect_default_branch_accepts_names_git_accepts(world): + """``_main`` הוא שם ענף תקין ב-git; הבדיקה היא על ``refs/heads/_main``, כמו אצל הצרכנים.""" + url = world.make_origin("openorg", "underscore", branch="_main") + assert world.svc.init_mirror(url, "underscore")["success"] + assert world.svc.detect_default_branch("underscore") == {"branch": "_main", "reason": None} + + +_SWEEP_PROBE = """ +import asyncio, sys, time, types +sys.path.insert(0, {tests!r}) +sys.path.insert(0, {repo!r}) +from _fake_mongo import FakeDB + +fake_database = types.ModuleType("database") +fake_database.db = types.SimpleNamespace(db=FakeDB()) +sys.modules["database"] = fake_database + +config = {config!r} + +def token_left(): + with open(config, encoding="utf-8") as fh: + return {token!r} in fh.read() + +import mcp_server.app as app_module +time.sleep(1.0) +print("AFTER-IMPORT=" + str(token_left()), flush=True) + +async def serve(): + async with app_module.app.router.lifespan_context(app_module.app): + deadline = time.monotonic() + 20 + while token_left() and time.monotonic() < deadline: + await asyncio.sleep(0.1) + +asyncio.run(serve()) +print("AFTER-LIFESPAN=" + str(token_left()), flush=True) +""" + + +def test_importing_the_mcp_app_does_not_sweep_but_serving_it_does(world, tmp_path): + """ייבוא של ``mcp_server.app`` (טסטים, כלים, REPL) לא נוגע במראות; עליית השרת כן. + + בתהליך נקי, כמו ש-uvicorn מייבא: ``create_app`` רץ בייבוא, ולכן ניקוי + שמופעל ממנו ישירות היה כותב ל-config של כל מראה גם בייבוא סתמי. + """ + pytest.importorskip("mcp") + import pathlib + + repo = str(pathlib.Path(__file__).resolve().parents[1]) + tests_dir = str(pathlib.Path(__file__).resolve().parent) + mirrors = tmp_path / "probe-mirrors" + mirrors.mkdir() + origin = world.make_origin("openorg", "served") + target = mirrors / "served.git" + # כמו מראה מלפני #3480, עם כתובת GitHub אמיתית — הניקוי מקומי ואינו פונה לרשת + world.git("clone", "-q", "--mirror", "--", origin, str(target)) + world.git("remote", "set-url", "origin", f"https://oauth2:{MAP_TOKEN}@github.com/openorg/served.git", cwd=target) + + env = dict(world.env) + env.update({"REPO_MIRROR_PATH": str(mirrors), "MCP_REPO_AUTOSYNC": "0"}) + for key in ("MCP_SERVER_URL", "WEBAPP_URL"): + env.pop(key, None) + probe = _SWEEP_PROBE.format(repo=repo, tests=tests_dir, config=str(target / "config"), token=MAP_TOKEN) + proc = subprocess.run( + [sys.executable, "-B", "-c", probe], capture_output=True, text=True, timeout=120, cwd=repo, env=env + ) + + assert "AFTER-IMPORT=True" in proc.stdout, proc.stdout + proc.stderr + assert "AFTER-LIFESPAN=False" in proc.stdout, proc.stdout + proc.stderr diff --git a/tests/test_git_mirror_service.py b/tests/test_git_mirror_service.py index 1c2094b0f..3d4e337f1 100644 --- a/tests/test_git_mirror_service.py +++ b/tests/test_git_mirror_service.py @@ -255,7 +255,7 @@ def test_token_source_per_owner_and_header_env(service, monkeypatch): """מקור הטוקן לכל בעלים, והכותרת שנבנית ממנו — ממוקדת ל-origin של GitHub ובלי טוקן ב-URL (#3480).""" import base64 - from services import git_mirror_service as gms + from services import mirror_credentials as creds monkeypatch.setenv("GITHUB_TOKENS", "Campaign-AI4U=ghp_AAA") monkeypatch.setenv("GITHUB_TOKEN", "ghp_GLOBAL") @@ -265,13 +265,13 @@ def test_token_source_per_owner_and_header_env(service, monkeypatch): monkeypatch.delenv("GITHUB_TOKEN") assert service._token_and_source_for_url("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/Zzz/repo.git") == (None, "none") - env = gms.GitMirrorService._network_env("ghp_AAA") + env = creds.network_env("ghp_AAA") pairs = {env[f"GIT_CONFIG_KEY_{i}"]: env[f"GIT_CONFIG_VALUE_{i}"] for i in range(int(env["GIT_CONFIG_COUNT"]))} expected = "Authorization: Basic " + base64.b64encode(b"oauth2:ghp_AAA").decode() assert pairs["http.https://github.com/.extraHeader"] == expected assert pairs["transfer.credentialsInUrl"] == "die" # בלי טוקן — אין כותרת בכלל, רק השומר - bare = gms.GitMirrorService._network_env(None) + bare = creds.network_env(None) assert bare["GIT_CONFIG_COUNT"] == "1" diff --git a/tests/test_start_webapp_mirror_sweep.py b/tests/test_start_webapp_mirror_sweep.py new file mode 100644 index 000000000..83e401fc8 --- /dev/null +++ b/tests/test_start_webapp_mirror_sweep.py @@ -0,0 +1,92 @@ +"""``scripts/start_webapp.sh`` מריץ את ניקוי ה-credentials של המראות (#3480) — באמת, ובלי להפיל את השירות. + +זה הטריגר היחיד של הניקוי בוובאפ בפרודקשן, ולכן הסקריפט עצמו רץ כאן, כמו ש-Render +מריץ אותו: ``bash scripts/start_webapp.sh``. רק ``gunicorn`` מוחלף בסקריפט קטן +ב-``PATH`` שיוצא בקוד שהטסט בוחר, ו-``python3`` מצביע על המפרש של הטסט. הניקוי עצמו +אמיתי: git אמיתי, מראות אמיתיות ב-``tmp_path``. הוא מקומי בלבד ואינו פונה לרשת. + +``subprocess.run`` מחכה ל-EOF על הפלט, והניקוי רץ ברקע עם אותו stdout — כך שהטסט +מחכה גם לו, בלי שינה ובלי ניחוש זמנים. +""" + +from __future__ import annotations + +import os +import pathlib +import shutil +import subprocess +import sys + +import pytest + +REPO = pathlib.Path(__file__).resolve().parents[1] +SCRIPT = REPO / "scripts" / "start_webapp.sh" +TOKEN = "ghp_TESTSTART0123456789abcdefghijABCDEFG" + +pytestmark = pytest.mark.skipif(shutil.which("bash") is None, reason="הסקריפט הוא bash") + + +def _git(*args: str, cwd: pathlib.Path | None = None, env: dict | None = None) -> str: + done = subprocess.run(["git", *args], cwd=cwd, env=env, check=True, capture_output=True, text=True) + return done.stdout.strip() + + +def _run_start_script(tmp_path: pathlib.Path, mirrors: pathlib.Path, gunicorn_exit: int = 0): + stubs = tmp_path / "stubs" + stubs.mkdir() + gunicorn = stubs / "gunicorn" + gunicorn.write_text(f"#!/usr/bin/env bash\necho \"stub gunicorn $*\"\nexit {gunicorn_exit}\n", encoding="utf-8") + gunicorn.chmod(0o755) + (stubs / "python3").symlink_to(sys.executable) + home = tmp_path / "home" + home.mkdir(exist_ok=True) + env = { + "PATH": f"{stubs}{os.pathsep}{os.environ['PATH']}", + "HOME": str(home), + "GIT_CONFIG_NOSYSTEM": "1", + "REPO_MIRROR_PATH": str(mirrors), + "WEBAPP_ENABLE_WARMUP": "0", + "ASSET_VERSION": "test", + "PYTHONDONTWRITEBYTECODE": "1", + } + return subprocess.run(["bash", str(SCRIPT)], capture_output=True, text=True, timeout=120, env=env) + + +def _legacy_mirror(tmp_path: pathlib.Path, mirrors: pathlib.Path, name: str) -> pathlib.Path: + """מראה כמו שהקוד יצר עד #3480: הטוקן בתוך ``remote.origin.url``.""" + env = {**os.environ, "HOME": str(tmp_path / "home"), "GIT_CONFIG_NOSYSTEM": "1"} + (tmp_path / "home").mkdir(exist_ok=True) + target = mirrors / f"{name}.git" + _git("init", "-q", "--bare", str(target), env=env) + _git("remote", "add", "origin", f"https://oauth2:{TOKEN}@github.com/someorg/{name}.git", cwd=target, env=env) + return target + + +def test_start_script_cleans_mirrors_and_logs_the_sweep(tmp_path): + mirrors = tmp_path / "mirrors" + mirrors.mkdir() + target = _legacy_mirror(tmp_path, mirrors, "legacy") + + proc = _run_start_script(tmp_path, mirrors) + + out = proc.stdout + proc.stderr + assert proc.returncode == 0, out + assert "stub gunicorn app:app" in out, "Gunicorn הופעל לפני הניקוי ולא נחסם בגללו" + assert "mirror credential sweep: checked=1 had_credentials=1 cleaned=1 failed=0" in out, out + assert "Mirror credential sweep finished" in out, out + assert TOKEN not in (target / "config").read_text() + assert TOKEN not in out + + +def test_a_failed_sweep_is_reported_and_does_not_stop_the_service(tmp_path): + mirrors = tmp_path / "mirrors" + mirrors.mkdir() + (mirrors / "junk.git").mkdir() # תיקייה שאינה ריפו: הניקוי שלה נכשל + + proc = _run_start_script(tmp_path, mirrors, gunicorn_exit=0) + + out = proc.stdout + proc.stderr + assert "failed=1" in out, out + assert "Mirror credential sweep reported failures" in out, out + # קוד היציאה של הסקריפט הוא של Gunicorn — כשל בניקוי אינו מפיל את השירות + assert proc.returncode == 0, out