Skip to content

fix(sandbox): acknowledge unchanged policy revisions - #2

Open
NaveCohenMonday wants to merge 58 commits into
mainfrom
2518-acknowledge-same-hash-revisions/nc
Open

fix(sandbox): acknowledge unchanged policy revisions#2
NaveCohenMonday wants to merge 58 commits into
mainfrom
2518-acknowledge-same-hash-revisions/nc

Conversation

@NaveCohenMonday

@NaveCohenMonday NaveCohenMonday commented Jul 28, 2026

Copy link
Copy Markdown
Owner

Summary

Decouple sandbox policy-revision acknowledgement from OPA policy loading. A
newer sandbox revision whose effective policy hash is already loaded can now be
acknowledged without reloading identical policy content, allowing desired and
current revisions to converge.

Related Issue

Fixes NVIDIA#2518

Changes

  • Track the acknowledged sandbox policy version separately from the loaded
    policy hash.
  • Acknowledge only newer, sandbox-scoped revisions with the same non-empty
    effective policy hash.
  • Wait for required policy-runtime or middleware reconciliation to succeed
    before acknowledging the revision.
  • Exclude global policy, local overrides, equal or older versions, empty
    hashes, and different policy content from the shortcut.
  • Document the same-hash revision acknowledgement invariant.

Testing

  • CARGO_INCREMENTAL=0 mise run pre-commit passes
  • Unit tests added/updated
  • E2E tests added/updated (not applicable)

Focused tests:

CARGO_INCREMENTAL=0 mise exec -- cargo test -p openshell-sandbox \
  unchanged_ --lib

test result: ok. 2 passed; 0 failed

Checklist

Signed-off-by: Evan Lezar <elezar@nvidia.com>
@NaveCohenMonday
NaveCohenMonday force-pushed the 2518-acknowledge-same-hash-revisions/nc branch from 036f3d9 to 177f70a Compare July 28, 2026 15:40
derekwaynecarr and others added 18 commits July 28, 2026 15:56
Replace yield_now() spin-loop with sleep(10ms) polling in
delete_handler_ends_telemetry_for_the_resolved_sandbox_id. The
single-threaded tokio runtime starves the spawn_blocking threads
used by SQLite when yield_now() burns 100% CPU waiting for the
delete gate entry count. Increase the timeout from 1s to 5s for
consistency with similar guard tests.

Signed-off-by: Derek Carr <decarr@redhat.com>
Signed-off-by: Evan Lezar <elezar@nvidia.com>
Signed-off-by: Evan Lezar <elezar@nvidia.com>
* feat(examples): add supervisor middleware content guard

Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>

* docs(examples): refine middleware preview warning

Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>

* fix(examples): add middleware policy version

Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>

* refactor(supervisor-middleware): simplify service endpoints

Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>

* fix(examples): adapt content guard to middleware enums

Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>

* fix(examples): align content guard with merged middleware

Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>

* feat(examples): add content guard smoke flow

Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>

* chore(examples): remove smoke launcher test

Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>

* fix(examples): align content guard smoke with main

Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>

* fix(examples): address content guard review feedback

Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>

* refactor(examples): parse cargo metadata with jq

Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>

* fix(examples): prioritize longest content matches

Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>

* docs(examples): use GitHub warning alert

Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>

* fix(examples): merge overlapping content matches

Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>

* docs(examples): render preview warning on GitHub

Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>

---------

Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>
…#2463)

The Kubernetes driver's default workspace PVC never set storageClassName,
so on clusters with no default StorageClass the PVC stayed Pending and
sandbox creation failed.

Add a workspace_storage_class option to KubernetesComputeConfig, wired
through SandboxPodParams into the generated volumeClaimTemplates. When
non-empty it sets storageClassName; empty preserves the current behavior
of relying on the cluster default StorageClass.

Expose it via the OPENSHELL_K8S_WORKSPACE_STORAGE_CLASS env var on both
the standalone driver and the embedded gateway runtime defaults, and via
the server.workspaceStorageClass Helm value.

Closes NVIDIA#2442

Signed-off-by: lr90 <qiuweimin@matrixorigin.cn>
* feat(sandbox): use policy-first OCI image identity

Closes NVIDIA#2331

Preserve per-field policy omission, derive Docker and Podman fallbacks from the inspected immutable image, and resolve the final numeric identity before starting agent children.

Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>

* fix(sandbox): preserve declared process identities

Keep explicit policy values and OCI-declared names intact, defer passwd lookup until a primary GID is required, and refresh stale policy examples.

Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>

* fix(supervisor): reuse resolved OCI identity

Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>

* fix(supervisor): allow Linux pre-exec arguments

Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>

* fix(kubernetes): protect resolved sandbox identity

Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>

* fix(sandbox): prepare workspace for OCI identity

Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>

* refactor(sandbox): own only workspace root

Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>

* fix(sandbox): harden partial identity drops

Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>

* test(sandbox): scope OCI image e2e to Docker

Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>

* fix(sandbox): narrow OCI identity fallback scope

Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>

* test(podman): cover OCI identity launch

Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>

* fix(podman): exercise OCI fallback in E2E

Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>

---------

Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
…#2446)

parse_duration_to_ms was moved to commands/common.rs since the
original PR was opened, but it still split the last byte of the
input with split_at(s.len() - 1), which panics when the final
character is multi-byte UTF-8 (e.g. 'openshell logs my-sandbox
--since 5€').

Split off the last character using its UTF-8 length instead, so
invalid units surface the intended 'unknown duration unit' error.
Add regression tests in commands/common.rs.

Signed-off-by: Andrew White <andrewh@cdw.com>
Bumps [docker/login-action](https://github.com/docker/login-action) from 4.5.1 to 4.5.2.
- [Release notes](https://github.com/docker/login-action/releases)
- [Commits](docker/login-action@abd2ef4...371161b)

---
updated-dependencies:
- dependency-name: docker/login-action
  dependency-version: 4.5.2
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…ivers (NVIDIA#2153)

* refactor(compute): extract create_sandbox_record and update_sandbox_record helpers

Split apply_sandbox_update_locked into named helpers to make the two
distinct paths explicit: create_sandbox_record for first-observation events
and update_sandbox_record for subsequent driver snapshots on existing sandboxes.
The dispatcher now uses a match on the existing record rather than an
early-return guard.

No behavior change.

Signed-off-by: Evan Lezar <elezar@nvidia.com>

* refactor(compute): make sandbox readiness gateway-owned across all drivers

Introduce compute_phase_components and apply_readiness_conditions to
centralise the gateway's phase composition logic. The public SandboxPhase
is now determined by combining the backend phase reported by the driver with
supervisor session presence, independent of the driver implementation.

Remove SupervisorReadiness from the driver contract. Running containers
always report BackendReady; the gateway owns the Ready decision. Rename
the dispatcher match to three arms so that status-less events for existing
sandboxes are a documented no-op rather than a silent pass-through.

Drop backend_ready_no_session and the SupervisorNotConnected condition.
The BackendReady driver condition plus the Provisioning phase already
communicates that the backend is up but the supervisor has not connected.
The redundant condition added noise without new information.

Closes NVIDIA#1951

Signed-off-by: Evan Lezar <elezar@nvidia.com>

* fix(compute): expose disconnected supervisor readiness

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* docs(sandboxes): clarify supervisor readiness lifecycle

Signed-off-by: Evan Lezar <elezar@nvidia.com>

---------

Signed-off-by: Evan Lezar <elezar@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Co-authored-by: Drew Newberry <anewberry@nvidia.com>
Bumps [actions/stale](https://github.com/actions/stale) from 10.4.0 to 11.0.0.
- [Release notes](https://github.com/actions/stale/releases)
- [Changelog](https://github.com/actions/stale/blob/main/CHANGELOG.md)
- [Commits](actions/stale@1e223db...4391f3d)

---
updated-dependencies:
- dependency-name: actions/stale
  dependency-version: 11.0.0
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…DIA#2523)

The subprocess-based integration tests inherit the full parent
environment. If the developer has OPENSHELL_GATEWAY_INSECURE=true
set in their shell, it leaks into the spawned CLI process and causes
it to connect with .with_no_client_auth(), skipping the mTLS client
certificate. The test server requires mTLS, so it responds with
CertificateRequired and the test fails.

Strip OPENSHELL_GATEWAY_INSECURE, OPENSHELL_GATEWAY,
OPENSHELL_GATEWAY_ENDPOINT, and OPENSHELL_WORKSPACE from the
subprocess environment. The test already sets --gateway and
--gateway-endpoint explicitly via CLI args, so these env vars
should not influence the subprocess behavior.

Reported-by: Seth Jennings

Signed-off-by: Roland Huß <rhuss@redhat.com>
Signed-off-by: Evan Lezar <elezar@nvidia.com>
ForwardSpec accepts IPv6 bind addresses, but ssh_forward_arg() emitted
them unbracketed (e.g. ::1:8080:127.0.0.1:8080), which OpenSSH rejects
as a bad local forwarding specification. access_url() likewise produced
invalid URLs like http://::1:8080/.

Extract a shared bracket_ipv6_host() helper and use it in
ssh_forward_arg(), access_url() (via format_gateway_url), and
format_gateway_url() so IPv6 literals are bracketed consistently.

Fixes NVIDIA#2279

Signed-off-by: Russell Bryant <rbryant@redhat.com>
* test(vm): add composable QEMU test guests

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* docs(vm): describe test VM directory structure

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* refactor(vm): replace shell catalog functions

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* test(vm): add Fedora release guest support

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* test(vm): enable rootless Podman socket

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* feat(test-guest): add OCI-backed image caching

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* perf(test-guest): accelerate cached guest startup

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* fix(test-guest): address review feedback

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* fix(test-guest): verify OCI cache provenance

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* fix(test-guest): harden cached guest reuse

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* fix(test-guest): refresh runtime setup state

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* feat(test-guest): support E2E runner inputs

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* fix(test-guest): harden runner and OCI reuse

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* feat(test-guest): prepare Podman E2E artifacts

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* fix(test-guest): address review findings

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* revert(test-guest): remove recent Podman artifact changes

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* fix(test-guest): canonicalize scp source paths

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* refactor(test-guest): provision artifacts with Ansible

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* feat(test-guest): populate missing caches on startup

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

---------

Signed-off-by: Drew Newberry <anewberry@nvidia.com>
* feat(auth): implement RFC 0011 Phase 2 workspace authorization

Signed-off-by: Derek Carr <decarr@redhat.com>

* fix(auth): address PR review feedback on workspace authorization

- Docker e2e: add --health-port and switch readiness probe from
  `openshell status` to `curl /healthz`, fixing a false-positive
  readiness check in OIDC mode where the CLI exited 0 without
  actually contacting the gateway

- ListWorkspaces: move membership filtering from post-query N+1
  lookups into a SQL EXISTS subquery so pagination applies to the
  visible set, not the global ordering. Add generic
  list_with_membership to the persistence layer.

- Descriptor validator: reject role/scope fields on unauthenticated
  and sandbox auth modes, and allow-list workspace_role as
  user/admin and global_role as platform_admin to catch typos at
  startup

Signed-off-by: Derek Carr <decarr@redhat.com>

* fix(server): use authed request in delete telemetry test

The workspace authorization added by the Phase 2 auth changes requires
a Principal on every delete request. The delete-telemetry test was still
using a bare Request::new, so extract_principal failed before the handler
could acquire the delete gate, causing a 5-second timeout flake.

Signed-off-by: Derek Carr <decarr@redhat.com>

* fix(auth): address gator review findings for workspace authorization

- Inject unauthenticated-local-dev principal in no-auth gateway mode so
  handlers that call extract_principal() always find one.
- Cap label-selector membership query at MAX_PAGE_SIZE instead of
  u32::MAX to bound the in-memory read.
- Authorize workspace membership before resolving workspace existence in
  all sandbox RPCs to prevent workspace-name enumeration by non-members.
- Remove dead_code allow on AuthorizedWorkspace.workspace now that
  callers use the normalized name from the authz result.

Signed-off-by: Derek Carr <decarr@redhat.com>

* fix(auth): close workspace-name oracle and label-selector truncation

Swap authorize-before-resolve ordering in 27 handlers across
provider.rs, service.rs, policy.rs, and workspace.rs to prevent
CWE-203 workspace-name enumeration by non-members.

Add combined membership+label SQL query (list_with_membership_and_selector)
to both persistence backends so ListWorkspaces with label selectors no
longer silently drops results beyond the first page of membership matches.

Signed-off-by: Derek Carr <decarr@redhat.com>

* test(auth): add non-member rejection and membership+label persistence tests

Add comprehensive test coverage for workspace authorization changes:
- Non-member rejection tests across all 44 workspace-scoped handlers
  (sandbox, provider, service, policy, workspace, inference) verifying
  PERMISSION_DENIED is returned instead of NOT_FOUND to prevent
  CWE-203 workspace-name oracle
- Persistence test for list_with_membership_and_selector verifying
  SQL-level membership EXISTS + label filtering, multiple predicates,
  no-match cases, and pagination

Signed-off-by: Derek Carr <decarr@redhat.com>

* fix(auth): format merged import line in sandbox tests

Signed-off-by: Derek Carr <decarr@redhat.com>

* fix(auth): address gator re-review findings on workspace authorization

- Fix TUI unconditionally setting providers_v2_enabled after provider
  refresh; read the actual gateway setting via GetGatewayConfig at
  startup instead
- Fix SQLite json_extract with dotted label keys (e.g. example.com/env)
  by quoting the key in the JSON path
- Add authed_request wrappers to upstream OCI identity tests that were
  missing a principal after rebase
- Add test proving GetGatewayConfig is accessible without Platform Admin
- Add test for dotted/prefixed Kubernetes-style label key filtering

Signed-off-by: Derek Carr <decarr@redhat.com>

* fix(auth): address second gator re-review findings

- Loosen GetGatewayConfig from platform_admin to scope-only so workspace
  users can discover providers_v2_enabled during sandbox creation with
  inferred-provider commands; update proto descriptor, descriptor
  validation, and RFC 0011 access table
- Add validate_label_selector to handle_list_workspaces and escape
  single quotes in SQLite json_extract interpolation (CWE-89
  defense-in-depth)
- Re-fetch providers_v2_enabled after TUI gateway switch so the new
  gateway's capability is reflected
- Add e2e test for workspace user with inferred-provider command
- Add persistence test for adversarial label keys with SQL injection
  attempts
- Add handler test for invalid label selector rejection in
  ListWorkspaces

Signed-off-by: Derek Carr <decarr@redhat.com>

* fix(auth): address third gator review findings

- Cap label selector pairs at 64 (CWE-400) to bound SQLite dynamic SQL
- Add SCOPE_ONLY_METHODS allowlist for scope-without-role RPCs (CWE-863)
- Normalize ID-based data-plane handlers to return NOT_FOUND for
  unauthorized sandboxes, closing the cross-workspace oracle (CWE-203)
- Fix TUI provider profile cache lookup key mismatch for legacy
  providers with empty profile_workspace
- Add whoami to CLI skill reference command tree
- Update TUI skill doc with workspace, provider, and settings coverage
- Document scope/workspace orthogonality on GetGatewayConfig proto

Signed-off-by: Derek Carr <decarr@redhat.com>

* fix(auth): extend CWE-203 normalization to policy.rs sandbox handlers

GetSandboxConfig and GetSandboxLogs in policy.rs had the same
fetch-before-authorize pattern that leaked cross-workspace sandbox
existence. Promote fetch_and_authorize_sandbox to pub(super) and
use it from both sandbox.rs and policy.rs handlers.

Signed-off-by: Derek Carr <decarr@redhat.com>

* test(auth): update assertions for CWE-203 sandbox ID normalization

Cross-workspace sandbox access via ID-based handlers now returns
NOT_FOUND instead of PERMISSION_DENIED to prevent existence inference.
Update the unit test and OIDC e2e assertion to match.

Signed-off-by: Derek Carr <decarr@redhat.com>

* fix(auth): narrow CWE-203 error mapping and correct whoami output formats

Only remap PERMISSION_DENIED to NOT_FOUND in fetch_and_authorize_sandbox
and RevokeSshSession, letting INTERNAL and UNAUTHENTICATED propagate
as-is. Fix whoami --output format values in cli-reference.md to match
the actual CLI (table/json/yaml, not text/json).

Signed-off-by: Derek Carr <decarr@redhat.com>

* fix(ci): share network namespace with Keycloak in containerized CI

In GitHub Actions job containers, Docker port publishing lands on the
host, not inside the job container. Detect this environment and attach
Keycloak to the job container's network namespace instead, with
hardened defaults (cap-drop ALL, no-new-privileges, loopback-only
listener).

Signed-off-by: Derek Carr <decarr@redhat.com>

---------

Signed-off-by: Derek Carr <decarr@redhat.com>
* fix(gator): preserve resolved review feedback

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* fix(gator): make follow-up reviews pragmatic

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* fix(gator): enforce review convergence

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

---------

Signed-off-by: Drew Newberry <anewberry@nvidia.com>
* test(vm): add composable QEMU test guests

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* docs(vm): describe test VM directory structure

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* refactor(vm): replace shell catalog functions

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* test(vm): add Fedora release guest support

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* test(vm): enable rootless Podman socket

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* feat(test-guest): add OCI-backed image caching

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* perf(test-guest): accelerate cached guest startup

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* fix(test-guest): address review feedback

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* fix(test-guest): verify OCI cache provenance

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* fix(test-guest): harden cached guest reuse

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* fix(test-guest): refresh runtime setup state

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* feat(test-guest): support E2E runner inputs

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* fix(test-guest): harden runner and OCI reuse

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* feat(test-guest): prepare Podman E2E artifacts

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* feat(e2e): add host and test VM runner

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* test(e2e): add VM-backed Podman shutdown suite

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* fix(e2e): use renamed test guest app

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* chore(e2e): rename runner task

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* fix(e2e): make guest smoke examples portable

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* test(e2e): remove Podman shutdown suite

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* refactor(e2e): use test guest Podman setup

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* refactor(e2e): run Rust suites directly

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* refactor(e2e): simplify suite runner

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* refactor(e2e): isolate runner runtime state

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* fix(e2e): prepare Podman VM runtime

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* perf(e2e): speed up cached guest startup

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* perf(e2e): streamline guest gateway startup

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

---------

Signed-off-by: Drew Newberry <anewberry@nvidia.com>
* refactor(server): isolate gateway listener context

Signed-off-by: Evan Lezar <elezar@nvidia.com>

* refactor(server): preserve gateway listener binding

Signed-off-by: Evan Lezar <elezar@nvidia.com>

* fix(server): preserve covered listener callback scope

Signed-off-by: Evan Lezar <elezar@nvidia.com>

* refactor(server): reuse gateway listener spec

Signed-off-by: Evan Lezar <elezar@nvidia.com>

* ci(e2e): pin rootless podman packages

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

---------

Signed-off-by: Evan Lezar <elezar@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Co-authored-by: Drew Newberry <anewberry@nvidia.com>
@NaveCohenMonday
NaveCohenMonday force-pushed the 2518-acknowledge-same-hash-revisions/nc branch from 177f70a to 182ef51 Compare July 30, 2026 10:21
@NaveCohenMonday
NaveCohenMonday force-pushed the 2518-acknowledge-same-hash-revisions/nc branch from 182ef51 to c20b6b8 Compare July 30, 2026 18:34
krishicks and others added 7 commits July 30, 2026 20:29
Add an opt-in OTLP/gRPC trace exporter to the gateway. Export is enabled
by the presence of an `[openshell.gateway.otlp]` table with an endpoint;
there is no separate toggle.

Instrumented:

- Inbound request server spans, named for the RPC (`$service/$method`) or
  `{method} {path}` for plain HTTP. They continue valid W3C `traceparent`
  context when present and start a new trace otherwise. gRPC spans also
  carry `rpc.system`, `rpc.service`, `rpc.method`, and trailer-derived
  `rpc.grpc.status_code`.
- Compute driver calls (create, delete, list, get, validate, watch) as
  client spans anchored on the `ComputeDriver` contract.
- Store reads and writes as children of the current request or loop span.
- Work with no inbound request: compute driver initialization, the sandbox
  reconcile sweep, provider credential refresh tick, and driver watch events.
  Each roots one operation trace so its child work does not arrive as anonymous
  single-span traces.

This is deliberately not exhaustive. Auth, policy evaluation, and
middleware remain uninstrumented, as do store lifecycle calls (`ping`,
`close`) that a readiness poll would turn into a span per tick. The aim is
a useful trace tree at a reviewable size; coverage can grow against real
traces.

Design notes:

- The TOML table owns whether and where to export. The SDK `OTEL_*`
  variables own how; sampling, batching, and limits are not mirrored into
  gateway config.
- The OpenTelemetry layer exports spans only. Existing `tracing` events
  remain on the stdout and sandbox-log paths and are not copied into trace
  payloads.
- Telemetry never blocks the gateway. A malformed endpoint logs an error
  and disables export rather than failing startup, and buffered spans are
  drained during graceful shutdown.
- Failed spans carry error status without a separate `error.type` attribute.
  Request spans use HTTP status and gRPC response trailers; driver spans use
  the returned gRPC status; autonomous loop spans record failed results
  explicitly. Store spans exempt `UniqueViolation` and `Conflict`, because
  those errors report expected contention such as a held lease or an
  optimistic-concurrency retry.
- The compute driver is reachable only through `TracedDriver::call`, so a
  call cannot skip its span. This is the client half of a client/server pair
  and the single place to inject context if drivers move out of process.
- Tests share one process-wide subscriber and in-memory exporter because
  `tracing` caches callsite interest globally.

Inbound W3C trace context is propagated into gateway request spans. Context
is not yet injected into outbound driver calls, so a future out-of-process
driver would still need propagation at the `TracedDriver` seam.

The Helm chart is intentionally unchanged, so OTLP export cannot yet be
enabled on a chart-deployed gateway.

Refs NVIDIA#2507

Signed-off-by: Kris Hicks <khicks@nvidia.com>
…2524)

Add a published Issue Triage and Lifecycle page and align AGENTS.md,
CONTRIBUTING.md, README.md, the PR template, and the issue-handling
skills on the state:*/agent:* label model.

Signed-off-by: Kris Hicks <khicks@nvidia.com>
* fix(ci): preserve KVM access across udev restarts

Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>

* fix(ci): wait for KVM udev event

Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>

---------

Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
Extract common OpenTelemetry provider construction into openshell-otel so
OpenShell services share one OTLP/gRPC export implementation.

The shared crate owns:

- typed exporter setup errors and non-fatal provider enablement;
- endpoint trimming and URI validation before lazy exporter connection;
- fixed and environment-or-default service-name policies;
- service version and caller-supplied resource attributes;
- batch tracer-provider construction; and
- span-only tracing layers that exclude OpenTelemetry exporter callsites.

Migrate the gateway to the shared provider while retaining its configurable
service name, error marking, and tracing test collector. Add the shared crate
to the architecture inventory and document the tracing boundary.

Refs NVIDIA#2507

Signed-off-by: Kris Hicks <khicks@nvidia.com>
* feat(compute): query gateway listener requirements

Signed-off-by: Evan Lezar <elezar@nvidia.com>

* feat(compute): add Podman listener requirements

Signed-off-by: Evan Lezar <elezar@nvidia.com>

* test(docker): use default gateway bind address

Signed-off-by: Evan Lezar <elezar@nvidia.com>

* fix(gateway): avoid wildcard primary listener

Signed-off-by: Evan Lezar <elezar@nvidia.com>

* fix(podman): validate callback listener discovery

Signed-off-by: Evan Lezar <elezar@nvidia.com>

* fix(server): support split dual-stack listeners

Signed-off-by: Evan Lezar <elezar@nvidia.com>

* fix(podman): support legacy rootless listener discovery

Signed-off-by: Evan Lezar <elezar@nvidia.com>

* test(e2e): accept loopback plaintext rejection

Signed-off-by: Evan Lezar <elezar@nvidia.com>

* docs(agent): add callback listener diagnostics

Signed-off-by: Evan Lezar <elezar@nvidia.com>

* fix(server): restrict compute callback listeners

Signed-off-by: Evan Lezar <elezar@nvidia.com>

* fix(podman): validate local callback port

Signed-off-by: Evan Lezar <elezar@nvidia.com>

* test(server): clarify callback listener contract

Signed-off-by: Evan Lezar <elezar@nvidia.com>

* fix(podman): require pasta for local callbacks

Signed-off-by: Evan Lezar <elezar@nvidia.com>

* docs(gateway): document RPM listener default

Signed-off-by: Evan Lezar <elezar@nvidia.com>

* refactor(server): keep listener provenance diagnostic-only

Signed-off-by: Evan Lezar <elezar@nvidia.com>

* fix(compute): preserve callback listener isolation

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* test(e2e): remove Podman callback relay

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* fix(packaging): preserve Podman callback loopback

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* ci(e2e): run VM smoke on nested-virt runner

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* ci(e2e): gate VM smoke on usable KVM

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* ci(e2e): probe KVM through VM driver

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* ci(e2e): tolerate hosted KVM denial

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* test(server): close traced futures before assertions

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

* revert: remove tracing test stabilization

Signed-off-by: Drew Newberry <anewberry@nvidia.com>

---------

Signed-off-by: Evan Lezar <elezar@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Co-authored-by: Drew Newberry <anewberry@nvidia.com>
Give each span-assertion test its own thread-scoped in-memory exporter so
parallel tests cannot contaminate or reset captured spans. Keep a bare global
tracing registry only to preserve callsite interest, and serialize scoped
subscriber changes because tracing caches that interest process-wide.

Remove the test-only OTLP collector, polling delivery barrier,
transport-specific test, and direct opentelemetry-proto dependency. Seed the
expected persistence conflict before tracing begins so its assertion window
contains only the operation under test.

Signed-off-by: Kris Hicks <khicks@nvidia.com>
dependabot Bot and others added 4 commits July 31, 2026 22:40
Bumps [docker/login-action](https://github.com/docker/login-action) from 4.5.2 to 4.6.0.
- [Release notes](https://github.com/docker/login-action/releases)
- [Commits](docker/login-action@371161b...dbcb813)

---
updated-dependencies:
- dependency-name: docker/login-action
  dependency-version: 4.6.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* chore(build): bump sccache to 0.16.0

Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>

* fix(build): preserve default sccache lock entry

Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>

---------

Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
* test(server): stabilize watch span cancellation test

Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>

* test(server): poll span exporter at fixed interval

Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>

---------

Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>
The log prefix value passed to nft contains colons (e.g.
openshell:bypass:sandbox-cc817378:) but was not wrapped in double
quotes. Since nft concatenates argv entries and parses the result,
the bare colons cause a syntax error that silently prevents all
bypass-attempt LOG rules from installing.

Wrap log prefix values in nft-quoted strings via a new nft_quote()
helper that strips embedded double-quotes (nft quoted strings don't
support escape sequences). All four log-rule generation sites (TCP
and UDP, for both per-sandbox and sidecar rulesets) are updated.

Fixes NVIDIA#2470

Signed-off-by: Grace Smith <grasmith@redhat.com>
@NaveCohenMonday
NaveCohenMonday force-pushed the 2518-acknowledge-same-hash-revisions/nc branch from c20b6b8 to 4e6ed1b Compare August 2, 2026 08:51
SDAChess and others added 24 commits August 3, 2026 09:05
Establish the Phase 1 reference implementation proposed by RFC 0012
while retaining the existing Cargo and Mise workflows during evaluation.

- pin Bazel 9.1.1 and configure Bzlmod, rules_rs, LLVM, protobuf, and
  Rust 1.95 toolchains
- import third-party crates from Cargo metadata and propagate the
  workspace version into Bazel targets
- add library, binary, proc-macro, unit-test, and integration-test
  targets across the supported Rust workspace crates and drivers
- generate protobuf Rust sources and descriptor sets under Bazel while
  preserving Cargo-compatible generated-code imports
- annotate aws-lc-sys and zstd-sys native dependencies, build Z3 4.15.2
  from source, and generate z3-sys bindings
- make CLI and procfs test fixtures available as explicit Bazel inputs
  without relying on fixed host binary paths
- define optimized release targets for Linux x86_64 and aarch64 CLI,
  sandbox, and gateway binaries, plus macOS aarch64 artifacts
- add Bazel, buildifier, and lcov to the Nix development environment

RFC: 0012 (rfc12 branch)
Refs: NVIDIA#2491

Signed-off-by: Simon Scatton <sscatton@nvidia.com>
* build(bazel): add OpenTelemetry crate targets

Signed-off-by: Simon Scatton <sscatton@nvidia.com>

* fix(bazel): provide sandbox test policy input

Signed-off-by: Simon Scatton <sscatton@nvidia.com>

---------

Signed-off-by: Simon Scatton <sscatton@nvidia.com>
Run the constructor gateway-discovery error test only on Linux, matching the
production branch that inspects the Podman bridge gateway. macOS uses its
Podman machine callback path and correctly skips that inspection.

Signed-off-by: Kris Hicks <khicks@nvidia.com>
Continue distributed traces across the gateway-to-driver process boundary and
export VM driver spans to the same OTLP/gRPC collector. The driver reports as
the distinct openshell-driver-vm service.

Updated the gateway architecture and configuration reference with a generic
external-driver forwarding contract.

Instrumented:

- Every RemoteComputeDriver RPC injects the active W3C trace context into
  tonic metadata. Managed VM readiness and runtime initialization give startup
  capability probes stable parent operations rather than isolated root spans.
- A tonic service layer creates fixed, low-cardinality server spans for every
  ComputeDriver RPC. New handlers inherit tracing automatically; failures record
  OpenTelemetry error status and the gRPC status code.
- Background provisioning remains attached to CreateSandbox after the RPC
  returns without extending the RPC span lifetime.
- Provisioning records image preparation, bootstrap image resolution, overlay
  preparation, lifecycle configuration, pre-launch hooks, guest preparation,
  and launcher spawn as child spans.
- VM startup reconciliation roots one trace for the persisted-sandbox scan,
  with per-sandbox restore and provision operations beneath it. The root remains
  open until all spawned restore tasks finish.
- Delete cleanup records its own child operation.

Design notes:

- The gateway forwards its configured OTLP endpoint to managed external
  drivers. SDK `OTEL_*` variables continue to own sampling, batching, limits,
  headers, and transport tuning.
- The VM driver has its own tracer provider and service resource so trace
  backends preserve the service boundary.
- RPC operation names come from an explicit method mapping, keeping cardinality
  bounded without parsing the protobuf descriptor set at runtime.
- Propagation uses a remote SpanContext for spawned provisioning. This keeps
  one trace while allowing the CreateSandbox server span to finish when the
  RPC response is sent.
- Startup restoration is independent of gateway requests. It begins at the VM
  driver reconciliation span rather than attaching to an unrelated RPC.
- Existing tracing events remain on the logging path. The OpenTelemetry layer
  exports spans only and excludes the SDK exporter callsites to avoid recursive
  traces.
- Export configuration failures do not prevent the driver from serving, and
  buffered spans are drained during graceful shutdown.
- Trace fields identify drivers, sandboxes, images, lifecycle phases, and gRPC
  outcomes without recording credentials, sandbox tokens, or request query
  parameters.

Refs NVIDIA#2507

Signed-off-by: Kris Hicks <khicks@nvidia.com>
Repeated Bazel test targets with --runs_per_test at 10, 20, 50, 100, and
200 runs. The tests failed intermittently when multiple instances ran
concurrently. This indicates timing, shared tracing state, socket readiness,
or parallel-safety issues that need focused follow-up before re-enabling the
tests.

A normal non-repeated bazel test //... run passes with these tests ignored.

Disabled tests:
- sandbox_forward_foreground_fails_when_ssh_exits_before_listener_opens
- sandbox_forward_background_terminates_owned_child_when_listener_never_opens
- podman_socket_probe_accepts_successful_ping_response
- podman_socket_probe_rejects_docker_ping_response
- docker_socket_probe_accepts_successful_ping_response
- docker_socket_probe_rejects_podman_ping_response
- docker_socket_detection_returns_the_responsive_candidate
- podman_socket_detection_returns_the_responsive_candidate
- driver_watch_events_are_roots_and_store_operations_have_parents
- reconcile_sweeps_are_roots_and_operations_have_parents
- gateway_listeners_bind_ipv6_wildcard_and_ipv4_callback_on_same_port
- watch_producer_releases_request_span_when_client_disconnects
- expected_conflicts_leave_the_span_unmarked
- store_spans_record_what_they_touched_as_attributes
- store_operations_export_spans_with_parents
- refresh_worker_ticks_are_roots_and_store_operations_have_parents

Signed-off-by: Simon Scatton <sscatton@nvidia.com>
* feat(build): add system CA root mode

Allow distro builds to use native trust stores for supervisor upstream TLS while keeping bundled Mozilla roots as the default. Avoid bundled root crates in system-ca-roots builds by using native-root TLS features and z3 0.20.

Signed-off-by: Adam Miller <admiller@redhat.com>

* fix(build): keep CA root feature in telemetry-off verification

The telemetry-off task uses --no-default-features which now disables
bundled-ca-roots in addition to telemetry, triggering the compile_error
guard. Re-enable bundled-ca-roots explicitly so the task verifies only
telemetry compilation.

Signed-off-by: Scott Burdine <sburdine@nvidia.com>
Signed-off-by: politerealism <burdcat17@gmail.com>

* fix(sdk): disable oauth2 default features to prevent webpki-roots leak

The bare `oauth2 = "5"` dependency re-enabled default features
(rustls-tls → reqwest/rustls-tls → webpki-roots), defeating the
system-ca-roots feature gate. Mirror the CLI fix: disable defaults
and enable only the `reqwest` feature.

Signed-off-by: Quinn Burdine <sburdine@redhat.com>
Signed-off-by: politerealism <burdcat17@gmail.com>

* refactor(build): simplify CA root selection to single feature toggle

Replace mutually exclusive bundled-ca-roots / system-ca-roots features
with a single bundled-ca-roots toggle. Disabling it implies system roots
via rustls-native-certs, which is now a regular (non-optional) dependency.
This fixes cargo --all-features and simplifies the distro build interface
from --no-default-features --features system-ca-roots to just
--no-default-features.

Signed-off-by: Quinn Burdine <sburdine@redhat.com>
Signed-off-by: politerealism <burdcat17@gmail.com>

* feat(build): add system-ca-roots convenience alias and fix verify task

Add a system-ca-roots feature alias on openshell-sandbox that includes
all other defaults (telemetry) except bundled-ca-roots, so distro
builds can use --no-default-features --features system-ca-roots without
manually re-adding unrelated defaults. Update the verify CI task to use
the alias and scope checks to the sandbox package. Fix task description
to use "build mode" terminology instead of implying a Cargo feature.

Signed-off-by: Quinn Burdine <sburdine@redhat.com>
Signed-off-by: politerealism <burdcat17@gmail.com>

* ci: fix system CA roots step name to use build mode terminology

Signed-off-by: Quinn Burdine <sburdine@redhat.com>
Signed-off-by: politerealism <burdcat17@gmail.com>

* refactor(sandbox): reorder features to place system-ca-roots alias near default

Signed-off-by: Quinn Burdine <sburdine@redhat.com>
Signed-off-by: politerealism <burdcat17@gmail.com>

* fix(proxy): unwrap Result from build_upstream_client_config in tests

The function signature changed to return Result but the test call sites
were not updated, causing type mismatch compilation errors in CI.

Signed-off-by: Quinn Burdine <sburdine@redhat.com>
Signed-off-by: politerealism <burdcat17@gmail.com>

---------

Signed-off-by: Adam Miller <admiller@redhat.com>
Signed-off-by: Scott Burdine <sburdine@nvidia.com>
Signed-off-by: politerealism <burdcat17@gmail.com>
Signed-off-by: Quinn Burdine <sburdine@redhat.com>
Co-authored-by: Adam Miller <admiller@redhat.com>
Keep the completed instrumented handler future in an explicit pinned box and
drop it after the await. This releases the handler-side request span clone
before the disconnect test checks producer ownership, avoiding compiler- and
platform-dependent retention of an unfinished span.

Signed-off-by: Kris Hicks <khicks@nvidia.com>
…A#2600)

Document Bazel targets alongside existing mise commands with a prominent
experimental notice linking to RFC 0012. Includes Bazelisk install
instructions, .bazelignore guidance for Cargo coexistence, and a mapping
of available build and test targets.

Signed-off-by: Roland Huß <rhuss@redhat.com>
* feat(sandbox): honor Docker OCI working directories

Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>

* fix(sandbox): honor effective workspace access

Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>

* test(sandbox): cover enforced workspace denial

Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>

* docs(docker): explain effective workdir checks

Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>

* fix(sandbox): validate effective workspace writes

Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>

* fix(sandbox): reserve supervisor control roots

Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>

* refactor(sandbox): centralize control paths

Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>

* fix(sandbox): reserve OCI runtime mount roots

Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>

---------

Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
Update the docs for `sandbox create` and `exec` to dissuade use of `--env` for
secrets, and enhance the docs for `--provider` to explain what it's for.

Signed-off-by: Kris Hicks <khicks@nvidia.com>
* perf(net): set TCP_NODELAY on all tunnel and proxy TCP hops

The sandbox tunnel added ~44 ms of latency to every small
request/response because no socket in the path disabled Nagle's
algorithm, so sub-MSS writes waited on delayed ACKs at each hop.

Set TCP_NODELAY on every latency-sensitive TCP socket:

- gateway: accepted connections on the public listener (gRPC relay
  frames and WS tunnel writes)
- CLI: edge tunnel local accept + underlying WebSocket TCP stream,
  insecure TLS connector (tonic's default connector already does
  this), and service-forward accepted sockets
- supervisor: direct-tcpip connect into the sandbox netns, TCP relay
  target dials, egress proxy accepted connections, and all upstream
  CONNECT/HTTP dials (via a new connect_upstream helper)

Setting TCP_NODELAY on connect is best-effort: a failure only costs
latency, so we log and continue rather than fail the connection.

The sandbox SSH transport rides a unix domain socket and gRPC client
channels use tonic defaults (nodelay on), so no change is needed there.

Fixes NVIDIA#2219

Signed-off-by: Jim Meyer <jim@meyer4hire.com>

* refactor(net): house TCP_NODELAY helper in a shared net module

Address review feedback on the TCP_NODELAY change:

- Move the shared set-nodelay helper out of supervisor_session into a
  new crate-private `net` module in openshell-supervisor-process, so
  ssh and supervisor_session no longer reach across modules through a
  pub(crate) item.
- Make the best-effort comments at each call site terse and consistent.

No behavior change; the benchmark ladder reproduces the same numbers.

Signed-off-by: Jim Meyer <jim@meyer4hire.com>

* refactor(net): consolidate TCP_NODELAY helpers into openshell_core::net

Move the best-effort TCP_NODELAY helpers into the shared openshell_core::net
module so every crate dials and configures sockets the same way:

- Add set_tcp_nodelay_best_effort (accepted/existing streams) and
  connect_tcp_nodelay_best_effort (dial + set) with unit tests.
- Migrate all call sites in openshell-cli, openshell-server, and the
  supervisor crates to the shared helpers.
- Remove the crate-private net module from openshell-supervisor-process.
- Document socket guidance in AGENTS.md (Network Sockets).

Signed-off-by: Jim Meyer <jim@meyer4hire.com>

* perf(net): set TCP_NODELAY on exec bridge and metadata server

The gateway-side single-use SSH-over-relay loopback bridge and the
sandbox IMDS metadata server were missed latency-sensitive TCP hops.
Set TCP_NODELAY on the accepted client connection and both russh client
dials of the exec bridge — interactive keystrokes and line-buffered PTY
output are the most tinygram-heavy traffic in the system — and on the
metadata server's accepted connections.

Also log unrecognized MaybeTlsStream variants in the edge tunnel so a
future TLS-backend change surfaces a silent TCP_NODELAY miss instead of
skipping it quietly.

Signed-off-by: Jim Meyer <jim@meyer4hire.com>

* perf(net): set TCP_NODELAY on openshell-sdk socket paths

The openshell-sdk crate landed on main with its own copies of the CLI's
hand-rolled sockets, which the CLI and TUI are meant to consume. Give
them the same treatment as the CLI equivalents:

- edge_tunnel: the accepted local tunnel connection and the WebSocket's
  underlying TCP socket (plain and rustls variants).
- transport: the dial in InsecureTlsConnector, tonic's custom-connector
  path.

Only these hand-rolled sockets need it. Tonic's own connector defaults
tcp_nodelay to true and applies it itself, so plain Endpoint::connect
callers were already covered.

Signed-off-by: Jim Meyer <jim@meyer4hire.com>

---------

Signed-off-by: Jim Meyer <jim@meyer4hire.com>
* feat(credentials): add provider credential storage drivers

Signed-off-by: Taylor Mutch <taylormutch@gmail.com>

* fix(credentials): harden credential update handling

Signed-off-by: Taylor Mutch <taylormutch@gmail.com>

* fix(credentials): harden credential driver security, correctness, and performance

Address review findings from the credential storage drivers PR:

- Route additional_credentials through the driver on refresh to prevent
  silent data loss for multi-credential providers (e.g. AWS STS)
- Clean up stored credential handles on CAS failure during refresh to
  prevent orphaned secrets in external backends
- Enforce namespace validation in the Kubernetes Secrets driver to
  prevent cross-namespace credential access when allow_reference_namespace
  is not enabled
- Cache Vault Kubernetes auth tokens with 80% TTL to avoid re-authenticating
  on every credential operation
- Parallelize resolve_credentials in all three drivers using try_join_all
  for faster sandbox startup
- Add existingSecret support for the KEK Secret to fix helm template/GitOps
  workflows where lookup returns empty and regenerates the key
- Document RBAC blast radius for the Kubernetes Secrets credential driver
  and recommend a dedicated namespace

Signed-off-by: Varsha Prasad <varshaprasad96@gmail.com>
Signed-off-by: Varsha Prasad Narsing <varshaprasad96@gmail.com>

* fix(credentials): add optimistic concurrency, fix thundering herd, parallelize operations

Use resourceVersion optimistic concurrency with retry loop for K8s
Secret ownership checks to prevent TOCTOU races. Switch Vault token
cache from RwLock to Mutex with double-check pattern to prevent
thundering herd on cache miss. Parallelize credential store and delete
operations across independent keys using try_join_all.

Signed-off-by: Varsha Prasad <varshaprasad96@gmail.com>
Signed-off-by: Varsha Prasad Narsing <varshaprasad96@gmail.com>

* fix(credentials): handle partial failures, add delete retry, consolidate cleanup

Replace try_join_all with join_all in credential store/delete operations
to handle partial failures — successfully-stored handles are cleaned up
when another key fails. Add retry loop with conflict detection to
db-credstore delete_credential, matching the K8s driver pattern.
Consolidate 4 manual cleanup_pre_stored_provider_credentials call sites
into a single error handler using an async block. Remove inconsistent
.trim() from db-credstore validate_handle_owner.

Signed-off-by: Varsha Prasad <varshaprasad96@gmail.com>
Signed-off-by: Varsha Prasad Narsing <varshaprasad96@gmail.com>

* fix(credentials): fix retry loop guard and remove unprotected validation

Remove attempt-count guard from 409/Aborted match arms in retry loops
so the post-loop Status::aborted error is reachable after exhausting
retries. Previously, last-attempt conflicts fell through to the
catch-all error arm, producing misleading Status::unavailable errors.

Remove duplicate validation calls that ran after
prepare_provider_credential_update but outside the cleanup-protected
async block, which would leak pre-stored handles on failure.

Signed-off-by: Varsha Prasad <varshaprasad96@gmail.com>
Signed-off-by: Varsha Prasad Narsing <varshaprasad96@gmail.com>

* fix(credentials): add workspace/provider UUID to credential backend paths

Include workspace and provider ID in credential backend object paths to ensure
cross-workspace uniqueness and prevent credential collision (GATOR-1806c9be-01).

- Updated credential driver proto to include workspace and provider_id fields
- Modified Vault driver to include workspace/provider_id in managed_secret_path
- Modified Kubernetes Secrets driver to include workspace/provider_id in
  credential_owner_id and managed_secret_name
- Updated all credential runtime calls to pass workspace/provider_id
- Updated tests to use the new signatures

This prevents two workspaces sharing the same external credential store from
colliding on provider names, which was a critical security issue (CWE-639).

* fix(credentials): preserve provider-level expiration for handle-backed credentials

Compute effective expiration from both provider and driver values using the
earliest non-zero timestamp and skip expired values before insertion
(GATOR-1806c9be-02).

- Modified resolve_provider_handles to check provider credential_expires_at_ms
- Skip expired credentials during resolution instead of returning them
- Use effective expiration (min of provider and driver) in resolution results
- Fix inference.rs to preserve earliest expiration when merging

This ensures handle-backed credentials respect the same expiration semantics
as inline credentials.

* fix(credentials): stage refresh changes under new handles before validation

Stage credential replacements under new immutable handles instead of reusing
existing handles to prevent overwriting committed values before validation/CAS
(GATOR-1806c9be-03).

- Stage credentials with empty existing_handles map to force new handle creation
- Validate and CAS before the new values are committed to backend storage
- Delete old handles only after successful CAS
- On CAS failure, delete only the newly staged handles
- This prevents CWE-362/CWE-367 race conditions where failed refreshes could
  still modify or delete the active credential

The fix ensures that a rejected refresh cannot modify the backend object still
referenced by the committed provider record.

* fix(credentials): add timeouts to credential driver RPCs

Apply configured timeouts to both startup capability negotiation and runtime
RPCs to prevent indefinite hangs (GATOR-1806c9be-05).

- Add DEFAULT_CREDENTIAL_DRIVER_RPC_TIMEOUT_SECS constant (30s)
- Apply timeout to GetCapabilities during startup connection
- Apply timeout to all runtime RPCs (store, delete, resolve)
- Use tokio::time::timeout to bound the entire GetCapabilities operation
  during startup, not just the socket connection
- Return contextual deadline errors on timeout

This prevents a faulty or overloaded driver from hanging gateway operations
indefinitely.

* fix(credentials): fix test to use consistent workspace/provider identity

The Kubernetes auth Vault resolve test was constructing a managed path with
test-workspace/test-provider-id but sending default/prov-123 in the request,
causing validation to reject the request (GATOR-18e32351-01).

- Update test to use test-workspace and test-provider-id in the request to
  match the logical_path construction
- This ensures the test exercises the intended code path and validates
  Kubernetes auth resolution properly

The test now passes and correctly validates identity enforcement.

* fix(credentials): use unique staging ID for refresh to avoid overwrites

Stage refresh replacements under genuinely distinct immutable handles using
a unique staging ID to prevent overwriting committed values (GATOR-1806c9be-03).

- Generate a unique staging ID using UUID for each refresh operation
- Use this staging ID when storing credentials instead of the real provider ID
- Pass the same staging ID during cleanup on failure to delete only staged objects
- This ensures deterministic paths (Vault) and object names (K8s) don't collide
  with the committed provider's credentials

The fix prevents failed refreshes from silently replacing active credentials
or breaking providers by deleting still-referenced backend objects.

* fix(credentials): wrap credential driver RPCs in local timeouts

Add local tokio::time::timeout wrappers around credential driver RPCs to
bound non-compliant or stalled UDS peers (GATOR-1806c9be-05).

- Wrap StoreCredential, DeleteCredential, and ResolveCredentials in local timeouts
- Return contextual deadline_exceeded errors when timeouts occur
- Keep existing gRPC timeout metadata for compliant implementations
- GetCapabilities during startup was already wrapped in previous commit

This ensures a faulty local driver cannot hang gateway operations indefinitely,
even if it accepts the connection but never responds to the RPC.

* fix(credentials): preserve ownership for staged refreshes

Signed-off-by: Seth Jennings <sjenning@redhat.com>

* fix(credentials): bound startup capability probe

Signed-off-by: Seth Jennings <sjenning@redhat.com>

* test(provider): authenticate credential handler requests

Signed-off-by: Seth Jennings <sjenning@redhat.com>

* fix(ci): grant actions read to credential driver e2e

Signed-off-by: Seth Jennings <sjenning@redhat.com>

---------

Signed-off-by: Taylor Mutch <taylormutch@gmail.com>
Signed-off-by: Varsha Prasad <varshaprasad96@gmail.com>
Signed-off-by: Varsha Prasad Narsing <varshaprasad96@gmail.com>
Signed-off-by: Seth Jennings <sjenning@redhat.com>
Co-authored-by: Taylor Mutch <taylormutch@gmail.com>
Co-authored-by: Varsha Prasad Narsing <varshaprasad96@gmail.com>
Signed-off-by: Kris Hicks <khicks@nvidia.com>
* docs(readme): add theme-aware banner

Signed-off-by: Johnny Greco <jogreco@nvidia.com>

* docs(readme): exclude preview screenshot from tree

Signed-off-by: Johnny Greco <jogreco@nvidia.com>

---------

Signed-off-by: Johnny Greco <jogreco@nvidia.com>
…VIDIA#2271)

* feat(sdk/go): add Go SDK foundation, types, and sandbox client (A)

Add the Go SDK module with the full API contract and a working sandbox
client as the first vertical slice. All other resource clients are present
as stubs returning Unimplemented errors, to be replaced with real
implementations in subsequent PRs.

Contents:
- Module setup (go.mod, Makefile, mise.toml)
- All domain types (types/ package)
- Full ClientInterface with all sub-client accessors
- Shared infrastructure (errors, auth, gRPC connection, logging)
- Sandbox client with converter and tests (fully functional)
- Stub clients for remaining resources (exec, file, health, provider,
  profile, config, refresh, policy, service, ssh, tcp)

Part of the Go SDK decomposition plan (NVIDIA#2270).
Implements NVIDIA#2044.

* fix(sdk/go): address review feedback on PR NVIDIA#2271

- Make scheme parsing drive transport selection: http:// uses plaintext
  gRPC, https:// or no scheme uses TLS. Add regression tests.
- Add Resources and DriverConfig fields to SandboxTemplate and update
  both converter directions (SandboxFromProto/SandboxSpecToProto).
- Regenerate proto bindings from current canonical proto sources to
  eliminate drift (SigV4/MCP fields, params matchers, reserved fields).
- Run gofmt/goimports on all handwritten Go files.

Signed-off-by: Roland Huß <rhuss@redhat.com>

* fix(sdk/go): address principal engineer review findings

- Remove dead boolCount function that would fail golangci-lint (#1)
- Emit EventAdded for the first watch event instead of EventModified,
  matching k8s watch semantics (NVIDIA#7)
- Add mutex locking to all mock server methods that access the shared
  sandboxes map, fixing latent race conditions (NVIDIA#12)
- Skip HealthCheck integration test that calls an unimplemented stub (NVIDIA#13)
- Scope doc.go examples: mark sections for sub-clients not yet available
  in this PR with "available in a future release" (NVIDIA#4)
- Document Config.Timeout/RetryPolicy/Logger and WatchOptions fields
  as reserved for future use (#2, NVIDIA#6)

Signed-off-by: Roland Huß <rhuss@redhat.com>

* refactor(sdk/go): migrate mise config to centralized task include

Move Go SDK mise configuration from standalone sdk/go/mise.toml into
the project's centralized pattern:

- Add Go tools (go, golangci-lint, protoc-gen-go, protoc-gen-go-grpc)
  to root mise.toml [tools] section
- Create tasks/go.toml with all SDK tasks using go: namespace prefix
  and dir=sdk/go for working directory
- Update sdk/go/Makefile to reference namespaced task names
- Update proto:sync default path for monorepo layout

Addresses review feedback from drew on PR NVIDIA#2271 regarding mise
convention alignment.

Signed-off-by: Roland Huß <rhuss@redhat.com>

* refactor(sdk/go): remove UPSTREAM_VERSION standalone repo artifact

Remove sdk/go/proto/UPSTREAM_VERSION file and its exclusion from
proto:check. This was a leftover from the standalone repo prototype.
In a monorepo, proto drift is detectable via git diff between
sdk/go/proto/ and proto/ directly.

Signed-off-by: Roland Huß <rhuss@redhat.com>

* refactor(sdk/go): switch proto generation from protoc to buf

Replace raw protoc invocations with buf for Go SDK proto code generation,
aligning with the TS SDK approach (PR NVIDIA#2122).
- Add repo-level buf.yaml declaring proto/ as the buf module with lint
  and breaking change detection config
- Add sdk/go/buf.gen.yaml configuring buf to generate Go code directly
  from root proto/ (no more vendored .proto copies)
- Delete vendored .proto source files from sdk/go/proto/
- Rewrite go:proto:gen and go:proto:check mise tasks to use buf
- Remove go:proto:sync and go:proto:clean tasks (no longer needed)
- Add proto target to sdk/go/Makefile
- Add buf 1.72.0 to root mise.toml tool dependencies
- Include options.proto in generation (was stripped from vendored copies)
- Regenerate all .pb.go files via the new buf pipeline
Signed-off-by: Roland Huß <rhuss@redhat.com>

* test(sdk/go): add proto-converter field coverage detection

Use protobuf reflection to enumerate all fields on key proto messages
(SandboxSpec, SandboxTemplate, SandboxStatus, SandboxCondition,
SandboxPolicy) and compare against explicit handled/skipped sets in the
converter tests.

Unhandled fields produce warnings (t.Log), not failures, so proto
contributors are not forced to fix SDK converters in the same PR. Stale
entries in the handled set (removed proto fields) do fail, since they
indicate the converter references something that no longer exists.

A follow-up CI workflow will create GitHub issues when converter drift
lands on main.

Signed-off-by: Roland Huß <rhuss@redhat.com>

* fix(sdk/go): bump Go to 1.26 and fix errcheck lint violations

The upstream go.mod now has `toolchain go1.26.4`, which requires Go 1.26
to build golangci-lint. Bump the mise.toml Go version from 1.25 to 1.26
and wrap deferred Close() calls in test helpers to satisfy errcheck.

Assisted-By: 🤖 Claude Code

* feat(sdk/go): add ObjectMeta fields (annotations, workspace, deletion_timestamp)

Add three new proto ObjectMeta fields to Sandbox and Provider domain
types: Annotations (map), Workspace (string), and DeletionTimestamp
(*time.Time). Update converters in both directions, deep-copy maps at
the proto/SDK boundary, and add TimeFromMillisPtr/MillisFromTimePtr
helper functions.

Assisted-By: 🤖 Claude Code

* chore(sdk/go): regenerate proto bindings after rebase

Pick up workspace fields from upstream PR NVIDIA#2445 (Wire authorization
into workspace model). All request messages now include workspace
parameter in the generated Go bindings.

Assisted-By: 🤖 Claude Code

* feat(sdk/go): add workspace scoping to all RPC interfaces

Add workspace parameter to every sandbox-scoped RPC method across all
interfaces (Sandbox, Exec, File, Service, SSH, TCP, Config, Policy,
Provider, Profile, Refresh). The workspace string is passed as the
second parameter after ctx, following the convention workspace then
resource-name.

Key changes:
- SandboxInterface: all 10 methods gain workspace parameter
- sandbox_client.go: passes Workspace field in every proto request
- ListOptions: add AllWorkspaces field for cross-workspace queries
- All stub interfaces updated to match new signatures
- All sandbox client tests updated with "default" workspace

Assisted-By: 🤖 Claude Code

* chore(sdk/go): remove coverage.out from tracking

Assisted-By: 🤖 Claude Code

* fix(sdk/go): address review feedback from mrunalp

- Add RefreshStrategyAWSStsAssumeRole to match proto enum value 6,
  fulfilling the "all domain types upfront" contract
- Wrap context.DeadlineExceeded and context.Canceled in StatusError
  so IsDeadlineExceeded() and IsCancelled() helpers work correctly
- Return error from mapToStruct/SandboxSpecToProto instead of silently
  discarding structpb.NewStruct failures on invalid template maps

Signed-off-by: Roland Huss <rhuss@redhat.com>

* fix(sdk/go): address remaining review items

- Wire go:ci into root ci task so SDK is tested in repository CI
- Fix gofmt formatting on converter files
- Add goimports to mise.toml tools
- Add coverage.out to .gitignore
- Add Go SDK section to AGENTS.md and CONTRIBUTING.md
- Add regression tests for context-error wrapping (IsDeadlineExceeded,
  IsCancelled) and invalid template map rejection
- Remove panic from SandboxToProto, return error instead

Signed-off-by: Roland Huss <rhuss@redhat.com>

* fix(sdk/go): pin goimports version and update lockfile

Pin goimports to 0.48.0 instead of "latest" and regenerate mise.lock
to include the new entry.

Signed-off-by: Roland Huss <rhuss@redhat.com>

* fix(sdk/go): TLS.Insecure means skip-verify, not plaintext

Align TLS.Insecure semantics with the Rust SDK: Insecure: true now
uses TLS with InsecureSkipVerify (skip cert verification) instead of
switching to plaintext. Only the http:// scheme triggers plaintext.

This fixes token auth against dev/k3d gateways: StaticToken and
RefreshableToken require transport security, which real TLS (even
with InsecureSkipVerify) satisfies, but plaintext does not.

For http:// + token auth (dev gateways without TLS), wrap the auth
provider to override RequireTransportSecurity, matching the Rust
SDK's behavior where http:// accepts any auth mode.

Transport decision table (matches Rust SDK crates/openshell-sdk):
  http://  + any TLS config  -> plaintext (TLS config ignored)
  https:// + Insecure: true  -> TLS, skip cert verify
  https:// + Insecure: false -> TLS, full verification
  no scheme                  -> same as https://

Signed-off-by: Roland Huss <rhuss@redhat.com>

* feat(sdk/go): add missing policy proto fields

Add 6 previously silently dropped fields to the network policy types
and converters, preventing security-relevant data loss on round-trip:

NetworkEndpoint fields 19-23:
- CredentialSigning: SigV4 re-signing mode
- SigningService: AWS service name for SigV4
- SigningRegion: AWS region override for SigV4
- JsonRpcMaxBodyBytes: JSON-RPC body inspection limit
- Mcp: MCP-specific policy options (new McpOptions type)

L7Allow and L7DenyRule field 9:
- Params: MCP params matcher map for tools/call filtering

New type McpOptions with StrictToolNames and AllowAllKnownMcpMethods
optional booleans matching the proto definitions.

Signed-off-by: Roland Huss <rhuss@redhat.com>

* fix(sdk/go): enforce coverage test and extend to policy messages

Change coverage_test.go from t.Logf (silent) to t.Errorf so that
unhandled proto fields fail the test immediately. Add coverage tests
for NetworkEndpoint (23 fields), L7Allow (8 fields), L7DenyRule
(8 fields), and McpOptions (2 fields).

Any new proto field that is not in the handled set or explicitly
skipped now breaks the build, closing the silent-drift gap.

Signed-off-by: Roland Huss <rhuss@redhat.com>

* ci(sdk/go): add Go SDK job to branch-checks workflow

Add a Go SDK job to branch-checks.yml that runs mise run go:ci
(lint, build, test, proto-check, docs-check) on every PR. This
ensures the SDK is tested in CI, not just locally.

Signed-off-by: Roland Huss <rhuss@redhat.com>

* fix(sdk/go): address should-fix review items

NVIDIA#6 Fix broken godoc examples: add workspace parameter to all method
   calls in doc.go that were broken after workspace scoping.

NVIDIA#7 Add Err field to Event[T]: Watch error events now carry the
   underlying error instead of discarding it.

NVIDIA#8 Separate Unauthenticated from PermissionDenied: add
   ErrorUnauthenticated code and IsUnauthenticated() helper. gRPC
   Unauthenticated (401) now maps to its own code instead of
   collapsing into PermissionDenied (403).

NVIDIA#9 Add Unwrap to StatusError: replace dead Details field with Cause
   error field. StatusError.Unwrap() returns Cause, enabling
   errors.Is/As unwrapping. FromGRPCError and contextError both
   populate Cause.

Signed-off-by: Roland Huss <rhuss@redhat.com>

* ci(sdk/go): add go:format:check to CI pipeline

Add gofmt format verification to go:ci. Catches unformatted Go files
before they reach the PR. Fix formatting on coverage_test.go.

Signed-off-by: Roland Huss <rhuss@redhat.com>

* chore(sdk/go): remove Makefile in favor of mise tasks

All build, lint, test, and proto-gen tasks are already defined in
tasks/go.toml and invoked via mise. The Makefile was a leftover
that duplicated this and raised questions in review.

Signed-off-by: Roland Huß <rhuss@redhat.com>

* feat(sdk/go): sync proto bindings and add credential handle support

Regenerate Go proto bindings after rebase to pick up new
CredentialHandle message and Provider.credential_handles and
profile_workspace fields from upstream. Add domain types, converter
support, and proto field coverage tests for Provider and
CredentialHandle.

Signed-off-by: Roland Huß <rhuss@redhat.com>

* fix(sdk/go): reject plaintext auth leak and fix watch error handling

Reject http:// addresses when the auth provider requires transport
security instead of silently stripping the requirement. Remove the
insecureAuthWrapper that overrode RequireTransportSecurity.

Fix watch stream error handling: use blocking send for terminal
errors so they are never silently dropped when the channel is full,
and wrap mid-stream errors with converter.FromGRPCError so SDK error
helpers like IsUnavailable work on watch Event.Err.

Signed-off-by: Roland Huß <rhuss@redhat.com>

* fix(sdk/go): address review findings from multi-agent code review

- WaitReady now detects SandboxDeleting phase and returns immediately
  instead of polling indefinitely
- Watch goroutine defers streamCancel() to prevent context leaks
- Fix StopOnTerminal=false test to keep stream open (was wrong-reason
  pass due to stream ending, not StopOnTerminal logic)
- Add EventDeleted test covering the Deleting phase branch
- Add provider converter unit tests for CredentialHandle round-trip,
  nil handling, and empty maps

Signed-off-by: Roland Huß <rhuss@redhat.com>

---------

Signed-off-by: Roland Huß <rhuss@redhat.com>
Signed-off-by: Roland Huss <rhuss@redhat.com>
* docs(rfc): propose sandbox proxy egress adapter model

Signed-off-by: John Myers <johntmyers@users.noreply.github.com>

* docs(rfc): propose sandbox proxy egress adapter model

Signed-off-by: John Myers <johntmyers@users.noreply.github.com>

* docs(rfc): update sandbox proxy adapter proposal

Signed-off-by: John Myers <johntmyers@users.noreply.github.com>

* docs(rfc): account for supervisor middleware

Signed-off-by: John Myers <johntmyers@users.noreply.github.com>

* docs(rfc): include json-rpc and mcp l7 protocols

Signed-off-by: John Myers <johntmyers@users.noreply.github.com>

* docs(rfc): make process identity optional

Signed-off-by: John Myers <johntmyers@users.noreply.github.com>

* docs(rfc): clarify relay flow diagram

Signed-off-by: John Myers <johntmyers@users.noreply.github.com>

* docs(rfc): address proxy adapter review feedback

Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com>

* docs(rfc): enforce policy after middleware mutation

Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com>

* docs(rfc): define synthetic DNS correlation

Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com>

---------

Signed-off-by: John Myers <johntmyers@users.noreply.github.com>
Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com>
Co-authored-by: John Myers <johntmyers@users.noreply.github.com>
…gRPC channel (NVIDIA#2608)

Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>
Signed-off-by: Adrien Langou <alangou@nvidia.com>
* build(bazel): add credential driver targets

Signed-off-by: Simon Scatton <sscatton@nvidia.com>

* fix(bazel): sync default CA root features

Signed-off-by: Simon Scatton <sscatton@nvidia.com>

---------

Signed-off-by: Simon Scatton <sscatton@nvidia.com>
* build(bazel): stage VM runtime bundle

Signed-off-by: Simon Scatton <sscatton@nvidia.com>

* build(bazel): add VM driver targets

Signed-off-by: Simon Scatton <sscatton@nvidia.com>

---------

Signed-off-by: Simon Scatton <sscatton@nvidia.com>
* fix(e2e): separate Podman Machine loopback listeners

Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>

* test(e2e): remove shallow harness checks

Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>

* refactor(e2e): trim Podman listener workaround

Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>

* fix(e2e): bypass proxies for Podman health probe

Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>

---------

Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
Signed-off-by: Nave Cohen <nave@monday.com>
Signed-off-by: Nave Cohen <nave@monday.com>
@NaveCohenMonday
NaveCohenMonday force-pushed the 2518-acknowledge-same-hash-revisions/nc branch from 4e6ed1b to 25202b2 Compare August 9, 2026 10:11
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.

[Bug] Supervisor does not acknowledge newer sandbox policy revisions with an unchanged policy hash