fix: ls tool relative paths #7982
Conversation
dosubot
bot
added
the
size:M
|
💡 To request a new review, comment |
AI Code ReviewAI review failed due to service initialization issues. Please check the Continue API key and configuration. No specific line comments generated. 💡 To request a new detailed review, comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 1 file
Prompt for AI agents (all 3 issues)
Understand the root cause of the following 3 issues and fix them.
<file name="extensions/cli/src/tools/listFiles.ts">
<violation number="1" location="extensions/cli/src/tools/listFiles.ts:27">
Using includes("..") can cause false positives; prefer checking for parent traversal via startsWith("..") (or a project-root relative check).</violation>
<violation number="2" location="extensions/cli/src/tools/listFiles.ts:27">
Path validation allows absolute paths outside project and falsely blocks valid names containing ".."</violation>
<violation number="3" location="extensions/cli/src/tools/listFiles.ts:34">
Absolute paths can escape the project root; ensure dirPath stays within process.cwd() to prevent directory traversal.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| preprocess: async (args) => { | ||
| // Prevent "../" | ||
| const normalizedPath = path.normalize(args.dirpath); | ||
| if (normalizedPath.includes("..")) { |
There was a problem hiding this comment.
Using includes("..") can cause false positives; prefer checking for parent traversal via startsWith("..") (or a project-root relative check).
Prompt for AI agents
Address the following comment on extensions/cli/src/tools/listFiles.ts at line 27:
<comment>Using includes("..") can cause false positives; prefer checking for parent traversal via startsWith("..") (or a project-root relative check).</comment>
<file context>
@@ -22,28 +22,41 @@ export const listFilesTool: Tool = {
preprocess: async (args) => {
+ // Prevent "../"
+ const normalizedPath = path.normalize(args.dirpath);
+ if (normalizedPath.includes("..")) {
+ throw new Error(
+ 'For security reasons, cannot use "../" in dirPath. Stay within the project.',
</file context>
✅ Addressed in 3841322
| preprocess: async (args) => { | ||
| // Prevent "../" | ||
| const normalizedPath = path.normalize(args.dirpath); | ||
| if (normalizedPath.includes("..")) { |
There was a problem hiding this comment.
Path validation allows absolute paths outside project and falsely blocks valid names containing ".."
Prompt for AI agents
Address the following comment on extensions/cli/src/tools/listFiles.ts at line 27:
<comment>Path validation allows absolute paths outside project and falsely blocks valid names containing ".."</comment>
<file context>
@@ -22,28 +22,41 @@ export const listFilesTool: Tool = {
preprocess: async (args) => {
+ // Prevent "../"
+ const normalizedPath = path.normalize(args.dirpath);
+ if (normalizedPath.includes("..")) {
+ throw new Error(
+ 'For security reasons, cannot use "../" in dirPath. Stay within the project.',
</file context>
dosubot
bot
added
size:S
|
🎉 This PR is included in version 1.1.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 1.22.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 1.21.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Description
remake of #7795
Summary by cubic
Fixes listFiles tool to handle relative paths safely and predictably. Resolves dirpath against the current working directory, blocks "../", validates the directory early, and simplifies run.