Skip to content

Conversation

@jorris
Copy link
Contributor

@jorris jorris commented Nov 25, 2025

  • NOTE: This check is only a warning right now. It will be upgraded to
    deny once we are confident the rule and the data is correct and not
    blocking valid builds

JIRA: ROK-814

* NOTE: This check is only a warning right now. It will be upgraded to
  deny once we are confident the rule and the data is correct and not
  blocking valid builds

JIRA: ROK-814
@codecov
Copy link

codecov bot commented Nov 25, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines Coverage Δ
policy/release/rpm_build_deps/rpm_build_deps.rego 100.00% <100.00%> (ø)
...cy/release/rpm_build_deps/rpm_build_deps_test.rego 100.00% <100.00%> (ø)

... and 16 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jorris
Copy link
Contributor Author

jorris commented Nov 25, 2025

Tests Generated by Claude

Comment on lines 63 to 74
# Test with another invalid download location
test_invalid_download_location_wrong_domain if {
invalid_location := "https://malicious.org/package.rpm"
att := _sbom_attestation_with_download_location(invalid_location)
expected_locations := array.concat(["NOASSERTION"], _mock_allowed_locations)
expected := {{
"code": "rpm_build_deps.download_location_valid",
"msg": sprintf("Download Location is %s which is not in %v", [invalid_location, expected_locations]),
}}
lib.assert_equal_results(expected, rpm_build_deps.warn) with input.attestations as [att]
with data.rule_data.allowed_rpm_build_dependency_sources as _mock_allowed_locations
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

duplicate of test_invalid_download_location, can be removed

Comment on lines 76 to 84
# Test with location that doesn't match the patterns
test_invalid_download_location_no_pattern_match if {
# This location doesn't match any of the allowed patterns
invalid_location := "https://example.com/package.rpm"
att := _sbom_attestation_with_download_location(invalid_location)
results := rpm_build_deps.warn with input.attestations as [att]
with data.rule_data.allowed_rpm_build_dependency_sources as _mock_allowed_locations
count(results) > 0
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

duplicate of test_invalid_download_location, can be removed

# description: Builds have valid download locations for RPM build dependencies
# custom:
# short_name: download_location_valid
# failure_msg: Download Location is %s which is not in %v
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# failure_msg: Download Location is %s which is not in %v
# failure_msg: RPM build dependency source %s is not in the allowed list: %v.
# solution: >-
# The list of allowed RPM build dependency sources can be set via the
# `allowed_rpm_build_dependency_sources` rule data.

Copy link
Contributor

@st3penta st3penta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm. @conforma/devs can we merge it?

result := lib.result_helper(rego.metadata.chain(), [pkg.downloadLocation, valid_locations])
}

matches_any(branch, valid_locations) if {
Copy link
Member

@simonbaird simonbaird Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a blocker, but I'd suggest making the params more generic. "Branch" seems confusing in this context.

Suggested change
matches_any(branch, valid_locations) if {
matches_any(value, patterns) if {

And maybe we flip the order so it's consistent with regex.match:

Suggested change
matches_any(branch, valid_locations) if {
matches_any(patterns, value) if {

some pkg in s.packages

# NOASSERTION is displayed in the SBOM for the RPMS that have been built
valid_locations := array.concat(["NOASSERTION"], lib.rule_data("allowed_rpm_build_dependency_sources"))
Copy link
Member

@simonbaird simonbaird Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also not a blocker, but it might be tidier and perhaps more efficient to move the concat outside the rule. It's a pattern we follow elsewhere


warn ... {
   ...
   not matches_any(pkg.downloadLocation, _valid_locations)
}

# NOASSERTION is displayed in the SBOM for the RPMS that have been built
_valid_locations := array.concat(["NOASSERTION"], lib.rule_data("allowed_rpm_build_dependency_sources"))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants