From cb7c3f319dec0bb38cc2ef87da56b411be344044 Mon Sep 17 00:00:00 2001 From: Tobias Gesellchen Date: Sun, 10 May 2026 18:43:43 +0200 Subject: [PATCH] feat(setup): detect telnet-only migrated devices via getpdo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .../setup/migration_state_telnet_test.go | 102 ++++++++++++++++++ pkg/service/setup/setup.go | 33 ++++++ 2 files changed, 135 insertions(+) create mode 100644 pkg/service/setup/migration_state_telnet_test.go diff --git a/pkg/service/setup/migration_state_telnet_test.go b/pkg/service/setup/migration_state_telnet_test.go new file mode 100644 index 0000000..d354f49 --- /dev/null +++ b/pkg/service/setup/migration_state_telnet_test.go @@ -0,0 +1,102 @@ +package setup + +import ( + "errors" + "testing" +) + +func TestIsTelnetMigrated_TargetHostnamePresent(t *testing.T) { + m := &Manager{ServerURL: "http://example:8000"} + + summary := &MigrationSummary{ + TelnetVerifiedConfig: "margeServerUrl=http://example:8000\nbmxRegistryUrl=http://example:8000/bmx/registry/v1/services\n", + } + + if !m.isTelnetMigrated(summary) { + t.Error("isTelnetMigrated = false, want true when getpdo response contains our hostname") + } +} + +func TestIsTelnetMigrated_DifferentHostname(t *testing.T) { + m := &Manager{ServerURL: "http://example:8000"} + + summary := &MigrationSummary{ + TelnetVerifiedConfig: "margeServerUrl=https://streaming.bose.com\n", + } + + if m.isTelnetMigrated(summary) { + t.Error("isTelnetMigrated = true, want false when getpdo response points at the original cloud") + } +} + +func TestIsTelnetMigrated_EmptyVerifiedConfig(t *testing.T) { + m := &Manager{ServerURL: "http://example:8000"} + + summary := &MigrationSummary{} // TelnetVerifiedConfig empty + + if m.isTelnetMigrated(summary) { + t.Error("isTelnetMigrated = true, want false when TelnetVerifiedConfig is empty") + } +} + +// TestCheckIsMigrated_TelnetOnlyMigratedDevice covers the gap that motivated +// this iteration: SSH is unreachable, but the speaker has been pointed at +// our service via telnet (e.g. a firmware that refuses USB unlock). The +// migration UI must still report IsMigrated: true. +func TestCheckIsMigrated_TelnetOnlyMigratedDevice(t *testing.T) { + m := &Manager{ServerURL: "http://example:8000"} + + summary := &MigrationSummary{ + SSHSuccess: false, + TelnetVerifiedConfig: "margeServerUrl=http://example:8000\n", + } + + m.checkIsMigrated(summary, "192.0.2.1") + + if !summary.IsMigrated { + t.Error("IsMigrated = false, want true on a telnet-only migrated device with no SSH") + } +} + +// TestCheckIsMigrated_NoTelnetNoSSH ensures we don't false-positive when +// neither transport sees the redirect. +func TestCheckIsMigrated_NoTelnetNoSSH(t *testing.T) { + m := &Manager{ServerURL: "http://example:8000"} + + summary := &MigrationSummary{ + SSHSuccess: false, + TelnetVerifiedConfig: "", // probe failed + } + + m.checkIsMigrated(summary, "192.0.2.1") + + if summary.IsMigrated { + t.Error("IsMigrated = true, want false when neither SSH nor telnet sees the redirect") + } +} + +// TestCheckIsMigrated_TelnetSeesOriginalSSHSeesOriginal ensures we don't +// false-positive when both transports report unmigrated state. +func TestCheckIsMigrated_TelnetSeesOriginalSSHSeesOriginal(t *testing.T) { + m := &Manager{ + ServerURL: "http://example:8000", + NewSSH: func(string) SSHClient { + return &mockSSH{runFunc: func(string) (string, error) { return "", errors.New("file not found") }} + }, + } + + summary := &MigrationSummary{ + SSHSuccess: true, + TelnetVerifiedConfig: "margeServerUrl=https://streaming.bose.com\n", + ParsedCurrentConfig: &PrivateCfg{ + MargeServerUrl: "https://streaming.bose.com", + }, + CurrentResolvConf: "nameserver 8.8.8.8\n", + } + + m.checkIsMigrated(summary, "192.0.2.1") + + if summary.IsMigrated { + t.Error("IsMigrated = true, want false when both SSH and telnet see the original cloud URLs") + } +} diff --git a/pkg/service/setup/setup.go b/pkg/service/setup/setup.go index 90a6d07..05744e1 100644 --- a/pkg/service/setup/setup.go +++ b/pkg/service/setup/setup.go @@ -408,7 +408,18 @@ func (m *Manager) buildServerHTTPSURL(targetURL string) string { } // checkIsMigrated determines if the device is already migrated to AfterTouch. +// +// The telnet-based check runs first and unconditionally, because it is the +// only migration-state signal available on devices that do not expose SSH +// (USB-unlock-refusing firmware on SA-5, ST520, recent ST Portable). The +// SSH-based checks still run when SSH is reachable to cover the +// /etc/hosts and /etc/resolv.conf migration variants, neither of which +// shows up in `getpdo CurrentSystemConfiguration`. func (m *Manager) checkIsMigrated(summary *MigrationSummary, deviceIP string) { + if m.isTelnetMigrated(summary) { + summary.IsMigrated = true + } + if !summary.SSHSuccess { return } @@ -420,6 +431,28 @@ func (m *Manager) checkIsMigrated(summary *MigrationSummary, deviceIP string) { } } +// isTelnetMigrated reports whether the live device config (read via the +// telnet preflight's `getpdo CurrentSystemConfiguration`) already points +// at our service. Mirrors isXMLMigrated's substring-match semantics — any +// occurrence of our hostname in the response is enough. +func (m *Manager) isTelnetMigrated(summary *MigrationSummary) bool { + if summary.TelnetVerifiedConfig == "" { + return false + } + + parsedTarget, err := url.Parse(m.ServerURL) + if err != nil { + return false + } + + targetHost := parsedTarget.Hostname() + if targetHost == "" { + return false + } + + return strings.Contains(summary.TelnetVerifiedConfig, targetHost) +} + // isXMLMigrated checks whether current XML config already points to our server. func (m *Manager) isXMLMigrated(summary *MigrationSummary) bool { if summary.ParsedCurrentConfig == nil {