From 77d87d71039c0dbfd632f8aedbc6a0a130716be3 Mon Sep 17 00:00:00 2001 From: Simon Castagna Date: Tue, 13 Oct 2026 14:30:57 +0200 Subject: [PATCH 1/5] feat(evaluate): evaluate policy takes --path, keeping --policy as a deprecated alias --path (-p) names the files to send, so --identifier can name a published policy next to it (server#7081). --policy stays one release because sdlc-policies runs the latest CLI and passes it. Co-Authored-By: Claude Opus 5.5 --- cmd/kosli/evaluateHelpers.go | 8 +- cmd/kosli/evaluatePolicy.go | 43 ++++++---- cmd/kosli/evaluatePolicy_test.go | 82 ++++++++++++++----- .../testdata/empty-flag-audit-coverage.json | 1 + hack/empty-flag-audit/spec.json | 4 +- 5 files changed, 97 insertions(+), 41 deletions(-) diff --git a/cmd/kosli/evaluateHelpers.go b/cmd/kosli/evaluateHelpers.go index 82cfd138f..fe554c45c 100644 --- a/cmd/kosli/evaluateHelpers.go +++ b/cmd/kosli/evaluateHelpers.go @@ -497,10 +497,10 @@ func policyBundle(refs []string) (map[string]string, error) { // Keeping either copy of a file the evaluator loads would decide // for the author which policy runs. if evaluatorLoads(name) { - return nil, fmt.Errorf("%s is in both --policy %s and --policy %s; "+ + return nil, fmt.Errorf("%s is in both --path %s and --path %s; "+ "the evaluator would load only one of them", name, first, ref) } - logger.Warn("%s from --policy %s is not sent, as --policy %s already sends one", name, ref, first) + logger.Warn("%s from --path %s is not sent, as --path %s already sends one", name, ref, first) } } @@ -542,10 +542,10 @@ func policyRoots(refs []string) ([]string, error) { break } if isWithin(absolute, other.absolute) { - return nil, fmt.Errorf("--policy %s is inside --policy %s, so its files would be sent twice", ref, other.ref) + return nil, fmt.Errorf("--path %s is inside --path %s, so its files would be sent twice", ref, other.ref) } if isWithin(other.absolute, absolute) { - return nil, fmt.Errorf("--policy %s is inside --policy %s, so its files would be sent twice", other.ref, ref) + return nil, fmt.Errorf("--path %s is inside --path %s, so its files would be sent twice", other.ref, ref) } } if !repeated { diff --git a/cmd/kosli/evaluatePolicy.go b/cmd/kosli/evaluatePolicy.go index e2fc12523..910b3b93f 100644 --- a/cmd/kosli/evaluatePolicy.go +++ b/cmd/kosli/evaluatePolicy.go @@ -19,9 +19,9 @@ recorded it, and the verdict is printed here. Name what to evaluate with ` + "`--context trail=/`" + `, repeated once per trail. Trails named in one command are all evaluated at the same instant. -` + "`--policy`" + ` takes a Rego file or a directory, and can be given more than once, +` + "`--path`" + ` takes a Rego file or a directory, and can be given more than once, so a policy and a shared library are sent together from wherever each is kept. -Every ` + "`--policy`" + ` joins one bundle. A file is keyed by its own name, and a file +Every ` + "`--path`" + ` joins one bundle. A file is keyed by its own name, and a file in a directory by its path relative to that directory. Files in dot-directories, such as ` + "`.git`" + `, are left out; every other file travels, so notes such as ` + "`README.md`" + ` stay with the policy. @@ -36,7 +36,7 @@ Of what travels, the evaluator loads only: Every other file is carried but not read. Two loaded files under one name are refused, naming both. For any other file the -first ` + "`--policy`" + ` given wins, and the copy left behind is named on stderr. +first ` + "`--path`" + ` given wins, and the copy left behind is named on stderr. The policy package is the one that declares ` + "`allow`" + `; every other package is a library. Where several packages declare ` + "`allow`" + `, annotate an entrypoint in exactly @@ -63,15 +63,15 @@ const evaluatePolicyExample = ` # evaluate a policy against a trail: kosli evaluate policy \ --context trail=yourFlowName/yourTrailName \ - --policy yourPolicyFile.rego \ + --path yourPolicyFile.rego \ --api-token yourAPIToken \ --org yourOrgName # evaluate a policy directory with a shared library kept elsewhere: kosli evaluate policy \ --context trail=yourFlowName/yourTrailName \ - --policy yourPolicyDirectory \ - --policy path/to/ergo/ergo.rego \ + --path yourPolicyDirectory \ + --path path/to/ergo/ergo.rego \ --api-token yourAPIToken \ --org yourOrgName @@ -79,14 +79,14 @@ kosli evaluate policy \ kosli evaluate policy \ --context trail=yourFlowName/yourTrailName \ --context trail=anotherFlowName/anotherTrailName \ - --policy yourPolicyFile.rego \ + --path yourPolicyFile.rego \ --api-token yourAPIToken \ --org yourOrgName # evaluate a policy with parameters (inline JSON or @file.json): kosli evaluate policy \ --context trail=yourFlowName/yourTrailName \ - --policy yourPolicyFile.rego \ + --path yourPolicyFile.rego \ --params '{"protected_branch": "master"}' \ --api-token yourAPIToken \ --org yourOrgName @@ -94,7 +94,7 @@ kosli evaluate policy \ # evaluate a policy and record the outcome as a decision: kosli evaluate policy \ --context trail=yourFlowName/yourTrailName \ - --policy yourPolicyFile.rego \ + --path yourPolicyFile.rego \ --control yourControlIdentifier \ --flow yourFlowName \ --trail yourTrailName \ @@ -105,14 +105,15 @@ kosli evaluate policy \ # evaluate a policy and fail the step when it denies: kosli evaluate policy \ --context trail=yourFlowName/yourTrailName \ - --policy yourPolicyFile.rego \ + --path yourPolicyFile.rego \ --assert \ --api-token yourAPIToken \ --org yourOrgName` type evaluatePolicyOptions struct { contexts []string - policyRefs []string + paths []string + oldPolicies []string params string output string assert bool @@ -147,7 +148,9 @@ func newEvaluatePolicyCmd(out io.Writer) *cobra.Command { } cmd.Flags().StringArrayVar(&o.contexts, "context", []string{}, policyContextFlag) - cmd.Flags().StringArrayVarP(&o.policyRefs, "policy", "p", []string{}, "Path of a Rego policy file, or of a directory sent as one bundle. Repeat it to send a policy with libraries kept elsewhere.") + cmd.Flags().StringArrayVarP(&o.paths, "path", "p", []string{}, "Path of a Rego policy file, or of a directory sent as one bundle. Repeat it to send a policy with libraries kept elsewhere.") + // Kept one release because sdlc-policies runs the latest CLI and passes --policy. + cmd.Flags().StringArrayVar(&o.oldPolicies, "policy", []string{}, "Deprecated alias of --path.") cmd.Flags().StringVar(&o.params, "params", "", policyParamsFlag) cmd.Flags().StringVarP(&o.output, "output", "o", "table", outputFlag) cmd.Flags().BoolVar(&o.assert, "assert", false, policyAssertFlag) @@ -157,10 +160,14 @@ func newEvaluatePolicyCmd(out io.Writer) *cobra.Command { cmd.Flags().StringVar(&o.name, "name", "", policyDecisionNameFlag) cmd.Flags().StringVar(&o.fingerprint, "fingerprint", "", policyDecisionFingerprintFlag) - err := RequireFlags(cmd, []string{"context", "policy"}) + err := RequireFlags(cmd, []string{"context"}) if err != nil { logger.Error("failed to configure required flags: %v", err) } + err = DeprecateFlags(cmd, map[string]string{"policy": "use --path instead"}) + if err != nil { + logger.Error("failed to deprecate flags: %v", err) + } return cmd } @@ -168,9 +175,13 @@ func newEvaluatePolicyCmd(out io.Writer) *cobra.Command { func (o *evaluatePolicyOptions) run(out io.Writer) error { // Fetching a policy from a URL is on its way out, so this command does not // offer it, though the older evaluate commands still do. - for _, ref := range o.policyRefs { + policyRefs := append(append([]string{}, o.paths...), o.oldPolicies...) + if len(policyRefs) == 0 { + return fmt.Errorf("--path is required") + } + for _, ref := range policyRefs { if isRemotePolicyRef(ref) { - return fmt.Errorf("--policy takes a file or a directory on this machine, not a URL") + return fmt.Errorf("--path takes a file or a directory on this machine, not a URL") } } @@ -191,7 +202,7 @@ func (o *evaluatePolicyOptions) run(out io.Writer) error { } return runServerEvaluation(out, serverEvaluation{ - policyRefs: o.policyRefs, + policyRefs: policyRefs, params: o.params, trails: trails, decision: decision, diff --git a/cmd/kosli/evaluatePolicy_test.go b/cmd/kosli/evaluatePolicy_test.go index a2f723399..32948973e 100644 --- a/cmd/kosli/evaluatePolicy_test.go +++ b/cmd/kosli/evaluatePolicy_test.go @@ -22,7 +22,7 @@ type EvaluatePolicyCommandTestSuite struct { func (suite *EvaluatePolicyCommandTestSuite) cmd(host, extra string) string { return fmt.Sprintf( "evaluate policy --context trail=my-flow/my-trail "+ - "--policy testdata/policies/allow-all.rego "+ + "--path testdata/policies/allow-all.rego "+ "--host %s --org test-org --api-token test-token --max-api-retries 0 %s", host, extra) } @@ -49,12 +49,54 @@ func (suite *EvaluatePolicyCommandTestSuite) TestItOffersOnlyItsOwnFlags() { _, combined, _, _, err := executeCommandC("evaluate policy --help") require.NoError(suite.T(), err) - for _, flag := range []string{"--context", "--policy", "--params", "--output", "--assert"} { + for _, flag := range []string{"--context", "--path", "--params", "--output", "--assert"} { require.Contains(suite.T(), combined, flag) } - for _, flag := range []string{"--server-side", "--attestations", "--show-input", "--no-assert", "--sync"} { + for _, flag := range []string{"--server-side", "--attestations", "--show-input", "--no-assert", "--sync", "--policy"} { require.NotContains(suite.T(), combined, flag) } + require.Contains(suite.T(), combined, "-p, --path") +} + +func (suite *EvaluatePolicyCommandTestSuite) TestTheShorthandSendsThePath() { + server, fake := newFakeEvaluations(suite.T(), verdictAllowed) + + _, _, _, _, err := executeCommandC(fmt.Sprintf( + "evaluate policy --context trail=my-flow/my-trail -p testdata/policies/allow-all.rego "+ + "--host %s --org test-org --api-token test-token --max-api-retries 0", server.URL)) + + require.NoError(suite.T(), err) + files := fake.created[0]["policy"].(map[string]any)["files"].(map[string]any) + require.Equal(suite.T(), []string{"allow-all.rego"}, sortedKeys(files)) +} + +// --policy stays one release, because sdlc-policies runs the latest CLI and passes it. +func (suite *EvaluatePolicyCommandTestSuite) TestTheOldPolicyFlagStillSendsTheBundleAndWarns() { + server, fake := newFakeEvaluations(suite.T(), verdictAllowed) + + _, combined, _, _, err := executeCommandC(fmt.Sprintf( + "evaluate policy --context trail=my-flow/my-trail --policy testdata/policies/allow-all.rego "+ + "--host %s --org test-org --api-token test-token --max-api-retries 0", server.URL)) + + require.NoError(suite.T(), err) + files := fake.created[0]["policy"].(map[string]any)["files"].(map[string]any) + require.Equal(suite.T(), []string{"allow-all.rego"}, sortedKeys(files)) + require.Contains(suite.T(), combined, "Flag --policy has been deprecated, use --path instead") +} + +func (suite *EvaluatePolicyCommandTestSuite) TestThePathAndTheOldPolicyFlagJoinOneBundle() { + server, fake := newFakeEvaluations(suite.T(), verdictAllowed) + + _, _, _, _, err := executeCommandC(fmt.Sprintf( + "evaluate policy --context trail=my-flow/my-trail --path testdata/policies/bundle "+ + "--policy testdata/policies/allow-all.rego "+ + "--host %s --org test-org --api-token test-token --max-api-retries 0", server.URL)) + + require.NoError(suite.T(), err) + require.Len(suite.T(), fake.created, 1) + files := fake.created[0]["policy"].(map[string]any)["files"].(map[string]any) + require.Contains(suite.T(), files, "allow-all.rego") + require.Greater(suite.T(), len(files), 1) } // An author cannot see the evaluator, so the help is where they learn which @@ -65,7 +107,7 @@ func (suite *EvaluatePolicyCommandTestSuite) TestTheHelpSaysWhatTravelsAndWhatIs require.NoError(suite.T(), err) for _, says := range []string{ "more than once", "*_test.rego", "data.json", "data.yaml", "data.yml", - "*.ergo.yaml", "*.ergo.yml", "README.md", "dot-directories", "first `--policy`", + "*.ergo.yaml", "*.ergo.yml", "README.md", "dot-directories", "first `--path`", } { require.Contains(suite.T(), help, says) } @@ -89,8 +131,8 @@ func (suite *EvaluatePolicyCommandTestSuite) TestItNamesTheRequiredFlagItWasNotG missing string cmd string }{ - {"context", "evaluate policy --policy testdata/policies/allow-all.rego"}, - {"policy", "evaluate policy --context trail=my-flow/my-trail"}, + {"context", "evaluate policy --path testdata/policies/allow-all.rego"}, + {"--path", "evaluate policy --context trail=my-flow/my-trail"}, } { suite.Run(test.missing, func() { _, _, _, _, err := executeCommandC(test.cmd + " --org test-org --api-token test-token") @@ -294,7 +336,7 @@ func (suite *EvaluatePolicyCommandTestSuite) TestAMalformedContextIsRefusedBefor server, fake := newFakeEvaluations(suite.T(), verdictAllowed) _, combined, _, _, err := executeCommandC(fmt.Sprintf( - "evaluate policy --context %s --policy testdata/policies/allow-all.rego "+ + "evaluate policy --context %s --path testdata/policies/allow-all.rego "+ "--host %s --org test-org --api-token test-token --max-api-retries 0", test.value, server.URL)) @@ -316,7 +358,7 @@ func (suite *EvaluatePolicyCommandTestSuite) TestTooManyContextsAreRefusedBefore } _, _, _, _, err := executeCommandC(fmt.Sprintf( - "evaluate policy %s--policy testdata/policies/allow-all.rego "+ + "evaluate policy %s--path testdata/policies/allow-all.rego "+ "--host %s --org test-org --api-token test-token --max-api-retries 0", contexts, server.URL)) @@ -527,7 +569,7 @@ func (suite *EvaluatePolicyCommandTestSuite) TestADirectoryTravelsAsOneBundle() server, fake := newFakeEvaluations(suite.T(), verdictAllowed) _, _, _, _, err := executeCommandC(fmt.Sprintf( - "evaluate policy --context trail=my-flow/my-trail --policy testdata/policies/bundle "+ + "evaluate policy --context trail=my-flow/my-trail --path testdata/policies/bundle "+ "--host %s --org test-org --api-token test-token --max-api-retries 0", server.URL)) require.NoError(suite.T(), err) @@ -548,7 +590,7 @@ func (suite *EvaluatePolicyCommandTestSuite) TestADirectoryLeavesItsDotDirectori server, fake := newFakeEvaluations(suite.T(), verdictAllowed) _, _, _, _, err := executeCommandC(fmt.Sprintf( - "evaluate policy --context trail=my-flow/my-trail --policy %s "+ + "evaluate policy --context trail=my-flow/my-trail --path %s "+ "--host %s --org test-org --api-token test-token --max-api-retries 0", directory, server.URL)) require.NoError(suite.T(), err) @@ -559,7 +601,7 @@ func (suite *EvaluatePolicyCommandTestSuite) TestADirectoryLeavesItsDotDirectori func (suite *EvaluatePolicyCommandTestSuite) policyCmd(host string, roots ...string) string { policies := "" for _, root := range roots { - policies += " --policy " + root + policies += " --path " + root } return fmt.Sprintf("evaluate policy --context trail=my-flow/my-trail%s "+ "--host %s --org test-org --api-token test-token --max-api-retries 0", policies, host) @@ -578,7 +620,7 @@ func (suite *EvaluatePolicyCommandTestSuite) TestAPolicyAndALibraryTravelAsOneBu _, _, _, _, err := executeCommandC(suite.policyCmd(server.URL, control, filepath.Join(library, "ergo.rego"))) require.NoError(suite.T(), err) - require.Len(suite.T(), fake.created, 1, "one evaluation, not one per --policy") + require.Len(suite.T(), fake.created, 1, "one evaluation, not one per --path") files := fake.created[0]["policy"].(map[string]any)["files"].(map[string]any) require.Equal(suite.T(), []string{"README.md", "ergo.rego", "policy.rego"}, sortedKeys(files)) require.Equal(suite.T(), "package ergo\n", files["ergo.rego"]) @@ -616,8 +658,8 @@ func (suite *EvaluatePolicyCommandTestSuite) TestTwoLoadedFilesUnderOneNameAreRe require.Error(suite.T(), err) require.Contains(suite.T(), err.Error(), name) - require.Contains(suite.T(), err.Error(), first) - require.Contains(suite.T(), err.Error(), second) + require.Contains(suite.T(), err.Error(), "--path "+first) + require.Contains(suite.T(), err.Error(), "--path "+second) require.Empty(suite.T(), fake.created, "nothing is sent") }) } @@ -679,7 +721,7 @@ func (suite *EvaluatePolicyCommandTestSuite) TestARootInsideAnotherIsRefused() { _, _, _, _, err := executeCommandC(suite.policyCmd(server.URL, roots...)) require.Error(suite.T(), err) - require.Contains(suite.T(), err.Error(), roots[0]) + require.Contains(suite.T(), err.Error(), "--path "+roots[0]) require.Contains(suite.T(), err.Error(), roots[1]) require.Empty(suite.T(), fake.created, "nothing is sent") }) @@ -708,7 +750,7 @@ func (suite *EvaluatePolicyCommandTestSuite) TestAnEmptyDirectoryIsRefused() { server, fake := newFakeEvaluations(suite.T(), verdictAllowed) _, _, _, _, err := executeCommandC(fmt.Sprintf( - "evaluate policy --context trail=my-flow/my-trail --policy %s "+ + "evaluate policy --context trail=my-flow/my-trail --path %s "+ "--host %s --org test-org --api-token test-token --max-api-retries 0", directory, server.URL)) require.Error(suite.T(), err) @@ -744,7 +786,7 @@ func (suite *EvaluatePolicyCommandTestSuite) TestABundleOverTheCapsIsRefusedWith server, fake := newFakeEvaluations(suite.T(), verdictAllowed) _, _, _, _, err := executeCommandC(fmt.Sprintf( - "evaluate policy --context trail=my-flow/my-trail --policy %s "+ + "evaluate policy --context trail=my-flow/my-trail --path %s "+ "--host %s --org test-org --api-token test-token --max-api-retries 0", directory, server.URL)) @@ -762,11 +804,11 @@ func (suite *EvaluatePolicyCommandTestSuite) TestARemotePolicyIsRefused() { server, fake := newFakeEvaluations(suite.T(), verdictAllowed) _, combined, _, _, err := executeCommandC(fmt.Sprintf( - "evaluate policy --context trail=my-flow/my-trail --policy %s "+ + "evaluate policy --context trail=my-flow/my-trail --path %s "+ "--host %s --org test-org --api-token test-token --max-api-retries 0", ref, server.URL)) require.Error(suite.T(), err) - require.Contains(suite.T(), err.Error(), "--policy") + require.Contains(suite.T(), err.Error(), "--path") require.Empty(suite.T(), fake.created, "nothing is sent") require.NotContains(suite.T(), combined, "RESULT") }) @@ -778,7 +820,7 @@ func (suite *EvaluatePolicyCommandTestSuite) TestADryRunSendsNothing() { _, combined, _, _, err := executeCommandC(fmt.Sprintf( "evaluate policy --context trail=my-flow/my-trail "+ - "--policy testdata/policies/allow-all.rego "+ + "--path testdata/policies/allow-all.rego "+ "--host %s --org test-org --api-token DRY_RUN --max-api-retries 0", server.URL)) require.NoError(suite.T(), err) diff --git a/cmd/kosli/testdata/empty-flag-audit-coverage.json b/cmd/kosli/testdata/empty-flag-audit-coverage.json index 25a0718f0..dadfe5978 100644 --- a/cmd/kosli/testdata/empty-flag-audit-coverage.json +++ b/cmd/kosli/testdata/empty-flag-audit-coverage.json @@ -608,6 +608,7 @@ "name": "string", "output": "string", "params": "string", + "path": "stringArray", "policy": "stringArray", "trail": "string" }, diff --git a/hack/empty-flag-audit/spec.json b/hack/empty-flag-audit/spec.json index 53587a7d9..387d570b3 100644 --- a/hack/empty-flag-audit/spec.json +++ b/hack/empty-flag-audit/spec.json @@ -4918,7 +4918,7 @@ "args": [], "flags": { "context": "trail={flow}/{trail}", - "policy": "cmd/kosli/testdata/policies/allow-all.rego" + "path": "cmd/kosli/testdata/policies/allow-all.rego" }, "baseline_ok": false, "error": "the evaluation is created and run away from this machine, and the server the audit runs against does not offer it", @@ -4932,6 +4932,7 @@ "name", "output", "params", + "path", "policy", "trail" ], @@ -4944,6 +4945,7 @@ "name": "{name}", "output": "json", "params": "{}", + "path": "cmd/kosli/testdata/policies/allow-all.rego", "policy": "cmd/kosli/testdata/policies/allow-all.rego", "trail": "{trail}" }, From f903cdb76fa17d21cc4d03986a79b3f0892d4356 Mon Sep 17 00:00:00 2001 From: Simon Castagna Date: Tue, 13 Oct 2026 14:36:52 +0200 Subject: [PATCH 2/5] feat(evaluate): evaluate policy --identifier runs a published rego policy Sends policy: {"identifier": ...} so the server evaluates the policy's current version (server#7081). Exactly one of --identifier or --path is required, checked on resolved values so a KOSLI_IDENTIFIER exported by a pipeline cannot silently win or lose against --path. Co-Authored-By: Claude Opus 5.5 --- cmd/kosli/evaluateHelpers.go | 19 ++-- cmd/kosli/evaluatePolicy.go | 27 +++++- cmd/kosli/evaluatePolicy_test.go | 91 ++++++++++++++++++- .../testdata/empty-flag-audit-coverage.json | 1 + hack/empty-flag-audit/spec.json | 2 + internal/evaluations/client.go | 21 +++-- internal/evaluations/client_test.go | 12 +++ 7 files changed, 154 insertions(+), 19 deletions(-) diff --git a/cmd/kosli/evaluateHelpers.go b/cmd/kosli/evaluateHelpers.go index fe554c45c..a39af5de7 100644 --- a/cmd/kosli/evaluateHelpers.go +++ b/cmd/kosli/evaluateHelpers.go @@ -297,6 +297,7 @@ func evaluateServerSide(out io.Writer, o *commonEvaluateOptions, trails []evalua } type serverEvaluation struct { + identifier string policyRefs []string params string trails []evaluations.TrailRef @@ -320,17 +321,21 @@ func runServerEvaluation(out io.Writer, spec serverEvaluation) error { return err } - files, err := policyBundle(spec.policyRefs) - if err != nil { - return err + var files map[string]string + if spec.identifier == "" { + files, err = policyBundle(spec.policyRefs) + if err != nil { + return err + } } client := evaluations.NewClient(kosliClient, global.Host, global.ApiToken, global.DryRun) created, err := client.Create(global.Org, evaluations.CreateRequest{ - Trails: spec.trails, - Files: files, - Params: params, - Decision: spec.decision, + Trails: spec.trails, + Files: files, + Identifier: spec.identifier, + Params: params, + Decision: spec.decision, }) if err != nil { return serverSideRequestError(err) diff --git a/cmd/kosli/evaluatePolicy.go b/cmd/kosli/evaluatePolicy.go index 910b3b93f..3594c4d9b 100644 --- a/cmd/kosli/evaluatePolicy.go +++ b/cmd/kosli/evaluatePolicy.go @@ -19,6 +19,11 @@ recorded it, and the verdict is printed here. Name what to evaluate with ` + "`--context trail=/`" + `, repeated once per trail. Trails named in one command are all evaluated at the same instant. +Name the policy with one of: + - ` + "`--identifier`" + `: a rego policy published to Kosli. Its current version, the + highest-numbered valid one, is evaluated, and the evaluation cites that version. + - ` + "`--path`" + `: the policy's files on this machine, sent with the evaluation. + ` + "`--path`" + ` takes a Rego file or a directory, and can be given more than once, so a policy and a shared library are sent together from wherever each is kept. Every ` + "`--path`" + ` joins one bundle. A file is keyed by its own name, and a file @@ -55,7 +60,8 @@ Pass ` + "`--control`" + ` to record the outcome as a decision against that cont ` + "`--flow`" + ` and ` + "`--trail`" + ` given. The decision is recorded where the policy runs, so the verdict is never asserted from here. Without ` + "`--control`" + ` nothing is recorded. -Use ` + "`--params`" + ` to pass values the policy reads as ` + "`data.params`" + `. +Use ` + "`--params`" + ` to pass values the policy reads as ` + "`data.params`" + `. For a +published policy, they must match the params schema of its current version. Pass ` + "`--assert`" + ` to exit with a non-zero status when the policy denies. Use ` + "`--output json`" + ` for structured output.` @@ -67,6 +73,13 @@ kosli evaluate policy \ --api-token yourAPIToken \ --org yourOrgName +# evaluate the current version of a published rego policy against a trail: +kosli evaluate policy \ + --context trail=yourFlowName/yourTrailName \ + --identifier yourPolicyIdentifier \ + --api-token yourAPIToken \ + --org yourOrgName + # evaluate a policy directory with a shared library kept elsewhere: kosli evaluate policy \ --context trail=yourFlowName/yourTrailName \ @@ -112,6 +125,7 @@ kosli evaluate policy \ type evaluatePolicyOptions struct { contexts []string + identifier string paths []string oldPolicies []string params string @@ -148,6 +162,7 @@ func newEvaluatePolicyCmd(out io.Writer) *cobra.Command { } cmd.Flags().StringArrayVar(&o.contexts, "context", []string{}, policyContextFlag) + cmd.Flags().StringVar(&o.identifier, "identifier", "", "Identifier of a rego policy published to Kosli. Its current version is evaluated.") cmd.Flags().StringArrayVarP(&o.paths, "path", "p", []string{}, "Path of a Rego policy file, or of a directory sent as one bundle. Repeat it to send a policy with libraries kept elsewhere.") // Kept one release because sdlc-policies runs the latest CLI and passes --policy. cmd.Flags().StringArrayVar(&o.oldPolicies, "policy", []string{}, "Deprecated alias of --path.") @@ -176,8 +191,13 @@ func (o *evaluatePolicyOptions) run(out io.Writer) error { // Fetching a policy from a URL is on its way out, so this command does not // offer it, though the older evaluate commands still do. policyRefs := append(append([]string{}, o.paths...), o.oldPolicies...) - if len(policyRefs) == 0 { - return fmt.Errorf("--path is required") + // Checked on resolved values, so a KOSLI_IDENTIFIER exported by a pipeline + // is refused next to --path rather than quietly winning or losing. + if o.identifier != "" && len(policyRefs) > 0 { + return fmt.Errorf("only one of --identifier or --path can be given") + } + if o.identifier == "" && len(policyRefs) == 0 { + return fmt.Errorf("one of --identifier or --path is required") } for _, ref := range policyRefs { if isRemotePolicyRef(ref) { @@ -202,6 +222,7 @@ func (o *evaluatePolicyOptions) run(out io.Writer) error { } return runServerEvaluation(out, serverEvaluation{ + identifier: o.identifier, policyRefs: policyRefs, params: o.params, trails: trails, diff --git a/cmd/kosli/evaluatePolicy_test.go b/cmd/kosli/evaluatePolicy_test.go index 32948973e..9b316b1ba 100644 --- a/cmd/kosli/evaluatePolicy_test.go +++ b/cmd/kosli/evaluatePolicy_test.go @@ -126,13 +126,102 @@ func (suite *EvaluatePolicyCommandTestSuite) TestTheHelpSaysWhichRulesAreEvaluat } } +func (suite *EvaluatePolicyCommandTestSuite) identifierCmd(host, extra string) string { + return fmt.Sprintf( + "evaluate policy --context trail=my-flow/my-trail --identifier pr-approved "+ + "--host %s --org test-org --api-token test-token --max-api-retries 0 %s", + host, extra) +} + +func (suite *EvaluatePolicyCommandTestSuite) TestAnIdentifierSendsOnlyTheIdentifier() { + server, fake := newFakeEvaluations(suite.T(), verdictAllowed) + + _, combined, _, _, err := executeCommandC(suite.identifierCmd(server.URL, "")) + + require.NoError(suite.T(), err) + require.Len(suite.T(), fake.created, 1) + require.Equal(suite.T(), map[string]any{"identifier": "pr-approved"}, fake.created[0]["policy"]) + require.Contains(suite.T(), combined, "RESULT") +} + +func (suite *EvaluatePolicyCommandTestSuite) TestTheHelpOffersTheIdentifier() { + _, help, _, _, err := executeCommandC("evaluate policy --help") + + require.NoError(suite.T(), err) + require.Contains(suite.T(), help, "--identifier") + require.Contains(suite.T(), help, "current version") +} + +func (suite *EvaluatePolicyCommandTestSuite) TestAnIdentifierAndFilesAreRefusedTogether() { + for _, files := range []string{"--path testdata/policies/allow-all.rego", "--policy testdata/policies/allow-all.rego"} { + suite.Run(files, func() { + server, fake := newFakeEvaluations(suite.T(), verdictAllowed) + + _, _, _, _, err := executeCommandC(suite.identifierCmd(server.URL, files)) + + require.Error(suite.T(), err) + require.Contains(suite.T(), err.Error(), "only one of --identifier or --path can be given") + require.Empty(suite.T(), fake.created, "nothing is sent") + }) + } +} + +// KOSLI_IDENTIFIER binds to --identifier, so a pipeline that exports it must +// not have a --path quietly ignored or quietly preferred. +func (suite *EvaluatePolicyCommandTestSuite) TestAnIdentifierFromTheEnvironmentIsRefusedWithAPath() { + suite.T().Setenv("KOSLI_IDENTIFIER", "pr-approved") + server, fake := newFakeEvaluations(suite.T(), verdictAllowed) + + _, _, _, _, err := executeCommandC(suite.cmd(server.URL, "")) + + require.Error(suite.T(), err) + require.Contains(suite.T(), err.Error(), "only one of --identifier or --path can be given") + require.Empty(suite.T(), fake.created, "nothing is sent") +} + +func (suite *EvaluatePolicyCommandTestSuite) TestAnEmptyIdentifierIsRefused() { + server, fake := newFakeEvaluations(suite.T(), verdictAllowed) + + _, _, _, _, err := executeCommandC(fmt.Sprintf( + "evaluate policy --context trail=my-flow/my-trail --identifier '' "+ + "--host %s --org test-org --api-token test-token --max-api-retries 0", server.URL)) + + require.Error(suite.T(), err) + require.Contains(suite.T(), err.Error(), "flag '--identifier' was given an empty value") + require.Empty(suite.T(), fake.created, "nothing is sent") +} + +func (suite *EvaluatePolicyCommandTestSuite) TestAPublishedPolicyRefusalIsReportedAsTheServerPutIt() { + for _, test := range []struct { + name string + status int + message string + }{ + {"an unknown policy", 404, "Rego policy 'pr-approved' does not exist in org 'test-org'"}, + {"no valid version", 400, "Rego policy 'pr-approved' has no valid version to evaluate"}, + {"too many trails", 400, "The policy evaluates one trail, but 2 were given"}, + {"params off the schema", 400, "Params do not match the params schema of version 3: threshold: 'x' is not of type 'integer'"}, + } { + suite.Run(test.name, func() { + server := newRefusingServer(suite.T(), test.status, + fmt.Sprintf(`{"message":%q}`, test.message)) + + _, combined, _, _, err := executeCommandC(suite.identifierCmd(server.URL, "")) + + require.Error(suite.T(), err) + require.Contains(suite.T(), err.Error(), test.message) + require.NotContains(suite.T(), combined, "RESULT") + }) + } +} + func (suite *EvaluatePolicyCommandTestSuite) TestItNamesTheRequiredFlagItWasNotGiven() { for _, test := range []struct { missing string cmd string }{ {"context", "evaluate policy --path testdata/policies/allow-all.rego"}, - {"--path", "evaluate policy --context trail=my-flow/my-trail"}, + {"one of --identifier or --path is required", "evaluate policy --context trail=my-flow/my-trail"}, } { suite.Run(test.missing, func() { _, _, _, _, err := executeCommandC(test.cmd + " --org test-org --api-token test-token") diff --git a/cmd/kosli/testdata/empty-flag-audit-coverage.json b/cmd/kosli/testdata/empty-flag-audit-coverage.json index dadfe5978..03d1f6c6a 100644 --- a/cmd/kosli/testdata/empty-flag-audit-coverage.json +++ b/cmd/kosli/testdata/empty-flag-audit-coverage.json @@ -605,6 +605,7 @@ "control": "string", "fingerprint": "string", "flow": "string", + "identifier": "string", "name": "string", "output": "string", "params": "string", diff --git a/hack/empty-flag-audit/spec.json b/hack/empty-flag-audit/spec.json index 387d570b3..a28e887c1 100644 --- a/hack/empty-flag-audit/spec.json +++ b/hack/empty-flag-audit/spec.json @@ -4929,6 +4929,7 @@ "control", "fingerprint", "flow", + "identifier", "name", "output", "params", @@ -4942,6 +4943,7 @@ "control": "{control}", "fingerprint": "1bef738d0bb1e690500f99a5b57d958caf3a5eb3e00d9012e1f4369fc6812e01", "flow": "{flow}", + "identifier": "pr-approved", "name": "{name}", "output": "json", "params": "{}", diff --git a/internal/evaluations/client.go b/internal/evaluations/client.go index c7c04d6af..1e4a3fdb5 100644 --- a/internal/evaluations/client.go +++ b/internal/evaluations/client.go @@ -32,10 +32,13 @@ type TrailRef struct { // single instant, which is why several trails belong in one request rather // than in one request each. type CreateRequest struct { - Trails []TrailRef - Files map[string]string - Params map[string]any - Decision *Decision + Trails []TrailRef + // Exactly one of Files and Identifier is set: Files sends the policy + // inline, Identifier names a published one whose current version runs. + Files map[string]string + Identifier string + Params map[string]any + Decision *Decision } // Decision is where an evaluation records its outcome, and against which @@ -129,7 +132,7 @@ func (c *Client) Create(org string, request CreateRequest) (*Evaluation, error) DryRun: c.dryRun, Payload: createPayload{ Context: createContext{Trails: request.Trails}, - Policy: inlinePolicy{Files: request.Files}, + Policy: createPolicy{Files: request.Files, Identifier: request.Identifier}, Params: params, Decision: request.Decision, }, @@ -148,7 +151,7 @@ func (c *Client) Create(org string, request CreateRequest) (*Evaluation, error) // to be stated exactly here rather than inherited from a caller's struct. type createPayload struct { Context createContext `json:"context"` - Policy inlinePolicy `json:"policy"` + Policy createPolicy `json:"policy"` Params map[string]any `json:"params"` Decision *Decision `json:"decision,omitempty"` } @@ -157,8 +160,10 @@ type createContext struct { Trails []TrailRef `json:"trails"` } -type inlinePolicy struct { - Files map[string]string `json:"files"` +// The server takes one of the two variants and refuses a policy with both. +type createPolicy struct { + Files map[string]string `json:"files,omitempty"` + Identifier string `json:"identifier,omitempty"` } func decodeEvaluation(body string) (*Evaluation, error) { diff --git a/internal/evaluations/client_test.go b/internal/evaluations/client_test.go index 5e8aec5b7..e89a7155e 100644 --- a/internal/evaluations/client_test.go +++ b/internal/evaluations/client_test.go @@ -107,6 +107,18 @@ func TestCreateSendsExactlyTheAgreedPayload(t *testing.T) { require.Equal(t, map[string]any{"threshold": float64(2)}, seen.body["params"]) } +func TestCreateSendsOnlyTheIdentifierOfAPublishedPolicy(t *testing.T) { + server, seen := newFakeServer(t, http.StatusCreated, createdBody) + request := aCreateRequest() + request.Files = nil + request.Identifier = "pr-approved" + + _, err := newTestClient(t, server.URL, false).Create("my-org", request) + require.NoError(t, err) + + require.Equal(t, map[string]any{"identifier": "pr-approved"}, seen.body["policy"]) +} + func TestCreateSendsEveryTrailInOneEvaluation(t *testing.T) { server, seen := newFakeServer(t, http.StatusCreated, createdBody) From ee4c2a6d44eb45a128ff72587f19bb3e5f9af4f2 Mon Sep 17 00:00:00 2001 From: Simon Castagna Date: Tue, 13 Oct 2026 14:42:33 +0200 Subject: [PATCH 3/5] feat(evaluate): evaluate policy prints the policy version that ran The evaluation cites the policy it ran (server#7081): a published policy by identifier, version and digest, an inline one by its digest. It is printed as a POLICY row and a policy object in JSON. evaluate trail is unchanged, so its server-side path still prints what the local path does. Co-Authored-By: Claude Opus 5.5 --- cmd/kosli/evaluateHelpers.go | 38 ++++++++++++++--- cmd/kosli/evaluateHelpers_test.go | 10 ++++- cmd/kosli/evaluatePolicy.go | 7 ++- cmd/kosli/evaluatePolicy_test.go | 64 ++++++++++++++++++++++++++++ cmd/kosli/evaluateServerSide_test.go | 8 ++++ internal/evaluations/client.go | 22 +++++++--- internal/evaluations/client_test.go | 12 ++++++ 7 files changed, 146 insertions(+), 15 deletions(-) diff --git a/cmd/kosli/evaluateHelpers.go b/cmd/kosli/evaluateHelpers.go index a39af5de7..7742a20fb 100644 --- a/cmd/kosli/evaluateHelpers.go +++ b/cmd/kosli/evaluateHelpers.go @@ -276,7 +276,7 @@ func evaluateAndPrintResult(out io.Writer, policyRef string, input map[string]an logger.Warn("--output-rule values are only shown with --output json") } - return printEvaluateResult(out, result, input, outputFormat, showInput, params, assertOnDeny, "") + return printEvaluateResult(out, result, input, outputFormat, showInput, params, assertOnDeny, "", nil) } // evaluateServerSide asks the Kosli server to evaluate the named trails and @@ -297,8 +297,11 @@ func evaluateServerSide(out io.Writer, o *commonEvaluateOptions, trails []evalua } type serverEvaluation struct { - identifier string - policyRefs []string + identifier string + policyRefs []string + // Only evaluate policy prints the policy that ran: the evaluate trail + // commands must print the same bytes on either path. + citePolicy bool params string trails []evaluations.TrailRef decision *evaluations.Decision @@ -364,8 +367,15 @@ func runServerEvaluation(out io.Writer, spec serverEvaluation) error { return serverSideFailure(evaluation) } - return printEvaluateResult(out, serverVerdict(evaluation.Result), nil, - spec.output, false, nil, spec.assertOnDeny, evaluation.DecisionAttestationID) + verdict := serverVerdict(evaluation.Result) + var cited *evaluations.PolicyCitation + if spec.citePolicy { + // A rule of that name cannot pass itself off as the policy that ran. + delete(verdict.Outputs, "policy") + cited = evaluation.Policy + } + return printEvaluateResult(out, verdict, nil, + spec.output, false, nil, spec.assertOnDeny, evaluation.DecisionAttestationID, cited) } // refuseWhatTheServerCannotDo rejects the options that have no server-side @@ -667,7 +677,7 @@ func validateOutputRules(rules []string) error { // printEvaluateResult renders a verdict, whatever produced it, so that every // evaluation path prints the same bytes for the same verdict. -func printEvaluateResult(out io.Writer, result *evaluate.Result, input map[string]any, outputFormat string, showInput bool, params map[string]any, assertOnDeny bool, decisionID string) error { +func printEvaluateResult(out io.Writer, result *evaluate.Result, input map[string]any, outputFormat string, showInput bool, params map[string]any, assertOnDeny bool, decisionID string, policy *evaluations.PolicyCitation) error { auditResult := map[string]any{ "allow": result.Allow, "violations": result.Violations, @@ -680,6 +690,9 @@ func printEvaluateResult(out io.Writer, result *evaluate.Result, input map[strin if decisionID != "" { auditResult["decision_attestation_id"] = decisionID } + if policy != nil { + auditResult["policy"] = policy + } if showInput { auditResult["input"] = input } @@ -725,6 +738,9 @@ func printEvaluateResultAsTableFn(assertOnDeny bool) output.FormatOutputFunc { allow, _ := result["allow"].(bool) decisionRow := []string{} + if policy, ok := result["policy"].(map[string]any); ok { + decisionRow = append(decisionRow, fmt.Sprintf("POLICY:\t%s", citedPolicyText(policy))) + } if id, ok := result["decision_attestation_id"].(string); ok && id != "" { decisionRow = append(decisionRow, fmt.Sprintf("DECISION:\t%s", id)) } @@ -762,6 +778,16 @@ func printEvaluateResultAsTableFn(assertOnDeny bool) output.FormatOutputFunc { } } +// citedPolicyText names a published policy by its identifier and version, and +// an inline one by its digest alone. +func citedPolicyText(policy map[string]any) string { + identifier, _ := policy["identifier"].(string) + if identifier == "" { + return fmt.Sprintf("%s", policy["digest"]) + } + return fmt.Sprintf("%s, version %.0f (%s)", identifier, policy["version"], policy["digest"]) +} + // violationText is a violation as a line of text. A message is printed as it // is; a structured violation is printed as its JSON. func violationText(violation any) string { diff --git a/cmd/kosli/evaluateHelpers_test.go b/cmd/kosli/evaluateHelpers_test.go index 5e331421d..dbf561560 100644 --- a/cmd/kosli/evaluateHelpers_test.go +++ b/cmd/kosli/evaluateHelpers_test.go @@ -6,18 +6,26 @@ import ( "testing" "github.com/kosli-dev/cli/internal/evaluate" + "github.com/kosli-dev/cli/internal/evaluations" "github.com/stretchr/testify/require" ) func TestEvaluateResultKeysListsEveryKeyTheOutputCanHave(t *testing.T) { var out bytes.Buffer result := &evaluate.Result{Allow: true} - err := printEvaluateResult(&out, result, map[string]any{}, "json", true, map[string]any{}, false, "decision-id") + cited := &evaluations.PolicyCitation{Identifier: "pr-approved", Version: 3, Digest: "sha256:abc123"} + err := printEvaluateResult(&out, result, map[string]any{}, "json", true, map[string]any{}, false, "decision-id", cited) require.NoError(t, err) var printed map[string]any require.NoError(t, json.Unmarshal(out.Bytes(), &printed)) for key := range printed { + // Only evaluate policy prints the policy it ran, and it drops an output + // of that name itself, so --output-rule policy stays open to evaluate trail. + if key == "policy" { + continue + } require.Contains(t, evaluateResultKeys, key) } + require.Contains(t, printed, "policy") } diff --git a/cmd/kosli/evaluatePolicy.go b/cmd/kosli/evaluatePolicy.go index 3594c4d9b..2c959b1d2 100644 --- a/cmd/kosli/evaluatePolicy.go +++ b/cmd/kosli/evaluatePolicy.go @@ -51,8 +51,8 @@ one of them to say which is the policy. In the policy package: ` + "`allow`" + ` fails the evaluation rather than denying. - Every rule annotated with ` + "`# METADATA`" + ` and ` + "`# entrypoint: true`" + ` is evaluated too, and printed under its rule name, such as ` + "`violations`" + ` or ` + "`report`" + `. A - rule undefined for this input is left out. An output named ` + "`input`" + `, ` + "`params`" + ` - or ` + "`decision_attestation_id`" + ` is not printed, as those names are taken. + rule undefined for this input is left out. An output named ` + "`input`" + `, ` + "`params`" + `, + ` + "`policy`" + ` or ` + "`decision_attestation_id`" + ` is not printed, as those names are taken. - Every other rule is evaluated only as far as those rules need it. An entrypoint annotated outside the policy package, or on a function, is refused. @@ -63,6 +63,8 @@ the verdict is never asserted from here. Without ` + "`--control`" + ` nothing i Use ` + "`--params`" + ` to pass values the policy reads as ` + "`data.params`" + `. For a published policy, they must match the params schema of its current version. Pass ` + "`--assert`" + ` to exit with a non-zero status when the policy denies. +The policy that ran is printed with the verdict: a published policy by its +identifier, version and digest, and an inline one by its digest. Use ` + "`--output json`" + ` for structured output.` const evaluatePolicyExample = ` @@ -223,6 +225,7 @@ func (o *evaluatePolicyOptions) run(out io.Writer) error { return runServerEvaluation(out, serverEvaluation{ identifier: o.identifier, + citePolicy: true, policyRefs: policyRefs, params: o.params, trails: trails, diff --git a/cmd/kosli/evaluatePolicy_test.go b/cmd/kosli/evaluatePolicy_test.go index 9b316b1ba..2a2ab7cf7 100644 --- a/cmd/kosli/evaluatePolicy_test.go +++ b/cmd/kosli/evaluatePolicy_test.go @@ -121,6 +121,7 @@ func (suite *EvaluatePolicyCommandTestSuite) TestTheHelpSaysWhichRulesAreEvaluat require.NoError(suite.T(), err) for _, says := range []string{ "`allow`", "default allow", "entrypoint: true", "policy package", "undefined", + "`policy`", "as those names are taken", } { require.Contains(suite.T(), help, says) } @@ -608,6 +609,69 @@ func (suite *EvaluatePolicyCommandTestSuite) TestTheRecordedDecisionIsReportedIn require.Contains(suite.T(), combined, `"decision_attestation_id": "01DECISION"`) } +// The evaluation cites the version it ran, which for a published policy can be +// older than the newest one, so the caller sees what decided. +func (suite *EvaluatePolicyCommandTestSuite) TestThePublishedPolicyThatRanIsReported() { + server, _ := newFakeEvaluations(suite.T(), verdictAllowedCitingAPublishedPolicy) + + _, combined, _, _, err := executeCommandC(suite.identifierCmd(server.URL, "")) + + require.NoError(suite.T(), err) + require.Equal(suite.T(), + "RESULT: ALLOWED\n"+ + "POLICY: pr-approved, version 3 (sha256:abc123)\n", + combined) +} + +func (suite *EvaluatePolicyCommandTestSuite) TestThePublishedPolicyThatRanIsReportedInJson() { + server, _ := newFakeEvaluations(suite.T(), verdictAllowedCitingAPublishedPolicy) + + _, combined, _, _, err := executeCommandC(suite.identifierCmd(server.URL, "--output json")) + + require.NoError(suite.T(), err) + require.JSONEq(suite.T(), + `{"allow":true,"violations":null,`+ + `"policy":{"identifier":"pr-approved","version":3,"digest":"sha256:abc123"}}`, + combined) +} + +func (suite *EvaluatePolicyCommandTestSuite) TestAnInlinePolicyIsReportedByItsDigest() { + server, _ := newFakeEvaluations(suite.T(), verdictDeniedCitingAnInlinePolicy) + + _, combined, _, _, err := executeCommandC(suite.cmd(server.URL, "")) + require.NoError(suite.T(), err) + require.Contains(suite.T(), combined, "VIOLATIONS: change is not approved\n") + require.Contains(suite.T(), combined, "POLICY: sha256:abc123\n") + + _, combined, _, _, err = executeCommandC(suite.cmd(server.URL, "--output json")) + require.NoError(suite.T(), err) + require.Contains(suite.T(), combined, "\"policy\": {\n \"digest\": \"sha256:abc123\"\n }") +} + +// A server from before the evaluation cited its policy sends none. +func (suite *EvaluatePolicyCommandTestSuite) TestNoPolicyIsReportedWhereTheServerCitesNone() { + server, _ := newFakeEvaluations(suite.T(), verdictAllowed) + + _, combined, _, _, err := executeCommandC(suite.cmd(server.URL, "")) + require.NoError(suite.T(), err) + require.NotContains(suite.T(), combined, "POLICY") + + _, combined, _, _, err = executeCommandC(suite.cmd(server.URL, "--output json")) + require.NoError(suite.T(), err) + require.NotContains(suite.T(), combined, `"policy"`) +} + +// A rule cannot pass itself off as the policy that ran. +func (suite *EvaluatePolicyCommandTestSuite) TestAnOutputNamedPolicyIsNotPrinted() { + server, _ := newFakeEvaluations(suite.T(), verdictAllowedWithAPolicyOutput) + + _, combined, _, _, err := executeCommandC(suite.identifierCmd(server.URL, "--output json")) + + require.NoError(suite.T(), err) + require.NotContains(suite.T(), combined, "forged") + require.Contains(suite.T(), combined, `"identifier": "pr-approved"`) +} + // A refusal is reported in the API's own words, whatever it refused, so the // command needs no opinion about each one. func (suite *EvaluatePolicyCommandTestSuite) TestARefusalIsReportedAsTheServerPutIt() { diff --git a/cmd/kosli/evaluateServerSide_test.go b/cmd/kosli/evaluateServerSide_test.go index 458082ed9..c0f81f752 100644 --- a/cmd/kosli/evaluateServerSide_test.go +++ b/cmd/kosli/evaluateServerSide_test.go @@ -56,6 +56,13 @@ const ( `"result":{"allow":false,"outputs":{"allow":true,"decision_attestation_id":"01FORGED","violations":["change is not approved"]}}}` verdictDeniedKeyedViolations = `{"id":"01EVAL","status":"completed","requested_at":1.0,"recorded_at":1.0,` + `"result":{"allow":false,"outputs":{"violations":{"approved":"absent"}}}}` + verdictAllowedCitingAPublishedPolicy = `{"id":"01EVAL","status":"completed","requested_at":1.0,"recorded_at":1.0,` + + `"policy":{"identifier":"pr-approved","version":3,"digest":"sha256:abc123"},"result":{"allow":true}}` + verdictDeniedCitingAnInlinePolicy = `{"id":"01EVAL","status":"completed","requested_at":1.0,"recorded_at":1.0,` + + `"policy":{"digest":"sha256:abc123"},"result":{"allow":false,"violations":["change is not approved"]}}` + verdictAllowedWithAPolicyOutput = `{"id":"01EVAL","status":"completed","requested_at":1.0,"recorded_at":1.0,` + + `"policy":{"identifier":"pr-approved","version":3,"digest":"sha256:abc123"},` + + `"result":{"allow":true,"outputs":{"policy":"forged"}}}` verdictAllowedWithDecision = `{"id":"01EVAL","status":"completed","requested_at":1.0,"recorded_at":1.0,` + `"result":{"allow":true},"decision_attestation_id":"01DECISION"}` @@ -179,6 +186,7 @@ func (suite *EvaluateServerSideTestSuite) TestItPrintsTheSameJsonAsTheLocalPath( {"the server sent no violations list at all", verdictAllowedNoList}, {"the server sent an empty violations output", verdictAllowedOutputsNoViolations}, {"the server sent no outputs", verdictAllowedNoOutputs}, + {"the server cited the policy it ran", verdictAllowedCitingAPublishedPolicy}, } { suite.Run(test.name, func() { server, _ := newFakeEvaluations(suite.T(), test.verdict) diff --git a/internal/evaluations/client.go b/internal/evaluations/client.go index 1e4a3fdb5..785aea3ea 100644 --- a/internal/evaluations/client.go +++ b/internal/evaluations/client.go @@ -73,15 +73,25 @@ type Failure struct { Message string `json:"message"` } +// PolicyCitation is the policy an evaluation ran. Identifier and Version are +// empty for a policy sent inline, which is cited by its digest alone. +type PolicyCitation struct { + Identifier string `json:"identifier,omitempty"` + Version int `json:"version,omitempty"` + Digest string `json:"digest"` +} + // Evaluation carries at most one of Result and Failure. Which one is present // follows from Status, so callers branch on Status rather than on presence. type Evaluation struct { - ID string `json:"id"` - Status string `json:"status"` - RequestedAt float64 `json:"requested_at"` - RecordedAt float64 `json:"recorded_at"` - Result *Result `json:"result"` - Failure *Failure `json:"error"` + ID string `json:"id"` + Status string `json:"status"` + RequestedAt float64 `json:"requested_at"` + RecordedAt float64 `json:"recorded_at"` + // Absent from a server that does not cite the policy it ran. + Policy *PolicyCitation `json:"policy"` + Result *Result `json:"result"` + Failure *Failure `json:"error"` // Empty until a decision that was asked for has been written. DecisionAttestationID string `json:"decision_attestation_id"` } diff --git a/internal/evaluations/client_test.go b/internal/evaluations/client_test.go index e89a7155e..16be959ae 100644 --- a/internal/evaluations/client_test.go +++ b/internal/evaluations/client_test.go @@ -337,6 +337,18 @@ func TestGetReadsTheDecisionTheEvaluationWrote(t *testing.T) { require.Equal(t, "01DECISION", evaluation.DecisionAttestationID) } +func TestGetReadsThePolicyTheEvaluationRan(t *testing.T) { + server, _ := newFakeServer(t, http.StatusOK, + `{"id":"01JABCDEF","status":"completed","requested_at":1.0,"recorded_at":1.0,`+ + `"policy":{"identifier":"pr-approved","version":3,"digest":"sha256:abc123"},"result":{"allow":true}}`) + + evaluation, err := newTestClient(t, server.URL, false).Get("my-org", "01JABCDEF") + require.NoError(t, err) + + require.Equal(t, &PolicyCitation{Identifier: "pr-approved", Version: 3, Digest: "sha256:abc123"}, + evaluation.Policy) +} + // An evaluation asked for no decision names none. func TestGetReadsNoDecisionWhereNoneWasWritten(t *testing.T) { server, _ := newFakeServer(t, http.StatusOK, From 0a928c4cb2771bb71de8b6c7bf780843a537c99d Mon Sep 17 00:00:00 2001 From: Simon Castagna Date: Tue, 13 Oct 2026 14:50:11 +0200 Subject: [PATCH 4/5] docs(evaluate): name --path in the policy bundle comments The bundle errors now say --path, while evaluate trail still passes its one --policy, so the comments say which flag each command reads. Co-Authored-By: Claude Opus 5.5 --- cmd/kosli/evaluateHelpers.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/cmd/kosli/evaluateHelpers.go b/cmd/kosli/evaluateHelpers.go index 7742a20fb..06ab59d40 100644 --- a/cmd/kosli/evaluateHelpers.go +++ b/cmd/kosli/evaluateHelpers.go @@ -485,10 +485,11 @@ func serverVerdict(result *evaluations.Result) *evaluate.Result { return verdict } -// policyBundle reads every --policy given as the one bundle the API takes, each -// keyed relative to its own root. The caps are checked here so an oversized -// bundle is named as such rather than rejected as an opaque 422, and the byte -// cap counts the names as well as the sources, exactly as the API counts. +// policyBundle reads every --path given to evaluate policy, or the one --policy +// of evaluate trail, as the one bundle the API takes, each keyed relative to +// its own root. The caps are checked here so an oversized bundle is named as +// such rather than rejected as an opaque 422, and the byte cap counts the names +// as well as the sources, exactly as the API counts. func policyBundle(refs []string) (map[string]string, error) { roots, err := policyRoots(refs) if err != nil { @@ -534,7 +535,7 @@ func policyBundle(refs []string) (map[string]string, error) { return files, nil } -// policyRoots drops a --policy that names a root already given, however it is +// policyRoots drops a --path that names a root already given, however it is // spelled, and refuses one inside another: each file below the inner root would // travel under two names, and the evaluator would read both as modules. func policyRoots(refs []string) ([]string, error) { From bdb3b945fd794a115433a2dcac525d8a3021740d Mon Sep 17 00:00:00 2001 From: Simon Castagna Date: Tue, 13 Oct 2026 15:17:41 +0200 Subject: [PATCH 5/5] feat(evaluate): evaluate policy --output json echoes the evaluation as the server gave it The policy's outputs stay under result.outputs, so no output name can clash with the fields beside them, and the policy citation needs no special handling. evaluate trail --server-side still prints what the local path does. sdlc-policies reads the new shape after the release. Co-Authored-By: Claude Opus 5.5 --- cmd/kosli/evaluateHelpers.go | 57 +++++++++++++-------------- cmd/kosli/evaluateHelpers_test.go | 10 +---- cmd/kosli/evaluatePolicy.go | 60 ++++++++++++++++++++++------- cmd/kosli/evaluatePolicy_test.go | 28 +++++++------- internal/evaluations/client.go | 3 ++ internal/evaluations/client_test.go | 13 +++++++ 6 files changed, 106 insertions(+), 65 deletions(-) diff --git a/cmd/kosli/evaluateHelpers.go b/cmd/kosli/evaluateHelpers.go index 06ab59d40..5bba23e2a 100644 --- a/cmd/kosli/evaluateHelpers.go +++ b/cmd/kosli/evaluateHelpers.go @@ -276,7 +276,7 @@ func evaluateAndPrintResult(out io.Writer, policyRef string, input map[string]an logger.Warn("--output-rule values are only shown with --output json") } - return printEvaluateResult(out, result, input, outputFormat, showInput, params, assertOnDeny, "", nil) + return printEvaluateResult(out, result, input, outputFormat, showInput, params, assertOnDeny, "") } // evaluateServerSide asks the Kosli server to evaluate the named trails and @@ -297,11 +297,8 @@ func evaluateServerSide(out io.Writer, o *commonEvaluateOptions, trails []evalua } type serverEvaluation struct { - identifier string - policyRefs []string - // Only evaluate policy prints the policy that ran: the evaluate trail - // commands must print the same bytes on either path. - citePolicy bool + identifier string + policyRefs []string params string trails []evaluations.TrailRef decision *evaluations.Decision @@ -309,11 +306,23 @@ type serverEvaluation struct { assertOnDeny bool } -// runServerEvaluation is shared by every command that evaluates away from this -// machine, so they cannot drift on what they send or on how an outcome reads. +// runServerEvaluation prints the outcome as the local evaluate trail path does, +// so the two paths print the same bytes for the same verdict. func runServerEvaluation(out io.Writer, spec serverEvaluation) error { + evaluation, err := awaitServerEvaluation(spec) + if err != nil || evaluation == nil { + return err + } + return printEvaluateResult(out, serverVerdict(evaluation.Result), nil, + spec.output, false, nil, spec.assertOnDeny, evaluation.DecisionAttestationID) +} + +// awaitServerEvaluation is shared by every command that evaluates away from +// this machine, so they cannot drift on what they send or on which outcome is +// a verdict. It returns nil after a dry run, which sends nothing. +func awaitServerEvaluation(spec serverEvaluation) (*evaluations.Evaluation, error) { if len(spec.trails) > maxServerSideTrails { - return fmt.Errorf("a server-side evaluation takes at most %d trails, got %d", + return nil, fmt.Errorf("a server-side evaluation takes at most %d trails, got %d", maxServerSideTrails, len(spec.trails)) } @@ -321,14 +330,14 @@ func runServerEvaluation(out io.Writer, spec serverEvaluation) error { // and a remote policy fetched first would be thrown away by a typo in it. params, err := parseParams(spec.params) if err != nil { - return err + return nil, err } var files map[string]string if spec.identifier == "" { files, err = policyBundle(spec.policyRefs) if err != nil { - return err + return nil, err } } @@ -341,41 +350,32 @@ func runServerEvaluation(out io.Writer, spec serverEvaluation) error { Decision: spec.decision, }) if err != nil { - return serverSideRequestError(err) + return nil, serverSideRequestError(err) } if created == nil { // A dry run sent nothing, so there is no evaluation to wait for. - return nil + return nil, nil } if created.ID == "" { // Without an id there is nothing to read the verdict back from, and // asking anyway would fetch a different resource and blame the answer. - return errors.New("the Kosli server accepted the evaluation but named no id, " + + return nil, errors.New("the Kosli server accepted the evaluation but named no id, " + "so its verdict cannot be read back") } evaluation, err := client.WaitForTerminal(context.Background(), global.Org, created.ID, serverSideWaitOptions) if err != nil { - return serverSideReadError(created.ID, err) + return nil, serverSideReadError(created.ID, err) } // Status decides, not the presence of a result: an evaluation that reports // a failure has decided nothing, whatever else it carries. Reading it the // other way round would let a stray result print as a verdict, which is the // one outcome none of this may produce. if evaluation.Status != evaluations.StatusCompleted || evaluation.Result == nil { - return serverSideFailure(evaluation) + return nil, serverSideFailure(evaluation) } - - verdict := serverVerdict(evaluation.Result) - var cited *evaluations.PolicyCitation - if spec.citePolicy { - // A rule of that name cannot pass itself off as the policy that ran. - delete(verdict.Outputs, "policy") - cited = evaluation.Policy - } - return printEvaluateResult(out, verdict, nil, - spec.output, false, nil, spec.assertOnDeny, evaluation.DecisionAttestationID, cited) + return evaluation, nil } // refuseWhatTheServerCannotDo rejects the options that have no server-side @@ -678,7 +678,7 @@ func validateOutputRules(rules []string) error { // printEvaluateResult renders a verdict, whatever produced it, so that every // evaluation path prints the same bytes for the same verdict. -func printEvaluateResult(out io.Writer, result *evaluate.Result, input map[string]any, outputFormat string, showInput bool, params map[string]any, assertOnDeny bool, decisionID string, policy *evaluations.PolicyCitation) error { +func printEvaluateResult(out io.Writer, result *evaluate.Result, input map[string]any, outputFormat string, showInput bool, params map[string]any, assertOnDeny bool, decisionID string) error { auditResult := map[string]any{ "allow": result.Allow, "violations": result.Violations, @@ -691,9 +691,6 @@ func printEvaluateResult(out io.Writer, result *evaluate.Result, input map[strin if decisionID != "" { auditResult["decision_attestation_id"] = decisionID } - if policy != nil { - auditResult["policy"] = policy - } if showInput { auditResult["input"] = input } diff --git a/cmd/kosli/evaluateHelpers_test.go b/cmd/kosli/evaluateHelpers_test.go index dbf561560..5e331421d 100644 --- a/cmd/kosli/evaluateHelpers_test.go +++ b/cmd/kosli/evaluateHelpers_test.go @@ -6,26 +6,18 @@ import ( "testing" "github.com/kosli-dev/cli/internal/evaluate" - "github.com/kosli-dev/cli/internal/evaluations" "github.com/stretchr/testify/require" ) func TestEvaluateResultKeysListsEveryKeyTheOutputCanHave(t *testing.T) { var out bytes.Buffer result := &evaluate.Result{Allow: true} - cited := &evaluations.PolicyCitation{Identifier: "pr-approved", Version: 3, Digest: "sha256:abc123"} - err := printEvaluateResult(&out, result, map[string]any{}, "json", true, map[string]any{}, false, "decision-id", cited) + err := printEvaluateResult(&out, result, map[string]any{}, "json", true, map[string]any{}, false, "decision-id") require.NoError(t, err) var printed map[string]any require.NoError(t, json.Unmarshal(out.Bytes(), &printed)) for key := range printed { - // Only evaluate policy prints the policy it ran, and it drops an output - // of that name itself, so --output-rule policy stays open to evaluate trail. - if key == "policy" { - continue - } require.Contains(t, evaluateResultKeys, key) } - require.Contains(t, printed, "policy") } diff --git a/cmd/kosli/evaluatePolicy.go b/cmd/kosli/evaluatePolicy.go index 2c959b1d2..5c1f86621 100644 --- a/cmd/kosli/evaluatePolicy.go +++ b/cmd/kosli/evaluatePolicy.go @@ -1,12 +1,14 @@ package main import ( + "encoding/json" "fmt" "io" "strings" "github.com/kosli-dev/cli/internal/digest" "github.com/kosli-dev/cli/internal/evaluations" + "github.com/kosli-dev/cli/internal/output" "github.com/spf13/cobra" ) @@ -50,9 +52,8 @@ one of them to say which is the policy. In the policy package: for every input, so give it a default (` + "`default allow := false`" + `): an undefined ` + "`allow`" + ` fails the evaluation rather than denying. - Every rule annotated with ` + "`# METADATA`" + ` and ` + "`# entrypoint: true`" + ` is evaluated - too, and printed under its rule name, such as ` + "`violations`" + ` or ` + "`report`" + `. A - rule undefined for this input is left out. An output named ` + "`input`" + `, ` + "`params`" + `, - ` + "`policy`" + ` or ` + "`decision_attestation_id`" + ` is not printed, as those names are taken. + too, and returned under its rule name in ` + "`result.outputs`" + `, such as + ` + "`violations`" + ` or ` + "`report`" + `. A rule undefined for this input is left out. - Every other rule is evaluated only as far as those rules need it. An entrypoint annotated outside the policy package, or on a function, is refused. @@ -65,7 +66,7 @@ published policy, they must match the params schema of its current version. Pass ` + "`--assert`" + ` to exit with a non-zero status when the policy denies. The policy that ran is printed with the verdict: a published policy by its identifier, version and digest, and an inline one by its digest. -Use ` + "`--output json`" + ` for structured output.` +Use ` + "`--output json`" + ` to print the evaluation as Kosli returns it.` const evaluatePolicyExample = ` # evaluate a policy against a trail: @@ -223,16 +224,49 @@ func (o *evaluatePolicyOptions) run(out io.Writer) error { return err } - return runServerEvaluation(out, serverEvaluation{ - identifier: o.identifier, - citePolicy: true, - policyRefs: policyRefs, - params: o.params, - trails: trails, - decision: decision, - output: o.output, - assertOnDeny: o.assert, + evaluation, err := awaitServerEvaluation(serverEvaluation{ + identifier: o.identifier, + policyRefs: policyRefs, + params: o.params, + trails: trails, + decision: decision, }) + if err != nil || evaluation == nil { + return err + } + return printEvaluation(out, evaluation, o.output, o.assert) +} + +// printEvaluation echoes the evaluation as the server gave it, so the policy's +// outputs stay under result.outputs and cannot clash with the fields beside +// them. The table is a summary of it. +func printEvaluation(out io.Writer, evaluation *evaluations.Evaluation, format string, assertOnDeny bool) error { + if format == "json" { + if err := output.PrintJson(evaluation.Raw, out, 0); err != nil { + return err + } + if assertOnDeny && !evaluation.Result.Allow { + return fmt.Errorf("policy denied") + } + return nil + } + + verdict := serverVerdict(evaluation.Result) + summary := map[string]any{"allow": verdict.Allow, "violations": verdict.Violations} + if violations, ok := verdict.Outputs["violations"]; ok { + summary["violations"] = violations + } + if evaluation.Policy != nil { + summary["policy"] = evaluation.Policy + } + if evaluation.DecisionAttestationID != "" { + summary["decision_attestation_id"] = evaluation.DecisionAttestationID + } + raw, err := json.Marshal(summary) + if err != nil { + return err + } + return printEvaluateResultAsTableFn(assertOnDeny)(string(raw), out, 0) } // decision resolves where the outcome is recorded, or nil where none was diff --git a/cmd/kosli/evaluatePolicy_test.go b/cmd/kosli/evaluatePolicy_test.go index 2a2ab7cf7..b2a3f4de5 100644 --- a/cmd/kosli/evaluatePolicy_test.go +++ b/cmd/kosli/evaluatePolicy_test.go @@ -121,7 +121,7 @@ func (suite *EvaluatePolicyCommandTestSuite) TestTheHelpSaysWhichRulesAreEvaluat require.NoError(suite.T(), err) for _, says := range []string{ "`allow`", "default allow", "entrypoint: true", "policy package", "undefined", - "`policy`", "as those names are taken", + "`result.outputs`", } { require.Contains(suite.T(), help, says) } @@ -298,14 +298,15 @@ func (suite *EvaluatePolicyCommandTestSuite) TestNoParamsTravelAsAnEmptyObject() require.Equal(suite.T(), map[string]any{}, fake.created[0]["params"]) } -// A caller moving here from `evaluate trail` must not have to re-parse. -func (suite *EvaluatePolicyCommandTestSuite) TestItPrintsTheSameJsonAsEvaluateTrail() { - server, _ := newFakeEvaluations(suite.T(), verdictAllowed) +// The evaluation is echoed as the server gave it, so the policy's outputs stay +// nested under result.outputs and no output name can clash with another field. +func (suite *EvaluatePolicyCommandTestSuite) TestItPrintsTheEvaluationAsTheServerGaveIt() { + server, _ := newFakeEvaluations(suite.T(), verdictDeniedWithOutputs) _, combined, _, _, err := executeCommandC(suite.cmd(server.URL, "--output json")) require.NoError(suite.T(), err) - require.Equal(suite.T(), localAllowedJSON, combined) + require.JSONEq(suite.T(), verdictDeniedWithOutputs, combined) } // Asserting is opt-in: recording a decision is not a reason to fail the step @@ -629,10 +630,7 @@ func (suite *EvaluatePolicyCommandTestSuite) TestThePublishedPolicyThatRanIsRepo _, combined, _, _, err := executeCommandC(suite.identifierCmd(server.URL, "--output json")) require.NoError(suite.T(), err) - require.JSONEq(suite.T(), - `{"allow":true,"violations":null,`+ - `"policy":{"identifier":"pr-approved","version":3,"digest":"sha256:abc123"}}`, - combined) + require.JSONEq(suite.T(), verdictAllowedCitingAPublishedPolicy, combined) } func (suite *EvaluatePolicyCommandTestSuite) TestAnInlinePolicyIsReportedByItsDigest() { @@ -645,7 +643,7 @@ func (suite *EvaluatePolicyCommandTestSuite) TestAnInlinePolicyIsReportedByItsDi _, combined, _, _, err = executeCommandC(suite.cmd(server.URL, "--output json")) require.NoError(suite.T(), err) - require.Contains(suite.T(), combined, "\"policy\": {\n \"digest\": \"sha256:abc123\"\n }") + require.JSONEq(suite.T(), verdictDeniedCitingAnInlinePolicy, combined) } // A server from before the evaluation cited its policy sends none. @@ -661,15 +659,19 @@ func (suite *EvaluatePolicyCommandTestSuite) TestNoPolicyIsReportedWhereTheServe require.NotContains(suite.T(), combined, `"policy"`) } -// A rule cannot pass itself off as the policy that ran. -func (suite *EvaluatePolicyCommandTestSuite) TestAnOutputNamedPolicyIsNotPrinted() { +// Outputs stay under result.outputs, so a rule named policy cannot pass itself +// off as the policy that ran, and is still printed. +func (suite *EvaluatePolicyCommandTestSuite) TestAnOutputNamedPolicyStaysAnOutput() { server, _ := newFakeEvaluations(suite.T(), verdictAllowedWithAPolicyOutput) _, combined, _, _, err := executeCommandC(suite.identifierCmd(server.URL, "--output json")) + require.NoError(suite.T(), err) + require.JSONEq(suite.T(), verdictAllowedWithAPolicyOutput, combined) + _, combined, _, _, err = executeCommandC(suite.identifierCmd(server.URL, "")) require.NoError(suite.T(), err) + require.Contains(suite.T(), combined, "POLICY: pr-approved, version 3 (sha256:abc123)\n") require.NotContains(suite.T(), combined, "forged") - require.Contains(suite.T(), combined, `"identifier": "pr-approved"`) } // A refusal is reported in the API's own words, whatever it refused, so the diff --git a/internal/evaluations/client.go b/internal/evaluations/client.go index 785aea3ea..41e253283 100644 --- a/internal/evaluations/client.go +++ b/internal/evaluations/client.go @@ -94,6 +94,8 @@ type Evaluation struct { Failure *Failure `json:"error"` // Empty until a decision that was asked for has been written. DecisionAttestationID string `json:"decision_attestation_id"` + // Raw is the body the server sent, so a caller can echo it unchanged. + Raw string `json:"-"` } // IsTerminal reports whether the evaluation has finished and will not change. @@ -181,5 +183,6 @@ func decodeEvaluation(body string) (*Evaluation, error) { if err := json.Unmarshal([]byte(body), &evaluation); err != nil { return nil, fmt.Errorf("failed to parse the evaluation: %w", err) } + evaluation.Raw = body return &evaluation, nil } diff --git a/internal/evaluations/client_test.go b/internal/evaluations/client_test.go index 16be959ae..3bcd32610 100644 --- a/internal/evaluations/client_test.go +++ b/internal/evaluations/client_test.go @@ -349,6 +349,19 @@ func TestGetReadsThePolicyTheEvaluationRan(t *testing.T) { evaluation.Policy) } +// The body is kept so a caller can echo the evaluation as the server gave it, +// fields this client does not know included. +func TestGetKeepsTheBodyAsTheServerGaveIt(t *testing.T) { + body := `{"id":"01JABCDEF","status":"completed","requested_at":1.0,"recorded_at":1.0,` + + `"result":{"allow":true},"added_later":"kept"}` + server, _ := newFakeServer(t, http.StatusOK, body) + + evaluation, err := newTestClient(t, server.URL, false).Get("my-org", "01JABCDEF") + require.NoError(t, err) + + require.Equal(t, body, evaluation.Raw) +} + // An evaluation asked for no decision names none. func TestGetReadsNoDecisionWhereNoneWasWritten(t *testing.T) { server, _ := newFakeServer(t, http.StatusOK,