-
Notifications
You must be signed in to change notification settings - Fork 1.8k
feat: add allowSymlinks option to File transport for symlink handling #2595
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.
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds a new allowSymlinks option to the File transport to control whether symbolic links should be followed when writing logs. When set to false, the transport uses the O_NOFOLLOW flag to prevent writes to symlinks, enhancing security by preventing symlink-based attacks.
Key Changes:
- Added
allowSymlinksoption (defaults totruefor backward compatibility) - Modified stream creation to use
O_NOFOLLOWflag when symlinks are disallowed - Added comprehensive test coverage for both enabled and disabled symlink handling
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| lib/winston/transports/file.js | Adds allowSymlinks option initialization and implements O_NOFOLLOW flag logic in _createStream method |
| test/unit/winston/transports/file.test.js | Adds new test suite "Symlink Option" with tests for both allowSymlinks: false and default behavior |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
indexzero
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 once the workflow runs pass
|
@indexzero I've pushed additional tests to improve coverage and potentially resolve the pipeline failure |
This PR adds a new option
allowSymlinksto theFiletransport to give users control over whether symbolic links should be followed when writing logs.