-
Notifications
You must be signed in to change notification settings - Fork 30
feat: add --existing-policy flag in verify-policy command for example policies #1189
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
30b51e3 to
ce3dfd5
Compare
behnazh-w
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a unit test to tests/policy_engine/test_policy.py. Also add an integration test.
src/macaron/__main__.py
Outdated
| policy_content = file.read() | ||
| elif verify_policy_args.policy: | ||
| policy_dir = os.path.join(macaron.MACARON_PATH, "resources/policies/datalog") | ||
| available_policies = [policy[:-12] for policy in os.listdir(policy_dir) if policy.endswith(".dl.template")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be good to avoid hardcoding [:-12].
| available_policies = [policy[:-12] for policy in os.listdir(policy_dir) if policy.endswith(".dl.template")] | |
| policy_suffix = ".dl.template" | |
| available_policies = [ | |
| os.path.splitext(policy)[0].replace(policy_suffix, "") | |
| for policy in os.listdir(policy_dir) | |
| if policy.endswith(policy_suffix) | |
| ] |
src/macaron/__main__.py
Outdated
| policy_path = os.path.join(policy_dir, f"{verify_policy_args.policy}.dl.template") | ||
| with open(policy_path, encoding="utf-8") as file: | ||
| policy_content = file.read() | ||
| if verify_policy_args.package_url: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's check that the PURL using the packageurl library, which is already a dependency of Macaron:
try:
PackageURL.from_string(verify_policy_args.package_url)
except ValueError as error:
logger.error("The package url %s is not valid.", verify_policy_args.package_url)
return os.EX_USAGE
ae722e9 to
a2203de
Compare
40dbe23 to
ff9fff7
Compare
|
|
||
| apply_policy_to("check-dependencies", component_id) :- | ||
| is_component(component_id, purl), | ||
| match("<PACKAGE_PURL>@.*", purl). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if the user provides a PURL that contains version as well? Then the policy won't match the PURL.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does removing @.* resolve the issue?
from
match("<PACKAGE_PURL>@.*", purl).
to
match("<PACKAGE_PURL>", purl).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but that would mean we can't use wildcards, which are useful for creating more generic policies that apply to all versions. Alternatively, we could allow the wildcard within the PURL argument itself 🤔
behnazh-w
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest we add a --list-policies option to verify-policy, allowing users to easily see and choose from the available policies. We should also update the tutorial to include this new option. Additionally, we can provide a description file for each template policy and display that information when the command is used. To ensure consistency, let’s add a unit test that verifies every template policy has an associated description file.
| @@ -0,0 +1,53 @@ | |||
| ============================================================== | |||
| Verify with an existing example policy using --existing-policy | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Verify with an existing example policy using --existing-policy | |
| How to use the policy engine to verify with our predefined policies |
| Verify with an existing example policy using --existing-policy | ||
| ============================================================== | ||
|
|
||
| This short tutorial shows how to use the ``--existing-policy`` flag with the ``verify-policy`` subcommand to run one of the example (predefined) policies that ship with Macaron. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| This short tutorial shows how to use the ``--existing-policy`` flag with the ``verify-policy`` subcommand to run one of the example (predefined) policies that ship with Macaron. | |
| This tutorial shows how to use the ``--existing-policy`` flag with the ``verify-policy`` subcommand to run one of the predefined policies that ship with Macaron. |
| Use case | ||
| -------- | ||
|
|
||
| Use ``--existing-policy`` when you want to run one of the built-in example policies by name instead of providing a local policy file with ``--file``. Example policies are useful for quick checks or automated examples/tests. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Use ``--existing-policy`` when you want to run one of the built-in example policies by name instead of providing a local policy file with ``--file``. Example policies are useful for quick checks or automated examples/tests. | |
| Use ``--existing-policy`` when you want to run one of the built-in policies by name instead of providing a local policy file with ``--file``. Pre-defined policies are useful for quick checks or automated examples/tests. |
| Example | ||
| ------- | ||
|
|
||
| Run the ``malware-detection`` example policy against a package URL: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Run the ``malware-detection`` example policy against a package URL: | |
| Run the ``malware-detection`` policy against a package URL: |
| ./run_macaron.sh verify-policy \ | ||
| --database output/macaron.db \ | ||
| --existing-policy malware-detection \ | ||
| --package-url "pkg:pypi/django" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| --package-url "pkg:pypi/django" | |
| --package-url pkg:pypi/django@.* |
|
|
||
| def test_verify_existing_policy_not_found(tmp_path: Path) -> None: | ||
| """Requesting a non-existent policy returns usage error.""" | ||
| db_file = tmp_path / "macaron.db" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use os.path.join for constructing paths to be consistent with the rest of the codebase.
|
|
||
| def test_verify_existing_policy_success(tmp_path: Path) -> None: | ||
| """When an existing policy is provided and package-url is valid, verify_policy returns EX_OK.""" | ||
| db_file = tmp_path / "macaron.db" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use os.path.join for constructing paths to be consistent with the rest of the codebase.
|
|
||
| apply_policy_to("github_actions_vulns", component_id) :- | ||
| is_component(component_id, purl), | ||
| match("<PACKAGE_PURL>*", purl). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on our discussion, this should change to
| match("<PACKAGE_PURL>*", purl). | |
| match("<PACKAGE_PURL>", purl). |
|
|
||
| apply_policy_to("check-dependencies", component_id) :- | ||
| is_component(component_id, purl), | ||
| match("<PACKAGE_PURL>*", purl). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on our discussion, this should change to
| match("<PACKAGE_PURL>*", purl). | |
| match("<PACKAGE_PURL>", purl). |
|
|
||
| apply_policy_to("check-component", component_id) :- | ||
| is_component(component_id, purl), | ||
| match("<PACKAGE_PURL>*", purl). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on our discussion, this should change to
| match("<PACKAGE_PURL>*", purl). | |
| match("<PACKAGE_PURL>", purl). |
| #include "prelude.dl" | ||
|
|
||
| Policy("github_actions_vulns", component_id, "GitHub Actions Vulnerability Detection") :- | ||
| check_passed(component_id, "mcn_githubactions_vulnerabilities_1"). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please consistently use 4 spaces for indentation. This applies to all of the policy templates.
Signed-off-by: Demolus13 <[email protected]>
Signed-off-by: Demolus13 <[email protected]>
Signed-off-by: Demolus13 <[email protected]>
Signed-off-by: Demolus13 <[email protected]>
…olicy command Signed-off-by: Demolus13 <[email protected]>
Signed-off-by: Demolus13 <[email protected]>
Signed-off-by: Demolus13 <[email protected]>
Signed-off-by: Demolus13 <[email protected]>
Signed-off-by: Demolus13 <[email protected]>
… the .description file Signed-off-by: Demolus13 <[email protected]>
5381e89 to
8ab86ef
Compare
Summary
This Pull Request introduces a new
--existing-policyflag to theverify-policycommand, allowing users to run example policies by name without specifying a file path. It also adds support for policy templates.The new
--existing-policy(-e) flag executes a predefined policy template by name. If the template exists, it populates the<PACKAGE_PURL>placeholder with the value from the--package-url(-purl) argument and runs the policy. If the template name is not found, it lists all available templates.Example policies
Description of changes
--existing-policy(-e) argument to theverify-policyCLI command, enabling users to select and run example policies from the built-in resources.resources/policies/datalogdirectory.--package-url(-purl) argument to substitute the<PACKAGE_PURL>placeholder in policy templates.--existing-policy(-e) flag intests/policy_engine/test_existing_policy.pyRelated issues
N/A
Checklist
verifiedlabel should appear next to all of your commits on GitHub.