fix: symlink-safe, fail-closed jail for filesystem tools
This commit is contained in:
+1
-3
@@ -134,9 +134,7 @@ class FileEditTool(
|
|||||||
}
|
}
|
||||||
|
|
||||||
private fun isPathAllowed(path: Path): Boolean =
|
private fun isPathAllowed(path: Path): Boolean =
|
||||||
normalizedAllowedPaths.any { allowedPath ->
|
PathJail.isContained(path, normalizedAllowedPaths)
|
||||||
path.startsWith(allowedPath)
|
|
||||||
}
|
|
||||||
|
|
||||||
private fun mapExceptionToValidationResult(e: Throwable): ValidationResult =
|
private fun mapExceptionToValidationResult(e: Throwable): ValidationResult =
|
||||||
when (e) {
|
when (e) {
|
||||||
|
|||||||
+2
-2
@@ -62,8 +62,8 @@ class FileReadTool(
|
|||||||
return ValidationResult.Invalid("'end_line' must be >= 'start_line'.")
|
return ValidationResult.Invalid("'end_line' must be >= 'start_line'.")
|
||||||
|
|
||||||
return runCatching {
|
return runCatching {
|
||||||
val path = Paths.get(pathString).normalize().toAbsolutePath()
|
val path = Paths.get(pathString)
|
||||||
if (allowedPaths.isNotEmpty() && allowedPaths.none { path.startsWith(it.normalize().toAbsolutePath()) })
|
if (!PathJail.isContained(path, allowedPaths))
|
||||||
ValidationResult.Invalid("Path '$pathString' is not in the allowed list")
|
ValidationResult.Invalid("Path '$pathString' is not in the allowed list")
|
||||||
else
|
else
|
||||||
ValidationResult.Valid
|
ValidationResult.Valid
|
||||||
|
|||||||
+1
-3
@@ -132,9 +132,7 @@ class FileWriteTool(
|
|||||||
}
|
}
|
||||||
|
|
||||||
private fun isPathAllowed(path: Path): Boolean =
|
private fun isPathAllowed(path: Path): Boolean =
|
||||||
normalizedAllowedPaths.any { allowedPath ->
|
PathJail.isContained(path, normalizedAllowedPaths)
|
||||||
path.startsWith(allowedPath)
|
|
||||||
}
|
|
||||||
|
|
||||||
private fun mapExceptionToValidationResult(e: Throwable): ValidationResult =
|
private fun mapExceptionToValidationResult(e: Throwable): ValidationResult =
|
||||||
when (e) {
|
when (e) {
|
||||||
|
|||||||
+45
@@ -0,0 +1,45 @@
|
|||||||
|
package com.correx.infrastructure.tools.filesystem
|
||||||
|
|
||||||
|
import java.nio.file.Files
|
||||||
|
import java.nio.file.Path
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Symlink-safe path containment for the filesystem tools' jail. Mirrors the
|
||||||
|
* materializer's discipline (resolve, then enforce containment) but adds
|
||||||
|
* [Path.toRealPath] resolution so a symlink living inside an allowed directory
|
||||||
|
* cannot smuggle access to a target outside it. Fails CLOSED: an empty
|
||||||
|
* allow-list contains nothing.
|
||||||
|
*/
|
||||||
|
internal object PathJail {
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Resolve [path] to a symlink-resolved absolute path. If the target does not
|
||||||
|
* exist yet (new-file writes), resolve the nearest existing ancestor via
|
||||||
|
* [Path.toRealPath] and re-append the not-yet-created segments — so a
|
||||||
|
* symlinked ancestor is still followed to its real location before the
|
||||||
|
* containment check.
|
||||||
|
*/
|
||||||
|
fun realize(path: Path): Path {
|
||||||
|
val absolute = path.toAbsolutePath().normalize()
|
||||||
|
var existing: Path? = absolute
|
||||||
|
val missing = ArrayDeque<Path>()
|
||||||
|
while (existing != null && !Files.exists(existing)) {
|
||||||
|
existing.fileName?.let { missing.addFirst(it) }
|
||||||
|
existing = existing.parent
|
||||||
|
}
|
||||||
|
val anchor = existing ?: return absolute
|
||||||
|
val real = runCatching { anchor.toRealPath() }.getOrDefault(anchor)
|
||||||
|
return missing.fold(real) { acc, segment -> acc.resolve(segment) }.normalize()
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* True iff [path] resolves inside one of [allowedRoots]. Empty roots ⇒ false
|
||||||
|
* (fail closed). Both the candidate and each root are symlink-resolved before
|
||||||
|
* comparison.
|
||||||
|
*/
|
||||||
|
fun isContained(path: Path, allowedRoots: Set<Path>): Boolean {
|
||||||
|
if (allowedRoots.isEmpty()) return false
|
||||||
|
val real = realize(path)
|
||||||
|
return allowedRoots.any { real.startsWith(realize(it)) }
|
||||||
|
}
|
||||||
|
}
|
||||||
+47
@@ -0,0 +1,47 @@
|
|||||||
|
package com.correx.infrastructure.tools.filesystem
|
||||||
|
|
||||||
|
import org.junit.jupiter.api.Assertions.assertFalse
|
||||||
|
import org.junit.jupiter.api.Assertions.assertTrue
|
||||||
|
import org.junit.jupiter.api.Test
|
||||||
|
import java.nio.file.Files
|
||||||
|
|
||||||
|
class PathJailTest {
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `empty allow-list fails closed`() {
|
||||||
|
val dir = Files.createTempDirectory("jail").toRealPath()
|
||||||
|
assertFalse(PathJail.isContained(dir.resolve("f.txt"), emptySet()))
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `path inside an allowed root is contained`() {
|
||||||
|
val root = Files.createTempDirectory("jail-in").toRealPath()
|
||||||
|
assertTrue(PathJail.isContained(root.resolve("sub/f.txt"), setOf(root)))
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `path outside all allowed roots is rejected`() {
|
||||||
|
val root = Files.createTempDirectory("jail-root").toRealPath()
|
||||||
|
val other = Files.createTempDirectory("jail-other").toRealPath()
|
||||||
|
assertFalse(PathJail.isContained(other.resolve("f.txt"), setOf(root)))
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `non-existent target under an allowed root is contained`() {
|
||||||
|
val root = Files.createTempDirectory("jail-new").toRealPath()
|
||||||
|
assertTrue(PathJail.isContained(root.resolve("does/not/exist/yet.txt"), setOf(root)))
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `symlink inside an allowed root that points outside is rejected`() {
|
||||||
|
val root = Files.createTempDirectory("jail-link-root").toRealPath()
|
||||||
|
val outside = Files.createTempDirectory("jail-link-escape").toRealPath()
|
||||||
|
Files.writeString(outside.resolve("secret.txt"), "secret")
|
||||||
|
val link = root.resolve("escape")
|
||||||
|
Files.createSymbolicLink(link, outside)
|
||||||
|
|
||||||
|
// Without toRealPath() this string-prefixes inside root and would be allowed;
|
||||||
|
// symlink-resolved it lands in `outside`, so it must be rejected.
|
||||||
|
assertFalse(PathJail.isContained(link.resolve("secret.txt"), setOf(root)))
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user