Skip to content

Preserve expanded details blocks in safe-output bodies #75380

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.

Sign up for GitHub

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

Merged
pelikhan merged 4 commits into from
Nov 6, 2026
+63 −9
Merged

Preserve expanded details blocks in safe-output bodies #75380

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f04b1af
Initial plan
Copilot Nov 6, 2026
2464e80
Fix expanded details blocks in safe-output reports
Copilot Nov 6, 2026
2655d49
Merge remote-tracking branch 'origin/main' into copilot/fix-details-o…
Copilot Nov 6, 2026
56c1621
Clarify details open safe-output semantics
Copilot Nov 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fix-details-open-safe-output.md
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions actions/setup/js/report_ast.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ const { createExpirationLine } = require("./ephemerals.cjs");
* {type: "list", ordered: boolean, items: ReportBlock[][]} |
* {type: "quote", children: ReportBlock[]} |
* {type: "alert", kind: string, children: ReportBlock[]} |
* {type: "details", summary: string | ReportInline[], children: ReportBlock[]} |
* {type: "details", open?: boolean, summary: string | ReportInline[], children: ReportBlock[]} |
* {type: "table", headers: ReportInline[][], rows: ReportInline[][][]} |
* {type: "task", checked: boolean, children: ReportInline[]} |
* {type: "separator"} |
Expand Down Expand Up @@ -402,7 +402,7 @@ function normalizeBlocks(nodes, depth) {
if (node.type === "details") {
const summary = typeof node.summary === "string" ? normalizeReportText(node.summary).replace(/\s+/g, " ").trim() : normalizeInlines(node.summary, depth + 1);
if (!summary.length) throw new TypeError("Report details must have a summary");
result.push({ type: "details", summary, children });
result.push({ type: "details", ...(node.open === true ? { open: true } : {}), summary, children });
} else if (node.type === "alert") result.push({ type: "alert", kind: node.kind, children });
else result.push({ type: "quote", children });
break;
Expand Down Expand Up @@ -668,7 +668,7 @@ function renderBlocks(nodes, compactExpiration = false) {
case "alert":
return (node.type === "alert" ? `[!${node.kind}]\n` : "") + renderBlocks(node.children, node.type === "quote");
case "details":
return `<details>\n<summary>${typeof node.summary === "string" ? escapeSummaryText(node.summary) : renderInlines(node.summary, true)}</summary>\n\n${renderBlocks(node.children)}\n\n</details>`;
return `<details${node.open === true ? " open" : ""}>\n<summary>${typeof node.summary === "string" ? escapeSummaryText(node.summary) : renderInlines(node.summary, true)}</summary>\n\n${renderBlocks(node.children)}\n\n</details>`;
case "table": {
const row = cells => `| ${cells.map(cell => renderInlines(cell).replace(/(?<!\\)\|/g, "\\|")).join(" | ")} |`;
return [row(node.headers), `| ${node.headers.map(() => "---").join(" | ")} |`, ...node.rows.map(row)].join("\n");
Expand Down
44 changes: 44 additions & 0 deletions actions/setup/js/report_html_fidelity.test.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,50 @@ describe("typed report image presentation", () => {
});

describe("report HTML import boundary", () => {
it.each(["<details open><summary>Open</summary>", "<details open>\n<summary>Open</summary>"])("preserves expanded details blocks: %s", opening => {
const source = `${opening}\n\nbody\n\n</details>`;
const input = parse(source);
expect(input.children).toEqual([
{
type: "details",
open: true,
agentTainted: true,
summary: "Open",
children: [{ type: "paragraph", agentTainted: true, children: [{ type: "text", value: "body", agentTainted: true }] }],
},
]);
expect(normalizeReport(input).children[0].open).toBe(true);
const body = renderReport(input).body;
expect(body).toBe("<details open>\n<summary>Open</summary>\n\nbody\n\n</details>\n");
expect(renderReport(parse(body)).body).toBe(body);
expect(renderReport(parse(sanitizeContent(source))).body).toBe(body);
});

it.each(["<details><summary>Closed</summary>", "<details>\n<summary>Closed</summary>"])("keeps details collapsed by default: %s", opening => {
const input = parse(`${opening}\n\nbody\n\n</details>`);
expect(input.children[0].type).toBe("details");
expect(input.children[0]).not.toHaveProperty("open");
expect(normalizeReport(input).children[0]).not.toHaveProperty("open");
expect(renderReport(input).body).toBe("<details>\n<summary>Closed</summary>\n\nbody\n\n</details>\n");
});

it("recognizes expanded details immediately after a paragraph", () => {
const input = parse("Introduction\n<details open>\n<summary>Open</summary>\n\nbody\n\n</details>");
expect(input.children.map(node => node.type)).toEqual(["paragraph", "details"]);
expect(input.children[1].open).toBe(true);
expect(renderReport(input).body).toBe("Introduction\n\n<details open>\n<summary>Open</summary>\n\nbody\n\n</details>\n");
});

it.each(['class="extra"', 'open class="extra"', 'onclick="attack()"', 'open onclick="attack()"', 'open="true"', "open open"])("keeps unsupported details attributes inert: %s", attributes => {
for (const separator of ["", "\n"]) {
const input = parse(`<details ${attributes}>${separator}<summary>Rejected</summary>\n\nbody\n\n</details>`);
expect(input.children.some(node => node.type === "details")).toBe(false);
const body = renderReport(input).body;
expect(body).not.toContain("<details");
expect(body).toContain("&lt;details ");
}
});

it.each([
'<img src="https://github.com/o/r/image.png" alt="Mascot" width="120" height="80" align="right" />',
"<IMG ALIGN='RIGHT' HEIGHT='80' WIDTH='120' ALT='Mascot' SRC='https://github.com/o/r/image.png'>",
Expand Down
10 changes: 5 additions & 5 deletions actions/setup/js/report_markdown.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ function parseReportMarkdown(markdown, { context, inlineSlots, blockSlots, onBlo
const isFieldLine = line => /^(?:\{\w+\}\s*)+$/.test(line.trim()) && (!inlineSlots || [...line.matchAll(/\{(\w+)\}/g)].some(field => !Object.hasOwn(inlineSlots, field[1])));
const isBlockStart = line =>
openingFence(line) ||
/^(?:#{1,6}\s|>|<details>(?:\s*$|\s*<summary>)|<\/details>\s*$|[-*+]\s|\d+\.\s|---\s*$)/.test(line) ||
/^(?:#{1,6}\s|>|<details(?:\s+open)?>(?:\s*$|\s*<summary>)|<\/details>\s*$|[-*+]\s|\d+\.\s|---\s*$)/.test(line) ||
isFieldLine(line) ||
isBlockSlot(line) ||
(allowMetadata && /^(?:<!--|\[gh-aw-workflow-call-id\]: # ")/.test(line.trim())) ||
Expand Down Expand Up @@ -502,13 +502,13 @@ function parseReportMarkdown(markdown, { context, inlineSlots, blockSlots, onBlo
} else if ((match = /^(#{1,6})\s+(.+)$/.exec(line))) {
nodes.push(fromSource({ type: "heading", level: Math.max(minimumHeadingLevel, match[1].length), children: inline(match[2]) }));
index++;
} else if ((match = /^<details>\s*<summary>(.*)<\/summary>$/.exec(line.trim()))) {
} else if ((match = /^<details(\s+open)?>\s*<summary>(.*)<\/summary>$/.exec(line.trim()))) {

@github-actions github-actions Bot Nov 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Specification synchronization proposal (intentional compatible clarification): update docs/src/content/docs/specs/safe-outputs-specification.md §6.4 S4 (currently v1.29.18) to state that <details> and <details open> are the only accepted forms, including inline and next-line summaries; plain <details> remains collapsed; <details open> retains and renders the boolean open; valued, duplicate, event-handler, and other unsupported attributes MUST be rejected or escaped. Proposed replacement for the safe-tag bullet:

- For `<details>` elements, preserve only the exact forms `<details>` and `<details open>`, including when the summary is inline or appears on the following line.
  - `<details>` MUST remain collapsed.
  - `<details open>` MUST retain the boolean `open` attribute and render as `<details open>`.
  - Other `<details>` attributes, including valued `open` attributes, duplicate `open` attributes, event handlers, and mixed unsupported attributes, MUST be rejected or escaped rather than preserved.

Also add a v1.29.19 (2026-11-06) changelog entry and S4 conformance cases matching these tests. This preserves existing sanitization obligations and documents the behavior implemented here.

Copilot AI Nov 6, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated §6.4 S4 in 2655d49 to define the accepted <details> openings and attribute handling.

index++;
nodes.push(fromSource({ type: "details", summary: summary(match[1]), children: blocks(true, detailsDepth + 1) }));
} else if (line.trim() === "<details>" && /^<summary>.*<\/summary>$/.test(lines[index + 1]?.trim() || "")) {
nodes.push(fromSource({ type: "details", ...(match[1] ? { open: true } : {}), summary: summary(match[2]), children: blocks(true, detailsDepth + 1) }));
} else if ((match = /^<details(\s+open)?>$/.exec(line.trim())) && /^<summary>.*<\/summary>$/.test(lines[index + 1]?.trim() || "")) {
const summarySource = lines[index + 1].trim().slice("<summary>".length, -"</summary>".length);
index += 2;
nodes.push(fromSource({ type: "details", summary: summary(summarySource), children: blocks(true, detailsDepth + 1) }));
nodes.push(fromSource({ type: "details", ...(match[1] ? { open: true } : {}), summary: summary(summarySource), children: blocks(true, detailsDepth + 1) }));
} else if (/^>/.test(line)) {
const quoted = [];
while (index < lines.length && /^>/.test(lines[index])) quoted.push(lines[index++].replace(/^> ?/, ""));
Expand Down
7 changes: 6 additions & 1 deletion docs/src/content/docs/specs/safe-outputs-specification.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ sidebar:

# Safe Outputs MCP Gateway Specification

**Version**: 1.29.18<br>
**Version**: 1.29.19<br>
**Status**: Working Draft<br>
**Publication Date**: 2026-11-06<br>
**Editor**: GitHub Agentic Workflows Team<br>
Expand Down Expand Up @@ -5671,6 +5671,11 @@ Implementations MUST apply these transformations in order:
- `on*` attributes in HTML tags (onclick, onerror, etc.)
- Preserve safe GitHub Flavored Markdown tags:
- `<details>`, `<summary>`, `<sub>`, `<sup>`, `<kbd>`
- `<details>` and `<details open>` are the only accepted details openings; their
summaries may appear inline or on the following line. Plain `<details>` MUST
remain collapsed, and the boolean `open` attribute on `<details open>` MUST be
preserved and rendered. Valued, duplicate, event-handler, and other unsupported
details attributes MUST be rejected or escaped.

**S5: Command Injection Prevention**

Expand Down
Loading