diff --git a/bin/prod/save-settings b/bin/prod/save-settings index ac5abf0..d24bf86 100755 --- a/bin/prod/save-settings +++ b/bin/prod/save-settings @@ -19,7 +19,9 @@ update_settings(){ umount -q /mnt/emmc/ mount "${EMMC}p2" /mnt/emmc - echo -e "$CONTENT" > "$SETTINGS_FILE" + # printf, not echo -e: the content arrives with real newlines, and -e + # also turned any backslash in a passphrase into an escape (#157). + printf '%s\n' "$CONTENT" > "$SETTINGS_FILE" chmod 600 "$SETTINGS_FILE" umount /mnt/emmc } diff --git a/bin/prod/wifi-connect b/bin/prod/wifi-connect index 59a808a..c5fe464 100755 --- a/bin/prod/wifi-connect +++ b/bin/prod/wifi-connect @@ -18,11 +18,41 @@ info() { # A failed connection here (bad password, AP out of range, DHCP timeout) # would otherwise leave the board in station mode with no working # connection and no hotspot - unreachable until physical intervention (#90). -restore_hotspot() { - info "Connection failed - restoring hotspot ($AP_PROFILE) so the board stays reachable." +# Whether the adapter is actually serving the AP now. +hotspot_up() { + [ "$(iwctl device list 2>/dev/null \ + | sed $'s/\x1b\[[0-9;]*m//g' \ + | awk -v i="$INTERFACE" '$1 == i {print $NF}')" = "ap" ] +} + +start_hotspot() { iwctl ap "$INTERFACE" stop 2>/dev/null iwctl device "$INTERFACE" set-property Mode ap iwctl ap "$INTERFACE" start-profile "$AP_PROFILE" 2>/dev/null + sleep 1 + hotspot_up +} + +restore_hotspot() { + restore_profile + info "Connection failed - restoring hotspot ($AP_PROFILE) so the board stays reachable." + if start_hotspot; then + return 0 + fi + # Checked rather than assumed, because it has been seen not to happen: + # iwd 3.8 segfaulted during this on an A8 (status=11/SEGV, while + # `iwctl ap start-profile` ran), systemd restarted it, and it came back in + # station mode - so the board had no hotspot at all, which is the state + # this function exists to prevent (#90). One retry against the restarted + # daemon is enough to get it back. + info "The hotspot did not come up - retrying once." + sleep 2 + if start_hotspot; then + info "Hotspot restored on the second attempt." + return 0 + fi + info "WARNING: could not start the $AP_PROFILE hotspot. The board is reachable over Ethernet only." + return 1 } if [ -z "$SSID" ] || [ -z "$PASS" ]; then @@ -50,9 +80,36 @@ if [ "$CURRENT_MODE" == "ap" ]; then fi # 2. Create the iwd profile +# +# Kept aside first, because a failed attempt has to leave iwd as it found it. +# The profile carries AutoConnect=true, so a wrong passphrase left behind was +# retried by iwd on its own and on every boot - and a mistyped retry would +# overwrite a profile that worked (#150). +PROFILE="$IWD_DIR/$SSID.psk" +PREV_PROFILE="" +if [ -f "$PROFILE" ]; then + PREV_PROFILE=$(mktemp) + cp -p "$PROFILE" "$PREV_PROFILE" +fi + +# Put back what was there before this attempt, or nothing if there was nothing. +# forget first: iwd holds known networks in memory as well as on disk, and +# deleting the file alone would leave it retrying the passphrase that failed. +restore_profile() { + iwctl known-networks "$SSID" forget 2>/dev/null + rm -f "$PROFILE" + if [ -n "$PREV_PROFILE" ]; then + cp -p "$PREV_PROFILE" "$PROFILE" + rm -f "$PREV_PROFILE" + info "Restored the previous profile for $SSID." + else + info "Removed the new profile for $SSID." + fi +} + info "Provisioning iwd profile for $SSID..." mkdir -p "$IWD_DIR" -cat < "$IWD_DIR/$SSID.psk" +cat < "$PROFILE" [Settings] AutoConnect=true @@ -117,7 +174,28 @@ else info "$SSID did not appear in a scan within ${SCAN_TIMEOUT}s - trying anyway. If the connect also fails, check the name, the signal, and whether this adapter can use that band." fi +# The network iwd says it is on right now, if any. +connected_network() { + iwctl station "$INTERFACE" show 2>/dev/null \ + | sed $'s/\x1b\[[0-9;]*m//g' \ + | sed -n 's/^[[:space:]]*Connected network[[:space:]]\{2,\}//p' \ + | sed 's/[[:space:]]*$//' +} + # 4. Trigger the connection +# +# Drop the current association first. Without this, asking to connect to the +# network the board is already on did nothing at all: iwd stays associated with +# the session it already has, the address never goes away, and the wait below +# saw it immediately and reported success - for a passphrase that had never been +# tested. A wrong one then looked like it worked and was saved as if it did. +ALREADY=$(connected_network) +if [ -n "$ALREADY" ]; then + info "Disconnecting from $ALREADY first, so the passphrase is actually tested." + iwctl station "$INTERFACE" disconnect 2>/dev/null + sleep 2 +fi + info "Requesting iwd to connect..." # Still retried, but now only for a genuinely transient refusal - the slow-scan # case it used to paper over is handled above. @@ -183,6 +261,10 @@ install_source_route() { } # 5. Wait and verify IP address +# +# Trustworthy because of the disconnect above: iwd drops the lease with the +# association, so an address here belongs to the network just asked for rather +# than to the session the board already had. info "Waiting for DHCP..." MAX_RETRIES=30 COUNT=0 @@ -191,6 +273,7 @@ while [ $COUNT -lt $MAX_RETRIES ]; do if [ -n "$IP" ]; then info "Success! Connected with IP: $IP" install_source_route "${IP%%/*}" + [ -n "$PREV_PROFILE" ] && rm -f "$PREV_PROFILE" exit 0 fi sleep 1 diff --git a/bin/prod/wifi-scan b/bin/prod/wifi-scan index cea7362..7f5380e 100755 --- a/bin/prod/wifi-scan +++ b/bin/prod/wifi-scan @@ -39,10 +39,14 @@ sleep 3 # 4. Structured Output echo "---SCAN_RESULTS_START---" # This pipeline: -# 1. sed: Nukes ANSI escape codes (the \x1B[...m stuff) -# 2. sed: Removes the > marker -# 3. awk: Parses the columns safely +# 1. sed: Drops the grey stars. iwctl always prints four, and draws the unlit +# ones in grey (`***\e[1;90m*\e[0m` is three bars), so stripping the colour +# first turned every network into "****" whatever its signal (#154). +# 2. sed: Nukes the remaining ANSI escape codes (the \x1B[...m stuff) +# 3. sed: Removes the > marker +# 4. awk: Parses the columns safely iwctl station "$INTERFACE" get-networks | \ + sed $'s/\x1b\[1;90m\*\+//g' | \ sed $'s/\x1b\[[0-9;]*m//g' | \ sed 's/>//g' | \ awk ' diff --git a/client/src/App.vue b/client/src/App.vue index 30a3260..49818a4 100644 --- a/client/src/App.vue +++ b/client/src/App.vue @@ -177,6 +177,15 @@ ref="integritychecker" v-if="!options.magicmode" @integrity="imageIntegrity = $event" /> + + {{ confirmDelete ? "Delete?" : "Delete" }} +
(this.confirmDelete = false), 4000); + return; + } + this.confirmDelete = false; + const name = this.selectedLocalImage; + try { + const res = await axios.put(`/api/delete_image`, { filename: name }); + if (res.data.status == "ERROR") { + this.$waveui.notify(res.data.error, "error", 0); + } + } catch (err) { + this.$waveui.notify( + "Could not delete " + name + ": " + (err.response?.data || err), + "error", + 0 + ); + } + this.selectedLocalImage = null; + await this.getStatus(); + }, async installSelected() { let self = this; await axios @@ -1008,6 +1061,8 @@ export default { // A watcher runs when the selection actually changes, which is also what // keeps the Install button's state from flickering while polling redraws. selectedLocalImage() { + // A pending "Delete?" belongs to the image it was clicked for. + this.confirmDelete = false; this.onSelectedFileChanged(); }, }, diff --git a/client/src/components/TheWifiSetup.vue b/client/src/components/TheWifiSetup.vue index eb2163d..fc737ca 100644 --- a/client/src/components/TheWifiSetup.vue +++ b/client/src/components/TheWifiSetup.vue @@ -292,17 +292,43 @@ export default { this.boardReachable = false; } + // The server's own verdict on the attempt. The radio state alone cannot + // give it: iwd names the network while it is still *connecting*, so a + // wrong passphrase looked exactly like success for the few seconds before + // it failed, and the watch had stopped by then (#149). null when it + // cannot be read, which is not a verdict either way. + let attempt = null; + if (wifi && wantMode === "station") { + try { + const res = await axios.get('/api/wifi_poll_connect', { timeout: REQUEST_TIMEOUT }); + attempt = res.data || null; + } catch (err) { + attempt = null; + } + } + if (wifi) { if (wifi.mode !== this.reconnectFromMode) { this.sawTransition = true; } + if (wantMode === "station" && attempt && !attempt.isConnecting && attempt.error) { + this.wifi = wifi; + this.stopReconnectWatch(); + this.statusMessage = + `Could not join ${target}. The board is back on its own Recore hotspot.`; + this.$waveui.notify(this.statusMessage, "error", 0); + return; + } // Only "arrived" once the board reports the state we asked for. Right // after the request it is still on the old one, and treating that as - // success would report a connection that has not happened. + // success would report a connection that has not happened. For a + // network that also means an address and an attempt that has finished: + // named-but-still-associating is not joined. const arrived = wantMode === "ap" ? wifi.mode === "ap" - : wifi.mode === "station" && wifi.ssid === target; + : wifi.mode === "station" && wifi.ssid === target && !!wifi.ip && + !(attempt && attempt.isConnecting); if (arrived) { this.wifi = wifi; this.isWifiPresent = !!wifi.present; diff --git a/client/tests/unit/InstallGating.spec.js b/client/tests/unit/InstallGating.spec.js index 9fe78a5..1ef97c4 100644 --- a/client/tests/unit/InstallGating.spec.js +++ b/client/tests/unit/InstallGating.spec.js @@ -47,6 +47,82 @@ describe('Install is refused for an image that failed its integrity check', () = it.each(['INSTALLING', 'BACKUPING'])('stays clickable as Cancel during %s', (state) => { expect(disabled({ state, imageIntegrity: false })).toBe(false); }); + + // Through a transfer it read "Install" and stayed enabled, and a click sent + // cancel_installation or cancel_backup at the transfer (#156). + it.each(['DOWNLOADING', 'UPLOADING', 'MAGIC', 'UPLOADING_MAGIC'])( + 'is disabled during %s, for install and backup alike', (state) => { + expect(disabled({ state, imageIntegrity: true })).toBe(true); + expect(disabled({ state, flash: { selectedMethod: 1 } })).toBe(true); + }); + + it.each([ + ['DOWNLOADING', 0], ['UPLOADING', 0], ['UPLOADING', 1], ['MAGIC', 1], + ])('sends no cancel from a %s (method %i)', (state, selectedMethod) => { + const apiCall = vi.fn(); + App.methods.onInstallButtonClick.call({ + flash: { selectedMethod }, state, apiCall, + installSelected: vi.fn(), backupSelected: vi.fn(), + }); + expect(apiCall).not.toHaveBeenCalled(); + }); +}); + +describe('deleting an image (#153)', () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.useFakeTimers(); + }); + + function stand(over = {}) { + return { + selectedLocalImage: 'truncated.img.xz', + confirmDelete: false, + confirmDeleteTimer: null, + getStatus: vi.fn(), + $waveui: { notify: vi.fn() }, + ...over, + }; + } + + it('asks first, and deletes on the second click', async () => { + axios.put.mockResolvedValue({ data: { status: 'OK' } }); + const self = stand(); + await App.methods.onDeleteImageClick.call(self); + expect(axios.put).not.toHaveBeenCalled(); + expect(self.confirmDelete).toBe(true); + + await App.methods.onDeleteImageClick.call(self); + expect(axios.put).toHaveBeenCalledWith('/api/delete_image', { filename: 'truncated.img.xz' }); + expect(self.selectedLocalImage).toBe(null); + expect(self.getStatus).toHaveBeenCalled(); + }); + + it('forgets the first click after a few seconds', async () => { + const self = stand(); + await App.methods.onDeleteImageClick.call(self); + vi.advanceTimersByTime(5000); + expect(self.confirmDelete).toBe(false); + }); + + it('says why when the server refuses', async () => { + axios.put.mockResolvedValue({ data: { status: 'ERROR', error: 'busy: UPLOADING' } }); + const self = stand({ confirmDelete: true }); + await App.methods.onDeleteImageClick.call(self); + expect(self.$waveui.notify).toHaveBeenCalledWith('busy: UPLOADING', 'error', 0); + }); + + it('is only offered for an image, while nothing is running', () => { + const visible = (over) => App.methods.isDeleteButtonVisible.call({ + options: { magicmode: false }, flash: { selectedMethod: 0 }, + selectedLocalImage: 'a.img.xz', state: 'IDLE', ...over, + }); + expect(visible({})).toBe(true); + expect(visible({ state: 'UPLOADING' })).toBe(false); + expect(visible({ selectedLocalImage: null })).toBe(false); + expect(visible({ options: { magicmode: true } })).toBe(false); + expect(visible({ flash: { selectedMethod: 1 } })).toBe(false); + }); }); describe('IntegrityChecker reports its verdict', () => { diff --git a/client/tests/unit/TheWifiSetup.spec.js b/client/tests/unit/TheWifiSetup.spec.js index 160482a..fa5eede 100644 --- a/client/tests/unit/TheWifiSetup.spec.js +++ b/client/tests/unit/TheWifiSetup.spec.js @@ -26,7 +26,7 @@ function mountDialog(open = false) { // Default answers for everything the dialog fetches on open, so individual // tests only have to say what is different. -function stubStatus(wifi = {}, aps = []) { +function stubStatus(wifi = {}, aps = [], attempt = null) { axios.get.mockImplementation((url) => { if (url === '/api/get_status') { return Promise.resolve({ @@ -40,6 +40,9 @@ function stubStatus(wifi = {}, aps = []) { if (url === '/api/wifi_poll_scan') { return Promise.resolve({ status: 200, data: aps }) } + if (url === '/api/wifi_poll_connect' && attempt) { + return Promise.resolve({ status: 200, data: attempt }) + } return Promise.reject(new Error('unexpected GET ' + url)) }) } @@ -290,7 +293,7 @@ describe('reconnect watch after a mode switch', () => { expect(wrapper.vm.reconnecting).toBe(true) // ...and once it really has switched, it still reports success. - stubStatus({ mode: 'station', ssid: 'HomeNet' }) + stubStatus({ mode: 'station', ssid: 'HomeNet', ip: '10.0.0.5' }) await vi.advanceTimersByTimeAsync(2500) expect(wrapper.vm.statusMessage).toBe('Connected to HomeNet.') }) @@ -314,6 +317,42 @@ describe('reconnect watch after a mode switch', () => { expect(wrapper.vm.reconnecting).toBe(false) }) + it('does not call it connected while the board is still associating (#149)', async () => { + // iwd names the network in "Connected network" while it is still + // connecting, so station + the right SSID arrives before any verdict. With + // a wrong passphrase that was announced as a connection and the watch + // stopped, never seeing the fall back to the hotspot. + stubStatus({ mode: 'ap', ssid: 'Recore' }) + const wrapper = mountDialog(true) + await settle(wrapper) + wrapper.vm.selected = { SSID: 'HomeNet' } + + await wrapper.vm.startWifiConnect() + await flushProbe() + stubStatus({ mode: 'station', ssid: 'HomeNet' }, [], { isConnecting: true, error: null }) + await vi.advanceTimersByTimeAsync(2500) + + expect(wrapper.vm.statusMessage).not.toContain('Connected to') + expect(wrapper.vm.reconnecting).toBe(true) + }) + + it('reports a failed attempt from the server verdict (#149)', async () => { + // The live case: the board was caught mid-association looking joined, and + // by the next reading the attempt had failed. The server knows; ask it. + stubStatus({ mode: 'ap', ssid: 'Recore' }) + const wrapper = mountDialog(true) + await settle(wrapper) + wrapper.vm.selected = { SSID: 'HomeNet' } + + await wrapper.vm.startWifiConnect() + await flushProbe() + stubStatus({ mode: 'station', ssid: 'HomeNet' }, [], { isConnecting: false, error: 'exit status 1' }) + await vi.advanceTimersByTimeAsync(2500) + + expect(wrapper.vm.statusMessage).toContain('Could not join HomeNet') + expect(wrapper.vm.reconnecting).toBe(false) + }) + it('settles when the hotspot switch completes', async () => { stubStatus({ mode: 'station', ssid: 'HomeNet' }) const wrapper = mountDialog(true) diff --git a/reflash/cancel_test.go b/reflash/cancel_test.go new file mode 100644 index 0000000..ab3b974 --- /dev/null +++ b/reflash/cancel_test.go @@ -0,0 +1,213 @@ +package main + +import ( + "encoding/json" + "net/http/httptest" + "os" + "os/exec" + "path/filepath" + "strings" + "syscall" + "testing" + "time" +) + +// A job that runs until its xz is killed, the way backup-emmc and +// flash-from-file do: xz at the end of a pipeline, pipefail on. `yes` rather +// than `sleep`, so the producer dies of SIGPIPE with xz instead of keeping the +// pipeline alive after it. +const xzJob = `set -o pipefail +yes | xz -0 > /dev/null` + +// pollUntilSettled drives get_progress the way the client does while a job +// runs, and returns every state it reported. Polling is the point: #152 was +// get_progress turning CANCELLED into IDLE underneath a job that had not +// exited yet. +func pollUntilSettled(t *testing.T) []string { + t.Helper() + var seen []string + deadline := time.Now().Add(15 * time.Second) + for time.Now().Before(deadline) { + w := httptest.NewRecorder() + getProgress(w, httptest.NewRequest("GET", "/api/get_progress", nil)) + var p struct { + State string `json:"state"` + } + if err := json.Unmarshal(w.Body.Bytes(), &p); err != nil { + t.Fatalf("bad get_progress body: %v", err) + } + seen = append(seen, p.State) + if p.State == CANCELLED || p.State == ERROR || p.State == FINISHED { + return seen + } + time.Sleep(50 * time.Millisecond) + } + t.Fatalf("job never settled; states seen: %v", seen) + return nil +} + +// settledAs polls until the job settles, then keeps polling for a while: in +// #152 the state read CANCELLED first and was overwritten with ERROR once the +// killed pipeline finally exited, so the first settled answer is not the last. +func settledAs(t *testing.T, want string) { + t.Helper() + seen := pollUntilSettled(t) + for end := time.Now().Add(2 * time.Second); time.Now().Before(end); { + w := httptest.NewRecorder() + getProgress(w, httptest.NewRequest("GET", "/api/get_progress", nil)) + var p struct { + State string `json:"state"` + } + _ = json.Unmarshal(w.Body.Bytes(), &p) + if p.State != IDLE { + seen = append(seen, p.State) + } + time.Sleep(50 * time.Millisecond) + } + for _, s := range seen { + if s == ERROR && want != ERROR { + t.Fatalf("reported ERROR on the way (states %v), want %q", seen, want) + } + } + if last := seen[len(seen)-1]; last != want { + t.Fatalf("ended as %q (states %v), want %q", last, seen, want) + } +} + +func TestCancelledBackupIsCancelledNotAnError(t *testing.T) { + dir := setupTest(t) + mounts := filepath.Join(dir, "mounts") + fakeBin(t, dir, "mount-unmount-usb", `echo "$@" >> `+mounts) + // Writes its image first, like the real script, so there is a partial file + // for the cancel to clean up. + fakeBin(t, dir, "backup-emmc", `echo partial > "$1.img.xz" +`+xzJob) + state = &State{State: IDLE} + + body := strings.NewReader(`{"filename":"mybackup"}`) + startBackup(httptest.NewRecorder(), httptest.NewRequest("PUT", "/api/start_backup", body)) + cancelBackup(httptest.NewRecorder(), httptest.NewRequest("PUT", "/api/cancel_backup", nil)) + + settledAs(t, CANCELLED) + // backup-emmc names its output