527 lines
21 KiB
Markdown
527 lines
21 KiB
Markdown
# 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/)
|