From d5c5a0822335af9377601a25b568de877ff2f4ef Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Fri, 11 Sep 2026 20:37:37 -0500 Subject: [PATCH 1/6] Add managed Git skill plugin sync --- .../Skills/SkillInventoryRefresherTests.cs | 26 ++ .../Skills/SkillInventoryRefresher.cs | 36 ++- src/Netclaw.Actors/Skills/SkillScanner.cs | 17 +- src/Netclaw.Actors/Tools/SkillManageTool.cs | 9 + .../DaemonToolPathPolicyFactoryTests.cs | 14 + .../GitSkillPluginSyncServiceTests.cs | 302 ++++++++++++++++++ .../DaemonToolPathPolicyFactory.cs | 2 + src/Netclaw.Daemon/Program.cs | 10 + .../Services/GitSkillPluginAcquirer.cs | 25 +- .../Services/ServerFeedSkillSyncService.cs | 273 +++++++++++++++- .../Services/SkillDirectoryWatcherService.cs | 13 +- 11 files changed, 710 insertions(+), 17 deletions(-) create mode 100644 src/Netclaw.Daemon.Tests/Services/GitSkillPluginSyncServiceTests.cs diff --git a/src/Netclaw.Actors.Tests/Skills/SkillInventoryRefresherTests.cs b/src/Netclaw.Actors.Tests/Skills/SkillInventoryRefresherTests.cs index 70581c32d..64e58744a 100644 --- a/src/Netclaw.Actors.Tests/Skills/SkillInventoryRefresherTests.cs +++ b/src/Netclaw.Actors.Tests/Skills/SkillInventoryRefresherTests.cs @@ -72,6 +72,32 @@ public void Refresh_preserves_all_sources_and_applies_canonical_precedence() Assert.Contains(result.AcceptedSkills, skill => skill.Name == "new-native"); } + [Fact] + public void Managed_git_plugins_stay_between_server_feeds_and_external_sources() + { + var feedRoot = _paths.ServerFeedDirectory("managed"); + var gitRoot = Path.Join(_home, "git-plugin"); + var externalRoot = Path.Join(_home, "external"); + WriteSkill(_paths.SkillsDirectory, "native-only", "native"); + WriteSkill(feedRoot, "shared", "feed wins"); + WriteSkill(gitRoot, "shared", "git loses"); + WriteSkill(gitRoot, "git-only", "managed git"); + WriteSkill(externalRoot, "shared", "external loses"); + WriteSkill(externalRoot, "external-only", "external"); + + var refresher = CreateRefresher( + new SkillFeedsConfig { Feeds = [new SkillFeedSource { Name = "managed" }] }, + [new ResolvedExternalSource("external", [externalRoot], AllowSymlinks: false)]); + + refresher.ReplaceManagedGitPluginSourcesAndRefresh( + [new ResolvedExternalSource("managed-git:fixture", [gitRoot], AllowSymlinks: false)]); + var result = refresher.Refresh(); + + Assert.Equal("feed wins", _registry.GetByName("shared")!.Description); + Assert.Contains(result.AcceptedSkills, skill => skill.Name == "git-only"); + Assert.Contains(result.AcceptedSkills, skill => skill.Name == "external-only"); + } + [Fact] public void ReplaceAll_never_exposes_a_partially_replaced_inventory() { diff --git a/src/Netclaw.Actors/Skills/SkillInventoryRefresher.cs b/src/Netclaw.Actors/Skills/SkillInventoryRefresher.cs index eba30b808..cb27a7de0 100644 --- a/src/Netclaw.Actors/Skills/SkillInventoryRefresher.cs +++ b/src/Netclaw.Actors/Skills/SkillInventoryRefresher.cs @@ -19,6 +19,7 @@ public sealed class SkillInventoryRefresher private readonly IReadOnlyList _externalSources; private readonly SkillRegistry _registry; private readonly SkillIndexPublisher _indexPublisher; + private IReadOnlyList _managedGitPluginSources = []; public SkillInventoryRefresher( NetclawPaths paths, @@ -38,17 +39,38 @@ public MergedSkillScanResult Refresh() { lock (_refreshLock) { - var result = SkillScanner.ScanAndMerge( - _paths.SkillsDirectory, - ResolveServerFeedSources(), - _externalSources); + return RefreshCore(); + } + } - _registry.ReplaceAll(result.AcceptedSkills, result.Issues); - _indexPublisher.Publish(); - return result; + /// + /// Replaces the managed Git plugin paths and publishes one inventory snapshot. + /// + public MergedSkillScanResult ReplaceManagedGitPluginSourcesAndRefresh( + IReadOnlyList managedGitPluginSources) + { + ArgumentNullException.ThrowIfNull(managedGitPluginSources); + + lock (_refreshLock) + { + _managedGitPluginSources = managedGitPluginSources; + return RefreshCore(); } } + private MergedSkillScanResult RefreshCore() + { + var result = SkillScanner.ScanAndMerge( + _paths.SkillsDirectory, + ResolveServerFeedSources(), + _managedGitPluginSources, + _externalSources); + + _registry.ReplaceAll(result.AcceptedSkills, result.Issues); + _indexPublisher.Publish(); + return result; + } + private IReadOnlyList ResolveServerFeedSources() { var sources = new List(); diff --git a/src/Netclaw.Actors/Skills/SkillScanner.cs b/src/Netclaw.Actors/Skills/SkillScanner.cs index 3fa5fc3fd..e17492ef1 100644 --- a/src/Netclaw.Actors/Skills/SkillScanner.cs +++ b/src/Netclaw.Actors/Skills/SkillScanner.cs @@ -208,6 +208,17 @@ public static MergedSkillScanResult ScanAndMerge( string nativeSkillsDirectory, IReadOnlyList serverFeedSources, IReadOnlyList externalSources) + => ScanAndMerge(nativeSkillsDirectory, serverFeedSources, Array.Empty(), externalSources); + + /// + /// Scans all configured skill tiers. Managed Git plugins are lower priority than + /// organization feeds and higher priority than local external directories. + /// + public static MergedSkillScanResult ScanAndMerge( + string nativeSkillsDirectory, + IReadOnlyList serverFeedSources, + IReadOnlyList managedGitPluginSources, + IReadOnlyList externalSources) { var nativeScan = Scan(nativeSkillsDirectory, allowSymlinks: false, strictNameMatch: true); var allAccepted = new List(nativeScan.AcceptedSkills); @@ -219,7 +230,11 @@ public static MergedSkillScanResult ScanAndMerge( // Server feed sources (second tier — org-managed private skill servers) MergeSources(serverFeedSources, allAccepted, allIssues, knownNames, allowFrontmatterlessFlatFiles: false); - // External filesystem sources (third tier — Claude Code, Open Code, custom paths) + // Managed Git plugins are verified, immutable third-party content. They yield + // to organization feeds but take priority over locally discovered sources. + MergeSources(managedGitPluginSources, allAccepted, allIssues, knownNames, allowFrontmatterlessFlatFiles: false); + + // External filesystem sources are the lowest-precedence tier. MergeSources(externalSources, allAccepted, allIssues, knownNames, allowFrontmatterlessFlatFiles: true); return new MergedSkillScanResult(allAccepted, allIssues); diff --git a/src/Netclaw.Actors/Tools/SkillManageTool.cs b/src/Netclaw.Actors/Tools/SkillManageTool.cs index f45a88373..04f46e7c7 100644 --- a/src/Netclaw.Actors/Tools/SkillManageTool.cs +++ b/src/Netclaw.Actors/Tools/SkillManageTool.cs @@ -449,6 +449,8 @@ private bool IsSystemSkill(string name) return $"Cannot {verb} system skills. System skills are read-only."; if (IsServerFeedSkill(skill)) return $"Cannot {verb} server feed skills. Server feed skill directories are read-only."; + if (IsManagedGitPluginSkill(skill)) + return $"Cannot {verb} managed Git plugin skills. Managed Git plugin directories are read-only."; if (IsExternalSkill(skill)) return $"Cannot {verb} external skills. External skill directories are read-only."; return null; @@ -471,6 +473,13 @@ private bool IsExternalSkill(SkillEntry skill) return !PathUtility.IsWithinRoot(skillPath, nativeRoot); } + private bool IsManagedGitPluginSkill(SkillEntry skill) + { + var managedRoot = PathUtility.Normalize(_paths.ManagedGitSkillsDirectory); + var skillPath = PathUtility.Normalize(Path.GetDirectoryName(skill.FilePath)!); + return PathUtility.IsWithinRoot(skillPath, managedRoot); + } + private static void AtomicWrite(string path, string content) { var tempPath = path + ".tmp"; diff --git a/src/Netclaw.Daemon.Tests/Configuration/DaemonToolPathPolicyFactoryTests.cs b/src/Netclaw.Daemon.Tests/Configuration/DaemonToolPathPolicyFactoryTests.cs index 37f2ef329..f39d31b90 100644 --- a/src/Netclaw.Daemon.Tests/Configuration/DaemonToolPathPolicyFactoryTests.cs +++ b/src/Netclaw.Daemon.Tests/Configuration/DaemonToolPathPolicyFactoryTests.cs @@ -70,6 +70,20 @@ public void System_skills_are_readable_but_not_writable(ShellPlatform platform) Assert.True(policy.IsDenied(skillPath)); } + [Fact] + public void Managed_git_plugin_skills_are_readable_but_not_writable_or_shell_accessible() + { + var paths = new NetclawPaths(Path.Combine(Path.GetTempPath(), "netclaw-policy-contract")); + var policy = DaemonToolPathPolicyFactory.Create( + paths, + ShellExecutionEnvironment.CreateBash(ShellPlatform.Linux)); + var skillPath = Path.Combine(paths.ManagedGitSkillsDirectory, "fixture", "commit", "SKILL.md"); + + Assert.False(policy.IsReadDenied(skillPath)); + Assert.True(policy.IsDenied(skillPath)); + Assert.True(policy.CommandReferencesDeniedPath($"cat '{skillPath}'")); + } + [Theory] [InlineData("tool-index.md")] [InlineData("mcp/synthetic-server.md")] diff --git a/src/Netclaw.Daemon.Tests/Services/GitSkillPluginSyncServiceTests.cs b/src/Netclaw.Daemon.Tests/Services/GitSkillPluginSyncServiceTests.cs new file mode 100644 index 000000000..81dc45ec7 --- /dev/null +++ b/src/Netclaw.Daemon.Tests/Services/GitSkillPluginSyncServiceTests.cs @@ -0,0 +1,302 @@ +// ----------------------------------------------------------------------- +// +// Copyright (C) 2026 - 2026 Petabridge, LLC +// +// ----------------------------------------------------------------------- +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Time.Testing; +using Netclaw.Actors.Skills; +using Netclaw.Configuration; +using Netclaw.Daemon.Services; +using Netclaw.Security.Skills; +using Netclaw.Tests.Utilities; +using Xunit; + +namespace Netclaw.Daemon.Tests.Services; + +public sealed class GitSkillPluginSyncServiceTests : IDisposable +{ + private const string FirstCommit = "13e26d39ed01d97ea592235d041304d289f4ba07"; + private const string SecondCommit = "23e26d39ed01d97ea592235d041304d289f4ba08"; + private readonly DisposableTempDir _temp = new(); + private readonly NetclawPaths _paths; + private readonly FakeTimeProvider _time = new(); + + public GitSkillPluginSyncServiceTests() + { + _paths = new NetclawPaths(_temp.Path); + _paths.EnsureDirectoriesExist(); + } + + public void Dispose() => _temp.Dispose(); + + [Fact] + public async Task First_install_publishes_all_skills_in_one_inventory_snapshot() + { + var source = Source(); + var acquirer = new FakeAcquirer(_paths, source, FirstCommit, "1.0.0", "plugin-skill"); + var registry = new SkillRegistry(); + var publicationCount = 0; + var refresher = CreateRefresher(registry, () => publicationCount++); + var service = await CreateServiceAsync(source, refresher, acquirer, new RecordingSink()); + + var result = await service.SyncAsync(TestContext.Current.CancellationToken); + + Assert.Equal(2, publicationCount); + Assert.Equal(FirstCommit, Assert.Single(result.Sources).Commit); + Assert.NotNull(registry.GetByName("plugin-skill")); + } + + [Fact] + public async Task Branch_update_keeps_prior_directory_and_publishes_new_files() + { + var source = Source(); + var registry = new SkillRegistry(); + var refresher = CreateRefresher(registry, static () => { }); + var first = new FakeAcquirer(_paths, source, FirstCommit, "1.0.0", "plugin-skill", "old text"); + var service = await CreateServiceAsync(source, refresher, first, new RecordingSink()); + await service.SyncAsync(TestContext.Current.CancellationToken); + var oldPath = registry.GetByName("plugin-skill")!.FilePath; + + var second = new FakeAcquirer(_paths, source, SecondCommit, "2.0.0", "plugin-skill", "new text"); + var nextService = await CreateServiceAsync(source, refresher, second, new RecordingSink()); + await nextService.SyncAsync(TestContext.Current.CancellationToken); + + Assert.True(File.Exists(oldPath)); + Assert.Contains("old text", await File.ReadAllTextAsync(oldPath, TestContext.Current.CancellationToken)); + Assert.Contains("new text", await File.ReadAllTextAsync( + registry.GetByName("plugin-skill")!.FilePath, TestContext.Current.CancellationToken)); + } + + [Fact] + public async Task Equal_version_records_last_observed_commit_without_a_second_fetch() + { + var source = Source(); + var registry = new SkillRegistry(); + var refresher = CreateRefresher(registry, static () => { }); + var first = new FakeAcquirer(_paths, source, FirstCommit, "1.0.0", "plugin-skill"); + var service = await CreateServiceAsync(source, refresher, first, new RecordingSink()); + await service.SyncAsync(TestContext.Current.CancellationToken); + + var next = new FakeAcquirer(_paths, source, SecondCommit, "1.0.0", "plugin-skill"); + var nextService = await CreateServiceAsync(source, refresher, next, new RecordingSink()); + await nextService.SyncAsync(TestContext.Current.CancellationToken); + await nextService.SyncAsync(TestContext.Current.CancellationToken); + + Assert.Equal(1, next.AcquireCount); + var receipt = await new GitSkillPluginStateStore(_paths, _time).GetReceiptAsync( + source.Name, TestContext.Current.CancellationToken); + Assert.NotNull(receipt); + Assert.Equal(FirstCommit, receipt.InstalledCommit); + Assert.Equal(SecondCommit, receipt.LastObservedCommit); + Assert.False(Directory.Exists(_paths.ManagedGitSkillCommitDirectory( + source.Name, GitSkillPluginSourceValidator.Fingerprint(source), SecondCommit))); + } + + [Fact] + public async Task Commit_pin_does_not_resolve_and_known_rejection_survives_a_new_service() + { + var source = Source(); + source.ReferenceKind = GitSkillPluginReferenceKind.Commit; + source.Reference = FirstCommit.ToUpperInvariant(); + var registry = new SkillRegistry(); + var refresher = CreateRefresher(registry, static () => { }); + var store = await CreateStoreAsync(); + await store.SaveRejectionAsync( + source.Name, + GitSkillPluginSourceValidator.Fingerprint(source), + FirstCommit, + "unsafe plugin", + false, + TestContext.Current.CancellationToken); + var acquirer = new FakeAcquirer(_paths, source, FirstCommit, "1.0.0", "plugin-skill") + { + FailOnResolve = true, + }; + var service = new ServerFeedSkillSyncService( + new SkillFeedsConfig { Plugins = [source] }, + _paths, + refresher, + _time, + new NoOpSkillContentScanner(), + NullLogger.Instance, + store, + acquirer, + new RecordingSink()); + + var result = await service.SyncAsync(TestContext.Current.CancellationToken); + + Assert.Equal(0, acquirer.ResolveCount); + Assert.Equal(0, acquirer.AcquireCount); + Assert.Equal(1, Assert.Single(result.Sources).RejectedCount); + } + + [Fact] + public async Task Security_rejection_persists_and_emits_one_claimed_alert() + { + var source = Source(); + var registry = new SkillRegistry(); + var refresher = CreateRefresher(registry, static () => { }); + var acquirer = new FakeAcquirer(_paths, source, FirstCommit, "1.0.0", "plugin-skill") + { + Rejection = new GitSkillPluginRejectedException(FirstCommit, "scanner result", true), + }; + var alerts = new RecordingSink(); + var service = await CreateServiceAsync(source, refresher, acquirer, alerts); + + await service.SyncAsync(TestContext.Current.CancellationToken); + await service.SyncAsync(TestContext.Current.CancellationToken); + + var alert = Assert.Single(alerts.Alerts); + Assert.Equal("skill.plugin.security_rejected", alert.Type); + Assert.Equal(AlertType.SkillPluginSecurityRejected, alert.Category); + Assert.DoesNotContain("scanner result", alert.Summary); + } + + [Fact] + public async Task Startup_cleanup_removes_staging_and_orphan_commits_but_keeps_receipt_directory() + { + var source = Source(); + var store = await CreateStoreAsync(); + await store.SaveReceiptAsync(source, FirstCommit, "1.0.0", TestContext.Current.CancellationToken); + var fingerprint = GitSkillPluginSourceValidator.Fingerprint(source); + var selected = _paths.ManagedGitSkillCommitDirectory(source.Name, fingerprint, FirstCommit); + var orphan = _paths.ManagedGitSkillCommitDirectory(source.Name, fingerprint, SecondCommit); + var staging = Path.Combine(_paths.ManagedGitSkillDirectory(source.Name), ".staging", "candidate"); + Directory.CreateDirectory(selected); + Directory.CreateDirectory(orphan); + Directory.CreateDirectory(staging); + var registry = new SkillRegistry(); + var service = new ServerFeedSkillSyncService( + new SkillFeedsConfig { Plugins = [source] }, + _paths, + CreateRefresher(registry, static () => { }), + _time, + new NoOpSkillContentScanner(), + NullLogger.Instance, + store, + new FakeAcquirer(_paths, source, FirstCommit, "1.0.0", "plugin-skill"), + new RecordingSink()); + + await service.SyncAsync(TestContext.Current.CancellationToken); + + Assert.True(Directory.Exists(selected)); + Assert.False(Directory.Exists(orphan)); + Assert.False(Directory.Exists(staging)); + } + + private async Task CreateStoreAsync() + { + await new SchemaMigrator(_paths, NullLogger.Instance) + .MigrateAsync(_paths.SqliteDbPath, TestContext.Current.CancellationToken); + return new GitSkillPluginStateStore(_paths, _time); + } + + private async Task CreateServiceAsync( + GitSkillPluginSource source, + SkillInventoryRefresher refresher, + FakeAcquirer acquirer, + RecordingSink sink) + { + var store = await CreateStoreAsync(); + return new ServerFeedSkillSyncService( + new SkillFeedsConfig { Plugins = [source] }, + _paths, + refresher, + _time, + new NoOpSkillContentScanner(), + NullLogger.Instance, + store, + acquirer, + sink); + } + + private SkillInventoryRefresher CreateRefresher(SkillRegistry registry, Action publish) + => new( + _paths, + new SkillFeedsConfig(), + [], + registry, + new SkillIndexPublisher(registry, new SkillIndexContextLayer(), (_, _) => + { + publish(); + return true; + })); + + private static GitSkillPluginSource Source() => new() + { + Name = "fixture", + Repository = "owner/repository", + Format = "codex", + ReferenceKind = GitSkillPluginReferenceKind.Branch, + Reference = "main", + }; + + private sealed class FakeAcquirer( + NetclawPaths paths, + GitSkillPluginSource source, + string commit, + string? version, + string skillName, + string description = "plugin guidance") : IGitSkillPluginAcquirer + { + public bool FailOnResolve { get; init; } + public GitSkillPluginRejectedException? Rejection { get; init; } + public int ResolveCount { get; private set; } + public int AcquireCount { get; private set; } + + public Task AcquireAsync( + GitSkillPluginSource ignored, + CancellationToken cancellationToken) + => AcquireAsync(ignored, commit, cancellationToken); + + public Task ResolveCommitAsync( + GitSkillPluginSource ignored, + CancellationToken cancellationToken) + { + ResolveCount++; + if (FailOnResolve) + throw new InvalidOperationException("The commit pin must not resolve."); + return Task.FromResult(commit); + } + + public Task AcquireAsync( + GitSkillPluginSource ignored, + string resolvedCommit, + CancellationToken cancellationToken) + { + AcquireCount++; + if (Rejection is not null) + throw Rejection; + + var directory = paths.ManagedGitSkillCommitDirectory( + source.Name, + GitSkillPluginSourceValidator.Fingerprint(source), + resolvedCommit); + var skillDirectory = Path.Combine(directory, skillName); + Directory.CreateDirectory(skillDirectory); + File.WriteAllText(Path.Combine(skillDirectory, "SKILL.md"), $$""" + --- + name: {{skillName}} + description: {{description}} + metadata: + version: {{version ?? ""}} + --- + + # {{skillName}} + """); + return Task.FromResult(new GitSkillPluginCandidate( + resolvedCommit, + version, + directory, + SkillScanner.Scan(directory).AcceptedSkills, + [])); + } + } + + private sealed class RecordingSink : IOperationalNotificationSink + { + public List Alerts { get; } = []; + public void Emit(OperationalAlert alert) => Alerts.Add(alert); + } +} diff --git a/src/Netclaw.Daemon/Configuration/DaemonToolPathPolicyFactory.cs b/src/Netclaw.Daemon/Configuration/DaemonToolPathPolicyFactory.cs index 2ffede70b..877eb078b 100644 --- a/src/Netclaw.Daemon/Configuration/DaemonToolPathPolicyFactory.cs +++ b/src/Netclaw.Daemon/Configuration/DaemonToolPathPolicyFactory.cs @@ -38,6 +38,7 @@ public static ToolPathPolicy Create( ..processControlPaths, paths.SystemSkillsDirectory, paths.ServerFeedsDirectory, + paths.ManagedGitSkillsDirectory, paths.ToolingShadowDirectory, ]; string[] readDenyList = @@ -65,6 +66,7 @@ public static ToolPathPolicy Create( ..sqliteSidecars, ..processControlPaths, paths.ToolingShadowDirectory, + paths.ManagedGitSkillsDirectory, ]; return new ToolPathPolicy( diff --git a/src/Netclaw.Daemon/Program.cs b/src/Netclaw.Daemon/Program.cs index 08bf18777..4f31e27eb 100644 --- a/src/Netclaw.Daemon/Program.cs +++ b/src/Netclaw.Daemon/Program.cs @@ -52,6 +52,7 @@ using Netclaw.Search; using Netclaw.Tools; using Netclaw.Security; +using Netclaw.Security.Skills; using static Microsoft.Extensions.Logging.LogLevel; // Handled first, before any directory creation, lock-file acquisition, or host startup: @@ -941,6 +942,15 @@ static void ConfigureDaemonServices( services.AddHostedService(); // The runner owns one pass. The actor owns startup, timers, and shared requests. + services.AddSingleton(); + services.AddHttpClient("GitSkillPlugin") + .ConfigurePrimaryHttpMessageHandler(GitSkillPluginAcquirer.CreateHttpHandler) + .AddNetclawHeaders("git-skill-plugin"); + services.AddSingleton(sp => new GitSkillPluginAcquirer( + sp.GetRequiredService().CreateClient("GitSkillPlugin"), + paths, + sp.GetRequiredService(), + sp.GetRequiredService())); services.AddSingleton(); services.AddSingleton( sp => sp.GetRequiredService()); diff --git a/src/Netclaw.Daemon/Services/GitSkillPluginAcquirer.cs b/src/Netclaw.Daemon/Services/GitSkillPluginAcquirer.cs index 21be63f43..09c98f148 100644 --- a/src/Netclaw.Daemon/Services/GitSkillPluginAcquirer.cs +++ b/src/Netclaw.Daemon/Services/GitSkillPluginAcquirer.cs @@ -19,6 +19,13 @@ namespace Netclaw.Daemon.Services; internal interface IGitSkillPluginAcquirer { Task AcquireAsync(GitSkillPluginSource source, CancellationToken cancellationToken); + + Task ResolveCommitAsync(GitSkillPluginSource source, CancellationToken cancellationToken); + + Task AcquireAsync( + GitSkillPluginSource source, + string commit, + CancellationToken cancellationToken); } internal sealed record GitSkillPluginCandidate( @@ -134,13 +141,27 @@ public async Task AcquireAsync( if (!GitSkillPluginSourceValidator.TryValidateSource(source, out var sourceError)) throw new InvalidOperationException(sourceError); + var commit = await ResolveCommitAsync(source, cancellationToken); + return await AcquireAsync(source, commit, cancellationToken); + } + + public async Task AcquireAsync( + GitSkillPluginSource source, + string commit, + CancellationToken cancellationToken) + { + if (!GitSkillPluginSourceValidator.TryValidateSource(source, out var sourceError)) + throw new InvalidOperationException(sourceError); + if (string.IsNullOrWhiteSpace(commit) || commit.Length != 40 || !commit.All(char.IsAsciiHexDigit)) + throw new InvalidDataException("The resolved commit identity is invalid."); + + commit = commit.ToLowerInvariant(); using var timeout = new CancellationTokenSource( TimeSpan.FromSeconds(source.TimeoutSeconds), _timeProvider); using var linked = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, timeout.Token); var token = linked.Token; try { - var commit = await ResolveCommitAsync(source, token); var archivePath = Path.Combine(_paths.CacheDirectory, "git-skill-archives", $"{Guid.NewGuid():N}.tar.gz"); Directory.CreateDirectory(Path.GetDirectoryName(archivePath)!); try @@ -263,7 +284,7 @@ public async Task AcquireAsync( } } - internal async Task ResolveCommitAsync( + public async Task ResolveCommitAsync( GitSkillPluginSource source, CancellationToken cancellationToken) { diff --git a/src/Netclaw.Daemon/Services/ServerFeedSkillSyncService.cs b/src/Netclaw.Daemon/Services/ServerFeedSkillSyncService.cs index a60dc30a6..96215168f 100644 --- a/src/Netclaw.Daemon/Services/ServerFeedSkillSyncService.cs +++ b/src/Netclaw.Daemon/Services/ServerFeedSkillSyncService.cs @@ -37,6 +37,34 @@ internal sealed class ServerFeedSkillSyncService : IServerFeedSkillSyncRunner private readonly ISkillContentScanner _scanner; private readonly ILogger _logger; private readonly Func _clientFactory; + private readonly GitSkillPluginStateStore _pluginStateStore; + private readonly IGitSkillPluginAcquirer _pluginAcquirer; + private readonly IOperationalNotificationSink _notificationSink; + private bool _startupPluginCleanupComplete; + + public ServerFeedSkillSyncService( + SkillFeedsConfig feedsConfig, + NetclawPaths paths, + SkillInventoryRefresher inventoryRefresher, + TimeProvider timeProvider, + ISkillContentScanner scanner, + ILogger logger, + GitSkillPluginStateStore pluginStateStore, + IGitSkillPluginAcquirer pluginAcquirer, + IOperationalNotificationSink notificationSink) + : this( + feedsConfig, + paths, + inventoryRefresher, + timeProvider, + scanner, + logger, + CreateSkillServerClient, + pluginStateStore, + pluginAcquirer, + notificationSink) + { + } public ServerFeedSkillSyncService( SkillFeedsConfig feedsConfig, @@ -52,7 +80,10 @@ public ServerFeedSkillSyncService( timeProvider, scanner, logger, - CreateSkillServerClient) + CreateSkillServerClient, + new GitSkillPluginStateStore(paths, timeProvider), + new GitSkillPluginAcquirer(new HttpClient(), paths, timeProvider, scanner), + NullNotificationSink.Instance) { } @@ -100,7 +131,10 @@ internal ServerFeedSkillSyncService( timeProvider, scanner, logger, - clientFactory) + clientFactory, + new GitSkillPluginStateStore(paths, timeProvider), + new GitSkillPluginAcquirer(new HttpClient(), paths, timeProvider, scanner), + NullNotificationSink.Instance) { } @@ -111,7 +145,10 @@ private ServerFeedSkillSyncService( TimeProvider timeProvider, ISkillContentScanner scanner, ILogger logger, - Func clientFactory) + Func clientFactory, + GitSkillPluginStateStore pluginStateStore, + IGitSkillPluginAcquirer pluginAcquirer, + IOperationalNotificationSink notificationSink) { _feedsConfig = feedsConfig; _paths = paths; @@ -120,6 +157,9 @@ private ServerFeedSkillSyncService( _scanner = scanner; _logger = logger; _clientFactory = clientFactory; + _pluginStateStore = pluginStateStore; + _pluginAcquirer = pluginAcquirer; + _notificationSink = notificationSink; } /// @@ -158,9 +198,11 @@ private ServerFeedSkillSyncService( } } + var managedPluginSources = await SyncManagedGitPluginsAsync(sources, cancellationToken); + try { - var scan = RescanAndUpdateIndex(); + var scan = RescanAndUpdateIndex(managedPluginSources); var result = new SkillSyncResult.Response { PassId = passId, @@ -208,6 +250,223 @@ private ServerFeedSkillSyncService( } } + private async Task> SyncManagedGitPluginsAsync( + List rows, + CancellationToken cancellationToken) + { + var configuredSources = _feedsConfig.Plugins; + if (!GitSkillPluginSourceValidator.TryValidateSources(configuredSources, out var validationError)) + { + _logger.LogWarning("Managed Git plugin configuration is invalid: {Error}", validationError); + return ResolveManagedGitPluginSources( + await _pluginStateStore.LoadReceiptsAsync(cancellationToken)); + } + + await _pluginStateStore.RemoveSourcesExceptAsync( + configuredSources.Select(static source => source.Name).ToArray(), cancellationToken); + var receipts = await _pluginStateStore.LoadReceiptsAsync(cancellationToken); + + if (!_startupPluginCleanupComplete) + { + CleanupManagedGitPluginDirectories(receipts); + _startupPluginCleanupComplete = true; + } + + foreach (var source in configuredSources.Where(static source => source.Enabled)) + { + try + { + rows.Add(await SyncManagedGitPluginAsync(source, cancellationToken)); + } + catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) + { + throw; + } + catch (Exception ex) + { + _logger.LogWarning(ex, + "Managed Git plugin sync failed for '{PluginName}' — keeping the prior publication", + source.Name); + rows.Add(PluginFailure(source.Name)); + } + } + + receipts = await _pluginStateStore.LoadReceiptsAsync(cancellationToken); + return ResolveManagedGitPluginSources(receipts); + } + + private async Task SyncManagedGitPluginAsync( + GitSkillPluginSource source, + CancellationToken cancellationToken) + { + var sourceFingerprint = GitSkillPluginSourceValidator.Fingerprint(source); + var receipt = await _pluginStateStore.GetReceiptAsync(source.Name, cancellationToken); + var commit = source.ReferenceKind == GitSkillPluginReferenceKind.Commit + ? source.Reference.ToLowerInvariant() + : await _pluginAcquirer.ResolveCommitAsync(source, cancellationToken); + var installedDirectory = receipt is null + ? null + : _paths.ManagedGitSkillCommitDirectory( + receipt.SourceName, receipt.SourceFingerprint, receipt.InstalledCommit); + + if (receipt is not null + && string.Equals(receipt.SourceFingerprint, sourceFingerprint, StringComparison.Ordinal) + && Directory.Exists(installedDirectory) + && (string.Equals(receipt.InstalledCommit, commit, StringComparison.Ordinal) + || string.Equals(receipt.LastObservedCommit, commit, StringComparison.Ordinal))) + { + return PluginUnchanged(source.Name, receipt.InstalledCommit, receipt.InstalledVersion); + } + + var rejection = await _pluginStateStore.GetRejectionAsync( + source.Name, sourceFingerprint, commit, cancellationToken); + if (rejection is not null) + { + _logger.LogInformation( + "Managed Git plugin '{PluginName}' commit {Commit} remains rejected", + source.Name, commit); + return PluginRejected(source.Name, commit); + } + + try + { + var candidate = await _pluginAcquirer.AcquireAsync(source, commit, cancellationToken); + if (!Directory.Exists(candidate.Directory)) + throw new IOException("The immutable managed Git plugin candidate is missing."); + + if (receipt is not null + && string.Equals(receipt.SourceFingerprint, sourceFingerprint, StringComparison.Ordinal) + && Directory.Exists(installedDirectory) + && candidate.Version is not null + && string.Equals(candidate.Version, receipt.InstalledVersion, StringComparison.Ordinal)) + { + await _pluginStateStore.UpdateLastObservedCommitAsync( + source.Name, candidate.Commit, cancellationToken); + DeleteUnpublishedCandidate(candidate.Directory, installedDirectory); + return PluginUnchanged(source.Name, receipt.InstalledCommit, receipt.InstalledVersion); + } + + await _pluginStateStore.SaveReceiptAsync( + source, candidate.Commit, candidate.Version, cancellationToken); + return new SkillSyncResult.SourceRow + { + Name = source.Name, + ChangedCount = 1, + Sidecar = "not-applicable", + Commit = candidate.Commit, + Version = candidate.Version, + }; + } + catch (GitSkillPluginRejectedException rejectionException) + { + await _pluginStateStore.SaveRejectionAsync( + source.Name, + sourceFingerprint, + rejectionException.Commit, + rejectionException.Message, + rejectionException.SecurityRejection, + cancellationToken); + + if (rejectionException.SecurityRejection + && await _pluginStateStore.TryClaimSecurityAlertAsync( + source.Name, sourceFingerprint, rejectionException.Commit, cancellationToken)) + { + _notificationSink.Emit(OperationalAlert.Create( + _timeProvider, + "skill.plugin.security_rejected", + AlertType.SkillPluginSecurityRejected, + "A managed Git skill plugin failed a security check.", + AlertSeverity.Warning, + $"{sourceFingerprint}:{rejectionException.Commit}", + new Dictionary + { + ["source_fingerprint"] = sourceFingerprint, + ["commit"] = rejectionException.Commit, + })); + } + + _logger.LogWarning( + "Managed Git plugin '{PluginName}' commit {Commit} was rejected", + source.Name, rejectionException.Commit); + return PluginRejected(source.Name, rejectionException.Commit); + } + } + + private IReadOnlyList ResolveManagedGitPluginSources( + IReadOnlyList receipts) + => receipts + .Select(receipt => new + { + Receipt = receipt, + Directory = _paths.ManagedGitSkillCommitDirectory( + receipt.SourceName, receipt.SourceFingerprint, receipt.InstalledCommit), + }) + .Where(static candidate => Directory.Exists(candidate.Directory)) + .OrderBy(static candidate => candidate.Receipt.SourceName, StringComparer.Ordinal) + .Select(static candidate => new ResolvedExternalSource( + $"managed-git:{candidate.Receipt.SourceName}", + [candidate.Directory], + AllowSymlinks: false)) + .ToArray(); + + private void CleanupManagedGitPluginDirectories(IReadOnlyList receipts) + { + var selectedDirectories = receipts + .Select(receipt => Path.GetFullPath(_paths.ManagedGitSkillCommitDirectory( + receipt.SourceName, receipt.SourceFingerprint, receipt.InstalledCommit))) + .ToHashSet(StringComparer.Ordinal); + var root = _paths.ManagedGitSkillsDirectory; + if (!Directory.Exists(root)) + return; + + foreach (var stagingDirectory in Directory.EnumerateDirectories(root, ".staging", SearchOption.AllDirectories)) + Directory.Delete(stagingDirectory, recursive: true); + + foreach (var commitsDirectory in Directory.EnumerateDirectories(root, "commits", SearchOption.AllDirectories)) + { + foreach (var commitDirectory in Directory.EnumerateDirectories(commitsDirectory)) + { + if (!selectedDirectories.Contains(Path.GetFullPath(commitDirectory))) + Directory.Delete(commitDirectory, recursive: true); + } + } + } + + private void DeleteUnpublishedCandidate(string candidateDirectory, string installedDirectory) + { + if (!string.Equals(Path.GetFullPath(candidateDirectory), Path.GetFullPath(installedDirectory), StringComparison.Ordinal) + && Directory.Exists(candidateDirectory)) + { + Directory.Delete(candidateDirectory, recursive: true); + } + } + + private static SkillSyncResult.SourceRow PluginUnchanged(string name, string commit, string? version) => new() + { + Name = name, + UnchangedCount = 1, + Sidecar = "not-applicable", + Commit = commit, + Version = version, + }; + + private static SkillSyncResult.SourceRow PluginRejected(string name, string commit) => new() + { + Name = name, + RejectedCount = 1, + Sidecar = "not-applicable", + Commit = commit, + Error = "The plugin commit is rejected.", + }; + + private static SkillSyncResult.SourceRow PluginFailure(string name) => new() + { + Name = name, + FailedCount = 1, + Sidecar = "not-applicable", + Error = "The source sync failed. Existing files remain in use.", + }; + private async Task SyncFeedAsync(SkillFeedSource feed, CancellationToken cancellationToken) { var feedDir = _paths.ServerFeedDirectory(feed.Name); @@ -889,9 +1148,11 @@ private static bool IsZipSymlink(ZipArchiveEntry entry) return mode == 0 ? null : mode; } - private MergedSkillScanResult RescanAndUpdateIndex() + private MergedSkillScanResult RescanAndUpdateIndex( + IReadOnlyList managedGitPluginSources) { - var mergedResult = _inventoryRefresher.Refresh(); + var mergedResult = _inventoryRefresher.ReplaceManagedGitPluginSourcesAndRefresh( + managedGitPluginSources); if (mergedResult.Issues.Count > 0) { diff --git a/src/Netclaw.Daemon/Services/SkillDirectoryWatcherService.cs b/src/Netclaw.Daemon/Services/SkillDirectoryWatcherService.cs index f1f4c6e17..8f9577286 100644 --- a/src/Netclaw.Daemon/Services/SkillDirectoryWatcherService.cs +++ b/src/Netclaw.Daemon/Services/SkillDirectoryWatcherService.cs @@ -184,14 +184,25 @@ private void OnDebounceTimerFired() } } - private static bool ShouldIgnore(string fullPath) + private bool ShouldIgnore(string fullPath) { + if (IsWithin(fullPath, _paths.ManagedGitSkillsDirectory)) + return true; + // Ignore staging directories and temp files used by atomic write operations return fullPath.Contains($"{Path.DirectorySeparatorChar}.staging{Path.DirectorySeparatorChar}", StringComparison.Ordinal) || fullPath.Contains($"{Path.AltDirectorySeparatorChar}.staging{Path.AltDirectorySeparatorChar}", StringComparison.Ordinal) || fullPath.EndsWith(".tmp", StringComparison.OrdinalIgnoreCase); } + private static bool IsWithin(string path, string root) + { + var normalizedPath = Path.GetFullPath(path); + var normalizedRoot = Path.TrimEndingDirectorySeparator(Path.GetFullPath(root)) + + Path.DirectorySeparatorChar; + return normalizedPath.StartsWith(normalizedRoot, StringComparison.Ordinal); + } + public override void Dispose() { _debounceTimer?.Dispose(); From 4c01308b5a1c350667903d8b7769f72ae0b317e8 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Fri, 11 Sep 2026 20:45:29 -0500 Subject: [PATCH 2/6] Harden Git skill plugin sync --- ...killServerNativeSidecarIntegrationTests.cs | 17 +- .../GitSkillPluginSyncServiceTests.cs | 161 ++++++++++++++++++ .../ServerFeedSkillSyncResultTests.cs | 8 +- .../ServerFeedSkillSyncServiceTests.cs | 28 ++- ...ServerFeedSkillSyncToolIntegrationTests.cs | 13 +- ...killEndpointRouteBuilderExtensionsTests.cs | 24 ++- .../Services/ServerFeedSkillSyncService.cs | 66 ++----- 7 files changed, 258 insertions(+), 59 deletions(-) diff --git a/src/Netclaw.Daemon.IntegrationTests/SkillServerNativeSidecarIntegrationTests.cs b/src/Netclaw.Daemon.IntegrationTests/SkillServerNativeSidecarIntegrationTests.cs index 6ba1b377b..e4ce725fe 100644 --- a/src/Netclaw.Daemon.IntegrationTests/SkillServerNativeSidecarIntegrationTests.cs +++ b/src/Netclaw.Daemon.IntegrationTests/SkillServerNativeSidecarIntegrationTests.cs @@ -116,7 +116,15 @@ public async Task Syncs_skill_and_subagent_from_real_skillserver_container() TimeProvider.System, new NoOpSkillContentScanner(), NullLogger.Instance, - []); + [], + feed => new SkillServerClient(new HttpClient(new HttpClientHandler()) + { + BaseAddress = new Uri(feed.Url), + }), + CreatePluginStateStore(paths), + new GitSkillPluginAcquirer( + new HttpClient(new HttpClientHandler()), paths, TimeProvider.System, new NoOpSkillContentScanner()), + NullNotificationSink.Instance); await service.SyncAsync(CancellationToken.None); @@ -139,6 +147,13 @@ public async Task Syncs_skill_and_subagent_from_real_skillserver_container() Assert.NotNull(registry.TryGetByName("code-reviewer")); } + private static GitSkillPluginStateStore CreatePluginStateStore(NetclawPaths paths) + { + new SchemaMigrator(paths, NullLogger.Instance) + .MigrateAsync(paths.SqliteDbPath, CancellationToken.None).GetAwaiter().GetResult(); + return new GitSkillPluginStateStore(paths, TimeProvider.System); + } + private static async Task SeedSkillServerAsync() { using var client = new SkillServerClient(ServerUrl, ApiKey); diff --git a/src/Netclaw.Daemon.Tests/Services/GitSkillPluginSyncServiceTests.cs b/src/Netclaw.Daemon.Tests/Services/GitSkillPluginSyncServiceTests.cs index 81dc45ec7..6cca90bbd 100644 --- a/src/Netclaw.Daemon.Tests/Services/GitSkillPluginSyncServiceTests.cs +++ b/src/Netclaw.Daemon.Tests/Services/GitSkillPluginSyncServiceTests.cs @@ -185,6 +185,128 @@ public async Task Startup_cleanup_removes_staging_and_orphan_commits_but_keeps_r Assert.False(Directory.Exists(staging)); } + [Fact] + public async Task Disabled_source_does_not_publish_a_prior_receipt() + { + var source = Source(); + var store = await CreateStoreAsync(); + await store.SaveReceiptAsync(source, FirstCommit, "1.0.0", TestContext.Current.CancellationToken); + var directory = _paths.ManagedGitSkillCommitDirectory( + source.Name, GitSkillPluginSourceValidator.Fingerprint(source), FirstCommit); + Directory.CreateDirectory(Path.Combine(directory, "plugin-skill")); + File.WriteAllText(Path.Combine(directory, "plugin-skill", "SKILL.md"), SkillMarkdown("plugin-skill", "hidden")); + source.Enabled = false; + var acquirer = new FakeAcquirer(_paths, source, FirstCommit, "1.0.0", "plugin-skill"); + var registry = new SkillRegistry(); + var service = new ServerFeedSkillSyncService( + new SkillFeedsConfig { Plugins = [source] }, _paths, CreateRefresher(registry, static () => { }), _time, + new NoOpSkillContentScanner(), NullLogger.Instance, store, acquirer, + new RecordingSink()); + + await service.SyncAsync(TestContext.Current.CancellationToken); + + Assert.Equal(0, acquirer.ResolveCount); + Assert.Null(registry.GetByName("plugin-skill")); + Assert.True(Directory.Exists(directory)); + } + + [Fact] + public async Task Changed_source_acquisition_failure_keeps_the_prior_receipt_and_directory() + { + var original = Source(); + var store = await CreateStoreAsync(); + await store.SaveReceiptAsync(original, FirstCommit, "1.0.0", TestContext.Current.CancellationToken); + var oldDirectory = _paths.ManagedGitSkillCommitDirectory( + original.Name, GitSkillPluginSourceValidator.Fingerprint(original), FirstCommit); + Directory.CreateDirectory(oldDirectory); + var changed = Source(); + changed.Reference = "release"; + var acquirer = new FakeAcquirer(_paths, changed, SecondCommit, "2.0.0", "plugin-skill") + { + Failure = new IOException("network unavailable"), + }; + var service = new ServerFeedSkillSyncService( + new SkillFeedsConfig { Plugins = [changed] }, _paths, CreateRefresher(new SkillRegistry(), static () => { }), _time, + new NoOpSkillContentScanner(), NullLogger.Instance, store, acquirer, + new RecordingSink()); + + var result = await service.SyncAsync(TestContext.Current.CancellationToken); + + Assert.Equal(1, Assert.Single(result.Sources).FailedCount); + Assert.Equal(FirstCommit, (await store.GetReceiptAsync(original.Name, TestContext.Current.CancellationToken))!.InstalledCommit); + Assert.True(Directory.Exists(oldDirectory)); + } + + [Fact] + public async Task Scanner_service_failure_creates_no_rejection_or_alert() + { + var source = Source(); + var acquirer = new FakeAcquirer(_paths, source, FirstCommit, "1.0.0", "plugin-skill") + { + Failure = new GitSkillPluginScannerUnavailableException("scanner unavailable"), + }; + var alerts = new RecordingSink(); + var service = await CreateServiceAsync(source, CreateRefresher(new SkillRegistry(), static () => { }), acquirer, alerts); + + var result = await service.SyncAsync(TestContext.Current.CancellationToken); + var store = new GitSkillPluginStateStore(_paths, _time); + + Assert.Equal(1, Assert.Single(result.Sources).FailedCount); + Assert.Null(await store.GetRejectionAsync( + source.Name, GitSkillPluginSourceValidator.Fingerprint(source), FirstCommit, + TestContext.Current.CancellationToken)); + Assert.Empty(alerts.Alerts); + } + + [Fact] + public async Task Versionless_commit_change_publishes_the_new_commit() + { + var source = Source(); + var registry = new SkillRegistry(); + var refresher = CreateRefresher(registry, static () => { }); + await (await CreateServiceAsync( + source, refresher, new FakeAcquirer(_paths, source, FirstCommit, null, "plugin-skill"), new RecordingSink())) + .SyncAsync(TestContext.Current.CancellationToken); + var updated = new FakeAcquirer(_paths, source, SecondCommit, null, "plugin-skill", "new versionless content"); + await (await CreateServiceAsync(source, refresher, updated, new RecordingSink())) + .SyncAsync(TestContext.Current.CancellationToken); + + var receipt = await new GitSkillPluginStateStore(_paths, _time).GetReceiptAsync( + source.Name, TestContext.Current.CancellationToken); + Assert.NotNull(receipt); + Assert.Equal(SecondCommit, receipt.InstalledCommit); + Assert.Equal(1, updated.AcquireCount); + Assert.Contains("new versionless content", await File.ReadAllTextAsync( + registry.GetByName("plugin-skill")!.FilePath, TestContext.Current.CancellationToken)); + } + + [Fact] + public async Task One_failed_plugin_does_not_block_a_successful_plugin() + { + var failed = Source(); + failed.Name = "failed"; + var healthy = Source(); + healthy.Name = "healthy"; + var store = await CreateStoreAsync(); + var registry = new SkillRegistry(); + var service = new ServerFeedSkillSyncService( + new SkillFeedsConfig { Plugins = [failed, healthy] }, + _paths, + CreateRefresher(registry, static () => { }), + _time, + new NoOpSkillContentScanner(), + NullLogger.Instance, + store, + new TwoPluginAcquirer(_paths, failed, healthy), + new RecordingSink()); + + var result = await service.SyncAsync(TestContext.Current.CancellationToken); + + Assert.Equal(1, Assert.Single(result.Sources, row => row.Name == "failed").FailedCount); + Assert.Equal(1, Assert.Single(result.Sources, row => row.Name == "healthy").ChangedCount); + Assert.NotNull(registry.GetByName("healthy-skill")); + } + private async Task CreateStoreAsync() { await new SchemaMigrator(_paths, NullLogger.Instance) @@ -242,6 +364,7 @@ private sealed class FakeAcquirer( { public bool FailOnResolve { get; init; } public GitSkillPluginRejectedException? Rejection { get; init; } + public Exception? Failure { get; init; } public int ResolveCount { get; private set; } public int AcquireCount { get; private set; } @@ -266,6 +389,8 @@ public Task AcquireAsync( CancellationToken cancellationToken) { AcquireCount++; + if (Failure is not null) + throw Failure; if (Rejection is not null) throw Rejection; @@ -299,4 +424,40 @@ private sealed class RecordingSink : IOperationalNotificationSink public List Alerts { get; } = []; public void Emit(OperationalAlert alert) => Alerts.Add(alert); } + + private sealed class TwoPluginAcquirer(NetclawPaths paths, GitSkillPluginSource failed, GitSkillPluginSource healthy) + : IGitSkillPluginAcquirer + { + public Task AcquireAsync(GitSkillPluginSource source, CancellationToken cancellationToken) + => AcquireAsync(source, source.Name == failed.Name ? FirstCommit : SecondCommit, cancellationToken); + + public Task ResolveCommitAsync(GitSkillPluginSource source, CancellationToken cancellationToken) + => Task.FromResult(source.Name == failed.Name ? FirstCommit : SecondCommit); + + public Task AcquireAsync( + GitSkillPluginSource source, + string commit, + CancellationToken cancellationToken) + { + if (source.Name == failed.Name) + throw new IOException("network unavailable"); + + var directory = paths.ManagedGitSkillCommitDirectory( + healthy.Name, GitSkillPluginSourceValidator.Fingerprint(healthy), commit); + var skillDirectory = Path.Combine(directory, "healthy-skill"); + Directory.CreateDirectory(skillDirectory); + File.WriteAllText(Path.Combine(skillDirectory, "SKILL.md"), SkillMarkdown("healthy-skill", "healthy")); + return Task.FromResult(new GitSkillPluginCandidate( + commit, null, directory, SkillScanner.Scan(directory).AcceptedSkills, [])); + } + } + + private static string SkillMarkdown(string name, string description) => $$""" + --- + name: {{name}} + description: {{description}} + --- + + # {{name}} + """; } diff --git a/src/Netclaw.Daemon.Tests/Services/ServerFeedSkillSyncResultTests.cs b/src/Netclaw.Daemon.Tests/Services/ServerFeedSkillSyncResultTests.cs index 43fb42867..560aae0ea 100644 --- a/src/Netclaw.Daemon.Tests/Services/ServerFeedSkillSyncResultTests.cs +++ b/src/Netclaw.Daemon.Tests/Services/ServerFeedSkillSyncResultTests.cs @@ -29,6 +29,8 @@ public ServerFeedSkillSyncResultTests() { _paths = new NetclawPaths(_directory.Path); _paths.EnsureDirectoriesExist(); + new SchemaMigrator(_paths, NullLogger.Instance) + .MigrateAsync(_paths.SqliteDbPath, CancellationToken.None).GetAwaiter().GetResult(); } public void Dispose() => _directory.Dispose(); @@ -185,7 +187,11 @@ private ServerFeedSkillSyncService CreateService( scanner, NullLogger.Instance, [], - feed => new SkillServerClient(new HttpClient(handler) { BaseAddress = new Uri(feed.Url) })); + feed => new SkillServerClient(new HttpClient(handler) { BaseAddress = new Uri(feed.Url) }), + new GitSkillPluginStateStore(_paths, TimeProvider.System), + new GitSkillPluginAcquirer( + new HttpClient(new HttpClientHandler()), _paths, TimeProvider.System, new NoOpSkillContentScanner()), + NullNotificationSink.Instance); private static Task RunAsync(ServerFeedSkillSyncService service) => service.SyncAsync(TestContext.Current.CancellationToken); diff --git a/src/Netclaw.Daemon.Tests/Services/ServerFeedSkillSyncServiceTests.cs b/src/Netclaw.Daemon.Tests/Services/ServerFeedSkillSyncServiceTests.cs index 2efb4ceb8..8d0222351 100644 --- a/src/Netclaw.Daemon.Tests/Services/ServerFeedSkillSyncServiceTests.cs +++ b/src/Netclaw.Daemon.Tests/Services/ServerFeedSkillSyncServiceTests.cs @@ -32,6 +32,8 @@ public sealed class ServerFeedSkillSyncServiceTests : IDisposable private readonly SkillRegistry _skillRegistry = new(); private readonly SkillIndexContextLayer _skillIndexLayer = new(); private readonly SkillIndexPublisher _skillIndexPublisher; + private readonly GitSkillPluginStateStore _pluginStateStore; + private readonly IGitSkillPluginAcquirer _pluginAcquirer; public ServerFeedSkillSyncServiceTests() { @@ -41,6 +43,11 @@ public ServerFeedSkillSyncServiceTests() _skillRegistry, _skillIndexLayer, static (_, _) => true); + new SchemaMigrator(_paths, NullLogger.Instance) + .MigrateAsync(_paths.SqliteDbPath, CancellationToken.None).GetAwaiter().GetResult(); + _pluginStateStore = new GitSkillPluginStateStore(_paths, TimeProvider.System); + _pluginAcquirer = new GitSkillPluginAcquirer( + new HttpClient(new HttpClientHandler()), _paths, TimeProvider.System, new NoOpSkillContentScanner()); } public void Dispose() => _dir.Dispose(); @@ -409,7 +416,11 @@ private ServerFeedSkillSyncService CreateService(ISkillContentScanner? scanner = TimeProvider.System, scanner ?? new NoOpSkillContentScanner(), NullLogger.Instance, - []); + [], + _ => new SkillServerClient(new HttpClient(new HttpClientHandler())), + _pluginStateStore, + _pluginAcquirer, + NullNotificationSink.Instance); private static Task RunSyncAsync(ServerFeedSkillSyncService service) => service.SyncAsync(TestContext.Current.CancellationToken); @@ -440,7 +451,10 @@ private ServerFeedSkillSyncService CreateService(FakeHttpMessageHandler handler) if (feed.ApiKey is { Value: { Length: > 0 } apiKey }) client.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Bearer", apiKey); return new SkillServerClient(client); - }); + }, + _pluginStateStore, + _pluginAcquirer, + NullNotificationSink.Instance); } private ServerFeedSkillSyncService CreateControlledService( @@ -467,7 +481,10 @@ private ServerFeedSkillSyncService CreateControlledService( feed => new SkillServerClient(new HttpClient(handler) { BaseAddress = new Uri(feed.Url), - })); + }), + _pluginStateStore, + _pluginAcquirer, + NullNotificationSink.Instance); } private ServerFeedSkillSyncService CreateService( @@ -486,7 +503,10 @@ private ServerFeedSkillSyncService CreateService( feed => new SkillServerClient(new HttpClient(handler) { BaseAddress = new Uri(feed.Url), - })); + }), + _pluginStateStore, + _pluginAcquirer, + NullNotificationSink.Instance); private SkillSyncState ReadAgentSyncState() { diff --git a/src/Netclaw.Daemon.Tests/Services/ServerFeedSkillSyncToolIntegrationTests.cs b/src/Netclaw.Daemon.Tests/Services/ServerFeedSkillSyncToolIntegrationTests.cs index efcb7d8be..2bafc882e 100644 --- a/src/Netclaw.Daemon.Tests/Services/ServerFeedSkillSyncToolIntegrationTests.cs +++ b/src/Netclaw.Daemon.Tests/Services/ServerFeedSkillSyncToolIntegrationTests.cs @@ -95,7 +95,18 @@ private ServerFeedSkillSyncService CreateService(RevisionFeedHandler handler) feed => new SkillServerClient(new HttpClient(handler, disposeHandler: false) { BaseAddress = new Uri(feed.Url), - })); + }), + CreatePluginStateStore(), + new GitSkillPluginAcquirer( + new HttpClient(new HttpClientHandler()), _paths, TimeProvider.System, new NoOpSkillContentScanner()), + NullNotificationSink.Instance); + } + + private GitSkillPluginStateStore CreatePluginStateStore() + { + new SchemaMigrator(_paths, NullLogger.Instance) + .MigrateAsync(_paths.SqliteDbPath, CancellationToken.None).GetAwaiter().GetResult(); + return new GitSkillPluginStateStore(_paths, TimeProvider.System); } private sealed class RevisionFeedHandler : HttpMessageHandler diff --git a/src/Netclaw.Daemon.Tests/Skills/SkillEndpointRouteBuilderExtensionsTests.cs b/src/Netclaw.Daemon.Tests/Skills/SkillEndpointRouteBuilderExtensionsTests.cs index 9f1e9b219..a211e55e8 100644 --- a/src/Netclaw.Daemon.Tests/Skills/SkillEndpointRouteBuilderExtensionsTests.cs +++ b/src/Netclaw.Daemon.Tests/Skills/SkillEndpointRouteBuilderExtensionsTests.cs @@ -261,7 +261,10 @@ private static ServerFeedSkillSyncService CreateSyncService(SkillRegistry regist new SkillInventoryRefresher(paths, new SkillFeedsConfig(), [], registry, publisher), TimeProvider.System, new NoOpSkillContentScanner(), - Microsoft.Extensions.Logging.Abstractions.NullLogger.Instance); + Microsoft.Extensions.Logging.Abstractions.NullLogger.Instance, + CreatePluginStateStore(paths), + CreatePluginAcquirer(paths), + NullNotificationSink.Instance); } private static ServerFeedSkillSyncService CreateBlockingSyncService( @@ -287,9 +290,26 @@ private static ServerFeedSkillSyncService CreateBlockingSyncService( feed => new Netclaw.SkillClient.SkillServerClient(new HttpClient(handler) { BaseAddress = new Uri(feed.Url), - })); + }), + CreatePluginStateStore(paths), + CreatePluginAcquirer(paths), + NullNotificationSink.Instance); + } + + private static GitSkillPluginStateStore CreatePluginStateStore(NetclawPaths paths) + { + new SchemaMigrator(paths, Microsoft.Extensions.Logging.Abstractions.NullLogger.Instance) + .MigrateAsync(paths.SqliteDbPath, CancellationToken.None).GetAwaiter().GetResult(); + return new GitSkillPluginStateStore(paths, TimeProvider.System); } + private static IGitSkillPluginAcquirer CreatePluginAcquirer(NetclawPaths paths) + => new GitSkillPluginAcquirer( + new HttpClient(new HttpClientHandler()), + paths, + TimeProvider.System, + new NoOpSkillContentScanner()); + private sealed class BlockingFeedHandler : HttpMessageHandler { private const string SkillContent = "---\nname: route-proof\ndescription: Route proof.\n---\n\nRoute proof body.\n"; diff --git a/src/Netclaw.Daemon/Services/ServerFeedSkillSyncService.cs b/src/Netclaw.Daemon/Services/ServerFeedSkillSyncService.cs index 96215168f..e62d889e8 100644 --- a/src/Netclaw.Daemon/Services/ServerFeedSkillSyncService.cs +++ b/src/Netclaw.Daemon/Services/ServerFeedSkillSyncService.cs @@ -66,49 +66,6 @@ public ServerFeedSkillSyncService( { } - public ServerFeedSkillSyncService( - SkillFeedsConfig feedsConfig, - NetclawPaths paths, - SkillInventoryRefresher inventoryRefresher, - TimeProvider timeProvider, - ISkillContentScanner scanner, - ILogger logger) - : this( - feedsConfig, - paths, - inventoryRefresher, - timeProvider, - scanner, - logger, - CreateSkillServerClient, - new GitSkillPluginStateStore(paths, timeProvider), - new GitSkillPluginAcquirer(new HttpClient(), paths, timeProvider, scanner), - NullNotificationSink.Instance) - { - } - - internal ServerFeedSkillSyncService( - SkillFeedsConfig feedsConfig, - NetclawPaths paths, - SkillRegistry skillRegistry, - SkillIndexPublisher skillIndexPublisher, - TimeProvider timeProvider, - ISkillContentScanner scanner, - ILogger logger, - IReadOnlyList externalSources) - : this( - feedsConfig, - paths, - skillRegistry, - skillIndexPublisher, - timeProvider, - scanner, - logger, - externalSources, - CreateSkillServerClient) - { - } - internal ServerFeedSkillSyncService( SkillFeedsConfig feedsConfig, NetclawPaths paths, @@ -118,7 +75,10 @@ internal ServerFeedSkillSyncService( ISkillContentScanner scanner, ILogger logger, IReadOnlyList externalSources, - Func clientFactory) + Func clientFactory, + GitSkillPluginStateStore pluginStateStore, + IGitSkillPluginAcquirer pluginAcquirer, + IOperationalNotificationSink notificationSink) : this( feedsConfig, paths, @@ -132,9 +92,9 @@ internal ServerFeedSkillSyncService( scanner, logger, clientFactory, - new GitSkillPluginStateStore(paths, timeProvider), - new GitSkillPluginAcquirer(new HttpClient(), paths, timeProvider, scanner), - NullNotificationSink.Instance) + pluginStateStore, + pluginAcquirer, + notificationSink) { } @@ -395,6 +355,12 @@ await _pluginStateStore.SaveRejectionAsync( private IReadOnlyList ResolveManagedGitPluginSources( IReadOnlyList receipts) => receipts + .Where(receipt => _feedsConfig.Plugins.Any(source => source.Enabled + && string.Equals(source.Name, receipt.SourceName, StringComparison.Ordinal) + && string.Equals( + GitSkillPluginSourceValidator.Fingerprint(source), + receipt.SourceFingerprint, + StringComparison.Ordinal))) .Select(receipt => new { Receipt = receipt, @@ -420,14 +386,14 @@ private void CleanupManagedGitPluginDirectories(IReadOnlyList Date: Fri, 11 Sep 2026 20:49:19 -0500 Subject: [PATCH 3/6] Restrict Git plugin cleanup scope --- .../Services/GitSkillPluginAcquirerTests.cs | 19 ++++ .../GitSkillPluginSyncServiceTests.cs | 85 ++++++++++++++++- .../Services/GitSkillPluginAcquirer.cs | 4 +- .../Services/ServerFeedSkillSyncService.cs | 91 ++++++++++++------- 4 files changed, 164 insertions(+), 35 deletions(-) diff --git a/src/Netclaw.Daemon.Tests/Services/GitSkillPluginAcquirerTests.cs b/src/Netclaw.Daemon.Tests/Services/GitSkillPluginAcquirerTests.cs index f85f5716f..28e188700 100644 --- a/src/Netclaw.Daemon.Tests/Services/GitSkillPluginAcquirerTests.cs +++ b/src/Netclaw.Daemon.Tests/Services/GitSkillPluginAcquirerTests.cs @@ -408,6 +408,25 @@ public async Task Acquire_uses_a_fixed_commit_without_a_GitHub_commit_lookup() Assert.Single(handler.UserAgents); } + [Fact] + public async Task Acquire_accepts_a_64_character_fixed_commit() + { + var archive = CreateArchive( + ("repo/.codex-plugin/plugin.json", Manifest("1.0.0"), TarEntryType.RegularFile), + ("repo/skills/alpha/SKILL.md", Skill("alpha"), TarEntryType.RegularFile)); + var handler = new GitHubHandler(archive); + var commit = new string('a', 64); + var source = Source(); + source.ReferenceKind = GitSkillPluginReferenceKind.Commit; + source.Reference = commit; + + var candidate = await CreateAcquirer(handler).AcquireAsync( + source, commit, TestContext.Current.CancellationToken); + + Assert.Equal(commit, candidate.Commit); + Assert.Equal(0, handler.CommitRequestCount); + } + [Theory] [MemberData(nameof(CorruptArchives))] public async Task Acquire_rejects_malformed_archive_content(byte[] archive) diff --git a/src/Netclaw.Daemon.Tests/Services/GitSkillPluginSyncServiceTests.cs b/src/Netclaw.Daemon.Tests/Services/GitSkillPluginSyncServiceTests.cs index 6cca90bbd..f8bd78b1d 100644 --- a/src/Netclaw.Daemon.Tests/Services/GitSkillPluginSyncServiceTests.cs +++ b/src/Netclaw.Daemon.Tests/Services/GitSkillPluginSyncServiceTests.cs @@ -153,6 +153,46 @@ public async Task Security_rejection_persists_and_emits_one_claimed_alert() Assert.DoesNotContain("scanner result", alert.Summary); } + [Fact] + public async Task Restart_claims_and_emits_a_persisted_security_rejection_alert() + { + var source = Source(); + var store = await CreateStoreAsync(); + var fingerprint = GitSkillPluginSourceValidator.Fingerprint(source); + await store.SaveRejectionAsync( + source.Name, fingerprint, FirstCommit, "scanner result", true, TestContext.Current.CancellationToken); + var acquirer = new FakeAcquirer(_paths, source, FirstCommit, "1.0.0", "plugin-skill"); + var alerts = new RecordingSink(); + var service = new ServerFeedSkillSyncService( + new SkillFeedsConfig { Plugins = [source] }, _paths, CreateRefresher(new SkillRegistry(), static () => { }), _time, + new NoOpSkillContentScanner(), NullLogger.Instance, store, acquirer, alerts); + + await service.SyncAsync(TestContext.Current.CancellationToken); + + Assert.Equal(0, acquirer.AcquireCount); + Assert.Equal("skill.plugin.security_rejected", Assert.Single(alerts.Alerts).Type); + Assert.True((await store.GetRejectionAsync( + source.Name, fingerprint, FirstCommit, TestContext.Current.CancellationToken))!.AlertEmitted); + } + + [Fact] + public async Task Source_timeout_covers_branch_resolution_before_archive_acquisition() + { + var source = Source(); + source.TimeoutSeconds = 1; + var acquirer = new BlockingResolveAcquirer(); + var service = await CreateServiceAsync( + source, CreateRefresher(new SkillRegistry(), static () => { }), acquirer, new RecordingSink()); + + var sync = service.SyncAsync(TestContext.Current.CancellationToken); + await acquirer.ResolutionStarted.Task.WaitAsync(TestContext.Current.CancellationToken); + _time.Advance(TimeSpan.FromSeconds(1)); + var result = await sync; + + Assert.Equal(1, Assert.Single(result.Sources).FailedCount); + Assert.Equal(0, acquirer.AcquireCount); + } + [Fact] public async Task Startup_cleanup_removes_staging_and_orphan_commits_but_keeps_receipt_directory() { @@ -163,9 +203,15 @@ public async Task Startup_cleanup_removes_staging_and_orphan_commits_but_keeps_r var selected = _paths.ManagedGitSkillCommitDirectory(source.Name, fingerprint, FirstCommit); var orphan = _paths.ManagedGitSkillCommitDirectory(source.Name, fingerprint, SecondCommit); var staging = Path.Combine(_paths.ManagedGitSkillDirectory(source.Name), ".staging", "candidate"); + var publishedStagingResource = Path.Combine(selected, "plugin-skill", "resources", ".staging", "guide.md"); + var publishedCommitsResource = Path.Combine(selected, "plugin-skill", "resources", "commits", "guide.md"); Directory.CreateDirectory(selected); Directory.CreateDirectory(orphan); Directory.CreateDirectory(staging); + Directory.CreateDirectory(Path.GetDirectoryName(publishedStagingResource)!); + Directory.CreateDirectory(Path.GetDirectoryName(publishedCommitsResource)!); + File.WriteAllText(publishedStagingResource, "published staging resource"); + File.WriteAllText(publishedCommitsResource, "published commits resource"); var registry = new SkillRegistry(); var service = new ServerFeedSkillSyncService( new SkillFeedsConfig { Plugins = [source] }, @@ -183,6 +229,8 @@ public async Task Startup_cleanup_removes_staging_and_orphan_commits_but_keeps_r Assert.True(Directory.Exists(selected)); Assert.False(Directory.Exists(orphan)); Assert.False(Directory.Exists(staging)); + Assert.True(File.Exists(publishedStagingResource)); + Assert.True(File.Exists(publishedCommitsResource)); } [Fact] @@ -218,15 +266,18 @@ public async Task Changed_source_acquisition_failure_keeps_the_prior_receipt_and await store.SaveReceiptAsync(original, FirstCommit, "1.0.0", TestContext.Current.CancellationToken); var oldDirectory = _paths.ManagedGitSkillCommitDirectory( original.Name, GitSkillPluginSourceValidator.Fingerprint(original), FirstCommit); - Directory.CreateDirectory(oldDirectory); + var oldSkillPath = Path.Combine(oldDirectory, "plugin-skill", "SKILL.md"); + Directory.CreateDirectory(Path.GetDirectoryName(oldSkillPath)!); + File.WriteAllText(oldSkillPath, SkillMarkdown("plugin-skill", "prior package")); var changed = Source(); changed.Reference = "release"; var acquirer = new FakeAcquirer(_paths, changed, SecondCommit, "2.0.0", "plugin-skill") { Failure = new IOException("network unavailable"), }; + var registry = new SkillRegistry(); var service = new ServerFeedSkillSyncService( - new SkillFeedsConfig { Plugins = [changed] }, _paths, CreateRefresher(new SkillRegistry(), static () => { }), _time, + new SkillFeedsConfig { Plugins = [changed] }, _paths, CreateRefresher(registry, static () => { }), _time, new NoOpSkillContentScanner(), NullLogger.Instance, store, acquirer, new RecordingSink()); @@ -235,6 +286,8 @@ public async Task Changed_source_acquisition_failure_keeps_the_prior_receipt_and Assert.Equal(1, Assert.Single(result.Sources).FailedCount); Assert.Equal(FirstCommit, (await store.GetReceiptAsync(original.Name, TestContext.Current.CancellationToken))!.InstalledCommit); Assert.True(Directory.Exists(oldDirectory)); + Assert.Contains("prior package", await File.ReadAllTextAsync( + registry.GetByName("plugin-skill")!.FilePath, TestContext.Current.CancellationToken)); } [Fact] @@ -317,7 +370,7 @@ private async Task CreateStoreAsync() private async Task CreateServiceAsync( GitSkillPluginSource source, SkillInventoryRefresher refresher, - FakeAcquirer acquirer, + IGitSkillPluginAcquirer acquirer, RecordingSink sink) { var store = await CreateStoreAsync(); @@ -452,6 +505,32 @@ public Task AcquireAsync( } } + private sealed class BlockingResolveAcquirer : IGitSkillPluginAcquirer + { + private readonly TaskCompletionSource _never = new(TaskCreationOptions.RunContinuationsAsynchronously); + public TaskCompletionSource ResolutionStarted { get; } = new(TaskCreationOptions.RunContinuationsAsynchronously); + public int AcquireCount { get; private set; } + + public Task AcquireAsync(GitSkillPluginSource source, CancellationToken cancellationToken) + => throw new NotSupportedException(); + + public async Task ResolveCommitAsync(GitSkillPluginSource source, CancellationToken cancellationToken) + { + ResolutionStarted.TrySetResult(); + await _never.Task.WaitAsync(cancellationToken); + return ""; + } + + public Task AcquireAsync( + GitSkillPluginSource source, + string commit, + CancellationToken cancellationToken) + { + AcquireCount++; + throw new NotSupportedException(); + } + } + private static string SkillMarkdown(string name, string description) => $$""" --- name: {{name}} diff --git a/src/Netclaw.Daemon/Services/GitSkillPluginAcquirer.cs b/src/Netclaw.Daemon/Services/GitSkillPluginAcquirer.cs index 09c98f148..18ffcbc1e 100644 --- a/src/Netclaw.Daemon/Services/GitSkillPluginAcquirer.cs +++ b/src/Netclaw.Daemon/Services/GitSkillPluginAcquirer.cs @@ -152,7 +152,9 @@ public async Task AcquireAsync( { if (!GitSkillPluginSourceValidator.TryValidateSource(source, out var sourceError)) throw new InvalidOperationException(sourceError); - if (string.IsNullOrWhiteSpace(commit) || commit.Length != 40 || !commit.All(char.IsAsciiHexDigit)) + if (string.IsNullOrWhiteSpace(commit) + || commit.Length is not (40 or 64) + || !commit.All(char.IsAsciiHexDigit)) throw new InvalidDataException("The resolved commit identity is invalid."); commit = commit.ToLowerInvariant(); diff --git a/src/Netclaw.Daemon/Services/ServerFeedSkillSyncService.cs b/src/Netclaw.Daemon/Services/ServerFeedSkillSyncService.cs index e62d889e8..dd85b0fc7 100644 --- a/src/Netclaw.Daemon/Services/ServerFeedSkillSyncService.cs +++ b/src/Netclaw.Daemon/Services/ServerFeedSkillSyncService.cs @@ -259,11 +259,16 @@ await _pluginStateStore.RemoveSourcesExceptAsync( GitSkillPluginSource source, CancellationToken cancellationToken) { + using var timeout = new CancellationTokenSource( + TimeSpan.FromSeconds(source.TimeoutSeconds), _timeProvider); + using var sourceOperation = CancellationTokenSource.CreateLinkedTokenSource( + cancellationToken, timeout.Token); + var sourceToken = sourceOperation.Token; var sourceFingerprint = GitSkillPluginSourceValidator.Fingerprint(source); - var receipt = await _pluginStateStore.GetReceiptAsync(source.Name, cancellationToken); + var receipt = await _pluginStateStore.GetReceiptAsync(source.Name, sourceToken); var commit = source.ReferenceKind == GitSkillPluginReferenceKind.Commit ? source.Reference.ToLowerInvariant() - : await _pluginAcquirer.ResolveCommitAsync(source, cancellationToken); + : await _pluginAcquirer.ResolveCommitAsync(source, sourceToken); var installedDirectory = receipt is null ? null : _paths.ManagedGitSkillCommitDirectory( @@ -279,9 +284,17 @@ await _pluginStateStore.RemoveSourcesExceptAsync( } var rejection = await _pluginStateStore.GetRejectionAsync( - source.Name, sourceFingerprint, commit, cancellationToken); + source.Name, sourceFingerprint, commit, sourceToken); if (rejection is not null) { + if (rejection.SecurityRejection + && !rejection.AlertEmitted + && await _pluginStateStore.TryClaimSecurityAlertAsync( + source.Name, sourceFingerprint, commit, sourceToken)) + { + EmitSecurityRejectionAlert(sourceFingerprint, commit); + } + _logger.LogInformation( "Managed Git plugin '{PluginName}' commit {Commit} remains rejected", source.Name, commit); @@ -290,7 +303,7 @@ await _pluginStateStore.RemoveSourcesExceptAsync( try { - var candidate = await _pluginAcquirer.AcquireAsync(source, commit, cancellationToken); + var candidate = await _pluginAcquirer.AcquireAsync(source, commit, sourceToken); if (!Directory.Exists(candidate.Directory)) throw new IOException("The immutable managed Git plugin candidate is missing."); @@ -301,13 +314,13 @@ await _pluginStateStore.RemoveSourcesExceptAsync( && string.Equals(candidate.Version, receipt.InstalledVersion, StringComparison.Ordinal)) { await _pluginStateStore.UpdateLastObservedCommitAsync( - source.Name, candidate.Commit, cancellationToken); + source.Name, candidate.Commit, sourceToken); DeleteUnpublishedCandidate(candidate.Directory, installedDirectory); return PluginUnchanged(source.Name, receipt.InstalledCommit, receipt.InstalledVersion); } await _pluginStateStore.SaveReceiptAsync( - source, candidate.Commit, candidate.Version, cancellationToken); + source, candidate.Commit, candidate.Version, sourceToken); return new SkillSyncResult.SourceRow { Name = source.Name, @@ -325,24 +338,13 @@ await _pluginStateStore.SaveRejectionAsync( rejectionException.Commit, rejectionException.Message, rejectionException.SecurityRejection, - cancellationToken); + sourceToken); if (rejectionException.SecurityRejection && await _pluginStateStore.TryClaimSecurityAlertAsync( - source.Name, sourceFingerprint, rejectionException.Commit, cancellationToken)) + source.Name, sourceFingerprint, rejectionException.Commit, sourceToken)) { - _notificationSink.Emit(OperationalAlert.Create( - _timeProvider, - "skill.plugin.security_rejected", - AlertType.SkillPluginSecurityRejected, - "A managed Git skill plugin failed a security check.", - AlertSeverity.Warning, - $"{sourceFingerprint}:{rejectionException.Commit}", - new Dictionary - { - ["source_fingerprint"] = sourceFingerprint, - ["commit"] = rejectionException.Commit, - })); + EmitSecurityRejectionAlert(sourceFingerprint, rejectionException.Commit); } _logger.LogWarning( @@ -356,11 +358,7 @@ private IReadOnlyList ResolveManagedGitPluginSources( IReadOnlyList receipts) => receipts .Where(receipt => _feedsConfig.Plugins.Any(source => source.Enabled - && string.Equals(source.Name, receipt.SourceName, StringComparison.Ordinal) - && string.Equals( - GitSkillPluginSourceValidator.Fingerprint(source), - receipt.SourceFingerprint, - StringComparison.Ordinal))) + && string.Equals(source.Name, receipt.SourceName, StringComparison.Ordinal))) .Select(receipt => new { Receipt = receipt, @@ -375,6 +373,22 @@ private IReadOnlyList ResolveManagedGitPluginSources( AllowSymlinks: false)) .ToArray(); + private void EmitSecurityRejectionAlert(string sourceFingerprint, string commit) + { + _notificationSink.Emit(OperationalAlert.Create( + _timeProvider, + "skill.plugin.security_rejected", + AlertType.SkillPluginSecurityRejected, + "A managed Git skill plugin failed a security check.", + AlertSeverity.Warning, + $"{sourceFingerprint}:{commit}", + new Dictionary + { + ["source_fingerprint"] = sourceFingerprint, + ["commit"] = commit, + })); + } + private void CleanupManagedGitPluginDirectories(IReadOnlyList receipts) { var selectedDirectories = receipts @@ -385,15 +399,30 @@ private void CleanupManagedGitPluginDirectories(IReadOnlyList Date: Fri, 11 Sep 2026 20:51:27 -0500 Subject: [PATCH 4/6] Fix Git plugin Windows cleanup --- .../Services/GitSkillPluginSyncServiceTests.cs | 6 +++++- .../Services/SkillDirectoryWatcherService.cs | 11 ++--------- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/src/Netclaw.Daemon.Tests/Services/GitSkillPluginSyncServiceTests.cs b/src/Netclaw.Daemon.Tests/Services/GitSkillPluginSyncServiceTests.cs index f8bd78b1d..e83583234 100644 --- a/src/Netclaw.Daemon.Tests/Services/GitSkillPluginSyncServiceTests.cs +++ b/src/Netclaw.Daemon.Tests/Services/GitSkillPluginSyncServiceTests.cs @@ -28,7 +28,11 @@ public GitSkillPluginSyncServiceTests() _paths.EnsureDirectoriesExist(); } - public void Dispose() => _temp.Dispose(); + public void Dispose() + { + SqliteTestPools.Clear(_paths); + _temp.Dispose(); + } [Fact] public async Task First_install_publishes_all_skills_in_one_inventory_snapshot() diff --git a/src/Netclaw.Daemon/Services/SkillDirectoryWatcherService.cs b/src/Netclaw.Daemon/Services/SkillDirectoryWatcherService.cs index 8f9577286..51f35e2ac 100644 --- a/src/Netclaw.Daemon/Services/SkillDirectoryWatcherService.cs +++ b/src/Netclaw.Daemon/Services/SkillDirectoryWatcherService.cs @@ -7,6 +7,7 @@ using Microsoft.Extensions.Logging; using Netclaw.Actors.Skills; using Netclaw.Configuration; +using Netclaw.Security; namespace Netclaw.Daemon.Services; @@ -186,7 +187,7 @@ private void OnDebounceTimerFired() private bool ShouldIgnore(string fullPath) { - if (IsWithin(fullPath, _paths.ManagedGitSkillsDirectory)) + if (PathUtility.IsWithinRoot(fullPath, _paths.ManagedGitSkillsDirectory)) return true; // Ignore staging directories and temp files used by atomic write operations @@ -195,14 +196,6 @@ private bool ShouldIgnore(string fullPath) || fullPath.EndsWith(".tmp", StringComparison.OrdinalIgnoreCase); } - private static bool IsWithin(string path, string root) - { - var normalizedPath = Path.GetFullPath(path); - var normalizedRoot = Path.TrimEndingDirectorySeparator(Path.GetFullPath(root)) - + Path.DirectorySeparatorChar; - return normalizedPath.StartsWith(normalizedRoot, StringComparison.Ordinal); - } - public override void Dispose() { _debounceTimer?.Dispose(); From c6678432729c151b4a247e40983ac4e3e44f1f98 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Fri, 11 Sep 2026 20:54:44 -0500 Subject: [PATCH 5/6] Test Git receipt publication order --- .../GitSkillPluginSyncServiceTests.cs | 76 +++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/src/Netclaw.Daemon.Tests/Services/GitSkillPluginSyncServiceTests.cs b/src/Netclaw.Daemon.Tests/Services/GitSkillPluginSyncServiceTests.cs index e83583234..f49d856e1 100644 --- a/src/Netclaw.Daemon.Tests/Services/GitSkillPluginSyncServiceTests.cs +++ b/src/Netclaw.Daemon.Tests/Services/GitSkillPluginSyncServiceTests.cs @@ -3,6 +3,7 @@ // Copyright (C) 2026 - 2026 Petabridge, LLC // // ----------------------------------------------------------------------- +using Microsoft.Data.Sqlite; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Time.Testing; using Netclaw.Actors.Skills; @@ -72,6 +73,63 @@ public async Task Branch_update_keeps_prior_directory_and_publishes_new_files() registry.GetByName("plugin-skill")!.FilePath, TestContext.Current.CancellationToken)); } + [Fact] + public async Task Interleaved_refresh_keeps_the_prior_snapshot_until_receipt_publication() + { + var source = Source(); + var registry = new SkillRegistry(); + var refresher = CreateRefresher(registry, static () => { }); + var first = new FakeAcquirer(_paths, source, FirstCommit, "1.0.0", "plugin-skill", "old text"); + await (await CreateServiceAsync(source, refresher, first, new RecordingSink())) + .SyncAsync(TestContext.Current.CancellationToken); + var oldPath = registry.GetByName("plugin-skill")!.FilePath; + string? pathDuringInterleave = null; + string? contentDuringInterleave = null; + + var second = new FakeAcquirer(_paths, source, SecondCommit, "2.0.0", "plugin-skill", "new text") + { + BeforeReturn = () => + { + refresher.Refresh(); + pathDuringInterleave = registry.GetByName("plugin-skill")!.FilePath; + contentDuringInterleave = File.ReadAllText(pathDuringInterleave); + }, + }; + + await (await CreateServiceAsync(source, refresher, second, new RecordingSink())) + .SyncAsync(TestContext.Current.CancellationToken); + + Assert.Equal(oldPath, pathDuringInterleave); + Assert.Contains("old text", contentDuringInterleave); + Assert.Contains("new text", await File.ReadAllTextAsync( + registry.GetByName("plugin-skill")!.FilePath, TestContext.Current.CancellationToken)); + } + + [Fact] + public async Task Receipt_persistence_failure_keeps_the_prior_receipt_and_registry() + { + var source = Source(); + var registry = new SkillRegistry(); + var refresher = CreateRefresher(registry, static () => { }); + var first = new FakeAcquirer(_paths, source, FirstCommit, "1.0.0", "plugin-skill", "old text"); + await (await CreateServiceAsync(source, refresher, first, new RecordingSink())) + .SyncAsync(TestContext.Current.CancellationToken); + var oldPath = registry.GetByName("plugin-skill")!.FilePath; + + await AddReceiptFailureTriggerAsync(); + var second = new FakeAcquirer(_paths, source, SecondCommit, "2.0.0", "plugin-skill", "new text"); + var result = await (await CreateServiceAsync(source, refresher, second, new RecordingSink())) + .SyncAsync(TestContext.Current.CancellationToken); + + Assert.Equal(1, Assert.Single(result.Sources).FailedCount); + var receipt = await new GitSkillPluginStateStore(_paths, _time).GetReceiptAsync( + source.Name, TestContext.Current.CancellationToken); + Assert.NotNull(receipt); + Assert.Equal(FirstCommit, receipt.InstalledCommit); + Assert.Equal(oldPath, registry.GetByName("plugin-skill")!.FilePath); + Assert.Contains("old text", await File.ReadAllTextAsync(oldPath, TestContext.Current.CancellationToken)); + } + [Fact] public async Task Equal_version_records_last_observed_commit_without_a_second_fetch() { @@ -371,6 +429,22 @@ private async Task CreateStoreAsync() return new GitSkillPluginStateStore(_paths, _time); } + private async Task AddReceiptFailureTriggerAsync() + { + await using var connection = new SqliteConnection($"Data Source={_paths.SqliteDbPath}"); + await connection.OpenAsync(TestContext.Current.CancellationToken); + await using var command = connection.CreateCommand(); + command.CommandText = + """ + CREATE TRIGGER reject_git_skill_plugin_receipt_update + BEFORE UPDATE ON git_skill_plugin_receipts + BEGIN + SELECT RAISE(ABORT, 'receipt persistence failed'); + END; + """; + await command.ExecuteNonQueryAsync(TestContext.Current.CancellationToken); + } + private async Task CreateServiceAsync( GitSkillPluginSource source, SkillInventoryRefresher refresher, @@ -422,6 +496,7 @@ private sealed class FakeAcquirer( public bool FailOnResolve { get; init; } public GitSkillPluginRejectedException? Rejection { get; init; } public Exception? Failure { get; init; } + public Action? BeforeReturn { get; init; } public int ResolveCount { get; private set; } public int AcquireCount { get; private set; } @@ -467,6 +542,7 @@ public Task AcquireAsync( # {{skillName}} """); + BeforeReturn?.Invoke(); return Task.FromResult(new GitSkillPluginCandidate( resolvedCommit, version, From fa4705925e4c97f85219000e44510933086e493d Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Fri, 11 Sep 2026 21:03:04 -0500 Subject: [PATCH 6/6] Isolate Git plugin cleanup failures --- .../Services/ServerFeedSkillSyncService.cs | 70 ++++++++++++++----- 1 file changed, 52 insertions(+), 18 deletions(-) diff --git a/src/Netclaw.Daemon/Services/ServerFeedSkillSyncService.cs b/src/Netclaw.Daemon/Services/ServerFeedSkillSyncService.cs index dd85b0fc7..88cd9cbaa 100644 --- a/src/Netclaw.Daemon/Services/ServerFeedSkillSyncService.cs +++ b/src/Netclaw.Daemon/Services/ServerFeedSkillSyncService.cs @@ -399,32 +399,66 @@ private void CleanupManagedGitPluginDirectories(IReadOnlyList selectedDirectories) + { + DeleteManagedGitPluginDirectory(Path.Combine(sourceDirectory, ".staging")); + + foreach (var fingerprintDirectory in Directory.EnumerateDirectories(sourceDirectory)) + { + if (string.Equals( + Path.GetFileName(fingerprintDirectory), + ".staging", + StringComparison.Ordinal)) + { + continue; + } + + var commitsDirectory = Path.Combine(fingerprintDirectory, "commits"); + if (!Directory.Exists(commitsDirectory)) + continue; + + foreach (var commitDirectory in Directory.EnumerateDirectories(commitsDirectory)) + { + if (!selectedDirectories.Contains(Path.GetFullPath(commitDirectory))) + DeleteManagedGitPluginDirectory(commitDirectory); + } + } + } + + private void DeleteManagedGitPluginDirectory(string directory) + { + try + { + GitSkillPluginAcquirer.DeleteDirectory(directory); + } + catch (Exception ex) + { + _logger.LogWarning(ex, "Managed Git plugin cleanup failed for directory {Directory}", directory); + } } private void DeleteUnpublishedCandidate(string candidateDirectory, string installedDirectory)