Skip to content

Conversation

@Piskoo
Copy link
Collaborator

@Piskoo Piskoo commented Nov 7, 2025

This PR adds support for external http://, https://, chainloop:// policy sources to policy devel eval

Examples:

cl policy devel eval --policy https://raw.githubusercontent.com/chainloop-dev/chainloop/main/docs/examples/policies/quickstart/cdx-fresh.yaml --material sbom.json 
{
   "result": {
      "violations": [
         "SBOM created at: 2024-01-09T12:00:00Z which is too old (freshness limit set to 30 days)"
      ],
      "skip_reasons": [],
      "skipped": false
   }
}

cl policy devel eval --policy chainloop://sbom-ntia --material sbom.json
{
   "result": {
      "violations": [
         "missing author",
         "missing supplier for 'AES-256-GCM'",
         "missing supplier for 'ECDH'",
         "missing supplier for 'RSA-2048'",
         "missing supplier for 'SHA384'",
         "missing supplier for 'SHA512withRSA'",
         "missing supplier for 'TLSv1.2'",
         "missing supplier for 'google.com'",
         "missing unique identifier (PURL, CPE, SWID) for 'AES-256-GCM'",
         "missing unique identifier (PURL, CPE, SWID) for 'ECDH'",
         "missing unique identifier (PURL, CPE, SWID) for 'RSA-2048'",
         "missing unique identifier (PURL, CPE, SWID) for 'SHA384'",
         "missing unique identifier (PURL, CPE, SWID) for 'SHA512withRSA'",
         "missing unique identifier (PURL, CPE, SWID) for 'TLSv1.2'",
         "missing unique identifier (PURL, CPE, SWID) for 'google.com'",
         "missing version for 'AES-256-GCM'",
         "missing version for 'ECDH'",
         "missing version for 'RSA-2048'",
         "missing version for 'SHA384'",
         "missing version for 'SHA512withRSA'",
         "missing version for 'TLSv1.2'",
         "missing version for 'google.com'"
      ],
      "skip_reasons": [],
      "skipped": false
   }
}

Signed-off-by: Sylwester Piskozub <sylwesterpiskozub@gmail.com>
@Piskoo Piskoo marked this pull request as ready for review November 7, 2025 15:32

func createPolicies(policyPath string, inputs map[string]string) (*v1.Policies, error) {
// Check if the policy path already has a scheme (chainloop://, http://, https://, file://)
ref := policyPath
Copy link
Member

Choose a reason for hiding this comment

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

check if there is smth already on the way we parse the policies in the contracts

func (action *PolicyEval) Run() (*policydevel.EvalSummary, error) {
var attClient pb.AttestationServiceClient
if action.CPConnection != nil {
attClient = pb.NewAttestationServiceClient(action.CPConnection)
Copy link
Member

Choose a reason for hiding this comment

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

we might want to make sure this is not mandatory, I mean, if you do not provide a connection, the command should still work with file:// or http://

Copy link
Collaborator Author

@Piskoo Piskoo Nov 10, 2025

Choose a reason for hiding this comment

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

It's not mandatory, we use it if it's available otherwise it works the same way as before and it will return jwt related error to the user for chainloop:// policies

Copy link
Member

@migmartri migmartri left a comment

Choose a reason for hiding this comment

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

This is sweet, but is this a breaking change, in other words, would comments that used to provide a file path work out of the box?

Comment on lines +90 to +94
scheme, _ := policies.RefParts(policyPath)
if scheme == "" {
// Default to file://
ref = fmt.Sprintf("file://%s", policyPath)
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is sweet, but is this a breaking change, in other words, would comments that used to provide a file path work out of the box?
Do you need to provide file:// always now?

It works out of the box, file:// is optional, backwards compatibility is held by this fragment and that's the only reason we have to check for scheme.

Copy link
Member

Choose a reason for hiding this comment

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

awesome, that makes sense.

Copy link
Member

@migmartri migmartri left a comment

Choose a reason for hiding this comment

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

See my suggestion, thanks!

cmd.Flags().StringVar(&kind, "kind", "", fmt.Sprintf("Kind of the material: %q", schemaapi.ListAvailableMaterialKind()))
cmd.Flags().StringSliceVar(&annotations, "annotation", []string{}, "Key-value pairs of material annotations (key=value)")
cmd.Flags().StringVarP(&policyPath, "policy", "p", "policy.yaml", "Path to custom policy file")
cmd.Flags().StringVarP(&policyPath, "policy", "p", "policy.yaml", "Policy reference (file://, http://, https://, chainloop://)")
Copy link
Member

Choose a reason for hiding this comment

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

I'd personally remove file://, and add examples for example

Suggested change
cmd.Flags().StringVarP(&policyPath, "policy", "p", "policy.yaml", "Policy reference (file://, http://, https://, chainloop://)")
cmd.Flags().StringVarP(&policyPath, "policy", "p", "policy.yaml", "Policy reference (./my-policy.yaml, https://my-domain.com/my-policy.yaml, chainloop://my-stored-policy)")

Signed-off-by: Sylwester Piskozub <sylwesterpiskozub@gmail.com>
@Piskoo Piskoo merged commit 0364180 into chainloop-dev:main Nov 10, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants