Skip to content

feat: add MaybeDontAnalyzer security analyzer#2142

Open
robotdan wants to merge 4 commits intoOpenHands:mainfrom
maybedont:degroff/maybedont-analyzer
Open

feat: add MaybeDontAnalyzer security analyzer#2142
robotdan wants to merge 4 commits intoOpenHands:mainfrom
maybedont:degroff/maybedont-analyzer

Conversation

@robotdan
Copy link
Copy Markdown

@robotdan robotdan commented Feb 20, 2026 •
edited
Loading

Summary

Adds MaybeDontAnalyzer, a new SecurityAnalyzerBase implementation that validates agent actions against policy rules configured in a Maybe Don't Gateway instance.

  • Calls the gateway's POST /api/v1/action/validate endpoint before action execution
  • Maps ActionEvent fields (tool_name, tool_call.arguments, thought, summary) to the gateway's request format
  • Maps the gateway's risk_level response directly to SecurityRisk (HIGH/MEDIUM/LOW/UNKNOWN)
  • Graceful degradation: all errors (timeout, unreachable, 500, invalid JSON) return UNKNOWN
  • Follows the GraySwan analyzer patterns: Pydantic fields, lazy httpx client, model_post_init env var resolution, set_events/close lifecycle

How it fits with existing integrations

Layer What Covers
Security Analyzer (this PR) Pre-execution validation via REST endpoint ALL actions: shell commands, file ops, browser, tool calls
MCP Proxy (existing docs) Execution-time validation + proxying Only MCP tool calls routed through the gateway

MaybeDont doc for OpenHands

Configuration

from openhands.sdk.security.maybedont import MaybeDontAnalyzer

analyzer = MaybeDontAnalyzer()  # defaults to http://localhost:8080
# or
analyzer = MaybeDontAnalyzer(gateway_url="http://my-gateway:8080")
# or via env var
# MAYBE_DONT_GATEWAY_URL=http://my-gateway:8080

Files

File Purpose
openhands-sdk/openhands/sdk/security/maybedont/analyzer.py MaybeDontAnalyzer class
openhands-sdk/openhands/sdk/security/maybedont/__init__.py Package export
openhands-sdk/openhands/sdk/security/__init__.py Added to module __all__
tests/sdk/security/maybedont/test_maybedont_analyzer.py 41 unit tests
examples/01_standalone_sdk/40_maybedont_security_analyzer.py Usage example

Related

Test plan

  • uv run pytest tests/sdk/security/maybedont/ -v — 41 tests pass
  • uv run pytest tests/sdk/security/ -v — 119 tests pass (no regressions)
  • make format && make lint — clean
  • Integration tested against a live Maybe Don't Gateway (CEL deny rule for rm -rf → HIGH, safe commands → LOW)

🤖 Generated with Claude Code

@claude
feat: add MaybeDontAnalyzer security analyzer
8a7afa1 
Implements a SecurityAnalyzerBase that validates agent actions against
policy rules configured in a Maybe Don't Gateway instance. Calls the
gateway's POST /api/v1/action/validate endpoint and maps the response
risk_level directly to SecurityRisk.

- MaybeDontAnalyzer class following GraySwan patterns
- 41 tests covering init, request building, risk mapping, error handling,
  HTTP lifecycle, and end-to-end security_risk flow
- Example script (40_maybedont_security_analyzer.py)
- Exported from openhands.sdk.security module

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@robotdan robotdan mentioned this pull request Feb 20, 2026
Open
4 tasks
@claude
fix: Docker example in docstring that won't start
868df36 
- Add MAYBE_DONT_SERVER_LISTEN_ADDR=0.0.0.0:8080 (127.0.0.1 unreachable from host)
- Disable AI validation and audit report (require OpenAI API key)
- Match Docker command to docs PR (OpenHands/docs#350)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@all-hands-bot all-hands-bot requested a review from rbren February 25, 2026 12:30
@all-hands-bot
Copy link
Copy Markdown
Collaborator

all-hands-bot commented Feb 25, 2026

[Automatic Post]: I have assigned @rbren as a reviewer based on git blame information. Thanks in advance for the help!

@all-hands-bot
Copy link
Copy Markdown
Collaborator

all-hands-bot commented Mar 2, 2026

[Automatic Post]: This PR seems to be currently waiting for review. @rbren, could you please take a look when you have a chance?

@all-hands-bot
Copy link
Copy Markdown
Collaborator

all-hands-bot commented Mar 8, 2026

[Automatic Post]: It has been a while since there was any activity on this PR. @robotdan, are you still working on it? If so, please go ahead, if not then please request review, close it, or request that someone else follow up.

1 similar comment
@all-hands-bot
Copy link
Copy Markdown
Collaborator

all-hands-bot commented Mar 13, 2026

[Automatic Post]: It has been a while since there was any activity on this PR. @robotdan, are you still working on it? If so, please go ahead, if not then please request review, close it, or request that someone else follow up.

@all-hands-bot
Copy link
Copy Markdown
Collaborator

all-hands-bot commented Mar 17, 2026

[Automatic Post]: This PR seems to be currently waiting for review. @rbren, could you please take a look when you have a chance?

@all-hands-bot
Copy link
Copy Markdown
Collaborator

all-hands-bot commented Mar 23, 2026

[Automatic Post]: It has been a while since there was any activity on this PR. @robotdan, are you still working on it? If so, please go ahead, if not then please request review, close it, or request that someone else follow up.

@all-hands-bot
Copy link
Copy Markdown
Collaborator

all-hands-bot commented Mar 27, 2026

[Automatic Post]: This PR seems to be currently waiting for review. @rbren, could you please take a look when you have a chance?

@all-hands-bot
Copy link
Copy Markdown
Collaborator

all-hands-bot commented Apr 1, 2026

[Automatic Post]: It has been a while since there was any activity on this PR. @robotdan, are you still working on it? If so, please go ahead, if not then please request review, close it, or request that someone else follow up.

@all-hands-bot
Copy link
Copy Markdown
Collaborator

all-hands-bot commented Apr 6, 2026

[Automatic Post]: This PR seems to be currently waiting for review. @rbren, could you please take a look when you have a chance?

@all-hands-bot
Copy link
Copy Markdown
Collaborator

all-hands-bot commented Apr 12, 2026

[Automatic Post]: It has been a while since there was any activity on this PR. @robotdan, are you still working on it? If so, please go ahead, if not then please request review, close it, or request that someone else follow up.

1 similar comment
@all-hands-bot
Copy link
Copy Markdown
Collaborator

all-hands-bot commented Apr 17, 2026

[Automatic Post]: It has been a while since there was any activity on this PR. @robotdan, are you still working on it? If so, please go ahead, if not then please request review, close it, or request that someone else follow up.

@VascoSch92
Copy link
Copy Markdown
Contributor

VascoSch92 commented Apr 17, 2026

Hey @robotdan are you still working at this PR? Should I review it?

@xingyaoww OpenHands AI
Copy link
Copy Markdown
Collaborator

xingyaoww commented Apr 21, 2026 •
edited
Loading

@OpenHands /codereview /github-pr-review

🤖 This comment was automatically posted by the pr-triage skill (OpenHands) on behalf of a maintainer. See PR #2906 for skill details.

@openhands-ai
Copy link
Copy Markdown

openhands-ai Bot commented Apr 21, 2026

I'm on it! xingyaoww can track my progress at all-hands.dev

xingyaoww
xingyaoww approved these changes Apr 21, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@xingyaoww xingyaoww left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Good taste — Clean, well-structured integration that closely follows the existing GraySwan analyzer patterns. The code is straightforward, error handling is thorough, and the test suite covers the important paths.

A couple of minor suggestions inline, plus one thing to fix before the test-examples workflow picks this up.

This review was generated by an AI assistant (OpenHands) on behalf of the team.

Comment thread openhands-sdk/openhands/sdk/security/maybedont/analyzer.py
f"timeout={self.timeout}s, client_id={self.client_id}"
)

def set_events(self, events: Any) -> None:
Copy link
Copy Markdown
Collaborator

@xingyaoww xingyaoww Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: set_events takes Any here, but GraySwanAnalyzer.set_events uses the typed Sequence[LLMConvertibleEvent]. Using the typed signature keeps the interface consistent and gives callers better editor/type-checker support.

Suggested change
def set_events(self, events: Any) -> None:
def set_events(self, events: Sequence[LLMConvertibleEvent]) -> None:

(You'd need to add from collections.abc import Sequence to the imports.)

Comment thread openhands-sdk/openhands/sdk/security/maybedont/analyzer.py
Returns:
SecurityRisk level based on gateway response
"""
url = f"{self.gateway_url.rstrip('/')}/api/v1/action/validate"
Copy link
Copy Markdown
Collaborator

@xingyaoww xingyaoww Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: self.gateway_url is typed str | None, so pyright will flag .rstrip('/') here. After model_post_init it's always a str, but the type checker doesn't know that. A quick assert makes the intent explicit:

Suggested change
url = f"{self.gateway_url.rstrip('/')}/api/v1/action/validate"
assert self.gateway_url is not None
url = f"{self.gateway_url.rstrip('/')}/api/v1/action/validate"

Comment thread examples/01_standalone_sdk/40_maybedont_security_analyzer.py
@@ -0,0 +1,150 @@
"""OpenHands Agent SDK — Maybe Don't Security Analyzer Example
Copy link
Copy Markdown
Collaborator

@xingyaoww xingyaoww Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Important: This example is not in _EXCLUDED_EXAMPLES in tests/examples/test_examples.py, so the test-examples CI workflow will try to run it. It will fail because (a) there's no Maybe Don't Gateway in CI, and (b) it calls input() which raises EOFError in non-interactive mode.

Please add it to the exclusion list, similar to 16_llm_security_analyzer.py:

# In tests/examples/test_examples.py, _EXCLUDED_EXAMPLES:
"examples/01_standalone_sdk/40_maybedont_security_analyzer.py",

@openhands-ai
Copy link
Copy Markdown

openhands-ai Bot commented Apr 21, 2026

Summary

I completed a code review of PR #2142 ("feat: add MaybeDontAnalyzer security analyzer") and submitted an APPROVE review via the GitHub API with 3 inline comments.

What was done:

  • ✅ Retrieved and analyzed the full PR diff (6 files, ~1100 lines)
  • ✅ Compared the new MaybeDontAnalyzer against the existing GraySwanAnalyzer patterns and SecurityAnalyzerBase interface
  • ✅ Checked for security concerns, type safety, backward compatibility, and CI impact
  • ✅ Posted a single bundled review with inline comments using the GitHub PR review skill format
  • ✅ Used appropriate priority labels (🟠 Important, 🟡 Suggestion)
  • ✅ Followed the custom codereview guide's "default to APPROVE" policy since no critical/blocking issues were found
  • ✅ Included AI disclosure in the review body

Review findings posted:

  1. 🟡 set_events type signature — Uses Any instead of Sequence[LLMConvertibleEvent] (inconsistent with GraySwan)
  2. 🟡 gateway_url optional type — .rstrip('/') called on str | None field; pyright would flag it
  3. 🟠 Example not excluded from CI — 40_maybedont_security_analyzer.py is missing from _EXCLUDED_EXAMPLES and would fail in the test-examples workflow (no gateway + uses input())

No code changes were made

This was a review-only task — no files in the repository were modified, which is appropriate for a /codereview request.

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

Reviewers

@xingyaoww xingyaoww xingyaoww approved these changes

@rbren rbren Awaiting requested review from rbren

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@robotdan @all-hands-bot @VascoSch92 @xingyaoww @enyst