From fc9f1c14ce89f555d5bc02a1b59f7eb15f18398c Mon Sep 17 00:00:00 2001 From: Jonathan McCaffrey Date: Wed, 20 May 2026 16:31:53 +0000 Subject: [PATCH 1/5] docs: propose +2 code review policy and CODEOWNERS layout Adds an initial draft of the PLC +2 Code Review gate (MVSB-32942) along with a sectioned CODEOWNERS file mapping subsystems to placeholder @NVIDIA/flashdreams-* teams. The doc embeds the literal +2 Security Review Checklist from NVIDIA's PLC: Code Review Excellence training as an LLM-fillable table for reviewers. Both files are drafts pending team consensus on: - subsystem boundaries and per-recipe owner teams, - which paths require @NVIDIA/flashdreams-security sign-off, - branch protection settings to flip on (require Code Owner review, dismiss stale approvals, no admin bypass). Co-Authored-By: Claude Opus 4.7 (1M context) Signed-off-by: Jonathan McCaffrey --- .github/CODEOWNERS | 102 +++++++++++++++ docs/code_review.md | 302 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 404 insertions(+) create mode 100644 .github/CODEOWNERS create mode 100644 docs/code_review.md diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS new file mode 100644 index 000000000..718178e60 --- /dev/null +++ b/.github/CODEOWNERS @@ -0,0 +1,102 @@ +# FlashDreams CODEOWNERS +# +# This file declares the GitHub teams responsible for approving changes +# in each part of the repository. It is the source of truth for code +# review responsibility, as described in CONTRIBUTING.md and in +# docs/code_review.md. +# +# Format reference: https://docs.github.com/en/repositories/managing-your-repositories-settings-and-security/managing-repository-settings/about-code-owners +# +# Notes: +# * GitHub only honours ONE CODEOWNERS file per repository. This is +# that file. "Delegation" between subsystems is expressed by +# path-prefix rules below — each section behaves like its own +# CODEOWNERS for the subtree it covers. +# * The last matching rule wins, so more specific paths appear below +# more general ones. +# * Owners listed here are the project's qualified "+2" reviewers. +# They apply the +2 Security Review Checklist on every review (see +# docs/code_review.md) and are trained as required by NVIDIA PLC. +# Branch protection on `main` requires at least one approving +# review from a CODEOWNER for the touched paths. On +# security-sensitive paths, the `@NVIDIA/flashdreams-security` +# team's sign-off is additionally required. +# * External contributors are welcome on every path. CODEOWNERS does +# not gate who may *open* a PR — only who must *approve* it. +# * The PLC gate cannot be bypassed or waived; any exception requires +# a documented audit trail approved by the Security PIC. + +# --------------------------------------------------------------------- +# Default owners (fallback for any path not matched below) +# --------------------------------------------------------------------- +* @NVIDIA/flashdreams-maintainers + +# --------------------------------------------------------------------- +# Core library — flashdreams/ +# --------------------------------------------------------------------- +/flashdreams/ @NVIDIA/flashdreams-maintainers +/flashdreams/flashdreams/core/ @NVIDIA/flashdreams-core +/flashdreams/flashdreams/infra/ @NVIDIA/flashdreams-infra +/flashdreams/flashdreams/recipes/ @NVIDIA/flashdreams-recipes +/flashdreams/flashdreams/plugins/ @NVIDIA/flashdreams-core +/flashdreams/flashdreams/configs/ @NVIDIA/flashdreams-recipes +/flashdreams/flashdreams/_pytest_plugins/ @NVIDIA/flashdreams-maintainers +/flashdreams/tests/ @NVIDIA/flashdreams-maintainers + +# --------------------------------------------------------------------- +# Integrations — integrations// +# +# Each integration is a workspace member with its own pyproject.toml. +# The integration's lead reviewers own their subdirectory; the +# integrations team is added as a backstop so no integration is +# orphaned if its lead is unavailable. +# --------------------------------------------------------------------- +/integrations/ @NVIDIA/flashdreams-integrations +/integrations/alpadreams/ @NVIDIA/flashdreams-integrations @NVIDIA/flashdreams-alpadreams-owners +/integrations/alpadreams/ludus-renderer/ @NVIDIA/flashdreams-integrations @NVIDIA/flashdreams-ludus-renderer-owners +/integrations/causal_forcing/ @NVIDIA/flashdreams-integrations @NVIDIA/flashdreams-causal-forcing-owners +/integrations/cosmos_predict2/ @NVIDIA/flashdreams-integrations @NVIDIA/flashdreams-cosmos-owners +/integrations/fastvideo_causal_wan22/ @NVIDIA/flashdreams-integrations @NVIDIA/flashdreams-wan-owners +/integrations/lingbot/ @NVIDIA/flashdreams-integrations @NVIDIA/flashdreams-lingbot-owners +/integrations/self_forcing/ @NVIDIA/flashdreams-integrations @NVIDIA/flashdreams-causal-forcing-owners +/integrations/wan21/ @NVIDIA/flashdreams-integrations @NVIDIA/flashdreams-wan-owners + +# --------------------------------------------------------------------- +# Documentation — docs/ +# --------------------------------------------------------------------- +/docs/ @NVIDIA/flashdreams-docs +/docs/source/apis/ @NVIDIA/flashdreams-docs @NVIDIA/flashdreams-maintainers +/docs/code_review.md @NVIDIA/flashdreams-maintainers @NVIDIA/flashdreams-security + +# --------------------------------------------------------------------- +# Build, container, and CI plumbing +# --------------------------------------------------------------------- +/docker/ @NVIDIA/flashdreams-infra @NVIDIA/flashdreams-ci +/.github/ @NVIDIA/flashdreams-ci +/.github/workflows/ @NVIDIA/flashdreams-ci @NVIDIA/flashdreams-maintainers +/.github/CODEOWNERS @NVIDIA/flashdreams-maintainers @NVIDIA/flashdreams-security + +# --------------------------------------------------------------------- +# Workspace / dependency / packaging metadata +# +# Changes here ripple across every subsystem, so they require sign-off +# from the maintainers team. +# --------------------------------------------------------------------- +/pyproject.toml @NVIDIA/flashdreams-maintainers +/uv.lock @NVIDIA/flashdreams-maintainers +/flashdreams/pyproject.toml @NVIDIA/flashdreams-maintainers +/integrations/*/pyproject.toml @NVIDIA/flashdreams-integrations @NVIDIA/flashdreams-maintainers + +# --------------------------------------------------------------------- +# Licensing, attribution, and security policy +# +# These files touch our Apache-2.0 obligations and our coordinated +# disclosure posture. They require the security/licensing team in +# addition to maintainers. +# --------------------------------------------------------------------- +/LICENSE @NVIDIA/flashdreams-maintainers @NVIDIA/flashdreams-security +/LICENSES/ @NVIDIA/flashdreams-maintainers @NVIDIA/flashdreams-security +/NOTICE @NVIDIA/flashdreams-maintainers @NVIDIA/flashdreams-security +/reuse.toml @NVIDIA/flashdreams-maintainers @NVIDIA/flashdreams-security +/CONTRIBUTING.md @NVIDIA/flashdreams-maintainers +/README.md @NVIDIA/flashdreams-maintainers diff --git a/docs/code_review.md b/docs/code_review.md new file mode 100644 index 000000000..388cf3fc5 --- /dev/null +++ b/docs/code_review.md @@ -0,0 +1,302 @@ + + +# Code review and ownership + +Welcome — and thank you for sending a pull request to FlashDreams. +This document explains *who* approves changes in each part of the +repository, *what* the project's "+2 Code Review" gate actually is, +and *how* the policy fits alongside the project's broader contributor +guidance. + +It is a companion to [`CONTRIBUTING.md`](../CONTRIBUTING.md), not a +replacement. If you have not read CONTRIBUTING yet, start there for +the fork-and-PR mechanics, DCO sign-off, testing markers, and the +SPDX header convention. This document picks up where CONTRIBUTING's +[*Code review and merge*](../CONTRIBUTING.md#code-review-and-merge) +section leaves off. + +## At a glance + +- Every PR to `main` requires approval from a **qualified "+2" + reviewer** for the touched paths. "+2" describes the reviewer's + qualification level, not a head-count — one qualified +2 approval + is the bar, not two separate +1 approvals. +- The +2 reviewers for each subsystem are listed in + [`/.github/CODEOWNERS`](../.github/CODEOWNERS). The sections below + are a human-readable map of the same file. +- Reviewers apply the [+2 Security Review Checklist](#2-security-review-checklist) + at the bottom of this document on every review. +- For security-sensitive paths, the project's Security PIC (or + delegate from `@NVIDIA/flashdreams-security`) signs off in addition + to the subsystem CODEOWNER. +- Outside contributors are welcome everywhere. CODEOWNERS controls + who must *approve* a change, not who may *open* one. +- The +2 gate cannot be waived. Any operational exception requires a + documented audit trail approved by the Security PIC. + +## What "+2 Code Review" means here + +In some review systems "+2" means two separate +1 approvals. In the +FlashDreams policy it means a **single approval from a reviewer +qualified at the +2 level**: + +1. **Qualified reviewer.** CODEOWNERS membership is the project's + ledger of qualified +2 reviewers. Reviewers are trained as required + by NVIDIA PLC before being added. +2. **Checklist on every review.** Qualified reviewers run the + [+2 Security Review Checklist](#2-security-review-checklist) below + on every PR — it's a short, stable list, not a fresh checklist per + PR. +3. **Authenticated identity.** Approvals are tied to authenticated + GitHub accounts; shared accounts are not permitted. + +Mechanically on GitHub, the gate is implemented as branch protection +on `main`: + +- **Require pull request reviews before merging** — enabled. +- **Require review from Code Owners** — enabled. (This is the +2 + enforcement: only a qualified reviewer can satisfy it.) +- **Dismiss stale approvals when new commits are pushed** — enabled. +- **Restrict who can push to matching branches** — enabled; direct + pushes are not allowed. +- **Do not allow bypassing the above settings** — enabled, including + for administrators. + +The CI checks documented in +[`CONTRIBUTING.md` → Testing](../CONTRIBUTING.md#testing) must also +be green, and every commit must carry a DCO sign-off per +[`CONTRIBUTING.md` → DCO](../CONTRIBUTING.md#developer-certificate-of-origin-dco). +Together these four — qualified +2 approval, green CI, DCO sign-off, +and no protected-branch bypass — form the PLC release-readiness gate +for FlashDreams. + +## Why this matters + +The +2 review gate exists for four reasons, in order of practical +impact: + +- **Correctness.** A second pair of qualified eyes catches the + mistakes tests don't: a misplaced `assert`, a config default that + changes behaviour on a different GPU, a logging line that leaks + something it shouldn't. +- **Security.** The standing checklist makes common vulnerability + classes harder to ship by accident. +- **Shared ownership.** Each subsystem has named maintainers who + understand its trade-offs, so changes don't fall into review limbo + and aren't merged by people unfamiliar with the area. +- **Auditability.** Authenticated approvals on a protected branch + give the project a clean audit trail, which matters for shipping + FlashDreams as a supported NVIDIA product alongside the public OSS + release. + +We have kept the policy simple enough that it does not get in a +community contributor's way: contributors open PRs as normal, and the +qualified reviewer comes from CODEOWNERS automatically. + +## How ownership is organised + +GitHub honours exactly one CODEOWNERS file per repository, so all of +the rules live in [`/.github/CODEOWNERS`](../.github/CODEOWNERS). To +keep that file readable, it is divided into **sections by subsystem**, +with each section behaving like its own delegated CODEOWNERS for the +subtree it covers. The table below mirrors that structure so you can +find the right reviewer without reading the whole file. + +### Subsystems + +| Area | Paths | Qualified +2 reviewers | +|------|-------|------------------------| +| **Core library** | `flashdreams/flashdreams/core/`, `plugins/` | `@NVIDIA/flashdreams-core` | +| **Infra** | `flashdreams/flashdreams/infra/` | `@NVIDIA/flashdreams-infra` | +| **Recipes & configs** | `flashdreams/flashdreams/recipes/`, `configs/` | `@NVIDIA/flashdreams-recipes` | +| **Tests & pytest plugins** | `flashdreams/tests/`, `_pytest_plugins/` | `@NVIDIA/flashdreams-maintainers` | +| **Integrations** | `integrations//` | `@NVIDIA/flashdreams-integrations` + per-recipe owners | +| **Documentation** | `docs/` | `@NVIDIA/flashdreams-docs` | +| **Container & CI** | `docker/`, `.github/` | `@NVIDIA/flashdreams-ci` | +| **Workspace metadata** | `pyproject.toml`, `uv.lock`, per-package `pyproject.toml` | `@NVIDIA/flashdreams-maintainers` | +| **Licensing & security** | `LICENSE`, `LICENSES/`, `NOTICE`, `reuse.toml`, `CODEOWNERS` itself, `docs/code_review.md` | `@NVIDIA/flashdreams-maintainers` + `@NVIDIA/flashdreams-security` | +| **Fallback (anything else)** | `*` | `@NVIDIA/flashdreams-maintainers` | + +Each integration under `integrations/` additionally has a per-recipe +owners team (for example, `@NVIDIA/flashdreams-cosmos-owners` for +`integrations/cosmos_predict2/`). The integrations team is added as a +backstop so no recipe is orphaned if its lead is unavailable. + +### Reading the file + +CODEOWNERS rules are applied in order, with the **last matching line +winning**. A more specific path overrides a more general one — for +instance, `integrations/cosmos_predict2/` overrides the catch-all +`integrations/` line above it. If you are unsure who owns a file: + +```bash +# Validate the CODEOWNERS file itself +gh api repos/NVIDIA/flashdreams/codeowners/errors + +# Inspect recent reviewers for a path +git log -1 --format=%H -- +``` + +When in doubt, open the PR and request a review from +`@NVIDIA/flashdreams-maintainers`; they will route it to the right +qualified reviewer. + +## Security-sensitive paths + +A few paths require an additional approval from +`@NVIDIA/flashdreams-security` on top of the subsystem CODEOWNER: + +- `LICENSE`, `LICENSES/`, `NOTICE`, `reuse.toml` — Apache-2.0 + attribution and third-party license bookkeeping. +- `.github/CODEOWNERS` itself — changes to who can approve what. +- `docs/code_review.md` (this document) — changes to the review + policy. + +Mistakes in those files are unusually expensive to roll back, so the +project's Security PIC (or a delegate from the security team) signs +off on them as a matter of routine. + +For **vulnerability reports**, do *not* open a public issue or PR. +Follow NVIDIA's coordinated disclosure process at +, as described in +[`CONTRIBUTING.md` → Filing issues and security reports](../CONTRIBUTING.md#filing-issues-and-security-reports). + +## What this looks like from a contributor's perspective + +You don't need to think about most of this when opening a PR: + +1. Fork, branch, code, sign off, push, open a PR — same flow as in + [`CONTRIBUTING.md` → Submitting a pull request](../CONTRIBUTING.md#submitting-a-pull-request). +2. GitHub automatically tags the right CODEOWNERS based on the paths + you touched. They appear in the PR sidebar under "Reviewers + requested by Code owners". +3. A qualified +2 reviewer works through the checklist below and + approves; if you touched a security-sensitive path above, a + security reviewer also approves. +4. The squash-merge button lights up, and a maintainer presses it. + +If your PR spans multiple subsystems (say, a core change that also +updates an integration), you will see review requests from each +affected CODEOWNERS team. That is intentional: cross-subsystem +changes benefit from a qualified reviewer in each area. + +We aim for first review within two business days; if a PR is quieter +than that, please leave a friendly ping comment. + +## Becoming a +2 reviewer + +CODEOWNERS membership is how the project formalises long-term +ownership of a subsystem, and it's the main mechanism by which +governance opens up over time +(see [`CONTRIBUTING.md` → Project governance](../CONTRIBUTING.md#project-governance)). +Contributors — NVIDIA employee or not — who consistently land +high-quality work in an area, participate in reviews, and engage with +the issue tracker can be invited onto the relevant CODEOWNERS team +once any required NVIDIA PLC training is complete. + +There is no fixed time bar; we look for sustained good judgment about +when to ship, when to push back, and when to ask for help. If you'd +like to grow into that role, please say so in a Discussion or to any +maintainer — we'd rather hear it than not. + +## No bypass, no waiver + +The +2 gate is not waivable. Branch protection on `main` is +configured so that: + +- Administrators are *not* allowed to bypass review or status checks. +- Direct pushes to `main` are blocked. +- Force-pushes to `main` are blocked. + +If an operational situation genuinely requires an exception — for +example, an emergency security patch where the usual reviewer is +unavailable — the exception must be documented as an issue or +incident record, approved by the Security PIC in writing, and the +audit trail retained. We have not needed to use this path, and we +would prefer to keep it that way. + +## +2 Security Review Checklist + +Reviewers — human or LLM agent — run this checklist on every PR +before approving. It is the standing checklist from NVIDIA's *PLC: +Code Review Excellence* training; the items below are the literal +questions a +2 reviewer is expected to answer. + +**How to use the table.** For each item, set **Status** to one of: + +- `Pass` — the change clearly satisfies the item. +- `Fail` — the change clearly violates the item; block until fixed. +- `N/A` — the item does not apply to this PR (e.g. crypto items on a + pure-docs change). Explain why in the *Evidence / finding* column. +- `Human review` — the item requires judgment beyond what an LLM + agent should make alone; flag it for the human +2 reviewer. + +**Evidence / finding** should cite the relevant `path/to/file.py:LN` +or quote the specific lines or commit hash that justify the status. +For `Pass` on a non-trivial item, briefly say *why* (one short +sentence) rather than just `Pass`. LLM agents should err toward +`Human review` whenever they are uncertain, and should never mark an +item `Pass` without concrete evidence. + +| # | Category | Item | Question | Status | Evidence / finding | +|---|----------|------|----------|--------|--------------------| +| 1 | Process & Static Analysis | SCA Clean | Have all critical and high-severity issues identified by Static Code Analysis (SCA) tools been resolved? | | | +| 2 | Process & Static Analysis | Risk Assessed | Has the code been evaluated against the product's Threat and Vulnerability Analysis (TAVA)? | | | +| 3 | Input Validation | Trust Boundaries | Is all input crossing the trust boundary strictly validated? | | | +| 4 | Input Validation | Length & Range | Are the lengths and ranges of the data explicitly checked? | | | +| 5 | Input Validation | Failures | Does any input validation failure result in an error? | | | +| 6 | Safe Function Usage | C/C++ APIs | Have unsafe functions been replaced with secure alternatives (e.g. `snprintf` vs `sprintf`, `strlcpy` vs `strcpy`, `fgets` vs `gets`)? | | | +| 7 | Safe Function Usage | Python APIs | Are unsafe executions avoided (e.g. `literal_eval` vs `eval`, `shell=False` in `subprocess`)? | | | +| 8 | Variable Management | Initialization | Are all variables initialized before use with a deny-by-default approach? | | | +| 9 | Variable Management | Typing | Are unsigned types used unless negative values are required? | | | +| 10 | Variable Management | Scope | Are variables scoped minimally and not reused improperly? | | | +| 11 | Compilation & Resilience | Compiler Flags | Are strict security compiler flags enabled (e.g. `-Werror`, `-Wall`, `-fstack-protector-strong`)? | | | +| 12 | Compilation & Resilience | Fault Injection (if applicable) | Are mitigations in place for critical low-level code (e.g. redundancy, glitch resistance)? | | | +| 13 | Error Handling | Action Taken | Are errors properly handled and propagated (logging alone is insufficient)? | | | +| 14 | Error Handling | No Leakage | Do error flows prevent exposure of sensitive data (stack traces, memory, secrets)? | | | +| 15 | Error Handling | Resource Cleanup | Are resources properly freed during error handling? | | | +| 16 | Cryptography & Secrets | No Hardcoded Secrets | Is the code free of plaintext secrets (API keys, passwords, tokens, internal IPs)? | | | +| 17 | Cryptography & Secrets | Strong Crypto | Are only modern, vetted cryptographic algorithms used (e.g. CNSA Suite 2.0)? | | | +| 18 | Cryptography & Secrets | Authentication First | Is data authenticated before decryption? | | | +| 19 | Cryptography & Secrets | Safe Randomness | Are cryptographically secure RNGs used (e.g. `/dev/urandom` vs `rand()`)? | | | +| 20 | Access Control & Concurrency | Least Privilege | Are access controls enforced using allowlists vs blocklists? | | | +| 21 | Access Control & Concurrency | Shared Resources | Are shared resources protected against DoS or privilege abuse? | | | +| 22 | Access Control & Concurrency | Race Conditions | Is the code protected against TOCTOU vulnerabilities? | | | +| 23 | Access Control & Concurrency | No Backdoors | Are there no intentional or accidental bypass mechanisms? | | | + +The wording of the items above is reproduced from NVIDIA's *PLC: +Code Review Excellence* training; it is the authoritative version, +and changes to it should track the upstream training rather than +diverging here. + +## Changing this document or the CODEOWNERS file + +Both this file and `.github/CODEOWNERS` are themselves owned by +`@NVIDIA/flashdreams-maintainers` and `@NVIDIA/flashdreams-security`. +If you want to propose a change — new subsystem owners, a different +ownership boundary, a refinement to the +2 policy — please open an +issue or Discussion first so we can talk through it before the PR. +The goal is for the review policy to be predictable; we change it +deliberately rather than incrementally. + +--- + +Thanks for reading this far. The project is healthier when reviewers +and contributors share a clear picture of how decisions get made; if +anything above is unclear, that's a bug in the documentation and +we'd appreciate the issue. From 6533fa0a25b810dd6ee7809120abef36e6208df6 Mon Sep 17 00:00:00 2001 From: Jonathan McCaffrey Date: Wed, 20 May 2026 18:36:02 +0000 Subject: [PATCH 2/5] docs: simplify CODEOWNERS to three teams MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Collapses the team layout down to: - @NVIDIA/flashdreams-maintainers (default for everything not below) - @NVIDIA/flashdreams-integrations (integrations/) - @NVIDIA/flashdreams-security (second approver on licensing, dependency manifests, and review-policy files) Removes per-recipe owner sub-teams (alpadreams-owners, cosmos-owners, wan-owners, lingbot-owners, causal-forcing-owners, ludus-renderer- owners) and per-subdir routing for core / infra / recipes / plugins / configs / tests / docs / CI — all fall through to maintainers. Individual names can be appended inline in CODEOWNERS later if a path warrants it. Co-Authored-By: Claude Opus 4.7 (1M context) Signed-off-by: Jonathan McCaffrey --- .github/CODEOWNERS | 108 ++++++++++++++------------------------------ docs/code_review.md | 27 +++++------ 2 files changed, 47 insertions(+), 88 deletions(-) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 718178e60..3c3ef73e0 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -1,102 +1,64 @@ # FlashDreams CODEOWNERS # -# This file declares the GitHub teams responsible for approving changes -# in each part of the repository. It is the source of truth for code -# review responsibility, as described in CONTRIBUTING.md and in -# docs/code_review.md. +# Declares the GitHub teams responsible for approving changes in each +# part of the repository. Source of truth for code review +# responsibility, per CONTRIBUTING.md and docs/code_review.md. # # Format reference: https://docs.github.com/en/repositories/managing-your-repositories-settings-and-security/managing-repository-settings/about-code-owners # +# Teams (kept intentionally small): +# @NVIDIA/flashdreams-maintainers — default owners; cover core, +# infra, recipes, plugins, configs, +# tests, CI, docs, container, and +# workspace metadata. +# @NVIDIA/flashdreams-integrations — per-recipe maintainers under +# integrations/. +# @NVIDIA/flashdreams-security — additional sign-off on licensing +# and dependency-manifest files. +# # Notes: -# * GitHub only honours ONE CODEOWNERS file per repository. This is -# that file. "Delegation" between subsystems is expressed by -# path-prefix rules below — each section behaves like its own -# CODEOWNERS for the subtree it covers. -# * The last matching rule wins, so more specific paths appear below -# more general ones. +# * GitHub honours one CODEOWNERS file per repository; this is it. +# "Delegation" between subsystems is expressed by path-prefix rules +# below. The last matching rule wins, so more specific paths appear +# below more general ones. # * Owners listed here are the project's qualified "+2" reviewers. # They apply the +2 Security Review Checklist on every review (see # docs/code_review.md) and are trained as required by NVIDIA PLC. -# Branch protection on `main` requires at least one approving -# review from a CODEOWNER for the touched paths. On -# security-sensitive paths, the `@NVIDIA/flashdreams-security` -# team's sign-off is additionally required. +# Branch protection on `main` requires at least one approving review +# from a CODEOWNER for the touched paths. On security-sensitive +# paths, the security team's sign-off is additionally required. # * External contributors are welcome on every path. CODEOWNERS does # not gate who may *open* a PR — only who must *approve* it. # * The PLC gate cannot be bypassed or waived; any exception requires # a documented audit trail approved by the Security PIC. # --------------------------------------------------------------------- -# Default owners (fallback for any path not matched below) +# Default owners # --------------------------------------------------------------------- * @NVIDIA/flashdreams-maintainers # --------------------------------------------------------------------- -# Core library — flashdreams/ -# --------------------------------------------------------------------- -/flashdreams/ @NVIDIA/flashdreams-maintainers -/flashdreams/flashdreams/core/ @NVIDIA/flashdreams-core -/flashdreams/flashdreams/infra/ @NVIDIA/flashdreams-infra -/flashdreams/flashdreams/recipes/ @NVIDIA/flashdreams-recipes -/flashdreams/flashdreams/plugins/ @NVIDIA/flashdreams-core -/flashdreams/flashdreams/configs/ @NVIDIA/flashdreams-recipes -/flashdreams/flashdreams/_pytest_plugins/ @NVIDIA/flashdreams-maintainers -/flashdreams/tests/ @NVIDIA/flashdreams-maintainers - -# --------------------------------------------------------------------- -# Integrations — integrations// -# -# Each integration is a workspace member with its own pyproject.toml. -# The integration's lead reviewers own their subdirectory; the -# integrations team is added as a backstop so no integration is -# orphaned if its lead is unavailable. +# Integrations # --------------------------------------------------------------------- /integrations/ @NVIDIA/flashdreams-integrations -/integrations/alpadreams/ @NVIDIA/flashdreams-integrations @NVIDIA/flashdreams-alpadreams-owners -/integrations/alpadreams/ludus-renderer/ @NVIDIA/flashdreams-integrations @NVIDIA/flashdreams-ludus-renderer-owners -/integrations/causal_forcing/ @NVIDIA/flashdreams-integrations @NVIDIA/flashdreams-causal-forcing-owners -/integrations/cosmos_predict2/ @NVIDIA/flashdreams-integrations @NVIDIA/flashdreams-cosmos-owners -/integrations/fastvideo_causal_wan22/ @NVIDIA/flashdreams-integrations @NVIDIA/flashdreams-wan-owners -/integrations/lingbot/ @NVIDIA/flashdreams-integrations @NVIDIA/flashdreams-lingbot-owners -/integrations/self_forcing/ @NVIDIA/flashdreams-integrations @NVIDIA/flashdreams-causal-forcing-owners -/integrations/wan21/ @NVIDIA/flashdreams-integrations @NVIDIA/flashdreams-wan-owners - -# --------------------------------------------------------------------- -# Documentation — docs/ -# --------------------------------------------------------------------- -/docs/ @NVIDIA/flashdreams-docs -/docs/source/apis/ @NVIDIA/flashdreams-docs @NVIDIA/flashdreams-maintainers -/docs/code_review.md @NVIDIA/flashdreams-maintainers @NVIDIA/flashdreams-security - -# --------------------------------------------------------------------- -# Build, container, and CI plumbing -# --------------------------------------------------------------------- -/docker/ @NVIDIA/flashdreams-infra @NVIDIA/flashdreams-ci -/.github/ @NVIDIA/flashdreams-ci -/.github/workflows/ @NVIDIA/flashdreams-ci @NVIDIA/flashdreams-maintainers -/.github/CODEOWNERS @NVIDIA/flashdreams-maintainers @NVIDIA/flashdreams-security - -# --------------------------------------------------------------------- -# Workspace / dependency / packaging metadata -# -# Changes here ripple across every subsystem, so they require sign-off -# from the maintainers team. -# --------------------------------------------------------------------- -/pyproject.toml @NVIDIA/flashdreams-maintainers -/uv.lock @NVIDIA/flashdreams-maintainers -/flashdreams/pyproject.toml @NVIDIA/flashdreams-maintainers -/integrations/*/pyproject.toml @NVIDIA/flashdreams-integrations @NVIDIA/flashdreams-maintainers # --------------------------------------------------------------------- -# Licensing, attribution, and security policy +# Licensing, attribution, and dependency manifests # -# These files touch our Apache-2.0 obligations and our coordinated -# disclosure posture. They require the security/licensing team in -# addition to maintainers. +# Security review is required in addition to maintainers because these +# files touch Apache-2.0 obligations and the dependency / SBOM surface. # --------------------------------------------------------------------- /LICENSE @NVIDIA/flashdreams-maintainers @NVIDIA/flashdreams-security /LICENSES/ @NVIDIA/flashdreams-maintainers @NVIDIA/flashdreams-security /NOTICE @NVIDIA/flashdreams-maintainers @NVIDIA/flashdreams-security /reuse.toml @NVIDIA/flashdreams-maintainers @NVIDIA/flashdreams-security -/CONTRIBUTING.md @NVIDIA/flashdreams-maintainers -/README.md @NVIDIA/flashdreams-maintainers +/pyproject.toml @NVIDIA/flashdreams-maintainers @NVIDIA/flashdreams-security +/uv.lock @NVIDIA/flashdreams-maintainers @NVIDIA/flashdreams-security +/flashdreams/pyproject.toml @NVIDIA/flashdreams-maintainers @NVIDIA/flashdreams-security +/integrations/*/pyproject.toml @NVIDIA/flashdreams-integrations @NVIDIA/flashdreams-security + +# --------------------------------------------------------------------- +# Review-policy files themselves +# --------------------------------------------------------------------- +/.github/CODEOWNERS @NVIDIA/flashdreams-maintainers @NVIDIA/flashdreams-security +/docs/code_review.md @NVIDIA/flashdreams-maintainers @NVIDIA/flashdreams-security diff --git a/docs/code_review.md b/docs/code_review.md index 388cf3fc5..36f02c51e 100644 --- a/docs/code_review.md +++ b/docs/code_review.md @@ -119,23 +119,20 @@ find the right reviewer without reading the whole file. ### Subsystems +We keep ownership intentionally lean — three teams cover the whole +repository. + | Area | Paths | Qualified +2 reviewers | |------|-------|------------------------| -| **Core library** | `flashdreams/flashdreams/core/`, `plugins/` | `@NVIDIA/flashdreams-core` | -| **Infra** | `flashdreams/flashdreams/infra/` | `@NVIDIA/flashdreams-infra` | -| **Recipes & configs** | `flashdreams/flashdreams/recipes/`, `configs/` | `@NVIDIA/flashdreams-recipes` | -| **Tests & pytest plugins** | `flashdreams/tests/`, `_pytest_plugins/` | `@NVIDIA/flashdreams-maintainers` | -| **Integrations** | `integrations//` | `@NVIDIA/flashdreams-integrations` + per-recipe owners | -| **Documentation** | `docs/` | `@NVIDIA/flashdreams-docs` | -| **Container & CI** | `docker/`, `.github/` | `@NVIDIA/flashdreams-ci` | -| **Workspace metadata** | `pyproject.toml`, `uv.lock`, per-package `pyproject.toml` | `@NVIDIA/flashdreams-maintainers` | -| **Licensing & security** | `LICENSE`, `LICENSES/`, `NOTICE`, `reuse.toml`, `CODEOWNERS` itself, `docs/code_review.md` | `@NVIDIA/flashdreams-maintainers` + `@NVIDIA/flashdreams-security` | -| **Fallback (anything else)** | `*` | `@NVIDIA/flashdreams-maintainers` | - -Each integration under `integrations/` additionally has a per-recipe -owners team (for example, `@NVIDIA/flashdreams-cosmos-owners` for -`integrations/cosmos_predict2/`). The integrations team is added as a -backstop so no recipe is orphaned if its lead is unavailable. +| **Core, infra, recipes, plugins, configs, tests, docs, CI, container, workspace metadata** | everything not listed below | `@NVIDIA/flashdreams-maintainers` | +| **Integrations** | `integrations/` (all recipes) | `@NVIDIA/flashdreams-integrations` | +| **Licensing & dependency manifests** (additional reviewer) | `LICENSE`, `LICENSES/`, `NOTICE`, `reuse.toml`, `pyproject.toml`, `uv.lock`, per-package `pyproject.toml` | `@NVIDIA/flashdreams-security` (in addition to maintainers / integrations) | +| **Review-policy files** (additional reviewer) | `.github/CODEOWNERS`, `docs/code_review.md` | `@NVIDIA/flashdreams-security` (in addition to maintainers) | + +`@NVIDIA/flashdreams-maintainers` is the default owner for any path not +explicitly matched. We can split out additional teams later if any +subsystem grows enough to deserve its own ownership boundary, but the +current shape keeps the review surface small and easy to staff. ### Reading the file From 0081e46becef1b83b8a625ef491c0bacb20c4faf Mon Sep 17 00:00:00 2001 From: Jonathan McCaffrey Date: Wed, 20 May 2026 18:43:13 +0000 Subject: [PATCH 3/5] docs: reframe code_review.md around CODEOWNERS, refine ownership - Adds @NVIDIA/flashdreams-maintainers as an alternate owner on integrations/ so PRs are never blocked waiting on the integrations team alone. - Narrows LICENSE, reuse.toml, uv.lock, and flashdreams/pyproject.toml to @NVIDIA/flashdreams-security as sole approver. - Reframes docs/code_review.md to lead with CODEOWNERS-based community review (the GitHub Approve button + branch protection). The NVIDIA PLC "+2 Code Review" compliance requirement is now mentioned once, in a dedicated compliance-note section, rather than threading through the document. Co-Authored-By: Claude Opus 4.7 (1M context) Signed-off-by: Jonathan McCaffrey --- .github/CODEOWNERS | 39 ++++--- docs/code_review.md | 268 +++++++++++++++++++++----------------------- 2 files changed, 153 insertions(+), 154 deletions(-) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 3c3ef73e0..2dbd92a06 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -12,21 +12,25 @@ # tests, CI, docs, container, and # workspace metadata. # @NVIDIA/flashdreams-integrations — per-recipe maintainers under -# integrations/. -# @NVIDIA/flashdreams-security — additional sign-off on licensing -# and dependency-manifest files. +# integrations/. Maintainers can +# also review here. +# @NVIDIA/flashdreams-security — sole or additional approver on +# licensing and dependency-manifest +# files. # # Notes: # * GitHub honours one CODEOWNERS file per repository; this is it. # "Delegation" between subsystems is expressed by path-prefix rules # below. The last matching rule wins, so more specific paths appear # below more general ones. +# * When multiple owners are listed on a single rule, GitHub treats +# them as alternatives — any *one* of them satisfies the required +# review. # * Owners listed here are the project's qualified "+2" reviewers. # They apply the +2 Security Review Checklist on every review (see # docs/code_review.md) and are trained as required by NVIDIA PLC. # Branch protection on `main` requires at least one approving review -# from a CODEOWNER for the touched paths. On security-sensitive -# paths, the security team's sign-off is additionally required. +# from a CODEOWNER for the touched paths. # * External contributors are welcome on every path. CODEOWNERS does # not gate who may *open* a PR — only who must *approve* it. # * The PLC gate cannot be bypassed or waived; any exception requires @@ -39,22 +43,31 @@ # --------------------------------------------------------------------- # Integrations +# +# Maintainers can also review here so an integration PR is never +# blocked waiting on the integrations team alone. +# --------------------------------------------------------------------- +/integrations/ @NVIDIA/flashdreams-integrations @NVIDIA/flashdreams-maintainers + +# --------------------------------------------------------------------- +# Security-sensitive files +# +# These touch Apache-2.0 attribution and the dependency / SBOM surface, +# and are owned by the security team as sole approver. # --------------------------------------------------------------------- -/integrations/ @NVIDIA/flashdreams-integrations +/LICENSE @NVIDIA/flashdreams-security +/reuse.toml @NVIDIA/flashdreams-security +/uv.lock @NVIDIA/flashdreams-security +/flashdreams/pyproject.toml @NVIDIA/flashdreams-security # --------------------------------------------------------------------- -# Licensing, attribution, and dependency manifests +# Licensing & dependency-manifest files with additional security review # -# Security review is required in addition to maintainers because these -# files touch Apache-2.0 obligations and the dependency / SBOM surface. +# These remain maintainer-owned but require security sign-off too. # --------------------------------------------------------------------- -/LICENSE @NVIDIA/flashdreams-maintainers @NVIDIA/flashdreams-security /LICENSES/ @NVIDIA/flashdreams-maintainers @NVIDIA/flashdreams-security /NOTICE @NVIDIA/flashdreams-maintainers @NVIDIA/flashdreams-security -/reuse.toml @NVIDIA/flashdreams-maintainers @NVIDIA/flashdreams-security /pyproject.toml @NVIDIA/flashdreams-maintainers @NVIDIA/flashdreams-security -/uv.lock @NVIDIA/flashdreams-maintainers @NVIDIA/flashdreams-security -/flashdreams/pyproject.toml @NVIDIA/flashdreams-maintainers @NVIDIA/flashdreams-security /integrations/*/pyproject.toml @NVIDIA/flashdreams-integrations @NVIDIA/flashdreams-security # --------------------------------------------------------------------- diff --git a/docs/code_review.md b/docs/code_review.md index 36f02c51e..d53880e48 100644 --- a/docs/code_review.md +++ b/docs/code_review.md @@ -19,9 +19,8 @@ limitations under the License. Welcome — and thank you for sending a pull request to FlashDreams. This document explains *who* approves changes in each part of the -repository, *what* the project's "+2 Code Review" gate actually is, -and *how* the policy fits alongside the project's broader contributor -guidance. +repository, *how* code review works in practice, and *what* a +contributor or reviewer can expect day-to-day. It is a companion to [`CONTRIBUTING.md`](../CONTRIBUTING.md), not a replacement. If you have not read CONTRIBUTING yet, start there for @@ -32,114 +31,97 @@ section leaves off. ## At a glance -- Every PR to `main` requires approval from a **qualified "+2" - reviewer** for the touched paths. "+2" describes the reviewer's - qualification level, not a head-count — one qualified +2 approval - is the bar, not two separate +1 approvals. -- The +2 reviewers for each subsystem are listed in - [`/.github/CODEOWNERS`](../.github/CODEOWNERS). The sections below - are a human-readable map of the same file. -- Reviewers apply the [+2 Security Review Checklist](#2-security-review-checklist) +- Every PR to `main` requires approval from a **code owner** for the + touched paths. The code owners for each area of the repository are + listed in [`/.github/CODEOWNERS`](../.github/CODEOWNERS). +- A single approval from a code owner is the bar. GitHub treats the + teams listed on a CODEOWNERS line as alternatives — *any one* of + them can approve. +- Code owners run the [security review checklist](#security-review-checklist) at the bottom of this document on every review. -- For security-sensitive paths, the project's Security PIC (or - delegate from `@NVIDIA/flashdreams-security`) signs off in addition - to the subsystem CODEOWNER. - Outside contributors are welcome everywhere. CODEOWNERS controls who must *approve* a change, not who may *open* one. -- The +2 gate cannot be waived. Any operational exception requires a - documented audit trail approved by the Security PIC. - -## What "+2 Code Review" means here - -In some review systems "+2" means two separate +1 approvals. In the -FlashDreams policy it means a **single approval from a reviewer -qualified at the +2 level**: - -1. **Qualified reviewer.** CODEOWNERS membership is the project's - ledger of qualified +2 reviewers. Reviewers are trained as required - by NVIDIA PLC before being added. -2. **Checklist on every review.** Qualified reviewers run the - [+2 Security Review Checklist](#2-security-review-checklist) below - on every PR — it's a short, stable list, not a fresh checklist per - PR. -3. **Authenticated identity.** Approvals are tied to authenticated - GitHub accounts; shared accounts are not permitted. - -Mechanically on GitHub, the gate is implemented as branch protection -on `main`: +- The review requirement cannot be bypassed; any operational + exception requires a documented audit trail. + +## How review and merge work on GitHub + +There is no special button for code-owner approval — it's the +standard GitHub PR flow: + +1. **A contributor opens a pull request.** GitHub reads + [`.github/CODEOWNERS`](../.github/CODEOWNERS), computes which + teams own the paths touched, and adds them to the *Reviewers* + sidebar under "Reviewers requested by Code owners". +2. **A code owner clicks "Review changes → Approve"** in the GitHub + UI (or runs `gh pr review --approve` from the CLI). That single + approval, from a member of a CODEOWNERS team for the touched + paths, satisfies the "Require review from Code Owners" branch + protection check. +3. **CI and DCO must also be green** — formatting, type-checks, CI + tier markers per [`CONTRIBUTING.md` → Testing](../CONTRIBUTING.md#testing), + and a `Signed-off-by` trailer on every commit per + [`CONTRIBUTING.md` → DCO](../CONTRIBUTING.md#developer-certificate-of-origin-dco). +4. **A maintainer squash-merges.** The PR title and description + become the squash commit message. + +A non-CODEOWNER approval is welcome — review feedback from anyone +helps — but on its own it does not satisfy the protected-branch +check. + +Branch protection on `main` is configured so that: - **Require pull request reviews before merging** — enabled. -- **Require review from Code Owners** — enabled. (This is the +2 - enforcement: only a qualified reviewer can satisfy it.) +- **Require review from Code Owners** — enabled. - **Dismiss stale approvals when new commits are pushed** — enabled. - **Restrict who can push to matching branches** — enabled; direct pushes are not allowed. - **Do not allow bypassing the above settings** — enabled, including for administrators. -The CI checks documented in -[`CONTRIBUTING.md` → Testing](../CONTRIBUTING.md#testing) must also -be green, and every commit must carry a DCO sign-off per -[`CONTRIBUTING.md` → DCO](../CONTRIBUTING.md#developer-certificate-of-origin-dco). -Together these four — qualified +2 approval, green CI, DCO sign-off, -and no protected-branch bypass — form the PLC release-readiness gate -for FlashDreams. - -## Why this matters - -The +2 review gate exists for four reasons, in order of practical -impact: - -- **Correctness.** A second pair of qualified eyes catches the - mistakes tests don't: a misplaced `assert`, a config default that - changes behaviour on a different GPU, a logging line that leaks - something it shouldn't. -- **Security.** The standing checklist makes common vulnerability - classes harder to ship by accident. -- **Shared ownership.** Each subsystem has named maintainers who - understand its trade-offs, so changes don't fall into review limbo - and aren't merged by people unfamiliar with the area. -- **Auditability.** Authenticated approvals on a protected branch - give the project a clean audit trail, which matters for shipping - FlashDreams as a supported NVIDIA product alongside the public OSS - release. - -We have kept the policy simple enough that it does not get in a -community contributor's way: contributors open PRs as normal, and the -qualified reviewer comes from CODEOWNERS automatically. - ## How ownership is organised GitHub honours exactly one CODEOWNERS file per repository, so all of -the rules live in [`/.github/CODEOWNERS`](../.github/CODEOWNERS). To -keep that file readable, it is divided into **sections by subsystem**, -with each section behaving like its own delegated CODEOWNERS for the -subtree it covers. The table below mirrors that structure so you can -find the right reviewer without reading the whole file. +the rules live in [`/.github/CODEOWNERS`](../.github/CODEOWNERS). The +file is divided into sections by subsystem, with each section +behaving like its own delegated CODEOWNERS for the subtree it +covers. The table below mirrors that structure so you can find the +right reviewer without reading the whole file. ### Subsystems We keep ownership intentionally lean — three teams cover the whole repository. -| Area | Paths | Qualified +2 reviewers | -|------|-------|------------------------| -| **Core, infra, recipes, plugins, configs, tests, docs, CI, container, workspace metadata** | everything not listed below | `@NVIDIA/flashdreams-maintainers` | -| **Integrations** | `integrations/` (all recipes) | `@NVIDIA/flashdreams-integrations` | -| **Licensing & dependency manifests** (additional reviewer) | `LICENSE`, `LICENSES/`, `NOTICE`, `reuse.toml`, `pyproject.toml`, `uv.lock`, per-package `pyproject.toml` | `@NVIDIA/flashdreams-security` (in addition to maintainers / integrations) | -| **Review-policy files** (additional reviewer) | `.github/CODEOWNERS`, `docs/code_review.md` | `@NVIDIA/flashdreams-security` (in addition to maintainers) | - -`@NVIDIA/flashdreams-maintainers` is the default owner for any path not -explicitly matched. We can split out additional teams later if any -subsystem grows enough to deserve its own ownership boundary, but the -current shape keeps the review surface small and easy to staff. +| Area | Paths | Code owners | +|------|-------|-------------| +| **Default** (core, infra, recipes, plugins, configs, tests, docs, CI, container) | everything not listed below | `@NVIDIA/flashdreams-maintainers` | +| **Integrations** | `integrations/` (all recipes) | `@NVIDIA/flashdreams-integrations` *or* `@NVIDIA/flashdreams-maintainers` | +| **Security-sensitive files** | `LICENSE`, `reuse.toml`, `uv.lock`, `flashdreams/pyproject.toml` | `@NVIDIA/flashdreams-security` (sole owner) | +| **Licensing & dependency manifests with security review** | `LICENSES/`, `NOTICE`, root `pyproject.toml` | `@NVIDIA/flashdreams-maintainers` *or* `@NVIDIA/flashdreams-security` | +| **Per-integration packaging** | `integrations/*/pyproject.toml` | `@NVIDIA/flashdreams-integrations` *or* `@NVIDIA/flashdreams-security` | +| **Review-policy files** | `.github/CODEOWNERS`, `docs/code_review.md` | `@NVIDIA/flashdreams-maintainers` *or* `@NVIDIA/flashdreams-security` | + +`@NVIDIA/flashdreams-maintainers` is the default owner for any path +not explicitly matched. We can split out additional teams later if +any subsystem grows enough to deserve its own ownership boundary, +but the current shape keeps the review surface small and easy to +staff. + +**A note on "or" in the table above.** When CODEOWNERS lists +multiple owners on the same line, GitHub treats them as +alternatives — approval from *any one* of the listed teams +satisfies the required-review-from-Code-Owners check. If we ever +need a path to require sign-off from *both* the subsystem team +*and* the security team (rather than either), we will raise the +branch-protection "required approvals" count and split the rule +across two lines. ### Reading the file -CODEOWNERS rules are applied in order, with the **last matching line -winning**. A more specific path overrides a more general one — for -instance, `integrations/cosmos_predict2/` overrides the catch-all -`integrations/` line above it. If you are unsure who owns a file: +CODEOWNERS rules are applied in order, with the **last matching +line winning**. A more specific path overrides a more general one. +If you are unsure who owns a file: ```bash # Validate the CODEOWNERS file itself @@ -151,22 +133,23 @@ git log -1 --format=%H -- When in doubt, open the PR and request a review from `@NVIDIA/flashdreams-maintainers`; they will route it to the right -qualified reviewer. +person. ## Security-sensitive paths -A few paths require an additional approval from -`@NVIDIA/flashdreams-security` on top of the subsystem CODEOWNER: +A small set of paths is owned by `@NVIDIA/flashdreams-security`, +either as sole owner or as an additional approver: - `LICENSE`, `LICENSES/`, `NOTICE`, `reuse.toml` — Apache-2.0 attribution and third-party license bookkeeping. -- `.github/CODEOWNERS` itself — changes to who can approve what. -- `docs/code_review.md` (this document) — changes to the review - policy. +- `pyproject.toml`, `flashdreams/pyproject.toml`, + `integrations/*/pyproject.toml`, `uv.lock` — dependency manifests + and lock files. +- `.github/CODEOWNERS` and `docs/code_review.md` — the review + policy itself. -Mistakes in those files are unusually expensive to roll back, so the -project's Security PIC (or a delegate from the security team) signs -off on them as a matter of routine. +Mistakes in these files are unusually expensive to roll back, so the +security team signs off on them as a matter of routine. For **vulnerability reports**, do *not* open a public issue or PR. Follow NVIDIA's coordinated disclosure process at @@ -179,42 +162,38 @@ You don't need to think about most of this when opening a PR: 1. Fork, branch, code, sign off, push, open a PR — same flow as in [`CONTRIBUTING.md` → Submitting a pull request](../CONTRIBUTING.md#submitting-a-pull-request). -2. GitHub automatically tags the right CODEOWNERS based on the paths - you touched. They appear in the PR sidebar under "Reviewers - requested by Code owners". -3. A qualified +2 reviewer works through the checklist below and - approves; if you touched a security-sensitive path above, a - security reviewer also approves. +2. GitHub automatically tags the right code owners based on the + paths you touched. +3. A code owner works through the checklist below and approves. 4. The squash-merge button lights up, and a maintainer presses it. If your PR spans multiple subsystems (say, a core change that also updates an integration), you will see review requests from each affected CODEOWNERS team. That is intentional: cross-subsystem -changes benefit from a qualified reviewer in each area. +changes benefit from a reviewer in each area. -We aim for first review within two business days; if a PR is quieter -than that, please leave a friendly ping comment. +We aim for first review within two business days; if a PR is +quieter than that, please leave a friendly ping comment. -## Becoming a +2 reviewer +## Becoming a code owner CODEOWNERS membership is how the project formalises long-term ownership of a subsystem, and it's the main mechanism by which -governance opens up over time -(see [`CONTRIBUTING.md` → Project governance](../CONTRIBUTING.md#project-governance)). +governance opens up over time (see +[`CONTRIBUTING.md` → Project governance](../CONTRIBUTING.md#project-governance)). Contributors — NVIDIA employee or not — who consistently land -high-quality work in an area, participate in reviews, and engage with -the issue tracker can be invited onto the relevant CODEOWNERS team -once any required NVIDIA PLC training is complete. +high-quality work in an area, participate in reviews, and engage +with the issue tracker can be invited onto the relevant CODEOWNERS +team. -There is no fixed time bar; we look for sustained good judgment about -when to ship, when to push back, and when to ask for help. If you'd -like to grow into that role, please say so in a Discussion or to any -maintainer — we'd rather hear it than not. +There is no fixed time bar; we look for sustained good judgment +about when to ship, when to push back, and when to ask for help. If +you'd like to grow into that role, please say so in a Discussion or +to any maintainer — we'd rather hear it than not. ## No bypass, no waiver -The +2 gate is not waivable. Branch protection on `main` is -configured so that: +Branch protection on `main` is configured so that: - Administrators are *not* allowed to bypass review or status checks. - Direct pushes to `main` are blocked. @@ -223,32 +202,32 @@ configured so that: If an operational situation genuinely requires an exception — for example, an emergency security patch where the usual reviewer is unavailable — the exception must be documented as an issue or -incident record, approved by the Security PIC in writing, and the +incident record, approved by the security team in writing, and the audit trail retained. We have not needed to use this path, and we would prefer to keep it that way. -## +2 Security Review Checklist +## Security review checklist -Reviewers — human or LLM agent — run this checklist on every PR -before approving. It is the standing checklist from NVIDIA's *PLC: -Code Review Excellence* training; the items below are the literal -questions a +2 reviewer is expected to answer. +Code owners — human or LLM agent — run this checklist on every PR +before approving. It covers the most common ways a change can +introduce a security regression. **How to use the table.** For each item, set **Status** to one of: - `Pass` — the change clearly satisfies the item. - `Fail` — the change clearly violates the item; block until fixed. -- `N/A` — the item does not apply to this PR (e.g. crypto items on a - pure-docs change). Explain why in the *Evidence / finding* column. +- `N/A` — the item does not apply to this PR (e.g. crypto items on + a pure-docs change). Explain why in the *Evidence / finding* + column. - `Human review` — the item requires judgment beyond what an LLM - agent should make alone; flag it for the human +2 reviewer. + agent should make alone; flag it for a human code owner. **Evidence / finding** should cite the relevant `path/to/file.py:LN` or quote the specific lines or commit hash that justify the status. For `Pass` on a non-trivial item, briefly say *why* (one short sentence) rather than just `Pass`. LLM agents should err toward -`Human review` whenever they are uncertain, and should never mark an -item `Pass` without concrete evidence. +`Human review` whenever they are uncertain, and should never mark +an item `Pass` without concrete evidence. | # | Category | Item | Question | Status | Evidence / finding | |---|----------|------|----------|--------|--------------------| @@ -276,24 +255,31 @@ item `Pass` without concrete evidence. | 22 | Access Control & Concurrency | Race Conditions | Is the code protected against TOCTOU vulnerabilities? | | | | 23 | Access Control & Concurrency | No Backdoors | Are there no intentional or accidental bypass mechanisms? | | | -The wording of the items above is reproduced from NVIDIA's *PLC: -Code Review Excellence* training; it is the authoritative version, -and changes to it should track the upstream training rather than -diverging here. +## NVIDIA PLC compliance note + +FlashDreams is released alongside an NVIDIA Product Lifecycle (PLC) +"+2 Code Review" compliance requirement. The CODEOWNERS-based +review described above — one approval from a qualified code owner, +checklist applied, security team on sensitive paths, no bypass — +is what satisfies that requirement. Code owners are trained as +required by NVIDIA PLC before being added to a CODEOWNERS team, and +the checklist questions above are reused from NVIDIA's *PLC: Code +Review Excellence* training. No separate, parallel process is +needed. ## Changing this document or the CODEOWNERS file Both this file and `.github/CODEOWNERS` are themselves owned by `@NVIDIA/flashdreams-maintainers` and `@NVIDIA/flashdreams-security`. If you want to propose a change — new subsystem owners, a different -ownership boundary, a refinement to the +2 policy — please open an -issue or Discussion first so we can talk through it before the PR. -The goal is for the review policy to be predictable; we change it -deliberately rather than incrementally. +ownership boundary, a refinement to how review works — please open +an issue or Discussion first so we can talk through it before the +PR. The goal is for the review policy to be predictable; we change +it deliberately rather than incrementally. --- -Thanks for reading this far. The project is healthier when reviewers -and contributors share a clear picture of how decisions get made; if -anything above is unclear, that's a bug in the documentation and -we'd appreciate the issue. +Thanks for reading this far. The project is healthier when +reviewers and contributors share a clear picture of how decisions +get made; if anything above is unclear, that's a bug in the +documentation and we'd appreciate the issue. From c7cc0e955dc04e39a93c92f04c85d64f749733c3 Mon Sep 17 00:00:00 2001 From: Jonathan McCaffrey Date: Wed, 20 May 2026 20:12:37 +0000 Subject: [PATCH 4/5] docs: tighten exception clause, clarify external-contributor path - "No bypass" exception now documents an issue and retains the audit trail, without the in-writing-approval phrasing. - "Becoming a code owner" explains the two CODEOWNERS reference forms (team handle vs individual @username) and notes that the @NVIDIA/* teams require NVIDIA org membership, so external contributors are recognised by being added as individuals. - Rephrases the "from a contributor's perspective" intro to describe the flow welcomingly rather than waving it off. Co-Authored-By: Claude Opus 4.7 (1M context) Signed-off-by: Jonathan McCaffrey --- docs/code_review.md | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/docs/code_review.md b/docs/code_review.md index d53880e48..2905d74c3 100644 --- a/docs/code_review.md +++ b/docs/code_review.md @@ -158,7 +158,9 @@ Follow NVIDIA's coordinated disclosure process at ## What this looks like from a contributor's perspective -You don't need to think about most of this when opening a PR: +Here's what to expect when you open a pull request — the +mechanics are straightforward, and you'll see each step on the +PR page as it happens: 1. Fork, branch, code, sign off, push, open a PR — same flow as in [`CONTRIBUTING.md` → Submitting a pull request](../CONTRIBUTING.md#submitting-a-pull-request). @@ -183,8 +185,14 @@ governance opens up over time (see [`CONTRIBUTING.md` → Project governance](../CONTRIBUTING.md#project-governance)). Contributors — NVIDIA employee or not — who consistently land high-quality work in an area, participate in reviews, and engage -with the issue tracker can be invited onto the relevant CODEOWNERS -team. +with the issue tracker can be invited to become code owners. +CODEOWNERS supports two ways to be listed: team references such as +`@NVIDIA/flashdreams-integrations`, and individual GitHub handles +such as `@username`. The `@NVIDIA/*` teams require NVIDIA +organization membership, so contributors outside NVIDIA are +recognised by adding their individual handle to the relevant +CODEOWNERS rule — they still count as required reviewers for the +paths they own. There is no fixed time bar; we look for sustained good judgment about when to ship, when to push back, and when to ask for help. If @@ -201,10 +209,9 @@ Branch protection on `main` is configured so that: If an operational situation genuinely requires an exception — for example, an emergency security patch where the usual reviewer is -unavailable — the exception must be documented as an issue or -incident record, approved by the security team in writing, and the -audit trail retained. We have not needed to use this path, and we -would prefer to keep it that way. +unavailable — the exception must be documented as an issue and the +audit trail of the incident retained. We have not needed to use +this path, and we would prefer to keep it that way. ## Security review checklist From 9480367766aca7cb0f47428a79cb1ec29151abad Mon Sep 17 00:00:00 2001 From: Jonathan McCaffrey Date: Wed, 20 May 2026 20:46:00 +0000 Subject: [PATCH 5/5] docs: unify ownership tiers for licensing and dep manifests Three cleanups from RFC feedback: - Section comment that read "remain maintainer-owned but require security sign-off too" misrepresented OR semantics; rewritten to describe the actual alternate-approver model. - pyproject.toml and uv.lock (root) now share security-only ownership, matching the reality that uv mutates them together. flashdreams/ and integrations/* workspace members resolve into the root uv.lock and have no per-package lockfile, so only their pyproject.toml files appear in CODEOWNERS. - LICENSE, LICENSES/, NOTICE, and reuse.toml are now grouped into a single "Licensing and attribution" section, all security-only. - Adds the nested integrations/alpadreams/ludus-renderer sub-workspace (its own pyproject.toml + uv.lock) to the dependency-manifest tier. Co-Authored-By: Claude Opus 4.7 (1M context) Signed-off-by: Jonathan McCaffrey --- .github/CODEOWNERS | 55 +++++++++++++++++++++++++++------------------ docs/code_review.md | 4 ++-- 2 files changed, 35 insertions(+), 24 deletions(-) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 2dbd92a06..96b533b95 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -14,9 +14,9 @@ # @NVIDIA/flashdreams-integrations — per-recipe maintainers under # integrations/. Maintainers can # also review here. -# @NVIDIA/flashdreams-security — sole or additional approver on -# licensing and dependency-manifest -# files. +# @NVIDIA/flashdreams-security — sole or alternate approver on +# licensing, attribution, and +# dependency-manifest files. # # Notes: # * GitHub honours one CODEOWNERS file per repository; this is it. @@ -24,17 +24,17 @@ # below. The last matching rule wins, so more specific paths appear # below more general ones. # * When multiple owners are listed on a single rule, GitHub treats -# them as alternatives — any *one* of them satisfies the required -# review. -# * Owners listed here are the project's qualified "+2" reviewers. -# They apply the +2 Security Review Checklist on every review (see -# docs/code_review.md) and are trained as required by NVIDIA PLC. -# Branch protection on `main` requires at least one approving review -# from a CODEOWNER for the touched paths. +# them as alternatives (OR) — any *one* of them satisfies the +# required-review-from-Code-Owners check. +# * Owners listed here are the project's qualified code-review +# reviewers. They apply the security review checklist in +# docs/code_review.md on every review. Branch protection on `main` +# requires at least one approving review from a CODEOWNER for the +# touched paths. # * External contributors are welcome on every path. CODEOWNERS does # not gate who may *open* a PR — only who must *approve* it. -# * The PLC gate cannot be bypassed or waived; any exception requires -# a documented audit trail approved by the Security PIC. +# * The review requirement cannot be bypassed or waived; any +# exception requires a documented audit trail. # --------------------------------------------------------------------- # Default owners @@ -50,25 +50,36 @@ /integrations/ @NVIDIA/flashdreams-integrations @NVIDIA/flashdreams-maintainers # --------------------------------------------------------------------- -# Security-sensitive files +# Licensing and attribution # -# These touch Apache-2.0 attribution and the dependency / SBOM surface, -# and are owned by the security team as sole approver. +# Apache-2.0 obligations: the project license, per-file SPDX licenses, +# the NOTICE file, and the REUSE manifest are all security-owned so a +# trained reviewer signs off on attribution changes. # --------------------------------------------------------------------- /LICENSE @NVIDIA/flashdreams-security +/LICENSES/ @NVIDIA/flashdreams-security +/NOTICE @NVIDIA/flashdreams-security /reuse.toml @NVIDIA/flashdreams-security -/uv.lock @NVIDIA/flashdreams-security -/flashdreams/pyproject.toml @NVIDIA/flashdreams-security # --------------------------------------------------------------------- -# Licensing & dependency-manifest files with additional security review +# Dependency manifests and lockfiles +# +# `pyproject.toml` and `uv.lock` change together (any `uv add`, +# `uv lock`, or override edit touches both), so they share a single +# owner. `flashdreams/` and `integrations/*/` are workspace members +# whose dependencies resolve into the root `uv.lock` — they have no +# per-package lockfile. # -# These remain maintainer-owned but require security sign-off too. +# Per-integration `pyproject.toml` files keep the integrations team as +# an alternate owner so recipe-local dep changes don't always need to +# wait on security. # --------------------------------------------------------------------- -/LICENSES/ @NVIDIA/flashdreams-maintainers @NVIDIA/flashdreams-security -/NOTICE @NVIDIA/flashdreams-maintainers @NVIDIA/flashdreams-security -/pyproject.toml @NVIDIA/flashdreams-maintainers @NVIDIA/flashdreams-security +/pyproject.toml @NVIDIA/flashdreams-security +/uv.lock @NVIDIA/flashdreams-security +/flashdreams/pyproject.toml @NVIDIA/flashdreams-security /integrations/*/pyproject.toml @NVIDIA/flashdreams-integrations @NVIDIA/flashdreams-security +/integrations/alpadreams/ludus-renderer/pyproject.toml @NVIDIA/flashdreams-integrations @NVIDIA/flashdreams-security +/integrations/alpadreams/ludus-renderer/uv.lock @NVIDIA/flashdreams-security # --------------------------------------------------------------------- # Review-policy files themselves diff --git a/docs/code_review.md b/docs/code_review.md index 2905d74c3..21e3eaca8 100644 --- a/docs/code_review.md +++ b/docs/code_review.md @@ -97,8 +97,8 @@ repository. |------|-------|-------------| | **Default** (core, infra, recipes, plugins, configs, tests, docs, CI, container) | everything not listed below | `@NVIDIA/flashdreams-maintainers` | | **Integrations** | `integrations/` (all recipes) | `@NVIDIA/flashdreams-integrations` *or* `@NVIDIA/flashdreams-maintainers` | -| **Security-sensitive files** | `LICENSE`, `reuse.toml`, `uv.lock`, `flashdreams/pyproject.toml` | `@NVIDIA/flashdreams-security` (sole owner) | -| **Licensing & dependency manifests with security review** | `LICENSES/`, `NOTICE`, root `pyproject.toml` | `@NVIDIA/flashdreams-maintainers` *or* `@NVIDIA/flashdreams-security` | +| **Licensing & attribution** | `LICENSE`, `LICENSES/`, `NOTICE`, `reuse.toml` | `@NVIDIA/flashdreams-security` | +| **Dependency manifests & lockfiles** | `pyproject.toml`, `uv.lock`, `flashdreams/pyproject.toml`, nested `ludus-renderer/{pyproject.toml,uv.lock}` | `@NVIDIA/flashdreams-security` | | **Per-integration packaging** | `integrations/*/pyproject.toml` | `@NVIDIA/flashdreams-integrations` *or* `@NVIDIA/flashdreams-security` | | **Review-policy files** | `.github/CODEOWNERS`, `docs/code_review.md` | `@NVIDIA/flashdreams-maintainers` *or* `@NVIDIA/flashdreams-security` |