🛡️ Sentinel: [HIGH] Fix Path Traversal in TypeScript Module Resolution#290
🛡️ Sentinel: [HIGH] Fix Path Traversal in TypeScript Module Resolution#290bashandbone wants to merge 1 commit into
Conversation
Update manual path resolution logic in `typescript.rs` to correctly handle `Component::ParentDir`. Prevent popping past `RootDir` or `Prefix`, and ensure `ParentDir` is pushed if the components list is empty or its last element is `ParentDir`. Also adds a new journal entry and elides needlessly explicit lifetimes found by `clippy`. 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 path traversal vulnerability in the TypeScript dependency extractor’s manual path normalization while also applying minor refactors/formatting cleanups in AST and rule engine modules and adding a Sentinel incident note. Flow diagram for updated ParentDir handling in TypeScript path normalizationflowchart TD
A[Component is ParentDir] --> B{components.last}
B -->|None| C[Push ParentDir onto components]
B -->|ParentDir| C
B -->|RootDir or Prefix| D[Do nothing]
B -->|Other component| E[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:
- The manual path normalization logic around
Component::ParentDiris security-sensitive; consider extracting it into a small helper with clearly defined invariants so any future changes to module resolution reuse the same, vetted normalization behavior.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The manual path normalization logic around `Component::ParentDir` is security-sensitive; consider extracting it into a small helper with clearly defined invariants so any future changes to module resolution reuse the same, vetted normalization behavior.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
This PR is primarily a security hardening change in thread-flow’s TypeScript module resolution to address a reported path traversal issue during manual path normalization, alongside small refactors/formatting adjustments and a new Sentinel note documenting the finding.
Changes:
- Harden manual
../normalization logic in TypeScript dependency resolution to avoid popping pastRootDir/Prefix. - Small refactors/formatting tweaks in
rule-engineandast-engine. - Add a Sentinel security note documenting the vulnerability and prevention strategy.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/rule-engine/src/rule/referent_rule.rs | Minor formatting change in Registration::read() implementation. |
| crates/rule-engine/src/rule/mod.rs | Formatting-only change for readability in Rule::defined_vars(). |
| crates/rule-engine/src/check_var.rs | Simplify helper signatures by removing unnecessary lifetimes. |
| crates/flow/src/incremental/extractors/typescript.rs | Update manual path normalization for ParentDir handling in TS module resolution. |
| crates/ast-engine/src/tree_sitter/mod.rs | Small formatting/readability tweaks and test assertion formatting. |
| .jules/sentinel.md | New Sentinel entry documenting the path traversal issue and mitigation approach. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| std::path::Component::ParentDir => { | ||
| components.pop(); | ||
| // Prevent path traversal outside root | ||
| match components.last() { | ||
| Some(std::path::Component::ParentDir) | None => { | ||
| components.push(component); |
| @@ -0,0 +1,4 @@ | |||
| ## 2026-06-05 - [Path Traversal in Manual Path Normalization] | |||
🚨 Severity: HIGH
💡 Vulnerability: Path traversal vulnerability during manual path normalization in
thread-flow's TypeScript module resolution. The manual fallback for path resolution incorrectly popped path components when encounteringParentDir(../) without checking if the previous component was aRootDir(/),Prefix(e.g.,C:), or if the components list was empty, allowing path traversal outside the intended base directory.🎯 Impact: An attacker could potentially resolve modules outside of the intended project root, leading to unauthorized file access or analysis of sensitive files outside the workspace.
🔧 Fix: When manually normalizing paths with
std::path::Component, explicitly blockComponent::ParentDirfrom poppingComponent::RootDirorComponent::Prefix. If the components list is empty or its last element isComponent::ParentDir, the newComponent::ParentDiris pushed rather than ignored to correctly preserve relative paths like../../a.✅ Verification: Verified by running
cargo test -p thread-flow --test extractor_typescript_testsand checking the entire workspace withcargo +nightly fmtandcargo clippy --workspace -- -D warnings.PR created automatically by Jules for task 10141229190092945309 started by @bashandbone
Summary by Sourcery
Fix TypeScript module resolution to prevent path traversal outside the project root and make small refactors and documentation updates across related crates.
Bug Fixes:
Enhancements:
Documentation: