mirror of
https://github.com/NVIDIA/OpenShell.git
synced 2026-10-18 05:39:50 +08:00
openshell-in-openshell
101
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Drew Newberry
|
5bb970a7cc | feat(devshell): add provider-backed VM dev sandbox | ||
|
Taylor Mutch
|
a3b16c18ab | feat(auth): per-sandbox authentication to gateway (#1404) | ||
|
Colin Walters
|
e7f965a988 |
refactor(sandbox,driver-vm): Start moving to rustix (esp over libc unsafe) (#1505)
In the Rust ecosystem there's largely three ways to do system calls: - raw libc - nix - rustix Of the three, libc is almost all `unsafe` and really 95% of use cases should be either nix or rustix. nix is the original one, but after having looked at the code of both, I think rustix is just better designed and organized. It's also reached 1.0, whereas nix is still making semver-breaking changes (in fact we're behind here in this project). Now in practice, we have both *transitively* in the depchain already, and that's true for quite a lot of projects. But I think rustix is better, so let's add rustix as a workspace dependency (process feature) and migrate a few use cases to it - it's especially better than the raw libc which is suprisingly widespread. If we agree to do this, then many other calls can be ported. Signed-off-by: Colin Walters <walters@verbum.org> |
||
|
mjamivandJohn Myers
|
528fb29147 |
fix(sandbox): allow first-label L7 host wildcards (#1304)
* fix(sandbox): allow first-label L7 host wildcards * docs(sandbox): document L7 host wildcard contract + add OPA runtime tests - Add Host Wildcards section to architecture/security-policy.md describing accepted (first-label *, **, intra-label *-X) and rejected (bare, TLD, non-first-label, recursive-in-label) forms, and noting that wildcards never cross '.' boundaries. - Expand the policy-schema.mdx 'host' field description to reflect the same contract instead of only mentioning '*.example.com'. - Add OPA runtime tests asserting '*-aiplatform.googleapis.com' matches 'us-central1-aiplatform.googleapis.com' and does not match 'us-central1.aiplatform.googleapis.com' (cross-dot boundary). Locks validator/runtime alignment for intra-label wildcards. * chore: update mise lockfile * test(server): tolerate serialized inference upserts --------- Co-authored-by: John Myers <9696606+johntmyers@users.noreply.github.com> |
||
|
Derek Carr
|
2d9e5326f8 |
fix(sandbox): skip fork-exec socket ambiguity test on SELinux-enforcing hosts (#1449)
Exec'ing /bin/sleep (SELinux label bin_t) from a user_home_t test binary causes /proc/<pid>/exe readlink to return ENOENT on SELinux-enforcing hosts due to the cross-domain boundary. Skip the test at runtime when getenforce reports Enforcing. Also adds a ChildGuard drop guard for safe child cleanup on panic and increases the exec-detection deadline from 2s to 5s. Signed-off-by: Derek Carr <decarr@redhat.com> |
||
|
John T. Myers
|
d255cdd9c9 |
feat(providers): add credential refresh foundation (#1349)
* feat(providers): add credential refresh foundation * feat(providers): mint oauth refresh token credentials * fix(providers): allow delegated refresh bootstrap * fix(providers): guard refresh credential modes * fix(providers): refine refresh lifecycle UX * fix(cli): restore provider test gateway response import * fix(server): clean auth endpoint test qualifications * fix(providers): tighten refresh authorization and collisions * chore(providers): trim bundled v2 profiles * fix(providers): resolve refresh rebase fallout * fix(cli): accept rfc3339 credential expiry * test(providers): update inferred claude provider type * test(providers): avoid removed outlook default profile * test(providers): isolate attach limit fixtures |
||
|
Alec Fong
|
d620d65f6e |
feat(sandbox): inject DENO_CERT into sandbox child environment (#1441)
Sandbox processes running on Deno couldn't TLS-verify connections to services signed by OpenShell's per-sandbox ephemeral CA because Deno doesn't consult `SSL_CERT_FILE` / `NODE_EXTRA_CA_CERTS`. Deno reads its own `DENO_CERT` env var (additive PEM bundle, same shape as `NODE_EXTRA_CA_CERTS`) for that. Add `DENO_CERT` to `child_env::tls_env_vars` pointing at the standalone OpenShell CA cert (not the combined system+sandbox bundle, since DENO_CERT is additive). The three callers in `process.rs` / `ssh.rs` iterate the array, so the size bump from 5 to 6 entries flows through without caller changes. Updates the security best-practices doc to enumerate `DENO_CERT` alongside the other trust-store env vars. Tracks: brevdev/dex-ui#5 Signed-off-by: Alec Fong <alecf@nvidia.com> |
||
|
Russell Bryant
|
c5d1d76d94 |
refactor(sandbox): replace iptables with nftables for network policy enforcement (#1401)
Migrate all sandbox and VM driver network policy enforcement from iptables to nftables. nftables provides atomic ruleset loading, a cleaner rule syntax, and is the standard netfilter interface in modern kernels. Sandbox bypass enforcement (openshell-sandbox): - Replace iptables chain of individual rule insertions with a single atomic nftables ruleset load via nft -f - New nft_ruleset module with pure functions for ruleset generation and unit tests - Combine log and reject rules in one inet family table (handles both IPv4 and IPv6 in a single ruleset) - Fall back to reject-only ruleset when kernel lacks nft_log support - Enable net.netfilter.nf_log_all_netns so log rules work from non-init network namespaces - Use temp file for nft ruleset loading instead of stdin for compatibility with minimal VM guest environments VM TAP networking (openshell-driver-vm): - Replace iptables NAT/forwarding rules with nftables equivalents - New nft_ruleset module for TAP network rule generation with unit tests - Atomic table-per-TAP-device lifecycle (create/destroy) - Host-side rules provide NAT infrastructure and defense-in-depth isolation (input chain restricts VM to gateway port only, forward chain blocks unsolicited inbound); primary security enforcement happens inside the VM guest via the sandbox supervisor's own rules VM init script: - Load nft kernel modules at sandbox init - Enable nf_log_all_netns sysctl for bypass detection logging OCSF / docs: - Update firewall rule engine references from iptables to nftables - Document host firewall interaction model and two-layer enforcement architecture in VM driver README and compute drivers reference Closes #1335 Signed-off-by: Russell Bryant <rbryant@redhat.com> |
||
|
mjamiv
|
a54758cdaa | test(sandbox): cover inference stream truncation errors (#1418) | ||
|
Derek Carr
|
7f16d60ef1 |
feat(persistence): implement optimistic concurrency control with CAS (#1292)
Add resource_version-based optimistic concurrency control to the persistence layer. Every write now requires an explicit WriteCondition (MustCreate or MatchResourceVersion), enforced at compile time by gating unconditional put/put_message behind #[cfg(test)]. - Add WriteCondition enum and put_if for conditional writes - Add update_message_cas for atomic read-modify-write operations - Add list_messages/list_messages_with_selector helpers that hydrate resource_version from authoritative DB rows - Convert all production write paths to CAS-aware methods - Gate put/put_message behind #[cfg(test)] to prevent non-CAS writes - Use structured PersistenceError::UniqueViolation matching instead of string matching for duplicate detection - Hydrate resource_version from WriteResult directly on creates, eliminating unnecessary read-after-write round trips Signed-off-by: Derek Carr <decarr@redhat.com> |
||
|
Derek Carr
|
b4c7bc46e1 |
fix(sandbox): stabilize forked socket owner test (#1417)
Increase the polling deadline from 2s to 5s to tolerate loaded CI machines, and add a ChildGuard drop guard so the forked child is always reaped even if the test panics. Signed-off-by: Derek Carr <decarr@redhat.com> |
||
|
Mesut Oezdil
|
283defd285 |
fix(sandbox): allow HEAD where GET is permitted in L7 policy (#1382)
* fix(sandbox): allow HEAD where GET is permitted in L7 policy RFC 9110 §9.3.2 defines HEAD as identical to GET except no response body. method_matches in sandbox-policy.rego rejected HEAD even when GET was explicitly allowed, breaking package managers that probe with HEAD before fetching. Signed-off-by: mesutoezdil <mesudozdil@gmail.com> * test(sandbox): fix binary mismatch in HEAD/POST test and add deny-rule regression l7_head_denied_when_only_post_allowed used /bin/sh in the fixture but l7_input() sends /usr/bin/curl, so requests were denied by binary policy before the method rule could be evaluated. A regression allowing HEAD to match POST would have passed undetected. Fixes the fixture binary and adds l7_head_blocked_by_deny_rule_targeting_get to cover the deny_rules path of method_matches(), per review feedback. Signed-off-by: mesutoezdil <mesudozdil@gmail.com> --------- Signed-off-by: mesutoezdil <mesudozdil@gmail.com> |
||
|
Adam Miller
|
63bdcd1379 |
fix(sandbox): exempt host gateway from SSRF block for rootless Podman (#1279)
* fix(sandbox): exempt host gateway from SSRF block for rootless Podman pasta assigns a link-local IP (169.254.x.x) as the host gateway in rootless Podman containers, which was always blocked by SSRF guards. Read the trusted gateway IP from /etc/hosts at proxy startup (injected by the driver via host.openshell.internal / host.containers.internal aliases) and exempt only that specific IP from the link-local block, while keeping 169.254.169.254 (cloud metadata) hard-blocked regardless. * fix(net): add is_link_local_ip() helper and harden trusted-gateway SSRF Extract a public is_link_local_ip(IpAddr) helper in openshell-core::net covering IPv4 169.254.0.0/16, IPv6 fe80::/10, and IPv4-mapped variants. Refactor the inline IPv6 fe80::/10 bitmask in is_always_blocked_ip() and is_internal_ip() to use the new helper, eliminating two copies of the same pattern. Harden the trusted-gateway SSRF exemption in proxy.rs: both detect_trusted_host_gateway() and resolve_and_check_trusted_gateway() now reject always-blocked non-link-local IPs (loopback, unspecified) with a warning. The exemption was intended only for link-local addresses used by rootless Podman with pasta; a /etc/hosts entry mapping host.openshell.internal to 127.0.0.1 or 0.0.0.0 previously bypassed the loopback/unspecified SSRF invariant. Add 169.254. prefix to the generate_security_notes() host checks in mechanistic_mapper and policy server, closing a gap where link-local literal hosts were not flagged as internal/private in advisory notes. * fix(sandbox): warn on ambiguous /etc/hosts gateway and add real function tests When detect_trusted_host_gateway() finds more than one distinct IP for host.openshell.internal, emit a structured warning so operators can diagnose unexpected /etc/hosts state. The first entry still wins (and any runtime mismatch is caught by resolve_and_check_trusted_gateway), but the ambiguity is now observable. Replace inline guard-logic tests with real calls to resolve_and_check_trusted_gateway() using IP literals, which bypass DNS and give deterministic results. This exercises the actual function wiring rather than mirroring the predicate logic manually. Add parse_hosts_file_for_host tests covering: single entry, same-IP deduplication across lines, multiple distinct IPs (documents first-wins ordering), multi-alias lines, missing alias, and comment handling. * fix(sandbox): narrow trusted-gateway exemption to link-local IPs only Two security fixes addressing PR review feedback: 1. Replace the compound is_always_blocked_ip && !is_link_local_ip guard in detect_trusted_host_gateway() with a plain !is_link_local_ip check. Private RFC 1918 addresses (e.g. Docker bridge 172.17.0.1, K8s node 192.168.x.x) are not always-blocked, so they slipped through the old guard and received the SSRF exemption, bypassing resolve_and_reject_internal(). The exemption is only valid for the rootless Podman/pasta link-local case. 2. Normalize IPv4-mapped IPv6 in is_cloud_metadata_ip() so that ::ffff:169.254.169.254 is correctly identified as cloud metadata. is_link_local_ip() already recognises this form as link-local; without the normalization it would pass the metadata guard and receive the trusted-gateway exemption, violating the 'metadata is never exempted' invariant. Apply the same !is_link_local_ip tightening to resolve_and_check_trusted_gateway() as defense-in-depth, and update/add tests to cover both guards including IPv4-mapped metadata, private IP rejection, and the Docker bridge scenario. |
||
|
Russell Bryant
|
9a7c0df00b |
fix(sandbox): remove DNS resolution from mechanistic mapper to prevent data exfiltration (#1329)
* fix(sandbox): remove DNS resolution from mechanistic mapper to prevent data exfiltration The mechanistic mapper resolved denied hostnames via DNS to check whether they mapped to private IPs (for populating allowed_ips in proposals). This leaked the denied hostname through DNS queries even though the HTTP connection was blocked by policy — a data exfiltration vector. Remove resolve_allowed_ips_if_private() entirely. Proposals no longer include allowed_ips. If a user applies a proposed rule and the host resolves to a private IP, the proxy's SSRF defense denies the connection. That denial flows back through the aggregator, and the user can then explicitly configure allowed_ips in their policy. This two-step flow is also more explicit about private IP access, which is desirable given #1245. Closes #1169 Signed-off-by: Russell Bryant <russell.bryant@gmail.com> * fix(sandbox): update SSRF note and document two-step allowed_ips flow Update the mechanistic mapper SSRF security note to reflect that proposals no longer populate allowed_ips — approving a proposal does not bypass SSRF protection. Clarify the two-step approval flow in architecture/security-policy.md. Signed-off-by: Russell Bryant <russell.bryant@gmail.com> --------- Signed-off-by: Russell Bryant <russell.bryant@gmail.com> |
||
|
Taylor Mutch
|
7a0c444445 |
refactor!(auth): drop SSH handshake secret (#1274)
* refactor!(auth): drop SSH handshake secret in favor of mTLS The OPENSHELL_SSH_HANDSHAKE_SECRET / x-sandbox-secret mechanism was misnamed: it does not authenticate SSH (which flows over the RelayStream gRPC RPC and is gated by mTLS plus supervisor Unix-socket permissions). It only gated a small set of sandbox-to-gateway control-plane RPCs, and production deployments already enforce mTLS on that channel — so the shared secret was redundant. Replace the secret check with an mTLS-presence marker. Sandbox-class methods (ReportPolicyStatus, PushSandboxLogs, GetSandboxProviderEnvironment, SubmitPolicyAnalysis, GetSandboxConfig, GetInferenceBundle) accept callers without a Bearer token; the gRPC mTLS handshake is the trust boundary. Dual-auth methods treat Bearer-present as full-scope CLI access and Bearer-absent as sandbox-restricted scope via validate_sandbox_caller_update. Drops the secret from all drivers (K8s, Podman, VM), the sandbox gRPC interceptor, the Helm chart (values + pre-install hook + StatefulSet env), the RPM bootstrap script, the man pages, and the debug-openshell-cluster skill. Also removes the never-read ssh_handshake_skew_secs flag and config field. BREAKING CHANGE: --ssh-handshake-secret / OPENSHELL_SSH_HANDSHAKE_SECRET and --ssh-handshake-skew-secs / OPENSHELL_SSH_HANDSHAKE_SKEW_SECS are removed from the gateway, sandbox, and all driver binaries. The openshell-ssh-handshake K8s Secret is no longer managed by the chart; operators may delete the orphan. Deployments using --disable-gateway-auth must enforce caller authentication at the fronting proxy, since the gateway no longer validates a per-request secret on sandbox-class methods. Refs OS-174. * docs(auth): scrub residual SSH handshake secret references Sweep across docs, e2e scripts, the Podman driver README/NETWORKING notes, the RPM/Helm/setup guides, the gateway man page, and RFC 0003 to remove instructions and examples that still referenced OPENSHELL_SSH_HANDSHAKE_SECRET / --ssh-handshake-secret / ssh_handshake_skew_secs. The mechanism is gone; nothing should still suggest setting it. Negative-assertion regression tests are kept so the env var cannot silently be re-introduced. * test(drivers): drop SSH handshake secret negative-assertion tests The supporting code, env vars, CLI flags, and config plumbing are gone — these tests asserted absence of strings that no longer have any path to being set. Remove the guards from the Docker, Podman, Kubernetes, and VM driver test modules. |
||
|
Eric Curtin
|
6deb1f005a |
refactor(core): eliminate duplicate utilities across crates (#1381)
Consolidate shared constants and helpers into openshell-core to avoid duplicate definitions scattered across crates. Adds OPENSHELL_SANDBOX to the sandbox_env module so all drivers and the sandbox supervisor reference it via a typed constant rather than string literals, matching the existing pattern for SANDBOX_ID, ENDPOINT, and the other env vars. |
||
|
Alexander Watson
|
ea2fddbe2d |
feat(policy): agent-driven policy management — the agent half (#1323)
* feat(policy): plumb chunk_ids and rejection_reason through proposal pipeline Prereq plumbing for the agent revise-and-resubmit loop. Two narrow additive proto changes unblock the upcoming /wait endpoint (#1092), prover validation badge (#1097), and reject --guidance surfaces (#1098). - SubmitPolicyAnalysisResponse: add accepted_chunk_ids so the in-sandbox agent gets handles to watch its proposals. Surfaced through the typed grpc_client wrapper and policy.local's POST /v1/proposals 202 body. Closes #1094. - PolicyChunk + StoredDraftChunk + DraftChunkPayload: add validation_result (gateway prover verdict, populated by #1097) and rejection_reason (operator free-form text). Both plain strings; no enums, no parsing on the read path. Closes #1096. - RejectDraftChunk now persists the existing reason field into the chunk's rejection_reason so it round-trips back to the agent via GetDraftPolicy. UndoDraftChunk clears it on the way back to pending so consumers cannot read a stale guidance string from a prior reject -> re-approve -> undo cycle. Whole surface stays gated behind agent_policy_proposals_enabled. Two focused tests cover the round-trip and the undo-clears guarantee. Signed-off-by: Alexander Watson <zredlined@gmail.com> * feat(sandbox): add /v1/proposals/{id} and /wait long-poll to policy.local The agent feedback channel back from policy.local. Two new routes let the in-sandbox agent learn its proposal's outcome on a single blocking HTTP call — zero LLM tokens during the wait. - GET /v1/proposals/{chunk_id} returns the chunk's current state in one gateway call. - GET /v1/proposals/{chunk_id}/wait?timeout=<s> blocks until the chunk transitions out of pending. Default 60s, clamped [1, 300]. Agent re-issues on timeout to extend. Response carries the chunk's status plus the two feedback fields shipped in the prereq commit: rejection_reason (free-form reviewer text) and validation_result (gateway prover verdict, empty until #1097). On timeout: same shape with timed_out: true so the agent can disambiguate without parsing. Wait handler short-polls GetDraftPolicy every 1s inside the request with a tokio::time::Instant deadline. One gateway connection is opened per request and reused across all polls, so a 60s wait does one TLS handshake instead of sixty. A future commit can swap the loop body for a tokio::sync::broadcast driven by a watcher task — the agent-visible contract (URL, query, response shape) is independent of the polling implementation. All routes stay behind agent_policy_proposals_enabled. Closes #1092. Signed-off-by: Alexander Watson <zredlined@gmail.com> * docs(sandbox): teach policy_advisor skill the wait + redraft loop The agent-facing instructions for the feedback loop. The endpoints exist; this is the doc that makes them usable. policy_advisor.md gains: - API entries for GET /v1/proposals/{chunk_id} and /wait?timeout=<s>, including the field semantics (status, rejection_reason, validation_result, timed_out). - A note on the submit response's accepted_chunk_ids / rejection_reasons split so the agent handles partial acceptance. - Step 6 saves the chunk_ids and addresses any submit-time rejections before waiting. - Step 7 walks the four wait outcomes: approved (retry, with the honest "may still fail" caveat), rejected (read rejection_reason AND validation_result; address whichever has content), still-pending with timed_out (re-call), non-2xx (surface, do not retry). skills.rs gains two assertions on the skill content so a future edit cannot drop the wait endpoint or the rejection_reason directive silently. Closes #1095. Signed-off-by: Alexander Watson <zredlined@gmail.com> * test(policy-advisor): add end-to-end smoke for the agent feedback loop A focused smoke that exercises the new policy.local /wait endpoint on a live gateway + sandbox, separate from the existing no-LLM regression harness (which still drives the OLD retry-with-bash-loop recovery pattern). Two flows: - Flow A — approve-and-retry: agent submits, /wait blocks, host runs `openshell rule approve`, /wait returns status=approved. Confirms the happy path round-trip latency. - Flow B — reject-with-guidance: agent submits, /wait blocks, host runs `openshell rule reject --reason "..."`, /wait returns status=rejected with the exact reviewer text in rejection_reason. Confirms the free-form guidance contract round-trips through the agent feedback channel. No GitHub credentials needed — proposals are synthetic and never trigger outbound traffic. Both flows expect agent_policy_proposals_enabled=true and a running gateway. Adds three cases to sandbox-runner.sh: submit-test-proposal (no GH deps), proposal-status, proposal-wait. The existing put-file and submit-proposal cases used by test.sh are untouched. Signed-off-by: Alexander Watson <zredlined@gmail.com> * fix(policy-advisor): surface real CLI errors from wait-smoke preflight The preflight piped openshell's stderr to /dev/null and relied on jq to default the missing setting key to "<unset>", but under `set -euo pipefail` a non-zero exit from openshell makes the whole pipeline fail and the command substitution exits the script silently before the intended fail() message can print. Capture stderr explicitly, check the CLI exit code, and surface the real error plus the expected fix (port-forward + gateway add + select) when the CLI cannot reach the gateway. Signed-off-by: Alexander Watson <zredlined@gmail.com> * fix(policy-advisor): pass --json to settings get in wait-smoke preflight `openshell settings get --global` defaults to a human-readable table; jq cannot parse it and the preflight died with a numeric-literal error. Pass --json so jq gets actual JSON. Also touched up the suggested recovery commands in the preflight error to match the real CLI shape (`gateway add <endpoint> --name <name>` and the env-var override warning). Signed-off-by: Alexander Watson <zredlined@gmail.com> * fix(policy): dedup draft chunks only in mechanistic mode; return effective id The smoke harness for the agent feedback loop caught a real bug in the gateway: SubmitPolicyAnalysis's response carried a chunk_id that was never persisted whenever the SQL ON CONFLICT path fired. Two failure modes, both load-bearing: - Agent-authored proposals targeting the same host/port/binary (e.g. the redraft-after-rejection loop) silently folded into one row and any RejectDraftChunk by the new chunk_id failed with "chunk not found." Latent since #1151, surfaced by #1094 returning chunk_ids. - Mechanistic mode had the same class of bug — the dedup fold-in is the intended behavior there, but the response still advertised the newly-generated UUID instead of the existing row's id. Less visible because no current caller reads mechanistic chunk_ids back, but the proto contract was violated either way. Fix in three parts: - put_draft_chunk now takes Option<&str> dedup_key explicitly and returns the effective row id (via RETURNING). None binds NULL to the dedup_key column, which bypasses the partial-index ON CONFLICT path entirely. Caller-decides semantics replace store-side magic. - handle_submit_policy_analysis picks dedup_key per chunk using an allowlist (only "mechanistic" dedups) and pushes the returned effective_id to accepted_chunk_ids. New modes default to no-dedup so a misconfigured caller cannot silently lose proposals. - The two-copy draft_chunk_dedup_key helper consolidated to one observation_dedup_key in policy_store.rs with a doc comment. Tests: - agent_authored_submits_for_same_endpoint_do_not_dedup pins the redraft-loop contract: two intentional submissions with the same host/port/binary get distinct chunk_ids, both findable via GetDraftPolicy, both rejectable by id. - mechanistic_submits_for_same_endpoint_dedup_into_one_chunk locks in the observation-mode dedup AND asserts both submits return the same effective_id — would have caught the deeper bug. Proto: SubmitPolicyAnalysisRequest.analysis_mode doc updated to describe the actual semantics (mechanistic dedups, agent_authored and unknown modes do not). Signed-off-by: Alexander Watson <zredlined@gmail.com> * docs(examples): retarget policy-management demo at the /wait endpoint The narrated demo (examples/agent-driven-policy-management) has been the public face of this feature since #1151. Its agent prompt told Codex to retry the original PUT every few seconds for up to 120 seconds — a polling workaround for the missing /wait endpoint that this branch shipped. Update the demo to exercise /wait so the canonical reading of the feature reflects the actual UX win. - agent-task.md: step 4 is now "call /wait, branch on status" with the three outcomes spelled out (approved → retry once; rejected → read rejection_reason and revise or stop; pending+timed_out → re-issue /wait once, do NOT busy-loop or shorten the timeout). Also makes explicit that the demo submits one rule per proposal so accepted_chunk_ids[0] is the safe single id to wait on. - demo.sh: header docstring rewritten as a six-step loop that mirrors the README. narrate_sandbox_workflow drops its parallel numbering and uses bullets (the runtime narration is the agent's sub-actions, not a separate decomposition of the loop). Approve step header and success message now reference /wait waking the agent, not "policy hot-reload retry." - README.md: top-of-file flow expanded from 5 to 6 steps to include the /wait call and chunk_id capture; "Going further" section now describes both regression scripts and the boundary between them (real-GitHub retry vs. synthetic /wait wire test). Slow-path qualifier corrected from "image pull on first run" to "sandbox cold-start (SSH bring-up plus Codex install)". - wait-smoke.sh header rewritten to make it unambiguous this is a regression, NOT a tutorial, with explicit prereq commands instead of prereq descriptions, and a pointer at demo.sh for the narrated story. No code paths change; this is the readability pass. Signed-off-by: Alexander Watson <zredlined@gmail.com> * fix(examples): pass --yes on demo.sh's global setting writes Global setting updates require explicit confirmation in non-interactive mode; demo.sh's enable_agent_proposals and the cleanup restore path were missing --yes and hard-failed the preflight. Pre-existing issue that surfaced now that more of the demo runs through this path. No other behavior change. Signed-off-by: Alexander Watson <zredlined@gmail.com> * feat(sandbox): emit OCSF audit events for policy proposal lifecycle The demo's policy decision trace previously showed only the proxy enforcement story (HTTP:PUT DENIED, CONFIG:LOADED, HTTP:PUT ALLOWED). It was silent about who proposed what or who decided what — the audit-trail receipts for the agent feedback loop were missing. policy.local now emits sandbox-side OCSF events at the observation moments, into the same stream as the existing CONFIG:LOADED: - CONFIG:PROPOSED on submit_proposal acceptance. Per accepted chunk: the message names the chunk_id, target endpoint, L7 method/path, and binary so the trace correlates against the inbox card via chunk_id. - CONFIG:APPROVED on /wait observation of approved status. - CONFIG:REJECTED on /wait observation of rejected status. Carries the reviewer's free-form rejection_reason in the message AND as an unmapped field, both sanitized (control chars stripped, capped at 200 chars with an ellipsis marker). The agent still reads the raw text via GET /v1/proposals/{id}; sanitization is audit-side only, per AGENTS.md's no-secrets-in-OCSF rule. The submit path defends the audit_summaries / accepted_chunk_ids index pairing against a future gateway change that compresses past rejected chunks (the proto doesn't promise 1:1 ordering with the request). Today client-side validation makes the lengths always match; if they don't, the pairing falls back to a generic per-id event rather than mis-attribute. The wait handler's emit site fires once per terminal-status observation. Multiple concurrent waiters on the same chunk would each emit one event; acceptable for single-waiter-per-chunk demos and the right place to dedup is the SIEM. demo.sh's trace filter now surfaces the four CONFIG: events alongside HTTP:PUT, so the trace at the end of every run tells the full story from deny to allow via propose -> approve. wait-smoke.sh's prereq notes recommend redirecting kubectl port-forward output so its "Handling connection for 8090" lines don't bleed into demo narration. Three new unit tests on the sandbox-side helpers — summary builder happy path, fallback, and the rejection_reason sanitizer. Signed-off-by: Alexander Watson <zredlined@gmail.com> * feat(policy): /wait awaits local policy reload; demo auto-approves redrafts Three things in one commit, all surfaced by running the demo end-to-end against a real gateway and finding the agent had to draft a broader second proposal. 1. /wait race fix. Previously /wait returned `approved` the moment it observed the gateway's chunk status flip, but the local supervisor reloads policy on its own poll cycle (~10s in practice). The agent's retry would race the reload and hit the still-old policy, getting denied. Codex then drafted a broader rule and re-submitted — sound agent behavior, but not what /wait should provoke. Now /wait captures the local policy version at start, and after observed-approved waits for the supervisor to load a strictly-newer version before returning. Bounded by the caller's deadline; best-effort return if the deadline elapses without the version bumping. Two new unit tests pin the happy path and the deadline-clamped fallback. 2. demo.sh auto-approve loop. Replaces approve_when_pending + wait_for_agent with one approve_pending_until_agent_exits function that keeps watching for pending chunks and approving them until the agent process exits (or the configured timeout). Defense in depth against future redraft scenarios for any reason; today (post-fix #1) the agent should only submit one proposal per task, but we don't want to hang silently if it does submit more. 3. UX. Step headers now carry "[t+1.2s]" relative timestamps so reading the run output makes latency visible (the demo's whole point is the wait is cheap — surface that). A spin_wait helper renders an ASCII spinner during the watch loop so the demo never looks frozen on a TTY. Falls back to plain sleep on non-TTY contexts. Closes the race condition diagnosed from the trace timing where the gateway approved at t+0, sandbox observed at t+0.3s, but the supervisor didn't load v2 until t+9.4s — well after the agent had already retried and been denied. Signed-off-by: Alexander Watson <zredlined@gmail.com> * fix(sandbox): /wait detects policy reload by content, not the schema version The previous attempt at the /wait-after-approve race fix compared `SandboxPolicy.version` between /wait start and the policy reload — but that field is the *schema* version (constant 1), not a revision counter. Every comparison was `current(1) > baseline(1) == false`, so the wait blocked until the agent's 300s timeout regardless of whether the supervisor had actually reloaded. The demo SSH connection then timed out around the 240s mark. Diagnosed from a live run's OCSF trace: supervisor pulled v2 at +8.5s after approval (CONFIG:LOADED), but the sandbox-side CONFIG:APPROVED that my /wait emits didn't fire until +304s — exactly at the 300s deadline. Fix: compare the whole policy via prost's derived PartialEq. Any field change (network_policies map being the only one that actually mutates today) flips equality. A clone-per-200ms-tick on a few-KB proto is cheap inside the bounded wait window. Tests rewritten to match the new contract: the supervisor-reload fixture now keeps `version: 1` constant and changes `network_policies` contents, mirroring the exact failure mode from the live run. Signed-off-by: Alexander Watson <zredlined@gmail.com> * fix(examples): redact tokens with python literal-string replace, not sed The sed-based redact_log in demo.sh broke when one of the auth tokens contained a character that conflicted with sed's pattern parser ("unterminated substitute pattern" on the Codex JWT). The whole log tail then blanks on failure, hiding the very failure context we're trying to surface. Switch to a python subprocess that takes the tokens via argv and does literal str.replace. No regex, no delimiter games, no truncation. Signed-off-by: Alexander Watson <zredlined@gmail.com> * fix(sandbox): scope /wait reload check to the approved rule Reviewer (John Myers) flagged two failure modes in the prior whole-policy fingerprint approach used by policy.local /wait: - False sleep: when the supervisor reloads between two /wait calls (the skill tells the agent to re-issue on timed_out), the new call snapshots the already-updated policy as baseline and burns the full timeout waiting for a change that never comes. - False wakeup: any unrelated reload (other agent's approval, settings change) flips the diff, but the chunk's actual rule may not be loaded yet — the agent retries and hits policy_denied for no real signal. Replace the diff with rule-coverage. New public helper openshell_policy::policy_covers_rule reuses endpoints_overlap (so it matches add_rule's merge semantics, including the fold-into-existing-key case) plus an L7 allow check on method/path (so an existing endpoint that doesn't yet contain the proposed method doesn't signal coverage). Add policy_reloaded: true|false to the /wait response on approve, with a 500ms floor on the reload-wait phase so approvals arriving near the deadline still get a fair shot at reloaded=true. Update the policy_advisor skill to branch on it: reloaded=true → retry; reloaded=false → re-issue /wait once with timeout=30, then surface to user. Don't loop tightly. Tests: - 9 new unit tests in openshell-policy pinning coverage semantics (L4-only, L7 method gap, fold-into-existing-key, empty binaries). - 4 new tokio tests in policy_local mirroring John's exact scenarios. - wait-smoke.sh asserts policy_reloaded=true on Flow A. Signed-off-by: Alexander Watson <zredlined@gmail.com> * fix(server): make GetDraftPolicy dual-auth so /wait works under OIDC policy.local calls GetDraftPolicy from inside the sandbox supervisor via the sandbox gRPC client, which authenticates with the shared x-sandbox-secret. GetDraftPolicy was listed only in the Bearer-auth scope table (config:read) and was not in SANDBOX_SECRET_METHODS or DUAL_AUTH_METHODS, so OIDC-enabled gateways rejected those calls and the /wait long-poll surfaced gateway_lookup_failed. Local/no-OIDC setups happened to work because the auth check is short-circuited. Add GetDraftPolicy to DUAL_AUTH_METHODS, matching the existing GetSandboxConfig pattern (called by both CLI reviewer surfaces with Bearer and the sandbox supervisor with x-sandbox-secret). Dual-auth short-circuits the scope check for sandbox-secret callers, so the config:read entry in authz.rs continues to gate Bearer-only flows. Mirror the openshell_get_sandbox_config_is_dual_auth assertion for GetDraftPolicy. Note: ssh_handshake_secret is server-wide, not per-sandbox, so a sandbox-secret caller can today name any sandbox in a SubmitPolicyAnalysis request — and now in a GetDraftPolicy request. The exposure is symmetric with the existing SANDBOX_SECRET_METHODS pattern. Filed as a follow-up: per-sandbox secret binding, tracked separately. Signed-off-by: Alexander Watson <zredlined@gmail.com> * fix(ci): address rebased check failures Signed-off-by: Alexander Watson <zredlined@gmail.com> --------- Signed-off-by: Alexander Watson <zredlined@gmail.com> |
||
|
Piotr Mlocek
|
0797fefa44 | feat(gateway): add local-domain service routing (#1101) | ||
|
alangou
|
2532687e34 |
fix(secret): Add custom derive Debug for SecretResolver to prevent secret leakage with {:?} (#1322)
Signed-off-by: Adrien Langou <alangou@nvidia.com> |
||
|
Taylor MutchandDrew Newberry
|
ba77967fc4 |
refactor(docker): split gateway/supervisor Dockerfiles and use native rust builds (#1316)
* wip
* refactor(docker): use native rust builds for split gateway/supervisor images
Drop the in-Docker BUILD_FROM_SOURCE path so both images consume only
prebuilt binaries staged natively via tasks/scripts/stage-prebuilt-binaries.sh.
This mirrors what CI does and reuses the host's cargo target cache and
sccache across rebuilds.
- Dockerfile.gateway: nvcr.io/nvidia/distroless/cc:v4.0.4 base (the 4.0.0
tag does not exist on nvcr.io; the registry uses a v prefix). GNU-linked
binary copied to /usr/local/bin.
- Dockerfile.supervisor: scratch base, static musl binary. Static linkage
lets the image stay scratch while still being executable as a Kubernetes
init container.
- skaffold.yaml: each artifact invokes tasks/scripts/docker-build-image.sh,
which stages the binary natively (cargo / cargo-zigbuild) and then builds
the image. Drops the cross-build.sh dependency from the supervisor build.
- seccomp.rs: add a local SYS_kexec_file_load constant for musl/aarch64.
libc 0.2.185 omits the symbol from its musl/aarch64 bindings, so the
supervisor's seccomp filter previously failed to compile for that target.
- architecture/build.md: describe the native-first pipeline and per-image
runtime choices.
Local validation: gateway image 101MB (was 194MB), supervisor image 21.7MB.
helm:skaffold:run deploys cleanly; the static musl supervisor binary runs
correctly in a non-glibc agent container.
* refactor(docker): tighten binary perms via --chown + 0550
Replace `COPY --chmod=755` with `COPY --chown=<user> --chmod=0550` in
the gateway and supervisor Dockerfiles. The binary is no longer
world-readable or world-executable; ownership is pinned to the runtime
user.
- Gateway uses `--chown=nvs:nvs` + `USER nvs:nvs`, matching the only
non-root user defined in `nvcr.io/nvidia/distroless/cc` (UID 1000) and
the Helm chart's `securityContext.runAsUser: 1000`, which overrides
the Dockerfile USER at runtime.
- Supervisor uses numeric `--chown=65534:65534` because the scratch base
has no `/etc/passwd` for name resolution. The supervisor image is
only consumed by the init-container copy-self path; the destination
pod's runAsUser governs execute access.
Validated by deploying to a local k3d cluster via `helm:skaffold:run`
and confirming the gateway StatefulSet reaches 1/1 Running.
* ci(gpu): repoint GPU probe image lookup at Dockerfile.gateway
The previous awk parsed `FROM <image> AS gateway` from the now-deleted
`Dockerfile.images`. The new `Dockerfile.gateway` uses an ARG with a
default (`ARG GATEWAY_BASE_IMAGE=nvcr.io/nvidia/distroless/cc:v4.0.4`)
and `FROM ${GATEWAY_BASE_IMAGE} AS gateway`, so the old script returns
nothing.
Parse the ARG default value directly so the GPU prerequisites check
keeps using the gateway base image as a `nvidia-smi` probe target.
* ci(gpu): pin GPU probe to nvcr.io/nvidia/base/ubuntu:noble
The previous probe parsed the gateway base image out of the Dockerfile,
relying on the fact that the gateway ran on `nvcr.io/nvidia/base/ubuntu`
and that NVIDIA Container Toolkit CDI injection would populate
`nvidia-smi` and the supporting libs at runtime. The new gateway base
(`nvcr.io/nvidia/distroless/cc`) lacks `ldconfig`, a populated
`/usr/bin`, and the broader filesystem layout CDI injection assumes,
so it cannot serve as a GPU probe.
Pin the probe image explicitly to the NVIDIA-managed Ubuntu base. The
probe is independent of the gateway runtime and survives future base
swaps.
* fix(e2e-gpu): pass GPU probe image via env, drop Dockerfile.images parse
The Rust e2e test `gpu_request_for_each_discovered_device_matches_plain_container`
was still parsing the deleted `Dockerfile.images` to derive its GPU probe
image, panicking with `No such file or directory` after this PR's
Dockerfile split.
Move the probe image to a single source of truth in the workflow
(`OPENSHELL_E2E_GPU_PROBE_IMAGE` env at the job level) and require the
e2e test to read it from there. No silent codebase default — the test
panics with a pointer at the workflow if the env is missing, so it
fails loudly rather than drifting from CI.
The prereq probe step in the workflow now consumes the same env, so
the probe image is declared exactly once.
* fix(docker): keep supervisor binary root-owned for rootless Podman
The Podman driver mounts the supervisor image read-only into the sandbox
container at /opt/openshell/bin and runs that container as UID 0, but
deliberately drops DAC_OVERRIDE for hardening (container.rs:419). With
--chown=65534:65534 --chmod=0550 the binary was r-xr-x--- owned by UID
65534, so the container's UID 0 fell into "other" with no read or exec
access and the supervisor crashed on start (ContainerExited code 1).
Docker and Kubernetes both retain DAC_OVERRIDE, so root could still
exec the file — which is why this regression only surfaced in the
Podman e2e job.
Drop --chown from the supervisor COPY so the binary stays root-owned.
Keep --chmod=0550: the security win was dropping world-execute, not
changing the owner. The chown bought nothing here because the container
is always UID 0 regardless of driver, but it actively broke the only
driver that drops DAC_OVERRIDE.
---------
Co-authored-by: Drew Newberry <anewberry@nvidia.com>
|
||
|
Aaron Erickson 🦞andJohn Myers
|
9ea94b645d |
fix(sandbox): rewrite messaging credential placeholders (#1286)
* fix(sandbox): rewrite credential placeholders in websocket text frames Signed-off-by: Aaron Erickson <aerickson@nvidia.com> * fix(sandbox): harden websocket credential rewrite Signed-off-by: Aaron Erickson <aerickson@nvidia.com> * feat(sandbox): add websocket l7 inspection and compression Signed-off-by: Aaron Erickson <aerickson@nvidia.com> * fix(sandbox): harden websocket upgrade validation Signed-off-by: Aaron Erickson <aerickson@nvidia.com> * test(sandbox): cover route-selected websocket upgrades Signed-off-by: Aaron Erickson <aerickson@nvidia.com> * fix(sandbox): harden websocket negotiation parsing Signed-off-by: Aaron Erickson <aerickson@nvidia.com> * test(sandbox): add websocket conformance relay matrix Signed-off-by: Aaron Erickson <aerickson@nvidia.com> * test(e2e): add websocket conformance lane Signed-off-by: Aaron Erickson <aerickson@nvidia.com> * fix(policy): support websocket incremental rules Signed-off-by: Aaron Erickson <aerickson@nvidia.com> * feat(policy): enable websocket credential rewrite updates Signed-off-by: Aaron Erickson <aerickson@nvidia.com> * fix(cli): make websocket rewrite endpoint-local Signed-off-by: Aaron Erickson <aerickson@nvidia.com> * feat(sandbox): support graphql websocket policy Signed-off-by: Aaron Erickson <aerickson@nvidia.com> * fix(policy): allow private IPs for websocket endpoints * feat(sandbox): rewrite REST credential placeholders Signed-off-by: Aaron Erickson <aerickson@nvidia.com> * refactor(sandbox): generalize credential aliases Signed-off-by: Aaron Erickson <aerickson@nvidia.com> * fix(sandbox): rewrite encoded form credentials Signed-off-by: Aaron Erickson <aerickson@nvidia.com> * fix(sandbox): close websocket policy and provider alias gaps Signed-off-by: Aaron Erickson <aerickson@nvidia.com> * fix(e2e): route websocket probe through host gateway * fix(e2e): stabilize websocket probe handshake * fix(e2e): exercise websocket probe through proxy * ci: remove websocket conformance workflow --------- Signed-off-by: Aaron Erickson <aerickson@nvidia.com> Co-authored-by: John Myers <9696606+johntmyers@users.noreply.github.com> |
||
|
Piotr Mlocek
|
5abc36c461 | feat(relay): route forwarding through ForwardTcp (#1029) | ||
|
Alexander WatsonandJohn Myers
|
1c79b2131f |
feat: agent-driven policy management MVP (#1151)
* docs(rfc): add agent-driven policy management * docs(rfc): switch policy MVP to local API * docs(rfc): clarify policy advisor skill and local logs * feat(sandbox): add agent-driven policy proposal loop * test(examples): add codex policy dogfood loop * refactor(examples): make policy demo agent-agnostic * refactor(examples): colocate policy validation harness * docs(examples): add policy demo env sample * docs(examples): use placeholder env example * feat(sandbox): wire policy.local denials to OCSF JSONL log Wires GET /v1/denials?last=N on the sandbox-local policy advisor API to read recent OCSF JSONL events from /var/log/openshell-ocsf.YYYY-MM-DD.log, filter to network/L7 denials (action_id=2, class_uid 4001/4002), and return a compact summary newest-first. Default limit is 10, capped at 100. Ran inside spawn_blocking so file I/O does not block the policy.local handler. Other cleanup: - POST /v1/proposals now uses the typed grpc_client wrapper instead of raw_client, so accepted/rejected counts surface to the agent uniformly. Wrapper return type extended to the response struct. - Drop the 'add_rule' snake_case alias in the proposal JSON; canonical form is camelCase 'addRule', matching the PolicyMergeOperation convention used elsewhere. - skills/policy_advisor.md updated to match: documents the now-real /v1/denials?last=10 endpoint and uses 'addRule' consistently. - skills.rs test asserts on the canonical 'addRule' phrase rather than the removed 'PolicyMergeOperation' substring. * feat(cli): show L7 protocol/method/path in rule get output format_endpoint() previously rendered only host:port, dropping protocol, access, and the L7 rules array. That made openshell rule get text output unable to distinguish a broad L4 grant from a method/path-scoped L7 REST rule -- exactly the distinction a developer needs at approval time. New rendering tags each endpoint with its enforcement layer and surfaces allow/deny rules: bare L4: api.example:443 [L4] L7 read-only: api.example:443 [L7 rest, access=read-only] L7 method/path: api.example:443 [L7 rest, allow PUT /v1/foo/bar] Pure display change: no proto, gateway, or behavior changes. Unit test covers all three rendering cases with synthetic fixtures. * refactor(examples): rewrite policy demo as Codex-default loop Re-shape examples/agent-driven-policy-management/ to be a single, clean end-to-end demonstration of the agent-driven policy loop. A Codex agent inside an OpenShell sandbox attempts a GitHub Contents API write, hits a structured 403 from the L7 proxy, reads the policy_advisor skill, drafts a narrow addRule proposal via http://policy.local/v1/proposals, the host auto-approves, the sandbox hot-reloads policy, and the agent's retry succeeds. Whole loop runs in roughly two minutes. Demo cleanup: - Drop .env file ceremony. Defaults resolve from gh: owner via 'gh api user --jq .login', repo defaults to 'openshell-policy-demo', token from gh auth token / GITHUB_TOKEN / GH_TOKEN. With gh auth login and codex login already done, 'bash demo.sh' Just Works. - Codex-specific. Bootstraps ~/.codex/auth.json from credentials injected by the OpenShell provider, runs codex exec --sandbox danger-full-access (OpenShell is the actual security boundary; bwrap nesting cannot create user namespaces inside the sandbox container). - Tighter narrative output: a single 'Preflight' step, a run summary banner before launch, an inline narration of what's happening inside the sandbox while we poll for the proposal (including the literal structured 403 body the agent acts on), and an OCSF trace at the end filtered to the three events that tell the story (DENY, RELOAD, ALLOW). - Replace Python heredoc templating with sed; uploads use the single-flag pattern (--upload "${PAYLOAD_DIR}:/sandbox") with files referenced at the basename-prefixed path that #952 / #1028 established. - README documents the trust model honestly: structured rule is the contract, agent rationale is a hint, prover validation badge in progress per RFC 0001. Move the deterministic no-LLM regression harness out of examples/ into e2e/policy-advisor/ -- it was a parallel demo, not an example. Same loop without the LLM, useful for iterating on the proxy and policy.local API. * style(sandbox,cli): apply rustfmt Whitespace-only fixups caught by mise run pre-commit. No functional change. * perf(examples): cap Codex reasoning at 'low' in policy demo The demo task is mechanical (one HTTP request, parse a structured 403, post a JSON proposal, retry). Codex's default high-effort reasoning roughly doubles the demo's wall time without improving outcomes; running at 'low' lands the same minimal L7 grant in roughly half the time. Override with DEMO_CODEX_REASONING=medium (or higher) to compare runs. * fix(sandbox): harden policy.local denials endpoint Three changes addressing review feedback before merging the agent-driven policy management MVP: - Distinguish "OCSF JSONL enabled, no denials" from "OCSF JSONL disabled, nothing to read." The endpoint now returns a `log_available` flag and an explanatory `note` when the log file is missing, so the in-sandbox agent can give the developer an accurate hint instead of a misleading empty list. - Stop echoing the OCSF `message` field in the per-denial summary. The proxy's denial messages can include the request path with query string (e.g., `?access_token=...`); the structured `host`/`port`/`method`/ `path`/`binary` fields carry everything the agent needs to draft a proposal, and `path` is sourced from `http_request.url.path` which already excludes the query string. - Cap `read_request_body` at a 15s timeout. Bounds slowloris-style stalls from a misbehaving in-sandbox process. The proxy listener only accepts loopback connections so practical impact is small, but this is cheap defense-in-depth. New tests cover the missing-log signal and the message-redaction guarantee. * fix(examples): redact tokens in agent log tail and validate DEMO_FILE_DIR Two small hardening passes on the policy management demo: - `fail()` now pipes the agent log tail through a redactor that masks the GitHub token and Codex credential triple before printing. Codex itself is well-behaved about not echoing the token, but a misbehaving tool call could leak it; this is a final safety net before the log hits the developer's terminal (and any clipboard or chat history that follows). - `validate_env` now regex-checks DEMO_FILE_DIR with the same allow-list the other path-shaped variables use. The value is interpolated through sed with `|` as the delimiter when rendering the agent task; rejecting unsupported characters keeps the templating predictable and stops a user-supplied value from breaking out into a shell context. * refactor(sandbox): centralize policy.local routes and skill path Addresses review feedback that the deny body's `next_steps` array and the route table could drift apart. The route paths and skill location now live as `pub const`s in `policy_local.rs` and feed both: - the dispatcher in `route_request` that matches against them - a new `agent_next_steps()` helper that builds the JSON the L7 deny body embeds `l7/rest.rs::deny_response_body` calls `policy_local::agent_next_steps()` instead of inlining the array, so adding or renaming a route is a one-line change in `policy_local.rs` and the agent contract follows automatically. * feat(sandbox): switch /v1/denials to shorthand log pass-through Previously /v1/denials parsed `/var/log/openshell-ocsf.*.log` (OCSF JSONL) and returned structured per-event objects. JSONL is opt-in via `ocsf_json_enabled`, so the endpoint returned an empty list with a "log not enabled" hint by default — agents had to navigate a setup step before the inspect-recent-denials guidance was useful. Switch to reading the shorthand log at `/var/log/openshell.*.log`, which is always-on and the same human-readable format `openshell logs` displays. The endpoint now returns raw shorthand lines (newest first) — the agent reads them directly, no field parsing. Tradeoffs: - Removes the JSONL-on-by-default debate: shorthand is already on, no defaults change. - Updating shorthand is a single-file change in this repo; no schema rev needed when we want to add fields. Implementation: - `read_recent_denial_lines` walks shorthand log files newest-first, filters lines with ` OCSF ` AND ` DENIED ` (the OCSF action label, uppercase, space-bounded). - `collect_shorthand_log_files` matches `openshell.<date>.log`; the trailing dot in `SHORTHAND_LOG_PREFIX = "openshell."` excludes `openshell-ocsf.<date>.log` so JSONL-on doesn't bleed into responses. - 4096-byte cap per surfaced line as defense against pathological inputs. - Skill doc updated to reflect that `/v1/denials` returns raw shorthand lines, not structured fields. Defense-in-depth on query-string secrets: - `redact_query_strings` strips `?<query>` to `?[redacted]` from each surfaced line. The L7 relay path emits OCSF events using `redacted_target` (secret-placeholder redaction), but the FORWARD deny path in `proxy.rs` populates `OcsfUrl::new("http", host, path, port)` and `.message(...)` with the raw request path — query string included. Stripping queries at the consumer guards `/v1/denials` regardless of whether the upstream emit sites are tightened. The on-disk log is not rewritten by this change; that is a separate hardening task tracked for the FORWARD path emit sites in proxy.rs. - `truncate_at_char_boundary` is UTF-8 safe; redaction runs before truncation so a cut cannot slice mid-secret. Tests: - `recent_denials_returns_newest_first_from_shorthand_lines` covers the happy path with mixed allowed/denied/non-OCSF lines. - `recent_denials_skips_jsonl_log_files` confirms JSONL files don't surface even if present. - `recent_denials_truncates_pathological_lines` covers the cap. - `is_ocsf_denial_line_filters_correctly` covers the line-level filter. - `redact_query_strings_removes_query_from_url_token` and `redact_query_strings_removes_query_in_reason_tag` cover the redaction in both URL token and `[reason:...]` contexts. - `truncate_at_char_boundary_does_not_panic_on_multibyte` covers the UTF-8 safety. * chore(sandbox): align proto inits with main's L7 GraphQL additions Post-rebase fixups after #1083 (GraphQL L7 inspection) landed on main and introduced new fields on the proto types this branch constructs: - `crates/openshell-sandbox/src/l7/relay.rs`: two `deny_with_redacted_target` call sites (REST and GraphQL relay deny paths) now pass the `DenyResponseContext` argument that `rest::send_deny_response` expects. Both sites pass `host`, `port`, and `binary` from the existing `L7EvalContext`, matching the pattern used at the primary deny site. - `crates/openshell-sandbox/src/policy_local.rs`: `L7Allow`, `L7DenyRule`, and `NetworkEndpoint` proto initializers now populate the new GraphQL and path-scoping fields with empty defaults. Agent-authored proposals via `policy.local` target REST/SQL/L4 today; GraphQL operation matching is set on the gateway side or via direct YAML, so empty defaults are correct here. No behavior change. `cargo test -p openshell-sandbox --lib` (650 tests) and `cargo clippy -p openshell-sandbox --lib --tests -- -D warnings` clean. * feat(sandbox): gate agent policy proposals behind opt-in feature flag The agent-driven policy proposal surface delivered by this PR (skill install, `policy.local` API, `next_steps` array on L7 deny bodies) is now opt-in via the new `agent_policy_proposals_enabled` setting. Default false. Same shape as `providers_v2_enabled`: registered in `openshell-core::settings`, sandbox-level, hot-toggleable via the existing settings poll loop. Why: the surface is a novel agent-controlled mutation point in every sandbox. The per-proposal developer approval gate is a correctness control, but it doesn't address "should this sandbox have an agent-authoring API at all" — compliance teams may want that question closed. The flag is the second gate. Implementation: - New registry entry + `AGENT_POLICY_PROPOSALS_ENABLED_KEY` constant in `openshell-core::settings`. - `lib.rs`: process-wide `OnceLock<Arc<AtomicBool>>` mirroring the `OCSF_CTX` pattern. `agent_proposals_enabled()` is the single read point. - Initial settings fetch added to `run_sandbox` so skill install honors the flag at startup (not just on the poll loop's first tick). - Skill install in `run_sandbox` is gated on the flag. - `policy_local::route_request` returns `404 feature_disabled` for all routes when the flag is off — including the otherwise-public `current_policy` and `denials` routes. When the surface is off it's off entirely. - `policy_local::agent_next_steps` returns an empty array when the flag is off so deny bodies don't advertise routes that 404. - Poll loop updates the atomic on each tick, lazily installs the skill on a false→true transition (no claw-back on true→false; stale skill on disk is harmless because route + next_steps gate on the live atom). Tests: - Shared `test_helpers::ProposalsFlagGuard` mutex+atomic guard for the process-wide flag, used across `policy_local::tests` and `l7::rest::tests`. - New: `agent_next_steps_returns_empty_when_flag_off`, `agent_next_steps_returns_full_array_when_flag_on`, `route_request_returns_feature_disabled_when_flag_off`. - Updated existing tests that exercise the deny body or the route dispatcher to set the flag on first. - Full sandbox lib test suite: 653 pass, clippy clean. Demo and e2e: - `examples/agent-driven-policy-management/demo.sh` and `e2e/policy-advisor/test.sh` now snapshot the prior global value of the setting, set it to true before sandbox creation (so the supervisor's initial poll picks it up), and restore on exit (delete if previously unset, otherwise write the prior value back). Docs: - RFC 0001 MVP-implementation note documents the flag, default, and intended soft-launch posture. * test(policy-advisor): require proposal opt-in for e2e * refactor(sandbox): group policy poll loop state * test(e2e): isolate Kubernetes user namespace test --------- Co-authored-by: John Myers <9696606+johntmyers@users.noreply.github.com> |
||
|
John T. Myers
|
1d3b741ee3 |
feat(providers): support sandbox provider attach lifecycle (#1242)
* feat(providers): support sandbox provider attach lifecycle Closes #1171 Adds sandbox provider list, attach, and detach API/CLI support while keeping provider policy and credential resolution derived from current sandbox attachments. * fix(providers): refresh sandbox provider credentials Adds provider environment revisions and generation-scoped sandbox credential snapshots so future SSH and exec launches pick up provider attach, detach, and credential updates without mutating already-running processes. Also blocks provider deletion while attached to prevent stale sandbox provider references. * fix(providers): serialize sandbox object mutations * test(providers): cover sandbox provider attach lifecycle * test(providers): accept versioned credential placeholders |
||
|
Drew Newberry
|
62619eefc7 |
fix(docker): use supervisor image entrypoint path (#1259)
Signed-off-by: Drew Newberry <anewberry@nvidia.com> |
||
|
Drew Newberry
|
70a0f6c547 | refactor(cli): remove gateway lifecycle management (#1221) | ||
|
Taylor Mutch
|
b1cba79620 |
fix(sandbox): add copy-self subcommand for scratch-image init container (#1208)
* fix(sandbox): add copy-self subcommand for scratch-image init container The supervisor image is built FROM scratch and contains only the openshell-sandbox binary. The Kubernetes side-load init container previously invoked `sh -c "cp ..."`, which failed with `executable file 'sh' not found in $PATH` because the scratch image has no shell. Add a `copy-self <DEST>` subcommand to the supervisor binary that resolves its own executable, copies it to the destination path, and sets mode 0755. Update the Kubernetes driver init container to invoke this subcommand directly. Mirrors argoexec's emissary self-copy pattern. * fix(docker): use ubuntu base for supervisor image `FROM scratch` cannot exec the dynamically linked supervisor binary (no `/lib/ld-linux-*.so.*`), so the Kubernetes side-load init container hit `exec /openshell-sandbox: no such file or directory` even after the `copy-self` subcommand replaced the broken `sh -c` command. Switch the supervisor stage to `nvcr.io/nvidia/base/ubuntu:noble-20251013` — the same base the gateway image already uses — so glibc and the dynamic loader are present. Docker/Podman drivers continue to work unchanged (they extract or volume-mount the binary into agent containers that already have glibc); the Kubernetes init container now succeeds end-to-end. Verified on a k3d cluster: init container `openshell-supervisor-install` exits 0 on first try, binary lands at `/opt/openshell/bin/openshell-sandbox` (mode 0755), and the agent container reaches Ready. * docs(docker): refresh supervisor target description The file header still described the supervisor image as `FROM scratch, binary only`. Update it to match the current Ubuntu-base supervisor stage. |
||
|
John T. Myers
|
6b21804258 |
feat(policy): add GraphQL L7 inspection (#1083)
Support GraphQL L7 policies |
||
|
Drew Newberry
|
08001ca616 | fix(docker): harden supervisor startup and gateway routing (#1128) | ||
|
jtoelke2
|
32857eb650 |
fix(helm): grant node read access for GPU capacity checks (#1106)
* fix(helm): grant node read access for GPU capacity checks Signed-off-by: Jonas Toelke <jtoelke@nvidia.com> * ci: add WSL GPU failure diagnostics Signed-off-by: Jonas Toelke <jtoelke@nvidia.com> * fix(gpu): bump device plugin for WSL CDI * fix(sandbox): allow WSL GPU paths in Landlock Allow /dev/dxg and /usr/lib/wsl as GPU baseline paths so WSL CDI GPU sandboxes can initialize NVML. Native Linux skips these entries when the paths do not exist. * ci: remove temporary WSL GPU diagnostics --------- Signed-off-by: Jonas Toelke <jtoelke@nvidia.com> |
||
|
Mesut Oezdil
|
76093e78a3 |
refactor(sandbox): remove dead relay_response_to_client wrapper (#1125)
relay_response_to_client had no callers; all call sites use relay_response directly. The allow(dead_code) comment on parse_target_query was also misleading since proxy.rs calls it from production code, so that annotation is removed along with the incorrect comment. |
||
|
John T. Myers
|
5d0a44cbac |
fix(sandbox): invalidate stale l7 tunnels after reload (#1118)
Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> |
||
|
ddurst-nvidia
|
4c2564892b |
fix(sandbox): accept ENOENT in drop_privileges identity lookup tests (#1123)
User::from_name / Group::from_name may return Ok(None), producing our explicit "Sandbox … not found" errors, or Err when NSS fails first with ENOENT ("ENOENT: No such file or directory") before a definitive unknown-name result.
Accept either wording so nonexistent user/group tests stay meaningful across environments.
Signed-off-by: ddurst <267424412+ddurst-nvidia@users.noreply.github.com>
|
||
|
Mrunal Patel
|
084505425b |
feat(auth): add OIDC/Keycloak authentication with RBAC and scope-based permissions (#935)
* feat(auth): add OIDC/Keycloak authentication with RBAC Add OAuth2/OIDC authentication to the gateway server with role-based access control, CLI login flows, and full deployment plumbing. Server: JWT validation against configurable OIDC issuer (oidc.rs), JWKS key caching with TTL and rotation handling, method classification (unauthenticated/sandbox-secret/dual-auth/bearer), identity extraction with provider-agnostic Identity type, and RBAC enforcement via AuthzPolicy with configurable admin/user roles and auth-only mode. CLI: browser-based Authorization Code + PKCE flow, Client Credentials flow for CI/automation, token storage with refresh, gateway add/login/ logout commands, OIDC bearer token injection over mTLS transport, discovery endpoint for auto-configuration. Security: sandbox-secret scope restriction on UpdateConfig (policy sync only), anti-spoofing header stripping, dual-auth fallthrough from sandbox-secret to Bearer token. Deployment: OIDC config wired through DeployOptions, Docker env vars, Helm values/templates, HelmChart manifest, cluster-entrypoint.sh, and bootstrap scripts. Keycloak dev server script with pre-configured realm (test users, roles, PKCE client, CI client). Tested with Keycloak. The roles claim path and role names are configurable to support other OIDC providers. * feat(auth): add OAuth2 scope-based fine-grained permissions Add opt-in scope enforcement on top of existing OIDC role-based access control. When --oidc-scopes-claim is set, the server extracts scopes from the JWT and checks them per-method against an exhaustive scope map. Scopes: sandbox:read, sandbox:write, provider:read, provider:write, config:read, config:write, inference:read, inference:write, and openshell:all (wildcard). Methods not in the scope map require openshell:all. Scopes layer on top of roles and cannot escalate privilege. Auth-only mode (empty role names) still enforces scopes when enabled. Server: scopes_claim in OidcConfig, scope extraction from JWT (space-delimited and JSON array formats), standard OIDC scope filtering, scope check in AuthzPolicy after role check. CLI: --oidc-scopes on gateway add/start stored in metadata and consumed by gateway login, --oidc-scopes-claim on gateway start forwarded to server, scopes parameter in browser and client credentials OAuth2 flows with openid deduplication. Deployment: oidc_scopes_claim wired through DeployOptions, docker.rs, Helm, bootstrap scripts, and cluster entrypoint. Keycloak: realm config updated with built-in OIDC scopes and 9 OpenShell client scopes as optional on openshell-cli and openshell:all as default on openshell-ci. * fix(auth): address branch review findings Add GetInferenceBundle to sandbox-secret methods so sandbox inference route refresh works under OIDC. Make GetSandboxConfig dual-auth so CLI users can read sandbox settings with Bearer tokens. Preserve OIDC gateway metadata on restart — a bare gateway start without --oidc-* flags no longer erases the stored OIDC registration. Document CI client ID requirement (openshell-ci vs openshell-cli) in the testing guide. Add security note about auth-only mode blast radius for GitHub Actions. * fix(auth): complete review findings for OIDC auth boundary Move OpenShell/GetSandboxConfig from sandbox-secret-only to dual-auth so CLI users can read sandbox settings with Bearer tokens while sandbox supervisors continue using the shared secret. Add sandbox secret interceptor to the inference bundle fetch path so GetInferenceBundle works under OIDC-enabled gateways. Extract shared interceptor constructor to avoid duplication. Add GetSandboxConfig to the config:read scope map so scope enforcement applies consistently when scopes are enabled. Refactor OIDC metadata preservation into apply_oidc_gateway_metadata() with explicit resume semantics — only preserve existing OIDC metadata on real resume paths, not on fresh deployments. Update architecture docs and testing guide to reflect the corrected method classifications and add new test coverage for interceptor injection, scope requirements, metadata preservation, and dual-auth classification. * refactor(auth): use oauth2 crate for CLI OIDC flows Replace hand-written PKCE generation, authorization URL construction, token exchange, client credentials, and token refresh with the oauth2 crate's typed API. Eliminates sha2, hex, and getrandom dependencies from the CLI. The custom urlencoded() helper and manual form POST logic are replaced by BasicClient methods with proper type-state safety. Discovery and the callback server remain custom since the oauth2 crate does not provide OIDC discovery or a localhost redirect listener. * refactor(auth): move server auth modules into auth/ directory Group oidc.rs, authz.rs, identity.rs, and the auth HTTP endpoints under src/auth/ module directory. No behavioral changes. auth/mod.rs — module root, re-exports HTTP router auth/oidc.rs — JWT validation, JWKS caching, method classification auth/authz.rs — role and scope authorization policy auth/identity.rs — provider-agnostic Identity type auth/http.rs — /auth/connect and /auth/oidc-config endpoints * fix(auth): use RequestBody auth type for client credentials flow The oauth2 crate defaults to BasicAuth (HTTP Basic header) but Keycloak and most OIDC providers expect client_secret_post (credentials in the request body). Set AuthType::RequestBody explicitly to match the pre-refactor behavior. Also re-export Identity, IdentityProvider, and JwksCache from the auth module so ServerState's public API remains nameable by external consumers. * fix(auth): forward OPENSHELL_OIDC_SCOPES through cluster bootstrap Pass --oidc-scopes to gateway start so the metadata includes requested scopes after cluster bootstrap. Without this, users had to manually edit metadata.json to set scopes for gateway login. Usage: OPENSHELL_OIDC_SCOPES="openshell:all" mise run cluster * test(auth): add OIDC e2e tests for RBAC, scopes, and client credentials Add 10 end-to-end tests covering OIDC authentication against a live K3s cluster with Keycloak: RBAC (5 tests): admin can create providers, user cannot, user can list sandboxes, unauthenticated requests rejected, health probe works without auth. Scopes (4 tests): sandbox-scoped token can list sandboxes but not providers, openshell:all grants full access, no-scopes token denied. Client credentials (1 test): CI token via client_credentials grant. Tests are opt-in via OPENSHELL_E2E_OIDC=1 and OPENSHELL_E2E_OIDC_SCOPES=1 env vars. They derive the Keycloak URL from gateway metadata to match the server's configured issuer. Run with: OPENSHELL_E2E_OIDC=1 OPENSHELL_E2E_OIDC_SCOPES=1 \ PYTHONPATH=python uv run pytest e2e/python/oidc/ -v * fix(docs): fix markdown lint errors in OIDC architecture docs Add blank lines before lists and fenced code blocks to satisfy markdownlint MD031 and MD032 rules. |
||
|
Piotr Mlocek
|
ee2de81bc9 | fix(sandbox): preserve encoded slash policy from proto (#1073) | ||
|
John T. Myers
|
0914f3f4f7 |
fix(sandbox): log L7 parse denials (#1072)
Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> |
||
|
Drew Newberry
|
24724742a8 | ci(rust): enforce -D warnings on clippy (#1008) | ||
|
vince-brisebois
|
2adddaa6b2 |
feat: Adding qemu vm driver support with GPU pass-through (#992)
* Adding qemu vm driver support with GPU pass-through Signed-off-by: Vincent Caux-Brisebois <vcauxbrisebo@nvidia.com> * Add GPU rootfs variant, harden VFIO binding, and fix networking and supervisor reliability issues discovered during GPU VM bring-up. Signed-off-by: Vincent Caux-Brisebois <vcauxbrisebo@nvidia.com> --------- Signed-off-by: Vincent Caux-Brisebois <vcauxbrisebo@nvidia.com> |
||
|
John T. MyersandJohn Myers
|
2646b8c6c8 |
fix(sandbox): deny ambiguous socket ownership (#958)
Signed-off-by: John Myers <johntmyers@users.noreply.github.com> Co-authored-by: John Myers <johntmyers@users.noreply.github.com> |
||
|
Adam Miller
|
d44d8a1e27 |
feat: Openshell driver podman (#904)
* feat(podman): add Podman compute driver for rootless sandbox management Adds openshell-driver-podman, a new compute driver that manages OpenShell sandboxes as rootless Podman containers via the Podman REST API over a Unix socket. Enables local workstation sandboxes without Kubernetes. Driver features: - Bridge networking with ephemeral host-port mapping for rootless SSH reachability - Named volumes for workspace storage, Podman native health checks, GPU via CDI - Supervisor binary sideloaded via image volume mount (BYOC-compatible) - SSH handshake secret injected via Podman secrets API (not plaintext env) - Typed ContainerSpec structs, input validation, and path-traversal guards - Cgroups v2 required; fails fast on v1 hosts - Bounded event stream buffer; watch stream reconnection handled by server watch_loop - Graceful shutdown and standalone driver binary with gRPC bridge Rootless-specific fixes: - Skip drop_privileges when user namespace lacks SETUID/SETGID/DAC_READ_SEARCH caps - Add /run/netns tmpfs mount for ip netns in rootless containers - Use secret_env map (not secrets array) for env-var injection in libpod API - Resolve SSH endpoint to 127.0.0.1:<host_port> instead of unreachable bridge IP Server/sandbox hardening: - Split loopback and link-local SSRF gates; Podman/VM drivers allow loopback - Close SSRF bypass in SSH tunnel Host path by resolving DNS before connecting - Prevent OPENSHELL_* env var override by user-supplied spec environment maps - Disable SQLite pool idle_timeout/max_lifetime for in-memory databases - Emit deleted_event on 404-during-inspect instead of regressing sandbox phase - Key delete cleanup by stable sandbox_id to survive container label drift CLI fixes: - Restore --name as a named flag on sandbox create (not positional) - Fix exec command arg parsing to not consume sandboxed-command flags - Propagate SSH verbosity via OPENSHELL_SSH_LOG_LEVEL Build tooling: - Add tasks/scripts/container-engine.sh: auto-detects Podman or Docker, exposes unified ce_* helpers; all build/cluster/VM scripts updated to use it - Add docker:build:supervisor mise task for standalone supervisor image - Add openshell-driver-podman to Dockerfile.images pre-fetch/build stages - Add e2e/rust/e2e-podman.sh and e2e:podman mise task for full lifecycle testing Signed-off-by: Adam Miller <admiller@redhat.com> * fix(driver-podman): derive grpc endpoint from server bind port When a user starts the gateway on a non-default port (e.g. --port 8081), sandbox containers were receiving OPENSHELL_ENDPOINT pointing at the default port 8080. The driver's auto-detection fallback read OPENSHELL_BIND_ADDRESS from the environment, which was stale or unset, and fell back to DEFAULT_SERVER_PORT. Add gateway_port to PodmanComputeConfig and thread config.bind_address.port() from the server into the driver so the fallback uses the actual listening port. Remove the OPENSHELL_BIND_ADDRESS env var read and the extract_port_from_bind_address helper which are no longer needed. Add --gateway-port / OPENSHELL_GATEWAY_PORT to the standalone driver binary for parity when the driver is run outside the embedded server path. Signed-off-by: Adam Miller <admiller@redhat.com> * fix(driver-podman): address PR feedback on env test safety and cluster DNS docs Replace hand-rolled unsafe TempEnvVar RAII guard with temp_env::with_vars and a static ENV_LOCK mutex, fixing a data race in parallel test execution. The prior safety comment incorrectly claimed Cargo runs tests single-threaded. Update debug-openshell-cluster skill to accurately document the DNS proxy strategy (setup_dns_proxy + public DNS fallback) and clarify the separation between cluster DNS and sandbox agent DNS enforcement. Signed-off-by: Adam Miller <admiller@redhat.com> * fix(e2e): resolve CI failures in auth timeout, test harness, and formatting - Short-circuit browser_auth_flow when OPENSHELL_NO_BROWSER=1 instead of waiting the full 120s AUTH_TIMEOUT for a callback that never arrives - Add timeout to SandboxGuard::create() and create_with_upload() to prevent indefinite hangs (matches create_keep() which already had one) - Add missing '--' separator in no_proxy test before command args - Add #![cfg(feature = "e2e")] gate to sandbox_lifecycle.rs - Run cargo fmt on openshell-driver-podman - Refine cluster DNS docs for Podman in debug-openshell-cluster skill Signed-off-by: Adam Miller <admiller@redhat.com> * refactor(server): remove allows_loopback_endpoints from ComputeRuntime SSRF protection is now handled at the network and proxy layers (openshell-core net.rs, openshell-sandbox proxy.rs) rather than requiring per-driver flags on ComputeRuntime. Update architecture docs to reflect supervisor relay SSH transport and add rootless networking deep-dive. Signed-off-by: Adam Miller <admiller@redhat.com> --------- Signed-off-by: Adam Miller <admiller@redhat.com> |
||
|
John T. MyersandJohn Myers
|
7f8e2109e3 |
fix(sandbox): route console logs to stderr (#949)
Signed-off-by: John Myers <johntmyers@users.noreply.github.com> Co-authored-by: John Myers <johntmyers@users.noreply.github.com> |
||
|
Tinson Lai
|
2f8e8ac311 |
fix(sandbox): inject GIT_SSL_CAINFO so git clone trusts the sandbox CA (#918)
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 #790 Fixes NVIDIA/NemoClaw#2270 Fixes NVIDIA/NemoClaw#1828 Signed-off-by: Tinson Lai <tinsonl@nvidia.com> |
||
|
John T. Myers
|
f954e5927a |
fix(sandbox): resolve sandbox host aliases in SSRF checks (#912)
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> |
||
|
John T. Myers
|
e28ca07863 |
fix(sandbox): preserve explicit read-only baseline paths (#910)
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> |
||
|
Piotr Mlocek
|
a6d45528c1 | feat(server,sandbox): supervisor-initiated SSH connect and exec over gRPC-multiplexed relay (#867) | ||
|
John T. Myers
|
c960d480fd |
fix(sandbox): canonicalize HTTP request-targets before L7 policy evaluation (#878)
* 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.
|
||
|
John T. Myers
|
8a813aba49 |
fix(sandbox): apply supervisor seccomp prelude (#891)
* fix(sandbox): apply supervisor seccomp prelude Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * fix(sandbox): delay supervisor prelude until bootstrap --------- Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> |
||
|
John T. Myers
|
40e9bf6feb |
feat(policy): add incremental sandbox policy updates (#860)
* feat(policy): add incremental sandbox policy updates Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * docs(policies): expand incremental update guidance Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * feat(policy): audit incremental updates in gateway logs * docs(policy): quote glob specs in shell examples --------- Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> |
||
|
Piotr Mlocek
|
e39bb38040 | test(sandbox): fix flaky arm64 procfs binary_path tests (#881) | ||
|
mjamiv
|
ae7e901000 |
fix(sandbox): strip " (deleted)" suffix from unlinked /proc/<pid>/exe paths (#844)
* 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>
|