Skip to content

Commit 9bb944c

Browse files
committed
chore(stack): merge lifecycle main sync
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
2 parents e0ea8f4 + 78e352e commit 9bb944c

34 files changed

Lines changed: 3227 additions & 159 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: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,70 @@ 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 the attached providers
104+
and stamps it onto the effective policy at composition time. This provenance is
105+
internal, contains no credential identifiers or values, and is never trusted
106+
from user-authored policy.
107+
108+
Every evaluation clears provenance across the whole policy and re-derives it
109+
from two sources:
110+
111+
- the endpoints of attached provider profiles that carry credentials, and
112+
- the valid `credential_binding` entries of the sandbox policy that name an
113+
attached provider whose profile is endpointless.
114+
115+
A binding reduces to a host and port scope only. Dropping the path is
116+
deliberate: a path is not observable on an L4 or `tls: skip` endpoint, so a
117+
path-scoped derivation would omit the marker on exactly the surfaces the
118+
uninspected-credential gate exists to catch. A malformed binding — empty
119+
provider, missing host, or a port outside `1..=65535` — fails the evaluation
120+
instead of contributing a scope. Bindings naming an endpointful profile or an
121+
unattached provider contribute nothing; the gateway rejects those uses
122+
separately.
123+
124+
Both sources merge into one deduplicated scope set, and each endpoint is
125+
stamped once per evaluation from that set, so binding-derived scopes reach the
126+
same gates as profile-derived ones. The stamp is an assignment, not an
127+
accumulation, so an endpoint that stops matching a credentialed scope — or
128+
whose binding was removed — loses its marker in the same pass. This must remain
129+
a full recomputation: a delta-based derivation would let a series of
130+
individually valid edits reach a state no single edit would have admitted.
131+
132+
Credentialed L4-only and `tls: skip` endpoints fail policy validation unless the
133+
public `allow_uninspected_credentials` escape hatch is explicitly enabled. The
134+
flag defaults to `false` and is security-flagged in policy approval flows.
135+
Incremental merges only ever add the flag to a matching endpoint; clearing it
136+
requires removing the endpoint or replacing the policy.
137+
138+
The network supervisor independently enforces the same boundary. Credentialed
139+
WebSocket upgrades use the parsed relay, binary frames fail closed, and text
140+
placeholders require rewrite. REST bodies can continue streaming when body
141+
rewrite is disabled, but the relay withholds enough trailing bytes to detect a
142+
placeholder split across reads before forwarding its marker. Explicitly opted-in
143+
endpoints retain raw passthrough behavior.
144+
145+
Denials emit both the relevant network activity and a detection finding. Events
146+
identify only the destination, policy, and traffic surface; they never include
147+
credential names, placeholders, body content, or secret values.
148+
149+
Credential provenance is gateway-derived and deliberately absent from the policy
150+
YAML schema, so it does not survive a policy that never transits the gateway.
151+
Gateway-delivered policy is the authoritative source for this control, and a
152+
policy without provenance applies neither the raw-tunnel refusal nor the
153+
WebSocket binary-frame refusal. The request-body backstop still applies, because
154+
it keys off the presence of a secret resolver rather than endpoint provenance.
155+
156+
Two paths load a policy without provenance. A supervisor booting from a
157+
container-image policy is a bounded window: that policy is resynchronized to the
158+
gateway, which then serves a stamped effective policy. An explicit local Rego and
159+
data override is permanent, because gateway revisions are observed for settings
160+
and providers but never replace the local policy. When that override is combined
161+
with injected provider credentials, the supervisor emits a high-severity
162+
detection finding at startup naming the inactive controls.
163+
100164
## Live Updates
101165

102166
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: 73 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -351,6 +351,8 @@ fn parse_add_endpoint_spec(spec: &str) -> Result<NetworkEndpoint> {
351351
Ok(endpoint)
352352
}
353353

354+
const ALLOWED_IP_OPTION_PREFIX: &str = "allowed-ip=";
355+
354356
fn apply_add_endpoint_options(
355357
spec: &str,
356358
endpoint: &mut NetworkEndpoint,
@@ -368,6 +370,9 @@ fn apply_add_endpoint_options(
368370
));
369371
}
370372
match option {
373+
"allow-uninspected-credentials" => {
374+
endpoint.allow_uninspected_credentials = true;
375+
}
371376
"websocket-credential-rewrite" => {
372377
ensure_websocket_credential_rewrite_protocol(spec, endpoint)?;
373378
endpoint.websocket_credential_rewrite = true;
@@ -376,37 +381,40 @@ fn apply_add_endpoint_options(
376381
ensure_request_body_credential_rewrite_protocol(spec, endpoint)?;
377382
endpoint.request_body_credential_rewrite = true;
378383
}
379-
_ => {
380-
let Some(allowed_ip) = option.strip_prefix("allowed-ip=") else {
381-
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}'"
383-
));
384-
};
385-
let allowed_ip = allowed_ip.trim();
386-
if allowed_ip.is_empty() {
387-
return Err(miette!(
388-
"--add-endpoint allowed-ip option must include a CIDR or IP value in '{spec}'"
389-
));
390-
}
391-
if allowed_ip.contains(char::is_whitespace) {
392-
return Err(miette!(
393-
"--add-endpoint allowed-ip option must not contain whitespace in '{spec}'"
394-
));
395-
}
396-
if !endpoint
397-
.allowed_ips
398-
.iter()
399-
.any(|existing| existing == allowed_ip)
400-
{
401-
endpoint.allowed_ips.push(allowed_ip.to_string());
384+
_ if option.starts_with(ALLOWED_IP_OPTION_PREFIX) => {
385+
let allowed_ip =
386+
parse_allowed_ip_value(spec, &option[ALLOWED_IP_OPTION_PREFIX.len()..])?;
387+
if !endpoint.allowed_ips.contains(&allowed_ip) {
388+
endpoint.allowed_ips.push(allowed_ip);
402389
}
403390
}
391+
_ => {
392+
return Err(miette!(
393+
"--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}'"
394+
));
395+
}
404396
}
405397
}
406398

407399
Ok(())
408400
}
409401

402+
/// Validate the value part of an `allowed-ip=<CIDR-or-IP>` endpoint option.
403+
fn parse_allowed_ip_value(spec: &str, value: &str) -> Result<String> {
404+
let allowed_ip = value.trim();
405+
if allowed_ip.is_empty() {
406+
return Err(miette!(
407+
"--add-endpoint allowed-ip option must include a CIDR or IP value in '{spec}'"
408+
));
409+
}
410+
if allowed_ip.contains(char::is_whitespace) {
411+
return Err(miette!(
412+
"--add-endpoint allowed-ip option must not contain whitespace in '{spec}'"
413+
));
414+
}
415+
Ok(allowed_ip.to_string())
416+
}
417+
410418
fn parse_host(flag: &str, spec: &str, host: &str) -> Result<String> {
411419
let host = host.trim();
412420
if host.is_empty() {
@@ -454,6 +462,7 @@ fn dedup_strings(values: &[String]) -> Vec<String> {
454462
mod tests {
455463
use super::{
456464
PolicyUpdatePlan, build_policy_update_plan as build_policy_update_plan_with_options,
465+
parse_allowed_ip_value,
457466
};
458467
use openshell_policy::PolicyMergeOp;
459468

@@ -604,6 +613,25 @@ mod tests {
604613
assert!(endpoint.request_body_credential_rewrite);
605614
}
606615

616+
#[test]
617+
fn parse_add_endpoint_enables_allow_uninspected_credentials() {
618+
let plan = build_policy_update_plan(
619+
&["api.vendor.example:443::::allow-uninspected-credentials".to_string()],
620+
&[],
621+
&[],
622+
&[],
623+
&[],
624+
&[],
625+
None,
626+
)
627+
.expect("plan should build");
628+
629+
let PolicyMergeOp::AddRule { rule, .. } = &plan.preview_operations[0] else {
630+
panic!("expected add-rule preview");
631+
};
632+
assert!(rule.endpoints[0].allow_uninspected_credentials);
633+
}
634+
607635
#[test]
608636
fn parse_add_endpoint_merges_allowed_ips_with_websocket_options() {
609637
let plan = build_policy_update_plan(
@@ -665,6 +693,28 @@ mod tests {
665693
assert!(error.to_string().contains("allowed-ip option"));
666694
}
667695

696+
#[test]
697+
fn parse_allowed_ip_value_accepts_trimmed_cidr_and_ip() {
698+
assert_eq!(
699+
parse_allowed_ip_value("spec", "10.0.0.0/8").expect("CIDR should parse"),
700+
"10.0.0.0/8"
701+
);
702+
assert_eq!(
703+
parse_allowed_ip_value("spec", " 192.168.1.10 ").expect("IP should parse"),
704+
"192.168.1.10"
705+
);
706+
}
707+
708+
#[test]
709+
fn parse_allowed_ip_value_rejects_empty_and_interior_whitespace() {
710+
let empty = parse_allowed_ip_value("spec", " ").expect_err("empty value must fail");
711+
assert!(empty.to_string().contains("must include a CIDR or IP"));
712+
713+
let spaced =
714+
parse_allowed_ip_value("spec", "10.0.0.0/8 172.16.0.0/12").expect_err("must fail");
715+
assert!(spaced.to_string().contains("must not contain whitespace"));
716+
}
717+
668718
#[test]
669719
fn websocket_credential_rewrite_rejects_l4_endpoint() {
670720
let error = 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
// ---------------------------------------------------------------------------
@@ -1373,6 +1400,45 @@ mod tests {
13731400

13741401
// === Existing tests (preserved) ===
13751402

1403+
#[test]
1404+
fn byte_marker_detection_handles_raw_encoded_and_binary_input() {
1405+
assert!(contains_reserved_credential_marker_bytes(
1406+
b"openshell:resolve:env:API_TOKEN"
1407+
));
1408+
assert!(contains_reserved_credential_marker_bytes(
1409+
b"openshell%3Aresolve%3Aenv%3AAPI_TOKEN"
1410+
));
1411+
assert!(!contains_reserved_credential_marker_bytes(&[
1412+
0xff, 0x00, 0x01, 0x02
1413+
]));
1414+
}
1415+
1416+
fn fully_percent_encoded(marker: &str) -> String {
1417+
const HEX: &[u8; 16] = b"0123456789ABCDEF";
1418+
let mut encoded = String::with_capacity(marker.len() * 3);
1419+
for byte in marker.bytes() {
1420+
encoded.push('%');
1421+
encoded.push(char::from(HEX[usize::from(byte >> 4)]));
1422+
encoded.push(char::from(HEX[usize::from(byte & 0x0f)]));
1423+
}
1424+
encoded
1425+
}
1426+
1427+
#[test]
1428+
fn scan_tail_window_covers_longest_encoded_marker_form() {
1429+
for marker in [PLACEHOLDER_PREFIX, PROVIDER_ALIAS_MARKER] {
1430+
let encoded = fully_percent_encoded(marker);
1431+
assert!(
1432+
contains_reserved_credential_marker(&encoded),
1433+
"fully encoded {marker} must be detected"
1434+
);
1435+
assert!(
1436+
CREDENTIAL_MARKER_SCAN_TAIL_BYTES >= encoded.len() - 1,
1437+
"scan window must retain every byte of {encoded} but the last"
1438+
);
1439+
}
1440+
}
1441+
13761442
#[test]
13771443
fn provider_env_is_replaced_with_placeholders() {
13781444
let (child_env, resolver) = SecretResolver::from_provider_env(

0 commit comments

Comments
 (0)