-
Notifications
You must be signed in to change notification settings - Fork 5.8k
feat(cli): add --allow-env-file flag #31452
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThis pull request introduces a new Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (2)
cli/args/flags.rs (1)
1266-1284: Consider whether--allow-env-fileshould influencehas_permission_in_argv
Flags::has_permission_in_argvcurrently only checks for--allow-*/--deny-*permission flags and ignores--allow-env-file, even though this new flag effectively grants env read access (indirectly, via main.rs updatingpermissions.allow_env). If any callers usehas_permission_in_argvas a proxy for “user explicitly granted permissions,” you may want to either:
- extend it to treat
--allow-env-fileas permission-bearing, or- document that
--allow-env-fileis intentionally excluded.Right now this is just a behavioral subtlety, not a bug.
Also applies to: 4768-4781, 6933-6937
cli/main.rs (1)
719-741:--allow-env-filebehavior matches design; consider avoiding double dotenv parsing/loggingThis block correctly:
- Resolves
--allow-env-filepaths,- Parses each file to collect keys,
- Merges these keys into
flags.permissions.allow_envwhenallow_allis false, and- Loads env variables from those files so the process environment matches the permission allowlist.
This aligns with the PR goal of granting access only to vars defined in the specified env files.
One thing to consider: each env file is currently parsed twice—once via
get_env_vars_from_env_fileand again inload_env_variables_from_env_files. Any IO or parse error will also be logged twice. If you want to reduce overhead and log noise, you could refactor to parse each file once and reuse the result for both (a) setting env vars and (b) extracting keys forallow_env(for example, by having a helper that both setsstd::env::set_varand returns the key set). This would keep behavior identical while tightening the implementation. Based on learnings, this should still integrate cleanly withruntime/permissions.rs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.agent/workflows/setup_local_env.md(1 hunks)cli/args/flags.rs(15 hunks)cli/main.rs(2 hunks)cli/util/watch_env_tracker.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: For debugging Rust code, set breakpoints in IDE debuggers (VS Code with rust-analyzer, IntelliJ IDEA) or uselldbdirectly
Useeprintln!()ordbg!()macros for debug prints in Rust code
Files:
cli/util/watch_env_tracker.rscli/main.rscli/args/flags.rs
cli/main.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Main CLI entry point is in
cli/main.rsand should handle command routing
Files:
cli/main.rs
cli/args/flags.rs
📄 CodeRabbit inference engine (CLAUDE.md)
CLI flag parsing should be defined in
cli/args/flags.rs
Files:
cli/args/flags.rs
🧠 Learnings (3)
📚 Learning: 2025-11-24T16:19:37.808Z
Learnt from: CR
Repo: denoland/deno PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:19:37.808Z
Learning: Use `DENO_LOG=debug` or `DENO_LOG=<module>=debug` environment variable for verbose logging during development
Applied to files:
.agent/workflows/setup_local_env.md
📚 Learning: 2025-11-24T16:19:37.808Z
Learnt from: CR
Repo: denoland/deno PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:19:37.808Z
Learning: Applies to runtime/permissions.rs : Permission system is implemented in `runtime/permissions.rs`
Applied to files:
cli/main.rs
📚 Learning: 2025-11-24T16:19:37.808Z
Learnt from: CR
Repo: denoland/deno PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:19:37.808Z
Learning: Applies to cli/args/flags.rs : CLI flag parsing should be defined in `cli/args/flags.rs`
Applied to files:
cli/args/flags.rs
🧬 Code graph analysis (1)
cli/main.rs (1)
cli/util/watch_env_tracker.rs (3)
get_env_vars_from_env_file(260-292)load_env_variables_from_env_files(212-235)load_env_variables_from_env_files(238-258)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: test debug windows-x86_64
- GitHub Check: test debug linux-aarch64
- GitHub Check: test release linux-x86_64
- GitHub Check: test debug linux-x86_64
- GitHub Check: test debug macos-aarch64
- GitHub Check: test debug macos-x86_64
- GitHub Check: lint debug windows-x86_64
- GitHub Check: lint debug linux-x86_64
- GitHub Check: lint debug macos-x86_64
- GitHub Check: build libs
🔇 Additional comments (4)
cli/util/watch_env_tracker.rs (1)
260-292: Function logic is sound.The implementation correctly reads environment variables from a dotenv file and returns them as a HashMap. Error handling is appropriate: per-item parse errors are logged and processing continues (returning partial results), while file read errors return None. This matches the existing pattern in
load_env_file_inner.Minor observation: Unlike
load_env_file_inner(lines 97-108), this function doesn't explicitly check file existence before attempting to read. The outerErrcatch handles it, but the warning message is more generic than the specific "file not found" message inload_env_file_inner. Not a problem, just an inconsistency.cli/args/flags.rs (2)
789-790:--allow-env-fileflag plumbing and parsing look soundThe new
Flags::allow_env_filefield, theallow_env_file_arg()definition, andallow_env_file_arg_parse()are all consistent with the existingenv_filehandling: same default (.env), sameAppendsemantics, and non-destructiveget_manyparsing. The wiring intocache_parse,repl_parse, andruntime_args_parseensures the flag is captured for all relevant commands without altering existing behavior. No functional issues spotted here.Also applies to: 4768-4781, 5527-5533, 6206-6212, 6882-6906, 6933-6937
2215-2222: Consistent exposure of--allow-env-filealongside--env-fileEach subcommand that previously supported
--env-file(bench, cache, compile, eval, install, repl, run, serve, test) now also exposes--allow-env-file, which keeps the CLI surface predictable and avoids surprising gaps. The ordering next to--env-fileis also helpful for discoverability. This looks good.Also applies to: 2392-2398, 2570-2578, 2887-2892, 3237-3243, 3633-3639, 3655-3663, 3736-3742, 4003-4006
cli/main.rs (1)
72-72: Import is correctly wired and used
get_env_vars_from_env_fileis imported from the expected module and used below; no dead code or layering issues here.
| --- | ||
| description: Setup local development environment for Deno | ||
| --- | ||
|
|
||
| # Setup Local Deno Development Environment | ||
|
|
||
| This guide helps you set up your system to build and contribute to Deno. | ||
|
|
||
| ## Option 1: VS Code Dev Containers (Recommended) | ||
|
|
||
| If you use VS Code, the easiest way is to use the provided Dev Container | ||
| configuration. | ||
|
|
||
| 1. Install [Docker Desktop](https://www.docker.com/products/docker-desktop). | ||
| 2. Install the | ||
| [Dev Containers extension](https://marketplace.visualstudio.com/items?itemName=ms-vscode-remote.remote-containers) | ||
| in VS Code. | ||
| 3. Open the project folder in VS Code. | ||
| 4. Click "Reopen in Container" when prompted, or run the command | ||
| `Dev Containers: Reopen in Container`. | ||
|
|
||
| This will automatically set up all dependencies (Rust, Python, CMake, Protobuf, | ||
| etc.). | ||
|
|
||
| ## Option 2: Manual Setup | ||
|
|
||
| ### 1. Install Prerequisites | ||
|
|
||
| #### Rust | ||
|
|
||
| Deno requires a specific version of Rust. | ||
|
|
||
| ```bash | ||
| # Install rustup if you haven't | ||
| curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh | ||
|
|
||
| # Install the specific version required by Deno (currently 1.90.0) | ||
| rustup install 1.90.0 | ||
| rustup default 1.90.0 | ||
| rustup component add rustfmt clippy | ||
| ``` | ||
|
|
||
| #### Python 3 | ||
|
|
||
| Ensure you have Python 3 installed and accessible as `python` or `python3`. | ||
|
|
||
| #### Protobuf Compiler | ||
|
|
||
| - **Mac:** `brew install protobuf` | ||
| - **Linux:** `apt install -y protobuf-compiler` | ||
| - **Windows:** Download binary release from GitHub. | ||
|
|
||
| #### CMake | ||
|
|
||
| - **Mac:** `brew install cmake` | ||
| - **Linux:** `apt install -y cmake` | ||
|
|
||
| #### Native Compilers | ||
|
|
||
| - **Mac:** XCode Command Line Tools (`xcode-select --install`) | ||
| - **Linux:** `apt install -y build-essential libglib2.0-dev` | ||
|
|
||
| ### 2. Build Deno | ||
|
|
||
| ```bash | ||
| # Clone with submodules if you haven't already | ||
| git submodule update --init --recursive | ||
|
|
||
| # Build | ||
| cargo build -vv | ||
| ``` | ||
|
|
||
| ### 3. Verify Setup | ||
|
|
||
| Run the tests to ensure everything is working: | ||
|
|
||
| ```bash | ||
| # Run unit tests | ||
| cargo test -vv | ||
|
|
||
| # Format code | ||
| ./tools/format.js | ||
|
|
||
| # Lint code | ||
| ./tools/lint.js | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This documentation appears unrelated to the PR objective.
The PR adds a --allow-env-file flag for environment variable permissions, but this file documents general Deno development environment setup with no mention of the new flag. Was this file accidentally included, or is there a broader context not reflected in the PR description?
| ```bash | ||
| # Install rustup if you haven't | ||
| curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh | ||
|
|
||
| # Install the specific version required by Deno (currently 1.90.0) | ||
| rustup install 1.90.0 | ||
| rustup default 1.90.0 | ||
| rustup component add rustfmt clippy | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for rust-toolchain configuration files and version specifications
echo "=== Looking for rust-toolchain files ==="
fd -t f "rust-toolchain" .
echo -e "\n=== Checking for Rust version in TOML files ==="
rg -n "^\[" Cargo.toml 2>/dev/null | head -20
echo -e "\n=== Searching for version specifications (1.90.0 or rust version references) ==="
rg -n "1\.90\.0|rust.*version|toolchain" --type toml --type json --type yaml -C 2
echo -e "\n=== Checking GitHub Actions workflows for Rust version ==="
fd -t f "\.yml$|\.yaml$" .github/workflows 2>/dev/null -exec basename {} \; 2>/dev/null | head -5Repository: denoland/deno
Length of output: 170
🌐 Web query:
Deno rust version requirement 1.90.0
💡 Result:
Do you mean: (A) confirm which Rust version Deno requires to build (e.g. "Rust 1.90.0"), or (B) how to install/use Rust 1.90.0 for building Deno?
🌐 Web query:
Deno build requirements minimum Rust version
💡 Result:
Short answer: Deno requires the Rust toolchain version specified in the repository's rust-toolchain.toml file — check that file in the denoland/deno repo for the exact minimum Rust version for the release you plan to build. [1][2]
How to check locally or in the repo:
- View the file on GitHub: https://github.com/denoland/deno and open rust-toolchain.toml. [1]
- Or from a shell after cloning: cat rust-toolchain.toml (the file explicitly pins the Rust toolchain to use). [2]
Sources:
- denoland/deno repository (README and repo files). [1]
- deno CONTRIBUTING / building-from-source instructions (notes that Deno requires the Rust release listed in rust-toolchain.toml). [2]
If you want, I can fetch the current rust-toolchain.toml and tell you the exact Rust version right now.
🌐 Web query:
site:github.com/denoland/deno rust-toolchain.toml
💡 Result:
The file rust-toolchain.toml is present at the root of the denoland/deno repository on GitHub. [1][2]
Sources
- denoland/deno repository file list (shows rust-toolchain.toml). [1]
- rust-toolchain.toml blob page in the denoland/deno repo. [2]
🌐 Web query:
fetch denoland deno rust-toolchain.toml raw content current version
💡 Result:
Here’s the rust-toolchain.toml content from the denoland/deno repo (current toolchain channel in the file):
[toolchain]
channel = "1.89.0"
components = ["rustfmt", "clippy"].
Sources: [1], [2]
Sources:
[1] mirror of denoland/deno rust-toolchain.toml (shows content).
[2] denoland/deno repository file listing on GitHub.
Rust version in documentation is incorrect — should be 1.89.0, not 1.90.0.
The documentation specifies Rust 1.90.0 (lines 37–38), but the project's rust-toolchain.toml file specifies channel = "1.89.0". Using the wrong version will cause build failures or unnecessary upgrades. Update the documentation to match the actual requirement:
# Install the specific version required by Deno (currently 1.89.0)
rustup install 1.89.0
rustup default 1.89.0
rustup component add rustfmt clippy🤖 Prompt for AI Agents
.agent/workflows/setup_local_env.md around lines 33 to 41: the docs install Rust
1.90.0 but the project requires 1.89.0 per rust-toolchain.toml; update the three
rustup commands to use 1.89.0 (install, default) and keep adding rustfmt and
clippy so the documented commands match the project's required Rust channel.
Description
This PR adds a new
--allow-env-fileflag.This flag allows users to specify an environment file (like
.env) and automatically grants read permissions only for the variables defined in that file.This addresses the need for a secure way to load environment variables from a file without granting global
--allow-envpermissions, which might expose sensitive CLI-level variables.fixes #31448
Behavior
deno run --allow-env-file=.env main.ts: Loads.envand grants read access to variables defined in it.deno run --allow-env-file=.env --allow-env=FOO main.ts: Loads.env, grants access to variables in it, AND grants access toFOO.Verification
cargo fmtandtools/lint.js.