# Plan Review Panel — samsung-account-intake-rowfix

**Plan**: `.claude/plans/samsung-account-intake-rowfix.plan.md` · **Plan version**: `sha256:6870f84596052813e184d5ce2c63876a1bdc03b6b09dea647c4c8fbda5782ecb`
**Verdict**: `converged` via `multi-agent`
**Quorum**: 4/3 responses · 4 distinct roles (of 4 fielded) · passed=true
**Layers**: L1 converged · L2 converged · L3 not fired

> Reason: L1 + L2 quorum satisfied (4/3 responses, 4 distinct roles); L3 not fired

## Findings

| Perspective | Severity | Claim | Evidence |
|---|---|---|---|
| test | MEDIUM | Part of the Risk-table mitigation is false and no test covers it: 'even if the edits differ, the M5 mismatch check blocks it as changed'. The selection tuple is [row number, login ID, verdict]. A confirm call whose password or phone edit differs from the previewed one, while the verdict stays the same (e.g. still create), produces no mismatch. The row is then written with the unpreviewed value. | Plan Risks row 1 says '수정이 달라도 M5 괴리 판정이 changed 로 막는다(UI6) · `#4` endpoint it'. Observation 7 says '선택 목록은 `[행번호, 로그인아이디, 판정]`'. Endpoint it (1) in gates.mjs:66 only tests preview and confirm with the *same* edits, so no required it exercises confirm with a divergent rowEdits. Only the frontend lock (#6, static) protects this path. |
| test | LOW | The #5 counterfactual shows that the merge statement matters to the edits spec only. It does not show that confirm's write path (writeCreate/writeRevive via fileRows, Observation 9) receives the merged rows. #5 runs only samsung-intake-edits.spec.ts. | Plan Validation #5: 'npx jest … "$CD/samsung-intake-edits.spec.ts"'. The endpoint 'dataUpload 가 고친 비밀번호를 받는다' assertion is covered only if it lives in that same spec file. The Files table puts the endpoint its in edits.spec, and gates.mjs:85 lists the endpoint it among the expected reds, so this is probably covered. Severity stays LOW. |
| invariant | MEDIUM | The Risks table says that if the edits sent at confirm differ from the ones judged at preview, the M5 divergence check (UI6) blocks the row as `changed`. That is false for password and phone edits. The anchor compares only loginId and verdict, so a different password or phone that yields the same verdict (`create`) is applied silently. The server never binds confirm's `rowEdits` to what the preview judged. The only thing holding this is the client sending `judgedEdits`, and no server-side gate checks it. | insuniverse-server/apps/api/src/modules/fc/agency/samsung-intake-confirm.ts:109-112 (`unchanged = row !== undefined && row.loginId === selection.loginId && row.verdict === selection.verdict`). Plan Risks row 1: "수정이 달라도 M5 괴리 판정이 `changed` 로 막는다(UI6)". Plan DD2: "확정 때 수정이 다르면 그 행은 `changed` 다". None of the endpoint it cases in Task 4 exercises a confirm that sends a different password or phone than the preview. |
| invariant | MEDIUM | The skip predicate in check #1 accepts any existing baseline file as proof that a baseline was taken. If `rowfix.baseline.json` exists, `#1` skips `clean` and re-freezing and only reports. A stale baseline, for example one captured before the colmap commit or while the tree was dirty from an earlier attempt, would then widen what `#2`, `#3` and `#7` tolerate. The plan does not say that `baseline-report` checks the recorded base commit against the colmap commit it depends on. | Plan Validation lines 286-293: `if [ ! -f "$B" ]; then node "$G" clean ... fi; node "$G" baseline-report "$B"`. Expected output on line 283: "기준선이 있으면 다시 뜨지 않고 보고만 한다". Task 1 requires the colmap work to be committed first but gives no machine check that ties the baseline to it. |
| invariant | LOW | The rollback claim cannot be restored by a code revert once confirm has written edited values. The plan admits this in its rollback risk row, and no test pins that requests without `rowEdits` behave exactly as before; the existing preview and confirm specs are changed in the same window. | Plan Risks row: "`rowEdits` 없는 요청은 지금 동작 그대로. **반영된 운영 데이터는 코드 되돌림으로 안 돌아온다**". Task 3 changes the fake repository and the `toEqual` row shape in the preview spec, so `#2` compares against those changed specs, not against the untouched pre-change behaviour. |

## Refutation attempted

| Perspective | Verdict | What was attacked |
|---|---|---|
| architect | pass | I checked the claims the plan leans on in samsung-intake.service.ts, fc-agency.service.ts and agency-fc.repository.ts, and found no defect in the plan's structure. 1) DD2 says there is one place where edits get applied. Grep found `rejudge` defined at samsung-intake.service.ts:472 and called only from preview (:448) and confirm (:560). Confirm builds `fileRowOf` from the `fileRows` that `rejudge` returns (:585), and the writes read their password and phone from that map (:740, :776, :821). So edits applied inside `rejudge` right after the file is read reach the writes without a second write path. The plan's observation 9 holds. 2) DD5 says the preview uses the same method with the same values as confirm validation. `duplicateInDatabase` (fc-agency.service.ts:2118-2178) passes raw upload phones from `extractUploadIdentifiers` (:2449-2455) to `checkDuplicate` and strips hyphens only in the comparison (:2165). That matches the plan's call with the raw phone and hyphen-stripped matching. `checkDuplicate` exists at agency-fc.repository.ts:312. 3) The member-lookup error handling the plan says to mirror is at service.ts:499-507 as cited. The login-ID lookup keys are built from `rows` in `rejudge` (:489-495), so applying edits and exclusions before that point also changes which keys are queried, as Task 4 says. 4) Boundary: the new edits module stays pure (no DB or Nest), the database query stays in the service, the front end keeps the confirm lock as one function (`isConfirmBlocked`), and the plan does not modify `fc-agency.service.ts` or the repository (NOT Building, UI8). 5) I found no second owner of the edit state (DD9 puts it in `IntakePreview`). I also found no new verdict value without a consumer: the label map is a `Record` keyed by verdict, so a missing label fails tsc, which gate #3 runs. |
| security | pass | Checked the plan at .claude/plans/samsung-account-intake-rowfix.plan.md (lines 1-363) for five ways to attack it. I did not read the source modules it cites or its last 75 lines, so the claims below about the existing code are taken from the plan's own descriptions. 1. Forged or diverging rowEdits between preview and confirm. An admin could send confirm a password edit that preview never judged. The selection tuple [row, loginId, verdict] (plan GROUND #7) would not catch that. It is not an escalation, though: the same admin could already put any password in the file. Confirm still runs excelValidate/duplicateInDatabase on each created row (GROUND #2), and changing a loginId re-matches only inside the selected agency (DD5, rejudge). So there is no cross-agency path and no power the file upload did not already give. 2. The plaintext password edit leaking into durable outputs. DD6 covers four routes: judged rows carry no password field; the alarm and log record only counts; request.body.rowEdits is added to the pino redact list (the multipart field is not logged); and maskPassword reads the merged row (GROUND #9). The endpoint test (3) and the log test assert the password is absent. No route leaks the value. 3. Injection or traversal through edit values. parseRowEdits allows only whitelisted keys, values of 255 characters or fewer, and integer row numbers, and rejects any row not in the file (E0824). The plan says the values are used only for equality checks and writes, never joined into queries or paths. checkDuplicate goes through drizzle inArray, so values are bound parameters. No path to a consequence. 4. Partial or empty edits. An empty object is rejected. exclude is kept apart from value merging (DD2). Unknown-subscription rows can only be excluded, not edited. No weaker fallback exists. 5. Phone exposure (DD7). The phone number goes back only to the admin who uploaded the file. This matches the earlier M5 decision and is not a new trust boundary. Nothing I tried reached a real consequence. |
| test | pass | Read the plan's Tasks and its Validation block #1–#7, and the required-it list in gates.mjs:59-90. Checked 4 things. (1) Each Validate line runs the spec it changes. (2) The #4 names cover every Acceptance line. (3) The #5 counterfactual targets the merge anchor, with the expected red/green split. (4) The over-permissive direction: confirm with edits that were never previewed. The DD8/Risk claim that a divergent edit becomes `changed` has no test, and the tuple shape shows it only holds when the verdict changes. The frontend lock is covered only statically. No HIGH-severity gaps found. |
| invariant | pass | I read the plan and the confirm module the gate relies on. 1) I attacked the claim that the `planConfirm` loginId/verdict anchor also covers edit contents; it does not (MEDIUM). 2) I checked whether the one `checkDuplicate` call can only check one row; the repository signature takes arrays, so one call covers all rows and there is no fail-open. 3) I traced the unknown-input path for `rowEdits`: E0824 covers malformed JSON, unknown keys, rows not in the file and empty objects, and an E0824 goes to recovery, not to approval. 4) I checked the confirm lock for an open editor or pending edits (`isConfirmBlocked`). 5) I checked the `#1` baseline skip predicate (MEDIUM, stale baseline). 6) I checked that the `#5` counterfactual copy is deleted by the EXIT trap. 7) I checked that the `#2` machine-failure path goes red on an empty report. I found no path where a failure of the gate machinery ends in an approval. |

## Measurement

<!-- Written by plan-review/cli.js record on EVERY exit path, pass or halt.
     Machine-readable; do not hand-edit. A null field means the axis was
     not observed, never that it was zero. -->

```json
{
  "verdict": "converged",
  "source": "multi-agent",
  "layers": {
    "l1": "converged",
    "l2": "converged",
    "l3": "not fired"
  },
  "quorum": {
    "responded": 4,
    "required": 3,
    "roles": 4,
    "of": 4,
    "passed": true
  },
  "wall_clock_ms": 52422,
  "halt_stage": null,
  "backlog_appended": null,
  "backlog_skipped_nonblocking": null,
  "granted": 4,
  "reviewed_plan_hash": "sha256:6870f84596052813e184d5ce2c63876a1bdc03b6b09dea647c4c8fbda5782ecb",
  "plan_path": ".claude/plans/samsung-account-intake-rowfix.plan.md",
  "recorded_at": "2026-09-21T07:31:22.311Z"
}
```