Repository navigation
fix: review findings of 2026-10-07 (Markdown rendering, type lock, deleted topics, nosniff) - #1
Merged
Merged
Conversation
… drop javascript: URLs [*] Html::encode() before parsing turned `>` into `>`, so blockquotes became paragraphs, link titles with quotes broke, autolinks stopped working and `>` or `&` inside code blocks were encoded twice. Links and images also passed any URL through, including `javascript:`, which readers of the frontend would execute. SafeGithubMarkdown shows HTML written into the text as text (block HTML as escaped paragraph, inline tags escaped in place, entities kept), and the rendered HTML runs through HtmlPurifier, which keeps only safe URL schemes. The purifier cache lives in `@runtime/html-purifier`. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ions [*] Which version of an item is valid depends on `has_validity_period` of its type (date range or highest number). The flag could be switched at any time, silently changing what readers see and leaving versions with dates the type no longer expects, while the type of an item is locked for exactly this reason. The type now rejects the change once any of its items has a version, the form disables the checkbox with a hint, and the optional i18n migration adds the German text. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…e service [*] Downloads carry `X-Content-Type-Options: nosniff`, so browsers do not guess another type than the one detected on upload. `knowledge_get_file` read the stored file from the module filesystem directly and ignored the storage named by the file row; it now uses FileService::readStream() like the downloads, which also logs read errors in one place. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…w behaviour [*] ItemState described in review before a valid version, the code resolves a valid version first. The README named the wizard button `submit`, which is `submit-for-review`. Documents the validity period lock of types, the URL scheme filter of the Markdown rendering, the nosniff header and the new i18n migration. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…cation [*] A topic referenced only by a draft (`draft_topic_ids`) can be deleted, as the delete lock of topics checks the item assignments only. Publishing or submitting the draft then failed with "Topics is invalid.", an error on a field the editor cannot see or correct in the wizard: the select lists existing topics only. The existence of the topics is now checked when the editor picks them (step details), and the publication applies the topics that still exist. Co-Authored-By: Claude Fable 5.1 <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.
Review of the package for flaws and bugs, with scenario probes against the test suite (706 tests before, 707 after; all green on PHP 8.4).
Fixed
Html::encode()before parsing turned>into>, so blockquotes became paragraphs, link titles broke, autolinks stopped working and>/&inside code blocks were encoded twice. Links and images also passedjavascript:URLs through to readers of the frontend. NewSafeGithubMarkdownshows HTML as text, the output runs through HtmlPurifier (safe URL schemes only).has_validity_periodis locked once items of the type have versions. The flag decides which version of an item is valid (date range vs. highest number); switching it silently changed what readers see. The form disables the checkbox with a hint, the optional i18n migrationm261007_120000adds the German text.X-Content-Type-Options: nosniff;knowledge_get_filereads throughFileService::readStream()and honours the storage named by the file row like the downloads.ItemStateprecedence, wizard buttonsubmit-for-review, new behaviour in the README.Checked and found correct
Correction and withdrawal flows (extension of the previous version, correction of historical versions, types without validity period), four-eyes checks in the model, route permission prefix resolution (
knowledgedoes not grantknowledge-library_*), upload hardening (base name, MIME sniffing, size), output encoding of the views, MCP protocol handling.Known limitation (not changed)
Withdrawing the version in force while an upcoming version exists leaves a gap that no new version can fill: a new version must start after the upcoming one, and withdrawn versions cannot be corrected. The upcoming version has to be withdrawn as well.
🤖 Generated with Claude Code