From bc1b70b8a5e5938b72d260ed3219aee0f4b4ab1c Mon Sep 17 00:00:00 2001
From: Tobias Gesellchen <tobias@gesellix.de>
Date: Sun, 29 Mar 2026 18:50:23 +0200
Subject: [PATCH] Potential fix for code scanning alert no. 88: Uncontrolled
 data used in path expression

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
---
 pkg/service/datastore/datastore.go            | 42 +++++++++
 pkg/service/datastore/safe_identifier_test.go | 87 +++++++++++++++++++
 2 files changed, 129 insertions(+)
 create mode 100644 pkg/service/datastore/safe_identifier_test.go

diff --git a/pkg/service/datastore/datastore.go b/pkg/service/datastore/datastore.go
index 9dea77b..ecf15ea 100644
--- a/pkg/service/datastore/datastore.go
+++ b/pkg/service/datastore/datastore.go
@@ -22,6 +22,36 @@ func exists(path string) bool {
 	return err == nil
 }
 
+// isSafeIdentifier returns true if the given identifier is safe to use
+// as a single path component (for account IDs, device IDs, etc.).
+// It rejects empty strings, path separators, and parent directory references.
+func isSafeIdentifier(id string) bool {
+	if id == "" {
+		return false
+	}
+
+	// Disallow obvious path traversal / multi-component paths.
+	if strings.Contains(id, "/") || strings.Contains(id, "\\") || strings.Contains(id, "..") {
+		return false
+	}
+
+	// Allow a conservative set of characters commonly found in IDs:
+	// letters, digits, underscore, dash, dot, and colon (for MAC-like IDs).
+	for i := 0; i < len(id); i++ {
+		c := id[i]
+		if (c >= 'a' && c <= 'z') ||
+			(c >= 'A' && c <= 'Z') ||
+			(c >= '0' && c <= '9') ||
+			c == '_' || c == '-' || c == '.' || c == ':' {
+			continue
+		}
+
+		return false
+	}
+
+	return true
+}
+
 // DataStore represents the device and configuration storage.
 type DataStore struct {
 	// DataDir is the (possibly relative) base directory for all datastore files.
@@ -706,6 +736,18 @@ func (ds *DataStore) SaveDeviceInfo(account, device string, info *models.Service
 		return fmt.Errorf("device ID/name cannot be empty")
 	}
 
+	if !isSafeIdentifier(device) {
+		return fmt.Errorf("invalid device ID")
+	}
+
+	if account == "" {
+		return fmt.Errorf("account ID cannot be empty")
+	}
+
+	if !isSafeIdentifier(account) {
+		return fmt.Errorf("invalid account ID")
+	}
+
 	// Try to load existing device info to avoid overwriting existing details with empty values.
 	ds.mergeWithExistingDeviceInfo(account, device, info)
 
diff --git a/pkg/service/datastore/safe_identifier_test.go b/pkg/service/datastore/safe_identifier_test.go
new file mode 100644
index 0000000..b551c05
--- /dev/null
+++ b/pkg/service/datastore/safe_identifier_test.go
@@ -0,0 +1,87 @@
+package datastore
+
+import (
+	"os"
+	"testing"
+
+	"github.com/gesellix/bose-soundtouch/pkg/models"
+)
+
+func TestIsSafeIdentifier(t *testing.T) {
+	tests := []struct {
+		id       string
+		expected bool
+	}{
+		{"abc", true},
+		{"ABC", true},
+		{"123", true},
+		{"abc_123", true},
+		{"abc-123", true},
+		{"abc.123", true},
+		{"00:11:22:33:44:55", true},
+		{"", false},
+		{"/", false},
+		{"\\", false},
+		{"..", false},
+		{"../etc/passwd", false},
+		{"/etc/passwd", false},
+		{"a/b", false},
+		{"a\\b", false},
+		{"a..b", false},
+		{"a b", false},
+		{"a!b", false},
+		{"a@b", false},
+		{"a#b", false},
+		{"a$b", false},
+		{"a%b", false},
+		{"a^b", false},
+		{"a&b", false},
+		{"a*b", false},
+		{"a(b", false},
+		{"a)b", false},
+	}
+
+	for _, test := range tests {
+		result := isSafeIdentifier(test.id)
+		if result != test.expected {
+			t.Errorf("isSafeIdentifier(%q) = %v; expected %v", test.id, result, test.expected)
+		}
+	}
+}
+
+func TestSaveDeviceInfo_Validation(t *testing.T) {
+	tmpDir, err := os.MkdirTemp("", "datastore-test")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(tmpDir)
+
+	ds := NewDataStore(tmpDir)
+	info := &models.ServiceDeviceInfo{DeviceID: "dev1"}
+
+	tests := []struct {
+		account string
+		device  string
+		wantErr bool
+		errMsg  string
+	}{
+		{"acc1", "dev1", false, ""},
+		{"", "dev1", true, "account ID cannot be empty"},
+		{"acc1", "", true, "device ID/name cannot be empty"},
+		{"acc/1", "dev1", true, "invalid account ID"},
+		{"acc1", "dev/1", true, "invalid device ID"},
+		{"acc..1", "dev1", true, "invalid account ID"},
+		{"acc1", "dev..1", true, "invalid device ID"},
+	}
+
+	for _, test := range tests {
+		err := ds.SaveDeviceInfo(test.account, test.device, info)
+		if (err != nil) != test.wantErr {
+			t.Errorf("SaveDeviceInfo(%q, %q) error = %v, wantErr %v", test.account, test.device, err, test.wantErr)
+			continue
+		}
+		if test.wantErr && err.Error() != test.errMsg {
+			t.Errorf("SaveDeviceInfo(%q, %q) error message = %q, want %q", test.account, test.device, err.Error(), test.errMsg)
+		}
+	}
+}