feat(guardrails): steering channel + shell-in-file rule + capability-gap detector
Bundles three operator-reliability guardrails (Vikunja #28/#29/#30) plus the in-flight branch WIP they were built on top of (reasoning_content capture, operator/project profile editor, write-jail workspaceRoot fix) — the tree is interdependent (SessionOrchestrator references reasoningArtifactId from the WIP) and does not compile as separable subsets, so it lands as one commit. Guardrails: - #28 mid-stage steering: ClientMessage.SteerSession -> GlobalStreamHandler -> orchestrator.submitSteering, reusing SteeringNoteAddedEvent + existing context fold (advisory, non-authoritative; invariants #3/#7). Closes the gap where steering typed off an approval gate was silently dropped. - #29 shell-in-file guardrail: ShellInFileContentRule (core:toolintent) blocks a file_write whose content is a bare shell command (e.g. "mkdir -p ..."); FileWriteTool description now advertises auto-mkdir of parent dirs. Basename-allowlist so the extensionless case is caught; scripts/Makefiles/multiline exempt. - #30 pt1 capability-gap detector: deterministic CapabilityGapDetector maps stage intent -> implied ToolCapability, compares to granted tools, emits advisory CapabilityGapDetectedEvent in FreestyleDriver.lockAndRun. Recorded, never fails the gate and never auto-grants (invariants #3/#4/#5). Reflection rung is pt2. Verified: ./gradlew check green (whole tree).
This commit is contained in:
+1
@@ -145,6 +145,7 @@ class OpenAiCompatInferenceProvider(
|
||||
),
|
||||
latencyMs = System.currentTimeMillis() - startTime,
|
||||
toolCalls = toolCalls,
|
||||
reasoning = message.reasoningContent?.takeIf { it.isNotBlank() },
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
+3
-1
@@ -39,7 +39,9 @@ class FileWriteTool(
|
||||
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 relative path (creates or overwrites)"
|
||||
override val description: String = "Write content to a file at the specified relative path (creates or overwrites). " +
|
||||
"Writing to a nested path auto-creates any missing parent directories — there is no separate " +
|
||||
"mkdir step. Do not write shell commands into a file's content."
|
||||
override val parametersSchema: JsonObject = buildJsonObject {
|
||||
put("type", "object")
|
||||
putJsonObject("properties") {
|
||||
|
||||
+38
@@ -70,12 +70,44 @@ class ShellTool(
|
||||
when (val parsed = parseArgv(request.parameters["argv"])) {
|
||||
is ArgvParse.Bad -> ValidationResult.Invalid(parsed.reason)
|
||||
is ArgvParse.Ok -> when {
|
||||
// Baseline denylist runs FIRST, ahead of the allowlist — a hard floor that holds even
|
||||
// when allowedExecutables is empty (allow-all). Blocks the class of command that
|
||||
// fetches+executes remote code or prompts interactively: an unattended run has no TTY,
|
||||
// so a create-* scaffolder / npx blocks forever (or balloons the machine). The T2 gate
|
||||
// is not enough — an auto-approve loop waves these straight through.
|
||||
deniedReason(parsed.argv) != null ->
|
||||
ValidationResult.Invalid(deniedReason(parsed.argv)!!)
|
||||
allowedExecutables.isNotEmpty() && parsed.argv[0] !in allowedExecutables ->
|
||||
ValidationResult.Invalid("Executable '${parsed.argv[0]}' is not in the allowed list.")
|
||||
else -> ValidationResult.Valid
|
||||
}
|
||||
}
|
||||
|
||||
// Return a rejection reason if argv is a fetch-and-execute-remote-code or interactive-scaffolder
|
||||
// invocation, else null. Matched by shape, not just argv[0], because the danger for package
|
||||
// managers is the *subcommand* (`npm install` is fine; `npm create`/`npm init` scaffolds and
|
||||
// prompts). Deterministic, allowlist-independent — see [validateRequest].
|
||||
private fun deniedReason(argv: List<String>): String? {
|
||||
// Scan every token, not just argv[0]: a scaffolder can hide inside a shell command line that
|
||||
// runs via `sh -c` (`cd web && npm create vite`), where argv[0] is `cd`.
|
||||
val progs = argv.map { it.substringAfterLast('/') }
|
||||
val redirect = "Scaffold by writing files directly with file_write (package.json, configs, " +
|
||||
"sources) — do not shell out to a network installer; it needs a TTY and will hang unattended."
|
||||
val runner = progs.firstOrNull { it in REMOTE_EXEC_RUNNERS }
|
||||
val scaffolder = progs.firstOrNull { it.startsWith("create-") }
|
||||
val pmIdx = progs.indexOfFirst { it in PACKAGE_MANAGERS }
|
||||
return when {
|
||||
runner != null ->
|
||||
"'$runner' fetches and runs remote code with no TTY, which hangs an unattended run. $redirect"
|
||||
scaffolder != null ->
|
||||
"'$scaffolder' is an interactive project scaffolder that prompts on stdin. $redirect"
|
||||
pmIdx >= 0 && progs.getOrNull(pmIdx + 1) in SCAFFOLD_SUBCOMMANDS ->
|
||||
"'${progs[pmIdx]} ${progs[pmIdx + 1]}' scaffolds a project interactively " +
|
||||
"(prompts on stdin). $redirect"
|
||||
else -> null
|
||||
}
|
||||
}
|
||||
|
||||
private sealed interface ArgvParse {
|
||||
data class Ok(val argv: List<String>) : ArgvParse
|
||||
data class Bad(val reason: String) : ArgvParse
|
||||
@@ -179,6 +211,12 @@ class ShellTool(
|
||||
const val EMPTY_MSG = "Missing or empty 'argv' parameter. Expected a JSON array of strings."
|
||||
val SHELL_BUILTINS = setOf("cd", "export", "source", ".", "pushd", "popd", "umask", "ulimit")
|
||||
val SHELL_OPERATORS = setOf("&&", "||", "|", ">", ">>", "<", ";", "&", "2>", "2>&1")
|
||||
// Runners that download and execute an arbitrary remote package in one shot.
|
||||
val REMOTE_EXEC_RUNNERS = setOf("npx", "bunx", "pnpx")
|
||||
val PACKAGE_MANAGERS = setOf("npm", "yarn", "pnpm", "bun")
|
||||
// Package-manager subcommands that generate a project by pulling+running a remote scaffolder
|
||||
// and prompting on stdin (npm create vite, yarn create next-app, pnpm dlx, …).
|
||||
val SCAFFOLD_SUBCOMMANDS = setOf("create", "init", "dlx")
|
||||
}
|
||||
|
||||
private suspend fun runCmd(request: ToolRequest, process: Process): ToolResult = withTimeout(timeoutMs) {
|
||||
|
||||
+49
@@ -180,4 +180,53 @@ class ShellToolTest {
|
||||
fun `validateRequest accepts a clean string argv`() {
|
||||
assertEquals(ValidationResult.Valid, ShellTool().validateRequest(createRequest(listOf("mkdir", "-p", "dir"))))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `denylist blocks npm create even in allow-all mode`() {
|
||||
// Repro: gemma ran `npm create vite@latest` — a network scaffolder that prompts on stdin.
|
||||
// Empty allowedExecutables = allow-all, yet this must still be rejected.
|
||||
val tool = ShellTool(allowedExecutables = emptySet())
|
||||
val result = tool.validateRequest(createRequest(listOf("npm", "create", "vite")))
|
||||
assertTrue(result is ValidationResult.Invalid)
|
||||
val reason = (result as ValidationResult.Invalid).reason
|
||||
assertTrue(reason.contains("npm create"), reason)
|
||||
assertTrue(reason.contains("file_write"), reason)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `denylist blocks npx remote runner`() {
|
||||
val result = ShellTool().validateRequest(createRequest(listOf("npx", "create-react-app", "app")))
|
||||
assertTrue(result is ValidationResult.Invalid)
|
||||
assertTrue((result as ValidationResult.Invalid).reason.contains("remote code"), result.reason)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `denylist blocks create- scaffolder run directly`() {
|
||||
val result = ShellTool().validateRequest(createRequest(listOf("create-next-app", "app")))
|
||||
assertTrue(result is ValidationResult.Invalid)
|
||||
assertTrue((result as ValidationResult.Invalid).reason.contains("scaffolder"), result.reason)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `denylist catches scaffolder hidden in a shell command line`() {
|
||||
// argv[0] is `cd`, but `npm create` rides along and would run via sh -c.
|
||||
val result = ShellTool().validateRequest(createRequest(listOf("cd", "web", "&&", "npm", "create", "vite")))
|
||||
assertTrue(result is ValidationResult.Invalid)
|
||||
assertTrue((result as ValidationResult.Invalid).reason.contains("npm create"), result.reason)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `denylist allows npm install`() {
|
||||
// The subcommand is what's dangerous — `npm install` on existing files is fine.
|
||||
assertEquals(ValidationResult.Valid, ShellTool().validateRequest(createRequest(listOf("npm", "install"))))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `denylist takes precedence over the allowlist`() {
|
||||
// Even if npm is explicitly allowed, `npm create` stays blocked.
|
||||
val tool = ShellTool(allowedExecutables = setOf("npm"))
|
||||
val result = tool.validateRequest(createRequest(listOf("npm", "create", "vite")))
|
||||
assertTrue(result is ValidationResult.Invalid)
|
||||
assertTrue((result as ValidationResult.Invalid).reason.contains("file_write"), result.reason)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -8,6 +8,7 @@ dependencies {
|
||||
implementation(project(":core:inference"))
|
||||
implementation(project(":core:events"))
|
||||
implementation(project(":core:artifacts"))
|
||||
implementation(project(":core:tools"))
|
||||
implementation("com.fasterxml.jackson.dataformat:jackson-dataformat-toml:2.17.0")
|
||||
implementation("com.fasterxml.jackson.module:jackson-module-kotlin:2.17.0")
|
||||
testImplementation "org.junit.jupiter:junit-jupiter"
|
||||
|
||||
+70
@@ -0,0 +1,70 @@
|
||||
package com.correx.infrastructure.workflow
|
||||
|
||||
import com.correx.core.tools.contract.ToolCapability
|
||||
import com.correx.core.transitions.graph.StageConfig
|
||||
import com.correx.core.transitions.graph.WorkflowGraph
|
||||
|
||||
/**
|
||||
* A stage whose declared intent implies a tool capability it was not granted (Vikunja #30 part 1).
|
||||
* Advisory only — recorded so a future reflection rung (part 2, LLM "are you sure?") can consume it;
|
||||
* it never blocks the plan-compile gate and never grants a tool itself (invariants #3 LLM-proposes/
|
||||
* core-decides, #4 policy-absolute, #5 tiers-declared-not-inferred).
|
||||
*/
|
||||
data class CapabilityGap(
|
||||
val stageId: String,
|
||||
val impliedCapability: ToolCapability,
|
||||
val phrase: String,
|
||||
val grantedCapabilities: Set<ToolCapability>,
|
||||
)
|
||||
|
||||
/**
|
||||
* Deterministic, pure-Kotlin detector over a compiled plan graph — the plan-compile-gate counterpart
|
||||
* to [PlanLinter]'s structural checks. Zero inference, zero I/O: a function of the graph and a fixed
|
||||
* verb map, so it is replay-safe by construction (invariant #8).
|
||||
*
|
||||
* The verb→capability map below is intentionally small and conservative: each entry is a single
|
||||
* word/phrase that, in an imperative stage brief ("write the file", "run the build"), reliably
|
||||
* signals a concrete tool action. Words that are ambiguous or too common in ordinary planning
|
||||
* prose (e.g. "make", "handle", "process", "check") are deliberately excluded — a false positive
|
||||
* here is just a discarded advisory finding, but a map that fires on every brief would drown the
|
||||
* signal the future reflection rung needs.
|
||||
*/
|
||||
object CapabilityGapDetector {
|
||||
|
||||
private val impliedCapabilities: Map<ToolCapability, List<String>> = mapOf(
|
||||
ToolCapability.FILE_WRITE to listOf("write", "create", "generate", "scaffold", "save"),
|
||||
ToolCapability.FILE_READ to listOf("read", "inspect"),
|
||||
ToolCapability.SHELL_EXEC to listOf("run", "build", "execute", "compile"),
|
||||
ToolCapability.NETWORK_ACCESS to listOf("search the web", "fetch a url", "download"),
|
||||
)
|
||||
|
||||
/**
|
||||
* [toolCapabilities] resolves each stage's `allowedTools` name to the capabilities that tool
|
||||
* grants (a tool's [com.correx.core.tools.contract.Tool.requiredCapabilities]) — the caller
|
||||
* supplies it (typically `ToolRegistry.all().associate { it.name to it.requiredCapabilities }`)
|
||||
* so this detector stays a pure function of plain data, not a live registry.
|
||||
*/
|
||||
fun detect(graph: WorkflowGraph, toolCapabilities: Map<String, Set<ToolCapability>>): List<CapabilityGap> =
|
||||
graph.stages.entries.flatMap { (id, stage) ->
|
||||
val granted = grantedCapabilities(stage, toolCapabilities)
|
||||
val brief = briefOf(stage).lowercase()
|
||||
impliedCapabilities.entries.flatMap { (capability, phrases) ->
|
||||
if (capability in granted) {
|
||||
emptyList()
|
||||
} else {
|
||||
phrases.filter { phrase -> containsPhrase(brief, phrase) }
|
||||
.map { phrase -> CapabilityGap(id.value, capability, phrase, granted) }
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private fun grantedCapabilities(
|
||||
stage: StageConfig,
|
||||
toolCapabilities: Map<String, Set<ToolCapability>>,
|
||||
): Set<ToolCapability> = stage.allowedTools.flatMap { toolCapabilities[it].orEmpty() }.toSet()
|
||||
|
||||
private fun briefOf(stage: StageConfig): String = stage.metadata["promptInline"] ?: ""
|
||||
|
||||
private fun containsPhrase(text: String, phrase: String): Boolean =
|
||||
Regex("\\b${Regex.escape(phrase)}\\b").containsMatchIn(text)
|
||||
}
|
||||
+101
@@ -0,0 +1,101 @@
|
||||
package com.correx.infrastructure.workflow
|
||||
|
||||
import com.correx.core.artifacts.kind.ConfigArtifactKind
|
||||
import com.correx.core.artifacts.kind.JsonSchema
|
||||
import com.correx.core.artifacts.kind.TypedArtifactSlot
|
||||
import com.correx.core.events.types.ArtifactId
|
||||
import com.correx.core.events.types.StageId
|
||||
import com.correx.core.events.types.TransitionId
|
||||
import com.correx.core.tools.contract.ToolCapability
|
||||
import com.correx.core.transitions.conditions.AlwaysTrue
|
||||
import com.correx.core.transitions.graph.StageConfig
|
||||
import com.correx.core.transitions.graph.TransitionEdge
|
||||
import com.correx.core.transitions.graph.WorkflowGraph
|
||||
import kotlin.test.Test
|
||||
import kotlin.test.assertEquals
|
||||
import kotlin.test.assertTrue
|
||||
|
||||
class CapabilityGapDetectorTest {
|
||||
|
||||
private fun kind(id: String) =
|
||||
ConfigArtifactKind(id, JsonSchema(type = "object", properties = emptyMap(), additionalProperties = true))
|
||||
|
||||
private fun stage(
|
||||
prompt: String,
|
||||
tools: Set<String> = emptySet(),
|
||||
produces: List<String> = emptyList(),
|
||||
) = StageConfig(
|
||||
produces = produces.map { TypedArtifactSlot(ArtifactId(it), kind(it)) },
|
||||
allowedTools = tools,
|
||||
metadata = mapOf("promptInline" to prompt),
|
||||
)
|
||||
|
||||
private fun graph(stages: Map<String, StageConfig>, start: String = stages.keys.first()) = WorkflowGraph(
|
||||
id = "test",
|
||||
stages = stages.mapKeys { StageId(it.key) },
|
||||
transitions = setOf(TransitionEdge(TransitionId("e0"), StageId(start), StageId("done"), AlwaysTrue)),
|
||||
start = StageId(start),
|
||||
)
|
||||
|
||||
private val fileWriteTool = "file_write" to setOf(ToolCapability.FILE_WRITE)
|
||||
private val fileReadTool = "file_read" to setOf(ToolCapability.FILE_READ)
|
||||
private val shellTool = "shell" to setOf(ToolCapability.SHELL_EXEC)
|
||||
|
||||
@Test
|
||||
fun `a stage that writes a file without FILE_WRITE is a gap`() {
|
||||
val g = graph(mapOf("impl" to stage(prompt = "Create the new config file", tools = emptySet())))
|
||||
val gaps = CapabilityGapDetector.detect(g, emptyMap())
|
||||
val gap = gaps.single()
|
||||
assertEquals("impl", gap.stageId)
|
||||
assertEquals(ToolCapability.FILE_WRITE, gap.impliedCapability)
|
||||
assertEquals("create", gap.phrase)
|
||||
assertTrue(gap.grantedCapabilities.isEmpty())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a stage granted FILE_WRITE has no gap for a write-implying brief`() {
|
||||
val g = graph(mapOf("impl" to stage(prompt = "Create the new config file", tools = setOf(fileWriteTool.first))))
|
||||
val gaps = CapabilityGapDetector.detect(g, mapOf(fileWriteTool))
|
||||
assertTrue(gaps.isEmpty(), "expected no gap, got $gaps")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `the motivating case - a recovery style stage with read-only tools implies write`() {
|
||||
val prompt = "Read the gate output and edit the files to fix the reported cause, then save your work."
|
||||
val g = graph(mapOf("recovery" to stage(prompt = prompt, tools = setOf(fileReadTool.first))))
|
||||
val gaps = CapabilityGapDetector.detect(g, mapOf(fileReadTool))
|
||||
val writeGap = gaps.single { it.impliedCapability == ToolCapability.FILE_WRITE }
|
||||
assertEquals("save", writeGap.phrase)
|
||||
assertTrue(gaps.none { it.impliedCapability == ToolCapability.FILE_READ }, "read is granted, no gap expected")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `run a build without SHELL_EXEC is a gap`() {
|
||||
val g = graph(mapOf("verify" to stage(prompt = "Run the build and report the result")))
|
||||
val gaps = CapabilityGapDetector.detect(g, emptyMap())
|
||||
assertTrue(gaps.any { it.impliedCapability == ToolCapability.SHELL_EXEC && it.phrase == "run" })
|
||||
assertTrue(gaps.any { it.impliedCapability == ToolCapability.SHELL_EXEC && it.phrase == "build" })
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a stage with an unrelated brief and no tools has no gaps`() {
|
||||
val g = graph(mapOf("analyse" to stage(prompt = "Summarize the requirements at a high level")))
|
||||
val gaps = CapabilityGapDetector.detect(g, emptyMap())
|
||||
assertTrue(gaps.isEmpty(), "expected no gap, got $gaps")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a stage fully equipped for its brief has no gaps`() {
|
||||
val prompt = "Read the spec, run the build, and create the output file"
|
||||
val g = graph(
|
||||
mapOf(
|
||||
"full" to stage(
|
||||
prompt = prompt,
|
||||
tools = setOf(fileReadTool.first, shellTool.first, fileWriteTool.first),
|
||||
),
|
||||
),
|
||||
)
|
||||
val gaps = CapabilityGapDetector.detect(g, mapOf(fileReadTool, shellTool, fileWriteTool))
|
||||
assertTrue(gaps.isEmpty(), "expected no gap, got $gaps")
|
||||
}
|
||||
}
|
||||
+9
-3
@@ -26,6 +26,7 @@ class FreestylePlanningWorkflowTest {
|
||||
)
|
||||
|
||||
private val registry = DefaultArtifactKindRegistry().apply {
|
||||
register(llmKind("discovery"))
|
||||
register(llmKind("analysis"))
|
||||
register(llmKind("execution_plan"))
|
||||
}
|
||||
@@ -41,17 +42,22 @@ class FreestylePlanningWorkflowTest {
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `freestyle_planning toml parses with two non-terminal stages and both edges`() {
|
||||
fun `freestyle_planning toml parses with discovery start plus analyst and architect stages`() {
|
||||
val path = repoFile("examples/workflows/freestyle_planning.toml")
|
||||
assumeTrue(path != null, "freestyle_planning.toml not found from ${System.getProperty("user.dir")}")
|
||||
|
||||
val graph = TomlWorkflowLoader(registry).load(path!!)
|
||||
|
||||
assertEquals(StageId("discovery"), graph.start)
|
||||
assertEquals(
|
||||
setOf("analyst", "architect"),
|
||||
setOf("discovery", "analyst", "architect"),
|
||||
graph.stages.keys.map { it.value }.toSet(),
|
||||
)
|
||||
|
||||
val discoveryToAnalyst = graph.transitions.first { it.from == StageId("discovery") }
|
||||
assertEquals(StageId("analyst"), discoveryToAnalyst.to)
|
||||
assertTrue(discoveryToAnalyst.condition is ArtifactValidated)
|
||||
|
||||
val architect = graph.stages[StageId("architect")]!!
|
||||
assertTrue(
|
||||
architect.produces.any { it.name.value == "execution_plan" && it.kind.llmEmitted },
|
||||
@@ -68,7 +74,7 @@ class FreestylePlanningWorkflowTest {
|
||||
assertEquals(StageId("done"), architectToDone.to)
|
||||
assertTrue(architectToDone.condition is ArtifactValidated)
|
||||
|
||||
assertEquals(2, graph.transitions.size)
|
||||
assertEquals(3, graph.transitions.size)
|
||||
|
||||
// analyst frames the work: search + open one task or decompose into a graph (the architect
|
||||
// threads the named task into the plan).
|
||||
|
||||
+8
-2
@@ -28,6 +28,7 @@ class RolePipelineWorkflowTest {
|
||||
)
|
||||
|
||||
private val registry = DefaultArtifactKindRegistry().apply {
|
||||
register(llmKind("discovery"))
|
||||
register(llmKind("analysis"))
|
||||
register(llmKind("design"))
|
||||
register(llmKind("review_report"))
|
||||
@@ -54,11 +55,16 @@ class RolePipelineWorkflowTest {
|
||||
assumeTrue(graph != null, "role_pipeline.toml not found from ${System.getProperty("user.dir")}")
|
||||
graph!!
|
||||
|
||||
assertEquals(StageId("discovery"), graph.start)
|
||||
assertEquals(
|
||||
setOf("analyst", "architect", "decomposer", "implementer", "reviewer"),
|
||||
setOf("discovery", "analyst", "architect", "decomposer", "implementer", "reviewer"),
|
||||
graph.stages.keys.map { it.value }.toSet(),
|
||||
)
|
||||
assertEquals(7, graph.transitions.size)
|
||||
assertEquals(8, graph.transitions.size)
|
||||
|
||||
// discovery is the read-only clarification gate that precedes the analyst.
|
||||
val discoveryToAnalyst = graph.transitions.first { it.from == StageId("discovery") }
|
||||
assertEquals(StageId("analyst"), discoveryToAnalyst.to)
|
||||
|
||||
// The decomposer is the mandatory task-graph gate.
|
||||
assertEquals("true", graph.stages[StageId("decomposer")]!!.metadata["requireTaskDecompose"])
|
||||
|
||||
Reference in New Issue
Block a user