diff --git a/specs/37153-ai-viewtool-escape-output/spec.md b/specs/37153-ai-viewtool-escape-output/spec.md new file mode 100644 index 00000000000..a8837f7638a --- /dev/null +++ b/specs/37153-ai-viewtool-escape-output/spec.md @@ -0,0 +1,373 @@ +# Issue Resolution Specification: dotAI: escape viewtool output by default + +**Feature Branch**: `37153-ai-viewtool-escape-output` + +**Created**: 2026-09-11 + +**Status**: Draft + +**Type**: Issue / Bug Resolution + +**Related GitHub Issue**: [dotCMS/core#37153](https://github.com/dotCMS/core/issues/37153) (public stub) with full detail in `dotCMS/private-issues#675` — part of epic [dotCMS/core#37255](https://github.com/dotCMS/core/issues/37255) (dotAI Security Hardening) + +**Input**: User description: "37153" + + + +## Problem Statement *(mandatory)* + +The dotAI Velocity viewtool (`$ai`, registered in `toolbox.xml`) hands the AI provider's reply +to templates exactly as the provider returned it. Completions (`summarize`, `raw`), text +generation (`generateText`) and image generation (`generateImage`) all return a nested JSON +structure whose string values are unescaped. dotCMS Velocity does not auto-escape `$reference` +output (no reference-insertion event handler is configured), so a template that prints any of +those strings into a page emits them as live HTML. Printing the whole object is no safer: the +JSON serializer escapes only `"`, `\` and `` survives intact. + +Model output is not trustworthy markup. It can be steered by content the model was fed +(the prompt-injection surface tracked in #37152), by a crafted prompt, or by hallucination. +When the model emits ``, the default accessor's + model-text field is exactly + `<script>alert(1)</script><img src=x onerror=alert(1)>` (the OWASP + `Encode.forHtml` output of the stub string), and no string value anywhere in the + provider-output part contains a literal `<`, `>`, `"` or `'`. +- **AC-002**: For `search.query(String)`, `search.query(String,String)`, `search.query(Map)`, + `search.related(Contentlet,String)`, `search.related(ContentMap,String)` and for + `summarize`: when the indexed `extractedText` and the contentlet `title` both contain the + same markup, the default accessor returns `extractedText` escaped, returns `title` and + every other `dotCMSResults` value identical to today's payload (markup intact), and returns + the `query` value escaped. The same holds when the query targets the `cache` index. For + `generateImage`, `originalPrompt` is escaped. +- **AC-003**: For every accessor in AC-001 and AC-002, the same call through `$ai.unsafe` + returns a payload byte-for-byte identical to what the API method returns, including the + literal `` and ``. +- **AC-004**: Default and unsafe payloads are otherwise identical: same keys at every depth, + same array lengths, same non-string values. Only string values differ, and only inside the + provider-output part and the echoed-input values, and only where they contained characters + the encoder changes. +- **AC-005**: The escaped payload still resolves through property access + (`get("openAiResponse")` → `getJSONArray("choices")` → index 0 → `getJSONObject("message")` + → `getString("content")`), and the existing success-path tests in `CompletionsToolTest`, + `SearchToolTest` and `AIViewToolTest` pass unchanged. +- **AC-006**: The API method's own return value is not mutated by the default accessor: after + a default call, the object the API returned still contains the literal markup. For + `summarize`, `raw`, `search.*` and `generateImage`, a WireMock mapping that answers the + dedicated prompt with HTTP 500 makes the default accessor return, without throwing, an + error payload whose string values (`error`, and `stackTrace` until #37154 removes it) + contain no literal `<`, `>`, `"` or `'`. `generateText` rethrows today and is only asserted + to still throw. +- **AC-007**: `release-note.md` exists in this spec directory and covers everything listed for + it under In scope. +- **AC-008**: `CompletionsAPIImpl.summarize` and `EmbeddingsAPIImpl.searchForContent` still + return the literal markup, so the REST resources, which return those objects unmodified, + are unaffected. +- **Verification method**: + - Integration tests in the existing classes, all already in `MainSuite2b`: + `-Dit.test=CompletionsToolTest#`, + `-Dit.test=SearchToolTest#`, + `-Dit.test=AIViewToolTest#`. Markup is injected through new WireMock + mappings under `dotcms-integration/src/test/resources/mappings/` that match a dedicated + prompt string and return `` in + `choices[0].message.content` (chat) and in `data[0].revised_prompt` (image), following the + existing `light-speed-stub.json` / `ganymede-moon-image-stub.json` pattern, plus one + mapping answering HTTP 500 for AC-006. For search and summarize, the markup is seeded as + `extractedText` through `EmbeddingsDTODataGen.persistEmbeddings` against a contentlet + whose `title` also contains it; one case seeds and queries the `cache` index. Each test + runs the same call through `$ai` and `$ai.unsafe` and asserts AC-001 through AC-004 on the + pair; AC-006 asserts against the API's object after the default call. + - A plain unit test under `dotCMS/src/test` for the shared escaping routine: nested + `JSONObject`, `JSONArray` of strings, arrays of objects, plain `Map` and `List` including + an immutable `Map.of(...)`, mixed non-string leaves, `null`, empty structures, strings + already containing entities, and a string containing every HTML-significant character. + - AC-008: one targeted assertion through `CompletionsAPIImpl.summarize` / + `EmbeddingsAPIImpl.searchForContent` confirming the API layer still returns unescaped text, + so the REST layer is provably untouched. + - TDD gate (Constitution Principle V): the new tests are written first, approved, and shown + failing on `main` (they fail because the default accessor returns the literal `