Skip to content

‏158 משתני סביבה נצרכים בקוד ואינם בטבלת ה-Config Inspector #3297

Description

@amirbiron

המשך של #3296, שבו נכנסה קבוצת SENTRY ותוקן שיוך שגוי אחד.

המצב

נותרו בפער: 158

   שורה ברפרנס (תיאור מוכן):  90
   אזכור בפרוזה בלבד:          2
   לא ברפרנס כלל:             66

זו לא חלוקה טכנית אלא שתי עבודות שונות:

  • ה-90 — התיאור כבר כתוב ב-docs/environment-variables.rst ונקרא בידי אדם. ההוספה לטבלה היא העתקה של תיאור קיים והפקת services מהניתוח. קובץ אחד, סיכון נמוך, ועקביות בין שני מקורות האמת מובטחת מעצם הבנייה. זה בדיוק מה שנעשה לקבוצת SENTRY.
  • ה-66 — אין תיאור בשום מקום. כל אחד דורש ניסוח מהקוד, ובשני קבצים. שם הסיכון: תיאור שנשמע סביר ואינו נכון.

סדר מוצע: קודם 90, ואחר כך 66.

חלוקה לקבוצות

ALERT 12 · PREDICTIVE 9 · BACKUPS 8 · CACHE 8 · DISABLE 8 · ALERTS 7
METRICS 7 · GEMINI 6 · EMBEDDING 5 · OBS 5 · GITHUB 4 · שאר 79

קומיט לקבוצה. מאה וחמישים ושמונה שורות בקומיט אחד אינן ניתנות לביקורת.

איך נגזר השיוך לשירותים

ast בלבד:

שאלה המקור
מה מוצהר ConfigDefinition(...) ב-services/config_inspector_service.py
איפה נצרך os.getenv, os.environ, getattr(config, …), config.X, ושמות שדות ב-BotConfig — pydantic קורא לפי שם השדה
לאיזה שירות שייך הקובץ סגור ה-import מכל נקודת כניסה

הסגור מחושב פעמיים, וזה קריטי:

            ודאי   רופף   מותנה בלבד
bot           63    175      112
webapp        89    128       39
mcp           17     71       54
webserver     14    134      120

"ודאי" = ייבוא ברמת המודול. "רופף" = כולל ייבוא בתוך גופי פונקציות.

⚠️ הסקריפט מציע, הוא לא פוסק

זה לא זהירות תיאורטית — זה נמדד:

כיוון אחד — false positives. שני המועמדים הראשונים היו SENTRY_WEBHOOK_SECRET ו-SENTRY_WEBHOOK_DEDUP_WINDOW_SECONDS עם "חסר bot". בפועל main.py מייבא את services/webserver.py רק כאשר ENABLE_INTERNAL_SHARE_WEB דלוק — פיצ'ר שכבוי כברירת מחדל. שניים מתוך שניים נפסלו בבדיקה ידנית.

כיוון שני — false negatives. קבוצת SENTRY_POLL_* יצאה "לא ודאי" כי services/sentry_polling.py מיובא בתוך פונקציה. אבל main.py קורא SentryPoller.from_env() ורושם ג'וב חוזר ב-JobQueue — התהליך קורא את כולם בעלייה. סומנו bot בזכות הקריאה הידנית.

המסקנה: כל שורה שנכנסת מגיעה עם הראיה שלה — איזה קובץ קורא את המשתנה, ובאיזה סגור הוא יושב.

מה שהניתוח הזה לא יכול להוכיח

קריאה דינמית — os.getenv(name) עם משתנה ולא מחרוזת — קיימת בעשרות קבצים, בהם observability.py, metrics.py, resilience.py, services/job_registry.py, services/db_provider.py, mcp_server/repo_policy.py. משתנה שנקרא רק כך אינו נראה בניתוח סטטי. כך גם importlib דינמי.

גם אחרי שכל 158 ייכנסו, אי אפשר יהיה לטעון שהטבלה מלאה.

הפתרון האמיתי — בדיקה, לא מעבר ידני

מעבר חד-פעמי ייסחף שוב ברגע שמישהו יזיז os.getenv בין קבצים או יוסיף משתנה בלי הצהרה. הריפו כבר מכיר את הדפוס: tests/test_ai_map_freshness.py אוכף ש-AI-MAP.md תואם לתיעוד.

מוצע כסבב אחרון: בדיקה שמריצה את אותו ניתוח ונכשלת על משתנה שנצרך בקוד ואינו מוצהר — עם allowlist מפורש לחריגים (משתני מערכת, קבצי צד-שלישי, תשתית בדיקות) כדי שהיא לא תהפוך לרעש.

אימות לכל קבוצה

  • tests/test_config_inspector_service.py.
  • הרצת הסקריפט מחדש — הפער חייב לרדת בדיוק במספר השורות שנוספו. פער שלא זז, או שזז ביותר, אומר שההוספה שגויה. כך אומתה קבוצת SENTRY: 172 ← 158.
  • דגימה ידנית של שלוש שורות מול הקוד: הדיפולט זהה תו-בתו, והשירות תואם למה שהקובץ באמת נטען ממנו.
  • משתנה בלי דיפולט אמיתי מקבל default ריק, ולכן סטטוס Set ולא Modified שקרי — מכוסה ב-test_env_without_default_is_set_not_modified.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions