feat(#3345): improve error messages for Rego compilation failures

[fullsend-ai-coder]

Intercept rego_type_error and rego_parse_error from OPA/conftest compilation and wrap them with a user-friendly message that includes the bundled OPA version and remediation guidance.

Changes:

• Add internal/evaluator/rego_errors.go with isRegoCompilationError,
  bundledOPAVersion, and wrapRegoError helper functions
• Apply wrapRegoError in conftest_evaluator.go error paths from
  TestRunner.Run() and LoadWithData()
• Apply wrapRegoError in opa_evaluator.go error path from
  LoadWithData()
• Add unit tests for all new functions covering rego and non-rego
  error cases, OPA version lookup, and error preservation

The original low-level error is preserved via fmt.Errorf %w wrapping so it remains available for debugging.

---

Closes #3345

Post-script verification

• Branch is not main/master (agent/3345-rego-error-messages)
• Secret scan passed (gitleaks — d134e495b18cf8493cfea8791e310b58f0320797..HEAD)
• Pre-commit hooks passed (authoritative run on runner)
• Tests ran inside sandbox

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 8:53 AM UTC · Completed 8:59 AM UTC
Commit: 47d3320 · View workflow run →

[fullsend-ai-review]

Review — PR #3381

Verdict: request-changes

Overview

This PR adds user-friendly error wrapping for OPA/Rego compilation failures, fulfilling all four acceptance criteria from issue #3345. The implementation is clean and well-tested:

• New rego_errors.go with isRegoCompilationError, bundledOPAVersion, and wrapRegoError helper functions
• Error wrapping applied at three compilation error sites (2 in conftest_evaluator.go, 1 in opa_evaluator.go)
• Original error preserved via fmt.Errorf %w wrapping
• 186 lines of unit tests covering all functions, edge cases, and error chain preservation
• Test injection pattern (var readBuildInfo) matches existing convention in internal/version/version.go:35

The code logic is correct. However, there is one blocking issue.

---

Findings

🔴 HIGH — Acceptance test snapshot not updated

File: features/__snapshots__/validate_image.snap:2166-2170

The "Dropping rego capabilities" acceptance test (features/validate_image.feature:799) uses snapshot matching on stderr. The current snapshot expects the raw OPA error format:

Error: error validating image .../acceptance/ec-happy-day of component Unnamed: load: loading policies: get compiler: 3 errors occurred:
.../main.rego:14: rego_type_error: undefined function opa.runtime
.../main.rego:22: rego_type_error: undefined function http.send
.../main.rego:33: rego_type_error: undefined function net.lookup_ip_addr

After this PR, wrapRegoError intercepts these errors and produces a completely different message starting with policy compilation error: the policy uses Rego features or built-in functions not available.... The snapshot must be updated (e.g., via UPDATE_SNAPS=true make acceptance) for this test to pass.

Remediation: Run the acceptance test with UPDATE_SNAPS=true and commit the updated snapshot file. This is also an opportunity to verify that the improved error message renders well in the full end-to-end context.

---

🟡 LOW — Error message refers to "ec-cli" instead of current project name

File: internal/evaluator/rego_errors.go:72-76

The wrapped error message says "the one bundled in ec-cli" and "Upgrade ec-cli". The project has been renamed — cmd/root/root_cmd.go:69 identifies it as "Conforma CLI". User-facing error messages should use the current name for consistency.

Remediation: Replace ec-cli with conforma (the CLI binary name) or Conforma CLI in the error message template.

---

🟡 LOW — Hardcoded restricted-function list creates maintenance coupling

File: internal/evaluator/rego_errors.go:74

The error message hardcodes "opa.runtime, http.send, net.lookup_ip_addr" as example restricted builtins. The authoritative disallowed list lives in strictCapabilities() at conftest_evaluator.go:1199-1205. If the disallowed set changes, this error message becomes stale. The "e.g." qualifier provides some cover, but could still mislead users.

Remediation (follow-up): Consider extracting the disallowed builtins into a shared package-level variable that both strictCapabilities() and wrapRegoError() reference, or add a code comment cross-referencing the two locations.

---

🟡 LOW — ec test command path not covered

File: cmd/test/test.go

The ec test command calls conftest's runner.TestRunner.Run() directly without going through the wrapped conftestRunner.Run(). Users running ec test with incompatible policies will still see raw rego errors. This is a minor gap since ec test is a conftest-compatibility command, but it could confuse users who see different error quality between ec validate and ec test.

---

🟡 LOW — Only two of five OPA error categories are matched

File: internal/evaluator/rego_errors.go:40

isRegoCompilationError detects rego_type_error and rego_parse_error. OPA also defines rego_compile_error, rego_unsafe_var_error, and rego_recursion_error. While the two covered patterns are the most common for version/capability mismatches, a rego_compile_error from other causes would not get the improved message. Consider adding rego_compile_error at minimum.

---

Labels: PR modifies evaluator error handling for improved UX

Previous run

Review of PR #3381 — Improve error messages for Rego compilation failures

Verdict: request-changes — Two medium-severity findings that affect the PR's intended outcome and CI stability.

Summary

This PR adds a new rego_errors.go file with helper functions (isRegoCompilationError, bundledOPAVersion, wrapRegoError) that intercept rego_type_error and rego_parse_error from OPA/conftest compilation and wrap them with a user-friendly message including the bundled OPA version and remediation guidance. The wrapping is applied in three error paths: two in conftest_evaluator.go and one in opa_evaluator.go. Unit tests are comprehensive, and the %w wrapping correctly preserves the error chain.

The implementation is clean, well-structured, and addresses the core UX need described in #3345. However, two issues need resolution before merge.

Findings

1. Acceptance test snapshot not updated (medium — correctness)

File: features/__snapshots__/validate_image.snap (lines 2166–2170)

The "Dropping rego capabilities" acceptance test scenario (features/validate_image.feature:799) validates that ec validate image fails with raw rego_type_error messages when policies use sandboxed functions (opa.runtime, http.send, net.lookup_ip_addr). The snapshot currently expects:

Error: error validating image ... : load: loading policies: get compiler: 3 errors occurred:
.../main.rego:14: rego_type_error: undefined function opa.runtime
...

After this PR, wrapRegoError intercepts this error in conftestRunner.Run() (line 224), wrapping it with the "policy compilation error" message. The snapshot is not updated to reflect this change, so the acceptance test will fail in CI.

Remediation: Run acceptance tests with UPDATE_SNAPS=true make acceptance to regenerate the snapshot, or manually update the snapshot entry for [TestFeatures/Dropping rego capabilities:stderr - 1] to match the new wrapped error format.

2. Misleading error message for intentional capability restrictions (medium — correctness)

File: internal/evaluator/rego_errors.go (lines 62–73)

The wrapRegoError function attributes all rego_type_error occurrences to OPA version incompatibility:

"the policy uses Rego features or built-in functions not supported by the bundled OPA version (v1.15.2). This is likely caused by a version mismatch..."

However, rego_type_error also occurs when the strict capabilities sandbox intentionally strips dangerous built-ins (opa.runtime, http.send, net.lookup_ip_addr) — see strictCapabilities() in conftest_evaluator.go:1183. In this case the error is not a version mismatch; it's an intentional security restriction. A user whose policy calls opa.runtime() would be told to "upgrade ec-cli" when the actual issue is that the function is disallowed by design.

This is the exact error path validated by the "Dropping rego capabilities" acceptance test, so it's a realistic user scenario.

Remediation: Consider distinguishing between capability-restricted functions and genuinely unsupported features. For example, check whether the error mentions functions in the disallowed set from strictCapabilities() and produce a different message (e.g., "this function is restricted for security reasons"), or narrow the wrapping to only apply in opa_evaluator.go / error paths where capabilities are not being artificially restricted. Alternatively, adjust the message to mention capability restrictions as another possible cause.

What looks good

• Error chain preservation: Using fmt.Errorf with %w correctly preserves the original error for errors.Is/errors.As unwrapping.
• Test coverage: Unit tests cover nil errors, both error patterns, multi-error messages, non-matching errors, and build info edge cases. The readBuildInfo variable pattern for test injection is clean.
• Defensive nil handling: isRegoCompilationError(nil) correctly returns false, making wrapRegoError(nil) safe.
• Scope: The change is well-contained — only error wrapping is added, no behavioral changes to the evaluation pipeline itself.

---

Labels: PR modifies evaluator error handling and touches UX-facing error messages

[st3penta]

/fs-fix

[fullsend-ai-coder]

🤖 Finished Fix · ✅ Success · Started 12:00 PM UTC · Completed 12:12 PM UTC
Commit: 7c8ccca · View workflow run →

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 12:12 PM UTC · Completed 12:25 PM UTC
Commit: 7c8ccca · View workflow run →

[fullsend-ai-review]

See the review comment for full details.

[fullsend-ai-review]

[low] naming

Error message refers to "ec-cli" but the project has been renamed to "Conforma CLI" (see cmd/root/root_cmd.go:69). User-facing messages should use the current name.

Suggested fix: Replace "ec-cli" with "conforma" or "Conforma CLI" in the error message template.

[fullsend-ai-review]

[low] maintainability

The restricted-function list (opa.runtime, http.send, net.lookup_ip_addr) is hardcoded in the error message but the authoritative list lives in strictCapabilities() at conftest_evaluator.go:1199-1205. The lists could diverge over time.

Suggested fix: Extract the disallowed builtins into a shared variable, or add cross-reference comments.

[fullsend-ai-review]

[low] coverage-gap

Only rego_type_error and rego_parse_error are detected. OPA also defines rego_compile_error, rego_unsafe_var_error, and rego_recursion_error. Consider adding rego_compile_error at minimum.

Suggested fix: Add strings.Contains(msg, "rego_compile_error") to isRegoCompilationError.