fix: remove terminal module and implement concurrent log stream limiting

internal/modules/services/logs.go

@@ -19,6 +19,10 @@ const (
 	maxLogLines     = 10000
 )
 
+// logStreamSem limits concurrent log-follow connections. Each one forks a
+// journalctl or tail subprocess, so too many would exhaust system resources.
+var logStreamSem = make(chan struct{}, 10)
+
 // LogEntry is one log record. For the journal source it is distilled from
 // journalctl's JSON; for the file source only Message is set (the raw line,
 // which usually carries its own embedded timestamp).
@@ -128,6 +132,14 @@ func registerLogs(api huma.API, logFiles map[string][]string) {
 			return
 		}
 
+		select {
+		case logStreamSem <- struct{}{}:
+		default:
+			send.Data(ErrorEvent{Message: "too many concurrent log streams, try again later"})
+			return
+		}
+		defer func() { <-logStreamSem }()
+
 		var cmd string
 		var args []string
 		if in.Source == "file" {

internal/modules/services/logs_test.go

@@ -48,49 +48,74 @@ func TestResolveLogPath(t *testing.T) {
 		"nginx.service": {"/var/log/nginx/access.log", "/var/log/nginx/error.log"},
 	}
 
-	// Allowlisted path resolves whether the caller uses the bare or .service
-	// form, regardless of which form the config key used.
-	for _, unit := range []string{"nginx.service", "nginx"} {
-		if p, err := resolveLogPath(allow, unit, "/var/log/nginx/error.log"); err != nil || p != "/var/log/nginx/error.log" {
-			t.Errorf("allowlisted path for %q: got %q, %v", unit, p, err)
+	t.Run("allowlisted path via bare name", func(t *testing.T) {
+		p, err := resolveLogPath(allow, "nginx", "/var/log/nginx/error.log")
+		if err != nil || p != "/var/log/nginx/error.log" {
+			t.Errorf("got %q, %v", p, err)
 		}
-	}
+	})
+	t.Run("allowlisted path via service suffix", func(t *testing.T) {
+		p, err := resolveLogPath(allow, "nginx.service", "/var/log/nginx/error.log")
+		if err != nil || p != "/var/log/nginx/error.log" {
+			t.Errorf("got %q, %v", p, err)
+		}
+	})
 
-	// Everything else is rejected: empty path, non-listed path (traversal),
-	// listed path but wrong unit, and unit with no allowlist at all.
-	bad := []struct{ unit, path string }{
-		{"nginx.service", ""},
-		{"nginx.service", "/etc/shadow"},
-		{"nginx.service", "/var/log/nginx/access.log/../../../etc/shadow"},
-		{"sshd.service", "/var/log/nginx/error.log"},
-		{"unknown.service", "/var/log/nginx/error.log"},
-	}
-	for _, b := range bad {
-		if _, err := resolveLogPath(allow, b.unit, b.path); err == nil {
-			t.Errorf("resolveLogPath(%q, %q) = nil error, want rejection", b.unit, b.path)
-		}
+	for _, tt := range []struct {
+		name string
+		unit string
+		path string
+	}{
+		{name: "empty path", unit: "nginx.service", path: ""},
+		{name: "non-allowlisted path", unit: "nginx.service", path: "/etc/shadow"},
+		{name: "path traversal", unit: "nginx.service", path: "/var/log/nginx/access.log/../../../etc/shadow"},
+		{name: "wrong unit", unit: "sshd.service", path: "/var/log/nginx/error.log"},
+		{name: "unknown unit", unit: "unknown.service", path: "/var/log/nginx/error.log"},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			if _, err := resolveLogPath(allow, tt.unit, tt.path); err == nil {
+				t.Errorf("resolveLogPath(%q, %q) = nil error, want rejection", tt.unit, tt.path)
+			}
+		})
 	}
 }
 
 func TestJournalUnit(t *testing.T) {
-	cases := map[string]string{
-		"docker.service": "docker",
-		"docker":         "docker",
-		"sshd.service":   "sshd",
-		"foo.socket":     "foo.socket", // only .service is stripped
+	tests := []struct {
+		name string
+		in   string
+		want string
+	}{
+		{name: "docker service", in: "docker.service", want: "docker"},
+		{name: "docker bare", in: "docker", want: "docker"},
+		{name: "sshd service", in: "sshd.service", want: "sshd"},
+		{name: "socket unit", in: "foo.socket", want: "foo.socket"},
 	}
-	for in, want := range cases {
-		if got := journalUnit(in); got != want {
-			t.Errorf("journalUnit(%q) = %q, want %q", in, got, want)
-		}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := journalUnit(tt.in); got != tt.want {
+				t.Errorf("journalUnit(%q) = %q, want %q", tt.in, got, tt.want)
+			}
+		})
 	}
 }
 
 func TestClampLines(t *testing.T) {
-	cases := map[int]int{0: defaultLogLines, -5: defaultLogLines, 50: 50, 999999: maxLogLines}
-	for in, want := range cases {
-		if got := clampLines(in); got != want {
-			t.Errorf("clampLines(%d) = %d, want %d", in, got, want)
-		}
+	tests := []struct {
+		name string
+		in   int
+		want int
+	}{
+		{name: "zero", in: 0, want: defaultLogLines},
+		{name: "negative", in: -5, want: defaultLogLines},
+		{name: "fifty", in: 50, want: 50},
+		{name: "too large", in: 999999, want: maxLogLines},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := clampLines(tt.in); got != tt.want {
+				t.Errorf("clampLines(%d) = %d, want %d", tt.in, got, tt.want)
+			}
+		})
 	}
 }

internal/modules/services/services_test.go

@@ -3,20 +3,33 @@ package services
 import "testing"
 
 func TestValidateUnit(t *testing.T) {
-	valid := []string{"sshd.service", "getty@tty1.service", "foo.bar:baz-1.service", "a_b.timer"}
-	for _, u := range valid {
-		if err := validateUnit(u); err != nil {
-			t.Errorf("validateUnit(%q) = %v, want nil", u, err)
-		}
-	}
-
-	// Empty, flag-injection, and anything with shell/path metacharacters must
-	// be rejected before reaching systemctl.
-	invalid := []string{"", "-rf", "--now", "a b", "foo;rm -rf /", "a/b", "naughty$()", "x|y"}
-	for _, u := range invalid {
-		if err := validateUnit(u); err == nil {
-			t.Errorf("validateUnit(%q) = nil, want error", u)
-		}
+	for _, tt := range []struct {
+		name  string
+		value string
+		valid bool
+	}{
+		{name: "sshd.service", value: "sshd.service", valid: true},
+		{name: "template", value: "getty@tty1.service", valid: true},
+		{name: "dots and colons", value: "foo.bar:baz-1.service", valid: true},
+		{name: "timer", value: "a_b.timer", valid: true},
+		{name: "empty", value: "", valid: false},
+		{name: "flag injection", value: "-rf", valid: false},
+		{name: "double dash", value: "--now", valid: false},
+		{name: "space", value: "a b", valid: false},
+		{name: "shell metachar", value: "foo;rm -rf /", valid: false},
+		{name: "path sep", value: "a/b", valid: false},
+		{name: "subshell", value: "naughty$()", valid: false},
+		{name: "pipe", value: "x|y", valid: false},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			err := validateUnit(tt.value)
+			if tt.valid && err != nil {
+				t.Errorf("validateUnit(%q) = %v, want nil", tt.value, err)
+			}
+			if !tt.valid && err == nil {
+				t.Errorf("validateUnit(%q) = nil, want error", tt.value)
+			}
+		})
 	}
 }
 
@@ -25,16 +38,23 @@ func TestValidateUnit(t *testing.T) {
 // else (including substrings) must not, or unrelated services would also get
 // detached and bypass the synchronous error path.
 func TestIsSelf(t *testing.T) {
-	yes := []string{"nadir", "nadir.service"}
-	for _, u := range yes {
-		if !isSelf(u) {
-			t.Errorf("isSelf(%q) = false, want true", u)
-		}
-	}
-	no := []string{"", "sshd.service", "nadir-something.service", "nadir.timer", "not-nadir.service"}
-	for _, u := range no {
-		if isSelf(u) {
-			t.Errorf("isSelf(%q) = true, want false", u)
-		}
+	for _, tt := range []struct {
+		name  string
+		value string
+		want  bool
+	}{
+		{name: "bare name", value: "nadir", want: true},
+		{name: "with service suffix", value: "nadir.service", want: true},
+		{name: "empty", value: "", want: false},
+		{name: "other service", value: "sshd.service", want: false},
+		{name: "prefix substring", value: "nadir-something.service", want: false},
+		{name: "wrong suffix", value: "nadir.timer", want: false},
+		{name: "suffix mismatch", value: "not-nadir.service", want: false},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := isSelf(tt.value); got != tt.want {
+				t.Errorf("isSelf(%q) = %v, want %v", tt.value, got, tt.want)
+			}
+		})
 	}
 }