feat(security): Provenance + CapabilityAware + WriteFileTool migration (#480 PR 2/3) #482
refactor(security): rename Provenance.legacy and extract ToolPermissi…
…onRouter Provenance.legacy → fromNullableSessionId. The previous name implied legacy-only callers, but the dispatcher uses it too (PR 3 will fill in the chain). The new name describes what callers actually have — a possibly-null raw String id — without diluting the migration grep signal. ToolPermissionRouter extracts the structured-vs-legacy permission branching from StreamingAgentHandler so the contract is testable without standing up the agent loop. New ToolPermissionRouterTest pins all six branches: structured path, legacy path, toCapability throw → fallback, toCapability null → fail-fast, policy errors propagate, null sessionId works. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Loading branch information
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,112 @@ | ||
| package dev.aceclaw.daemon; | ||
|
|
||
| import com.fasterxml.jackson.databind.ObjectMapper; | ||
| import dev.aceclaw.core.agent.Tool; | ||
| import dev.aceclaw.security.Capability; | ||
| import dev.aceclaw.security.CapabilityAware; | ||
| import dev.aceclaw.security.PermissionDecision; | ||
| import dev.aceclaw.security.PermissionLevel; | ||
| import dev.aceclaw.security.PermissionManager; | ||
| import dev.aceclaw.security.PermissionRequest; | ||
| import dev.aceclaw.security.Provenance; | ||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
|
|
||
| import java.io.IOException; | ||
| import java.util.Objects; | ||
|
|
||
| /** | ||
| * Picks the right entry point on {@link PermissionManager} for a given tool | ||
| * call (#480 PR 2). Extracted from {@link StreamingAgentHandler} so the | ||
| * branching contract — "tools that implement {@link CapabilityAware} take | ||
| * the structured path; everything else takes legacy" — is testable without | ||
| * standing up the full agent loop. | ||
| * | ||
| * <h3>Three outcomes for a {@link CapabilityAware} tool</h3> | ||
| * | ||
| * <ol> | ||
| * <li>{@code toCapability(...)} returns a non-null {@link Capability} → | ||
| * call {@link PermissionManager#check(Capability, Provenance, String, String)}. | ||
| * The originating tool's name is the allowlist key (so historical | ||
| * "always allow {@code write_file}" approvals keep applying), and the | ||
| * caller-supplied human description carries through to the user prompt.</li> | ||
| * <li>{@code toCapability(...)} throws {@link RuntimeException} or | ||
| * {@link IOException} (parse failure, bad args) → log a warning and | ||
| * fall back to the legacy {@link PermissionRequest} path so the user | ||
| * still gets a meaningful approval prompt instead of an opaque crash.</li> | ||
| * <li>{@code toCapability(...)} returns {@code null} → contract violation; | ||
| * throw {@link IllegalStateException}. Silently downgrading to legacy | ||
| * here would mask a broken migration and drop structured policy/audit | ||
| * data. (Codex + CodeRabbit reviews on #482.)</li> | ||
| * </ol> | ||
| * | ||
| * <h3>What does NOT trigger the fallback</h3> | ||
| * | ||
| * Errors raised by {@code permissionManager.check} itself (policy / audit / | ||
| * runtime) are propagated as-is. Re-running the legacy path on those would | ||
| * mask real bugs and could change the decision pipeline behind the user's | ||
| * back. The fallback is strictly for the args-to-Capability conversion step. | ||
| */ | ||
| final class ToolPermissionRouter { | ||
|
|
||
| private static final Logger log = LoggerFactory.getLogger(ToolPermissionRouter.class); | ||
|
|
||
| private ToolPermissionRouter() { /* static-only */ } | ||
|
|
||
| /** | ||
| * Routes a tool call to the structured or legacy permission entry point. | ||
| * | ||
| * @param delegate the tool being invoked (may or may not be {@link CapabilityAware}) | ||
| * @param inputJson the JSON args the LLM supplied to the tool | ||
| * @param sessionId the owning session id, or {@code null} for daemon-internal calls | ||
| * @param description rich human-readable description for the approval prompt | ||
| * @param fallbackLevel risk level used when going through the legacy path | ||
| * @param permissionManager the manager that evaluates policy | ||
| * @param mapper Jackson mapper, used to parse {@code inputJson} for capability conversion | ||
| * @return the manager's decision | ||
| * @throws IllegalStateException if a {@link CapabilityAware} tool's | ||
| * {@code toCapability(...)} returns {@code null} (contract violation) | ||
| */ | ||
| static PermissionDecision check( | ||
| Tool delegate, | ||
| String inputJson, | ||
| String sessionId, | ||
| String description, | ||
| PermissionLevel fallbackLevel, | ||
| PermissionManager permissionManager, | ||
| ObjectMapper mapper) { | ||
| Objects.requireNonNull(delegate, "delegate"); | ||
| Objects.requireNonNull(inputJson, "inputJson"); | ||
| Objects.requireNonNull(description, "description"); | ||
| Objects.requireNonNull(fallbackLevel, "fallbackLevel"); | ||
| Objects.requireNonNull(permissionManager, "permissionManager"); | ||
| Objects.requireNonNull(mapper, "mapper"); | ||
|
|
||
| if (!(delegate instanceof CapabilityAware capAware)) { | ||
| var legacyRequest = new PermissionRequest(delegate.name(), description, fallbackLevel); | ||
| return permissionManager.check(legacyRequest, sessionId); | ||
| } | ||
|
|
||
| Capability capability; | ||
| boolean conversionThrew = false; | ||
| try { | ||
| capability = capAware.toCapability(mapper.readTree(inputJson)); | ||
| } catch (RuntimeException | IOException toCapErr) { | ||
| log.warn("CapabilityAware tool {} rejected args; falling back to legacy permission path: {}", | ||
| delegate.name(), toCapErr.getMessage()); | ||
| capability = null; | ||
| conversionThrew = true; | ||
| } | ||
| if (capability == null && !conversionThrew) { | ||
| throw new IllegalStateException( | ||
| "CapabilityAware tool " + delegate.name() | ||
| + " returned null capability (contract violation)"); | ||
| } | ||
| if (capability != null) { | ||
| var provenance = Provenance.fromNullableSessionId(sessionId); | ||
| return permissionManager.check(capability, provenance, delegate.name(), description); | ||
| } | ||
| var legacyRequest = new PermissionRequest(delegate.name(), description, fallbackLevel); | ||
| return permissionManager.check(legacyRequest, sessionId); | ||
| } | ||
| } | ||
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ToolPermissionRouter.checknow hard-fails withObjects.requireNonNull(inputJson), but the surrounding execution path intentionally tolerates missing/invalid tool args (e.g.,PermissionAwareTool.executecatches parse errors and continues, andContentBlock.ToolUsecan carry nullable input). With this guard, any tool call that arrives without an arguments payload throws an NPE before permission evaluation or legacy fallback, turning a previously recoverable flow into an unhandled failure. Please treat null args as an empty object (or route directly to legacy) instead of rejecting at the router boundary.Useful? React with 👍 / 👎.