diff --git a/cmd/kosli/evaluate.go b/cmd/kosli/evaluate.go index 702d9ae4a..40cfa2140 100644 --- a/cmd/kosli/evaluate.go +++ b/cmd/kosli/evaluate.go @@ -44,6 +44,7 @@ func newEvaluateCmd(out io.Writer) *cobra.Command { // Add subcommands cmd.AddCommand( + newEvaluatePolicyCmd(out), newEvaluateTrailCmd(out), newEvaluateTrailsCmd(out), newEvaluateInputCmd(out), diff --git a/cmd/kosli/evaluateHelpers.go b/cmd/kosli/evaluateHelpers.go index 265efc1f9..975ef796e 100644 --- a/cmd/kosli/evaluateHelpers.go +++ b/cmd/kosli/evaluateHelpers.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "io" + "io/fs" "net/http" "net/url" "os" @@ -36,6 +37,10 @@ const policyMaxBytes = 5 << 20 // 5 MiB // whenever that schema changes. const maxServerSideTrails = 100 +// maxPolicyBundleFiles mirrors the ceiling the evaluations API publishes on +// the entries in a policy bundle. +const maxPolicyBundleFiles = 100 + // serverPolicyMaxBytes mirrors the cap the evaluations API publishes on a // policy bundle, which counts the names as well as the sources. It is a fifth // of what a remote --policy read allows, so a policy can be fetched in full @@ -66,7 +71,7 @@ func (o *commonEvaluateOptions) addFlags(cmd *cobra.Command, policyDesc string) cmd.Flags().StringVarP(&o.output, "output", "o", "table", outputFlag) cmd.Flags().BoolVar(&o.showInput, "show-input", false, "[optional] Include the policy input data in the output.") cmd.Flags().StringSliceVar(&o.attestations, "attestations", nil, "[optional] Limit which attestations are included. Plain name for trail-level, dot-qualified (artifact.name) for artifact-level.") - cmd.Flags().StringVar(&o.params, "params", "", "[optional] Policy parameters as inline JSON or @file.json. Available in policies as data.params.") + cmd.Flags().StringVar(&o.params, "params", "", policyParamsFlag) cmd.Flags().BoolVar(&o.assert, "assert", false, "[optional] Exit with a non-zero status when the policy denies. This is the current default; pass --assert to lock it in across future releases.") cmd.Flags().BoolVar(&o.noAssert, "no-assert", false, "[optional] Print the result and always exit 0, even when the policy denies. Use when this command feeds another tool as a policy decision point.") cmd.MarkFlagsMutuallyExclusive("assert", "no-assert") @@ -260,7 +265,7 @@ func evaluateAndPrintResult(out io.Writer, policyRef string, input map[string]in return err } - return printEvaluateResult(out, result, input, outputFormat, showInput, params, assertOnDeny) + return printEvaluateResult(out, result, input, outputFormat, showInput, params, assertOnDeny, "") } // evaluateServerSide asks the Kosli server to evaluate the named trails and @@ -271,33 +276,50 @@ func evaluateServerSide(out io.Writer, o *commonEvaluateOptions, trails []evalua return err } - if len(trails) > maxServerSideTrails { + return runServerEvaluation(out, serverEvaluation{ + policyRef: o.policyRef, + params: o.params, + trails: trails, + output: o.output, + assertOnDeny: o.assertOnDeny(), + }) +} + +type serverEvaluation struct { + policyRef string + params string + trails []evaluations.TrailRef + decision *evaluations.Decision + output string + 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. +func runServerEvaluation(out io.Writer, spec serverEvaluation) error { + if len(spec.trails) > maxServerSideTrails { return fmt.Errorf("a server-side evaluation takes at most %d trails, got %d", - maxServerSideTrails, len(trails)) + maxServerSideTrails, len(spec.trails)) } // Parsed before the policy is read: --params is local and cheap to check, // and a remote policy fetched first would be thrown away by a typo in it. - params, err := parseParams(o.params) - if err != nil { - return err - } - - policySource, err := loadPolicy(o.policyRef) + params, err := parseParams(spec.params) if err != nil { return err } - files, err := policyBundle(o.policyRef, policySource) + files, err := policyBundle(spec.policyRef) if err != nil { return err } client := evaluations.NewClient(kosliClient, global.Host, global.ApiToken, global.DryRun) created, err := client.Create(global.Org, evaluations.CreateRequest{ - Trails: trails, - Files: files, - Params: params, + Trails: spec.trails, + Files: files, + Params: params, + Decision: spec.decision, }) if err != nil { return serverSideRequestError(err) @@ -327,7 +349,7 @@ func evaluateServerSide(out io.Writer, o *commonEvaluateOptions, trails []evalua } return printEvaluateResult(out, serverVerdict(evaluation.Result), nil, - o.output, false, nil, o.assertOnDeny()) + spec.output, false, nil, spec.assertOnDeny, evaluation.DecisionAttestationID) } // refuseWhatTheServerCannotDo rejects the options that have no server-side @@ -415,16 +437,79 @@ func serverVerdict(result *evaluations.Result) *evaluate.Result { return &evaluate.Result{Allow: result.Allow, Violations: violations} } -// policyBundle wraps the policy source as the one-file bundle the API takes, -// refusing one too large for it rather than letting the request be rejected. -// The cap counts the names as well as the sources, exactly as the API counts. -func policyBundle(ref string, source []byte) (map[string]string, error) { - key := policyBundleKey(ref) - if size := len(key) + len(source); size > serverPolicyMaxBytes { +// policyBundle reads what --policy names as the bundle the API takes. 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(ref string) (map[string]string, error) { + files, err := policyBundleFiles(ref) + if err != nil { + return nil, err + } + + if len(files) > maxPolicyBundleFiles { + return nil, fmt.Errorf("policy bundle holds %d files, over the limit of %d", + len(files), maxPolicyBundleFiles) + } + size := 0 + for name, source := range files { + size += len(name) + len(source) + } + if size > serverPolicyMaxBytes { return nil, fmt.Errorf("policy bundle is %d bytes, over the %d byte limit", size, serverPolicyMaxBytes) } - return map[string]string{key: string(source)}, nil + return files, nil +} + +func policyBundleFiles(ref string) (map[string]string, error) { + if !isRemotePolicyRef(ref) { + if info, err := os.Stat(ref); err == nil && info.IsDir() { + return policyDirectory(ref) + } + } + + source, err := loadPolicy(ref) + if err != nil { + return nil, err + } + return map[string]string{policyBundleKey(ref): string(source)}, nil +} + +// policyDirectory collects every file below root, keyed by its path relative +// to it. Nothing is left out by name: a rule here would refuse bundles the +// evaluator that runs them would have accepted. +func policyDirectory(root string) (map[string]string, error) { + files := map[string]string{} + err := filepath.WalkDir(root, func(path string, entry fs.DirEntry, err error) error { + if err != nil { + return err + } + if entry.IsDir() { + return nil + } + source, err := os.ReadFile(path) + if err != nil { + return fmt.Errorf("failed to read policy file: %w", err) + } + name, err := filepath.Rel(root, path) + if err != nil { + return err + } + // Relative paths reach the API spelled one way, whatever this machine + // spells them with. + files[filepath.ToSlash(name)] = string(source) + return nil + }) + if err != nil { + return nil, err + } + if len(files) == 0 { + // The API takes at least one file, so an empty directory is named here + // rather than sent to be refused. + return nil, fmt.Errorf("no file found under %s", root) + } + return files, nil } // policyBundleKey names the policy inside the uploaded bundle. Only the base @@ -448,11 +533,16 @@ func policyBundleKey(ref string) string { // 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]interface{}, outputFormat string, showInput bool, params map[string]interface{}, assertOnDeny bool) error { +func printEvaluateResult(out io.Writer, result *evaluate.Result, input map[string]interface{}, outputFormat string, showInput bool, params map[string]interface{}, assertOnDeny bool, decisionID string) error { auditResult := map[string]interface{}{ "allow": result.Allow, "violations": result.Violations, } + // Absent everywhere else, so a caller reading a verdict alone parses the + // same page as before. + if decisionID != "" { + auditResult["decision_attestation_id"] = decisionID + } if showInput { auditResult["input"] = input } @@ -497,11 +587,15 @@ func printEvaluateResultAsTableFn(assertOnDeny bool) output.FormatOutputFunc { } allow, _ := result["allow"].(bool) + decisionRow := []string{} + if id, ok := result["decision_attestation_id"].(string); ok && id != "" { + decisionRow = append(decisionRow, fmt.Sprintf("DECISION:\t%s", id)) + } var rows []string if allow { rows = append(rows, "RESULT:\tALLOWED") - tabFormattedPrint(out, []string{}, rows) + tabFormattedPrint(out, []string{}, append(rows, decisionRow...)) return nil } @@ -515,13 +609,13 @@ func printEvaluateResultAsTableFn(assertOnDeny bool) output.FormatOutputFunc { rows = append(rows, fmt.Sprintf("\t%s", v)) } } - tabFormattedPrint(out, []string{}, rows) + tabFormattedPrint(out, []string{}, append(rows, decisionRow...)) if assertOnDeny { return fmt.Errorf("policy denied: %v", violations) } return nil } - tabFormattedPrint(out, []string{}, rows) + tabFormattedPrint(out, []string{}, append(rows, decisionRow...)) if assertOnDeny { return fmt.Errorf("policy denied") } diff --git a/cmd/kosli/evaluatePolicy.go b/cmd/kosli/evaluatePolicy.go new file mode 100644 index 000000000..717b3e8e2 --- /dev/null +++ b/cmd/kosli/evaluatePolicy.go @@ -0,0 +1,249 @@ +package main + +import ( + "fmt" + "io" + "strings" + + "github.com/kosli-dev/cli/internal/digest" + "github.com/kosli-dev/cli/internal/evaluations" + "github.com/spf13/cobra" +) + +const evaluatePolicyShortDesc = `Evaluate a policy against a trail in Kosli.` + +const evaluatePolicyLongDesc = evaluatePolicyShortDesc + ` +The policy is evaluated where the trail is stored, against the trail as Kosli +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 single Rego file or a directory. A directory travels as one +bundle of every file below it, keyed by its path relative to that directory. + +Pass ` + "`--control`" + ` to record the outcome as a decision against that control, in the +` + "`--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`" + `. +Pass ` + "`--assert`" + ` to exit with a non-zero status when the policy denies. +Use ` + "`--output json`" + ` for structured output.` + +const evaluatePolicyExample = ` +# evaluate a policy against a trail: +kosli evaluate policy \ + --context trail=yourFlowName/yourTrailName \ + --policy yourPolicyFile.rego \ + --api-token yourAPIToken \ + --org yourOrgName + +# evaluate several trails at one instant: +kosli evaluate policy \ + --context trail=yourFlowName/yourTrailName \ + --context trail=anotherFlowName/anotherTrailName \ + --policy 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 \ + --params '{"protected_branch": "master"}' \ + --api-token yourAPIToken \ + --org yourOrgName + +# evaluate a policy and record the outcome as a decision: +kosli evaluate policy \ + --context trail=yourFlowName/yourTrailName \ + --policy yourPolicyFile.rego \ + --control yourControlIdentifier \ + --flow yourFlowName \ + --trail yourTrailName \ + --fingerprint yourArtifactFingerprint \ + --api-token yourAPIToken \ + --org yourOrgName + +# evaluate a policy and fail the step when it denies: +kosli evaluate policy \ + --context trail=yourFlowName/yourTrailName \ + --policy yourPolicyFile.rego \ + --assert \ + --api-token yourAPIToken \ + --org yourOrgName` + +type evaluatePolicyOptions struct { + contexts []string + policyRef string + params string + output string + assert bool + control string + flowName string + trailName string + name string + fingerprint string +} + +func newEvaluatePolicyCmd(out io.Writer) *cobra.Command { + o := new(evaluatePolicyOptions) + cmd := &cobra.Command{ + Use: "policy", + Short: evaluatePolicyShortDesc, + Long: evaluatePolicyLongDesc, + Example: evaluatePolicyExample, + Args: cobra.NoArgs, + PreRunE: func(cmd *cobra.Command, args []string) error { + err := RequireGlobalFlags(global, []string{"Org", "ApiToken"}) + if err != nil { + return ErrorBeforePrintingUsage(cmd, err.Error()) + } + return nil + }, + RunE: func(cmd *cobra.Command, args []string) error { + return o.run(out) + }, + // Hidden while the command is proved out against a server that can run + // it. Unhiding it is this line, and it also restores its docs page. + Hidden: true, + } + + cmd.Flags().StringArrayVar(&o.contexts, "context", []string{}, policyContextFlag) + cmd.Flags().StringVarP(&o.policyRef, "policy", "p", "", "Path of a Rego policy file, or of a directory sent as one bundle.") + cmd.Flags().StringVar(&o.params, "params", "", policyParamsFlag) + cmd.Flags().StringVarP(&o.output, "output", "o", "table", outputFlag) + cmd.Flags().BoolVar(&o.assert, "assert", false, policyAssertFlag) + cmd.Flags().StringVar(&o.control, "control", "", policyControlFlag) + cmd.Flags().StringVarP(&o.flowName, "flow", "f", "", policyDecisionFlowFlag) + cmd.Flags().StringVar(&o.trailName, "trail", "", policyDecisionTrailFlag) + cmd.Flags().StringVar(&o.name, "name", "", policyDecisionNameFlag) + cmd.Flags().StringVar(&o.fingerprint, "fingerprint", "", policyDecisionFingerprintFlag) + + err := RequireFlags(cmd, []string{"context", "policy"}) + if err != nil { + logger.Error("failed to configure required flags: %v", err) + } + + return cmd +} + +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. + if isRemotePolicyRef(o.policyRef) { + return fmt.Errorf("--policy takes a file or a directory on this machine, not a URL") + } + + // Refused before the request: a format refused where it is printed would + // leave a decision recorded, and a rerun would record a second. + if _, known := evaluatePolicyOutputs[o.output]; !known { + return fmt.Errorf("unsupported output format: %s. Valid formats are: [table, json]", o.output) + } + + trails, err := parseTrailContexts(o.contexts) + if err != nil { + return err + } + + decision, err := o.decision() + if err != nil { + return err + } + + return runServerEvaluation(out, serverEvaluation{ + policyRef: o.policyRef, + params: o.params, + trails: trails, + decision: decision, + output: o.output, + assertOnDeny: o.assert, + }) +} + +// decision resolves where the outcome is recorded, or nil where none was +// asked for. The flags are read as resolved values rather than as flags the +// caller typed, so KOSLI_FLOW and KOSLI_TRAIL satisfy the destination too. +func (o *evaluatePolicyOptions) decision() (*evaluations.Decision, error) { + if o.control == "" { + // Accepting these silently would look like a decision was recorded. + if given := namedDecisionFlags(o.name, o.fingerprint); given != "" { + return nil, fmt.Errorf("%s records a decision, so it needs --control", given) + } + // --flow and --trail are not refused with them: a pipeline sets those + // for every command it runs, and without a control they name nothing. + return nil, nil + } + + var missing []string + if o.flowName == "" { + missing = append(missing, "--flow") + } + if o.trailName == "" { + missing = append(missing, "--trail") + } + if len(missing) > 0 { + return nil, fmt.Errorf( + "a decision is recorded in a trail, so --control needs %s "+ + "(set as flags, or as KOSLI_FLOW and KOSLI_TRAIL)", + strings.Join(missing, " and ")) + } + + // A malformed fingerprint is ours to catch; one that names no artifact is + // the API's. + if o.fingerprint != "" { + if err := digest.ValidateDigest(o.fingerprint); err != nil { + return nil, err + } + } + + name := o.name + if name == "" { + name = o.control + "-decision" + } + + return &evaluations.Decision{ + Control: o.control, + Name: name, + Flow: o.flowName, + Trail: o.trailName, + Fingerprint: o.fingerprint, + }, nil +} + +var evaluatePolicyOutputs = map[string]bool{"table": true, "json": true} + +// namedDecisionFlags names the decision flags that were given, so a refusal +// speaks of what was typed. +func namedDecisionFlags(name, fingerprint string) string { + var given []string + if name != "" { + given = append(given, "--name") + } + if fingerprint != "" { + given = append(given, "--fingerprint") + } + return strings.Join(given, " and ") +} + +// parseTrailContexts reads the --context values as trail references, keeping +// the order they were given in. `trail` is the only kind of context there is +// today; the key is spelled out so another kind can be added without a second +// flag. +func parseTrailContexts(values []string) ([]evaluations.TrailRef, error) { + trails := make([]evaluations.TrailRef, 0, len(values)) + for _, value := range values { + kind, reference, found := strings.Cut(value, "=") + if !found || kind != "trail" { + return nil, fmt.Errorf("--context %q is not a context this command knows; "+ + "expected trail=/", value) + } + flow, trail, found := strings.Cut(reference, "/") + if !found || flow == "" || trail == "" || strings.Contains(trail, "/") { + return nil, fmt.Errorf("--context %q does not name a trail; "+ + "expected trail=/", value) + } + trails = append(trails, evaluations.TrailRef{Flow: flow, Trail: trail}) + } + return trails, nil +} diff --git a/cmd/kosli/evaluatePolicy_test.go b/cmd/kosli/evaluatePolicy_test.go new file mode 100644 index 000000000..11d58c01a --- /dev/null +++ b/cmd/kosli/evaluatePolicy_test.go @@ -0,0 +1,610 @@ +package main + +import ( + "fmt" + "os" + "path/filepath" + "sort" + "testing" + "time" + + "github.com/kosli-dev/cli/internal/evaluations" + "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" +) + +// This repository's test environment has no evaluator, so every test drives +// the stub the server-side suite stands up. +type EvaluatePolicyCommandTestSuite struct { + suite.Suite +} + +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 "+ + "--host %s --org test-org --api-token test-token --max-api-retries 0 %s", + host, extra) +} + +// Hidden while the command is proved out against a server that can run it. +func (suite *EvaluatePolicyCommandTestSuite) TestTheCommandIsHiddenForNow() { + _, listed, _, _, err := executeCommandC("evaluate --help") + + require.NoError(suite.T(), err) + require.NotContains(suite.T(), listed, evaluatePolicyShortDesc) + for _, sibling := range []string{"trail", "trails", "input"} { + require.Contains(suite.T(), listed, sibling, "the rest of the listing still renders") + } + + // Its own help still renders, for anyone told to try it. + _, help, _, _, err := executeCommandC("evaluate policy --help") + require.NoError(suite.T(), err) + require.Contains(suite.T(), help, "--context") +} + +// The flags that only make sense on this machine are absent rather than +// hidden, because a hidden flag is still reachable. +func (suite *EvaluatePolicyCommandTestSuite) TestItOffersOnlyItsOwnFlags() { + _, combined, _, _, err := executeCommandC("evaluate policy --help") + + require.NoError(suite.T(), err) + for _, flag := range []string{"--context", "--policy", "--params", "--output", "--assert"} { + require.Contains(suite.T(), combined, flag) + } + for _, flag := range []string{"--server-side", "--attestations", "--show-input", "--no-assert", "--sync"} { + require.NotContains(suite.T(), combined, flag) + } +} + +func (suite *EvaluatePolicyCommandTestSuite) TestItNamesTheRequiredFlagItWasNotGiven() { + for _, test := range []struct { + missing string + cmd string + }{ + {"context", "evaluate policy --policy testdata/policies/allow-all.rego"}, + {"policy", "evaluate policy --context trail=my-flow/my-trail"}, + } { + suite.Run(test.missing, func() { + _, _, _, _, err := executeCommandC(test.cmd + " --org test-org --api-token test-token") + + require.Error(suite.T(), err) + require.Contains(suite.T(), err.Error(), test.missing) + }) + } +} + +func (suite *EvaluatePolicyCommandTestSuite) TestItSendsThePolicyAndTheTrailAndPrintsTheVerdict() { + server, fake := newFakeEvaluations(suite.T(), verdictAllowed) + + _, combined, _, _, err := executeCommandC(suite.cmd(server.URL, "")) + + require.NoError(suite.T(), err) + require.Regexp(suite.T(), `RESULT:\s+ALLOWED`, combined) + + require.Len(suite.T(), fake.created, 1) + require.Equal(suite.T(), 0, fake.trailReads, "the trail is read where the policy runs") + require.Equal(suite.T(), 0, fake.unexpected) + + created := fake.created[0] + context := created["context"].(map[string]interface{}) + require.Equal(suite.T(), []interface{}{ + map[string]interface{}{"flow": "my-flow", "trail": "my-trail"}, + }, context["trails"]) + + files := created["policy"].(map[string]interface{})["files"].(map[string]interface{}) + require.Len(suite.T(), files, 1) + require.Contains(suite.T(), files, "allow-all.rego", "the policy travels under its own name") + require.Contains(suite.T(), files["allow-all.rego"], "package policy") +} + +// The body forbids what it does not name, so an unasked-for decision block is +// absent rather than empty. +func (suite *EvaluatePolicyCommandTestSuite) TestItAsksForNoDecision() { + server, fake := newFakeEvaluations(suite.T(), verdictAllowed) + + _, _, _, _, err := executeCommandC(suite.cmd(server.URL, "")) + + require.NoError(suite.T(), err) + require.NotContains(suite.T(), fake.created[0], "decision") +} + +func (suite *EvaluatePolicyCommandTestSuite) TestItPassesParamsOnUnchanged() { + for _, test := range []struct { + name string + flag string + want map[string]interface{} + }{ + {"inline json", `--params '{"min_approvers":2}'`, map[string]interface{}{"min_approvers": float64(2)}}, + {"a file", "--params @testdata/evaluate/params-low-threshold.json", map[string]interface{}{"threshold": float64(3)}}, + } { + suite.Run(test.name, func() { + server, fake := newFakeEvaluations(suite.T(), verdictAllowed) + + _, _, _, _, err := executeCommandC(suite.cmd(server.URL, test.flag)) + + require.NoError(suite.T(), err) + require.Equal(suite.T(), test.want, fake.created[0]["params"]) + }) + } +} + +// The API's params field rejects a null where it accepts an empty object. +func (suite *EvaluatePolicyCommandTestSuite) TestNoParamsTravelAsAnEmptyObject() { + server, fake := newFakeEvaluations(suite.T(), verdictAllowed) + + _, _, _, _, err := executeCommandC(suite.cmd(server.URL, "")) + + require.NoError(suite.T(), err) + require.Equal(suite.T(), map[string]interface{}{}, 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) + + _, combined, _, _, err := executeCommandC(suite.cmd(server.URL, "--output json")) + + require.NoError(suite.T(), err) + require.Equal(suite.T(), localAllowedJSON, combined) +} + +// Asserting is opt-in: recording a decision is not a reason to fail the step +// that asked for it. +func (suite *EvaluatePolicyCommandTestSuite) TestADenialPrintsInFullAndExitsZero() { + server, _ := newFakeEvaluations(suite.T(), verdictDenied) + + _, combined, _, _, err := executeCommandC(suite.cmd(server.URL, "")) + + require.NoError(suite.T(), err) + require.Regexp(suite.T(), `RESULT:\s+DENIED`, combined) + require.Contains(suite.T(), combined, "change is not approved") +} + +func (suite *EvaluatePolicyCommandTestSuite) TestAssertFailsOnADenial() { + server, _ := newFakeEvaluations(suite.T(), verdictDenied) + + _, combined, _, _, err := executeCommandC(suite.cmd(server.URL, "--assert")) + + require.Error(suite.T(), err) + require.Contains(suite.T(), err.Error(), "policy denied") + require.Regexp(suite.T(), `RESULT:\s+DENIED`, combined) + require.Contains(suite.T(), combined, "change is not approved") +} + +func (suite *EvaluatePolicyCommandTestSuite) TestAssertPassesOnAnAllow() { + server, _ := newFakeEvaluations(suite.T(), verdictAllowed) + + _, combined, _, _, err := executeCommandC(suite.cmd(server.URL, "--assert")) + + require.NoError(suite.T(), err) + require.Regexp(suite.T(), `RESULT:\s+ALLOWED`, combined) +} + +// The verdict is printed before the assertion, so the page is the same +// whichever exit code follows. +func (suite *EvaluatePolicyCommandTestSuite) TestAssertStillPrintsTheVerdictInJson() { + server, _ := newFakeEvaluations(suite.T(), verdictDenied) + + _, combined, _, _, err := executeCommandC(suite.cmd(server.URL, "--assert --output json")) + + require.Error(suite.T(), err) + require.Contains(suite.T(), combined, `"allow": false`) + require.Contains(suite.T(), combined, "change is not approved") +} + +// An evaluation that has not answered is never a verdict, with or without +// --assert. +func (suite *EvaluatePolicyCommandTestSuite) TestAnExpiredWaitNamesTheEvaluationAndNoVerdict() { + for _, extra := range []string{"", "--assert"} { + suite.Run("with "+extra, func() { + original := serverSideWaitOptions + serverSideWaitOptions = evaluations.WaitOptions{ + Timeout: 20 * time.Millisecond, + Initial: time.Millisecond, + Max: 2 * time.Millisecond, + } + defer func() { serverSideWaitOptions = original }() + + server, _ := newFakeEvaluations(suite.T(), createdPending) + + _, combined, _, _, err := executeCommandC(suite.cmd(server.URL, extra)) + + require.Error(suite.T(), err) + require.Contains(suite.T(), err.Error(), "still pending") + require.Contains(suite.T(), err.Error(), "01EVAL") + require.NotContains(suite.T(), combined, "DENIED") + require.NotContains(suite.T(), combined, "ALLOWED") + }) + } +} + +// Every trail resolves at one instant, which holds only if they travel in one +// evaluation. +func (suite *EvaluatePolicyCommandTestSuite) TestEveryContextGoesInOneEvaluation() { + server, fake := newFakeEvaluations(suite.T(), verdictAllowed) + + _, _, _, _, err := executeCommandC(suite.cmd(server.URL, + "--context trail=other-flow/second --context trail=my-flow/third")) + + require.NoError(suite.T(), err) + require.Len(suite.T(), fake.created, 1, "one evaluation, however many trails") + + context := fake.created[0]["context"].(map[string]interface{}) + require.Equal(suite.T(), []interface{}{ + map[string]interface{}{"flow": "my-flow", "trail": "my-trail"}, + map[string]interface{}{"flow": "other-flow", "trail": "second"}, + map[string]interface{}{"flow": "my-flow", "trail": "third"}, + }, context["trails"], "named in the order given") +} + +// The API stores a repeat once rather than refusing it, so refusing it here +// would be stricter than the thing being called. +func (suite *EvaluatePolicyCommandTestSuite) TestARepeatedContextIsSentAsGiven() { + server, fake := newFakeEvaluations(suite.T(), verdictAllowed) + + _, _, _, _, err := executeCommandC(suite.cmd(server.URL, "--context trail=my-flow/my-trail")) + + require.NoError(suite.T(), err) + context := fake.created[0]["context"].(map[string]interface{}) + require.Len(suite.T(), context["trails"], 2) +} + +func (suite *EvaluatePolicyCommandTestSuite) TestAMalformedContextIsRefusedBeforeAnyRequest() { + for _, test := range []struct { + name string + value string + }{ + {"no key", "my-flow/my-trail"}, + {"an unknown key", "artifact=my-flow/my-trail"}, + {"no flow and trail", "trail=my-trail"}, + {"an empty value", "trail="}, + {"an empty flow", "trail=/my-trail"}, + {"an empty trail", "trail=my-flow/"}, + {"more than a flow and a trail", "trail=my-flow/my-trail/extra"}, + } { + suite.Run(test.name, func() { + server, fake := newFakeEvaluations(suite.T(), verdictAllowed) + + _, combined, _, _, err := executeCommandC(fmt.Sprintf( + "evaluate policy --context %s --policy testdata/policies/allow-all.rego "+ + "--host %s --org test-org --api-token test-token --max-api-retries 0", + test.value, server.URL)) + + require.Error(suite.T(), err) + require.Contains(suite.T(), err.Error(), "trail=/", + "the refusal names the form it expects") + require.Empty(suite.T(), fake.created, "nothing is sent") + require.NotContains(suite.T(), combined, "RESULT") + }) + } +} + +func (suite *EvaluatePolicyCommandTestSuite) TestTooManyContextsAreRefusedBeforeAnyRequest() { + server, fake := newFakeEvaluations(suite.T(), verdictAllowed) + + contexts := "" + for i := 0; i <= maxServerSideTrails; i++ { + contexts += fmt.Sprintf("--context trail=my-flow/trail-%d ", i) + } + + _, _, _, _, err := executeCommandC(fmt.Sprintf( + "evaluate policy %s--policy testdata/policies/allow-all.rego "+ + "--host %s --org test-org --api-token test-token --max-api-retries 0", + contexts, server.URL)) + + require.Error(suite.T(), err) + require.Contains(suite.T(), err.Error(), fmt.Sprintf("%d", maxServerSideTrails)) + require.Empty(suite.T(), fake.created) +} + +func (suite *EvaluatePolicyCommandTestSuite) TestControlRecordsADecisionWhereItIsTold() { + server, fake := newFakeEvaluations(suite.T(), verdictAllowed) + + _, _, _, _, err := executeCommandC(suite.cmd(server.URL, + "--control SDLC-CTRL-0007 --flow release --trail my-trail "+ + "--fingerprint b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c")) + + require.NoError(suite.T(), err) + require.Equal(suite.T(), map[string]interface{}{ + "control": "SDLC-CTRL-0007", + "name": "SDLC-CTRL-0007-decision", + "flow": "release", + "trail": "my-trail", + "fingerprint": "b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c", + }, fake.created[0]["decision"]) +} + +// The default is computed here and sent, so the name is one the caller can +// predict. +func (suite *EvaluatePolicyCommandTestSuite) TestTheDecisionNameDefaultsToTheControl() { + server, fake := newFakeEvaluations(suite.T(), verdictAllowed) + + _, _, _, _, err := executeCommandC(suite.cmd(server.URL, + "--control SDLC-CTRL-0007 --flow release --trail my-trail")) + + require.NoError(suite.T(), err) + decision := fake.created[0]["decision"].(map[string]interface{}) + require.Equal(suite.T(), "SDLC-CTRL-0007-decision", decision["name"]) + require.NotContains(suite.T(), decision, "fingerprint", + "a decision about the trail carries no fingerprint") +} + +func (suite *EvaluatePolicyCommandTestSuite) TestAGivenNameIsSentAsGiven() { + server, fake := newFakeEvaluations(suite.T(), verdictAllowed) + + _, _, _, _, err := executeCommandC(suite.cmd(server.URL, + "--control SDLC-CTRL-0007 --flow release --trail my-trail --name code-review-decision")) + + require.NoError(suite.T(), err) + decision := fake.created[0]["decision"].(map[string]interface{}) + require.Equal(suite.T(), "code-review-decision", decision["name"]) +} + +// A pipeline sets the flow and the trail for every command it runs, so +// carrying them is not a reason to refuse a run that asked for no decision. +func (suite *EvaluatePolicyCommandTestSuite) TestADestinationWithoutAControlRecordsNothing() { + server, fake := newFakeEvaluations(suite.T(), verdictAllowed) + + _, combined, _, _, err := executeCommandC(suite.cmd(server.URL, "--flow release --trail my-trail")) + + require.NoError(suite.T(), err) + require.Regexp(suite.T(), `RESULT:\s+ALLOWED`, combined) + require.NotContains(suite.T(), fake.created[0], "decision") +} + +func (suite *EvaluatePolicyCommandTestSuite) TestWhatCannotBeAskedForIsRefusedBeforeAnyRequest() { + for _, test := range []struct { + name string + extra string + says []string + saysNot []string + }{ + {"a name with no control", "--name code-review-decision", + []string{"--name", "--control"}, []string{"--fingerprint"}}, + {"a fingerprint with no control", + "--fingerprint b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c", + []string{"--fingerprint", "--control"}, []string{"--name"}}, + {"a name and a fingerprint with no control", + "--name code-review-decision " + + "--fingerprint b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c", + []string{"--name", "--fingerprint", "--control"}, nil}, + {"a control with no destination", "--control SDLC-CTRL-0007", + []string{"--flow", "--trail"}, nil}, + {"a control with no trail", "--control SDLC-CTRL-0007 --flow release", + []string{"--trail"}, nil}, + // A malformed fingerprint is ours to catch, as on every other command + // that takes one. + {"a fingerprint that is not a SHA256", + "--control SDLC-CTRL-0007 --flow release --trail my-trail --fingerprint sha256:abc", + []string{"not a valid SHA256"}, nil}, + // Refused before the request, which with --control records a decision. + {"an output format that does not exist", + "--control SDLC-CTRL-0007 --flow release --trail my-trail --output tabel", + []string{"tabel", "table", "json"}, nil}, + } { + suite.Run(test.name, func() { + server, fake := newFakeEvaluations(suite.T(), verdictAllowed) + + _, combined, _, _, err := executeCommandC(suite.cmd(server.URL, test.extra)) + + require.Error(suite.T(), err) + for _, says := range test.says { + require.Contains(suite.T(), err.Error(), says) + } + for _, saysNot := range test.saysNot { + require.NotContains(suite.T(), err.Error(), saysNot, + "the refusal names what was typed, not what was not") + } + require.Empty(suite.T(), fake.created, "nothing is sent") + require.NotContains(suite.T(), combined, "RESULT") + }) + } +} + +// The environment satisfies the destination as the flags do. +func (suite *EvaluatePolicyCommandTestSuite) TestTheDestinationCanComeFromTheEnvironment() { + suite.T().Setenv("KOSLI_FLOW", "release") + suite.T().Setenv("KOSLI_TRAIL", "my-trail") + server, fake := newFakeEvaluations(suite.T(), verdictAllowed) + + _, _, _, _, err := executeCommandC(suite.cmd(server.URL, "--control SDLC-CTRL-0007")) + + require.NoError(suite.T(), err) + decision := fake.created[0]["decision"].(map[string]interface{}) + require.Equal(suite.T(), "release", decision["flow"]) + require.Equal(suite.T(), "my-trail", decision["trail"]) +} + +// A denial is a decision: it is recorded whether or not the caller asked the +// command to fail. +func (suite *EvaluatePolicyCommandTestSuite) TestADenialAsksForItsDecisionToo() { + server, fake := newFakeEvaluations(suite.T(), verdictDenied) + + _, combined, _, _, err := executeCommandC(suite.cmd(server.URL, + "--control SDLC-CTRL-0007 --flow release --trail my-trail --assert")) + + require.Error(suite.T(), err, "--assert still fails the step") + require.Contains(suite.T(), err.Error(), "policy denied") + require.Regexp(suite.T(), `RESULT:\s+DENIED`, combined) + require.Contains(suite.T(), fake.created[0], "decision") +} + +func (suite *EvaluatePolicyCommandTestSuite) TestTheRecordedDecisionIsReported() { + server, _ := newFakeEvaluations(suite.T(), verdictAllowedWithDecision) + + _, combined, _, _, err := executeCommandC(suite.cmd(server.URL, + "--control SDLC-CTRL-0007 --flow release --trail my-trail")) + + require.NoError(suite.T(), err) + require.Contains(suite.T(), combined, "01DECISION") +} + +func (suite *EvaluatePolicyCommandTestSuite) TestTheRecordedDecisionIsReportedInJson() { + server, _ := newFakeEvaluations(suite.T(), verdictAllowedWithDecision) + + _, combined, _, _, err := executeCommandC(suite.cmd(server.URL, + "--control SDLC-CTRL-0007 --flow release --trail my-trail --output json")) + + require.NoError(suite.T(), err) + require.Contains(suite.T(), combined, `"decision_attestation_id": "01DECISION"`) +} + +// 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() { + for _, test := range []struct { + name string + status int + message string + }{ + {"an unknown control", 404, "Control 'SDLC-CTRL-0007' does not exist in org 'test-org'"}, + {"an unresolvable trail", 404, "These trails do not exist in org 'test-org': my-flow/my-trail"}, + {"an archived destination", 400, "Flow named 'release' has been archived for organization 'test-org'"}, + {"an organisation without the entitlement", 403, + "Server-side evaluation is not enabled for this organization"}, + } { + suite.Run(test.name, func() { + server := newRefusingServer(suite.T(), test.status, + fmt.Sprintf(`{"message":%q}`, test.message)) + + _, combined, _, _, err := executeCommandC(suite.cmd(server.URL, + "--control SDLC-CTRL-0007 --flow release --trail my-trail")) + + require.Error(suite.T(), err) + require.Contains(suite.T(), err.Error(), test.message) + require.Contains(suite.T(), err.Error(), fmt.Sprintf("%d", test.status)) + require.NotContains(suite.T(), combined, "RESULT") + }) + } +} + +// A policy that could not run has decided nothing: it is never a denial, and +// nothing is recorded. +func (suite *EvaluatePolicyCommandTestSuite) TestAClassifiedFailureIsNotADenial() { + server, _ := newFakeEvaluations(suite.T(), + `{"id":"01EVAL","status":"failed","requested_at":1.0,"recorded_at":1.0,`+ + `"error":{"kind":"compile","message":"policy.rego:4: unexpected token"}}`) + + _, combined, _, _, err := executeCommandC(suite.cmd(server.URL, + "--control SDLC-CTRL-0007 --flow release --trail my-trail --assert")) + + require.Error(suite.T(), err) + require.Contains(suite.T(), err.Error(), "compile") + require.Contains(suite.T(), err.Error(), "policy.rego:4: unexpected token") + require.NotContains(suite.T(), err.Error(), "denied") + require.NotContains(suite.T(), combined, "DENIED") +} + +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 "+ + "--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]interface{})["files"].(map[string]interface{}) + require.Equal(suite.T(), []string{"README.md", "lib/helpers.rego", "policy.rego"}, sortedKeys(files), + "keyed by path relative to the directory, and nothing left out by name") + require.Contains(suite.T(), files["policy.rego"], "package policy") + require.Contains(suite.T(), files["lib/helpers.rego"], "package lib.helpers") +} + +// The API takes at least one file. +func (suite *EvaluatePolicyCommandTestSuite) TestAnEmptyDirectoryIsRefused() { + directory := suite.T().TempDir() + server, fake := newFakeEvaluations(suite.T(), verdictAllowed) + + _, _, _, _, err := executeCommandC(fmt.Sprintf( + "evaluate policy --context trail=my-flow/my-trail --policy %s "+ + "--host %s --org test-org --api-token test-token --max-api-retries 0", directory, server.URL)) + + require.Error(suite.T(), err) + require.Contains(suite.T(), err.Error(), directory) + require.Empty(suite.T(), fake.created) +} + +func (suite *EvaluatePolicyCommandTestSuite) TestABundleOverTheCapsIsRefusedWithTheCapNamed() { + for _, test := range []struct { + name string + build func(string) + says string + }{ + {"too many files", func(directory string) { + for i := 0; i <= maxPolicyBundleFiles; i++ { + require.NoError(suite.T(), os.WriteFile( + filepath.Join(directory, fmt.Sprintf("policy-%d.rego", i)), + []byte("package policy\n"), 0644)) + } + }, fmt.Sprintf("%d", maxPolicyBundleFiles)}, + {"too many bytes", func(directory string) { + source := make([]byte, serverPolicyMaxBytes+1) + for i := range source { + source[i] = 'a' + } + require.NoError(suite.T(), os.WriteFile( + filepath.Join(directory, "policy.rego"), source, 0644)) + }, fmt.Sprintf("%d", serverPolicyMaxBytes)}, + } { + suite.Run(test.name, func() { + directory := suite.T().TempDir() + test.build(directory) + server, fake := newFakeEvaluations(suite.T(), verdictAllowed) + + _, _, _, _, err := executeCommandC(fmt.Sprintf( + "evaluate policy --context trail=my-flow/my-trail --policy %s "+ + "--host %s --org test-org --api-token test-token --max-api-retries 0", + directory, server.URL)) + + require.Error(suite.T(), err) + require.Contains(suite.T(), err.Error(), test.says) + require.Empty(suite.T(), fake.created, "nothing is sent") + }) + } +} + +// A policy comes from the machine that runs the command. +func (suite *EvaluatePolicyCommandTestSuite) TestARemotePolicyIsRefused() { + for _, ref := range []string{"http://policies.example.com/pr.rego", "https://policies.example.com/pr.rego"} { + suite.Run(ref, func() { + server, fake := newFakeEvaluations(suite.T(), verdictAllowed) + + _, combined, _, _, err := executeCommandC(fmt.Sprintf( + "evaluate policy --context trail=my-flow/my-trail --policy %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.Empty(suite.T(), fake.created, "nothing is sent") + require.NotContains(suite.T(), combined, "RESULT") + }) + } +} + +func (suite *EvaluatePolicyCommandTestSuite) TestADryRunSendsNothing() { + 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 DRY_RUN --max-api-retries 0", server.URL)) + + require.NoError(suite.T(), err) + require.Empty(suite.T(), fake.created) + require.Equal(suite.T(), 0, fake.reads, "nothing was created, so there is no verdict to wait for") + require.NotContains(suite.T(), combined, "RESULT") +} + +func sortedKeys(files map[string]interface{}) []string { + keys := make([]string, 0, len(files)) + for key := range files { + keys = append(keys, key) + } + sort.Strings(keys) + return keys +} + +func TestEvaluatePolicyCommandTestSuite(t *testing.T) { + suite.Run(t, new(EvaluatePolicyCommandTestSuite)) +} diff --git a/cmd/kosli/evaluateServerSide_test.go b/cmd/kosli/evaluateServerSide_test.go index b60b85b66..ef37e4698 100644 --- a/cmd/kosli/evaluateServerSide_test.go +++ b/cmd/kosli/evaluateServerSide_test.go @@ -44,6 +44,8 @@ const ( `"result":{"allow":true}}` verdictDenied = `{"id":"01EVAL","status":"completed","requested_at":1.0,"recorded_at":1.0,` + `"result":{"allow":false,"violations":["change is not approved"]}}` + verdictAllowedWithDecision = `{"id":"01EVAL","status":"completed","requested_at":1.0,"recorded_at":1.0,` + + `"result":{"allow":true},"decision_attestation_id":"01DECISION"}` // What the local path prints for a verdict with nothing to report. The // server path has to match it byte for byte, or the two disagree on the diff --git a/cmd/kosli/root.go b/cmd/kosli/root.go index 320698cbb..32c689145 100644 --- a/cmd/kosli/root.go +++ b/cmd/kosli/root.go @@ -147,6 +147,14 @@ Paths the list already matches stay excluded whatever is later added there, so k flowNamesFlag = "[defaulted] The comma separated list of Kosli flows. Defaults to all flows of the org." outputFlag = "[defaulted] The format of the output. Valid formats are: [table, json]." serverSideFlag = "[hidden] Evaluate the policy on the Kosli server rather than on this machine. Unsupported and subject to change." + policyParamsFlag = "[optional] Policy parameters as inline JSON or @file.json. Available in policies as data.params." + policyAssertFlag = "[optional] Exit with a non-zero status when the policy denies. Without it the verdict is printed and the command exits 0." + policyContextFlag = "What to evaluate, as trail=/. Repeat it to evaluate several trails at one instant." + policyControlFlag = "[optional] Record the outcome as a decision against this control. Without it nothing is recorded." + policyDecisionFlowFlag = "[conditional] The Kosli flow the decision is recorded in. Only required with --control." + policyDecisionTrailFlag = "[conditional] The Kosli trail the decision is recorded in. Only required with --control." + policyDecisionNameFlag = "[defaulted] The attestation name the decision is recorded under. Defaults to -decision." + policyDecisionFingerprintFlag = "[optional] The SHA256 fingerprint of the artifact the decision is about. Without it the decision is about the trail." outputFlagWithMarkdown = "[defaulted] The format of the output. Valid formats are: [table, json, markdown]." searchByNameFlag = "[optional] Only list flows whose name contains this substring. The Kosli API supports alphanumeric characters and '-'." ignoreCaseFlag = "[optional] Perform case-insensitive matching for --name. By default matching is case sensitive." diff --git a/cmd/kosli/testdata/empty-flag-audit-coverage.json b/cmd/kosli/testdata/empty-flag-audit-coverage.json index 6a40d2f23..01381dd12 100644 --- a/cmd/kosli/testdata/empty-flag-audit-coverage.json +++ b/cmd/kosli/testdata/empty-flag-audit-coverage.json @@ -598,6 +598,18 @@ "policy": "string", "show-input": "bool" }, + "evaluate policy": { + "assert": "bool", + "context": "stringArray", + "control": "string", + "fingerprint": "string", + "flow": "string", + "name": "string", + "output": "string", + "params": "string", + "policy": "string", + "trail": "string" + }, "evaluate trail": { "assert": "bool", "attestations": "stringSlice", diff --git a/cmd/kosli/testdata/policies/bundle/README.md b/cmd/kosli/testdata/policies/bundle/README.md new file mode 100644 index 000000000..dd80703cd --- /dev/null +++ b/cmd/kosli/testdata/policies/bundle/README.md @@ -0,0 +1 @@ +The bundle's own notes, which are not a policy and do not travel with it. diff --git a/cmd/kosli/testdata/policies/bundle/lib/helpers.rego b/cmd/kosli/testdata/policies/bundle/lib/helpers.rego new file mode 100644 index 000000000..52a5bc662 --- /dev/null +++ b/cmd/kosli/testdata/policies/bundle/lib/helpers.rego @@ -0,0 +1,3 @@ +package lib.helpers + +always_true := true diff --git a/cmd/kosli/testdata/policies/bundle/policy.rego b/cmd/kosli/testdata/policies/bundle/policy.rego new file mode 100644 index 000000000..1a7908e35 --- /dev/null +++ b/cmd/kosli/testdata/policies/bundle/policy.rego @@ -0,0 +1,5 @@ +package policy + +import data.lib.helpers + +allow := helpers.always_true diff --git a/docs/handover/6920-evaluate-an-inline-policy-and-record-its-decis.md b/docs/handover/6920-evaluate-an-inline-policy-and-record-its-decis.md new file mode 100644 index 000000000..38d1caed9 --- /dev/null +++ b/docs/handover/6920-evaluate-an-inline-policy-and-record-its-decis.md @@ -0,0 +1,87 @@ +# Issue #6920: Evaluate an inline policy and record its decision in one request: `kosli evaluate policy` + +> **Last updated:** 2026-09-17 +> **Issue:** https://github.com/kosli-dev/server/issues/6920 +> **Implementation plan:** [docs/plans/6920-evaluate-policy.md](../plans/6920-evaluate-policy.md) +> **Collaborators:** Simon Castagna (engineer), Claude (claude-opus-5) + +--- + +## Problem Definition + +Server-side evaluation reaches the CLI today only through the hidden `--server-side` flag on `kosli evaluate trail` (#6700). It evaluates, and it records nothing. The decision Kosli stores still comes from a second command: a pipeline step reads `allow` out of a local report and asserts it back with `kosli attest decision --compliant=`. Kosli may have done the evaluating, but the decision in the audit record is asserted by the pipeline, from a value the pipeline could have typed by hand. + +`kosli evaluate policy` is one command and one request: the caller sends the policy, the trail references, a control and a destination, and the decision is recorded where the evaluation runs. The decision value never passes through this CLI. + +**Why it matters.** This is the first command whose output *is* the governance record, with no human-asserted value in between. It is what lets our own controls stop being self-reported. + +**Constraints and acceptance criteria.** + +- One request evaluates and records. No second command, no client-asserted compliance value. +- Without `--control` the evaluation runs, the verdict prints, and nothing is recorded. +- A denial is a decision, recorded as non-compliant with its violations. A policy that cannot run is not: it records nothing and must never print a denial. +- Destination flags without `--control` are refused, and a control or a destination the caller cannot write to is refused before the evaluation is queued. +- The command always waits for a terminal status; `--assert` exits non-zero on denial and changes nothing else. +- `--policy` accepts a single file or a directory on this machine, within the published bundle caps. A URL is refused. +- Output and `--output json` match `kosli evaluate trail`, so a caller switching commands does not re-parse. +- An organisation without the server-side evaluation entitlement is refused in words that name it. +- `--name` defaults to `-decision`, so the common case names only the control. + +**Scope.** Asynchronous evaluation is out of the first cut: the command always waits. Versioned and published policies are out of scope for the ticket, and again by decision for this round: an inline policy is the only policy there is until publishing exists. The policy-bundle digest goes with them. Also out: migrating our own controls onto this command, `kosli evaluate input`, and removing the client-side evaluator or the hidden `--server-side` flag. + +**Visibility.** The command is hidden while it is proved out, so it appears in no help listing and generates no docs page. The ticket asks for a published command; that is the last slice's decision, not a change of intent. + +**Public repository.** This repository is public. The plan, the code and the help text stay at the contract a caller can see — the published API schema, the requests sent and the answers received. + +--- + +## Plan + +Transcribed from [docs/plans/6920-evaluate-policy.md](../plans/6920-evaluate-policy.md), committed on this branch. Read it before starting any slice: it holds the contract this ticket adds, the command surface, the outcome mapping and a per-slice test list ready to copy into `TODO.md`. It builds on the #6700 plan rather than repeating it. Each slice is independently mergeable. + +- [x] Slice 1 — the command exists, evaluates one trail and prints the verdict. +- [x] Slice 2 — `--assert` exits non-zero on a denial. +- [x] Slice 3 — `--context` names what is evaluated, and is always required. +- [x] Slice 4 — `--control` records a decision, with `--flow` and `--trail` as its destination. +- [x] Slice 5 — refusals travel in the API's own words, and a classified failure is never a denial. +- [x] Slice 6 — a directory of policy files as one bundle, with the caps refused here. +- [ ] Slice 7 — help text, docs, changelog, lint, full test run, and a staging check against an entitled organisation. + +--- + +## PRs & Branches + +| Branch | PR | Status | +|--------|----|--------| +| `6920-evaluate-policy` | #1201 | open | + +--- + +## Decisions Made + +- The command declares its own options rather than inheriting the evaluate commands' shared ones, because four of those flags have no meaning here and inheriting them only to hide them is how two commands drift apart. +- Asserting is opt-in on this command and the default is silent, which is the reverse of `evaluate trail`. A command that records a decision should not fail a pipeline unless the caller asked it to, and the flag that asks is the one the tutorial already publishes. +- What is evaluated and where a decision lands are named separately: `--context` is the only way to say what to evaluate and is always required, while `--flow` and `--trail` name the destination alone. Neither is refused for being present without `--control`, because a pipeline sets them as environment variables for every command it runs, and refusing them would refuse an ordinary run that asked for no decision. The ticket's example predates this split. +- A malformed fingerprint, an unknown output format and a decision flag without its control are all refused before the request. The format is the reason this matters more here than on the other evaluate commands: those check it where they print, which costs nothing, while this one would have recorded a decision and then failed on the format, so a rerun would record a second. +- The command is hidden for now, against the ticket, which asks for a published one. Nothing about it can be exercised end to end until it runs against a server that can evaluate, and a command listed in `kosli evaluate --help` is a contract from the moment it ships. Hiding it is one line, and unhiding it also restores its documentation page; the ticket's published contract is the wrap-up slice's business. +- A policy comes from the machine that runs the command: this command does not fetch one from a URL, though the older evaluate commands do, because that way of naming a policy is on its way out and a new command should not take it on. +- A directory of policy files travels whole, with nothing left out by name and nothing here reading the modules. What a bundle may hold, and what its modules may import, is for the evaluator that runs it to judge; a rule here would refuse bundles the evaluator would have accepted, and would go stale as the evaluator changes. Only the published caps and an empty directory are refused here, because those the caller can act on before sending. +- Refusals are passed on as the API worded them rather than being classified here, and the slice that was to give each case a sentence of its own was cut back to two tests. A list of cases in the CLI would go stale against the server that writes them, and the one thing that must not vary — a failed policy never reading as a denial — is pinned by a test instead. +- The destination is read as a resolved value rather than as a flag the caller typed, so `KOSLI_FLOW` and `KOSLI_TRAIL` satisfy `--control` exactly as the flags do. +- The first cut of the command is synchronous only, and `--sync` is not offered: with nothing to opt into, the flag would name the one behaviour there is. A command whose purpose is recording a decision should not return before the decision exists. An asynchronous mode, and the `--sync` flag that would pair with it, belong to a later ticket if anyone asks for them. +- `--name` defaults to `-decision` rather than being required beside `--control`, so the common case names the control once. The default is computed before the request is sent, because a name the caller can predict is worth more than one chosen further away. +- Nothing is recorded without `--control`, and a destination flag without one is refused rather than ignored: accepting it would read as a decision having been recorded when none was. +- The verdict printer is reused, and the recorded decision id is the one thing added to it: an extra row in the table and an extra key in the json, present only where a decision was written. A caller moving from `evaluate trail` parses the same page, and one that asked for a decision gets the identifier it needs to read the record back. (supersedes an earlier decision to keep it outside the payload, which would have left json callers without it) +- Versioned policies are excluded from this round by the engineer, on top of the ticket's own exclusion of policy publishing. Until publishing exists, an inline policy is the only policy there is. +- Every outcome keeps the one exit code this CLI has always used, against the ticket's request for three. A single failure exit path is a product-wide convention and not this command's to change, as #6700 also found. The obligation moves to the wording instead: a denial, a broken policy, an unfinished evaluation and a refused destination each get a sentence of their own, and none may read as another. +- A slice of its own for refused flag combinations was dropped once the command became synchronous and `--name` gained a default: the only refusal left is a destination flag without `--control`, which belongs with the decision block that gives it meaning. +- The policy-bundle digest is not built. It belongs with versioned policies, so a decision recorded now cites the policy it ran by the evaluation that ran it and nothing more. +- Tests drive a stubbed server, as under #6700 and for the same reason: this repository's test environment cannot complete a server-side evaluation. That an evaluation really records a decision has to be checked on staging, which is why the wrap-up slice carries a manual check rather than a test. + +--- + +## Next Steps + +- [ ] Open a ticket for how a directory bundle is read: symlinks are followed today, so a link pointing out of the bundle is uploaded, and the caps are checked only after every file has been read. Raised in review on #1201 and deliberately left for its own ticket, together with saying in the help text that a directory is sent whole. +- [ ] Slice 7: help text, docs, changelog, the full integration run, and a staging check against an entitled organisation. +- [ ] Check what the create endpoint refuses for each destination failure, against staging, so Slice 5's messages are written from real answers rather than guessed. diff --git a/docs/plans/6920-evaluate-policy.md b/docs/plans/6920-evaluate-policy.md new file mode 100644 index 000000000..83438e51a --- /dev/null +++ b/docs/plans/6920-evaluate-policy.md @@ -0,0 +1,230 @@ +# Plan: `kosli evaluate policy` — evaluate and record a decision in one request + +> **Ticket:** https://github.com/kosli-dev/server/issues/6920 +> **Status:** written 2026-09-17 against CLI `main` @ `f4f57577`. No slice started. +> **Audience:** the agent or engineer who implements this. Follow the repo's TDD and thin-slice workflow (`CLAUDE.md`). Create a `## feat(evaluate): kosli evaluate policy` section in `TODO.md` from the slice list below before coding. +> **Builds on:** [docs/plans/6700-evaluate-server-side-flag.md](6700-evaluate-server-side-flag.md), which established the evaluations client, the wait, and the shared verdict printer. Read its sections 2 and 4 first: the create/read contract, the timing, and the outcome mapping are not repeated here. +> **Out of scope:** versioned and published policies, and the policy-bundle digest that belongs with them; asynchronous evaluation; migrating our own controls onto the command; removing the client-side evaluator or the hidden `--server-side` flag. + +**Public repository.** This repository is public. Everything below stays at the contract a caller can see: the published API schema, the requests the CLI sends, and the answers it gets. Nothing about how the platform is built belongs in this plan, in code comments, or in help text. + +--- + +## 1. What we build + +One published command that evaluates a policy and records the outcome in the same request: + +```shell +kosli evaluate policy \ + --flow my-release-flow \ + --trail "$GITHUB_SHA" \ + --policy ./policies/SDLC-CTRL-0007-code-review/policy.rego \ + --control SDLC-CTRL-0007 \ + --fingerprint "$ARTIFACT_FINGERPRINT" \ + --params '{"protected_branch": "master"}' \ + --assert +``` + +What it replaces is a two-step pipeline pattern: evaluate, read `allow` out of a local report, then assert that value back with `kosli attest decision --compliant=`. In that shape the recorded decision is whatever the pipeline typed. Here the caller asks for the decision and never carries its value. + +Three things follow from that, and they set every rule below: + +1. **The decision value never passes through this CLI.** The command sends the policy, the trail references and a destination. It does not read a verdict and then write it. +2. **A denial is a decision; a broken policy is not.** A policy that cannot run has decided nothing and must never print a denial or leave a non-compliant decision behind. +3. **Without `--control` nothing is recorded.** The evaluation runs, the verdict prints, the destination flags are not required and nothing is written. + +--- + +## 2. Contract + +Source of truth: the evaluations endpoints in the published Kosli API schema. Sections 2.1–2.3 of the #6700 plan still hold. This ticket adds one optional block to the create body and one optional field to the read response. + +### 2.1 Create, with a decision + +``` +POST /api/v2/evaluations/{org} +``` + +```json +{ + "context": { "trails": [ { "flow": "release", "trail": "my-trail" } ] }, + "policy": { "files": { "policy.rego": "package policy\n\nallow := true\n" } }, + "params": { }, + "decision": { + "control": "SDLC-CTRL-0007", + "name": "SDLC-CTRL-0007-decision", + "flow": "release", + "trail": "my-trail", + "fingerprint": "" + } +} +``` + +- `decision` is optional. Absent, the evaluation stands alone and writes nothing. Every object in this body still forbids unknown fields, so the block is sent only when asked for, never as nulls. +- `control`, `name`, `flow`, `trail` are required inside the block; `fingerprint` is optional, and absent the decision is recorded against the trail itself. +- The destination is checked when the evaluation is created, not after it has run: a control that does not exist, a destination the token cannot write to, a flow that cannot be attested to, and a fingerprint that is not in that trail are all refused at create time. The caller is never told "queued" for a decision that will never land. +- Refusals keep the envelope and the status codes listed in the #6700 plan; the messages name what was refused. The CLI passes them on untouched — see 4.4. + +### 2.2 Read + +The evaluation resource gains `decision_attestation_id`: the id of the decision this evaluation wrote, present only where one was asked for and has been written. Everything else about the read is unchanged. + +--- + +## 3. Command surface + +**Hidden for now.** The command is registered with `Hidden`, so it is in no help listing and gets no docs page, and it runs for anyone who names it. The ticket asks for a published command; unhiding it is one line, and it belongs to the wrap-up slice, once the command has been run against a server that can evaluate. + +| Flag | Required | Meaning | +|---|---|---| +| `--context` | yes | Repeatable `trail=/`. What is evaluated, all of it at one instant. | +| `--policy`, `-p` | yes | A `.rego` file or a directory on this machine. | +| `--params` | no | Inline JSON or `@file.json`, unchanged, read by the policy as `data.params`. | +| `--control` | no | The control the decision answers. Present, a decision is recorded; absent, nothing is. | +| `--flow`, `-f` | with `--control` | Flow the decision is recorded in. | +| `--trail` | with `--control` | Trail the decision is recorded in. | +| `--name` | no | The attestation name the decision is recorded under. Defaults to `-decision`. | +| `--fingerprint` | no | The artifact the decision is about. Absent, the decision is about the trail. | +| `--assert` | no | Exit non-zero when the policy denies. | +| `--output`, `-o` | no | `table` (default) or `json`, the same shapes `evaluate trail` prints. | + +**What is evaluated and where the decision lands are separate.** `--context` is the only way to name what is evaluated, and it is always required. `--flow` and `--trail` name nothing but the destination, so they are not required without `--control` — and they are commonly set as `KOSLI_FLOW` and `KOSLI_TRAIL` for every command in a pipeline, which is reason enough not to refuse a run that happens to carry them. The ticket's own example predates this split; the command surface here is the one to build. + +Not offered, and why: `--sync` (the command is always synchronous, so there is nothing to opt into), `--no-assert` (asserting is opt-in here, so its opposite is the default and needs no flag), `--attestations` and `--show-input` (filtering and input display happen where the evaluation runs, and this command never evaluates locally), `--server-side` (this command has no other side). + +**Waiting.** The command always waits for a terminal status and prints the verdict. `--assert` changes the exit code on a denial and nothing else. There is no asynchronous mode: a command whose purpose is recording a decision should not return before the decision exists. If one is ever wanted, it is a flag and a ticket of its own. + +--- + +## 4. Design decisions (assumptions for the implementer) + +### 4.1 One command, its own options + +`evaluate policy` declares its own options struct rather than embedding `commonEvaluateOptions`. It shares the policy load, the params parse, the evaluations client and the verdict printer, and nothing else: the shared struct carries four flags this command must not offer, and inheriting them to hide them again is how the two commands drift apart. + +### 4.2 The verdict printer is reused unchanged + +Output shape and `--output json` must match `evaluate trail`, so a caller switching commands does not re-parse. That is a constraint on this command, not a licence to change the printer. Anything new this command has to say — the recorded decision id — is said around the verdict, not inside its payload. + +### 4.3 What is refused before any request + +- `--name` or `--fingerprint` without `--control`: refused, naming `--control`. They are meaningless alone, and accepting them would look like a decision was recorded. +- `--control` without a `--flow` and a `--trail` to record in: refused, naming both. The destination is required on the wire. +- `--flow` and `--trail` without `--control`: accepted and ignored. They are set as environment variables for every command in a pipeline, so refusing them would refuse an ordinary run that asked for no decision. +- A `--context` that is not `trail=/`: refused, naming the form it expects. + +The caps are refused before a request too, but each is checked where its flag is built: the trail ceiling with `--context`, the bundle size with a directory of policy files. + +Each refusal says why, in its own sentence, rather than relying on cobra's "these flags conflict" wording. + +With `--control` and no `--name`, the name is `-decision`. The default is computed where the request is built and sent explicitly, because the field is required on the wire and a name the caller can predict is worth more than one chosen further away. + +### 4.4 Outcome mapping + +Four outcomes, and they must never be confused with each other: + +| Outcome | What prints | Exit | +|---|---|---| +| Completed, allowed | the verdict | 0 | +| Completed, denied | the verdict and its violations | non-zero under `--assert`, else 0 | +| Failed (a classified policy failure) | the failure and its kind, never a verdict | non-zero | +| Unfinished when the wait expires | the evaluation id, never a verdict | non-zero | + +Where the server explained itself, its words are passed on untouched. Two refusals keep wording of our own, exactly as in #6700: an organisation that is not entitled to server-side evaluation, and a server too old to serve the route. + +### 4.5 One exit code, and messages that tell the outcomes apart + +The ticket asks for denial, a broken policy and a fault of ours to be three distinguishable exit codes. They stay one code, as everywhere else in this CLI, and the outcomes are told apart by what they say. #6700 made the same call: a single failure exit path is a product-wide convention, and it is not this command's to change. The obligation that remains is on the wording — a broken policy, an unfinished evaluation and a refused destination each need their own sentence, and none of them may read as a denial. Slice 5 is where that is proved. + +### 4.6 A directory of policy files + +`--policy` pointing at a directory uploads every file below it as one bundle, keyed by path relative to that directory, within the published 100-file and 1 MiB caps. Nothing is left out by name, and nothing here reads the modules: what a bundle may hold, and what its modules may import, is the evaluator's to judge, and a rule here would refuse bundles the evaluator would have accepted. An empty directory is named here, because the API takes at least one file. A single file keeps today's behaviour: one entry named after the file, no extension imposed. A URL is refused: fetching a policy from one is on its way out, so this command never offers it. + +### 4.7 `--context` + +`--context trail=/`, repeated once per trail. The form is `key=value` so that other kinds of context can be added later without a second flag, and `trail` is the only key there is today. The decision's destination is named separately and need not be among them. + +### 4.8 Tests drive a stubbed server + +Unchanged from #6700, and the reason is unchanged: this repository's test environment cannot complete a server-side evaluation. Every test of this command drives a stubbed HTTP server that answers the documented shapes. The cost is that these tests pin our half of the contract only; that a decision really lands has to be checked on staging, and that check is in the wrap-up slice. + +### 4.9 A new command costs more than the command + +Adding a command and its flags means updating the command-surface fixture and the empty-flag audit's own specification, or the build goes red. Budget for it in the first slice, not the last. + +--- + +## 5. Slices + +Each is independently mergeable and leaves no half-finished behaviour exposed. + +### Slice 1: the command exists, evaluates one trail and prints the verdict + +The thinnest end-to-end path: `--flow`, `--trail`, `--policy`, `--params`, no decision, no asserting. Waits for a terminal status and prints the verdict, exiting 0 whatever it is. + +- Command registered under `evaluate`, `--help` reads correctly, required flags enforced +- One trail sent as the context, policy uploaded as a one-file bundle, params passed through +- Output matches `evaluate trail` byte for byte, table and json +- A dry run sends nothing and says so +- The command-surface fixture and the audit specification updated + +### Slice 2: `--assert` + +- `--assert` exits non-zero on a denial, and 0 on an allow +- Without it, a denial still prints in full and exits 0 +- An expired wait names the evaluation, prints no verdict, and fails whether or not `--assert` was given + +### Slice 3: `--context` names what is evaluated + +Replaces `--flow`/`--trail` as the evaluation target; they return in Slice 4 as the decision's destination alone. + +- `--context trail=/`, required, repeatable, order preserved +- A malformed or unknown context is refused, naming the form expected, before any request +- More than the published ceiling of trails is refused here, naming the cap +- A repeated pair travels as given, since the API stores it once rather than refusing it + +### Slice 4: `--control` records a decision + +- `--control` (+ `--flow`, `--trail`, optional `--name`, `--fingerprint`) sends the decision block +- Absent `--name`, the name sent is `-decision` +- `--name` or `--fingerprint` without `--control` is refused; `--flow` and `--trail` without it are not +- `--control` without a destination is refused, naming `--flow` and `--trail` +- Absent `--control`, no decision block is sent at all +- The recorded decision id is read back and reported once the evaluation completes +- A denial records a decision too — nothing about the decision block depends on the verdict + +### Slice 5: refusals travel in the API's own words + +Deliberately shallow. A refusal is reported as the status the API answered with and whatever it said about why, whatever it refused, so the command carries no list of cases to keep in step with the server. What is pinned here is that this holds for this command, and that a classified failure is never worded as a denial. + +- A refusal carrying a message is reported with that message and that status, and prints no verdict +- A policy that could not run reports its kind and its message, never a denial, and records nothing + +### Slice 6: a directory of policy files + +Relative keys, the file and byte caps refused here with the cap named, an empty directory named here, the bundle's contents left for the evaluator to judge, and a URL refused. + +### Slice 7: wrap-up + +Unhiding the command, help text and documentation, the changelog entry, `make lint`, the full integration run, and a manual check against staging with an entitled organisation: allow, deny, a broken policy, a decision recorded and read back, and a destination the token cannot write to. + +--- + +## 6. Test strategy + +- Command tests in a suite of their own, needing no running server, driving a stub that answers the documented shapes — as `evaluateServerSide_test.go` does today. +- Client tests in `internal/evaluations` for the new block and the new field: sent only when asked for, absent otherwise, and decoded when read back. +- The existing evaluate suites stay on the local test server and must stay green: nothing here changes them. +- Golden output comparisons against `evaluate trail` for the verdict, so the two cannot drift. + +--- + +## 7. Settled by the ticket owner + +Recorded here so no slice reopens them: + +1. **The policy digest is not built.** It belongs with versioned policies, and nothing in this command sends or shows one. +2. **One exit code.** Clear messages carry the difference between a denial, a broken policy and a fault of ours — see 4.5. +3. **Synchronous only.** No asynchronous mode and no `--sync` flag; the command always waits. +4. **`--name` defaults to `-decision`.** +5. **`--context` is always required, and is the only thing that names what is evaluated.** `--flow` and `--trail` name the decision's destination alone, and are never refused for being present without one. diff --git a/hack/empty-flag-audit/spec.json b/hack/empty-flag-audit/spec.json index 2a33f7243..1ec2b7c7a 100644 --- a/hack/empty-flag-audit/spec.json +++ b/hack/empty-flag-audit/spec.json @@ -4776,5 +4776,59 @@ "json" ] ] + }, + "evaluate policy": { + "args": [], + "flags": { + "context": "trail={flow}/{trail}", + "policy": "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", + "needs": "server-side evaluation", + "flags_to_test": [ + "assert", + "context", + "control", + "fingerprint", + "flow", + "name", + "output", + "params", + "policy", + "trail" + ], + "flag_values": { + "assert": "true", + "context": "trail={flow}/{trail}", + "control": "{control}", + "fingerprint": "1bef738d0bb1e690500f99a5b57d958caf3a5eb3e00d9012e1f4369fc6812e01", + "flow": "{flow}", + "name": "{name}", + "output": "json", + "params": "{}", + "policy": "cmd/kosli/testdata/policies/allow-all.rego", + "trail": "{trail}" + }, + "setup": [ + { + "argv": [ + "create", + "flow", + "{flow}", + "--use-empty-template" + ] + }, + { + "argv": [ + "begin", + "trail", + "{trail}", + "--flow", + "{flow}" + ] + } + ], + "verify": [] } } diff --git a/internal/evaluations/client.go b/internal/evaluations/client.go index ded957bea..fb114738b 100644 --- a/internal/evaluations/client.go +++ b/internal/evaluations/client.go @@ -32,9 +32,22 @@ 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]interface{} + Trails []TrailRef + Files map[string]string + Params map[string]interface{} + Decision *Decision +} + +// Decision is where an evaluation records its outcome, and against which +// control. The evaluation writes it itself, so the verdict never travels back +// through a caller that could assert a different one. +type Decision struct { + Control string `json:"control"` + Name string `json:"name"` + Flow string `json:"flow"` + Trail string `json:"trail"` + // Absent, the decision is about the trail rather than an artifact in it. + Fingerprint string `json:"fingerprint,omitempty"` } // Result is the policy's verdict. A denial is a result, not a failure. @@ -61,6 +74,8 @@ type Evaluation struct { RecordedAt float64 `json:"recorded_at"` 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"` } // IsTerminal reports whether the evaluation has finished and will not change. @@ -108,9 +123,10 @@ func (c *Client) Create(org string, request CreateRequest) (*Evaluation, error) Token: c.token, DryRun: c.dryRun, Payload: createPayload{ - Context: createContext{Trails: request.Trails}, - Policy: inlinePolicy{Files: request.Files}, - Params: params, + Context: createContext{Trails: request.Trails}, + Policy: inlinePolicy{Files: request.Files}, + Params: params, + Decision: request.Decision, }, }) if err != nil { @@ -126,9 +142,10 @@ func (c *Client) Create(org string, request CreateRequest) (*Evaluation, error) // every model behind this endpoint forbids unknown fields: the wire shape has // to be stated exactly here rather than inherited from a caller's struct. type createPayload struct { - Context createContext `json:"context"` - Policy inlinePolicy `json:"policy"` - Params map[string]interface{} `json:"params"` + Context createContext `json:"context"` + Policy inlinePolicy `json:"policy"` + Params map[string]interface{} `json:"params"` + Decision *Decision `json:"decision,omitempty"` } type createContext struct { diff --git a/internal/evaluations/client_test.go b/internal/evaluations/client_test.go index 614ce241c..e559ffbbf 100644 --- a/internal/evaluations/client_test.go +++ b/internal/evaluations/client_test.go @@ -259,3 +259,79 @@ func TestCreateSurvivesAMessageThatIsNotText(t *testing.T) { }) } } + +func TestCreateSendsNoDecisionWhenNoneIsAskedFor(t *testing.T) { + server, seen := newFakeServer(t, http.StatusCreated, createdBody) + + _, err := newTestClient(t, server.URL, false).Create("my-org", aCreateRequest()) + require.NoError(t, err) + + // The body forbids what it does not name, so an absent decision is absent + // rather than null. + require.NotContains(t, seen.body, "decision") +} + +func TestCreateSendsTheDecisionItWasGiven(t *testing.T) { + server, seen := newFakeServer(t, http.StatusCreated, createdBody) + + request := aCreateRequest() + request.Decision = &Decision{ + Control: "SDLC-CTRL-0007", + Name: "SDLC-CTRL-0007-decision", + Flow: "release", + Trail: "my-trail", + Fingerprint: "b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c", + } + + _, err := newTestClient(t, server.URL, false).Create("my-org", request) + require.NoError(t, err) + + require.Equal(t, map[string]interface{}{ + "control": "SDLC-CTRL-0007", + "name": "SDLC-CTRL-0007-decision", + "flow": "release", + "trail": "my-trail", + "fingerprint": "b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c", + }, seen.body["decision"]) +} + +// A decision about the trail itself carries no fingerprint, and an empty one +// is not the same as none. +func TestCreateLeavesOutAnAbsentFingerprint(t *testing.T) { + server, seen := newFakeServer(t, http.StatusCreated, createdBody) + + request := aCreateRequest() + request.Decision = &Decision{ + Control: "SDLC-CTRL-0007", + Name: "SDLC-CTRL-0007-decision", + Flow: "release", + Trail: "my-trail", + } + + _, err := newTestClient(t, server.URL, false).Create("my-org", request) + require.NoError(t, err) + + require.NotContains(t, seen.body["decision"], "fingerprint") +} + +func TestGetReadsTheDecisionTheEvaluationWrote(t *testing.T) { + server, _ := newFakeServer(t, http.StatusOK, + `{"id":"01JABCDEF","status":"completed","requested_at":1.0,"recorded_at":1.0,`+ + `"result":{"allow":true},"decision_attestation_id":"01DECISION"}`) + + evaluation, err := newTestClient(t, server.URL, false).Get("my-org", "01JABCDEF") + require.NoError(t, err) + + require.Equal(t, "01DECISION", evaluation.DecisionAttestationID) +} + +// An evaluation asked for no decision names none. +func TestGetReadsNoDecisionWhereNoneWasWritten(t *testing.T) { + server, _ := newFakeServer(t, http.StatusOK, + `{"id":"01JABCDEF","status":"completed","requested_at":1.0,"recorded_at":1.0,"result":{"allow":true}}`) + + evaluation, err := newTestClient(t, server.URL, false).Get("my-org", "01JABCDEF") + require.NoError(t, err) + + require.Empty(t, evaluation.DecisionAttestationID) +}