Secure Code Review

START: Code Review Required
    │
    ├─→ Does code handle user input?
    │   ├─→ YES → Apply Input Validation Checklist
    │   └─→ NO → Continue
    │
    ├─→ Does code access database?
    │   ├─→ YES → Apply SQL Injection Prevention
    │   └─→ NO → Continue
    │
    ├─→ Does code handle authentication/authorization?
    │   ├─→ YES → Apply Authentication Security Checklist
    │   └─→ NO → Continue
    │
    ├─→ Does code process sensitive data?
    │   ├─→ YES → Apply Data Protection Checklist
    │   └─→ NO → Continue
    │
    ├─→ Does code interact with external systems?
    │   ├─→ YES → Apply API Security Checklist
    │   └─→ NO → Continue
    │
    └─→ Run SAST tools (SonarCloud, CodeQL)
        └─→ Document findings and mitigation

@Service
public class DocumentService {
    @Autowired
    private SecurityContext securityContext;
    
    @PreAuthorize("hasRole('USER')")
    public Document getDocument(Long documentId) {
        Document doc = documentRepository.findById(documentId)
            .orElseThrow(() -> new ResourceNotFoundException("Document not found"));
        
        // Verify ownership before returning
        if (!doc.getOwnerId().equals(getCurrentUserId())) {
            throw new AccessDeniedException("Cannot access document owned by another user");
        }
        
        return doc;
    }
}

// BAD: No authorization check, anyone can access any document
@GetMapping("/documents/{id}")
public Document getDocument(@PathVariable Long id) {
    return documentRepository.findById(id).orElse(null);
}

@Service
public class UserService {
    @Autowired
    private PasswordEncoder passwordEncoder; // BCryptPasswordEncoder
    
    public void createUser(String username, String password) {
        String hashedPassword = passwordEncoder.encode(password);
        User user = new User(username, hashedPassword);
        userRepository.save(user);
    }
    
    public boolean verifyPassword(String rawPassword, String hashedPassword) {
        return passwordEncoder.matches(rawPassword, hashedPassword);
    }
}

// BAD: Storing passwords in plain text
public void createUser(String username, String password) {
    User user = new User(username, password); // NEVER DO THIS
    userRepository.save(user);
}

@Repository
public interface PoliticianRepository extends JpaRepository<Politician, Long> {
    // JPA query with named parameter - safe from SQL injection
    @Query("SELECT p FROM Politician p WHERE p.firstName = :firstName AND p.lastName = :lastName")
    List<Politician> findByName(@Param("firstName") String firstName, 
                                @Param("lastName") String lastName);
}

// Using JPA Criteria API - also safe
public List<Politician> searchPoliticians(String searchTerm) {
    CriteriaBuilder cb = entityManager.getCriteriaBuilder();
    CriteriaQuery<Politician> query = cb.createQuery(Politician.class);
    Root<Politician> politician = query.from(Politician.class);
    
    query.where(cb.like(politician.get("firstName"), "%" + searchTerm + "%"));
    return entityManager.createQuery(query).getResultList();
}

// BAD: String concatenation in query
@Query(value = "SELECT * FROM politician WHERE first_name = '" + firstName + "'", 
       nativeQuery = true)
List<Politician> findByName(String firstName); // SQL INJECTION RISK!

// BAD: Direct JDBC with concatenation
public List<Politician> search(String name) {
    String sql = "SELECT * FROM politician WHERE name = '" + name + "'";
    return jdbcTemplate.query(sql, new PoliticianMapper()); // VULNERABLE!
}

// Example: Rate limiting for login attempts
@Service
public class LoginService {
    private static final int MAX_ATTEMPTS = 5;
    private static final Duration LOCKOUT_DURATION = Duration.ofMinutes(15);
    
    private final LoadingCache<String, Integer> loginAttempts = CacheBuilder.newBuilder()
        .expireAfterWrite(LOCKOUT_DURATION)
        .build(new CacheLoader<String, Integer>() {
            public Integer load(String key) {
                return 0;
            }
        });
    
    public void login(String username, String password) {
        // Check if account is locked
        int attempts = loginAttempts.get(username);
        if (attempts >= MAX_ATTEMPTS) {
            throw new AccountLockedException(
                "Account locked due to too many failed attempts. Try again in 15 minutes.");
        }
        
        // Attempt authentication
        boolean authenticated = authenticate(username, password);
        
        if (!authenticated) {
            loginAttempts.put(username, attempts + 1);
            throw new BadCredentialsException("Invalid credentials");
        }
        
        // Success - reset counter
        loginAttempts.invalidate(username);
    }
}

# application-production.yml
spring:
  profiles:
    active: production
  
  # Disable debug endpoints
  boot:
    admin:
      client:
        enabled: false
  
  # Secure session management
  session:
    timeout: 30m
    cookie:
      secure: true
      http-only: true
      same-site: strict
  
  # Hide implementation details
  mvc:
    throw-exception-if-no-handler-found: true
  resources:
    add-mappings: false

# Security headers
server:
  error:
    include-message: never
    include-stacktrace: never
    include-exception: false

# BAD: Debug enabled in production
spring:
  profiles:
    active: development
  
  # Exposes sensitive endpoints
  boot:
    admin:
      client:
        enabled: true
  
  # Leak stack traces
server:
  error:
    include-stacktrace: always
    include-exception: true

# Run OWASP Dependency Check before adding new dependencies
mvn org.owasp:dependency-check-maven:check

# Review the report
ls -la target/dependency-check-report.html

# Fail build on high severity vulnerabilities
mvn verify -Dowasp.failOnCVSS=7

<!-- Use dependencyManagement to control versions -->
<dependencyManagement>
    <dependencies>
        <dependency>
            <groupId>org.springframework</groupId>
            <artifactId>spring-framework-bom</artifactId>
            <version>5.3.34</version>
            <type>pom</type>
            <scope>import</scope>
        </dependency>
    </dependencies>
</dependencyManagement>

<!-- Enable OWASP Dependency Check -->
<plugin>
    <groupId>org.owasp</groupId>
    <artifactId>dependency-check-maven</artifactId>
    <version>9.0.7</version>
    <configuration>
        <failBuildOnCVSS>7</failBuildOnCVSS>
        <suppressionFiles>
            <suppressionFile>dependency-check-suppressions.xml</suppressionFile>
        </suppressionFiles>
    </configuration>
</plugin>

@Configuration
@EnableWebSecurity
public class SecurityConfig {
    
    @Bean
    public SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
        http
            .authorizeHttpRequests(auth -> auth
                .requestMatchers("/public/**").permitAll()
                .requestMatchers("/admin/**").hasRole("ADMIN")
                .anyRequest().authenticated()
            )
            .formLogin(form -> form
                .loginPage("/login")
                .failureHandler(authenticationFailureHandler())
                .successHandler(authenticationSuccessHandler())
            )
            .logout(logout -> logout
                .logoutSuccessUrl("/login?logout")
                .invalidateHttpSession(true)
                .deleteCookies("JSESSIONID")
            )
            .sessionManagement(session -> session
                .sessionCreationPolicy(SessionCreationPolicy.IF_REQUIRED)
                .maximumSessions(1)
                .expiredUrl("/login?expired")
            )
            .csrf(csrf -> csrf.csrfTokenRepository(CookieCsrfTokenRepository.withHttpOnlyFalse()));
        
        return http.build();
    }
    
    @Bean
    public PasswordEncoder passwordEncoder() {
        // BCrypt with strength 12
        return new BCryptPasswordEncoder(12);
    }
}

// Use safe deserialization with explicit type validation
@Bean
public ObjectMapper objectMapper() {
    ObjectMapper mapper = new ObjectMapper();
    
    // Do NOT use activateDefaultTyping for untrusted input
    // If polymorphism is required, use @JsonTypeInfo on specific DTOs
    // with a strict allowlist-based PolymorphicTypeValidator
    
    // Adjust serialization behavior if needed
    mapper.disable(SerializationFeature.FAIL_ON_EMPTY_BEANS);
    // Keep FAIL_ON_UNKNOWN_PROPERTIES enabled by default to catch unexpected/malicious fields
    
    return mapper;
}

@Component
public class SecurityAuditLogger {
    private static final Logger auditLog = LoggerFactory.getLogger("SECURITY_AUDIT");
    
    public void logAuthenticationSuccess(String username, String ipAddress) {
        auditLog.info("Authentication successful - User: {}, IP: {}", 
            sanitizeForLog(username), 
            sanitizeForLog(ipAddress));
    }
    
    public void logAuthenticationFailure(String username, String ipAddress, String reason) {
        auditLog.warn("Authentication failed - User: {}, IP: {}, Reason: {}", 
            sanitizeForLog(username), 
            sanitizeForLog(ipAddress),
            reason);
    }
    
    public void logAccessDenied(String username, String resource) {
        auditLog.warn("Access denied - User: {}, Resource: {}", 
            sanitizeForLog(username), 
            sanitizeForLog(resource));
    }
    
    // Prevent log injection
    private String sanitizeForLog(String input) {
        if (input == null) return "null";
        return input.replaceAll("[\n\r\t]", "_");
    }
}

@Service
public class ExternalDataService {
    private static final Set<String> ALLOWED_HOSTS = Set.of(
        "api.riksdagen.se",
        "api.worldbank.org",
        "data.val.se"
    );
    
    public String fetchExternalData(String url) throws IOException {
        URL parsedUrl;
        try {
            parsedUrl = new URL(url);
        } catch (MalformedURLException e) {
            throw new IllegalArgumentException("Invalid URL", e);
        }
        
        // Validate protocol
        if (!parsedUrl.getProtocol().equals("https")) {
            throw new IllegalArgumentException("Only HTTPS URLs allowed");
        }
        
        // Validate host against allowlist
        if (!ALLOWED_HOSTS.contains(parsedUrl.getHost())) {
            throw new IllegalArgumentException("Host not in allowlist: " + parsedUrl.getHost());
        }
        
        // Fetch data with timeout
        HttpURLConnection conn = (HttpURLConnection) parsedUrl.openConnection();
        conn.setConnectTimeout(5000);
        conn.setReadTimeout(5000);
        
        return IOUtils.toString(conn.getInputStream(), StandardCharsets.UTF_8);
    }
}

name: "CodeQL Security Analysis"
on:
  push:
    branches: [ main, develop ]
  pull_request:
    branches: [ main ]
  schedule:
    - cron: '0 0 * * 0' # Weekly scan

jobs:
  analyze:
    name: Analyze
    runs-on: ubuntu-latest
    permissions:
      actions: read
      contents: read
      security-events: write
    
    steps:
    - name: Checkout repository
      uses: actions/checkout@v4
    
    - name: Initialize CodeQL
      uses: github/codeql-action/init@v3
      with:
        languages: java
        queries: security-and-quality
    
    - name: Build
      run: mvn clean compile -DskipTests
    
    - name: Perform CodeQL Analysis
      uses: github/codeql-action/analyze@v3