# 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/)
