feat(evaluate): add --output-rule to add policy rules to the output

[jbpros]

Adds --output-rule to kosli evaluate input|trail|trails. It puts extra policy rules in the JSON output, next to allow and violations.

Why: a policy can build a full evidence report (like the rego evidence report library does), but kosli evaluate drops everything except allow and violations. The only workaround so far was smuggling JSON strings through violations.

kosli evaluate trails $SHAS --flow f --policy p.rego --output-rule report --output json | jq .report

How it behaves

• Repeatable, or comma-separated: --output-rule report,summary.
• Rules are evaluated whether the policy allows or denies.
• A rule the policy doesn't declare is an error. A declared rule with no value gives null.
• Names that clash with output keys (allow, violations, input, params, decision_attestation_id) are refused.
• Table output leaves them out and warns on stderr that they only show with --output json.

Not doing now

• --server-side: refused for now, because the opa-lambda only returns allow and violations. Follow-up: Support --output-rule with --server-side #1215.
• Multi-file policies locally: --policy still loads a single module, so a library has to be bundled into package policy. To be fixed separately.

Checklist

• Docs are autogenerated from CLI help — make sure the help text reflects your changes
• Helm chart (charts/k8s-reporter/) updated, if needed. Note: these changes live in a separate PR
• Terraform provider and related changes (terraform-provider-kosli, terraform-aws-evidence-reporter, terraform-aws-kosli-reporter) updated, if needed

🤖 Generated with Claude Code

[claude]

Claude finished @jbpros's task in 7m 21s —— View job

---

Review: --output-rule for kosli evaluate

• Gather context (CLAUDE.md, PR diff)
• Static analysis (go build, go vet, go mod tidy)
• Code review
• Post findings (4 inline comments)

Static analysis

Check	Result
go build ./...	✅ clean
go vet ./...	✅ clean
go mod tidy + git diff go.mod go.sum	✅ no drift
go test ./internal/evaluate/	⚠️ not run — go test is outside this job's allowed tools, so I reviewed the tests by reading them. The integration suites need the Docker Compose server anyway.

Findings (all inline)

1. Detached doc comment (cmd/kosli/evaluateHelpers.go:542) — evaluateResultKeys and validateOutputRules were inserted between the printEvaluateResult doc comment and the function, so the comment now documents the var. Suggestion attached.
2. Reserved-key list can drift (evaluateHelpers.go:560) — evaluateResultKeys hand-duplicates the keys printEvaluateResult writes. Add a new output key later (as decision_attestation_id recently was) and an --output-rule of that name passes validation, then gets silently clobbered — the rule loop runs before the reserved assignments, so the user's value vanishes with no error. The existing test repeats the same literal list, so it can't catch the drift.
3. Ref-head rules and empty names (internal/evaluate/rego.go:90) — declared is keyed on Head.Name, which isn't the plain name for a ref head like report.summary := "x"; --output-rule report would be refused even though data.policy.report evaluates fine. Report-library policies are exactly the ones likely to use ref heads. Related: --output-rule report, yields an empty name that flows toward the string-concatenated query data.policy.. Validating names as identifiers up front closes both.
4. Recompiles the module per rule (rego.go:54) — evalQuery builds a fresh rego.New each call, so --output-rule a,b,c compiles the policy 4–5 times. A single data.policy query, or a hoisted PrepareForEval, gets it to one compile. Not a blocker; matters most for the big report policies this feature targets.

Minor, not worth blocking: evaluateAndPrintResult / printEvaluateResult are up to 8 positional params — an options struct would read better next time one is added. printEvaluateResult also emits the --output-rule warning, which is dead on the server-side path that shares it; evaluateAndPrintResult would be a more natural home.

What's good

• The slicing is exemplary: nine commits, each one capability (add the flag → repeat/comma → undeclared rule → clash guard → table warning → server-side refusal → docs → audit), each green on its own. Matches the CLAUDE.md workflow closely.
• Test coverage is genuinely thorough for the interesting axes — allow and deny, repeated and comma-separated, declared-but-undefined giving null, table output warning on stderr, every reserved name refused via a table-driven subtest, and the server-side refusal alongside the existing --attestations/--show-input ones.
• The evalQuery extraction is a real cleanup: it removed the duplicated rego.New + store setup between Evaluate and collectViolations rather than adding a third copy.
• Refusing --server-side with a message that says why (the server only returns allow and violations) follows the established refuseWhatTheServerCannotDo convention exactly, and the hack/empty-flag-audit/spec.json update keeps the flag under audit instead of quietly exempting it.
• Clean call on rejecting rule names that collide with output keys instead of namespacing them — the output stays flat and readable, and the failure is loud.

None of the above blocks merge; item 3 is the one I'd want a test for before shipping, since it decides whether real report policies work.
· branch feat/evaluate-output-rule

[claude]

The printEvaluateResult doc comment is now detached from the function it documents — evaluateResultKeys and validateOutputRules were inserted between them, so godoc now reads the comment as documenting the var.

Suggested change

	// printEvaluateResult renders a verdict, whatever produced it, so that every
	// evaluation path prints the same bytes for the same verdict.
	var evaluateResultKeys = []string{"allow", "violations", "input", "params", "decision_attestation_id"}
	func validateOutputRules(rules []string) error {
	for _, rule := range rules {
	if slices.Contains(evaluateResultKeys, rule) {
	return fmt.Errorf("--output-rule cannot be '%s', it is already part of the output", rule)
	}
	}
	return nil
	}
	var evaluateResultKeys = []string{"allow", "violations", "input", "params", "decision_attestation_id"}
	// validateOutputRules refuses a rule name that printEvaluateResult already
	// uses as an output key, so a policy rule can never shadow the verdict.
	func validateOutputRules(rules []string) error {
	for _, rule := range rules {
	if slices.Contains(evaluateResultKeys, rule) {
	return fmt.Errorf("--output-rule cannot be '%s', it is already part of the output", rule)
	}
	}
	return nil
	}
	// printEvaluateResult renders a verdict, whatever produced it, so that every
	// evaluation path prints the same bytes for the same verdict.

[claude]

evaluateResultKeys is a hand-maintained copy of the keys this function writes, and nothing ties the two together. decision_attestation_id was itself a recent addition; the next output key added below will pass validateOutputRules unnoticed, and because this rule loop runs before the reserved keys are assigned, the reserved key silently wins and the user's --output-rule value disappears with no error.

Cheapest guard is a test that asserts every key printEvaluateResult can emit (verdict + decision id + --show-input + --params) is present in evaluateResultKeys; the test in evaluateInput_test.go currently repeats the same literal list, so it can't catch the drift. A comment here pointing at evaluateResultKeys would help too.

Fix this →

[claude]

Each output rule re-parses and re-compiles the whole module: evalQuery builds a fresh rego.New(...) every call, so --output-rule a,b,c compiles the policy 4–5 times (allow, violations, plus one per rule). For the evidence-report policies this feature exists for (large modules, often the whole rego evidence-report library bundled in), compilation dominates evaluation time.

Two cheaper shapes:

• rego.New(rego.Query("data.policy"), ...) once, then pick allow / violations / each output rule out of the returned object — one compile, one eval;
• or keep per-rule queries but hoist a rego.PrepareForEval of the compiled module and reuse the prepared query.

Not a blocker at today's rule counts, but worth a follow-up.

[claude]

Two edge cases the declared map doesn't cover, both worth a test in rego_test.go:

1. Ref-head rules. Head.Name is only the plain rule name for single-var heads; for a ref head such as report.summary := "x" the name is not report, so --output-rule report is rejected even though data.policy.report exists and evaluates fine. Using rule.Head.Ref() (first term) rather than Head.Name would cover both spellings. Policies built from a report library are exactly the ones likely to use ref heads.
2. Empty rule names. --output-rule report, yields ["report", ""] from StringSliceVar. Today that fails with policy does not declare a '' rule (acceptable), but if any parse path ever puts "" in declared, the empty name flows into evalQuery as the query data.policy. and surfaces as a raw Rego parse error. Rejecting a non-identifier rule name up front in validateOutputRules would close both the ugly message and the string-concatenated query.

The concatenation at line 69 ("data.policy."+rule) is safe only because an arbitrary string can't match a declared rule name — a name check would make that explicit rather than incidental.

[claude]

This guard is now repeated verbatim in all three run() methods (evaluateInput.go:119, evaluateTrail.go:122, evaluateTrails.go:115), while the shared choke point evaluateAndPrintResult — which every one of them funnels into — doesn't check. A fourth evaluate subcommand that embeds commonEvaluateOptions gets the flag from addFlags for free but silently loses the clash check unless the author remembers this line.

Moving the call into evaluateAndPrintResult (and leaving refuseWhatTheServerCannotDo to handle --server-side) makes the guard structural. One behaviour change to note either way: --server-side --output-rule allow currently reports the clash rather than the more useful "not supported with --server-side".

[mbevc1]

@jbpros rebaser to main should fix that failing test