refactor: user-IPC 認可の realpathCache を撤去し毎回 fresh 解決にする (#453) - #460
Merged
Merged
Conversation
- 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 再実測で確認)
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.
概要
user-IPC 認可 (
assertPathAllowed/assertWritePathAllowed/isPathAllowed/isPathWithinAnyAllowedRoot/canonicalize) が持っていた realpath 結果の LRU cache (上限 256・invalidation 無し) を撤去し、毎回 fresh に realpath するようにした。#453 の「判断が要る点」に対する結論は cache を捨てる。判断の根拠 (いずれも本セッションで確定させた事実):
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 認可だけ別扱いにする根拠が無かったnodeprobe の実測は深さ 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 回」につき定数回で、ループ経路は無いことを確認済み副産物として、cache の存在を前提にしていた機構が消える:
invalidateRealpathCacheEntryと fs.ts のwithStaleCacheRetry(ELOOP を「cache stale」/「認可後 swap」に切り分けて 1 度だけ再認可する helper)。cache が無ければ認可時点で canonical の末端が symlink であり得るのは realpath が解決できないとき (dangling / 循環) だけなので、ELOOP はそのまま fail-closed で伝播させる。関連 Issue
closes #453
移行 Stage
変更内容
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 と対になるopen-nofollow.ts/search.test.ts/migration-plan.md/src/types/errors.ts/src/lib/errors.tsの関連記述も実態に合わせる動作確認
electron-vite build成功検証エビデンス
リスク分類
tier: medium — classify-risk.sh の reasons は空 (
{"tier": "medium", "reasons": []})実行した検証
./node_modules/.bin/vitest run./node_modules/.bin/vitest runpython3 scratchpad/s153-mutant-cache.py+ vitestpython3 scratchpad/s153-mutant-ancestor.py+ vitestnode scratchpad/s153-realpath-cost.jsnode scratchpad/s153-eloop-causes.jstsc --noEmit -p tsconfig.{node,web,e2e}.json./node_modules/.bin/biome check ../node_modules/.bin/electron-vite buildpnpm test:e2ee2e/code-block-copy.spec.ts:17(clipboard、本 diff と無関係)pnpm test:e2e:electron(xvfb)レビュー指摘と対応
すべて (a) 本 PR で fix。(b) 起票 / (c) 対応しない は 0 件。
追跡先
backlog delta: 起票 0 件 / 本 PR で close 1 件 (#453) / 現在 open 18 件
本 PR で fix しない finding: なし
Draft 判定