🛡️ Sentinel: [CRITICAL] Fix arbitrary code execution in dependency container#359
Open
bashandbone wants to merge 1 commit into
Open
🛡️ Sentinel: [CRITICAL] Fix arbitrary code execution in dependency container#359bashandbone wants to merge 1 commit into
bashandbone wants to merge 1 commit into
Conversation
…Call resolution to a strict whitelist. Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com>
Contributor
|
👋 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. |
Contributor
Reviewer's guide (collapsed on small PRs)Reviewer's GuideTightens AST-based type string evaluation in the DI container to prevent arbitrary code execution by restricting allowed function calls and documenting the security fix in the Sentinel log. Sequence diagram for secured AST type evaluation in DI containersequenceDiagram
actor Configurator
participant Container as DIContainer
participant TypeValidator
participant ast
Configurator->>DIContainer: _safe_eval_type(type_string)
DIContainer->>ast: parse(type_string, mode="eval")
ast-->>DIContainer: ast.Expression
DIContainer->>TypeValidator: visit(ast.Expression)
loop walk_nodes
TypeValidator->>TypeValidator: generic_visit(node)
alt node is ast.Call
TypeValidator->>TypeValidator: check node.func.id in {Depends, depends, Field, PrivateAttr, Tag}
alt allowed wrapper
TypeValidator-->>TypeValidator: continue traversal
else forbidden call
TypeValidator-->>DIContainer: raise TypeError("Forbidden function call in type string")
DIContainer-->>Configurator: reject type string
end
else other allowed node types
TypeValidator-->>TypeValidator: continue traversal
else forbidden node type
TypeValidator-->>DIContainer: raise TypeError("Forbidden AST node in type string")
DIContainer-->>Configurator: reject type string
end
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
|
🤖 Hi @bashandbone, I've received your request, and I'm working on it now! You can track my progress in the logs for more details. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
|
🤖 I'm sorry @bashandbone, but I was unable to process your request. Please see the logs for more details. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens CodeWeaver’s DI container string-type evaluation (_safe_eval_type) to prevent arbitrary function execution when resolving annotations from strings, and records the incident in Sentinel notes.
Changes:
- Restricts
ast.Callduring AST validation to an explicit allowlist of wrapper functions used in dependency/type metadata. - Removes unconditional allowance of
ast.Call/ast.keywordfrom the generic allowed-node list and handles them explicitly. - Documents the vulnerability, learning, and prevention guidance in
.jules/sentinel.md.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/codeweaver/core/di/container.py |
Tightens AST validation for string annotation evaluation by allowlisting specific call wrappers. |
.jules/sentinel.md |
Adds a Sentinel entry documenting the unsafe AST evaluation RCE/ACE issue and mitigation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+114
to
+117
| if isinstance(node, ast.Call): | ||
| # Restricting arbitrary function calls during type evaluation prevents ACE vulnerabilities | ||
| if not isinstance(node.func, ast.Name) or node.func.id not in {"Depends", "depends", "Field", "PrivateAttr", "Tag"}: | ||
| raise TypeError(f"Forbidden function call in type string: {node.func.id if isinstance(node.func, ast.Name) else type(node.func).__name__}") |
Comment on lines
+114
to
+117
| if isinstance(node, ast.Call): | ||
| # Restricting arbitrary function calls during type evaluation prevents ACE vulnerabilities | ||
| if not isinstance(node.func, ast.Name) or node.func.id not in {"Depends", "depends", "Field", "PrivateAttr", "Tag"}: | ||
| raise TypeError(f"Forbidden function call in type string: {node.func.id if isinstance(node.func, ast.Name) else type(node.func).__name__}") |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🚨 Severity: CRITICAL
💡 Vulnerability: The
_safe_eval_typefunction allowed arbitraryast.Callexecution during Pydantic/dependency string type resolution (Arbitrary Code Execution viaeval()).🎯 Impact: Attackers who could control a configuration that maps to a type string or those manipulating dependency annotations could execute arbitrary callables directly from the OS, bypassing access controls.
🔧 Fix: Removed
ast.Callfrom the global unconditionally allowed list inTypeValidator.generic_visit. Implemented a targeted allowlist allowing only explicitly required dependency wrappers (Depends,depends,Field,PrivateAttr,Tag).✅ Verification: Ran
mise //:teston thetests/unit/core/and the system passed successfully without regressions. Tested the local exploit bypassing the ast logic and it threw a forbidden exception safely.PR created automatically by Jules for task 3732493649190943774 started by @bashandbone
Summary by Sourcery
Harden AST-based type string evaluation in the dependency injection container to prevent arbitrary function execution and document the security fix in Sentinel notes.
Bug Fixes:
Documentation: