From 4d2e8677d46d63318e1be902afd4e9b702a985b9 Mon Sep 17 00:00:00 2001
From: Michal <michal@itaz.eu>
Date: Tue, 17 Mar 2026 22:31:17 +0000
Subject: [PATCH] fix: PID file permission handling + root check

- Require root when dnsmasq is needed (clear error message)
- Handle stale PID files owned by different user (remove + recreate)
- Create bastion dir with 755 permissions
- 3 new PID file tests (30 total)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---
 bastion/src/bastion/src/main.ts         | 38 +++++++++++++++++--------
 bastion/src/bastion/tests/state.test.ts | 37 +++++++++++++++++++++++-
 2 files changed, 62 insertions(+), 13 deletions(-)

diff --git a/bastion/src/bastion/src/main.ts b/bastion/src/bastion/src/main.ts
index 5264a04..d4021fc 100644
--- a/bastion/src/bastion/src/main.ts
+++ b/bastion/src/bastion/src/main.ts
@@ -92,24 +92,38 @@ export async function startBastion(overrides: Partial<BastionConfig> = {}): Prom
   config = populateNetworkConfig(config);
 
   // PID file management: kill old instance if running
-  const pidFile = `${config.bastionDir}/bastion.pid`;
-  mkdirSync(config.bastionDir, { recursive: true });
+  // Bastion needs root for dnsmasq (DHCP port 67)
+  if (!config.skipDnsmasq && process.getuid?.() !== 0) {
+    logger.error("Must run as root (dnsmasq needs DHCP/TFTP ports). Use: sudo lab init bastion standalone start");
+    process.exit(1);
+  }
 
-  if (existsSync(pidFile)) {
-    const oldPid = parseInt(readFileSync(pidFile, "utf-8").trim(), 10);
-    if (!isNaN(oldPid)) {
-      try {
-        process.kill(oldPid, "SIGTERM");
-        logger.info(`Killed old bastion process (PID ${oldPid})`);
-        await new Promise((r) => setTimeout(r, 1000));
-      } catch {
-        // Process already dead, continue
+  mkdirSync(config.bastionDir, { recursive: true, mode: 0o755 });
+  const pidFile = `${config.bastionDir}/bastion.pid`;
+
+  // Kill old instance if running
+  try {
+    if (existsSync(pidFile)) {
+      const oldPid = parseInt(readFileSync(pidFile, "utf-8").trim(), 10);
+      if (!isNaN(oldPid)) {
+        try {
+          process.kill(oldPid, "SIGTERM");
+          logger.info(`Killed old bastion process (PID ${oldPid})`);
+          await new Promise((r) => setTimeout(r, 1000));
+        } catch {
+          // Process already dead
+        }
       }
+      // Remove stale PID file (may be owned by different user)
+      try { unlinkSync(pidFile); } catch { /* ignore */ }
     }
+  } catch {
+    // Can't read PID file — try to remove it
+    try { unlinkSync(pidFile); } catch { /* ignore */ }
   }
 
   // Write current PID
-  writeFileSync(pidFile, String(process.pid));
+  writeFileSync(pidFile, String(process.pid), { mode: 0o644 });
 
   // Prepare directories
   mkdirSync(config.tftpDir, { recursive: true });
diff --git a/bastion/src/bastion/tests/state.test.ts b/bastion/src/bastion/tests/state.test.ts
index 1ff18ec..494b479 100644
--- a/bastion/src/bastion/tests/state.test.ts
+++ b/bastion/src/bastion/tests/state.test.ts
@@ -1,5 +1,5 @@
 import { describe, it, expect, beforeEach, afterEach } from "vitest";
-import { mkdirSync, rmSync, existsSync, readFileSync } from "node:fs";
+import { mkdirSync, rmSync, existsSync, readFileSync, writeFileSync, chmodSync } from "node:fs";
 import { join } from "node:path";
 import { tmpdir } from "node:os";
 import { StateManager } from "../src/services/state.js";
@@ -103,3 +103,38 @@ describe("StateManager", () => {
     expect(parsed.installed["aa:bb:cc:dd:ee:ff"].hostname).toBe("node1");
   });
 });
+
+describe("PID file handling", () => {
+  let testDir: string;
+
+  beforeEach(() => {
+    testDir = join(tmpdir(), `bastion-pid-test-${Date.now()}-${Math.random().toString(36).slice(2)}`);
+    mkdirSync(testDir, { recursive: true });
+  });
+
+  afterEach(() => {
+    rmSync(testDir, { recursive: true, force: true });
+  });
+
+  it("handles stale PID file from previous run", () => {
+    const pidFile = join(testDir, "bastion.pid");
+    // Simulate a stale PID file with a dead process
+    writeFileSync(pidFile, "999999999");
+    // Should be readable
+    const pid = parseInt(readFileSync(pidFile, "utf-8").trim(), 10);
+    expect(pid).toBe(999999999);
+  });
+
+  it("handles corrupted PID file gracefully", () => {
+    const pidFile = join(testDir, "bastion.pid");
+    writeFileSync(pidFile, "not-a-number\n");
+    const pid = parseInt(readFileSync(pidFile, "utf-8").trim(), 10);
+    expect(isNaN(pid)).toBe(true);
+  });
+
+  it("handles missing bastion directory", () => {
+    const missingDir = join(testDir, "nonexistent", "deep");
+    mkdirSync(missingDir, { recursive: true });
+    expect(existsSync(missingDir)).toBe(true);
+  });
+});