trending/OpenShell
1
0
Fork 0
mirror of https://github.com/NVIDIA/OpenShell.git synced 2026-10-24 06:34:33 +08:00
Code Issues Packages Projects Releases Wiki Activity

refactor(policy): unify proposal admission under managed maximum

Browse Source 
Signed-off-by: Alexander Watson <zredlined@users.noreply.github.com>
This commit is contained in:
Alexander Watson
2026-07-08 14:10:44 -07:00
parent 0cd2922321
commit 8ae313fb8f
9 changed files with 345 additions and 1350 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/openshell-cli/src/main.rs
+3 -113
View File
@@ -1331,18 +1331,6 @@ enum SandboxCommands {
#[arg(long = "env", value_name = "KEY=VALUE")]
envs: Vec<String>,
/// Approval mode for agent-authored policy proposals.
///
/// `manual` (default): every proposal lands in the draft inbox for
/// human review, regardless of the prover verdict.
///
/// `auto`: proposals whose prover delta is empty are approved
/// automatically; proposals with findings still require human
/// approval. Auto mode is an explicit opt-in — `OpenShell`'s
/// default-deny posture is preserved unless you choose otherwise.
#[arg(long, value_parser = ["manual", "auto"], default_value = "manual")]
approval_mode: String,
/// Command to run after "--" (defaults to an interactive shell).
#[arg(last = true, allow_hyphen_values = true)]
command: Vec<String>,
@@ -1748,30 +1736,6 @@ enum PolicyCommands {
#[arg(long)]
yes: bool,
},
/// Prove properties of a sandbox policy — or find counterexamples.
#[command(help_template = LEAF_HELP_TEMPLATE, next_help_heading = "FLAGS")]
Prove {
/// Path to `OpenShell` sandbox policy YAML.
#[arg(long, value_hint = ValueHint::FilePath)]
policy: String,
/// Path to credential descriptor YAML.
#[arg(long, value_hint = ValueHint::FilePath)]
credentials: String,
/// Path to capability registry directory (default: bundled).
#[arg(long, value_hint = ValueHint::DirPath)]
registry: Option<String>,
/// Path to accepted risks YAML.
#[arg(long, value_hint = ValueHint::FilePath)]
accepted_risks: Option<String>,
/// One-line-per-finding output (for demos and CI).
#[arg(long)]
compact: bool,
},
}
#[derive(Subcommand, Debug)]
@@ -2330,28 +2294,6 @@ async fn main() -> Result<()> {
// -----------------------------------------------------------
// Top-level policy (was `sandbox policy`)
// -----------------------------------------------------------
Some(Commands::Policy {
command:
Some(PolicyCommands::Prove {
policy,
credentials,
registry,
accepted_risks,
compact,
}),
}) => {
// Prove runs locally — no gateway needed.
let exit_code = openshell_prover::prove(
&policy,
&credentials,
registry.as_deref(),
accepted_risks.as_deref(),
compact,
)?;
if exit_code != 0 {
std::process::exit(exit_code);
}
}
Some(Commands::Policy {
command: Some(policy_cmd),
}) => {
@@ -2482,7 +2424,6 @@ async fn main() -> Result<()> {
}
run::gateway_setting_delete(&ctx.endpoint, "policy", yes, &tls).await?;
}
PolicyCommands::Prove { .. } => unreachable!(),
}
}
@@ -2671,7 +2612,6 @@ async fn main() -> Result<()> {
no_auto_providers,
labels,
envs,
approval_mode,
command,
} => {
// Resolve --tty / --no-tty into an Option<bool> override.
@@ -2758,7 +2698,6 @@ async fn main() -> Result<()> {
auto_providers_override,
labels: labels_map,
environment: env_map,
approval_mode: &approval_mode,
},
&tls,
))
@@ -4559,42 +4498,6 @@ mod tests {
}
}
/// `sandbox create` defaults `--approval-mode` to `"manual"`. The CLI
/// always sends an explicit value so the wire form is human-readable
/// (the gateway treats `""` as `"manual"` too, but the CLI's job is to
/// be unambiguous).
#[test]
fn sandbox_create_approval_mode_defaults_to_manual() {
let cli = Cli::try_parse_from(["openshell", "sandbox", "create"])
.expect("sandbox create with no flags should parse");
match cli.command {
Some(Commands::Sandbox {
command: Some(SandboxCommands::Create { approval_mode, .. }),
..
}) => {
assert_eq!(approval_mode, "manual");
}
other => panic!("expected SandboxCommands::Create, got: {other:?}"),
}
}
/// `--approval-mode auto` parses through.
#[test]
fn sandbox_create_approval_mode_accepts_auto() {
let cli =
Cli::try_parse_from(["openshell", "sandbox", "create", "--approval-mode", "auto"])
.expect("--approval-mode auto should parse");
match cli.command {
Some(Commands::Sandbox {
command: Some(SandboxCommands::Create { approval_mode, .. }),
..
}) => {
assert_eq!(approval_mode, "auto");
}
other => panic!("expected SandboxCommands::Create, got: {other:?}"),
}
}
#[test]
fn sandbox_create_permission_mode_is_optional_and_accepts_managed_modes() {
let default = Cli::try_parse_from(["openshell", "sandbox", "create"])
@@ -4634,23 +4537,10 @@ mod tests {
])
.is_err()
);
}
/// `--approval-mode <bogus>` is rejected by clap's value parser, so the
/// CLI can't smuggle through a future-mode value that the gateway
/// doesn't yet know about.
#[test]
fn sandbox_create_approval_mode_rejects_unknown_value() {
let result = Cli::try_parse_from([
"openshell",
"sandbox",
"create",
"--approval-mode",
"auto_on_low_risk",
]);
assert!(
result.is_err(),
"--approval-mode auto_on_low_risk should be rejected until added to the value parser"
Cli::try_parse_from(["openshell", "sandbox", "create", "--approval-mode", "auto",])
.is_err(),
"the removed unmanaged approval mode must not remain in the CLI"
);
}
crates/openshell-cli/src/run.rs
+1 -65
View File
@@ -1777,7 +1777,7 @@ async fn finalize_sandbox_create_session(
/// Infrastructure parameters (`server`, `gateway_name`, `tls`) remain positional
/// on the function signature, following the `provider_refresh_config(server, input, tls)`
/// precedent. This struct captures sandbox-specific options.
#[derive(Debug)]
#[derive(Debug, Default)]
pub struct SandboxCreateConfig<'a> {
pub name: Option<&'a str>,
pub from: Option<&'a str>,
@@ -1797,33 +1797,6 @@ pub struct SandboxCreateConfig<'a> {
pub auto_providers_override: Option<bool>,
pub labels: HashMap<String, String>,
pub environment: HashMap<String, String>,
pub approval_mode: &'a str,
}
impl Default for SandboxCreateConfig<'_> {
fn default() -> Self {
Self {
name: None,
from: None,
uploads: &[],
keep: false,
gpu_requirements: None,
cpu: None,
memory: None,
driver_config_json: None,
editor: None,
providers: &[],
policy: None,
permission_mode: None,
forward: None,
command: &[],
tty_override: None,
auto_providers_override: None,
labels: HashMap::new(),
environment: HashMap::new(),
approval_mode: "manual",
}
}
}
/// Create a sandbox with default settings.
@@ -1852,7 +1825,6 @@ pub async fn sandbox_create(
auto_providers_override,
labels,
environment,
approval_mode,
} = config;
if editor.is_some() && !command.is_empty() {
@@ -1971,38 +1943,6 @@ pub async fn sandbox_create(
let _ = save_last_sandbox(gateway, &sandbox_name);
}
// Persist `--approval-mode` as a sandbox-scoped setting now that the
// sandbox exists. `manual` is the implicit default (no setting needed);
// any other value is written so it survives sandbox restarts and can be
// flipped later via `openshell settings set <name> proposal_approval_mode`.
// If the write fails the sandbox still runs in default `manual` — surface
// the recovery command so the user can retry.
if approval_mode != "manual" {
let setting = parse_cli_setting_value(settings::PROPOSAL_APPROVAL_MODE_KEY, approval_mode)?;
match client
.update_config(UpdateConfigRequest {
name: sandbox_name.clone(),
policy: None,
setting_key: settings::PROPOSAL_APPROVAL_MODE_KEY.to_string(),
setting_value: Some(setting),
delete_setting: false,
global: false,
merge_operations: vec![],
expected_resource_version: 0,
})
.await
{
Ok(_) => {}
Err(status) => {
eprintln!(
"{} failed to set approval mode '{approval_mode}' on sandbox '{sandbox_name}': {}\n retry with: openshell settings set {sandbox_name} proposal_approval_mode {approval_mode}",
"warning:".yellow().bold(),
status.message(),
);
}
}
}
// Set up display — interactive terminals get a step-based checklist with
// spinners; non-interactive (pipes / CI) get timestamped lines.
let mut display = if interactive {
@@ -6231,10 +6171,6 @@ fn parse_cli_setting_value(key: &str, raw_value: &str) -> Result<SettingValue> {
let value = match setting.kind {
SettingValueKind::String => {
// Reject typos client-side so `openshell settings set ...
// proposal_approval_mode autom` errors immediately instead of
// round-tripping through the server. The server enforces the
// same check independently for non-CLI callers.
setting
.validate_string_value(raw_value)
.map_err(|allowed| {
crates/openshell-core/src/settings.rs
+2 -66
View File
@@ -85,28 +85,6 @@ pub const PROVIDERS_V2_ENABLED_KEY: &str = "providers_v2_enabled";
/// still applies when this flag is on.
pub const AGENT_POLICY_PROPOSALS_ENABLED_KEY: &str = "agent_policy_proposals_enabled";
/// Approval mode for agent-authored policy proposals.
///
/// `"manual"` (the default when unset): every proposal lands in the draft
/// inbox for human review, regardless of the prover verdict. `"auto"`:
/// proposals whose prover delta is empty are approved automatically;
/// proposals with findings still require human approval. Any other value
/// (typos, future-reserved modes like `"auto_on_low_risk"`) falls back to
/// manual — auto mode is an explicit, exact opt-in.
///
/// Resolution precedence (matches the rest of the settings model): gateway
/// scope wins over sandbox scope. A reviewer can pin manual mode for a
/// fleet by setting it globally; per-sandbox overrides only apply when no
/// global is set.
pub const PROPOSAL_APPROVAL_MODE_KEY: &str = "proposal_approval_mode";
/// Allowed values for [`PROPOSAL_APPROVAL_MODE_KEY`].
///
/// Any other string is rejected at configure time (so operators get immediate
/// feedback on typos like `"autom"`) while the runtime resolver still
/// fail-closes on unknown persisted values for defense in depth.
pub const PROPOSAL_APPROVAL_MODE_VALUES: &[&str] = &["manual", "auto"];
pub const REGISTERED_SETTINGS: &[RegisteredSetting] = &[
// Gateway-level opt-in for provider profile policy composition. Defaults
// to false when unset.
@@ -130,13 +108,6 @@ pub const REGISTERED_SETTINGS: &[RegisteredSetting] = &[
kind: SettingValueKind::Bool,
allowed_string_values: None,
},
// Approval mode for agent-authored proposals. See
// PROPOSAL_APPROVAL_MODE_KEY for details. Defaults to manual.
RegisteredSetting {
key: PROPOSAL_APPROVAL_MODE_KEY,
kind: SettingValueKind::String,
allowed_string_values: Some(PROPOSAL_APPROVAL_MODE_VALUES),
},
];
/// Resolve a setting descriptor from the registry by key.
@@ -168,9 +139,8 @@ pub fn parse_bool_like(raw: &str) -> Option<bool> {
#[cfg(test)]
mod tests {
use super::{
PROPOSAL_APPROVAL_MODE_KEY, PROPOSAL_APPROVAL_MODE_VALUES, PROVIDERS_V2_ENABLED_KEY,
REGISTERED_SETTINGS, RegisteredSetting, SettingValueKind, parse_bool_like,
registered_keys_csv, setting_for_key,
PROVIDERS_V2_ENABLED_KEY, REGISTERED_SETTINGS, RegisteredSetting, SettingValueKind,
parse_bool_like, registered_keys_csv, setting_for_key,
};
#[test]
@@ -203,40 +173,6 @@ mod tests {
assert!(setting.validate_string_value("").is_ok());
}
#[test]
fn proposal_approval_mode_accepts_manual_and_auto() {
let setting = setting_for_key(PROPOSAL_APPROVAL_MODE_KEY)
.expect("proposal_approval_mode should be registered");
assert_eq!(setting.kind, SettingValueKind::String);
assert_eq!(
setting.allowed_string_values,
Some(PROPOSAL_APPROVAL_MODE_VALUES)
);
assert!(setting.validate_string_value("manual").is_ok());
assert!(setting.validate_string_value("auto").is_ok());
}
#[test]
fn proposal_approval_mode_rejects_typos_and_future_modes() {
let setting = setting_for_key(PROPOSAL_APPROVAL_MODE_KEY)
.expect("proposal_approval_mode should be registered");
for bad in [
"autom",
"AUTO",
"Manual",
"",
" auto",
"auto_on_low_risk",
"yes",
] {
let err = setting
.validate_string_value(bad)
.expect_err(&format!("expected '{bad}' to be rejected"));
// Caller gets the allowed slice back for diagnostics.
assert_eq!(err, PROPOSAL_APPROVAL_MODE_VALUES);
}
}
// ---- parse_bool_like ----
#[test]
crates/openshell-ocsf/src/format/shorthand.rs
+12 -13
View File
@@ -316,10 +316,10 @@ impl OcsfEvent {
);
let what = e.base.message.as_deref().unwrap_or("config");
// Bracketed suffix carries the structured provenance fields a
// reviewer needs to scan a CONFIG audit line. Auto-approval
// emits `auto`/`source`/`prover_delta`; every config change
// also carries `policy_version` and `policy_hash`. Order is
// stable so logs are greppable.
// reviewer needs to scan a CONFIG audit line. Managed
// automatic application emits `auto`/`source`/`approval_basis`;
// every config change also carries `policy_version` and
// `policy_hash`. Order is stable so logs are greppable.
let suffix = e
.base
.unmapped
@@ -333,6 +333,7 @@ impl OcsfEvent {
};
push("auto");
push("source");
push("approval_basis");
push("prover_delta");
push("resolved_from");
if let Some(ver) = u.get("policy_version").and_then(|v| v.as_str()) {
@@ -916,17 +917,15 @@ mod tests {
);
}
/// Auto-approval audit events carry `auto`, `source`, `prover_delta`, and
/// `resolved_from` as unmapped fields. Lock the suffix order so operators
/// (and the demo's grep) can rely on it.
/// Managed automatic application audit events carry the approval basis as
/// an unmapped field. Lock the suffix order so operators can rely on it.
#[test]
fn test_config_state_change_shorthand_includes_auto_approve_fields() {
fn test_config_state_change_shorthand_includes_managed_apply_fields() {
let mut b = base(5019, "Device Config State Change", 5, "Discovery", 1, "Log");
b.set_message("auto-approved: no new prover findings (source=agent_authored)");
b.set_message("managed maximum auto-applied proposal (source=agent_authored)");
b.add_unmapped("auto", serde_json::json!("true"));
b.add_unmapped("source", serde_json::json!("agent_authored"));
b.add_unmapped("prover_delta", serde_json::json!("empty"));
b.add_unmapped("resolved_from", serde_json::json!("sandbox"));
b.add_unmapped("approval_basis", serde_json::json!("managed_maximum"));
b.add_unmapped("policy_version", serde_json::json!("v4"));
b.add_unmapped("policy_hash", serde_json::json!("sha256:cafe"));
@@ -941,8 +940,8 @@ mod tests {
let shorthand = event.format_shorthand();
assert_eq!(
shorthand,
"CONFIG:APPROVED [INFO] auto-approved: no new prover findings (source=agent_authored) \
[auto:true source:agent_authored prover_delta:empty resolved_from:sandbox \
"CONFIG:APPROVED [INFO] managed maximum auto-applied proposal (source=agent_authored) \
[auto:true source:agent_authored approval_basis:managed_maximum \
version:v4 hash:sha256:cafe]"
);
}
crates/openshell-server/src/grpc/policy.rs
+313 -1048
View File
File diff suppressed because it is too large Load Diff
crates/openshell-server/src/grpc/provider.rs
+1 -1
View File
@@ -760,7 +760,7 @@ fn count_as_u32(count: usize) -> u32 {
u32::try_from(count).unwrap_or(u32::MAX)
}
fn host_patterns_can_overlap(first: &str, second: &str) -> bool {
pub(super) fn host_patterns_can_overlap(first: &str, second: &str) -> bool {
let first = first.to_ascii_lowercase();
let second = second.to_ascii_lowercase();
if !first.contains('*') {
crates/openshell-supervisor-network/src/policy_local.rs
+1 -1
View File
@@ -782,7 +782,7 @@ async fn fetch_chunk_or_404(
/// next on the redraft loop — identity (`chunk_id`, `status`), the proposal
/// it submitted (`rule_name`, `binary`), the two feedback signals
/// (`rejection_reason` from the reviewer, `validation_result` from the
/// gateway prover), and (on /wait) `policy_reloaded` so the agent can tell
/// gateway admission or advisory context), and (on /wait) `policy_reloaded` so the agent can tell
/// "approved AND the new rule is loaded — safe to retry" from "approved
/// but the supervisor hasn't reloaded yet — re-issue /wait or surface to
/// user". Display-only proto fields (`hit_count`, `confidence`, `stage`,
crates/openshell-tui/src/ui/sandbox_draft.rs
+6 -36
View File
@@ -479,59 +479,29 @@ fn approval_annotation(chunk: &PolicyChunk) -> Option<ApprovalAnnotation> {
return None;
}
if validation == "prover: no new findings" {
if chunk.status == "approved" {
return Some(ApprovalAnnotation {
kind: ApprovalAnnotationKind::AutoApproved,
short_label: "auto-approved".to_string(),
detail_label: "proposal was auto-approved; no additional risk detected".to_string(),
});
}
if validation.starts_with("managed: apply:") && chunk.status == "approved" {
return Some(ApprovalAnnotation {
kind: ApprovalAnnotationKind::RequiresReview,
short_label: "review required".to_string(),
detail_label: "rule requires review; no additional risk detected".to_string(),
kind: ApprovalAnnotationKind::AutoApproved,
short_label: "auto-applied".to_string(),
detail_label: validation.to_string(),
});
}
let issues = validation_issue_summary(validation);
if chunk.status == "approved" {
return Some(ApprovalAnnotation {
kind: ApprovalAnnotationKind::Reviewed,
short_label: "reviewed".to_string(),
detail_label: format!("rule was approved after review; possible issues: {issues}"),
detail_label: validation.to_string(),
});
}
Some(ApprovalAnnotation {
kind: ApprovalAnnotationKind::RequiresReview,
short_label: "review required".to_string(),
detail_label: format!(
"rule was not auto-approved and requires review; possible issues: {issues}"
),
detail_label: validation.to_string(),
})
}
fn validation_issue_summary(validation: &str) -> String {
let mut issues = Vec::new();
for line in validation.lines().skip(1) {
let Some((category, _)) = line.trim().split_once(':') else {
continue;
};
let label = category.trim().replace('_', " ");
if !label.is_empty() && !issues.contains(&label) {
issues.push(label);
}
}
if issues.is_empty() {
validation.lines().next().unwrap_or(validation).to_string()
} else {
issues.join(", ")
}
}
fn format_endpoint_summary(endpoint: &NetworkEndpoint) -> String {
let host_port = if endpoint.port > 0 {
format!("{}:{}", endpoint.host, endpoint.port)
proto/openshell.proto
+6 -7
View File
@@ -338,10 +338,9 @@ message SandboxSpec {
ResourceRequirements resource_requirements = 9;
reserved 10;
reserved "gpu_device";
// Field 11 was `proposal_approval_mode`. The approval mode is now a
// runtime setting (gateway or sandbox scope) read via UpdateConfig /
// GetSandboxConfig, so it can be flipped on a running sandbox and
// managed fleet-wide.
// Field 11 was the legacy unmanaged proposal approval mode. Automatic
// proposal application now requires a gateway-managed maximum policy and
// the sandbox's immutable managed permission mode.
reserved 11;
reserved "proposal_approval_mode";
}
@@ -1896,8 +1895,8 @@ message DraftChunkPayload {
string binary = 9;
// Current draft version for the owning sandbox.
int64 draft_version = 10;
// Gateway prover verdict for this chunk; empty until prover runs.
// Mirrors PolicyChunk.validation_result.
// Managed-admission context or a non-gating gateway advisory. Empty when
// neither applies. Mirrors PolicyChunk.validation_result.
string validation_result = 11;
// Operator-supplied free-form rejection text; empty for non-rejected
// chunks. Mirrors PolicyChunk.rejection_reason.
@@ -1936,7 +1935,7 @@ message StoredDraftChunk {
int32 hit_count = 15;
int64 first_seen_ms = 16;
int64 last_seen_ms = 17;
// Gateway prover verdict; empty until the prover runs. See PolicyChunk.
// Managed-admission context or a non-gating gateway advisory. See PolicyChunk.
string validation_result = 18;
// Operator-supplied free-form rejection text. See PolicyChunk.
string rejection_reason = 19;