From 8a8e34b180e1634e29d960e2eb535e3f5bc298e3 Mon Sep 17 00:00:00 2001 From: Stephen Griffin Date: Thu, 28 May 2026 17:29:10 -0400 Subject: [PATCH 1/5] fix linting on save --- .vscode/settings.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index a55decd6..b0f3e36a 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -2,7 +2,7 @@ "editor.formatOnSave": false, "eslint.validate": ["javascript", "javascriptreact", "typescript"], "editor.codeActionsOnSave": { - "source.fixAll": "explicit" + "source.fixAll.eslint": "explicit" }, "jest.jestCommandLine": "npm test --", "jest.autoRun": { From 090c05c08085ae71f8bb0c9482602f3b0b58cfc9 Mon Sep 17 00:00:00 2001 From: Stephen Griffin Date: Fri, 29 May 2026 09:38:01 -0400 Subject: [PATCH 2/5] Stronger sanitization on html content --- src/Scripts/Strings.ts | 8 --- src/Scripts/row/ArchivedRow.test.ts | 70 +++++++++++++++++++------ src/Scripts/row/ArchivedRow.ts | 20 ++++++- src/Scripts/ui/newMobilePaneIosFrame.ts | 9 ++-- 4 files changed, 78 insertions(+), 29 deletions(-) diff --git a/src/Scripts/Strings.ts b/src/Scripts/Strings.ts index e0a3e575..2ed73fb3 100644 --- a/src/Scripts/Strings.ts +++ b/src/Scripts/Strings.ts @@ -115,14 +115,6 @@ export class Strings { return ""; } - public static mapValueToURL(text: string): string { - try { - return ["", this.htmlEncode(text), ""].join(""); - } catch { - return text; - } - } - // Join an array with char, dropping empty/missing entries public static joinArray(array: (string | number | null)[] | null, char: string): string { if (!array) return ""; diff --git a/src/Scripts/row/ArchivedRow.test.ts b/src/Scripts/row/ArchivedRow.test.ts index 9bb2887a..4724da66 100644 --- a/src/Scripts/row/ArchivedRow.test.ts +++ b/src/Scripts/row/ArchivedRow.test.ts @@ -1,33 +1,73 @@ import { ArchivedRow } from "./ArchivedRow"; +import { Decoder } from "../2047"; import { Strings } from "../Strings"; -jest.mock("../Strings", () => ({ - // eslint-disable-next-line @typescript-eslint/naming-convention - Strings: { - mapHeaderToURL: jest.fn(), - mapValueToURL: jest.fn() - } -})); +jest.mock("../Strings", () => { + const actualStrings = jest.requireActual("../Strings") as typeof import("../Strings"); + + return { + // eslint-disable-next-line @typescript-eslint/naming-convention + Strings: { + mapHeaderToURL: jest.fn((value: string) => actualStrings.Strings.mapHeaderToURL(value)), + htmlEncode: jest.fn((value: string) => actualStrings.Strings.htmlEncode(value)) + } + }; +}); describe("ArchivedRow", () => { - const header = "testHeader"; + const header = "Archived-At"; const label = "testLabel"; let archivedRow: ArchivedRow; beforeEach(() => { - (Strings.mapHeaderToURL as jest.Mock).mockReturnValue("mockedHeaderURL"); - (Strings.mapValueToURL as jest.Mock).mockReturnValue("mockedValueURL"); + jest.clearAllMocks(); archivedRow = new ArchivedRow(header, label); }); it("should set url using Strings.mapHeaderToURL", () => { expect(Strings.mapHeaderToURL).toHaveBeenCalledWith(header, label); - expect(archivedRow.url).toBe("mockedHeaderURL"); + expect(archivedRow.url).toBe("Archived-At"); + }); + + it("should return html href for bracketed http(s) links", () => { + archivedRow.value = ""; + expect(Strings.htmlEncode).not.toHaveBeenCalled(); + expect(archivedRow.valueUrl).toBe("https://example.test/path"); }); - it("should return valueUrl using Strings.mapValueToURL", () => { - archivedRow["valueInternal"] = "internalValue"; - expect(archivedRow.valueUrl).toBe("mockedValueURL"); - expect(Strings.mapValueToURL).toHaveBeenCalledWith("internalValue"); + it("should html encode non-bracketed values", () => { + archivedRow.value = "https://example.test/path"; + const valueUrl = archivedRow.valueUrl; + expect(Strings.htmlEncode).toHaveBeenCalledWith(archivedRow.value); + expect(valueUrl).toBe("https://example.test/path"); + }); + + it("should html encode raw img payload instead of rendering executable HTML", () => { + archivedRow.value = ""; + const valueUrl = archivedRow.valueUrl; + expect(Strings.htmlEncode).toHaveBeenCalledWith(archivedRow.value); + expect(valueUrl).toContain("<img"); + expect(valueUrl).toContain(">"); + expect(valueUrl).not.toContain(" { + const encodedPayload = "=?UTF-8?B?PGltZyBzcmM9eCBvbmVycm9yPWFsZXJ0KCdYU1MnKT4=?="; + archivedRow.value = Decoder.clean2047Encoding(encodedPayload); + + expect(archivedRow.valueUrl).toContain("<img"); + expect(archivedRow.valueUrl).toContain(">"); + expect(archivedRow.valueUrl).not.toContain(" { + const url = "https://example.com/test/list/foo-users@lists.example.com/message/mysubject/"; + archivedRow.value = `<${url}>`; + + expect(archivedRow.valueUrl).toContain("\s*/i); + if (match && match[1]){ + try{ + const url = new URL(match[1]); + if (["http:", "https:"].includes(url.protocol)){ + return ["", Strings.htmlEncode(url.href), ""].join(""); + } + } catch { + // if we can't make URL from it, default to htmlEncode + } + } + + return Strings.htmlEncode(this.valueInternal); + } +} \ No newline at end of file diff --git a/src/Scripts/ui/newMobilePaneIosFrame.ts b/src/Scripts/ui/newMobilePaneIosFrame.ts index 2eb88547..a19803c3 100644 --- a/src/Scripts/ui/newMobilePaneIosFrame.ts +++ b/src/Scripts/ui/newMobilePaneIosFrame.ts @@ -22,6 +22,7 @@ import { ReceivedRow } from "../row/ReceivedRow"; import { Row } from "../row/Row"; import { SummaryRow } from "../row/SummaryRow"; import { escapeAndHighlight, getViolationsForRow, highlightHtml } from "../rules/ViolationUtils"; +import { Strings } from "../Strings"; // This is the "new-mobile" UI rendered in newMobilePaneIosFrame.html @@ -103,7 +104,7 @@ function addCalloutEntry(name: string, value: string | number | null, parent: HT const template = document.getElementById("popover-entry-template") as HTMLTemplateElement; const clone = template.content.cloneNode(true) as DocumentFragment; const p = clone.querySelector("p") as HTMLElement; - p.innerHTML = "" + name + ": " + value; + p.innerHTML = "" + name + ": " + Strings.htmlEncode(value.toString()); parent.appendChild(clone); } } @@ -234,10 +235,10 @@ function buildReceivedTab(viewModel: HeaderModel): void { timelineTime.textContent = currentTime.format("h:mm:ss"); const timelineSubtitle = innerClone.querySelector(".timeline-item-subtitle") as HTMLElement; - timelineSubtitle.innerHTML = "From: " + row.from; + timelineSubtitle.innerHTML = "From: " + Strings.htmlEncode(row.from.toString()); const timelineText = innerClone.querySelector(".timeline-item-text") as HTMLElement; - timelineText.innerHTML = "To: " + row.by; + timelineText.innerHTML = "To: " + Strings.htmlEncode(row.by.toString()); currentTimeEntry.appendChild(innerClone); } else { @@ -272,7 +273,7 @@ function buildReceivedTab(viewModel: HeaderModel): void { timelineTime.textContent = entryTime.format("h:mm:ss"); const timelineSubtitle = innerClone.querySelector(".timeline-item-subtitle") as HTMLElement; - timelineSubtitle.innerHTML = "To: " + row.by; + timelineSubtitle.innerHTML = "To: " + Strings.htmlEncode(row.by.toString()); const delayText = innerClone.querySelector(".delay-text") as HTMLElement; delayText.textContent = row.delay.value !== null ? String(row.delay.value) : ""; From 45ccee1b3bdd3d609d7edcbf6ca214ff21315a2d Mon Sep 17 00:00:00 2001 From: Stephen Griffin Date: Fri, 29 May 2026 09:52:58 -0400 Subject: [PATCH 3/5] Apply suggestions from code review Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Stephen Griffin --- src/Scripts/row/ArchivedRow.test.ts | 10 +++++++--- src/Scripts/row/ArchivedRow.ts | 17 +++++++++++------ 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/src/Scripts/row/ArchivedRow.test.ts b/src/Scripts/row/ArchivedRow.test.ts index 4724da66..49a25590 100644 --- a/src/Scripts/row/ArchivedRow.test.ts +++ b/src/Scripts/row/ArchivedRow.test.ts @@ -8,7 +8,7 @@ jest.mock("../Strings", () => { return { // eslint-disable-next-line @typescript-eslint/naming-convention Strings: { - mapHeaderToURL: jest.fn((value: string) => actualStrings.Strings.mapHeaderToURL(value)), + mapHeaderToURL: jest.fn((headerName: string, text?: string) => actualStrings.Strings.mapHeaderToURL(headerName, text)), htmlEncode: jest.fn((value: string) => actualStrings.Strings.htmlEncode(value)) } }; @@ -31,8 +31,12 @@ describe("ArchivedRow", () => { it("should return html href for bracketed http(s) links", () => { archivedRow.value = ""; - expect(Strings.htmlEncode).not.toHaveBeenCalled(); - expect(archivedRow.valueUrl).toBe("https://example.test/path"); + const valueUrl = archivedRow.valueUrl; + + expect(valueUrl).toContain(" { diff --git a/src/Scripts/row/ArchivedRow.ts b/src/Scripts/row/ArchivedRow.ts index 75f13714..824c126f 100644 --- a/src/Scripts/row/ArchivedRow.ts +++ b/src/Scripts/row/ArchivedRow.ts @@ -9,15 +9,20 @@ export class ArchivedRow extends SummaryRow { } // Return the URL for the archived message, or the encoded value if it's not a valid URL. override get valueUrl(): string { - const match = this.valueInternal.match(/\s*<(.*)>\s*/i); - if (match && match[1]){ - try{ + const match = this.valueInternal.match(/^\s*<([^>]+)>\s*$/); + if (match?.[1]) { + try { const url = new URL(match[1]); - if (["http:", "https:"].includes(url.protocol)){ - return ["", Strings.htmlEncode(url.href), ""].join(""); + if (url.protocol === "http:" || url.protocol === "https:") { + const a = document.createElement("a"); + a.href = url.href; + a.target = "_blank"; + a.rel = "noopener noreferrer"; + a.textContent = url.href; + return a.outerHTML; } } catch { - // if we can't make URL from it, default to htmlEncode + // Fall through to encoded output. } } From c10bbc2be731018e493aea7d64f5d311e0c4de19 Mon Sep 17 00:00:00 2001 From: Stephen Griffin Date: Fri, 29 May 2026 10:03:27 -0400 Subject: [PATCH 4/5] Fix test cases --- src/Scripts/row/ArchivedRow.test.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/Scripts/row/ArchivedRow.test.ts b/src/Scripts/row/ArchivedRow.test.ts index 49a25590..bb872dce 100644 --- a/src/Scripts/row/ArchivedRow.test.ts +++ b/src/Scripts/row/ArchivedRow.test.ts @@ -26,7 +26,7 @@ describe("ArchivedRow", () => { it("should set url using Strings.mapHeaderToURL", () => { expect(Strings.mapHeaderToURL).toHaveBeenCalledWith(header, label); - expect(archivedRow.url).toBe("Archived-At"); + expect(archivedRow.url).toBe("testLabel"); }); it("should return html href for bracketed http(s) links", () => { @@ -69,9 +69,10 @@ describe("ArchivedRow", () => { it("should render anchor only for strict angle-bracketed http(s) URL", () => { const url = "https://example.com/test/list/foo-users@lists.example.com/message/mysubject/"; archivedRow.value = `<${url}>`; + const valueUrl = archivedRow.valueUrl; - expect(archivedRow.valueUrl).toContain(" Date: Fri, 29 May 2026 10:37:10 -0400 Subject: [PATCH 5/5] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Stephen Griffin --- src/Scripts/row/ArchivedRow.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Scripts/row/ArchivedRow.ts b/src/Scripts/row/ArchivedRow.ts index 824c126f..d9086a79 100644 --- a/src/Scripts/row/ArchivedRow.ts +++ b/src/Scripts/row/ArchivedRow.ts @@ -5,7 +5,6 @@ import { SummaryRow } from "./SummaryRow"; export class ArchivedRow extends SummaryRow { constructor(header: string, label: string) { super(header, label); - this.url = Strings.mapHeaderToURL(header, label); } // Return the URL for the archived message, or the encoded value if it's not a valid URL. override get valueUrl(): string {