diff --git a/.jules/sentinel.md b/.jules/sentinel.md index cdf88010..d6de1fe1 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -88,3 +88,8 @@ **Vulnerability:** CSP 해시 불일치로 인한 인라인 스타일 차단 **Learning:** 브라우저는 인라인 스크립트와 스타일의 내부 텍스트(공백과 줄바꿈 포함)를 정확하게 해싱하여 Content-Security-Policy(CSP) 해시와 비교합니다. Kotlin의 멀티라인 문자열(`"""`)을 사용하여 템플릿에 콘텐츠를 주입할 때 암묵적인 여백이나 줄바꿈이 추가되면 최종 HTML 문자열이 변경되어 CSP 해시가 무효화됩니다. **Prevention:** 콘텐츠를 해싱하기 전에 `.trimIndent()`를 적용하여 원본 문자열을 정규화하고, HTML 템플릿에 주입할 때 ``와 같이 공백 없이 주입하여 해시가 완벽하게 일치하도록 해야 합니다. + +## 2024-08-05 - [html4tree] index.html 교체 시 TOCTOU 방지 +**Vulnerability:** 기존 `index.html`을 교체할 때 `StandardCopyOption.REPLACE_EXISTING`만 사용하면, 교체되는 순간(TOCTOU)에 다른 프로세스가 파일에 접근하거나 쓰기를 시도할 수 있습니다. +**Learning:** 파일 교체 작업은 시스템에서 지원하는 경우 원자적(Atomic)으로 이루어져야 중간 상태가 노출되지 않으며, 파일 교체로 인한 레이스 컨디션을 방지할 수 있습니다. +**Prevention:** `Files.move` 시 `StandardCopyOption.ATOMIC_MOVE`를 사용하되, 이를 지원하지 않는 파일 시스템(예: 특정 Docker 환경의 overlayfs)을 위해 `AtomicMoveNotSupportedException` 발생 시 일반 교체로 폴백(Fallback)하도록 구현하십시오. diff --git a/src/main/kotlin/html4tree/main.kt b/src/main/kotlin/html4tree/main.kt index e93fbea7..7385439a 100644 --- a/src/main/kotlin/html4tree/main.kt +++ b/src/main/kotlin/html4tree/main.kt @@ -300,12 +300,20 @@ fun process_ignore_file(curr_dir: File, dirFilesNames: Array? = null): S return files_to_exclude } -fun write_index_file(curr_dir: File, content: String) { +fun write_index_file( + curr_dir: File, + content: String, + moveFile: (java.nio.file.Path, java.nio.file.Path, Array) -> java.nio.file.Path = { src, dest, options -> Files.move(src, dest, *options) } +) { val indexPath = curr_dir.toPath().resolve("index.html") val tempPath = Files.createTempFile(curr_dir.toPath(), ".index-", ".html") try { Files.write(tempPath, content.toByteArray(Charsets.UTF_8)) - Files.move(tempPath, indexPath, StandardCopyOption.REPLACE_EXISTING) + try { + moveFile(tempPath, indexPath, arrayOf(StandardCopyOption.ATOMIC_MOVE, StandardCopyOption.REPLACE_EXISTING)) + } catch (e: java.nio.file.AtomicMoveNotSupportedException) { + moveFile(tempPath, indexPath, arrayOf(StandardCopyOption.REPLACE_EXISTING)) + } } finally { Files.deleteIfExists(tempPath) } diff --git a/src/test/kotlin/html4tree/MainTest.kt b/src/test/kotlin/html4tree/MainTest.kt index 83739c9c..1ceab699 100644 --- a/src/test/kotlin/html4tree/MainTest.kt +++ b/src/test/kotlin/html4tree/MainTest.kt @@ -346,20 +346,36 @@ class MainTest { assertTrue(htmlContent.contains("margin: 0 auto;")) } + @Test + fun testWriteIndexFileFallbackOnAtomicMoveNotSupported() { + var fallbackCalled = false + val mockMove: (java.nio.file.Path, java.nio.file.Path, Array) -> java.nio.file.Path = { src, dest, options -> + if (options.contains(java.nio.file.StandardCopyOption.ATOMIC_MOVE)) { + throw java.nio.file.AtomicMoveNotSupportedException(src.toString(), dest.toString(), "Mocked") + } + fallbackCalled = true + java.nio.file.Files.move(src, dest, *options) + } + + write_index_file(tempDir, "test content", mockMove) + + assertTrue(fallbackCalled, "Fallback to regular move was not called") + val indexFile = File(tempDir, "index.html") + assertTrue(indexFile.exists()) + assertEquals("test content", indexFile.readText()) + } + @Test fun testWriteIndexFileCleansUpTempFileOnFailure() { - // Files.move cannot replace a non-empty directory, so this drives the - // exception path through write_index_file's finally block. - val indexDir = File(tempDir, "index.html") - indexDir.mkdir() - File(indexDir, "occupant.txt").writeText("keep") - - assertFailsWith { - write_index_file(tempDir, "content") + // Using mock move to simulate a failure and cover the default moveFile fallback logic + val mockMoveFails: (java.nio.file.Path, java.nio.file.Path, Array) -> java.nio.file.Path = { src, dest, options -> + throw java.io.IOException("Mock IO Exception") + } + + assertFailsWith { + write_index_file(tempDir, "content", mockMoveFails) } - assertTrue(indexDir.isDirectory) - assertEquals("keep", File(indexDir, "occupant.txt").readText()) val leftoverTemp = tempDir.listFiles()?.filter { it.name.startsWith(".index-") } ?: emptyList() assertTrue(leftoverTemp.isEmpty(), "temporary index file should be cleaned up on failure") }