🛡️ Sentinel: [MEDIUM] Fix TOCTOU vulnerability via Atomic File Move

.jules/sentinel.md

@@ -88,3 +88,8 @@
 **Vulnerability:** CSP 해시 불일치로 인한 인라인 스타일 차단
 **Learning:** 브라우저는 인라인 스크립트와 스타일의 내부 텍스트(공백과 줄바꿈 포함)를 정확하게 해싱하여 Content-Security-Policy(CSP) 해시와 비교합니다. Kotlin의 멀티라인 문자열(`"""`)을 사용하여 템플릿에 콘텐츠를 주입할 때 암묵적인 여백이나 줄바꿈이 추가되면 최종 HTML 문자열이 변경되어 CSP 해시가 무효화됩니다.
 **Prevention:** 콘텐츠를 해싱하기 전에 `.trimIndent()`를 적용하여 원본 문자열을 정규화하고, HTML 템플릿에 주입할 때 `<style>${exactContent}</style>`와 같이 공백 없이 주입하여 해시가 완벽하게 일치하도록 해야 합니다.
+
+## 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)하도록 구현하십시오.

CHANGELOG.md

@@ -19,3 +19,6 @@ All notable changes to this project are documented in this file.
 
 - Record the CSP byte-identity decision, threat boundary, verification contract,
   and current W3C Working Draft reference in `docs/doctoring`.
+
+### Security
+- Enhance crash-consistency of `index.html` generation by using `StandardCopyOption.ATOMIC_MOVE` on supporting filesystems (falling back to standard replacement), protecting against Time-of-Check to Time-of-Use (TOCTOU) file corruption.

docs/doctoring.md

@@ -0,0 +1 @@
+The use of `StandardCopyOption.ATOMIC_MOVE` when regenerating `index.html` leverages Java NIO provider semantics to ensure crash-consistent best-effort publication. This prevents partial file states or corruption if the generator is interrupted or if there is concurrent access. If the underlying filesystem provider rejects the atomic option (e.g., across mount points or in specific Docker environments), the implementation gracefully falls back to a standard `REPLACE_EXISTING` move. While this fallback does not provide atomic guarantees, it maintains basic compatibility where strict atomicity cannot be enforced.

src/main/kotlin/html4tree/main.kt

@@ -300,12 +300,20 @@ fun process_ignore_file(curr_dir: File, dirFilesNames: Array<String>? = 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<out java.nio.file.CopyOption>) -> 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)
     }

src/test/kotlin/html4tree/MainTest.kt

@@ -346,10 +346,80 @@ class MainTest {
         assertTrue(htmlContent.contains("margin: 0 auto;"))
     }
 
+    @Test
+    fun testWriteIndexFileFallbackSuccessful() {
+        var fallbackCalled = false
+        val mockMove: (java.nio.file.Path, java.nio.file.Path, Array<out java.nio.file.CopyOption>) -> 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(), "Simulated provider rejection")
+            }
+            fallbackCalled = true
+            java.nio.file.Files.move(src, dest, *options)
+        }
+
+        write_index_file(tempDir, "test content", mockMove)
+
+        assertTrue(fallbackCalled, "Fallback should occur when Atomic Move fails")
+        val indexFile = File(tempDir, "index.html")
+        assertTrue(indexFile.exists())
+        assertEquals("test content", indexFile.readText())
+    }
+
+    @Test
+    fun testWriteIndexFileTempPlacementInSameDirectory() {
+        var tempFileDir: java.io.File? = null
+        val mockMove: (java.nio.file.Path, java.nio.file.Path, Array<out java.nio.file.CopyOption>) -> java.nio.file.Path = { src, dest, options ->
+            tempFileDir = src.toFile().parentFile
+            java.nio.file.Files.move(src, dest, *options)
+        }
+
+        write_index_file(tempDir, "temp dir content", mockMove)
+
+        assertTrue(tempFileDir != null, "temp file dir should not be null")
+        assertEquals(tempDir.absolutePath, tempFileDir!!.absolutePath, "Temporary file must be created in the target directory to support atomic moves")
+    }
+
+    @Test
+    fun testWriteIndexFileAtomicMoveSuccess() {
+        var atomicUsed = false
+        val mockMove: (java.nio.file.Path, java.nio.file.Path, Array<out java.nio.file.CopyOption>) -> java.nio.file.Path = { src, dest, options ->
+            if (options.contains(java.nio.file.StandardCopyOption.ATOMIC_MOVE)) {
+                atomicUsed = true
+            }
+            java.nio.file.Files.move(src, dest, *options)
+        }
+
+        write_index_file(tempDir, "atomic content", mockMove)
+
+        assertTrue(atomicUsed, "Atomic move option should be used by default")
+        val indexFile = File(tempDir, "index.html")
+        assertTrue(indexFile.exists())
+        assertEquals("atomic content", indexFile.readText())
+    }
+
+    @Test
+    fun testWriteIndexFileFallbackFailureCleansTempAndPreservesTarget() {
+        val targetIndex = File(tempDir, "index.html")
+        targetIndex.writeText("original target")
+
+        val mockMoveFails: (java.nio.file.Path, java.nio.file.Path, Array<out java.nio.file.CopyOption>) -> 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(), "Simulated provider rejection")
+            }
+            throw java.io.IOException("Fallback simulated IO failure")
+        }
+
+        assertFailsWith<java.io.IOException> {
+            write_index_file(tempDir, "failed content", mockMoveFails)
+        }
+
+        assertEquals("original target", targetIndex.readText(), "Target should be preserved on fallback failure")
+        val leftoverTemp = tempDir.listFiles()?.filter { it.name.startsWith(".index-") } ?: emptyList()
+        assertTrue(leftoverTemp.isEmpty(), "temporary index file should be cleaned up on failure")
+    }
+
     @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")
@@ -358,10 +428,9 @@ class MainTest {
             write_index_file(tempDir, "content")
         }
 
-        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")
+        assertEquals("keep", File(indexDir, "occupant.txt").readText())
     }
 
     @Test