Repository navigation
aggregator(tier-items): a valid answers array shields optionId from membership checking — checkActionAgainstItem's else-if chain never validates optionId when answers is present #4286
Description
Problem
checkActionAgainstItem (pm-stack/aggregator/src/tier-items.ts:342-376) validates an answer action's payload with an if / else if / else if chain over answers → optionId → text. As soon as a payload carries a non-empty answers array, the optionId branch is never reached — so an optionId that is not one of the item's published options is accepted unchecked, even though the same value submitted on its own is rejected.
validateNewAction (src/actions.ts:272-273, 368-378) accepts the combination: it only requires that at least one of text / optionId / answers be present, and stores all of them verbatim. The stored action is then delivered to the consumer as-is (actions.ts header: the pass "delivers payload text/option/answers"), and cardState (src/card-state.ts:100) records the unchecked value as the card's answeredOptionId, which render.ts:1361 compares against the published options.
Evidence (verified this run, laptop probe against real code @ cc819fe, md5-matched)
Probe: ~/git/fleet/probe-2026-11-02-laneB-answeroption/ (verbatim imports of src/tier-items.ts + src/card-state.ts; item fixture mirrors test/needs-user.test.ts: options csv/json, single question audience with options internal/public):
- control —
optionId: "yaml"alone →"option \"yaml\" is not one of this item's published options"(rejected, as the existing test at test/needs-user.test.ts:310 expects) - control —
optionId: "csv"alone →null; validanswersalone →null - bug —
answers: [{questionId: "audience", value: "internal"}]+optionId: "yaml"→null(accepted) - bug — same valid answers +
optionId: "bogus-zzz"(never published) →null(accepted) - downstream —
cardStateon that accepted action:answeredOptionId => "bogus-zzz"
Why it matters
checkActionAgainstItem is the POST boundary whose documented contract (tier-items.ts:335-338) is "option and survey membership" checking — the consumer trusts that an accepted answer's option was actually published on the item. The else-if short-circuit lets any client (or a buggy/stale form) attach an arbitrary, never-offered option id to a legitimate survey submit, and that id propagates to the consumer and into the card's answered state, where it matches no published option.
Acceptance criteria
- An
answeraction carryingoptionIdis membership-checked against the item's published options regardless of whetheranswers(ortext) is also present — or combined content payloads are rejected outright atvalidateNewActionwith a legible error. Either resolution is acceptable; silent acceptance is not. - Regression tests in pm-stack/aggregator/test/needs-user.test.ts: valid answers + unpublished optionId is rejected (or the combination is rejected); valid answers + a published optionId behaves per the chosen contract; the existing single-field cases are unchanged.
- Aggregator test suite passes (
bun testin pm-stack/aggregator).
Distinct from: #1486 (yes/no answer values unvalidated), #3393/#3396 (impossible calendar dates), #1476 (duplicate answer widgets in the UI).
Activity
dezren39 commentedon Nov 2, 2026
🤖 Agent note · filed
dezren39 commentedon Nov 2, 2026
agent-note:v1 — task 6161 triage CONFIRMED (lane A, 2026-11-02)
Reproduction on current main (cc819fe), fresh this run. pm-stack/aggregator/src/tier-items.ts in the laptop primary clone has md5 ab984916e51351b88331212a49ac8aae — byte-identical to the creator probe base. Re-ran the probe (verbatim imports from the primary clone):
Root cause is the if/else-if chain at tier-items.ts:342-371: the optionId membership branch is unreachable whenever a non-empty answers array is present.
Route (pinned for the fix): membership-check optionId whenever it is present — hoist that check out of the else-if chain in checkActionAgainstItem. Do NOT reject combined payloads instead: answers+optionId is a legitimate combined shape once the optionId is actually checked. Do NOT extend the text branch's required-questions check to combined payloads either (would break partial survey submits, #1451). Regression tests in pm-stack/aggregator/test/needs-user.test.ts: answers-valid + unpublished optionId rejected, answers-valid + published optionId accepted, single-field payloads unchanged.
Follow-up: fix enqueued as task 6165 (small_fix, dedupe repo-health:v1:fix:developing-today/code:issue:4286). Task 6161 completes as triaged.
dezren39 commentedon Nov 2, 2026
🤖 Agent note · picked up
dezren39 commentedon Nov 2, 2026
🤖 Agent note · done — PR opened