Skip to content

Commit 805df58

Browse files
author
Lukas Kraic
committed
fix(security): add pathspec separator and improve path validation
- Add '--' before file pathspecs in git commands to prevent option injection (e.g., filename starting with '-' being interpreted as option) Affected commands: status, add, restore, reset - Improve validateFilePath to use path.relative() for robust containment check that correctly handles edge cases like filesystem root as repo root
1 parent 8975c4b commit 805df58

1 file changed

Lines changed: 13 additions & 9 deletions

File tree

server/routes/git.js

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,12 @@ function validateFilePath(rootDir, filePath) {
2323
const resolvedRoot = path.resolve(rootDir);
2424
const resolvedPath = path.resolve(rootDir, filePath);
2525

26-
// Ensure the resolved path starts with the root directory
27-
if (!resolvedPath.startsWith(resolvedRoot + path.sep) && resolvedPath !== resolvedRoot) {
26+
// Use path.relative for robust containment check that handles edge cases
27+
// like filesystem root as repository root
28+
const relativePath = path.relative(resolvedRoot, resolvedPath);
29+
30+
// Path escapes root if relative path starts with '..' or is absolute
31+
if (relativePath.startsWith('..') || path.isAbsolute(relativePath)) {
2832
throw new Error(`Invalid file path: path escapes repository root`);
2933
}
3034

@@ -191,7 +195,7 @@ router.get('/diff', async (req, res) => {
191195
const gitRoot = await validateGitRepository(projectPath);
192196

193197
// Check if file is untracked or deleted
194-
const { stdout: statusOutput } = await execFileAsync('git', ['status', '--porcelain', file], { cwd: gitRoot });
198+
const { stdout: statusOutput } = await execFileAsync('git', ['status', '--porcelain', '--', file], { cwd: gitRoot });
195199
const isUntracked = statusOutput.startsWith('??');
196200
const isDeleted = statusOutput.trim().startsWith('D ') || statusOutput.trim().startsWith(' D');
197201

@@ -255,7 +259,7 @@ router.get('/file-with-diff', async (req, res) => {
255259
const gitRoot = await validateGitRepository(projectPath);
256260

257261
// Check file status
258-
const { stdout: statusOutput } = await execFileAsync('git', ['status', '--porcelain', file], { cwd: gitRoot });
262+
const { stdout: statusOutput } = await execFileAsync('git', ['status', '--porcelain', '--', file], { cwd: gitRoot });
259263
const isUntracked = statusOutput.startsWith('??');
260264
const isDeleted = statusOutput.trim().startsWith('D ') || statusOutput.trim().startsWith(' D');
261265

@@ -365,7 +369,7 @@ router.post('/commit', async (req, res) => {
365369

366370
// Stage selected files
367371
for (const file of files) {
368-
await execFileAsync('git', ['add', file], { cwd: gitRoot });
372+
await execFileAsync('git', ['add', '--', file], { cwd: gitRoot });
369373
}
370374

371375
// Commit with message
@@ -1090,7 +1094,7 @@ router.post('/discard', async (req, res) => {
10901094
const gitRoot = await validateGitRepository(projectPath);
10911095

10921096
// Check file status to determine correct discard command
1093-
const { stdout: statusOutput } = await execFileAsync('git', ['status', '--porcelain', file], { cwd: gitRoot });
1097+
const { stdout: statusOutput } = await execFileAsync('git', ['status', '--porcelain', '--', file], { cwd: gitRoot });
10941098

10951099
if (!statusOutput.trim()) {
10961100
return res.status(400).json({ error: 'No changes to discard for this file' });
@@ -1112,10 +1116,10 @@ router.post('/discard', async (req, res) => {
11121116
}
11131117
} else if (status.includes('M') || status.includes('D')) {
11141118
// Modified or deleted file - restore from HEAD
1115-
await execFileAsync('git', ['restore', file], { cwd: gitRoot });
1119+
await execFileAsync('git', ['restore', '--', file], { cwd: gitRoot });
11161120
} else if (status.includes('A')) {
11171121
// Added file - unstage it
1118-
await execFileAsync('git', ['reset', 'HEAD', file], { cwd: gitRoot });
1122+
await execFileAsync('git', ['reset', 'HEAD', '--', file], { cwd: gitRoot });
11191123
}
11201124

11211125
res.json({ success: true, message: `Changes discarded for ${file}` });
@@ -1138,7 +1142,7 @@ router.post('/delete-untracked', async (req, res) => {
11381142
const gitRoot = await validateGitRepository(projectPath);
11391143

11401144
// Check if file is actually untracked
1141-
const { stdout: statusOutput } = await execFileAsync('git', ['status', '--porcelain', file], { cwd: gitRoot });
1145+
const { stdout: statusOutput } = await execFileAsync('git', ['status', '--porcelain', '--', file], { cwd: gitRoot });
11421146

11431147
if (!statusOutput.trim()) {
11441148
return res.status(400).json({ error: 'File is not untracked or does not exist' });

0 commit comments

Comments
 (0)