Repository navigation
fix(v3/updater): replace and relaunch the AppImage, not its read-only mount - #6201
AlbinoGeek wants to merge 3 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review. WalkthroughWhen the executable runs inside an AppImage mount and the ChangesAppImage target resolution
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to The updater selects the AppImage file for replacement and relaunch under the implemented AppImage conditions. No identified issue prevents merging after normal checks. Security Architecture ReviewSecurity architecture risk: 🟡 Moderate · up to The fix targets the correct installed AppImage, but also makes persistent AppImage replacement subject to an existing non-atomic swap procedure. Interruption or competing updates can undermine automatic recovery. No new elevated-privilege or remote attack path was established. Retained concerns
Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. A rabbit checks the mount at night Comment |
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The target resolution is guarded against inherited environment variables and is adequately covered by focused tests.
Review effort: Balanced
Findings: None
What changed in this PR
Fixes AppImage self-updates by targeting the writable AppImage file instead of its temporary read-only mount.
Changes:
- Resolves
$APPIMAGEonly when the executable resides under$APPDIR. - Adds AppImage and fallback-path tests.
- Updates updater documentation and changelog.
| File | Description |
|---|---|
v3/UNRELEASED_CHANGELOG.md |
Records the AppImage updater fix. |
v3/pkg/updater/spawn.go |
Resolves the correct AppImage update and relaunch target. |
v3/pkg/updater/spawn_test.go |
Tests AppImage detection and fallback cases. |
docs/mpress/content/guides/updater.md |
Documents AppImage update behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
0e9c059 to
d24db08
Compare
|
Automated v3 GA test run (taliesin-ai) for head ``, run 2026-10-04 against master baseline
* Same failure on master The app was not launched (no GUI session over SSH). The Linux-specific paths in this PR were not exercised on Linux. This run doesn't review or approve the PR. |
45d92e8 to
bf4260a
Compare
|
Can I get a human reviewer, or did I make this wrong? |
Description
Inside an AppImage,
os.Executable()is the read-only squashfs mount, so the swap and helper spawn targeted an unwritable path that disappears on exit.When the process runs from
$APPDIR, the updater now targets and relaunches$APPIMAGEinstead.$APPIMAGEis inherited by child processes, so it is only used when the executable really lives under$APPDIR.Type of change
How Has This Been Tested?
Added
spawn_test.gocovering the AppImage and non-AppImage cases.cd v3 && go test ./pkg/updater/... && go vet ./pkg/updater/...pass on this branch alone;gofmt -lon the changed files is empty.Test Configuration
Fedora Linux 44 Workstation, amd64, GNOME on Wayland. Only the
pkg/updaterunit tests were run for this change, so nowails doctoroutput applies.Checklist:
website/src/pages/changelog.mdxwith details of this PR (v3 changelog entries are added automatically)An entry is added to
v3/UNRELEASED_CHANGELOG.mdand the updater guide is updated.Overlap with sibling PRs: this is one of three independent updater PRs from the same author (EXDEV staging, AppImage self-path,
OnUpdateApplied). Each branch is based on master and passes on its own. They touch the same files in a few places (spawn.go,helper.go,updater.go, the guide, and the changelog), including the smallresolveTargethelper that two of them also add, so whichever merges second may need a trivial textual rebase.The code and this description were written with an AI assistant (Claude). I reviewed the diff and ran the tests listed above.
Summary by CodeRabbit
.AppImagefile identified by theAPPIMAGEenvironment variable, rather than attempting to replace read-only mounted contents. This applies when the updater is running from within the AppImage mount..AppImagefile, not the read-only mount, when running inside an AppImage.