security: harden daemon, DNS, forwarder, and Windows script generation

- Sanitize project/service names in PowerShell script output (M1)
- Add mutex to daemon maps to prevent concurrent write panic (M2)
- Restrict Unix socket permissions to 0660 (M3)
- Validate loopback IPs before netlink add/remove (L2)
- Restrict collision file permissions to 0600 (L3)
- Replace pgrep with PID file for daemon management (L4)
- Add 5s dial timeout to TCP forwarder (L5)
- Validate Docker Compose labels against allowed charset (L6)
- Fix test race conditions in DNS and forwarder test helpers

Author: alysnnix

cmd/devproxy/main.go

@@ -11,7 +11,9 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"strconv"
 	"strings"
+	"syscall"
 
 	"github.com/alysnnix/devproxy/internal/daemon"
 	"github.com/alysnnix/devproxy/internal/ipman"
@@ -70,34 +72,30 @@ func runDaemon() {
 }
 
 func runDown() {
-	// Find and kill all devproxy daemon processes
-	out, err := exec.Command("pgrep", "-f", "devproxy daemon").Output()
+	pidPath := "/run/devproxy/devproxy.pid"
+
+	data, err := os.ReadFile(pidPath)
 	if err != nil {
 		fmt.Println("No devproxy daemon running")
 		return
 	}
 
-	pids := strings.Fields(strings.TrimSpace(string(out)))
-	myPid := fmt.Sprintf("%d", os.Getpid())
-
-	killed := 0
-	for _, pid := range pids {
-		if pid == myPid {
-			continue
-		}
-		if err := exec.Command("kill", "-9", pid).Run(); err != nil {
-			fmt.Fprintf(os.Stderr, "failed to kill PID %s: %v\n", pid, err)
-		} else {
-			fmt.Printf("killed devproxy daemon (PID %s)\n", pid)
-			killed++
-		}
+	pid, err := strconv.Atoi(strings.TrimSpace(string(data)))
+	if err != nil {
+		fmt.Fprintf(os.Stderr, "invalid PID file: %v\n", err)
+		os.Remove(pidPath)
+		return
 	}
 
-	if killed == 0 {
-		fmt.Println("No devproxy daemon running")
+	if err := syscall.Kill(pid, syscall.SIGTERM); err != nil {
+		fmt.Fprintf(os.Stderr, "failed to stop daemon (PID %d): %v\n", pid, err)
+		os.Remove(pidPath)
 		return
 	}
 
+	fmt.Printf("stopped devproxy daemon (PID %d)\n", pid)
+	os.Remove(pidPath)
+
 	// Cleanup stale state
 	s := state.New("")
 	m := ipman.New(s)

internal/api/api.go

@@ -47,7 +47,7 @@ func (a *API) ListenAndServe() error {
 		return err
 	}
 	// Allow non-root users to connect
-	os.Chmod(a.sockPath, 0666)
+	os.Chmod(a.sockPath, 0660)
 	a.listener = ln
 
 	mux := http.NewServeMux()

internal/daemon/daemon.go

@@ -6,7 +6,9 @@ import (
 	"log/slog"
 	"os"
 	"os/signal"
+	"path/filepath"
 	"sort"
+	"sync"
 	"syscall"
 	"time"
 
@@ -21,6 +23,7 @@ import (
 )
 
 type Daemon struct {
+	mu         sync.Mutex
 	state      *state.State
 	ipman      *ipman.IPManager
 	dns        *dns.Server
@@ -52,6 +55,11 @@ func (d *Daemon) Run() error {
 	sigCh := make(chan os.Signal, 1)
 	signal.Notify(sigCh, syscall.SIGTERM, syscall.SIGINT, syscall.SIGHUP)
 
+	// Write PID file
+	pidPath := filepath.Join(filepath.Dir(d.socketPath), "devproxy.pid")
+	os.WriteFile(pidPath, []byte(fmt.Sprintf("%d", os.Getpid())), 0644)
+	defer os.Remove(pidPath)
+
 	// Cleanup stale state
 	slog.Info("cleaning up stale state...")
 	d.ipman.CleanupLoopbackIPs()
@@ -111,6 +119,9 @@ func (d *Daemon) Run() error {
 }
 
 func (d *Daemon) OnContainerStart(ctx context.Context, info watcher.ContainerInfo) error {
+	d.mu.Lock()
+	defer d.mu.Unlock()
+
 	project := info.Project
 
 	// Allocate IP if not already assigned
@@ -220,6 +231,9 @@ func (d *Daemon) OnContainerStart(ctx context.Context, info watcher.ContainerInf
 }
 
 func (d *Daemon) OnContainerDie(ctx context.Context, info watcher.ContainerInfo) error {
+	d.mu.Lock()
+	defer d.mu.Unlock()
+
 	project := info.Project
 	service := info.Service
 
@@ -263,6 +277,9 @@ func (d *Daemon) OnContainerDie(ctx context.Context, info watcher.ContainerInfo)
 }
 
 func (d *Daemon) teardownAll() {
+	d.mu.Lock()
+	defer d.mu.Unlock()
+
 	for _, cancel := range d.cancelFns {
 		cancel()
 	}

internal/dns/dns_test.go

@@ -11,8 +11,19 @@ func startTestServer(t *testing.T) (*Server, string) {
 	t.Helper()
 	s := New("127.0.0.1:0") // random port
 	go s.ListenAndServe()
-	time.Sleep(50 * time.Millisecond)
-	addr := s.Addr()
+
+	var addr string
+	for i := 0; i < 100; i++ {
+		addr = s.Addr()
+		if addr != "127.0.0.1:0" {
+			break
+		}
+		time.Sleep(5 * time.Millisecond)
+	}
+	if addr == "127.0.0.1:0" {
+		t.Fatal("DNS server did not start in time")
+	}
+
 	t.Cleanup(func() { s.Shutdown() })
 	return s, addr
 }

internal/forwarder/forwarder.go

@@ -6,6 +6,7 @@ import (
 	"log/slog"
 	"net"
 	"sync"
+	"time"
 )
 
 type Forwarder struct {
@@ -62,7 +63,7 @@ func (f *Forwarder) Start(ctx context.Context) error {
 }
 
 func (f *Forwarder) handleConn(ctx context.Context, clientConn net.Conn) {
-	targetConn, err := net.Dial("tcp", f.targetAddr)
+	targetConn, err := net.DialTimeout("tcp", f.targetAddr, 5*time.Second)
 	if err != nil {
 		slog.Error("failed to connect to target", "target", f.targetAddr, "error", err)
 		clientConn.Close()

internal/forwarder/forwarder_test.go

@@ -33,16 +33,29 @@ func startEchoServer(t *testing.T) string {
 	return ln.Addr().String()
 }
 
+func waitForForwarder(t *testing.T, f *Forwarder) string {
+	t.Helper()
+	for i := 0; i < 100; i++ {
+		addr := f.ListenAddr()
+		if addr != "127.0.0.1:0" {
+			return addr
+		}
+		time.Sleep(5 * time.Millisecond)
+	}
+	t.Fatal("forwarder did not start listening in time")
+	return ""
+}
+
 func TestForwardBidirectional(t *testing.T) {
 	echoAddr := startEchoServer(t)
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
 
 	f := New("127.0.0.1:0", echoAddr)
 	go f.Start(ctx)
-	time.Sleep(50 * time.Millisecond)
+	addr := waitForForwarder(t, f)
 
-	conn, err := net.Dial("tcp", f.ListenAddr())
+	conn, err := net.Dial("tcp", addr)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -67,12 +80,12 @@ func TestForwardShutdown(t *testing.T) {
 
 	f := New("127.0.0.1:0", echoAddr)
 	go f.Start(ctx)
-	time.Sleep(50 * time.Millisecond)
+	addr := waitForForwarder(t, f)
 
 	cancel()
 	time.Sleep(50 * time.Millisecond)
 
-	_, err := net.DialTimeout("tcp", f.ListenAddr(), 100*time.Millisecond)
+	_, err := net.DialTimeout("tcp", addr, 100*time.Millisecond)
 	if err == nil {
 		t.Error("expected connection refused after shutdown")
 	}
@@ -85,10 +98,10 @@ func TestForwardMultipleConnections(t *testing.T) {
 
 	f := New("127.0.0.1:0", echoAddr)
 	go f.Start(ctx)
-	time.Sleep(50 * time.Millisecond)
+	addr := waitForForwarder(t, f)
 
 	for i := 0; i < 5; i++ {
-		conn, err := net.Dial("tcp", f.ListenAddr())
+		conn, err := net.Dial("tcp", addr)
 		if err != nil {
 			t.Fatal(err)
 		}
@@ -112,10 +125,9 @@ func TestForwardPortConflict(t *testing.T) {
 	f1 := New("127.0.0.1:0", echoAddr)
 	errCh1 := make(chan error, 1)
 	go func() { errCh1 <- f1.Start(ctx) }()
-	time.Sleep(50 * time.Millisecond)
+	boundAddr := waitForForwarder(t, f1)
 
 	// Sanity-check: the first forwarder should be working.
-	boundAddr := f1.ListenAddr()
 	conn, err := net.Dial("tcp", boundAddr)
 	if err != nil {
 		t.Fatalf("first forwarder unreachable: %v", err)
@@ -170,9 +182,9 @@ func TestForwardHalfClose(t *testing.T) {
 
 	f := New("127.0.0.1:0", serverAddr)
 	go f.Start(ctx)
-	time.Sleep(50 * time.Millisecond)
+	addr := waitForForwarder(t, f)
 
-	conn, err := net.Dial("tcp", f.ListenAddr())
+	conn, err := net.Dial("tcp", addr)
 	if err != nil {
 		t.Fatal(err)
 	}

internal/ipman/netlink.go

@@ -9,6 +9,11 @@ import (
 )
 
 func (m *IPManager) addLoopbackIPNetlink(ip string) error {
+	parsed := net.ParseIP(ip)
+	if parsed == nil || !parsed.IsLoopback() {
+		return fmt.Errorf("refusing to add non-loopback IP %s to lo", ip)
+	}
+
 	lo, err := netlink.LinkByName("lo")
 	if err != nil {
 		return fmt.Errorf("failed to get loopback interface: %w", err)
@@ -28,6 +33,11 @@ func (m *IPManager) addLoopbackIPNetlink(ip string) error {
 }
 
 func (m *IPManager) removeLoopbackIPNetlink(ip string) error {
+	parsed := net.ParseIP(ip)
+	if parsed == nil || !parsed.IsLoopback() {
+		return fmt.Errorf("refusing to remove non-loopback IP %s from lo", ip)
+	}
+
 	lo, err := netlink.LinkByName("lo")
 	if err != nil {
 		return fmt.Errorf("failed to get loopback interface: %w", err)

internal/state/state.go

@@ -92,7 +92,7 @@ func (s *State) SaveCollision(project, ip string) error {
 	if err != nil {
 		return err
 	}
-	return os.WriteFile(s.collisionsPath(), data, 0644)
+	return os.WriteFile(s.collisionsPath(), data, 0600)
 }
 
 func (s *State) LoadCollisions() (map[string]string, error) {

internal/watcher/watcher.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"log/slog"
+	"regexp"
 	"sync"
 	"time"
 
@@ -12,6 +13,8 @@ import (
 	"github.com/docker/docker/api/types/filters"
 )
 
+var validNameRe = regexp.MustCompile(`^[a-zA-Z0-9_-]+$`)
+
 // DockerClient is the subset of the Docker API used by the Watcher.
 // The full client.APIClient satisfies this interface.
 type DockerClient interface {
@@ -50,7 +53,17 @@ func (w *Watcher) projectMutex(project string) *sync.Mutex {
 }
 
 func extractComposeInfo(labels map[string]string) (project, service string) {
-	return labels["com.docker.compose.project"], labels["com.docker.compose.service"]
+	project = labels["com.docker.compose.project"]
+	service = labels["com.docker.compose.service"]
+	if project != "" && (!validNameRe.MatchString(project) || len(project) > 128) {
+		slog.Warn("invalid compose project name in container label", "project", project)
+		return "", ""
+	}
+	if service != "" && (!validNameRe.MatchString(service) || len(service) > 128) {
+		slog.Warn("invalid compose service name in container label", "service", service)
+		return "", ""
+	}
+	return project, service
 }
 
 func backoffSequence() []time.Duration {

internal/watcher/watcher_test.go

@@ -27,6 +27,43 @@ func TestExtractComposeProjectMissing(t *testing.T) {
 	}
 }
 
+func TestExtractComposeInfoRejectsInvalidProject(t *testing.T) {
+	labels := map[string]string{
+		"com.docker.compose.project": `foo"; rm -rf /; #`,
+		"com.docker.compose.service": "postgres",
+	}
+	project, service := extractComposeInfo(labels)
+	if project != "" || service != "" {
+		t.Errorf("expected empty strings for invalid project, got project=%q service=%q", project, service)
+	}
+}
+
+func TestExtractComposeInfoRejectsInvalidService(t *testing.T) {
+	labels := map[string]string{
+		"com.docker.compose.project": "myapp",
+		"com.docker.compose.service": "pg;drop table",
+	}
+	project, service := extractComposeInfo(labels)
+	if project != "" || service != "" {
+		t.Errorf("expected empty strings for invalid service, got project=%q service=%q", project, service)
+	}
+}
+
+func TestExtractComposeInfoRejectsTooLongName(t *testing.T) {
+	long := ""
+	for i := 0; i < 129; i++ {
+		long += "a"
+	}
+	labels := map[string]string{
+		"com.docker.compose.project": long,
+		"com.docker.compose.service": "svc",
+	}
+	project, _ := extractComposeInfo(labels)
+	if project != "" {
+		t.Error("expected empty project for name exceeding 128 chars")
+	}
+}
+
 func TestBackoffSequence(t *testing.T) {
 	durations := backoffSequence()
 	expected := []time.Duration{