Skip to content

[AAASM-4166] 🔒 (core): Fail closed on UNSPECIFIED policy verdict#217

Merged
Chisanan232 merged 2 commits into
masterfrom
v0.0.1/AAASM-4166/unspecified_fail_closed
Jul 5, 2026
Merged

[AAASM-4166] 🔒 (core): Fail closed on UNSPECIFIED policy verdict#217
Chisanan232 merged 2 commits into
masterfrom
v0.0.1/AAASM-4166/unspecified_fail_closed

Conversation

@Chisanan232

Copy link
Copy Markdown
Contributor

Description

The proto3 zero-value policy decision UNSPECIFIED (=0, "no decision rendered") was folded into _ALLOW_DECISIONS in agent_assembly/core/runtime_interceptor.py, so a non-authoritative verdict was treated as an authoritative allow and proceeded under enforce. Python (and Go) failed open here, whereas the Node SDK already fails closed (Decision::Unspecified routes to its default: → deny).

This PR drops "unspecified" from _ALLOW_DECISIONS so it falls through to the existing fail-closed path (_on_query_failure): deny under enforce, allow under observe — exactly how any other non-authoritative/unrecognized decision is handled, and matching Node.

Defense-in-depth / cross-SDK consistency (LOW): only reachable if the trusted local runtime returns an unset/partial decision (a runtime bug, not attacker-controllable over the ownership-checked UDS); the gateway/proxy/eBPF layers remain authoritative.

Type of Change

  • 🔧 Bug fix

Breaking Changes

  • No

Related Issues

  • Related JIRA ticket: AAASM-4166

Testing

  • Unit tests added/updated

test_unspecified_decision_denies_under_enforce (deny under enforce) and test_unspecified_decision_allows_under_observe (fail-open under observe) added; the test_known_good_decisions_allow_under_enforce assertion that codified the old behavior was updated. Full suite: 742 passed, 17 skipped. ruff check clean; mypy agent_assembly shows only the pre-existing optional-_core/grpc-stub baseline (unchanged by this PR; pre-commit mypy hook passes).

Cross-repo note

This is the Python half of AAASM-4166; the Go half is a separate PR against ai-agent-assembly/go-sdk.

Rebase note (shared file)

AAASM-4167 also edits runtime_interceptor.py (a different function, ~line 285). This PR's changes are localized to _ALLOW_DECISIONS (~line 61) and its docstring, so the hunks are disjoint and git auto-merges; whichever of the two PRs merges second may need a trivial rebase.

Checklist

  • Code follows project style guidelines
  • Self-review completed
  • All tests passing

Chisanan232 and others added 2 commits July 5, 2026 21:15
The proto3 zero-value decision "unspecified" ("no decision rendered") was in
_ALLOW_DECISIONS, so a non-authoritative verdict was treated as an authoritative
allow and proceeded under enforce — Python failed open where Node fails closed.
Drop "unspecified" from _ALLOW_DECISIONS so it falls through to the existing
fail-closed path (deny under enforce, allow under observe), matching the Node
SDK (AAASM-4166). Update the known-good-under-enforce assertion that codified
the old behavior.

Refs AAASM-4166

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01R7vqjjo5nrebYNt8WnCNbz
Regression test for AAASM-4166: an "unspecified" runtime decision denies under
enforce (fail-closed, was allow) and still proceeds under observe (fail-open),
matching the treatment of any other non-authoritative verdict.

Refs AAASM-4166

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01R7vqjjo5nrebYNt8WnCNbz
@codecov

codecov Bot commented Jul 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@sonarqubecloud

sonarqubecloud Bot commented Jul 5, 2026

Copy link
Copy Markdown

@Chisanan232

Copy link
Copy Markdown
Contributor Author

Claude Code review — COMMENT (not an approval)

Verdict: approve-ready. Correct, minimal, well-tested fix for AAASM-4166 (LOW / defense-in-depth, cross-SDK consistency).

CI: Green — 19/19 checks pass. Codecov + SonarCloud quality gate both passed.

Scope vs ticket: Full coverage. "unspecified" dropped from _ALLOW_DECISIONS (now {"allow", "redact"}), so an UNSPECIFIED verdict falls through to the final _on_query_failure(...) — deny under enforce, allow under observe — exactly like any other non-authoritative decision, matching the Node SDK's default:→deny. Both new tests (test_unspecified_decision_denies_under_enforce, ..._allows_under_observe) assert precisely that, and the stale test_known_good_decisions_allow_under_enforce parametrization was correctly narrowed to drop unspecified.

Side-effects: None. allow and redact remain in the allow-set, so no legit allow/redact verdict is wrongly denied — verified by reading the check_tool_start decision ladder. The runtime remains authoritative on redaction. Only reachable when the trusted local runtime emits an unset verdict (a runtime bug, not attacker-controllable over the ownership-checked UDS); gateway/proxy/eBPF stay authoritative.

Cross-SDK convergence: Confirmed — this + go-sdk #129 both make UNSPECIFIED deny under enforce, closing the py/go fail-open gap vs. Node. That is the point of 4166.

Shared-file / merge-order note: This edits runtime_interceptor.py at _ALLOW_DECISIONS (~line 61) + its docstring (~line 248). PR #216 (AAASM-4167) edits the same file at the non-dict guard (~line 285). I verified the hunks are genuinely disjoint with no semantic conflict: #216's guard sits upstream and only diverts non-dict returns (which never reach the _ALLOW_DECISIONS check), while this PR only changes handling of dict returns whose decision == "unspecified" (which pass the isinstance guard untouched). Either merge order is safe; whichever lands second needs only a trivial textual rebase.

— Claude Code review

@Chisanan232 Chisanan232 merged commit 19093b7 into master Jul 5, 2026
26 checks passed
@Chisanan232 Chisanan232 deleted the v0.0.1/AAASM-4166/unspecified_fail_closed branch July 5, 2026 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant