diff --git a/Cargo.lock b/Cargo.lock index 691b422ee..6a122858f 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 b4aa9d5fe..aa127278a 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 c6cd1c39c..ba3c6cc99 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,14 @@ pub enum Error { code(spk::build::validation::strong_inheritance_var_description) )] StrongInheritanceVarDescriptionRequired, + + #[error("A valid SPDX license required, nothing specified")] + #[diagnostic(severity(warning), code(spk::build::validation::spdx_license))] + SpdxLicenseMissing, + #[error("A valid SPDX license required, got {given:?}")] + #[diagnostic(severity(warning), code(spk::build::validation::spdx_license))] + SpdxLicenseInvalid { 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 1ec45ad1d..7fe5f4317 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..c87444b4b --- /dev/null +++ b/crates/spk-build/src/validation/spdx_license.rs @@ -0,0 +1,75 @@ +// 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::SpdxLicenseMissing), + RuleKind::Allow | RuleKind::Require if !is_valid => { + Status::Required(Error::SpdxLicenseInvalid { + 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 95132d0b6..b8f7c5475 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/metadata/meta.rs b/crates/spk-schema/src/metadata/meta.rs index 81cd31b4c..1055f625c 100644 --- a/crates/spk-schema/src/metadata/meta.rs +++ b/crates/spk-schema/src/metadata/meta.rs @@ -14,39 +14,22 @@ use crate::{Error, Result}; #[path = "./meta_test.rs"] mod meta_test; -#[derive(Clone, Debug, Deserialize, Eq, Hash, Ord, PartialEq, PartialOrd, Serialize)] +#[derive(Default, Clone, Debug, Deserialize, Eq, Hash, Ord, PartialEq, PartialOrd, Serialize)] pub struct Meta { #[serde(default, skip_serializing_if = "Option::is_none")] pub description: Option, #[serde(default, skip_serializing_if = "Option::is_none")] pub homepage: Option, - #[serde( - default = "Meta::default_license", - skip_serializing_if = "String::is_empty" - )] - pub license: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub license: Option, #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] pub labels: BTreeMap, } -impl Default for Meta { - fn default() -> Self { - Meta { - description: None, - homepage: None, - license: Self::default_license(), - labels: Default::default(), - } - } -} - impl Meta { pub fn is_default(&self) -> bool { self == &Self::default() } - fn default_license() -> String { - "Unlicensed".into() - } pub fn has_label_with_value(&self, label: &str, value: &str) -> bool { if let Some(label_value) = self.labels.get(label) { diff --git a/crates/spk-schema/src/metadata/meta_test.rs b/crates/spk-schema/src/metadata/meta_test.rs index 38f76ac9b..3d4b3bd02 100644 --- a/crates/spk-schema/src/metadata/meta_test.rs +++ b/crates/spk-schema/src/metadata/meta_test.rs @@ -16,10 +16,7 @@ fn test_package_meta_missing() { }"#, ); assert!(spec.is_ok()); - assert_eq!( - spec.unwrap().meta.license, - crate::metadata::Meta::default_license() - ); + assert_eq!(spec.unwrap().meta.license, None); } #[rstest] @@ -32,7 +29,7 @@ fn test_package_meta_basic() { "#, ) .unwrap(); - assert_eq!(meta.license, crate::metadata::Meta::default_license()); + assert_eq!(meta.license, None); assert!(meta.description.is_some()); assert!(meta.homepage.is_none()); assert!(meta.labels.contains_key("department")); diff --git a/crates/spk-schema/src/package.rs b/crates/spk-schema/src/package.rs index 62e07b59f..e04accb55 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 0007fa644..918569277 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 79beb7c80..bcfa33b90 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 6fff1cf51..9db140090 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 ecd118912..2bd90bb3e 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