Skip to content

fix(permissions): split comma-separated paths in --ignore-read#35661

Merged
bartlomieju merged 1 commit into
denoland:mainfrom
chatman-media:fix/ignore-read-comma-split
Jul 3, 2026
Merged

fix(permissions): split comma-separated paths in --ignore-read#35661
bartlomieju merged 1 commit into
denoland:mainfrom
chatman-media:fix/ignore-read-comma-split

Conversation

@chatman-media

Copy link
Copy Markdown
Contributor

The help text for --ignore-read advertises comma-separated paths the same way --allow-read/--deny-read do (--ignore-read="/etc,/var/log.txt"), but the parse site just did a plain .collect() with no comma-splitting, so a comma-separated value got stored as one literal string that never matches a real path — the ignore rule silently does nothing. --ignore-env handles this correctly via use_value_delimiter, which is what made --ignore-read stand out as the inconsistent one rather than this being intentional.

Fixed it to go through flat_escape_split_commas, same as the adjacent allow-read/deny-read blocks. Added a test covering the comma-separated case.

The help text for --ignore-read advertises comma-separated paths just
like --allow-read and --deny-read (--ignore-read="/etc,/var/log.txt"),
but the parse site just did a plain .collect() with no comma-splitting,
so the whole string got stored as one literal entry that never matches
a real path. --ignore-env handles this correctly via use_value_delimiter,
which is what tipped me off that --ignore-read was the odd one out
rather than this being intentional.

Added a test covering the comma-separated case.
@deno-cla-assistant

deno-cla-assistant Bot commented Jun 30, 2026

Copy link
Copy Markdown

Deno Individual Contributor License Agreement

All contributors have signed the CLA. Thank you!

Re-run CLA check


This is an automated message from CLA Assistant

@bartlomieju

Copy link
Copy Markdown
Member

Please sign the CLA so we can land this PR

@chatman-media

Copy link
Copy Markdown
Contributor Author

Done, should be green now.

@bartlomieju bartlomieju left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks!

@bartlomieju bartlomieju merged commit c239456 into denoland:main Jul 3, 2026
268 of 270 checks passed
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