Skip to content

aggregator: Phase 1 of #2747 — guard the two path-segment decodes (/discuss, /item) on the shared contract #2748

Closed

Description

@dezren39

aggregator: Phase 1 of #2747 — guard the two path-segment decodes (/discuss, /item) on the shared contract

Phase 1 of design #2747 (one decode for path segments — a malformed percent-encoding is a 400, never a 500). Design doc: files/designs/code-path-segment-decode.md in the keep-the-four-repos-healthy goal workspace.

Problem

The aggregator has the family's only unfiled sites — two of them (audit at origin/main 72545cb9):

  • pm-stack/aggregator/src/server.ts:730 — GET /discuss/<ref>: const objectRef = decodeURIComponent(discussMatch[1]);
  • pm-stack/aggregator/src/server.ts:842 — GET /item/<id>: const id = decodeURIComponent(itemMatch[1]);

Both route patterns match any non-empty suffix, the decode is unguarded, and fetchHandler (server.ts:900-909) has no try/catch — a malformed percent sequence (%, %zz, a truncated escape, a truncated UTF-8 sequence like %E0%A4%A) throws a raw URIError out of the handler: under Bun.serve the request fails as an unhandled 500 with no error body. The same defect is filed for the three sibling apps (#2633 lab-ops-hub, #2693 projector, #2720 cf-container-hello); the aggregator instance was missed by all three filings.

Scope (per design #2747)

  • Add one guard helper in the aggregator's own code (a decodePathSegment(raw) -> string | null-shaped function in the module that best fits the app's layout) returning null on URIError — it wraps the decode only, nothing downstream.
  • Both routes call it immediately after matching: null → a JSON 400 naming the bad encoding (the aggregator's established bad-input shape — JSON 400s on its POST routes at server.ts:412,423,432,456,484, JSON 404 for an unknown project id). Only the decoded value proceeds downstream.
  • Decode-once semantics for well-formed input are unchanged: %2F still decodes inside values, 100%25 still yields the literal 100%.

Acceptance criteria

  • GET /discuss/<malformed> and GET /item/<malformed> (each of %, %zz, a truncated escape, %E0%A4%A) return a JSON 400 naming the bad encoding; no exception escapes the handler.
  • Well-formed controls behave exactly as before: a plain ref/id, a %20 segment, the routes' legitimate encoded forms, and 100%25 (a valid encoding of a literal % — must not 400).
  • A well-formed ref/id naming nothing keeps its current behaviour.
  • Handler-level tests pin the shared probe matrix from design(path-decode): one decode for path segments — a malformed percent-encoding is a 400, never a 500 #2747 (the four malformed forms + the controls) for both routes.
  • No other aggregator route changes behaviour; genuine downstream failures keep their current outcomes.

agent-note:v1 — filed by the repo design architect (dedicated code job, pass 20) as Phase 1 of #2747. Sibling phases: Phase 2 adopts #2633 + #2720, Phase 3 adopts #2693 + the closing audit. Do not split this phase per-route — the helper and the matrix are the shared deliverable.

Activity

  1. added
    kind/bugSomething isn't working (prefixed form of the classic bug label)
    task/triageQueue packet kind: verify, reproduce, and route this item
    priority/normalOrdinary priority (the default)
    queue/queuedHas a queued task in the Supabase agent queue
    effort/sSmall: expected under 2 hours of work
    app/pm-stackpm-stack (Pad/Grist) in developing-today/code
    on Oct 30, 2026
  2. dezren39 commented on Oct 30, 2026

    @dezren39
    MemberAuthor

    agent-note:v1 — design-architect pass 20 (2026-10-30): filed as Phase 1 of design #2747 (one decode for path segments) and linked as its sub-issue (verified in #2747's sub_issues list). A triage packet for this issue is being enqueued to the pilot queue (repo-health-code-pilot) in this same pass. Scope is the aggregator only — the sibling apps are Phases 2–3 via their own already-filed issues (#2633, #2720, #2693); do not expand this issue to cover them, and do not split it per-route (the guard helper + shared probe matrix are the deliverable).

  3. dezren39 commented on Oct 31, 2026

    @dezren39
    MemberAuthor

    agent-note:v1
    state: done
    task: 4014 (repo-health-code-pilot, lane D) — triage
    verdict: CONFIRMED on current main fa7e71b
    note: Both decode sites verified verbatim via a fresh gh api read of pm-stack/aggregator/src/server.ts at main: :730 decodeURIComponent(discussMatch[1]) and :842 decodeURIComponent(itemMatch[1]), both unguarded, route regexes match any non-empty suffix, no decodePathSegment helper exists in the file, and fetchHandler (:900) awaits handler(req) with no try/catch — a malformed percent-encoding throws a raw URIError out of the handler (unhandled 500), exactly as filed. Siblings: #2693 and #2720 closed/fixed, #2633 still open separately, design #2747 open. Scoped fix enqueued as queue task 4376 (decodePathSegment helper + JSON 400 at both sites + probe-matrix handler tests).

  4. added
    work/assignedQueue worker lifecycle state
    and removed
    queue/queuedHas a queued task in the Supabase agent queue
    on Oct 31, 2026
  5. 4 remaining items

  6. added
    queue/parkedQueue task is parked (waiting on CI, time, or an event)
    status/parkedDeliberately paused with a recorded resume condition
    and removed
    queue/claimedQueue worker lifecycle state
    on Oct 31, 2026
  7. dezren39 commented on Oct 31, 2026

    @dezren39
    MemberAuthor

    🤖 Agent note · parked

  8. added
    queue/claimedQueue 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 Oct 31, 2026
  9. dezren39 commented on Oct 31, 2026

    @dezren39
    MemberAuthor

    Agent note — picked up (resume)

    • Queue task: 4376 (lane B shift, 2026-10-31)
    • State: PR aggregator: guard /discuss and /item path-segment decodes — JSON 400 on malformed percent-encoding (#2748) #3020 already open at head 5476725fe; resumed from checkpoint awaiting_ci.
    • Fresh verification: Codex review state for head 5476725fe = completed, no findings, none unresolved; no unresolved review threads; diff is exactly the packet scope (new decodePathSegment helper + both call sites + handler tests).
    • CI: no Actions checks have started on this PR (fleet runner backlog, fleet-wide this shift); laptop verification from the implementing checkpoint: path-segment tests 22/22 pass, full aggregator suite at main baseline (7 pre-existing fails).
    • Action: merging under the standing merge directive on laptop verification + clean Codex; post-merge CI to be watched by fleet monitor.
  10. dezren39 commented on Oct 31, 2026

    @dezren39
    MemberAuthor

    Agent note — done

    • Queue task: 4376 completed fixed_merged (lane B, 2026-10-31).
    • PR aggregator: guard /discuss and /item path-segment decodes — JSON 400 on malformed percent-encoding (#2748) #3020 merged (squash) at 2026-10-31T11:11:09Z, merge SHA a9df3018c78d7f68822a70f5566b6846b1d0b122; this issue closed by the merge.
    • Verification: Codex clean for head 5476725fe (no findings, none unresolved), no unresolved review threads, diff exactly packet scope; laptop path-segment tests 22/22 (implementing checkpoint), full aggregator suite at main baseline.
    • CI note: no Actions checks ever started on the PR (fleet runner backlog); merged under the standing directive on laptop verification + clean Codex — post-merge CI on main is the watch item.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

app/pm-stackpm-stack (Pad/Grist) in developing-today/codeeffort/sSmall: expected under 2 hours of workkind/bugSomething isn't working (prefixed form of the classic bug label)priority/normalOrdinary priority (the default)source/design-architectFiled by the repo design architect jobtask/triageQueue packet kind: verify, reproduce, and route this item

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions