-
Notifications
You must be signed in to change notification settings - Fork 3k
fix(triage): render the verify report as sanitized markdown, not an escaped pre dump #8147
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
35066bc
47c4867
a67c6a0
09375d0
f614380
289df62
b0ae340
7175d35
33951c3
c30b53f
cea325f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -3325,6 +3325,240 @@ jobs: | |||||||||||||||
| printf '%s%s\n' "$esc" "$truncated" | ||||||||||||||||
| printf '</code></pre>\n\n</details>\n\n' | ||||||||||||||||
| } | ||||||||||||||||
| # Render report.md as MARKDOWN instead of an escaped <pre> dump. | ||||||||||||||||
| # The report is a curated bilingual document (tables, nested | ||||||||||||||||
| # <details>, headings); the pre/code embedding displayed it as a | ||||||||||||||||
| # wall of raw source (#8140's comment was the exhibit). The section | ||||||||||||||||
| # is wrapped in a collapsed <details> so it still costs one line in | ||||||||||||||||
| # the conversation but renders as real markdown when opened. | ||||||||||||||||
| # | ||||||||||||||||
| # A node sanitizer (not sed) does the escaping so it can tell code | ||||||||||||||||
| # regions apart from prose. CommonMark does NOT decode entities in | ||||||||||||||||
| # code spans or fenced blocks — they render literally there — so | ||||||||||||||||
| # escaping & < > @ unconditionally showed the reader &&, | ||||||||||||||||
| # <T>, @pkg inside the very commands, generic types, and | ||||||||||||||||
| # scoped-package paths a verification report is read to copy (the | ||||||||||||||||
| # #8140 symptom, relocated into code). The security floor now rests | ||||||||||||||||
| # on four line-independent guarantees: | ||||||||||||||||
| # 1. in PROSE every < is escaped, then only the structural tags | ||||||||||||||||
| # the report uses as raw HTML (details/summary) are un-escaped | ||||||||||||||||
| # back to live tags — no other tag can form in ordinary | ||||||||||||||||
| # prose, so <img>/<script>/onerror never render there. A | ||||||||||||||||
| # line-level inHtml flag tracks raw-HTML blocks opened by | ||||||||||||||||
| # details/summary (at any indent, so list-nested folds | ||||||||||||||||
| # enter the state too): inside one, neither fence lines | ||||||||||||||||
| # NOR code spans are code — CommonMark/GitHub reads both | ||||||||||||||||
| # as literal text in a raw-HTML block — so the sanitizer | ||||||||||||||||
| # prose-escapes the whole line instead of splitting it | ||||||||||||||||
| # through proseLine, closing the divergence where GitHub | ||||||||||||||||
| # saw live HTML the sanitizer treated as inert code. | ||||||||||||||||
| # & and > are left alone in prose (a decoded entity | ||||||||||||||||
| # yields text, never markup, and an unescaped > cannot | ||||||||||||||||
| # open a tag once < is escaped), which keeps &&, ->, and | ||||||||||||||||
| # blockquotes readable. Code-span parity is a paragraph | ||||||||||||||||
| # property in CommonMark but a line property here, so a prose | ||||||||||||||||
| # line carrying an unmatched backtick run makes the rest of the | ||||||||||||||||
| # paragraph's code/prose split unknowable; the scanner fails | ||||||||||||||||
| # closed and prose-escapes every line until the next blank line | ||||||||||||||||
| # (over-escaping is the safe direction — a multi-line code span | ||||||||||||||||
| # shows its entities literally, inert not live). Outside HTML | ||||||||||||||||
| # blocks, inside code spans/fences < & @ are left alone: <img> | ||||||||||||||||
| # and @mentions are inert under a code/pre ancestor, and | ||||||||||||||||
| # escaping them there only mangles the rendered text; | ||||||||||||||||
| # 2. the comment-open token is broken EVERYWHERE, prose and code | ||||||||||||||||
| # alike (<!-- becomes an escaped no-op, the autofix-proven | ||||||||||||||||
| # neutralizer). The break must stay global: the upsert greps | ||||||||||||||||
| # the RAW body for the running marker and a fence prints | ||||||||||||||||
| # verbatim, so a forged marker inside a fence must not survive; | ||||||||||||||||
| # 3. in prose @ gains a zero-width space (@​) — renders | ||||||||||||||||
| # identically, never fires a mention (GitHub decodes @ | ||||||||||||||||
| # back to @ before the mention filter runs, so the entity | ||||||||||||||||
| # alone was inert; the ZWSP breaks the mention token). A | ||||||||||||||||
| # mention cannot fire under a code/pre ancestor, so code | ||||||||||||||||
| # keeps a literal @; | ||||||||||||||||
| # 4. <details> folds are balanced over PROSE only — a </details> | ||||||||||||||||
| # quoted in a code span/fence is inert text and is no longer | ||||||||||||||||
| # counted — surplus closers are dropped (they would otherwise | ||||||||||||||||
| # close the wrapping fold early) and unclosed opens are closed | ||||||||||||||||
| # at the end, so a malformed report can neither swallow the | ||||||||||||||||
| # footer nor escape its wrapper. The flat scanner diverges | ||||||||||||||||
| # from GitHub's container-aware parser whenever a fence it | ||||||||||||||||
| # holds open cannot exist in GitHub's view, so two signals | ||||||||||||||||
| # exit non-zero (emit_report then degrades to the escaped | ||||||||||||||||
| # fallback rather than guessing): a fence still open at EOF | ||||||||||||||||
| # (GitHub closes a list-nested fence at the container's end, | ||||||||||||||||
| # not at EOF), and a non-blank line that dedents below the | ||||||||||||||||
| # fence opener's indent while it is open — the container | ||||||||||||||||
| # boundary moved, so GitHub already closed the fence and a | ||||||||||||||||
| # balancing closer later in the file would otherwise let the | ||||||||||||||||
| # scanner pass unescaped prose through escCode. | ||||||||||||||||
| # Accepted tradeoff (named, not accidental): rendering promotes the | ||||||||||||||||
| # report from inert text to parsed markdown, so [links](…) and | ||||||||||||||||
| #  now render live where emit_block showed them literal. | ||||||||||||||||
| # report.md is agent output from a sandbox that ran PR code; images | ||||||||||||||||
| # are camo-proxied (mostly noise) and a phishing link under the bot | ||||||||||||||||
| # identity is the residual surface. That surface exists only while | ||||||||||||||||
| # the four guarantees hold, so the fail-closed bails above are what | ||||||||||||||||
| # keep deferring link defusing defensible; defusing link targets is | ||||||||||||||||
| # a deliberate follow-up, not done here. Two safe-but-ugly display | ||||||||||||||||
| # costs are named, not bugs: a code span that opens on one line and | ||||||||||||||||
| # closes on the next is prose-escaped (the reader sees <T> | ||||||||||||||||
| # inside it — the #8140 symptom relocated but inert), and escCode | ||||||||||||||||
| # breaks <!-- to <!\-\- which renders LITERALLY in a fence | ||||||||||||||||
| # (entities do not decode there), so a fenced comment example shows | ||||||||||||||||
| # as <!\-\-. | ||||||||||||||||
| # An OVERSIZED report falls back to emit_block wholesale (cut | ||||||||||||||||
| # markdown dangles fences/folds), as does any sanitizer failure. | ||||||||||||||||
| emit_report() { | ||||||||||||||||
| local file="$1" max="$2" | ||||||||||||||||
| [ -n "$file" ] && [ -f "$file" ] || return 0 | ||||||||||||||||
| if [ "$(wc -c < "$file")" -gt "$max" ]; then | ||||||||||||||||
| echo "::warning::emit_report fell back to escaped embedding (report exceeds size cap) for $file" >&2 | ||||||||||||||||
| emit_block 'Verification report (report.md, truncated)' "$file" "$max" | ||||||||||||||||
| return 0 | ||||||||||||||||
| fi | ||||||||||||||||
| local san_file | ||||||||||||||||
| san_file="$(mktemp)" | ||||||||||||||||
| # Node is present on every runner that runs this job (emit_block | ||||||||||||||||
| # already shells out to it for the UTF-8-safe cut). The script | ||||||||||||||||
| # rides inside a single-quoted shell string, so it uses double | ||||||||||||||||
| # quotes throughout and carries no single quotes of its own. | ||||||||||||||||
| local node_status=0 | ||||||||||||||||
| node -e ' | ||||||||||||||||
| const fs = require("node:fs"); | ||||||||||||||||
| const text = fs.readFileSync(process.argv[1], "utf8").replace(/[\u0000\u0001]/g, ""); | ||||||||||||||||
| function escProse(s) { | ||||||||||||||||
| // Allowlisted tags are stashed behind a \u0001 sentinel (stripped | ||||||||||||||||
| // from input) BEFORE the < pass and restored from it, so a literal | ||||||||||||||||
| // <details> the author typed stays escaped text instead of being | ||||||||||||||||
| // promoted back to a live tag by the un-escape step. | ||||||||||||||||
| return s | ||||||||||||||||
| .replace(/<(\/?)(details|summary)>/g, "\u0001$1$2>") | ||||||||||||||||
| .replace(/</g, "<") | ||||||||||||||||
| .replace(/\u0001(\/?)(details|summary)>/g, "<$1$2>") | ||||||||||||||||
| .replace(/<!--/g, "<!\\-\\-") | ||||||||||||||||
| .replace(/@/g, "@​"); | ||||||||||||||||
| } | ||||||||||||||||
| function escCode(s) { | ||||||||||||||||
| return s.replace(/<!--/g, "<!\\-\\-"); | ||||||||||||||||
| } | ||||||||||||||||
| let depth = 0; | ||||||||||||||||
| function balance(s) { | ||||||||||||||||
| return s.replace(/<\/?details>/g, function (t) { | ||||||||||||||||
| if (t === "<details>") { depth += 1; return t; } | ||||||||||||||||
| if (depth > 0) { depth -= 1; return t; } | ||||||||||||||||
| return ""; | ||||||||||||||||
| }); | ||||||||||||||||
| } | ||||||||||||||||
| function proseLine(line) { | ||||||||||||||||
| let out = ""; | ||||||||||||||||
| let buf = ""; | ||||||||||||||||
| let i = 0; | ||||||||||||||||
| const flush = function () { | ||||||||||||||||
| if (buf) { out += balance(escProse(buf)); buf = ""; } | ||||||||||||||||
| }; | ||||||||||||||||
| while (i < line.length) { | ||||||||||||||||
| if (line[i] !== "`") { buf += line[i]; i += 1; continue; } | ||||||||||||||||
| // A backslash-escaped backtick does not OPEN a code span (the | ||||||||||||||||
| // CommonMark escape rule consumes it first) but still CLOSES one | ||||||||||||||||
| // — an asymmetry a line-scoped scanner cannot model. Fail closed | ||||||||||||||||
| // like an unmatched run so the whole line is prose-escaped. | ||||||||||||||||
| let bs = 0, b = i - 1; | ||||||||||||||||
| while (b >= 0 && line[b] === "\\") { bs += 1; b -= 1; } | ||||||||||||||||
| if (bs % 2 === 1) { unmatched = true; buf += line[i]; i += 1; continue; } | ||||||||||||||||
| let j = i; | ||||||||||||||||
| while (j < line.length && line[j] === "`") j += 1; | ||||||||||||||||
| const run = j - i; | ||||||||||||||||
| let k = j, cs = -1, ce = -1; | ||||||||||||||||
| while (k < line.length) { | ||||||||||||||||
| if (line[k] === "`") { | ||||||||||||||||
| let m = k; | ||||||||||||||||
| while (m < line.length && line[m] === "`") m += 1; | ||||||||||||||||
| if (m - k === run) { cs = k; ce = m; break; } | ||||||||||||||||
| k = m; | ||||||||||||||||
| } else k += 1; | ||||||||||||||||
| } | ||||||||||||||||
| if (cs === -1) { unmatched = true; buf += line.slice(i, j); i = j; } | ||||||||||||||||
| else { flush(); out += escCode(line.slice(i, ce)); i = ce; } | ||||||||||||||||
| } | ||||||||||||||||
| flush(); | ||||||||||||||||
| return out; | ||||||||||||||||
| } | ||||||||||||||||
| const lines = text.split("\n"); | ||||||||||||||||
| const out = []; | ||||||||||||||||
| let inFence = false, fc = "", fl = 0, fi = 0, inHtml = false; | ||||||||||||||||
| let spanUnknown = false, unmatched = false; | ||||||||||||||||
| for (const line of lines) { | ||||||||||||||||
| if (!inFence) { | ||||||||||||||||
| const blank = /^\s*$/.test(line); | ||||||||||||||||
| if (inHtml && blank) inHtml = false; | ||||||||||||||||
| if (blank) spanUnknown = false; | ||||||||||||||||
| const m = inHtml ? null : line.match(/^ {0,3}(`{3,}|~{3,})(.*)$/); | ||||||||||||||||
| if (m && !(m[1][0] === "`" && m[2].indexOf("`") !== -1)) { | ||||||||||||||||
| inFence = true; fc = m[1][0]; fl = m[1].length; fi = line.match(/^ */)[0].length; | ||||||||||||||||
| out.push(escCode(line)); | ||||||||||||||||
| continue; | ||||||||||||||||
| } | ||||||||||||||||
| let rendered; | ||||||||||||||||
| if (inHtml || spanUnknown) { | ||||||||||||||||
| rendered = balance(escProse(line)); | ||||||||||||||||
| } else { | ||||||||||||||||
| unmatched = false; | ||||||||||||||||
| rendered = proseLine(line); | ||||||||||||||||
| if (unmatched) { spanUnknown = true; rendered = balance(escProse(line)); } | ||||||||||||||||
|
Comment on lines
+3506
to
+3508
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion]
Suggested change
中文说明当一行散文同时包含 — qwen3.8-max-preview via Qwen Code /review |
||||||||||||||||
| } | ||||||||||||||||
| if (/^\s*<\/?(details|summary)\b/.test(rendered)) inHtml = true; | ||||||||||||||||
| out.push(rendered); | ||||||||||||||||
| } else { | ||||||||||||||||
| if (/\S/.test(line) && line.match(/^ */)[0].length < fi) process.exit(3); | ||||||||||||||||
| const cm = line.match(/^ {0,3}(`{3,}|~{3,})[ \t]*$/); | ||||||||||||||||
| if (cm && cm[1][0] === fc && cm[1].length >= fl) inFence = false; | ||||||||||||||||
|
Comment on lines
+3514
to
+3515
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] The sanitizer's code-region parser is exercised only through backtick fences and single-backtick code spans; its tilde-fence branch and its multi-backtick span run-length matching have no test coverage, so a regression in either branch ships green. — Failure scenario: every fixture uses 中文说明净化器的代码区域解析器只通过反引号围栏和单反引号代码跨度被测试;其波浪号围栏分支与多反引号跨度的 run 长度匹配没有测试覆盖,因此这两个分支的回归会绿灯通过。触发场景:所有 fixture 都用 — qwen3.8-max-preview via Qwen Code /review |
||||||||||||||||
| out.push(escCode(line)); | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
| if (inFence) process.exit(3); | ||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Critical] The EOF-open-fence guard ( Failure scenario: a report containing Track the fence opener's indent and treat a non-blank line at a lower indent as a divergence signal (exit non-zero to trigger the escaped fallback):
Suggested change
(where 中文说明EOF 处开放围栏的守卫( 触发场景:报告包含 修复:跟踪围栏打开者的缩进,将缩进更低的非空行视为分歧信号(以非零退出触发转义回退)。 — qwen3.8-max-preview via Qwen Code /review |
||||||||||||||||
| let result = out.join("\n"); | ||||||||||||||||
| if (depth > 0) { | ||||||||||||||||
| if (result && !result.endsWith("\n")) result += "\n"; | ||||||||||||||||
| result += "</details>\n".repeat(depth); | ||||||||||||||||
| } | ||||||||||||||||
|
qwen-code-dev-bot marked this conversation as resolved.
|
||||||||||||||||
| process.stdout.write(result); | ||||||||||||||||
| ' "$file" > "$san_file" || node_status=$? | ||||||||||||||||
| if [ "$node_status" -ne 0 ]; then | ||||||||||||||||
| if [ "$node_status" -eq 3 ]; then | ||||||||||||||||
| echo "::warning::emit_report fell back to escaped embedding (report ended inside an open code fence) for $file" >&2 | ||||||||||||||||
|
Comment on lines
+3528
to
+3529
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] Exit code 3 conflates two distinct sanitizer bailouts (EOF-open-fence and dedent-below-fence-indent) under a warning that describes only the EOF case. — Failure scenario: a list-nested fence whose content dedents below the opener indent triggers the dedent guard, but the log says "report ended inside an open code fence" — an oncall engineer would look for an unclosed fence at EOF, not find one, and waste time or dismiss the warning. Use a distinct exit code (e.g. 中文说明退出码 3 将两种不同的净化器退出(EOF 处开放围栏和缩进低于围栏打开者)合并为一条仅描述 EOF 情况的警告。触发场景:列表嵌套围栏的内容缩进低于打开者时触发缩进守卫,但日志显示"报告在开放代码围栏内结束"——值班工程师会寻找 EOF 处未闭合的围栏,找不到后浪费时间或忽略该警告。建议对缩进守卫使用独立退出码(如 — qwen3.8-max-preview via Qwen Code /review |
||||||||||||||||
| else | ||||||||||||||||
| echo "::warning::emit_report fell back to escaped embedding (sanitize failed) for $file" >&2 | ||||||||||||||||
| fi | ||||||||||||||||
| rm -f "$san_file" | ||||||||||||||||
| emit_block 'Verification report (report.md, escaped fallback)' "$file" "$max" | ||||||||||||||||
| return 0 | ||||||||||||||||
| fi | ||||||||||||||||
| # Wrap in a collapsed <details> (one-line footprint, markdown when | ||||||||||||||||
| # opened) and bound the WHOLE emitted section — wrapper, balanced | ||||||||||||||||
| # report, appended closers — so the cap is a true bound on what | ||||||||||||||||
| # lands in the comment. The old normal path skipped the header | ||||||||||||||||
| # bytes the deficit branch did budget; folding the wrapper into | ||||||||||||||||
| # the measured output removes that asymmetry and the separate | ||||||||||||||||
| # fold-closer gate (closers now land in san_file before this gate). | ||||||||||||||||
| local out_file | ||||||||||||||||
| out_file="$(mktemp)" | ||||||||||||||||
| { | ||||||||||||||||
| printf '<details>\n<summary>Verification report</summary>\n\n' | ||||||||||||||||
| cat "$san_file" | ||||||||||||||||
| printf '\n</details>\n' | ||||||||||||||||
| } > "$out_file" | ||||||||||||||||
| rm -f "$san_file" | ||||||||||||||||
| if [ "$(wc -c < "$out_file")" -gt "$max" ]; then | ||||||||||||||||
| echo "::warning::emit_report fell back to escaped embedding (sanitized output exceeds size cap) for $file" >&2 | ||||||||||||||||
| rm -f "$out_file" | ||||||||||||||||
| emit_block 'Verification report (report.md, truncated)' "$file" "$max" | ||||||||||||||||
| return 0 | ||||||||||||||||
| fi | ||||||||||||||||
| cat "$out_file" | ||||||||||||||||
| rm -f "$out_file" | ||||||||||||||||
| printf '\n' | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| # Host the agent's evidence images (if any) on a per-PR branch | ||||||||||||||||
| # (pr-assets/<N>-verify, matching hand-run convention) and build | ||||||||||||||||
|
|
@@ -3695,7 +3929,7 @@ jobs: | |||||||||||||||
| if [ -n "${MISSING_REPORT_NOTE:-}" ]; then | ||||||||||||||||
| printf '%s\n\n' "$MISSING_REPORT_NOTE" | ||||||||||||||||
| fi | ||||||||||||||||
| emit_block 'Verification report (report.md)' "$REPORT" 45000 | ||||||||||||||||
| emit_report "$REPORT" 45000 | ||||||||||||||||
| if [ -n "$EVIDENCE_SECTION" ]; then | ||||||||||||||||
| printf '%s' "$EVIDENCE_SECTION" | ||||||||||||||||
| fi | ||||||||||||||||
|
|
||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.