From ea0bcb15d251df5e7555001dee6be016c573be01 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Thu, 2 Jul 2026 12:49:08 +0000 Subject: [PATCH 1/2] Allow synced skill resources to run via shell --- .../.system/files/skill-authoring/SKILL.md | 20 +- .../Skills/SkillScannerTests.cs | 4 +- .../Tools/SkillToolTests.cs | 48 ++- src/Netclaw.Actors/Skills/SkillScanner.cs | 60 ++-- src/Netclaw.Actors/Tools/SkillManageTool.cs | 73 +++-- .../Tools/SkillReadResourceTool.cs | 73 +++-- .../ServerFeedSkillSyncServiceTests.cs | 122 +++++++ src/Netclaw.Daemon/Program.cs | 2 - .../Services/ServerFeedSkillSyncService.cs | 298 +++++++++++++++--- .../Services/SkillSyncHelpers.cs | 37 ++- .../ToolPathPolicyTests.cs | 12 +- 11 files changed, 582 insertions(+), 167 deletions(-) create mode 100644 src/Netclaw.Daemon.Tests/Services/ServerFeedSkillSyncServiceTests.cs diff --git a/feeds/skills/.system/files/skill-authoring/SKILL.md b/feeds/skills/.system/files/skill-authoring/SKILL.md index a243827dc..3e2d4fdea 100644 --- a/feeds/skills/.system/files/skill-authoring/SKILL.md +++ b/feeds/skills/.system/files/skill-authoring/SKILL.md @@ -3,7 +3,7 @@ name: skill-authoring description: "How to create, edit, and manage Netclaw skills. Read this when you need to synthesize a new skill from a session, understand the skill file format, or use the skill_manage tool." metadata: author: netclaw - version: "1.7.1" + version: "1.7.2" --- # Skill Authoring @@ -49,6 +49,7 @@ skill-name/ references/ # Optional: detail documents loaded on demand scripts/ # Optional: executable helpers assets/ # Optional: templates, static resources + tools/ # Optional: any additional files/directories are allowed ``` ### Flat-file layout @@ -59,8 +60,8 @@ skill-name.md # YAML frontmatter + markdown instructions (no resources) Flat `.md` files with valid YAML frontmatter are accepted as skills for compatibility with Claude Code and other platforms. Flat-file skills cannot -have resource subdirectories. If both `skill-name/SKILL.md` and -`skill-name.md` exist, the directory version takes precedence. +have resources. If both `skill-name/SKILL.md` and `skill-name.md` exist, the +directory version takes precedence. Name matching depends on the source: - Netclaw-managed local skills use strict identity checks. The frontmatter @@ -169,13 +170,16 @@ Keep under 5000 tokens. Include: ## Progressive Disclosure -Put detail in subdirectories, not in the main SKILL.md: +Put detail in additional files, not in the main SKILL.md. Netclaw discovers any +safe relative file under the skill directory as a resource; `references/`, +`scripts/`, and `assets/` are conventions, not hard requirements: | Directory | Purpose | |-----------|---------| | `references/` | Detailed documentation, research, examples | | `scripts/` | Executable helpers (shell scripts, Python) | | `assets/` | Templates, static files, config samples | +| `tools/` or other paths | Additional helper files allowed by AgentSkills.io | The skill body references these files explicitly: "See `references/deployment-checklist.md` for the full checklist." The agent loads @@ -211,7 +215,7 @@ Content scanning rules: - All skills are scanned uniformly regardless of origin — system, user, and all other skills use the same policy (High risk → Reject, Medium → Warn, Low → Allow). - Rejected scans fail closed: Netclaw returns the rejection reason and leaves the previous on-disk content unchanged. - Warnings may still allow a mutation or read, but the warning text is surfaced in the tool result. -- Resource files must stay under `references/`, `scripts/`, or `assets/`; other paths are rejected. +- Resource paths must be safe relative file paths under the skill directory. Absolute paths, empty segments, `.`, `..`, and root `SKILL.md` are rejected. Hard rules: - The frontmatter `name` must match the target skill name for `create` and `edit`. @@ -228,10 +232,12 @@ Skills live in two locations: | Directory | Source | Editable | |-----------|--------|----------| | `~/.netclaw/skills/.system/` | Official Netclaw feed (synced from CDN) | No — read-only | +| `~/.netclaw/skills/.server-feeds//` | Private skill-server feeds | No — read-only | | `~/.netclaw/skills/` (root) | Operator-placed or user-created via `skill_manage` | Yes | -System skills (`.system/`) cannot be edited, patched, or deleted via -`skill_manage`. They are maintained by the Netclaw release process. +System skills (`.system/`) and private server-feed skills (`.server-feeds/`) +cannot be edited, patched, or deleted via `skill_manage`. They are maintained +by their sync services. All skills — regardless of origin — are visible in the skill index and available to all sessions. The skill index is a compressed file listing diff --git a/src/Netclaw.Actors.Tests/Skills/SkillScannerTests.cs b/src/Netclaw.Actors.Tests/Skills/SkillScannerTests.cs index f88200158..b6440bcf9 100644 --- a/src/Netclaw.Actors.Tests/Skills/SkillScannerTests.cs +++ b/src/Netclaw.Actors.Tests/Skills/SkillScannerTests.cs @@ -136,6 +136,7 @@ public void Discovers_directory_based_skill_with_resources() WriteSkillFile("web-search", "references/flight-pricing.md", "# Flight Pricing"); WriteSkillFile("web-search", "references/restaurant-search.md", "# Restaurant Search"); WriteSkillFile("web-search", "scripts/validate.sh", "#!/bin/bash\necho ok"); + WriteSkillFile("web-search", "tools/check", "#!/bin/bash\necho check"); var result = SkillScanner.Scan(_skillsDir); @@ -145,10 +146,11 @@ public void Discovers_directory_based_skill_with_resources() Assert.Equal(Path.Combine(_skillsDir, "web-search", "SKILL.md"), result.AcceptedSkills[0].FilePath); Assert.Equal(Path.Combine(_skillsDir, "web-search"), result.AcceptedSkills[0].SkillDirectory); Assert.NotNull(result.AcceptedSkills[0].ResourcePaths); - Assert.Equal(3, result.AcceptedSkills[0].ResourcePaths!.Count); + Assert.Equal(4, result.AcceptedSkills[0].ResourcePaths!.Count); Assert.Contains("references/flight-pricing.md", result.AcceptedSkills[0].ResourcePaths!); Assert.Contains("references/restaurant-search.md", result.AcceptedSkills[0].ResourcePaths!); Assert.Contains("scripts/validate.sh", result.AcceptedSkills[0].ResourcePaths!); + Assert.Contains("tools/check", result.AcceptedSkills[0].ResourcePaths!); } [Fact] diff --git a/src/Netclaw.Actors.Tests/Tools/SkillToolTests.cs b/src/Netclaw.Actors.Tests/Tools/SkillToolTests.cs index e27b3ef4c..ca81c0945 100644 --- a/src/Netclaw.Actors.Tests/Tools/SkillToolTests.cs +++ b/src/Netclaw.Actors.Tests/Tools/SkillToolTests.cs @@ -336,6 +336,25 @@ public async Task SkillReadResource_ReadsValidPath() Assert.Equal("# Guide Content", result); } + [Fact] + public async Task SkillReadResource_ReadsArbitraryAdditionalFilePath() + { + WriteSkill("my-skill", """ + --- + name: my-skill + description: Test skill. + --- + # My Skill + """); + WriteFile("my-skill", "tools/check", "#!/bin/bash\necho ok"); + ScanSkills(); + + var tool = new SkillReadResourceTool(_registry, new NoOpSkillContentScanner()); + var result = await tool.ExecuteAsync(ToolInput.Create("SkillName", "my-skill", "ResourcePath", "tools/check"), PersonalCtx, TestContext.Current.CancellationToken); + + Assert.Contains("echo ok", result); + } + [Fact] public async Task SkillReadResource_RejectsPathTraversal() { @@ -351,7 +370,7 @@ public async Task SkillReadResource_RejectsPathTraversal() var tool = new SkillReadResourceTool(_registry, new NoOpSkillContentScanner()); var result = await tool.ExecuteAsync(ToolInput.Create("SkillName", "my-skill", "ResourcePath", "../../etc/passwd"), PersonalCtx, TestContext.Current.CancellationToken); - Assert.Contains("not allowed", result); + Assert.Contains("cannot contain", result); } [Fact] @@ -387,7 +406,7 @@ public async Task SkillReadResource_RejectsDisallowedPrefix() var tool = new SkillReadResourceTool(_registry, new NoOpSkillContentScanner()); var result = await tool.ExecuteAsync(ToolInput.Create("SkillName", "my-skill", "ResourcePath", "SKILL.md"), PersonalCtx, TestContext.Current.CancellationToken); - Assert.Contains("must start with", result); + Assert.Contains("Use skill_load", result); } [Fact] @@ -495,7 +514,25 @@ Original content here. } [Fact] - public async Task SkillManage_WriteFile_ValidatesPath() + public async Task SkillManage_WriteFile_RejectsTraversalPath() + { + WriteSkill("wf-test", """ + --- + name: wf-test + description: Write file test. + --- + # WF + """); + ScanSkills(); + + var tool = CreateManageTool(); + var result = await tool.ExecuteAsync(ToolInput.Create("Action", "write_file", "Name", "wf-test", "FilePath", "../file.md", "FileContent", "content"), PersonalCtx, TestContext.Current.CancellationToken); + + Assert.Contains("cannot contain", result); + } + + [Fact] + public async Task SkillManage_WriteFile_AllowsArbitraryAdditionalFilePath() { WriteSkill("wf-test", """ --- @@ -507,9 +544,10 @@ public async Task SkillManage_WriteFile_ValidatesPath() ScanSkills(); var tool = CreateManageTool(); - var result = await tool.ExecuteAsync(ToolInput.Create("Action", "write_file", "Name", "wf-test", "FilePath", "baddir/file.md", "FileContent", "content"), PersonalCtx, TestContext.Current.CancellationToken); + var result = await tool.ExecuteAsync(ToolInput.Create("Action", "write_file", "Name", "wf-test", "FilePath", "tools/check", "FileContent", "#!/bin/bash\necho ok"), PersonalCtx, TestContext.Current.CancellationToken); - Assert.Contains("must start with", result); + Assert.Contains("File written: tools/check", result); + Assert.True(File.Exists(Path.Combine(_paths.SkillsDirectory, "wf-test", "tools", "check"))); } [Fact] diff --git a/src/Netclaw.Actors/Skills/SkillScanner.cs b/src/Netclaw.Actors/Skills/SkillScanner.cs index ff449c464..0b78b456e 100644 --- a/src/Netclaw.Actors/Skills/SkillScanner.cs +++ b/src/Netclaw.Actors/Skills/SkillScanner.cs @@ -14,8 +14,8 @@ namespace Netclaw.Actors.Skills; /// /// Discovers skills under the skills directory using the AgentSkills.io /// directory layout: each skill is a directory containing SKILL.md -/// with YAML frontmatter. Optional subdirectories (scripts/, -/// references/, assets/) hold progressive-disclosure resources. +/// with YAML frontmatter. Additional files under the skill directory are +/// progressive-disclosure resources. /// public static partial class SkillScanner { @@ -28,11 +28,6 @@ public static partial class SkillScanner /// public const string SystemCategory = ".system"; - /// - /// Standard subdirectories within a skill directory that contain resources. - /// - private static readonly string[] ResourceSubdirectories = ["scripts", "references", "assets"]; - [GeneratedRegex(@"^#\s+(.+)$", RegexOptions.Multiline)] private static partial Regex HeadingRegex(); @@ -571,7 +566,7 @@ private static (bool HasSubagentMetadata, string? Subagent, string? Error) Parse } /// - /// Enumerates resource files in standard subdirectories of a skill directory. + /// Enumerates non-root resource files under a skill directory. /// Returns null if no resources are found. /// private static IReadOnlyList? EnumerateResources(string skillDirectory, string rootDirectory, List issues, bool allowSymlinks = false) @@ -579,39 +574,32 @@ private static (bool HasSubagentMetadata, string? Subagent, string? Error) Parse List? resources = null; var canonicalRoot = PathUtility.Normalize(rootDirectory); - foreach (var subDirName in ResourceSubdirectories) + string[] files; + try { - var subDir = Path.Combine(skillDirectory, subDirName); - if (!Directory.Exists(subDir)) - continue; + files = Directory.GetFiles(skillDirectory, "*", SearchOption.AllDirectories); + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) + { + issues.Add(new SkillScanIssue( + Path: skillDirectory, + Kind: SkillScanIssueKind.ResourceEnumerationFailed, + Message: $"Failed to enumerate resources: {ex.Message}")); + return null; + } - if (ValidateCanonicalPath(subDir, canonicalRoot, issues, $"resource directory '{subDirName}'", allowSymlinks) is null) - return null; + foreach (var file in files.OrderBy(static f => f, StringComparer.Ordinal)) + { + if (PathUtility.AreEquivalentPaths(file, Path.Combine(skillDirectory, SkillFileName))) + continue; - string[] files; - try - { - files = Directory.GetFiles(subDir, "*", SearchOption.AllDirectories); - } - catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) - { - issues.Add(new SkillScanIssue( - Path: subDir, - Kind: SkillScanIssueKind.ResourceEnumerationFailed, - Message: $"Failed to enumerate resources: {ex.Message}")); + if (ValidateCanonicalPath(file, canonicalRoot, issues, "resource file", allowSymlinks) is null) return null; - } - foreach (var file in files) - { - if (ValidateCanonicalPath(file, canonicalRoot, issues, "resource file", allowSymlinks) is null) - return null; - - var relativePath = Path.GetRelativePath(skillDirectory, file) - .Replace(Path.DirectorySeparatorChar, '/'); - resources ??= []; - resources.Add(relativePath); - } + var relativePath = Path.GetRelativePath(skillDirectory, file) + .Replace(Path.DirectorySeparatorChar, '/'); + resources ??= []; + resources.Add(relativePath); } return resources; diff --git a/src/Netclaw.Actors/Tools/SkillManageTool.cs b/src/Netclaw.Actors/Tools/SkillManageTool.cs index 3e220dfee..7e9cdfa7e 100644 --- a/src/Netclaw.Actors/Tools/SkillManageTool.cs +++ b/src/Netclaw.Actors/Tools/SkillManageTool.cs @@ -24,9 +24,6 @@ namespace Netclaw.Actors.Tools; Grant = "builtin")] public sealed partial class SkillManageTool : NetclawTool { - private static readonly HashSet AllowedResourcePrefixes = - new(StringComparer.OrdinalIgnoreCase) { "references", "scripts", "assets" }; - [GeneratedRegex(@"^[a-z0-9]([a-z0-9-]*[a-z0-9])?$")] private static partial Regex ValidNameRegex(); @@ -212,9 +209,9 @@ private async Task PatchAsync(Params args, CancellationToken ct) var targetPath = skill.FilePath; if (!string.IsNullOrWhiteSpace(args.FilePath)) { - var fileError = ValidateResourcePath(args.FilePath); - if (fileError is not null) return fileError; - targetPath = Path.Combine(skill.SkillDirectory, args.FilePath); + if (!TryNormalizeResourcePath(args.FilePath, out var normalizedPath, out var fileError)) + return fileError; + targetPath = Path.Combine(skill.SkillDirectory, normalizedPath); } if (!File.Exists(targetPath)) @@ -244,7 +241,7 @@ private async Task PatchAsync(Params args, CancellationToken ct) var scanSubject = targetPath == skill.FilePath ? name - : $"{name}:{args.FilePath}"; + : $"{name}:{Path.GetRelativePath(skill.SkillDirectory, targetPath).Replace(Path.DirectorySeparatorChar, '/')}"; var scanResult = await _scanner.ScanAsync(scanSubject, newContent, ct); if (!scanResult.IsAllowed) return $"Content scan rejected: {scanResult.Reason}"; @@ -319,15 +316,15 @@ private async Task WriteFileAsync(Params args, CancellationToken ct) var readOnlyError = GuardReadOnly(skill, "write files in"); if (readOnlyError is not null) return readOnlyError; - var fileError = ValidateResourcePath(args.FilePath); - if (fileError is not null) return fileError; + if (!TryNormalizeResourcePath(args.FilePath, out var normalizedPath, out var fileError)) + return fileError; - var fullPath = Path.GetFullPath(Path.Combine(skill.SkillDirectory, args.FilePath)); + var fullPath = Path.GetFullPath(Path.Combine(skill.SkillDirectory, normalizedPath)); if (!PathUtility.IsWithinRoot(fullPath, skill.SkillDirectory)) return "Resolved path is outside the skill directory."; var scanResult = await _scanner.ScanAsync( - $"{name}:{args.FilePath}", + $"{name}:{normalizedPath}", args.FileContent, ct); if (!scanResult.IsAllowed) @@ -338,7 +335,7 @@ private async Task WriteFileAsync(Params args, CancellationToken ct) AtomicWrite(fullPath, args.FileContent); var rescan = RescanAndUpdateIndex(); - var message = $"File written: {args.FilePath}"; + var message = $"File written: {normalizedPath}"; if (scanResult.Verdict == ScanVerdict.Warning) message += $" (warning: {scanResult.Reason})"; @@ -361,15 +358,15 @@ private string RemoveFile(Params args) var readOnlyError = GuardReadOnly(skill, "remove files from"); if (readOnlyError is not null) return readOnlyError; - var fileError = ValidateResourcePath(args.FilePath); - if (fileError is not null) return fileError; + if (!TryNormalizeResourcePath(args.FilePath, out var normalizedPath, out var fileError)) + return fileError; - var fullPath = Path.GetFullPath(Path.Combine(skill.SkillDirectory, args.FilePath)); + var fullPath = Path.GetFullPath(Path.Combine(skill.SkillDirectory, normalizedPath)); if (!PathUtility.IsWithinRoot(fullPath, skill.SkillDirectory)) return "Resolved path is outside the skill directory."; if (!File.Exists(fullPath)) - return $"File not found: {args.FilePath}"; + return $"File not found: {normalizedPath}"; File.Delete(fullPath); @@ -381,7 +378,7 @@ private string RemoveFile(Params args) Directory.Delete(dir); } - return AppendScanWarnings($"File removed: {args.FilePath}", RescanAndUpdateIndex()); + return AppendScanWarnings($"File removed: {normalizedPath}", RescanAndUpdateIndex()); } // --- Helpers --- @@ -441,23 +438,45 @@ private bool IsSystemSkill(string name) return null; } - private static string? ValidateResourcePath(string? path) + private static bool TryNormalizeResourcePath(string? path, out string normalized, out string error) { + normalized = string.Empty; + error = string.Empty; + if (string.IsNullOrWhiteSpace(path)) - return "FilePath is required."; + { + error = "FilePath is required."; + return false; + } if (Path.IsPathRooted(path)) - return "Absolute paths are not allowed."; + { + error = "Absolute paths are not allowed."; + return false; + } + + normalized = path.Trim().Replace('\\', '/'); + if (normalized.Length == 0 || normalized.StartsWith('/') || normalized.EndsWith('/')) + { + error = "FilePath must be a relative file path inside the skill directory."; + return false; + } - if (path.Contains("..", StringComparison.Ordinal)) - return "Path traversal ('..') is not allowed."; + var segments = normalized.Split('/'); + if (segments.Any(static segment => segment.Length == 0 || segment is "." or "..")) + { + error = "FilePath cannot contain empty, '.', or '..' segments."; + return false; + } - var normalized = path.Replace('\\', '/'); - var firstSegment = normalized.Split('/')[0]; - if (!AllowedResourcePrefixes.Contains(firstSegment)) - return $"FilePath must start with one of: {string.Join(", ", AllowedResourcePrefixes)}. Got '{firstSegment}'."; + normalized = string.Join('/', segments); + if (string.Equals(normalized, "SKILL.md", StringComparison.OrdinalIgnoreCase)) + { + error = "Use create or edit to update SKILL.md; file operations are for additional resources."; + return false; + } - return null; + return true; } private string? GuardReadOnly(SkillEntry skill, string verb) diff --git a/src/Netclaw.Actors/Tools/SkillReadResourceTool.cs b/src/Netclaw.Actors/Tools/SkillReadResourceTool.cs index a98b07662..3d104b12a 100644 --- a/src/Netclaw.Actors/Tools/SkillReadResourceTool.cs +++ b/src/Netclaw.Actors/Tools/SkillReadResourceTool.cs @@ -13,21 +13,14 @@ namespace Netclaw.Actors.Tools; /// -/// Reads a resource file from a skill's references/, scripts/, or assets/ directory. +/// Reads a resource file from a skill directory. /// Scoped to the skill's directory with path traversal prevention. /// [NetclawTool("skill_read_resource", - "Read a resource file from a skill's references, scripts, or assets directory.", + "Read a resource file from a skill directory.", Grant = "builtin")] public sealed partial class SkillReadResourceTool : NetclawTool { - private static readonly HashSet AllowedPrefixes = new(StringComparer.OrdinalIgnoreCase) - { - "references", - "scripts", - "assets" - }; - private readonly SkillRegistry _skillRegistry; private readonly ISkillContentScanner _scanner; private readonly SkillSyncConfig _skillSyncConfig; @@ -35,7 +28,7 @@ public sealed partial class SkillReadResourceTool : NetclawTool ExecuteAsync(Params args, ToolExecutionCon if (skill is null) return $"Skill '{skillName}' not found."; - var resourcePath = args.ResourcePath.Trim(); - - // Reject absolute paths - if (Path.IsPathRooted(resourcePath)) - return "Absolute paths are not allowed. Use a relative path like 'references/doc.md'."; - - // Reject path traversal - if (resourcePath.Contains("..", StringComparison.Ordinal)) - return "Path traversal ('..') is not allowed."; - - // Normalize separators - resourcePath = resourcePath.Replace('\\', '/'); - - // Must start with an allowed prefix - var firstSegment = resourcePath.Split('/')[0]; - if (!AllowedPrefixes.Contains(firstSegment)) - return $"Resource path must start with one of: {string.Join(", ", AllowedPrefixes)}. Got '{firstSegment}'."; + if (!TryNormalizeResourcePath(args.ResourcePath, out var resourcePath, out var pathError)) + return pathError; // Resolve the full path and verify it's within the skill directory var fullPath = Path.GetFullPath(Path.Combine(skill.SkillDirectory, resourcePath)); @@ -123,6 +101,47 @@ protected override async Task ExecuteAsync(Params args, ToolExecutionCon } } + private static bool TryNormalizeResourcePath(string? path, out string normalized, out string error) + { + normalized = string.Empty; + error = string.Empty; + + if (string.IsNullOrWhiteSpace(path)) + { + error = "ResourcePath is required."; + return false; + } + + if (Path.IsPathRooted(path)) + { + error = "Absolute paths are not allowed. Use a relative path like 'references/doc.md'."; + return false; + } + + normalized = path.Trim().Replace('\\', '/'); + if (normalized.Length == 0 || normalized.StartsWith('/') || normalized.EndsWith('/')) + { + error = "Resource path must be a relative file path inside the skill directory."; + return false; + } + + var segments = normalized.Split('/'); + if (segments.Any(static segment => segment.Length == 0 || segment is "." or "..")) + { + error = "Resource path cannot contain empty, '.', or '..' segments."; + return false; + } + + normalized = string.Join('/', segments); + if (string.Equals(normalized, "SKILL.md", StringComparison.OrdinalIgnoreCase)) + { + error = "Use skill_load to read SKILL.md; resource paths must refer to additional files."; + return false; + } + + return true; + } + /// /// Returns true if any path component strictly between /// (exclusive) and (inclusive) is a symlink. The walk diff --git a/src/Netclaw.Daemon.Tests/Services/ServerFeedSkillSyncServiceTests.cs b/src/Netclaw.Daemon.Tests/Services/ServerFeedSkillSyncServiceTests.cs new file mode 100644 index 000000000..2429ff8c6 --- /dev/null +++ b/src/Netclaw.Daemon.Tests/Services/ServerFeedSkillSyncServiceTests.cs @@ -0,0 +1,122 @@ +// ----------------------------------------------------------------------- +// +// Copyright (C) 2026 - 2026 Petabridge, LLC +// +// ----------------------------------------------------------------------- +using System.IO.Compression; +using System.Text; +using Microsoft.Extensions.Logging.Abstractions; +using Netclaw.Actors.Skills; +using Netclaw.Configuration; +using Netclaw.Configuration.Feeds; +using Netclaw.Daemon.Services; +using Netclaw.Security.Skills; +using Netclaw.Tests.Utilities; +using Xunit; + +namespace Netclaw.Daemon.Tests.Services; + +public sealed class ServerFeedSkillSyncServiceTests : IDisposable +{ + private readonly DisposableTempDir _dir = new(); + private readonly NetclawPaths _paths; + private readonly SkillRegistry _skillRegistry = new(); + private readonly SkillIndexContextLayer _indexLayer = new(); + + public ServerFeedSkillSyncServiceTests() + { + _paths = new NetclawPaths(_dir.Path); + _paths.EnsureDirectoriesExist(); + } + + [Fact] + public async Task ExtractArchiveAsync_AllowsArbitraryResourcesAndPreservesExecutableMode() + { + var skillContent = Encoding.UTF8.GetBytes(""" + --- + name: packaged + description: Packaged skill. + --- + + # Packaged + """); + var scriptContent = Encoding.UTF8.GetBytes("#!/bin/bash\necho ok\n"); + var binaryContent = new byte[] { 0x00, 0x01, 0xFF, 0x02 }; + var archive = BuildArchive( + ("SKILL.md", skillContent, 0x1A4), + ("tools/check", scriptContent, 0x1ED), + ("assets/icon.bin", binaryContent, 0x1A4)); + + var files = await CreateService().ExtractArchiveAsync( + "packaged", "private", archive, TestContext.Current.CancellationToken); + + Assert.NotNull(files); + Assert.Contains(files!, file => file.RelativePath == "tools/check" && file.UnixMode == 0x1ED); + Assert.Contains(files, file => file.RelativePath == "assets/icon.bin"); + + var feedDir = _paths.ServerFeedDirectory("private"); + await SkillSyncHelpers.ReplaceSkillDirectoryAsync( + feedDir, "packaged", files!, TestContext.Current.CancellationToken); + + var skillDir = Path.Combine(feedDir, "packaged"); + Assert.Equal(scriptContent, await File.ReadAllBytesAsync(Path.Combine(skillDir, "tools", "check"), TestContext.Current.CancellationToken)); + Assert.Equal(binaryContent, await File.ReadAllBytesAsync(Path.Combine(skillDir, "assets", "icon.bin"), TestContext.Current.CancellationToken)); + + if (!OperatingSystem.IsWindows()) + { + var mode = (int)File.GetUnixFileMode(Path.Combine(skillDir, "tools", "check")) & 0x1FF; + Assert.Equal(0x1ED, mode); + } + } + + [Fact] + public async Task ExtractArchiveAsync_RejectsTraversalEntries() + { + var skillContent = Encoding.UTF8.GetBytes(""" + --- + name: packaged + description: Packaged skill. + --- + + # Packaged + """); + var archive = BuildArchive( + ("SKILL.md", skillContent, 0x1A4), + ("../escape.sh", Encoding.UTF8.GetBytes("echo no"), 0x1ED)); + + var files = await CreateService().ExtractArchiveAsync( + "packaged", "private", archive, TestContext.Current.CancellationToken); + + Assert.Null(files); + } + + public void Dispose() => _dir.Dispose(); + + private ServerFeedSkillSyncService CreateService(ISkillContentScanner? scanner = null) + => new( + new SkillFeedsConfig(), + _paths, + _skillRegistry, + _indexLayer, + TimeProvider.System, + scanner ?? new NoOpSkillContentScanner(), + NullLogger.Instance, + []); + + private static byte[] BuildArchive(params (string Path, byte[] Content, int UnixMode)[] entries) + { + using var stream = new MemoryStream(); + using (var archive = new ZipArchive(stream, ZipArchiveMode.Create, leaveOpen: true)) + { + foreach (var (path, content, unixMode) in entries) + { + var entry = archive.CreateEntry(path); + entry.ExternalAttributes = unixMode << 16; + using var entryStream = entry.Open(); + entryStream.Write(content); + } + } + + return stream.ToArray(); + } +} diff --git a/src/Netclaw.Daemon/Program.cs b/src/Netclaw.Daemon/Program.cs index 1d0219560..ae9163958 100644 --- a/src/Netclaw.Daemon/Program.cs +++ b/src/Netclaw.Daemon/Program.cs @@ -571,8 +571,6 @@ static void ConfigureDaemonServices( paths.PidFilePath, paths.LockFilePath, paths.RestartManifestPath, - paths.SystemSkillsDirectory, - paths.ServerFeedsDirectory, }; var toolPathPolicy = new ToolPathPolicy(writeDenyList, readDenyList, shellIndicatorList); services.AddSingleton(toolPathPolicy); diff --git a/src/Netclaw.Daemon/Services/ServerFeedSkillSyncService.cs b/src/Netclaw.Daemon/Services/ServerFeedSkillSyncService.cs index 73a4a5b14..8e488fea8 100644 --- a/src/Netclaw.Daemon/Services/ServerFeedSkillSyncService.cs +++ b/src/Netclaw.Daemon/Services/ServerFeedSkillSyncService.cs @@ -3,7 +3,9 @@ // Copyright (C) 2026 - 2026 Petabridge, LLC // // ----------------------------------------------------------------------- +using System.IO.Compression; using System.Net.Http.Headers; +using System.Text; using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging; using Netclaw.Actors.Skills; @@ -23,6 +25,10 @@ namespace Netclaw.Daemon.Services; /// internal sealed class ServerFeedSkillSyncService : BackgroundService { + private const string ArchiveType = "archive"; + private const string SkillFileName = "SKILL.md"; + private static readonly UTF8Encoding StrictUtf8 = new(false, true); + private readonly SkillFeedsConfig _feedsConfig; private readonly NetclawPaths _paths; private readonly SkillRegistry _skillRegistry; @@ -190,66 +196,81 @@ private async Task SyncFeedAsync(SkillFeedSource feed, CancellationToken cancell try { - var mainContent = await DownloadAndVerifyAsync( - httpClient, entry.Url, digestHex, entry.Name, feed.TimeoutSeconds, cancellationToken); - if (mainContent is null) - continue; - - var mainScan = await _scanner.ScanAsync(entry.Name, mainContent, cancellationToken); - if (!mainScan.IsAllowed) + List? downloadedFiles; + if (string.Equals(entry.Type, ArchiveType, StringComparison.OrdinalIgnoreCase)) { - _logger.LogWarning( - "Rejected skill '{SkillName}' from feed '{FeedName}': {Reason}", - entry.Name, feed.Name, mainScan.Reason); - continue; - } + var archiveBytes = await DownloadAndVerifyBytesAsync( + httpClient, entry.Url, digestHex, entry.Name, feed.TimeoutSeconds, cancellationToken); + if (archiveBytes is null) + continue; - var downloadedFiles = new List + downloadedFiles = await ExtractArchiveAsync(entry.Name, feed.Name, archiveBytes, cancellationToken); + if (downloadedFiles is null) + continue; + } + else { - new("SKILL.md", mainContent) - }; + var mainContent = await DownloadAndVerifyAsync( + httpClient, entry.Url, digestHex, entry.Name, feed.TimeoutSeconds, cancellationToken); + if (mainContent is null) + continue; - if (entry.Resources is { Count: > 0 }) - { - var allFilesOk = true; - foreach (var resource in entry.Resources) + var mainScan = await _scanner.ScanAsync(entry.Name, mainContent, cancellationToken); + if (!mainScan.IsAllowed) { - var normalizedPath = SkillSyncHelpers.ValidateResourcePath(resource.Path); - if (normalizedPath is null) - { - _logger.LogWarning( - "Rejected resource path for '{SkillName}' from feed '{FeedName}': {Path}", - entry.Name, feed.Name, resource.Path); - allFilesOk = false; - break; - } + _logger.LogWarning( + "Rejected skill '{SkillName}' from feed '{FeedName}': {Reason}", + entry.Name, feed.Name, mainScan.Reason); + continue; + } - var resourceDigest = NormalizeDigest(resource.Digest); - var fileContent = await DownloadAndVerifyAsync( - httpClient, resource.Url, resourceDigest, - $"{entry.Name}/{resource.Path}", feed.TimeoutSeconds, cancellationToken); - if (fileContent is null) - { - allFilesOk = false; - break; - } + downloadedFiles = new List + { + new(SkillFileName, mainContent) + }; - var fileScan = await _scanner.ScanAsync( - $"{entry.Name}:{normalizedPath}", fileContent, cancellationToken); - if (!fileScan.IsAllowed) + if (entry.Resources is { Count: > 0 }) + { + var allFilesOk = true; + foreach (var resource in entry.Resources) { - _logger.LogWarning( - "Rejected resource for '{SkillName}' from feed '{FeedName}' at {Path}: {Reason}", - entry.Name, feed.Name, normalizedPath, fileScan.Reason); - allFilesOk = false; - break; + var normalizedPath = SkillSyncHelpers.ValidateResourcePath(resource.Path); + if (normalizedPath is null) + { + _logger.LogWarning( + "Rejected resource path for '{SkillName}' from feed '{FeedName}': {Path}", + entry.Name, feed.Name, resource.Path); + allFilesOk = false; + break; + } + + var resourceDigest = NormalizeDigest(resource.Digest); + var fileContent = await DownloadAndVerifyAsync( + httpClient, resource.Url, resourceDigest, + $"{entry.Name}/{resource.Path}", feed.TimeoutSeconds, cancellationToken); + if (fileContent is null) + { + allFilesOk = false; + break; + } + + var fileScan = await _scanner.ScanAsync( + $"{entry.Name}:{normalizedPath}", fileContent, cancellationToken); + if (!fileScan.IsAllowed) + { + _logger.LogWarning( + "Rejected resource for '{SkillName}' from feed '{FeedName}' at {Path}: {Reason}", + entry.Name, feed.Name, normalizedPath, fileScan.Reason); + allFilesOk = false; + break; + } + + downloadedFiles.Add(new DownloadedSkillFile(normalizedPath, fileContent)); } - downloadedFiles.Add(new DownloadedSkillFile(normalizedPath, fileContent)); + if (!allFilesOk) + continue; } - - if (!allFilesOk) - continue; } await SkillSyncHelpers.ReplaceSkillDirectoryAsync( @@ -307,13 +328,33 @@ private static HttpClient CreateHttpClientForFeed(SkillFeedSource feed) private async Task DownloadAndVerifyAsync( HttpClient httpClient, string url, string expectedSha256Hex, string label, int timeoutSeconds, CancellationToken cancellationToken) + { + var contentBytes = await DownloadAndVerifyBytesAsync( + httpClient, url, expectedSha256Hex, label, timeoutSeconds, cancellationToken); + if (contentBytes is null) + return null; + + try + { + return StrictUtf8.GetString(contentBytes); + } + catch (DecoderFallbackException ex) + { + _logger.LogWarning("Downloaded content for {Label} is not valid UTF-8: {Message}", label, ex.Message); + return null; + } + } + + private async Task DownloadAndVerifyBytesAsync( + HttpClient httpClient, string url, string expectedSha256Hex, string label, + int timeoutSeconds, CancellationToken cancellationToken) { try { using var cts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); cts.CancelAfter(TimeSpan.FromSeconds(timeoutSeconds)); - var content = await httpClient.GetStringAsync(url, cts.Token); + var content = await httpClient.GetByteArrayAsync(url, cts.Token); var hash = SkillSyncHelpers.ComputeSha256(content); if (!string.Equals(hash, expectedSha256Hex, StringComparison.OrdinalIgnoreCase)) @@ -338,6 +379,163 @@ private static HttpClient CreateHttpClientForFeed(SkillFeedSource feed) } } + internal async Task?> ExtractArchiveAsync( + string skillName, + string feedName, + byte[] archiveBytes, + CancellationToken cancellationToken) + { + try + { + using var stream = new MemoryStream(archiveBytes); + using var archive = new ZipArchive(stream, ZipArchiveMode.Read, leaveOpen: false); + var paths = new HashSet(StringComparer.OrdinalIgnoreCase); + var files = new List(); + var hasSkillFile = false; + + foreach (var entry in archive.Entries) + { + var isDirectory = entry.FullName.EndsWith("/", StringComparison.Ordinal) + || entry.FullName.EndsWith("\\", StringComparison.Ordinal); + var normalizedPath = NormalizeArchiveEntryPath(entry.FullName, isDirectory); + if (normalizedPath is null) + { + _logger.LogWarning( + "Rejected archive for skill '{SkillName}' from feed '{FeedName}': unsafe entry path '{Path}'", + skillName, feedName, entry.FullName); + return null; + } + + if (IsZipSymlink(entry)) + { + _logger.LogWarning( + "Rejected archive for skill '{SkillName}' from feed '{FeedName}': symlink entry '{Path}'", + skillName, feedName, normalizedPath); + return null; + } + + if (isDirectory) + continue; + + if (!paths.Add(normalizedPath)) + { + _logger.LogWarning( + "Rejected archive for skill '{SkillName}' from feed '{FeedName}': duplicate entry '{Path}'", + skillName, feedName, normalizedPath); + return null; + } + + using var entryStream = entry.Open(); + using var entryBytes = new MemoryStream(); + await entryStream.CopyToAsync(entryBytes, cancellationToken); + var content = entryBytes.ToArray(); + var unixMode = GetUnixMode(entry); + if (unixMode is { } mode && (mode & ~0x1FF) != 0) + { + _logger.LogWarning( + "Rejected archive for skill '{SkillName}' from feed '{FeedName}': unsupported Unix mode {UnixMode} on '{Path}'", + skillName, feedName, mode, normalizedPath); + return null; + } + + if (string.Equals(normalizedPath, SkillFileName, StringComparison.OrdinalIgnoreCase)) + { + hasSkillFile = true; + string skillContent; + try + { + skillContent = StrictUtf8.GetString(content); + } + catch (DecoderFallbackException ex) + { + _logger.LogWarning( + "Rejected archive for skill '{SkillName}' from feed '{FeedName}': SKILL.md is not valid UTF-8: {Message}", + skillName, feedName, ex.Message); + return null; + } + + var mainScan = await _scanner.ScanAsync(skillName, skillContent, cancellationToken); + if (!mainScan.IsAllowed) + { + _logger.LogWarning( + "Rejected archive skill '{SkillName}' from feed '{FeedName}': {Reason}", + skillName, feedName, mainScan.Reason); + return null; + } + } + else if (TryDecodeUtf8(content, out var textContent)) + { + var fileScan = await _scanner.ScanAsync( + $"{skillName}:{normalizedPath}", textContent, cancellationToken); + if (!fileScan.IsAllowed) + { + _logger.LogWarning( + "Rejected archive resource for '{SkillName}' from feed '{FeedName}' at {Path}: {Reason}", + skillName, feedName, normalizedPath, fileScan.Reason); + return null; + } + } + + files.Add(new DownloadedSkillFile(normalizedPath, content, unixMode)); + } + + if (!hasSkillFile) + { + _logger.LogWarning( + "Rejected archive for skill '{SkillName}' from feed '{FeedName}': missing SKILL.md", + skillName, feedName); + return null; + } + + return files + .OrderBy(static file => string.Equals(file.RelativePath, SkillFileName, StringComparison.OrdinalIgnoreCase) ? 0 : 1) + .ThenBy(static file => file.RelativePath, StringComparer.Ordinal) + .ToList(); + } + catch (InvalidDataException ex) + { + _logger.LogWarning( + "Rejected archive for skill '{SkillName}' from feed '{FeedName}': invalid ZIP archive: {Message}", + skillName, feedName, ex.Message); + return null; + } + } + + private static string? NormalizeArchiveEntryPath(string path, bool isDirectory) + { + var normalized = path.Trim().Replace('\\', '/'); + if (isDirectory) + normalized = normalized.TrimEnd('/'); + + if (string.Equals(normalized, SkillFileName, StringComparison.OrdinalIgnoreCase)) + return SkillFileName; + + return SkillSyncHelpers.ValidateResourcePath(normalized); + } + + private static bool TryDecodeUtf8(byte[] content, out string text) + { + try + { + text = StrictUtf8.GetString(content); + return true; + } + catch (DecoderFallbackException) + { + text = string.Empty; + return false; + } + } + + private static bool IsZipSymlink(ZipArchiveEntry entry) + => ((entry.ExternalAttributes >> 16) & 0xF000) == 0xA000; + + private static int? GetUnixMode(ZipArchiveEntry entry) + { + var mode = (entry.ExternalAttributes >> 16) & 0xFFF; + return mode == 0 ? null : mode; + } + private void RescanAndUpdateIndex() { var resolvedServerFeeds = new List(); diff --git a/src/Netclaw.Daemon/Services/SkillSyncHelpers.cs b/src/Netclaw.Daemon/Services/SkillSyncHelpers.cs index b96a76bdc..adb4b75ac 100644 --- a/src/Netclaw.Daemon/Services/SkillSyncHelpers.cs +++ b/src/Netclaw.Daemon/Services/SkillSyncHelpers.cs @@ -13,8 +13,6 @@ namespace Netclaw.Daemon.Services; internal static class SkillSyncHelpers { - internal static readonly string[] AllowedResourcePrefixes = ["references", "scripts", "assets"]; - // A directory is a skill iff it owns a SKILL.md — the same rule SkillScanner uses. private const string SkillFileName = "SKILL.md"; @@ -23,7 +21,12 @@ internal static class SkillSyncHelpers internal static string ComputeSha256(string content) { var bytes = Encoding.UTF8.GetBytes(content); - var hash = SHA256.HashData(bytes); + return ComputeSha256(bytes); + } + + internal static string ComputeSha256(byte[] content) + { + var hash = SHA256.HashData(content); return Convert.ToHexStringLower(hash); } @@ -32,15 +35,21 @@ internal static string ComputeSha256(string content) if (string.IsNullOrWhiteSpace(path)) return null; - if (Path.IsPathRooted(path) || path.Contains("..", StringComparison.Ordinal)) + if (Path.IsPathRooted(path)) + return null; + + var normalized = path.Trim().Replace('\\', '/'); + if (normalized.Length == 0 || normalized.StartsWith('/') || normalized.EndsWith('/')) return null; - var normalized = path.Replace('\\', '/'); - var firstSegment = normalized.Split('/')[0]; - if (!AllowedResourcePrefixes.Contains(firstSegment, StringComparer.OrdinalIgnoreCase)) + var segments = normalized.Split('/'); + if (segments.Any(static segment => segment.Length == 0 || segment is "." or "..")) return null; - return normalized; + normalized = string.Join('/', segments); + return string.Equals(normalized, SkillFileName, StringComparison.OrdinalIgnoreCase) + ? null + : normalized; } internal static SkillSyncState ReadSyncState(string path, ILogger logger) @@ -175,7 +184,9 @@ internal static async Task ReplaceSkillDirectoryAsync( { var targetPath = Path.Combine(stagingDir, file.RelativePath.Replace('/', Path.DirectorySeparatorChar)); Directory.CreateDirectory(Path.GetDirectoryName(targetPath)!); - await File.WriteAllTextAsync(targetPath, file.Content, cancellationToken); + await File.WriteAllBytesAsync(targetPath, file.Content, cancellationToken); + if (file.UnixMode is { } unixMode && !OperatingSystem.IsWindows()) + File.SetUnixFileMode(targetPath, (UnixFileMode)unixMode); } if (Directory.Exists(skillDir)) @@ -204,4 +215,10 @@ internal static async Task ReplaceSkillDirectoryAsync( } } -internal sealed record DownloadedSkillFile(string RelativePath, string Content); +internal sealed record DownloadedSkillFile(string RelativePath, byte[] Content, int? UnixMode = null) +{ + public DownloadedSkillFile(string relativePath, string content, int? unixMode = null) + : this(relativePath, Encoding.UTF8.GetBytes(content), unixMode) + { + } +} diff --git a/src/Netclaw.Security.Tests/ToolPathPolicyTests.cs b/src/Netclaw.Security.Tests/ToolPathPolicyTests.cs index 776bcc8f7..fa5b1f4fd 100644 --- a/src/Netclaw.Security.Tests/ToolPathPolicyTests.cs +++ b/src/Netclaw.Security.Tests/ToolPathPolicyTests.cs @@ -176,8 +176,6 @@ private static ToolPathPolicy CreateProductionPolicy() "/home/user/.netclaw/netclaw.pid", "/home/user/.netclaw/netclaw.lock", "/home/user/.netclaw/cache/restart-manifest.json", - "/home/user/.netclaw/skills/.system", - "/home/user/.netclaw/skills/.server-feeds", }; return new ToolPathPolicy(writeDeny, readDeny, shellIndicators); } @@ -259,6 +257,16 @@ public void CommandReferencesDeniedPath_blocks_control_plane_lifecycle_files() Assert.True(policy.CommandReferencesDeniedPath("cat ~/.netclaw/cache/restart-manifest.json")); } + [Theory] + [InlineData("bash /home/user/.netclaw/skills/.system/my-skill/tools/check")] + [InlineData("/home/user/.netclaw/skills/.server-feeds/my-feed/feed-skill/tools/check")] + public void CommandReferencesDeniedPath_allows_synced_skill_resource_execution(string command) + { + var policy = CreateProductionPolicy(); + + Assert.False(policy.CommandReferencesDeniedPath(command)); + } + // Regression: directory-scoped approvals let a user grant a single root // (e.g., /home/user/safe/) once, after which all subsequent shell commands // under that root auto-approve. The design promises that ToolPathPolicy From 254281804d52e3ef9efd64f7a5840a1e178ead23 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Thu, 2 Jul 2026 13:45:33 +0000 Subject: [PATCH 2/2] Consolidate skill resource path validation --- src/Netclaw.Actors/Tools/SkillManageTool.cs | 53 ++---------- .../Tools/SkillReadResourceTool.cs | 45 +--------- src/Netclaw.Actors/Tools/SkillResourcePath.cs | 84 +++++++++++++++++++ 3 files changed, 92 insertions(+), 90 deletions(-) create mode 100644 src/Netclaw.Actors/Tools/SkillResourcePath.cs diff --git a/src/Netclaw.Actors/Tools/SkillManageTool.cs b/src/Netclaw.Actors/Tools/SkillManageTool.cs index 7e9cdfa7e..3f4bbd72c 100644 --- a/src/Netclaw.Actors/Tools/SkillManageTool.cs +++ b/src/Netclaw.Actors/Tools/SkillManageTool.cs @@ -209,8 +209,8 @@ private async Task PatchAsync(Params args, CancellationToken ct) var targetPath = skill.FilePath; if (!string.IsNullOrWhiteSpace(args.FilePath)) { - if (!TryNormalizeResourcePath(args.FilePath, out var normalizedPath, out var fileError)) - return fileError; + if (!SkillResourcePath.TryNormalize(args.FilePath, out var normalizedPath, out var fileError)) + return SkillResourcePath.FormatManageError(fileError); targetPath = Path.Combine(skill.SkillDirectory, normalizedPath); } @@ -316,8 +316,8 @@ private async Task WriteFileAsync(Params args, CancellationToken ct) var readOnlyError = GuardReadOnly(skill, "write files in"); if (readOnlyError is not null) return readOnlyError; - if (!TryNormalizeResourcePath(args.FilePath, out var normalizedPath, out var fileError)) - return fileError; + if (!SkillResourcePath.TryNormalize(args.FilePath, out var normalizedPath, out var fileError)) + return SkillResourcePath.FormatManageError(fileError); var fullPath = Path.GetFullPath(Path.Combine(skill.SkillDirectory, normalizedPath)); if (!PathUtility.IsWithinRoot(fullPath, skill.SkillDirectory)) @@ -358,8 +358,8 @@ private string RemoveFile(Params args) var readOnlyError = GuardReadOnly(skill, "remove files from"); if (readOnlyError is not null) return readOnlyError; - if (!TryNormalizeResourcePath(args.FilePath, out var normalizedPath, out var fileError)) - return fileError; + if (!SkillResourcePath.TryNormalize(args.FilePath, out var normalizedPath, out var fileError)) + return SkillResourcePath.FormatManageError(fileError); var fullPath = Path.GetFullPath(Path.Combine(skill.SkillDirectory, normalizedPath)); if (!PathUtility.IsWithinRoot(fullPath, skill.SkillDirectory)) @@ -438,47 +438,6 @@ private bool IsSystemSkill(string name) return null; } - private static bool TryNormalizeResourcePath(string? path, out string normalized, out string error) - { - normalized = string.Empty; - error = string.Empty; - - if (string.IsNullOrWhiteSpace(path)) - { - error = "FilePath is required."; - return false; - } - - if (Path.IsPathRooted(path)) - { - error = "Absolute paths are not allowed."; - return false; - } - - normalized = path.Trim().Replace('\\', '/'); - if (normalized.Length == 0 || normalized.StartsWith('/') || normalized.EndsWith('/')) - { - error = "FilePath must be a relative file path inside the skill directory."; - return false; - } - - var segments = normalized.Split('/'); - if (segments.Any(static segment => segment.Length == 0 || segment is "." or "..")) - { - error = "FilePath cannot contain empty, '.', or '..' segments."; - return false; - } - - normalized = string.Join('/', segments); - if (string.Equals(normalized, "SKILL.md", StringComparison.OrdinalIgnoreCase)) - { - error = "Use create or edit to update SKILL.md; file operations are for additional resources."; - return false; - } - - return true; - } - private string? GuardReadOnly(SkillEntry skill, string verb) { if (IsSystemCategory(skill)) diff --git a/src/Netclaw.Actors/Tools/SkillReadResourceTool.cs b/src/Netclaw.Actors/Tools/SkillReadResourceTool.cs index 3d104b12a..7a80e8d10 100644 --- a/src/Netclaw.Actors/Tools/SkillReadResourceTool.cs +++ b/src/Netclaw.Actors/Tools/SkillReadResourceTool.cs @@ -56,8 +56,8 @@ protected override async Task ExecuteAsync(Params args, ToolExecutionCon if (skill is null) return $"Skill '{skillName}' not found."; - if (!TryNormalizeResourcePath(args.ResourcePath, out var resourcePath, out var pathError)) - return pathError; + if (!SkillResourcePath.TryNormalize(args.ResourcePath, out var resourcePath, out var pathError)) + return SkillResourcePath.FormatReadError(pathError); // Resolve the full path and verify it's within the skill directory var fullPath = Path.GetFullPath(Path.Combine(skill.SkillDirectory, resourcePath)); @@ -101,47 +101,6 @@ protected override async Task ExecuteAsync(Params args, ToolExecutionCon } } - private static bool TryNormalizeResourcePath(string? path, out string normalized, out string error) - { - normalized = string.Empty; - error = string.Empty; - - if (string.IsNullOrWhiteSpace(path)) - { - error = "ResourcePath is required."; - return false; - } - - if (Path.IsPathRooted(path)) - { - error = "Absolute paths are not allowed. Use a relative path like 'references/doc.md'."; - return false; - } - - normalized = path.Trim().Replace('\\', '/'); - if (normalized.Length == 0 || normalized.StartsWith('/') || normalized.EndsWith('/')) - { - error = "Resource path must be a relative file path inside the skill directory."; - return false; - } - - var segments = normalized.Split('/'); - if (segments.Any(static segment => segment.Length == 0 || segment is "." or "..")) - { - error = "Resource path cannot contain empty, '.', or '..' segments."; - return false; - } - - normalized = string.Join('/', segments); - if (string.Equals(normalized, "SKILL.md", StringComparison.OrdinalIgnoreCase)) - { - error = "Use skill_load to read SKILL.md; resource paths must refer to additional files."; - return false; - } - - return true; - } - /// /// Returns true if any path component strictly between /// (exclusive) and (inclusive) is a symlink. The walk diff --git a/src/Netclaw.Actors/Tools/SkillResourcePath.cs b/src/Netclaw.Actors/Tools/SkillResourcePath.cs new file mode 100644 index 000000000..d51cb6a87 --- /dev/null +++ b/src/Netclaw.Actors/Tools/SkillResourcePath.cs @@ -0,0 +1,84 @@ +// ----------------------------------------------------------------------- +// +// Copyright (C) 2026 - 2026 Petabridge, LLC +// +// ----------------------------------------------------------------------- +namespace Netclaw.Actors.Tools; + +internal static class SkillResourcePath +{ + private const string SkillFileName = "SKILL.md"; + + internal static bool TryNormalize(string? path, out string normalized, out SkillResourcePathError error) + { + normalized = string.Empty; + error = SkillResourcePathError.None; + + if (string.IsNullOrWhiteSpace(path)) + { + error = SkillResourcePathError.Required; + return false; + } + + if (Path.IsPathRooted(path)) + { + error = SkillResourcePathError.Absolute; + return false; + } + + normalized = path.Trim().Replace('\\', '/'); + if (normalized.Length == 0 || normalized.StartsWith('/') || normalized.EndsWith('/')) + { + error = SkillResourcePathError.NotRelativeFile; + return false; + } + + var segments = normalized.Split('/'); + if (segments.Any(static segment => segment.Length == 0 || segment is "." or "..")) + { + error = SkillResourcePathError.InvalidSegment; + return false; + } + + normalized = string.Join('/', segments); + if (string.Equals(normalized, SkillFileName, StringComparison.OrdinalIgnoreCase)) + { + error = SkillResourcePathError.SkillFile; + return false; + } + + return true; + } + + internal static string FormatReadError(SkillResourcePathError error) + => error switch + { + SkillResourcePathError.Required => "ResourcePath is required.", + SkillResourcePathError.Absolute => "Absolute paths are not allowed. Use a relative path like 'references/doc.md'.", + SkillResourcePathError.NotRelativeFile => "Resource path must be a relative file path inside the skill directory.", + SkillResourcePathError.InvalidSegment => "Resource path cannot contain empty, '.', or '..' segments.", + SkillResourcePathError.SkillFile => "Use skill_load to read SKILL.md; resource paths must refer to additional files.", + _ => "Invalid resource path." + }; + + internal static string FormatManageError(SkillResourcePathError error) + => error switch + { + SkillResourcePathError.Required => "FilePath is required.", + SkillResourcePathError.Absolute => "Absolute paths are not allowed.", + SkillResourcePathError.NotRelativeFile => "FilePath must be a relative file path inside the skill directory.", + SkillResourcePathError.InvalidSegment => "FilePath cannot contain empty, '.', or '..' segments.", + SkillResourcePathError.SkillFile => "Use create or edit to update SKILL.md; file operations are for additional resources.", + _ => "Invalid file path." + }; +} + +internal enum SkillResourcePathError +{ + None, + Required, + Absolute, + NotRelativeFile, + InvalidSegment, + SkillFile +}