Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 14 additions & 39 deletions GUIDES/REPO_SYNC_ENGINE_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,8 @@ print(f"Disk writable: {os.access(mirror_path, os.W_OK)}")

## מימוש GitMirrorService

> ℹ️ **אימות מול GitHub (#3480).** הקוד שלמטה הוא המדריך המקורי, והוא אינו מסונכרן עם הקוד בכל פרט. בחלק של האימות הוא עודכן: הטוקן לא נכנס ל-URL, כי git שומר את ה-URL בטקסט גלוי בקובץ ה-`config` של המראה. המימוש — `services/mirror_credentials.py` ו-`GitMirrorService._run_network_git`; ההסבר — `docs/mcp-server.rst`, הסעיף "אימות מול GitHub — הטוקן לא נשמר במראה".

צור קובץ `services/git_mirror_service.py`:

```python
Expand Down Expand Up @@ -220,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:
"""
מחיקת טוקנים רגישים מפלט/לוגים
Expand Down Expand Up @@ -398,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:
Expand Down
4 changes: 2 additions & 2 deletions docs/environment-variables.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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``
Expand Down
19 changes: 19 additions & 0 deletions docs/mcp-server.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1739,6 +1739,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_<n>``/``GIT_CONFIG_VALUE_<n>`` ולא בשורת הפקודה (``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 מה-lifespan של האפליקציה (``attach_credential_sweep`` ב-``mcp_server/repo_autosync.py`` — לא בייבוא של ``mcp_server.app``), ובוובאפ מתוך ``scripts/start_webapp.sh``. שורת הלוג של המעבר הזה היא האימות:

.. code-block:: text

mirror credential sweep: checked=<n> had_credentials=<n> cleaned=<n> failed=<n> sources={"<repo>": "map", ...}

**המראות נקיות רק כש-``failed=0``.** מראה שנספרה ב-``failed`` עלולה עדיין להחזיק את הטוקן ב-``config``; לכל אחת מהן יש שורת ``mirror credential sweep: <repo> failed (<reason>)`` עם הסיבה, והניסיון חוזר לפני ה-fetch הבא שלה ובעלייה הבאה. ``sources`` אומר לכל מראה מאיפה יגיע הטוקן שלה אם הריפו ידרוש הזדהות, בלי הטוקן עצמו — כך רואים אילו מראות נשענות על ``GITHUB_TOKEN``. שורה שלא מופיעה בלוג של אחד השירותים פירושה שהמעבר לא רץ שם.

**שומר.** כל פקודת רשת רצה עם ``transfer.credentialsInUrl=die``: מראה שעדיין נושאת credentials ב-URL נכשלת לפני בקשת רשת, ו-git עצמו מסתיר את הסיסמה בהודעה. ``git remote`` מותר ב-``_run_git_command`` רק בשתי הצורות ``get-url origin`` ו-``set-url origin <url>``, וה-URL חייב לעבור את ``_validate_repo_url``.

.. _mcp-repo-read-misses:

ענף שלא במראה, מול קובץ שלא קיים
Expand Down
2 changes: 2 additions & 0 deletions docs/whats-new.rst
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ What's New

2026-10-04
Comment thread
amirbiron marked this conversation as resolved.
----------
- **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 שלהן לריפו פרטי ייכשל באימות; מראה כזו מתקנים במחיקה ושכפול מחדש.
- **fix (MCP): ``codekeeper_note_str_replace`` מודד את התוצאה לפני שהוא בונה אותה (#3495).** עד היום הכלי בנה את גוף הפתק החדש ורק אז השווה אותו ל-``MAX_NOTE_CHARS``, ולכן ``replace_all`` על מחרוזת קצרה עם ``new_string`` ארוך בנה בזיכרון מחרוזת באורך מספר המופעים כפול אורך ההחלפה לפני שנדחה: פתק של 2,000 ``"a"`` והחלפה של 10,000 תווים — 20 מיליון תווים, שיא הקצאה של 20,019,845 בתים על בקשה של 10,097 בתים (נמדד ב-``test_a_note_replace_all_is_checked_before_its_result_is_built`` על הקוד שלפני התיקון). בתקרות של הייצור — פתק של ``MAX_NOTE_CHARS`` תווים, ו-``new_string`` שנכנס בגוף בקשה של 2MiB (התקרה ש-``request_bytes_for`` גוזר מ-``MAX_CODE_SIZE`` של 300,000) — זה כ-40GB לפני הסירוב: בקשה אחת שמפילה את תהליך ה-MCP לכל המשתמשים, ותקרת הגוף והגבלת הקצב אינן עוזרות, כי הן חוסמות כל גורם לבדו ולא את המכפלה. עכשיו הכלי עובר באותה בדיקה מוקדמת של ``codekeeper_edit_file`` (``_apply_edit`` ב-``mcp_server/handlers.py``), והתשובה על תוצאה ארוכה מדי שאינה רווחים בלבד היא אותה תשובה — ``content_too_long`` עם ``max`` — רק לפני ההקצאה: אותו מקרה נגמר בשיא של כ-23KB, וכך גם פתק מלא עד התקרה. ``max_size`` של ``_apply_edit`` הפך לחובה, בלי ברירת מחדל, כדי שכלי הבא שיחליף טקסט דרכו לא יוכל לשכוח אותו. **שינוי בקצה:** תוצאה של רווחים בלבד שארוכה מהתקרה מחזירה עכשיו ``content_too_long`` ולא ``empty_content`` — שני הסירובים אינם כותבים דבר. ראו :ref:`mcp-multi-edit`.
- **ci (MyPy): קריאה שחסר בה ארגומנט חובה עוצרת את ה-CI — ``call-arg`` נוסף לשער.** המעבר הרחב של mypy ב-``.github/workflows/ci.yml`` נכשל עד היום רק על ``attr-defined`` ו-``return-value``. ``call-arg`` — ארגומנט חובה שחסר, או ארגומנט שאינו קיים — הודפס ביומן ולא עצר דבר, ובפייתון זו שגיאה שמתגלה רק כשהקריאה רצה: אתר קריאה בלי טסט נופל בייצור. עכשיו הוא חוסם, וזה מה שהופך את ``max_size`` החובה של ``_apply_edit`` לבדיקה סטטית — נבדק ש-``note_str_replace`` בלי ``max_size`` מפיל את השער, ושעם השער הקודם אותו קוד עבר. המופע היחיד שהיה בריפו (mypy 2.4.0) — מחלקה מקומית בשם ``_Ctx`` שהוגדרה פעמיים ב-``setup_bot_data`` ב-``main.py`` — קיבל שם משלו, ``_CtxPredictive``.

Expand Down
21 changes: 19 additions & 2 deletions mcp_server/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,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,
Expand All @@ -172,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,
Expand All @@ -185,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()
42 changes: 42 additions & 0 deletions mcp_server/repo_autosync.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@

from __future__ import annotations

import contextlib
import logging
import os
import re
Expand Down Expand Up @@ -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.

Expand Down
25 changes: 25 additions & 0 deletions scripts/start_webapp.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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 &
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
wait "$APP_PID"
Loading
Loading