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

**Plan**: `.claude/plans/samsung-account-intake-m4.plan.md` · **Plan version**: `sha256:4c43d7b5c798d087700071adf41806763dbbcef3bb17f96307d84445314a4309`
**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 |
|---|---|---|---|
| security | LOW | The unscoped lookup has no agency filter, so `others[]` and `otherAgencyCodes` can show codes (and possibly names) of any agency, not just Samsung's s1/s2/s3. Any agency that shares a login ID with the file will appear. The only reader is the allowlisted internal admin, so this is residual disclosure, not escalation. The plan never states that non-Samsung agencies can appear in the response. | plan.md Task 3: "WHERE 는 `.where(inArray(tnMember.memLoginId, loginIds));` 한 줄(기업 조건 없음)" with inner join on tnAgency and selected columns including "기업코드·기업명"; Task 2 summary "`others[]`(매칭 많은 순)"; guard is class-level `@UseGuards(AdminJwtAuthGuard, SamsungIntakeAdminGuard)` at samsung-intake-admin.controller.ts:26 |
| test | LOW | The wiring test only checks that the two format checks are called. It does not check that their results decide the verdict. If `judgeIntakeRows` called `isValidLoginId`/`isValidPassword` and then ignored the return value (the over-permissive direction), `#5` would still be green, and no `#8` counterfactual covers this axis. | plan.md Task 3: "배선 it 은 `AgencyFcService.prototype` 의 두 형식 검사에 spy 를 걸고 합성 픽스처 preview 가 그 메서드를 부르는지 본다". gates.mjs:104-113 COUNTERFACTUAL has only 3 mustFail axes (isPublic, repository WHERE, mismatchSuspected), none for format-check results. The plan discloses that other assertion content is left to review (§NOT Building 'spec 단정 내용'). |
| test | LOW | Tests for rule collisions (invalid format plus duplicate, duplicate plus member of the selected agency, resigned plus unknown subscription state) are folded into existing it blocks. `#4` checks only it names and assertion count >= 1. Deleting those extra assertions would leave every gate green. | plan.md Task 2: "규칙 사이 충돌은 it 안에서 함께 단정한다". gates.mjs:239-241 counts only `numPassingAsserts >= 1` per title. This is disclosed under §NOT Building 'spec 단정 내용' and blind-spot registry gates.mjs:122. |
| invariant | MEDIUM | Task 1 offers two ways to clear the unrelated prettier change in the server submodule: revert it, or commit it separately. Committing it separately moves the server HEAD away from the hard-coded SRV_BASE, so check #1 can never pass again (and neither can the checks after it). The only fix would be editing SRV_BASE in the plan, which changes the plan hash after the receipt is sealed and leaves the receipt stale. The gate stays closed, so this is not a gate that opens. But one of the plan's two sanctioned options leads to a dead end that breaks the receipt anchor. | plan.md:139 "관측 2 의 prettier 변경을 **사람이 먼저 정리한다**(되돌리거나 별도 커밋 …)" vs plan.md:243 `SRV_BASE=a4bb84fa…` and plan.md:268 `[ "$(git -C "$S" rev-parse HEAD)" = "$SRV_BASE" ] \|\| { echo "#1 FAIL: 서버 HEAD …"; false; }`; plan.md:64 observation 2 says the change is in the server submodule |
| invariant | LOW | In check #8, the mustPass partner for the repository axis is the `agencies:` it. That it uses a fake repository, so its passing does not prove the repository counterfactual copy was actually used. If the copy fails at runtime inside the `repository:` it for an unrelated reason, the mustFail still turns red and #8 reports that the check discriminates. A copy that fails at import time is still caught as a broken suite, so the remaining gap is small. | gates.mjs:109-113 mustPass lists 'agencies: …' for the repository axis; plan.md:168-169 "`agencies:` it 과 `preview:` it 은 가짜 리포지터리 객체를 쓰고, 실제 `AgencyFcRepository` 는 `repository:` it 만 만든다"; gates.mjs:306-307 catches only load-stage breakage |

## Refutation attempted

| Perspective | Verdict | What was attacked |
|---|---|---|
| architect | pass | I checked the citations the design leans on, and each one says what the plan claims. agency-fc.repository.ts:344 `getAgencyListByCorporateCode(corporateCodes)` selects agcId, code and name. fc-agency.service.ts:2391/2396 are private regex checks that do not use `this`, so passing them unbound is safe, and 1040 is the existing upload loop. fc.module.ts:36-44 already provides AgencyFcService and AgencyFcRepository and exports only FcService (observation 20), so adding one provider is enough for dependency injection. samsung-intake.dto.ts:8-18 has SamsungIntakeParseDto with `intent` and `filePassword`, so the preview DTO can extend it and keep the same field name that log masking depends on. I tested whether the unscoped query is really fenced to one caller. The fence rests on the module not exporting the repository plus the `#5` callers file check. A service inside FcModule could still inject the repository, but it would have to name the method, and grep-based `#5` would catch that. I checked whether the copies `#8` writes (the repository copy includes the method name) could make `#5` wrong. `#5` runs before `#8`, and a copy left behind by an interrupted run makes the check fail, not pass, so the result stays correct. I looked at layering: the pure judge function gets the format checks as function arguments with no DB or Nest, the controller calls a service rather than the repository (observation 8), and the parse spec's constructor change is already in the file table (observation 9). I looked for stranded or missing consumers. The subscription union has an `unknown-subscription-state` review item, and M5's reuse of the pure function is left to M5 on purpose (UI4). I found no defect with evidence behind it. The earlier architect points from the fan-out (bracket access to private methods, DTO inheritance, one function with five responsibilities, the IN-list limit) were already ruled on, with reasons, in the plan's Fan-out ruling table (F9-F12), and I found no new evidence against those rulings. |
| security | pass | Checked whether the new handlers skip the guard. The guard is set on the class (controller.ts:26), and gate #8's counterfactual copy adds isPublic to preview. Checked the file-password logging path. pino.ts:54 only masks `request.body.filePassword`. Preview reuses the same field name because its DTO inherits the parse DTO. The multipart path is covered by the M3 log its, which gate #5 requires (parse.spec.ts:144-181). Tried to get the plaintext member passwords into the response: the row result type has no password value key, and gate #5 checks the parse output as a control and gate #9 checks the front-end types. Checked whether member names or phone numbers from other agencies leak. The query's select excludes them and `name` comes from the file (Task 3 select assertion). Tried spoofing agencyCode: `@IsIn` on the 3 codes plus a DB lookup that returns E0820, and sf is excluded. Tried to make the unscoped query spread to other callers: FcModule does not export the repository (fc.module.ts:44) and gate #5 `callers` restricts which files can use it. Checked whether counterfactual copies left behind could get committed: gate #7 counts untracked server files as out-of-scope changes. Nothing is stored between preview and confirm (UI3), so there is no forged-state or partial-record trust surface. No absolute paths or secrets go into committed artifacts beyond the SHA pins that already existed. Only residual: agency codes outside Samsung's can show in the counts, and only to the allowlisted admin (LOW). |
| test | pass | Read the whole plan (Tasks, Validation #1-#10, Acceptance, fan-out rulings) and the matching parts of gates.mjs: SPEC_GROUPS, COUNTERFACTUAL mustFail/mustPass, write-cf and its anchors, the write-cf-config moduleNameMapper, spec-names (catches empty its and suites that die at load), jest-regress (compares per-test IDs), tsc-regress (compares error signatures plus the scope regex), callers, admin-tsc and scope-check. I attacked 5 things. 1) Could the isPublic counterfactual miss the new handler? No. The existing access spec (samsung-intake-access.spec.ts:29-40) builds its handler list from the prototype, so a copy with isPublic on preview turns it red. 2) Could the repository counterfactual pass without really testing? No. The copy adds `eq(agcId, 0)`, and the it asserts that `agc_id` is absent from the rendered SQL. 3) Does observation 9 (the constructor argument-count error in the parse spec) actually reach tsc? Server tsconfig.json has no include/exclude, so spec files are compiled, and SRV_SCOPE_RE covers modules/fc. 4) Are the mustPass/mustFail pairs independent? The judge per-agency-count it deliberately does not assert mismatch, and the agencies/preview its use fake repositories. 5) Is the over-permissive direction tested? Yes: sf and codes outside the allowed list are rejected, the unscoped query is confined to 3 files by `callers`, and password-value absence has a control group. Task 4 frontend behaviour has no automated test, but the plan discloses that as (미검증) under UI10. I found no HIGH defect that can be backed by evidence. `#3` failing on any fc-scope tsc error is only unpassable if fc errors already exist at baseline, and I could not check that without running tsc. |
| invariant | pass | Checked the gate script's failure paths for missing or damaged inputs. A missing jest report or one with 0 tests fails (gates.mjs:28,133,166). Server tsc with a non-zero exit code and no readable errors fails (gates.mjs:135,177). A frontend tsc run whose program is empty or missing the M4 files fails (gates.mjs:138,210-213). A suite that dies while loading counts as a `::(suite)` failure (gates.mjs:26,230). A test with no assertions fails (gates.mjs:240). A counterfactual anchor that appears 0 or 2+ times fails (gates.mjs:275,288). A missing or cut-off baseline.json fails in baseline-report (gates.mjs:156). If the process dies mid-write, tryJSON returns null and the check fails. The #1 skip that happens when baseline.json already exists still checks both submodule HEADs (plan.md:268-269, gates.mjs:157-158). Tampering with the baseline is openly listed as a blind spot (gates.mjs:124). Leftover counterfactual copies from a killed run are untracked files, and #7 flags them as out of scope (gates.mjs:248). The credential-file check applies no matter what the file table says. The PRD status value can be overridden by env, but the gate still fails if the value doesn't match. The counterfactual jest config fails closed if the base config can't be required. Tried to find a path that ends in approval when a tool is missing, an artifact is missing, or a write is cut off, and found none. The only defect is the Task 1 option that leaves SRV_BASE unsatisfiable. It fails closed but forces a plan edit after the receipt, which breaks the receipt anchor. |

## 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": 110446,
  "halt_stage": null,
  "backlog_appended": null,
  "backlog_skipped_nonblocking": null,
  "granted": 4,
  "reviewed_plan_hash": "sha256:4c43d7b5c798d087700071adf41806763dbbcef3bb17f96307d84445314a4309",
  "plan_path": ".claude/plans/samsung-account-intake-m4.plan.md",
  "recorded_at": "2026-09-17T06:55:22.104Z"
}
```