# Arch Security Review

> [Architecture] Use when reviewing code for security vulnerabilities, implementing authorization, or ensuring data protection.

- Skill: `majiayu000/arch-security-review` (Agent Skill, multi-file: 2 files)
- Install (CLI): `npx skillmds add majiayu000/arch-security-review`
- Raw SKILL.md: https://api.skillmd.com/api/skills/majiayu000/arch-security-review/raw
- Safety review: pending
- Works with: Claude Code, Claude.ai, OpenAI Codex
- Category: Security
- Author: majiayu000 (https://skillmd.com/u/majiayu000)
- Updated: 2026-09-09
- Page: https://skillmd.com/skills/majiayu000/arch-security-review

---


> **[IMPORTANT]** Use `TaskCreate` to break ALL work into small tasks BEFORE starting — including tasks for each file read. This prevents context loss from long files. For simple tasks, AI MUST ask user whether to skip.

**Prerequisites:** **MUST READ** `.claude/skills/shared/evidence-based-reasoning-protocol.md` before executing.

- `docs/project-reference/domain-entities-reference.md` — Domain entity catalog, relationships, cross-service sync (read when task involves business entities/models)

> **Critical Purpose:** Ensure quality — no flaws, no bugs, no missing updates, no stale content. Verify both code AND documentation.

## Quick Summary

**Goal:** Review code for security vulnerabilities against OWASP Top 10 and enforce authorization, data protection, and secure coding patterns.

**Workflow:**

1. **Pre-Flight** — Identify security-sensitive areas, check OWASP relevance, review existing patterns
2. **OWASP Audit** — Evaluate code against all 10 categories (access control, injection, auth, etc.)
3. **Project Checks** — Verify authorization attributes, entity access expressions, input validation
4. **Report** — Document findings with severity, vulnerable vs secure code examples

**Key Rules:**

- Always check both backend and frontend attack surfaces
- Use project authorization attributes and entity-level access expressions, never rely on UI-only guards (see docs/project-reference/backend-patterns-reference.md)
- Validate all external data with project validation API, never trust client input (see docs/project-reference/backend-patterns-reference.md)

**Be skeptical. Apply critical thinking, sequential thinking. Every claim needs traced proof, confidence percentages (Idea should be more than 80%).**

# Security Review Workflow

## When to Use This Skill

- Security audit of code changes
- Implementing authentication/authorization
- Data protection review
- Vulnerability assessment

## Pre-Flight Checklist

- [ ] Identify security-sensitive areas
- [ ] Review OWASP Top 10 relevance
- [ ] Check for existing security patterns
- [ ] Plan remediation approach

## OWASP Top 10 Checklist

### 1. Broken Access Control

```csharp
// :x: VULNERABLE - No authorization check
[HttpGet("{id}")]
public async Task<Employee> Get(string id)
    => await repo.GetByIdAsync(id);

// :white_check_mark: SECURE - Authorization enforced
[HttpGet("{id}")]
[Authorize(Roles.Manager, Roles.Admin)] // project authorization attribute (see docs/project-reference/backend-patterns-reference.md)
public async Task<Employee> Get(string id)
{
    var employee = await repo.GetByIdAsync(id);

    // Verify access to this specific resource
    if (employee.CompanyId != RequestContext.CurrentCompanyId())
        throw new UnauthorizedAccessException();

    return employee;
}
```

### 2. Cryptographic Failures

```csharp
// :x: VULNERABLE - Storing plain text secrets
var apiKey = config["ApiKey"];
await SaveToDatabase(apiKey);

// :white_check_mark: SECURE - Encrypt sensitive data
var encryptedKey = encryptionService.Encrypt(apiKey);
await SaveToDatabase(encryptedKey);

// Use secure configuration
var apiKey = config.GetValue<string>("ApiKey");  // From Azure Key Vault
```

### 3. Injection

```csharp
// :x: VULNERABLE - SQL Injection
var sql = $"SELECT * FROM Users WHERE Name = '{name}'";
await context.Database.ExecuteSqlRawAsync(sql);

// :white_check_mark: SECURE - Parameterized query
await context.Users.Where(u => u.Name == name).ToListAsync();

// Or if raw SQL needed:
await context.Database.ExecuteSqlRawAsync(
    "SELECT * FROM Users WHERE Name = @p0", name);
```

### 4. Insecure Design

```csharp
// :x: VULNERABLE - No rate limiting
[HttpPost("login")]
public async Task<IActionResult> Login(LoginRequest request)
    => await authService.Login(request);

// :white_check_mark: SECURE - Rate limiting applied
[HttpPost("login")]
[RateLimit(MaxRequests = 5, WindowSeconds = 60)]
public async Task<IActionResult> Login(LoginRequest request)
    => await authService.Login(request);
```

### 5. Security Misconfiguration

```csharp
// :x: VULNERABLE - Detailed errors in production
app.UseDeveloperExceptionPage();  // Exposes stack traces

// :white_check_mark: SECURE - Generic errors in production
if (env.IsDevelopment())
    app.UseDeveloperExceptionPage();
else
    app.UseExceptionHandler("/Error");
```

### 6. Vulnerable Components

```bash
# Check for vulnerable packages
dotnet list package --vulnerable

# Update vulnerable packages
dotnet outdated
```

### 7. Authentication Failures

```csharp
// :x: VULNERABLE - Weak password policy
if (password.Length >= 4) { }

// :white_check_mark: SECURE - Strong password policy
public class PasswordPolicy
{
    public bool Validate(string password)
    {
        return password.Length >= 12
            && password.Any(char.IsUpper)
            && password.Any(char.IsLower)
            && password.Any(char.IsDigit)
            && password.Any(c => !char.IsLetterOrDigit(c));
    }
}
```

### 8. Data Integrity Failures

```csharp
// :x: VULNERABLE - No validation of external data
var userData = await externalApi.GetUserAsync(id);
await SaveToDatabase(userData);

// :white_check_mark: SECURE - Validate external data
var userData = await externalApi.GetUserAsync(id);
var validation = userData.Validate();
if (!validation.IsValid)
    throw new ValidationException(validation.Errors);
await SaveToDatabase(userData);
```

### 9. Logging Failures

```csharp
// :x: VULNERABLE - Logging sensitive data
Logger.LogInformation("User login: {Email} {Password}", email, password);

// :white_check_mark: SECURE - Redact sensitive data
Logger.LogInformation("User login: {Email}", email);
// Never log passwords, tokens, or PII
```

### 10. SSRF (Server-Side Request Forgery)

```csharp
// :x: VULNERABLE - User-controlled URL
var url = request.WebhookUrl;
await httpClient.GetAsync(url);  // Could access internal services

// :white_check_mark: SECURE - Validate and restrict URLs
if (!IsAllowedUrl(request.WebhookUrl))
    throw new SecurityException("Invalid webhook URL");

private bool IsAllowedUrl(string url)
{
    var uri = new Uri(url);
    return AllowedDomains.Contains(uri.Host)
        && uri.Scheme == "https";
}
```

## Authorization Patterns

**⚠️ MUST READ:** CLAUDE.md for authorization controller/handler patterns, `RequestContext` usage, and entity-level access filters (see docs/project-reference/backend-patterns-reference.md).

## Data Protection

### Sensitive Data Handling

```csharp
public class SensitiveDataHandler
{
    // Encrypt at rest
    public string EncryptForStorage(string plainText)
        => encryptionService.Encrypt(plainText);

    // Mask for display
    public string MaskEmail(string email)
    {
        var parts = email.Split('@');
        return $"{parts[0][0]}***@{parts[1]}";
    }

    // Never log sensitive data
    public void LogUserAction(User user)
    {
        Logger.LogInformation("User action: {UserId}", user.Id);
        // NOT: Logger.Log("User: {Email} {Phone}", user.Email, user.Phone);
    }
}
```

### File Upload Security

```csharp
public async Task<IActionResult> Upload(IFormFile file)
{
    // Validate file type
    var allowedTypes = new[] { ".pdf", ".docx", ".xlsx" };
    var extension = Path.GetExtension(file.FileName).ToLowerInvariant();
    if (!allowedTypes.Contains(extension))
        return BadRequest("Invalid file type");

    // Validate file size
    if (file.Length > 10 * 1024 * 1024)  // 10MB
        return BadRequest("File too large");

    // Scan for malware (if available)
    if (!await antivirusService.ScanAsync(file))
        return BadRequest("File rejected by security scan");

    // Generate safe filename
    var safeFileName = $"{Guid.NewGuid()}{extension}";

    // Save to isolated storage
    await fileService.SaveAsync(file, safeFileName);

    return Ok();
}
```

## Security Scanning Commands

```bash
# .NET vulnerability scan
dotnet list package --vulnerable

# Outdated packages
dotnet outdated

# Secret scanning
grep -r "password\|secret\|apikey" --include="*.cs" --include="*.json"

# Hardcoded credentials
grep -r "Password=\"" --include="*.cs"
grep -r "connectionString.*password" --include="*.json"
```

## Security Review Checklist

### Authentication

- [ ] Strong password policy enforced
- [ ] Account lockout after failed attempts
- [ ] Secure session management
- [ ] JWT tokens properly validated
- [ ] Refresh token rotation

### Authorization

- [ ] All endpoints require authentication
- [ ] Role-based access control implemented
- [ ] Resource-level permissions checked
- [ ] No privilege escalation possible

### Input Validation

- [ ] All inputs validated
- [ ] SQL injection prevented (parameterized queries)
- [ ] XSS prevented (output encoding)
- [ ] File uploads validated
- [ ] URL validation for redirects

### Data Protection

- [ ] Sensitive data encrypted at rest
- [ ] HTTPS enforced
- [ ] No sensitive data in logs
- [ ] Proper error handling (no stack traces)

### Dependencies

- [ ] No known vulnerable packages
- [ ] Dependencies regularly updated
- [ ] Third-party code reviewed

## Anti-Patterns to AVOID

:x: **Trusting client input**

```csharp
var isAdmin = request.IsAdmin;  // User-supplied!
```

:x: **Exposing internal errors**

```csharp
catch (Exception ex) { return BadRequest(ex.ToString()); }
```

:x: **Hardcoded secrets**

```csharp
var apiKey = "sk_live_xxxxx";
```

:x: **Insufficient logging**

```csharp
// No audit trail for sensitive operations
await DeleteAllUsers();
```

## Verification Checklist

- [ ] OWASP Top 10 reviewed
- [ ] Authentication/authorization verified
- [ ] Input validation complete
- [ ] Sensitive data protected
- [ ] No hardcoded secrets
- [ ] Logging appropriate (no PII)
- [ ] Dependencies scanned

## Related

- `arch-performance-optimization`
- `arch-cross-service-integration`
- `code-review`

---

**IMPORTANT Task Planning Notes (MUST FOLLOW)**

- Always plan and break work into many small todo tasks
- Always add a final review todo task to verify work quality and identify fixes/enhancements