Skip to content

Commit 5de1bc1

Browse files
committed
feat(execpolicy): match absolute path rules exactly
Typed path rules previously only matched after workspace-relative normalization, which requires the call to live inside the workspace — a rule pinning an absolute location (a real home, /root, a Windows profile, or a literal ~ spelling passed through unexpanded) could never match, leaving home-absolute File-tool reads unmatchable. Add a rooted-rule-only exact-match fallback: separators fold to '/', case folds on case-insensitive platforms, and a relative rule keeps its workspace-relative semantics untouched. No wildcards, so the deny direction keeps its precision. Signed-off-by: asto <asto18089@126.com>
1 parent 681f95a commit 5de1bc1

1 file changed

Lines changed: 171 additions & 7 deletions

File tree

crates/execpolicy/src/lib.rs

Lines changed: 171 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -444,13 +444,24 @@ impl ExecPolicyEngine {
444444
None => true,
445445
})
446446
.filter(|(_, rule)| match (rule.path.as_deref(), ctx.path) {
447-
(Some(pattern), Some(_)) => match (
448-
normalize_workspace_relative_path(pattern, ctx.cwd),
449-
normalized_path.as_deref(),
450-
) {
451-
(Some(pattern), Some(path)) => pattern == path,
452-
_ => false,
453-
},
447+
(Some(pattern), Some(call_path)) => {
448+
let ws_rule = normalize_workspace_relative_path(pattern, ctx.cwd);
449+
match (ws_rule, normalized_path.as_deref()) {
450+
// Workspace-relative normalization fails for a call
451+
// outside the workspace or a rule that names one, and
452+
// on a POSIX host a Windows-spelled rule/call pair
453+
// parses as unrelated relative forms. A rule spelling
454+
// an ABSOLUTE path must still be able to match such a
455+
// call exactly, or pinned locations (a real home,
456+
// `/root`, a Windows profile) are unmatchable. The
457+
// helper only fires for rooted rules, so relative
458+
// semantics are unchanged.
459+
(Some(ws_rule), Some(ws_call)) => {
460+
ws_rule == ws_call || absolute_path_rule_matches(pattern, call_path)
461+
}
462+
_ => absolute_path_rule_matches(pattern, call_path),
463+
}
464+
}
454465
(Some(_), None) => false,
455466
(None, _) => true,
456467
})
@@ -1117,6 +1128,33 @@ fn is_windows_absolute_path(value: &str) -> bool {
11171128
bytes.len() >= 3 && bytes[0].is_ascii_alphabetic() && bytes[1] == b':' && bytes[2] == b'/'
11181129
}
11191130

1131+
/// Exact-match fallback for a typed path rule that names an ABSOLUTE path.
1132+
///
1133+
/// The primary match normalizes both sides to workspace-relative form, which
1134+
/// only succeeds when the call lives inside the workspace — so a rule pinning
1135+
/// a location outside it (a real home, `/root`, another user's home, or a
1136+
/// literal `~` spelling the tool passed through unexpanded) could never match.
1137+
/// This fallback fires only when workspace normalization failed on either
1138+
/// side, and only for a ROOTED rule (leading `/`, `~`, or a Windows drive):
1139+
/// separators fold to `/`, case folds on case-insensitive platforms, and the
1140+
/// comparison is plain equality. A relative rule never reaches it, so
1141+
/// workspace-relative semantics are unchanged, and because there are no
1142+
/// wildcards the deny direction keeps its precision while the allow direction
1143+
/// can only ever match the exact path the rule spells.
1144+
fn absolute_path_rule_matches(rule_path: &str, call_path: &str) -> bool {
1145+
let fold = |value: &str| {
1146+
let value = value.trim().replace('\\', "/");
1147+
if platform_paths_are_case_insensitive() {
1148+
value.to_ascii_lowercase()
1149+
} else {
1150+
value
1151+
}
1152+
};
1153+
let rule = fold(rule_path);
1154+
let rooted = rule.starts_with('/') || rule.starts_with("~/") || is_windows_absolute_path(&rule);
1155+
rooted && rule == fold(call_path)
1156+
}
1157+
11201158
fn has_windows_drive_prefix(value: &str) -> bool {
11211159
let bytes = value.as_bytes();
11221160
bytes.len() >= 2 && bytes[0].is_ascii_alphabetic() && bytes[1] == b':'
@@ -2207,6 +2245,132 @@ mod tests {
22072245
assert!(decision.requires_approval);
22082246
}
22092247

2248+
#[test]
2249+
fn typed_ask_absolute_path_rule_matches_absolute_call_outside_workspace() {
2250+
let engine =
2251+
ExecPolicyEngine::with_rulesets(vec![Ruleset::user(vec![], vec![]).with_ask_rules(
2252+
vec![ToolAskRule {
2253+
tool: "read_file".into(),
2254+
command: None,
2255+
command_exact: false,
2256+
path: Some("/root/.ssh/config".into()),
2257+
workspace: None,
2258+
action: PermissionAction::Deny,
2259+
}],
2260+
)]);
2261+
2262+
// An absolute rule must reach a call outside the workspace that the
2263+
// workspace-relative normalization cannot express.
2264+
let decision = engine
2265+
.check(ExecPolicyContext {
2266+
command: "",
2267+
cwd: "/workspace",
2268+
tool: Some("read_file"),
2269+
path: Some("/root/.ssh/config"),
2270+
ask_for_approval: AskForApproval::OnFailure,
2271+
sandbox_mode: Some("workspace-write"),
2272+
})
2273+
.unwrap();
2274+
assert_eq!(decision.matched_action, Some(PermissionAction::Deny));
2275+
2276+
// A different absolute path must not match.
2277+
let decision = engine
2278+
.check(ExecPolicyContext {
2279+
command: "",
2280+
cwd: "/workspace",
2281+
tool: Some("read_file"),
2282+
path: Some("/root/.ssh/known_hosts"),
2283+
ask_for_approval: AskForApproval::OnFailure,
2284+
sandbox_mode: Some("workspace-write"),
2285+
})
2286+
.unwrap();
2287+
assert_eq!(decision.matched_rule, None);
2288+
}
2289+
2290+
#[test]
2291+
fn typed_ask_literal_tilde_rule_matches_unexpanded_call_spelling() {
2292+
let engine =
2293+
ExecPolicyEngine::with_rulesets(vec![Ruleset::user(vec![], vec![]).with_ask_rules(
2294+
vec![ToolAskRule {
2295+
tool: "read_file".into(),
2296+
command: None,
2297+
command_exact: false,
2298+
path: Some("~/.ssh/config".into()),
2299+
workspace: None,
2300+
action: PermissionAction::Deny,
2301+
}],
2302+
)]);
2303+
2304+
let decision = engine
2305+
.check(ExecPolicyContext {
2306+
command: "",
2307+
cwd: "/workspace",
2308+
tool: Some("read_file"),
2309+
path: Some("~/.ssh/config"),
2310+
ask_for_approval: AskForApproval::OnFailure,
2311+
sandbox_mode: Some("workspace-write"),
2312+
})
2313+
.unwrap();
2314+
assert_eq!(decision.matched_action, Some(PermissionAction::Deny));
2315+
}
2316+
2317+
#[test]
2318+
fn typed_ask_relative_path_rule_still_rejects_absolute_call() {
2319+
// The absolute fallback is rooted-rule-only: a relative rule keeps
2320+
// its workspace-relative semantics and must not reach an absolute
2321+
// call path through it.
2322+
let engine = ExecPolicyEngine::with_rulesets(vec![
2323+
Ruleset::user(vec![], vec![])
2324+
.with_ask_rules(vec![ToolAskRule::file_path("edit_file", "src/a.rs")]),
2325+
]);
2326+
2327+
let decision = engine
2328+
.check(ExecPolicyContext {
2329+
command: "",
2330+
cwd: "/workspace",
2331+
tool: Some("edit_file"),
2332+
path: Some("/src/a.rs"),
2333+
ask_for_approval: AskForApproval::OnFailure,
2334+
sandbox_mode: Some("workspace-write"),
2335+
})
2336+
.unwrap();
2337+
assert_eq!(decision.matched_rule, None);
2338+
}
2339+
2340+
#[test]
2341+
fn typed_ask_absolute_path_rule_folds_separators_and_case_on_windows() {
2342+
let engine =
2343+
ExecPolicyEngine::with_rulesets(vec![Ruleset::user(vec![], vec![]).with_ask_rules(
2344+
vec![ToolAskRule {
2345+
tool: "read_file".into(),
2346+
command: None,
2347+
command_exact: false,
2348+
path: Some("C:/Users/u/.aws/credentials".into()),
2349+
workspace: None,
2350+
action: PermissionAction::Deny,
2351+
}],
2352+
)]);
2353+
2354+
let decision = engine
2355+
.check(ExecPolicyContext {
2356+
command: "",
2357+
cwd: r"C:\workspace",
2358+
tool: Some("read_file"),
2359+
path: Some(r"C:\Users\U\.AWS\credentials"),
2360+
ask_for_approval: AskForApproval::OnFailure,
2361+
sandbox_mode: Some("workspace-write"),
2362+
})
2363+
.unwrap();
2364+
// The rule folds `C:/Users/u/...` and the call folds `C:\Users\U\...`
2365+
// to the same form on a case-insensitive platform; on a
2366+
// case-sensitive one the case difference is a different file.
2367+
if platform_paths_are_case_insensitive() {
2368+
assert_eq!(decision.matched_action, Some(PermissionAction::Deny));
2369+
} else {
2370+
assert_eq!(decision.matched_rule, None);
2371+
}
2372+
}
2373+
22102374
// ── deny / allow action tests ──────────────────────────────────────────
22112375

22122376
#[test]

0 commit comments

Comments
 (0)