Skip to content

fix: address 3 critical security issues (fixes #15, #16, #17)#19

Open
SuperInstance wants to merge 1 commit intomainfrom
greenhorn/security-fixes
Open

fix: address 3 critical security issues (fixes #15, #16, #17)#19
SuperInstance wants to merge 1 commit intomainfrom
greenhorn/security-fixes

Conversation

@SuperInstance
Copy link
Copy Markdown
Owner

@SuperInstance SuperInstance commented Apr 12, 2026

Summary

#15: Zero Bytecode Verification

  • Added verify_bytecode() static method to Interpreter class
  • Rejects empty bytecode with clear ValueError
  • Rejects bytecode exceeding 1 MB (MAX_BYTECODE_SIZE)
  • Verification runs at the top of execute() before the fetch-decode loop

#16: CAP Opcodes Unenforced

  • Added capabilities: set[int] attribute to the VM interpreter
  • CAP_GRANT(reg): adds capability ID to the set, writes 0 (success) to R0
  • CAP_REVOKE(reg): removes capability ID from the set, writes 0 (success) to R0
  • CAP_REQUIRE(reg): checks if capability is in the set; raises VMA2AError if missing
  • CAP_REQUEST(reg): forwarded to external A2A handler (no auto-grant)
  • Capabilities cleared on reset()

#17: NaN Trust Poison

  • Added NaN detection via math.isnan() in record_interaction() for capability_match, behavior_signature, and latency_ms
  • Added NaN detection in check_trust() for threshold
  • All NaN values raise ValueError with descriptive message
  • capability_match clamped to [0.0, 1.0] on input
  • threshold clamped to [0.0, 1.0] on input

Testing

  • 14 new regression tests in tests/test_security.py
  • Full suite: 2378 passed, 0 failed, 50 warnings
  • No existing tests broken

Files Changed

  • src/flux/vm/interpreter.py — bytecode verification + CAP enforcement
  • src/flux/a2a/trust.py — NaN protection + clamping
  • tests/test_security.py — regression tests for all 3 fixes

Staging: Open in Devin

Copy link
Copy Markdown

@beta-devin-ai-integration beta-devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 potential issue.

View 4 additional findings in Devin Review.

Staging: Open in Devin

Comment thread src/flux/a2a/trust.py
Comment on lines +151 to +154
Raises
------
ValueError
If *capability_match* or *behavior_signature* is NaN.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Docstring omits latency_ms from Raises section despite NaN check in code

The record_interaction docstring's Raises section states If *capability_match* or *behavior_signature* is NaN, but the code at src/flux/a2a/trust.py:165-168 also raises ValueError when latency_ms is NaN. A caller relying on the docstring to understand which parameters are validated would not know that NaN latency_ms also raises. The test test_nan_latency_raises at tests/test_security.py:199-207 confirms this behavior exists, making the docstring inaccurate.

Suggested change
Raises
------
ValueError
If *capability_match* or *behavior_signature* is NaN.
Raises
------
ValueError
If *capability_match*, *behavior_signature*, or *latency_ms* is NaN.
Staging: Open in Devin

Was this helpful? React with 👍 or 👎 to provide feedback.

Debug

Playground

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