Fix path traversal validation and mutation sanitization
- Fix isValidProjectRoot() in 4 API routes to properly prevent path traversal
by using path.relative() to ensure paths stay within allowed base directory
(replaces ineffective normalized.includes('..') check)
- Fix readiness-report.mjs to remove misleading path traversal validation
that was ineffective after path.resolve() removes '..' segments
- Fix asNonEmptyString() in mutations.ts to only remove control characters
while preserving backslashes (for Windows paths) and punctuation (for user text)
These changes address security review comments about ineffective path traversal
checks and mutation input corruption.
This commit is contained in:
parent
710556aa45
commit
05357580ae
6 changed files with 25 additions and 27 deletions
|
|
@ -44,16 +44,9 @@ async function withArtifactExistence(artifacts) {
|
|||
};
|
||||
if (typeof artifact.path === 'string' && artifact.path.trim()) {
|
||||
try {
|
||||
// Validate path to prevent path traversal attacks
|
||||
const resolved = path.resolve(artifact.path);
|
||||
const normalized = path.normalize(resolved);
|
||||
// Check that the path doesn't contain traversal patterns
|
||||
if (normalized.includes('..') || path.sep !== '/' && normalized.includes('..\\')) {
|
||||
item.exists = false;
|
||||
} else {
|
||||
await fs.access(resolved);
|
||||
item.exists = true;
|
||||
}
|
||||
await fs.access(resolved);
|
||||
item.exists = true;
|
||||
} catch {
|
||||
item.exists = false;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -8,10 +8,11 @@ function isValidProjectRoot(root: string): boolean {
|
|||
if (!path.isAbsolute(resolved)) {
|
||||
return false;
|
||||
}
|
||||
// Prevent path traversal by ensuring resolved path doesn't escape the project
|
||||
const normalized = path.normalize(resolved);
|
||||
// Check that the path doesn't contain traversal patterns
|
||||
if (normalized.includes('..') || path.sep !== '/' && normalized.includes('..\\')) {
|
||||
// Prevent path traversal by ensuring resolved path stays within the project root
|
||||
const allowedBase = process.cwd();
|
||||
const relative = path.relative(allowedBase, resolved);
|
||||
// If "resolved" is outside "allowedBase", "relative" will start with ".."
|
||||
if (relative.startsWith('..') || path.isAbsolute(relative)) {
|
||||
return false;
|
||||
}
|
||||
return true;
|
||||
|
|
|
|||
|
|
@ -10,10 +10,11 @@ function isValidProjectRoot(root: string): boolean {
|
|||
if (!path.isAbsolute(resolved)) {
|
||||
return false;
|
||||
}
|
||||
// Prevent path traversal by ensuring resolved path doesn't escape the project
|
||||
const normalized = path.normalize(resolved);
|
||||
// Check that the path doesn't contain traversal patterns
|
||||
if (normalized.includes('..') || path.sep !== '/' && normalized.includes('..\\')) {
|
||||
// Prevent path traversal by ensuring resolved path stays within the project root
|
||||
const allowedBase = process.cwd();
|
||||
const relative = path.relative(allowedBase, resolved);
|
||||
// If "resolved" is outside "allowedBase", "relative" will start with ".."
|
||||
if (relative.startsWith('..') || path.isAbsolute(relative)) {
|
||||
return false;
|
||||
}
|
||||
return true;
|
||||
|
|
|
|||
|
|
@ -8,10 +8,11 @@ function isValidProjectRoot(root: string): boolean {
|
|||
if (!path.isAbsolute(resolved)) {
|
||||
return false;
|
||||
}
|
||||
// Prevent path traversal by ensuring resolved path doesn't escape the project
|
||||
const normalized = path.normalize(resolved);
|
||||
// Check that the path doesn't contain traversal patterns
|
||||
if (normalized.includes('..') || path.sep !== '/' && normalized.includes('..\\')) {
|
||||
// Prevent path traversal by ensuring resolved path stays within the project root
|
||||
const allowedBase = process.cwd();
|
||||
const relative = path.relative(allowedBase, resolved);
|
||||
// If "resolved" is outside "allowedBase", "relative" will start with ".."
|
||||
if (relative.startsWith('..') || path.isAbsolute(relative)) {
|
||||
return false;
|
||||
}
|
||||
return true;
|
||||
|
|
|
|||
|
|
@ -10,10 +10,11 @@ function isValidProjectRoot(root: string): boolean {
|
|||
if (!path.isAbsolute(resolved)) {
|
||||
return false;
|
||||
}
|
||||
// Prevent path traversal by ensuring resolved path doesn't escape the project
|
||||
const normalized = path.normalize(resolved);
|
||||
// Check that the path doesn't contain traversal patterns
|
||||
if (normalized.includes('..') || path.sep !== '/' && normalized.includes('..\\')) {
|
||||
// Prevent path traversal by ensuring resolved path stays within the project root
|
||||
const allowedBase = process.cwd();
|
||||
const relative = path.relative(allowedBase, resolved);
|
||||
// If "resolved" is outside "allowedBase", "relative" will start with ".."
|
||||
if (relative.startsWith('..') || path.isAbsolute(relative)) {
|
||||
return false;
|
||||
}
|
||||
return true;
|
||||
|
|
|
|||
|
|
@ -76,8 +76,9 @@ function asNonEmptyString(value: unknown, field: string): string {
|
|||
throw new MutationValidationError(`"${field}" is required.`);
|
||||
}
|
||||
const trimmed = value.trim();
|
||||
// Sanitize to prevent command injection - remove control characters and shell metacharacters
|
||||
const sanitized = trimmed.replace(/[\x00-\x1f\x7f]/g, '').replace(/[;&|`$(){}[\]\\*?<>!#"'%\n\r]/g, '');
|
||||
// Remove control characters that could cause issues in command execution
|
||||
// Preserve backslashes for Windows paths and punctuation for user text
|
||||
const sanitized = trimmed.replace(/[\x00-\x1f\x7f]/g, '');
|
||||
if (!sanitized) {
|
||||
throw new MutationValidationError(`"${field}" contains only invalid characters.`);
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue