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>
This commit is contained in:
Tobias Gesellchen
2026-03-29 19:14:48 +02:00
co-authored by lnx01
parent a8140ad4fd
commit bc1b70b8a5
2 changed files with 129 additions and 0 deletions
+42
View File
@@ -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)
@@ -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)
}
}
}