A Worked Secure Design Review
One feature, taken from a requirement to a reviewed design, applying the whole course end to end.
Everything in this course, applied once, to a single realistic feature: "customers can export their transaction history and have it delivered to a URL they configure."
Why this feature
It is deliberately dense. In four lines of requirement it touches authorization, tenancy, data exposure, resource exhaustion, SSRF, audit, retention and business-flow abuse. Real features are like this.
The process
- Clarify. Who exports, what data, how much, delivered how?
- Draw the flow and mark every trust boundary.
- STRIDE against each boundary.
- Decide per threat: mitigate, transfer, accept or eliminate.
- Design with the mitigations in it, not bolted on.
- Write the tests that prove each control.
The finding that changes the design
"Delivered to a URL they configure" is SSRF as a product requirement, and it also means your servers POST customer data to an address a customer chose. The right answer is usually not to defend it but to change the requirement: deliver to a signed download URL, or to a registered and verified webhook endpoint. That is the most valuable thing a design review produces — a requirement changed before anything is built.
What good looks like
The mitigations are in the design rather than added afterwards. Every one names a test. Accepted risks have an owner and a date. And the whole thing fits on two pages.
Example
# The requirement, as written
"Customers can export their transaction history as CSV and have it delivered
to a URL they configure."
# Four lines. Eight security domains:
# authorization — whose transactions? which tenant?
# data exposure — which columns are in the CSV?
# resource use — how many rows? what does it cost?
# SSRF — "a URL they configure"
# exfiltration — we POST their data to that URL
# audit — who exported what, and when?
# retention — how long does the file exist?
# business abuse — what if they export continuously?When to use it
- A design review replaces a client-supplied delivery URL with a signed download link, removing an SSRF vector before implementation.
- A threat model on an export feature surfaces an unbounded query that would have taken the database down at launch.
- A team ships an export feature with per-control tests written from the design document rather than added after a review.
More examples
The threat model, with decisions
Recording the options considered and why one was chosen is what makes the decision defensible later — and it is what a new engineer needs to avoid reopening it.
# docs/threat-model/transaction-export.md
## Feature: transaction export
Owner: @billing Reviewed: 2026-08-04 Next: 2027-02-04
### Data flow, with trust boundaries (═══)
Browser ═══ API ─── Job queue ─── Worker ─── Database
(HOSTILE) (trusted) │ (trusted)
═══
Object storage ═══ Customer's URL
(HOSTILE)
### STRIDE
| # | Cat | Threat | L | I | Decision | Control | Test |
|---|-----|--------|---|---|----------|---------|------|
| 1 | I | Export another customer's transactions | H | Crit | Mitigate | tenant+user from the token; RLS on the query | `export.test:"rejects foreign tenant"` |
| 2 | I | CSV contains internal columns | H | High | Mitigate | explicit column allowlist in `ExportSerializer` | `export.test:"public columns only"` |
| 3 | D | Unbounded export exhausts the database | M | High | Mitigate | async queue, 1 concurrent per tenant, 1M row cap, statement timeout | `load/export.k6.js` |
| 4 | **I** | **"delivered to a URL they configure" = SSRF, and we POST their data to it** | **H** | **Crit** | **ELIMINATE** | **requirement changed — see below** | n/a |
| 5 | T | Download URL shared or replayed | M | Med | Mitigate | signed URL, 15 min, single use, bound to the requesting user | `export.test:"url expires"` |
| 6 | R | Cannot prove who exported what | M | High | Mitigate | audit: actor, tenant, filters, row count, ip, request id | `audit.test` |
| 7 | E | A non-admin exports the whole tenant | L | Crit | Mitigate | `requireRole('admin')` on the tenant-wide variant | `authz.test` |
| 8 | I | The file persists in storage indefinitely | M | Med | Mitigate | lifecycle rule: delete after 7 days | infra test |
| 9 | D | Continuous exporting as scraping | M | Med | Mitigate | 5 exports per tenant per day; alert above baseline | `quota.test` |
| 10 | S | A stolen token used to export | L | High | **Accept** | 10-min tokens + volume alerting on exports | — |
### Finding 4 — the requirement changed
The original requirement ("delivered to a URL they configure") is SSRF by
design AND turns our infrastructure into an exfiltration channel for their own
data to an address we do not control.
Options considered:
a) Defend it: resolve + validate + connect-to-IP + no redirects.
Workable, and we still POST customer data to an arbitrary host.
b) Registered webhook endpoints only, verified at registration.
c) Signed download URL; the customer pulls.
→ Chose (c), with (b) available for customers who ask.
Rationale: (c) removes the outbound request entirely. No SSRF surface, no
exfiltration path, and it is simpler to build. Agreed with @product 2026-08-04.
### Accepted risks
- **#10**: sender-constrained tokens (DPoP) would close this. Deferred to Q4.
Accepted by @security-lead 2026-08-04. Revisit if export volume grows.
# The design review's single most valuable output is line 4.The implementation, with every control visible
Annotating each control with its threat-model row number is what keeps the design and the code connected — a reviewer can verify coverage without re-deriving the analysis.
// Each control traces back to a row in the threat model.
import { z } from 'zod';
const exportSchema = z.object({
from: z.coerce.date(),
to: z.coerce.date(),
format: z.enum(['csv', 'json']),
scope: z.enum(['own', 'tenant']).default('own'),
// NOTE: no deliveryUrl. Threat #4 — eliminated at the requirement.
}).strict()
.refine((v) => v.to > v.from, 'to must be after from')
.refine((v) => v.to - v.from <= 366 * 864e5, 'range must be one year or less');
app.post('/api/exports',
auth,
validate({ body: exportSchema }),
quota('export', { perTenantPerDay: 5 }), // #9
async (req, res) => {
// #7 — a tenant-wide export requires a role
if (req.body.scope === 'tenant' && req.user.role !== 'admin') {
return res.status(403).json({ error: 'insufficient_permissions' });
}
// #3 — estimate BEFORE queueing, and refuse what is too large
const estimate = await estimateRowCount(req.user, req.body);
if (estimate > 1_000_000) {
return res.status(400).json({
error: 'range_too_large', estimatedRows: estimate,
detail: 'Narrow the date range or export in parts.',
});
}
// #3 — one concurrent export per tenant
if (await activeExportsFor(req.user.tenant) >= 1) {
return res.status(409).json({ error: 'export_already_running' });
}
// Identity is captured HERE, from the credential, and travels with the job.
const job = await queue.add('export', {
tenantId: req.user.tenant, // #1
userId: req.user.id,
scope: req.body.scope,
from: req.body.from, to: req.body.to, format: req.body.format,
requestId: req.id,
});
await audit.record({ // #6
event: 'data.export_requested',
actor: { id: req.user.id, tenantId: req.user.tenant,
impersonatedBy: req.sessionData?.impersonatedBy },
target: { type: 'export', id: job.id },
context: { ip: req.ip, requestId: req.id },
metadata: { scope: req.body.scope, from: req.body.from, to: req.body.to,
estimatedRows: estimate },
});
res.status(202).json({ exportId: job.id, status: 'queued' });
});
// ── The worker ───────────────────────────────────────────────────────
worker.process('export', async (job) => {
const { tenantId, userId, scope, from, to, format } = job.data;
if (!tenantId || !userId) throw new Error('job missing identity'); // #1
// #1 — RLS enforces tenancy even if the query below were wrong
const rows = await withTenantTx(tenantId, (tx) =>
tx('transactions')
.where('created_at', '>=', from).andWhere('created_at', '<', to)
.modify((q) => scope === 'own' && q.where({ user_id: userId }))
.orderBy('created_at')
.limit(1_000_000) // #3
.timeout(300_000));
// #2 — explicit columns; the serializer refuses an empty field list
const file = await ExportSerializer.write(rows, format);
// #8 — namespaced key, and a lifecycle rule deletes it after 7 days
const key = `exports/${tenantId}/${job.id}.${format}`;
await s3.putObject({
Bucket: EXPORTS_BUCKET, Key: key, Body: file,
ServerSideEncryption: 'aws:kms',
ContentDisposition: 'attachment',
Metadata: { tenantId, userId, expiresAt: addDays(new Date(), 7).toISOString() },
});
await audit.record({ // #6
event: 'data.exported',
actor: { id: userId, tenantId },
target: { type: 'export', id: job.id },
metadata: { rowCount: rows.length, bytes: file.length, scope },
});
await notifyUser(userId, { exportId: job.id, rows: rows.length });
});
// ── Download: signed, short-lived, single use, bound to the user ─────
app.get('/api/exports/:id/download', auth, async (req, res) => {
const record = await db('exports')
.where({ id: req.params.id, tenant_id: req.user.tenant }) // #1
.first();
if (!record) return res.status(404).json({ error: 'not_found' });
// #5 — the requester must be the person who asked for it
if (record.user_id !== req.user.id && req.user.role !== 'admin') {
return res.status(404).json({ error: 'not_found' });
}
if (record.downloaded_at) {
return res.status(410).json({ error: 'already_downloaded' }); // single use
}
await db('exports').where({ id: record.id }).update({ downloaded_at: new Date() });
await audit.record({ event: 'data.export_downloaded',
actor: { id: req.user.id, tenantId: req.user.tenant },
target: { type: 'export', id: record.id },
context: { ip: req.ip, requestId: req.id } });
res.redirect(await signedUrl(record.storage_key, { expiresIn: 900 })); // #5
});The tests that make the model true
Asserting that the eliminated field is rejected — rather than simply not implementing it — is what stops threat #4 being reintroduced by a later feature request.
// Every mitigation in the threat model names a test. Here they are, in the
// same order, so coverage is verifiable at a glance.
describe('transaction export — threat model coverage', () => {
// #1 — cross-tenant and cross-user
it('never exports another tenant\'s transactions', async () => {
const res = await createExport(aliceToken, { scope: 'own' });
const rows = await downloadAndParse(res.body.exportId, aliceToken);
const foreign = await db('transactions')
.whereIn('id', rows.map((r) => r.id)).whereNot({ tenant_id: acme.id });
expect(foreign).toHaveLength(0);
});
it('RLS blocks it even with a deliberately unscoped query', async () => {
const rows = await withTenantTx(acme.id, (tx) =>
tx.raw('SELECT * FROM transactions')); // no WHERE at all
expect(rows.rows.every((r) => r.tenant_id === acme.id)).toBe(true);
});
// #2 — column allowlist
it('the CSV contains only public columns', async () => {
const rows = await downloadAndParse(await exportFor(alice), aliceToken);
const columns = Object.keys(rows[0]);
expect(columns).toEqual(['id', 'date', 'description', 'amount', 'currency']);
for (const forbidden of ['internal_notes', 'risk_score', 'raw_payload']) {
expect(columns).not.toContain(forbidden);
}
});
// #3 — resource bounds
it('refuses a range that would exceed the row cap', async () => {
await seedTransactions(acme.id, 2_000_000);
const res = await createExport(aliceToken, { from: '2000-01-01', to: '2030-01-01' });
expect(res.status).toBe(400);
expect(res.body.error).toBe('range_too_large');
});
it('allows only one concurrent export per tenant', async () => {
await createExport(aliceToken);
const second = await createExport(bobToken); // same tenant
expect(second.status).toBe(409);
});
// #4 — the eliminated threat: assert the field is not accepted at all
it('does not accept a delivery URL', async () => {
const res = await request(app).post('/api/exports')
.set('Authorization', `Bearer ${aliceToken}`)
.send({ from: '2026-01-01', to: '2026-02-01', format: 'csv',
deliveryUrl: 'http://169.254.169.254/' });
expect(res.status).toBe(400); // .strict() rejects it
});
// #5 — download URL
it('the download link expires and is single use', async () => {
const id = await exportFor(alice);
expect((await download(id, aliceToken)).status).toBe(302);
expect((await download(id, aliceToken)).status).toBe(410); // second attempt
});
it('another user cannot download it', async () => {
const id = await exportFor(alice);
expect((await download(id, bobToken)).status).toBe(404);
});
// #6 — audit
it('records who exported what', async () => {
const id = await exportFor(alice);
const entries = await db('audit_log').where({ target_id: id }).orderBy('at');
expect(entries.map((e) => e.event)).toEqual([
'data.export_requested', 'data.exported',
]);
expect(entries[1].metadata.rowCount).toBeGreaterThan(0);
expect(entries[0].actor_id).toBe(alice.id);
});
// #7 — role on the tenant-wide variant
it('a non-admin cannot export the whole tenant', async () => {
expect((await createExport(aliceToken, { scope: 'tenant' })).status).toBe(403);
expect((await createExport(adminToken, { scope: 'tenant' })).status).toBe(202);
});
// #9 — quota
it('limits exports per tenant per day', async () => {
for (let i = 0; i < 5; i++) { await completeExport(aliceToken); }
expect((await createExport(aliceToken)).status).toBe(429);
});
});
// #8 (retention) is infrastructure — asserted in the Terraform test suite:
// the exports bucket has a lifecycle rule expiring objects after 7 days.
// One test per mitigation, named after the threat. A reviewer can hold the
// model beside the suite and see that nothing was quietly dropped.
Discussion