Skip to content

🛡️ Sentinel: [CRITICAL] Fix command injection vulnerability with execSync usage #12

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
praxstack wants to merge 2 commits into
base: main
Choose a base branch
from
Open

🛡️ Sentinel: [CRITICAL] Fix command injection vulnerability with execSync usage #12

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
994b95b
Fix command injection vulnerability by substituting execSync with exe…
google-labs-jules[bot] Mar 4, 2026
b9fae8d
Fix command injection vulnerability by substituting execSync with exe…
google-labs-jules[bot] Mar 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions bin/ai-review.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#!/usr/bin/env node

const { execSync, spawn, execFileSync } = require('child_process');
const { spawn, execFileSync } = require('child_process');
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 4, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Update integration test expectation after removing execSync.

This file no longer contains execSync, but Line 230 in run-tests.js still checks binContent.includes('execSync'), which will make E1-1 fail after this change set.

Suggested fix (outside this file) 
- if (binContent.includes('export-ai-review.js') && binContent.includes('execSync')) {
+ if (binContent.includes('export-ai-review.js') && (binContent.includes('execFileSync') || binContent.includes('spawn'))) {
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@bin/ai-review.js` at line 3, The integration test in run-tests.js currently
asserts binContent.includes('execSync') which will fail because bin/ai-review.js
now imports execFileSync instead; update the test logic in run-tests.js to check
for the actual symbol present (e.g., replace the includes('execSync') assertion
with includes('execFileSync') or otherwise assert on the presence of
'execFileSync' or 'spawn' via the binContent variable) so the test matches the
current ai-review.js exports.

const path = require('path');
const fs = require('fs');

Expand Down Expand Up @@ -75,7 +75,7 @@

function checkGitRepo() {
try {
execSync('git rev-parse --git-dir', { stdio: 'ignore' });
execFileSync('git', ['rev-parse', '--git-dir'], { stdio: 'ignore' });
return true;
} catch (error) {
console.error('❌ Error: Not a git repository');
Expand Down Expand Up @@ -110,9 +110,9 @@
const url = `http://localhost:${port}`;
console.log(`\n🌐 Opening browser: ${url}`);

const start = (process.platform == 'darwin' ? 'open' : process.platform == 'win32' ? 'start' : 'xdg-open');
const start = (process.platform === 'darwin' ? 'open' : process.platform === 'win32' ? 'start' : 'xdg-open');
try {
execSync(`${start} ${url}`, { stdio: 'ignore' });
execFileSync(start, [url], { stdio: 'ignore' });
} catch (error) {
Comment on lines +113 to 116
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 4, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🌐 Web query:

Node.js execFileSync Windows start command shell builtin

💡 Result:

start is a cmd.exe built-in, not an actual executable, so execFileSync('start', ...) fails on Windows unless you run it through a shell / cmd.exe. Also, when the target path is quoted, start treats the first quoted string as the window title, so you usually must pass an empty title ("") first. [1][2][3]

Use one of these patterns:

1) execFileSync via cmd.exe

const { execFileSync } = require('node:child_process');

execFileSync('cmd.exe', ['/d', '/s', '/c', 'start', '""', 'C:\\Path With Spaces\\file.txt'], {
  windowsHide: true,
  stdio: 'ignore',
});
  • /c = run command then exit (so Node doesn’t hang on an interactive shell). [2]

2) execSync (string form; simplest)

const { execSync } = require('node:child_process');

execSync('start "" "C:\\Path With Spaces\\file.txt"', { shell: 'cmd.exe', stdio: 'ignore' });

Notes

  • If you omit the empty title and your path is quoted, start may interpret (part of) the path as the title instead. [3]
  • execFileSync does not spawn a shell by default; enabling shell: true (or explicitly running cmd.exe /c ...) is how you access shell built-ins. [1]

Sources: Node.js child_process docs [1]; explanation that start is a shell builtin + need "" title [2]; start quoting/title behavior [3].

🏁 Script executed:

# First, let's examine the actual code in question
head -n 120 bin/ai-review.js | tail -n 15

Repository: PrakharMNNIT/ai-visual-code-review

Length of output: 600

🏁 Script executed:

# Check if there are any other places in the codebase that handle Windows start command
rg -i "start.*url|execFileSync.*win" --type js

Repository: PrakharMNNIT/ai-visual-code-review

Length of output: 151

🏁 Script executed:

# Check the imports at the top of bin/ai-review.js to confirm execFileSync usage
head -n 30 bin/ai-review.js | grep -E "require|import|execFileSync"

Repository: PrakharMNNIT/ai-visual-code-review

Length of output: 191

--open on Windows will always hit the fallback path.

start is a cmd.exe builtin, not an actual executable, so execFileSync('start', ...) fails on Windows without a shell wrapper. The current code will always catch the error and display the fallback message instead of opening the browser.

Use execFileSync('cmd', ['/c', 'start', '', url], { stdio: 'ignore' }) for Windows, or a cross-platform opener utility.

Suggested fix 
-      const start = (process.platform === 'darwin' ? 'open' : process.platform === 'win32' ? 'start' : 'xdg-open');
       try {
-        execFileSync(start, [url], { stdio: 'ignore' });
+        if (process.platform === 'win32') {
+          execFileSync('cmd', ['/c', 'start', '', url], { stdio: 'ignore' });
+        } else {
+          const opener = process.platform === 'darwin' ? 'open' : 'xdg-open';
+          execFileSync(opener, [url], { stdio: 'ignore' });
+        }
       } catch (error) {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const start = (process.platform === 'darwin' ? 'open' : process.platform === 'win32' ? 'start' : 'xdg-open');
try {
execSync(`${start} ${url}`, { stdio: 'ignore' });
execFileSync(start, [url], { stdio: 'ignore' });
} catch (error) {
try {
if (process.platform === 'win32') {
execFileSync('cmd', ['/c', 'start', '', url], { stdio: 'ignore' });
} else {
const opener = process.platform === 'darwin' ? 'open' : 'xdg-open';
execFileSync(opener, [url], { stdio: 'ignore' });
}
} catch (error) {
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@bin/ai-review.js` around lines 113 - 116, The Windows branch currently tries
to call the cmd builtin via execFileSync(start, [url], ...) which always fails;
update the logic around the start variable and the execFileSync invocation in
the block using execFileSync to use the proper Windows invocation
(execFileSync('cmd', ['/c', 'start', '', url], { stdio: 'ignore' })) when
process.platform === 'win32', or alternatively swap to a cross-platform opener
utility; ensure the non-Windows branches still call execFileSync(start, [url], {
stdio: 'ignore' }) and remove the shell-less attempt to run 'start' directly so
the catch block is only hit on real failures.

console.log('💡 Could not open browser automatically');
console.log(' Please open manually:', url);
Expand Down Expand Up @@ -206,7 +206,7 @@
if (i + 1 < args.length) {
port = parseInt(args[i + 1]);
i++; // Skip next argument
if (isNaN(port) || port < 1 || port > 65535) {

Check warning on line 209 in bin/ai-review.js

View workflow job for this annotation

GitHub Actions / test (20.x) 

Blocks are nested too deeply (5). Maximum allowed is 4

Check warning on line 209 in bin/ai-review.js

View workflow job for this annotation

GitHub Actions / test (18.x) 

Blocks are nested too deeply (5). Maximum allowed is 4
console.error('❌ Invalid port number');
process.exit(1);
}
Expand Down
3,815 changes: 3,815 additions & 0 deletions pnpm-lock.yaml
View file
Open in desktop

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions run-tests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

const { execSync } = require('child_process');
const fs = require('fs');
const path = require('path');

Check warning on line 10 in run-tests.js

View workflow job for this annotation

GitHub Actions / test (20.x) 

'path' is assigned a value but never used

Check warning on line 10 in run-tests.js

View workflow job for this annotation

GitHub Actions / test (18.x) 

'path' is assigned a value but never used

// Test results
const results = {
Expand Down Expand Up @@ -113,7 +113,7 @@

// Test pattern matching logic directly
console.log('Test: Pattern matching function');
const exportScript = require('./scripts/export-ai-review.js');

Check warning on line 116 in run-tests.js

View workflow job for this annotation

GitHub Actions / test (20.x) 

'exportScript' is assigned a value but never used

Check warning on line 116 in run-tests.js

View workflow job for this annotation

GitHub Actions / test (18.x) 

'exportScript' is assigned a value but never used
// This test requires examining the matchPattern function
console.log('⏭️ SKIP: Direct pattern matching (requires module exports)');
results.skipped.push('B1: Pattern matching unit test');
Expand Down Expand Up @@ -270,7 +270,7 @@
}

// Summary
console.log('\n' + '='.repeat(60));
console.log(`\n${ '='.repeat(60)}`);
console.log('📊 TEST RESULTS SUMMARY');
console.log('='.repeat(60));
console.log(`✅ Passed: ${results.passed.length}`);
Expand All @@ -293,7 +293,7 @@
results.skipped.forEach(test => console.log(` - ${test}`));
}

console.log('\n' + '='.repeat(60));
console.log(`\n${ '='.repeat(60)}`);

if (results.failed.length === 0) {
console.log('✅ ALL TESTS PASSED! Ready for deployment.');
Expand Down
22 changes: 11 additions & 11 deletions scripts/export-ai-review.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@
* Supports include/exclude file patterns
*/

const { execSync, execFileSync } = require('child_process');
const { execFileSync } = require('child_process');
const { writeFileSync, mkdirSync, existsSync } = require('fs');

Check warning on line 10 in scripts/export-ai-review.js

View workflow job for this annotation

GitHub Actions / test (20.x) 

'existsSync' is assigned a value but never used

Check warning on line 10 in scripts/export-ai-review.js

View workflow job for this annotation

GitHub Actions / test (20.x) 

'mkdirSync' is assigned a value but never used

Check warning on line 10 in scripts/export-ai-review.js

View workflow job for this annotation

GitHub Actions / test (20.x) 

'writeFileSync' is assigned a value but never used

Check warning on line 10 in scripts/export-ai-review.js

View workflow job for this annotation

GitHub Actions / test (18.x) 

'existsSync' is assigned a value but never used

Check warning on line 10 in scripts/export-ai-review.js

View workflow job for this annotation

GitHub Actions / test (18.x) 

'mkdirSync' is assigned a value but never used

Check warning on line 10 in scripts/export-ai-review.js

View workflow job for this annotation

GitHub Actions / test (18.x) 

'writeFileSync' is assigned a value but never used
const path = require('path');

Check warning on line 11 in scripts/export-ai-review.js

View workflow job for this annotation

GitHub Actions / test (20.x) 

'path' is assigned a value but never used

Check warning on line 11 in scripts/export-ai-review.js

View workflow job for this annotation

GitHub Actions / test (18.x) 

'path' is assigned a value but never used

// Import DiffService for consistent formatting
const DiffService = require('../services/diffService');

Check warning on line 14 in scripts/export-ai-review.js

View workflow job for this annotation

GitHub Actions / test (20.x) 

'DiffService' is assigned a value but never used

Check warning on line 14 in scripts/export-ai-review.js

View workflow job for this annotation

GitHub Actions / test (18.x) 

'DiffService' is assigned a value but never used
const GitStatusParser = require('../services/gitStatusParser');

Check warning on line 15 in scripts/export-ai-review.js

View workflow job for this annotation

GitHub Actions / test (20.x) 

'GitStatusParser' is assigned a value but never used

Check warning on line 15 in scripts/export-ai-review.js

View workflow job for this annotation

GitHub Actions / test (18.x) 

'GitStatusParser' is assigned a value but never used
const ReviewGenerator = require('../services/ReviewGenerator');

// Parse command line arguments
Expand Down Expand Up @@ -113,7 +113,7 @@

// Check if we're in a git repository
try {
execSync('git rev-parse --git-dir', { stdio: 'ignore' });
execFileSync('git', ['rev-parse', '--git-dir'], { stdio: 'ignore' });
} catch (error) {
console.error('❌ Error: Not a git repository');
console.error('💡 Please run this command inside a git repository');
Expand All @@ -125,15 +125,15 @@
let deletedFiles = [];

try {
const output = execSync('git diff --cached --name-only', { encoding: 'utf-8' });
const output = execFileSync('git', ['diff', '--cached', '--name-only'], { encoding: 'utf-8' });
stagedFiles = output.trim() ? output.trim().split('\n').filter(f => f.length > 0) : [];
} catch (error) {
console.error('❌ Error getting staged files:', error.message);
process.exit(1);
}

try {
const deletedOutput = execSync('git ls-files --deleted', { encoding: 'utf-8' });
const deletedOutput = execFileSync('git', ['ls-files', '--deleted'], { encoding: 'utf-8' });
deletedFiles = deletedOutput.trim() ? deletedOutput.trim().split('\n').filter(f => f.length > 0) : [];
} catch (error) {
// Ignore error, deletedFiles will remain empty
Expand Down Expand Up @@ -189,7 +189,7 @@
// Directory exclusion: "node_modules/" or "node_modules/*"
if (pattern.endsWith('/') || pattern.endsWith('/*')) {
const dir = pattern.replace(/\/?\\*?$/, '');
if (file.startsWith(dir + '/')) return true;
if (file.startsWith(`${dir }/`)) return true;
}

// Extension pattern: "*.log"
Expand All @@ -203,13 +203,13 @@
// Convert glob pattern to regex
// scripts/**/*.js -> ^scripts\/(.*\/)?[^/]*\.js$
// ** means "zero or more directories", so **/ becomes (.*\/)?
let regexPattern = pattern
const regexPattern = pattern
.replace(/\*\*\//g, '__DOUBLESTARSLASH__') // Placeholder for **/
.replace(/\./g, '\\.') // Escape dots
.replace(/\//g, '\\/') // Escape forward slashes
.replace(/\*/g, '[^/]*') // * matches any characters except /
.replace(/__DOUBLESTARSLASH__/g, '(.*\\/)?'); // **/ means zero or more dirs
const regex = new RegExp('^' + regexPattern + '$');
const regex = new RegExp(`^${ regexPattern }$`);
if (regex.test(file)) return true;
}

Expand All @@ -219,7 +219,7 @@
.replace(/\./g, '\\.')
.replace(/\//g, '\\/')
.replace(/\*/g, '[^/]*');
const regex = new RegExp('^' + regexPattern + '$');
const regex = new RegExp(`^${ regexPattern }$`);
if (regex.test(file)) return true;
}

Expand Down Expand Up @@ -256,7 +256,7 @@
}
}

console.log(`\n📊 Summary:`);
console.log('\n📊 Summary:');
console.log(` Included: ${includedFiles.length} files`);
console.log(` Excluded: ${excludedFiles.length} files`);
console.log(` Too large: ${largeFiles.length} files`);
Expand All @@ -269,9 +269,9 @@
// Generate AI review content
if (config.splitMode) {
const outDirName = config.outputDir === 'AI_REVIEWS' ? undefined : config.outputDir; // undefined triggers timestamp default in service
console.log(`\nmode: ✂️ Split Mode`);
console.log('\nmode: ✂️ Split Mode');
if (outDirName) console.log(`Output: ${outDirName}/`);
else console.log(`Output: [Auto-Timestamped Directory]`);
else console.log('Output: [Auto-Timestamped Directory]');

(async () => {
try {
Expand Down
11 changes: 6 additions & 5 deletions server.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#!/usr/bin/env node

const express = require('express');
const { execFile } = require('child_process');

Check warning on line 4 in server.js

View workflow job for this annotation

GitHub Actions / test (20.x) 

'execFile' is assigned a value but never used

Check warning on line 4 in server.js

View workflow job for this annotation

GitHub Actions / test (18.x) 

'execFile' is assigned a value but never used
const { writeFileSync, readFileSync, existsSync, mkdirSync } = require('fs');
const path = require('path');
const cors = require('cors');
Expand Down Expand Up @@ -97,7 +97,7 @@

const userAgent = req.get('User-Agent') || '';
const queryString = JSON.stringify(req.query);
const fullUrl = req.path + (req.query ? '?' + new URLSearchParams(req.query).toString() : '');
const fullUrl = req.path + (req.query ? `?${ new URLSearchParams(req.query).toString()}` : '');

const suspiciousFound = suspiciousPatterns.some(pattern =>
pattern.test(fullUrl) ||
Expand Down Expand Up @@ -178,7 +178,8 @@

// Additional checks for suspicious patterns
const suspiciousPatterns = [
/\x00/, // Null bytes
// eslint-disable-next-line no-control-regex
/\u0000/, // Null bytes
/[<>"|*?]/, // Dangerous file characters
/^\//, // Absolute paths
/^[a-zA-Z]:\\/, // Windows absolute paths
Expand Down Expand Up @@ -333,7 +334,7 @@
const files = output.trim() ? output.trim().split('\n').filter(f => f.length > 0) : [];

// Get file statuses to determine if files are deleted
let fileStatuses = {};
const fileStatuses = {};
try {
const statusOutput = await GitService.execute('diff-cached', ['--name-status']);
const statusLines = statusOutput.trim().split('\n').filter(line => line.length > 0);
Expand Down Expand Up @@ -566,7 +567,7 @@
return res.status(400).json({
success: false,
error: `Found ${deletedFiles.length} deleted file(s) but they are not staged. Use "git add -A" to stage all changes including deletions.`,
deletedFiles: deletedFiles,
deletedFiles,
suggestion: 'Run "git add -A" to stage all changes, then try again.',
timestamp: new Date().toISOString()
});
Expand Down Expand Up @@ -660,7 +661,7 @@
return res.status(400).json({
success: false,
error: `Found ${deletedFiles.length} deleted file(s) but they are not staged.Use "git add -A" to stage all changes including deletions.`,
deletedFiles: deletedFiles,
deletedFiles,
suggestion: 'Run "git add -A" to stage all changes, then try again.',
timestamp: new Date().toISOString()
});
Expand Down
24 changes: 12 additions & 12 deletions services/ReviewGenerator.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,10 @@ class ReviewGenerator {
const createdFiles = [];

// Generate Summary Content
let summaryContent = `# 🔍 Code Review Summary\n\n`;
let summaryContent = '# 🔍 Code Review Summary\n\n';
summaryContent += `**Generated:** ${new Date().toLocaleString()}\n`;
summaryContent += `**Total Files:** ${includedFiles.length}\n\n`;
summaryContent += `## 📂 Review Files\n`;
summaryContent += '## 📂 Review Files\n';

// Process files concurrently for better performance
const filePromises = includedFiles.map(async (file) => {
Expand Down Expand Up @@ -100,15 +100,15 @@ class ReviewGenerator {
static async generateUnifiedReview({ includedFiles, excludedFiles = [], largeFiles = [] }) {
const timestamp = new Date().toLocaleString();
let content = `# 🔍 Code Review - ${timestamp}\n\n`;
content += `**Project:** AI Visual Code Review\n`;
content += `**Generated by:** AI Visual Code Review v2.0\n\n`;
content += '**Project:** AI Visual Code Review\n';
content += '**Generated by:** AI Visual Code Review v2.0\n\n';

// Change Summary
try {
const stats = await GitService.getDiffStats();
content += `## 📊 Change Summary\n\n\`\`\`\n${stats.trim()}\n\`\`\`\n\n`;
} catch (err) {
content += `## 📊 Change Summary\n\n(Stats not available)\n\n`;
content += '## 📊 Change Summary\n\n(Stats not available)\n\n';
}

content += `## 📝 Files Changed (${includedFiles.length} selected)\n\n`;
Expand Down Expand Up @@ -139,7 +139,7 @@ class ReviewGenerator {
let fileContent = '';

// Get file status
let fileStatus = fileStatuses[file] || 'M ';
const fileStatus = fileStatuses[file] || 'M ';
const statusInfo = GitStatusParser.parse(fileStatus);

fileContent += `\n${GitStatusParser.getMarkdownHeader(fileStatus, file)}\n\n`;
Expand Down Expand Up @@ -207,7 +207,7 @@ class ReviewGenerator {
}
}

content += this.getChecklistTemplate().replace("# 🔍 Code Review", "").replace("## 📊 File Information", ""); // Remove duplicate headers from checklist template if reusing
content += this.getChecklistTemplate().replace('# 🔍 Code Review', '').replace('## 📊 File Information', ''); // Remove duplicate headers from checklist template if reusing

// Write to file
const filePath = path.join(process.cwd(), 'AI_REVIEW.md');
Expand Down Expand Up @@ -243,10 +243,10 @@ class ReviewGenerator {

let content = `# 📄 Code Review: \`${file}\`\n\n`;
content += `**Generated:** ${new Date().toLocaleString()}\n`;
content += `**Project:** AI Visual Code Review\n`;
content += `**Review Type:** Individual File Analysis\n\n`;
content += '**Project:** AI Visual Code Review\n';
content += '**Review Type:** Individual File Analysis\n\n';

content += `## 📊 File Information\n\n`;
content += '## 📊 File Information\n\n';
content += `**Type:** ${fileTypeMap[ext] || 'Source File 📄'}\n`;
content += `**Path:** \`${file}\`\n`;
content += `**Extension:** ${ext || 'None'}\n\n`;
Expand All @@ -261,7 +261,7 @@ class ReviewGenerator {
.filter(([lineId]) => lineId.includes(file.replace(/[^a-zA-Z0-9]/g, '_')));

if (fileLineComments.length > 0) {
content += `## 🔍 Line Comments\n\n`;
content += '## 🔍 Line Comments\n\n';
fileLineComments.forEach(([lineId, comment]) => {
content += `- **${lineId}:** ${comment}\n`;
});
Expand All @@ -271,7 +271,7 @@ class ReviewGenerator {
// Diff
try {
const diff = await GitService.getDiffForFile(file);
content += `## 📝 Changes\n\n`;
content += '## 📝 Changes\n\n';
content += DiffService.generateEnhancedDiffMarkdown(diff);
} catch (error) {
content += `## ❌ Error\n\nCould not load diff for \`${file}\`: ${error.message}\n\n`;
Expand Down
2 changes: 1 addition & 1 deletion services/gitStatusParser.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class GitStatusParser {
*/
static parse(statusCode) {
// Normalize status code to 2 characters
const normalizedStatus = (statusCode + ' ').slice(0, 2);
const normalizedStatus = (`${statusCode } `).slice(0, 2);
const indexStatus = normalizedStatus[0];
const workingStatus = normalizedStatus[1];

Expand Down
2 changes: 1 addition & 1 deletion src/middleware/security.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ function requestLogger(req, res, next) {

const userAgent = req.get('User-Agent') || '';
const queryString = JSON.stringify(req.query);
const fullUrl = req.path + (req.query ? '?' + new URLSearchParams(req.query).toString() : '');
const fullUrl = req.path + (req.query ? `?${ new URLSearchParams(req.query).toString()}` : '');

const suspiciousFound = suspiciousPatterns.some(pattern =>
pattern.test(fullUrl) ||
Expand Down
3 changes: 2 additions & 1 deletion src/utils/validation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ function validateFileRequest(file) {

// Additional checks for suspicious patterns
const suspiciousPatterns = [
/\x00/, // Null bytes
// eslint-disable-next-line no-control-regex
/\u0000/, // Null bytes
Comment on lines +34 to +35
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 4, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's examine the specific lines mentioned in the review
wc -l src/utils/validation.js

Repository: PrakharMNNIT/ai-visual-code-review

Length of output: 105

🏁 Script executed:

# Read the relevant sections of the file
sed -n '30,50p' src/utils/validation.js

Repository: PrakharMNNIT/ai-visual-code-review

Length of output: 828

🏁 Script executed:

# Search for Biome configuration files
fd -name "biome.json|biome.jsonc|.biomerc|.biomerc.json"

Repository: PrakharMNNIT/ai-visual-code-review

Length of output: 308

🏁 Script executed:

# Search for ESLint and Biome directives in the file
rg -n "no-control-regex|biome-ignore|u0000" src/utils/validation.js

Repository: PrakharMNNIT/ai-visual-code-review

Length of output: 170

🏁 Script executed:

# Find biome config files with correct fd syntax
fd "biome" --type f

Repository: PrakharMNNIT/ai-visual-code-review

Length of output: 88

🏁 Script executed:

# Check if biome is in package.json
cat package.json | grep -i biome

Repository: PrakharMNNIT/ai-visual-code-review

Length of output: 60

🏁 Script executed:

# Look at lines 40-50 to see what's at lines 42-43
sed -n '40,50p' src/utils/validation.js

Repository: PrakharMNNIT/ai-visual-code-review

Length of output: 476

🏁 Script executed:

# Search for any biome directives in the entire codebase
rg "biome-ignore|@biome" --type js

Repository: PrakharMNNIT/ai-visual-code-review

Length of output: 60

🏁 Script executed:

# Read the biome.json configuration
cat biome.json

Repository: PrakharMNNIT/ai-visual-code-review

Length of output: 1852

🏁 Script executed:

# Get exact line numbers - read full validation.js to count properly
cat -n src/utils/validation.js | sed -n '30,50p'

Repository: PrakharMNNIT/ai-visual-code-review

Length of output: 975

/\u0000/ and /[\r\n]/ violate Biome and can fail lint.

Lines 35 and 39 use control-character regexes; the ESLint directive on line 34 won't silence Biome's noControlCharactersInRegex rule (different linters). Prefer explicit string checks to keep validation and pass lint.

Suggested fix 
   // Additional checks for suspicious patterns
+  const hasNullByte = file.includes('\0');
+  const hasNewlines = /[\r\n]/.test(file);
   const suspiciousPatterns = [
-    // eslint-disable-next-line no-control-regex
-    /\u0000/,           // Null bytes
     /[<>"|*?]/,       // Dangerous file characters
     /^\//,            // Absolute paths
     /^[a-zA-Z]:\\/,   // Windows absolute paths
-    /[\r\n]/,         // Newline injection
   ];
 
-  if (suspiciousPatterns.some(pattern => pattern.test(file))) {
+  if (hasNullByte || hasNewlines || suspiciousPatterns.some(pattern => pattern.test(file))) {
     return { valid: false, error: 'File path contains invalid characters' };
   }
🧰 Tools
🪛 Biome (2.4.4)

[error] 35-35: Unexpected control character in a regular expression.

(lint/suspicious/noControlCharactersInRegex)

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/utils/validation.js` around lines 34 - 35, Replace the control-character
regular expressions /\u0000/ and /[\r\n]/ in the validation logic with explicit
string checks (e.g., use value.includes('\u0000') and (value.includes('\r') ||
value.includes('\n')) or indexOf !== -1) so Biome's noControlCharactersInRegex
rule is not triggered; update the validation function that currently uses those
regex literals to return the same boolean result via these string checks and
remove the eslint-disable comment.

/[<>"|*?]/, // Dangerous file characters
/^\//, // Absolute paths
/^[a-zA-Z]:\\/, // Windows absolute paths
Expand Down
Loading