θΏ™ζ˜―indexlocζδΎ›ηš„ζœεŠ‘οΌŒδΈθ¦θΎ“ε…₯任何密码
Skip to content

Conversation

@timothycarambat
Copy link
Member

Pull Request Type

  • ✨ feat
  • πŸ› fix
  • ♻️ refactor
  • πŸ’„ style
  • πŸ”¨ chore
  • πŸ“ docs

What is in this change?

Enforces normalization of the submitted URLs to not honor relative pathing.

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

@review-agent-prime
Copy link

server/utils/files/index.js

It's great to see that you've added a normalizePath function to sanitize the file paths. However, it would be even better if you could also validate the file paths to ensure they are within the expected directory. This can help prevent directory traversal attacks where an attacker could potentially access sensitive files outside of the intended directory.
Create Issue
See the diff
Checkout the fix

    function validatePath(basePath, targetPath) {
      const resolvedPath = path.resolve(basePath, targetPath);
      if (resolvedPath.startsWith(basePath)) {
        return resolvedPath;
      }
      throw new Error('Invalid path');
    }
git fetch origin && git checkout -b ReviewBot/Impro-o6z4zej origin/ReviewBot/Impro-o6z4zej

Currently, you're using fs.readFileSync to read the file data. While this is fine for smaller files, it could potentially block the Node.js event loop if the file is large, leading to performance issues. Consider using fs.readFile instead, which is asynchronous and won't block the event loop.
Create Issue
See the diff
Checkout the fix

    fs.readFile(fullPath, "utf8", (err, data) => {
      if (err) throw err;
      return JSON.parse(data);
    });
git fetch origin && git checkout -b ReviewBot/Impro-7joft1h origin/ReviewBot/Impro-7joft1h

@timothycarambat timothycarambat merged commit 026849d into master Jan 15, 2024
@timothycarambat timothycarambat deleted the secuirty/remove-document-normalization branch January 15, 2024 00:36
cabwds pushed a commit to cabwds/anything-llm that referenced this pull request Jul 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants