fix(toolintent): key the read-before-write exemption on content provenance, not parameter shape
The exemption added in 6a8a7b31 was broader than the invariant it stood on. "Tool
declares a SOURCE_PATH" is a claim about the parameter list; the safe property is
"every byte written derives from an existing source object rather than from
model-supplied content". A future transform or import tool could name a source and
still write model-controlled output, and would have inherited the exemption.
ToolCapability.CONTENT_FROM_SOURCE now carries that provenance claim explicitly.
file_copy declares it; ReadBeforeWriteRule.appliesTo stands down only for calls that
do, so ToolCallAssessor skips the rule rather than the rule skipping itself. The
capability is recorded on the invocation event like every other one, so replay
classifies a call by what it actually claimed instead of re-deriving it from
parameters.
Tool availability is by declared tool name, not capability-set containment, so the
extra capability does not narrow which stages can reach file_copy.
Tests: the exemption is asserted through ToolCallAssessor, plus a source-naming tool
WITHOUT the provenance capability that stays gated. ./gradlew check green.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -22,6 +22,7 @@ CORREX kernel team. This is the most cross-cutting module in the codebase — ch
|
||||
- Domain event files: `ApprovalEvents`, `ArtifactEvents`, `ContextEvents`, `InferenceEvents`, `OrchestrationEvents`, `RouterEvents`, `SessionEvents`, `TaskEvents`, `ToolEvents`, `IntentEvents`, `RiskAssessedEvent`, `JournalCompactedEvent`, and many more — all payload definitions live here.
|
||||
- `FailureAttribution` / `FailureAttributor` — the terminal-failure taxonomy (`AGENT`, `HARNESS`, `WORKFLOW`, `ENVIRONMENT`, `PROVIDER`, `OPERATOR`, `UNKNOWN`) carried as `WorkflowFailedEvent.attribution`, plus the deterministic reason→layer mapping used both at emission and when classifying historical events. One primary attribution per terminal event; a multi-cause chain is the session's `FailureTicketOpenedEvent`s, not a second structure.
|
||||
- `LspDiagnosticsCompletedEvent` records pulled language-server diagnostics or a graceful skip reason; replay consumes this observation and never contacts the server.
|
||||
- `ToolCapability.CONTENT_FROM_SOURCE` — content-provenance claim: the bytes a call writes derive entirely from an existing source object it names, never from model output. Recorded on the invocation event like every other capability, so replay classifies the call by what it actually claimed. Only declare it on a tool whose output is a faithful reproduction of its source.
|
||||
- Shared vocabulary: `IdentityTypes` (SessionId, TaskId, etc.), `Tier`, `TokenUsage`, `ToolReceipt`, `ToolRequest`, `RiskLevel`, `RetryPolicy`, `GrantScope`, `GrantLedger`.
|
||||
|
||||
## Work Guidance
|
||||
|
||||
@@ -23,6 +23,18 @@ enum class ToolCapability {
|
||||
*/
|
||||
DIRECTORY_LIST,
|
||||
FILE_WRITE,
|
||||
|
||||
/**
|
||||
* Content provenance: every byte this call writes is derived from an existing source object the
|
||||
* call names (a file on disk, a stored artifact), never from model-supplied content. It is a
|
||||
* claim about WHERE the bytes come from, not about the shape of the parameter list — a transform
|
||||
* or import tool that mixes in model-authored output must NOT declare it.
|
||||
*
|
||||
* Carried alongside [FILE_WRITE] (such a call still mutates the filesystem) so the gates that
|
||||
* exist to stop a model writing from memory can stand down: requiring a prior `file_read` of a
|
||||
* copied file's bytes is unsatisfiable for a binary and defeats the point of copying it.
|
||||
*/
|
||||
CONTENT_FROM_SOURCE,
|
||||
NETWORK_ACCESS,
|
||||
SHELL_EXEC,
|
||||
PROCESS_SPAWN,
|
||||
|
||||
@@ -34,7 +34,7 @@ CORREX kernel team. This module enforces Hard Invariant #9 for the tool-call pat
|
||||
- Hard Invariant #5: every tool call must be assessed before execution. Assessment result is recorded as `ToolCallAssessmentEvents` in `core:events`.
|
||||
- New rules implement `ToolCallRule` and are registered in `WorkspacePolicy`. Do not add rule logic directly to `ToolCallAssessor`.
|
||||
- Resolve every model-supplied path through `ToolPath.resolve` (`core:tools`) — the one canonical normalization rule, shared with the filesystem tools. A rule that resolves paths itself will judge a different path than the tool operates on (the `~` bug: `~/x` resolved to `<workspace>/~/x`, so a real home-directory file was reported as a non-existent in-workspace file and the out-of-workspace prompt never fired).
|
||||
- A call that declares a `SOURCE_PATH` takes its content from disk, not from the model, so `ReadBeforeWriteRule` exempts it: requiring a read of a copied file's bytes is unsatisfiable for binaries and defeats the purpose of the tool.
|
||||
- `ReadBeforeWriteRule` exempts calls declaring `ToolCapability.CONTENT_FROM_SOURCE` — every byte written comes from an existing source object, so there is no model-authored content to clobber with, and requiring a read of a copied binary is unsatisfiable. The exemption keys on that declared PROVENANCE, never on the presence of a `SOURCE_PATH` parameter: a transform or import tool may name a source and still write model-controlled output, and must stay gated. It lives in `appliesTo`, so `ToolCallAssessor` skips the rule entirely.
|
||||
|
||||
## Verification
|
||||
|
||||
|
||||
+8
-8
@@ -6,7 +6,6 @@ import com.correx.core.toolintent.ToolCallAssessment
|
||||
import com.correx.core.toolintent.ToolCallAssessmentInput
|
||||
import com.correx.core.toolintent.ToolCallRule
|
||||
import com.correx.core.toolintent.maxAction
|
||||
import com.correx.core.tools.contract.ParamRole
|
||||
import com.correx.core.tools.contract.ToolCapability
|
||||
import com.correx.core.tools.contract.ToolPath
|
||||
import com.correx.core.validation.model.ValidationIssue
|
||||
@@ -26,15 +25,16 @@ import java.nio.file.Path
|
||||
class ReadBeforeWriteRule : ToolCallRule {
|
||||
|
||||
override fun appliesTo(capabilities: Set<ToolCapability>): Boolean =
|
||||
ToolCapability.FILE_WRITE in capabilities
|
||||
ToolCapability.FILE_WRITE in capabilities &&
|
||||
// A call whose bytes come entirely from an existing source object
|
||||
// ([ToolCapability.CONTENT_FROM_SOURCE]) has nothing for this gate to protect: there is
|
||||
// no model-authored content to clobber the file with. The exemption keys on that
|
||||
// declared provenance, NOT on the presence of a source-path parameter — a future
|
||||
// transform or import tool could name a source and still write model-controlled output,
|
||||
// and must stay gated.
|
||||
ToolCapability.CONTENT_FROM_SOURCE !in capabilities
|
||||
|
||||
override fun assess(input: ToolCallAssessmentInput): ToolCallAssessment {
|
||||
// A call that declares a SOURCE_PATH takes its content from a file on disk, not from the
|
||||
// model's memory, so there is nothing for this gate to protect: demanding a file_read of the
|
||||
// source (or of the target being replaced by it) would force the very bytes-through-context
|
||||
// round-trip that such tools exist to avoid — and is unsatisfiable for a binary file.
|
||||
if (input.paramRoles.containsValue(ParamRole.SOURCE_PATH)) return ToolCallAssessment()
|
||||
|
||||
val root = input.workspace.workspaceRoot
|
||||
val readReal = input.session.reads.map { realOf(input, root, it) }.toSet()
|
||||
|
||||
|
||||
+27
-3
@@ -14,6 +14,7 @@ import com.correx.core.tools.contract.ToolCapability
|
||||
import java.nio.file.Path
|
||||
import kotlin.test.Test
|
||||
import kotlin.test.assertEquals
|
||||
import kotlin.test.assertFalse
|
||||
import kotlin.test.assertTrue
|
||||
|
||||
/**
|
||||
@@ -114,14 +115,17 @@ class PathNormalizationRuleTest {
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a content-from-disk call is exempt from read-before-write even when the dest exists`() {
|
||||
fun `a content-from-source call is exempt from read-before-write`() {
|
||||
// Overwriting an existing binary via a copy must not demand a file_read of it first — that
|
||||
// read is impossible to satisfy usefully and is exactly the round-trip file_copy removes.
|
||||
// The exemption lives in appliesTo, so ToolCallAssessor never runs the gate for such a call.
|
||||
val exempt = setOf(ToolCapability.FILE_WRITE, ToolCapability.CONTENT_FROM_SOURCE)
|
||||
assertFalse(ReadBeforeWriteRule().appliesTo(exempt))
|
||||
val dest = Path.of("/work/project/public/logo.png")
|
||||
val r = ReadBeforeWriteRule().assess(
|
||||
val r = ToolCallAssessor(listOf(ReadBeforeWriteRule())).assess(
|
||||
input(
|
||||
mapOf("source" to "assets/logo.png", "dest" to "public/logo.png"),
|
||||
setOf(ToolCapability.FILE_WRITE),
|
||||
exempt,
|
||||
FakeProbe(existing = setOf(dest)),
|
||||
paramRoles = mapOf("source" to ParamRole.SOURCE_PATH, "dest" to ParamRole.PATH),
|
||||
tool = "file_copy",
|
||||
@@ -131,6 +135,26 @@ class PathNormalizationRuleTest {
|
||||
assertTrue(r.issues.isEmpty())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `naming a source does not by itself earn the exemption`() {
|
||||
// The invariant is content provenance, not parameter shape: a hypothetical transform tool
|
||||
// that reads a source AND writes model-authored output stays gated.
|
||||
val target = "/work/project/src/A.kt"
|
||||
val capabilities = setOf(ToolCapability.FILE_WRITE)
|
||||
assertTrue(ReadBeforeWriteRule().appliesTo(capabilities))
|
||||
val r = ReadBeforeWriteRule().assess(
|
||||
input(
|
||||
mapOf("source" to "template.kt", "path" to target),
|
||||
capabilities,
|
||||
FakeProbe(existing = setOf(Path.of(target))),
|
||||
paramRoles = mapOf("source" to ParamRole.SOURCE_PATH, "path" to ParamRole.PATH),
|
||||
tool = "file_transform",
|
||||
),
|
||||
)
|
||||
assertEquals(RiskAction.BLOCK, r.disposition)
|
||||
assertEquals("READ_BEFORE_WRITE", r.issues.single().code)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a model-authored write still requires a prior read`() {
|
||||
val target = "/work/project/src/A.kt"
|
||||
|
||||
@@ -16,7 +16,7 @@ Adapter for `core:tools`. Depends on `core:tools`, `core:events`, `core:approval
|
||||
- Web search and web fetch results are environment observations; they must be recorded as events by callers to preserve replay determinism (invariant #9).
|
||||
- `ToolConfig` is the only configuration surface; pass via `InfrastructureModule.createToolExecutor()`.
|
||||
- `buildTools()` extension on `ToolConfig` assembles the full tool list; add new tools there, not in the registry directly.
|
||||
- `file_copy` copies one existing file to another path (`{source, dest}`) so static/binary assets never pass through the model's token stream. Same jail, anchor and `fileWrite.enabled` toggle as `file_write`; `dest` is the mutated target (`ParamRole.PATH`, the only affected path), `source` is read-only (`ParamRole.SOURCE_PATH`) and may sit under an operator-granted out-of-workspace path exactly as a `file_read` may. One regular file per call: no recursion, no globs.
|
||||
- `file_copy` copies one existing file to another path (`{source, dest}`) so static/binary assets never pass through the model's token stream. Same jail, anchor and `fileWrite.enabled` toggle as `file_write`; It declares `ToolCapability.CONTENT_FROM_SOURCE` (its bytes are a faithful copy of `source`), which is what exempts it from the read-before-write gate — a tool that mixes model-authored output into its result must not declare it. `dest` is the mutated target (`ParamRole.PATH`, the only affected path), `source` is read-only (`ParamRole.SOURCE_PATH`) and may sit under an operator-granted out-of-workspace path exactly as a `file_read` may. One regular file per call: no recursion, no globs.
|
||||
- Every path parameter resolves through `ToolPath.resolve` (`core:tools`), which expands a leading `~` and anchors relatives on the bound workspace's working dir. Do not re-implement path resolution in a tool.
|
||||
- Filesystem mutation is split by intent: `file_write` only writes (`{path, content}`), `file_edit` edits, and `file_delete` only deletes (`{path}`) — deletion is a separately-named capability so a model can never delete by getting a write-mode parameter wrong. `file_delete` shares `file_write`'s path jail and `fileWrite.enabled` toggle and carries `ToolCapability.FILE_WRITE`.
|
||||
- `list_dir` is shallow by default, but collapses a non-symlink single-child directory chain (bounded depth) to the first branch point and explains that expansion in its output; recursive listings retain normal tree traversal.
|
||||
|
||||
+5
-1
@@ -76,7 +76,11 @@ class FileCopyTool(
|
||||
)
|
||||
}
|
||||
override val tier: Tier = Tier.T2
|
||||
override val requiredCapabilities: Set<ToolCapability> = setOf(ToolCapability.FILE_WRITE)
|
||||
// CONTENT_FROM_SOURCE is the provenance claim: the bytes written come entirely from the file
|
||||
// named by `source`. Declared here, recorded on the invocation event, and replayed — not
|
||||
// re-derived from the parameter list.
|
||||
override val requiredCapabilities: Set<ToolCapability> =
|
||||
setOf(ToolCapability.FILE_WRITE, ToolCapability.CONTENT_FROM_SOURCE)
|
||||
override val paramRoles: Map<String, ParamRole> =
|
||||
mapOf("source" to ParamRole.SOURCE_PATH, "dest" to ParamRole.PATH)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user