fix(cli): trust root-owned per-user /home entries during activation walk - #2309
BumaldaOverTheWater94 wants to merge 2 commits into
Conversation
|
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. What that means for this PR, concretely:
Things that will genuinely speed it up whenever review does happen:
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. |
DeusData
left a comment
There was a problem hiding this comment.
Thank you for #2306 — a root-owned /home/<name> symlink is a common Linux layout, and the check you added is narrow in the right way: only root can create or change that entry, so another user cannot race the lstat and realpath window, and every component of the resolved path is still walked with O_NOFOLLOW.
Three things before it merges:
- Memory-core lint. The
lintjob is red because raw allocator use grew insrc/cli/activation_transaction.c(80 to 82). Building the mapped path in a stack buffer and returningactivation_string_copy(buf)keeps the count flat, and callers stillfree()it as before. - A test. This loosens a security check, so it needs one that fails without the change. Either a test seam that swaps the
/homeprefix and the uid checks, or a Linux container test that runs as root, would do. - Consider folding it into the existing
/homealias loop (the #2175 branch), which does almost the same walk. Note one deliberate difference to keep visible: the alias branch requires a root-owned target, while this one also accepts a target owned by the current user. That is reasonable, and worth one comment saying so.
Thank you again.
Managed Linux hosts keep /home a real directory and point /home/<user> at another tree (e.g. /local/home/<user>). The O_NOFOLLOW walk rejected that entry and install failed with 'activation transaction I/O failed'. Resolve /home/<name> only when /home is root-owned and not group/world writable, the entry is a root-owned symlink, and it resolves to a directory owned by root or the current user. Arbitrary user-owned symlinks are still rejected. Fixes DeusData#2306 Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: BumaldaOverTheWater94 <83429948+BumaldaOverTheWater94@users.noreply.github.com>
Address review on DeusData#2309: - Fold the /home/<name> handling into the existing DeusData#2175 alias loop via a shared activation_alias_map() that builds the mapped path in a stack buffer and returns activation_string_copy(); raw allocator sites in activation_transaction.c drop from 80 to 78 (baseline lowered). - Comment the deliberate difference: the per-user entry may resolve to a directory owned by the current account, the whole-alias case may not. - Add cbm_activation_transaction_set_home_root_for_testing() (test seams only) and a Linux test that fails without the per-user branch. Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: Glen Ko <gleko@amazon.com>
a9a2535 to
48265f5
Compare
Fixes #2306
Linux
installfailed withactivation transaction I/O failedwhen/homeis a real directory but/home/<user>is a root-owned symlink (for example/home/alice -> /local/home/alice). The #2175 alias handling only covers/homeitself being a symlink.activation_posix_walk_path()now also resolves/home/<name>(Linux only), but only when all of these hold:/homeis a root-owned directory that isn't group- or world-writableArbitrary user-owned symlinks are still rejected by the
O_NOFOLLOWwalk.Verification (AL2023 x86_64, gcc 11.5,
/home/$USERis a root-owned symlink to/local/home/$USER). I raninstall -y --force --skip-config --dir=$HOME_SANDBOX/binwith a sandbox HOME/CBM_CACHE_DIR under/home/$USER:main@ same base:error: failed to stage install candidate: activation transaction I/O failed, rc=1Install complete, rc=0scripts/test.shsuites: 8175 passed, 0 failed, 8 skipped (143 suites)clang-format20--dry-run --Werror: clean. clang-tidy and cppcheck weren't available locally.No unit test is included: the case needs a root-owned symlink under
/home, which the existing fixtures can't create without root.Written with Claude Code (AI-assisted), on behalf of the account owner, who is accountable for the change.