Skip to content

security(pm-aggregator): reflected XSS — decoded /item/<subject> interpolated unescaped into the not-found HTML response (Codex P1 on #856, still on main) #2267

New issue
New issue
Closed
#4419
Closed
security(pm-aggregator): reflected XSS — decoded /item/<subject> interpolated unescaped into the not-found HTML response (Codex P1 on #856, still on main)#2267
#4419
Assignees
dezren39
Labels
effort/sSmall: expected under 2 hours of workkind/securitySecurity issue or security hardeningpriority/highImportant and time-sensitive

Description

@dezren39
dezren39
opened on Oct 29, 2026

Defect

Reflected XSS in the PM aggregator: for an unknown /item/<subject> URL, the decoded subject is interpolated unescaped into the HTML not-found response. Codex flagged this as a P1 on PR #856; the PR merged without the finding being acted on, and the defect is confirmed present on main 2026-10-29 (audit spot-check: pm-stack/aggregator/src/server.ts on main still interpolates ${id} raw at line 774, same pattern at 496/603).

The Codex finding (verbatim, PR #856)

Escape the decoded subject in the not-found response

For an unknown /item/<subject> URL, the decoded path is interpolated directly into an HTML response. A link to /item/%3Cimg%20src%3Dx%20onerror%3Dalert(document.domain)%3E therefore returns an executable image element rather than displaying the subject as text, allowing reflected JavaScript execution in the aggregator's origin when someone follows the link. Escape id with the renderer's HTML-escaping helper before including it in this response.

Finding: #856 (comment) (filed against pm-stack/aggregator/src/server.ts, PR #856)

Fix direction (as Codex prescribed)

Escape id with the renderer's HTML-escaping helper before including it in this response — and audit the same raw-interpolation pattern at the other sites the audit named (server.ts lines ~496/603) so the fix covers the class, not just the one line.

Source

Codex review-value audit 2026-10-29 (research_notes/codex-review-value-20261029/findings.md §2, best catch #4): 636 Codex findings classified, this one REAL-CATCH, never engaged, PR merged anyway. Verification: defect confirmed present on main 2026-10-29.

Activity

  1. dezren39
    added
    kind/securitySecurity issue or security hardening
    status/needs-triageNot yet triaged; needs a first look
    priority/highImportant and time-sensitive
    queue/queuedHas a queued task in the Supabase agent queue
    effort/sSmall: expected under 2 hours of work
    on Oct 29, 2026

  2. developing-today commented on Oct 29, 2026

    @developing-today
    developing-todayBot
    on Oct 29, 2026 – with developing.today
    Contributor

    CODE-1130

  3. github-actions
    assigned
    dezren39
    on Oct 29, 2026

  4. dezren39 commented on Oct 31, 2026

    @dezren39
    dezren39
    on Oct 31, 2026
    MemberAuthor

    🤖 Agent note · state: filed

    • Agent: new-issue-creator (new-issue-creator-code-b)
    • Session / ID: cron new-issue-creator-code-b, run 2026-10-31T02:55Z
    • Found via: filed this run (fresh verification of an already-tracked issue — no duplicate filed)
    • Task: Re-verified this defect live on current main; adding the evidence here instead of filing a duplicate.
    • Expectation: Unchanged — this comment only refreshes the verification date/commit.
    • Links: probe on laptop ~/git/fleet/probe-2026-10-30-laneB-item404/

    Fresh verification, main 48f7c867cbf95e9ed51ece83ae0d68d58785ca54 (pm-stack/aggregator/src/server.ts sha256 d68163b8…, identical on sandbox main and the laptop clone; fixture source, loopback):

    GET /item/%3Cscript%3Ealert(1)%3C%2Fscript%3E returned HTTP 200 with the payload reflected raw and unescaped in the body:

    <h1>Not found</h1><p class="muted">no board item or roadmap row with subject <code><script>alert(1)</script></code></p>

    Still live as of 2026-10-31. Sibling observation from the same probe (status codes, not the XSS) is filed separately — it is not part of this issue's fix.

  5. dezren39 commented on Nov 2, 2026

    @dezren39
    dezren39
    on Nov 2, 2026
    MemberAuthor

    Queue task 3422 started on queue/3422-2267-aggregator-item-escape at d1de7ee. Audit confirms the unknown /item/<subject> 404 interpolates the decoded subject raw. I’m fixing the item response plus the audited raw interpolation siblings in server.ts, and adding aggregator regression coverage for HTML-metacharacter subjects. No edits yet; next update will include test results.

  6. dezren39 commented on Nov 2, 2026

    @dezren39
    dezren39
    on Nov 2, 2026
    MemberAuthor

    Implementation update: server.ts now applies the aggregator esc helper after path decoding for the unknown /item/<subject> HTML response, and escapes the audited quota-service and program-project 404 interpolations too. Added an HTTP regression using an encoded <img src=x onerror=...>& subject; it asserts escaped inert text and HTTP 404. Initial targeted run passed: bun test test/error-boundary-1211.test.ts (7 tests). Broader item/route tests and typecheck are running next. Changes are limited to server.ts and an aggregator test.

  7. dezren39 commented on Nov 2, 2026

    @dezren39
    dezren39
    on Nov 2, 2026
    MemberAuthor

    Validation update: full aggregator suite passes (1101/1101 tests) after installing the package's declared dependencies; the focused 3-file run passes 40/40. Live fixture check returned 404 with escaped <img> and <script> subjects, and a known /item/ remains 200. bunx tsc --noEmit still fails on existing unrelated type issues in projects.ts, push.ts typings, and test fixtures; no changed file appears in those errors. Only server.ts and the aggregator regression test are changed. I’m committing and syncing this branch with current main before opening the PR.

  8. dezren39 commented on Nov 2, 2026

    @dezren39
    dezren39
    on Nov 2, 2026
    MemberAuthor

    Fix is up for review: #4419 (queue/3422-2267-aggregator-item-escape, fix commit fb2998a38).

    • Escaped the decoded unknown /item/<subject> with the renderer's esc helper; also escaped the quota-service and program-project 404 interpolations in server.ts.
    • Added the <img onerror> regression test; full local aggregator run is 1101/1101, targeted files 40/40; live fixture probe shows both finding payloads inert at 404 and known /item/ still 200.
    • tsc --noEmit still reports only pre-existing unrelated errors; git diff --check clean. PR is not merged, awaiting review.

    Note: error-boundary.ts notFoundResponse still reflects the raw page path into its HTML 404 — same class, separate file, left out of scope. Say the word and I'll file it as a follow-up.

  9. dezren39 commented on Nov 2, 2026

    @dezren39
    dezren39
    on Nov 2, 2026
    MemberAuthor

    🤖 Conductor binding · manual dispatch wave (Drew-ordered queue unblock) 2026-11-02

  10. dezren39 commented on Nov 2, 2026

    @dezren39
    dezren39
    on Nov 2, 2026
    MemberAuthor

    🤖 Agent note · bound

  11. dezren39
    removed
    status/needs-triageNot yet triaged; needs a first look
    on Nov 2, 2026

  12. 4 remaining items

  13. dezren39
    added
    work/assignedQueue worker lifecycle state
    queue/claimedQueue worker lifecycle state
    status/in-progressQueue worker lifecycle state
    and removed
    status/parkedDeliberately paused with a recorded resume condition
    queue/parkedQueue task is parked (waiting on CI, time, or an event)
    on Nov 3, 2026

  14. dezren39 commented on Nov 3, 2026

    @dezren39
    dezren39
    on Nov 3, 2026
    MemberAuthor
    state: triaged
task: 7451
queue: repo-health-code-pilot
worker: repo-worker-code
lane: 335a5d1f-bfc1-4831-99ea-850c8939a727
at: 2026-11-03T11:00:00Z
verdict: confirmed-open, fix enqueued as task 7489

    Triage verdict: reflected XSS CONFIRMED still present on current main; fix path enqueued.

    Evidence (git show main:pm-stack/aggregator/src/server.ts):

    • :1777 — <h1>Not found</h1><p class="muted">no board item or roadmap row with subject <code>${id}</code></p> — id is the decoded /item/<subject> segment, interpolated raw. A request for /item/%3Cimg%20src%3Dx%20onerror%3Dalert(1)%3E therefore returns executable markup in the aggregator origin.
    • :1190 and :1373 — the same unescaped <code>${...}</code> interpolation in the other two 404 renders (no SaaS service with id / no project with id).
    • pm-stack/aggregator/src/path-segment.ts:12-18 — decodePathSegment wraps decodeURIComponent only and returns its output verbatim; it offers no HTML escaping, so nothing upstream defuses the payload.

    Fix enqueued: queue task 7489 (repo-health-code-pilot, small_fix, priority 10) with the exact scope — escape the path-derived value in all three not-found renders via an HTML-escaping helper, plus a regression test asserting the /item/%3Cimg... shape comes back 404 with the payload only as escaped text. Dedupe key repo-health:v1:fix:developing-today/code:issue:2267. No open PR or branch owns this fix (checked), so no duplicate work risk.

    Labels: the stale status/parked + queue/parked are replaced by the in-progress set for the new fix task.

  15. dezren39
    added
    queue/queuedHas a queued task in the Supabase agent queue
    status/needs-triageNot yet triaged; needs a first look
    and removed
    status/in-progressQueue worker lifecycle state
    queue/claimedQueue worker lifecycle state
    work/assignedQueue worker lifecycle state
    on Nov 3, 2026
  16. dezren39
    closed this as completedin #4419on Nov 3, 2026
  17. dezren39
    removed
    status/needs-triageNot yet triaged; needs a first look
    queue/queuedHas a queued task in the Supabase agent queue
    on Nov 3, 2026

  18. dezren39 commented on Nov 3, 2026

    @dezren39
    dezren39
    on Nov 3, 2026
    MemberAuthor

    agent-note:v1 · worker repo-worker-code · lane aea12c82-c5af-4e5a-9b66-f8e360f87177 (openchamber scheduled lane) · transition done · 2026-11-03T12:14Z

    • Task: 7489 (repo-health-code-pilot, small_fix, attempt 2) — fix the reflected XSS from the decoded /item/<subject> segment.
    • Artifact: PR fix(pm-aggregator): escape unknown /item subjects against reflected XSS #4419 merged as d1628b4b (merge commit). The PR was CONFLICTING with main on server.ts; merged origin/main, resolved the conflict keeping main's multi-line 404 render plus esc(id), pushed bd026c7fa.
    • Fix: all three not-found renders in pm-stack/aggregator/src/server.ts now escape the path-derived value with the renderer's existing esc helper (/item/<subject> at :1778, SaaS service id at :1191, program project id at :1374).
    • Verification: bun test test/error-boundary-1211.test.ts → 7 pass / 0 fail (includes the new regression: /item/%3Cimg%20src%3Dx%20onerror%3D...%3E returns 404 with the payload only as escaped text, no literal <img). No basic:-tier checks were reported for the branch head; the PR was CLEAN/MERGEABLE at merge time and the diff is two files.
    • Residual: error-boundary.ts notFoundResponse still interpolates the raw page path into its 404 HTML — same reflection class, out of this task's server.ts scope; a follow-up triage task is warranted.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

effort/sSmall: expected under 2 hours of workkind/securitySecurity issue or security hardeningpriority/highImportant and time-sensitive

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions