Skip to content

Commit aafe073

Browse files
mjamivdrew
authored andcommitted
fix(kubernetes): address PVC review follow-ups
1 parent 30ad385 commit aafe073

4 files changed

Lines changed: 201 additions & 176 deletions

File tree

crates/openshell-core/src/driver_mounts.rs

Lines changed: 29 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33

44
//! Shared validation helpers for driver-config mounts.
55
6-
use std::collections::HashSet;
76
use std::path::Path;
87

98
/// `SELinux` relabelling mode for bind mounts.
@@ -92,6 +91,19 @@ pub fn validate_container_mount_target(target: &str) -> Result<(), String> {
9291
if !target.starts_with('/') {
9392
return Err("mount target must be an absolute container path".to_string());
9493
}
94+
if target != "/" {
95+
let segments = target.split('/').skip(1).collect::<Vec<_>>();
96+
let has_internal_empty_segment = segments
97+
.iter()
98+
.take(segments.len().saturating_sub(1))
99+
.any(|segment| segment.is_empty());
100+
if has_internal_empty_segment || segments.contains(&".") {
101+
return Err(
102+
"mount target must be normalized and must not contain empty path segments or '.'"
103+
.to_string(),
104+
);
105+
}
106+
}
95107
let path = Path::new(target);
96108
if path == Path::new("/") {
97109
return Err("mount target must not be the container root".to_string());
@@ -128,22 +140,6 @@ pub fn path_is_or_under(path: &Path, parent: &Path) -> bool {
128140
path == parent || path.starts_with(parent)
129141
}
130142

131-
/// Validate that already-normalized driver mount targets are unique.
132-
pub fn validate_unique_mount_targets<'a>(
133-
targets: impl IntoIterator<Item = &'a str>,
134-
driver_name: &str,
135-
) -> Result<(), String> {
136-
let mut seen = HashSet::new();
137-
for target in targets {
138-
if !seen.insert(target) {
139-
return Err(format!(
140-
"duplicate {driver_name} driver_config mount target '{target}'"
141-
));
142-
}
143-
}
144-
Ok(())
145-
}
146-
147143
#[cfg(test)]
148144
mod tests {
149145
use super::*;
@@ -180,33 +176,6 @@ mod tests {
180176
validate_container_mount_target("/etc/openshell-tools").unwrap();
181177
}
182178

183-
#[test]
184-
fn path_is_or_under_matches_boundaries() {
185-
assert!(path_is_or_under(
186-
Path::new("/sandbox"),
187-
Path::new("/sandbox")
188-
));
189-
assert!(path_is_or_under(
190-
Path::new("/sandbox/work"),
191-
Path::new("/sandbox")
192-
));
193-
assert!(!path_is_or_under(
194-
Path::new("/sandbox-work"),
195-
Path::new("/sandbox")
196-
));
197-
}
198-
199-
#[test]
200-
fn unique_mount_targets_rejects_duplicates() {
201-
let err =
202-
validate_unique_mount_targets(["/sandbox/work", "/sandbox/work"], "test").unwrap_err();
203-
204-
assert_eq!(
205-
err,
206-
"duplicate test driver_config mount target '/sandbox/work'"
207-
);
208-
}
209-
210179
#[test]
211180
fn mount_subpath_must_be_relative_without_parent_dirs() {
212181
assert!(validate_mount_subpath("project/a").is_ok());
@@ -230,4 +199,20 @@ mod tests {
230199
"mount target must not contain surrounding whitespace"
231200
);
232201
}
202+
#[test]
203+
fn mount_target_rejects_internal_empty_or_dot_segments() {
204+
assert_eq!(
205+
validate_container_mount_target("/sandbox/work//tmp").unwrap_err(),
206+
"mount target must be normalized and must not contain empty path segments or '.'"
207+
);
208+
assert_eq!(
209+
validate_container_mount_target("/sandbox/work/./tmp").unwrap_err(),
210+
"mount target must be normalized and must not contain empty path segments or '.'"
211+
);
212+
assert_eq!(
213+
validate_container_mount_target("/sandbox/work/../../tmp").unwrap_err(),
214+
"mount target must not contain '..'"
215+
);
216+
validate_container_mount_target("/sandbox/work/").unwrap();
217+
}
233218
}

crates/openshell-driver-docker/src/lib.rs

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ use openshell_core::proto_struct::{
4949
deserialize_optional_non_empty_string_list, struct_to_json_value,
5050
};
5151
use openshell_core::{Config, Error, Result as CoreResult};
52-
use std::collections::HashMap;
52+
use std::collections::{HashMap, HashSet};
5353
use std::io::Read;
5454
use std::net::{IpAddr, SocketAddr};
5555
use std::path::{Path, PathBuf};
@@ -1845,7 +1845,7 @@ fn validate_docker_driver_mounts(
18451845
mounts: &[DockerDriverMountConfig],
18461846
enable_bind_mounts: bool,
18471847
) -> Result<(), Status> {
1848-
let mut targets = Vec::with_capacity(mounts.len());
1848+
let mut targets = HashSet::new();
18491849
for mount in mounts {
18501850
let target = match mount {
18511851
DockerDriverMountConfig::Bind { source, target, .. } => {
@@ -1899,10 +1899,14 @@ fn validate_docker_driver_mounts(
18991899
};
19001900
driver_mounts::validate_container_mount_target(target)
19011901
.map_err(Status::failed_precondition)?;
1902-
targets.push(driver_mounts::normalize_mount_target(target));
1902+
let normalized_target = driver_mounts::normalize_mount_target(target);
1903+
if !targets.insert(normalized_target.clone()) {
1904+
return Err(Status::failed_precondition(format!(
1905+
"duplicate docker driver_config mount target '{normalized_target}'"
1906+
)));
1907+
}
19031908
}
1904-
driver_mounts::validate_unique_mount_targets(targets.iter().map(String::as_str), "docker")
1905-
.map_err(Status::failed_precondition)
1909+
Ok(())
19061910
}
19071911

19081912
fn validate_optional_positive_integral_i64(

0 commit comments

Comments
 (0)