Skip to content

feat: add RailException support and improve error handling #1178

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 13, 2025

Conversation

Pouyanpi
Copy link
Collaborator

@Pouyanpi Pouyanpi commented May 7, 2025

  • implement RailException for injection detection (a must have for checks)
  • Add TypedDict for structured return values
  • improve error handling for malformed YARA rules

Summary by GH Copilot

This pull request enhances the injection detection system by introducing a structured return type, improving error handling, and adding new tests for edge cases. The changes aim to make the system more robust and easier to integrate into workflows.

Code Enhancements

  • Introduced a TypedDict named InjectionDetectionResult to standardize the return type for injection detection functions, containing is_injection, text, and detections fields. (nemoguardrails/library/injection_detection/actions.py, nemoguardrails/library/injection_detection/actions.pyR52-R57)
  • Updated _omit_injection, _sanitize_injection, and _reject_injection functions to return structured tuples or lists for better clarity and integration. (nemoguardrails/library/injection_detection/actions.py, [1] [2] [3]

Improved Error Handling

Flow Updates

  • Updated the injection detection flow logic in .co files to use the new InjectionDetectionResult format, improving clarity and handling of different actions (reject, omit, sanitize). (nemoguardrails/library/injection_detection/flows.co, [1]; nemoguardrails/library/injection_detection/flows.v1.co, [2]

Testing Improvements

  • Added new test cases to validate edge scenarios, such as exceptions for specific actions, malformed YARA rules, and behavior when exceptions are enabled. (tests/test_injection_detection.py, tests/test_injection_detection.pyR623-R768)
  • Refactored existing tests to align with the updated function signatures and return types. (tests/test_injection_detection.py, [1] [2]

- Add TypedDict for structured return values
- implement RailException for injection detection (a must have for checks)
- improve error handling for malformed YARA rules
@Pouyanpi Pouyanpi requested a review from Copilot May 7, 2025 09:57
@Pouyanpi Pouyanpi self-assigned this May 7, 2025
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request enhances the injection detection system by introducing structured return values using a new TypedDict and improving error handling for malformed YARA rules.

  • Introduced InjectionDetectionResult for structured detection output
  • Updated injection detection functions (_omit_injection, _sanitize_injection, _reject_injection) to return tuples for consistency
  • Improved error handling in _load_rules to log YARA compilation errors gracefully and updated tests accordingly

Reviewed Changes

Copilot reviewed 2 out of 4 changed files in this pull request and generated 1 comment.

File Description
tests/test_injection_detection.py Updated tests to match new injection detection return types and added cases for RailException handling and graceful failure for malformed YARA rules.
nemoguardrails/library/injection_detection/actions.py Added InjectionDetectionResult TypedDict, updated injection detection functions to return tuples, and improved error messaging in _load_rules.
Files not reviewed (2)
  • nemoguardrails/library/injection_detection/flows.co: Language not supported
  • nemoguardrails/library/injection_detection/flows.v1.co: Language not supported

@Pouyanpi Pouyanpi added this to the v0.14.0 milestone May 7, 2025
@codecov-commenter
Copy link

codecov-commenter commented May 7, 2025

Codecov Report

Attention: Patch coverage is 87.17949% with 5 lines in your changes missing coverage. Please review.

Project coverage is 68.44%. Comparing base (b65cf0e) to head (74ec6c0).

Files with missing lines Patch % Lines
...oguardrails/library/injection_detection/actions.py 87.17% 5 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1178      +/-   ##
===========================================
+ Coverage    68.24%   68.44%   +0.19%     
===========================================
  Files          161      161              
  Lines        15938    15948      +10     
===========================================
+ Hits         10877    10915      +38     
+ Misses        5061     5033      -28     
Flag Coverage Δ
python 68.44% <87.17%> (+0.19%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...oguardrails/library/injection_detection/actions.py 90.07% <87.17%> (+9.91%) ⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Pouyanpi Pouyanpi requested a review from erickgalinkin May 7, 2025 10:14
@Pouyanpi Pouyanpi requested review from erickgalinkin and removed request for erickgalinkin May 7, 2025 11:18
Copy link
Collaborator

@erickgalinkin erickgalinkin left a comment

Choose a reason for hiding this comment

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

This looks good to me, thanks!

@Pouyanpi Pouyanpi requested a review from cparisien May 9, 2025 10:50
@Pouyanpi Pouyanpi merged commit 7e60dda into develop May 13, 2025
39 checks passed
@Pouyanpi Pouyanpi deleted the feat/injection-detection-exceptions branch May 13, 2025 19:11
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.

4 participants