From bfdaaf75168c8316366c0ce7b32253bc5c319206 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Sun, 10 May 2026 00:36:56 +0000 Subject: [PATCH] fix(doctor): suppress warnings for explicit Personal posture and tool profile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the user explicitly chooses Personal posture via `netclaw init`, doctor should not warn about Personal + HostAllowed shell or an unrestricted Personal tool profile — these are intentional choices. Only warn when the values are implicit (resolved from fallback defaults), preserving doctor's defensive posture for ambiguous configs. Fixes #949 Changes: - SecurityPolicyDoctorCheck: only warn about Personal + HostAllowed when DeploymentPosture is not explicitly set - ToolAudienceProfilesDoctorCheck: only warn about unrestricted Personal profile when it's using fallback defaults (not explicitly written) - Updated tests to reflect new behavior --- .../Doctor/SecurityPolicyDoctorCheckTests.cs | 33 +++++++++++-- .../ToolAudienceProfilesDoctorCheckTests.cs | 49 ++++++++++++++++--- .../Doctor/SecurityPolicyDoctorCheck.cs | 8 ++- .../Doctor/ToolAudienceProfilesDoctorCheck.cs | 9 +++- 4 files changed, 85 insertions(+), 14 deletions(-) diff --git a/src/Netclaw.Cli.Tests/Doctor/SecurityPolicyDoctorCheckTests.cs b/src/Netclaw.Cli.Tests/Doctor/SecurityPolicyDoctorCheckTests.cs index c8d17d1e..2622220a 100644 --- a/src/Netclaw.Cli.Tests/Doctor/SecurityPolicyDoctorCheckTests.cs +++ b/src/Netclaw.Cli.Tests/Doctor/SecurityPolicyDoctorCheckTests.cs @@ -1,4 +1,4 @@ -// ----------------------------------------------------------------------- +// ----------------------------------------------------------------------- // // Copyright (C) 2026 - 2026 Petabridge, LLC // @@ -57,8 +57,10 @@ public async Task NullPosture_StrictDisabled_IsError() } [Fact] - public async Task ExplicitPersonalPosture_HostAllowed_Warns() + public async Task ExplicitPersonalPosture_HostAllowed_Passes() { + // When the user explicitly sets Personal + HostAllowed, doctor should + // respect that intentional choice and pass cleanly. WriteConfig(""" { "configVersion": 1, @@ -73,8 +75,31 @@ public async Task ExplicitPersonalPosture_HostAllowed_Warns() var check = new SecurityPolicyDoctorCheck(_paths); var result = await check.RunAsync(TestContext.Current.CancellationToken); - Assert.Equal(DoctorSeverity.Warning, result.Severity); - Assert.Contains("full host access", result.Message); + Assert.Equal(DoctorSeverity.Pass, result.Severity); + Assert.Contains("Personal", result.Message); + } + + [Fact] + public async Task ImplicitPersonalPosture_HostAllowed_Warns() + { + // When DeploymentPosture is missing and StrictDefaults is false, + // the fallback resolves to Personal with HostAllowed — this should + // warn because the user didn't explicitly choose this. + WriteConfig(""" + { + "configVersion": 1, + "Security": { + "StrictDefaults": false + } + } + """); + + var check = new SecurityPolicyDoctorCheck(_paths); + var result = await check.RunAsync(TestContext.Current.CancellationToken); + + // StrictDefaults=false with no DeploymentPosture is an Error first + Assert.Equal(DoctorSeverity.Error, result.Severity); + Assert.Contains("silently assumes Personal posture", result.Message); } [Fact] diff --git a/src/Netclaw.Cli.Tests/Doctor/ToolAudienceProfilesDoctorCheckTests.cs b/src/Netclaw.Cli.Tests/Doctor/ToolAudienceProfilesDoctorCheckTests.cs index a231d36c..380da7cd 100644 --- a/src/Netclaw.Cli.Tests/Doctor/ToolAudienceProfilesDoctorCheckTests.cs +++ b/src/Netclaw.Cli.Tests/Doctor/ToolAudienceProfilesDoctorCheckTests.cs @@ -1,4 +1,4 @@ -// ----------------------------------------------------------------------- +// ----------------------------------------------------------------------- // // Copyright (C) 2026 - 2026 Petabridge, LLC // @@ -87,8 +87,11 @@ public async Task TeamFilesystemAll_IsError() } [Fact] - public async Task UnrestrictedPersonalProfile_Warns() + public async Task UnrestrictedPersonalProfile_Explicit_NoUnrestrictedWarning() { + // When Personal profile is explicitly written, unrestricted access is intentional. + // Doctor should not warn about the unrestricted profile itself, but may still + // warn about missing shell_execute approval gate. WriteConfig( """ { @@ -111,9 +114,38 @@ public async Task UnrestrictedPersonalProfile_Warns() var check = new ToolAudienceProfilesDoctorCheck(_paths); var result = await check.RunAsync(TestContext.Current.CancellationToken); - Assert.Equal(DoctorSeverity.Warning, result.Severity); + // Should not warn about unrestricted profile when it's explicit + Assert.DoesNotContain("Personal profile allows all tools", result.Message); + // May still warn about missing shell approval gate (different message) + Assert.Contains("without an explicit shell_execute approval gate", result.Message); + } + + [Fact] + public async Task UnrestrictedPersonalProfile_Implicit_Warns() + { + // When Personal profile is NOT in config (fallback defaults), unrestricted + // access should warn. AudienceProfiles must exist but Personal must be absent. + WriteConfig( + """ + { + "configVersion": 1, + "Tools": { + "ShellMode": "HostAllowed", + "AudienceProfiles": { + "Public": { + "ToolsMode": "AllowList" + } + } + } + } + """); + + var check = new ToolAudienceProfilesDoctorCheck(_paths); + var result = await check.RunAsync(TestContext.Current.CancellationToken); + + // Should warn about missing profiles and unrestricted fallback + Assert.Contains("Missing explicit profiles for", result.Message); Assert.Contains("Personal profile allows all tools", result.Message); - Assert.Contains("host shell", result.Message); } [Fact] @@ -191,8 +223,11 @@ public async Task McpServerWithToolGrants_NoSupplyChainWarning() } [Fact] - public async Task RecommendedProfiles_Pass() + public async Task RecommendedProfiles_WarnsAboutShellGateOnly() { + // CreateProfiles() writes all three profiles explicitly, so Personal is + // explicit. The unrestricted warning is suppressed, but the shell + // approval gate warning still fires (no ApprovalPolicy on Personal). var toolConfig = new ToolConfig { ShellMode = ShellExecutionMode.HostAllowed, @@ -209,7 +244,9 @@ public async Task RecommendedProfiles_Pass() var result = await check.RunAsync(TestContext.Current.CancellationToken); Assert.Equal(DoctorSeverity.Warning, result.Severity); - Assert.Contains("Personal profile allows all tools", result.Message); + // Unrestricted warning suppressed for explicit Personal + Assert.DoesNotContain("Personal profile allows all tools", result.Message); + // Shell approval gate warning still fires Assert.Contains("without an explicit shell_execute approval gate", result.Message); } diff --git a/src/Netclaw.Cli/Doctor/SecurityPolicyDoctorCheck.cs b/src/Netclaw.Cli/Doctor/SecurityPolicyDoctorCheck.cs index c59688da..86bb932b 100644 --- a/src/Netclaw.Cli/Doctor/SecurityPolicyDoctorCheck.cs +++ b/src/Netclaw.Cli/Doctor/SecurityPolicyDoctorCheck.cs @@ -1,4 +1,4 @@ -// ----------------------------------------------------------------------- +// ----------------------------------------------------------------------- // // Copyright (C) 2026 - 2026 Petabridge, LLC // @@ -63,8 +63,12 @@ public Task RunAsync(CancellationToken cancellationToken = de warnings.Add("DeploymentPosture not set; strict fallback resolved to Public."); } + // Only warn about Personal + HostAllowed when the posture is implicit + // (resolved from fallback defaults). If the user explicitly set + // DeploymentPosture = Personal, they chose this intentionally. if (effective.DeploymentPosture == DeploymentPosture.Personal - && effective.ShellExecutionMode == ShellExecutionMode.HostAllowed) + && effective.ShellExecutionMode == ShellExecutionMode.HostAllowed + && !config.DeploymentPosture.HasValue) { warnings.Add("Personal posture with HostAllowed shell — full host access is enabled."); } diff --git a/src/Netclaw.Cli/Doctor/ToolAudienceProfilesDoctorCheck.cs b/src/Netclaw.Cli/Doctor/ToolAudienceProfilesDoctorCheck.cs index 861eca11..1ca6646d 100644 --- a/src/Netclaw.Cli/Doctor/ToolAudienceProfilesDoctorCheck.cs +++ b/src/Netclaw.Cli/Doctor/ToolAudienceProfilesDoctorCheck.cs @@ -1,4 +1,4 @@ -// ----------------------------------------------------------------------- +// ----------------------------------------------------------------------- // // Copyright (C) 2026 - 2026 Petabridge, LLC // @@ -90,7 +90,12 @@ public Task RunAsync(CancellationToken cancellationToken = de warnings.Add($"Missing explicit profiles for {string.Join(", ", missingProfiles)}; fallback defaults are in effect."); } - if (IsUnrestrictedPersonalProfile(toolConfig.AudienceProfiles.Personal)) + // Only warn about unrestricted Personal when it's using fallback defaults. + // If the Personal profile was explicitly written (e.g., by `netclaw init`), + // the user made an intentional choice and this warning is noise. + var personalExplicit = !missingProfiles.Contains("personal"); + if (IsUnrestrictedPersonalProfile(toolConfig.AudienceProfiles.Personal) + && !personalExplicit) { warnings.Add("Personal profile allows all tools and unrestricted filesystem access."); if (toolConfig.ShellMode == ShellExecutionMode.HostAllowed)