Ask ten things: what is being protected, which algorithm and mode, where the key lives, where the nonce comes from, how passwords are hashed, how secrets are compared, which random source is used, how failures are handled, how rotation works, and whether a vetted library is used. Any vague answer is a finding.
The ten questions
- What is the goal? Confidentiality, integrity, authentication, or password storage? Each has a different tool, and mixing them up is the root of many bugs.
- Which algorithm and mode? OWASP recommends AES with at least 128 bit keys, ideally 256, and names GCM and CCM as the first preference. ECB should not be used outside very specific circumstances.
- Where does the key come from and live? Generated from a CSPRNG, not typed words. Not hard coded, not in version control. OWASP prefers platform secure storage such as an HSM, cloud key vault or secrets manager.
- Where does the IV or nonce come from? Generated by the library, never constant. CWE-329 covers predictable IVs in CBC.
- Are passwords hashed with a password hash? Not encrypted, and not a plain fast hash. OWASP names Argon2id first.
- Are secrets compared in constant time? CWE-208 describes the timing weakness. Use the platform function.
- Is the random source secure? OWASP says PRNGs must not be used for anything security critical.
- What happens on failure? A failed authentication check must discard the data. Never fall back to plaintext.
- How will the key rotate? OWASP says the code and processes to rotate a key should exist before they are needed.
- Is this a vetted library, and is it current? CWE-327 says to avoid custom cryptography.
Red flags in a diff
| Seen in the diff | Why it matters |
|---|---|
| A cipher name with no mode | The default may be unsafe |
| Constant byte string near an encrypt call | Fixed IV or key |
| md5 or sha1 near a security decision | Broken for integrity |
| General random function for tokens | Predictable output |
| == on a MAC or token | Timing leak |
| A try block that returns data on decrypt error | Failed authentication ignored |
| Key in a config file in the repo | Exposure through version control |
| New homemade protocol or format | Unreviewed design |
Make it routine
Put the ten questions into the pull request template, shown only when the diff touches files that import a crypto package. A short list that appears at the right time works better than a long policy no one reads.
Pair it with automated checks. Code scanning can flag weak algorithm names and hard coded keys. Dependency scanning can flag a crypto library with a known advisory. A policy gate can then stop the merge until a human answers.
Try it yourself
Open the pull request gate demo and watch a check block a change. Think about which of the red flags above you could turn into a rule that blocks, and which should stay a human question.
Click through it at your own pace, or open it full screen.
Safeguard can generate an SBOM and scan your dependencies, so outdated cryptography libraries with known advisories show up in one inventory, and policies can gate a build on them. A free scan is at gold.safeguard.sh/scan.
Quick check
Who should review crypto changes?
Anyone, with the checklist. Add a second reviewer with security experience for new protocols, new key storage, or anything on the red flag list.
Can a linter replace this review?
No. A linter finds known bad patterns. It cannot tell whether the design protects the right thing.
Sources
Last reviewed: 19 October 2026.