feat: הודעת שגיאת ולידציה עוברת את שער הפרטיות — חריג מגודר לסוג אחד

Merged
AM
2mo ago
+406-19

✨ תיאור קצר

כשל ולידציה הגיע לדשבורד כמונה בלי שם השדה שנדחה, כך ש"הסוכן טעה" ו"הסכימה לא ברורה" נראו זהים. מעכשיו $mcp_error_message נשלח כאשר $mcp_error_type הוא בדיוק ValidationError, ובשום מקרה אחר.

מומש באותו מנגנון של החריג הקיים מ-#3315 ולא כשער שני: מילון ממבחין לרשימת היתר נוספת, שנאספת לאותה קבוצה allowed באותה שורה. _ALLOWED_BY_EVENT מפתח לפי שם האירוע, _ALLOWED_BY_ERROR_TYPE לפי סוג השגיאה.

📦 שינויים עיקריים

  • קוד (Backend)
  • בוט טלגרם
  • מסד נתונים/מיגרציות
  • תיעוד (docs/)
  • DevOps/CI/CD

⚠️ העלות נמדדה לפני שהתקבלה, ולא הונחה

המחיר אמיתי, והוא אושר במפורש אחרי שהוצג. ההודעה של Pydantic מכילה את input_value — הערך שנדחה.

מדדתי על pydantic 2.12.3, הגרסה שמקובעת ב-requirements/base.txt:119 (ולא על 2.13.5 שמותקנת בסביבת הבדיקה שלי):

1 validation error for Args
file_name
  Field required [type=missing, input_value={'code': 'PASSWORD=hunter...nAWS_KEY=AKIA_BOTTOM\n'}, input_type=dict]

הערך מקוצץ ל-50 תווים — אבל הקיצוץ שומר ראש וזנב. קלט בן 500+ תווים שהסתיים ב-AWS_KEY=AKIA_BOTTOM החזיר את הזנב במלואו. סוד באמצע נבלע; סוד בקצה עובר. זו לא הגנה, זו הגרלה לפי מיקום.

והמקרה הרחב יותר הוא הנפוץ: כששדה חובה חסר (type=missing), Pydantic מדווח כ-input_value את אובייקט הארגומנטים כולו. כלומר codekeeper_save_file בלי file_name שם קטע מגוף הקובץ בהודעה. הכלים שנושאים תוכן: save_file, edit_file, append_file, create_note, update_note, note_str_replace.

הגידור הוא בהיקף ולא בניקוי — סוג שגיאה אחד, מפתח אחד. אין כאן ניסיון לחטא את המחרוזת של Pydantic, כי סניטציה מומצאת על טקסט חופשי נכשלת בשקט; זה בדיוק bugbot-rules/secret-in-derived-text.md.

האורך כבר מגודר על ידי ה-SDK. $mcp_error_message מקוצץ ל-2,048 תווים (posthog/mcp/_truncation.py:29). ב-Pydantic עצמו אין תקרה על מספר השגיאות — 300 שדות שגויים מייצרים הודעה של 77,619 תווים — ולכן הסתמכות על המקור בלבד לא הייתה מספיקה.

ומה שה-SDK לא עושה: המנקה שלו פועל מילה-מילה לפי רווחים, ומדדתי ש-KEY='sk-live-...' עובר בשלום כי הצורה key='value' היא מילה אחת שלא נראית כמו סוד. ה-docstring שלו אומר את זה בעצמו: "A host with strict requirements should gate free text in before_send".

🔒 fail-closed

$mcp_error_type חסר, לא-מחרוזת, או כל ערך אחר → ההודעה חסומה. אין ברירת מחדל מתירנית. $mcp_parameters נשאר חסום גם על האירוע הזה, כך שהחריג אינו הופך לדרך לקרוא את הארגומנטים.

הבדיקה היא על הטיפוס ולא על הייצוג הטקסטואלי, ולא בכדי — ראו את מוטציה 6 למטה.

🧪 בדיקות

17 טסטים חדשים. מוטציה לכל אחד:

#מוטציהתוצאה
1מסירים את החריג לגמרי3 נפלו
2התאמה מטושטשת במקום שוויון מדויק3 נפלו
3פותחים כל סוג שגיאה19 נפלו
4החריג פותח גם $mcp_parameters3 נפלו
5מפסיקים להשחיר את $exception_list3 נפלו
6str(value) במקום isinstance✅ אחרי תוספת
7type(value) is str — נוקשה מדי✅ אחרי תוספת

מוטציה 6 שרדה בסבב הראשון, וזה ממצא בטסטים ולא בקוד. str(None) הוא "None", str([...]) הוא "[...]" — אף אחד מהם אינו "ValidationError", אז כל המקרים שבדקתי חסמו בשתי הגרסאות. הפער היחיד הוא אובייקט שאינו מחרוזת אבל מדפיס את עצמו כשם המותר (StrEnum, wrapper של ספרייה) — והוא היה פותח את השער. $mcp_error_type מגיע מ-SDK חיצוני, אז זה לא תיאורטי. הוספתי טסט לשני הכיוונים, כדי שהתיקון גם לא יהפוך לנוקשה מדי וייחסם str subclass תקין.

  • RuntimeError נבדק במפורש — הסוג שכבר עקף את השער פעם אחת ב-#3315
  • ValueError, TypeError, KeyError, ToolError, Exception, מחרוזת ריקה — כולם חסומים
  • validationerror, VALIDATIONERROR, עם רווח מוביל או נגרר — כולם חסומים
  • None, 42, 3.5, True, רשימה, מילון — חסומים, ובלי להפיל את ה-hook
  • $mcp_error_type על אירוע missing_capability לא פותח שם הודעה
  • $exception_list[*].value נשאר מושחר גם על אירוע ולידציה
  • tests/test_mcp_*.py — 510 עוברים, אפס נופלים
  • ⚠️ flake8 אינו מותקן בקונטיינר הזה. הרצתי py_compile ואורך שורות מול max-line-length=120

📝 סוג שינוי

  • feat: פיצ'ר חדש

✅ צ'קליסט

  • הקוד עוקב אחרי הסגנון
  • בדיקות רצות ועוברות
  • תיעוד עודכן — ה-docstring של המודול ו-docs/mcp-server.rst אמרו שניהם שטקסט חופשי יוצא במסלול מוצהר אחד. שניהם אומרים עכשיו שניים, והשני נושא את המדידה ואת האזהרה. הצהרת פרטיות שמבטיחה יותר ממה שהקוד עושה גרועה ממסמך שלא מצהיר כלום — וזה בדיוק הממצא שנתפס בסבב ריוויו קודם
  • לא נוספו ג'ובים חדשים
  • לא נוספו משתני סביבה
  • אין סודות/מפתחות בקוד
  • אין מחיקות מסוכנות
  • הודעת הקומיט תואמת Conventional Commits
  • עיינתי במסמכי אתר התיעוד — docs/mcp-server.rst (סעיף mcp-analytics וטבלת המאפיינים), docs/doc-authoring.rst. וב-amir-bug-patterns: CRITICAL-PATTERNS.md K13 ו-bugbot-rules/secret-in-derived-text.md, לפי שורת הטריגר "הרכבת מחרוזת שמכילה סוד, הודעות חריגה, ניקוי לוגים"

🧩 השפעות/סיכונים

השינוי כולו ב-mcp_server/analytics.py ובתיעוד. אין נגיעה בכלים, בשרת או במסד.

הסיכון הוא זה שמתואר למעלה, והוא נבחר במודע: קטע מתחילת או מסוף ערך שנדחה יגיע לפוסטהוג בכל שגיאת ולידציה על כלי שנושא תוכן. שגיאות כאלה אינן נדירות — שתיים קרו בסשן הפיתוח עצמו.

🧯 סיכון / החזרה לאחור (Rollback)

מחיקת הערך מ-_ALLOWED_BY_ERROR_TYPE מחזירה את ההודעה למצב חסום. אין מיגרציה ואין שינוי סכימה.

🔗 קישורים

  • ממשיך את #3315, שבנה את שער הפרטיות ואת מנגנון החריג
  • ה-SDK: posthog 7.45.3; pydantic מקובע ל-2.12.3

🤖 Generated with Claude Code

https://claude.ai/code/session_0154RpRM1aCgLXCwUqDgywYp


Generated by Claude Code

Summary by Sourcery

Permit validation error messages to pass the MCP analytics privacy gate under a narrowly scoped, fail-closed exception while keeping all other sensitive payloads protected.

New Features:

  • Allow $mcp_error_message through the privacy gate only for exactly typed ValidationError events.

Bug Fixes:

  • Prevent structured or non-string values from bypassing conditional privacy allowlists, including the existing missing-capability exception.

Enhancements:

  • Keep error parameters, exception values, and all unrelated error messages redacted while preserving fail-closed behavior for invalid discriminator values.

Documentation:

  • Update MCP analytics privacy documentation to describe the validation-error exception and its measured privacy trade-off.

Tests:

  • Add comprehensive privacy tests covering exact error-type matching, fail-closed handling, structured payloads, exception-list redaction, and string-type validation.