diff --git a/internal/gateway/client.go b/internal/gateway/client.go index d8e1caa1..75de73c9 100644 --- a/internal/gateway/client.go +++ b/internal/gateway/client.go @@ -212,6 +212,9 @@ func (c *Client) RemoteAddr() string { return c.remoteAddr } // TenantID returns the resolved tenant UUID (uuid.Nil means cross-tenant). func (c *Client) TenantID() uuid.UUID { return c.tenantID } +// TenantSlug returns the resolved tenant URL slug (set during connect). +func (c *Client) TenantSlug() string { return c.tenantSlug } + // IsOwner returns true if the client has the owner role (tenant management + full access). func (c *Client) IsOwner() bool { return c.role == permissions.RoleOwner } diff --git a/internal/gateway/router.go b/internal/gateway/router.go index dd36568a..4c7f8748 100644 --- a/internal/gateway/router.go +++ b/internal/gateway/router.go @@ -83,6 +83,9 @@ func (r *MethodRouter) Handle(ctx context.Context, client *Client, req *protocol if client.TenantID() != uuid.Nil { ctx = store.WithTenantID(ctx, client.TenantID()) } + if slug := client.TenantSlug(); slug != "" { + ctx = store.WithTenantSlug(ctx, slug) + } slog.Debug("handling method", "method", req.Method, "client", client.id, "req_id", req.ID) handler(ctx, client, req) diff --git a/internal/http/auth.go b/internal/http/auth.go index c5353bda..f1ee3f78 100644 --- a/internal/http/auth.go +++ b/internal/http/auth.go @@ -159,6 +159,7 @@ type authResult struct { Authenticated bool KeyData *store.APIKeyData // non-nil when authenticated via API key TenantID uuid.UUID // resolved tenant; always concrete after resolution + TenantSlug string // resolved tenant slug for filesystem paths } // resolveAuth determines the caller's role from the request. @@ -187,9 +188,11 @@ func resolveAuthWithBearer(r *http.Request, bearer string) authResult { if tid, err := uuid.Parse(tenantVal); err == nil { if t, err := pkgTenantCache.GetTenant(r.Context(), tid); err == nil && t != nil { res.TenantID = t.ID + res.TenantSlug = t.Slug } } else if t, err := pkgTenantCache.GetTenantBySlug(r.Context(), tenantVal); err == nil && t != nil { res.TenantID = t.ID + res.TenantSlug = t.Slug } } if res.TenantID == uuid.Nil { @@ -206,6 +209,12 @@ func resolveAuthWithBearer(r *http.Request, bearer string) authResult { res.TenantID = store.MasterTenantID } else { res.TenantID = keyData.TenantID + // Resolve tenant slug for filesystem path scoping. + if pkgTenantCache != nil { + if t, err := pkgTenantCache.GetTenant(r.Context(), keyData.TenantID); err == nil && t != nil { + res.TenantSlug = t.Slug + } + } } return res } @@ -262,6 +271,9 @@ func enrichContext(ctx context.Context, r *http.Request, auth authResult) contex tenantID = store.MasterTenantID } ctx = store.WithTenantID(ctx, tenantID) + if auth.TenantSlug != "" { + ctx = store.WithTenantSlug(ctx, auth.TenantSlug) + } slog.Debug("security.http_auth_resolved", "path", r.URL.Path, "role", string(auth.Role), diff --git a/internal/http/skills_upload.go b/internal/http/skills_upload.go index 8912444d..19af0112 100644 --- a/internal/http/skills_upload.go +++ b/internal/http/skills_upload.go @@ -115,7 +115,7 @@ func (h *SkillsHandler) handleUpload(w http.ResponseWriter, r *http.Request) { } // Determine version (always increment — includes archived skills so re-upload gets v2+) - version := h.skills.GetNextVersion(slug) + version := h.skills.GetNextVersion(r.Context(), slug) // Extract to filesystem: tenant-scoped skills-store/slug/version/ tenantSkillsBase := h.tenantSkillsDir(r) diff --git a/internal/skills/seeder.go b/internal/skills/seeder.go index 057d371d..9d063dd7 100644 --- a/internal/skills/seeder.go +++ b/internal/skills/seeder.go @@ -19,7 +19,7 @@ import ( // SystemSkillStore is the minimal interface needed by the seeder. type SystemSkillStore interface { UpsertSystemSkill(ctx context.Context, p pg.SkillCreateParams) (uuid.UUID, bool, string, error) - GetNextVersion(slug string) int + GetNextVersion(ctx context.Context, slug string) int BumpVersion() UpdateSkill(ctx context.Context, id uuid.UUID, updates map[string]interface{}) error StoreMissingDeps(ctx context.Context, id uuid.UUID, missing []string) error @@ -100,7 +100,7 @@ func (s *Seeder) Seed(ctx context.Context) (seeded int, skipped int, skills []se fmMap = parseSimpleYAML(fm) } - version := s.store.GetNextVersion(slug) + version := s.store.GetNextVersion(ctx, slug) destDir := filepath.Join(s.managedDir, slug, fmt.Sprintf("%d", version)) desc := description diff --git a/internal/store/pg/skills.go b/internal/store/pg/skills.go index b262cd81..188bc186 100644 --- a/internal/store/pg/skills.go +++ b/internal/store/pg/skills.go @@ -187,7 +187,8 @@ func (s *PGSkillStore) scanSkillInfoList(rows *sql.Rows) []store.SkillInfo { } // StoreMissingDeps persists the missing_deps list for a skill into the deps JSONB column. -// Only updates system skills unscoped; custom skills require tenant match. +// Works for both system and custom skills. System skills bypass tenant filter; +// custom skills require tenant_id match for cross-tenant safety. func (s *PGSkillStore) StoreMissingDeps(ctx context.Context, id uuid.UUID, missing []string) error { if missing == nil { missing = []string{} @@ -196,11 +197,10 @@ func (s *PGSkillStore) StoreMissingDeps(ctx context.Context, id uuid.UUID, missi if err != nil { return err } - // System skills can be updated without tenant; custom skills need tenant scope. - // Use is_system check to ensure cross-tenant safety for custom skills. + tid := tenantIDForInsert(ctx) _, err = s.db.ExecContext(ctx, - `UPDATE skills SET deps = $1, updated_at = NOW() WHERE id = $2 AND is_system = true`, - encoded, id, + `UPDATE skills SET deps = $1, updated_at = NOW() WHERE id = $2 AND (is_system = true OR tenant_id = $3)`, + encoded, id, tid, ) if err == nil { s.BumpVersion() diff --git a/internal/store/pg/skills_admin.go b/internal/store/pg/skills_admin.go index ee5ee871..86fd4896 100644 --- a/internal/store/pg/skills_admin.go +++ b/internal/store/pg/skills_admin.go @@ -95,9 +95,12 @@ func (s *PGSkillStore) ListSystemSkillDirs(ctx context.Context) map[string]strin return dirs } -// IsSystemSkill checks if a skill slug belongs to a system skill. +// IsSystemSkill checks if a skill slug belongs to a system skill (master tenant only). func (s *PGSkillStore) IsSystemSkill(slug string) bool { var isSystem bool - err := s.db.QueryRow("SELECT is_system FROM skills WHERE slug = $1", slug).Scan(&isSystem) + err := s.db.QueryRow( + "SELECT is_system FROM skills WHERE slug = $1 AND tenant_id = $2 AND is_system = true", + slug, store.MasterTenantID, + ).Scan(&isSystem) return err == nil && isSystem } diff --git a/internal/store/pg/skills_crud.go b/internal/store/pg/skills_crud.go index f6c5c520..843176de 100644 --- a/internal/store/pg/skills_crud.go +++ b/internal/store/pg/skills_crud.go @@ -111,10 +111,12 @@ func (s *PGSkillStore) DeleteSkill(ctx context.Context, id uuid.UUID) error { return nil } -// slugAdvisoryLock returns a stable int64 lock key derived from a slug string, -// suitable for use with pg_advisory_xact_lock. -func slugAdvisoryLock(slug string) int64 { +// tenantSlugAdvisoryLock returns a stable int64 lock key derived from tenant ID + slug, +// suitable for use with pg_advisory_xact_lock. Tenant-scoped so different tenants +// uploading the same slug don't contend on the same lock. +func tenantSlugAdvisoryLock(tenantID uuid.UUID, slug string) int64 { h := fnv.New64a() + h.Write(tenantID[:]) h.Write([]byte(slug)) return int64(h.Sum64()) } @@ -143,27 +145,27 @@ func (s *PGSkillStore) CreateSkillManaged(ctx context.Context, p SkillCreatePara } defer tx.Rollback() - // Acquire advisory lock scoped to this transaction so concurrent calls for - // the same slug serialize version calculation and the upsert atomically. - if _, err := tx.ExecContext(ctx, "SELECT pg_advisory_xact_lock($1)", slugAdvisoryLock(p.Slug)); err != nil { - return uuid.Nil, fmt.Errorf("advisory lock: %w", err) - } - - // Compute next version atomically under the lock. - var version int - if err := tx.QueryRowContext(ctx, - "SELECT COALESCE(MAX(version), 0) + 1 FROM skills WHERE slug = $1", - p.Slug, - ).Scan(&version); err != nil { - return uuid.Nil, fmt.Errorf("get next version: %w", err) - } - // Resolve tenant_id: default to MasterTenantID if no tenant in context. tenantID := store.TenantIDFromContext(ctx) if tenantID == uuid.Nil { tenantID = store.MasterTenantID } + // Acquire advisory lock scoped to this transaction so concurrent calls for + // the same (tenant, slug) pair serialize version calculation and the upsert atomically. + if _, err := tx.ExecContext(ctx, "SELECT pg_advisory_xact_lock($1)", tenantSlugAdvisoryLock(tenantID, p.Slug)); err != nil { + return uuid.Nil, fmt.Errorf("advisory lock: %w", err) + } + + // Compute next version atomically under the lock, scoped to tenant. + var version int + if err := tx.QueryRowContext(ctx, + "SELECT COALESCE(MAX(version), 0) + 1 FROM skills WHERE slug = $1 AND tenant_id = $2", + p.Slug, tenantID, + ).Scan(&version); err != nil { + return uuid.Nil, fmt.Errorf("get next version: %w", err) + } + id := store.GenNewID() var returnedID uuid.UUID err = tx.QueryRowContext(ctx, @@ -215,14 +217,15 @@ func (s *PGSkillStore) GetSkillFilePath(ctx context.Context, id uuid.UUID) (file return filePath, slug, version, isSystem, err == nil } -// GetNextVersion returns the next version number for a skill slug. +// GetNextVersion returns the next version number for a skill slug, scoped to tenant. // NOTE: This function has an inherent race condition — two concurrent callers // for the same slug can receive the same version number. Use it only for // informational purposes (e.g. display). For write paths, use CreateSkillManaged // which computes the version atomically under a pg_advisory_xact_lock. -func (s *PGSkillStore) GetNextVersion(slug string) int { +func (s *PGSkillStore) GetNextVersion(ctx context.Context, slug string) int { + tid := tenantIDForInsert(ctx) var maxVersion int - s.db.QueryRow("SELECT COALESCE(MAX(version), 0) FROM skills WHERE slug = $1", slug).Scan(&maxVersion) + s.db.QueryRowContext(ctx, "SELECT COALESCE(MAX(version), 0) FROM skills WHERE slug = $1 AND tenant_id = $2", slug, tid).Scan(&maxVersion) return maxVersion + 1 } @@ -230,17 +233,18 @@ func (s *PGSkillStore) GetNextVersion(slug string) int { // Safe for concurrent write paths (patch, create). Returns version and a cleanup func // that MUST be called to release the lock (commits the transaction). func (s *PGSkillStore) GetNextVersionLocked(ctx context.Context, slug string) (int, func() error, error) { + tenantID := tenantIDForInsert(ctx) tx, err := s.db.BeginTx(ctx, nil) if err != nil { return 0, nil, fmt.Errorf("begin tx: %w", err) } - if _, err := tx.ExecContext(ctx, "SELECT pg_advisory_xact_lock($1)", slugAdvisoryLock(slug)); err != nil { + if _, err := tx.ExecContext(ctx, "SELECT pg_advisory_xact_lock($1)", tenantSlugAdvisoryLock(tenantID, slug)); err != nil { tx.Rollback() return 0, nil, fmt.Errorf("advisory lock: %w", err) } var version int if err := tx.QueryRowContext(ctx, - "SELECT COALESCE(MAX(version), 0) + 1 FROM skills WHERE slug = $1", slug, + "SELECT COALESCE(MAX(version), 0) + 1 FROM skills WHERE slug = $1 AND tenant_id = $2", slug, tenantID, ).Scan(&version); err != nil { tx.Rollback() return 0, nil, fmt.Errorf("get next version: %w", err) diff --git a/internal/store/pg/skills_grants.go b/internal/store/pg/skills_grants.go index aec9a9a8..fa48eae3 100644 --- a/internal/store/pg/skills_grants.go +++ b/internal/store/pg/skills_grants.go @@ -14,15 +14,29 @@ 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). func (s *PGSkillStore) GrantToAgent(ctx context.Context, skillID, agentID uuid.UUID, version int, grantedBy string) 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") + } + _, 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(), tenantIDForInsert(ctx), + store.GenNewID(), skillID, agentID, version, grantedBy, time.Now(), tid, ) if err != nil { return err diff --git a/internal/tools/publish_skill.go b/internal/tools/publish_skill.go index 1a477175..2d08e451 100644 --- a/internal/tools/publish_skill.go +++ b/internal/tools/publish_skill.go @@ -119,7 +119,7 @@ func (t *PublishSkillTool) Execute(ctx context.Context, args map[string]any) *Re } // Version + destination (tenant-scoped) - version := t.skills.GetNextVersion(slug) + version := t.skills.GetNextVersion(ctx, slug) destDir := filepath.Join(t.tenantSkillsDir(ctx), slug, fmt.Sprintf("%d", version)) if err := os.MkdirAll(destDir, 0755); err != nil { return ErrorResult(fmt.Sprintf("failed to create destination: %v", err)) diff --git a/internal/tools/skill_manage.go b/internal/tools/skill_manage.go index f3b7101a..a23645a5 100644 --- a/internal/tools/skill_manage.go +++ b/internal/tools/skill_manage.go @@ -134,7 +134,7 @@ func (t *SkillManageTool) executeCreate(ctx context.Context, args map[string]any } // Version + destination (tenant-scoped) - version := t.skills.GetNextVersion(slug) + version := t.skills.GetNextVersion(ctx, slug) destDir := filepath.Join(t.tenantSkillsDir(ctx), slug, fmt.Sprintf("%d", version)) if err := os.MkdirAll(destDir, 0755); err != nil { return ErrorResult(fmt.Sprintf("failed to create skill directory: %v", err))