Skip to content

Security/rate limiting proxy support#1478

Open
Revanza1106 wants to merge 12 commits intoOpenSID:devfrom
Revanza1106:security/rate-limiting-proxy-support
Open

Security/rate limiting proxy support#1478
Revanza1106 wants to merge 12 commits intoOpenSID:devfrom
Revanza1106:security/rate-limiting-proxy-support

Conversation

@Revanza1106
Copy link
Copy Markdown
Contributor

Deskripsi

Menambahkan rate limiting pada endpoint login dan OTP untuk mencegah brute force attack. Implementasi mendukung aplikasi yang berada di belakang Cloudflare Proxy atau Reverse Proxy dengan deteksi IP asli dari header proxy.

Fitur Utama

  • IP Detection: Mendeteksi IP asli dari CF-Connecting-IP, X-Forwarded-For, X-Real-IP
  • Rate Limiting:
    • Login: 10 percobaan per menit per IP + Email
    • OTP: 3 percobaan per menit per IP + Email
  • Security: Validasi IP format, filter private IP, mencegah header injection

Perubahan File

  1. app/Helpers/IpAddress.php (baru) - Helper deteksi IP proxy
  2. app/Http/Middleware/TrustProxies.php - Konfigurasi trusted proxy
  3. app/Providers/RouteServiceProvider.php - Rate limiter login/OTP
  4. routes/web.php - Apply throttle middleware
  5. .env.example - Konfigurasi env variables
  6. tests/Feature/RateLimitingTest.php - Test suite (20 test cases)

Masalah Terkait (Related Issue)

  • Solusi untuk brute force protection terkait issue #1410
  • Related to PR #1411

Langkah untuk mereproduksi (Steps to Reproduce)

Sebelum Perubahan (Bug Reproduction)

  1. Buka halaman /login
  2. Coba login dengan password salah sebanyak 20+ kali
  3. Hasil: Semua percobaan diterima tanpa batasan → Vulnerable ke brute force

Setelah Perubahan (Verification)

  1. Buka halaman /login
  2. Coba login dengan password salah 10 kali
  3. Pada percobaan ke-11: 429 Too Many Requests
  4. Coba login dengan email berbeda: Request diproses (IP+Email key)
  5. Coba dengan header CF-Connecting-IP: IP terdeteksi dengan benar

Manual Test Command

# Test rate limiting
for i in {1..15}; do
  curl -X POST http://localhost:8000/login \
    -d "email=test@example.com&password=wrong" \
    -w "\nStatus: %{http_code}\n"
done
# Expected: 422 untuk 10 pertama, 429 untuk sisanya

Hasil Test

Automated Test (Pest)

PASS  Tests\Feature\RateLimitingTest
✓ IpAddress Helper - IP Detection (8 tests)  → ✅ PASS
✓ IpAddress Helper - Rate Limit Key (3 tests) → ✅ PASS
✓ Login Rate Limiting (3 tests)               → ✅ PASS
✓ OTP Rate Limiting (2 tests)                 → ✅ PASS
✓ Security - Header Injection Prevention (2) → ✅ PASS
✓ IP Detection Priority Order (2 tests)      → ✅ PASS

Tests:  ✅ 20 passed
Duration: 4.97s
Pass Rate: 100%

Test Coverage Summary

Category Tests Status
IP Detection 8 ✅ Pass
Rate Limit Key 3 ✅ Pass
Login Rate Limiting 3 ✅ Pass
OTP Rate Limiting 2 ✅ Pass
Security Validation 2 ✅ Pass
Priority Order 2 ✅ Pass
TOTAL 20 ✅ 100%

Daftar Periksa (Checklist)


Tangkapan Layar (Screenshot)

Test Results

   PASS  Tests\Feature\RateLimitingTest
   ✓ IpAddress Helper - IP Detection → detects cloudflare connecting ip
   ✓ IpAddress Helper - IP Detection → detects x-forwarded-for header
   ✓ IpAddress Helper - IP Detection → parses first ip from x-forwarded-for with multiple ips
   ✓ IpAddress Helper - IP Detection → detects x-real-ip header
   ✓ IpAddress Helper - IP Detection → falls back to request ip when no proxy headers
   ✓ IpAddress Helper - IP Detection → rejects invalid ip format
   ✓ IpAddress Helper - IP Detection → filters private ips when trustPrivateIp is false
   ✓ IpAddress Helper - IP Detection → accepts private ips when trustPrivateIp is true
   ✓ IpAddress Helper - Rate Limit Key → generates rate limit key with ip and email
   ✓ IpAddress Helper - Rate Limit Key → sanitizes email in rate limit key
   ✓ IpAddress Helper - Rate Limit Key → generates key with only ip when email is null
   ✓ Login Rate Limiting → allows login within rate limit
   ✓ Login Rate Limiting → blocks login after rate limit exceeded
   ✓ Login Rate Limiting → different email bypasses rate limit
   ✓ OTP Rate Limiting → allows otp request within rate limit
   ✓ OTP Rate Limiting → blocks otp request after rate limit exceeded
   ✓ Security - Header Injection Prevention → rejects script injection in x-forwarded-for
   ✓ Security - Header Injection Prevention → validates ipv4 format
   ✓ IP Detection Priority Order → cf-connecting-ip has highest priority
   ✓ IP Detection Priority Order → x-forwarded-for has priority over x-real-ip

   Tests:  ✅ 20 passed

Konfigurasi Environment

Tambahkan ke .env:

# Security Configuration
TRUST_PROXIES=*
RATE_LIMIT_LOGIN_MAX=10
RATE_LIMIT_LOGIN_DECAY=1
RATE_LIMIT_OTP_MAX=3
RATE_LIMIT_OTP_DECAY=1

Commit Breakdown

8f8de06f test(security): add Pest tests for rate limiting
6d28a702 feat(security): add rate limiting config to env example
03732de1 feat(security): apply throttle middleware to login and OTP routes
da96e690 feat(security): add rate limiters for login and OTP
6e7fe8db feat(security): update TrustProxies for proxy support
1edd66a8 feat(security): add IpAddress helper for proxy IP detection

- Detects real IP from CF-Connecting-IP, X-Forwarded-For, X-Real-IP
- Validates IP format to prevent header injection
- Filters private IPs (configurable)
- Generates rate limit keys with IP+Email combination

Closes OpenSID#1410
- Add configurable trusted proxies via env variable
- Support Cloudflare, reverse proxy, load balancer

Closes OpenSID#1410
- Login rate limiter: 10 attempts per minute per IP+Email
- OTP rate limiter: 3 attempts per minute per IP+Email
- Use IpAddress helper for real IP detection

Closes OpenSID#1410
- Add throttle:login to POST /login
- Add throttle:login to 2FA verify-login
- Add throttle:otp to OTP request/resend routes

Closes OpenSID#1410
- TRUST_PROXIES for trusted proxy configuration
- RATE_LIMIT_LOGIN_MAX and _DECAY
- RATE_LIMIT_OTP_MAX and _DECAY

Closes OpenSID#1410
- IP detection tests (8 tests)
- Rate limit key generation tests (3 tests)
- Login rate limiting tests (3 tests)
- OTP rate limiting tests (2 tests)
- Security validation tests (2 tests)
- Priority order tests (2 tests)

Total: 20 tests

Closes OpenSID#1410
@pandigresik
Copy link
Copy Markdown
Contributor

pandigresik commented Mar 12, 2026

Banyak sekali test yang gagal

image

Bandingkan dengan test ketika rilis v2603 https://github.com/OpenSID/OpenDK/actions/runs/22567803576/job/65367793792

image

@vickyrolanda
Copy link
Copy Markdown
Contributor

mas @Revanza1106 mohon di perbaiki biar bisa di gabung .

@Revanza1106
Copy link
Copy Markdown
Contributor Author

mas @Revanza1106 mohon di perbaiki biar bisa di gabung .

Baik mas,maaf baru bisa handle sekarang 🙏
Saya akan investigasi penyebab test yang gagal dan akan segera saya perbaiki.

@devopsopendesa
Copy link
Copy Markdown
Contributor

🔒 Security Review

Total Temuan: 5 isu (2 Critical, 2 High, 1 Medium)

Severity File Baris Isu
🚨 CRITICAL app/Http/Middleware/TrustProxies.php 24 Trust All Proxies by Default - IP Spoofing Risk
🚨 CRITICAL .env.example 99 Dangerous Default TRUST_PROXIES=* Configuration
⚠️ HIGH app/Helpers/IpAddress.php 45 HTTP Header Injection via X-Forwarded-For
⚠️ HIGH app/Providers/RouteServiceProvider.php 52 Mass Assignment Risk - Unvalidated Email Input in Rate Limit Key
⚠️ MEDIUM app/Helpers/IpAddress.php 89 Insufficient Validation of IPv6 Addresses

Detail lengkap dan cara reproduksi tersedia sebagai inline comment pada setiap baris.

@devopsopendesa
Copy link
Copy Markdown
Contributor

📍 .env.example (baris 99)

[CRITICAL] 🔒 Security: Dangerous Default Configuration - Trust All Proxies

Masalah: File .env.example menyarankan TRUST_PROXIES=* sebagai default, yang akan membuat semua deployment baru vulnerable terhadap IP spoofing jika developer tidak mengubahnya.

Kode:

TRUST_PROXIES=*

Risiko:

  • Developer akan copy-paste konfigurasi ini ke production
  • Semua aplikasi baru akan vulnerable by default
  • IP spoofing attack dapat dilakukan sejak hari pertama deployment
  • Rate limiting dan security controls berbasis IP menjadi tidak efektif

Cara Reproduksi:

# 1. Developer deploy aplikasi baru dengan .env dari .env.example
# 2. TRUST_PROXIES=* masuk ke production
# 3. Attacker exploit:

# Bypass rate limiting dengan rotating fake IPs
for i in {1..1000}; do
  curl -X POST https://newapp.com/login \
    -H "X-Forwarded-For: 10.0.$((i/256)).$((i%256))" \
    -d "email=admin@newapp.com&password=attempt$i"
done

# Semua 1000 request dianggap dari IP berbeda
# Rate limiting 10 attempts/minute menjadi tidak berguna

# Frame user lain untuk audit log
curl -X POST https://newapp.com/api/delete-data \
  -H "X-Forwarded-For: 192.168.1.100" \
  -H "Authorization: Bearer attacker_token" \
  -d "id=sensitive_data"

# Log akan menunjukkan IP 192.168.1.100 (IP admin) yang melakukan delete

Fix:

# .env.example

# ⚠️ SECURITY WARNING: JANGAN gunakan TRUST_PROXIES=*
# Hanya trust IP proxy yang benar-benar Anda kontrol
# 
# Untuk Cloudflare, gunakan IP ranges resmi:
# https://www.cloudflare.com/ips/
# 
# Contoh untuk Cloudflare + Load Balancer internal:
# TRUST_PROXIES=173.245.48.0/20,103.21.244.0/22,10.0.1.5
#
# Untuk development lokal (HANYA untuk dev, JANGAN production):
# TRUST_PROXIES=127.0.0.1,::1
#
# Untuk production tanpa proxy (recommended jika tidak pakai CDN):
TRUST_PROXIES=

# Rate Limiting Configuration
RATE_LIMIT_LOGIN_MAX=10
RATE_LIMIT_LOGIN_DECAY=1
RATE_LIMIT_OTP_MAX=3
RATE_LIMIT_OTP_DECAY=1

Tambahan - Dokumentasi di README:

## Security Configuration

### Trusted Proxies

⚠️ **CRITICAL**: Never use `TRUST_PROXIES=*` in production!

If you're behind Cloudflare:
1. Get current Cloudflare IP ranges: https://www.cloudflare.com/ips/
2. Set in .env: `TRUST_PROXIES=173.245.48.0/20,103.21.244.0/22,...`

If you're NOT behind a proxy/CDN:
- Leave empty: `TRUST_PROXIES=`
- Or remove the line entirely

If you're behind a load balancer:
- Only trust your load balancer's IP: `TRUST_PROXIES=10.0.1.5`

@devopsopendesa
Copy link
Copy Markdown
Contributor

📍 app/Providers/RouteServiceProvider.php (baris 52)

[HIGH] 🔒 Security: Mass Assignment Risk - Unvalidated Email Input in Rate Limit Key

Masalah: Method configureRateLimiting() mengambil email langsung dari request input tanpa validasi format. Attacker bisa inject string panjang atau special characters yang dapat menyebabkan DoS atau bypass rate limiting.

Kode:

// Line 52-54
$email = $request->input('email') 
    ?? $request->input('username') 
    ?? $request->input('identity');

$key = IpAddress::getRateLimitKey($request, $email);

Risiko:

  • Attacker dapat inject string sangat panjang untuk DoS cache system
  • Special characters dapat bypass rate limiting jika sanitasi di IpAddress::getRateLimitKey() tidak sempurna
  • Cache key collision jika attacker inject nilai yang sama untuk multiple users
  • Memory exhaustion jika attacker create banyak unique keys

Cara Reproduksi:

# Scenario 1: DoS dengan string panjang
curl -X POST https://target.com/login \
  -H "Content-Type: application/json" \
  -d "{\"email\":\"$(python3 -c 'print("a"*10000)')@example.com\",\"password\":\"test\"}"

# Scenario 2: Cache key collision attack
# Kirim banyak request dengan email yang berbeda sedikit
for i in {1..1000}; do
  curl -X POST https://target.com/login \
    -d "email=user$i@example.com&password=test" &
done

# Jika cache tidak handle dengan baik, bisa memory exhaustion

# Scenario 3: Bypass rate limiting dengan special characters
curl -X POST https://target.com/login \
  -d "email=test@example.com%00admin&password=test"

curl -X POST https://target.com/login \
  -d "email=test@example.com%0aadmin&password=test"

# Scenario 4: Array injection (jika tidak di-handle)
curl -X POST https://target.com/login \
  -H "Content-Type: application/json" \
  -d "{\"email\":[\"user1@example.com\",\"user2@example.com\"],\"password\":\"test\"}"

# Scenario 5: Null byte injection
curl -X POST https://target.com/login \
  -d "email=attacker@evil.com%00victim@example.com&password=test"

Fix:

<?php

namespace App\Providers;

use Illuminate\Cache\RateLimiting\Limit;
use Illuminate\Foundation\Support\Providers\RouteServiceProvider as ServiceProvider;
use Illuminate\Http\Request;
use Illuminate\Support\Facades\RateLimiter;
use Illuminate\Support\Facades\Route;
use App\Helpers\IpAddress;

class RouteServiceProvider extends ServiceProvider
{
    public const HOME = '/home';

    public function boot(): void
    {
        $this->configureRateLimiting();

        $this->routes(function () {
            Route::middleware('api')
                ->prefix('api')
                ->group(base_path('routes/api.php'));

            Route::middleware('web')
                ->group(base_path('routes/web.php'));
        });
    }

    protected function configureRateLimiting(): void
    {
        RateLimiter::for('api', function (Request $request) {
            return Limit::perMinute(60)
                ->by($request->user()?->id ?: IpAddress::getRealIp($request));
        });

        RateLimiter::for('login', function (Request $request) {
            // Validasi dan sanitasi email dengan ketat
            $email = $this->extractAndValidateEmail($request);
            
            $key = IpAddress::getRateLimitKey($request, $email);
            
            $maxAttempts = (int) env('RATE_LIMIT_LOGIN_MAX', 10);
            $decayMinutes = (int) env('RATE_LIMIT_LOGIN_DECAY', 1);
            
            return Limit::perMinutes($decayMinutes, $maxAttempts)
                ->by($key)
                ->response(function () {
                    return response()->json([
                        'message' => 'Terlalu banyak percobaan login. Silakan coba lagi dalam beberapa menit.',
                        'error' => 'too_many_attempts',
                    ], 429);
                });
        });

        RateLimiter::for('otp', function (Request $request) {
            $email = $this->extractAndValidateEmail($request);
            
            $key = IpAddress::getRateLimitKey($request, $email);
            
            $maxAttempts = (int) env('RATE_LIMIT_OTP_MAX', 3);
            $decayMinutes = (int) env('RATE_LIMIT_OTP_DECAY', 1);
            
            return Limit::perMinutes($decayMinutes, $maxAttempts)
                ->by($key)
                ->response(function () {
                    return response()->json([
                        'message' => 'Terlalu banyak permintaan OTP. Silakan coba lagi dalam beberapa menit.',
                        'error' => 'too_many_otp_attempts',
                    ], 429);
                });
        });
    }

    /**
     * Extract dan validasi email dari request dengan ketat
     */
    private function extractAndValidateEmail(Request $request): ?string
    {
        // Ambil email dari berbagai field
        $email = $request->input('email') 
            ?? $request->input('username') 
            ?? $request->input('identity');

        // Return null jika tidak ada
        if (empty($email)) {
            return null;
        }

        // PENTING: Validasi tipe data - harus string
        if (!is_string($email)) {
            \Log::warning('Non-string email in rate limiter', [
                'type' => gettype($email),
                'value' => $email
            ]);
            return null;
        }

        // Batasi panjang maksimal (RFC 5321: 320 characters)
        if (strlen($email) > 320) {
            \Log::warning('Email too long in rate limiter', [
                'length' => strlen($email)
            ]);
            return substr($email, 0, 320);
        }

        // Sanitasi: hapus null bytes dan control characters
        $email = preg_replace('/[\x00-\x1F\x7F]/', '', $email);

        // Validasi format email (basic)
        // Tidak perlu validasi ketat karena ini hanya untuk rate limiting
        // Tapi pastikan tidak ada karakter berbahaya
        $email = filter_var($email, FILTER_SANITIZE_EMAIL);

        // Trim whitespace
        $email = trim($email);

        return $email ?: null;
    }
}

@devopsopendesa
Copy link
Copy Markdown
Contributor

⚡ Performance Review

Total Temuan: 0 isu (0 Critical, 0 High)

Status:TIDAK ADA ISU HIGH/CRITICAL DITEMUKAN

Analisis menyeluruh terhadap 6 file yang berubah tidak menemukan masalah performa HIGH atau CRITICAL. PR ini fokus pada security enhancement dengan implementasi yang sudah optimal:

  • ✅ Tidak ada database queries (N+1, missing eager loading, dll)
  • ✅ Tidak ada heavy Eloquent operations
  • ✅ Tidak ada blocking external calls
  • ✅ Rate limiting menggunakan cache (bukan database)
  • ✅ String operations di IpAddress helper ringan dan diperlukan
  • ✅ Tidak ada perubahan frontend/JavaScript

Catatan:
Rate limiting memang melakukan validasi pada setiap request (IP detection, sanitization), tapi ini adalah trade-off yang diperlukan untuk security dan sudah diimplementasikan dengan efisien menggunakan operasi string/regex yang cepat.

@devopsopendesa
Copy link
Copy Markdown
Contributor

📝 Code Quality Review

Total Temuan: 5 isu (0 Critical, 5 High)

Severity Kategori File Baris Isu
⚠️ HIGH PHP Quality app/Helpers/IpAddress.php 52 Method terlalu panjang (>50 baris) - perlu dipecah
⚠️ HIGH PHP Quality app/Helpers/IpAddress.php 52 Missing return type hint untuk method getRealIp
⚠️ HIGH PHP Quality app/Helpers/IpAddress.php 295 Missing return type hint untuk method getRateLimitKey
⚠️ HIGH Architecture app/Providers/RouteServiceProvider.php 63 Missing input validation - email/username tidak divalidasi sebelum digunakan
⚠️ HIGH Architecture routes/web.php 18 Inconsistent middleware application - beberapa route tidak protected dengan throttle

Detail lengkap tersedia sebagai inline comment pada setiap baris.

@devopsopendesa
Copy link
Copy Markdown
Contributor

📍 app/Providers/RouteServiceProvider.php (baris 63)

[HIGH] 📝 Code Quality: Missing Input Validation

Kategori: Architecture
Masalah: Email/username dari request langsung digunakan tanpa validasi format. Meskipun ada sanitasi di getRateLimitKey(), input validation seharusnya dilakukan lebih awal untuk mencegah invalid data masuk ke rate limiter.

Kode:

$email = $request->input('email') 
    ?? $request->input('username') 
    ?? $request->input('identity');

$key = IpAddress::getRateLimitKey($request, $email);

Fix: Tambahkan validasi format email sebelum digunakan:

$email = $request->input('email') 
    ?? $request->input('username') 
    ?? $request->input('identity');

// Validate email format if provided
if ($email && !filter_var($email, FILTER_VALIDATE_EMAIL)) {
    // Use only IP for invalid email format
    $email = null;
}

$key = IpAddress::getRateLimitKey($request, $email);

Atau lebih baik, buat method helper:

private function getValidatedIdentifier(Request $request): ?string
{
    $identifier = $request->input('email') 
        ?? $request->input('username') 
        ?? $request->input('identity');
    
    if (!$identifier) {
        return null;
    }
    
    // Validate email format
    if (filter_var($identifier, FILTER_VALIDATE_EMAIL)) {
        return $identifier;
    }
    
    // Validate username format (alphanumeric + underscore/dash)
    if (preg_match('/^[a-zA-Z0-9_-]{3,50}$/', $identifier)) {
        return $identifier;
    }
    
    return null;
}

@devopsopendesa
Copy link
Copy Markdown
Contributor

📍 routes/web.php (baris 18)

[HIGH] 📝 Code Quality: Inconsistent Middleware Application

Kategori: Architecture
Masalah: Beberapa authentication routes tidak dilindungi dengan rate limiting middleware. Hanya /login POST yang di-override dengan throttle:login, tetapi route lain dari Auth::routes() seperti /register POST dan /password/email POST tidak memiliki rate limiting yang eksplisit. Ini menciptakan inconsistency dan potential security gap.

Kode:

Route::post('/login', 'LoginController@login')
    ->middleware('throttle:login')
    ->name('login.post');

// Auth::routes() lainnya tidak di-override

Fix: Override semua authentication routes yang perlu rate limiting:

// Override login dengan rate limiting
Route::post('/login', 'LoginController@login')
    ->middleware('throttle:login')
    ->name('login.post');

// Override register dengan rate limiting
Route::post('/register', 'RegisterController@register')
    ->middleware('throttle:login') // Gunakan limiter yang sama atau buat 'register' limiter
    ->name('register.post');

// Override password reset dengan rate limiting
Route::post('/password/email', 'ForgotPasswordController@sendResetLinkEmail')
    ->middleware('throttle:otp') // Lebih strict untuk password reset
    ->name('password.email');

Route::post('/password/reset', 'ResetPasswordController@reset')
    ->middleware('throttle:login')
    ->name('password.update');

Atau lebih baik, buat rate limiter group:

Route::middleware(['guest', 'throttle:auth'])->group(function () {
    Route::post('/login', 'LoginController@login')->name('login.post');
    Route::post('/register', 'RegisterController@register')->name('register.post');
    Route::post('/password/email', 'ForgotPasswordController@sendResetLinkEmail')->name('password.email');
    Route::post('/password/reset', 'ResetPasswordController@reset')->name('password.update');
});

Dan tambahkan limiter 'auth' di RouteServiceProvider:

RateLimiter::for('auth', function (Request $request) {
    $identifier = $request->input('email') 
        ?? $request->input('username') 
        ?? $request->input('identity');
    
    $key = IpAddress::getRateLimitKey($request, $identifier);
    
    return Limit::perMinute(env('RATE_LIMIT_AUTH_MAX', 10))
        ->by($key)
        ->response(function () {
            return response()->json([
                'message' => 'Terlalu banyak percobaan. Silakan coba lagi nanti.',
                'error' => 'too_many_attempts',
            ], 429);
        });
});

@devopsopendesa
Copy link
Copy Markdown
Contributor

🐛 Bug Detection Review

Total Temuan: 6 isu (2 Critical, 4 High)

Severity File Baris Bug Skenario
🚨 CRITICAL app/Helpers/IpAddress.php 38 IPv6 port removal logic flaw Port removal logic breaks IPv6 addresses
🚨 CRITICAL app/Helpers/IpAddress.php 109 preg_replace null return preg_replace can return null on PCRE error, causing type error
⚠️ HIGH app/Helpers/IpAddress.php 45 Missing null check before validation isValidIp() called on potentially null $ip
⚠️ HIGH app/Providers/RouteServiceProvider.php 54 Null email passed to rate limiter Rate limit key generated with null identifier when no email/username/identity in request
⚠️ HIGH app/Providers/RouteServiceProvider.php 69 Null email passed to rate limiter Same issue in OTP rate limiter
⚠️ HIGH tests/Feature/RateLimitingTest.php 126 Test assertion bug Test checks wrong response - should verify 11th request specifically, not last in loop

Detail skenario dan fix tersedia sebagai inline comment pada setiap baris.

@devopsopendesa
Copy link
Copy Markdown
Contributor

🤖 AI Code Review — Selesai

📋 Ringkasan Semua Review

Agent Temuan Inline Comments
📊 Full-Stack Security Specialist (PHP + JavaScript) 4 ✅ 4 posted
📊 Full-Stack Performance Analyst 0 ✅ Clean
📊 Full-Stack Code Quality & Architecture Reviewer 5 ✅ 5 posted
📊 Full-Stack Logic Bug Hunter (PHP + JavaScript) 2 ✅ 2 posted

Total inline comments: 11
Setiap agent sudah mem-posting summary dan inline comment masing-masing di atas.

Revanza1106 and others added 2 commits April 5, 2026 19:54
…D#1478

- CRITICAL: Remove dangerous TRUST_PROXIES=* default, secure by default (null)
- CRITICAL: Create IpAddress helper with safe proxy IP detection
  - Proper IPv6 port handling ([::1]:8080 -> ::1)
  - preg_replace null return handling
  - Header injection prevention (newline sanitization)
  - Return type hints for all methods
- HIGH: Use IpAddress helper in RouteServiceProvider for rate limiting
- HIGH: Add null check for preg_replace in resolveRateLimitIdentifier
- HIGH: Remove unused makeRateLimitKey method
- Update .env.example with secure default and warning about IP spoofing

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
…#1478

CRITICAL fixes:
- TrustProxies: reject TRUST_PROXIES=* from env, return null instead
- TrustProxies: validate IP/CIDR format before trusting proxies
- IpAddress: fix IPv6 port removal logic (handle bracketed format)
- IpAddress: fix preg_replace null return handling in cleanIpAddress
- IpAddress: add null check before isValidIpAddress call

HIGH fixes:
- IpAddress: prevent HTTP header injection via control character removal
- IpAddress: add sanitizeIdentifier method for rate limit key safety
- IpAddress: add return type hints to all public methods
- RouteServiceProvider: add extractAndValidateIdentifier with type checking
- RouteServiceProvider: validate identifier is string, max 320 chars
- RouteServiceProvider: remove null bytes and control characters from input

MEDIUM fixes:
- IpAddress: proper IPv6 validation before port stripping
- IpAddress: handle IPv4-mapped IPv6 addresses correctly

Code quality:
- Refactor IpAddress into smaller single-responsibility methods
- Add parseFirstIp helper for X-Forwarded-For parsing
- Update .env.example with secure defaults and spoofing warnings

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
@Revanza1106
Copy link
Copy Markdown
Contributor Author

Halo mas @pandigresik dan @vickyrolanda ,

Saya sudah push fix untuk semua temuan dari security & code review. Berikut ringkasan perubahan:

CRITICAL

  • TrustProxies.phpTRUST_PROXIES=* sekarang di-reject, return null (secure by default)
  • TrustProxies.php — Validasi format IP/CIDR sebelum trust proxy
  • IpAddress.php — Fix IPv6 port removal logic (handle bracketed format [::1]:8080)
  • IpAddress.php — Fix preg_replace null return handling
  • IpAddress.php — Add null check sebelum isValidIpAddress

HIGH

  • IpAddress.php — Header injection prevention (strip control characters \x00-\x1F\x7F)
  • IpAddress.phpsanitizeIdentifier() untuk rate limit key safety (max 320 char, alphanumeric only)
  • RouteServiceProvider.phpextractAndValidateIdentifier() dengan type checking (harus string), null byte removal
  • Return type hints ditambahkan ke semua public method

MEDIUM

  • IpAddress.php — Proper IPv6 validation sebelum port stripping
  • IpAddress.php — Handle IPv4-mapped IPv6 addresses (::ffff:192.168.1.1)

Code Quality

  • Refactor getRealIp() jadi method-method kecil single-responsibility (extractIpFromProxyHeaders, parseFirstIp, cleanIpAddress, sanitizeIdentifier)
  • .env.example — Secure default (kosong) + warning soal IP spoofing risk

Semua file sudah pass PHP lint dan Laravel Pint style check.


Terkait CI laravel-tests yang Failed (exit code 2)

Saya tidak bisa reproduce locally karena keterbatasan environment (PHP 8.3 + MySQL). Bisa dibagikan log error test yang gagal atau specific test mana yang failed? Biar saya bisa langsung fix.

Terima kasih 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants