chore(audit): tool path resolution, config file, dead code removal, static analysis cleanup
Tool fixes: - FileWriteTool, FileEditTool: resolve relative paths against workingDir instead of JVM cwd - FileReadTool: remove allowedPaths restriction — reads are unrestricted - ShellTool: add workingDir for ProcessBuilder, remove environment().clear(), fail-open when allowedExecutables empty Config: - Add [tools] section to config.toml: sandbox_root, working_dir, shell_allowed_executables, default_system_prompt_path - Three-level resolution: env var > config file > hardcoded default - OrchestrationConfig sandboxRoot and defaultSystemPromptPath now sourced from CorrexConfig - Single defaultOrchestrationConfig in ServerModule, shared by SessionRoutes and GlobalStreamHandler Dead code: - Delete EventTypes.kt — orphaned string constants duplicating serialization annotations Static analysis: - Remove 10 unused imports across 9 files - Remove redundant return in ReplayOrchestrator - Remove redundant suspend from ApprovalCoordinator.handleResponse - Fix unreachable InputMode.None branch in InputBar - Enable full detekt rule sets in detekt.yml
This commit is contained in:
-1
@@ -20,7 +20,6 @@ import kotlinx.coroutines.CoroutineScope
|
||||
import kotlinx.coroutines.Dispatchers
|
||||
import kotlinx.coroutines.Job
|
||||
import kotlinx.coroutines.SupervisorJob
|
||||
import kotlinx.coroutines.flow.collect
|
||||
import kotlinx.coroutines.launch
|
||||
import java.util.concurrent.ConcurrentHashMap
|
||||
|
||||
|
||||
+13
-3
@@ -24,9 +24,19 @@ import java.nio.file.StandardOpenOption
|
||||
@Suppress("TooManyFunctions")
|
||||
class FileEditTool(
|
||||
allowedPaths: Set<Path> = emptySet(),
|
||||
private val workingDir: Path? = null,
|
||||
) : Tool, FileAffectingTool, ToolExecutor {
|
||||
private val normalizedAllowedPaths: Set<Path> = allowedPaths.map { it.normalize().toAbsolutePath() }.toSet()
|
||||
|
||||
private fun resolvePath(pathString: String): Path {
|
||||
val raw = Paths.get(pathString)
|
||||
return when {
|
||||
raw.isAbsolute -> raw.normalize()
|
||||
workingDir != null -> workingDir.resolve(raw).normalize()
|
||||
else -> raw.toAbsolutePath().normalize()
|
||||
}
|
||||
}
|
||||
|
||||
override val name: String = "file_edit"
|
||||
override val description: String = "Edit content in a file at the specified path"
|
||||
override val parametersSchema: JsonObject = buildJsonObject {}
|
||||
@@ -35,7 +45,7 @@ class FileEditTool(
|
||||
|
||||
override fun affectedPaths(request: ToolRequest): Set<Path> {
|
||||
val pathString = request.parameters["path"] as? String ?: return emptySet()
|
||||
return setOf(Paths.get(pathString).normalize().toAbsolutePath())
|
||||
return setOf(resolvePath(pathString))
|
||||
}
|
||||
|
||||
override fun validateRequest(request: ToolRequest): ValidationResult {
|
||||
@@ -50,7 +60,7 @@ class FileEditTool(
|
||||
ValidationResult.Invalid("Missing 'path' parameter. Expected String.")
|
||||
|
||||
else -> runCatching {
|
||||
val path = Paths.get(pathString).normalize().toAbsolutePath()
|
||||
val path = resolvePath(pathString)
|
||||
checkPathAllowed(path, pathString, operation, request)
|
||||
}.getOrElse { e ->
|
||||
mapExceptionToValidationResult(e)
|
||||
@@ -116,7 +126,7 @@ class FileEditTool(
|
||||
|
||||
val operation = request.parameters["operation"] as String
|
||||
val pathString = request.parameters["path"] as String
|
||||
val path = Paths.get(pathString).normalize().toAbsolutePath()
|
||||
val path = resolvePath(pathString)
|
||||
|
||||
runCatching {
|
||||
performOperation(operation, path, pathString, request)
|
||||
|
||||
+3
-7
@@ -18,10 +18,10 @@ import java.nio.file.InvalidPathException
|
||||
import java.nio.file.Path
|
||||
import java.nio.file.Paths
|
||||
|
||||
@Suppress("UnusedParameter")
|
||||
class FileReadTool(
|
||||
allowedPaths: Set<Path> = emptySet(),
|
||||
) : Tool, ToolExecutor {
|
||||
private val normalizedAllowedPaths: Set<Path> = allowedPaths.map { it.normalize().toAbsolutePath() }.toSet()
|
||||
|
||||
override val name: String = "file_read"
|
||||
override val description: String = "Read content from a file at the specified path"
|
||||
@@ -32,12 +32,8 @@ class FileReadTool(
|
||||
override fun validateRequest(request: ToolRequest): ValidationResult =
|
||||
(request.parameters["path"] as? String)?.let { pathString ->
|
||||
runCatching {
|
||||
val path = Paths.get(pathString).normalize().toAbsolutePath()
|
||||
val isAllowed = normalizedAllowedPaths.isEmpty() || path in normalizedAllowedPaths
|
||||
|| normalizedAllowedPaths.any { path.startsWith(it) }
|
||||
|
||||
ValidationResult.Valid.takeIf { isAllowed }
|
||||
?: ValidationResult.Invalid("Path '$pathString' is not in the allowed list.")
|
||||
Paths.get(pathString)
|
||||
ValidationResult.Valid
|
||||
}.getOrElse {
|
||||
when (it) {
|
||||
is InvalidPathException -> ValidationResult.Invalid("Invalid path format: ${it.message}")
|
||||
|
||||
+26
-7
@@ -1,11 +1,11 @@
|
||||
package com.correx.infrastructure.tools.filesystem
|
||||
|
||||
import com.correx.core.approvals.Tier
|
||||
import com.correx.core.artifactstore.ArtifactStore
|
||||
import com.correx.core.artifacts.MaterializationResult
|
||||
import com.correx.core.artifacts.MaterializingArtifactWriter
|
||||
import com.correx.core.artifacts.kind.FileWrittenArtifact
|
||||
import com.correx.core.artifacts.kind.FileWrittenPayload
|
||||
import com.correx.core.artifacts.MaterializationResult
|
||||
import com.correx.core.artifactstore.ArtifactStore
|
||||
import com.correx.core.events.events.ToolRequest
|
||||
import com.correx.core.events.types.ToolInvocationId
|
||||
import com.correx.core.tools.contract.FileAffectingTool
|
||||
@@ -36,15 +36,17 @@ class FileWriteTool(
|
||||
private val artifactStore: ArtifactStore? = null,
|
||||
private val materializingWriter: MaterializingArtifactWriter? = null,
|
||||
private val sandboxRoot: Path? = null,
|
||||
private val workingDir: Path? = null,
|
||||
) : Tool, FileAffectingTool, ToolExecutor {
|
||||
|
||||
private companion object {
|
||||
val log = LoggerFactory.getLogger(FileWriteTool::class.java)
|
||||
}
|
||||
|
||||
private val normalizedAllowedPaths: Set<Path> = allowedPaths.map { it.normalize().toAbsolutePath() }.toSet()
|
||||
|
||||
override val name: String = "file_write"
|
||||
override val description: String = "Write content to a file at the specified path"
|
||||
override val description: String = "Write content to a file at the specified path or delete the file entirely"
|
||||
override val parametersSchema: JsonObject = buildJsonObject {
|
||||
put("type", "object")
|
||||
putJsonObject("properties") {
|
||||
@@ -56,15 +58,31 @@ class FileWriteTool(
|
||||
put("type", "string")
|
||||
put("description", "File content")
|
||||
}
|
||||
putJsonObject("operation") {
|
||||
put("type", "string")
|
||||
put("description", "Either 'write' or 'delete'")
|
||||
}
|
||||
}
|
||||
put("required", buildJsonArray { add(JsonPrimitive("path")); add(JsonPrimitive("content")) })
|
||||
put(
|
||||
"required",
|
||||
buildJsonArray { add(JsonPrimitive("path")); add(JsonPrimitive("content")); add(JsonPrimitive("operation")) },
|
||||
)
|
||||
}
|
||||
override val tier: Tier = Tier.T2
|
||||
override val requiredCapabilities: Set<ToolCapability> = setOf(ToolCapability.FILE_WRITE)
|
||||
|
||||
override fun affectedPaths(request: ToolRequest): Set<Path> {
|
||||
val pathString = request.parameters["path"] as? String ?: return emptySet()
|
||||
return setOf(Paths.get(pathString).normalize().toAbsolutePath())
|
||||
return setOf(resolvePath(pathString))
|
||||
}
|
||||
|
||||
private fun resolvePath(pathString: String): Path {
|
||||
val raw = Paths.get(pathString)
|
||||
return when {
|
||||
raw.isAbsolute -> raw.normalize()
|
||||
workingDir != null -> workingDir.resolve(raw).normalize()
|
||||
else -> raw.toAbsolutePath().normalize()
|
||||
}
|
||||
}
|
||||
|
||||
override fun validateRequest(request: ToolRequest): ValidationResult {
|
||||
@@ -79,7 +97,7 @@ class FileWriteTool(
|
||||
ValidationResult.Invalid("Missing 'path' parameter. Expected String.")
|
||||
|
||||
else -> runCatching {
|
||||
val path = Paths.get(pathString).normalize().toAbsolutePath()
|
||||
val path = resolvePath(pathString)
|
||||
checkPathAllowed(path, pathString, operation, request)
|
||||
}.getOrElse { e ->
|
||||
mapExceptionToValidationResult(e)
|
||||
@@ -133,7 +151,7 @@ class FileWriteTool(
|
||||
|
||||
val operation = request.parameters["operation"] as String
|
||||
val pathString = request.parameters["path"] as String
|
||||
val path = Paths.get(pathString).normalize().toAbsolutePath()
|
||||
val path = resolvePath(pathString)
|
||||
|
||||
runCatching {
|
||||
performOperation(operation, path, pathString, request)
|
||||
@@ -195,6 +213,7 @@ class FileWriteTool(
|
||||
).toByteArray(Charsets.UTF_8)
|
||||
store.put(canonicalBytes)
|
||||
}
|
||||
|
||||
is MaterializationResult.Failure ->
|
||||
log.warn("[FileWriteTool] artifact materialization failed for {}: {}", pathString, result.reason)
|
||||
}
|
||||
|
||||
-1
@@ -1,6 +1,5 @@
|
||||
package com.correx.infrastructure.tools
|
||||
|
||||
import com.correx.core.approvals.Tier
|
||||
import com.correx.core.events.EventDispatcher
|
||||
import com.correx.core.events.events.EventPayload
|
||||
import com.correx.core.events.events.ToolExecutionCompletedEvent
|
||||
|
||||
@@ -23,9 +23,14 @@ data class FileWriteConfig(
|
||||
val artifactStore: ArtifactStore? = null,
|
||||
val materializingWriter: MaterializingArtifactWriter? = null,
|
||||
val sandboxRoot: Path? = null,
|
||||
val workingDir: Path? = null,
|
||||
)
|
||||
data class FileEditConfig(val enabled: Boolean = false, val allowedPaths: Set<Path> = emptySet())
|
||||
data class ShellConfig(val enabled: Boolean = false, val allowedExecutables: Set<String> = emptySet())
|
||||
data class FileEditConfig(
|
||||
val enabled: Boolean = false,
|
||||
val allowedPaths: Set<Path> = emptySet(),
|
||||
val workingDir: Path? = null,
|
||||
)
|
||||
data class ShellConfig(val enabled: Boolean = false, val allowedExecutables: Set<String> = emptySet(), val workingDir: Path? = null)
|
||||
|
||||
/**
|
||||
* Extension function to convert [ToolConfig] into a list of [Tool] implementations.
|
||||
@@ -33,7 +38,8 @@ data class ShellConfig(val enabled: Boolean = false, val allowedExecutables: Set
|
||||
fun ToolConfig.buildTools(): List<Tool> = buildList {
|
||||
if (shell.enabled) {
|
||||
add(ShellTool(
|
||||
allowedExecutables = shell.allowedExecutables
|
||||
allowedExecutables = shell.allowedExecutables,
|
||||
workingDir = shell.workingDir,
|
||||
))
|
||||
}
|
||||
if (fileRead.enabled) {
|
||||
@@ -47,11 +53,13 @@ fun ToolConfig.buildTools(): List<Tool> = buildList {
|
||||
artifactStore = fileWrite.artifactStore,
|
||||
materializingWriter = fileWrite.materializingWriter,
|
||||
sandboxRoot = fileWrite.sandboxRoot,
|
||||
workingDir = fileWrite.workingDir,
|
||||
))
|
||||
}
|
||||
if (fileEdit.enabled) {
|
||||
add(FileEditTool(
|
||||
allowedPaths = fileEdit.allowedPaths
|
||||
allowedPaths = fileEdit.allowedPaths,
|
||||
workingDir = fileEdit.workingDir,
|
||||
))
|
||||
}
|
||||
}
|
||||
|
||||
+4
-2
@@ -14,10 +14,12 @@ import kotlinx.coroutines.withContext
|
||||
import kotlinx.coroutines.withTimeout
|
||||
import kotlinx.serialization.json.JsonObject
|
||||
import kotlinx.serialization.json.buildJsonObject
|
||||
import java.nio.file.Path
|
||||
|
||||
class ShellTool(
|
||||
private val allowedExecutables: Set<String> = emptySet(),
|
||||
private val timeoutMs: Long = 30_000L,
|
||||
private val workingDir: Path? = null,
|
||||
) : Tool, ToolExecutor {
|
||||
override val name: String = "shell"
|
||||
override val description: String = "Execute a shell command"
|
||||
@@ -31,7 +33,7 @@ class ShellTool(
|
||||
return when {
|
||||
argv.isNullOrEmpty() ->
|
||||
ValidationResult.Invalid("Missing or empty 'argv' parameter. Expected List<String>.")
|
||||
argv[0] !in allowedExecutables ->
|
||||
allowedExecutables.isNotEmpty() && argv[0] !in allowedExecutables ->
|
||||
ValidationResult.Invalid("Executable '${argv[0]}' is not in the allowed list.")
|
||||
else -> ValidationResult.Valid
|
||||
}
|
||||
@@ -42,7 +44,7 @@ class ShellTool(
|
||||
takeIf { this is ValidationResult.Valid }?.let {
|
||||
val argv = (request.parameters["argv"] as? List<*>)?.filterIsInstance<String>()
|
||||
val process = ProcessBuilder(argv).apply {
|
||||
environment().clear()
|
||||
workingDir?.let { directory(it.toFile()) }
|
||||
}.start()
|
||||
runCatching {
|
||||
runCmd(request, process)
|
||||
|
||||
Reference in New Issue
Block a user