fix(compliance): remove config that reads as live but does nothing

[aabusair]

Clears three items from the compliance-engine backlog. They are one bug wearing three hats: something that looks like working configuration, sits where configuration lives, and silently does nothing when used.

No behaviour change to enforcement. Proven below, not asserted.

1. LICENSES_JSON pointed at a path that is never checked out

required-compliance.yml set it to .github-tools/data/licenses_all.json, but that job's checkout is sparse-checkout: scripts. The directory never existed on the runner.

It was inert only by luck: policy_selector fetches both catalogues over the API and hands them to requires_cla in memory, and license_detector consults LICENSES_JSON from disk only when given no catalog_data. A future caller that omitted catalog_data would have hit a missing file in production — and ruleset 12239008 pins this workflow to refs/heads/main, so there is no pre-merge signal there at all.

Removed the variable rather than adding data to the sparse-checkout: deleting makes the intent true, and matches how the code actually works.

2. spdx_aliases and force_spdx were documented but unread

Both sat in cla/allowlist.yml as ready-to-uncomment examples. Neither has ever been read by any code — _override_requires_cla reads only require_cla and allow_dco.

A commented-out example is a promise. Someone uncomments it, gets no error, and concludes it took effect. That is worse than no documentation, because this file is the most authoritative-looking source a reader has.

3. The top-level repos: block is a fallback, not a supplement

fetch_shared_config reads license_overrides.repos and falls back to a top-level repos: only when that is absent or empty. It is populated, so anything added to a top-level repos: is dropped in silence. It now warns and names what it ignored.

Fixed while in those lines

allowlist_data.get("license_overrides", {}) returns None when the key is present but null — a .get() default only applies to an absent key. So a file containing a bare license_overrides: raised AttributeError, which the surrounding except swallowed into "Enforcing CLA for every repo". One empty key cost the entire policy. Both lookups now use or {}.

Fails closed, so this was never a security hole — but one stray key taking the whole org to CLA is a harsh failure mode for a typo.

The paths filter, a third time

TestGateWorkflowPaths guards required-compliance.yml — which was not in the tests.yml paths filter, so the guard would not have run when the file it protects was edited.

That is the third instance: cla/** was missing in #74, data/** in #76. Each time the guard existed and simply never ran. Rather than fix the instance again, TestTestsWorkflowPathsFilter now asserts the filter covers every file the suite reads, deriving that list from the same constants the tests use instead of a hand-kept copy that would drift the same way.

Verification

95 tests pass (was 92).

Every new guard mutation-tested. Each mutation is caught by the named test and by no other, with the tree restored clean after each:

Mutation	Caught by
Reinstate LICENSES_JSON outside the sparse-checkout	test_every_tools_path_in_env_is_actually_checked_out
Restore the commented force_spdx	test_commented_out_keys_are_also_read_by_live_code
Restore the commented spdx_aliases	test_commented_out_keys_are_also_read_by_live_code
Drop the shadowing warning	test_shadowing_is_reported_rather_than_silent
Turn the fallback into a merge	test_nested_repos_shadows_top_level_entirely
Restore the fragile .get() default	test_null_license_overrides_does_not_discard_the_whole_allowlist
Revert the paths filter	test_filter_covers_every_file_the_suite_reads
Desync the two triggers	test_both_triggers_declare_the_same_filter

Two tests guard the guards (test_commented_key_guard_can_actually_see_a_commented_key, test_coverage_check_rejects_a_path_outside_the_filter), because both new checks use narrow regex/glob matching that could otherwise pass vacuously on an empty set.

The allowlist edit is provably inert. Replaying origin/main's file and this one through the real fetch_shared_config offline gives identical allowlist_ok, allowlist_repos and allowlist_data, and identical decisions for Broadcom Source Available, Broadcom Proprietary, Apache-2.0 and GPL-2.0.

Done offline deliberately: fetch_mothership_file has no ?ref=, so a pre-merge sweeper run reads the default branch — it would test the old file, prove nothing, and write real statuses and comments to real PRs on the way.

Scope notes

• The normansden allowlist bypass was already removed by chore(compliance): remove dead workflow-override section from the CLA allowlist #74; nothing left to do.
• The commit-email blind spot in build_unowned_repo_owners_csv.py is not here — that script lives outside this repo by design, since it queries an internal PII endpoint. Being handled separately.

🤖 Generated with Claude Code

[aabusair]

Follow-up commit: ecf4949 — I introduced a regression, found and fixed it

Re-verifying by probing malformed allowlist shapes against origin/main, rather than re-reading the diff, showed the shadowing warning in e423b61 was worse than the bug it describes.

sorted(legacy_repos) assumed a dict. A top-level repos: holding a list of dicts raises TypeError; an int raises too. Both get swallowed by the enclosing except into "Enforcing CLA for every repo". origin/main handled both fine.

Measured per shape of a top-level repos::

shape	origin/main	e423b61	now
list of dicts	ok	discarded	ok
integer	ok	discarded	ok
nested entry null	discarded	discarded	ok
nested entry true	discarded	discarded	ok

The bottom two are pre-existing and in the same failure family three lines down: r_config.get("require_cla") on a null or boolean entry raises AttributeError, taking the whole file with it. A single missing indent under one repo cost every gated repo in the org. Fixed alongside, since I was already in those lines.

Malformed entries are now skipped with a warning naming the repo. The direction stays safe — allowlist_repos is the DCO-only set, so a skipped entry is held at CLA, the stricter document, and the readable entries survive. Blast radius goes from org-wide to one repo.

One test was lying

test_warning_renders_a_non_dict_legacy_block_without_raising originally used a one-element list. sorted() on a single element never performs a comparison, so it passed with the broken code in place — it could not fail for the scenario its own name describes. It now uses two dicts.

Mutation testing caught that. Review did not, and neither did the first pass of this PR.

Verification

• 101 tests (was 95).
• All 11 mutations across both harnesses caught by their named tests, tree restored clean after each — including re-introducing the sorted() regression and letting a skipped entry through into the DCO-only set.

Worth noting one of my mutations was initially invalid: I tried to make the malformed entry fall through, but the continue guard above made that line unreachable, so it was a no-op that looked like a missed test. Re-cut as appending to allowlist_repos from the skip branch, which is the real "let it through" case, and it's caught.