[PPSC-927] fix(supply-chain): survive armis-cli upgrades in init wrappers

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
 
 ---

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,28 @@ 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 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 [ -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, safeCli, pm, pm, pm)
 	}
 	b.WriteString(markerEnd + "\n")
 	return b.String()
@@ -128,8 +154,29 @@ 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 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 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, safeCli, pm, pm, pm)
 	}
 	b.WriteString(markerEnd + "\n")
 	return b.String()
@@ -139,20 +186,37 @@ 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/<version>/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 {
+	// 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
+	}
 	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) {

internal/supplychain/shell_test.go

@@ -493,3 +493,125 @@ func TestDetectShells(t *testing.T) {
 		}
 	})
 }
+
+// TestGenerateWrapper_PosixContainsGuard verifies the bash/zsh wrapper is
+// 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, "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. 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, "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)
+	}
+	if !strings.Contains(wrapper, "command npm $argv") {
+		t.Errorf("fish wrapper missing the un-wrapped fallback `command npm $argv`:\n%s", wrapper)
+	}
+}
+
+// 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 <abs> ]` 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 <abs>` 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.
+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)
+	}
+}