Skip to content

Commit ed575e0

Browse files
PhysShellclaude
andcommitted
fix(S2-1): canonically bind plan envelope, carry+require full identity, normalize read OSError
Closes the three S2-slice-1 blockers (arbiter). No new scope. Blocker 1 — the validated-plan's authority envelope was only shape-checked (one type/one file) and compared to the candidates by RAW dict equality, so a self-consistent forged pair could smuggle a wrong type/file, wrong constraints, mismatched selected_findings, or extra nested fields. validate_apply_inputs now builds the CANONICAL projection of target_api / selection / source_files from the candidates' known keys and requires the plan to EQUAL it exactly. Blocker 2 — the code indexed c["event_identity"], but the shared candidates validator did not require it, so a hash-bound candidate missing an identity field passed validation and the apply gate raised a bare KeyError (violating "every violation -> ApplyError"). validate_candidates_bundle now requires the full identity set (event/source/handler + *_identity + *_identity_kind) as strings, and the apply context carries all of them (not just the human display) for the future span-node guard. Blocker 3 — the preimage SHA read (_sha_file) sat AFTER `except CollectError`, so an OSError between the root-confinement check and the read (file vanished / permission / replaced) leaked as a traceback. The resolve + read are now under one try with an added `except OSError -> ApplyError`. tests/test_fix_apply.py grows to 32 checks: canonical-projection mismatches (wrong type/file/constraints/selected_findings, unknown nested field in target/selection/source), missing/wrong-typed candidate identity fields, the full-identity-context assertion, and a source-read OSError (chmod-0, capability-guarded) -> ApplyError. ruff + mypy (ownlang) + run_tests green; S1 unaffected by the stricter candidates validator. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JxKjqdGEFzq4UzZupw379G
1 parent 1c577d7 commit ed575e0

3 files changed

Lines changed: 112 additions & 10 deletions

File tree

‎ownlang/fix_apply.py‎

Lines changed: 40 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -114,10 +114,32 @@ def validate_apply_inputs(
114114
# Hash binding: the candidates are the plan's own input.
115115
if bundle_sha256(candidates) != validated_plan["input_bundle_sha256"]:
116116
raise ApplyError("candidates sha256 does not match validated-plan.input_bundle_sha256")
117-
if validated_plan["target_api"] != candidates["target_api"]:
118-
raise ApplyError("validated-plan.target_api does not match candidates.target_api")
119-
if validated_plan["source_files"] != candidates["source_files"]:
120-
raise ApplyError("validated-plan.source_files does not match candidates.source_files")
117+
118+
# The plan's materialized authority envelope must EQUAL the canonical projection of
119+
# the candidates' known keys — not a raw dict copy. This rejects a self-consistent
120+
# forged pair that smuggled extra nested fields, a wrong type/file, wrong constraints,
121+
# or mismatched selected_findings into the plan.
122+
the_type = candidates["selection"]["allowed_types"][0]
123+
the_source = candidates["source_files"][0]
124+
expected_target_api = {"subscribe": candidates["target_api"]["subscribe"]}
125+
expected_selection = {
126+
"allowed_types": [{"full_name": the_type["full_name"], "file": the_type["file"]}],
127+
"selected_findings": candidates["selection"].get("selected_findings"),
128+
"constraints": {
129+
"max_types_changed": 1,
130+
"max_files_changed": 1,
131+
"allow_helper_changes": False,
132+
"allow_config_changes": False,
133+
"allow_suppressions": False,
134+
},
135+
}
136+
expected_source_files = [{"path": the_source["path"], "sha256": the_source["sha256"]}]
137+
if validated_plan["target_api"] != expected_target_api:
138+
raise ApplyError("validated-plan.target_api is not the canonical projection of candidates")
139+
if validated_plan["selection"] != expected_selection:
140+
raise ApplyError("validated-plan.selection is not the canonical projection of candidates")
141+
if validated_plan["source_files"] != expected_source_files:
142+
raise ApplyError("validated-plan.source_files is not the canonical projection")
121143

122144
candidate_by_id = {c["finding_id"]: c for c in candidates["candidates"]}
123145
seen: set[str] = set()
@@ -142,6 +164,9 @@ def validate_apply_inputs(
142164
raise ApplyError(
143165
f"{fid}: convert_acquire requires an inotify_property_changed contract"
144166
)
167+
# The FULL identity contract the future span-node guard needs (not just the
168+
# human display). validate_candidates_bundle guarantees all are present + str,
169+
# so the direct index cannot KeyError.
145170
convert.append({
146171
"finding_id": fid,
147172
"file": c["file"],
@@ -150,7 +175,11 @@ def validate_apply_inputs(
150175
"event": c["event"],
151176
"event_identity": c["event_identity"],
152177
"source": c["source"],
178+
"source_identity": c["source_identity"],
179+
"source_identity_kind": c["source_identity_kind"],
153180
"handler": c["handler"],
181+
"handler_identity": c["handler_identity"],
182+
"handler_identity_kind": c["handler_identity_kind"],
154183
})
155184
else:
156185
manual.append(fid)
@@ -167,13 +196,18 @@ def validate_apply_inputs(
167196
if start_b < end_a:
168197
raise ApplyError("overlapping convert_acquire spans")
169198

170-
# Source guard: confine to root, then match the pristine preimage SHA.
199+
# Source guard: confine to root, then match the pristine preimage SHA. A read that
200+
# fails between the confinement check and the hash (file vanished / permission /
201+
# replaced) is normalized to ApplyError, never a leaked OSError.
171202
source = candidates["source_files"][0]
172203
try:
173204
canonical, abs_path = _resolve_source(root, source["path"])
205+
actual_sha = _sha_file(abs_path)
174206
except CollectError as exc:
175207
raise ApplyError(f"source path: {exc}") from exc
176-
if _sha_file(abs_path) != source["sha256"]:
208+
except OSError as exc:
209+
raise ApplyError(f"cannot read source file: {exc}") from exc
210+
if actual_sha != source["sha256"]:
177211
raise ApplyError(f"STALE SOURCE / PREIMAGE MISMATCH for {canonical}")
178212

179213
return {

‎ownlang/fix_candidates.py‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -416,7 +416,10 @@ def validate_candidates_bundle(bundle: Any) -> None:
416416
raise CollectError(f"{cctx}: containing_type must be {type_name!r}")
417417
if _field(c, "file", "str", cctx) != source_path:
418418
raise CollectError(f"{cctx}: file must be {source_path!r}")
419-
for k in ("event", "source", "handler"):
419+
# Display + full identity set — required so downstream (S2 apply) can index them
420+
# without a KeyError and the span-node guard gets the real identity contract.
421+
for k in ("event", "source", "handler", "event_identity", "source_identity",
422+
"source_identity_kind", "handler_identity", "handler_identity_kind"):
420423
_field(c, k, "str", cctx)
421424
contract = _field(c, "event_contract", "str", cctx)
422425
if contract not in _CONTRACTS:

‎tests/test_fix_apply.py‎

Lines changed: 68 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -125,9 +125,52 @@ def raises(vplan: dict, cands: dict, root: str) -> bool:
125125
check(ctx["source_file"] == _REL
126126
and ctx["target_subscribe"] == "WeakEvents.AddPropertyChanged",
127127
"context carries source + target")
128-
check(ctx["convert_acquire"][0]["source"] == "_pub"
129-
and ctx["convert_acquire"][0]["handler"] == "OnChanged",
130-
"convert target carries candidate identity")
128+
target0 = ctx["convert_acquire"][0]
129+
check(target0["source"] == "_pub" and target0["handler"] == "OnChanged",
130+
"convert target carries candidate display identity")
131+
check(set(target0) >= {"source_identity", "source_identity_kind", "handler_identity",
132+
"handler_identity_kind", "event_identity", "containing_type"},
133+
"convert target carries the FULL identity contract")
134+
135+
# Blocker 1: the plan envelope must be the canonical projection of candidates.
136+
def vgood() -> dict:
137+
return _vplan(cands, [_decision(_FID_A, "convert_acquire", 100),
138+
_decision(_FID_B, "manual_review", 200)])
139+
v = vgood()
140+
v["selection"]["allowed_types"][0]["full_name"] = "Other.Type"
141+
check(raises(v, cands, root), "wrong selected type refused")
142+
v = vgood()
143+
v["selection"]["allowed_types"][0]["file"] = "N/Other.cs"
144+
check(raises(v, cands, root), "wrong selected type file refused")
145+
v = vgood()
146+
v["selection"]["constraints"]["max_types_changed"] = 2
147+
check(raises(v, cands, root), "wrong constraints refused")
148+
v = vgood()
149+
v["selection"]["selected_findings"] = [_FID_A]
150+
check(raises(v, cands, root), "selected_findings mismatch refused")
151+
v = vgood()
152+
v["target_api"]["extra"] = 1
153+
check(raises(v, cands, root), "unknown nested target_api field refused")
154+
v = vgood()
155+
v["selection"]["extra"] = 1
156+
check(raises(v, cands, root), "unknown nested selection field refused")
157+
v = vgood()
158+
v["source_files"][0]["extra"] = 1
159+
check(raises(v, cands, root), "unknown nested source_files field refused")
160+
161+
# Blocker 2: a candidate missing an identity field is a controlled ApplyError.
162+
for missing in ("event_identity", "source_identity", "handler_identity",
163+
"handler_identity_kind"):
164+
bc_cand = _cand(_FID_A, inpc, "inotify_property_changed", 100)
165+
del bc_cand[missing]
166+
bc = _bundle([bc_cand], sha)
167+
bv = _vplan(bc, [_decision(_FID_A, "convert_acquire", 100)])
168+
check(raises(bv, bc, root), f"candidate missing {missing} -> ApplyError")
169+
bad_type_cand = _cand(_FID_A, inpc, "inotify_property_changed", 100)
170+
bad_type_cand["source_identity"] = 42
171+
bt = _bundle([bad_type_cand], sha)
172+
btv = _vplan(bt, [_decision(_FID_A, "convert_acquire", 100)])
173+
check(raises(btv, bt, root), "candidate identity of wrong type -> ApplyError")
131174

132175
# hash binding: mutate candidates after the plan was built
133176
mutated = _bundle([_cand(_FID_A, inpc, "inotify_property_changed", 100),
@@ -202,6 +245,28 @@ def raises(vplan: dict, cands: dict, root: str) -> bool:
202245
# the file does not exist under root2 -> _resolve_source refuses (not a regular file)
203246
check(raises(ev, esc, root2), "missing/uncontained source refused")
204247

248+
# Blocker 3: an OSError reading the source (perms) is normalized to ApplyError.
249+
with tempfile.TemporaryDirectory() as root3:
250+
p = os.path.join(root3, "N", "C.cs")
251+
os.makedirs(os.path.dirname(p), exist_ok=True)
252+
body = b"class C {}\n"
253+
with open(p, "wb") as fh:
254+
fh.write(body)
255+
os.chmod(p, 0)
256+
unreadable = True
257+
try:
258+
with open(p, "rb"):
259+
unreadable = False # perms not enforced (Windows / running as root) -> skip
260+
except OSError:
261+
pass
262+
if unreadable:
263+
sha3 = "sha256:" + hashlib.sha256(body).hexdigest()
264+
oc = _bundle([_cand(_FID_A, ("convert_acquire", "manual_review"),
265+
"inotify_property_changed", 100)], sha3)
266+
ov = _vplan(oc, [_decision(_FID_A, "convert_acquire", 100)])
267+
check(raises(ov, oc, root3), "source read OSError -> ApplyError")
268+
os.chmod(p, 0o644) # restore so the tempdir cleans up
269+
205270
print(f"fix-apply (S2 slice 1): {ok} ok, {bad} bad")
206271
return bad
207272

0 commit comments

Comments
 (0)