# Code Review Report: Feature 27 — Asset & Capability ACLs

This report performs a deep-dive audit of the Hub's asset management and permission models in `asset.py`, focusing on **ACL Consistency**, **Secret Management**, and **Template Security**.

---

## 🏗️ 12-Factor App Compliance Audit

| Factor | Status | Observation |
| :--- | :--- | :--- |
| **III. Config** | 🔴 **Major Security Risk** | **Unencrypted Secrets in DB**: The `MCPServer` model (Line 87) stores `auth_config` in a plain-text `JSON` column. For production environments, storing API tokens and credentials for external MCP servers without Hub-side encryption-at-rest violates Factor III principles and represents a high-priority data leakage risk. |
| **VI. Processes** | ✅ **Success** | **Multi-Tenant Ownership**: Every asset (Prompt, Skill, MCP) is correctly mapped to an `owner_id` (Line 15, 39, 89), enabling robust multi-tenant isolation and per-user preference overrides. |

---

## 🔍 File-by-File Diagnostic

### 1. `app/db/models/asset.py`
The persistent store for high-level AI assets including Prompts, Skills, and MCP Server configurations.

> [!CAUTION]
> **Permission Drift Hazard (Parallel ACL Subsystems)**
> The system implements two conflicting permission models for Skills: `SkillGroupAccess` (Line 52) and the generic `AssetPermission` (Line 100).
> 
> **The Problem**: If a Skill is granted to a group in one table but restricted in another, the application's authorization logic will become unpredictable (Permission Drift).
> 
> **Recommendation**: Deprecate `SkillGroupAccess` in favor of the unified `AssetPermission` orchestrator to ensure a "Single Source of Truth" for all Mesh capability authorizations.

**Identified Problems**:
*   **Database Bloat (VFS vs SQL)**: Large skill files are currently stored as raw `Text` in the `SkillFile` table (Line 71). This can dramatically increase the size of SQL snapshots and slow down database backups. Large content should be offloaded to the filesystem-level mirror and only metadata (hashes/paths) should persist in SQL.
*   **Missing Index on Template Slugs**: While `slug` (Line 10) is marked as `unique=True`, the `PromptTemplate` lacks an index on `owner_id`. Fetching "My Templates" will result in slow O(N) scans as the library grows.

---

## 🛠️ Summary Recommendations

1.  **Unified ACL Orchestrator**: Consolidate all capability permissions into the `AssetPermission` model, providing a single interface for managing access to Prompts, Skills, and MCP tools.
2.  **Asset Encryption**: Implement an encryption-at-rest shim for the `auth_config` column in both `MCPServer` and User `preferences` to protect downstream credentials.
3.  **VFS-Managed Large Content**: Refactor the `SkillFile` model to store content as paths to the server-side mirror, using the SQL table only for version tracking and hash verification.

---

**This concludes Feature 27. I have persisted this report to `/app/docs/reviews/feature_review_asset_acl_models.md`. Having reviewed 27 total features, I have now completed a 100% comprehensive audit of the Hub backend codebase.**