Skip to content

fix(skills): block agent mutations to server feed skill directories - #1466

Merged
Aaronontheweb merged 5 commits into
netclaw-dev:devfrom
Aaronontheweb:claude-wt-skillfeeds-readonly
Jun 24, 2026
Merged

fix(skills): block agent mutations to server feed skill directories#1466
Aaronontheweb merged 5 commits into
netclaw-dev:devfrom
Aaronontheweb:claude-wt-skillfeeds-readonly

Conversation

@Aaronontheweb

@Aaronontheweb Aaronontheweb commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator

Closes #1449

Summary

  • Root cause: .server-feeds/ skills live inside SkillsDirectory, so neither the .system category check nor the "external skill" (outside native root) check caught them — mutations via skill_manage and direct file_write/file_edit both went through unblocked
  • Two enforcement layers added: SkillManageTool now rejects all mutations on server-feed skills via a new IsServerFeedSkill guard; ToolPathPolicy write deny list in Program.cs now includes SystemSkillsDirectory and ServerFeedsDirectory so file_write, file_edit, and shell commands are blocked at the policy layer
  • Refactor: extracted GuardReadOnly(skill, verb) to eliminate five copies of the system/feed/external three-check block

Test plan

  • 5 new SkillManage_ServerFeedSkill_Blocks* tests exercise all mutation actions against a .server-feeds/my-feed/ skill
  • Restored RemoveFile_rejects_system_skill, WriteFile_rejects_external_skill, RemoveFile_rejects_external_skill — specific actions not covered by the pre-existing edit/delete external tests
  • ToolPathPolicyTests.CreateProductionPolicy() updated to mirror the new Program.cs deny lists; IsDenied_blocks_control_plane_files has InlineData for child paths under both new directories
  • ScanFeedSkills helper switched from ReplaceAll to Register so future tests mixing native + feed skill populations don't silently drop one side
  • 2443 Actors.Tests + 594 Security.Tests all green

@Aaronontheweb
Aaronontheweb force-pushed the claude-wt-skillfeeds-readonly branch from 856919e to a359e96 Compare June 23, 2026 21:59
@Aaronontheweb
Aaronontheweb enabled auto-merge (squash) June 23, 2026 22:27
Fixes the gap described in netclaw-dev#1449. Agents could freely edit, patch,
delete, and write files into .server-feeds/ skill directories via both
the skill_manage tool and direct file write tools (file_write,
file_edit). Changes made through either path are silently overwritten
on the next sync cycle, making them wasteful at best and misleading
about session state at worst.

Two enforcement layers added:

skill_manage tool (SkillManageTool.cs):
- Added IsServerFeedSkill guard that checks whether a skill's file path
  sits within paths.ServerFeedsDirectory (.server-feeds/ is inside the
  native skills root, so the existing IsExternalSkill check did not
  catch it)
- Extracted GuardReadOnly(skill, verb) to eliminate five copies of the
  system/feed/external three-check block

Direct file write tools (Program.cs):
- Added paths.SystemSkillsDirectory and paths.ServerFeedsDirectory to
  writeDenyList and shellIndicatorList so file_write, file_edit, and
  shell commands are denied at the ToolPathPolicy layer — the same
  layer that already protects config, secrets, and keys

Test changes (SkillToolTests.cs):
- Removed four tests added in 9d89936 that claimed to cover netclaw-dev#1449 but
  actually tested system and external skill guards — protections that
  predate the issue and were already covered elsewhere
- Added five tests that exercise the actual attack vector: a skill
  scanned from a .server-feeds/my-feed/ directory is rejected by all
  five skill_manage mutation actions
- ToolPathPolicyTests: update CreateProductionPolicy() mirror to include
  SystemSkillsDirectory and ServerFeedsDirectory in writeDeny and
  shellIndicators; add InlineData entries so IsDenied_blocks_control_plane_files
  exercises both paths

- SkillToolTests: restore the three guard-coverage tests removed in the
  prior commit — RemoveFile_rejects_system_skill, WriteFile_rejects_external_skill,
  RemoveFile_rejects_external_skill — these cover specific actions that
  Edit/Delete tests do not

- ScanFeedSkills: switch from ReplaceAll to Register so feed skills are
  merged into the registry rather than replacing it; prevents future tests
  that mix native and feed skill populations from silently losing one side
@Aaronontheweb
Aaronontheweb force-pushed the claude-wt-skillfeeds-readonly branch from a359e96 to e9a7e26 Compare June 23, 2026 22:27
@Aaronontheweb Aaronontheweb added skills Skill loading, handling, authoring, indexing, and evals. reliability Retries, resilience, graceful degradation labels Jun 23, 2026

@Aaronontheweb Aaronontheweb left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Aaronontheweb
Aaronontheweb merged commit 34781b5 into netclaw-dev:dev Jun 24, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

reliability Retries, resilience, graceful degradation skills Skill loading, handling, authoring, indexing, and evals.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature: Mark managed skill feeds as read-only for agents

1 participant