Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

61 changes: 53 additions & 8 deletions architecture/security-policy.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,21 +68,66 @@ because it changes the effective access model for every sandbox on the gateway.
## Policy Advisor

The policy advisor pipeline turns observed denials into draft policy
recommendations:
recommendations. There are two proposers (sandbox-side mechanistic mapper,
agent-authored via `policy.local`); the gateway is the single referee.

1. **Submit.** Both proposers POST through the same `SubmitPolicyAnalysis`
path. Each chunk is persisted with its `analysis_mode` for audit provenance.
2. **Validate.** The gateway runs the prover (`openshell-prover`) on every
chunk regardless of mode. The prover builds a Z3 model from the merged
policy plus the sandbox's attached-provider credential set, then computes
the delta of findings between the current baseline and the merged policy.
3. **Auto-approval gate (proposer-agnostic, opt-in).** Auto-approval fires
when *both* (a) the prover delta is empty (`prover: no new findings`) AND
(b) the `proposal_approval_mode` setting resolves to `"auto"` — gateway
scope wins, sandbox scope is the per-sandbox override, default is
`"manual"`. When both hold, the gateway internally invokes the approve
path with actor identity `system:auto`. The audit event uses
`CONFIG:APPROVED` and carries `auto=true`, `source=<mode>`,
`prover_delta=empty`, and `resolved_from=<gateway|sandbox>` as unmapped
fields, with message text `"auto-approved: no new prover findings"` —
never `safe`. The opt-in gate preserves OpenShell's default-deny
posture: with no setting at either scope, every proposal lands in
`pending` for human review, even when the prover sees no findings.
4. **Implicit supersede.** On any successful submission, the gateway scans
the sandbox's pending chunks for matches on `(host, port, binary)` and
auto-rejects the older ones with reason `"superseded by chunk X"`. This
gives the agent a refinement path (broad mechanistic L4 → narrow agent
L7) without an explicit `supersedes_chunk_id` field.
5. **Escalation.** Anything else lands in `pending` for human review.

## What the prover decides

The prover answers four formal questions about each proposed policy
change. Each "yes" answer becomes its own categorical finding — there is
no severity grade. Any finding (of any category) blocks auto-approval.
The categories are intended to be (mostly) mutually exclusive per
underlying change: the gateway suppresses `capability_expansion` paths
whose `(binary, host, port)` is also in the `credential_reach_expansion`
delta, so a brand-new credentialed reach surfaces as one finding rather
than one reach + N method findings.

| Category | The prover detects… |
|---|---|
| `link_local_reach` | The proposal grants reach to a host in `169.254.0.0/16` or `fe80::/10`. Unconditional — cloud-metadata endpoints serve credentials regardless of sandbox state. |
| `l7_bypass_credentialed` | The proposal lets a binary using a non-HTTP wire protocol (`git-remote-https`, `ssh`, `nc`) reach a host where a sandbox credential is in scope. The L7 proxy cannot inspect the wire protocol; the reviewer decides whether to trust the binary with the credential. |
| `credential_reach_expansion` | A binary gained credentialed reach to a (host, port) it could not reach before. New authenticated reach is a stated intent change; the reviewer confirms the binary should authenticate to the host at all. |
| `capability_expansion` | On a (binary, host, port) that already had credentialed reach, the policy adds a new HTTP method. The reviewer sees exactly which method was added (e.g., PUT) and decides if it's part of the agent's task. |

1. The sandbox aggregates denied network events.
2. A mechanistic mapper proposes minimal endpoint, binary, or rule additions.
3. The gateway validates and stores draft recommendations.
4. A human or admin workflow approves or rejects drafts.
5. Approved drafts merge into the target sandbox policy.
"Credential in scope" is sandbox-coarse, not binary-fine: a credential is
considered in scope if the sandbox has a provider attached whose
`target_hosts` include the proposed endpoint's host. v1 does not model
credential scopes (read-only vs write); presence is enough.

Proposals intentionally omit `allowed_ips`. If a proposed rule targets a host
that resolves to a private IP, the proxy's runtime SSRF classification blocks
the connection. The operator must then add an explicit `allowed_ips` entry to
permit it — a two-step flow that keeps SSRF protection on by default.

The advisor should propose narrow additions and preserve explicit-deny behavior.
It is a workflow aid, not an automatic permission grant.
The advisor proposes narrow additions and preserves explicit-deny behavior.
Auto-approval is gated on prover determinism, not human judgment; an LLM-based
contextual reviewer is a deliberate future addition layered on top of the
deterministic prover gate.

## Security Logging

Expand Down
73 changes: 73 additions & 0 deletions crates/openshell-cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1030,6 +1030,11 @@ enum DoctorCommands {
}

#[derive(Subcommand, Debug)]
// `Create` carries enough optional fields to be ~3x larger than the next
// variant; boxing it would obscure the clap derive ergonomics for one
// (rare) enum allocation per parse, which isn't worth the readability
// cost.
#[allow(clippy::large_enum_variant)]
enum SandboxCommands {
/// Create a sandbox.
#[command(help_template = LEAF_HELP_TEMPLATE, next_help_heading = "FLAGS")]
Expand Down Expand Up @@ -1138,6 +1143,18 @@ enum SandboxCommands {
#[arg(long = "label")]
labels: 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>,
Expand Down Expand Up @@ -2383,6 +2400,7 @@ async fn main() -> Result<()> {
auto_providers,
no_auto_providers,
labels,
approval_mode,
command,
} => {
// Resolve --tty / --no-tty into an Option<bool> override.
Expand Down Expand Up @@ -2451,6 +2469,7 @@ async fn main() -> Result<()> {
tty_override,
auto_providers_override,
&labels_map,
&approval_mode,
&tls,
))
.await?;
Expand Down Expand Up @@ -3653,6 +3672,60 @@ 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:?}"),
}
}

/// `--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"
);
}

#[test]
fn sandbox_create_resource_flags_parse() {
let cli = Cli::try_parse_from([
Expand Down
39 changes: 39 additions & 0 deletions crates/openshell-cli/src/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1622,6 +1622,7 @@ pub async fn sandbox_create(
tty_override: Option<bool>,
auto_providers_override: Option<bool>,
labels: &HashMap<String, String>,
approval_mode: &str,
tls: &TlsOptions,
) -> Result<()> {
if editor.is_some() && !command.is_empty() {
Expand Down Expand Up @@ -1730,6 +1731,37 @@ 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![],
})
.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 {
Expand Down Expand Up @@ -6030,6 +6062,13 @@ pub async fn sandbox_draft_get(
chunk.security_notes.yellow()
);
}
if !chunk.validation_result.is_empty() {
println!(
" {} {}",
"Validation:".dimmed(),
chunk.validation_result.cyan()
);
}

if let Some(ref rule) = chunk.proposed_rule {
println!(" {} {}", "Endpoints:".dimmed(), format_endpoints(rule));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -739,6 +739,7 @@ async fn sandbox_create_keeps_command_sessions_by_default() {
Some(false),
Some(false),
&HashMap::new(),
"manual",
&tls,
)
.await
Expand Down Expand Up @@ -780,6 +781,7 @@ async fn sandbox_create_sends_cpu_and_memory_limits_only() {
Some(false),
Some(false),
&HashMap::new(),
"manual",
&tls,
)
.await
Expand Down Expand Up @@ -865,6 +867,7 @@ async fn sandbox_create_returns_vm_error_without_waiting_for_timeout() {
Some(false),
Some(false),
&HashMap::new(),
"manual",
&tls,
)
.await
Expand Down Expand Up @@ -917,6 +920,7 @@ async fn sandbox_create_keeps_waiting_while_vm_progress_arrives() {
Some(false),
Some(false),
&HashMap::new(),
"manual",
&tls,
)
.await
Expand Down Expand Up @@ -961,6 +965,7 @@ async fn sandbox_create_times_out_when_only_logs_arrive() {
Some(false),
Some(false),
&HashMap::new(),
"manual",
&tls,
)
.await
Expand Down Expand Up @@ -1001,6 +1006,7 @@ async fn sandbox_create_deletes_command_sessions_with_no_keep() {
Some(false),
Some(false),
&HashMap::new(),
"manual",
&tls,
)
.await
Expand Down Expand Up @@ -1045,6 +1051,7 @@ async fn sandbox_create_deletes_shell_sessions_with_no_keep() {
Some(true),
Some(false),
&HashMap::new(),
"manual",
&tls,
)
.await
Expand Down Expand Up @@ -1089,6 +1096,7 @@ async fn sandbox_create_keeps_sandbox_with_hidden_keep_flag() {
Some(false),
Some(false),
&HashMap::new(),
"manual",
&tls,
)
.await
Expand Down Expand Up @@ -1133,6 +1141,7 @@ async fn sandbox_create_keeps_sandbox_with_forwarding() {
Some(false),
Some(false),
&HashMap::new(),
"manual",
&tls,
)
.await
Expand Down
21 changes: 21 additions & 0 deletions crates/openshell-core/src/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,21 @@ 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";

pub const REGISTERED_SETTINGS: &[RegisteredSetting] = &[
// Gateway-level opt-in for provider profile policy composition. Defaults
// to false when unset.
Expand All @@ -79,6 +94,12 @@ pub const REGISTERED_SETTINGS: &[RegisteredSetting] = &[
key: AGENT_POLICY_PROPOSALS_ENABLED_KEY,
kind: SettingValueKind::Bool,
},
// 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,
},
// Test-only keys live behind the `dev-settings` feature flag so they
// don't appear in production builds.
#[cfg(feature = "dev-settings")]
Expand Down
Loading
Loading