13 Commits

Author SHA1 Message Date
chaim 0340a670ed security: address all findings from vulnerability assessment
Implements the fixes catalogued under Task Master tasks 1-19, derived
from the security audit at .claude/plans/adaptive-sleeping-diffie.md.

Critical
- C-1 SSRF userinfo bypass: PDF proxy now rejects URLs containing
  userinfo or non-default ports, and asserts netloc == hostname.
- C-2 No authentication: optional NADLAN_API_KEY gate guards
  /api/search/*, /api/appraisals/pdf and /mcp until Traefik mTLS
  is wired up. Default behavior unchanged when env unset.

High
- H-1 Error info leak: HTTPException details replaced with generic
  string; full exceptions still logged via logger.exception.
- H-2 Security headers middleware: HSTS, X-Frame-Options, nosniff,
  Referrer-Policy, Permissions-Policy, CSP, server-banner stripped.
- H-3 Swagger gated: /api/docs and /api/openapi.json hidden when
  ENVIRONMENT=production.
- H-4 PDF proxy DoS: streams in 8KB chunks, rejects upstream above
  50MB Content-Length, also rejects mid-stream overrun. Magic-byte
  check moved before StreamingResponse so we can 502 cleanly.
- H-5 Per-IP rate limit: token-bucket per (path, IP) on /mcp,
  /api/search/*, /api/appraisals/pdf. Reads X-Forwarded-For for
  client IP behind Traefik.

Medium
- M-1 CORS: ALLOWED_ORIGINS env, allow_headers narrowed, no creds.
- M-2 Deal model: extra="ignore" + explicit fields for gushNum,
  parcelNum, subParcelNum, streetNameHeb, houseNum, deal_source,
  deal_type, deal_type_description, distance_meters. Frontend now
  reads snake_case.
- M-3 Input validators: regex on block, plot, appraiser names,
  dates, address, free-text — Hebrew + Latin + safe punctuation.
- M-4 COOLIFY_UUID moved out of deploy.yaml into Gitea Actions
  secret.
- M-5 Base images pinned to sha256 digests.
- M-6 Runtime deps tightened to ~= compatible-release.
- M-7 1MB request-body size limit middleware.

Low / privacy
- L-1 Heebo self-hosted via @fontsource. Removed Google Fonts links
  and CSP entries. Added meta referrer + meta robots noindex.
- L-2 Cache-Control: immutable for /assets, no-cache for HTML.
- L-3 Logger no longer prints user-supplied addresses; emits length
  only.
- L-4 Dockerfile healthcheck uses literal port 8000.
- L-5 PDF proxy stream cleanup uses contextlib.suppress.

Tests: 331 passed, 17 skipped. Frontend typecheck passes.
Findings file: .claude/plans/adaptive-sleeping-diffie.md
Tasks: .taskmaster/tasks/tasks.json

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-25 17:59:10 +00:00
Nitzan Pomerantz b87cb268d5 Improve: MCP efficiency & fix radius filtering
- Remove bloat fields (shape, objectid, etc), add sequential IDs
- Add lang param (he/en) for Hebrew/English text values
- Reduce JSON whitespace (indent=None)
- Fix distance_meters: extract centroid from WKT shape geometry
- Add search_coordinates to all address-based tool responses
- Fix radius filtering: properly filter deals beyond radius_meters
- Change default radius from 30m to 50m
- Fix get_deal_statistics: return NO deals (stats only)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-12-08 09:34:42 +02:00
Nitzan Pomerantz 4de1a4822b Add: Return outlier deals in analysis responses
Feature:
- New parameter `include_outlier_deals` (default=True) in:  - filter_deals_for_analysis()
  - calculate_deal_statistics()
  - get_valuation_comparables() MCP tool
  - get_deal_statistics() MCP tool

Behavior:
- When ON: response includes `outlier_deals` field with removed deals
- LLM can see what was filtered out, answer questions about it
- Maintains full transparency on outlier removal

Terminology fixed:
- "outlier_deals" = deals removed as outliers (clearer than "filtered_deals")
- "filtered deals" = deals that PASSED filtering
- Added to OutlierReport model

All 311 tests pass

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-12-06 23:26:21 +02:00
Nitzan Pomerantz 8e591423da Change default IQR multiplier to 1.0 for more aggressive outlier filtering
- Change ANALYSIS_IQR_MULTIPLIER default from 1.5 to 1.0 in config.py
- Add iqr_multiplier parameter to all filtering & statistics functions
- Allow runtime override via MCP tools (get_valuation_comparables, get_deal_statistics)
- Update CLAUDE.md docs with new default & override examples

Rationale: k=1.0 catches more suspicious deals (e.g. 43% below median) while
still preserving legitimate edge cases via hard bounds. Users can override
per-call for more conservative filtering (k=1.5) if needed.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-28 22:40:29 +02:00
Nitzan P 421dec37b5 Fix duplicate polygon queries causing redundant API calls
Bug: When the Govmap API returns the same polygon_id multiple times
(which can happen for addresses with multiple associated polygons),
the code was processing each duplicate separately, leading to:
- Duplicate API calls for street/neighborhood deals
- Wasted API quota and slower performance
- Confusing logs showing same polygon queried multiple times

Example from logs:
  INFO: Querying polygon 53283601 (distance: 0m from search point)
  INFO: Getting street deals for polygon: 53283601
  INFO: Querying polygon 53283601 (distance: 0m from search point)
  INFO: Getting street deals for polygon: 53283601

Root cause:
- nearby_polygons API response can contain duplicate polygon_ids
- Code was appending all polygons without deduplication
- Log message claimed "unique polygon IDs" but didn't enforce it

Solution:
- Use dict to deduplicate polygons by polygon_id (lines 612-643)
- When duplicates exist, keep the one with shortest distance
- Convert dict to list after deduplication
- Now truly have unique polygon IDs as the log claims

Impact:
- Reduces API calls (fewer duplicates = less load on Govmap API)
- Faster queries (less redundant processing)
- Clearer logs (no confusing duplicate entries)
- Better performance for addresses with many associated polygons

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-27 00:07:15 +02:00
Nitzan P ab59e13936 Fix: Same-building detection using correct API field names
The same-building detection was failing because the code was accessing
deal.street_name and deal.house_number (Pydantic model field names),
but the Govmap API actually returns streetNameHeb/streetNameEng and
houseNum as extra fields.

Modified address construction to check multiple field names using
getattr() to handle the API's actual field names, falling back to
model field names if needed.

This fix ensures same-building deals are correctly identified and
prioritized with priority=0.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-25 09:20:14 +02:00
Nitzan P a809049f5e Feat: Implement distance-based deal prioritization and filtering
Implements smart deal prioritization based on distance from search point,
ensuring most relevant comparables are selected first.

Problem:
- Queried all polygons equally without distance consideration
- No filtering for deals that are too far away
- Street/neighborhood deals could be from distant locations
- No way to prefer closest polygon when multiple available

Solution:
1. Sort polygons by distance from search point (closest first)
2. Query polygons progressively, stop early if enough deals found
3. Calculate and store distance_meters for each deal
4. Filter deals by configurable distance thresholds:
   - Street deals: max 500m (configurable)
   - Neighborhood deals: max 1000m (configurable)
5. Sort by priority → distance → date (prefer closer deals)

Changes:

Config (nadlan_mcp/config.py):
- Added max_street_deal_distance_meters (default: 500m)
- Added max_neighborhood_deal_distance_meters (default: 1000m)

Client (nadlan_mcp/govmap/client.py):
- Extract polygon metadata with coordinates
- Sort polygons by distance using utils.calculate_distance()
- Progressive polygon querying (closest first, stop when enough deals)
- Calculate deal distance (use polygon distance as approximation)
- Filter street deals beyond max_street_deal_distance_meters
- Filter neighborhood deals beyond max_neighborhood_deal_distance_meters
- Store distance_meters on each Deal (dynamic attribute)
- Updated sorting: Priority → Distance → Date

Benefits:
- More relevant comparables (from nearby locations)
- Reduced API calls (query fewer polygons, stop early)
- Better performance (fewer deals to process)
- Configurable distance thresholds
- Transparent (distance info available in each deal)

Example Configuration:
export MAX_STREET_DEAL_DISTANCE_METERS=300    # Conservative
export MAX_NEIGHBORHOOD_DEAL_DISTANCE_METERS=500

Testing:
- All 326 tests pass
- Backward compatible (high default limits)
- Works with existing outlier filtering

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-24 23:26:47 +02:00
Nitzan Pomerantz 865aae1b73 Fix autocomplete non-determinism causing 0 deals
Autocomplete returns different coordinates for vague queries (e.g. "תל אביב דיזנגוף"),
sometimes landing on points with no nearby polygon_ids within 30m radius.

- Try top 3 autocomplete results until one has polygons
- Fallback radius expansion (30m→200m) if no polygons found
- Fix limit validation bug (min 1 for deal queries)

Fixes: compare_addresses, get_valuation_comparables, get_deal_statistics

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-14 23:48:12 +02:00
Nitzan Pomerantz e4aa6487ff Ruff fixes 2025-10-30 22:24:40 +02:00
Nitzan Pomerantz 3478426006 Fix too many polygons issue 2025-10-27 23:58:57 +02:00
Nitzan Pomerantz 247ddad8cd CRITICAL FIX: get_deals_by_radius returns polygon metadata, not deals
## Root Cause
After Pydantic migration, get_deals_by_radius() was attempting to validate
API responses as Deal objects. However, this endpoint returns POLYGON METADATA
(with fields: dealscount, polygon_id, settlementNameHeb), NOT individual deals.

All responses failed Pydantic validation (missing dealAmount, dealDate),
resulting in empty lists and 0 deals returned from ALL queries.

## Fixes Applied

### 1. client.py - get_deals_by_radius()
- Return type: `List[Deal]` → `List[Dict[str, Any]]`
- Remove Pydantic validation - return raw metadata dicts
- Update docstring to clarify this returns polygon metadata
- Add note to use find_recent_deals_for_address() for actual deals

### 2. client.py - find_recent_deals_for_address()
- Update to handle polygon metadata dicts (not Deal objects)
- Use dict.get('polygon_id') instead of model attribute access
- Rename variable: `nearby_deals` → `nearby_polygons` for clarity

### 3. fastmcp_server.py - get_deals_by_radius() tool
- Update to handle dict responses (not Deal objects)
- Remove strip_bloat_fields() call (not needed for metadata)
- Update docstring with WARNING about polygon metadata
- Change response keys: "deals" → "polygons", "total_deals" → "total_polygons"

### 4. Tests
- test_govmap_client.py: Update to expect dicts, not Deal objects
- test_fastmcp_tools.py: Update 3 tests to mock dict responses

## Impact
-  find_recent_deals_for_address() NOW WORKS (was returning 0 deals)
-  All 174 tests passing
-  E2E API test confirmed working with real data

## API Behavior Documented
get_deals_by_radius endpoint design:
1. Returns polygon/area metadata (not individual deals)
2. Extract polygon_ids from metadata
3. Call get_street_deals(polygon_id) to get actual deals
4. This workflow is automated in find_recent_deals_for_address()

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-10-27 09:38:21 +02:00
Nitzan Pomerantz 0ac9d136cd Implementation of phase 4.1 2025-10-26 10:58:46 +02:00
Nitzan Pomerantz 7757694077 Phase 3: Complete govmap package extraction (step 2/3)
- Created market_analysis.py with market analysis functions:
  - parse_deal_dates() - Date parsing and filtering helper
  - calculate_market_activity_score() - Activity metrics
  - analyze_investment_potential() - Investment analysis
  - get_market_liquidity() - Liquidity metrics

- Created client.py with GovmapClient class:
  - Core API methods (autocomplete, get deals, etc.)
  - Validation methods delegating to validators module
  - Utility methods delegating to utils module
  - Filtering, statistics, and analysis methods delegating to respective modules

- Updated govmap/__init__.py to export GovmapClient

All modules maintain backward compatibility through delegation pattern.
Next: Delete old govmap.py and update imports.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-10-25 13:18:19 +03:00