The most useful code review best practices for Java center on the bug classes a compiler and a passing test suite will never catch: injection, unsafe deserialization, broken access control, and hardcoded secrets. Style nits are easy to automate away. What actually protects a Java service is a reviewer who reads a diff with an attacker's mindset and knows where the language and its ecosystem set traps.
This guide is the checklist I use when reviewing Java pull requests, ordered roughly by how often each issue causes real incidents.
Automate the boring half first
Before a human looks at anything, the pipeline should have already run. If reviewers spend attention on formatting, naming, and import order, they have less left for logic and security. Wire up a formatter (google-java-format or Spotless), Checkstyle or PMD for conventions, and SpotBugs with the find-sec-bugs plugin for a first security pass. Add a dependency scanner so a reviewer is never the first to learn a transitive library shipped a known CVE.
The rule of thumb: if a tool can decide it deterministically, a tool should. Humans review intent, trust boundaries, and correctness.
Read every place untrusted input enters
Most Java vulnerabilities start where external data crosses into your code. In a review, trace each request parameter, header, uploaded file, message-queue payload, and database value to where it gets used. The dangerous sinks:
- String-concatenated SQL. Any query built with
+instead of aPreparedStatementparameter is an injection finding, full stop. Runtime.execorProcessBuilderwith input-derived arguments.- Reflection driven by user-controlled class names.
- File paths assembled from request data without canonicalization (path traversal).
A concrete example of the SQL case, and its fix:
// Flag this in review
String sql = "SELECT * FROM users WHERE email = '" + email + "'";
stmt.execute(sql);
// Require this instead
PreparedStatement ps = conn.prepareStatement(
"SELECT * FROM users WHERE email = ?");
ps.setString(1, email);
If your team writes a lot of dynamic SQL, that is a signal to adopt a query builder or JPA criteria API rather than trusting every developer to concatenate safely.
Treat deserialization as a red flag
Java's native serialization has a long history of remote-code-execution gadgets, and the same danger shows up in libraries that turn bytes into objects. When a review touches ObjectInputStream, or a JSON/XML library configured with polymorphic type handling, slow down. The classic example is Jackson databind with default typing enabled, which has produced a steady stream of gadget-chain CVEs over the years. If you maintain Jackson-based code, our Jackson databind security guide covers the safe configuration.
In review, ask: does this endpoint deserialize data from a source the user can influence? If yes, is polymorphic typing off, and is there an allow-list of expected types? "We only accept our own format" is not a control an attacker respects.
Check authorization on every handler, not just authentication
Authentication answers "who are you"; authorization answers "are you allowed to do this." Spring Security makes the first easy and the second easy to forget. A frequent finding is a controller method that verifies a valid session but never checks that the logged-in user owns the resource in the path:
@GetMapping("/orders/{id}")
public Order get(@PathVariable Long id) {
return orderRepo.findById(id).orElseThrow(); // any user reads any order
}
The reviewer's job is to confirm the ownership or role check exists. Broken object-level authorization is one of the most common and highest-impact web bugs, and it is invisible to a scanner that does not understand your data model.
Hunt for secrets and unsafe defaults
Grep the diff mentally for anything that looks like a credential: password =, apiKey, connection strings, private keys. Secrets belong in a vault or environment configuration, never in source. Also watch for security-relevant defaults: TLS verification disabled for "testing," a permissive CORS config, setAllowedOrigins("*") on a state-changing endpoint, or logging that prints full request bodies containing tokens.
Keep reviews small and give evidence
The best security review process is one people actually complete. A 1,000-line pull request gets rubber-stamped; a 200-line one gets read. Encourage small, focused PRs. When you flag something, explain the exploit scenario briefly rather than just asserting it is wrong, because reviewers who understand the "why" catch the next instance themselves. Feeding known-vulnerable dependencies into the conversation helps too: an SCA tool such as Safeguard can surface that a library pulled in transitively is affected by a published CVE, so the discussion is about a real advisory rather than a hunch.
FAQ
What is the single highest-value thing to review in Java code?
Trust boundaries. Trace untrusted input to its sinks and confirm each one is parameterized, validated, or escaped. Injection and broken authorization cause more real incidents than any other category.
Should security review be separate from normal code review?
For most teams, no. Bake security into the standard review with a shared checklist. Reserve a separate deep review by a security specialist for high-risk changes like authentication, crypto, or payment flows.
How do I review dependency changes safely?
Diff the lockfile, not just the manifest, so you see transitive changes. Confirm no added dependency carries a known vulnerability and that version bumps are intentional. A dependency scanner in CI makes this a check rather than a chore.
Do static analysis tools replace human review?
No. Tools catch known patterns like tainted-data flows and unsafe API calls, which frees reviewers to focus on business logic, authorization, and design. The two are complementary, not interchangeable.