From 5f5b13c6a40c70eb5339cd1a527b9d44fa2bc61e Mon Sep 17 00:00:00 2001 From: Chaim Date: Wed, 5 Aug 2026 08:53:53 +0000 Subject: [PATCH] =?UTF-8?q?feat(guard):=20=D7=9B=D7=9C=D7=9C-=D7=A7=D7=A9?= =?UTF-8?q?=D7=99=D7=A8=D7=94=20=D7=9C-INV-AG3=20=E2=80=94=20=D7=94=D7=A9?= =?UTF-8?q?=D7=A2=D7=A8=20=D7=93=D7=99=D7=95=D7=95=D7=97=20"=D7=AA=D7=A7?= =?UTF-8?q?=D7=99=D7=9F"=20=D7=A2=D7=9C=20allow-list=20=D7=A9=D7=90=D7=99?= =?UTF-8?q?=D7=A9=20=D7=90=D7=99=D7=A0=D7=95=20=D7=90=D7=95=D7=9B=D7=A3?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit כללים 1–4 משווים קבצים לקבצים, וזה לא אומר דבר על **אכיפה**. רשימת tools: הופכת ל-allow-list רק כשה-runtime בוחר את הסוכן (--agent ); בלעדיו אותו קובץ בדיוק נמסר כ---append-system-prompt-file — פרוזה שהמודל רשאי לקיים או להתעלם ממנה — וכל 108 הכלים נשארים נגישים. התגלה 2026-08-05: סוכן אחד נושא 41 הענקות **בלי** --agent. זו הרשימה הגדולה במערכת, והשער דיווח עליה OK — כי הוא מעולם לא הסתכל על הקשירה. שער שמדווח "תקין" על רשימה בלתי-נאכפת גרוע מהיעדר שער: הוא מייצר ביטחון-שווא. כלל 5 (host-only, --check-bindings): מוודא ש-extraArgs מכיל ["--agent", ] התואם ל-instructionsEntryFile. הקשירה יושבת ב-DB של הפלטפורמה, שה-CI לא רואה, ולכן הבדיקה היא opt-in ומריצה psql בתת-תהליך במקום להוסיף תלות-דרייבר שהייתה שוברת את תכונת ה-stdlib-בלבד שמסלול ה-CI נשען עליה. DB בלתי-נגיש מחזיר [] — "לא נבדק", לא "אין הפרות". ובמסלול ה-CI, השער אומר עכשיו במפורש מה **לא** נבדק, במקום להדפיס OK חשוף שמזמין את הקורא להסיק שהרשימות נאכפות. מדווח רק על סוכנים שבאמת מצהירים הענקות — סוכן בלי tools: אינו ממצא. אומת: מסלול-CI ירוק; --check-bindings תופס את legal-ceo עם הנימוק "extraArgs is empty" ומציע את שתי החלופות (להוסיף --agent, או למחוק tools: ולתעד כבלתי-מוגבל) — במפורש לא שתיהן, כי זו הפרת G2. invariants: INV-AG3 — מרחיב מקובץ לזמן-ריצה. G2 — מקיים; אין מפת-הרשאות שנייה, רק אימות שהמפה הקיימת נאכפת. Co-Authored-By: Claude Opus 5 --- scripts/SCRIPTS.md | 2 +- scripts/agent_tool_grants_guard.py | 113 +++++++++++++++++++++++++++++ 2 files changed, 114 insertions(+), 1 deletion(-) diff --git a/scripts/SCRIPTS.md b/scripts/SCRIPTS.md index 6782dbb..e884181 100644 --- a/scripts/SCRIPTS.md +++ b/scripts/SCRIPTS.md @@ -99,7 +99,7 @@ |--------|------|---------|-----------| | `spec-guard.sh` | bash | **PreToolUse hook לאכיפת "פרוטוקול כתיבת-קוד"** (CLAUDE.md §פרוטוקול כתיבת-קוד) — בכל Edit/Write/MultiEdit על נתיב-קוד (`web/`, `mcp-server/`, `web-ui/src/`, `scripts/`, `adapters/`) מזריק תזכורת ל-Claude לקרוא את `docs/spec/00-constitution.md`+ספ-התחום ולוודא קיום G1–G12 — לפני שכותבים. **+ leak-guard בזמן-אמת (G12):** על כתיבה ל-`mcp-server/src/*` בודק את התוכן-הנכתב (`new_string`/`content`) ומזהיר אם מוזרק מונח-Paperclip לשכבת-האינטליגנציה (לא-deduped). המקבילה האינטראקטיבית ל-INV-AG1. קלט JSON ב-stdin, פלט `hookSpecificOutput.additionalContext` (non-blocking, exit 0). Dedup פעם-בסשן לתזכורת-הספ. רשום ב-`.claude/settings.json`. | נקרא אוטומטית ע"י Claude Code (hook) | | `leak_guard.py` | python | **המאכף הקנוני של INV-G12 (שער-הפלטפורמה / docs/spec/X15 §4 / R4).** שני כללים קשיחים: (1) `mcp-server/src` ללא סמלי-Paperclip (allowlist מנומק לפי substring); (2) רק `web/agent_platform_port.py` (+ קבצי-המעטפת) מייבאים את לקוח-Paperclip. stdlib-בלבד (אין venv). `leak_guard.py` = סריקת-repo (exit 1 על הפרה); `leak_guard.py ...` = קבצים נתונים (ל-hook). משותף ל-spec-guard.sh (hook), ל-CI (`.gitea/workflows/leak-guard.yaml`) ול-`mcp-server/tests/test_platform_port_leak_guard.py`. | CI + hook + pytest | -| `agent_tool_grants_guard.py` | python | **המאכף הקנוני של INV-AG3 (מפת-הרשאות הסוכנים / docs/spec/X4-agents.md §2א).** ה-frontmatter `tools:` של סוכן הוא **allow-list סגורה** — כלי הרשום בשרת-ה-MCP אך חסר ממנה אינו ניתן לקריאה, גם כשהשרת מחובר. ארבעה כללים קשיחים: (1) כל `mcp__legal-ai__X` המופיע ב-`web/` (delegation שיוצר issue לסוכן) מוענק לסוכן כלשהו; (2) כל `mcp__legal-ai__X` בגוף קובץ-סוכן מוענק ב-frontmatter של **אותו** קובץ; (3) אין הענקה לכלי שאינו רשום ב-`@mcp.tool`; (4) שם-כלי בגרשיים-הפוכים ללא תחילית — מוענק, או מסווג ב-`CONTRASTIVE_OK` עם נימוק (כולל בדיקת-התיישנות לסיווגים). מחריג קבצים שאינם סוכני-claude_local: `hermes-curator.md`, `legal-analyst-gemini-critique.md`, `HEARTBEAT.md`. stdlib-בלבד. נבנה אחרי CMP-229 (2026-08-04) — `analyze_protocol` נרשם בשרת ב-2026-06-30 בלי הענקה, ו-#226 הורה למנתח להריץ אותו. CI: `.gitea/workflows/agent-tool-grants.yaml`. | CI | +| `agent_tool_grants_guard.py` | python | **המאכף הקנוני של INV-AG3 (מפת-הרשאות הסוכנים / docs/spec/X4-agents.md §2א).** ה-frontmatter `tools:` של סוכן הוא **allow-list סגורה** — כלי הרשום בשרת-ה-MCP אך חסר ממנה אינו ניתן לקריאה, גם כשהשרת מחובר. ארבעה כללים קשיחים: (1) כל `mcp__legal-ai__X` המופיע ב-`web/` (delegation שיוצר issue לסוכן) מוענק לסוכן כלשהו; (2) כל `mcp__legal-ai__X` בגוף קובץ-סוכן מוענק ב-frontmatter של **אותו** קובץ; (3) אין הענקה לכלי שאינו רשום ב-`@mcp.tool`; (4) שם-כלי בגרשיים-הפוכים ללא תחילית — מוענק, או מסווג ב-`CONTRASTIVE_OK` עם נימוק (כולל בדיקת-התיישנות לסיווגים). מחריג קבצים שאינם סוכני-claude_local: `hermes-curator.md`, `legal-analyst-gemini-critique.md`, `HEARTBEAT.md`. **כלל 5 (host-only, ‏`--check-bindings`):** allow-list נאכפת רק כשה-runtime בוחר את הסוכן (`--agent `); בלעדיו אותו קובץ נמסר כ-`--append-system-prompt-file` — פרוזה, לא שער — וכל הכלים נשארים נגישים. הקשירה יושבת ב-DB של הפלטפורמה ולכן ה-CI לא רואה אותה; בלי הדגל השער **אומר זאת במפורש** במקום לרמוז על אכיפה שלא אימת. stdlib-בלבד. נבנה אחרי CMP-229 (2026-08-04) — `analyze_protocol` נרשם בשרת ב-2026-06-30 בלי הענקה, ו-#226 הורה למנתח להריץ אותו. CI: `.gitea/workflows/agent-tool-grants.yaml`. | CI | | `check_undefined_names.py` | python | **CI gate ל-undefined names (מחלקת ה-NameError).** מריץ pyflakes על `web`, `mcp-server/src`, `scripts` ומפיל build (exit 1) רק על "undefined name"/"may be undefined" — לא על imports-לא-בשימוש/f-strings (רעש). זו בדיוק מחלקת-הבאג של PR #249 (שינוי-שם תיק → 500): שם שמופנה אך לא מיובא/מוגדר, חבוי בתוך `background_tasks` עד זמן-ריצה. דורש pyflakes (ה-workflow מתקין ל-venv זמני). משותף ל-CI (`.gitea/workflows/lint.yaml`). | CI | | `auto-sync-cases.sh` | bash | סנכרון תיקי ערר ל-Gitea — רץ כל דקה | `* * * * *` (cron) | | `host_sync.sh` | bash | מסנכרן את עץ-המארח `~/legal-ai` ל-origin/main (ff-only) כדי שקוד-המארח (כותב/פאנלים/MCP שרצים מהעץ, לא בקונטיינר) יתעדכן אחרי merge; restart מדויק ל-chat/court-fetch/reaper רק כשקבציהם משתנים. בטוח: אף-פעם לא force; tasks.json הדירטי נשמר. סוגר את פער-פריסת-המארח (TaskMaster #160) | `* * * * *` (cron, flock) | diff --git a/scripts/agent_tool_grants_guard.py b/scripts/agent_tool_grants_guard.py index 6e918c6..6eb66f7 100755 --- a/scripts/agent_tool_grants_guard.py +++ b/scripts/agent_tool_grants_guard.py @@ -40,6 +40,18 @@ Plus one reviewed-exception rule: job, or a DB column that merely shares a tool's name. Each is classified once in ``CONTRASTIVE_OK`` below; anything new fails until reviewed. +And one host-only rule, opt-in via ``--check-bindings``: + + 5. **Bindings.** Rules 1–4 compare files to files, which says nothing about + whether an allow-list is *enforced*. It is only enforced when the runtime + selects that agent (``--agent ``); without the flag the same file is + delivered as ``--append-system-prompt-file`` — prose, not a gate — and every + tool stays reachable. Found on 2026-08-05: one agent declared 41 grants with + no ``--agent`` flag, so the largest allow-list in the system was inert while + this guard reported OK. The binding lives in the platform DB, so CI cannot + see it; without the flag the guard now says so out loud instead of implying + enforcement it never verified. + NOT AGENTS (no frontmatter by design — the adapter sends the file as a raw prompt, so YAML would leak into it): ``hermes-curator.md`` (deepseek_local), ``legal-analyst-gemini-critique.md`` (gemini_local). ``HEARTBEAT.md`` is a @@ -50,6 +62,9 @@ Usage: """ from __future__ import annotations +import json +import os +import subprocess import re import sys from pathlib import Path @@ -110,6 +125,79 @@ def agent_files() -> list[Path]: return sorted(p for p in AGENTS_DIR.glob("*.md") if p.name not in NOT_AGENTS) +def check_bindings() -> list[tuple[str, str]]: + """Return [(agent file, why)] for agents whose allow-list nothing enforces. + + A ``tools:`` list is only an allow-list when the runtime is told which agent + to be. The local adapter enforces it under ``--agent ``; without that + flag the very same file is delivered as ``--append-system-prompt-file``, i.e. + prose the model may follow or ignore, and every tool stays reachable. + + Found the hard way on 2026-08-05: one agent carried 41 grants and no + ``--agent`` flag, so the largest allow-list in the system was inert — and + this guard had been reporting OK on it, because Rules 1–4 only ever compare + files to files. + + Host-only. The binding lives in the platform's database, which CI cannot + reach, so this shells out to ``psql`` rather than adding a driver dependency + that would break the stdlib-only property the CI path relies on. Returns [] + when the database is unreachable — an unreachable DB is "not checked", not + "no violations", and the caller prints that distinction. + """ + + sql = ( + "select adapter_config->>'instructionsEntryFile', " + "coalesce(adapter_config->>'extraArgs','') " + "from agents where adapter_type='claude_local' " + "and adapter_config->>'instructionsEntryFile' is not null;" + ) + try: + out = subprocess.run( + ["psql", "-h", "localhost", "-p", "54329", "-U", "paperclip", + "-d", "paperclip", "-X", "-A", "-t", "-F", "\t", "-c", sql], + capture_output=True, text=True, timeout=20, + env={**os.environ, "PGPASSWORD": os.environ.get("PGPASSWORD", "paperclip")}, + ) + except (OSError, subprocess.SubprocessError): + return [] + if out.returncode != 0: + return [] + + bad: dict[str, str] = {} + for line in out.stdout.splitlines(): + if "\t" not in line: + continue + entry_file, extra = line.split("\t", 1) + entry_file = entry_file.strip() + if not entry_file or entry_file in NOT_AGENTS: + continue + want = entry_file[:-3] if entry_file.endswith(".md") else entry_file + try: + args = json.loads(extra) if extra.strip() else [] + except json.JSONDecodeError: + args = [] + # Only a literal ["--agent", ""] pair binds the allow-list. + ok = any( + a == "--agent" and i + 1 < len(args) and args[i + 1] == want + for i, a in enumerate(args) + ) + if not ok: + bad[entry_file] = ( + "extraArgs is empty" if not args + else f"extraArgs={extra.strip()} does not select '{want}'" + ) + + # Only report agents that actually declare grants — an agent with no tools: + # list has nothing to enforce and is not a finding. + result = [] + for path in agent_files(): + if path.name in bad: + fm, _ = split_frontmatter(path.read_text(encoding="utf-8", errors="ignore")) + if TOOL_RE.findall(fm): + result.append((path.name, bad[path.name])) + return sorted(result) + + def main() -> int: registered = server_tools() if not registered: @@ -183,6 +271,20 @@ def main() -> int: f"file — drop the exception." ) + # Rule 5 — a grant list only binds if the runtime actually selects that agent. + unenforced = check_bindings() if "--check-bindings" in sys.argv else None + if unenforced: + for name, detail in unenforced: + violations.append( + f"[5 binding] .claude/agents/{name} declares a tools: allow-list, but " + f"the runtime does not select that agent — {detail}.\n" + f" The list is inert: it is delivered as prompt text only, so " + f"every tool remains callable.\n" + f" fix: set adapter_config.extraArgs to " + f'["--agent", "{name[:-3]}"], or drop tools: and document the agent as ' + f"unrestricted. Not both." + ) + if violations: print(f"INV-AG3 agent-tool-grants guard: {len(violations)} violation(s)\n") for v in violations: @@ -199,6 +301,17 @@ def main() -> int: f"({len(agent_files())} agents, {len(all_granted)} distinct grants, " f"{len(registered)} tools registered)" ) + if unenforced is None: + # Say plainly what was NOT checked. A guard that prints a bare "OK" invites + # the reader to conclude the allow-lists are enforced; this one has only + # compared files to files. Enforcement is a runtime property (see Rule 5), + # and on 2026-08-05 exactly one agent was found declaring 41 grants that + # nothing enforces — while this guard reported OK. + print( + " note: file-level only. Whether each allow-list is actually ENFORCED " + "depends on the\n runtime passing --agent , which needs the platform " + "DB — re-run with --check-bindings\n on the host to verify." + ) return 0