diff --git a/openspec/changes/simplify-shell-policy-evaluator/evidence/refactor-reduction-revision.md b/openspec/changes/simplify-shell-policy-evaluator/evidence/refactor-reduction-revision.md index 18667465f..8d47a3371 100644 --- a/openspec/changes/simplify-shell-policy-evaluator/evidence/refactor-reduction-revision.md +++ b/openspec/changes/simplify-shell-policy-evaluator/evidence/refactor-reduction-revision.md @@ -70,6 +70,16 @@ The slice removes another 57 production lines and four control-flow lines. It ad The cumulative footprint uses 8,463 lines and 517 control-flow lines. The complete reduction gate remains open. +## Consolidated path and actor snapshot slice + +This slice removes candidate identity and causal scope copies from path facts. Each intent and fallback view now owns its resolution base, while the candidate remains the sole owner of its ID and parser occurrence. + +The actor adapter also reuses one empty-result constructor. Validation snapshots the candidate and near-miss collections without cloning their sealed immutable elements. + +The slice removes another 35 production lines without changing the control-flow count. It removes one test line while preserving the mutable-list, malformed-evidence, path-base, and causal-fallback regressions. + +The cumulative footprint uses 8,428 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. diff --git a/src/Netclaw.Actors.Tests/Tools/ShellPolicyEvaluationTests.cs b/src/Netclaw.Actors.Tests/Tools/ShellPolicyEvaluationTests.cs index a39f98155..5c9274c10 100644 --- a/src/Netclaw.Actors.Tests/Tools/ShellPolicyEvaluationTests.cs +++ b/src/Netclaw.Actors.Tests/Tools/ShellPolicyEvaluationTests.cs @@ -152,7 +152,7 @@ public void Causal_protected_path_check_denies_an_invalid_known_value() var invalidFacts = facts with { Intent = new ShellPolicyResolvedPathView( - Assert.IsType(facts.IntentScope), + Assert.IsType(facts.Intent).ResolutionBase, [invalid]) }; @@ -194,9 +194,11 @@ public void Causal_protected_path_check_does_not_treat_unknown_as_a_denied_path( var unknownFacts = facts with { Intent = new ShellPolicyResolvedPathView( - Assert.IsType(facts.IntentScope), + Assert.IsType(facts.Intent).ResolutionBase, [unknown]), - Fallbacks = [] + Fallbacks = facts.Fallbacks + .Select(static view => view with { Facts = [] }) + .ToArray() }; Assert.False(policy.CausalIntentReferencesProtectedPath(unknownFacts)); @@ -507,7 +509,7 @@ public async Task Reviewed_safe_real_scope_stage_uses_projected_path_facts( + $"directory={candidate.Candidate.Directory}; " + $"sourceCwd={candidate.SourceOccurrence?.WorkingDirectory}; " + $"real={facts.RealScope}; " - + $"facts=[{string.Join(", ", facts.Real?.Facts ?? [])}]"; + + $"facts=[{string.Join(", ", facts.Real.Facts)}]"; }))); } @@ -520,14 +522,13 @@ public void Unproved_non_file_semantics_keep_reviewed_safe_policy_strict() "tr"); var candidate = Assert.Single(evaluation.Candidates); var facts = evaluation.Projection.PathFacts[candidate.Id.Value]; - var real = Assert.IsType(facts.Real); var invalid = facts with { - Real = real with { HasUnprovedNonFileSystemSemantics = true } + Real = facts.Real with { HasUnprovedNonFileSystemSemantics = true } }; Assert.False(policy.IsReviewedSafeCandidate( - candidate.Candidate, + candidate, invalid, context.Invocation)); } @@ -725,7 +726,7 @@ public void Causal_uncovered_context_retains_the_complete_approval_context() } [Fact] - public void Path_facts_preserve_candidate_and_real_scope_identity() + public void Path_facts_preserve_real_scope_and_source_resolution() { var (evaluation, _, _) = CreateReviewedSafeEvaluation( "head README.md", @@ -735,10 +736,8 @@ public void Path_facts_preserve_candidate_and_real_scope_identity() var facts = evaluation.Projection.PathFacts[candidate.Id.Value]; - Assert.Same(candidate.SourceOccurrence, facts.SourceOccurrence); Assert.Equal(ShellPolicyPathResolutionState.Known, facts.RealScope.State); Assert.Equal("/work", facts.RealScope.Path?.Value); - Assert.NotNull(facts.Real); Assert.Contains( facts.Real.Facts, fact => fact.Source.Origin == ShellPolicyPathOrigin.EffectiveArgument @@ -808,9 +807,9 @@ public void Path_facts_keep_candidate_scope_separate_from_the_command_base() var facts = evaluation.Projection.PathFacts[candidate.Id.Value]; Assert.Equal("/work/sub", facts.RealScope.Path?.Value); - Assert.Equal("/work", facts.Real?.ResolutionBase.Path?.Value); + Assert.Equal("/work", facts.Real.ResolutionBase.Path?.Value); Assert.Contains( - Assert.IsType(facts.Real).Facts, + facts.Real.Facts, fact => fact.Source.Origin == ShellPolicyPathOrigin.EffectiveArgument && fact.Paths.Any(path => path.Value == "/work/sub/file.txt")); } @@ -827,8 +826,8 @@ public void Causal_path_facts_keep_intent_and_fallback_resolutions_distinct() var facts = evaluation.Projection.PathFacts[candidate.Id.Value]; - Assert.Equal("/tmp", facts.IntentScope?.Path?.Value); - Assert.Contains(facts.FallbackScopes, scope => scope.Path?.Value == "/work"); + Assert.Equal("/tmp", facts.Intent?.ResolutionBase.Path?.Value); + Assert.Contains(facts.Fallbacks, view => view.ResolutionBase.Path?.Value == "/work"); Assert.Contains( Assert.IsType(facts.Intent).Facts, fact => fact.Source.Origin == ShellPolicyPathOrigin.EffectiveArgument diff --git a/src/Netclaw.Actors/Tools/ScopedShellSafeVerbPolicy.cs b/src/Netclaw.Actors/Tools/ScopedShellSafeVerbPolicy.cs index 049c931ed..bb2263983 100644 --- a/src/Netclaw.Actors/Tools/ScopedShellSafeVerbPolicy.cs +++ b/src/Netclaw.Actors/Tools/ScopedShellSafeVerbPolicy.cs @@ -150,17 +150,18 @@ private bool IsReviewedDiagnostic( } internal bool ShortCircuitsCausalIntent( - ApprovalCandidate candidate, + ShellPolicyCandidate projected, ShellPolicyCandidatePathFacts pathFacts, ToolInvocationContext context) { + var candidate = projected.Candidate; if (context.Audience != TrustAudience.Personal || candidate is not { Shell: ApprovalShell.Bash, VerbTokens: { } } - || pathFacts.IntentScope is not + || pathFacts.Intent?.ResolutionBase is not { State: ShellPolicyPathResolutionState.Known, Path: { } intentPath @@ -171,7 +172,7 @@ internal bool ShortCircuitsCausalIntent( ShellPathStyle.Posix) || !IsReviewedDiagnostic( candidate, - pathFacts.SourceOccurrence, + projected.SourceOccurrence, [intentPath.Value], pathFacts.Intent)) { @@ -196,19 +197,20 @@ internal bool ShortCircuits( ? ShellPathStyle.Windows : ShellPathStyle.Posix; var facts = ShellPolicyPathFacts.Create([projected], pathStyle); - return ShortCircuits(projected.Candidate, facts[projected.Id.Value], context); + return ShortCircuits(projected, facts[projected.Id.Value], context); } internal bool ShortCircuits( - ApprovalCandidate candidate, + ShellPolicyCandidate projected, ShellPolicyCandidatePathFacts pathFacts, ToolInvocationContext context) { + var candidate = projected.Candidate; var safeRoots = ResolveDeclaredSafeSpaceRoots(context); if (safeRoots.Count == 0 || !IsReviewedDiagnostic( candidate, - pathFacts.SourceOccurrence, + projected.SourceOccurrence, safeRoots, pathFacts.Real) || pathFacts.RealScope is not diff --git a/src/Netclaw.Actors/Tools/ShellApprovalEvidence.cs b/src/Netclaw.Actors/Tools/ShellApprovalEvidence.cs index 52f0e213b..ca27c526b 100644 --- a/src/Netclaw.Actors/Tools/ShellApprovalEvidence.cs +++ b/src/Netclaw.Actors/Tools/ShellApprovalEvidence.cs @@ -21,7 +21,9 @@ internal async Task MatchAsync( ArgumentNullException.ThrowIfNull(request); if (request.Candidates.Count == 0 || approvalService is null) - return CreateEmptyResult(request.Candidates); + return CreateEmptyResult( + request.Candidates, + new PersistentGrantStoreStatus.Ready()); if (approvalService is IShellApprovalMatchService shellApprovalService) { @@ -50,15 +52,7 @@ private static ShellApprovalMatchResult ConvertCompatibilityResult( var aggregateStoreStatus = result.PersistentStoreFailure is { } aggregateFailure ? (PersistentGrantStoreStatus)new PersistentGrantStoreStatus.Unavailable(aggregateFailure) : new PersistentGrantStoreStatus.Ready(); - return new ShellApprovalMatchResult( - aggregateStoreStatus, - Array.AsReadOnly(candidates - .Select(static candidate => new ShellGrantCandidateMatch( - candidate.CandidateId, - Match: null, - GrantCoverage: null, - NearMisses: [])) - .ToArray())); + return CreateEmptyResult(candidates, aggregateStoreStatus); } if (checks.Count != candidates.Count) @@ -117,9 +111,10 @@ private static ShellApprovalMatchResult ConvertCompatibilityResult( } private static ShellApprovalMatchResult CreateEmptyResult( - IReadOnlyList candidates) + IReadOnlyList candidates, + PersistentGrantStoreStatus storeStatus) => new( - new PersistentGrantStoreStatus.Ready(), + storeStatus, Array.AsReadOnly(candidates .Select(static candidate => new ShellGrantCandidateMatch( candidate.CandidateId, @@ -142,9 +137,6 @@ second.VerbTokens is not null && internal sealed class ValidatedShellGrantEvidence { - private readonly IReadOnlyList _candidateEvidence; - private readonly IReadOnlyList _approvalMatches; - private ValidatedShellGrantEvidence( PersistentGrantStoreStatus persistentStore, IReadOnlyList sourceCandidates, @@ -153,17 +145,17 @@ private ValidatedShellGrantEvidence( { PersistentStore = persistentStore; SourceCandidates = sourceCandidates; - _candidateEvidence = Array.AsReadOnly(candidateEvidence); - _approvalMatches = Array.AsReadOnly(approvalMatches); + CandidateEvidence = Array.AsReadOnly(candidateEvidence); + ApprovalMatches = Array.AsReadOnly(approvalMatches); } internal PersistentGrantStoreStatus PersistentStore { get; } internal IReadOnlyList SourceCandidates { get; } - internal IReadOnlyList CandidateEvidence => _candidateEvidence; + internal IReadOnlyList CandidateEvidence { get; } - internal IReadOnlyList ApprovalMatches => _approvalMatches; + internal IReadOnlyList ApprovalMatches { get; } internal static bool TryCreate( ShellApprovalMatchResult result, @@ -185,7 +177,6 @@ internal static bool TryCreate( return false; var expectedById = candidates.ToDictionary(static candidate => candidate.Id); - var seenIds = new HashSet(); var validated = new ValidatedShellGrantCandidateEvidence[candidates.Count]; var approvalMatches = new List(candidates.Count); for (var index = 0; index < candidateMatches.Length; index++) @@ -193,8 +184,7 @@ internal static bool TryCreate( var candidateMatch = candidateMatches[index]; if (!TryCopyCandidateEvidence(candidateMatch, out var candidateEvidence) || candidateEvidence is null - || !expectedById.TryGetValue(candidateEvidence.CandidateId, out var candidate) - || !seenIds.Add(candidateEvidence.CandidateId) + || !expectedById.Remove(candidateEvidence.CandidateId, out var candidate) || !TryValidateCandidateEvidence( candidate, candidateEvidence, @@ -287,23 +277,7 @@ private static bool TryCopyCandidateEvidence( if (nearMisses.Any(static nearMiss => nearMiss is null)) return false; - snapshot = new ShellGrantCandidateMatch( - source.CandidateId, - source.Match is null - ? null - : new ToolApprovalMatch( - source.Match.Pattern, - source.Match.Source, - source.Match.Scope), - source.GrantCoverage, - Array.AsReadOnly(nearMisses - .Select(static nearMiss => new ShellApprovalNearMiss( - nearMiss.Grant, - nearMiss.Reason)) - .ToArray())) - { - GrantCreatedAt = source.GrantCreatedAt - }; + snapshot = source with { NearMisses = Array.AsReadOnly(nearMisses) }; return true; } diff --git a/src/Netclaw.Actors/Tools/ShellPolicyCoordinator.cs b/src/Netclaw.Actors/Tools/ShellPolicyCoordinator.cs index 3d6256a66..e82216d02 100644 --- a/src/Netclaw.Actors/Tools/ShellPolicyCoordinator.cs +++ b/src/Netclaw.Actors/Tools/ShellPolicyCoordinator.cs @@ -447,7 +447,7 @@ internal static ShellPolicyStageResult RealScope( && !evaluation.IsCovered(candidate.Id))) { if (!policy.IsReviewedSafeCandidate( - candidate.Candidate, + candidate, evaluation.Projection.PathFacts[candidate.Id.Value], invocation)) { @@ -484,7 +484,7 @@ internal static ShellPolicyStageResult IntentScope( || candidate.IntentPrerequisites.Any(prerequisite => !evaluation.IsCovered(prerequisite)) || !policy.IsReviewedSafeIntentCandidate( - candidate.Candidate, + candidate, evaluation.Projection.PathFacts[candidate.Id.Value], invocation)) { diff --git a/src/Netclaw.Actors/Tools/ShellPolicyPathFacts.cs b/src/Netclaw.Actors/Tools/ShellPolicyPathFacts.cs index dc4aa18cf..9533649d1 100644 --- a/src/Netclaw.Actors/Tools/ShellPolicyPathFacts.cs +++ b/src/Netclaw.Actors/Tools/ShellPolicyPathFacts.cs @@ -52,12 +52,8 @@ internal sealed record ShellPolicyResolvedPathView( } internal sealed record ShellPolicyCandidatePathFacts( - ShellPolicyCandidateId CandidateId, - CommandOccurrence? SourceOccurrence, ShellPolicyScopePathFact RealScope, - ShellPolicyScopePathFact? IntentScope, - IReadOnlyList FallbackScopes, - ShellPolicyResolvedPathView? Real, + ShellPolicyResolvedPathView Real, ShellPolicyResolvedPathView? Intent, IReadOnlyList Fallbacks); @@ -80,7 +76,7 @@ internal static IReadOnlyList Create( var sourceFacts = candidate.SourceOccurrence is { } occurrence ? GetOrCreateSourceFacts(sourceCache, occurrence) - : null; + : ShellPolicyOccurrencePathFacts.Empty; var occurrenceDirectory = candidate.SourceOccurrence?.WorkingDirectory is ShellValueDomain.Exact exact ? exact.Value @@ -96,35 +92,25 @@ is ShellValueDomain.Exact exact : ResolveScope( candidate.IntentDirectory, pathStyle); - var fallbackScopes = candidate.IntentFallbackDirectories - .Select(path => ResolveScope(path, pathStyle)) - .ToArray(); - - var real = sourceFacts?.Resolve( + var real = sourceFacts.Resolve( realBase, pathStyle, candidate.Candidate.Shell); - var intent = sourceFacts is not null && intentBase is { } intentScope + var intent = intentBase is { } intentScope ? sourceFacts.Resolve( intentScope, pathStyle, candidate.Candidate.Shell) : null; - var fallbacks = sourceFacts is null - ? [] - : fallbackScopes - .Select(path => sourceFacts.Resolve( - path, - pathStyle, - candidate.Candidate.Shell)) - .ToArray(); + var fallbacks = candidate.IntentFallbackDirectories + .Select(path => sourceFacts.Resolve( + ResolveScope(path, pathStyle), + pathStyle, + candidate.Candidate.Shell)) + .ToArray(); projected[index] = new ShellPolicyCandidatePathFacts( - candidate.Id, - candidate.SourceOccurrence, realScope, - intentBase, - Array.AsReadOnly(fallbackScopes), real, intent, Array.AsReadOnly(fallbacks)); @@ -165,6 +151,8 @@ private static ShellPolicyOccurrencePathFacts GetOrCreateSourceFacts( internal sealed class ShellPolicyOccurrencePathFacts { + internal static ShellPolicyOccurrencePathFacts Empty { get; } = new([], false, false); + private readonly IReadOnlyList _facts; private readonly bool _hasUnprovedNonFileSystemSemantics; private readonly bool _hasUnprovedBashGlobSemantics; diff --git a/src/Netclaw.Actors/Tools/ToolAccessPolicy.cs b/src/Netclaw.Actors/Tools/ToolAccessPolicy.cs index 775d46bf0..ed224291d 100644 --- a/src/Netclaw.Actors/Tools/ToolAccessPolicy.cs +++ b/src/Netclaw.Actors/Tools/ToolAccessPolicy.cs @@ -344,7 +344,7 @@ internal void MarkSessionScratchRetry( } internal bool IsReviewedSafeCandidate( - ApprovalCandidate candidate, + ShellPolicyCandidate candidate, ShellPolicyCandidatePathFacts pathFacts, ToolInvocationContext context) => _safeVerbPolicy is not null @@ -354,7 +354,7 @@ internal bool IsReviewedSafeCandidate( context); internal bool IsReviewedSafeIntentCandidate( - ApprovalCandidate candidate, + ShellPolicyCandidate candidate, ShellPolicyCandidatePathFacts pathFacts, ToolInvocationContext context) => _safeVerbPolicy is not null @@ -367,15 +367,16 @@ internal bool CausalIntentReferencesProtectedPath( ShellPolicyCandidatePathFacts facts) { ArgumentNullException.ThrowIfNull(facts); - if (facts.IntentScope is not { } intent + if (facts.Intent?.ResolutionBase is not { } intent || string.IsNullOrWhiteSpace(intent.AuthoredValue) - || facts.FallbackScopes.Count == 0) + || facts.Fallbacks.Count == 0) { return true; } if (ScopeReferencesProtectedPath(intent) - || facts.FallbackScopes.Any(ScopeReferencesProtectedPath)) + || facts.Fallbacks.Any(fallback => + ScopeReferencesProtectedPath(fallback.ResolutionBase))) { return true; }