Skip to content

fix(supervisor): opt-in reconcile of drifted sandbox state ownership (#2336)#2338

Open
TonyLuo-NV wants to merge 1 commit into
NVIDIA:mainfrom
TonyLuo-NV:fix/sandbox-ownership-reboot-reconcile
Open

fix(supervisor): opt-in reconcile of drifted sandbox state ownership (#2336)#2338
TonyLuo-NV wants to merge 1 commit into
NVIDIA:mainfrom
TonyLuo-NV:fix/sandbox-ownership-reboot-reconcile

Conversation

@TonyLuo-NV

Copy link
Copy Markdown
Contributor

What & why

Fixes (partially, as a contained stop-gap) #2336.

On the rootless Podman/Docker drivers OpenShell attaches no pinned user-namespace mapping and bind-mounts host state as plain rbinds, so after a host reboot shifts the rootless subuid base a persisted sandbox state tree can come back owned by an unrelated host user (root/systemd-network/dnsmasq) with restrictive modes. The sandbox can then no longer read its own state (readonly sqlite, EACCES). Today prepare_filesystem only chowns newly-created read_write paths, so an existing tree is never repaired. (Downstream report: NVIDIA/NemoClaw#6972.)

Change

Add an opt-in, default-off toggle OPENSHELL_RECONCILE_SANDBOX_OWNERSHIP. When set (1/true), prepare_filesystem recursively re-owns an existing sandbox-writable (read_write) path whose owner no longer matches the configured sandbox identity, so a drifted tree becomes readable again under the current mapping.

Deliberately conservative — it does not change existing behavior or architecture:

  • Default off → the create-only chown contract is byte-for-byte unchanged. The existing test prepare_filesystem_skips_chown_for_existing_read_write_paths still passes.
  • A path already owned by the target identity is skipped after a single stat, so healthy launches are a no-op.
  • Only paths the policy already declares sandbox-writable are touched.
  • Symlinks are never followed (reuses the existing chown_sandbox_home symlink refusal).
  • The userns/idmap model is untouched.

This is a mitigation that heals the symptom each launch; it does not pin the mapping. The durable fix (idmapped mounts / deterministic --uidmap/--gidmap) is the architectural work tracked in #2336 and left to maintainers — hence draft, to confirm the toggle approach is wanted before polishing docs.

Tests

cargo test -p openshell-supervisor-process -p openshell-core — all pass, including 5 new root-free unit tests:

  • path_owner_matches_detects_current_and_mismatched_owner
  • path_owner_matches_treats_symlink_as_matching
  • reconcile_read_write_ownership_is_noop_when_already_owned
  • reconcile_read_write_ownership_skips_missing_paths
  • reconcile_read_write_ownership_triggers_on_mismatch

and the preserved prepare_filesystem_skips_chown_for_existing_read_write_paths. cargo fmt --check and cargo clippy --all-targets clean on both crates.

Open questions for maintainers

Signed-off-by: Tony Luo xialuo@nvidia.com

Rootless Podman/Docker sandboxes attach no pinned user-namespace mapping and
bind-mount host state as plain rbinds, so a persisted state tree can come back
owned by an unrelated host user after a host reboot shifts the rootless subuid
base -- leaving the sandbox unable to read its own state (readonly sqlite,
EACCES). prepare_filesystem only chowns newly-created read_write paths, so an
existing tree is never repaired.

Add an opt-in OPENSHELL_RECONCILE_SANDBOX_OWNERSHIP toggle (default off). When
enabled, prepare_filesystem recursively re-owns an existing sandbox-writable
path whose owner no longer matches the configured identity. A correctly-owned
tree is skipped after a single stat, so healthy launches are a no-op and the
existing create-only chown contract -- and its test -- are preserved. Symlinks
are never followed. Only policy-declared read_write paths are considered.

Contained stop-gap; the durable fix (pinning the userns mapping / idmapped
mounts) is tracked in NVIDIA#2336.

Signed-off-by: Tony Luo <xialuo@nvidia.com>
@copy-pr-bot

copy-pr-bot Bot commented Jul 17, 2026

Copy link
Copy Markdown

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant