Include enforcement date in warnings for future effective_on violations

[Acepresso]

When the CLI demotes a deny result to a warning due to a future effective_on date, the warning message is now enhanced to indicate when enforcement begins.

Before: "The required 'cpe' label is missing"
After: "The required 'cpe' label is missing. This will become a failure starting on 2025-01-01T00:00:00Z"

This applies to both sources of effective_on: rule annotations and rule_data items. The effective_on date is included as-is (RFC3339 format), matching the pattern used by trusted_task's future_deny_rule warnings.
No duplicate warnings for trusted_task rules. The enhancement only applies to failures being demoted, not to existing warning rules like future_deny_rule.

Test plan

• Unit tests for formatFutureEnforcementMessage covering: valid effective_on, missing metadata, non-string value, empty string, nil metadata
• Existing TestConftestEvaluatorEvaluateSeverity updated and passing
• Full evaluator unit test suite passes with no regressions
• Run ec validate against a policy with a future effective_on date and verify the warning message includes the enforcement date

Ref: https://redhat.atlassian.net/browse/EC-1901
Assisted-by: Claude Opus 4.6

[coderabbitai]

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Enterprise

Run ID: 56eb698b-f2a8-42e5-a224-d20e9f5f82bd

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[fullsend-ai-review]

🤖 Review · ⚠️ Cancelled · Started 3:21 PM UTC · Ended 3:25 PM UTC
Commit: 87c4a29 · View workflow run →

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 3:26 PM UTC · Completed 3:31 PM UTC
Commit: 87c4a29 · View workflow run →

[fullsend-ai-review]

Review — Approve

PR: #3412 — Include enforcement date in warnings for future effective_on violations
Author: Acepresso · Status: Draft · Files changed: 3 · Diff: +111 / −3

Summary

When the CLI demotes a deny/failure result to a warning because its effective_on date is in the future, the warning message is now enhanced to indicate when enforcement begins (e.g., "The required 'cpe' label is missing. This will become a failure starting on 2025-01-01T00:00:00Z").

The change introduces a helper function formatFutureEnforcementMessage and applies it in both LegacyPostEvaluationFilter.CategorizeResults and UnifiedPostEvaluationFilter.CategorizeResults. The helper is well-guarded: it returns the original message unchanged when metadata is nil, the effective_on key is missing, or the value is non-string or empty. Implicitly, the isResultEffective gate in the calling code ensures only validly-parseable future dates reach the enhancement path.

Findings

All findings are low severity — none are blocking.

---

1. strings.TrimRight vs strings.TrimSuffix — subtle behavior difference

Severity: low · File: internal/evaluator/filters.go · Line: ~1233

strings.TrimRight(originalMessage, ".") removes all trailing characters in the cutset, not just a single trailing period. If a message ever ends with "..." (e.g., an ellipsis), all three dots are stripped. strings.TrimSuffix(originalMessage, ".") would remove exactly one trailing period, which is the likely intent.

In practice, policy violation messages rarely end with ellipses, so the risk is cosmetic only.

Suggested fix:

return fmt.Sprintf("%s. This will become a failure starting on %s",
    strings.TrimSuffix(originalMessage, "."), effectiveOnStr)

---

2. Duplicated logic across Legacy and Unified filter implementations

Severity: low · File: internal/evaluator/filters.go · Lines: ~1000–1008, ~1125–1133

The same 7-line block (inner if !isResultEffective → create enhancedResult → append) is copy-pasted into both LegacyPostEvaluationFilter.CategorizeResults and UnifiedPostEvaluationFilter.CategorizeResults. If this logic changes in the future (e.g., message format update, additional guards), both sites must be updated in lockstep.

Consider extracting a small helper like:

func appendWithEnhancement(result Result, effectiveTime time.Time, warnings *[]Result) {
    if !isResultEffective(result, effectiveTime) {
        result.Message = formatFutureEnforcementMessage(result)
    }
    *warnings = append(*warnings, result)
}

This would eliminate the duplication and make future changes single-site.

---

3. Edge case: severity: warning + future effective_on produces misleading message

Severity: low · File: internal/evaluator/filters.go · Lines: ~1000, ~1125

When a deny result carries both severity: warning (demotes it to a warning by annotation) and a future effective_on date, the outer condition getSeverity(result) == severityWarning || !isResultEffective(result, effectiveTime) is satisfied by both disjuncts. The inner if !isResultEffective(...) then fires, appending "This will become a failure starting on ." However, after that date passes, the severity: warning annotation still keeps the result as a warning — it never actually becomes a failure. The message is technically misleading.

This requires contradictory rule configuration (severity: warning plus a future enforcement date is unusual), so real-world impact is negligible. No fix needed now, but worth a comment in the code if the team agrees.

---

Verification

• ✅ The formatFutureEnforcementMessage helper is thoroughly unit-tested (6 cases: valid date, missing metadata, non-string value, empty string, nil metadata, trailing period)
• ✅ The existing TestConftestEvaluatorEvaluateSeverity integration test correctly updated expected output
• ✅ isResultEffective returns true for unparseable dates, preventing the enhancement branch from executing with invalid date strings — good implicit validation
• ✅ No cross-repo contract changes — Result.Message is user-facing text, not an API boundary
• ✅ No security concerns
• ⚠ No test coverage for the UnifiedPostEvaluationFilter.CategorizeResults path (the duplicated logic), though the implementation is identical

[fullsend-ai-review]

[low] correctness

strings.TrimRight(originalMessage, ".") removes all trailing characters in the cutset, not just a single trailing period. If a message ends with "..." (ellipsis), all three dots are stripped. strings.TrimSuffix would be more precise.

Suggested fix: Replace strings.TrimRight(originalMessage, ".") with strings.TrimSuffix(originalMessage, ".") to remove exactly one trailing period.

[fullsend-ai-review]

[low] maintainability

The same 7-line enhancement block is duplicated in both LegacyPostEvaluationFilter.CategorizeResults and UnifiedPostEvaluationFilter.CategorizeResults. Changes to message format or guards require updating both sites.

Suggested fix: Extract a small helper function (e.g., appendWithEnhancement) to eliminate the duplication.

[fullsend-ai-review]

[low] correctness

When a deny result has both severity:warning AND a future effective_on, the message claims it will become a failure, but the severity annotation keeps it as a warning even after the date passes. This is misleading for contradictory rule configurations.

Suggested fix: Consider guarding the enhancement to only fire when the demotion reason is exclusively the future effective_on (i.e., getSeverity != severityWarning), or add a code comment noting this edge case.