From 5439387c6239535b19899bca6a3813bb9a11f19a Mon Sep 17 00:00:00 2001
From: Mark Yen <mark.yen@suse.com>
Date: Wed, 25 Sep 2024 16:21:19 -0700
Subject: [PATCH] Diagnostics: bin-in-shell: Fix existing PATH

If Rancher Desktop was launched with the PATH such that `~/.rd/bin/` is
already in the PATH (e.g. because integration was previously set up when we
logged in), we would end up always passing the test even when the
configuration was set to disable it (which would have otherwise been
reflected the next time the user logs in).  Fix this by explicitly cleaning
the PATH before we spawn the shell to do the check.

This also fixes an error where if a shell was not installed we throw an
exception (that gets logged but does not display in the UI) instead of
passing the test.  This also means if the user installs a missing shell
while Rancher Desktop was running, re-running the check will detect that.

Signed-off-by: Mark Yen <mark.yen@suse.com>
---
 .../main/diagnostics/rdBinInShell.ts          | 35 ++++++++++++-------
 1 file changed, 23 insertions(+), 12 deletions(-)

diff --git a/pkg/rancher-desktop/main/diagnostics/rdBinInShell.ts b/pkg/rancher-desktop/main/diagnostics/rdBinInShell.ts
index bcda66da3d6..e3437955bab 100644
--- a/pkg/rancher-desktop/main/diagnostics/rdBinInShell.ts
+++ b/pkg/rancher-desktop/main/diagnostics/rdBinInShell.ts
@@ -1,4 +1,3 @@
-import os from 'os';
 import path from 'path';
 
 import which from 'which';
@@ -22,18 +21,16 @@ mainEvents.on('settings-update', (cfg) => {
 export class RDBinInShellPath implements DiagnosticsChecker {
   constructor(id: string, executable: string, ...args: string[]) {
     this.id = id;
-    if (['darwin', 'linux'].includes(os.platform())) {
-      this.executable = which.sync(executable, { nothrow: true }) ?? '';
-    }
+    this.executable = executable;
     this.args = args.concat(`printf "\n${ pathOutputDelimiter }%s\n" "$PATH"`);
   }
 
   id: string;
-  executable = '';
+  executable: string;
   args: string[];
   category = DiagnosticsCategory.Utilities;
   applicable(): Promise<boolean> {
-    return Promise.resolve(!!this.executable);
+    return Promise.resolve(['darwin', 'linux'].includes(process.platform));
   }
 
   async check(): Promise<DiagnosticsCheckerResult> {
@@ -42,19 +39,33 @@ export class RDBinInShellPath implements DiagnosticsChecker {
     let description: string;
 
     try {
-      const { stdout } = await spawnFile(this.executable, this.args, { stdio: ['ignore', 'pipe', 'pipe'] });
+      const executable = await which(this.executable, { nothrow: true });
+
+      if (!executable) {
+        return {
+          passed:      true, // No need to throw a diagnostic in this case.
+          description: `Failed to find ${ this.executable } executable`,
+          fixes:       [{ description: `Install ${ this.executable }` }],
+        };
+      }
+
+      const integrationPath = RDBinInShellPath.removeTrailingSlash(paths.integration);
+      const currentPaths = process.env.PATH?.split(path.delimiter) ?? ['/usr/local/bin', '/usr/bin', '/bin'];
+      const fixedPath = currentPaths.map(RDBinInShellPath.removeTrailingSlash).filter(p => p !== integrationPath).join(path.delimiter);
+      const { stdout } = await spawnFile(
+        this.executable,
+        this.args,
+        { stdio: ['ignore', 'pipe', 'pipe'], env: { ...process.env, PATH: fixedPath } });
       const dirs = stdout.split('\n')
         .filter(line => line.startsWith(pathOutputDelimiter))
-        .pop()?.split(':')
+        .pop()?.split(path.delimiter)
         .map(RDBinInShellPath.removeTrailingSlash) ?? [];
-      const integrationPath = RDBinInShellPath.removeTrailingSlash(paths.integration);
       const desiredDirs = dirs.filter(p => p === integrationPath);
-      const exe = path.basename(this.executable);
 
       passed = desiredDirs.length > 0;
-      description = `The \`~/.rd/bin\` directory has not been added to the \`PATH\`, so command-line utilities are not configured in your **${ exe }** shell.`;
+      description = `The \`~/.rd/bin\` directory has not been added to the \`PATH\`, so command-line utilities are not configured in your **${ this.executable }** shell.`;
       if (passed) {
-        description = `The \`~/.rd/bin\` directory is found in your \`PATH\` as seen from **${ exe }**.`;
+        description = `The \`~/.rd/bin\` directory is found in your \`PATH\` as seen from **${ this.executable }**.`;
       } else if (pathStrategy !== PathManagementStrategy.RcFiles) {
         const description = `You have selected manual \`PATH\` configuration;
             consider letting Rancher Desktop automatically configure it.`;