diff --git a/Directory.Packages.props b/Directory.Packages.props index 25a9b1e4c..4c4a3611c 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -75,7 +75,7 @@ - + diff --git a/IMPLEMENTATION_PLAN.md b/IMPLEMENTATION_PLAN.md index 7933362ad..1e803f4a7 100644 --- a/IMPLEMENTATION_PLAN.md +++ b/IMPLEMENTATION_PLAN.md @@ -245,6 +245,12 @@ Done when: and path-shape facts introduced in 0.3.1. This store-v3 slice preserves those parser token facts without executable-private command rules; later parent tasks consume the new value-domain facts in the coordinator. +- [ ] Netclaw consumes public ShellSyntaxTree `0.3.3` for the parser-owned + authored filesystem domain. Local code accepts only `Exact` and `FiniteSet`. + It checks each value through path policy and keeps unsafe transforms strict. + The Release build and all 7,138 runnable tests pass. The suite reports 15 + expected platform or opt-in skips. Adversarial review and CI remain before + completion. - [x] The expanded 247-test matrix covers command-substitution and PowerShell execution-region behavior. Known command-owned regions reuse independently matched host and body grants after Netclaw accounts for the parsed body. diff --git a/openspec/changes/structure-shell-approval-policy/design.md b/openspec/changes/structure-shell-approval-policy/design.md index e6522c896..14448917c 100644 --- a/openspec/changes/structure-shell-approval-policy/design.md +++ b/openspec/changes/structure-shell-approval-policy/design.md @@ -265,7 +265,7 @@ authority outside code review. At minimum `find`, `awk`, `rg`, and `sort` are not eligible. Production code has no flag-specific exceptions. The existing `git ls-tree` special case is deleted. -### 7. Consume ShellSyntaxTree 0.3.1 facts through 0.3.2 explicitly +### 7. Consume ShellSyntaxTree 0.3.1 facts through 0.3.3 explicitly Netclaw uses effective `AnalyzedArgument.Value` for runtime-sensitive checks. It may use `AuthoredValue` for approval matching only after the maintainer @@ -275,13 +275,10 @@ contract decides whether an effective value is path-relevant. Every finite effective value for an `IsPath` argument still passes `ToolPathPolicy`; an unknown path-relevant value stays strict. -ShellSyntaxTree 0.3.2 publishes D14's finite `AuthoredValue`, but its effective -argument has `Argument.IsPath == false`. Netclaw cannot infer file authority -from that contradiction. An authored finite value may enter `ToolPathPolicy` -only after ShellSyntaxTree supplies a separate general parser-owned authored -operand-role fact. Until that additive fact exists and Netclaw adopts it, D14 -remains strict. The change does not invent the public member name or infer the -role from an executable's private grammar. +ShellSyntaxTree 0.3.3 publishes D14's finite `AuthoredFileSystemValue`. Netclaw +accepts only `Exact` and `FiniteSet`. Each value enters `ToolPathPolicy` and the +approval scope check. Unknown and all other alternatives stay strict. Netclaw +does not infer the role from an executable's private grammar. `AuthoredPathShape` is lexical shape only. It may make review stricter, but it never establishes that an executable treats an argument as a filesystem diff --git a/openspec/changes/structure-shell-approval-policy/evidence/approval-matrix.json b/openspec/changes/structure-shell-approval-policy/evidence/approval-matrix.json index 9a5783fab..b42bee63f 100644 --- a/openspec/changes/structure-shell-approval-policy/evidence/approval-matrix.json +++ b/openspec/changes/structure-shell-approval-policy/evidence/approval-matrix.json @@ -132,8 +132,8 @@ "observed": "Once", "classification": "ShellSyntaxTreeFactGap", "owner": "ShellSyntaxTree", - "sstExpectation": "With PublishAuthoredSourceFacts enabled, effective cat value is Unknown; AuthoredValue is the four pre-field-splitting /work words and AuthoredPathShape is Posix.", - "netclawExpectation": "Allow only if product policy accepts pre-field-splitting authored words, applies path checks conservatively, and covers cat plus echo." + "sstExpectation": "With ShellSyntaxTree 0.3.3 and PublishAuthoredSourceFacts enabled, effective cat Value is Unknown; AuthoredValue is the four pre-field-splitting /work words; AuthoredFileSystemValue is the same normalized finite set.", + "netclawExpectation": "Accept only Exact or FiniteSet AuthoredFileSystemValue alternatives, check every path through product policy, and independently cover cat plus echo." }, { "id": "D15", diff --git a/openspec/changes/structure-shell-approval-policy/evidence/netclaw-policy-fixtures.json b/openspec/changes/structure-shell-approval-policy/evidence/netclaw-policy-fixtures.json index 46c3611ab..1566620bf 100644 --- a/openspec/changes/structure-shell-approval-policy/evidence/netclaw-policy-fixtures.json +++ b/openspec/changes/structure-shell-approval-policy/evidence/netclaw-policy-fixtures.json @@ -345,7 +345,7 @@ }, "candidates": [ { "id": 0, "tokens": ["echo"], "realDirectory": "/work", "intentDirectory": null, "expectedCoverage": "ReviewedSafePolicy" }, - { "id": 1, "tokens": ["cat"], "realDirectory": "/work", "intentDirectory": null, "expectedCoverage": "Uncovered" } + { "id": 1, "tokens": ["cat"], "realDirectory": "/work", "intentDirectory": null, "expectedCoverage": "PersistentGlobal" } ], "authoredPathFacts": [ { @@ -358,16 +358,22 @@ "/work/src/Discovery/Discovery.csproj", "/work/tests/Hosting.Tests/Hosting.Tests.csproj" ], + "authoredFileSystemValues": [ + "/work/src/App/App.csproj", + "/work/src/Hosting/Hosting.csproj", + "/work/src/Discovery/Discovery.csproj", + "/work/tests/Hosting.Tests/Hosting.Tests.csproj" + ], "authoredPathShape": "Posix", - "expectedPathPolicy": "RequiresApproval" + "expectedPathPolicy": "Allow" } ], "expectedTrace": [ - { "stage": "CanonicalFacts", "candidateId": 1, "executableBasename": "cat", "outcome": "Uncovered", "reason": "MissingAuthoredOperandRole", "coverage": "Uncovered", "scopeRelation": "None", "grantTimestamp": null }, + { "stage": "StoredGrantMatch", "candidateId": 1, "executableBasename": "cat", "outcome": "Covered", "reason": "GlobalPhraseMatch", "coverage": "PersistentGlobal", "scopeRelation": "Global", "grantTimestamp": null }, { "stage": "ReviewedSafePolicy", "candidateId": 0, "executableBasename": "echo", "outcome": "Covered", "reason": "ReadOnlyUnderRealRoot", "coverage": "ReviewedSafePolicy", "scopeRelation": "UnderRealRoot", "grantTimestamp": null }, - { "stage": "Completion", "candidateId": null, "executableBasename": null, "outcome": "RequiresApproval", "reason": "UnresolvedPathFacts", "coverage": null, "scopeRelation": null, "grantTimestamp": null } + { "stage": "Completion", "candidateId": null, "executableBasename": null, "outcome": "Allow", "reason": "AllCandidatesCovered", "coverage": null, "scopeRelation": null, "grantTimestamp": null } ], - "expectedFinal": { "outcome": "RequiresApproval", "reason": "UnresolvedPathFacts" } + "expectedFinal": { "outcome": "Allow", "reason": "AllCandidatesCovered" } }, { "evidenceId": "D18", diff --git a/openspec/changes/structure-shell-approval-policy/proposal.md b/openspec/changes/structure-shell-approval-policy/proposal.md index 6d82faaa2..188143fb3 100644 --- a/openspec/changes/structure-shell-approval-policy/proposal.md +++ b/openspec/changes/structure-shell-approval-policy/proposal.md @@ -26,10 +26,10 @@ session and persistent grant snapshots. decision-gated v2 migration. - Emit a bounded, redacted decision trace that also supplies near-miss data. - Adopt the authored/effective fact separation introduced in ShellSyntaxTree - 0.3.1 through the corrected public 0.3.2 package. -- Keep D14 strict until ShellSyntaxTree supplies a general parser-owned - authored operand-role fact; lexical path shape alone cannot create file - authority. + 0.3.1 and the authored filesystem fact from public 0.3.3. +- Use the 0.3.3 authored filesystem fact for D14. Accept only exact or finite + values. Check each path through product policy. Lexical path shape alone + cannot create file authority. - Pin the exact sanitized D01-D18 catalog and adversarial cases. No production branch will parse an executable's private options or operands. @@ -55,7 +55,6 @@ No production branch will parse an executable's private options or operands. per-candidate coverage from one atomic snapshot. - UX: covered diagnostic chains stop prompting; unresolved syntax remains one-time-only. -- Dependencies: implementation consumes public ShellSyntaxTree 0.3.2 for the - current authored/effective facts. D14 requires a later additive parser fact - before its authored paths can receive policy coverage. +- Dependencies: implementation consumes public ShellSyntaxTree 0.3.3 for the + authored, effective, and authored filesystem facts. - Documentation/evals: operator guidance and approval behavioral evals change. diff --git a/openspec/changes/structure-shell-approval-policy/specs/tool-approval-gates/spec.md b/openspec/changes/structure-shell-approval-policy/specs/tool-approval-gates/spec.md index 77d9d28b6..914ff56cf 100644 --- a/openspec/changes/structure-shell-approval-policy/specs/tool-approval-gates/spec.md +++ b/openspec/changes/structure-shell-approval-policy/specs/tool-approval-gates/spec.md @@ -602,10 +602,10 @@ second control-flow tokenizer. Supported static loops SHALL expose candidates. Unsupported branches and runtime-generated loops SHALL remain strict. An effective finite argument SHALL enter path policy when the parser-owned -`Argument.IsPath` role is true. An authored finite argument whose effective -role is false SHALL remain strict until ShellSyntaxTree supplies a separate, -general parser-owned authored operand-role fact. `AuthoredPathShape` SHALL NOT -substitute for that fact or create file authority. +`Argument.IsPath` role is true. ShellSyntaxTree 0.3.3 `Exact` and `FiniteSet` +`AuthoredFileSystemValue` facts SHALL also enter path policy. Unknown and all +other alternatives SHALL stay strict. `AuthoredPathShape` SHALL NOT substitute +for the stronger fact or create file authority. A legacy scanner MAY add a denial when canonical analysis is incomplete. It SHALL NOT allow, create candidates, create persistent options, or widen scope. @@ -618,10 +618,10 @@ SHALL NOT allow, create candidates, create persistent options, or widen scope. - **THEN** the authored values do not create file authority - **AND** lexical `AuthoredPathShape` does not cover the candidate -#### Scenario: General authored operand role unlocks finite path checks +#### Scenario: ShellSyntaxTree 0.3.3 unlocks finite D14 path checks -- **GIVEN** a later ShellSyntaxTree version reports D14 finite authored values -- **AND** it supplies a parser-owned authored filesystem-operand role +- **GIVEN** ShellSyntaxTree 0.3.3 reports a finite D14 + `AuthoredFileSystemValue` - **WHEN** the maintainer-approved authored-source policy evaluates it - **THEN** each finite `cat` path passes `ToolPathPolicy` - **AND** the presence of `for` alone does not force a prompt @@ -686,9 +686,9 @@ ShellSyntaxTree analysis. PowerShell SHALL use the selected dialect and `PwshInitialStateMode.Unknown`. Netclaw SHALL use effective values for runtime and deny policy. It MAY use -ShellSyntaxTree 0.3.2 authored values only for the explicitly approved approval -perspective. Unknown policy-relevant values SHALL not create reusable or safe -coverage. +authored values only for the approved approval perspective. It SHALL route +ShellSyntaxTree 0.3.3 authored filesystem values through path policy. Unknown +policy-relevant values SHALL not create reusable or safe coverage. Deny-only defensive scans MAY deny incomplete input but SHALL never authorize it. diff --git a/openspec/changes/structure-shell-approval-policy/tasks.md b/openspec/changes/structure-shell-approval-policy/tasks.md index 0fda9b7b3..35d6928e7 100644 --- a/openspec/changes/structure-shell-approval-policy/tasks.md +++ b/openspec/changes/structure-shell-approval-policy/tasks.md @@ -100,18 +100,17 @@ - [ ] 6.5 Keep native PowerShell causal scope strict and record native Windows expected results. -## 7. ShellSyntaxTree 0.3.1 facts through 0.3.2 +## 7. ShellSyntaxTree 0.3.1 facts through 0.3.3 - [x] 7.1 Upgrade the central package to public 0.3.2, which includes the 0.3.1 authored-source facts and keeps same-language child shells strict. - [x] 7.2 Consume effective `Value` for runtime checks and approved `AuthoredValue` only for the documented approval perspective. - [x] 7.3 Treat `IntegerRange` and `Concatenation` as bounded scalar data only. -- [ ] 7.4 Check every finite effective value whose `Argument.IsPath` is true - through `ToolPathPolicy`. Keep D14 strict until ShellSyntaxTree supplies a - general parser-owned authored operand-role fact; after adoption, check every - finite authored path through the same policy. Treat `AuthoredPathShape` as - lexical-only and keep unknown path values strict. +- [x] 7.4 Check every finite effective value whose `Argument.IsPath` is true + through `ToolPathPolicy`. Check each ShellSyntaxTree 0.3.3 `Exact` or + `FiniteSet` `AuthoredFileSystemValue` through the same policy. Treat + `AuthoredPathShape` as lexical-only. Keep unknown path values strict. - [x] 7.5 Delete the broad Bash environment-variable relaxation and its superseded tests. - [ ] 7.6 Pin exact D02, D10, and D14 input-to-coverage results. diff --git a/src/Netclaw.Security.Tests/ShellApprovalEvidenceContractTests.cs b/src/Netclaw.Security.Tests/ShellApprovalEvidenceContractTests.cs index 5b49e72fb..f7ea73580 100644 --- a/src/Netclaw.Security.Tests/ShellApprovalEvidenceContractTests.cs +++ b/src/Netclaw.Security.Tests/ShellApprovalEvidenceContractTests.cs @@ -7,6 +7,8 @@ using System.Text.Json; using System.Text.Json.Serialization; using System.Text.RegularExpressions; +using Netclaw.Configuration; +using Netclaw.Tools; using ShellSyntaxTree; using Xunit; @@ -18,7 +20,7 @@ public sealed partial class ShellApprovalEvidenceContractTests private const string PolicyFixturesFile = "netclaw-policy-fixtures.json"; private const string PostMergeHarvestFile = "post-1890-approval-harvest.json"; private const string ApprovalMatrixSha256 = - "d2a6e64421af337d1c54f1f955934057398176b456e585bfce015ef7ffa24e7d"; + "0169105efe87b345d9a82d777ef86909e31fa81a5255cc0cc30f32fbe4d0d6b0"; [Fact] public void Approval_matrix_matches_the_locked_cross_repository_artifact() @@ -139,13 +141,16 @@ public void Exact_symbolic_parser_facts_match_the_policy_fixtures() Assert.Equal("Unknown", fact.EffectiveValue); var authored = Assert.IsType(argument.AuthoredValue); Assert.Equal(fact.AuthoredValues, authored.Values); + var authoredFileSystem = Assert.IsType( + argument.AuthoredFileSystemValue); + Assert.Equal(fact.AuthoredFileSystemValues, authoredFileSystem.Values); Assert.Equal(fact.AuthoredPathShape, argument.AuthoredPathShape.ToString()); } } } [Fact] - public void Authored_path_fixture_keeps_missing_operand_role_strict() + public void Authored_path_fixture_uses_the_strong_filesystem_domain() { var fixtures = DeserializeFixtures(File.ReadAllBytes(EvidencePath(PolicyFixturesFile))); var fixture = Assert.Single(fixtures.Cases, item => item.AuthoredPathFacts is { Count: > 0 }); @@ -159,8 +164,45 @@ public void Authored_path_fixture_keeps_missing_operand_role_strict() Assert.False(fact.ArgumentIsPath); Assert.Equal(fact.ArgumentIsPath, argument.Argument.IsPath); - Assert.Equal("RequiresApproval", fact.ExpectedPathPolicy); - Assert.Equal("RequiresApproval", fixture.ExpectedFinal.Outcome); + var authoredFileSystem = Assert.IsType( + argument.AuthoredFileSystemValue); + Assert.Equal(fact.AuthoredFileSystemValues, authoredFileSystem.Values); + Assert.Equal("Allow", fact.ExpectedPathPolicy); + Assert.Equal("Allow", fixture.ExpectedFinal.Outcome); + } + + [Fact] + public void D14_fixture_is_covered_by_its_typed_grant_and_safe_paths() + { + var fixtures = DeserializeFixtures(File.ReadAllBytes(EvidencePath(PolicyFixturesFile))); + var fixture = Assert.Single(fixtures.Cases, item => item.EvidenceId == "D14"); + var environment = CreateEnvironment(fixture.Environment); + var matcher = new ShellApprovalMatcher(environment); + var arguments = new Dictionary + { + ["Command"] = fixture.Command, + ["WorkingDirectory"] = fixture.InitialWorkingDirectory, + }; + var grants = fixture.Available.PersistentGrants.Select(grant => + ApprovalEntry.CreateTokenPrefix( + Enum.Parse(grant.Shell), + grant.Tokens, + grant.Directory)).ToList(); + + var invocation = matcher.AnalyzeInvocation( + new ToolName("shell_execute"), + arguments); + + Assert.False(invocation.IsMessy); + Assert.True(matcher.IsApproved( + new ToolName("shell_execute"), + arguments, + grants, + fixture.InitialWorkingDirectory)); + Assert.False(new ToolPathPolicy(environment, ["/protected"]) + .CommandReferencesDeniedPath( + fixture.Command, + fixture.InitialWorkingDirectory)); } [Fact] @@ -515,6 +557,8 @@ internal sealed record PolicyAuthoredPathFact public required List AuthoredValues { get; init; } + public required List AuthoredFileSystemValues { get; init; } + public required string AuthoredPathShape { get; init; } public required string ExpectedPathPolicy { get; init; } diff --git a/src/Netclaw.Security.Tests/ShellApprovalMatcherTests.cs b/src/Netclaw.Security.Tests/ShellApprovalMatcherTests.cs index 64ad0fa56..fb5ad2c6d 100644 --- a/src/Netclaw.Security.Tests/ShellApprovalMatcherTests.cs +++ b/src/Netclaw.Security.Tests/ShellApprovalMatcherTests.cs @@ -93,6 +93,79 @@ public void Bash_live_read_and_diagnostic_shapes_are_reusable(string command) }); } + [Fact(SkipUnless = nameof(IsPosix), Skip = "POSIX-only path semantics")] + public void Bash_finite_filesystem_loop_uses_bounded_path_scopes() + { + const string command = + "for f in src/A.cs src/B.cs; do cat /work/$f; done"; + var arguments = Args(command, "/work"); + + var analysis = _matcher.AnalyzeInvocation( + new ToolName("shell_execute"), + arguments); + + Assert.False(analysis.IsMessy); + var candidate = Assert.Single(analysis.Candidates); + Assert.Equal("cat", candidate.Verb); + Assert.Equal("/work", candidate.Directory); + Assert.True(_matcher.IsApproved( + new ToolName("shell_execute"), + arguments, + [InDir("cat", "/work")], + cwd: "/work")); + } + + [Fact(SkipUnless = nameof(IsPosix), Skip = "POSIX-only path semantics")] + public void Bash_finite_filesystem_loop_keeps_external_scopes_exact() + { + const string command = + "for f in /work/A.cs /work2/B.cs; do cat \"$f\"; done"; + + var analysis = _matcher.AnalyzeInvocation( + new ToolName("shell_execute"), + Args(command, "/work")); + + Assert.False(analysis.IsMessy); + Assert.Equal( + ["/work/A.cs", "/work2/B.cs"], + analysis.Candidates.Select(static candidate => candidate.Directory)); + } + + [Fact(SkipUnless = nameof(IsPosix), Skip = "POSIX-only path semantics")] + public void Bash_finite_filesystem_loop_rejects_a_symlink_scope() + { + var root = Path.Combine( + Path.GetTempPath(), + $"netclaw-authored-loop-{Guid.NewGuid():N}"); + var projectDirectory = Path.Combine(root, "project"); + var externalDirectory = Path.Combine(root, "external"); + var externalFile = Path.Combine(externalDirectory, "secret.txt"); + var link = Path.Combine(projectDirectory, "link.txt"); + Directory.CreateDirectory(projectDirectory); + Directory.CreateDirectory(externalDirectory); + File.WriteAllText(externalFile, "secret"); + File.CreateSymbolicLink(link, externalFile); + + try + { + var arguments = Args( + "for f in link.txt safe.txt; do cat \"$f\"; done", + projectDirectory); + + Assert.True(_matcher.IsMessy( + new ToolName("shell_execute"), + arguments)); + Assert.Empty(_matcher.ExtractCandidates( + new ToolName("shell_execute"), + arguments)); + } + finally + { + File.Delete(link); + Directory.Delete(root, recursive: true); + } + } + [Fact] public void Power_shell_matcher_uses_the_native_power_shell_grammar() { diff --git a/src/Netclaw.Security.Tests/ShellCommandAnalysisTests.cs b/src/Netclaw.Security.Tests/ShellCommandAnalysisTests.cs index f2159f073..b306270e0 100644 --- a/src/Netclaw.Security.Tests/ShellCommandAnalysisTests.cs +++ b/src/Netclaw.Security.Tests/ShellCommandAnalysisTests.cs @@ -247,6 +247,22 @@ public void Bash_finite_loop_data_keeps_static_structure() Assert.Equal(["first", "second"], authored.Values); } + [Fact] + public void Bash_finite_filesystem_loop_uses_the_strong_authored_domain() + { + var analysis = _analyzer.Analyze( + "for f in src/A.cs src/B.cs; do cat /work/$f; done", + "/work"); + + Assert.Equal(ShellAnalysisFailure.None, analysis.Failure); + Assert.False(analysis.HasDynamicSyntax, Describe(analysis)); + var argument = Assert.Single(Assert.Single(analysis.Commands).Arguments); + Assert.IsType(argument.Value); + var authored = Assert.IsType( + argument.AuthoredFileSystemValue); + Assert.Equal(["/work/src/A.cs", "/work/src/B.cs"], authored.Values); + } + [Theory] [InlineData("status-report \"$1\"")] [InlineData("rm \"$1\"")] @@ -256,6 +272,8 @@ public void Bash_finite_loop_data_keeps_static_structure() [InlineData("sh -c \"$1\"")] [InlineData("sh -c \"$?\"")] [InlineData("for value in first second; do rm \"$value\"; done")] + [InlineData("for f in 'src/A.cs /etc/passwd'; do cat /work/$f; done")] + [InlineData("for f in '*.cs'; do cat /work/$f; done")] public void Bash_unknown_or_authority_bearing_data_stays_dynamic(string command) { var analyzer = new ShellCommandAnalyzer(BashEnvironment); diff --git a/src/Netclaw.Security.Tests/ToolPathPolicyTests.cs b/src/Netclaw.Security.Tests/ToolPathPolicyTests.cs index 2f048cd7f..7a4f13de1 100644 --- a/src/Netclaw.Security.Tests/ToolPathPolicyTests.cs +++ b/src/Netclaw.Security.Tests/ToolPathPolicyTests.cs @@ -67,6 +67,16 @@ public void CommandReferencesDeniedPath_allows_safe_commands() Assert.False(policy.CommandReferencesDeniedPath("echo hello")); } + [Fact] + public void CommandReferencesDeniedPath_checks_finite_authored_filesystem_values() + { + var policy = new ToolPathPolicy(["/work/src/B.cs"]); + const string command = + "for f in src/A.cs src/B.cs; do cat /work/$f; done"; + + Assert.True(policy.CommandReferencesDeniedPath(command, "/work")); + } + [Fact] public void CommandReferencesDeniedPath_checks_native_power_shell_path() { diff --git a/src/Netclaw.Security/IToolApprovalMatcher.cs b/src/Netclaw.Security/IToolApprovalMatcher.cs index 3a829fcfd..81c2e0821 100644 --- a/src/Netclaw.Security/IToolApprovalMatcher.cs +++ b/src/Netclaw.Security/IToolApprovalMatcher.cs @@ -8,6 +8,7 @@ using System.Text.Json; using Netclaw.Configuration; using Netclaw.Tools; +using ShellSyntaxTree; namespace Netclaw.Security; @@ -348,6 +349,21 @@ private IReadOnlyList ExtractCandidatesViaAnalysis( directories.Add(ResolveAuthorizationScope(verb, arg, resolved, pathStyle)); } + + foreach (var argument in occurrence.Arguments) + { + if (argument.Argument.IsPath) + continue; + + var authoredDirectories = ResolveAuthoredFileSystemDirectories( + argument.AuthoredFileSystemValue, + clauseWorkingDirectory, + pathStyle); + if (authoredDirectories is null) + return null; + + directories.AddRange(authoredDirectories); + } } foreach (var redirect in occurrence.Redirects) @@ -392,6 +408,80 @@ private IReadOnlyList ExtractCandidatesViaAnalysis( return directories.Distinct(StringComparer.Ordinal).ToList(); } + private static IReadOnlyList? ResolveAuthoredFileSystemDirectories( + ShellValueDomain domain, + string? workingDirectory, + ShellPathStyle pathStyle) + { + if (domain is ShellValueDomain.Unknown) + return []; + + IReadOnlyList paths; + switch (domain) + { + case ShellValueDomain.Exact exact: + paths = [exact.Value]; + break; + case ShellValueDomain.FiniteSet finite: + paths = finite.Values; + break; + default: + return null; + } + + var directories = new List(paths.Count); + foreach (var path in paths) + { + if (string.IsNullOrWhiteSpace(path) + || !IsRootedForPathStyle(path, pathStyle) + || UsesHostPathStyle(pathStyle) && HasUnsafeHostPath(path)) + { + return null; + } + + directories.Add(path); + } + + if (!string.IsNullOrWhiteSpace(workingDirectory) + && IsRootedForPathStyle(workingDirectory, pathStyle) + && directories.All(path => IsWithinRootForPathStyle( + path, + workingDirectory, + pathStyle))) + { + return [workingDirectory]; + } + + return directories; + } + + private static bool IsWithinRootForPathStyle( + string path, + string root, + ShellPathStyle pathStyle) + { + // Parser values can describe Windows paths on a POSIX test host. + // Host Path APIs cannot make this grammar-specific comparison. + var comparison = pathStyle == ShellPathStyle.Windows + ? StringComparison.OrdinalIgnoreCase + : StringComparison.Ordinal; + if (string.Equals(path, root, comparison)) + return true; + if (!path.StartsWith(root, comparison)) + return false; + if (root.EndsWith("/", StringComparison.Ordinal) + || pathStyle == ShellPathStyle.Windows + && root.EndsWith("\\", StringComparison.Ordinal)) + { + return true; + } + + return path.Length > root.Length + && (path[root.Length] == '/' + || pathStyle == ShellPathStyle.Windows + && path[root.Length] == '\\'); + } + private static string? ResolveAuthorizationScope( string verb, ShellSyntaxTree.Arg arg, diff --git a/src/Netclaw.Security/ShellCommandAnalysis.cs b/src/Netclaw.Security/ShellCommandAnalysis.cs index cd947d0ed..2789810f2 100644 --- a/src/Netclaw.Security/ShellCommandAnalysis.cs +++ b/src/Netclaw.Security/ShellCommandAnalysis.cs @@ -340,7 +340,8 @@ private bool CommandHasDynamicSyntax( && !IsAccountedExecutionRegionArgument( command, arg, - accountedRegionArguments)) + accountedRegionArguments) + && !HasBoundedAuthoredFileSystemValue(command, arg)) || command.Clause.Args.Any(static arg => arg.IsPath && arg.Kind != ArgKind.Glob @@ -412,6 +413,14 @@ private static bool IsAccountedExecutionRegionArgument( => argument.Argument.Kind == ArgKind.DynamicSkip && accountedRegionArguments.Contains(argument.Element); + private static bool HasBoundedAuthoredFileSystemValue( + CommandOccurrence command, + Arg argument) + => command.Arguments.Any(analyzed => + ReferenceEquals(analyzed.Argument, argument) + && analyzed.AuthoredFileSystemValue is ShellValueDomain.Exact + or ShellValueDomain.FiniteSet); + private static bool IsKnownAncestor(ShellSyntaxNode ancestor) => ancestor is ShellBlockSyntax or SimpleCommandSyntax @@ -432,12 +441,25 @@ private static bool HasUnsupportedWorkingDirectory(ShellValueDomain workingDirec private static bool HasUnsupportedArgumentDomain(AnalyzedArgument argument) { + if (argument.AuthoredFileSystemValue is not ShellValueDomain.Unknown + and not ShellValueDomain.Exact + and not ShellValueDomain.FiniteSet) + { + return true; + } + var value = argument.Value; - if (value is ShellValueDomain.Unknown - && !argument.Argument.IsPath - && argument.AuthoredValue is not ShellValueDomain.Unknown) + if (value is ShellValueDomain.Unknown) { - value = argument.AuthoredValue; + if (argument.AuthoredFileSystemValue is not ShellValueDomain.Unknown) + { + value = argument.AuthoredFileSystemValue; + } + else if (!argument.Argument.IsPath + && argument.AuthoredValue is not ShellValueDomain.Unknown) + { + value = argument.AuthoredValue; + } } return value switch diff --git a/src/Netclaw.Security/ToolPathPolicy.cs b/src/Netclaw.Security/ToolPathPolicy.cs index d7944b50a..b80507d4f 100644 --- a/src/Netclaw.Security/ToolPathPolicy.cs +++ b/src/Netclaw.Security/ToolPathPolicy.cs @@ -306,13 +306,16 @@ private bool StructuredAnalysisReferencesDeniedPath( foreach (var effective in occurrence.Arguments) { - if (!effective.Element.IsPath) + if (effective.Element.IsPath + && DomainReferencesDeniedPath(effective.Value)) { - continue; + return true; } - if (DomainReferencesDeniedPath(effective.Value)) + if (DomainReferencesDeniedPath(effective.AuthoredFileSystemValue)) + { return true; + } } foreach (var redirect in occurrence.Redirects)