Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 83 additions & 0 deletions src/Netclaw.Actors.Tests/Tools/DispatchingToolExecutorTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,14 @@
using Microsoft.Extensions.AI;
using Microsoft.Extensions.Logging;
using Netclaw.Actors.Hosting;
using Netclaw.Actors.Jobs;
using Netclaw.Actors.Tools;
using Netclaw.Configuration;
using Netclaw.Security;
using Netclaw.Tests.Utilities;
using Netclaw.Tools;
using Xunit;
using static Netclaw.Actors.Jobs.BackgroundJobProtocol;

namespace Netclaw.Actors.Tests.Tools;

Expand Down Expand Up @@ -889,6 +891,87 @@ await Assert.ThrowsAsync<ToolApprovalRequiredException>(() =>
}
}

[Fact]
public async Task check_background_job_status_query_from_non_interactive_turn_does_not_require_approval()
{
// Regression: a status query (Cancel=false) is read-only and job-scoped,
// so a non-interactive turn (reminder/webhook) must NOT hit the approval
// gate — nothing can answer the prompt there, so the session wedges.
// Cancel=true stays gated. See BackgroundJobApprovalMatcher.
var config = new ToolConfig { ShellMode = ShellExecutionMode.HostAllowed };
config.AudienceProfiles.Personal.ApprovalPolicy = new ToolApprovalConfig
{
ToolOverrides = new Dictionary<string, ToolApprovalMode>(StringComparer.Ordinal)
{
["shell_execute"] = ToolApprovalMode.Approval
}
};

var system = ActorSystem.Create($"tool-approval-{Guid.NewGuid():N}");
Comment thread
github-code-quality[bot] marked this conversation as resolved.
Fixed
try
{
var fakeJobManager = system.ActorOf(Props.Create(() => new FakeJobStatusManager()), "fake-job-manager");
var registry = new ToolRegistry();
registry.WithFirstPartyTools(config, new NetclawPaths(), new ToolPathPolicy([]), new ShellCommandPolicy());
registry.WithBackgroundJobTools(fakeJobManager);

var approvalActor = system.ActorOf(ToolApprovalActor.CreateProps(), "tool-approval");
var approvalService = new AkkaToolApprovalService(new StubRequiredActor(approvalActor));
var executor = new DispatchingToolExecutor(
registry,
new ToolAccessPolicy(
config,
new EffectivePolicyDefaults(
DeploymentPosture.Personal,
TrustAudience.Personal,
ShellExecutionMode.HostAllowed,
UsedStrictFallback: false),
new ShellCommandPolicy(),
new ToolPathPolicy([])),
approvalService);

var nonInteractiveContext = TestToolExecutionContext.CreateBound("reminder/exec-1", null, new TestToolExecutionContextOptions
{
Audience = TrustAudience.Personal,
Boundary = TrustBoundary.TrustedInstance,
ChannelType = "slack",
InteractiveApproval = TestToolExecutionContext.InteractiveApproval(false)
});

// Status query: must NOT throw ToolApprovalRequiredException.
var statusCall = new FunctionCallContent(
"call-status",
"check_background_job",
ToolInput.Create("JobId", "abc123"));
_ = await executor.ExecuteAsync(statusCall, nonInteractiveContext, TestContext.Current.CancellationToken);

// Cancel: a mutation, must still throw ToolApprovalRequiredException.
var cancelCall = new FunctionCallContent(
"call-cancel",
"check_background_job",
ToolInput.Create("JobId", "abc123", "Cancel", true));
await Assert.ThrowsAsync<ToolApprovalRequiredException>(() =>
executor.ExecuteAsync(cancelCall, nonInteractiveContext, TestContext.Current.CancellationToken));
}
finally
{
await system.Terminate();
}
}

private sealed class FakeJobStatusManager : ReceiveActor
{
public FakeJobStatusManager()
{
Receive<QueryBackgroundJob>(_ => Sender.Tell(new BackgroundJobStatusResponse
{
JobId = new BackgroundJobId("abc123"),
Status = BackgroundJobStatus.Running,
Found = false
}));
}
}

[Fact]
public async Task One_time_approval_bypasses_policy_for_matching_shell_patterns()
{
Expand Down
58 changes: 57 additions & 1 deletion src/Netclaw.Actors.Tests/Tools/ToolApprovalGateTests.cs
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
// -----------------------------------------------------------------------
// -----------------------------------------------------------------------
// <copyright file="ToolApprovalGateTests.cs" company="Petabridge, LLC">
// Copyright (C) 2026 - 2026 Petabridge, LLC <https://petabridge.com>
// </copyright>
// -----------------------------------------------------------------------
using Akka.Actor;
using Microsoft.Extensions.AI;
using Netclaw.Actors.Jobs;
using Netclaw.Actors.Protocol;
using Netclaw.Actors.Tools;
using Netclaw.Configuration;
Expand Down Expand Up @@ -50,6 +52,60 @@ private static INetclawTool ShellTool()
return new ShellTool(config, new ToolPathPolicy([]), new ShellCommandPolicy());
}

// check_background_job is a shell-coupled tool (ToolAccessPolicy.IsShellCoupledTool)
// that routes through ShellApprovalMatcher, whose IsFailClosedOnPersonal returns
// true unconditionally. That forces ToolApprovalMode.Approval for every Personal
// invocation with no explicit override — including pure read-only status queries
// (Cancel=false) scoped to the caller's own jobs. On a non-interactive turn
// (reminder/webhook) nothing can answer the prompt, so the session wedges waiting
// on an approval that never arrives. Status queries are read-only and job-scoped;
// only Cancel=true is a mutation and should stay approval-gated.
[Fact]
public void check_background_job_status_query_does_not_require_approval()
{
var policy = CreatePolicy(ToolApprovalMode.Approval);
var tool = new CheckBackgroundJobTool(ActorRefs.Nobody);
var args = new Dictionary<string, object?> { ["JobId"] = "abc123", ["Cancel"] = false };

var decision = policy.AuthorizeInvocation(tool, PersonalContext(), args);

Assert.True(decision.Allowed);
Assert.False(decision.NeedsApproval);
}

[Fact]
public void check_background_job_status_query_from_non_interactive_automation_does_not_require_approval()
{
var policy = CreatePolicy(ToolApprovalMode.Approval);
var tool = new CheckBackgroundJobTool(ActorRefs.Nobody);
var args = new Dictionary<string, object?> { ["JobId"] = "abc123", ["Cancel"] = false };
var context = TestToolExecutionContext.CreateBound(
"reminder/exec-1",
null,
new TestToolExecutionContextOptions
{
Audience = TrustAudience.Personal,
InteractiveApproval = TestToolExecutionContext.InteractiveApproval(false)
});

var decision = policy.AuthorizeInvocation(tool, context, args);

Assert.True(decision.Allowed);
Assert.False(decision.NeedsApproval);
}

[Fact]
public void check_background_job_cancel_requires_approval()
{
var policy = CreatePolicy(ToolApprovalMode.Approval);
var tool = new CheckBackgroundJobTool(ActorRefs.Nobody);
var args = new Dictionary<string, object?> { ["JobId"] = "abc123", ["Cancel"] = true };

var decision = policy.AuthorizeInvocation(tool, PersonalContext(), args);

Assert.True(decision.NeedsApproval);
}

[Fact]
public void Shell_in_deny_mode_returns_deny()
{
Expand Down
102 changes: 102 additions & 0 deletions src/Netclaw.Actors/Jobs/BackgroundJobApprovalMatcher.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
// -----------------------------------------------------------------------
// <copyright file="BackgroundJobApprovalMatcher.cs" company="Petabridge, LLC">
// Copyright (C) 2026 - 2026 Petabridge, LLC <https://petabridge.com>
// </copyright>
// -----------------------------------------------------------------------
using System.Text.Json;
using Netclaw.Configuration;
using Netclaw.Security;
using Netclaw.Tools;

namespace Netclaw.Actors.Jobs;

/// <summary>
/// Approval matcher for <see cref="CheckBackgroundJobTool"/>.
///
/// <para>
/// A status query (<c>Cancel=false</c>) is read-only: it asks the
/// <c>BackgroundJobManagerActor</c> for the caller's own job state and the
/// manager enforces an exact <c>SessionId + Audience + Boundary</c> match
/// before answering, so no cross-session data can leak. Status queries must
/// NOT be fail-closed on the Personal audience — otherwise a non-interactive
/// turn (reminder, webhook, sub-agent without an approval bridge) wedges
/// waiting on an approval prompt no one can answer, which stalls the very
/// background-job workflow the tool exists to serve.
/// </para>
///
/// <para>
/// A cancellation (<c>Cancel=true</c>) IS a mutation and stays fail-closed:
/// it kills a running process, so it keeps requiring interactive approval
/// exactly like the shell command that launched the job.
/// </para>
///
/// <para>
/// Launching a background job is unaffected by this matcher. A background
/// launch is a <c>shell_execute</c> call with <c>_background: true</c>, which
/// routes through <see cref="ShellApprovalMatcher"/> and the ordinary shell
/// approval gate. This matcher only governs the query/cancel follow-up tool.
/// </para>
/// </summary>
public sealed class BackgroundJobApprovalMatcher : IToolApprovalMatcher
{
public static readonly BackgroundJobApprovalMatcher Instance = new();

/// <summary>
/// Argument key carrying the cancellation flag on <c>check_background_job</c>.
/// </summary>
private const string CancelArgumentKey = "Cancel";

public string GetApprovalModeKey(ToolName toolName, IDictionary<string, object?>? arguments)
=> toolName.Value;

/// <summary>
/// Only a cancellation request fails closed. A status query is read-only
/// and job-scoped, so it must not force interactive approval on Personal.
/// A missing or unparsable <c>Cancel</c> value counts as a status query —
/// fail toward the read-only path, never toward an approval prompt for a
/// query. The tool's own argument binding rejects invalid values before
/// execution, so this cannot mask a real cancellation.
/// </summary>
public bool IsFailClosedOnPersonal(ToolName toolName, IDictionary<string, object?>? arguments)
=> IsCancelRequest(arguments);

public IReadOnlyList<string> ExtractPatterns(ToolName toolName, IDictionary<string, object?>? arguments)
=> [toolName.Value];

public IReadOnlyList<string> ExtractCandidateVerbs(ToolName toolName, IDictionary<string, object?>? arguments)
=> [toolName.Value];

public IReadOnlyList<ApprovalCandidate> ExtractCandidates(ToolName toolName, IDictionary<string, object?>? arguments)
=> [new ApprovalCandidate(toolName.Value, Directory: null)];

public bool IsApproved(
ToolName toolName,
IDictionary<string, object?>? arguments,
IReadOnlyList<ApprovalEntry> approvedEntries,
string? cwd)
=> ApprovalPatternMatching.MatchesAny(toolName.Value, approvedEntries);

public bool IsMessy(ToolName toolName, IDictionary<string, object?>? arguments)
=> false;

public string FormatForDisplay(ToolName toolName, IDictionary<string, object?>? arguments)
=> toolName.Value;

private static bool IsCancelRequest(IDictionary<string, object?>? arguments)
{
if (arguments is null)
return false;

if (!arguments.TryGetValue(CancelArgumentKey, out var value) || value is null)
return false;

return value switch
{
bool b => b,
JsonElement { ValueKind: JsonValueKind.True } => true,
JsonElement { ValueKind: JsonValueKind.False } => false,
string s => bool.TryParse(s, out var parsed) && parsed,
_ => false
};
}
}
16 changes: 15 additions & 1 deletion src/Netclaw.Actors/Tools/ToolAccessPolicy.cs
Original file line number Diff line number Diff line change
Expand Up @@ -174,9 +174,23 @@ public ToolAccessDecision AuthorizeInvocation(
}
}

return CheckApprovalGate(toolName, context, arguments, ShellApprovalMatcher.Instance);
return CheckApprovalGate(toolName, context, arguments, SelectShellApprovalMatcher(toolName));
}

/// <summary>
/// Selects the approval matcher for shell-coupled tools. Real shell
/// commands use <see cref="ShellApprovalMatcher"/> (verb-chain extraction,
/// unconditional fail-closed on Personal). <c>check_background_job</c> uses
/// <see cref="BackgroundJobApprovalMatcher"/> because its status query is
/// read-only and job-scoped (the job manager enforces an exact
/// SessionId+Audience+Boundary match), so only its Cancel=true mutation
/// must fail closed on Personal.
/// </summary>
private static IToolApprovalMatcher SelectShellApprovalMatcher(ToolName toolName)
=> string.Equals(toolName.Value, CheckBackgroundJobTool.ToolName, StringComparison.Ordinal)
? BackgroundJobApprovalMatcher.Instance
: ShellApprovalMatcher.Instance;

/// <summary>
/// For non-interactive channels, validates that the working directory and all
/// path-like arguments in a shell command are write-authorized for the channel's
Expand Down
Loading