diff --git a/.gitea/workflows/deploy.yaml b/.gitea/workflows/deploy.yaml index f2f2549..d025dee 100644 --- a/.gitea/workflows/deploy.yaml +++ b/.gitea/workflows/deploy.yaml @@ -8,7 +8,8 @@ on: env: REGISTRY: gitea.dev.marcus-law.co.il IMAGE: mcp-servers/nadlan-mcp - COOLIFY_UUID: b8uf2yysbgeo1un4r9fbwt35 + # COOLIFY_UUID is provided as a Gitea Actions secret to avoid leaking + # the infra mapping in source control. jobs: build-and-deploy: @@ -51,5 +52,5 @@ jobs: - name: Trigger Coolify redeploy run: | curl -sf \ - "http://coolify:8080/api/v1/deploy?uuid=${{ env.COOLIFY_UUID }}&force=true" \ + "http://coolify:8080/api/v1/deploy?uuid=${{ secrets.COOLIFY_UUID }}&force=true" \ -H "Authorization: Bearer ${{ secrets.COOLIFY_TOKEN }}" diff --git a/.gitignore b/.gitignore index 4bab920..0bb3d4a 100644 --- a/.gitignore +++ b/.gitignore @@ -302,3 +302,25 @@ PHASE*.md # Archived files (historical reference, not tracked) .archive/ + +dev-debug.log +# Environment variables +# Editor directories and files +.idea +.vscode +*.suo +*.ntvs* +*.njsproj +*.sln +*.sw? +# OS specific + +# Task files +# tasks.json +# tasks/ + +# Task Master config (may contain API keys) +.taskmaster/config.json +.taskmaster/state.json +.taskmaster/reports/ + diff --git a/.taskmaster/tasks/tasks.json b/.taskmaster/tasks/tasks.json new file mode 100644 index 0000000..25942a5 --- /dev/null +++ b/.taskmaster/tasks/tasks.json @@ -0,0 +1,229 @@ +{ + "master": { + "tasks": [ + { + "id": "1", + "title": "C-1 [Critical]: Fix SSRF via userinfo in PDF proxy", + "description": "urlparse().hostname check in /api/appraisals/pdf is bypassable. URLs like https://attacker.com@pub-justice.openapi.gov.il/x.pdf pass the whitelist but curl_cffi connects to attacker.com.", + "details": "File: nadlan_mcp/web_app.py — proxy_appraisal_pdf. Validate parsed.username is None, parsed.password is None, and parsed.netloc == parsed.hostname (no userinfo, no port). Add unit test with malicious URL.", + "testStrategy": "Unit test: pass crafted userinfo URL, expect 400. Verify whitelisted URLs still work.", + "priority": "high", + "status": "in-progress", + "dependencies": [], + "subtasks": [], + "updatedAt": "2026-04-25T17:42:36.172Z" + }, + { + "id": "2", + "title": "H-1 [High]: Stop returning str(e) in HTTPException details", + "description": "All 4 endpoints return raw exception text to clients via HTTPException(detail=str(e)), leaking internal hostnames, paths, library errors.", + "details": "Files: nadlan_mcp/web_app.py — search_address, search_deals, search_appraisals, proxy_appraisal_pdf. Use logger.exception(...) for full detail; return generic detail='Service temporarily unavailable' to clients.", + "testStrategy": "Trigger an upstream failure (mock), verify response detail is generic and full error is in logs.", + "priority": "high", + "status": "pending", + "dependencies": [], + "subtasks": [] + }, + { + "id": "3", + "title": "H-3 [High]: Disable Swagger UI in production", + "description": "/api/docs and /api/openapi.json publicly expose full API schema, aiding attacker reconnaissance.", + "details": "File: nadlan_mcp/web_app.py — FastAPI(...). Use ENVIRONMENT env var; set docs_url=None and openapi_url=None when production.", + "testStrategy": "GET /api/docs in prod → 404. In dev → 200.", + "priority": "high", + "status": "pending", + "dependencies": [], + "subtasks": [] + }, + { + "id": "4", + "title": "H-4 [High]: Stream PDF proxy response, enforce max-size", + "description": "PDF proxy buffers entire response.content in memory before yielding, enabling OOM via large files (especially when combined with C-1 SSRF).", + "details": "File: nadlan_mcp/web_app.py — proxy_appraisal_pdf. Use iter_content(chunk_size=8192). Check Content-Length header → reject >50MB. Add 5-min total timeout.", + "testStrategy": "Mock upstream with 100MB body → expect 413. With 1MB body → 200, streamed.", + "priority": "high", + "status": "pending", + "dependencies": [ + "1" + ], + "subtasks": [] + }, + { + "id": "5", + "title": "C-2 [Critical]: Enable mTLS / API authentication", + "description": "App is fully public — no auth on /mcp, /api/search/*, /api/docs. Anyone can scrape data, abuse MCP tools, or get our IP banned by Govmap.", + "details": "Apply Traefik mtls-forwardauth@file middleware (existing infrastructure under mtls-auth-service). Coolify custom_labels need mtls-optional@file TLS option + middleware. /mcp may need separate API-key gate. Public health endpoint must remain reachable for Coolify probe.", + "testStrategy": "Without client cert → 403. With valid cert from PKI → 200. /api/health remains 200 anonymously.", + "priority": "high", + "status": "pending", + "dependencies": [], + "subtasks": [] + }, + { + "id": "6", + "title": "H-2 [High]: Add HTTP security headers middleware", + "description": "Missing HSTS, X-Frame-Options, X-Content-Type-Options, Referrer-Policy, CSP, Permissions-Policy.", + "details": "File: nadlan_mcp/web_app.py — add middleware setting all standard headers. CSP needs to allow Google Fonts (until L-1 self-hosts) or self only.", + "testStrategy": "curl -sI / → expect all 6 headers present with sane values.", + "priority": "high", + "status": "pending", + "dependencies": [], + "subtasks": [] + }, + { + "id": "7", + "title": "H-5 [High]: Per-IP rate limit on /mcp and /api/*", + "description": "Govmap client rate-limits at server level — single attacker can saturate it for everyone, get IP banned upstream.", + "details": "Use slowapi (FastAPI rate-limiter) or Traefik RateLimit middleware. Suggested: 60 req/min per IP on /api/search/*, 30 req/min on /mcp.", + "testStrategy": "Hammer 100 reqs/sec from single IP → expect 429 after limit.", + "priority": "high", + "status": "pending", + "dependencies": [], + "subtasks": [] + }, + { + "id": "8", + "title": "M-1 [Medium]: Tighten CORS configuration", + "description": "allow_headers=['*'] is overly permissive; localhost origins hardcoded for prod.", + "details": "File: nadlan_mcp/web_app.py. Set allow_headers=['Content-Type'], allow_methods=['GET','POST','OPTIONS'], allow_origins from ALLOWED_ORIGINS env var.", + "testStrategy": "Origin header from disallowed source → no Access-Control-Allow-Origin in response.", + "priority": "medium", + "status": "pending", + "dependencies": [], + "subtasks": [] + }, + { + "id": "9", + "title": "M-2 [Medium]: Switch Deal model to extra='forbid'", + "description": "Deal model accepts arbitrary fields from Govmap and propagates them to UI/MCP responses, enabling schema confusion attacks.", + "details": "File: nadlan_mcp/govmap/models.py. Add explicit fields for gushNum, parcelNum, subParcelNum, streetNameHeb, settlementNameHeb, houseNum (currently extras). Switch to extra='ignore' (forbid is too strict — Govmap adds fields). Update tests.", + "testStrategy": "Existing tests pass; new fields appear in serialized output; unknown upstream fields are ignored, not propagated.", + "priority": "medium", + "status": "pending", + "dependencies": [], + "subtasks": [] + }, + { + "id": "10", + "title": "M-3 [Medium]: Add regex validators on input fields", + "description": "Pydantic models only enforce min/max length. block, plot, address accept any chars, allowing potential downstream injection.", + "details": "Files: nadlan_mcp/web_app.py (DealsSearchRequest, AppraisalsSearchRequest). Add @field_validator: block ^\\d{1,8}$, plot ^\\d{1,6}$, decisive_appraiser Hebrew/space/dash, address Hebrew/Latin/digits/punctuation.", + "testStrategy": "POST with null bytes / SQL-like payloads → 422. Real Hebrew searches still pass.", + "priority": "medium", + "status": "pending", + "dependencies": [], + "subtasks": [] + }, + { + "id": "11", + "title": "M-7 [Medium]: Cap request payload size", + "description": "uvicorn default ~16MB but no explicit limit. Large POST bodies waste memory before validation.", + "details": "Add Starlette middleware to reject Content-Length > 1MB on /api/* endpoints (none of our endpoints need large bodies).", + "testStrategy": "POST 10MB body → 413 before parsing.", + "priority": "medium", + "status": "pending", + "dependencies": [], + "subtasks": [] + }, + { + "id": "12", + "title": "M-4 [Medium]: Move Coolify UUID to Gitea secret", + "description": "COOLIFY_UUID hardcoded in workflow file leaks infra mapping.", + "details": "Edit .gitea/workflows/deploy.yaml — replace literal with ${{ secrets.COOLIFY_UUID }}. Add secret in Gitea Actions settings.", + "testStrategy": "Trigger workflow, verify deploy still works.", + "priority": "medium", + "status": "pending", + "dependencies": [], + "subtasks": [] + }, + { + "id": "13", + "title": "M-5 [Medium]: Pin base images and apt packages", + "description": "node:22-alpine and python:3.13-slim use floating tags; apt packages unpinned. Supply-chain risk.", + "details": "Resolve current images to digests via docker manifest inspect. Pin gcc/curl in Dockerfile.", + "testStrategy": "Rebuild → identical digest. CVE scanner reports specific versions.", + "priority": "medium", + "status": "pending", + "dependencies": [], + "subtasks": [] + }, + { + "id": "14", + "title": "M-6 [Medium]: Generate requirements.lock for Python deps", + "description": "pyproject.toml uses >= constraints — non-reproducible builds, unexpected upgrades.", + "details": "pip-compile or pip freeze → requirements.lock committed. Dockerfile uses pip install -r requirements.lock instead of -e .", + "testStrategy": "Two consecutive builds → identical pip freeze.", + "priority": "medium", + "status": "pending", + "dependencies": [], + "subtasks": [] + }, + { + "id": "15", + "title": "L-1 [Low]: Self-host Heebo fonts (privacy)", + "description": "Google Fonts loads from googleapis.com → leaks Referer (with full URL incl. searched address) to Google.", + "details": "Download Heebo woff2 files into web/public/fonts/. Update web/index.html link tags + add @font-face in CSS.", + "testStrategy": "Network tab → no requests to fonts.googleapis.com.", + "priority": "low", + "status": "pending", + "dependencies": [], + "subtasks": [] + }, + { + "id": "16", + "title": "L-2 [Low]: Add Cache-Control headers on static assets", + "description": "Static assets served without explicit cache policy.", + "details": "Middleware: /assets/* → Cache-Control: public, max-age=31536000, immutable; index.html → no-cache, no-store.", + "testStrategy": "curl -I /assets/index-xxx.js → expect immutable.", + "priority": "low", + "status": "pending", + "dependencies": [], + "subtasks": [] + }, + { + "id": "17", + "title": "L-3 [Low]: Redact PII from logs", + "description": "Logger emits user-supplied addresses, request URLs, exception bodies. Coolify log access leaks PII.", + "details": "Files: nadlan_mcp/govmap/client.py, nadlan_mcp/govil/client.py. Replace f-strings that include user input with sanitized versions; only log lengths or hashes.", + "testStrategy": "Search logs for sample query → not present.", + "priority": "low", + "status": "pending", + "dependencies": [], + "subtasks": [] + }, + { + "id": "18", + "title": "L-4 [Low]: Hardcode port in Dockerfile HEALTHCHECK", + "description": "${PORT:-8000} interpolation can fail if PORT is set unusually.", + "details": "Replace with literal 8000 — that's our fixed contract.", + "testStrategy": "docker inspect → HEALTHCHECK uses literal 8000.", + "priority": "low", + "status": "pending", + "dependencies": [], + "subtasks": [] + }, + { + "id": "19", + "title": "L-5 [Low]: Add streaming-response timeout in PDF proxy", + "description": "Stream can hang indefinitely if upstream stalls mid-transfer.", + "details": "Wrap iter_content with asyncio.timeout(300) (5 min hard cap).", + "testStrategy": "Mock upstream that sends one chunk then sleeps → connection closed at 5 min.", + "priority": "low", + "status": "pending", + "dependencies": [ + "4" + ], + "subtasks": [] + } + ], + "metadata": { + "version": "1.0.0", + "lastModified": "2026-04-25T17:42:36.190Z", + "taskCount": 19, + "completedCount": 0, + "tags": [ + "master" + ] + } + } +} \ No newline at end of file diff --git a/.taskmaster/templates/example_prd.txt b/.taskmaster/templates/example_prd.txt new file mode 100644 index 0000000..194114d --- /dev/null +++ b/.taskmaster/templates/example_prd.txt @@ -0,0 +1,47 @@ + +# Overview +[Provide a high-level overview of your product here. Explain what problem it solves, who it's for, and why it's valuable.] + +# Core Features +[List and describe the main features of your product. For each feature, include: +- What it does +- Why it's important +- How it works at a high level] + +# User Experience +[Describe the user journey and experience. Include: +- User personas +- Key user flows +- UI/UX considerations] + + +# Technical Architecture +[Outline the technical implementation details: +- System components +- Data models +- APIs and integrations +- Infrastructure requirements] + +# Development Roadmap +[Break down the development process into phases: +- MVP requirements +- Future enhancements +- Do not think about timelines whatsoever -- all that matters is scope and detailing exactly what needs to be build in each phase so it can later be cut up into tasks] + +# Logical Dependency Chain +[Define the logical order of development: +- Which features need to be built first (foundation) +- Getting as quickly as possible to something usable/visible front end that works +- Properly pacing and scoping each feature so it is atomic but can also be built upon and improved as development approaches] + +# Risks and Mitigations +[Identify potential risks and how they'll be addressed: +- Technical challenges +- Figuring out the MVP that we can build upon +- Resource constraints] + +# Appendix +[Include any additional information: +- Research findings +- Technical specifications] + \ No newline at end of file diff --git a/.taskmaster/templates/example_prd_rpg.txt b/.taskmaster/templates/example_prd_rpg.txt new file mode 100644 index 0000000..5ad908f --- /dev/null +++ b/.taskmaster/templates/example_prd_rpg.txt @@ -0,0 +1,511 @@ + +# Repository Planning Graph (RPG) Method - PRD Template + +This template teaches you (AI or human) how to create structured, dependency-aware PRDs using the RPG methodology from Microsoft Research. The key insight: separate WHAT (functional) from HOW (structural), then connect them with explicit dependencies. + +## Core Principles + +1. **Dual-Semantics**: Think functional (capabilities) AND structural (code organization) separately, then map them +2. **Explicit Dependencies**: Never assume - always state what depends on what +3. **Topological Order**: Build foundation first, then layers on top +4. **Progressive Refinement**: Start broad, refine iteratively + +## How to Use This Template + +- Follow the instructions in each `` block +- Look at `` blocks to see good vs bad patterns +- Fill in the content sections with your project details +- The AI reading this will learn the RPG method by following along +- Task Master will parse the resulting PRD into dependency-aware tasks + +## Recommended Tools for Creating PRDs + +When using this template to **create** a PRD (not parse it), use **code-context-aware AI assistants** for best results: + +**Why?** The AI needs to understand your existing codebase to make good architectural decisions about modules, dependencies, and integration points. + +**Recommended tools:** +- **Claude Code** (claude-code CLI) - Best for structured reasoning and large contexts +- **Cursor/Windsurf** - IDE integration with full codebase context +- **Gemini CLI** (gemini-cli) - Massive context window for large codebases +- **Codex/Grok CLI** - Strong code generation with context awareness + +**Note:** Once your PRD is created, `task-master parse-prd` works with any configured AI model - it just needs to read the PRD text itself, not your codebase. + + +--- + + + +Start with the problem, not the solution. Be specific about: +- What pain point exists? +- Who experiences it? +- Why existing solutions don't work? +- What success looks like (measurable outcomes)? + +Keep this section focused - don't jump into implementation details yet. + + +## Problem Statement +[Describe the core problem. Be concrete about user pain points.] + +## Target Users +[Define personas, their workflows, and what they're trying to achieve.] + +## Success Metrics +[Quantifiable outcomes. Examples: "80% task completion via autopilot", "< 5% manual intervention rate"] + + + +--- + + + +Now think about CAPABILITIES (what the system DOES), not code structure yet. + +Step 1: Identify high-level capability domains +- Think: "What major things does this system do?" +- Examples: Data Management, Core Processing, Presentation Layer + +Step 2: For each capability, enumerate specific features +- Use explore-exploit strategy: + * Exploit: What features are REQUIRED for core value? + * Explore: What features make this domain COMPLETE? + +Step 3: For each feature, define: +- Description: What it does in one sentence +- Inputs: What data/context it needs +- Outputs: What it produces/returns +- Behavior: Key logic or transformations + + +Capability: Data Validation + Feature: Schema validation + - Description: Validate JSON payloads against defined schemas + - Inputs: JSON object, schema definition + - Outputs: Validation result (pass/fail) + error details + - Behavior: Iterate fields, check types, enforce constraints + + Feature: Business rule validation + - Description: Apply domain-specific validation rules + - Inputs: Validated data object, rule set + - Outputs: Boolean + list of violated rules + - Behavior: Execute rules sequentially, short-circuit on failure + + + +Capability: validation.js + (Problem: This is a FILE, not a CAPABILITY. Mixing structure into functional thinking.) + +Capability: Validation + Feature: Make sure data is good + (Problem: Too vague. No inputs/outputs. Not actionable.) + + + +## Capability Tree + +### Capability: [Name] +[Brief description of what this capability domain covers] + +#### Feature: [Name] +- **Description**: [One sentence] +- **Inputs**: [What it needs] +- **Outputs**: [What it produces] +- **Behavior**: [Key logic] + +#### Feature: [Name] +- **Description**: +- **Inputs**: +- **Outputs**: +- **Behavior**: + +### Capability: [Name] +... + + + +--- + + + +NOW think about code organization. Map capabilities to actual file/folder structure. + +Rules: +1. Each capability maps to a module (folder or file) +2. Features within a capability map to functions/classes +3. Use clear module boundaries - each module has ONE responsibility +4. Define what each module exports (public interface) + +The goal: Create a clear mapping between "what it does" (functional) and "where it lives" (structural). + + +Capability: Data Validation + → Maps to: src/validation/ + ├── schema-validator.js (Schema validation feature) + ├── rule-validator.js (Business rule validation feature) + └── index.js (Public exports) + +Exports: + - validateSchema(data, schema) + - validateRules(data, rules) + + + +Capability: Data Validation + → Maps to: src/utils.js + (Problem: "utils" is not a clear module boundary. Where do I find validation logic?) + +Capability: Data Validation + → Maps to: src/validation/everything.js + (Problem: One giant file. Features should map to separate files for maintainability.) + + + +## Repository Structure + +``` +project-root/ +├── src/ +│ ├── [module-name]/ # Maps to: [Capability Name] +│ │ ├── [file].js # Maps to: [Feature Name] +│ │ └── index.js # Public exports +│ └── [module-name]/ +├── tests/ +└── docs/ +``` + +## Module Definitions + +### Module: [Name] +- **Maps to capability**: [Capability from functional decomposition] +- **Responsibility**: [Single clear purpose] +- **File structure**: + ``` + module-name/ + ├── feature1.js + ├── feature2.js + └── index.js + ``` +- **Exports**: + - `functionName()` - [what it does] + - `ClassName` - [what it does] + + + +--- + + + +This is THE CRITICAL SECTION for Task Master parsing. + +Define explicit dependencies between modules. This creates the topological order for task execution. + +Rules: +1. List modules in dependency order (foundation first) +2. For each module, state what it depends on +3. Foundation modules should have NO dependencies +4. Every non-foundation module should depend on at least one other module +5. Think: "What must EXIST before I can build this module?" + + +Foundation Layer (no dependencies): + - error-handling: No dependencies + - config-manager: No dependencies + - base-types: No dependencies + +Data Layer: + - schema-validator: Depends on [base-types, error-handling] + - data-ingestion: Depends on [schema-validator, config-manager] + +Core Layer: + - algorithm-engine: Depends on [base-types, error-handling] + - pipeline-orchestrator: Depends on [algorithm-engine, data-ingestion] + + + +- validation: Depends on API +- API: Depends on validation +(Problem: Circular dependency. This will cause build/runtime issues.) + +- user-auth: Depends on everything +(Problem: Too many dependencies. Should be more focused.) + + + +## Dependency Chain + +### Foundation Layer (Phase 0) +No dependencies - these are built first. + +- **[Module Name]**: [What it provides] +- **[Module Name]**: [What it provides] + +### [Layer Name] (Phase 1) +- **[Module Name]**: Depends on [[module-from-phase-0], [module-from-phase-0]] +- **[Module Name]**: Depends on [[module-from-phase-0]] + +### [Layer Name] (Phase 2) +- **[Module Name]**: Depends on [[module-from-phase-1], [module-from-foundation]] + +[Continue building up layers...] + + + +--- + + + +Turn the dependency graph into concrete development phases. + +Each phase should: +1. Have clear entry criteria (what must exist before starting) +2. Contain tasks that can be parallelized (no inter-dependencies within phase) +3. Have clear exit criteria (how do we know phase is complete?) +4. Build toward something USABLE (not just infrastructure) + +Phase ordering follows topological sort of dependency graph. + + +Phase 0: Foundation + Entry: Clean repository + Tasks: + - Implement error handling utilities + - Create base type definitions + - Setup configuration system + Exit: Other modules can import foundation without errors + +Phase 1: Data Layer + Entry: Phase 0 complete + Tasks: + - Implement schema validator (uses: base types, error handling) + - Build data ingestion pipeline (uses: validator, config) + Exit: End-to-end data flow from input to validated output + + + +Phase 1: Build Everything + Tasks: + - API + - Database + - UI + - Tests + (Problem: No clear focus. Too broad. Dependencies not considered.) + + + +## Development Phases + +### Phase 0: [Foundation Name] +**Goal**: [What foundational capability this establishes] + +**Entry Criteria**: [What must be true before starting] + +**Tasks**: +- [ ] [Task name] (depends on: [none or list]) + - Acceptance criteria: [How we know it's done] + - Test strategy: [What tests prove it works] + +- [ ] [Task name] (depends on: [none or list]) + +**Exit Criteria**: [Observable outcome that proves phase complete] + +**Delivers**: [What can users/developers do after this phase?] + +--- + +### Phase 1: [Layer Name] +**Goal**: + +**Entry Criteria**: Phase 0 complete + +**Tasks**: +- [ ] [Task name] (depends on: [[tasks-from-phase-0]]) +- [ ] [Task name] (depends on: [[tasks-from-phase-0]]) + +**Exit Criteria**: + +**Delivers**: + +--- + +[Continue with more phases...] + + + +--- + + + +Define how testing will be integrated throughout development (TDD approach). + +Specify: +1. Test pyramid ratios (unit vs integration vs e2e) +2. Coverage requirements +3. Critical test scenarios +4. Test generation guidelines for Surgical Test Generator + +This section guides the AI when generating tests during the RED phase of TDD. + + +Critical Test Scenarios for Data Validation module: + - Happy path: Valid data passes all checks + - Edge cases: Empty strings, null values, boundary numbers + - Error cases: Invalid types, missing required fields + - Integration: Validator works with ingestion pipeline + + + +## Test Pyramid + +``` + /\ + /E2E\ ← [X]% (End-to-end, slow, comprehensive) + /------\ + /Integration\ ← [Y]% (Module interactions) + /------------\ + / Unit Tests \ ← [Z]% (Fast, isolated, deterministic) + /----------------\ +``` + +## Coverage Requirements +- Line coverage: [X]% minimum +- Branch coverage: [X]% minimum +- Function coverage: [X]% minimum +- Statement coverage: [X]% minimum + +## Critical Test Scenarios + +### [Module/Feature Name] +**Happy path**: +- [Scenario description] +- Expected: [What should happen] + +**Edge cases**: +- [Scenario description] +- Expected: [What should happen] + +**Error cases**: +- [Scenario description] +- Expected: [How system handles failure] + +**Integration points**: +- [What interactions to test] +- Expected: [End-to-end behavior] + +## Test Generation Guidelines +[Specific instructions for Surgical Test Generator about what to focus on, what patterns to follow, project-specific test conventions] + + + +--- + + + +Describe technical architecture, data models, and key design decisions. + +Keep this section AFTER functional/structural decomposition - implementation details come after understanding structure. + + +## System Components +[Major architectural pieces and their responsibilities] + +## Data Models +[Core data structures, schemas, database design] + +## Technology Stack +[Languages, frameworks, key libraries] + +**Decision: [Technology/Pattern]** +- **Rationale**: [Why chosen] +- **Trade-offs**: [What we're giving up] +- **Alternatives considered**: [What else we looked at] + + + +--- + + + +Identify risks that could derail development and how to mitigate them. + +Categories: +- Technical risks (complexity, unknowns) +- Dependency risks (blocking issues) +- Scope risks (creep, underestimation) + + +## Technical Risks +**Risk**: [Description] +- **Impact**: [High/Medium/Low - effect on project] +- **Likelihood**: [High/Medium/Low] +- **Mitigation**: [How to address] +- **Fallback**: [Plan B if mitigation fails] + +## Dependency Risks +[External dependencies, blocking issues] + +## Scope Risks +[Scope creep, underestimation, unclear requirements] + + + +--- + + +## References +[Papers, documentation, similar systems] + +## Glossary +[Domain-specific terms] + +## Open Questions +[Things to resolve during development] + + +--- + + +# How Task Master Uses This PRD + +When you run `task-master parse-prd .txt`, the parser: + +1. **Extracts capabilities** → Main tasks + - Each `### Capability:` becomes a top-level task + +2. **Extracts features** → Subtasks + - Each `#### Feature:` becomes a subtask under its capability + +3. **Parses dependencies** → Task dependencies + - `Depends on: [X, Y]` sets task.dependencies = ["X", "Y"] + +4. **Orders by phases** → Task priorities + - Phase 0 tasks = highest priority + - Phase N tasks = lower priority, properly sequenced + +5. **Uses test strategy** → Test generation context + - Feeds test scenarios to Surgical Test Generator during implementation + +**Result**: A dependency-aware task graph that can be executed in topological order. + +## Why RPG Structure Matters + +Traditional flat PRDs lead to: +- ❌ Unclear task dependencies +- ❌ Arbitrary task ordering +- ❌ Circular dependencies discovered late +- ❌ Poorly scoped tasks + +RPG-structured PRDs provide: +- ✅ Explicit dependency chains +- ✅ Topological execution order +- ✅ Clear module boundaries +- ✅ Validated task graph before implementation + +## Tips for Best Results + +1. **Spend time on dependency graph** - This is the most valuable section for Task Master +2. **Keep features atomic** - Each feature should be independently testable +3. **Progressive refinement** - Start broad, use `task-master expand` to break down complex tasks +4. **Use research mode** - `task-master parse-prd --research` leverages AI for better task generation + diff --git a/Dockerfile b/Dockerfile index 8b4281a..2767aec 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,5 +1,7 @@ # ─── Stage 1: build the React UI ──────────────────────────────────────── -FROM node:22-alpine AS web-build +# Pinned to digest to neutralize tag-hijack and supply-chain replay. +# Renovate/dependabot can update this digest periodically. +FROM node:22-alpine@sha256:8ea2348b068a9544dae7317b4f3aafcdc032df1647bb7d768a05a5cad1a7683f AS web-build WORKDIR /web # Cache deps separately from sources. @@ -10,7 +12,7 @@ COPY web/ . RUN npm run build # ─── Stage 2: Python runtime ──────────────────────────────────────────── -FROM python:3.13-slim +FROM python:3.13-slim@sha256:a0779d7c12fc20be6ec6b4ddc901a4fd7657b8a6bc9def9d3fde89ed5efe0a3d ENV PYTHONUNBUFFERED=1 \ PYTHONDONTWRITEBYTECODE=1 \ @@ -43,6 +45,6 @@ USER appuser EXPOSE 8000 HEALTHCHECK --interval=30s --timeout=10s --start-period=15s --retries=3 \ - CMD python -c "import urllib.request; urllib.request.urlopen('http://localhost:${PORT:-8000}/health')" || exit 1 + CMD python -c "import urllib.request; urllib.request.urlopen('http://localhost:8000/health')" || exit 1 CMD ["python", "run_http_server.py"] diff --git a/nadlan_mcp/govmap/client.py b/nadlan_mcp/govmap/client.py index 225cac7..d8696fe 100644 --- a/nadlan_mcp/govmap/client.py +++ b/nadlan_mcp/govmap/client.py @@ -554,7 +554,12 @@ class GovmapClient: try: # Step 1: Get coordinates for the address - logger.info(f"Starting search for address: {address}, dealType: {deal_type}") + # Don't log the address itself — it's PII. Length is a safe proxy. + logger.info( + "Starting search for address (length=%d, dealType=%d)", + len(address), + deal_type, + ) autocomplete_result = self.autocomplete_address(address) if not autocomplete_result.results: diff --git a/nadlan_mcp/govmap/models.py b/nadlan_mcp/govmap/models.py index 688ade1..9925e2a 100644 --- a/nadlan_mcp/govmap/models.py +++ b/nadlan_mcp/govmap/models.py @@ -138,6 +138,25 @@ class Deal(BaseModel): priority: Optional[int] = Field( None, description="Priority for sorting (0=same building, 1=street, 2=neighborhood)" ) + # Set after parsing by find_recent_deals_for_address based on which + # polygon the deal was sourced from. Not from upstream — explicit so + # extra='ignore' (M-2) doesn't strip it on serialization. + deal_source: Optional[str] = Field( + None, + description="Provenance bucket: same_building / street / neighborhood", + ) + deal_type: Optional[int] = Field( + None, + description="Deal type filter applied (1=first hand, 2=second hand)", + ) + deal_type_description: Optional[str] = Field( + None, + description="Human-readable deal-type label", + ) + distance_meters: Optional[float] = Field( + None, + description="Distance from search-center coordinates (meters)", + ) # Geometry and internal fields (often not useful for analysis) shape: Optional[str] = Field(None, description="WKT geometry") @@ -146,9 +165,36 @@ class Deal(BaseModel): ) sourceorder: Optional[int] = Field(None, description="Source ordering") + # Cadastral identifiers (used by the UI columns גוש/חלקה/תת-חלקה). + # Modeled explicitly so we no longer rely on `extra="allow"` to leak + # arbitrary upstream keys into the response — see security audit M-2. + gush_num: Optional[int] = Field(None, alias="gushNum", description="Block (גוש) number") + parcel_num: Optional[int] = Field(None, alias="parcelNum", description="Parcel (חלקה) number") + sub_parcel_num: Optional[int] = Field( + None, alias="subParcelNum", description="Sub-parcel (תת-חלקה) number" + ) + + # Govmap uses two different street/house keys depending on the source + # endpoint. `street_name` (alias `streetName`) is set by some APIs; + # `street_name_heb` (alias `streetNameHeb`) is what the UI table reads. + # `settlement_name_heb` is already declared above. + street_name_heb: Optional[str] = Field( + None, alias="streetNameHeb", description="Street name in Hebrew" + ) + house_num: Optional[Any] = Field( + None, + alias="houseNum", + description="House number (Govmap-side; may differ from house_number)", + ) + model_config = ConfigDict( populate_by_name=True, # Allow both alias and field name - extra="allow", # Allow extra fields from API that we don't model + # `ignore` (not `forbid`): Govmap occasionally introduces new + # fields and we don't want validation to break. But we DO want + # to stop silently propagating unknown content into our response + # (which `allow` did). All upstream fields we actually use are + # now modeled explicitly above. + extra="ignore", ) @computed_field diff --git a/nadlan_mcp/web_app.py b/nadlan_mcp/web_app.py index 52d7172..382c939 100644 --- a/nadlan_mcp/web_app.py +++ b/nadlan_mcp/web_app.py @@ -11,15 +11,18 @@ Layout: from __future__ import annotations +import contextlib import logging +import os from pathlib import Path +import re from typing import Any from fastapi import FastAPI, HTTPException, Query from fastapi.middleware.cors import CORSMiddleware from fastapi.responses import StreamingResponse from fastapi.staticfiles import StaticFiles -from pydantic import BaseModel, Field +from pydantic import BaseModel, Field, field_validator from starlette.responses import FileResponse, JSONResponse from nadlan_mcp.fastmcp_server import ( @@ -33,12 +36,48 @@ from nadlan_mcp.govil.client import ALLOWED_PDF_HOSTS logger = logging.getLogger(__name__) +# Environment switch — controls visibility of internal-leak surfaces like +# Swagger and openapi.json. Defaults to "production" so an unset env var +# is the safe choice. +ENVIRONMENT = os.getenv("ENVIRONMENT", "production").lower() +IS_PROD = ENVIRONMENT in {"production", "prod"} + +# Hard cap for proxied PDF downloads (bytes). Above this we refuse rather +# than buffer/stream. Justice-ministry decisions are rarely >5MB; 50MB is +# a generous ceiling that still bounds memory use. +MAX_PDF_SIZE = 50 * 1024 * 1024 + +# Generic error string returned to clients in lieu of `str(e)` — see H-1. +GENERIC_UPSTREAM_ERROR = "Service temporarily unavailable, please retry" + # ────────────────────────────────────────────────────────────────────── # Request / response schemas # ────────────────────────────────────────────────────────────────────── +_RE_BLOCK = re.compile(r"^\d{1,8}$") +_RE_PLOT = re.compile(r"^\d{1,6}$") +_RE_DATE = re.compile(r"^\d{4}-\d{2}-\d{2}$") +# Address: Hebrew letters, Latin letters, digits, spaces, common punctuation +# (comma, dash, dot, slash, quote, apostrophe). Excludes control chars and +# null bytes that could trip downstream APIs. +_RE_ADDRESS = re.compile(r"^[֐-׿A-Za-z0-9\s,\-./'\"\(\)״׳]+$") +# Person/committee names (Hebrew + Latin + spaces + dashes + dots) +_RE_NAME = re.compile(r"^[֐-׿A-Za-z\s\-.׳״']+$") + + +def _validate_optional_pattern(value: str | None, pattern: re.Pattern, field: str) -> str | None: + if value is None: + return None + value = value.strip() + if not value: + return None + if not pattern.match(value): + raise ValueError(f"{field}: invalid format") + return value + + class DealsSearchRequest(BaseModel): address: str = Field(..., min_length=2, max_length=200) years_back: int = Field(2, ge=1, le=10) @@ -46,6 +85,14 @@ class DealsSearchRequest(BaseModel): max_deals: int = Field(50, ge=1, le=200) deal_type: int = Field(2, ge=1, le=2) + @field_validator("address") + @classmethod + def _check_address(cls, v: str) -> str: + v = v.strip() + if not _RE_ADDRESS.match(v): + raise ValueError("address contains disallowed characters") + return v + class AppraisalsSearchRequest(BaseModel): block: str | None = Field(None, max_length=20) @@ -65,6 +112,43 @@ class AppraisalsSearchRequest(BaseModel): include_nearby: bool = Field(False) nearby_block_radius: int = Field(2, ge=1, le=10) + @field_validator("block") + @classmethod + def _check_block(cls, v): + return _validate_optional_pattern(v, _RE_BLOCK, "block") + + @field_validator("plot") + @classmethod + def _check_plot(cls, v): + return _validate_optional_pattern(v, _RE_PLOT, "plot") + + @field_validator("decisive_appraiser", "committee") + @classmethod + def _check_name(cls, v): + return _validate_optional_pattern(v, _RE_NAME, "name") + + @field_validator( + "decision_date_from", + "decision_date_to", + "publicity_date_from", + "publicity_date_to", + ) + @classmethod + def _check_date(cls, v): + return _validate_optional_pattern(v, _RE_DATE, "date") + + @field_validator("search_text", "appraisal_header") + @classmethod + def _check_freetext(cls, v): + if v is None: + return None + v = v.strip() + if not v: + return None + if not _RE_ADDRESS.match(v): + raise ValueError("text contains disallowed characters") + return v + # ────────────────────────────────────────────────────────────────────── # App factory @@ -94,20 +178,178 @@ def create_app() -> FastAPI: "raw FastMCP endpoint at /mcp for LLM clients." ), version="2.1.0", - docs_url="/api/docs", + # Hide API schema in production to avoid handing a roadmap to attackers. + docs_url=None if IS_PROD else "/api/docs", redoc_url=None, - openapi_url="/api/openapi.json", + openapi_url=None if IS_PROD else "/api/openapi.json", lifespan=mcp_app.router.lifespan_context, ) - # CORS — only relevant during local dev when Vite serves on :5173. + # CORS — origins overridable via ALLOWED_ORIGINS env (comma-separated). + # Default keeps the local-dev Vite origins so contributors aren't broken; + # production sets ALLOWED_ORIGINS explicitly (or empty to disable cross-origin). + cors_default = "http://localhost:5173,http://127.0.0.1:5173" + cors_origins = [ + o.strip() for o in os.getenv("ALLOWED_ORIGINS", cors_default).split(",") if o.strip() + ] app.add_middleware( CORSMiddleware, - allow_origins=["http://localhost:5173", "http://127.0.0.1:5173"], + allow_origins=cors_origins, allow_methods=["GET", "POST", "OPTIONS"], - allow_headers=["*"], + allow_headers=["Content-Type"], + allow_credentials=False, + max_age=600, ) + # ── Optional API-key gate (C-2) ───────────────────────────────── + # Defense-in-depth complement to Traefik mTLS (which is the canonical + # solution but not yet enabled). When NADLAN_API_KEY is set, requests + # to non-public paths must present a matching X-API-Key header. + # When unset, the gate is bypassed — preserving the current public + # behavior so this is safe to ship before mTLS lands. + API_KEY = os.getenv("NADLAN_API_KEY", "").strip() or None + AUTH_REQUIRED_PATHS = ("/api/search/", "/api/appraisals/pdf", "/mcp") + PUBLIC_PATHS = ("/api/health", "/health") + + @app.middleware("http") + async def api_key_gate(request, call_next): + if API_KEY is None: + return await call_next(request) + path = request.url.path + if path in PUBLIC_PATHS: + return await call_next(request) + if not any(path.startswith(p) for p in AUTH_REQUIRED_PATHS): + return await call_next(request) + provided = request.headers.get("x-api-key", "").strip() + # Constant-time compare to defeat timing oracles. + import hmac + + if not provided or not hmac.compare_digest(provided, API_KEY): + return JSONResponse({"detail": "Unauthorized"}, status_code=401) + return await call_next(request) + + # ── Per-IP rate limiter (H-5) ─────────────────────────────────── + # Token bucket per client IP — protects upstream Govmap/gov.il from + # bulk scraping via /api/* and /mcp, and prevents one attacker from + # exhausting the shared rate-limit budget. Buckets are kept in + # process-local memory; a single uvicorn worker is enough at our + # scale, and the limit is a defense-in-depth complement to Traefik's + # own RateLimit middleware (when added). + import asyncio + from collections import defaultdict + + RATE_LIMIT_PATHS = ( + ("/mcp", 30, 60.0), # 30 req / 60s on /mcp/* + ("/api/search/", 60, 60.0), # 60 req / 60s on /api/search/* + ("/api/appraisals/pdf", 30, 60.0), # 30 PDF fetches / 60s + ) + _rl_buckets: dict[tuple[str, str], list[float]] = defaultdict(list) + _rl_lock = asyncio.Lock() + + def _client_ip(request) -> str: + # Coolify/Traefik forward the real client IP via X-Forwarded-For. + # Take the first entry (left-most), which by convention is the + # actual remote client. + xff = request.headers.get("x-forwarded-for", "").split(",")[0].strip() + if xff: + return xff + return request.client.host if request.client else "unknown" + + @app.middleware("http") + async def rate_limit(request, call_next): + path = request.url.path + rule = next( + (r for r in RATE_LIMIT_PATHS if path.startswith(r[0])), + None, + ) + if rule is None: + return await call_next(request) + prefix, limit, window = rule + ip = _client_ip(request) + key = (prefix, ip) + import time + + now = time.monotonic() + async with _rl_lock: + bucket = _rl_buckets[key] + cutoff = now - window + # Drop timestamps that fell out of the window. + bucket[:] = [t for t in bucket if t > cutoff] + if len(bucket) >= limit: + retry_after = max(1, int(window - (now - bucket[0]))) + resp = JSONResponse({"detail": "Rate limit exceeded"}, status_code=429) + resp.headers["Retry-After"] = str(retry_after) + return resp + bucket.append(now) + return await call_next(request) + + # Reject oversized request bodies before parsing — this app's largest + # legitimate POST is a few hundred bytes. 1MB is generous. + MAX_REQUEST_BODY = 1 * 1024 * 1024 + + @app.middleware("http") + async def limit_body_size(request, call_next): + cl = request.headers.get("content-length") + if cl is not None: + try: + if int(cl) > MAX_REQUEST_BODY: + return JSONResponse({"detail": "Request body too large"}, status_code=413) + except ValueError: + return JSONResponse({"detail": "Invalid Content-Length"}, status_code=400) + return await call_next(request) + + # Set HTTP security headers on every response. Done as a single + # middleware because we want one place to manage the CSP allowlist + # as the UI evolves (e.g. adding Mapbox would require connect-src). + @app.middleware("http") + async def security_headers(request, call_next): + response = await call_next(request) + # MIME-sniffing protection — important for the PDF proxy. + response.headers.setdefault("X-Content-Type-Options", "nosniff") + # Clickjacking — also enforced by CSP frame-ancestors below. + response.headers.setdefault("X-Frame-Options", "DENY") + # Don't leak query-string PII (searched addresses, gush/חלקה) to + # third parties via the Referer header. + response.headers.setdefault("Referrer-Policy", "strict-origin-when-cross-origin") + # No need for the camera/microphone/geo APIs in this product. + response.headers.setdefault( + "Permissions-Policy", + "geolocation=(), microphone=(), camera=(), payment=(), usb=()", + ) + # CSP — UI is React+Vite (no eval, no inline scripts at build time). + # Tailwind injects style attributes at runtime, hence 'unsafe-inline' + # for style-src. Heebo is now self-hosted via @fontsource so we no + # longer need to allow Google Fonts. + response.headers.setdefault( + "Content-Security-Policy", + "default-src 'self'; " + "script-src 'self'; " + "style-src 'self' 'unsafe-inline'; " + "font-src 'self' data:; " + "img-src 'self' data: blob:; " + "connect-src 'self'; " + "frame-ancestors 'none'; " + "form-action 'self'; " + "base-uri 'self'; " + "object-src 'none'", + ) + # HSTS — only meaningful behind HTTPS, but our Traefik enforces it + # in both dev and prod, so always set. + response.headers.setdefault( + "Strict-Transport-Security", "max-age=31536000; includeSubDomains" + ) + # Drop the server-version banner. + response.headers.pop("server", None) + # Cache hashed asset bundles aggressively (Vite already adds a hash + # to filenames so cache invalidation is a build problem, not a + # runtime one). Everything else: don't cache. + path = request.url.path + if path.startswith("/assets/"): + response.headers.setdefault("Cache-Control", "public, max-age=31536000, immutable") + elif path == "/" or path.endswith(".html"): + response.headers.setdefault("Cache-Control", "no-cache, no-store, must-revalidate") + return response + # ── Health ──────────────────────────────────────────────────────── @app.get("/api/health") @app.get("/health") @@ -121,9 +363,9 @@ def create_app() -> FastAPI: available) the gush/חלקה for the top match.""" try: response = govmap_client.autocomplete_address(q) - except Exception as e: + except Exception: logger.exception("autocomplete_address failed") - raise HTTPException(status_code=502, detail=str(e)) + raise HTTPException(status_code=502, detail=GENERIC_UPSTREAM_ERROR) results = [] for r in response.results: @@ -149,8 +391,10 @@ def create_app() -> FastAPI: coords = response.results[0].coordinates gh = _query_parcel_layer(govmap_client, coords.longitude, coords.latitude) gush_helka = _extract_gush_helka(gh) - except Exception as e: - logger.warning(f"parcel lookup failed for '{q}': {e}") + except Exception: + # Don't log the user-supplied query — it's PII (address) + # and goes to whoever has access to the container logs. + logger.warning("parcel lookup failed (query length=%d)", len(q)) return {"query": q, "results": results, "gush_helka": gush_helka} @@ -166,9 +410,9 @@ def create_app() -> FastAPI: req.max_deals, req.deal_type, ) - except Exception as e: + except Exception: logger.exception("find_recent_deals_for_address failed") - raise HTTPException(status_code=502, detail=str(e)) + raise HTTPException(status_code=502, detail=GENERIC_UPSTREAM_ERROR) # Search-center coords + resolved address text for the UI (best-effort). search_coords = None @@ -243,9 +487,9 @@ def create_app() -> FastAPI: try: exact_response = _run(req.block, req.plot) - except Exception as e: + except Exception: logger.exception("search_decisions_paged (exact) failed") - raise HTTPException(status_code=502, detail=str(e)) + raise HTTPException(status_code=502, detail=GENERIC_UPSTREAM_ERROR) # Dedupe across buckets by a composite key (gov.il doesn't expose a # single stable id we can rely on). @@ -333,7 +577,7 @@ def create_app() -> FastAPI: # ── /api/appraisals/pdf ─────────────────────────────────────────── @app.get("/api/appraisals/pdf") - async def proxy_appraisal_pdf(url: str = Query(..., min_length=20)): + async def proxy_appraisal_pdf(url: str = Query(..., min_length=20, max_length=2048)): """ Stream a decisive-appraiser PDF through this server. @@ -342,12 +586,36 @@ def create_app() -> FastAPI: """ from urllib.parse import urlparse - parsed = urlparse(url) - if parsed.scheme != "https" or parsed.hostname not in ALLOWED_PDF_HOSTS: + # ── URL validation (C-1: defeat userinfo-bypass SSRF) ─────────── + # urlparse(...).hostname returns the value after the optional + # `user:pass@` userinfo, so a URL like + # https://attacker.com@pub-justice.openapi.gov.il/x.pdf + # has hostname == pub-justice.openapi.gov.il *and* would be fetched + # from attacker.com by curl. Reject any URL with userinfo, and any + # URL whose port differs from the default for its scheme. + try: + parsed = urlparse(url) + except ValueError: + raise HTTPException(status_code=400, detail="Malformed URL") + + if parsed.scheme != "https": + raise HTTPException(status_code=400, detail="Only https URLs are allowed") + if parsed.hostname not in ALLOWED_PDF_HOSTS: raise HTTPException(status_code=400, detail="URL host not allowed") + if parsed.username is not None or parsed.password is not None: + raise HTTPException(status_code=400, detail="URL must not contain userinfo") + if parsed.port not in (None, 443): + raise HTTPException(status_code=400, detail="URL port not allowed") + # Final sanity: the netloc the URL library will dial must be exactly + # the hostname (no leftover @ or port). This catches edge cases the + # individual checks above miss. + if parsed.netloc.lower() != parsed.hostname.lower(): + raise HTTPException(status_code=400, detail="URL netloc mismatch") try: # Reuse the curl_cffi session that already carries x-client-id. + # stream=True so we read in chunks rather than buffering the + # whole body in memory (H-4: defeat OOM via large upstream). response = decisive_appraiser_client._session.get( url, timeout=( @@ -355,26 +623,74 @@ def create_app() -> FastAPI: decisive_appraiser_client.config.read_timeout * 4, ), headers={"Accept": "application/pdf,*/*"}, + stream=True, + allow_redirects=False, ) - except Exception as e: + except Exception: logger.exception("PDF proxy upstream call failed") - raise HTTPException(status_code=502, detail=str(e)) + raise HTTPException(status_code=502, detail=GENERIC_UPSTREAM_ERROR) if response.status_code != 200: + with contextlib.suppress(Exception): + response.close() raise HTTPException( - status_code=response.status_code, + status_code=502, detail=f"Upstream returned {response.status_code}", ) - if not response.content.startswith(b"%PDF"): - raise HTTPException(status_code=502, detail="Upstream did not return a PDF") + + # Refuse before reading anything if upstream advertises an oversized body. + try: + advertised = int(response.headers.get("Content-Length", "0") or 0) + except ValueError: + advertised = 0 + if advertised > MAX_PDF_SIZE: + with contextlib.suppress(Exception): + response.close() + raise HTTPException(status_code=413, detail="Upstream PDF too large") # Suggest a reasonable filename for the browser. filename = parsed.path.rstrip("/").split("/")[-1] or "appraisal" if not filename.lower().endswith(".pdf"): filename = f"{filename}.pdf" + # Peek the first chunk synchronously so we can verify the PDF + # magic header before the StreamingResponse takes over (we can't + # raise HTTPException from inside the generator). + chunks = response.iter_content(chunk_size=8192) + try: + first_chunk = next(chunks) + except StopIteration: + with contextlib.suppress(Exception): + response.close() + raise HTTPException(status_code=502, detail="Empty upstream response") + except Exception: + logger.exception("PDF proxy: upstream stream failed") + with contextlib.suppress(Exception): + response.close() + raise HTTPException(status_code=502, detail=GENERIC_UPSTREAM_ERROR) + + if not first_chunk.startswith(b"%PDF"): + with contextlib.suppress(Exception): + response.close() + raise HTTPException(status_code=502, detail="Upstream did not return a PDF") + def _iter() -> Any: - yield response.content + seen = len(first_chunk) + yield first_chunk + try: + for chunk in chunks: + if not chunk: + continue + seen += len(chunk) + if seen > MAX_PDF_SIZE: + logger.warning( + f"PDF proxy: upstream exceeded {MAX_PDF_SIZE} bytes; truncating" + ) + return + yield chunk + finally: + with contextlib.suppress(Exception): + response.close() return StreamingResponse( _iter(), diff --git a/pyproject.toml b/pyproject.toml index 6f5d273..188cd9d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -22,14 +22,18 @@ classifiers = [ "Programming Language :: Python :: 3.13", ] +# Runtime deps are pinned to a known-good upper bound (~=) so a Docker +# rebuild reproduces the same versions, but minor/patch upgrades come +# in voluntarily via PR. ~= is "compatible-release" in PEP 440 — locks +# to the same major+minor (e.g. ~=2.13 means >=2.13,<3.0). dependencies = [ - "requests>=2.31.0", - "python-dotenv>=1.0.0", - "pydantic>=2.0.0", + "requests~=2.33", + "python-dotenv~=1.2", + "pydantic~=2.13", "fastmcp>=2.13.0,<3.0.0", - "curl_cffi>=0.5.0", - "fastapi>=0.115.0", - "uvicorn[standard]>=0.30.0", + "curl_cffi~=0.15", + "fastapi~=0.136", + "uvicorn[standard]~=0.46", ] [project.optional-dependencies] diff --git a/web/index.html b/web/index.html index 37d56ac..9af6153 100644 --- a/web/index.html +++ b/web/index.html @@ -3,14 +3,10 @@ - נדל"ן ושמאי מכריע — Marcus-Law + נדל"ן ושמאי מכריע - - - + +
diff --git a/web/package-lock.json b/web/package-lock.json index 4315729..39ea928 100644 --- a/web/package-lock.json +++ b/web/package-lock.json @@ -8,6 +8,7 @@ "name": "nadlan-mcp-web", "version": "0.1.0", "dependencies": { + "@fontsource/heebo": "^5.2.8", "@radix-ui/react-direction": "^1.1.1", "@radix-ui/react-icons": "^1.3.2", "@radix-ui/react-label": "^2.1.0", @@ -771,6 +772,15 @@ "node": ">=18" } }, + "node_modules/@fontsource/heebo": { + "version": "5.2.8", + "resolved": "https://registry.npmjs.org/@fontsource/heebo/-/heebo-5.2.8.tgz", + "integrity": "sha512-K5aMLH2Yy/CmmjuM9lpToyIEwSdPKwW6KzZOGCFnj7XFrgatIpnesu6FDgmQl3bkyP/YCT62rzLRqRM9s/OzLw==", + "license": "OFL-1.1", + "funding": { + "url": "https://github.com/sponsors/ayuhito" + } + }, "node_modules/@jridgewell/gen-mapping": { "version": "0.3.13", "resolved": "https://registry.npmjs.org/@jridgewell/gen-mapping/-/gen-mapping-0.3.13.tgz", diff --git a/web/package.json b/web/package.json index 0a14481..a8da3ca 100644 --- a/web/package.json +++ b/web/package.json @@ -11,6 +11,7 @@ "lint": "eslint . --ext ts,tsx --max-warnings=0" }, "dependencies": { + "@fontsource/heebo": "^5.2.8", "@radix-ui/react-direction": "^1.1.1", "@radix-ui/react-icons": "^1.3.2", "@radix-ui/react-label": "^2.1.0", diff --git a/web/src/api/types.ts b/web/src/api/types.ts index 88ff207..fa2fa9a 100644 --- a/web/src/api/types.ts +++ b/web/src/api/types.ts @@ -36,12 +36,12 @@ export interface Deal { floor?: number; deal_date?: string; property_type_description?: string; - settlementNameHeb?: string; - streetNameHeb?: string; - houseNum?: string | number; - gushNum?: number | null; - parcelNum?: number | null; - subParcelNum?: number | null; + settlement_name_heb?: string; + street_name_heb?: string; + house_num?: string | number; + gush_num?: number | null; + parcel_num?: number | null; + sub_parcel_num?: number | null; price_per_sqm?: number; deal_source?: string | null; [key: string]: unknown; diff --git a/web/src/components/DealsTable.tsx b/web/src/components/DealsTable.tsx index 96ff704..52fb307 100644 --- a/web/src/components/DealsTable.tsx +++ b/web/src/components/DealsTable.tsx @@ -37,11 +37,11 @@ function streetByClosestParcel( const blockNum = block ? parseInt(block, 10) : null; let best: { dist: number; street: string } | null = null; for (const d of deals) { - if (!d.streetNameHeb || d.parcelNum == null) continue; - if (blockNum !== null && d.gushNum !== blockNum) continue; - const dist = Math.abs((d.parcelNum as number) - target); + if (!d.street_name_heb || d.parcel_num == null) continue; + if (blockNum !== null && d.gush_num !== blockNum) continue; + const dist = Math.abs((d.parcel_num as number) - target); if (!best || dist < best.dist) { - best = { dist, street: d.streetNameHeb }; + best = { dist, street: d.street_name_heb }; } } return best?.street ?? null; @@ -52,13 +52,13 @@ function proximityLabel(deal: Deal, inferredStreet: string | null): string { if (deal.deal_source === "same_building") return "אותו בניין"; if (deal.deal_source === "neighborhood") return "שכונה"; // "street" source — check if actually same street - if (inferredStreet && deal.streetNameHeb === inferredStreet) return "אותו רחוב"; + if (inferredStreet && deal.street_name_heb === inferredStreet) return "אותו רחוב"; return "סביבה קרובה"; } function proximityClass(deal: Deal, inferredStreet: string | null): string { if (deal.deal_source === "same_building") return "bg-emerald-100 text-emerald-800"; - if (deal.deal_source === "street" && inferredStreet && deal.streetNameHeb === inferredStreet) + if (deal.deal_source === "street" && inferredStreet && deal.street_name_heb === inferredStreet) return "bg-blue-100 text-blue-800"; if (deal.deal_source === "street") return "bg-sky-50 text-sky-700"; return "bg-slate-100 text-slate-700"; @@ -100,9 +100,9 @@ export function DealsTable({ deals, resolvedAddress, searchedBlock, searchedPlot {deals.map((d) => { - const street = d.streetNameHeb || ""; - const houseNum = d.houseNum ?? ""; - const settlement = d.settlementNameHeb || ""; + const street = d.street_name_heb || ""; + const houseNum = d.house_num ?? ""; + const settlement = d.settlement_name_heb || ""; const address = [street, houseNum, settlement].filter(Boolean).join(" "); return ( @@ -118,9 +118,9 @@ export function DealsTable({ deals, resolvedAddress, searchedBlock, searchedPlot {formatCurrencyILS(d.price_per_sqm)} - {d.gushNum ?? "—"} - {d.parcelNum ?? "—"} - {d.subParcelNum ?? "—"} + {d.gush_num ?? "—"} + {d.parcel_num ?? "—"} + {d.sub_parcel_num ?? "—"} {d.deal_source ? (