From 48f25595ca147a65515403b03ed4e15e673e634b Mon Sep 17 00:00:00 2001 From: Kris <2040188027@qq.com> Date: Wed, 30 Sep 2026 23:34:36 +0800 Subject: [PATCH 1/3] fix(cli): read PATH at its real length and probe Windows shim dirs (#221) Two independent reasons `install` missed opencode on Windows 11: 1. find_in_path() copied PATH into a fixed 4 KB stack buffer through cbm_safe_getenv(), and that helper REFUSES a value it cannot fit instead of truncating it. On a machine whose PATH exceeds 4096 characters the whole PATH was therefore skipped before a single directory was probed. 2. The fallback list had no Windows-shaped entries, and it probed only the bare name, so npm's %APPDATA%\npm, mise's %LOCALAPPDATA%\mise\shims and scoop's %USERPROFILE%\scoop\shims could never match a .cmd shim. - Add cbm_env_dup() to platform.c: the allocating sibling of cbm_safe_getenv, memory-core owned, wide-read on Windows so a non-ASCII PATH is not mangled by the ANSI code page. cli.c no longer needs in the middle of the file, nor any raw allocation. - find_in_path() now exits once, so the PATH copy is freed on every path instead of leaking on each success. - Keep the existing pipx (~/.local/bin) and cargo (~/.cargo/bin) fallbacks, built from the home_dir argument rather than cbm_get_home_dir(), and add the npm/mise/scoop locations. PATHEXT is probed for every fallback directory, not only for PATH entries. - Bound the fallback loop by NUM_PATHS, not NUM_RETRIES. - Hoist the PATHEXT list into one file-scope constant and add braces to every one-line statement. Regression guards in tests/repro/repro_issue221.c: a PATH longer than 4096 characters whose LAST entry holds the fixture, and the Windows fallback shims driven through APPDATA / LOCALAPPDATA / the home_dir argument. Both are red against the previous code and green with this change. The PATH case is not Windows-specific: the same 4 KB cap hid entries on macOS and Linux. No raw allocator call site is added under src/, so the memory-core ratchet is untouched; clang-format and the no-skips policy both pass locally. Signed-off-by: Kris <2040188027@qq.com> --- src/cli/cli.c | 115 ++++++++++++++----- src/foundation/platform.c | 71 ++++++++++++ src/foundation/platform.h | 13 +++ tests/repro/repro_issue221.c | 208 +++++++++++++++++++++++++++++++++++ 4 files changed, 379 insertions(+), 28 deletions(-) diff --git a/src/cli/cli.c b/src/cli/cli.c index 6e33759e4..d8e91e7a0 100644 --- a/src/cli/cli.c +++ b/src/cli/cli.c @@ -22,6 +22,7 @@ #include "foundation/platform.h" #include "foundation/constants.h" #include "foundation/log.h" +#include "foundation/mem_core.h" #include "foundation/sha256.h" #include "cli/client_adapter.h" #include "mcp/mcp.h" // cbm_mcp_tool_input_schema — CLI flag parser + per-tool --help @@ -1076,36 +1077,50 @@ static bool is_executable(const char *path) { #endif } +/* Windows ships CLIs as extension-bearing shims -- .cmd from npm, .ps1 from + * mise, .exe from scoop -- so a bare-name probe never matches there. One + * definition serves both the PATH scan and the fallback-directory probe (#221). */ +#ifdef _WIN32 +static const char *const WIN_EXEC_EXTS[] = {".exe", ".cmd", ".bat", ".ps1", NULL}; +#endif + /* Search for an executable named `name` in the PATH environment variable. - * Returns the full path in `out` (max out_sz) if found, else empty string. */ + * Returns the full path in `out` (max out_sz) if found, else empty string. + * + * The PATH copy is sized to the REAL value, not to a fixed 4 KB stack buffer. + * That buffer was the first root cause of #221: a Windows dev machine's PATH + * routinely runs past 4096 characters, and cbm_safe_getenv() REFUSES a value it + * cannot fit rather than truncating it -- so find_in_path() returned false + * before looking at a single directory, and opencode (installed through + * mise/npm/scoop and present on PATH) read as "not installed". The same 4 KB + * cap hid entries on macOS/Linux too; the fix is not Windows-specific. */ static bool find_in_path(const char *name, char *out, size_t out_sz) { - char path_copy[CLI_BUF_4K]; - if (!cbm_safe_getenv("PATH", path_copy, sizeof(path_copy), NULL)) { + char *path_copy = cbm_env_dup("PATH"); + if (!path_copy) { return false; } - char *saveptr; + + /* One exit. PATH is a full live copy of the environment, the scan has + * several ways to stop, and the previous shape returned straight out of the + * loop -- which is exactly what leaked a PATH-sized allocation per call. */ + bool found = false; + char *saveptr = NULL; char *dir = strtok_r(path_copy, PATH_DELIM, &saveptr); - while (dir) { + while (dir != NULL && !found) { snprintf(out, out_sz, "%s/%s", dir, name); - if (is_executable(out)) { - return true; - } + found = is_executable(out); #ifdef _WIN32 - /* On Windows executables carry an extension (PATHEXT). A CLI like - * opencode is often installed as a .cmd / .ps1 / .exe shim (e.g. via - * mise or npm), so the bare-name probe above misses it (#221). Try the - * common executable extensions before moving to the next PATH entry. */ - static const char *const win_exts[] = {".exe", ".cmd", ".bat", ".ps1", NULL}; - for (int i = 0; win_exts[i]; i++) { - snprintf(out, out_sz, "%s/%s%s", dir, name, win_exts[i]); - if (is_executable(out)) { - return true; - } + /* PATHEXT variants for THIS entry, before moving to the next one. */ + for (int i = 0; !found && WIN_EXEC_EXTS[i] != NULL; i++) { + snprintf(out, out_sz, "%s/%s%s", dir, name, WIN_EXEC_EXTS[i]); + found = is_executable(out); } #endif dir = strtok_r(NULL, PATH_DELIM, &saveptr); } - return false; + + cbm_free(CBM_MEM_CLASS_OTHER, path_copy); + return found; } const char *cbm_find_cli(const char *name, const char *home_dir) { @@ -1119,22 +1134,66 @@ const char *cbm_find_cli(const char *name, const char *home_dir) { if (!home_dir || !home_dir[0]) { return ""; } + /* PATH-independent fallbacks, tried in order. Every home-derived entry is + * built from the `home_dir` ARGUMENT and not from cbm_get_home_dir(): a + * dry-run scan or a test passes a synthetic home and must not be shown the + * host's agents. `/` separators are fine on Windows -- the wide-safe file + * layer accepts them, and the rest of this file already composes paths so. */ enum { NUM_PATHS = 5 }; char paths[NUM_PATHS][CLI_BUF_512]; - snprintf(paths[0], sizeof(paths[0]), "/usr/local/bin/%s", name); - snprintf(paths[1], sizeof(paths[1]), "%s/.npm/bin/%s", home_dir, name); - snprintf(paths[2], sizeof(paths[2]), "%s/.local/bin/%s", home_dir, name); - snprintf(paths[3], sizeof(paths[3]), "%s/.cargo/bin/%s", home_dir, name); -#ifdef __APPLE__ - snprintf(paths[4], sizeof(paths[4]), "/opt/homebrew/bin/%s", name); + memset(paths, 0, sizeof(paths)); + int n = 0; +#ifdef _WIN32 + /* Locations that were already probed on Windows before #221 and are real + * there: pipx installs into ~/.local/bin and cargo into ~/.cargo/bin, so + * replacing the list rather than extending it would make pipx- and + * cargo-installed agents disappear. */ + snprintf(paths[n++], sizeof(paths[0]), "%s/.local/bin/%s", home_dir, name); + snprintf(paths[n++], sizeof(paths[0]), "%s/.cargo/bin/%s", home_dir, name); + /* Windows-shaped locations (issue #221, second root cause): npm puts its + * global bin under %APPDATA%\npm, and mise and scoop expose shims under + * %LOCALAPPDATA%\mise\shims and %USERPROFILE%\scoop\shims. Each is read + * through its own environment seam, so the list stays testable. */ + const char *appdata = cbm_app_config_dir(); + if (appdata) { + snprintf(paths[n++], sizeof(paths[0]), "%s/npm/%s", appdata, name); + } + const char *localapp = cbm_app_local_dir(); + if (localapp) { + snprintf(paths[n++], sizeof(paths[0]), "%s/mise/shims/%s", localapp, name); + } + snprintf(paths[n++], sizeof(paths[0]), "%s/scoop/shims/%s", home_dir, name); #else - paths[4][0] = '\0'; + snprintf(paths[n++], sizeof(paths[0]), "/usr/local/bin/%s", name); + snprintf(paths[n++], sizeof(paths[0]), "%s/.npm/bin/%s", home_dir, name); + snprintf(paths[n++], sizeof(paths[0]), "%s/.local/bin/%s", home_dir, name); + snprintf(paths[n++], sizeof(paths[0]), "%s/.cargo/bin/%s", home_dir, name); +#ifdef __APPLE__ + snprintf(paths[n++], sizeof(paths[0]), "/opt/homebrew/bin/%s", name); #endif - for (int i = 0; i < NUM_RETRIES; i++) { - if (paths[i][0] && is_executable(paths[i])) { +#endif + /* NUM_PATHS, not NUM_RETRIES: both are 5 today, so the old bound only + * looked right by coincidence -- what the loop means is the list length. */ + for (int i = 0; i < NUM_PATHS; i++) { + if (!paths[i][0]) { + continue; + } + if (is_executable(paths[i])) { snprintf(buf, sizeof(buf), "%s", paths[i]); return buf; } +#ifdef _WIN32 + /* The npm/mise/scoop shims carry an extension, so every fallback + * directory needs the same PATHEXT probe the PATH scan got. */ + for (int e = 0; WIN_EXEC_EXTS[e] != NULL; e++) { + char extpath[CLI_BUF_512]; + snprintf(extpath, sizeof(extpath), "%s%s", paths[i], WIN_EXEC_EXTS[e]); + if (is_executable(extpath)) { + snprintf(buf, sizeof(buf), "%s", extpath); + return buf; + } + } +#endif } return ""; } diff --git a/src/foundation/platform.c b/src/foundation/platform.c index cf9a0f469..410536030 100644 --- a/src/foundation/platform.c +++ b/src/foundation/platform.c @@ -7,6 +7,7 @@ #include "foundation/compat.h" #include "foundation/constants.h" +#include "foundation/mem_core.h" #include "foundation/platform_internal.h" #include #include @@ -437,6 +438,76 @@ const char *cbm_safe_getenv(const char *name, char *buf, size_t buf_sz, const ch return NULL; } +/* See platform.h. The long-value sibling of cbm_safe_getenv: there is no caller + * buffer to overrun, so a PATH past 4 KB is read in full instead of being + * refused (#221). Both the wide staging buffer and the UTF-8 result are + * allocated through the memory core, which is also the only allocator the + * caller may release them with -- keeping the memory-core ratchet honest means + * this file adds no raw allocation site to pay for the new read. */ +char *cbm_env_dup(const char *name) { + if (!name || !name[0]) { + return NULL; + } +#ifdef _WIN32 + /* Same wide route as cbm_safe_getenv: _environ holds ANSI-code-page bytes, + * so a non-ASCII PATH would arrive mojibake'd or with '?' substituted -- + * invalid in a Windows path and fatal to every file API downstream. */ + wchar_t wname[CBM_SZ_256]; + int wn = MultiByteToWideChar(CP_UTF8, MB_ERR_INVALID_CHARS, name, -1, wname, CBM_SZ_256); + if (wn <= 0) { + return NULL; + } + SetLastError(ERROR_SUCCESS); + DWORD needed = GetEnvironmentVariableW(wname, NULL, 0U); + DWORD lookup_error = GetLastError(); + if (needed == 0U) { + if (lookup_error != ERROR_ENVVAR_NOT_FOUND) { + /* Present but empty. Distinct from unset, and callers are entitled + * to tell the two apart, so return "" and not NULL. */ + return cbm_mem_strdup(CBM_MEM_CLASS_OTHER, ""); + } + return NULL; + } + wchar_t *wvalue = cbm_calloc(CBM_MEM_CLASS_OTHER, (size_t)needed * sizeof(*wvalue)); + if (!wvalue) { + return NULL; + } + SetLastError(ERROR_SUCCESS); + DWORD got = GetEnvironmentVariableW(wname, wvalue, needed); + DWORD read_error = GetLastError(); + if (got >= needed || (got == 0U && read_error != ERROR_SUCCESS)) { + cbm_free(CBM_MEM_CLASS_OTHER, wvalue); + return NULL; + } + int utf8_size = + WideCharToMultiByte(CP_UTF8, WC_ERR_INVALID_CHARS, wvalue, -1, NULL, 0, NULL, NULL); + char *utf8 = utf8_size > 0 ? cbm_alloc(CBM_MEM_CLASS_OTHER, (size_t)utf8_size) : NULL; + int converted = utf8 ? WideCharToMultiByte(CP_UTF8, WC_ERR_INVALID_CHARS, wvalue, -1, utf8, + utf8_size, NULL, NULL) + : 0; + if (!utf8 || converted != utf8_size) { + cbm_free(CBM_MEM_CLASS_OTHER, utf8); + cbm_free(CBM_MEM_CLASS_OTHER, wvalue); + return NULL; + } + cbm_free(CBM_MEM_CLASS_OTHER, wvalue); + return utf8; +#else + /* Same environ walk as cbm_safe_getenv: getenv() returns a pointer another + * thread can invalidate with setenv(), so the value is copied out here. */ + char **env = CBM_ENVIRON; + if (env) { + size_t name_len = strlen(name); + for (; *env; env++) { + if (strncmp(*env, name, name_len) == 0 && (*env)[name_len] == '=') { + return cbm_mem_strdup(CBM_MEM_CLASS_OTHER, *env + name_len + SKIP_ONE); + } + } + } + return NULL; +#endif +} + /* See platform.h. The shape here is the one src/main.c:1104 already uses for * --port=: an end pointer says where the read stopped, errno catches a number * too large, and *end == '\0' catches anything left over. */ diff --git a/src/foundation/platform.h b/src/foundation/platform.h index 43738f621..52e01dc46 100644 --- a/src/foundation/platform.h +++ b/src/foundation/platform.h @@ -138,6 +138,19 @@ int cbm_default_worker_count(bool initial); * Returns NULL when the variable is unset and fallback is NULL. */ const char *cbm_safe_getenv(const char *name, char *buf, size_t buf_sz, const char *fallback); +/* Allocating sibling of cbm_safe_getenv, for a value that must not be capped at + * a caller-chosen size. A Windows PATH routinely runs past the 4096-byte buffer + * cbm_safe_getenv's callers pass, and the copy helper REPORTS that as failure + * rather than truncating -- so the whole PATH was skipped and every directory + * past the cut became invisible (#221). + * + * Returns a UTF-8 copy owned by the memory core: release it with + * cbm_free(CBM_MEM_CLASS_OTHER, ptr), never free(). NULL when the variable is + * unset or cannot be read; a variable that exists but is empty yields "". + * On Windows the value is read with the wide API and converted, the same route + * cbm_safe_getenv takes, so a non-ASCII value survives intact. */ +char *cbm_env_dup(const char *name); + /* Read an environment variable as a whole number. * * Answers true only when the variable is set, is not empty, and reads cleanly diff --git a/tests/repro/repro_issue221.c b/tests/repro/repro_issue221.c index cb4d27fd4..2baf2705d 100644 --- a/tests/repro/repro_issue221.c +++ b/tests/repro/repro_issue221.c @@ -152,7 +152,215 @@ TEST(repro_issue221_opencode_pathext_lookup) { PASS(); } +/* ── #221 regression: PATH longer than the old 4 KB copy ───────────────── */ + +/* Snapshot/restore one environment variable. The suite swaps PATH and the + * Windows profile roots, and a leaked swap would poison every later suite. */ +static char *repro221_save_env(const char *name) { + const char *value = getenv(name); + return value ? strdup(value) : NULL; +} + +static void repro221_restore_env(const char *name, char *saved) { + if (saved) { + cbm_setenv(name, saved, 1); + free(saved); + } else { + cbm_unsetenv(name); + } +} + +/* + * repro_issue221_long_path_beyond_four_kb + * + * find_in_path() used to copy PATH into a fixed 4096-byte stack buffer through + * cbm_safe_getenv(). That helper does not truncate -- it REFUSES a value it + * cannot fit -- so on a machine whose PATH exceeds 4096 characters the whole + * PATH was skipped and every agent on it read as "not installed". That is the + * layout the #221 reporter had: mise, npm and scoop each append entries. + * + * The fixture directory is the LAST PATH entry, so any truncation of the copy + * hides exactly the directory under test and nothing else. This case is not + * Windows-specific -- the same 4 KB cap hid entries on macOS and Linux. + * + * RED before the fix (empty result), GREEN after it. + */ +TEST(repro_issue221_long_path_beyond_four_kb) { + const char *created = th_mktempdir("repro221long"); + if (!created) { + FAIL("th_mktempdir failed"); + } + char tmpdir[512]; + snprintf(tmpdir, sizeof(tmpdir), "%s", created); + +#ifdef _WIN32 + const char *fixture_name = "cbm-repro221-long.cmd"; + const char *fixture_body = "@echo off\r\nrem fake long-PATH agent\r\n"; + const char path_delim = ';'; +#else + const char *fixture_name = "cbm-repro221-long"; + const char *fixture_body = "#!/bin/sh\n# fake long-PATH agent\n"; + const char path_delim = ':'; +#endif + + char fixture_path[1024]; + snprintf(fixture_path, sizeof(fixture_path), "%s/%s", tmpdir, fixture_name); + if (th_write_file(fixture_path, fixture_body) != 0) { + th_cleanup(tmpdir); + FAIL("could not write the long-PATH fixture"); + } + th_make_executable(fixture_path); + + /* + * Filler entries name directories that do not exist: the scan has to walk + * past every one of them to reach the fixture, which is appended after the + * filler so that it always sits beyond the 4096-byte mark. + */ + enum { FILLER_TARGET = 4600, PATH_CAP = FILLER_TARGET + 1024 }; + char long_path[PATH_CAP]; + size_t used = 0U; + while (used < (size_t)FILLER_TARGET) { + int written = snprintf(long_path + used, sizeof(long_path) - used, + "/nonexistent-repro221-filler%05zu%c", used, path_delim); + if (written <= 0 || (size_t)written >= sizeof(long_path) - used) { + th_cleanup(tmpdir); + FAIL("could not build the long PATH filler"); + } + used += (size_t)written; + } + ASSERT_GT(used, 4096); + int tail = snprintf(long_path + used, sizeof(long_path) - used, "%s", tmpdir); + if (tail <= 0 || (size_t)tail >= sizeof(long_path) - used) { + th_cleanup(tmpdir); + FAIL("could not append the fixture directory to the long PATH"); + } + + char *saved_path = repro221_save_env("PATH"); + cbm_setenv("PATH", long_path, 1); + + /* A home that does not exist keeps the PATH-independent fallbacks out of + * the way: the fixture's directory is the only place that can satisfy it. */ + const char *result = cbm_find_cli("cbm-repro221-long", "/nonexistent-repro221-home"); + + repro221_restore_env("PATH", saved_path); + + ASSERT_NOT_NULL(result); + ASSERT(result[0] != '\0'); + ASSERT(strncmp(result, tmpdir, strlen(tmpdir)) == 0); + ASSERT(strstr(result, "cbm-repro221-long") != NULL); + + th_cleanup(tmpdir); + PASS(); +} + +/* ── #221 regression: the Windows fallback shim locations ───────────────── */ + +/* + * repro_issue221_windows_fallback_shims + * + * An agent the PATH scan cannot see must still be found where Windows actually + * puts CLIs: %APPDATA%\npm (npm's global bin), %LOCALAPPDATA%\mise\shims, + * %USERPROFILE%\scoop\shims, plus the pipx and cargo homes (~\.local\bin, + * ~\.cargo\bin) that were already probed before #221 passed and must not be + * dropped by the fix. + * + * Each location is reached through its own seam -- APPDATA / LOCALAPPDATA, or + * the home_dir argument -- so this never reads the developer's real profile. + * Every shim carries a .cmd extension, which is why the fallback probe needs + * the same PATHEXT treatment the PATH scan got. + * + * Windows-only: the directory layout it asserts is the one Windows installers + * produce. */ +TEST(repro_issue221_windows_fallback_shims) { +#ifndef _WIN32 + SKIP_PLATFORM("Windows-only: npm/mise/scoop shim fallback layout"); +#else + const char *created = th_mktempdir("repro221fb"); + if (!created) { + FAIL("th_mktempdir failed"); + } + char root[512]; + snprintf(root, sizeof(root), "%s", created); + + char *saved_appdata = repro221_save_env("APPDATA"); + char *saved_localappdata = repro221_save_env("LOCALAPPDATA"); + char *saved_userprofile = repro221_save_env("USERPROFILE"); + char *saved_path = repro221_save_env("PATH"); + + /* APPDATA/LOCALAPPDATA/USERPROFILE all point at the fixture root. The + * scoop, pipx and cargo entries are built from the home_dir ARGUMENT, so + * USERPROFILE here mirrors what a real profile would supply. */ + cbm_setenv("APPDATA", root, 1); + cbm_setenv("LOCALAPPDATA", root, 1); + cbm_setenv("USERPROFILE", root, 1); + cbm_setenv("PATH", "/nonexistent-repro221-path", 1); + + /* One shim at a time, so the returned path proves which entry matched + * rather than merely that something did. Each case removes its shim before + * the next one runs, so the earlier entries cannot shadow the later ones. */ + const struct { + const char *subdir; + const char *label; + } cases[] = { + {".local/bin", "pipx home"}, {".cargo/bin", "cargo home"}, {"npm", "APPDATA npm"}, + {"mise/shims", "LOCALAPPDATA mise"}, {"scoop/shims", "scoop home"}, + }; + const char *shim_name = "cbm-repro221-fb.cmd"; + const size_t case_count = sizeof(cases) / sizeof(cases[0]); + + for (size_t i = 0; i < case_count; i++) { + char dir[512]; + snprintf(dir, sizeof(dir), "%s/%s", root, cases[i].subdir); + if (th_mkdir_p(dir) != 0) { + repro221_restore_env("PATH", saved_path); + repro221_restore_env("USERPROFILE", saved_userprofile); + repro221_restore_env("LOCALAPPDATA", saved_localappdata); + repro221_restore_env("APPDATA", saved_appdata); + th_cleanup(root); + FAIL("could not create the fallback fixture directory"); + } + char shim[1024]; + snprintf(shim, sizeof(shim), "%s/%s", dir, shim_name); + if (th_write_file(shim, "@echo off\r\nrem fake fallback shim\r\n") != 0) { + repro221_restore_env("PATH", saved_path); + repro221_restore_env("USERPROFILE", saved_userprofile); + repro221_restore_env("LOCALAPPDATA", saved_localappdata); + repro221_restore_env("APPDATA", saved_appdata); + th_cleanup(root); + FAIL("could not write the fallback shim"); + } + + const char *result = cbm_find_cli("cbm-repro221-fb", root); + char expected_prefix[512]; + snprintf(expected_prefix, sizeof(expected_prefix), "%s/%s/", root, cases[i].subdir); + bool matched = result != NULL && result[0] != '\0' && + strncmp(result, expected_prefix, strlen(expected_prefix)) == 0; + (void)remove(shim); + + if (!matched) { + printf(" fallback case %zu (%s) did not resolve in %s\n", i, cases[i].label, dir); + repro221_restore_env("PATH", saved_path); + repro221_restore_env("USERPROFILE", saved_userprofile); + repro221_restore_env("LOCALAPPDATA", saved_localappdata); + repro221_restore_env("APPDATA", saved_appdata); + th_cleanup(root); + FAIL("a Windows fallback shim location was not probed"); + } + } + + repro221_restore_env("PATH", saved_path); + repro221_restore_env("USERPROFILE", saved_userprofile); + repro221_restore_env("LOCALAPPDATA", saved_localappdata); + repro221_restore_env("APPDATA", saved_appdata); + + th_cleanup(root); + PASS(); +#endif +} + /* ── Suite ──────────────────────────────────────────────────────────────── */ SUITE(repro_issue221) { RUN_TEST(repro_issue221_opencode_pathext_lookup); + RUN_TEST(repro_issue221_long_path_beyond_four_kb); + RUN_TEST(repro_issue221_windows_fallback_shims); } From 788dbe075ea62e83b817582267fb734c09a6419b Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Tue, 20 Oct 2026 18:33:16 +0200 Subject: [PATCH 2/3] fix(platform): cbm_env_dup answers through the settings resolver (#221) Since this branch was written, main routes cbm_safe_getenv through a startup settings resolver before it reads the process environment. cbm_env_dup, its allocating sibling, read only the process environment, so after the merge the two helpers could give different answers for the same name. Consult the resolver first, exactly as cbm_safe_getenv does: a handled value is copied through the memory core, and a handled NULL masks the variable. The resolver serves CBM_* settings today, so PATH lookups behave as before; this keeps both helpers one view of the environment by construction. platform_env_dup_honours_the_settings_resolver pins the three cases: resolved, masked, and falling through to the process environment. Signed-off-by: Martin Vogel --- src/foundation/platform.c | 6 +++++ src/foundation/platform.h | 4 +++- tests/test_platform.c | 48 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 57 insertions(+), 1 deletion(-) diff --git a/src/foundation/platform.c b/src/foundation/platform.c index ee407e258..d11df686e 100644 --- a/src/foundation/platform.c +++ b/src/foundation/platform.c @@ -478,6 +478,12 @@ char *cbm_env_dup(const char *name) { if (!name || !name[0]) { return NULL; } + /* The same view cbm_safe_getenv gives: the startup settings resolver + * answers first, and a handled NULL masks the variable. */ + const char *resolved = NULL; + if (environment_resolver && environment_resolver(name, &resolved)) { + return resolved ? cbm_mem_strdup(CBM_MEM_CLASS_OTHER, resolved) : NULL; + } #ifdef _WIN32 /* Same wide route as cbm_safe_getenv: _environ holds ANSI-code-page bytes, * so a non-ASCII PATH would arrive mojibake'd or with '?' substituted -- diff --git a/src/foundation/platform.h b/src/foundation/platform.h index aa3536b96..40b020222 100644 --- a/src/foundation/platform.h +++ b/src/foundation/platform.h @@ -157,7 +157,9 @@ const char *cbm_native_getenv(const char *name, char *buf, size_t buf_sz, const * cbm_free(CBM_MEM_CLASS_OTHER, ptr), never free(). NULL when the variable is * unset or cannot be read; a variable that exists but is empty yields "". * On Windows the value is read with the wide API and converted, the same route - * cbm_safe_getenv takes, so a non-ASCII value survives intact. */ + * cbm_safe_getenv takes, so a non-ASCII value survives intact. Like + * cbm_safe_getenv, an installed settings resolver answers before the process + * environment does. */ char *cbm_env_dup(const char *name); /* Read an environment variable as a whole number. diff --git a/tests/test_platform.c b/tests/test_platform.c index d1434c2ee..8b18a4884 100644 --- a/tests/test_platform.c +++ b/tests/test_platform.c @@ -7,6 +7,7 @@ #include "../src/foundation/compat_regex.h" #include "../src/foundation/constants.h" #include "../src/foundation/compat_thread.h" +#include "../src/foundation/mem_core.h" #include "../src/foundation/platform.h" #include "../src/foundation/platform_internal.h" #include "../src/foundation/system_info_internal.h" @@ -895,6 +896,52 @@ TEST(platform_env_long_refuses_what_it_cannot_read) { PASS(); } +/* cbm_env_dup (#221) gives the same environment view as cbm_safe_getenv: an + * installed settings resolver answers first, a handled NULL masks the + * variable, and an unhandled name falls through to the process environment. */ +static bool platform_env_dup_test_resolver(const char *name, const char **value) { + if (strcmp(name, "CBM_TEST_ENV_DUP_RESOLVED") == 0) { + *value = "from-settings"; + return true; + } + if (strcmp(name, "CBM_TEST_ENV_DUP_MASKED") == 0) { + *value = NULL; + return true; + } + return false; +} + +TEST(platform_env_dup_honours_the_settings_resolver) { + ASSERT_EQ(cbm_setenv("CBM_TEST_ENV_DUP_RESOLVED", "from-environment", 1), 0); + ASSERT_EQ(cbm_setenv("CBM_TEST_ENV_DUP_MASKED", "from-environment", 1), 0); + ASSERT_EQ(cbm_setenv("CBM_TEST_ENV_DUP_NATIVE", "from-environment", 1), 0); + + cbm_set_environment_resolver(platform_env_dup_test_resolver); + char *resolved = cbm_env_dup("CBM_TEST_ENV_DUP_RESOLVED"); + char *masked = cbm_env_dup("CBM_TEST_ENV_DUP_MASKED"); + char *native = cbm_env_dup("CBM_TEST_ENV_DUP_NATIVE"); + char safe[64]; + const char *safe_value = cbm_safe_getenv("CBM_TEST_ENV_DUP_RESOLVED", safe, sizeof(safe), NULL); + cbm_set_environment_resolver(NULL); + + bool safe_ok = safe_value && strcmp(safe_value, "from-settings") == 0; + bool resolved_ok = resolved && strcmp(resolved, "from-settings") == 0; + bool masked_ok = masked == NULL; + bool native_ok = native && strcmp(native, "from-environment") == 0; + cbm_free(CBM_MEM_CLASS_OTHER, resolved); + cbm_free(CBM_MEM_CLASS_OTHER, masked); + cbm_free(CBM_MEM_CLASS_OTHER, native); + (void)cbm_unsetenv("CBM_TEST_ENV_DUP_RESOLVED"); + (void)cbm_unsetenv("CBM_TEST_ENV_DUP_MASKED"); + (void)cbm_unsetenv("CBM_TEST_ENV_DUP_NATIVE"); + + ASSERT_TRUE(safe_ok); + ASSERT_TRUE(resolved_ok); + ASSERT_TRUE(masked_ok); + ASSERT_TRUE(native_ok); + PASS(); +} + #ifdef _WIN32 /* cbm_safe_getenv reads Windows' wide environment as UTF-8. Its matching * setter must update that same wide environment; _putenv_s alone interprets @@ -1420,6 +1467,7 @@ SUITE(platform) { RUN_TEST(platform_default_workers_env_unset); RUN_TEST(platform_env_long_reads_a_clean_number); RUN_TEST(platform_env_long_refuses_what_it_cannot_read); + RUN_TEST(platform_env_dup_honours_the_settings_resolver); RUN_TEST(platform_system_info); RUN_TEST(regex_compile_refuses_oversized_expansion); RUN_TEST(regex_compile_accepts_expansion_under_budget); From 079fb2067a8432310dac88240f926000b94f0582 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Tue, 20 Oct 2026 18:33:20 +0200 Subject: [PATCH 3/3] test(cli): run the #221 long-PATH and Windows shim regressions in the cli suite The two regressions sat in tests/repro/repro_issue221.c, which builds only into the non-gating bug-repro runner, so CI could not stop the fix from regressing. The Windows-only case also skipped on macOS and Linux, and scripts/repro.sh reports any skipped case as an incomplete board. Move both into tests/test_cli.c next to the other cbm_find_cli tests, unchanged apart from their names: the cli suite runs on every CI leg, and SKIP_PLATFORM is how that suite already marks Windows-only cases. tests/repro/repro_issue221.c is back to its content on main. Signed-off-by: Martin Vogel --- tests/repro/repro_issue221.c | 208 ----------------------------------- tests/test_cli.c | 208 +++++++++++++++++++++++++++++++++++ 2 files changed, 208 insertions(+), 208 deletions(-) diff --git a/tests/repro/repro_issue221.c b/tests/repro/repro_issue221.c index 2baf2705d..cb4d27fd4 100644 --- a/tests/repro/repro_issue221.c +++ b/tests/repro/repro_issue221.c @@ -152,215 +152,7 @@ TEST(repro_issue221_opencode_pathext_lookup) { PASS(); } -/* ── #221 regression: PATH longer than the old 4 KB copy ───────────────── */ - -/* Snapshot/restore one environment variable. The suite swaps PATH and the - * Windows profile roots, and a leaked swap would poison every later suite. */ -static char *repro221_save_env(const char *name) { - const char *value = getenv(name); - return value ? strdup(value) : NULL; -} - -static void repro221_restore_env(const char *name, char *saved) { - if (saved) { - cbm_setenv(name, saved, 1); - free(saved); - } else { - cbm_unsetenv(name); - } -} - -/* - * repro_issue221_long_path_beyond_four_kb - * - * find_in_path() used to copy PATH into a fixed 4096-byte stack buffer through - * cbm_safe_getenv(). That helper does not truncate -- it REFUSES a value it - * cannot fit -- so on a machine whose PATH exceeds 4096 characters the whole - * PATH was skipped and every agent on it read as "not installed". That is the - * layout the #221 reporter had: mise, npm and scoop each append entries. - * - * The fixture directory is the LAST PATH entry, so any truncation of the copy - * hides exactly the directory under test and nothing else. This case is not - * Windows-specific -- the same 4 KB cap hid entries on macOS and Linux. - * - * RED before the fix (empty result), GREEN after it. - */ -TEST(repro_issue221_long_path_beyond_four_kb) { - const char *created = th_mktempdir("repro221long"); - if (!created) { - FAIL("th_mktempdir failed"); - } - char tmpdir[512]; - snprintf(tmpdir, sizeof(tmpdir), "%s", created); - -#ifdef _WIN32 - const char *fixture_name = "cbm-repro221-long.cmd"; - const char *fixture_body = "@echo off\r\nrem fake long-PATH agent\r\n"; - const char path_delim = ';'; -#else - const char *fixture_name = "cbm-repro221-long"; - const char *fixture_body = "#!/bin/sh\n# fake long-PATH agent\n"; - const char path_delim = ':'; -#endif - - char fixture_path[1024]; - snprintf(fixture_path, sizeof(fixture_path), "%s/%s", tmpdir, fixture_name); - if (th_write_file(fixture_path, fixture_body) != 0) { - th_cleanup(tmpdir); - FAIL("could not write the long-PATH fixture"); - } - th_make_executable(fixture_path); - - /* - * Filler entries name directories that do not exist: the scan has to walk - * past every one of them to reach the fixture, which is appended after the - * filler so that it always sits beyond the 4096-byte mark. - */ - enum { FILLER_TARGET = 4600, PATH_CAP = FILLER_TARGET + 1024 }; - char long_path[PATH_CAP]; - size_t used = 0U; - while (used < (size_t)FILLER_TARGET) { - int written = snprintf(long_path + used, sizeof(long_path) - used, - "/nonexistent-repro221-filler%05zu%c", used, path_delim); - if (written <= 0 || (size_t)written >= sizeof(long_path) - used) { - th_cleanup(tmpdir); - FAIL("could not build the long PATH filler"); - } - used += (size_t)written; - } - ASSERT_GT(used, 4096); - int tail = snprintf(long_path + used, sizeof(long_path) - used, "%s", tmpdir); - if (tail <= 0 || (size_t)tail >= sizeof(long_path) - used) { - th_cleanup(tmpdir); - FAIL("could not append the fixture directory to the long PATH"); - } - - char *saved_path = repro221_save_env("PATH"); - cbm_setenv("PATH", long_path, 1); - - /* A home that does not exist keeps the PATH-independent fallbacks out of - * the way: the fixture's directory is the only place that can satisfy it. */ - const char *result = cbm_find_cli("cbm-repro221-long", "/nonexistent-repro221-home"); - - repro221_restore_env("PATH", saved_path); - - ASSERT_NOT_NULL(result); - ASSERT(result[0] != '\0'); - ASSERT(strncmp(result, tmpdir, strlen(tmpdir)) == 0); - ASSERT(strstr(result, "cbm-repro221-long") != NULL); - - th_cleanup(tmpdir); - PASS(); -} - -/* ── #221 regression: the Windows fallback shim locations ───────────────── */ - -/* - * repro_issue221_windows_fallback_shims - * - * An agent the PATH scan cannot see must still be found where Windows actually - * puts CLIs: %APPDATA%\npm (npm's global bin), %LOCALAPPDATA%\mise\shims, - * %USERPROFILE%\scoop\shims, plus the pipx and cargo homes (~\.local\bin, - * ~\.cargo\bin) that were already probed before #221 passed and must not be - * dropped by the fix. - * - * Each location is reached through its own seam -- APPDATA / LOCALAPPDATA, or - * the home_dir argument -- so this never reads the developer's real profile. - * Every shim carries a .cmd extension, which is why the fallback probe needs - * the same PATHEXT treatment the PATH scan got. - * - * Windows-only: the directory layout it asserts is the one Windows installers - * produce. */ -TEST(repro_issue221_windows_fallback_shims) { -#ifndef _WIN32 - SKIP_PLATFORM("Windows-only: npm/mise/scoop shim fallback layout"); -#else - const char *created = th_mktempdir("repro221fb"); - if (!created) { - FAIL("th_mktempdir failed"); - } - char root[512]; - snprintf(root, sizeof(root), "%s", created); - - char *saved_appdata = repro221_save_env("APPDATA"); - char *saved_localappdata = repro221_save_env("LOCALAPPDATA"); - char *saved_userprofile = repro221_save_env("USERPROFILE"); - char *saved_path = repro221_save_env("PATH"); - - /* APPDATA/LOCALAPPDATA/USERPROFILE all point at the fixture root. The - * scoop, pipx and cargo entries are built from the home_dir ARGUMENT, so - * USERPROFILE here mirrors what a real profile would supply. */ - cbm_setenv("APPDATA", root, 1); - cbm_setenv("LOCALAPPDATA", root, 1); - cbm_setenv("USERPROFILE", root, 1); - cbm_setenv("PATH", "/nonexistent-repro221-path", 1); - - /* One shim at a time, so the returned path proves which entry matched - * rather than merely that something did. Each case removes its shim before - * the next one runs, so the earlier entries cannot shadow the later ones. */ - const struct { - const char *subdir; - const char *label; - } cases[] = { - {".local/bin", "pipx home"}, {".cargo/bin", "cargo home"}, {"npm", "APPDATA npm"}, - {"mise/shims", "LOCALAPPDATA mise"}, {"scoop/shims", "scoop home"}, - }; - const char *shim_name = "cbm-repro221-fb.cmd"; - const size_t case_count = sizeof(cases) / sizeof(cases[0]); - - for (size_t i = 0; i < case_count; i++) { - char dir[512]; - snprintf(dir, sizeof(dir), "%s/%s", root, cases[i].subdir); - if (th_mkdir_p(dir) != 0) { - repro221_restore_env("PATH", saved_path); - repro221_restore_env("USERPROFILE", saved_userprofile); - repro221_restore_env("LOCALAPPDATA", saved_localappdata); - repro221_restore_env("APPDATA", saved_appdata); - th_cleanup(root); - FAIL("could not create the fallback fixture directory"); - } - char shim[1024]; - snprintf(shim, sizeof(shim), "%s/%s", dir, shim_name); - if (th_write_file(shim, "@echo off\r\nrem fake fallback shim\r\n") != 0) { - repro221_restore_env("PATH", saved_path); - repro221_restore_env("USERPROFILE", saved_userprofile); - repro221_restore_env("LOCALAPPDATA", saved_localappdata); - repro221_restore_env("APPDATA", saved_appdata); - th_cleanup(root); - FAIL("could not write the fallback shim"); - } - - const char *result = cbm_find_cli("cbm-repro221-fb", root); - char expected_prefix[512]; - snprintf(expected_prefix, sizeof(expected_prefix), "%s/%s/", root, cases[i].subdir); - bool matched = result != NULL && result[0] != '\0' && - strncmp(result, expected_prefix, strlen(expected_prefix)) == 0; - (void)remove(shim); - - if (!matched) { - printf(" fallback case %zu (%s) did not resolve in %s\n", i, cases[i].label, dir); - repro221_restore_env("PATH", saved_path); - repro221_restore_env("USERPROFILE", saved_userprofile); - repro221_restore_env("LOCALAPPDATA", saved_localappdata); - repro221_restore_env("APPDATA", saved_appdata); - th_cleanup(root); - FAIL("a Windows fallback shim location was not probed"); - } - } - - repro221_restore_env("PATH", saved_path); - repro221_restore_env("USERPROFILE", saved_userprofile); - repro221_restore_env("LOCALAPPDATA", saved_localappdata); - repro221_restore_env("APPDATA", saved_appdata); - - th_cleanup(root); - PASS(); -#endif -} - /* ── Suite ──────────────────────────────────────────────────────────────── */ SUITE(repro_issue221) { RUN_TEST(repro_issue221_opencode_pathext_lookup); - RUN_TEST(repro_issue221_long_path_beyond_four_kb); - RUN_TEST(repro_issue221_windows_fallback_shims); } diff --git a/tests/test_cli.c b/tests/test_cli.c index 539b0df18..c3d02c45c 100644 --- a/tests/test_cli.c +++ b/tests/test_cli.c @@ -4245,6 +4245,212 @@ TEST(cli_find_cli_fallback_paths) { PASS(); } +/* ── #221 regression: PATH longer than the old 4 KB copy ───────────────── */ + +/* Snapshot/restore one environment variable. The suite swaps PATH and the + * Windows profile roots, and a leaked swap would poison every later suite. */ +static char *cli_issue221_save_env(const char *name) { + const char *value = getenv(name); + return value ? strdup(value) : NULL; +} + +static void cli_issue221_restore_env(const char *name, char *saved) { + if (saved) { + cbm_setenv(name, saved, 1); + free(saved); + } else { + cbm_unsetenv(name); + } +} + +/* + * cli_find_cli_long_path_beyond_four_kb_issue221 + * + * find_in_path() used to copy PATH into a fixed 4096-byte stack buffer through + * cbm_safe_getenv(). That helper does not truncate -- it REFUSES a value it + * cannot fit -- so on a machine whose PATH exceeds 4096 characters the whole + * PATH was skipped and every agent on it read as "not installed". That is the + * layout the #221 reporter had: mise, npm and scoop each append entries. + * + * The fixture directory is the LAST PATH entry, so any truncation of the copy + * hides exactly the directory under test and nothing else. This case is not + * Windows-specific -- the same 4 KB cap hid entries on macOS and Linux. + * + * RED before the fix (empty result), GREEN after it. + */ +TEST(cli_find_cli_long_path_beyond_four_kb_issue221) { + const char *created = th_mktempdir("repro221long"); + if (!created) { + FAIL("th_mktempdir failed"); + } + char tmpdir[512]; + snprintf(tmpdir, sizeof(tmpdir), "%s", created); + +#ifdef _WIN32 + const char *fixture_name = "cbm-repro221-long.cmd"; + const char *fixture_body = "@echo off\r\nrem fake long-PATH agent\r\n"; + const char path_delim = ';'; +#else + const char *fixture_name = "cbm-repro221-long"; + const char *fixture_body = "#!/bin/sh\n# fake long-PATH agent\n"; + const char path_delim = ':'; +#endif + + char fixture_path[1024]; + snprintf(fixture_path, sizeof(fixture_path), "%s/%s", tmpdir, fixture_name); + if (th_write_file(fixture_path, fixture_body) != 0) { + th_cleanup(tmpdir); + FAIL("could not write the long-PATH fixture"); + } + th_make_executable(fixture_path); + + /* + * Filler entries name directories that do not exist: the scan has to walk + * past every one of them to reach the fixture, which is appended after the + * filler so that it always sits beyond the 4096-byte mark. + */ + enum { FILLER_TARGET = 4600, PATH_CAP = FILLER_TARGET + 1024 }; + char long_path[PATH_CAP]; + size_t used = 0U; + while (used < (size_t)FILLER_TARGET) { + int written = snprintf(long_path + used, sizeof(long_path) - used, + "/nonexistent-repro221-filler%05zu%c", used, path_delim); + if (written <= 0 || (size_t)written >= sizeof(long_path) - used) { + th_cleanup(tmpdir); + FAIL("could not build the long PATH filler"); + } + used += (size_t)written; + } + ASSERT_GT(used, 4096); + int tail = snprintf(long_path + used, sizeof(long_path) - used, "%s", tmpdir); + if (tail <= 0 || (size_t)tail >= sizeof(long_path) - used) { + th_cleanup(tmpdir); + FAIL("could not append the fixture directory to the long PATH"); + } + + char *saved_path = cli_issue221_save_env("PATH"); + cbm_setenv("PATH", long_path, 1); + + /* A home that does not exist keeps the PATH-independent fallbacks out of + * the way: the fixture's directory is the only place that can satisfy it. */ + const char *result = cbm_find_cli("cbm-repro221-long", "/nonexistent-repro221-home"); + + cli_issue221_restore_env("PATH", saved_path); + + ASSERT_NOT_NULL(result); + ASSERT(result[0] != '\0'); + ASSERT(strncmp(result, tmpdir, strlen(tmpdir)) == 0); + ASSERT(strstr(result, "cbm-repro221-long") != NULL); + + th_cleanup(tmpdir); + PASS(); +} + +/* ── #221 regression: the Windows fallback shim locations ───────────────── */ + +/* + * cli_find_cli_windows_fallback_shims_issue221 + * + * An agent the PATH scan cannot see must still be found where Windows actually + * puts CLIs: %APPDATA%\npm (npm's global bin), %LOCALAPPDATA%\mise\shims, + * %USERPROFILE%\scoop\shims, plus the pipx and cargo homes (~\.local\bin, + * ~\.cargo\bin) that were already probed before #221 passed and must not be + * dropped by the fix. + * + * Each location is reached through its own seam -- APPDATA / LOCALAPPDATA, or + * the home_dir argument -- so this never reads the developer's real profile. + * Every shim carries a .cmd extension, which is why the fallback probe needs + * the same PATHEXT treatment the PATH scan got. + * + * Windows-only: the directory layout it asserts is the one Windows installers + * produce. */ +TEST(cli_find_cli_windows_fallback_shims_issue221) { +#ifndef _WIN32 + SKIP_PLATFORM("Windows-only: npm/mise/scoop shim fallback layout"); +#else + const char *created = th_mktempdir("repro221fb"); + if (!created) { + FAIL("th_mktempdir failed"); + } + char root[512]; + snprintf(root, sizeof(root), "%s", created); + + char *saved_appdata = cli_issue221_save_env("APPDATA"); + char *saved_localappdata = cli_issue221_save_env("LOCALAPPDATA"); + char *saved_userprofile = cli_issue221_save_env("USERPROFILE"); + char *saved_path = cli_issue221_save_env("PATH"); + + /* APPDATA/LOCALAPPDATA/USERPROFILE all point at the fixture root. The + * scoop, pipx and cargo entries are built from the home_dir ARGUMENT, so + * USERPROFILE here mirrors what a real profile would supply. */ + cbm_setenv("APPDATA", root, 1); + cbm_setenv("LOCALAPPDATA", root, 1); + cbm_setenv("USERPROFILE", root, 1); + cbm_setenv("PATH", "/nonexistent-repro221-path", 1); + + /* One shim at a time, so the returned path proves which entry matched + * rather than merely that something did. Each case removes its shim before + * the next one runs, so the earlier entries cannot shadow the later ones. */ + const struct { + const char *subdir; + const char *label; + } cases[] = { + {".local/bin", "pipx home"}, {".cargo/bin", "cargo home"}, {"npm", "APPDATA npm"}, + {"mise/shims", "LOCALAPPDATA mise"}, {"scoop/shims", "scoop home"}, + }; + const char *shim_name = "cbm-repro221-fb.cmd"; + const size_t case_count = sizeof(cases) / sizeof(cases[0]); + + for (size_t i = 0; i < case_count; i++) { + char dir[512]; + snprintf(dir, sizeof(dir), "%s/%s", root, cases[i].subdir); + if (th_mkdir_p(dir) != 0) { + cli_issue221_restore_env("PATH", saved_path); + cli_issue221_restore_env("USERPROFILE", saved_userprofile); + cli_issue221_restore_env("LOCALAPPDATA", saved_localappdata); + cli_issue221_restore_env("APPDATA", saved_appdata); + th_cleanup(root); + FAIL("could not create the fallback fixture directory"); + } + char shim[1024]; + snprintf(shim, sizeof(shim), "%s/%s", dir, shim_name); + if (th_write_file(shim, "@echo off\r\nrem fake fallback shim\r\n") != 0) { + cli_issue221_restore_env("PATH", saved_path); + cli_issue221_restore_env("USERPROFILE", saved_userprofile); + cli_issue221_restore_env("LOCALAPPDATA", saved_localappdata); + cli_issue221_restore_env("APPDATA", saved_appdata); + th_cleanup(root); + FAIL("could not write the fallback shim"); + } + + const char *result = cbm_find_cli("cbm-repro221-fb", root); + char expected_prefix[512]; + snprintf(expected_prefix, sizeof(expected_prefix), "%s/%s/", root, cases[i].subdir); + bool matched = result != NULL && result[0] != '\0' && + strncmp(result, expected_prefix, strlen(expected_prefix)) == 0; + (void)remove(shim); + + if (!matched) { + printf(" fallback case %zu (%s) did not resolve in %s\n", i, cases[i].label, dir); + cli_issue221_restore_env("PATH", saved_path); + cli_issue221_restore_env("USERPROFILE", saved_userprofile); + cli_issue221_restore_env("LOCALAPPDATA", saved_localappdata); + cli_issue221_restore_env("APPDATA", saved_appdata); + th_cleanup(root); + FAIL("a Windows fallback shim location was not probed"); + } + } + + cli_issue221_restore_env("PATH", saved_path); + cli_issue221_restore_env("USERPROFILE", saved_userprofile); + cli_issue221_restore_env("LOCALAPPDATA", saved_localappdata); + cli_issue221_restore_env("APPDATA", saved_appdata); + + th_cleanup(root); + PASS(); +#endif +} + /* ═══════════════════════════════════════════════════════════════════ * Dry-run flag parsing (port of TestDryRun) * ═══════════════════════════════════════════════════════════════════ */ @@ -18307,6 +18513,8 @@ SUITE(cli) { RUN_TEST(cli_find_cli_result_is_thread_local); RUN_TEST(cli_find_cli_on_path); RUN_TEST(cli_find_cli_fallback_paths); + RUN_TEST(cli_find_cli_long_path_beyond_four_kb_issue221); + RUN_TEST(cli_find_cli_windows_fallback_shims_issue221); /* Dry-run flag parsing (1 test — install_test.go) */ RUN_TEST(cli_dry_run_flags);