tiennm99/goclaw
1
0 Fork 0
mirror of https://github.com/tiennm99/goclaw.git synced 2026-10-13 18:15:34 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(skills): scope agent grant status joins

Browse Source
This commit is contained in:
Duy Nguyen committed 2026-05-18 20:44:45 +07:00
1 parent 2686a6c989
commit 8ef8fc4f73
4 files changed
+82 -2
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

internal/store/pg/skills_grants.go
+3 -1
View File
@@ -297,8 +297,10 @@ func (s *PGSkillStore) ListWithGrantStatus(ctx context.Context, agentID uuid.UUI
return nil, err
}
tenantCond := ""
grantTenantCond := ""
if tc != "" {
tenantCond = fmt.Sprintf(" AND (s.is_system = true OR s.tenant_id = $%d)", 2)
grantTenantCond = fmt.Sprintf(" AND sag.tenant_id = $%d", 2)
}
rows, err := s.db.QueryContext(ctx,
`SELECT s.id, s.name, s.slug, COALESCE(s.description, ''), s.visibility, s.version,
@@ -307,7 +309,7 @@ func (s *PGSkillStore) ListWithGrantStatus(ctx context.Context, agentID uuid.UUI
sag.pinned_version,
s.is_system
FROM skills s
LEFT JOIN skill_agent_grants sag ON s.id = sag.skill_id AND sag.agent_id = $1
LEFT JOIN skill_agent_grants sag ON s.id = sag.skill_id AND sag.agent_id = $1`+grantTenantCond+`
WHERE s.status = 'active'`+tenantCond+`
ORDER BY s.name`, append([]any{agentID}, tcArgs...)...)
if err != nil {
internal/store/sqlitestore/skills_grants.go
+4 -1
View File
@@ -310,14 +310,17 @@ func (s *SQLiteSkillStore) ListWithGrantStatus(ctx context.Context, agentID uuid
return nil, err
}
tenantCond := ""
grantTenantCond := ""
if len(tArgs) > 0 {
tenantCond = " AND (s.is_system = 1 OR s.tenant_id = ?)"
grantTenantCond = " AND sag.tenant_id = ?"
}
_ = tClause
queryArgs := []any{agentID}
if len(tArgs) > 0 {
queryArgs = append(queryArgs, tArgs...)
queryArgs = append(queryArgs, tArgs...)
}
rows, err := s.db.QueryContext(ctx,
@@ -327,7 +330,7 @@ func (s *SQLiteSkillStore) ListWithGrantStatus(ctx context.Context, agentID uuid
sag.pinned_version,
s.is_system
FROM skills s
LEFT JOIN skill_agent_grants sag ON s.id = sag.skill_id AND sag.agent_id = ?
LEFT JOIN skill_agent_grants sag ON s.id = sag.skill_id AND sag.agent_id = ?`+grantTenantCond+`
WHERE s.status = 'active'`+tenantCond+`
ORDER BY s.name`, queryArgs...)
if err != nil {
internal/store/sqlitestore/skills_test.go
+37
View File
@@ -140,6 +140,43 @@ func TestSQLiteSkillStore_RevokeFromAgentDoesNotDemoteCrossTenantSkill(t *testin
}
}
func TestSQLiteSkillStore_ListWithGrantStatusIgnoresForeignTenantGrant(t *testing.T) {
_, skillStore, db := newTestSQLiteSkillStoreWithDB(t)
tenantA, _ := seedSQLiteTenantAgent(t, db)
tenantB, agentB := seedSQLiteTenantAgent(t, db)
ctxA := store.WithTenantID(context.Background(), tenantA)
skillID := uuid.New()
if _, err := db.Exec(
`INSERT INTO skills (id, name, slug, owner_id, visibility, version, status, file_path, is_system, tenant_id)
VALUES (?, 'System Skill', ?, 'system', 'internal', 1, 'active', ?, 1, ?)`,
skillID.String(), "system-grant-status-"+skillID.String()[:8], filepath.Join(t.TempDir(), "system-skill", "1"), store.MasterTenantID.String(),
); err != nil {
t.Fatalf("insert system skill: %v", err)
}
if _, err := db.Exec(
`INSERT INTO skill_agent_grants (id, skill_id, agent_id, pinned_version, granted_by, can_manage, tenant_id)
VALUES (?, ?, ?, 1, 'tenant-b-admin', 1, ?)`,
uuid.New().String(), skillID.String(), agentB.String(), tenantB.String(),
); err != nil {
t.Fatalf("insert foreign tenant grant: %v", err)
}
skills, err := skillStore.ListWithGrantStatus(ctxA, agentB)
if err != nil {
t.Fatalf("ListWithGrantStatus error: %v", err)
}
for _, skill := range skills {
if skill.ID == skillID {
if skill.Granted || skill.CanManage {
t.Fatalf("foreign tenant grant leaked into tenant A status: granted=%v canManage=%v", skill.Granted, skill.CanManage)
}
return
}
}
t.Fatalf("system skill %s not returned for tenant A", skillID)
}
func newTestSQLiteSkillStore(t *testing.T) (context.Context, *SQLiteSkillStore) {
ctx, skillStore, _ := newTestSQLiteSkillStoreWithDB(t)
return ctx, skillStore
tests/integration/v3_skills_store_test.go
+38
View File
@@ -403,6 +403,44 @@ func TestStoreSkill_RevokeFromAgentDoesNotDemoteCrossTenantSkill(t *testing.T) {
}
}
func TestStoreSkill_ListWithGrantStatusIgnoresForeignTenantGrant(t *testing.T) {
db := testDB(t)
tenantA, _ := seedTenantAgent(t, db)
tenantB, agentB := seedTenantAgent(t, db)
ctxA := tenantCtx(tenantA)
s := newSkillStore(t)
skillID := uuid.New()
if _, err := db.Exec(
`INSERT INTO skills (id, name, slug, owner_id, visibility, version, status, file_path, is_system, tenant_id)
VALUES ($1, 'System Skill', $2, 'system', 'internal', 1, 'active', $3, true, $4)`,
skillID, "system-grant-status-"+skillID.String()[:8], "/tmp/skills/system-skill/1", store.MasterTenantID,
); err != nil {
t.Fatalf("insert system skill: %v", err)
}
if _, err := db.Exec(
`INSERT INTO skill_agent_grants (id, skill_id, agent_id, pinned_version, granted_by, can_manage, tenant_id)
VALUES ($1, $2, $3, 1, 'tenant-b-admin', true, $4)`,
uuid.New(), skillID, agentB, tenantB,
); err != nil {
t.Fatalf("insert foreign tenant grant: %v", err)
}
skills, err := s.ListWithGrantStatus(ctxA, agentB)
if err != nil {
t.Fatalf("ListWithGrantStatus error: %v", err)
}
for _, skill := range skills {
if skill.ID == skillID {
if skill.Granted || skill.CanManage {
t.Fatalf("foreign tenant grant leaked into tenant A status: granted=%v canManage=%v", skill.Granted, skill.CanManage)
}
return
}
}
t.Fatalf("system skill %s not returned for tenant A", skillID)
}
func TestStoreSkill_TenantIsolation(t *testing.T) {
db := testDB(t)
tenantA, _ := seedTenantAgent(t, db)