🛡️ Sentinel: [CRITICAL/HIGH] Fix path traversal in file download#316
🛡️ Sentinel: [CRITICAL/HIGH] Fix path traversal in file download#316bashandbone wants to merge 1 commit into
Conversation
This commit fixes a path traversal vulnerability in the TypeScript extractor's manual path resolution logic. It ensures that `std::path::Component::ParentDir` resolution safely handles root, prefix, and leading parent directories without inadvertently erasing boundaries or swallowing components. Additionally, this commit creates the sentinel journal with learnings and includes a minor valid code clippy lint fix. Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com>
|
👋 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 GuideFixes a HIGH-severity path traversal bug in the TypeScript dependency extractor’s manual path normalization, and performs small lifetime/borrowing cleanups in rule-engine variable checking plus adds a Sentinel incident note. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The manual component normalization logic in the TypeScript extractor is now security-sensitive; consider extracting it into a small helper function with a brief comment about the invariants (never popping RootDir/Prefix, preserving leading ParentDir) to make future modifications safer and easier to reason about.
- In the
ParentDirhandling match arm, using ause std::path::Component;import and matching onComponent::...variants (possibly with amatches!guard) could improve readability and reduce repetition inside the nested match.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The manual component normalization logic in the TypeScript extractor is now security-sensitive; consider extracting it into a small helper function with a brief comment about the invariants (never popping RootDir/Prefix, preserving leading ParentDir) to make future modifications safer and easier to reason about.
- In the `ParentDir` handling match arm, using a `use std::path::Component;` import and matching on `Component::...` variants (possibly with a `matches!` guard) could improve readability and reduce repetition inside the nested match.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
🚨 Severity: HIGH
💡 Vulnerability: Path traversal in TypeScript module resolution fallback when canonicalize fails. Popping components indiscriminately via
ParentDirallowed potential traversal beyond root directories.🎯 Impact: Allows traversing outside of expected directories when resolving missing or virtual module files.
🔧 Fix: Prevented popping of
ParentDirwhen at root, prefix, or already aParentDir, ensuring consistent bounds checking and safepushing of relative path references.✅ Verification: Tested manual component resolution logic and ran test suites. Also validated against clippy and formatting checks.
PR created automatically by Jules for task 17825619782213999374 started by @bashandbone
Summary by Sourcery
Fix path traversal handling in the TypeScript dependency extractor and clean up rule engine variable checking signatures.
Bug Fixes:
Enhancements:
Documentation: