Skip to content

Fix: Stop PRB Door B auditor over-reaching on scope-exclusive policies (#2511) - #845

Closed
anatolykoyfman wants to merge 1 commit into
rossoctl:mainfrom
s-and-p-team:fix/2511-door-b-auditor
Closed

Fix: Stop PRB Door B auditor over-reaching on scope-exclusive policies (#2511)#845
anatolykoyfman wants to merge 1 commit into
rossoctl:mainfrom
s-and-p-team:fix/2511-door-b-auditor

Conversation

@anatolykoyfman

@anatolykoyfman anatolykoyfman commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes rossoctl/rossoctl#2511 — the PRB (Policy Rules Builder) Door B auditor over-reached on scope-exclusive policies, rejecting a correct empty deny proposal to exhaustion and returning HTTP 422.

When a scope-exclusive policy named a different role (e.g. "Only developers may read source") while onboarding a non-grantee focal role (rossoctl-admin), Door B (build_role_denies, focal = user role, DENY-only) demanded a focal-role deny that does not belong to this pass. The complement deny is the scope-focal pass's job (rules 4 & 6), not Door B's. The proposer was always correct; only the auditor over-reached, burning the retry budget → PolicyRulesBuilderError → 422.

The fix (auditor-only)

A MISSING-PROHIBITION guardrail in _AUDITOR_SYSTEM tells the auditor not to reject a proposal for a missing focal deny when the restriction names a different entity (rule 4). An absent focal prohibition is a defect only when the scenario policy is exclusive/restrictive about the focal entity itself (rule 6) or explicitly prohibits it (rule 5).

The shared proposer/auditor rules are byte-identical to baseline_PROPOSER_SYSTEM, _SAFETY, _MAPPING_RULES, _DENY_RULES unchanged. An earlier attempt that carried the reassurance in the shared rule-4 text regressed the proposer's legitimate description-driven denies; moving it to the auditor alone fixes both. The change can therefore only make Door B denies more conservative — grant behavior cannot change.

Files (3)

  • aiac/src/aiac/agent/policy_rules_builder/prompts.py_AUDITOR_SYSTEM guardrail only
  • aiac/test/agent/policy_rules_builder/test_graph_live_llm.py — slice 7 repro relaxed to the real requirement
  • aiac/CONTEXT.md — scope-exclusivity / role-exclusivity glossary entries

Verification

  • -m llm (real LLM, no cluster): full PRB live-LLM suite 7/7. Slice 7 repro test_door_b_no_overreach_on_scope_exclusivity green 3/3 across model variance (no raise / no ALLOW / denies ⊆ {source-read, source-write}). Slice 6 exclusivity safety net (Testers may access only issues → DENY source-read/write) intact. Slice 4 description-driven-deny regression guard green.

Notes

Assisted-By: Claude (Anthropic AI) noreply@anthropic.com

Summary by CodeRabbit

  • Bug Fixes

    • Corrected policy handling so scope-exclusive language about another role does not incorrectly create a denial for the focal role.
    • Prevented unrelated roles or scopes from receiving unintended restrictions.
    • Allowed valid policies with no focal-role prohibitions to process successfully instead of returning an error.
  • Documentation

    • Added glossary definitions clarifying scope-exclusive and role-exclusive language and how each is evaluated.

…s (#2511)

The Door B (user-role-focal, deny-only) auditor was rejecting a correct
empty-prohibition proposal to exhaustion when the scenario policy is
scope-exclusive about a DIFFERENT role ("Only developers may read source"):
it demanded a focal-role deny that does not belong to this pass, burned the
retry budget, and raised PolicyRulesBuilderError -> HTTP 422. That complement
deny is the scope-focal pass's job, so an absent Door B prohibition is correct.

Fix is auditor-only: a MISSING-PROHIBITION guardrail in _AUDITOR_SYSTEM tells
the auditor not to reject a proposal for a missing focal deny when the
restriction names a different entity (rule 4). The shared proposer/auditor
rules are reverted to baseline (byte-identical) — carrying this reassurance in
the shared rules regressed the proposer's legitimate description-driven denies.

- prompts.py: auditor-only guardrail; _PROPOSER_SYSTEM/_SAFETY/_MAPPING_RULES/
  _DENY_RULES unchanged from baseline (grant behaviour untouched)
- test_graph_live_llm.py: slice 7 now asserts the real requirement — no 422,
  no ALLOW, denies subset of {source-read, source-write}; a duplicate source
  deny is a harmless echo of the scope-focal pass, not the defect
- CONTEXT.md: scope-exclusivity / role-exclusivity glossary entries

Verified: -m llm PRB suite 7/7 (slice 7 green 3/3 across mode-mixing).

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Anatoly Koyfman <anatoly@il.ibm.com>
@coderabbitai

coderabbitai Bot commented Sep 2, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: f3647709-2cb0-4680-9b78-27fd10f89d2e

📥 Commits

Reviewing files that changed from the base of the PR and between 26799ac and 4a0ea5d.

📒 Files selected for processing (3)
  • aiac/CONTEXT.md
  • aiac/src/aiac/agent/policy_rules_builder/prompts.py
  • aiac/test/agent/policy_rules_builder/test_graph_live_llm.py

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The auditor prompt now permits empty Door B prohibition lists when restrictions target another entity. Documentation defines scope- and role-exclusivity. A live-LLM test verifies that unrelated roles do not receive scope-exclusive denies.

Changes

Door B scope-exclusivity handling

Layer / File(s) Summary
Clarify exclusivity ownership
aiac/CONTEXT.md, aiac/src/aiac/agent/policy_rules_builder/prompts.py
The glossary distinguishes scope-exclusivity from role-exclusivity. The auditor accepts empty prohibitions unless exclusivity or an explicit prohibition targets the focal role.
Validate unrelated-role handling
aiac/test/agent/policy_rules_builder/test_graph_live_llm.py
A live-LLM test verifies that scope-exclusive source access for developers does not produce ALLOW results or denies for the unrelated issues scope.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 4a0ea

This localized fix prevents valid scope-exclusive policies from failing with a 422 while preserving existing deny-only and conflict safeguards; no actionable merge-blocking risk remains beyond normal checks and review.

Suggested reviewers: abigailgold

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. (1 skipped: 1 … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the main change: preventing the Door B auditor from over-reaching on scope-exclusive policies.
Linked Issues check ✅ Passed The prompt change addresses issue #2511 by allowing an empty Door B deny list when exclusivity names a different role. The added live-LLM test checks that the focal unrelated role does not receive an …
Out of Scope Changes check ✅ Passed The changes are within scope. They modify the Door B auditor prompt, add the issue-specific regression test, and document the scope-exclusivity and role-exclusivity distinction needed to explain the f…
Full details: Linked Issues check

Explanation

The prompt change addresses issue #2511 by allowing an empty Door B deny list when exclusivity names a different role. The added live-LLM test checks that the focal unrelated role does not receive an invalid deny or ALLOW result. The provided context also reports that the full live-LLM suite passes, which supports preservation of focal-role exclusivity behavior.

Full details: Out of Scope Changes check

Explanation

The changes are within scope. They modify the Door B auditor prompt, add the issue-specific regression test, and document the scope-exclusivity and role-exclusivity distinction needed to explain the fix.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@huang195

huang195 commented Sep 8, 2026

Copy link
Copy Markdown
Member

@anatolykoyfman this PR has been opened for a week. Just wanted to check if this is still in a draft status and not yet ready for review.

@anatolykoyfman

Copy link
Copy Markdown
Contributor Author

@huang195 This is still a draft. I still work out some details that were flagged after this PR was created. Should I close it, and open when my solution finally works out ?

@anatolykoyfman

Copy link
Copy Markdown
Contributor Author

I was decided not to proceed with this PR. The problem will be reevaluated, and PR will be opened for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants