From 7d410b26d04b72bfa77b98334a8b3fcbb4dfb3d8 Mon Sep 17 00:00:00 2001
From: Tam Nhu Tran <kaitran.ntt@gmail.com>
Date: Sat, 28 Mar 2026 18:12:18 -0400
Subject: [PATCH] =?UTF-8?q?fix(docker):=20address=20review=20findings=20?=
 =?UTF-8?q?=E2=80=94=20PID=20guard,=20deleteBinary=20guard,=20blocked=20fa?=
 =?UTF-8?q?llback?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- Guard child.pid falsy in bootstrap (PID 0 creates immortal phantom lock)
- Add ETXTBSY/EBUSY guard to deleteBinary() (same vuln as downloadAndInstall)
- Fix error message to suggest container restart (not circular ccs docker update)
- Tighten status.blocked guard to handle missing blocker gracefully
---
 src/cliproxy/binary/installer.ts         | 15 ++++++++++++---
 src/docker/docker-bootstrap.ts           |  5 +++--
 src/web-server/health/cliproxy-checks.ts | 10 ++++++----
 3 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/src/cliproxy/binary/installer.ts b/src/cliproxy/binary/installer.ts
index 1c951722..7c8a91b9 100644
--- a/src/cliproxy/binary/installer.ts
+++ b/src/cliproxy/binary/installer.ts
@@ -50,7 +50,7 @@ export async function downloadAndInstall(
         if (verbose)
           console.error(`[cliproxy] Binary is running, skipping update: ${existingBinary}`);
         throw new Error(
-          `CLIProxy binary is currently running and cannot be replaced. Stop the running instance first, or use 'ccs docker update' to update in place.`
+          'CLIProxy binary is currently running and cannot be replaced. Restart the container to apply the update.'
         );
       }
       throw error;
@@ -114,8 +114,17 @@ export function deleteBinary(binPath: string, verbose = false, backend?: CLIProx
   const effectiveBackend = backend ?? DEFAULT_BACKEND;
   const binaryPath = path.join(binPath, getExecutableName(effectiveBackend));
   if (fs.existsSync(binaryPath)) {
-    fs.unlinkSync(binaryPath);
-    if (verbose) console.error(`[cliproxy] Deleted: ${binaryPath}`);
+    try {
+      fs.unlinkSync(binaryPath);
+      if (verbose) console.error(`[cliproxy] Deleted: ${binaryPath}`);
+    } catch (error: unknown) {
+      const code =
+        error instanceof Error && 'code' in error ? (error as { code: string }).code : '';
+      if (code === 'ETXTBSY' || code === 'EBUSY') {
+        throw new Error('CLIProxy binary is currently running and cannot be deleted.');
+      }
+      throw error;
+    }
   }
 }
 
diff --git a/src/docker/docker-bootstrap.ts b/src/docker/docker-bootstrap.ts
index 96629391..00ea40a2 100644
--- a/src/docker/docker-bootstrap.ts
+++ b/src/docker/docker-bootstrap.ts
@@ -37,8 +37,9 @@ async function runCliproxy(): Promise<number> {
     // Register session lock so dashboard can detect the running proxy
     let sessionId: string | undefined;
     child.on('spawn', () => {
-      const version = getInstalledCliproxyVersion() ?? undefined;
-      sessionId = registerSession(CLIPROXY_DEFAULT_PORT, child.pid ?? 0, version, 'plus');
+      if (!child.pid) return;
+      const version = getInstalledCliproxyVersion();
+      sessionId = registerSession(CLIPROXY_DEFAULT_PORT, child.pid, version, 'plus');
     });
 
     child.on('error', reject);
diff --git a/src/web-server/health/cliproxy-checks.ts b/src/web-server/health/cliproxy-checks.ts
index 07e63cc6..92335d6a 100644
--- a/src/web-server/health/cliproxy-checks.ts
+++ b/src/web-server/health/cliproxy-checks.ts
@@ -158,14 +158,16 @@ export async function checkCliproxyPort(): Promise<HealthCheck> {
     };
   }
 
-  if (status.blocked && status.blocker) {
+  if (status.blocked) {
     return {
       id: 'cliproxy-port',
       name: 'CLIProxy Port',
       status: 'warning',
-      message: `Occupied by ${status.blocker.processName}`,
-      details: `PID ${status.blocker.pid}`,
-      fix: `Kill process: kill ${status.blocker.pid}`,
+      message: status.blocker
+        ? `Occupied by ${status.blocker.processName}`
+        : 'Port occupied by unknown process',
+      details: status.blocker ? `PID ${status.blocker.pid}` : undefined,
+      ...(status.blocker && { fix: `Kill process: kill ${status.blocker.pid}` }),
     };
   }