Skip to content

🛡️ Sentinel: [CRITICAL/HIGH] Fix path traversal in manual path resolution#268

Open
bashandbone wants to merge 1 commit into
mainfrom
sentinel-fix-path-traversal-18235841697886059545
Open

🛡️ Sentinel: [CRITICAL/HIGH] Fix path traversal in manual path resolution#268
bashandbone wants to merge 1 commit into
mainfrom
sentinel-fix-path-traversal-18235841697886059545

Conversation

@bashandbone
Copy link
Copy Markdown
Contributor

@bashandbone bashandbone commented May 27, 2026

🚨 Severity: HIGH
💡 Vulnerability: Improper path normalization allowed ParentDir to 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:

  • Correct manual path normalization to prevent improper handling of parent directory components that could allow traversal outside the intended scope.

Documentation:

  • Add Sentinel security note documenting the path traversal vulnerability, its cause, and recommended prevention pattern.

…tion

Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com>
@google-labs-jules
Copy link
Copy Markdown
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 @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

Copilot AI review requested due to automatic review settings May 27, 2026 17:37
@sourcery-ai
Copy link
Copy Markdown
Contributor

sourcery-ai Bot commented May 27, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Updates the TypeScript dependency extractor’s manual path normalization logic to handle ParentDir components safely around roots/prefixes and stacked .., and documents the vulnerability and fix in Sentinel notes.

Flow diagram for updated ParentDir path normalization logic

flowchart 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]
Loading

File-Level Changes

Change Details Files
Harden manual path normalization against path traversal when encountering ParentDir components.
  • Replace unconditional components.pop() on ParentDir with logic that inspects the last component before modifying the stack
  • Preserve RootDir and Prefix components by ignoring ParentDir when they are the last element
  • Correctly handle leading or stacked ParentDir segments by pushing .. when the stack is empty or the last component is also ParentDir
  • Only pop the last component for ParentDir when it is a normal path segment that can be safely removed
crates/flow/src/incremental/extractors/typescript.rs
Document the discovered path traversal vulnerability and remediation in Sentinel notes.
  • Add a Sentinel markdown entry describing the original improper path normalization behavior
  • Record key learning on how RootDir, Prefix, and stacked ParentDir must be handled
  • Capture preventative guidelines for future manual path resolution using std::path::Component
.jules/sentinel.md

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've left some high level feedback:

  • 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.
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.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 for ParentDir in resolve_module_path.
  • Add .jules/sentinel.md documenting 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.

Comment thread .jules/sentinel.md
@@ -0,0 +1,4 @@
## 2024-05-27 - Path Traversal in Custom Path Normalization
Comment thread .jules/sentinel.md
@@ -0,0 +1,4 @@
## 2024-05-27 - Path Traversal in Custom Path Normalization
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants