From 3378e79afc76a34615c99a6a7a2e224649fc7c89 Mon Sep 17 00:00:00 2001 From: Javier Rodriguez Date: Mon, 10 Aug 2026 11:02:24 +0200 Subject: [PATCH 1/6] feat(cli): add flags to override policy inputs (replace, not append) Add --policy-input for setting a policy input to an inline literal value and --policy-input-from-file-replace for sourcing it from a file, both of which replace the contract-declared value instead of appending to it. This makes it possible to override a scalar policy input at run time, which the existing append-only --policy-input-from-file cannot do; the append flag is unchanged. Assisted-by: Claude Code Signed-off-by: Javier Rodriguez Chainloop-Trace-Sessions: 8f1220ca-1af0-4865-89f3-9705c9df189c --- app/cli/cmd/attestation_add.go | 96 ++++++++---- app/cli/cmd/policy_input_file_test.go | 31 +++- app/cli/documentation/cli-reference.mdx | 37 +++-- app/cli/pkg/action/attestation_add.go | 85 +++++++---- app/cli/pkg/action/attestation_add_test.go | 66 +++++++-- app/cli/pkg/action/policy_input_file.go | 123 ++++++++++++---- app/cli/pkg/action/policy_input_file_test.go | 103 ++++++++++++- pkg/policies/pfm6906_override_test.go | 93 ++++++++++++ pkg/policies/policies.go | 30 +++- pkg/policies/runtime_inputs.go | 105 +++++++++++--- pkg/policies/runtime_inputs_test.go | 145 +++++++++++++++++-- 11 files changed, 764 insertions(+), 150 deletions(-) create mode 100644 pkg/policies/pfm6906_override_test.go diff --git a/app/cli/cmd/attestation_add.go b/app/cli/cmd/attestation_add.go index 0938dc1d1..9169513fb 100644 --- a/app/cli/cmd/attestation_add.go +++ b/app/cli/cmd/attestation_add.go @@ -42,6 +42,8 @@ func newAttestationAddCmd() *cobra.Command { var annotationsFlag []string var noStrictValidation bool var policyInputFromFileFlag []string + var policyInputFromFileReplaceFlag []string + var policyInputFlag []string var maxExtractEntries int var maxExtractSize string @@ -81,7 +83,16 @@ func newAttestationAddCmd() *cobra.Command { # Scope an input to a specific policy with a : prefix so it only applies to that policy attachment. chainloop attestation add --name sigcheck --value sigcheckResult.csv --kind SYSINTERNALS_SIGCHECK \ --policy-input-from-file trusted-binaries-signed:ignored_paths=exception.csv:Path \ - --policy-input-from-file trusted-binaries-vendor-keys:third_party_paths=exception.csv:Path`, + --policy-input-from-file trusted-binaries-vendor-keys:third_party_paths=exception.csv:Path + + # Override a scalar policy input for a single run with an inline literal value. Unlike --policy-input-from-file + # (which appends), --policy-input REPLACES the contract-declared value, so a scalar input stays a scalar. + chainloop attestation add --name fuzz --value report.txt --kind RADAMSA_REPORT \ + --policy-input radamsa-min-iterations:min_iterations=10 + + # Replace (rather than append) a contract-declared list from a file column with --policy-input-from-file-replace. + chainloop attestation add --name sigcheck --value sigcheckResult.csv --kind SYSINTERNALS_SIGCHECK \ + --policy-input-from-file-replace ignored_paths=exception.csv:Path`, RunE: func(cmd *cobra.Command, _ []string) error { maxExtractSizeBytes, err := bytefmt.ToBytes(maxExtractSize) if err != nil { @@ -118,9 +129,16 @@ func newAttestationAddCmd() *cobra.Command { return err } - // Parse and resolve the policy input files (column -> policy input). - // Done once here; the resolved local paths are reused across retries. - policyInputFiles, err := resolvePolicyInputFiles(policyInputFromFileFlag) + // Parse and resolve the policy input files (column -> policy input), + // both the append and the replace variants. Done once here; the + // resolved local paths are reused across retries. + policyInputFiles, err := resolvePolicyInputFiles(policyInputFromFileFlag, policyInputFromFileReplaceFlag) + if err != nil { + return err + } + + // Parse the inline --policy-input override values (no file to resolve). + policyInputs, err := parsePolicyInputs(policyInputFlag) if err != nil { return err } @@ -143,7 +161,7 @@ func newAttestationAddCmd() *cobra.Command { return fmt.Errorf("loading resource: %w", err) } } - resp, err := a.Run(cmd.Context(), attestationID, name, rawValuePath, kind, annotations, policyInputFiles) + resp, err := a.Run(cmd.Context(), attestationID, name, rawValuePath, kind, annotations, policyInputFiles, policyInputs) if err != nil { return err } @@ -187,7 +205,9 @@ func newAttestationAddCmd() *cobra.Command { flagAttestationID(cmd) cmd.Flags().StringVar(&kind, "kind", "", fmt.Sprintf("kind of the material to be recorded: %q", schemaapi.ListAvailableMaterialKind())) cmd.Flags().BoolVar(&noStrictValidation, "no-strict-validation", false, "skip strict schema validation for structured materials (SBOM_CYCLONEDX_JSON, OPENAPI_SPEC, ASYNCAPI_SPEC, OSSF_SCORECARD_JSON)") - cmd.Flags().StringArrayVar(&policyInputFromFileFlag, "policy-input-from-file", nil, "feed a policy input from a column of a CSV or JSON file, in the format [:]=[:] (e.g. ignored_paths=exception.csv:Path); an optional : prefix scopes the input to a single policy (matched by name or ref), otherwise it applies to every declaring policy; is a single top-level column/field name and defaults to the input name; repeatable. The file is also recorded as EVIDENCE.") + cmd.Flags().StringArrayVar(&policyInputFromFileFlag, "policy-input-from-file", nil, "feed a policy input from a column of a CSV or JSON file, in the format [:]=[:] (e.g. ignored_paths=exception.csv:Path); the values are APPENDED to any contract-declared value; an optional : prefix scopes the input to a single policy (matched by name or ref), otherwise it applies to every declaring policy; is a single top-level column/field name and defaults to the input name; repeatable. The file is also recorded as EVIDENCE.") + cmd.Flags().StringArrayVar(&policyInputFromFileReplaceFlag, "policy-input-from-file-replace", nil, "like --policy-input-from-file but the extracted values REPLACE (override) any contract-declared value for the input instead of being appended to it; same [:]=[:] format; repeatable. The file is also recorded as EVIDENCE.") + cmd.Flags().StringArrayVar(&policyInputFlag, "policy-input", nil, "set a policy input to a literal value that REPLACES (overrides) any contract-declared value for the input, in the format [:]= (e.g. min_iterations=10); use this to override a scalar input at run time; an optional : prefix scopes it to a single policy (matched by name or ref), otherwise it applies to every declaring policy; repeatable.") // Optional OCI registry credentials cmd.Flags().StringVar(®istryServer, "registry-server", "", fmt.Sprintf("OCI repository server, ($%s)", registryServerEnvVarName)) @@ -213,33 +233,59 @@ func newAttestationAddCmd() *cobra.Command { return cmd } -// resolvePolicyInputFiles parses each --policy-input-from-file value and -// resolves its file reference to a local path (downloading URLs to a temporary -// file, mirroring how --value is handled). -func resolvePolicyInputFiles(raw []string) ([]*action.PolicyInputFromFile, error) { +// resolvePolicyInputFiles parses each --policy-input-from-file (append) and +// --policy-input-from-file-replace (replace) value and resolves its file +// reference to a local path (downloading URLs to a temporary file, mirroring how +// --value is handled). Both variants are returned in one slice, distinguished by +// PolicyInputFromFile.Replace. +func resolvePolicyInputFiles(rawAppend, rawReplace []string) ([]*action.PolicyInputFromFile, error) { + if len(rawAppend) == 0 && len(rawReplace) == 0 { + return nil, nil + } + + result := make([]*action.PolicyInputFromFile, 0, len(rawAppend)+len(rawReplace)) + for _, group := range []struct { + raw []string + replace bool + }{{rawAppend, false}, {rawReplace, true}} { + for _, r := range group.raw { + pif, err := action.ParsePolicyInputFromFile(r, group.replace) + if err != nil { + return nil, err + } + + path, err := resourceloader.GetPathForResource(pif.File) + if err != nil { + var uerr *resourceloader.UnrecognizedSchemeError + if errors.As(err, &uerr) { + path = pif.File + } else { + return nil, fmt.Errorf("loading policy input file: %w", err) + } + } + pif.File = path + + result = append(result, pif) + } + } + + return result, nil +} + +// parsePolicyInputs parses each --policy-input value into an inline override. +// Unlike the file variants there is no file reference to resolve. +func parsePolicyInputs(raw []string) ([]*action.PolicyInput, error) { if len(raw) == 0 { return nil, nil } - result := make([]*action.PolicyInputFromFile, 0, len(raw)) + result := make([]*action.PolicyInput, 0, len(raw)) for _, r := range raw { - pif, err := action.ParsePolicyInputFromFile(r) + pi, err := action.ParsePolicyInput(r) if err != nil { return nil, err } - - path, err := resourceloader.GetPathForResource(pif.File) - if err != nil { - var uerr *resourceloader.UnrecognizedSchemeError - if errors.As(err, &uerr) { - path = pif.File - } else { - return nil, fmt.Errorf("loading policy input file: %w", err) - } - } - pif.File = path - - result = append(result, pif) + result = append(result, pi) } return result, nil diff --git a/app/cli/cmd/policy_input_file_test.go b/app/cli/cmd/policy_input_file_test.go index 87bf3cc90..f63925aff 100644 --- a/app/cli/cmd/policy_input_file_test.go +++ b/app/cli/cmd/policy_input_file_test.go @@ -27,11 +27,12 @@ import ( func TestResolvePolicyInputFiles(t *testing.T) { testCases := []struct { - name string - raw []string - want []*action.PolicyInputFromFile - wantNil bool - wantErr bool + name string + raw []string + rawReplace []string + want []*action.PolicyInputFromFile + wantNil bool + wantErr bool }{ { name: "nil input returns nil", @@ -71,11 +72,25 @@ func TestResolvePolicyInputFiles(t *testing.T) { raw: []string{"ignored_paths=env://CHAINLOOP_TEST_DEFINITELY_UNSET_VAR"}, wantErr: true, }, + { + name: "append and replace variants both parsed, Replace set accordingly", + raw: []string{"ignored_paths=/no/exist1.csv:Path"}, + rawReplace: []string{"min_iterations=/no/exist2.csv:Iterations"}, + want: []*action.PolicyInputFromFile{ + {Input: "ignored_paths", Column: "Path", File: "/no/exist1.csv"}, + {Input: "min_iterations", Column: "Iterations", File: "/no/exist2.csv", Replace: true}, + }, + }, + { + name: "malformed replace value propagates the parse error", + rawReplace: []string{"missing-equals"}, + wantErr: true, + }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - got, err := resolvePolicyInputFiles(tc.raw) + got, err := resolvePolicyInputFiles(tc.raw, tc.rawReplace) if tc.wantErr { assert.Error(t, err) return @@ -97,7 +112,7 @@ func TestResolvePolicyInputFilesExistingFile(t *testing.T) { path := filepath.Join(dir, "exception.csv") require.NoError(t, os.WriteFile(path, []byte("Path\nc:\\a.dll\n"), 0600)) - got, err := resolvePolicyInputFiles([]string{"ignored_paths=" + path + ":Path"}) + got, err := resolvePolicyInputFiles([]string{"ignored_paths=" + path + ":Path"}, nil) require.NoError(t, err) require.Len(t, got, 1) assert.Equal(t, &action.PolicyInputFromFile{Input: "ignored_paths", Column: "Path", File: path}, got[0]) @@ -109,7 +124,7 @@ func TestResolvePolicyInputFilesExistingFile(t *testing.T) { func TestResolvePolicyInputFilesResolvesEnv(t *testing.T) { t.Setenv("CHAINLOOP_TEST_POLICY_INPUT", `["c:\\a.dll"]`) - got, err := resolvePolicyInputFiles([]string{"ignored_paths=env://CHAINLOOP_TEST_POLICY_INPUT"}) + got, err := resolvePolicyInputFiles([]string{"ignored_paths=env://CHAINLOOP_TEST_POLICY_INPUT"}, nil) require.NoError(t, err) require.Len(t, got, 1) diff --git a/app/cli/documentation/cli-reference.mdx b/app/cli/documentation/cli-reference.mdx index 89558b5b5..c42b9c7ca 100755 --- a/app/cli/documentation/cli-reference.mdx +++ b/app/cli/documentation/cli-reference.mdx @@ -254,24 +254,35 @@ Scope an input to a specific policy with a : prefix so it only applies t chainloop attestation add --name sigcheck --value sigcheckResult.csv --kind SYSINTERNALS_SIGCHECK \ --policy-input-from-file trusted-binaries-signed:ignored_paths=exception.csv:Path \ --policy-input-from-file trusted-binaries-vendor-keys:third_party_paths=exception.csv:Path + +Override a scalar policy input for a single run with an inline literal value. Unlike --policy-input-from-file +(which appends), --policy-input REPLACES the contract-declared value, so a scalar input stays a scalar. +chainloop attestation add --name fuzz --value report.txt --kind RADAMSA_REPORT \ +--policy-input radamsa-min-iterations:min_iterations=10 + +Replace (rather than append) a contract-declared list from a file column with --policy-input-from-file-replace. +chainloop attestation add --name sigcheck --value sigcheckResult.csv --kind SYSINTERNALS_SIGCHECK \ +--policy-input-from-file-replace ignored_paths=exception.csv:Path ``` Options ``` ---annotation strings additional annotation in the format of key=value ---attestation-id string Unique identifier of the in-progress attestation --h, --help help for add ---kind string kind of the material to be recorded: ["ARTIFACT" "ASYNCAPI_SPEC" "ATTESTATION" "BLACKDUCK_SCA_JSON" "CERTCC_DRANZER" "CHAINLOOP_AI_AGENT_CONFIG" "CHAINLOOP_AI_CODING_SESSION" "CHAINLOOP_PR_INFO" "CHAINLOOP_RUNNER_CONTEXT" "CHECKMARX_JSON" "COBERTURA_XML" "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" "GITLEAKS_JSON" "GRAPHQL_SPEC" "HELM_CHART" "JACOCO_XML" "JUNIT_XML" "OPENAPI_SPEC" "OPENVEX" "OSSF_SCORECARD_JSON" "RADAMSA_CRASHES" "RADAMSA_REPORT" "SARIF" "SBOM_CYCLONEDX_JSON" "SBOM_SPDX_JSON" "SLSA_PROVENANCE" "STRING" "SYSINTERNALS_ACCESSCHK" "SYSINTERNALS_SIGCHECK" "TRUFFLEHOG_JSON" "TWISTCLI_SCAN_JSON" "YELP_DETECT_SECRETS_BASELINE" "ZAP_DAST_ZIP"] ---max-extract-entries int max number of files to extract when --value is an archive (default 10000) ---max-extract-size string max total uncompressed size to extract when --value is an archive (default "1GiB") ---name string name of the material as shown in the contract ---no-strict-validation skip strict schema validation for structured materials (SBOM_CYCLONEDX_JSON, OPENAPI_SPEC, ASYNCAPI_SPEC, OSSF_SCORECARD_JSON) ---policy-input-from-file stringArray feed a policy input from a column of a CSV or JSON file, in the format [:]=[:] (e.g. ignored_paths=exception.csv:Path); an optional : prefix scopes the input to a single policy (matched by name or ref), otherwise it applies to every declaring policy; is a single top-level column/field name and defaults to the input name; repeatable. The file is also recorded as EVIDENCE. ---registry-password string registry password, ($CHAINLOOP_REGISTRY_PASSWORD) ---registry-server string OCI repository server, ($CHAINLOOP_REGISTRY_SERVER) ---registry-username string registry username, ($CHAINLOOP_REGISTRY_USERNAME) ---value string value to be recorded +--annotation strings additional annotation in the format of key=value +--attestation-id string Unique identifier of the in-progress attestation +-h, --help help for add +--kind string kind of the material to be recorded: ["ARTIFACT" "ASYNCAPI_SPEC" "ATTESTATION" "BLACKDUCK_SCA_JSON" "CERTCC_DRANZER" "CHAINLOOP_AI_AGENT_CONFIG" "CHAINLOOP_AI_CODING_SESSION" "CHAINLOOP_PR_INFO" "CHAINLOOP_RUNNER_CONTEXT" "CHECKMARX_JSON" "COBERTURA_XML" "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" "GITLEAKS_JSON" "GRAPHQL_SPEC" "HELM_CHART" "JACOCO_XML" "JUNIT_XML" "OPENAPI_SPEC" "OPENVEX" "OSSF_SCORECARD_JSON" "RADAMSA_CRASHES" "RADAMSA_REPORT" "SARIF" "SBOM_CYCLONEDX_JSON" "SBOM_SPDX_JSON" "SLSA_PROVENANCE" "STRING" "SYSINTERNALS_ACCESSCHK" "SYSINTERNALS_SIGCHECK" "TRUFFLEHOG_JSON" "TWISTCLI_SCAN_JSON" "YELP_DETECT_SECRETS_BASELINE" "ZAP_DAST_ZIP"] +--max-extract-entries int max number of files to extract when --value is an archive (default 10000) +--max-extract-size string max total uncompressed size to extract when --value is an archive (default "1GiB") +--name string name of the material as shown in the contract +--no-strict-validation skip strict schema validation for structured materials (SBOM_CYCLONEDX_JSON, OPENAPI_SPEC, ASYNCAPI_SPEC, OSSF_SCORECARD_JSON) +--policy-input stringArray set a policy input to a literal value that REPLACES (overrides) any contract-declared value for the input, in the format [:]= (e.g. min_iterations=10); use this to override a scalar input at run time; an optional : prefix scopes it to a single policy (matched by name or ref), otherwise it applies to every declaring policy; repeatable. +--policy-input-from-file stringArray feed a policy input from a column of a CSV or JSON file, in the format [:]=[:] (e.g. ignored_paths=exception.csv:Path); the values are APPENDED to any contract-declared value; an optional : prefix scopes the input to a single policy (matched by name or ref), otherwise it applies to every declaring policy; is a single top-level column/field name and defaults to the input name; repeatable. The file is also recorded as EVIDENCE. +--policy-input-from-file-replace stringArray like --policy-input-from-file but the extracted values REPLACE (override) any contract-declared value for the input instead of being appended to it; same [:]=[:] format; repeatable. The file is also recorded as EVIDENCE. +--registry-password string registry password, ($CHAINLOOP_REGISTRY_PASSWORD) +--registry-server string OCI repository server, ($CHAINLOOP_REGISTRY_SERVER) +--registry-username string registry username, ($CHAINLOOP_REGISTRY_USERNAME) +--value string value to be recorded ``` Options inherited from parent commands diff --git a/app/cli/pkg/action/attestation_add.go b/app/cli/pkg/action/attestation_add.go index 15636f03e..3d15e04c0 100644 --- a/app/cli/pkg/action/attestation_add.go +++ b/app/cli/pkg/action/attestation_add.go @@ -101,16 +101,16 @@ func NewAttestationAdd(cfg *AttestationAddOpts) (*AttestationAdd, error) { var ErrAttestationNotInitialized = errors.New("attestation not yet initialized") -func (action *AttestationAdd) Run(ctx context.Context, attestationID, materialName, materialValue, materialType string, annotations map[string]string, policyInputFiles []*PolicyInputFromFile) ([]*AttestationStatusMaterial, error) { +func (action *AttestationAdd) Run(ctx context.Context, attestationID, materialName, materialValue, materialType string, annotations map[string]string, policyInputFiles []*PolicyInputFromFile, policyInputs []*PolicyInput) ([]*AttestationStatusMaterial, error) { // initialize the crafter. If attestation-id is provided we assume the attestation is performed using remote state crafter, err := newCrafter(&newCrafterStateOpts{enableRemoteState: (attestationID != ""), localStatePath: action.localStatePath}, action.CPConnection, action.opts...) if err != nil { return nil, fmt.Errorf("failed to load crafter: %w", err) } - // Resolve runtime policy inputs from the provided files before adding the - // material, so a malformed file aborts the add early. - runtimeInputs, err := buildRuntimeInputs(policyInputFiles) + // Resolve runtime policy inputs from the provided files and inline values + // before adding the material, so a malformed file aborts the add early. + runtimeInputs, err := buildRuntimeInputs(policyInputFiles, policyInputs) if err != nil { return nil, err } @@ -163,7 +163,7 @@ func (action *AttestationAdd) Run(ctx context.Context, attestationID, materialNa // The runtime inputs still apply to every exploded material's policy // evaluation (they flow through addOpts); only the per-input EVIDENCE // materials are not recorded on the explode path. - action.Logger.Warn().Msg("--policy-input-from-file values apply to policy evaluation but are not recorded as evidence materials when expanding an archive") + action.Logger.Warn().Msg("policy input files apply to policy evaluation but are not recorded as evidence materials when expanding an archive") } limits := materials.ArchiveLimits{MaxEntries: action.maxExtractEntries, MaxTotalSize: action.maxExtractSize} // AddMaterialsFromArchive also records the source archive as an EVIDENCE @@ -249,7 +249,8 @@ func shouldExplode(materialType, value string) (materials.ArchiveFormat, error) // returns nil when there are none. Defined at package scope so it can name the // crafter package type (the Run method shadows it with a local variable). func runtimeInputAddOpts(runtimeInputs *policies.RuntimeInputs) []crafter.AddOpt { - if runtimeInputs == nil || (len(runtimeInputs.Global) == 0 && len(runtimeInputs.Scoped) == 0) { + if runtimeInputs == nil || (len(runtimeInputs.Global) == 0 && len(runtimeInputs.Scoped) == 0 && + len(runtimeInputs.GlobalOverride) == 0 && len(runtimeInputs.ScopedOverride) == 0) { return nil } return []crafter.AddOpt{crafter.WithRuntimeInputs(runtimeInputs)} @@ -262,43 +263,77 @@ func withSourceArchiveEvidence(opts []crafter.AddOpt) []crafter.AddOpt { return append(opts, crafter.WithSourceArchiveEvidence()) } -// buildRuntimeInputs reads each policy input file and returns the extracted -// values grouped for the policy engine: unscoped entries under Global and -// policy-scoped entries under Scoped[policy]. Values are newline-joined and -// accumulated via policies.MergeRuntimeInputs so repeated inputs merge using the -// same multi-value encoding the engine expects (it splits inputs back on -// newlines and commas). As with contract-declared arguments, individual values -// must not embed those delimiters; path globs, the intended use, never do. -func buildRuntimeInputs(policyInputFiles []*PolicyInputFromFile) (*policies.RuntimeInputs, error) { - if len(policyInputFiles) == 0 { +// buildRuntimeInputs reads each policy input file and combines it with the +// inline --policy-input values, returning them grouped for the policy engine. +// Append-mode file inputs (--policy-input-from-file) go under Global/Scoped and +// are newline-joined via policies.MergeRuntimeInputs so repeated inputs merge +// using the multi-value encoding the engine expects (it splits inputs back on +// newlines and commas). Replace-mode file inputs (--policy-input-from-file-replace) +// and every inline value go under GlobalOverride/ScopedOverride and replace the +// contract value instead of appending, keeping a scalar override a scalar. As +// with contract-declared arguments, individual append values must not embed +// those delimiters; path globs, the intended use, never do. +func buildRuntimeInputs(policyInputFiles []*PolicyInputFromFile, policyInputs []*PolicyInput) (*policies.RuntimeInputs, error) { + if len(policyInputFiles) == 0 && len(policyInputs) == 0 { return nil, nil } ri := &policies.RuntimeInputs{ - Global: map[string]string{}, - Scoped: map[string]map[string]string{}, + Global: map[string]string{}, + Scoped: map[string]map[string]string{}, + GlobalOverride: map[string]string{}, + ScopedOverride: map[string]map[string]string{}, } + for _, pif := range policyInputFiles { values, err := ExtractColumnValues(pif.File, pif.Column) if err != nil { return nil, fmt.Errorf("extracting %q from %q: %w", pif.Column, pif.File, err) } - // Unscoped entries go to Global; policy-scoped entries to their own - // Scoped[policy] map. Because global and scoped values live in separate - // maps, they never collide here even when they share an input name; - // forPolicy is what later merges a policy's scoped values over Global. - add := map[string]string{pif.Input: strings.Join(values, "\n")} - if pif.Policy == "" { - ri.Global = policies.MergeRuntimeInputs(ri.Global, add) + joined := strings.Join(values, "\n") + if pif.Replace { + addOverrideInput(ri, pif.Policy, pif.Input, joined) } else { - ri.Scoped[pif.Policy] = policies.MergeRuntimeInputs(ri.Scoped[pif.Policy], add) + addAppendInput(ri, pif.Policy, pif.Input, joined) } } + for _, pi := range policyInputs { + addOverrideInput(ri, pi.Policy, pi.Input, pi.Value) + } + return ri, nil } +// addAppendInput accumulates an append-mode runtime input. Unscoped entries go +// to Global; policy-scoped entries to their own Scoped[policy] map. Because +// global and scoped values live in separate maps they never collide here even +// when they share an input name; forPolicy is what later merges a policy's +// scoped values over Global. +func addAppendInput(ri *policies.RuntimeInputs, policy, input, value string) { + add := map[string]string{input: value} + if policy == "" { + ri.Global = policies.MergeRuntimeInputs(ri.Global, add) + } else { + ri.Scoped[policy] = policies.MergeRuntimeInputs(ri.Scoped[policy], add) + } +} + +// addOverrideInput records a replace-mode runtime input. When the same key is +// supplied more than once the last value wins, since a replace — unlike an +// append — has no meaningful accumulation. +func addOverrideInput(ri *policies.RuntimeInputs, policy, input, value string) { + if policy == "" { + ri.GlobalOverride[input] = value + return + } + if ri.ScopedOverride[policy] == nil { + ri.ScopedOverride[policy] = map[string]string{} + } + ri.ScopedOverride[policy][input] = value +} + // addPolicyInputEvidence adds each policy input file as an EVIDENCE material, // cross-linked with the evaluated material in both directions via the // chainloop.material.references annotation: each evidence material points at the diff --git a/app/cli/pkg/action/attestation_add_test.go b/app/cli/pkg/action/attestation_add_test.go index 7dcb2f35e..3da37ab19 100644 --- a/app/cli/pkg/action/attestation_add_test.go +++ b/app/cli/pkg/action/attestation_add_test.go @@ -185,7 +185,7 @@ func TestAddReference(t *testing.T) { } func TestBuildRuntimeInputsNil(t *testing.T) { - got, err := buildRuntimeInputs(nil) + got, err := buildRuntimeInputs(nil, nil) assert.NoError(t, err) assert.Nil(t, got) } @@ -196,14 +196,22 @@ func TestBuildRuntimeInputs(t *testing.T) { path := filepath.Join(dir, "exception.csv") require.NoError(t, os.WriteFile(path, []byte("Path,Extra\na.dll,x\nb.dll,y\n"), 0600)) + // Expected joined values for the Path and Extra columns respectively. + const ( + wantAB = "a.dll\nb.dll" + wantXY = "x\ny" + ) + t.Run("unscoped inputs land in Global", func(t *testing.T) { got, err := buildRuntimeInputs([]*PolicyInputFromFile{ {Input: "ignored_paths", Column: "Path", File: path}, - }) + }, nil) require.NoError(t, err) assert.Equal(t, &policies.RuntimeInputs{ - Global: map[string]string{"ignored_paths": "a.dll\nb.dll"}, - Scoped: map[string]map[string]string{}, + Global: map[string]string{"ignored_paths": wantAB}, + Scoped: map[string]map[string]string{}, + GlobalOverride: map[string]string{}, + ScopedOverride: map[string]map[string]string{}, }, got) }) @@ -211,14 +219,16 @@ func TestBuildRuntimeInputs(t *testing.T) { got, err := buildRuntimeInputs([]*PolicyInputFromFile{ {Policy: "trusted-binaries-signed", Input: "ignored_paths", Column: "Path", File: path}, {Policy: "trusted-binaries-vendor-keys", Input: "third_party_paths", Column: "Path", File: path}, - }) + }, nil) require.NoError(t, err) assert.Equal(t, &policies.RuntimeInputs{ Global: map[string]string{}, Scoped: map[string]map[string]string{ - "trusted-binaries-signed": {"ignored_paths": "a.dll\nb.dll"}, - "trusted-binaries-vendor-keys": {"third_party_paths": "a.dll\nb.dll"}, + "trusted-binaries-signed": {"ignored_paths": wantAB}, + "trusted-binaries-vendor-keys": {"third_party_paths": wantAB}, }, + GlobalOverride: map[string]string{}, + ScopedOverride: map[string]map[string]string{}, }, got) }) @@ -226,7 +236,7 @@ func TestBuildRuntimeInputs(t *testing.T) { got, err := buildRuntimeInputs([]*PolicyInputFromFile{ {Policy: "p", Input: "ignored_paths", Column: "Path", File: path}, {Policy: "p", Input: "ignored_paths", Column: "Extra", File: path}, - }) + }, nil) require.NoError(t, err) assert.Equal(t, map[string]string{"ignored_paths": "a.dll\nb.dll\nx\ny"}, got.Scoped["p"]) }) @@ -235,9 +245,45 @@ func TestBuildRuntimeInputs(t *testing.T) { got, err := buildRuntimeInputs([]*PolicyInputFromFile{ {Input: "ignored_paths", Column: "Path", File: path}, {Policy: "p", Input: "ignored_paths", Column: "Extra", File: path}, + }, nil) + require.NoError(t, err) + assert.Equal(t, map[string]string{"ignored_paths": wantAB}, got.Global) + assert.Equal(t, map[string]string{"ignored_paths": wantXY}, got.Scoped["p"]) + }) + + t.Run("replace-mode file inputs land in the override maps", func(t *testing.T) { + got, err := buildRuntimeInputs([]*PolicyInputFromFile{ + {Input: "ignored_paths", Column: "Path", File: path, Replace: true}, + {Policy: "p", Input: "third_party_paths", Column: "Extra", File: path, Replace: true}, + }, nil) + require.NoError(t, err) + assert.Empty(t, got.Global) + assert.Empty(t, got.Scoped) + assert.Equal(t, map[string]string{"ignored_paths": wantAB}, got.GlobalOverride) + assert.Equal(t, map[string]string{"third_party_paths": wantXY}, got.ScopedOverride["p"]) + }) + + t.Run("inline values land in the override maps, last write wins", func(t *testing.T) { + got, err := buildRuntimeInputs(nil, []*PolicyInput{ + {Input: testInputMinIter, Value: "5"}, + {Input: testInputMinIter, Value: "10"}, + {Policy: testPolicyRadamsa, Input: testInputMinIter, Value: "20"}, + }) + require.NoError(t, err) + assert.Equal(t, map[string]string{testInputMinIter: "10"}, got.GlobalOverride) + assert.Equal(t, map[string]string{testInputMinIter: "20"}, got.ScopedOverride[testPolicyRadamsa]) + }) + + t.Run("append files, replace files and inline values coexist", func(t *testing.T) { + got, err := buildRuntimeInputs([]*PolicyInputFromFile{ + {Input: "ignored_paths", Column: "Path", File: path}, + {Input: "extra_paths", Column: "Extra", File: path, Replace: true}, + }, []*PolicyInput{ + {Policy: testPolicyRadamsa, Input: testInputMinIter, Value: "10"}, }) require.NoError(t, err) - assert.Equal(t, map[string]string{"ignored_paths": "a.dll\nb.dll"}, got.Global) - assert.Equal(t, map[string]string{"ignored_paths": "x\ny"}, got.Scoped["p"]) + assert.Equal(t, map[string]string{"ignored_paths": wantAB}, got.Global) + assert.Equal(t, map[string]string{"extra_paths": wantXY}, got.GlobalOverride) + assert.Equal(t, map[string]string{testInputMinIter: "10"}, got.ScopedOverride[testPolicyRadamsa]) }) } diff --git a/app/cli/pkg/action/policy_input_file.go b/app/cli/pkg/action/policy_input_file.go index 9f7424aca..68498d8bc 100644 --- a/app/cli/pkg/action/policy_input_file.go +++ b/app/cli/pkg/action/policy_input_file.go @@ -27,8 +27,8 @@ import ( "github.com/chainloop-dev/chainloop/pkg/tabular" ) -// PolicyInputFromFile describes a single --policy-input-from-file flag value: a -// policy input name fed from a named column of a CSV or JSON file. +// PolicyInputFromFile describes a single --policy-input-from-file[-replace] flag +// value: a policy input name fed from a named column of a CSV or JSON file. type PolicyInputFromFile struct { // Policy optionally scopes the input to a specific policy (its name or ref). // Empty means the input is global and applies to every declaring policy. @@ -39,6 +39,25 @@ type PolicyInputFromFile struct { Column string // File is the source CSV or JSON file path. File string + // Replace reports whether the extracted values replace the contract-declared + // value for the input (--policy-input-from-file-replace) rather than being + // appended to it (--policy-input-from-file). + Replace bool +} + +// PolicyInput describes a single --policy-input flag value: a policy input name +// set to a literal value supplied directly on the command line. The value +// always replaces (overrides) any contract-declared value for the input rather +// than being appended, which is what makes a scalar input overridable at run +// time. +type PolicyInput struct { + // Policy optionally scopes the input to a specific policy (its name or ref). + // Empty means the input is global and applies to every declaring policy. + Policy string + // Input is the destination policy input name (e.g. "min_iterations"). + Input string + // Value is the literal value to set. + Value string } // scopeDelimiter separates an optional policy scope from the input name on the @@ -53,6 +72,39 @@ const scopeDelimiter = ":" // was omitted, which would otherwise be mistaken for the digest. const digestScheme = "@sha256" +// parsePolicyInputKey splits the left-hand side of a policy-input flag value +// (everything before "=") into an optional ":" scope prefix and the +// input name. The optional prefix scopes the input to a single policy (matched +// against its name or ref); without it the input is global. Because a policy ref +// may itself contain ":" (scheme, digest) but an input name never does, the +// scope is taken as everything before the *last* ":". flag names the originating +// CLI flag so errors point at the right one. +func parsePolicyInputKey(lhs, raw, flag string) (policy, input string, err error) { + if i := strings.LastIndex(lhs, scopeDelimiter); i >= 0 { + policy = strings.TrimSpace(lhs[:i]) + input = strings.TrimSpace(lhs[i+1:]) + if policy == "" { + return "", "", fmt.Errorf("invalid %s %q: missing policy scope before %q", flag, raw, scopeDelimiter) + } + // A bare "@sha256:" (no input) would be mis-split into + // policy "@sha256" and input ""; reject it with guidance. + // The right-hand side is left unnamed so the message stays accurate for + // every flag (a file for --policy-input-from-file[-replace], a literal + // value for --policy-input). + if strings.HasSuffix(policy, digestScheme) { + return "", "", fmt.Errorf("invalid %s %q: versioned policy scope is missing an input name; expected an input name after the digest, as in @sha256::", flag, raw) + } + } else { + input = strings.TrimSpace(lhs) + } + + if input == "" { + return "", "", fmt.Errorf("invalid %s %q: missing input name", flag, raw) + } + + return policy, input, nil +} + // ParsePolicyInputFromFile parses a single flag value of the form // "[:]=[:]". The optional ":" prefix scopes // the input to a single policy (matched against its name or ref); without it the @@ -63,37 +115,28 @@ const digestScheme = "@sha256" // key. The column is the segment after the last ":"; since a column name never // contains a path separator, a trailing ":<...>" whose ":" belongs to the file // (a Windows drive letter like C:\data\... or a URL scheme like https://) is not -// mistaken for a column. -func ParsePolicyInputFromFile(raw string) (*PolicyInputFromFile, error) { +// mistaken for a column. replace records whether the values replace the contract +// value (--policy-input-from-file-replace) rather than being appended to it +// (--policy-input-from-file), and only affects the flag name shown in errors. +func ParsePolicyInputFromFile(raw string, replace bool) (*PolicyInputFromFile, error) { + flag := "--policy-input-from-file" + if replace { + flag = "--policy-input-from-file-replace" + } + lhs, rhs, found := strings.Cut(raw, "=") if !found { - return nil, fmt.Errorf("invalid --policy-input-from-file %q: expected [:]=[:]", raw) + return nil, fmt.Errorf("invalid %s %q: expected [:]=[:]", flag, raw) } - // Split off the optional ":" scope prefix at the last ":": a policy - // ref may contain colons (scheme, digest) but the input name never does. - var policy, input string - if i := strings.LastIndex(lhs, scopeDelimiter); i >= 0 { - policy = strings.TrimSpace(lhs[:i]) - input = strings.TrimSpace(lhs[i+1:]) - if policy == "" { - return nil, fmt.Errorf("invalid --policy-input-from-file %q: missing policy scope before %q", raw, scopeDelimiter) - } - // A bare "@sha256:" (no input) would be mis-split into - // policy "@sha256" and input ""; reject it with guidance. - if strings.HasSuffix(policy, digestScheme) { - return nil, fmt.Errorf("invalid --policy-input-from-file %q: versioned policy scope is missing an input name; expected @sha256::=", raw) - } - } else { - input = strings.TrimSpace(lhs) + policy, input, err := parsePolicyInputKey(lhs, raw, flag) + if err != nil { + return nil, err } rhs = strings.TrimSpace(rhs) - if input == "" { - return nil, fmt.Errorf("invalid --policy-input-from-file %q: missing input name", raw) - } if rhs == "" { - return nil, fmt.Errorf("invalid --policy-input-from-file %q: missing file path", raw) + return nil, fmt.Errorf("invalid %s %q: missing file path", flag, raw) } // Default the column to the input name; override it only when a ":" @@ -108,10 +151,36 @@ func ParsePolicyInputFromFile(raw string) (*PolicyInputFromFile, error) { } if file == "" { - return nil, fmt.Errorf("invalid --policy-input-from-file %q: missing file path", raw) + return nil, fmt.Errorf("invalid %s %q: missing file path", flag, raw) + } + + return &PolicyInputFromFile{Policy: policy, Input: input, Column: column, File: file, Replace: replace}, nil +} + +// ParsePolicyInput parses a single --policy-input flag value of the form +// "[:]=", where is a literal set directly on the +// command line. The optional ":" prefix has the same scoping semantics +// as --policy-input-from-file. The value always overrides (replaces) any +// contract-declared value for the input. +func ParsePolicyInput(raw string) (*PolicyInput, error) { + const flag = "--policy-input" + + lhs, rhs, found := strings.Cut(raw, "=") + if !found { + return nil, fmt.Errorf("invalid %s %q: expected [:]=", flag, raw) + } + + policy, input, err := parsePolicyInputKey(lhs, raw, flag) + if err != nil { + return nil, err + } + + value := strings.TrimSpace(rhs) + if value == "" { + return nil, fmt.Errorf("invalid %s %q: missing value", flag, raw) } - return &PolicyInputFromFile{Policy: policy, Input: input, Column: column, File: file}, nil + return &PolicyInput{Policy: policy, Input: input, Value: value}, nil } // ExtractColumnValues reads the given CSV or JSON file and returns the values of diff --git a/app/cli/pkg/action/policy_input_file_test.go b/app/cli/pkg/action/policy_input_file_test.go index 2ba267a1e..c752c5e2a 100644 --- a/app/cli/pkg/action/policy_input_file_test.go +++ b/app/cli/pkg/action/policy_input_file_test.go @@ -24,6 +24,12 @@ import ( "github.com/stretchr/testify/require" ) +// Repeated domain strings across the action test files, extracted to satisfy goconst. +const ( + testPolicyRadamsa = "radamsa-min-iterations" + testInputMinIter = "min_iterations" +) + func TestParsePolicyInputFromFile(t *testing.T) { testCases := []struct { name string @@ -125,7 +131,102 @@ func TestParsePolicyInputFromFile(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - got, err := ParsePolicyInputFromFile(tc.raw) + got, err := ParsePolicyInputFromFile(tc.raw, false) + if tc.wantErr { + assert.Error(t, err) + return + } + require.NoError(t, err) + assert.Equal(t, tc.want, got) + }) + } +} + +func TestParsePolicyInputFromFileReplace(t *testing.T) { + // The replace variant shares all parsing with the append variant and only + // sets Replace=true; a representative case plus the flag-name in the error. + got, err := ParsePolicyInputFromFile("radamsa-min-iterations:min_iterations=values.csv:Iterations", true) + require.NoError(t, err) + assert.Equal(t, &PolicyInputFromFile{ + Policy: testPolicyRadamsa, + Input: testInputMinIter, + Column: "Iterations", + File: "values.csv", + Replace: true, + }, got) + + _, err = ParsePolicyInputFromFile(testInputMinIter, true) + require.Error(t, err) + assert.Contains(t, err.Error(), "--policy-input-from-file-replace") +} + +func TestParsePolicyInput(t *testing.T) { + testCases := []struct { + name string + raw string + want *PolicyInput + wantErr bool + }{ + { + name: "input and value", + raw: "min_iterations=10", + want: &PolicyInput{Input: testInputMinIter, Value: "10"}, + }, + { + name: "policy-scoped input and value", + raw: "radamsa-min-iterations:min_iterations=10", + want: &PolicyInput{Policy: testPolicyRadamsa, Input: testInputMinIter, Value: "10"}, + }, + { + name: "policy-scoped input pinned to a version", + raw: "radamsa-min-iterations@sha256:deadbeef:min_iterations=10", + want: &PolicyInput{Policy: "radamsa-min-iterations@sha256:deadbeef", Input: testInputMinIter, Value: "10"}, + }, + { + name: "provider-style scope keeps its colon", + raw: "builtin:radamsa-min-iterations:min_iterations=10", + want: &PolicyInput{Policy: "builtin:radamsa-min-iterations", Input: testInputMinIter, Value: "10"}, + }, + { + name: "value with a comma is kept verbatim", + raw: "ignored_paths=a,b,c", + want: &PolicyInput{Input: "ignored_paths", Value: "a,b,c"}, + }, + { + name: "surrounding whitespace trimmed", + raw: " radamsa-min-iterations : min_iterations = 10 ", + want: &PolicyInput{Policy: testPolicyRadamsa, Input: testInputMinIter, Value: "10"}, + }, + { + name: "missing equals", + raw: "min_iterations:10", + wantErr: true, + }, + { + name: "missing input name", + raw: "=10", + wantErr: true, + }, + { + name: "missing value", + raw: "min_iterations=", + wantErr: true, + }, + { + name: "empty policy scope", + raw: ":min_iterations=10", + wantErr: true, + }, + { + name: "versioned scope missing an input name", + raw: "radamsa-min-iterations@sha256:deadbeef=10", + wantErr: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + got, err := ParsePolicyInput(tc.raw) if tc.wantErr { assert.Error(t, err) return diff --git a/pkg/policies/pfm6906_override_test.go b/pkg/policies/pfm6906_override_test.go new file mode 100644 index 000000000..a8ddb45e3 --- /dev/null +++ b/pkg/policies/pfm6906_override_test.go @@ -0,0 +1,93 @@ +// +// Copyright 2026 The Chainloop Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package policies + +import ( + "context" + "testing" + + "github.com/chainloop-dev/chainloop/pkg/policies/engine" + "github.com/chainloop-dev/chainloop/pkg/policies/engine/rego" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// radamsaMinIterationsPolicy mirrors the failing line of the real +// radamsa-min-iterations policy: to_number(input.args.min_iterations). With a +// multi-value (array) argument to_number raises an eval_type_error under the +// engine's strict-builtin-errors mode; with a scalar it evaluates and the +// numeric comparison runs. +const radamsaMinIterationsPolicy = `package main +import rego.v1 + +result := { + "skipped": false, + "violations": violations, + "skip_reason": "", +} + +violations contains msg if { + n := to_number(input.args.min_iterations) + n < 100 + msg := sprintf("min_iterations %v is below the required 100", [n]) +} +` + +// TestPFM6906ScalarOverrideFixesToNumberArrayError reproduces the exact failure +// reported in PFM-6906 and proves the new --policy-input override path fixes it, +// exercising the real rego engine end to end. +// +// - Contract declares min_iterations=100 in its `with:`. +// - The old --policy-input-from-file behavior APPENDS the runtime value, so +// getInputArguments yields the array ["100","10"] and to_number fails with +// "eval_type_error: to_number ... got array" — the reported error. +// - The new --policy-input (and --policy-input-from-file-replace) behavior +// REPLACES the value via OverrideRuntimeInputs, so min_iterations stays the +// scalar "10", to_number succeeds, and the policy evaluates cleanly. +func TestPFM6906ScalarOverrideFixesToNumberArrayError(t *testing.T) { + eng := rego.NewEngine() + policy := &engine.Policy{Name: "radamsa-min-iterations", Source: []byte(radamsaMinIterationsPolicy)} + material := []byte(`{}`) + + contractWith := map[string]string{"min_iterations": "100"} + runtime := map[string]string{"min_iterations": "10"} + + t.Run("append path reproduces the to_number array error", func(t *testing.T) { + with := MergeRuntimeInputs(contractWith, runtime) + args := getInputArguments(with) + + // The append merge turns the scalar into a two-element list. + require.Equal(t, []string{"100", "10"}, args["min_iterations"]) + + _, err := eng.Verify(context.Background(), policy, material, args) + require.Error(t, err) + assert.Contains(t, err.Error(), "to_number") + assert.Contains(t, err.Error(), "array") + }) + + t.Run("override path keeps a scalar and evaluates cleanly", func(t *testing.T) { + with := OverrideRuntimeInputs(contractWith, runtime) + args := getInputArguments(with) + + // The override replaces the contract value; it stays a single scalar. + require.Equal(t, "10", args["min_iterations"]) + + res, err := eng.Verify(context.Background(), policy, material, args) + require.NoError(t, err) + require.Len(t, res.Violations, 1) + assert.Contains(t, res.Violations[0].Violation, "min_iterations 10 is below the required 100") + }) +} diff --git a/pkg/policies/policies.go b/pkg/policies/policies.go index d11c81586..efabfe2f2 100644 --- a/pkg/policies/policies.go +++ b/pkg/policies/policies.go @@ -386,11 +386,14 @@ func (pv *PolicyVerifier) evaluatePolicyAttachment(ctx context.Context, attachme } // Resolve the runtime-supplied inputs that apply to this policy (global plus - // any scoped to its name/ref) and merge them additively onto the contract - // arguments before computing the effective values. - effectiveRuntime, matchedScopes := opts.runtimeInputs.forPolicy(policy.GetMetadata().GetName(), attachment.GetRef()) + // any scoped to its name/ref). Append-mode inputs merge additively onto the + // contract arguments; replace-mode inputs are then applied on top, replacing + // whatever value (contract or appended) the key held so an override does not + // collapse into a multi-value list through append-time newline joining. + appendRuntime, replaceRuntime, matchedScopes := opts.runtimeInputs.forPolicy(policy.GetMetadata().GetName(), attachment.GetRef()) opts.scopeTracker.mark(matchedScopes...) - with := MergeRuntimeInputs(attachment.GetWith(), effectiveRuntime) + with := MergeRuntimeInputs(attachment.GetWith(), appendRuntime) + with = OverrideRuntimeInputs(with, replaceRuntime) args, err := ComputeArguments(policy.GetMetadata().GetName(), policy.GetSpec().GetInputs(), with, opts.bindings, pv.logger) if err != nil { @@ -400,13 +403,26 @@ func (pv *PolicyVerifier) evaluatePolicyAttachment(ctx context.Context, attachme // Record which runtime inputs actually applied to this policy (i.e. made it // into the computed args because the policy declares them). The values // themselves live in `with`; this only flags the overridden input names. + // Skipped entirely on the common path with no runtime inputs to avoid a + // per-attachment map allocation. var runtimeInputOverrides []string - for k := range effectiveRuntime { - if _, ok := args[k]; ok { + if len(appendRuntime) > 0 || len(replaceRuntime) > 0 { + overrideNames := make(map[string]struct{}, len(appendRuntime)+len(replaceRuntime)) + for k := range appendRuntime { + if _, ok := args[k]; ok { + overrideNames[k] = struct{}{} + } + } + for k := range replaceRuntime { + if _, ok := args[k]; ok { + overrideNames[k] = struct{}{} + } + } + for k := range overrideNames { runtimeInputOverrides = append(runtimeInputOverrides, k) } + slices.Sort(runtimeInputOverrides) } - slices.Sort(runtimeInputOverrides) sources := make([]string, 0) evalResults := make([]*engine.EvaluationResult, 0) diff --git a/pkg/policies/runtime_inputs.go b/pkg/policies/runtime_inputs.go index 67b478373..7795f0b2f 100644 --- a/pkg/policies/runtime_inputs.go +++ b/pkg/policies/runtime_inputs.go @@ -22,36 +22,71 @@ import ( ) // RuntimeInputs holds policy input values supplied at runtime (e.g. via -// --policy-input-from-file). Inputs are either global (applied to every policy -// attachment that declares them) or scoped to a specific policy (applied only -// to the attachment whose metadata name or ref matches the scope key). +// --policy-input-from-file, --policy-input-from-file-replace or --policy-input). +// Inputs are either global (applied to every policy attachment that declares +// them) or scoped to a specific policy (applied only to the attachment whose +// metadata name or ref matches the scope key). Independently, each input is +// merged onto the contract value either additively (append) or by replacing it +// (override); the two modes live in separate maps so the same input name can be +// appended by one flag and never collides with an override of another. type RuntimeInputs struct { - // Global inputs, keyed by input name. + // Global holds append-mode inputs, keyed by input name. Global map[string]string - // Scoped inputs, keyed by policy scope (a policy name or ref) then input name. + // Scoped holds append-mode inputs, keyed by policy scope (a policy name or + // ref) then input name. Scoped map[string]map[string]string + // GlobalOverride holds replace-mode inputs, keyed by input name. + GlobalOverride map[string]string + // ScopedOverride holds replace-mode inputs, keyed by policy scope then input + // name. + ScopedOverride map[string]map[string]string +} + +// empty reports whether ri carries no inputs at all. Nil-safe. +func (ri *RuntimeInputs) empty() bool { + return ri == nil || (len(ri.Global) == 0 && len(ri.Scoped) == 0 && + len(ri.GlobalOverride) == 0 && len(ri.ScopedOverride) == 0) } // forPolicy returns the runtime inputs that apply to a policy attachment // identified by its metadata name and raw ref, together with the scope keys -// that matched. The returned map merges the global inputs with any scoped -// entries whose key matches the attachment (additively when they share an -// input name). Returns (nil, nil) when nothing applies. Nil-safe. -func (ri *RuntimeInputs) forPolicy(name, ref string) (map[string]string, []string) { - if ri == nil || (len(ri.Global) == 0 && len(ri.Scoped) == 0) { - return nil, nil +// that matched. Append-mode inputs (appendInputs) merge the global inputs with +// any scoped entries whose key matches the attachment (additively when they +// share an input name). Replace-mode inputs (replaceInputs) merge the global +// overrides with any matching scoped overrides, the scoped ones winning. The +// caller layers appendInputs onto the contract additively and then applies +// replaceInputs on top, so a replace always wins over an append for the same +// key. Returns (nil, nil, nil) when nothing applies. Nil-safe. +func (ri *RuntimeInputs) forPolicy(name, ref string) (appendInputs, replaceInputs map[string]string, matched []string) { + if ri.empty() { + return nil, nil, nil + } + + seen := make(map[string]struct{}) + markMatch := func(scope string) { + if _, ok := seen[scope]; !ok { + seen[scope] = struct{}{} + matched = append(matched, scope) + } } - effective := ri.Global - var matched []string + appendInputs = ri.Global for scope, inputs := range ri.Scoped { if policyScopeMatches(scope, name, ref) { - matched = append(matched, scope) - effective = MergeRuntimeInputs(effective, inputs) + markMatch(scope) + appendInputs = MergeRuntimeInputs(appendInputs, inputs) + } + } + + replaceInputs = ri.GlobalOverride + for scope, inputs := range ri.ScopedOverride { + if policyScopeMatches(scope, name, ref) { + markMatch(scope) + replaceInputs = OverrideRuntimeInputs(replaceInputs, inputs) } } - return effective, matched + return appendInputs, replaceInputs, matched } // policyScopeMatches reports whether a runtime-input scope key targets the @@ -112,6 +147,28 @@ func MergeRuntimeInputs(with, runtimeInputs map[string]string) map[string]string return merged } +// OverrideRuntimeInputs returns the given arguments with the override inputs +// applied by replacement: each override key's value replaces whatever the +// arguments held for that key (a contract value or an appended runtime value), +// rather than being newline-appended. This is what makes a scalar input +// overridable at run time: the value no longer collapses into a multi-value +// list through append-time newline joining. The value is still normalized like +// any other input downstream (getInputArguments splits it on newlines and +// commas), so a single comma-free value such as "10" stays a scalar while a +// value that itself contains commas expands into a list; a literal comma must +// be escaped as "\,". The input maps are not mutated. +func OverrideRuntimeInputs(with, overrides map[string]string) map[string]string { + if len(overrides) == 0 { + return with + } + + merged := make(map[string]string, len(with)+len(overrides)) + maps.Copy(merged, with) + maps.Copy(merged, overrides) + + return merged +} + // scopeTracker records, concurrency-safely, which runtime-input scope keys were // matched by at least one policy attachment during a material evaluation. type scopeTracker struct { @@ -134,9 +191,9 @@ func (t *scopeTracker) mark(keys ...string) { } } -// unmatched returns the sorted scope keys declared in ri that were never marked -// (i.e. matched no policy attachment), so the caller can warn about likely -// typos. Nil-safe. +// unmatched returns the sorted scope keys declared in ri (across both append- +// and replace-mode scoped inputs) that were never marked (i.e. matched no policy +// attachment), so the caller can warn about likely typos. Nil-safe. func (t *scopeTracker) unmatched(ri *RuntimeInputs) []string { if t == nil || ri == nil { return nil @@ -144,8 +201,16 @@ func (t *scopeTracker) unmatched(ri *RuntimeInputs) []string { t.mu.Lock() defer t.mu.Unlock() - var out []string + scopes := make(map[string]struct{}, len(ri.Scoped)+len(ri.ScopedOverride)) for scope := range ri.Scoped { + scopes[scope] = struct{}{} + } + for scope := range ri.ScopedOverride { + scopes[scope] = struct{}{} + } + + var out []string + for scope := range scopes { if _, ok := t.seen[scope]; !ok { out = append(out, scope) } diff --git a/pkg/policies/runtime_inputs_test.go b/pkg/policies/runtime_inputs_test.go index 619c4a1fa..90a5c20c1 100644 --- a/pkg/policies/runtime_inputs_test.go +++ b/pkg/policies/runtime_inputs_test.go @@ -21,6 +21,13 @@ import ( "github.com/stretchr/testify/assert" ) +// Repeated domain strings, extracted to satisfy goconst. +const ( + keyMinIterations = "min_iterations" + policyRadamsaMinIter = "radamsa-min-iterations" + scopeShared = "shared" +) + func TestMergeRuntimeInputs(t *testing.T) { testCases := []struct { name string @@ -162,15 +169,17 @@ func TestPolicyScopeMatches(t *testing.T) { func TestRuntimeInputsForPolicy(t *testing.T) { t.Run("nil receiver returns nothing", func(t *testing.T) { var ri *RuntimeInputs - got, matched := ri.forPolicy("p", "p") - assert.Nil(t, got) + appendInputs, replaceInputs, matched := ri.forPolicy("p", "p") + assert.Nil(t, appendInputs) + assert.Nil(t, replaceInputs) assert.Nil(t, matched) }) t.Run("global inputs apply to every policy", func(t *testing.T) { ri := &RuntimeInputs{Global: map[string]string{"ignored_paths": "a"}} - got, matched := ri.forPolicy("some-policy", "some-policy") - assert.Equal(t, map[string]string{"ignored_paths": "a"}, got) + appendInputs, replaceInputs, matched := ri.forPolicy("some-policy", "some-policy") + assert.Equal(t, map[string]string{"ignored_paths": "a"}, appendInputs) + assert.Nil(t, replaceInputs) assert.Empty(t, matched) }) @@ -179,12 +188,12 @@ func TestRuntimeInputsForPolicy(t *testing.T) { "trusted-binaries-signed": {"ignored_paths": "a"}, }} - got, matched := ri.forPolicy("trusted-binaries-signed", "chainloop://trusted-binaries-signed@sha256:abc") - assert.Equal(t, map[string]string{"ignored_paths": "a"}, got) + appendInputs, _, matched := ri.forPolicy("trusted-binaries-signed", "chainloop://trusted-binaries-signed@sha256:abc") + assert.Equal(t, map[string]string{"ignored_paths": "a"}, appendInputs) assert.ElementsMatch(t, []string{"trusted-binaries-signed"}, matched) - got, matched = ri.forPolicy("trusted-binaries-vendor-keys", "chainloop://trusted-binaries-vendor-keys") - assert.Empty(t, got) + appendInputs, _, matched = ri.forPolicy("trusted-binaries-vendor-keys", "chainloop://trusted-binaries-vendor-keys") + assert.Empty(t, appendInputs) assert.Empty(t, matched) }) @@ -195,21 +204,120 @@ func TestRuntimeInputsForPolicy(t *testing.T) { "trusted-binaries-signed": {"ignored_paths": "s"}, }, } - got, matched := ri.forPolicy("trusted-binaries-signed", "trusted-binaries-signed") - assert.Equal(t, map[string]string{"ignored_paths": "g\ns"}, got) + appendInputs, replaceInputs, matched := ri.forPolicy("trusted-binaries-signed", "trusted-binaries-signed") + assert.Equal(t, map[string]string{"ignored_paths": "g\ns"}, appendInputs) + assert.Nil(t, replaceInputs) assert.ElementsMatch(t, []string{"trusted-binaries-signed"}, matched) }) - t.Run("does not mutate the global map", func(t *testing.T) { + t.Run("global override applies to every policy", func(t *testing.T) { + ri := &RuntimeInputs{GlobalOverride: map[string]string{keyMinIterations: "10"}} + appendInputs, replaceInputs, matched := ri.forPolicy(policyRadamsaMinIter, policyRadamsaMinIter) + assert.Nil(t, appendInputs) + assert.Equal(t, map[string]string{keyMinIterations: "10"}, replaceInputs) + assert.Empty(t, matched) + }) + + t.Run("scoped override applies only to the matching policy and matched is tracked", func(t *testing.T) { + ri := &RuntimeInputs{ScopedOverride: map[string]map[string]string{ + policyRadamsaMinIter: {keyMinIterations: "10"}, + }} + + _, replaceInputs, matched := ri.forPolicy(policyRadamsaMinIter, "chainloop://radamsa-min-iterations@sha256:abc") + assert.Equal(t, map[string]string{keyMinIterations: "10"}, replaceInputs) + assert.ElementsMatch(t, []string{policyRadamsaMinIter}, matched) + + _, replaceInputs, matched = ri.forPolicy("other-policy", "other-policy") + assert.Empty(t, replaceInputs) + assert.Empty(t, matched) + }) + + t.Run("scoped override wins over global override for the same input", func(t *testing.T) { ri := &RuntimeInputs{ - Global: map[string]string{"ignored_paths": "g"}, - Scoped: map[string]map[string]string{"p": {"ignored_paths": "s"}}, + GlobalOverride: map[string]string{keyMinIterations: "5"}, + ScopedOverride: map[string]map[string]string{ + policyRadamsaMinIter: {keyMinIterations: "10"}, + }, + } + _, replaceInputs, matched := ri.forPolicy(policyRadamsaMinIter, policyRadamsaMinIter) + assert.Equal(t, map[string]string{keyMinIterations: "10"}, replaceInputs) + assert.ElementsMatch(t, []string{policyRadamsaMinIter}, matched) + }) + + t.Run("append and override for the same policy are returned separately", func(t *testing.T) { + ri := &RuntimeInputs{ + Global: map[string]string{"ignored_paths": "a"}, + GlobalOverride: map[string]string{keyMinIterations: "10"}, + } + appendInputs, replaceInputs, _ := ri.forPolicy("p", "p") + assert.Equal(t, map[string]string{"ignored_paths": "a"}, appendInputs) + assert.Equal(t, map[string]string{keyMinIterations: "10"}, replaceInputs) + }) + + t.Run("does not mutate the global maps", func(t *testing.T) { + ri := &RuntimeInputs{ + Global: map[string]string{"ignored_paths": "g"}, + Scoped: map[string]map[string]string{"p": {"ignored_paths": "s"}}, + GlobalOverride: map[string]string{keyMinIterations: "10"}, } - _, _ = ri.forPolicy("p", "p") + _, _, _ = ri.forPolicy("p", "p") assert.Equal(t, map[string]string{"ignored_paths": "g"}, ri.Global) + assert.Equal(t, map[string]string{keyMinIterations: "10"}, ri.GlobalOverride) }) } +func TestOverrideRuntimeInputs(t *testing.T) { + testCases := []struct { + name string + with map[string]string + overrides map[string]string + want map[string]string + }{ + { + name: "no overrides returns args unchanged", + with: map[string]string{keyMinIterations: "100"}, + overrides: nil, + want: map[string]string{keyMinIterations: "100"}, + }, + { + name: "override replaces the contract value", + with: map[string]string{keyMinIterations: "100"}, + overrides: map[string]string{keyMinIterations: "10"}, + want: map[string]string{keyMinIterations: "10"}, + }, + { + name: "override on a different key is added alongside", + with: map[string]string{"paths": "**"}, + overrides: map[string]string{keyMinIterations: "10"}, + want: map[string]string{"paths": "**", keyMinIterations: "10"}, + }, + { + name: "override replaces an appended multi-value with a scalar", + with: map[string]string{keyMinIterations: "100\n50"}, + overrides: map[string]string{keyMinIterations: "10"}, + want: map[string]string{keyMinIterations: "10"}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + got := OverrideRuntimeInputs(tc.with, tc.overrides) + assert.Equal(t, tc.want, got) + }) + } +} + +// TestOverrideRuntimeInputsDoesNotMutate ensures the input maps are left untouched. +func TestOverrideRuntimeInputsDoesNotMutate(t *testing.T) { + with := map[string]string{keyMinIterations: "100"} + overrides := map[string]string{keyMinIterations: "10"} + + _ = OverrideRuntimeInputs(with, overrides) + + assert.Equal(t, map[string]string{keyMinIterations: "100"}, with) + assert.Equal(t, map[string]string{keyMinIterations: "10"}, overrides) +} + func TestScopeTrackerUnmatched(t *testing.T) { testCases := []struct { name string @@ -234,6 +342,15 @@ func TestScopeTrackerUnmatched(t *testing.T) { matched: []string{"beta"}, want: []string{"alpha", "zebra"}, }, + { + name: "unmatched override scopes are reported too, deduped with append scopes", + ri: &RuntimeInputs{ + Scoped: map[string]map[string]string{"alpha": {}, scopeShared: {}}, + ScopedOverride: map[string]map[string]string{"zebra": {}, scopeShared: {}}, + }, + matched: []string{"alpha"}, + want: []string{scopeShared, "zebra"}, + }, } for _, tc := range testCases { From e2ca5965177c82a6c4379a88597ecdf32217551a Mon Sep 17 00:00:00 2001 From: Javier Rodriguez Date: Mon, 10 Aug 2026 11:25:17 +0200 Subject: [PATCH 2/6] fix(cli): make scoped policy-input precedence deterministic When several runtime-input scopes match one policy attachment and set the same input, apply them most-specific-last (digest-pinned > scheme/org-qualified > bare name, ties by scope string) so the merged value is deterministic regardless of Go map iteration order; this covers both append and replace modes. Also document that an inline --policy-input takes precedence over --policy-input-from-file-replace for the same input. Assisted-by: Claude Code Signed-off-by: Javier Rodriguez Chainloop-Trace-Sessions: 8f1220ca-1af0-4865-89f3-9705c9df189c --- app/cli/pkg/action/attestation_add.go | 6 +++ app/cli/pkg/action/attestation_add_test.go | 12 +++++ pkg/policies/runtime_inputs.go | 56 ++++++++++++++++++---- pkg/policies/runtime_inputs_test.go | 49 +++++++++++++++++++ 4 files changed, 114 insertions(+), 9 deletions(-) diff --git a/app/cli/pkg/action/attestation_add.go b/app/cli/pkg/action/attestation_add.go index 3d15e04c0..5029a222d 100644 --- a/app/cli/pkg/action/attestation_add.go +++ b/app/cli/pkg/action/attestation_add.go @@ -273,6 +273,12 @@ func withSourceArchiveEvidence(opts []crafter.AddOpt) []crafter.AddOpt { // contract value instead of appending, keeping a scalar override a scalar. As // with contract-declared arguments, individual append values must not embed // those delimiters; path globs, the intended use, never do. +// +// Precedence for the same input+scope: file inputs are applied first and inline +// --policy-input values last, so an inline value deterministically wins over a +// --policy-input-from-file-replace for the same key. (Cobra exposes each +// repeatable flag as its own slice with no cross-flag ordering, so this fixed +// precedence — rather than raw CLI argument order — is what we can guarantee.) func buildRuntimeInputs(policyInputFiles []*PolicyInputFromFile, policyInputs []*PolicyInput) (*policies.RuntimeInputs, error) { if len(policyInputFiles) == 0 && len(policyInputs) == 0 { return nil, nil diff --git a/app/cli/pkg/action/attestation_add_test.go b/app/cli/pkg/action/attestation_add_test.go index 3da37ab19..230739fe4 100644 --- a/app/cli/pkg/action/attestation_add_test.go +++ b/app/cli/pkg/action/attestation_add_test.go @@ -274,6 +274,18 @@ func TestBuildRuntimeInputs(t *testing.T) { assert.Equal(t, map[string]string{testInputMinIter: "20"}, got.ScopedOverride[testPolicyRadamsa]) }) + t.Run("inline --policy-input wins over a file-replace for the same input", func(t *testing.T) { + // File-replace fills min_iterations from the file column; the inline value + // is applied afterwards and must win deterministically. + got, err := buildRuntimeInputs([]*PolicyInputFromFile{ + {Input: testInputMinIter, Column: "Path", File: path, Replace: true}, + }, []*PolicyInput{ + {Input: testInputMinIter, Value: "10"}, + }) + require.NoError(t, err) + assert.Equal(t, map[string]string{testInputMinIter: "10"}, got.GlobalOverride) + }) + t.Run("append files, replace files and inline values coexist", func(t *testing.T) { got, err := buildRuntimeInputs([]*PolicyInputFromFile{ {Input: "ignored_paths", Column: "Path", File: path}, diff --git a/pkg/policies/runtime_inputs.go b/pkg/policies/runtime_inputs.go index 7795f0b2f..2373978bd 100644 --- a/pkg/policies/runtime_inputs.go +++ b/pkg/policies/runtime_inputs.go @@ -18,6 +18,7 @@ package policies import ( "maps" "slices" + "strings" "sync" ) @@ -70,23 +71,60 @@ func (ri *RuntimeInputs) forPolicy(name, ref string) (appendInputs, replaceInput } } + // Apply matching scopes least-specific first so that, when several scopes + // target the same attachment and set the same input, the most specific one + // is applied last and wins — deterministically, regardless of Go's map + // iteration order. appendInputs = ri.Global - for scope, inputs := range ri.Scoped { - if policyScopeMatches(scope, name, ref) { - markMatch(scope) - appendInputs = MergeRuntimeInputs(appendInputs, inputs) - } + for _, scope := range matchingScopes(ri.Scoped, name, ref) { + markMatch(scope) + appendInputs = MergeRuntimeInputs(appendInputs, ri.Scoped[scope]) } replaceInputs = ri.GlobalOverride - for scope, inputs := range ri.ScopedOverride { + for _, scope := range matchingScopes(ri.ScopedOverride, name, ref) { + markMatch(scope) + replaceInputs = OverrideRuntimeInputs(replaceInputs, ri.ScopedOverride[scope]) + } + + return appendInputs, replaceInputs, matched +} + +// matchingScopes returns the keys of scoped that target the attachment +// identified by (name, ref), ordered so more specific scopes come last. Callers +// apply them in that order, so a later (more specific) scope wins when several +// set the same input. Ties break on the scope string, so the order — and thus +// the merged result — is deterministic regardless of Go's randomized map +// iteration. +func matchingScopes[V any](scoped map[string]V, name, ref string) []string { + var out []string + for scope := range scoped { if policyScopeMatches(scope, name, ref) { - markMatch(scope) - replaceInputs = OverrideRuntimeInputs(replaceInputs, inputs) + out = append(out, scope) } } + slices.SortFunc(out, func(a, b string) int { + if d := scopeSpecificity(a) - scopeSpecificity(b); d != 0 { + return d + } + return strings.Compare(a, b) + }) + return out +} - return appendInputs, replaceInputs, matched +// scopeSpecificity scores how narrowly a scope key targets a policy: a scope +// that pins a digest is the most specific, a scope carrying a scheme or org path +// (a fuller ref) is more specific than a bare policy name. +func scopeSpecificity(scope string) int { + _, digest := splitPolicyRef(scope) + score := 0 + if digest != "" { + score += 2 + } + if strings.Contains(scope, "://") || strings.Contains(scope, "/") { + score++ + } + return score } // policyScopeMatches reports whether a runtime-input scope key targets the diff --git a/pkg/policies/runtime_inputs_test.go b/pkg/policies/runtime_inputs_test.go index 90a5c20c1..fb05b9252 100644 --- a/pkg/policies/runtime_inputs_test.go +++ b/pkg/policies/runtime_inputs_test.go @@ -26,6 +26,7 @@ const ( keyMinIterations = "min_iterations" policyRadamsaMinIter = "radamsa-min-iterations" scopeShared = "shared" + refRadamsaDigest = "chainloop://radamsa-min-iterations@sha256:abc" ) func TestMergeRuntimeInputs(t *testing.T) { @@ -254,6 +255,36 @@ func TestRuntimeInputsForPolicy(t *testing.T) { assert.Equal(t, map[string]string{keyMinIterations: "10"}, replaceInputs) }) + t.Run("most specific scoped override wins deterministically", func(t *testing.T) { + // Both a bare-name scope and a digest-pinned ref scope match the same + // attachment and set the same input to different values. + ri := &RuntimeInputs{ScopedOverride: map[string]map[string]string{ + policyRadamsaMinIter: {keyMinIterations: "10"}, + refRadamsaDigest: {keyMinIterations: "20"}, + }} + + // Run many times: Go map iteration is randomized, but the digest-pinned + // (most specific) scope must win every time. + for range 50 { + _, replaceInputs, matched := ri.forPolicy(policyRadamsaMinIter, refRadamsaDigest) + assert.Equal(t, map[string]string{keyMinIterations: "20"}, replaceInputs) + assert.ElementsMatch(t, []string{policyRadamsaMinIter, refRadamsaDigest}, matched) + } + }) + + t.Run("scoped appends apply least-specific first, deterministically", func(t *testing.T) { + ri := &RuntimeInputs{Scoped: map[string]map[string]string{ + policyRadamsaMinIter: {keyMinIterations: "a"}, + refRadamsaDigest: {keyMinIterations: "b"}, + }} + + for range 50 { + appendInputs, _, _ := ri.forPolicy(policyRadamsaMinIter, refRadamsaDigest) + // least specific (bare "a") first, most specific ("b") last + assert.Equal(t, map[string]string{keyMinIterations: "a\nb"}, appendInputs) + } + }) + t.Run("does not mutate the global maps", func(t *testing.T) { ri := &RuntimeInputs{ Global: map[string]string{"ignored_paths": "g"}, @@ -266,6 +297,24 @@ func TestRuntimeInputsForPolicy(t *testing.T) { }) } +func TestMatchingScopesOrder(t *testing.T) { + name := policyRadamsaMinIter + scheme := "chainloop://" + policyRadamsaMinIter // scheme-qualified, no digest + digest := policyRadamsaMinIter + "@sha256:abc" // digest-pinned, no scheme + ref := refRadamsaDigest // digest + scheme + // Four scopes that all match the attachment, at increasing specificity: + // bare name < scheme-qualified < digest-pinned < digest+scheme. + scoped := map[string]struct{}{ + name: {}, + scheme: {}, + digest: {}, + ref: {}, + } + + got := matchingScopes(scoped, name, ref) + assert.Equal(t, []string{name, scheme, digest, ref}, got) +} + func TestOverrideRuntimeInputs(t *testing.T) { testCases := []struct { name string From e4401c9f8413db8f5782b57b52ce3cd85ee5bee4 Mon Sep 17 00:00:00 2001 From: Javier Rodriguez Date: Mon, 10 Aug 2026 11:38:15 +0200 Subject: [PATCH 3/6] fix(cli): rank provider-qualified policy-input scopes above bare names scopeSpecificity only bumped for '://' or '/', so a provider-prefixed scope (provider:name) scored the same as a bare name and could lose the lexical tie-break. Treat any ':' or '/' beyond the bare name (scheme, org path or provider prefix) as qualification, while stripping the '@sha256:' digest suffix so its colon does not inflate the score. Assisted-by: Claude Code Signed-off-by: Javier Rodriguez Chainloop-Trace-Sessions: 8f1220ca-1af0-4865-89f3-9705c9df189c --- pkg/policies/runtime_inputs.go | 19 ++++++++++++++++--- pkg/policies/runtime_inputs_test.go | 10 ++++++++++ 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/pkg/policies/runtime_inputs.go b/pkg/policies/runtime_inputs.go index 2373978bd..a4132b987 100644 --- a/pkg/policies/runtime_inputs.go +++ b/pkg/policies/runtime_inputs.go @@ -113,15 +113,28 @@ func matchingScopes[V any](scoped map[string]V, name, ref string) []string { } // scopeSpecificity scores how narrowly a scope key targets a policy: a scope -// that pins a digest is the most specific, a scope carrying a scheme or org path -// (a fuller ref) is more specific than a bare policy name. +// that pins a digest is the most specific, and a scope carrying anything beyond +// the bare policy name — a scheme (chainloop://), an org path (org/name) or a +// provider prefix (provider:name) — is more specific than a bare name. func scopeSpecificity(scope string) int { _, digest := splitPolicyRef(scope) + + // Drop the "@sha256:" suffix so its own ':' does not count as + // provider/scheme qualification below. + head := scope + if digest != "" { + if i := strings.Index(scope, "@"); i >= 0 { + head = scope[:i] + } + } + score := 0 if digest != "" { score += 2 } - if strings.Contains(scope, "://") || strings.Contains(scope, "/") { + // A scheme (chainloop://), org path (org/name) or provider prefix + // (provider:name) all introduce a ':' or '/' beyond the bare name. + if strings.ContainsAny(head, ":/") { score++ } return score diff --git a/pkg/policies/runtime_inputs_test.go b/pkg/policies/runtime_inputs_test.go index fb05b9252..afb9694ee 100644 --- a/pkg/policies/runtime_inputs_test.go +++ b/pkg/policies/runtime_inputs_test.go @@ -315,6 +315,16 @@ func TestMatchingScopesOrder(t *testing.T) { assert.Equal(t, []string{name, scheme, digest, ref}, got) } +func TestMatchingScopesProviderQualifiedOutranksBare(t *testing.T) { + // A "provider:policy" scope carries no scheme "//" or org "/", only a ':'; + // it must still outrank a conflicting bare-name scope. + name := policyRadamsaMinIter + provider := "builtin:" + policyRadamsaMinIter + + got := matchingScopes(map[string]struct{}{name: {}, provider: {}}, name, name) + assert.Equal(t, []string{name, provider}, got) // bare (score 0) then provider (score 1) +} + func TestOverrideRuntimeInputs(t *testing.T) { testCases := []struct { name string From 31a3b6278955ccbc12ea98ee6b761c1ad15c8290 Mon Sep 17 00:00:00 2001 From: Javier Rodriguez Date: Mon, 10 Aug 2026 11:58:23 +0200 Subject: [PATCH 4/6] feat(cli): add reserved --append flag, drop --policy-input-from-file-replace Add a visible --append boolean reserved for a future release: it warns and has no effect yet, but will later control whether --policy-input and --policy-input-from-file append to rather than replace the contract-declared value. Remove --policy-input-from-file-replace, since that future model makes replace the default for --policy-input-from-file and a dedicated replace flag redundant. Inline scalar override via --policy-input is unchanged. Assisted-by: Claude Code Signed-off-by: Javier Rodriguez Chainloop-Trace-Sessions: 8f1220ca-1af0-4865-89f3-9705c9df189c --- app/cli/cmd/attestation_add.go | 72 +++++++++----------- app/cli/cmd/policy_input_file_test.go | 31 +++------ app/cli/documentation/cli-reference.mdx | 34 ++++----- app/cli/pkg/action/attestation_add.go | 28 +++----- app/cli/pkg/action/attestation_add_test.go | 29 +------- app/cli/pkg/action/policy_input_file.go | 22 ++---- app/cli/pkg/action/policy_input_file_test.go | 20 +----- pkg/policies/pfm6906_override_test.go | 6 +- pkg/policies/runtime_inputs.go | 2 +- 9 files changed, 78 insertions(+), 166 deletions(-) diff --git a/app/cli/cmd/attestation_add.go b/app/cli/cmd/attestation_add.go index 9169513fb..813575e4c 100644 --- a/app/cli/cmd/attestation_add.go +++ b/app/cli/cmd/attestation_add.go @@ -42,8 +42,8 @@ func newAttestationAddCmd() *cobra.Command { var annotationsFlag []string var noStrictValidation bool var policyInputFromFileFlag []string - var policyInputFromFileReplaceFlag []string var policyInputFlag []string + var appendFlag bool var maxExtractEntries int var maxExtractSize string @@ -88,11 +88,7 @@ func newAttestationAddCmd() *cobra.Command { # Override a scalar policy input for a single run with an inline literal value. Unlike --policy-input-from-file # (which appends), --policy-input REPLACES the contract-declared value, so a scalar input stays a scalar. chainloop attestation add --name fuzz --value report.txt --kind RADAMSA_REPORT \ - --policy-input radamsa-min-iterations:min_iterations=10 - - # Replace (rather than append) a contract-declared list from a file column with --policy-input-from-file-replace. - chainloop attestation add --name sigcheck --value sigcheckResult.csv --kind SYSINTERNALS_SIGCHECK \ - --policy-input-from-file-replace ignored_paths=exception.csv:Path`, + --policy-input radamsa-min-iterations:min_iterations=10`, RunE: func(cmd *cobra.Command, _ []string) error { maxExtractSizeBytes, err := bytefmt.ToBytes(maxExtractSize) if err != nil { @@ -129,10 +125,9 @@ func newAttestationAddCmd() *cobra.Command { return err } - // Parse and resolve the policy input files (column -> policy input), - // both the append and the replace variants. Done once here; the - // resolved local paths are reused across retries. - policyInputFiles, err := resolvePolicyInputFiles(policyInputFromFileFlag, policyInputFromFileReplaceFlag) + // Parse and resolve the policy input files (column -> policy input). + // Done once here; the resolved local paths are reused across retries. + policyInputFiles, err := resolvePolicyInputFiles(policyInputFromFileFlag) if err != nil { return err } @@ -143,6 +138,12 @@ func newAttestationAddCmd() *cobra.Command { return err } + // --append is reserved: it will later toggle append/replace semantics + // for --policy-input and --policy-input-from-file, but does nothing yet. + if appendFlag { + logger.Warn().Msg("--append has no effect yet; reserved for a future release") + } + // In some cases, the attestation state is stored remotely. To control concurrency we use // optimistic locking. We retry the operation if the state has changed since we last read it. return runWithBackoffRetry( @@ -206,8 +207,8 @@ func newAttestationAddCmd() *cobra.Command { cmd.Flags().StringVar(&kind, "kind", "", fmt.Sprintf("kind of the material to be recorded: %q", schemaapi.ListAvailableMaterialKind())) cmd.Flags().BoolVar(&noStrictValidation, "no-strict-validation", false, "skip strict schema validation for structured materials (SBOM_CYCLONEDX_JSON, OPENAPI_SPEC, ASYNCAPI_SPEC, OSSF_SCORECARD_JSON)") cmd.Flags().StringArrayVar(&policyInputFromFileFlag, "policy-input-from-file", nil, "feed a policy input from a column of a CSV or JSON file, in the format [:]=[:] (e.g. ignored_paths=exception.csv:Path); the values are APPENDED to any contract-declared value; an optional : prefix scopes the input to a single policy (matched by name or ref), otherwise it applies to every declaring policy; is a single top-level column/field name and defaults to the input name; repeatable. The file is also recorded as EVIDENCE.") - cmd.Flags().StringArrayVar(&policyInputFromFileReplaceFlag, "policy-input-from-file-replace", nil, "like --policy-input-from-file but the extracted values REPLACE (override) any contract-declared value for the input instead of being appended to it; same [:]=[:] format; repeatable. The file is also recorded as EVIDENCE.") cmd.Flags().StringArrayVar(&policyInputFlag, "policy-input", nil, "set a policy input to a literal value that REPLACES (overrides) any contract-declared value for the input, in the format [:]= (e.g. min_iterations=10); use this to override a scalar input at run time; an optional : prefix scopes it to a single policy (matched by name or ref), otherwise it applies to every declaring policy; repeatable.") + cmd.Flags().BoolVar(&appendFlag, "append", false, "reserved for a future release: will control whether --policy-input and --policy-input-from-file append to (rather than replace) the contract-declared value; has no effect yet") // Optional OCI registry credentials cmd.Flags().StringVar(®istryServer, "registry-server", "", fmt.Sprintf("OCI repository server, ($%s)", registryServerEnvVarName)) @@ -233,40 +234,33 @@ func newAttestationAddCmd() *cobra.Command { return cmd } -// resolvePolicyInputFiles parses each --policy-input-from-file (append) and -// --policy-input-from-file-replace (replace) value and resolves its file -// reference to a local path (downloading URLs to a temporary file, mirroring how -// --value is handled). Both variants are returned in one slice, distinguished by -// PolicyInputFromFile.Replace. -func resolvePolicyInputFiles(rawAppend, rawReplace []string) ([]*action.PolicyInputFromFile, error) { - if len(rawAppend) == 0 && len(rawReplace) == 0 { +// resolvePolicyInputFiles parses each --policy-input-from-file value and +// resolves its file reference to a local path (downloading URLs to a temporary +// file, mirroring how --value is handled). +func resolvePolicyInputFiles(raw []string) ([]*action.PolicyInputFromFile, error) { + if len(raw) == 0 { return nil, nil } - result := make([]*action.PolicyInputFromFile, 0, len(rawAppend)+len(rawReplace)) - for _, group := range []struct { - raw []string - replace bool - }{{rawAppend, false}, {rawReplace, true}} { - for _, r := range group.raw { - pif, err := action.ParsePolicyInputFromFile(r, group.replace) - if err != nil { - return nil, err - } + result := make([]*action.PolicyInputFromFile, 0, len(raw)) + for _, r := range raw { + pif, err := action.ParsePolicyInputFromFile(r) + if err != nil { + return nil, err + } - path, err := resourceloader.GetPathForResource(pif.File) - if err != nil { - var uerr *resourceloader.UnrecognizedSchemeError - if errors.As(err, &uerr) { - path = pif.File - } else { - return nil, fmt.Errorf("loading policy input file: %w", err) - } + path, err := resourceloader.GetPathForResource(pif.File) + if err != nil { + var uerr *resourceloader.UnrecognizedSchemeError + if errors.As(err, &uerr) { + path = pif.File + } else { + return nil, fmt.Errorf("loading policy input file: %w", err) } - pif.File = path - - result = append(result, pif) } + pif.File = path + + result = append(result, pif) } return result, nil diff --git a/app/cli/cmd/policy_input_file_test.go b/app/cli/cmd/policy_input_file_test.go index f63925aff..87bf3cc90 100644 --- a/app/cli/cmd/policy_input_file_test.go +++ b/app/cli/cmd/policy_input_file_test.go @@ -27,12 +27,11 @@ import ( func TestResolvePolicyInputFiles(t *testing.T) { testCases := []struct { - name string - raw []string - rawReplace []string - want []*action.PolicyInputFromFile - wantNil bool - wantErr bool + name string + raw []string + want []*action.PolicyInputFromFile + wantNil bool + wantErr bool }{ { name: "nil input returns nil", @@ -72,25 +71,11 @@ func TestResolvePolicyInputFiles(t *testing.T) { raw: []string{"ignored_paths=env://CHAINLOOP_TEST_DEFINITELY_UNSET_VAR"}, wantErr: true, }, - { - name: "append and replace variants both parsed, Replace set accordingly", - raw: []string{"ignored_paths=/no/exist1.csv:Path"}, - rawReplace: []string{"min_iterations=/no/exist2.csv:Iterations"}, - want: []*action.PolicyInputFromFile{ - {Input: "ignored_paths", Column: "Path", File: "/no/exist1.csv"}, - {Input: "min_iterations", Column: "Iterations", File: "/no/exist2.csv", Replace: true}, - }, - }, - { - name: "malformed replace value propagates the parse error", - rawReplace: []string{"missing-equals"}, - wantErr: true, - }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - got, err := resolvePolicyInputFiles(tc.raw, tc.rawReplace) + got, err := resolvePolicyInputFiles(tc.raw) if tc.wantErr { assert.Error(t, err) return @@ -112,7 +97,7 @@ func TestResolvePolicyInputFilesExistingFile(t *testing.T) { path := filepath.Join(dir, "exception.csv") require.NoError(t, os.WriteFile(path, []byte("Path\nc:\\a.dll\n"), 0600)) - got, err := resolvePolicyInputFiles([]string{"ignored_paths=" + path + ":Path"}, nil) + got, err := resolvePolicyInputFiles([]string{"ignored_paths=" + path + ":Path"}) require.NoError(t, err) require.Len(t, got, 1) assert.Equal(t, &action.PolicyInputFromFile{Input: "ignored_paths", Column: "Path", File: path}, got[0]) @@ -124,7 +109,7 @@ func TestResolvePolicyInputFilesExistingFile(t *testing.T) { func TestResolvePolicyInputFilesResolvesEnv(t *testing.T) { t.Setenv("CHAINLOOP_TEST_POLICY_INPUT", `["c:\\a.dll"]`) - got, err := resolvePolicyInputFiles([]string{"ignored_paths=env://CHAINLOOP_TEST_POLICY_INPUT"}, nil) + got, err := resolvePolicyInputFiles([]string{"ignored_paths=env://CHAINLOOP_TEST_POLICY_INPUT"}) require.NoError(t, err) require.Len(t, got, 1) diff --git a/app/cli/documentation/cli-reference.mdx b/app/cli/documentation/cli-reference.mdx index c42b9c7ca..be9aaeb96 100755 --- a/app/cli/documentation/cli-reference.mdx +++ b/app/cli/documentation/cli-reference.mdx @@ -259,30 +259,26 @@ Override a scalar policy input for a single run with an inline literal value. Un (which appends), --policy-input REPLACES the contract-declared value, so a scalar input stays a scalar. chainloop attestation add --name fuzz --value report.txt --kind RADAMSA_REPORT \ --policy-input radamsa-min-iterations:min_iterations=10 - -Replace (rather than append) a contract-declared list from a file column with --policy-input-from-file-replace. -chainloop attestation add --name sigcheck --value sigcheckResult.csv --kind SYSINTERNALS_SIGCHECK \ ---policy-input-from-file-replace ignored_paths=exception.csv:Path ``` Options ``` ---annotation strings additional annotation in the format of key=value ---attestation-id string Unique identifier of the in-progress attestation --h, --help help for add ---kind string kind of the material to be recorded: ["ARTIFACT" "ASYNCAPI_SPEC" "ATTESTATION" "BLACKDUCK_SCA_JSON" "CERTCC_DRANZER" "CHAINLOOP_AI_AGENT_CONFIG" "CHAINLOOP_AI_CODING_SESSION" "CHAINLOOP_PR_INFO" "CHAINLOOP_RUNNER_CONTEXT" "CHECKMARX_JSON" "COBERTURA_XML" "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" "GITLEAKS_JSON" "GRAPHQL_SPEC" "HELM_CHART" "JACOCO_XML" "JUNIT_XML" "OPENAPI_SPEC" "OPENVEX" "OSSF_SCORECARD_JSON" "RADAMSA_CRASHES" "RADAMSA_REPORT" "SARIF" "SBOM_CYCLONEDX_JSON" "SBOM_SPDX_JSON" "SLSA_PROVENANCE" "STRING" "SYSINTERNALS_ACCESSCHK" "SYSINTERNALS_SIGCHECK" "TRUFFLEHOG_JSON" "TWISTCLI_SCAN_JSON" "YELP_DETECT_SECRETS_BASELINE" "ZAP_DAST_ZIP"] ---max-extract-entries int max number of files to extract when --value is an archive (default 10000) ---max-extract-size string max total uncompressed size to extract when --value is an archive (default "1GiB") ---name string name of the material as shown in the contract ---no-strict-validation skip strict schema validation for structured materials (SBOM_CYCLONEDX_JSON, OPENAPI_SPEC, ASYNCAPI_SPEC, OSSF_SCORECARD_JSON) ---policy-input stringArray set a policy input to a literal value that REPLACES (overrides) any contract-declared value for the input, in the format [:]= (e.g. min_iterations=10); use this to override a scalar input at run time; an optional : prefix scopes it to a single policy (matched by name or ref), otherwise it applies to every declaring policy; repeatable. ---policy-input-from-file stringArray feed a policy input from a column of a CSV or JSON file, in the format [:]=[:] (e.g. ignored_paths=exception.csv:Path); the values are APPENDED to any contract-declared value; an optional : prefix scopes the input to a single policy (matched by name or ref), otherwise it applies to every declaring policy; is a single top-level column/field name and defaults to the input name; repeatable. The file is also recorded as EVIDENCE. ---policy-input-from-file-replace stringArray like --policy-input-from-file but the extracted values REPLACE (override) any contract-declared value for the input instead of being appended to it; same [:]=[:] format; repeatable. The file is also recorded as EVIDENCE. ---registry-password string registry password, ($CHAINLOOP_REGISTRY_PASSWORD) ---registry-server string OCI repository server, ($CHAINLOOP_REGISTRY_SERVER) ---registry-username string registry username, ($CHAINLOOP_REGISTRY_USERNAME) ---value string value to be recorded +--annotation strings additional annotation in the format of key=value +--append reserved for a future release: will control whether --policy-input and --policy-input-from-file append to (rather than replace) the contract-declared value; has no effect yet +--attestation-id string Unique identifier of the in-progress attestation +-h, --help help for add +--kind string kind of the material to be recorded: ["ARTIFACT" "ASYNCAPI_SPEC" "ATTESTATION" "BLACKDUCK_SCA_JSON" "CERTCC_DRANZER" "CHAINLOOP_AI_AGENT_CONFIG" "CHAINLOOP_AI_CODING_SESSION" "CHAINLOOP_PR_INFO" "CHAINLOOP_RUNNER_CONTEXT" "CHECKMARX_JSON" "COBERTURA_XML" "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" "GITLEAKS_JSON" "GRAPHQL_SPEC" "HELM_CHART" "JACOCO_XML" "JUNIT_XML" "OPENAPI_SPEC" "OPENVEX" "OSSF_SCORECARD_JSON" "RADAMSA_CRASHES" "RADAMSA_REPORT" "SARIF" "SBOM_CYCLONEDX_JSON" "SBOM_SPDX_JSON" "SLSA_PROVENANCE" "STRING" "SYSINTERNALS_ACCESSCHK" "SYSINTERNALS_SIGCHECK" "TRUFFLEHOG_JSON" "TWISTCLI_SCAN_JSON" "YELP_DETECT_SECRETS_BASELINE" "ZAP_DAST_ZIP"] +--max-extract-entries int max number of files to extract when --value is an archive (default 10000) +--max-extract-size string max total uncompressed size to extract when --value is an archive (default "1GiB") +--name string name of the material as shown in the contract +--no-strict-validation skip strict schema validation for structured materials (SBOM_CYCLONEDX_JSON, OPENAPI_SPEC, ASYNCAPI_SPEC, OSSF_SCORECARD_JSON) +--policy-input stringArray set a policy input to a literal value that REPLACES (overrides) any contract-declared value for the input, in the format [:]= (e.g. min_iterations=10); use this to override a scalar input at run time; an optional : prefix scopes it to a single policy (matched by name or ref), otherwise it applies to every declaring policy; repeatable. +--policy-input-from-file stringArray feed a policy input from a column of a CSV or JSON file, in the format [:]=[:] (e.g. ignored_paths=exception.csv:Path); the values are APPENDED to any contract-declared value; an optional : prefix scopes the input to a single policy (matched by name or ref), otherwise it applies to every declaring policy; is a single top-level column/field name and defaults to the input name; repeatable. The file is also recorded as EVIDENCE. +--registry-password string registry password, ($CHAINLOOP_REGISTRY_PASSWORD) +--registry-server string OCI repository server, ($CHAINLOOP_REGISTRY_SERVER) +--registry-username string registry username, ($CHAINLOOP_REGISTRY_USERNAME) +--value string value to be recorded ``` Options inherited from parent commands diff --git a/app/cli/pkg/action/attestation_add.go b/app/cli/pkg/action/attestation_add.go index 5029a222d..99c822f28 100644 --- a/app/cli/pkg/action/attestation_add.go +++ b/app/cli/pkg/action/attestation_add.go @@ -265,20 +265,13 @@ func withSourceArchiveEvidence(opts []crafter.AddOpt) []crafter.AddOpt { // buildRuntimeInputs reads each policy input file and combines it with the // inline --policy-input values, returning them grouped for the policy engine. -// Append-mode file inputs (--policy-input-from-file) go under Global/Scoped and -// are newline-joined via policies.MergeRuntimeInputs so repeated inputs merge -// using the multi-value encoding the engine expects (it splits inputs back on -// newlines and commas). Replace-mode file inputs (--policy-input-from-file-replace) -// and every inline value go under GlobalOverride/ScopedOverride and replace the -// contract value instead of appending, keeping a scalar override a scalar. As -// with contract-declared arguments, individual append values must not embed -// those delimiters; path globs, the intended use, never do. -// -// Precedence for the same input+scope: file inputs are applied first and inline -// --policy-input values last, so an inline value deterministically wins over a -// --policy-input-from-file-replace for the same key. (Cobra exposes each -// repeatable flag as its own slice with no cross-flag ordering, so this fixed -// precedence — rather than raw CLI argument order — is what we can guarantee.) +// File inputs (--policy-input-from-file) go under Global/Scoped and are +// newline-joined via policies.MergeRuntimeInputs so repeated inputs merge using +// the multi-value encoding the engine expects (it splits inputs back on newlines +// and commas). Inline values (--policy-input) go under GlobalOverride/ScopedOverride +// and replace the contract value instead of appending, keeping a scalar override +// a scalar. As with contract-declared arguments, individual append values must +// not embed those delimiters; path globs, the intended use, never do. func buildRuntimeInputs(policyInputFiles []*PolicyInputFromFile, policyInputs []*PolicyInput) (*policies.RuntimeInputs, error) { if len(policyInputFiles) == 0 && len(policyInputs) == 0 { return nil, nil @@ -297,12 +290,7 @@ func buildRuntimeInputs(policyInputFiles []*PolicyInputFromFile, policyInputs [] return nil, fmt.Errorf("extracting %q from %q: %w", pif.Column, pif.File, err) } - joined := strings.Join(values, "\n") - if pif.Replace { - addOverrideInput(ri, pif.Policy, pif.Input, joined) - } else { - addAppendInput(ri, pif.Policy, pif.Input, joined) - } + addAppendInput(ri, pif.Policy, pif.Input, strings.Join(values, "\n")) } for _, pi := range policyInputs { diff --git a/app/cli/pkg/action/attestation_add_test.go b/app/cli/pkg/action/attestation_add_test.go index 230739fe4..c7ca7b8d7 100644 --- a/app/cli/pkg/action/attestation_add_test.go +++ b/app/cli/pkg/action/attestation_add_test.go @@ -251,18 +251,6 @@ func TestBuildRuntimeInputs(t *testing.T) { assert.Equal(t, map[string]string{"ignored_paths": wantXY}, got.Scoped["p"]) }) - t.Run("replace-mode file inputs land in the override maps", func(t *testing.T) { - got, err := buildRuntimeInputs([]*PolicyInputFromFile{ - {Input: "ignored_paths", Column: "Path", File: path, Replace: true}, - {Policy: "p", Input: "third_party_paths", Column: "Extra", File: path, Replace: true}, - }, nil) - require.NoError(t, err) - assert.Empty(t, got.Global) - assert.Empty(t, got.Scoped) - assert.Equal(t, map[string]string{"ignored_paths": wantAB}, got.GlobalOverride) - assert.Equal(t, map[string]string{"third_party_paths": wantXY}, got.ScopedOverride["p"]) - }) - t.Run("inline values land in the override maps, last write wins", func(t *testing.T) { got, err := buildRuntimeInputs(nil, []*PolicyInput{ {Input: testInputMinIter, Value: "5"}, @@ -274,28 +262,15 @@ func TestBuildRuntimeInputs(t *testing.T) { assert.Equal(t, map[string]string{testInputMinIter: "20"}, got.ScopedOverride[testPolicyRadamsa]) }) - t.Run("inline --policy-input wins over a file-replace for the same input", func(t *testing.T) { - // File-replace fills min_iterations from the file column; the inline value - // is applied afterwards and must win deterministically. - got, err := buildRuntimeInputs([]*PolicyInputFromFile{ - {Input: testInputMinIter, Column: "Path", File: path, Replace: true}, - }, []*PolicyInput{ - {Input: testInputMinIter, Value: "10"}, - }) - require.NoError(t, err) - assert.Equal(t, map[string]string{testInputMinIter: "10"}, got.GlobalOverride) - }) - - t.Run("append files, replace files and inline values coexist", func(t *testing.T) { + t.Run("append files (via Global) and inline overrides coexist", func(t *testing.T) { got, err := buildRuntimeInputs([]*PolicyInputFromFile{ {Input: "ignored_paths", Column: "Path", File: path}, - {Input: "extra_paths", Column: "Extra", File: path, Replace: true}, }, []*PolicyInput{ {Policy: testPolicyRadamsa, Input: testInputMinIter, Value: "10"}, }) require.NoError(t, err) assert.Equal(t, map[string]string{"ignored_paths": wantAB}, got.Global) - assert.Equal(t, map[string]string{"extra_paths": wantXY}, got.GlobalOverride) + assert.Empty(t, got.GlobalOverride) assert.Equal(t, map[string]string{testInputMinIter: "10"}, got.ScopedOverride[testPolicyRadamsa]) }) } diff --git a/app/cli/pkg/action/policy_input_file.go b/app/cli/pkg/action/policy_input_file.go index 68498d8bc..1833bbc97 100644 --- a/app/cli/pkg/action/policy_input_file.go +++ b/app/cli/pkg/action/policy_input_file.go @@ -27,8 +27,9 @@ import ( "github.com/chainloop-dev/chainloop/pkg/tabular" ) -// PolicyInputFromFile describes a single --policy-input-from-file[-replace] flag -// value: a policy input name fed from a named column of a CSV or JSON file. +// PolicyInputFromFile describes a single --policy-input-from-file flag value: a +// policy input name fed from a named column of a CSV or JSON file. Its values +// are appended to any contract-declared value for the input. type PolicyInputFromFile struct { // Policy optionally scopes the input to a specific policy (its name or ref). // Empty means the input is global and applies to every declaring policy. @@ -39,10 +40,6 @@ type PolicyInputFromFile struct { Column string // File is the source CSV or JSON file path. File string - // Replace reports whether the extracted values replace the contract-declared - // value for the input (--policy-input-from-file-replace) rather than being - // appended to it (--policy-input-from-file). - Replace bool } // PolicyInput describes a single --policy-input flag value: a policy input name @@ -115,14 +112,9 @@ func parsePolicyInputKey(lhs, raw, flag string) (policy, input string, err error // key. The column is the segment after the last ":"; since a column name never // contains a path separator, a trailing ":<...>" whose ":" belongs to the file // (a Windows drive letter like C:\data\... or a URL scheme like https://) is not -// mistaken for a column. replace records whether the values replace the contract -// value (--policy-input-from-file-replace) rather than being appended to it -// (--policy-input-from-file), and only affects the flag name shown in errors. -func ParsePolicyInputFromFile(raw string, replace bool) (*PolicyInputFromFile, error) { - flag := "--policy-input-from-file" - if replace { - flag = "--policy-input-from-file-replace" - } +// mistaken for a column. +func ParsePolicyInputFromFile(raw string) (*PolicyInputFromFile, error) { + const flag = "--policy-input-from-file" lhs, rhs, found := strings.Cut(raw, "=") if !found { @@ -154,7 +146,7 @@ func ParsePolicyInputFromFile(raw string, replace bool) (*PolicyInputFromFile, e return nil, fmt.Errorf("invalid %s %q: missing file path", flag, raw) } - return &PolicyInputFromFile{Policy: policy, Input: input, Column: column, File: file, Replace: replace}, nil + return &PolicyInputFromFile{Policy: policy, Input: input, Column: column, File: file}, nil } // ParsePolicyInput parses a single --policy-input flag value of the form diff --git a/app/cli/pkg/action/policy_input_file_test.go b/app/cli/pkg/action/policy_input_file_test.go index c752c5e2a..967df62e4 100644 --- a/app/cli/pkg/action/policy_input_file_test.go +++ b/app/cli/pkg/action/policy_input_file_test.go @@ -131,7 +131,7 @@ func TestParsePolicyInputFromFile(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - got, err := ParsePolicyInputFromFile(tc.raw, false) + got, err := ParsePolicyInputFromFile(tc.raw) if tc.wantErr { assert.Error(t, err) return @@ -142,24 +142,6 @@ func TestParsePolicyInputFromFile(t *testing.T) { } } -func TestParsePolicyInputFromFileReplace(t *testing.T) { - // The replace variant shares all parsing with the append variant and only - // sets Replace=true; a representative case plus the flag-name in the error. - got, err := ParsePolicyInputFromFile("radamsa-min-iterations:min_iterations=values.csv:Iterations", true) - require.NoError(t, err) - assert.Equal(t, &PolicyInputFromFile{ - Policy: testPolicyRadamsa, - Input: testInputMinIter, - Column: "Iterations", - File: "values.csv", - Replace: true, - }, got) - - _, err = ParsePolicyInputFromFile(testInputMinIter, true) - require.Error(t, err) - assert.Contains(t, err.Error(), "--policy-input-from-file-replace") -} - func TestParsePolicyInput(t *testing.T) { testCases := []struct { name string diff --git a/pkg/policies/pfm6906_override_test.go b/pkg/policies/pfm6906_override_test.go index a8ddb45e3..8251fbc46 100644 --- a/pkg/policies/pfm6906_override_test.go +++ b/pkg/policies/pfm6906_override_test.go @@ -54,9 +54,9 @@ violations contains msg if { // - The old --policy-input-from-file behavior APPENDS the runtime value, so // getInputArguments yields the array ["100","10"] and to_number fails with // "eval_type_error: to_number ... got array" — the reported error. -// - The new --policy-input (and --policy-input-from-file-replace) behavior -// REPLACES the value via OverrideRuntimeInputs, so min_iterations stays the -// scalar "10", to_number succeeds, and the policy evaluates cleanly. +// - The new --policy-input behavior REPLACES the value via +// OverrideRuntimeInputs, so min_iterations stays the scalar "10", to_number +// succeeds, and the policy evaluates cleanly. func TestPFM6906ScalarOverrideFixesToNumberArrayError(t *testing.T) { eng := rego.NewEngine() policy := &engine.Policy{Name: "radamsa-min-iterations", Source: []byte(radamsaMinIterationsPolicy)} diff --git a/pkg/policies/runtime_inputs.go b/pkg/policies/runtime_inputs.go index a4132b987..9c14914fa 100644 --- a/pkg/policies/runtime_inputs.go +++ b/pkg/policies/runtime_inputs.go @@ -23,7 +23,7 @@ import ( ) // RuntimeInputs holds policy input values supplied at runtime (e.g. via -// --policy-input-from-file, --policy-input-from-file-replace or --policy-input). +// --policy-input-from-file or --policy-input). // Inputs are either global (applied to every policy attachment that declares // them) or scoped to a specific policy (applied only to the attachment whose // metadata name or ref matches the scope key). Independently, each input is From 4ccb7f611ceacf72fad261ce56fa873b76ada6eb Mon Sep 17 00:00:00 2001 From: Javier Rodriguez Date: Mon, 10 Aug 2026 12:00:49 +0200 Subject: [PATCH 5/6] refactor(cli): drop the no-op runtime warning on --append The reserved --append flag stays visible with help text noting it has no effect yet; the per-invocation warning was unnecessary noise and is removed. Assisted-by: Claude Code Signed-off-by: Javier Rodriguez Chainloop-Trace-Sessions: 8f1220ca-1af0-4865-89f3-9705c9df189c --- app/cli/cmd/attestation_add.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/app/cli/cmd/attestation_add.go b/app/cli/cmd/attestation_add.go index 813575e4c..da095bc9d 100644 --- a/app/cli/cmd/attestation_add.go +++ b/app/cli/cmd/attestation_add.go @@ -138,12 +138,6 @@ func newAttestationAddCmd() *cobra.Command { return err } - // --append is reserved: it will later toggle append/replace semantics - // for --policy-input and --policy-input-from-file, but does nothing yet. - if appendFlag { - logger.Warn().Msg("--append has no effect yet; reserved for a future release") - } - // In some cases, the attestation state is stored remotely. To control concurrency we use // optimistic locking. We retry the operation if the state has changed since we last read it. return runWithBackoffRetry( From 5dbf491c6f18869736d4f6f1f90de77248de0ab6 Mon Sep 17 00:00:00 2001 From: Javier Rodriguez Date: Mon, 10 Aug 2026 13:06:17 +0200 Subject: [PATCH 6/6] refactor(policies): simplify runtime-override name collection Collapse the two filter loops into one range over the append/replace maps and use slices.Sorted(maps.Keys(...)) instead of a manual collect-and-sort loop. The args[k] membership check is kept: ComputeArguments drops runtime keys the policy doesn't declare, so the field must report only overrides that actually applied. Assisted-by: Claude Code Signed-off-by: Javier Rodriguez Chainloop-Trace-Sessions: 8f1220ca-1af0-4865-89f3-9705c9df189c --- pkg/policies/policies.go | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/pkg/policies/policies.go b/pkg/policies/policies.go index efabfe2f2..b96d33d64 100644 --- a/pkg/policies/policies.go +++ b/pkg/policies/policies.go @@ -20,6 +20,7 @@ import ( "encoding/base64" "errors" "fmt" + "maps" "net/url" "path/filepath" "regexp" @@ -401,27 +402,22 @@ func (pv *PolicyVerifier) evaluatePolicyAttachment(ctx context.Context, attachme } // Record which runtime inputs actually applied to this policy (i.e. made it - // into the computed args because the policy declares them). The values - // themselves live in `with`; this only flags the overridden input names. - // Skipped entirely on the common path with no runtime inputs to avoid a - // per-attachment map allocation. + // into the computed args because the policy declares them — ComputeArguments + // drops runtime keys the policy doesn't declare). The values themselves live + // in `with`; this only flags the overridden input names, deduped across the + // append and replace maps. Skipped on the common path with no runtime inputs + // to avoid a per-attachment map allocation. var runtimeInputOverrides []string if len(appendRuntime) > 0 || len(replaceRuntime) > 0 { overrideNames := make(map[string]struct{}, len(appendRuntime)+len(replaceRuntime)) - for k := range appendRuntime { - if _, ok := args[k]; ok { - overrideNames[k] = struct{}{} - } - } - for k := range replaceRuntime { - if _, ok := args[k]; ok { - overrideNames[k] = struct{}{} + for _, m := range []map[string]string{appendRuntime, replaceRuntime} { + for k := range m { + if _, ok := args[k]; ok { + overrideNames[k] = struct{}{} + } } } - for k := range overrideNames { - runtimeInputOverrides = append(runtimeInputOverrides, k) - } - slices.Sort(runtimeInputOverrides) + runtimeInputOverrides = slices.Sorted(maps.Keys(overrideNames)) } sources := make([]string, 0)