Skip to content

The Rust Engineering Handbook / Chapter 98

Team Standards, Code Review, Mentoring, and Unsafe Governance

Scale Rust expertise with enforceable repository defaults, risk-shaped review, explicit unsafe change control, and mentoring that distributes authority.

The pull request replaces a checked slice lookup in the message decoder with get_unchecked. The benchmark reports a 1.8 percent throughput improvement. A // SAFETY: comment says the index was validated earlier. The author wants approval from the team’s Rust expert; the Rust expert asks the domain reviewer whether empty extension fields can reach this branch; the domain reviewer assumes the type system prevents that. After two days, nobody owns the decision.

The diff has exposed a governance defect, not merely an unsafe block. Expertise is being treated as a person to summon instead of a set of responsibilities, evidence, and escalation rules. The specialist understands Rust’s validity and aliasing obligations. The domain reviewer understands which messages exist. The author owns the proposed proof and measurement. A release owner understands rollout and re-audit triggers. Approval requires their knowledge to meet; it does not require one person to possess all of it.

A scalable Rust standard sends ordinary work through ordinary review and makes exceptional obligations unmistakable. It automates stable mechanics, assigns risk-shaped review, records the contracts the compiler cannot check, and turns every specialist decision into teaching material that reduces the next bottleneck.

Responsibility follows the claim

Four roles are enough for most changes, and roles may be filled by different people over time:

Responsibility Must establish Must not be assumed to establish
Author intent, bounded diff, self-review, tests, measurements, documented trade-offs independent correctness or production authorization
Domain reviewer business and protocol behavior, failure effects, operability, realistic tests Rust memory-safety proof merely from domain familiarity
API or safety specialist public compatibility or unsafe obligation chain, evidence adequacy, containment the domain’s undocumented inputs or release readiness
Release owner rollout, rollback, monitoring, incident ownership, re-audit trigger implementation correctness without reviewer evidence

A safe internal refactor may need only author and domain reviewer. A public API change adds a compatibility reviewer. An unsafe boundary adds a safety specialist and a written safety case. A new direct dependency adds supply-chain and operational review. Risk determines the route; job title does not.

A responsibility map routes an ordinary safe change from author through domain review to release ownership. A separate unsafe-boundary route adds a documented obligation chain, specialist review, targeted evidence, and explicit re-audit triggers before release ownership. The paths show that domain and specialist knowledge meet without making either role responsible for the other's claims.
Unsafe governance scales when the change carries its proof through named responsibilities; asking one expert to approve every claim leaves both the domain and the release decision unowned.

The map is a minimum flow, not permission to serialize all work. Domain and specialist review can proceed together after the author supplies a coherent change. “Specialist approval” is not a ceremonial label: the approver must identify the safety contract, trace every caller and callee obligation, and judge whether evidence exercises the risky mechanism.

Repository defaults make policy executable

Put defaults in versioned files close to the code. A one-page standard should link to the actual configuration rather than duplicating it.

At workspace level, establish:

  • Rust edition, declared MSRV, resolver, and supported target policy;
  • formatting version and any intentional rustfmt configuration;
  • Rust and Clippy lint levels inherited by member crates where appropriate;
  • dependency source, feature, license, advisory, and exception policy;
  • commands for format, check, test, documentation tests, Clippy, and supported feature/target combinations;
  • generated-code boundaries and how generated output is reviewed;
  • unsafe inventory location, ownership, review date, and re-audit triggers;
  • documentation, decision-record, changelog, and release requirements.

Pinning a toolchain can make CI and local results reproducible. Declaring an MSRV answers a different compatibility question. Test both when both are promises. A current pinned compiler passing does not prove MSRV compatibility, and an MSRV build does not exercise new lints used by current development.

Formatting should be automatic and uninteresting. CI rejects drift; reviewers do not debate line wrapping. Lints need an ownership rule. Start from compiler warnings and a deliberate Clippy policy, enable higher-cost or opinionated lint groups selectively, and record local allowances beside the reason. -D warnings in CI can be useful for application work, but toolchain updates may introduce new warnings. A library with a broad MSRV or downstream compatibility promise may need staged lint upgrades. Maximal lint count is not a quality metric.

Cargo supports workspace lint configuration that packages can inherit. Keep exceptions narrow. Crate-root #![allow(...)] hides too much; prefer the smallest item or module that accurately represents generated code, platform shims, numerical intent, or another justified boundary. Give each allowance a removal condition or review trigger when it represents debt.

Review Rust through four lenses

Reviewing line by line without a model favors syntax comments over system risk. Use four passes suited to the change.

Authority and lifetime

Trace who owns each durable resource: data, buffer, file, socket, task, permit, lock guard, callback, foreign handle, and side effect. Check whether borrows express temporary observation or accidentally lock callers into storage choices. Ask when destruction occurs on success, error, panic, cancellation, and partial initialization.

Treat clone, Arc, 'static, Box::leak, and Arc<Mutex<_>> as design decisions when introduced to silence ownership errors. They may be correct. The review must name the new sharing, allocation, retention, contention, or lifecycle contract. Verify that a lock guard or borrow does not cross an await unless the synchronization design explicitly requires it.

Public API and compatibility

Inspect signatures as contracts: ownership transfer, allocation, panic conditions, error taxonomy, thread-safety, future Send behavior, feature flags, MSRV, platform availability, and SemVer exposure. Check that public dependency types do not accidentally make a third party part of the compatibility surface. Constructors should establish usable invariants; invalid intermediate states should not escape merely to mimic an old class shape.

An API reviewer asks which changes callers can observe, not whether the implementation is elegant. Adding a generic parameter, changing auto-trait behavior, tightening a lifetime, adding an enum variant, changing feature unification, or altering error behavior may matter even when ordinary callers still compile.

Failure and operations

Walk allocation failure where relevant, parse rejection, dependency timeout, overload, cancellation, panic, shutdown, upgrade, and rollback. Check queue bounds and task ownership. Ensure logs and metrics identify actionable states without leaking secrets or creating unbounded cardinality. Review configuration defaults and degraded modes as code, not deployment trivia.

Evidence and maintenance

Tests should defend the contract at the cheapest credible level: unit properties, compile-pass/fail API tests, integration behavior, model checking, Miri, sanitizers, fuzzing, benchmarks, or platform tests as the risk requires. A benchmark change needs correctness checks and variance context. New abstraction must pay for itself in a simpler contract, not only fewer local lines.

Review generated code, macro expansion, build scripts, enabled features, and dependency diffs when they change the artifact. The pull request view of src/ is not the complete program.

Unsafe is an inventory plus an obligation chain

A blanket ban on unsafe is honest for many application crates and useful as #![forbid(unsafe_code)] where exceptions are not intended. It is not a complete organization policy. FFI, allocators, kernels, drivers, SIMD, data structures, and safe abstractions may require unsafe operations. The governance objective is to concentrate them in owned boundaries and make their proof reviewable.

The inventory should include unsafe blocks, unsafe functions and traits, unsafe impls, foreign declarations, mutable statics, inline assembly, union access, generated unsafe code, and dependencies whose safety is material to the product. For each boundary record:

  • owning team and at least two people able to review or respond;
  • safe API exposed and callers that rely on it;
  • invariants and prohibited states;
  • each unsafe operation and the obligation it consumes;
  • evidence: tests, Miri or sanitizer jobs, fuzz targets, model reasoning, platform validation, and benchmark justification;
  • external assumptions about ABI, hardware, allocator, thread, provenance, or generated code;
  • last review and triggers for another review.

Rust 2024 warns by default when an unsafe operation inside an unsafe fn lacks an explicit unsafe block. Denying unsafe_op_in_unsafe_fn strengthens the visual boundary between the caller’s obligation and the implementation’s proof. It does not prove the block correct. Require a # Safety section on public unsafe functions and traits and a nearby SAFETY explanation for an unsafe operation. A comment that merely says “checked above” is too weak; name the condition, where it was established, and why it remains true here.

Return to the decoder diff. The proposed change must answer:

  1. Which index range makes get_unchecked valid?
  2. Which parser state establishes that range for every message variant, including empty and truncated extensions?
  3. Can mutation, aliasing, callback, panic, or concurrency invalidate it before use?
  4. Does the safe wrapper prevent every caller from violating the assumption?
  5. Which targeted tests, fuzz corpus, Miri run, or other evidence stresses the boundary?
  6. Does the measured gain survive end-to-end workload variance, and is it worth the permanent proof cost?

If bounds-check elimination already occurs, the unsafe change offers no benefit. If the gain is real but small, a safe representation that carries a validated nonempty range may recover most of it. If unsafe remains justified, keep the block minimal and make the safe wrapper the review center.

The fixture encodes routing without pretending to perform review:

if matches!(kind, ChangeKind::PublicApi | ChangeKind::UnsafeBoundary)
    && !evidence.api_or_safety_reviewer
{
    missing.push("API or safety specialist");
}
if matches!(kind, ChangeKind::UnsafeBoundary) && !evidence.contract_documented {
    missing.push("documented safety contract");
}

Policy automation can say evidence is absent. It cannot judge whether “index less than length” covers pointer provenance, initialization, aliasing, or concurrent mutation. Preserve human accountability.

Dependency approval begins with the requested capability

Review the capability before the crate. Ask whether the standard library, an existing dependency, a small internal implementation, a subprocess, or a platform facility already supplies it. Then inspect direct and transitive packages, enabled features, MSRV, licenses, advisories, maintenance and release health, build scripts, procedural macros, native code, unsafe surface, target support, provenance, and update ownership.

Approval is scoped to version range, feature set, target use, and product boundary. A parser used offline does not automatically earn approval in a privileged network service. Record who monitors advisories and who can remove or patch it. Lockfiles and reproducible builds preserve resolution evidence; they do not certify trust.

An exception needs an owner, reason, compensating controls, expiry or review date, and exit condition. Emergency dependency response may shorten the normal review path, but it should add follow-up review rather than erase the record.

Documentation is part of the type boundary

Public items document ownership, errors, panic conditions, safety requirements, cancellation, blocking, allocation, complexity where important, feature and target behavior, and examples that compile. Internal modules document invariants and why the chosen representation protects them. Operations docs cover configuration, signals, readiness, shutdown, dashboards, rollback, and incident diagnosis.

Design records are appropriate when a choice crosses team boundaries, adds unsafe code, exposes a public API, commits to a runtime or protocol, reverses a repository default, or accepts notable performance or dependency risk. Keep the decision concise: context, considered alternatives, decision, evidence, consequences, owner, and revisit trigger. Do not turn every refactor into governance paperwork.

Mentoring must transfer decisions

An onboarding path should increase authority in observable steps:

  1. read the workspace map, build and test policy, incident history, and selected safety cases;
  2. make a bounded safe change with a pairing partner;
  3. review ownership and failure behavior in another change;
  4. own a small API or operational decision and write its record;
  5. shadow an unsafe or dependency review, restating the obligation chain independently;
  6. lead a change through rollout and retrospective with backup.

Pairing is most useful at decision points: interpreting a borrow failure, choosing an ownership boundary, tracing cancellation, designing an error type, or reducing unsafe scope. One person typing while another silently watches transfers little. Alternate driver and reviewer; ask the learner to predict compiler behavior and operational consequences before running the tool.

Use office hours and rotating review duty for questions that can wait. Keep a searchable decision log and short annotated examples drawn from the codebase. Name components with a primary owner and a capable backup. Measure expert bottlenecks through review wait time, concentration of approvals, incident-call patterns, and components with only one qualified maintainer—not through training attendance.

The objective is not to eliminate specialists. Deep expertise remains essential. The objective is to reserve specialist attention for specialist claims while expanding the number of engineers who can formulate those claims, provide evidence, and handle ordinary work.

Incidents rewrite standards with evidence

After an incident, trace which defense was missing or misleading. A deadlock may reveal an undocumented lock order; a cancellation data loss may reveal that the API hid a commit point; a memory defect may reveal an incomplete unsafe contract; a bad upgrade may reveal that the feature or MSRV matrix omitted a supported path.

Update the smallest durable control: a type invariant, test, lint, runbook, review question, safety case, ownership assignment, or escalation threshold. Avoid adding a broad rule that would not have prevented the event. Record a re-audit trigger when compiler semantics, dependency internals, hardware assumptions, public callers, or the enclosing safe abstraction changes.

Review the standard on a cadence and after material incidents. Remove obsolete rules. A policy that only grows becomes unreadable, and engineers route around it.

Draft the engineering standard and escalation policy

Write a standard short enough to use during a pull request. It should contain concrete defaults and links to executable configuration.

Required standard sections

  1. supported edition, pinned development toolchain, MSRV, targets, and ownership;
  2. exact local and CI commands, formatting policy, lint levels, and allowance rule;
  3. review routes for safe internal, public API, unsafe boundary, direct dependency, operational, and generated-code changes;
  4. ownership/lifetime, API, failure/operations, and evidence questions;
  5. unsafe inventory fields, safety documentation, specialist qualifications, and re-audit triggers;
  6. dependency approval, exceptions, advisories, and removal ownership;
  7. public, invariant, design-record, and runbook documentation expectations;
  8. onboarding milestones, pairing practice, review rotation, and backup ownership;
  9. incident feedback and policy-pruning cadence.

Escalation decisions

For each change class, state who may approve, what evidence is mandatory, who resolves disagreement, and what happens when capacity is unavailable. A credible answer to unavailable safety review is “the unsafe change waits or is redesigned safely,” not “the author self-approves because release is urgent.” A production security fix can use an emergency path with two accountable reviewers, contained rollout, and mandatory retrospective, but urgency does not make an unreviewed obligation true.

Test the draft against five pull requests: a safe internal refactor, a public enum change, a new async runtime feature, a direct dependency with a build script, and the decoder’s get_unchecked optimization. If every change follows the same heavyweight route, the policy will bottleneck. If unsafe or public compatibility changes look ordinary, it is too weak. If reviewers cannot point to the exact configuration, inventory, or decision record, it is not yet executable.

The mature standard does two things at once: it lowers friction for established safe defaults and raises the quality of evidence at boundaries where Rust asks humans to carry the proof. That distribution of responsibility is what lets a migration become a maintained system. The remaining question is temporal: how these agreements, dependencies, editions, MSRV promises, incidents, and safety cases stay healthy over years of change.

Sources and version notes

  • The Cargo workspace reference documents shared workspace lint configuration and inheritance mechanics. Teams still choose which levels and exceptions fit their compatibility policy.
  • The Clippy usage and configuration documentation describe current invocation and configuration behavior, including MSRV-aware lint configuration. Clippy configuration itself is documented as unstable, so pin and review assumptions.
  • The Rust 2024 Edition Guide documents the default warning for unsafe operations without explicit blocks inside unsafe functions. The standard-library unsafe keyword documentation demonstrates # Safety contracts and explicit safety reasoning.
  • The executable examples/rust-engineering-handbook/part-15/adoption-governance fixture uses Rust 2024, declares MSRV 1.85, and forbids unsafe code in the policy model. It verifies routing mechanics, not the adequacy of a real safety review.