Skip to content

Commit 8238dd8

Browse files
Haruka-Kayaclaude
andcommitted
fix(resolution): merge.py レビュー指摘対応 — RED-to-GREEN テスト化+inline 化(microsoft#3169
@MohammadHaroonAbuomar のレビュー対応: - 【必須】test_conditions_overlap_..._unsat_bias は同一 condition 2つで _condition_key 一致の早期 return を踏むだけで、main でも pass=新ロジックを 検証できていなかった。レビュアー提案の具体例で書き直し: 親 deny = {and:[{x in []}, {tool eq shell}]}(_condition_unsatisfiable が unsatisfiable と分類するが tool==shell で子と重なる)vs 非同一の子 allow。 → main では子 allow が生き残る(fail-open)= RED、本ブランチでは drop = GREEN を実機で確認済み。これにより本指摘が defense-in-depth ではなく実在の fail-open であることも実証。 - _condition_key 早期 return の sanity guard を別テストに分離。 - minor: _flag エイリアスを廃し ignore_left_unsatisfiable を直接渡す(grep性)。 検証: test_merge_deny.py + test_manifest_resolution.py 58 passed。 新テストは main で RED / 本ブランチで GREEN を確認。 Refs microsoft#3137 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VT2CfGX3tmPsQaofuuWwED
1 parent a32197b commit 8238dd8

2 files changed

Lines changed: 42 additions & 11 deletions

File tree

agent-governance-python/agt-policies/src/agt/manifest_resolution/merge.py

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -257,30 +257,37 @@ def _conditions_disjoint(
257257
if not isinstance(left, dict) or not isinstance(right, dict):
258258
return False
259259

260-
_flag = ignore_left_unsatisfiable
261260
left_or = _compound_items(left, "or")
262261
if left_or is not None:
263262
return all(
264-
_conditions_disjoint(item, right, ignore_left_unsatisfiable=_flag)
263+
_conditions_disjoint(
264+
item, right, ignore_left_unsatisfiable=ignore_left_unsatisfiable
265+
)
265266
for item in left_or
266267
)
267268
right_or = _compound_items(right, "or")
268269
if right_or is not None:
269270
return all(
270-
_conditions_disjoint(left, item, ignore_left_unsatisfiable=_flag)
271+
_conditions_disjoint(
272+
left, item, ignore_left_unsatisfiable=ignore_left_unsatisfiable
273+
)
271274
for item in right_or
272275
)
273276

274277
left_and = _compound_items(left, "and")
275278
if left_and is not None:
276279
return any(
277-
_conditions_disjoint(item, right, ignore_left_unsatisfiable=_flag)
280+
_conditions_disjoint(
281+
item, right, ignore_left_unsatisfiable=ignore_left_unsatisfiable
282+
)
278283
for item in left_and
279284
)
280285
right_and = _compound_items(right, "and")
281286
if right_and is not None:
282287
return any(
283-
_conditions_disjoint(left, item, ignore_left_unsatisfiable=_flag)
288+
_conditions_disjoint(
289+
left, item, ignore_left_unsatisfiable=ignore_left_unsatisfiable
290+
)
284291
for item in right_and
285292
)
286293

agent-governance-python/agt-policies/tests/test_merge_deny.py

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -99,9 +99,33 @@ def test_satisfiable_compound_parent_deny_still_protects() -> None:
9999
assert "allow-admin" not in names
100100

101101

102-
def test_conditions_overlap_protects_parent_against_left_unsat_bias() -> None:
103-
# Deny-immutability path must not declare "no overlap" by classifying the
104-
# PARENT (left) condition unsatisfiable. Same field + same value overlap.
105-
parent_cond = {"field": "tool", "operator": "eq", "value": "shell"}
106-
child_cond = {"field": "tool", "operator": "eq", "value": "shell"}
107-
assert _conditions_overlap(parent_cond, child_cond) is True
102+
def test_unsat_classified_parent_deny_still_protects_overlapping_child() -> None:
103+
# RED on main, GREEN here. The parent deny's condition is classified
104+
# unsatisfiable by `_condition_unsatisfiable` (the empty-`in` conjunct),
105+
# yet it still overlaps the child via its other conjunct (`tool == shell`).
106+
# On main, `_conditions_disjoint` short-circuits on the parent's
107+
# unsatisfiability, declares the deny disjoint, and the child allow
108+
# survives — neutralising the parent deny (fail-open, ADR-0014). The fix
109+
# ignores the parent's unsatisfiability on the deny path, so the overlap is
110+
# detected and the child allow is dropped.
111+
parent = _doc([{
112+
"name": "deny-shell", "action": "deny",
113+
"condition": {"and": [
114+
{"field": "x", "operator": "in", "value": []},
115+
{"field": "tool", "operator": "eq", "value": "shell"},
116+
]},
117+
}])
118+
child = _doc([{
119+
"name": "allow-shell", "action": "allow", "priority": 100,
120+
"condition": {"field": "tool", "operator": "eq", "value": "shell"},
121+
}])
122+
names = [r["name"] for r in merge_documents([parent, child])]
123+
assert "deny-shell" in names
124+
assert "allow-shell" not in names
125+
126+
127+
def test_conditions_overlap_identical_conditions() -> None:
128+
# Sanity guard for the `_condition_key` fast-path (non-identical-dict cases
129+
# are covered by the merge_documents tests above).
130+
cond = {"field": "tool", "operator": "eq", "value": "shell"}
131+
assert _conditions_overlap(dict(cond), dict(cond)) is True

0 commit comments

Comments
 (0)