test(sessions): run POSIX cross-user tests on macOS in CI

[andychoquette]

Fixes: #263

What was the problem/requirement? (What/Why)

We need macOS support for cross-user Sessions (running Session actions as a jobRunAsUser). The reference Python implementation (openjd-sessions-for-python) needed three code fixes for macOS (OpenJobDescription/openjd-sessions-for-python#335); issue #263 tracks confirming whether the Rust crate needs the same treatment, and validating it on real macOS either way. Until now, nothing built, ran, or CI-tested the cross-user path on macOS.

What was the solution? (How)

No production code changes were needed — the issue's thesis held. The crate's mechanism (process_group(0) at spawn, killpg for signalling, no setsid(1) binary, no pgrep, no procfs) is pure POSIX syscalls, all present on macOS. The embedded-helper design also never needs to discover a pgid across the sudo boundary (the helper reports it over stdout), so it is structurally immune to both bugs the Python implementation had.

The change is test infrastructure + docs:

• A cross-user-macos CI job in ci.yml. macOS runners have no Docker but do have passwordless sudo, so the same user/group layout as the Linux test containers is provisioned directly on the host with Directory Services (sysadminctl/dseditgroup), then the existing test_cross_user:: suite runs with --include-ignored.
• A macOS section in specs/sessions/cross-user-testing.md documenting the two macOS-specific provisioning requirements discovered:
  1. The default per-user TMPDIR (/var/folders/<hash>/T, mode 0700) is not traversable by the target user, so sudo -u <user> -i <helper_path> cannot reach the extracted helper binary (fails as "Helper process closed stdout unexpectedly"). The job sets a world-traversable TMPDIR. This constraint applies to real deployments' session roots as well.
  2. macOS does not create a self-named group per user the way useradd does; the tempdir cleanup test chowns to user:user, so provisioning creates that group explicitly.

What is the impact of this change?

macOS cross-user Sessions are now validated and continuously regression-tested in CI, alongside the existing Linux (localuser + LDAP) and Windows cross-user jobs. Deployers get a documented requirement for session-root traversability on macOS.

How was this change tested?

• Full default test suite on macOS 26.5 (arm64), rustc 1.97.1: 438 tests pass.
• All 16 test_cross_user:: integration tests pass on a real macOS host (including terminate_tree process-tree reaping, SIGTERM notify traps, uid/env isolation, cancel handles, and the disjoint-user permission rejection), first on a local machine and then in the new CI job, which passed on its first run.
• The full CI matrix (fmt, clippy, cargo-deny, MSRV, Linux/Windows/macOS build-test, all three cross-user variants) runs green on my fork.

Was this change documented?

Yes — specs/sessions/cross-user-testing.md (new macOS section with a Linux-vs-macOS provisioning table and a deployment note) and a line in DEVELOPMENT.md. No docstring changes needed (no code changes).

Is this a breaking change?

No. CI + docs only.

Does this change impact security?

No production code changes. The CI job provisions throwaway test users with passwordless sudo scoped to those users only, on ephemeral GitHub-hosted runners. The validation itself exercises the existing security boundary (cross-user isolation, disjoint-group rejection) rather than modifying it.

---

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[leongdl]

I'll start by exploring the repo structure and the key files.Found the empty cfg(target_os = "linux") table already. Now hunting the specific patterns.I'll start by locating the relevant test files.I'll start by reading the diff and the existing CI workflow.## Answer

Yes — with one exception, and the exception is the interesting part.

I read the PR (3 files, CI + docs only, no code) and then audited the cfg(unix) production paths against macOS. The process-control half of the PR's thesis is correct and it is correct structurally, not by luck. The temp-directory half is not: there is a real macOS divergence in production code, and the PR handles it with a doc note plus a CI env override rather than code — which also means CI no longer covers it.

Line numbers below are from this checkout's HEAD (rfc008-wrap-fixes); PR 285 touches no code, so they still apply. I did not run the cross-user suite on macOS.

Where the abstraction genuinely is clean

Every mechanism the PR description names, I checked and it holds:

• Gating in openjd-sessions is cfg(unix) / cfg(windows) only — zero cfg(target_os = ...) in production code. No procfs reads, no pgrep/ps/pkill, no setsid(1) binary (only the setsid() syscall at subprocess.rs:155), no pidfd/prctl/signalfd/clone.
• process_group(0) (helper/src/runner.rs:39) plus setsid() make every child a group leader, so pid == pgid and all signalling is killpg (subprocess.rs:104,113). ESRCH from a reaped leader is discarded at every call site, so the post-wait() sweeps are harmless on both platforms.
• pre_exec is fork-safe: libc::dup2 and libc::setsid only, no allocation, no getpwnam, no logging. This is the class that aborts on macOS, and the code avoids it.
• The nix surface used (killpg, chown, mkdir, Group::from_name, User::from_uid, dup, poll) is fully available on macOS. Notably there is no setgroups/initgroups/getgrouplist, so macOS's NGROUPS_MAX == 16 trap is unreachable.
• Cross-user signalling never crosses a uid boundary — the host only kills its own sudo child; job-process killpg happens inside the helper, already running as the job user. So macOS's kill-permission rules are never exercised differently.
• The helper's write-then-exec is safe on macOS: per-session UUID filename sidesteps the stale-codesign-cache-by-path failure, fs::write applies no quarantine xattr, nothing rewrites the path after exec.

The embedded-helper design being immune to pgid discovery across the sudo boundary is a real architectural property, and it is the reason the Python fixes don't apply. That part of the description is accurate.

The exception: std::env::temp_dir() is the one Linux-shaped line

tempdir.rs:70, in default_openjd_dir():

#[cfg(unix)]
{
    std::env::temp_dir().join("OpenJD")
}

The docstring says "typically /tmp/OpenJD". Verified on this macOS host: TMPDIR=/var/folders/x0/…/T, mode 0700, owner-only. So on any macOS process that inherits a launchd per-user TMPDIR, the default session root is not traversable by the job user — the helper can't be exec'd, the working dir can't be entered, and sudo -i rm -rf at session.rs:1040 can't reach the files. session.rs:572 and tempdir.rs:159 both take this default when session_root_directory is None.

Three consequences worth deciding on before merge:

1. The claim needs a qualifier. "The full POSIX cross-user suite passes on macOS with no code changes" is true only with a non-default TMPDIR. The spec says this three lines down in a table row and again in the deployment note; it should be in the sentence itself.
2. The guard checks the opposite direction. find_missing_sticky_bit (tempdir.rs:95) flags world-writable-without-sticky — a security property. A 0700 ancestor sails through StickyBitPolicy::Strict and the session then fails at first exec with "Helper process closed stdout unexpectedly" (cross_user_helper.rs:615). There is no access(X_OK) or ancestor o+x/g+x check anywhere on the POSIX side (I grepped). On Linux /tmp satisfies both properties, which is why this never surfaced.
3. The CI job removes the coverage. The job sets TMPDIR=/private/tmp/openjd-tests at 1777 job-wide, and the three TempDir::new(None, …) tests are exactly the ones that would have exercised the default. So the job tests a temp-root shape no default macOS login-session process has, and a regression in 0700 handling would not be caught.

My suggestion is small: one #[ignore]d POSIX test that builds a 0700 parent, points a session root under it, and asserts the resulting error. Pinning today's behaviour is enough — it makes both a regression and a future improvement visible, and it gives the deployment note something falsifiable behind it. A pre-flight traversability check with an actionable error would be better, but that's a separate change.

Worth noting the nuance for the deployment note: whether this bites depends on launch context. launchd sets the private TMPDIR for per-user domain services and login sessions; system LaunchDaemons typically get /tmp. So the same binary works as a daemon and fails when a developer runs it from a terminal.

Test-integrity findings (these affect what "validated on macOS" means)

• test_cross_user_tempdir_disjoint_fails (test_cross_user.rs:503-524) asserts nothing on macOS. The body is if let Ok(td) = result { … } with "Err is also acceptable". Its comment claims chown failure is silent (let _ = chown), but tempdir.rs:179-186 now returns SessionError::PathPermissions — I read both. So the result is always Err, the if let body is dead, and zero assertions run. The PR names this as the disjoint-permission test. It pins nothing on any platform, and the stale comment is what makes it look like it does.
• test_cross_user_subprocess_terminate (:138-162) asserts only state == Timeout || state == Failed with no stdout check. Failed is also what exec-failure or permission-denied produces, so a macOS inability to run the script as the target user is indistinguishable from a successful timeout-kill. Its siblings add a stdout.contains(...) check, which rescues them.
• test_cross_user_subprocess_notify never asserts "Trapped" appears, so a straight SIGKILL passes identically — the SIGTERM trap path the description credits is not actually pinned. Same for terminate_tree: no pid/group check distinguishes "killed via killpg" from "orphaned, pipe closed". On macOS, process-group kill is the single behaviour most worth proving.
• The job runs a bare filter. libtest exits 0 when a filter matches nothing, so any future rename or cfg narrowing turns this job silently green. The Linux jobs are safe here only because they run unfiltered.

No silent-skip risk in the suite as a whole: require_target_user() panics on unset env vars, and the "Verify provisioning" step is genuinely good — it hard-fails on every provisioning assumption including sudo -u … -i /usr/bin/true. Impersonation is really asserted by test_cross_user_runner_uid.

CI job, smaller items

• No needs: build-test and its own read/write cache in debug, while cross-user-windows deliberately does needs: + cache/restore + --release with a comment explaining why. The macOS cache key also drops the rustc hash and .cargo/config.toml hash that build-test documents as necessary, and there's no equivalent of the "discard stale target/" step.
• No timeout-minutes (family-wide gap, but this is the leaked-process-group workload) and no if: always() teardown, which the Windows job has.
• set -euxo pipefail echoes -password "OpenJD-ci-test-1!" into a public log. The value is already in source so nothing new leaks, but generating it (as Windows does) is free — nothing needs it, since sudo is NOPASSWD.
• dseditgroup -o edit -d … staff || true looks inert: sysadminctl puts users in staff via PrimaryGroupID, not as a member record, and the preceding PrimaryGroupID reassignment has already moved them. Either drop it or drop the || true so a wrong assumption fails loudly.
• TMPDIR only needs to be step-scoped; making it job-wide is what forces the pre-checkout "Create temp root" step.
• runner is hardcoded in two places; createhomedir runs before the PrimaryGroupID change, leaving the disjoint user's home group as staff.

Docs

"Open Directory integration is exercised implicitly because nix::unistd::User::from_name() resolves through Directory Services" overstates it — these are local DS accounts, the same path as any local macOS user, not the networked-directory coverage the LDAP variant provides. Suggest saying there is no networked-directory variant on macOS.

Two pre-existing items, not this PR's

• PosixSessionUser::new(user, None) defaults the group to the process's effective group (session_user.rs:36-42) — on macOS that's staff for every ordinary local account, and downstream 0o770/0o750 chmods then widen the working dir, embedded files, and helper binary to all local users. I checked the reference implementation: Python does exactly the same (_session_user.py:109, grp.getgrgid(os.getegid())). So this is inherited parity, not a Rust defect, and out of scope here — but it's a macOS-specific sharp edge worth a line in the spec.
• Cargo.toml:62 [target.'cfg(target_os = "linux")'.dependencies] is an empty table with a comment describing what the cfg(unix) build-dependencies below actually do. Dead; removing it is a no-op. Ironically it's the strongest evidence for the PR's thesis: there is no functional Linux gating anywhere in the crate.
• build.rs:49: is_unix = target.contains("linux") || target.contains("unix") || cfg!(unix). Darwin triples match neither contains, so correctness rests on the host cfg!(unix). Fine for native macOS; a cross-build to darwin from a Windows host would take the Windows branch and panic. target.contains("unix") matches no real triple.

Not verified

I did not run the suite on macOS, so the loose-assertion findings are "these tests could pass for the wrong reason", not "they do". I could not confirm from runner-images docs that GitHub's macos-latest really sets a 0700 /var/folders TMPDIR — I confirmed the shape on this host only. One stat -f %Lp "$TMPDIR" echo before the override would keep that honest and make the override self-documenting.

[github-actions]

The comment claims the directory "must not be left behind" on chown failure, but that is not what happens. TempDir::new (tempdir.rs:167-198) does create_dir(&path) first, then nix::unistd::chown(...).map_err(...)?. On chown failure the ? returns Err before the TempDir struct is ever constructed, so its Drop-based best-effort cleanup never runs — the randomly-named subdirectory is left orphaned under the parent (here /tmp/OpenJD).

The test also does not assert the no-leftover property, so the comment is describing behavior that is neither guaranteed by the code nor checked here. Either drop the "and the directory must not be left behind" clause, or (better) have the code remove the just-created directory when chown fails and assert its absence.

[andychoquette]

Addressed feedback from @leongdl:

The one real production divergence — temp_dir(). Added an #[ignore]d test (tempdir_under_untraversable_parent_is_currently_created) that pins today's behavior: TempDir::new under a 0700 parent succeeds, and find_missing_sticky_bit does not (and is not meant to) flag it. So both a regression and a future pre-flight check become visible. I kept the actual pre-flight traversability check out of this PR — it's a library behavior change, and I've filed it as a follow-up rather than smuggle it into a CI/test PR. The deployment note now also spells out the launch-context nuance you raised (per-user domain / login session gets the private TMPDIR; system LaunchDaemons get /tmp — so the same binary works as a daemon and fails from a terminal).

Test integrity — all correct, all fixed:

• test_cross_user_tempdir_disjoint_fails was dead (the if let Ok body never ran because chown now returns Err via ?, and the "chown is silent" comment described deleted code). It now asserts Err(SessionError::PathPermissions) and that no directory is left behind.
• notify/terminate/terminate_tree were asserting too loosely. But digging in, I found asserting "Trapped" on notify would have been wrong: run_subprocess's timeout uses CancelMethod::Terminate (immediate SIGKILL), so the trap correctly never fires there. The SIGTERM path was genuinely unpinned, so I added test_cross_user_notify_delivers_sigterm, which cancels via the handle with a grace period (NotifyThenTerminate) against the trap script and asserts "Trapped" actually appears — i.e. SIGTERM really crosses the sudo boundary and reaches the workload's process group. terminate now asserts run-and-cutoff (distinguishing a real kill from failure-to-launch), and terminate_tree asserts neither the child ("Log from test 19") nor the parent ("Log from runner 19") completes, so an orphaned child would now fail the test.

CI: empty-filter guard (counts matched tests via --list and fails if zero — thanks, that one would have bitten us silently), timeout-minutes: 30, if: always() user/sudoers teardown, dropped the inert dseditgroup -d ... staff, and a stat -f echo so the TMPDIR override is self-documenting. Left the debug-profile/no-needs cache difference as-is for now since this job doesn't share the build-test cache, but happy to align it if you'd prefer.

Docs: softened the Open Directory claim (local DS accounts, same local-node path as any local user — not networked-directory coverage), and added a spec note on the PosixSessionUser::new(user, None) default-group-widening edge (which, as you noted, is inherited parity with the Python reference, so out of scope to change here).

Follow-up issue for the two library-behavior items (pre-flight traversability check; whether the default-group behavior should change on macOS): . I did not re-run the suite claim-by-claim on macOS before your review, but I have now — all 17 cross-user tests pass on a real Mac, including the new SIGTERM one.

[github-actions]

The docstring on this test (lines 101–102) is now stale and contradicts the new NOTE just below it. It still says "the trap handler should fire", but the added NOTE correctly explains that run_subprocess's timeout uses CancelMethod::Terminate (immediate SIGKILL, per runner/mod.rs:44), so the SIGTERM trap is intentionally not expected to fire here — which is exactly why the contains("Trapped") assertion was removed. Consider updating the doc comment to match, so the contract of this test (kill mid-flight, no trap) is unambiguous.

[github-actions]

This revised docstring is misleading. It says the workload "traps SIGTERM but does not exit, so the runner must escalate to SIGKILL" — but the timeout path in run_subprocess does not escalate through SIGTERM at all. It uses CancelMethod::Terminate, and on timeout subprocess.rs sends SIGKILL directly ("Action timed out, sending SIGKILL to process group", subprocess.rs:656). The SIGTERM trap in long_running_ignore.sh never fires here.

As a consequence there is no observable behavioral difference between using long_running_ignore.sh and long_running.sh for this timeout test — both are killed by a single SIGKILL regardless of their TERM trap. If the intent is to exercise a genuine SIGTERM→SIGKILL escalation, this test does not do that (only the notify test with a grace period does); the docstring should be corrected to describe a direct SIGKILL-on-timeout kill.