Skip to content

feat(docker): scope managed resources by host UID - #5136

Open
vineethsai7 wants to merge 2 commits into
NVIDIA:mainfrom
vineethsai7:feat/4273-v1
Open

vineethsai7 wants to merge 2 commits into
NVIDIA:mainfrom
vineethsai7:feat/4273-v1

Conversation

@vineethsai7

@vineethsai7 vineethsai7 commented Nov 2, 2026 •

Copy link
Copy Markdown

Summary

Add an opt-in Docker resource scope for per-user gateways on a shared Linux host. Each gateway uses its effective host UID with the existing Docker sandbox namespace, so its startup reconciliation and sandbox operations select its own resources.

This is a draft design and implementation slice. Packaged gateways still need automatic port selection and registration before the full multi-user experience works without manual configuration. The issue is still state:triage-needed; design feedback is welcome.

Related Issue

Part of #4273

Changes

  • Add [openshell.drivers.docker].scope_by_host_uid, defaulting to false.
  • Reuse the driver's existing namespace in container labels and filters, private volume names, and host-side sandbox state. When enabled, the namespace is uid-<effective UID>-<sandbox_label>.
  • Refuse startup in scoped mode if Docker still has OpenShell containers or volumes under the unscoped label. This keeps a config change from hiding existing sandboxes from their gateway.
  • Document the opt-in, migration rule, and Docker daemon trust boundary; add unit and mock-Docker tests.

Design

Proposed complete path

Keep the per-user gateway model now used by the Linux packages and Snap. Each OS user retains a separate gateway database, TLS material, and client registration. The gateway's loopback port must differ for each concurrent user; loopback alone is not the authentication boundary, so each client must continue to present that gateway's own credentials.

For packaged defaults, the next slice should bind the loopback listener to port 0 before constructing the compute driver, pass the assigned nonzero port to Docker and Podman supervisors, and publish the ready endpoint in a private per-user runtime file. Package startup and CLI registration should read that endpoint and refresh the user's gateway record after restarts. Publish only after the listener and gateway are ready, and reject stale records by checking the authenticated endpoint. This avoids a fixed port collision and the collision risk of a UID-to-port hash. The Docker driver's current fixed-port validation must move after the listener binds.

For Docker, the existing sandbox_label namespace already controls resource creation, list and watch, startup cleanup, private volume names, and host state. This slice derives a different namespace for each host UID at that point. It uses the compute driver's existing selection path, where Docker resource ownership belongs. There is no new gateway authorization layer or event schema. The namespace is a driver scope, not a privilege boundary against direct Docker API access. A later installation identifier could distinguish two gateways under one UID and handle UID reuse if that becomes a supported case.

Alternatives considered

  • A shared gateway with per-user mTLS certificates or Unix peer credentials and workspace authorization could work, but it requires certificate issuance or local peer identity mapping, role policy, and automatic workspace provisioning. It is a larger change than extending the packaged per-user services.
  • OIDC with workspaces already provides multi-user authorization, but adds an identity provider to a single shared host.
  • Gateway interceptors are after authentication and do not cover every list, exec, or connection path. They cannot supply this isolation boundary.
  • Manually assigning a unique Docker sandbox_label and port to each user works today but is fragile. This option derives the Docker scope from the process UID, leaving only the port and registration gap.

User-facing example

For a fresh per-user Docker gateway, an operator can set:

[openshell]
version = 2

[openshell.gateway]
compute_driver = "docker"
bind_address = "127.0.0.1:17671" # a distinct port is still required in this slice

[openshell.drivers.docker]
scope_by_host_uid = true
# sandbox_label defaults to "default"

From that user's account:

openshell gateway add https://127.0.0.1:17671 --local --name openshell
openshell sandbox list

For UID 1000, the Docker workload, supervisor, and private volume label is openshell.ai/sandbox-namespace=uid-1000-default; UID 1001 selects uid-1001-default. No new OCSF event is emitted by this option.

Compatibility

The option is off by default, so existing gateways retain their labels, volume names, and state paths. Enable it before creating sandboxes. If unscoped managed containers or volumes remain, startup fails with an actionable error; delete the old sandboxes through their original gateway first. Keep each scoped gateway under a stable host UID. Gateways under the same UID still share a Docker scope. Anyone who can use the Docker daemon directly can inspect or alter all users' containers regardless of OpenShell's gateway separation.

Release Notes

Docker gateways can opt in to host-UID-scoped sandbox resources with scope_by_host_uid = true. Packaged multi-user setup still requires separate ports and registrations.

Testing

  • Checks appropriate to the affected code and behavior pass: cargo fmt -p openshell-driver-docker -- --check, cargo clippy --quiet --locked -p openshell-driver-docker --all-targets -- -D warnings, uv run tasks/scripts/check_docs_nav.py, Fern check, and Markdown lint. All ran through dcb with nice -n 15.
  • Unit tests added and updated: cargo test --quiet --locked -p openshell-driver-docker passed (154 passed, 2 ignored). Tests cover distinct UID scopes and a mock Docker API with legacy containers or volumes.
  • E2E tests added or updated. A two-OS-user, one-Docker-daemon test belongs with the packaged port and registration work. No test host was used for this slice.

Fern reported zero errors and three warnings that are also present with the base configuration page: an MDX parse warning, skipped redirect check without authentication, and theme contrast.

Follow-ups

  1. Bind a per-user dynamic loopback port, publish the ready endpoint, and update package and CLI registration so no hand-edited port is needed.
  2. Enable Docker UID scoping in fresh per-user package defaults after defining migration for existing unscoped sandboxes; document the supported shape in installation guidance.
  3. Exercise two OS users against one Docker daemon: create, list, connect, exec, delete, restart, and upgrade one gateway while verifying the other's resources remain intact. Include the Podman package path for port behavior.
  4. Decide whether same-UID gateways or UID reuse require a persistent installation identifier in addition to the UID scope.

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)
  • Architecture docs updated (Docker driver README and gateway configuration)

Review follow-up

Reviewed upstream PR #5136 against NVIDIA/OpenShell main on 2026-11-02. Full findings and scenarios are in REVIEW-5136.md.

  • R1 (P2) — Fixed in 06d162c1e. Scoped Docker workload, staging, and supervisor names now include a stable namespace digest. Unscoped names retain their previous format. The identical-sandbox-ID regression failed before and passed after the fix.
  • R2 (P2) — Fixed in 06d162c1e. Pending deletion now verifies Docker ownership labels and removes the inspected container ID. The foreign-container regression failed before and passed after the fix; a separate test checks ID-based removal.
  • R3 (P3) — Fixed in 06d162c1e. The migration mock now verifies the managed-by and unscoped-namespace Docker filters.
  • R4 (P3) — Fixed in 06d162c1e. Migration docs and the troubleshooting skill now instruct operators to stop the old gateway after deleting old sandboxes and before enabling UID scoping.

GitHub has no inline comments or submitted reviews for this PR. The two issue-level comments are a workflow-vetting bot notice and the author's DCO affirmation; neither requires a code change.

Follow-up checks on 06d162c1e, all through dcb with nice -n 15: cargo fmt -p openshell-driver-docker -- --check; cargo clippy --quiet --locked -p openshell-driver-docker --all-targets -- -D warnings; cargo test --quiet --locked -p openshell-driver-docker (157 passed, 2 ignored); uv run tasks/scripts/check_docs_nav.py; Fern check (0 errors, 3 existing warnings); and markdownlint-cli2@0.22.0 (0 errors). A Docker E2E run was not available on the build box because the Docker CLI is absent, and no test host was used.

Manual verification

PR: NVIDIA/OpenShell #5136 (feat/4273-v1), head 06d162c1e642.
Before: upstream main be5e49499 (openshell 0.1.7-dev.40+gbe5e49499), two per-user gateways on one Docker host with the default sandbox label. After: PR head with scope_by_host_uid = true.

Environment

  • Host: RHEL 9.8 x86_64 (EC2), Docker Engine 29.8.1 (one root daemon shared by both users), SELinux enforcing.
  • Two disposable local OS users, A (uid 1056) and B (uid 1057), both members of the docker group. Each runs its own gateway (own SQLite database, own JWT keys, own loopback port, plaintext local-development config), Docker compute driver, and the same sandbox_label (the default, default). No host OpenShell service was involved.
  • Binaries built on an Ubuntu 24.04 box (cargo build --release), supervisor and sandbox runtime packaged into local images and set as supervisor_image / sandbox_runtime_image. Gateway and CLI were run on RHEL 9 through an Ubuntu 24.04 glibc loader.

Steps

# user A and user B, each in their own HOME
openshell-gateway                              # compute_driver = "docker", sandbox_label default, own port
openshell gateway add http://127.0.0.1:<port> --name <name>
openshell -g <name> sandbox create --name a-sbx --no-tty -- sleep 3000     # A; b-sbx for B
openshell -g <name> sandbox list
# after: [openshell.drivers.docker] scope_by_host_uid = true on both gateways

Before (main)

A starts a gateway and a sandbox a-sbx. Then B starts a gateway (same Docker host, same label) and creates b-sbx.

# B's gateway start, 19:13:31Z; A's containers killed by the Docker daemon events:
container kill ...openshell-default--a-sbx-...-supervisor
container kill ...openshell-default--a-sbx-...            (19:13:32Z and again 19:13:42Z)
$ docker ps -a --filter label=openshell.ai/sandbox-namespace=default
openshell-default--b-sbx-...-supervisor   Up 28 seconds (healthy)
openshell-default--b-sbx-...              Up 29 seconds
openshell-default--a-sbx-...              Exited (137) 27 seconds ago
$ openshell -g <A's gateway> sandbox list
a-sbx  2026-11-02 19:13:16  Error

Starting B's gateway killed A's running sandbox. B's gateway log has no explicit line about it; the kills coincide with B's gateway start-up, which looks at every OpenShell container under the shared default label (A's sandbox is unknown to B's database). Nobody asked B to touch a-sbx.

After (PR head, scope_by_host_uid = true)

  1. Startup with legacy unscoped resources present (the two default-label containers and six private volumes left from the run above):
Error:   × execution error: configuration error: scope_by_host_uid requires an empty
  │ unscoped Docker namespace 'default'; remove its OpenShell containers and
  │ volumes before enabling it

The gateway refuses to start, as documented, instead of hiding or adopting the legacy resources.

  1. After removing those legacy containers and volumes, both gateways started with scope_by_host_uid = true:
$ (A) openshell -g osp-sa sandbox list           a-sbx  Ready
$ (B) openshell -g osp-sb sandbox list           No sandboxes found.      <- B starts after A's sandbox exists
$ (B) ... sandbox create --name b-sbx ...; sandbox list      b-sbx  Ready
$ (B) openshell -g osp-sb sandbox delete a-sbx   "Sandbox a-sbx already deleted"  (no such sandbox for B)
$ (B) openshell -g osp-sb sandbox exec a-sbx -- id    sandbox not found
$ (B) restart B's gateway; sandbox list          b-sbx  Ready
$ (A) openshell -g osp-sa sandbox list           a-sbx  Ready                  <- unaffected
$ docker ps -a (namespace label)
openshell-021e2443...-default--a-sbx-...           Up   ns=uid-1056-default
openshell-86182493...-default--b-sbx-...           Up   ns=uid-1057-default

A's sandbox stayed Ready through B's gateway start, B's sandbox creation and B's gateway restart. Each user's containers carry their own uid-<uid>-default namespace and namespace-prefixed container names.

Result

PASS for the documented behaviour: scoped gateways do not see or touch each other's sandboxes (including across a gateway restart), and scoped startup refuses unscoped legacy resources with the documented message. Before: FAIL on main (one user's gateway start killed the other user's running sandbox).

Limits seen (documented by the PR, not tested as bugs): the scope is a driver-level label, not a privilege boundary against direct Docker API access (both users are in the docker group and could use the daemon directly). sandbox delete of a name the gateway does not know answers "already deleted" rather than "not found".

Signed-off-by: Vineeth Sai Narajala <vnarajal@cisco.com>
@copy-pr-bot

copy-pr-bot Bot commented Nov 2, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@vineethsai7

Copy link
Copy Markdown
Author

I have read the DCO document and I hereby sign the DCO.

Signed-off-by: Vineeth Sai Narajala <vnarajal@cisco.com>
@vineethsai7
vineethsai7 marked this pull request as ready for review November 3, 2026 04:23
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-03T04:28:16.077578Z 06d162c Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 06d162c1e6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +1049 to +1051
if docker_config.scope_by_host_uid {
reject_unscoped_docker_resources(&docker, &docker_config.sandbox_label).await?;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Guard the reverse migration out of UID scope

When an operator turns scope_by_host_uid off after creating scoped sandboxes, this condition skips the only migration guard. The runtime namespace then reverts to the bare sandbox_label, while list, delete, and reconciliation paths filter that namespace exactly (for example in list_managed_container_summaries), so the existing uid-<uid>-... containers, volumes, and private state become invisible to the gateway and may remain running or orphaned. Reject startup while the current UID-scoped namespace is nonempty, or provide an explicit reverse migration.

Useful? React with 👍 / 👎.

This branch has not been deployed

No deployments
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