Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions server/opensandbox_server/services/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,16 @@
# Must match components/egress/pkg/constants/configuration.go EnvEgressToken
OPENSANDBOX_EGRESS_TOKEN = "OPENSANDBOX_EGRESS_TOKEN"
OPENSANDBOX_EGRESS_MITMPROXY_TRANSPARENT = "OPENSANDBOX_EGRESS_MITMPROXY_TRANSPARENT"

EGRESS_ENV_PREFIX = "OPENSANDBOX_EGRESS_"
# Must match components/egress/pkg/constants/configuration.go EnvEgressHTTPAddr
OPENSANDBOX_EGRESS_HTTP_ADDR = "OPENSANDBOX_EGRESS_HTTP_ADDR"
RESERVED_EGRESS_ENV_VARS = frozenset({
EGRESS_RULES_ENV,
EGRESS_MODE_ENV,
OPENSANDBOX_EGRESS_TOKEN,
OPENSANDBOX_EGRESS_HTTP_ADDR,
})
Comment thread
Pangjiping marked this conversation as resolved.
OPENSANDBOX_RUNTIME_VOLUME_NAME = "opensandbox-bin"
OPENSANDBOX_RUNTIME_MOUNT_PATH = "/opt/opensandbox"

Expand Down Expand Up @@ -146,6 +156,9 @@ class SnapshotErrorCodes:
"EGRESS_MODE_ENV",
"OPENSANDBOX_EGRESS_TOKEN",
"OPENSANDBOX_EGRESS_MITMPROXY_TRANSPARENT",
"EGRESS_ENV_PREFIX",
"OPENSANDBOX_EGRESS_HTTP_ADDR",
"RESERVED_EGRESS_ENV_VARS",
"OPENSANDBOX_RUNTIME_VOLUME_NAME",
"OPENSANDBOX_RUNTIME_MOUNT_PATH",
"SandboxErrorCodes",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ def create_workload(
egress_auth_token: Optional[str] = None,
egress_mode: str = EGRESS_MODE_DNS,
credential_proxy_enabled: bool = False,
egress_env: Optional[Dict[str, Optional[str]]] = None,
) -> Dict[str, Any]:
"""Create an agent-sandbox Sandbox CRD workload."""
if is_windows_profile(platform):
Expand All @@ -158,6 +159,7 @@ def create_workload(
egress_auth_token=egress_auth_token,
egress_mode=egress_mode,
credential_proxy_enabled=credential_proxy_enabled,
egress_env=egress_env,
)

if volumes:
Expand Down Expand Up @@ -245,6 +247,7 @@ def _build_pod_spec(
egress_auth_token: Optional[str] = None,
egress_mode: str = EGRESS_MODE_DNS,
credential_proxy_enabled: bool = False,
egress_env: Optional[Dict[str, Optional[str]]] = None,
) -> Dict[str, Any]:
"""Build pod spec dict for the Sandbox CRD."""
disable_ipv6_for_egress = (
Expand Down Expand Up @@ -291,6 +294,7 @@ def _build_pod_spec(
egress_auth_token=egress_auth_token,
egress_mode=egress_mode,
credential_proxy_enabled=credential_proxy_enabled,
extra_env=egress_env,
)

return pod_spec
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ def create_workload(
egress_auth_token: Optional[str] = None,
egress_mode: str = EGRESS_MODE_DNS,
credential_proxy_enabled: bool = False,
egress_env: Optional[Dict[str, Optional[str]]] = None,
) -> Dict[str, Any]:
"""Create a BatchSandbox in template mode or pool mode."""
extensions = extensions or {}
Expand Down Expand Up @@ -225,6 +226,7 @@ def create_workload(
egress_auth_token=egress_auth_token,
egress_mode=egress_mode,
credential_proxy_enabled=credential_proxy_enabled,
extra_env=egress_env,
)

if volumes:
Expand Down
37 changes: 37 additions & 0 deletions server/opensandbox_server/services/k8s/create_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@
from opensandbox_server.api.schema import CreateSandboxRequest
from opensandbox_server.config import AppConfig, EGRESS_MODE_DNS
from opensandbox_server.services.constants import (
EGRESS_ENV_PREFIX,
OPENSANDBOX_EGRESS_MITMPROXY_TRANSPARENT,
RESERVED_EGRESS_ENV_VARS,
SANDBOX_EGRESS_AUTH_TOKEN_METADATA_KEY,
SANDBOX_SECURE_ACCESS_TOKEN_METADATA_KEY,
SANDBOX_ID_LABEL,
Expand All @@ -29,6 +32,34 @@
)
from opensandbox_server.services.validators import calculate_expiration_or_raise

Pair = tuple[Dict[str, Optional[str]], Dict[str, Optional[str]]]


def _split_egress_env(
env: Optional[Dict[str, Optional[str]]],
) -> Pair:
"""Split request env into (sandbox_env, egress_env) by OPENSANDBOX_EGRESS_ prefix.

Raises ValueError if a user-supplied key collides with a reserved internal var.
"""
if not env:
return {}, {}

sandbox_env: Dict[str, Optional[str]] = {}
egress_env: Dict[str, Optional[str]] = {}
for key, value in env.items():
if key.startswith(EGRESS_ENV_PREFIX):
if key in RESERVED_EGRESS_ENV_VARS:
raise ValueError(
f"Environment variable '{key}' is reserved and cannot be overridden"
)
egress_env[key] = value
Comment thread
Pangjiping marked this conversation as resolved.
Comment on lines +51 to +56

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve prefixed envs when no sidecar exists

For Kubernetes create requests that do not include networkPolicy, this branch still removes every non-reserved OPENSANDBOX_EGRESS_... variable from the sandbox env and stores it only in egress_env. Because _build_create_workload_context only sets an egress image/token when request.network_policy is present, apply_egress_to_spec() returns without creating a sidecar, so these variables are silently injected nowhere instead of into the requested sandbox runtime. Only split these keys when an egress sidecar will actually be created, or reject this combination explicitly.

Useful? React with 👍 / 👎.

if key == OPENSANDBOX_EGRESS_MITMPROXY_TRANSPARENT:
sandbox_env[key] = value
else:
sandbox_env[key] = value
return sandbox_env, egress_env


@dataclass
class _CreateWorkloadContext:
Expand All @@ -41,6 +72,8 @@ class _CreateWorkloadContext:
egress_auth_token: Optional[str]
credential_proxy_enabled: bool
secure_access_token: Optional[str]
sandbox_env: Dict[str, Optional[str]]
egress_env: Dict[str, Optional[str]]


def _build_create_workload_context(
Expand Down Expand Up @@ -84,6 +117,8 @@ def _build_create_workload_context(
if request.resource_limits and request.resource_limits.root:
resource_limits = request.resource_limits.root

sandbox_env, egress_env = _split_egress_env(request.env)
Comment thread
Pangjiping marked this conversation as resolved.

return _CreateWorkloadContext(
labels=labels,
annotations=annotations,
Expand All @@ -94,4 +129,6 @@ def _build_create_workload_context(
egress_auth_token=egress_auth_token,
credential_proxy_enabled=credential_proxy_enabled,
secure_access_token=secure_access_token,
sandbox_env=sandbox_env,
egress_env=egress_env,
)
6 changes: 6 additions & 0 deletions server/opensandbox_server/services/k8s/egress_helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ def apply_egress_to_spec(
egress_auth_token: Optional[str] = None,
egress_mode: str = EGRESS_MODE_DNS,
credential_proxy_enabled: bool = False,
extra_env: Optional[Dict[str, Optional[str]]] = None,
) -> None:
"""
Append the egress sidecar to ``containers``. When ``egress.disable_ipv6`` is enabled,
Expand All @@ -94,6 +95,11 @@ def apply_egress_to_spec(
env.append({"name": OPENSANDBOX_EGRESS_MITMPROXY_TRANSPARENT, "value": "true"})
if egress_auth_token:
env.append({"name": OPENSANDBOX_EGRESS_TOKEN, "value": egress_auth_token})
if extra_env:
for name, value in extra_env.items():
if credential_proxy_enabled and name == OPENSANDBOX_EGRESS_MITMPROXY_TRANSPARENT:
continue
env.append({"name": name, "value": value or ""})
Comment thread
Pangjiping marked this conversation as resolved.
Comment thread
Pangjiping marked this conversation as resolved.
Comment on lines +100 to +102

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reject insecure MITM when using credential proxy

When credentialProxy.enabled is true, a request can still pass OPENSANDBOX_EGRESS_MITMPROXY_SSL_INSECURE=true through env, and this loop appends it to the egress sidecar. The credential vault explicitly refuses to operate in that mode (components/egress/pkg/credentialvault/vault.go:373-378), so credential-vault writes for that sandbox fail even though the create request asked the server to provision credential proxy support. Filter or reject this env var on the credential-proxy path the same way the transparent MITM flag is protected.

Useful? React with 👍 / 👎.


sidecar: Dict[str, Any] = {
"name": "egress",
Expand Down
21 changes: 11 additions & 10 deletions server/opensandbox_server/services/k8s/kubernetes_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -423,16 +423,16 @@ async def create_sandbox(self, request: CreateSandboxRequest) -> CreateSandboxRe
sandbox_id = self.generate_sandbox_id()

created_at = datetime.now(timezone.utc)
context = _build_create_workload_context(
app_config=self.app_config,
request=request,
sandbox_id=sandbox_id,
created_at=created_at,
egress_token_factory=generate_egress_token,
secure_access_token_factory=generate_secure_access_token,
)


try:
context = _build_create_workload_context(
app_config=self.app_config,
request=request,
sandbox_id=sandbox_id,
created_at=created_at,
egress_token_factory=generate_egress_token,
secure_access_token_factory=generate_secure_access_token,
)
apply_access_renew_extend_seconds_to_mapping(context.annotations, request.extensions)
apply_extensions_to_annotations(context.annotations, request.extensions)

Expand All @@ -453,7 +453,7 @@ async def create_sandbox(self, request: CreateSandboxRequest) -> CreateSandboxRe
namespace=self.namespace,
image_spec=request.image,
entrypoint=request.entrypoint,
env=request.env or {},
env=context.sandbox_env,
resource_limits=context.resource_limits,
labels=context.labels,
annotations=context.annotations or None,
Expand All @@ -465,6 +465,7 @@ async def create_sandbox(self, request: CreateSandboxRequest) -> CreateSandboxRe
egress_auth_token=context.egress_auth_token,
egress_mode=context.egress_mode,
credential_proxy_enabled=context.credential_proxy_enabled,
egress_env=context.egress_env,
volumes=request.volumes,
platform=request.platform,
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ def create_workload(
egress_auth_token: Optional[str] = None,
egress_mode: str = EGRESS_MODE_DNS,
credential_proxy_enabled: bool = False,
egress_env: Optional[Dict[str, Optional[str]]] = None,
) -> Dict[str, Any]:
"""
Create a new workload resource.
Expand Down
132 changes: 132 additions & 0 deletions server/tests/k8s/test_egress_helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
import json
from typing import Optional

import pytest

from opensandbox_server.api.schema import NetworkPolicy, NetworkRule
from opensandbox_server.config import EGRESS_MODE_DNS, EGRESS_MODE_DNS_NFT
from opensandbox_server.services.constants import (
Expand All @@ -26,6 +28,7 @@
OPENSANDBOX_RUNTIME_MOUNT_PATH,
OPENSANDBOX_RUNTIME_VOLUME_NAME,
)
from opensandbox_server.services.k8s.create_helpers import _split_egress_env
from opensandbox_server.services.k8s.egress_helper import (
apply_egress_to_spec,
build_security_context_for_sandbox_container,
Expand Down Expand Up @@ -379,10 +382,139 @@ def test_no_op_when_no_egress_image(self):

assert len(containers) == 0

def test_extra_env_injected_into_sidecar(self):
containers: list = []
network_policy = NetworkPolicy(
default_action="deny",
egress=[NetworkRule(action="allow", target="example.com")],
)
extra = {
"OPENSANDBOX_EGRESS_MITMPROXY_SCRIPT": "/scripts/auth.py",
"OPENSANDBOX_EGRESS_LOG_LEVEL": "debug",
}

apply_egress_to_spec(
containers,
network_policy,
"opensandbox/egress:v1.1.0",
extra_env=extra,
)

env_by_name = {e["name"]: e["value"] for e in containers[0]["env"]}
assert env_by_name["OPENSANDBOX_EGRESS_MITMPROXY_SCRIPT"] == "/scripts/auth.py"
assert env_by_name["OPENSANDBOX_EGRESS_LOG_LEVEL"] == "debug"

def test_extra_env_none_value_becomes_empty_string(self):
containers: list = []
network_policy = NetworkPolicy(
default_action="deny",
egress=[NetworkRule(action="allow", target="example.com")],
)

apply_egress_to_spec(
containers,
network_policy,
"opensandbox/egress:v1.1.0",
extra_env={"OPENSANDBOX_EGRESS_LOG_LEVEL": None},
)

env_by_name = {e["name"]: e["value"] for e in containers[0]["env"]}
assert env_by_name["OPENSANDBOX_EGRESS_LOG_LEVEL"] == ""

def test_extra_env_mitm_ignored_when_credential_proxy_enabled(self):
containers: list = []
network_policy = NetworkPolicy(
default_action="deny",
egress=[NetworkRule(action="allow", target="example.com")],
)

apply_egress_to_spec(
containers,
network_policy,
"opensandbox/egress:v1.1.0",
credential_proxy_enabled=True,
extra_env={"OPENSANDBOX_EGRESS_MITMPROXY_TRANSPARENT": "false"},
)

mitm_vals = [
e["value"]
for e in containers[0]["env"]
if e["name"] == OPENSANDBOX_EGRESS_MITMPROXY_TRANSPARENT
]
assert mitm_vals == ["true"]

def test_extra_env_empty_dict_is_noop(self):
containers: list = []
network_policy = NetworkPolicy(
default_action="deny",
egress=[NetworkRule(action="allow", target="example.com")],
)

apply_egress_to_spec(
containers,
network_policy,
"opensandbox/egress:v1.1.0",
extra_env={},
)

env_names = {e["name"] for e in containers[0]["env"]}
assert env_names == {EGRESS_RULES_ENV, EGRESS_MODE_ENV}


class TestPrepExecdInitForEgress:
def test_returns_privileged_security_dict_and_prefixed_script(self):
base = "cp ./execd /opt/opensandbox/execd"
script, sc = prep_execd_init_for_egress(base)
assert sc == {"privileged": True}
assert "/proc/sys/net/ipv6/conf/all/disable_ipv6" in script
assert script.endswith(base)


class TestSplitEgressEnv:
def test_splits_by_prefix(self):
env = {
"MY_VAR": "hello",
"OPENSANDBOX_EGRESS_LOG_LEVEL": "debug",
"OTHER": "world",
}
sandbox_env, egress_env = _split_egress_env(env)
assert sandbox_env == {"MY_VAR": "hello", "OTHER": "world"}
assert egress_env == {"OPENSANDBOX_EGRESS_LOG_LEVEL": "debug"}

def test_none_returns_empty_dicts(self):
sandbox_env, egress_env = _split_egress_env(None)
assert sandbox_env == {}
assert egress_env == {}

def test_empty_returns_empty_dicts(self):
sandbox_env, egress_env = _split_egress_env({})
assert sandbox_env == {}
assert egress_env == {}

def test_no_egress_vars(self):
env = {"FOO": "bar", "BAZ": "qux"}
sandbox_env, egress_env = _split_egress_env(env)
assert sandbox_env == env
assert egress_env == {}

def test_rejects_reserved_rules(self):
with pytest.raises(ValueError, match="reserved"):
_split_egress_env({"OPENSANDBOX_EGRESS_RULES": "evil"})

def test_rejects_reserved_mode(self):
with pytest.raises(ValueError, match="reserved"):
_split_egress_env({"OPENSANDBOX_EGRESS_MODE": "evil"})

def test_rejects_reserved_token(self):
with pytest.raises(ValueError, match="reserved"):
_split_egress_env({"OPENSANDBOX_EGRESS_TOKEN": "evil"})

def test_allows_mitmproxy_transparent(self):
env = {"OPENSANDBOX_EGRESS_MITMPROXY_TRANSPARENT": "true"}
sandbox_env, egress_env = _split_egress_env(env)
assert sandbox_env == {"OPENSANDBOX_EGRESS_MITMPROXY_TRANSPARENT": "true"}
assert egress_env == {"OPENSANDBOX_EGRESS_MITMPROXY_TRANSPARENT": "true"}

def test_rejects_reserved_http_addr(self):
with pytest.raises(ValueError, match="reserved"):
_split_egress_env({"OPENSANDBOX_EGRESS_HTTP_ADDR": "0.0.0.0:9999"})
Loading