Skip to content

Commit b00108b

Browse files
committed
fix(drivers): tighten local mount validation
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
1 parent 1c044a4 commit b00108b

11 files changed

Lines changed: 552 additions & 288 deletions

File tree

architecture/compute-runtimes.md

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -42,12 +42,13 @@ but currently ignores them.
4242

4343
Docker and Podman also accept per-sandbox driver-config mounts for existing
4444
runtime-managed named volumes and tmpfs mounts. Podman additionally accepts
45-
image mounts through its image-volume API. User-supplied host bind mounts are
46-
available only when explicitly enabled in the active local driver table of
47-
`gateway.toml`. Host bind mounts are an unsafe operator override because they
48-
place gateway-host filesystem state inside the sandbox and can negate OpenShell
49-
workspace isolation and filesystem-policy controls. Driver-owned supervisor,
50-
token, and TLS bind mounts stay reserved.
45+
image mounts through its image-volume API. User-supplied bind and volume mounts
46+
default to read-only. Direct host bind mounts, and Docker local-driver
47+
bind-backed named volumes, are available only when explicitly enabled in the
48+
active local driver table of `gateway.toml`. Host bind mounts are an unsafe
49+
operator override because they place gateway-host filesystem state inside the
50+
sandbox and can negate OpenShell workspace isolation and filesystem-policy
51+
controls. Driver-owned supervisor, token, and TLS bind mounts stay reserved.
5152

5253
Kubernetes deployments may set an AppArmor profile on sandbox agent containers
5354
through the driver configuration. The Helm chart defaults sandbox agents to
Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,153 @@
1+
// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
//! Shared validation helpers for driver-config mounts.
5+
6+
use std::path::Path;
7+
8+
const RESERVED_MOUNT_TARGETS: &[&str] = &[
9+
"/opt/openshell",
10+
"/etc/openshell",
11+
"/etc/openshell-tls",
12+
"/run/netns",
13+
];
14+
15+
/// Validate a non-empty driver mount source.
16+
pub fn validate_mount_source(source: &str, field: &str) -> Result<String, String> {
17+
let source = source.trim();
18+
if source.is_empty() {
19+
return Err(format!("{field} must not be empty"));
20+
}
21+
if source.as_bytes().contains(&0) {
22+
return Err(format!("{field} must not contain NUL bytes"));
23+
}
24+
Ok(source.to_string())
25+
}
26+
27+
/// Validate a bind mount source as an absolute host path.
28+
pub fn validate_absolute_mount_source(source: &str, field: &str) -> Result<String, String> {
29+
let source = validate_mount_source(source, field)?;
30+
if !Path::new(&source).is_absolute() {
31+
return Err(format!("{field} must be an absolute host path"));
32+
}
33+
Ok(source)
34+
}
35+
36+
/// Validate a relative subpath inside a runtime-managed mount source.
37+
pub fn validate_mount_subpath(subpath: &str) -> Result<String, String> {
38+
let subpath = subpath.trim();
39+
if subpath.is_empty() {
40+
return Err("mount subpath must not be empty".to_string());
41+
}
42+
if subpath.as_bytes().contains(&0) {
43+
return Err("mount subpath must not contain NUL bytes".to_string());
44+
}
45+
let path = Path::new(subpath);
46+
if path.is_absolute()
47+
|| path
48+
.components()
49+
.any(|component| matches!(component, std::path::Component::ParentDir))
50+
{
51+
return Err("mount subpath must be relative and must not contain '..'".to_string());
52+
}
53+
Ok(subpath.to_string())
54+
}
55+
56+
/// Validate a container-side mount target for user-supplied driver mounts.
57+
pub fn validate_container_mount_target(target: &str) -> Result<String, String> {
58+
let target = normalize_container_mount_target(target);
59+
if target.is_empty() {
60+
return Err("mount target must not be empty".to_string());
61+
}
62+
if target.as_bytes().contains(&0) {
63+
return Err("mount target must not contain NUL bytes".to_string());
64+
}
65+
if !target.starts_with('/') {
66+
return Err("mount target must be an absolute container path".to_string());
67+
}
68+
if target == "/" {
69+
return Err("mount target must not be the container root".to_string());
70+
}
71+
let path = Path::new(&target);
72+
if path
73+
.components()
74+
.any(|component| matches!(component, std::path::Component::ParentDir))
75+
{
76+
return Err("mount target must not contain '..'".to_string());
77+
}
78+
if target == "/sandbox" {
79+
return Err("mount target '/sandbox' is reserved for the OpenShell workspace".to_string());
80+
}
81+
for reserved in RESERVED_MOUNT_TARGETS {
82+
if path_is_or_under(&target, reserved) {
83+
return Err(format!(
84+
"mount target '{target}' conflicts with reserved OpenShell path '{reserved}'"
85+
));
86+
}
87+
}
88+
Ok(target)
89+
}
90+
91+
fn normalize_container_mount_target(target: &str) -> String {
92+
let target = target.trim();
93+
if target == "/" {
94+
return target.to_string();
95+
}
96+
target.trim_end_matches('/').to_string()
97+
}
98+
99+
fn path_is_or_under(path: &str, parent: &str) -> bool {
100+
path == parent
101+
|| path
102+
.strip_prefix(parent)
103+
.is_some_and(|rest| rest.starts_with('/'))
104+
}
105+
106+
#[cfg(test)]
107+
mod tests {
108+
use super::*;
109+
110+
#[test]
111+
fn container_target_allows_paths_under_workspace() {
112+
assert_eq!(
113+
validate_container_mount_target("/sandbox/work/").unwrap(),
114+
"/sandbox/work"
115+
);
116+
}
117+
118+
#[test]
119+
fn container_target_rejects_workspace_root_only() {
120+
let err = validate_container_mount_target("/sandbox/").unwrap_err();
121+
122+
assert!(err.contains("reserved for the OpenShell workspace"));
123+
}
124+
125+
#[test]
126+
fn container_target_rejects_reserved_openshell_tls_legacy_path() {
127+
let err = validate_container_mount_target("/etc/openshell-tls/client").unwrap_err();
128+
129+
assert!(err.contains("/etc/openshell-tls"));
130+
}
131+
132+
#[test]
133+
fn container_target_rejects_reserved_openshell_tree() {
134+
let err = validate_container_mount_target("/etc/openshell/tls/client").unwrap_err();
135+
136+
assert!(err.contains("/etc/openshell"));
137+
}
138+
139+
#[test]
140+
fn container_target_does_not_prefix_match_unrelated_paths() {
141+
assert_eq!(
142+
validate_container_mount_target("/etc/openshell-tools").unwrap(),
143+
"/etc/openshell-tools"
144+
);
145+
}
146+
147+
#[test]
148+
fn mount_subpath_must_be_relative_without_parent_dirs() {
149+
assert_eq!(validate_mount_subpath(" project/a ").unwrap(), "project/a");
150+
assert!(validate_mount_subpath("/project").is_err());
151+
assert!(validate_mount_subpath("../project").is_err());
152+
}
153+
}

crates/openshell-core/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
1212
pub mod auth;
1313
pub mod config;
14+
pub mod driver_mounts;
1415
pub mod driver_utils;
1516
pub mod error;
1617
pub mod forward;
@@ -22,6 +23,7 @@ pub mod net;
2223
pub mod paths;
2324
pub mod progress;
2425
pub mod proto;
26+
pub mod proto_struct;
2527
pub mod sandbox_env;
2628
pub mod settings;
2729
pub mod telemetry;
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
//! Helpers for decoding `google.protobuf.Struct` values.
5+
6+
/// Convert a protobuf Struct into a JSON object for typed serde decoding.
7+
#[must_use]
8+
pub fn struct_to_json_object(
9+
config: &prost_types::Struct,
10+
) -> serde_json::Map<String, serde_json::Value> {
11+
config
12+
.fields
13+
.iter()
14+
.map(|(key, value)| (key.clone(), value_to_json(value)))
15+
.collect()
16+
}
17+
18+
/// Convert a protobuf Struct into a JSON value for typed serde decoding.
19+
#[must_use]
20+
pub fn struct_to_json_value(config: &prost_types::Struct) -> serde_json::Value {
21+
serde_json::Value::Object(struct_to_json_object(config))
22+
}
23+
24+
/// Convert a protobuf Value into a JSON value for typed serde decoding.
25+
#[must_use]
26+
pub fn value_to_json(value: &prost_types::Value) -> serde_json::Value {
27+
match value.kind.as_ref() {
28+
Some(prost_types::value::Kind::NumberValue(num)) => serde_json::Number::from_f64(*num)
29+
.map_or(serde_json::Value::Null, serde_json::Value::Number),
30+
Some(prost_types::value::Kind::StringValue(val)) => serde_json::Value::String(val.clone()),
31+
Some(prost_types::value::Kind::BoolValue(val)) => serde_json::Value::Bool(*val),
32+
Some(prost_types::value::Kind::StructValue(val)) => {
33+
serde_json::Value::Object(struct_to_json_object(val))
34+
}
35+
Some(prost_types::value::Kind::ListValue(list)) => {
36+
serde_json::Value::Array(list.values.iter().map(value_to_json).collect())
37+
}
38+
Some(prost_types::value::Kind::NullValue(_)) | None => serde_json::Value::Null,
39+
}
40+
}

crates/openshell-driver-docker/README.md

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ mount types:
4646
has `enable_bind_mounts = true`.
4747
- `volume`: mounts an existing Docker named volume. The driver validates that
4848
the volume exists before provisioning and never creates or removes it.
49+
Docker local-driver volumes created with bind options are treated as host
50+
bind mounts and require `enable_bind_mounts = true`.
4951
- `tmpfs`: mounts an in-memory filesystem with optional `options`,
5052
`size_bytes`, and `mode`.
5153

@@ -58,9 +60,11 @@ the Docker driver-config schema. The driver still uses internal bind mounts
5860
for OpenShell-owned supervisor, token, and TLS material.
5961

6062
Docker `bind` mounts accept `source`, `target`, and optional `read_only`.
61-
Docker `volume` mounts may include `subpath`. Mount targets must be absolute
62-
container paths and must not replace the workspace root (`/sandbox`) or overlap
63-
OpenShell supervisor files, auth material, TLS material, or `/run/netns`.
63+
Docker `volume` mounts may include `subpath`. User-supplied bind and volume
64+
mounts are read-only by default; set `read_only: false` to make them writable.
65+
Mount targets must be absolute container paths and must not replace the
66+
workspace root (`/sandbox`) or overlap OpenShell supervisor files,
67+
`/etc/openshell`, `/etc/openshell-tls`, or `/run/netns`.
6468

6569
Example named-volume usage:
6670

0 commit comments

Comments
 (0)