security: fix critical/high review findings from production readiness audit

CRITICAL:
- #1: Command injection via SUDO_USER — switched execSync to
  execFileSync in getUserHome() to bypass shell interpolation
- #2: Symlink attack in /tmp — selfUpdate() now uses mkdtempSync
  with 0700 perms instead of hardcoded /tmp paths

HIGH:
- #3: SHA256 verification failure — download binary with artifact
  name so sha256sum --check finds the correct file
- Light-Heart-Labs#4: Broken rollback — verify new binary via exitCode check,
  not dead catch block (throwOnError: false skips catch)
- Light-Heart-Labs#5: Data loss in uninstall — docker compose down -v now
  conditional on !keepData
- Light-Heart-Labs#6: rm -rf path safety — refuse system directories (/, /home,
  /root, /usr, etc.) with structural depth check

MEDIUM:
- Light-Heart-Labs#7: Model download failure halts install (throw instead of
  silent return) — prevents llama-server crash-loop
- Light-Heart-Labs#8: Tier change now applies CTX_SIZE even when model name
  unchanged (Tier 1->2 both use qwen3-8b but differ in context)

Tests: updated model.test.ts to expect throw on download failure
All 138 tests passing

Author: igorls

dream-server/cli-installer/docs/context_source.md

@@ -0,0 +1,88 @@
+# Production Readiness Review — Dream Server CLI Installer
+
+## Project Context
+
+Dream Server is a local AI management platform that orchestrates a multi-container Docker stack (LLM inference via llama-server, Open WebUI chat, SearXNG search, ComfyUI image gen, n8n workflows, Qdrant vector DB, etc.). This CLI installer is the primary user-facing tool for installing, configuring, updating, and diagnosing the entire stack.
+
+**Runtime**: Bun (TypeScript), distributed as a single static binary for Linux x64/arm64.
+**Privilege model**: Runs as a regular user or via `sudo`. Detects `SUDO_USER` to resolve the real user's home directory. Docker socket access is required.
+**Target users**: Non-technical users running bare-metal GPU servers (NVIDIA/AMD). The installer must be robust to partial failures, network interruptions, and misconfigurations.
+**Threat model**: The binary self-updates from GitHub Releases with SHA256 verification. Secrets are generated with `crypto.getRandomValues()`. The installer runs `docker compose up` on the user's system, so command injection via .env values is a risk surface.
+
+## Review Scope
+
+This is a **production readiness** review before merging into `main`. The codebase is feature-complete (Phases 1-3 done, 138 tests passing). We need to identify:
+
+1. Bugs or crash vectors that would break the installer for end users
+2. Security issues in secret handling, self-update, or command execution
+3. Error handling gaps — any situation where the installer would hang, crash silently, or leave the system in a broken state
+4. Robustness issues — race conditions, timeouts, unhandled edge cases
+5. Test coverage gaps — critical paths that are not tested
+
+## Attached Context Packs
+
+| File | Contents | Token Estimate |
+|------|----------|---------------|
+| `context_source.md` | All source code: commands, lib, phases, entry point | ~28K |
+| `context_tests.md` | All 21 test files covering 138 tests | ~20K |
+
+## Focus Areas
+
+### 1. Command Injection & Input Sanitization
+- Can malicious `.env` values inject shell commands? The env parser reads user-edited files and values end up in `exec()` calls via Docker compose.
+- Are user-supplied paths (`--dir`) sanitized before use in `exec()`, `Bun.write()`, `join()`?
+- Is the `SUDO_USER` environment variable trusted safely in `getUserHome()` (used in `execSync(getent passwd $SUDO_USER)`)?
+
+### 2. Self-Update Security
+- Is the SHA256 verification in `update.ts` correct? Check for TOCTOU between download and verification, partial download corruption, and checksum file format parsing.
+- Can the rollback mechanism leave the binary in a broken state?
+- Is the binary URL construction safe from path traversal?
+
+### 3. Error Handling & Graceful Degradation
+- Are there any `await` calls without timeout or error handling that could hang forever?
+- Do all `process.exit()` calls have appropriate cleanup (dangling Docker containers, partial file writes)?
+- What happens if Docker daemon crashes mid-install? Is the state recoverable on re-run?
+- Are all `catch {}` blocks handling errors appropriately, or are some silently swallowing important failures?
+
+### 4. Concurrency & Race Conditions
+- Are there any TOCTOU issues (checking file existence then reading/writing)?
+- Can concurrent installer runs corrupt the `.env` file or data directories?
+- Is the health check retry loop safe against resource exhaustion?
+
+### 5. Secret Generation & Handling
+- Are secrets generated with sufficient entropy? Check `crypto.getRandomValues()` usage.
+- Are secrets accidentally logged or printed to stdout?
+- Could `.env` merge logic accidentally expose or duplicate secrets?
+
+### 6. Docker & System Interaction
+- Is the compose command detection (`docker compose` vs `docker-compose` vs `sudo docker compose`) robust?
+- Are there timeout values that are too short for slow systems (e.g., model download, large image pulls)?
+- Does `nvidia-smi` parsing handle all real-world output formats?
+- Is port checking via `ss`/`netstat` reliable? Does the regex catch IPv6 bindings?
+
+### 7. Test Coverage Gaps
+- Are there critical paths in the source code that have NO corresponding test?
+- Are mock setups realistic? (e.g., do they mock at the right level, or do they allow real system calls to leak through?)
+- Are edge cases tested? (empty .env, corrupt .env, Docker daemon unreachable, no network, disk full)
+
+### 8. Code Quality & Maintainability
+- Dead code, unused imports, or unreachable branches
+- Functions with too many responsibilities
+- Magic strings or numbers that should be constants
+- Inconsistent error message formatting
+
+## Output Format
+
+For each finding, provide:
+
+- **Severity**: CRITICAL / HIGH / MEDIUM / LOW
+- **File**: path and line numbers
+- **Category**: which focus area (1-8 above)
+- **Description**: what the issue is
+- **Impact**: what happens in production
+- **Suggested Fix**: concrete code-level recommendation
+
+Group findings by severity (CRITICAL first). End with:
+- Total counts per severity
+- Top 3 highest-priority fixes
+- Overall production readiness assessment (READY / READY WITH CAVEATS / NOT READY)

dream-server/cli-installer/docs/context_tests.md

@@ -0,0 +1,187 @@
+# Production Readiness Review — Dream Server CLI Installer
+
+Here is the production readiness review for the Dream Server CLI Installer based on the provided source code and test suite.
+
+## Findings
+
+### CRITICAL
+
+**1. Root Command Injection via `SUDO_USER` Variable**
+
+- **Severity**: CRITICAL
+- **File**: `src/lib/config.ts`, line 21
+- **Category**: 1. Command Injection & Input Sanitization
+- **Description**: The `getUserHome()` function resolves the original user's home directory by passing the `SUDO_USER` environment variable directly into a template literal: `execSync(\`getent passwd ${sudoUser}`)`. Because `execSync` with a string argument spawns a shell (`/bin/sh`) to evaluate the command, any shell metacharacters will be executed.
+- **Impact**: A user or compromised script executing the installer via `sudo` can pass a maliciously crafted `SUDO_USER` variable (e.g., `SUDO_USER="root; rm -rf /"`), leading to arbitrary command execution as `root` before the CLI even processes application logic. This is a trivial Local Privilege Escalation (LPE).
+- **Suggested Fix**: Use `execFileSync` to pass arguments as a strict array, bypassing the shell entirely:
+
+```typescript
+import { execFileSync } from "node:child_process";
+// ...
+const result = execFileSync("getent", ["passwd", sudoUser], {
+  encoding: "utf-8",
+  timeout: 2000,
+});
+```
+
+**2. Arbitrary File Overwrite / Privilege Escalation via `/tmp` (Symlink Attack)**
+
+- **Severity**: CRITICAL
+- **File**: `src/commands/update.ts`, lines 101-102
+- **Category**: 2. Self-Update Security & 4. Concurrency & Race Conditions
+- **Description**: The self-updater downloads the new binary and checksum to hardcoded, world-writable paths (`/tmp/dream-installer-update`). When `curl -o` executes, it will follow any existing symlinks at that location.
+- **Impact**: An unprivileged local attacker can pre-create `/tmp/dream-installer-update` as a symlink pointing to a critical system file (e.g., `/etc/shadow` or `/usr/bin/sudo`). When an admin runs `sudo dream-installer update`, `curl` will overwrite the target file with the binary, resulting in complete system compromise or destruction.
+- **Suggested Fix**: Use `node:fs` `mkdtempSync` to create a secure, randomly named directory with strict `0700` permissions.
+
+```typescript
+import { mkdtempSync } from "node:fs";
+import { tmpdir } from "node:os";
+const tmpDir = mkdtempSync(join(tmpdir(), "dream-update-"));
+const tmpPath = join(tmpDir, getBinaryName());
+```
+
+### HIGH
+
+**3. Guaranteed SHA256 Verification Failure**
+
+- **Severity**: HIGH
+- **File**: `src/commands/update.ts`, lines 118-121
+- **Category**: 2. Self-Update Security
+- **Description**: The downloaded `.sha256` checksum file generated by standard release tooling contains the original artifact filename (e.g., `dream-installer-linux-x64`). However, the installer saves the downloaded binary as `dream-installer-update`. When `sha256sum --check` reads the file, it searches for the original filename in `/tmp`, fails to find it, and unconditionally aborts the update.
+- **Impact**: The self-update integrity check is fundamentally broken and will permanently fail in production.
+- **Suggested Fix**: Combine this with the fix for Finding #2 by downloading the binary into the secure temporary directory using its actual artifact name (`getBinaryName()`) so `sha256sum` can successfully locate it.
+
+**4. Broken Rollback Mechanism Deletes Working Backups**
+
+- **Severity**: HIGH
+- **File**: `src/commands/update.ts`, lines 142-144
+- **Category**: 3. Error Handling & Graceful Degradation
+- **Description**: After replacing the binary, the updater validates it using `await exec([currentBinary, '--version'], { throwOnError: false })`. Because `throwOnError` is explicitly false, `exec` returns the `exitCode` instead of throwing an exception if the new binary segfaults or fails.
+- **Impact**: The `catch` block containing the rollback logic is completely bypassed. The script incorrectly assumes success, deletes the working `.bak` backup file, and leaves the user with a permanently bricked CLI.
+- **Suggested Fix**: Explicitly check the `exitCode` and manually throw an error to trigger the rollback logic:
+
+```typescript
+const { exitCode } = await exec([currentBinary, "--version"], {
+  throwOnError: false,
+  timeout: 5000,
+});
+if (exitCode !== 0) throw new Error("New binary failed execution");
+```
+
+**5. Unconditional Deletion of Named Volumes (Data Loss)**
+
+- **Severity**: HIGH
+- **File**: `src/commands/uninstall.ts`, lines 94-98
+- **Category**: 3. Error Handling & Graceful Degradation
+- **Description**: During the uninstall process, the CLI executes `docker compose down -v` unconditionally. The check for the user's `--keep-data` flag doesn't occur until a later step involving directory removal.
+- **Impact**: Any containers relying on Docker named volumes (e.g., Qdrant, Postgres) will have their data irrevocably wiped, even if the user explicitly provided the `--keep-data` argument.
+- **Suggested Fix**: Conditionally append the `-v` flag to the arguments array.
+
+```typescript
+const downArgs = ["down"];
+if (!opts.keepData) downArgs.push("-v");
+await exec([...composeCmd, ...downArgs], {
+  cwd: installDir,
+  throwOnError: false,
+  timeout: 30_000,
+});
+```
+
+**6. Catastrophic Root Directory Wipe Vector**
+
+- **Severity**: HIGH
+- **File**: `src/commands/uninstall.ts`, line 114
+- **Category**: 1. Command Injection & Input Sanitization
+- **Description**: The uninstaller runs `exec(['rm', '-rf', installDir])` with no structural path validation. It relies solely on the prior detection of a `.env` file.
+- **Impact**: If a user runs `dream-installer uninstall --dir / --force` (or `--dir /home`) and an errant `.env` file happens to exist in that location, the script will wipe the host's root filesystem or user home directory.
+- **Suggested Fix**: Implement strict path safety guards against top-level system directories.
+
+```typescript
+import { resolve } from "node:path";
+const target = resolve(installDir);
+if (["/", "/home", "/root", "/usr", "/etc"].includes(target)) {
+  throw new Error(
+    "Safety check failed: Refusing to delete critical system directory.",
+  );
+}
+```
+
+### MEDIUM
+
+**7. Installer Silently Proceeds When Model Download Fails**
+
+- **Severity**: MEDIUM
+- **File**: `src/phases/model.ts`, lines 91-98
+- **Category**: 3. Error Handling & Graceful Degradation
+- **Description**: If the LLM model download fails after 3 attempts, `downloadModel` simply prints a console message and returns `void` normally instead of halting execution.
+- **Impact**: The `install.ts` orchestrator proceeds to start the Docker Compose stack. `llama-server` immediately enters a crash-loop because the `.gguf` file is missing. The user receives a misleading "Installation Complete" message for a broken system.
+- **Suggested Fix**: Throw an error to halt the installation cleanly.
+
+```typescript
+if (!success)
+  throw new Error("Model download failed. Cannot proceed with installation.");
+```
+
+**8. Missing Context Size Updates on Tier Changes**
+
+- **Severity**: MEDIUM
+- **File**: `src/commands/config.ts`, lines 94-96
+- **Category**: 3. Error Handling & Graceful Degradation
+- **Description**: When applying a new tier, the code checks `if (tierConfig.model !== currentModel)` before updating `.env`. If a user upgrades between tiers that share the same base model (e.g., upgrading Tier 1 to Tier 2 both use `qwen3-8b`), the condition evaluates to false.
+- **Impact**: Upgrading tiers fails to increase `CTX_SIZE` and `MAX_CONTEXT` from 16384 to 32768, depriving the user of the expected performance upgrade.
+- **Suggested Fix**: Always apply configuration changes if a new tier is selected, or compare the explicit tier ID instead of the model name.
+
+**9. Fallback Logic Dead Code via `throwOnError: false**`
+
+- **Severity**: MEDIUM
+- **File**: `src/commands/status.ts` (lines 75, 126) & `src/commands/doctor.ts` (line 195)
+- **Category**: 3. Error Handling & Graceful Degradation
+- **Description**: Multiple scripts wrap `exec(..., { throwOnError: false })` in `try...catch` blocks. Because `exec` returns an exit code object rather than throwing, the `catch` blocks are completely dead code.
+- **Impact**: Fallbacks (like parsing older non-JSON `docker compose ps` formats) are never triggered. In `doctor.ts`, if `nvidia-smi` crashes, it parses an empty string into `NaN < 535` (false) and logs a blank successful driver string instead of failing.
+- **Suggested Fix**: Remove `{ throwOnError: false }` from `exec` calls that are expected to trigger a `catch` block on failure.
+
+**10. Zero Test Coverage for Critical Execution Paths**
+
+- **Severity**: MEDIUM
+- **File**: `tests/update.test.ts` & `tests/doctor.test.ts`
+- **Category**: 7. Test Coverage Gaps
+- **Description**: `update.test.ts` explicitly bypasses `selfUpdate()` using `skipSelfUpdate: true`. `doctor.test.ts` never imports or executes `doctor()`, but instead manually reproduces and tests its internal regex logic.
+- **Impact**: The highest-risk operations (binary replacement, SHA validation, system diagnostics) have 0% effective test coverage, which allowed the critical TOCTOU and checksum bugs to remain undetected.
+- **Suggested Fix**: Write integration tests that invoke the actual functions, mocking out `globalThis.fetch` and `fs.mkdtempSync` for updates, and `shell.exec` for the doctor command.
+
+### LOW
+
+**11. MacOS Incompatibility for Disk Check**
+
+- **Severity**: LOW
+- **File**: `src/commands/doctor.ts` (line 179) & `src/phases/detection.ts` (line 58)
+- **Category**: 6. Docker & System Interaction
+- **Description**: The script executes `df -BG`. The `-B` flag is GNU-specific. On MacOS environments, this throws an "illegal option" error, bypassing the disk space check.
+- **Suggested Fix**: Use a POSIX-compliant format like `df -m` and perform the gigabyte division mathematically.
+
+**12. Doctor Command Hides Valid Port Conflicts**
+
+- **Severity**: LOW
+- **File**: `src/commands/doctor.ts`, lines 152-156
+- **Category**: 8. Code Quality & Maintainability
+- **Description**: `doctor.ts` silently hides port conflicts if Docker is running (`&& !composeCmd`). If an external service has hijacked a port while Docker is running, the user is never warned.
+- **Suggested Fix**: Surface the occupied ports via `ui.info`, appending "(expected if Dream Server is running)" rather than suppressing the output entirely.
+
+---
+
+## Summary
+
+- **Total Counts per Severity**
+- **CRITICAL**: 2
+- **HIGH**: 4
+- **MEDIUM**: 4
+- **LOW**: 2
+
+- **Top 3 Highest-Priority Fixes**
+
+1. **Fix Command Injection**: Immediately swap `execSync` for `execFileSync` in `getUserHome()` to close the `SUDO_USER` root escalation vector.
+2. **Rewrite Self-Update Logic**: Use `fs.mkdtempSync` to prevent symlink exploits, ensure the downloaded file matches the `.sha256` payload name, and fix the `throwOnError: false` bug so rollbacks actually work.
+3. **Implement Uninstall Safeguards**: Add strict path validation to prevent catastrophic system directory deletion, and conditionally append `-v` to protect Docker volumes when `--keep-data` is requested.
+
+- **Overall Production Readiness Assessment**: **NOT READY**
+  The CLI installer has excellent orchestration logic and UX, but it ships with a root command injection vulnerability, catastrophic uninstallation data loss vectors, and a fundamentally broken self-update mechanism. Once the Critical and High severity findings are remediated, the codebase will be ready for production distribution.

dream-server/cli-installer/docs/deep_think_prompt.md

@@ -112,13 +112,23 @@ export async function config(opts: ConfigOptions): Promise<void> {
 
     const [tierId, tierConfig] = tierEntries[tierChoice];
 
-    if (tierConfig.model !== currentModel) {
+    const currentTier = getEnv('TIER');
+    const tierChanged = tierId !== currentTier;
+    const modelChanged = tierConfig.model !== currentModel;
+
+    if (tierChanged || modelChanged) {
       setEnv('LLM_MODEL', tierConfig.model);
       setEnv('GGUF_FILE', tierConfig.ggufFile);
       setEnv('CTX_SIZE', String(tierConfig.context));
       setEnv('MAX_CONTEXT', String(tierConfig.context));
+      setEnv('TIER', tierId);
       changed = true;
-      ui.ok(`Model: ${currentModel} → ${tierConfig.model}`);
+
+      if (modelChanged) {
+        ui.ok(`Model: ${currentModel} → ${tierConfig.model}`);
+      } else {
+        ui.ok(`Tier ${currentTier} → ${tierId} (context: ${tierConfig.context})`);
+      }
 
       // Check if new model needs downloading
       const modelsDir = join(installDir, 'data', 'models');

dream-server/cli-installer/docs/review_1.md

@@ -6,7 +6,7 @@ import { DEFAULT_INSTALL_DIR } from '../lib/config.ts';
 import * as ui from '../lib/ui.ts';
 import * as prompts from '../lib/prompts.ts';
 import { existsSync } from 'node:fs';
-import { join } from 'node:path';
+import { join, resolve } from 'node:path';
 
 export interface UninstallOptions {
   dir?: string;
@@ -94,14 +94,16 @@ export async function uninstall(opts: UninstallOptions): Promise<void> {
       }
     }
 
-    // ── Step 3: Remove Docker volumes ──
-    try {
-      await exec(
-        [...composeCmd, 'down', '-v'],
-        { cwd: installDir, throwOnError: false, timeout: 30_000 },
-      );
-      ui.ok('Docker volumes removed');
-    } catch { /* volumes may not exist */ }
+    // ── Step 3: Remove Docker volumes (only if NOT keeping data) ──
+    if (!opts.keepData) {
+      try {
+        await exec(
+          [...composeCmd, 'down', '-v'],
+          { cwd: installDir, throwOnError: false, timeout: 30_000 },
+        );
+        ui.ok('Docker volumes removed');
+      } catch { /* volumes may not exist */ }
+    }
 
     // ── Step 4: Remove network ──
     try {
@@ -118,6 +120,14 @@ export async function uninstall(opts: UninstallOptions): Promise<void> {
   } else {
     const deleteData = opts.force || await prompts.confirm(`Delete installation directory ${installDir}?`);
     if (deleteData) {
+      // Safety guard: refuse to rm -rf critical system directories
+      const target = resolve(installDir);
+      const DANGEROUS_PATHS = ['/', '/home', '/root', '/usr', '/etc', '/var', '/boot', '/bin', '/sbin', '/lib', '/opt', '/tmp'];
+      if (DANGEROUS_PATHS.includes(target) || target.split('/').filter(Boolean).length < 2) {
+        ui.fail(`Safety check: refusing to delete system directory: ${target}`);
+        return;
+      }
+
       const delSpinner = new ui.Spinner(`Removing ${installDir}...`);
       delSpinner.start();
       try {