From 0cfd02d0457cb942b276a6cb4778400b63f82331 Mon Sep 17 00:00:00 2001 From: ghzhost Date: Mon, 24 Aug 2026 01:19:00 +0000 Subject: [PATCH] fix(policy): enforce allowed_metros whitelist before applying location exceptions (#14) --- src/openjobradar/policy/location.py | 46 ++++++++++++--------------- tests/test_location_policy.py | 48 +++++++++++++++++++++++++---- 2 files changed, 62 insertions(+), 32 deletions(-) diff --git a/src/openjobradar/policy/location.py b/src/openjobradar/policy/location.py index 857ce1a..99c5577 100644 --- a/src/openjobradar/policy/location.py +++ b/src/openjobradar/policy/location.py @@ -19,6 +19,7 @@ in the verdict reason. - Unparseable/no location text -> fail closed (rule ``no_location``). """ + from __future__ import annotations import re @@ -125,9 +126,7 @@ def _cadence_phrase(days: int | None) -> str: return f"{days} day(s)/week" if days is not None else "unstated cadence" -def _site_based_verdict( - raw_location: str | None, normalized: str, section: Mapping[str, Any] -) -> LocationVerdict: +def _site_based_verdict(raw_location: str | None, normalized: str, section: Mapping[str, Any]) -> LocationVerdict: metros: list[Any] = section.get("allowed_metros") or [] metro_names = [m for m in metros if isinstance(m, str)] exceptions = [e for e in (section.get("exceptions") or []) if isinstance(e, Mapping)] @@ -152,34 +151,31 @@ def _site_based_verdict( detail += "; no listed exception covers this posting" return LocationVerdict(DENY, "onsite_exceeds_limit", detail + ".") + if matched_metro is None: + if metro_names: + return LocationVerdict( + DENY, + "outside_metros", + f"Location {raw_location!r} is outside your allowed metros.", + ) + return LocationVerdict( + DENY, + "no_metros_configured", + "Your location policy whitelists no on-site metros; only remote roles pass.", + ) + if winning_exception is not None: note = winning_exception.get("note") detail = ( f"Exception {winning_exception['match']!r} covers this posting " - f"({_cadence_phrase(days)}" - + (f"; {note}" if note else "") - + ")." + f"({_cadence_phrase(days)}" + (f"; {note}" if note else "") + ")." ) return LocationVerdict(ALLOW, "exception_applied", detail) - if matched_metro is not None: - return LocationVerdict( - ALLOW, - "metro_allowed", - f"{matched_metro} is in your allowed metros ({_cadence_phrase(days)}).", - ) - - if metro_names: - return LocationVerdict( - DENY, - "outside_metros", - f"Location {raw_location!r} is outside your allowed metros.", - ) - return LocationVerdict( - DENY, - "no_metros_configured", - "Your location policy whitelists no on-site metros; only remote roles pass.", + ALLOW, + "metro_allowed", + f"{matched_metro} is in your allowed metros ({_cadence_phrase(days)}).", ) @@ -190,9 +186,7 @@ def evaluate_location(raw_location: str | None, policy: Mapping[str, Any]) -> Lo normalized = _normalize(raw_location or "") if not normalized: - return LocationVerdict( - DENY, "no_location", "Posting carries no usable location information." - ) + return LocationVerdict(DENY, "no_location", "Posting carries no usable location information.") if _REMOTE_RE.search(normalized): return LocationVerdict(ALLOW, "remote", f"Remote-friendly role ({raw_location!r}).") diff --git a/tests/test_location_policy.py b/tests/test_location_policy.py index 730e3d3..a220caa 100644 --- a/tests/test_location_policy.py +++ b/tests/test_location_policy.py @@ -1,4 +1,5 @@ """Location-policy evaluation: prototype semantics, config-driven (M1.2 / ADR-0012).""" + from __future__ import annotations from hypothesis import given @@ -49,17 +50,13 @@ def test_exception_raises_ceiling_and_is_cited() -> None: } ], ) - allowed = evaluate_location( - "Downtown Sacramento, CA — 4 days onsite per week", policy - ) + allowed = evaluate_location("Downtown Sacramento, CA — 4 days onsite per week", policy) assert allowed.allowed assert allowed.rule == "exception_applied" assert "'Downtown Sacramento'" in allowed.reason assert "near transit" in allowed.reason - rejected = evaluate_location( - "Downtown Sacramento, CA — 5 days onsite per week", policy - ) + rejected = evaluate_location("Downtown Sacramento, CA — 5 days onsite per week", policy) assert not rejected.allowed assert "even with exception" in rejected.reason @@ -126,3 +123,42 @@ def test_verdict_truthiness_matches_status() -> None: def test_constants_are_stable() -> None: assert DENY == "deny" + + +def test_exception_does_not_bypass_metro_whitelist() -> None: + policy = { + "location_policy": { + "allowed_metros": ["Davis", "Sacramento", "Bay Area"], + "max_onsite_days_per_week": 2, + "exceptions": [ + { + "match": "quarterly onsite summit", + "max_onsite_days_per_week": 5, + "note": "occasional travel OK", + } + ], + } + } + verdict = evaluate_location("Miami, FL — hybrid, quarterly onsite summit, 2 days onsite/week", policy) + assert not verdict.allowed + assert verdict.rule == "outside_metros" + assert "outside your allowed metros" in verdict.reason + + +def test_exception_does_not_bypass_empty_whitelist() -> None: + policy = { + "location_policy": { + "allowed_metros": [], + "max_onsite_days_per_week": 2, + "exceptions": [ + { + "match": "onsite summit", + "max_onsite_days_per_week": 5, + "note": "occasional travel OK", + } + ], + } + } + verdict = evaluate_location("Dallas, TX — onsite summit", policy) + assert not verdict.allowed + assert verdict.rule == "no_metros_configured"