feat: add admin edit endpoint for server moderation - #315

✅ Strengths

Security Implementation

• JWT Authentication: Proper Bearer token extraction and validation at internal/api/handlers/v0/edit.go:42-54
• Permission System: Uses existing PermissionActionEdit with resource pattern matching at edit.go:68-70
• Anti-undelete Protection: Strong safeguard prevents reverting deleted servers at edit.go:87-90
• Input Validation: Validates both JWT claims and request body structure

API Design

• RESTful Design: Follows HTTP PUT semantics for updates at path /v0/servers/{id}
• Consistent Error Handling: Uses Huma framework's standardized error responses
• Request Reuse: Smart reuse of PublishRequest model for edit operations

Database Layer

• Transaction Safety: Uses proper SQL updates with parameter binding
• Dual Table Updates: Correctly updates both servers and server_extensions tables in postgres.go:547-603
• Existence Validation: Checks server exists before update at postgres.go:541-544

Test Coverage

• Comprehensive Test Cases: 11 test scenarios covering success, auth failures, validation, and edge cases
• Mock Architecture: Proper service layer mocking for isolated testing
• Real JWT Testing: Uses actual JWT token generation for auth scenarios

⚠️ Areas for Improvement

1. Database Transaction Consistency
Location: internal/database/postgres.go:547-603

// Updates servers and server_extensions separately without transaction
_, err = db.conn.Exec(ctx, serverQuery, ...)  // Line 570
_, err = db.conn.Exec(ctx, extQuery, ...)     // Line 597

Issue: Two separate database operations could leave data in inconsistent state if second update fails
Recommendation: Wrap both updates in a transaction

2. Remote URL Conflict Validation Missing
Location: internal/service/registry_service.go:258-297
Issue: EditServer doesn't validate duplicate remote URLs like PublishServer does at lines 234-256
Recommendation: Add remote URL conflict validation to prevent duplicate remotes

3. Resource Pattern Permission Logic
Location: internal/api/handlers/v0/edit.go:68-70

if !jwtManager.HasPermission(editRequest.Server.Name, auth.PermissionActionEdit, claims.Permissions) {

Issue: Permission check happens after JSON parsing but the server name could be changed in the edit
Recommendation: Consider checking permissions against the existing server name, not the new name

4. Error Response Consistency
Location: internal/api/handlers/v0/edit.go:98
Issue: Service layer errors return as 400 Bad Request, but some might be more appropriate as 500 Internal Server Error
Recommendation: More granular error handling based on error types

📋 Minor Observations

Schema Documentation

• Schema correctly updated with deleted status and clear description at server.schema.json
• Good documentation of the intended use case for deleted status

Code Style

• Consistent with existing codebase patterns
• Good separation of concerns between layers
• Proper use of context timeouts

🔒 Security Assessment

Authorization: ✅ Well implemented

• JWT validation with proper error handling
• Resource-based permissions with pattern matching
• Bearer token format validation

Input Validation: ✅ Comprehensive

• JSON structure validation
• Extension field validation
• Server detail validation using existing validators

Anti-abuse: ✅ Strong protections

• Cannot undelete servers once marked as deleted
• Proper permission checking for edit operations
• Reuses existing validation logic to prevent bypasses