fix(appimage): unblock release smoke by relaxing the userns sysctl (main copy of #5251) - #5252
Conversation
The Linux desktop job has failed every Release Production run since tinyhumansai#5189 reached release (promote tinyhumansai#5203). The AppImage itself builds, signs and boots fine; the new bounded startup smoke is what fails, deterministically, with exit 133 instead of the expected 124: FATAL:content/browser/zygote_host/zygote_host_impl_linux.cc:128] No usable sandbox! If you are running on Ubuntu 23.10+ ... Ubuntu 23.10+ sets kernel.apparmor_restrict_unprivileged_userns=1, which denies unprivileged user-namespace creation to every unconfined process on the runner, so CEF's zygote cannot build its sandbox and aborts on SIGTRAP. tinyhumansai#5189 reached for Chromium's second documented remedy, a per-executable AppArmor profile carrying `userns,`. The profile loads successfully but never takes effect, because AppArmor attaches profiles by execve path while the sharun launcher runs the app through the AppDir's bundled dynamic loader rather than exec'ing shared/bin/OpenHuman directly (Chromium then re-execs /proc/self/exe for the zygote). Apply Chromium's first documented remedy instead: relax the sysctl for the smoke window and restore its original value afterwards, including on interrupt. It is path-independent, so it cannot miss the way profile attachment does. The AppArmor profile is retained alongside it -- it is harmless and keeps the narrower grant for hosts without the sysctl. Setting OPENHUMAN_CEF_NO_SANDBOX is deliberately not the fix: it is #[cfg(debug_assertions)]-gated in app/src-tauri/src/lib.rs and therefore inert in the release AppImage, and CEF's command line is built explicitly via command_line_args() so a --no-sandbox argv flag would not reach it either. Keeping the sandbox enabled also means the smoke keeps exercising the real production configuration. The toggle is a no-op on hosts that are already permissive or that have no such sysctl key, so it never reaches for sudo outside the restricted-Ubuntu case it exists for. Refs: https://github.com/tinyhumansai/openhuman/actions/runs/30361949794
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe AppImage runtime smoke now temporarily relaxes the AppArmor user namespace sysctl restriction, records and restores its prior value, handles interruption cleanup, and adds deterministic tests for failure, TERM, permissive-host, and missing-key scenarios. ChangesAppImage user namespace restriction handling
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant AppImageSmoke
participant SysctlHelpers
participant SudoSysctl
participant Cleanup
AppImageSmoke->>SysctlHelpers: Relax user namespace restriction
SysctlHelpers->>SudoSysctl: Read and set sysctl to 0
AppImageSmoke->>AppImageSmoke: Run AppImage userns smoke
AppImageSmoke->>Cleanup: Remove profile and restore sysctl
Cleanup->>SysctlHelpers: Restore recorded value
SysctlHelpers->>SudoSysctl: Set previous sysctl value
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: def15a2273
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| sudo --non-interactive sysctl -q -w "$SMOKE_USERNS_SYSCTL=0" \ | ||
| || { runtime_validation_error "could not relax $SMOKE_USERNS_SYSCTL for the AppImage smoke"; return 1; } | ||
|
|
||
| printf '%s\n' "$current" >"$previous_file" |
There was a problem hiding this comment.
Save sysctl state before the privileged write
When the release workflow is canceled or receives SIGTERM after this sysctl ...=0 succeeds but before the next line records $previous_file, the interrupt cleanup has no saved value and restore_smoke_userns_restriction no-ops, leaving kernel.apparmor_restrict_unprivileged_userns globally relaxed on that runner. Record the original value before changing the sysctl (and delete that record only after a successful restore or failed relax) so the existing signal trap can always put the host back.
Useful? React with 👍 / 👎.
| local previous | ||
| previous="$(cat "$previous_file")" | ||
| rm -f "$previous_file" | ||
| sudo --non-interactive sysctl -q -w "$SMOKE_USERNS_SYSCTL=$previous" \ | ||
| || { runtime_validation_error "could not restore $SMOKE_USERNS_SYSCTL to $previous"; return 1; } | ||
| echo "[appimage-runtime] Restored $SMOKE_USERNS_SYSCTL to $previous" |
There was a problem hiding this comment.
The state file is removed before the
sudo sysctl call. If sudo fails transiently, the previous value is unrecoverable — the runner stays with userns=0 and there is nothing left to inspect or retry. Moving rm -f to after the successful restore is the safer order.
| local previous | |
| previous="$(cat "$previous_file")" | |
| rm -f "$previous_file" | |
| sudo --non-interactive sysctl -q -w "$SMOKE_USERNS_SYSCTL=$previous" \ | |
| || { runtime_validation_error "could not restore $SMOKE_USERNS_SYSCTL to $previous"; return 1; } | |
| echo "[appimage-runtime] Restored $SMOKE_USERNS_SYSCTL to $previous" | |
| local previous | |
| previous="$(cat "$previous_file")" | |
| sudo --non-interactive sysctl -q -w "$SMOKE_USERNS_SYSCTL=$previous" \ | |
| || { runtime_validation_error "could not restore $SMOKE_USERNS_SYSCTL to $previous"; return 1; } | |
| rm -f "$previous_file" | |
| echo "[appimage-runtime] Restored $SMOKE_USERNS_SYSCTL to $previous" |
|
| Filename | Overview |
|---|---|
| scripts/release/validate-appimage-runtime.sh | Adds relax/restore functions for kernel.apparmor_restrict_unprivileged_userns, wiring them into smoke_extracted_apprun_with_userns before/after the AppArmor profile install and on all interrupt paths; state file is deleted before the restore sudo call (minor ordering concern). |
| scripts/release/test-strip-appimage-rpaths.sh | Adds a fake sysctl binary to the restricted-host fixture and extends expected command-order assertions for both the failing-smoke and TERM-interrupt paths; adds a dedicated no-op loop covering the permissive-host (value=0) and absent-key (sysctl exits 1) scenarios. |
Sequence Diagram
sequenceDiagram
participant W as smoke_extracted_apprun_with_userns
participant R as relax_smoke_userns_restriction
participant A as install_smoke_userns_profile
participant S as smoke_extracted_apprun
participant RA as remove_smoke_userns_profile
participant RS as restore_smoke_userns_restriction
W->>W: set HUP/INT/TERM traps
W->>W: rm -f userns_sysctl_file
W->>R: relax(userns_sysctl_file)
R->>R: sysctl -n key (read current)
alt "current == 0 or absent"
R-->>W: no-op, return 0
else "current == 1"
R->>R: "sudo sysctl -q -w key=0"
R->>R: write current to state file
R-->>W: return 0
end
W->>A: install AppArmor profile
alt AppArmor fails
A-->>W: return 1
W->>RS: restore(userns_sysctl_file)
W->>W: restore signal traps, return 1
end
W->>S: smoke (timeout 15s)
S-->>W: smoke_status
W->>RA: remove AppArmor profile
W->>RS: restore(userns_sysctl_file)
RS->>RS: "sudo sysctl -q -w key=previous"
RS->>RS: rm -f state file
W->>W: restore signal traps
W-->>W: return smoke_status (or remove/restore status)
Reviews (1): Last reviewed commit: "fix(appimage): unblock release smoke by ..." | Re-trigger Greptile
Summary
x86_64AppImage startup smoke added by fix(appimage): anchor sharun library paths to AppDir #5189 has never passed and fails deterministically with exit 133 instead of the expected 124.kernel.apparmor_restrict_unprivileged_userns=1, which denies unprivileged user namespaces to unconfined processes, so CEF's zygote aborts withNo usable sandbox!.shared/bin/OpenHumandirectly.validate-appimage-runtime.shand its fixture tests.Problem
The
Strip host graphics libs and validate final AppImagestep fails onubuntu(x86_64). The AppImage itself is fine — it builds, signs, bundleslibcef.so, and boots far enough to log[cef-startup]. What fails is the new bounded startup smoke:smoke_extracted_appruntreatstimeout's status124as success. CEF dies onSIGTRAPabout one second in, so the smoke sees133and fails the release.This is a regression from #5189, and the smoke has never passed
e3a7f52172e5b5e7b2— Promote main → release (#5203), which carried #5189a40fbb79d9c1e2e657The failure is byte-identical across all three. #5189 merged into
main, where the production AppImage build does not run, so this was never exercised before promotion.ubuntu-arm64is unaffected only becausebuild-desktop.ymlenables the smoke forx86_64alone.Why #5189's AppArmor profile does not work
install_smoke_userns_profileimplements Chromium's per-executable-profile remedy against$appdir/shared/bin/OpenHuman. The profile loads — the log confirmsLoaded temporary AppArmor userns profile for: .../shared/bin/OpenHuman— yetunshareis still denied. AppArmor attaches profiles by the execve'd path, and the sharun launcher runs the app through the AppDir's bundled dynamic loader rather than exec'ingshared/bin/OpenHuman; Chromium then re-execs/proc/self/exefor the zygote. The confinement target never matches, so theuserns,grant is never in force.Solution
In
scripts/release/validate-appimage-runtime.sh:relax_smoke_userns_restriction/restore_smoke_userns_restriction, which setkernel.apparmor_restrict_unprivileged_userns=0for the smoke window and restore the host's original value afterwards — including on the existingHUP/INT/TERMinterrupt path and when AppArmor setup itself fails. The runner is left exactly as it was found.0or absent on the host, so it never reaches forsudooutside the restricted-Ubuntu case it exists for.smoke_extracted_apprun_with_usernsis unchanged; the saved-state file is derived as$profile_file.sysctl. No change tobuild-desktop.ymlis required.Resulting privileged-command sequence, pinned by fixtures:
Rejected alternative:
OPENHUMAN_CEF_NO_SANDBOX/--no-sandboxThe smoke currently unsets
OPENHUMAN_CEF_NO_SANDBOX, so setting it looks like the obvious fix. It is not:#[cfg(debug_assertions)]-gated inapp/src-tauri/src/lib.rs(forcedis hard-codedfalsein release builds, deliberately, per the security review on feat(skills): scheduled dashboard + run/new pages + [github] preflight gate + composio-only GitHub I/O #2875). It is inert in the release AppImage.--no-sandboxargv flag would not reach CEF either: the CEF command line is built explicitly viacommand_line_args()rather than forwarded from process argv.Keeping the sandbox enabled is also the better outcome — the smoke goes on exercising the real production configuration rather than a weakened one.
Impact
APPIMAGE_RUNTIME_SMOKE: '0'forx86_64in.github/workflows/build-desktop.ymldisables the gate in one line.Verification
scripts/release/test-strip-appimage-rpaths.shis Linux-only and skips on macOS (SKIP: patchelf not installed), so the new logic was additionally driven directly against the same command fakes:1, restore1, state file removed;0) → nosudo, no state file;sudo, no state file;37, and no leaked state.The observed sequence matches the fixture expectations exactly. This cannot be validated end-to-end by normal PR CI — see the note below.
Related
releasevia promote Promote main → release #5203)release: fix(appimage): unblock release smoke by relaxing the userns sysctl #5251maincopy; both carry commit0be92171b).install_smoke_userns_profileshould be dropped entirely once a green run confirms the sysctl is the effective remedy; if kept, retarget it at the sharun execve path so the grant actually attaches.scripts/release/**, so a smoke gate cannot merge unexercised again.Verification by a maintainer is required
Regular PR CI does not run the production Linux AppImage build, so neither this PR's checks nor #5251's can prove the fix. A maintainer must re-dispatch the
Release Productionworkflow (againstrelease, via #5251) to confirm theDesktop: ubuntujob now reachesstatus 124and passes. ThismainPR needs no separate dispatch — it is the identical commit.Submission Checklist
test-strip-appimage-rpaths.sh: updated both expected privileged-command-order fixtures (failing-smoke andTERM-interrupt paths), added no-op coverage for permissive-host and absent-key, and assertions that the saved-state file never leaks.scripts/release/**, covered by thetest-strip-appimage-rpaths.shfixture suite.diff-coverover Vitest + cargo-llvm-cov does not instrument shell; no TS/Rust lines changed.N/A: CI/release tooling change, no product feature row affected.## Related—N/A: no feature IDs affected.sysctl/sudoare already required by this script.N/A: restores the intended behaviour of an existing automated release gate; the manual checklist is unchanged.Closes #NNN—N/A: no tracking issue; this PR is driven by the failed release run linked above.AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
fix/appimage-smoke-userns-main-DAPPIMG-39513def15a227(cherry-pick of0be92171bfrom thereleasePR fix(appimage): unblock release smoke by relaxing the userns sysctl #5251)Validation Run
pnpm --filter openhuman-app format:check—N/A: no files under app/ changed.pnpm typecheck—N/A: no TypeScript changed.bash -non both scripts; newrelax/restorefunctions andsmoke_extracted_apprun_with_usernsdriven against the fixture command fakes (restricted / permissive / absent-key / wrapper ordering / failure-status preservation).scripts/release/test-strip-appimage-rpaths.shskips on macOS (patchelfis Linux-only) and runs in CI.N/A: no Rust changed.N/A: no Tauri code changed.Validation Blocked
command:Release Production→Build desktop matrix / Desktop: ubuntuerror:not runnable locally — Linux-only AppImage bundling plus an Xvfb/AppArmor runtime smoke; this work was done on macOS, and the job is not triggered by regular PR CI.impact:the end-to-end fix can only be confirmed by a maintainer re-dispatchingRelease Production. Diagnosis was derived from the full failing log and the scripts; the new shell logic is verified locally against command fakes.Behavior Changes
Parity Contract
smoke_extracted_apprun_with_userns, the124-means-alive success rule, the forbidden-loader-diagnostic greps, secret scrubbing, and signal-trap restore are all unchanged.0or is absent, so non-Ubuntu and already-permissive hosts follow exactly the previous code path; restore is idempotent and runs on the success, failure, AppArmor-setup-failure, and interrupt paths.Duplicate / Superseded PR Handling
maincopy.maindoes not regress on the next promotion.Summary by CodeRabbit
Bug Fixes
Tests