Skip to content

Commit 2f4ffa6

Browse files
committed
fix(cli): accept allow-encoded-slash in policy update
Also merge the two protocol check helpers into one. Signed-off-by: Eric Curtin <eric.curtin@docker.com>
1 parent e5adf6f commit 2f4ffa6

3 files changed

Lines changed: 63 additions & 25 deletions

File tree

‎crates/openshell-cli/src/main.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2191,8 +2191,8 @@ enum PolicyCommands {
21912191
name: Option<String>,
21922192

21932193
/// Add or merge an endpoint: host:port[:access[:protocol[:enforcement[:options]]]].
2194-
/// Options include allowed-ip=..., credential rewrite flags, and
2195-
/// allow-uninspected-credentials.
2194+
/// Options include allowed-ip=..., allow-encoded-slash, credential
2195+
/// rewrite flags, and allow-uninspected-credentials.
21962196
#[arg(long = "add-endpoint")]
21972197
add_endpoints: Vec<String>,
21982198

‎crates/openshell-cli/src/policy_update.rs‎

Lines changed: 57 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -268,37 +268,28 @@ fn l7_target_to_proto(target: &L7RuleTarget) -> ProtoL7RuleTarget {
268268
}
269269
}
270270

271-
fn ensure_websocket_credential_rewrite_protocol(
271+
/// Require the endpoint protocol segment to be one of `allowed` for `option`.
272+
fn ensure_option_protocol(
272273
spec: &str,
273274
endpoint: &NetworkEndpoint,
275+
option: &str,
276+
allowed: &[&str],
274277
) -> Result<()> {
275-
if matches!(endpoint.protocol.as_str(), "rest" | "websocket") {
278+
if allowed.contains(&endpoint.protocol.as_str()) {
276279
return Ok(());
277280
}
278281
let protocol = if endpoint.protocol.is_empty() {
279282
"<empty>"
280283
} else {
281284
endpoint.protocol.as_str()
282285
};
286+
let allowed = allowed
287+
.iter()
288+
.map(|protocol| format!("'{protocol}'"))
289+
.collect::<Vec<_>>()
290+
.join(" or ");
283291
Err(miette!(
284-
"websocket-credential-rewrite endpoint option requires --add-endpoint protocol segment to be 'rest' or 'websocket'; got '{protocol}' in '{spec}'"
285-
))
286-
}
287-
288-
fn ensure_request_body_credential_rewrite_protocol(
289-
spec: &str,
290-
endpoint: &NetworkEndpoint,
291-
) -> Result<()> {
292-
if endpoint.protocol == "rest" {
293-
return Ok(());
294-
}
295-
let protocol = if endpoint.protocol.is_empty() {
296-
"<empty>"
297-
} else {
298-
endpoint.protocol.as_str()
299-
};
300-
Err(miette!(
301-
"request-body-credential-rewrite endpoint option requires --add-endpoint protocol segment to be 'rest'; got '{protocol}' in '{spec}'"
292+
"{option} endpoint option requires --add-endpoint protocol segment to be {allowed}; got '{protocol}' in '{spec}'"
302293
))
303294
}
304295

@@ -504,12 +495,16 @@ fn apply_add_endpoint_options(
504495
"allow-uninspected-credentials" => {
505496
endpoint.allow_uninspected_credentials = true;
506497
}
498+
"allow-encoded-slash" => {
499+
ensure_option_protocol(spec, endpoint, option, &["rest", "websocket"])?;
500+
endpoint.allow_encoded_slash = true;
501+
}
507502
"websocket-credential-rewrite" => {
508-
ensure_websocket_credential_rewrite_protocol(spec, endpoint)?;
503+
ensure_option_protocol(spec, endpoint, option, &["rest", "websocket"])?;
509504
endpoint.websocket_credential_rewrite = true;
510505
}
511506
"request-body-credential-rewrite" => {
512-
ensure_request_body_credential_rewrite_protocol(spec, endpoint)?;
507+
ensure_option_protocol(spec, endpoint, option, &["rest"])?;
513508
endpoint.request_body_credential_rewrite = true;
514509
}
515510
_ if option.starts_with(ALLOWED_IP_OPTION_PREFIX) => {
@@ -521,7 +516,7 @@ fn apply_add_endpoint_options(
521516
}
522517
_ => {
523518
return Err(miette!(
524-
"--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}'"
519+
"--add-endpoint options segment supports only 'allow-uninspected-credentials', 'allow-encoded-slash', 'websocket-credential-rewrite', 'request-body-credential-rewrite', and 'allowed-ip=<CIDR-or-IP>'; got '{option}' in '{spec}'"
525520
));
526521
}
527522
}
@@ -831,6 +826,45 @@ mod tests {
831826
assert!(endpoint.request_body_credential_rewrite);
832827
}
833828

829+
#[test]
830+
fn parse_add_endpoint_enables_allow_encoded_slash_on_rest_and_websocket() {
831+
for protocol in ["rest", "websocket"] {
832+
let plan = build_policy_update_plan(
833+
&[format!(
834+
"registry.npmjs.org:443:full:{protocol}:enforce:allow-encoded-slash"
835+
)],
836+
&[],
837+
&[],
838+
&[],
839+
&[],
840+
&[],
841+
None,
842+
)
843+
.expect("plan should build");
844+
845+
let PolicyMergeOp::AddRule { rule, .. } = &plan.preview_operations[0] else {
846+
panic!("expected add-rule preview");
847+
};
848+
assert!(rule.endpoints[0].allow_encoded_slash, "{protocol}");
849+
}
850+
}
851+
852+
#[test]
853+
fn allow_encoded_slash_rejects_endpoint_without_http_protocol() {
854+
for spec in [
855+
"registry.npmjs.org:443::::allow-encoded-slash",
856+
"registry.npmjs.org:443::tcp::allow-encoded-slash",
857+
] {
858+
let error =
859+
build_policy_update_plan(&[spec.to_string()], &[], &[], &[], &[], &[], None)
860+
.expect_err("plan should fail");
861+
assert!(
862+
error.to_string().contains("to be 'rest' or 'websocket'"),
863+
"{error}"
864+
);
865+
}
866+
}
867+
834868
#[test]
835869
fn parse_add_endpoint_enables_allow_uninspected_credentials() {
836870
let plan = build_policy_update_plan(

‎docs/how-it-works/policies/network-rules.mdx‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -340,6 +340,10 @@ request, which the read-only preset denies. Run `npm install --no-audit`, or
340340
replace `access: read-only` with explicit rules that also allow the audit's
341341
`POST` requests. An endpoint cannot combine `access` and `rules`.
342342

343+
To set the option on a live sandbox without replacing the policy, add
344+
`allow-encoded-slash` to the `--add-endpoint` options segment of
345+
`openshell policy update`, which requires a `rest` or `websocket` endpoint.
346+
343347
Verify that a scoped package lookup succeeds:
344348

345349
```shell

0 commit comments

Comments
 (0)