Refreshes docs/images/ui-{settings,devices,sync,migration}.png by
driving the web UI in chromedp against a synthetic speaker, so
documentation can be regenerated without real hardware and without
leaking personal data from the local network.
Three independent pieces:
- pkg/service/testing/fakespeaker — embeddable library serving the
HTTP and telnet surface the migration wizard probes (/info,
/presets, /recents and a getpdo CurrentSystemConfiguration reply
that places the device on the unmigrated happy path).
- cmd/dummy-speaker — thin CLI wrapping the library; self-registers
with a running service via POST /setup/devices.
- scripts/screenshots — chromedp runner driven by a JSON manifest;
decoupled from speaker/service setup so it can target any backend
URL. run.sh orchestrates a one-shot end-to-end capture and seeds
settings.json with a generic hostname plus discovery disabled to
keep real-network state out of the captures.
Captures are at DPR=2 for retina-sharp text.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Hard-deletes everything marked DEPRECATED in the previous commit:
Files:
- pkg/service/setup/telnet_probe.go
- pkg/service/setup/telnet_probe_test.go
- pkg/service/handlers/handlers_telnet_probe.go
- pkg/service/handlers/probe_registry.go
- pkg/service/handlers/probe_registry_test.go
Edits:
- Server.probes field + initialization (server.go).
- Routes /probe/{token}, /probe/{token}/*, and
/setup/telnet-probe/{deviceId} (main.go).
- checkTelnetRoundTrip() in script.js.
The passive observer (peer_probe.go + handlers_peer_probe.go) is now
the only reachability check for migrated speakers; unmigrated/partial
states surface a skip row pointing at the Apply + reboot cycle, as
documented in TELNET-MIGRATION-METHOD.md §9.8.
isCommandNotFound and parseGetpdoConfig remain — they are used by
telnet_migration, telnet_preflight, marge_pairing, and
preflight_crosscheck.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The swUpdate daemon caches its target URL at boot and ignores live
`sys configuration` writes, so the active flip in
RunTelnetRoundTripProbe never reaches the running daemon — confirmed
empirically on a fully-migrated speaker (FW 27.0.6) where both the
runtime and persistence layers were flipped and the device still
dialed the previously-cached `/updates/soundtouch` URL plus
DNS-intercepted `/streaming/software/update/account/*`. The probe URL
was never observed.
Marks DEPRECATED:
- pkg/service/setup/telnet_probe.go: ProbeRegistrar,
TelnetProbeResult, generateProbeToken, RunTelnetRoundTripProbe.
- pkg/service/handlers/handlers_telnet_probe.go: HandleTelnetProbe,
HandleProbeInbound, telnetProbeTimeout, telnetProbeResponse.
- pkg/service/handlers/probe_registry.go: probeRegistry.
- Server.probes field.
- /probe/{token}[/*] and /setup/telnet-probe/{deviceId} routes.
Adds §9.8 to docs/analysis/TELNET-MIGRATION-METHOD.md documenting the
daemon-cache finding, the diagnostic that confirmed it, the passive
observer replacement, the pre-flight branch on migration state, and
the canonical telnet flow (Apply config → reboot → passive
validation). All code symbols remain in place this commit; the
follow-up commit performs the hard delete.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The pre-flight panel's reachability check now picks one of two paths
based on summary.is_migrated:
- Migrated → run the new passive peer-reachability probe
(POST /setup/peer-probe/{deviceId}) and label the row
"Reachability check (passive observer)".
- Not migrated (incl. partial) → render a skip row
"Round-trip validation runs after Apply + reboot" with the
rationale "daemon caches swUpdateUrl at boot". Per-axis state
remains visible in the State card so the user sees which parts
are already in place.
Adds checkPeerReachability() alongside checkTelnetRoundTrip(). The
latter is marked DEPRECATED inline — no longer called by the
orchestrator, scheduled for removal in a follow-up commit.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
RunPeerReachabilityProbe is the post-migration replacement for the
active swUpdateUrl round-trip: register the device IP with the
in-process observer, nudge :8090/swUpdateCheck, and wait for any
inbound from that IP. No device-state mutation. Any inbound counts
as proof — on a migrated speaker, DNS interception routes the
daemon's outbounds through this service regardless of which URL it
resolved internally, so reachability reduces to "did the device
dial us at all."
PeerHit and the abstract observer interface live in setup alongside
the probe logic; handlers.peerObserver implements the interface and
the existing observer files now import from setup.
Route: POST /setup/peer-probe/{deviceId}. Timeout: 30s, surfaced as
result.ElapsedMs so the budget can be tuned from real data. The
pre-flight orchestrator gains the branch in the next commit.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds an in-process observer that records device->service requests by
source IP. PeerObserverMiddleware fires on every inbound after RealIP
trust and Recoverer; the registry exposes Register/Signal/Forget keyed
on the device IP with a buffered one-shot delivery.
No callers yet — this is the substrate for the passive reachability
probe that replaces the broken active swUpdateUrl round-trip on
migrated speakers.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The speaker decodes /streaming/account/.../full into a protobuf message where
recents>recent>source>sourceproviderid is a required field. A laut.fm recent
(location "/custom/v1/playback/...") POSTed against an account with no
Sources.xml fell into classifyLearnedSource's default branch, which wrote
sourceKey type="INVALID" with no providerid. That entry then re-appeared
in /full with an empty <sourceproviderid> element, which the post-marshal
strip-empty step deleted entirely — aborting the speaker's account sync
with "MargePB.account.devices.device[N].recents.recent[K].source.sourceproviderid"
missing and forcing a 60-second retry loop.
Three changes, each defended by the new regression test:
* classifyLearnedSource recognises LocalInternetRadio via sourceProviderID
== 11 and via the /custom/v1/playback/ URL pattern, and stops writing the
"INVALID" sentinel that locked sources out of every read-side repair path.
* mapToFullResponseSource falls back to the canonical SourceProviderID
keyed by source ID (10002/10003/10004/10005) so already-poisoned data
on disk still renders a non-empty providerid at /full time, with no
manual data scrub required.
* AccountFullToXML no longer strips empty <sourceproviderid> elements.
The strip-empty was added for parity with upstream's standalone <sources>
block, but it's wrong inside recents/preset source blocks where the field
is protobuf-required.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
"Test first, decide later" affordance: the same check sequence Apply
runs is now reachable without committing to the migration. Useful
for spot-checking a speaker after editing URLs, or for verifying a
fresh device is reachable before the user commits to writing
anything.
Two buttons, one per Apply path:
- #plan-preflight-btn (Suggested Plan side) — reads the chosen
method from plan-apply-btn.dataset.method, same source the
real Apply uses, so what's tested matches what would be
applied.
- #customize-preflight-btn (Custom Plan side) — walks the same
radio choices applyCustomPlan reads and builds the same
methods array, then runs the checks against it.
Both share the existing pre-flight panel and runApplyPreflight
orchestrator. New renderPreflightPreviewSummary terminates the
panel with a single Close button instead of Proceed Anyway /
Cancel — there's nothing to proceed to in preview mode.
Both Pre-flight buttons share the disabled-state gate of their
Apply counterparts (no plan / invalid URLs disables both) so users
can't accidentally pre-flight a plan that wouldn't apply.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Previously the SSH-capable branch and the telnet-only branch were
mutually exclusive — speakers with both transports reachable only
got the curl-from-device HTTPS check, never the round-trip probe.
That left a class of bugs invisible to pre-flight: an asymmetric
network path where the speaker's userspace can reach our service
(curl works) but the swUpdateUrl fan-out can't (or vice versa).
Each transport now gets its own check; both run when both are
reachable. The two exercise meaningfully different code paths in
the speaker:
- SSH curl-from-device: speaker's normal userspace HTTP stack
over an arbitrary inbound TCP to our HTTP/HTTPS port.
- Telnet round-trip: speaker's firmware-internal swUpdateCheck
fan-out, which writes to its own DNS resolver and outbound
HTTP code path that the curl test doesn't go near.
A speaker that passes one and fails the other reveals a real
connectivity asymmetry worth surfacing before the migration
writes its target URLs.
Cost: ~1s extra on the success path (probe is fast on healthy FW
27.0.6), up to ~6s extra on the timeout path. The probe restores
the runtime swUpdateUrl unconditionally so there's no lingering
state regardless of outcome.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The landing-tab overview still framed SSH as a hard prerequisite —
"Migration requires SSH access." That was true under the original
design, but the wizard now probes both SSH and Telnet:17000
automatically and uses whichever the device exposes. SSH-less
speakers (USB-unlock-refusing firmware like SA-5, ST520, recent ST
Portables) can migrate over telnet without ever opening a shell.
Updates:
- Prerequisite box retitled "Speaker shell access" with two
sub-bullets that match the state card's Transports row:
* SSH — richest option, required for XML / DNS / CA install,
same USB-stick procedure as before
* Telnet:17000 — SSH-less fallback, no setup, HTTP-only
- Step 1 (Settings) now mentions that Target URL can be edited
inline on the Migration tab with Save as default, since the
Settings tab is no longer the only place to set it.
- Step 4 (Migration) replaces "we recommend the XML Configuration
method" with a description of the actual wizard: Apply
Suggested Plan, Customize three-axis form, and the visible
pre-flight check.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The pre-flight connection check always hit summary.server_https_url
(the HTTPS health endpoint), regardless of what URL the migration
would actually write to the speaker. That gave a useful baseline
("can the device reach our service over HTTPS at all?") but didn't
test the right thing for HTTP-target migrations — the dominant
configuration when SSH is available and the user goes with the
Suggested Plan's XML+HTTP default.
preflightConnectionTestURL now picks the test URL by intent:
- methods.includes("resolv") → server_https_url. DNS interception
leaves the device hitting https://*.bose.com (firmware-hardcoded
scheme) which DNS redirects to our HTTPS endpoint; testing the
health URL is the right shape.
- URL-flip methods (xml / telnet) → derived from the user's
targetUrl: scheme + host + "/health". HTTP-target migrations get
an HTTP test, HTTPS-target migrations get an HTTPS test (still
with use_explicit_ca=true so the trust path is forward-looking
when CA install is part of the plan).
- Fallback to server_https_url when targetUrl can't be parsed, so
older call shapes keep working.
The row label is now dynamic: "HTTPS connection from device" or
"HTTP connection from device" depending on the actual test scheme,
so the panel tells the user which path is being exercised.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The /etc/hosts migration method has been hidden from the UI since
before the wizard refactor — the Customize three-axis form doesn't
expose it, the suggested-plan engine never picks it, and
onCustomizeChange explicitly force-hides the legacy
#hosts-redirection-test pane. The pane was sitting in the DOM doing
nothing.
Removed:
- The hosts-redirection-test <div> (button, result pane, header)
- test-hosts-btn.onclick wiring in showSummary
- The testHostsRedirection() function (orphaned once the button is
gone)
- The show("hosts-redirection-test", false) toggle in
onCustomizeChange (orphaned once the pane is gone)
Backend untouched:
- /setup/test-hosts/{deviceId} and HandleTestHostsRedirection still
exist for API back-compat. Same pattern we used when retiring the
XML method's self/proxied/original dropdowns — only the UI
surface moves; the manager-level entry points stay.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two lint findings flagged by golangci-lint:
- telnet_probe.go:90 — t.Dial()'s local err shadowed the outer
url.Parse error (govet shadow). Renamed the inner one to
dialErr.
- migration_summary_telnet_test.go:20 — telnetSummaryEnv didn't
call t.Helper(), so test failures pointed at the helper rather
than the calling test (thelper). Now mirrors the t.Helper() in
telnetSummaryEnvWithInfo.
No behaviour change.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The Telnet method pane still said "After a successful migration a
Pair Account panel will appear below this one" — but pair-account-pane
was removed three commits ago when pairing was folded into the Plan
card as a configured-up-front step that runs as part of Apply. The
note pointed users at a panel that no longer exists.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces the placeholder "skip — telnet round-trip probe not yet
implemented" branch with an actual call to POST /setup/telnet-probe
when SSH is unreachable but Telnet:17000 is. SSH-less speakers now
get real reachability verification before any migration step runs,
instead of being silently ignored by the pre-flight pipeline.
Decision tree for the reachability check:
- SSH reachable → HTTPS connection test from device (existing)
- Telnet:17000 only → Telnet round-trip probe (new)
- neither → skip with "no transport reachable" message
The probe row reports its result inline with the existing pre-flight
panel idiom (🕐 / ⟳ / ✅ / ❌), surfacing elapsed_ms on success so
users see how long the round-trip took. Failure messages from the
backend (timeout, sys configuration rejected, dial refused) propagate
verbatim so the user knows which step of the orchestration tripped.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Fills the SSH-less gap the curl-from-device test leaves in the
pre-flight panel: instead of skipping connectivity verification on
USB-unlock-refusing speakers, we drive a round-trip from the device
itself using only telnet:17000 and the device's own :8090 API.
Sequence (Manager.RunTelnetRoundTripProbe):
1. telnet `getpdo CurrentSystemConfiguration` — capture the
speaker's current swUpdateUrl so we can restore it.
2. Generate a random hex token; register a one-shot signal
channel under it via the new probeRegistry on Server.
3. telnet `sys configuration swUpdateUrl <targetURL>/probe/<token>`
— runtime layer only, no envswitch boseurls set, so the
persistence layer keeps the original and a reboot heals the
device naturally if our restore step fails.
4. HTTP GET :8090/swUpdateCheck — the cleanest :8090 endpoint
that triggers exactly one outbound to the configured
swUpdateUrl. Read-only on the cloud side, doesn't depend on
margeAccountUUID, doesn't start an actual update.
5. Wait on the registered channel up to telnetProbeTimeout (6s).
6. telnet `sys configuration swUpdateUrl <original>` — restore
in a deferred call so it runs even on the failure path.
New /probe/{token}[/*] catch-all on the root router signals the
matching channel when the speaker's outbound lands; the response is
a minimal `<swUpdateIndex/>` so the device's swUpdateCheck doesn't
choke on a missing structure. The {token}/* sub-path is registered
because some firmware appends a path component to the configured
swUpdateUrl.
POST /setup/telnet-probe/{deviceId}?target_url=… exposes the
orchestrator as a single REST call returning {ok, result: {reached,
restored, original_url, probe_url, elapsed_ms, logs}, error?}.
Tests cover: happy path with channel signalled by the fake registrar
when the :8090 trigger fires, timeout when no inbound arrives,
abort when getpdo doesn't expose swUpdateUrl, abort when the
firmware rejects sys configuration, dial failure, invalid target URL.
Frontend wiring (visible pre-flight panel) lands in the next
commit.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pairing was previously its own post-telnet pop-up pane —
loadAccountIDSuggestions(deviceId) was called only after a successful
telnet migration, leaving the user to interact with a separate panel
and click a separate "Pair Account" button. XML migrations didn't
surface pairing at all.
The Plan card now has its own Account pairing section between Service
URLs and Suggested plan, with the same affordances (current state,
7-digit input, Generate button, datastore picker) but always
visible. The implicit intent — read by readPlanPairTarget — is:
- empty input + currently paired → no pairing step (current ID kept)
- empty input + currently unpaired → no pairing step (warning hint visible)
- input matches summary.account_id → no pairing step
- input is exactly 7 digits, differs → pair step queued at Apply
- input is non-empty but malformed → blocks Apply with a clear error
Both Apply orchestrators (applySuggestedPlan, applyCustomPlan) now
queue a `pairAccount(deviceId, accountId)` call when the intent says
to. It runs *after* the URL flip / DNS / CA steps so the user sees
the migration succeed before pairing — pairing is independent of
the migration target so order is purely UX. First-failure-aborts is
preserved: a pair-account error stops the rest of the sequence.
Removed:
- #pair-account-pane HTML and all its descendants
- loadAccountIDSuggestions / generateAccountID / pairAccount(deviceId)
(the old pane-bound functions)
- the "if method === telnet → loadAccountIDSuggestions" trigger in migrate()
Added:
- renderPlanPairing(summary, deviceId) — populates the section on
every showSummary
- loadPlanAccountSuggestions(deviceId) — fetches /setup/account-id-
suggestions; gracefully degrades on failure
- onPlanPairIDChange / onPlanPairPick / generatePlanAccountID — UI
handlers with implicit-intent status hints
- readPlanPairTarget — orchestrator-facing intent extractor
- pairAccount(deviceId, accountId) — POSTs and throws on failure
(replaces the old pane-bound function with a step-friendly shape)
- resetPlanCardForDeviceSwitch clears the pairing input on speaker
change so the previous device's ID can't leak
Backend untouched — all the pairing endpoints (/setup/account-id-
suggestions, /setup/pair-account) and the setup.PairAccount + telnet-
fallback logic stay exactly as-is.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces the silent confirm()-dialog pre-flight with an inline panel
that pops up the moment Apply is clicked, walks through each
applicable check live, and surfaces the result before any backend
operation touches the speaker.
Three checks run in order:
1. Backend summary re-check (always) — the existing
runPreflightCheck logic, repackaged as the first row in the
panel. Catches transport/resolve_ip drift since the cached
summary loaded.
2. HTTPS connection from the device (when SSH is reachable) —
reuses /setup/test-connection with use_explicit_ca=true so the
test exercises the trust path even when CA install is part of
the plan. Identical to the manual "Test with Explicit CA.crt"
button under HTTPS Connection Test, but runs without requiring
the user to click it. SSH-less devices show a "skip" row with
a note pointing at the future telnet round-trip probe.
3. DNS redirection from the device (only when resolv is in the
plan and SSH is reachable) — reuses /setup/test-dns. Same
parity as #2 with the manual "Test DNS Redirection" button.
UX:
- Each check renders with 🕐 pending → ⟳ running → ✅ ok / ❌
fail / — skipped, so the user sees feedback while the backend
works.
- On all green: a 700ms hold lets the success state register, then
Apply auto-proceeds.
- On any red: a "Proceed Anyway" / "Cancel" pair appears; default
is to abort, but the user can override on a known false-positive.
Both Apply paths (applySuggestedPlan and applyCustomPlan) now share
runApplyPreflight and awaitPreflightDecision; the unused
confirmPreflightIssues helper is removed.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The Plan-card preview is now optimistic and renders client-side on
every keystroke (previous commit), so the view can drift from what
the backend would actually do — at least until the next summary
fetch. Runtime state can also drift between the cached summary the
user is looking at and the moment they click Apply (a transport
goes down, DNS hostname stops resolving, etc).
Adds runPreflightCheck which both Apply paths call once before
kicking off any backend operation:
- applySuggestedPlan calls it with the single chosen method.
- applyCustomPlan calls it with the full list of operations the
sequence will run (flip method, optional resolv, optional
trust-ca) so the SSH/Telnet reachability requirement is checked
against the actual fresh summary, not the stale cached one.
The check covers four classes of inconsistency:
- resolve_ip_error from the device's perspective
- SSH reachable when xml / resolv / trust-ca is queued
- Telnet:17000 reachable when telnet is queued
- The backend's planned_config XML contains every per-field URL
override we're about to send (sanity check that the client's
optimistic preview agrees with the server's render before we
write to the speaker)
On any issue, confirmPreflightIssues shows them in a confirm()
dialog so the user can override on a known-false-positive (slow
DNS, etc.) but the default is to abort.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two related fixes for the Plan-card → Customize-pane preview flow:
1. Live planned-XML preview. The Customize panel's "Planned Config
(AfterTouch)" pane previously showed summary.planned_config —
server-rendered, only updated on the next showSummary fetch. So
editing a URL field in the Plan card had no visible effect on the
preview until the user manually refreshed. The new
renderPlannedXMLPreview composes the same XML client-side from
plan-target-url + the four override inputs, mirroring exactly what
migrateViaXML writes (target-derived defaults + applyURLOverrides),
and is called from validatePlanURLs which already runs on every
keystroke.
2. Per-device form-state isolation on speaker switch. The Plan card
inputs preserve manual edits across summary refreshes (force=false)
so a user's typed URL doesn't get clobbered by a re-fetch. That
semantic is right within one device but wrong across devices: if
the user edited a URL on speaker A and then picked speaker B in
the dropdown, A's value silently appeared in B's preview.
showSummary now compares the previous summary-device-id to the new
one and, on change, calls resetPlanCardForDeviceSwitch to clear
the four URL inputs, the Soundcork checkbox, the "saved" hint
dataset, the URL-validation banner, and both apply-status lines.
The downstream fillPlanURLInputs(defaults, force=false) then fills
the now-empty inputs with the new device's canonical defaults.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
With XML+resolv selected together, the bottom panes rendered as
"Current XML | Planned XML | Planned resolv hook" plus a separate
full-width "Current /etc/resolv.conf" block above — three panes plus
a hanger above, each pair scattered.
Restructured into two side-by-side .diff-container rows that each
pair their own Current/Planned columns:
- #xml-diff-row — Current Config (on Speaker) | Planned Config (AfterTouch)
- #resolv-diff-row — Current /etc/resolv.conf | Planned /etc/resolv.conf Hook
current-resolv-pane moved out of its standalone wrapper into the
resolv row. The deprecated #planned-hosts-pane is removed entirely
(hosts is no longer offered as a method, per the earlier UI cleanup).
onCustomizeChange now toggles the row IDs instead of per-pane IDs,
and uses display:"" rather than display:"block" so the .diff-container
flex layout isn't accidentally overridden.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces the migration-method dropdown and its toggleMigrationMethod
visibility logic with a unified three-axis form inside the Customize
details:
- URL flip transport: XML over SSH / Telnet (Port 17000) / Skip
- DNS interception: None / /etc/resolv.conf hook
- Local CA install: checkbox (SSH-only)
Each radio/checkbox has a transport-availability hint next to it
(e.g. "(SSH unreachable)" or "(already trusted)") so users see *why*
an option is disabled before they pick. renderCustomizeForm runs on
every summary load to recompute these hints and pick a valid initial
selection when the previous default isn't reachable.
applyCustomPlan orchestrates the chosen combination as a sequence of
existing backend calls:
- URL flip != none → POST /setup/migrate?method={xml,telnet}
- DNS = resolv → POST /setup/migrate?method=resolv
(already includes the CA install, so an explicit
CA step is skipped in that case)
- CA install only → POST /setup/trust-ca
Steps run in order; the first failure aborts the rest. After the
sequence completes, refreshSummary repopulates the state card.
migrate() now takes the method as an explicit parameter instead of
reading it from the dropdown; applySuggestedPlan and applyCustomPlan
both pass it directly. The legacy "Confirm Migration" button is
removed (Apply Custom Plan supersedes it). The reboot-method picker
now reads the URL flip radio rather than the dropdown.
The legacy per-method preview/test panes (xml-diff, planned-xml,
planned-resolv, current-resolv, dns-redirection-test) become
visibility-driven by the radio choices via onCustomizeChange instead
of the dropdown's toggleMigrationMethod (now removed). The hosts-
related panes are forced hidden — hosts is the deprecated method.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The Plan card's per-field URL editor now drives both XML and Telnet
migrations via the same marge_url / stats_url / sw_update_url / bmx_url
options, so the two duplicate places that used to set those values are
gone:
- The XML method's "Service Implementations" table (#service-options)
with its self/proxied/original dropdowns. The legacy options keys
(marge / stats / sw_update / bmx) stay accepted by the backend's
applyProxyOptions for any direct API user, but the UI no longer
sets them.
- The "URL Targets" sub-pane inside #telnet-method-pane with its
parallel set of telnet-marge-url / etc. inputs and its own
Reset-to-defaults button. The Telnet pane retains its
explanatory header and limitations note (no CA install, pairing
panel below) — only the duplicate URL editor is gone.
Stripped the now-dead JS:
- showSummary's #service-options visibility toggle and
parsed_current_config-driven population of orig-marge etc.
- showSummary's reads of opt-marge / opt-stats / opt-sw_update /
opt-bmx in the summary query string.
- migrate's reads of those same fields in the migrate query string.
- fillTelnetURLInputs / readTelnetURLOptions /
resetTelnetURLsToDefaults / defaultTelnetURLs entirely.
- renderTelnetPreflight entirely (its writes were all into the
removed elements; the state card and Plan card now own all the
surfaces it used to populate).
- toggleMigrationMethod's serviceOptions branches.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a Service URLs section to the Plan card with four free-form URL
inputs (margeServerUrl, statsServerUrl, swUpdateUrl, bmxRegistryUrl), a
"Current on Device" column populated from telnet getpdo (falling back
to the SSH-read XML config), a Soundcork-mode checkbox that flips the
/marge suffix on margeServerUrl, and a Reset-to-defaults button.
Validation runs on every keystroke (oninput) and on each summary
render: each URL must parse via the URL constructor, the scheme must
be http or https, the hostname must be non-empty, and "localhost" or
"127.0.0.1" are explicitly rejected (the speaker can't reach this
machine via that name). Invalid inputs get a red border, an inline
error list surfaces under the table, and the Apply Suggested Plan
button is disabled until everything is valid. migrate() also gates on
validatePlanURLs() and surfaces a clear status message rather than
sending typoed URLs that would silently brick the speaker.
The Plan card's per-field URLs feed both XML and Telnet migrations
via the marge_url / stats_url / sw_update_url / bmx_url options the
backend's applyURLOverrides honors. The legacy XML dropdowns
(self/proxied/original) and the duplicate URL Targets table inside
the Telnet pane stay in the markup for now — the next iteration
removes them once we're confident the Plan card flow covers
everything.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds applyURLOverrides — a tiny helper that, given a PrivateCfg and the
migration options map, copies any non-empty marge_url / stats_url /
sw_update_url / bmx_url value into the matching PrivateCfg field. The
helper runs after applyProxyOptions in both the read path
(GetMigrationSummary's planned-config preview) and the write path
(migrateViaXML's actual XML upload), so the planned diff and the file
the migration writes both reflect what the user typed.
Precedence: a literal *_url override wins over the legacy
self/proxied/original mode set on the same field, because the user
picked a URL and the migration honors it verbatim. Empty/missing
overrides leave the field unchanged. The legacy mode handling stays
in place for API back-compat — only the UI is moving away from it.
Tests cover the helper directly, the override-vs-mode precedence rule,
and a full GetMigrationSummary round-trip that verifies the override
shows up in the rendered PlannedConfig XML.
This is the data-layer half of the upcoming unified per-field URL
editor in the Plan card; no UI changes here.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two halves of the same flakiness fix:
- pkg/telnet defaults: dial 2s→4s, read 5s→7s, write 2s→3s,
idleWindow 400ms→600ms. The diagnostic shell on FW 27.0.6
occasionally takes >2s to accept a fresh TCP connection (likely
while servicing other work), and the previous tight budget
produced flaky preflight results on healthy speakers that
consistently recovered on a second attempt.
- state card: when the probe error wraps an i/o timeout / "timed out"
/ "connection reset", the panel now appends a hint pointing the
user at the ↻ refresh button next to the device dropdown — instead
of leaving the user to assume telnet is permanently unreachable.
looksTransient() keeps the substring match conservative so genuine
"connection refused" / "host unreachable" errors keep the original
framing.
The 4s dial budget adds at most ~2s to summary loads on devices
where telnet is genuinely down; that's an acceptable trade-off for
removing the false-negative reports.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The state card now duplicates everything the legacy overview
paragraphs reported, so the redundant block between the card and the
Customize details was visible-but-stale: SSH/Telnet status, the two
Backup status paragraphs, Remote Services line, and the AfterTouch
Local Root CA Trusted line.
Removed wholesale, plus the original-config-pane and toggleOriginalConfig
that the Show Original Config button drove. Kept "Trust CA Now" and
"Download CA cert" (per user request), relocating both into the state
card's CA / TLS cell as inline actions next to the verdict — the
verdict text now writes to a #state-ca-line sub-span so re-renders
don't clobber the buttons.
Also gated the HTTPS Connection Test pane on summary.ssh_success: the
backend's TestConnection uploads a temp CA file and runs curl on the
device via SSH, so the panel makes no sense when SSH isn't reachable.
A telnet-poke + service-side observation alternative is on the roadmap
but not implemented yet.
Stripped the dead JS branches that wrote to ssh-status, ca-trust-status,
remote-services-status/found, original-config-status, no-original-config-status,
original-config-content, original-config-pane, and backup-config-btn.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
toggleMigrationMethod()'s XML branch never reset
#dns-port-warning, so switching from resolv back to xml left the
"DNS Discovery is DISABLED" warning visible while the XML method was
selected — where the warning is irrelevant.
Reset the display to "none" in the default (XML) branch alongside
the existing telnet/hosts branches that already do this. The next
iteration's redesign of the Customize panel folds this state into
per-method preconditions and removes the global warning entirely;
this fix keeps the current UI honest until then.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Step-2 wizard, foundation iteration. Adds a new Plan card below the
state card on the Migration tab with three sections:
- Target service URL: editable input mirrored bidirectionally with
the canonical #target-domain field on Settings, plus a "Save as
default" button that POSTs to /setup/settings (preserving other
fields and the "***" secret-unchanged convention).
- Capabilities: which transports the speaker exposes (SSH and
Telnet:17000), and which migration recipes AfterTouch can offer
given those transports — the "possible vs supported" surface that
teaches the user *why* options are available before they pick.
- Suggested plan: a one-click "Apply Suggested Plan" button driven
by computeSuggestedPlan. The conservative default picks XML over
SSH with HTTP (no DNS, no CA install) when SSH works; falls back
to Telnet:17000 + HTTP when only telnet is reachable; and
explains the absence of a path otherwise. Already-migrated
devices show an info message instead of a button.
The legacy Migration Method dropdown, per-method panes, and action
buttons (Confirm/Revert/Reboot/Cancel) are preserved verbatim but
wrapped in a <details>"Customize this migration"</details> that opens
on demand. After a successful migrate(), the customize section is
auto-expanded so the prominent Reboot affordance is reachable from
the suggested-plan flow too.
The Apply button currently delegates to the existing migrate() entry
point by setting the dropdown value programmatically, which keeps the
options-plumbing path identical until the next iteration moves the
per-field URL editor and validation into the Plan card.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The URL Configuration cell flagged "Original (Bose cloud)" with a red
❌ even when the DNS hook (or /etc/hosts redirects, deprecated though
it is) was actively intercepting those hostnames and routing them at
AfterTouch — i.e. the expected migrated state for the DNS method.
urlConfigVerdict now factors in resolv_migrated/hosts_migrated:
- URL flip (xml or telnet) active → ✅ "AfterTouch URLs"
- URL flip not active, DNS interception on → ✅ "Original (Bose
cloud) — intercepted via DNS, device reaches AfterTouch"
- URL flip not active, no DNS interception → ❌ "Original (Bose
cloud) — not intercepted, device will reach the real Bose cloud"
The third case is the only one that's actually broken; the first two
are valid migrated states for different methods.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The migration summary now opens with a dedicated state panel that
surfaces, in three tight blocks:
- Transports — SSH and Telnet:17000 reachability, telnet banner if
any, and a probe-error sub-line when a TCP dial succeeded but the
shell rejected getpdo.
- Migration State — three rows for the orthogonal axes: URL
Configuration (verdict from xml_migrated/telnet_migrated, with the
four URL fields shown as on-disk vs live pairs underneath), DNS
Interception (resolv hook / hosts redirects / none), and CA / TLS
(local root CA installed yes/no).
- Preconditions — remote_services persistence, account-pairing
state (from is_paired / live margeAccountUUID), and the XML
.original backup presence.
Pure UI restructuring of data the backend already exposes. The
existing dropdown, method-specific panes, diff view, and per-field
service-options table are untouched so step 2 (the wizard refactor)
can replace them in a focused diff. The legacy SSH/Telnet status
paragraphs and the cross-check warnings banner stay below the card
during the transition; the next iteration removes them once the card
is the canonical surface.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds XMLMigrated, HostsMigrated, ResolvMigrated, TelnetMigrated, and
IsPaired as explicit fields on the summary so the UI can render
partial-state cells (URLs flipped via telnet but the on-disk XML
hasn't caught up; DNS interception in place but no CA installed; etc.)
and surface pairing as its own precondition. IsMigrated remains
backward-compatible — it is now the OR of the four migration axes.
checkIsMigrated stops short-circuiting and writes each axis verdict
unconditionally so a "partial" state on any axis is always visible to
the UI even when another axis already reports the device migrated.
populateDeviceInfo now derives IsPaired from the live :8090/info
margeAccountUUID (clobbering any stale datastore copy), so a
factory-reset speaker is correctly flagged as unpaired.
Tests cover the per-axis verdicts independently and the IsPaired
derivation in both the populated and empty live-info cases.
This is the data layer for the upcoming three-axis "state view" panel
on the migration tab. No frontend or behavior changes here.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a circled-arrow (↻) button beside the migration tab's device
dropdown that re-runs the summary fetch for the selected speaker.
Reuses the existing refreshSummary() entry point, which now also
falls back to the dropdown value when no summary has been loaded yet
so the button works on a freshly-selected device too.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The live SoundTouch firmware (FW 27.0.6.46330.5043500, ST 20) replies
to `getpdo CurrentSystemConfiguration` with a Protobuf-text-like
nested-block format, not the key=value format my parser was written
against:
margeServerUrl {
text: "https://streaming.bose.com"
}
statsServerUrl {
text: "https://events.api.bosecm.com"
}
...
->OK
->
Effect of the bug: the four "Current on Device" cells in the telnet
URL Targets table stayed empty after a summary load, and the
crossCheckPreflights helper silently produced no warnings even when
SSH-XML and telnet-getpdo would have disagreed. Both behaviours were
reported from a real-device summary fetched against the running
service.
Both parsers (Go setup.parseGetpdoConfig and JS
parseTelnetVerifiedConfig) now accept the protobuf-text shape and keep
the legacy key=value path as a tolerance fallback. An isIdentifier
guard prevents protobuf "text: …" lines from being misread as flat
fields and keeps prompt characters (->, ->OK) out of the result map.
A new TestParseGetpdoConfig_ProtobufTextRealDevice test pins the
parser to the verbatim live response so this regression cannot recur
silently.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The migration tab gains:
- Telnet (Port 17000) status line in the summary box, mirroring the
SSH connection line. Shows ✅/❌, the device's diagnostic shell
banner if any, and a probe-error block when a TCP dial succeeded
but the shell rejected getpdo.
- Cross-check warnings banner that surfaces summary.warnings (the
SSH-XML vs telnet-getpdo URL diffs from the parallel preflight) as
informational notices above the migration controls.
- URL Targets table inside the telnet method pane with four editable
inputs (Marge, Stats, Software Update, BMX Registry) pre-filled
from the canonical defaultTelnetURLs(target_url) derivation. Each
row shows the device's current value alongside, parsed from
summary.telnet_verified_config. A "Reset to defaults" button wipes
user edits in the table.
- Migrate / Reboot buttons now enable when *either* SSH or telnet is
reachable, so the SSH-less telnet path can actually be triggered
from the UI.
The four URL inputs are folded into the migrate query string as the
marge_url / stats_url / sw_update_url / bmx_url options the handler now
recognises. Empty fields are omitted so the service's
telnetURLsFromOptions canonical fallback runs.
JS helpers parseTelnetVerifiedConfig and defaultTelnetURLs mirror the
Go-side parseGetpdoConfig and defaultTelnetURLs — keep them in sync.
I cannot run a browser test from this environment, so this change is
verified only by go build, the Go test suite (setup + handlers, race),
and node --check on the modified script.js. Worth a manual smoke test
of: switching to telnet, observing the inputs pre-fill, editing one
field, kicking off a migration, and reading back the warnings banner
on a freshly-migrated speaker.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Extracts the migration-options query-string parsing into a single
parseMigrationOptions helper used by both HandleGetMigrationSummary and
HandleMigrateDevice. The allow-list now covers two families:
- marge / stats / sw_update / bmx (XML method's per-field
self|proxied|original implementation selectors, unchanged)
- marge_url / stats_url / sw_update_url / bmx_url (telnet method's
per-field URL overrides; empty values fall back to the canonical
derivation in setup.telnetURLsFromOptions)
Unknown keys are still dropped, so the manager only sees parameters the
handler explicitly opted into. Tests cover the allow-list, the noise
filter, and the empty-query case.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Refactors telnetURLConfigCommands into a telnetURLs value type with
explicit per-field URLs (Marge, Stats, SwUpdate, BmxRegistry) and adds
telnetURLsFromOptions to resolve those four URLs from a base targetURL
plus optional per-field overrides via the migration options map
(marge_url, stats_url, sw_update_url, bmx_url).
Envswitch derivation rule: arg1 = u.Marge verbatim, arg2 = u.SwUpdate
verbatim. The soundcork case (Marge has /marge appended) is handled
without any branching — envswitch arg1 carries the same suffix and the
parallel persistence layer stays consistent with the runtime layer on
the next reboot.
The default path is unchanged for users who only enter a base URL: all
four fields share targetURL with the canonical /updates/soundtouch and
/bmx/registry/v1/services suffixes. MigrateSpeaker plumbs the options
map through so the existing handler's option dictionary works for telnet
without UI changes; the UI can layer per-field input on top later.
Existing telnet migration tests updated to call the new signature.
TestMigrateViaTelnet_SoundcorkMargeSuffixPropagatesToEnvswitch is the
load-bearing regression test for the derivation rule.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When both preflights succeed, GetMigrationSummary now compares the URL
fields in the parsed SoundTouchSdkPrivateCfg.xml (read via SSH) against
the matching keys in `getpdo CurrentSystemConfiguration` (read via
telnet) and appends a Warnings entry for any field whose values differ.
The two sources can briefly disagree because `sys configuration …`
writes the runtime layer while envswitch writes the parallel persistence
layer and the on-device XML file is only re-rendered after a reboot.
The warning text says exactly that, so the UI can surface a non-fatal
hint instead of treating a freshly-migrated-but-not-yet-rebooted device
as broken.
Adds Warnings []string on MigrationSummary, parseGetpdoConfig (a
key=value parser tolerant to banner/prompt noise), and
crossCheckPreflights wired in as step 9 of GetMigrationSummary.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds Manager.isTelnetMigrated, which substring-matches m.ServerURL's
hostname against TelnetVerifiedConfig — the response captured by the
preflight's `getpdo CurrentSystemConfiguration`. Mirrors the existing
isXMLMigrated semantics so users see consistent migration-state
detection regardless of which transport the device exposes.
checkIsMigrated no longer early-returns on !SSHSuccess. Telnet runs
first and unconditionally; the SSH-based hosts/resolv.conf checks still
run when SSH is reachable, since neither variant shows up in
`getpdo CurrentSystemConfiguration`. This closes the gap where a
USB-unlock-refusing speaker (SA-5, ST520, recent ST Portable) that had
already been migrated via telnet was silently reported as IsMigrated:
false in the UI.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
GetMigrationSummary now kicks off telnetPreflight in a goroutine at
entry and merges the four Telnet* fields into the main summary just
before returning. Wall time becomes max(ssh, telnet); the two transports
are queried independently and their results combined — SSH retains
visibility into /etc/hosts, /etc/resolv.conf and the on-device XML
config, while telnet contributes the live URL set readable via
`getpdo CurrentSystemConfiguration` without root.
Race-free by construction: the goroutine writes to its own
MigrationSummary instance and only the four telnet fields are copied
back. Verified with `go test -race`.
Tests cover telnet-only, ssh-only, and both-succeed paths.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds Manager.telnetPreflight that dials port 17000, captures the banner,
and runs `getpdo CurrentSystemConfiguration` to read back the device's
live URL configuration. Errors are recorded on TelnetProbeError instead
of returned, so the probe is best-effort and never breaks summary
construction.
This is the data-gathering layer that the four already-declared
TelnetReachable / TelnetBanner / TelnetVerifiedConfig / TelnetProbeError
fields on MigrationSummary were waiting for. Subsequent iterations wire
the preflight into GetMigrationSummary (in parallel with SSH) and use
TelnetVerifiedConfig as a SSH-free signal for "already migrated".
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous filepath.IsLocal-up-front pattern in safeJoin/safeJoin-equivalents
turned out not to satisfy CodeQL's go/path-injection rule — the post-validation
filepath.Join still constructs the joined string from tainted input, so the
analyser conservatively assumes the os.* sink that consumes it is tainted
too. Only one of 34 alerts closed on the previous attempt.
Switch to *os.Root (Go 1.24+, available on the project's 1.26.3 toolchain).
The Go runtime guarantees that operations on a Root cannot escape the
anchored directory regardless of what's in the relative path, and CodeQL has
a built-in model that recognises *os.Root.* methods as path-traversal
sanitisers. Result: every os.* sink in the datastore, marge, recorder,
mirror parity-mismatch writer, and docs handler is now reached only via a
*os.Root, which closes the rule-level alerts cleanly.
Changes per file:
* pkg/service/datastore/datastore.go — Adds a `root *os.Root` to DataStore,
lazily opened at first use (after MkdirAll-ing baseDir) and closed by a
new `(*DataStore).Close()`. Adds package-private helpers
(rootStat / rootReadFile / rootWriteFile / rootMkdirAll / rootRemove /
rootRemoveAll / rootRename / rootReadDir / rootOpen / rootExists) plus
three exported wrappers (ReadDirUnderBase, MkdirAllUnderBase,
WriteFileUnderBase) for the cross-package marge / handlers callers.
Every os.* call that previously consumed safeJoin output now goes through
these helpers. The post-join belt-and-suspenders prefix check inside
safeJoin is preserved as a defence-in-depth fallback.
* pkg/service/marge/marge.go — Replaces the five `os.ReadDir(devicesDir)`
call sites with `ds.ReadDirUnderBase(...)` so the datastore's root
enforces containment.
* pkg/service/proxy/recorder.go — Mirrors the datastore pattern with its
own `root *os.Root` anchored at Recorder.BaseDir, lazily opened. New
helpers convert the eight existing `os.*` sites that consume sessionID
/ relPath / sanitizedSegments inputs. The earlier safeJoin (filepath.IsLocal
pre-check) stays in place as the same belt-and-suspenders guard.
* pkg/service/handlers/handlers_docs.go — Opens a *os.Root at "docs" via
sync.Once and reads file content (and SUMMARY.md sidebar) through it.
Removes the prior filepath.IsLocal pre-check; the runtime now guarantees
containment.
* pkg/service/handlers/mirror_middleware.go — Routes the parity-mismatch
JSON write through `s.ds.WriteFileUnderBase` so the datastore's root
performs the path-traversal sanitiser.
Behavioural fix: *os.File.ReadDir(-1) returns directory entries in
filesystem order, but os.ReadDir is documented to sort by name and at least
one regression test
(handlers.TestMargeAccountFullExcludesEmptyAmazonSource) depends on the
sorted contract. Both rootReadDir helpers explicitly sort by name to match.
All test suites pass for the touched packages; the unrelated
TestDocsConsistency failure about untracked working-tree docs is
pre-existing. golangci-lint reports 0 issues.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Wire up X-Real-IP / X-Forwarded-For / True-Client-IP support for
deployments fronted by a reverse proxy, while staying safe on flat-LAN
deployments where a malicious speaker could spoof those headers
directly.
Two new fields on `datastore.Settings`:
* TrustForwardedHeaders (bool, default false) — opt-in switch.
* TrustedProxyCIDRs ([]string, default `["127.0.0.0/8", "::1/128"]`)
— only requests whose immediate TCP peer falls in one of these
blocks may have their source IP rewritten from forwarded headers.
Loopback default matches the documented same-host nginx layout in
docs/guides/HTTPS-SETUP.md.
New middleware in `pkg/service/handlers/middleware_realip.go`:
* TrustedRealIP wraps `chi/middleware.RealIP` with a trusted-peer
gate. When the immediate TCP peer is in the allowlist, chi's
parsing handles the actual header → IP rewrite. When it isn't
(e.g. a speaker sending forwarded headers itself), we ignore the
headers and r.RemoteAddr stays as-is.
* ParseTrustedProxyCIDRs converts string CIDRs into *net.IPNet,
applying the loopback default on empty input and erroring loudly
on invalid entries.
Server.TrustedRealIPMiddleware() returns the middleware (or nil) by
reading the live settings; the router setup in
cmd/soundtouch-service/main.go installs it as the very first
middleware so SnapshotMiddleware and downstream handlers see the
correct r.RemoteAddr.
HandleMargePowerOn now prefers r.RemoteAddr over the body's
self-reported `<IPAddress>` for outbound credential push:
* The body field is treated as a hint only — a malicious LAN speaker
could set it to any value; using it for outbound HTTP requests is
the SSRF surface the previous zeroconf hardening was guarding
against from the sink side. Fixing it at the source as well closes
the gap entirely.
* When body IP and TCP source disagree, a log line names both and
the device ID so the discrepancy is investigable.
* RemoteAddr is unparseable → fall back to the body so we don't
silently drop the priming.
docs/guides/HTTPS-SETUP.md gains a follow-up note next to the existing
nginx snippet explaining the new flag, the loopback-only default, and
the explicit warning against enabling the flag on a flat-LAN
deployment without a real proxy.
Eleven test cases in middleware_realip_test.go lock in the gate
behaviour: trusted peers honoured for X-Real-IP / X-Forwarded-For /
no-headers / IPv6, untrusted peers' headers ignored, garbage values
rejected, ParseTrustedProxyCIDRs covers default / override / invalid.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Building on the strict literal-IP validator from the previous commit,
make the runtime error self-explanatory so anyone tripping on a
hostname URL can fix it in one shot:
* Errors now lead with the offending zeroconf URL and the rejected
host, so wrapping by GetInfo / PushCredentials / pushSimplifiedToken
doesn't bury the actual bad value.
* The "host must be a literal IP" error suggests two concrete one-liner
resolutions (`getent hosts <name>` and `dig +short <name>`) so the
user has a copy-paste fix.
* The "host is not on a local network" error names the accepted ranges
(loopback / RFC1918 private / link-local v4+v6) so the user knows
what they're allowed to pass.
docs/guides/SOUNDTOUCH-SERVICE.md gains a bullet under Security
Considerations explaining the constraint and the rationale (LAN-resident
SSRF surface), so the strict behaviour is documented rather than a
surprise.
The 17 TestValidateZcBaseURL cases still pass — only the message bodies
changed.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CodeQL re-fired three new go/request-forgery alerts (#134/135/136) on
the lines my previous validateZcBaseURL refactor introduced. The
previous validator accepted hostname-style hosts unchanged, so even
though the IP-class check ran when applicable, u.String() at the call
sites still emitted the original tainted host into the request URL —
which is exactly what CodeQL traces.
Tighten validateZcBaseURL to:
* require the host to parse as a literal IP — DNS / mDNS hostnames
are rejected (with a clear error explaining the caller should
resolve to a private IP first); doing the lookup inside the
validator would re-introduce the SSRF surface CodeQL is flagging,
because malicious DNS could point a *.local name at a public host
between the lookup and the request.
* require that IP to be loopback / RFC1918 private / IPv4-or-IPv6
link-local. Anything else (global IPs in either family) is refused.
* rebuild the returned *url.URL from validated components — scheme
(already checked), the validated IP literal joined with the
original port, and the original path. Pre-existing query/fragment
are stripped so callers attach their own ?action= cleanly. CodeQL
recognises this fresh-construction pattern as taint sanitisation.
In practice this matches what SoundTouch speakers actually announce:
IP-based zeroconf URLs at port 8200 against an LAN address. The
existing PushCredentials_FullRoundTrip and FallbackOnGetInfoFailure
tests already exercise the loopback path through httptest.NewServer
and pass unchanged.
Adds TestValidateZcBaseURL covering 17 inputs — 9 accept (loopback,
private 10/172/192, link-local v4, IPv6 loopback, IPv6 link-local,
strips query) and 8 reject (public IPv4, public IPv6, hostname,
plain hostname, ftp/file schemes, empty host, unparseable) — to lock
the new contract in.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous commit made credential-header redaction unconditional in
proxy log output, which is the right safety floor for production but
inconvenient for local debugging when a developer wants to inspect
Authorization / Cookie / X-Bose-Token values flowing through the
service.
Add an explicit "I-know-what-I-am-doing" toggle:
* New LoggingProxy.UnsafeLogCredentialHeaders bool field.
* Default off — the redaction floor stays in place.
* Reads the LOG_PROXY_CREDENTIALS env var so a developer can flip it
on without recompiling, mirroring the existing LOG_PROXY_BODY
pattern.
* When true, formatHeaders skips both the always-sensitive floor and
the broader Redact policy, so log lines contain raw header values.
CodeQL's go/clear-text-logging rule continues to be satisfied because
the default code path still redacts; only an explicit opt-in via
configuration produces unredacted output, mirroring how
AllowInsecureUpstreamTLS works.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CodeQL alerts #121, #122, #123 (go/request-forgery) flagged the three
client.Get / client.PostForm sites in pkg/service/zeroconf/zeroconf.go
that build their request URL by string-concatenating the caller-supplied
zcBaseURL with "?action=…". The base URL ultimately originates from a
device-pairing payload that the speaker pushes to us, so unvalidated
input could redirect outbound HTTP requests to arbitrary hosts (server-
side request forgery).
Add validateZcBaseURL which:
* parses zcBaseURL via net/url so the scheme and host are first-class
values rather than substrings,
* requires the scheme to be http or https,
* rejects literal IP hosts that aren't loopback / RFC1918 private /
link-local — those are the only places a real SoundTouch speaker
can live on a local network, and a global IP would be an obvious
exfiltration target,
* leaves hostname-style hosts (e.g. mDNS *.local) accepted: name
resolution itself is a separate trust boundary on the local segment.
A small withAction helper builds the per-call URL from the validated
base URL via url.Values rather than string concatenation, which CodeQL
recognises as a non-tainted construction.
GetInfo, PushCredentials and pushSimplifiedToken each call
validateZcBaseURL up-front so all three CodeQL alerts close in a
single pass. PushCredentials also re-validates even though it then
calls GetInfo (which validates again) so the fallback to
pushSimplifiedToken on getInfo failure is also gated.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CodeQL alerts #70 and #71 (go/disabled-certificate-check) flagged the
hard-coded `InsecureSkipVerify: true` in handlers_proxy.go (the
/proxy/{url} reverse proxy) and mirror_middleware.go (the parity-check
mirror). Both target *.bose.com whose certificate chain is becoming
unreliable post end-of-service, but unconditionally disabling
verification is still wrong: a deployment that doesn't actually need
the bypass loses TLS hygiene for free.
Add an `AllowInsecureUpstreamTLS bool` field to datastore.Settings,
default false. Read it in both call sites — they aren't on a hot path
— and pass the value as InsecureSkipVerify. CodeQL accepts the
configurable boolean as a non-flag (vs. the previously hard-coded
`true`), and the runtime behaviour now defaults to verifying
certificates with an explicit opt-in for the broken-chain scenario.
Behaviour change: TLS upstream traffic is verified by default. Anyone
relying on the previous always-skip behaviour can re-enable it by
setting `"allow_insecure_upstream_tls": true` in settings.json.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CodeQL alert #43 (go/clear-text-logging) flagged that headers flow to
log.Printf in pkg/service/proxy/proxy.go. The existing implementation
only redacted when LoggingProxy.Redact was true — an opt-in. CodeQL is
right to flag this: the safety floor for credential-bearing headers
should not depend on caller configuration.
Split the sensitive-header list into two:
* alwaysSensitiveHeaders — Authorization, Proxy-Authorization, Cookie,
Set-Cookie, X-Api-Key, X-Bose-Token. Redacted unconditionally,
regardless of LoggingProxy.Redact.
* sensitiveHeaders — kept as a compatibility alias pointing at the same
list, and still gated on Redact for any future use cases that want
*additional* opt-in redaction beyond the floor.
Behaviour change is strict tightening: nothing that was previously
hidden becomes visible, and credentials that would have been logged
when Redact was false are now hidden by default.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CodeQL flagged five reflected-XSS sites where caller-supplied query
parameters or path segments were concatenated into HTML responses
without escaping:
* handlers_mgmt.go:147 — Spotify oauth error landing page
* handlers_mgmt.go:490 — Amazon oauth error landing page
* handlers_docs.go:65 — <title> built from r.URL.Path
* recorder_middleware.go:83, mirror_middleware.go:205 — passthrough
Write()s carrying tainted bytes from the three sources above
Wrap each user-controlled value in html.EscapeString before it lands
in the HTML body. The escaped output covers the upstream sources so
the middleware passthrough alerts close as well.
For handlers_docs the rendered markdown (`output`) and sidebar are
server-controlled (loaded from on-disk doc files) and intentionally
contain HTML, so only the URL path is escaped — the documentation
content itself still renders normally.
Handler test suite passes; pre-existing TestDocsConsistency failure
about untracked working-tree docs is unrelated.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
