From a6fff434a418159e242fa327841a9c3d5bb7947b Mon Sep 17 00:00:00 2001
From: saphid <4596216+saphid@users.noreply.github.com>
Date: Mon, 28 Sep 2026 14:45:54 +1000
Subject: [PATCH] Website feedback: attribution first, escape <, wait out the
 fill timer; maintainers only in the approval queue

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
---
 .github/workflows/approve-contributor.yml |  9 ++++++---
 site/lib/feedback.js                      | 23 ++++++++++++++---------
 site/public/js/feedback.js                |  6 ++++--
 site/test/feedback.test.mjs               |  8 +++++---
 4 files changed, 29 insertions(+), 17 deletions(-)

diff --git a/.github/workflows/approve-contributor.yml b/.github/workflows/approve-contributor.yml
index 665128b..68cc7fd 100644
--- a/.github/workflows/approve-contributor.yml
+++ b/.github/workflows/approve-contributor.yml
@@ -9,9 +9,12 @@ on:
 
 jobs:
   approve:
-    # Only comments that might approve someone join the queue, and they run one
-    # at a time so two lgtm replies can't race on APPROVED_CONTRIBUTORS.
-    if: contains(github.event.comment.body, 'lgtm')  # contains() ignores case
+    # Only maintainers' comments that might approve someone join the queue, and
+    # they run one at a time so two lgtm replies can't race on APPROVED_CONTRIBUTORS.
+    # (The script below still checks for write access.)
+    if: >-
+      contains(github.event.comment.body, 'lgtm') &&
+      contains(fromJSON('["OWNER", "MEMBER", "COLLABORATOR"]'), github.event.comment.author_association)
     concurrency:
       group: approve-contributor
       cancel-in-progress: false
diff --git a/site/lib/feedback.js b/site/lib/feedback.js
index 91df858..d697474 100644
--- a/site/lib/feedback.js
+++ b/site/lib/feedback.js
@@ -20,11 +20,13 @@ const oneLine = (value, max) => String(value ?? "").replace(/\s+/g, " ").trim().
 // Mentions in someone else's text would ping strangers, and issue references
 // (#1, owner/repo#1, GH-1, github.com links) would add backlinks to other
 // people's issues, so break them all with a zero-width space. Escaping & first
-// stops &commat; and &num; from turning back into @ and # when GitHub renders.
+// stops &commat; and &num; from turning back into @ and # when GitHub renders,
+// and escaping < keeps out raw HTML such as an unclosed <!-- comment.
 const ZWSP = "\u200b";
 export function defang(text) {
   return text
     .replace(/&/g, "&amp;")
+    .replace(/</g, "&lt;")
     .replace(/@(?=[A-Za-z0-9])/g, `@${ZWSP}`)
     .replace(/#(?=\d)/g, `#${ZWSP}`)
     .replace(/\b(GH)-(?=\d)/gi, `$1${ZWSP}-`)
@@ -37,7 +39,9 @@ export function validate(input) {
   if (oneLine(input.website, 200)) return { spam: true };
   // Measured in the browser with a monotonic clock, so clock skew doesn't matter.
   const elapsed = Number(input.elapsed);
-  if (!Number.isFinite(elapsed) || elapsed < MIN_FILL_MS) return { spam: true };
+  if (!Number.isFinite(elapsed)) return { spam: true };
+  // The page waits this long before sending, so only scripts get here; say so anyway.
+  if (elapsed < MIN_FILL_MS) return { error: "That was quick. Send it again in a moment." };
 
   const kind = Object.hasOwn(KINDS, input.kind) ? input.kind : "other";
   const title = oneLine(input.title, LIMITS.title[1]);
@@ -70,16 +74,17 @@ export function buildIssue(value) {
     ["SteamOS build", value.steamos],
   ].filter(([, v]) => v);
 
-  const lines = [defang(value.message), ""];
+  // Our own lines go first, so nothing in the sender's text can hide them.
+  const lines = [
+    value.github
+      ? `> Sent from the website feedback form by @${value.github}.`
+      : "> Sent from the website feedback form. The sender left no GitHub username, so they won't see replies here.",
+    "",
+  ];
   if (details.length) {
     lines.push("| | |", "|---|---|", ...details.map(([k, v]) => `| ${k} | ${defang(v).replace(/\|/g, "\\|")} |`), "");
   }
-  lines.push(
-    "---",
-    value.github
-      ? `Sent from the website feedback form by @${value.github}.`
-      : "Sent from the website feedback form. The sender left no GitHub username, so they won't see replies here.",
-  );
+  lines.push(defang(value.message));
 
   return {
     title: `${kind.title}: ${value.title}`,
diff --git a/site/public/js/feedback.js b/site/public/js/feedback.js
index 385e1df..c608f5a 100644
--- a/site/public/js/feedback.js
+++ b/site/public/js/feedback.js
@@ -50,7 +50,6 @@ form.addEventListener("submit", async (e) => {
   e.preventDefault();
   errorBox.hidden = true;
   const data = Object.fromEntries(new FormData(form));
-  data.elapsed = Math.round(performance.now() - started);
 
   if (data.title.trim().length < 5) {
     form.elements.title.focus();
@@ -64,10 +63,13 @@ form.addEventListener("submit", async (e) => {
   send.disabled = true;
   send.textContent = "Sending…";
   try {
+    // The server treats anything sent sooner as a script (site/lib/feedback.js MIN_FILL_MS).
+    const wait = 3100 - (performance.now() - started);
+    if (wait > 0) await new Promise((resolve) => setTimeout(resolve, wait));
     const res = await fetch("/api/feedback", {
       method: "POST",
       headers: { "content-type": "application/json" },
-      body: JSON.stringify(data),
+      body: JSON.stringify({ ...data, elapsed: Math.round(performance.now() - started) }),
     });
     const reply = await res.json().catch(() => ({}));
     if (!res.ok) throw new Error(reply.error || "Something went wrong sending that.");
diff --git a/site/test/feedback.test.mjs b/site/test/feedback.test.mjs
index dcd03e2..5a2621b 100644
--- a/site/test/feedback.test.mjs
+++ b/site/test/feedback.test.mjs
@@ -19,9 +19,9 @@ test("accepts a normal report and trims it", () => {
   assert.equal(value.kind, "bug");
 });
 
-test("flags the honeypot and too-fast submissions as spam", () => {
+test("flags the honeypot as spam and asks fast senders to retry", () => {
   assert.deepEqual(validate(form({ website: "http://spam" })), { spam: true });
-  assert.deepEqual(validate(form({ elapsed: 500 })), { spam: true });
+  assert.match(validate(form({ elapsed: 500 })).error, /again/);
   assert.deepEqual(validate(form({ elapsed: undefined })), { spam: true });
 });
 
@@ -43,7 +43,8 @@ test("builds a labelled issue that credits a GitHub user", () => {
   assert.equal(issue.title, "Bug report: Live view freezes");
   assert.deepEqual(issue.labels, ["feedback", "bug"]);
   assert.match(issue.body, /\| Frame Control version \| 0\.3\.1 \|/);
-  assert.match(issue.body, /by @octocat\.$/);
+  assert.match(issue.body, /^> Sent from the website feedback form by @octocat\./);
+  assert.ok(issue.body.endsWith(value.message));
 });
 
 test("anonymous feedback says replies won't reach the sender", () => {
@@ -57,6 +58,7 @@ test("breaks mentions, issue refs and table cells in user text", () => {
   assert.equal(defang("email me@example.com"), "email me@\u200bexample.com");
   assert.equal(defang("see valve/steam#7 and GH-8"), "see valve/steam#\u200b7 and GH\u200b-8");
   assert.equal(defang("&commat;valve &#64;valve &num;3"), "&amp;commat;valve &amp;#\u200b64;valve &amp;num;3");
+  assert.equal(defang("end <!--"), "end &lt;!--");
   assert.equal(defang("https://github.com/a/b/issues/1"), "https://github\u200b.com/a/b/issues/1");
   const issue = buildIssue(validate(form({ os: "a | b" })).value);
   assert.match(issue.body, /\| a \\\| b \|/);