diff --git a/changelog.d/calculate-malformed-payload-400.fixed.md b/changelog.d/calculate-malformed-payload-400.fixed.md new file mode 100644 index 000000000..1a63c6479 --- /dev/null +++ b/changelog.d/calculate-malformed-payload-400.fixed.md @@ -0,0 +1 @@ +Return 400 instead of 500 when a calculate payload fails situation parsing. diff --git a/changelog.d/lb-cloud-armor-runbook.added.md b/changelog.d/lb-cloud-armor-runbook.added.md new file mode 100644 index 000000000..b2472ebec --- /dev/null +++ b/changelog.d/lb-cloud-armor-runbook.added.md @@ -0,0 +1 @@ +Add Cloud Armor rate-limiting runbook and policy snapshots for the public API load balancer. diff --git a/docs/migration/armor/20260721T142259Z-pol-api-lb.yaml b/docs/migration/armor/20260721T142259Z-pol-api-lb.yaml new file mode 100644 index 000000000..7e1194801 --- /dev/null +++ b/docs/migration/armor/20260721T142259Z-pol-api-lb.yaml @@ -0,0 +1,50 @@ +creationTimestamp: '2026-07-21T07:16:39.177-07:00' +description: Rate limiting for api.policyengine.org (lb-api). See docs/migration/lb-cloud-armor-runbook.md +fingerprint: b-PKxv9L5OA= +id: '200433714284642728' +kind: compute#securityPolicy +labelFingerprint: 42WmSpB8rSM= +name: pol-api-lb +rules: +- action: throttle + description: '' + kind: compute#securityPolicyRule + match: + expr: + expression: request.path.matches('^/[a-z]{2}/metadata$') + preview: true + priority: 1000 + rateLimitOptions: + conformAction: allow + enforceOnKey: IP + exceedAction: deny(429) + rateLimitThreshold: + count: 30 + intervalSec: 60 +- action: throttle + description: '' + kind: compute#securityPolicyRule + match: + expr: + expression: request.path.matches('^/[a-z]{2}/calculate(?:-full)?$') + preview: true + priority: 1100 + rateLimitOptions: + conformAction: allow + enforceOnKey: IP + exceedAction: deny(429) + rateLimitThreshold: + count: 75 + intervalSec: 60 +- action: allow + description: default rule + kind: compute#securityPolicyRule + match: + config: + srcIpRanges: + - '*' + versionedExpr: SRC_IPS_V1 + preview: false + priority: 2147483647 +selfLink: https://www.googleapis.com/compute/v1/projects/policyengine-api/global/securityPolicies/pol-api-lb +type: CLOUD_ARMOR diff --git a/docs/migration/lb-cloud-armor-runbook.md b/docs/migration/lb-cloud-armor-runbook.md new file mode 100644 index 000000000..35c412bea --- /dev/null +++ b/docs/migration/lb-cloud-armor-runbook.md @@ -0,0 +1,137 @@ +# Cloud Armor rate-limiting runbook + +Repeated-use runbook for managing Cloud Armor security policies on a load +balancer's backend services: creating throttle rules safely (preview first), +deciding when to enforce them, tuning, and rolling back. The authoritative +record of what is currently deployed is the newest exported snapshot in +`docs/migration/armor/` — this document describes procedure, not state. + +## Current deployment (summary — see newest snapshot for truth) + +| | | +|---|---| +| Policy | `pol-api-lb`, attached to both backend services of the public API LB | +| Rules | Per-IP throttles on the metadata and calculate path families; thresholds in the snapshot | +| Mode | Created in preview 2026-07-21; enforcement pending the gates below | + +Origin: overnight bot waves saturated backends / churned autoscaling +(2026-07-13 → 2026-07-21 incidents; details in the cutover execution plan). + +## Design principles + +1. **Only throttle path families the frontends never poll.** Client apps poll + some endpoints at ~1 req/s per session (society-wide calculation status, + report status), and NAT'd offices aggregate many users onto one client IP. + A per-IP rule that matches a polled path will 429 legitimate sessions — + that is a design error, not a threshold-tuning problem. Before adding any + rule, check the frontend code and LB logs for polling behavior on the + matched paths. +2. **No blanket rules, no ban actions.** Throttling (429 per excess request) + self-heals the moment a client slows down; bans amplify any false positive. +3. **Preview first, always.** Every new or changed rule starts in preview + (matched and logged, never enforced) and graduates only through the gates + below. +4. **Rate limits address volume abuse, not expensive-request abuse.** A + handful of heavy requests that saturate capacity sits below any threshold + that spares real users — that is a capacity/scaling problem, not a Cloud + Armor problem. +5. Cloud Armor's CEL `.matches()` regex rejects capture groups — use + non-capturing `(?:...)`. + +## Known caveat + +Cloud Armor only sees traffic that crosses the load balancer. Serverless +default URLs (`*.run.app`, `*.appspot.com`) bypass it entirely. This is +acceptable while ingress must stay open for CI smoke tests; revisit when +ingress is locked down to the LB. + +## Procedures + +Set once per shell: + +```sh +PROJECT= +POLICY= +BACKENDS=" [ ...]" +``` + +### Create a policy and attach it + +```sh +gcloud compute security-policies create $POLICY --project=$PROJECT \ + --description=". See docs/migration/lb-cloud-armor-runbook.md" + +for BS in $BACKENDS; do + gcloud compute backend-services update $BS \ + --security-policy=$POLICY --global --project=$PROJECT +done +``` + +Immediately after attaching: verify serving is unaffected (sampled curls +against the public hostname, uptime monitor green, one live-suite run). + +### Add a per-IP throttle rule (in preview) + +```sh +gcloud compute security-policies rules create \ + --project=$PROJECT --security-policy=$POLICY \ + --expression="request.path.matches('')" \ + --action=throttle \ + --rate-limit-threshold-count= --rate-limit-threshold-interval-sec= \ + --conform-action=allow --exceed-action=deny-429 --enforce-on-key=IP \ + --preview +``` + +### Snapshot after every policy change + +```sh +gcloud compute security-policies export $POLICY --project=$PROJECT \ + --file-name=docs/migration/armor/$(date -u +%Y%m%dT%H%M%SZ)-$POLICY.yaml +``` + +Commit the snapshot (mirrors the `urlmap/` convention). + +### Preview → enforce gates + +Enforce a rule only when BOTH hold: + +1. **Preview observation (≥24h, spanning the traffic pattern the rule + targets — e.g. an overnight bot window):** LB log entries carry + `jsonPayload.previewSecurityPolicy` (`name`, `outcome`). Group would-be + throttle hits by client IP network, user agent, and path. Gate: **zero + hits from monitors, app-referred traffic, or polling clients; hits present + on the abusive sources the rule targets.** +2. **Empirical threshold check:** from ≥7 days of LB logs, compute the per-IP + peak request rate over the rule's interval for the matched path family, + split legitimate vs other. Gate: **worst legitimate rate ≤ half the + threshold.** + +```sh +gcloud compute security-policies rules update \ + --project=$PROJECT --security-policy=$POLICY --no-preview +``` + +Snapshot and commit; verify on the next abuse wave: 429s appear in LB logs +(`jsonPayload.enforcedSecurityPolicy.outcome="DENY"`), autoscaler churn and +monitor incidents stop. + +If a legitimate source shows up in preview hits: raise the threshold +(`rules update --rate-limit-threshold-count=`), stay in +preview, restart the observation window. + +### Tuning / rollback + +- Change a threshold: `rules update --rate-limit-threshold-count=` + (snapshot after). +- Disable enforcement fast: `rules update --preview` (back to + log-only; propagates in minutes). +- Full detach: `gcloud compute backend-services update + --security-policy="" --global --project=$PROJECT` per backend service. The + policy remains, unattached. + +## Cost + +Standard tier, pay-as-you-go: $5/policy/mo + $1/rule/mo + $0.75/M requests +evaluated (global policies; verified against the Billing Catalog API +2026-07-21). Order-of-magnitude ~$10/mo for one policy with a few rules at +sub-million monthly request volume. diff --git a/policyengine_api/endpoints/household.py b/policyengine_api/endpoints/household.py index 24e14661d..616da54e7 100644 --- a/policyengine_api/endpoints/household.py +++ b/policyengine_api/endpoints/household.py @@ -10,6 +10,7 @@ format_unrecognized_inputs_message, ) from policyengine_api.utils.payload_validators import validate_country +from policyengine_core.errors import SituationParsingError def get_countries(): @@ -274,6 +275,19 @@ def get_calculate(country_id: str, add_missing: bool = False) -> dict: try: result = country.calculate(household_json, policy_json) + except SituationParsingError as e: + # Malformed household payloads (e.g. a dict where a number belongs) + # are client errors, not server errors — mostly bot traffic. + response_body = dict( + status="error", + message=f"Invalid household payload: {e}", + result=None, + ) + return Response( + json.dumps(response_body), + status=400, + mimetype="application/json", + ) except Exception as e: logging.exception(e) response_body = dict( diff --git a/tests/unit/endpoints/test_calculate_error_statuses.py b/tests/unit/endpoints/test_calculate_error_statuses.py new file mode 100644 index 000000000..286bab6a3 --- /dev/null +++ b/tests/unit/endpoints/test_calculate_error_statuses.py @@ -0,0 +1,97 @@ +from flask import Flask +import pytest +from policyengine_core.errors import SituationParsingError + +from policyengine_api.endpoints import household as household_endpoint + +from .test_calculate_deprecated_inputs import DummyCountry + +HOUSEHOLD = { + "people": { + "you": { + "age": {"2026": 40}, + "employment_income": {"2026": 30_000}, + } + } +} + + +class ParsingErrorCountry(DummyCountry): + def calculate(self, household, policy): + raise SituationParsingError( + ["people", "you", "employment_income", "2026"], + "Can't deal with value: expected type number, received '{}'.", + ) + + +class CrashingCountry(DummyCountry): + def calculate(self, household, policy): + raise RuntimeError("engine exploded") + + +def make_client(monkeypatch, country, add_missing=False): + monkeypatch.setattr( + household_endpoint, + "get_countries", + lambda: {"us": country}, + ) + app = Flask(__name__) + if add_missing: + monkeypatch.setattr( + household_endpoint, + "add_yearly_variables", + lambda household, country_id: household, + ) + + def handler(country_id): + return household_endpoint.get_calculate(country_id, add_missing=True) + + app.add_url_rule( + "//calculate-full", + "calculate_full", + handler, + methods=["POST"], + ) + else: + app.add_url_rule( + "//calculate", + "calculate", + household_endpoint.get_calculate, + methods=["POST"], + ) + return app.test_client() + + +def test__calculate__returns_400_on_situation_parsing_error(monkeypatch): + client = make_client(monkeypatch, ParsingErrorCountry()) + + response = client.post("/us/calculate", json={"household": HOUSEHOLD}) + + assert response.status_code == 400 + payload = response.get_json() + assert payload["status"] == "error" + assert payload["result"] is None + assert payload["message"].startswith("Invalid household payload") + + +def test__calculate_full__returns_400_on_situation_parsing_error(monkeypatch): + client = make_client(monkeypatch, ParsingErrorCountry(), add_missing=True) + + response = client.post("/us/calculate-full", json={"household": HOUSEHOLD}) + + assert response.status_code == 400 + payload = response.get_json() + assert payload["status"] == "error" + assert payload["result"] is None + assert payload["message"].startswith("Invalid household payload") + + +def test__calculate__returns_500_on_unexpected_error(monkeypatch): + client = make_client(monkeypatch, CrashingCountry()) + + response = client.post("/us/calculate", json={"household": HOUSEHOLD}) + + assert response.status_code == 500 + payload = response.get_json() + assert payload["status"] == "error" + assert "engine exploded" in payload["message"]