fix(shared): proof の events 要素を構造検証し metadata 水増しを塞ぐ - #246
Merged
Conversation
検証経路は `if (!event) continue;` で falsy 要素を黙って読み飛ばす一方、 `recomputeProofMetadata` の totalEvents は `events.length` で数えていた。この非対称 のせいで、正規 proof の events 末尾に null を積み、metadata.totalEvents を配列長へ 合わせて typingProofHash を再計算するだけで、sequence / previousHash / finalHash / 署名チェックポイントの整合を保ったまま totalEvents を任意に水増しできた。末尾が null だと totalTypingTime の再計算も 0 に落ち、`claimed < recomputed` 検査まで 無力化していた (秘密情報は不要で、すべて再計算できる)。 再カウント (docs/system-spec.md §7 の Layer 1) の手前に構造ゲートを置き、events の 全要素が sequence / timestamp / type / previousHash / posw を持つ object であることを 要求する。ゲートは verifyProofMetadata に置いた — verify (web) の verificationWorker と verify-cli (verifyProofFile 経由) が共通で必ず通る唯一の入口で、1 箇所で両経路に効く。 検査するフィールドはいずれも verifyChain が既に参照しているものなので、現在検証できる proof が新たに弾かれることはない (proof フォーマットは変更していない)。 verification.test.ts の metadata 系 fixture は sequence/previousHash/posw を持たない 部分オブジェクトだったため、構造を満たす metadataEvents ヘルパ経由に変えた (アサーションは変更なし)。 Closes #221 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
🚀 Preview Deployment
Deployed from commit 5319813 |
33 tasks
shinyaoguri
added a commit
that referenced
this pull request
Aug 15, 2026
…) (#264) ## 目的 2026-08 レビュー (#243) の残件のうち shared の 2 件。どちらも「同じことを決める式が複数箇所にあり、片方が取り残される」という同型の問題なので、**文書ルールではなく機械 (tsc / 単一関数) で同期を強制する**形に直す。 ## #222 — VALID_EVENT_TYPES が union から取り残されていた `InputTypeValidator.ts` の手書き Set は **26 種**、`types/events.ts` の `EventType` union は **29 種**。機械差分の結果、欠落は次の 3 種ちょうど・余分は 0 種で、いずれも editor から実際に発火する: | 欠落 | 発火元 | |---|---| | `environmentProbe` | `tracking/EnvironmentTracker.ts:63` | | `fullscreenChange` | `tracking/FullscreenTracker.ts:143` | | `examOpened` | `ui/tabs/TabManager.ts:362` | 手書きをやめ `Record<EventType, true>` のキーから導出した。**以後 union に型を足すと tsc が Set 側の記入漏れを検出する**のが本質で、欠落 3 種が埋まるのは副産物。`VALID_INPUT_TYPES` は差分ゼロだったが、同じ drift を将来止めるため同じ形に揃えた。 ### 現時点の実害はゼロであることの確認 `validateEventType` は**どの検証経路にも配線されていません** (定義と re-export のみ)。本 PR でも配線しません — 未知イベント型を持つ将来 proof を fail-closed で弾く判断には後方互換の議論が要り、#223 と同じ論点になるためです。危険なのは「#246 で構造検証の置き場ができたので、誰かが何気なく配線した瞬間に exam proof が全滅する」という時限性で、そこは本 PR で解消されます。 ## #235 — isPureTyping の判定が 3 箇所にあった | 場所 | 式 | benign 除外 | |---|---|---| | `verification.ts` `recomputeProofMetadata` (**採点側の正**) | 非 benign bulk + 乖離 snapshot を除外 | あり | | `TypingProof.ts:822` (export 時の自己申告) | `bulkInsertEvents === 0` | **なし** | | `TypingProof.ts:867` 付近 (自己検証) | `metadata.bulkInsertEvents === 0` | **なし** | Monaco の括弧自動閉じは既定 on (`monaco.editor.create` は `autoClosingBrackets` を指定しておらず `'languageDefined'`) なので、`(` を 1 つ打つだけで**自己申告が false に落ち、採点側の true と食い違う値が proof に焼かれて**いた。 `structuralEdit.ts` に `evaluatePureTyping(events)` を新設し、`SessionProvenanceLedger` の replay と `isSuspiciousBulkInsert` / `isBenignEditorInsert` / `isFlaggedBulkInsert` / `isDivergentContentSnapshot` の組み合わせを**この関数だけが持つ**状態にした。`recomputeProofMetadata` と `TypingProof` の双方がこれを呼ぶ。 **副次的な効果**: export 側は逆に緩すぎた面もあり、複数行の一括投入 (Monaco が `insertParagraph` で記録するため `isSuspiciousBulkInsert` に載らない) が申告に反映されていなかった。一本化でこちらも塞がる。 ### `TypingProof.ts:867` の扱い 呼び出し元を全パッケージで grep したところ **このメソッドは未使用**で、引数は `proofData` のみ・events を入手する経路がありません (任意の proof を受ける API なので `this.events` を代用するのは不正)。そのため寄せず、JSDoc に「3 つ目の定義ではない。events が無いためメタデータのみの粗い自己チェックで、採点側とは一致しない (こちらの方が厳しく false に倒れる)。判定値は `verifyProofMetadata` から取ること」を明記した。 ### 後方互換 - `metadata.isPureTyping` は `typingProofData` に含まれず `typingProofHash` の入力ではない (`types/proof.ts` の 154-177 行 vs 245-249 行で確認) → **値が変わっても既存 proof のハッシュ照合に影響しない** - `metadata.bulkInsertEvents` は `isSuspiciousBulkInsert` の素のカウントのまま。`verifyProofMetadata` の完全一致検査は不変 (ここを変えると既存 proof が invalid になる) - `isSuspiciousBulkInsert` 本体も未変更 ## 確認方法 テストを先に書き、**実装前に赤くなることを実測してから**仕上げた。 | テスト | 修正前 | 内容 | |---|---|---| | 括弧自動閉じだけのセッション → `isPureTyping` true | 🔴 `expected false to be true` | 本命 (#235 の再現) | | 複数行の一括投入 → false | 🔴 `expected true to be false` | 境界。export 側が素通しだった laundering 口 | | `evaluatePureTyping` と `recomputeProofMetadata` の結論一致 | 🔴 `expected true to be false` | 2 経路の固定 | | `VALID_EVENT_TYPES` が union と同集合 | 🔴 `environmentProbe: expected false to be true` | #222 の回帰 | 修正前から緑だったケース (input types 側) は、`INPUT_TYPE_MEMBERS` から要素を削る / union 外の値を混ぜる、を実際に試して赤くなることを確認済み。 ```bash npm run lint && npm run test:run --workspaces --if-present && npm run build ``` すべて green。shared は 475 passed (修正前 468 + 新規 7)。**`webCliParity.test.ts` は 12 件 green のまま** — ここが赤くなったら判定の一本化に失敗しているという意味で、本 PR の一番重要なゲート。 `packages/shared/CLAUDE.md` に不変条件 8 (`isPureTyping` の判定点は 1 つ) を追記し、型導出で不要になった手動同期の記述を更新した。 Closes #222 Closes #235
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
目的
events配列の要素型が未検証で、null要素によりtotalEvents/totalTypingTimeを水増しできる問題 (#221, security/high) を塞ぐ。検証経路 (
verifyChain/recomputeProofMetadata/verifyContentReplay) はif (!event) continue;で falsy 要素を黙って読み飛ばす一方、recomputeProofMetadataのtotalEventsはevents.lengthで数えていた。この非対称のため、秘密情報なしで次の手順が成立していた。events末尾にnullを大量に積むtypingProofData.metadata.totalEventsを配列長に合わせるtypingProofHashを再計算するsequence / previousHash / finalHash / 署名チェックポイントはすべて実イベントのみで整合するため、
valid: trueのまま totalEvents を任意に水増しできた。さらに末尾がnullだとtotalTypingTimeの再計算がevents[events.length-1]?.timestamp ?? 0→ 0 に落ち、claimed < recomputed検査も無力化していた。docs/system-spec.md §7 の攻撃マトリクス「metadata の偽装は Layer 1 の再カウントで検出」という保証が破れている状態だった。
変更点
packages/shared/src/verification.tsverifyEventArrayStructure(events)を追加。eventsが配列であり、全要素がsequence(有限数) /timestamp(有限数) /type(string) /previousHash(string | null) /posw(object) を持つ object であることを検証する。最初の違反要素の index をerrorAtで返すverifyProofMetadataの先頭でこのゲートを通す。通らなければ再カウントを行わずvalid: falseで返すpackages/shared/src/index.ts:verifyEventArrayStructure/EventArrayStructureResultを公開packages/shared/src/__tests__/eventStructureGate.test.ts(新規): 攻撃再現と構造検証の単体テストpackages/shared/src/__tests__/verification.test.ts: metadata 系 fixture を構造を満たすmetadataEventsヘルパ経由に変更 (アサーションは変更なし)ゲートを
verifyProofMetadataに置いた理由Issue の修正案は「parse 直後の入口」だが、verify (web) と verify-cli が共通で必ず通る入口は parse 段階には存在しない。
packages/verify/src/workers/verificationWorker.tsが shared の検証関数群を直接呼ぶ。verifyProofFileは使わないfileProcessing/parser.ts(isProofFile) を使わない (verify 独自のFileProcessor/ZipFileProcessor)verifyProofFile経由両者が共通で必ず通り、かつ壊れている保証 (Layer 1 の再カウント) そのものを担うのが
verifyProofMetadataなので、ここ 1 箇所に置いた。web/CLI どちらもmetadataValid = false→ 全体 invalid に伝播する。既存の
if (!event) continue;は防御的コードとしてそのまま残している。後方互換
検査する 5 フィールドはいずれも
verifyChainが既に無条件で参照・比較しているもの (event.sequence/event.timestamp/event.previousHash/event.posw.iterations) なので、これらを欠く proof は現時点でも検証を通らない。したがって現在検証できる正規 proof が新たに弾かれることはない (MIN_SUPPORTED_VERSION1.0.0 の proof を含む)。proof フォーマット自体は変更していない。確認方法
先に再現テストを書き、修正を一時的に無効化して赤くなること (攻撃シナリオ 5 件が fail) を確認してから仕上げた。
追加テスト (
eventStructureGate.test.ts, 13 件):nullを積んでtotalEventsとtypingProofHashを再計算した proof が reject される (攻撃再現)totalTypingTimeが申告値の照合に使われないverifyEventArrayStructureの単体 (空配列は valid / 最初の違反 index / 非配列 / posw 欠落 / sequence 型違反 /previousHash: nullは許容)残課題 (このPRの範囲外)
summarizeProcess(packages/shared/src/processSummary.ts:147のevents[i]!) など advisory 層はnull要素で TypeError を投げうる。verify-cli は本 PR の後も「検証は invalid と判定 → その後 advisory 層で例外 → cli.ts の catch で exit 1」という経路になり、判定自体は正しいがメッセージが分かりにくい。別 Issue にするのが妥当 (#221 でも副作用として言及されている)。Closes #221
🤖 Generated with Claude Code