Skip to content

fix(policy-devel): inputs don't accept multiple values #2355

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.

Sign up for GitHub

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

Merged
Piskoo merged 2 commits into from
Aug 13, 2025
Merged

fix(policy-devel): inputs don't accept multiple values #2355

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d674636
fix multivalued input for eval
Piskoo Aug 13, 2025
d73285a
add gen cli ref
Piskoo Aug 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/cli/cmd/policy_develop_eval.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ evaluates the policy against the provided material or attestation.`,
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().StringSliceVar(&inputs, "input", []string{}, "Key-value pairs of policy inputs (key=value)")

@migmartri migmartri Aug 13, 2025

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.

mind explaining why the previous one didn't work? Thanks!

@Piskoo Piskoo Aug 13, 2025

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.

The issue was caused by how Cobra treats commas for StringSliceVar. Here Cobra does preprocessing where values are split with , into separate strings, where in StringArrayVar strings are kept as is.

@migmartri migmartri Aug 13, 2025

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.

so in that case we should use String no? StringSliceVar allows us to do

--input foo=bar --input bar=baz

is what I wrote correct?

@Piskoo Piskoo Aug 13, 2025

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.

StringArrayVar also allows

--input foo=bar --input bar=baz

but also allows

--input foo=bar,baz

to be treated as multi value, but has a trade-off of not being able to declare

--input foo=bar,bar=baz

because currently that becomes

[]string{"foo=bar,bar", "baz"}

Both StringSliceVar and StringArrayVar have drawbacks that can be fixed with custom parsing logic

@migmartri migmartri Aug 13, 2025

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.

right, but my question is, do we need multi input or regular strings are enough? I thought that we were asking to provide comma separated values and the engine was doing smth, I guess that not, that we want to have arrays.

@Piskoo Piskoo Aug 13, 2025

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.

We want multi input, the previous change related to commas was for escaping comma separated values, before there was no way to pass e.g.

with:
  foo: bar, baz

to get

"foo": "bar, baz"

now \, can be used to get that output. But in this case using StringArrayVar makes it so the input is preprocessed by Cobra before it is passed to engine

cmd.Flags().StringArrayVar(&inputs, "input", []string{}, "Key-value pairs of policy inputs (key=value)")
cmd.Flags().StringSliceVar(&allowedHostnames, "allowed-hostnames", []string{}, "Additional hostnames allowed for http.send requests in policies")

return cmd
Expand Down
2 changes: 1 addition & 1 deletion app/cli/documentation/cli-reference.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2848,7 +2848,7 @@ Options
--allowed-hostnames strings Additional hostnames allowed for http.send requests in policies
--annotation strings Key-value pairs of material annotations (key=value)
-h, --help help for eval
--input strings Key-value pairs of policy inputs (key=value)
--input stringArray Key-value pairs of policy inputs (key=value)
--kind string Kind of the material: ["ARTIFACT" "ATTESTATION" "BLACKDUCK_SCA_JSON" "CHAINLOOP_RUNNER_CONTEXT" "CONTAINER_IMAGE" "CSAF_INFORMATIONAL_ADVISORY" "CSAF_SECURITY_ADVISORY" "CSAF_SECURITY_INCIDENT_RESPONSE" "CSAF_VEX" "EVIDENCE" "GHAS_CODE_SCAN" "GHAS_DEPENDENCY_SCAN" "GHAS_SECRET_SCAN" "GITLAB_SECURITY_REPORT" "HELM_CHART" "JACOCO_XML" "JUNIT_XML" "OPENVEX" "SARIF" "SBOM_CYCLONEDX_JSON" "SBOM_SPDX_JSON" "SLSA_PROVENANCE" "STRING" "TWISTCLI_SCAN_JSON" "ZAP_DAST_ZIP"]
--material string Path to material or attestation file
-p, --policy string Path to custom policy file (default "policy.yaml")
Expand Down
Loading