Repository navigation
fix(config): honor trusted_config_paths over previously-ignored configs - #11152
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughConfig trust resolution now distinguishes explicit ignores from persisted ignores and allows ChangesTrusted Config Path Precedence
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant E2ETest
participant MiseEnv
participant ConfigDiscovery
participant ConfigTrust
E2ETest->>MiseEnv: run env with persisted ignored config
MiseEnv->>ConfigDiscovery: discover project config
ConfigDiscovery->>ConfigTrust: evaluate ignore and trust rules
ConfigTrust-->>ConfigDiscovery: exclude config
E2ETest->>MiseEnv: run env with trusted config path
MiseEnv->>ConfigDiscovery: rediscover project config
ConfigDiscovery->>ConfigTrust: prioritize trusted config path
ConfigTrust-->>ConfigDiscovery: include config
ConfigDiscovery-->>MiseEnv: load PROJECT
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR updates config trust precedence so configured trusted paths can override persisted ignores. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (5): Last reviewed commit: "fix(config): honor trusted_config_paths ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/config/config_file/mod.rs (2)
457-508: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueOptional: Avoid redundant path canonicalization.
is_trusted_via_config_paths,is_ignored_via_setting, andis_persisted_ignoredperformpath.canonicalize()internally. When these helpers are called fromis_trusted, they receivecanonicalized_path.as_path()(from line 348), resulting in redundant filesystem syscalls for paths that have already been resolved.If this becomes a hot path (e.g., during config discovery over many files), consider extracting inner
_canonicalvariants that accept an already-canonicalized&Pathto skip the extra syscalls, while keeping these outer helpers for external callers.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/config/config_file/mod.rs` around lines 457 - 508, Optionally eliminate redundant canonicalization in the is_trusted flow by adding internal _canonical helper variants for is_trusted_via_config_paths, is_ignored_via_setting, and is_persisted_ignored that accept an already-canonicalized path. Have is_trusted call these variants with canonicalized_path.as_path(), while preserving the existing public helpers’ canonicalization for external callers and their current precedence and failure behavior.
372-378: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReuse
is_trusted_via_config_pathsto DRY up this logic.These lines duplicate the exact logic introduced below in
is_trusted_via_config_paths. Reusing the helper makes this block more concise.♻️ Proposed refactor
- let settings = Settings::get(); - for p in settings.trusted_config_paths() { - if canonicalized_path.starts_with(p) { - add_trusted(canonicalized_path.to_path_buf()); - return true; - } - } + if is_trusted_via_config_paths(path) { + add_trusted(canonicalized_path.to_path_buf()); + return true; + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/config/config_file/mod.rs` around lines 372 - 378, In the current trust-checking function, replace the inline Settings::get and trusted_config_paths iteration with a call to the existing is_trusted_via_config_paths helper, preserving the current canonicalized_path matching and return behavior. Remove only the duplicated loop logic and keep add_trusted handling unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/config/config_file/mod.rs`:
- Around line 457-508: Optionally eliminate redundant canonicalization in the
is_trusted flow by adding internal _canonical helper variants for
is_trusted_via_config_paths, is_ignored_via_setting, and is_persisted_ignored
that accept an already-canonicalized path. Have is_trusted call these variants
with canonicalized_path.as_path(), while preserving the existing public helpers’
canonicalization for external callers and their current precedence and failure
behavior.
- Around line 372-378: In the current trust-checking function, replace the
inline Settings::get and trusted_config_paths iteration with a call to the
existing is_trusted_via_config_paths helper, preserving the current
canonicalized_path matching and return behavior. Remove only the duplicated loop
logic and keep add_trusted handling unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: e7fed740-c354-43fd-8b7b-bfe94d19663f
📒 Files selected for processing (3)
e2e/cli/test_trusted_config_paths_override_ignoresrc/config/config_file/mod.rssrc/config/mod.rs
There was a problem hiding this comment.
🧹 Nitpick comments (1)
e2e/cli/test_trusted_config_paths_override_ignore (1)
29-35: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCover explicit-ignore precedence as well.
This test verifies that
trusted_config_pathsoverrides the persisted ignore list, but it does not verify the other half of the contract: an explicitignored_config_pathsentry must still block loading even when the same path is trusted. Add a case assertingmise envomitsPROJECTwhen both settings are present.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e/cli/test_trusted_config_paths_override_ignore` around lines 29 - 35, Extend the trusted/ignored configuration path test to cover both settings targeting the same path. Add an explicit ignored_config_paths case alongside trusted_config_paths, run mise env, and assert that PROJECT is omitted, while preserving the existing trusted override assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@e2e/cli/test_trusted_config_paths_override_ignore`:
- Around line 29-35: Extend the trusted/ignored configuration path test to cover
both settings targeting the same path. Add an explicit ignored_config_paths case
alongside trusted_config_paths, run mise env, and assert that PROJECT is
omitted, while preserving the existing trusted override assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: c5b4e977-ac97-440e-a94a-ce6eeff93534
📒 Files selected for processing (3)
e2e/cli/test_trusted_config_paths_override_ignoresrc/config/config_file/mod.rssrc/config/mod.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- src/config/mod.rs
- src/config/config_file/mod.rs
`trusted_config_paths` (MISE_TRUSTED_CONFIG_PATHS) had no effect for any config that had been added to the persisted ignore list — by answering "No" to the trust prompt or running `mise trust --ignore`. `is_trusted()` consulted the ignore list before `trusted_config_paths`, so the setting never took effect, which also contradicted the `mise trust` warnings that a settings-trusted path "will still be trusted". Precedence is now: 1. `ignored_config_paths` setting — explicit hard block (unchanged) 2. `trusted_config_paths` / global config — overrides the persisted ignore list 3. persisted ignore list (dismissed prompt / `mise trust --ignore`) The override is applied in both the trust check (`is_trusted`) and config discovery (`config_path_is_ignored`), so a settings-trusted config is actually discovered and loaded, and `mise doctor`'s ignored-config list reflects it. Adds an e2e regression test. Addresses jdx#3544 *This PR was prepared by an AI coding assistant.* Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Head branch was pushed to by a user without write access
|
Rebased onto the latest |
trusted_config_paths(MISE_TRUSTED_CONFIG_PATHS) had no effect for any configthat had been added to the persisted ignore list — by answering "No" to the
trust prompt or running
mise trust --ignore.is_trusted()consulted theignore list before
trusted_config_paths, so the setting never took effect,which also contradicted the
mise trustwarnings that a settings-trusted path"will still be trusted".
Precedence is now:
ignored_config_pathssetting — explicit hard block (unchanged)trusted_config_paths/ global config — overrides the persisted ignore listmise trust --ignore)The override is applied in both the trust check (
is_trusted) and configdiscovery (
config_path_is_ignored), so a settings-trusted config is actuallydiscovered and loaded, and
mise doctor's ignored-config list reflects it.Adds an e2e regression test.
Addresses #3544
This PR was prepared by an AI coding assistant.
Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com
Summary by CodeRabbit
MISE_TRUSTED_CONFIG_PATHSis set, corresponding configurations are now discovered and loaded as trusted (including their environment variables).mise trust --show, and related environment output) more accurately reflects the active trusted-path rules.MISE_TRUSTED_CONFIG_PATHSoverriding persisted ignore behavior and validating trust status output.