Skip to content
Open
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
12 changes: 8 additions & 4 deletions .agents/skills/generate-sandbox-policy/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,10 @@ For this tier, default to:
- `access: read-only` when the user says "read", "browse", "view", "query", "fetch"
- `access: read-write` when the user says "read-write", "create", "update" (but not "delete")
- `access: full` when the user says "full access", "everything", "unrestricted"
- L4-only (no `protocol`) when the user says "just allow it", "pass through", "no inspection"
- L4-only (omit `protocol`, or use explicit `protocol: tcp`) when the user says
"just allow it", "pass through", "no inspection". Prefer omission unless the
user wants the transport intent stated explicitly; both currently have the
same host/port enforcement behavior.

### Moderate Tier (host + partial path knowledge)

Expand Down Expand Up @@ -188,7 +191,7 @@ Follow this decision tree based on the detail tier and user intent:
```
Is L7 inspection needed?
├─ No (user wants pass-through / "just allow it")
│ └─ Generate L4-only policy (no protocol, no tls, no rules/access)
│ └─ Generate L4-only policy (no protocol, or protocol: tcp; no tls/rules/access)
└─ Yes (user wants method/path control)
Expand Down Expand Up @@ -372,7 +375,8 @@ Before presenting the policy to the user, verify correctness **and** flag breadt
### Hard Errors (would block sandbox startup)

- [ ] `rules` and `access` are NOT both present on the same endpoint
- [ ] If `protocol` is set, either `rules` or `access` is also present
- [ ] If an L7 `protocol` is set, either `rules` or `access` is also present;
`protocol: tcp` is L4-only and must not contain either field
- [ ] If `tls: terminate` is set, `protocol` is also set
- [ ] `rules` list is not empty when present
- [ ] If `protocol: sql`, `enforcement` is not `enforce`
Expand Down Expand Up @@ -402,7 +406,7 @@ Evaluate the generated policy for overly broad access and **include warnings in

| Condition | Warning to show |
|-----------|----------------|
| **L4-only** (no `protocol`) | "This policy allows all HTTP methods and paths without inspection. The proxy will only check host:port and binary identity. Consider adding `protocol: rest` with a preset if you want method-level control." |
| **L4-only** (no `protocol`, or `protocol: tcp`) | "This policy allows all HTTP methods and paths without inspection. The proxy will only check host:port and binary identity. Consider adding `protocol: rest` with a preset if you want method-level control." |
| **`access: full`** | "This policy allows all HTTP methods (including DELETE) on all paths. If you don't need DELETE, `read-write` is safer. If you only need to read, `read-only` is the most restrictive option." |
| **`access: full` + `enforcement: audit`** | "Full access in audit mode provides no actual restriction — all traffic flows through. This is effectively a monitoring-only policy." |
| **`access: read-write`** when user hasn't confirmed write need | "This policy allows POST, PUT, and PATCH on all paths. If you only need to read data, `read-only` is more restrictive." |
Expand Down
3 changes: 3 additions & 0 deletions .agents/skills/openshell-cli/cli-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,9 @@ Incrementally merge live network policy changes into the current sandbox policy.
Notes:

- The sandbox name defaults to the last-used sandbox.
- `protocol` accepts `tcp` for explicit L4-only host/port policy. It is
currently equivalent to omitting the protocol and cannot be combined with
`access`, `rules`, or L7 enforcement options.
- `--add-allow` and `--add-deny` operate on REST and WebSocket endpoints. Use full YAML for JSON-RPC, MCP, SQL, or other policy structure.
- `--wait` cannot be combined with `--dry-run`.
- Use `policy set` when replacing the full policy or changing static sections.
Expand Down
12 changes: 8 additions & 4 deletions architecture/sandbox.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,17 +60,21 @@ socket inode.

CONNECT and absolute-form forward HTTP are explicit-proxy adapters over the same
egress pipeline. Each adapter normalizes its request into an egress intent, and
the shared authorization result carries the process evidence used by destination
validation and relay selection. During the compatibility migration, endpoint
state is hydrated at the adapters' existing policy query points; it is not yet
one atomic, generation-consistent authorization result. Destination validation
the shared authorization result carries the process evidence and endpoint
metadata used by destination validation and relay selection. Network action,
matched policy, endpoint configuration, and exact-host authorization are
evaluated as one atomic snapshot from one policy generation. Destination validation
returns an unopened connector so adapters retain their existing response and
upstream-dial timing. CONNECT prepares a generation-pinned relay context before
entering shared TLS-terminated or plaintext HTTP relays; non-HTTP traffic uses
the shared raw byte relay after the existing adapter gates. Forward HTTP retains
its guarded single-request relay while sharing authorization, request context,
policy-pinning, and destination boundaries.
Adapter-specific response and OCSF event shapes remain at the protocol boundary.
Policy authors may use `protocol: tcp` as an explicit spelling of the existing
L4 passthrough behavior. Omitting `protocol` remains equivalent. The egress
intent reserves a transparent TCP adapter and a policy-DNS-pinned address, but
DNS serving and transparent TCP capture are not active yet.

Provider credential placeholders are resolved through the live provider state
for each HTTP request, after destination and L7 policy admission. A static
Expand Down
49 changes: 47 additions & 2 deletions crates/openshell-cli/src/policy_update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -327,9 +327,14 @@ fn parse_add_endpoint_spec(spec: &str) -> Result<NetworkEndpoint> {
"--add-endpoint access segment must be one of read-only, read-write, or full; got '{access}' in '{spec}'"
));
}
if !protocol.is_empty() && !matches!(protocol, "rest" | "websocket" | "sql") {
if !protocol.is_empty() && !matches!(protocol, "tcp" | "rest" | "websocket" | "sql") {
Comment thread
johntmyers marked this conversation as resolved.
return Err(miette!(
"--add-endpoint protocol segment must be 'rest', 'websocket', or 'sql'; got '{protocol}' in '{spec}'"
"--add-endpoint protocol segment must be 'tcp', 'rest', 'websocket', or 'sql'; got '{protocol}' in '{spec}'"
));
}
if protocol == "tcp" && (!access.is_empty() || !enforcement.is_empty()) {
return Err(miette!(
"--add-endpoint protocol 'tcp' does not support access or enforcement in '{spec}'"
));
}
if !enforcement.is_empty() && !matches!(enforcement, "enforce" | "audit") {
Expand Down Expand Up @@ -538,6 +543,26 @@ mod tests {
assert_eq!(endpoint.enforcement, "enforce");
}

#[test]
fn parse_add_endpoint_accepts_explicit_tcp_protocol() {
let plan = build_policy_update_plan(
&["database.example.com:5432::tcp".to_string()],
&[],
&[],
&[],
&[],
&[],
None,
)
.expect("plan should build");

let PolicyMergeOp::AddRule { rule, .. } = &plan.preview_operations[0] else {
panic!("expected add-rule preview");
};
assert_eq!(rule.endpoints[0].protocol, "tcp");
assert!(rule.endpoints[0].access.is_empty());
}

#[test]
fn parse_add_endpoint_enables_websocket_credential_rewrite() {
let plan = build_policy_update_plan(
Expand Down Expand Up @@ -787,6 +812,26 @@ mod tests {
);
}

#[test]
fn parse_add_endpoint_rejects_l7_fields_with_tcp() {
let error = build_policy_update_plan(
&["database.example.com:5432::tcp:enforce".to_string()],
&[],
&[],
&[],
&[],
&[],
None,
)
.expect_err("TCP must reject L7 enforcement");

assert!(
error
.to_string()
.contains("does not support access or enforcement")
);
}

#[test]
fn parse_remove_endpoint_rejects_out_of_range_port() {
let error = build_policy_update_plan(
Expand Down
42 changes: 39 additions & 3 deletions crates/openshell-policy/src/ambiguity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,12 @@ fn overlapping_ports(left: &NetworkEndpoint, right: &NetworkEndpoint) -> Vec<u32

fn connection_conflicts(left: &NetworkEndpoint, right: &NetworkEndpoint) -> Vec<String> {
let mut conflicts = Vec::new();
push_conflict(
&mut conflicts,
"transparent_tcp_eligible",
&is_explicit_tcp(&left.protocol),
&is_explicit_tcp(&right.protocol),
);
push_conflict(
&mut conflicts,
"tls",
Expand All @@ -184,13 +190,17 @@ fn connection_conflicts(left: &NetworkEndpoint, right: &NetworkEndpoint) -> Vec<
conflicts
}

fn is_explicit_tcp(protocol: &str) -> bool {
protocol.eq_ignore_ascii_case("tcp")
}

/// Keep request-pipeline ambiguity checks aligned with Rego's
/// `endpoint_has_extended_config` predicate. Plain L4 endpoints authorize a
/// destination but do not participate in endpoint-config selection, so they
/// cannot compete with the single L7/connection-config endpoint selected for
/// that request.
fn endpoint_contributes_request_pipeline_metadata(endpoint: &NetworkEndpoint) -> bool {
!endpoint.protocol.is_empty()
(!endpoint.protocol.is_empty() && !endpoint.protocol.eq_ignore_ascii_case("tcp"))
|| !endpoint.allowed_ips.is_empty()
|| !endpoint.tls.is_empty()
|| endpoint.credential_binding.is_some()
Expand All @@ -201,8 +211,8 @@ fn request_pipeline_conflicts(left: &NetworkEndpoint, right: &NetworkEndpoint) -
push_conflict(
&mut conflicts,
"protocol",
&left.protocol.to_ascii_lowercase(),
&right.protocol.to_ascii_lowercase(),
&normalized_request_protocol(&left.protocol),
&normalized_request_protocol(&right.protocol),
);
push_conflict(
&mut conflicts,
Expand Down Expand Up @@ -300,6 +310,14 @@ fn request_pipeline_conflicts(left: &NetworkEndpoint, right: &NetworkEndpoint) -
conflicts
}

fn normalized_request_protocol(protocol: &str) -> String {
if protocol.eq_ignore_ascii_case("tcp") {
String::new()
} else {
protocol.to_ascii_lowercase()
}
}

fn websocket_graphql_policy(endpoint: &NetworkEndpoint) -> bool {
let allow_rule_has_graphql_fields = endpoint.rules.iter().any(|rule| {
rule.allow.as_ref().is_some_and(|allow| {
Expand Down Expand Up @@ -1027,4 +1045,22 @@ mod tests {
1
);
}

#[test]
fn explicit_tcp_and_omitted_protocol_are_ambiguous_for_native_tcp_eligibility() {
let mut explicit_tcp = endpoint("api.example.com", 443);
explicit_tcp.protocol = "tcp".to_string();
explicit_tcp.tls = "skip".to_string();
let mut omitted = endpoint("api.example.com", 443);
omitted.tls = "skip".to_string();

let ambiguities = find_endpoint_ambiguities(&policy_with(explicit_tcp, omitted));
assert_eq!(ambiguities.len(), 1);
assert!(
ambiguities[0]
.conflicts
.iter()
.any(|conflict| conflict.contains("transparent_tcp_eligible"))
);
}
}
91 changes: 87 additions & 4 deletions crates/openshell-policy/src/l7_validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,34 @@ impl L7Protocol {
}
}

/// Returns whether the authored protocol explicitly selects L4 TCP handling.
///
/// `tcp` is intentionally not an [`L7Protocol`]. It is the explicit spelling
/// of the existing L4 behavior and does not enable request inspection.
pub fn is_explicit_tcp_protocol(protocol: &str) -> bool {
protocol.eq_ignore_ascii_case("tcp")
}

/// Reject additional L7-only fields represented outside
/// [`L7EndpointFields`] by the runtime and provider-profile schemas.
///
/// Callers pass only authored fields with a non-default value. Keeping the
/// diagnostic construction here ensures both activation paths use the same
/// explicit-TCP contract.
pub fn validate_explicit_tcp_additional_fields(
protocol: &str,
present_fields: &[&str],
) -> Vec<String> {
if !is_explicit_tcp_protocol(protocol) || present_fields.is_empty() {
return Vec::new();
}

vec![format!(
"protocol tcp does not support L7-only fields: {}; remove those fields",
present_fields.join(", ")
)]
}

/// Fields extracted from an endpoint definition needed for L7 semantic
/// validation. Both profile lint and the runtime validator construct this
/// from their own data representation.
Expand Down Expand Up @@ -78,17 +106,27 @@ pub fn validate_l7_endpoint_semantics(ep: &L7EndpointFields<'_>) -> Vec<String>
let mut errors = Vec::new();
let protocol = ep.protocol;
let l7_protocol = L7Protocol::parse(protocol);
let explicit_tcp = is_explicit_tcp_protocol(protocol);
let jsonrpc_family = l7_protocol.is_some_and(L7Protocol::is_jsonrpc_family);
let is_mcp = matches!(l7_protocol, Some(L7Protocol::Mcp));
let is_jsonrpc = matches!(l7_protocol, Some(L7Protocol::JsonRpc));

// 1. Unknown protocol
if !protocol.is_empty() && l7_protocol.is_none() {
if !protocol.is_empty() && l7_protocol.is_none() && !explicit_tcp {
errors.push(format!(
"unknown protocol '{protocol}' (expected rest, websocket, graphql, sql, json-rpc, or mcp)"
"unknown protocol '{protocol}' (expected tcp, rest, websocket, graphql, sql, json-rpc, or mcp)"
));
}

// Explicit TCP is an L4 marker, not an inspection protocol. Reject L7
// policy fields instead of silently ignoring them.
if explicit_tcp && (!ep.access.is_empty() || ep.has_rules || ep.has_deny_rules) {
Comment thread
johntmyers marked this conversation as resolved.
errors.push(
"protocol tcp does not support access, rules, or deny_rules; remove those L7 fields"
.to_string(),
);
}

// 2. rules + access mutually exclusive
if ep.has_rules && !ep.access.is_empty() {
errors.push("rules and access are mutually exclusive".to_string());
Expand Down Expand Up @@ -119,7 +157,7 @@ pub fn validate_l7_endpoint_semantics(ep: &L7EndpointFields<'_>) -> Vec<String>

// 5. Non-MCP, non-JSON-RPC protocol requires rules or access (JSON-RPC's
// dedicated message is emitted by rule 4).
if !protocol.is_empty() && !is_mcp && !is_jsonrpc && !ep.has_rules && ep.access.is_empty() {
if l7_protocol.is_some() && !is_mcp && !is_jsonrpc && !ep.has_rules && ep.access.is_empty() {
errors.push("protocol requires rules or access to define allowed traffic".to_string());
}

Expand All @@ -141,7 +179,7 @@ pub fn validate_l7_endpoint_semantics(ep: &L7EndpointFields<'_>) -> Vec<String>
}

// 8. deny_rules require protocol
if ep.has_deny_rules && protocol.is_empty() {
if ep.has_deny_rules && l7_protocol.is_none() {
errors.push("deny_rules require protocol (L7 inspection must be enabled)".to_string());
}

Expand Down Expand Up @@ -379,6 +417,51 @@ mod tests {
assert!(errors.is_empty(), "expected no errors, got: {errors:?}");
}

#[test]
fn explicit_tcp_is_valid_without_l7_fields() {
let ep = L7EndpointFields {
protocol: "tcp",
access: "",
has_rules: false,
has_deny_rules: false,
rules_would_deny_all: false,
allow_all_known_mcp_methods: false,
};
let errors = validate_l7_endpoint_semantics(&ep);
assert!(errors.is_empty(), "expected no errors, got: {errors:?}");
assert!(is_explicit_tcp_protocol("TCP"));
assert_eq!(L7Protocol::parse("tcp"), None);
}

#[test]
fn explicit_tcp_rejects_l7_fields() {
let ep = L7EndpointFields {
protocol: "tcp",
access: "full",
has_rules: false,
has_deny_rules: false,
rules_would_deny_all: false,
allow_all_known_mcp_methods: false,
};
let errors = validate_l7_endpoint_semantics(&ep);
assert_eq!(
errors,
vec![
"protocol tcp does not support access, rules, or deny_rules; remove those L7 fields"
]
);
}

#[test]
fn explicit_tcp_rejects_additional_l7_fields() {
let errors =
validate_explicit_tcp_additional_fields("tcp", &["enforcement", "credential_signing"]);

assert_eq!(errors.len(), 1);
assert!(errors[0].contains("enforcement, credential_signing"));
assert!(validate_explicit_tcp_additional_fields("rest", &["enforcement"]).is_empty());
}

#[test]
fn l7_protocol_parse_known_variants() {
assert_eq!(L7Protocol::parse("rest"), Some(L7Protocol::Rest));
Expand Down
5 changes: 4 additions & 1 deletion crates/openshell-policy/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,10 @@ pub use compose::{
PROVIDER_RULE_NAME_PREFIX, ProviderPolicyLayer, compose_effective_policy,
is_provider_rule_name, provider_rule_name, strip_provider_rule_names,
};
pub use l7_validate::{L7EndpointFields, L7Protocol, validate_l7_endpoint_semantics};
pub use l7_validate::{
L7EndpointFields, L7Protocol, validate_explicit_tcp_additional_fields,
validate_l7_endpoint_semantics,
};
pub use merge::{
PolicyMergeError, PolicyMergeOp, PolicyMergeResult, PolicyMergeWarning, generated_rule_name,
merge_policy, policy_covers_rule,
Expand Down
Loading
Loading