Repository navigation
š”ļø Sentinel: [MEDIUM] Block function reflection attributes in ASTScanner - #290
Conversation
Enhance FORBIDDEN_ATTRIBUTES in xcore/kernel/security/section.py to block function reflection/introspection attributes (__closure__, __defaults__, __kwdefaults__, __func__, __self__, __module__) during AST scanning to prevent sandbox bypasses.
|
š Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a š emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
Reviewer's GuideThe PR hardens ASTScanner against function introspection by adding closure, default-value, bound-method, and module reflection attributes to FORBIDDEN_ATTRIBUTES, with a regression test confirming all six accesses are rejected and reported. Flow diagram for ASTScanner blocking function reflectionflowchart LR
Plugin[Untrusted plugin code] --> ASTScanner[ASTScanner]
ASTScanner --> Forbidden[FORBIDDEN_ATTRIBUTES]
Forbidden --> Reject[Reject attribute access]
Reject --> Report[Report security violation]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
š Deployed on https://docs-preview-290--candid-tanuki-92021a.netlify.app |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="xcore/kernel/security/section.py" line_range="31-36" />
<code_context>
"__globals__",
"__subclasses__",
"__code__",
+ "__closure__",
+ "__defaults__",
+ "__kwdefaults__",
+ "__func__",
+ "__self__",
+ "__module__",
"__mro__",
"__builtins__",
</code_context>
<issue_to_address>
**String helpers bypass attribute blocking**
When a plugin passes a newly forbidden reflection name to an `attrgetter` helper, `ASTScanner` checks only `ast.Attribute` names, so it misses names passed as strings to `operator.attrgetter` or `functools.attrgetter`. The scan passes and the plugin can still inspect function internals.
Have `ASTScanner` reject newly forbidden names passed to `attrgetter` helpers.
</issue_to_address>
### Comment 2
<location path="xcore/kernel/security/section.py" line_range="31-36" />
<code_context>
"__globals__",
"__subclasses__",
"__code__",
+ "__closure__",
+ "__defaults__",
+ "__kwdefaults__",
+ "__func__",
+ "__self__",
+ "__module__",
"__mro__",
"__builtins__",
</code_context>
<issue_to_address>
**Ephemeral plugins run after failed scans**
When an EPHEMERAL-mode plugin uses a newly forbidden attribute, `EphemeralActivator.activate` logs the scan errors and continues to `EphemeralHandler.start`, whose warm pool loads the plugin through the in-process `LifecycleManager`. The forbidden reflection access therefore runs in the kernel process.
Make `EphemeralActivator.activate` stop activation when the scan fails.
</issue_to_address>
### Comment 3
<location path="xcore/kernel/security/section.py" line_range="31-36" />
<code_context>
"__globals__",
"__subclasses__",
"__code__",
+ "__closure__",
+ "__defaults__",
+ "__kwdefaults__",
+ "__func__",
+ "__self__",
+ "__module__",
"__mro__",
"__builtins__",
</code_context>
<issue_to_address>
**Security guide omits blocked attributes**
The sandboxed-plugin guide lists `__subclasses__`, `__mro__`, `__globals__`, and `__builtins__` as blocked attributes but omits the six added here, so plugin authors relying on the guide do not learn that these accesses are rejected during scanning.
Update the sandboxed-plugin guideās blocked-attribute list to include the six new entries.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 2 findings to address first, and this adds a security policy decision that rejects any scanned code using these function attributes; if the denylist is wrong or incomplete, the behavior is wrong for every affected scan from the moment it ships, and an ordinary revert does not recover any denied deployment decisions already made.
Blocking findings: xcore/kernel/security/section.py:36, xcore/kernel/security/section.py:36
| "__closure__", | ||
| "__defaults__", | ||
| "__kwdefaults__", | ||
| "__func__", | ||
| "__self__", | ||
| "__module__", |
There was a problem hiding this comment.
š High Ā· String helpers bypass attribute blocking
When a plugin passes a newly forbidden reflection name to an attrgetter helper, ASTScanner checks only ast.Attribute names, so it misses names passed as strings to operator.attrgetter or functools.attrgetter. The scan passes and the plugin can still inspect function internals.
Have ASTScanner reject newly forbidden names passed to attrgetter helpers.
Prompt for AI agents
In `xcore/kernel/security/section.py` at lines 31-36:
**String helpers bypass attribute blocking**
When a plugin passes a newly forbidden reflection name to an `attrgetter` helper, `ASTScanner` checks only `ast.Attribute` names, so it misses names passed as strings to `operator.attrgetter` or `functools.attrgetter`. The scan passes and the plugin can still inspect function internals.
Have `ASTScanner` reject newly forbidden names passed to `attrgetter` helpers.| "__closure__", | ||
| "__defaults__", | ||
| "__kwdefaults__", | ||
| "__func__", | ||
| "__self__", | ||
| "__module__", |
There was a problem hiding this comment.
š High Ā· Ephemeral plugins run after failed scans
When an EPHEMERAL-mode plugin uses a newly forbidden attribute, EphemeralActivator.activate logs the scan errors and continues to EphemeralHandler.start, whose warm pool loads the plugin through the in-process LifecycleManager. The forbidden reflection access therefore runs in the kernel process.
Make EphemeralActivator.activate stop activation when the scan fails.
Prompt for AI agents
In `xcore/kernel/security/section.py` at lines 31-36:
**Ephemeral plugins run after failed scans**
When an EPHEMERAL-mode plugin uses a newly forbidden attribute, `EphemeralActivator.activate` logs the scan errors and continues to `EphemeralHandler.start`, whose warm pool loads the plugin through the in-process `LifecycleManager`. The forbidden reflection access therefore runs in the kernel process.
Make `EphemeralActivator.activate` stop activation when the scan fails.| "__closure__", | ||
| "__defaults__", | ||
| "__kwdefaults__", | ||
| "__func__", | ||
| "__self__", | ||
| "__module__", |
There was a problem hiding this comment.
āŖ Low Ā· Security guide omits blocked attributes
The sandboxed-plugin guide lists __subclasses__, __mro__, __globals__, and __builtins__ as blocked attributes but omits the six added here, so plugin authors relying on the guide do not learn that these accesses are rejected during scanning.
Update the sandboxed-plugin guideās blocked-attribute list to include the six new entries.
Prompt for AI agents
In `xcore/kernel/security/section.py` at lines 31-36:
**Security guide omits blocked attributes**
The sandboxed-plugin guide lists `__subclasses__`, `__mro__`, `__globals__`, and `__builtins__` as blocked attributes but omits the six added here, so plugin authors relying on the guide do not learn that these accesses are rejected during scanning.
Update the sandboxed-plugin guideās blocked-attribute list to include the six new entries.
1 participant
Summary
Added function reflection/introspection attributes (
__closure__,__defaults__,__kwdefaults__,__func__,__self__,__module__) toFORBIDDEN_ATTRIBUTESinxcore/kernel/security/section.py. This prevents untrusted plugins from inspecting function closures, default values, or bound method targets during AST scanning.Changes
FORBIDDEN_ATTRIBUTESinxcore/kernel/security/section.py.test_ast_scanner_blocks_reflection_forbidden_attributesintests/unit/security/test_sentinel_fixes.py.PR created automatically by Jules for task 9968355375119502117 started by @traoreera
Summary by Sourcery
Block function reflection attributes during AST scanning to strengthen plugin security.
Bug Fixes:
Tests: