Fix HTML-escaping of check error messages - #222
Merged
Conversation
CheckError rendered its localized error message with html/template, which contextually escapes <, >, &, and " in template data for safe HTML embedding. That's the wrong behavior for a library whose primary consumer is a plain-text error or JSON API response body: any error whose template data contains those characters (a value echoed back by a custom checker via NewCheckErrorWithData, which is a documented extension point) comes out corrupted, e.g. `<script>&"'` renders as `<script>&"'`. No built-in checker triggers this today -- their template data is either numeric (gte/lte's "n") or a developer-chosen name (eq-field's field name, hash's algorithm), not raw user input -- so this was a latent bug for the built-ins, but a live one for any custom checker using this exact pattern. Switch to text/template, which performs no such escaping. Fixes #197 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FLdVmP5daHiknrTW4Geh2i
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #222 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 52 52
Lines 896 896
=========================================
Hits 896 896 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
cinar
enabled auto-merge (squash)
September 5, 2026 16:32
…aping # Conflicts: # CHANGELOG.md
…aping # Conflicts: # CHANGELOG.md
cinar
added a commit
that referenced
this pull request
Sep 5, 2026
## Problem Every PR added its own bullet to `CHANGELOG.md`'s `## [Unreleased]` section, always inserting at (or near) the same spot. With PRs landing close together, that made this file a near-guaranteed merge conflict on every concurrent pull request — the exact problem `DOC.md` caused before #180 dropped it in favor of pkg.go.dev. This wasn't theoretical: it's what just happened resolving conflicts on #219–#222. Each conflict resolution round immediately conflicted again as the next PR merged, because every remaining branch's `### Fixed` entry inserted into the same place. ## Fix - Freeze the current `## [Unreleased]` section as a one-time snapshot (documenting the six P0 bugfixes plus the earlier accumulated Added/Changed/Removed entries) and add a note explaining the new policy at the top of the file. - Going forward, unreleased changes are covered by [GitHub Releases](https://github.com/cinar/checker/releases), whose notes are generated automatically from merged pull requests — no file to conflict on. `CHANGELOG.md` only gains a new entry when a version is actually tagged, added by hand from that release's generated notes (a maintainer decision, not a per-PR one). - Updated `CLAUDE.md`'s conventions section: PRs no longer touch `CHANGELOG.md`. No code changes; docs/process only. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01FLdVmP5daHiknrTW4Geh2i Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
check_error.gorenders localized error messages withhtml/template, which contextually escapes<,>,&, and"in template data. That's the wrong behavior for a validation library whose primary consumer is a plain-text or JSON API error response.Verified against
mainbefore writing this fix: no built-in checker triggers visible corruption today — the template data they pass viaNewCheckErrorWithDatais either numeric (gte/lte'sn) or a developer-chosen name (eq-field's field name,hash's algorithm), never raw user input. But a custom checker registered viaRegisterMaker— a documented extension point — that echoes a checked value back intoData(e.g. for a "must not contain X" style message) gets corrupted output:renders as
Fix
Switch the import from
html/templatetotext/template, which performs no such escaping. No other code changes needed — the two packages share the sameParse/ExecuteAPI.Testing
TestCheckErrorWithDataDoesNotHTMLEscape, asserting a message with<,>,&,",'in its template data renders unescaped.go vet,gosecclean.Fixes #197
🤖 Generated with Claude Code
https://claude.ai/code/session_01FLdVmP5daHiknrTW4Geh2i