Repository navigation
fix(minimal): accept a sticky root-owned directory and check before changing the host - #2195
Merged
norrietaylor merged 2 commits intoOct 13, 2026
Merged
Conversation
…hanging the host min finalize-install refused to install the DNS service on a Mac whose /Library/PrivilegedHelperTools carries the sticky bit, and left the resolver file and the local range behind when it refused. The directory check no longer tests the sticky bit. On a root-owned directory closed to group and other writes the bit grants nobody anything: it only narrows who may rename or remove an entry. Ownership and the write bits are still checked on the directory and every ancestor. The macOS step now runs the service program's verified copy first, for a bare program and for an app bundle alike. The copy holds every check that can refuse, so a refusal leaves no resolver file and no range unit behind.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Title check |  |
The title clearly describes both primary changes: accepting sticky root-owned directories and performing checks before host changes. It uses a concise Conventional Commit format. |
| Description check | |
The description explains the problem, implementation changes, tests, verification results, known failures, and unrun checks. It is mostly complete, although it does not use the template's exact Summar… |
| Docstring Coverage | |
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 15 functions across 1 files. |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
✨ Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
I’m a rabbit checking paths in the night
Root-owned and closed means the permissions are right
Sticky bits pass; the checks carry on
Copy comes first, then resolver is drawn
Units follow, and I hop toward the dawn
Comment @coderabbitai help to get the list of available commands.
Contributor
|
Approved: no findings at Nothing to do. Clobber: none: |
The Linux step had the same ordering as the macOS one: it made the resolver link before the service program's verified copy, so a refused copy left the link behind with no service to answer. The answerer steps are split into a copy half and a unit half, as on macOS. The copy runs first, then the resolver link, then the units.
jtnkminimal
approved these changes
Oct 13, 2026
This was referenced Oct 13, 2026
norrietaylor
added a commit
that referenced
this pull request
Oct 15, 2026
…hanging the host (#2195) * fix(minimal): accept a sticky root-owned directory and check before changing the host min finalize-install refused to install the DNS service on a Mac whose /Library/PrivilegedHelperTools carries the sticky bit, and left the resolver file and the local range behind when it refused. The directory check no longer tests the sticky bit. On a root-owned directory closed to group and other writes the bit grants nobody anything: it only narrows who may rename or remove an entry. Ownership and the write bits are still checked on the directory and every ancestor. The macOS step now runs the service program's verified copy first, for a bare program and for an app bundle alike. The copy holds every check that can refuse, so a refusal leaves no resolver file and no range unit behind. * fix(minimal): run the linux verified copy before the resolver link The Linux step had the same ordering as the macOS one: it made the resolver link before the service program's verified copy, so a refused copy left the link behind with no service to answer. The answerer steps are split into a copy half and a unit half, as on macOS. The copy runs first, then the resolver link, then the units. (cherry picked from commit eefd8ef)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
On a Mac whose
/Library/PrivilegedHelperToolscarries the sticky bit,min finalize-installstops with:Two defects are behind that report.
/etc/resolver/min.internaland loaded the local-range unit before it ran the directory check. A refused install left both behind, with no service to answer.Change
root_dir_stepsno longer tests the sticky bit. It still requires root ownership and no group or other write, on the directory and on every ancestor up to/.macos_names_stepsruns the service program's verified copy first, for a bare program and for an app bundle. The copy holds every check that can refuse (directory custody, hash pin, Developer ID re-check), so a refusal leaves no resolver file and no range unit behind.linux_names_stepsdoes the same. The Linux answerer steps are split into a copy half and a unit half, as on macOS: the copy runs first, then the resolver link, then the units.docs/internal/release-pipeline.mddescribes the check as it now is.Tests
privileged_copy_refuses_a_user_writable_ancestornow accepts mode1755and refuses1777.copy_runs_before_anything_changes_the_hostasserts the directory check precedes every host change: the resolver file and the range unit on macOS (bare and bundle form), the resolver link and the unit files on Linux.advisory_command_reserves_the_range_on_macos,linux_advisory_command_is_unchangedandfinalize_install_show_script_is_an_annotated_posix_scriptare updated to the new step order.Verification
Run from macOS by cross-compiling to musl in Docker:
cross clippy -p minimal --all-targets -- -D warnings: clean.cross test -p minimal: 438 passed, 2 failed. Every resolver test passes. The two failures are incrates/minimal/src/tests.rs, which this PR does not touch:a_stopped_vm_does_not_hold_a_box_name_resolution (a timing bound, 1.3–1.6 s under emulation) andlegacy_network_spellings_parse_with_hint(stderr capture). I did not run them on the base commit to confirm they fail there too.just lint-prose docs/internal/release-pipeline.md: 0 alerts.Not run:
just e2e,just test-vm, the real privileged step on a Mac with a sticky/Library/PrivilegedHelperTools, and the real privileged step on a Linux host.Needs a forward-port to main.
Summary by CodeRabbit