Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
andychoquette wants to merge 4 commits into
base: main
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7c783e8
test(sessions): run POSIX cross-user tests on macOS in CI
andychoquette Jul 27, 2026
ff6d873
Merge branch 'main' into macos-support
andychoquette Aug 7, 2026
fd69061
docs(test): correct the timeout-path docstrings on cross-user tests
andychoquette Aug 7, 2026
d62ca94
test(sessions): make the tree-kill assertions actually exercise the t…
andychoquette Aug 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
140 changes: 140 additions & 0 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,146 @@ jobs:
bash scripts/run_cross_user_tests.sh
fi

# macOS has no Docker, but the runner has passwordless sudo, so the test
# users/groups are provisioned directly on the host with Directory Services
# (the macOS analog of the useradd/groupadd calls in the Linux containers).
cross-user-macos:
name: Cross-User Tests (macOS)
runs-on: macos-latest
timeout-minutes: 30
env:
OPENJD_TEST_SUDO_TARGET_USER: openjd-target
OPENJD_TEST_SUDO_SHARED_GROUP: openjd-shared
OPENJD_TEST_SUDO_DISJOINT_USER: openjd-disjoint
OPENJD_TEST_SUDO_DISJOINT_GROUP: openjd-disjointgrp
# The default per-user TMPDIR (/var/folders/<hash>/T) is mode 0700, so the
# target user cannot traverse to the session working directory or execute
# the extracted helper binary inside it. Tests need a world-traversable
# temp root (real deployments use a shared session root such as
# /var/lib/deadline/sessions for the same reason). Job-scoped, so it must
# be created before any step that writes temp files -- see the first step.
TMPDIR: /private/tmp/openjd-tests
Comment thread
andychoquette marked this conversation as resolved.
steps:
- name: Create temp root
# TMPDIR is exported to every step; the directory must exist before
# checkout/rustup/cache write their first temp file.
run: |
sudo mkdir -p "${TMPDIR}"
sudo chmod 1777 "${TMPDIR}"
# Record the default per-user TMPDIR this runner would otherwise use,
# so the override is self-documenting: the tests rely on it being
# world-traversable, unlike the default /var/folders/<hash>/T (0700).
echo "default TMPDIR mode: $(stat -f '%Lp %N' "${TMPDIR:-/tmp}" 2>/dev/null || true)"
echo "runner default temp: ${RUNNER_TEMP:-unset}"
- uses: actions/checkout@v7
- name: Install rust stable
run: |
rustup toolchain install stable --profile minimal
rustup override set stable
- uses: actions/cache@v6
with:
path: |
~/.cargo/registry/index
~/.cargo/registry/cache
~/.cargo/git/db
target
crates/openjd-sessions/src/helper/target
key: rust-cross-user-macos-${{ hashFiles('**/Cargo.lock') }}
restore-keys: |
rust-cross-user-macos-
- name: Provision test users and groups
run: |
set -euxo pipefail

sudo dseditgroup -o create "${OPENJD_TEST_SUDO_SHARED_GROUP}"
sudo dseditgroup -o create "${OPENJD_TEST_SUDO_DISJOINT_GROUP}"

# Target user: impersonated by the tests; shares a group with runner.
sudo sysadminctl -addUser "${OPENJD_TEST_SUDO_TARGET_USER}" \
-fullName "OpenJD Test Target" -password "OpenJD-ci-test-1!" -shell /bin/zsh
sudo createhomedir -c -u "${OPENJD_TEST_SUDO_TARGET_USER}" > /dev/null
sudo dseditgroup -o edit -a "${OPENJD_TEST_SUDO_TARGET_USER}" -t user "${OPENJD_TEST_SUDO_SHARED_GROUP}"
# Linux useradd gives every user a self-named group and the tempdir
# cleanup test chowns to "user:user"; macOS does not, so create it.
sudo dseditgroup -o create "${OPENJD_TEST_SUDO_TARGET_USER}"
sudo dseditgroup -o edit -a "${OPENJD_TEST_SUDO_TARGET_USER}" -t user "${OPENJD_TEST_SUDO_TARGET_USER}"

# Disjoint user: shares no relevant group with the runner. What the
# disjoint test exercises is that the runner is NOT a member of the
# DISJOINT_GROUP that TempDir chowns to; on macOS both accounts are
# unavoidably in `staff` and Apple's implicit groups (everyone,
# localaccounts, ...), so make the disjoint user's PRIMARY group the
# disjoint group rather than staff to keep the setup as close to the
# Linux container's (truly disjoint) layout as macOS allows.
sudo sysadminctl -addUser "${OPENJD_TEST_SUDO_DISJOINT_USER}" \
-fullName "OpenJD Test Disjoint" -password "OpenJD-ci-test-1!" -shell /bin/zsh
sudo createhomedir -c -u "${OPENJD_TEST_SUDO_DISJOINT_USER}" > /dev/null
sudo dseditgroup -o edit -a "${OPENJD_TEST_SUDO_DISJOINT_USER}" -t user "${OPENJD_TEST_SUDO_DISJOINT_GROUP}"
Comment thread
andychoquette marked this conversation as resolved.
disjoint_gid="$(dscl . -read "/Groups/${OPENJD_TEST_SUDO_DISJOINT_GROUP}" PrimaryGroupID | awk '{print $2}')"
sudo dscl . -create "/Users/${OPENJD_TEST_SUDO_DISJOINT_USER}" PrimaryGroupID "${disjoint_gid}"
# NOTE: no `dseditgroup -d ... staff` here — sysadminctl places users in
# staff via PrimaryGroupID, not a member record, and the reassignment
# above has already moved them off it. A delete would be a no-op.

# The test-running user joins the shared group (matches the Docker layout).
sudo dseditgroup -o edit -a runner -t user "${OPENJD_TEST_SUDO_SHARED_GROUP}"
Comment thread
andychoquette marked this conversation as resolved.

# Passwordless sudo from runner to the test users, mirroring the
# hostuser rule in the Linux containers.
echo "runner ALL=(${OPENJD_TEST_SUDO_TARGET_USER},${OPENJD_TEST_SUDO_DISJOINT_USER}) NOPASSWD: ALL" | sudo tee /etc/sudoers.d/openjd-cross-user-tests
sudo chmod 440 /etc/sudoers.d/openjd-cross-user-tests
sudo visudo -cf /etc/sudoers.d/openjd-cross-user-tests

sudo dscacheutil -flushcache
- name: Verify provisioning
run: |
set -euxo pipefail
id "${OPENJD_TEST_SUDO_TARGET_USER}"
id "${OPENJD_TEST_SUDO_DISJOINT_USER}"
# Check membership at the PROCESS-CREDENTIAL level, not just the
# directory record: bare `id -Gn` reports this step's own kernel group
# set. On macOS, new processes resolve supplementary groups through
# Directory Services at spawn, so steps created after provisioning pick
# up the membership even though the parent Runner.Worker process
# predates it -- this asserts that stays true (the test step below
# inherits its groups the same way this step does).
id -Gn | tr ' ' '\n' | grep -qx "${OPENJD_TEST_SUDO_SHARED_GROUP}"
id -Gn runner | tr ' ' '\n' | grep -qx "${OPENJD_TEST_SUDO_SHARED_GROUP}"
if id -Gn "${OPENJD_TEST_SUDO_DISJOINT_USER}" | tr ' ' '\n' | grep -qx "${OPENJD_TEST_SUDO_SHARED_GROUP}"; then
echo "disjoint user must not be in the shared group" && exit 1
fi
# The property the disjoint tempdir test relies on: the runner is not
# a member of the disjoint group (TempDir chowns to it and must fail).
if id -Gn runner | tr ' ' '\n' | grep -qx "${OPENJD_TEST_SUDO_DISJOINT_GROUP}"; then
echo "runner must not be in the disjoint group" && exit 1
fi
# And the disjoint user's primary group is the disjoint group, not staff.
test "$(id -gn "${OPENJD_TEST_SUDO_DISJOINT_USER}")" = "${OPENJD_TEST_SUDO_DISJOINT_GROUP}"
sudo -u "${OPENJD_TEST_SUDO_TARGET_USER}" -i /usr/bin/true
# The sibling Windows modules are cfg-gated off on unix, so selecting the
# test_cross_user:: module path runs exactly the POSIX cross-user tests.
- name: Run cross-user tests
run: |
set -euo pipefail
# libtest exits 0 when a filter matches nothing, so a future rename or
# cfg change to the test module would silently turn this job green.
# Assert the filter still selects tests before running them.
count=$(cargo test -p openjd-sessions --features test-utils --test integration -- \
--include-ignored --list test_cross_user:: 2>/dev/null | grep -c ': test$' || true)
echo "cross-user tests matched: ${count}"
if [ "${count}" -eq 0 ]; then
echo "ERROR: test_cross_user:: filter matched no tests — the module may have been renamed or cfg'd out." >&2
exit 1
fi
cargo test -p openjd-sessions --features test-utils --test integration -- \
--include-ignored --test-threads=1 test_cross_user::
- name: Remove test users and sudoers rule
if: always()
run: |
sudo rm -f /etc/sudoers.d/openjd-cross-user-tests || true
sudo sysadminctl -deleteUser "${OPENJD_TEST_SUDO_TARGET_USER}" 2>/dev/null || true
sudo sysadminctl -deleteUser "${OPENJD_TEST_SUDO_DISJOINT_USER}" 2>/dev/null || true

# Reuses the Windows build-test cache via a restore-only step so we don't
# pay another 3+ minutes recompiling openjd-expr/model/sessions. The
# test-utils feature used here isn't enabled in build-test, so cargo will
Expand Down
5 changes: 3 additions & 2 deletions DEVELOPMENT.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,9 @@ project was originally ported from Python and the prompts used, see
2. `cargo` (included with the Rust toolchain).
3. Nightly rustfmt for formatting checks (`rustup toolchain install nightly`).

Linux, macOS, and Windows all work. Some sessions tests require Docker (Linux) or a test
user account (Windows) — see `specs/sessions/cross-user-testing.md`.
Linux, macOS, and Windows all work. Some sessions tests require Docker (Linux), locally
provisioned test users (macOS), or a test user account (Windows) — see
`specs/sessions/cross-user-testing.md`.

## The three artifacts

Expand Down
88 changes: 78 additions & 10 deletions crates/openjd-sessions/src/tempdir.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,14 +169,34 @@ impl TempDir {
source: e,
})?;

// Ownership/permission setup after this point can fail (e.g. chown to a
// group the process user is not a member of). The TempDir struct — and
// its Drop-based cleanup — does not exist yet, so on any such failure
// remove the just-created directory rather than orphaning it.
if let Err(e) = Self::setup_permissions(&path, _user) {
let _ = std::fs::remove_dir_all(&path);
return Err(e);
}

Ok(Self {
path,
cleaned_up: false,
})
}

/// Apply ownership and permissions to a freshly-created session directory.
///
/// Split out of [`TempDir::new`] so that a failure here can trigger removal
/// of the directory (the caller owns that cleanup).
fn setup_permissions(path: &Path, _user: Option<&dyn SessionUser>) -> Result<(), SessionError> {
#[cfg(unix)]
{
use std::os::unix::fs::PermissionsExt;
let mode = if let Some(u) = _user.filter(|u| !u.is_process_user()) {
// Cross-user: chown group then set 0o770
// chown before chmod — security: don't grant group access if chown fails
if let Ok(Some(grp)) = nix::unistd::Group::from_name(u.group()) {
nix::unistd::chown(&path, None, Some(grp.gid)).map_err(|e| {
nix::unistd::chown(path, None, Some(grp.gid)).map_err(|e| {
SessionError::PathPermissions {
path: path.display().to_string(),
reason: format!(
Expand All @@ -190,12 +210,12 @@ impl TempDir {
} else {
0o700
};
std::fs::set_permissions(&path, std::fs::Permissions::from_mode(mode)).map_err(
|e| SessionError::TempDir {
path: path.clone(),
std::fs::set_permissions(path, std::fs::Permissions::from_mode(mode)).map_err(|e| {
SessionError::TempDir {
path: path.to_path_buf(),
source: e,
},
)?;
}
})?;
}

// Windows: set DACL — full control for process user, modify for session user.
Expand Down Expand Up @@ -225,10 +245,7 @@ impl TempDir {
}
}

Ok(Self {
path,
cleaned_up: false,
})
Ok(())
}

pub fn path(&self) -> &Path {
Expand Down Expand Up @@ -317,6 +334,57 @@ mod tests {
assert_eq!(result, None);
}

/// A cross-user TempDir under a parent that the session user cannot traverse
/// (e.g. a 0700 directory such as macOS's default per-user `$TMPDIR`,
/// `/var/folders/<hash>/T`) is created by the process user but is unusable:
/// `sudo -u <user> -i <helper>` cannot reach the helper binary and the
/// working directory cannot be entered. This test pins TODAY'S behavior —
/// creation currently *succeeds* (the traversability of ancestors is not
/// checked) — so that both a regression and a future pre-flight
/// traversability check become visible here. `find_missing_sticky_bit`
/// only checks the opposite direction (world-writable WITHOUT sticky), so
/// it does not catch this.
///
/// Requires a non-root process user (root traverses any directory), so it
/// is ignored by default and run explicitly.
#[cfg(unix)]
#[test]
#[ignore = "documents un-traversable-parent behavior; run explicitly as non-root"]
fn tempdir_under_untraversable_parent_is_currently_created() {
use std::os::unix::fs::PermissionsExt;

if nix::unistd::geteuid().is_root() {
eprintln!("skipping: root traverses any directory regardless of mode");
return;
}

let tmp = tempfile::TempDir::new().unwrap();
let private = tmp.path().join("private");
std::fs::create_dir(&private).unwrap();
std::fs::set_permissions(&private, std::fs::Permissions::from_mode(0o700)).unwrap();

// Same-user TempDir under the 0700 parent: creation succeeds today.
// No ancestor o+x/g+x (traversability) check is performed.
let td = TempDir::new(Some(&private), Some("untraversable-"), None)
.expect("TempDir::new currently succeeds even under a 0700 parent");
assert!(td.path().exists());
// The un-traversability is a property of the 0700 PARENT, which a
// different (session) user could not enter -- asserted directly here
// since this test runs single-user.
let parent_mode = std::fs::metadata(&private).unwrap().permissions().mode() & 0o777;
assert_eq!(
parent_mode,
0o700,
"the parent is owner-only; another user cannot traverse it to reach {}",
td.path().display()
);
assert!(
find_missing_sticky_bit(td.path()).is_none(),
"sticky-bit policy does not (and is not meant to) flag an \
un-traversable ancestor"
);
}

/// Mirrors Python TestTempDirWindows::test_windows_temp_dir — verifies the
/// warning when `PROGRAMDATA` is unset.
///
Expand Down
Loading
Loading