From 0e9445af4748605f3274b64c60b704483cede5aa Mon Sep 17 00:00:00 2001 From: Tobias Gesellchen Date: Mon, 25 May 2026 10:50:33 +0200 Subject: [PATCH] =?UTF-8?q?fix(security):=20sec7=20=E2=80=94=20log-injecti?= =?UTF-8?q?on=20sweep,=20sanitizeErr=20helper?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ~30 remaining go/log-injection alerts share a common pattern: other positional args in a log call are wrapped in sanitizeLog() but the trailing 'err' value (via "%v") is not. CodeQL traces taint through error chains back to the log.Printf call site itself. Add sanitizeErr(err error) string to every affected package's logutil.go (strips newlines from err.Error(), returns "" when nil). Three packages had no logutil.go yet; new files added for cmd/soundtouch-cli, cmd/websocket-demo, and examples. Call-site changes (replace "%v, err" with "%s, sanitizeErr(err)" and wrap any other unsanitised args in sanitizeLog): pkg/client: - websocket.go:42 DefaultLogger.Printf now pre-formats and sanitises the entire message (all variadic args sanitised) - websocket.go:445 err → sanitizeErr(err) pkg/service/handlers: - handlers_account_mgmt.go:44 err - handlers_bmx_tunein.go:324,336 err (stationID already sanitised) - handlers_marge.go:288,510 err (deviceID/account already done) - handlers_mgmt.go:409,436,720 err - handlers_setup.go:1345 session + err - server.go:500 bind - server.go:504,863,944,1029, err (deviceIP/accountID already done) 1164,1174 pkg/service/marge: - marge.go:1469,1923 saveErr / err pkg/service/setup: - setup.go:1417,2316,2462 fmt.Printf — deviceIP / hostsContent / ip pkg/service/stockholm: - proxy.go:117 effectiveTarget.String() + err pkg/service/zeroconf: - zeroconf.go:312 err pkg/service/proxy: - recorder.go:403 err (task.path already sanitised) pkg/service/datastore: - datastore.go:940 werr (device already sanitised) pkg/discovery: - dns.go:72 strings.Join(derived) - dns.go:503 d.upstreamDNS (fmt.Sprint of []string) cmd/soundtouch-cli: - cmd_events.go:571 VerboseLogger.Printf — pre-format + sanitise - common.go:335 PrintError message cmd/websocket-demo: - main.go:576 VerboseLogger.Printf — pre-format + sanitise examples: - recording-filename-demo.go:79 err Co-Authored-By: Claude Sonnet 4.6 --- cmd/soundtouch-cli/cmd_events.go | 2 +- cmd/soundtouch-cli/common.go | 2 +- cmd/soundtouch-cli/logutil.go | 24 +++++++++++++++++++ cmd/websocket-demo/logutil.go | 24 +++++++++++++++++++ cmd/websocket-demo/main.go | 2 +- examples/logutil.go | 24 +++++++++++++++++++ examples/recording-filename-demo.go | 2 +- pkg/client/logutil.go | 11 +++++++++ pkg/client/websocket.go | 4 ++-- pkg/discovery/dns.go | 4 ++-- pkg/discovery/logger.go | 11 +++++++++ pkg/service/datastore/datastore.go | 2 +- pkg/service/datastore/logutil.go | 11 +++++++++ pkg/service/handlers/handlers_account_mgmt.go | 2 +- pkg/service/handlers/handlers_bmx_tunein.go | 4 ++-- pkg/service/handlers/handlers_marge.go | 4 ++-- pkg/service/handlers/handlers_mgmt.go | 6 ++--- pkg/service/handlers/handlers_setup.go | 2 +- pkg/service/handlers/logutil.go | 11 +++++++++ pkg/service/handlers/server.go | 18 +++++++------- pkg/service/marge/logutil.go | 11 +++++++++ pkg/service/marge/marge.go | 6 ++--- pkg/service/proxy/logutil.go | 11 +++++++++ pkg/service/proxy/recorder.go | 2 +- pkg/service/setup/logutil.go | 11 +++++++++ pkg/service/setup/setup.go | 6 ++--- pkg/service/stockholm/logutil.go | 11 +++++++++ pkg/service/stockholm/proxy.go | 2 +- pkg/service/zeroconf/logutil.go | 11 +++++++++ pkg/service/zeroconf/zeroconf.go | 2 +- 30 files changed, 207 insertions(+), 36 deletions(-) create mode 100644 cmd/soundtouch-cli/logutil.go create mode 100644 cmd/websocket-demo/logutil.go create mode 100644 examples/logutil.go diff --git a/cmd/soundtouch-cli/cmd_events.go b/cmd/soundtouch-cli/cmd_events.go index baedb9b..d51edde 100644 --- a/cmd/soundtouch-cli/cmd_events.go +++ b/cmd/soundtouch-cli/cmd_events.go @@ -568,7 +568,7 @@ type VerboseLogger struct{} func (v *VerboseLogger) Printf(format string, args ...interface{}) { timestamp := time.Now().Format("15:04:05") - fmt.Printf("[%s] [WebSocket] %s\n", timestamp, fmt.Sprintf(format, args...)) + fmt.Printf("[%s] [WebSocket] %s\n", timestamp, sanitizeLog(fmt.Sprintf(format, args...))) } type SilentLogger struct{} diff --git a/cmd/soundtouch-cli/common.go b/cmd/soundtouch-cli/common.go index 9f40708..850d4a0 100644 --- a/cmd/soundtouch-cli/common.go +++ b/cmd/soundtouch-cli/common.go @@ -332,7 +332,7 @@ func PrintSuccess(message string) { // PrintError prints a standard error message func PrintError(message string) { - fmt.Printf("✗ %s\n", message) + fmt.Printf("✗ %s\n", sanitizeLog(message)) } // PrintWarning prints a standard warning message diff --git a/cmd/soundtouch-cli/logutil.go b/cmd/soundtouch-cli/logutil.go new file mode 100644 index 0000000..5d6f751 --- /dev/null +++ b/cmd/soundtouch-cli/logutil.go @@ -0,0 +1,24 @@ +package main + +import "strings" + +// sanitizeLog strips newline characters from s to prevent log-injection +// (CodeQL go/log-injection). Values from speakers, HTTP requests, and +// external APIs may contain attacker-controlled newlines. +func sanitizeLog(s string) string { + s = strings.ReplaceAll(s, "\n", `\n`) + s = strings.ReplaceAll(s, "\r", `\r`) + + return s +} + +// sanitizeErr returns err.Error() with newlines stripped to prevent log +// injection when error messages contain user-controlled values. Use in +// place of bare "%v, err" in log calls where err may wrap external data. +func sanitizeErr(err error) string { + if err == nil { + return "" + } + + return sanitizeLog(err.Error()) +} diff --git a/cmd/websocket-demo/logutil.go b/cmd/websocket-demo/logutil.go new file mode 100644 index 0000000..5d6f751 --- /dev/null +++ b/cmd/websocket-demo/logutil.go @@ -0,0 +1,24 @@ +package main + +import "strings" + +// sanitizeLog strips newline characters from s to prevent log-injection +// (CodeQL go/log-injection). Values from speakers, HTTP requests, and +// external APIs may contain attacker-controlled newlines. +func sanitizeLog(s string) string { + s = strings.ReplaceAll(s, "\n", `\n`) + s = strings.ReplaceAll(s, "\r", `\r`) + + return s +} + +// sanitizeErr returns err.Error() with newlines stripped to prevent log +// injection when error messages contain user-controlled values. Use in +// place of bare "%v, err" in log calls where err may wrap external data. +func sanitizeErr(err error) string { + if err == nil { + return "" + } + + return sanitizeLog(err.Error()) +} diff --git a/cmd/websocket-demo/main.go b/cmd/websocket-demo/main.go index 3804e08..4623a77 100644 --- a/cmd/websocket-demo/main.go +++ b/cmd/websocket-demo/main.go @@ -573,7 +573,7 @@ type VerboseLogger struct{} func (v *VerboseLogger) Printf(format string, args ...interface{}) { timestamp := time.Now().Format("15:04:05") - fmt.Printf("[%s] [WebSocket] %s\n", timestamp, fmt.Sprintf(format, args...)) + fmt.Printf("[%s] [WebSocket] %s\n", timestamp, sanitizeLog(fmt.Sprintf(format, args...))) } // SilentLogger provides no-op WebSocket logging diff --git a/examples/logutil.go b/examples/logutil.go new file mode 100644 index 0000000..5d6f751 --- /dev/null +++ b/examples/logutil.go @@ -0,0 +1,24 @@ +package main + +import "strings" + +// sanitizeLog strips newline characters from s to prevent log-injection +// (CodeQL go/log-injection). Values from speakers, HTTP requests, and +// external APIs may contain attacker-controlled newlines. +func sanitizeLog(s string) string { + s = strings.ReplaceAll(s, "\n", `\n`) + s = strings.ReplaceAll(s, "\r", `\r`) + + return s +} + +// sanitizeErr returns err.Error() with newlines stripped to prevent log +// injection when error messages contain user-controlled values. Use in +// place of bare "%v, err" in log calls where err may wrap external data. +func sanitizeErr(err error) string { + if err == nil { + return "" + } + + return sanitizeLog(err.Error()) +} diff --git a/examples/recording-filename-demo.go b/examples/recording-filename-demo.go index 859ad46..1a3f624 100644 --- a/examples/recording-filename-demo.go +++ b/examples/recording-filename-demo.go @@ -76,7 +76,7 @@ func main() { // Record the interaction err = recorder.Record(req.category, httpReq, httpRes) if err != nil { - log.Printf("Failed to record interaction: %v", err) + log.Printf("Failed to record interaction: %s", sanitizeErr(err)) continue } diff --git a/pkg/client/logutil.go b/pkg/client/logutil.go index 7677c98..b6529fe 100644 --- a/pkg/client/logutil.go +++ b/pkg/client/logutil.go @@ -11,3 +11,14 @@ func sanitizeLog(s string) string { return s } + +// sanitizeErr returns err.Error() with newlines stripped to prevent log +// injection when error messages contain user-controlled values. Use in +// place of bare "%v, err" in log calls where err may wrap external data. +func sanitizeErr(err error) string { + if err == nil { + return "" + } + + return sanitizeLog(err.Error()) +} diff --git a/pkg/client/websocket.go b/pkg/client/websocket.go index 046dc7e..cd6c25c 100644 --- a/pkg/client/websocket.go +++ b/pkg/client/websocket.go @@ -39,7 +39,7 @@ type DefaultLogger struct{} // Printf implements the Logger interface by printing formatted messages with a WebSocket prefix. func (d DefaultLogger) Printf(format string, v ...interface{}) { - log.Printf("[WebSocket] "+format, v...) + log.Printf("[WebSocket] %s", sanitizeLog(fmt.Sprintf(format, v...))) } // WebSocketConfig holds configuration for WebSocket client @@ -442,7 +442,7 @@ func (ws *WebSocketClient) handleSpecialMessage(data []byte) { ws.fireRawMessage(data, err) if err != nil { - ws.logger.Printf("Unknown special message type: %v", err) + ws.logger.Printf("Unknown special message type: %s", sanitizeErr(err)) ws.logger.Printf("Raw message: %s", sanitizeLog(string(data))) return diff --git a/pkg/discovery/dns.go b/pkg/discovery/dns.go index 510ce5e..8518f41 100644 --- a/pkg/discovery/dns.go +++ b/pkg/discovery/dns.go @@ -69,7 +69,7 @@ type DiscoveredHost struct { func NewDNSDiscovery(upstreamDNS []string, serviceIP, serverURL string) *DNSDiscovery { derived := DeriveOAuthHostnames(serverURL) if len(derived) > 0 { - log.Printf("[DNS] Auto-hijacking OAuth subdomains derived from serverURL %q: %s", sanitizeLog(serverURL), strings.Join(derived, ", ")) + log.Printf("[DNS] Auto-hijacking OAuth subdomains derived from serverURL %q: %s", sanitizeLog(serverURL), sanitizeLog(strings.Join(derived, ", "))) } return &DNSDiscovery{ @@ -500,7 +500,7 @@ func (d *DNSDiscovery) Start(addr string) error { } }() - log.Printf("[DNS] Discovery servers starting on %s (upstream: %s, intercept IP: %s)", sanitizeLog(addr), d.upstreamDNS, sanitizeLog(d.serviceIP)) + log.Printf("[DNS] Discovery servers starting on %s (upstream: %s, intercept IP: %s)", sanitizeLog(addr), sanitizeLog(fmt.Sprint(d.upstreamDNS)), sanitizeLog(d.serviceIP)) // Wait for first error return <-errChan diff --git a/pkg/discovery/logger.go b/pkg/discovery/logger.go index 83530f4..0f8d797 100644 --- a/pkg/discovery/logger.go +++ b/pkg/discovery/logger.go @@ -61,3 +61,14 @@ func remoteAddrString(w interface{ RemoteAddr() net.Addr }) string { return "" } + +// sanitizeErr returns err.Error() with newlines stripped to prevent log +// injection when error messages contain user-controlled values. Use in +// place of bare "%v, err" in log calls where err may wrap external data. +func sanitizeErr(err error) string { + if err == nil { + return "" + } + + return sanitizeLog(err.Error()) +} diff --git a/pkg/service/datastore/datastore.go b/pkg/service/datastore/datastore.go index 8771dbc..5ad30f9 100644 --- a/pkg/service/datastore/datastore.go +++ b/pkg/service/datastore/datastore.go @@ -937,7 +937,7 @@ func (ds *DataStore) GetPresets(account, device string) ([]models.ServicePreset, log.Printf("[Datastore] Presets.xml for device %s used legacy format; rewriting in canonical form", sanitizeLog(device)) if werr := ds.SavePresets(account, device, presets); werr != nil { - log.Printf("[Datastore] failed to rewrite normalised Presets.xml for device %s: %v", sanitizeLog(device), werr) + log.Printf("[Datastore] failed to rewrite normalised Presets.xml for device %s: %s", sanitizeLog(device), sanitizeErr(werr)) } } diff --git a/pkg/service/datastore/logutil.go b/pkg/service/datastore/logutil.go index 5661eb9..44c49b7 100644 --- a/pkg/service/datastore/logutil.go +++ b/pkg/service/datastore/logutil.go @@ -11,3 +11,14 @@ func sanitizeLog(s string) string { return s } + +// sanitizeErr returns err.Error() with newlines stripped to prevent log +// injection when error messages contain user-controlled values. Use in +// place of bare "%v, err" in log calls where err may wrap external data. +func sanitizeErr(err error) string { + if err == nil { + return "" + } + + return sanitizeLog(err.Error()) +} diff --git a/pkg/service/handlers/handlers_account_mgmt.go b/pkg/service/handlers/handlers_account_mgmt.go index d444821..2e79fd1 100644 --- a/pkg/service/handlers/handlers_account_mgmt.go +++ b/pkg/service/handlers/handlers_account_mgmt.go @@ -41,7 +41,7 @@ func (s *Server) HandleMgmtAccountDetails(w http.ResponseWriter, r *http.Request // 1. Get account info accountInfo, err := s.ds.GetAccountInfo(accountID) if err != nil { - log.Printf("[Mgmt] Failed to get account info for %s: %v", sanitizeLog(accountID), err) + log.Printf("[Mgmt] Failed to get account info for %s: %s", sanitizeLog(accountID), sanitizeErr(err)) accountInfo = &models.ServiceAccountInfo{AccountID: accountID} } diff --git a/pkg/service/handlers/handlers_bmx_tunein.go b/pkg/service/handlers/handlers_bmx_tunein.go index cc32aae..c9e0965 100644 --- a/pkg/service/handlers/handlers_bmx_tunein.go +++ b/pkg/service/handlers/handlers_bmx_tunein.go @@ -321,7 +321,7 @@ func (s *Server) HandleTuneInSearchNext(w http.ResponseWriter, r *http.Request) func (s *Server) HandleTuneInFavorite(w http.ResponseWriter, r *http.Request) { stationID := chi.URLParam(r, "stationID") if err := s.ds.SaveTuneInFavorite(stationID); err != nil { - log.Printf("Failed to persist TuneIn favorite %s: %v", sanitizeLog(stationID), err) + log.Printf("Failed to persist TuneIn favorite %s: %s", sanitizeLog(stationID), sanitizeErr(err)) } w.Header().Set("Content-Type", "application/json") @@ -333,7 +333,7 @@ func (s *Server) HandleTuneInFavorite(w http.ResponseWriter, r *http.Request) { func (s *Server) HandleTuneInDeleteFavorite(w http.ResponseWriter, r *http.Request) { stationID := chi.URLParam(r, "stationID") if err := s.ds.DeleteTuneInFavorite(stationID); err != nil { - log.Printf("Failed to delete TuneIn favorite %s: %v", sanitizeLog(stationID), err) + log.Printf("Failed to delete TuneIn favorite %s: %s", sanitizeLog(stationID), sanitizeErr(err)) } w.Header().Set("Content-Type", "application/json") diff --git a/pkg/service/handlers/handlers_marge.go b/pkg/service/handlers/handlers_marge.go index 075db9e..015b3a1 100644 --- a/pkg/service/handlers/handlers_marge.go +++ b/pkg/service/handlers/handlers_marge.go @@ -285,7 +285,7 @@ func (s *Server) HandleMargePowerOn(w http.ResponseWriter, r *http.Request) { } if err := s.ds.SaveDeviceInfo(accountID, deviceID, info); err != nil { - log.Printf("[Marge] Failed to save device info for %s: %v", sanitizeLog(deviceID), err) + log.Printf("[Marge] Failed to save device info for %s: %s", sanitizeLog(deviceID), sanitizeErr(err)) } } @@ -507,7 +507,7 @@ func (s *Server) HandleMargeUpdatePreset(w http.ResponseWriter, r *http.Request) data, err := marge.UpdatePreset(s.ds, account, device, presetNumber, body) if err != nil { - log.Printf("[Marge] UpdatePreset failed for account=%s, device=%s, preset=%d: %v", sanitizeLog(account), sanitizeLog(device), presetNumber, err) + log.Printf("[Marge] UpdatePreset failed for account=%s, device=%s, preset=%d: %s", sanitizeLog(account), sanitizeLog(device), presetNumber, sanitizeErr(err)) http.Error(w, err.Error(), http.StatusInternalServerError) return diff --git a/pkg/service/handlers/handlers_mgmt.go b/pkg/service/handlers/handlers_mgmt.go index 5420953..11430fe 100644 --- a/pkg/service/handlers/handlers_mgmt.go +++ b/pkg/service/handlers/handlers_mgmt.go @@ -406,7 +406,7 @@ func (s *Server) HandleMgmtSpotifyEntity(w http.ResponseWriter, r *http.Request) name, imageURL, err := svc.ResolveEntity(request.URI) if err != nil { - log.Printf("[Mgmt] Spotify entity resolve error: %v", err) + log.Printf("[Mgmt] Spotify entity resolve error: %s", sanitizeErr(err)) http.Error(w, `{"error":"entity resolution failed"}`, http.StatusInternalServerError) return @@ -433,7 +433,7 @@ func (s *Server) HandleMgmtPrimeDevice(w http.ResponseWriter, r *http.Request) { deviceIP, err := s.resolveDeviceIDToIP(deviceID) if err != nil { - log.Printf("[Mgmt] Prime failed: %v", err) + log.Printf("[Mgmt] Prime failed: %s", sanitizeErr(err)) http.Error(w, fmt.Sprintf(`{"error":"%v"}`, err), http.StatusNotFound) return @@ -717,7 +717,7 @@ func (s *Server) HandleMgmtPrimeDeviceAmazon(w http.ResponseWriter, r *http.Requ deviceIP, err := s.resolveDeviceIDToIP(deviceID) if err != nil { - log.Printf("[Mgmt] Amazon prime failed: %v", err) + log.Printf("[Mgmt] Amazon prime failed: %s", sanitizeErr(err)) http.Error(w, fmt.Sprintf(`{"error":"%v"}`, err), http.StatusNotFound) return diff --git a/pkg/service/handlers/handlers_setup.go b/pkg/service/handlers/handlers_setup.go index d50ec09..e52226a 100644 --- a/pkg/service/handlers/handlers_setup.go +++ b/pkg/service/handlers/handlers_setup.go @@ -1342,7 +1342,7 @@ func (s *Server) HandleDownloadSession(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Disposition", fmt.Sprintf("attachment; filename=\"%s.tar.gz\"", session)) if err := s.recorder.ArchiveSession(session, w); err != nil { - log.Printf("Error archiving session %s: %v", session, err) + log.Printf("Error archiving session %s: %s", sanitizeLog(session), sanitizeErr(err)) // Since we already set headers, if we have an error here it might be partially written. // But for now, simple error handling. return diff --git a/pkg/service/handlers/logutil.go b/pkg/service/handlers/logutil.go index d7e8f12..3bd88bd 100644 --- a/pkg/service/handlers/logutil.go +++ b/pkg/service/handlers/logutil.go @@ -11,3 +11,14 @@ func sanitizeLog(s string) string { return s } + +// sanitizeErr returns err.Error() with newlines stripped to prevent log +// injection when error messages contain user-controlled values. Use in +// place of bare "%v, err" in log calls where err may wrap external data. +func sanitizeErr(err error) string { + if err == nil { + return "" + } + + return sanitizeLog(err.Error()) +} diff --git a/pkg/service/handlers/server.go b/pkg/service/handlers/server.go index 91d4beb..7e00fc7 100644 --- a/pkg/service/handlers/server.go +++ b/pkg/service/handlers/server.go @@ -497,11 +497,11 @@ func (s *Server) resolveServerURLIP(serverURL string) (string, error) { } func (s *Server) startDNSDiscovery(bind string, upstreamList []string) { - log.Printf("[DNS] Starting DNS discovery server on %s", bind) + log.Printf("[DNS] Starting DNS discovery server on %s", sanitizeLog(bind)) serviceIP, err := s.resolveServerURLIP(s.serverURL) if err != nil { - log.Printf("[DNS] Cannot start DNS discovery server: %v", err) + log.Printf("[DNS] Cannot start DNS discovery server: %s", sanitizeErr(err)) s.dnsEnabled = false @@ -860,7 +860,7 @@ func (s *Server) PrimeDeviceWithSpotify(deviceIP string) { if errors.Is(err, spotify.ErrAddUserNoOp) { log.Printf("[Spotify Watchdog] Successfully primed %s (ZeroConf addUser was an expected no-op)", sanitizeLog(deviceIP)) } else { - log.Printf("[Spotify Watchdog] Failed to prime %s: %v", sanitizeLog(deviceIP), err) + log.Printf("[Spotify Watchdog] Failed to prime %s: %s", sanitizeLog(deviceIP), sanitizeErr(err)) } } else { log.Printf("[Spotify Watchdog] Successfully primed %s", sanitizeLog(deviceIP)) @@ -941,7 +941,7 @@ func (s *Server) resolvePairedAccount(deviceIP, host string) (accountID, deviceI deviceID = info.DeviceID } } else { - log.Printf("[Spotify Watchdog] live /info lookup for %s failed: %v (falling back to datastore account=%q)", sanitizeLog(deviceIP), err, sanitizeLog(accountID)) + log.Printf("[Spotify Watchdog] live /info lookup for %s failed: %s (falling back to datastore account=%q)", sanitizeLog(deviceIP), sanitizeErr(err), sanitizeLog(accountID)) } } @@ -1026,7 +1026,7 @@ func (s *Server) handleDiscoveredDevice(d models.DiscoveredDevice) { // 1. Always fetch live device info from /info endpoint as the authoritative source liveInfo, err := s.sm.GetLiveDeviceInfo(d.Host) if err != nil { - log.Printf("Failed to fetch live device info for %s at %s: %v", sanitizeLog(d.Name), sanitizeLog(d.Host), err) + log.Printf("Failed to fetch live device info for %s at %s: %s", sanitizeLog(d.Name), sanitizeLog(d.Host), sanitizeErr(err)) // Fallback to discovery info if /info is not available s.handleDiscoveredDeviceFallback(d) @@ -1095,7 +1095,7 @@ func (s *Server) handleDiscoveredDevice(d models.DiscoveredDevice) { // 7. Save the updated device info if err := s.ds.SaveDeviceInfo(accountID, deviceID, info); err != nil { - log.Printf("Failed to save device info for %s: %v", sanitizeLog(deviceID), err) + log.Printf("Failed to save device info for %s: %s", sanitizeLog(deviceID), sanitizeErr(err)) return } @@ -1118,7 +1118,7 @@ func (s *Server) handleDiscoveredDevice(d models.DiscoveredDevice) { log.Printf("Creating default Sources.xml for device %s", sanitizeLog(deviceID)) if err := s.ds.SaveConfiguredSources(accountID, deviceID, sources); err != nil { - log.Printf("Failed to save default sources for %s: %v", sanitizeLog(deviceID), err) + log.Printf("Failed to save default sources for %s: %s", sanitizeLog(deviceID), sanitizeErr(err)) } } } @@ -1161,7 +1161,7 @@ func (s *Server) handleDiscoveredDeviceFallback(d models.DiscoveredDevice) { } if err := s.ds.SaveDeviceInfo(accountID, deviceID, info); err != nil { - log.Printf("Failed to save device info for %s: %v", sanitizeLog(deviceID), err) + log.Printf("Failed to save device info for %s: %s", sanitizeLog(deviceID), sanitizeErr(err)) return } @@ -1171,7 +1171,7 @@ func (s *Server) handleDiscoveredDeviceFallback(d models.DiscoveredDevice) { log.Printf("Creating default Sources.xml for device %s (fallback)", sanitizeLog(deviceID)) if err := s.ds.SaveConfiguredSources(accountID, deviceID, sources); err != nil { - log.Printf("Failed to save default sources for %s: %v", sanitizeLog(deviceID), err) + log.Printf("Failed to save default sources for %s: %s", sanitizeLog(deviceID), sanitizeErr(err)) } } } diff --git a/pkg/service/marge/logutil.go b/pkg/service/marge/logutil.go index 8868cf4..392e740 100644 --- a/pkg/service/marge/logutil.go +++ b/pkg/service/marge/logutil.go @@ -11,3 +11,14 @@ func sanitizeLog(s string) string { return s } + +// sanitizeErr returns err.Error() with newlines stripped to prevent log +// injection when error messages contain user-controlled values. Use in +// place of bare "%v, err" in log calls where err may wrap external data. +func sanitizeErr(err error) string { + if err == nil { + return "" + } + + return sanitizeLog(err.Error()) +} diff --git a/pkg/service/marge/marge.go b/pkg/service/marge/marge.go index b59855e..cbbbd3e 100644 --- a/pkg/service/marge/marge.go +++ b/pkg/service/marge/marge.go @@ -1465,8 +1465,8 @@ func resolvePresetSource(ds *datastore.DataStore, account, device string, source sources = append(sources, canonical) if saveErr := ds.SaveConfiguredSources(account, device, sources); saveErr != nil { - log.Printf("[Marge] UpdatePreset(preset=%d): SaveConfiguredSources after auto-add failed: %v — the preset will land but the source may not survive a service restart", - presetNumber, saveErr) + log.Printf("[Marge] UpdatePreset(preset=%d): SaveConfiguredSources after auto-add failed: %s — the preset will land but the source may not survive a service restart", + presetNumber, sanitizeErr(saveErr)) } return &sources[len(sources)-1], sources @@ -1920,7 +1920,7 @@ func persistLearnedSource(ds *datastore.DataStore, account, device string, sourc } if err := ds.SaveConfiguredSources(account, device, updatedSources); err != nil { - log.Printf("[MARGE_ERR] Failed to persist learned source for %s: %v", sanitizeLog(device), err) + log.Printf("[MARGE_ERR] Failed to persist learned source for %s: %s", sanitizeLog(device), sanitizeErr(err)) } } diff --git a/pkg/service/proxy/logutil.go b/pkg/service/proxy/logutil.go index f021336..2486622 100644 --- a/pkg/service/proxy/logutil.go +++ b/pkg/service/proxy/logutil.go @@ -11,3 +11,14 @@ func sanitizeLog(s string) string { return s } + +// sanitizeErr returns err.Error() with newlines stripped to prevent log +// injection when error messages contain user-controlled values. Use in +// place of bare "%v, err" in log calls where err may wrap external data. +func sanitizeErr(err error) string { + if err == nil { + return "" + } + + return sanitizeLog(err.Error()) +} diff --git a/pkg/service/proxy/recorder.go b/pkg/service/proxy/recorder.go index d29ea31..f6458da 100644 --- a/pkg/service/proxy/recorder.go +++ b/pkg/service/proxy/recorder.go @@ -400,7 +400,7 @@ func (r *Recorder) save(task recordingTask) { } if err := r.rootWriteFile(task.path, buf.Bytes(), 0644); err != nil { - log.Printf("failed to write recording to %s: %v", sanitizeLog(task.path), err) + log.Printf("failed to write recording to %s: %s", sanitizeLog(task.path), sanitizeErr(err)) } _ = r.updateEnvFile(task.replacements) diff --git a/pkg/service/setup/logutil.go b/pkg/service/setup/logutil.go index c6c5dc5..b43de6e 100644 --- a/pkg/service/setup/logutil.go +++ b/pkg/service/setup/logutil.go @@ -11,3 +11,14 @@ func sanitizeLog(s string) string { return s } + +// sanitizeErr returns err.Error() with newlines stripped to prevent log +// injection when error messages contain user-controlled values. Use in +// place of bare "%v, err" in log calls where err may wrap external data. +func sanitizeErr(err error) string { + if err == nil { + return "" + } + + return sanitizeLog(err.Error()) +} diff --git a/pkg/service/setup/setup.go b/pkg/service/setup/setup.go index 14c284f..3a01245 100644 --- a/pkg/service/setup/setup.go +++ b/pkg/service/setup/setup.go @@ -1414,7 +1414,7 @@ func (m *Manager) migrateViaHosts(deviceIP, targetURL string) (string, error) { logs += "Verified /etc/hosts on device\n" - fmt.Printf("Updated /etc/hosts on %s:\n%s\n", deviceIP, hostsContent) + fmt.Printf("Updated /etc/hosts on %s:\n%s\n", sanitizeLog(deviceIP), sanitizeLog(hostsContent)) // 5. Inject CA Certificate summary := &MigrationSummary{} @@ -2313,7 +2313,7 @@ func (m *Manager) addTemporaryHostEntry(client SSHClient, deviceIP, testDomain, return fmt.Errorf("failed to add test entry to /etc/hosts: %w", uploadErr) } - fmt.Printf("Updated /etc/hosts on %s with test entry:\n%s\n", deviceIP, newHostsContent) + fmt.Printf("Updated /etc/hosts on %s with test entry:\n%s\n", sanitizeLog(deviceIP), sanitizeLog(newHostsContent)) return nil } @@ -2459,7 +2459,7 @@ func (m *Manager) resolveIP(host string, client SSHClient) (string, error) { if start != -1 && end > start { ip := output[start+1 : end] if net.ParseIP(ip) != nil { - fmt.Printf("Resolved %s to %s from device\n", host, ip) + fmt.Printf("Resolved %s to %s from device\n", sanitizeLog(host), sanitizeLog(ip)) return ip, nil } } diff --git a/pkg/service/stockholm/logutil.go b/pkg/service/stockholm/logutil.go index b7dcc5c..016628c 100644 --- a/pkg/service/stockholm/logutil.go +++ b/pkg/service/stockholm/logutil.go @@ -11,3 +11,14 @@ func sanitizeLog(s string) string { return s } + +// sanitizeErr returns err.Error() with newlines stripped to prevent log +// injection when error messages contain user-controlled values. Use in +// place of bare "%v, err" in log calls where err may wrap external data. +func sanitizeErr(err error) string { + if err == nil { + return "" + } + + return sanitizeLog(err.Error()) +} diff --git a/pkg/service/stockholm/proxy.go b/pkg/service/stockholm/proxy.go index 34e5155..00bce51 100644 --- a/pkg/service/stockholm/proxy.go +++ b/pkg/service/stockholm/proxy.go @@ -114,7 +114,7 @@ func HandleProxy(w http.ResponseWriter, r *http.Request, cfg *Config, state *Nat resp, err := executeProxyRequest(r, effectiveTarget, body, cfg, state) if err != nil { - log.Printf("[Stockholm proxy] %s %s failed: %v", r.Method, effectiveTarget, err) + log.Printf("[Stockholm proxy] %s %s failed: %s", r.Method, sanitizeLog(effectiveTarget.String()), sanitizeErr(err)) http.Error(w, "Proxy request failed", http.StatusBadGateway) return diff --git a/pkg/service/zeroconf/logutil.go b/pkg/service/zeroconf/logutil.go index 6b36c09..5db6be9 100644 --- a/pkg/service/zeroconf/logutil.go +++ b/pkg/service/zeroconf/logutil.go @@ -11,3 +11,14 @@ func sanitizeLog(s string) string { return s } + +// sanitizeErr returns err.Error() with newlines stripped to prevent log +// injection when error messages contain user-controlled values. Use in +// place of bare "%v, err" in log calls where err may wrap external data. +func sanitizeErr(err error) string { + if err == nil { + return "" + } + + return sanitizeLog(err.Error()) +} diff --git a/pkg/service/zeroconf/zeroconf.go b/pkg/service/zeroconf/zeroconf.go index 97a9ff0..77569bc 100644 --- a/pkg/service/zeroconf/zeroconf.go +++ b/pkg/service/zeroconf/zeroconf.go @@ -309,7 +309,7 @@ func PushCredentials(zcBaseURL, username, accessToken string) error { speakerPublicKey, err := GetInfo(zcBaseURL) if err != nil { - log.Printf("[ZeroConf] getInfo failed (%v), falling back to simplified token push", err) + log.Printf("[ZeroConf] getInfo failed (%s), falling back to simplified token push", sanitizeErr(err)) return pushSimplifiedToken(zcBaseURL, username, accessToken) }