feat(security): deny loosest approval/sandbox values at project scope (#417)
Continues #417 by closing the value-level escalation case for the two pure-loosening values: * `approval_policy = "auto"` would auto-approve every tool call that the user's stricter setting (\`suggest\`, \`never\`, etc.) was prompting on. Pure escalation; project should never be able to set this. * `sandbox_mode = "danger-full-access"` exits the workspace sandbox entirely. Pure escalation; project should never be able to set this. Both denies are unconditional at project scope — the user's prior value (or absence) doesn't matter. The denied value emits a stderr warning so users see the deny. Sub-tightening comparisons (e.g. user `"never"` → project `"on-request"` is allowed even though it loosens) stay v0.8.9 follow-up because they need a richer ordering check across all `approval_policy` / `sandbox_mode` values. Tests: * `project_overlay_denies_approval_auto_and_sandbox_danger_values` exercises both escalation values in the same merge and confirms a non-escalation field on the same project file still applies. * `project_overlay_preserves_user_strict_value_when_project_tries_to_loosen` exercises the belt-and-suspenders case: user has `approval_policy = "never"`, project tries `"auto"`, the user's strict value survives.
This commit is contained in:
+8
-3
@@ -55,9 +55,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
config that spawns arbitrary stdio servers under the user's
|
||||
identity. The denied key emits a stderr warning so a user who
|
||||
expected the override sees the deny instead of a silent drop.
|
||||
Less-dangerous escalation prevention (e.g. denying
|
||||
`approval_policy = "auto"` over a stricter user value) stays
|
||||
v0.8.9 follow-up because it needs richer value comparison.
|
||||
- **Project-scope value-deny for the loosest postures** (#417
|
||||
follow-up) — `approval_policy = "auto"` and
|
||||
`sandbox_mode = "danger-full-access"` are pure escalation
|
||||
values, denied unconditionally at project scope regardless
|
||||
of the user's prior value. Sub-tightening comparisons
|
||||
(e.g. user `"never"` → project `"on-request"` is allowed
|
||||
even though it loosens) stay v0.8.9 follow-up because they
|
||||
need a richer ordering check.
|
||||
- **Sub-agent role taxonomy expansion** (#404) — adds `Implementer`
|
||||
("land this change with the minimum surrounding edit") and
|
||||
`Verifier` ("run the test suite, report pass/fail with evidence")
|
||||
|
||||
+76
-5
@@ -3298,11 +3298,12 @@ fn merge_project_config(config: &mut Config, workspace: &Path) {
|
||||
|
||||
// String fields a project may legitimately override (model,
|
||||
// approval/sandbox tightening, notes path, reasoning effort).
|
||||
// Note: `approval_policy` / `sandbox_mode` are allowed but we
|
||||
// intentionally don't reject loosening here — that's a richer
|
||||
// value-comparison check filed for v0.8.9 follow-up. The `never`
|
||||
// / `read-only` posture is the common project-config pattern
|
||||
// and tightening always wins.
|
||||
// Loosening *values* like `approval_policy = "auto"` and
|
||||
// `sandbox_mode = "danger-full-access"` are denied unconditionally
|
||||
// — those are pure escalation regardless of the user's prior
|
||||
// value. Sub-tightening comparisons (e.g. user `"never"` →
|
||||
// project `"on-request"`) stay v0.8.9 follow-up because they
|
||||
// need a richer ordering check.
|
||||
for (key, field) in [
|
||||
("model", &mut config.default_text_model),
|
||||
("reasoning_effort", &mut config.reasoning_effort),
|
||||
@@ -3313,6 +3314,21 @@ fn merge_project_config(config: &mut Config, workspace: &Path) {
|
||||
if let Some(v) = table.get(key).and_then(toml::Value::as_str)
|
||||
&& !v.is_empty()
|
||||
{
|
||||
// #417 escalation deny: project cannot push the session
|
||||
// to the loosest values. Other strings flow through the
|
||||
// existing config validator on load.
|
||||
let is_escalation = matches!(
|
||||
(key, v),
|
||||
("approval_policy", "auto") | ("sandbox_mode", "danger-full-access")
|
||||
);
|
||||
if is_escalation {
|
||||
eprintln!(
|
||||
"warning: project-scope `{key} = \"{v}\"` is ignored — \
|
||||
project config cannot escalate to the loosest value. \
|
||||
(See #417.)"
|
||||
);
|
||||
continue;
|
||||
}
|
||||
*field = Some(v.to_string());
|
||||
}
|
||||
}
|
||||
@@ -3922,6 +3938,61 @@ sandbox_mode = "read-only"
|
||||
assert_eq!(config.sandbox_mode.as_deref(), Some("read-only"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn project_overlay_denies_approval_auto_and_sandbox_danger_values() {
|
||||
// #417 value-deny: the loosest values (`approval_policy = "auto"`,
|
||||
// `sandbox_mode = "danger-full-access"`) are pure escalation.
|
||||
// Even when the user hasn't set these fields, the project
|
||||
// can't push the session to the loosest posture.
|
||||
let tmp = workspace_with_project_config(
|
||||
r#"
|
||||
approval_policy = "auto"
|
||||
sandbox_mode = "danger-full-access"
|
||||
model = "deepseek-v4-pro"
|
||||
"#,
|
||||
);
|
||||
let mut config = Config::default();
|
||||
merge_project_config(&mut config, tmp.path());
|
||||
assert_eq!(
|
||||
config.approval_policy, None,
|
||||
"project-scope `approval_policy = \"auto\"` must be denied"
|
||||
);
|
||||
assert_eq!(
|
||||
config.sandbox_mode, None,
|
||||
"project-scope `sandbox_mode = \"danger-full-access\"` must be denied"
|
||||
);
|
||||
// Non-escalation overrides on the same merge succeed —
|
||||
// the deny is per-key, not per-file.
|
||||
assert_eq!(
|
||||
config.default_text_model.as_deref(),
|
||||
Some("deepseek-v4-pro"),
|
||||
"non-escalation overrides should still apply"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn project_overlay_preserves_user_strict_value_when_project_tries_to_loosen() {
|
||||
// Belt-and-suspenders: if the user has `approval_policy = "never"`
|
||||
// and the project tries `approval_policy = "auto"`, the deny
|
||||
// keeps the user's strict value rather than falling through to
|
||||
// None.
|
||||
let tmp = workspace_with_project_config(
|
||||
r#"
|
||||
approval_policy = "auto"
|
||||
"#,
|
||||
);
|
||||
let mut config = Config {
|
||||
approval_policy: Some("never".to_string()),
|
||||
..Config::default()
|
||||
};
|
||||
merge_project_config(&mut config, tmp.path());
|
||||
assert_eq!(
|
||||
config.approval_policy.as_deref(),
|
||||
Some("never"),
|
||||
"user's strict approval_policy must survive a project escalation attempt"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn project_overlay_overrides_max_subagents_and_allow_shell() {
|
||||
let tmp = workspace_with_project_config(
|
||||
|
||||
Reference in New Issue
Block a user