chore: check and report four-eyes to Kosli

[mbevc1]

Add four eyes evaluation and attest result to Kosli.

[claude]

Claude finished @mbevc1's task in 2m 57s —— View job

---

Review of PR #891 — Four-Eyes Evaluation & Attestation

• Run static analysis (go build, go vet, go mod tidy)
• Read all changed files
• Review workflow changes (init_kosli.yml)
• Review Rego policy (four-eyes-policy.rego)
• Review JSON schema (four-eyes-result-schema.json)
• Review setup script (setup_attestation_type.sh)
• Review flow template changes
• Post inline comments and summary

---

Summary

Static analysis: All clean — go build, go vet, and go mod tidy pass with no issues. This PR adds no Go code, only workflow/policy/config files.

Overall: This is a well-structured PR. The positive-assertion model in the Rego policy is a good design choice — allow is false by default and only becomes true when every trail explicitly satisfies trail_compliant. The fail-closed-by-construction approach (flagged by @tooky and @JonJagger in earlier discussion) is the right call. The separation of violations (diagnostic) from the compliance decision (allow) is thoughtful and prevents formatting bugs from affecting the policy outcome.

Previous review issues addressed:

• ✅ --org flag added to both new workflow steps
• ✅ 4eyes.sh scratch file removed
• ✅ four-eyes-result moved to main-flow-template.yml (not release template)
• ✅ run: | block scalar used for steps with \ continuations and embedded JSON
• ✅ --params '{"attestation_name": "pr"}' added to evaluate step
• ✅ count(pr.approvers) > 0 guard added against vacuous truth
• ✅ Error fallback JSON on evaluate failure
• ✅ Setup script accepts org as argument

Remaining items (all minor/nits — see inline comments):

1. Default attestation name mismatch (four-eyes-policy.rego:27): Default is "pr-review" but workflow uses "pr". Won't bite in CI (params override it), but is a trap for local testing/debugging.
2. Duplication in has_independent_approval (lines 98–113 vs 117–132): Two near-identical rule bodies could share a helper. Not blocking.
3. regex.match for plain substring ("noreply@github.com"): Works correctly but contains() would be more semantically clear for non-regex patterns.
4. Possible race between attest and evaluate (workflow lines 93–105): Worth confirming that the Kosli API is synchronous so the just-created PR attestation is visible to evaluate trails.

What looks good:

• Positive-assertion model is fail-closed by construction — no accidental approval from empty sets
• Clean separation of compliance logic from violation diagnostics
• Fallback JSON on command failure prevents pipeline breakage
• Schema with --jq ".allow == true" in the attestation type is a nice belt-and-suspenders check
• Setup script is properly guarded with set -euo pipefail and token validation

[tooky]

@mbevc1 I wonder if we need to think about this pattern - as @JonJagger pointed out, this can lead to accidental approval, where the violation_reasons ends up as an empty array not because there were no violoations but because of a mistake in the rego.

[mbevc1]

That's a fair point, but I wonder if there is something we can do here or we should record this and circle back

[JonJagger]

FWIW - I found that Claude had no problem re-writing the old tutorial to switch to positive assertions (as Tooky suggests).
https://docs.kosli.com/tutorials/evaluate_trails_with_opa
I think doing it now makes sense, especially if Sofus can then choose to employ the changes too.

[JonJagger]

The core principles of using positive assertions is also in the readme in this repo https://github.com/kosli-dev/snyk-blog#3-continuous-environment-scanning-workflows

[tooky]

yeah maybe. I was wondering if we should do soemthing where expressly capture that we have checked each "rule" and then check that collection.

Something like:

{
  "results": [
    {
      "allow": true,
    },
    {
      "allow": false,
    },
    {
      "allow": true,
    }
  ]
}

default allow := false

allow if {
	count(input.results) > 0
	every result in input.results {
		result.allow == true
	}
}

[mbevc1]

@tooky @JonJagger pushed new policy which is using positive assertions