Skip to content

[AAASM-4167] 🔒 (core): Fail closed on non-dict runtime policy return#216

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

[AAASM-4167] 🔒 (core): Fail closed on non-dict runtime policy return#216
Chisanan232 merged 2 commits into
masterfrom
v0.0.1/AAASM-4167/nondict_return_clean_deny

Conversation

@Chisanan232

Copy link
Copy Markdown
Contributor

Description

Robustness hardening for the SDK-layer runtime interceptor. In RuntimeQueryInterceptor.check_tool_start (agent_assembly/core/runtime_interceptor.py), the result.get("decision", ...) extraction sat outside the try/except guarding query_policy(). If query_policy() returned a non-dict (e.g. None from a malformed/partial runtime response), .get raised AttributeError instead of following the clean fail-closed deny path.

This adds an isinstance(result, dict) guard immediately after the try/except that routes a non-dict result through the existing _on_query_failure path — so it denies under enforce and allows under observe, exactly like the other unauthoritative-verdict paths (raising query, error-sentinel decisions, unknown decisions).

Not a fail-open bug: the pre-fix behaviour never returned allow — the exception aborted the call. This is purely graceful-degradation hardening. Realistically unreachable today (the PyO3 query_policy returns a dict or raises, and a raise was already caught), filed as LOW.

Type of Change

  • 🔧 Bug fix

Breaking Changes

  • No

Related Issues

  • Related JIRA ticket: AAASM-4167

Testing

  • Unit tests added/updated

New parametrized regression tests in test/unit/core/test_runtime_interceptor.py inject a runtime whose query_policy returns a non-dict (None, str, int, list) and assert a clean deny under enforce and a clean allow under observe — no AttributeError.

  • Targeted: pytest test/unit/core/test_runtime_interceptor.py → 59 passed
  • Full suite: pytest test/ → 749 passed, 17 skipped (optional native/framework deps)
  • ruff check on changed files clean; mypy agent_assembly shows only the pre-existing optional-_core-native-module import gaps

Checklist

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

Note — shared-file coordination with AAASM-4166

AAASM-4166 edits this same file (runtime_interceptor.py) at ~line 78 (_ALLOW_DECISIONS); this PR touches a different function at ~line 285. The hunks are disjoint so git auto-merges, but whichever PR merges second may need a trivial rebase. The diff here is kept minimal and localized to keep the hunks disjoint.

🤖 Generated with Claude Code

https://claude.ai/code/session_01R7vqjjo5nrebYNt8WnCNbz

Chisanan232 and others added 2 commits July 5, 2026 21:06
A non-dict query_policy() result (e.g. None from a malformed/partial
runtime response) previously reached result.get() outside the try/except
and raised AttributeError instead of the clean fail-closed deny. Guard
with isinstance before extraction so it degrades through _on_query_failure.

refs AAASM-4167

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01R7vqjjo5nrebYNt8WnCNbz
Injects a runtime whose query_policy returns a non-dict (None/str/int/list)
and asserts a clean deny under enforce and a clean allow under observe,
guarding against the AttributeError that result.get() raised before the
isinstance guard.

refs AAASM-4167

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 robustness-hardening fix for AAASM-4167 (LOW; graceful-degradation, not a fail-open).

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

Scope vs ticket: Full coverage. An isinstance(result, dict) guard is inserted immediately after the try/except around query_policy(), routing a non-dict return through _on_query_failure("runtime returned non-dict result") — clean deny under enforce, allow under observe — instead of raising AttributeError on result.get. Parametrized regression tests cover None/str/int/list under both postures.

Side-effects: None on the happy path. The guard fires only for non-dict results; a normal dict flows straight to the existing .get("decision") ladder unchanged (verified by reading the surrounding block). As the ticket notes, pre-fix behavior never returned allow — the exception aborted the call — so this is purely turning an ungraceful AttributeError into a structured deny. No behavior regression.

Shared-file / merge-order note: This edits runtime_interceptor.py at the non-dict guard (~line 285). PR #217 (AAASM-4166) edits the same file at _ALLOW_DECISIONS (~line 61) + docstring (~line 248). I verified the hunks are genuinely disjoint with no semantic conflict: this guard is upstream of #217's decision-set check and only diverts non-dict returns (which never reach that check), so neither PR changes how the other's path is reached. Either merge order is safe; whichever lands second needs only a trivial textual rebase.

— Claude Code review

@Chisanan232 Chisanan232 merged commit ed5e98d into master Jul 5, 2026
26 checks passed
@Chisanan232 Chisanan232 deleted the v0.0.1/AAASM-4167/nondict_return_clean_deny 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