From f75ef88afd82856c675e006087652c0c2394dd0b Mon Sep 17 00:00:00 2001 From: Yiftach Cohen Date: Mon, 8 Jun 2026 16:15:33 +0200 Subject: [PATCH 1/4] [PPSC-927] fix(supply-chain): make init shell wrappers survive armis-cli upgrades resolveCliPath embedded the fully symlink-resolved binary path, which on Homebrew is the version-pinned Cellar dir (e.g. .../Cellar/armis-cli/1.11.0/...). After `brew upgrade armis-cli` deleted that dir, every wrapped package manager (npm, pnpm, bun, pip, uv, poetry, npx) failed to run in new shells. - resolveCliPath now prefers the bare name `armis-cli` (re-resolved from PATH on every call) when on PATH, else the stable symlink path from filepath.Abs(os.Executable()) without EvalSymlinks. - Generated wrappers are now fail-closed: if armis-cli cannot be found at invocation time, the wrapper warns loudly on stderr and runs the real package manager un-wrapped so installs never silently break. - Add cliBinaryName const; add tests for the guard and resolveCliPath. Forward-only: existing users must re-run `armis-cli supply-chain init` once after upgrading to a build with this fix. --- docs/CHANGELOG.md | 2 + internal/supplychain/shell.go | 68 +++++++++++++++++++++---- internal/supplychain/shell_test.go | 82 ++++++++++++++++++++++++++++++ 3 files changed, 142 insertions(+), 10 deletions(-) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 21f733f3..4d22cad4 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -17,6 +17,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- `supply-chain init`: the injected shell wrappers no longer break after an `armis-cli` upgrade. The wrapper now references `armis-cli` by bare name (resolved from `PATH` on every call) when it is on `PATH`, falling back to the stable symlink path otherwise — previously it embedded the fully symlink-resolved binary path, which on Homebrew was the version-pinned Cellar directory (e.g. `…/Cellar/armis-cli/1.11.0/…`). After `brew upgrade armis-cli` deleted that directory, every wrapped package manager (npm, pnpm, bun, pip, uv, poetry, npx) failed to run in new shells. The wrappers are also now fail-closed: if `armis-cli` cannot be found at invocation time, the wrapper prints a loud warning to stderr that enforcement has lapsed and runs the real package manager un-wrapped, so installs never silently break. Wrappers injected before this fix must be refreshed by re-running `armis-cli supply-chain init` once. + ### Security --- diff --git a/internal/supplychain/shell.go b/internal/supplychain/shell.go index cd0b7f42..02035006 100644 --- a/internal/supplychain/shell.go +++ b/internal/supplychain/shell.go @@ -18,6 +18,12 @@ const ( markerEnd = "# <<< armis-cli supply-chain <<<" ) +// cliBinaryName is the armis-cli executable name. resolveCliPath embeds it in +// the generated wrappers (preferring this bare, PATH-resolved name so upgrades +// stay transparent) and uses it to probe $PATH; centralizing the literal keeps +// the two in sync. +const cliBinaryName = "armis-cli" + // goosWindows is runtime.GOOS on Windows. Centralized so the several // platform guards across this package (and its tests) share one literal. const goosWindows = "windows" @@ -114,8 +120,21 @@ func generatePosixWrapper(pms []string, cli string) string { var b strings.Builder b.WriteString(markerStart + "\n") for _, pm := range sanitizePMNames(pms) { - // armis:ignore cwe:78 reason:pm is constrained to ^[a-z][a-z0-9-]*(\.[0-9]+)?$ by sanitizePMNames, so any dot is followed only by digits (not a shell metacharacter); safeCli is shellQuote-escaped - fmt.Fprintf(&b, "%s() {\n command %s supply-chain wrap %s \"$@\"\n}\n", pm, safeCli, pm) + // The `command -v` guard makes the wrapper fail-closed: if armis-cli is + // not resolvable at invocation time (e.g. a stale absolute path left by a + // package-manager upgrade), the wrapper warns loudly on stderr and runs the + // real package manager un-wrapped rather than failing the command outright. + // armis:ignore cwe:78 reason:pm is constrained to ^[a-z][a-z0-9-]*(\.[0-9]+)?$ by sanitizePMNames, so any dot is followed only by digits (not a shell metacharacter); safeCli is shellQuote-escaped; command -v is used only for presence detection and its output is discarded + fmt.Fprintf(&b, + "%s() {\n"+ + " if command -v %s >/dev/null 2>&1; then\n"+ + " command %s supply-chain wrap %s \"$@\"\n"+ + " else\n"+ + " printf '[armis] armis-cli not found - running %s WITHOUT supply-chain enforcement\\n' >&2\n"+ + " command %s \"$@\"\n"+ + " fi\n"+ + "}\n", + pm, safeCli, safeCli, pm, pm, pm) } b.WriteString(markerEnd + "\n") return b.String() @@ -128,8 +147,21 @@ func generateFishWrapper(pms []string, cli string) string { var b strings.Builder b.WriteString(markerStart + "\n") for _, pm := range sanitizePMNames(pms) { - // armis:ignore cwe:78 reason:pm is constrained to ^[a-z][a-z0-9-]*(\.[0-9]+)?$ by sanitizePMNames, so any dot is followed only by digits (not a shell metacharacter); safeCli is shellQuote-escaped - fmt.Fprintf(&b, "function %s\n command %s supply-chain wrap %s $argv\nend\n", pm, safeCli, pm) + // The `command -v` guard makes the wrapper fail-closed: if armis-cli is + // not resolvable at invocation time (e.g. a stale absolute path left by a + // package-manager upgrade), the wrapper warns loudly on stderr and runs the + // real package manager un-wrapped rather than failing the command outright. + // armis:ignore cwe:78 reason:pm is constrained to ^[a-z][a-z0-9-]*(\.[0-9]+)?$ by sanitizePMNames, so any dot is followed only by digits (not a shell metacharacter); safeCli is shellQuote-escaped; command -v is used only for presence detection and its output is discarded + fmt.Fprintf(&b, + "function %s\n"+ + " if command -v %s >/dev/null 2>&1\n"+ + " command %s supply-chain wrap %s $argv\n"+ + " else\n"+ + " printf '[armis] armis-cli not found - running %s WITHOUT supply-chain enforcement\\n' >&2\n"+ + " command %s $argv\n"+ + " end\n"+ + "end\n", + pm, safeCli, safeCli, pm, pm, pm) } b.WriteString(markerEnd + "\n") return b.String() @@ -139,20 +171,36 @@ func shellQuote(s string) string { return "'" + strings.ReplaceAll(s, "'", "'\\''") + "'" } +// resolveCliPath returns the most upgrade-proof reference to the armis-cli +// binary to embed in the generated shell wrapper functions. +// +// Priority order: +// 1. The bare name "armis-cli" when it is resolvable on $PATH. The shell +// re-resolves the name on every invocation, so a package-manager upgrade +// (e.g. `brew upgrade armis-cli`, which moves the binary into a new +// versioned directory) is transparent — the wrapper never embeds a +// version-pinned path that a later upgrade would delete. +// 2. Otherwise the stable absolute path from filepath.Abs(os.Executable()). +// We deliberately do NOT call filepath.EvalSymlinks here: on a Homebrew +// install os.Executable() reports the stable /opt/homebrew/bin/armis-cli +// symlink, and resolving it would pin the wrapper to the versioned Cellar +// path (…/Cellar/armis-cli//bin/armis-cli) that `brew upgrade` +// removes, breaking every wrapped package manager. +// 3. The literal "armis-cli" if os.Executable() fails — better a PATH-resolved +// name than nothing. func resolveCliPath() string { + if IsOnPath(cliBinaryName) { + return cliBinaryName + } exe, err := os.Executable() if err != nil { - return "armis-cli" + return cliBinaryName } abs, err := filepath.Abs(exe) if err != nil { return exe } - resolved, err := filepath.EvalSymlinks(abs) - if err != nil { - return abs - } - return resolved + return abs } func InjectFunctions(shells []Shell, pms []string) ([]string, error) { diff --git a/internal/supplychain/shell_test.go b/internal/supplychain/shell_test.go index 32e7763d..cec178b7 100644 --- a/internal/supplychain/shell_test.go +++ b/internal/supplychain/shell_test.go @@ -493,3 +493,85 @@ func TestDetectShells(t *testing.T) { } }) } + +// TestGenerateWrapper_PosixContainsGuard verifies the bash/zsh wrapper is +// fail-closed: it guards the armis-cli invocation with `command -v`, warns on +// stderr when the binary is missing, and falls back to running the real package +// manager so an install never silently breaks after an armis-cli upgrade. +func TestGenerateWrapper_PosixContainsGuard(t *testing.T) { + wrapper := GenerateWrapper("bash", []string{"npm"}) + + if !strings.Contains(wrapper, "if command -v") { + t.Errorf("posix wrapper missing `command -v` guard:\n%s", wrapper) + } + if !strings.Contains(wrapper, "armis-cli not found") { + t.Errorf("posix wrapper missing the missing-binary warning:\n%s", wrapper) + } + if !strings.Contains(wrapper, `command npm "$@"`) { + t.Errorf("posix wrapper missing the un-wrapped fallback `command npm \"$@\"`:\n%s", wrapper) + } +} + +// TestGenerateWrapper_FishContainsGuard is the fish-syntax counterpart to +// TestGenerateWrapper_PosixContainsGuard. +func TestGenerateWrapper_FishContainsGuard(t *testing.T) { + wrapper := GenerateWrapper("fish", []string{"npm"}) + + if !strings.Contains(wrapper, "if command -v") { + t.Errorf("fish wrapper missing `command -v` guard:\n%s", wrapper) + } + if !strings.Contains(wrapper, "armis-cli not found") { + t.Errorf("fish wrapper missing the missing-binary warning:\n%s", wrapper) + } + if !strings.Contains(wrapper, "command npm $argv") { + t.Errorf("fish wrapper missing the un-wrapped fallback `command npm $argv`:\n%s", wrapper) + } +} + +// TestGenerateWrapper_WarningReferencesPMName verifies the fallback warning names +// the package manager the user actually invoked (not a hard-coded "npm"), for +// every shell and including versioned pip variants. +func TestGenerateWrapper_WarningReferencesPMName(t *testing.T) { + for _, shell := range []string{"bash", "zsh", "fish"} { + for _, pm := range []string{"npm", "pip3.12", "poetry"} { + wrapper := GenerateWrapper(shell, []string{pm}) + want := "running " + pm + " WITHOUT supply-chain enforcement" + if !strings.Contains(wrapper, want) { + t.Errorf("%s wrapper for %q missing PM name in warning %q:\n%s", shell, pm, want, wrapper) + } + } + } +} + +// TestResolveCliPath_PrefersBareNameWhenOnPath verifies that when armis-cli is +// resolvable on $PATH, the wrapper embeds the bare name so the shell re-resolves +// it on every call — surviving package-manager upgrades that move the binary. +func TestResolveCliPath_PrefersBareNameWhenOnPath(t *testing.T) { + dir := t.TempDir() + fname := cliBinaryName + if runtime.GOOS == goosWindows { + fname = cliBinaryName + ".exe" + } + if err := os.WriteFile(filepath.Join(dir, fname), []byte{}, 0o755); err != nil { //nolint:gosec + t.Fatalf("seed %s: %v", cliBinaryName, err) + } + t.Setenv("PATH", dir) + + if got := resolveCliPath(); got != cliBinaryName { + t.Errorf("resolveCliPath() = %q, want %q (bare name when on PATH)", got, cliBinaryName) + } +} + +// TestResolveCliPath_FallsBackToAbsWhenNotOnPath verifies that when armis-cli is +// not on $PATH, resolveCliPath returns an absolute path (or the bare-name +// fallback) — but never resolves symlinks, so it cannot return a version-pinned +// Cellar-style path that an upgrade would delete. +func TestResolveCliPath_FallsBackToAbsWhenNotOnPath(t *testing.T) { + // Non-empty but empty dir: armis-cli is not resolvable on PATH. + t.Setenv("PATH", t.TempDir()) + + got := resolveCliPath() + if got != cliBinaryName && !filepath.IsAbs(got) { + t.Errorf("resolveCliPath() = %q, want an absolute path or the %q fallback", got, cliBinaryName) + } +} From d52b6596a2cdb47657d2c627181817d021a19e94 Mon Sep 17 00:00:00 2001 From: Yiftach Cohen Date: Mon, 8 Jun 2026 16:22:53 +0200 Subject: [PATCH 2/4] [PPSC-927] fix(supply-chain): suppress CWE-426 false positive on resolveCliPath The code-scanning check flags resolveCliPath for untrusted search path because it returns the bare name "armis-cli" (PATH-resolved by the shell). This is a false positive: cliBinaryName is a hardcoded constant (not user input), the generated wrapper already resolves the package manager itself by bare name (`command npm`), and supply-chain init configures the current user's own shell, whose $PATH is not a trust boundary for a local CLI. Add an armis:ignore matching the existing cwe:426 cwe:427 annotations on IsOnPath and DetectInstalledPMs in this file. --- internal/supplychain/shell.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/supplychain/shell.go b/internal/supplychain/shell.go index 02035006..342da0c4 100644 --- a/internal/supplychain/shell.go +++ b/internal/supplychain/shell.go @@ -189,6 +189,7 @@ func shellQuote(s string) string { // 3. The literal "armis-cli" if os.Executable() fails — better a PATH-resolved // name than nothing. func resolveCliPath() string { + // armis:ignore cwe:426 cwe:427 reason:cliBinaryName is the hardcoded literal "armis-cli", not user input; supply-chain init configures the current user's own interactive shell, whose $PATH is not a trust boundary for a local CLI. The generated wrapper already resolves the package manager itself by bare name (e.g. `command npm`), so embedding the bare armis-cli name adds no search-path exposure the shell does not already have — an attacker able to write to a $PATH dir could shadow npm/pip directly. if IsOnPath(cliBinaryName) { return cliBinaryName } From 05acf317b967886e3d5c40c1416e549899a57d9e Mon Sep 17 00:00:00 2001 From: Yiftach Cohen Date: Mon, 8 Jun 2026 16:49:05 +0200 Subject: [PATCH 3/4] [PPSC-927] fix(supply-chain): use fish-native `command -q` in fish wrapper guard fish's `command` builtin does not accept POSIX `-v`; `command -v` would error and make the guard always take the else branch, silently disabling enforcement for every fish user even when armis-cli is present. Switch the fish guard to `command -q` (fish 3.0+), which is the native presence check and emits no output (so the `>/dev/null 2>&1` redirect is no longer needed). bash/zsh keep `command -v`, which is correct POSIX. Update the fish test to assert `-q`. Addresses PR #216 review comments from copilot-pull-request-reviewer. --- internal/supplychain/shell.go | 8 +++++--- internal/supplychain/shell_test.go | 8 +++++--- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/internal/supplychain/shell.go b/internal/supplychain/shell.go index 342da0c4..975e9540 100644 --- a/internal/supplychain/shell.go +++ b/internal/supplychain/shell.go @@ -147,14 +147,16 @@ func generateFishWrapper(pms []string, cli string) string { var b strings.Builder b.WriteString(markerStart + "\n") for _, pm := range sanitizePMNames(pms) { - // The `command -v` guard makes the wrapper fail-closed: if armis-cli is + // The `command -q` guard makes the wrapper fail-closed: if armis-cli is // not resolvable at invocation time (e.g. a stale absolute path left by a // package-manager upgrade), the wrapper warns loudly on stderr and runs the // real package manager un-wrapped rather than failing the command outright. - // armis:ignore cwe:78 reason:pm is constrained to ^[a-z][a-z0-9-]*(\.[0-9]+)?$ by sanitizePMNames, so any dot is followed only by digits (not a shell metacharacter); safeCli is shellQuote-escaped; command -v is used only for presence detection and its output is discarded + // fish's `command` builtin has no POSIX `-v`; `-q`/`--query` (fish 3.0+) is + // the native presence check and emits no output, so no redirect is needed. + // armis:ignore cwe:78 reason:pm is constrained to ^[a-z][a-z0-9-]*(\.[0-9]+)?$ by sanitizePMNames, so any dot is followed only by digits (not a shell metacharacter); safeCli is shellQuote-escaped; command -q is used only for presence detection and its output is discarded fmt.Fprintf(&b, "function %s\n"+ - " if command -v %s >/dev/null 2>&1\n"+ + " if command -q %s\n"+ " command %s supply-chain wrap %s $argv\n"+ " else\n"+ " printf '[armis] armis-cli not found - running %s WITHOUT supply-chain enforcement\\n' >&2\n"+ diff --git a/internal/supplychain/shell_test.go b/internal/supplychain/shell_test.go index cec178b7..324d7243 100644 --- a/internal/supplychain/shell_test.go +++ b/internal/supplychain/shell_test.go @@ -513,12 +513,14 @@ func TestGenerateWrapper_PosixContainsGuard(t *testing.T) { } // TestGenerateWrapper_FishContainsGuard is the fish-syntax counterpart to -// TestGenerateWrapper_PosixContainsGuard. +// TestGenerateWrapper_PosixContainsGuard. The guard uses fish-native `command -q` +// (not POSIX `command -v`, which fish's `command` builtin does not accept — using +// it would make the guard always fail and silently disable enforcement for fish). func TestGenerateWrapper_FishContainsGuard(t *testing.T) { wrapper := GenerateWrapper("fish", []string{"npm"}) - if !strings.Contains(wrapper, "if command -v") { - t.Errorf("fish wrapper missing `command -v` guard:\n%s", wrapper) + if !strings.Contains(wrapper, "if command -q") { + t.Errorf("fish wrapper missing `command -q` guard:\n%s", wrapper) } if !strings.Contains(wrapper, "armis-cli not found") { t.Errorf("fish wrapper missing the missing-binary warning:\n%s", wrapper) From f1aaa65ea002ea932aaaa02af6b51a8beb1f4b68 Mon Sep 17 00:00:00 2001 From: Yiftach Cohen Date: Mon, 8 Jun 2026 17:53:51 +0200 Subject: [PATCH 4/4] [PPSC-927] fix(supply-chain): add executable-path check to wrapper guard When resolveCliPath() falls back to an absolute path (armis-cli not on PATH at init time), the wrapper guard's bare `command -v`/`command -q` check is given a slash-containing argument. That is unspecified across shells and can report the binary missing even when it exists, making the wrapper silently take the `else` branch and bypass enforcement. Prepend an executable-path check that reliably handles the absolute-path case while keeping the PATH lookup for the bare-name case: POSIX: `if [ -x ] || command -v ...` fish: `if test -x ; or command -q ` Tests assert the `command -v`/`command -q` substring rather than the exact `if` prefix so the guard can be extended without breaking them, and a new test locks in the executable-path check for the absolute-path case. Addresses copilot-pull-request-reviewer feedback on PR #216. --- internal/supplychain/shell.go | 45 ++++++++++++++++++---------- internal/supplychain/shell_test.go | 48 ++++++++++++++++++++++++++---- 2 files changed, 72 insertions(+), 21 deletions(-) diff --git a/internal/supplychain/shell.go b/internal/supplychain/shell.go index 975e9540..8dbe1200 100644 --- a/internal/supplychain/shell.go +++ b/internal/supplychain/shell.go @@ -120,21 +120,28 @@ func generatePosixWrapper(pms []string, cli string) string { var b strings.Builder b.WriteString(markerStart + "\n") for _, pm := range sanitizePMNames(pms) { - // The `command -v` guard makes the wrapper fail-closed: if armis-cli is - // not resolvable at invocation time (e.g. a stale absolute path left by a - // package-manager upgrade), the wrapper warns loudly on stderr and runs the - // real package manager un-wrapped rather than failing the command outright. - // armis:ignore cwe:78 reason:pm is constrained to ^[a-z][a-z0-9-]*(\.[0-9]+)?$ by sanitizePMNames, so any dot is followed only by digits (not a shell metacharacter); safeCli is shellQuote-escaped; command -v is used only for presence detection and its output is discarded + // The guard makes the wrapper fail-closed: if armis-cli is not resolvable + // at invocation time (e.g. a stale absolute path left by a package-manager + // upgrade), the wrapper warns loudly on stderr and runs the real package + // manager un-wrapped rather than failing the command outright. + // + // Two checks, because resolveCliPath() may embed either a bare name or an + // absolute path: `[ -x %s ]` reliably accepts an executable at an absolute + // path (POSIX `command -v` with a slash-containing argument is unspecified + // and some shells report it missing even when it exists), while `command -v` + // handles the bare-name PATH lookup. For a bare name `[ -x ]` checks the CWD + // (normally absent) and falls through to the PATH lookup. + // armis:ignore cwe:78 reason:pm is constrained to ^[a-z][a-z0-9-]*(\.[0-9]+)?$ by sanitizePMNames, so any dot is followed only by digits (not a shell metacharacter); safeCli is shellQuote-escaped; both `[ -x ]` and `command -v` are used only for presence detection and their output is discarded fmt.Fprintf(&b, "%s() {\n"+ - " if command -v %s >/dev/null 2>&1; then\n"+ + " if [ -x %s ] || command -v %s >/dev/null 2>&1; then\n"+ " command %s supply-chain wrap %s \"$@\"\n"+ " else\n"+ " printf '[armis] armis-cli not found - running %s WITHOUT supply-chain enforcement\\n' >&2\n"+ " command %s \"$@\"\n"+ " fi\n"+ "}\n", - pm, safeCli, safeCli, pm, pm, pm) + pm, safeCli, safeCli, safeCli, pm, pm, pm) } b.WriteString(markerEnd + "\n") return b.String() @@ -147,23 +154,29 @@ func generateFishWrapper(pms []string, cli string) string { var b strings.Builder b.WriteString(markerStart + "\n") for _, pm := range sanitizePMNames(pms) { - // The `command -q` guard makes the wrapper fail-closed: if armis-cli is - // not resolvable at invocation time (e.g. a stale absolute path left by a - // package-manager upgrade), the wrapper warns loudly on stderr and runs the - // real package manager un-wrapped rather than failing the command outright. - // fish's `command` builtin has no POSIX `-v`; `-q`/`--query` (fish 3.0+) is - // the native presence check and emits no output, so no redirect is needed. - // armis:ignore cwe:78 reason:pm is constrained to ^[a-z][a-z0-9-]*(\.[0-9]+)?$ by sanitizePMNames, so any dot is followed only by digits (not a shell metacharacter); safeCli is shellQuote-escaped; command -q is used only for presence detection and its output is discarded + // The guard makes the wrapper fail-closed: if armis-cli is not resolvable + // at invocation time (e.g. a stale absolute path left by a package-manager + // upgrade), the wrapper warns loudly on stderr and runs the real package + // manager un-wrapped rather than failing the command outright. + // + // Two checks, because resolveCliPath() may embed either a bare name or an + // absolute path: `test -x %s` reliably accepts an executable at an absolute + // path (fish's `command -q` with a slash-containing argument is unspecified + // across versions and can report it missing even when it exists), while + // `command -q`/`--query` (fish 3.0+) handles the bare-name PATH lookup and + // emits no output. For a bare name `test -x` checks the CWD (normally + // absent) and falls through to the PATH lookup. + // armis:ignore cwe:78 reason:pm is constrained to ^[a-z][a-z0-9-]*(\.[0-9]+)?$ by sanitizePMNames, so any dot is followed only by digits (not a shell metacharacter); safeCli is shellQuote-escaped; both `test -x` and `command -q` are used only for presence detection and their output is discarded fmt.Fprintf(&b, "function %s\n"+ - " if command -q %s\n"+ + " if test -x %s; or command -q %s\n"+ " command %s supply-chain wrap %s $argv\n"+ " else\n"+ " printf '[armis] armis-cli not found - running %s WITHOUT supply-chain enforcement\\n' >&2\n"+ " command %s $argv\n"+ " end\n"+ "end\n", - pm, safeCli, safeCli, pm, pm, pm) + pm, safeCli, safeCli, safeCli, pm, pm, pm) } b.WriteString(markerEnd + "\n") return b.String() diff --git a/internal/supplychain/shell_test.go b/internal/supplychain/shell_test.go index 324d7243..0e662539 100644 --- a/internal/supplychain/shell_test.go +++ b/internal/supplychain/shell_test.go @@ -495,13 +495,18 @@ func TestDetectShells(t *testing.T) { } // TestGenerateWrapper_PosixContainsGuard verifies the bash/zsh wrapper is -// fail-closed: it guards the armis-cli invocation with `command -v`, warns on -// stderr when the binary is missing, and falls back to running the real package -// manager so an install never silently breaks after an armis-cli upgrade. +// fail-closed: it guards the armis-cli invocation with a `command -v` PATH lookup +// (alongside an `[ -x ]` absolute-path check), warns on stderr when the binary is +// missing, and falls back to running the real package manager so an install never +// silently breaks after an armis-cli upgrade. +// +// The assertion matches the `command -v` substring rather than the exact `if` +// prefix so the guard can be extended (e.g. with the `[ -x … ]` path check) without +// breaking this test, as long as the fail-closed PATH lookup remains. func TestGenerateWrapper_PosixContainsGuard(t *testing.T) { wrapper := GenerateWrapper("bash", []string{"npm"}) - if !strings.Contains(wrapper, "if command -v") { + if !strings.Contains(wrapper, "command -v") { t.Errorf("posix wrapper missing `command -v` guard:\n%s", wrapper) } if !strings.Contains(wrapper, "armis-cli not found") { @@ -516,10 +521,14 @@ func TestGenerateWrapper_PosixContainsGuard(t *testing.T) { // TestGenerateWrapper_PosixContainsGuard. The guard uses fish-native `command -q` // (not POSIX `command -v`, which fish's `command` builtin does not accept — using // it would make the guard always fail and silently disable enforcement for fish). +// +// The assertion matches the `command -q` substring rather than the exact `if` +// prefix so the guard can be extended (e.g. with the `test -x …` path check) without +// breaking this test, as long as the fish-native presence check remains. func TestGenerateWrapper_FishContainsGuard(t *testing.T) { wrapper := GenerateWrapper("fish", []string{"npm"}) - if !strings.Contains(wrapper, "if command -q") { + if !strings.Contains(wrapper, "command -q") { t.Errorf("fish wrapper missing `command -q` guard:\n%s", wrapper) } if !strings.Contains(wrapper, "armis-cli not found") { @@ -530,6 +539,35 @@ func TestGenerateWrapper_FishContainsGuard(t *testing.T) { } } +// TestGenerateWrapper_AbsolutePathGuardChecksExecutable verifies that when +// resolveCliPath falls back to an absolute path (armis-cli not on PATH at init +// time), the guard includes an executable-path check on that path. A bare +// `command -v`/`command -q` with a slash-containing argument is unspecified across +// shells and can report the binary missing even when it exists, which would make +// the wrapper always take the `else` branch and silently bypass enforcement. The +// `[ -x … ]` / `test -x …` check makes the absolute-path case reliable. +func TestGenerateWrapper_AbsolutePathGuardChecksExecutable(t *testing.T) { + const absPath = "/opt/homebrew/bin/armis-cli" + quoted := shellQuote(absPath) + + posix := generatePosixWrapper([]string{"npm"}, absPath) + if !strings.Contains(posix, "[ -x "+quoted+" ]") { + t.Errorf("posix wrapper missing `[ -x ]` executable check for an absolute CLI path:\n%s", posix) + } + // The PATH lookup must remain alongside the path check for the bare-name case. + if !strings.Contains(posix, "command -v "+quoted) { + t.Errorf("posix wrapper missing `command -v` PATH lookup alongside the path check:\n%s", posix) + } + + fish := generateFishWrapper([]string{"npm"}, absPath) + if !strings.Contains(fish, "test -x "+quoted) { + t.Errorf("fish wrapper missing `test -x ` executable check for an absolute CLI path:\n%s", fish) + } + if !strings.Contains(fish, "command -q "+quoted) { + t.Errorf("fish wrapper missing `command -q` PATH lookup alongside the path check:\n%s", fish) + } +} + // TestGenerateWrapper_WarningReferencesPMName verifies the fallback warning names // the package manager the user actually invoked (not a hard-coded "npm"), for // every shell and including versioned pip variants.