From f5badbd0ab9974437985316baf0444439bdb1359 Mon Sep 17 00:00:00 2001 From: Ryan Bottriell Date: Sat, 13 Jul 2024 15:52:10 -0700 Subject: [PATCH] Add validation rule to check for an spdx license Signed-off-by: Ryan Bottriell --- Cargo.lock | 10 + Cargo.toml | 1 + crates/spk-build/Cargo.toml | 1 + crates/spk-build/src/validation/error.rs | 8 + crates/spk-build/src/validation/mod.rs | 2 + .../spk-build/src/validation/spdx_license.rs | 77 +++++++ .../src/validation/spdx_license_test.rs | 192 ++++++++++++++++++ crates/spk-build/src/validation/validator.rs | 4 + crates/spk-schema/src/package.rs | 15 ++ crates/spk-schema/src/spec.rs | 6 + crates/spk-schema/src/v0/spec.rs | 4 + crates/spk-schema/src/validation.rs | 6 + docs/error_codes/_index.md | 15 ++ docs/ref/spec.md | 29 +-- 14 files changed, 356 insertions(+), 14 deletions(-) create mode 100644 crates/spk-build/src/validation/spdx_license.rs create mode 100644 crates/spk-build/src/validation/spdx_license_test.rs diff --git a/Cargo.lock b/Cargo.lock index 8dbfc11d3..ce4c9d7f2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3424,6 +3424,15 @@ dependencies = [ "windows-sys 0.48.0", ] +[[package]] +name = "spdx" +version = "0.10.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "47317bbaf63785b53861e1ae2d11b80d6b624211d42cb20efcd210ee6f8a14bc" +dependencies = [ + "smallvec", +] + [[package]] name = "spfs" version = "0.40.0" @@ -3786,6 +3795,7 @@ dependencies = [ "serde", "serde_json", "serde_yaml 0.9.27", + "spdx", "spfs", "spk-exec", "spk-schema", diff --git a/Cargo.toml b/Cargo.toml index 87eefc882..f02b6b5dc 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -89,6 +89,7 @@ serde_json = "1.0" serde_yaml = "0.9.25" serial_test = "3.1" shellexpand = "3.1.0" +spdx = "0.10" spfs = { path = "crates/spfs" } spfs-cli-common = { path = "crates/spfs-cli/common" } spfs-encoding = { path = "crates/spfs-encoding" } diff --git a/crates/spk-build/Cargo.toml b/crates/spk-build/Cargo.toml index 6c93272df..8b7fd79f1 100644 --- a/crates/spk-build/Cargo.toml +++ b/crates/spk-build/Cargo.toml @@ -30,6 +30,7 @@ spk-exec = { workspace = true } spk-solve = { workspace = true } spk-schema = { workspace = true } spk-storage = { workspace = true } +spdx = { workspace = true } strum = { workspace = true } thiserror = { workspace = true } miette = { workspace = true } diff --git a/crates/spk-build/src/validation/error.rs b/crates/spk-build/src/validation/error.rs index 54e8c4fab..efc0bbf64 100644 --- a/crates/spk-build/src/validation/error.rs +++ b/crates/spk-build/src/validation/error.rs @@ -10,6 +10,7 @@ use thiserror::Error; pub type Result = std::result::Result; +/// Errors returned by failed package validation rules. #[derive(Diagnostic, Debug, Error, Clone, PartialEq, Eq)] #[diagnostic( url( @@ -162,4 +163,11 @@ pub enum Error { code(spk::build::validation::strong_inheritance_var_description) )] StrongInheritanceVarDescriptionRequired, + + #[error("A valid SPDX license required, got {given:?}")] + #[diagnostic(severity(warning), code(spk::build::validation::spdx_license))] + SpdxLicenseRequired { given: String }, + #[error("Package should not have a license specified")] + #[diagnostic(severity(warning), code(spk::build::validation::spdx_license))] + SpdxLicenseDenied, } diff --git a/crates/spk-build/src/validation/mod.rs b/crates/spk-build/src/validation/mod.rs index 638a3e7ea..549cbfbcd 100644 --- a/crates/spk-build/src/validation/mod.rs +++ b/crates/spk-build/src/validation/mod.rs @@ -10,6 +10,7 @@ mod error; mod inherit_requirements; mod long_var_description; mod recursive_build; +mod spdx_license; mod strong_inheritance_var_desc; mod validator; @@ -21,5 +22,6 @@ pub use error::{Error, Result}; pub use inherit_requirements::InheritRequirementsValidator; pub use long_var_description::LongVarDescriptionValidator; pub use recursive_build::RecursiveBuildValidator; +pub use spdx_license::SpdxLicenseValidator; pub use strong_inheritance_var_desc::StrongInheritanceVarDescriptionValidator; pub use validator::{Outcome, Report, Status, Subject, Validator}; diff --git a/crates/spk-build/src/validation/spdx_license.rs b/crates/spk-build/src/validation/spdx_license.rs new file mode 100644 index 000000000..b16132239 --- /dev/null +++ b/crates/spk-build/src/validation/spdx_license.rs @@ -0,0 +1,77 @@ +// Copyright (c) Sony Pictures Imageworks, et al. +// SPDX-License-Identifier: Apache-2.0 +// https://github.com/imageworks/spk + +use spk_schema::validation::{ + ValidationMatcherDiscriminants, + ValidationRuleDiscriminants as RuleKind, +}; +use spk_schema::{Package, Variant}; + +use super::{Error, Outcome, Report, Status, Subject}; +use crate::report::{BuildReport, BuildSetupReport}; + +#[cfg(test)] +#[path = "./spdx_license_test.rs"] +mod spdx_license_test; + +pub struct SpdxLicenseValidator { + pub kind: RuleKind, +} + +impl super::validator::sealed::Sealed for SpdxLicenseValidator {} + +#[async_trait::async_trait] +impl super::Validator for SpdxLicenseValidator { + async fn validate_setup(&self, setup: &BuildSetupReport) -> Report + where + P: Package, + V: Variant + Send + Sync, + { + let meta = setup.package.metadata(); + let exists = meta.license.is_some(); + let is_valid = match meta.license.as_ref() { + Some(value) => spdx::license_id(value).is_some(), + None => true, + }; + let status = match self.kind { + RuleKind::Require if !exists => Status::Required(Error::SpdxLicenseRequired { + given: meta.license.clone().unwrap_or_default(), + }), + RuleKind::Allow | RuleKind::Require if !is_valid => { + Status::Required(Error::SpdxLicenseRequired { + given: meta.license.clone().unwrap_or_default(), + }) + } + RuleKind::Deny if exists && is_valid => Status::Denied(Error::SpdxLicenseDenied), + _ => Status::Allowed, + }; + Outcome { + locality: String::new(), + subject: Subject::Everything, + status, + condition: ValidationMatcherDiscriminants::SpdxLicense, + } + .into() + } + + async fn validate_build(&self, report: &BuildReport) -> Report + where + P: Package, + V: Variant + Send + Sync, + { + let is_empty = report.output.collected_changes.is_empty(); + let status = match self.kind { + RuleKind::Deny if is_empty => Status::Denied(Error::EmptyPackageDenied), + RuleKind::Require if !is_empty => Status::Required(Error::EmptyPackageRequired), + _ => Status::Allowed, + }; + Outcome { + locality: String::new(), + subject: Subject::Everything, + status, + condition: ValidationMatcherDiscriminants::EmptyPackage, + } + .into() + } +} diff --git a/crates/spk-build/src/validation/spdx_license_test.rs b/crates/spk-build/src/validation/spdx_license_test.rs new file mode 100644 index 000000000..8162cc6c8 --- /dev/null +++ b/crates/spk-build/src/validation/spdx_license_test.rs @@ -0,0 +1,192 @@ +// Copyright (c) Sony Pictures Imageworks, et al. +// SPDX-License-Identifier: Apache-2.0 +// https://github.com/imageworks/spk + +use std::sync::Arc; + +use spfs::tracking::Manifest; +use spk_schema::foundation::option_map; +use spk_schema::validation::ValidationMatcher; +use spk_schema::{spec, Package, ValidationRule}; +use spk_solve::Solution; + +use crate::report::BuildSetupReport; +use crate::validation::Validator; + +macro_rules! basic_setup { + ($pkg:tt) => {{ + let package = Arc::new(spec!($pkg)); + + let environment = Solution::default(); + BuildSetupReport { + environment, + variant: option_map! {}, + environment_filesystem: Manifest::new( + spfs::tracking::Entry::empty_dir_with_open_perms_with_data(package.ident().clone()), + ), + package, + } + }}; +} + +#[tokio::test] +async fn test_license_allowed_empty() { + let setup = basic_setup!( + { + "pkg": "base/1.0.0/3TCOOP2W", + "meta": {}, + "sources": [], + "build": { + "script": "echo building...", + }, + } + ); + + ValidationRule::Allow { + condition: ValidationMatcher::SpdxLicense, + } + .validate_setup(&setup) + .await + .into_result() + .expect("Should allow no license with default allow rule"); +} + +#[tokio::test] +async fn test_license_allowed_valid() { + let setup = basic_setup!( + { + "pkg": "base/1.0.0/3TCOOP2W", + "meta": { + "license": "Apache-2.0" // from spdx license list + }, + "sources": [], + "build": { + "script": "echo building...", + }, + } + ); + + ValidationRule::Allow { + condition: ValidationMatcher::SpdxLicense, + } + .validate_setup(&setup) + .await + .into_result() + .expect("Should allow a known license with default allow rule"); +} + +#[tokio::test] +async fn test_license_allowed_invalid() { + let setup = basic_setup!( + { + "pkg": "base/1.0.0/3TCOOP2W", + "meta": { + "license": "unknown" // NOT from spdx license list + }, + "sources": [], + "build": { + "script": "echo building...", + }, + } + ); + + ValidationRule::Allow { + condition: ValidationMatcher::SpdxLicense, + } + .validate_setup(&setup) + .await + .into_result() + .expect_err("Should fail with default allow rule and invalid license"); +} + +#[tokio::test] +async fn test_license_require_empty() { + let setup = basic_setup!( + { + "pkg": "base/1.0.0/3TCOOP2W", + "meta": {}, + "sources": [], + "build": { + "script": "echo building...", + }, + } + ); + + ValidationRule::Require { + condition: ValidationMatcher::SpdxLicense, + } + .validate_setup(&setup) + .await + .into_result() + .expect_err("Should fail when no license and require rule"); +} + +#[tokio::test] +async fn test_license_deny_empty() { + let setup = basic_setup!( + { + "pkg": "base/1.0.0/3TCOOP2W", + "meta": {}, + "sources": [], + "build": { + "script": "echo building...", + }, + } + ); + + ValidationRule::Deny { + condition: ValidationMatcher::SpdxLicense, + } + .validate_setup(&setup) + .await + .into_result() + .expect("Should allow empty license with deny rule"); +} + +#[tokio::test] +async fn test_license_deny_invalid() { + let setup = basic_setup!( + { + "pkg": "base/1.0.0/3TCOOP2W", + "meta": { + "license": "unknown" // NOT from license list + }, + "sources": [], + "build": { + "script": "echo building...", + }, + } + ); + + ValidationRule::Deny { + condition: ValidationMatcher::SpdxLicense, + } + .validate_setup(&setup) + .await + .into_result() + .expect("Should allow invalid license with deny rule"); +} + +#[tokio::test] +async fn test_license_deny_valid() { + let setup = basic_setup!( + { + "pkg": "base/1.0.0/3TCOOP2W", + "meta": { + "license": "Apache-2.0" + }, + "sources": [], + "build": { + "script": "echo building...", + }, + } + ); + + ValidationRule::Deny { + condition: ValidationMatcher::SpdxLicense, + } + .validate_setup(&setup) + .await + .into_result() + .expect_err("Should fail with valid license and deny rule"); +} diff --git a/crates/spk-build/src/validation/validator.rs b/crates/spk-build/src/validation/validator.rs index d1b2a42e6..f2e9c5309 100644 --- a/crates/spk-build/src/validation/validator.rs +++ b/crates/spk-build/src/validation/validator.rs @@ -42,6 +42,10 @@ macro_rules! rule_to_validator { let kind = spk_schema::validation::ValidationRuleDiscriminants::from($rule); #[allow(unused_braces)] match $rule.condition() { + ValidationMatcher::SpdxLicense => { + let $bind = super::SpdxLicenseValidator { kind }; + $op + } ValidationMatcher::EmptyPackage => { let $bind = super::EmptyPackageValidator { kind }; $op diff --git a/crates/spk-schema/src/package.rs b/crates/spk-schema/src/package.rs index 4e9866b05..8f6268e24 100644 --- a/crates/spk-schema/src/package.rs +++ b/crates/spk-schema/src/package.rs @@ -32,6 +32,9 @@ pub trait Package: /// This includes the version and optional build fn ident(&self) -> &BuildIdent; + /// The additional metadata attached to this package + fn metadata(&self) -> &crate::metadata::Meta; + /// The values for this packages options used for this build. fn option_values(&self) -> OptionMap; @@ -142,6 +145,10 @@ impl Package for std::sync::Arc { (**self).ident() } + fn metadata(&self) -> &crate::metadata::Meta { + (**self).metadata() + } + fn option_values(&self) -> OptionMap { (**self).option_values() } @@ -214,6 +221,10 @@ impl Package for Box { (**self).ident() } + fn metadata(&self) -> &crate::metadata::Meta { + (**self).metadata() + } + fn option_values(&self) -> OptionMap { (**self).option_values() } @@ -286,6 +297,10 @@ impl Package for &T { (**self).ident() } + fn metadata(&self) -> &crate::metadata::Meta { + (**self).metadata() + } + fn option_values(&self) -> OptionMap { (**self).option_values() } diff --git a/crates/spk-schema/src/spec.rs b/crates/spk-schema/src/spec.rs index 260a6b2f9..7bd4d62b1 100644 --- a/crates/spk-schema/src/spec.rs +++ b/crates/spk-schema/src/spec.rs @@ -562,6 +562,12 @@ impl Package for Spec { } } + fn metadata(&self) -> &crate::metadata::Meta { + match self { + Spec::V0Package(spec) => spec.metadata(), + } + } + fn option_values(&self) -> OptionMap { match self { Spec::V0Package(spec) => spec.option_values(), diff --git a/crates/spk-schema/src/v0/spec.rs b/crates/spk-schema/src/v0/spec.rs index 88b474843..a13148f2c 100644 --- a/crates/spk-schema/src/v0/spec.rs +++ b/crates/spk-schema/src/v0/spec.rs @@ -222,6 +222,10 @@ impl Package for Spec { &self.pkg } + fn metadata(&self) -> &crate::metadata::Meta { + &self.meta + } + fn option_values(&self) -> OptionMap { let mut opts = OptionMap::default(); for opt in self.build.options.iter() { diff --git a/crates/spk-schema/src/validation.rs b/crates/spk-schema/src/validation.rs index 61c8576fd..bd310f106 100644 --- a/crates/spk-schema/src/validation.rs +++ b/crates/spk-schema/src/validation.rs @@ -71,6 +71,9 @@ impl ValidationSpec { /// The default rules assumed for all packages pub fn default_rules() -> Vec { vec![ + ValidationRule::Allow { + condition: ValidationMatcher::SpdxLicense, + }, ValidationRule::Deny { condition: ValidationMatcher::EmptyPackage, }, @@ -223,6 +226,7 @@ pub enum ValidationMatcher { InheritRequirements { packages: Vec, }, + SpdxLicense, } #[derive( @@ -314,6 +318,7 @@ impl<'de> Deserialize<'de> for ValidationRule { let kind = map.next_value::()?; match kind { Kind::EmptyPackage => Ok(ValidationMatcher::EmptyPackage), + Kind::SpdxLicense => Ok(ValidationMatcher::SpdxLicense), Kind::CollectAllFiles => Ok(ValidationMatcher::EmptyPackage), Kind::StrongInheritanceVarDescription => { Ok(ValidationMatcher::StrongInheritanceVarDescription) @@ -386,6 +391,7 @@ impl Serialize for ValidationRule { | ValidationMatcher::CollectAllFiles | ValidationMatcher::StrongInheritanceVarDescription | ValidationMatcher::LongVarDescription + | ValidationMatcher::SpdxLicense | ValidationMatcher::EmptyPackage => {} ValidationMatcher::InheritRequirements { packages } => { if !packages.is_empty() { diff --git a/docs/error_codes/_index.md b/docs/error_codes/_index.md index c8206daec..4344c38ba 100644 --- a/docs/error_codes/_index.md +++ b/docs/error_codes/_index.md @@ -58,6 +58,21 @@ This validation is triggered when a description is not provided for strong inher In cases where a description is not required, see the documentation section on [build variable description]({{< ref "../use/spec" >}}#buildvariabledescription) +#### `spk::build::validation::spdx_license` + +This validation is triggered when a valid spdx license is not provided. By default, a license is not required, but when given it must be from the [SPDX License List](https://spdx.org/licenses/). + +In cases where a custom license string is needed, reverse the default validation like so: + +```yaml +... + +build: + validation: + - deny: SpdxLicense +... +``` + ## Spfs Errors ### `spfs::generic` diff --git a/docs/ref/spec.md b/docs/ref/spec.md index 4037381fa..4ee64bb0a 100644 --- a/docs/ref/spec.md +++ b/docs/ref/spec.md @@ -190,17 +190,18 @@ build: ##### Available Validation Rules -| Name (default) | Property | Type | Description | -| ------------------------------ | -------- | ------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| EmptyPackage (Deny) | | | Matched when no files are installed to spfs during the build | -| AlterExistingFiles (Deny) | | | Matched when a package modifies files from other packages when building | -| | packages | _List[_str_]_ | Only match when the modified files belong to one of these named packages | -| | action | _str_ | Only match this type of change, one of `Change`, `Remove`, or `Touch` | -| CollectExistingFiles (Deny) | | | Matched when a package collects files from other packages in the build environment | -| | packages | _List[_str_]_ | Only match when the modified files belong to one of these named packages. The special `Self` value can be used to refer to the current package's name. | -| InheritRequirements (Required) | | | Matched when a package in the build environment has an inherited requirement that is not present in the package generated by this build. | -| | packages | _List[_str_]_ | Only match when the inherited requirement comes from one of these named packages. | -| RecursiveBuild (Deny) | | | Matched when the build environment contains another version of the package being built. This rule implicitly enables rules to allow modifying and collecting files from the previous version of this package. Additional rules can be added to reverse these implicit ones | +| Name (default) | Property | Type | Description | +| ------------------------------ | -------- | ------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| EmptyPackage (Deny) | | | Matched when no files are installed to spfs during the build | +| AlterExistingFiles (Deny) | | | Matched when a package modifies files from other packages when building | +| | packages | _List[_str_]_ | Only match when the modified files belong to one of these named packages | +| | action | _str_ | Only match this type of change, one of `Change`, `Remove`, or `Touch` | +| CollectExistingFiles (Deny) | | | Matched when a package collects files from other packages in the build environment | +| | packages | _List[_str_]_ | Only match when the modified files belong to one of these named packages. The special `Self` value can be used to refer to the current package's name. | +| InheritRequirements (Required) | | | Matched when a package in the build environment has an inherited requirement that is not present in the package generated by this build. | +| | packages | _List[_str_]_ | Only match when the inherited requirement comes from one of these named packages. | +| RecursiveBuild (Deny) | | | Matched when the build environment contains another version of the package being built. This rule implicitly enables rules to allow modifying and collecting files from the previous version of this package. Additional rules can be added to reverse these implicit ones | +| SpdxLicense (Allow) | | | Matched when the package being built has a valid spdx license identifier in the metadata (meta.license). Use `Require` to ensure that a license is provided and valid. `Allow` ensures that a provided value is valid but also allows no license. `Deny` can be used to ensure no license is specified. Remove the validation altogether if a custom license is needed (not recommended) | For example: @@ -345,8 +346,8 @@ A build option can be one of [VariableRequest](#variablerequest), or [PackageReq #### PackageRequest -| Field | Type | Description | -| ------------------- | --------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| Field | Type | Description | +| ------------------- | --------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | pkg | _[`RangeIdentifier`](#rangeidentifier)_ | Specifies a desired package, components and acceptable version range. | | prereleasePolicy | _[PreReleasePolicy](#prereleasepolicy)_ | Defines how pre-release versions should be handled when resolving this request | | inclusionPolicy | _[InclusionPolicy](#inclusionpolicy)_ | Defines when the requested package should be included in the environment | @@ -419,7 +420,7 @@ The package identifier takes the form `[/[/]]`, where: | Component | Description | | --------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | name | The package name, can only have lowercase letter and dashes (`-`) | -| version | The version number, see [versioning]({{< ref "../use/versioning" >}}) | +| version | The version number, see [versioning]({{< ref "../use/versioning" >}}) | | build | The build string, should not be specified in a spec file as it is generated by the system at build time. Digests are calculated based on the package build options, and there are two special values `src` and `embedded` for source packages and embedded packages, respectively | ## Compat