fix(batch): prevent Markdown report injection

[yashrajp22]

Scan-controlled names, findings and ledger fields could add rows, headings or HTML to batch Markdown reports. Render those values as literal text and use safe code-span delimiters while preserving the original JSON data.

Validation: Rendered structure, backticks, pipes, HTML and literal-value regression tests.

[rng1995]

[SkillSpector Review]

Hi @yashrajp22, thank you for bringing the batch scan's Markdown report up to the same standard as the core report, with tests that check both the structure and the literal values!

Value and readiness: The batch report has the same weakness as the core report, and here one skill can hide the results of the others. On main, a skill whose manifest name is evil\n<!-- and whose row sorts first breaks its table row and opens an HTML comment. That hides every later row and the HIGH/CRITICAL details for the other skills in the run. With this change, the name, language, severity, rule ID, explanation, remediation, location and ledger fields are escaped or put in code spans, and the same input leaves the table and headings intact. JSON output is unchanged. One existing test needs updating before merge.

Material findings

1. [Blocker] contrib/batch_scan/reports.py:517: ledger reason codes now render as read\_error. That breaks contrib/batch_scan/tests/test_inspection_reporting.py:65 (assert "read_error" in rendered for _format_markdown, in test_batch_formats_preserve_every_child_ledger_exception), which passes on main. CI does not collect contrib/ tests (make test-ci runs tests/ only), so the green run on bf97e339 did not catch it. Expected fix: render the Markdown before asserting, as the new tests/test_batch_scan_reports.py does, or keep reason codes in code spans.
2. [Non-blocking] contrib/batch_scan/reports.py:378-397 duplicates the helpers that #735 adds to src/skillspector/nodes/report.py, and the copies already differ. This _markdown_plain_text does not call _clean_text, so an ANSI sequence such as \x1b[2J leaves [2J behind, while the core report strips the whole sequence. After #735 lands, importing one shared helper would keep the two reports consistent.
3. [Non-blocking] ~ is not escaped (same as in #735). In this layout a field can only strike through its own text, because no two scan-controlled prose fields share an inline span outside bold. Adding ~ to the shared escaper (see the #735 review) would still keep the two reports consistent.

PIC tradeoffs: Same raw-readability tradeoff as #735. The .md source now has backslash escapes in prose and identifiers (read\_error, could not read\.), while the rendered report is unchanged. This PR does not conflict with #735 (different files). I recommend landing #735 first, then this PR, ideally switched to #735's helpers.

Verification and gaps: I read the diff against e9f74277 (the patch is identical against the current merge base fdb040b1) and every Markdown output point in _format_markdown. The only values still written unescaped are scanner-computed numbers (counts, score, confidence). The skill name comes from the manifest or the directory name (runner.entry_from_result), so it is attacker-controlled. I built the main and PR versions of the risk table for the evil\n<!-- name by hand and parsed them with markdown-it-py (CommonMark + tables). On main, an html_block swallows the later row and section. With this PR, all rows and headings stay. The new tests cover 10 fields x 6 payloads, plus literal round-trips and JSON preservation. CI: the current head is a github-actions merge of main (bringing in #727, #731 and #722) with action_required and no runs. Author commit bf97e339 has the same patch-id as this head and passed all 6 checks, and no later main commit touches these files. Tests not executed locally per policy.

---

Decision: Changes Requested (reviewed head 69c49babfafefcd8abea17521621a8118ae9d016; the PR's own diff is identical to 7c63f343, newer commits are automated merges of main)

[rng1995]

[SkillSpector Review]

Hi @yashrajp22, thank you for the quick turnaround on the batch Markdown escaping, and for adding the tilde and ANSI cases to the tests!

Value and readiness: This now delivers what it set out to do. Every scan-controlled field in the batch Markdown report is escaped or placed in a code span, so one hostile skill name or finding can no longer break the table or hide the other skills' results. The broken contrib ledger test is fixed, and the helpers now behave exactly like #735's. This is ready for maintainer review. Merge should wait for the test-unit job, which is still running on this head.

Previous findings:

• Resolved (Blocker: escaped reason codes broke the contrib ledger test). contrib/batch_scan/tests/test_inspection_reporting.py:66-71 now renders the Markdown with markdown-it before asserting. - **read\_error** ... renders as <strong>read_error</strong>, so both reason-code assertions pass and the escaping stays.
• Partly resolved (Non-blocking: duplicated helpers that differed from #735). _markdown_plain_text now calls the core _clean_text (contrib/batch_scan/reports.py:33, :381), so \x1b[2J is removed completely instead of leaving [2J behind. The three helpers at :379-399 now match #735's _markdown_plain_text, _markdown_cell and _markdown_code (src/skillspector/nodes/report.py:1411, :1417, :1423 at #735's head b469dd5c) line for line, apart from docstrings. They are still copies; see finding 1.
• Resolved (Non-blocking: ~ was not escaped). ~ is now in the escape class at reports.py:389. The literal-text test enables strikethrough and fails on any s_open token, and it includes a ~~hidden~~ payload (tests/test_batch_scan_reports.py:40, :77, :99).

Material findings

1. [Non-blocking] contrib/batch_scan/reports.py:379-399: yes, this file should reuse #735's helpers once #735 lands. #735 is now approved and adds the same three functions to skillspector.nodes.report. This file already imports _clean_text from that module. Importing _markdown_plain_text, _markdown_cell (as _markdown_text) and _markdown_code instead of keeping the copies deletes about 20 lines, and any later escaping fix would land once for both reports. A follow-up PR is fine, so the stack is not held up.

PIC tradeoffs: Unchanged from the last round. The .md source now contains backslash escapes (read\_error, could not read\.), while the rendered report is unchanged.

Verification and gaps:

• New author commits since my last review: 41b0660e (fix) and 01516f68 (formatting). Both are by the PR author and signed off. The other new commits are seven github-actions merges of main (58c52c84 through fd0095fa). None of them touch this PR's three files, so the PR's own diff is exactly the previously reviewed diff plus 41b0660e and 01516f68.
• I rechecked every Markdown output point in _format_markdown. Only scanner-computed numbers (counts, score, confidence) are written unescaped.
• I parsed hand-built output with markdown-it-py 4.2 (library only). The new tests would fail on the old head: ~~hidden~~ in a prose field produced s_open, and the ANSI case left [2J.
• markdown-it-py is already installed through rich, so the new import in the contrib test adds no dependency.
• ruff check and ruff format --check on src/ and tests/ pass on the full tree at this head. CI does not collect contrib/ tests, so I verified the fixed contrib test by trace only.
• git merge-tree is clean against main and against #735.
• CI on fd0095fa: lint, DCO, changes and OpenCode TypeScript tests pass, and docker-smoke is skipped. test-unit was still running at 17:45Z. It started at 16:29Z, while the same job took about 54 minutes on main at this base. Nothing in this change (pure Markdown helpers and their tests) can block, so if the job hangs or times out, a rerun is the first step.
• Landing order: this PR and #735 are independent (no shared files), and either can land first. The batch stack is main <- #733 <- #734 <- #743, so this PR lands first.
• I did not run the tests locally, per policy.

---

Decision: Approved (reviewed head fd0095fa1a6db95c2624fa47340a08906e3cb8e2)

[yashrajp22]

Thanks for the suggestion! The shared-helper cleanup is now included in #735 at ea35f51. The batch report reuses the core helpers, and the combined core and batch checks pass.