diff --git a/Cargo.lock b/Cargo.lock index 29919124a..f3323c049 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3584,6 +3584,7 @@ dependencies = [ "openshell-core", "openshell-ocsf", "openshell-policy", + "openshell-providers", "openshell-router", "rand_core 0.6.4", "rcgen", diff --git a/crates/openshell-core/src/settings.rs b/crates/openshell-core/src/settings.rs index 897317a5a..b0a437fbe 100644 --- a/crates/openshell-core/src/settings.rs +++ b/crates/openshell-core/src/settings.rs @@ -60,8 +60,9 @@ pub const PROVIDERS_V2_ENABLED_KEY: &str = "providers_v2_enabled"; pub const AGENT_POLICY_PROPOSALS_ENABLED_KEY: &str = "agent_policy_proposals_enabled"; pub const REGISTERED_SETTINGS: &[RegisteredSetting] = &[ - // Gateway-level opt-in for provider profile policy composition. Defaults - // to false when unset. + // Gateway-level control for provider profile policy composition. Defaults + // to true when unset; set false to attach provider credentials without + // adding the provider profile's network policy layer. RegisteredSetting { key: PROVIDERS_V2_ENABLED_KEY, kind: SettingValueKind::Bool, diff --git a/crates/openshell-providers/src/profiles.rs b/crates/openshell-providers/src/profiles.rs index 588e77702..c03391086 100644 --- a/crates/openshell-providers/src/profiles.rs +++ b/crates/openshell-providers/src/profiles.rs @@ -880,7 +880,19 @@ mod tests { ProviderProfileCategory::SourceControl as i32 ); assert_eq!(proto.endpoints.len(), 2); - assert_eq!(proto.binaries.len(), 4); + assert_eq!(proto.binaries.len(), 8); + assert!( + proto + .binaries + .iter() + .any(|binary| binary.path == "/usr/bin/curl") + ); + assert!( + proto + .binaries + .iter() + .any(|binary| binary.path == "/usr/local/bin/node") + ); } #[test] diff --git a/crates/openshell-sandbox/Cargo.toml b/crates/openshell-sandbox/Cargo.toml index 29919ede4..517400ac4 100644 --- a/crates/openshell-sandbox/Cargo.toml +++ b/crates/openshell-sandbox/Cargo.toml @@ -18,6 +18,7 @@ path = "src/main.rs" openshell-core = { path = "../openshell-core" } openshell-ocsf = { path = "../openshell-ocsf" } openshell-policy = { path = "../openshell-policy" } +openshell-providers = { path = "../openshell-providers" } openshell-router = { path = "../openshell-router" } # Async runtime diff --git a/crates/openshell-sandbox/src/grpc_client.rs b/crates/openshell-sandbox/src/grpc_client.rs index 19a6831e5..35b7051a9 100644 --- a/crates/openshell-sandbox/src/grpc_client.rs +++ b/crates/openshell-sandbox/src/grpc_client.rs @@ -10,10 +10,11 @@ use std::time::Duration; use miette::{IntoDiagnostic, Result, WrapErr}; use openshell_core::proto::{ DenialSummary, GetDraftPolicyRequest, GetInferenceBundleRequest, GetInferenceBundleResponse, - GetSandboxConfigRequest, GetSandboxProviderEnvironmentRequest, PolicyChunk, PolicySource, - PolicyStatus, ReportPolicyStatusRequest, SandboxPolicy as ProtoSandboxPolicy, - SubmitPolicyAnalysisRequest, SubmitPolicyAnalysisResponse, UpdateConfigRequest, - inference_client::InferenceClient, open_shell_client::OpenShellClient, + GetSandboxConfigRequest, GetSandboxProviderEnvironmentRequest, ListSandboxProvidersRequest, + PolicyChunk, PolicySource, PolicyStatus, ReportPolicyStatusRequest, + SandboxPolicy as ProtoSandboxPolicy, SubmitPolicyAnalysisRequest, SubmitPolicyAnalysisResponse, + UpdateConfigRequest, datamodel::v1::Provider, inference_client::InferenceClient, + open_shell_client::OpenShellClient, }; use tonic::service::interceptor::InterceptedService; use tonic::transport::{Certificate, Channel, ClientTlsConfig, Endpoint, Identity}; @@ -379,6 +380,20 @@ impl CachedOpenShellClient { Ok(response.into_inner().chunks) } + /// List providers attached to this sandbox. Used by policy.local so + /// in-sandbox agents can observe attachment state without seeing secrets. + pub async fn list_sandbox_providers(&self, sandbox_name: &str) -> Result> { + let response = self + .client + .clone() + .list_sandbox_providers(ListSandboxProvidersRequest { + sandbox_name: sandbox_name.to_string(), + }) + .await + .into_diagnostic()?; + Ok(response.into_inner().providers) + } + /// Report policy load status back to the server. pub async fn report_policy_status( &self, diff --git a/crates/openshell-sandbox/src/l7/rest.rs b/crates/openshell-sandbox/src/l7/rest.rs index c513499f4..0a8bc662e 100644 --- a/crates/openshell-sandbox/src/l7/rest.rs +++ b/crates/openshell-sandbox/src/l7/rest.rs @@ -2332,7 +2332,13 @@ mod tests { body["next_steps"][0]["path"], "/etc/openshell/skills/policy_advisor.md" ); - assert_eq!(body["next_steps"][3]["body_type"], "PolicyMergeOperation"); + let submit_step = body["next_steps"] + .as_array() + .expect("next_steps should be an array") + .iter() + .find(|step| step["action"] == "submit_proposal") + .expect("next_steps should include submit_proposal"); + assert_eq!(submit_step["body_type"], "PolicyMergeOperation"); assert!( !body.to_string().contains("secret-token"), "deny body must not leak query params or credential values" diff --git a/crates/openshell-sandbox/src/mechanistic_mapper.rs b/crates/openshell-sandbox/src/mechanistic_mapper.rs index 1df6953ed..432659974 100644 --- a/crates/openshell-sandbox/src/mechanistic_mapper.rs +++ b/crates/openshell-sandbox/src/mechanistic_mapper.rs @@ -212,6 +212,7 @@ pub async fn generate_proposals(summaries: &[DenialSummary]) -> Vec let stage = denials .first() .map_or_else(|| "connect".to_string(), |d| d.denial_stage.clone()); + let human_summary = rule_name.clone(); proposals.push(PolicyChunk { id: String::new(), // Assigned by the gateway on persist @@ -232,6 +233,11 @@ pub async fn generate_proposals(summaries: &[DenialSummary]) -> Vec binary: binary.clone(), validation_result: String::new(), rejection_reason: String::new(), + human_summary, + intent_summary: String::new(), + request_type: "network_policy".to_string(), + provider_name: String::new(), + provider_type: String::new(), }); } diff --git a/crates/openshell-sandbox/src/policy_local.rs b/crates/openshell-sandbox/src/policy_local.rs index 657fd760f..0721da089 100644 --- a/crates/openshell-sandbox/src/policy_local.rs +++ b/crates/openshell-sandbox/src/policy_local.rs @@ -9,6 +9,7 @@ use openshell_core::proto::{ SandboxPolicy as ProtoSandboxPolicy, }; use openshell_ocsf::{ConfigStateChangeBuilder, SeverityId, StateId, StatusId, ocsf_emit}; +use openshell_providers::get_default_profile; use serde::Deserialize; use std::collections::HashMap; use std::path::{Path, PathBuf}; @@ -32,6 +33,7 @@ pub const SKILL_PATH: &str = "/etc/openshell/skills/policy_advisor.md"; pub const ROUTE_POLICY_CURRENT: &str = "/v1/policy/current"; pub const ROUTE_DENIALS: &str = "/v1/denials"; pub const ROUTE_PROPOSALS: &str = "/v1/proposals"; +pub const ROUTE_PROVIDERS: &str = "/v1/providers"; /// Per-proposal status and long-poll routes live below this prefix: /// `GET /v1/proposals/{chunk_id}` — immediate status /// `GET /v1/proposals/{chunk_id}/wait?timeout` — long-poll until terminal @@ -154,6 +156,7 @@ async fn route_request( match (method, route) { ("GET", ROUTE_POLICY_CURRENT) => current_policy_response(ctx).await, ("GET", ROUTE_DENIALS) => recent_denials_response(ctx, query).await, + ("GET", ROUTE_PROVIDERS) => attached_providers_response(ctx).await, ("POST", ROUTE_PROPOSALS) => submit_proposal(ctx, body).await, ("GET", path) if path.starts_with(ROUTE_PROPOSALS_PREFIX) => { proposal_state_route(ctx, path, query).await @@ -233,6 +236,11 @@ pub fn agent_next_steps() -> serde_json::Value { "method": "GET", "url": format!("http://{host}{ROUTE_DENIALS}?last=5"), }, + { + "action": "inspect_attached_providers", + "method": "GET", + "url": format!("http://{host}{ROUTE_PROVIDERS}"), + }, { "action": "submit_proposal", "method": "POST", @@ -667,7 +675,7 @@ async fn proposal_status_response( Ok(session) => session, Err(err) => return err, }; - fetch_chunk_or_404(&session, chunk_id, false).await + fetch_chunk_or_404(ctx, &session, chunk_id, false).await } /// `GET /v1/proposals/{chunk_id}/wait?timeout=` — block until terminal or @@ -749,18 +757,120 @@ fn chunk_not_found_payload(chunk_id: &str) -> (u16, serde_json::Value) { ) } +async fn attached_providers_response(ctx: &PolicyLocalContext) -> (u16, serde_json::Value) { + let session = match open_lookup_session(ctx).await { + Ok(session) => session, + Err(err) => return err, + }; + let providers = match session + .client + .list_sandbox_providers(session.sandbox_name) + .await + { + Ok(providers) => providers, + Err(e) => return (502, error_payload("gateway_lookup_failed", e.to_string())), + }; + + let providers = providers + .into_iter() + .map(|provider| { + let mut credential_keys = provider.credentials.keys().cloned().collect::>(); + let mut config_keys = provider.config.keys().cloned().collect::>(); + credential_keys.sort(); + config_keys.sort(); + serde_json::json!({ + "provider_name": provider + .metadata + .as_ref() + .map(|meta| meta.name.clone()) + .unwrap_or_default(), + "provider_type": provider.r#type, + "credential_keys": credential_keys, + "config_keys": config_keys, + }) + }) + .collect::>(); + + (200, serde_json::json!({ "providers": providers })) +} + async fn fetch_chunk_or_404( + ctx: &PolicyLocalContext, session: &LookupSession<'_>, chunk_id: &str, timed_out: bool, ) -> (u16, serde_json::Value) { match fetch_chunk(session, chunk_id).await { - Ok(Some(chunk)) => (200, chunk_state_payload(&chunk, timed_out, false)), + Ok(Some(chunk)) => { + let policy_reloaded = chunk_applied_locally(ctx, session, &chunk).await; + (200, chunk_state_payload(&chunk, timed_out, policy_reloaded)) + } Ok(None) => chunk_not_found_payload(chunk_id), Err(err) => err, } } +async fn chunk_applied_locally( + ctx: &PolicyLocalContext, + session: &LookupSession<'_>, + chunk: &PolicyChunk, +) -> bool { + if chunk.status != "approved" { + return false; + } + if chunk.request_type == "provider" { + let provider_name = chunk.provider_name.trim(); + if provider_name.is_empty() { + return true; + } + let attached = match session + .client + .list_sandbox_providers(session.sandbox_name) + .await + { + Ok(providers) => providers.into_iter().find(|provider| { + provider + .metadata + .as_ref() + .is_some_and(|meta| meta.name.trim().eq_ignore_ascii_case(provider_name)) + }), + Err(_) => return false, + }; + let Some(attached) = attached else { + return false; + }; + + let Some(profile) = get_default_profile(attached.r#type.trim()) else { + return true; + }; + let Some(attached_name) = attached + .metadata + .as_ref() + .map(|meta| meta.name.trim()) + .filter(|name| !name.is_empty()) + else { + return true; + }; + let snapshot = ctx.current_policy.read().await.clone(); + let Some(policy) = snapshot.as_ref() else { + return false; + }; + let provider_rule_name = openshell_policy::provider_rule_name(attached_name); + if policy.network_policies.contains_key(&provider_rule_name) { + return true; + } + let provider_rule = profile.network_policy_rule(&provider_rule_name); + return openshell_policy::policy_covers_rule(policy, &provider_rule); + } + let Some(rule) = chunk.proposed_rule.as_ref() else { + return false; + }; + let snapshot = ctx.current_policy.read().await.clone(); + snapshot + .as_ref() + .is_some_and(|policy| openshell_policy::policy_covers_rule(policy, rule)) +} + /// Build the agent-facing response for a chunk. /// /// Selection rule: include the fields the agent needs to decide what to do @@ -782,6 +892,9 @@ fn chunk_state_payload( "status": chunk.status, "rule_name": chunk.rule_name, "binary": chunk.binary, + "request_type": chunk.request_type, + "provider_name": chunk.provider_name, + "provider_type": chunk.provider_type, "rejection_reason": chunk.rejection_reason, "validation_result": chunk.validation_result, }); @@ -931,18 +1044,27 @@ fn proposal_chunks_from_body(body: &[u8]) -> std::result::Result std::result::Result, intent_summary: &str, ) -> std::result::Result { let mut rule = network_rule_from_json(add_rule.rule)?; @@ -971,6 +1094,11 @@ fn policy_chunk_from_add_rule( .first() .map(|binary| binary.path.clone()) .unwrap_or_default(); + let headline = human_summary + .map(str::trim) + .filter(|summary| !summary.is_empty()) + .unwrap_or(&rule_name) + .to_string(); Ok(PolicyChunk { id: String::new(), @@ -991,6 +1119,66 @@ fn policy_chunk_from_add_rule( binary, validation_result: String::new(), rejection_reason: String::new(), + human_summary: headline, + intent_summary: intent_summary.to_string(), + request_type: "network_policy".to_string(), + provider_name: String::new(), + provider_type: String::new(), + }) +} + +fn policy_chunk_from_provider_request( + request: RequestProviderJson, + human_summary: Option<&str>, + intent_summary: &str, +) -> std::result::Result { + let provider_name = request.provider_name.trim(); + if provider_name.is_empty() { + return Err("requestProvider.providerName is required".to_string()); + } + let provider_type = request.provider_type.unwrap_or_default(); + let provider_type = provider_type.trim(); + let rule_name = request + .rule_name + .as_deref() + .map(str::trim) + .filter(|name| !name.is_empty()) + .map_or_else( + || format!("request_provider_{}", provider_name.replace('-', "_")), + ToString::to_string, + ); + let headline = human_summary + .map(str::trim) + .filter(|summary| !summary.is_empty()) + .map_or_else( + || format!("Attach provider {provider_name}"), + ToString::to_string, + ); + + Ok(PolicyChunk { + id: String::new(), + status: "pending".to_string(), + rule_name, + proposed_rule: None, + rationale: intent_summary.to_string(), + security_notes: String::new(), + confidence: 0.75, + denial_summary_ids: vec![], + created_at_ms: 0, + decided_at_ms: 0, + stage: "agent".to_string(), + supersedes_chunk_id: String::new(), + hit_count: 1, + first_seen_ms: 0, + last_seen_ms: 0, + binary: String::new(), + validation_result: String::new(), + rejection_reason: String::new(), + human_summary: headline, + intent_summary: intent_summary.to_string(), + request_type: "provider".to_string(), + provider_name: provider_name.to_string(), + provider_type: provider_type.to_string(), }) } @@ -1208,6 +1396,8 @@ fn error_payload(error: &str, detail: String) -> serde_json::Value { #[derive(Debug, Deserialize)] struct ProposalRequest { + #[serde(default)] + human_summary: Option, #[serde(default)] intent_summary: Option, #[serde(default)] @@ -1221,6 +1411,16 @@ struct AddNetworkRuleJson { rule: NetworkPolicyRuleJson, } +#[derive(Debug, Deserialize)] +struct RequestProviderJson { + #[serde(default, rename = "ruleName")] + rule_name: Option, + #[serde(rename = "providerName")] + provider_name: String, + #[serde(default, rename = "providerType")] + provider_type: Option, +} + #[derive(Debug, Deserialize)] struct NetworkPolicyRuleJson { #[serde(default)] @@ -1293,6 +1493,7 @@ mod tests { #[test] fn proposal_chunks_from_body_accepts_add_rule_operation() { let body = br#"{ + "human_summary": "Allow GitHub repo creation", "intent_summary": "Allow gh to create one repo.", "operations": [ { @@ -1331,6 +1532,9 @@ mod tests { assert_eq!(chunks.len(), 1); assert_eq!(chunks[0].rule_name, "github_api_repo_create"); + assert_eq!(chunks[0].human_summary, "Allow GitHub repo creation"); + assert_eq!(chunks[0].intent_summary, "Allow gh to create one repo."); + assert_eq!(chunks[0].request_type, "network_policy"); assert_eq!(chunks[0].rationale, "Allow gh to create one repo."); assert_eq!(chunks[0].binary, "/usr/bin/gh"); let rule = chunks[0].proposed_rule.as_ref().unwrap(); @@ -1345,6 +1549,35 @@ mod tests { ); } + #[test] + fn proposal_chunks_from_body_accepts_provider_request_operation() { + let body = br#"{ + "human_summary": "Attach GitHub provider", + "intent_summary": "Review pull requests with the host-managed GitHub token.", + "operations": [ + { + "requestProvider": { + "providerName": "github", + "providerType": "github" + } + } + ] + }"#; + + let chunks = proposal_chunks_from_body(body).unwrap(); + + assert_eq!(chunks.len(), 1); + assert_eq!(chunks[0].request_type, "provider"); + assert_eq!(chunks[0].provider_name, "github"); + assert_eq!(chunks[0].provider_type, "github"); + assert_eq!(chunks[0].human_summary, "Attach GitHub provider"); + assert_eq!( + chunks[0].intent_summary, + "Review pull requests with the host-managed GitHub token." + ); + assert!(chunks[0].proposed_rule.is_none()); + } + #[test] fn proposal_chunks_from_body_rejects_query_in_l7_path() { let body = br#"{ @@ -1558,12 +1791,13 @@ mod tests { let _guard = ProposalsFlagGuard::set_blocking(true); let steps = agent_next_steps(); let arr = steps.as_array().expect("agent_next_steps is an array"); - assert_eq!(arr.len(), 4, "expected 4 next_steps when feature is on"); + assert_eq!(arr.len(), 5, "expected 5 next_steps when feature is on"); let actions: Vec<&str> = arr .iter() .filter_map(|v| v.get("action").and_then(serde_json::Value::as_str)) .collect(); assert!(actions.contains(&"read_skill")); + assert!(actions.contains(&"inspect_attached_providers")); assert!(actions.contains(&"submit_proposal")); } @@ -1736,6 +1970,16 @@ mod tests { assert_eq!(body["error"], "gateway_unavailable"); } + #[tokio::test] + async fn providers_route_returns_503_when_no_gateway() { + let _guard = ProposalsFlagGuard::set(true).await; + let ctx = PolicyLocalContext::new(None, None, Some("test-sandbox".to_string())); + + let (status, body) = route_request(&ctx, "GET", "/v1/providers", &[]).await; + assert_eq!(status, 503); + assert_eq!(body["error"], "gateway_unavailable"); + } + #[tokio::test] async fn proposal_routes_return_feature_disabled_when_flag_off() { let _guard = ProposalsFlagGuard::set(false).await; diff --git a/crates/openshell-sandbox/src/skills/policy_advisor.md b/crates/openshell-sandbox/src/skills/policy_advisor.md index 8ca64f977..8bb9d1595 100644 --- a/crates/openshell-sandbox/src/skills/policy_advisor.md +++ b/crates/openshell-sandbox/src/skills/policy_advisor.md @@ -37,8 +37,10 @@ The sandbox-local policy API is reachable at `http://policy.local`: already loaded a policy containing the approved rule. Use this endpoint instead of polling `/v1/proposals/{chunk_id}`. -The proposal body takes an `intent_summary` and one or more `addRule` -operations. Each `addRule` carries a complete narrow `NetworkPolicyRule`. +The proposal body takes an optional `human_summary`, an `intent_summary`, and +one or more operations. Use `addRule` for network policy. Use +`requestProvider` when the task needs an existing host-managed credential +provider attached to the sandbox. ## Workflow @@ -84,6 +86,7 @@ A complete narrow REST-inspected rule looks like this: ```json { + "human_summary": "Allow GitHub repository content writes", "intent_summary": "Allow gh to update repository contents in NVIDIA/OpenShell only.", "operations": [ { @@ -119,6 +122,23 @@ A complete narrow REST-inspected rule looks like this: } ``` +A provider request looks like this: + +```json +{ + "human_summary": "Attach GitHub provider", + "intent_summary": "Use the host-managed GitHub token to review pull requests without exposing the raw token in the sandbox.", + "operations": [ + { + "requestProvider": { + "providerName": "github", + "providerType": "github" + } + } + ] +} +``` + ## Norms - Do not propose wildcard hosts such as `**` or `*.com`. diff --git a/crates/openshell-server/src/auth/oidc.rs b/crates/openshell-server/src/auth/oidc.rs index 554fa0593..4fdf97095 100644 --- a/crates/openshell-server/src/auth/oidc.rs +++ b/crates/openshell-server/src/auth/oidc.rs @@ -46,6 +46,7 @@ const SANDBOX_SECRET_METHODS: &[&str] = &[ "/openshell.v1.OpenShell/ReportPolicyStatus", "/openshell.v1.OpenShell/PushSandboxLogs", "/openshell.v1.OpenShell/GetSandboxProviderEnvironment", + "/openshell.v1.OpenShell/ListSandboxProviders", "/openshell.v1.OpenShell/SubmitPolicyAnalysis", "/openshell.sandbox.v1.SandboxService/GetSandboxConfig", "/openshell.inference.v1.Inference/GetInferenceBundle", @@ -475,6 +476,9 @@ mod tests { assert!(is_sandbox_secret_method( "/openshell.v1.OpenShell/GetSandboxProviderEnvironment" )); + assert!(is_sandbox_secret_method( + "/openshell.v1.OpenShell/ListSandboxProviders" + )); assert!(is_sandbox_secret_method( "/openshell.v1.OpenShell/ReportPolicyStatus" )); diff --git a/crates/openshell-server/src/grpc/policy.rs b/crates/openshell-server/src/grpc/policy.rs index 2930ed975..9597311a6 100644 --- a/crates/openshell-server/src/grpc/policy.rs +++ b/crates/openshell-server/src/grpc/policy.rs @@ -10,6 +10,8 @@ #![allow(clippy::cast_precision_loss)] // f64->f32 for confidence scores #![allow(clippy::items_after_statements)] // DB_PORTS const inside function +use super::provider::get_provider_record; +use super::validation::validate_sandbox_spec; use crate::persistence::{DraftChunkRecord, ObjectId, ObjectName, ObjectType, PolicyRecord, Store}; use crate::policy_store::PolicyStoreExt; use crate::{ServerState, auth::oidc}; @@ -49,7 +51,7 @@ use openshell_policy::{ use openshell_providers::{get_default_profile, normalize_provider_type}; use prost::Message; use sha2::{Digest, Sha256}; -use std::collections::{BTreeMap, HashMap}; +use std::collections::{BTreeMap, HashMap, HashSet}; use std::net::{IpAddr, Ipv4Addr}; use std::sync::Arc; use tonic::{Request, Response, Status}; @@ -58,7 +60,9 @@ use tracing::{debug, info, warn}; use super::validation::{ level_matches, source_matches, validate_policy_safety, validate_static_fields_unchanged, }; -use super::{MAX_PAGE_SIZE, StoredSettingValue, StoredSettings, clamp_limit, current_time_ms}; +use super::{ + MAX_PAGE_SIZE, MAX_PROVIDERS, StoredSettingValue, StoredSettings, clamp_limit, current_time_ms, +}; // --------------------------------------------------------------------------- // Constants @@ -585,6 +589,7 @@ async fn profile_provider_policy_layers( fn bool_setting_enabled(settings: &StoredSettings, key: &str) -> Result { match settings.settings.get(key) { + None if key == settings::PROVIDERS_V2_ENABLED_KEY => Ok(true), None => Ok(false), Some(StoredSettingValue::Bool(value)) => Ok(*value), Some(_) => Err(Status::internal(format!( @@ -1331,6 +1336,123 @@ pub(super) async fn handle_push_sandbox_logs( // Draft policy recommendation handlers // --------------------------------------------------------------------------- +fn normalize_draft_request_type(request_type: &str) -> String { + match request_type.trim() { + "" | "network_policy" => "network_policy".to_string(), + "provider" => "provider".to_string(), + other => other.to_string(), + } +} + +fn is_provider_request_chunk(chunk: &DraftChunkRecord) -> bool { + normalize_draft_request_type(&chunk.request_type) == "provider" +} + +fn summarize_provider_request_chunk(chunk: &DraftChunkRecord) -> Result { + if chunk.provider_name.trim().is_empty() { + return Err(Status::invalid_argument( + "provider request missing provider_name", + )); + } + let provider_type = if chunk.provider_type.trim().is_empty() { + String::new() + } else { + format!(" type={}", chunk.provider_type) + }; + Ok(format!( + "attach-provider {}{provider_type}", + chunk.provider_name + )) +} + +async fn approve_provider_request_chunk( + state: &Arc, + sandbox_name: &str, + chunk: &DraftChunkRecord, +) -> Result { + let provider_name = chunk.provider_name.trim(); + if provider_name.is_empty() { + return Err(Status::invalid_argument( + "provider request missing provider_name", + )); + } + + let provider = get_provider_record(state.store.as_ref(), provider_name) + .await + .map_err(|err| { + if err.code() == tonic::Code::NotFound { + Status::failed_precondition(format!("provider '{provider_name}' not found")) + } else { + err + } + })?; + if !chunk.provider_type.trim().is_empty() && provider.r#type != chunk.provider_type.trim() { + return Err(Status::failed_precondition(format!( + "provider '{provider_name}' has type '{}', expected '{}'", + provider.r#type, + chunk.provider_type.trim() + ))); + } + + let _sandbox_sync_guard = state.compute.sandbox_sync_guard().await; + let mut sandbox = state + .store + .get_message_by_name::(sandbox_name) + .await + .map_err(|e| Status::internal(format!("fetch sandbox failed: {e}")))? + .ok_or_else(|| Status::not_found("sandbox not found"))?; + let spec = sandbox + .spec + .as_mut() + .ok_or_else(|| Status::failed_precondition("sandbox spec is missing"))?; + + dedupe_strings(&mut spec.providers); + let attached = if spec.providers.iter().any(|name| name == provider_name) { + false + } else { + if spec.providers.len() >= MAX_PROVIDERS { + return Err(Status::invalid_argument(format!( + "providers list exceeds maximum ({MAX_PROVIDERS})" + ))); + } + spec.providers.push(provider_name.to_string()); + true + }; + validate_sandbox_spec(sandbox_name, spec)?; + + state + .store + .put_message(&sandbox) + .await + .map_err(|e| Status::internal(format!("persist sandbox failed: {e}")))?; + + info!( + sandbox_name, + provider_name, attached, "ApproveDraftChunk: provider request attached provider" + ); + Ok(attached) +} + +async fn current_sandbox_policy_marker( + state: &Arc, + sandbox_id: &str, +) -> Result<(i64, String), Status> { + let config = handle_get_sandbox_config( + state, + Request::new(GetSandboxConfigRequest { + sandbox_id: sandbox_id.to_string(), + }), + ) + .await? + .into_inner(); + Ok((i64::from(config.version), config.policy_hash)) +} + +fn dedupe_strings(values: &mut Vec) { + let mut seen = HashSet::new(); + values.retain(|value| seen.insert(value.clone())); +} + pub(super) async fn handle_submit_policy_analysis( state: &Arc, request: Request, @@ -1361,16 +1483,30 @@ pub(super) async fn handle_submit_policy_analysis( let mut accepted_chunk_ids: Vec = Vec::new(); for chunk in &req.proposed_chunks { + let request_type = normalize_draft_request_type(&chunk.request_type); if chunk.rule_name.is_empty() { rejected += 1; rejection_reasons.push("chunk missing rule_name".to_string()); continue; } - if chunk.proposed_rule.is_none() { + if request_type == "network_policy" && chunk.proposed_rule.is_none() { rejected += 1; rejection_reasons.push(format!("chunk '{}' missing proposed_rule", chunk.rule_name)); continue; } + if request_type == "provider" && chunk.provider_name.trim().is_empty() { + rejected += 1; + rejection_reasons.push(format!("chunk '{}' missing provider_name", chunk.rule_name)); + continue; + } + if request_type != "network_policy" && request_type != "provider" { + rejected += 1; + rejection_reasons.push(format!( + "chunk '{}' has unsupported request_type '{}'", + chunk.rule_name, chunk.request_type + )); + continue; + } let now_ms = current_time_ms().map_err(|e| Status::internal(format!("timestamp error: {e}")))?; @@ -1424,6 +1560,15 @@ pub(super) async fn handle_submit_policy_analysis( }, validation_result: String::new(), rejection_reason: String::new(), + human_summary: chunk.human_summary.clone(), + intent_summary: if chunk.intent_summary.is_empty() { + chunk.rationale.clone() + } else { + chunk.intent_summary.clone() + }, + request_type, + provider_name: chunk.provider_name.trim().to_string(), + provider_type: chunk.provider_type.trim().to_string(), }; // Mechanistic mode dedups N denials targeting the same endpoint // into one chunk. All other modes (agent-authored proposals, future @@ -1562,12 +1707,20 @@ pub(super) async fn handle_approve_draft_chunk( port = chunk.port, hit_count = chunk.hit_count, prev_status = %chunk.status, - "ApproveDraftChunk: merging rule into active policy" + request_type = %normalize_draft_request_type(&chunk.request_type), + "ApproveDraftChunk: approving draft chunk" ); - let (version, hash) = - merge_chunk_into_policy(state.store.as_ref(), &sandbox_id, &chunk).await?; - let chunk_summary = summarize_draft_chunk_rule(&chunk)?; + let (version, hash, chunk_summary) = if is_provider_request_chunk(&chunk) { + approve_provider_request_chunk(state, sandbox.object_name(), &chunk).await?; + let (version, hash) = current_sandbox_policy_marker(state, &sandbox_id).await?; + (version, hash, summarize_provider_request_chunk(&chunk)?) + } else { + let (version, hash) = + merge_chunk_into_policy(state.store.as_ref(), &sandbox_id, &chunk).await?; + let chunk_summary = summarize_draft_chunk_rule(&chunk)?; + (version, hash, chunk_summary) + }; let now_ms = current_time_ms().map_err(|e| Status::internal(format!("timestamp error: {e}")))?; @@ -1753,11 +1906,18 @@ pub(super) async fn handle_approve_all_draft_chunks( "ApproveAllDraftChunks: merging chunk" ); - let (version, hash) = - merge_chunk_into_policy(state.store.as_ref(), &sandbox_id, chunk).await?; + let (version, hash, chunk_summary) = if is_provider_request_chunk(chunk) { + approve_provider_request_chunk(state, sandbox.object_name(), chunk).await?; + let (version, hash) = current_sandbox_policy_marker(state, &sandbox_id).await?; + (version, hash, summarize_provider_request_chunk(chunk)?) + } else { + let (version, hash) = + merge_chunk_into_policy(state.store.as_ref(), &sandbox_id, chunk).await?; + let chunk_summary = summarize_draft_chunk_rule(chunk)?; + (version, hash, chunk_summary) + }; last_version = version; last_hash = hash; - let chunk_summary = summarize_draft_chunk_rule(chunk)?; let now_ms = current_time_ms().map_err(|e| Status::internal(format!("timestamp error: {e}")))?; @@ -2135,6 +2295,11 @@ fn draft_chunk_record_to_proto(record: &DraftChunkRecord) -> Result(&sandbox_name) + .await + .unwrap() + .unwrap(); + assert_eq!( + updated.spec.unwrap().providers, + vec!["work-github".to_string()] + ); + } + /// A reviewer's free-form rejection reason must round-trip through /// persistence and surface on the chunk via `GetDraftPolicy`, so the /// in-sandbox agent can read the guidance and redraft. The MVP-v2 agent @@ -4867,6 +5127,11 @@ mod tests { last_seen_ms: 0, validation_result: String::new(), rejection_reason: String::new(), + human_summary: String::new(), + intent_summary: String::new(), + request_type: "network_policy".to_string(), + provider_name: String::new(), + provider_type: String::new(), }; let (version, _) = merge_chunk_into_policy(&store, &chunk.sandbox_id, &chunk) @@ -4963,6 +5228,11 @@ mod tests { last_seen_ms: 0, validation_result: String::new(), rejection_reason: String::new(), + human_summary: String::new(), + intent_summary: String::new(), + request_type: "network_policy".to_string(), + provider_name: String::new(), + provider_type: String::new(), }; let (version, _) = merge_chunk_into_policy(&store, sandbox_id, &chunk) @@ -5064,6 +5334,11 @@ mod tests { last_seen_ms: 0, validation_result: String::new(), rejection_reason: String::new(), + human_summary: String::new(), + intent_summary: String::new(), + request_type: "network_policy".to_string(), + provider_name: String::new(), + provider_type: String::new(), }; let (version, _) = merge_chunk_into_policy(&store, sandbox_id, &chunk) diff --git a/crates/openshell-server/src/policy_store.rs b/crates/openshell-server/src/policy_store.rs index 9a6333543..81d4ec107 100644 --- a/crates/openshell-server/src/policy_store.rs +++ b/crates/openshell-server/src/policy_store.rs @@ -365,6 +365,11 @@ pub fn draft_chunk_payload_from_record(chunk: &DraftChunkRecord) -> PersistenceR draft_version: chunk.draft_version, validation_result: chunk.validation_result.clone(), rejection_reason: chunk.rejection_reason.clone(), + human_summary: chunk.human_summary.clone(), + intent_summary: chunk.intent_summary.clone(), + request_type: chunk.request_type.clone(), + provider_name: chunk.provider_name.clone(), + provider_type: chunk.provider_type.clone(), } .encode_to_vec()) } @@ -407,5 +412,10 @@ pub fn draft_chunk_record_from_parts( last_seen_ms: updated_at_ms, validation_result: wrapper.validation_result, rejection_reason: wrapper.rejection_reason, + human_summary: wrapper.human_summary, + intent_summary: wrapper.intent_summary, + request_type: wrapper.request_type, + provider_name: wrapper.provider_name, + provider_type: wrapper.provider_type, }) } diff --git a/crates/openshell-tui/src/ui/sandbox_draft.rs b/crates/openshell-tui/src/ui/sandbox_draft.rs index a9cd6b0c9..d0603df4e 100644 --- a/crates/openshell-tui/src/ui/sandbox_draft.rs +++ b/crates/openshell-tui/src/ui/sandbox_draft.rs @@ -7,7 +7,7 @@ use crate::app::App; use openshell_core::proto::PolicyChunk; use ratatui::Frame; use ratatui::layout::{Constraint, Direction, Layout, Rect}; -use ratatui::style::Modifier; +use ratatui::style::{Modifier, Style}; use ratatui::text::{Line, Span}; use ratatui::widgets::{Block, Borders, Clear, Padding, Paragraph, Wrap}; @@ -111,18 +111,13 @@ pub fn draw(frame: &mut Frame<'_>, app: &mut App, area: Rect) { spans.push(Span::raw(" ")); } - // Endpoint summary (host:port). - let endpoint_str = chunk - .proposed_rule - .as_ref() - .and_then(|r| r.endpoints.first()) - .map(|ep| format!("{}:{}", ep.host, ep.port)) - .unwrap_or_default(); + let headline = chunk_headline(chunk); + let target_str = chunk_target(chunk); - spans.push(Span::styled(&chunk.rule_name, name_style)); - if !endpoint_str.is_empty() { + spans.push(Span::styled(headline, name_style)); + if !target_str.is_empty() { spans.push(Span::styled(" ", t.muted)); - spans.push(Span::styled(endpoint_str, t.accent)); + spans.push(Span::styled(target_str, t.accent)); } // Show binary name (just the filename, not full path) if present. if !chunk.binary.is_empty() { @@ -132,6 +127,19 @@ pub fn draw(frame: &mut Frame<'_>, app: &mut App, area: Rect) { } spans.push(Span::raw(" ")); spans.push(Span::styled(format!("[{}]", chunk.status), status_style)); + if !chunk.validation_result.is_empty() { + spans.push(Span::styled(" ", t.muted)); + spans.push(Span::styled( + format!("[{}]", chunk.validation_result), + validation_style(chunk, t), + )); + } + if chunk_has_l4_scope_warning(chunk) { + spans.push(Span::styled(" [L4]", t.status_warn)); + } + if chunk.status == "rejected" && !chunk.rejection_reason.is_empty() { + spans.push(Span::styled(" guidance", t.status_err)); + } spans.push(Span::styled( format!(" {:.0}%", chunk.confidence * 100.0), t.muted, @@ -198,6 +206,33 @@ pub fn draw_detail_popup( ]), ]; + if !chunk.human_summary.is_empty() { + lines.push(Line::from(vec![ + Span::styled("Summary: ", t.muted), + Span::styled(&chunk.human_summary, t.text), + ])); + } + + if !chunk.validation_result.is_empty() { + lines.push(Line::from(vec![ + Span::styled("Validation: ", t.muted), + Span::styled(&chunk.validation_result, validation_style(chunk, t)), + ])); + } + + if chunk.request_type == "provider" { + lines.push(Line::from(vec![ + Span::styled("Provider: ", t.muted), + Span::styled(&chunk.provider_name, t.accent), + ])); + if !chunk.provider_type.is_empty() { + lines.push(Line::from(vec![ + Span::styled("Type: ", t.muted), + Span::styled(&chunk.provider_type, t.text), + ])); + } + } + // Binary (denormalized from the denial). if !chunk.binary.is_empty() { lines.push(Line::from(vec![ @@ -232,11 +267,15 @@ pub fn draw_detail_popup( lines.push(Line::from("")); lines.push(Line::from(Span::styled("Endpoints:", t.muted))); for ep in &rule.endpoints { - lines.push(Line::from(vec![ + let mut endpoint_spans = vec![ Span::raw(" "), Span::styled("-> ", t.muted), Span::styled(format!("{}:{}", ep.host, ep.port), t.accent), - ])); + ]; + if ep.protocol.is_empty() && ep.access.is_empty() && ep.rules.is_empty() { + endpoint_spans.push(Span::styled(" L4 only", t.status_warn)); + } + lines.push(Line::from(endpoint_spans)); } // Binaries. @@ -252,8 +291,13 @@ pub fn draw_detail_popup( } } - // Rationale. - if !chunk.rationale.is_empty() { + if !chunk.intent_summary.is_empty() { + lines.push(Line::from("")); + lines.push(Line::from(vec![ + Span::styled("Intent: ", t.muted), + Span::styled(&chunk.intent_summary, t.text), + ])); + } else if !chunk.rationale.is_empty() { lines.push(Line::from("")); lines.push(Line::from(vec![ Span::styled("Rationale: ", t.muted), @@ -270,6 +314,22 @@ pub fn draw_detail_popup( )])); } + if chunk_has_l4_scope_warning(chunk) { + lines.push(Line::from("")); + lines.push(Line::from(vec![Span::styled( + "! L4-only request: this allows host/port reachability without HTTP method or path scoping.", + t.status_warn.add_modifier(Modifier::BOLD), + )])); + } + + if chunk.status == "rejected" && !chunk.rejection_reason.is_empty() { + lines.push(Line::from("")); + lines.push(Line::from(vec![ + Span::styled("Guidance: ", t.status_err.add_modifier(Modifier::BOLD)), + Span::styled(&chunk.rejection_reason, t.text), + ])); + } + // Action hints — state-aware toggle keys. lines.push(Line::from("")); let mut hint_spans: Vec> = Vec::new(); @@ -430,6 +490,51 @@ fn truncate_str(s: &str, max_len: usize) -> String { } } +fn chunk_headline(chunk: &PolicyChunk) -> &str { + if chunk.human_summary.is_empty() { + &chunk.rule_name + } else { + &chunk.human_summary + } +} + +fn chunk_target(chunk: &PolicyChunk) -> String { + if chunk.request_type == "provider" { + if chunk.provider_name.is_empty() { + return String::new(); + } + if chunk.provider_type.is_empty() { + return format!("provider {}", chunk.provider_name); + } + return format!("provider {} ({})", chunk.provider_name, chunk.provider_type); + } + chunk + .proposed_rule + .as_ref() + .and_then(|r| r.endpoints.first()) + .map(|ep| format!("{}:{}", ep.host, ep.port)) + .unwrap_or_default() +} + +fn chunk_has_l4_scope_warning(chunk: &PolicyChunk) -> bool { + chunk.proposed_rule.as_ref().is_some_and(|rule| { + rule.endpoints + .iter() + .any(|ep| ep.protocol.is_empty() && ep.access.is_empty() && ep.rules.is_empty()) + }) +} + +fn validation_style(chunk: &PolicyChunk, theme: &crate::theme::Theme) -> Style { + let value = chunk.validation_result.to_ascii_lowercase(); + if value.contains("fail") || value.contains("error") || value.contains("deny") { + theme.status_err + } else if value.contains("warn") || value.contains("l4") { + theme.status_warn + } else { + theme.status_ok + } +} + fn format_short_time(epoch_ms: i64) -> String { if epoch_ms <= 0 { return String::from("--:--:--"); diff --git a/docs/sandboxes/manage-providers.mdx b/docs/sandboxes/manage-providers.mdx index 6f50e1725..5edc76eb8 100644 --- a/docs/sandboxes/manage-providers.mdx +++ b/docs/sandboxes/manage-providers.mdx @@ -55,14 +55,13 @@ openshell provider create --name my-api --type generic --credential API_KEY This looks up the current value of `$API_KEY` in your shell and stores it. Provider profile metadata is available for known provider types. Provider profile -network policy is gateway opt-in: +network policy is enabled by default. To attach provider credentials without +adding provider profile network policy layers, disable it: ```shell -openshell settings set --global --key providers_v2_enabled --value true +openshell settings set --global --key providers_v2_enabled --value false ``` -Without `providers_v2_enabled=true`, provider behavior remains credential-only. - ## Manage Providers List, inspect, update, and delete providers from the active gateway. @@ -101,14 +100,13 @@ openshell sandbox create --provider my-claude --provider my-github -- claude Each `--provider` flag attaches one provider. The sandbox receives all credentials from every attached provider at runtime. Profile-managed providers -also contribute provider-generated network policy entries when -`providers_v2_enabled` is enabled at the gateway. When the setting is disabled, -providers keep the previous behavior and only provide credentials. +also contribute provider-generated network policy entries unless +`providers_v2_enabled` is disabled at the gateway. When the setting is disabled, +providers only provide credentials. -Providers cannot be added to a running sandbox. If you need to attach an -additional provider, delete the sandbox and recreate it with all required -providers specified. +Provider attachment changes on a running sandbox require operator approval and +are applied through the draft policy workflow. diff --git a/proto/openshell.proto b/proto/openshell.proto index d6bcbece2..d40d59daf 100644 --- a/proto/openshell.proto +++ b/proto/openshell.proto @@ -1305,6 +1305,17 @@ message PolicyChunk { // back to the in-sandbox agent so it can revise the proposal. // Empty for non-rejected chunks. string rejection_reason = 18; + // Human-readable proposal headline for inbox display. + string human_summary = 19; + // Agent-supplied intent context for inbox display. + string intent_summary = 20; + // Proposal kind. Empty and "network_policy" mean proposed_rule is merged. + // "provider" means approval attaches provider_name to the sandbox. + string request_type = 21; + // Existing provider record requested by a provider proposal. + string provider_name = 22; + // Optional expected provider type, used for display and validation hints. + string provider_type = 23; } // Notification that the draft policy was updated. @@ -1516,6 +1527,16 @@ message DraftChunkPayload { // Operator-supplied free-form rejection text; empty for non-rejected // chunks. Mirrors PolicyChunk.rejection_reason. string rejection_reason = 12; + // Mirrors PolicyChunk.human_summary. + string human_summary = 13; + // Mirrors PolicyChunk.intent_summary. + string intent_summary = 14; + // Mirrors PolicyChunk.request_type. + string request_type = 15; + // Mirrors PolicyChunk.provider_name. + string provider_name = 16; + // Mirrors PolicyChunk.provider_type. + string provider_type = 17; } // Internal stored policy revision row materialized from the generic objects table. @@ -1554,4 +1575,14 @@ message StoredDraftChunk { string validation_result = 18; // Operator-supplied free-form rejection text. See PolicyChunk. string rejection_reason = 19; + // Human-readable proposal headline. See PolicyChunk. + string human_summary = 20; + // Agent-supplied intent context. See PolicyChunk. + string intent_summary = 21; + // Proposal kind. See PolicyChunk. + string request_type = 22; + // Existing provider record requested by a provider proposal. See PolicyChunk. + string provider_name = 23; + // Optional expected provider type. See PolicyChunk. + string provider_type = 24; } diff --git a/providers/github.yaml b/providers/github.yaml index cc24ae922..907b9291a 100644 --- a/providers/github.yaml +++ b/providers/github.yaml @@ -23,4 +23,12 @@ endpoints: protocol: rest access: read-only enforcement: enforce -binaries: [/usr/bin/gh, /usr/local/bin/gh, /usr/bin/git, /usr/local/bin/git] +binaries: + - /usr/bin/gh + - /usr/local/bin/gh + - /usr/bin/git + - /usr/local/bin/git + - /usr/bin/curl + - /usr/local/bin/curl + - /usr/bin/node + - /usr/local/bin/node diff --git a/providers/gitlab.yaml b/providers/gitlab.yaml index 6d6535c75..97da4edb4 100644 --- a/providers/gitlab.yaml +++ b/providers/gitlab.yaml @@ -23,4 +23,12 @@ endpoints: protocol: rest access: read-write enforcement: enforce -binaries: [/usr/bin/glab, /usr/local/bin/glab, /usr/bin/git, /usr/local/bin/git] +binaries: + - /usr/bin/glab + - /usr/local/bin/glab + - /usr/bin/git + - /usr/local/bin/git + - /usr/bin/curl + - /usr/local/bin/curl + - /usr/bin/node + - /usr/local/bin/node