Skip to content
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

fix(cli): update permission prompt message for compiled binaries #24081

Conversation

yazan-abdalrahman
Copy link
Contributor

When running a compiled Deno program that requires permissions, the prompt message previously instructed users to rerun with --allow-* flags, which is incorrect for binaries.

Changes:

  • Updated the permission prompt message to instruct users to specify required permissions during the compilation step using deno compile --allow-*.
  • Adjusted PermissionDenied error messages to reflect the correct instructions for binaries.

This fix ensures that users are correctly informed about how to handle permissions for compiled Deno binaries.

Resolves #23250

When running a compiled Deno program that requires permissions, the prompt message previously instructed users to rerun with `--allow-*` flags, which is incorrect for binaries.

Changes:
- Updated the permission prompt message to instruct users to specify required permissions during the compilation step using `deno compile --allow-*`.
- Adjusted `PermissionDenied` error messages to reflect the correct instructions for binaries.

This fix ensures that users are correctly informed about how to handle permissions for compiled Deno binaries.

Resolves denoland#23250
@CLAassistant
Copy link

CLAassistant commented Jun 2, 2024

CLA assistant check
All committers have signed the CLA.

@yazan-abdalrahman yazan-abdalrahman changed the title Fix: Update permission prompt message for compiled binaries fix(cli): Update permission prompt message for compiled binaries Jun 2, 2024
@yazan-abdalrahman yazan-abdalrahman changed the title fix(cli): Update permission prompt message for compiled binaries fix(cli): update permission prompt message for compiled binaries Jun 3, 2024
@lucacasonato
Copy link
Member

Did you write this PR yourself, or is it AI generated? The addition of global_flags crate seems somewhat unnecessary?

@yazan-abdalrahman
Copy link
Contributor Author

Did you write this PR yourself, or is it AI generated? The addition of global_flags crate seems somewhat unnecessary?

Hi @lucacasonato

I wrote it,
I searched about dependencies to help me pass a flag to indicate that I am running from binary and not uniray. I found this is_unary param in the prompt method; it was hardcoded with a value of true; it was not passed along with the flow, so it is difficult to update the flow to pass it, so we need a flag to indicate we are running from binary standalone by passing the flag. I believe this idea is better.

Do you have a better way?

runtime/permissions/lib.rs Outdated Show resolved Hide resolved
runtime/permissions/prompter.rs Outdated Show resolved Hide resolved
…-to-indicate-compile-time-permission-specification' into Update-permission-prompt-message-to-indicate-compile-time-permission-specification
@yazan-abdalrahman
Copy link
Contributor Author

Hello @bartlomieju
, could you please review the latest changes?

@yazan-abdalrahman
Copy link
Contributor Author

@bartlomieju Can we merge it?

@bartlomieju bartlomieju added this to the 1.46 milestone Jul 18, 2024
Copy link
Member

@dsherret dsherret left a comment

Choose a reason for hiding this comment

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

LGTM

@dsherret dsherret enabled auto-merge (squash) August 20, 2024 00:46
@dsherret dsherret merged commit 4f49f70 into denoland:main Aug 20, 2024
17 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.

Misleading recommendation for permissions when using compiled binary
5 participants