Home
Services
Secure Code Review

05Station 05 · Build

Secure Code Review

A human reads the changes that matter: auth, payments, crypto, parsing and anything an attacker can reach. Scanners do the rest.

Book a call →Walk the line

What a finding looks like here

Not a PDF. A comment on the line, with a fix you can apply.

src/rewards/redeem.tsPR #318 · partner redemption

1export async function redeem(userId: string, offerId: string) {

2 const offer = await offers.get(offerId);

3- const balance = await ledger.balance(userId);

4- if (balance < offer.cost) throw new InsufficientPoints();

5- await ledger.debit(userId, offer.cost);

ATALAIARace condition

The check and the debit are two calls. Two requests at the same moment both pass the check and spend the same points twice. Suggested change: one conditional debit in the ledger.

DEV LEAD

Applied. Added a test that fires two redeems in parallel.

6+ const ok = await ledger.debitIf(userId, offer.cost, { atLeast: offer.cost });

7+ if (!ok) throw new InsufficientPoints();

8 return partner.issueVoucher(offer, userId);

9}

One review, start to finish

Pick a step, or let it play. Every line is what someone in the room actually says.

Step 1Find the risky paths

Auth, money, data access, parsing, crypto and external input.

ATALAIAWhich code moves points or checks who you are?

DEV LEADredeem(), the ledger client and the partner callback parser.

ATALAIAThose always get a human review. The scanners take the rest.

Leaves the roomRisky paths: auth, redeem, ledger, parser

Step 2Read the diffs

Line by line, with the threat model next to the code.

ATALAIAIn redeem(), the balance check runs, then the write. Nothing in between.

DEVELOPERIt's fast, though. Can two requests really land together?

ATALAIAEasily, with a script. Two threads, one balance, two vouchers.

Leaves the roomPR #318: 214 lines read

Step 3Comment where devs work

Findings land as PR comments with a suggested fix.

ATALAIAThe fix is in the PR as a suggestion: one conditional debit.

DEVELOPERApplied. My test with two parallel redeems now fails one of them.

Leaves the room3 PR comments, 1 suggested fix

Step 4Teach the pattern

Checklists and pairing so the next review needs us less.

DEV LEADCan we turn this into a checklist for the senior devs?

ATALAIAEight checks for money paths. Next review you lead, and I watch.

Leaves the roomChecklist: 8 checks for money paths

ATALAIA
DEV LEAD
DEVELOPER
DEVELOPER 2
DEVELOPER 3

Before and after

Before

Scanners are good at known patterns and blind to logic. Broken authorisation, race conditions and trust mistakes need someone who reads the code and asks why.

Every pull request gets the same review, whatever it touches
Authorisation logic is spread across many services
Scanner findings are mostly ignored

After

Reviewed diffs with findings as review comments, not a report
Fixes suggested in code
A short list of risky paths that always get a security review
Review checklists your senior devs can use

Typical shape: Per release, per feature, or a standing slice of review capacity.

Next on the line

Every station reports to the same tower. These are the neighbours.

Station 08 · BuildSecurity Scans

SAST · SCA · Secrets · IaC · Container

See the station →Station 03 · DesignThreat Modeling

Business, devs and cyber, one table

See the station →Station 04 · BuildToolchain Hardening

IDEs, AI agents, MCP, endpoints

See the station →

Talk it through

A 30-minute call. No slides, no price list, and a next step either way.

Book a call →Walk every station