Allow synced skill resources to run via shell#1551
Conversation
Aaronontheweb
left a comment
There was a problem hiding this comment.
Not quite done, just doing my first review pass. Found some issues.
| /// <summary> | ||
| /// Standard subdirectories within a skill directory that contain resources. | ||
| /// </summary> | ||
| private static readonly string[] ResourceSubdirectories = ["scripts", "references", "assets"]; |
There was a problem hiding this comment.
Rather than scan static skill resource directories, we enumerate them on the fly now since the AgentSkills.io standard doesn't mandate strictly these ones.
| var subDir = Path.Combine(skillDirectory, subDirName); | ||
| if (!Directory.Exists(subDir)) | ||
| continue; | ||
| files = Directory.GetFiles(skillDirectory, "*", SearchOption.AllDirectories); |
There was a problem hiding this comment.
find all files in this skill directory
| return null; | ||
| foreach (var file in files.OrderBy(static f => f, StringComparer.Ordinal)) | ||
| { | ||
| if (PathUtility.AreEquivalentPaths(file, Path.Combine(skillDirectory, SkillFileName))) |
There was a problem hiding this comment.
validate symlinks et al
| // ----------------------------------------------------------------------- | ||
| namespace Netclaw.Actors.Tools; | ||
|
|
||
| internal static class SkillResourcePath |
There was a problem hiding this comment.
Standardized helpers for working with agent skill resources
| } | ||
| } | ||
|
|
||
| internal async Task<List<DownloadedSkillFile>?> ExtractArchiveAsync( |
There was a problem hiding this comment.
in the next release of skill-server, it will expose skills as .zip archives
| return null; | ||
| } | ||
|
|
||
| var mainScan = await _scanner.ScanAsync(skillName, skillContent, cancellationToken); |
There was a problem hiding this comment.
We still scan the content inside the archive
| paths.PidFilePath, | ||
| paths.LockFilePath, | ||
| paths.RestartManifestPath, | ||
| paths.SystemSkillsDirectory, |
There was a problem hiding this comment.
Allows these folder to be shell-executable
| "/home/user/.netclaw/netclaw.pid", | ||
| "/home/user/.netclaw/netclaw.lock", | ||
| "/home/user/.netclaw/cache/restart-manifest.json", | ||
| "/home/user/.netclaw/skills/.system", |
There was a problem hiding this comment.
Allows tool execution inside here
| { | ||
| var policy = CreateProductionPolicy(); | ||
|
|
||
| Assert.False(policy.CommandReferencesDeniedPath(command)); |
There was a problem hiding this comment.
Validate that we no longer deny attempting to execute resources that are provided with skills
Closes #1537.
Summary
This PR lets synced skill resources run through existing
shell_executeinstead of adding a new execution tool.Changes
.systemand.server-feedsfrom the shell protected-path indicator list while keeping them write-deniedtype: archiveprivate skill-server artifacts with safe ZIP extractionreferences/,scripts/, orassets/skill-authoringguidance for arbitrary AgentSkills.io resourcesSafety
SKILL.md, and unsupported special Unix modesshell_executeand the normal audience/approval policyskill_execute_resourcetool is addedValidation
dotnet build Netclaw.slnxdotnet test src/Netclaw.Security.Tests/Netclaw.Security.Tests.csproj --filter "FullyQualifiedName~ToolPathPolicyTests"dotnet test src/Netclaw.Actors.Tests/Netclaw.Actors.Tests.csproj --filter "FullyQualifiedName~SkillScannerTests|FullyQualifiedName~SkillToolTests"dotnet test src/Netclaw.Daemon.Tests/Netclaw.Daemon.Tests.csproj --filter "FullyQualifiedName~ServerFeedSkillSyncServiceTests|FullyQualifiedName~SkillSyncHelpersTests|FullyQualifiedName~SystemSkillSyncServiceTests"dotnet slopwatch analyzepwsh ./scripts/Add-FileHeaders.ps1 -VerifyNot Run
./evals/run-evals.sh: eval-target credentials are not configured in this non-interactive environment (NETCLAW_EVAL_PROVIDER_TYPE,NETCLAW_EVAL_PROVIDER_ENDPOINT,NETCLAW_EVAL_MODEL_ID)Related