Repository navigation
feat(governance): let a policy name decision models, points and endpoints - #23719
iamwhatever wants to merge 1 commit into
Conversation
UX Review (Opus 5.5) — ⏭️ skippedRevision |
Design Review (Opus 5.5) — ✅ PASSDesign-level review of Design-Verdict: PASS No design-level blockers. The change adds one catalog row to the existing inner-ruleset shape, and with no list named, decisions behave exactly as before. Notes
[DESIGN-REVIEWED] 7156757 Description read[DESCRIPTION-READ] 286de4ed5fd187bb9907cea8ea59ee455d253ccf358fa3d990f3f6b6e679915a That sha256 names everything this verdict read: the PR description, plus the 1 evidence file(s) the workflow collected from it and handed this review. It covers that evidence because the media strip replaces every attachment URL with the same placeholder, so an edit swapping one attachment for another would leave the description's captured text identical while what this review judged changed. Recompute it with |
First Principles Review (Opus 5.5) — ✅ PASSPremise-level review of First-Principles-Verdict: PASS No default moves: the lists only narrow when a policy names them. Two riders reach past the decision seam. Notes
What this change shipsInventory (5 items) — 3 justifiedIntent: let an enterprise policy restrict which decision models, points and endpoints the decision seam may use. ADDITION.
[FIRST-PRINCIPLES-REVIEWED] 7156757 Description read[DESCRIPTION-READ] 80445d1cbdafa1f8bbd076e5ec86df5939acb6581680d9b227b2155cf3c75e3a That sha256 names the PR description this verdict read. Recompute it with |
Opus 5.5 Review — 🔴 changes requested (blocking)Reviewed The new IDNA normalisation in BLOCKING — src/kiro_crew/platform/governance_parts/matching.py:122 [BLOCK-MERGE] 7156757 Verdict parsed from the review's SHA-scoped output markers for commit False positive or not applicable? A repository writer can comment: |
GPT 6.1 Review — 🔴 changes requested (blocking)GPT 6.1 found at least one blocking issue that must be resolved before merging This comment is updated in place on each push. BLOCKING -- src/kiro_crew/platform/governance_parts/matching.py:122 -- IDNA conversion disables existing Unicode host globs
[BLOCK-MERGE] 7156757 Adjudication (Opus 5.5) — is blocking on each finding proportionate?The fenced finding (F1) stands: I'm upholding it as written. Nothing else was submitted for adjudication. F1: This is unbounded harm, of the governance-ceiling kind. It is a regression in this PR: a
False positive or not applicable? A repository writer can comment: |
597974c to
72a3966
Compare
|
Fixed in 72a3966: GPT BLOCKING on |
72a3966 to
6024f7e
Compare
|
Fixed in 6024f7e: both BLOCKING findings on |
6024f7e to
a5ed698
Compare
|
Round 3, fixed in a5ed698.
|
a5ed698 to
7156757
Compare
|
Round 4, fixed in the shared
This class came back in four rounds, so per AGENTS.md I am narrowing rather than chasing a fifth spelling. A reading no parser can canonicalize is asked as |
Problem / Motivation
Goal: A security policy can name the exact decision models, decision points and provider endpoints the decision seam may use, and a request outside them is not sent.
capabilities.decisionsis an on/off gate with no list (RFC crew-enterprise-edition, appendix rows "Decision providers"partial, "Decision models"decisions.models, "Decision kinds"decisions.pointsand "Decision endpoints"decisions.endpoints, allno).Why it matters
A fleet that approves one model at one endpoint for some decision points must be able to refuse the rest, rather than choosing between the whole seam and nothing.
Not a goal
_query_level: with no policy, or a policy without these lists, decisions behave as today.capabilities.decisions_local.What changed (motivation → approach → change)
Goal → the existing CapabilityGate inner-ruleset shape → one catalog row change, one probe, one call site.
governance_parts/catalog.py:capabilities.decisionsgainsscope_matchersmodels(identifier),points(identifier),endpoints(host).decisions/capability.targets_denied: askspoints:<point>and, for the Jev lane,models:<model>andendpoints:<host>for every reading of the host; each evaluated and audited, never short-circuited; fail-closed. The items are bracketed with the governance answer generation, asdenied_sidesdoes, so one request is never answered by two policies.decisions/gate.decide: calls it once the lane and model are known, before the scrub. A refusal returnsNoneand writes no decision row, like the capability refusal.modelsandendpointsbind the decision provider, which only the Jev lane uses; the judge's LLM lane runs on the session's own model provider through the session runner, which picks the served model itself, so it is asked about its point only.governance_parts/matching.url_policy_hosts: every host a client could connect to for the endpoint (the standard library plain and host-percent-decoded,urllib3,yarl; non-ASCII in IDNA form), so a root dot, a backslash, a percent escape or an ideographic dot cannot dodge an endpoint deny. The same helper is added identically in feat(governance): let a policy name the OTLP collector hosts #23716 and feat(governance): let a policy name the feature content hosts #23717.governance.md(decision seam section).Call site wired:
decisions/gate.py::decide(through_targets_denied).Backwards compatibility
Compatible: with no policy, or a policy that does not name these lists, every item is permitted and
decidesends as today. The extra SEL rows appear only on a consented install, past the existing capability check.Removes nothing: the lists only narrow when a policy names them.
Tests
test/test_governance_list_decisions.py: each key parses with its catalog matcher; a list allowing X permits X and denies Y; gate off denies; no policy or no list permits.test/test_decisions_gate.py(TestDecisionLists): no policy sends; a list naming this request's point, model or endpoint sends; a list not naming it sends nothing, writes no row and audits the items; gate off sends nothing; the LLM lane is asked about its point only; a check error denies; a policy swap between items denies and is audited; root-dot, backslash, ideographic-dot and percent spellings cannot dodge an endpoint deny.Proof: not provable -- other: new feature; the tests assert behaviour that main does not have
Manual verification
N/A — unit coverage drives
decidewith a real policy and the real evaluator.Related Issues
Refs #23206. RFC: #23208.
Checklist
feat|fix|docs|style|refactor|perf|test|chore|ci|build|revert: ...)Contribution License Agreement
Contributions are accepted under the project's Apache-2.0 license.