feat(skills): privacy/visibility controls for agent-owned skills

internal/gateway/methods/skills.go

@@ -10,6 +10,7 @@ import (
 	"github.com/nextlevelbuilder/goclaw/internal/gateway"
 	"github.com/nextlevelbuilder/goclaw/internal/i18n"
 	"github.com/nextlevelbuilder/goclaw/internal/permissions"
+	"github.com/nextlevelbuilder/goclaw/internal/skills"
 	"github.com/nextlevelbuilder/goclaw/internal/store"
 	"github.com/nextlevelbuilder/goclaw/pkg/protocol"
 )
@@ -38,6 +39,12 @@ func (m *SkillsMethods) Register(router *gateway.MethodRouter) {
 func (m *SkillsMethods) handleList(ctx context.Context, client *gateway.Client, req *protocol.RequestFrame) {
 	allSkills := m.store.ListSkills(ctx)
 
+	// Visibility filter: non-admins see system skills, public skills, and
+	// their own private skills. Admins see everything in the tenant.
+	if !permissions.HasMinRole(client.Role(), permissions.RoleAdmin) {
+		allSkills = store.FilterVisibleSkills(ctx, allSkills)
+	}
+
 	result := make([]map[string]any, 0, len(allSkills))
 	for _, s := range allSkills {
 		entry := map[string]any{
@@ -116,6 +123,13 @@ func (m *SkillsMethods) handleGet(ctx context.Context, client *gateway.Client, r
 		return
 	}
 
+	// Visibility gate: hide private skills from non-owners (admins bypass).
+	if !permissions.HasMinRole(client.Role(), permissions.RoleAdmin) &&
+		!store.IsSkillVisibleTo(ctx, info.OwnerID, info.Visibility, info.IsSystem) {
+		client.SendResponse(protocol.NewErrorResponse(req.ID, protocol.ErrNotFound, i18n.T(locale, i18n.MsgNotFound, "skill", params.Name)))
+		return
+	}
+
 	content, _ := m.store.LoadSkill(ctx, params.Name)
 
 	resp := map[string]any{
@@ -196,8 +210,9 @@ func (m *SkillsMethods) handleUpdate(ctx context.Context, client *gateway.Client
 		return
 	}
 
-	// Ownership check: only skill owner or admin can update.
+	// Ownership check first: only skill owner or admin can update.
 	// Fail-closed: if store doesn't implement skillOwnerGetter, deny non-admin callers.
+	// Auth-before-validate avoids leaking skill-existence info via validation errors.
 	if !permissions.HasMinRole(client.Role(), permissions.RoleAdmin) {
 		ownerGetter, ok := m.store.(skillOwnerGetter)
 		if !ok {
@@ -210,6 +225,18 @@ func (m *SkillsMethods) handleUpdate(ctx context.Context, client *gateway.Client
 		}
 	}
 
+	// Validate visibility enum if present — fail closed before mutating the DB.
+	if v, ok := params.Updates["visibility"]; ok {
+		vs, _ := v.(string)
+		if err := skills.ValidateVisibility(vs); err != nil {
+			client.SendResponse(protocol.NewErrorResponse(req.ID, protocol.ErrInvalidRequest, i18n.T(locale, i18n.MsgInvalidVisibility, vs)))
+			return
+		}
+		if vs != "" {
+			params.Updates["visibility"] = skills.NormalizeVisibility(vs)
+		}
+	}
+
 	if err := updater.UpdateSkill(ctx, skillID, params.Updates); err != nil {
 		client.SendResponse(protocol.NewErrorResponse(req.ID, protocol.ErrInternal, err.Error()))
 		return

internal/i18n/catalog_en.go

@@ -113,6 +113,7 @@ func init() {
 		// Skills
 		MsgSkillsUpdateNotSupported: "skills.update not supported for file-based skills",
 		MsgCannotResolveSkillID:     "cannot resolve skill ID for file-based skill",
+		MsgInvalidVisibility:        "invalid visibility %q: must be one of private, public",
 
 		// Logs
 		MsgInvalidLogAction: "action must be 'start' or 'stop'",

internal/i18n/catalog_vi.go

@@ -113,6 +113,7 @@ func init() {
 		// Skills
 		MsgSkillsUpdateNotSupported: "skills.update không được hỗ trợ với skill dựa trên tệp",
 		MsgCannotResolveSkillID:     "không thể xác định ID skill dựa trên tệp",
+		MsgInvalidVisibility:        "visibility không hợp lệ %q: phải là private hoặc public",
 
 		// Logs
 		MsgInvalidLogAction: "action phải là 'start' hoặc 'stop'",

internal/i18n/catalog_zh.go

@@ -113,6 +113,7 @@ func init() {
 		// Skills
 		MsgSkillsUpdateNotSupported: "基于文件的Skill不支持 skills.update",
 		MsgCannotResolveSkillID:     "无法解析基于文件的Skill ID",
+		MsgInvalidVisibility:        "无效的 visibility %q：必须为 private 或 public",
 
 		// Logs
 		MsgInvalidLogAction: "action 必须是 'start' 或 'stop'",

internal/i18n/keys.go

@@ -114,6 +114,7 @@ const (
 	// --- Skills ---
 	MsgSkillsUpdateNotSupported = "error.skills_update_not_supported" // "skills.update not supported for file-based skills"
 	MsgCannotResolveSkillID     = "error.cannot_resolve_skill_id"     // "cannot resolve skill ID for file-based skill"
+	MsgInvalidVisibility        = "error.invalid_visibility"          // "invalid visibility %q: must be one of private, public"
 
 	// --- Logs ---
 	MsgInvalidLogAction = "error.invalid_log_action" // "action must be 'start' or 'stop'"

internal/skills/visibility.go

@@ -0,0 +1,51 @@
+package skills
+
+import (
+	"fmt"
+	"strings"
+)
+
+// Skill visibility values.
+const (
+	VisibilityPrivate = "private"
+	VisibilityPublic  = "public"
+)
+
+// DefaultVisibility is assigned when a caller does not specify one.
+// Private matches the historical hardcoded default and is the safer choice.
+const DefaultVisibility = VisibilityPrivate
+
+// validVisibilities enumerates the accepted enum values. System skills use
+// "public"; user-published skills default to "private".
+var validVisibilities = map[string]struct{}{
+	VisibilityPrivate: {},
+	VisibilityPublic:  {},
+}
+
+// NormalizeVisibility lowercases + trims the input and returns the default
+// when empty. It does not validate — pair with ValidateVisibility.
+func NormalizeVisibility(v string) string {
+	v = strings.ToLower(strings.TrimSpace(v))
+	if v == "" {
+		return DefaultVisibility
+	}
+	return v
+}
+
+// ValidateVisibility returns an error if v is not one of the supported enum
+// values. An empty string is treated as valid (caller applies the default).
+func ValidateVisibility(v string) error {
+	if v == "" {
+		return nil
+	}
+	if _, ok := validVisibilities[strings.ToLower(strings.TrimSpace(v))]; !ok {
+		return fmt.Errorf("invalid visibility %q: must be one of private, public", v)
+	}
+	return nil
+}
+
+// IsValidVisibility reports whether v is a recognized enum value. Empty is false.
+func IsValidVisibility(v string) bool {
+	_, ok := validVisibilities[strings.ToLower(strings.TrimSpace(v))]
+	return ok
+}

internal/skills/visibility_test.go

@@ -0,0 +1,41 @@
+package skills
+
+import "testing"
+
+func TestValidateVisibility(t *testing.T) {
+	tests := []struct {
+		name    string
+		input   string
+		wantErr bool
+	}{
+		{"empty ok (caller defaults)", "", false},
+		{"private", "private", false},
+		{"public", "public", false},
+		{"uppercase normalized", "PRIVATE", false},
+		{"whitespace normalized", "  public  ", false},
+		{"team rejected (v1 scope)", "team", true},
+		{"garbage rejected", "nope", true},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := ValidateVisibility(tt.input)
+			if (err != nil) != tt.wantErr {
+				t.Fatalf("ValidateVisibility(%q) err=%v, wantErr=%v", tt.input, err, tt.wantErr)
+			}
+		})
+	}
+}
+
+func TestNormalizeVisibility(t *testing.T) {
+	cases := map[string]string{
+		"":          DefaultVisibility,
+		"private":   "private",
+		"PUBLIC":    "public",
+		"  public ": "public",
+	}
+	for in, want := range cases {
+		if got := NormalizeVisibility(in); got != want {
+			t.Errorf("NormalizeVisibility(%q) = %q, want %q", in, got, want)
+		}
+	}
+}

internal/store/pg/skills.go

@@ -78,7 +78,7 @@ func (s *PGSkillStore) ListSkills(ctx context.Context) []store.SkillInfo {
 	// Tenant filter: system skills visible globally, custom skills scoped to tenant.
 	var scanned []skillInfoRowWithFrontmatter
 	if err := pkgSqlxDB.SelectContext(ctx, &scanned,
-		`SELECT id, name, slug, description, visibility, tags, version, is_system, status, enabled, deps, frontmatter, file_path
+		`SELECT id, name, slug, description, visibility, owner_id, tags, version, is_system, status, enabled, deps, frontmatter, file_path
 		 FROM skills WHERE (status IN ('active', 'archived') OR is_system = true) AND (is_system = true OR tenant_id = $1)
 		 ORDER BY name`, tid); err != nil {
 		return nil
@@ -105,7 +105,7 @@ func (s *PGSkillStore) ListAllSkills(ctx context.Context) []store.SkillInfo {
 	}
 	var scanned []skillInfoRow
 	if err := pkgSqlxDB.SelectContext(ctx, &scanned,
-		`SELECT id, name, slug, description, visibility, tags, version, is_system, status, enabled, deps, file_path
+		`SELECT id, name, slug, description, visibility, owner_id, tags, version, is_system, status, enabled, deps, file_path
 		 FROM skills WHERE enabled = true AND status != 'deleted' AND (is_system = true OR tenant_id = $1)
 		 ORDER BY name`, tid); err != nil {
 		return nil
@@ -118,7 +118,7 @@ func (s *PGSkillStore) ListAllSkills(ctx context.Context) []store.SkillInfo {
 func (s *PGSkillStore) ListAllSystemSkills(ctx context.Context) []store.SkillInfo {
 	var scanned []skillInfoRow
 	if err := pkgSqlxDB.SelectContext(ctx, &scanned,
-		`SELECT id, name, slug, description, visibility, tags, version, is_system, status, enabled, deps, file_path
+		`SELECT id, name, slug, description, visibility, owner_id, tags, version, is_system, status, enabled, deps, file_path
 		 FROM skills WHERE is_system = true AND enabled = true AND status != 'deleted'
 		 ORDER BY name`); err != nil {
 		return nil

internal/store/pg/skills_content.go

@@ -90,13 +90,13 @@ func (s *PGSkillStore) BuildSummary(ctx context.Context, allowList []string) str
 
 func (s *PGSkillStore) GetSkill(ctx context.Context, name string) (*store.SkillInfo, bool) {
 	var id uuid.UUID
-	var skillName, slug, visibility string
+	var skillName, slug, visibility, ownerID string
 	var desc *string
 	var tags []string
 	var version int
 	var isSystem bool
 	var filePath *string
-	q := "SELECT id, name, slug, description, visibility, tags, version, is_system, file_path FROM skills WHERE slug = $1 AND status = 'active'"
+	q := "SELECT id, name, slug, description, visibility, owner_id, tags, version, is_system, file_path FROM skills WHERE slug = $1 AND status = 'active'"
 	args := []any{name}
 	if !store.IsCrossTenant(ctx) {
 		tid := store.TenantIDFromContext(ctx)
@@ -106,12 +106,13 @@ func (s *PGSkillStore) GetSkill(ctx context.Context, name string) (*store.SkillI
 		q += " AND (is_system = true OR tenant_id = $2)"
 		args = append(args, tid)
 	}
-	err := s.db.QueryRowContext(ctx, q, args...).Scan(&id, &skillName, &slug, &desc, &visibility, pq.Array(&tags), &version, &isSystem, &filePath)
+	err := s.db.QueryRowContext(ctx, q, args...).Scan(&id, &skillName, &slug, &desc, &visibility, &ownerID, pq.Array(&tags), &version, &isSystem, &filePath)
 	if err != nil {
 		return nil, false
 	}
 	info := buildSkillInfo(id.String(), skillName, slug, desc, version, s.baseDir, filePath)
 	info.Visibility = visibility
+	info.OwnerID = ownerID
 	info.Tags = tags
 	info.IsSystem = isSystem
 	return &info, true
@@ -121,9 +122,11 @@ func (s *PGSkillStore) FilterSkills(ctx context.Context, allowList []string) []s
 	all := s.ListSkills(ctx)
 	var filtered []store.SkillInfo
 	if allowList == nil {
-		// No allowList → return all enabled skills (for agent injection)
+		// No allowList → return all enabled skills visible to the caller
+		// (for agent injection). Private skills owned by others are hidden
+		// so they don't leak across tenant members.
 		for _, sk := range all {
-			if sk.Enabled {
+			if sk.Enabled && store.IsSkillVisibleTo(ctx, sk.OwnerID, sk.Visibility, sk.IsSystem) {
 				filtered = append(filtered, sk)
 			}
 		}
@@ -148,14 +151,14 @@ func (s *PGSkillStore) FilterSkills(ctx context.Context, allowList []string) []s
 // Used by admin operations (e.g. toggle) that need full skill info.
 // Tenant filter: system skills visible globally, custom skills scoped to tenant.
 func (s *PGSkillStore) GetSkillByID(ctx context.Context, id uuid.UUID) (store.SkillInfo, bool) {
-	var name, slug, visibility, status string
+	var name, slug, visibility, ownerID, status string
 	var desc *string
 	var tags []string
 	var version int
 	var isSystem, enabled bool
 	var depsRaw []byte
 	var filePath *string
-	q := `SELECT name, slug, description, visibility, tags, version, is_system, status, enabled, deps, file_path
+	q := `SELECT name, slug, description, visibility, owner_id, tags, version, is_system, status, enabled, deps, file_path
 		 FROM skills WHERE id = $1`
 	args := []any{id}
 	if !store.IsCrossTenant(ctx) {
@@ -166,12 +169,13 @@ func (s *PGSkillStore) GetSkillByID(ctx context.Context, id uuid.UUID) (store.Sk
 		q += " AND (is_system = true OR tenant_id = $2)"
 		args = append(args, tid)
 	}
-	err := s.db.QueryRowContext(ctx, q, args...).Scan(&name, &slug, &desc, &visibility, pq.Array(&tags), &version, &isSystem, &status, &enabled, &depsRaw, &filePath)
+	err := s.db.QueryRowContext(ctx, q, args...).Scan(&name, &slug, &desc, &visibility, &ownerID, pq.Array(&tags), &version, &isSystem, &status, &enabled, &depsRaw, &filePath)
 	if err != nil {
 		return store.SkillInfo{}, false
 	}
 	info := buildSkillInfo(id.String(), name, slug, desc, version, s.baseDir, filePath)
 	info.Visibility = visibility
+	info.OwnerID = ownerID
 	info.Tags = tags
 	info.IsSystem = isSystem
 	info.Status = status

internal/store/pg/skills_scan_rows.go

@@ -18,6 +18,7 @@ type skillInfoRow struct {
 	Slug       string         `db:"slug"`
 	Desc       *string        `db:"description"`
 	Visibility string         `db:"visibility"`
+	OwnerID    string         `db:"owner_id"`
 	Tags       pq.StringArray `db:"tags"`
 	Version    int            `db:"version"`
 	IsSystem   bool           `db:"is_system"`
@@ -37,6 +38,7 @@ type skillInfoRowWithFrontmatter struct {
 func (r *skillInfoRow) toSkillInfo(baseDir string) store.SkillInfo {
 	info := buildSkillInfo(r.ID.String(), r.Name, r.Slug, r.Desc, r.Version, baseDir, r.FilePath)
 	info.Visibility = r.Visibility
+	info.OwnerID = r.OwnerID
 	info.Tags = []string(r.Tags)
 	info.IsSystem = r.IsSystem
 	info.Status = r.Status

internal/store/skill_store.go

@@ -16,6 +16,7 @@ type SkillInfo struct {
 	Source      string   `json:"source" db:"-"`
 	Description string   `json:"description" db:"description"`
 	Visibility  string   `json:"visibility,omitempty" db:"visibility"`
+	OwnerID     string   `json:"owner_id,omitempty" db:"owner_id"`
 	Tags        []string `json:"tags,omitempty" db:"tags"`
 	Version     int      `json:"version,omitempty" db:"version"`
 	IsSystem    bool     `json:"is_system,omitempty" db:"is_system"`

internal/store/sqlitestore/skills.go

@@ -71,7 +71,7 @@ func (s *SQLiteSkillStore) ListSkills(ctx context.Context) []store.SkillInfo {
 	s.mu.RUnlock()
 
 	rows, err := s.db.QueryContext(ctx,
-		`SELECT id, name, slug, description, visibility, tags, version, is_system, status, enabled, deps, frontmatter, file_path
+		`SELECT id, name, slug, description, visibility, owner_id, tags, version, is_system, status, enabled, deps, frontmatter, file_path
 		 FROM skills WHERE (status IN ('active', 'archived') OR is_system = 1) AND (is_system = 1 OR tenant_id = ?)
 		 ORDER BY name`, tid)
 	if err != nil {
@@ -82,19 +82,20 @@ func (s *SQLiteSkillStore) ListSkills(ctx context.Context) []store.SkillInfo {
 	var result []store.SkillInfo
 	for rows.Next() {
 		var id uuid.UUID
-		var name, slug, visibility, status string
+		var name, slug, visibility, ownerID, status string
 		var desc *string
 		var tagsJSON []byte
 		var version int
 		var isSystem, enabled bool
 		var depsRaw, fmRaw []byte
 		var filePath *string
-		if err := rows.Scan(&id, &name, &slug, &desc, &visibility, &tagsJSON, &version,
+		if err := rows.Scan(&id, &name, &slug, &desc, &visibility, &ownerID, &tagsJSON, &version,
 			&isSystem, &status, &enabled, &depsRaw, &fmRaw, &filePath); err != nil {
 			continue
 		}
 		info := buildSkillInfo(id.String(), name, slug, desc, version, s.baseDir, filePath)
 		info.Visibility = visibility
+		info.OwnerID = ownerID
 		scanJSONStringArray(tagsJSON, &info.Tags)
 		info.IsSystem = isSystem
 		info.Status = status