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

**Plan**: `.claude/plans/samsung-account-intake-colmap.plan.md` · **Plan version**: `sha256:f87bc82d8331ac0aa43dfdeb02367e3d6d59c9dba06babb3251b04aa19041dfb`
**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 |
|---|---|---|---|
| architect | LOW | DD5 (a confirmed layout turns off the header warning) is written twice. The parse controller forces `headerUnexpected` to false on its own, and `judgeIntakeRows` adds `layoutConfirmed`. The two paths can drift apart, because no single predicate owns the rule. | Plan Task 3: "`fields` 까지 받은 parse 는 응답 `headerUnexpected` 를 false 로 둔다" (controller) and separately "`headerUnexpected = !input.headerExpected && !input.layoutConfirmed`" (service). Current code has the same split: samsung-intake-admin.controller.ts:92 and samsung-intake.service.ts:336. Spec `endpoint:` covers both paths, so the drift risk is limited. |
| security | LOW | The client-supplied `layout` field turns off the header-suspicion check (E0822). Any caller that sends `fields` skips the 'form suspicion' confirmation, and the server has no way to know a person actually looked at the columns. | plan Task 3: `headerUnexpected = !input.headerExpected && !input.layoutConfirmed` and 'fields 까지 받은 parse 는 응답 headerUnexpected 를 false 로 둔다'. DD5 gives the reason. |
| security | LOW | Residual password exposure: a column with no header whose passwords break the format (no special character) and that nobody picks as password shows its samples on screen. The plan discloses this. | plan §NOT Building '가림 판정의 완전성' and the Risks row '가림 밖으로 비밀번호가 화면에 보인다'. The viewer is the admin who uploaded the file. |
| test | MEDIUM | Gate #5 can prove only that the whole masking condition does something. It cannot prove each of the three hiding reasons in DD4 is tested on its own. The counterfactual replaces the entire `const hidden = <expr>;` with `false`. That means code that drops branch (b), the header-alias rule, or branch (c), the value-shape rule, can still pass #4 and #5 if the mustFail it's fixture has a column that another rule also hides. The plan never says each branch needs its own fixture column with its own `hiddenReason` assertion. The Risks table names (c) as the mitigation for exactly the unlabeled-password-column case. | .claude/plans/samsung-account-intake-colmap.gates.mjs:85-91 (re `/const hidden =[^;]+;/` → `const hidden = false;`, mustFail has only 2 its). Plan Task 2 step 4 and DD4 '(a)(b)(c) 셋 중 하나면 참'. The required it name at gates.mjs:61 covers both kinds of column in one it: '비밀번호로 보이는 칸과 비밀번호 열은 …'. Risks row: 'DD4 (a)(b)(c) 세 겹 · `#4` 가림 it · `#5` 반사실'. |
| test | LOW | The parse-endpoint check that the password is absent is a real check against the real producer. It runs `JSON.stringify` over the whole response and uses an encrypted fixture that contains `Synth!0001`/`Synth!0003`, so it covers the new `layout` field. It does not run in the #5 counterfactual, though. The fixture header is the canonical '비밀번호', so on this path rules (a) and (b) both hide the column and neither is tested on its own. This is a minor gap. | insuniverse-server/apps/api/src/modules/fc/agency/samsung-intake-parse.spec.ts:61-80 (not.toContain loop at line 79), gates.mjs:75 (the parse it is required in #4), gates.mjs:303-305 (the counterfactual runs only samsung-intake-layout.spec.ts) |
| invariant | MEDIUM | The E0822 header-suspicion gate is switched off by the mere presence of a `layout` multipart field. The server has no evidence that a person confirmed anything; the client only asserts it. Any preview/confirm request that carries a well-formed layout skips `headerUnexpected`. The plan reasons that a person picking column uses counts as the header check, and the frontend only sends a layout after confirmation or for a canonical file. That makes this a deliberate trade-off rather than accidental drift. Still, the skip predicate relies on the client's word, not on proof the check happened, and no gate pins down that the frontend only sends an unconfirmed layout on the canonical path. | plan Task 3: "`headerUnexpected = !input.headerExpected && !input.layoutConfirmed` 로 둔다(DD5)"; DD5 "`layout` 을 받은 판정은 헤더 의심(`headerUnexpected`)을 켜지 않는다"; #6 front-contract (gates.mjs:242) only checks, by regex, that canonical appears within 400 chars of <IntakeColumnMap>. It does not check that onConfirm is the only path that sets confirmedLayout. |
| invariant | LOW | #1 baseline reuse does not check whether the existing baseline file was captured from the current starting point. If a baseline.json is left over from a different launch, it is reported ok and trusted by #2, #3 and #7. It is only validated as three 40-hex hashes. | plan Validation `if [ ! -f "$B" ]; then ... fi; node "$G" baseline-report "$B"`; gates.mjs:154-160 baseline-report only regex-checks base_commit/srv_base/adm_base. |

## Refutation attempted

| Perspective | Verdict | What was attacked |
|---|---|---|
| architect | pass | 1. Checked the cited anchors and found them accurate. `readSheet` is at xlsx.ts:232. `headerExpected` is at xlsx.ts:255 and is only an input to the warning. `judgeIntakeRows` is at service.ts:298, with `headerUnexpected` at service.ts:336. `parseSelections` is at confirm.ts:70. The parse controller calls no service and does not return header values (controller.ts:77-94). 2. Attacked DD3, which says a request without a layout behaves as today. `readIntakeRows(buf, pw, layout?)` is optional and keeps its return shape, so the existing callers (controller:77, service:477) still work. 3. Attacked DD10, which says the file is opened once. Parse uses `readIntakeLayout` for rows and view together. `rejudge` uses `readIntakeRows` only. No path decrypts twice. 4. Attacked the password-hiding boundary. The view goes through a single `hidden` expression (checked by `#5`). Parse still returns only counts, never rows. 5. Attacked DD5 against the sheet-name intent check (E0822). `sheetIntentMismatch` is based on the sheet name, and the chosen sheet name still flows through, so that check stays independent. 6. Attacked the purity of the layout module. It does not use DB or Nest. The only coupling is `canonical` depending on the existing `headerExpected`, which is intended. 7. Attacked the stateless-layout claim (UI6). The layout is re-sent with every request, and the front end appends it in 3 places, checked by `#6`. 8. The only leftover is the duplicated DD5 rule above (LOW). |
| security | pass | Attacked the DD4 masking with a data row chosen as the header row. Clause (c) now checks the header cell value too, and #5 counterfactual covers it. Checked for layout JSON leaking into logs: it goes in a multipart field and holds no secret. Checked for out-of-range sheet, column and headerRow values: E0823 covers these with explicit bounds 1-26 and 0-10, the sheet index is checked against the sheet count, and the DTO has MaxLength 2000. Checked whether the layout could be forged to escalate privilege. It only changes which columns are read on an admin-only endpoint that already needs the file and its password. Preview and confirm both re-read through the same rejudge path, and M5 marks a mismatch as changed. Checked the spec fixtures for durable leaks: they use synthetic Synth!00NN passwords built in code, and #7 blocks xlsx attachments. Found no path from hostile input to a HIGH consequence. |
| test | pass | I read the plan and the required-it and counterfactual tables in the gates script (#4 SPEC, CF). I also read the password-absence it and its control it in the parse spec. Claims I attacked: 1) masking stops password leaks (DD4). The counterfactual is coarse, so each branch is not tested on its own (reported above). 2) Canonical files skip human confirmation, checked in the over-permissive direction. The parse spec's `toEqual` pins `canonical:false` for a non-canonical fixture, so a `canonical` that is always true would fail that check. 3) DD5 turns off header suspicion only when a layout is given. The required its 68, 71 and 73 check this in both directions: a layout is present, and no layout reads canonical positions. 4) Validate lines against the changed files. Task 2 and Task 3 run jest on the new spec and the whole fc/agency suite. The frontend is covered only by tsc and the static contract (#6), and the plan declares that. 5) Existing tests that encode the old behaviour. The sheet-two rejection and the header-less row-1 check stay, because the path with no layout is unchanged (DD3). The parse spec's `toEqual` change is declared up front. I found no HIGH defect. |
| invariant | pass | 1. Fail-open behaviour of the gate machinery. I checked gates.mjs jest-regress (lines 162-169): a missing or empty report fails via jestReadable, not green. tsc-regress (170-189): a non-zero rc with no parsable signatures fails, and files not loaded into the program fail. write-cf (191-204): an anchor count other than 1 fails. counterfactual (205-217): suite load failures, missing required it blocks and a vacuously green copy all fail. front-contract (220-253): a missing file fails. scope: a missing baseline fails. 2. The #1 clean-only baseline and the path that reuses an existing baseline. 3. Rollback: with no layout, DD3 keeps the old positional path, the E0819 two-sheet rejection and the observation-6 specs, which are exercised under #2. 4. The requireFields asymmetry: parse uses false and rejudge uses true, so a confirm without fields is rejected with E0823 and does not silently fall back to position. 5. Whether the E0822 skip predicate is anchored. It is anchored only by the client. 6. Password-masking fail direction: a single `const hidden` sentence covered by the #5 counterfactual. No HIGH/CRITICAL fail-open path found. |

## 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": 126623,
  "halt_stage": null,
  "backlog_appended": null,
  "backlog_skipped_nonblocking": null,
  "granted": 4,
  "reviewed_plan_hash": "sha256:f87bc82d8331ac0aa43dfdeb02367e3d6d59c9dba06babb3251b04aa19041dfb",
  "plan_path": ".claude/plans/samsung-account-intake-colmap.plan.md",
  "recorded_at": "2026-09-21T07:30:13.661Z"
}
```