mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-11 03:13:24 +00:00
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.
This commit is contained in:
1 parent
8f5aad5c07
commit
3a62bb50e8
18 files changed
+303
-43
No files matched your search
@@ -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.
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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"`
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
}
|
||||
@@ -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
|
||||
@@ -0,0 +1 @@
|
||||
-- Irreversible cleanup migration.
|
||||
@@ -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)
|
||||
);
|
||||
@@ -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)
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -138,6 +138,7 @@
|
||||
"update": "更新授权",
|
||||
"grant": "授权",
|
||||
"save": "保存",
|
||||
"revoke": "撤销授权",
|
||||
"selectAgent": "选择 Agent",
|
||||
"allowManage": "允许此 Agent 编辑或删除 Skill",
|
||||
"canManage": "可编辑",
|
||||
|
||||
@@ -116,11 +116,6 @@ export function SkillAgentGrantsDialog({
|
||||
</DialogHeader>
|
||||
|
||||
<div className="space-y-4 overflow-y-auto min-h-0 pr-1">
|
||||
<div className="rounded-md border p-3 text-sm">
|
||||
<span className="text-muted-foreground">{t("owner")}:</span>{" "}
|
||||
<span className="font-mono">{skill.owner_id || t("unknownOwner")}</span>
|
||||
</div>
|
||||
|
||||
<div className="space-y-2">
|
||||
<Label>{t("grants.current")}</Label>
|
||||
{grants.length === 0 ? (
|
||||
@@ -140,7 +135,15 @@ export function SkillAgentGrantsDialog({
|
||||
)}
|
||||
</div>
|
||||
</div>
|
||||
<Button variant="ghost" size="icon" className="h-8 w-8" disabled={loading} onClick={() => handleRevoke(grant)}>
|
||||
<Button
|
||||
variant="ghost"
|
||||
size="icon"
|
||||
className="h-8 w-8"
|
||||
disabled={loading}
|
||||
aria-label={t("grants.revoke")}
|
||||
title={t("grants.revoke")}
|
||||
onClick={() => handleRevoke(grant)}
|
||||
>
|
||||
<Trash2 className="h-4 w-4 text-destructive" />
|
||||
</Button>
|
||||
</div>
|
||||
|
||||
@@ -65,13 +65,6 @@ export function SkillTableRow({
|
||||
{tab === "custom" && (
|
||||
<td className="px-4 py-3 text-sm text-muted-foreground">{skill.author || "—"}</td>
|
||||
)}
|
||||
{tab === "custom" && (
|
||||
<td className="px-4 py-3">
|
||||
<span className="block max-w-[12rem] truncate font-mono text-xs text-muted-foreground">
|
||||
{skill.owner_id || t("unknownOwner")}
|
||||
</span>
|
||||
</td>
|
||||
)}
|
||||
<td className="px-4 py-3">
|
||||
<div className="flex flex-col gap-1">
|
||||
<Badge
|
||||
|
||||
@@ -168,7 +168,6 @@ export function SkillsPage() {
|
||||
<th className="px-4 py-3 text-left font-medium">{t("columns.name")}</th>
|
||||
<th className="px-4 py-3 text-left font-medium">{t("columns.description")}</th>
|
||||
{tab === "custom" && <th className="px-4 py-3 text-left font-medium">{t("columns.author")}</th>}
|
||||
{tab === "custom" && <th className="px-4 py-3 text-left font-medium">{t("columns.owner")}</th>}
|
||||
<th className="px-4 py-3 text-left font-medium">{t("columns.status")}</th>
|
||||
{tab === "custom" && <th className="px-4 py-3 text-left font-medium">{t("columns.visibility")}</th>}
|
||||
<th className="px-4 py-3 text-right font-medium">{t("columns.actions")}</th>
|
||||
|
||||
@@ -12,7 +12,6 @@ export interface SkillInfo {
|
||||
enabled?: boolean;
|
||||
tenant_enabled?: boolean | null;
|
||||
author?: string;
|
||||
owner_id?: string;
|
||||
missing_deps?: string[];
|
||||
}
|
||||
|
||||
|
||||
Reference in new issue
Block a user