Skip to content

Commit 173a0bc

Browse files
committed
fix(policy): gate uninspected credentialed endpoints
Signed-off-by: Adrien Langou <alangou@nvidia.com>
1 parent 877ddba commit 173a0bc

34 files changed

Lines changed: 2635 additions & 76 deletions

File tree

.agents/skills/generate-sandbox-policy/SKILL.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -384,6 +384,7 @@ Before presenting the policy to the user, verify correctness **and** flag breadt
384384
- [ ] Middleware `order` values are unique and no selected chain exceeds 10 stages
385385
- [ ] No fail-closed middleware selector can cover a `tls: skip` endpoint
386386
- [ ] Any required WebSocket control advertises `WEBSOCKET_MESSAGE/PRE_CREDENTIALS`, and the user understands that V1 does not inspect binary messages
387+
- [ ] Endpoints contributed by a credentialed provider are not L4-only or `tls: skip` unless `allow_uninspected_credentials: true` explicitly records the exception
387388

388389
### Schema Warnings (log-only, but should be fixed)
389390

@@ -418,6 +419,7 @@ Evaluate the generated policy for overly broad access and **include warnings in
418419
| **Broad CIDR** in `allowed_ips` (e.g., `10.0.0.0/8`) | "This `allowed_ips` entry covers a very broad range. Consider narrowing to a specific subnet (e.g., `10.0.5.0/24`) to minimize exposure." |
419420
| **`on_error: fail_open`** | "This middleware can be bypassed when it is unavailable, rejects configuration, returns an invalid result, or exceeds its body limit. Use `fail_closed` unless availability is more important than this control." |
420421
| **Broad middleware host selector** | "This middleware attaches independently of the admitting network rule to every matching destination, then runs only for operation bindings its implementation advertises. Narrow `endpoints.include` or add exclusions if the attachment is not required for every matching host." |
422+
| **`allow_uninspected_credentials: true`** | "This endpoint may carry provider credentials on traffic OpenShell cannot inspect or rewrite. Prefer an inspected protocol and credential rewrite; keep this exception only when raw traffic is required." |
421423

422424
Format breadth warnings clearly in the output, e.g.:
423425

.agents/skills/openshell-cli/cli-reference.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -384,6 +384,7 @@ Incrementally merge live network policy changes into the current sandbox policy.
384384
Notes:
385385

386386
- The sandbox name defaults to the last-used sandbox.
387+
- `--add-endpoint` options are comma-separated: `allowed-ip=<CIDR-or-IP>`, `websocket-credential-rewrite`, `request-body-credential-rewrite`, and `allow-uninspected-credentials`. The last option is a security-sensitive exception for provider-credentialed L4-only, `tls: skip`, or otherwise uninspectable traffic.
387388
- `--add-allow` and `--add-deny` operate on REST and WebSocket endpoints. Use full YAML for JSON-RPC, MCP, SQL, or other policy structure.
388389
- `--wait` cannot be combined with `--dry-run`.
389390
- Use `policy set` when replacing the full policy or changing static sections.

architecture/security-policy.md

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,53 @@ raw relay by default. A `protocol: rest` endpoint can opt in to
9797
after an allowed `101` upgrade; server-to-client traffic and all other upgraded
9898
protocols remain raw passthrough.
9999

100+
## Credentialed Endpoints
101+
102+
OpenShell keeps provider credentials on paths it can inspect or rewrite by
103+
default. The gateway derives credential provenance from attached provider
104+
profiles and stamps it onto the effective policy at composition time. This
105+
provenance is internal, contains no credential identifiers or values, and is
106+
never trusted from user-authored policy.
107+
108+
Every evaluation clears provenance across the whole policy and re-derives it
109+
from the full set of attached provider profiles. The stamp is an assignment,
110+
not an accumulation, so an endpoint that stops matching a credentialed scope
111+
loses its marker in the same pass. This must remain a full recomputation: a
112+
delta-based derivation would let a series of individually valid edits reach a
113+
state no single edit would have admitted.
114+
115+
Credentialed L4-only and `tls: skip` endpoints fail policy validation unless the
116+
public `allow_uninspected_credentials` escape hatch is explicitly enabled. The
117+
flag defaults to `false` and is security-flagged in policy approval flows.
118+
Incremental merges only ever add the flag to a matching endpoint; clearing it
119+
requires removing the endpoint or replacing the policy.
120+
121+
The network supervisor independently enforces the same boundary. Credentialed
122+
WebSocket upgrades use the parsed relay, binary frames fail closed, and text
123+
placeholders require rewrite. REST bodies can continue streaming when body
124+
rewrite is disabled, but the relay withholds enough trailing bytes to detect a
125+
placeholder split across reads before forwarding its marker. Explicitly opted-in
126+
endpoints retain raw passthrough behavior.
127+
128+
Denials emit both the relevant network activity and a detection finding. Events
129+
identify only the destination, policy, and traffic surface; they never include
130+
credential names, placeholders, body content, or secret values.
131+
132+
Credential provenance is gateway-derived and deliberately absent from the policy
133+
YAML schema, so it does not survive a policy that never transits the gateway.
134+
Gateway-delivered policy is the authoritative source for this control, and a
135+
policy without provenance applies neither the raw-tunnel refusal nor the
136+
WebSocket binary-frame refusal. The request-body backstop still applies, because
137+
it keys off the presence of a secret resolver rather than endpoint provenance.
138+
139+
Two paths load a policy without provenance. A supervisor booting from a
140+
container-image policy is a bounded window: that policy is resynchronized to the
141+
gateway, which then serves a stamped effective policy. An explicit local Rego and
142+
data override is permanent, because gateway revisions are observed for settings
143+
and providers but never replace the local policy. When that override is combined
144+
with injected provider credentials, the supervisor emits a high-severity
145+
detection finding at startup naming the inactive controls.
146+
100147
## Live Updates
101148

102149
The gateway stores sandbox-authored policy revisions separately from derived

crates/openshell-cli/src/main.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1791,6 +1791,8 @@ enum PolicyCommands {
17911791
name: Option<String>,
17921792

17931793
/// Add or merge an endpoint: host:port[:access[:protocol[:enforcement[:options]]]].
1794+
/// Options include allowed-ip=..., credential rewrite flags, and
1795+
/// allow-uninspected-credentials.
17941796
#[arg(long = "add-endpoint")]
17951797
add_endpoints: Vec<String>,
17961798

crates/openshell-cli/src/policy_update.rs

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -368,6 +368,9 @@ fn apply_add_endpoint_options(
368368
));
369369
}
370370
match option {
371+
"allow-uninspected-credentials" => {
372+
endpoint.allow_uninspected_credentials = true;
373+
}
371374
"websocket-credential-rewrite" => {
372375
ensure_websocket_credential_rewrite_protocol(spec, endpoint)?;
373376
endpoint.websocket_credential_rewrite = true;
@@ -379,7 +382,7 @@ fn apply_add_endpoint_options(
379382
_ => {
380383
let Some(allowed_ip) = option.strip_prefix("allowed-ip=") else {
381384
return Err(miette!(
382-
"--add-endpoint options segment supports only 'websocket-credential-rewrite', 'request-body-credential-rewrite', and 'allowed-ip=<CIDR-or-IP>'; got '{option}' in '{spec}'"
385+
"--add-endpoint options segment supports only 'allow-uninspected-credentials', 'websocket-credential-rewrite', 'request-body-credential-rewrite', and 'allowed-ip=<CIDR-or-IP>'; got '{option}' in '{spec}'"
383386
));
384387
};
385388
let allowed_ip = allowed_ip.trim();
@@ -604,6 +607,25 @@ mod tests {
604607
assert!(endpoint.request_body_credential_rewrite);
605608
}
606609

610+
#[test]
611+
fn parse_add_endpoint_enables_allow_uninspected_credentials() {
612+
let plan = build_policy_update_plan(
613+
&["api.vendor.example:443::::allow-uninspected-credentials".to_string()],
614+
&[],
615+
&[],
616+
&[],
617+
&[],
618+
&[],
619+
None,
620+
)
621+
.expect("plan should build");
622+
623+
let PolicyMergeOp::AddRule { rule, .. } = &plan.preview_operations[0] else {
624+
panic!("expected add-rule preview");
625+
};
626+
assert!(rule.endpoints[0].allow_uninspected_credentials);
627+
}
628+
607629
#[test]
608630
fn parse_add_endpoint_merges_allowed_ips_with_websocket_options() {
609631
let plan = build_policy_update_plan(

crates/openshell-core/src/secrets.rs

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,24 @@ const PROVIDER_ALIAS_MARKER: &str = "OPENSHELL-RESOLVE-ENV-";
1313
/// Public access to the placeholder prefix for fail-closed scanning in other modules.
1414
pub const PLACEHOLDER_PREFIX_PUBLIC: &str = PLACEHOLDER_PREFIX;
1515
pub const PROVIDER_ALIAS_MARKER_PUBLIC: &str = PROVIDER_ALIAS_MARKER;
16+
/// Longest wire form of a reserved marker: percent-encoding expands every
17+
/// marker byte to three bytes (`%XX`), and detection decodes in a single pass.
18+
const LONGEST_RESERVED_MARKER_WIRE_BYTES: usize =
19+
3 * if PLACEHOLDER_PREFIX.len() > PROVIDER_ALIAS_MARKER.len() {
20+
PLACEHOLDER_PREFIX.len()
21+
} else {
22+
PROVIDER_ALIAS_MARKER.len()
23+
};
24+
25+
/// Retain this many trailing bytes when scanning a streamed request body so a
26+
/// reserved marker split across reads cannot be forwarded before detection.
27+
///
28+
/// A marker is only detected while all of its wire bytes sit in the scan buffer
29+
/// at once, so the retained window must hold every byte of the longest form but
30+
/// the last. A window shorter than that lets a caller split a fully
31+
/// percent-encoded marker so its leading bytes are forwarded before the rest
32+
/// arrives, and the reassembled remainder no longer decodes to the marker.
33+
pub const CREDENTIAL_MARKER_SCAN_TAIL_BYTES: usize = LONGEST_RESERVED_MARKER_WIRE_BYTES;
1634

1735
/// Characters that are valid in an env var key name (used to extract
1836
/// placeholder boundaries within concatenated strings like path segments).
@@ -36,6 +54,15 @@ pub fn contains_reserved_credential_marker(value: &str) -> bool {
3654
contains_raw_reserved_marker(&decoded)
3755
}
3856

57+
pub fn contains_reserved_credential_marker_bytes(value: &[u8]) -> bool {
58+
if value.is_empty() {
59+
return false;
60+
}
61+
String::from_utf8_lossy(value)
62+
.split('\0')
63+
.any(contains_reserved_credential_marker)
64+
}
65+
3966
// ---------------------------------------------------------------------------
4067
// Error and result types
4168
// ---------------------------------------------------------------------------
@@ -1304,6 +1331,45 @@ mod tests {
13041331

13051332
// === Existing tests (preserved) ===
13061333

1334+
#[test]
1335+
fn byte_marker_detection_handles_raw_encoded_and_binary_input() {
1336+
assert!(contains_reserved_credential_marker_bytes(
1337+
b"openshell:resolve:env:API_TOKEN"
1338+
));
1339+
assert!(contains_reserved_credential_marker_bytes(
1340+
b"openshell%3Aresolve%3Aenv%3AAPI_TOKEN"
1341+
));
1342+
assert!(!contains_reserved_credential_marker_bytes(&[
1343+
0xff, 0x00, 0x01, 0x02
1344+
]));
1345+
}
1346+
1347+
fn fully_percent_encoded(marker: &str) -> String {
1348+
const HEX: &[u8; 16] = b"0123456789ABCDEF";
1349+
let mut encoded = String::with_capacity(marker.len() * 3);
1350+
for byte in marker.bytes() {
1351+
encoded.push('%');
1352+
encoded.push(char::from(HEX[usize::from(byte >> 4)]));
1353+
encoded.push(char::from(HEX[usize::from(byte & 0x0f)]));
1354+
}
1355+
encoded
1356+
}
1357+
1358+
#[test]
1359+
fn scan_tail_window_covers_longest_encoded_marker_form() {
1360+
for marker in [PLACEHOLDER_PREFIX, PROVIDER_ALIAS_MARKER] {
1361+
let encoded = fully_percent_encoded(marker);
1362+
assert!(
1363+
contains_reserved_credential_marker(&encoded),
1364+
"fully encoded {marker} must be detected"
1365+
);
1366+
assert!(
1367+
CREDENTIAL_MARKER_SCAN_TAIL_BYTES >= encoded.len() - 1,
1368+
"scan window must retain every byte of {encoded} but the last"
1369+
);
1370+
}
1371+
}
1372+
13071373
#[test]
13081374
fn provider_env_is_replaced_with_placeholders() {
13091375
let (child_env, resolver) = SecretResolver::from_provider_env(

crates/openshell-policy/src/lib.rs

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,10 @@ struct NetworkPolicyRuleDef {
103103

104104
#[derive(Debug, Serialize, Deserialize)]
105105
#[serde(deny_unknown_fields)]
106+
#[allow(
107+
clippy::struct_excessive_bools,
108+
reason = "Endpoint DTO mirrors independent policy schema toggles."
109+
)]
106110
struct NetworkEndpointDef {
107111
#[serde(default, skip_serializing_if = "String::is_empty")]
108112
host: String,
@@ -144,6 +148,10 @@ struct NetworkEndpointDef {
144148
/// placeholders before forwarding upstream. Defaults to false.
145149
#[serde(default, skip_serializing_if = "std::ops::Not::not")]
146150
request_body_credential_rewrite: bool,
151+
/// Explicitly permits credentials on traffic paths that `OpenShell` cannot
152+
/// inspect or rewrite. Defaults to false.
153+
#[serde(default, skip_serializing_if = "std::ops::Not::not")]
154+
allow_uninspected_credentials: bool,
147155
#[serde(default, skip_serializing_if = "String::is_empty")]
148156
persisted_queries: String,
149157
#[serde(default, skip_serializing_if = "BTreeMap::is_empty")]
@@ -745,6 +753,10 @@ fn to_proto(raw: PolicyFile) -> Result<SandboxPolicy> {
745753
allow_encoded_slash: e.allow_encoded_slash,
746754
websocket_credential_rewrite: e.websocket_credential_rewrite,
747755
request_body_credential_rewrite: e.request_body_credential_rewrite,
756+
allow_uninspected_credentials: e.allow_uninspected_credentials,
757+
// Provider credential provenance is derived by the
758+
// gateway and cannot be authored in policy YAML.
759+
provider_credentialed: false,
748760
// Advisor provenance is internal runtime state, not
749761
// a user-authored policy schema field.
750762
advisor_proposed: false,
@@ -901,6 +913,7 @@ fn from_proto(policy: &SandboxPolicy) -> PolicyFile {
901913
allow_encoded_slash: e.allow_encoded_slash,
902914
websocket_credential_rewrite: e.websocket_credential_rewrite,
903915
request_body_credential_rewrite: e.request_body_credential_rewrite,
916+
allow_uninspected_credentials: e.allow_uninspected_credentials,
904917
persisted_queries: e.persisted_queries.clone(),
905918
graphql_persisted_queries: e
906919
.graphql_persisted_queries
@@ -3486,6 +3499,32 @@ network_policies:
34863499
assert!(yaml_out.contains("request_body_credential_rewrite: true"));
34873500
}
34883501

3502+
#[test]
3503+
fn round_trip_preserves_allow_uninspected_credentials() {
3504+
let yaml = r"
3505+
version: 1
3506+
network_policies:
3507+
vendor_api:
3508+
endpoints:
3509+
- host: api.vendor.example
3510+
port: 443
3511+
tls: skip
3512+
allow_uninspected_credentials: true
3513+
";
3514+
let proto1 = parse_sandbox_policy(yaml).expect("parse failed");
3515+
let yaml_out = serialize_sandbox_policy(&proto1).expect("serialize failed");
3516+
let proto2 = parse_sandbox_policy(&yaml_out).expect("re-parse failed");
3517+
3518+
let ep = &proto2.network_policies["vendor_api"].endpoints[0];
3519+
assert!(ep.allow_uninspected_credentials);
3520+
assert!(
3521+
!ep.provider_credentialed,
3522+
"provider provenance must not be authorable from policy YAML"
3523+
);
3524+
assert!(yaml_out.contains("allow_uninspected_credentials: true"));
3525+
assert!(!yaml_out.contains("provider_credentialed"));
3526+
}
3527+
34893528
#[test]
34903529
fn websocket_credential_rewrite_defaults_false() {
34913530
let yaml = r"
@@ -3504,6 +3543,8 @@ network_policies:
35043543
let ep = &proto.network_policies["gateway"].endpoints[0];
35053544
assert!(!ep.websocket_credential_rewrite);
35063545
assert!(!ep.request_body_credential_rewrite);
3546+
assert!(!ep.allow_uninspected_credentials);
3547+
assert!(!ep.provider_credentialed);
35073548
}
35083549

35093550
#[test]

crates/openshell-policy/src/merge.rs

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1282,6 +1282,7 @@ fn merge_endpoint(
12821282
existing.allow_encoded_slash |= incoming.allow_encoded_slash;
12831283
existing.websocket_credential_rewrite |= incoming.websocket_credential_rewrite;
12841284
existing.request_body_credential_rewrite |= incoming.request_body_credential_rewrite;
1285+
existing.allow_uninspected_credentials |= incoming.allow_uninspected_credentials;
12851286
existing.advisor_proposed |= incoming.advisor_proposed;
12861287
normalize_endpoint(existing);
12871288
Ok(())
@@ -3100,6 +3101,48 @@ mod tests {
31003101
assert!(endpoint.request_body_credential_rewrite);
31013102
}
31023103

3104+
#[test]
3105+
fn add_rule_merges_allow_uninspected_credentials_flag() {
3106+
let mut policy = restrictive_default_policy();
3107+
policy.network_policies.insert(
3108+
"existing".to_string(),
3109+
NetworkPolicyRule {
3110+
name: "existing".to_string(),
3111+
endpoints: vec![NetworkEndpoint {
3112+
host: "api.vendor.example".to_string(),
3113+
port: 443,
3114+
ports: vec![443],
3115+
..Default::default()
3116+
}],
3117+
..Default::default()
3118+
},
3119+
);
3120+
3121+
let incoming = NetworkPolicyRule {
3122+
name: "incoming".to_string(),
3123+
endpoints: vec![NetworkEndpoint {
3124+
host: "api.vendor.example".to_string(),
3125+
port: 443,
3126+
ports: vec![443],
3127+
allow_uninspected_credentials: true,
3128+
..Default::default()
3129+
}],
3130+
..Default::default()
3131+
};
3132+
3133+
let result = merge_policy(
3134+
policy,
3135+
&[PolicyMergeOp::AddRule {
3136+
rule_name: "allow_api_vendor_example_443".to_string(),
3137+
rule: incoming,
3138+
}],
3139+
)
3140+
.expect("merge should succeed");
3141+
3142+
let endpoint = &result.policy.network_policies["existing"].endpoints[0];
3143+
assert!(endpoint.allow_uninspected_credentials);
3144+
}
3145+
31033146
#[test]
31043147
fn add_allow_expands_access_preset() {
31053148
let mut policy = restrictive_default_policy();

0 commit comments

Comments
 (0)