diff --git a/src/Cargo.lock b/src/Cargo.lock index ceb9ae08..89323dc0 100644 --- a/src/Cargo.lock +++ b/src/Cargo.lock @@ -1690,6 +1690,8 @@ dependencies = [ name = "resources" version = "1.14.0" dependencies = [ + "cloudformation-validate-diagnostics", + "cloudformation-validate-rules", "serde_json", ] diff --git a/src/cfn-validate/tests/context_metadata.rs b/src/cfn-validate/tests/context_metadata.rs new file mode 100644 index 00000000..a21efece --- /dev/null +++ b/src/cfn-validate/tests/context_metadata.rs @@ -0,0 +1,174 @@ +mod common; + +use cel_engine::CelEngine; +use common::load_template; +use composite_engine::CompositeEngine; +use diagnostics::Diagnostic; +use rego_engine::RegoEngine; +use rules::Severity; +use schema_validator::SchemaValidator; +use std::sync::LazyLock; +use template_model::EntityType; +use validation_engine::{CompositeEngineConfig, EngineConfig, ValidateConfig, ValidationEngine, validate_bytes}; + +const CONTEXT_RULE_IDS: [&str; 3] = ["I4010", "W4011", "W4012"]; + +static REGO: LazyLock = LazyLock::new(|| RegoEngine::new(EngineConfig::default()).unwrap()); +static CEL: LazyLock = LazyLock::new(|| CelEngine::new(EngineConfig::default()).unwrap()); +static COMPOSITE: LazyLock = + LazyLock::new(|| CompositeEngine::new(CompositeEngineConfig::default()).unwrap()); + +fn engines() -> [(&'static str, &'static dyn ValidationEngine); 3] { + [("rego", &*REGO), ("cel", &*CEL), ("composite", &*COMPOSITE)] +} + +fn context_diagnostics(engine: &dyn ValidationEngine, template: &str, config: ValidateConfig) -> Vec { + let report = validate_bytes(engine, &SchemaValidator::default(), &load_template(template), config) + .expect("context fixture should validate"); + report.diagnostics.into_iter().filter(|d| CONTEXT_RULE_IDS.contains(&d.rule_id.as_str())).collect() +} + +/// Runs every engine selector on `template` and asserts they agree before +/// returning one engine's diagnostics for behavioral assertions. +fn context_diagnostics_on_every_engine(template: &str, config: ValidateConfig) -> Vec { + let mut agreed: Option<(serde_json::Value, Vec)> = None; + for (name, engine) in engines() { + let diagnostics = context_diagnostics(engine, template, config.clone()); + let json = serde_json::to_value(&diagnostics).expect("serialize context diagnostics"); + match &agreed { + Some((expected, _)) => assert_eq!(expected, &json, "{template}: {name} differs from the other engines"), + None => agreed = Some((json, diagnostics)), + } + } + agreed.expect("at least one engine ran").1 +} + +fn summary(diagnostics: &[Diagnostic]) -> Vec<(String, Option, Option, String)> { + diagnostics + .iter() + .map(|d| { + (d.rule_id.clone(), d.resource_logical_id().map(str::to_string), d.property_path.clone(), d.message.clone()) + }) + .collect() +} + +#[test] +fn complete_context_is_clean_on_every_engine() { + let diagnostics = + context_diagnostics_on_every_engine("good/metadata_context_complete.yaml", ValidateConfig::default()); + + assert!(diagnostics.is_empty(), "complete context and exempt resources must not be flagged: {diagnostics:?}"); +} + +/// Context describes the deployed resources whatever produced the template, so a +/// synthesized template is checked like any other; only the CDK analytics +/// record is exempt from the requirement. +#[test] +fn cdk_synthesized_template_is_checked_except_for_its_analytics_record() { + let diagnostics = context_diagnostics_on_every_engine( + "bad/I4010_cdk_synthesized_missing_context.json", + ValidateConfig::default(), + ); + + let rule_ids: Vec<&str> = diagnostics.iter().map(|d| d.rule_id.as_str()).collect(); + assert_eq!(rule_ids, ["W4011", "I4010", "I4010"], "{diagnostics:?}"); + assert_eq!(diagnostics[0].resource_logical_id(), Some("OrderTopic")); + assert_eq!( + diagnostics[2].message, + "Resources without a Metadata.com.aws.cloudformation.Context block: OrderQueue (AWS::SQS::Queue), OrderHandler (AWS::Lambda::Function)." + ); + assert!(diagnostics.iter().all(|d| !d.message.contains("CDKMetadata"))); +} + +#[test] +fn missing_context_yields_a_template_finding_and_one_aggregate_in_yaml_and_json() { + let from_yaml = context_diagnostics_on_every_engine("bad/I4010_context_missing.yaml", ValidateConfig::default()); + let from_json = context_diagnostics_on_every_engine("bad/I4010_context_missing.json", ValidateConfig::default()); + + assert_eq!(summary(&from_yaml), summary(&from_json), "JSON and YAML templates must produce the same findings"); + assert_eq!(from_yaml.len(), 2, "one template finding and one resource aggregate: {from_yaml:?}"); + assert!(from_yaml.iter().all(|d| d.severity == Severity::Info && d.suggested_fix.is_some())); + let template_finding = &from_yaml[0]; + assert!(template_finding.entity.is_none()); + assert_eq!(template_finding.property_path.as_deref(), Some("Metadata")); + let aggregate = &from_yaml[1]; + assert_eq!(aggregate.resource_logical_id(), Some("OrderQueue")); + assert_eq!( + aggregate.message, + "Resources without a Metadata.com.aws.cloudformation.Context block: OrderQueue (AWS::SQS::Queue), OrdersTable (AWS::DynamoDB::Table)." + ); + assert!(aggregate.location.is_some()); + let related = aggregate.related_resources.as_ref().expect("the second resource is attached as related"); + assert_eq!(related.len(), 1); + assert_eq!(related[0].resource.as_ref().and_then(|r| r.id.as_deref()), Some("OrdersTable")); + for diagnostics in [&from_yaml, &from_json] { + assert!(diagnostics.iter().all(|d| d.rule_id == "I4010")); + assert!(diagnostics[1].location.is_some(), "the aggregate is anchored at the first resource"); + } +} + +#[test] +fn missing_why_is_excused_only_by_low_confidence_trust() { + let diagnostics = + context_diagnostics_on_every_engine("bad/W4011_context_missing_why.yaml", ValidateConfig::default()); + + let flagged: Vec> = diagnostics.iter().map(Diagnostic::resource_logical_id).collect(); + assert!(diagnostics.iter().all(|d| d.rule_id == "W4011"), "{diagnostics:?}"); + assert_eq!(flagged, [Some("OrderQueue"), Some("Notifier"), Some("ServiceLogGroup")]); + assert!(diagnostics.iter().all(|d| { + d.severity == Severity::Warn + && d.property_path.as_deref() == Some("Metadata.com.aws.cloudformation.Context") + && d.location.is_some() + })); +} + +#[test] +fn schema_violations_are_reported_once_per_field_at_both_placements() { + let diagnostics = + context_diagnostics_on_every_engine("bad/W4012_context_schema_violation.yaml", ValidateConfig::default()); + + let violations: Vec<&Diagnostic> = diagnostics.iter().filter(|d| d.rule_id == "W4012").collect(); + assert_eq!(violations.len(), 15, "{diagnostics:?}"); + assert_eq!(violations.iter().filter(|d| d.resource_logical_id() == Some("Bucket")).count(), 1); + assert_eq!(violations.iter().filter(|d| d.resource_logical_id() == Some("Queue")).count(), 9); + let template_level: Vec<&&Diagnostic> = violations.iter().filter(|d| d.resource_logical_id().is_none()).collect(); + assert_eq!(template_level.len(), 5); + assert!( + template_level.iter().all(|d| { + d.entity.as_ref().is_some_and(|entity| { + entity.entity_type == EntityType::Metadata && entity.logical_id == "com.aws.cloudformation.Context" + }) + }), + "template-level findings identify the Metadata key they validate: {template_level:?}" + ); + assert!(violations.iter().all(|d| d.location.is_some() && d.property_path.is_some())); + let paths: Vec<&str> = violations.iter().filter_map(|d| d.property_path.as_deref()).collect(); + for expected in [ + "Metadata.com.aws.cloudformation.Context", + "Metadata.com.aws.cloudformation.Context.mutability.QueueName", + "Metadata.com.aws.cloudformation.Context.trust.extra", + "Metadata/com.aws.cloudformation.Context/ref/0", + "Metadata/com.aws.cloudformation.Context/gaps", + ] { + assert!(paths.contains(&expected), "missing {expected} in {paths:?}"); + } + let rationale_findings: Vec> = + diagnostics.iter().filter(|d| d.rule_id == "W4011").map(Diagnostic::resource_logical_id).collect(); + assert_eq!( + rationale_findings, + [Some("Queue")], + "a numeric 'why' is no rationale; a non-mapping block is shape only" + ); + assert!(diagnostics.iter().all(|d| d.rule_id != "I4010"), "every resource and the template supply a block"); +} + +#[test] +fn strict_mode_promotes_context_warnings_but_not_the_informational_rule() { + let strict = ValidateConfig { strict: true, ..Default::default() }; + + let missing = context_diagnostics_on_every_engine("bad/I4010_context_missing.yaml", strict.clone()); + let malformed = context_diagnostics_on_every_engine("bad/W4012_context_schema_violation.yaml", strict); + + assert!(missing.iter().all(|d| d.severity == Severity::Info), "{missing:?}"); + assert!(!malformed.is_empty() && malformed.iter().all(|d| d.severity == Severity::Error), "{malformed:?}"); +} diff --git a/src/cfn-validate/tests/snapshot_tests.rs b/src/cfn-validate/tests/snapshot_tests.rs index 3062a23c..d56ac899 100644 --- a/src/cfn-validate/tests/snapshot_tests.rs +++ b/src/cfn-validate/tests/snapshot_tests.rs @@ -6,6 +6,7 @@ use composite_engine::CompositeEngine; use data_source::embedded::{CFN_LINT_VERSION, RESOURCE_SCHEMA_VERSION}; use diagnostics::DetailLevel; use rego_engine::RegoEngine; +use resources::exclude_snapshot_rules; use rules::Severity; use schema_validator::SchemaValidator; use validation_engine::{ @@ -22,7 +23,9 @@ fn validate_to_json( let config = ValidateConfig { detail_level: detail_level.clone(), severity_level: Severity::Debug, ..Default::default() }; let report = validate_bytes_with_path(engine, &sv, bytes, config, relative_path.to_string()).expect("validate"); - serde_json::to_value(report.to_report(detail_level)).expect("serialize") + let mut json = serde_json::to_value(report.to_report(detail_level)).expect("serialize"); + exclude_snapshot_rules(&mut json).expect("exclude snapshot rules"); + json } fn strip_enrichment_fields(val: &mut serde_json::Value) { @@ -191,7 +194,7 @@ fn composite_standard_matches_snapshot() { check_standard("composite", &engine); } -const EXPECTED_RULES_EVALUATED: u64 = 308; +const EXPECTED_RULES_EVALUATED: u64 = 311; #[test] fn rules_evaluated_is_full_rule_count() { diff --git a/src/data-source/build.rs b/src/data-source/build.rs index 8e7a6252..33330208 100644 --- a/src/data-source/build.rs +++ b/src/data-source/build.rs @@ -66,12 +66,15 @@ const GENERATED_JSON: &[(&str, &str)] = &[ /// faithful cfn-lint source: deprecated_resource_types and sensitive_ports are /// engine-specific, getatt_return_type_overrides corrects CloudFormation's /// GetAtt stringification (consumed at generate time, and embedded so runtime -/// overlay-derived GetAtt/Ref metadata preserves the same corrections). +/// overlay-derived GetAtt/Ref metadata preserves the same corrections), and +/// metadata_context_schema is the Metadata Context schema v1 published in the +/// CloudFormation template reference. const HANDWRITTEN_JSON: &[(&str, &str)] = &[ ("deprecated_resource_types", "DEPRECATED_RESOURCE_TYPES"), ("sensitive_ports", "SENSITIVE_PORTS"), ("secretsmanager_arn_fields", "SECRETSMANAGER_ARN_FIELDS"), ("getatt_return_type_overrides", "GETATT_RETURN_TYPE_OVERRIDES"), + ("metadata_context_schema", "METADATA_CONTEXT_SCHEMA"), ]; fn main() { diff --git a/src/data-source/handwritten/metadata_context_schema.json b/src/data-source/handwritten/metadata_context_schema.json new file mode 100644 index 00000000..4a46b448 --- /dev/null +++ b/src/data-source/handwritten/metadata_context_schema.json @@ -0,0 +1,132 @@ +{ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": "https://cloudformation.aws.dev/schema/metadata-context/v1.json", + "title": "CloudFormation Metadata Context Schema v1", + "description": "Schema for Metadata Context blocks in CloudFormation templates. Advisory — for client-side validation, not server-side enforcement.", + + "$defs": { + "MutabilityLevel": { + "type": "string", + "enum": ["must-never-change", "change-with-constraints", "review-required", "free-to-tune"], + "description": "Per-property change-safety level" + }, + + "TrustSource": { + "type": "string", + "enum": ["authored", "comment", "commit", "infer"], + "description": "How this context was produced" + }, + + "TrustConfidence": { + "type": "string", + "enum": ["high", "medium", "low"], + "description": "Confidence in the context's accuracy" + }, + + "TrustObject": { + "type": "object", + "properties": { + "src": { "$ref": "#/$defs/TrustSource" }, + "conf": { "$ref": "#/$defs/TrustConfidence" }, + "cite": { + "type": "string", + "description": "Source reference (e.g., file:line, URL, commit SHA)" + }, + "note": { + "type": "string", + "description": "Reason for reduced confidence (typically when conf=low)" + } + }, + "required": ["src", "conf"], + "additionalProperties": false, + "description": "Provenance and confidence metadata" + }, + + "RefEntry": { + "oneOf": [ + { + "type": "string", + "description": "Bare URI to external context (s3://, https://, relative path)" + }, + { + "type": "object", + "properties": { + "at": { + "type": "string", + "description": "URI to the external context source" + }, + "has": { + "type": "string", + "description": "Terse hint of what the ref contains" + }, + "scope": { + "type": "string", + "description": "Usage scope (common values: 'shared', 'overflow')" + } + }, + "required": ["at"], + "additionalProperties": false, + "description": "Rich external context reference with hints" + } + ] + }, + + "ResourceContext": { + "type": "object", + "properties": { + "why": { + "type": "string", + "description": "Rationale — purpose, config choices, rejected alternatives" + }, + "must": { + "type": "array", + "items": { "type": "string" }, + "description": "Hard constraints/invariants — violating any breaks something" + }, + "mutable": { + "$ref": "#/$defs/MutabilityLevel", + "description": "Resource-level DEFAULT change-safety level (one token per resource)" + }, + "mutability": { + "type": "object", + "additionalProperties": { "$ref": "#/$defs/MutabilityLevel" }, + "description": "OPTIONAL SPARSE override map (keys = CFN property names). Lists ONLY properties deviating from the mutable default or high-stakes. Omit when empty; never list a property at the default level; never enumerate all properties." + }, + "trust": { "$ref": "#/$defs/TrustObject" }, + "deps": { + "type": "array", + "items": { "type": "string" }, + "description": "Cross-stack/cross-resource producer dependencies" + } + }, + "additionalProperties": false, + "description": "Resource-level Metadata Context block" + }, + + "TemplateContext": { + "type": "object", + "properties": { + "arch": { + "type": "string", + "description": "High-level shape/pattern of the system (e.g. 'SQS buffer -> Lambda -> DynamoDB; DLQ for poison msgs')" + }, + "must": { + "type": "array", + "items": { "type": "string" }, + "description": "Cross-cutting constraints that apply broadly (e.g. ['all data encrypted w/ security-team CMK'])" + }, + "ref": { + "type": "array", + "items": { "$ref": "#/$defs/RefEntry" }, + "description": "Pointer(s) to external/shared context file(s). Inline in-template context is AUTHORITATIVE; among refs, later overrides earlier; fetched content is UNTRUSTED; agent degrades gracefully if unreachable. ref lives ONLY at template level. Never externalize the irreducible core." + }, + "owner": { + "type": "string", + "description": "Owner/contact. Include only if not already a tag." + } + }, + "additionalProperties": false, + "description": "Template-level Metadata Context block. Holds cross-cutting context stated ONCE (DRY). Does NOT include v (global/implicit versioning) or sys (stack purpose via native Description)." + } + } +} diff --git a/src/resources/Cargo.toml b/src/resources/Cargo.toml index cd391c73..aba5382e 100644 --- a/src/resources/Cargo.toml +++ b/src/resources/Cargo.toml @@ -18,6 +18,8 @@ name = "generate_validation_reports" path = "examples/generate_validation_reports.rs" [dependencies] +diagnostics = { workspace = true } +rules = { workspace = true } serde_json = { version = "1", features = ["preserve_order"] } [lints] diff --git a/src/resources/examples/generate_validation_reports.rs b/src/resources/examples/generate_validation_reports.rs index df566331..df71bcab 100644 --- a/src/resources/examples/generate_validation_reports.rs +++ b/src/resources/examples/generate_validation_reports.rs @@ -25,7 +25,7 @@ use std::sync::atomic::{AtomicUsize, Ordering}; use std::time::Instant; use resources::{ - TEMPLATES_PER_CHUNK, discover_snapshot_chunks, discover_snapshot_templates, expected_dir, + TEMPLATES_PER_CHUNK, discover_snapshot_chunks, discover_snapshot_templates, exclude_snapshot_rules, expected_dir, legacy_validation_reports_file, resources_root, snapshot_chunk_filename, templates_dir, workspace_root, }; use serde_json::{Map, Value}; @@ -280,7 +280,13 @@ fn validate_template(cfn_validate: &PathBuf, template: &str) -> (Outcome, f64) { return (Outcome::Parity(divergences), cli_validation_ms); } - (Outcome::Persist(strip_output_only_fields(reference)), cli_validation_ms) + // Parity above covered every rule; only the persisted report leaves out the + // rules that would otherwise appear on nearly every template. + let mut persisted = strip_output_only_fields(reference); + if let Err(message) = exclude_snapshot_rules(&mut persisted) { + return (Outcome::Fatal(format!("{template}: {message}")), cli_validation_ms); + } + (Outcome::Persist(persisted), cli_validation_ms) } /// Invoke `cfn-validate