Skip to content

Commit e98ea3e

Browse files
authored
feat(policy): add agentic approval loop (#1528)
1 parent f1fc87e commit e98ea3e

47 files changed

Lines changed: 7198 additions & 2886 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.github/workflows/docker-build.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ jobs:
162162
cargo-version: ${{ inputs['cargo-version'] }}
163163
image-tag: ${{ needs.resolve.outputs.image_tag_base }}
164164
checkout-ref: ${{ inputs['checkout-ref'] }}
165-
features: openshell-core/dev-settings
165+
features: ${{ inputs.component == 'gateway' && 'openshell-core/dev-settings bundled-z3' || 'openshell-core/dev-settings' }}
166166
artifact-name: ${{ needs.resolve.outputs.artifact_prefix }}-linux-${{ matrix.arch }}
167167
secrets: inherit
168168

.github/workflows/e2e-test.yml

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,10 @@ jobs:
8989
chown openshell-test: "/run/user/$(id -u openshell-test)"
9090
chmod 700 "/run/user/$(id -u openshell-test)"
9191
chown -R openshell-test: .
92+
mkdir -p /home/openshell-test/.cache/mise /home/openshell-test/.cargo /home/openshell-test/.local/state/mise
93+
chown -R openshell-test: /home/openshell-test/.cache /home/openshell-test/.cargo /home/openshell-test/.local
94+
install -m 0755 "$(command -v mise)" /usr/local/bin/mise
95+
chmod a+x /root /root/.local /root/.local/bin
9296
for dir in /root/.cargo /root/.rustup /root/.local/share/mise /opt/mise; do
9397
[ -d "$dir" ] && chmod -R a+rX "$dir"
9498
done
@@ -107,9 +111,12 @@ jobs:
107111
runuser -u openshell-test -- env \
108112
XDG_RUNTIME_DIR="/run/user/${TESTUID}" \
109113
HOME="/home/openshell-test" \
110-
PATH="/root/.cargo/bin:/opt/mise/shims:/opt/mise/bin:${PATH}" \
111-
CARGO_HOME="/root/.cargo" \
114+
PATH="/usr/local/bin:/root/.cargo/bin:/opt/mise/shims:/root/.local/bin:${PATH}" \
115+
CARGO_HOME="/home/openshell-test/.cargo" \
112116
RUSTUP_HOME="/root/.rustup" \
117+
MISE_DATA_DIR="/opt/mise" \
118+
MISE_CACHE_DIR="/home/openshell-test/.cache/mise" \
119+
MISE_STATE_DIR="/home/openshell-test/.local/state/mise" \
113120
OPENSHELL_SUPERVISOR_IMAGE="${OPENSHELL_SUPERVISOR_IMAGE}" \
114121
OPENSHELL_REGISTRY="${OPENSHELL_REGISTRY}" \
115122
OPENSHELL_REGISTRY_HOST="${OPENSHELL_REGISTRY_HOST}" \

.github/workflows/release-dev.yml

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -435,7 +435,7 @@ jobs:
435435
run: |
436436
set -euo pipefail
437437
mise x -- rustup target add ${{ matrix.target }}
438-
mise x -- cargo zigbuild --release --target ${{ matrix.zig_target }} -p openshell-server --bin openshell-gateway
438+
mise x -- cargo zigbuild --release --target ${{ matrix.zig_target }} -p openshell-server --bin openshell-gateway --features bundled-z3
439439
mkdir -p artifacts/bin
440440
install -m 0755 target/${{ matrix.target }}/release/openshell-gateway artifacts/bin/openshell-gateway
441441
@@ -445,6 +445,11 @@ jobs:
445445
OUTPUT="$(artifacts/bin/openshell-gateway --version)"
446446
echo "$OUTPUT"
447447
grep -q '^openshell-gateway ' <<<"$OUTPUT"
448+
ldd artifacts/bin/openshell-gateway || true
449+
if ldd artifacts/bin/openshell-gateway | grep -q 'libz3'; then
450+
echo "gateway binary must not depend on shared libz3; build with bundled-z3" >&2
451+
exit 1
452+
fi
448453
449454
- name: Verify glibc symbol floor
450455
run: tasks/scripts/verify-glibc-symbols.sh 2.31 artifacts/bin/openshell-gateway

.github/workflows/release-tag.yml

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -469,7 +469,7 @@ jobs:
469469
run: |
470470
set -euo pipefail
471471
mise x -- rustup target add ${{ matrix.target }}
472-
mise x -- cargo zigbuild --release --target ${{ matrix.zig_target }} -p openshell-server --bin openshell-gateway
472+
mise x -- cargo zigbuild --release --target ${{ matrix.zig_target }} -p openshell-server --bin openshell-gateway --features bundled-z3
473473
mkdir -p artifacts/bin
474474
install -m 0755 target/${{ matrix.target }}/release/openshell-gateway artifacts/bin/openshell-gateway
475475
@@ -479,6 +479,11 @@ jobs:
479479
OUTPUT="$(artifacts/bin/openshell-gateway --version)"
480480
echo "$OUTPUT"
481481
grep -q '^openshell-gateway ' <<<"$OUTPUT"
482+
ldd artifacts/bin/openshell-gateway || true
483+
if ldd artifacts/bin/openshell-gateway | grep -q 'libz3'; then
484+
echo "gateway binary must not depend on shared libz3; build with bundled-z3" >&2
485+
exit 1
486+
fi
482487
483488
- name: Verify glibc symbol floor
484489
run: tasks/scripts/verify-glibc-symbols.sh 2.31 artifacts/bin/openshell-gateway

.github/workflows/rust-native-build.yml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,10 @@ jobs:
245245
# Record linkage so image runtime drift is visible in logs.
246246
ldd --version
247247
ldd "$BIN" || true
248+
if [[ "${{ inputs.component }}" == "gateway" ]] && ldd "$BIN" | grep -q 'libz3'; then
249+
echo "gateway binary must not depend on shared libz3; enable bundled-z3 for image artifacts" >&2
250+
exit 1
251+
fi
248252
249253
- name: Verify glibc symbol floor
250254
if: inputs.component == 'gateway'

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

architecture/security-policy.md

Lines changed: 59 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -89,21 +89,71 @@ because it changes the effective access model for every sandbox on the gateway.
8989
## Policy Advisor
9090

9191
The policy advisor pipeline turns observed denials into draft policy
92-
recommendations:
93-
94-
1. The sandbox aggregates denied network events.
95-
2. A mechanistic mapper proposes minimal endpoint, binary, or rule additions.
96-
3. The gateway validates and stores draft recommendations.
97-
4. A human or admin workflow approves or rejects drafts.
98-
5. Approved drafts merge into the target sandbox policy.
92+
recommendations. There are two proposers (sandbox-side mechanistic mapper,
93+
agent-authored via `policy.local`); the gateway is the single referee.
94+
When enabled, L7 `policy_denied` responses include both structured
95+
`next_steps` and a short `agent_guidance` string so generic agents can continue
96+
through the proposal loop instead of treating the denial as terminal.
97+
98+
1. **Submit.** Both proposers POST through the same `SubmitPolicyAnalysis`
99+
path. Each chunk is persisted with its `analysis_mode` for audit provenance.
100+
2. **Validate.** The gateway runs the prover (`openshell-prover`) on every
101+
chunk regardless of mode. The prover builds a Z3 model from the merged
102+
policy plus the sandbox's attached-provider credential set, then computes
103+
the delta of findings between the current baseline and the merged policy.
104+
3. **Auto-approval gate (proposer-agnostic, opt-in).** Auto-approval fires
105+
when *both* (a) the prover delta is empty (`prover: no new findings`) AND
106+
(b) the `proposal_approval_mode` setting resolves to `"auto"` — gateway
107+
scope wins, sandbox scope is the per-sandbox override, default is
108+
`"manual"`. When both hold, the gateway internally invokes the approve
109+
path with actor identity `system:auto`. The audit event uses
110+
`CONFIG:APPROVED` and carries `auto=true`, `source=<mode>`,
111+
`prover_delta=empty`, and `resolved_from=<gateway|sandbox>` as unmapped
112+
fields, with message text `"auto-approved: no new prover findings"`
113+
never `safe`. The opt-in gate preserves OpenShell's default-deny
114+
posture: with no setting at either scope, every proposal lands in
115+
`pending` for human review, even when the prover sees no findings.
116+
4. **Implicit supersede.** On any successful submission, the gateway scans
117+
the sandbox's pending chunks for matches on `(host, port, binary)` and
118+
auto-rejects the older ones with reason `"superseded by chunk X"`. This
119+
gives the agent a refinement path (broad mechanistic L4 → narrow agent
120+
L7) without an explicit `supersedes_chunk_id` field.
121+
5. **Escalation.** Anything else lands in `pending` for human review.
122+
123+
## What the prover decides
124+
125+
The prover answers four formal questions about each proposed policy
126+
change. Each "yes" answer becomes its own categorical finding — there is
127+
no severity grade. Any finding (of any category) blocks auto-approval.
128+
The categories are intended to be (mostly) mutually exclusive per
129+
underlying change: the gateway suppresses `capability_expansion` paths
130+
whose `(binary, host, port)` is also in the `credential_reach_expansion`
131+
delta, so a brand-new credentialed reach surfaces as one finding rather
132+
than one reach + N method findings.
133+
134+
| Category | The prover detects… |
135+
|---|---|
136+
| `link_local_reach` | The proposal grants reach to a host in `169.254.0.0/16`, `fe80::/10`, or a known metadata hostname such as `metadata.google.internal`. Unconditional — cloud-metadata endpoints serve credentials regardless of sandbox state. |
137+
| `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. |
138+
| `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. |
139+
| `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. |
140+
141+
"Credential in scope" is sandbox-coarse, not binary-fine: a credential is
142+
considered in scope if the sandbox has a provider attached whose
143+
`target_hosts` include the proposed endpoint's host, including runtime-like
144+
first-label wildcard coverage such as `*.github.com` covering
145+
`api.github.com`. v1 does not model credential scopes (read-only vs write);
146+
presence is enough.
99147

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

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

108158
## Security Logging
109159

crates/openshell-cli/src/main.rs

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1148,6 +1148,11 @@ enum DoctorCommands {
11481148
}
11491149

11501150
#[derive(Subcommand, Debug)]
1151+
// `Create` carries enough optional fields to be ~3x larger than the next
1152+
// variant; boxing it would obscure the clap derive ergonomics for one
1153+
// (rare) enum allocation per parse, which isn't worth the readability
1154+
// cost.
1155+
#[allow(clippy::large_enum_variant)]
11511156
enum SandboxCommands {
11521157
/// Create a sandbox.
11531158
#[command(help_template = LEAF_HELP_TEMPLATE, next_help_heading = "FLAGS")]
@@ -1256,6 +1261,18 @@ enum SandboxCommands {
12561261
#[arg(long = "label")]
12571262
labels: Vec<String>,
12581263

1264+
/// Approval mode for agent-authored policy proposals.
1265+
///
1266+
/// `manual` (default): every proposal lands in the draft inbox for
1267+
/// human review, regardless of the prover verdict.
1268+
///
1269+
/// `auto`: proposals whose prover delta is empty are approved
1270+
/// automatically; proposals with findings still require human
1271+
/// approval. Auto mode is an explicit opt-in — `OpenShell`'s
1272+
/// default-deny posture is preserved unless you choose otherwise.
1273+
#[arg(long, value_parser = ["manual", "auto"], default_value = "manual")]
1274+
approval_mode: String,
1275+
12591276
/// Command to run after "--" (defaults to an interactive shell).
12601277
#[arg(last = true, allow_hyphen_values = true)]
12611278
command: Vec<String>,
@@ -2526,6 +2543,7 @@ async fn main() -> Result<()> {
25262543
auto_providers,
25272544
no_auto_providers,
25282545
labels,
2546+
approval_mode,
25292547
command,
25302548
} => {
25312549
// Resolve --tty / --no-tty into an Option<bool> override.
@@ -2594,6 +2612,7 @@ async fn main() -> Result<()> {
25942612
tty_override,
25952613
auto_providers_override,
25962614
&labels_map,
2615+
&approval_mode,
25972616
&tls,
25982617
))
25992618
.await?;
@@ -4134,6 +4153,60 @@ mod tests {
41344153
}
41354154
}
41364155

4156+
/// `sandbox create` defaults `--approval-mode` to `"manual"`. The CLI
4157+
/// always sends an explicit value so the wire form is human-readable
4158+
/// (the gateway treats `""` as `"manual"` too, but the CLI's job is to
4159+
/// be unambiguous).
4160+
#[test]
4161+
fn sandbox_create_approval_mode_defaults_to_manual() {
4162+
let cli = Cli::try_parse_from(["openshell", "sandbox", "create"])
4163+
.expect("sandbox create with no flags should parse");
4164+
match cli.command {
4165+
Some(Commands::Sandbox {
4166+
command: Some(SandboxCommands::Create { approval_mode, .. }),
4167+
..
4168+
}) => {
4169+
assert_eq!(approval_mode, "manual");
4170+
}
4171+
other => panic!("expected SandboxCommands::Create, got: {other:?}"),
4172+
}
4173+
}
4174+
4175+
/// `--approval-mode auto` parses through.
4176+
#[test]
4177+
fn sandbox_create_approval_mode_accepts_auto() {
4178+
let cli =
4179+
Cli::try_parse_from(["openshell", "sandbox", "create", "--approval-mode", "auto"])
4180+
.expect("--approval-mode auto should parse");
4181+
match cli.command {
4182+
Some(Commands::Sandbox {
4183+
command: Some(SandboxCommands::Create { approval_mode, .. }),
4184+
..
4185+
}) => {
4186+
assert_eq!(approval_mode, "auto");
4187+
}
4188+
other => panic!("expected SandboxCommands::Create, got: {other:?}"),
4189+
}
4190+
}
4191+
4192+
/// `--approval-mode <bogus>` is rejected by clap's value parser, so the
4193+
/// CLI can't smuggle through a future-mode value that the gateway
4194+
/// doesn't yet know about.
4195+
#[test]
4196+
fn sandbox_create_approval_mode_rejects_unknown_value() {
4197+
let result = Cli::try_parse_from([
4198+
"openshell",
4199+
"sandbox",
4200+
"create",
4201+
"--approval-mode",
4202+
"auto_on_low_risk",
4203+
]);
4204+
assert!(
4205+
result.is_err(),
4206+
"--approval-mode auto_on_low_risk should be rejected until added to the value parser"
4207+
);
4208+
}
4209+
41374210
#[test]
41384211
fn sandbox_create_resource_flags_parse() {
41394212
let cli = Cli::try_parse_from([

crates/openshell-cli/src/run.rs

Lines changed: 57 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1693,6 +1693,7 @@ pub async fn sandbox_create(
16931693
tty_override: Option<bool>,
16941694
auto_providers_override: Option<bool>,
16951695
labels: &HashMap<String, String>,
1696+
approval_mode: &str,
16961697
tls: &TlsOptions,
16971698
) -> Result<()> {
16981699
if editor.is_some() && !command.is_empty() {
@@ -1806,6 +1807,38 @@ pub async fn sandbox_create(
18061807
let _ = save_last_sandbox(gateway, &sandbox_name);
18071808
}
18081809

1810+
// Persist `--approval-mode` as a sandbox-scoped setting now that the
1811+
// sandbox exists. `manual` is the implicit default (no setting needed);
1812+
// any other value is written so it survives sandbox restarts and can be
1813+
// flipped later via `openshell settings set <name> proposal_approval_mode`.
1814+
// If the write fails the sandbox still runs in default `manual` — surface
1815+
// the recovery command so the user can retry.
1816+
if approval_mode != "manual" {
1817+
let setting = parse_cli_setting_value(settings::PROPOSAL_APPROVAL_MODE_KEY, approval_mode)?;
1818+
match client
1819+
.update_config(UpdateConfigRequest {
1820+
name: sandbox_name.clone(),
1821+
policy: None,
1822+
setting_key: settings::PROPOSAL_APPROVAL_MODE_KEY.to_string(),
1823+
setting_value: Some(setting),
1824+
delete_setting: false,
1825+
global: false,
1826+
merge_operations: vec![],
1827+
expected_resource_version: 0,
1828+
})
1829+
.await
1830+
{
1831+
Ok(_) => {}
1832+
Err(status) => {
1833+
eprintln!(
1834+
"{} failed to set approval mode '{approval_mode}' on sandbox '{sandbox_name}': {}\n retry with: openshell settings set {sandbox_name} proposal_approval_mode {approval_mode}",
1835+
"warning:".yellow().bold(),
1836+
status.message(),
1837+
);
1838+
}
1839+
}
1840+
}
1841+
18091842
// Set up display — interactive terminals get a step-based checklist with
18101843
// spinners; non-interactive (pipes / CI) get timestamped lines.
18111844
let mut display = if interactive {
@@ -5519,7 +5552,23 @@ fn parse_cli_setting_value(key: &str, raw_value: &str) -> Result<SettingValue> {
55195552
})?;
55205553

55215554
let value = match setting.kind {
5522-
SettingValueKind::String => setting_value::Value::StringValue(raw_value.to_string()),
5555+
SettingValueKind::String => {
5556+
// Reject typos client-side so `openshell settings set ...
5557+
// proposal_approval_mode autom` errors immediately instead of
5558+
// round-tripping through the server. The server enforces the
5559+
// same check independently for non-CLI callers.
5560+
setting
5561+
.validate_string_value(raw_value)
5562+
.map_err(|allowed| {
5563+
miette::miette!(
5564+
"invalid value '{}' for key '{}'; expected one of: {}",
5565+
raw_value,
5566+
key,
5567+
allowed.join(", ")
5568+
)
5569+
})?;
5570+
setting_value::Value::StringValue(raw_value.to_string())
5571+
}
55235572
SettingValueKind::Int => {
55245573
let parsed = raw_value.trim().parse::<i64>().map_err(|_| {
55255574
miette::miette!(
@@ -6739,6 +6788,13 @@ pub async fn sandbox_draft_get(
67396788
chunk.security_notes.yellow()
67406789
);
67416790
}
6791+
if !chunk.validation_result.is_empty() {
6792+
println!(
6793+
" {} {}",
6794+
"Validation:".dimmed(),
6795+
chunk.validation_result.cyan()
6796+
);
6797+
}
67426798

67436799
if let Some(ref rule) = chunk.proposed_rule {
67446800
println!(" {} {}", "Endpoints:".dimmed(), format_endpoints(rule));

0 commit comments

Comments
 (0)