feat(standards): harden CODEOWNERS and pr-quality ruleset enforcement

scripts/compliance-audit.sh

@@ -992,6 +992,24 @@ _ruleset_contents_one() {
     return 0
   fi
 
+  # --- Check parameterless rule types codified in the source of truth --------
+  # Rules like `deletion` and `non_fast_forward` in pr-quality.json carry no
+  # `.parameters` block, so the parameter-level diff below does not inspect
+  # them. Run this before the `$rtype` early return so that a live ruleset
+  # missing both `$rtype` and a parameterless rule reports all missing rules in
+  # a single audit run.
+  local req_types req_type has_req_type
+  req_types=$(jq -r '.rules[]? | select((has("parameters") | not) or .parameters == null) | .type // empty' "$codified_file" 2>/dev/null || echo "")
+  while IFS= read -r req_type; do
+    [ -z "$req_type" ] && continue
+    has_req_type=$(echo "$rs" | jq --arg t "$req_type" 'any(.rules[]?; .type==$t)' 2>/dev/null || echo "false")
+    if [ "$has_req_type" != "true" ]; then
+      add_finding "$repo" "rulesets" "ruleset-drift-$name-missing-rule-$req_type" "error" \
+        "Ruleset \`$name\` is missing required \`$req_type\` rule codified in \`standards/rulesets/$name.json\`; run \`scripts/apply-rulesets.sh --repo $ORG/$repo\` to converge." \
+        "$std_ref"
+    fi
+  done <<< "$req_types"
+
   # A ruleset that exists but has lost the codified rule type entirely is drift.
   local has_rule
   has_rule=$(echo "$rs" | jq --arg t "$rtype" 'any(.rules[]?; .type==$t)' 2>/dev/null || echo "false")
@@ -1314,7 +1332,8 @@ check_legacy_rulesets() {
 check_codeowners() {
   local repo="$1"
 
-  # CODEOWNERS can be in root, .github/, or docs/
+  # CODEOWNERS can be in .github/, root, or docs/ (probed in GitHub's actual
+  # precedence order: .github/CODEOWNERS → CODEOWNERS → docs/CODEOWNERS).
   local found=false
   local codeowners_content=""
   # #437: distinguish a genuinely-absent file (a deterministic HTTP 404 on every
@@ -1324,7 +1343,7 @@ check_codeowners() {
   # (#487): capture the raw response + status that gh_api's `--jq` masks, retry
   # non-deterministic failures, and treat any non-404 failure as inconclusive.
   local any_unreadable=false
-  for path in CODEOWNERS .github/CODEOWNERS docs/CODEOWNERS; do
+  for path in .github/CODEOWNERS CODEOWNERS docs/CODEOWNERS; do
     local raw="" rc=1 i
     for i in 1 2 3; do
       raw=$(gh api "repos/$ORG/$repo/contents/$path" 2>&1) && rc=0 || rc=$?
@@ -1364,14 +1383,21 @@ check_codeowners() {
     return 0
   fi
 
-  # Extract non-comment, non-blank owner lines for accurate matching.
-  # Each such line has the form: <pattern> <owner1> [<owner2> ...]
+  # Extract non-comment, non-blank owner lines (stripping carriage returns and
+  # inline `#` comments) for accurate matching. Each such line has the form:
+  #   <pattern> <owner1> [<owner2> ...]
   # Standard (codeowners-standard.md):
   #   1. @combsmith/org-leads MUST be the FIRST owner on every line.
-  #   2. Additional teams (@combsmith/<slug>) are allowed.
-  #   3. Individual users (@username without "/") are forbidden.
+  #   2. Additional teams (@combsmith/<slug>) are allowed on path-specific rules;
+  #      the default `*` catch-all rule MUST list only `@combsmith/org-leads`.
+  #   3. Individual users and non-org owners are forbidden — every owner token
+  #      must be a team in the form `@combsmith/<slug>`.
   local owner_lines
-  owner_lines=$(echo "$codeowners_content" | grep -v '^[[:space:]]*#' | grep -v '^[[:space:]]*$')
+  owner_lines=$(printf '%s\n' "$codeowners_content" \
+    | tr -d '\r' \
+    | grep -v '^[[:space:]]*#' \
+    | sed -E 's/[[:space:]]+#.*$//' \
+    | grep -v '^[[:space:]]*$' || true)
 
   if [ -z "$owner_lines" ]; then
     add_finding "$repo" "settings" "codeowners-empty" "error" \
@@ -1381,46 +1407,64 @@ check_codeowners() {
   fi
 
   # Rule 1: @combsmith/org-leads MUST be the first owner on every line
-  # (the first whitespace-separated token after the pattern).
+  # (the first whitespace-separated token after the pattern, normalizing any
+  # `\ ` escaped spaces inside the path pattern first).
   local bad_first_owner=""
   while IFS= read -r line; do
     [ -z "$line" ] && continue
-    # awk $2 = first owner token after the pattern
+    # Collapse `\ ` escaped spaces in the pattern so awk $2 is the first owner token
     local first_owner
-    first_owner=$(echo "$line" | awk '{print $2}')
-    if [ "$first_owner" != "@combsmith/org-leads" ]; then
+    first_owner=$(printf '%s\n' "$line" | sed 's/\\ /\\x20/g' | awk '{print $2}')
+    if [ "$first_owner" != "@${ORG}/org-leads" ]; then
       bad_first_owner="$line"
       break
     fi
   done <<< "$owner_lines"
   if [ -n "$bad_first_owner" ]; then
     add_finding "$repo" "settings" "codeowners-org-leads-not-first" "error" \
-      "CODEOWNERS owner lines must list \`@combsmith/org-leads\` as the FIRST owner. Offending line: \`$bad_first_owner\`" \
+      "CODEOWNERS owner lines must list \`@${ORG}/org-leads\` as the FIRST owner. Offending line: \`$bad_first_owner\`" \
       "standards/codeowners-standard.md"
   fi
 
-  # Rule 3: no individual users — every owner token must be a team (contain "/").
-  # Collect owner tokens from all lines (everything starting with @).
+  # Rule 3: no individual users, non-org owners, or malformed team slugs —
+  # every owner token (fields 2..NF on each owner line after normalizing `\ `
+  # escaped spaces in the pattern) must match `@combsmith/<team-slug>` where
+  # GitHub team slugs are lowercase alphanumeric and hyphens (`[a-z0-9-]+`).
   local individual_owners
-  individual_owners=$(echo "$owner_lines" \
-    | tr ' \t' '\n' \
-    | grep -E '^@' \
-    | grep -vE '^@[^/]+/' \
+  individual_owners=$(printf '%s\n' "$owner_lines" \
+    | sed 's/\\ /\\x20/g' \
+    | awk -v org="$ORG" '{
+        for (i = 2; i <= NF; i++) {
+          if ($i ~ /^#/) break
+          if ($i !~ ("^@" org "/[a-z0-9-]+$")) print $i
+        }
+      }' \
     | sort -u || true)
   if [ -n "$individual_owners" ]; then
     local joined
     joined=$(echo "$individual_owners" | tr '\n' ' ')
     add_finding "$repo" "settings" "codeowners-individual-users" "error" \
-      "CODEOWNERS contains forbidden individual user owners: ${joined% } — only teams (@combsmith/<slug>) are allowed; manage membership via teams" \
+      "CODEOWNERS contains forbidden individual user, non-org, or invalid-slug owners: ${joined% } — only teams (@${ORG}/<slug>) are allowed; manage membership via teams" \
       "standards/codeowners-standard.md"
   fi
 
-  # Advisory: a catch-all `*` pattern should exist so files unmatched by any
-  # path-specific rule still have an owner.
-  if ! echo "$owner_lines" | awk '{print $1}' | grep -qxF '*'; then
+  # Catch-all `*` rule: a default `*` pattern should exist so files unmatched by
+  # any path-specific rule still have an owner, and it must list ONLY
+  # `@combsmith/org-leads` (additional teams belong on path-specific rules only,
+  # otherwise they can satisfy `require_code_owner_review` repo-wide).
+  if ! printf '%s\n' "$owner_lines" | sed 's/\\ /\\x20/g' | awk '{print $1}' | grep -qxF '*'; then
     add_finding "$repo" "settings" "codeowners-no-catchall" "warning" \
       "CODEOWNERS has no default \`*\` catch-all pattern — files not matched by a path rule will have no owner and \`require_code_owner_review\` will not apply to them" \
       "standards/codeowners-standard.md"
+  else
+    local bad_catchall=""
+    bad_catchall=$(printf '%s\n' "$owner_lines" \
+      | awk '$1 == "*" && NF > 2 { print; exit }')
+    if [ -n "$bad_catchall" ]; then
+      add_finding "$repo" "settings" "codeowners-catchall-extra-owners" "error" \
+        "Default \`*\` catch-all line in CODEOWNERS must list ONLY \`@${ORG}/org-leads\` (\`* @${ORG}/org-leads\`); assign additional teams on path-specific rules instead. Offending line: \`$bad_catchall\`" \
+        "standards/codeowners-standard.md"
+    fi
   fi
 }
 

standards/codeowners-standard.md

@@ -12,9 +12,13 @@ Every owner line in a CODEOWNERS file MUST follow these rules:
 1. **`@combsmith/org-leads` MUST be the FIRST owner** on every owner line
    (so it always satisfies `require_code_owner_review`, regardless of
    path-specific overrides).
-2. **Additional teams are allowed** as subsequent owners when finer-grained
-   ownership is desired (e.g., `@combsmith/security-leads` for
-   `/SECURITY.md`).
+2. **Additional teams are allowed on path-specific rules** as subsequent owners
+   when finer-grained ownership is desired (e.g., `@combsmith/security-leads`
+   for `/SECURITY.md`). **The default `*` catch-all rule MUST list ONLY
+   `@combsmith/org-leads`** (`* @combsmith/org-leads`) with no additional teams
+   (because GitHub permits any listed owner on a matching rule to satisfy
+   `require_code_owner_review`, adding another team to `*` would grant that
+   team repo-wide code-owner approval standing).
 3. **Individual named users are forbidden** as owners. All owners MUST be
    teams in the form `@combsmith/<team-slug>`. Manage individual
    membership through team membership instead.
@@ -28,11 +32,14 @@ Every owner line in a CODEOWNERS file MUST follow these rules:
 /.github/       @combsmith/org-leads @combsmith/platform-leads
 ```
 
-The legacy direct-listing pattern is **forbidden**:
+The legacy direct-listing pattern and extra teams on the `*` catch-all are **forbidden**:
 
 ```text
 # DO NOT USE — individuals and bot accounts as owners
 * @don-petry @dependabot-automerge-petry
+
+# DO NOT USE — additional teams on the default '*' catch-all
+* @combsmith/org-leads @combsmith/security-leads
 ```
 
 ## Why

standards/rulesets/README.md

@@ -20,7 +20,7 @@ these rulesets are enforced.
 
 | File | Ruleset | Target | Enforces |
 |------|---------|--------|----------|
-| [`pr-quality.json`](pr-quality.json) | `pr-quality` | `~DEFAULT_BRANCH` | 1 approval, code-owner review, thread resolution, dismiss-stale, squash-only merge |
+| [`pr-quality.json`](pr-quality.json) | `pr-quality` | `~DEFAULT_BRANCH` | 1 approval, code-owner review, thread resolution, dismiss-stale, last-push approval, squash-only merge, block force-pushes & branch deletion |
 | [`code-quality.json`](code-quality.json) | `code-quality` | `~DEFAULT_BRANCH` | Required status checks (SonarCloud, CodeQL, agent-shield, dependency-audit), and code scanning merge protection: CodeQL, security alerts `medium_or_higher`, alerts `errors` — a floor the applier never lowers ([standard](../github-settings.md#code-scanning-merge-protection-all-repositories)) |
 | [`release-channel-tags.json`](release-channel-tags.json) | `release-channel-tags` | `refs/tags/**` | Blocks unauthorized **update/deletion** of any tag (protects moving channel pointers + version tags); creation stays free |
 | [`duplicate-decl-gate.json`](duplicate-decl-gate.json) | `duplicate-decl-gate` | `~DEFAULT_BRANCH` | Required status check `duplicate-decl-gate` — the #1485 corruption-class merge backstop. **Targeted** to the reusable-hosting meta-repos only (see below) |

standards/rulesets/pr-quality.json

@@ -20,6 +20,12 @@
     }
   },
   "rules": [
+    {
+      "type": "deletion"
+    },
+    {
+      "type": "non_fast_forward"
+    },
     {
       "type": "pull_request",
       "parameters": {

test/scripts/apply-rulesets/apply-rulesets.bats

@@ -102,6 +102,15 @@ EOF
   [ "$output" = "false" ]
 }
 
+@test "pr-quality.json: enforces branch deletion and force-push protection (deletion + non_fast_forward)" {
+  # Anti-divergence guard: github-settings.md#pr-quality requires Allow force
+  # pushes: No (non_fast_forward) and Allow deletions: No (deletion), which
+  # OpenSSF Scorecard Branch-Protection also checks on the default branch.
+  run jq -r '[.rules[].type] | sort | join(",")' "$RULESETS_DIR/pr-quality.json"
+  [ "$status" -eq 0 ]
+  [ "$output" = "deletion,non_fast_forward,pull_request" ]
+}
+
 @test "release-channel-tags.json: tag ruleset, generic refs/tags/** include, update+deletion only" {
   run jq -er '.target == "tag"
     and (.conditions.ref_name.include == ["refs/tags/**"])

test/scripts/compliance-audit/codeowners-catchall.bats

@@ -77,3 +77,105 @@ REPO_ROOT="$(cd "$BATS_TEST_DIRNAME/../../.." && pwd)"
   run _has_catchall "$(cat "$REPO_ROOT/.github/CODEOWNERS")"
   [ "$status" -eq 0 ]
 }
+
+# ---------------------------------------------------------------------------
+# End-to-end check_codeowners validation (precedence, inline comments,
+# catch-all extra owners, non-org/bare owner tokens)
+# ---------------------------------------------------------------------------
+
+_run_codeowners_body() {
+  local body="$1"
+  local tmp
+  tmp="$(mktemp -d "$BATS_TEST_TMPDIR/co.XXXXXX")"
+  mkdir -p "$tmp/bin"
+  printf '#!/usr/bin/env bash\nexit 0\n' > "$tmp/bin/sleep"
+  chmod +x "$tmp/bin/sleep"
+  local b64
+  b64="$(printf '%s' "$body" | base64 | tr -d '\n')"
+  cat > "$tmp/bin/gh" <<EOF
+#!/usr/bin/env bash
+printf '{"content":"%s"}\n' "$b64"
+EOF
+  chmod +x "$tmp/bin/gh"
+  PATH="$tmp/bin:$PATH" REPORT_DIR="$tmp" bash -c '
+    set -uo pipefail
+    echo "[]" > "$REPORT_DIR/findings.json"
+    # shellcheck disable=SC1090
+    source "'"$REPO_ROOT"'/scripts/compliance-audit.sh"
+    check_codeowners "demo-repo"
+    cat "$REPORT_DIR/findings.json"
+  '
+  local rc=$?
+  rm -rf "$tmp"
+  return "$rc"
+}
+
+@test "check_codeowners: inline '#' comments on catch-all and path rules are ignored" {
+  body=$'* @combsmith/org-leads # default owner (not @individual)\n/security/ @combsmith/org-leads @combsmith/security-leads # cc @someone\n'
+  run _run_codeowners_body "$body"
+  [ "$status" -eq 0 ]
+  [ "$output" = "[]" ]
+}
+
+@test "check_codeowners: catch-all '*' with extra team is flagged (codeowners-catchall-extra-owners)" {
+  body=$'* @combsmith/org-leads @combsmith/other-team\n'
+  run _run_codeowners_body "$body"
+  [ "$status" -eq 0 ]
+  [[ "$output" == *"codeowners-catchall-extra-owners"* ]]
+  [[ "$output" == *"error"* ]]
+}
+
+@test "check_codeowners: non-org team, bare username, or malformed team slug is flagged (codeowners-individual-users)" {
+  body=$'* @combsmith/org-leads\n/docs/ @combsmith/org-leads @other-org/team bareuser @combsmith/Org_Leads.With.Dots\n'
+  run _run_codeowners_body "$body"
+  [ "$status" -eq 0 ]
+  [[ "$output" == *"codeowners-individual-users"* ]]
+  [[ "$output" == *"@other-org/team"* ]]
+  [[ "$output" == *"bareuser"* ]]
+  [[ "$output" == *"@combsmith/Org_Leads.With.Dots"* ]]
+}
+
+@test "check_codeowners: CRLF line endings do not trigger false positives" {
+  body=$'* @combsmith/org-leads\r\n/docs/ @combsmith/org-leads @combsmith/doc-leads\r\n'
+  run _run_codeowners_body "$body"
+  [ "$status" -eq 0 ]
+  [ "$output" = "[]" ]
+}
+
+@test "check_codeowners: escaped-space path patterns do not mis-tokenize owners" {
+  body=$'* @combsmith/org-leads\nmy\\ dir/ @combsmith/org-leads\npath/with\\ multiple\\ spaces/ @combsmith/org-leads @combsmith/platform-leads\n'
+  run _run_codeowners_body "$body"
+  [ "$status" -eq 0 ]
+  [ "$output" = "[]" ]
+}
+
+@test "check_codeowners: probes .github/CODEOWNERS before root CODEOWNERS (GitHub precedence)" {
+  local tmp
+  tmp="$(mktemp -d "$BATS_TEST_TMPDIR/co-prec.XXXXXX")"
+  mkdir -p "$tmp/bin"
+  printf '#!/usr/bin/env bash\nexit 0\n' > "$tmp/bin/sleep"
+  chmod +x "$tmp/bin/sleep"
+  local good_b64 bad_b64
+  good_b64="$(printf '* @combsmith/org-leads\n' | base64 | tr -d '\n')"
+  bad_b64="$(printf '* @individual\n' | base64 | tr -d '\n')"
+  cat > "$tmp/bin/gh" <<EOF
+#!/usr/bin/env bash
+case "\$*" in
+  *"contents/.github/CODEOWNERS"*) printf '{"content":"%s"}\n' "$good_b64" ;;
+  *"contents/CODEOWNERS"*)         printf '{"content":"%s"}\n' "$bad_b64" ;;
+  *) echo "gh: Not Found (HTTP 404)" >&2; exit 1 ;;
+esac
+EOF
+  chmod +x "$tmp/bin/gh"
+  run env PATH="$tmp/bin:$PATH" REPORT_DIR="$tmp" bash -c '
+    set -uo pipefail
+    echo "[]" > "$REPORT_DIR/findings.json"
+    # shellcheck disable=SC1090
+    source "'"$REPO_ROOT"'/scripts/compliance-audit.sh"
+    check_codeowners "demo-repo"
+    cat "$REPORT_DIR/findings.json"
+  '
+  rm -rf "$tmp"
+  [ "$status" -eq 0 ]
+  [ "$output" = "[]" ]
+}

test/scripts/compliance-audit/ruleset-contents-drift.bats

@@ -63,9 +63,12 @@ case "\$path" in
     ;;
   */rulesets/101)
     prq='$prq'
+    prq_default='$PRQ_PARAMS'
     if [ "\$prq" = "FETCHFAIL" ]; then echo "gh: Server Error (HTTP 500)" >&2; exit 1; fi
     if [ "\$prq" = "NORULE" ]; then echo '{"name":"pr-quality","id":101,"rules":[{"type":"deletion","parameters":null}]}'; exit 0; fi
-    echo "{\"name\":\"pr-quality\",\"id\":101,\"source\":\"combsmith/demo-repo\",\"rules\":[{\"type\":\"pull_request\",\"parameters\":\$prq}]}"
+    if [ "\$prq" = "NODELETION" ]; then echo "{\"name\":\"pr-quality\",\"id\":101,\"source\":\"combsmith/demo-repo\",\"rules\":[{\"type\":\"non_fast_forward\"},{\"type\":\"pull_request\",\"parameters\":\$prq_default}]}"; exit 0; fi
+    if [ "\$prq" = "NONONFF" ]; then echo "{\"name\":\"pr-quality\",\"id\":101,\"source\":\"combsmith/demo-repo\",\"rules\":[{\"type\":\"deletion\"},{\"type\":\"pull_request\",\"parameters\":\$prq_default}]}"; exit 0; fi
+    echo "{\"name\":\"pr-quality\",\"id\":101,\"source\":\"combsmith/demo-repo\",\"rules\":[{\"type\":\"deletion\"},{\"type\":\"non_fast_forward\"},{\"type\":\"pull_request\",\"parameters\":\$prq}]}"
     ;;
   */rulesets/102)
     cq='$cq'
@@ -186,9 +189,24 @@ MOCK
 # ---------------------------------------------------------------------------
 # A ruleset that exists but has lost the codified rule type entirely is drift
 # ---------------------------------------------------------------------------
-@test "pr-quality present but missing its pull_request rule → finding" {
+@test "pr-quality present but missing its pull_request rule → finding (plus any missing parameterless rule)" {
   run _run_contents "NORULE" "$CQ_PARAMS"
   [ "$status" -eq 0 ]
-  [[ "$output" == *"ruleset-drift-pr-quality-missing-rule"* ]]
+  [[ "$output" == *"ruleset-drift-pr-quality-missing-rule\""* ]]
+  [[ "$output" == *"ruleset-drift-pr-quality-missing-rule-non_fast_forward"* ]]
+  [[ "$output" == *"error"* ]]
+}
+
+@test "pr-quality present but missing its deletion rule → finding" {
+  run _run_contents "NODELETION" "$CQ_PARAMS"
+  [ "$status" -eq 0 ]
+  [[ "$output" == *"ruleset-drift-pr-quality-missing-rule-deletion"* ]]
+  [[ "$output" == *"error"* ]]
+}
+
+@test "pr-quality present but missing its non_fast_forward rule → finding" {
+  run _run_contents "NONONFF" "$CQ_PARAMS"
+  [ "$status" -eq 0 ]
+  [[ "$output" == *"ruleset-drift-pr-quality-missing-rule-non_fast_forward"* ]]
   [[ "$output" == *"error"* ]]
 }