🛡️ Sentinel: [HIGH] Fix path traversal in manual path normalization#308
🛡️ Sentinel: [HIGH] Fix path traversal in manual path normalization#308bashandbone wants to merge 1 commit into
Conversation
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 guide (collapsed on small PRs)Reviewer's GuideFixes a path traversal vulnerability in the TypeScript dependency extractor’s manual path normalization and adds a Sentinel security note documenting the issue and its prevention. 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 new
ParentDirhandling duplicates theComponentvariants as fully-qualified paths; consider importingstd::path::Componentat the top of the file so the match arms and stack logic read more clearly. - It might be helpful to encode the intent of the new
ParentDirrules in a small helper function (e.g.,normalize_parent_dir_component(components, component)) to keep the main loop focused on flow and reduce the chance of subtle regressions if this logic needs to be updated again.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `ParentDir` handling duplicates the `Component` variants as fully-qualified paths; consider importing `std::path::Component` at the top of the file so the match arms and stack logic read more clearly.
- It might be helpful to encode the intent of the new `ParentDir` rules in a small helper function (e.g., `normalize_parent_dir_component(components, component)`) to keep the main loop focused on flow and reduce the chance of subtle regressions if this logic needs to be updated again.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
🚨 Severity
HIGH
💡 Vulnerability
A path traversal vulnerability existed in
crates/flow/src/incremental/extractors/typescript.rsduring manual module path resolution fallback. When iterating throughstd::path::Componententries and encounteringComponent::ParentDir(../), the code unconditionally popped the last component from the constructed path's component list. This allowed../components to pop root directories (Component::RootDir), prefixes (Component::Prefix), or erroneously ignored leading../sequences by popping from an empty stack, leading to unauthorized path traversal and incorrectly constructed relative paths.🎯 Impact
The application could have resolved paths outside of intended base directories or constructed corrupted internal module graphs, potentially leading to unauthorized file access during graph building and parsing phases.
🔧 Fix
Updated
Component::ParentDirhandling to explicitly check the top of the stack. It now blocksParentDirfrom poppingRootDirorPrefix. Additionally, it correctly preserves leading../sequences by explicitly pushingParentDirwhen the component list is empty or the last element is also aParentDir.✅ Verification
cargo +nightly fmtcargo clippy --workspace -- -D warningscargo test -p thread-flow --test extractor_typescript_tests.origor.rejpatch files remain.PR created automatically by Jules for task 14907076982299675260 started by @bashandbone
Summary by Sourcery
Fix unsafe manual path normalization in the TypeScript dependency extractor to prevent path traversal and document the incident in Sentinel metadata.
Bug Fixes:
../components from popping root or prefix segments during manual path normalization in the TypeScript extractor.Documentation: