Skip to content

feat(policies): structured violation output with typed finding schemas#2954

Merged
migmartri merged 7 commits intochainloop-dev:mainfrom
migmartri:feat/structured-policy-violations
Mar 30, 2026
Merged

feat(policies): structured violation output with typed finding schemas#2954
migmartri merged 7 commits intochainloop-dev:mainfrom
migmartri:feat/structured-policy-violations

Conversation

@migmartri
Copy link
Copy Markdown
Member

Summary

  • Policies can now return structured violation objects (maps with message + typed fields) alongside plain strings
  • New finding_type field in policy metadata declares the expected output schema (VULNERABILITY, SAST, LICENSE_VIOLATION)
  • Typed proto messages (PolicyVulnerabilityFinding, PolicySASTFinding, PolicyLicenseViolationFinding) with buf.validate constraints
  • oneof finding on Violation message stores validated structured data in the CAS-stored PolicyEvaluationBundle
  • Behavior matrix: no finding_type + strings (unchanged), no finding_type + objects (warning), finding_type + strings (error), finding_type + objects (validate + store)
  • Both Rego and WASM engines accept structured violations

Closes #2948

…hemas

Policies can now return structured violation objects (maps) in addition
to plain strings. When a policy declares `finding_type` in its metadata,
the engine validates violations against typed proto schemas
(PolicyVulnerabilityFinding, PolicySASTFinding, PolicyLicenseViolationFinding)
and stores the validated data in the CAS-stored PolicyEvaluationBundle.

Closes chainloop-dev#2948

Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4 issues found across 34 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="app/cli/internal/policydevel/eval.go">

<violation number="1" location="app/cli/internal/policydevel/eval.go:51">
P3: Avoid `omitempty` on `warnings` so the output shape is explicit and stable for consumers.

(Based on your team's feedback about preferring explicit states over omission in serialized output.) [FEEDBACK_USED]</violation>
</file>

<file name="pkg/attestation/crafter/api/attestation/v1/crafting_state.proto">

<violation number="1" location="pkg/attestation/crafter/api/attestation/v1/crafting_state.proto:325">
P2: Severity is documented as an enum-like set but not validated, so invalid values can pass typed finding validation.</violation>

<violation number="2" location="pkg/attestation/crafter/api/attestation/v1/crafting_state.proto:327">
P1: `cvss_v3_score` lacks the documented 0.0–10.0 validation range.</violation>
</file>

<file name="app/controlplane/api/workflowcontract/v1/crafting_schema.proto">

<violation number="1" location="app/controlplane/api/workflowcontract/v1/crafting_schema.proto:287">
P2: `finding_type` is documented as a fixed set of values but is not schema-validated, so invalid values can be accepted and only fail later at runtime.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread pkg/attestation/crafter/api/attestation/v1/crafting_state.proto Outdated
Comment thread pkg/attestation/crafter/api/attestation/v1/crafting_state.proto
Comment thread app/controlplane/api/workflowcontract/v1/crafting_schema.proto Outdated
Comment thread app/cli/internal/policydevel/eval.go Outdated
…rnings field

- Add buf.validate range constraint (0.0-10.0) on cvss_v3_score field
- Remove omitempty from Warnings JSON tag for stable output shape
- Add test for out-of-range CVSS score validation

Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
…oto mapping

- Add buf.validate string.in constraint on finding_type field
- Document the mapping between finding_type values and their proto messages

Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
@migmartri
Copy link
Copy Markdown
Member Author

Code review

Found 1 issue:

  1. New parseWasmViolation function has no test coverage (CLAUDE.md says "When implementing new functionality, follow TDD: write failing tests first, then implement the code to make them pass")

The function handles three code paths (string violations, structured map violations, and unknown types) but pkg/policies/engine/wasm/engine_test.go was not modified. The parallel Rego engine has comprehensive structured violation tests (TestRego_StructuredViolations, TestRego_StructuredSASTViolations, TestRego_StructuredLicenseViolations), but the WASM engine only tests the pre-existing string violation path.

// The violation can be either a JSON string or a JSON object with a required "message" field.
func parseWasmViolation(policyName string, raw json.RawMessage) (*engine.PolicyViolation, error) {
var v any
if err := json.Unmarshal(raw, &v); err != nil {
return nil, fmt.Errorf("invalid violation JSON: %w", err)
}
switch typed := v.(type) {
case string:
return &engine.PolicyViolation{
Subject: policyName,
Violation: typed,
}, nil
case map[string]any:
return engine.NewStructuredViolation(policyName, typed)
default:
return nil, fmt.Errorf("violation must be a string or object, got %T", v)
}
}

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
Comment thread pkg/policies/engine/rego/testfiles/structured_sast.rego
Comment thread pkg/policies/findings/registry.go
Add a builtin Rego function that creates structured vulnerability
findings, providing a cleaner API for policy authors instead of
manually constructing violation map objects.

Signature: chainloop.vulnerability(message, external_id, package_purl, severity)
Optional fields can be added via Rego's object.union.

Signed-off-by: Miguel Martinez <miguel@chainloop.dev>
@migmartri migmartri merged commit 260e547 into chainloop-dev:main Mar 30, 2026
15 checks passed
@migmartri migmartri deleted the feat/structured-policy-violations branch March 30, 2026 14:02
}

func vulnerabilityImpl(_ topdown.BuiltinContext, operands []*ast.Term, iter func(*ast.Term) error) error {
if _, ok := operands[0].Value.(ast.String); !ok {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

operands[0] is the name of the function, not the first argument, if I'm not wrong. I think all these validations are not needed, the Rego engine already takes care.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Typed structured output for policy violation findings

2 participants