From 3a62bb50e858f3ba795c26a1ef97e0554580d17a Mon Sep 17 00:00:00 2001 From: Goon Date: Mon, 18 May 2026 17:40:41 +0700 Subject: [PATCH] fix(skills): enforce tenant scope on agent grants Reject cross-tenant skill grant and revoke operations before grant rows or skill visibility can be changed. Clean legacy invalid grant rows in PostgreSQL and SQLite migrations, hide owner IDs from skill API/UI responses, and cover the tenant-isolation cases with PG and SQLite regression tests. --- CHANGELOG.md | 10 ++ internal/gateway/methods/skills.go | 6 -- internal/store/pg/skills_grants.go | 64 ++++++++--- internal/store/skill_store.go | 2 +- internal/store/sqlitestore/schema.go | 13 ++- internal/store/sqlitestore/skills_grants.go | 55 +++++++++- internal/store/sqlitestore/skills_test.go | 101 +++++++++++++++++- internal/upgrade/version.go | 2 +- ..._skill_agent_grants_scope_cleanup.down.sql | 1 + ...67_skill_agent_grants_scope_cleanup.up.sql | 8 ++ tests/integration/v3_skills_store_test.go | 57 ++++++++++ ui/web/src/i18n/locales/en/skills.json | 1 + ui/web/src/i18n/locales/vi/skills.json | 1 + ui/web/src/i18n/locales/zh/skills.json | 1 + .../skills/skill-agent-grants-dialog.tsx | 15 +-- ui/web/src/pages/skills/skill-table-row.tsx | 7 -- ui/web/src/pages/skills/skills-page.tsx | 1 - ui/web/src/types/skill.ts | 1 - 18 files changed, 303 insertions(+), 43 deletions(-) create mode 100644 migrations/000067_skill_agent_grants_scope_cleanup.down.sql create mode 100644 migrations/000067_skill_agent_grants_scope_cleanup.up.sql diff --git a/CHANGELOG.md b/CHANGELOG.md index 4a0bb245..ec84603a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ All notable changes to GoClaw are documented here. For full documentation, see [ ### Added +- **Skill agent manage grants** — Adds per-agent skill edit/delete grants with + backend checks, HTTP/WS support, SQLite and PostgreSQL schema updates, and web + dashboard controls for granting and revoking manage access. + - **Packages Update Flow (Phase 2a: pip + npm)** — closes #900 (Phase 2a). Extends Phase 1 update infrastructure to pip and npm package sources. `/v1/packages/updates` now returns mixed-source results with an `availability: {github, pip, npm}` map. @@ -71,6 +75,12 @@ All notable changes to GoClaw are documented here. For full documentation, see [ ### Fixed +- **Skill grant tenant isolation.** Agent skill grants now validate both the + skill and agent tenant scope before insert, revoke, grant listing, or + can-manage checks. Visibility auto-promote/auto-demote updates are scoped to + the calling tenant or system skills so one tenant cannot mutate another + tenant's skill. + - **Agent provider switching.** Saving an agent after changing provider/model now handles cleared ChatGPT OAuth routing config without writing SQL NULL into NOT NULL JSON config columns. diff --git a/internal/gateway/methods/skills.go b/internal/gateway/methods/skills.go index 3d2caf1f..740f699b 100644 --- a/internal/gateway/methods/skills.go +++ b/internal/gateway/methods/skills.go @@ -56,9 +56,6 @@ 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 } @@ -149,9 +146,6 @@ 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 } diff --git a/internal/store/pg/skills_grants.go b/internal/store/pg/skills_grants.go index d2c2990c..cde3b66b 100644 --- a/internal/store/pg/skills_grants.go +++ b/internal/store/pg/skills_grants.go @@ -15,21 +15,14 @@ import ( // GrantToAgent grants a skill to an agent with version pinning. // 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). +// Validates both the skill and agent belong to the requesting tenant. 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, - "SELECT tenant_id FROM agents WHERE id = $1", agentID, - ).Scan(&agentTenantID); err != nil { - return fmt.Errorf("agent not found") - } - if agentTenantID != tid { - return fmt.Errorf("agent not found") + if err := s.verifySkillGrantScope(ctx, skillID, agentID, tid); err != nil { + return err } now := time.Now() @@ -60,8 +53,10 @@ func (s *PGSkillStore) GrantToAgent(ctx context.Context, skillID, agentID uuid.U // Auto-promote: private → internal (so ListAccessible query includes it for granted agents) _, err = s.db.ExecContext(ctx, - `UPDATE skills SET visibility = 'internal', updated_at = NOW() WHERE id = $1 AND visibility = 'private'`, - skillID) + `UPDATE skills + SET visibility = 'internal', updated_at = NOW() + WHERE id = $1 AND visibility = 'private' AND (is_system = true OR tenant_id = $2)`, + skillID, tid) if err != nil { slog.Warn("skill_grants: failed to auto-promote visibility", "skill_id", skillID, "error", err) // Non-fatal: grant was already created successfully @@ -74,6 +69,10 @@ func (s *PGSkillStore) GrantToAgent(ctx context.Context, skillID, agentID uuid.U // RevokeFromAgent revokes a skill grant from an agent. // Auto-demotes visibility from 'internal' back to 'private' when no agent grants remain. func (s *PGSkillStore) RevokeFromAgent(ctx context.Context, skillID, agentID uuid.UUID) error { + tid := tenantIDForInsert(ctx) + if err := s.verifySkillInGrantScope(ctx, skillID, tid); err != nil { + return err + } tClause, tArgs, _, err := scopeClause(ctx, 3) if err != nil { return err @@ -90,9 +89,9 @@ func (s *PGSkillStore) RevokeFromAgent(ctx context.Context, skillID, agentID uui // avoiding a race window between COUNT and UPDATE. _, err = s.db.ExecContext(ctx, `UPDATE skills SET visibility = 'private', updated_at = NOW() - WHERE id = $1 AND visibility = 'internal' + WHERE id = $1 AND visibility = 'internal' AND (is_system = true OR tenant_id = $2) AND NOT EXISTS (SELECT 1 FROM skill_agent_grants WHERE skill_id = $1)`, - skillID) + skillID, tid) if err != nil { slog.Warn("skill_grants: failed to auto-demote visibility", "skill_id", skillID, "error", err) } @@ -101,6 +100,37 @@ func (s *PGSkillStore) RevokeFromAgent(ctx context.Context, skillID, agentID uui return nil } +func (s *PGSkillStore) verifySkillGrantScope(ctx context.Context, skillID, agentID, tenantID uuid.UUID) error { + if err := s.verifySkillInGrantScope(ctx, skillID, tenantID); err != nil { + return err + } + + var agentTenantID uuid.UUID + if err := s.db.QueryRowContext(ctx, + "SELECT tenant_id FROM agents WHERE id = $1", agentID, + ).Scan(&agentTenantID); err != nil { + return fmt.Errorf("agent not found") + } + if agentTenantID != tenantID { + return fmt.Errorf("agent not found") + } + return nil +} + +func (s *PGSkillStore) verifySkillInGrantScope(ctx context.Context, skillID, tenantID uuid.UUID) error { + var skillTenantID uuid.UUID + var isSystem bool + if err := s.db.QueryRowContext(ctx, + "SELECT tenant_id, is_system FROM skills WHERE id = $1", skillID, + ).Scan(&skillTenantID, &isSystem); err != nil { + return fmt.Errorf("skill not found") + } + if !isSystem && skillTenantID != tenantID { + return fmt.Errorf("skill not found") + } + return nil +} + // ListAgentGrants returns all skill grants for an agent. func (s *PGSkillStore) ListAgentGrants(ctx context.Context, agentID uuid.UUID) ([]SkillGrantInfo, error) { tClause, tArgs, _, err := scopeClause(ctx, 2) @@ -119,6 +149,9 @@ func (s *PGSkillStore) ListAgentGrants(ctx context.Context, agentID uuid.UUID) ( // ListAgentGrantsForSkill returns all agent grants for one skill. func (s *PGSkillStore) ListAgentGrantsForSkill(ctx context.Context, skillID uuid.UUID) ([]store.SkillAgentGrantInfo, error) { + if err := s.verifySkillInGrantScope(ctx, skillID, tenantIDForInsert(ctx)); err != nil { + return nil, err + } tClause, tArgs, _, err := scopeClause(ctx, 2) if err != nil { return nil, err @@ -135,6 +168,9 @@ func (s *PGSkillStore) ListAgentGrantsForSkill(ctx context.Context, skillID uuid // 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) { + if err := s.verifySkillInGrantScope(ctx, skillID, tenantIDForInsert(ctx)); err != nil { + return false, err + } tClause, tArgs, _, err := scopeClause(ctx, 3) if err != nil { return false, err diff --git a/internal/store/skill_store.go b/internal/store/skill_store.go index 4a976acf..eeac4720 100644 --- a/internal/store/skill_store.go +++ b/internal/store/skill_store.go @@ -16,7 +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"` + OwnerID string `json:"-" 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"` diff --git a/internal/store/sqlitestore/schema.go b/internal/store/sqlitestore/schema.go index 208bae97..0266d667 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 = 35 +const SchemaVersion = 36 // migrations maps version → SQL to apply when upgrading FROM that version. // schema.sql always represents the LATEST full schema (for fresh DBs). @@ -599,6 +599,17 @@ CREATE INDEX IF NOT EXISTS idx_ws_activity_retention ON workstation_activity(c // 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 35 → 36: remove legacy cross-tenant skill-agent grant rows. + 35: `DELETE FROM skill_agent_grants +WHERE id IN ( + SELECT sag.id + FROM skill_agent_grants sag + JOIN skills s ON sag.skill_id = s.id + JOIN agents a ON sag.agent_id = a.id + WHERE sag.tenant_id <> a.tenant_id + OR (s.is_system = 0 AND sag.tenant_id <> s.tenant_id) +);`, + // 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. diff --git a/internal/store/sqlitestore/skills_grants.go b/internal/store/sqlitestore/skills_grants.go index bf2bba9d..088c8d48 100644 --- a/internal/store/sqlitestore/skills_grants.go +++ b/internal/store/sqlitestore/skills_grants.go @@ -5,6 +5,7 @@ package sqlitestore import ( "context" "database/sql" + "fmt" "log/slog" "time" @@ -29,6 +30,9 @@ func (s *SQLiteSkillStore) GrantToAgent(ctx context.Context, skillID, agentID uu id := store.GenNewID() now := time.Now().UTC() tid := tenantIDForInsert(ctx) + if err := s.verifySkillGrantScope(ctx, skillID, agentID, tid); err != nil { + return err + } var err error if len(canManage) > 0 { _, err = s.db.ExecContext(ctx, @@ -56,8 +60,10 @@ func (s *SQLiteSkillStore) GrantToAgent(ctx context.Context, skillID, agentID uu // Auto-promote: private → internal. _, err = s.db.ExecContext(ctx, - `UPDATE skills SET visibility = 'internal', updated_at = ? WHERE id = ? AND visibility = 'private'`, - time.Now().UTC(), skillID) + `UPDATE skills + SET visibility = 'internal', updated_at = ? + WHERE id = ? AND visibility = 'private' AND (is_system = 1 OR tenant_id = ?)`, + time.Now().UTC(), skillID, tid) if err != nil { slog.Warn("skill_grants: failed to auto-promote visibility", "skill_id", skillID, "error", err) } @@ -68,6 +74,10 @@ func (s *SQLiteSkillStore) GrantToAgent(ctx context.Context, skillID, agentID uu // RevokeFromAgent revokes a skill grant from an agent. func (s *SQLiteSkillStore) RevokeFromAgent(ctx context.Context, skillID, agentID uuid.UUID) error { + tid := tenantIDForInsert(ctx) + if err := s.verifySkillInGrantScope(ctx, skillID, tid); err != nil { + return err + } tClause, tArgs, err := scopeClause(ctx) if err != nil { return err @@ -82,9 +92,9 @@ func (s *SQLiteSkillStore) RevokeFromAgent(ctx context.Context, skillID, agentID // Auto-demote: internal → private when no grants remain. _, err = s.db.ExecContext(ctx, `UPDATE skills SET visibility = 'private', updated_at = ? - WHERE id = ? AND visibility = 'internal' + WHERE id = ? AND visibility = 'internal' AND (is_system = 1 OR tenant_id = ?) AND NOT EXISTS (SELECT 1 FROM skill_agent_grants WHERE skill_id = ?)`, - time.Now().UTC(), skillID, skillID) + time.Now().UTC(), skillID, tid, skillID) if err != nil { slog.Warn("skill_grants: failed to auto-demote visibility", "skill_id", skillID, "error", err) } @@ -93,6 +103,37 @@ func (s *SQLiteSkillStore) RevokeFromAgent(ctx context.Context, skillID, agentID return nil } +func (s *SQLiteSkillStore) verifySkillGrantScope(ctx context.Context, skillID, agentID, tenantID uuid.UUID) error { + if err := s.verifySkillInGrantScope(ctx, skillID, tenantID); err != nil { + return err + } + + var agentTenantID uuid.UUID + if err := s.db.QueryRowContext(ctx, + "SELECT tenant_id FROM agents WHERE id = ?", agentID, + ).Scan(&agentTenantID); err != nil { + return fmt.Errorf("agent not found") + } + if agentTenantID != tenantID { + return fmt.Errorf("agent not found") + } + return nil +} + +func (s *SQLiteSkillStore) verifySkillInGrantScope(ctx context.Context, skillID, tenantID uuid.UUID) error { + var skillTenantID uuid.UUID + var isSystem bool + if err := s.db.QueryRowContext(ctx, + "SELECT tenant_id, is_system FROM skills WHERE id = ?", skillID, + ).Scan(&skillTenantID, &isSystem); err != nil { + return fmt.Errorf("skill not found") + } + if !isSystem && skillTenantID != tenantID { + return fmt.Errorf("skill not found") + } + return nil +} + // ListAgentGrants returns all skill grants for an agent. func (s *SQLiteSkillStore) ListAgentGrants(ctx context.Context, agentID uuid.UUID) ([]SkillGrantInfo, error) { tClause, tArgs, err := scopeClause(ctx) @@ -121,6 +162,9 @@ func (s *SQLiteSkillStore) ListAgentGrants(ctx context.Context, agentID uuid.UUI // ListAgentGrantsForSkill returns all agent grants for one skill. func (s *SQLiteSkillStore) ListAgentGrantsForSkill(ctx context.Context, skillID uuid.UUID) ([]store.SkillAgentGrantInfo, error) { + if err := s.verifySkillInGrantScope(ctx, skillID, tenantIDForInsert(ctx)); err != nil { + return nil, err + } tClause, tArgs, err := scopeClause(ctx) if err != nil { return nil, err @@ -147,6 +191,9 @@ func (s *SQLiteSkillStore) ListAgentGrantsForSkill(ctx context.Context, skillID // 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) { + if err := s.verifySkillInGrantScope(ctx, skillID, tenantIDForInsert(ctx)); err != nil { + return false, err + } tClause, tArgs, err := scopeClause(ctx) if err != nil { return false, err diff --git a/internal/store/sqlitestore/skills_test.go b/internal/store/sqlitestore/skills_test.go index d72ba7c9..fc3d6847 100644 --- a/internal/store/sqlitestore/skills_test.go +++ b/internal/store/sqlitestore/skills_test.go @@ -4,10 +4,13 @@ package sqlitestore import ( "context" + "database/sql" "path/filepath" "reflect" "testing" + "github.com/google/uuid" + "github.com/nextlevelbuilder/goclaw/internal/store" ) @@ -67,7 +70,82 @@ func TestSQLiteSkillStore_CreateSkillManaged_PersistsArchivedDependencyState(t * } } +func TestSQLiteSkillStore_GrantToAgentRejectsCrossTenantSkill(t *testing.T) { + _, skillStore, db := newTestSQLiteSkillStoreWithDB(t) + tenantA, agentA := seedSQLiteTenantAgent(t, db) + tenantB, _ := seedSQLiteTenantAgent(t, db) + ctxA := store.WithTenantID(context.Background(), tenantA) + ctxB := store.WithTenantID(context.Background(), tenantB) + + skillID, err := skillStore.CreateSkillManaged(ctxB, store.SkillCreateParams{ + Name: "Tenant B Skill", + Slug: "tenant-b-skill-" + tenantB.String()[:8], + OwnerID: "user-1", + Visibility: "private", + FilePath: filepath.Join(t.TempDir(), "tenant-b-skill", "1"), + }) + if err != nil { + t.Fatalf("CreateSkillManaged error: %v", err) + } + + if err := skillStore.GrantToAgent(ctxA, skillID, agentA, 1, "user-1", true); err == nil { + t.Fatal("GrantToAgent allowed tenant A to grant tenant B skill") + } + + grants, err := skillStore.ListAgentGrantsForSkill(ctxB, skillID) + if err != nil { + t.Fatalf("ListAgentGrantsForSkill error: %v", err) + } + if len(grants) != 0 { + t.Fatalf("cross-tenant grant was inserted: %+v", grants) + } + + got, ok := skillStore.GetSkillByID(ctxB, skillID) + if !ok { + t.Fatal("GetSkillByID returned !ok") + } + if got.Visibility != "private" { + t.Fatalf("cross-tenant grant changed visibility to %q, want private", got.Visibility) + } +} + +func TestSQLiteSkillStore_RevokeFromAgentDoesNotDemoteCrossTenantSkill(t *testing.T) { + _, skillStore, db := newTestSQLiteSkillStoreWithDB(t) + tenantA, agentA := seedSQLiteTenantAgent(t, db) + tenantB, _ := seedSQLiteTenantAgent(t, db) + ctxA := store.WithTenantID(context.Background(), tenantA) + ctxB := store.WithTenantID(context.Background(), tenantB) + + skillID, err := skillStore.CreateSkillManaged(ctxB, store.SkillCreateParams{ + Name: "Tenant B Skill", + Slug: "tenant-b-revoke-skill-" + tenantB.String()[:8], + OwnerID: "user-1", + Visibility: "internal", + FilePath: filepath.Join(t.TempDir(), "tenant-b-revoke-skill", "1"), + }) + if err != nil { + t.Fatalf("CreateSkillManaged error: %v", err) + } + + if err := skillStore.RevokeFromAgent(ctxA, skillID, agentA); err == nil { + t.Fatal("RevokeFromAgent allowed tenant A to revoke tenant B skill") + } + + got, ok := skillStore.GetSkillByID(ctxB, skillID) + if !ok { + t.Fatal("GetSkillByID returned !ok") + } + if got.Visibility != "internal" { + t.Fatalf("cross-tenant revoke demoted visibility to %q, want internal", got.Visibility) + } +} + func newTestSQLiteSkillStore(t *testing.T) (context.Context, *SQLiteSkillStore) { + ctx, skillStore, _ := newTestSQLiteSkillStoreWithDB(t) + return ctx, skillStore +} + +func newTestSQLiteSkillStoreWithDB(t *testing.T) (context.Context, *SQLiteSkillStore, *sql.DB) { t.Helper() db, err := OpenDB(filepath.Join(t.TempDir(), "skills.db")) @@ -79,5 +157,26 @@ func newTestSQLiteSkillStore(t *testing.T) (context.Context, *SQLiteSkillStore) t.Fatalf("EnsureSchema error: %v", err) } - return store.WithTenantID(context.Background(), store.MasterTenantID), NewSQLiteSkillStore(db, t.TempDir()) + return store.WithTenantID(context.Background(), store.MasterTenantID), NewSQLiteSkillStore(db, t.TempDir()), db +} + +func seedSQLiteTenantAgent(t *testing.T, db *sql.DB) (uuid.UUID, uuid.UUID) { + t.Helper() + + tenantID := uuid.New() + agentID := uuid.New() + if _, err := db.Exec( + `INSERT INTO tenants (id, name, slug, status) VALUES (?, ?, ?, 'active')`, + tenantID.String(), "tenant-"+tenantID.String()[:8], "t"+tenantID.String()[:8], + ); err != nil { + t.Fatalf("insert tenant: %v", err) + } + if _, err := db.Exec( + `INSERT INTO agents (id, tenant_id, agent_key, agent_type, status, provider, model, owner_id) + VALUES (?, ?, ?, 'predefined', 'active', 'test', 'test-model', 'user-1')`, + agentID.String(), tenantID.String(), "agent-"+agentID.String()[:8], + ); err != nil { + t.Fatalf("insert agent: %v", err) + } + return tenantID, agentID } diff --git a/internal/upgrade/version.go b/internal/upgrade/version.go index 4a4ce17b..240912b1 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 = 66 +const RequiredSchemaVersion uint = 67 diff --git a/migrations/000067_skill_agent_grants_scope_cleanup.down.sql b/migrations/000067_skill_agent_grants_scope_cleanup.down.sql new file mode 100644 index 00000000..cfa54c36 --- /dev/null +++ b/migrations/000067_skill_agent_grants_scope_cleanup.down.sql @@ -0,0 +1 @@ +-- Irreversible cleanup migration. diff --git a/migrations/000067_skill_agent_grants_scope_cleanup.up.sql b/migrations/000067_skill_agent_grants_scope_cleanup.up.sql new file mode 100644 index 00000000..f024264f --- /dev/null +++ b/migrations/000067_skill_agent_grants_scope_cleanup.up.sql @@ -0,0 +1,8 @@ +DELETE FROM skill_agent_grants sag +USING skills s, agents a +WHERE sag.skill_id = s.id + AND sag.agent_id = a.id + AND ( + sag.tenant_id <> a.tenant_id + OR (s.is_system = false AND sag.tenant_id <> s.tenant_id) + ); diff --git a/tests/integration/v3_skills_store_test.go b/tests/integration/v3_skills_store_test.go index 0def8c97..882ac6dc 100644 --- a/tests/integration/v3_skills_store_test.go +++ b/tests/integration/v3_skills_store_test.go @@ -346,6 +346,63 @@ func TestStoreSkill_GrantToAgent(t *testing.T) { } } +func TestStoreSkill_GrantToAgentRejectsCrossTenantSkill(t *testing.T) { + db := testDB(t) + tenantA, agentA := seedTenantAgent(t, db) + tenantB, _ := seedTenantAgent(t, db) + ctxA := tenantCtx(tenantA) + ctxB := tenantCtx(tenantB) + s := newSkillStore(t) + + skillB := seedSkill(t, s, ctxB, "grant-cross-tenant-"+tenantB.String()[:8], "Tenant B Skill") + + if err := s.GrantToAgent(ctxA, skillB, agentA, 1, "test-owner", true); err == nil { + t.Fatal("GrantToAgent allowed tenant A to grant tenant B skill") + } + + grants, err := s.ListAgentGrantsForSkill(ctxB, skillB) + if err != nil { + t.Fatalf("ListAgentGrantsForSkill: %v", err) + } + if len(grants) != 0 { + t.Fatalf("cross-tenant grant was inserted: %+v", grants) + } + + got, ok := s.GetSkillByID(ctxB, skillB) + if !ok { + t.Fatal("GetSkillByID for tenant B skill returned false") + } + if got.Visibility != "private" { + t.Fatalf("cross-tenant grant changed visibility to %q, want private", got.Visibility) + } +} + +func TestStoreSkill_RevokeFromAgentDoesNotDemoteCrossTenantSkill(t *testing.T) { + db := testDB(t) + tenantA, agentA := seedTenantAgent(t, db) + tenantB, _ := seedTenantAgent(t, db) + ctxA := tenantCtx(tenantA) + ctxB := tenantCtx(tenantB) + s := newSkillStore(t) + + skillB := seedSkill(t, s, ctxB, "revoke-cross-tenant-"+tenantB.String()[:8], "Tenant B Skill") + if err := s.UpdateSkill(ctxB, skillB, map[string]any{"visibility": "internal"}); err != nil { + t.Fatalf("UpdateSkill: %v", err) + } + + if err := s.RevokeFromAgent(ctxA, skillB, agentA); err == nil { + t.Fatal("RevokeFromAgent allowed tenant A to revoke tenant B skill") + } + + got, ok := s.GetSkillByID(ctxB, skillB) + if !ok { + t.Fatal("GetSkillByID for tenant B skill returned false") + } + if got.Visibility != "internal" { + t.Fatalf("cross-tenant revoke demoted visibility to %q, want internal", got.Visibility) + } +} + func TestStoreSkill_TenantIsolation(t *testing.T) { db := testDB(t) tenantA, _ := seedTenantAgent(t, db) diff --git a/ui/web/src/i18n/locales/en/skills.json b/ui/web/src/i18n/locales/en/skills.json index c18380f8..c8df5947 100644 --- a/ui/web/src/i18n/locales/en/skills.json +++ b/ui/web/src/i18n/locales/en/skills.json @@ -28,6 +28,7 @@ "update": "Update grant", "grant": "Grant", "save": "Save", + "revoke": "Revoke grant", "selectAgent": "Select agent", "allowManage": "Allow this agent to edit or delete the skill", "canManage": "Can edit", diff --git a/ui/web/src/i18n/locales/vi/skills.json b/ui/web/src/i18n/locales/vi/skills.json index c99e9813..5833c4c4 100644 --- a/ui/web/src/i18n/locales/vi/skills.json +++ b/ui/web/src/i18n/locales/vi/skills.json @@ -138,6 +138,7 @@ "update": "Cập nhật grant", "grant": "Grant", "save": "Lưu", + "revoke": "Thu hồi grant", "selectAgent": "Chọn agent", "allowManage": "Cho phép agent này sửa hoặc xóa skill", "canManage": "Được sửa", diff --git a/ui/web/src/i18n/locales/zh/skills.json b/ui/web/src/i18n/locales/zh/skills.json index d74788d5..b3c0444d 100644 --- a/ui/web/src/i18n/locales/zh/skills.json +++ b/ui/web/src/i18n/locales/zh/skills.json @@ -138,6 +138,7 @@ "update": "更新授权", "grant": "授权", "save": "保存", + "revoke": "撤销授权", "selectAgent": "选择 Agent", "allowManage": "允许此 Agent 编辑或删除 Skill", "canManage": "可编辑", diff --git a/ui/web/src/pages/skills/skill-agent-grants-dialog.tsx b/ui/web/src/pages/skills/skill-agent-grants-dialog.tsx index f9ecb9d1..399d0481 100644 --- a/ui/web/src/pages/skills/skill-agent-grants-dialog.tsx +++ b/ui/web/src/pages/skills/skill-agent-grants-dialog.tsx @@ -116,11 +116,6 @@ export function SkillAgentGrantsDialog({
-
- {t("owner")}:{" "} - {skill.owner_id || t("unknownOwner")} -
-
{grants.length === 0 ? ( @@ -140,7 +135,15 @@ export function SkillAgentGrantsDialog({ )}
- diff --git a/ui/web/src/pages/skills/skill-table-row.tsx b/ui/web/src/pages/skills/skill-table-row.tsx index 078fff42..97bc6bbb 100644 --- a/ui/web/src/pages/skills/skill-table-row.tsx +++ b/ui/web/src/pages/skills/skill-table-row.tsx @@ -65,13 +65,6 @@ export function SkillTableRow({ {tab === "custom" && ( {skill.author || "—"} )} - {tab === "custom" && ( - - - {skill.owner_id || t("unknownOwner")} - - - )}
{t("columns.name")} {t("columns.description")} {tab === "custom" && {t("columns.author")}} - {tab === "custom" && {t("columns.owner")}} {t("columns.status")} {tab === "custom" && {t("columns.visibility")}} {t("columns.actions")} diff --git a/ui/web/src/types/skill.ts b/ui/web/src/types/skill.ts index f4edfbf6..ade8342c 100644 --- a/ui/web/src/types/skill.ts +++ b/ui/web/src/types/skill.ts @@ -12,7 +12,6 @@ export interface SkillInfo { enabled?: boolean; tenant_enabled?: boolean | null; author?: string; - owner_id?: string; missing_deps?: string[]; }