security(skills): prevent workspace skills from shadowing builtins
Builtin skill names are now reserved — workspace and global skills with the same name as a builtin are skipped with a warning log. LoadSkill also checks builtins first. This prevents supply chain attacks where a malicious skill replaces a trusted builtin. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
2bb7a7568f
commit
147e26ac60
2 changed files with 46 additions and 12 deletions
|
|
@ -96,9 +96,37 @@ func NewSkillsLoader(workspace string, globalSkills string, builtinSkills string
|
|||
}
|
||||
}
|
||||
|
||||
// builtinSkillNames returns the set of validated skill names from the builtin directory.
|
||||
func (sl *SkillsLoader) builtinSkillNames() map[string]bool {
|
||||
names := make(map[string]bool)
|
||||
if sl.builtinSkills == "" {
|
||||
return names
|
||||
}
|
||||
dirs, err := os.ReadDir(sl.builtinSkills)
|
||||
if err != nil {
|
||||
return names
|
||||
}
|
||||
for _, d := range dirs {
|
||||
if !d.IsDir() {
|
||||
continue
|
||||
}
|
||||
skillFile := filepath.Join(sl.builtinSkills, d.Name(), "SKILL.md")
|
||||
if _, err := os.Stat(skillFile); err != nil {
|
||||
continue
|
||||
}
|
||||
name := d.Name()
|
||||
if metadata := sl.getSkillMetadata(skillFile); metadata != nil && metadata.Name != "" {
|
||||
name = metadata.Name
|
||||
}
|
||||
names[name] = true
|
||||
}
|
||||
return names
|
||||
}
|
||||
|
||||
func (sl *SkillsLoader) ListSkills() []SkillInfo {
|
||||
skills := make([]SkillInfo, 0)
|
||||
seen := make(map[string]bool)
|
||||
builtinNames := sl.builtinSkillNames()
|
||||
|
||||
addSkills := func(dir, source string) {
|
||||
if dir == "" {
|
||||
|
|
@ -133,6 +161,12 @@ func (sl *SkillsLoader) ListSkills() []SkillInfo {
|
|||
if seen[info.Name] {
|
||||
continue
|
||||
}
|
||||
// Block workspace/global skills from shadowing builtins.
|
||||
if source != "builtin" && builtinNames[info.Name] {
|
||||
slog.Warn("skill shadows a builtin and will be skipped",
|
||||
"name", info.Name, "source", source)
|
||||
continue
|
||||
}
|
||||
seen[info.Name] = true
|
||||
skills = append(skills, info)
|
||||
}
|
||||
|
|
@ -147,7 +181,15 @@ func (sl *SkillsLoader) ListSkills() []SkillInfo {
|
|||
}
|
||||
|
||||
func (sl *SkillsLoader) LoadSkill(name string) (string, bool) {
|
||||
// 1. load from workspace skills first (project-level)
|
||||
// If this is a builtin skill, always load from builtin to prevent shadowing.
|
||||
if sl.builtinSkills != "" {
|
||||
skillFile := filepath.Join(sl.builtinSkills, name, "SKILL.md")
|
||||
if content, err := os.ReadFile(skillFile); err == nil {
|
||||
return sl.stripFrontmatter(string(content)), true
|
||||
}
|
||||
}
|
||||
|
||||
// 1. load from workspace skills (project-level)
|
||||
if sl.workspaceSkills != "" {
|
||||
skillFile := filepath.Join(sl.workspaceSkills, name, "SKILL.md")
|
||||
if content, err := os.ReadFile(skillFile); err == nil {
|
||||
|
|
@ -163,14 +205,6 @@ func (sl *SkillsLoader) LoadSkill(name string) (string, bool) {
|
|||
}
|
||||
}
|
||||
|
||||
// 3. finally load from builtin skills
|
||||
if sl.builtinSkills != "" {
|
||||
skillFile := filepath.Join(sl.builtinSkills, name, "SKILL.md")
|
||||
if content, err := os.ReadFile(skillFile); err == nil {
|
||||
return sl.stripFrontmatter(string(content)), true
|
||||
}
|
||||
}
|
||||
|
||||
return "", false
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -163,7 +163,7 @@ func TestListSkillsWorkspaceOverridesGlobal(t *testing.T) {
|
|||
assert.Equal(t, "workspace version", skills[0].Description)
|
||||
}
|
||||
|
||||
func TestListSkillsGlobalOverridesBuiltin(t *testing.T) {
|
||||
func TestListSkillsBuiltinCannotBeShadowed(t *testing.T) {
|
||||
tmp := t.TempDir()
|
||||
ws := filepath.Join(tmp, "workspace")
|
||||
global := filepath.Join(tmp, "global")
|
||||
|
|
@ -176,8 +176,8 @@ func TestListSkillsGlobalOverridesBuiltin(t *testing.T) {
|
|||
skills := sl.ListSkills()
|
||||
|
||||
assert.Len(t, skills, 1)
|
||||
assert.Equal(t, "global", skills[0].Source)
|
||||
assert.Equal(t, "global version", skills[0].Description)
|
||||
assert.Equal(t, "builtin", skills[0].Source)
|
||||
assert.Equal(t, "builtin version", skills[0].Description)
|
||||
}
|
||||
|
||||
func TestListSkillsMetadataNameDedup(t *testing.T) {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue