🛡️ Sentinel: [CRITICAL/HIGH] Fix path traversal in manual path resolution#268
🛡️ Sentinel: [CRITICAL/HIGH] Fix path traversal in manual path resolution#268bashandbone wants to merge 1 commit into
Conversation
…tion 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 GuideUpdates the TypeScript dependency extractor’s manual path normalization logic to handle Flow diagram for updated ParentDir path normalization logicflowchart TD
A[Encounter Component::ParentDir] --> B[Get last from components]
B --> C{last is RootDir or Prefix}
C -- Yes --> D[Do nothing]
C -- No --> E{last is ParentDir or None}
E -- Yes --> F[Push ParentDir onto components]
E -- No --> G[Pop last component from components]
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:
- Consider extracting the
ParentDirhandling into a small helper function with a clear name (e.g.,normalize_parent_dir_component) so the normalization rules (root/prefix, stacked.., etc.) are easier to read and reuse if similar logic is needed elsewhere. - It may be worth adding an inline comment near the
RootDir/Prefixbranch to briefly explain whyParentDiris ignored in those cases, since this behavior is security-sensitive and non-obvious at first glance.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider extracting the `ParentDir` handling into a small helper function with a clear name (e.g., `normalize_parent_dir_component`) so the normalization rules (root/prefix, stacked `..`, etc.) are easier to read and reuse if similar logic is needed elsewhere.
- It may be worth adding an inline comment near the `RootDir`/`Prefix` branch to briefly explain why `ParentDir` is ignored in those cases, since this behavior is security-sensitive and non-obvious at first glance.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull request overview
Fixes a path traversal weakness in the TypeScript dependency extractor's manual path normalization fallback. The fallback (used when canonicalize() fails, e.g., for missing files) previously called components.pop() unconditionally on ParentDir, which could drop RootDir/Prefix segments or silently collapse stacked leading .. components, potentially resolving outside the intended scope. The new logic preserves root/prefix and pushes .. when the stack is empty or already ends with ...
Changes:
- Replace unconditional
components.pop()with last-component-aware handling forParentDirinresolve_module_path. - Add
.jules/sentinel.mddocumenting the vulnerability, root cause, and prevention pattern.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| crates/flow/src/incremental/extractors/typescript.rs | Component-by-component normalization that preserves RootDir/Prefix and retains stacked leading ... |
| .jules/sentinel.md | New Sentinel learning note describing the path traversal issue and prevention guidance. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1,4 @@ | |||
| ## 2024-05-27 - Path Traversal in Custom Path Normalization | |||
| @@ -0,0 +1,4 @@ | |||
| ## 2024-05-27 - Path Traversal in Custom Path Normalization | |||
🚨 Severity: HIGH
💡 Vulnerability: Improper path normalization allowed
ParentDirto improperly discard leading paths or ignore root.🎯 Impact: Attackers could potentially traverse paths outside the intended scope if symlinks or missing files are involved.
🔧 Fix: Implemented correct component-by-component popping that respects RootDir, Prefix, and stacked ParentDir.
✅ Verification: Verified with
cargo test.PR created automatically by Jules for task 18235841697886059545 started by @bashandbone
Summary by Sourcery
Fix path traversal vulnerability in TypeScript dependency path normalization and document the issue in Sentinel notes.
Bug Fixes:
Documentation: