Skip to content

Commit 81ada81

Browse files
sarg3ntclaude
andcommitted
fix: address Copilot PR review comments
- toast.js: fix enforceMaxToasts() infinite loop by splicing the toast from activeToasts synchronously before calling dismiss(), so the array length decreases each iteration - apt_runner.go: guard GetUpdateLog against panic when id < 8 chars; return an error instead of slicing out of bounds - Remove toast.test.html from static/ (publicly served); test harness should not ship in production builds - Add validateSnapshotID() to reject snapshot IDs containing path separators or other unsafe characters, preventing path traversal in updates.go, apt_runner.go, and pm_apt.go - Restore systemd hardening: add ProtectHome=read-only, PrivateTmp=true, and ProtectKernelTunables=true while keeping NoNewPrivileges=false and omitting ProtectSystem=strict (agent needs write access to /etc and /var) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent a2dc45d commit 81ada81

6 files changed

Lines changed: 37 additions & 405 deletions

File tree

gearbox-agent/deploy/gearbox-agent.service

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,14 @@ Restart=always
2626
RestartSec=5
2727

2828
# Security hardening
29-
# Note: ProtectSystem is intentionally not set to "strict" because gearbox-agent
30-
# needs broad system access for package management (apt install/upgrade),
31-
# service control, HAProxy config, certificate management, and system reboots.
29+
# Note: ProtectSystem=strict cannot be used because gearbox-agent needs write
30+
# access to /etc (HAProxy config, certificates) and /var (package management,
31+
# data dir). ProtectHome=read-only is set since the agent runs as root and only
32+
# needs read access to home directories.
3233
NoNewPrivileges=false
33-
ProtectHome=no
34+
ProtectHome=read-only
35+
PrivateTmp=true
36+
ProtectKernelTunables=true
3437

3538
# Logging
3639
StandardOutput=journal

gearbox-agent/internal/gears/updates/apt_runner.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -397,6 +397,9 @@ func (r *AptRunner) failOperation(op *AptOperation, errorMsg string) {
397397

398398
// StartRestore begins a streaming snapshot restore operation.
399399
func (r *AptRunner) StartRestore(snapshotID string) (string, error) {
400+
if err := validateSnapshotID(snapshotID); err != nil {
401+
return "", err
402+
}
400403
operationID := uuid.New().String()
401404
ctx, cancel := context.WithCancel(context.Background())
402405

@@ -674,6 +677,9 @@ func (r *AptRunner) ListUpdateLogs(limit int) ([]UpdateLog, error) {
674677

675678
// GetUpdateLog returns a specific update log with full output.
676679
func (r *AptRunner) GetUpdateLog(id string) (*UpdateLog, error) {
680+
if len(id) < 8 {
681+
return nil, fmt.Errorf("invalid log ID")
682+
}
677683
entries, err := os.ReadDir(updateLogsDir)
678684
if err != nil {
679685
return nil, fmt.Errorf("failed to read update logs directory: %w", err)

gearbox-agent/internal/gears/updates/pm_apt.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -482,6 +482,9 @@ func (a *aptPackageManager) CreateSnapshot(reason string) (*AptSnapshot, error)
482482
}
483483

484484
func (a *aptPackageManager) RestoreSnapshot(snapshotID string) error {
485+
if err := validateSnapshotID(snapshotID); err != nil {
486+
return err
487+
}
485488
snapshotDir := "/var/lib/gearbox-agent/snapshots"
486489
snapshotFile := fmt.Sprintf("%s/%s.selections", snapshotDir, snapshotID)
487490

gearbox-agent/internal/gears/updates/updates.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,17 @@ func (c *UpdatesCollector) CancelReboot() error {
300300

301301
// --- Snapshot Management ---
302302

303+
// validSnapshotID matches only safe snapshot IDs (timestamp + hex suffix, no path separators).
304+
var validSnapshotID = regexp.MustCompile(`^[A-Za-z0-9_-]+$`)
305+
306+
// validateSnapshotID returns an error if id is empty or contains path-unsafe characters.
307+
func validateSnapshotID(id string) error {
308+
if id == "" || !validSnapshotID.MatchString(id) {
309+
return fmt.Errorf("invalid snapshot ID")
310+
}
311+
return nil
312+
}
313+
303314
// CreateSnapshot creates a package snapshot for rollback capability.
304315
// The snapshot format depends on the active package manager.
305316
func (c *UpdatesCollector) CreateSnapshot(reason string) (*AptSnapshot, error) {
@@ -421,6 +432,9 @@ func computeDowngrades(versionsFile string) []string {
421432

422433
// DeleteSnapshot removes a snapshot.
423434
func (c *UpdatesCollector) DeleteSnapshot(snapshotID string) error {
435+
if err := validateSnapshotID(snapshotID); err != nil {
436+
return err
437+
}
424438
snapshotDir := "/var/lib/gearbox-agent/snapshots"
425439

426440
selectionsPath := fmt.Sprintf("%s/%s.selections", snapshotDir, snapshotID)
@@ -461,6 +475,9 @@ type SnapshotPreview struct {
461475

462476
// PreviewRestore computes what changes restoring a snapshot would make without applying them.
463477
func (c *UpdatesCollector) PreviewRestore(snapshotID string) (*SnapshotPreview, error) {
478+
if err := validateSnapshotID(snapshotID); err != nil {
479+
return nil, err
480+
}
464481
snapshotDir := "/var/lib/gearbox-agent/snapshots"
465482
selectionsFile := fmt.Sprintf("%s/%s.selections", snapshotDir, snapshotID)
466483
versionsFile := fmt.Sprintf("%s/%s.versions", snapshotDir, snapshotID)

gearbox/static/js/utils/toast.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -319,8 +319,10 @@
319319

320320
function enforceMaxToasts() {
321321
while (activeToasts.length > DEFAULTS.maxToasts) {
322-
// Remove the oldest toast (first in array)
323-
dismiss(activeToasts[0]);
322+
// Splice synchronously so the length decreases each iteration,
323+
// then dismiss (which animates out and does a no-op splice later).
324+
var oldest = activeToasts.splice(0, 1)[0];
325+
dismiss(oldest);
324326
}
325327
}
326328

0 commit comments

Comments
 (0)