Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,16 @@ The slice removes another 45 production lines and eight test lines. It does not

The cumulative footprint uses 8,520 lines and 521 control-flow lines. The complete reduction gate remains open.

## Unified reviewed-safe path slice

This slice removes the unused aggregate reviewed-safe route. The compatibility entry now projects candidate-scoped path facts and calls the same reviewed-safe method as the coordinator.

The evaluation also reuses the projection's immutable candidate snapshot, and the path-fact projection returns its immutable candidate list directly instead of wrapping a second indexed container.

The slice removes another 57 production lines and four control-flow lines. It adds 19 test lines while moving the existing direct safe-policy cases onto the shared typed route.

The cumulative footprint uses 8,463 lines and 517 control-flow lines. The complete reduction gate remains open.

## Preliminary coverage and risk

The audit used `dotnet-coverage` 18.10.0 and `crap4dotnet` 0.1.1.
Expand Down
51 changes: 35 additions & 16 deletions src/Netclaw.Actors.Tests/Tools/ScopedShellSafeVerbPolicyTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,19 @@ private static bool ShortCircuits(
string verb,
string? cwd,
ToolInvocationContext context) =>
policy.AllShortCircuit([Candidate(verb)], cwd, context);
AllShortCircuit(policy, [Candidate(verb)], cwd, context);

private static bool AllShortCircuit(
ScopedShellSafeVerbPolicy policy,
IReadOnlyList<ApprovalCandidate> candidates,
string? cwd,
ToolInvocationContext context)
{
if (candidates.Count == 0)
return false;

return candidates.All(candidate => policy.ShortCircuits(candidate, cwd, context));
}

private ToolInvocationContext PersonalContext(string? projectDir = null, string? sessionDir = null)
=> TestToolExecutionContext.CreateBound("session-1", sessionDir ?? _sessionDir, new TestToolExecutionContextOptions
Expand Down Expand Up @@ -191,7 +203,7 @@ public void All_short_circuit_returns_false_when_any_verb_is_unsafe()
var policy = new ScopedShellSafeVerbPolicy(VerbList("grep", "cat"));
var ctx = PersonalContext(projectDir: _projectDir);

Assert.False(policy.AllShortCircuit(Candidates("grep", "git push"), _projectDir, ctx));
Assert.False(AllShortCircuit(policy, Candidates("grep", "git push"), _projectDir, ctx));
}

[Fact]
Expand All @@ -200,7 +212,7 @@ public void All_short_circuit_returns_true_when_every_verb_is_safe_and_in_space(
var policy = new ScopedShellSafeVerbPolicy(VerbList("grep", "cat", "wc"));
var ctx = PersonalContext(projectDir: _projectDir);

Assert.True(policy.AllShortCircuit(Candidates("grep", "cat", "wc"), _projectDir, ctx));
Assert.True(AllShortCircuit(policy, Candidates("grep", "cat", "wc"), _projectDir, ctx));
}

[Fact]
Expand All @@ -209,7 +221,7 @@ public void Empty_candidate_list_does_not_short_circuit()
var policy = new ScopedShellSafeVerbPolicy(VerbList("grep"));
var ctx = PersonalContext(projectDir: _projectDir);

Assert.False(policy.AllShortCircuit([], _projectDir, ctx));
Assert.False(AllShortCircuit(policy, [], _projectDir, ctx));
}

[Fact]
Expand Down Expand Up @@ -244,7 +256,11 @@ public void New_safe_verb_chained_with_mutating_verb_still_prompts()
var policy = new ScopedShellSafeVerbPolicy(VerbList("whoami"));
var ctx = PersonalContext(projectDir: _projectDir);

Assert.False(policy.AllShortCircuit(Candidates("whoami", "git push origin main"), _projectDir, ctx));
Assert.False(AllShortCircuit(
policy,
Candidates("whoami", "git push origin main"),
_projectDir,
ctx));
}

[Fact]
Expand All @@ -253,7 +269,8 @@ public void Reviewed_phrase_matches_a_longer_canonical_token_chain()
var policy = new ScopedShellSafeVerbPolicy(VerbList("git ls-tree"));
var ctx = PersonalContext(projectDir: _projectDir);

Assert.True(policy.AllShortCircuit(
Assert.True(AllShortCircuit(
policy,
[Candidate("git ls-tree feature", _projectDir)],
_projectDir,
ctx));
Expand All @@ -279,7 +296,7 @@ public void Argument_before_reviewed_phrase_stays_strict(string commandTemplate)
["WorkingDirectory"] = _projectDir
});

Assert.False(policy.AllShortCircuit(candidates, _projectDir, ctx));
Assert.False(AllShortCircuit(policy, candidates, _projectDir, ctx));
}

[Theory]
Expand All @@ -302,7 +319,7 @@ public void Path_shaped_option_operand_outside_safe_root_stays_strict(
["WorkingDirectory"] = _projectDir
});

Assert.False(policy.AllShortCircuit(candidates, _projectDir, ctx));
Assert.False(AllShortCircuit(policy, candidates, _projectDir, ctx));
}

[Fact]
Expand All @@ -320,7 +337,7 @@ public void Path_shaped_option_operand_under_safe_root_remains_eligible()
["WorkingDirectory"] = _projectDir
});

Assert.True(policy.AllShortCircuit(candidates, _projectDir, ctx));
Assert.True(AllShortCircuit(policy, candidates, _projectDir, ctx));
}

[Fact]
Expand All @@ -337,7 +354,7 @@ public void Path_shaped_data_under_safe_root_does_not_create_new_authority()
["WorkingDirectory"] = _projectDir
});

Assert.True(policy.AllShortCircuit(candidates, _projectDir, ctx));
Assert.True(AllShortCircuit(policy, candidates, _projectDir, ctx));
}

[Fact]
Expand All @@ -361,7 +378,7 @@ public void PowerShell_compatibility_paths_use_posix_host_roots()
["WorkingDirectory"] = _projectDir
});

Assert.True(policy.AllShortCircuit(candidates, _projectDir, ctx));
Assert.True(AllShortCircuit(policy, candidates, _projectDir, ctx));
}

[Fact]
Expand All @@ -370,7 +387,8 @@ public void Prefix_collision_does_not_match_reviewed_phrase()
var policy = new ScopedShellSafeVerbPolicy(VerbList("git ls-tree"));
var ctx = PersonalContext(projectDir: _projectDir);

Assert.False(policy.AllShortCircuit(
Assert.False(AllShortCircuit(
policy,
[Candidate("git ls-treex feature", _projectDir)],
_projectDir,
ctx));
Expand All @@ -383,7 +401,7 @@ public void Candidate_without_canonical_tokens_stays_strict()
var ctx = PersonalContext(projectDir: _projectDir);
var candidate = new ApprovalCandidate("head", _projectDir);

Assert.False(policy.AllShortCircuit([candidate], _projectDir, ctx));
Assert.False(AllShortCircuit(policy, [candidate], _projectDir, ctx));
}

[Fact]
Expand All @@ -392,7 +410,8 @@ public void Candidate_from_another_shell_stays_strict()
var policy = new ScopedShellSafeVerbPolicy(VerbList("Get-Content"));
var ctx = PersonalContext(projectDir: _projectDir);

Assert.False(policy.AllShortCircuit(
Assert.False(AllShortCircuit(
policy,
[Candidate("Get-Content", _projectDir, ApprovalShell.PowerShell)],
_projectDir,
ctx));
Expand All @@ -413,7 +432,7 @@ public void Dotted_symlink_directory_does_not_short_circuit()
var ctx = PersonalContext(projectDir: _projectDir);
var candidate = Candidate("find", link);

Assert.False(policy.AllShortCircuit([candidate], _projectDir, ctx));
Assert.False(AllShortCircuit(policy, [candidate], _projectDir, ctx));
}
finally
{
Expand All @@ -428,7 +447,7 @@ public void Candidate_path_outside_safe_spaces_falls_through_to_prompt()
var ctx = PersonalContext(projectDir: _projectDir);
var candidates = new[] { Candidate("cat", _outsideDir) };

Assert.False(policy.AllShortCircuit(candidates, _projectDir, ctx));
Assert.False(AllShortCircuit(policy, candidates, _projectDir, ctx));
}

[Fact]
Expand Down
14 changes: 7 additions & 7 deletions src/Netclaw.Actors.Tests/Tools/ShellPolicyEvaluationTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ public void Causal_protected_path_check_denies_an_invalid_known_value()
var consumer = Assert.Single(
evaluation.Candidates,
static candidate => candidate.Role == ShellPolicyCandidateRole.CausalIntentConsumer);
var facts = evaluation.Projection.PathFacts.For(consumer.Id);
var facts = evaluation.Projection.PathFacts[consumer.Id.Value];
var source = Assert.Single(
Assert.IsType<ShellPolicyResolvedPathView>(facts.Intent).Facts,
static fact => fact.Source.Origin == ShellPolicyPathOrigin.EffectiveArgument).Source;
Expand All @@ -167,7 +167,7 @@ public void Causal_protected_path_check_does_not_treat_unknown_as_a_denied_path(
var consumer = Assert.Single(
evaluation.Candidates,
static candidate => candidate.Role == ShellPolicyCandidateRole.CausalIntentConsumer);
var facts = evaluation.Projection.PathFacts.For(consumer.Id);
var facts = evaluation.Projection.PathFacts[consumer.Id.Value];
var environment = ShellExecutionEnvironment.CreatePowerShell(
@"C:\Program Files\PowerShell\7\pwsh.exe",
PwshDialect.PowerShell7);
Expand Down Expand Up @@ -502,7 +502,7 @@ public async Task Reviewed_safe_real_scope_stage_uses_projected_path_facts(
"; ",
evaluation.Candidates.Select(candidate =>
{
var facts = evaluation.Projection.PathFacts.For(candidate.Id);
var facts = evaluation.Projection.PathFacts[candidate.Id.Value];
return $"{candidate.Candidate.Verb}: "
+ $"directory={candidate.Candidate.Directory}; "
+ $"sourceCwd={candidate.SourceOccurrence?.WorkingDirectory}; "
Expand All @@ -519,7 +519,7 @@ public void Unproved_non_file_semantics_keep_reviewed_safe_policy_strict()
interactive: true,
"tr");
var candidate = Assert.Single(evaluation.Candidates);
var facts = evaluation.Projection.PathFacts.For(candidate.Id);
var facts = evaluation.Projection.PathFacts[candidate.Id.Value];
var real = Assert.IsType<ShellPolicyResolvedPathView>(facts.Real);
var invalid = facts with
{
Expand Down Expand Up @@ -733,7 +733,7 @@ public void Path_facts_preserve_candidate_and_real_scope_identity()
"head");
var candidate = Assert.Single(evaluation.Candidates);

var facts = evaluation.Projection.PathFacts.For(candidate.Id);
var facts = evaluation.Projection.PathFacts[candidate.Id.Value];

Assert.Same(candidate.SourceOccurrence, facts.SourceOccurrence);
Assert.Equal(ShellPolicyPathResolutionState.Known, facts.RealScope.State);
Expand Down Expand Up @@ -805,7 +805,7 @@ public void Path_facts_keep_candidate_scope_separate_from_the_command_base()
evaluation.Candidates,
static candidate => candidate.Candidate.Directory == "/work/sub");

var facts = evaluation.Projection.PathFacts.For(candidate.Id);
var facts = evaluation.Projection.PathFacts[candidate.Id.Value];

Assert.Equal("/work/sub", facts.RealScope.Path?.Value);
Assert.Equal("/work", facts.Real?.ResolutionBase.Path?.Value);
Expand All @@ -825,7 +825,7 @@ public void Causal_path_facts_keep_intent_and_fallback_resolutions_distinct()
evaluation.Candidates,
static candidate => candidate.Role == ShellPolicyCandidateRole.CausalIntentConsumer);

var facts = evaluation.Projection.PathFacts.For(candidate.Id);
var facts = evaluation.Projection.PathFacts[candidate.Id.Value];

Assert.Equal("/tmp", facts.IntentScope?.Path?.Value);
Assert.Contains(facts.FallbackScopes, scope => scope.Path?.Value == "/work");
Expand Down
52 changes: 9 additions & 43 deletions src/Netclaw.Actors/Tools/ScopedShellSafeVerbPolicy.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,25 +35,6 @@ public ScopedShellSafeVerbPolicy(SafeVerbList safeVerbs)
_safeVerbs = safeVerbs;
}

/// <summary>
/// Returns true when each candidate has a safe verb and a safe effective
/// directory. The candidate directory takes precedence over the cwd.
/// </summary>
public bool AllShortCircuit(
IReadOnlyList<ApprovalCandidate> candidates,
string? cwd,
ToolInvocationContext context)
{
if (candidates.Count == 0)
return false;

return candidates.All(candidate => ShortCircuits(
candidate,
candidate.SourceOccurrence,
cwd,
context));
}

/// <summary>
/// Returns true when declaring <paramref name="cwd"/> as the project root
/// would make every candidate eligible for the reviewed-safe short circuit.
Expand Down Expand Up @@ -204,33 +185,18 @@ internal bool ShortCircuitsCausalIntent(

internal bool ShortCircuits(
ApprovalCandidate candidate,
CommandOccurrence? sourceOccurrence,
string? cwd,
ToolInvocationContext context)
{
var safeRoots = ResolveSafeSpaceRoots(context);
var resolvedPaths = ResolveCompatibilityPaths(candidate, sourceOccurrence);
if (safeRoots.Count == 0
|| !IsReviewedDiagnostic(candidate, sourceOccurrence, safeRoots, resolvedPaths))
{
return false;
}

var effectiveDirectory = candidate.Directory ?? cwd;
if (string.IsNullOrWhiteSpace(effectiveDirectory))
return false;

try
{
var fullDirectory = Path.GetFullPath(effectiveDirectory);
return safeRoots.Any(root => IsSafePath(fullDirectory, root));
}
catch (Exception ex) when (ex is ArgumentException
or NotSupportedException
or PathTooLongException)
{
return false;
}
var projected = new ShellPolicyCandidate(
new ShellPolicyCandidateId(0),
candidate.Directory is null ? candidate with { Directory = cwd } : candidate,
candidate.SourceOccurrence);
var pathStyle = OperatingSystem.IsWindows()
? ShellPathStyle.Windows
: ShellPathStyle.Posix;
var facts = ShellPolicyPathFacts.Create([projected], pathStyle);
return ShortCircuits(projected.Candidate, facts[projected.Id.Value], context);
}

internal bool ShortCircuits(
Expand Down
6 changes: 3 additions & 3 deletions src/Netclaw.Actors/Tools/ShellPolicyCoordinator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ internal static ShellPolicyStageResult ProtectedCausalPaths(
return evaluation.Candidates.Any(candidate =>
candidate.Role == ShellPolicyCandidateRole.CausalIntentConsumer
&& policy.CausalIntentReferencesProtectedPath(
evaluation.Projection.PathFacts.For(candidate.Id)))
evaluation.Projection.PathFacts[candidate.Id.Value]))
? new ShellPolicyStageResult.Complete(
ToolAuthorizationDecision.Deny("shell_references_protected_path"))
: new ShellPolicyStageResult.Continue();
Expand Down Expand Up @@ -448,7 +448,7 @@ internal static ShellPolicyStageResult RealScope(
{
if (!policy.IsReviewedSafeCandidate(
candidate.Candidate,
evaluation.Projection.PathFacts.For(candidate.Id),
evaluation.Projection.PathFacts[candidate.Id.Value],
invocation))
{
continue;
Expand Down Expand Up @@ -485,7 +485,7 @@ internal static ShellPolicyStageResult IntentScope(
!evaluation.IsCovered(prerequisite))
|| !policy.IsReviewedSafeIntentCandidate(
candidate.Candidate,
evaluation.Projection.PathFacts.For(candidate.Id),
evaluation.Projection.PathFacts[candidate.Id.Value],
invocation))
{
continue;
Expand Down
16 changes: 6 additions & 10 deletions src/Netclaw.Actors/Tools/ShellPolicyEvaluation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,6 @@ internal ShellPolicyAuthorization(

internal sealed class ShellPolicyEvaluation
{
private readonly ShellPolicyCandidate[] _candidates;
private readonly IReadOnlyList<ShellPolicyCandidate> _candidateView;
private readonly ShellPolicyCoverageSource[] _coverage;
private readonly ShellPolicyDecisionTraceBuilder _trace = new();
private ToolAuthorizationDecision? _terminalDecision;
Expand All @@ -162,19 +160,17 @@ internal ShellPolicyEvaluation(ShellPolicyProjection projection)
ArgumentNullException.ThrowIfNull(projection);

Projection = projection;
_candidates = projection.Candidates.ToArray();
_candidateView = Array.AsReadOnly(_candidates);
_coverage = new ShellPolicyCoverageSource[_candidates.Length];
_coverage = new ShellPolicyCoverageSource[projection.Candidates.Count];
}

internal ShellPolicyProjection Projection { get; }

internal IReadOnlyList<ShellPolicyCandidate> Candidates => _candidateView;
internal IReadOnlyList<ShellPolicyCandidate> Candidates => Projection.Candidates;

internal bool AllCovered => !_coverage.Contains(ShellPolicyCoverageSource.Uncovered);

internal IReadOnlyList<ShellPolicyCandidate> UncoveredCandidates =>
Array.AsReadOnly(_candidates
Array.AsReadOnly(Projection.Candidates
.Where((_, index) => _coverage[index] == ShellPolicyCoverageSource.Uncovered)
.ToArray());

Expand Down Expand Up @@ -271,10 +267,10 @@ internal ShellPolicyStageResult Cover(
return new ShellPolicyStageResult.Complete(_terminalDecision);

var index = candidate.Id.Value;
if ((uint)index >= (uint)_candidates.Length)
if ((uint)index >= (uint)Candidates.Count)
return Fail(ShellPolicyFault.InvalidCandidateId);

if (!ReferenceEquals(candidate, _candidates[index]))
if (!ReferenceEquals(candidate, Projection.Candidates[index]))
return Fail(ShellPolicyFault.CandidateFactsChanged);

if (_coverage[index] != ShellPolicyCoverageSource.Uncovered)
Expand Down Expand Up @@ -310,7 +306,7 @@ internal ShellPolicyStageResult Complete(
ToolAuthorizationOutcome.Allowed => AllCovered
|| allowsUncoveredOneTime
&& decision.AllowReason == ToolAllowReason.OneTimeApproval,
ToolAuthorizationOutcome.RequiresApproval => _candidates.Length == 0 || !AllCovered,
ToolAuthorizationOutcome.RequiresApproval => Candidates.Count == 0 || !AllCovered,
ToolAuthorizationOutcome.Denied => true,
_ => false,
};
Expand Down
Loading
Loading