Skip to content

fix(cli): handle trusted Linux system aliases during install activation - #2203

Merged
DeusData merged 5 commits into
DeusData:mainfrom
DavidHLP:fix/issue-2175
Sep 20, 2026
Merged

DeusData merged 5 commits into
DeusData:mainfrom
DavidHLP:fix/issue-2175

Conversation

@DavidHLP

@DavidHLP DavidHLP commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

Summary

Fix Linux install staging when the target path is reached through a root-owned system alias.

activation_posix_walk_path() now resolves only the explicit Linux aliases /tmp, /var, and /home when they are root-owned symlinks. Arbitrary user-owned symlinks remain rejected, and macOS keeps its existing /tmp and /var handling.

Why

On Linux layouts such as Atomic-style /home -> /var/home, and in the reproducer /tmp -> /tmp-real, the secure path walk rejected a system-managed alias before the install candidate could be staged. The command then reported:

failed to stage install candidate: activation transaction I/O failure

Validation

  • activation_transaction,cli: 338 passed in the Ubuntu 24.04 Docker test image.
  • Patched build and git diff --check: passed.
  • Root-owned /tmp alias: both missing-target and existing-target cases succeeded.
  • Root-owned /home -> /var/home: succeeded.
  • Arbitrary user-owned symlink: rejected; target was not created.

Review scope

  • The exact Fedora bootc/SELinux image was not tested.
  • The issue's reported exit code 0 was not reproduced; the controlled rejected-symlink case returned 1.
  • Exit-code handling is unchanged in this PR.

Fixes #2175

@github-actions

Copy link
Copy Markdown

Thanks for opening this — it has been seen, and it is queued.

This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence.

Current review status: working through a backlog. 0.9.1-rc.1 is out, so the release freeze that held reviews is over — but it left a large queue of open pull requests behind it, and we are reading through them oldest-first. The background is in discussion #1144.

What that means for this PR, concretely:

  • It will not be closed for inactivity. No stale bot touches pull requests here.
  • It may still sit a while before a human reads it. That is on us, not on you.
  • Older PRs are read first, so a recent one is not being skipped — it is behind a queue.

Things that will genuinely speed it up whenever review does happen:

  • Keep it rebased on main — the tree is moving quickly right now, and a conflicting branch cannot be reviewed as the diff you intended.
  • Get CI green, or say which failures you believe are pre-existing.
  • Keep the change to one claim. Bundled features and refactors get split before they get merged, which costs you a round trip.
  • Every commit needs a sign-off (git commit -s) — CI enforces DCO.

If this fixes a bug, a reproduction we can run is worth more than a description of the symptom.

Thanks for contributing, and sorry in advance for the wait.

Signed-off-by: DavidHLP <144919470+DavidHLP@users.noreply.github.com>
@DavidHLP
DavidHLP marked this pull request as ready for review September 14, 2026 01:21
@DavidHLP
DavidHLP requested a review from DeusData as a code owner September 14, 2026 01:21
@DavidHLP

Copy link
Copy Markdown
Contributor Author

Verification update

I rebuilt the patched v0.10.8 binary in the existing Ubuntu 24.04 Docker test image and reran the focused validation.

Results:

  • activation_transaction,cli: 338 passed
  • root-owned /tmp alias with a missing target parent: rc=0, binary installed
  • existing target directory: rc=0
  • Atomic-style root-owned /home -> /var/home: rc=0, binary installed
  • arbitrary user-owned symlink: rc=1, target was not created

The reported direct-command exit-code discrepancy is not reproducible with this binary: the rejected user-owned-symlink case returns 1, and the current main propagates the install handler result. I am not adding an unrelated exit-code change without a reproducer.

The PR workflow still has one underlying failing job, test / test-msan; the other PR checks are green. Its log reports a daemon-bootstrap test timeout without a MemorySanitizer report, and the contributor token cannot rerun that job because GitHub requires repository admin rights. Please rerun that job when convenient.

Signed-off-by: DavidHLP <lysf15520112973@163.com>
@DavidHLP

Copy link
Copy Markdown
Contributor Author

Follow-up for commit 581f0c7:

The alias list is now Linux-only for /home; macOS retains its previous /tmp and /var policy.

I rebuilt this new head in Docker and reran the issue matrix:

  • product build: build_rc=0
  • root-owned /tmp alias + missing target parent: rc=0, target created
  • existing target directory: rc=0, target created
  • arbitrary user-owned symlink: rc=1, target not created
  • root-owned Atomic-style /home -> /var/home: rc=0, target created
  • final matrix status: 0

The Linux fix remains minimal and the existing O_NOFOLLOW rejection boundary is preserved.

Comment thread src/cli/activation_transaction.c
Signed-off-by: DavidHLP <lysf15520112973@163.com>
@JohnGalt1717

Copy link
Copy Markdown

Please get this merged. Right now you can't install codebase-memory on Ubuntu 26.04 to name just one because of this.

@DeusData DeusData added bug Something isn't working ux/behavior Display bugs, docs, adoption UX security Security vulnerabilities, hardening priority/high Needs near-term maintainer attention; high-impact bug, regression, safety issue, or release blocker. labels Sep 19, 2026
@DeusData

Copy link
Copy Markdown
Owner

Thank you for the Linux alias fix and the platform matrix. Current activation handling still makes this a real compatibility gap, but the trust-boundary details need more review. We need more time to review before giving an integration decision; this is not a request for speculative rebases or broader permission exemptions.

@DavidHLP

Copy link
Copy Markdown
Contributor Author

Thanks for the clarification. Understood. I’ve kept the change limited to the explicit Linux aliases and left the existing rejection of arbitrary user-owned symlinks unchanged. I won’t make speculative rebases or broaden permission exemptions. I also tightened the PR description to state the current scope, validation, and remaining limits clearly. I’ll wait for the trust-boundary review.

@DeusData

Copy link
Copy Markdown
Owner

Approved. The trust-boundary question that was holding this has been settled, and I want to record the reasoning rather than just flipping a switch.

The gate here is narrow in three independent ways, and all three have to hold before anything is resolved:

  1. A fixed allowlist, not a policy — /tmp, /var, /home on Linux. Nothing else is considered, so this cannot grow into general symlink following by accident.
  2. The entry must actually be a symlink (S_ISLNK), so a real directory at those paths is untouched.
  3. The symlink must be root-owned (st_uid != 0continue). Arbitrary user symlinks still fail the O_NOFOLLOW walk below, which is the property that made the original macOS version safe.

That last point is what makes the extension sound: an attacker who can plant a root-owned symlink at /home already has root, so this grants no capability that did not already exist. Meanwhile the legitimate case it unblocks is real and common — immutable and Atomic Linux layouts expose /home as a root-owned alias for /var/home, which is precisely why installation fails there today.

I also appreciate that you narrowed rather than broadened when asked. Your 2026-09-20 note — keeping the change to explicit Linux aliases, leaving the rejection of arbitrary user-owned symlinks untouched, and declining to make speculative rebases — is the right instinct on a security-adjacent path. Splitting the lstat guard into separate early-continue branches is also a genuine readability gain at no semantic cost.

@JohnGalt1717 — thank you for the nudge; you were right that this is a real compatibility gap rather than an edge case.

On timing: main currently carries one failing test unrelated to this PR (a coverage test written against an older response format; fix in #2248). I am landing that first so this does not merge onto a red base, then merging here. Nothing further is needed from you.

@DeusData

Copy link
Copy Markdown
Owner

Merged — main is green again as of 36f7708, so this lands on a clean base as promised. Thank you for the fix, the platform matrix, and for narrowing the scope when asked rather than broadening it. @JohnGalt1717, Ubuntu 26.04 installs should work from the next release.

@DeusData
DeusData merged commit 5b4be65 into DeusData:main Sep 20, 2026
34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working priority/high Needs near-term maintainer attention; high-impact bug, regression, safety issue, or release blocker. security Security vulnerabilities, hardening ux/behavior Display bugs, docs, adoption UX

Projects

None yet

Development

Successfully merging this pull request may close these issues.

install subcommand always fails: 'failed to stage install candidate: activation transaction I/O failed' (v0.10.8, Linux btrfs)

4 participants