mirror of
https://github.com/NVIDIA/OpenShell.git
synced 2026-10-16 05:20:22 +08:00
John T. Myers
256f7fc884
* fix(sandbox,server): fix chunk merge duplicates and OPA variable collision with overlapping policies
Two related bugs triggered when a draft rule approval creates a second
policy entry for the same host:port:
1. merge_chunk_into_policy looked up existing rules by chunk.rule_name
(auto-generated as allow_{host}_{port}), which never matched the
user's original rule name. Now scans all network_policies entries
for a host:port endpoint match before falling back to insertion,
and merges allowed_ips into the existing endpoint.
2. The Rego allow_request rule and _matching_endpoint_configs
comprehension used 'some ep; ep := policy.endpoints[_]' which
caused regorus to error with 'duplicated definition of local
variable ep' when multiple policies covered the same host:port.
Refactored to isolate endpoint iteration inside helper functions
(_policy_allows_l7, _policy_endpoint_configs) so variables are
scoped per-policy evaluation.
Refs: #567
* test(e2e): add overlapping policy tests and update FWD-2 for implicit allowed_ips
- Update FWD-2 (test_forward_proxy_denied_without_allowed_ips ->
test_forward_proxy_allows_private_ip_host_without_allowed_ips):
literal IP host no longer requires explicit allowed_ips, expects 200.
- Add OVL-1: overlapping L4 policies for same host:port must not crash
OPA and should allow forward proxy connections.
- Add OVL-2: overlapping L7 policies for same host:port must not crash
OPA and should allow CONNECT tunnel establishment.
Refs: #567
* style: apply cargo fmt formatting
* test(e2e): update SSRF-3 and SSRF-6 for implicit allowed_ips behavior
SSRF-6: Private IP with literal IP host now gets implicit allowed_ips
from PR #570, so CONNECT returns 200 instead of 403.
SSRF-3: Loopback is still blocked but via the always-blocked path
(implicit allowed_ips is synthesized, then resolve_and_check_allowed_ips
catches it). Log message says 'always-blocked' instead of 'internal
address'.
* fix(e2e): use negative assertion for SSRF-6 when nothing listens on target port
When the SSRF check passes but nothing listens on the target port,
recv() returns empty bytes. Use 'assert 403 not in' (matching SSRF-4
pattern) instead of 'assert 200 in'.
* fix(e2e): update provider tests for redacted credential values
PR #569 changed credential redaction from clearing the map to
replacing values with 'REDACTED'. Update e2e assertions to expect
credential keys with REDACTED values instead of an empty map.
413 lines
14 KiB
Python
413 lines
14 KiB
Python
# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
|
|
# SPDX-License-Identifier: Apache-2.0
|
|
|
|
"""E2E tests for supervisor-managed provider placeholders in sandboxes.
|
|
|
|
Provider credentials are fetched at runtime by the sandbox supervisor via the
|
|
GetSandboxProviderEnvironment gRPC call. Sandboxed child processes should see
|
|
placeholder values (not raw secrets). Credentials must never be present in the
|
|
persisted sandbox spec environment map.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
from contextlib import contextmanager
|
|
from typing import TYPE_CHECKING
|
|
|
|
import grpc
|
|
import pytest
|
|
|
|
from openshell._proto import datamodel_pb2, openshell_pb2, sandbox_pb2
|
|
|
|
if TYPE_CHECKING:
|
|
from collections.abc import Callable, Iterator
|
|
|
|
from openshell import Sandbox, SandboxClient
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Policy helpers
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
def _default_policy() -> sandbox_pb2.SandboxPolicy:
|
|
"""Build a sandbox policy with standard filesystem/process/landlock settings."""
|
|
return sandbox_pb2.SandboxPolicy(
|
|
version=1,
|
|
filesystem=sandbox_pb2.FilesystemPolicy(
|
|
include_workdir=True,
|
|
read_only=["/usr", "/lib", "/etc", "/app"],
|
|
read_write=["/sandbox", "/tmp"],
|
|
),
|
|
landlock=sandbox_pb2.LandlockPolicy(compatibility="best_effort"),
|
|
process=sandbox_pb2.ProcessPolicy(
|
|
run_as_user="sandbox", run_as_group="sandbox"
|
|
),
|
|
)
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Provider lifecycle helper
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@contextmanager
|
|
def provider(
|
|
stub: object,
|
|
*,
|
|
name: str,
|
|
provider_type: str,
|
|
credentials: dict[str, str],
|
|
) -> Iterator[str]:
|
|
"""Create a provider for the duration of the block, then delete it."""
|
|
_delete_provider(stub, name)
|
|
stub.CreateProvider(
|
|
openshell_pb2.CreateProviderRequest(
|
|
provider=datamodel_pb2.Provider(
|
|
name=name,
|
|
type=provider_type,
|
|
credentials=credentials,
|
|
)
|
|
)
|
|
)
|
|
try:
|
|
yield name
|
|
finally:
|
|
_delete_provider(stub, name)
|
|
|
|
|
|
def _delete_provider(stub: object, name: str) -> None:
|
|
"""Delete a provider, ignoring not-found errors."""
|
|
try:
|
|
stub.DeleteProvider(openshell_pb2.DeleteProviderRequest(name=name))
|
|
except grpc.RpcError as exc:
|
|
if hasattr(exc, "code") and exc.code() == grpc.StatusCode.NOT_FOUND:
|
|
pass
|
|
else:
|
|
raise
|
|
|
|
|
|
# ===========================================================================
|
|
# Tests: placeholder visibility
|
|
# ===========================================================================
|
|
|
|
|
|
def test_provider_credentials_available_as_env_vars(
|
|
sandbox: Callable[..., Sandbox],
|
|
sandbox_client: SandboxClient,
|
|
) -> None:
|
|
"""Sandbox child processes see provider env vars as placeholders."""
|
|
with provider(
|
|
sandbox_client._stub,
|
|
name="e2e-test-provider-env",
|
|
provider_type="claude",
|
|
credentials={"ANTHROPIC_API_KEY": "sk-e2e-test-key-12345"},
|
|
) as provider_name:
|
|
spec = datamodel_pb2.SandboxSpec(
|
|
policy=_default_policy(),
|
|
providers=[provider_name],
|
|
)
|
|
|
|
def read_env_var() -> str:
|
|
import os
|
|
|
|
return os.environ.get("ANTHROPIC_API_KEY", "NOT_SET")
|
|
|
|
with sandbox(spec=spec, delete_on_exit=True) as sb:
|
|
result = sb.exec_python(read_env_var)
|
|
assert result.exit_code == 0, result.stderr
|
|
value = result.stdout.strip()
|
|
assert value == "openshell:resolve:env:ANTHROPIC_API_KEY"
|
|
assert value != "sk-e2e-test-key-12345"
|
|
|
|
|
|
def test_generic_provider_credentials_available_as_env_vars(
|
|
sandbox: Callable[..., Sandbox],
|
|
sandbox_client: SandboxClient,
|
|
) -> None:
|
|
"""Generic provider env vars are placeholders, not raw secrets."""
|
|
with provider(
|
|
sandbox_client._stub,
|
|
name="e2e-test-generic-provider-env",
|
|
provider_type="generic",
|
|
credentials={
|
|
"CUSTOM_SERVICE_TOKEN": "token-generic-123",
|
|
"CUSTOM_SERVICE_URL": "https://internal.example.test/api",
|
|
},
|
|
) as provider_name:
|
|
spec = datamodel_pb2.SandboxSpec(
|
|
policy=_default_policy(),
|
|
providers=[provider_name],
|
|
)
|
|
|
|
def read_generic_env_vars() -> str:
|
|
import os
|
|
|
|
token = os.environ.get("CUSTOM_SERVICE_TOKEN", "NOT_SET")
|
|
url = os.environ.get("CUSTOM_SERVICE_URL", "NOT_SET")
|
|
return f"{token}|{url}"
|
|
|
|
with sandbox(spec=spec, delete_on_exit=True) as sb:
|
|
result = sb.exec_python(read_generic_env_vars)
|
|
assert result.exit_code == 0, result.stderr
|
|
assert (
|
|
result.stdout.strip()
|
|
== "openshell:resolve:env:CUSTOM_SERVICE_TOKEN|openshell:resolve:env:CUSTOM_SERVICE_URL"
|
|
)
|
|
|
|
|
|
def test_nvidia_provider_injects_nvidia_api_key_env_var(
|
|
sandbox: Callable[..., Sandbox],
|
|
sandbox_client: SandboxClient,
|
|
) -> None:
|
|
"""NVIDIA provider projects a placeholder env value into child processes."""
|
|
with provider(
|
|
sandbox_client._stub,
|
|
name="e2e-test-nvidia-provider-env",
|
|
provider_type="nvidia",
|
|
credentials={"NVIDIA_API_KEY": "nvapi-e2e-test-key"},
|
|
) as provider_name:
|
|
spec = datamodel_pb2.SandboxSpec(
|
|
policy=_default_policy(),
|
|
providers=[provider_name],
|
|
)
|
|
|
|
def read_nvidia_key() -> str:
|
|
import os
|
|
|
|
return os.environ.get("NVIDIA_API_KEY", "NOT_SET")
|
|
|
|
with sandbox(spec=spec, delete_on_exit=True) as sb:
|
|
result = sb.exec_python(read_nvidia_key)
|
|
assert result.exit_code == 0, result.stderr
|
|
assert result.stdout.strip() == "openshell:resolve:env:NVIDIA_API_KEY"
|
|
|
|
|
|
# ===========================================================================
|
|
# Tests: security & edge cases
|
|
# ===========================================================================
|
|
|
|
|
|
def test_ssh_handshake_secret_not_visible_in_exec_environment(
|
|
sandbox: Callable[..., Sandbox],
|
|
) -> None:
|
|
def read_handshake_secret() -> str:
|
|
import os
|
|
|
|
return os.environ.get("OPENSHELL_SSH_HANDSHAKE_SECRET", "NOT_SET")
|
|
|
|
with sandbox(delete_on_exit=True) as sb:
|
|
result = sb.exec_python(read_handshake_secret)
|
|
assert result.exit_code == 0, result.stderr
|
|
assert result.stdout.strip() == "NOT_SET"
|
|
|
|
|
|
def test_create_sandbox_rejects_unknown_provider(
|
|
sandbox_client: SandboxClient,
|
|
) -> None:
|
|
"""CreateSandbox fails fast when a provider name does not exist."""
|
|
spec = datamodel_pb2.SandboxSpec(
|
|
policy=_default_policy(),
|
|
providers=["nonexistent-provider-xyz"],
|
|
)
|
|
with pytest.raises(grpc.RpcError) as exc_info:
|
|
sandbox_client.create(spec=spec)
|
|
|
|
assert exc_info.value.code() == grpc.StatusCode.FAILED_PRECONDITION
|
|
assert "nonexistent-provider-xyz" in (exc_info.value.details() or "")
|
|
|
|
|
|
def test_credentials_not_in_persisted_spec_environment(
|
|
sandbox: Callable[..., Sandbox],
|
|
sandbox_client: SandboxClient,
|
|
) -> None:
|
|
"""Provider credentials should NOT appear in the sandbox spec's environment map."""
|
|
with provider(
|
|
sandbox_client._stub,
|
|
name="e2e-test-no-persist",
|
|
provider_type="claude",
|
|
credentials={"ANTHROPIC_API_KEY": "sk-should-not-persist"},
|
|
) as provider_name:
|
|
spec = datamodel_pb2.SandboxSpec(
|
|
policy=_default_policy(),
|
|
providers=[provider_name],
|
|
)
|
|
|
|
with sandbox(spec=spec, delete_on_exit=True) as sb:
|
|
fetched = sandbox_client._stub.GetSandbox(
|
|
openshell_pb2.GetSandboxRequest(name=sb.sandbox.name)
|
|
)
|
|
persisted_env = dict(fetched.sandbox.spec.environment)
|
|
assert "ANTHROPIC_API_KEY" not in persisted_env, (
|
|
"credentials should not be persisted in sandbox spec environment"
|
|
)
|
|
|
|
|
|
# ===========================================================================
|
|
# Tests: provider update merge semantics
|
|
# ===========================================================================
|
|
|
|
|
|
def test_update_provider_preserves_unset_credentials_and_config(
|
|
sandbox_client: SandboxClient,
|
|
) -> None:
|
|
"""Updating one credential must not clobber other credentials or config."""
|
|
stub = sandbox_client._stub
|
|
name = "merge-test-preserve"
|
|
_delete_provider(stub, name)
|
|
|
|
try:
|
|
stub.CreateProvider(
|
|
openshell_pb2.CreateProviderRequest(
|
|
provider=datamodel_pb2.Provider(
|
|
name=name,
|
|
type="generic",
|
|
credentials={"KEY_A": "val-a", "KEY_B": "val-b"},
|
|
config={"BASE_URL": "https://example.com"},
|
|
)
|
|
)
|
|
)
|
|
|
|
stub.UpdateProvider(
|
|
openshell_pb2.UpdateProviderRequest(
|
|
provider=datamodel_pb2.Provider(
|
|
name=name,
|
|
type="",
|
|
credentials={"KEY_A": "rotated-a"},
|
|
)
|
|
)
|
|
)
|
|
|
|
got = stub.GetProvider(openshell_pb2.GetProviderRequest(name=name))
|
|
p = got.provider
|
|
# Credential keys are preserved but values are redacted.
|
|
assert len(p.credentials) > 0, "credential keys should be preserved"
|
|
for key, val in p.credentials.items():
|
|
assert val == "REDACTED", (
|
|
f"credential '{key}' should be REDACTED, got '{val}'"
|
|
)
|
|
assert p.config["BASE_URL"] == "https://example.com", (
|
|
"config should be preserved"
|
|
)
|
|
finally:
|
|
_delete_provider(stub, name)
|
|
|
|
|
|
def test_update_provider_empty_maps_preserves_all(
|
|
sandbox_client: SandboxClient,
|
|
) -> None:
|
|
"""Sending empty credential and config maps should be a no-op."""
|
|
stub = sandbox_client._stub
|
|
name = "merge-test-noop"
|
|
_delete_provider(stub, name)
|
|
|
|
try:
|
|
stub.CreateProvider(
|
|
openshell_pb2.CreateProviderRequest(
|
|
provider=datamodel_pb2.Provider(
|
|
name=name,
|
|
type="generic",
|
|
credentials={"TOKEN": "secret"},
|
|
config={"URL": "https://api.example.com"},
|
|
)
|
|
)
|
|
)
|
|
|
|
stub.UpdateProvider(
|
|
openshell_pb2.UpdateProviderRequest(
|
|
provider=datamodel_pb2.Provider(
|
|
name=name,
|
|
type="",
|
|
)
|
|
)
|
|
)
|
|
|
|
got = stub.GetProvider(openshell_pb2.GetProviderRequest(name=name))
|
|
p = got.provider
|
|
# Credential keys are preserved but values are redacted.
|
|
assert len(p.credentials) > 0, "credential keys should be preserved"
|
|
for key, val in p.credentials.items():
|
|
assert val == "REDACTED", (
|
|
f"credential '{key}' should be REDACTED, got '{val}'"
|
|
)
|
|
assert p.config["URL"] == "https://api.example.com"
|
|
finally:
|
|
_delete_provider(stub, name)
|
|
|
|
|
|
def test_update_provider_merges_config_preserves_credentials(
|
|
sandbox_client: SandboxClient,
|
|
) -> None:
|
|
"""Updating only config should not touch credentials."""
|
|
stub = sandbox_client._stub
|
|
name = "merge-test-config-only"
|
|
_delete_provider(stub, name)
|
|
|
|
try:
|
|
stub.CreateProvider(
|
|
openshell_pb2.CreateProviderRequest(
|
|
provider=datamodel_pb2.Provider(
|
|
name=name,
|
|
type="generic",
|
|
credentials={"API_KEY": "original-key"},
|
|
config={"ENDPOINT": "https://old.example.com"},
|
|
)
|
|
)
|
|
)
|
|
|
|
stub.UpdateProvider(
|
|
openshell_pb2.UpdateProviderRequest(
|
|
provider=datamodel_pb2.Provider(
|
|
name=name,
|
|
type="",
|
|
config={"ENDPOINT": "https://new.example.com"},
|
|
)
|
|
)
|
|
)
|
|
|
|
got = stub.GetProvider(openshell_pb2.GetProviderRequest(name=name))
|
|
p = got.provider
|
|
# Credential keys are preserved but values are redacted.
|
|
assert len(p.credentials) > 0, "credential keys should be preserved"
|
|
for key, val in p.credentials.items():
|
|
assert val == "REDACTED", (
|
|
f"credential '{key}' should be REDACTED, got '{val}'"
|
|
)
|
|
assert p.config["ENDPOINT"] == "https://new.example.com"
|
|
finally:
|
|
_delete_provider(stub, name)
|
|
|
|
|
|
def test_update_provider_rejects_type_change(
|
|
sandbox_client: SandboxClient,
|
|
) -> None:
|
|
"""Attempting to change a provider's type must be rejected."""
|
|
stub = sandbox_client._stub
|
|
name = "merge-test-type-reject"
|
|
_delete_provider(stub, name)
|
|
|
|
try:
|
|
stub.CreateProvider(
|
|
openshell_pb2.CreateProviderRequest(
|
|
provider=datamodel_pb2.Provider(
|
|
name=name,
|
|
type="generic",
|
|
credentials={"KEY": "val"},
|
|
)
|
|
)
|
|
)
|
|
|
|
with pytest.raises(grpc.RpcError) as exc_info:
|
|
stub.UpdateProvider(
|
|
openshell_pb2.UpdateProviderRequest(
|
|
provider=datamodel_pb2.Provider(
|
|
name=name,
|
|
type="nvidia",
|
|
)
|
|
)
|
|
)
|
|
assert exc_info.value.code() == grpc.StatusCode.INVALID_ARGUMENT
|
|
assert "type cannot be changed" in exc_info.value.details()
|
|
finally:
|
|
_delete_provider(stub, name)
|