Python runtime requires /dev/urandom access during initialization
to seed the hash randomizer. The _default_policy() in provider tests
was missing this path, causing exec_python tests to fail with:
'Fatal Python error: _Py_HashRandomization_Init: failed to get
random numbers to initialize Python'.
Add /dev/urandom to read_only paths to match the policy used in
test_sandbox_policy.py, allowing Python to initialize successfully.
Signed-off-by: Derek Carr <decarr@redhat.com>
Previously `openshell sandbox upload <local-dir> <remote-dir>` flattened
the local directory's contents into the remote destination, dropping the
source directory's basename. This diverged from the standard `scp -r` /
`cp -r` semantics that operators expect from any *nix-style transfer
tool, and could silently shadow files at the destination.
Now an upload of `foo` to `/sandbox/dest/` lands at `/sandbox/dest/foo/`,
matching `scp -r foo remote:/sandbox/dest/` and `cp -r foo /sandbox/dest/`.
Paths without a meaningful basename (`.`, `/`) preserve the legacy flat
extraction.
Closes#885
* feat(server): add Prometheus metrics infrastructure and gRPC/HTTP request metrics
Add metrics exposition via a dedicated server port (--metrics-port,
default disabled) following the same optional-listener pattern as the
health port. The metrics crate facade records counters and histograms
in the MultiplexedService hot path, and a PrometheusHandle renders them
at GET /metrics on the dedicated port.
Metrics added:
- openshell_server_grpc_requests_total (counter: method, code)
- openshell_server_grpc_request_duration_seconds (histogram: method, code)
- openshell_server_http_requests_total (counter: path, status)
- openshell_server_http_request_duration_seconds (histogram: path, status)
* feat(helm): add metrics port to openshell-server helm chart
Expose the Prometheus metrics endpoint in the helm chart by adding
service.metricsPort (default 9090) to values, the --metrics-port arg
and container port to the statefulset, and a metrics service port.
Set metricsPort to 0 to disable.
Most contributors cannot assign PRs or issues to themselves due to
repository settings. Remove the --assignee flag from skill templates
to prevent gh CLI errors.
Clears four High-severity container findings in the CI image, which
installs helm via mise.toml:
- CVE-2026-35204 / GHSA-vmx8-mqv2-9gmg (plugin path traversal)
- CVE-2026-35205 / GHSA-q5jf-9vfq-h4h7 (plugin .prov verification fails open)
Both are fixed in helm 4.1.4.
Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com>
Ubuntu Noble's git links against libcurl-gnutls, which does not read
SSL_CERT_FILE. Without GIT_SSL_CAINFO, `git clone` over HTTPS fails
inside every community sandbox image with "server certificate
verification failed".
Add GIT_SSL_CAINFO to tls_env_vars pointing at the same combined CA
bundle already used by CURL_CA_BUNDLE / REQUESTS_CA_BUNDLE.
Fixes#790FixesNVIDIA/NemoClaw#2270FixesNVIDIA/NemoClaw#1828
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Closes#879
Resolve policy hostnames against the sandbox's /etc/hosts before DNS so Kubernetes hostAliases are visible to the proxy while keeping the existing allowed_ips SSRF enforcement semantics intact.
Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com>
Closes#905
Skip proxy baseline read-write enrichment when a path is already explicitly configured as read-only, add regression coverage for the proto and local policy code paths, and document the precedence rule in the sandbox architecture docs.
Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com>
the transfer of very large sandbox images to containerd can
timeout depending on the size of the image and the speed of
the local host.
Made-with: Cursor
The 30s read_timeout on the shared kube client was killing the
long-lived watch streams during idle periods, causing a reconnect
cycle every 30 seconds. Use a separate client with no read_timeout
for watch_sandboxes so the streams stay open indefinitely.
Move /health, /healthz, and /readyz to a dedicated plaintext HTTP port
(default 8081) so Kubernetes probes work without mTLS client certificates.
- Add health_bind_address to Config with --health-port CLI arg
- Spawn standalone axum::serve for health_router on the health port
- Remove health routes from the main multiplexed HTTP router
- Update Helm statefulset probes from tcpSocket to httpGet on health port
- Fix cluster-healthcheck.sh to open/close TCP without sending data,
avoiding InvalidContentType TLS errors in the gateway log
Wrap gRPC and HTTP services with TraceLayer to log method, path,
status, and latency for every request. Health/readiness probe
endpoints are logged at debug level to reduce noise.
* fix(sandbox): canonicalize HTTP request-targets before L7 policy evaluation
The L7 REST proxy evaluated OPA path rules against the raw request-target
and forwarded the raw header block byte-for-byte to the upstream. The
upstream normalized `..`, `%2e%2e`, and `//` before dispatching, so a
compromised agent inside the sandbox could bypass any path-based allow
rule by prefixing the blocked path with an allowed one (e.g.
`GET /public/../secret` matches `/public/**` at the proxy but the
upstream serves `/secret`). The inference-routing detection had the same
normalization gap via `normalize_inference_path`, which only stripped
scheme+authority and preserved dot-segments verbatim.
- Add `crates/openshell-sandbox/src/l7/path.rs` with
`canonicalize_request_target`. Percent-decodes (with uppercase hex
re-emission), resolves `.` / `..` segments per RFC 3986 5.2.4,
collapses doubled slashes, strips trailing `;params`, rejects
fragments / raw non-ASCII / control bytes / encoded slashes (unless
explicitly enabled per-endpoint), and returns a `rewritten` flag so
callers know when to rewrite the outbound request line.
- Wire the canonicalizer into `rest.rs::parse_http_request`. The
canonical path is the `target` on `L7Request` that OPA sees, and
when the canonical form differs from the raw input the request line
is rebuilt in `raw_header` so the upstream dispatches on the exact
bytes the policy evaluated.
- Canonicalize the forward (plain HTTP proxy) path at
`proxy.rs`'s second L7 evaluation site. A non-canonical request-target
is rejected with 400 Bad Request and an OCSF `NetworkActivity` Fail
event rather than silently passed through.
- Replace the old `normalize_inference_path` body with a call to the
canonicalizer. On canonicalization error the raw path is returned
(so inference detection simply misses and the normal forward path
handles and rejects).
- Document the invariant in `sandbox-policy.rego`: `input.request.path`
is pre-canonicalized so rules must not attempt defensive matching
against `..` or `%2e%2e`.
- Architecture doc (`architecture/sandbox.md`) lists the new module.
- 24 new unit tests cover dot segments, percent-encoded dot segments,
double-slash collapse, encoded-slash reject/opt-in, null/control byte
rejection, legitimate percent-encoded bytes round-tripping, mixed-case
percent normalization, fragment rejection, absolute-form stripping,
empty / length-bounded / non-ASCII handling, and regression payloads
for the specific bypasses above.
Tracks OS-99.
* fix(sandbox): close forward-proxy parser-differential and wire allow_encoded_slash
Addresses two review findings on the L7 path canonicalization change.
1. `handle_forward_proxy` previously evaluated OPA on the canonical path
but wrote the raw path to the upstream via `rewrite_forward_request`,
leaving the parser-differential open for plain-HTTP forward-proxy
traffic even though it was closed for the L7 tunnel path. `path` is
now reassigned to the canonical form inside the L7 block before the
outbound write, so the bytes dispatched to the upstream match what
OPA evaluated. Covered by a new regression test against
`rewrite_forward_request`.
2. `allow_encoded_slash` is now wired through the YAML policy schema,
the proto `NetworkEndpoint` message, the Rego endpoint config, and
the `L7EndpointConfig` → `RestProvider` canonicalization options.
Operators can opt in per-endpoint for upstreams like GitLab that
embed `%2F` in namespaced resource paths; the default remains
strict. The `RestProvider` gains a constructor
`RestProvider::with_options` so `relay_rest` constructs a provider
with per-endpoint options while `relay_passthrough_with_credentials`
retains strict defaults.
Integration tests:
- `parse_http_request_canonicalizes_target_and_rewrites_raw_header`
drives a non-canonical request through the parser and asserts both
`L7Request.target` (what OPA evaluates) and `raw_header` (what the
upstream receives) are canonical.
- `parse_http_request_canonicalization_preserves_query_string`,
`parse_http_request_leaves_canonical_input_byte_for_byte`,
`parse_http_request_rejects_traversal_above_root`,
`parse_http_request_preserves_http_10_version_on_rewrite`.
- `parse_http_request_accepts_encoded_slash_when_endpoint_opts_in` /
`_rejects_encoded_slash_by_default` verify the
`allow_encoded_slash` gate.
- `test_rewrite_forward_request_uses_canonical_path_on_the_wire`
asserts the fix to the forward-proxy flow.
- `parse_l7_config_allow_encoded_slash_{defaults_false,opt_in}` verify
the YAML/Rego wiring.
* feat(cli): Allow printing only the policy when retrieving sandbox info
* feat(cli): Indicate policy source and revision when retrieving sandboxes
* cargo fmt
The CLI and TUI build an SSH ProxyCommand string by interpolating fields
from CreateSshSessionResponse into a shell command. Three fields -
sandbox_id, token, and the assembled gateway_url (composed from
gateway_scheme, gateway_host, gateway_port, connect_path) - were not
passed through shell_escape, even though exe_command and gateway_name
were. The resulting string is handed to `ssh -o ProxyCommand=...` which
OpenSSH routes through `/bin/sh -c`, so a hostile or compromised gateway
could inject shell metacharacters and execute arbitrary commands under
the developer's workstation account.
- Add build_proxy_command in openshell-core::forward that wraps every
interpolated value in shell_escape. Use it at all four ProxyCommand
sites: openshell-cli/src/ssh.rs (ssh_session_config) and the three
TUI sites in openshell-tui/src/lib.rs (shell connect, sandbox exec,
port-forward reconciliation).
- Add validate_ssh_session_response in openshell-core::forward, called
at each site immediately after response.into_inner(). Enforces a
conservative charset per field (sandbox_id, token, gateway_host,
gateway_scheme, gateway_port, connect_path, host_key_fingerprint) so
malformed responses fail loudly at the gRPC trust boundary before
shell escaping is attempted. Belt-and-suspenders with the escape.
- Harden render_ssh_config so `gateway` and `name` are shell-escaped
even though validate_gateway_name currently gates `gateway`.
- Document the charset contract on CreateSshSessionResponse in
proto/openshell.proto: servers must uphold these rules and clients
reject responses that violate them.
- Add regression unit tests covering adversarial values in every field
and a build_proxy_command test asserting no shell metacharacter
remains outside single-quoted regions.
Tracks OS-100.
* fix(sandbox): strip " (deleted)" suffix from unlinked /proc/<pid>/exe paths
When a running binary is unlinked from its filesystem path — the common
case is a `docker cp` hot-swap of `/opt/openshell/bin/openshell-sandbox`
during the `cluster-deploy-fast` dev upgrade workflow — the Linux kernel
appends the literal string ` (deleted)` to the `/proc/<pid>/exe` readlink
target. The tainted `PathBuf` then flows into `collect_ancestor_binaries`
and on into `BinaryIdentityCache::verify_or_cache`, which tries to
`std::fs::metadata` the path. `stat()` fails with `ENOENT` because the
literal suffix isn't a real filesystem path, and the CONNECT proxy denies
every outbound request with:
ancestor integrity check failed for \
/opt/openshell/bin/openshell-sandbox (deleted): \
Failed to stat ...: No such file or directory (os error 2)
Reproduced in production 2026-04-15: a cluster-deploy-fast-style hot-swap
of the supervisor binary caused every pod whose PID 1 held the now-deleted
inode to deny ALL outbound CONNECTs (slack.com, registry.npmjs.org,
169.254.169.254, etc.), breaking Slack REST delivery, npm installs, and
IMDS probes simultaneously. Existing pre-hot-swap TCP tunnels (e.g. Slack
Socket Mode WSS) kept working because they never re-evaluate the proxy.
Strip the suffix in `binary_path()` so downstream callers see the clean,
grep-friendly path. This aligns the cache key and log messages with the
original on-disk location.
Note: stripping the suffix does NOT by itself make the identity cache
tolerant of a legitimate binary replacement — `verify_or_cache` will now
`stat` and hash whatever currently lives at the stripped path, which is
the NEW binary, and surface a clearer `Binary integrity violation` error.
Fully unblocking the cluster-deploy-fast hot-swap workflow needs a
follow-up that either (a) reads running-binary content from
`/proc/<pid>/exe` directly via `File::open` (procfs resolves this to the
live in-memory executable even when the original inode has been unlinked),
or (b) keys the identity cache by exec dev+inode instead of path. Happy to
send that as a separate PR once the approach is decided — filing this
narrow fix first because it stands on its own: it fixes a concrete
misleading error and unblocks the obvious next step.
Added `binary_path_strips_deleted_suffix` test that copies `/bin/sleep`
to a temp path, spawns a child from it, unlinks the temp binary, verifies
the raw readlink contains the ` (deleted)` suffix, then asserts the public
API returns the stripped path.
Signed-off-by: mjamiv <michael.commack@gmail.com>
* fix(sandbox): narrow the " (deleted)" suffix strip and exercise it end-to-end
Address review feedback from @johntmyers on the initial version of this PR:
procfs::binary_path
- Only strip the kernel's " (deleted)" suffix when stat() on the raw
readlink target reports NotFound. A live executable whose basename
literally ends with " (deleted)" is now returned unchanged instead of
being silently truncated, which matters because identity.rs hashes
whatever this function returns as a trust anchor.
- Operate on raw bytes via OsStrExt, so filenames that are not valid
UTF-8 still get exactly one suffix stripped. The previous
strip_suffix-on-&str path skipped non-UTF-8 entirely and fell through
to returning the tainted path.
- Expand the doc comment to describe both guardrails.
procfs tests
- binary_path_preserves_live_deleted_basename: copy /bin/sleep to a
live file literally named "sleepy (deleted)", spawn it, and assert
that the returned path still ends with " (deleted)".
- binary_path_strips_suffix_for_non_utf8_filename: exec a binary whose
basename contains a 0xFF byte, unlink it, and assert that
binary_path returns the stripped non-UTF-8 path. Writes the bytes
with OpenOptions + sync_all + explicit drop so the write fd is fully
released before exec() to avoid ETXTBSY under concurrent tests.
proxy: extract resolve_process_identity helper
- Pull the peer-resolution + TOFU verify + ancestor walk + cmdline
collection block out of evaluate_opa_tcp into resolve_process_identity.
- Introduce IdentityError which carries the deny reason along with
whatever partial identity data was resolved before the failure so
evaluate_opa_tcp can thread that into ConnectDecision unchanged.
- evaluate_opa_tcp now calls the helper and proceeds straight to the
OPA evaluate step; the surface visible to OPA and OCSF is unchanged.
proxy: end-to-end regression test for the hot-swap contract
- resolve_process_identity_surfaces_binary_integrity_violation_on_hot_swap
stands up a real TcpListener, copies /bin/bash to a temp path,
primes BinaryIdentityCache with that binary, spawns bash with a
/dev/tcp one-liner that opens a real connection to the listener,
and captures the peer's ephemeral port from accept().
- Simulates `docker cp` correctly: unlink the running binary (which
persists via the child's exec mapping) and create a fresh file
with different bytes at the same path. Writing in place is
rejected by the kernel with ETXTBSY, so the old single-inode
approach did not actually model the production failure mode.
- Asserts the error returned by resolve_process_identity contains
"Binary integrity violation" (from BinaryIdentityCache) and does
NOT contain "Failed to stat" or "(deleted)" — the pre-PR-#844
failure mode. The binary field on the error is populated and is
free of the tainted suffix.
- Skips cleanly if /bin/bash is not installed. Child process is
always reaped before the assertion block so a failure does not
leak a sleeping process.
---------
Signed-off-by: mjamiv <michael.commack@gmail.com>
* docs: refresh user-facing docs for recent sandbox and inference changes
- architecture: document system CA loading for upstream TLS, `tls: skip`
as the opt-out, gateway state persistence across restarts, and OCSF
structured logging surface.
- inference: document per-provider header allowlist, Authorization
stripping, 120s streaming idle tolerance, and extended-thinking
timeout guidance.
- manage-sandboxes: add "Execute a Command in a Sandbox" section for
`openshell sandbox exec` with flag reference.
- security best practices: expand seccomp denylist (unconditional and
conditional blocks), document two-phase Landlock probe, High-severity
`landlock-unavailable` finding, and inference keep-alive closure.
- observability logging: document port in HTTP log URLs, `[reason:...]`
denial suffixes, proxy 403/502 JSON error bodies, and Landlock
CONFIG:ENABLED/CONFIG:OTHER events.
Signed-off-by: Miyoung Choi <miyoungc@nvidia.com>
* docs(architecture): tighten high-level architecture page
Trim implementation detail (CA bundle paths, deprecated TLS keys, SSH
handshake secret) from the high-level architecture page, fix accuracy
issues surfaced during deep audit, and expand uncommon acronyms on first
mention.
- Drop unsupported "cost-based routing" claim from Privacy Router row.
- Replace "brokers requests across the platform" with auth-boundary
description.
- Add "inference" to Policy Engine constraint list per AGENTS.md.
- Expand Deny rule to include SSRF, blocked control-plane port, and L7
deny paths in addition to deny-by-default.
- Switch Allow/Deny labels from hyphen to colon; remove em dashes and a
double space.
- Expand LLM, SSRF, L7, TLS, CA, PEM, SSH, OCSF, and JSONL on first use.
Signed-off-by: Miyoung Choi <miyoungc@nvidia.com>
Made-with: Cursor
* docs: address audit feedback on refresh PR
- observability/logging: rewrite the allowed_ips paragraph after the
Denial Reasons table; the previous wording said authors "can use"
invalid entries while also stating they were rejected, which was
contradictory and conflated load-time validation with the runtime
per-CONNECT denial phrases the section documents.
- about/architecture: split compound sentences in the new Gateway
Lifecycle and Observability sections so each clause stands alone.
- inference/about: drop the streaming-tolerance sentence from the prose
paragraph since the dedicated Streaming reliability table row already
covers it.
Signed-off-by: Miyoung Choi <miyoungc@nvidia.com>
Made-with: Cursor
* docs(architecture): address review feedback on architecture page
- Drop the Privacy Router row from the Components table; it is not
yet a separately exposed customer-facing component.
- Update the page description and intro count to match the remaining
three components (gateway, sandbox, policy engine).
- Split the policy decision into the three modes that the engine
actually implements: Explicit Deny (deny rules and hardening rules,
takes precedence), Allow, and Implicit Deny (no rule matched).
Signed-off-by: Miyoung Choi <miyoungc@nvidia.com>
Made-with: Cursor
* docs(architecture): convert policy decisions to a table
Promote the three policy decisions (Explicit Deny, Allow, Implicit
Deny) to a top-level table with Decision, When it applies, and
Outcome columns instead of a nested bulleted list under list item 5.
Top-level tables render reliably across markdown renderers, where
nested-in-list tables do not.
Signed-off-by: Miyoung Choi <miyoungc@nvidia.com>
Made-with: Cursor
* docs: repharse a bit
---------
Signed-off-by: Miyoung Choi <miyoungc@nvidia.com>
* fix(sandbox): block AF_NETLINK in seccomp unconditionally
Move AF_NETLINK to the unconditional socket-domain block list alongside
AF_PACKET, AF_BLUETOOTH, and AF_VSOCK. Previously it was only blocked in
NetworkMode::Block, leaving it accessible in Proxy mode where network
namespace isolation already scopes netlink to the sandbox's own veth —
making this a defense-in-depth hardening rather than a live exposure.
Closes OS-94
* fix(sandbox): scope inference.local interception to port 443
The pre-OPA interception for inference.local matched on hostname alone,
allowing any port to bypass OPA policy evaluation — including under
deny-all (network_policies: {}). Add a port check so only port 443
takes the interception path; all other ports on inference.local now
fall through to OPA and are subject to normal policy evaluation.
Closes OS-95
* fix(sandbox): enforce RLIMIT_NPROC to prevent fork bomb DoS
Set a hard limit of 512 processes per UID in harden_child_process(),
applied before privilege drop so the sandbox user cannot raise it.
Prevents unrestricted fork() from exhausting the process table — most
relevant for local dev mode where K8s pod cgroup pids.max is absent.
Closes OS-96
* chore(sandbox): fix rustfmt formatting for seccomp blocked_domains
---------
Co-authored-by: John Myers <johntmyers@users.noreply.github.com>
Each RFC now lives in its own folder (rfc/NNNN-short-name/) with
README.md as the main proposal. This gives each RFC room for diagrams,
images, and other supporting files without cluttering the top level.
Move 0000-template into the new layout and update rfc/README.md to
describe the convention.
The proxy's upstream TLS client only trusted Mozilla root CAs
(webpki-roots), which prevented TLS termination from working with
internal/corporate hosts using private CA certificates.
Load system CA certificates from the container's trust store
(e.g. /etc/ssl/certs/ca-certificates.crt) in addition to
webpki-roots. This allows custom sandbox images to include
corporate CAs via update-ca-certificates.
Signed-off-by: Matthias Osswald <mat.osswald@sap.com>
`git describe --tags --long --match "v*"` matches the `vm-dev` tag
alongside release tags. When `vm-dev` sits on or past the latest
release tag in the commit graph (currently the case on main), git
picks it, and the resulting `vm-dev-N-gSHA` string strips the leading
`v` down to `m-dev-N-gSHA`. With `commits == 0` that's returned
verbatim, producing a binary that reports itself as `m-dev`:
$ openshell --version
openshell m-dev
Confirmed on v0.0.28 (#832) and reproduced on v0.0.29 as well.
Restrict the glob to numeric release tags (`v[0-9]*`). All release
tags follow `v\d+\.\d+\.\d+`, so this loses no valid version — it
only filters out `vm-dev`, `vm-prod`, and any similar non-release
tags that share the `v` prefix.
Verified locally on this repo's current HEAD:
# before
$ git describe --tags --long --match 'v*'
vm-dev-0-g355d845
# after
$ git describe --tags --long --match 'v[0-9]*'
v0.0.29-2-g355d845
Closes#832
Signed-off-by: mjamiv <michael.commack@gmail.com>
format_sse_error only escaped `\` and `"`, leaving two problems:
1. Control characters (`\n`, `\r`, `\t`, and all `\u0000-\u001F`) in
`reason` produce output that fails `serde_json::from_str` — defeating
#834's goal of giving clients a parseable SSE truncation signal.
2. An unescaped `\n\n` inside `reason` splits the single error event
into two SSE frames, letting a misbehaving upstream inject a forged
frame (e.g. a fake tool_calls delta) into the client's stream. Latent
today since all in-tree callers pass static strings, but a footgun
for any future caller passing upstream error text, and the function's
docstring already invites dynamic reasons.
Replace the manual escape with `serde_json::to_writer` (already a
workspace dep of `openshell-sandbox`). Add unit tests for control
character escaping and SSE event-boundary injection.
Closes#840
Signed-off-by: mjamiv <michael.commack@gmail.com>
* feat(policy): add deny rules to network policy schema
Closes#565
Add L7 deny rules that block specific requests even when allowed by
access presets or explicit allow rules. Deny rules mirror the full
capability set of allow rules (method, path, query params, SQL command)
and take precedence -- if a request matches any deny rule, it is blocked
regardless of allow rules.
This enables the "allow everything except these specific operations"
pattern without enumerating every allowed endpoint. For example, granting
read-write access to GitHub while blocking PR approvals, branch
protection changes, and ruleset modifications.
* fix(policy): deny query matching fails closed, mirror allow-side validation
Addresses PR review findings P1 and P3:
P1: Deny-side query matching now uses fail-closed semantics. If ANY
value for a query key matches the deny matcher, the deny fires. The
previous implementation reused allow-side "all values must match"
logic which allowed ?force=true&force=false to bypass a deny on
force=true.
P3: Deny-side query validation now mirrors the full allow-side checks:
empty any lists, non-string matcher values, glob+any mutual exclusion,
glob type checks, and glob syntax warnings are all validated.
Drew Newberry