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

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

Closes #1009

- Add private/public visibility enum with validator + normalizer
  (internal/skills/visibility.go)
- Add IsSkillVisibleTo/FilterVisibleSkills authorization helper with
  three-identity ownership check (actor/user/sender) matching #915
- Propagate owner_id into SkillInfo and all PG/SQLite SELECTs so the
  filter has the data it needs
- Agent injection path (FilterSkills, nil allowList) now hides private
  skills owned by other users — fixes the leak vector across tenant
  members
- publish_skill: accept visibility param (defaults to private), replaces
  hardcoded literal
- skill_manage: visibility settable on create and editable via patch,
  including a content-less visibility-only patch that skips version bump
- skills.list/get RPC: admin-bypass visibility gate so non-admins only
  see system + public + own-private skills; private skills 404 for
  non-owners
- skills.update RPC: validate + normalize visibility enum before persist
  (fail closed on unknown values)

* fix(skills): address PR review — i18n error, normalize visibility, auth-first

- Add MsgInvalidVisibility i18n key (en/vi/zh) and use it in skills.update
  RPC instead of raw validator error text.
- Reorder skills.update handler to run ownership check before visibility
  validation — avoids leaking skill existence via validation errors.
- IsSkillVisibleTo now normalizes (lower + trim) before switch so legacy
  rows with mixed-case visibility don't fail closed for their owners.
- Extend TestIsSkillVisibleTo with uppercase/whitespace cases.

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

internal/store/sqlitestore/skills_content.go

@@ -85,13 +85,13 @@ func (s *SQLiteSkillStore) BuildSummary(ctx context.Context, allowList []string)
 
 func (s *SQLiteSkillStore) 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 tagsJSON []byte
 	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 = ? AND status = 'active'"
+	q := "SELECT id, name, slug, description, visibility, owner_id, tags, version, is_system, file_path FROM skills WHERE slug = ? AND status = 'active'"
 	args := []any{name}
 	if !store.IsCrossTenant(ctx) {
 		tid := store.TenantIDFromContext(ctx)
@@ -101,11 +101,12 @@ func (s *SQLiteSkillStore) GetSkill(ctx context.Context, name string) (*store.Sk
 		q += " AND (is_system = 1 OR tenant_id = ?)"
 		args = append(args, tid)
 	}
-	if err := s.db.QueryRowContext(ctx, q, args...).Scan(&id, &skillName, &slug, &desc, &visibility, &tagsJSON, &version, &isSystem, &filePath); err != nil {
+	if err := s.db.QueryRowContext(ctx, q, args...).Scan(&id, &skillName, &slug, &desc, &visibility, &ownerID, &tagsJSON, &version, &isSystem, &filePath); err != nil {
 		return nil, false
 	}
 	info := buildSkillInfo(id.String(), skillName, slug, desc, version, s.baseDir, filePath)
 	info.Visibility = visibility
+	info.OwnerID = ownerID
 	scanJSONStringArray(tagsJSON, &info.Tags)
 	info.IsSystem = isSystem
 	return &info, true
@@ -116,7 +117,7 @@ func (s *SQLiteSkillStore) FilterSkills(ctx context.Context, allowList []string)
 	var filtered []store.SkillInfo
 	if allowList == nil {
 		for _, sk := range all {
-			if sk.Enabled {
+			if sk.Enabled && store.IsSkillVisibleTo(ctx, sk.OwnerID, sk.Visibility, sk.IsSystem) {
 				filtered = append(filtered, sk)
 			}
 		}
@@ -139,13 +140,13 @@ func (s *SQLiteSkillStore) FilterSkills(ctx context.Context, allowList []string)
 
 // GetSkillByID returns a SkillInfo for any skill by UUID regardless of status.
 func (s *SQLiteSkillStore) 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 tagsJSON, depsRaw []byte
 	var version int
 	var isSystem, enabled bool
 	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 = ?`
 	args := []any{id}
 	if !store.IsCrossTenant(ctx) {
@@ -156,12 +157,13 @@ func (s *SQLiteSkillStore) GetSkillByID(ctx context.Context, id uuid.UUID) (stor
 		q += " AND (is_system = 1 OR tenant_id = ?)"
 		args = append(args, tid)
 	}
-	if err := s.db.QueryRowContext(ctx, q, args...).Scan(&name, &slug, &desc, &visibility, &tagsJSON,
+	if err := s.db.QueryRowContext(ctx, q, args...).Scan(&name, &slug, &desc, &visibility, &ownerID, &tagsJSON,
 		&version, &isSystem, &status, &enabled, &depsRaw, &filePath); err != nil {
 		return store.SkillInfo{}, false
 	}
 	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

internal/store/visibility_filter.go

@@ -0,0 +1,53 @@
+package store
+
+import (
+	"context"
+	"strings"
+)
+
+// IsSkillVisibleTo returns true if the caller identified by ctx can discover
+// the given skill. Rules:
+//   - System skills are visible to everyone.
+//   - Empty or "public" visibility is treated as public (legacy rows default
+//     to "public" for safety since older stores did not enforce the field).
+//   - "private" skills are only visible to the owner. Three identity strings
+//     are considered (actor, user, sender) to match the same identities
+//     isOwnerOfSkill checks for backward compatibility (#915).
+//
+// Admin/master-scope bypass is the caller's responsibility — this helper
+// reflects the non-privileged baseline.
+func IsSkillVisibleTo(ctx context.Context, ownerID, visibility string, isSystem bool) bool {
+	if isSystem {
+		return true
+	}
+	// Normalize to defend against historical rows with mixed case / whitespace
+	// that bypassed the write-path normalizer.
+	switch strings.ToLower(strings.TrimSpace(visibility)) {
+	case "", "public":
+		return true
+	case "private":
+		if ownerID == "" {
+			// No owner recorded — treat as public (historical data).
+			return true
+		}
+		actorID := ActorIDFromContext(ctx)
+		userID := UserIDFromContext(ctx)
+		senderID := SenderIDFromContext(ctx)
+		return ownerID == actorID || ownerID == userID || ownerID == senderID
+	default:
+		// Unknown enum value: fail closed (hide).
+		return false
+	}
+}
+
+// FilterVisibleSkills returns skills the caller can discover. Uses
+// IsSkillVisibleTo for each entry.
+func FilterVisibleSkills(ctx context.Context, skills []SkillInfo) []SkillInfo {
+	out := make([]SkillInfo, 0, len(skills))
+	for _, s := range skills {
+		if IsSkillVisibleTo(ctx, s.OwnerID, s.Visibility, s.IsSystem) {
+			out = append(out, s)
+		}
+	}
+	return out
+}

internal/store/visibility_filter_test.go

@@ -0,0 +1,62 @@
+package store
+
+import (
+	"context"
+	"testing"
+)
+
+func TestIsSkillVisibleTo(t *testing.T) {
+	alice := "alice"
+	bob := "bob"
+	ctx := WithUserID(context.Background(), alice)
+
+	tests := []struct {
+		name       string
+		owner      string
+		visibility string
+		isSystem   bool
+		want       bool
+	}{
+		{"system skill visible to anyone", "system", "private", true, true},
+		{"public visible to non-owner", bob, "public", false, true},
+		{"empty visibility treated as public", bob, "", false, true},
+		{"private visible to owner", alice, "private", false, true},
+		{"private hidden from non-owner", bob, "private", false, false},
+		{"private with no owner treated as public", "", "private", false, true},
+		{"unknown enum fails closed", bob, "team", false, false},
+		{"uppercase private matched for owner", alice, "PRIVATE", false, true},
+		{"whitespace public treated as public", bob, "  public  ", false, true},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := IsSkillVisibleTo(ctx, tt.owner, tt.visibility, tt.isSystem)
+			if got != tt.want {
+				t.Fatalf("IsSkillVisibleTo(owner=%q, vis=%q, sys=%v) = %v, want %v",
+					tt.owner, tt.visibility, tt.isSystem, got, tt.want)
+			}
+		})
+	}
+}
+
+func TestFilterVisibleSkills(t *testing.T) {
+	ctx := WithUserID(context.Background(), "alice")
+	skills := []SkillInfo{
+		{Slug: "sys", IsSystem: true, Visibility: "public"},
+		{Slug: "mine-private", OwnerID: "alice", Visibility: "private"},
+		{Slug: "theirs-private", OwnerID: "bob", Visibility: "private"},
+		{Slug: "theirs-public", OwnerID: "bob", Visibility: "public"},
+	}
+	got := FilterVisibleSkills(ctx, skills)
+	gotSlugs := map[string]bool{}
+	for _, s := range got {
+		gotSlugs[s.Slug] = true
+	}
+	for _, want := range []string{"sys", "mine-private", "theirs-public"} {
+		if !gotSlugs[want] {
+			t.Errorf("expected %q in filtered output, got %v", want, gotSlugs)
+		}
+	}
+	if gotSlugs["theirs-private"] {
+		t.Errorf("leaked private skill to non-owner: %v", gotSlugs)
+	}
+}

internal/tools/publish_skill.go

@@ -56,6 +56,11 @@ func (t *PublishSkillTool) Parameters() map[string]any {
 				"type":        "string",
 				"description": "Path to skill directory containing SKILL.md (absolute or relative to workspace)",
 			},
+			"visibility": map[string]any{
+				"type":        "string",
+				"enum":        []string{skills.VisibilityPrivate, skills.VisibilityPublic},
+				"description": "Who can discover this skill. 'private' (default) is visible only to the owner; 'public' is visible to anyone in the tenant.",
+			},
 		},
 		"required": []string{"path"},
 	}
@@ -67,6 +72,12 @@ func (t *PublishSkillTool) Execute(ctx context.Context, args map[string]any) *Re
 		return ErrorResult("path is required")
 	}
 
+	rawVisibility, _ := args["visibility"].(string)
+	if err := skills.ValidateVisibility(rawVisibility); err != nil {
+		return ErrorResult(err.Error())
+	}
+	visibility := skills.NormalizeVisibility(rawVisibility)
+
 	// Resolve path: absolute or relative to workspace
 	dir := rawPath
 	if !filepath.IsAbs(dir) {
@@ -141,7 +152,7 @@ func (t *PublishSkillTool) Execute(ctx context.Context, args map[string]any) *Re
 		Slug:        slug,
 		Description: &desc,
 		OwnerID:     ownerID,
-		Visibility:  "private",
+		Visibility:  visibility,
 		Version:     version,
 		FilePath:    destDir,
 		FileSize:    fileSize,

internal/tools/skill_manage.go

@@ -87,12 +87,17 @@ func (t *SkillManageTool) Parameters() map[string]any {
 			},
 			"find": map[string]any{
 				"type":        "string",
-				"description": "Exact text to find in the current SKILL.md. Required for patch.",
+				"description": "Exact text to find in the current SKILL.md. Required for patch unless only 'visibility' is being updated.",
 			},
 			"replace": map[string]any{
 				"type":        "string",
 				"description": "Replacement text. Required for patch.",
 			},
+			"visibility": map[string]any{
+				"type":        "string",
+				"enum":        []string{skills.VisibilityPrivate, skills.VisibilityPublic},
+				"description": "Skill visibility. For create: defaults to 'private'. For patch: updates who can discover the skill without creating a new version.",
+			},
 		},
 		"required": []string{"action"},
 	}
@@ -125,6 +130,12 @@ func (t *SkillManageTool) executeCreate(ctx context.Context, args map[string]any
 		return ErrorResult(fmt.Sprintf("content too large (%d bytes, max %d)", len(content), maxSkillContentSize))
 	}
 
+	rawVisibility, _ := args["visibility"].(string)
+	if err := skills.ValidateVisibility(rawVisibility); err != nil {
+		return ErrorResult(err.Error())
+	}
+	visibility := skills.NormalizeVisibility(rawVisibility)
+
 	// Security scan before any disk write
 	violations, safe := skills.GuardSkillContent(content)
 	if !safe {
@@ -183,7 +194,7 @@ func (t *SkillManageTool) executeCreate(ctx context.Context, args map[string]any
 		Slug:        slug,
 		Description: &desc,
 		OwnerID:     ownerID,
-		Visibility:  "private",
+		Visibility:  visibility,
 		Version:     version,
 		FilePath:    destDir,
 		FileSize:    fileSize,
@@ -238,11 +249,16 @@ func (t *SkillManageTool) executePatch(ctx context.Context, args map[string]any)
 	slug, _ := args["slug"].(string)
 	find, _ := args["find"].(string)
 	replace, _ := args["replace"].(string)
+	rawVisibility, _ := args["visibility"].(string)
 	if slug == "" {
 		return ErrorResult("slug is required for action=patch")
 	}
-	if find == "" {
-		return ErrorResult("find is required for action=patch")
+	if err := skills.ValidateVisibility(rawVisibility); err != nil {
+		return ErrorResult(err.Error())
+	}
+	// Patch requires at least one of: content edit (find) or visibility change.
+	if find == "" && rawVisibility == "" {
+		return ErrorResult("patch requires either 'find' (content edit) or 'visibility' (metadata update)")
 	}
 
 	info, ok := t.skills.GetSkill(ctx, slug)
@@ -266,6 +282,26 @@ func (t *SkillManageTool) executePatch(ctx context.Context, args map[string]any)
 		return ErrorResult(fmt.Sprintf("cannot manage skill %q: you are not the owner", slug))
 	}
 
+	// Visibility-only patch path: no content change, no new version.
+	if find == "" && rawVisibility != "" {
+		skillID, err := uuid.Parse(info.ID)
+		if err != nil {
+			return ErrorResult(fmt.Sprintf("invalid skill ID in database: %v", err))
+		}
+		newVisibility := skills.NormalizeVisibility(rawVisibility)
+		if err := t.skills.UpdateSkill(ctx, skillID, map[string]any{
+			"visibility": newVisibility,
+			"updated_at": time.Now(),
+		}); err != nil {
+			return ErrorResult(fmt.Sprintf("failed to update skill visibility: %v", err))
+		}
+		slog.Info("skill_manage: visibility updated", "slug", slug, "visibility", newVisibility)
+		if t.loader != nil {
+			t.loader.BumpVersion()
+		}
+		return NewResult(fmt.Sprintf("Skill %q visibility set to %s.", slug, newVisibility))
+	}
+
 	// Read current SKILL.md from latest version
 	current, err := os.ReadFile(info.Path)
 	if err != nil {
@@ -316,13 +352,17 @@ func (t *SkillManageTool) executePatch(ctx context.Context, args map[string]any)
 	if err != nil {
 		return ErrorResult(fmt.Sprintf("invalid skill ID in database: %v", err))
 	}
-	if err := t.skills.UpdateSkill(ctx, skillID, map[string]any{
+	updates := map[string]any{
 		"version":    newVer,
 		"file_path":  destDir,
 		"file_size":  fileSize,
 		"file_hash":  &fileHash,
 		"updated_at": time.Now(),
-	}); err != nil {
+	}
+	if rawVisibility != "" {
+		updates["visibility"] = skills.NormalizeVisibility(rawVisibility)
+	}
+	if err := t.skills.UpdateSkill(ctx, skillID, updates); err != nil {
 		return ErrorResult(fmt.Sprintf("failed to update skill in database: %v", err))
 	}