move docs to docs/ folder, merge architecture files, update references
This commit is contained in:
526
docs/DEV_PORTAL_M09_REVIEW.md
Normal file
526
docs/DEV_PORTAL_M09_REVIEW.md
Normal file
@@ -0,0 +1,526 @@
|
||||
# Milestone 9: App Review System
|
||||
|
||||
**Status**: Decided
|
||||
**Goal**: Automated and manual review process for app submissions.
|
||||
|
||||
## Decision
|
||||
|
||||
**Go validation workers + SQLite** for self-hosted review pipeline:
|
||||
|
||||
```
|
||||
Validation: Go workers with concurrent file processing
|
||||
Storage: SQLite (review state in portal.db)
|
||||
Queue: In-memory channel + SQLite persistence
|
||||
UI: htmx server-rendered pages (admin section)
|
||||
```
|
||||
|
||||
### Rationale
|
||||
|
||||
1. **Go concurrency** - Process multiple files in parallel with goroutines
|
||||
2. **Single binary** - No separate queue service needed
|
||||
3. **Simple state** - Review state in SQLite alongside app data
|
||||
4. **htmx admin UI** - Server-rendered review queue, no SPA needed
|
||||
|
||||
### Architecture
|
||||
|
||||
```
|
||||
┌─────────────────────────────────────────────────────────────────┐
|
||||
│ mosis-portal container │
|
||||
│ ┌────────────────────────────────────────────────────────────┐ │
|
||||
│ │ Go Binary │ │
|
||||
│ │ ┌─────────────┐ ┌────────────────┐ │ │
|
||||
│ │ │ Upload API │───►│ Review Service │ │ │
|
||||
│ │ │ POST /v1/ │ │ - Queue submit │ │ │
|
||||
│ │ │ versions │ │ - Track state │ │ │
|
||||
│ │ └─────────────┘ └───────┬────────┘ │ │
|
||||
│ │ │ │ │
|
||||
│ │ ┌─────────────────────────▼────────────────────────────┐ │ │
|
||||
│ │ │ Validation Worker Pool │ │ │
|
||||
│ │ │ • Tier 1: Package validation (ZIP, manifest, sig) │ │ │
|
||||
│ │ │ • Tier 2: Content validation (RML, RCSS, Lua) │ │ │
|
||||
│ │ │ • Tier 3: Security analysis (patterns, perms) │ │ │
|
||||
│ │ │ • Tier 4: Quality checks (description, icons) │ │ │
|
||||
│ │ └───────────────────────────────────────────────────────┘ │ │
|
||||
│ │ │ │ │
|
||||
│ │ ┌─────────────────────────▼────────────────────────────┐ │ │
|
||||
│ │ │ Admin Review UI (htmx) │ │ │
|
||||
│ │ │ • /admin/review-queue │ │ │
|
||||
│ │ │ • /admin/review/:id │ │ │
|
||||
│ │ └───────────────────────────────────────────────────────┘ │ │
|
||||
│ └──────────────────────────────┬─────────────────────────────┘ │
|
||||
│ │ │
|
||||
│ /volume1/mosis/ │ │
|
||||
│ ├── data/portal.db ◄───────────┘ │
|
||||
│ └── packages/{dev}/{app}/{ver}/ (validation target) │
|
||||
└─────────────────────────────────────────────────────────────────┘
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Overview
|
||||
|
||||
The review system ensures apps meet quality and security standards before publication. Balances automation with manual review for edge cases.
|
||||
|
||||
---
|
||||
|
||||
## Review Pipeline
|
||||
|
||||
```
|
||||
┌─────────┐ ┌───────────┐ ┌─────────┐ ┌──────────┐ ┌───────────┐
|
||||
│ Submit │──►│ Automated │──►│ Manual │──►│ Approved │──►│ Published │
|
||||
│ │ │ Checks │ │ Review │ │ │ │ │
|
||||
└─────────┘ └───────────┘ └─────────┘ └──────────┘ └───────────┘
|
||||
│ │
|
||||
▼ ▼
|
||||
┌─────────┐ ┌─────────┐
|
||||
│ Failed │ │Rejected │
|
||||
│(auto-fix)│ │(feedback)│
|
||||
└─────────┘ └─────────┘
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Automated Checks
|
||||
|
||||
### Tier 1: Package Validation (Blocking)
|
||||
|
||||
| Check | Description | Action on Fail |
|
||||
|-------|-------------|----------------|
|
||||
| Valid ZIP | Package is valid ZIP format | Reject |
|
||||
| Manifest exists | manifest.json at root | Reject |
|
||||
| Manifest valid | JSON parses, required fields | Reject |
|
||||
| Signature valid | Package signed with registered key | Reject |
|
||||
| Entry exists | Entry point file exists | Reject |
|
||||
| Size limits | Under max package size | Reject |
|
||||
| No forbidden files | No .exe, .dll, etc. | Reject |
|
||||
|
||||
### Tier 2: Content Validation (Blocking)
|
||||
|
||||
| Check | Description | Action on Fail |
|
||||
|-------|-------------|----------------|
|
||||
| RML valid | All RML files parse | Reject |
|
||||
| RCSS valid | All RCSS files parse | Reject |
|
||||
| Lua syntax | All Lua files parse | Reject |
|
||||
| Icons valid | Icons are valid images | Reject |
|
||||
| Icon sizes | Required icon sizes present | Reject |
|
||||
| Path safety | No path traversal attempts | Reject |
|
||||
|
||||
### Tier 3: Security Analysis (Warning/Flag)
|
||||
|
||||
| Check | Description | Action on Fail |
|
||||
|-------|-------------|----------------|
|
||||
| Dangerous patterns | Known malicious Lua patterns | Flag for review |
|
||||
| Excessive permissions | Unusual permission combos | Flag for review |
|
||||
| Obfuscated code | Heavily obfuscated Lua | Flag for review |
|
||||
| External URLs | Hardcoded external URLs | Flag for review |
|
||||
| Large assets | Unusually large files | Warning |
|
||||
|
||||
### Tier 4: Quality Checks (Warning)
|
||||
|
||||
| Check | Description | Action on Fail |
|
||||
|-------|-------------|----------------|
|
||||
| Description length | Meaningful description | Warning |
|
||||
| Release notes | Non-empty release notes | Warning |
|
||||
| Icon quality | Not placeholder/blank | Warning |
|
||||
| Localization | Locale files complete | Warning |
|
||||
|
||||
---
|
||||
|
||||
## Implementation
|
||||
|
||||
### Validation Worker
|
||||
|
||||
```go
|
||||
type ValidationResult struct {
|
||||
Passed bool
|
||||
Errors []ValidationError
|
||||
Warnings []ValidationWarning
|
||||
Flags []ReviewFlag
|
||||
}
|
||||
|
||||
type ValidationError struct {
|
||||
Code string
|
||||
Message string
|
||||
File string
|
||||
Line int
|
||||
}
|
||||
|
||||
func ValidatePackage(packagePath string) ValidationResult {
|
||||
result := ValidationResult{Passed: true}
|
||||
|
||||
// Tier 1: Package validation
|
||||
if err := validateZip(packagePath); err != nil {
|
||||
result.AddError("INVALID_ZIP", err.Error())
|
||||
return result
|
||||
}
|
||||
|
||||
manifest, err := extractManifest(packagePath)
|
||||
if err != nil {
|
||||
result.AddError("INVALID_MANIFEST", err.Error())
|
||||
return result
|
||||
}
|
||||
|
||||
if err := validateSignature(packagePath, manifest); err != nil {
|
||||
result.AddError("INVALID_SIGNATURE", err.Error())
|
||||
return result
|
||||
}
|
||||
|
||||
// Tier 2: Content validation
|
||||
files, _ := listFiles(packagePath)
|
||||
for _, file := range files {
|
||||
switch filepath.Ext(file) {
|
||||
case ".rml":
|
||||
if err := validateRML(file); err != nil {
|
||||
result.AddError("INVALID_RML", err.Error(), file)
|
||||
}
|
||||
case ".rcss":
|
||||
if err := validateRCSS(file); err != nil {
|
||||
result.AddError("INVALID_RCSS", err.Error(), file)
|
||||
}
|
||||
case ".lua":
|
||||
if err := validateLua(file); err != nil {
|
||||
result.AddError("INVALID_LUA", err.Error(), file)
|
||||
}
|
||||
if flags := analyzeLuaSecurity(file); len(flags) > 0 {
|
||||
result.Flags = append(result.Flags, flags...)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Tier 3: Security analysis
|
||||
if hasDangerousPatterns(files) {
|
||||
result.AddFlag("DANGEROUS_PATTERNS", "Code contains suspicious patterns")
|
||||
}
|
||||
|
||||
// Tier 4: Quality checks
|
||||
if len(manifest.Description) < 10 {
|
||||
result.AddWarning("SHORT_DESCRIPTION", "Description is very short")
|
||||
}
|
||||
|
||||
result.Passed = len(result.Errors) == 0
|
||||
return result
|
||||
}
|
||||
```
|
||||
|
||||
### Dangerous Pattern Detection
|
||||
|
||||
```go
|
||||
var dangerousPatterns = []struct {
|
||||
Pattern *regexp.Regexp
|
||||
Reason string
|
||||
}{
|
||||
{
|
||||
regexp.MustCompile(`loadstring\s*\(`),
|
||||
"Dynamic code execution",
|
||||
},
|
||||
{
|
||||
regexp.MustCompile(`debug\s*\.\s*\w+`),
|
||||
"Debug library usage",
|
||||
},
|
||||
{
|
||||
regexp.MustCompile(`os\s*\.\s*execute`),
|
||||
"OS command execution",
|
||||
},
|
||||
{
|
||||
regexp.MustCompile(`io\s*\.\s*\w+`),
|
||||
"Direct I/O operations",
|
||||
},
|
||||
{
|
||||
regexp.MustCompile(`ffi\s*\.\s*\w+`),
|
||||
"FFI usage",
|
||||
},
|
||||
{
|
||||
regexp.MustCompile(`package\s*\.\s*loadlib`),
|
||||
"Native library loading",
|
||||
},
|
||||
}
|
||||
|
||||
func analyzeLuaSecurity(content string) []ReviewFlag {
|
||||
var flags []ReviewFlag
|
||||
for _, dp := range dangerousPatterns {
|
||||
if dp.Pattern.MatchString(content) {
|
||||
flags = append(flags, ReviewFlag{
|
||||
Type: "SECURITY",
|
||||
Reason: dp.Reason,
|
||||
})
|
||||
}
|
||||
}
|
||||
return flags
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Manual Review
|
||||
|
||||
### When Required
|
||||
|
||||
| Trigger | Reason |
|
||||
|---------|--------|
|
||||
| New developer | First app submission |
|
||||
| Dangerous permissions | camera, microphone, contacts, location |
|
||||
| Security flags | Automated checks flagged concerns |
|
||||
| User reports | Existing app reported |
|
||||
| Appeal | Developer contests rejection |
|
||||
|
||||
### Review Queue UI
|
||||
|
||||
```
|
||||
┌─────────────────────────────────────────────────────────────┐
|
||||
│ Review Queue [14 pending] │
|
||||
├─────────────────────────────────────────────────────────────┤
|
||||
│ │
|
||||
│ Filter: [All ▼] [Flagged first ▼] │
|
||||
│ │
|
||||
│ ┌──────────────────────────────────────────────────────┐ │
|
||||
│ │ 🚩 Weather Pro v2.0.0 │ │
|
||||
│ │ com.newdev.weather • New developer │ │
|
||||
│ │ Permissions: location, network │ │
|
||||
│ │ Flags: First submission │ │
|
||||
│ │ Submitted: 2 hours ago │ │
|
||||
│ │ [Review] [Auto-approve]│ │
|
||||
│ ├──────────────────────────────────────────────────────┤ │
|
||||
│ │ 🚩 Photo Editor v1.5.0 │ │
|
||||
│ │ com.trusted.photos • Verified developer │ │
|
||||
│ │ Permissions: camera, storage │ │
|
||||
│ │ Flags: DANGEROUS_PATTERNS (1) │ │
|
||||
│ │ Submitted: 5 hours ago │ │
|
||||
│ │ [Review] [Auto-approve]│ │
|
||||
│ └──────────────────────────────────────────────────────┘ │
|
||||
│ │
|
||||
└─────────────────────────────────────────────────────────────┘
|
||||
```
|
||||
|
||||
### Review Detail View
|
||||
|
||||
```
|
||||
┌─────────────────────────────────────────────────────────────┐
|
||||
│ Review: Weather Pro v2.0.0 │
|
||||
├─────────────────────────────────────────────────────────────┤
|
||||
│ │
|
||||
│ Package: com.newdev.weather │
|
||||
│ Developer: newdev@example.com (New - first app) │
|
||||
│ Submitted: Jan 15, 2024 10:30 AM │
|
||||
│ │
|
||||
│ ┌─────────────────────────────────────────────────────┐ │
|
||||
│ │ Automated Checks │ │
|
||||
│ │ ✓ Package validation passed │ │
|
||||
│ │ ✓ Content validation passed │ │
|
||||
│ │ ⚠ Warning: SHORT_DESCRIPTION │ │
|
||||
│ │ 🚩 Flag: First submission from new developer │ │
|
||||
│ └─────────────────────────────────────────────────────┘ │
|
||||
│ │
|
||||
│ Permissions Requested: │
|
||||
│ • location (fine) - Used for weather forecasts │
|
||||
│ • network - Required for API calls │
|
||||
│ │
|
||||
│ Files: [Expand to browse] │
|
||||
│ ├── manifest.json │
|
||||
│ ├── assets/ │
|
||||
│ │ ├── main.rml │
|
||||
│ │ └── scripts/ │
|
||||
│ │ └── weather.lua [View source] │
|
||||
│ └── icons/ │
|
||||
│ │
|
||||
│ Reviewer Notes: │
|
||||
│ ┌─────────────────────────────────────────────────────┐ │
|
||||
│ │ │ │
|
||||
│ └─────────────────────────────────────────────────────┘ │
|
||||
│ │
|
||||
│ [Approve] [Reject with feedback] [Request changes] │
|
||||
│ │
|
||||
└─────────────────────────────────────────────────────────────┘
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Review States
|
||||
|
||||
```
|
||||
┌─────────┐
|
||||
│ Draft │
|
||||
└────┬────┘
|
||||
│ submit
|
||||
▼
|
||||
┌─────────┐
|
||||
┌───────│Uploaded │
|
||||
│ └────┬────┘
|
||||
│ │ validation
|
||||
│ ▼
|
||||
│ ┌──────────┐
|
||||
failed │ │Validating│
|
||||
│ └────┬─────┘
|
||||
│ │
|
||||
│ ┌───────┴───────┐
|
||||
│ │ │
|
||||
▼ ▼ ▼
|
||||
┌────────┐ ┌─────────┐
|
||||
│ Failed │ │In Review│
|
||||
└────────┘ └────┬────┘
|
||||
│
|
||||
┌───────────┼───────────┐
|
||||
│ │ │
|
||||
▼ ▼ ▼
|
||||
┌──────────┐ ┌────────┐ ┌──────────┐
|
||||
│ Approved │ │Rejected│ │ Changes │
|
||||
└────┬─────┘ └────────┘ │ Requested│
|
||||
│ └──────────┘
|
||||
│ publish
|
||||
▼
|
||||
┌──────────┐
|
||||
│Published │
|
||||
└──────────┘
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Rejection Feedback
|
||||
|
||||
### Feedback Template
|
||||
|
||||
```json
|
||||
{
|
||||
"reason": "SECURITY_CONCERN",
|
||||
"message": "Your app contains patterns that raise security concerns.",
|
||||
"details": [
|
||||
{
|
||||
"file": "scripts/main.lua",
|
||||
"line": 42,
|
||||
"issue": "Usage of loadstring() is not allowed",
|
||||
"suggestion": "Use static Lua code instead of dynamic evaluation"
|
||||
}
|
||||
],
|
||||
"can_resubmit": true,
|
||||
"appeal_available": true
|
||||
}
|
||||
```
|
||||
|
||||
### Common Rejection Reasons
|
||||
|
||||
| Code | Description |
|
||||
|------|-------------|
|
||||
| `SECURITY_CONCERN` | Security issues found |
|
||||
| `QUALITY_ISSUE` | Doesn't meet quality standards |
|
||||
| `POLICY_VIOLATION` | Violates content policy |
|
||||
| `METADATA_ISSUE` | Incorrect/misleading metadata |
|
||||
| `PERMISSION_ABUSE` | Unnecessary permissions |
|
||||
| `COPYRIGHT` | Copyright/trademark issues |
|
||||
|
||||
---
|
||||
|
||||
## Appeal Process
|
||||
|
||||
```
|
||||
1. Developer receives rejection
|
||||
2. Developer clicks "Appeal" (within 14 days)
|
||||
3. Provides justification
|
||||
4. Different reviewer examines
|
||||
5. Final decision (approve/uphold rejection)
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Review SLA
|
||||
|
||||
| Submission Type | Target Time |
|
||||
|-----------------|-------------|
|
||||
| Auto-approved | Instant |
|
||||
| New developer | 24 hours |
|
||||
| Flagged | 48 hours |
|
||||
| Appeal | 72 hours |
|
||||
|
||||
---
|
||||
|
||||
## API Endpoints
|
||||
|
||||
```yaml
|
||||
# Developer endpoints
|
||||
POST /v1/apps/:id/versions/:vid/submit:
|
||||
summary: Submit for review
|
||||
response: { version, estimated_review_time }
|
||||
|
||||
GET /v1/apps/:id/versions/:vid/review-status:
|
||||
summary: Get review status
|
||||
response: { status, feedback?, estimated_completion }
|
||||
|
||||
POST /v1/apps/:id/versions/:vid/appeal:
|
||||
summary: Appeal rejection
|
||||
body: { justification }
|
||||
response: { appeal_id, status }
|
||||
|
||||
# Internal review endpoints (admin only)
|
||||
GET /v1/admin/review-queue:
|
||||
summary: Get pending reviews
|
||||
response: { items[], total }
|
||||
|
||||
GET /v1/admin/review/:version_id:
|
||||
summary: Get review details
|
||||
response: { version, validation_result, flags }
|
||||
|
||||
POST /v1/admin/review/:version_id/approve:
|
||||
summary: Approve version
|
||||
body: { notes? }
|
||||
response: { version }
|
||||
|
||||
POST /v1/admin/review/:version_id/reject:
|
||||
summary: Reject version
|
||||
body: { reason, message, details[] }
|
||||
response: { version }
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Metrics
|
||||
|
||||
### Review Performance
|
||||
|
||||
| Metric | Target |
|
||||
|--------|--------|
|
||||
| Auto-approval rate | > 70% |
|
||||
| Average review time | < 24 hours |
|
||||
| Rejection rate | < 20% |
|
||||
| Appeal overturn rate | < 10% |
|
||||
|
||||
### Dashboard
|
||||
|
||||
```sql
|
||||
-- Review stats
|
||||
SELECT
|
||||
DATE_TRUNC('week', submitted_at) as week,
|
||||
COUNT(*) as submissions,
|
||||
COUNT(*) FILTER (WHERE status = 'published') as approved,
|
||||
COUNT(*) FILTER (WHERE status = 'rejected') as rejected,
|
||||
AVG(EXTRACT(EPOCH FROM (reviewed_at - submitted_at))/3600) as avg_hours
|
||||
FROM app_versions
|
||||
WHERE submitted_at > NOW() - INTERVAL '30 days'
|
||||
GROUP BY week;
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Deliverables
|
||||
|
||||
- [x] Review approach decided (Go workers + SQLite + htmx admin)
|
||||
- [ ] Validation worker implementation (Go concurrent file processing)
|
||||
- [ ] Dangerous pattern database (regex patterns in code)
|
||||
- [ ] Review queue UI (htmx server-rendered)
|
||||
- [ ] Reviewer tools (file browser, source viewer)
|
||||
- [ ] Rejection feedback system
|
||||
- [ ] Appeal workflow
|
||||
- [ ] Review metrics queries
|
||||
|
||||
---
|
||||
|
||||
## Open Questions
|
||||
|
||||
1. ~~Automated approval for trusted developers?~~ → Yes, after 3+ approved apps
|
||||
2. ~~Community moderators?~~ → Defer to post-MVP (single admin for now)
|
||||
3. Content policy document? → Create during M12 Docs
|
||||
4. ~~Rate limiting resubmissions?~~ → Max 3 resubmits per day per app
|
||||
|
||||
---
|
||||
|
||||
## References
|
||||
|
||||
- [Apple App Review Guidelines](https://developer.apple.com/app-store/review/guidelines/)
|
||||
- [Google Play Policy](https://play.google.com/about/developer-content-policy/)
|
||||
Reference in New Issue
Block a user