fix: harden gh issue escape hygiene

This commit is contained in:
Codex
2026-05-20 14:31:57 +00:00
parent 0464aa01a8
commit 05bc8aab5a
7 changed files with 575 additions and 38 deletions
+111
View File
@@ -128,6 +128,47 @@ async function startMockGitHub(): Promise<{ baseUrl: string; requests: MockReque
updated_at: "2026-05-20T03:10:00Z",
},
];
const scanIssues = [
{
id: 2501,
number: 51,
title: "polluted issue",
body: "## Update\\n- item with `code`\\n| a | b |\\n",
state: "open",
html_url: "https://github.com/pikasTech/unidesk/issues/51",
comments: 1,
user: { login: "runner" },
labels: [],
created_at: "2026-05-20T04:00:00Z",
updated_at: "2026-05-20T04:30:00Z",
},
{
id: 2502,
number: 52,
title: "explanatory issue",
body: "文档说明:字面量 `\\n` 只是在示例中提到,不代表正文污染。\n",
state: "open",
html_url: "https://github.com/pikasTech/unidesk/issues/52",
comments: 1,
user: { login: "runner" },
labels: [],
created_at: "2026-05-20T04:05:00Z",
updated_at: "2026-05-20T04:35:00Z",
},
{
id: 2503,
number: 53,
title: "null body issue",
body: null,
state: "open",
html_url: "https://github.com/pikasTech/unidesk/issues/53",
comments: 0,
user: { login: "runner" },
labels: [],
created_at: "2026-05-20T04:10:00Z",
updated_at: "2026-05-20T04:40:00Z",
},
];
const comments = [
{
id: 1,
@@ -138,6 +179,29 @@ async function startMockGitHub(): Promise<{ baseUrl: string; requests: MockReque
updated_at: "2026-05-20T00:30:00Z",
},
];
const scanComments: Record<number, JsonRecord[]> = {
51: [
{
id: 5101,
body: "comment line 1\\ncomment line 2\\twith tab",
html_url: "https://github.com/pikasTech/unidesk/issues/51#issuecomment-5101",
user: { login: "runner" },
created_at: "2026-05-20T04:40:00Z",
updated_at: "2026-05-20T04:40:00Z",
},
],
52: [
{
id: 5201,
body: "说明性提到字面量 `\\n`,用于描述问题本身。",
html_url: "https://github.com/pikasTech/unidesk/issues/52#issuecomment-5201",
user: { login: "runner" },
created_at: "2026-05-20T04:45:00Z",
updated_at: "2026-05-20T04:45:00Z",
},
],
53: [],
};
const server = createServer(async (req, res) => {
const body = await collectBody(req);
requests.push({ method: req.method ?? "", url: req.url ?? "", body });
@@ -161,6 +225,16 @@ async function startMockGitHub(): Promise<{ baseUrl: string; requests: MockReque
sendJson(res, 200, issueList);
return;
}
if (req.method === "GET" && req.url === "/repos/pikasTech/unidesk/issues?state=all&per_page=4") {
sendJson(res, 200, scanIssues);
return;
}
for (const [issueNumber, issueComments] of Object.entries(scanComments)) {
if (req.method === "GET" && req.url === `/repos/pikasTech/unidesk/issues/${issueNumber}/comments?per_page=100`) {
sendJson(res, 200, issueComments);
return;
}
}
if (req.method === "PATCH" && req.url === "/repos/pikasTech/unidesk/issues/20") {
const parsed = JSON.parse(body) as JsonRecord;
sendJson(res, 200, { ...issue, body: String(parsed.body ?? issue.body), updated_at: "2026-05-20T01:05:00Z" });
@@ -225,6 +299,33 @@ export async function runGhCliIssueGuardContract(): Promise<JsonRecord> {
assertCondition(Array.isArray(firstLabels) && firstLabels[0]?.name === "cli", "issue list default fields should include labels", listDefaultData);
assertCondition(defaultIssues.every((item) => typeof item.number === "number" && typeof item.url === "string"), "issue list default fields should expose stable JSON", listDefaultData);
const scanEscape = await runCli(["gh", "issue", "scan-escape", "--repo", "pikasTech/unidesk", "--limit", "4", "--dry-run"], env);
assertCondition(scanEscape.status === 0, "issue scan-escape dry-run should succeed", scanEscape.json ?? { stdout: scanEscape.stdout });
const scanData = dataOf(scanEscape.json ?? {});
assertCondition(scanData.dryRun === true && scanData.planned === true, "scan-escape dry-run should be explicit", scanData);
const scanSummary = scanData.summary as JsonRecord;
assertCondition(Number(scanSummary.suspectedPollution ?? 0) >= 2, "scan should find suspected pollution in body/comment", scanSummary);
assertCondition(Number(scanSummary.explanatoryMention ?? 0) >= 1, "scan should classify explanatory literal backslash-n separately", scanSummary);
assertCondition(Number(scanSummary.bodyRisks ?? 0) >= 1, "scan should report null/short body risks", scanSummary);
const scanFindings = scanData.findings as JsonRecord[];
assertCondition(Array.isArray(scanFindings), "scan should expose findings array", scanData);
assertCondition(scanFindings.some((finding) => finding.issueNumber === 51 && finding.classification === "suspected-pollution" && finding.bodyKind === "issue-body" && typeof finding.bodyId === "string"), "polluted issue body should be suspected with body id", scanFindings);
assertCondition(scanFindings.some((finding) => finding.commentId === 5101 && finding.classification === "suspected-pollution" && finding.bodyKind === "comment-body" && String(finding.bodyId ?? "").includes("comment:5101")), "polluted comment should include comment id and body id", scanFindings);
assertCondition(scanFindings.some((finding) => finding.issueNumber === 52 && finding.classification === "explanatory-mention"), "explanatory literal backslash-n should not be pollution", scanFindings);
assertCondition(scanFindings.some((finding) => finding.issueNumber === 53 && finding.kind === "null-body" && finding.classification === "risk"), "null body should be guarded as risk", scanFindings);
const cleanupSuggestions = scanData.cleanupSuggestions as JsonRecord[];
assertCondition(Array.isArray(cleanupSuggestions), "scan should expose cleanupSuggestions", scanData);
assertCondition(cleanupSuggestions.some((suggestion) => suggestion.issueNumber === 51 && suggestion.type === "issue-body" && typeof suggestion.bodyId === "string" && suggestion.action === "rewrite-issue-body-with-body-file"), "issue body cleanup suggestion should use body-file rewrite with body id", cleanupSuggestions);
assertCondition(cleanupSuggestions.some((suggestion) => suggestion.commentId === 5101 && suggestion.type === "comment-body" && String(suggestion.bodyId ?? "").includes("comment:5101") && suggestion.action === "review-comment-manually"), "comment cleanup suggestion should be manual review with body id", cleanupSuggestions);
assertCondition(cleanupSuggestions.every((suggestion) => suggestion.issueNumber !== 52), "explanatory mention should not create cleanup suggestion", cleanupSuggestions);
const scanPatchCount = mock.requests.filter((request) => request.method === "PATCH" || request.method === "DELETE" || request.method === "POST").length;
assertCondition(scanPatchCount === 0, "scan-escape must not write GitHub", { requests: mock.requests });
const cleanupPlan = await runCli(["gh", "issue", "cleanup-plan", "--repo", "pikasTech/unidesk", "--limit", "4"], env);
assertCondition(cleanupPlan.status === 0, "issue cleanup-plan should succeed as read-only alias", cleanupPlan.json ?? { stdout: cleanupPlan.stdout });
const cleanupPlanData = dataOf(cleanupPlan.json ?? {});
assertCondition(cleanupPlanData.command === "issue cleanup-plan" && cleanupPlanData.dryRun === true, "cleanup-plan should remain dry-run", cleanupPlanData);
const badListField = await runCli(["gh", "issue", "list", "--repo", "pikasTech/unidesk", "--json", "number,body"], env);
assertCondition(badListField.status !== 0, "issue list unsupported --json field should fail", badListField.json ?? { stdout: badListField.stdout });
const badListFieldData = failedDataOf(badListField.json ?? {});
@@ -314,6 +415,13 @@ export async function runGhCliIssueGuardContract(): Promise<JsonRecord> {
const appendFile = join(tmp, "append.md");
writeFileSync(appendFile, "\n- appended `code`\n| c | d |\n| --- | --- |\n| 3 | 4 |\n", "utf8");
const issueCreateDryRun = await runCli(["gh", "issue", "create", "--repo", "pikasTech/unidesk", "--title", "body file dry-run", "--body-file", appendFile, "--dry-run"], env);
assertCondition(issueCreateDryRun.status === 0, "issue create dry-run should succeed", issueCreateDryRun.json ?? { stdout: issueCreateDryRun.stdout });
const issueCreateDryRunData = dataOf(issueCreateDryRun.json ?? {});
const issueCreateBodySource = issueCreateDryRunData.bodySource as JsonRecord;
assertCondition(issueCreateDryRunData.planned === true && issueCreateBodySource.kind === "body-file" && issueCreateBodySource.path === appendFile, "issue create dry-run should expose body-file source", issueCreateDryRunData);
assertCondition(issueCreateDryRunData.request && typeof issueCreateDryRunData.request === "object", "issue create dry-run should expose request plan", issueCreateDryRunData);
const appendDryRun = await runCli(["gh", "issue", "update", "20", "--repo", "pikasTech/unidesk", "--mode", "append", "--body-file", appendFile, "--dry-run"], env);
assertCondition(appendDryRun.status === 0, "issue update append dry-run should succeed", appendDryRun.json ?? { stdout: appendDryRun.stdout });
const appendData = dataOf(appendDryRun.json ?? {});
@@ -354,6 +462,9 @@ export async function runGhCliIssueGuardContract(): Promise<JsonRecord> {
"issue list supports state/limit/json with stable selected fields",
"acceptance issue list command succeeds under mock GitHub",
"issue list default fields include labels and filter pull requests",
"issue scan-escape classifies pollution, explanatory mentions, and body risks",
"issue cleanup-plan remains dry-run with body/comment cleanup suggestions",
"issue create dry-run exposes body-file source and request plan",
"issue list unsupported fields and states fail structurally",
"issue view supports body,title,state,comments selection",
"unsupported --json fields fail structurally",