feat(drivers): inject local sandbox identity#2335
Conversation
Make Docker and Podman select a validated numeric agent identity and inject it into the supervisor, allowing userless images to run without a baked-in sandbox account.\n\nCloses #2331 Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
|
Thank you for your interest in contributing to OpenShell, @matthewgrossman. This project uses a vouch system for first-time contributors. Before submitting a pull request, you need to be vouched by a maintainer. To get vouched:
See CONTRIBUTING.md for details. |
E2E Test AttestationThe local Docker E2E test passed against the committed implementation. Podman is not installed on this development host; the PR adds the equivalent test to the repository's rootless Podman E2E lane.
Test SummaryTests Executed
The fixture asserts that the image contains neither a named |
|
Thank you for your interest in contributing to OpenShell, @matthewgrossman. This project uses a vouch system for first-time contributors. Before submitting a pull request, you need to be vouched by a maintainer. To get vouched:
See CONTRIBUTING.md for details. |
| sandbox_uid: None, | ||
| sandbox_gid: None, |
There was a problem hiding this comment.
QUestion: Why is the default not DEFAULT_SANDBOX_UID?
There was a problem hiding this comment.
the idea was to keep all of the resolution within resolve_sandbox_entity, versus defaulting in multiple locations. But I do think that's a bit confusing, I think it's worth updating here IMO
There was a problem hiding this comment.
Updated this so the Docker and Podman configs now use DEFAULT_SANDBOX_UID as a concrete UID default. We keep sandbox_gid optional so that, when it is omitted, it follows the selected UID (for example, a UID-only override of 12345 resolves to 12345:12345). That matches the effective UID/GID fallback behavior in the Kubernetes and VM drivers.
| /// Numeric UID for the agent process. Defaults to 10001. | ||
| #[arg(long, env = "OPENSHELL_PODMAN_SANDBOX_UID")] | ||
| sandbox_uid: Option<u32>, | ||
|
|
||
| /// Numeric GID for the agent process. Defaults to the resolved UID. | ||
| #[arg(long, env = "OPENSHELL_PODMAN_SANDBOX_GID")] | ||
| sandbox_gid: Option<u32>, | ||
|
|
There was a problem hiding this comment.
Out of scope for this change, but is annonymous struct embedding possible in rust so that one could group common driver options like these?
There was a problem hiding this comment.
so it looks like to be a non-native feature, but serde has their flatten https://serde.rs/attr-flatten.html
- Clap:
#[command(flatten)] - Serde:
#[serde(flatten)]
For example:
struct SandboxIdentityArgs {
#[arg(long)]
sandbox_uid: Option<u32>,
#[arg(long)]
sandbox_gid: Option<u32>,
}
struct Args {
#[command(flatten)]
sandbox_identity: SandboxIdentityArgs,
}Looks like we do use it in some places:
There was a problem hiding this comment.
(thanks for giving an excuse to do an learning-dive on how derive macros work)
so yes this is something we could do, but I wonder if we wait until some amount of these common attributes stabilized in beta+? I mean they're all new to me, so maybe they are stable and I just don't know
| # SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| FROM public.ecr.aws/docker/library/python:3.13-slim |
There was a problem hiding this comment.
Question: Why not something even smaller? Do we need python?
There was a problem hiding this comment.
nope we don't, swapped this to debian bookwork slim
| //! - A running Docker- or Podman-backed OpenShell gateway | ||
| //! - Docker or Podman runtime running (for image build) |
There was a problem hiding this comment.
Do these strictly need Docker or Podman drivers? (not for image build).
|
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. |
Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
|
🌿 Preview your docs: https://nvidia-preview-pr-2335.docs.buildwithfern.com/openshell |
Summary
Make the Docker and Podman compute drivers choose and inject a validated numeric agent identity at runtime, so OpenShell-managed sandboxes no longer require a baked-in
sandboxuser or group. The supervisor now presents the same numeric identity and/sandboxhome consistently for primary and SSH-launched agent processes.Related Issue
Closes #2331
Changes
10001:10001, support operator overrides, validate both values against the policy identity range, and override image/user-supplied identity environment variables.HOME,USER, andLOGNAMEconsistently when dropping to a numeric identity.USERis not authoritative for an OpenShell-managed agent.Deviations from Plan
Local Docker validation completed. Podman is not installed on this development host, so the new Podman E2E target was compiled locally and is left to the repository's rootless Podman CI lane for runtime validation.
Testing
mise run pre-commitpassesTests added:
custom_imagecompile.docker_sandbox_from_userless_dockerfilepasses locally;podman_sandbox_from_userless_imageis covered by rootless Podman CI.Checklist
Documentation updated:
architecture/compute-runtimes.mdandarchitecture/sandbox.md: runtime identity ownership and flow.