Improve the post-install script - #353

• X
• Mastodon
• LinkedIn
• Facebook

data/DownloadUrls.psd1 (1)

14-18: Third-party source for curl x86 build.

The x86 curl URL points to a SourceForge project (muldersoft/cURL) rather than the official curl.se distribution. Consider the supply chain security implications of using third-party builds, or document why the official source doesn't provide x86 builds.

postInstall.ps1 (1)

183-210: ARM architecture detection may need refinement.

Architecture value 5 represents 32-bit ARM, not necessarily ARM64. On Windows ARM64, the processor may report differently. Consider using $env:PROCESSOR_ARCHITECTURE as an additional check, which returns ARM64 on 64-bit ARM systems.

 switch ($proc.Architecture) {
     0       { $arch = "x86"   }   # 32-bit Intel/AMD
-    5       { $arch = "arm64" }   # treat ARM as arm64 target
+    5       {                     # ARM variant
+        if ([System.Environment]::Is64BitOperatingSystem) {
+            $arch = "arm64"
+        } else {
+            $arch = "x86"   # fallback for 32-bit ARM
+        }
+    }
     9       { $arch = "x64"   }   # 64-bit Intel/AMD

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

setup.iss (2)

43-43: Typo in component description.

"Download and Nmap and Npcap" should be "Download Nmap and Npcap".

---

104-105: API key embedded in installer metadata.

This key is exposed in the compiled installer and should be treated as public. Confirm this is an ingestion-only key with minimal permissions.

postInstall.ps1 (1)

87-91: Privacy consideration: machine name as telemetry identifier.

Using $env:COMPUTERNAME as the fallback distinct_id may be more identifying than intended for anonymous telemetry.

lib/adb.js (1)

94-95: Add validation for the adb binary path.

The path now assumes android_platform_tools was installed to AppData. If this component wasn't selected during installation, calls to getDevices(), pull(), push(), etc. will fail. While sendShell silently catches errors, other methods don't handle missing binaries gracefully.

Consider adding a validation helper or fallback:

+    this.validateBinPath = function() {
+        var FSO = CreateObject("Scripting.FileSystemObject");
+        if (!FSO.FileExists(this.binPath)) {
+            throw new Error("ADB binary not found at: " + this.binPath + 
+                ". Ensure 'Android Platform Tools' component was installed.");
+        }
+        return this;
+    };
+
     // set the binary path
     this.binPath = SYS.getAppDataDir() + "\\android_platform_tools\\adb.exe";
     this._interface.setPrefix(this.binPath);

Alternatively, consumers can use setBinPath() to override with a custom location when the default isn't available.

postInstall.ps1 (2)

700-711: Consider silent installation flags for GTK runtime.

The installer runs without arguments, which may display a GUI requiring user interaction. For unattended installations, consider adding silent flags if supported.

         if (Test-Path $GtkRuntimeInstaller) {
             Write-Host "[*] Running GTK runtime installer (wait): $GtkRuntimeInstaller"
-            Start-Process -FilePath $GtkRuntimeInstaller -Wait -ErrorAction Stop
+            Start-Process -FilePath $GtkRuntimeInstaller -ArgumentList "/S" -Wait -ErrorAction Stop
         }

Verify the correct silent flag for the GTK runtime installer (commonly /S or /silent).

---

776-792: Npcap/Nmap installers may require user interaction.

These installers are executed without silent flags. Npcap in particular often prompts for driver installation approval. For fully automated installs, consider:

         if (Test-Path $NpcapInstaller) {
             Write-Host "[*] Running Npcap installer (wait): $NpcapInstaller"
-            Start-Process -FilePath $NpcapInstaller -Wait -ErrorAction Stop
+            # /S for silent, /winpcap_mode for compatibility
+            Start-Process -FilePath $NpcapInstaller -ArgumentList "/S" -Wait -ErrorAction Stop
         }

Note: Silent Npcap installation may require additional flags. Verify with Npcap documentation.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

postInstall.ps1 (2)

104-108: Privacy issue: Machine name used as telemetry identifier.

This fallback to $env:COMPUTERNAME exposes the machine name in telemetry, which is user-identifying and not anonymous. A previous review suggested generating a persistent random UUID instead.

Based on past review feedback, consider generating and persisting an anonymous UUID rather than sending the raw machine name.

---

209-214: Critical: Incorrect ARM architecture mapping.

Win32_Processor.Architecture value 5 is ARM (32-bit), not ARM64. Value 12 is ARM64 (64-bit) but is not handled and will fall back to x86. This causes incorrect binary downloads on ARM systems.

Per Microsoft documentation, update the switch to:

        switch ($proc.Architecture) {
            0       { $arch = "x86"   }   # 32-bit Intel/AMD
-            5       { $arch = "arm64" }   # treat ARM as arm64 target
+            5       { $arch = "arm"   }   # 32-bit ARM
            9       { $arch = "x64"   }   # 64-bit Intel/AMD
+            12      { $arch = "arm64" }   # 64-bit ARM
            default { $arch = "x86"   }   # fallback
        }

postInstall.ps1 (5)

281-281: Consider adding timeout to prevent indefinite hangs.

Invoke-WebRequest without a timeout can hang indefinitely on unresponsive endpoints. Adding -TimeoutSec would improve resilience.

-            Invoke-WebRequest -Uri $Url -OutFile $DestinationPath -UseBasicParsing
+            Invoke-WebRequest -Uri $Url -OutFile $DestinationPath -UseBasicParsing -TimeoutSec 300

---

406-488: Optional: Reduce code duplication in download phase.

The download logic is highly repetitive across components. Consider extracting into a helper function to improve maintainability.

Example refactoring:

function Download-Component {
    param(
        [string]$ComponentName,
        [string]$DestinationPath
    )
    
    if (Test-ComponentSelected -Name $ComponentName) {
        $url = Get-DownloadUrl -Component $ComponentName -Arch $arch
        if ($url) {
            Download-File -Url $url -DestinationPath $DestinationPath
        }
        else {
            Write-Host "[*] $ComponentName URL not available for arch: $arch. Skipping download."
        }
    }
    else {
        Write-Host "[*] $ComponentName component not selected. Skipping download."
    }
}

# Then use: Download-Component -ComponentName "python" -DestinationPath $PythonCompressed

---

835-835: Consider logging VC_redist installer errors.

Using -ErrorAction SilentlyContinue suppresses installer errors. If the VC redistributable is required, installation failures will go unnoticed.

-            Start-Process -FilePath $vcRedist.FullName -Wait -ErrorAction SilentlyContinue
+            try {
+                Start-Process -FilePath $vcRedist.FullName -Wait -ErrorAction Stop
+            }
+            catch {
+                Write-Host "[WARN] VC_redist.x86 installer failed: $($_.Exception.Message)"
+            }

---

716-720: Consider checking for administrator privileges.

Installers like GTK runtime, Npcap, and Nmap typically require administrator privileges. The script doesn't check for or request elevation, which may cause silent failures.

Add an admin check at the script start:

function Test-Administrator {
    $currentUser = [Security.Principal.WindowsIdentity]::GetCurrent()
    $principal = New-Object Security.Principal.WindowsPrincipal($currentUser)
    return $principal.IsInRole([Security.Principal.WindowsBuiltInRole]::Administrator)
}

if (-not (Test-Administrator)) {
    Write-Host "[WARN] Some components may require administrator privileges to install."
    Write-Host "[WARN] Consider running this script as administrator."
}

Also applies to: 794-796, 803-805

---

256-295: Recommend adding checksum validation for downloaded files.

The script downloads and executes files without verifying their integrity. If URLs in DownloadUrls.psd1 are compromised or DNS is poisoned, malicious files could be installed.

Consider adding SHA256 checksums to DownloadUrls.psd1 and validating after download:

function Verify-FileChecksum {
    param(
        [string]$FilePath,
        [string]$ExpectedHash
    )
    
    if (-not $ExpectedHash) {
        Write-Host "[WARN] No checksum provided for verification."
        return $true
    }
    
    $actualHash = (Get-FileHash -Path $FilePath -Algorithm SHA256).Hash
    if ($actualHash -eq $ExpectedHash) {
        Write-Host "[*] Checksum verified: $FilePath"
        return $true
    }
    else {
        Write-Host "[ERROR] Checksum mismatch for $FilePath"
        Write-Host "        Expected: $ExpectedHash"
        Write-Host "        Actual  : $actualHash"
        return $false
    }
}

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

postInstall.ps1 (2)

104-108: Privacy concern: Machine name used as telemetry ID (duplicate issue).

This issue was previously flagged but remains unaddressed. Using $env:COMPUTERNAME as the fallback for distinct_id can expose identifying information. The previous suggestion to generate and persist a random UUID instead of sending the raw machine name is still valid and should be implemented for better privacy.

---

209-214: Critical: Missing ARM 32-bit architecture support (duplicate issue).

This issue was previously flagged but remains unaddressed. The switch statement is missing case 5 for ARM (32-bit). Per Microsoft documentation, Win32_Processor.Architecture value 5 represents ARM (32-bit), while 12 represents ARM64 (64-bit). The current code only handles 12, which means 32-bit ARM systems will incorrectly fall back to x86 binaries.

The previous review suggested adding case 5:

         switch ($proc.Architecture) {
             0       { $arch = "x86"   }   # 32-bit Intel/AMD
+            5       { $arch = "arm"   }   # 32-bit ARM
             12      { $arch = "arm64" }   # treat ARM as arm64 target
             9       { $arch = "x64"   }   # 64-bit Intel/AMD
             default { $arch = "x86"   }   # fallback
         }

postInstall.ps1 (1)

835-835: Consider consistent error handling for installer execution.

The VC_redist installer uses -ErrorAction SilentlyContinue (line 835) while other installers (GTK runtime at line 720, Npcap at line 796, Nmap at line 805) use -ErrorAction Stop. If this is intentional because VC_redist is optional, consider adding a comment to clarify. Otherwise, using consistent error handling would improve maintainability.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro