◇ OpenCyber Academy
Engineering · Chapter 45 · English

Chapter 45 — Independent design review

Open, free learning for anyone who wants to protect people and technology. No account is required to read this text or its worked answers.

Chapter 45 — Independent design review

Lesson map

Lesson 45 published. Map, example, EX133.

flowchart TD
  N0["Learning outcomes"]
  N1["45.1 Review rights"]
  N2["45.2 Checklist"]
  N3["45.3 Dissent"]
  N4["Practice note"]
  N5["45.1b External reviewers and SLOs"]
  N6["Field note — Friday merge ban"]
  N7["Synthesis"]
  N0 --> N1 --> N2 --> N3 --> N4 --> N5 --> N6 --> N7
  N7 --> EX["Worked example"]
  EX --> Q["Exercise EX133"]

Logical drawing

flowchart LR
  N0["Learning outcomes"]
  N1["45.1 Review rights"]
  N2["45.2 Checklist"]
  N3["45.3 Dissent"]
  N4["Practice note"]
  N5["45.1b External reviewers and SLOs"]
  N6["Field note — Friday merge ban"]
  N7["Synthesis"]
  N0 --> N1 --> N2 --> N3 --> N4 --> N5 --> N6 --> N7 --> Q["EX133"]

[Learning outcomes]
    |
    v
[45.1 Review rights]
    |
    v
[45.2 Checklist]
    |
    v
[45.3 Dissent]
    |
    v
[Practice note]
    |
    v
[45.1b External reviewers and SLOs]
    |
    v
[Field note — Friday merge ban]
    |
    v
[Synthesis]
    |
    v
[Worked example] --> [Exercise EX133]

Learning outcomes

You will (1) run a review that can reject; (2) use a checklist tied to safety requirements; (3) record dissenting opinions.

45.1 Review rights

Reviewers may block merge for fail-open paths, missing tests, SIS reachability regressions, or empty scores in UI. LGTM without the checklist is invalid.

At ABP, CNHP reviewers have written authority to block a merge on any of these grounds, and the plant manager has agreed not to override a block to meet a deadline. A reviewer who approves without completing the checklist has not reviewed the change, and the merge is reverted until a proper review is done. Lena keeps a record of blocks and their reasons, which showed that most blocks concerned missing tests rather than unsafe designs.

45.2 Checklist

Fail-closed; policy authn; schema forbids payloads; coverage has denominator; secrets redaction; rollback path documented; SIS reachability test green.

Lena's checklist at ABP has one line for each item, and each line links to the requirement it enforces, such as S-1 for fail-closed behaviour and R-12 for SIS-B reachability. The reviewer ticks a line only after seeing the evidence, such as the test that fails when policy is missing. Rollback is checked by naming the previous version and the command to restore it, and the reachability test must have run on the exact commit being merged.

45.3 Dissent

If one reviewer disagrees with a conduit or allow-list design, the dissent goes into the ADR even if overruled — a future plant incident review needs that trail.

Synthetic case — Ashford Batch Plant. A coverage badge PR was blocked twice under this checklist; the third revision shipped the honest UI (Chapter 35).

At ABP the external reviewer's dissent about the collector sending alerts to the HMI was recorded verbatim, and in that case it changed the design. Lena keeps overruled dissents in the same place, because a later incident may prove them right.

45.4 Errors and limits

Reviews after merge are archaeology, not control.

At ABP a small change to the alert format was once merged before review because the deadline was close, and the review that followed found it had removed the reason code that operators relied on. The fix required a second release during a window. Since then CNHP's merge queue refuses any change without a completed checklist, and a review requested after merging is recorded as a process failure in the retrospective rather than counted as a review.

Practice note

Reviewers run the monitor's critical tests on the synthetic plant fixtures before the meeting — not only read the diff.

45.1b External reviewers and SLOs

Once per term, a control engineer from outside the CNHP squad reviews a safety-critical PR. Reviews complete within an agreed number of business days or the change expires from the merge queue (not auto-merges). Forbid Friday-evening merges before windows unless a safety hotfix with Lab I evidence.

Blocking comments must cite a requirement id or checklist item number. Repeated checklist skips by the same reviewer trigger a coaching conversation. The calendar is part of the control design.

ABP's external reviewer came from a water utility.

Field note — Friday merge ban

A non-hotfix UI merge on Friday evening before a Saturday window was reverted. The calendar rule exists because tired reviewers miss fail-open paths. Heroic late merges are a smell.

The external reviewer's findings are filed like any other review, with requirement ids.

A review that misses its SLO is reported in the retrospective with the reason it was late.

45.5 Worked example — a dissent that changed the design (synthetic ABP)

In the independent design review, an external reviewer from a water utility challenged the plan to let the monitoring collector send alerts directly to the HMI. Her argument: an alert flood could distract operators during an upset, and a compromised collector would gain a path into L2. The review checklist item "does any security component add a conduit into a control zone?" had been ticked incorrectly. The dissent is recorded verbatim, the design changes so alerts go to the SOC and the shift lead's phone instead, and the review SLO (comments answered within five working days) is met.

Exercises

Synthesis

Reviews that can say no, with a checklist and a memory.

Continue studying — chapter 45 of 50

← Chapter 44 (draft) · Course chapter index · Chapter 46 (draft) →