feat(security): enforce cloud_review fail-closed before non-local reviewer dispatch (D23) #101

Merged

pdurlej merged 1 commit from claude/cloud-review-enforcement into main

2026-06-23 01:11:36 +02:00

claude commented

2026-06-22 23:56:32 +02:00

Collaborator

What

Closes the highest-severity gap from the 2026-06-16 audit: effective_cloud_review() (the D7/D20 posture rule) was defined + unit-tested but never called in the live dispatch path — review_run sent the diff to whatever OLLAMA_BASE_URL pointed at, regardless of classification. A sensitive-class PR could have its diff sent off-box.

Two layers, fail-closed (operator-approved design, #9)

review_run.py — new CloudReviewForbiddenError + _is_local_endpoint; ReviewRunInput.cloud_review_posture (default "forbidden"); _findings_from_ollama hard-raises before any model call when posture != "allowed" AND the endpoint is non-local. Loopback endpoints always permitted; unparseable host → non-local (fail-closed).
cli.py — _review_input_from_args classifies the changed files + resolves effective_cloud_review(classification, lane) into the posture; the review-run handler maps CloudReviewForbiddenError → exit 2.

Sub-decisions (D23, recorded in decisions.md)

hard fail (exit 2), never soft-fail-with-metadata.
"local" = 127.0.0.1 / localhost / ::1 / 0.0.0.0; unparseable fails closed.
required-with-redaction = forbidden until a redactor exists (separate future slice).

Tests

tests/test_cloud_review_enforcement.py: forbidden+cloud raises · allowed+cloud dispatches · forbidden+local dispatches · default posture fails closed · _is_local_endpoint cases.
326 tests green. stdlib-only (urllib.parse). No existing test set a non-local OLLAMA_BASE_URL, so nothing regressed.

Merge order

cli.py overlaps the keystone (#97, already merged) — branched off post-keystone main, no conflict.

From the docs-maturity wave (item #9). Do not merge automatically — operator merge only.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

## What Closes the **highest-severity gap** from the 2026-06-16 audit: `effective_cloud_review()` (the D7/D20 posture rule) was defined + unit-tested but **never called in the live dispatch path** — `review_run` sent the diff to whatever `OLLAMA_BASE_URL` pointed at, regardless of classification. A sensitive-class PR could have its diff sent off-box. ## Two layers, fail-closed (operator-approved design, #9) 1. **review_run.py** — new `CloudReviewForbiddenError` + `_is_local_endpoint`; `ReviewRunInput.cloud_review_posture` (default **`"forbidden"`**); `_findings_from_ollama` **hard-raises before any model call** when posture `!= "allowed"` AND the endpoint is non-local. Loopback endpoints always permitted; unparseable host → non-local (fail-closed). 2. **cli.py** — `_review_input_from_args` classifies the changed files + resolves `effective_cloud_review(classification, lane)` into the posture; the `review-run` handler maps `CloudReviewForbiddenError` → exit 2. ## Sub-decisions (D23, recorded in decisions.md) - **hard fail** (exit 2), never soft-fail-with-metadata. - **"local"** = `127.0.0.1` / `localhost` / `::1` / `0.0.0.0`; unparseable fails closed. - **`required-with-redaction`** = forbidden until a redactor exists (separate future slice). ## Tests - `tests/test_cloud_review_enforcement.py`: forbidden+cloud raises · allowed+cloud dispatches · forbidden+**local** dispatches · **default posture fails closed** · `_is_local_endpoint` cases. - **326 tests green.** stdlib-only (`urllib.parse`). No existing test set a non-local `OLLAMA_BASE_URL`, so nothing regressed. ## Merge order - `cli.py` overlaps the keystone (#97, already merged) — branched off post-keystone main, no conflict. From the docs-maturity wave (item #9). **Do not merge automatically — operator merge only.** Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Rows
Columns