diff --git a/README.md b/README.md index 07312c9..dd81792 100644 --- a/README.md +++ b/README.md @@ -45,6 +45,7 @@ Usage: cyclonelab Commands: validate Checks that a file is valid JSON and conforms to the CycloneDX schema transform Applies a declarative transformation recipe to a CycloneDX SBOM + lint Checks that a transformation YAML file is well-formed, without requiring an SBOM suggest Suggests useful component fields missing from a CycloneDX SBOM help Print this message or the help of the given subcommand(s) diff --git a/llms.md b/llms.md index 71f7df2..b11d324 100644 --- a/llms.md +++ b/llms.md @@ -1,7 +1,7 @@ # cyclonelab > `cyclonelab` is a CLI for generating and manipulating [CycloneDX](https://cyclonedx.org/) Software Bills of -> Materials (SBOMs). It ships three subcommands — `validate`, `transform`, `suggest` — and +> Materials (SBOMs). It ships four subcommands — `validate`, `transform`, `lint`, `suggest` — and > supports CycloneDX spec versions **1.5**, **1.6**, and **1.7** (JSON only). This file gives an LLM (ChatGPT, Gemini, > Claude, ...) enough detail to write correct `cyclonelab` invocations and valid `transform` YAML recipes. @@ -13,6 +13,7 @@ cyclonelab Commands: validate Check that a file is valid JSON and conforms to the CycloneDX schema transform Apply a declarative YAML transformation recipe to a CycloneDX SBOM + lint Check that a transformation YAML file is well-formed, without requiring an SBOM suggest Suggest useful component/metadata fields missing from a CycloneDX SBOM help Print this message or the help of the given subcommand(s) ``` @@ -362,6 +363,37 @@ cyclonelab transform template-sbom.cdx.json recipe.yaml "dist/{$artifact_stem}-s --variable repo=code-rhapsodie/cyclonelab --variable version=1.2.0 ``` +## `cyclonelab lint` + +``` +cyclonelab lint +``` + +Statically checks a `transform` recipe YAML file for well-formedness — **no `SBOM_FILE` is read or required**, so +it can run before an SBOM even exists (e.g. as a fast CI check on a recipe change). It performs every check +`transform` does on the recipe file itself, minus anything that requires resolving a JSONPath against an actual +document: + +- YAML parses and matches the recipe schema (same `file:line:column: message` error as `transform` on failure). +- No duplicate step `id`s; every `manual` step has a non-empty `description`; every `upgrade` step's + `version_target` is reachable by a bundled recipe. +- Every `target`/`source`/`paths` JSONPath is syntactically valid (a plain syntax check — it cannot know whether the + path will match anything in a real document, since none is loaded). +- Action-specific option combinations are checked exactly as `transform` checks them at run time: `add` has exactly + one of `value`/`valueFrom`, and `valueFrom`'s `file`/`generator` fields are consistent (known `format`, a + `generator: hash` has `algo: sha256` and exactly one of `path`/`url`); `merge`'s `value` is valid JSON, and a + `target[]` value is a JSON array; a `target[]`/`value[]` append form isn't combined with `when`. +- `foreach`'s reserved iteration variable names (`artifact_name`, `artifact_stem`, `artifact_path`) don't collide + with a declared `variables:` entry. +- Every `{$name}` placeholder used in a step resolves to either a declared `variables:` entry or (when `foreach` is + set) a reserved iteration variable name — an unresolvable one is almost always a typo, since `{$var}` templating + silently leaves an unknown placeholder untouched instead of failing at `transform` run time, so `lint` turns that + silent no-op into a hard error. A declared variable that no step ever references is only a warning (not fatal), + printed to stdout. + +On success: prints `'' looks valid.` (after any unused-variable warnings) and exits 0. On any of the above +failing: prints an error and exits non-zero, same as `transform` would once it got that far. + ## CycloneDX specifics - **Supported `specVersion` values**: `1.5`, `1.6`, `1.7` — JSON format only. The matching JSON Schema is bundled and diff --git a/src/cli.rs b/src/cli.rs index 75c8322..68451ac 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -1,7 +1,7 @@ use anyhow::Result; use clap::{Parser, Subcommand}; -use crate::commands::{suggest, transform, validate}; +use crate::commands::{lint, suggest, transform, validate}; use crate::version; #[derive(Debug, Parser)] @@ -21,6 +21,8 @@ enum Commands { Validate(validate::ValidateArgs), /// Applies a declarative transformation recipe to a CycloneDX SBOM. Transform(transform::TransformArgs), + /// Checks that a transformation YAML file is well-formed, without requiring an SBOM. + Lint(lint::LintArgs), /// Suggests useful component fields missing from a CycloneDX SBOM. Suggest(suggest::SuggestArgs), } @@ -30,6 +32,7 @@ impl Cli { match &self.command { Commands::Validate(args) => validate::run(args), Commands::Transform(args) => transform::run(args), + Commands::Lint(args) => lint::run(args), Commands::Suggest(args) => suggest::run(args), } } diff --git a/src/commands/lint.rs b/src/commands/lint.rs new file mode 100644 index 0000000..d842c6e --- /dev/null +++ b/src/commands/lint.rs @@ -0,0 +1,112 @@ +//! `lint` subcommand: statically checks a transformation YAML file for +//! well-formedness, without requiring an SBOM (see +//! `doc/transform/README.md`). + +use std::collections::HashSet; +use std::path::PathBuf; + +use anyhow::{Result, bail}; +use clap::Args; + +use crate::commands::transform; +use crate::transform_actions; + +#[derive(Debug, Args)] +pub struct LintArgs { + /// YAML file describing the transformation steps. + transform_file: PathBuf, +} + +pub fn run(args: &LintArgs) -> Result<()> { + if !args.transform_file.is_file() { + bail!( + "Unable to find transformation file '{}'", + args.transform_file.display() + ); + } + + let transform_file = transform::load_transform_file(&args.transform_file)?; + + transform_actions::validate_steps(&transform_file.steps)?; + transform_actions::lint_steps(&transform_file.steps)?; + transform::check_foreach_variable_conflicts( + transform_file.foreach.as_ref(), + &transform_file.variables, + )?; + check_variables(&transform_file)?; + + println!("'{}' looks valid.", args.transform_file.display()); + Ok(()) +} + +/// Cross-checks declared variables against every `{$name}` placeholder used +/// across the file's steps: a name used but never declared (almost always a +/// typo, since `util::template::render` leaves an unknown placeholder +/// untouched instead of failing) is a hard error; a declared variable never +/// referenced is only a warning. +fn check_variables(transform_file: &transform::TransformFile) -> Result<()> { + let mut declared: HashSet = transform_file.variables.keys().cloned().collect(); + if transform_file.foreach.is_some() { + declared.extend(transform::FOREACH_VAR_NAMES.iter().map(|s| s.to_string())); + } + + let serialized = serde_json::to_string(&transform_file.steps)?; + let used = referenced_variable_names(&serialized); + + for name in &used { + if !declared.contains(name.as_str()) { + bail!("variable '{name}' is used in a step but never declared in 'variables:'"); + } + } + + for name in transform_file.variables.keys() { + if !used.contains(name.as_str()) { + println!("warning: variable '{name}' is declared but never used"); + } + } + + Ok(()) +} + +/// Every `{$name}` placeholder found in `text` (e.g. every step, +/// JSON-serialized), regardless of whether it resolves to a declared +/// variable. +fn referenced_variable_names(text: &str) -> HashSet { + let mut names = HashSet::new(); + let mut rest = text; + while let Some(start) = rest.find("{$") { + let after = &rest[start + 2..]; + let Some(end) = after.find('}') else { + break; + }; + let candidate = &after[..end]; + if !candidate.is_empty() + && candidate + .chars() + .all(|c| c.is_ascii_alphanumeric() || c == '_') + { + names.insert(candidate.to_string()); + } + rest = &after[end + 1..]; + } + names +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn referenced_variable_names_finds_every_placeholder() { + let names = referenced_variable_names(r#"{"target":"{$repo}/{$version}"}"#); + assert_eq!( + names, + HashSet::from(["repo".to_string(), "version".to_string()]) + ); + } + + #[test] + fn referenced_variable_names_ignores_a_dollar_not_forming_a_placeholder() { + assert!(referenced_variable_names("no placeholder here").is_empty()); + } +} diff --git a/src/commands/mod.rs b/src/commands/mod.rs index 5687020..1cb12a7 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -5,6 +5,7 @@ //! function, then register it in `crate::cli::Commands` and in //! `crate::cli::Cli::run`. +pub mod lint; pub mod suggest; pub mod transform; pub mod validate; diff --git a/src/commands/transform.rs b/src/commands/transform.rs index e9e6e58..0c26747 100644 --- a/src/commands/transform.rs +++ b/src/commands/transform.rs @@ -18,8 +18,10 @@ use crate::util::template::{matches_single_wildcard, render}; /// Names of the ambient variables `foreach` injects for each matched file /// (see `doc/transform/foreach.md` §"Variables d'itération"): reserved, so a -/// declared `variables:` entry cannot reuse one of them. -const FOREACH_VAR_NAMES: [&str; 3] = ["artifact_name", "artifact_stem", "artifact_path"]; +/// declared `variables:` entry cannot reuse one of them. Also used by `lint` +/// (see `commands::lint`), which needs the same reserved names to check +/// variable usage without running `foreach` for real. +pub(crate) const FOREACH_VAR_NAMES: [&str; 3] = ["artifact_name", "artifact_stem", "artifact_path"]; #[derive(Debug, Args)] pub struct TransformArgs { @@ -37,22 +39,25 @@ pub struct TransformArgs { variables: Vec, } +/// Also used, read-only, by `lint` (see `commands::lint`), which needs the +/// same deserialization and static checks `transform` runs before it ever +/// looks at an SBOM. #[derive(Debug, Deserialize)] -struct TransformFile { +pub(crate) struct TransformFile { #[serde(default)] from: Option, #[serde(default)] #[allow(dead_code)] to: Option, #[serde(default)] - variables: HashMap, + pub(crate) variables: HashMap, #[serde(default)] - foreach: Option, - steps: Vec, + pub(crate) foreach: Option, + pub(crate) steps: Vec, } #[derive(Debug, Deserialize)] -struct VariableDecl { +pub(crate) struct VariableDecl { #[serde(default)] env: Option, #[serde(default)] @@ -65,7 +70,7 @@ struct VariableDecl { /// `steps` pipeline once per file found in `dir` matching `pattern`, instead /// of running it once on a fixed `OUTPUT_FILE`. #[derive(Debug, Deserialize)] -struct ForeachDecl { +pub(crate) struct ForeachDecl { dir: PathBuf, pattern: String, } @@ -285,7 +290,7 @@ fn run_foreach( /// iteration variable names (see `doc/transform/foreach.md` §"Variables /// d'itération") — checked once at load time, regardless of how many (if /// any) files `foreach` will later match. -fn check_foreach_variable_conflicts( +pub(crate) fn check_foreach_variable_conflicts( foreach: Option<&ForeachDecl>, declared: &HashMap, ) -> Result<()> { @@ -357,7 +362,7 @@ fn load_and_validate_sbom(sbom_file: &std::path::Path) -> Result { Ok(document) } -fn load_transform_file(transform_file: &std::path::Path) -> Result { +pub(crate) fn load_transform_file(transform_file: &std::path::Path) -> Result { let content = fs::read_to_string(transform_file) .with_context(|| format!("Unable to read '{}'", transform_file.display()))?; yaml_serde::from_str(&content).map_err(|err| { diff --git a/src/transform_actions/add.rs b/src/transform_actions/add.rs index 5fcff83..45340e5 100644 --- a/src/transform_actions/add.rs +++ b/src/transform_actions/add.rs @@ -187,6 +187,43 @@ impl AddStep { } } + /// Static checks performable without a document: `target`'s JSONPath + /// syntax, and the same option combinations `apply`/`compute_value` + /// require at run time (see `doc/transform/action-add.md`) — used by + /// `lint`. + pub(crate) fn lint(&self) -> Result<()> { + match self.target.strip_suffix("[]") { + Some(prefix) => { + if self.when.is_some() { + bail!("'when' is not supported when 'target' ends with '[]'"); + } + jsonpath::literal(prefix).map(|_| ()) + } + None => jsonpath::check_syntax(&self.target), + } + .with_context(|| format!("invalid target '{}'", self.target))?; + + match &self.value_from { + Some(value_from) => value_from.lint()?, + None if self.value.is_none() => { + bail!("neither 'value' nor 'valueFrom' is set"); + } + None => {} + } + + if self.target.ends_with("[]") + && let Some(value) = &self.value + && !value.is_array() + { + bail!( + "'value' must be an array because target '{}' ends with '[]'", + self.target + ); + } + + Ok(()) + } + fn compute_value(&self, ctx: &StepContext) -> Result { match &self.value_from { None => self.value.clone().with_context(|| { @@ -224,6 +261,23 @@ impl ValueFrom { } } + /// Static checks on the option combination, mirroring [`ValueFrom::resolve`]'s + /// run-time checks — used by [`AddStep::lint`]. + fn lint(&self) -> Result<()> { + match (&self.file, &self.generator) { + (Some(_), Some(_)) => bail!("'valueFrom' cannot set both 'file' and 'generator'"), + (None, None) => bail!("'valueFrom' needs either 'file' or 'generator'"), + (Some(_), None) => match self.format.as_deref() { + None | Some("json") | Some("text") => Ok(()), + Some(other) => bail!( + "unsupported 'valueFrom.format' \"{other}\" for 'valueFrom.file' (expected \ + \"text\", or omit it for JSON)" + ), + }, + (None, Some(generator)) => generator.lint(self), + } + } + /// Reads `file` (relative to `ctx.base_dir`) — always a hard step error /// if it can't be read, so a recipe never silently ships without the /// evidence/content it was meant to embed. `format` then decides how the @@ -258,6 +312,34 @@ impl ValueFrom { } impl Generator { + /// Static checks on `value_from`'s fields for this generator, mirroring + /// [`Generator::generate_hash`]'s run-time checks — used by + /// [`ValueFrom::lint`]. + fn lint(self, value_from: &ValueFrom) -> Result<()> { + match self { + Generator::Uuid | Generator::Timestamp => Ok(()), + Generator::Hash => { + match value_from.algo.as_deref() { + Some("sha256") => {} + Some(other) => bail!( + "unsupported hash algorithm '{}' (only 'sha256' is supported)", + other + ), + None => bail!("'valueFrom.generator: hash' requires 'algo'"), + } + match (&value_from.path, &value_from.url) { + (Some(_), None) | (None, Some(_)) => Ok(()), + (Some(_), Some(_)) => { + bail!("'valueFrom' cannot set both 'path' and 'url'") + } + (None, None) => { + bail!("'valueFrom.generator: hash' needs either 'path' or 'url'") + } + } + } + } + } + fn generate(self, value_from: &ValueFrom, ctx: &StepContext) -> Result { match self { Generator::Uuid => Ok(Value::String(Uuid::new_v4().to_string())), @@ -794,6 +876,126 @@ mod tests { assert!(raw.ends_with("\"}]")); } + #[test] + fn lint_accepts_a_well_formed_step() { + let step: AddStep = serde_json::from_value(json!({ + "target": "$.metadata.newField", + "value": "hello", + })) + .unwrap(); + step.lint().unwrap(); + } + + #[test] + fn lint_rejects_an_invalid_target() { + let step: AddStep = serde_json::from_value(json!({ + "target": "$.a[", + "value": "hello", + })) + .unwrap(); + assert!(step.lint().is_err()); + } + + #[test] + fn lint_rejects_neither_value_nor_value_from() { + let step: AddStep = serde_json::from_value(json!({"target": "$.a"})).unwrap(); + let err = step.lint().unwrap_err(); + assert!(err.to_string().contains("neither 'value' nor 'valueFrom'")); + } + + #[test] + fn lint_rejects_a_non_array_value_on_an_append_target() { + let step: AddStep = serde_json::from_value(json!({ + "target": "$.a[]", + "value": {"not": "an array"}, + })) + .unwrap(); + let err = step.lint().unwrap_err(); + assert!(err.to_string().contains("must be an array")); + } + + #[test] + fn lint_rejects_when_combined_with_an_append_target() { + let step: AddStep = serde_json::from_value(json!({ + "target": "$.a[]", + "value": [1], + "when": "array", + })) + .unwrap(); + let err = step.lint().unwrap_err(); + assert!(err.to_string().contains("'when' is not supported")); + } + + #[test] + fn lint_rejects_value_from_with_both_file_and_generator() { + let step: AddStep = serde_json::from_value(json!({ + "target": "$.a", + "valueFrom": {"file": "a.json", "generator": "uuid"}, + })) + .unwrap(); + let err = step.lint().unwrap_err(); + assert!(err.to_string().contains("both 'file' and 'generator'")); + } + + #[test] + fn lint_rejects_an_unsupported_value_from_file_format() { + let step: AddStep = serde_json::from_value(json!({ + "target": "$.a", + "valueFrom": {"file": "a.json", "format": "yaml"}, + })) + .unwrap(); + let err = step.lint().unwrap_err(); + assert!(err.to_string().contains("yaml")); + } + + #[test] + fn lint_rejects_an_unsupported_hash_algo() { + let step: AddStep = serde_json::from_value(json!({ + "target": "$.a", + "valueFrom": {"generator": "hash", "algo": "md5", "path": "a.bin"}, + })) + .unwrap(); + let err = step.lint().unwrap_err(); + assert!(err.to_string().contains("md5")); + } + + #[test] + fn lint_rejects_hash_generator_with_both_path_and_url() { + let step: AddStep = serde_json::from_value(json!({ + "target": "$.a", + "valueFrom": { + "generator": "hash", + "algo": "sha256", + "path": "a.bin", + "url": "http://example.invalid/a.bin", + }, + })) + .unwrap(); + let err = step.lint().unwrap_err(); + assert!(err.to_string().contains("path") && err.to_string().contains("url")); + } + + #[test] + fn lint_rejects_hash_generator_without_path_or_url() { + let step: AddStep = serde_json::from_value(json!({ + "target": "$.a", + "valueFrom": {"generator": "hash", "algo": "sha256"}, + })) + .unwrap(); + let err = step.lint().unwrap_err(); + assert!(err.to_string().contains("path") && err.to_string().contains("url")); + } + + #[test] + fn lint_accepts_a_valid_hash_generator() { + let step: AddStep = serde_json::from_value(json!({ + "target": "$.a", + "valueFrom": {"generator": "hash", "algo": "sha256", "path": "a.bin"}, + })) + .unwrap(); + step.lint().unwrap(); + } + /// Minimal single-shot HTTP/1.1 server returning `body` for one request, /// used to exercise `valueFrom.url` without depending on network access /// or an HTTP-mocking crate. diff --git a/src/transform_actions/manual.rs b/src/transform_actions/manual.rs index bf79dfe..55f9f07 100644 --- a/src/transform_actions/manual.rs +++ b/src/transform_actions/manual.rs @@ -2,7 +2,7 @@ //! automatically. Never touches the document — a plain warning (see //! `doc/transform/action-manual.md`). -use anyhow::Result; +use anyhow::{Context, Result}; use serde::{Deserialize, Serialize}; use serde_json::Value; @@ -30,6 +30,15 @@ impl ManualStep { PathsField::Many { paths } => paths, } } + + /// Static check performable without a document: every path's JSONPath + /// syntax — used by `lint`. + pub(crate) fn lint(&self) -> Result<()> { + for pattern in self.paths() { + jsonpath::check_syntax(pattern).with_context(|| format!("invalid path '{pattern}'"))?; + } + Ok(()) + } } impl Action for ManualStep { @@ -87,6 +96,18 @@ mod tests { assert_eq!(doc, before); } + #[test] + fn lint_accepts_valid_paths() { + let step: ManualStep = serde_json::from_value(json!({"paths": ["$.a", "$..b"]})).unwrap(); + step.lint().unwrap(); + } + + #[test] + fn lint_rejects_an_invalid_path() { + let step: ManualStep = serde_json::from_value(json!({"target": "$.a["})).unwrap(); + assert!(step.lint().is_err()); + } + #[test] fn multiple_paths_only_present_ones_are_resolved() { let step: ManualStep = diff --git a/src/transform_actions/merge.rs b/src/transform_actions/merge.rs index 098e9e8..7882e63 100644 --- a/src/transform_actions/merge.rs +++ b/src/transform_actions/merge.rs @@ -15,6 +15,33 @@ pub struct MergeStep { pub value: String, } +impl MergeStep { + /// Static checks performable without a document, mirroring [`Action::apply`]'s + /// run-time checks: `value` is valid JSON, `target`'s JSONPath syntax, and + /// (when `target` ends with `[]`) that `value` is a JSON array — used by + /// `lint`. + pub(crate) fn lint(&self) -> Result<()> { + let fragment: Value = + serde_json::from_str(&self.value).context("'value' is not valid JSON")?; + + let (target, append) = match self.target.strip_suffix("[]") { + Some(prefix) => (prefix, true), + None => (self.target.as_str(), false), + }; + + jsonpath::literal(target).with_context(|| format!("invalid target '{}'", self.target))?; + + if append && !fragment.is_array() { + bail!( + "'value' must be a JSON array because target '{}' ends with '[]'", + self.target + ); + } + + Ok(()) + } +} + impl Action for MergeStep { fn apply(&self, doc: &mut Value, ctx: &StepContext) -> Result<()> { let fragment: Value = serde_json::from_str(&self.value) @@ -157,6 +184,48 @@ mod tests { assert_eq!(doc, json!({"field": {"a": 1}})); } + #[test] + fn lint_accepts_a_well_formed_step() { + let step: MergeStep = serde_json::from_value(json!({ + "target": "$.metadata.component", + "value": "{\"type\": \"library\"}", + })) + .unwrap(); + step.lint().unwrap(); + } + + #[test] + fn lint_rejects_invalid_json_value() { + let step: MergeStep = serde_json::from_value(json!({ + "target": "$.field", + "value": "{not json", + })) + .unwrap(); + let err = step.lint().unwrap_err(); + assert!(err.to_string().contains("not valid JSON")); + } + + #[test] + fn lint_rejects_an_invalid_target() { + let step: MergeStep = serde_json::from_value(json!({ + "target": "$.a[", + "value": "{}", + })) + .unwrap(); + assert!(step.lint().is_err()); + } + + #[test] + fn lint_rejects_a_non_array_value_on_an_append_target() { + let step: MergeStep = serde_json::from_value(json!({ + "target": "$.field[]", + "value": "{\"a\": 1}", + })) + .unwrap(); + let err = step.lint().unwrap_err(); + assert!(err.to_string().contains("must be a JSON array")); + } + #[test] fn invalid_json_fragment_is_an_explicit_step_error() { let step: MergeStep = serde_json::from_value(json!({ diff --git a/src/transform_actions/mod.rs b/src/transform_actions/mod.rs index 61bac97..bdcc4d1 100644 --- a/src/transform_actions/mod.rs +++ b/src/transform_actions/mod.rs @@ -15,7 +15,7 @@ pub mod upgrade; use std::collections::HashMap; use std::path::{Path, PathBuf}; -use anyhow::{Result, bail}; +use anyhow::{Context, Result, bail}; use serde::{Deserialize, Serialize}; use serde_json::Value; @@ -147,6 +147,27 @@ pub fn validate_steps(steps: &[Step]) -> Result<()> { Ok(()) } +/// Runs every step's own static checks — JSONPath syntax, action-specific +/// option combinations — without touching any document (see the `lint` +/// method each action module implements on its step struct). Used by +/// `commands::lint`, on top of [`validate_steps`], which the `lint` command +/// also runs first. +pub fn lint_steps(steps: &[Step]) -> Result<()> { + for step in steps { + let result = match &step.action { + StepAction::Add(s) => s.lint(), + StepAction::Remove(s) => s.lint(), + StepAction::Move(s) => s.lint(), + StepAction::Merge(s) => s.lint(), + StepAction::Transform(s) => s.lint(), + StepAction::Manual(s) => s.lint(), + StepAction::Upgrade(_) => Ok(()), + }; + result.with_context(|| format!("step '{}'", step.id))?; + } + Ok(()) +} + fn substitute_strings(value: &mut Value, vars: &[(&str, &str)]) { match value { Value::String(s) => *s = crate::util::template::render(s, vars), diff --git a/src/transform_actions/move.rs b/src/transform_actions/move.rs index 8cd85c2..f634701 100644 --- a/src/transform_actions/move.rs +++ b/src/transform_actions/move.rs @@ -17,6 +17,14 @@ pub struct MoveStep { pub when: Option, } +impl MoveStep { + /// Static check performable without a document: `source` and `target`'s + /// JSONPath syntax and pairing — used by `lint`. + pub(crate) fn lint(&self) -> Result<()> { + jsonpath::paired_target_key(&self.source, &self.target).map(|_| ()) + } +} + impl Action for MoveStep { fn apply(&self, doc: &mut Value, ctx: &StepContext) -> Result<()> { let target_key = jsonpath::paired_target_key(&self.source, &self.target).with_context(|| { @@ -82,6 +90,20 @@ mod tests { assert_eq!(doc, json!({"a": 1})); } + #[test] + fn lint_accepts_a_matching_source_and_target() { + let step: MoveStep = + serde_json::from_value(json!({"source": "$..old", "target": "$..new"})).unwrap(); + step.lint().unwrap(); + } + + #[test] + fn lint_rejects_an_invalid_target() { + let step: MoveStep = + serde_json::from_value(json!({"source": "$.a", "target": "$.["})).unwrap(); + assert!(step.lint().is_err()); + } + #[test] fn recursive_pattern_pairs_source_and_target_per_occurrence() { let step: MoveStep = diff --git a/src/transform_actions/remove.rs b/src/transform_actions/remove.rs index e954ad4..0c385b3 100644 --- a/src/transform_actions/remove.rs +++ b/src/transform_actions/remove.rs @@ -1,7 +1,7 @@ //! `remove` action: deletes the value present at a location in the document //! (see `doc/transform/action-remove.md`). -use anyhow::Result; +use anyhow::{Context, Result}; use serde::{Deserialize, Serialize}; use serde_json::Value; @@ -16,6 +16,15 @@ pub struct RemoveStep { pub when: Option, } +impl RemoveStep { + /// Static check performable without a document: `target`'s JSONPath + /// syntax — used by `lint`. + pub(crate) fn lint(&self) -> Result<()> { + jsonpath::check_syntax(&self.target) + .with_context(|| format!("invalid target '{}'", self.target)) + } +} + impl Action for RemoveStep { fn apply(&self, doc: &mut Value, _ctx: &StepContext) -> Result<()> { let locations = jsonpath::resolve(doc, &self.target)?; @@ -69,6 +78,18 @@ mod tests { assert_eq!(doc, json!({"nested": {"other": 3}})); } + #[test] + fn lint_accepts_a_valid_target() { + let step: RemoveStep = serde_json::from_value(json!({"target": "$..field"})).unwrap(); + step.lint().unwrap(); + } + + #[test] + fn lint_rejects_an_invalid_target() { + let step: RemoveStep = serde_json::from_value(json!({"target": "$.a["})).unwrap(); + assert!(step.lint().is_err()); + } + #[test] fn when_excludes_occurrences_of_a_different_shape() { let step: RemoveStep = diff --git a/src/transform_actions/structural.rs b/src/transform_actions/structural.rs index efb66c8..423678a 100644 --- a/src/transform_actions/structural.rs +++ b/src/transform_actions/structural.rs @@ -64,6 +64,16 @@ pub struct MapArrayFields { pub item: Map, } +impl TransformStep { + /// Static check performable without a document: `source` and `target`'s + /// JSONPath syntax and pairing — used by `lint`. The per-`Strategy` + /// fields are already fully validated by strong typing at deserialize + /// time, with no further semantic constraint worth checking statically. + pub(crate) fn lint(&self) -> Result<()> { + jsonpath::paired_target_key(&self.source, &self.target).map(|_| ()) + } +} + impl Action for TransformStep { fn apply(&self, doc: &mut Value, ctx: &StepContext) -> Result<()> { let target_key = jsonpath::paired_target_key(&self.source, &self.target).with_context(|| { @@ -226,6 +236,28 @@ mod tests { } } + #[test] + fn lint_accepts_a_matching_source_and_target() { + let step: TransformStep = serde_json::from_value(json!({ + "source": "$..evidence.identity", + "target": "$..evidence.identity", + "strategy": "wrap-in-array", + })) + .unwrap(); + step.lint().unwrap(); + } + + #[test] + fn lint_rejects_an_invalid_target() { + let step: TransformStep = serde_json::from_value(json!({ + "source": "$.a", + "target": "$.[", + "strategy": "wrap-in-array", + })) + .unwrap(); + assert!(step.lint().is_err()); + } + #[test] fn wrap_in_array_without_item_wraps_the_value_as_is() { let step: TransformStep = serde_json::from_value(json!({ diff --git a/src/util/jsonpath.rs b/src/util/jsonpath.rs index 60f3aa9..3770fe6 100644 --- a/src/util/jsonpath.rs +++ b/src/util/jsonpath.rs @@ -394,6 +394,13 @@ pub fn set(doc: &mut Value, path: &[PathElem], value: Value) -> Result<()> { Ok(()) } +/// Checks only that `path` is syntactically valid (per the grammar +/// documented at the top of this module), without resolving it against any +/// document — used by `lint`, which has no SBOM to resolve against. +pub fn check_syntax(path: &str) -> Result<()> { + parse(path).map(|_| ()) +} + /// Parses `path` and requires it to designate a single literal location (no /// `*`/`..`) — used by `add`/`merge` targets, which must unambiguously /// create missing structure. @@ -660,6 +667,30 @@ mod tests { assert_eq!(doc, json!({"a": 1})); } + #[test] + fn check_syntax_accepts_every_supported_form() { + for path in [ + "$", + "$.a.b", + "$.[\"$schema\"]", + "$.a[*]", + "$..a", + "$.a[?type==foo]", + ] { + assert!(check_syntax(path).is_ok(), "expected '{path}' to be valid"); + } + } + + #[test] + fn check_syntax_rejects_malformed_paths() { + for path in ["a.b", "$.", "$.a[", "$.a[?type=foo]"] { + assert!( + check_syntax(path).is_err(), + "expected '{path}' to be invalid" + ); + } + } + #[test] fn literal_rejects_wildcards_and_recursive_descent() { assert!(literal("$.a[*]").is_err());