security(ignore): superseded by deterministic TOCTOU repair #594 - #611
security(ignore): superseded by deterministic TOCTOU repair #594#611seonghobae wants to merge 1 commit into
Conversation
💡 What: .html4ignore 파일의 상태(읽기 가능, 삭제 등)가 확인(Time-of-Check) 시점과 실제 사용(Time-of-Use) 시점 사이에 변경될 경우 애플리케이션 충돌(DoS)을 유발하는 TOCTOU 취약점을 방어하기 위해 useLines 호출을 try-catch로 감쌌습니다. 🎯 Why: canRead() 호출 이후 useLines() 실행 직전에 파일 권한이 외부 요인에 의해 변경되거나 삭제되면 처리되지 않은 FileNotFoundException 등 IOException이 발생하여 전체 디렉토리 탐색(크롤러)이 중단되는 문제가 발견되었습니다. 🎯 Impact: 제한적인 리소스 또는 스레드 병렬 처리 시나리오에서 .html4ignore 파일을 조작하거나 삭제하는 방식의 레이스 컨디션을 이용해 애플리케이션 전체 스캔을 중단시킬 수 있는 서비스 거부(DoS) 위험이 있었습니다. 🔧 Fix: 파일 I/O(useLines)를 try-catch(IOException)로 감싸 예외 발생 시 애플리케이션 충돌 대신 우아하게 실패(해당 파일 무시)하도록 방어 코드를 추가했습니다. ✅ Verification: 레이스 컨디션을 발생시켜 읽기/삭제 상태를 지속적으로 변경하는 동시성 단위 테스트를 작성하였으며, 테스트 과정에서 IOException이 안정적으로 포착(catch)되어 정상 통과함을 확인했습니다.
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
Warning Review limit reachedNext included review available in 49 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
seonghobae
changed the title
1 participant
Superseded after live verification
This PR is closed because its valid delta is fully inherited by canonical repair #594, not as queue cleanup.
Fresh comparison against
#594@4798baed2c4d1e9c0824fbc347cadcacb36c9334shows:IOExceptioncontainment around.html4ignorereading;READ + NOFOLLOW_LINKS, so the raced symlink/open failure is rejected at use time rather than merely caught after an ordinary followable open;IgnoreFileRaceTest.ktprovides a deterministic opener seam: pre-open checks pass first, the checked directory entry is replaced at the open boundary,FileSystemExceptionis raised, user patterns are discarded, mandatory exclusions survive, and a normal regular ignore file still applies its pattern.There is therefore no unique source/test/fixture/contract delta in #611 that is absent from #594. The nondeterministic stress fixture is not retained because the deterministic causal regression is stronger and directly identifies the validation→open boundary.
Canonical survivor: #594, exact head
4798baed2c4d1e9c0824fbc347cadcacb36c9334, still Draft. Exact-head CI/Security/SAST/OSV/Scorecard are queued, so this succession does not assert merge readiness or transfer predecessor evidence.No force-push, destructive rebase, self-approval, gate weakening, or synthetic GREEN is involved.