mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
path-uri: add lexical containment (#29614)
## Why Executor-owned paths must stay portable while the orchestrator reasons about them. Converting a Windows or remote path to the orchestrator host's native path just to check containment breaks that boundary. ## What changed - Add lexical containment to `PathUri`. - Compare URI authorities and complete path segments, so `plugin-other` is not treated as a child of `plugin`. - Fail closed for encoded path separators and opaque fallback URIs. For example: ```text file:///C:/plugins/foo/assets/icon.svg is below file:///C:/plugins/foo file:///C:/plugins/foo2/icon.svg is not below file:///C:/plugins/foo ``` This is the shared foundation for keeping executor-owned plugin resources URI-native without consulting the orchestrator filesystem.
This commit is contained in:
@@ -247,6 +247,33 @@ impl PathUri {
|
||||
std::iter::successors(Some(self.clone()), Self::parent)
|
||||
}
|
||||
|
||||
/// Returns true when this URI is lexically equal to or below `base`.
|
||||
///
|
||||
/// Containment is computed using URI authority and path-segment boundaries,
|
||||
/// without consulting the host filesystem. Percent-encoded path separators
|
||||
/// fail closed because native path conversion may interpret them as segment
|
||||
/// boundaries. Opaque fallback URIs created by [`Self::from_abs_path`] only
|
||||
/// contain themselves.
|
||||
pub fn starts_with(&self, base: &Self) -> bool {
|
||||
if self == base {
|
||||
return true;
|
||||
}
|
||||
if decode_bad_path_uri(&self.0).is_some() || decode_bad_path_uri(&base.0).is_some() {
|
||||
return false;
|
||||
}
|
||||
if self.0.host_str() != base.0.host_str() {
|
||||
return false;
|
||||
}
|
||||
|
||||
let Some(path_segments) = containment_path_segments(&self.0) else {
|
||||
return false;
|
||||
};
|
||||
let Some(base_segments) = containment_path_segments(&base.0) else {
|
||||
return false;
|
||||
};
|
||||
path_segments.starts_with(&base_segments)
|
||||
}
|
||||
|
||||
/// Lexically resolves native absolute or relative path text against this URI.
|
||||
///
|
||||
/// Path text is interpreted using the POSIX or Windows convention inferred
|
||||
@@ -542,6 +569,19 @@ fn is_windows_drive_uri_segment(segment: &str) -> bool {
|
||||
matches!(segment.as_bytes(), [drive, b':'] if drive.is_ascii_alphabetic())
|
||||
}
|
||||
|
||||
fn containment_path_segments(url: &Url) -> Option<Vec<&str>> {
|
||||
let segments = url
|
||||
.path_segments()?
|
||||
.filter(|segment| !segment.is_empty())
|
||||
.collect::<Vec<_>>();
|
||||
(!segments.iter().any(|segment| {
|
||||
urlencoding::decode_binary(segment.as_bytes())
|
||||
.iter()
|
||||
.any(|byte| matches!(*byte, b'/' | b'\\'))
|
||||
}))
|
||||
.then_some(segments)
|
||||
}
|
||||
|
||||
fn infer_opaque_path_convention(path_bytes: &[u8]) -> Option<PathConvention> {
|
||||
if path_bytes.starts_with(b"/") {
|
||||
return Some(PathConvention::Posix);
|
||||
|
||||
@@ -283,9 +283,16 @@ fn structurally_valid_bad_path_uri_with_invalid_native_payload_fails_conversion(
|
||||
fn bad_path_uris_are_opaque_to_lexical_operations() {
|
||||
let uri = PathUri::parse("file:///%00/bad/path/YQ")
|
||||
.expect("canonical base64 fallback URI should parse");
|
||||
let other = PathUri::parse("file:///%00/bad/path/Yg")
|
||||
.expect("canonical base64 fallback URI should parse");
|
||||
let root = PathUri::parse("file:///").expect("valid root URI");
|
||||
|
||||
assert_eq!(uri.basename(), None);
|
||||
assert_eq!(uri.parent(), None);
|
||||
assert!(uri.starts_with(&uri));
|
||||
assert!(!uri.starts_with(&root));
|
||||
assert!(!uri.starts_with(&other));
|
||||
assert!(!other.starts_with(&uri));
|
||||
assert_eq!(uri.join(""), Ok(uri.clone()));
|
||||
assert_eq!(
|
||||
uri.join("child"),
|
||||
@@ -698,6 +705,58 @@ fn join_uses_the_base_uri_path_convention() {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn starts_with_uses_uri_segment_boundaries() {
|
||||
for (path, base, expected) in [
|
||||
("file:///workspace/plugin", "file:///", true),
|
||||
("file:///workspace/plugin", "file:///workspace/plugin", true),
|
||||
(
|
||||
"file:///workspace/plugin/assets/icon.svg",
|
||||
"file:///workspace/plugin",
|
||||
true,
|
||||
),
|
||||
(
|
||||
"file:///workspace/plugin-other/icon.svg",
|
||||
"file:///workspace/plugin",
|
||||
false,
|
||||
),
|
||||
(
|
||||
"file:///C:/plugins/foo/assets/icon.svg",
|
||||
"file:///C:/plugins/foo",
|
||||
true,
|
||||
),
|
||||
(
|
||||
"file:///C:/plugins/foo2/assets/icon.svg",
|
||||
"file:///C:/plugins/foo",
|
||||
false,
|
||||
),
|
||||
(
|
||||
"file://server/share/plugins/foo/icon.svg",
|
||||
"file://server/share/plugins/foo",
|
||||
true,
|
||||
),
|
||||
(
|
||||
"file://other/share/plugins/foo/icon.svg",
|
||||
"file://server/share/plugins/foo",
|
||||
false,
|
||||
),
|
||||
(
|
||||
"file:///workspace/plugin/%2F..%2Foutside",
|
||||
"file:///workspace/plugin",
|
||||
false,
|
||||
),
|
||||
(
|
||||
"file:///workspace/plugin/%5C..%5Coutside",
|
||||
"file:///workspace/plugin",
|
||||
false,
|
||||
),
|
||||
] {
|
||||
let path = PathUri::parse(path).expect("valid path URI");
|
||||
let base = PathUri::parse(base).expect("valid base URI");
|
||||
assert_eq!(path.starts_with(&base), expected);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn to_url_returns_the_validated_url() {
|
||||
let uri = PathUri::parse("file://localhost/workspace/a%20file.rs").expect("valid file URI");
|
||||
|
||||
Reference in New Issue
Block a user