From 482f302d5461dd6227c8635a63e2955df7e4eed8 Mon Sep 17 00:00:00 2001 From: Chaim Date: Sat, 6 Jun 2026 14:14:39 +0000 Subject: [PATCH] fix(security+agents): GAP-57 fail-loud PAPERCLIP_DB_URL + FU-13 analyst tool alignment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GAP-57 (אבטחה, CWE-798 / INV-ENV4): ה-default הקשיח postgresql://paperclip:paperclip@... הוסר מ-3 קבצי web/. נוסף resolver משותף require_paperclip_db_url() ב-paperclip_api.py שנכשל בקול אם PAPERCLIP_DB_URL לא מוגדר — במקום ליפול בשקט ל-creds ידועים. Coolify מגדיר את המשתנה (אומת), אז הייצור לא נפגע. (2 מופעים בסקריפטים מקומיים נותרו ל-FU-15 המלא.) FU-13 (INV-AG3, GAP-46): יישור הרשאות-סוכן. התברר שהפער שמופה ב-31.5 היה רחב מדי — יוחס לפי תיאור-תפקיד, לא ההוראות בפועל. הכרעת-יו"ר "היבריד": - legal-analyst: נוסף aggregate_claims_to_arguments (frontmatter + שלב 7) — הכלי שמקבץ את הטענות שהוא חילץ לטיעונים משפטיים. - extract_references/extract_internal_citations הם מטלת-researcher (שכבר מחזיק אותם), לא analyst — הוסרו מרשימת "החסרים". - legal-researcher: כבר היה תקין; ה-spec היה מיושן. עודכנו X4-agents.md (§2א, INV-AG3) ו-gap-audit.md (FU-13 ✅, FU-15 חלקי). Co-Authored-By: Claude Opus 4.8 (1M context) --- .claude/agents/legal-analyst.md | 2 ++ docs/spec/X4-agents.md | 20 +++++++++++++------- docs/spec/gap-audit.md | 6 +++++- web/app.py | 11 ++++------- web/paperclip_api.py | 19 +++++++++++++++++++ web/paperclip_client.py | 7 +++---- 6 files changed, 46 insertions(+), 19 deletions(-) diff --git a/.claude/agents/legal-analyst.md b/.claude/agents/legal-analyst.md index 75c6b14..6a676e6 100644 --- a/.claude/agents/legal-analyst.md +++ b/.claude/agents/legal-analyst.md @@ -16,6 +16,7 @@ tools: - mcp__legal-ai__extract_claims - mcp__legal-ai__extract_appraiser_facts - mcp__legal-ai__get_claims + - mcp__legal-ai__aggregate_claims_to_arguments - mcp__legal-ai__search_case_documents - mcp__legal-ai__search_decisions - mcp__legal-ai__search_precedent_library @@ -122,6 +123,7 @@ tools: - **טיפול בכשל:** אם `extract_claims` החזיר `partial=true` או 0 טענות ממסמך לא ריק — נסה שוב פעם אחת. אם עדיין נכשל — סטטוס issue = `blocked`, פרסם comment עם הפירוט. 5. **חלץ עובדות שמאי** — לכל מסמך `doc_type='appraisal'` בתיק, הרץ `extract_appraiser_facts(case_number)` (פעם אחת לתיק, מטפל בכל השומות). **חובה בכל ערר השבחה (8xxx) ופיצויים (9xxx) — בלי זה ה-writer לא יוכל לכתוב את בלוק ז עם מספרים מדויקים.** 6. וודא שכל פריט מסווג ל-claim_type הנכון +7. **קבץ טענות לטיעונים משפטיים** — לאחר שכל הטענות חולצו וסוּוגו, הרץ `aggregate_claims_to_arguments(case_number)` שמקבץ את הפרופוזיציות הגולמיות לטיעונים משפטיים מובחנים (~6-12 לכל צד). זהו קלט מובנה לבלוק ז (טענות הצדדים) ולבלוק י (דיון) — הכותב נשען עליו. אם 0 טענות חולצו — דלג. הפלט עובר שער-אישור (ראה `get_legal_arguments`). ### שלב 2: ניתוח מעמיק הצג במבנה הבא: diff --git a/docs/spec/X4-agents.md b/docs/spec/X4-agents.md index 02d0627..c47df7f 100644 --- a/docs/spec/X4-agents.md +++ b/docs/spec/X4-agents.md @@ -64,12 +64,18 @@ Paperclip בקונפליקט (project-specific מנצח default), אך אינו כל קובץ-סוכן מצהיר ב-frontmatter `tools:` (כולם: `Read/Bash/Grep/Glob` + תת-קבוצת `mcp__legal-ai__*`). מפת-ההרשאות חייבת **לתאום** את מה שהוראות-הסוכן מצריכות ([X9 INV-TOOL6](X9-mcp-tool-contract.md), INV-AG3 להלן). -פערים שנמצאו (אומת 31.5.2026 — `legal-analyst.md` לא מעניק את 3 הכלים; תיקון ב-**FU-13**): -| סוכן | כלי-חילוץ שההוראות דורשות אך **לא** מוענקים | -|------|---------------------------------------------| -| legal-analyst | `aggregate_claims_to_arguments`, `extract_references`, `extract_internal_citations` | -| legal-researcher | `precedent_extract_halachot`, `precedent_extract_metadata`, `precedent_process_pending` | +**סטטוס FU-13 — נסגר (2026-06-06):** GAP-46 טופל בהכרעת-יו"ר "היבריד". התברר שהפער שמופה ב-31.5 +היה רחב מדי — הכלים יוחסו לפי *תיאור-התפקיד*, לא לפי ההוראות בפועל. ההכרעה: + +| סוכן | מצב בפועל | פעולה ב-FU-13 | +|------|-----------|----------------| +| legal-researcher | כבר מעניק `extract_references` + `precedent_extract_halachot`/`precedent_extract_metadata`/`precedent_process_pending` (frontmatter) | ✅ אין פער — היה מיושן | +| legal-analyst | חסר `aggregate_claims_to_arguments`; הוראותיו לא השתמשו בו | ✅ נוסף ל-frontmatter + שלב 7 ב-"שלב 1" (קיבוץ טענות→טיעונים) | + +`extract_references` / `extract_internal_citations` הם **מטלת-מחקר** (חילוץ ציטוטים/רפרנסים) ושייכים +ל-`legal-researcher` (שמחזיק אותם) — **לא** ל-`legal-analyst`, שמאמת פסיקה דרך *חיפוש* (§8א בקובץ-הסוכן), +לא חילוץ. לכן הוסרו מרשימת "החסרים" של ה-analyst (INV-AG3 "לא עודף"). → [gap-audit GAP-46](gap-audit.md). @@ -131,8 +137,8 @@ another company`, [X2 §2](X2-multi-company.md)). מצריכות מוענק, וכלי שמוענק-ולא-בשימוש נבחן. מופע של [G10](00-constitution.md#inv-g10-המערכת-מסייעת--שערים-אנושיים-הם-invariant) (שערים מוגדרים) ו-[G2](00-constitution.md#inv-g2-מקור-אמת-יחיד--אין-מסלולים-מקבילים-מתפצלים); מקביל ל-[X9 INV-TOOL6](X9-mcp-tool-contract.md). **מקור-סמכות:** frontmatter `tools:` מול ה-instructions בקבצי-[.claude/agents/](../../.claude/agents/). (פרויקטלי-תפעולי.) -**אכיפה:** בדיקת-עקביות tools↔instructions (יעד FU-13). **כיום אין.** -**הפרה ידועה:** legal-analyst חסר `aggregate_claims_to_arguments`/`extract_references`/`extract_internal_citations`; researcher חסר טריגרי-חילוץ (§2א; [gap-audit GAP-46](gap-audit.md)). +**אכיפה:** בדיקת-עקביות tools↔instructions (FU-13 ✅ 2026-06-06). אכיפה אוטומטית עתידית — בתת-פרויקט 5 (spec-guardian). +**הפרה ידועה:** — (טופל ב-FU-13: legal-analyst קיבל `aggregate_claims_to_arguments`; researcher כבר היה תקין; `extract_references`/`extract_internal_citations` הם מטלת-researcher, לא analyst — ראה §2א). --- diff --git a/docs/spec/gap-audit.md b/docs/spec/gap-audit.md index 2cfe306..2d2af89 100644 --- a/docs/spec/gap-audit.md +++ b/docs/spec/gap-audit.md @@ -188,9 +188,12 @@ - **מכסה:** GAP-38..43 · **invariants:** INV-DM4–DM6 · **effort:** M · **תלויות:** FU-1 - **סוג:** code + data-migration קל — provenance, שער-אישור ל-legal_arguments, CHECK-enums, FK, איחוד case_precedents -### FU-13 — סוכנים + skills +### FU-13 — סוכנים + skills — ✅ נסגר (2026-06-06) - **מכסה:** GAP-46 (מרחיב GAP-23) · **invariants:** INV-AG3, INV-TOOL6 · **effort:** S · **תלויות:** ה-spec יציב - **סוג:** code/docs — שלמות-הרשאות (tools↔instructions), DRY-boilerplate, dedup-skills +- **סטטוס:** הכרעת-יו"ר "היבריד". התברר שהפער ב-31.5 היה רחב מדי (יוחס לפי תיאור-תפקיד, לא הוראות בפועל). + researcher כבר היה תקין (מיושן ב-spec). analyst קיבל `aggregate_claims_to_arguments` + שלב 7 ("שלב 1"); + `extract_references`/`extract_internal_citations` נשארו אצל researcher (מטלת-מחקר, לא analyst). עודכן [X4 §2א](X4-agents.md). ### FU-14 — חוזה כלי-ה-MCP - **מכסה:** GAP-44,45,47..54 · **invariants:** INV-TOOL1–TOOL5 · **effort:** L · **תלויות:** FU-1 @@ -199,6 +202,7 @@ ### FU-15 — deploy/env/secrets - **מכסה:** GAP-55..62 · **invariants:** INV-ENV1–ENV5 · **effort:** M · **תלויות:** — - **סוג:** code/config + **chair-decision** (rotation סודות) — env-catalog SSoT, מקור-config יחיד, de-hardcode, drift מלא, start.sh עמיד +- **סטטוס חלקי:** GAP-57 (creds plaintext, אבטחה CWE-798) **נסגר ב-web/ 2026-06-06** — 3 מופעים ב-`web/paperclip_api.py`/`paperclip_client.py`/`app.py` הומרו ל-`require_paperclip_db_url()` fail-loud. נותרו 2 מופעים בסקריפטים מקומיים (`sync_agents_across_companies.py`, `sync_missing_agent_skills.py`) + GAP-55,56,58–62 — לטיפול ב-FU-15 המלא. --- diff --git a/web/app.py b/web/app.py index 8df0601..e3b249b 100644 --- a/web/app.py +++ b/web/app.py @@ -45,7 +45,7 @@ from web.mcp_env_catalog import ( normalize_for_compare, ) from web.progress_store import ProgressStore -from web.paperclip_api import emit_case_status_webhook, pc_request +from web.paperclip_api import emit_case_status_webhook, pc_request, require_paperclip_db_url from web.paperclip_client import ( COMPANIES as PAPERCLIP_COMPANIES, accept_interaction as pc_accept_interaction, @@ -3907,9 +3907,7 @@ async def api_mcp_blocks(): @app.get("/api/settings/paperclip-companies") async def api_paperclip_companies(): """List all companies from Paperclip's DB.""" - pc_url = os.environ.get( - "PAPERCLIP_DB_URL", "postgresql://paperclip:paperclip@127.0.0.1:54329/paperclip" - ) + pc_url = require_paperclip_db_url() # INV-ENV4 / GAP-57: fail loud, no creds default try: conn = await asyncpg.connect(pc_url) try: @@ -4082,9 +4080,8 @@ async def api_reset_methodology(category: str, key: str): # ── Skill Management API ─────────────────────────────────────────── -PAPERCLIP_DB_URL = os.environ.get( - "PAPERCLIP_DB_URL", "postgresql://paperclip:paperclip@127.0.0.1:54329/paperclip" -) +# INV-ENV4 / GAP-57: no hard-coded credential default — fail loud if unset. +PAPERCLIP_DB_URL = require_paperclip_db_url() # Paperclip skills directory. In the Coolify container this is bind-mounted from # the host's ~/.paperclip/instances/default/skills (see PAPERCLIP_SKILLS_DIR env). # Fallback to the host path for local/dev use. diff --git a/web/paperclip_api.py b/web/paperclip_api.py index 9435f4b..d8de22f 100644 --- a/web/paperclip_api.py +++ b/web/paperclip_api.py @@ -32,6 +32,25 @@ PAPERCLIP_BOARD_API_KEY = os.environ.get("PAPERCLIP_BOARD_API_KEY", "") DEFAULT_TIMEOUT = 15.0 +def require_paperclip_db_url() -> str: + """Return ``PAPERCLIP_DB_URL`` or fail loud — never fall back to a default. + + INV-ENV4 / GAP-57: a credential must not live in source as a default value. + The Coolify container and local tooling supply this explicitly; its absence + is a misconfiguration we surface immediately rather than silently connecting + with a well-known default credential (CWE-798). Mirrors the fail-loud guard + on ``PAPERCLIP_BOARD_API_KEY`` in ``_build_headers``. + """ + url = os.environ.get("PAPERCLIP_DB_URL", "") + if not url: + raise RuntimeError( + "PAPERCLIP_DB_URL not set — refusing hard-coded credential fallback " + "(INV-ENV4 / GAP-57). Set PAPERCLIP_DB_URL in the environment " + "(Coolify for the container, or your shell/.env for local tooling)." + ) + return url + + def _build_headers(run_id: str | None, has_body: bool) -> dict[str, str]: if not PAPERCLIP_BOARD_API_KEY: raise RuntimeError("PAPERCLIP_BOARD_API_KEY not set — cannot call Paperclip API") diff --git a/web/paperclip_client.py b/web/paperclip_client.py index efb1ee6..05d4f73 100644 --- a/web/paperclip_client.py +++ b/web/paperclip_client.py @@ -13,13 +13,12 @@ import uuid import asyncpg -from web.paperclip_api import pc_request +from web.paperclip_api import pc_request, require_paperclip_db_url logger = logging.getLogger(__name__) -PAPERCLIP_DB_URL = os.environ.get( - "PAPERCLIP_DB_URL", "postgresql://paperclip:paperclip@127.0.0.1:54329/paperclip" -) +# INV-ENV4 / GAP-57: no hard-coded credential default — fail loud if unset. +PAPERCLIP_DB_URL = require_paperclip_db_url() PLUGIN_ID = "53461b5a-7f58-411a-9952-72f9c8d4a328" # marcusgroup.legal-ai # PAPERCLIP_API_URL — moved to web.paperclip_api (used only by pc_request now).