Skip to content

Commit 08fbc1b

Browse files
authored
fix(policy): name the field in policy type errors (#4174)
* fix(policy): name the field in policy type errors Signed-off-by: Eric Curtin <eric.curtin@docker.com> * fix(policy): refresh example lockfiles Signed-off-by: Eric Curtin <eric.curtin@docker.com> --------- Signed-off-by: Eric Curtin <eric.curtin@docker.com>
1 parent 8d87d88 commit 08fbc1b

6 files changed

Lines changed: 82 additions & 5 deletions

File tree

‎Cargo.lock‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Cargo.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ socket2 = "0.6"
8585
# Serialization
8686
serde = { version = "1", features = ["derive"] }
8787
serde_json = "1"
88+
serde_path_to_error = "0.1"
8889
serde_yml = { package = "noyalib", version = "0.0.28", default-features = false, features = ["std", "compat-serde-yaml"] }
8990
toml = "0.8"
9091
apollo-parser = "0.8.5"

‎crates/openshell-policy-schema/Cargo.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ repository.workspace = true
1414
miette = { workspace = true }
1515
serde = { workspace = true }
1616
serde_json = { workspace = true }
17+
serde_path_to_error = { workspace = true }
1718
serde_yml = { workspace = true }
1819
thiserror = { workspace = true }
1920

‎crates/openshell-policy-schema/src/lib.rs‎

Lines changed: 55 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -591,9 +591,17 @@ pub fn parse_policy_with_limits(source: &str, limits: ParseLimits) -> Result<Pol
591591
if let Some(unknown_field) = find_unknown_field(&value) {
592592
miette::bail!("unknown field '{}' in authored policy", unknown_field.path);
593593
}
594-
let policy: PolicyDocument = serde_yml::from_value(&value)
595-
.into_diagnostic()
596-
.wrap_err("failed to decode sandbox policy fields")?;
594+
let policy: PolicyDocument =
595+
serde_path_to_error::deserialize(serde_yml::Deserializer::new(&value))
596+
.map_err(|error| {
597+
let path = bound_path(error.path().to_string());
598+
if path == "." {
599+
miette::miette!("{}", error.inner())
600+
} else {
601+
miette::miette!("{path}: {}", error.inner())
602+
}
603+
})
604+
.wrap_err("failed to decode sandbox policy fields")?;
597605
validate_policy(&policy)?;
598606
Ok(policy)
599607
}
@@ -937,11 +945,15 @@ fn sequence(value: Option<&serde_yml::Value>) -> &[serde_yml::Value] {
937945
}
938946

939947
fn join(parent: &str, child: &str) -> String {
940-
let mut path = if parent.is_empty() {
948+
bound_path(if parent.is_empty() {
941949
child.to_owned()
942950
} else {
943951
format!("{parent}.{child}")
944-
};
952+
})
953+
}
954+
955+
// Cap diagnostic paths so oversized authored keys cannot bloat errors.
956+
fn bound_path(mut path: String) -> String {
945957
if path.len() > MAX_UNKNOWN_FIELD_PATH_BYTES {
946958
let mut end = MAX_UNKNOWN_FIELD_PATH_BYTES - 3;
947959
while !path.is_char_boundary(end) {
@@ -1279,6 +1291,44 @@ mod tests {
12791291
}
12801292
}
12811293

1294+
fn decode_error_chain(source: &str) -> Vec<String> {
1295+
let error = parse_policy(source).expect_err("type mismatch must fail closed");
1296+
error.chain().map(ToString::to_string).collect()
1297+
}
1298+
1299+
#[test]
1300+
fn type_errors_name_the_offending_field() {
1301+
let source = "version: 1\nnetwork_policies:\n github_api:\n endpoints:\n - host: api.github.com\n port: \"443\"\n";
1302+
assert_eq!(
1303+
decode_error_chain(source),
1304+
[
1305+
"failed to decode sandbox policy fields",
1306+
"network_policies.github_api.endpoints[0].port: type mismatch: expected unsigned integer, found string",
1307+
]
1308+
);
1309+
}
1310+
1311+
#[test]
1312+
fn type_errors_without_a_path_keep_their_message() {
1313+
let chain = decode_error_chain("- version: 1\n");
1314+
assert_eq!(chain[0], "failed to decode sandbox policy fields");
1315+
assert!(
1316+
!chain[1].starts_with(".: "),
1317+
"unexpected prefix in {chain:?}"
1318+
);
1319+
}
1320+
1321+
#[test]
1322+
fn bounds_type_error_paths_under_long_keys() {
1323+
let policy_name = "é".repeat(2_000);
1324+
let source = format!(
1325+
"version: 1\nnetwork_policies:\n {policy_name}:\n endpoints:\n - host: example.com\n port: \"443\"\n"
1326+
);
1327+
let chain = decode_error_chain(&source);
1328+
assert!(chain[1].contains("...: type mismatch"));
1329+
assert!(chain[1].len() <= MAX_UNKNOWN_FIELD_PATH_BYTES + 100);
1330+
}
1331+
12821332
#[test]
12831333
fn accepts_open_user_data_maps() {
12841334
let source = r#"

‎examples/governance-interceptor/Cargo.lock‎

Lines changed: 12 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎examples/supervisor-middleware-content-guard/Cargo.lock‎

Lines changed: 12 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)