From 912b87404f69dc8a0b8001b816de5898986e90fc Mon Sep 17 00:00:00 2001 From: SG Command Date: Tue, 18 Aug 2026 00:16:35 +0200 Subject: [PATCH] fix: scope project skills by tenant --- doc/llm/log/20260818_00.md | 9 ++++++++ internal/store/skills.go | 41 +++++++++++++++++++++++++++++------ internal/store/skills_test.go | 32 +++++++++++++++++++++++++++ 3 files changed, 75 insertions(+), 7 deletions(-) create mode 100644 doc/llm/log/20260818_00.md create mode 100644 internal/store/skills_test.go diff --git a/doc/llm/log/20260818_00.md b/doc/llm/log/20260818_00.md new file mode 100644 index 0000000..4a12615 --- /dev/null +++ b/doc/llm/log/20260818_00.md @@ -0,0 +1,9 @@ +# issue-42 tenant skills + +- AMCS2 MCP calls were attempted but cancelled by the environment, so this local fallback log captures the required thought summary. +- Model selection: Tuesday Claude primary was unavailable because its OAuth access token expired; Codex gpt-5.5 was used as the final fallback. +- `tea issue 42` crashed before returning issue data, and Gitea DNS was unavailable, so the fix was inferred from the issue branch name and tenant-skill code paths. +- Implemented tenant enforcement for project skill links in `internal/store/skills.go`: add, remove, and list now require the project and skill to belong to the active tenant when a tenant context is present. +- Added focused tests for the tenant predicate helper in `internal/store/skills_test.go`. +- Verification: `go test ./internal/store -v`, `go test ./internal/tools -v`, and `go test ./internal/store ./internal/tools` passed. +- Full `go test ./...` was attempted but failed on existing environment constraints: missing `ui/dist` embedded assets and sandbox-blocked `httptest` listener sockets. diff --git a/internal/store/skills.go b/internal/store/skills.go index 70b329d..b914f9d 100644 --- a/internal/store/skills.go +++ b/internal/store/skills.go @@ -6,6 +6,7 @@ import ( "strings" "git.warky.dev/wdevs/amcs/internal/generatedmodels" + "git.warky.dev/wdevs/amcs/internal/tenancy" ext "git.warky.dev/wdevs/amcs/internal/types" ) @@ -307,21 +308,35 @@ func (db *DB) GetGuardrail(ctx context.Context, id int64) (ext.AgentGuardrail, e // Project Skills func (db *DB) AddProjectSkill(ctx context.Context, projectID, skillID int64, override bool) error { - _, err := db.pool.Exec(ctx, ` + args := []any{projectID, skillID, override} + tenantWhere := projectSkillTenantWhere(ctx, &args, "p", "s") + tag, err := db.pool.Exec(ctx, ` insert into project_skills (project_id, skill_id, override) - values ($1, $2, $3) + select $1, $2, $3 + from projects p + join agent_skills s on s.id = $2 + where p.id = $1`+tenantWhere+` on conflict (project_id, skill_id) do update set override = excluded.override - `, projectID, skillID, override) + `, args...) if err != nil { return fmt.Errorf("add project skill: %w", err) } + if tag.RowsAffected() == 0 { + return fmt.Errorf("project or skill not found") + } return nil } func (db *DB) RemoveProjectSkill(ctx context.Context, projectID, skillID int64) error { + args := []any{projectID, skillID} + tenantWhere := projectSkillTenantWhere(ctx, &args, "p", "s") tag, err := db.pool.Exec(ctx, ` - delete from project_skills where project_id = $1 and skill_id = $2 - `, projectID, skillID) + delete from project_skills ps + using projects p, agent_skills s + where ps.project_id = $1 + and ps.skill_id = $2 + and p.id = ps.project_id + and s.id = ps.skill_id`+tenantWhere, args...) if err != nil { return fmt.Errorf("remove project skill: %w", err) } @@ -332,15 +347,18 @@ func (db *DB) RemoveProjectSkill(ctx context.Context, projectID, skillID int64) } func (db *DB) ListProjectSkills(ctx context.Context, projectID int64) ([]ext.AgentSkill, error) { + args := []any{projectID} + tenantWhere := projectSkillTenantWhere(ctx, &args, "p", "s") rows, err := db.pool.Query(ctx, ` select s.id, s.name, s.description, s.content, s.tags::text[], s.language_tags::text[], s.library_tags::text[], s.framework_tags::text[], s.domain_tags::text[], s.created_at, s.updated_at, ps.override from agent_skills s join project_skills ps on ps.skill_id = s.id - where ps.project_id = $1 + join projects p on p.id = ps.project_id + where ps.project_id = $1`+tenantWhere+` order by s.name - `, projectID) + `, args...) if err != nil { return nil, fmt.Errorf("list project skills: %w", err) } @@ -360,6 +378,15 @@ func (db *DB) ListProjectSkills(ctx context.Context, projectID int64) ([]ext.Age return skills, rows.Err() } +func projectSkillTenantWhere(ctx context.Context, args *[]any, projectAlias, skillAlias string) string { + if key, ok := tenancy.KeyFromContext(ctx); ok { + *args = append(*args, key) + placeholder := fmt.Sprintf("$%d", len(*args)) + return fmt.Sprintf(" and %s.tenant_id = %s and %s.tenant_id = %s", projectAlias, placeholder, skillAlias, placeholder) + } + return "" +} + // Project Guardrails func (db *DB) AddProjectGuardrail(ctx context.Context, projectID, guardrailID int64) error { diff --git a/internal/store/skills_test.go b/internal/store/skills_test.go new file mode 100644 index 0000000..9238578 --- /dev/null +++ b/internal/store/skills_test.go @@ -0,0 +1,32 @@ +package store + +import ( + "context" + "testing" + + "git.warky.dev/wdevs/amcs/internal/tenancy" +) + +func TestProjectSkillTenantWhere(t *testing.T) { + args := []any{int64(1), int64(2), true} + where := projectSkillTenantWhere(tenancy.WithTenantKey(context.Background(), "tenant-a"), &args, "p", "s") + + if where != " and p.tenant_id = $4 and s.tenant_id = $4" { + t.Fatalf("where = %q", where) + } + if len(args) != 4 || args[3] != "tenant-a" { + t.Fatalf("args = %#v", args) + } +} + +func TestProjectSkillTenantWhereWithoutTenant(t *testing.T) { + args := []any{int64(1), int64(2)} + where := projectSkillTenantWhere(context.Background(), &args, "p", "s") + + if where != "" { + t.Fatalf("where = %q, want empty", where) + } + if len(args) != 2 { + t.Fatalf("args = %#v", args) + } +} -- 2.54.0