diff --git a/docs/project-changelog.md b/docs/project-changelog.md index 596fc922..e5fcd321 100644 --- a/docs/project-changelog.md +++ b/docs/project-changelog.md @@ -6,6 +6,24 @@ Significant changes, features, and fixes in reverse chronological order. ## 2026-05-17 +### Skills: agent manage grants + +**Fixes** + +- Added explicit per-agent skill manage grants so agents can edit/delete skills they were authorized to maintain even when `owner_id` no longer matches their current actor identity. +- Auto-granted manage permission to the creating/publishing agent for new managed skills. + +**UI** + +- Show custom skill owner IDs in the Skills table. +- Added Skills page controls to grant agent skill access and edit/delete permission. + +**Tests** + +- Added PG/SQLite grant coverage and verified Go builds plus Web UI build. + +--- + ### Agents: provider switch save fix **Fixes** diff --git a/internal/gateway/methods/skills.go b/internal/gateway/methods/skills.go index ac5349d1..3d2caf1f 100644 --- a/internal/gateway/methods/skills.go +++ b/internal/gateway/methods/skills.go @@ -17,7 +17,7 @@ import ( // skillOwnerGetter is an optional interface for stores that can return a skill's owner ID. type skillOwnerGetter interface { - GetSkillOwnerID(id uuid.UUID) (string, bool) + GetSkillOwnerID(ctx context.Context, id uuid.UUID) (string, bool) } // SkillsMethods handles skills.list, skills.get, skills.update. @@ -56,6 +56,9 @@ func (m *SkillsMethods) handleList(ctx context.Context, client *gateway.Client, "is_system": s.IsSystem, "enabled": s.Enabled, } + if s.OwnerID != "" { + entry["owner_id"] = s.OwnerID + } if s.ID != "" { entry["id"] = s.ID } @@ -146,6 +149,9 @@ func (m *SkillsMethods) handleGet(ctx context.Context, client *gateway.Client, r if info.Visibility != "" { resp["visibility"] = info.Visibility } + if info.OwnerID != "" { + resp["owner_id"] = info.OwnerID + } if len(info.Tags) > 0 { resp["tags"] = info.Tags } @@ -219,7 +225,7 @@ func (m *SkillsMethods) handleUpdate(ctx context.Context, client *gateway.Client client.SendResponse(protocol.NewErrorResponse(req.ID, protocol.ErrUnauthorized, i18n.T(locale, i18n.MsgPermissionDenied, "skills.update"))) return } - if ownerID, found := ownerGetter.GetSkillOwnerID(skillID); found && ownerID != client.UserID() { + if ownerID, found := ownerGetter.GetSkillOwnerID(ctx, skillID); found && ownerID != client.UserID() { client.SendResponse(protocol.NewErrorResponse(req.ID, protocol.ErrUnauthorized, i18n.T(locale, i18n.MsgPermissionDenied, "skills.update"))) return } diff --git a/internal/http/skills.go b/internal/http/skills.go index 52078ca3..ee1e17dc 100644 --- a/internal/http/skills.go +++ b/internal/http/skills.go @@ -37,7 +37,7 @@ type SkillsHandler struct { msgBus *bus.MessageBus tenantCfgStore store.SkillTenantConfigStore tenantStore store.TenantStore - db *sql.DB // for export/import direct queries + db *sql.DB // for export/import direct queries uploadLocks sync.Map // per-slug mutex; bounded by validated slug set, entries are tiny (*sync.Mutex) } @@ -92,6 +92,7 @@ func (h *SkillsHandler) RegisterRoutes(mux *http.ServeMux) { mux.HandleFunc("PUT /v1/skills/{id}", h.adminMiddleware(h.handleUpdate)) mux.HandleFunc("DELETE /v1/skills/{id}", h.adminMiddleware(h.handleDelete)) // Skill grants (admin+) + mux.HandleFunc("GET /v1/skills/{id}/grants/agent", h.adminMiddleware(h.handleListAgentGrants)) mux.HandleFunc("POST /v1/skills/{id}/grants/agent", h.adminMiddleware(h.handleGrantAgent)) mux.HandleFunc("DELETE /v1/skills/{id}/grants/agent/{agentID}", h.adminMiddleware(h.handleRevokeAgent)) mux.HandleFunc("POST /v1/skills/{id}/grants/user", h.adminMiddleware(h.handleGrantUser)) diff --git a/internal/http/skills_grants.go b/internal/http/skills_grants.go index d359b94b..eccebcad 100644 --- a/internal/http/skills_grants.go +++ b/internal/http/skills_grants.go @@ -33,6 +33,25 @@ func (h *SkillsHandler) handleListAgentSkills(w http.ResponseWriter, r *http.Req writeJSON(w, http.StatusOK, map[string]any{"skills": skills}) } +func (h *SkillsHandler) handleListAgentGrants(w http.ResponseWriter, r *http.Request) { + locale := store.LocaleFromContext(r.Context()) + idStr := r.PathValue("id") + skillID, err := uuid.Parse(idStr) + if err != nil { + writeJSON(w, http.StatusBadRequest, map[string]string{"error": i18n.T(locale, i18n.MsgInvalidID, "skill")}) + return + } + + grants, err := h.skills.ListAgentGrantsForSkill(r.Context(), skillID) + if err != nil { + slog.Error("failed to list skill agent grants", "skill_id", skillID, "error", err) + writeJSON(w, http.StatusInternalServerError, map[string]string{"error": i18n.T(locale, i18n.MsgFailedToList, "skill grants")}) + return + } + + writeJSON(w, http.StatusOK, map[string]any{"grants": grants}) +} + func (h *SkillsHandler) handleGrantAgent(w http.ResponseWriter, r *http.Request) { locale := store.LocaleFromContext(r.Context()) userID := store.UserIDFromContext(r.Context()) @@ -53,8 +72,9 @@ func (h *SkillsHandler) handleGrantAgent(w http.ResponseWriter, r *http.Request) } var req struct { - AgentID string `json:"agent_id"` - Version int `json:"version"` + AgentID string `json:"agent_id"` + Version int `json:"version"` + CanManage *bool `json:"can_manage"` } if !bindJSON(w, r, locale, &req) { return @@ -70,8 +90,14 @@ func (h *SkillsHandler) handleGrantAgent(w http.ResponseWriter, r *http.Request) req.Version = 1 } - if err := h.skills.GrantToAgent(r.Context(), skillID, agentID, req.Version, userID); err != nil { - writeJSON(w, http.StatusInternalServerError, map[string]string{"error": err.Error()}) + var grantErr error + if req.CanManage == nil { + grantErr = h.skills.GrantToAgent(r.Context(), skillID, agentID, req.Version, userID) + } else { + grantErr = h.skills.GrantToAgent(r.Context(), skillID, agentID, req.Version, userID, *req.CanManage) + } + if grantErr != nil { + writeJSON(w, http.StatusInternalServerError, map[string]string{"error": grantErr.Error()}) return } diff --git a/internal/http/skills_upload_test.go b/internal/http/skills_upload_test.go index f8dca60a..f8ae2935 100644 --- a/internal/http/skills_upload_test.go +++ b/internal/http/skills_upload_test.go @@ -528,7 +528,7 @@ func (s *skillManageStoreStub) StoreMissingDeps(_ context.Context, id uuid.UUID, s.skills[id] = skill return nil } -func (s *skillManageStoreStub) GrantToAgent(context.Context, uuid.UUID, uuid.UUID, int, string) error { +func (s *skillManageStoreStub) GrantToAgent(context.Context, uuid.UUID, uuid.UUID, int, string, ...bool) error { return nil } func (s *skillManageStoreStub) RevokeFromAgent(context.Context, uuid.UUID, uuid.UUID) error { @@ -541,6 +541,12 @@ func (s *skillManageStoreStub) RevokeFromUser(context.Context, uuid.UUID, string func (s *skillManageStoreStub) ListWithGrantStatus(context.Context, uuid.UUID) ([]store.SkillWithGrantStatus, error) { return nil, nil } +func (s *skillManageStoreStub) ListAgentGrantsForSkill(context.Context, uuid.UUID) ([]store.SkillAgentGrantInfo, error) { + return nil, nil +} +func (s *skillManageStoreStub) AgentCanManageSkill(context.Context, uuid.UUID, uuid.UUID) (bool, error) { + return false, nil +} func (s *skillManageStoreStub) GetSkillFilePath(context.Context, uuid.UUID) (string, string, int, bool, bool) { return "", "", 0, false, false } diff --git a/internal/store/pg/skills_grants.go b/internal/store/pg/skills_grants.go index 5a42f698..d2c2990c 100644 --- a/internal/store/pg/skills_grants.go +++ b/internal/store/pg/skills_grants.go @@ -2,6 +2,7 @@ package pg import ( "context" + "database/sql" "fmt" "log/slog" "time" @@ -15,12 +16,11 @@ import ( // Auto-promotes visibility from 'private' to 'internal' so the skill // becomes accessible via ListAccessible for granted agents. // Validates the agent belongs to the requesting tenant (prevents cross-tenant grant injection). -func (s *PGSkillStore) GrantToAgent(ctx context.Context, skillID, agentID uuid.UUID, version int, grantedBy string) error { +func (s *PGSkillStore) GrantToAgent(ctx context.Context, skillID, agentID uuid.UUID, version int, grantedBy string, canManage ...bool) error { if err := store.ValidateUserID(grantedBy); err != nil { return err } tid := tenantIDForInsert(ctx) - // Verify agent belongs to the requesting tenant. var agentTenantID uuid.UUID if err := s.db.QueryRowContext(ctx, @@ -32,12 +32,28 @@ func (s *PGSkillStore) GrantToAgent(ctx context.Context, skillID, agentID uuid.U return fmt.Errorf("agent not found") } - _, err := s.db.ExecContext(ctx, - `INSERT INTO skill_agent_grants (id, skill_id, agent_id, pinned_version, granted_by, created_at, tenant_id) - VALUES ($1, $2, $3, $4, $5, $6, $7) - ON CONFLICT (skill_id, agent_id) DO UPDATE SET pinned_version = EXCLUDED.pinned_version`, - store.GenNewID(), skillID, agentID, version, grantedBy, time.Now(), tid, - ) + now := time.Now() + var err error + if len(canManage) > 0 { + _, err = s.db.ExecContext(ctx, + `INSERT INTO skill_agent_grants (id, skill_id, agent_id, pinned_version, granted_by, can_manage, created_at, tenant_id) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8) + ON CONFLICT (skill_id, agent_id) DO UPDATE SET + pinned_version = EXCLUDED.pinned_version, + granted_by = EXCLUDED.granted_by, + can_manage = EXCLUDED.can_manage`, + store.GenNewID(), skillID, agentID, version, grantedBy, canManage[0], now, tid, + ) + } else { + _, err = s.db.ExecContext(ctx, + `INSERT INTO skill_agent_grants (id, skill_id, agent_id, pinned_version, granted_by, created_at, tenant_id) + VALUES ($1, $2, $3, $4, $5, $6, $7) + ON CONFLICT (skill_id, agent_id) DO UPDATE SET + pinned_version = EXCLUDED.pinned_version, + granted_by = EXCLUDED.granted_by`, + store.GenNewID(), skillID, agentID, version, grantedBy, now, tid, + ) + } if err != nil { return err } @@ -101,6 +117,41 @@ func (s *PGSkillStore) ListAgentGrants(ctx context.Context, agentID uuid.UUID) ( return result, nil } +// ListAgentGrantsForSkill returns all agent grants for one skill. +func (s *PGSkillStore) ListAgentGrantsForSkill(ctx context.Context, skillID uuid.UUID) ([]store.SkillAgentGrantInfo, error) { + tClause, tArgs, _, err := scopeClause(ctx, 2) + if err != nil { + return nil, err + } + var result []store.SkillAgentGrantInfo + err = pkgSqlxDB.SelectContext(ctx, &result, + "SELECT agent_id, pinned_version, granted_by, can_manage FROM skill_agent_grants WHERE skill_id = $1"+tClause+" ORDER BY created_at DESC", + append([]any{skillID}, tArgs...)...) + if err != nil { + return nil, err + } + return result, nil +} + +// AgentCanManageSkill reports whether an agent has explicit edit/delete rights for a skill. +func (s *PGSkillStore) AgentCanManageSkill(ctx context.Context, skillID, agentID uuid.UUID) (bool, error) { + tClause, tArgs, _, err := scopeClause(ctx, 3) + if err != nil { + return false, err + } + var canManage bool + err = s.db.QueryRowContext(ctx, + "SELECT can_manage FROM skill_agent_grants WHERE skill_id = $1 AND agent_id = $2"+tClause, + append([]any{skillID, agentID}, tArgs...)...).Scan(&canManage) + if err == sql.ErrNoRows { + return false, nil + } + if err != nil { + return false, err + } + return canManage, nil +} + // GrantToUser grants a skill to a user (for internal visibility skills). func (s *PGSkillStore) GrantToUser(ctx context.Context, skillID uuid.UUID, userID, grantedBy string) error { if err := store.ValidateUserID(userID); err != nil { @@ -216,6 +267,7 @@ func (s *PGSkillStore) ListWithGrantStatus(ctx context.Context, agentID uuid.UUI rows, err := s.db.QueryContext(ctx, `SELECT s.id, s.name, s.slug, COALESCE(s.description, ''), s.visibility, s.version, (sag.id IS NOT NULL) AS granted, + COALESCE(sag.can_manage, false) AS can_manage, sag.pinned_version, s.is_system FROM skills s @@ -230,7 +282,7 @@ func (s *PGSkillStore) ListWithGrantStatus(ctx context.Context, agentID uuid.UUI var result []store.SkillWithGrantStatus for rows.Next() { var r store.SkillWithGrantStatus - if err := rows.Scan(&r.ID, &r.Name, &r.Slug, &r.Description, &r.Visibility, &r.Version, &r.Granted, &r.PinnedVer, &r.IsSystem); err != nil { + if err := rows.Scan(&r.ID, &r.Name, &r.Slug, &r.Description, &r.Visibility, &r.Version, &r.Granted, &r.CanManage, &r.PinnedVer, &r.IsSystem); err != nil { slog.Warn("skill_grants: scan error in ListWithGrantStatus", "error", err) continue } diff --git a/internal/store/skill_store.go b/internal/store/skill_store.go index 478725ee..4a976acf 100644 --- a/internal/store/skill_store.go +++ b/internal/store/skill_store.go @@ -89,10 +89,19 @@ type SkillWithGrantStatus struct { Visibility string `json:"visibility" db:"visibility"` Version int `json:"version" db:"version"` Granted bool `json:"granted" db:"granted"` + CanManage bool `json:"can_manage" db:"can_manage"` PinnedVer *int `json:"pinned_version,omitempty" db:"pinned_version"` IsSystem bool `json:"is_system" db:"is_system"` } +// SkillAgentGrantInfo is a grant row for one skill across agents. +type SkillAgentGrantInfo struct { + AgentID uuid.UUID `json:"agent_id" db:"agent_id"` + PinnedVersion int `json:"pinned_version" db:"pinned_version"` + GrantedBy string `json:"granted_by" db:"granted_by"` + CanManage bool `json:"can_manage" db:"can_manage"` +} + // SkillManageStore extends SkillStore with CRUD, ownership, and grant operations // needed by HTTP upload handlers and agent tools (skill_manage, publish_skill). // Implemented by both PGSkillStore and SQLiteSkillStore. @@ -119,11 +128,13 @@ type SkillManageStore interface { ListSystemSkillDirs(ctx context.Context) map[string]string StoreMissingDeps(ctx context.Context, id uuid.UUID, missing []string) error // Grants - GrantToAgent(ctx context.Context, skillID, agentID uuid.UUID, version int, grantedBy string) error + GrantToAgent(ctx context.Context, skillID, agentID uuid.UUID, version int, grantedBy string, canManage ...bool) error RevokeFromAgent(ctx context.Context, skillID, agentID uuid.UUID) error GrantToUser(ctx context.Context, skillID uuid.UUID, userID, grantedBy string) error RevokeFromUser(ctx context.Context, skillID uuid.UUID, userID string) error ListWithGrantStatus(ctx context.Context, agentID uuid.UUID) ([]SkillWithGrantStatus, error) + ListAgentGrantsForSkill(ctx context.Context, skillID uuid.UUID) ([]SkillAgentGrantInfo, error) + AgentCanManageSkill(ctx context.Context, skillID, agentID uuid.UUID) (bool, error) // Files GetSkillFilePath(ctx context.Context, id uuid.UUID) (filePath string, slug string, version int, isSystem bool, ok bool) } diff --git a/internal/store/sqlitestore/schema.go b/internal/store/sqlitestore/schema.go index b06ba9b3..208bae97 100644 --- a/internal/store/sqlitestore/schema.go +++ b/internal/store/sqlitestore/schema.go @@ -16,7 +16,7 @@ var schemaSQL string // SchemaVersion is the current SQLite schema version. // Bump this when adding new migration steps below. -const SchemaVersion = 34 +const SchemaVersion = 35 // migrations maps version → SQL to apply when upgrading FROM that version. // schema.sql always represents the LATEST full schema (for fresh DBs). @@ -596,6 +596,9 @@ CREATE INDEX IF NOT EXISTS idx_ws_activity_retention ON workstation_activity(c // Version 33 → 34: per-agent ordered provider/model fallback config. 33: `ALTER TABLE agents ADD COLUMN model_fallback TEXT NOT NULL DEFAULT '{}';`, + // Version 34 → 35: agent skill grants can optionally allow skill management. + 34: `ALTER TABLE skill_agent_grants ADD COLUMN can_manage INTEGER NOT NULL DEFAULT 0;`, + // Version 23 → 24: vault_documents scope/ownership consistency triggers. // Mirrors PG migration 000055 CHECK constraint; SQLite cannot add CHECK via // ALTER TABLE so we use BEFORE INSERT + BEFORE UPDATE triggers instead. @@ -975,6 +978,8 @@ func idempotentColumnMigration(version int) (string, string, bool) { return "webhooks", "encrypted_secret", true case 33: return "agents", "model_fallback", true + case 34: + return "skill_agent_grants", "can_manage", true default: return "", "", false } diff --git a/internal/store/sqlitestore/schema.sql b/internal/store/sqlitestore/schema.sql index b3d749c5..553e71ee 100644 --- a/internal/store/sqlitestore/schema.sql +++ b/internal/store/sqlitestore/schema.sql @@ -408,6 +408,7 @@ CREATE TABLE IF NOT EXISTS skill_agent_grants ( agent_id TEXT NOT NULL REFERENCES agents(id) ON DELETE CASCADE, pinned_version INT NOT NULL, granted_by VARCHAR(255) NOT NULL, + can_manage INTEGER NOT NULL DEFAULT 0, tenant_id TEXT NOT NULL REFERENCES tenants(id), created_at TEXT DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')), UNIQUE(skill_id, agent_id) diff --git a/internal/store/sqlitestore/skills_grants.go b/internal/store/sqlitestore/skills_grants.go index 344189c8..bf2bba9d 100644 --- a/internal/store/sqlitestore/skills_grants.go +++ b/internal/store/sqlitestore/skills_grants.go @@ -4,6 +4,7 @@ package sqlitestore import ( "context" + "database/sql" "log/slog" "time" @@ -20,19 +21,35 @@ type SkillGrantInfo struct { } // GrantToAgent grants a skill to an agent with version pinning. -func (s *SQLiteSkillStore) GrantToAgent(ctx context.Context, skillID, agentID uuid.UUID, version int, grantedBy string) error { +func (s *SQLiteSkillStore) GrantToAgent(ctx context.Context, skillID, agentID uuid.UUID, version int, grantedBy string, canManage ...bool) error { if err := store.ValidateUserID(grantedBy); err != nil { return err } - // Upsert grant. id := store.GenNewID() - _, err := s.db.ExecContext(ctx, - `INSERT INTO skill_agent_grants (id, skill_id, agent_id, pinned_version, granted_by, created_at, tenant_id) - VALUES (?, ?, ?, ?, ?, ?, ?) - ON CONFLICT (skill_id, agent_id) DO UPDATE SET pinned_version = excluded.pinned_version`, - id, skillID, agentID, version, grantedBy, time.Now().UTC(), tenantIDForInsert(ctx), - ) + now := time.Now().UTC() + tid := tenantIDForInsert(ctx) + var err error + if len(canManage) > 0 { + _, err = s.db.ExecContext(ctx, + `INSERT INTO skill_agent_grants (id, skill_id, agent_id, pinned_version, granted_by, can_manage, created_at, tenant_id) + VALUES (?, ?, ?, ?, ?, ?, ?, ?) + ON CONFLICT (skill_id, agent_id) DO UPDATE SET + pinned_version = excluded.pinned_version, + granted_by = excluded.granted_by, + can_manage = excluded.can_manage`, + id, skillID, agentID, version, grantedBy, canManage[0], now, tid, + ) + } else { + _, err = s.db.ExecContext(ctx, + `INSERT INTO skill_agent_grants (id, skill_id, agent_id, pinned_version, granted_by, created_at, tenant_id) + VALUES (?, ?, ?, ?, ?, ?, ?) + ON CONFLICT (skill_id, agent_id) DO UPDATE SET + pinned_version = excluded.pinned_version, + granted_by = excluded.granted_by`, + id, skillID, agentID, version, grantedBy, now, tid, + ) + } if err != nil { return err } @@ -102,6 +119,51 @@ func (s *SQLiteSkillStore) ListAgentGrants(ctx context.Context, agentID uuid.UUI return result, rows.Err() } +// ListAgentGrantsForSkill returns all agent grants for one skill. +func (s *SQLiteSkillStore) ListAgentGrantsForSkill(ctx context.Context, skillID uuid.UUID) ([]store.SkillAgentGrantInfo, error) { + tClause, tArgs, err := scopeClause(ctx) + if err != nil { + return nil, err + } + rows, err := s.db.QueryContext(ctx, + "SELECT agent_id, pinned_version, granted_by, can_manage FROM skill_agent_grants WHERE skill_id = ?"+tClause+" ORDER BY created_at DESC", + append([]any{skillID}, tArgs...)...) + if err != nil { + return nil, err + } + defer rows.Close() + + var result []store.SkillAgentGrantInfo + for rows.Next() { + var g store.SkillAgentGrantInfo + if err := rows.Scan(&g.AgentID, &g.PinnedVersion, &g.GrantedBy, &g.CanManage); err != nil { + slog.Warn("skill_grants: scan error in ListAgentGrantsForSkill", "error", err) + continue + } + result = append(result, g) + } + return result, rows.Err() +} + +// AgentCanManageSkill reports whether an agent has explicit edit/delete rights for a skill. +func (s *SQLiteSkillStore) AgentCanManageSkill(ctx context.Context, skillID, agentID uuid.UUID) (bool, error) { + tClause, tArgs, err := scopeClause(ctx) + if err != nil { + return false, err + } + var canManage bool + err = s.db.QueryRowContext(ctx, + "SELECT can_manage FROM skill_agent_grants WHERE skill_id = ? AND agent_id = ?"+tClause, + append([]any{skillID, agentID}, tArgs...)...).Scan(&canManage) + if err == sql.ErrNoRows { + return false, nil + } + if err != nil { + return false, err + } + return canManage, nil +} + // GrantToUser grants a skill to a user. func (s *SQLiteSkillStore) GrantToUser(ctx context.Context, skillID uuid.UUID, userID, grantedBy string) error { if err := store.ValidateUserID(userID); err != nil { @@ -214,6 +276,7 @@ func (s *SQLiteSkillStore) ListWithGrantStatus(ctx context.Context, agentID uuid rows, err := s.db.QueryContext(ctx, `SELECT s.id, s.name, s.slug, COALESCE(s.description, ''), s.visibility, s.version, (sag.id IS NOT NULL) AS granted, + COALESCE(sag.can_manage, 0) AS can_manage, sag.pinned_version, s.is_system FROM skills s @@ -229,7 +292,7 @@ func (s *SQLiteSkillStore) ListWithGrantStatus(ctx context.Context, agentID uuid for rows.Next() { var r store.SkillWithGrantStatus if err := rows.Scan(&r.ID, &r.Name, &r.Slug, &r.Description, &r.Visibility, - &r.Version, &r.Granted, &r.PinnedVer, &r.IsSystem); err != nil { + &r.Version, &r.Granted, &r.CanManage, &r.PinnedVer, &r.IsSystem); err != nil { slog.Warn("skill_grants: scan error in ListWithGrantStatus", "error", err) continue } diff --git a/internal/tools/publish_skill.go b/internal/tools/publish_skill.go index dd3f50a7..227033ce 100644 --- a/internal/tools/publish_skill.go +++ b/internal/tools/publish_skill.go @@ -170,7 +170,7 @@ func (t *PublishSkillTool) Execute(ctx context.Context, args map[string]any) *Re // Auto-grant to calling agent (granted-by = owner, same as CreateSkillManaged) agentID := store.AgentIDFromContext(ctx) if agentID != uuid.Nil { - if err := t.skills.GrantToAgent(ctx, id, agentID, version, ownerID); err != nil { + if err := t.skills.GrantToAgent(ctx, id, agentID, version, ownerID, true); err != nil { slog.Warn("publish_skill: auto-grant failed", "error", err) } } diff --git a/internal/tools/skill_manage.go b/internal/tools/skill_manage.go index 16b70545..aabcd6b1 100644 --- a/internal/tools/skill_manage.go +++ b/internal/tools/skill_manage.go @@ -51,6 +51,29 @@ func isOwnerOfSkill(ctx context.Context, skills store.SkillManageStore, slug str return ownerID == actorID || ownerID == userID || ownerID == senderID } +func canManageSkill(ctx context.Context, skills store.SkillManageStore, info *store.SkillInfo) bool { + if isOwnerOfSkill(ctx, skills, info.Slug) { + return true + } + if info.ID == "" { + return false + } + skillID, err := uuid.Parse(info.ID) + if err != nil { + return false + } + agentID := store.AgentIDFromContext(ctx) + if agentID == uuid.Nil { + return false + } + ok, err := skills.AgentCanManageSkill(ctx, skillID, agentID) + if err != nil { + slog.Warn("skill_manage: manage grant check failed", "skill", info.Slug, "agent_id", agentID, "error", err) + return false + } + return ok +} + // tenantSkillsDir returns the skills-store directory scoped to the calling agent's tenant. func (t *SkillManageTool) tenantSkillsDir(ctx context.Context) string { tid := store.TenantIDFromContext(ctx) @@ -211,7 +234,7 @@ func (t *SkillManageTool) executeCreate(ctx context.Context, args map[string]any granted := false agentID := store.AgentIDFromContext(ctx) if agentID != uuid.Nil { - if err := t.skills.GrantToAgent(ctx, id, agentID, version, ownerID); err != nil { + if err := t.skills.GrantToAgent(ctx, id, agentID, version, ownerID, true); err != nil { slog.Warn("skill_manage: auto-grant failed", "error", err) } else { granted = true @@ -278,7 +301,7 @@ func (t *SkillManageTool) executePatch(ctx context.Context, args map[string]any) // where DM owners got the raw channel sender) // A DM user merged to "viettx" with Telegram ID "386246614" matches all // three of their skills regardless of when they were created. - if !isOwnerOfSkill(ctx, t.skills, slug) { + if !canManageSkill(ctx, t.skills, info) { return ErrorResult(fmt.Sprintf("cannot manage skill %q: you are not the owner", slug)) } @@ -392,7 +415,7 @@ func (t *SkillManageTool) executeDelete(ctx context.Context, args map[string]any // Ownership check: only the skill owner can delete. // Same three-identity match as the patch flow above (#915). - if !isOwnerOfSkill(ctx, t.skills, slug) { + if !canManageSkill(ctx, t.skills, info) { return ErrorResult(fmt.Sprintf("cannot manage skill %q: you are not the owner", slug)) } diff --git a/internal/upgrade/version.go b/internal/upgrade/version.go index ab3b06d7..4a4ce17b 100644 --- a/internal/upgrade/version.go +++ b/internal/upgrade/version.go @@ -2,4 +2,4 @@ package upgrade // RequiredSchemaVersion is the schema migration version this binary requires. // Bump this whenever adding a new SQL migration file. -const RequiredSchemaVersion uint = 65 +const RequiredSchemaVersion uint = 66 diff --git a/migrations/000066_skill_agent_manage_grants.down.sql b/migrations/000066_skill_agent_manage_grants.down.sql new file mode 100644 index 00000000..6ff8efba --- /dev/null +++ b/migrations/000066_skill_agent_manage_grants.down.sql @@ -0,0 +1,2 @@ +ALTER TABLE skill_agent_grants + DROP COLUMN IF EXISTS can_manage; diff --git a/migrations/000066_skill_agent_manage_grants.up.sql b/migrations/000066_skill_agent_manage_grants.up.sql new file mode 100644 index 00000000..8cf33e67 --- /dev/null +++ b/migrations/000066_skill_agent_manage_grants.up.sql @@ -0,0 +1,2 @@ +ALTER TABLE skill_agent_grants + ADD COLUMN IF NOT EXISTS can_manage BOOLEAN NOT NULL DEFAULT FALSE; diff --git a/tests/integration/v3_skills_store_test.go b/tests/integration/v3_skills_store_test.go index 5d9cd130..0def8c97 100644 --- a/tests/integration/v3_skills_store_test.go +++ b/tests/integration/v3_skills_store_test.go @@ -271,6 +271,9 @@ func TestStoreSkill_GrantToAgent(t *testing.T) { if !g.Granted { t.Error("expected Granted=true for granted skill") } + if g.CanManage { + t.Error("expected CanManage=false by default") + } found = true break } @@ -295,6 +298,37 @@ func TestStoreSkill_GrantToAgent(t *testing.T) { t.Error("granted skill not found in ListAccessible") } + if err := s.GrantToAgent(ctx, skillID, agentID, 1, "test-owner", true); err != nil { + t.Fatalf("GrantToAgent can_manage: %v", err) + } + canManage, err := s.AgentCanManageSkill(ctx, skillID, agentID) + if err != nil { + t.Fatalf("AgentCanManageSkill: %v", err) + } + if !canManage { + t.Error("expected AgentCanManageSkill=true after manage grant") + } + if err := s.GrantToAgent(ctx, skillID, agentID, 1, "test-owner"); err != nil { + t.Fatalf("GrantToAgent preserve can_manage: %v", err) + } + canManage, err = s.AgentCanManageSkill(ctx, skillID, agentID) + if err != nil { + t.Fatalf("AgentCanManageSkill after preserve grant: %v", err) + } + if !canManage { + t.Error("expected omitted can_manage grant update to preserve existing manage permission") + } + if err := s.GrantToAgent(ctx, skillID, agentID, 1, "test-owner", false); err != nil { + t.Fatalf("GrantToAgent can_manage false: %v", err) + } + canManage, err = s.AgentCanManageSkill(ctx, skillID, agentID) + if err != nil { + t.Fatalf("AgentCanManageSkill after false grant: %v", err) + } + if canManage { + t.Error("expected explicit can_manage=false to revoke manage permission") + } + // Revoke if err := s.RevokeFromAgent(ctx, skillID, agentID); err != nil { t.Fatalf("RevokeFromAgent: %v", err) diff --git a/ui/web/src/i18n/locales/en/skills.json b/ui/web/src/i18n/locales/en/skills.json index a5f1dce3..c18380f8 100644 --- a/ui/web/src/i18n/locales/en/skills.json +++ b/ui/web/src/i18n/locales/en/skills.json @@ -11,11 +11,30 @@ "description": "Description", "source": "Source", "author": "Author", + "owner": "Owner", "visibility": "Visibility", "status": "Status", "actions": "Actions" }, "noDescription": "No description", + "owner": "Owner", + "unknownOwner": "Unknown", + "grants": { + "title": "Agent access for {{name}}", + "manage": "Manage agent access", + "current": "Current agent grants", + "none": "No agent grants yet.", + "add": "Add grant", + "update": "Update grant", + "grant": "Grant", + "save": "Save", + "selectAgent": "Select agent", + "allowManage": "Allow this agent to edit or delete the skill", + "canManage": "Can edit", + "loadFailed": "Failed to load grants", + "saveFailed": "Failed to save grant", + "revokeFailed": "Failed to revoke grant" + }, "visibility": { "clickToCycle": "Click to change visibility" }, diff --git a/ui/web/src/i18n/locales/vi/skills.json b/ui/web/src/i18n/locales/vi/skills.json index 9f35cecb..c99e9813 100644 --- a/ui/web/src/i18n/locales/vi/skills.json +++ b/ui/web/src/i18n/locales/vi/skills.json @@ -13,7 +13,8 @@ "author": "Tác giả", "visibility": "Hiển thị", "status": "Trạng thái", - "actions": "Thao tác" + "actions": "Thao tác", + "owner": "Chủ sở hữu" }, "noDescription": "Không có mô tả", "visibility": { @@ -125,5 +126,23 @@ "rescanUpdated": "Đã cập nhật {{count}} skill", "rescanNoChanges": "Tất cả skill đã cập nhật", "rescanFailed": "Không thể quét lại dependencies" + }, + "owner": "Chủ sở hữu", + "unknownOwner": "Không rõ", + "grants": { + "title": "Quyền agent cho {{name}}", + "manage": "Quản lý quyền agent", + "current": "Grant agent hiện tại", + "none": "Chưa có grant agent.", + "add": "Thêm grant", + "update": "Cập nhật grant", + "grant": "Grant", + "save": "Lưu", + "selectAgent": "Chọn agent", + "allowManage": "Cho phép agent này sửa hoặc xóa skill", + "canManage": "Được sửa", + "loadFailed": "Không thể tải grant", + "saveFailed": "Không thể lưu grant", + "revokeFailed": "Không thể thu hồi grant" } } diff --git a/ui/web/src/i18n/locales/zh/skills.json b/ui/web/src/i18n/locales/zh/skills.json index 1ec658e9..d74788d5 100644 --- a/ui/web/src/i18n/locales/zh/skills.json +++ b/ui/web/src/i18n/locales/zh/skills.json @@ -13,7 +13,8 @@ "author": "作者", "visibility": "可见性", "status": "状态", - "actions": "操作" + "actions": "操作", + "owner": "所有者" }, "noDescription": "暂无描述", "visibility": { @@ -125,5 +126,23 @@ "rescanUpdated": "已更新 {{count}} 个技能", "rescanNoChanges": "所有技能已是最新", "rescanFailed": "重新扫描依赖失败" + }, + "owner": "所有者", + "unknownOwner": "未知", + "grants": { + "title": "{{name}} 的 Agent 权限", + "manage": "管理 Agent 权限", + "current": "当前 Agent 授权", + "none": "暂无 Agent 授权。", + "add": "添加授权", + "update": "更新授权", + "grant": "授权", + "save": "保存", + "selectAgent": "选择 Agent", + "allowManage": "允许此 Agent 编辑或删除 Skill", + "canManage": "可编辑", + "loadFailed": "无法加载授权", + "saveFailed": "无法保存授权", + "revokeFailed": "无法撤销授权" } } diff --git a/ui/web/src/pages/skills/hooks/use-skills.ts b/ui/web/src/pages/skills/hooks/use-skills.ts index a842c67e..8dc3ac9a 100644 --- a/ui/web/src/pages/skills/hooks/use-skills.ts +++ b/ui/web/src/pages/skills/hooks/use-skills.ts @@ -7,7 +7,7 @@ import { queryKeys } from "@/lib/query-keys"; import { toast } from "@/stores/use-toast-store"; import i18next from "i18next"; import { userFriendlyError } from "@/lib/error-utils"; -import type { SkillInfo, SkillFile, SkillVersions } from "@/types/skill"; +import type { SkillInfo, SkillFile, SkillVersions, SkillAgentGrant } from "@/types/skill"; export type { SkillInfo, SkillFile, SkillVersions }; @@ -105,6 +105,34 @@ export function useSkills() { [http, invalidate], ); + const listAgentGrants = useCallback( + async (id: string) => { + const res = await http.get<{ grants: SkillAgentGrant[] }>(`/v1/skills/${id}/grants/agent`); + return res.grants ?? []; + }, + [http], + ); + + const grantSkillToAgent = useCallback( + async (id: string, agentId: string, version: number, canManage: boolean) => { + await http.post<{ ok: string }>(`/v1/skills/${id}/grants/agent`, { + agent_id: agentId, + version, + can_manage: canManage, + }); + await invalidate(); + }, + [http, invalidate], + ); + + const revokeSkillFromAgent = useCallback( + async (id: string, agentId: string) => { + await http.delete<{ ok: string }>(`/v1/skills/${id}/grants/agent/${agentId}`); + await invalidate(); + }, + [http, invalidate], + ); + const getSkillVersions = useCallback( async (id: string) => { return http.get(`/v1/skills/${id}/versions`); @@ -220,6 +248,7 @@ export function useSkills() { return { skills, loading, refresh: invalidate, getSkill, uploadSkill, updateSkill, deleteSkill, + listAgentGrants, grantSkillToAgent, revokeSkillFromAgent, getSkillVersions, getSkillFiles, getSkillFileContent, rescanDeps, installDeps, installSingleDep, toggleSkill, setTenantConfig, deleteTenantConfig, }; diff --git a/ui/web/src/pages/skills/skill-agent-grants-dialog.tsx b/ui/web/src/pages/skills/skill-agent-grants-dialog.tsx new file mode 100644 index 00000000..f9ecb9d1 --- /dev/null +++ b/ui/web/src/pages/skills/skill-agent-grants-dialog.tsx @@ -0,0 +1,184 @@ +import { useEffect, useMemo, useState } from "react"; +import { useTranslation } from "react-i18next"; +import { Plus, Trash2, ShieldCheck } from "lucide-react"; +import { + Dialog, + DialogContent, + DialogHeader, + DialogTitle, +} from "@/components/ui/dialog"; +import { Button } from "@/components/ui/button"; +import { Badge } from "@/components/ui/badge"; +import { Label } from "@/components/ui/label"; +import { Switch } from "@/components/ui/switch"; +import { + Select, + SelectContent, + SelectItem, + SelectTrigger, + SelectValue, +} from "@/components/ui/select"; +import { useAgents } from "@/pages/agents/hooks/use-agents"; +import type { SkillAgentGrant, SkillInfo } from "@/types/skill"; + +interface SkillAgentGrantsDialogProps { + skill: SkillInfo; + onClose: () => void; + onLoad: (skillId: string) => Promise; + onGrant: (skillId: string, agentId: string, version: number, canManage: boolean) => Promise; + onRevoke: (skillId: string, agentId: string) => Promise; +} + +export function SkillAgentGrantsDialog({ + skill, + onClose, + onLoad, + onGrant, + onRevoke, +}: SkillAgentGrantsDialogProps) { + const { t } = useTranslation("skills"); + const { agents } = useAgents(); + const [grants, setGrants] = useState([]); + const [agentId, setAgentId] = useState(""); + const [canManage, setCanManage] = useState(false); + const [loading, setLoading] = useState(false); + const [error, setError] = useState(""); + + useEffect(() => { + if (!skill.id) return; + setLoading(true); + setError(""); + onLoad(skill.id) + .then(setGrants) + .catch((err) => setError(err instanceof Error ? err.message : t("grants.loadFailed"))) + .finally(() => setLoading(false)); + }, [skill.id, onLoad, t]); + + const agentNames = useMemo(() => { + const map = new Map(); + for (const agent of agents) map.set(agent.id, agent.display_name || agent.agent_key); + return map; + }, [agents]); + + const selectedGrant = grants.find((grant) => grant.agent_id === agentId); + + useEffect(() => { + setCanManage(selectedGrant?.can_manage ?? false); + }, [selectedGrant]); + + const handleGrant = async () => { + if (!skill.id || !agentId) return; + setLoading(true); + setError(""); + try { + await onGrant(skill.id, agentId, skill.version ?? 1, canManage); + setGrants((current) => { + const next: SkillAgentGrant = { + agent_id: agentId, + pinned_version: skill.version ?? 1, + granted_by: "", + can_manage: canManage, + }; + if (current.some((grant) => grant.agent_id === agentId)) { + return current.map((grant) => (grant.agent_id === agentId ? next : grant)); + } + return [...current, next]; + }); + setAgentId(""); + setCanManage(false); + } catch (err) { + setError(err instanceof Error ? err.message : t("grants.saveFailed")); + } finally { + setLoading(false); + } + }; + + const handleRevoke = async (grant: SkillAgentGrant) => { + if (!skill.id) return; + setLoading(true); + setError(""); + try { + await onRevoke(skill.id, grant.agent_id); + setGrants((current) => current.filter((item) => item.agent_id !== grant.agent_id)); + if (agentId === grant.agent_id) setAgentId(""); + } catch (err) { + setError(err instanceof Error ? err.message : t("grants.revokeFailed")); + } finally { + setLoading(false); + } + }; + + return ( + !open && onClose()}> + + + {t("grants.title", { name: skill.name })} + + +
+
+ {t("owner")}:{" "} + {skill.owner_id || t("unknownOwner")} +
+ +
+ + {grants.length === 0 ? ( +

{t("grants.none")}

+ ) : ( +
+ {grants.map((grant) => ( +
+
+

{agentNames.get(grant.agent_id) || grant.agent_id}

+
+ v{grant.pinned_version} + {grant.can_manage && ( + + {t("grants.canManage")} + + )} +
+
+ +
+ ))} +
+ )} +
+ +
+ + + + +
+ + {error &&

{error}

} +
+ + + ); +} diff --git a/ui/web/src/pages/skills/skill-table-row.tsx b/ui/web/src/pages/skills/skill-table-row.tsx index 71a16a46..078fff42 100644 --- a/ui/web/src/pages/skills/skill-table-row.tsx +++ b/ui/web/src/pages/skills/skill-table-row.tsx @@ -1,5 +1,5 @@ import { useTranslation } from "react-i18next"; -import { Zap, Pencil, Trash2 } from "lucide-react"; +import { Zap, Pencil, Trash2, Users } from "lucide-react"; import { Button } from "@/components/ui/button"; import { Badge } from "@/components/ui/badge"; import { Switch } from "@/components/ui/switch"; @@ -20,6 +20,7 @@ interface SkillTableRowProps { toggling: string | null; onView: (name: string) => void; onEdit: (skill: SkillInfo) => void; + onManageGrants: (skill: SkillInfo) => void; onDelete: (skill: SkillInfo) => void; onToggle: (skill: SkillInfo, enabled: boolean) => void; onCycleVisibility: (skill: SkillInfo) => void; @@ -30,7 +31,7 @@ interface SkillTableRowProps { /** Single row in the skills table with inline status, visibility, and action controls. */ export function SkillTableRow({ skill, tab, hasTenantScope, toggling, - onView, onEdit, onDelete, onToggle, onCycleVisibility, + onView, onEdit, onManageGrants, onDelete, onToggle, onCycleVisibility, onSetTenantConfig, onDeleteTenantConfig, }: SkillTableRowProps) { const { t } = useTranslation("skills"); @@ -64,6 +65,13 @@ export function SkillTableRow({ {tab === "custom" && ( {skill.author || "—"} )} + {tab === "custom" && ( + + + {skill.owner_id || t("unknownOwner")} + + + )}
onEdit(skill)} className="gap-1"> + {!skill.is_system && ( + + )} {!skill.is_system && (