Skip to content

fix(cli): accept all valid sys permission descriptors - #36753

Merged
bartlomieju merged 1 commit into
denoland:mainfrom
hugojosefson:fix/allow-sys-inspector-cli-parser
Sep 8, 2026
Merged

bartlomieju merged 1 commit into
denoland:mainfrom
hugojosefson:fix/allow-sys-inspector-cli-parser

Conversation

@hugojosefson

@hugojosefson hugojosefson commented Sep 1, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Regression

Deno 2.9.6 has a regression (compared to 2.9.5) where it stops accepting valid granular sys permissions during CLI parsing. For example:

$ deno run --allow-sys=inspector 'data:application/javascript,console.log("ok")'
error: unknown sys descriptor: 'inspector'

The deno_cli_parser cutover introduced a second sys-descriptor allowlist that had drifted from deno_permissions::SysDescriptor::parse. It omitted inspector and other valid descriptors such as setuid and umask.

Fix

This PR removes the duplicate list and validates both --allow-sys and --deny-sys values with the runtime parser. Validation does not replace the original flag value, so aliases such as username retain their existing CLI representation.

Parser tests cover accepted descriptors, rejected descriptors, and alias preservation for both flags.

Related: #36519, which covers node:inspector sys-permission behavior and needs --allow-sys=inspector.

Testing done

cargo test -p deno_cli_parser --features deno_core/v8
./x build
./target/debug/deno run --allow-sys=inspector 'data:application/javascript,console.log("ok")'
./x fmt
./x lint

AI usage

I used OpenAI GPT-5.6 models via OpenCode for all of this.

@deno-cla-assistant

deno-cla-assistant Bot commented Sep 1, 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

@hugojosefson hugojosefson changed the title fix(cli): accept runtime sys permission descriptors fix(cli): accept all valid sys permission descriptors Sep 1, 2026
@hugojosefson
hugojosefson marked this pull request as ready for review September 1, 2026 10:25
@hugojosefson
hugojosefson force-pushed the fix/allow-sys-inspector-cli-parser branch 3 times, most recently from ef66a27 to ae3ae0d Compare September 3, 2026 07:06
@hugojosefson
hugojosefson force-pushed the fix/allow-sys-inspector-cli-parser branch from ae3ae0d to 5ec7877 Compare September 3, 2026 15:52

@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.

LGTM, thanks!

@bartlomieju
bartlomieju enabled auto-merge (squash) September 8, 2026 08:31
@bartlomieju
bartlomieju merged commit 4bfbfeb into denoland:main Sep 8, 2026
137 checks passed
petamoriken added a commit to petamoriken/deno that referenced this pull request Sep 8, 2026
Resolve conflict in libs/cli_parser/src/convert.rs: upstream (denoland#36753)
replaced the hardcoded VALID_SYS_DESCRIPTORS list with
SysDescriptor::parse, which already accepts the "localFonts"
descriptor added on this branch, so the local list is no longer needed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@hugojosefson
hugojosefson deleted the fix/allow-sys-inspector-cli-parser branch September 8, 2026 12:01
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