fix: validate delegation path sets exactly
This commit is contained in:
parent
a4a9b002c6
commit
f43c8ac65c
|
|
@ -97,35 +97,58 @@ fn permission_denies_requested(denied: Permission, requested: Permission) -> boo
|
||||||
}
|
}
|
||||||
|
|
||||||
fn rule_covers(available: &ResolvedRule, requested: &ResolvedRule) -> bool {
|
fn rule_covers(available: &ResolvedRule, requested: &ResolvedRule) -> bool {
|
||||||
if !permission_covers(available.permission, requested.permission) {
|
permission_covers(available.permission, requested.permission)
|
||||||
return false;
|
&& rule_path_set_contains(available, requested)
|
||||||
}
|
|
||||||
if available.recursive {
|
|
||||||
return requested.target.starts_with(&available.target);
|
|
||||||
}
|
|
||||||
!requested.recursive
|
|
||||||
&& (requested.target == available.target
|
|
||||||
|| direct_child(&requested.target, &available.target))
|
|
||||||
}
|
}
|
||||||
|
|
||||||
fn denial_overlaps_requested(deny: &ResolvedRule, requested: &ResolvedRule) -> bool {
|
fn denial_overlaps_requested(deny: &ResolvedRule, requested: &ResolvedRule) -> bool {
|
||||||
if !permission_denies_requested(deny.permission, requested.permission) {
|
permission_denies_requested(deny.permission, requested.permission)
|
||||||
return false;
|
&& rule_path_sets_overlap(deny, requested)
|
||||||
|
}
|
||||||
|
|
||||||
|
fn rule_path_set_contains(available: &ResolvedRule, requested: &ResolvedRule) -> bool {
|
||||||
|
match (available.recursive, requested.recursive) {
|
||||||
|
// A recursive grant contains every possible requested path below its target.
|
||||||
|
(true, _) => requested.target.starts_with(&available.target),
|
||||||
|
// A non-recursive grant contains only the target and its direct children;
|
||||||
|
// a recursive request always includes descendants beyond that finite-depth
|
||||||
|
// set.
|
||||||
|
(false, true) => false,
|
||||||
|
// Two non-recursive rules have the same finite-depth set only when their
|
||||||
|
// target is identical. A request rooted at a direct child would also grant
|
||||||
|
// that child's children, which are grandchildren of `available.target`.
|
||||||
|
(false, false) => requested.target == available.target,
|
||||||
}
|
}
|
||||||
match (deny.recursive, requested.recursive) {
|
}
|
||||||
|
|
||||||
|
fn rule_path_sets_overlap(left: &ResolvedRule, right: &ResolvedRule) -> bool {
|
||||||
|
match (left.recursive, right.recursive) {
|
||||||
(true, true) => {
|
(true, true) => {
|
||||||
deny.target.starts_with(&requested.target) || requested.target.starts_with(&deny.target)
|
left.target.starts_with(&right.target) || right.target.starts_with(&left.target)
|
||||||
}
|
}
|
||||||
(true, false) => requested.target.starts_with(&deny.target),
|
(true, false) => recursive_and_non_recursive_sets_overlap(left, right),
|
||||||
(false, true) => deny.target.starts_with(&requested.target),
|
(false, true) => recursive_and_non_recursive_sets_overlap(right, left),
|
||||||
(false, false) => {
|
(false, false) => {
|
||||||
deny.target == requested.target
|
left.target == right.target
|
||||||
|| direct_child(&deny.target, &requested.target)
|
|| direct_child(&left.target, &right.target)
|
||||||
|| direct_child(&requested.target, &deny.target)
|
|| direct_child(&right.target, &left.target)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn recursive_and_non_recursive_sets_overlap(
|
||||||
|
recursive: &ResolvedRule,
|
||||||
|
non_recursive: &ResolvedRule,
|
||||||
|
) -> bool {
|
||||||
|
// The non-recursive set is `{target} + direct children`. It overlaps a
|
||||||
|
// recursive subtree when either the non-recursive target is inside that
|
||||||
|
// subtree, or the recursive subtree begins at the non-recursive target or
|
||||||
|
// one of its direct children.
|
||||||
|
non_recursive.target.starts_with(&recursive.target)
|
||||||
|
|| recursive.target == non_recursive.target
|
||||||
|
|| direct_child(&recursive.target, &non_recursive.target)
|
||||||
|
}
|
||||||
|
|
||||||
fn direct_child(child: &Path, parent: &Path) -> bool {
|
fn direct_child(child: &Path, parent: &Path) -> bool {
|
||||||
child.parent().is_some_and(|candidate| candidate == parent)
|
child.parent().is_some_and(|candidate| candidate == parent)
|
||||||
}
|
}
|
||||||
|
|
@ -517,14 +540,18 @@ mod tests {
|
||||||
use super::*;
|
use super::*;
|
||||||
use tempfile::TempDir;
|
use tempfile::TempDir;
|
||||||
|
|
||||||
fn allow_rule(target: &Path, permission: Permission) -> ScopeRule {
|
fn rule(target: &Path, permission: Permission, recursive: bool) -> ScopeRule {
|
||||||
ScopeRule {
|
ScopeRule {
|
||||||
target: target.to_path_buf(),
|
target: target.to_path_buf(),
|
||||||
permission,
|
permission,
|
||||||
recursive: true,
|
recursive,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn allow_rule(target: &Path, permission: Permission) -> ScopeRule {
|
||||||
|
rule(target, permission, true)
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn writable_shortcut_permits_root() {
|
fn writable_shortcut_permits_root() {
|
||||||
let dir = TempDir::new().unwrap();
|
let dir = TempDir::new().unwrap();
|
||||||
|
|
@ -640,6 +667,80 @@ mod tests {
|
||||||
assert!(!scope.is_writable(&nested.join("deep.txt")));
|
assert!(!scope.is_writable(&nested.join("deep.txt")));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn delegation_non_recursive_grant_rejects_child_non_recursive_request() {
|
||||||
|
let dir = TempDir::new().unwrap();
|
||||||
|
let child = dir.path().join("child");
|
||||||
|
std::fs::create_dir(&child).unwrap();
|
||||||
|
let delegation = DelegationScope::from_config(&ScopeConfig {
|
||||||
|
allow: vec![rule(dir.path(), Permission::Write, false)],
|
||||||
|
deny: Vec::new(),
|
||||||
|
})
|
||||||
|
.unwrap();
|
||||||
|
|
||||||
|
assert!(
|
||||||
|
!delegation
|
||||||
|
.allows_rule(&rule(&child, Permission::Write, false))
|
||||||
|
.unwrap(),
|
||||||
|
"a non-recursive child request includes grandchildren outside the parent non-recursive grant"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn delegation_non_recursive_grant_allows_exact_non_recursive_request() {
|
||||||
|
let dir = TempDir::new().unwrap();
|
||||||
|
let delegation = DelegationScope::from_config(&ScopeConfig {
|
||||||
|
allow: vec![rule(dir.path(), Permission::Write, false)],
|
||||||
|
deny: Vec::new(),
|
||||||
|
})
|
||||||
|
.unwrap();
|
||||||
|
|
||||||
|
assert!(
|
||||||
|
delegation
|
||||||
|
.allows_rule(&rule(dir.path(), Permission::Write, false))
|
||||||
|
.unwrap(),
|
||||||
|
"identical non-recursive path sets should be delegable"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn delegation_recursive_grant_allows_child_non_recursive_request() {
|
||||||
|
let dir = TempDir::new().unwrap();
|
||||||
|
let child = dir.path().join("child");
|
||||||
|
std::fs::create_dir(&child).unwrap();
|
||||||
|
let delegation = DelegationScope::from_config(&ScopeConfig {
|
||||||
|
allow: vec![rule(dir.path(), Permission::Write, true)],
|
||||||
|
deny: Vec::new(),
|
||||||
|
})
|
||||||
|
.unwrap();
|
||||||
|
|
||||||
|
assert!(
|
||||||
|
delegation
|
||||||
|
.allows_rule(&rule(&child, Permission::Write, false))
|
||||||
|
.unwrap(),
|
||||||
|
"recursive parent grants cover non-recursive child path sets"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn delegation_non_recursive_deny_overlaps_child_recursive_request() {
|
||||||
|
let dir = TempDir::new().unwrap();
|
||||||
|
let child = dir.path().join("child");
|
||||||
|
std::fs::create_dir(&child).unwrap();
|
||||||
|
let delegation = DelegationScope::from_config(&ScopeConfig {
|
||||||
|
allow: vec![rule(dir.path(), Permission::Write, true)],
|
||||||
|
deny: vec![rule(dir.path(), Permission::Write, false)],
|
||||||
|
})
|
||||||
|
.unwrap();
|
||||||
|
|
||||||
|
assert!(
|
||||||
|
!delegation
|
||||||
|
.allows_rule(&rule(&child, Permission::Write, true))
|
||||||
|
.unwrap(),
|
||||||
|
"non-recursive deny at the parent includes the direct child path requested recursively"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn empty_allow_rejected() {
|
fn empty_allow_rejected() {
|
||||||
let cfg = ScopeConfig {
|
let cfg = ScopeConfig {
|
||||||
|
|
|
||||||
|
|
@ -192,6 +192,13 @@ fn dummy_manifest_with_delegation(allow_root: &Path, allow_delegation: bool) ->
|
||||||
} else {
|
} else {
|
||||||
ScopeConfig::default()
|
ScopeConfig::default()
|
||||||
};
|
};
|
||||||
|
dummy_manifest_with_scopes(direct_scope, delegation_scope)
|
||||||
|
}
|
||||||
|
|
||||||
|
fn dummy_manifest_with_scopes(
|
||||||
|
direct_scope: ScopeConfig,
|
||||||
|
delegation_scope: ScopeConfig,
|
||||||
|
) -> PodManifest {
|
||||||
PodManifestConfig {
|
PodManifestConfig {
|
||||||
pod: PodMetaConfig {
|
pod: PodMetaConfig {
|
||||||
name: Some("root".into()),
|
name: Some("root".into()),
|
||||||
|
|
@ -366,6 +373,75 @@ async fn spawn_pod_requires_explicit_delegation_even_with_direct_scope() {
|
||||||
clear_env();
|
clear_env();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn spawn_pod_rejects_child_non_recursive_scope_under_parent_non_recursive_delegation() {
|
||||||
|
let _env = EnvGuard::acquire();
|
||||||
|
|
||||||
|
let allow_root = TempDir::new().unwrap();
|
||||||
|
let child = allow_root.path().join("child");
|
||||||
|
std::fs::create_dir(&child).unwrap();
|
||||||
|
let (_tmp, runtime_base, spawner_socket, spawner_rd) =
|
||||||
|
setup_spawner("root", allow_root.path()).await;
|
||||||
|
|
||||||
|
let direct_scope = ScopeConfig {
|
||||||
|
allow: vec![ScopeRule {
|
||||||
|
target: allow_root.path().to_path_buf(),
|
||||||
|
permission: Permission::Write,
|
||||||
|
recursive: true,
|
||||||
|
}],
|
||||||
|
deny: Vec::new(),
|
||||||
|
};
|
||||||
|
let delegation_scope = ScopeConfig {
|
||||||
|
allow: vec![ScopeRule {
|
||||||
|
target: allow_root.path().to_path_buf(),
|
||||||
|
permission: Permission::Write,
|
||||||
|
recursive: false,
|
||||||
|
}],
|
||||||
|
deny: Vec::new(),
|
||||||
|
};
|
||||||
|
let manifest = dummy_manifest_with_scopes(direct_scope, delegation_scope);
|
||||||
|
|
||||||
|
let registry = SpawnedPodRegistry::new(spawner_rd.clone());
|
||||||
|
let def = spawn_pod_tool_with_runtime_command(
|
||||||
|
"root".into(),
|
||||||
|
spawner_socket,
|
||||||
|
runtime_base,
|
||||||
|
allow_root.path().to_path_buf(),
|
||||||
|
registry,
|
||||||
|
None,
|
||||||
|
manifest,
|
||||||
|
shared_scope_for(allow_root.path()),
|
||||||
|
builtin_prompts(),
|
||||||
|
mock_runtime_command(),
|
||||||
|
);
|
||||||
|
let (_meta, tool) = def();
|
||||||
|
|
||||||
|
let input = json!({
|
||||||
|
"name": "child-nonrecursive-overgrant",
|
||||||
|
"task": "hello",
|
||||||
|
"profile": "inherit",
|
||||||
|
"scope": [{
|
||||||
|
"target": child.to_str().unwrap(),
|
||||||
|
"permission": "write",
|
||||||
|
"recursive": false
|
||||||
|
}]
|
||||||
|
})
|
||||||
|
.to_string();
|
||||||
|
|
||||||
|
let err = tool.execute(&input).await.unwrap_err();
|
||||||
|
match err {
|
||||||
|
ToolError::InvalidArgument(message) => {
|
||||||
|
assert!(
|
||||||
|
message.contains("outside this Pod's delegation scope grant"),
|
||||||
|
"{message}"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
other => panic!("expected InvalidArgument, got {other:?}"),
|
||||||
|
}
|
||||||
|
|
||||||
|
clear_env();
|
||||||
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn spawn_pod_rejects_scope_outside_spawner() {
|
async fn spawn_pod_rejects_scope_outside_spawner() {
|
||||||
let _env = EnvGuard::acquire();
|
let _env = EnvGuard::acquire();
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue
Block a user