fix: prevents paanic on violations as array

[gusfcarvalho]

Summary by CodeRabbit

• Bug Fixes

  • Policies are now filtered to the compliance framework namespace; only relevant packages are evaluated.
  • More robust handling of policy outputs: validates shapes before decoding, returns clear contextual errors (policy package + file) instead of crashes or debug prints.

• Tests

  • Added test coverage for policies that emit violations as a set to ensure correct decoding and single-violation results.

[coderabbitai]

Caution

Review failed

Pull request was closed or merged during review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 15818f8d-0868-42b0-be5a-595ee4f97fb1

📥 Commits

Reviewing files that changed from the base of the PR and between 4218519 and 7a78292.

📒 Files selected for processing (1)

• policy-manager/policy-manager.go

---

📝 Walkthrough

Walkthrough

PolicyManager.Execute now validates expression output types, normalizes violation results from object or set shapes into JSON entries, decodes them into Violation objects with contextual errors, and avoids panics. A new test verifies handling of set-defined violations.

Changes

Policy Evaluation Error Handling

Layer / File(s)	Summary
Normalize violation entries policy-manager/policy-manager.go	Adds normalizeViolationEntries to convert violation results from object (map[string]interface{}) or set ([]interface{}) shapes into uniform JSON-encoded entries.
Namespace gate for module evaluation policy-manager/policy-manager.go	Skips modules outside the data.compliance_framework namespace so only compliance policies are evaluated.
Validate module outputs and unmarshal violations policy-manager/policy-manager.go	PolicyManager.Execute now requires expression Value to be map[string]interface{} before treating it as module outputs; it uses normalized entries to unmarshal each Violation, returning errors that include policy package and file on failures.
Return structured decode errors policy-manager/policy-manager.go	Decoding module outputs into EvalOutput no longer panics or prints debug output; it returns errors that wrap mapstructure.Decode failures and include policy package/file context.
Test: supports violations declared as a set policy-manager/policy-manager_test.go	Adds a subtest defining an inline Rego policy where violation is produced as a set; executes PolicyManager.Execute with violating input and asserts a single parsed Violation with expected fields.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

I nibble bytes beneath the log, 🐇
I sort the shapes that once were fog,
No panic now when types misbehave,
Just tidy errors, calm and brave.
Tests hop forward — peace in the code.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title contains a typo ('paanic' instead of 'panic') and describes the main fix: handling violations as an array to prevent panics.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@policy-manager/policy-manager_test.go`:
- Line 103: Replace the redundant explicit type declaration for the test
variable `data` with Go's short variable declaration so the type is inferred
from `make(map[string]interface{})`; locate the `data` declaration in
policy-manager_test.go (currently `var data map[string]interface{} =
make(map[string]interface{})`) and change it to use `:=` to satisfy ST1023 and
simplify the code.

In `@policy-manager/policy-manager.go`:
- Around line 183-190: The error message in the violations type-check block
(where you do val.(map[string]interface{}) into violationsMap) is misleading
because it assumes val is a slice; update the error to report the actual runtime
type or a more generic message: include fmt.Sprintf("got %T", val) (or change to
"expected violations to be a map/object") and keep the existing context
(result.Policy.Package, result.Policy.File) so the error reads accurately for
violationsMap, val, result.Policy.Package and result.Policy.File.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 21ec03db-aa57-485f-83f5-a58c8e44165c

📥 Commits

Reviewing files that changed from the base of the PR and between bf76279 and 2fdade0.

📒 Files selected for processing (2)

• policy-manager/policy-manager.go
• policy-manager/policy-manager_test.go

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@policy-manager/policy-manager.go`:
- Around line 187-189: The json.Unmarshal error returned for the violation
decode should be wrapped with context so failures are diagnosable; instead of
returning the raw err from json.Unmarshal([]byte(violation), viol) update the
error return to include identifying context (e.g., policy ID/name and
file/package) — reference the variables json.Unmarshal, violation and viol in
policy-manager.go and wrap the error with fmt.Errorf or errors.Wrapf (e.g.,
"failed to unmarshal violation for policy %s: %w", policyID, err) and add the
required import if missing.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: e0f700ff-2b2f-410f-8af9-d854f0c0de48

📥 Commits

Reviewing files that changed from the base of the PR and between 2fdade0 and 6364dae.

📒 Files selected for processing (2)

• policy-manager/policy-manager.go
• policy-manager/policy-manager_test.go

[coderabbitai]

Actionable comments posted: 0

[Copilot]

Pull request overview

This PR hardens the policy execution path in policy-manager to avoid panics when OPA returns violation in different shapes (map vs array), and improves error reporting/context while removing leftover debug output.

Changes:

• Normalize violation outputs from OPA (map-encoded set keys vs slice entries) into a single decoding path with contextual errors.
• Avoid panics / unsafe type assertions by validating module output types and returning structured errors.
• Add a regression test covering violation contains { ... } (set-style) policies.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
policy-manager/policy-manager.go	Adds normalization for OPA violation shapes, improves type-safety and error messages, and removes debug printing/panic.
policy-manager/policy-manager_test.go	Adds a test ensuring set-style violation rules decode correctly into a single violation.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.