Skip to content

Commit 779023d

Browse files
tachyon-beepclaude
andcommitted
refactor(wardline): centralize scan-routing validation in the service layer
The scan-route request-routing decision — "is request-side routing allowed, and is the cell-spec well-formed?" — was hand-copied into both the HTTP (/wardline/scan-results) and MCP (scan_route) adapters, alongside the cell-spec parse and a byte-for-byte _parse_wardline_cell_map helper. The copies had already drifted: HTTP rejected an empty cell_by_severity (422) while MCP silently accepted an empty severity_map and routed nothing — the "check added to one transport not the other" failure mode the layering exists to prevent. Extract service.resolve_scan_routing (+ ResolvedRouting, _parse_cell_map_env) as the single decision. It raises WardlineRoutingError carrying a `kind` discriminator; each adapter maps kind to its own taxonomy — HTTP 500/403/422 (_WARDLINE_ROUTING_STATUS), MCP collapses all three to INVALID_CELL_SPEC (via _service_error, before the generic ServiceError case). Both dead _parse_wardline_cell_map copies are removed; env reads stay in the adapters. Behavior-preserving for every pinned case (HTTP 403/"server-owned" + 422; MCP INVALID_CELL_SPEC; the malformed-finding path still raises INVALID_ARGUMENT from inside route_wardline_scan, kept distinct from routing errors). One intended change closes the drift: an empty per-severity map is now rejected up front on both transports — no silent governance skip. TDD: tests/service/test_wardline.py (14 cases) + a new MCP regression test_scan_route_rejects_empty_severity_map. Full suite 699 passed, mypy + ruff clean, coverage floors hold, policy-boundary-check + governance-gate + SEI oracle green. rc4 review finding #4 (legis-604ddb8dd4). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 020c0c6 commit 779023d

7 files changed

Lines changed: 393 additions & 145 deletions

File tree

CHANGELOG.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,19 @@ listed as not-yet-built.
212212
`canonical.canonical_json` (whose `ensure_ascii=False` would change the signed
213213
bytes). Behavior-preserving — existing per-channel golden vectors unchanged,
214214
plus a new cross-channel anti-drift test. No change to signatures on the wire.
215+
- **Wardline scan-routing validation centralised in the service layer** — "is
216+
request-side routing allowed, and is the cell-spec well-formed?" is a
217+
governance decision that was hand-copied into both the HTTP
218+
(`/wardline/scan-results`) and MCP (`scan_route`) adapters, along with the
219+
cell-spec parse and a `_parse_wardline_cell_map` helper. The copies had already
220+
drifted: the HTTP adapter rejected an empty `cell_by_severity` (422) while MCP
221+
silently accepted an empty `severity_map` and routed nothing. The decision now
222+
lives in `service.resolve_scan_routing`, raising a `WardlineRoutingError` whose
223+
`kind` each adapter maps to its own taxonomy (HTTP 500/403/422 by kind; MCP
224+
collapses to `INVALID_CELL_SPEC`) — so a new routing rule is added once and
225+
cannot reach one transport but not the other. Behavior-preserving for every
226+
pinned case; the one intended change closes the drift (an empty per-severity
227+
map is now rejected up front on both transports — no silent governance skip).
215228

216229
### Fixed
217230
- **Ingest accepts realistic scans** — the over-strict Wardline ingest validator

src/legis/api/app.py

Lines changed: 36 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,12 @@
4848
from legis.governance.signoff_binding import bind_signoff_to_issue
4949
from legis.identity.entity_key import EntityKey
5050
from legis.identity.resolver import IdentityResolver
51-
from legis.service.errors import AuditIntegrityError, InvalidArgumentError, NotEnabledError
51+
from legis.service.errors import (
52+
AuditIntegrityError,
53+
InvalidArgumentError,
54+
NotEnabledError,
55+
WardlineRoutingError,
56+
)
5257
from legis.service.governance import compute_override_rate as _compute_override_rate
5358
from legis.service.governance import evaluate_policy as _evaluate_policy
5459
from legis.service.governance import request_signoff as _request_signoff
@@ -58,7 +63,10 @@
5863
from legis.service.governance import submit_override as _submit_override
5964
from legis.service.governance import submit_protected_override as _submit_protected_override
6065
from legis.service.governance import verified_records as _verified_records
61-
from legis.service.wardline import route_wardline_scan as _route_wardline_scan
66+
from legis.service.wardline import (
67+
resolve_scan_routing,
68+
route_wardline_scan as _route_wardline_scan,
69+
)
6270
from legis.policy.grammar import PolicyGrammar, default_grammar
6371
from legis.pulls.models import PullRequest, PullRequestState
6472
from legis.pulls.surface import PullSurface
@@ -67,7 +75,6 @@
6775
ScanOutcome,
6876
WardlineDirtyTreeError,
6977
WardlinePayloadError,
70-
WardlineSeverity,
7178
)
7279

7380
security = HTTPBearer(auto_error=False)
@@ -247,20 +254,13 @@ class CheckRunIn(BaseModel):
247254
finished_at: str | None = None
248255

249256

250-
def _parse_wardline_cell_map(raw: str) -> dict[WardlineSeverity, WardlineCellPolicy]:
251-
mapping: dict[WardlineSeverity, WardlineCellPolicy] = {}
252-
for part in raw.split(","):
253-
if not part.strip():
254-
continue
255-
severity_raw, sep, cell_raw = part.partition("=")
256-
if not sep:
257-
raise ValueError("cell map entries must be SEVERITY=cell")
258-
mapping[WardlineSeverity[severity_raw.strip()]] = WardlineCellPolicy(
259-
cell_raw.strip()
260-
)
261-
if not mapping:
262-
raise ValueError("cell map must not be empty")
263-
return mapping
257+
# Wardline scan-routing rejections (raised by service.resolve_scan_routing) map
258+
# to HTTP status by kind; the MCP adapter collapses the same kinds to one code.
259+
_WARDLINE_ROUTING_STATUS = {
260+
WardlineRoutingError.SERVER_MISCONFIGURED: 500,
261+
WardlineRoutingError.SERVER_OWNED: 403,
262+
WardlineRoutingError.MALFORMED: 422,
263+
}
264264

265265

266266
def _check_to_dict(run: CheckRun) -> dict:
@@ -772,72 +772,37 @@ def policy_evaluate(body: PolicyEvalIn, actor: str = Depends(verify_writer)) ->
772772

773773
@app.post("/wardline/scan-results")
774774
def wardline_scan_results(body: ScanResultsIn, actor: str = Depends(verify_writer)) -> dict:
775-
server_cell = os.environ.get("LEGIS_WARDLINE_CELL")
776-
server_cell_by_severity = os.environ.get("LEGIS_WARDLINE_CELL_BY_SEVERITY")
777-
if server_cell and server_cell_by_severity:
778-
raise HTTPException(status_code=500, detail="server Wardline routing is misconfigured")
779-
server_routing = server_cell is not None or server_cell_by_severity is not None
780-
if server_routing and (
781-
body.cell is not None or body.cell_by_severity is not None or body.fail_on is not None
782-
):
783-
raise HTTPException(status_code=403, detail="Wardline routing is server-owned")
784-
if not server_routing:
785-
if os.environ.get("LEGIS_UNSAFE_WARDLINE_REQUEST_ROUTING") != "1":
786-
raise HTTPException(
787-
status_code=403,
788-
detail="Wardline routing is server-owned; configure LEGIS_WARDLINE_CELL or LEGIS_WARDLINE_CELL_BY_SEVERITY",
789-
)
790-
if body.fail_on is not None:
791-
if body.cell is None or body.cell_by_severity is not None:
792-
raise HTTPException(
793-
status_code=422,
794-
detail="fail_on routing requires cell and forbids cell_by_severity",
795-
)
796-
elif (body.cell is None) == (body.cell_by_severity is None):
797-
raise HTTPException(status_code=422,
798-
detail="provide exactly one of cell or cell_by_severity")
799-
if body.cell_by_severity is not None and not body.cell_by_severity:
800-
raise HTTPException(status_code=422, detail="cell_by_severity must not be empty")
801-
802-
policy: WardlineCellPolicy | None = None
803-
cell_map: dict[WardlineSeverity, WardlineCellPolicy] | None = None
804-
fail_on: WardlineSeverity | None = None
805775
try:
806-
if server_cell_by_severity is not None:
807-
cell_map = _parse_wardline_cell_map(server_cell_by_severity)
808-
cells = set(cell_map.values())
809-
elif server_cell is not None:
810-
policy = WardlineCellPolicy(server_cell)
811-
cells = {policy}
812-
elif body.cell_by_severity is not None:
813-
cell_map = {WardlineSeverity[sev]: WardlineCellPolicy(cell)
814-
for sev, cell in body.cell_by_severity.items()}
815-
cells = set(cell_map.values())
816-
else:
817-
policy = WardlineCellPolicy(body.cell)
818-
if body.fail_on is not None:
819-
fail_on = WardlineSeverity[body.fail_on]
820-
cells = {policy, WardlineCellPolicy.SURFACE_ONLY}
821-
else:
822-
cells = {policy}
823-
except (KeyError, ValueError) as exc:
824-
raise HTTPException(status_code=422, detail=f"unknown cell/severity: {exc}")
776+
routing = resolve_scan_routing(
777+
server_cell=os.environ.get("LEGIS_WARDLINE_CELL"),
778+
server_cell_by_severity=os.environ.get("LEGIS_WARDLINE_CELL_BY_SEVERITY"),
779+
request_cell=body.cell,
780+
request_severity_map=body.cell_by_severity,
781+
request_fail_on=body.fail_on,
782+
allow_request_routing=(
783+
os.environ.get("LEGIS_UNSAFE_WARDLINE_REQUEST_ROUTING") == "1"
784+
),
785+
)
786+
except WardlineRoutingError as exc:
787+
raise HTTPException(
788+
status_code=_WARDLINE_ROUTING_STATUS[exc.kind], detail=str(exc)
789+
) from exc
825790

826791
# Only provision the governance store when a surface cell can actually run:
827792
# engine() lazily creates .weft/legis/legis-governance.db, so a pure block_escalate scan
828793
# must not touch it. signoff_gate is an injected param (no side effect).
829-
needs_engine = bool(cells & {WardlineCellPolicy.SURFACE_OVERRIDE,
830-
WardlineCellPolicy.SURFACE_ONLY})
794+
needs_engine = bool(routing.cells & {WardlineCellPolicy.SURFACE_OVERRIDE,
795+
WardlineCellPolicy.SURFACE_ONLY})
831796
try:
832797
routed = _route_wardline_scan(
833798
body.scan,
834799
agent_id=_recorded_actor(actor, body.agent_id),
835800
identity=identity,
836801
engine=engine() if needs_engine else None,
837802
signoff=signoff_gate,
838-
policy=policy,
839-
cell_map=cell_map,
840-
fail_on=fail_on,
803+
policy=routing.policy,
804+
cell_map=routing.cell_map,
805+
fail_on=routing.fail_on,
841806
artifact_key=(
842807
os.environ["LEGIS_WARDLINE_ARTIFACT_KEY"].encode("utf-8")
843808
if os.environ.get("LEGIS_WARDLINE_ARTIFACT_KEY")

src/legis/mcp.py

Lines changed: 28 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
NotEnabledError,
4444
NotFoundError,
4545
ServiceError,
46+
WardlineRoutingError,
4647
)
4748
from legis.service.explain import explain_policy
4849
from legis.service.governance import (
@@ -53,10 +54,9 @@
5354
request_signoff,
5455
verified_records as service_verified_records,
5556
)
56-
from legis.service.wardline import route_wardline_scan
57+
from legis.service.wardline import resolve_scan_routing, route_wardline_scan
5758
from legis.store.audit_store import AuditStore
58-
from legis.wardline.governor import WardlineCellPolicy
59-
from legis.wardline.ingest import ScanOutcome, WardlineDirtyTreeError, WardlineSeverity
59+
from legis.wardline.ingest import ScanOutcome, WardlineDirtyTreeError
6060

6161

6262
_AGENT_TOOLS = frozenset(
@@ -414,6 +414,11 @@ def _service_error(exc: Exception) -> dict[str, Any]:
414414
return _tool_error("NOT_FOUND", str(exc))
415415
if isinstance(exc, InvalidArgumentError):
416416
return _tool_error("INVALID_ARGUMENT", str(exc))
417+
if isinstance(exc, WardlineRoutingError):
418+
# All three routing kinds (server-misconfigured / server-owned /
419+
# malformed) collapse to one MCP code; the HTTP adapter splits them by
420+
# status. Must precede the generic ServiceError case below.
421+
return _tool_error("INVALID_CELL_SPEC", str(exc))
417422
if isinstance(exc, GitError):
418423
return _tool_error("GIT_ERROR", str(exc))
419424
if isinstance(exc, ServiceError):
@@ -519,22 +524,6 @@ def _registry(runtime: McpRuntime) -> PolicyCellRegistry:
519524
return runtime.cell_registry or fail_closed_policy_cells()
520525

521526

522-
def _parse_wardline_cell_map(raw: str) -> dict[WardlineSeverity, WardlineCellPolicy]:
523-
mapping: dict[WardlineSeverity, WardlineCellPolicy] = {}
524-
for part in raw.split(","):
525-
if not part.strip():
526-
continue
527-
severity_raw, sep, cell_raw = part.partition("=")
528-
if not sep:
529-
raise ValueError("cell map entries must be SEVERITY=cell")
530-
mapping[WardlineSeverity[severity_raw.strip()]] = WardlineCellPolicy(
531-
cell_raw.strip()
532-
)
533-
if not mapping:
534-
raise ValueError("cell map must not be empty")
535-
return mapping
536-
537-
538527
def _explanation_payload(explanation) -> dict[str, Any]:
539528
payload = explanation.to_payload()
540529
payload["available_moves"] = [
@@ -925,69 +914,34 @@ def _tool_policy_evaluate(runtime: McpRuntime, args: dict[str, Any]) -> dict[str
925914

926915

927916
def _tool_scan_route(runtime: McpRuntime, args: dict[str, Any]) -> dict[str, Any]:
928-
server_cell = os.environ.get("LEGIS_WARDLINE_CELL")
929-
server_cell_by_severity = os.environ.get("LEGIS_WARDLINE_CELL_BY_SEVERITY")
930-
if server_cell and server_cell_by_severity:
931-
return _tool_error(
932-
"INVALID_CELL_SPEC", "server Wardline routing is misconfigured"
933-
)
934-
has_cell = "cell" in args
935-
has_map = "severity_map" in args
936-
has_fail_on = "fail_on" in args
937-
server_routing = server_cell is not None or server_cell_by_severity is not None
938-
if server_routing and (has_cell or has_map or has_fail_on):
939-
return _tool_error(
940-
"INVALID_CELL_SPEC", "Wardline routing is server-owned"
941-
)
942-
if not server_routing:
943-
if os.environ.get("LEGIS_UNSAFE_WARDLINE_REQUEST_ROUTING") != "1":
944-
return _tool_error(
945-
"INVALID_CELL_SPEC",
946-
"Wardline routing is server-owned; configure "
947-
"LEGIS_WARDLINE_CELL or LEGIS_WARDLINE_CELL_BY_SEVERITY",
948-
)
949-
if has_fail_on:
950-
if not has_cell or has_map:
951-
return _tool_error(
952-
"INVALID_CELL_SPEC",
953-
"fail_on routing requires cell and forbids severity_map",
954-
)
955-
elif has_cell == has_map:
956-
return _tool_error(
957-
"INVALID_CELL_SPEC",
958-
"provide exactly one of cell or severity_map",
959-
)
917+
# "severity_map" must be an object if present (transport-type check); the
918+
# governance decision — is request routing allowed, and is the spec
919+
# well-formed? — lives in resolve_scan_routing, shared with the HTTP adapter.
920+
# A WardlineRoutingError propagates to call_tool's translator → INVALID_CELL_SPEC.
921+
request_severity_map = (
922+
_require_object(args, "severity_map") if "severity_map" in args else None
923+
)
924+
routing = resolve_scan_routing(
925+
server_cell=os.environ.get("LEGIS_WARDLINE_CELL"),
926+
server_cell_by_severity=os.environ.get("LEGIS_WARDLINE_CELL_BY_SEVERITY"),
927+
request_cell=args.get("cell"),
928+
request_severity_map=request_severity_map,
929+
request_fail_on=args.get("fail_on"),
930+
allow_request_routing=(
931+
os.environ.get("LEGIS_UNSAFE_WARDLINE_REQUEST_ROUTING") == "1"
932+
),
933+
)
960934
scan = _require_object(args, "scan")
961-
scan_policy: WardlineCellPolicy | None = None
962-
scan_cell_map: dict[WardlineSeverity, WardlineCellPolicy] | None = None
963-
scan_fail_on: WardlineSeverity | None = None
964-
try:
965-
if server_cell_by_severity is not None:
966-
scan_cell_map = _parse_wardline_cell_map(server_cell_by_severity)
967-
elif server_cell is not None:
968-
scan_policy = WardlineCellPolicy(server_cell)
969-
elif has_cell:
970-
scan_policy = WardlineCellPolicy(_require(args, "cell"))
971-
if has_fail_on:
972-
scan_fail_on = WardlineSeverity[_require(args, "fail_on")]
973-
else:
974-
raw_map = _require_object(args, "severity_map")
975-
scan_cell_map = {
976-
WardlineSeverity[severity]: WardlineCellPolicy(cell)
977-
for severity, cell in raw_map.items()
978-
}
979-
except (KeyError, ValueError) as exc:
980-
return _tool_error("INVALID_CELL_SPEC", str(exc))
981935
try:
982936
routed = route_wardline_scan(
983937
scan,
984938
agent_id=runtime.agent_id,
985939
identity=runtime.identity,
986940
engine=_engine(runtime),
987941
signoff=runtime.signoff_gate,
988-
policy=scan_policy,
989-
cell_map=scan_cell_map,
990-
fail_on=scan_fail_on,
942+
policy=routing.policy,
943+
cell_map=routing.cell_map,
944+
fail_on=routing.fail_on,
991945
artifact_key=(
992946
runtime.wardline_artifact_key
993947
or (

src/legis/service/errors.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,25 @@ class InvalidArgumentError(ServiceError):
2828
"""Caller input is structurally valid for the transport but invalid for Legis."""
2929

3030

31+
class WardlineRoutingError(ServiceError):
32+
"""A Wardline scan-routing request is not permitted or is malformed.
33+
34+
Carries a ``kind`` discriminator so each adapter can preserve its own
35+
taxonomy without re-implementing the decision: the HTTP adapter maps
36+
``server_misconfigured`` → 500, ``server_owned`` → 403, ``malformed`` → 422,
37+
while the MCP adapter collapses all three to ``INVALID_CELL_SPEC``. Adapters
38+
switch on the ``kind`` attribute, never on message text.
39+
"""
40+
41+
SERVER_MISCONFIGURED = "server_misconfigured"
42+
SERVER_OWNED = "server_owned"
43+
MALFORMED = "malformed"
44+
45+
def __init__(self, kind: str, message: str) -> None:
46+
super().__init__(message)
47+
self.kind = kind
48+
49+
3150
class ProtectedKeyRequiredError(ServiceError):
3251
"""A protected trail was read without the HMAC key needed to verify it.
3352

0 commit comments

Comments
 (0)