add app review system with validation pipeline and admin htmx UI
This commit is contained in:
405
portal/internal/review/service.go
Normal file
405
portal/internal/review/service.go
Normal file
@@ -0,0 +1,405 @@
|
||||
// Package review provides app review and validation services
|
||||
package review
|
||||
|
||||
import (
|
||||
"archive/zip"
|
||||
"context"
|
||||
"fmt"
|
||||
"io"
|
||||
"regexp"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"github.com/omixlab/mosis-portal/internal/database"
|
||||
"github.com/omixlab/mosis-portal/pkg/mospkg"
|
||||
)
|
||||
|
||||
// ReviewFlag represents a security or quality flag
|
||||
type ReviewFlag struct {
|
||||
Type string `json:"type"`
|
||||
Severity string `json:"severity"` // "info", "warning", "critical"
|
||||
Reason string `json:"reason"`
|
||||
File string `json:"file,omitempty"`
|
||||
Line int `json:"line,omitempty"`
|
||||
}
|
||||
|
||||
// FullValidationResult extends ValidationResult with security flags
|
||||
type FullValidationResult struct {
|
||||
*mospkg.ValidationResult
|
||||
Flags []ReviewFlag `json:"flags,omitempty"`
|
||||
RequiresManual bool `json:"requires_manual"`
|
||||
AutoApprovable bool `json:"auto_approvable"`
|
||||
ValidationTimeMs int64 `json:"validation_time_ms"`
|
||||
}
|
||||
|
||||
// Service handles app review operations
|
||||
type Service struct {
|
||||
db *database.DB
|
||||
}
|
||||
|
||||
// New creates a new review service
|
||||
func New(db *database.DB) *Service {
|
||||
return &Service{db: db}
|
||||
}
|
||||
|
||||
// ValidatePackage performs full validation on a package
|
||||
func (s *Service) ValidatePackage(packagePath string) (*FullValidationResult, error) {
|
||||
start := time.Now()
|
||||
|
||||
// Run basic validation
|
||||
basicResult, err := mospkg.ValidatePackage(packagePath)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
result := &FullValidationResult{
|
||||
ValidationResult: basicResult,
|
||||
Flags: []ReviewFlag{},
|
||||
AutoApprovable: true,
|
||||
}
|
||||
|
||||
// If basic validation failed, no need to continue
|
||||
if !basicResult.Valid {
|
||||
result.AutoApprovable = false
|
||||
result.ValidationTimeMs = time.Since(start).Milliseconds()
|
||||
return result, nil
|
||||
}
|
||||
|
||||
// Run security analysis (Tier 3)
|
||||
flags, err := s.analyzeSecurityRisks(packagePath)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
result.Flags = append(result.Flags, flags...)
|
||||
|
||||
// Run quality checks (Tier 4)
|
||||
qualityFlags := s.checkQuality(basicResult.Manifest)
|
||||
result.Flags = append(result.Flags, qualityFlags...)
|
||||
|
||||
// Determine if manual review is required
|
||||
result.RequiresManual = s.requiresManualReview(result)
|
||||
if result.RequiresManual {
|
||||
result.AutoApprovable = false
|
||||
}
|
||||
|
||||
// Check for critical flags
|
||||
for _, flag := range result.Flags {
|
||||
if flag.Severity == "critical" {
|
||||
result.AutoApprovable = false
|
||||
}
|
||||
}
|
||||
|
||||
result.ValidationTimeMs = time.Since(start).Milliseconds()
|
||||
return result, nil
|
||||
}
|
||||
|
||||
// SubmitForReview submits a version for review
|
||||
func (s *Service) SubmitForReview(ctx context.Context, versionID string) error {
|
||||
return s.db.UpdateVersionStatus(ctx, versionID, "in_review")
|
||||
}
|
||||
|
||||
// ApproveVersion approves a version
|
||||
func (s *Service) ApproveVersion(ctx context.Context, versionID, reviewerNotes string) error {
|
||||
return s.db.ApproveVersion(ctx, versionID, reviewerNotes)
|
||||
}
|
||||
|
||||
// RejectVersion rejects a version with feedback
|
||||
func (s *Service) RejectVersion(ctx context.Context, versionID string, feedback *RejectionFeedback) error {
|
||||
return s.db.RejectVersion(ctx, versionID, feedback.Reason, feedback.Message)
|
||||
}
|
||||
|
||||
// RejectionFeedback contains rejection details
|
||||
type RejectionFeedback struct {
|
||||
Reason string `json:"reason"`
|
||||
Message string `json:"message"`
|
||||
Details []RejectionDetail `json:"details,omitempty"`
|
||||
CanResubmit bool `json:"can_resubmit"`
|
||||
}
|
||||
|
||||
// RejectionDetail provides specific feedback for an issue
|
||||
type RejectionDetail struct {
|
||||
File string `json:"file"`
|
||||
Line int `json:"line,omitempty"`
|
||||
Issue string `json:"issue"`
|
||||
Suggestion string `json:"suggestion,omitempty"`
|
||||
}
|
||||
|
||||
// Dangerous patterns to detect in Lua code
|
||||
var dangerousPatterns = []struct {
|
||||
Pattern *regexp.Regexp
|
||||
Reason string
|
||||
Severity string
|
||||
}{
|
||||
{
|
||||
regexp.MustCompile(`loadstring\s*\(`),
|
||||
"Dynamic code execution via loadstring",
|
||||
"critical",
|
||||
},
|
||||
{
|
||||
regexp.MustCompile(`load\s*\([^)]*\)`),
|
||||
"Dynamic code loading",
|
||||
"critical",
|
||||
},
|
||||
{
|
||||
regexp.MustCompile(`debug\s*\.\s*\w+`),
|
||||
"Debug library usage",
|
||||
"critical",
|
||||
},
|
||||
{
|
||||
regexp.MustCompile(`os\s*\.\s*execute`),
|
||||
"OS command execution",
|
||||
"critical",
|
||||
},
|
||||
{
|
||||
regexp.MustCompile(`os\s*\.\s*remove`),
|
||||
"File deletion via os.remove",
|
||||
"critical",
|
||||
},
|
||||
{
|
||||
regexp.MustCompile(`io\s*\.\s*(open|popen|read|write|lines)`),
|
||||
"Direct file I/O operations",
|
||||
"critical",
|
||||
},
|
||||
{
|
||||
regexp.MustCompile(`ffi\s*\.\s*\w+`),
|
||||
"FFI (foreign function interface) usage",
|
||||
"critical",
|
||||
},
|
||||
{
|
||||
regexp.MustCompile(`package\s*\.\s*loadlib`),
|
||||
"Native library loading",
|
||||
"critical",
|
||||
},
|
||||
{
|
||||
regexp.MustCompile(`package\s*\.\s*cpath`),
|
||||
"C library path modification",
|
||||
"critical",
|
||||
},
|
||||
{
|
||||
regexp.MustCompile(`rawset\s*\(\s*_G`),
|
||||
"Global environment modification",
|
||||
"warning",
|
||||
},
|
||||
{
|
||||
regexp.MustCompile(`setfenv\s*\(`),
|
||||
"Environment modification",
|
||||
"warning",
|
||||
},
|
||||
{
|
||||
regexp.MustCompile(`getfenv\s*\(`),
|
||||
"Environment access",
|
||||
"warning",
|
||||
},
|
||||
{
|
||||
regexp.MustCompile(`https?://[^\s"']+`),
|
||||
"Hardcoded external URL",
|
||||
"info",
|
||||
},
|
||||
{
|
||||
regexp.MustCompile(`require\s*\(\s*["'][^"']+["']\s*\)`),
|
||||
"Module require (verify allowed modules)",
|
||||
"info",
|
||||
},
|
||||
}
|
||||
|
||||
func (s *Service) analyzeSecurityRisks(packagePath string) ([]ReviewFlag, error) {
|
||||
var flags []ReviewFlag
|
||||
|
||||
reader, err := zip.OpenReader(packagePath)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
defer reader.Close()
|
||||
|
||||
for _, file := range reader.File {
|
||||
if file.FileInfo().IsDir() {
|
||||
continue
|
||||
}
|
||||
|
||||
ext := strings.ToLower(strings.TrimPrefix(file.Name, "."))
|
||||
if !strings.HasSuffix(file.Name, ".lua") {
|
||||
continue
|
||||
}
|
||||
|
||||
// Read Lua file content
|
||||
rc, err := file.Open()
|
||||
if err != nil {
|
||||
continue
|
||||
}
|
||||
|
||||
content, err := io.ReadAll(io.LimitReader(rc, 1024*1024)) // 1MB limit
|
||||
rc.Close()
|
||||
if err != nil {
|
||||
continue
|
||||
}
|
||||
|
||||
contentStr := string(content)
|
||||
|
||||
// Check against dangerous patterns
|
||||
for _, dp := range dangerousPatterns {
|
||||
if dp.Pattern.MatchString(contentStr) {
|
||||
// Find line number
|
||||
lineNum := findLineNumber(contentStr, dp.Pattern)
|
||||
|
||||
flags = append(flags, ReviewFlag{
|
||||
Type: "SECURITY",
|
||||
Severity: dp.Severity,
|
||||
Reason: dp.Reason,
|
||||
File: file.Name,
|
||||
Line: lineNum,
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// Check for obfuscated code (high entropy, meaningless variable names)
|
||||
if isLikelyObfuscated(contentStr) {
|
||||
flags = append(flags, ReviewFlag{
|
||||
Type: "SECURITY",
|
||||
Severity: "warning",
|
||||
Reason: "Code appears to be obfuscated",
|
||||
File: file.Name,
|
||||
})
|
||||
}
|
||||
|
||||
_ = ext // Unused but may be useful for future file type checks
|
||||
}
|
||||
|
||||
return flags, nil
|
||||
}
|
||||
|
||||
func findLineNumber(content string, pattern *regexp.Regexp) int {
|
||||
loc := pattern.FindStringIndex(content)
|
||||
if loc == nil {
|
||||
return 0
|
||||
}
|
||||
|
||||
lineNum := 1
|
||||
for i := 0; i < loc[0] && i < len(content); i++ {
|
||||
if content[i] == '\n' {
|
||||
lineNum++
|
||||
}
|
||||
}
|
||||
return lineNum
|
||||
}
|
||||
|
||||
func isLikelyObfuscated(content string) bool {
|
||||
// Simple heuristics for obfuscation detection:
|
||||
// 1. High ratio of single-character variable names
|
||||
// 2. Many string.char() calls
|
||||
// 3. Long lines with minimal whitespace
|
||||
|
||||
singleCharVars := regexp.MustCompile(`\blocal\s+[a-z]\s*=`)
|
||||
matches := singleCharVars.FindAllString(content, -1)
|
||||
if len(matches) > 20 {
|
||||
return true
|
||||
}
|
||||
|
||||
stringCharCalls := strings.Count(content, "string.char")
|
||||
if stringCharCalls > 10 {
|
||||
return true
|
||||
}
|
||||
|
||||
// Check for long lines (obfuscators often produce very long lines)
|
||||
lines := strings.Split(content, "\n")
|
||||
longLines := 0
|
||||
for _, line := range lines {
|
||||
if len(line) > 500 {
|
||||
longLines++
|
||||
}
|
||||
}
|
||||
if longLines > 3 {
|
||||
return true
|
||||
}
|
||||
|
||||
return false
|
||||
}
|
||||
|
||||
func (s *Service) checkQuality(manifest *mospkg.Manifest) []ReviewFlag {
|
||||
var flags []ReviewFlag
|
||||
|
||||
if manifest == nil {
|
||||
return flags
|
||||
}
|
||||
|
||||
// Check description length
|
||||
if len(manifest.Description) < 10 {
|
||||
flags = append(flags, ReviewFlag{
|
||||
Type: "QUALITY",
|
||||
Severity: "info",
|
||||
Reason: "Description is very short (less than 10 characters)",
|
||||
})
|
||||
}
|
||||
|
||||
// Check for placeholder icon paths
|
||||
if manifest.Icons.Size32 == "" && manifest.Icons.Size64 == "" && manifest.Icons.Size128 == "" {
|
||||
flags = append(flags, ReviewFlag{
|
||||
Type: "QUALITY",
|
||||
Severity: "warning",
|
||||
Reason: "No icons specified in manifest",
|
||||
})
|
||||
}
|
||||
|
||||
// Check for sensitive permissions
|
||||
sensitivePerms := map[string]bool{
|
||||
"camera": true,
|
||||
"microphone": true,
|
||||
"contacts": true,
|
||||
"location": true,
|
||||
}
|
||||
for _, perm := range manifest.Permissions {
|
||||
if sensitivePerms[perm] {
|
||||
flags = append(flags, ReviewFlag{
|
||||
Type: "PERMISSION",
|
||||
Severity: "info",
|
||||
Reason: fmt.Sprintf("App requests sensitive permission: %s", perm),
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
return flags
|
||||
}
|
||||
|
||||
func (s *Service) requiresManualReview(result *FullValidationResult) bool {
|
||||
// Always require manual review for:
|
||||
// 1. Any critical security flags
|
||||
// 2. More than 3 warnings
|
||||
// 3. Sensitive permissions
|
||||
|
||||
criticalCount := 0
|
||||
warningCount := 0
|
||||
hasSensitivePerms := false
|
||||
|
||||
for _, flag := range result.Flags {
|
||||
switch flag.Severity {
|
||||
case "critical":
|
||||
criticalCount++
|
||||
case "warning":
|
||||
warningCount++
|
||||
}
|
||||
if flag.Type == "PERMISSION" {
|
||||
hasSensitivePerms = true
|
||||
}
|
||||
}
|
||||
|
||||
if criticalCount > 0 {
|
||||
return true
|
||||
}
|
||||
if warningCount > 3 {
|
||||
return true
|
||||
}
|
||||
if hasSensitivePerms {
|
||||
return true
|
||||
}
|
||||
|
||||
return false
|
||||
}
|
||||
|
||||
// GetReviewQueue returns versions pending review
|
||||
func (s *Service) GetReviewQueue(ctx context.Context, limit, offset int) ([]database.VersionWithApp, int, error) {
|
||||
return s.db.GetVersionsInReview(ctx, limit, offset)
|
||||
}
|
||||
|
||||
// GetReviewDetails returns details for a specific version under review
|
||||
func (s *Service) GetReviewDetails(ctx context.Context, versionID string) (*database.VersionWithApp, error) {
|
||||
return s.db.GetVersionWithApp(ctx, versionID)
|
||||
}
|
||||
Reference in New Issue
Block a user