fix(skills): harden managed skill path validation

internal/handlers/skills.go

@@ -101,10 +101,10 @@ func (h *ContainerdHandler) UpsertSkills(c echo.Context) error {
 
 	for _, raw := range req.Skills {
 		parsed := skillset.ParseFile(raw, "")
-		if !skillset.IsValidName(parsed.Name) {
+		dirPath, dirErr := skillset.ManagedSkillDirForName(parsed.Name)
+		if dirErr != nil {
 			return echo.NewHTTPError(http.StatusBadRequest, "skill must have a valid name in YAML frontmatter")
 		}
-		dirPath := path.Join(skillset.ManagedDir(), parsed.Name)
 		if err := client.Mkdir(ctx, dirPath); err != nil {
 			return echo.NewHTTPError(http.StatusInternalServerError, fmt.Sprintf("mkdir failed: %v", err))
 		}
@@ -149,10 +149,10 @@ func (h *ContainerdHandler) DeleteSkills(c echo.Context) error {
 
 	for _, name := range req.Names {
 		skillName := strings.TrimSpace(name)
-		if !skillset.IsValidName(skillName) {
+		managedDir, dirErr := skillset.ManagedSkillDirForName(skillName)
+		if dirErr != nil {
 			return echo.NewHTTPError(http.StatusBadRequest, "invalid skill name")
 		}
-		managedDir := path.Join(skillset.ManagedDir(), skillName)
 		if _, statErr := client.Stat(ctx, managedDir); statErr != nil {
 			return fsHTTPError(statErr)
 		}

internal/handlers/skills_test.go

@@ -205,6 +205,24 @@ func TestDeleteSkillsAPIRejectsExternalOnlySkill(t *testing.T) {
 	}
 }
 
+func TestUpsertSkillsAPIRejectsTraversalName(t *testing.T) {
+	env := newSkillsTestEnv(t)
+
+	_, err := env.callJSON(t, http.MethodPost, "/bots/:bot_id/container/skills", SkillsUpsertRequest{
+		Skills: []string{"---\nname: ..\ndescription: Escape\n---\n\n# Escape"},
+	}, env.handler.UpsertSkills)
+	if err == nil {
+		t.Fatal("expected upserting traversal skill name to fail")
+	}
+	var httpErr *echo.HTTPError
+	if !errors.As(err, &httpErr) {
+		t.Fatalf("expected echo.HTTPError, got %T", err)
+	}
+	if httpErr.Code != http.StatusBadRequest {
+		t.Fatalf("upsert traversal status = %d, want 400", httpErr.Code)
+	}
+}
+
 func TestLoadSkillsUsesEffectiveSetAndPromptReflectsOverrideFallback(t *testing.T) {
 	env := newSkillsTestEnv(t)
 	managedPath := path.Join(skillset.ManagedDir(), "alpha", "SKILL.md")

internal/skills/skills.go

@@ -102,6 +102,19 @@ func ManagedDir() string {
 	return ManagedDirPath
 }
 
+func ManagedSkillDirForName(name string) (string, error) {
+	name = strings.TrimSpace(name)
+	if !IsValidName(name) {
+		return "", bridge.ErrBadRequest
+	}
+
+	dirPath := path.Clean(path.Join(ManagedDirPath, name))
+	if dirPath == ManagedDirPath || !strings.HasPrefix(dirPath, ManagedDirPath+"/") {
+		return "", bridge.ErrBadRequest
+	}
+	return dirPath, nil
+}
+
 func ContainerEnv() []string {
 	return []string{
 		"HOME=" + config.DefaultDataMount,
@@ -184,7 +197,10 @@ func ApplyAction(ctx context.Context, client fileClient, req ActionRequest) erro
 				return bridge.ErrBadRequest
 			}
 		}
-		dirPath := path.Join(ManagedDirPath, target.Name)
+		dirPath, err := ManagedSkillDirForName(target.Name)
+		if err != nil {
+			return err
+		}
 		if err := client.Mkdir(ctx, dirPath); err != nil {
 			return err
 		}
@@ -247,6 +263,12 @@ func IsValidName(name string) bool {
 	if name == "" {
 		return false
 	}
+	if name == "." || name == ".." {
+		return false
+	}
+	if strings.HasPrefix(name, ".") || strings.Contains(name, "..") {
+		return false
+	}
 	for _, r := range name {
 		switch {
 		case r >= 'a' && r <= 'z':

internal/skills/skills_test.go

@@ -2,11 +2,13 @@ package skills
 
 import (
 	"context"
+	"errors"
 	"io"
 	"slices"
 	"strings"
 	"testing"
 
+	"github.com/memohai/memoh/internal/workspace/bridge"
 	pb "github.com/memohai/memoh/internal/workspace/bridgepb"
 )
 
@@ -107,6 +109,62 @@ func TestApplyActionAdoptAndDisable(t *testing.T) {
 	}
 }
 
+func TestApplyActionAdoptRejectsInvalidManagedName(t *testing.T) {
+	client := newFakeClient()
+	externalPath := pathJoin("/data/.agents/skills", "escape", "SKILL.md")
+	client.listings["/data/.agents/skills"] = []*pb.FileEntry{{Path: "escape", IsDir: true}}
+	client.files[externalPath] = "---\nname: ..\ndescription: Escape\n---\n\n# Escape"
+
+	err := ApplyAction(context.Background(), client, ActionRequest{
+		Action:     ActionAdopt,
+		TargetPath: externalPath,
+	})
+	if !errors.Is(err, bridge.ErrBadRequest) {
+		t.Fatalf("adopt err = %v, want ErrBadRequest", err)
+	}
+	if _, ok := client.files[pathJoin(ManagedDirPath, "..", "SKILL.md")]; ok {
+		t.Fatalf("unexpected managed write for invalid adopted name")
+	}
+}
+
+func TestIsValidNameRejectsTraversalPatterns(t *testing.T) {
+	for _, name := range []string{
+		"",
+		".",
+		"..",
+		".hidden",
+		"alpha..beta",
+		"../escape",
+		"alpha/../beta",
+	} {
+		if IsValidName(name) {
+			t.Fatalf("IsValidName(%q) = true, want false", name)
+		}
+	}
+
+	for _, name := range []string{"alpha", "alpha-beta", "alpha_beta", "alpha.beta"} {
+		if !IsValidName(name) {
+			t.Fatalf("IsValidName(%q) = false, want true", name)
+		}
+	}
+}
+
+func TestManagedSkillDirForNameRejectsEscapingNames(t *testing.T) {
+	for _, name := range []string{".", "..", ".alpha", "alpha..beta"} {
+		if _, err := ManagedSkillDirForName(name); !errors.Is(err, bridge.ErrBadRequest) {
+			t.Fatalf("ManagedSkillDirForName(%q) err = %v, want ErrBadRequest", name, err)
+		}
+	}
+
+	dirPath, err := ManagedSkillDirForName("alpha.beta")
+	if err != nil {
+		t.Fatalf("ManagedSkillDirForName(valid) returned error: %v", err)
+	}
+	if dirPath != pathJoin(ManagedDirPath, "alpha.beta") {
+		t.Fatalf("ManagedSkillDirForName(valid) = %q, want %q", dirPath, pathJoin(ManagedDirPath, "alpha.beta"))
+	}
+}
+
 func TestDiscoveryRootsMatchCurrentPolicy(t *testing.T) {
 	roots := DiscoveryRoots()
 	want := []Root{