Skip to content

refactor: user-IPC 認可の realpathCache を撤去し毎回 fresh 解決にする (#453) - #460

Merged
ymnao merged 5 commits into
mainfrom
refactor/drop-realpath-cache
Aug 1, 2026
Merged

ymnao merged 5 commits into
mainfrom
refactor/drop-realpath-cache

Conversation

@ymnao

@ymnao ymnao commented Aug 1, 2026 •

Copy link
Copy Markdown
Owner

概要

user-IPC 認可 (assertPathAllowed / assertWritePathAllowed / isPathAllowed / isPathWithinAnyAllowedRoot / canonicalize) が持っていた realpath 結果の LRU cache (上限 256・invalidation 無し) を撤去し、毎回 fresh に realpath するようにした。#453 の「判断が要る点」に対する結論は cache を捨てる。

判断の根拠 (いずれも本セッションで確定させた事実):

  • **cache key は「realpath に成功した path 自身」**だった。実在 file では full path 1 本が載るだけで、鮮度を保存するのは「同じ path の 2 回目以降」に限られる。初回アクセスは元から fresh なので、symlink retarget への追随が「その path を過去に触ったか」という観測不能な条件で変わっていた
  • invalidation は持てない。retarget の通知経路が無い (chokidar は followSymlinks: false で retarget を change event として emit する保証がない)。これは L3 index 取り込みゲート resolveInsideRoot が元から cache を通していない理由 (394 Phase D follow-up B: realpath cache invalidation on watcher batch + L2-miss TOCTOU #406 Finding 1) と同根で、user-IPC 認可だけ別扱いにする根拠が無かった
  • 削減できていたコストが小さい。認可 1 回につき realpath 1 回で、node probe の実測は深さ 8 の path で 21.9 µs (macOS warm dcache、symlink を 1 段挟んで 31.3 µs)。fs:read の open + stat + read に埋没する。call site はすべて「IPC 1 回 / 画像 1 枚 / workspace open 1 回」につき定数回で、ループ経路は無いことを確認済み
  • 境界が緩む方向の変化は無い。祖先 retarget が外を指せば fresh 解決は拒否 (現状より厳しい)、内を指せば新しい解決先で許可 (ユーザーの見え方と一致)

副産物として、cache の存在を前提にしていた機構が消える: invalidateRealpathCacheEntry と fs.ts の withStaleCacheRetry (ELOOP を「cache stale」/「認可後 swap」に切り分けて 1 度だけ再認可する helper)。cache が無ければ認可時点で canonical の末端が symlink であり得るのは realpath が解決できないとき (dangling / 循環) だけなので、ELOOP はそのまま fail-closed で伝播させる。

関連 Issue

closes #453

移行 Stage

  • Stage 6: 仕上げ・配布・切り替え

変更内容

  • path-guard.ts: realpathCache / REALPATH_CACHE_MAX / cachedRealpath / invalidateRealpathCacheEntry を削除。realpathBestEffort は realpath 直呼びに戻し、clearWorkspaceRoots から cache clear を落とす。撤去の根拠を doc として残す
  • fs.ts: withStaleCacheRetry を削除し、readFileImpl / readFileBase64Impl / writeFileImpl を「認可 → O_NOFOLLOW I/O」の直列に戻す。ELOOP の意味論を doc に書き直す
  • fs.test.ts: cache hit を swap 窓の fixture に使っていた 6 本を、「前回の認可を持ち越さない」性質の pin として意味を書き直す (fixture と assert は据え置き)
  • path-guard.test.ts: 認可 API 側の鮮度 pin を 5 本追加 (末端 4 本 + 中間 dir symlink = 祖先 fall-through 1 本)。resolveInsideRoot の 394 Phase D follow-up B: realpath cache invalidation on watcher batch + L2-miss TOCTOU #406 回帰 test と対になる
  • ADR-0011 の「注意すべき影響」から retarget のズレを解消済みに変更。open-nofollow.ts / search.test.ts / migration-plan.md / src/types/errors.ts / src/lib/errors.ts の関連記述も実態に合わせる
  • ELOOP の user 向け文言を「リンク先が変わったため開けませんでした」→「リンクの参照先を解決できないため開けませんでした」に変更 (dangling / 循環では何も「変わって」いないため、旧文言は全ケースで真ではなかった)

動作確認

  • unit test 3008 件 pass (test を 1 行も変更しない状態で 3003 件 pass = 等価性証拠。commit d81b864 時点)
  • cache 再導入 mutant で書き直した pin 10 本が KILL されることを実測 (非 vacuous の確認)
  • 祖先だけを cache する部分的再導入 mutant が既存 993 件を素通りすることを実測 → 追加した 1 本で KILL
  • typecheck 3 本 (node / web / e2e) pass、biome pass、electron-vite build 成功
  • e2e はローカル実行不可 (listen EPERM) のため CI で確認 → 16 checks 全 green。e2e 186 passed + 1 flaky = 187 (main と同数)、electron-e2e 22 passed
検証エビデンス

リスク分類

tier: medium — classify-risk.sh の reasons は空 ({"tier": "medium", "reasons": []})

実行した検証

種別 コマンド 結果
テスト (全体) ./node_modules/.bin/vitest run PASS 3008 passed / 2 skipped
テスト (等価性証拠) commit d81b864 時点で ./node_modules/.bin/vitest run PASS 3003 passed (test 無変更)
Mutation (cache 再導入) python3 scratchpad/s153-mutant-cache.py + vitest 10 KILLED / 123 生存 (狙った 10 本がちょうど落ちる)
Mutation (祖先のみ cache) python3 scratchpad/s153-mutant-ancestor.py + vitest 追加 test 1 本のみ KILLED (追加前は 993 件素通り)
Probe (realpath コスト) node scratchpad/s153-realpath-cost.js 21.9 µs/call (深さ 8) / 31.3 µs (symlink 1 段) / Map.get 0.008 µs
Probe (ELOOP の原因) node scratchpad/s153-eloop-causes.js dangling: realpath=ENOENT, open=ELOOP / 循環: realpath=ELOOP, open=ELOOP
Typecheck tsc --noEmit -p tsconfig.{node,web,e2e}.json PASS (3 本とも)
Lint ./node_modules/.bin/biome check . PASS (394 files, 1 info)
Build ./node_modules/.bin/electron-vite build PASS
CI (16 checks) GitHub Actions on PR #460 全 pass
e2e (CI) pnpm test:e2e 186 passed + 1 flaky = 187 (main と同数)。flaky は e2e/code-block-copy.spec.ts:17 (clipboard、本 diff と無関係)
electron-e2e (CI) pnpm test:e2e:electron (xvfb) 22 passed
レビュー /simplify 4 観点 (sonnet 並列) 2 findings (2 fixed)
レビュー code-reviewer (fable) round 1 Warning 4 / Suggestion 1 → 全 5 件 fixed (c10a22c)
レビュー code-reviewer (fable) round 2 Warning 3 / Suggestion 5 → 全 8 件 fixed (4a9ce76)
レビュー codex-review security 実行不能 (auth token refresh 失敗) → fable 代替: verdict=pass / 0 findings

レビュー指摘と対応

# Finding 行き先
1 migration-plan.md — realpath async 化を #453 の成果として書いていた (実際は別コミットで解消済み) (a) fix (c10a22c → 4a9ce76 で最終化)
2 path-guard.ts — 「ADR-0011 の受容事項 ①」の cross-ref が別の記述を指していた (a) fix (c10a22c)
3 fs.ts / path-guard.ts / open-nofollow.ts / ADR-0011 — ELOOP の原因を「dangling か真の race だけ」と断定していたが、循環 symlink も同じ経路で ELOOP になる (probe 実測) (a) fix (c10a22c)
4 fs.ts — 「どちらも再試行で解ける状態ではない」が偽 (workspace 内へ swap された race は再試行で成功しうる) (a) fix (c10a22c)
5 src/lib/errors.ts — ELOOP を non-transient に分類する根拠コメントが本 PR で偽になっていた (「main 側で cache 破棄 + 再認可済み」) (a) fix (c10a22c → 4a9ce76 で数値も訂正)
6 path-guard.test.ts — 祖先 fall-through の鮮度が unpinned (祖先のみ cache する mutant が 993 件を素通り、実測) (a) fix: test 1 本追加 (c10a22c)、未存在 suffix を 2 段に強化 (4a9ce76)
7 fs.test.ts — mutant kill signature の記述が実測と違った (ENOENT ではなく ELOOP) (a) fix (c10a22c)
8 ADR-0011 — 「無条件で成立」が強すぎる (L2 hit / Windows / FileTree 非対称の例外が残る) (a) fix (c10a22c → 4a9ce76 で位置表現も訂正)
9 fs.test.ts:587 — 「fall-through = dangling のときだけ」の旧主張が残っていた (a) fix (4a9ce76)
10 src/types/errors.ts — ELOOP kind コメント「symlink loop ではなく」が新事実と矛盾 (a) fix (4a9ce76)
11 src/lib/errors.ts — user 向け文言「リンク先が変わったため」が dangling / 循環では偽 (a) fix (4a9ce76、test の pin も同時更新)
12 fs.ts / ADR-0011 / errors.ts — 「窓は µs 単位」は T1→T2 窓自体の実測ではなかった (a) fix (4a9ce76)
13 fs.ts — 原因列挙に「認可時点で」の時点限定が無かった (a) fix (4a9ce76)

すべて (a) 本 PR で fix。(b) 起票 / (c) 対応しない は 0 件。

追跡先

backlog delta: 起票 0 件 / 本 PR で close 1 件 (#453) / 現在 open 18 件

本 PR で fix しない finding: なし

Draft 判定

  • 判定: normal
  • 根拠: step 4 — 残る (a) fix は無し、(b) / (c) も 0 件。security 観点の第二意見 (codex 不能のため fable 代替) は verdict=pass

ymnao added 5 commits August 1, 2026 23:00
- realpathCache / cachedRealpath / invalidateRealpathCacheEntry を削除し、
  realpathBestEffort を realpath 直呼びに戻す
- fs.ts の withStaleCacheRetry を削除。ELOOP の切り分け ((a) cache stale /
  (b) 認可後 swap) のうち (a) が構造的に消えるため、readFileImpl /
  readFileBase64Impl / writeFileImpl は認可 → O_NOFOLLOW I/O の直列に戻し、
  ELOOP はそのまま呼び手へ伝播させる (fail-closed)
- 判断の根拠: cache key は成功した path 自身で、鮮度を保存するのは同一 path の
  2 回目以降だけだった (初回は元から fresh)。invalidation は retarget の通知経路が
  無いため持てず、これは resolveInsideRoot が cache を通さない理由 (#406 Finding 1)
  と同根。削減できていたコストは認可 1 回あたり realpath 1 回 = 実測 ~22µs
- user-IPC 認可が resolveInsideRoot と同じ鮮度に揃い、ADR-0011 の不変条件が
  cache の温度に依存しなくなった
- 等価性証拠: 既存 unit test 3003 件を 1 行も変更せず全 pass
- fs.test.ts の「末端 symlink の境界」describe は cache hit を swap 窓の
  決定的 fixture に使っていた。cache 撤去でその終状態は作れないため、
  6 本の test 名とコメントを「一度 read / write した path が retarget されたら
  次の認可が新しい解決先で判定する」性質の pin として書き直す
  (fixture と assert は据え置き。cache を再導入すると外向きは ELOOP、
  内向き alias は ENOENT で落ちる = vacuous ではない)
- dangling symlink 経由の O_NOFOLLOW pin (#418 本命の escape regression 含む)
  は cache に依存しないため無変更
- path-guard.test.ts に認可 API 側の鮮度 pin を 4 本追加。resolveInsideRoot の
  #406 回帰 test と対になる性質で、外 → 内 の retarget も反映される双方向まで押さえる
- ADR-0011「注意すべき影響」の ①「retarget 直後は両者が一時的にズレる」を
  解消済みに変更。不変条件 (検索結果に出る集合 = fs:read で開ける集合) が
  cache の温度に依存せず無条件で成立するようになった
- 同 ② の記述から「ELOOP を受けたら cache を捨てて再認可」の機構名を落とし、
  ELOOP が「dangling か真の race」だけを意味することを明記
- search.test.ts の対比コメント (index ゲートだけが fresh) を更新
- migration-plan.md の「realpath の async 化」候補は解消済みとして畳む
  (`cachedRealpathSync` は既に存在せず、cache 自体も本 PR で消えた)
レビュー指摘の反映。

- **ELOOP の原因列挙が不正確だった**: 「dangling か真の race だけ」と書いていたが、
  循環 symlink (a -> b -> a) も realpath が ELOOP で失敗して祖先 fall-through し、
  canonical の末端が symlink のまま残る (probe 実測)。さらに race のうち
  「解決先が workspace 内」のケースは再試行で成功しうるため「どちらも再試行で
  解けない」も偽だった。fs.ts / path-guard.ts / open-nofollow.ts / ADR-0011 の
  4 箇所を、事実 (realpath が解決できない symlink か真の race) と判断の根拠
  (窓が µs 単位なので専用の再試行機構を持たない) に書き直す
- src/lib/errors.ts の ELOOP non-transient 分類の根拠コメントが本 PR で偽に
  なっていた (「main 側で cache 破棄 + 再認可まで済ませた上での失敗」)。分類自体は
  維持し、根拠を頻度と fail-closed の選好に書き直す
- **祖先 fall-through の鮮度が unpinned だった**: 祖先の解決結果だけを cache する
  部分的再導入は既存 993 test を素通りする (実測)。中間 dir symlink の retarget を
  押さえる test を 1 本追加し、同 mutant がこの 1 本で落ちることを確認
- fs.test.ts の mutant kill signature の記述を実測に合わせる (ENOENT ではなく ELOOP)
- ADR-0011 の「無条件で成立」を「cache の温度という前提条件を失った」に限定
  (同 ADR は L2 hit 経路 / Windows / FileTree 非対称の例外を引き続き受容している)
- migration-plan.md の「realpath の async 化」は #453 とは別に解消済みだった旨に訂正
round 2 レビュー指摘の反映 (8 件すべて fix)。

- fs.test.ts の describe 冒頭に「fall-through = dangling のときだけ」という
  旧主張が残っていた (同 commit で他 4 箇所は訂正済み)。循環 symlink も同じ経路を
  通るため「realpath が解決できなかったとき」に統一
- src/types/errors.ts の ELOOP kind コメント「symlink loop ではなく」を訂正。
  文字通りの循環 symlink でも同じ errno になる (git.ts は既に正しく書いていた)
- **user 向け文言**「リンク先が変わったため開けませんでした」→「リンクの参照先を
  解決できないため開けませんでした」。dangling / 循環では何も「変わって」いないため
  全ケースで真ではなかった。errors.test.ts の pin と根拠コメントも同時に更新
- errors.ts の非 transient 判断の根拠にあった「backoff (数秒)」を実際の値に訂正
  (withRetry は 200/400/800ms = 計 ~1.4 秒、autosave 経路のみ 5 秒〜)
- 「窓は µs 単位」は T1→T2 窓自体の実測ではなかったため「handler 1 実行内の
  await 1 回分」に置き換える (fs.ts / ADR-0011)
- fs.ts の原因列挙に「認可時点で」の時点限定を追加 (open-nofollow.ts と同じ形に)
- ADR-0011 の例外参照から位置表現 (「下の」) を外す。3 つ中 2 つは上にあった
- 祖先鮮度 test の未存在 suffix を 2 段にして、「深さ N 段以上だけ cache する」型の
  部分的再導入も同じ 1 本で kill できるようにする (mutant 再実測で確認)
@ymnao
ymnao merged commit d279324 into main Aug 1, 2026
17 checks passed
@ymnao
ymnao deleted the refactor/drop-realpath-cache branch August 1, 2026 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

418 follow-up: user-IPC 認可の realpath 鮮度 (realpathCache の invalidation) を判断する

1 participant