diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml new file mode 100644 index 0000000..c1f9f90 --- /dev/null +++ b/.github/workflows/test.yml @@ -0,0 +1,15 @@ +name: Test plugin + +on: + push: + pull_request: + +jobs: + backend: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: Shell syntax + run: bash -n backend.sh tests/*.sh tests/fake/* + - name: Backend regression suite + run: bash tests/run.sh diff --git a/Panel.qml b/Panel.qml index 29d3613..9400cd7 100644 --- a/Panel.qml +++ b/Panel.qml @@ -214,14 +214,15 @@ Panel { if (kind === "name") { if (!requestRename(profile)) return } else { - handOffToEditor(profile) + if (!handOffToEditor(profile)) return } close() } function handOffToEditor(profile) { + if (!wireguard.editConfig(profile, "")) return false editHandedOff = true - wireguard.editConfig(profile, "") + return true } // Returns whether the prompt opened, so callers know whether the panel has @@ -351,6 +352,10 @@ Panel { root.editHandedOff = false if (!root.opened) root.open() } + // Cancel, no-change and completed saves are terminal but not failures. + // Retire the UI-only marker so a later headless editor failure cannot + // mistake this panel for the caller that needs reopening. + function onEditFinished() { root.editHandedOff = false } } IpcHandler { @@ -361,9 +366,15 @@ Panel { function hide(): void { root.close() } // VPN toggle, not panel visibility — open/close/show/hide already cover // the popup, and the bar's left click promises the same thing. - function toggle(): void { wireguard.toggle() } - function refresh(): string { wireguard.refresh(); return "ok" } - function down(): string { wireguard.disconnectAll(); return "ok" } + function toggle(): string { + return wireguard.toggle() ? "ok" : "error: " + wireguard.actionRejection + } + function refresh(): string { + return wireguard.refresh() ? "ok" : "error: " + wireguard.actionRejection + } + function down(): string { + return wireguard.disconnectAll() ? "ok" : "error: " + wireguard.actionRejection + } function status(): string { return wireguard.statusText } // The connection grid without the panel. Rates and ping only move while // something is watching them, so a headless caller sees the totals and @@ -375,8 +386,7 @@ Panel { var name = wireguard.sanitizeName(path) if (!wireguard.isValidName(name)) return "error: cannot derive an interface name from " + path if (wireguard.countByIfname(name) > 1) return "error: ambiguous: several profiles use the interface " + name - wireguard.importFile(path, name) - return name + return wireguard.importFile(path, name) ? name : "error: " + wireguard.actionRejection } // Takes a connection name or a profile UUID; a name shared by several // profiles is refused rather than resolved to an arbitrary one. @@ -388,8 +398,7 @@ Panel { if (n > 1) return "error: ambiguous name: " + target + " — use a UUID: " + wireguard.uuidsForName(target).join(" ") profile = wireguard.findByName(target) } - wireguard.editConfig(profile, "") - return "ok" + return wireguard.editConfig(profile, "") ? "ok" : "error: " + wireguard.actionRejection } // Same target resolution as edit; the new name is a display label, so // anything single-line goes — except a name another profile already @@ -405,11 +414,14 @@ Panel { var value = String(newName || "").trim() if (value === "") return "error: the new name must not be empty" if (value !== profile.name && wireguard.countByName(value) > 0) return "error: a profile named " + value + " already exists" - wireguard.renameConfig(profile, value) - return "ok" + return wireguard.renameConfig(profile, value) ? "ok" : "error: " + wireguard.actionRejection + } + function importPick(): string { + return wireguard.pickConfigFile() ? "ok" : "error: " + wireguard.actionRejection + } + function importPaste(): string { + return wireguard.pasteConfig() ? "ok" : "error: " + wireguard.actionRejection } - function importPick(): string { wireguard.pickConfigFile(); return "ok" } - function importPaste(): string { wireguard.pasteConfig(); return "ok" } // Headless export — no warning dialog: an explicit path in argv is // already deliberate in a way a panel click is not. The file lands 0600. function exportConfig(target: string, path: string): string { @@ -421,8 +433,7 @@ Panel { profile = wireguard.findByName(target) } if (String(path || "") === "") return "error: no destination path" - wireguard.exportToPath(profile, path) - return "ok" + return wireguard.exportToPath(profile, path) ? "ok" : "error: " + wireguard.actionRejection } // The QR has its own window, so this never touches the panel — a // headless caller gets the code centred on screen and nothing else. @@ -504,8 +515,7 @@ Panel { // in the way of zenity and of the rename window as it is of the QR. else if (t === "e" || t === "E") { if (root.cursorActive && root.focusSection === "configs") { - root.handOffToEditor(root.selectedProfile()) - root.close() + if (root.handOffToEditor(root.selectedProfile())) root.close() } } else if (t === "n" || t === "N") { diff --git a/README.md b/README.md index 711782d..ae1cc6e 100644 --- a/README.md +++ b/README.md @@ -32,7 +32,8 @@ containing hook directives are rejected with a clear message. - **`wireguard-tools`** — only `wg` is used, to validate keys before import. - **`zenity`** — optional, for the file picker and the config editor. `kdialog` or `yad` also work for the file picker. -- **`wl-clipboard`** — optional, for importing a config from the clipboard. +- **`wl-clipboard`** — optional, for importing a config from the clipboard + and copying connection details. - **`qrencode`** — optional, for showing a profile as a QR code. - **`notify-send`** (libnotify) — optional, for the toast when a tunnel is deactivated externally. @@ -43,13 +44,12 @@ No new privileges for any of it: everything still runs as your user. ```bash omarchy plugin add https://github.com/glafeara/omarchy-wireguard.git -omarchy plugin enable glafeara.wireguard -omarchy bar plugin add glafeara.wireguard right +omarchy plugin enable glafeara.wireguard right ``` -Plugins land disabled so you can read the code before enabling it — this one -is three QML files and one shell script. Add `--yes` to any of the commands -to skip the prompts. +`plugin add` is interactive by default; add `--yes` to that command to skip +its prompt. The explicit plugin id and `right` placement make `plugin enable` +non-interactive. The plugin itself is five QML files and one shell script. ## Using it @@ -139,6 +139,15 @@ itself failed — a plain statement that the state is unknown. A successful activation confirms NetworkManager's state, not the peer's reachability: WireGuard has no connected/disconnected handshake state to report. +Replacing a profile during import is transactional too. If NetworkManager +refuses deletion of the old active profile and also refuses its rollback, +the operation reports that the state is unknown (backend exit `6`) instead +of implying that only the deletion failed. Both the old and fully built +replacement profiles are retained for manual recovery; the error names their +UUIDs rather than discarding the only new configuration. Exit `6` also marks +a failed cleanup of an incomplete replacement, so editor saves never retry +automatically on top of a profile that NetworkManager refused to remove. + **Renaming** changes the profile's display name (`connection.id`) only — spaces are fine, duplicates are refused. The interface name never changes; that one obeys kernel rules and belongs to import. The prompt opens in its @@ -189,8 +198,8 @@ they were. | `pingHost` | `1.1.1.1` | any host, or empty to disable the probe | ```bash -omarchy bar plugin set glafeara.wireguard refreshIntervalSec 30 -omarchy bar plugin set glafeara.wireguard pingHost "" # no latency probe +omarchy bar set glafeara.wireguard refreshIntervalSec 30 +omarchy bar set glafeara.wireguard pingHost "" # no latency probe ``` ## IPC @@ -211,7 +220,12 @@ omarchy-shell glafeara.wireguard exportConfig kz ~/kz.conf # 0600, private key omarchy-shell glafeara.wireguard qr kz # QR window, centred on screen ``` -Name-based commands refuse an ambiguous name and list the matching UUIDs +Commands that start an asynchronous action return `ok` only when it has +actually been accepted. Control actions reject another running control action; +picker, clipboard import, QR and export reject only their own already-running +worker, while an editor may open during a control action and queues its save. +An editor rejects a second editor or a queued editor save. `rename` to the +current name is an idempotent `ok`. Name-based commands refuse an ambiguous name and list the matching UUIDs instead — pass a UUID to disambiguate. `details` answers with the addresses whether or not the panel is open, and with `--` for everything sampled — rates, totals and ping — because sampling stops with the panel. A figure @@ -228,10 +242,16 @@ an old one. connected, so the bar's quick toggle reconnects what you actually used. - `$XDG_RUNTIME_DIR/omarchy-wireguard..{lock,intent,notified}` — the cross-instance lock, the short-lived "this deactivation was ours" - markers behind the notifications, and the toast cooldown stamp. tmpfs, - gone at reboot. -- `$XDG_RUNTIME_DIR/wg-qr.*.png` — the QR image while its window is open; - deleted on close. + markers behind the notifications, and the toast cooldown stamp. The + backend requires a private, current-user runtime directory (or its safe + `/run/user/` fallback) and refuses to use `/tmp`. These files are + private and gone at reboot. +- `$XDG_RUNTIME_DIR/wg-qr..*.png` — the QR image while its window + is open; deleted on close. On the next shell startup, images whose owner + PID is dead are safely reaped without touching another live monitor's QR. +- `$XDG_RUNTIME_DIR/wg-edit..*` — private editor buffers and + result files while zenity is open; deleted on every editor exit and reaped + on the next shell startup after a crash. - `/sys/class/net//statistics/{rx,tx}_bytes` — read-only, for the traffic line, plus the interface's address and MTU for the detail grid. - **One ICMP echo to `pingHost` every three seconds while the panel is @@ -246,22 +266,19 @@ scripts, no services, no telemetry. ## Tests -`tests/` holds four suites that run the backend against a fake `nmcli` on +`tests/` holds backend suites that run against fake `nmcli`, `wg`, and +`qrencode` commands on `PATH` — the only way to exercise the switch rollback paths, since a dead endpoint does not make `nmcli connection up` fail — plus a manual checklist (`tests/checklist.md`) for what needs real tunnels: ```bash -bash tests/test-status.sh -bash tests/test-connect.sh -bash tests/test-notify.sh -bash tests/test-details.sh +bash tests/run.sh ``` ## Uninstall ```bash -omarchy bar plugin remove glafeara.wireguard omarchy plugin remove glafeara.wireguard ``` diff --git a/Service.qml b/Service.qml index ea7221f..127236e 100644 --- a/Service.qml +++ b/Service.qml @@ -35,6 +35,10 @@ Item { property string lastUuid: "" property string actionStatus: "" property string lastError: "" + // Public action methods return true only after they actually started work. + // IPC uses actionRejection to avoid acknowledging a request that a busy + // widget would otherwise quietly discard. + property string actionRejection: "" readonly property bool busy: controlProcess.running readonly property string statusText: active ? "VPN: " + activeNames.join(" ") : "VPN disconnected" @@ -166,13 +170,38 @@ Item { return n } + function rejectAction(reason) { + actionRejection = String(reason) + lastError = actionRejection + return false + } + function refresh() { - if (statusProcess.running) return + // Timer and manual refreshes coalesce. This is not an operation failure: + // recording it in lastError would make the bar urgent forever after a + // normal timer overlap, but IPC can still return the rejection reason. + if (statusProcess.running) { + actionRejection = "a refresh is already running" + return false + } + actionRejection = "" // The observation time for mark-active is when this snapshot is // *requested* — anything that happens while the poll runs or waits to // be parsed is "after the observation" and must keep its marker. _statusStartedAt = Math.floor(Date.now() / 1000) statusProcess.running = true + return true + } + + // Control operations need a post-change snapshot, even if an earlier status + // poll is in flight. Timers and manual refreshes deliberately do not queue: + // otherwise a slow status command could keep polling forever. + function refreshAfterChange() { + if (statusProcess.running) { + _refreshAfterStatus = true + return false + } + return refresh() } function sampleTraffic() { @@ -517,43 +546,66 @@ Item { } function connectTo(profile) { - if (busy || !profile || !profile.uuid) return + if (busy) return rejectAction("another WireGuard operation is already running") + if (!profile || !profile.uuid) return rejectAction("no such profile") + actionRejection = "" actionStatus = "Connecting " + profile.name + "…" _pendingConnect = String(profile.uuid) runControl(["connect", profile.uuid]) + return true } function disconnectOne(profile) { - if (busy || !profile || !profile.uuid) return + if (busy) return rejectAction("another WireGuard operation is already running") + if (!profile || !profile.uuid) return rejectAction("no such profile") + actionRejection = "" actionStatus = "Disconnecting " + profile.name + "…" runControl(["down", profile.uuid]) + return true } function disconnectAll() { - if (busy) return + if (busy) return rejectAction("another WireGuard operation is already running") + actionRejection = "" actionStatus = "Disconnecting…" runControl(["down-all"]) + return true } function deleteConfig(profile) { - if (busy || !profile || !profile.uuid) return + if (busy) return rejectAction("another WireGuard operation is already running") + if (!profile || !profile.uuid) return rejectAction("no such profile") + actionRejection = "" actionStatus = "Deleting " + profile.name + "…" runControl(["delete", profile.uuid]) + return true } // Changes connection.id only — a display label, so spaces and length are // fine. The interface name never moves; that is import's job. function renameConfig(profile, newName) { - if (busy || !profile || !profile.uuid) return + if (busy) return rejectAction("another WireGuard operation is already running") + if (!profile || !profile.uuid) return rejectAction("no such profile") var value = String(newName || "").trim() - if (value === "" || value === profile.name) return + if (value === "") return rejectAction("the new name must not be empty") + // Idempotent success: no backend call is needed when it is already named + // as requested, but IPC may still honestly answer ok. + if (value === profile.name) { + actionRejection = "" + lastError = "" + actionStatus = "" + return true + } + actionRejection = "" actionStatus = "Renaming " + profile.name + "…" runControl(["rename", profile.uuid, value]) + return true } function toggle() { - if (active) disconnectAll() - else if (toggleProfile !== null) connectTo(toggleProfile) + if (active) return disconnectAll() + if (toggleProfile !== null) return connectTo(toggleProfile) + return rejectAction("no WireGuard profile is available") } // Import: a picked file or pasted text becomes an NM connection profile, @@ -563,17 +615,23 @@ Item { signal importReady(string kind, string payload, string suggestedName) function pickConfigFile() { - if (busy || pickerProcess.running) return + if (busy) return rejectAction("another WireGuard operation is already running") + if (pickerProcess.running) return rejectAction("the file picker is already open") + actionRejection = "" lastError = "" actionStatus = "Waiting for the file picker…" pickerProcess.running = true + return true } function pasteConfig() { - if (busy || clipboardProcess.running) return + if (busy) return rejectAction("another WireGuard operation is already running") + if (clipboardProcess.running) return rejectAction("clipboard import is already running") + actionRejection = "" lastError = "" actionStatus = "Reading clipboard…" clipboardProcess.running = true + return true } // Opens the profile as wg-quick text in zenity's editable view (the @@ -590,9 +648,17 @@ Item { // closed the panel to get out of zenity's way, and lastError has nowhere // to be read. A cancelled or unchanged edit is not a failure. signal editFailed(string reason) + // Terminal editor outcomes that need no panel rescue: cancellation, + // unchanged text, and a completed save. Panel uses this only to retire its + // handoff marker, so a later headless failure cannot reopen the panel. + signal editFinished() function editConfig(profile, seedText) { - if (!profile || !profile.uuid || editProcess.running) return + if (!profile || !profile.uuid) return rejectAction("no such profile") + if (editProcess.running) return rejectAction("the editor is already open") + if (_pendingSaveUuid !== "" || _editRetryUuid !== "") + return rejectAction("a previous editor save is still pending") + actionRejection = "" _editUuid = String(profile.uuid) _editName = String(profile.name) if (!seedText) lastError = "" @@ -603,6 +669,7 @@ Item { editProcess.stdinEnabled = true editProcess.command = ["bash", backendPath, "edit", _editUuid, _editName] editProcess.running = true + return true } // Export hands the config — private key included — out of NetworkManager's @@ -620,14 +687,17 @@ Item { readonly property bool qrVisible: qrLoading || qrPath !== "" || qrError !== "" function exportToPath(profile, path) { - if (!profile || !profile.uuid || exportProcess.running) return + if (!profile || !profile.uuid) return rejectAction("no such profile") + if (exportProcess.running) return rejectAction("an export is already running") var dest = String(path || "") - if (dest === "") return + if (dest === "") return rejectAction("no destination path") + actionRejection = "" lastError = "" _exportDest = dest actionStatus = "Exporting " + profile.name + "…" exportProcess.command = ["bash", backendPath, "export-file", profile.uuid, dest] exportProcess.running = true + return true } // Returns "" when a code is on its way, or why nothing will appear. @@ -662,16 +732,31 @@ Item { // The process itself is left to finish: killing qrencode mid-write would // strand the file mktemp already created. _qrWanted = false - if (qrPath !== "") { - qrCleanupProcess.command = ["rm", "-f", "--", qrPath] - qrCleanupProcess.running = true - } + removeQrFile(qrPath) qrPath = "" qrName = "" qrLoading = false qrError = "" } + // Each path gets its own detached remover. A shared Process can have its + // command overwritten by a second close or an unwanted render result, + // leaving private-key PNGs behind; detached children also outlive Service + // destruction long enough to perform this tiny, path-specific cleanup. + function removeQrFile(path) { + var knownPath = String(path || "") + if (knownPath !== "") Quickshell.execDetached(["rm", "-f", "--", knownPath]) + } + + // A shell reload destroys this Service without a window-close signal. The + // current PNG is known to this instance, so remove only that path; do not + // sweep wg-qr.* broadly because another monitor can legitimately own one. + Component.onDestruction: closeQr() + // SIGKILL and a hard shell crash cannot run the destruction handler. The + // backend identifies PNGs by this shell's parent PID, so startup safely + // reaps files from a dead previous shell without touching a live monitor. + Component.onCompleted: Quickshell.execDetached(["bash", backendPath, "cleanup-runtime"]) + function _flushDrops() { if (notifyProcess.running || _dropQueue.length === 0) return var drop = _dropQueue.shift() @@ -704,14 +789,16 @@ Item { // profile the user never pointed at. The panel blocks this earlier with a // clearer message; this is the backstop for the headless entry points. function importFile(path, name) { - if (busy || !path || !name) return + if (busy) return rejectAction("another WireGuard operation is already running") + if (!path || !name) return rejectAction("a config path and name are required") if (countByIfname(name) > 1) { - lastError = "Several profiles use the interface " + name + " — not replacing an ambiguous match" - return + return rejectAction("several profiles use the interface " + name + " — not replacing an ambiguous match") } + actionRejection = "" var existing = findByIfname(name) actionStatus = "Importing " + name + "…" runControl(["import", String(name), existing ? existing.uuid : "", String(path)]) + return true } // Writes a queued editor save once controlProcess is free. Bypasses @@ -736,14 +823,16 @@ Item { } function importText(text, name) { - if (busy || !text || !name) return + if (busy) return rejectAction("another WireGuard operation is already running") + if (!text || !name) return rejectAction("config text and a name are required") if (countByIfname(name) > 1) { - lastError = "Several profiles use the interface " + name + " — not replacing an ambiguous match" - return + return rejectAction("several profiles use the interface " + name + " — not replacing an ambiguous match") } + actionRejection = "" var existing = findByIfname(name) actionStatus = "Importing " + name + "…" runControl(["import", String(name), existing ? existing.uuid : ""], String(text)) + return true } // The connection (and interface) is named after the file, and the kernel @@ -849,6 +938,9 @@ Item { // True while lastError describes a failed status poll, so a successful // poll knows it may clear it. property bool _pollError: false + // One coalesced poll requested while statusProcess was running. This keeps + // post-control state fresh even if the normal interval is as high as 3600s. + property bool _refreshAfterStatus: false property string _pendingConnect: "" property string _exportDest: "" // The UUID the running details query is about — the answer is filed under @@ -976,6 +1068,10 @@ Item { root.lastError = root.elide(statusStderr.text || "Failed to read WireGuard status") root._pollError = true } + if (root._refreshAfterStatus) { + root._refreshAfterStatus = false + Qt.callLater(root.refreshAfterChange) + } } } @@ -1028,11 +1124,13 @@ Item { return } // 3 = Cancel, 4 = nothing changed; neither is worth a message. + if (exitCode === 3 || exitCode === 4) { + root.editFinished() + return + } if (exitCode !== 0) { - if (exitCode !== 3 && exitCode !== 4) { - root.lastError = "Could not open " + name - root.editFailed(root.lastError) - } + root.lastError = "Could not open " + name + root.editFailed(root.lastError) return } var text = String(editStdout.text || "") @@ -1120,10 +1218,7 @@ Item { // The window closed while we rendered: nobody is waiting for this, and // a successful result is key material — delete it and say nothing. if (!root._qrWanted) { - if (path !== "") { - qrCleanupProcess.command = ["rm", "-f", "--", path] - qrCleanupProcess.running = true - } + root.removeQrFile(path) return } if (exitCode === 0 && path !== "") { @@ -1135,12 +1230,6 @@ Item { } } - Process { - id: qrCleanupProcess - running: false - command: [] - } - Process { id: notifyProcess running: false @@ -1259,6 +1348,11 @@ Item { // editor would target the already-deleted old profile — so the retry // state is cleared and only the reason is shown. var savedNotUp = op === "import" && exitCode === 5 + var completedEditorSave = op === "import" && root._editRetryName !== "" + // 6 means import needs manual recovery: rollback or incomplete-profile + // cleanup kept one or more replacements. Never reopen an editor + // automatically: another save could hide that recovery state. + var importStateUnknown = op === "import" && exitCode === 6 if (exitCode === 0 || savedNotUp) { if (root._pendingConnect !== "") root.rememberLast(root._pendingConnect) root.lastError = savedNotUp @@ -1268,19 +1362,27 @@ Item { root._editRetryUuid = "" root._editRetryName = "" root._editRetryText = "" + if (completedEditorSave) root.editFinished() } else { root.actionStatus = "" // 20/21 (connect only): the switch failed; the backend's stderr says // whether the previous tunnels were restored (20) or the rollback // itself failed (21). Either way the poll below shows what is up. var reason = root.elide(root._controlError || "NetworkManager operation failed") + if (importStateUnknown) { + root.lastError = reason + root.editFailed(reason) + root._editRetryUuid = "" + root._editRetryName = "" + root._editRetryText = "" // A write refused by wg_check must not cost the edit that produced // it; hand the text back to the editor with the reason attached. - if (op === "import" && root._editRetryName !== "") root.retryEdit(root._editRetryUuid, root._editRetryName, root._editRetryText, reason) - else root.lastError = reason + } else if (op === "import" && root._editRetryName !== "") { + root.retryEdit(root._editRetryUuid, root._editRetryName, root._editRetryText, reason) + } else root.lastError = reason } root._pendingConnect = "" - root.refresh() + root.refreshAfterChange() // An edit can move the address, the endpoint or the routes without // moving the tunnel, so the grid is refetched even when the primary // profile is the one it already describes. diff --git a/backend.sh b/backend.sh index bb215a4..35646a1 100755 --- a/backend.sh +++ b/backend.sh @@ -38,13 +38,17 @@ # text (stdin, or [file]); replaces [old-uuid], # keeping its connection.id and interface-name, # and reconnects if it was active. Exit 5 means -# the profile was saved but reconnecting failed. +# the profile was saved but reconnecting failed; +# exit 6 means manual recovery is required +# (rollback or incomplete-profile cleanup left +# one or both profiles in an unknown state). # export print the profile as wg-quick config text # export-file write the config to a file, mode 0600 # qr-png render the config as a QR PNG in # XDG_RUNTIME_DIR, print its path (exit 2 = # qrencode missing; the caller deletes the # file when done) +# cleanup-runtime remove QR/editor files owned by a dead shell # edit zenity editor round-trip (seed text on stdin) # notify-drop decide whether an observed deactivation was # external; exit 0 = external (toast sent, @@ -58,6 +62,7 @@ set -o pipefail # value like "Address = *" must stay a literal asterisk, not a file list. set -f export LC_ALL=C +umask 077 die() { printf '%s\n' "$*" >&2; exit 1; } @@ -66,10 +71,35 @@ need() { command -v "$1" >/dev/null 2>&1 || die "$1 is not installed"; } # One writer at a time, across every widget instance (one per monitor). # The fd stays open for the life of the process — including through the # `exec nmcli` tail calls — so the lock covers the whole operation. +RUNTIME_DIR="" + +# State files coordinate separate bar instances and can suppress desktop +# notifications, so they must never fall back to a predictable path in /tmp. +# XDG_RUNTIME_DIR is normally a mode-0700, per-user tmpfs. A non-desktop +# invocation may not inherit it; /run/user/ is the same safe location +# when it exists. Refuse anything else rather than following a symlink or +# truncating an attacker-controlled file. +ensure_runtime_dir() { + [ -n "$RUNTIME_DIR" ] && return 0 + local dir="${XDG_RUNTIME_DIR:-/run/user/$(id -u)}" mode + [ -n "$dir" ] && [ -d "$dir" ] && [ ! -L "$dir" ] && [ -O "$dir" ] || + die "A private XDG_RUNTIME_DIR is required for WireGuard state" + mode="$(stat -Lc '%a' -- "$dir" 2>/dev/null)" || + die "Cannot inspect XDG_RUNTIME_DIR" + case "$mode" in + ''|*[!0-7]*) die "XDG_RUNTIME_DIR has unsafe permissions" ;; + esac + # The XDG runtime directory is private (0700): even state names and marker + # timing should not be exposed to another local account. + [ $((8#$mode & 0077)) -eq 0 ] || + die "XDG_RUNTIME_DIR has unsafe permissions" + RUNTIME_DIR="$dir" +} + lock() { need flock - local dir="${XDG_RUNTIME_DIR:-/tmp}" - exec 9>>"$dir/omarchy-wireguard.$(id -u).lock" || die "Cannot open the lock file" + ensure_runtime_dir + exec 9>>"$RUNTIME_DIR/omarchy-wireguard.$(id -u).lock" || die "Cannot open the lock file" flock -w 30 9 || die "Another WireGuard operation is already running" } @@ -88,7 +118,10 @@ lock() { INTENT_TTL=7200 -runtime_state() { printf '%s/omarchy-wireguard.%s.%s' "${XDG_RUNTIME_DIR:-/tmp}" "$(id -u)" "$1"; } +runtime_state() { + ensure_runtime_dir + printf '%s/omarchy-wireguard.%s.%s' "$RUNTIME_DIR" "$(id -u)" "$1" +} # Marker-file helpers. Failures are swallowed: a missed marker costs one # spurious notification, not a broken operation. Both rewrites drop expired @@ -392,7 +425,10 @@ cmd_rename() { # The QML refuses duplicates too, but each widget instance judges by its # own possibly-stale snapshot — this check under the flock is the # authoritative one. NetworkManager itself would happily take the dup. - nmcli --escape no -t -f NAME connection show | grep -qxF "$new_id" && + local names + names="$(nmcli --escape no -t -f NAME connection show)" || + die "Could not list existing profile names" + printf '%s\n' "$names" | grep -qxF "$new_id" && die "A profile named $new_id already exists" exec nmcli connection modify "$uuid" connection.id "$new_id" } @@ -519,7 +555,7 @@ parse_config() { } cmd_import() { - local name="$1" old_uuid="${2:-}" src="${3:-}" + local name="$1" old_uuid="${2:-}" src="${3:-}" uuid="" import_committed=0 old_was_active=0 need wg need nmcli # Replacing keeps the old profile's identity. connection.id is a free-form @@ -544,8 +580,70 @@ cmd_import() { fi parse_config + # Allocate the UUID ourselves. nmcli's success message is human-readable + # output and not an API; parsing it could leave an orphan if its wording + # changes. Linux supplies a dependency-free, cryptographically random UUID. + uuid="$(cat /proc/sys/kernel/random/uuid 2>/dev/null)" || + die "Could not allocate a UUID for the imported profile" + [[ "$uuid" =~ ^[0-9a-f]{8}(-[0-9a-f]{4}){3}-[0-9a-f]{12}$ ]] || + die "Could not allocate a UUID for the imported profile" + cleanup_import() { + [ "$import_committed" = 1 ] && return 0 + remove_incomplete_replacement() { + if ! nmcli connection delete "$uuid" >/dev/null 2>&1; then + printf '%s\n' "Could not remove incomplete replacement $uuid; state is unknown — check connections manually" >&2 + # Keeping the replacement is safer than retrying an editor save on + # top of it. Disarm the EXIT trap before terminal recovery exit 6. + import_committed=1 + trap - EXIT HUP INT TERM + exit 6 + fi + } + # A signal can be delivered while nmcli is finishing old-profile delete. + # Inspect the resulting state before deleting the replacement: if old is + # gone, preserving new is the only outcome that cannot leave zero usable + # profiles. If old remains and was active, restore it *before* deleting + # new; a failed restoration keeps both profiles and says the state is + # unknown. A query failure likewise preserves new rather than destroying + # the only profile that may remain. + if [ -n "$old_uuid" ]; then + if nmcli --escape no -g connection.id connection show "$old_uuid" >/dev/null 2>&1; then + if [ "$old_was_active" = 1 ]; then + if nmcli connection up "$old_uuid" >/dev/null 2>&1; then + clear_intent "$old_uuid" + remove_incomplete_replacement + else + import_committed=1 + printf '%s\n' "Interrupted replacement left tunnel state unknown. Kept replacement $uuid and old profile $old_uuid — check your connections manually" >&2 + trap - EXIT HUP INT TERM + exit 6 + fi + else + remove_incomplete_replacement + fi + else + import_committed=1 + printf '%s\n' "Could not determine whether old profile $old_uuid remains. Kept replacement $uuid; state is unknown — check your connections manually" >&2 + trap - EXIT HUP INT TERM + exit 6 + fi + else + remove_incomplete_replacement + fi + } + # Once connection add is attempted, every error and signal removes the + # temporary profile until the replacement transaction commits. + trap 'cleanup_import' EXIT + trap 'if [ "$import_committed" = 1 ]; then trap - EXIT HUP INT TERM; printf "%s\n" "Saved, but reconnecting was interrupted" >&2; exit 5; else exit 1; fi' HUP INT TERM + commit_import() { + import_committed=1 + # Keep the signal handler: it maps a later interruption to saved-but-not- + # reconnected exit 5. Only the destructive EXIT cleanup is disarmed. + trap - EXIT + } + local -a args=(connection add type wireguard - con-name ".$ifname.import.$$" ifname "$ifname" autoconnect no) + con-name ".$ifname.import.$$" connection.uuid "$uuid" ifname "$ifname" autoconnect no) if [ -n "$ADDR4" ]; then args+=(ipv4.method manual ipv4.addresses "$ADDR4") else args+=(ipv4.method disabled); fi if [ -n "$ADDR6" ]; then args+=(ipv6.method manual ipv6.addresses "$ADDR6") @@ -569,16 +667,10 @@ cmd_import() { args+=(ipv4.route-table "$TABLE" ipv6.route-table "$TABLE") ;; esac - local out uuid + local out out="$(nmcli "${args[@]}" 2>&1)" || die "$out" - uuid="$(printf '%s\n' "$out" | grep -oE '[0-9a-f]{8}(-[0-9a-f]{4}){3}-[0-9a-f]{12}' | head -1)" - [ -n "$uuid" ] || die "Could not determine the new profile's UUID" - - # From here on a failure must not leave the half-built profile behind. - fail() { - nmcli connection delete "$uuid" >/dev/null 2>&1 - die "$1" - } + # From here on a failure is covered by cleanup_import's EXIT trap. + fail() { die "$1"; } # Secrets go in over the interactive editor's stdin, not argv. local feed peer @@ -616,11 +708,14 @@ cmd_import() { # tunnels while the editor sat open. out="$(nmcli connection modify "$uuid" connection.id "$con_id" 2>&1)" || fail "Could not rename the imported profile: $out" + # A fresh profile is persistent and complete at this point. There is no + # old profile to protect, so later signals must keep the usable new one. + [ -z "$old_uuid" ] && commit_import if [ -n "$old_uuid" ]; then - local was_active=0 active + local active active="$(active_wg_uuids)" || fail "Could not list active connections" - printf '%s\n' "$active" | grep -qxF "$old_uuid" && was_active=1 - if [ "$was_active" = 1 ]; then + printf '%s\n' "$active" | grep -qxF "$old_uuid" && old_was_active=1 + if [ "$old_was_active" = 1 ]; then mark_down "$old_uuid" out="$(nmcli connection down "$old_uuid" 2>&1)" || { clear_intent "$old_uuid" @@ -630,11 +725,27 @@ cmd_import() { out="$(nmcli connection delete "$old_uuid" 2>&1)" || { # Roll back: the tunnel was only taken down for the swap, so a failed # swap must not leave the VPN silently off. - [ "$was_active" = 1 ] && nmcli connection up "$old_uuid" >/dev/null 2>&1 && - clear_intent "$old_uuid" + if [ "$old_was_active" = 1 ]; then + if nmcli connection up "$old_uuid" >/dev/null 2>&1; then + clear_intent "$old_uuid" + else + # Both profiles still exist, but neither activation outcome is + # trustworthy. Keep the fully built replacement as well as old; + # deleting it here would throw away the only new configuration. + commit_import + trap - HUP INT TERM + printf '%s\n' "Could not delete the old profile: $out" >&2 + printf '%s\n' "Rollback failed; tunnel state is unknown. Kept replacement $uuid and old profile $old_uuid — check your connections manually" >&2 + exit 6 + fi + fi fail "Could not delete the old profile: $out" } - if [ "$was_active" = 1 ]; then + # The old profile is gone. From this exact boundary the replacement is + # committed even if activation is interrupted or fails (exit 5): deleting + # it in an EXIT trap here would leave the user with neither profile. + commit_import + if [ "$old_was_active" = 1 ]; then # The replacement is complete; a failed activation must not delete it, # and must not read as a failed save either — the UI would reopen the # editor against the already-deleted old profile. Exit 5 says "saved, @@ -646,6 +757,8 @@ cmd_import() { clear_intent "$uuid" fi fi + [ "$import_committed" = 1 ] || commit_import + trap - HUP INT TERM } # --------------------------------------------------------------------------- @@ -658,6 +771,9 @@ cmd_export() { local pk="" port="" mtu="" fwmark="" peer_routes="" rtable="" local addr4="" addr6="" dns4="" dns6="" dnssearch="" peers="" local line key value + local raw + raw="$(nmcli -s -t connection show "$uuid")" || + die "Could not read the profile from NetworkManager" while IFS= read -r line; do key="${line%%:*}" value="$(unescape "${line#*:}")" @@ -675,7 +791,7 @@ cmd_export() { ipv6.dns) dns6="$value" ;; ipv4.dns-search) dnssearch="$value" ;; esac - done < <(nmcli -s -t connection show "$uuid") + done <<< "$raw" [ -n "$pk" ] || die "Could not read the private key from NetworkManager" local joined @@ -725,8 +841,9 @@ cmd_export() { # with mode 0600 no matter what existed at the destination: written as a # 0600 temp in the destination directory, then renamed over the target — # plain `umask 077` + `>` would keep a pre-existing file's 0644. The QR PNG -# lives only in XDG_RUNTIME_DIR (tmpfs on Omarchy, mode 0700, gone by -# reboot). +# lives only in XDG_RUNTIME_DIR (tmpfs on Omarchy, mode 0700). Explicit close +# and Service destruction remove it; a hard crash is reaped on the next +# Service startup by cleanup-runtime. # --------------------------------------------------------------------------- cmd_export_file() { @@ -740,7 +857,8 @@ cmd_export_file() { [ -d "$dir" ] || die "No such directory: $dir" umask 077 tmp="$(mktemp -- "$dir/.wg-export.XXXXXX")" || die "Cannot create a file in $dir" - trap 'rm -f "$tmp"' EXIT HUP INT TERM + trap 'rm -f "$tmp"; exit 1' HUP INT TERM + trap 'rm -f "$tmp"' EXIT cmd_export "$uuid" > "$tmp" || die "Could not export the profile" # -T: dest is the file itself, never a directory to move into. mv -fT -- "$tmp" "$dest" || die "Could not write $dest" @@ -749,23 +867,57 @@ cmd_export_file() { # Renders the PNG and prints its path — the panel displays it inline and # deletes it when the QR dialog closes, so the file's lifetime belongs to -# the caller, not to a trap here. XDG_RUNTIME_DIR is tmpfs with mode 0700: -# nothing lands on disk, and a crashed caller leaks the file only until -# reboot. +# the caller, not to a trap here. XDG_RUNTIME_DIR is tmpfs with mode 0700; +# a crashed caller's owned file is safely reaped on the next Service start. cmd_qr_png() { local uuid="$1" dir png command -v qrencode >/dev/null 2>&1 || { echo "qrencode is not installed — sudo pacman -S qrencode" >&2; exit 2; } - dir="${XDG_RUNTIME_DIR:-}" - [ -n "$dir" ] && [ -d "$dir" ] || - die "XDG_RUNTIME_DIR is not available — refusing to write key material anywhere less private" - umask 077 - png="$(mktemp -- "$dir/wg-qr.XXXXXX.png")" || die "Cannot create a file in $dir" + ensure_runtime_dir + dir="$RUNTIME_DIR" + # The parent is Quickshell's Process child. All monitors in one shell share + # that PID, so cleanup-runtime can distinguish a crashed old shell from a live + # sibling monitor without sweeping another visible QR. + png="$(mktemp -- "$dir/wg-qr.$PPID.XXXXXX.png")" || die "Cannot create a file in $dir" + # A trapped signal is otherwise handled after the foreground encoder exits + # and bash would continue to print a now-deleted path as a false success. + trap 'rm -f -- "$png"; exit 1' HUP INT TERM + trap 'rm -f -- "$png"' EXIT cmd_export "$uuid" | qrencode -t PNG -s 6 -m 2 -o "$png" || - { rm -f "$png"; die "Could not encode the config as a QR code"; } + die "Could not encode the config as a QR code" + trap - EXIT HUP INT TERM printf '%s\n' "$png" } +# Remove only known-safe stale secret-bearing runtime files. New-format names +# carry their shell owner PID; a live owner may still have a window open on +# another monitor. Legacy QR names predate ownership and are retained for a +# day so an in-place plugin update cannot erase a displayed old-format QR. +cmd_cleanup_runtime() { + local dir png base owner now mtime + ensure_runtime_dir + dir="$RUNTIME_DIR" + now="$(date +%s)" + # Globbing is disabled globally while parsing config values, so enumerate + # through find rather than temporarily re-enabling it around key material. + while IFS= read -r -d '' png; do + [ -f "$png" ] && [ ! -L "$png" ] || continue + base="${png##*/}" + if [[ "$base" =~ ^wg-qr\.([0-9]+)\.[A-Za-z0-9]{6}\.png$ || "$base" =~ ^wg-edit\.([0-9]+)\.[A-Za-z0-9]{6}$ ]]; then + owner="${BASH_REMATCH[1]}" + kill -0 "$owner" 2>/dev/null && continue + rm -f -- "$png" + continue + fi + # Legacy wg-qr.XXXXXX.png: no owner to inspect, so only reap a clearly + # stale regular file. PID reuse remains a theoretical limitation of the + # ownership format and is documented in the manual checklist. + mtime="$(stat -Lc '%Y' -- "$png" 2>/dev/null)" || continue + case "$mtime" in ''|*[!0-9]*) continue ;; esac + [ $((now - mtime)) -gt 86400 ] && rm -f -- "$png" + done < <(find "$dir" -maxdepth 1 -type f \( -name 'wg-qr.*.png' -o -name 'wg-edit.*' \) -print0) +} + # --------------------------------------------------------------------------- # edit: zenity round-trip. Exit codes: 2 = no zenity, 3 = Cancel, # 4 = saved with no changes. Seed text (a rejected edit being retried) @@ -773,13 +925,20 @@ cmd_qr_png() { # --------------------------------------------------------------------------- cmd_edit() { - local uuid="$1" name="$2" seed src tmp buf + local uuid="$1" name="$2" seed src dir tmp buf seed="$(cat)" command -v zenity >/dev/null 2>&1 || exit 2 src="$(cmd_export "$uuid")" || exit 1 - umask 077 - tmp="$(mktemp)" && buf="$(mktemp)" || exit 1 - trap 'rm -f "$tmp" "$buf"' EXIT + ensure_runtime_dir + dir="$RUNTIME_DIR" + # Both files contain a complete WireGuard config. Keep them in the private + # runtime directory, never /tmp, and tag them for safe crash cleanup. + tmp="$(mktemp -- "$dir/wg-edit.$PPID.XXXXXX")" || exit 1 + # Install cleanup before attempting the second allocation: it can fail + # after tmp already holds private config output. + trap 'rm -f -- "$tmp" "${buf:-}"; exit 1' HUP INT TERM + trap 'rm -f -- "$tmp" "${buf:-}"' EXIT + buf="$(mktemp -- "$dir/wg-edit.$PPID.XXXXXX")" || exit 1 if [ -n "$seed" ]; then printf '%s\n' "$seed" > "$buf" else printf '%s\n' "$src" > "$buf"; fi zenity --text-info --editable --filename="$buf" \ @@ -809,6 +968,7 @@ case "${1:-}" in export) cmd_export "$2" ;; export-file) cmd_export_file "$2" "$3" ;; qr-png) cmd_qr_png "$2" ;; + cleanup-runtime|cleanup-qr) cmd_cleanup_runtime ;; edit) cmd_edit "$2" "$3" ;; - *) die "Usage: backend.sh status|details|connect|down|down-all|delete|rename|import|export|export-file|qr-png|edit ..." ;; + *) die "Usage: backend.sh status|details|connect|down|down-all|delete|rename|import|export|export-file|qr-png|cleanup-runtime|edit ..." ;; esac diff --git a/manifest.json b/manifest.json index 1c1575d..ae01a47 100644 --- a/manifest.json +++ b/manifest.json @@ -2,7 +2,7 @@ "schemaVersion": 1, "id": "glafeara.wireguard", "name": "Omawire", - "version": "2.5.0", + "version": "2.5.1", "author": "glafeara", "license": "MIT", "description": "Tunnel status, transactional switching, import, rename, QR export, live traffic and drop notifications in the Omarchy bar. Unofficial third-party widget for WireGuard tunnels; not affiliated with the WireGuard project.", @@ -17,6 +17,7 @@ "description": "Toggle, switch and rename NetworkManager WireGuard tunnels, import .conf files, show QR codes, watch traffic, get a toast when a tunnel drops. No sudo required.", "category": "Network", "allowMultiple": false, + "defaultSection": "right", "defaults": { "refreshIntervalSec": 10, "pingHost": "1.1.1.1" diff --git a/tests/checklist.md b/tests/checklist.md index da21c0f..0eca2bf 100644 --- a/tests/checklist.md +++ b/tests/checklist.md @@ -81,6 +81,13 @@ unreachable, which is why the failure paths live in `test-connect.sh`. - [ ] Scanning with the phone's WireGuard app imports a working tunnel. - [ ] `omarchy-shell glafeara.wireguard exportConfig ` writes a 0600 file that re-imports losslessly. +- [ ] Close a QR window, then reload the shell while another QR is visible; + the known `$XDG_RUNTIME_DIR/wg-qr.*.png` disappears in both cases. With + two monitors, closing or reloading one instance never deletes the + other instance's visible QR. Kill the shell while a code is visible, + then start it again: the new shell removes the dead shell's QR, while + a code owned by a live sibling shell remains. (A reused PID is the + residual, extremely narrow limitation.) ## Notifications @@ -113,3 +120,9 @@ unreachable, which is why the failure paths live in `test-connect.sh`. it; `qr` while it is up does the same. - [ ] The import prompt (`i` / `v`) still lives inside the panel and gets the focus back on cancel. +- [ ] Start a slow import or switch, then call every IPC action that uses the + control worker (`down`, `importConfig`, `rename`, `importPick`, + `importPaste`): each replies `error: …`. An editor and an export may + start independently during that operation; a second editor or export + rejects its already-running worker. While an editor save is queued, + another `edit` reports the pending-save error and cannot overwrite it. diff --git a/tests/fake/id b/tests/fake/id new file mode 100755 index 0000000..7e9f3d5 --- /dev/null +++ b/tests/fake/id @@ -0,0 +1,5 @@ +#!/bin/bash +# An unallocated uid keeps the unset-XDG_RUNTIME_DIR regression isolated: the +# backend must refuse rather than create /tmp state when /run/user/ is +# unavailable. +if [ "${1:-}" = "-u" ]; then printf '%s\n' 424242; else /usr/bin/id "$@"; fi diff --git a/tests/fake/mktemp b/tests/fake/mktemp new file mode 100755 index 0000000..24c09d2 --- /dev/null +++ b/tests/fake/mktemp @@ -0,0 +1,13 @@ +#!/bin/bash +set -u +# Only intercept editor allocations in the dedicated regression. Test setup +# and every other mktemp call continue to use the host implementation. +if [ -n "${FAKE_DIR:-}" ] && [ -e "$FAKE_DIR/fail-second-mktemp" ] && [[ "${*: -1}" == *wg-edit.* ]]; then + count_file="$FAKE_DIR/mktemp-editor-count" + count=0 + [ -f "$count_file" ] && count="$(cat "$count_file")" + count=$((count + 1)) + printf '%s\n' "$count" > "$count_file" + [ "$count" -gt 1 ] && exit 1 +fi +exec /usr/bin/mktemp "$@" diff --git a/tests/fake/nmcli b/tests/fake/nmcli index 3348266..69fa830 100755 --- a/tests/fake/nmcli +++ b/tests/fake/nmcli @@ -5,6 +5,10 @@ # fail-up. if present, `connection up ` fails # fail-down. if present, `connection down ` fails # props. terse `connection show ` output (details) +# names one connection.id per line (rename duplicate check) +# fail-list-names force the rename list query to fail +# partial-export. output followed by a failing export query +# fail-delete. force deletion to fail # log every invocation, one line set -u echo "nmcli $*" >> "$FAKE_DIR/log" @@ -19,6 +23,32 @@ case "$*" in exit 0 ;; esac +# The export query intentionally has a separate fixture: it is secret-bearing +# and its failure must never yield a partial config as a successful export. +if [ "$1" = "-s" ] && [ "$2" = "-t" ] && [ "$3" = "connection" ] && [ "$4" = "show" ]; then + u="$5" + if [ -f "$FAKE_DIR/partial-export.$u" ]; then + cat "$FAKE_DIR/partial-export.$u" + exit 17 + fi + [ -f "$FAKE_DIR/export.$u" ] || exit 10 + cat "$FAKE_DIR/export.$u" + exit 0 +fi + +if [ "$1" = "-s" ] && [ "$2" = "-g" ] && [ "$4" = "connection" ] && [ "$5" = "show" ]; then + prop="$3"; u="$6" + [ -f "$FAKE_DIR/props.$u" ] || exit 10 + sed -n "s/^$prop://p" "$FAKE_DIR/props.$u" | head -1 + exit 0 +fi + +if [ "$1" = "--escape" ] && [ "$2" = "no" ] && [ "$3" = "-t" ] && [ "$4" = "-f" ] && [ "$5" = "NAME" ]; then + [ -e "$FAKE_DIR/fail-list-names" ] && exit 17 + cat "$FAKE_DIR/names" 2>/dev/null + exit 0 +fi + # -t connection show — the property dump the details command reads. if [ "$*" = "-t connection show ${4:-}" ]; then [ -f "$FAKE_DIR/props.$4" ] || { echo "Error: $4 - no such connection profile." >&2; exit 10; } @@ -26,23 +56,76 @@ if [ "$*" = "-t connection show ${4:-}" ]; then exit 0 fi -if [ "$1" = "--escape" ]; then # --escape no -g connection.interface-name connection show - u="${7}" +if [ "$1" = "--escape" ]; then # --escape no -g property connection show + prop="$4"; u="${7}" + if [ "$prop" = "connection.id" ]; then + [ -e "$FAKE_DIR/fail-show.$u" ] && exit 17 + if [ -e "$FAKE_DIR/fail-show-after.$u" ]; then + count_file="$FAKE_DIR/show-count.$u" + count=0; [ -f "$count_file" ] && count="$(cat "$count_file")" + count=$((count + 1)); printf '%s\n' "$count" > "$count_file" + [ "$count" -gt 1 ] && exit 17 + fi + if [ -f "$FAKE_DIR/id.$u" ]; then cat "$FAKE_DIR/id.$u" + elif [ -f "$FAKE_DIR/props.$u" ]; then printf '%s\n' "$u" + else exit 10 + fi + exit 0 + fi [ -f "$FAKE_DIR/ifname.$u" ] || exit 10 cat "$FAKE_DIR/ifname.$u" exit 0 fi if [ "$1" = "connection" ]; then - u="$3" case "$2" in + add) + [ -e "$FAKE_DIR/fail-add" ] && exit 4 + u="" + for ((i=1; i<=$#; i++)); do + [ "${!i}" = "connection.uuid" ] || continue + j=$((i+1)); u="${!j}"; break + done + [ -n "$u" ] || exit 99 + : > "$FAKE_DIR/props.$u" + # Deliberately no UUID in stdout: the backend must not parse this text. + [ -e "$FAKE_DIR/slow-add" ] && sleep 5 + echo "Connection successfully added" + exit 0 ;; + edit) + u="$3" + feed="$(cat)" + pk="$(printf '%s\n' "$feed" | sed -n 's/^set wireguard.private-key //p' | head -1)" + peers="$(printf '%s\n' "$feed" | sed -n 's/^set wireguard.peers //p' | head -1)" + { printf 'wireguard.private-key:%s\n' "$pk"; [ -n "$peers" ] && printf 'wireguard.peers:%s\n' "$peers"; } > "$FAKE_DIR/props.$u" + # nmcli edit may return success even when a `set` was rejected; model + # that behaviour so the backend's post-save verification is exercised. + [ -e "$FAKE_DIR/fail-edit" ] && { : > "$FAKE_DIR/props.$u"; exit 0; } + exit 0 ;; + modify) + u="$3" + [ -e "$FAKE_DIR/fail-modify.$u" ] && exit 4 + if [ "${4:-}" = "connection.id" ]; then printf '%s\n' "$5" > "$FAKE_DIR/id.$u"; fi + exit 0 ;; + delete) + u="$3" + if [ -e "$FAKE_DIR/fail-delete.$u" ] || [ -e "$FAKE_DIR/fail-delete-any" ]; then + echo "Error: deletion of $u failed" >&2 + exit 4 + fi + rm -f "$FAKE_DIR/props.$u" "$FAKE_DIR/id.$u" + exit 0 ;; up) - if [ -e "$FAKE_DIR/fail-up.$u" ]; then echo "Error: activation of $u failed" >&2; exit 4; fi + u="$3" + if [ -e "$FAKE_DIR/fail-up.$u" ] || [ -e "$FAKE_DIR/fail-up-any" ]; then echo "Error: activation of $u failed" >&2; exit 4; fi + [ -e "$FAKE_DIR/slow-up-any" ] && sleep 5 grep -qxF "$u" "$FAKE_DIR/active" || echo "$u" >> "$FAKE_DIR/active" exit 0 ;; down) + u="$3" if [ -e "$FAKE_DIR/fail-down.$u" ]; then echo "Error: deactivation of $u failed" >&2; exit 4; fi grep -vxF "$u" "$FAKE_DIR/active" > "$FAKE_DIR/active.tmp"; mv "$FAKE_DIR/active.tmp" "$FAKE_DIR/active" + [ -e "$FAKE_DIR/slow-down-any" ] && sleep 5 exit 0 ;; esac fi diff --git a/tests/fake/qrencode b/tests/fake/qrencode new file mode 100755 index 0000000..3e7e6de --- /dev/null +++ b/tests/fake/qrencode @@ -0,0 +1,12 @@ +#!/bin/bash +set -u +out="" +while [ "$#" -gt 0 ]; do + if [ "$1" = "-o" ]; then out="$2"; shift 2; continue; fi + shift +done +[ -n "$out" ] || exit 2 +[ -n "${FAKE_DIR:-}" ] && [ -e "$FAKE_DIR/fail-qr" ] && exit 4 +[ -n "${FAKE_DIR:-}" ] && [ -e "$FAKE_DIR/slow-qr" ] && sleep 5 +cat >/dev/null +printf 'PNG' > "$out" diff --git a/tests/fake/wg b/tests/fake/wg new file mode 100755 index 0000000..951a519 --- /dev/null +++ b/tests/fake/wg @@ -0,0 +1,5 @@ +#!/bin/bash +# Imports only use `wg pubkey` as a syntactic key check. The fake accepts the +# fixture keys so backend transaction tests never depend on host packages. +cat >/dev/null +printf '%s\n' 'fake-public-key=' diff --git a/tests/fake/zenity b/tests/fake/zenity new file mode 100755 index 0000000..1266d6b --- /dev/null +++ b/tests/fake/zenity @@ -0,0 +1,8 @@ +#!/bin/bash +set -u +for arg in "$@"; do + case "$arg" in --filename=*) printf '%s\n' "${arg#--filename=}" > "$FAKE_DIR/edit-buffer-path" ;; esac +done +[ -e "$FAKE_DIR/slow-zenity" ] && sleep 5 +# An unchanged, valid config makes backend.sh take its no-change path. +printf '%s\n' '[Interface]' 'PrivateKey = private' diff --git a/tests/run.sh b/tests/run.sh new file mode 100755 index 0000000..3fc0b6a --- /dev/null +++ b/tests/run.sh @@ -0,0 +1,8 @@ +#!/bin/bash +set -u +here="$(cd "$(dirname "$0")" && pwd)" +rc=0 +for suite in test-status.sh test-connect.sh test-notify.sh test-details.sh test-safety.sh test-qml-contracts.sh; do + bash "$here/$suite" || rc=1 +done +exit "$rc" diff --git a/tests/test-qml-contracts.sh b/tests/test-qml-contracts.sh new file mode 100755 index 0000000..181abed --- /dev/null +++ b/tests/test-qml-contracts.sh @@ -0,0 +1,51 @@ +#!/bin/bash +# Static regressions for Service control-flow contracts. Quickshell's typed +# IpcHandler prevents standalone Panel.qml linting, so these pin the two +# failure paths that must not be changed into silent UI actions. +set -u +here="$(cd "$(dirname "$0")" && pwd)" +service="$here/../Service.qml" +pass=0 fail=0 + +expect() { + local name="$1" condition="$2" + if eval "$condition"; then echo "PASS $name"; pass=$((pass + 1)) + else echo "FAIL $name"; fail=$((fail + 1)); fi +} + +unknown_block="$(awk '/if \(importStateUnknown\)/,/} else if \(op === "import"/' "$service")" +expect "exit 6 clears retry state and returns UI control without auto-retry" \ + 'printf "%s\n" "$unknown_block" | grep -q "_editRetryUuid = \"\"" \ + && printf "%s\n" "$unknown_block" | grep -q "root.editFailed(reason)" \ + && ! printf "%s\n" "$unknown_block" | grep -q "retryEdit"' + +saved_block="$(awk '/if \(exitCode === 0 \|\| savedNotUp\)/,/} else {/' "$service")" +expect "exit 5 clears editor retry state without auto-retry" \ + 'printf "%s\n" "$saved_block" | grep -q "_editRetryUuid = \"\"" \ + && ! printf "%s\n" "$saved_block" | grep -q "retryEdit"' + +rename_block="$(awk '/if \(value === profile.name\)/,/return true/' "$service")" +expect "idempotent rename clears stale action errors" \ + 'printf "%s\n" "$rename_block" | grep -q "actionRejection = \"\"" \ + && printf "%s\n" "$rename_block" | grep -q "lastError = \"\"" \ + && printf "%s\n" "$rename_block" | grep -q "actionStatus = \"\""' + +refresh_block="$(awk '/function refresh\(\)/,/^ }/' "$service")" +change_refresh_block="$(awk '/function refreshAfterChange\(\)/,/^ }/' "$service")" +status_block="$(awk '/id: statusProcess/,/^ }/' "$service")" +expect "only post-control refresh schedules one follow-up poll" \ + 'grep -q "property bool _refreshAfterStatus: false" "$service" \ + && ! printf "%s\n" "$refresh_block" | grep -q "_refreshAfterStatus = true" \ + && printf "%s\n" "$change_refresh_block" | grep -q "_refreshAfterStatus = true" \ + && printf "%s\n" "$status_block" | grep -q "_refreshAfterStatus = false" \ + && printf "%s\n" "$status_block" | grep -q "Qt.callLater(root.refreshAfterChange)"' + +commit_block="$(awk '/commit_import\(\)/,/^ }/' "$here/../backend.sh")" +expect "import commit keeps a terminal signal handler instead of a trap gap" \ + 'printf "%s\n" "$commit_block" | grep -q "import_committed=1" \ + && printf "%s\n" "$commit_block" | grep -q "trap - EXIT" \ + && ! printf "%s\n" "$commit_block" | grep -q "trap - EXIT HUP INT TERM"' + +echo "----" +echo "$pass passed, $fail failed" +[ "$fail" = 0 ] diff --git a/tests/test-safety.sh b/tests/test-safety.sh new file mode 100755 index 0000000..16cb231 --- /dev/null +++ b/tests/test-safety.sh @@ -0,0 +1,215 @@ +#!/bin/bash +# Regression tests for secret-bearing export, runtime-state safety, import +# cleanup/rollback and QR cleanup. All NetworkManager calls hit fake/nmcli. +set -u +here="$(cd "$(dirname "$0")" && pwd)" +backend="$here/../backend.sh" +export PATH="$here/fake:$PATH" +pass=0 fail=0 + +fresh() { + export FAKE_DIR="$(mktemp -d)" + export XDG_RUNTIME_DIR="$FAKE_DIR/runtime" + mkdir "$XDG_RUNTIME_DIR" + chmod 700 "$XDG_RUNTIME_DIR" + : > "$FAKE_DIR/active"; : > "$FAKE_DIR/log" +} + +done_case() { + local name="$1" check="$2" + if eval "$check"; then echo "PASS $name"; pass=$((pass + 1)) + else echo "FAIL $name"; fail=$((fail + 1)); [ -f "$FAKE_DIR/log" ] && sed 's/^/ /' "$FAKE_DIR/log"; fi + rm -rf "$FAKE_DIR" +} + +config() { + printf '%s\n' '[Interface]' 'PrivateKey = test-private-key' 'Address = 10.0.0.2/32' '' '[Peer]' 'PublicKey = test-peer-key' 'AllowedIPs = 0.0.0.0/0' +} + +fresh +printf '%s\n' 'wireguard.private-key:private' > "$FAKE_DIR/partial-export.U1" +printf 'unchanged' > "$FAKE_DIR/dest.conf" +bash "$backend" export U1 >/dev/null 2>&1; export_rc=$? +bash "$backend" export-file U1 "$FAKE_DIR/dest.conf" >/dev/null 2>&1; file_rc=$? +done_case "partial nmcli export fails and never overwrites destination" \ + '[ "$export_rc" != 0 ] && [ "$file_rc" != 0 ] && [ "$(cat "$FAKE_DIR/dest.conf")" = unchanged ]' + +fresh +printf '%s\n' 'wireguard.private-key:private' 'wireguard.peers:peer= allowed-ips=0.0.0.0/0' > "$FAKE_DIR/export.U1" +bash "$backend" export-file U1 "$FAKE_DIR/dest.conf" >/dev/null 2>&1; good_export_rc=$? +dest_mode="$(stat -Lc '%a' "$FAKE_DIR/dest.conf")" +done_case "successful export-file is complete and mode 0600" \ + '[ "$good_export_rc" = 0 ] && [ "$dest_mode" = 600 ] && grep -q "PrivateKey = private" "$FAKE_DIR/dest.conf" && grep -q "\[Peer\]" "$FAKE_DIR/dest.conf"' + +fresh +mkdir "$FAKE_DIR/unsafe"; chmod 777 "$FAKE_DIR/unsafe" +XDG_RUNTIME_DIR="$FAKE_DIR/unsafe" bash "$backend" down-all >/dev/null 2>&1; unsafe_rc=$? +mkdir "$FAKE_DIR/public755" "$FAKE_DIR/public750"; chmod 755 "$FAKE_DIR/public755"; chmod 750 "$FAKE_DIR/public750" +XDG_RUNTIME_DIR="$FAKE_DIR/public755" bash "$backend" down-all >/dev/null 2>&1; public755_rc=$? +XDG_RUNTIME_DIR="$FAKE_DIR/public750" bash "$backend" down-all >/dev/null 2>&1; public750_rc=$? +ln -s "$XDG_RUNTIME_DIR" "$FAKE_DIR/link" +XDG_RUNTIME_DIR="$FAKE_DIR/link" bash "$backend" down-all >/dev/null 2>&1; link_rc=$? +env -u XDG_RUNTIME_DIR bash "$backend" down-all >/dev/null 2>&1; unset_rc=$? +done_case "unset, non-private, unsafe and symlink runtime directories are refused" '[ "$unsafe_rc" != 0 ] && [ "$public755_rc" != 0 ] && [ "$public750_rc" != 0 ] && [ "$link_rc" != 0 ] && [ "$unset_rc" != 0 ]' + +fresh +: > "$FAKE_DIR/fail-list-names" +bash "$backend" rename U1 renamed >/dev/null 2>&1; rename_rc=$? +done_case "rename fails closed when name listing fails" '[ "$rename_rc" != 0 ] && ! grep -q "connection modify" "$FAKE_DIR/log"' + +fresh +config | bash "$backend" import wg0 >/dev/null 2>&1; import_rc=$? +new_count="$(find "$FAKE_DIR" -maxdepth 1 -name 'props.*' | wc -l)" +done_case "import succeeds without parsing nmcli human output" '[ "$import_rc" = 0 ] && [ "$new_count" = 1 ]' + +fresh +: > "$FAKE_DIR/fail-edit" +config | bash "$backend" import wg0 >/dev/null 2>&1; failed_import_rc=$? +new_count="$(find "$FAKE_DIR" -maxdepth 1 -name 'props.*' | wc -l)" +done_case "failure after connection add cleans temporary profile" '[ "$failed_import_rc" != 0 ] && [ "$new_count" = 0 ]' + +fresh +: > "$FAKE_DIR/fail-edit"; : > "$FAKE_DIR/fail-delete-any" +config | bash "$backend" import wg0 >/dev/null 2>"$FAKE_DIR/stderr"; failed_cleanup_rc=$? +new_count="$(find "$FAKE_DIR" -maxdepth 1 -name 'props.*' | wc -l)" +done_case "failed temporary-profile cleanup warns with replacement UUID" \ + '[ "$failed_cleanup_rc" = 6 ] && [ "$new_count" = 1 ] && grep -q "Could not remove incomplete replacement" "$FAKE_DIR/stderr"' + +fresh +: > "$FAKE_DIR/slow-add" +config | bash "$backend" import wg0 >/dev/null 2>&1 & import_pid=$! +sleep 0.2; kill -TERM "$import_pid"; wait "$import_pid"; interrupted_import_rc=$? +new_count="$(find "$FAKE_DIR" -maxdepth 1 -name 'props.*' | wc -l)" +done_case "interrupted import cleans the temporary profile" '[ "$interrupted_import_rc" != 0 ] && [ "$new_count" = 0 ]' + +fresh +echo OLD > "$FAKE_DIR/active"; echo wg0 > "$FAKE_DIR/ifname.OLD"; echo old > "$FAKE_DIR/id.OLD"; : > "$FAKE_DIR/props.OLD" +: > "$FAKE_DIR/slow-down-any" +config | bash "$backend" import wg0 OLD >/dev/null 2>&1 & down_pid=$! +for _ in $(seq 1 20); do + grep -q '^nmcli connection down OLD' "$FAKE_DIR/log" && break + sleep 0.05 +done +kill -TERM "$down_pid"; wait "$down_pid"; interrupted_down_rc=$? +new_count="$(find "$FAKE_DIR" -maxdepth 1 -name 'props.*' | wc -l)" +done_case "interrupt after old down restores old and cleans uncommitted replacement" \ + '[ "$interrupted_down_rc" != 0 ] && [ "$new_count" = 1 ] && [ -e "$FAKE_DIR/props.OLD" ] && grep -qx OLD "$FAKE_DIR/active"' + +fresh +echo OLD > "$FAKE_DIR/active"; echo wg0 > "$FAKE_DIR/ifname.OLD"; echo old > "$FAKE_DIR/id.OLD"; : > "$FAKE_DIR/props.OLD" +: > "$FAKE_DIR/slow-down-any"; : > "$FAKE_DIR/fail-up.OLD" +config | bash "$backend" import wg0 OLD >/dev/null 2>"$FAKE_DIR/stderr" & failed_down_pid=$! +for _ in $(seq 1 20); do + grep -q '^nmcli connection down OLD' "$FAKE_DIR/log" && break + sleep 0.05 +done +kill -TERM "$failed_down_pid"; wait "$failed_down_pid"; failed_down_rc=$? +new_count="$(find "$FAKE_DIR" -maxdepth 1 -name 'props.*' | wc -l)" +done_case "interrupt after old down with failed restore keeps both profiles" \ + '[ "$failed_down_rc" = 6 ] && [ "$new_count" = 2 ] && grep -qi "state unknown" "$FAKE_DIR/stderr"' + +fresh +echo OLD > "$FAKE_DIR/active"; echo wg0 > "$FAKE_DIR/ifname.OLD"; echo old > "$FAKE_DIR/id.OLD"; : > "$FAKE_DIR/props.OLD" +: > "$FAKE_DIR/slow-down-any"; : > "$FAKE_DIR/fail-show-after.OLD" +config | bash "$backend" import wg0 OLD >/dev/null 2>"$FAKE_DIR/stderr" & failed_query_pid=$! +for _ in $(seq 1 20); do + grep -q '^nmcli connection down OLD' "$FAKE_DIR/log" && break + sleep 0.05 +done +kill -TERM "$failed_query_pid"; wait "$failed_query_pid"; failed_query_rc=$? +new_count="$(find "$FAKE_DIR" -maxdepth 1 -name 'props.*' | wc -l)" +done_case "cleanup old-query failure is terminal and keeps replacement" \ + '[ "$failed_query_rc" = 6 ] && [ "$new_count" = 2 ] && grep -q "Could not determine whether old profile" "$FAKE_DIR/stderr"' + +fresh +echo OLD > "$FAKE_DIR/active"; echo wg0 > "$FAKE_DIR/ifname.OLD"; echo old > "$FAKE_DIR/id.OLD"; : > "$FAKE_DIR/props.OLD" +: > "$FAKE_DIR/slow-up-any" +config | bash "$backend" import wg0 OLD >/dev/null 2>&1 & replacement_pid=$! +for _ in $(seq 1 20); do + grep -q '^nmcli connection up ' "$FAKE_DIR/log" && break + sleep 0.05 +done +kill -TERM "$replacement_pid"; wait "$replacement_pid"; interrupted_replacement_rc=$? +new_count="$(find "$FAKE_DIR" -maxdepth 1 -name 'props.*' | wc -l)" +done_case "interrupt after old delete keeps committed replacement" \ + '[ "$interrupted_replacement_rc" = 5 ] && [ "$new_count" = 1 ] && ! [ -e "$FAKE_DIR/props.OLD" ]' + +fresh +echo OLD > "$FAKE_DIR/active"; echo wg0 > "$FAKE_DIR/ifname.OLD"; echo old > "$FAKE_DIR/id.OLD"; : > "$FAKE_DIR/props.OLD" +: > "$FAKE_DIR/fail-up-any" +config | bash "$backend" import wg0 OLD >/dev/null 2>"$FAKE_DIR/stderr"; saved_rc=$? +new_count="$(find "$FAKE_DIR" -maxdepth 1 -name 'props.*' | wc -l)" +done_case "saved-but-not-up keeps replacement and exits 5" '[ "$saved_rc" = 5 ] && [ "$new_count" = 1 ] && ! [ -e "$FAKE_DIR/props.OLD" ]' + +fresh +echo OLD > "$FAKE_DIR/active"; echo wg0 > "$FAKE_DIR/ifname.OLD"; echo old > "$FAKE_DIR/id.OLD"; : > "$FAKE_DIR/props.OLD" +: > "$FAKE_DIR/fail-delete.OLD"; : > "$FAKE_DIR/fail-up.OLD" +config | bash "$backend" import wg0 OLD >/dev/null 2>"$FAKE_DIR/stderr"; rollback_rc=$? +new_count="$(find "$FAKE_DIR" -maxdepth 1 -name 'props.*' | wc -l)" +done_case "failed replacement rollback reports unknown state and keeps both profiles" \ + '[ "$rollback_rc" = 6 ] && grep -qi "state is unknown" "$FAKE_DIR/stderr" && [ "$new_count" = 2 ] && [ -e "$FAKE_DIR/props.OLD" ]' + +fresh +: > "$FAKE_DIR/props.U1" +bash "$backend" delete U1 >/dev/null 2>&1; delete_rc=$? +done_case "delete uses the UUID target through NetworkManager" \ + '[ "$delete_rc" = 0 ] && grep -q "connection delete U1" "$FAKE_DIR/log" && ! [ -e "$FAKE_DIR/props.U1" ]' + +fresh +printf '%s\n' 'wireguard.private-key:private' > "$FAKE_DIR/export.U1" +: > "$FAKE_DIR/fail-qr" +bash "$backend" qr-png U1 >/dev/null 2>&1; qr_rc=$? +png_count="$(find "$XDG_RUNTIME_DIR" -name 'wg-qr.*.png' | wc -l)" +done_case "failed QR encoding removes private-key PNG" '[ "$qr_rc" != 0 ] && [ "$png_count" = 0 ]' + +fresh +printf '%s\n' 'wireguard.private-key:private' > "$FAKE_DIR/export.U1" +: > "$FAKE_DIR/slow-qr" +bash "$backend" qr-png U1 >/dev/null 2>&1 & pid=$! +sleep 0.2; kill -TERM "$pid"; wait "$pid"; signal_rc=$? +png_count="$(find "$XDG_RUNTIME_DIR" -name 'wg-qr.*.png' | wc -l)" +done_case "interrupted QR encoding removes private-key PNG" '[ "$signal_rc" != 0 ] && [ "$png_count" = 0 ]' + +fresh +live_qr="$XDG_RUNTIME_DIR/wg-qr.$$.aaaaaa.png" +dead_qr="$XDG_RUNTIME_DIR/wg-qr.99999999.bbbbbb.png" +printf PNG > "$live_qr"; printf PNG > "$dead_qr" +bash "$backend" cleanup-runtime >/dev/null 2>&1; cleanup_rc=$? +done_case "runtime stale cleanup keeps live QR owner and removes dead owner" \ + '[ "$cleanup_rc" = 0 ] && [ -f "$live_qr" ] && ! [ -e "$dead_qr" ]' + +fresh +printf '%s\n' 'wireguard.private-key:private' > "$FAKE_DIR/export.U1" +: > "$FAKE_DIR/slow-zenity" +bash "$backend" edit U1 tunnel /dev/null 2>&1 & editor_pid=$! +for _ in $(seq 1 20); do + [ -f "$FAKE_DIR/edit-buffer-path" ] && break + sleep 0.05 +done +edit_count="$(find "$XDG_RUNTIME_DIR" -maxdepth 1 -name 'wg-edit.*' | wc -l)" +buffer_path="$(cat "$FAKE_DIR/edit-buffer-path" 2>/dev/null || true)" +buffer_mode="$(stat -Lc '%a' "$buffer_path" 2>/dev/null || true)" +kill -TERM "$editor_pid"; wait "$editor_pid"; editor_rc=$? +post_signal_edit_count="$(find "$XDG_RUNTIME_DIR" -maxdepth 1 -name 'wg-edit.*' | wc -l)" +done_case "editor secrets use private runtime files, never bare mktemp" \ + '[ "$editor_rc" != 0 ] && [ "$edit_count" = 2 ] && [ "$post_signal_edit_count" = 0 ] && [ "$buffer_mode" = 600 ] && [[ "$buffer_path" == "$XDG_RUNTIME_DIR"/* ]]' + +fresh +printf '%s\n' 'wireguard.private-key:private' > "$FAKE_DIR/export.U1" +: > "$FAKE_DIR/fail-second-mktemp" +bash "$backend" edit U1 tunnel /dev/null 2>&1; second_mktemp_rc=$? +edit_count="$(find "$XDG_RUNTIME_DIR" -maxdepth 1 -name 'wg-edit.*' | wc -l)" +done_case "failed second editor mktemp cleans the first secret file" \ + '[ "$second_mktemp_rc" != 0 ] && [ "$edit_count" = 0 ]' + +fresh +live_edit="$XDG_RUNTIME_DIR/wg-edit.$$.cccccc" +dead_edit="$XDG_RUNTIME_DIR/wg-edit.99999999.dddddd" +printf secret > "$live_edit"; printf secret > "$dead_edit" +bash "$backend" cleanup-runtime >/dev/null 2>&1; edit_cleanup_rc=$? +done_case "runtime stale cleanup keeps live editor owner and removes dead owner" \ + '[ "$edit_cleanup_rc" = 0 ] && [ -f "$live_edit" ] && ! [ -e "$dead_edit" ]' + +echo "----" +echo "$pass passed, $fail failed" +[ "$fail" = 0 ]