🛡️ Sentinel: [CRITICAL] Fix path traversal in TypeScript module resolution#282
Open
bashandbone wants to merge 1 commit into
Open
🛡️ Sentinel: [CRITICAL] Fix path traversal in TypeScript module resolution#282bashandbone wants to merge 1 commit into
bashandbone wants to merge 1 commit into
Conversation
…ution This commit resolves a critical path traversal vulnerability in the TypeScript extractor's `resolve_import_path` method. When `canonicalize` falls back to manual path normalization, the previous code unconditionally popped path components on `ParentDir` (`..`). This behavior was unsafe, as it could erroneously pop structural roots (like `RootDir` or `Prefix`) or drop valid preceding `..` references when resolving inherently relative paths (e.g. `../../a`). A robust match on the last component is now employed to ensure that path traversal sequences are correctly accumulated or removed without escaping the legitimate hierarchy constraints. A new entry has also been created in the `.jules/sentinel.md` journal capturing this security learning. 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 GuideFixes a critical path traversal vulnerability in the TypeScript extractor’s manual path normalization by making ParentDir handling component-aware, and includes minor Rust formatting/clarity tweaks plus a new Sentinel security journal entry. Flow diagram for secure ParentDir handling in TypeScript path normalizationflowchart TD
A[Iterate resolved.components] --> B{component is ParentDir?}
B -->|No| C["components.push(component)"]
B -->|Yes| D{components.last exists?}
D -->|No| E["components.push(ParentDir)"]
D -->|Yes| F{last is Normal?}
F -->|Yes| G["components.pop"]
F -->|No| H{last is ParentDir?}
H -->|Yes| I["components.push(ParentDir)"]
H -->|No| J["Do nothing<br/>// keep RootDir or Prefix"]
C --> K[Next component]
E --> K
G --> K
I --> K
J --> K
K --> L{More components?}
L -->|Yes| A
L -->|No| M["Normalized components ready"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens the TypeScript dependency extractor’s relative import path normalization to prevent path traversal when canonicalize() fails (e.g., for non-existent paths), and records the incident in the Jules Sentinel journal.
Changes:
- Fix manual
..handling in TypeScript module path resolution to avoid poppingRootDir/WindowsPrefixand to preserve consecutive..sequences. - Apply minor formatting/readability changes in the rule engine and AST engine code.
- Add a Sentinel journal entry documenting the vulnerability and prevention guidance.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/flow/src/incremental/extractors/typescript.rs | Harden fallback path normalization for relative imports to prevent traversal above root/prefix. |
| crates/rule-engine/src/rule/referent_rule.rs | Formatting-only change to condense a chained read/clone call. |
| crates/rule-engine/src/rule/mod.rs | Formatting-only change to improve readability of defined_vars() collection. |
| crates/ast-engine/src/tree_sitter/mod.rs | Formatting-only changes in UTF-8 fallback handling and a test assertion. |
| .jules/sentinel.md | Add Sentinel journal entry describing the vulnerability and mitigation guidance. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1,4 @@ | |||
| ## 2025-06-01 - Path Traversal Vulnerability in TypeScript Import Resolution | |||
| **Vulnerability:** A path traversal vulnerability existed in the manual path normalization logic (`resolved.components()`) inside the TypeScript extractor's `resolve_import_path` when `canonicalize` fails. The code un-conditionally popped the last path component when encountering `std::path::Component::ParentDir`, which could erroneously pop `RootDir` (`/`) or `Prefix` (`C:\`), or incorrectly remove a legitimate preceding `../` in paths like `../../a`. | |||
| @@ -0,0 +1,4 @@ | |||
| ## 2025-06-01 - Path Traversal Vulnerability in TypeScript Import Resolution | |||
Comment on lines
+811
to
+827
| // 🛡️ SECURITY: Prevent path traversal by explicitly blocking Component::ParentDir | ||
| // from popping RootDir/Prefix. Also correctly handle when the component list | ||
| // is empty or already ends with ParentDir to preserve paths like ../../a | ||
| if let Some(last) = components.last() { | ||
| match last { | ||
| std::path::Component::Normal(_) => { | ||
| components.pop(); | ||
| } | ||
| std::path::Component::ParentDir => { | ||
| components.push(component); | ||
| } | ||
| // Don't pop RootDir or Prefix | ||
| _ => {} | ||
| } | ||
| } else { | ||
| components.push(component); | ||
| } |
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
A path traversal vulnerability existed in the manual path normalization logic (
resolved.components()) inside the TypeScript extractor'sresolve_import_path. When systemcanonicalizefails (which happens when pointing to non-existent files), the fallback manual normalization unconditionally.pop()'d the path component list upon encountering astd::path::Component::ParentDir(..). This allowed it to maliciously pop theRootDir(/), WindowsPrefix(C:\), or overwrite valid preceding..references in naturally relative paths.🎯 Impact
If
resolve_import_pathprocesses user-influenced input or aggressively nested..module paths without validation, it could result in escaping the target directory bounds and traversing into unintended directories on the host file system.🔧 Fix
Replaced the unconditional
.pop()with a pattern-matching verification against the.last()element of the components list.ParentDir(..) is prevented from popping foundational components likeRootDirorPrefix.ParentDir, the newParentDiris appended to correctly accumulate sequences like../../dir..pop()when it is safely matched against aNormalfile/folder path component..jules/sentinel.md.✅ Verification
cargo test -p thread-flow --test extractor_typescript_testsand all 25 tests passed successfully.cargo +nightly fmt..jules/sentinel.mdjournal per project specifications.PR created automatically by Jules for task 12883902628681392337 started by @bashandbone
Summary by Sourcery
Fix unsafe path normalization in the TypeScript dependency extractor to prevent path traversal and record the incident in the Sentinel security journal.
Bug Fixes:
Enhancements:
Documentation: