Add GoClaw Hub marketplace integration

[bnqtoan]

Summary

• Registry client (internal/registry/client.go) — HTTP client for Hub API: search, download, check-update, categories
• 3 agent tools: marketplace_search, marketplace_hire, marketplace_check_updates
• Install confirmation page (/marketplace/install) — server-rendered HTML page for Hub-initiated installs with HMAC signature verification
• Version tracking — saves hub_slug + hub_version in team settings JSONB for update detection
• Duplicate detection — detects already-installed teams and shows "Update" instead of "Install"

How it works

From GoClaw (agent-driven)

• User: "find me a content team" → marketplace_search → shows results with stats
• User: "hire content-squad" → marketplace_hire → downloads bundle, imports team
• User: "check for updates" → marketplace_check_updates → queries Hub for newer versions

From Hub (web-driven)

• User clicks "Install" on Hub listing → Hub generates HMAC-signed URL
• Browser opens GoClaw's /marketplace/install page → shows team details + agents
• Admin clicks "Install Team" → GoClaw downloads bundle from Hub → imports via /v1/teams/import
• Already-installed teams show "Update Team" with warning

Configuration

{
  "tools": {
    "marketplace": {
      "api_base": "https://hub-api.vibery.app/v1",
      "api_key": "your-registry-key"
    }
  }
}

Or env: GOCLAW_MARKETPLACE_API_KEY

Security

• No Hub credentials stored in GoClaw
• Install links signed with HMAC-SHA256 using the shared registry key
• Bundle downloads authenticated via X-Registry-Key header
• Paid listings show purchase link instead of attempting download

Files changed (13 files, +1795 lines)

File	Purpose
internal/registry/client.go	Hub API client
internal/tools/marketplace_search.go	Search tool
internal/tools/marketplace_hire.go	Hire/install tool
internal/tools/marketplace_check_updates.go	Update checker tool
internal/http/marketplace_install_page.go	Web install confirmation page
internal/gateway/server.go	Register install handler
cmd/gateway.go	Wire marketplace tools + handler
cmd/gateway_setup.go	Config reference
internal/config/config_channels.go	Marketplace config struct
docs/marketplace-integration.md	Integration guide

Test plan

• Search: "find content teams" → returns results with ratings, downloads, price
• Hire free team: downloads + imports + team appears in DB
• Hire paid team: shows purchase link with price
• Check updates: finds installed Hub teams, queries for newer versions
• Web install: signed URL → confirmation page → install → success
• Duplicate detection: already-installed team shows "Update" button
• Invalid signature: shows error, blocks install
• Bundle download works through HTTPS/Traefik

[mrgoonie]

📋 PR Review — GoClaw Hub Marketplace Integration

🎯 Tổng quan

PR này thêm marketplace integration để user có thể search, hire (download + install) pre-built AI agent teams từ GoClaw Hub.

Scope: 13 files, +1795 lines

• internal/registry/client.go — Hub API client
• internal/tools/marketplace_*.go — 3 tools (search, hire, check_updates)
• internal/http/marketplace_install_page.go — Web install confirmation page
• Integration vào gateway + config
• Documentation

---

✅ Điểm Tốt

Aspect	Đánh giá
Architecture	✅ Tách biệt rõ ràng: registry client → tools → HTTP handler
Security	✅ HMAC-SHA256 signature verification cho install links
Authentication	✅ Không lưu Hub credentials, dùng X-Registry-Key header
UX	✅ Có confirmation page trước khi install, phân biệt Free/Paid
Error handling	✅ Check empty file, HTTP status, context timeout
Duplicate detection	✅ Phát hiện team đã install, show "Update" thay vì "Install"
Version tracking	✅ Lưu hub_slug + hub_version vào team settings JSONB

---

⚠️ Issues Cần Fix

1. 🔴 CRITICAL — Race condition trong saveHubMetadata()

File: internal/tools/marketplace_hire.go:246

func (t *MarketplaceHireTool) saveHubMetadata(teamName, slug string) {
    // Chạy trong goroutine → race condition nếu hire nhiều teams cùng lúc
    go t.saveHubMetadata(listing.Title, slug)
    
    // teamName có thể không unique
    t.db.Exec(
        `UPDATE agent_teams SET settings = settings || $1::jsonb WHERE name = $2`,
        string(metaJSON), teamName,
    )
}

Vấn đề:

• Goroutine không có context → không thể cancel, không timeout
• teamName không unique → có thể update nhầm team khác
• Silent failure → không check error từ db.Exec()

Fix đề xuất:

func (t *MarketplaceHireTool) saveHubMetadata(ctx context.Context, teamID uuid.UUID, slug string) {
    metadata := map[string]any{
        "hub_slug":    slug,
        "hub_version": 1,
    }
    metaJSON, _ := json.Marshal(metadata)
    
    _, err := t.db.ExecContext(ctx,
        `UPDATE agent_teams SET settings = settings || $1::jsonb WHERE id = $2`,
        string(metaJSON), teamID, // ← Dùng UUID, không dùng name
    )
    if err != nil {
        slog.Warn("marketplace: failed to save hub metadata", "error", err)
    }
}

---

2. 🟡 HIGH — Thiếu validation bundle sau download

File: internal/tools/marketplace_hire.go:156

if written == 0 {
    return ErrorResult(fmt.Sprintf("Download produced empty file for '%s'", listing.Title))
}

Vấn đề:

• Chỉ check file size = 0
• Không validate đây có phải valid tar.gz không
• Không check bundle format có đúng schema không
• Risk: corrupted/malicious bundle có thể được import

Fix đề xuất:

// Validate tar.gz format trước khi import
if err := validateBundleFormat(bundlePath); err != nil {
    return ErrorResult(fmt.Sprintf("Invalid bundle format: %v", err))
}

func validateBundleFormat(path string) error {
    f, err := os.Open(path)
    if err != nil {
        return err
    }
    defer f.Close()
    
    gzr, err := gzip.NewReader(f)
    if err != nil {
        return fmt.Errorf("not a valid gzip file: %w", err)
    }
    defer gzr.Close()
    
    tr := tar.NewReader(gzr)
    _, err = tr.Next()
    return err
}

---

3. 🟡 HIGH — Hardcoded Hub API URL transformation

File: internal/tools/marketplace_hire.go:103

hubURL := strings.TrimSuffix(t.client.BaseURL, "/v1")
buyURL := strings.Replace(hubURL, "hub-api.", "hub.", 1)
buyURL = strings.Replace(buyURL, ":9401", ":9402", 1)

Vấn đề:

• Hardcoded transformation từ API URL → frontend URL
• Magic strings: hub-api., :9401, :9402
• Không work cho custom Hub deployments

Fix đề xuất:
Thêm frontend_base_url vào config:

{
  "tools": {
    "marketplace": {
      "api_base": "https://hub-api.vibery.app/v1",
      "frontend_base": "https://hub.vibery.app",
      "api_key": "..."
    }
  }
}

---

4. 🟡 MEDIUM — Signature verification không có expiry

File: internal/http/marketplace_install_page.go:169

func (h *MarketplaceInstallHandler) verifySignature(params map[string]string) error {
    // Chỉ check HMAC, không check timestamp
}

Vấn đề:

• Install link không có expiry → nếu leak sig, attacker có thể reuse vô thời hạn
• Không có nonce → replay attack possible

Fix đề xuất:
Thêm ts (timestamp) parameter:

func (h *MarketplaceInstallHandler) verifySignature(params map[string]string) error {
    tsStr := params["ts"]
    if tsStr == "" {
        return fmt.Errorf("missing timestamp")
    }
    
    ts, err := strconv.ParseInt(tsStr, 10, 64)
    if err != nil {
        return fmt.Errorf("invalid timestamp")
    }
    
    // Link expires after 15 minutes
    if time.Now().Unix()-ts > 900 {
        return fmt.Errorf("link expired")
    }
    // ... verify HMAC
}

---

5. 🟡 MEDIUM — Không có rate limiting cho download

File: internal/registry/client.go:127

func (c *Client) Download(ctx context.Context, slug string, goclawVersion string) (io.ReadCloser, error) {
    // Không có rate limit
}

Vấn đề:

• User/agent có thể spam download → overload Hub API
• Không có retry logic với exponential backoff

Fix đề xuất:

type Client struct {
    // ...
    rateLimiter *rate.Limiter
}

func NewClient(baseURL, apiKey string) *Client {
    return &Client{
        rateLimiter: rate.NewLimiter(rate.Every(2*time.Second), 5),
    }
}

func (c *Client) Download(...) (io.ReadCloser, error) {
    if err := c.rateLimiter.Wait(ctx); err != nil {
        return nil, fmt.Errorf("rate limit exceeded: %w", err)
    }
    // ...
}

---

6. 🟢 LOW — Thiếu logging cho marketplace operations

File: internal/tools/marketplace_search.go:54

if err != nil {
    return ErrorResult(fmt.Sprintf("Search failed: %v", err))
}

Vấn đề:

• Không log search query, result count → khó debug/audit
• Không track metrics

Fix đề xuất:

slog.Info("marketplace: search", "query", query, "category", category, "results", len(listings))

---

📊 Summary

Severity	Count	Issues
🔴 CRITICAL	1	Race condition in saveHubMetadata()
🟡 HIGH	2	Bundle validation, Hardcoded URL
🟡 MEDIUM	2	Signature expiry, Rate limiting
🟢 LOW	1	Logging

---

💡 Recommendation

🟡 APPROVE WITH CHANGES

Feature có giá trị cao, architecture đúng hướng. Cần fix trước khi merge:

Bắt buộc:

• ✅ Race condition (dùng UUID + context)
• ✅ Bundle format validation
• ✅ Signature expiry (chống replay attack)

Nên có:

• ⚠️ Config option cho frontend URL
• ⚠️ Rate limiting cho download

Follow-up PR:

• 📝 Enhanced logging/metrics

---

Great work @bnqtoan! 🚀 Let me know if you want to discuss any of these points.

[bnqtoan]

All 6 issues addressed in commit 65c7188:

#	Severity	Issue	Fix
1	CRITICAL	Race condition in saveHubMetadata	Removed goroutine, added context, error logging
2	HIGH	No bundle validation	Added gzip validation before import
3	HIGH	Hardcoded URL transformation	Added frontend_base config with fallback
4	MEDIUM	No signature expiry	Added ts param with 15-min expiry check
5	MEDIUM	No rate limiting	Added 200ms min interval to registry client
6	LOW	Missing logging	Added slog.Info to search, hire, check_updates

Hub side also updated (commit 0eee36a) to include ts timestamp in signed install links.

[viettranx]

Review: GoClaw Hub Marketplace Integration

Kiến trúc tổng thể ổn — registry client, agent tools, và server-rendered install page tách biệt rõ ràng. Tuy nhiên phát hiện 26 issues cần xử lý, trong đó 9 là blockers.

---

🔴 Security Critical — Phải fix trước merge

S1. XSS — HTML template không escape input
internal/http/marketplace_install_page.go — renderConfirm, renderSuccess, renderError

Dùng fmt.Sprintf đổ trực tiếp giá trị từ URL params vào HTML mà không escape. title, slug, agents đều controllable qua signed URL. Attacker craft URL với title=<script>alert(1)</script> → XSS.

Fix: Dùng html/template (Go stdlib) thay vì fmt.Sprintf.

---

S2. Credential leak — Registry key trong HTML + URL
marketplace_install_page.go — renderConfirm

<input type="hidden" name="registry_key" value="%s">

GET /marketplace/install là public (chỉ cần signed URL). Bất kỳ ai mở link đều thấy registry_key trong View Source. Key này authenticate với Hub API → full credential leak. Cũng nằm trong URL query params → browser history, server logs, proxy logs, referer headers.

Fix: Server giữ registry key trong memory, không truyền qua URL/form.

---

S3. SSRF — Download bundle không validate URL
marketplace_install_page.go:441 và marketplace_hire.go

Cả 2 nơi download bundle không kiểm tra SSRF. Project đã có CheckSSRF() trong internal/tools/web_shared.go — không được tái sử dụng.

Fix: Gọi CheckSSRF(bundleURL) trước mỗi HTTP request tới external URL.

---

S4. Signed URL không có expiry — Replay attack vĩnh viễn
verifySignature — params được ký: slug, title, agents, bundle_url, registry_key. Không có timestamp, không có nonce. URL hợp lệ bị replay mãi mãi.

Fix: Thêm timestamp param + server reject nếu time.Now() - timestamp > 15 minutes.

---

S5. Debug log in lộ API key
marketplace_install_page.go:579 — fmt.Printf in 8 ký tự đầu API key + 16 ký tự signature ra stdout. Không thể tắt trong production.

Fix: Xóa hoặc dùng slog.Debug không in key material.

---

🔴 Bugs — Phải fix

B1. Read after close — client.go Download():

resp.Body.Close()
body, _ := io.ReadAll(resp.Body)  // đọc body ĐÃ đóng → luôn rỗng

B2. Context leak — client.go Download(): cancel không bao giờ gọi. Comment sai — đóng HTTP body KHÔNG cancel context → goroutine leak.

B3. Nil panic — marketplace_install_page.go:450: dlReq, _ := http.NewRequest(...) ignore error → panic nếu URL malformed.

B4. Test không compile — marketplace_test.go: NewMarketplaceHireTool(client) nhưng constructor cần 4 params (client, gwURL, token, db).

---

🟡 Architecture — Nên fix trước merge

A1. Dùng raw *sql.DB thay vì store interface
Codebase dùng store interface pattern (store.TeamStore, etc.) cho tất cả DB operations. PR truyền pgStores.DB thẳng → phá vỡ abstraction, không hỗ trợ SQLite desktop edition, không thể mock tests, không enforce multi-tenant isolation.

A2. Không propagate tenant context
Queries agent_teams mà KHÔNG filter tenant_id:

SELECT name FROM agent_teams WHERE settings::text LIKE '%' || $1 || '%'

→ Trong multi-tenant deployment, trả về data từ BẤT KỲ tenant nào.

A3. Desktop/Lite edition không được xem xét

• Không check edition.Current() → marketplace tools register trong lite (giới hạn 1 team)
• SQLite không có ::jsonb operator → queries sẽ panic

A4. SQL: LIKE trên JSONB::text

WHERE settings::text LIKE '%hub_slug%'

Full table scan, không tận dụng index, false positives nếu slug chứa %/_.
→ Dùng: WHERE settings->>'hub_slug' = $1

---

🟢 Other Issues

#	Vấn đề
S6	Path traversal — slug trong filepath không validate [a-z0-9-]
S7	os.MkdirAll(dlDir, 0777) — nên 0755
S8	Không giới hạn download size — dùng io.LimitReader
B5	Enabled field trong config bị code bỏ qua
B6	Doc sai env var precedence (nói env > config, code ngược lại)
M1	http.DefaultClient không timeout cho download
M2	go saveHubMetadata(...) fire-and-forget, không log error
M3	saveHubMetadata match by team name (không unique) thay vì id
M4	Thiếu trailing newline ở 6 files

---

Tóm tắt

Mức độ	Số lượng
Security Critical/High	5
Bugs	4
Architecture	4
Other	13

9 blockers cần fix: S1-S5, B1-B4. Architecture issues (A1-A3) strongly recommended trước merge.

[bnqtoan]

All review fixes deployed and verified:

Tests:

• Registry client: 2/2 pass
• Marketplace tools: 3/3 pass (updated mock to return valid gzip, assertions for no-gateway mode)
• Hub smoke tests: 27/28 pass

Deployed to:

• Hub API: https://hub-api.vibery.app (install link now includes ts for 15min expiry)
• GoClaw test: https://goclaw-test.vibery.app (all 6 fixes live)

Ready for re-review.

[bnqtoan]

Addressed all 9 blockers from @viettranx review:

Security (5 fixed):

• S1: html/template replaces fmt.Sprintf — all user input auto-escaped
• S2: registry_key removed from URL and form — server injects from config
• S3: SSRF validation on all bundle download URLs (rejects private IPs, requires HTTPS)
• S5: Debug log replaced with slog.Debug, no key material printed
• S6: Slug validated [a-z0-9-] — blocks path traversal
• S7: Dir permissions 0777 → 0755
• S8: Download size limited to 100MB via io.LimitReader

Bugs (3 fixed):

• B1: Read-after-close in Download() — moved ReadAll before Close
• B2: Context leak — added defer cancel()
• B3: Nil panic — http.NewRequest errors now checked

Architecture:

• A4: Replaced LIKE '%hub_slug%' with settings->>'hub_slug' proper JSONB operator

Tests: 5/5 pass. Build clean.

Note: A1-A3 (store interface, tenant scoping, SQLite/Lite compat) deferred to follow-up — they require broader refactoring.

[mrgoonie]

🔍 Code Review — Add GoClaw Hub marketplace integration

🎯 Tổng quan

PR thêm tích hợp GoClaw Hub marketplace: registry client, 3 agent tools (search, hire, check_updates), và install confirmation page với HMAC signature verification.

Scope: 13 files, +1795 lines

---

✅ Điểm Tốt

1. Security-first design: HMAC-SHA256 signature verification cho install links, SSRF protection với URL validation, rate limiting trong registry client
2. Bundle validation: Check gzip format trước khi import, giới hạn download 100MB
3. Tenant isolation đúng pattern: Uses settings || $1::jsonb để merge metadata, không override settings cũ
4. Test coverage tốt: Unit tests cho registry client + tools với mock server

---

⚠️ Issues Cần Fix

1. 🔴 CRITICAL — Race condition trong registry client rate limiting

File: internal/registry/client.go:27-32

func (c *Client) rateLimit() {
	c.mu.Lock()
	defer c.mu.Unlock()
	elapsed := time.Since(c.lastRequest)
	if elapsed < c.minInterval {
		time.Sleep(c.minInterval - elapsed)
	}
	c.lastRequest = time.Now()
}

Vấn đề:

• time.Sleep() trong lock → block tất cả goroutines khác gọi cùng client
• High concurrency (hundreds of agents searching simultaneously) → severe contention
• Should release lock before sleeping

Fix đề xuất:

func (c *Client) rateLimit() {
	c.mu.Lock()
	elapsed := time.Since(c.lastRequest)
	var wait time.Duration
	if elapsed < c.minInterval {
		wait = c.minInterval - elapsed
	}
	c.lastRequest = time.Now()
	c.mu.Unlock()
	
	if wait > 0 {
		time.Sleep(wait)
	}
}

---

2. 🟡 HIGH — Missing bundle format validation (security)

File: internal/tools/marketplace_hire.go:158-163

// Validate bundle is a valid gzip file
if err := validateBundle(bundlePath); err != nil {
	return ErrorResult(fmt.Sprintf("Bundle validation failed for '%s': %v", listing.Title, err))
}

Vấn đề:

• validateBundle() chỉ check gzip header, không verify tar structure
• Malicious tar.gz có thể chứa path traversal (../../../etc/passwd), symlinks, hoặc file độc hại
• Risk: SSRF + arbitrary file write khi extract

Fix đề xuất:

func validateBundle(path string) error {
	f, err := os.Open(path)
	if err != nil {
		return err
	}
	defer f.Close()
	
	gz, err := gzip.NewReader(f)
	if err != nil {
		return fmt.Errorf("not a valid gzip file: %w", err)
	}
	defer gz.Close()
	
	tr := tar.NewReader(gz)
	seenFiles := 0
	for {
		header, err := tr.Next()
		if err == io.EOF {
			break
		}
		if err != nil {
			return fmt.Errorf("invalid tar: %w", err)
		}
		
		// Block path traversal
		if strings.Contains(header.Name, "..") || filepath.IsAbs(header.Name) {
			return fmt.Errorf("blocked path traversal: %s", header.Name)
		}
		
		// Block symlinks
		if header.Typeflag == tar.TypeSymlink || header.Typeflag == tar.TypeLink {
			return fmt.Errorf("blocked symlink: %s", header.Name)
		}
		
		seenFiles++
		if seenFiles > 1000 {
			return fmt.Errorf("too many files in bundle")
		}
	}
	
	return nil
}

---

3. 🟡 HIGH — Hardcoded API base URL

File: internal/registry/client.go:22

func NewClient(baseURL, apiKey string) *Client {
	// ...
}

File: cmd/gateway.go:414-416

apiBase := cfg.Tools.Marketplace.APIBase
if apiBase == "" {
	apiBase = "https://hub-api.vibery.app/v1"
}

Vấn đề:

• Default URL hardcoded → khó test với mock server, khó deploy on-prem
• Should be config-driven với clear error nếu thiếu

Fix đề xuất:

// internal/config/config_channels.go
type MarketplaceConfig struct {
	Enabled      bool   `json:"enabled,omitempty"`
	APIBase      string `json:"api_base"`              // Required
	APIKey       string `json:"api_key"`               // Required
	FrontendBase string `json:"frontend_base,omitempty"`
}

// cmd/gateway.go
if cfg.Tools.Marketplace.APIBase == "" || cfg.Tools.Marketplace.APIKey == "" {
	slog.Warn("marketplace tools disabled: missing api_base or api_key")
} else {
	mktClient := registry.NewClient(cfg.Tools.Marketplace.APIBase, cfg.Tools.Marketplace.APIKey)
	// ...
}

---

4. 🟡 MEDIUM — Missing signature expiry check trong download

File: internal/registry/client.go:176-201

func (c *Client) Download(ctx context.Context, slug string, goclawVersion string) (io.ReadCloser, error) {
	// ...
	dlCtx, cancel := context.WithTimeout(context.Background(), 60*time.Second)
	// ...
}

Vấn đề:

• Download URL từ Hub có thể có expiry (signed URL)
• Không có mechanism để detect expired download links
• Should add retry với fresh link nếu download fails với 403/410

---

5. 🟡 MEDIUM — Missing rate limiting metrics

File: internal/registry/client.go:27-32

func (c *Client) rateLimit() {
	// ...
}

Vấn đề:

• Rate limiting nhưng không log khi bị throttle
• Khó debug performance issues dưới load
• Should add debug log: slog.Debug("rate limited", "wait", wait)

---

6. 🟢 LOW — Missing error context trong SQL query

File: internal/http/marketplace_install_page.go:129-132

func (h *MarketplaceInstallHandler) findInstalledTeam(slug string) string {
	if h.db == nil || slug == "" {
		return ""
	}
	var name string
	h.db.QueryRow(`SELECT id, name FROM agent_teams WHERE settings->>'hub_slug' = $1 LIMIT 1`, slug).Scan(&name)
	return name
}

Vấn đề:

• Scan() error ignored → nếu query fails (DB connection lost) → silent failure
• Should log error: if err := rows.Scan(&id, &name); err != nil { slog.Warn(...); return "" }

---

📊 Summary

Severity	Count	Issues
🔴 CRITICAL	1	Race condition in rate limiting (lock held during sleep)
🟡 HIGH	2	Missing tar validation, hardcoded API URL
🟡 MEDIUM	2	Missing download link expiry handling, no rate limit metrics
🟢 LOW	1	Ignored SQL error in findInstalledTeam

---

💡 Recommendation

🟡 APPROVE WITH CHANGES

Blocking issues (must fix before merge):

1. 🔴 Race condition in rateLimit() — potential performance bottleneck under load
2. 🟡 Bundle validation — security risk (path traversal, symlinks)

Should fix (can be post-merge):
3. 🟡 Hardcoded API URL — make config required
4. 🟡 MEDIUM issues — observability improvements

---

Great work @bnqtoan! 🚀 Marketplace integration là một tính năng phức tạp và PR được viết rất cẩn thận. Let me know if you want to discuss any of these points.

[bnqtoan]

Addressed @mrgoonie's review (6 items):

#	Severity	Issue	Fix
1	CRITICAL	Rate limiter holds lock during sleep	Release lock before time.Sleep()
2	HIGH	Bundle only validates gzip header	Full tar validation: path traversal, symlinks, file count (1000 max). Note: handleTeamImport → readTeamImportArchive() already validates tar structure during actual import — this is defense-in-depth
3	HIGH	Hardcoded default API URL	Removed. Both api_base and api_key now required. Warning logged if key set without base
4	MEDIUM	No rate limit logging	Added slog.Debug with wait duration
5	MEDIUM	Download retry	Skipped — download links don't expire in our implementation
6	LOW	SQL error ignored	Added slog.Warn for non-ErrNoRows errors

Tests: 5/5 pass. Build clean.

[bnqtoan]

Final self-review before merge

Ran full diff audit (2167 lines). Here's what remains:

Must fix before merge

#	Issue	Action
1	docker-compose.test-deploy.yml contains real API key	Remove from PR — deployment artifact, not source
2	docs/marketplace-integration.md is stale	References old default URL (removed), wrong env precedence, emoji in examples

Clarifications for reviewers

Re @viettranx A1 (raw *sql.DB vs store interface): Acknowledged — the marketplace tools bypass the store interface pattern. This is intentional for the initial PR to avoid coupling to GoClaw's internal store interfaces which may change. A follow-up PR should wrap this in a MarketplaceMetadataStore interface. Tracked.

Re @viettranx A3 (desktop/Lite edition): Marketplace tools register unconditionally when API key is set. For Lite edition (1 team limit), the team import handler itself enforces limits — marketplace doesn't need separate checking. SQLite ->>'hub_slug' syntax is supported in SQLite 3.38+ (2022) which all editions use.

Re @mrgoonie #2 (bundle tar validation): Added full tar structure validation (path traversal, symlinks, file count) as defense-in-depth. Note: the primary security gate is handleTeamImport → readTeamImportArchive() which already validates tar structure, blocks .. paths, and verifies the bundle schema. The pre-flight check in validateBundle() is an early rejection to avoid POSTing obviously invalid files to the import endpoint.

Re @viettranx M3 (saveHubMetadata matches by name): Still matches by team name because the import response (TeamImportSummary) only returns team_name + agent_keys, not the team UUID. The WHERE clause also checks settings->>'hub_slug' IS NULL to only update newly-created teams, reducing false positives. Proper fix requires upstream change to return team ID from import.

Minor (post-merge)

• marketplace_search.go still uses emoji in LLM tool output (functional, not web UI)
• example-marketplace-config.json has enabled field that code ignores
• Categories() doesn't call rateLimit() — will fix
• Missing trailing newlines on 5 files

Will push the must-fix items now.

[mrgoonie]

🔍 Re-Review — PR #639 (After fixes in 2d5aef6)

🎯 Tổng quan

Re-review sau khi author đã address 26 issues từ @viettranx + fixes từ reviewers trước. Commit mới nhất: 2d5aef6

Scope: 13 files, +1795 lines (same)

---

✅ Fixes Verified

Issue	Severity	Status	Fix Summary
Race condition in rateLimit()	🔴 CRITICAL	✅ Fixed	Lock released before time.Sleep()
Bundle validation missing	🟡 HIGH	✅ Fixed	validateBundle() checks gzip + tar, blocks path traversal + symlinks, limits 1000 files
Hardcoded URL transformation	🟡 HIGH	✅ Fixed	Added FrontendBase config with fallback derivation
No signature expiry	🟡 MEDIUM	✅ Fixed	ts param + 15min expiry check in verifySignature()
Missing logging	🟢 LOW	✅ Fixed	slog.Info added to search/hire/check_updates
XSS (fmt.Sprintf HTML)	🔴 Security	✅ Fixed	Using template.HTMLEscape() in renderConfirm
Credential leak (registry_key in URL)	🔴 Security	✅ Fixed	Key injected server-side, not from URL/form
SSRF (no URL validation)	🔴 Security	✅ Fixed	validateExternalURL() with DNS resolution + private IP check
Debug log leak API key	🔴 Security	✅ Fixed	Only logs "match": boolean
Read after close (B1)	🔴 Bug	✅ Fixed	body read before resp.Body.Close()
Context leak (B2)	🔴 Bug	✅ Fixed	contextCloser wrapper cancels context on Close()
Nil panic (B3)	🔴 Bug	✅ Fixed	Error checked from http.NewRequest()
Test compile fail (B4)	🔴 Bug	✅ Fixed	Tests updated with correct constructor params

---

⚠️ Remaining Issues (Post-fix)

1. 🟡 MEDIUM — Raw *sql.DB instead of store interface

File: internal/tools/marketplace_hire.go:251-262, internal/http/marketplace_install_page.go:129-134

// Still using raw *sql.DB
_, err := t.db.ExecContext(ctx,
    `UPDATE agent_teams SET settings = settings || $1::jsonb WHERE name = $2 ...`,
    string(metaJSON), teamName,
)

Vấn đề:

• Codebase dùng store.TeamStore interface pattern
• Raw DB breaks abstraction → không mock được tests, không hỗ trợ SQLite desktop edition
• Query WHERE name = $2 không unique → potential race condition

Fix đề xuất (follow-up):

// Use store interface + team ID instead of name
_, err := s.teamStore.UpdateSettings(ctx, teamID, map[string]any{
    "hub_slug": slug,
    "hub_version": 1,
})

---

2. 🟡 MEDIUM — Missing tenant isolation

File: internal/tools/marketplace_check_updates.go:55, internal/http/marketplace_install_page.go:129

SELECT id, name, settings FROM agent_teams 
WHERE settings->>'hub_slug' IS NOT NULL

Vấn đề:

• Không filter tenant_id → trong multi-tenant deployment, query trả về data từ ALL tenants
• Violates tenant isolation principle

Fix đề xuất (follow-up):

// Add tenant_id filter
WHERE tenant_id = $1 AND settings->>'hub_slug' IS NOT NULL

---

3. 🟡 MEDIUM — SQLite/JSONB compatibility

File: internal/tools/marketplace_check_updates.go:55

WHERE settings->>'hub_slug' IS NOT NULL

Vấn đề:

• ->> JSONB operator là PostgreSQL-specific
• SQLite dùng json_extract(settings, '$.hub_slug')
• Desktop edition sẽ panic khi chạy query này

Fix đề xuất (follow-up):

• Abstract query behind store.TeamStore interface
• Implement PG + SQLite versions separately

---

4. 🟢 LOW — Missing trailing newlines

File: 6 files (same as @viettranx noted)

• internal/registry/client.go
• internal/tools/marketplace_search.go
• internal/tools/marketplace_hire.go
• internal/tools/marketplace_check_updates.go
• internal/tools/marketplace_test.go
• example-marketplace-config.json

---

📊 Summary

Severity	Count	Status
🔴 CRITICAL	9 (security + bugs)	✅ All fixed
🟡 HIGH	2	✅ All fixed
🟡 MEDIUM	4	⚠️ 3 remaining (architecture, follow-up)
🟢 LOW	13	⚠️ 4 remaining (newlines, minor)

---

💡 Recommendation

🟢 APPROVE

Rationale:

• ✅ All 9 blockers (S1-S5, B1-B4) from @viettranx đã fix
• ✅ Security issues addressed: XSS, credential leak, SSRF, signature expiry
• ✅ Bug fixes verified: race condition, context leak, nil panic, test compile
• ⚠️ Architecture issues (A1-A3) là follow-up improvements, không blocking

Recommended follow-up PRs:

1. Abstract DB queries behind store.TeamStore interface (supports SQLite + tenant isolation)
2. Add edition gating for lite mode (5 agents, 1 team limit)
3. Add metrics/observability (Prometheus counters for search/hire/update operations)

---

Great work @bnqtoan! 🚀 Fast turnaround on 26 issues — impressive! Architecture improvements can be done in follow-up PRs without blocking this feature.

[bnqtoan]

Addressed remaining review items

Fixed in latest commits:

A1 — Raw *sql.DB → store interface

• MarketplaceHireTool, MarketplaceCheckUpdatesTool, MarketplaceInstallHandler now accept store.TeamCRUDStore instead of *sql.DB
• Wiring in cmd/gateway.go updated to pass pgStores.Teams

A2 — Tenant scoping

• All raw SQL queries replaced with ListTeams() which is already tenant-scoped by the store layer

A3 — SQLite/Desktop compatibility

• No more raw JSONB queries (->>'hub_slug') — all filtering done in Go after ListTeams()
• Works with both PG and SQLite stores

M1 — Download timeout

• marketplace_hire.go download now uses &http.Client{Timeout: 60s} instead of http.DefaultClient

M2 — saveHubMetadata error handling

• Caller now checks and logs errors from saveHubMetadata

M3 — Team matching by hub_slug instead of name

• saveHubMetadata now finds teams by matching hub_slug in settings JSON, not by name

M4 — Trailing newlines

• Added to marketplace_test.go, marketplace_search.go, client.go

Also includes fix for team duplication on marketplace install — agents are now reused by key instead of creating duplicates, and teams are updated by name instead of always inserting new.