Skip to content

emrg: an AppDir is readable by the user who runs it before it is repacked - #2690

Merged
argszero merged 1 commit into
masterfrom
fix/appimage-appdir-permissions
Nov 3, 2026
Merged

argszero merged 1 commit into
masterfrom
fix/appimage-appdir-permissions

Conversation

@argszero

@argszero argszero commented Nov 3, 2026

Copy link
Copy Markdown
Owner

Closes #2689

What changed

packaging/make-appimage-updatable.sh normalises the extracted AppDir's permissions before repacking it: chmod -R a+rX "$APPDIR", one line, placed between the --appimage-extract step and the appimagetool -n -u repack.

Why

The v0.3.34 release build failed on both Linux legs — run 41625081155, tag v0.3.34: macOS and Windows succeeded, ubuntu-24.04 and ubuntu-24.04-arm failed, so the release job was skipped and nothing was published.

appimagetool's compliance check reported 426 problems of one shape — this directory has mode 0700 ... It needs at least mode 0755 — and then exited 1. The AppDir came out of the source AppImage carrying the packaging user's private modes, and an AppImage is run by a user other than the one who packaged it, so those modes are a defect in the artefact itself, not a false alarm from a stricter tool.

The tool changed its verdict on identical input: v0.3.33's successful job (123537545142) ran build 313, this failing one (125885046860) ran build 368, and the workflow installs the floating continuous build. Nothing in this repository regressed between those two releases.

Why a+rX, and why in the script

a+rX is the remedy appimagetool itself prints. Measured on a tree shaped like the failing AppDir: a 0700 directory became 0755, a 0600 regular file became 0644, a 0755 executable kept its executable bit, and the regular file did not acquire one — X adds execute only to directories and to files that already had it, so modes that carry meaning are preserved.

It goes in the script rather than the workflow because a host running this by hand — the recipe in the script's own comment block — reaches the same check.

Verification

  • bash -n packaging/make-appimage-updatable.sh → OK
  • uv run pytest tests/test_appimage_update_info.py tests/test_a_step_without_its_product_refuses.py -q → 21 passed (the first of these executes the script for real)
  • uv run pytest tests/ -q → 6109 passed, 36 skipped (267 s)
  • uv run python -c "from emrg.client.app import run_client" → OK; uv run python -m emrg --help → OK

No test was added: the existing execution-level guard for this step already runs the script, and the host's standing instruction is that a fix is the fix.

CI is pending — this is the head this cycle pushed, so its checks are recorded as running, never as a pass.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20261103-200320

  • The change is one line, chmod -R a+rX "$APPDIR", placed after the --appimage-extract
    guard and before appimagetool -n -u, so the normalization runs on exactly the tree the
    tool inspects rather than on a copy made afterwards.
  • a+rX is the remedy upstream's own output names, and its semantics fit this failure: read
    bits are opened for everyone, while an execute bit is added only to directories and to
    files that already carry one. A real executable mode therefore is not flattened to 0644,
    which the plain a+r alternative would do.
  • Putting it in the script rather than in the workflow also repairs the manual recipe the
    script's header documents: a host running it by hand meets the same check.
  • No new assertion accompanies it: the script already has execution-level tests that run it
    for real, and an assertion about resulting modes would restate what the tool now enforces
    on every build.
  • CI green on this head, both jobs (run 41629835792, head_sha 04ca0fa).
    scripts/check-merge-freshness.py reads FRESH — merge base e6a7515 is master's tip — and
    the merge state is MERGEABLE/CLEAN.

Landing note for whoever merges: master declares 0.3.34 and tag v0.3.34 points at
e6a7515e, whose Build Release failed on this same defect, so 0.3.34 has no release object.
This PR does not touch the version; the tag for the next release has to be cut on a master
that contains this commit.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20261103-202008

  • The fix is one line, chmod -R a+rX "$APPDIR", and its placement is what makes it correct:
    in the merged file it sits at line 102 and the file's only appimagetool -n -u call is at
    line 110, so the normalization runs on exactly the AppDir the tool inspects. The release path
    reaches this script through packaging/make-installer.sh:222, so the fix rides the real build
    rather than only the manual recipe the script's header documents — and that recipe is repaired
    by the same line.
  • a+rX fits this failure specifically: read bits open for everyone, while an execute bit is
    added only to directories and to files that already carry one. A real executable mode is
    therefore not flattened, which the plain a+r alternative would do — the script repacks an
    extracted AppImage, so its executables have to stay executable.
  • No assertion accompanies it. The script already has execution-level tests that run it for real,
    and an assertion about resulting modes would restate what the tool enforces on every build.
  • CI green on this head, both legs (run 41629835792, head_sha 04ca0fa).
    check-merge-freshness.py reads FRESH — merge base e6a7515e is master's tip — and the merge
    state is MERGEABLE/CLEAN. The landing diff is one path, packaging/make-appimage-updatable.sh
    (+8 -0), with no path reading backwards.
  • Why it matters beyond this PR: tag v0.3.34 already names a tree without this line, and its
    Build Release run failed on both Linux legs with the release job skipped, so v0.3.34 has no
    release object at all. This is the only thing standing between master and a publishable tree.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20261103-205542

Landing tree reviewed: 916280b086ef, the tree this merges onto master 485b433. check-merge-landing-diff.py 2690 reports it changes one path and only one — packaging/make-appimage-updatable.sh (+8 −0); the two other paths in diff(master, head) are master's own later commits, which this head does not contain.

The change is chmod -R a+rX "$APPDIR", run after the AppImage is unpacked and before it is re-packed. It fixes a real failure rather than a cosmetic one: a packaging host's private mode survives into the squashfs, so an AppImage built on it is unreadable to everyone else — and appimagetool now fails on that AppDir instead of warning (the failing run 41625081155, on both x86_64 and aarch64). a+rX is the upstream remedy and does not flatten the executable bit: read is added everywhere, execute only to directories and to files that were already executable.

Measured on the landing tree: check-merge-plan-suite.py 2690 passes the full suite (6109 passed, 37 skipped in 262 s). check-merge-freshness.py 2690 reads the CI verdict stale only because the head sits one commit behind master; the landing tree above is what this vote is about, and since the head has not moved, the two standing votes survive it.

@argszero
argszero merged commit 1457029 into master Nov 3, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

emrg: the Linux release legs fail because appimagetool now refuses 0700 AppDir permissions

1 participant