Skip to content

fix(policy): add referenced script traversal#2312

Merged
migmartri merged 8 commits into
chainloop-dev:mainfrom
Piskoo:fix-policy-rego-files-loading
Aug 8, 2025
Merged

fix(policy): add referenced script traversal#2312
migmartri merged 8 commits into
chainloop-dev:mainfrom
Piskoo:fix-policy-rego-files-loading

Conversation

@Piskoo

@Piskoo Piskoo commented Aug 6, 2025

Copy link
Copy Markdown
Contributor

This PR changes the way policy file is linted.

  • when *.yaml file is passed all referenced *.rego files will also be linted.
  • when directory is passed or default (cwd) is used it looks for a policy.yaml file, if not found linting fails. (before it would lint all valid *.yaml and *.rego files)

Closes #2310

Piskoo added 2 commits August 6, 2025 10:39
Signed-off-by: Sylwester Piskozub <sylwesterpiskozub@gmail.com>
Signed-off-by: Sylwester Piskozub <sylwesterpiskozub@gmail.com>
@Piskoo Piskoo marked this pull request as ready for review August 6, 2025 09:05
@Piskoo Piskoo requested review from javirln, jiparis and migmartri August 6, 2025 09:05
Comment thread app/cli/internal/policydevel/lint.go Outdated
Signed-off-by: Sylwester Piskozub <sylwesterpiskozub@gmail.com>
Comment thread app/cli/internal/policydevel/lint.go Outdated
for _, file := range files {
if file.IsDir() {
// Loads referenced rego files from all YAML files in the policy
func loadReferencedRegoFiles(policy *PolicyToLint) error {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

very nit: the argument could be a receiver instead func (policy *PolicyToLint) loadReferenceRegoFiles() {

Signed-off-by: Sylwester Piskozub <sylwesterpiskozub@gmail.com>
Comment thread app/cli/internal/policydevel/lint.go
}

cmd.Flags().StringVarP(&policyPath, "policy", "p", ".", "Path to policy directory")
cmd.Flags().StringVarP(&policyPath, "policy", "p", "policy.yaml", "Path to policy file")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

should we do the same approach in eval?

Also, should we change init so the default created file is policy.yaml?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

actually eval works like this it seems, it doesn't load a default though.

the init might need update too

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I thought about it, I'll update it in this PR, although some docs changes will be needed as well

Piskoo added 3 commits August 7, 2025 07:39
Signed-off-by: Sylwester Piskozub <sylwesterpiskozub@gmail.com>
Signed-off-by: Sylwester Piskozub <sylwesterpiskozub@gmail.com>
Signed-off-by: Sylwester Piskozub <sylwesterpiskozub@gmail.com>
@Piskoo Piskoo marked this pull request as draft August 7, 2025 13:30
Signed-off-by: Sylwester Piskozub <sylwesterpiskozub@gmail.com>
@Piskoo Piskoo marked this pull request as ready for review August 8, 2025 09:33
@migmartri migmartri merged commit 971be38 into chainloop-dev:main Aug 8, 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.

Confusing linting behavior

3 participants