Skip to content

fix: priority of default deny read on macos#184

Merged
jy-tan merged 2 commits into
fencesandbox:mainfrom
shfx:fix/default-deny-read-macos-priority
Jun 22, 2026
Merged

fix: priority of default deny read on macos#184
jy-tan merged 2 commits into
fencesandbox:mainfrom
shfx:fix/default-deny-read-macos-priority

Conversation

@shfx

@shfx shfx commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Hi. I noticed a maybe unclear or undefined behavior, please correct me if I'm wrong.

With this config, .env files are still accessible but should not be:

{
  "filesystem": {
    "defaultDenyRead": true,
    "allowRead": ["."],
    "denyRead": ["**/.env", "**/.env.*"]
  }
}

IMHO we want denyReads to take precedence over allowReads. I created this changeset that apparently works on my machine. If you think this is incorrect then maybe we can find better way of blocking important files when defaultDenyRead is true and we still want to have entire dir readable like ".".

Anyways, thanks for the project, it's pretty cool :)

@shfx shfx requested a review from jy-tan as a code owner June 17, 2026 18:17

@jy-tan jy-tan left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix @shfx! This makes sense. Could you also add coverage that fails on the actual precedence regression? e.g., assert the generated macOS profile includes explicit deny file-read-data / deny file-read-metadata rules for defaultDenyRead + allowRead "." + denyRead "**/.env", or add a darwin integration test showing .env inside the allowed workspace cannot be read. Thanks!

@shfx shfx force-pushed the fix/default-deny-read-macos-priority branch from f2d2b2a to 0b2192e Compare June 20, 2026 16:30
@jy-tan jy-tan merged commit 6bc53a9 into fencesandbox:main Jun 22, 2026
5 checks passed
@shfx

shfx commented Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

🙇🏻

@shfx shfx deleted the fix/default-deny-read-macos-priority branch June 22, 2026 12:45
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