Addresses recommendations 4 and 7 from #515 comment 5231931569: a
green-looking getpdo readback only confirms the sys configuration
writes were accepted, not that they'll survive a reboot (that's what
the envswitch-persisted layer decides). Labels the getpdo line in both
migrateViaTelnet and runTelnetInjection's CLI/log output accordingly,
softens migrateViaTelnet's "succeeded" wording to "accepted", and adds
the same one-line caveat to TELNET-MIGRATION-METHOD.md #2.3 (previously
only in TELNET-COMMAND-REFERENCE.md).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Community hardware testing (bitranox, JRpersonal) on 2026-08-09 retracted
the earlier "inter-command delay is necessary" theory and established that
envswitch boseurls set commits the whole runtime layer (not just its two
arguments), has no read form, and doesn't ack with "OK". Corrects
TELNET-MIGRATION-METHOD.md and TELNET-COMMAND-REFERENCE.md accordingly,
retracts the stale "confirmed necessary" command-delay claim in
enable_ssh.go/cmd_setup.go, and lowers DefaultTelnetCommandDelay 5s -> 3s
as a smaller hedge now that the delay itself is known not to be the
mechanism.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
ErrorsResponse.Error() only returned the <error> element's text body,
dropping the name attribute entirely. Some speaker error responses
have a Message that just restates Value as text (e.g. a bare "1047"
for SOURCE_ALREADY_REMOVED), so callers only ever saw the useless
numeric string. Found while live-debugging a Deezer account
add/remove cycle on real hardware, where the raw XML
(<error value="1047" name="SOURCE_ALREADY_REMOVED">1047</error>)
carried real information only in the name attribute.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Guards against whitespace-only stationID/podcastID/encodedName path
segments and tightens the existing empty-string checks on the search
q/cursor query params. Spotted while reviewing stalkerquatre-oss's
fork diff for TuneIn handling improvements; their s0/Radio fallback
defaults were skipped as unprecedented invented values that would
mask malformed requests instead of erroring.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Follow-up to the 3s default from earlier in #515: the reporter agreed
5s is a better trade-off (issue comment 5230881285) — more headroom
than the original guess, still comfortably under the ~7s gap their
manual A/B test used.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
#515 comment 5230833551: on a genuinely unpaired (factory-reset) device,
margeServerUrl is reportedly never polled at all, so the boseurls
SSH-enable injection has no read cycle to fire on regardless of any
command delay. enable-ssh now checks /info first and, if
margeAccountUUID is empty, pairs the device via the existing
PairAccount helper (HTTP /setMargeAccount, telnet fallback) before
running the injection.
Adds setup.Manager.EnsureMargeAccountPaired plus --no-auto-pair (skip
entirely) and --account (use a specific 7-digit ID instead of a
generated one, e.g. to match one already in the datastore) flags on
enable-ssh. Pairing failure is a warning, not fatal, since the claim
is unconfirmed on this specific hardware and existing working flows
must not regress.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Per #515 comment 5228449448: on a real Lifestyle console, the same 6
commands (5 sys configuration/envswitch + reboot) sent back-to-back left
sshd down after reboot, but succeeded sent one at a time with ~7s gaps —
same commands, same order, same device, minutes apart. Sending fast may
not let the device fully process one command before the next arrives.
Adds --command-delay (setup.DefaultTelnetCommandDelay, 3s), threaded
through EnableSSHViaTelnetFullConfig/runTelnetInjection (pause after each
of the 5 commands) and runEnableSSHInjection (one more pause before the
reboot). 0 restores the old back-to-back behavior. The reporter didn't
try to find the true minimum, just confirmed ~7s works and speculated
"a second or two may well be enough" — 3s is a middle ground, tunable via
the flag if a specific device needs more.
Also prints an approximate total for the injection phase up front (6
steps x delay, ~18s at the default) so the command doesn't look hung —
separate from the existing --wait message for sshd coming up after
reboot, which can take much longer.
Refs #515
Follow-up to #591, prompted by the update-check notice showing a raw
https:// URL as plain text instead of a clickable link. Made it general
rather than a one-off fix, since future announcements may also want to
link to docs.
Added Announcement.LinkText/LinkURL (+ LinkURLFunc, the dynamic
counterpart, for the update-check entry's per-release URL) alongside the
existing Message/MessageFunc pair. Both frontends render it as a real
<a> element now: the admin UI (innerHTML) escapes Message/LinkText/LinkURL
via the existing escapeHtml() before composing the markup — previously
Message went into innerHTML unescaped, which this incidentally hardens;
the player (Preact/htm) templates an actual <a> rather than interpolating
a string, since Preact escapes string children by default and a raw
<a href=...> string would otherwise render as literal text, not a link.
Rephrased the #419 admin-gate announcement to use the new field too (was
a plain "See issue #419 for details." text mention).
Bug found while wiring this up: UpdateCheckState never persisted the
release URL, only the version — so after a restart, the announcement
would show a correct message but a broken/empty link until the next live
check completed (which can be up to a full interval away, since a fresh
check is skipped when the persisted last-check is still recent). Fixed by
adding UpdateCheckState.LastReleaseURL and threading it through
Checker.persist/NewChecker's seeding path, with a test
(TestNewChecker_SeedsFromPersistedState) that would have caught it.
Also fixed two gocritic rangeValCopy findings in
handlers_announcements.go (switched to index-based iteration) surfaced by
the Announcement struct growing with the new fields.
Refs #591
Addresses the Semgrep finding on PR #599
(go.lang.security.audit.crypto.math_random.math-random-used) on the
update-check jitter delay, and applies the same treatment to the #419
activity-log filename suffix, which has the same non-security shape but
predates this PR's diff so it wasn't flagged.
Neither value is ever compared, kept secret, or otherwise security-
sensitive (a sleep duration and a filename-uniqueness suffix), so
crypto/rand would only add error-handling overhead for no real benefit.
Suppressed with the same // nosemgrep: <rule-id> pattern already used in
the mock-amazon/mock-spotify/mock-tunein servers, mirroring the existing
//nolint:gosec on the same lines.
Refs #591
Fifth and final piece of #591's initial implementation. Extends
/api/setup/version with update_available/latest_version/
latest_release_url (nil-safe via Server.UpdateCheckResult, defaults to
Available: false when the check was never enabled). Response switched from
map[string]string to map[string]interface{} to carry the new bool field;
updated the one existing test that decoded into the old stricter type.
Documents UPDATE_CHECK_ENABLED/UPDATE_CHECK_INTERVAL in the Configuration
Options reference table, explicit that this is the only network call
AfterTouch makes beyond speaker/provider traffic when enabled, and that it
defaults off.
This closes out the initial #591 implementation per the design doc
(_/i591/design-update-check.md): UpdateCheckState persistence, the
updatecheck.Checker package, background goroutine wiring with jitter/
backoff, reusing #419's Announcements mechanism instead of a second notice
UI, and this version-info exposure. `make check` passes end to end
(including the Docker HTTP integration suite).
Refs #591
Fourth piece of #591 — the "minimal and future-proof at once" move from
the design doc: no new notice UI, just one new entry in the #419
announcements list, which is already rendered in both the admin UI and the
player and already has per-ID dismissal.
Added Announcement.MessageFunc/DismissKeyFunc (nil = use the static
Message/ID, as before, so the existing #419 entry is unaffected) since
this entry's text names a specific version and its dismissal must be
per-version — dismissing the notice for v1.2.0 must not suppress a later
notice for v1.3.0. HandleListAnnouncements/HandleDismissAnnouncement now
compute the effective key through Announcement.dismissKey(s) rather than
reading the static ID field directly.
Refs #591
Third piece of #591. --update-check-enabled/--update-check-interval
(UPDATE_CHECK_ENABLED/UPDATE_CHECK_INTERVAL), default off/24h, following
the same local main.go flag pattern as discovery-enabled — not pkg/config,
which soundtouch-service doesn't import at all (correction to the issue's
proposed location, see the design doc).
Background goroutine modeled on startDeviceDiscovery: startup jitter
(0-5min), skips the immediate check if the persisted last-check is still
fresh, backs off retries to no sooner than 1h after a failure, logs once
per newly-detected version. The decision logic (shouldCheckImmediately,
shouldSkipDueToBackoff, logUpdateIfNewlyAvailable) is split into pure,
directly-testable functions rather than living inline in the goroutine.
Server gets a SetUpdateChecker/UpdateCheckResult pair (nil-safe) so the
next two pieces (announcement, /api/setup/version) can read the current
state without importing updatecheck's construction details.
Manually verified against a running instance: enabled via flags, no panic,
service stays responsive (jitter means the actual first check can take up
to 5 minutes to fire, so this only confirms the wiring, not a live
GitHub response — that's covered by the previous commit's httptest-backed
unit tests).
Refs #591
Second piece of #591. Standalone package (pkg/service/updatecheck):
GitHub releases API client, golang.org/x/mod/semver comparison (promoted
from indirect to direct dependency), persisted state via datastore's
UpdateCheckState. Dev/(devel)/dirty current versions skip the comparison
entirely rather than guessing; prereleases are excluded even though
GitHub's /releases/latest endpoint shouldn't return one anyway (defensive).
Deliberately decoupled from handlers.Server/main.go: repo and current
version are constructor arguments, not hardcoded, so a future CLI-side
check could reuse this as a plain import rather than a rewrite (open
question 2 in the design doc).
Not wired into the service yet — nothing calls NewChecker/CheckNow outside
tests.
Refs #591
First piece of #591 (opt-in periodic update check). A small persisted
state (last_checked_at, last_seen_version) under update-check.json,
mirroring Settings' Get/Save shape — separate from Settings itself since
this is runtime state, not operator-editable config.
Not wired to anything yet.
Refs #591
Prompted by not being able to tell where a locally-run instance's data dir
actually was without inspecting the running process (ps/lsof). Adds
data_dir to /api/setup/version's response, resolved to an absolute path so
it's unambiguous regardless of whether --data-dir/DATA_DIR was relative or
left at the default.
Shown as a read-only line at the top of the Settings tab, not the always-
visible footer — the footer is prime real estate seen on every tab/every
page load, and this is a rarely-needed piece of diagnostic info that
belongs alongside the rest of System Settings instead.
Also filled in a test-helper gap: the pkg/service/handlers package's
internal test router (main_test.go) never registered /setup/version at
all, unlike the real production router — added it so the new test (and any
future one exercising this endpoint) can actually run.
Unrelated to #419, but found while verifying that work against a running
instance.
The backend (field, validation, guard rail, live-reload middleware) has
worked since the first #419 commit, and the announcement banner correctly
told people "you can opt in now in Settings" — but there was never
actually a control in Settings to do that with. Caught by manual testing
against a running instance: the banner rendered fine, proving chunks 1-6
worked, but Settings had nothing to act on.
Adds a select (mirroring default_landing's pattern) with the tri-state
choices spelled out in plain language, wired into updateSettings()/
fetchSettings() alongside the other fields.
Verified end-to-end against a running instance: setting it to "enabled"
(with non-default credentials) persists, and immediately gates /admin
(401 without credentials, 200 with) without a restart.
Refs #419
The design's three-area target model (chooser/app/admin) and the backend
(HandleListAnnouncements' target=app filtering) already supported this, but
nothing in soundtouch-player's frontend called it — chunk 5 only wired the
admin UI. New Announcements Preact component (static/js/components/), mounted
in App() above the main content so it's visible across every page, styled
with the app's existing CSS variables (dark-mode-aware, unlike the admin
UI's hardcoded inline colors). Currently renders nothing, since no
announcement in the list targets "app" yet (only the admin-gate notice,
targeting "admin") — this is just closing the parity gap so a future
app-targeted announcement has somewhere to show up.
Verified end-to-end against a running instance: the component is served
under /app/static/js/components/, app.js references it, and
/api/announcements?target=app responds correctly (empty today).
Refs #419
Seventh and final piece of #419's initial rollout. Adds addActivityLog to
buildDiagnosticArchive, walking stats/activity/ and bundling every event
file verbatim (same idea as the per-device XML bundling, mirroring
addSettingsJSON's placement). Without this, the privacy guarantee discussed
during design ("local-only, but included in an explicit diagnostic export")
would have been aspirational rather than true — caught before documenting
it as fact.
Documents the activity log in DIAGNOSTIC-EXPORT.md, anchored to the
existing "all data stays on your network" language in
SOUNDTOUCH-SERVICE-ANNOUNCEMENT.md.
This closes out the initial #419 implementation: AdminAreaAuth setting +
guard rail, BasicAuthAdmin gate, activity log + dismissal cache,
announcements + dismiss endpoint, admin UI banner, health check nudge, and
now diagnostic-export coverage + docs. Still opt-in only (AdminAreaAuth
defaults to unset) — flipping the default is a separate, later change per
the design doc's rollout plan.
Refs #419
Sixth piece of #419. Visibility-only nudge, same spirit as
mgmt_default_credentials: surfaces on the Health tab that the admin-area
gate exists and is unset, for operators who dismissed the announcement
banner or never saw it on an older release. Does not gate anything.
Refs #419
Fifth piece of #419: wires up the announcements endpoint added in the
previous commit. A banner container sits outside the tab-content divs
(index.html) so it stays visible across all tabs, not just one. Fetched
once on page load alongside settings/version/devices; dismissing calls the
server-side dismiss endpoint (not a client-only localStorage flag, so it
stays dismissed across sessions/devices) and removes it from the DOM
immediately.
Manually verified end-to-end against a running instance: the banner
container renders, the admin-gate notice appears by default, dismissing it
removes it from subsequent /api/announcements responses. No Go test
coverage — this is frontend-only wiring of already-tested endpoints.
Refs #419
Fourth piece of #419: a small in-code (not admin-authored) announcement
list, target-scoped ("app"/"admin", "chooser" reserved but not wired since
the landing page has no JS yet) and filterable by live server state via
ShowWhile. First entry: the admin-area-gate heads-up, shown on the admin
target while AdminAreaAuth is unset.
GET /api/announcements?target=... and POST /api/announcements/{id}/dismiss
are deliberately NOT behind BasicAuthAdmin — the whole point of the gate
notice is to reach operators who haven't set up credentials yet, the exact
audience an admin-only endpoint would exclude. The dismiss endpoint
validates id against the known announcement list before it reaches
RecordActivity, since this is the one call site where an id comes from an
HTTP request rather than a compile-time constant.
Updated the router snapshot (testdata/router_routes.txt) for the two new
routes.
Not wired into any UI yet — nothing calls these endpoints.
Refs #419
Third piece of #419. BasicAuthAdmin() mirrors BasicAuthMgmt but reads the
live AdminAreaAuth mode and credentials on every request instead of
capturing them once at router-setup time, so toggling the Settings-UI
switch takes effect immediately.
Split mountSetupAPI into mountSetupAPIShared (ca.crt, tts/speak, tts/config
— used directly by soundtouch-cli and soundtouch-player, must stay reachable
regardless of the gate) and mountSetupAPIAdmin (everything else). Wired the
gate around /admin and both mountSetupAPIAdmin mounts (/setup, /api/setup).
Stockholm's optional legacy setup wizard is intentionally left out of scope.
Also fixes two lint issues introduced in the prior commit (unchecked
json.Marshal in tests, HandleUpdateSettings over the cyclomatic complexity
threshold) since `make lint` wasn't run before that commit landed.
Refs #419
Second piece of #419: a generic, local-only, append-only activity log
(datastore.RecordActivity/GetActivityRecords, one file per event under
stats/activity/<kind>/, same shape as SaveUsageStats) meant to back the
upcoming announcement-banner dismissals and be reusable for other admin-UI
action kinds later.
The read path never touches disk: a scoped startup scan folds prior
dismissals into an in-memory map once, RecordDismissal updates it
write-through. Same id can recur with a new timestamp (re-shown, dismissed
again) — it's a log, not a keyed store.
Not wired to anything user-facing yet — no announcements exist to dismiss.
Refs #419
First piece of the #419 admin-area gate: a persisted, live-reloadable
tri-state setting ("" unset / "enabled" / "disabled") so a later release
can flip the default from open to gated without breaking an explicit
opt-out. Rejects enabling while MGMT_USERNAME/MGMT_PASSWORD are still the
published default, since that would give a false sense of security.
No behavior change yet — nothing reads this field to actually gate
anything. That's the next chunk.
Refs #419
The placeholder state is expected and harmless (every code path already
treats it safely, and it self-heals once language/provider settings are
saved), but the old wording read like a file-corruption error and leaked
the internal account.json filename to users. Reword it to explain the
actual (benign) state instead.
Refs #360
HandleUpdateSettings and HandleUpdateLoggingSettings each built a fresh
datastore.Settings{} from scratch before saving, so any field not covered
by that handler's own DTO (e.g. hand-edited trust_forwarded_headers /
trusted_proxy_cidrs) was silently reset to its zero value on every save.
Load the persisted settings first and overlay only the fields each
handler actually owns, matching the pattern already used elsewhere
(addMargeHostToTLSFix).
Fixes#589
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
## What
Adds a new `runtime_bmx_url_stale` health check to `soundtouch-service`.
For each reachable speaker it reads the **runtime** `bmxRegistryUrl`
(from the on-device `SoundTouchSdkPrivateCfg.xml` via SSH, or `getpdo
CurrentSystemConfiguration` over telnet) and warns when it still points
at the shut-down Bose cloud, offering a copy-paste re-migrate command.
## Why
Radio source types (TUNEIN / RADIO_BROWSER / LOCAL_INTERNET_RADIO) are
delivered to the speaker through the BMX registry. A speaker whose
runtime `bmxRegistryUrl` still names the Bose cloud can never mount
them, even though the service's own `/sources` listing is correct. The
existing `sources_xml_diff` check reports the *symptom* ("missing 3
source types"); this check reports the *per-device cause*, so an
operator sees exactly which speakers still need re-migrating.
This is the recurring "radio missing after migration" theme (relates to
#549, #547, #546, #493). In the #549 diagnostic, 6 of 9 speakers had
never actually been migrated (all four runtime URLs still on
`content.api.bose.io` / `streaming.bose.com`) while the service itself
looked healthy; this check would have surfaced that per device
immediately.
## False-positive guard
Under a DNS-based migration (AfterTouch acting as the speaker's DNS
server) a cloud URL is legitimate: the redirect happens at the DNS
layer, not by rewriting the on-device URL. So the check stays silent
while the service's own DNS interception is running (`GetDNSRunning`).
The router-DNS variant (the LAN's DNS points at AfterTouch without our
DNS server running) cannot be detected here, so it is called out as a
known exception in the finding text rather than suppressed.
## Changes
- `pkg/service/health/checks_runtime_bmx_url.go` (+ unit test): the
check, following the injected-closure pattern of `checks_marge_url.go`.
`assessRuntimeBmxURL` is the pure, testable core; `isBoseCloudHost` does
a domain-suffix match on the Bose cloud domains.
- `pkg/service/handlers/handlers_export.go`: a lightweight
`readSpeakerBmxRegistryURL(ip)` reader (SSH then telnet), reusing the
existing export imports. The diagnostic export path itself is unchanged.
- `pkg/service/handlers/server.go`: registers the check, wiring
`GetDNSRunning` as the guard.
The health package stays free of the SSH / `setup` imports (the reader
lives in the handlers layer), matching the existing dependency boundary.
## Testing
- `go test ./pkg/service/health/` green (new tests: cloud URL warns;
AfterTouch URL and empty URL do not; `isBoseCloudHost` matrix).
- `go vet` and `golangci-lint` clean on both packages; `go build
./cmd/soundtouch-service/` succeeds.
Not tied to a single issue to close; it complements the #549 / #547 /
#546 / #493 cluster as a diagnostic aid.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
## Summary
- #269's stuck Spotify presets traced to the account link never
completing, invisible because the admin UI silently swallowed 401s from
`/api/mgmt/*` instead of prompting for a retry. The browser's own Basic
Auth caching already works correctly here (verified live); the bug was
purely missing feedback.
- Adds two Health-tab checks: Spotify configured but no account linked,
and Management API credentials still at the published default.
Refs #269, #419.
## Test plan
- [x] `make check` (fmt, vet, unit tests) clean
- [x] `make lint` clean
- [x] Live-tested end to end with a headless Chrome (chromedp) against a
local build: confirmed the new failure-path messages render correctly
for `fetchSpotifyStatus`, `fetchAccountList`, and `linkSpotify`
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Relates to #571.
## What
Adds a **Name / IP** sort toggle to the Player device list. The choice
is persisted in `localStorage` (`aftertouch_device_sort`), following the
same preference pattern as the service-URL field in `PlayURL.js`.
## Why
The device list was previously ordered only by IP: the service datastore
keys devices by IP address and Go marshals map keys lexicographically,
so the frontend received an already-IP-ordered object and rendered it
as-is. BirdyBA (#571) asked to be able to sort by name instead.
## Changes
- `DeviceList.js`: a `sortEntries()` helper plus a `useState`-backed
toggle seeded from `localStorage`. Name mode sorts by `device.info.name`
(falling back to the IP key when a device has no name yet); IP mode
sorts the IP key **numerically** (`.2` before `.10`), which also tidies
the old lexicographic ordering.
- `css/app.css`: additive `.device-sort` / `.sort-btn` styling, reusing
the existing accent / `.active` look. No existing rules touched.
No backend change: the device name and IP are already in the payload.
## Testing
- `node --check` on `DeviceList.js` passes.
- `make build-player` succeeds (the static tree is `//go:embed`ed into
the binary).
- Manual: open the Player, toggle Name / IP, confirm the order changes
and the choice survives a page reload.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Follow-up polish to the HTTPS-URL settings work:
- Group Target Domain and the derived HTTPS URL under a single bold
"Service URLs" section header, matching the existing section-header
pattern (Landing page, TLS extra hosts, Device Discovery) rather than a
one-off fieldset box — consistent across the whole Settings tab.
- Tighten the spacing so the HTTPS URL sits with Target Domain (drop the
empty :443-status reserved line, reduce the intra-group gap) instead of
floating toward the next section.
- Make "Landing page" a bold header for the same consistency.
- Expand the HTTPS URL override hint: HTTPS is only needed for certain
features (DNS redirect, Spotify/Amazon login, cert trust); it derives
from the Target Domain; and if you don't need plain HTTP you can set the
Target Domain itself to an https:// URL, no override required.
refs #355
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two follow-ups to the derive/show/override settings work:
- The "advanced override" affordance reused the .info-toggle style with a
text label, which is an 18px circular icon badge — the label rendered as
a broken blue circle. Use the icon-toggle pattern like TLS extra hosts:
a small ⓘ that reveals a details block containing the explanation and the
override input.
- Existing installs persist their old effective HTTPS URL in the (now
override) https_server_url field, so the UI showed "(override)" even when
the value equals what we would derive. On load, treat an override that
exactly matches the derived URL as "derive" (clear it), so default
installs show "(derived from Target Domain)"; genuinely custom values are
kept as overrides.
Verified live: an existing settings.json with https_server_url equal to the
derived value now reports an empty override, and the served admin HTML uses
the ⓘ toggle.
refs #355
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The diagnostic export wrote the raw persisted https_server_url, which is
now the override (empty when the URL is derived). Report the effective
HTTPS URL actually in use plus the override as a separate field, so a
diagnostic makes an advertised-URL/listener mismatch legible instead of
showing an empty field.
refs #355
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The HTTPS URL AfterTouch advertises (and points speakers at for the
DNS-redirect, OAuth, install-ca and cert-trust flows) was a separate,
internally-tracked value: sourced only from --https-server-url /
HTTPS_SERVER_URL / the settings file, defaulting to the machine hostname,
and never shown or editable in the web UI. So it could silently diverge
from the Target Domain (e.g. a different host, or a port-less value that
fell back to 443 while the listener was on 8443 — the root of #355), with
no way to see or fix it in the UI.
Make it derive + show + override:
- DeriveHTTPSURL resolves the effective HTTPS URL: an explicit override
wins; otherwise it follows the Target Domain (same host, https, on the
configured HTTPS port); an already-https Target Domain is honoured
verbatim (its port is not second-guessed); empty falls back to the
hostname default. So changing the Target Domain updates the HTTPS URL
automatically for the common single-host case.
- The persisted https_server_url is now the *override* (empty = derive).
Existing installs carry their old value here, so it is preserved as an
override — no silent change on upgrade; clearing it opts into derive.
- The server keeps httpsServerURL as the effective value, so all
consumers (cert SANs, migration, export, health) are unchanged; it is
recomputed whenever the Target Domain or override changes.
- Settings API returns https_server_url (effective) plus
https_server_url_override; the Settings page shows the effective URL
with a derived/override note and an "advanced" override field.
Verified live on a clean data dir: derive from an http Target Domain,
auto-follow when the Target Domain changes, explicit override, an https
Target Domain kept verbatim, and override persistence across restart.
Unit tests cover DeriveHTTPSURL including the already-https cases.
refs #355
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Follow-up to the previous commit. The cert-chain check dialed the
advertised HTTPS URL, whose port defaults to 443 when the URL omits it
(splitHTTPSHostPort). The advertised URL comes from
--https-server-url / HTTPS_SERVER_URL / the settings file and is not
editable in the web UI, so when it lost its port it silently pointed the
check (and speakers) at 443 while the real listener was on 8443 — the
exact "port 443" complaint in issue #355.
Thread the actual HTTPS listener port into the check (new
Server.SetHTTPSListenAddr, wired from config.httpsAddr). When the dial
fails and the advertised port differs from the listener port, emit a
mismatch-specific warning that names both ports and offers the corrected
HTTPS_SERVER_URL, while still deferring to reverse-proxy setups. A
reachable endpoint never reaches this branch.
Reproduced locally on a clean data dir: seeding a port-less
https_server_url with the listener on 8443 previously errored on
:443; it now warns with both ports and the fix. Regression tests added.
refs #355
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The `service_cert_chain` check ("HTTPS endpoint TLS configuration") dials
the service's own configured HTTPS URL. When that dial fails before any
certificate is presented (connection refused, timeout, handshake reset),
it reported a hard red error.
But from inside the service we can't distinguish "the endpoint is down"
from "the advertised HTTPS URL simply isn't reachable from here" — and
the latter is a normal, healthy deployment: TLS terminated by a reverse
proxy in front of AfterTouch, or a Docker-published port / LAN-only
hostname that the container itself can't dial. In those setups the red
error is a false alarm (issue #355: reporter runs HTTP 8080 / HTTPS 8443
and noted "in my configuration that is expected").
Downgrade that specific case (no cert presented) to a warning, reword it
to name the expected reverse-proxy / unreachable-advertised-URL case, and
add an `openssl s_client` command to verify the endpoint from a client
that actually reaches the advertised URL. Cert-classification outcomes
(own-CA info, foreign-chain warning) are unchanged.
Reproduced locally on a clean data dir before/after: custom ports and
localhost/127.0.0.1 already returned INFO; only the unreachable-URL case
produced the error, which now returns a warning.
refs #355
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Update the HTTPS reverse-proxy guide and the trust_forwarded_headers /
trusted_proxy_cidrs settings comments to reflect that the client IP is now
resolved from X-Forwarded-For only (no longer X-Real-IP / True-Client-IP),
read via the request context rather than by rewriting r.RemoteAddr. The
nginx example now sets X-Forwarded-For.
(Release note staged locally at _/releases/v0_117_0.md, which is gitignored
like prior release notes.)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
chi v5.3.0 deprecates middleware.RealIP (IP-spoofing advisories), which
failed the Lint and Static Security Analysis CI jobs (SA1019). Replace the
RealIP wrapper with chi's middleware.ClientIP: ClientIPFromRemoteAddr is
always applied so middleware.GetClientIP is populated, and when
trust_forwarded_headers is set and the immediate peer is a trusted-proxy
CIDR, ClientIPFromXFF resolves the real client from X-Forwarded-For
(rightmost entry outside the trusted CIDRs). The immediate-peer trust gate
is preserved, so a non-trusted peer's XFF is ignored. CIDR strings are
validated with netip.ParsePrefix first to avoid ClientIPFromXFF's panic.
Behavior change: only X-Forwarded-For is honored now (RealIP also read
X-Real-IP / True-Client-IP). Docs and a release note follow.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Route every HTTP read of the client IP through a single clientHost(r)
helper backed by chi's new middleware.GetClientIP, falling back to the
socket peer from r.RemoteAddr. AddDeviceToAccount now takes a bare client
host instead of a "host:port" RemoteAddr. Behavior is unchanged in this
commit (no ClientIP middleware is wired yet, so the fallback is always
taken); a follow-up wires middleware.ClientIP and removes the deprecated
middleware.RealIP.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The check title asserted "Speaker /sources matches service Sources.xml",
but the row renders as a warning when they differ, so "matches" plus a
warning read as a contradiction (reported in #493). Reword to "should
match" so the title states the expectation; the per-finding messages and
severities already convey whether it holds and what the differences are.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Per-device health findings previously labelled the device by account
and device IDs only (e.g. "account 3230304 · device 08DF1F0BA325"),
which is hard to place at a glance. Add display-only Name and IP fields
to health.Target and fill them centrally via EnrichTargets after the
checks run, so individual checks don't each have to look up the device
record. Both the live health endpoint and the diagnostic export go
through the new Server.runHealthChecks helper, and the Health tab renders
the friendly name first, then account/device IDs, then IP.
Fixes match on Account+Device only, so the new fields don't affect
quick-fix dispatch.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds a third way to push a clip to a speaker, surfaced by @dagrider in
#517: POST SetAVTransportURI + Play to the speaker's UPnP MediaRenderer
control endpoint (port 8091). Unlike /speaker play_info it needs no
app_key and no DNS interception, so it works on a plain LAN; the
trade-off is it switches the speaker to the UPNP source and replaces the
current playback (no duck-and-resume).
- pkg/client: SetAVTransportURI, AVTransportPlay, PlayURLViaUPnP (+ the
:8091 control-URL derivation and SOAP plumbing), with tests.
- cmd/soundtouch-cli: `speaker url-upnp --url <url>`.
- docs: document the UPnP/AVTransport option under POST /speaker.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Delete RemoveMember, ClearMembers and HasMember (deprecated in the
previous commit) plus their tests. They had no production callers after
the zone remove paths moved to /removeZoneSlave.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
RemoveMember, ClearMembers and HasMember have no production callers: the
zone remove paths now use /removeZoneSlave instead of a /setZone rebuild,
and standalone is done by dissolving the zone. Mark them Deprecated ahead
of removal in the next commit (keeps history legible).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Removing one member from a multi-member zone did nothing. The remove
paths rebuilt the zone with /setZone and the remaining members, but
/setZone is additive: it never drops a member that is simply absent from
the list. It only "removed" when the resulting set was empty (equivalent
to dissolve), which is why removing the last member worked but removing
one of several did not.
Switch all three remove paths to the dedicated /removeZoneSlave endpoint
(already implemented as client.RemoveZoneSlave):
- HandleZoneRemove (web UI "remove member")
- HandleZoneLeave (web UI slave "leave zone")
- RemoveFromZone (client lib, used by CLI `zone remove`)
DissolveZone (setZone master-only) and HandleZoneAdd (additive setZone)
are correct and unchanged. Adds handler regression tests for remove/leave
and rewrites TestClient_RemoveFromZone to assert /removeZoneSlave (the old
test removed one of two members but only checked that setZone was called,
never that the member was dropped).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
play_info notifications make the speaker validate the app_key via
GET /v1/auth against a hardcoded Bose host; without DNS interception that
call can't resolve and /speaker times out with ALLEGROWEBSERVER_TIMEOUT
(1046). Document the requirement on POST /speaker (plus the no-DNS
LOCAL_INTERNET_RADIO alternative), and have the "Test DNS path" health
check mention that TTS/play_info depends on the same DNS path.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
checkCACertTrusted matched only the static "# AfterTouch" label in the
device's trust bundle. After the service CA was regenerated (e.g. a
recreated container with a fresh/empty data dir), the stale label was
still present, so the migration wrongly reported the speaker as already
trusting the new CA and skipped re-installing it, leaving the speaker
unable to validate TLS to the service.
When the service CA is available, compare the actual cert payload and
re-install on mismatch; fall back to the label only when the CA can't be
read (CLI callers without Crypto). Adds regression tests for the
stale-label and no-Crypto cases.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
After an in-place migration the firmware sometimes does not activate the
radio source types (LOCAL_INTERNET_RADIO, TUNEIN, RADIO_BROWSER) even
though the entries are present in the device's own Sources.xml; a reboot
and a sourcesUpdated notification do not help. The root cause is not yet
understood, so this documents the user-confirmed workaround (factory
reset + re-migrate) rather than changing migration behaviour:
- New troubleshooting section with a stable anchor, linked from the
sources_xml_diff health check and the Radio Browser reference.
- Capture the speaker's on-device /mnt/nv/BoseApp-Persistence/1/Sources.xml
in the diagnostic export (when SSH is available), so a future report
taken before a factory reset carries the evidence to pin down the cause.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
