Repository navigation
security(aggregator): comment githubUrl is stored unvalidated and rendered as a link href — a javascript: URL reaches the dashboard #950
Description
Finding
The dashboard comments flow stores a caller-supplied githubUrl with no validation and later renders it as a link href. HTML escaping prevents attribute breakout but not dangerous schemes: a stored javascript: or data: URL becomes a clickable script link in the dashboard.
Evidence (PR #920, merge 12620917dd5f, current main a2eef6f7920a)
pm-stack/aggregator/src/server.ts:150-156:POST /api/comments/github-resultacceptsurl: typeof body.url === "string" ? body.url : undefined— any string, no pattern check.pm-stack/aggregator/src/comments.ts:173-179:CommentsStore.markGithubResultstores it verbatim (c.githubUrl = result.url); no validation exists anywhere incomments.ts.pm-stack/aggregator/src/render.ts:231: renders<a href="${esc(c.githubUrl)}">view</a>.esc()escapes&<>"'(attribute breakout is blocked) but passes ajavascript:scheme through unchanged.
Why it matters
The comments API is unauthenticated by design (loopback + tailnet trust model, per the comments.ts header comment), so anything that can POST to it — a compromised tailnet peer, a stray script, another agent run — can plant a script-scheme link in a dashboard the maintainer clicks during triage. Low severity because of that trust model and because the click is required, but the fix is one allowlist check at the single write path.
Acceptance criteria
markGithubResult(or the endpoint) rejects anyurlnot matching^https://github\.com/.- A test posts a
javascript:URL to the github-result flow and asserts rejection (no storedgithubUrl). - The existing flow — a real
https://github.com/…result URL is recorded and rendered as the "view" link — still works.
Found by the pr-diff-review job reviewing PR #920 against current main. Dedupe: searches for githubUrl / github-result URL validation / dashboard comment href javascript returned no existing issue (only the closed parent implementation issue #906).
Activity
developing-today commentedon Oct 26, 2026
CODE-162
dezren39 commentedon Oct 26, 2026
Agent note — state: filed; Agent: pr-diff-review job; Session/ID: pr-diff-review run 2026-10-25 23:37 CDT; Found via: filed this run (PR diff review against current main a2eef6f). A repo-task/v1 packet for this issue is being enqueued to the repo-health-code-pilot queue.
dezren39 commentedon Nov 2, 2026
Agent note — triaged (OpenChamber scheduled lane)
repo-worker-code), lane7b47536b-c162-444c-be8a-3a9538cc4cb5ad5cb371b. No open PR or branch references security(aggregator): comment githubUrl is stored unvalidated and rendered as a link href — a javascript: URL reaches the dashboard #950; no hard-stop labels; safe to fix.CommentsStore+dashCommentHtmlfrom a read-onlyorigin/mainsnapshot —markGithubResultstoredjavascript:alert(document.domain)verbatim anddashCommentHtmlrendered<a href="javascript:alert(document.domain)">view</a>. Write pathserver.ts:757-766accepts any string;comments.tsvalidatesgithubRefformat (GH_REF) but never the result URL;esc()inrender.ts:98blocks attribute breakout, not schemes.^https://github\.com/) inmarkGithubResult(and/or the endpoint) + regression test per the issue acceptance criteria.62 remaining items
dezren39 commentedon Nov 3, 2026
agent-note:v1
Merge pull request #4599 from developing-today/queue/6484-950-githubu…
dezren39 commentedon Nov 3, 2026
Merged — PR #4599
🤖 Agent note · done
https://github.com/before storinggithubUrlqueue/6484-950-githuburl-allowlistOwnership gate cleared. The previous three lanes parked on "PR #4599 authored by dezren39 = Drew-owned". That read was wrong: PR #4599 is the canonical PR for this task (body says "Fixes #950 (queue task 6484)", head branch is the task branch), and it carries no
held,user/owned,do-not-mergeordo-not-auto-editlabel.dezren39is the fleet's shared commit identity, not evidence of Drew ownership. Verified fresh: issue #950 OPEN, PR #4599 OPEN/non-draft at7ff55cbfc, only checkbasic: OpenCode metered-provider guard= pass.Fix as merged (
pm-stack/aggregator/src/comments.ts):GH_RESULT_URL = /^https:\/\/github\.com\//and a guard inCommentsStore.markGithubResultthat throws for any result URL outside it, with comment state unchanged (the endpoint already maps the throw to 400). Single write-path choke point, so endpoint and direct store callers are both covered.test/slice3.test.ts:javascript:,data:, plainhttp://github.com/,https://evil-github.life-white.uk/…andhttps://github.life-white.uk.evil.com/…all refused with no storedgithubUrl; a realhttps://github.com/…result still posts and renders the view link; endpoint test asserts 400 for ajavascript:payload.Verification this run: worktree
~/git/fleet/6484-950at7ff55cbfc,bun test test/slice3.test.ts— 34 pass, 0 fail (203 assertions). Diff reviewed against the packet: matches intent, no unrelated changes.Merged with a merge commit; the issue closes via the PR's
Fixes #950.developing-today commentedon Nov 3, 2026
agent-note:v1
state: reviewed (coverage loop-back)
Coverage pass 2026-11-03, loop-back review: this issue was re-reviewed (drift / overlap / already-handled / stale / blockers / ordering) so every open issue in the four repos is reviewed and in the system. Verdict: still live; in the system as task 8608 (triage, queued, priority 30) in repo-health-code-pilot. Labels audited and synced. If it is already handled at current main, its triage closes it with evidence; if oversized, the triage files the split; blockers belong on this issue.
Found via: coverage loop-back 2026-11-03
dezren39 commentedon Nov 3, 2026
🤖 Agent note · triaged — already done (closing verb for loop-back task 8608)
repo-worker-code), OpenChamber scheduled lane335a5d1f-bfc1-4831-99ea-850c8939a72769e3b33eabb3a7be9f3c2da0378220825ad3770a, branchqueue/6484-950-githuburl-allowlist).22892c316):pm-stack/aggregator/src/comments.ts:360-364rejects any github-result URL that does not start withhttps://github.com/.status/*,queue/*orwork/*labels on the issue.