Triage and Remediation

A review produces findings. A finding produces a fix only if it survives the handoff — and most do not, because the ticket describes a symptom, the severity is copied from a rule label, and nobody wrote the test that would have stopped the same defect from returning through the next call site.

From finding to fix

The gap between a review and a remediation is a document. The reviewer knows the path — source, transformations, sink, preconditions — and the developer knows the domain. A ticket that carries only the sink hands the developer the job of rediscovering the path, and the response to that is either a patch at the sink or a question, and both are slower than the write-up would have been.

The workable shape is five fields: where, what reaches it, what the attacker gets, what pattern fixes it, and the test. That is short enough to write in the file and specific enough that the fix does not need a meeting.

Triage and severity

Scanner severities describe the rule, not the finding. The same rule produces a critical against an internet-facing endpoint and an informational against an admin-only, feature-flagged path — and the queue is only useful if those are ordered correctly. Two questions decide it: who can reach it, and what does reaching it give them.

Fix patterns that hold

A fix that filters the reported payload fixes the report. A fix that removes the capability fixes the class. The table below is the short version — each row maps a defect class to the cause, the minimal structural fix, and the assertion that proves it. Deep per-class recipes, with code for each framework, are in Remediation Guidance.

The regression test

The test that usually gets written for a security fix asserts the happy path. It passes before the fix, which means it is a smoke test with a security-themed name. A regression test asserts the attacker's request, against the expectation that it now fails — and the only way to know it is a regression test is to revert the fix and watch it fail.

// the fix that was applied: check the owner on the loaded row
router.get('/invoices/:id', requireAuth, async (req, res) => {
  const invoice = await Invoice.findByPk(req.params.id);
  if (!invoice) return res.sendStatus(404);
  if (invoice.customerId !== req.user.customerId) return res.sendStatus(404);
  res.json(invoice);
});

// and the test that was written for it - asserting the happy path
test('returns the invoice', async () => {
  const res = await request(app).get('/invoices/1').set(auth(userA));
  expect(res.status).toBe(200);
});

// the fix that was applied: check the owner on the loaded row
router.get('/invoices/:id', requireAuth, async (req, res) => {
  const invoice = await Invoice.findByPk(req.params.id);
  if (!invoice) return res.sendStatus(404);
  if (invoice.customerId !== req.user.customerId) return res.sendStatus(404);
  res.json(invoice);
});

// and the test that was written for it - asserting the happy path
test('returns the invoice', async () => {
  const res = await request(app).get('/invoices/1').set(auth(userA));
  expect(res.status).toBe(200);
});

// the test that fails on the old code: the attacker's request
test('does not return another customer invoice', async () => {
  const res = await request(app).get('/invoices/1').set(auth(userB));
  expect(res.status).toBe(404);
  expect(res.body.invoiceNumber).toBeUndefined();
});

// and the one that proves the fix did not break the owner's path
test('returns the invoice to its owner', async () => {
  const res = await request(app).get('/invoices/1').set(auth(userA));
  expect(res.status).toBe(200);
});

// reverting the ownership check makes the first test fail - that is
// the property that makes it a regression test rather than a smoke test

// the test that fails on the old code: the attacker's request
test('does not return another customer invoice', async () => {
  const res = await request(app).get('/invoices/1').set(auth(userB));
  expect(res.status).toBe(404);
  expect(res.body.invoiceNumber).toBeUndefined();
});

// and the one that proves the fix did not break the owner's path
test('returns the invoice to its owner', async () => {
  const res = await request(app).get('/invoices/1').set(auth(userA));
  expect(res.status).toBe(200);
});

// reverting the ownership check makes the first test fail - that is
// the property that makes it a regression test rather than a smoke test

Developer handoff

Hand off in order of consequence, not in the order the findings were written. A single high-severity finding with a complete path gets fixed in a day; twelve mediums in one ticket get triaged once and forgotten. If the report has to be batched, batch by class — "these three are the same missing ownership check" is a single change to a reviewer's list and a single change to a developer's.

Walk the first one through together. The pattern is what transfers: once a developer has seen an ownership check written against the loaded row rather than the route, they will find the remaining four themselves, and those four never reach your queue.

Measuring progress

Open-finding count is the metric that improves when you stop looking. The three that do not lie are the age of the oldest open high-severity finding, the share of findings closed with a regression test, and the ratio of findings found by a human to findings found by a scanner — a ratio that should fall as the rule set learns your codebase, and rise when a new class of defect appears.

Handoff Contents

Closing a Finding Honestly

Closing Checklist

Programme Notes