Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 62 additions & 0 deletions e2e-win/settings_set_path_list.Tests.ps1
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
Describe 'mise settings set with a path list' {
BeforeAll {
$script:OriginalDir = Get-Location
# Saved here, restored in AfterAll: Pester runs every suite in one process, so removing an
# inherited value would leave the next suite without it.
$script:OriginalTrusted = [Environment]::GetEnvironmentVariable('MISE_TRUSTED_CONFIG_PATHS', 'Process')
$script:TestRoot = Join-Path $TestDrive ([System.Guid]::NewGuid().ToString())
New-Item -ItemType Directory -Path $script:TestRoot | Out-Null
$env:MISE_TRUSTED_CONFIG_PATHS = $script:TestRoot
$script:ConfigPath = Join-Path $script:TestRoot "mise.toml"

# Counting `C:\` distinguishes the two behaviours without depending on how toml_edit
# quotes a string: the colon split severed every drive letter from its path, so the
# broken output held a bare `C` entry and no `C:\` at all.
function Get-DriveRootedCount {
$written = Get-Content $script:ConfigPath | Out-String
([regex]::Matches($written, [regex]::Escape('C:\'))).Count
}
}

AfterAll {
Set-Location $script:OriginalDir
if ($null -eq $script:OriginalTrusted) {
Remove-Item Env:MISE_TRUSTED_CONFIG_PATHS -ErrorAction Ignore
} else {
[Environment]::SetEnvironmentVariable('MISE_TRUSTED_CONFIG_PATHS', $script:OriginalTrusted, 'Process')
}
}

BeforeEach {
# `task.disable_paths` rather than `trusted_config_paths`, which is global_only: --local
# would not apply to it and the write would land in the real user config.
Set-Location $script:TestRoot
"[tools]" | Out-File -Encoding ascii $script:ConfigPath
}

It 'keeps a lone path whole' {
# No list separator anywhere, and it still came apart before: the drive letter's colon
# is not a separator.
mise settings set --local task.disable_paths 'C:\one'
Get-DriveRootedCount | Should -Be 1
(mise settings get task.disable_paths | Out-String) | Should -Match ([regex]::Escape('C:\one'))
}

It 'splits on the semicolon rather than the drive letter' {
mise settings set --local task.disable_paths 'C:\one;C:\two'
Get-DriveRootedCount | Should -Be 2
$got = mise settings get task.disable_paths | Out-String
$got | Should -Match ([regex]::Escape('C:\one'))
$got | Should -Match ([regex]::Escape('C:\two'))
}

It 'appends without severing the path when adding' {
# `mise settings add` reaches the same parser, so it carried the same defect.
mise settings set --local task.disable_paths 'C:\one'
mise settings add --local task.disable_paths 'C:\two'
Get-DriveRootedCount | Should -Be 2
$got = mise settings get task.disable_paths | Out-String
$got | Should -Match ([regex]::Escape('C:\one'))
$got | Should -Match ([regex]::Escape('C:\two'))
}
}
2 changes: 1 addition & 1 deletion schema/mise.json
Original file line number Diff line number Diff line change
Expand Up @@ -2045,7 +2045,7 @@
},
"trusted_config_paths": {
"default": [],
"description": "This is a list of config paths that mise will automatically mark as trusted. Any config files under these paths will be trusted without prompting. Set to `[\"/\"]` to trust all config files, effectively disabling the trust mechanism. Paths are separated by the OS path separator when using the environment variable (`:` on Unix, `;` on Windows).",
"description": "This is a list of config paths that mise will automatically mark as trusted. Any config files under these paths will be trusted without prompting. Set to `[\"/\"]` to trust all config files, effectively disabling the trust mechanism. Paths are separated by the OS path separator when using the environment variable, `mise settings set`, or `mise settings add` (`:` on Unix, `;` on Windows).",
"type": "array",
"items": {
"type": "string"
Expand Down
20 changes: 10 additions & 10 deletions settings.toml
Original file line number Diff line number Diff line change
Expand Up @@ -406,8 +406,8 @@ This is an early-init setting: it must be set in `.miserc.toml`, environment
variables, or CLI flags. Setting it in `mise.toml` will have no effect because
config file discovery has already occurred by the time `mise.toml` is read.

Paths are separated by the OS path separator when using the environment variable
(`:` on Unix, `;` on Windows).
Paths are separated by the OS path separator when using the environment variable,
`mise settings set`, or `mise settings add` (`:` on Unix, `;` on Windows).
"""
env = "MISE_CEILING_PATHS"
parse_env = "list_by_os_path_separator"
Expand Down Expand Up @@ -1333,8 +1333,8 @@ This is an early-init setting: it must be set in `.miserc.toml`, environment
variables, or CLI flags. Setting it in `mise.toml` will have no effect because
config file discovery has already occurred by the time `mise.toml` is read.

Paths are separated by the OS path separator when using the environment variable
(`:` on Unix, `;` on Windows).
Paths are separated by the OS path separator when using the environment variable,
`mise settings set`, or `mise settings add` (`:` on Unix, `;` on Windows).
"""
env = "MISE_IGNORED_CONFIG_PATHS"
parse_env = "list_by_os_path_separator"
Expand Down Expand Up @@ -2494,8 +2494,8 @@ This is useful for shared environments like Docker containers or bastion hosts w
base set of tools is pre-installed in a shared location, and users can install additional
tools in their own directory.

Paths are separated by the OS path separator when using the environment variable
(`:` on Unix, `;` on Windows).
Paths are separated by the OS path separator when using the environment variable,
`mise settings set`, or `mise settings add` (`:` on Unix, `;` on Windows).
"""
env = "MISE_SHARED_INSTALL_DIRS"
optional = true
Expand Down Expand Up @@ -2919,8 +2919,8 @@ description = "Paths that mise will not look for tasks in."
docs = """
Paths that mise will not look for tasks in.

Paths are separated by the OS path separator when using the environment variable
(`:` on Unix, `;` on Windows).
Paths are separated by the OS path separator when using the environment variable,
`mise settings set`, or `mise settings add` (`:` on Unix, `;` on Windows).
"""
env = "MISE_TASK_DISABLE_PATHS"
parse_env = "list_by_os_path_separator"
Expand Down Expand Up @@ -3220,8 +3220,8 @@ default = []
description = """This is a list of config paths that mise will automatically mark as \
trusted. Any config files under these paths will be trusted without prompting. \
Set to `["/"]` to trust all config files, effectively disabling the trust mechanism. \
Paths are separated by the OS path separator when using the environment variable \
(`:` on Unix, `;` on Windows)."""
Paths are separated by the OS path separator when using the environment variable, \
`mise settings set`, or `mise settings add` (`:` on Unix, `;` on Windows)."""
env = "MISE_TRUSTED_CONFIG_PATHS"
global_only = true
parse_env = "list_by_os_path_separator"
Expand Down
107 changes: 104 additions & 3 deletions src/cli/settings/set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ pub fn set(mut key: &str, value: &str, add: bool, local: bool) -> Result<()> {
SettingsType::Duration => parse_duration(value)?,
SettingsType::Url | SettingsType::Path | SettingsType::String => value.into(),
SettingsType::ListString => parse_list_by_comma(value)?,
SettingsType::ListPath => parse_list_by_colon(value)?,
SettingsType::ListPath => parse_list_by_os_path_separator(value)?,
SettingsType::SetString => parse_set_by_comma(value)?,
SettingsType::IndexMap => parse_indexmap_by_json(value)?,
SettingsType::BoolOrString => parse_bool(value).unwrap_or_else(|_| value.into()),
Expand Down Expand Up @@ -129,11 +129,26 @@ fn parse_list_by_comma(value: &str) -> Result<toml_edit::Value> {
Ok(value.split(',').map(|s| s.trim().to_string()).collect())
}

fn parse_list_by_colon(value: &str) -> Result<toml_edit::Value> {
/// Split a path list the way the host does: `;` on Windows, `:` on Unix.
///
/// Splitting on `:` unconditionally made `mise settings set trusted_config_paths 'C:\proj'`
/// store `["C", "\proj"]`. A Windows drive letter's colon is not a separator, so even a lone
/// path with no list separator in it came apart, leaving no way to write one from the CLI.
/// #9058 gave the environment-variable route the OS separator; this is the CLI route it missed.
///
/// Deliberately the same `std::env::split_paths` that `list_by_os_path_separator` in
/// `config::settings` uses for `parse_env = "list_by_os_path_separator"`, so the two entry
/// points to the same settings cannot drift apart again. On Windows it also strips surrounding
/// double quotes, matching how the OS itself reads a path list.
fn parse_list_by_os_path_separator(value: &str) -> Result<toml_edit::Value> {
if value.is_empty() || value == "[]" {
return Ok(toml_edit::Array::new().into());
}
Ok(value.split(':').map(|s| s.trim().to_string()).collect())
// `std::env` rather than `crate::env`, which is what `env` resolves to elsewhere in mise.
Ok(std::env::split_paths(value)
.map(|p| p.to_string_lossy().trim().to_string())
.filter(|s| !s.is_empty())
.collect())
}

fn parse_set_by_comma(value: &str) -> Result<toml_edit::Value> {
Expand Down Expand Up @@ -188,3 +203,89 @@ static AFTER_LONG_HELP: &str = color_print::cstr!(
$ <bold>mise settings idiomatic_version_file=true</bold>
"#
);

#[cfg(test)]
mod tests {
use super::*;
use pretty_assertions::assert_eq;

fn entries(value: toml_edit::Value) -> Vec<String> {
value
.as_array()
.expect("a path list parses to an array")
.iter()
.map(|v| v.as_str().expect("array of strings").to_string())
.collect()
}

fn parse(value: &str) -> Vec<String> {
entries(parse_list_by_os_path_separator(value).unwrap())
}

fn expect(items: &[&str]) -> Vec<String> {
items.iter().map(|s| s.to_string()).collect()
}

/// The defect this function exists for: a drive letter's colon is not a separator, so a
/// path list splits on `;` here. Asserted only on Windows -- `\` and `;` are ordinary
/// filename characters on unix, where `C:\a` really is one relative path named `C:\a`.
#[cfg(windows)]
#[test]
fn windows_drive_letter_is_not_a_separator() {
// The sharpest form: no list separator anywhere, and it still came apart before.
assert_eq!(parse(r"C:\a"), expect(&[r"C:\a"]));
assert_eq!(
parse(r"C:\a\one;C:\b\two"),
expect(&[r"C:\a\one", r"C:\b\two"])
);
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved

/// `split_paths` unquotes on Windows, the way the OS reads a path list -- which is the only
/// way to spell a path containing the separator. Asserted so that swapping the split for a
/// hand-rolled one would fail here rather than quietly drop the quotes into the config.
#[cfg(windows)]
#[test]
fn windows_quoted_paths_are_unquoted() {
assert_eq!(
parse(r#""C:\Program Files\one";"C:\Program Files\two""#),
expect(&[r"C:\Program Files\one", r"C:\Program Files\two"])
);
// Quoting is per entry, not all-or-nothing.
assert_eq!(parse(r#""C:\a b";C:\c"#), expect(&[r"C:\a b", r"C:\c"]));
}

/// Unix keeps the colon it always had. This is the control for the change above: the
/// separator moved with the platform rather than everywhere.
#[cfg(not(windows))]
#[test]
fn unix_still_splits_on_colon() {
assert_eq!(parse("/a/one:/b/two"), expect(&["/a/one", "/b/two"]));
assert_eq!(parse("/a"), expect(&["/a"]));
}

#[test]
fn empty_input_parses_to_an_empty_array() {
assert!(parse("").is_empty());
assert!(parse("[]").is_empty());
}

#[test]
fn surrounding_whitespace_is_trimmed() {
#[cfg(windows)]
let (input, expected) = (r" C:\a ; C:\b ", expect(&[r"C:\a", r"C:\b"]));
#[cfg(not(windows))]
let (input, expected) = (" /a : /b ", expect(&["/a", "/b"]));
assert_eq!(parse(input), expected);
}

/// An empty segment is not a path. `list_by_os_path_separator` and `env::split_colon_list`
/// both drop them; writing one into the config would only be dropped again on read.
#[test]
fn empty_segments_are_dropped() {
#[cfg(windows)]
let (input, expected) = (r";C:\a;;C:\b;", expect(&[r"C:\a", r"C:\b"]));
#[cfg(not(windows))]
let (input, expected) = (":/a::/b:", expect(&["/a", "/b"]));
assert_eq!(parse(input), expected);
}
}
Loading