fix(security): cap /api/file + MCP read_file at 5 MiB (RAN-9) #67
There are no files selected for viewing
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| package io.github.randomcodespace.iq.api; | ||
|
|
||
| import java.io.BufferedReader; | ||
| import java.io.IOException; | ||
| import java.nio.charset.StandardCharsets; | ||
| import java.nio.file.Files; | ||
| import java.nio.file.Path; | ||
|
|
||
| /** | ||
| * Reads files for the read-only serving layer with a hard byte cap. | ||
| * | ||
| * <p>The two entry points that surface repo content over HTTP — {@code GET /api/file} | ||
| * and the {@code read_file} MCP tool — must never load unbounded content into the JVM | ||
| * heap; a multi-GB file would OOM the serving process and become a trivial DoS vector. | ||
| * | ||
| * <p>Behaviour: | ||
| * <ul> | ||
| * <li>Without a line range, the file's on-disk size is checked first and the read | ||
| * is rejected if it exceeds the cap.</li> | ||
| * <li>With a {@code startLine}/{@code endLine} range, the file is read line-by-line | ||
| * via a {@link BufferedReader}; only lines in range are retained and the | ||
| * accumulated UTF-8 byte count is capped the same way.</li> | ||
| * </ul> | ||
| */ | ||
| public final class SafeFileReader { | ||
|
|
||
| public static final class FileTooLargeException extends RuntimeException { | ||
| private final long size; | ||
| private final long max; | ||
|
|
||
| public FileTooLargeException(long size, long max) { | ||
| super("File exceeds max size: " + size + " bytes (max " + max + " bytes)"); | ||
| this.size = size; | ||
| this.max = max; | ||
| } | ||
|
|
||
| public long size() { return size; } | ||
| public long max() { return max; } | ||
| } | ||
|
|
||
| private SafeFileReader() {} | ||
|
|
||
| public static String read(Path path, Integer startLine, Integer endLine, long maxBytes) | ||
|
Check failure on line 43 in src/main/java/io/github/randomcodespace/iq/api/SafeFileReader.java
|
||
| throws IOException { | ||
| if (startLine == null && endLine == null) { | ||
| long size = Files.size(path); | ||
| if (size > maxBytes) { | ||
| throw new FileTooLargeException(size, maxBytes); | ||
| } | ||
| return Files.readString(path, StandardCharsets.UTF_8); | ||
| } | ||
| int start = Math.max(1, startLine != null ? startLine : 1); | ||
| int end = endLine != null ? Math.max(start, endLine) : Integer.MAX_VALUE; | ||
| StringBuilder sb = new StringBuilder(); | ||
| long accumulated = 0; | ||
| boolean first = true; | ||
| try (BufferedReader reader = Files.newBufferedReader(path, StandardCharsets.UTF_8)) { | ||
| String line; | ||
| int idx = 0; | ||
| while ((line = reader.readLine()) != null) { | ||
|
Check warning on line 60 in src/main/java/io/github/randomcodespace/iq/api/SafeFileReader.java
|
||
| idx++; | ||
| if (idx < start) continue; | ||
|
aksOps marked this conversation as resolved.
|
||
| if (idx > end) break; | ||
| long lineBytes = line.getBytes(StandardCharsets.UTF_8).length; | ||
| long add = lineBytes + (first ? 0L : 1L); | ||
| if (accumulated + add > maxBytes) { | ||
| throw new FileTooLargeException(accumulated + add, maxBytes); | ||
| } | ||
| if (!first) sb.append('\n'); | ||
| sb.append(line); | ||
|
aksOps marked this conversation as resolved.
|
||
| accumulated += add; | ||
| first = false; | ||
| } | ||
| } | ||
| return sb.toString(); | ||
| } | ||
| } | ||
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
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
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| package io.github.randomcodespace.iq.config.unified; | ||
| public record ServingConfig(Integer port, String bindAddress, Boolean readOnly, Long maxFileBytes, Neo4jConfig neo4j) { | ||
| public static ServingConfig empty() { return new ServingConfig(null, null, null, null, Neo4jConfig.empty()); } | ||
| } |
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,68 @@ | ||
| package io.github.randomcodespace.iq.api; | ||
|
|
||
| import org.junit.jupiter.api.Test; | ||
| import org.junit.jupiter.api.io.TempDir; | ||
|
|
||
| import java.io.IOException; | ||
| import java.nio.charset.StandardCharsets; | ||
| import java.nio.file.Files; | ||
| import java.nio.file.Path; | ||
|
|
||
| import static org.junit.jupiter.api.Assertions.assertEquals; | ||
| import static org.junit.jupiter.api.Assertions.assertThrows; | ||
| import static org.junit.jupiter.api.Assertions.assertTrue; | ||
|
|
||
| class SafeFileReaderTest { | ||
|
|
||
| @Test | ||
| void readsWholeFileUnderCap(@TempDir Path tempDir) throws IOException { | ||
| Path f = tempDir.resolve("small.txt"); | ||
| Files.writeString(f, "hello", StandardCharsets.UTF_8); | ||
| assertEquals("hello", SafeFileReader.read(f, null, null, 1024L)); | ||
| } | ||
|
|
||
| @Test | ||
| void rejectsWholeFileExceedingCap(@TempDir Path tempDir) throws IOException { | ||
| Path f = tempDir.resolve("big.txt"); | ||
| byte[] payload = new byte[2048]; | ||
| java.util.Arrays.fill(payload, (byte) 'x'); | ||
| Files.write(f, payload); | ||
| SafeFileReader.FileTooLargeException e = assertThrows( | ||
| SafeFileReader.FileTooLargeException.class, | ||
| () -> SafeFileReader.read(f, null, null, 1024L)); | ||
| assertEquals(2048L, e.size()); | ||
| assertEquals(1024L, e.max()); | ||
| } | ||
|
|
||
| @Test | ||
| void streamsLineRangeWithoutLoadingWholeFile(@TempDir Path tempDir) throws IOException { | ||
| Path f = tempDir.resolve("ranged.txt"); | ||
| StringBuilder sb = new StringBuilder(); | ||
| for (int i = 1; i <= 100; i++) sb.append("line").append(i).append('\n'); | ||
| Files.writeString(f, sb.toString(), StandardCharsets.UTF_8); | ||
|
|
||
| // Whole-file readString with cap=64 would fail, but the 3-line range fits. | ||
| assertEquals("line2\nline3\nline4", | ||
| SafeFileReader.read(f, 2, 4, 64L)); | ||
| } | ||
|
|
||
| @Test | ||
| void rejectsLineRangeExceedingCap(@TempDir Path tempDir) throws IOException { | ||
| Path f = tempDir.resolve("ranged.txt"); | ||
| StringBuilder sb = new StringBuilder(); | ||
| for (int i = 1; i <= 100; i++) sb.append("line").append(i).append('\n'); | ||
| Files.writeString(f, sb.toString(), StandardCharsets.UTF_8); | ||
|
|
||
| SafeFileReader.FileTooLargeException e = assertThrows( | ||
| SafeFileReader.FileTooLargeException.class, | ||
| () -> SafeFileReader.read(f, 1, 100, 32L)); | ||
| assertTrue(e.max() == 32L); | ||
|
Check warning on line 59 in src/test/java/io/github/randomcodespace/iq/api/SafeFileReaderTest.java
|
||
| } | ||
|
|
||
| @Test | ||
| void clampsStartLineToOneWhenNegative(@TempDir Path tempDir) throws IOException { | ||
| Path f = tempDir.resolve("clamp.txt"); | ||
| Files.writeString(f, "a\nb\nc\n", StandardCharsets.UTF_8); | ||
| assertEquals("a\nb", SafeFileReader.read(f, -3, 2, 1024L)); | ||
| } | ||
| } | ||
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.