Skip to content

fix(io): attempt atomic publication of generated index files #296

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
seonghobae wants to merge 2 commits into
base: master
Choose a base branch
from
+29 −2
Draft

fix(io): attempt atomic publication of generated index files #296

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ec56a0f
🛡️ Sentinel: [MEDIUM] Fix TOCTOU vulnerability in file write operations
seonghobae Jul 28, 2026
3beb6d4
Merge branch 'master' into sentinel-fix-toctou-16582868529218540884
opencode-agent[bot] Aug 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .jules/sentinel.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,3 +83,8 @@
**Vulnerability:** 정적 HTML 생성 도구에서 매번 다른 Nonce를 동적으로 생성하여 CSP에 적용하는 것은, 캐싱 효율을 저하시킬 뿐만 아니라 정적 배포 환경(예: GitHub Pages 등)에서 올바른 보안 정책 수립을 방해할 수 있는 안티 패턴입니다.
**Learning:** 정적으로 고정된 인라인 스타일이나 스크립트에는 난수화된 Nonce보다 콘텐츠 자체의 해시(SHA-256 등)를 사용하는 것이 안전하고 일관된 방식임을 배웠습니다.
**Prevention:** 자동 생성되는 정적 HTML의 콘텐츠 보안 정책(CSP)에는 `style-src 'sha256-<HASH>'` 방식을 적용하고, `<style>` 태그에서 불필요한 `nonce` 속성을 제거하여 브라우저의 무결성 검증 기능을 적극 활용하십시오.

## 2024-07-28 - [html4tree] 원자적 파일 이동을 통한 TOCTOU 및 경쟁 상태(Race Condition) 방지
**Vulnerability:** `index.html` 파일을 쓸 때 임시 파일을 생성하고 일반적인 `REPLACE_EXISTING`으로 복사/이동하면, 다른 프로세스나 스레드가 동일한 파일에 접근할 때 불완전한 파일을 읽거나 교체 중간 시점에 조작될 수 있는 Time-Of-Check to Time-Of-Use (TOCTOU) 취약점이 발생할 수 있습니다.
**Learning:** 로컬 파일 시스템에 파일을 기록할 때는, 중간 상태가 노출되는 것을 방지하기 위해 파일 쓰기를 임시 파일에 완료한 뒤, 대상 경로로 '원자적(atomic)'으로 이동(move)하는 것이 안전합니다.
**Prevention:** `Files.move` 수행 시 가급적 `StandardCopyOption.ATOMIC_MOVE` 옵션을 사용하십시오. 파일 시스템이 이를 지원하지 않아 `AtomicMoveNotSupportedException`이 발생할 수 있으므로, 해당 예외를 포착하여 일반 교체(fallback)를 수행하도록 방어적으로 프로그래밍해야 합니다.
15 changes: 13 additions & 2 deletions src/main/kotlin/html4tree/main.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import java.security.MessageDigest
import java.nio.file.Files
import java.nio.file.LinkOption
import java.nio.file.StandardCopyOption
import java.nio.file.AtomicMoveNotSupportedException
import java.nio.file.attribute.BasicFileAttributes
import java.util.Base64
import com.github.ajalt.clikt.core.CliktCommand
Expand Down Expand Up @@ -229,12 +230,22 @@ 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,
moveAtomic: (java.nio.file.Path, java.nio.file.Path) -> Unit = { source, target ->
Files.move(source, target, StandardCopyOption.ATOMIC_MOVE, StandardCopyOption.REPLACE_EXISTING)
}
) {
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 {
moveAtomic(tempPath, indexPath)
} catch (e: AtomicMoveNotSupportedException) {
Files.move(tempPath, indexPath, StandardCopyOption.REPLACE_EXISTING)
}
Comment on lines +233 to +248

@coderabbitai coderabbitai Bot Aug 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -eu
printf '%s\n' '--- target implementation ---'
sed -n '210,270p' src/main/kotlin/html4tree/main.kt
printf '%s\n' '--- related test ---'
sed -n '335,395p' src/test/kotlin/html4tree/MainTest.kt
printf '%s\n' '--- sentinel entry ---'
sed -n '75,100p' .jules/sentinel.md
printf '%s\n' '--- relevant usages and declarations ---'
rg -n -C 3 'write_index_file|AtomicMoveNotSupportedException|ATOMIC_MOVE|index\.html' src/main src/test .jules

Repository: ContextualWisdomLab/html4tree

Length of output: 27307

🏁 Script executed:

#!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
for p in [
    Path("src/main/kotlin/html4tree/main.kt"),
    Path("src/test/kotlin/html4tree/MainTest.kt"),
]:
    text = p.read_text()
    print(f"{p}: {len(text.splitlines())} lines")
    for i, line in enumerate(text.splitlines(), 1):
        if "write_index_file" in line or "AtomicMoveNotSupportedException" in line or "ATOMIC_MOVE" in line:
            print(f"{i}: {line}")
PY

Repository: ContextualWisdomLab/html4tree

Length of output: 1072

🌐 Web query:

Java Files.move ATOMIC_MOVE REPLACE_EXISTING existing target option ignored provider-specific replacement official documentation

💡 Result:

According to the official Java documentation for the java.nio.file.Files.move method, when the ATOMIC_MOVE option is specified, all other CopyOption arguments (including REPLACE_EXISTING) are ignored [1][2]. The behavior of ATOMIC_MOVE regarding an existing target is defined as implementation-specific: if the target file exists, the underlying file system implementation may either replace the existing file or fail by throwing an IOException [1][3]. Because ATOMIC_MOVE must be performed as an atomic file system operation, the method will throw an AtomicMoveNotSupportedException if the operation cannot be completed atomically—such as when the target location is on a different FileStore or is associated with a different file system provider [1][2]. In summary, you cannot rely on REPLACE_EXISTING to force a replacement when using ATOMIC_MOVE. If you require atomic behavior and the target file might exist, you must handle the potential for an IOException or implementation-dependent behavior [1][4].

Citations:

🏁 Script executed:

#!/bin/bash
set -eu
# Probe the host JDK's default provider with an existing regular target.
# This standalone probe does not load or execute repository code.
if command -v java >/dev/null 2>&1; then
  java -version 2>&1 | head -n 2
  tmpdir="$(mktemp -d)"
  trap 'rm -rf "$tmpdir"' EXIT
  cat >"$tmpdir/MoveProbe.java" <<'JAVA'
import java.nio.charset.StandardCharsets;
import java.nio.file.*;
public class MoveProbe {
  public static void main(String[] args) throws Exception {
    Path dir = Paths.get(args[0]);
    Path source = Files.writeString(dir.resolve("source"), "new", StandardCharsets.UTF_8);
    Path target = Files.writeString(dir.resolve("target"), "old", StandardCharsets.UTF_8);
    try {
      Files.move(source, target, StandardCopyOption.ATOMIC_MOVE, StandardCopyOption.REPLACE_EXISTING);
      System.out.println("atomic_move=" + Files.readString(target));
    } catch (Exception e) {
      System.out.println("atomic_move_exception=" + e.getClass().getName());
      System.out.println("target_after_atomic=" + Files.readString(target));
    }
    Path fallbackSource = Files.writeString(dir.resolve("fallback-source"), "fallback", StandardCharsets.UTF_8);
    try {
      Files.move(fallbackSource, target, StandardCopyOption.REPLACE_EXISTING);
      System.out.println("fallback_move=" + Files.readString(target));
    } catch (Exception e) {
      System.out.println("fallback_move_exception=" + e.getClass().getName());
    }
  }
}
JAVA
  javac "$tmpdir/MoveProbe.java"
  java -cp "$tmpdir" MoveProbe "$tmpdir"
else
  echo "java_unavailable"
fi

Repository: ContextualWisdomLab/html4tree

Length of output: 325

원자적 교체 정책을 코드, 테스트, 문서에서 일치시키세요.

ATOMIC_MOVE를 지정하면 REPLACE_EXISTING은 무시됩니다. 기존 index.html의 교체 여부는 파일 시스템 제공자에 따라 달라집니다. AtomicMoveNotSupportedException 이외의 교체 실패도 명시적으로 처리하고, 비원자적 fallback을 허용할지 결정하세요.

  • src/test/kotlin/html4tree/MainTest.kt:366-375: 기존 index.html을 남긴 상태에서 fallback 교체를 테스트하세요.
  • .jules/sentinel.md:87-90: fallback이 원자성을 보장하지 않는 호환성 경로임을 명시하세요.
🧰 Tools
🪛 detekt (1.23.8)

[warning] 246-246: The caught exception is swallowed. The original exception could be lost.

(detekt.exceptions.SwallowedException)

📍 Affects 3 files
  • src/main/kotlin/html4tree/main.kt#L233-L248 (this comment)
  • src/test/kotlin/html4tree/MainTest.kt#L365-L375
  • .jules/sentinel.md#L86-L90
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/kotlin/html4tree/main.kt` around lines 233 - 248, Update
write_index_file in src/main/kotlin/html4tree/main.kt:233-248 to define and
document the replacement policy, explicitly handle replacement failures beyond
AtomicMoveNotSupportedException, and either reject or deliberately use the
non-atomic fallback while preserving cleanup. In
src/test/kotlin/html4tree/MainTest.kt:365-375, add coverage that keeps an
existing index.html and verifies fallback replacement. In
.jules/sentinel.md:86-90, state that the fallback is a compatibility path and
does not guarantee atomicity.

Source: Coding guidelines

} finally {
Files.deleteIfExists(tempPath)
}
Expand Down
11 changes: 11 additions & 0 deletions src/test/kotlin/html4tree/MainTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,17 @@ class MainTest {
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")

// test fallback on AtomicMoveNotSupportedException
indexDir.deleteRecursively() // remove the directory to allow successful write
var fallbackCalled = false
val mockMoveAtomic: (java.nio.file.Path, java.nio.file.Path) -> Unit = { _, _ ->
fallbackCalled = true
throw java.nio.file.AtomicMoveNotSupportedException("source", "target", "Not supported")
}
write_index_file(tempDir, "content2", mockMoveAtomic)
assertTrue(fallbackCalled, "fallback should be called when AtomicMoveNotSupportedException is thrown")
assertEquals("content2", File(tempDir, "index.html").readText())
}

@Test
Expand Down
Loading