fix(core): Set default for allowed file access dirs #22279
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (all 1 issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/@n8n/config/src/configs/security.config.ts">
<violation number="1" location="packages/@n8n/config/src/configs/security.config.ts:11">
Defaulting `restrictFileAccessTo` to `/home/user/...` blocks the real n8n data directory on every environment where the username isn’t literally `user`, so file access now fails by default in the standard Docker image and most installs.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
This comment has been minimized.
This comment has been minimized.
|
E2E Tests: n8n tests passed after 10m 1.3s Run Details
Groups
This message was posted automatically by
currents.dev | Integration Settings
|
ivov
force-pushed
the
cat-57-secure-default-for-n8n_restrict_file_access_to
branch
8 times, most recently
from
f865bb6 to
838c8c1
Compare
ivov
force-pushed
the
cat-57-secure-default-for-n8n_restrict_file_access_to
branch
from
838c8c1 to
6e6ba2c
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This comment has been minimized.
This comment has been minimized.
| */ | ||
| @Env('N8N_RESTRICT_FILE_ACCESS_TO') | ||
| restrictFileAccessTo: string = ''; | ||
| restrictFileAccessTo: string = '~/.n8n-files'; |
There was a problem hiding this comment.
I think it would be good to keep the default value inside the N8N home folder. The breaking change docs mention the default will be
./data directory inside the n8n home folder.
There was a problem hiding this comment.
We already have N8N_BLOCK_FILE_ACCESS_TO_N8N_FILES defaulting to true which according to the code blocks ~/.n8n. This clashes with a default that allowlists a subdir inside ~/.n8n for user operations. Seems more secure to guide users to keep user data separate from n8n internals or they might end up disabling the blocklist for convenience? Product-wise I'm not clear on how these two settings are meant to interact.
There was a problem hiding this comment.
Hmmm okay, that makes sense. Let's keep it like this and update the v2 breaking changes doc. Could you do that? 🙏
There was a problem hiding this comment.
Should we add a test that ~/mydir works as expected?
There was a problem hiding this comment.
I think we want to keep changes to a minimum or? #22280 (review)
There was a problem hiding this comment.
Yes, but if we introduce new logic it would be good to have some tests for it.
https://linear.app/n8n/issue/CAT-57