test: pin the sanitizer allowlist and resource-policy boundaries #49

Merged

james.maes merged 5 commits from test/pin-untrusted-input-boundaries into main 2026-11-11 00:32:29 +00:00

Conversation 1 Commits 5 Files Changed 12

+1149 -21

james.maes commented 2026-11-10 22:49:15 +00:00

Owner

Copy link

Copy source

Closes #48.

Problem

A mutation run on main showed that the sanitizer allowlists and several resource-policy and validation checks could be weakened without any test failing.

Change

• SanitizerAllowlistTests: exact, independent pins for allowed elements, attributes and link schemes (every other standard element and about 110 other attributes are checked to be removed).
• DocumentResourcePolicyBoundaryTests: requests built from the document's own URI and varied in one component at a time (scheme, port, credentials, host), exact served and opened file types, each forbidden character, heading fragment spelling.
• UntrustedInputValidationTests: theme colours and labels, metadata escaping, source-marker unpredictability, raw heading levels, per-property settings repair, schema approval, exact size limits.
• Fuzz corpus now generates the media, form and metadata elements its oracle forbids; the oracle also rejects target, ping, name and autofocus.
• WindowsFileNames (Core): one written-out rule for reserved device names and forbidden characters, replacing two different lists. Behaviour change: device names are recognised in every spelling Windows treats as a device, COM0, LPT0, CONIN$ and CONOUT$ are included, and a double quote is refused in resource paths. Threat model updated.

Verification (run)

• Local gate script: all pass. Core tests 1781 passed, line coverage 100%.
• Mutation check: 54 mutants in the touched areas (allowlists, schemes, origin checks, file types, device names, theme validation, settings repair, frontmatter escaping, source marker, size caps): 53 killed. The one survivor (Math.Max(0, count) in DocumentFind.SetRenderedCount) is equivalent, because Total already takes the maximum with a non-negative count.
• Not pinned, deliberately: the 16 Mi-character limits at exactly the limit (a 32 MB string per test for an off-by-one with no security effect).

Not run: the FlaUI desktop suite. This change does not touch the WinUI project.

AI assistance

Written with Claude Fable 5.2. An independent read-only review of this diff is recorded below before merge.

🤖 Generated with Claude Code

Closes #48. ## Problem A mutation run on `main` showed that the sanitizer allowlists and several resource-policy and validation checks could be weakened without any test failing. ## Change - `SanitizerAllowlistTests`: exact, independent pins for allowed elements, attributes and link schemes (every other standard element and about 110 other attributes are checked to be removed). - `DocumentResourcePolicyBoundaryTests`: requests built from the document's own URI and varied in one component at a time (scheme, port, credentials, host), exact served and opened file types, each forbidden character, heading fragment spelling. - `UntrustedInputValidationTests`: theme colours and labels, metadata escaping, source-marker unpredictability, raw heading levels, per-property settings repair, schema approval, exact size limits. - Fuzz corpus now generates the media, form and metadata elements its oracle forbids; the oracle also rejects `target`, `ping`, `name` and `autofocus`. - `WindowsFileNames` (Core): one written-out rule for reserved device names and forbidden characters, replacing two different lists. Behaviour change: device names are recognised in every spelling Windows treats as a device, `COM0`, `LPT0`, `CONIN$` and `CONOUT$` are included, and a double quote is refused in resource paths. Threat model updated. ## Verification (run) - Local gate script: all pass. Core tests 1781 passed, line coverage 100%. - Mutation check: 54 mutants in the touched areas (allowlists, schemes, origin checks, file types, device names, theme validation, settings repair, frontmatter escaping, source marker, size caps): 53 killed. The one survivor (`Math.Max(0, count)` in `DocumentFind.SetRenderedCount`) is equivalent, because `Total` already takes the maximum with a non-negative count. - Not pinned, deliberately: the 16 Mi-character limits at exactly the limit (a 32 MB string per test for an off-by-one with no security effect). Not run: the FlaUI desktop suite. This change does not touch the WinUI project. ## AI assistance Written with Claude Fable 5.2. An independent read-only review of this diff is recorded below before merge. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

james.maes added 1 commit 2026-11-10 22:49:15 +00:00

test: pin the sanitizer allowlist and resource-policy boundaries ... 8770f075ea

Widening the sanitizer's element, attribute or scheme allowlist left every
test green, and resource-policy tests rejected their inputs for unrelated
reasons, so removing a single check went unnoticed.

Add an independent exact pin for each allowlist, single-component
variations for origin, file type and forbidden characters, and killing
tests for theme colours and labels, metadata escaping, per-property
settings repair, schema approval and exact size limits. The fuzz corpus
now generates the media, form and metadata elements its oracle forbids.

Reserved Windows device names and forbidden characters become one
written-out rule, WindowsFileNames, used by the resource policy and the
theme export name. It covers every device name in any letter case, with
any extension and with spaces before the extension, plus COM0, LPT0,
CONIN$ and CONOUT$, and a double quote is refused like the other
forbidden characters.

Closes #48

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Signed-off-by: James Maes <james@kof22.com>

james.maes added 1 commit 2026-11-10 23:19:06 +00:00

test: compare the configured allowlists as sets and reject undecodable theme text ... 6ca8284f1b

The element pin missed elements it did not list and five it listed in a
context where the parser never creates them. The sanitizer's configured
elements, attributes and schemes, and the resource policy's file types,
are now compared with independent lists as sets, so any change to them
has to be made in the test as well. Table-only elements are tested inside
a table.

A theme string holding an unpaired surrogate escape made the JSON reader
throw InvalidOperationException, which import and saved-theme repair do
not handle. ReaderTheme now reports it as invalid theme JSON like every
other malformed value.

Also pin the styling-class filter, the exact path prefix and document
comparison, the reserved-name rule's negatives and the export-name
trimming; describe the reserved-name rule as the legacy Win32 rule applied
on every Windows version; and run the device spellings as two tests
instead of 960 theory rows.

Refs #48

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Signed-off-by: James Maes <james@kof22.com>

james.maes added 1 commit 2026-11-10 23:44:22 +00:00

fix(themes): reject a theme with undecodable text or a repeated property anywhere ... 419de2c263

A theme is validated for the strings this version reads, but it is kept,
renamed, edited and re-serialized as a whole document. An unpaired
surrogate escape in a property name or in a value this version ignores,
or a property name that appears twice, was accepted and then threw
InvalidOperationException or ArgumentException from import, rename, copy,
edit and saved-theme repair, none of which handle those.

ReaderTheme now walks the document once and reports either case as
invalid theme JSON, the failure every caller already handles.

Also pin the code-language class length limit and that the source
attribute is allowed only during a source-aware render.

Refs #48

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Signed-off-by: James Maes <james@kof22.com>

james.maes added 1 commit 2026-11-11 00:15:03 +00:00

fix(themes): keep renamed themes within the size limit ... 2fff66cf76

Renaming, copying and editing a theme wrote every quote mark, angle
bracket and non-ASCII character as a six-character escape, so a theme
well inside the size limit could grow past it and be refused. Those
operations now write characters as themselves.

Theme text that is not well-formed UTF-16 is refused as an invalid
theme. Tests pin that property names differing only in case, the same
name in two objects, an escaped spelling of a repeated name and an
undecodable string directly inside an array are each handled as
intended.

Refs #48

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Signed-off-by: James Maes <james@kof22.com>

james.maes added 1 commit 2026-11-11 00:27:51 +00:00

fix(themes): leave room for a kept theme to grow when rewritten ... 7d3cf921ce

JSON writes emoji, no-break spaces and private-use glyphs as
six-character escapes, so renaming a theme made of them could push it
past the import limit and be refused. The import limit still bounds
the file read; a kept theme now has its own, larger limit, so a theme
that could be imported can always be renamed, copied and edited.

Refs #48

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Signed-off-by: James Maes <james@kof22.com>

james.maes commented 2026-11-11 00:32:27 +00:00

Author

Owner

Copy link

Copy source

Adversarial review record

An independent read-only reviewer tried to break this branch over four rounds. It built scratch copies, ran its own mutation and fuzz campaigns, and never edited the branch.

Round	Head	Verdict	Outcome
1	8770f07	BLOCK	Allowlist pin was not exact; one test claim was false. Fixed in 6ca8284.
2	6ca8284	BLOCK	Undecodable text in property names or unread values, and repeated property names, still broke rename and import. Fixed in 419de2c.
3	419de2c	PASS	240,000 generated themes: only JsonException from parsing, no exception from any later operation. 102 mutants, 94 killed; four test gaps and two minor findings. Fixed in 2fff66c.
4	2fff66c	PASS	The four gaps are closed; relaxed output breaks no consumer of the theme JSON. One minor residual (some characters still expand when rewritten), fixed in 7d3cf92 by giving kept themes their own size limit.

Left as is on purpose: a stored theme that repeats a property name, or holds undecodable text in a value the app does not read, is dropped when settings load. Such a theme could be displayed before but broke rename, copy and edit. This goes in the release notes.

Known residual, tracked for the theme-management rework: copying a theme that an edit has grown to within a few characters of the stored limit is refused with an error the dialog does not yet handle.

Gates on 7d3cf92: locked restore, zero-warning build, format, core tests at 100% line coverage, adapter tests, fuzz; Gitea CI green.

This is an automated review, not a human one.

## Adversarial review record An independent read-only reviewer tried to break this branch over four rounds. It built scratch copies, ran its own mutation and fuzz campaigns, and never edited the branch. | Round | Head | Verdict | Outcome | | --- | --- | --- | --- | | 1 | 8770f07 | BLOCK | Allowlist pin was not exact; one test claim was false. Fixed in 6ca8284. | | 2 | 6ca8284 | BLOCK | Undecodable text in property names or unread values, and repeated property names, still broke rename and import. Fixed in 419de2c. | | 3 | 419de2c | PASS | 240,000 generated themes: only JsonException from parsing, no exception from any later operation. 102 mutants, 94 killed; four test gaps and two minor findings. Fixed in 2fff66c. | | 4 | 2fff66c | PASS | The four gaps are closed; relaxed output breaks no consumer of the theme JSON. One minor residual (some characters still expand when rewritten), fixed in 7d3cf92 by giving kept themes their own size limit. | Left as is on purpose: a stored theme that repeats a property name, or holds undecodable text in a value the app does not read, is dropped when settings load. Such a theme could be displayed before but broke rename, copy and edit. This goes in the release notes. Known residual, tracked for the theme-management rework: copying a theme that an edit has grown to within a few characters of the stored limit is refused with an error the dialog does not yet handle. Gates on 7d3cf92: locked restore, zero-warning build, format, core tests at 100% line coverage, adapter tests, fuzz; Gitea CI green. This is an automated review, not a human one.

james.maes merged commit a1320e489e into main 2026-11-11 00:32:29 +00:00

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

Clear labels

accessibility

Barrier affecting people with disabilities

bug

Something isn't working

deferred

Explicitly deferred work

dependencies

Pull requests that update a dependency file

documentation

Improvements or additions to documentation

duplicate

This issue or pull request already exists

enhancement

New feature or request

github_actions

Pull requests that update GitHub Actions code

good first issue

Good for newcomers

help wanted

Extra attention is needed

invalid

This doesn't seem right

javascript

Pull requests that update javascript code

quality

Testing, reliability, and maintainability

question

Further information is requested

refactor

Internal changes without intended behavior changes

release

Packaging, signing, and release delivery

wontfix

This will not be worked on

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

forge-recovery (Forge Recovery) james.maes (James Maes)

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: KofTwentyTwo/MarkdownViewr.Windows#49