* feat(cli): add JSON/YAML output format to provider list command
Closes#1745
Add -o/--output flag support (json, yaml, table) to openshell provider list
by migrating it to use the existing generic output formatter from #1750.
This follows the established pattern used by sandbox list, gateway list,
and provider list-profiles commands.
Changes:
- crates/openshell-cli/src/run.rs: Add provider_to_json() helper that maps
Provider proto fields to JSON, including only credential keys (not values)
for security. Add output parameter to provider_list() and insert early-return
for structured formats.
- crates/openshell-cli/src/main.rs: Add output field to ProviderCommands::List
with conflicts_with constraints between names and output flags. Update
invocation to pass output parameter.
- crates/openshell-cli/tests/provider_commands_integration.rs: Update
provider_list() call to include output parameter.
Security: The provider_to_json() helper only exposes credential keys, never
credential values, preventing CWE-532 (insertion of sensitive information
into output).
* refactor(cli): expose only config keys in provider list JSON/YAML output
Update provider_to_json() to only include config keys (not values) in
structured output, matching the security model used for credentials.
This prevents potential exposure of sensitive configuration values.
Changed field name from "config" to "config_keys" to clarify that only
keys are included, consistent with "credential_keys" field naming.
* test(cli): add comprehensive tests for provider list JSON/YAML output
Add three layers of test coverage for provider list structured output:
1. Unit tests in run.rs for provider_to_json() (8 tests):
- Core field validation (id, name, type)
- Security: Credential keys exposed, values hidden
- Security: Config keys exposed, values hidden
- Metadata field inclusion/omission logic
- Empty field omission behavior
- Credential expiration times
2. CLI parsing tests in main.rs (4 tests):
- JSON/YAML output format acceptance
- Conflicts between --names and -o flags
3. Integration tests in provider_commands_integration.rs (3 tests):
- JSON output execution with populated provider
- YAML output execution with populated provider
- Empty provider list edge case
Security focus: Multiple tests verify that credential and config VALUES
are never exposed in JSON/YAML output, only keys. This prevents CWE-532
(insertion of sensitive information into output).
All 778 existing unit tests continue to pass, plus 15 new tests added.
* feat(cli): format created_at as human-readable datetime in provider list JSON/YAML
Update provider_to_json() to format created_at_ms as a human-readable
datetime string using format_epoch_ms(), matching the pattern used by
sandbox_to_json().
Changes:
- Field name: created_at_ms → created_at
- Value format: Raw milliseconds → "YYYY-MM-DD HH:MM:SS"
- Implementation: Use format_epoch_ms(meta.created_at_ms)
This change:
- Aligns provider list output with sandbox list output for consistency
- Makes JSON/YAML output more human-readable for interactive use
- Follows the established pattern across all *_to_json() functions
Breaking change: Field renamed and type changed (number → string).
However, this feature was just added in PR #1830 and hasn't been
released yet, so no users are affected.
Example output:
Before: {"created_at_ms": 1234567890000}
After: {"created_at": "2009-02-13 23:31:30"}
Tests updated:
- provider_to_json_includes_metadata_fields_when_present: Assert formatted string
- provider_to_json_omits_zero_metadata_fields: Update field name
- provider_to_json_formats_created_at_as_human_readable: New test validating format
* docs(cli): document JSON/YAML output format for provider list
Add documentation for the -o/--output flag on provider list command,
following the established pattern from sandbox list documentation.
Changes:
- Add examples showing -o json and -o yaml usage
- Explain output structure (metadata, credential keys, config keys)
- Emphasize security design (keys-only exposure, never values)
- Cross-reference existing Credential Injection section
The documentation matches the feature added in a26a6341 and refined
in subsequent commits (509db497, a5f59ce9).
Signed-off-by: Jeff MAURY <jmaury@redhat.com>
---------
Signed-off-by: Jeff MAURY <jmaury@redhat.com>
* feat(bootstrap): add system gateway registry for installer defaults
Adds a read-only installer-seeded gateway registry that the CLI consults after per-user gateway config. The registry uses the same layout as per-user config with `active_gateway` at the root and `gateways/<name>/metadata.json` beneath it. By default the system config root is `/etc/openshell`, while `OPENSHELL_SYSTEM_GATEWAY_DIR` remains available as an override for packages that need a different location. User-managed gateways continue to shadow installer entries on name collision.
Originally-authored-by: Mark Shuttleworth <mark@ubuntu.com>
Signed-off-by: Alex Lewontin <alex.lewontin@canonical.com>
* feat(cli): show gateway config source in list and term
Expose whether a gateway registration comes from user or system config in `openshell gateway list`, the TUI gateway pane, and list JSON output. The CLI also refuses to remove system-managed registrations and the smoke tests cover the new list output.
Signed-off-by: Alex Lewontin <alex.lewontin@canonical.com>
* fix(bootstrap): preserve user shadowing on invalid metadata
Signed-off-by: Alex Lewontin <alex.lewontin@canonical.com>
* fix(cli): keep system fallback out of rollback state
* docs(gateway): describe system config fallback layout
* test(bootstrap): cover system gateway last_sandbox persistence
* test(gateway): cover system-only removal rejection
* fix(bootstrap): validate gateway names before path joins
---------
Signed-off-by: Alex Lewontin <alex.lewontin@canonical.com>
* feat(providers): support SPIFFE-backed token grants
Add provider profile token_grant metadata and expand endpoint-specific
dynamic credentials so sandbox supervisors can request SPIFFE JWT-SVIDs,
exchange them with an OAuth-style token endpoint, cache returned access
tokens, and inject bearer tokens into matching HTTP requests.
Wire Kubernetes and Helm deployments to mount the provider SPIFFE Workload
API socket into sandbox pods for token grant exchange.
Signed-off-by: Taylor Mutch <taylormutch@gmail.com>
Signed-off-by: Gordon Sim <gsim@redhat.com>
* test(examples): add SPIFFE token grant demo
Add a reusable alpha/beta demo that deploys a SPIFFE-verifying token issuer
and protected services, imports a token-grant provider profile, creates a
sandbox, and verifies endpoint-specific bearer tokens.
The script leaves Kubernetes workloads in place, deletes sandboxes through
openshell unless KEEP_SANDBOX=1, and prints protected service logs as proof
of life.
Signed-off-by: Taylor Mutch <taylormutch@gmail.com>
* fix(providers): harden SPIFFE token grants
* fix(providers): harden dynamic token grants
Signed-off-by: Taylor Mutch <taylormutch@gmail.com>
* fix(providers): harden token grant handling
---------
Signed-off-by: Taylor Mutch <taylormutch@gmail.com>
Signed-off-by: Gordon Sim <gsim@redhat.com>
BREAKING CHANGE: GPU sandbox mode is no longer inferred from sandbox image names. Users must pass --gpu to request GPU resources.
Signed-off-by: Evan Lezar <elezar@nvidia.com>
* feat(telemetry): add build-time option to compile out telemetry
Gate anonymous telemetry emission behind a default-on `telemetry` Cargo
feature in openshell-core. The data model (enums, validation, emit_*/enabled*
signatures) stays always-compiled, while the endpoint, HTTP client, queue, and
emission code are feature-gated. With the feature off, enabled() returns false
and emit_* are no-ops, so dependent crates compile unchanged and no telemetry
endpoint, HTTP client, or emission code is included in the binary.
chrono and reqwest become optional dependencies of openshell-core, dropped from
its dependency graph when telemetry is disabled.
Thread the switch through the workspace: every crate depends on openshell-core
with default-features = false, and the default-on `telemetry` passthrough lives
on the binary crates that emit or collect telemetry (openshell-server,
openshell-sandbox, openshell-driver-vm). In-process drivers inherit it via
resolver v2 feature unification.
Build a telemetry-free binary with, e.g.:
cargo build --release -p openshell-server --no-default-features
The runtime OPENSHELL_TELEMETRY_ENABLED switch is unchanged for default builds.
Signed-off-by: Russell Bryant <russell.bryant@gmail.com>
* ci(telemetry): guard that telemetry can be compiled out
Add tasks/scripts/verify-telemetry-compiled-out.sh, which inspects a built
binary for telemetry markers (the telemetry endpoint host and client ID) that
exist only when emission code is compiled in. The rust:verify:telemetry-off
mise task builds the gateway with default features (positive control: markers
must be present, so the absent checks can never be silently vacuous) and with
--no-default-features (markers must be absent), and checks the
--no-default-features sandbox binary as well.
Wire the task into the Rust branch-checks job so a regression that reintroduces
telemetry code into a --no-default-features build fails CI.
Signed-off-by: Russell Bryant <russell.bryant@gmail.com>
---------
Signed-off-by: Russell Bryant <russell.bryant@gmail.com>
* fix(cli): fall back to regular upload when git filtering excludes all files
When uploading a directory whose contents are entirely excluded by
.gitignore, git_sync_files returns an empty file list. The git-aware
upload path silently accepted this and printed "Upload complete" without
transferring any files. Now both call sites (direct CLI handler and
sandbox_upload_plan) check for an empty file list and fall through to the
regular, unfiltered upload path instead.
Closes#1778
Signed-off-by: Russell Bryant <rbryant@nvidia.com>
* test(e2e): add regression test for gitignored upload fallback (#1778)
Reproduces the reported workflow: download a file from a sandbox into a
local git repo where the target directory is gitignored, then re-upload
that directory without --no-git-ignore. Before the fix the upload
silently succeeded with zero files transferred.
Signed-off-by: Russell Bryant <rbryant@nvidia.com>
* fix(cli): warn on stderr when gitignore fallback triggers
Address PR review: print a notice when .gitignore filtering excludes
all files and the CLI falls back to unfiltered upload. Add a
GitFilteredEmpty variant to SandboxUploadPlan so the caller can
distinguish this case and emit a visible warning. Document the
default gitignore filtering and fallback behavior in manage-sandboxes.
* fix(cli): warn on stderr for direct upload gitignore fallback
The SandboxCommands::Upload path in main.rs had the same silent
fallback as sandbox_create. Restructure the git_sync_files check to
emit the same .gitignore warning before falling back to unfiltered
upload. Assert the warning in the E2E regression test.
* docs(sandboxes): fix misleading upload fallback guidance
Replace inaccurate "explicit file list" advice with actionable
alternatives: --no-git-ignore, uploading outside the work tree,
or git force-adding the files.
* fix(cli): collapse nested ifs to satisfy clippy::collapsible_if
* style(cli): apply rustfmt to main.rs
---------
Signed-off-by: Russell Bryant <rbryant@nvidia.com>
* feat(cli): add --env flag to sandbox create and exec
Add `--env KEY=VALUE` (repeatable) to `openshell sandbox create` and
`openshell sandbox exec`, wiring user-specified environment variables
into the existing `SandboxSpec.environment` and
`ExecSandboxRequest.environment` protobuf fields which were previously
only accessible via the SDK/gRPC API.
Fix a bug where user-specified environment variables set via
`SandboxSpec.environment` were silently dropped inside sandbox SSH
sessions. The SSH handler's `apply_child_env()` calls `env_clear()` to
strip supervisor-internal secrets before spawning user shells, but this
also wiped container-level env vars including user-specified ones. This
was introduced in 5fd4885a (2026-02-26, "feat(sandbox): VS Code
Remote-SSH support with platform detection fix") which added
`env_clear()` to fix a VS Code platform detection timeout caused by
leaked supervisor env vars.
The fix introduces an `OPENSHELL_USER_ENVIRONMENT` sidecar env var:
compute drivers (Docker, Podman, Kubernetes, VM) serialize the
user-specified environment as JSON into this variable at container
creation time, the sandbox supervisor deserializes it at startup, and
the SSH handler re-injects the values into child processes after
`env_clear()`. User-specified vars are applied at lowest priority so
proxy, TLS, and provider credentials still take precedence.
Signed-off-by: Russell Bryant <russell.bryant@gmail.com>
* fix(sandbox): address PR review feedback for env var injection
- VM driver: restructure build_guest_environment to apply user env
before required driver vars, preventing user-specified values from
overriding OPENSHELL_ENDPOINT, PATH, and other critical vars.
Matches the pattern already used by Docker/Podman/Kubernetes.
- All drivers: insert OPENSHELL_USER_ENVIRONMENT sidecar after
environment.extend(user_env) so a user-supplied key of the same
name cannot clobber the JSON payload the supervisor parses.
- Podman driver: fix template/spec env precedence — template env is
now applied first with spec overwriting, matching the other drivers
(spec is user-specified and should win over image defaults).
- CLI: add parse_env_pairs() that rejects OPENSHELL_* prefixed keys
with a clear error message. Both sandbox create and exec call sites
updated to use it.
- SSH handler: filter out OPENSHELL_* keys from user_environment
before injecting into child processes.
- Docs: add "Set Environment Variables" section to manage-sandboxes.mdx
documenting --env for both create and exec, per-command override
semantics, and the OPENSHELL_* reservation.
- Tests: add sandbox_create_env_rejects_reserved_prefix test.
- Fix rustfmt formatting in lib.rs, container.rs, driver.rs.
* fix(server): reject OPENSHELL_* env keys at API boundary
Add server-side validation to reject environment variable keys starting
with OPENSHELL_ in SandboxSpec.environment, SandboxTemplate.environment,
and ExecSandboxRequest.environment. This closes the gap where SDK/gRPC
callers (bypassing the CLI) could inject reserved keys that affect
supervisor-internal state.
The CLI already rejects these keys via parse_env_pairs, but the API
boundary is the authoritative enforcement point since all clients
(CLI, SDK, direct gRPC) pass through it.
* fix(server): validate env key names and control chars at API boundary
Add POSIX env key format validation (^[A-Za-z_][A-Za-z0-9_]*$) and
control character rejection for SandboxSpec.environment,
SandboxTemplate.environment, and ExecSandboxRequest.environment at the
gRPC validation layer. This catches invalid keys like "1BAD" or
"BAD-NAME" and values containing newlines at the API boundary rather
than letting them surface as driver-specific container/pod failures.
Also add matching CLI-side validation in parse_env_pairs so users get
immediate feedback. Update docs to say "OpenShell rejects" rather than
"the CLI rejects" since enforcement is now at both boundaries.
* fix(server): allow exec helper keys and add env size limit
Split exec env validation so OPENSHELL_PYFUNC_B64 (used by the Python
SDK's exec_python()) passes through while runtime-clobberable keys
like OPENSHELL_SANDBOX_ID are still rejected. Create-time validation
remains strict — all OPENSHELL_* keys are blocked.
Add a 256 KiB total serialized-size limit on create-time environment
maps. The drivers serialize the full map into a single
OPENSHELL_USER_ENVIRONMENT env var; capping the input at the API
boundary prevents driver/runtime-specific startup failures from
oversized env blocks.
Add template environment control-character test and exec helper key
acceptance test.
---------
Signed-off-by: Russell Bryant <russell.bryant@gmail.com>
* feat(cli): add generic output formatter to eliminate --output flag duplication
Closes#1750
Creates a new output.rs module with generic helper functions for formatting
CLI command output in JSON, YAML, or table formats. This eliminates ~38 lines
of duplicated code across 4 commands, provides a single source of truth for
output formatting, and makes it easier to add new output formats in the future.
Changes:
- crates/openshell-cli/src/output.rs: New module with 6 helper functions and 14 tests
- crates/openshell-cli/src/lib.rs: Register output module
- crates/openshell-cli/src/run.rs: Migrate 4 commands to use helpers
- gateway_list(): 23 lines → 5 lines
- sandbox_list(): 17 lines → 5 lines
- provider_list_profiles(): 12 lines → 7 lines
- provider_profile_export(): 11 lines → 8 lines
- architecture/cli.md: New CLI architecture documentation
- architecture/README.md: Add reference to CLI architecture doc
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: Jeff MAURY <jmaury@redhat.com>
* fix: apply recommandations from gator-agent
Signed-off-by: Jeff MAURY <jmaury@redhat.com>
* fix: fix wrong CLI output test
Signed-off-by: Jeff MAURY <jmaury@redhat.com>
---------
Signed-off-by: Jeff MAURY <jmaury@redhat.com>
Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
* feat(providers): add Google Vertex AI provider
Adds Vertex AI provider profiles, routing, credential refresh plumbing, CLI support, docs, and regression coverage. Keeps the related NETLINK_ROUTE seccomp allowance needed by Vertex client tooling that calls getifaddrs.
* docs: add Vertex AI sandbox usage for Claude Code and OpenCode
Cover the full end-to-end setup for running Claude Code and OpenCode
inside an OpenShell sandbox via inference.local with a Vertex AI backend:
- google-vertex-ai.mdx: add 'Use from a Sandbox' section with tabbed
examples for Claude Code (--bare flag, no /v1 suffix) and OpenCode
(/v1 suffix required). Add providers_v2_enabled prerequisite and
--no-verify note for global region. Document policy proposals table
covering metadata.google.internal (always blocked), downloads.claude.ai,
and storage.googleapis.com.
- inference-routing.mdx: expand 'Use the Local Endpoint' section with
tabbed examples for Claude Code, OpenCode, Python OpenAI SDK, and
Python Anthropic SDK. Add notes explaining the /v1 path suffix
difference between clients.
- supported-agents.mdx: update Claude Code and OpenCode rows to mention
inference.local support and correct base URL requirements.
* fix: address vertex review findings
* test(sandbox): retry on spurious Ok in fork-exec ambiguity test
On arm64 under heavy CI load, the /proc fd scan in
find_socket_inode_owners can transiently miss the parent process's
socket fd entry, returning only the child as an owner. This causes
resolve_process_identity to return Ok (single owner, no ambiguity
check fires) instead of the expected ambiguous-ownership Err.
Extend the retry loop to also handle unexpected Ok results, mirroring
the existing retry for transient Err results. 10 retries at 50ms gives
a 500ms settling window, which is sufficient for procfs to stabilize
on loaded arm64 runners.
* fix: address vertex review regressions
* docs(router): clarify stream_response semantics for Vertex rawPredict routing
Document the three call sites of prepare_backend_request and their
stream_response values in a caller table:
- send_backend_request: false → :rawPredict (unary endpoint)
- send_backend_request_streaming: true → :streamRawPredict
- verify_backend_endpoint: explicitly false to probe the unary endpoint
Cross-reference the table from build_provider_url and
is_vertex_anthropic_rawpredict_route so the stream_response=true guard
in the suffix upgrade branch is understood in full context.
Also note that is_vertex_anthropic_rawpredict_route is a structural
predicate (model_in_path + anthropic_messages + :rawPredict suffix),
not a named-provider check, so any future provider with the same route
shape inherits the transforms automatically.
* refactor(proto): move phase and current_policy_version into SandboxStatus
Move phase and current_policy_version from SandboxSpec into
SandboxStatus to correctly model mutable runtime state. Update all
callers in the gateway server, TUI, and Python SDK to read and write
these fields through SandboxStatus accessors.
Signed-off-by: Derek Carr <decarr@redhat.com>
* fix(server): preserve sandbox status on statusless driver updates
When a driver update arrives without a status payload (e.g. before
Kubernetes populates the status subresource), preserve the stored
phase, conditions, and current policy version instead of resetting
them. Adds a regression test covering the edge case.
Signed-off-by: Derek Carr <decarr@redhat.com>
---------
Signed-off-by: Derek Carr <decarr@redhat.com>
Thread the gateway_insecure flag through gateway_add(), gateway_login(),
and all OIDC HTTP clients so that --gateway-insecure and
OPENSHELL_GATEWAY_INSECURE apply to OIDC discovery, token exchange, and
token refresh requests.
Previously, the flag only affected gRPC connections to the gateway. OIDC
HTTP clients (reqwest::get and http_client) always verified TLS
certificates, causing gateway registration and login to fail when the
OIDC issuer used a self-signed certificate (common on OpenShift with
edge-terminated routes).
Fixes#1534
Signed-off-by: Adel Zaalouk <azaalouk@redhat.com>
* fix(cli): add json output for policy get
* test(cli): cover policy get full json output
* fix(cli): address policy get json clippy
---------
Co-authored-by: John Myers <9696606+johntmyers@users.noreply.github.com>
Add -o/--output flag to `openshell gateway list` matching the existing
sandbox list pattern, enabling machine-readable output for scripting.
Signed-off-by: Florent Benoit <fbenoit@redhat.com>
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>
Support machine-readable output for `openshell sandbox list` via
`-o json` and `-o yaml` flags, matching the pattern established for
provider profile commands. Rename ProviderProfileOutput to shared
OutputFormat enum.
Signed-off-by: Florent Benoit <fbenoit@redhat.com>
* feat(exec): add bidirectional streaming for interactive TTY sessions
The existing ExecSandbox RPC sends stdin upfront in the request body,
making interactive programs (bash, top, vim) unusable with --tty. This
adds an ExecSandboxInteractive bidirectional streaming RPC that forwards
live keystrokes, terminal resize events, and EOF to the sandbox.
- Add ExecSandboxInteractive RPC, ExecSandboxInput and
ExecSandboxWindowResize messages to the proto definition
- Implement server handler with relay bridging, PTY allocation via
russh channel.split(), and timeout support
- Implement CLI client with raw mode, spawn_blocking stdin reader,
SIGWINCH resize forwarding, and process::exit for clean shutdown
- Route to interactive path only when --tty is explicitly passed
Signed-off-by: Florent Benoit <fbenoit@redhat.com>
* fix(exec): address interactive TTY streaming review feedback
Server-side (sandbox.rs):
- Close SSH channel after EOF so PTY-backed programs (bash, vim, top)
terminate on client disconnect instead of leaking the session
- Break output loop when tx.send() fails (client gone) to tear down
the streaming pipeline promptly
- Default zero cols/rows to 80x24 before request_pty, matching the
proto "0 = use default" contract and preventing 1x1 terminals for
non-CLI clients
Client-side (run.rs):
- Replace spawn_blocking stdin reader with a detached std::thread so
the tokio runtime shutdown does not hang on a thread blocked in
stdin.read(). This removes the need for std::process::exit(),
allowing save_last_sandbox() in the caller to execute normally
- Switch from futures::channel::mpsc with try_send() to
tokio::sync::mpsc with blocking_send() for proper backpressure —
try_send silently stopped forwarding all input when the channel
filled during paste or slow network
- Add TaskGuard (abort-on-drop) for the resize task to ensure cleanup
on all return paths including early errors
---------
Signed-off-by: Florent Benoit <fbenoit@redhat.com>
Move EdgeAuthInterceptor from openshell-cli to openshell-core so the TUI
can use it without a circular dependency. The TUI now checks gateway
metadata for OIDC auth mode, loads the stored bearer token, and injects
it into gRPC requests via the shared interceptor.
When switching gateways, the TUI dispatches on auth_mode to build either
an mTLS or OIDC-authenticated channel. Expired tokens surface a clear
error directing users to re-authenticate with `openshell gateway login`.
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.
* fix(cli): cp-style sandbox download and workspace-boundary check
`openshell sandbox download` produced a directory at the host destination
instead of writing the source bytes as a regular file, and did not refuse
sandbox-side paths that resolved outside `/sandbox` (only `creds.json` was
rejected, by filesystem policy rather than by an explicit check).
Mirrors the cp-style design from the upload-side fixes (#667 / #694 and
#885 / #952): destination resolution now respects trailing slashes and
existing-directory destinations, single-file sources land as regular files
via a staged extraction and atomic rename, and the function delegates the
SSH tar stream to a shared helper. Adds an explicit lexical
workspace-boundary check on the sandbox-side source — required on the
download side because filesystem policy alone is not a substitute for an
explicit check when crossing the trust boundary in the leak direction.
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
* fix(cli): canonicalise sandbox download path remotely and quote tar member
The lexical check in `validate_sandbox_source_path` clears any path whose
components stay under `/sandbox` after `.` and `..` are folded out, but it
cannot see symlinks. A workspace-relative symlink such as
`/sandbox/etc-link -> /etc` lets `openshell sandbox download <name>
/sandbox/etc-link/passwd /tmp/passwd` slip through, then `tar -C
/sandbox/etc-link passwd` follows the link on the sandbox side and
streams `/etc/passwd` back to the host.
Add `canonicalize_sandbox_source_path` which runs `realpath -e --` over
SSH and rejects the result unless it still equals `/sandbox` or starts
with `/sandbox/`. The lexical guard stays as a cheap fail-fast for
obviously-bad inputs, and the rest of the download flow now uses the
canonical path everywhere a tar invocation or probe might dereference a
symlink.
While editing the single-file branch, also add the `--` separator before
the basename in `tar cf - -C <parent> -- <name>`. Without it, a sandbox
file whose basename starts with `-` (e.g. `--checkpoint-action=...`) is
parsed by GNU tar as an option rather than a member, which at best
fails the wrap and at worst triggers tar option behaviour. The directory
branch passes literal `.` and is unaffected.
Cover both fixes with pure-function tests: `is_under_sandbox_workspace`
gets accept/reject pairs (the same predicate the post-realpath
validator uses) and the new `build_single_file_tar_cmd` helper asserts
the `--` separator and shell escaping survive any future refactor.
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
* chore(cli): drop redundant tempfile dev-dependency entry
`tempfile` was promoted from `[dev-dependencies]` to `[dependencies]`
once the cp-style single-file download path started using
`TempDir::new_in` from production code. The original `[dev-dependencies]`
line stayed behind by accident — entries under `[dependencies]` are
already available to test compilation, so the second declaration is
dead weight.
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
* test(cli): add E2E coverage for sandbox download paths
Adds four end-to-end tests for `openshell sandbox download` so the
happy path and both Codex regressions have explicit coverage:
* `sandbox_download_file_only` — seeds a single file via the new
`SandboxGuard::exec` harness helper and pulls it back, exercising
`sandbox_sync_down_file` without an upload phase.
* `sandbox_download_directory_only` — seeds a small directory tree and
pulls it recursively, exercising `sandbox_sync_down_directory`.
* `sandbox_download_rejects_symlinks_pointing_outside_workspace` —
regression for the high-severity finding. Plants three escape shapes
(`/sandbox/etc-link -> /etc`, `/sandbox/passwd-link -> /etc/passwd`,
and a path with a symlinked component `/sandbox/etc-link/passwd`) and
asserts all three are refused with a workspace-boundary error before
any tar streams. Also checks the host destination stays empty so a
silent leak would surface.
* `sandbox_download_handles_dash_leading_basename` — regression for the
medium-severity finding. Seeds a file named `--checkpoint-action=evil`
and verifies it downloads with byte-for-byte contents, proving the
`--` separator stops tar parsing the basename as an option.
The harness gains a single new method, `SandboxGuard::exec`, that wraps
`openshell sandbox exec --name <name> --no-tty -- <argv>`. Existing
tests preferred the upload path to populate sandbox state, but the
regression cases need to plant filesystem state the upload flow cannot
produce (symlinks, files whose basenames clap would otherwise reject).
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
* docs(cli): note cp-style destination and workspace boundary on download
`openshell sandbox download` now refuses any source path that resolves
outside `/sandbox` (lexically or through a symlink), and the
single-file branch follows `cp`-style placement rules. Document both
so users hitting the workspace-boundary error or the
"land at exact path vs land inside dir" decision can self-serve.
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
* test(cli): tolerate miette line-wrapping in symlink-rejection assertions
`sandbox_download_rejects_symlinks_pointing_outside_workspace` matched
the substring `"outside the sandbox workspace"` against the CLI's error
output. The check is correct in spirit but fragile in practice: miette
wraps long single-line errors across multiple lines at terminal width
and inserts a `│ ` continuation marker, so the substring breaks across
the wrap and the test fails even though the canonicalisation step
correctly refused the source.
Split the match into three short markers — `resolves to`, `outside the`,
`sandbox workspace` — each guaranteed to fit on one wrapped line. The
combined check is stricter than the original (it now requires evidence
that the remote `realpath -e` step actually ran, not just the boundary
refusal) without inheriting the wrap fragility.
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
---------
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Switch reqwest from `rustls-tls` (bundled Mozilla webpki roots) to
`rustls-tls-native-roots` (OS certificate store) so that the CLI
trusts the same CAs as the rest of the system. This fixes OIDC
discovery failures against Keycloak instances using certificates
signed by internal CAs present in the system trust bundle.
* 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>
Jeff MAURY