From bc8790c5ba19e1ee5783ec0630a6a1dbbd96f019 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 20:06:13 +0000 Subject: [PATCH 1/4] feat(ownir): bound source coordinates and nesting depth, Python-first MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `OwnIR` is a file some frontend wrote, and two of its shapes had no bound at all. The reference accepted both because PYTHON has no bound — integers are arbitrary-precision, recursion is limited only by the interpreter's stack. That is not generosity in the contract, it is an accident of the reference's implementation leaking into it. #259 cp1 measured the consequence: the Rust port refuses documents the reference accepts, across nine coordinate paths and both nesting trees. The honest reading is not "the port is over-strict" — it is that the fact vocabulary was only implementable in a language with bignums and a deep stack. Widening Rust to match (arbitrary precision, unbounded recursion) would spread the accident instead of fixing it. So, per the standing migration rule that a divergence is a Rust bug UNLESS behaviour changes in a separate Python-first PR: this is that PR. spec/OwnIR.md §4.2 the contract, normative ownlang/ownir.py line/column ranges, flow-body depth ownlang/obligations.py event-tree depth, event line range **Coordinates fit a signed 64-bit integer.** Every `line` is in [-2^63, 2^63-1] — services, ctor_line, root_resolve_sites[], scope_cache_sites[], effects, bindings, params, protocol events — and every `column` is 1..=2^63-1, absent, or null. A coordinate nothing downstream can hold is not a usable coordinate. **Flow bodies and event trees nest at most 32 levels.** Measured from both ends rather than picked: the deepest nesting in any OwnIR fixture in this repository is 3 (events: 2), and a parser with the widespread 128-level JSON recursion cap stops accepting these documents at 62, because each `if` costs two JSON levels. 32 is an order of magnitude above what producers emit and about half way to the ceiling consumers can parse. It is stated in the OwnIR domain — nested bodies — not in JSON levels, because nested bodies are what a frontend can reason about. Both are rejections at the strict door. `check_facts()` on un-validated facts keeps degrading to absent: two entry points, two contracts, as with `column` already. Twelve mutations. Ten caught, one invalid (raising the limit to 1000 makes CPython itself hit RecursionError, which is its own argument), and one SURVIVED and had to be fixed: P11 widening INT64_MAX to 2^64-1 was NOT caught. Because the test imported the constants it was testing, so every boundary case moved with them: it could prove the limits were enforced consistently and could not prove they were the right limits. The spec's numbers are now literals in the test. This is the same failure the cp1 ledger had one layer up — a test written in terms of the value under test asserts self-consistency, not correctness — and it is worth having hit it twice, in two places, to see that it is a pattern and not an incident. The off-by-one is also measured rather than reasoned: `_check_flow_columns` probes every op for then/else/body whether or not it has them, so checking depth before the early return counted the absent ones and rejected a body at exactly the limit. Only the at-limit case catches that, which is why every limit here is pinned at three points. No existing fixture changes: nothing in the tree nests past 3 or carries an out-of-range coordinate. Refs #250, #259. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CJF7MBi1ijU5m9cJVWgQsM --- ownlang/obligations.py | 51 +++++- ownlang/ownir.py | 52 +++++- spec/OwnIR.md | 46 +++++ tests/test_ownir_defensive_limits.py | 241 +++++++++++++++++++++++++++ 4 files changed, 381 insertions(+), 9 deletions(-) create mode 100644 tests/test_ownir_defensive_limits.py diff --git a/ownlang/obligations.py b/ownlang/obligations.py index c0880387..747a5e5d 100644 --- a/ownlang/obligations.py +++ b/ownlang/obligations.py @@ -61,6 +61,33 @@ # matcher vocabulary for `opens`/`closes`/`barriers`/`allow`. MATCHER_KINDS = frozenset({"assign", "call"}) +# --------------------------------------------------------------------------- +# defensive limits on externally supplied structure (spec/OwnIR.md §4.2) +# --------------------------------------------------------------------------- + +# The representable range of an OwnIR source-coordinate integer. +# +# Python integers are unbounded; a consumer's are not. Leaving the range open +# means the fact vocabulary is only *implementable* in a language with bignums, +# which is a contract accident rather than a decision — so the bound is stated +# here and enforced, instead of being discovered downstream as a port bug. +INT64_MIN = -(2 ** 63) +INT64_MAX = 2 ** 63 - 1 + +# Maximum nesting of `if`/`while` bodies in a flow body or an event tree. +# +# Chosen by measurement, from both ends: +# * the deepest nesting in any OwnIR fixture in this repository is 3 +# (`tests/fixtures/lowered/hoist_neg_nested_depth.facts.json`); +# * a JSON parser applying the common 128-level recursion cap stops accepting +# these documents at 62 levels, because each `if` costs two JSON levels. +# 32 sits an order of magnitude above anything a producer has emitted and +# roughly half way to the ceiling every consumer can still parse. +# +# The limit is on the OwnIR *domain* — nested bodies — not on JSON nesting, +# because that is the thing a frontend can reason about. +MAX_NESTING_DEPTH = 32 + class ProtocolFactsError(ValueError): """A malformed protocol/event fact. `load()` wraps this in `OwnIRError` @@ -212,6 +239,10 @@ def _opt_line(raw: dict[str, Any], ctx: str) -> int: v = raw.get("line", 0) if not isinstance(v, int) or isinstance(v, bool): raise ProtocolFactsError(f"{ctx}: 'line' must be an integer, got {v!r}") + if not INT64_MIN <= v <= INT64_MAX: + raise ProtocolFactsError( + f"{ctx}: 'line' must fit a signed 64-bit integer, got {v} " + f"(spec/OwnIR.md §4.2)") return v @@ -296,9 +327,18 @@ def parse_protocol(raw: Any) -> Protocol: methods=tuple(methods_raw), description=desc) -def parse_events(raw: Any, ctx: str) -> tuple[Event, ...]: +def parse_events(raw: Any, ctx: str, depth: int = 0) -> tuple[Event, ...]: """Parse an ordered event list (recursive over `if`/`while`), fail-loud on - an unknown `ev` — the same rule as an unknown flow op (OwnIR IR4).""" + an unknown `ev` — the same rule as an unknown flow op (OwnIR IR4). + + `depth` counts enclosing `if`/`while` bodies; the top-level list is 0. The + bound is a defensive limit on external input (spec/OwnIR.md §4.2), not a + reachable property of real code — the deepest event tree in this + repository's fixtures is 2.""" + if depth > MAX_NESTING_DEPTH: + raise ProtocolFactsError( + f"{ctx}: events nested deeper than {MAX_NESTING_DEPTH} levels " + f"(spec/OwnIR.md §4.2)") if not isinstance(raw, list): raise ProtocolFactsError(f"{ctx}: events must be an array, got {raw!r}") out: list[Event] = [] @@ -332,10 +372,11 @@ def parse_events(raw: Any, ctx: str) -> tuple[Event, ...]: out.append(ThrowEv(line=line)) elif ev == "if": out.append(IfEv(line=line, - then=parse_events(e.get("then", []), ctx), - orelse=parse_events(e.get("else", []), ctx))) + then=parse_events(e.get("then", []), ctx, depth + 1), + orelse=parse_events(e.get("else", []), ctx, depth + 1))) else: # "while" — EVENT_KINDS is closed, checked above - out.append(WhileEv(line=line, body=parse_events(e.get("body", []), ctx))) + out.append(WhileEv( + line=line, body=parse_events(e.get("body", []), ctx, depth + 1))) return tuple(out) diff --git a/ownlang/ownir.py b/ownlang/ownir.py index 49d56ebb..02273693 100644 --- a/ownlang/ownir.py +++ b/ownlang/ownir.py @@ -148,6 +148,9 @@ from .effects import find_effect_storms from .evidence import code_flow, di_path_steps from .obligations import ( + INT64_MAX, + INT64_MIN, + MAX_NESTING_DEPTH, MethodEvents, Protocol, ProtocolFactsError, @@ -536,6 +539,21 @@ def build_sarif(findings: list[Finding], severity: str = "error") -> dict[str, A } +def _check_int_range(v: int, where: str, field: str = "line") -> None: + """The representable range of an OwnIR source coordinate (spec/OwnIR.md §4.2). + + Python integers are unbounded; a consumer's are not. Without this bound the + fact vocabulary is only implementable in a language with bignums, which is a + contract accident rather than a decision — and it surfaces downstream as a + port rejecting a document the reference accepted. Stated and enforced here + instead, Python-first. + """ + if not INT64_MIN <= v <= INT64_MAX: + raise OwnIRError( + f"{where} {field!r} must fit a signed 64-bit integer, got {v} " + f"(spec/OwnIR.md §4.2)") + + def _check_column(v: Any, where: str) -> None: """Fail-loud shape check for an optional source `column` (#317). @@ -554,21 +572,38 @@ def _check_column(v: Any, where: str) -> None: if isinstance(v, bool) or not isinstance(v, int) or v < 1: raise OwnIRError( f"{where} 'column' must be a 1-based integer or absent, got {v!r}") + # …and bounded above, for the same reason `line` is (spec/OwnIR.md §4.2): + # a column no consumer can represent is not a usable coordinate. + _check_int_range(v, where, "column") -def _check_flow_columns(nodes: Any, where: str) -> None: - """Validate `column` on every flow op, including inside `if`/`while` bodies. +def _check_flow_columns(nodes: Any, where: str, depth: int = 0) -> None: + """Validate `column` on every flow op, including inside `if`/`while` bodies, + and bound how deeply those bodies may nest. Recursive because a hoisted branch acquire - the path most likely to be - forgotten - lives inside a nested body, not at the top level.""" + forgotten - lives inside a nested body, not at the top level. + + `depth` counts enclosing bodies; the top-level list is 0. The bound is a + defensive limit on external input (spec/OwnIR.md §4.2), not a property real + code reaches - the deepest flow body in this repository's fixtures is 3.""" + # The early return comes FIRST. Every op is probed for `then`/`else`/`body` + # whether or not it has them, so checking depth before this would count the + # absent ones and reject a body at exactly the limit — measured, not + # reasoned: `body nesting 32 (at limit)` was rejected until this order was + # fixed. Only a list that actually exists is a level. if not isinstance(nodes, list): return + if depth > MAX_NESTING_DEPTH: + raise OwnIRError( + f"{where} nested deeper than {MAX_NESTING_DEPTH} levels " + f"(spec/OwnIR.md §4.2)") for n in nodes: if not isinstance(n, dict): continue _check_column(n.get("column"), f"{where} op {n.get('op')!r}") for key in ("then", "else", "body"): - _check_flow_columns(n.get(key), where) + _check_flow_columns(n.get(key), where, depth + 1) def load(path: str) -> dict[str, Any]: @@ -679,12 +714,14 @@ def load(path: str) -> dict[str, Any]: ln = s.get("line", 0) if not isinstance(ln, int) or isinstance(ln, bool): raise OwnIRError("service 'line' must be an integer") + _check_int_range(ln, "service") # the consuming-constructor location (optional, P-006 Q#1) is validated like file/line. if not isinstance(s.get("ctor_file", "?"), str): raise OwnIRError("service 'ctor_file' must be a string") cln = s.get("ctor_line", 0) if not isinstance(cln, int) or isinstance(cln, bool): raise OwnIRError("service 'ctor_line' must be an integer") + _check_int_range(cln, "service", "ctor_line") if not isinstance(s.get("ctor_type", ""), str): raise OwnIRError("service 'ctor_type' must be a string") # DI004 call-site metadata (optional): an array of {type, file, line} objects. @@ -697,6 +734,8 @@ def load(path: str) -> dict[str, Any]: raise OwnIRError( "service 'root_resolve_sites' must be an array of " "{type:str, file:str, line:int} objects") + for site in sites: + _check_int_range(site.get("line", 0), "service root_resolve_site") # DI005 (scope-cached captive): types resolved from a self-created scope and cached # into a field, plus their field-store sites — validated like root_resolves / its sites. scope_cached = s.get("scope_cached", []) @@ -712,6 +751,8 @@ def load(path: str) -> dict[str, Any]: raise OwnIRError( "service 'scope_cache_sites' must be an array of " "{type:str, file:str, line:int} objects") + for site in csites: + _check_int_range(site.get("line", 0), "service scope_cache_site") # Optional reactive-effect graph (EFF001 — effect storm, P-020). Additive and # optional: an older core simply ignores it. Each effect carries its render-scope # binding table; the core (ownlang/effects.py) decides identity stability. @@ -728,6 +769,7 @@ def load(path: str) -> dict[str, Any]: eln = eff.get("line", 0) if not isinstance(eln, int) or isinstance(eln, bool): raise OwnIRError("effect 'line' must be an integer") + _check_int_range(eln, "effect") binds = eff.get("bindings", []) if not isinstance(binds, list) or not all(isinstance(b, dict) for b in binds): raise OwnIRError("effect 'bindings' must be a JSON array of objects") @@ -742,6 +784,7 @@ def load(path: str) -> dict[str, Any]: bln = b.get("line", 0) if not isinstance(bln, int) or isinstance(bln, bool): raise OwnIRError("binding 'line' must be an integer") + _check_int_range(bln, "binding") # Optional per-method flow bodies (P-016 B0b/B2 — local IDisposable # acquire/use/release over a CFG). Additive/optional; an older core ignores it. fns = result.get("functions", []) @@ -777,6 +820,7 @@ def load(path: str) -> dict[str, Any]: if not isinstance(pl, int) or isinstance(pl, bool): raise OwnIRError( f"parameter 'line' must be an integer, got {pl!r}") + _check_int_range(pl, "parameter") _check_column(p.get("column"), "parameter") peff = p.get("effect") if peff is not None and peff not in _PARAM_EFFECTS: diff --git a/spec/OwnIR.md b/spec/OwnIR.md index 774b750d..ae269aa2 100644 --- a/spec/OwnIR.md +++ b/spec/OwnIR.md @@ -172,6 +172,52 @@ when a real `startLine` is present. It is what OwnAudit's `finding-occurrence/v1 physical anchor reads (Own.NET#317, PhysShell/OwnAudit#58); a finding without one is anchored line-only, which is a degradation rather than a failure. +### 4.2 Defensive limits on externally supplied structure (normative) + +`OwnIR` is untrusted input: a file a frontend wrote, that `load()` reads. Two of +its shapes were unbounded, and an unbounded contract is only *implementable* in +a language that happens to have the same capabilities the reference does. Both +are now bounded, and the bound is part of the vocabulary rather than a property +of whichever consumer reads it first. + +**Source-coordinate integers fit a signed 64-bit integer.** + +- every `line` — on services, `ctor_line`, `root_resolve_sites[]`, + `scope_cache_sites[]`, effects, bindings, params, and protocol events — lies + in `[-2^63, 2^63 - 1]`; +- every `column` (§4.1) is `1..=2^63 - 1`, or absent, or `null`. + +Python integers are unbounded, so the reference accepted coordinates no other +consumer could represent. That is not a generosity worth keeping: a coordinate +nothing downstream can hold is not a usable coordinate, and leaving it legal +turns every port into a source of "the reference accepted this and I cannot". +The bound is stated here and enforced in `load()`. + +**Flow bodies and protocol event trees nest at most 32 levels.** + +`functions[].body` and `protocol_functions[].events` nest through `then`, +`else` and `body`. The limit counts those enclosing bodies — the top-level list +is level 0 — so a document nesting exactly 32 is accepted and 33 is rejected. + +32 is chosen from measurement at both ends: + +- the deepest nesting in any `OwnIR` fixture in this repository is **3** + (`tests/fixtures/lowered/hoist_neg_nested_depth.facts.json`); the deepest + event tree is **2**; +- a JSON parser applying the widespread 128-level recursion cap stops accepting + these documents at **62** levels, because each `if` costs two JSON levels + (an object and an array). + +So the limit sits an order of magnitude above anything a producer has emitted +and roughly half way to the ceiling a consumer can still parse. It is expressed +in the `OwnIR` domain — nested bodies — and not in JSON levels, because nested +bodies are the thing a frontend can reason about; the ratio between the two is +an encoding detail. + +Both limits are **rejections at the strict door**, not coercions. `check_facts()` +on un-validated facts keeps its existing degrade-to-absent behaviour: two entry +points, two contracts, as with `column` in §4.1. + ## 5. Flow bodies (`functions[]`) A flow function has a `name`, a `file`, and a `body`: an ordered list of flow diff --git a/tests/test_ownir_defensive_limits.py b/tests/test_ownir_defensive_limits.py new file mode 100644 index 00000000..ef98a854 --- /dev/null +++ b/tests/test_ownir_defensive_limits.py @@ -0,0 +1,241 @@ +#!/usr/bin/env python3 +"""The two defensive limits on externally supplied OwnIR (spec/OwnIR.md §4.2). + +`OwnIR` is a file some frontend wrote. Two of its shapes had no bound at all, +and the reference accepted both because *Python* has no bound: integers are +arbitrary-precision, and recursion is limited only by the interpreter's stack. + +That is not generosity, it is an accident of the reference's implementation +leaking into the contract. It surfaced as a measured Python-accept/Rust-reject +pair in #259 cp1 — the Rust port refusing documents the reference took — and the +honest reading of that is not "the port is over-strict". It is that the +vocabulary was only implementable in a language with bignums and a deep stack. + +So the fix is Python-first, which is the migration's standing rule: a +Rust/Python divergence is a Rust bug **unless behaviour changes in a separate +Python-first PR**. This is that PR. + +## What is asserted here, and why the boundary and not just the failure + +Each limit is pinned at three points: below, exactly at, and one past. A test +that only checks "something far too big is rejected" cannot tell a correct limit +from one that is off by one — and the nesting limit *was* off by one when first +written, because `_check_flow_columns` probes every op for `then`/`else`/`body` +whether or not it has them, so the absent ones were counting as levels. At-limit +rejected. Only the boundary case could catch that. + +## Why 32 + +Measured from both ends rather than picked: + +* the deepest nesting in any `OwnIR` fixture in this repository is 3; +* a JSON parser with the widespread 128-level recursion cap stops accepting + these documents at 62 levels, since each `if` costs two JSON levels. + +Run: python tests/test_ownir_defensive_limits.py + python tests/run_tests.py (in the suite) +""" + +from __future__ import annotations + +import json +import os +import sys +import tempfile +from typing import Any + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) + +from ownlang.obligations import INT64_MAX, INT64_MIN, MAX_NESTING_DEPTH +from ownlang.ownir import OwnIRError, load + + +def _load(document: Any) -> str | None: + """Run the strict door; return None on accept, the message on reject.""" + directory = tempfile.mkdtemp() + path = os.path.join(directory, "facts.ownir.json") + try: + with open(path, "w", encoding="utf-8") as f: + json.dump(document, f) + try: + load(path) + return None + except OwnIRError as e: + return str(e) + finally: + if os.path.exists(path): + os.unlink(path) + os.rmdir(directory) + + +def _svc(**kw: Any) -> dict[str, Any]: + base: dict[str, Any] = {"name": "S", "lifetime": "singleton"} + base.update(kw) + return base + + +def _body(depth: int, key: str = "then") -> dict[str, Any]: + """A function flow body nested `depth` levels through `key`.""" + node: dict[str, Any] = {"op": "acquire", "column": 1} + for _ in range(depth): + node = {"op": "if" if key != "body" else "while", key: [node]} + return {"ownir_version": 0, "functions": [{"body": [node]}]} + + +def _events(depth: int, key: str = "then") -> dict[str, Any]: + """A protocol event tree nested `depth` levels through `key`.""" + node: dict[str, Any] = {"ev": "return", "line": 1} + for _ in range(depth): + node = {"ev": "if" if key != "body" else "while", "line": 1, key: [node]} + return {"ownir_version": 0, + "protocol_functions": [{"name": "M", "events": [node]}]} + + +# Every path a source coordinate travels. Named as (label, builder) so a new +# coordinate field that skips the check shows up as a missing row rather than +# as nothing at all. +LINE_PATHS: list[tuple[str, Any]] = [ + ("services[].line", lambda v: {"services": [_svc(line=v)]}), + ("services[].ctor_line", lambda v: {"services": [_svc(ctor_line=v)]}), + ("services[].root_resolve_sites[].line", + lambda v: {"services": [_svc(root_resolve_sites=[{"line": v}])]}), + ("services[].scope_cache_sites[].line", + lambda v: {"services": [_svc(scope_cache_sites=[{"line": v}])]}), + ("effects[].line", lambda v: {"effects": [{"line": v}]}), + ("effects[].bindings[].line", + lambda v: {"effects": [{"bindings": [{"line": v}]}]}), + ("functions[].params[].line", + lambda v: {"functions": [{"params": [{"name": "p", "line": v}]}]}), + ("protocol_functions[].events[].line", + lambda v: {"protocol_functions": [ + {"name": "M", "events": [{"ev": "return", "line": v}]}]}), +] + +COLUMN_PATHS: list[tuple[str, Any]] = [ + ("subscriptions[].column", + lambda v: {"components": [{"subscriptions": [{"column": v}]}]}), + ("functions[].params[].column", + lambda v: {"functions": [{"params": [{"name": "p", "column": v}]}]}), + ("functions[].body[].column", + lambda v: {"functions": [{"body": [{"op": "a", "column": v}]}]}), +] + + +# The spec's numbers, written as LITERALS. +# +# The rest of this file imports the constants from the module, which means it +# moves whenever they do — so on its own it can prove the limits are enforced +# consistently and cannot prove they are the *right* limits. Measured: widening +# `INT64_MAX` to `2**64 - 1` SURVIVED mutation until these three lines existed, +# because every boundary case simply followed the constant. +# +# A parity ledger caught the same class of error one layer up in #259 cp1. It is +# worth stating plainly: a test written in terms of the value under test asserts +# self-consistency, not correctness. +SPEC_INT64_MIN = -9223372036854775808 +SPEC_INT64_MAX = 9223372036854775807 +SPEC_MAX_NESTING_DEPTH = 32 + + +def _fail(message: str) -> int: + print(f"FAIL: {message}") + return 1 + + +def run() -> int: + failures = 0 + + # The constants match spec/OwnIR.md §4.2, checked against literals so that + # changing a limit fails here rather than silently redefining every + # boundary case below. + for name, actual, expected in ( + ("INT64_MIN", INT64_MIN, SPEC_INT64_MIN), + ("INT64_MAX", INT64_MAX, SPEC_INT64_MAX), + ("MAX_NESTING_DEPTH", MAX_NESTING_DEPTH, SPEC_MAX_NESTING_DEPTH), + ): + if actual != expected: + failures += _fail( + f"{name} is {actual}, spec/OwnIR.md §4.2 says {expected}. " + f"Changing a defensive limit is a contract change: update the " + f"spec, this literal, and the Rust side together") + + # ---- line: the full signed-64 range, and one step outside each end ----- + for label, build in LINE_PATHS: + for value, expect_reject in ((INT64_MIN, False), (0, False), + (INT64_MAX, False), + (INT64_MIN - 1, True), (INT64_MAX + 1, True)): + err = _load(build(value)) + if expect_reject and err is None: + failures += _fail(f"{label}: {value} accepted, expected reject") + elif not expect_reject and err is not None: + failures += _fail(f"{label}: {value} rejected — {err}") + elif expect_reject and "signed 64-bit" not in (err or ""): + failures += _fail( + f"{label}: {value} rejected for the wrong reason — {err}") + + # ---- column: 1-based below, and the same upper bound ------------------ + for label, build in COLUMN_PATHS: + for value, expect_reject in ((1, False), (INT64_MAX, False), + (INT64_MAX + 1, True)): + err = _load(build(value)) + if expect_reject and err is None: + failures += _fail(f"{label}: {value} accepted, expected reject") + elif not expect_reject and err is not None: + failures += _fail(f"{label}: {value} rejected — {err}") + # The 1-based rule still fires first for a low column, so the new upper + # bound cannot have replaced it. + if "1-based" not in (_load(build(0)) or ""): + failures += _fail(f"{label}: 0 no longer reports the 1-based rule") + + # ---- nesting: below, exactly at, one past — for both trees and both + # recursive keys, because `then`/`else`/`body` are three separate call + # sites and a limit added to one of them would pass a `then`-only test. + for label, build in (("functions[].body", _body), + ("protocol_functions[].events", _events)): + for key in ("then", "else", "body"): + for depth, expect_reject in ((MAX_NESTING_DEPTH - 1, False), + (MAX_NESTING_DEPTH, False), + (MAX_NESTING_DEPTH + 1, True)): + err = _load(build(depth, key)) + if expect_reject and err is None: + failures += _fail( + f"{label} via {key!r}: depth {depth} accepted, " + f"expected reject (the limit is {MAX_NESTING_DEPTH})") + elif not expect_reject and err is not None: + failures += _fail( + f"{label} via {key!r}: depth {depth} rejected — {err}") + elif expect_reject and "nested deeper" not in (err or ""): + failures += _fail( + f"{label} via {key!r}: depth {depth} rejected for the " + f"wrong reason — {err}") + + # ---- the tolerances the limits must NOT have tightened ---------------- + # A non-list body is skipped, not rejected; the reference returns early. + # Bounding depth is the kind of change that quietly turns that into a + # rejection, so it is asserted rather than assumed. + for label, document in ( + ("non-list body", {"ownir_version": 0, "functions": [{"body": 7}]}), + ("body of scalars", + {"ownir_version": 0, "functions": [{"body": [1, "x", None]}]}), + ("absent sections", {"ownir_version": 0}), + ("null column", + {"ownir_version": 0, + "components": [{"subscriptions": [{"column": None}]}]}), + ): + err = _load(document) + if err is not None: + failures += _fail(f"{label} must still be accepted — {err}") + + if failures: + return 1 + print( + f"ownir defensive limits OK: coordinates in " + f"[{INT64_MIN}, {INT64_MAX}] over {len(LINE_PATHS)} line paths and " + f"{len(COLUMN_PATHS)} column paths; nesting <= {MAX_NESTING_DEPTH} " + f"over 2 trees x 3 keys, each pinned at limit and limit+1" + ) + return 0 + + +if __name__ == "__main__": + raise SystemExit(run()) From 9f909c705ba4153b3d3bc5ca70a607e4b140e9c9 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 20:14:47 +0000 Subject: [PATCH 2/4] =?UTF-8?q?fix(ownir):=20put=20the=20limits=20in=20the?= =?UTF-8?q?=20JSON=20schema,=20and=20stop=20overclaiming=20in=20=C2=A74.2?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two review findings on bc8790c, both real, one with a wrong consequence attached. **The schema had to carry the bounds too.** `spec/ownir.schema.json` is what a NON-Python consumer validates against. With the bounds only in `load()`, a producer could be schema-valid and still refused at the door — the same cross-consumer mismatch this change exists to remove, one layer further out. Added `$defs.sourceLine` (signed-64) and a `maximum` on `sourceColumn`, and rebound all 21 inline `"line": {"type": "integer"}` fields to it. The test now asserts the schema's four numbers against the same literals as the code, so the two cannot drift. Mutation-proved both ways: widening `sourceLine.maximum` to `2^64-1` and dropping `sourceColumn.maximum` are each caught. **§4.2 said "every `line`" and two line-bearing fields escaped it.** The finding is right that the sentence overclaimed. Its stated consequence — that this "preserves the Python/port divergence" — is not: measured, both implementations accept those fields, because neither types them. python subscription line 2^80 : ACCEPT rust: ACCEPT python subscription line "x" : ACCEPT rust: ACCEPT python flow op line 2^80 : ACCEPT rust: ACCEPT python flow op line "x" : ACCEPT rust: ACCEPT `components[].subscriptions[].line` and the `line` on a flow op inside `functions[].body` are checked NOWHERE by `load()` — not for range, and not even for type. #325's validator has no check for them either (grep `"line"` in strict.rs: services, effects, bindings, params, sites — not these two). So this is not a parity gap, and closing it is not part of removing one. It is a separate contract question: whether a coordinate no rule reads should nevertheless have to be well-formed. Extending the check would be a new restriction on documents accepted today, arriving inside a PR whose job is to close a measured divergence — so §4.2 now enumerates exactly the fields it enforces, marks the word "validated" as load-bearing, and records the two exceptions with the measurement instead of quietly widening or quietly overclaiming. Corpus scan for the record: 150 JSON files, zero offenders on either path, so extending it later would break nothing in the tree. Refs #250, #259. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CJF7MBi1ijU5m9cJVWgQsM --- spec/OwnIR.md | 18 ++++++++-- spec/ownir.schema.json | 51 ++++++++++++++++------------ tests/test_ownir_defensive_limits.py | 22 ++++++++++++ 3 files changed, 66 insertions(+), 25 deletions(-) diff --git a/spec/OwnIR.md b/spec/OwnIR.md index ae269aa2..3939f1d1 100644 --- a/spec/OwnIR.md +++ b/spec/OwnIR.md @@ -182,11 +182,23 @@ of whichever consumer reads it first. **Source-coordinate integers fit a signed 64-bit integer.** -- every `line` — on services, `ctor_line`, `root_resolve_sites[]`, - `scope_cache_sites[]`, effects, bindings, params, and protocol events — lies - in `[-2^63, 2^63 - 1]`; +- every **validated** `line` — `services[].line`, `services[].ctor_line`, + `services[].root_resolve_sites[].line`, `services[].scope_cache_sites[].line`, + `effects[].line`, `effects[].bindings[].line`, `functions[].params[].line`, + `protocol_functions[].events[].line` — lies in `[-2^63, 2^63 - 1]`; - every `column` (§4.1) is `1..=2^63 - 1`, or absent, or `null`. +The word *validated* is load-bearing, and the exception is recorded rather than +papered over. Two line-bearing fields are checked **nowhere** by `load()` — not +for range, and not even for type: `components[].subscriptions[].line` and the +`line` on a flow op inside `functions[].body`. Measured, `{"line": "x"}` and +`{"line": true}` are accepted on both. That predates this section, and both +implementations agree about it — neither the reference nor the Rust port types +those fields — so it is **not** a parity gap and closing it is not part of +removing one. It is a separate contract question: whether a coordinate that no +rule reads should nevertheless be well-formed. Until it is answered, the bound +above claims exactly the fields it covers. + Python integers are unbounded, so the reference accepted coordinates no other consumer could represent. That is not a generosity worth keeping: a coordinate nothing downstream can hold is not a usable coordinate, and leaving it legal diff --git a/spec/ownir.schema.json b/spec/ownir.schema.json index d2a21ce9..c603da2e 100644 --- a/spec/ownir.schema.json +++ b/spec/ownir.schema.json @@ -49,7 +49,14 @@ "sourceColumn": { "description": "1-based source column of the SAME node `line` anchors on. Optional and additive, so OWNIR_VERSION stays 0 (spec/OwnIR.md §2): a producer that does not report one omits it and no consumer substitutes a value — not 0, not 1, and never recovered by re-reading the source line. Rides to SARIF as region.startColumn, where the OwnAudit occurrence anchor reads it (Own.NET#317).", "type": "integer", - "minimum": 1 + "minimum": 1, + "maximum": 9223372036854775807 + }, + "sourceLine": { + "description": "A source line. Bounded to a signed 64-bit integer (spec/OwnIR.md \u00a74.2): Python integers are unbounded and the reference used to accept coordinates no other consumer could represent, which made the vocabulary implementable only in a language with bignums. A coordinate nothing downstream can hold is not a usable coordinate. Pinned to ownlang/obligations.py::INT64_MIN/INT64_MAX.", + "type": "integer", + "minimum": -9223372036854775808, + "maximum": 9223372036854775807 }, "resourceKind": { "description": "The resource-kind discriminator (spec/OwnIR.md §4). It selects the analysis path, so a present-but-unknown value is rejected at load (fail-loud) and a new kind must bump OWNIR_VERSION. Pinned to ownlang/ownir.py::_KNOWN_RESOURCE_KINDS.", @@ -82,7 +89,7 @@ "properties": { "type": { "type": "string" }, "file": { "type": "string" }, - "line": { "type": "integer" } + "line": { "$ref": "#/$defs/sourceLine" } } }, "component": { @@ -101,7 +108,7 @@ "description": "One owned-resource record (spec/OwnIR.md §4). An unreleased record is OWN001 at `line`; a released one nets balanced and stays silent.", "type": "object", "properties": { - "line": { "type": "integer" }, + "line": { "$ref": "#/$defs/sourceLine" }, "column": { "$ref": "#/$defs/sourceColumn" }, "event": { "description": "The event/handle identifier for this owned resource (e.g. `bus.CustomerChanged`, `_timer.Tick`); carried into the finding message and rendered output.", @@ -168,7 +175,7 @@ "required": ["name"], "properties": { "name": { "type": "string" }, - "line": { "type": "integer" }, + "line": { "$ref": "#/$defs/sourceLine" }, "column": { "$ref": "#/$defs/sourceColumn" }, "effect": { "$ref": "#/$defs/paramEffect" } } @@ -183,7 +190,7 @@ "description": "A new owned local (Let+Acquire); kind:\"pool\" tags it a pooled buffer.", "properties": { "op": { "const": "acquire" }, - "line": { "type": "integer" }, + "line": { "$ref": "#/$defs/sourceLine" }, "column": { "$ref": "#/$defs/sourceColumn" }, "var": { "type": "string" }, "kind": { "type": "string", "enum": ["pool"] } @@ -195,7 +202,7 @@ "description": "Release of the local's handle.", "properties": { "op": { "const": "release" }, - "line": { "type": "integer" }, + "line": { "$ref": "#/$defs/sourceLine" }, "column": { "$ref": "#/$defs/sourceColumn" }, "var": { "type": "string" } }, @@ -206,7 +213,7 @@ "description": "Use of the handle.", "properties": { "op": { "const": "use" }, - "line": { "type": "integer" }, + "line": { "$ref": "#/$defs/sourceLine" }, "column": { "$ref": "#/$defs/sourceColumn" }, "var": { "type": "string" } }, @@ -217,7 +224,7 @@ "description": "Overspan (POOL005: a full-length view of a pooled buffer).", "properties": { "op": { "const": "overspan" }, - "line": { "type": "integer" }, + "line": { "$ref": "#/$defs/sourceLine" }, "column": { "$ref": "#/$defs/sourceColumn" }, "var": { "type": "string" } }, @@ -228,7 +235,7 @@ "description": "Ownership transfer out; `var` optional (a bare return).", "properties": { "op": { "const": "return" }, - "line": { "type": "integer" }, + "line": { "$ref": "#/$defs/sourceLine" }, "column": { "$ref": "#/$defs/sourceColumn" }, "var": { "type": "string" } }, @@ -239,7 +246,7 @@ "description": "A new owning handle joined to `src`'s alias set (wrap/adopt, D5.4).", "properties": { "op": { "const": "alias_join" }, - "line": { "type": "integer" }, + "line": { "$ref": "#/$defs/sourceLine" }, "column": { "$ref": "#/$defs/sourceColumn" }, "var": { "type": "string" }, "src": { "type": "string" } @@ -251,7 +258,7 @@ "description": "A Call checked against the callee's contract; a fresh-returning callee mints an acquire for `result` (D5.2). An optional `sig` (the callee's canonical parameter-type list, same format as a function record's `sig`) resolves the call against that overload's own summary; absent or unmatched, the name-merged summary applies (stage 2 fallback — degraded, never a wrong overload).", "properties": { "op": { "const": "call" }, - "line": { "type": "integer" }, + "line": { "$ref": "#/$defs/sourceLine" }, "column": { "$ref": "#/$defs/sourceColumn" }, "callee": { "type": "string" }, "sig": { "type": "string" }, @@ -265,7 +272,7 @@ "description": "An If with both branches lowered.", "properties": { "op": { "const": "if" }, - "line": { "type": "integer" }, + "line": { "$ref": "#/$defs/sourceLine" }, "column": { "$ref": "#/$defs/sourceColumn" }, "then": { "type": "array", "items": { "$ref": "#/$defs/flowOp" } }, "else": { "type": "array", "items": { "$ref": "#/$defs/flowOp" } } @@ -277,7 +284,7 @@ "description": "A While — a back-edge the core's worklist fixpoint converges over (A1).", "properties": { "op": { "const": "while" }, - "line": { "type": "integer" }, + "line": { "$ref": "#/$defs/sourceLine" }, "column": { "$ref": "#/$defs/sourceColumn" }, "body": { "type": "array", "items": { "$ref": "#/$defs/flowOp" } } }, @@ -295,7 +302,7 @@ "weak_deps": { "type": "array", "items": { "type": "string" } }, "root_resolves": { "type": "array", "items": { "type": "string" } }, "file": { "type": "string" }, - "line": { "type": "integer" }, + "line": { "$ref": "#/$defs/sourceLine" }, "ctor_file": { "type": "string" }, "ctor_line": { "type": "integer" }, "ctor_type": { "type": "string" }, @@ -311,7 +318,7 @@ "description": "Whether the effect performs I/O (default false).", "type": "boolean" }, - "line": { "type": "integer" }, + "line": { "$ref": "#/$defs/sourceLine" }, "deps": { "description": "The effect's dependency-array names.", "type": "array", @@ -333,7 +340,7 @@ "type": "string" }, "refs": { "type": "array", "items": { "type": "string" } }, - "line": { "type": "integer" } + "line": { "$ref": "#/$defs/sourceLine" } } }, "protocolMatcher": { @@ -420,7 +427,7 @@ "ev": { "const": "assign" }, "target": { "type": "string", "minLength": 1 }, "value": { "type": ["boolean", "null"] }, - "line": { "type": "integer" } + "line": { "$ref": "#/$defs/sourceLine" } }, "required": ["ev", "target"] }, @@ -431,7 +438,7 @@ "ev": { "const": "call" }, "callee": { "type": "string", "minLength": 1 }, "arg": { "type": ["string", "null"] }, - "line": { "type": "integer" } + "line": { "$ref": "#/$defs/sourceLine" } }, "required": ["ev", "callee"] }, @@ -440,7 +447,7 @@ "description": "A normal method exit.", "properties": { "ev": { "const": "return" }, - "line": { "type": "integer" } + "line": { "$ref": "#/$defs/sourceLine" } }, "required": ["ev"] }, @@ -449,7 +456,7 @@ "description": "An exceptional method exit. Frontends thread finally bodies onto exits, like the flow lowering (§5).", "properties": { "ev": { "const": "throw" }, - "line": { "type": "integer" } + "line": { "$ref": "#/$defs/sourceLine" } }, "required": ["ev"] }, @@ -458,7 +465,7 @@ "description": "A branch with both arms lowered.", "properties": { "ev": { "const": "if" }, - "line": { "type": "integer" }, + "line": { "$ref": "#/$defs/sourceLine" }, "then": { "type": "array", "items": { "$ref": "#/$defs/protocolEvent" } }, "else": { "type": "array", "items": { "$ref": "#/$defs/protocolEvent" } } }, @@ -469,7 +476,7 @@ "description": "A loop; the checker solves the body to a local fixpoint.", "properties": { "ev": { "const": "while" }, - "line": { "type": "integer" }, + "line": { "$ref": "#/$defs/sourceLine" }, "body": { "type": "array", "items": { "$ref": "#/$defs/protocolEvent" } } }, "required": ["ev"] diff --git a/tests/test_ownir_defensive_limits.py b/tests/test_ownir_defensive_limits.py index ef98a854..f7fb1cd1 100644 --- a/tests/test_ownir_defensive_limits.py +++ b/tests/test_ownir_defensive_limits.py @@ -159,6 +159,28 @@ def run() -> int: f"Changing a defensive limit is a contract change: update the " f"spec, this literal, and the Rust side together") + # ---- the JSON schema carries the same numbers ------------------------- + # `spec/ownir.schema.json` is the contract a NON-Python consumer validates + # against. If the bounds live only in `load()`, a producer can be schema- + # valid and still refused at the door — which is the same cross-consumer + # mismatch this change exists to remove, one layer out. + schema_path = os.path.join( + os.path.dirname(__file__), "..", "spec", "ownir.schema.json") + with open(schema_path, encoding="utf-8") as f: + defs = json.load(f)["$defs"] + for name, key, expected in ( + ("sourceLine", "minimum", SPEC_INT64_MIN), + ("sourceLine", "maximum", SPEC_INT64_MAX), + ("sourceColumn", "minimum", 1), + ("sourceColumn", "maximum", SPEC_INT64_MAX), + ): + actual = defs.get(name, {}).get(key) + if actual != expected: + failures += _fail( + f"spec/ownir.schema.json $defs.{name}.{key} is {actual!r}, " + f"expected {expected} — the schema and `load()` must state the " + f"same bound or a producer can satisfy one and fail the other") + # ---- line: the full signed-64 range, and one step outside each end ----- for label, build in LINE_PATHS: for value, expect_reject in ((INT64_MIN, False), (0, False), From 4125978d405ebfed07c2d9aaf7977c0c24cf201d Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 20:23:23 +0000 Subject: [PATCH 3/4] fix(ownir): bind sourceLine to exactly the paths load() checks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three review findings on 9f909c7. All three real, and the middle one is mine — the schema edit was a blanket search-and-replace and it overshot. 1. `sourceColumn` refused `null`. `load()` accepts `column: null` (`_check_column` returns early on None) and §4.2 says so in the same commit, so the schema contradicted both. Now `["integer", "null"]`; `minimum`/`maximum` only constrain numbers, so real values keep their bounds. Pre-existing, but §4.2 made it visible. 2. `resourceRecord.line` and every `flowOp.line` were bound to `sourceLine` — the two paths §4.2 documents, in this same PR, as validated NOWHERE by `load()`. Binding them makes a producer schema-invalid on documents the door accepts, which is the same cross-consumer mismatch this change exists to remove, pointing the other way. Ten sites reverted to unrestricted integers. 3. `services[].ctor_line` was NOT bound, and `load()` does check it, so a schema-valid document could fail at the door. The blanket replace matched `"line"` and this key is `ctor_line`. Now bound. Both directions are defects, and this file shipped one of each. So the test no longer spot-checks four numbers — it asserts the whole binding map: bound service.line, service.ctor_line, site.line, effect.line, binding.line, param.line, protocolEvent.line unbound resourceRecord.line, flowOp.line with the reason attached to each side: bound because `load()` checks it, unbound because §4.2 records that it does not. A stale entry fails too — a def that no longer has the key stops being evidence. Three mutations, all caught: binding `flowOp.line`, unbinding `ctor_line`, and reverting `sourceColumn` to integer-only. Also: the first attempt at this fix round-tripped the schema through `json.dumps` and produced a 604-line reformat for a 12-line change. Redone as text edits. A diff nobody can read is not a small change however few things it means to say. Refs #250, #259. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CJF7MBi1ijU5m9cJVWgQsM --- spec/ownir.schema.json | 24 ++++++------- tests/test_ownir_defensive_limits.py | 54 +++++++++++++++++++++++++++- 2 files changed, 65 insertions(+), 13 deletions(-) diff --git a/spec/ownir.schema.json b/spec/ownir.schema.json index c603da2e..90903413 100644 --- a/spec/ownir.schema.json +++ b/spec/ownir.schema.json @@ -48,7 +48,7 @@ "$defs": { "sourceColumn": { "description": "1-based source column of the SAME node `line` anchors on. Optional and additive, so OWNIR_VERSION stays 0 (spec/OwnIR.md §2): a producer that does not report one omits it and no consumer substitutes a value — not 0, not 1, and never recovered by re-reading the source line. Rides to SARIF as region.startColumn, where the OwnAudit occurrence anchor reads it (Own.NET#317).", - "type": "integer", + "type": ["integer", "null"], "minimum": 1, "maximum": 9223372036854775807 }, @@ -108,7 +108,7 @@ "description": "One owned-resource record (spec/OwnIR.md §4). An unreleased record is OWN001 at `line`; a released one nets balanced and stays silent.", "type": "object", "properties": { - "line": { "$ref": "#/$defs/sourceLine" }, + "line": { "type": "integer" }, "column": { "$ref": "#/$defs/sourceColumn" }, "event": { "description": "The event/handle identifier for this owned resource (e.g. `bus.CustomerChanged`, `_timer.Tick`); carried into the finding message and rendered output.", @@ -190,7 +190,7 @@ "description": "A new owned local (Let+Acquire); kind:\"pool\" tags it a pooled buffer.", "properties": { "op": { "const": "acquire" }, - "line": { "$ref": "#/$defs/sourceLine" }, + "line": { "type": "integer" }, "column": { "$ref": "#/$defs/sourceColumn" }, "var": { "type": "string" }, "kind": { "type": "string", "enum": ["pool"] } @@ -202,7 +202,7 @@ "description": "Release of the local's handle.", "properties": { "op": { "const": "release" }, - "line": { "$ref": "#/$defs/sourceLine" }, + "line": { "type": "integer" }, "column": { "$ref": "#/$defs/sourceColumn" }, "var": { "type": "string" } }, @@ -213,7 +213,7 @@ "description": "Use of the handle.", "properties": { "op": { "const": "use" }, - "line": { "$ref": "#/$defs/sourceLine" }, + "line": { "type": "integer" }, "column": { "$ref": "#/$defs/sourceColumn" }, "var": { "type": "string" } }, @@ -224,7 +224,7 @@ "description": "Overspan (POOL005: a full-length view of a pooled buffer).", "properties": { "op": { "const": "overspan" }, - "line": { "$ref": "#/$defs/sourceLine" }, + "line": { "type": "integer" }, "column": { "$ref": "#/$defs/sourceColumn" }, "var": { "type": "string" } }, @@ -235,7 +235,7 @@ "description": "Ownership transfer out; `var` optional (a bare return).", "properties": { "op": { "const": "return" }, - "line": { "$ref": "#/$defs/sourceLine" }, + "line": { "type": "integer" }, "column": { "$ref": "#/$defs/sourceColumn" }, "var": { "type": "string" } }, @@ -246,7 +246,7 @@ "description": "A new owning handle joined to `src`'s alias set (wrap/adopt, D5.4).", "properties": { "op": { "const": "alias_join" }, - "line": { "$ref": "#/$defs/sourceLine" }, + "line": { "type": "integer" }, "column": { "$ref": "#/$defs/sourceColumn" }, "var": { "type": "string" }, "src": { "type": "string" } @@ -258,7 +258,7 @@ "description": "A Call checked against the callee's contract; a fresh-returning callee mints an acquire for `result` (D5.2). An optional `sig` (the callee's canonical parameter-type list, same format as a function record's `sig`) resolves the call against that overload's own summary; absent or unmatched, the name-merged summary applies (stage 2 fallback — degraded, never a wrong overload).", "properties": { "op": { "const": "call" }, - "line": { "$ref": "#/$defs/sourceLine" }, + "line": { "type": "integer" }, "column": { "$ref": "#/$defs/sourceColumn" }, "callee": { "type": "string" }, "sig": { "type": "string" }, @@ -272,7 +272,7 @@ "description": "An If with both branches lowered.", "properties": { "op": { "const": "if" }, - "line": { "$ref": "#/$defs/sourceLine" }, + "line": { "type": "integer" }, "column": { "$ref": "#/$defs/sourceColumn" }, "then": { "type": "array", "items": { "$ref": "#/$defs/flowOp" } }, "else": { "type": "array", "items": { "$ref": "#/$defs/flowOp" } } @@ -284,7 +284,7 @@ "description": "A While — a back-edge the core's worklist fixpoint converges over (A1).", "properties": { "op": { "const": "while" }, - "line": { "$ref": "#/$defs/sourceLine" }, + "line": { "type": "integer" }, "column": { "$ref": "#/$defs/sourceColumn" }, "body": { "type": "array", "items": { "$ref": "#/$defs/flowOp" } } }, @@ -304,7 +304,7 @@ "file": { "type": "string" }, "line": { "$ref": "#/$defs/sourceLine" }, "ctor_file": { "type": "string" }, - "ctor_line": { "type": "integer" }, + "ctor_line": { "$ref": "#/$defs/sourceLine" }, "ctor_type": { "type": "string" }, "root_resolve_sites": { "type": "array", "items": { "$ref": "#/$defs/site" } }, "scope_cached": { "type": "array", "items": { "type": "string" } }, diff --git a/tests/test_ownir_defensive_limits.py b/tests/test_ownir_defensive_limits.py index f7fb1cd1..e65fe7ee 100644 --- a/tests/test_ownir_defensive_limits.py +++ b/tests/test_ownir_defensive_limits.py @@ -173,14 +173,66 @@ def run() -> int: ("sourceLine", "maximum", SPEC_INT64_MAX), ("sourceColumn", "minimum", 1), ("sourceColumn", "maximum", SPEC_INT64_MAX), + # `column: null` is accepted by `load()`, so the schema must permit it. + # `minimum`/`maximum` only constrain numbers, so the bounds above still + # apply to real values. + ("sourceColumn", "type", ["integer", "null"]), ): actual = defs.get(name, {}).get(key) if actual != expected: failures += _fail( f"spec/ownir.schema.json $defs.{name}.{key} is {actual!r}, " - f"expected {expected} — the schema and `load()` must state the " + f"expected {expected!r} — the schema and `load()` must state the " f"same bound or a producer can satisfy one and fail the other") + # …and the schema binds `sourceLine` on EXACTLY the paths `load()` checks. + # + # Both directions are defects and both were shipped in the first attempt at + # this file. Binding a path the loader ignores makes a producer schema- + # invalid while the door accepts it; leaving a checked path unbound makes it + # schema-valid while the door refuses it. A blanket search-and-replace over + # `"line"` did the first to `resourceRecord` and `flowOp` — the two paths + # §4.2 documents as unvalidated — and the second to `ctor_line`, whose key + # simply differs. + # + # So the map is asserted as a map, not spot-checked. + BOUND = {"service": ["line", "ctor_line"], "site": ["line"], + "effect": ["line"], "binding": ["line"], "param": ["line"], + "protocolEvent": ["line"]} + UNBOUND = {"resourceRecord": ["line"], "flowOp": ["line"]} + + def _line_refs(node: Any, key: str, out: list[Any]) -> None: + if isinstance(node, dict): + for k, v in node.items(): + if k == key and isinstance(v, dict): + out.append(v.get("$ref") or v.get("type")) + else: + _line_refs(v, key, out) + elif isinstance(node, list): + for v in node: + _line_refs(v, key, out) + + for group, expect_bound in ((BOUND, True), (UNBOUND, False)): + for def_name, keys in group.items(): + for key in keys: + found: list[Any] = [] + _line_refs(defs.get(def_name, {}), key, found) + if not found: + failures += _fail( + f"$defs.{def_name} has no {key!r} — the binding map is " + f"stale, which means it is no longer evidence") + for ref in found: + is_bound = ref == "#/$defs/sourceLine" + if is_bound != expect_bound: + want = ("$ref sourceLine" if expect_bound + else "an unrestricted integer") + why = ("`load()` checks this path" + if expect_bound else + "`load()` does NOT check this path (§4.2)") + failures += _fail( + f"$defs.{def_name}.{key} is {ref!r}, expected " + f"{want} — {why}") + # ---- line: the full signed-64 range, and one step outside each end ----- for label, build in LINE_PATHS: for value, expect_reject in ((INT64_MIN, False), (0, False), From c6e3699ce620ef38d7468927b7f683ac0112972f Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 20:54:48 +0000 Subject: [PATCH 4/4] test(ownir): assert the whole subschema, not just its $ref MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The binding map added in 4125978 checked `ref != "#/$defs/sourceLine"` for the unbound paths, which is not the property it claims. Measured, both of these SURVIVED: flowOp.line -> $ref sourceColumn SURVIVED flowOp.line -> {"type":"integer","maximum":1000} SURVIVED Either one makes an unbound path narrower than `load()`, which is the mismatch the map exists to prevent — the assertion just could not see it, because it only knew one way of being narrow. `_line_refs` collapsed each property to `$ref or type` and threw the rest away. It now keeps the subschema, and the unbound side requires a plain `{"type": "integer"}` with none of `$ref`, `minimum`, `maximum`, `exclusiveMinimum`, `exclusiveMaximum`, `enum`, `const`, `multipleOf`. Four mutations, all caught: the two above, `line` retyped to string, and `service.line` losing its binding (the other direction, kept as a regression). Third time this PR that an assertion has been too loose to distinguish the mutation it was written for — the constants, then the binding direction, now the binding shape. Each was found by mutating rather than by reading, which is the only reason any of them are in the diff. Refs #250, #259. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CJF7MBi1ijU5m9cJVWgQsM --- tests/test_ownir_defensive_limits.py | 44 ++++++++++++++++++---------- 1 file changed, 29 insertions(+), 15 deletions(-) diff --git a/tests/test_ownir_defensive_limits.py b/tests/test_ownir_defensive_limits.py index e65fe7ee..6d76fbd4 100644 --- a/tests/test_ownir_defensive_limits.py +++ b/tests/test_ownir_defensive_limits.py @@ -201,37 +201,51 @@ def run() -> int: "protocolEvent": ["line"]} UNBOUND = {"resourceRecord": ["line"], "flowOp": ["line"]} - def _line_refs(node: Any, key: str, out: list[Any]) -> None: + # Whole subschemas, not just their `$ref`. Asserting only "the ref is not + # sourceLine" left an unbound path free to be tightened another way — + # measured: pointing `flowOp.line` at `sourceColumn`, or giving it an inline + # `maximum`, both SURVIVED until this kept the object. + def _subschemas(node: Any, key: str, out: list[Any]) -> None: if isinstance(node, dict): for k, v in node.items(): if k == key and isinstance(v, dict): - out.append(v.get("$ref") or v.get("type")) + out.append(v) else: - _line_refs(v, key, out) + _subschemas(v, key, out) elif isinstance(node, list): for v in node: - _line_refs(v, key, out) + _subschemas(v, key, out) + + # Anything that would make an UNBOUND path narrower than `load()`. + NARROWING = ("$ref", "minimum", "maximum", "exclusiveMinimum", + "exclusiveMaximum", "enum", "const", "multipleOf") for group, expect_bound in ((BOUND, True), (UNBOUND, False)): for def_name, keys in group.items(): for key in keys: found: list[Any] = [] - _line_refs(defs.get(def_name, {}), key, found) + _subschemas(defs.get(def_name, {}), key, found) if not found: failures += _fail( f"$defs.{def_name} has no {key!r} — the binding map is " f"stale, which means it is no longer evidence") - for ref in found: - is_bound = ref == "#/$defs/sourceLine" - if is_bound != expect_bound: - want = ("$ref sourceLine" if expect_bound - else "an unrestricted integer") - why = ("`load()` checks this path" - if expect_bound else - "`load()` does NOT check this path (§4.2)") + for sub in found: + if expect_bound: + if sub.get("$ref") != "#/$defs/sourceLine": + failures += _fail( + f"$defs.{def_name}.{key} is {sub!r}, expected " + f"$ref sourceLine — `load()` checks this path, " + f"so a schema-valid document must not be able " + f"to fail at the door") + continue + narrowed = [k for k in NARROWING if k in sub] + if narrowed or sub.get("type") != "integer": failures += _fail( - f"$defs.{def_name}.{key} is {ref!r}, expected " - f"{want} — {why}") + f"$defs.{def_name}.{key} is {sub!r}, expected a " + f"plain integer with no {'/'.join(NARROWING[:3])}… " + f"— `load()` does NOT check this path (§4.2), so " + f"any narrowing makes a document schema-invalid " + f"that the door accepts") # ---- line: the full signed-64 range, and one step outside each end ----- for label, build in LINE_PATHS: