Fix Uncontrolled data used in path expression

[odaysec]

Pull Request Type

anything-llm/server/utils/files/index.js

Lines 180 to 190 in 388960e

	const filePath = path.resolve(documentsPath, normalizePath(filename));
	if (
	!fs.existsSync(filePath) ||
	!isWithin(documentsPath, filePath) ||
	!fs.lstatSync(filePath).isFile()
	)
	return;
	console.log(`Purging source document of ${filename}.`);
	fs.rmSync(filePath);

fix the issue need to ensure that the filename input is properly sanitized and validated before constructing the file path. Specifically:

1. Use the path.resolve function to normalize the path and remove any .. segments.
2. Verify that the resolved path starts with the documentsPath to ensure it is confined to the intended directory.
3. Throw an error or return early if the validation fails.

The normalizePath function should also be updated to provide stricter sanitization, ensuring that the resulting path is safe to use.

---

• ✨ feat
• 🐛 fix
• ♻️ refactor
• 💄 style
• 🔨 chore
• 📝 docs

Developer Validations

• I ran yarn lint from the root of the repo & committed changes
• Relevant documentation has been updated
• I have tested my code functionality
• Docker build succeeds locally

[timothycarambat]

Looking for feedback on comments

[timothycarambat]

This is a good improvement to apply 👍

[timothycarambat]

const sanitizedFilename = normalizePath(filename);
const filePath = path.resolve(documentsPath, sanitizedFilename);

is the same as

const filePath = path.resolve(documentsPath, normalizePath(filename));

and this block

 !filePath.startsWith(documentsPath) || // Ensure the path is within documentsPath
!fs.existsSync(filePath) ||
 !fs.lstatSync(filePath).isFile()

filePath.startsWith(documentsPath) does a similar check of what isWithin does already, no? So functionally, the logic has not changed here. We could have both or check within before checking existence, maybe?