Secure Software Development / Advanced: components, evidence, and response / Lesson 15

Review security-sensitive changes by data and authority flow

Follow a change beyond its diff and turn observations into evidence-backed review findings.

About 4 min readContent reviewed 2026-10-28

On this page

Lesson goals

Trace untrusted values and trusted authority to the final use.
Write a finding with a concrete violated invariant and narrow repair.

Review the change in its surrounding flow

A small diff can change a large trust boundary. Adding an optional workshop id to a shared export helper may affect routes, background jobs, and administrative tools that call it. Read the requirement and decision record, identify callers, and trace how subject and resource scope reach the helper. Do not assume unchanged code is irrelevant when the new call pattern alters its assumptions. Focus first on authority, protected state, and interpretation boundaries; formatting and naming observations should not obscure a consequential missing permission check.

Connect a value to its final interpretation

A value-flow note can identify the source, transformations, validation, and final destination without copying an entire codebase. Include where a claimed organizer id becomes trusted, where a workshop is loaded under scope, and where contact fields are projected. The first example checks completeness of a fictional review trace. It does not analyze source code or prove the named controls exist. A reviewer must inspect the actual final adapter call and policy invocation. Helpers named safe or validated still need evidence about their contract and their callers.

python Check a fictional trace inventory

required = {"source", "trustedSubject", "resourceScope", "destination"}
trace = {"source":"request-workshop-id", "trustedSubject":"reviewed-identity-context", "resourceScope":"assigned-workshops", "destination":"permitted-export-projection"}
assert required <= trace.keys()
assert all(trace[key] for key in required)
incomplete = {k:v for k,v in trace.items() if k != "resourceScope"}
assert not required <= incomplete.keys()
print("review-trace completeness checked")

Expected output

review-trace completeness checked

The complete program verifies that four review fields are present and detects an omitted scope field. It validates a record structure, not the truth of its claims. No source analyzer, scanner, or application is run.

Write a finding that supports a decision

A useful finding states the location, controlled input, missing boundary, affected outcome, and proposed narrow correction. “This looks insecure” is difficult to verify or resolve. For the fictional export, explain that a client-selected workshop is loaded without assignment scope before attendee contacts are returned. Link a synthetic denied case and its expected unchanged state. The second record is custom editorial data, not a published vulnerability report. Its identifier is local and carries no severity claim about real software. Preserve uncertainty when code context or reachability remains unverified.

json A concrete fictional code-review finding

{"finding":"LAB-REVIEW-04","location":"fictional-export-helper","controlledInput":"requested-workshop-id","missingBoundary":"assignment-scoped-resource-load","protectedOutcome":"attendee-contact-confidentiality","repair":"apply-reviewed-scope-before-projection","regression":"different-organizer-no-export","status":"requires-implementation-evidence"}

This non-runnable record describes a hypothetical defect and regression. It is not evidence that this repository or any deployed product has the defect. The repair needs actual source review and approved execution before the finding can be closed.

Use automation to focus human reasoning

Tool output can reveal a suspicious flow or dependency, but it does not remove the need to inspect context. A false positive should be dismissed with a supported explanation of the actual boundary, not with a generic suppression. A true finding needs a repair that addresses the cause rather than merely changing syntax until the rule stops matching. Review suppressions and generated code assumptions explicitly. This lesson does not execute static analyzers or scanners; it teaches how their evidence would fit into a review with an accountable owner and a concrete verification plan.

Close with evidence and adjacent cases

After the correction, inspect the actual enforcement location and execute the focused synthetic regression at the approved boundary. Include an assigned-organizer success so the repair cannot simply block all exports. If the helper is shared, assess other callers that rely on the changed contract. Record source revision, observed results, and untested assumptions. A reviewed line change is useful evidence of intent, while executed behavior supplies a different kind of evidence. Keep both without claiming that one finding’s closure establishes the security of the whole application.

Common errors and fixes

Only changed lines are reviewed

Why it happens: Caller assumptions and shared boundaries are missed.
How to fix it: Trace the changed contract through relevant callers and final uses.
Prevent it next time: Review scope according to impact rather than diff size.

A finding says only insecure input

Why it happens: The controlled value and violated invariant are absent.
How to fix it: Explain the concrete source-to-outcome path and narrow repair.
Prevent it next time: Require a reproducible synthetic case or explicit evidence gap.

A suppression closes a real cause

Why it happens: The rule disappears while the unsafe boundary remains.
How to fix it: Review actual semantics and verify the protected outcome.
Prevent it next time: Treat suppressions as reviewed evidence decisions.

Check your understanding

1. A helper is renamed safe_export and the tool warning disappears. What should the reviewer ask?

Show worked solution

Ask whether resource loading is actually scoped before projection, whether trusted subject context is used, and whether the denied fixture now preserves state. A name or changed rule match is not a semantic repair. Inspect relevant callers and require focused observed evidence.

2. Why review a background caller when only the shared helper changed?