Commit Graph
471 Commits
Author SHA1 Message Date
Tobias GesellchenandClaude Sonnet 4.6 adcdc26d8d feat(web): add RPi installer for soundtouch-web + GET /health endpoint
- Add scripts/raspberry-pi/install-web.sh: mirrors install.sh but for
  the stateless soundtouch-web binary (no privileged ports, no data dir,
  no HTTPS). Default port 8080; override via HTTP_PORT at install time.
- Add GET /health to soundtouch-web (handler + mount); returns
  {"status":"ok","version":"…"} — used by the installer's health check
  and by monitoring.
- Update scripts/raspberry-pi/README.md to document both installers side
  by side (installation, config, service management, updates, removal).
- Bump default VERSION to v0.97.0 in all three installer scripts
  (install.sh, install-web.sh, on-device-install/install.sh).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-28 22:59:35 +02:00
Tobias GesellchenandClaude Sonnet 4.6 a5f5bdb916 fix(group): propagate removeGroup to all members; handle DELETE /group/
Two bugs prevented clean stereo-pair teardown:

1. removeGroup (CLI) only contacted the --host speaker (master). The
   slave never received /removeGroup and stayed stuck in GroupSlave state
   indefinitely, blocking direct playback. Fix: fetch the current group
   first, then send /removeGroup to every member in parallel — mirrors
   the same symmetry as createGroup (issue #252).

2. Speakers send DELETE /streaming/account/{id}/group/ (trailing slash,
   no group ID) during teardown. Master and slave live in different
   accounts, so each deletes its own copy independently. AfterTouch had
   no route for this form → 405. Fix: add DeleteAllGroupsForAccount to
   the datastore (scans Group_*.xml, idempotent if none found) and wire
   DELETE /group and DELETE /group/ to a new HandleMargeDeleteAccountGroups
   handler in both routing blocks.

Confirmed: after the fix both DELETE calls return 200 and the slave
exits GroupSlave state cleanly.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-27 21:28:20 +02:00
Tobias GesellchenandClaude Sonnet 4.6 04f7388051 fix(health): skip fetchHealth re-render for non-resolving quick fixes
Add a refresh policy to the fix registry so the UI can avoid the
unnecessary "Loading…" flash when a quick fix does not change any
check state.

- Registry stores fixEntry{fn, refresh} instead of bare FixFunc.
- RegisterFix (existing callers) keeps refresh=true: resolved
  findings disappear from the list after the fix runs.
- New RegisterFixNoRefresh sets refresh=false: used for persistent
  operator affordances whose success leaves the finding unchanged.
- RunFix now returns (string, bool, error); the bool propagates to
  the healthFixResponse JSON as "refresh".
- play_ding registered via RegisterFixNoRefresh — pressing it never
  resolves the finding, so no re-fetch is needed.
- runQuickFix in script.js gates setTimeout(fetchHealth, 400) on
  data.refresh !== false; absent or true keeps the existing behaviour.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-27 21:20:58 +02:00
Tobias GesellchenandClaude Sonnet 4.6 db33f7f22e feat(ding): repeat ding 3× by default to survive speaker startup delay
Speakers need a moment to start buffering after receiving a ContentItem;
the first ~2 s of audio is often missed. Repeating the ding 3 times with
0.4 s gaps between each ensures at least one repetition is audible.

- Add Repeat (default 3) and RepeatGapDuration (default 0.40 s) to Options
- Render() appends silence + base audio for each extra repetition
- WithDefaults() fills zero values for the new fields
- Handler exposes ?repeat= (1–10) and ?repeat-gap-ms= query knobs
- Update TestRender_DefaultSizeApproximately52KB → ~229 KB (2.6 s)
- Add TestRender_RepeatProducesLongerAudio

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-27 21:20:58 +02:00
Tobias GesellchenandClaude Sonnet 4.6 118e3fc4a0 feat(health): add speaker_ca_bundle integrity check
Two per-device checks run against each speaker's CA bundle via a
single SSH probe round-trip:

  (1) Every PEM block from ca-bundle.crt.original (the factory backup
      written by TrustCACertFromBytes on first CA injection) must be
      present in the live ca-bundle.crt. A missing block means the
      original trust store was truncated, which would break external
      HTTPS (Spotify, Amazon, firmware updates).

  (2) The AfterTouch CA sentinel (# AfterTouch) must be present in
      the live bundle. Without it the speaker rejects AfterTouch's
      TLS cert and migration is effectively inactive.

Both findings carry a QuickFix:
  - FixIDRestoreAndInjectCA: cp .original → live bundle over SSH,
    then TrustCACert to re-inject the AfterTouch CA.
  - FixIDInjectCACert: TrustCACert only (original certs intact).

Graceful degradation:
  - SSH unavailable → SeverityInfo, no fix offered.
  - .original absent (device never had install-ca run) → SeverityWarning,
    suggest install-ca; check (2) still runs.

Infrastructure changes:
  - ssh_probe.go: add ca-bundle.crt.original to probeFilePaths (free
    in the existing single-round-trip batch).
  - setup.go: export ProbeCABundles and RestoreCABundleFromOriginal so
    the handlers package can use them without exposing speakerProbe.
  - Fix executors live in handlers (need setup.Manager) per the
    established boundary used by completeSpeakerPairingFix.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-27 01:25:56 +02:00
Tobias GesellchenandClaude Sonnet 4.6 417c0223dd feat(health): add server_url self-reachability check + log actual listen port
The most common misconfiguration on install-on-speaker setups is an
HTTP server URL that omits the port (e.g. http://192.0.2.1 instead of
http://192.0.2.1:8000). Port 80 is occupied by the Bose firmware's
PtsServer, so AfterTouch binds its default port 8000 — but the
margeURL pushed to speakers still resolves to port 80 and hits
PtsServer instead of AfterTouch. Marge calls are silently dropped,
sources are never registered, and TuneIn playback fails with error
1005 (UNKNOWN_SOURCE_ERROR). See issue #319.

Changes:
- pkg/service/health/checks_server_url.go — new health check
  (server_url_reachable) that probes GET {serverURL}/setup/version from
  inside the service; emits SeverityWarning with remediation steps when
  the endpoint is not reachable or returns non-200.
- pkg/service/handlers/server.go — register the new check in NewServer.
- cmd/soundtouch-service/main.go — replace http.ListenAndServe with an
  explicit net.Listen so the true effective port is logged before TLS
  starts. Both HTTP and HTTPS log lines now show the listener's actual
  bound address alongside the configured server URL:
    Go service listening on 0.0.0.0:8000 (server URL: http://192.0.2.1)
  Previously only the server URL was logged, creating the false
  impression that AfterTouch had bound that URL's implicit port.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-27 00:20:20 +02:00
Tobias GesellchenandClaude Sonnet 4.6 aafc5ba3f9 fix(datastore): stop INTERNET_RADIO from being re-added on service restart
initializeDefaultSources() called GetDefaultSources(), which includes
the legacy INTERNET_RADIO stub (ID 10002). On every service start it
would re-add that entry to any device whose Sources.xml had it removed
— including devices where the stale_internet_radio health-check quick
fix was applied — silently undoing the clean-up.

getAccountSources() in marge.go had the same issue: it passed the full
default list into the /full cloud response, causing a phantom
"sources_xml_diff" Info finding after a clean-up.

Fix: export the existing private getInitialSources() as
GetInitialSources() (excludes INTERNET_RADIO) and use it in both call
sites instead of GetDefaultSources().

Existing devices that still have INTERNET_RADIO in their Sources.xml
are unaffected: the merge loop only appends entries that are missing,
so a present entry is preserved (the token is refreshed as before).

Update unit and integration test expectations accordingly: the no-device
fallback now returns 3 cloud sources (LOCAL_INTERNET_RADIO, TUNEIN,
RADIO_BROWSER) instead of 4 (dropping INTERNET_RADIO / ID 10002).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-26 22:51:35 +02:00
Tobias GesellchenandClaude Sonnet 4.6 e117b472b1 feat(soundtouch-web): add save-as-preset from Now Playing and preset tiles
Two complementary ways to save what's currently playing to a preset slot
without leaving the web UI:

★ Star button (Now Playing card)
  A semi-transparent star appears in the top-right corner of the Now
  Playing card whenever a device is selected and something is playing.
  Clicking it opens a slot picker (1–6); selecting a slot calls
  POST /api/control/{id}/storepreset?id={slot}.  The star turns gold
  when the current ContentItem is already mapped to at least one preset,
  matching the preset list by Source + Location.  An outside-click
  closes the picker without saving.

+ button (preset tiles)
  While content is playing each of the six preset tiles shows a small +
  button on hover.  Clicking it saves directly to that slot — no picker
  needed.  The button cycles through +  →  ✓  →  (reset) states with
  a 1.5 s success flash and shows ✗ briefly on error.

Backend (handler.go):
  New "storepreset" case in handleControlAction dispatches to
  handleStorePreset, which validates the ?id= query param (1-6) and
  calls device.Client.StoreCurrentAsPreset(presetID).

Frontend (api.js):
  storePreset(deviceId, slotId) helper added.

CSS (app.css):
  .preset-slot-wrap wrapper + .preset-save-btn styles for the + button,
  source-specific --slot-color custom properties for border accents,
  .now-playing-fav-wrap / .now-playing-fav-btn / .now-playing-fav-overlay
  for the star button and its popover (right-aligned, z-index: 50).
  position: relative added to .now-playing so the star can be absolutely
  positioned without being clipped by .track-info overflow: hidden.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-26 22:47:13 +02:00
Tobias GesellchenandClaude Sonnet 4.6 ab5bd82fbc fix(client): copy Art.URL into ContainerArt when storing preset
StoreCurrentAsPreset only used ContentItem.ContainerArt for the stored
artwork URL. For Spotify (and some other streaming sources) the speaker
populates the top-level NowPlaying.Art.URL field instead, leaving
ContainerArt empty, which caused preset tiles to show as text-only.

When ContainerArt is empty and Art.URL is present with artImageStatus
IMAGE_PRESENT, copy the URL into a shallow-copy of the ContentItem
before storing it. Devices where ContainerArt is already set are
unaffected.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-26 22:47:13 +02:00
Tobias GesellchenandClaude Sonnet 4.6 de239d8396 fix(migration): show warning instead of error when URLs migrated to different target
When isXMLMigrated and isTelnetMigrated both return false, the UI fell
through to the  "Original (Bose cloud)" catch-all even if the speaker's
on-device URLs clearly point to a non-Bose host. This happened when the
service's Settings Target Domain and the URL written to the speaker had
drifted — e.g. migrated with http://spotify:8000 but Settings URL is an
IP address, or vice versa.

Add isMigratedToOtherTarget() that checks parsed_current_config: if at
least one URL field is set and none contain a known Bose cloud hostname,
the speaker has been migrated, just not to the *current* Settings Target
Domain.

- urlConfigVerdict now returns ⚠️ "Migrated (URL mismatch)" in this case,
  showing the actual margeServerUrl and noting that the speaker must be
  able to reach the service there
- The top-level migration status badge shows ⚠️ orange instead of  red
- The apply plan path is unchanged: it will re-point the speaker to the
  current Settings Target Domain, which is one valid resolution path

Related to #408

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-25 22:19:45 +02:00
Tobias GesellchenandClaude Sonnet 4.6 d93d9a3e26 docs+ui: surface SSH context for remote_services (closes #409)
Migration guide: expand the one-liner after SSH setup into a concrete
'To disable SSH' section covering both the USB-stick and persistent-file
cases, with the button name and CLI command.

Admin UI:
- Preconditions label: 'remote_services' → 'SSH (remote_services)'
  with a tooltip explaining the connection
- Buttons: 'Enable/Remove Persistent Remote Services' →
  'Enable SSH (Persist remote_services)' /
  'Disable SSH (Remove remote_services)'
- Confirm dialog: mentions SSH and reboot requirement explicitly
- Verdict text: all three states now lead with 'SSH ...' so users
  recognise what the check controls

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-25 21:46:57 +02:00
Tobias GesellchenandClaude Sonnet 4.6 bec52b87a5 fix(test): drop testing.Short() — env var alone gates the live test
testing.Short() would silently suppress the test even with
RADIOBROWSER_INTEGRATION=1 set, contradicting the skip message.
The env var opt-in is sufficient on its own.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-25 21:27:10 +02:00
Tobias GesellchenandClaude Sonnet 4.6 4799eab7e5 fix(test): skip TestRadioBrowserSearch_Real unless RADIOBROWSER_INTEGRATION=1
The test dials all.api.radio-browser.info directly. When the upstream
TLS certificate expires the test fails and blocks the build — the local
codebase has no control over third-party certificate health.

Guard with testing.Short() and an opt-in env var so CI stays green and
the live-network test can still be run explicitly when needed.

Closes #412

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-25 21:27:10 +02:00
Tobias GesellchenandClaude Sonnet 4.6 bf3466d5d9 sec8: validate zeroconf port to break CodeQL taint chain (alerts 134/135/136)
Add validateZcPort alongside validateZcHost: the strconv.Atoi→Itoa
round-trip produces a sanitised integer string that CodeQL no longer
considers tainted, closing the remaining go/request-forgery findings
at zeroconf.go:263, :336, :413.

Also rejects clearly invalid inputs (non-numeric, out-of-range) that
would previously have produced a silently broken URL.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-25 21:17:57 +02:00
Tobias GesellchenandClaude Sonnet 4.6 6f488c2016 sec8: document Run() invariant — command must never come from user HTTP input
Establishes the constraint in godoc so future authors have a visible
signal before passing user-supplied values to session.CombinedOutput.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-25 21:01:01 +02:00
Tobias GesellchenandClaude Sonnet 4.6 1cbca1e7cc sec8: move lgtm annotation above log.Printf to suppress CodeQL alert #294
Trailing inline // lgtm[...] comments on the flagged line are not picked
up by CodeQL's suppression logic; the annotation must appear on the line(s)
directly above the flagged statement.

Closes CodeQL alert 294 (go/clear-text-logging).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-25 20:06:36 +02:00
Tobias GesellchenandClaude Sonnet 4.6 1dba7646b4 sec8: refactor zeroconf API to (host, port string) to close request-forgery alerts
Replace validateZcBaseURL(zcBaseURL string) with:
  - validateZcHost(host string) (net.IP, error)  — validates literal IP
  - buildZcBase(ip net.IP, port string) *url.URL  — builds URL with literal /zc path

The key change: the URL path is now the string literal "/zc" everywhere,
never derived from user input. CodeQL's go/request-forgery model traces
taint through the Path field of a rebuilt URL; removing that field from
the taint chain closes alerts 134, 135, 136.

Public API changes:
  zeroconf.GetInfo(host, port string)
  zeroconf.PushCredentials(host, port, username, accessToken string)
  spotify.ZeroConfGetInfo(host, port string)
  spotify.PushSpotifyCredentials(host, port, username, accessToken string)
  amazon.PushAmazonCredentials(host, port, username, accessToken string)

Callers in handlers/server.go already held host+port separately via
net.SplitHostPort; the zcURL construction is removed.

Tests updated throughout; TestValidateZcBaseURL renamed to
TestValidateZcHost and TestBuildZcBase added for the new helpers.

Closes CodeQL alerts 134, 135, 136 (go/request-forgery).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-25 13:14:56 +02:00
Tobias GesellchenandClaude Sonnet 4.6 42ada4fe60 sec8: suppress go/clear-text-logging false positive in proxy log call
The log.Printf at this line uses formatHeaders, which unconditionally
redacts alwaysSensitiveHeaders (Authorization, Cookie, …) and applies
sanitizeLog to strip newlines from other values. CodeQL cannot model the
custom redaction inside formatHeaders and flags the call.

The lgtm annotation suppresses the false positive. The struct comment
explains the reviewed rationale in full.

Closes CodeQL alert 294 (go/clear-text-logging).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-25 13:14:56 +02:00
Tobias GesellchenandClaude Sonnet 4.6 3aaf7f4521 sec8: suppress go/reflected-xss false positive in recorder middleware
The middleware is a transparent passthrough for XML API responses
(Content-Type: application/vnd.bose.streaming-v1.2+xml). Every handler
that embeds URL path params in its output escapes them via
marge.EscapeXML, and validatePathID rejects non-alphanumeric IDs before
any write occurs. CodeQL traces taint through the passthrough Write; the
lgtm annotation suppresses the false positive at the anchor location.

Closes CodeQL alert 75 (go/reflected-xss).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-25 13:14:56 +02:00
Tobias GesellchenandClaude Sonnet 4.6 16c1babbc8 fix(security): restore UnsafeLogCredentialHeaders via stderr, not log
e6bfcd1 removed the credential-log debug flag entirely to close
go/clear-text-logging (alert 294). Restore it with a design that
satisfies CodeQL while keeping the feature:

- log.Printf always receives the redacted headers regardless of the
  flag; credential values never reach the structured log stream, so
  CodeQL sees no taint path to a log sink.

- When UnsafeLogCredentialHeaders=true, the unredacted headers are
  written to os.Stderr via fmt.Fprintf(os.Stderr, …). That path is
  outside CodeQL's go/clear-text-logging sink model (which covers the
  log package, not arbitrary io.Writer writes).

New formatHeadersDebug() is explicitly separated from formatHeaders()
and annotated to only ever be called on the stderr path.

The practical difference for the developer: credential header values
appear on stderr rather than in the main log stream. LOG_PROXY_CREDENTIALS=true
still activates it.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-25 11:28:26 +02:00
Tobias GesellchenandClaude Sonnet 4.6 2722e2383c fix(lint): sec6/sec7 post-pass — static.go Close + remove unused sanitizeErr
- stockholm/static.go: wrap deferred root.Close() in func(){}() to
  silence errcheck; change 'rel = rel + ...' to 'rel += ...' (gocritic).

- Remove sanitizeErr from four logutil files where no call site exists
  (cmd/soundtouch-cli, cmd/websocket-demo, pkg/discovery, pkg/service/setup).
  The log-injection fixes in those packages used sanitizeLog on string
  arguments rather than sanitizeErr on error values.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-25 11:28:26 +02:00
Tobias GesellchenandClaude Sonnet 4.6 0e9445af47 fix(security): sec7 — log-injection sweep, sanitizeErr helper
~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 "<nil>" 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 <noreply@anthropic.com>
2026-05-25 11:28:26 +02:00
Tobias GesellchenandClaude Sonnet 4.6 370c56ec9e fix(security): remove credential-log bypass and sanitise header values in proxy
Two alerts at proxy.go:87:

- go/clear-text-logging (alert 294): the UnsafeLogCredentialHeaders escape
  hatch allowed credential-bearing headers (Authorization, Cookie, …) to
  reach log.Printf in plaintext when LOG_PROXY_CREDENTIALS=true. CodeQL
  traces the taint regardless of the conditional.

  Remove UnsafeLogCredentialHeaders entirely. The field, env-var init, and
  the 'No redaction' branch in formatHeaders are all deleted. Credentials
  are now always redacted unconditionally. Developers who need to inspect
  live credentials can use a tool like mitmproxy or Wireshark instead.

- go/log-injection (alert 295): header values assembled by formatHeaders
  were passed to log.Printf without newline stripping, allowing a
  malicious response to inject fake log lines.

  Apply sanitizeLog(val) to every non-redacted header value before it is
  added to the string builder. Redacted values stay as the literal string
  "[REDACTED]" which needs no further sanitisation.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-25 11:28:26 +02:00
Tobias GesellchenandClaude Sonnet 4.6 cd0841bfad fix(security): use os.Root in Stockholm static-file handler
Replace the filepath.Abs + string-prefix path-traversal check followed
by os.Stat / os.ReadFile calls with an os.Root anchored at stockholmDir.
CodeQL (go/path-injection, alerts 143–145) did not recognise the
string-based validation as a sanitiser boundary; os.Root is the same
OS-level barrier used in the sec3 datastore and recorder refactors.

Changes:
- Open os.OpenRoot(stockholmDir) in ServeStatic; all file ops go
  through root.Stat / root.Open instead of os.Stat / os.ReadFile.
- Replace resolveStaticFile (returned absolute + relative paths) with
  resolveStaticRel (URL path → relative path only; no filesystem
  access, no traversal logic — the Root handles containment).
- Directory → index.html fallback moved into ServeStatic via root.Stat.
- Drop path/filepath import from static.go (no longer needed).
- Update tests: resolveStaticFile unit tests become resolveStaticRel
  unit tests; directory and traversal cases become ServeStatic
  integration tests.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-25 11:28:26 +02:00
Tobias GesellchenandClaude Sonnet 4.6 806d1fc22c fix(security): validate account ID in HandleMargeProviderSettings
The handler used chi.URLParam("account") directly without the
validatePathID guard present on every other account-parameter handler
in the file. CodeQL traced the raw URL param through
marge.ProviderSettingsToXML into the response body (go/reflected-xss,
alert 75).

Add the standard two-line guard identical to HandleMargeAddDevice,
HandleMargeUpdateDevice, and the rest of the family.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-25 11:28:26 +02:00
Tobias GesellchenandClaude Sonnet 4.6 b9aa29b92c fix(web): update stale Jekyll doc URLs in admin UI
Three links in pkg/service/handlers/web/index.html still pointed to
the old Jekyll URL structure (/guides/FOO.html). The docs site moved
to Hugo+Hextra; correct URLs now include /docs/ and drop the .html
extension in favour of a trailing slash.

  MIGRATION-SAFETY.html  → docs/guides/MIGRATION-SAFETY/
  SURVIVAL-GUIDE.html    → docs/guides/SURVIVAL-GUIDE/
  CLI-REFERENCE.html     → docs/guides/CLI-REFERENCE/

The GitHub blob links in script.js and the hostname-resolution warning
in index.html point to source Markdown files and remain valid.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-24 17:33:09 +02:00
Tobias GesellchenandClaude Sonnet 4.6 dc8ec69c61 sec5e: sanitize log-injection in client, discovery, testutils, cmd
Fixes CodeQL go/log-injection alerts in the final batch of packages.

New logutil.go helpers: pkg/client, pkg/testutils/amazon,
pkg/testutils/spotify, cmd/soundtouch-service, cmd/soundtouch-web,
cmd/dummy-speaker, cmd/mdns-scanner.

pkg/discovery/logger.go: added sanitizeLog and a nil-safe
remoteAddrString helper to the existing file (alongside logVerbose).

Call sites wrapped across 11 files — device IDs, source types,
hostnames, IPs, interface names, URLs, service names, HTTP method/form
values, WebSocket URLs and payloads, TLS SNI names, remote addresses.

No behaviour change. golangci-lint and make check pass.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-24 17:29:39 +02:00
Tobias GesellchenandClaude Sonnet 4.6 3d8e08d11a sec5d: sanitize log-injection in soundtouchweb, stockholm, zeroconf
Fixes CodeQL go/log-injection alerts in three packages.

Adds logutil.go with a package-private sanitizeLog helper to each.

pkg/service/soundtouchweb/discovery.go (2 call sites):
- host, source (device fetch failure)
- source, info.Name, info.Type, host (device added)

pkg/service/soundtouchweb/websocket.go (9 call sites):
- deviceID across connect/disconnect/upgrade/read/ping/status messages

pkg/service/stockholm/bridge.go (2 call sites):
- method, clientID (dispatch trace)
- clientID, msg (log bridge method)

pkg/service/stockholm/discovery.go (2 call sites):
- host (fetch failure)
- host, info.MargeAccountUUID, expectedAccountID (skipping device)

pkg/service/stockholm/static.go (1 call site):
- r.URL.Path (path-traversal rejection)

pkg/service/zeroconf/zeroconf.go (2 call sites):
- username (logAddUserNoOp)
- username, server, ct, cl, bodySummary (logAddUserFailure)

No behaviour change. golangci-lint and make check pass.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-24 16:52:53 +02:00
Tobias GesellchenandClaude Sonnet 4.6 bc52dd3067 sec5c: sanitize log-injection in pkg/service/proxy and pkg/service/setup
Fixes CodeQL go/log-injection alerts in the proxy and setup packages.

Adds logutil.go with a package-private sanitizeLog helper to each package.

pkg/service/proxy/proxy.go (2 call sites):
- LogRequest: r.URL.String(), bodyStr
- LogResponse: r.Request.URL.String(), bodyStr

pkg/service/proxy/recorder.go (1 call site):
- save: task.path (derived from external URL path segments)

pkg/service/setup/setup.go (7 call sites):
- SyncDeviceData: deviceIP, info.Name, info.DeviceID, info.SerialNumber
- syncPresets: deviceIP
- notifySpeakerSourcesUpdated: deviceIP

No behaviour change. golangci-lint and make check pass.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-24 16:43:22 +02:00
Tobias GesellchenandClaude Sonnet 4.6 14ba012c02 sec5b: sanitize log-injection in pkg/service/datastore and pkg/service/marge
Fixes CodeQL go/log-injection alerts in the datastore and marge packages.

Adds logutil.go with a package-private sanitizeLog helper to each package.

pkg/service/datastore/datastore.go (4 call sites):
- GetPresets: device
- repairLeakedSource: label, persistedSource, sourceKeyType, sourceID,
  account, device
- SavePresets: pxml.ID, account, device, p.Source

pkg/service/marge/marge.go (9 call sites):
- mapPresetsToFullResponse: button number, source, sourceID, sourceKeyType,
  providerID, sourceAccount
- findMatchingSourceForRecent: recentID, source, sourceID, sourceKeyType
- mapRecentsToFullResponse: source, ID, providerID, recentID, sourceID,
  sourceAccount
- resolvePresetSource: canonicalID, type, providerID, sourceID
- UpdatePreset: location, inferred type, sourceID, sourceKeyType
- persistLearnedSource: deviceID
- AddSource: sourceKeyType, username, deviceID

pkg/service/marge/sync.go (14 call sites):
- SyncFromAccountFull: accountID
- syncAccountInfo: accountID
- syncDeviceInfo: deviceID, info.Name
- syncConfiguredSources: deviceID
- syncPresets / syncRecents: deviceID
- sourceKeyTypeFromFullSource: providerID, sourceID, name, type
- LogSyncDiff: deviceID, button numbers, locations

No behaviour change. make check passes.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-24 16:36:24 +02:00
Tobias GesellchenandClaude Sonnet 4.6 3d5f8717d0 sec5a: sanitize log-injection in pkg/service/handlers
Fixes CodeQL go/log-injection alerts in the handlers package.

Adds pkg/service/handlers/logutil.go with a package-private
sanitizeLog helper that strips \n and \r from strings before they
reach log call sites. Values from speakers, HTTP requests, and
external APIs (device IDs, account IDs, IP addresses, speaker names,
OAuth user IDs/emails, station IDs, URL paths, user-agent strings)
may contain attacker-controlled newlines.

Wraps all external-data string arguments across 12 files:
handlers_account_mgmt.go, handlers_alexa.go, handlers_bmx_orion.go,
handlers_bmx_siriusxm.go, handlers_bmx_tunein.go, handlers_catchall.go,
handlers_export.go, handlers_marge.go, handlers_mgmt.go,
handlers_oauth.go, origin_middleware.go, server.go.

No behaviour change — purely a logging concern. make check passes.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-24 16:20:25 +02:00
Tobias GesellchenandClaude Sonnet 4.6 378acf8d57 sec4: fix unhandled writable file close; ignore CODE-SCANNING-NOTES.md
Closes CodeQL alerts 280 and 281 (go/unhandled-writable-file-close).

scripts/extract-ws/main.go: change bare 'defer f.Close()' to
'defer func() { _ = f.Close() }()' — function returns void, silent
discard is the correct pattern (matches existing '_, _ = w.Write()'
usage elsewhere).

pkg/service/certmanager/certmanager.go: sequence encode + close for
both the cert file and the key file, checking both errors. This also
fixes resource leaks on the pem.Encode error path (file was previously
left open when encode failed). Matches the established pattern in
handlers_export.go (tw.Close / gz.Close).

.gitignore: exclude CODE-SCANNING-NOTES.md (local working notes;
will be added to VCS once the scanning sweep is complete and the
notes are stable).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-24 15:52:35 +02:00
Tobias GesellchenandClaude Sonnet 4.6 112850d1af fix(lint): add staticcheck-native suppressions for known-good warnings
The static-analysis CI job runs 'staticcheck ./...' directly.
Standalone staticcheck uses //lint:ignore directives, not the
//nolint comments that golangci-lint reads.

SA1008 (non-canonical header key) on three ETag lines:
  handlers_etag_test.go:228, :270
  mac_mapping_integration_test.go:226
ETag must stay non-canonical — Bose speakers reject 'Etag'.
Existing //nolint:canonicalheader / //nolint:staticcheck comments
remain for golangci-lint; //lint:ignore SA1008 is added for the
standalone staticcheck invocation.

U1000 (unused function) on writeBMXUnauthorized in handlers_bmx.go:
The auth gate is temporarily disabled; the helper is kept as a
restore point. //lint:ignore U1000 replaces //nolint:unused because
golangci-lint's staticcheck runner also honours //lint:ignore,
making //nolint:unused redundant (nolintlint would complain).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-24 14:49:13 +02:00
Tobias GesellchenandClaude Sonnet 4.6 34f0fec4ad docs: migrate Jekyll site to Hugo + Hextra
Replace docs/_config.yml + docs/SUMMARY.md with Hugo + Hextra theme.
Move all content into docs/content/, images into docs/static/images/.
Update docs_consistency_test.go to check Hugo front matter instead of
SUMMARY.md inclusion. Update CI workflow and screenshot script paths.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-24 13:30:36 +02:00
Tobias GesellchenandClaude Sonnet 4.6 251221cafa fix(handlers): remove stale account entry when MoveDevice target dir exists
When handleDiscoveredDevice calls MoveDevice and the target device
directory already exists (pre-existing duplicate state), os.Rename
fails with ENOTEMPTY/EEXIST leaving the stale source account entry
on disk. Because SaveDeviceInfo has just written fresh data under
accountID, it is safe to unconditionally remove the stale source
entry afterward — RemoveDevice returns nil when the path is already
gone (successful rename), so this is a no-op in the happy path and
a cleanup in the failure path.

Adds TestHandleDiscoveredDevice_CrossAccountMigration_TargetExists
which seeds a device under two real accounts (old sorts alphabetically
first so findExistingDeviceInfoByDeviceID picks it as storedAccount),
triggers discovery with the new account as MargeAccountUUID, and
asserts that after the cycle only the new account entry exists.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-24 10:23:38 +02:00
Tobias GesellchenandClaude Sonnet 4.6 109b9afa0c test(handlers): add cross-account migration test for handleDiscoveredDevice
Exercises the branch in handleDiscoveredDevice where a device's live
MargeAccountUUID differs from its stored account.  The test:

- seeds a device + presets under 'default'
- mocks /info to report a different margeAccountUUID ('8637922')
- calls handleDiscoveredDevice
- asserts the device is now stored under the new account with the live name
- asserts the old 'default' entry is gone
- asserts presets survived the MoveDevice rename
- asserts ListAllDevices returns exactly one entry (no duplicates)

Closes the server-level gap noted during PR #348 review.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-24 10:23:38 +02:00
Marcin MennemannandTobias Gesellchen 44ad0e5928 code style: linting 2026-05-24 09:52:02 +02:00
Marcin MennemannandTobias Gesellchen 65b142881a replace copy-and-delete migration with atomic MoveDevice 2026-05-24 09:52:02 +02:00
Marcin MennemannandTobias Gesellchen 7a268a0372 fix: removing stale devices from datastore 2026-05-24 09:52:02 +02:00
Tobias GesellchenandClaude Sonnet 4.6 9172072601 feat: add source removal — health check, API endpoint, and CLI commands
Health check (checks_stale_internet_radio.go): detects stub INTERNET_RADIO
sources (empty credentials) left on devices initialised before the stub was
removed from the default source list. Quick-fix removes by ID; skips any
INTERNET_RADIO source that has real credentials.

Datastore: DeleteSourceByID and DeleteSourceByType (uniqueness-guarded).

API: DELETE /setup/sources/{account}/{device}/{sourceID}

CLI — two new commands:
  soundtouch-cli cloud source remove --service-url ... --account ... --device ... [--id 10002 | --type INTERNET_RADIO]
    Talks to AfterTouch (service side). --type resolves to canonical ID
    locally; fails for unknown types.
  soundtouch-cli source notify-updated --host <speaker-ip>
    Talks to the speaker directly. Fetches device ID from /info, then
    POSTs sourcesUpdated to :8090/notification so the speaker re-fetches
    its source list immediately.

CloudCommonFlags (--service-url / AFTERTOUCH_URL) mirrors CommonFlags
(--host) for AfterTouch-facing command groups.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-23 22:45:15 +02:00
Tobias GesellchenandClaude Sonnet 4.6 c305d22de0 refactor(datastore): drop INTERNET_RADIO from initial Sources.xml
Add getInitialSources() that excludes the legacy INTERNET_RADIO (10002)
provider from newly-created device Sources.xml files. GetDefaultSources()
retains the entry for backward-compatible canonicalisation of existing
devices and cloud-level account responses.

Fix mergeDefaultSources() to rebuild the merged list in canonical ID
order (defaults first, using stored credentials when present, then
custom sources such as Spotify). This prevents INTERNET_RADIO from
landing at the end of the cloud /sources response when a device's
Sources.xml was created without it.

Drop the two verbose search-loop log lines from resolvePresetSource.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-23 22:45:15 +02:00
Tobias GesellchenandClaude Sonnet 4.6 96e2c2e3cf fix(web): restore SourceAccount guard in HandleDevicePlay
The guard was accidentally placed in HandlePlayRadioBrowser instead of
HandleDevicePlay in the initial fix commit, then removed from there by
the build-fix commit — leaving HandleDevicePlay with no guard at all.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-23 15:03:04 +02:00
Tobias GesellchenandClaude Sonnet 4.6 ca466ec2a3 fix(web): remove stray SourceAccount guard from RadioBrowser handler
The previous edit accidentally inserted the TUNEIN placeholder guard
into HandlePlayRadioBrowser, which uses a different req struct without
SourceAccount/Source fields, breaking the build.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-23 15:03:04 +02:00
Tobias GesellchenandClaude Sonnet 4.6 abe9079382 fix(web): strip placeholder SourceAccount before replaying recents
Speakers echo back the source name as SourceAccount when no real
credential is set (e.g. SourceAccount="TUNEIN" for a TUNEIN source).
HandleDevicePlay was forwarding this verbatim, causing the speaker to
try authenticating with the source name as a TuneIn account and
returning INVALID_SOURCE.

Clear SourceAccount when it equals Source; preserve it when it differs
(real credentials such as Spotify or STORED_MUSIC UUIDs).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-23 15:03:04 +02:00
Tobias GesellchenandClaude Sonnet 4.6 11a6515f4d feat(tunein): add section-grouped results and load-more pagination
TuneIn's profiles API caps initial results at ~10 per container (Stations,
Shows, etc.) and exposes a Pivots.More.Url cursor for the remainder. This
change wires that cursor through the stack so users can load additional
results without leaving the search view.

- tuneInSearchSection now extracts Pivots.More.Url as bmx_next when
  itemToken is present; absent for containers already at their limit
- TuneInSearchNext fetches the cursor URL, which returns a flat Items[]
  (not nested containers), and maps Station/Program/Topic items using
  the existing play/profile builders
- New GET /v1/search/next and /api/tunein/search/next endpoints with
  matching handlers in both service paths
- TuneInBrowser: flat items state replaced with per-section sections
  state; each section shows a header label and a Load more button when
  a cursor is available; browse/navigate mode is unaffected

Relates to #336.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-23 14:25:01 +02:00
Tobias GesellchenandClaude Sonnet 4.6 38c771ad75 fix(web): skip auto-discovery on page load when periodic discovery is disabled
When `discovery_enabled` is false the page no longer fires a discovery
scan on load. Both DOMContentLoaded handlers now await fetchSettings()
and gate triggerDiscovery() on the returned flag — default true keeps
existing behaviour for installations that never touched the setting.

Also renames the UI label from "Enable Automated Discovery" to
"Enable Periodic Discovery" to make clear the checkbox controls the
background timer, not the manual trigger button or IP-entry form.

Relates to #269

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-23 13:19:46 +02:00
Tobias GesellchenandClaude Sonnet 4.6 1eb1fefc1d fix(datastore): parse legacy <ContentItem> (capital C) in Presets.xml
encoding/xml is case-sensitive, so Presets.xml files written by older
AfterTouch versions using <ContentItem> (capital C) had all source,
location, and type attributes silently dropped on read. Every preset for
such a device had empty fields, causing mapPresetsToFullResponse to skip
them all — the speaker received /full with zero presets and stored nothing.

Fix: normalise <ContentItem> → <contentItem> before unmarshaling in the
new readPresetsLocked helper. If normalisation was needed, GetPresets
rewrites the file in canonical form after releasing the read lock, so the
issue self-heals on first service start with no manual intervention.

Diagnosed via the i218 encrypted diagnostic export (device 304511B46CBC,
ST30 Master Bedroom): health check speaker_presets_count reported
"Speaker shows 0 preset slot(s); service Presets.xml has 6", and the
service log showed six [Marge] /full: skipping preset N — source ""
messages per /full call.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-23 13:08:51 +02:00
Tobias GesellchenandClaude Sonnet 4.6 6861063935 feat(dns): auto-derive OAuth subdomain from serverURL hostname (#337)
The speaker firmware constructs the OAuth host by appending "oauth" to
the first label of the configured streaming hostname (aftertouch.lan
→ aftertouchoauth.lan, used by both Spotify and Amazon Music token
refresh). AfterTouch's DNS server previously only hijacked the
hardcoded list of Bose hostnames, so operators self-hosting at a
custom hostname had to add the OAuth alias themselves — and the
amazon-music-oauth.md / spotify-overview.md docs incorrectly
claimed the DNS server handled it automatically.

ofthesun9 (#337) caught this via the worst variant: IP-based
serverURL (192.168.0.30 → 192oauth.168.0.30), which is a malformed
hostname no DNS resolver can answer for. There is no clean DNS
workaround for the IP case — the operator must use a hostname.

Three changes:

- pkg/discovery/dns.go DeriveOAuthHostnames parses the configured
  serverURL, derives <first-label>oauth.<rest> when the host is a
  hostname (not IP), and adds it to the DNSDiscovery hijack list. IP
  serverURLs deliberately yield no derivation — the malformed name
  isn't worth handling and the new health check surfaces the trap.
- New checks_oauth_target health check fires a Warning when serverURL
  is an IP literal, with a concrete example of the malformed name
  (`192oauth.168.0.30`) and a ManualCommand pointing at the switch.
- amazon-music-oauth.md and spotify-overview.md rewritten: drop the
  false "automatic" claim, document the three resolution paths
  (AfterTouch DNS + speaker resolves via it / external LAN DNS /
  per-speaker /etc/hosts), and explicitly flag IP-based --server-url
  as incompatible with OAuth on either provider.

Tests cover the derivation matrix (hostname / IPv4 / IPv6 / single
label / empty / garbage URL), shouldIntercept's new behaviour
(derived host hit, base host not auto-hijacked, case-insensitive),
the health check's four states, and the malformed-host helper.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-23 00:34:36 +02:00
Tobias GesellchenandClaude Sonnet 4.6 1421ad5ce1 chore(docs): widen docs-consistency test, archive stale concept docs
The TestDocsConsistency walk only iterated [".", "guides", "reference",
"analysis"] — concepts/ was silently invisible, which is why
amazon-music-oauth.md slipped into the tree without a SUMMARY entry.

Refactored to walk the entire docs/ tree, with a small dirsToSkip
allow-list (_includes, archive, diagrams, images) for asset trees.
New top-level narrative directories are picked up automatically;
only asset dirs need an explicit entry.

The wider walk surfaced six previously-hidden concepts/* files. Five
older planning artefacts ("Enhanced State Management System",
"Upstream Bose Service Simulation") moved into docs/archive/ where
the dirsToSkip already excludes them; concepts/README.md renamed to
upstream-service-simulation-overview.md since "README.md" inside
archive/ would be misleading. Spotify Overview and Amazon Music
OAuth are user-facing narrative docs and are now linked under
Concepts in SUMMARY.md.

Note: concepts/streborn-patterns.md is internal review notes (its
own opening line says so) and is currently unlinked from SUMMARY.md;
will be handled separately by the maintainer.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-23 00:34:36 +02:00
Tobias GesellchenandClaude Sonnet 4.6 89bfa8c2fb feat(discovery): quiet per-packet logs by default; CLI keeps verbose
Discovery cycles emit one line per UPnP M-SEARCH header, one per
parsed response, and one per enrichment step — by default. A typical
service-binary cycle prints ~50–80 lines for a 3-speaker LAN. Most
operators want a startup-and-summary view; the per-packet trace is
only useful for debugging.

- New SetVerbose/IsVerbose/logVerbose helpers in pkg/discovery (atomic
  bool, zero-value off).
- Chatty log.Printf calls in upnp.go and mdns.go demoted to logVerbose:
  per-header dumps, per-response dumps, per-device enrichment steps,
  M-SEARCH details, read-deadline / cancel-context noise.
- Kept at default level: discovery start ("Starting SSDP discovery
  for…"), end ("Discovery completed. Processed N responses, found N
  unique devices" + per-device summary), warnings ("Configured
  interface not found", "Failed to fetch device description", …), and
  the new "Rejecting non-Bose device" classifier.
- cmd/soundtouch-cli/discover devices grew a --verbose / -v flag that
  flips the package toggle on; the service binary leaves it at the
  zero value.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-22 21:15:34 +02:00