diff --git a/.claude/rules/conventions.md b/.claude/rules/conventions.md index 04ff5fe3..6365098c 100644 --- a/.claude/rules/conventions.md +++ b/.claude/rules/conventions.md @@ -9,4 +9,5 @@ - ✓ Document code by `.claude/rules/code-documentation.md` (YARD: rationale, reusable code, extension contracts, syntax). - ✗ Don't disable cops inline without a justification comment on the same line. - ✗ Don't create god-object services or `utils.rb` dumping grounds; name classes after what they do (`MTG::BulkImporter`, `Exports::CollectionArchive`). +- ✗ Don't add a gem or a package under a licence that isn't in `config/licence_policy.yml` to get `bin/ci` green: allowing one is the maintainer's decision, recorded there with a reason, and a named exception is for a tool the app never loads (ADR 0031). - ✗ Don't add gems for trivial functionality; prefer Rails built-ins (Solid*, `has_secure_password`, Active Storage, `Rails.cache`). diff --git a/.claude/rules/testing.md b/.claude/rules/testing.md index 2fd7692a..8ed44eff 100644 --- a/.claude/rules/testing.md +++ b/.claude/rules/testing.md @@ -1,6 +1,6 @@ # Testing (RSpec) -- ✓ `bin/ci` (driven by `config/ci.rb`) is the project's local CI tool and the pre-push gate. Keep it maintained: it runs `bundle exec rspec` (not the default `bin/rails test`), `bin/rubocop`, `bin/brakeman`, `bin/bundler-audit`, and `bin/importmap audit`. When adding a new check, add it to `config/ci.rb`. +- ✓ `bin/ci` (driven by `config/ci.rb`) is the project's local CI tool and the pre-push gate. Keep it maintained: it runs `bundle exec rspec` (not the default `bin/rails test`), `bin/rubocop`, `bin/brakeman`, `bin/bundler-audit`, `bin/importmap audit`, and `bin/rails licences:check` (a dependency's licence is allowed in `config/licence_policy.yml`, spec 091). When adding a new check, add it to `config/ci.rb`. - ✓ Mirror `app/` in `spec/` (`spec/models`, `spec/requests`, `spec/system`, `spec/jobs`); files end in `_spec.rb`. - ✓ Prefer request specs over controller specs; keep system specs (Capybara) for critical Hotwire flows. - ✓ FactoryBot with minimal valid factories and traits; prefer `build`/`build_stubbed` when persistence isn't needed. diff --git a/CLAUDE.md b/CLAUDE.md index 8e634bb4..5ae5f6bd 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -4,7 +4,7 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co ## Project Status -Collector is a self-hostable, multi-tenant web app for tracking collectibles, starting with Magic: The Gathering cards. The Rails app skeleton exists (stack, RSpec, `bin/ci`, Compose and Kamal deployment). Feature 002 added a collectible-agnostic catalog core (`app/models/catalog/`) with an MTG extension (`app/models/mtg/`), Scryfall bulk-data ingestion via a background refresh, and a card search at `/catalog/entries`, which has needed sign-in since accounts (spec 004) unless the operator opens it with `COLLECTOR_PUBLIC_CATALOG` (spec 023). Accounts (row-level tenancy through `Current.account`) and collections of lots (spec 004, list view spec 006) exist, and the card scanner (specs 005, 007–009) adds scanned cards to the collection through its confirm flow. Admins start and watch catalog refreshes and art index builds at `/admin/catalog`, and see, retry and discard background jobs at `/admin/jobs` (spec 015). A collector's data has a documented export format, and a collector downloads, previews and applies an export file from the account menu or More (`CollectionExport`, `CollectionImport`, `CollectionImport::Apply`; [`docs/architecture/data-portability.md`](docs/architecture/data-portability.md)). Every page's footer ends with Collector's licence line, AGPL-3.0 with a link to the source of the running build, and shows the running version and short SHA, which the image's build passes as `COLLECTOR_VERSION`, `COLLECTOR_REVISION` and `COLLECTOR_SOURCE_URL` and `Collector::BuildFacts` reads once at boot; `bin/image-smoke` fails an image without them (spec 084). The line ends with a link to `/licences`, which anyone can open and which lists the licences of the third-party software and assets in the image: the gems from the running bundle, the OCR engine, the importmap JavaScript, the fonts and the icons from `config/third_party_licences.yml`, kept honest by a guard spec (`Collector::ThirdPartyLicences`, `LicencesController`, spec 087). A collector adds cards from a set by collector number at `/collection/number_entry` ("Add by number", spec 026): one line per number, quantity and finish, each added at once to a per-account list with Undo (`NumberEntry`, `NumberEntry::Line`), which Done clears; the list is working state, left out of the export. A collector manages the places their collection is kept in at `/locations` (`Location`, spec 029) and records which location a lot is in when adding or editing it, or moves some or all of a lot's copies with Move copies (`Lot::Move`, spec 032; a location that holds copies can't be removed; export format version 3) and moves every copy of many lots at once from bulk mode with Move to… (`Lot::BulkMove`, spec 041), opens a location's lots in bulk mode from its removal refusal page with one button (`Locations::SelectionsController`, spec 059), and the collection's grid tiles and table rows show it (spec 033) and filter by it (spec 037). A collector creates, renames and removes decks at `/decks` (`Deck`, `Deck::Creation`, `Catalog::Formats`, spec 062, [ADR 0028](docs/adr/0028-decks-are-locations-with-zones.md)): a deck is a location of kind `deck` whose zones are child locations that hold its copies, the collectible extension registers the formats, a lot in a zone is shown as "Deck › Zone", the export format (version 4) carries decks, a collector changes a deck's format at `/decks/:id/format/edit` (`Deck::FormatChange`, spec 064), opens a deck's lots in bulk mode from its removal refusal page (`Decks::SelectionsController`, spec 067), and opens a deck at `/decks/:id` to read the cards they own in it, zone by zone, with their cost and legality (`Deck::Page`, the seam `Catalog::DeckCards`, spec 066), and the deck's shape, its mana curve, card types and coloured mana symbols, in a panel beside the zones (`Catalog::DeckShape`, `MTG::DeckShape`, spec 069), and records the cards a deck's zone needs and they don't own as proxies, shown beside the owned copies and counted in a deck's cards but never as owned (`Deck::Proxy`, spec 068): a change of format moves them into Considering and the export carries them (format version 4 has `proxies`). A collector swaps a proxy for a free copy they own (Use my copy) or a new copy (Add a new copy) from the row's menu, and the proxy shrinks or goes in the same write (`Deck::Proxy::UseCopies`, `Deck::Proxy::AddCopy`, `Lot.free`, `Lot::MoveIntoZone`, spec 070). A collector adds a card to a deck from its card page, their free copies or a proxy (`Deck::Addition`, `Lot::FreeCopies`, spec 071). The card page shows how many copies are free to use and which decks hold the rest, in its stats and an "In decks" table (`Catalog::CardOverview`, `Deck::CardUses`, spec 073). A collector checks a deck on demand from its page, its size, copy limits and the cards with owned copies and proxies in one zone, and nothing about the check is stored (`Catalog::DeckChecks`, `MTG::Collecting.deck_rules`, `Decks::ChecksController`, spec 079). An operator backs up a running instance and restores it onto a fresh volume with two rake tasks, `backup:create[DIRECTORY]` and `backup:restore[FILE]` (`Collector::Backup`, `lib/collector/backup/`, spec 030, ADR 0024). Market prices will live in a fifth SQLite database, `prices` (`PricesRecord`, `db/prices_migrate/`, `db/prices_structure.sql`; spec 081, ADR 0029; [prices](docs/architecture/prices.md)), whose five tables exist, with a price source contract and Scryfall's prices through it (`Prices::Sources`, `MTG::Scryfall::PriceQuotes`, `PriceSource.sync!`; spec 085, #364) and no refresh or reader yet, and the backup holds it beside the primary (backup format version 2); a server start refuses a volume that holds the prices database and no primary (`Collector::StartCheck`, `start:check`, spec 086). CI proves an upgrade from the previous release's published image (`bin/upgrade-smoke`, `script/upgrade/`, spec 031, #42; [release and deploy](docs/architecture/release-and-deploy.md)). License: AGPL-3.0. +Collector is a self-hostable, multi-tenant web app for tracking collectibles, starting with Magic: The Gathering cards. The Rails app skeleton exists (stack, RSpec, `bin/ci`, Compose and Kamal deployment). Feature 002 added a collectible-agnostic catalog core (`app/models/catalog/`) with an MTG extension (`app/models/mtg/`), Scryfall bulk-data ingestion via a background refresh, and a card search at `/catalog/entries`, which has needed sign-in since accounts (spec 004) unless the operator opens it with `COLLECTOR_PUBLIC_CATALOG` (spec 023). Accounts (row-level tenancy through `Current.account`) and collections of lots (spec 004, list view spec 006) exist, and the card scanner (specs 005, 007–009) adds scanned cards to the collection through its confirm flow. Admins start and watch catalog refreshes and art index builds at `/admin/catalog`, and see, retry and discard background jobs at `/admin/jobs` (spec 015). A collector's data has a documented export format, and a collector downloads, previews and applies an export file from the account menu or More (`CollectionExport`, `CollectionImport`, `CollectionImport::Apply`; [`docs/architecture/data-portability.md`](docs/architecture/data-portability.md)). Every page's footer ends with Collector's licence line, AGPL-3.0 with a link to the source of the running build, and shows the running version and short SHA, which the image's build passes as `COLLECTOR_VERSION`, `COLLECTOR_REVISION` and `COLLECTOR_SOURCE_URL` and `Collector::BuildFacts` reads once at boot; `bin/image-smoke` fails an image without them (spec 084). The line ends with a link to `/licences`, which anyone can open and which lists the licences of the third-party software and assets in the image: the gems from the running bundle, the OCR engine, the importmap JavaScript, the fonts and the icons from `config/third_party_licences.yml`, kept honest by a guard spec (`Collector::ThirdPartyLicences`, `LicencesController`, spec 087), and `bin/ci checks` fails a gem or a manifest entry under a licence that `config/licence_policy.yml` doesn't allow or name as an exception (`Collector::ThirdPartyLicences::Policy` and `Check`, `bin/rails licences:check`, spec 091, [ADR 0031](docs/adr/0031-a-dependency-licence-is-a-decision.md)). A collector adds cards from a set by collector number at `/collection/number_entry` ("Add by number", spec 026): one line per number, quantity and finish, each added at once to a per-account list with Undo (`NumberEntry`, `NumberEntry::Line`), which Done clears; the list is working state, left out of the export. A collector manages the places their collection is kept in at `/locations` (`Location`, spec 029) and records which location a lot is in when adding or editing it, or moves some or all of a lot's copies with Move copies (`Lot::Move`, spec 032; a location that holds copies can't be removed; export format version 3) and moves every copy of many lots at once from bulk mode with Move to… (`Lot::BulkMove`, spec 041), opens a location's lots in bulk mode from its removal refusal page with one button (`Locations::SelectionsController`, spec 059), and the collection's grid tiles and table rows show it (spec 033) and filter by it (spec 037). A collector creates, renames and removes decks at `/decks` (`Deck`, `Deck::Creation`, `Catalog::Formats`, spec 062, [ADR 0028](docs/adr/0028-decks-are-locations-with-zones.md)): a deck is a location of kind `deck` whose zones are child locations that hold its copies, the collectible extension registers the formats, a lot in a zone is shown as "Deck › Zone", the export format (version 4) carries decks, a collector changes a deck's format at `/decks/:id/format/edit` (`Deck::FormatChange`, spec 064), opens a deck's lots in bulk mode from its removal refusal page (`Decks::SelectionsController`, spec 067), and opens a deck at `/decks/:id` to read the cards they own in it, zone by zone, with their cost and legality (`Deck::Page`, the seam `Catalog::DeckCards`, spec 066), and the deck's shape, its mana curve, card types and coloured mana symbols, in a panel beside the zones (`Catalog::DeckShape`, `MTG::DeckShape`, spec 069), and records the cards a deck's zone needs and they don't own as proxies, shown beside the owned copies and counted in a deck's cards but never as owned (`Deck::Proxy`, spec 068): a change of format moves them into Considering and the export carries them (format version 4 has `proxies`). A collector swaps a proxy for a free copy they own (Use my copy) or a new copy (Add a new copy) from the row's menu, and the proxy shrinks or goes in the same write (`Deck::Proxy::UseCopies`, `Deck::Proxy::AddCopy`, `Lot.free`, `Lot::MoveIntoZone`, spec 070). A collector adds a card to a deck from its card page, their free copies or a proxy (`Deck::Addition`, `Lot::FreeCopies`, spec 071). The card page shows how many copies are free to use and which decks hold the rest, in its stats and an "In decks" table (`Catalog::CardOverview`, `Deck::CardUses`, spec 073). A collector checks a deck on demand from its page, its size, copy limits and the cards with owned copies and proxies in one zone, and nothing about the check is stored (`Catalog::DeckChecks`, `MTG::Collecting.deck_rules`, `Decks::ChecksController`, spec 079). An operator backs up a running instance and restores it onto a fresh volume with two rake tasks, `backup:create[DIRECTORY]` and `backup:restore[FILE]` (`Collector::Backup`, `lib/collector/backup/`, spec 030, ADR 0024). Market prices will live in a fifth SQLite database, `prices` (`PricesRecord`, `db/prices_migrate/`, `db/prices_structure.sql`; spec 081, ADR 0029; [prices](docs/architecture/prices.md)), whose five tables exist, with a price source contract and Scryfall's prices through it (`Prices::Sources`, `MTG::Scryfall::PriceQuotes`, `PriceSource.sync!`; spec 085, #364) and no refresh or reader yet, and the backup holds it beside the primary (backup format version 2); a server start refuses a volume that holds the prices database and no primary (`Collector::StartCheck`, `start:check`, spec 086). CI proves an upgrade from the previous release's published image (`bin/upgrade-smoke`, `script/upgrade/`, spec 031, #42; [release and deploy](docs/architecture/release-and-deploy.md)). License: AGPL-3.0. Mission and principles: `.claude/memory/foundation.md`. Goal and spec rules: `docs/north-star.md`. Order of work: `docs/roadmap.md`. Decisions: `docs/adr/` (index in its README). Current rules per subsystem: `docs/architecture/` (area docs). Detailed coding rules: `.claude/rules/`. Feature specs: `docs/specs/` (index: `docs/specs/README.md`). @@ -21,7 +21,7 @@ Ruby 4.0.7, Rails 8.1.4, SQLite in every environment (including production), Hot - Lint (autocorrect): `bin/rubocop -a` - API docs: `bin/yard doc` (spec 054, ADR 0027) writes the browsable site to `doc/` (ignored by Git, left out of the image): the classes of `app` and `lib` from their comments, with the area docs and ADRs as pages and the area map as the front page. A YARD warning, such as an unresolved `{…}` in a comment, fails it, and fails the `bin/ci checks` step "Docs: YARD builds without warnings". What to write in comments (the why, reusable code, extension contracts) is in `.claude/rules/code-documentation.md`. - Security: `bin/brakeman`, `bin/bundler-audit`, `bin/importmap audit` -- Local CI (single CI definition, `config/ci.rb`): `bin/ci` — setup, migration guard, RuboCop, YARD, the comment ratchet (spec 056: public code the branch adds or changes needs a summary; `.claude/rules/code-documentation.md`), Brakeman, bundler-audit, importmap audit, RSpec, serially. Groups (spec 035): `bin/ci checks` (everything but RSpec), `bin/ci rspec` (setup and the whole suite), `bin/ci rspec N/M` (shard N of M as GitHub runs it, to replay a failed shard; it prints a `CI=true bin/rspec --seed …` command for the same files, order and seed). GitHub Actions runs `bin/ci checks` and the RSpec shards in parallel, then builds, smoke-tests and publishes the container image; the rules are in [release and deploy](docs/architecture/release-and-deploy.md) (procedures: `docs/releasing.md`). Change CI steps in `config/ci.rb`. +- Local CI (single CI definition, `config/ci.rb`): `bin/ci` — setup, migration guard, RuboCop, YARD, the comment ratchet (spec 056: public code the branch adds or changes needs a summary; `.claude/rules/code-documentation.md`), Brakeman, bundler-audit, importmap audit, the licence check (`bin/rails licences:check`: every gem and manifest entry is under a licence `config/licence_policy.yml` allows or names as an exception, spec 091; a session runs it through `bin/ci checks`), RSpec, serially. Groups (spec 035): `bin/ci checks` (everything but RSpec), `bin/ci rspec` (setup and the whole suite), `bin/ci rspec N/M` (shard N of M as GitHub runs it, to replay a failed shard; it prints a `CI=true bin/rspec --seed …` command for the same files, order and seed). GitHub Actions runs `bin/ci checks` and the RSpec shards in parallel, then builds, smoke-tests and publishes the container image; the rules are in [release and deploy](docs/architecture/release-and-deploy.md) (procedures: `docs/releasing.md`). Change CI steps in `config/ci.rb`. - Shards' run-time log: `script/ci/record_runtime_log` rewrites `spec/support/runtime_log.txt` from a serial run; when to refresh it is in the area doc above. It runs the suite, so not while another run uses the worktree's test database. - Backup smoke: `bin/backup-smoke [restore-image]` (spec 030): backs up a running container and restores it into a second one under another secret, the documented ways; CI runs it after `bin/image-smoke` on both architectures; it needs a container engine, so it is not a `bin/ci` step. - Upgrade smoke: `bin/upgrade-smoke ` (spec 031): boots the previous release's published image, seeds it, starts a build on the same volume and compares the databases and what the app serves; CI runs it in the image job, before the push, on both architectures; it needs a container engine and a Ruby with the bundle, so it is not a `bin/ci` step. `ruby script/upgrade/guard.rb` is a `bin/ci` step. diff --git a/config/ci.rb b/config/ci.rb index b984b8e9..3167e080 100644 --- a/config/ci.rb +++ b/config/ci.rb @@ -32,6 +32,7 @@ step "Security: Brakeman code analysis", "bin/brakeman --quiet --no-pager --exit-on-warn --exit-on-error" step "Security: Gem audit", "bin/bundler-audit" step "Security: Importmap vulnerability audit", "bin/importmap audit" + step "Licences: dependencies are under allowed licences", "bin/rails licences:check" end step "Tests: RSpec", rspec unless selection.group == :checks diff --git a/config/licence_policy.yml b/config/licence_policy.yml new file mode 100644 index 00000000..1d7f5d90 --- /dev/null +++ b/config/licence_policy.yml @@ -0,0 +1,34 @@ +# The licences Collector's dependencies may be under, and the dependencies that are let through by name (spec 091, ADR 0031). +# `bin/ci checks` fails a gem, a pinned JavaScript package, an OCR package, a font or an icon set under a licence that is not +# in `allowed` and has no entry in `exceptions`. Adding to either list is a decision for the maintainer: say why in `reason`, +# and put the change in a pull request of its own or beside the dependency it is for. +# +# A component with several licences passes if any one is allowed. A licence is compared exactly as an SPDX id: "MIT License" +# or an expression in one string fails. An exception names a gem, the licences it was reviewed under (a set) and a reason; it +# goes stale, and fails the check, when the lockfile no longer names the gem, the gem declares other licences, or the gem is +# allowed without it. `all_required: true` marks a gem whose list of licences is not a choice, so every one must be allowed. Why +# it exists: a gem such as vcr declares Hippocratic-2.1 and MIT, and the rule "any one allowed" would pass it on MIT, so an +# ordinary exception for it would read as stale (the gem is allowed without it). With `all_required` the gem fails without the +# exception, the exception is what lets it through, and it goes stale only when every one of the gem's licences is allowed. +# Copy this form only for a gem whose declared alternatives you do not trust; an ordinary exception is for a gem that fails. + +allowed: + - { id: MIT, reason: "Permissive, notice only: the copyright line and licence text travel with the copies, which the third-party licences page carries (spec 087)" } + - { id: BSD-2-Clause, reason: "Permissive, notice only; carried by the third-party licences page" } + - { id: BSD-3-Clause, reason: "Permissive, notice only, and a clause against using the authors' names to endorse; carried by the third-party licences page" } + - { id: Apache-2.0, reason: "Permissive, with a patent grant and a notice file that travels with the copies; carried by the third-party licences page" } + - { id: ISC, reason: "Permissive, notice only; the icons' licence" } + - { id: Ruby, reason: "Ruby's own licence, declared by the default gems, usually beside BSD-2-Clause" } + - { id: OFL-1.1, reason: "The licence of the self-hosted typefaces: the licence text travels with the font files, which are unmodified copies of their packages" } + +exceptions: + - gem: bundler-audit + licences: [GPL-3.0-or-later] + reason: "A command-line audit run by bin/ci; the application never loads it, and it leaves the image when #408 lands" + - gem: brakeman + licences: [Brakeman Public Use License] + reason: "A static analyser run by bin/ci and the commit hook; the application never loads it, and it leaves the image when #408 lands" + - gem: vcr + licences: [Hippocratic-2.1, MIT] + all_required: true + reason: "A test recorder the application never loads. It declares Hippocratic-2.1 and MIT, ships no licence file, and issue #408 calls it Hippocratic-2.1, so its declared MIT is not trusted. all_required is why this is an exception at all: without it the gem would pass on its MIT and this entry would read as stale. It goes stale only when every licence it declares is allowed" diff --git a/docs/adr/0031-a-dependency-licence-is-a-decision.md b/docs/adr/0031-a-dependency-licence-is-a-decision.md new file mode 100644 index 00000000..d3c1a483 --- /dev/null +++ b/docs/adr/0031-a-dependency-licence-is-a-decision.md @@ -0,0 +1,52 @@ +# 0031: A dependency's licence is a decision: an allowed list, named exceptions, and a CI step that enforces both + +## Status + +Proposed (2026-11-13). Numbered 0031 because 0030 is spec 090's, on its own branch. Written with spec 091; the maintainer ruled its decisions 1 to 9 as recommended on 2026-11-13 and asked for this ADR (the coordinator's number 0031). It becomes Accepted with the pull request. + +**Date:** 2026-11-13 +**Feature:** 091-licence-check-in-ci + +## Context + +Collector is AGPL-3.0 as one codebase, and the maintainer intends a hosted service on the same code ([households and hosted service design](../superpowers/specs/2026-11-13-households-and-hosted-service-design.md), decision on one AGPL codebase). A dependency's licence is then something the maintainer wants to decide, not find out: a copyleft, field-of-use or source-available licence can arrive with a gem or a pinned JavaScript package. Until spec 091 nothing read which licence a dependency was under: spec 087's guard fails a pin or a font with no manifest entry and its page lists the licences, but a gem under the GPL-3.0 would pass `bin/ci`. + +Measured on `main` at `71212946` (spec 091, Problem Statement): 142 gems in every Bundler group, 13 distinct sets of licences; all but two gems declare at least one of MIT, BSD-2-Clause, BSD-3-Clause, Apache-2.0 or Ruby. The two are development and test tools the application never loads, `bundler-audit` (GPL-3.0-or-later) and `brakeman` (Brakeman Public Use License). Three gems declare several licences and pass on one (`rdoc`, `diff-lcs`, `vcr`). The manifest side holds MIT, Apache-2.0, OFL-1.1 and ISC. + +Facts about the tools: a gemspec's `licenses` is a list the author gives, and RubyGems gives it no either/or meaning (it is commonly written as a choice); the lockfile names gems, and `Bundler.load.specs` gives the installed ones with their declared `licenses`; a JavaScript package has no machine-readable metadata in CI (there is no `node_modules` and no network), so its licence is the `spdx` a person typed into `config/third_party_licences.yml` from the package. + +## Options considered + +**For what the check compares against:** + +- **A. An allowed list of SPDX identifiers, with named exceptions (chosen).** Anything not on the list fails, so a new kind of licence is a decision by default. Costs: an occasional exception for a fine licence that is spelt unusually. +- **B. A deny list of copyleft families.** Fewer failures, but a licence the author didn't think of (a field-of-use licence such as Hippocratic-2.1, a source-available licence) passes silently. Not chosen. +- **C. No check; rely on review.** The state before this ADR. A copyleft gem enters as a transitive dependency of a bump nobody reads. Not chosen. + +**For a component that declares several licences:** + +- **A. It passes if any one is allowed (chosen).** The check reads the list as a choice, as it is commonly written, and the page shows the same identifiers. Costs: a gem whose list is not a choice passes on its allowed id, and the check trusts that id. `vcr` was the doubtful case: it ships no licence file, and issue #408 calls it Hippocratic-2.1, so the maintainer ruled it a named exception (2026-11-13, `all_required`), and the rule stays as ruled for `rdoc` and `diff-lcs` (spec 091 decisions 2 and 3). +- **B. Every one must be allowed.** Stricter and unambiguous, but `diff-lcs`, `vcr` and `rdoc` would each need an exception for a licence nobody chose to use, which buries the exceptions that are real decisions. Not chosen. + +**For which gems are judged:** + +- **A. Every Bundler group (chosen).** Independent of what the image installs ([#408](https://github.com/plainprogrammer/Collector/issues/408)) and of the `Dockerfile`; a development tool under a restricted licence is a recorded exception. +- **B. Only what the image ships.** Smaller list of exceptions, but it depends on #408 and on a Dockerfile edit, and a contributor's tools escape. Not chosen. + +## Decision + +1. **`config/licence_policy.yml` records the allowed SPDX identifiers, each with a reason, and the exceptions.** Allowed: `MIT`, `BSD-2-Clause`, `BSD-3-Clause`, `Apache-2.0`, `ISC`, `Ruby` and `OFL-1.1`, and nothing else. Adding an identifier is a change to that file, which shows in review and is the maintainer's decision. +2. **A component with several identifiers passes if any one is allowed;** a component with none, or with a string that isn't exactly an allowed identifier (`MIT License`, an expression in one string), fails and names the string. Nothing is normalised. +3. **An exception names a gem, the set of licences it was reviewed under and the reason.** It goes stale, and fails the check, when the lockfile no longer names the gem, when the gem declares other licences, or when the gem is allowed without it. An exception may mark a gem whose list is not a choice (`all_required`), so every identifier must be allowed and the exception is what lets it through. Exceptions are for gems; a manifest entry under a disallowed licence is a change to the list or no dependency. Today: `bundler-audit`, `brakeman` and `vcr`. +4. **Every Bundler group is judged, and so is every entry of the third-party manifest** (JavaScript, OCR engine, fonts, icons) by its `spdx`, by the same rules. A run with some group left out judges what is installed, and a gem the lockfile names but the bundle lacks is neither judged nor stale; a manifest key the reader doesn't list is not judged (spec 091 decisions 3, 4 and 6). A manifest entry with a misspelt `spdx` has no exception and is corrected. +5. **The check is a step in `bin/ci checks`,** `bin/rails licences:check`, which judges the list spec 087's reader builds and also reports spec 087's guard, so a pull request that adds a gem or a pin under a licence off the list fails the checks job. The failure names the dependency, its licence and the policy file. + +## Consequences + +- **Good:** a dependency that declares only licences off the list can't enter unnoticed; the diff that allows one is one small file with a reason beside it. The page and the check read one list, so they can't disagree about what ships. +- **Good:** a release that changes an excepted gem's licence makes its exception stale, so the change is seen. +- **Cost:** a gem with an unusual spelling of an allowed licence needs an exception, or a correction of the list, until a better rule is worth its parser (spec 091 Out of Scope). +- **Limit:** the check trusts what is declared. A gem whose metadata says MIT and whose files say otherwise passes, and a manifest entry's `spdx` is only as good as the person who typed it from the package (its `source` says where); and a gem whose list of licences isn't a choice (`vcr`, above) passes on its allowed id unless it is excepted. The check makes no claim that a listed licence is compatible with the AGPL-3.0; the list is the maintainer's decision, recorded. +- **Limit:** a licence expressed as "all of A and B" can't be written in the metadata the check reads; it is judged by its listed identifiers, any one of which may pass. +- **Cost:** `bin/ci checks` gains a step that boots the application and reads about 400 KB of licence files (its time is recorded in spec 091's `verification.md`). +- **Follow-up:** [#408](https://github.com/plainprogrammer/Collector/issues/408) takes the two exceptions' gems out of the image; the exceptions stay, as development-tool exceptions. diff --git a/docs/adr/README.md b/docs/adr/README.md index e78d3f7b..44d7cc74 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -65,6 +65,10 @@ Add a line here with each new ADR. All are Accepted unless the line says otherwi - [0029](0029-prices-database-held-by-the-backup.md): Prices live in a fifth database with `WITHOUT ROWID` history, and the backup holds it (backup format version 2) - [0026](0026-forms-show-field-errors-through-the-design-system.md): Forms show field errors through the design system, not Rails' wrapper +### Dependencies and licensing + +- [0031](0031-a-dependency-licence-is-a-decision.md): A dependency's licence is a decision: an allowed list, named exceptions, and a CI step that enforces both (Proposed) + ### Expected Named by the [roadmap](../roadmap.md) and written with their specs: none now. The upload-retention ADR expected by spec 028 is 0022 above, and the YARD ADR (spec 027 slice B) is 0027 above (it was expected as 0023, then as the next free number: 0023 went to locations, 0025 to lots in locations, 0026 to form errors). diff --git a/docs/architecture/catalog-core.md b/docs/architecture/catalog-core.md index 7b041c19..67bc8e09 100644 --- a/docs/architecture/catalog-core.md +++ b/docs/architecture/catalog-core.md @@ -150,6 +150,14 @@ The page opens with a sentence saying Collector is free software under the AGPL- **What keeps it from going stale** (decision 3, AC-5.1–AC-5.8): a guard spec in the suite (`spec/third_party_licences_manifest_spec.rb`, running `Collector::ThirdPartyLicences::Audit`) fails, naming the offender, when a pinned OCR package's digest has no entry or an entry's digest is not pinned; an importmap pin outside `app/javascript` has no entry, finds no asset, or has a changed file digest or gem version; a font file is added, removed or changed; an icon file is in no entry; a gem has neither licence metadata nor a licence file; or an entry lacks a field or names a missing, empty or outside text file (an entry says `text: none` with a reason when its source has none). The suite holds every group and the image all but `development`, so the guard covers a superset; `bin/image-smoke` proves the page in the image (see [release and deploy](release-and-deploy.md)). The core's files for this page name no collectible (`spec/licences_core_spec.rb`, as `spec/footer_core_spec.rb`). +**What each one may be under** (spec 091, ADR 0031): `config/licence_policy.yml` records the allowed SPDX identifiers, each with a reason, and the named exceptions; `bin/ci checks` fails a gem, or a manifest entry (JavaScript, OCR engine, font, icon), under a licence that is not on the list. The step is "Licences: dependencies are under allowed licences", `bin/rails licences:check[policy]` (the optional argument is a policy file's path), after the importmap audit and outside the RSpec group. It judges the very list the page shows (`Collector::ThirdPartyLicences::Policy` over `ThirdPartyLicences.build`'s sections, so there is no second reader of the bundle) and also reports the guard above (`Check` joins `Audit`'s problems and the policy's), so an importmap pin with no entry fails the checks job as well as the suite. A failure names the dependency, its licence and the policy file (decisions 1, 5, 7, AC-1.1, AC-4.3); the sentences about components come in the order the page lists them and the stale exceptions' follow, in the policy file's order. The rules: + +- **Allowed:** `MIT`, `BSD-2-Clause`, `BSD-3-Clause`, `Apache-2.0`, `ISC`, `Ruby`, `OFL-1.1`, and nothing else: another licence is the maintainer's decision, recorded in that file with a reason (decision 1). +- **Several licences:** a component passes if any one is allowed; one with none, or with a string that is not exactly an allowed id (`MIT License`, `MIT OR Apache-2.0`), fails and names the string. Nothing is normalised (decisions 2 and 5, AC-1.2–AC-1.4). That lets `rdoc` in as `Ruby`, and `diff-lcs` as `MIT`. +- **Exceptions** are for gems, each naming the gem, the licences it was reviewed under (a set) and a reason. One is stale, and fails, when the lockfile no longer names the gem, the gem declares other licences, or the gem is allowed without it; a gem the lockfile names and the bundle lacks (a run with a group left out) is neither judged nor stale (decision 3, AC-3.1, AC-3.2, AC-3.5). Today `bundler-audit` (GPL-3.0-or-later), `brakeman` (Brakeman Public Use License) and `vcr`, each a tool the application never loads. `vcr` declares `Hippocratic-2.1` and `MIT`, whose `MIT` the maintainer did not trust, so its exception says `all_required: true`: every licence of such a gem must be allowed, so it fails without the exception, and the exception goes stale only when every one is allowed. Without that form the any-one rule would pass `vcr` on `MIT` and an ordinary exception for it would read as stale (decision 3, AC-3.6). +- **Every Bundler group** is judged (the suite and CI hold all of them), so the rule doesn't depend on what the image installs; a manifest entry is judged by its `spdx` by the same rules, and an exception doesn't apply to one (decisions 4 and 6, AC-2.1–AC-2.3). +- **Limits:** the check trusts what is declared: a gem's metadata, and a manifest entry's `spdx`, which whoever adds it types from the package (its `source` says where); a manifest key the reader doesn't list is neither shown nor judged; it claims nothing about compatibility with the AGPL-3.0 (ADR 0031). + The page is at most 1 MiB of HTML (AC-1.9; 557 KB measured with the suite's bundle, rendered in about 10 ms). The row's pattern is `Licences` in the [design system](../design-system/README.md) (`app/assets/stylesheets/collector/additions.css`, "Licences (spec 087)"). ### Collector-number order @@ -204,6 +212,7 @@ A card image fits its slot with `object-fit: contain`, centred, at the largest s | Footer type and clearance | 12px on a 16px line, `ink-muted`; padding `space-8` wide, `space-4` below 640px; 80px clearance below 640px | `app/assets/stylesheets/collector/additions.css` ("Footer") | Small print at the size of the line the card page's attribution replaced; the tab bar's 80px clearance moves from `c-main` to the footer | spec 018 FR-4, AC-3.1, AC-3.2 | | `Collector::BuildFacts::LICENCES_PATH` | `"/licences"` | `lib/collector/build_facts.rb` | The address of the third-party licences page, a constant because the licence line's parts are built in `lib/` with no route helpers; a spec asserts it equals `licences_path` | spec 087 decision 1, AC-2.3 | | `Collector::ThirdPartyLicences::TEXT_FILE` | `/licen[sc]e\|copying\|copyright\|notice\|\A(bsdl\|legal)(\.\w+)?\z/i` | `lib/collector/third_party_licences.rb` | The names a licence file has among the bundle's gems (`MIT-LICENSE`, `LICENSE-DEPENDENCIES.md`, `license_of_rb-readline`, and the Ruby gems' `BSDL` and `LEGAL`), matched on a gem's top-level files only | spec 087 decision 3, AC-1.6 | +| `Collector::ThirdPartyLicences::Policy::PATH`, `DECISION` | `config/licence_policy.yml`; `ADR 0031` | `lib/collector/third_party_licences/policy.rb` | The policy file the check reads and the decision its failure message points at | spec 091 decision 8, AC-1.1 | | `Collector::ThirdPartyLicences::MAX_TEXT_BYTES` | 1 MiB | `lib/collector/third_party_licences.rb` | A larger file is not a licence text, and the page embeds every text, so it counts as unreadable ("Licence text not found") | spec 087 decision 4, AC-4.3 | | The licences page's ceiling | 1 MiB of HTML | `spec/requests/licences_spec.rb` | Set against 394 KB of gem licence texts (557 KB measured with the suite's bundle, rendered in about 10 ms); a bundle that outgrows it fails the example and raising it is a decision | spec 087 AC-1.9, NFR Performance | @@ -250,6 +259,7 @@ A card image fits its slot with `object-fit: contain`, centred, at the largest s - [Spec 069](../specs/069-deck-shape-panel/spec.md) (issue [#256](https://github.com/plainprogrammer/Collector/issues/256)): decisions 4–7 as they touch the mana value column (its first reader); the panel itself is [collection and lots](collection-and-lots.md)'. - [Research 016](../specs/016-scryfall-hosting-terms/research.md): all (a research note; its findings were ruled on 2026-10-11). - [Spec 018](../specs/018-content-notice-footer/spec.md): all, but AC-2.3 and the parts of FR-2 that [spec 084](../specs/084-version-and-licence-in-app/spec.md) decision 1 supersedes, and FR-2 for the scanner page, which [spec 082](../specs/082-scanner-capture-always-in-view/spec.md) decision 16 supersedes (the notice's place there; the rest of spec 082 is [scanner](scanner.md)'s). [Spec 084](../specs/084-version-and-licence-in-app/spec.md) (issue [#374](https://github.com/plainprogrammer/Collector/issues/374)): decisions 1, 2, 11 and 12 and the stories' criteria that touch the footer (the build facts' own rules are [release and deploy](release-and-deploy.md)'), but on the scanner page, where spec 082 decision 16 puts the small print in the sheets' foot. [Spec 019](../specs/019-uncropped-card-images/spec.md): all. [Spec 023](../specs/023-signed-out-catalog-browsing/spec.md): all but decision 7's wording of the not-loaded notice, which is [catalog operations](catalog-operations.md)'. +- [Spec 091](../specs/091-licence-check-in-ci/spec.md) (issue [#402](https://github.com/plainprogrammer/Collector/issues/402)): all, as far as the policy file, the check and its rules go; [ADR 0031](../adr/0031-a-dependency-licence-is-a-decision.md). - [Spec 087](../specs/087-third-party-licences-in-app/spec.md) (issue [#376](https://github.com/plainprogrammer/Collector/issues/376)): all, as far as the page, the line's third sentence and its guard; its smoke check is [release and deploy](release-and-deploy.md)'s. - [Spec 073](../specs/073-card-page-deck-stats/spec.md) (issue [#50](https://github.com/plainprogrammer/Collector/issues/50)): decision 4's place of "In decks" in the card page's order, decision 1's and AC-3.1's rule that a visitor's page reads no collection table; the stats and the section themselves are [collection and lots](collection-and-lots.md)'. - [Spec 057](../specs/057-deck-design-rules/spec.md): decision 12 (the design system is the project's own, so the notes that said an override waits for a re-export are retired). @@ -268,3 +278,4 @@ A card image fits its slot with `object-fit: contain`, centred, at the largest s | 2026-11-13 | Spec 084 (Collector's licence line ends every page's footer, for everyone, worded from the build facts; the footer is always present and every shell footed, superseding spec 018 AC-2.3 and FR-2's "absent"; the shared partial `layouts/notice_lines` a focused page's sheet foot renders too; #374). Written during the build (T4.2) and checked here | [#378](https://github.com/plainprogrammer/Collector/pull/378) | | 2026-11-13 | Spec 087 (the licence line's third sentence and link, and the third-party licences page: who sees it, the sections and a row, "Licence text not found", where the list comes from (the running bundle, the manifest and its texts), the memo, the guard, the page's ceiling; superseding the wording of spec 084's licence-line ACs; #376). Written during the build (T6.1) and checked here | [#412](https://github.com/plainprogrammer/Collector/pull/412) | | 2026-11-13 | Spec 082's notice on the scanner (decision 16, patch 1.0.3): a one-screen focused page has no footer and carries the small print, the licence line last, in its sheets' foot and its noscript notice, the focused-page exception to spec 084 decisions 1 and 11, AC-1.1 and AC-4.3; supersedes spec 018 FR-2 for the scanner page | [#409](https://github.com/plainprogrammer/Collector/pull/409) | +| 2026-11-13 | Spec 091 (the licence policy file `config/licence_policy.yml` and the `licences:check` step of `bin/ci checks`: the allowed list, any one of several licences, named exceptions that go stale and the `all_required` form, every bundle group, the manifest's entries judged, the guard reported with it; `Policy`, `Check`; #402); ADR 0031 | spec 091's pull request | diff --git a/docs/architecture/release-and-deploy.md b/docs/architecture/release-and-deploy.md index d6cccc5d..1a2c43ab 100644 --- a/docs/architecture/release-and-deploy.md +++ b/docs/architecture/release-and-deploy.md @@ -55,7 +55,7 @@ How Collector is built into an image, published, deployed and kept safe: the con - **The base** (decision 5, AC-3.3, AC-3.4, AC-3.7): `git merge-base HEAD origin/main`, whatever `HEAD` is, so GitHub's detached pull request merge commit and a tag push are judged by the same rule, unlike `Collector::PushScope`, which refuses a detached `HEAD`. It fails, and never passes, when `origin/main`, the merge base or the full history is missing or the clone is shallow, naming what to fetch, and when a changed file YARD can't parse; an unchanged one YARD can't parse is ignored. After a merge of `main` into the branch, what `main` brought in isn't judged. - **The history it needs:** the `checks` job's checkout has `fetch-depth: 0`, which the check depends on and which stays (decision 5, AC-4.9; `ci.yml:23-26`, `spec/script/upgrade/files_spec.rb`). - **Output and budget** (decisions 5 and 6, AC-3.1, AC-3.5, AC-4.3): one line per offender (`path:line: Name has no comment (added|changed)`, with a hint for a class nested in another namespace), a count, the rule's path (`YardRatchet::RULE`), and nothing written into the checkout. The budget is 30 s; it measured about 0.3 to 0.4 s on a branch that changes no judged file, and 1.07 s for 165 files added against an empty base (spec 056 `verification.md`; the pull request's `checks` log is recorded there when it exists). -- **`bin/ci` takes one optional group, checked before any step runs.** With no argument it runs every step serially in its old order (setup, the migration guard, RuboCop, the YARD build, the comment ratchet, Brakeman, bundler-audit, the importmap audit, RSpec), the suite in one process; `checks` runs all but RSpec; `rspec` runs setup and the whole suite; `rspec N/M` (1 ≤ N ≤ M) runs setup and shard N of M. Any other argument exits non-zero before any step and names the accepted forms (spec 035 decision 4, AC-1.1–AC-1.3, AC-1.6; spec 054 decision 2, AC-2.3; `lib/collector/ci_groups.rb`, `config/ci.rb:5-9`). Locally `bin/ci` and the hook stay serial; only GitHub shards (spec 035 decision 2). +- **`bin/ci` takes one optional group, checked before any step runs.** With no argument it runs every step serially in its old order (setup, the migration guard, RuboCop, the YARD build, the comment ratchet, Brakeman, bundler-audit, the importmap audit, the licence check, RSpec), the suite in one process; `checks` runs all but RSpec; `rspec` runs setup and the whole suite; `rspec N/M` (1 ≤ N ≤ M) runs setup and shard N of M. Any other argument exits non-zero before any step and names the accepted forms (spec 035 decision 4, AC-1.1–AC-1.3, AC-1.6; spec 054 decision 2, AC-2.3; `lib/collector/ci_groups.rb`, `config/ci.rb:5-9`). Locally `bin/ci` and the hook stay serial; only GitHub shards (spec 035 decision 2). - **Every RSpec run of `bin/ci` prints the 10 slowest examples** (`--profile 10`), in a plain run, `bin/ci rspec` and each shard, so a regression like spec 035's parity example shows in every log. It is not a gate: no exit status depends on it (spec 035 decision 8, AC-1.9; `config/ci.rb:12`, `config/ci.rb:21`). - **Shards come from the committed run-time log.** `Collector::CiShard` forms shard N of M through `parallel_tests`' own grouping (`ParallelTests::RSpec::Runner.tests_in_groups`) by run time from `spec/support/runtime_log.txt`, with `allowed_missing_percent: 100`; without the log it groups by file size and `bin/ci` prints that balance may be worse. The groups depend only on the commit and the log, never on the machine's cores, and the M shards hold every spec file exactly once (spec 035 decision 6, AC-1.5, AC-2.6; `lib/collector/ci_shard.rb`, `config/ci.rb:19`). A stale log changes only balance, never which files run: a file missing from it gets the average time and a deleted one is ignored (spec 035 decision 6, NFR Reliability). Refresh it with `script/ci/record_runtime_log`, a serial run that keeps only the `path:seconds` lines, sorted by path, when shard times on GitHub differ by more than a minute or spec files are added or removed in bulk (spec 035 decision 6; `script/ci/record_runtime_log`). - **Each shard prints its seed and a replay before its examples run**: the file count, the grouping, the seed (`SEED`, or a random one chosen before the run) and `CI=true bin/rspec --seed S --profile 10 `, which runs the same files in the same order with the same seed. `CI` is set because `config/environments/test.rb` eager-loads when it is present, as it is in `CI.run` and on GitHub. A shard runs in one process against `storage/test.sqlite3` (spec 035 decision 6, AC-1.4, AC-2.3; `config/ci.rb:14-22`, `lib/collector/ci_shard.rb:25`). @@ -308,6 +308,7 @@ The proof stops each instance it boots and compares databases (the previous rele - ADRs [0008](../adr/0008-public-images-from-the-private-repository.md), [0009](../adr/0009-image-tags-and-release-channels.md), [0010](../adr/0010-native-multi-architecture-image-builds.md), [0011](../adr/0011-own-ghcr-cleanup-script.md), [0016](../adr/0016-sqlite-in-every-environment.md), [0022](../adr/0022-stored-import-upload-retention.md), [0024](../adr/0024-backup-with-vacuum-into.md), [0029](../adr/0029-prices-database-held-by-the-backup.md); and, for the image's contents, [0012](../adr/0012-decode-art-images-with-imagemagick.md) and [0013](../adr/0013-active-storage-variants-with-vips.md). - [Spec 054](../specs/054-adopt-yard/spec.md) (the image's ignore rules for the API documentation and the YARD step of `bin/ci`) and [ADR 0027](../adr/0027-yard-for-api-documentation.md). - [Spec 056](../specs/056-yard-comment-ratchet/spec.md) (the comment ratchet step), issue [#264](https://github.com/plainprogrammer/Collector/issues/264). +- [Spec 091](../specs/091-licence-check-in-ci/spec.md) (AC-4.1, AC-4.5: the licence check is a `bin/ci checks` step after the importmap audit, with no workflow change; its rules are in [catalog core](catalog-core.md)), issue [#402](https://github.com/plainprogrammer/Collector/issues/402). - [Spec 084](../specs/084-version-and-licence-in-app/spec.md): decisions 3 to 7, 9 and 10 (the build facts in the image, what each build shows, the cleaning rules, the smoke check, the hand-built paths, the reader), issue [#374](https://github.com/plainprogrammer/Collector/issues/374); the licence line they feed is catalog core's. [Spec 087](../specs/087-third-party-licences-in-app/spec.md) (issue [#376](https://github.com/plainprogrammer/Collector/issues/376)): AC-1.3, AC-6.1, AC-6.2, the smoke check of the licences page; the page itself is catalog core's. - Reference: [`docs/releasing.md`](../releasing.md), [`docs/backup-restore.md`](../backup-restore.md), `README.md` ("Self-hosting", "Backup and restore"). - Issues [#31](https://github.com/plainprogrammer/Collector/issues/31) (local builds held the OCR engine twice; fixed, guarded by `bin/image-smoke` and `.dockerignore`), [#42](https://github.com/plainprogrammer/Collector/issues/42) (the upgrade proof), [#145](https://github.com/plainprogrammer/Collector/issues/145) (spike: where CI's time went, and the pre-push hook that stopped gating past its timeout; findings in spec 035's [research.md](../specs/035-ci-shards/research.md)), [#109](https://github.com/plainprogrammer/Collector/issues/109), [#132](https://github.com/plainprogrammer/Collector/issues/132) (actions pinned to SHAs), [#164](https://github.com/plainprogrammer/Collector/issues/164) (the advisory response, spec 036), [#122](https://github.com/plainprogrammer/Collector/issues/122) (the art index read "ready" after a restore; fixed by spec 039) and [#217](https://github.com/plainprogrammer/Collector/issues/217) (the backup smoke's collection-page check failed on arm64; the smoke scripts capture before they test), [#103](https://github.com/plainprogrammer/Collector/issues/103) (adopting YARD; spec 054), [#306](https://github.com/plainprogrammer/Collector/issues/306) (the runners could not resolve Ubuntu's mirrors, so every shard failed installing libvips; the install step falls back to public resolvers) and [#177](https://github.com/plainprogrammer/Collector/issues/177) (the upgrade proof's stop was killed at the end of the grace period: Solid Queue's start-up windows; the proof waits for the queue to start, and the limit for an operator is stated); open: [#21](https://github.com/plainprogrammer/Collector/issues/21). @@ -328,3 +329,4 @@ The proof stops each instance it boots and compares databases (the previous rele | 2026-11-13 | Spec 081 (decisions 6 to 8, 10 and 11: backup format version 2 with `prices.sqlite3`, the two snapshots, the restore's two placings and the killed case (#375), the five-database fresh volume, the upgrade proof's and the backup smoke's prices coverage, named constants); ADR 0029 | [#405](https://github.com/plainprogrammer/Collector/pull/405) | | 2026-11-13 | Spec 087 (`bin/image-smoke` reads the third-party licences page: 200 before any user exists, the `tesseract.js` and `turbo-rails` rows, a gem's licence file under its own name, no `yard` row, and the sign-in page's link; no `Dockerfile` or workflow change; #376). Written during the build (T6.1) and checked here | [#412](https://github.com/plainprogrammer/Collector/pull/412) | | 2026-11-13 | Spec 086 (decisions 1, 2, 4, 5, 6 and 7, FR-1 to FR-4: the start check before `db:prepare` on a server start, what makes a primary "there", the message and exit status, the restore's changed advice, the smoke leg that proves it on the image; #375) | [#415](https://github.com/plainprogrammer/Collector/pull/415) | +| 2026-11-13 | Spec 091 (the licence check is a `bin/ci checks` step after the importmap audit, with no workflow change; #402) | spec 091's pull request | diff --git a/docs/lessons/2026-11-13-an-exception-for-a-dual-gem-reads-as-stale.md b/docs/lessons/2026-11-13-an-exception-for-a-dual-gem-reads-as-stale.md new file mode 100644 index 00000000..7abbb9e4 --- /dev/null +++ b/docs/lessons/2026-11-13-an-exception-for-a-dual-gem-reads-as-stale.md @@ -0,0 +1,24 @@ +--- +date: 2026-11-13 +spec: 091 (#402) +tags: [licences, ci, policy] +--- + +# Lesson: an exception for a gem that passes the any-one-of rule reads as stale + +## Context + +Spec 091's licence check passes a gem that declares several licences when any one is allowed. `vcr` declares `Hippocratic-2.1` and `MIT`, and the maintainer ruled it a named exception because its `MIT` is unverified. + +## What happened + +Under the first draft's rules, `vcr` passed on `MIT`, so an ordinary exception for it was "allowed without it" and the check called it stale. An exception could not express "don't trust the allowed alternative". Spec 0.5.0 added `all_required: true`: every licence of such a gem must be allowed, so the gem fails without the exception, and the exception goes stale only when every licence is allowed. + +## What to do next time + +- When a rule has an "any one passes" clause, ask what an exception means for a component that already passes by it, before writing the stale rule. +- Say why a non-obvious form exists in the spec and in the file's header, as spec 091 decision 3 and `config/licence_policy.yml` do: the next exception will be copied from it. + +## Signals to watch for + +- A "stale when it would pass without it" rule beside a rule that lets a component through on one of several values. diff --git a/docs/lessons/2026-11-13-rake-exit-in-an-in-process-spec.md b/docs/lessons/2026-11-13-rake-exit-in-an-in-process-spec.md new file mode 100644 index 00000000..53d63c33 --- /dev/null +++ b/docs/lessons/2026-11-13-rake-exit-in-an-in-process-spec.md @@ -0,0 +1,24 @@ +--- +date: 2026-11-13 +spec: 091 (#402) +tags: [rake, specs, plans] +--- + +# Lesson: a rake task that calls `exit` ends the RSpec process unless the spec rescues it + +## Context + +Spec 091's `licences:check` task prints its problems and calls `exit 1`, like `start:check`. Its spec runs the task in-process, so a failing run is observed without a child process. + +## What happened + +The plan's first rake example expected the task to pass and said that a problem "fails the example". RSpec doesn't rescue `SystemExit`, so a problem would have ended the whole process, with no summary, and taken a CI shard down. The plan review (Fable, read-only) found it before any code was written. The spec now has a `run_check` helper that swaps `$stdout` and `$stderr`, rescues `SystemExit` and returns the output and the status, and the passing example runs against a policy that allows every licence the bundle declares, so a licence change fails the step in `bin/ci checks` and not a shard. + +## What to do next time + +- Run a task that can `exit` through a helper that rescues `SystemExit` and returns the status, never through a bare `invoke`. +- Judge the real tree in the CI step, not in a spec that a licence or dependency change can break in a shard. + +## Signals to watch for + +- A task spec whose passing example calls `Rake::Task[...].invoke` on a task with an `exit` branch. diff --git a/docs/roadmap.md b/docs/roadmap.md index 80069af2..dbf78279 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -191,7 +191,7 @@ Taken between specs. - Spec 035, from #145: `bin/ci` groups, used by GitHub's RSpec shards and by the pre-push hook, which skips RSpec for a documentation-only branch. **Shipped 2026-11-01:** [spec 035](specs/035-ci-shards/spec.md), [PR #149](https://github.com/plainprogrammer/Collector/pull/149), with its verification, review fixes and four shards in a follow-up pull request; GitHub's verdict went from a median 348 s to 140 s. - [#164](https://github.com/plainprogrammer/Collector/issues/164): dependency advisories: Dependabot's security updates for the Ruby gems as one group, a daily scheduled run of both audits on `main`, and the response to a red audit written into the release-and-deploy area doc (found on 2026-11-03, when five Dependabot pull requests failed on advisories for gems that no Dependabot pull request covered). **Done 2026-11-03:** [spec 036](specs/036-advisory-response/spec.md), [PR #163](https://github.com/plainprogrammer/Collector/pull/163); the maintainer's post-merge checks are in its `verification.md`. - [#401](https://github.com/plainprogrammer/Collector/issues/401): contribution policy: a sign-off on every commit, and licence identifiers in the source (accounts design, 2026-11-13). -- [#402](https://github.com/plainprogrammer/Collector/issues/402): CI fails on a dependency whose licence isn't allowed (accounts design, 2026-11-13). +- [#402](https://github.com/plainprogrammer/Collector/issues/402): CI fails on a dependency whose licence isn't allowed (accounts design, 2026-11-13). **In progress:** [spec 091](specs/091-licence-check-in-ci/spec.md): a `bin/ci checks` step judges the gems and the manifest's JavaScript, OCR, font and icon entries against an allowed list in `config/licence_policy.yml`, with named exceptions. - [#122](https://github.com/plainprogrammer/Collector/issues/122): the art index panel and `catalog:status` say "ready" when the finished build's index file is missing (after a restore, or a lost directory). Done when its pull request merges: [spec 039](specs/039-art-index-missing-file/spec.md). - [#374](https://github.com/plainprogrammer/Collector/issues/374): show the running version, the commit it was built from and Collector's licence (AGPL-3.0, with a link to the source of the running build) in the footer on every page; the image carries its build facts and `bin/image-smoke` checks them. **In progress:** [spec 084](specs/084-version-and-licence-in-app/spec.md). - [#376](https://github.com/plainprogrammer/Collector/issues/376): list the licences of the third-party software and assets in the image (the gems, the OCR engine, the JavaScript, the fonts), readable from the app (split from #374 by the maintainer's ruling of 2026-11-13, spec 084 decision 8). **In progress:** [spec 087](specs/087-third-party-licences-in-app/spec.md). diff --git a/docs/specs/091-licence-check-in-ci/plan.md b/docs/specs/091-licence-check-in-ci/plan.md new file mode 100644 index 00000000..cb0cd89c --- /dev/null +++ b/docs/specs/091-licence-check-in-ci/plan.md @@ -0,0 +1,782 @@ +# Implementation Plan: CI Fails on a Dependency Whose Licence Isn't Allowed (spec 091, issue #402) + +**Spec:** `docs/specs/091-licence-check-in-ci/spec.md` (Approved v0.5.0) · **ADR:** [0031](../../adr/0031-a-dependency-licence-is-a-decision.md) (Proposed) · **Issue:** #402 · **Branch:** `091-licence-check-in-ci` · **Created:** 2026-11-13 +**Status:** Approved (Gate B, 2026-11-13, the coordinator under the maintainer's delegation, no second plan round). Written by my main session; plan review round 1 (Fable, read-only): APPROVE WITH CHANGES (0 blocking, 3 major, 8 minor), all fixed in this version. + +## Context + +A `bin/ci checks` step, `bin/rails licences:check`, judges every component of the list spec 087's reader builds (`Collector::ThirdPartyLicences.build`: the gems of the bundle and the manifest's JavaScript, OCR engine, font and icon entries) against `config/licence_policy.yml`, which holds the allowed SPDX ids and the named exceptions (ADR 0031). It also reports spec 087's guard (`Audit`), so a pin with no manifest entry fails the checks job too. Two new classes under `lib/collector/third_party_licences/` (`Policy`, `Check`), one rake task, one policy file, one `config/ci.rb` step. No second reader of the bundle: the one extra input is the lockfile's gem names. + +Facts the tasks rely on, read from the files on `main` at `71212946`: + +- `ThirdPartyLicences::Component` is `Data.define(:name, :version, :identifiers, :home, :copyright, :texts, :note)`; `Section` is `Data.define(:key, :title, :components)`, with `key` one of `:javascript, :ocr, :fonts, :icons, :gems` (`ThirdPartyLicences::TITLES`, page order). `ThirdPartyLicences.load_manifest(path)` is `YAML.safe_load_file(path) || {}` and is public; `Audit.new` takes nine keyword inputs and `Audit#problems` returns sentences, and returns early with "has no javascript entry" for a pin with no entry, before it reads the served file. +- `config/ci.rb`'s `unless selection.group == :rspec` block ends with `step "Security: Importmap vulnerability audit", "bin/importmap audit"`; `spec/config/ci_definition_spec.rb` already places the YARD and ratchet steps by line index. `.claude/settings.json` allows `bin/ci` and `bin/ci *` but no `bin/rails licences:check`, so a session runs the task only through `bin/ci checks` or in-process in a spec (nothing here edits that file). +- Rake task specs live in `spec/tasks/` with `type: :task` and `Rails.application.load_tasks` (`spec/tasks/catalog_rake_spec.rb`); `start.rake` is the model for `warn` plus `exit 1`. +- Measured before: `bin/ci checks` took **35.5 s** total (`46.48s user 6.65s system`) on the author's machine on 2026-11-13 at `71212946`, run with `time bin/ci checks` (the ratchet step, YARD, RuboCop, Brakeman and both audits included). T2.4 measures the same way after. + +## Global Constraints + +- **Repo rules:** `CLAUDE.md`, `.claude/rules/*.md`. Spec files under `lib/` are tagged `type: :model`; task specs `type: :task`; `RSpec/ExampleLength` max 15; `RSpec/MultipleMemoizedHelpers` max 5 (use `def`); multi-expectation examples use `:aggregate_failures`. Prose documents are verified by reading and recorded in `verification.md`, never from RSpec (`.claude/rules/testing.md`). +- **Core names no collectible.** `spec/licences_core_spec.rb` (T4.1) reads whole files, comments included, for `/mtg|magic|wizards|scryfall|fan content/i`: none of those words in the new class files, the rake task or the policy file. +- **One commit per task, test first** (red, implement, green, RuboCop), Conventional Commits with `(#402)`. The trailer is the attribution reminder in force at commit time; a Sonnet implementer's commits name Sonnet 5.5. One git operation per shell call; `git add` and `git commit` separate (the pre-commit hook reads the command text). Every `git grep` ends with `':!.kamal/secrets'`. Never read `.kamal/secrets`, `config/master.key`, `.env*`, `storage/` or any `*.sqlite3`. +- **No agent runs the `sqlite3` command, `bin/rails console`, `bundle add`/`update`, `bin/importmap pin`, or anything else on the ask list, and none runs `bin/rails licences:check` as a Bash command** (it isn't allowed in `.claude/settings.json` and would prompt): run the task in-process in its spec, and the real tree through `bin/ci checks`. Never touch `Dockerfile`, `.github/workflows/**`, `config/deploy*.yml` or `.claude/settings.json`. `config/ci.rb` is a plain edit. +- **Comments:** every public class, module and method gets a summary (the comment ratchet in `bin/ci checks`), with `@param`/`@return` on reusable code, and `@see file:catalog-core.md` and `@see file:0031-a-dependency-licence-is-a-decision.md`. No `{…}` outside YARD links (write paths in backticks). +- **No migration, no export, import or backup change** (the export format stays 4); no gem, no JavaScript; `spec/support/runtime_log.txt` is not refreshed; the existing `Audit`'s rules and `spec/third_party_licences_manifest_spec.rb` are not changed. +- **Run `bin/yard doc` only through `bin/ci checks`** (a plain `bin/yard` isn't on the allow list); an implementer who adds a class runs `bin/ci checks` once at the end of its batch, not per task. +- **Shared test database:** one test run at a time in this worktree; a targeted `bin/rspec ` needs no CI slot, a full `bin/ci` and a push do (the coordinator's). + +## Goal + +A pull request that adds a gem, or a pinned JavaScript package, font or icon set, under a licence the maintainer hasn't allowed fails `bin/ci checks`, with a message naming the dependency, its licence and the policy file; the allowed licences and each exception are recorded in the repository. + +## Pre-implementation gates + +- **Simplicity (≤3 components): pass.** (1) `Policy` (the rules) with `config/licence_policy.yml`; (2) `Check` (the join with spec 087's guard) and the rake task; (3) the `config/ci.rb` step. +- **Anti-abstraction: pass.** `Data` values and `YAML.safe_load_file` through `ThirdPartyLicences.load_manifest`; `Bundler::LockfileParser` called directly; `Check` exists only because the issue's outcome needs the guard and the policy in one step, and it is eight lines. +- **Integration-first: pass.** The rules take injected sections (Phase 1) before the real file (T2.1), the class that joins them (T2.2) and the task (T2.3) before the step (T2.4). + +## Files + +| File | Change | Responsibility | +|---|---|---| +| `lib/collector/third_party_licences/policy.rb` | create | the rules over injected sections, policy data and lockfile names; `for_repository`, `locked_names` | +| `lib/collector/third_party_licences/check.rb` | create | joins an injected audit's problems and the policy's | +| `lib/tasks/licences.rake` | create | `licences:check[policy]`: prints, exits 1 on a problem | +| `config/licence_policy.yml` | create | the seven allowed ids, the exceptions (T2.1; `vcr` in T3.1) | +| `config/ci.rb` | edit | the step (T2.4) | +| `spec/lib/collector/third_party_licences/policy_spec.rb`, `check_spec.rb` | create | the rules by injection | +| `spec/tasks/licences_rake_spec.rb` | create | the task in-process | +| `spec/config/ci_definition_spec.rb`, `spec/licences_core_spec.rb` | edit | the step's place; the core files | +| `docs/architecture/catalog-core.md`, `release-and-deploy.md`, `CLAUDE.md`, `.claude/rules/testing.md`, `.claude/rules/conventions.md`, `docs/roadmap.md`, `docs/specs/README.md`, `docs/adr/README.md` | edit | Story 5 | +| `docs/specs/091-licence-check-in-ci/verification.md` | create | evidence, timings, the review | + +## Phase 1: The rules + +**Implements** FR-2 (the class); **satisfies** AC-1.1 to AC-1.6, AC-2.1 to AC-2.3, AC-3.1 to AC-3.3, AC-3.5, AC-3.6. One implementer takes T1.1 to T1.3 (three commits). + +### T1.1 Policy: shape and judging (AC-1.1 to AC-1.6, AC-2.1, AC-2.2, AC-3.3 shape) + +Test first, `spec/lib/collector/third_party_licences/policy_spec.rb`; it fails with `uninitialized constant ...::Policy`. + +```ruby +require "rails_helper" +require "tmpdir" + +# Spec 091 decisions 1, 2, 5 and 6 and Stories 1 and 2: the rules over injected sections and policy data, so each fails on a +# small fixture and passes on the baseline. The real tree is judged by the bin/ci checks step (AC-4.2). +RSpec.describe Collector::ThirdPartyLicences::Policy, type: :model do + let(:path) { "config/licence_policy.yml" } + + def component(name, *identifiers, version: "1.0", texts: []) + Collector::ThirdPartyLicences::Component.new(name:, version:, identifiers:, home: nil, copyright: nil, texts:, note: nil) + end + + def section(key, *components, title: key.to_s.capitalize) + Collector::ThirdPartyLicences::Section.new(key:, title:, components:) + end + + def data(allowed: %w[MIT BSD-2-Clause Ruby], exceptions: []) + { "allowed" => allowed.map { { "id" => it, "reason" => "a reason" } }, "exceptions" => exceptions } + end + + def problems_for(*sections, data: data(), locked: []) + described_class.new(data:, path:, sections:, locked_names: locked).problems + end + + it "passes a bundle of allowed licences, a dual among them (AC-1.5)" do + sections = [ section(:gems, component("rake", "MIT"), component("erb", "BSD-2-Clause", "Ruby")) ] + + expect(problems_for(*sections)).to be_empty + end + + it "fails a gem under a licence off the list, naming the gem, its version, the licence and the policy file (AC-1.1)", :aggregate_failures do + problems = problems_for(section(:gems, component("viral", "GPL-3.0-only", version: "2.1.0"))) + + expect(problems.size).to eq(1) + expect(problems.first).to include("gem viral 2.1.0", "GPL-3.0-only", path) + end + + it "passes a gem with any one allowed licence and fails one with none, listing every one (AC-1.2)", :aggregate_failures do + expect(problems_for(section(:gems, component("dual", "GPL-2.0-only", "MIT")))).to be_empty + expect(problems_for(section(:gems, component("copyleft", "GPL-2.0-only", "Artistic-1.0-Perl"))).first) + .to include("GPL-2.0-only", "Artistic-1.0-Perl") + end + + it "fails a gem that declares no licence, and one whose string is not an exact allowed id (AC-1.3, AC-1.4)", :aggregate_failures do + expect(problems_for(section(:gems, component("bare"))).first).to include("gem bare 1.0", "no licence") + expect(problems_for(section(:gems, component("filed", texts: [ [ "LICENSE", "text" ] ]))).first).to include("gem filed 1.0", "no licence") + %w[MIT\ License Apache\ 2.0 MIT\ OR\ Apache-2.0].each do |string| + expect(problems_for(section(:gems, component("odd", string))).first).to include(string) + end + end + + it "reports every offender once, in the order the sections list them, each naming the file (AC-1.6)", :aggregate_failures do + problems = problems_for(section(:javascript, component("alpha", "GPL-3.0-only")), + section(:gems, component("charlie", "AGPL-3.0-only"), component("bravo", "LGPL-3.0-only"))) + + expect(problems.size).to eq(3) + expect(problems).to all(include(path)) + expect(problems.map { it[/alpha|bravo|charlie/] }).to eq(%w[alpha charlie bravo]) + end + + it "judges a manifest entry of every section by its identifiers, with decision 2's rule (AC-2.1, AC-2.2)", :aggregate_failures do + %i[javascript ocr fonts icons].each do |key| + problems = problems_for(section(key, component("pkg", "GPL-3.0-only"), title: "Kind")) + + expect(problems.first).to include("Kind", "\"pkg\"", "GPL-3.0-only", path) + end + expect(problems_for(section(:fonts, component("two", "OFL-1.1", "MIT")))).to be_empty + expect(problems_for(section(:icons, component("none"))).first).to include("no licence") + end + + describe "the policy's own shape (AC-3.3)" do + def shape_problems(value) = problems_for(section(:gems), data: value) + + it "names an allowed entry without an id or a reason, a duplicate id, and a data that is not a mapping", :aggregate_failures do + expect(shape_problems({ "allowed" => [ { "id" => "MIT" } ] }).first).to include(path, "allowed entry 1") + expect(shape_problems({ "allowed" => [ { "reason" => "r" } ] }).first).to include("allowed entry 1") + expect(shape_problems({ "allowed" => [ { "id" => "MIT", "reason" => "r" }, { "id" => "MIT", "reason" => "r" } ] }).first).to include("MIT", "twice") + expect(shape_problems([ "MIT" ]).first).to include(path, "mapping") + end + + it "names an exception missing its gem, licences or reason, and a duplicate gem", :aggregate_failures do + bad = { "gem" => "x", "licences" => [], "reason" => "r" } + ok = { "gem" => "y", "licences" => [ "GPL-3.0-only" ], "reason" => "r" } + + expect(shape_problems(data(exceptions: [ bad ])).first).to include("exception entry 1", "(x)") + expect(shape_problems(data(exceptions: [ ok.except("reason") ])).first).to include("exception entry 1") + expect(shape_problems(data(exceptions: [ ok, ok ])).first).to include("gem y", "twice") + end + + it "judges nothing while the policy is malformed (it would report every gem)" do + expect(problems_for(section(:gems, component("viral", "GPL-3.0-only")), data: { "allowed" => "MIT" }).size).to eq(1) + end + end +end +``` + +Implementation, `lib/collector/third_party_licences/policy.rb` (T1.1 builds everything but the exceptions, which T1.2 adds; the class below is the final shape of T1.1): + +```ruby +module Collector + class ThirdPartyLicences + # Judges the components of {ThirdPartyLicences.build}'s sections against the allowed list and the named exceptions in + # `config/licence_policy.yml` (spec 091), as a list of problems each naming its offender, the licence and the file. + # + # Every input is injected, so a spec shows each rule failing on a small fixture; `for_repository` (T1.3) builds the real ones + # and the `licences:check` task prints the problems. A component passes if any one of its identifiers is allowed (spec 091 + # decision 2); one with none, or with a string that is not exactly an allowed id, fails (decision 5). Nothing here reads + # a gem or a licence file: the one reader is {ThirdPartyLicences} (no second reader of the bundle). + # + # @see file:catalog-core.md + # @see file:0031-a-dependency-licence-is-a-decision.md + class Policy + # The policy file, relative to `Rails.root`. + PATH = "config/licence_policy.yml".freeze + + # The decision the failure message points at. + DECISION = "ADR 0031".freeze + + # A named exception, as the rules use it: the `gem`, its `licences` (sorted, a set) and whether its list is `all_required`. + # Its `reason` is for the reader of the file; no rule reads it. + Exemption = Data.define(:gem, :licences, :all_required) + + # @param data [Hash{String=>Array}, Object] the loaded policy: `allowed` (each `id`, `reason`) and `exceptions` + # (each `gem`, `licences`, `reason`, optional `all_required`); anything but a mapping is a problem, not a crash + # @param path [String] the policy's path, as the messages print it + # @param sections [Array] what {ThirdPartyLicences.build} returns + # @param locked_names [Array] the gem names the lockfile holds, once each + def initialize(data:, path:, sections:, locked_names:) + @data = data + @path = path.to_s + @sections = sections + @locked_names = locked_names + end + + # How many components the sections hold, which is what a passing run reports it judged. + # + # @return [Integer] + def component_count = components.size + + # What is off the policy, one sentence per offender, each naming it, its licence and the policy file. + # + # Pure of side effects. While the policy's own shape is wrong it reports only that, because judging against a + # malformed list would blame every gem. + # + # @return [Array] empty when everything agrees; the policy's shape problems alone when it is malformed + def problems + shape = shape_problems + return shape if shape.any? + + judge_problems + exemption_problems + end + + private + + def components = @sections.flat_map { |section| section.components.map { |component| [ section, component ] } } + + def raw_allowed = Array(@data["allowed"]) + + def raw_exceptions = Array(@data["exceptions"]) + + def allowed_ids = @allowed_ids ||= raw_allowed.filter_map { it["id"] if it.is_a?(Hash) } + + def filled?(value) = value.is_a?(String) && !value.strip.empty? + + def shape_problems + return [ "#{@path} isn't a mapping with `allowed` and `exceptions`" ] unless @data.is_a?(Hash) + + allowed_shape + exception_shape + end + + def allowed_shape + bad = raw_allowed.each_with_index.reject { |entry, _| entry.is_a?(Hash) && filled?(entry["id"]) && filled?(entry["reason"]) } + .map { |_, index| "#{@path}: allowed entry #{index + 1} needs an `id` and a `reason`" } + bad + repeated(allowed_ids).map { "#{@path}: allowed id #{it} is listed twice" } + end + + def exception_shape + bad = raw_exceptions.each_with_index.reject { |entry, _| well_formed_exception?(entry) } + .map { |entry, index| "#{@path}: exception entry #{index + 1} (#{entry.is_a?(Hash) ? entry["gem"] : "not a mapping"}) needs a `gem`, `licences` (a list) and a `reason`" } + bad + repeated(raw_exceptions.filter_map { it["gem"] if it.is_a?(Hash) }).map { "#{@path}: the exception for gem #{it} is listed twice" } + end + + def well_formed_exception?(entry) + entry.is_a?(Hash) && filled?(entry["gem"]) && filled?(entry["reason"]) && + entry["licences"].is_a?(Array) && entry["licences"].any? && entry["licences"].all? { filled?(it) } + end + + def repeated(values) = values.tally.select { |_, count| count > 1 }.keys + + def judge_problems + components.flat_map { |section, component| judge(section, component) } + end + + # The sentence(s) for one component: none when it passes. + def judge(section, component) + ids = component.identifiers.uniq.sort + return [ "#{label(section, component)} declares no licence; #{@path} can judge only a declared SPDX id (#{DECISION})" ] if ids.empty? + + allowed?(ids, strict: false) ? [] : [ disallowed(section, component, ids) ] + end + + def allowed?(ids, strict:) + strict ? ids.all? { allowed_ids.include?(it) } : ids.any? { allowed_ids.include?(it) } + end + + def label(section, component) + section.key == :gems ? "gem #{component.name} #{component.version}" : "#{section.title} entry #{component.name.inspect} (#{component.version})" + end + + def disallowed(section, component, ids) + way_out = section.key == :gems ? "allow the licence there, or list the gem under `exceptions` with its reason" : "allow the licence there, or drop the dependency" + "#{label(section, component)} is under #{ids.join(", ")}, none of which is on the allowed list in #{@path}: #{way_out} (#{DECISION})" + end + + # T1.2 fills this in; until then there is none. + def exemption_problems = [] + end + end +end +``` + +Verify green (`bin/rspec spec/lib/collector/third_party_licences/policy_spec.rb`), `bin/rubocop -a`. Commit `feat(licences): the policy's rules over injected sections (#402)`. + +### T1.2 Exceptions, staleness and `all_required` (AC-3.1, AC-3.2, AC-3.5 first clause, AC-3.6, AC-2.3) + +Add to `policy_spec.rb` (red first; the AC-2.3 and AC-3.5 examples already pass on T1.1's code, so they are pins, not red): + +```ruby + describe "exceptions (decision 3)" do + let(:viral) { component("viral", "GPL-3.0-only") } + let(:exception) { { "gem" => "viral", "licences" => [ "GPL-3.0-only" ], "reason" => "a tool, never loaded" } } + + def judged(exceptions, *components, locked: %w[viral], allowed: %w[MIT]) + problems_for(section(:gems, *components), data: data(allowed:, exceptions:), locked:) + end + + it "lets a gem through that an exception names with exactly its licences, in any order (AC-3.1)", :aggregate_failures do + expect(judged([ exception ], viral)).to be_empty + two = { "gem" => "two", "licences" => %w[B A], "reason" => "r" } + expect(judged([ two ], component("two", "A", "B"), locked: %w[two])).to be_empty + end + + it "calls an exception stale when the lockfile no longer names the gem, the gem's licences changed, or it is allowed without it (AC-3.2)", :aggregate_failures do + expect(judged([ exception ], locked: [])).to contain_exactly(a_string_including("viral", "lockfile")) + changed = judged([ exception ], component("viral", "AGPL-3.0-only")) + expect(changed).to include(a_string_including("viral", "AGPL-3.0-only", "GPL-3.0-only", "stale")) + expect(changed.size).to eq(2) # the stale exception and the disallowed licence + expect(judged([ exception ], viral, allowed: %w[MIT GPL-3.0-only])).to contain_exactly(a_string_including("viral", "allowed without it")) + end + + it "judges nothing of a gem the lockfile names and the bundle doesn't hold, and calls no exception for it stale (AC-3.5)" do + expect(judged([ exception ], locked: %w[viral])).to be_empty + end + + it "applies to gems only: a manifest entry of the same name is judged by the list (AC-2.3)" do + problems = problems_for(section(:javascript, viral, title: "JavaScript"), data: data(exceptions: [ exception ]), locked: %w[viral]) + + expect(problems).to include(a_string_including("JavaScript entry", "GPL-3.0-only")) + end + + describe "`all_required`" do + let(:vcr) { component("vcr", "Hippocratic-2.1", "MIT") } + let(:strict) { { "gem" => "vcr", "licences" => %w[Hippocratic-2.1 MIT], "all_required" => true, "reason" => "its MIT is not trusted" } } + + it "passes a dual gem as its allowed id with no exception, and by name with the exception (AC-3.6)", :aggregate_failures do + expect(judged([], vcr, locked: %w[vcr])).to be_empty + expect(judged([ strict ], vcr, locked: %w[vcr])).to be_empty + end + + it "fails a gem whose exception's licences no longer match, because every licence must then be allowed (AC-3.6)", :aggregate_failures do + problems = judged([ strict ], component("vcr", "Hippocratic-2.1", "MIT", "BSD-2-Clause"), locked: %w[vcr], allowed: %w[MIT BSD-2-Clause]) + + expect(problems).to include(a_string_including("vcr", "Hippocratic-2.1", "every licence")) + expect(problems).to include(a_string_including("stale")) + end + + it "calls the exception stale once every licence is allowed (AC-3.6)" do + expect(judged([ strict ], vcr, locked: %w[vcr], allowed: %w[MIT Hippocratic-2.1])).to contain_exactly(a_string_including("vcr", "stale", "allowed without it")) + end + end + end +``` + +Implementation: replace the T1.1 stand-ins. + +```ruby + + def exemptions + @exemptions ||= raw_exceptions.map do |entry| + Exemption.new(gem: entry["gem"], licences: entry["licences"].uniq.sort, all_required: entry["all_required"] == true) + end + end + + def exemption_for(section, component) + exemptions.find { it.gem == component.name } if section.key == :gems + end +``` + +`judge` becomes: + +```ruby + def judge(section, component) + ids = component.identifiers.uniq.sort + return [ "#{label(section, component)} declares no licence; #{@path} can judge only a declared SPDX id (#{DECISION})" ] if ids.empty? + + exemption = exemption_for(section, component) + return [] if exemption && ids == exemption.licences + + allowed?(ids, strict: exemption&.all_required) ? [] : [ disallowed(section, component, ids, strict: exemption&.all_required) ] + end +``` + +`disallowed` takes `strict:` and reads `"#{label} is under #{ids.join(", ")}, which includes #{(ids - allowed_ids).join(", ")}, off the allowed list in #{@path} (every licence of this gem must be allowed): #{way_out} (#{DECISION})"` when strict, else the T1.1 text. `exemption_problems`: + +```ruby + def exemption_problems + gems = @sections.find { it.key == :gems }&.components.to_a + exemptions.flat_map do |exemption| + component = gems.find { it.name == exemption.gem } + if !@locked_names.include?(exemption.gem) + [ "#{@path}: the exception for gem #{exemption.gem} is stale: the lockfile no longer names it (#{DECISION})" ] + elsif component.nil? + [] # in the lockfile and not installed (a run with a group left out): neither judged nor stale + else + stale_problem(exemption, component.identifiers.uniq.sort) + end + end + end + + def stale_problem(exemption, ids) + if ids != exemption.licences + [ "#{@path}: the exception for gem #{exemption.gem} is stale: it declares #{ids.join(", ")} now and the exception covers #{exemption.licences.join(", ")}; review it (#{DECISION})" ] + elsif allowed?(ids, strict: exemption.all_required) + [ "#{@path}: the exception for gem #{exemption.gem} is stale: the gem is allowed without it (#{DECISION})" ] + else + [] + end + end +``` + +Green, RuboCop, commit `feat(licences): named exceptions that go stale, and the all_required form (#402)`. + +### T1.3 Reading the real inputs (AC-3.3 unreadable file, AC-3.5 second clause) + +Red first, in `policy_spec.rb`: + +```ruby + describe ".for_repository" do + it "reads the lockfile's gem names once each, with no `bundler` (AC-3.5)", :aggregate_failures do + names = described_class.locked_names + + expect(names).to include("rails", "sqlite3") + expect(names).to eq(names.uniq) + expect(names).not_to include("bundler") + end + + it "raises for a policy file it can't read, and reports a file that isn't a mapping (AC-3.3)", :aggregate_failures do + Dir.mktmpdir("licence-policy") do |dir| + list = File.join(dir, "list.yml") + File.write(list, "- MIT\n") + + expect { described_class.for_repository(policy_path: File.join(dir, "absent.yml"), sections: []) }.to raise_error(SystemCallError) + expect(described_class.for_repository(policy_path: list, sections: []).problems.first).to include(list.delete_prefix("#{Rails.root}/"), "mapping") + end + end + end +``` +The class comment's `for_repository` in backticks becomes the YARD link `{.for_repository}` in this task, once the method exists. Implement: + +```ruby + class << self + # The policy of this repository: the file at `policy_path`, the sections of the running bundle and manifest, and the + # lockfile's gem names. + # + # Reads the policy and, unless `sections` is given, every licence file {ThirdPartyLicences.build} reads (about 400 KB). + # + # @param policy_path [String, Pathname] the policy file; default `config/licence_policy.yml` + # @param sections [Array] the components to judge; default the real ones + # @return [Policy] + # @raise [SystemCallError] when the policy file can't be read + # @raise [Psych::Exception] when it isn't valid YAML or holds more than plain data + def for_repository(policy_path: Rails.root.join(PATH), sections: ThirdPartyLicences.build.sections) + new(data: ThirdPartyLicences.load_manifest(policy_path), path: display_path(policy_path), sections:, locked_names:) + end + + # The gem names the lockfile holds, once each. + # + # A gem the lockfile names for several platforms repeats there (`ffi`, `sqlite3`); `bundler` is in the bundle and + # not in the lockfile, so it is never among them. + # + # @return [Array] + def locked_names + Bundler::LockfileParser.new(Bundler.default_lockfile.read).specs.map(&:name).uniq + end + + private + + # The path as a message prints it: relative to the application when it is inside it. + def display_path(path) = path.to_s.delete_prefix("#{Rails.root}/") + end +``` +Commit `feat(licences): the policy reads the real file, sections and lockfile names (#402)`. + +**Phase 1 verification:** `bin/rspec spec/lib/collector/third_party_licences/policy_spec.rb` green (about 20 examples); `bin/rubocop` clean. + +## Phase 2: The real policy, the join, the task and the step + +**Implements** FR-1, FR-2 (`for_repository`), FR-3, FR-4; **satisfies** AC-3.4 (first two exceptions), AC-4.1 to AC-4.5. One implementer takes T2.1 and T2.2, another T2.3 and T2.4 (or one takes all four, four commits). + +### T2.1 `config/licence_policy.yml` + +No code test (AC-3.4 is verified by reading; T2.3's run proves it judges). The file, with the header and the reasons copied exactly (no collectible is named): + +```yaml +# The licences Collector's dependencies may be under, and the dependencies that are let through by name (spec 091, ADR 0031). +# `bin/ci checks` fails a gem, a pinned JavaScript package, an OCR package, a font or an icon set under a licence that is not +# in `allowed` and has no entry in `exceptions`. Adding to either list is a decision for the maintainer: say why in `reason`, +# and put the change in a pull request of its own or beside the dependency it is for. +# +# A component with several licences passes if any one is allowed. A licence is compared exactly as an SPDX id: "MIT License" +# or an expression in one string fails. An exception names a gem, the licences it was reviewed under (a set) and a reason; it +# goes stale, and fails the check, when the lockfile no longer names the gem, the gem declares other licences, or the gem is +# allowed without it. `all_required: true` marks a gem whose list of licences is not a choice, so every one must be allowed. Why +# it exists: a gem such as vcr declares Hippocratic-2.1 and MIT, and the rule "any one allowed" would pass it on MIT, so an +# ordinary exception for it would read as stale (the gem is allowed without it). With `all_required` the gem fails without the +# exception, the exception is what lets it through, and it goes stale only when every one of the gem's licences is allowed. +# Copy this form only for a gem whose declared alternatives you do not trust; an ordinary exception is for a gem that fails. + +allowed: + - { id: MIT, reason: "Permissive, notice only: the copyright line and licence text travel with the copies, which the third-party licences page carries (spec 087)" } + - { id: BSD-2-Clause, reason: "Permissive, notice only; carried by the third-party licences page" } + - { id: BSD-3-Clause, reason: "Permissive, notice only, and a clause against using the authors' names to endorse; carried by the third-party licences page" } + - { id: Apache-2.0, reason: "Permissive, with a patent grant and a notice file that travels with the copies; carried by the third-party licences page" } + - { id: ISC, reason: "Permissive, notice only; the icons' licence" } + - { id: Ruby, reason: "Ruby's own licence, declared by the default gems, usually beside BSD-2-Clause" } + - { id: OFL-1.1, reason: "The licence of the self-hosted typefaces: the licence text travels with the font files, which are unmodified copies of their packages" } + +exceptions: + - gem: bundler-audit + licences: [GPL-3.0-or-later] + reason: "A command-line audit run by bin/ci; the application never loads it, and it leaves the image when #408 lands" + - gem: brakeman + licences: [Brakeman Public Use License] + reason: "A static analyser run by bin/ci and the commit hook; the application never loads it, and it leaves the image when #408 lands" +``` + +Commit `feat(licences): the allowed licences and the first two exceptions (#402)`. + +### T2.2 `Check` (AC-4.4) + +`spec/lib/collector/third_party_licences/check_spec.rb`, red first: + +```ruby +require "rails_helper" +require "tmpdir" + +# Spec 091 decision 7, AC-4.4: Check joins an injected audit's problems and the policy's, the guard's first. +RSpec.describe Collector::ThirdPartyLicences::Check, type: :model do + def policy_reporting(*problems) = instance_double(Collector::ThirdPartyLicences::Policy, problems:, component_count: 3) + + def audit_reporting(*problems) = instance_double(Collector::ThirdPartyLicences::Audit, problems:) + + it "lists the audit's problems first and the policy's second, and counts the components the policy judged", :aggregate_failures do + check = described_class.new(audit: audit_reporting("pin has no entry"), policy: policy_reporting("gem x is under GPL")) + + expect(check.problems).to eq([ "pin has no entry", "gem x is under GPL" ]) + expect(check.component_count).to eq(3) + end + + it "carries the guard's sentence for an importmap pin with no manifest entry (decision 7)" do + pin = Collector::ThirdPartyLicences::Audit::Pin.new(name: "lodash", asset: "/x/vendor/javascript/lodash.js") + audit = Collector::ThirdPartyLicences::Audit.new(manifest: {}, text_dir: Dir.tmpdir, specs: [], ocr_digests: [], pins: [ pin ], + pin_directories: [], app_javascript: "/x/app/javascript", font_files: {}, icon_files: []) + + expect(described_class.new(audit:, policy: policy_reporting).problems).to eq([ "importmap pin lodash has no javascript entry" ]) + end +end +``` + +Implementation, `lib/collector/third_party_licences/check.rb`: + +```ruby +module Collector + class ThirdPartyLicences + # Everything the `licences:check` step reports: spec 087's guard (the manifest, the pins and the bundle agree) and the + # policy (every component is under an allowed licence), in one list, so a pull request that adds an importmap pin with + # no entry fails `bin/ci checks` as well as the suite (spec 091 decision 7). + # + # @see file:catalog-core.md + class Check + # The check of this repository: the real guard and the real policy. + # + # Reads the manifest, every licence file and the lockfile; needs the application booted (the importmap). + # + # @param policy_path [String, Pathname] the policy file; default `config/licence_policy.yml` + # @return [Check] + # @raise [SystemCallError] when the policy or the manifest can't be read + # @raise [Psych::Exception] when either isn't plain YAML + def self.for_repository(policy_path: Rails.root.join(Policy::PATH)) + new(audit: Audit.for_repository, policy: Policy.for_repository(policy_path:)) + end + + # @param audit [#problems] spec 087's guard, or a stand-in + # @param policy [#problems, #component_count] the policy, or a stand-in + def initialize(audit:, policy:) + @audit = audit + @policy = policy + end + + # The guard's problems, then the policy's. + # + # @return [Array] empty when both agree + def problems = @audit.problems + @policy.problems + + # How many components the policy judged. + # + # @return [Integer] + def component_count = @policy.component_count + end + end +end +``` + +Commit `feat(licences): Check joins the guard's problems and the policy's (#402)`. + +### T2.3 `licences:check` (AC-4.2, AC-4.3) + +`spec/tasks/licences_rake_spec.rb`, red first (`Don't know how to build task 'licences:check'`): + +```ruby +require "rails_helper" +require "rake" +require "tmpdir" + +# Spec 091 FR-3, AC-4.2 and AC-4.3. The step judges the repository's own policy in `bin/ci checks` (evidence in verification.md); +# here the task runs in-process, so a licence change in the bundle can't take a shard down: the passing run uses a policy that +# allows every licence the list declares, the failing one a policy that allows nothing. +RSpec.describe "licences rake task", type: :task do # rubocop:disable RSpec/DescribeClass -- rake tasks have no class + before { Rails.application.load_tasks unless Rake::Task.task_defined?("licences:check") } + + after { Rake::Task["licences:check"].reenable } + + # Runs the task with its output captured and `exit` rescued (RSpec doesn't rescue SystemExit, so an unrescued exit would end + # the whole process), as [standard output, standard error, exit status]. + def run_check(*args) + out = StringIO.new + err = StringIO.new + original = [ $stdout, $stderr ] + $stdout, $stderr = out, err + status = begin + Rake::Task["licences:check"].invoke(*args) + 0 + rescue SystemExit => error + error.status + end + [ out.string, err.string, status ] + ensure + $stdout, $stderr = original + end + + def with_policy(yaml) + Dir.mktmpdir("licence-policy") do |dir| + file = File.join(dir, "policy.yml") + File.write(file, yaml) + yield file + end + end + + def allow_everything_yaml + ids = Collector::ThirdPartyLicences.build.sections.flat_map(&:components).flat_map(&:identifiers).uniq + { "allowed" => ids.map { { "id" => it, "reason" => "a fixture" } }, "exceptions" => [] }.to_yaml + end + + it "exits 0 and prints how many components it judged on standard output (AC-4.2)", :aggregate_failures do + with_policy(allow_everything_yaml) do |file| + out, err, status = run_check(file) + + expect(status).to eq(0) + expect(out).to match(/Licences: \d+ components judged/) + expect(err).to be_empty + end + end + + it "exits 1 against a policy that allows nothing, printing each dependency, its licence and the file on standard error (AC-4.3)", :aggregate_failures do + with_policy("allowed: []\nexceptions: []\n") do |file| + _out, err, status = run_check(file) + + expect(status).to eq(1) + expect(err).to include("gem rake", "MIT", file) + expect(err.lines.size).to be > 100 + end + end + + it "exits 1 naming the file when it can't be read", :aggregate_failures do + _out, err, status = run_check("/nonexistent/policy.yml") + + expect(status).to eq(1) + expect(err).to include("/nonexistent/policy.yml") + end +end +``` +The first example doesn't judge the repository's own policy, which `bin/ci checks` does (AC-4.2's evidence is that run, recorded in `verification.md`). + +Implementation, `lib/tasks/licences.rake`: + +```ruby +# The licence check of spec 091: `bin/ci checks` runs it. It judges every gem of the bundle and every entry of the +# third-party manifest against config/licence_policy.yml, and reports spec 087's guard, so it needs the application. +namespace :licences do + desc "Fail when a dependency is under a licence config/licence_policy.yml doesn't allow (spec 091); optional policy path" + task :check, [ :policy ] => :environment do |_task, args| + policy_path = Rails.root.join(args[:policy].presence || Collector::ThirdPartyLicences::Policy::PATH) + begin + check = Collector::ThirdPartyLicences::Check.for_repository(policy_path:) + problems = check.problems + rescue SystemCallError, Psych::Exception => error + warn "Licences: can't read #{policy_path} or the licence manifest: #{error.message}" + exit 1 + end + + if problems.any? + warn "Licences: #{problems.size} problem#{"s" unless problems.one?}:", *problems.map { " - #{it}" } + exit 1 + end + puts "Licences: #{check.component_count} components judged; each is under an allowed licence or a named exception." + end +end +``` +Commit `feat(licences): the licences:check task (#402)`. + +### T2.4 The `config/ci.rb` step (AC-4.1) and the timing + +Red first: add to `spec/config/ci_definition_spec.rb`: + +```ruby + it "runs the licence check in bin/ci checks, directly after the importmap audit and not with the RSpec group (spec 091 AC-4.1)", :aggregate_failures do + lines = Rails.root.join("config/ci.rb").readlines(chomp: true).map(&:strip) + audit = lines.index('step "Security: Importmap vulnerability audit", "bin/importmap audit"') + licences = lines.index('step "Licences: dependencies are under allowed licences", "bin/rails licences:check"') + guarded = lines.index("unless selection.group == :rspec") + + expect(licences).not_to be_nil + expect(licences).to eq(audit + 1) + expect(licences).to be < lines.each_index.find { |i| i > guarded && lines[i] == "end" } + end +``` +Then in `config/ci.rb`, after the importmap audit line, add ` step "Licences: dependencies are under allowed licences", "bin/rails licences:check"`. Green. Run `bin/ci checks` twice and `time` it as the baseline was (`time bin/ci checks`), recording both totals and the step's own line ("passed in N s") for `verification.md`; the first run after the edit includes a cold Bootsnap cache, so record the second. Not a full `bin/ci` and no slot. Commit `feat(ci): the licence check is a step of bin/ci checks (#402)`. + +**Phase 2 verification:** `bin/rspec spec/lib/collector/third_party_licences spec/tasks/licences_rake_spec.rb spec/config/ci_definition_spec.rb` green; `bin/ci checks` green with the step's line `Licences: ... components judged`. + +## Phase 3: `vcr` (ruled by the maintainer, 2026-11-13: a named exception) + +**Satisfies** AC-3.4's third entry and AC-3.6 on the real tree. One task of its own, because the coordinator asked for the entry to be separable; the `all_required` rule is in T1.2's commit and stays whatever happens to the entry. Spec 0.5.1 and ADR 0031 already record the ruling. + +### T3.1 The `vcr` exception (spec decisions 2 and 3, AC-3.4, AC-3.6) + +Add to `exceptions:` in `config/licence_policy.yml`: + +```yaml + - gem: vcr + licences: [Hippocratic-2.1, MIT] + all_required: true + reason: "A test recorder the application never loads. It declares Hippocratic-2.1 and MIT, ships no licence file, and issue #408 calls it Hippocratic-2.1, so its declared MIT is not trusted. all_required is why this is an exception at all: without it the gem would pass on its MIT and this entry would read as stale. It goes stale only when every licence it declares is allowed" +``` +Evidence (no new spec: AC-3.4 is read-and-record): `bin/ci checks` stays green with the entry (the step judges `vcr` by name); with the entry removed, `spec/lib/.../policy_spec.rb`'s `all_required` examples already prove the form, and a local run would then pass `vcr` as `MIT`. Commit `feat(licences): vcr is let through by name, its declared MIT not trusted (#402)`. + +## Phase 4: Documents, the core spec and the evidence + +**Satisfies** AC-5.1 to AC-5.4. T4.1 is with an implementer; T4.2 and T4.3 are mine (prose, verified by reading). + +### T4.1 The core names no collectible (AC-5.1) + +Edit `spec/licences_core_spec.rb`'s file list: add `lib/collector/third_party_licences/policy.rb`, `lib/collector/third_party_licences/check.rb`, `lib/tasks/licences.rake` and `config/licence_policy.yml` to `files`, and bump the first example's `expect(files.size).to be > 6` to `> 10`. The new files exist by then, so the edited example passes at once; the task is added coverage, not a red-first change, and its commit body says so. Commit `test(licences): the core spec covers the policy files (#402)`. + +### T4.2 Area docs, CLAUDE.md and rules (AC-5.2, AC-5.3), by reading + +- `docs/architecture/catalog-core.md`: in "The third-party licences page", a paragraph "**What it may be under**" with the policy file, decisions 2 to 6 of the spec in the doc's style, the step and the task, each with its source inline (spec 091 decision n, AC-n.n; ADR 0031); Named constants: `Policy::PATH`, `Policy::DECISION`; Sources: spec 091, ADR 0031, issue #402; Distillation log row (written by `distill-feature`). +- `docs/architecture/release-and-deploy.md`: the step among the `bin/ci checks` steps (the "`bin/ci` takes one optional group" bullet's order, and the Scope line that lists the checks steps), with its source. +- `CLAUDE.md`: Commands: the Local CI step list names "the licence check" after the importmap audit, and `bin/rails licences:check` is noted as run only through `bin/ci checks` in a session; Project Status: one sentence naming `config/licence_policy.yml`, `Policy`, `Check` and spec 091 / ADR 0031. +- `.claude/rules/testing.md`: the `bin/ci` bullet lists the licence check among what it runs; `.claude/rules/conventions.md`: a ✗ bullet "Don't add a dependency under a licence that isn't in `config/licence_policy.yml` to get CI green: allowing one is the maintainer's decision, with a reason (ADR 0031)". +- `docs/adr/README.md`: 0031's line already carries "(Proposed)"; the distillation step changes it when the PR merges. + +### T4.3 Index, roadmap, verification (AC-5.4) + +- `docs/specs/README.md`: row 091 status and areas filled by `distill-feature`; `docs/roadmap.md`: the #402 row "Done when its pull request merges". +- `docs/specs/091-licence-check-in-ci/verification.md`: the AC table with evidence (spec runs, the `bin/ci checks` run with the step's line, the before and after times (35.5 s before), the model split, the review rounds), and `## Implementation review` with the verdict first (sdd-review Mode B, Opus 5.5, read-only). + +## Spec coverage + +| Spec | Task | +|---|---| +| FR-1 | T2.1, T3.1 | +| FR-2 | T1.1 to T1.3, T2.2 | +| FR-3 | T2.2, T2.3 | +| FR-4 | T2.4 | +| AC-1.1 to AC-1.6 | T1.1 | +| AC-2.1, AC-2.2 | T1.1 | +| AC-2.3 | T1.2 | +| AC-2.4, AC-4.2 | T2.3 (in-process), T2.4 (`bin/ci checks` run recorded) | +| AC-3.1, AC-3.2, AC-3.6 | T1.2 | +| AC-3.3 | T1.1 (shape), T1.3 (unreadable), T2.3 (the task) | +| AC-3.4 | T2.1, T3.1 (by reading) | +| AC-3.5 | T1.2 (first clause), T1.3 (second) | +| AC-4.1 | T2.4 | +| AC-4.3 | T2.3 | +| AC-4.4 | T2.2 | +| AC-4.5 | no workflow file in the diff (checked in `verification.md`) | +| AC-5.1 | T4.1 | +| AC-5.2, AC-5.3, AC-5.4 | T4.2, T4.3 | + +## Verification and finish + +1. `bin/rspec` on the new files after each batch; `bin/ci checks` after T2.4 and after the docs (timings recorded; the step's line in the output is the AC-4.2 evidence). +2. Implementation review (`sdd-review` Mode B, Opus 5.5, read-only) recorded in `verification.md`. +3. `session-wrap`, then `distill-feature` with its digest sent to the coordinator before it is committed. +4. One full `bin/ci` under a coordinator slot, then the push, a draft PR that closes #402, CI green, `gh pr ready`, assignee. + +## Owed to the maintainer + +- T4.2 edits two files under `.claude/rules/`; if the harness prompts for them, the maintainer answers (nothing here edits `.claude/settings.json`). +- A session can't run `bin/rails licences:check` without a prompt, because `.claude/settings.json` lists no such command; add `Bash(bin/rails licences:check*)` if wanted (the task still runs through `bin/ci checks`). diff --git a/docs/specs/091-licence-check-in-ci/spec.md b/docs/specs/091-licence-check-in-ci/spec.md new file mode 100644 index 00000000..feb8e016 --- /dev/null +++ b/docs/specs/091-licence-check-in-ci/spec.md @@ -0,0 +1,180 @@ +# Feature 091: CI Fails on a Dependency Whose Licence Isn't Allowed + +**Status:** Approved (2026-11-13, Gate A: the coordinator under the maintainer's delegation, v0.4.0; amended to 0.5.1: the maintainer ruled `vcr` a named exception, 2026-11-13) +**Version:** 0.5.2 +**Created:** 2026-11-13 +**Last Updated:** 2026-11-13 +**Depends on:** [spec 087](../087-third-party-licences-in-app/spec.md) (the reader of the bundle and the manifest, and the guard; frozen, by [catalog core](../../architecture/catalog-core.md)), [spec 035](../035-ci-shards/spec.md) (`bin/ci checks`; frozen, by [release and deploy](../../architecture/release-and-deploy.md)) +**Branch:** `091-licence-check-in-ci` +**Issue:** [#402](https://github.com/plainprogrammer/Collector/issues/402) (the 1.0 milestone, housekeeping lane) +**ADR:** [0031](../../adr/0031-a-dependency-licence-is-a-decision.md) (Proposed; the maintainer ruled decision 9 on 2026-11-13). +**Frozen:** the current rules are in [`catalog-core.md`](../../architecture/catalog-core.md) and [`release-and-deploy.md`](../../architecture/release-and-deploy.md) (whole spec); see the [specs index](../README.md) + +--- + +## Changelog + +| Version | Date | Change | +|---------|------|--------| +| 0.5.2 | 2026-11-13 | Decision 3 says plainly why `all_required` exists (the coordinator's request at Gate B); no requirement changed | +| 0.5.1 | 2026-11-13 | **The maintainer's ruling on `vcr`** (2026-11-13, relayed by the coordinator): `vcr` is a named exception in `config/licence_policy.yml`, beside `bundler-audit` and `brakeman`; it is a test-only tool the application never loads, its gemspec lists `Hippocratic-2.1` beside `MIT`, it ships no licence file, and whether the `MIT` is a real choice is unverified. The any-one-of rule stays as ruled for `rdoc` and `diff-lcs`. The *(pending)* marks come off; AC-3.4's `vcr` clause is unconditional and its reason clause no longer says "never shipped". No other requirement changed | +| 0.5.0 | 2026-11-13 | **`vcr` (pending the maintainer).** The coordinator recommends a named exception for `vcr` instead of letting it in on its declared `MIT` (it ships no licence file and issue #408 calls it Hippocratic-2.1). An exception can't express that under 0.4.0's rules, because without it `vcr` would pass as `MIT` and the exception would be stale, so decision 3 gains the `all_required` form and AC-3.6 proves it. The default of the draft is the exception form; the policy's `vcr` entry is a task of its own in the plan, so the maintainer's ruling either way is one task. The rest is unchanged | +| 0.4.0 | 2026-11-13 | The maintainer's rulings of 2026-11-13, relayed by the coordinator: decisions 1 to 9 as recommended (the list of seven, any one of a dual, named stale-checked exceptions for `bundler-audit` and `brakeman`, every group, no normalising, the manifest judged, the `bin/ci checks` step extending spec 087, `config/licence_policy.yml`, and ADR 0031). The *(pending)* marks come off; decision 2's table names every gem the dual rule lets in. Open question 2 (OFL-1.1 for every kind) is closed by decision 1's one list. No requirement changed | +| 0.3.0 | 2026-11-13 | Spec review round 2 (Fable, Mode A, APPROVE WITH CHANGES, 0 blocking, 1 major, 7 minor) fixed: a `Check` joins an injected audit's problems and the policy's, so AC-4.4 has a seam (FR-3, decision 7); the lockfile names' de-duplication is `Policy.for_repository`'s and proved on the real lockfile (AC-3.5); AC-2.4 and AC-4.2 are evidenced by the recorded `bin/ci checks` run; double reports are stated (AC-1.6); AC-4.3 says how the path is passed and that a spec runs the task in-process; the header version. Old AC-4.4 (a task-level stand-in) became a criterion on the new seam, the timing criterion moved into the Performance NFR, and the workflow criterion is AC-4.5. No requirement was removed | +| 0.2.0 | 2026-11-13 | Spec review round 1 (Fable, Mode A, APPROVE WITH CHANGES, 0 blocking, 4 major, 8 minor) fixed. The step also reports spec 087's guard, so a pin with no entry fails `bin/ci checks` too (decision 7); `Policy.for_repository` is named and AC-4.4 dropped; the exception rules say order-insensitive, take the lockfile's gem names as an input and have an AC for a gem the lockfile names and the bundle doesn't hold (decision 3, AC-3.5); the fallout in the area docs, CLAUDE.md and `.claude/rules/testing.md` is listed and the step's place proved in `spec/config/ci_definition_spec.rb` (Story 4, Story 5); the limits of the manifest's section keys and of the image are stated (decisions 6 and 8); house style. No requirement was removed | +| 0.1.0 | 2026-11-13 | First draft from issue #402, the licensing research of 2026-11-13 and the task brief. Decisions 1 to 9 are the author's recommendations, put to the coordinator and so to the maintainer as scope questions; they are marked *(pending)* until ruled. The allowed list and the exceptions are the maintainer's to rule | + +## Problem Statement + +Collector is AGPL-3.0 and will be offered as a hosted service on the same code (the [households and hosted service design](../../superpowers/specs/2026-11-13-households-and-hosted-service-design.md)). A dependency's licence is then a decision of the project's, not a detail of a pull request: a copyleft or field-of-use licence that arrives with a new gem, or with a JavaScript package an agent pins, binds what the project may do with the code. Today nothing looks. Spec 087 lists the licences on a page and its guard fails when a pin or a font has no entry, but no check reads *which* licence a dependency is under, so a gem under the GPL-3.0 would pass `bin/ci` and ship. + +Measured on `main` at `71212946`, the bundle holds **142 gems** in every group (the suite's view), with 13 distinct sets of licences. **Two** are under a licence a permissive-only list would not allow: `bundler-audit` (GPL-3.0-or-later) and `brakeman` (Brakeman Public Use License), both development and test tools the app never loads. Three more pass only because a gem declares several licences: `rdoc` (Ruby and GPL-2.0-only), `diff-lcs` (MIT, Artistic-1.0-Perl and GPL-2.0-or-later) and `vcr` (Hippocratic-2.1 and MIT). The manifest side holds MIT (Turbo, Stimulus, the OCR language data), Apache-2.0 (the OCR engine), OFL-1.1 (three fonts) and ISC (the icons). + +> **Terms.** *Allowed list*: the SPDX identifiers the project has decided to depend on, in `config/licence_policy.yml` (decision 1). *Exception*: a named gem that is under a licence off the list, with the licence it was reviewed under and the reason (decision 3). *Component*: one entry of the list spec 087's reader builds, a gem or a manifest entry (JavaScript, OCR engine, font, icon). *Judged*: compared with the allowed list and the exceptions by this spec's rules. + +## Goals + +- A pull request that adds a gem, or a JavaScript package, font or icon set the manifest lists, under a licence the project hasn't allowed fails `bin/ci checks`, and the failure names the dependency, its licence and the file that holds the allowed list. +- The allowed licences, and each exception with its reason, are recorded in the repository. Allowing a copyleft dependency is a change to that file, so it shows in review. +- The check reads the list spec 087's reader builds, so the page and the check can't disagree about what the image holds or under which licence. +- Each rule is proved by injection on a small fixture and the real tree is shown to pass, as spec 087's guard is. + +## Non-Goals + +- Deciding whether a licence is compatible with the AGPL-3.0, or any legal conclusion. The list is the maintainer's decision, recorded; the check enforces it and claims nothing about why it is right. +- Reading a licence from a gem's files. The check trusts the declared identifiers (decision 5); a gem that declares one thing and ships another is a risk it states and doesn't close. +- Verifying a manifest entry's `spdx` against the package's own metadata: there is no `node_modules`, and CI reads no network (decision 6). +- Changing what the image installs ([#408](https://github.com/plainprogrammer/Collector/issues/408)), or the contribution policy ([#401](https://github.com/plainprogrammer/Collector/issues/401)). +- Checking Ruby, its default gems the lockfile doesn't name, or the operating system's packages in the image (spec 087's Non-Goals). +- A change to `.github/workflows/**`, the `Dockerfile` or a deploy file: GitHub runs `bin/ci checks`, which gains the step. +- Judging the licences of Collector's own code, or an SPDX expression parser (decision 5). + +## Users and Context + +**Primary users:** a maintainer or an agent adding or bumping a dependency, who must be told by CI that its licence needs a ruling and where to record one. +**Secondary users:** the maintainer reviewing a change to the allowed list, and an operator or contributor who wants to know what the project allows. +**Usage context:** read in CI output and in a pull request's diff of one small file. + +## Decisions *(ruled by the maintainer, 2026-11-13, as recommended)* + +1. **The allowed list is seven identifiers, by SPDX id, in `config/licence_policy.yml`.** `MIT`, `BSD-2-Clause`, `BSD-3-Clause`, `Apache-2.0`, `ISC`, `Ruby` and `OFL-1.1`. Each has a one-line `reason`. What each lets in today: + + | Id | Lets in today | + |---|---| + | `MIT` | 100 gems declare it alone, plus `nio4r` and `marcel` (each also under another allowed id) and `diff-lcs` and `vcr` (decision 2); the JavaScript (Turbo, Stimulus) and the OCR language data | + | `BSD-2-Clause` | `rexml`, `rubyzip`, `nio4r` and 23 default and Ruby-project gems (`base64`, `bigdecimal`, `drb`, `logger`, `securerandom`, `uri`, `racc`, `prettyprint`, `pp`, `erb`, `tsort`, `rbs`, `io-console`, `irb`, `timeout`, `date`, `net-protocol`, `net-imap`, `net-pop`, `net-smtp`, `matrix`, `debug`, `ostruct`; each also `Ruby`) | + | `BSD-3-Clause` | `ffi`, `puma`, `sqlite3` | + | `Apache-2.0` | `websocket-driver`, `websocket-extensions`, `addressable`, `msgpack`, `selenium-webdriver`, `marcel`; `tesseract.js` and `tesseract.js-core` | + | `ISC` | the icons (Lucide) | + | `Ruby` | `json`, `reline`, `rdoc` (see decision 2) and the 23 gems above | + | `OFL-1.1` | the three font families; no gem | + + Not allowed, so a decision when one arrives: any GPL, LGPL, AGPL, MPL or EPL licence, Artistic, a field-of-use or ethical licence (Hippocratic), a source-available licence, `0BSD`, `Unlicense`, `CC0-1.0` and `BSD-3-Clause-Clear`. Adding `0BSD` or `Unlicense` the first time one is met is a one-line change. +2. **A component with several identifiers passes if any one is allowed.** The gemspec's `licenses` list is a choice of licences by convention, and spec 087's reader shows it as a list of identifiers. Three gems pass today only because of this rule, so the maintainer rules on names (the table; `nio4r` and `marcel` below it pass on either id): + + | Gem | Declares | Passes as | Ids that aren't allowed | + |---|---|---|---| + | `rdoc` 8.1.0 | `Ruby`, `GPL-2.0-only` | `Ruby` | `GPL-2.0-only` | + | `diff-lcs` 1.6.2 | `MIT`, `Artistic-1.0-Perl`, `GPL-2.0-or-later` | `MIT` | `Artistic-1.0-Perl`, `GPL-2.0-or-later` | + | `vcr` 6.4.1 | `Hippocratic-2.1`, `MIT` | `MIT` | `Hippocratic-2.1` | + + In prose: **`rdoc` 8.1.0** (`Ruby`, with `GPL-2.0-only`; passes as `Ruby`), **`diff-lcs` 1.6.2** (`MIT`, `Artistic-1.0-Perl` and `GPL-2.0-or-later`; passes as `MIT`; its files hold `LICENCE.md` and `docs/COPYING.txt`) and **`vcr` 6.4.1** (`Hippocratic-2.1` and `MIT`; passes as `MIT`; the gem ships no licence file, so its licence page row has no text). `nio4r` and `marcel` pass on either id. The rule trusts the declared `MIT` of the last two (`diff-lcs` ships its licence texts, `vcr` ships none, so only `vcr` rests on the declaration alone, and issue #408 calls it Hippocratic-2.1). **`vcr` is a named exception (ruled 2026-11-13):** in decision 3's `all_required` form, so `vcr` fails the any-one-of rule and is let through by name with the reason, because whether its `MIT` is a real choice is unverified. The rule stays as ruled for `rdoc` and `diff-lcs`, which pass on `Ruby` and `MIT`. +3. **Exceptions are named per gem, in the same file, each with the licences it was reviewed under and a reason.** An exception is `gem`, `licences` (the identifiers the gem declares, compared as a set, so order doesn't matter: the bundle already holds both orders of one pair) and `reason`. It lets that gem pass while it declares exactly those. It is stale, and fails the check, when the lockfile no longer names the gem, when the gem declares other identifiers (a new release changed its licence, which is a new decision), or when the gem would pass without it. A gem the lockfile names and the bundle doesn't hold (a run with `BUNDLE_WITHOUT`) is neither judged nor stale. The lockfile's names are a second input, read with `Bundler::LockfileParser` and de-duplicated (`ffi`, `sqlite3` and others repeat per platform); `bundler` itself is in the bundle and not in the lockfile, so an exception for it would always be stale, and it declares an allowed licence anyway. An exception may say `all_required: true`, meaning the gem's list is not a choice: every identifier must be allowed, so the gem fails without the exception, and the exception is stale when every identifier is now allowed (not when one is). **Why the form exists, plainly:** `vcr` declares `Hippocratic-2.1` and `MIT`, so the any-one-of rule would pass it on `MIT` and an ordinary exception for it would be stale ("allowed without it"); `all_required` is how an exception can mean "don't trust the allowed alternative". The policy file's header says the same, for whoever copies the form. Today three: **`bundler-audit`** (`GPL-3.0-or-later`; a command-line audit run by `bin/ci` and never loaded by the application) and **`brakeman`** (`Brakeman Public Use License`; a static analyser run by `bin/ci` and the commit hook, never loaded). The third is **`vcr`** (`Hippocratic-2.1` and `MIT`, `all_required: true`; a test-group HTTP recorder never loaded by the application; reason: its declared `MIT` is not trusted, since it ships no licence file and the issue calls it Hippocratic-2.1). The first two leave the image when #408 lands; the exceptions then stay as development-tool exceptions. Exceptions are for gems; a manifest entry under a disallowed licence is a change to the list or no dependency. +4. **Every bundle group is judged.** The check reads `Bundler.load.specs`, as spec 087's reader does, which in development and CI holds every group. A development or test tool under a restricted licence is then a recorded exception and not a silent pass, and the rule doesn't depend on what the image installs (#408) or on the `Dockerfile`. A run with `BUNDLE_WITHOUT` set judges only what is installed; CI and development install every group. +5. **No licence, or one that isn't an allowed id, fails; nothing is normalised.** A component with no identifiers fails (spec 087's guard accepts a gem with a licence file and no declaration, for display; the policy can't judge a file). An identifier is compared exactly with the list: `MIT License`, `Apache 2.0` and an expression in one string such as `MIT OR Apache-2.0` all fail, naming the string, and the fix is the exception or the list. This costs an occasional exception for a misspelt but fine licence and guarantees nothing is let in by a guess. +6. **The manifest's entries are judged by the same rule.** Every entry of `config/third_party_licences.yml` (JavaScript, OCR engine, fonts, icons) is judged by its `spdx`. Spec 087's guard already fails an importmap pin or an OCR package with no entry, so a JavaScript package arrives with an entry, and the entry must then pass. Two limits are stated. A key of the manifest that the reader doesn't list (`ThirdPartyLicences::TITLES`) is neither shown nor judged, as spec 087 leaves it. And `spdx` is typed from the package's metadata by whoever adds the entry (each entry's `source` says where); the check can't read a package it doesn't have. +7. **The check is a `config/ci.rb` step in `bin/ci checks`, over a new class that reads spec 087's list.** The step "Licences: dependencies are under allowed licences" runs `bin/rails licences:check`, in the group of the other checks (not the RSpec group), so GitHub's checks job runs it with no workflow change. A new class under `lib/collector/third_party_licences/`, `Policy`, judges the sections `Collector::ThirdPartyLicences.build` returns, with `Policy.for_repository(policy_path:)` building the real inputs as `Audit.for_repository` does; there is no second reader of the bundle (the lockfile's gem names, decision 3, are the one added input). A small `Check.new(audit:, policy:)` joins the two: the guard's problems first, then the policy's, as one list of sentences for the task to print; it takes any audit that answers `problems`, so a spec injects a stand-in. The task so reports spec 087's `Audit.for_repository.problems`, so an importmap pin or an OCR package with no manifest entry fails `bin/ci checks` as the issue's outcome says and not only the RSpec shards; the guard spec stays. The rules are proved in an RSpec file like `audit_spec.rb` (the shards run them); the real tree is judged by the step's run (AC-4.2) and not by a duplicate spec. The task takes the policy's path as an argument, so a spec (and a maintainer) can run the real task against a policy that allows nothing. +8. **The file is `config/licence_policy.yml`, apart from the page's manifest.** The manifest is display data (names, digests, texts) and its reader ignores unknown keys; a ruling the maintainer makes belongs in a file whose diff is one decision. It sits in `config/`, which the image keeps, and so do the class and the task (autoloaded from `lib/`); that is intended and harmless, as the manifest is. The task needs `:environment` (the importmap and the asset load path), unlike `start:check` and the backup tasks. Its header says how to propose an addition. +9. **An ADR for the rule, if the maintainer wants one.** I recommend ADR 0031: "a copyleft dependency is a decision" is of lasting weight (it constrains what the hosted service may build on), and the rules (any one of several, every group, named exceptions that go stale) are why-not-what. The YAML holds the list and the reasons; the ADR holds the rule. The alternative is no ADR, with the rule in `catalog-core.md` as spec 087 did. + +## User Stories and Acceptance Criteria + +### Story 1: A gem under a licence that isn't allowed fails CI + +**As** a maintainer **I want** `bin/ci checks` to fail when a gem is added under a licence I haven't allowed **so that** a copyleft dependency is my decision and not an accident. + +- [ ] **AC-1.1** Given a stand-in bundle with a gem declaring `GPL-3.0-only` and an allowed list without it When the policy is judged Then it fails once, and the message names the gem and its version, its licence `GPL-3.0-only`, and the policy file's path as it was given (`config/licence_policy.yml` for the real one). +- [ ] **AC-1.2** Given a gem declaring `MIT` and `GPL-2.0-only` When it is judged Then it passes (decision 2); given one declaring only `GPL-2.0-only` and `Artistic-1.0-Perl` Then it fails and the message lists both. +- [ ] **AC-1.3** Given a gem with no declared licence, with or without a licence file When it is judged Then it fails and says it declares none. +- [ ] **AC-1.4** Given a gem declaring `MIT License`, `Apache 2.0` or `MIT OR Apache-2.0` When it is judged Then it fails and names the string as written. +- [ ] **AC-1.5** Given a stand-in bundle of gems, each under an allowed id (including the dual `BSD-2-Clause` and `Ruby`) When it is judged Then there is no problem. +- [ ] **AC-1.6** Given several offenders When they are judged Then the problems name each, one sentence per component, in the order `ThirdPartyLicences.build` lists them (sections in page order, then name regardless of case), each naming the policy file's path. A component the guard also reports (a gem with neither identifier nor licence file, an entry with no `spdx`) is reported by both and by `Check` twice, once per source; the policy's own rules give one sentence per component, a stale exception and a disallowed licence for the same gem being two sentences. + +### Story 2: The JavaScript, fonts and icons the manifest lists are judged too + +**As** a maintainer **I want** a pinned JavaScript package, a font or an icon set held to the same list as a gem **so that** the importmap side can't carry a licence the gems couldn't. + +- [ ] **AC-2.1** Given a stand-in manifest whose JavaScript entry says `spdx: GPL-3.0-only` When it is judged Then it fails naming the entry, the section and the licence; likewise an OCR, font or icon entry. +- [ ] **AC-2.2** Given a manifest entry with several `spdx` identifiers When it is judged Then decision 2's rule applies, and one with none fails. +- [ ] **AC-2.3** Given a gem exception for a gem name that is also a manifest entry's name When the entry is judged Then the exception doesn't apply (decision 3: exceptions are for gems). +- [ ] **AC-2.4** Given the repository's real manifest When it is judged against the real policy Then there is no problem (verified by the run of AC-4.2, not by a spec on the real tree). + +### Story 3: Exceptions are named and stay honest + +**As** the maintainer **I want** each licence I let through to be a named exception with a reason, which CI stops me forgetting **so that** the list of exceptions is always the true one. + +- [ ] **AC-3.1** Given a gem under a disallowed licence with an exception naming that gem and exactly its licences When it is judged Then it passes. +- [ ] **AC-3.2** Given an exception whose gem the lockfile no longer names, one whose `licences` differ from what the gem now declares, or one for a gem that is allowed without it When it is judged Then it fails as stale, naming the exception and which of the three it is; an exception whose `licences` list the same identifiers in another order is not stale (decision 3). +- [ ] **AC-3.3** Given a policy entry missing its `id` or `reason`, or an exception missing its `gem`, `licences` or `reason`, a duplicate id, or a duplicate exception When the policy is loaded Then it fails naming the entry; a policy file that can't be read or isn't a mapping fails the step with the path. +- [ ] **AC-3.4** Given the real policy file When it is read Then it allows exactly the identifiers of decision 1 and excepts exactly `bundler-audit`, `brakeman` and `vcr` (the last in `all_required` form), each with a reason that names why the application never loads the gem. +- [ ] **AC-3.6** Given a gem declaring `MIT` and `Hippocratic-2.1` When it is judged with no exception Then it passes as `MIT`; with an `all_required` exception naming it and exactly those licences Then it is let through by name, and without that exception but with the form's rule on Then it fails naming `Hippocratic-2.1`; given an `all_required` exception for a gem whose licences are all allowed Then it is stale. +- [ ] **AC-3.5** Given a gem the lockfile names that the bundle doesn't hold (`BUNDLE_WITHOUT`), with or without an exception When the policy is judged Then it is neither judged nor reported stale; given the real `Gemfile.lock` When `Policy.for_repository` reads its gem names (it de-duplicates them; the policy itself takes names as an injected input) Then they hold no repeat and no `bundler`. + +### Story 4: The step runs in `bin/ci checks` and shows its failure + +**As** a contributor **I want** the failure to say what to do **so that** I record a decision or drop the dependency, and don't hunt through the output. + +- [ ] **AC-4.1** Given `config/ci.rb` When it is read Then it has the step "Licences: dependencies are under allowed licences" running `bin/rails licences:check`, inside the block `bin/ci rspec` skips, after "Security: Importmap vulnerability audit" (proved in `spec/config/ci_definition_spec.rb`, as the YARD and ratchet steps are). +- [ ] **AC-4.2** Given today's tree When `bin/rails licences:check` runs Then it exits 0, prints how many components it judged on standard output, and reports no problem of spec 087's guard either. +- [ ] **AC-4.3** Given the task run, in-process with its `SystemExit` rescued, as `bin/rails "licences:check[path]"` passes the path (as `catalog:refresh[mtg]` passes its argument), against a policy file that allows no licence When it runs Then it exits 1, and on standard error prints one line per component with the dependency's name, its licence and the policy file's path (the fixture's `allowed: []` is valid and holds no exceptions). +- [ ] **AC-4.4** Given `Check` with a stand-in audit that reports one problem and a policy that reports another When it runs Then it lists the audit's problem first and the policy's second, and with a stand-in audit built from a manifest with an importmap pin and no entry Then the guard's sentence is among them (decision 7). +- [ ] **AC-4.5** Given `.github/workflows/**` When the pull request is read Then no workflow changed. + +### Story 5: It lives where an agent looks + +**As** an agent **I want** the rule written where I read the commands and the area docs **so that** I know before I add a dependency. + +The criteria that name a document (AC-3.4, AC-4.5, AC-5.x) are verified by reading and recorded in `verification.md`, as `.claude/rules/testing.md` asks; AC-2.4 and AC-4.2 are evidenced by the `bin/ci checks` run recorded there. + +- [ ] **AC-5.1** Given `spec/licences_core_spec.rb` When it runs Then its file list covers the policy file and the new class, and none names a collectible, source or rights holder. +- [ ] **AC-5.2** Given the area doc [catalog core](../../architecture/catalog-core.md) When the work is done Then it records the policy file, the rules of decisions 2 to 6, the task and the step, each with its source inline, and its Named constants table lists the new constants; [release and deploy](../../architecture/release-and-deploy.md) lists the step among the `bin/ci checks` steps. +- [ ] **AC-5.3** Given `CLAUDE.md` When the work is done Then its Commands list has `bin/rails licences:check` (and its Local CI step list names the step) and its Project Status names the policy file; `.claude/rules/testing.md` lists the step among what `bin/ci` runs, and `.claude/rules/conventions.md` says a new dependency under a licence off the list is a decision, not a fix for CI. +- [ ] **AC-5.4** Given `docs/roadmap.md` and `docs/specs/README.md` When the work is done Then the roadmap's #402 row is marked, and the index has spec 091's row. + +## Functional Requirements + +- **FR-1** `config/licence_policy.yml`: `allowed` (each `id`, `reason`) and `exceptions` (each `gem`, `licences`, `reason`), read with `YAML.safe_load`. +- **FR-2** A class under `lib/collector/third_party_licences/` that judges `ThirdPartyLicences.build`'s sections against a policy and the lockfile's gem names and returns one sentence per problem; every input injectable, as the audit's are. +- **FR-3** `Check`, which joins an injected audit's problems and the policy's (the guard's first), and `licences:check`, a rake task that builds the real audit and policy, prints the joined problems on standard error and exits 1 when there are any, and prints the count on standard output when there are none; it takes an optional policy path. +- **FR-4** The `config/ci.rb` step. + +## Non-Functional Requirements + +- **Performance:** the step reads the licence files `ThirdPartyLicences.build` reads (about 400 KB) and boots the application; `bin/ci checks` is recorded before (35.5 s total on the author's machine, 2026-11-13, `main` at `71212946`) and after in `verification.md`, with the step's own time. It adds no database work. +- **Security:** the policy is read with `YAML.safe_load`; the task reads no network and writes nothing. +- **Collectible-agnostic core:** AC-5.1. +- **Self-hosting:** nothing to configure; the check runs in CI and in development, not in the image. + +### Hosted-door rules + +- No source is touched, no collection data is read, no choice is stored, and no export field is added, so the export format is unchanged. + +## Error Scenarios + +| Situation | Result | +|---|---| +| A new gem under an unlisted licence | `bin/ci checks` fails at the step, naming the gem, its version, the licence and `config/licence_policy.yml` (AC-1.1) | +| A new importmap pin with no manifest entry | The step reports spec 087's guard and fails (AC-4.4), as do the RSpec shards; once the entry exists the step judges it (decision 6) | +| A gem's new release changes its licence | An exception for it is stale and fails (AC-3.2); the gem without one fails if the new licence is off the list | +| The policy file is missing or malformed | The step fails naming the file (AC-3.3) | +| `BUNDLE_WITHOUT` hides the test group | The check judges what is installed and doesn't call an exception for a missing gem stale if the lockfile still names it (AC-3.5) | + +## Open Questions + +None open. The maintainer ruled decisions 1 to 9 as recommended on 2026-11-13 and `vcr` a named exception the same day. The one list holds `OFL-1.1` for every kind, as the ruling of decision 1 states it (no per-kind scope; none was put to the maintainer, so a scope is a later change). + +## Out of Scope (Candidates) + +- An SPDX expression parser (`MIT OR Apache-2.0` in one string). +- Reading a licence from a package's own files to compare with its declaration. +- A scheduled run of the check on `main` for a licence that changes without a pull request (Dependabot's updates arrive as pull requests, which run the step). diff --git a/docs/specs/091-licence-check-in-ci/verification.md b/docs/specs/091-licence-check-in-ci/verification.md new file mode 100644 index 00000000..61353472 --- /dev/null +++ b/docs/specs/091-licence-check-in-ci/verification.md @@ -0,0 +1,54 @@ +# Verification: spec 091, CI fails on a dependency whose licence isn't allowed (#402) + +**Status:** in progress. Built and checked locally on 2026-11-13; the implementation review (`sdd-review` Mode B, Opus 5.5), the wrap, the one full `bin/ci` and the push are not done yet. + +## Evidence so far (fresh runs, 2026-11-13) + +- **Specs:** `bin/rspec spec/lib/collector/third_party_licences spec/tasks/licences_rake_spec.rb spec/config/ci_definition_spec.rb spec/licences_core_spec.rb spec/third_party_licences_manifest_spec.rb spec/lib/collector/third_party_licences_spec.rb`: 102 examples, 0 failures (seed 32188). +- **`bin/ci checks`:** green. The step's line: `Licences: 151 components judged; each is under an allowed licence or a named exception.` (AC-4.2); the step took 1.28 s of 25.64 s. +- **Timing, `bin/ci checks`, same command `time bin/ci checks`:** before 35.5 s total (main at `71212946`); after 30.98 s (cold), 24.70 s and 25.80 s. The totals are below the baseline because other workers share the machine, so the step's own cost, 1.2 to 1.3 s, is the figure that counts. +- **Failure on the real task (AC-4.3):** a policy copy without `Apache-2.0` and without the `brakeman` exception made `bin/rails "licences:check[]"` print "Licences: 8 problems:" with one line per dependency, e.g. `gem addressable 2.9.2 is under Apache-2.0, none of which is on the allowed list in : allow the licence there, or list the gem under exceptions with its reason (ADR 0031)`, and exit 1 (including the OCR engine's JavaScript-side entries, `tesseract.js` and `tesseract.js-core`). +- **`vcr`:** the real run prints no problem with the exception; with the entry commented out it also prints none (it passes on its `MIT`); with `all_required` removed it reports the exception stale ("allowed without it"). This is why the form exists (spec decision 3). + +## Acceptance criteria + +| AC | Evidence | +|---|---| +| AC-1.1 to AC-1.6, AC-2.1 to AC-2.3 | `policy_spec.rb` (injected sections) | +| AC-3.1, AC-3.2, AC-3.5, AC-3.6 | `policy_spec.rb`, exceptions and `all_required` groups | +| AC-3.3 | `policy_spec.rb` (shape, unreadable file), `licences_rake_spec.rb` (unreadable policy) | +| AC-3.4 | read: `config/licence_policy.yml` allows the seven ids and excepts `bundler-audit`, `brakeman`, `vcr` (`all_required`) with reasons | +| AC-4.1 | `spec/config/ci_definition_spec.rb` | +| AC-4.2, AC-2.4 | the `bin/ci checks` run above | +| AC-4.3 | `licences_rake_spec.rb` and the run above | +| AC-4.4 | `check_spec.rb` | +| AC-4.5 | no file under `.github/workflows/` in the diff | +| AC-5.1 | `spec/licences_core_spec.rb` (shown failing on a planted word, then restored) | +| AC-5.2, AC-5.3 | read: `catalog-core.md`, `release-and-deploy.md`, `CLAUDE.md`, `.claude/rules/testing.md`, `conventions.md` | +| AC-5.4 | roadmap and specs index rows (`distill-feature` finishes them) | + +## Models + +Spec, ADR 0031 and plan: my main session (Sonnet 5.5). Spec, ADR and plan reviews: Fable 5.2, read-only (spec 2 rounds, ADR 1, plan 1). Implementers: Sonnet 5.5 (three batches: T1.1 to T1.3, T2.1 to T2.4, T3.1 with T4.1). Docs (T4.2): my main session. + +## Owed + +- **`vcr` (to the maintainer):** `vcr` 6.4.1 declares `Hippocratic-2.1` and `MIT` and ships no licence file; whether its `MIT` is a real choice is unverified upstream. It is a named `all_required` exception for that reason (spec decision 3). Checking upstream would settle it. +- **README step list (to the maintainer):** README.md's numbered `bin/ci` step list (lines 124 to 132) predates this branch (it lacks the migration guard, YARD and the ratchet) and now also lacks the licence check; not changed, as the spec doesn't require it. +- The full `bin/ci` under a coordinator slot, then the push. + +## Implementation review + +SPEC-ALIGNED WITH FINDINGS + +Opus 5.5, `sdd-superpowers:sdd-review` Mode B, read-only, 2026-11-13, over `git diff 71212946..HEAD`. No blocking or major finding. It ran the five feature spec files (40 examples, 0 failures) and `bin/rails licences:check` on the real tree (151 components, exit 0, as above), and found no change under `.github/`, no Dockerfile, deploy or settings file, and no collectible named in the new files. Every AC and FR is covered (the table above), and the docs state what the code does. + +Findings, all minor: + +1. `all_required` was never type-checked (a string `"true"` read as false). Fixed: a shape problem now names the entry. +2. An empty policy file loaded as `{}` and produced about 151 "not allowed" lines instead of one shape problem. Fixed: a mapping without `allowed` is one shape problem; `allowed: []` stays valid. +3. Stale-exception sentences follow the component sentences, not interleaved. Fixed in the `#problems` comment and the area doc. +4. The `Policy` class comment omitted `all_required`. Fixed. +5. The `DECISION` constant's comment said what, not why. Fixed. +6. The specs index row said Draft while the spec is Approved. Fixed. +7. README.md's numbered `bin/ci` step list was already stale before this branch (no migration guard, YARD or ratchet) and now also lacks the licence check. Not changed: the spec doesn't require it; owed to the maintainer as a README tidy. diff --git a/docs/specs/README.md b/docs/specs/README.md index d911e5dc..b6a82538 100644 --- a/docs/specs/README.md +++ b/docs/specs/README.md @@ -103,3 +103,4 @@ A plan is deleted from the tree in a commit of its own after its feature's pull | [085 A Price Source Contract, and Scryfall's Prices Through It](085-price-source-contract/spec.md) | Frozen, by [prices](../architecture/prices.md) | [Prices](../architecture/prices.md) (the source contract, Scryfall's source, the source and series rows) | retired in `e78c902bc0f9f6158bfddc1fbfffdf1edf9da3e4`; `git show e78c902bc0f9f6158bfddc1fbfffdf1edf9da3e4^:docs/specs/085-price-source-contract/plan.md` | | | | [086 A Start Refuses a Volume That Holds a Prices Database and No Primary](086-start-refuses-orphan-prices/spec.md) | Frozen, by [release and deploy](../architecture/release-and-deploy.md) | [Release and deploy](../architecture/release-and-deploy.md) (the start check before `db:prepare`, its message, the restore's advice and the smoke leg); [prices](../architecture/prices.md) (a pointer only) | retired in `2c5939d4604cc1221534677c3e4bcc4aa5bdc916`; `git show 2c5939d4604cc1221534677c3e4bcc4aa5bdc916^:docs/specs/086-start-refuses-orphan-prices/plan.md` | In part, spec 081 decision 8's "Collector must never be started on such a volume" and AC-5.9's advice (the start now refuses it; the restore's advice says so) ([#375](https://github.com/plainprogrammer/Collector/issues/375)) | | | [087 The Licences of the Third-Party Software and Assets in the Image, Readable from the App](087-third-party-licences-in-app/spec.md) | Frozen, by [catalog core](../architecture/catalog-core.md) (the licence line's third sentence, the page, the reader, the manifest, the guard), [release and deploy](../architecture/release-and-deploy.md) (the smoke check) and [tenancy and accounts](../architecture/tenancy-and-accounts.md) (the endpoint); the `Licences` pattern is in the [design system](../design-system/README.md) | [Catalog core](../architecture/catalog-core.md) (the footer and the third-party licences page); [release and deploy](../architecture/release-and-deploy.md) (the smoke check); [tenancy and accounts](../architecture/tenancy-and-accounts.md) (the endpoint that skips sign-in); [design system](../design-system/README.md) (`Licences`, `Footer`) | retired in `11de80918d734d3b6e001d16b013f8a77c820ffc`; `git show 11de80918d734d3b6e001d16b013f8a77c820ffc^:docs/specs/087-third-party-licences-in-app/plan.md` | 084: AC-1.4's "no other part of the line is a link", the full wording of AC-2.1, AC-2.3, AC-2.4 and AC-2.14 and decision 2's wording table, AC-2.13's two links, FR-1's "with two links", the Accessibility NFR's and Users and Context's two links, decision 12's no-script notice (it gains the link), the Non-Goals "a licence page in the app" and "Listing third-party licences" ([#376](https://github.com/plainprogrammer/Collector/issues/376)) | | +| [091 CI Fails on a Dependency Whose Licence Isn't Allowed](091-licence-check-in-ci/spec.md) | Frozen, by [catalog core](../architecture/catalog-core.md) (the policy file, the check and its rules) and [release and deploy](../architecture/release-and-deploy.md) (the `bin/ci checks` step); [ADR 0031](../adr/0031-a-dependency-licence-is-a-decision.md) | [Catalog core](../architecture/catalog-core.md) (the third-party licences page, its guard, the policy and the check); [release and deploy](../architecture/release-and-deploy.md) (the step) | retirement owed | | | diff --git a/lib/collector/third_party_licences/check.rb b/lib/collector/third_party_licences/check.rb new file mode 100644 index 00000000..7ba5b61a --- /dev/null +++ b/lib/collector/third_party_licences/check.rb @@ -0,0 +1,39 @@ +module Collector + class ThirdPartyLicences + # Everything the `licences:check` step reports: spec 087's guard (the manifest, the pins and the bundle agree) and the + # policy (every component is under an allowed licence), in one list, so a pull request that adds an importmap pin with + # no entry fails `bin/ci checks` as well as the suite (spec 091 decision 7). + # + # @see file:catalog-core.md + class Check + # The check of this repository: the real guard and the real policy. + # + # Reads the manifest, every licence file and the lockfile; needs the application booted (the importmap). + # + # @param policy_path [String, Pathname] the policy file; default `config/licence_policy.yml` + # @return [Check] + # @raise [SystemCallError] when the policy or the manifest can't be read + # @raise [Psych::Exception] when either isn't plain YAML + def self.for_repository(policy_path: Rails.root.join(Policy::PATH)) + new(audit: Audit.for_repository, policy: Policy.for_repository(policy_path:)) + end + + # @param audit [#problems] spec 087's guard, or a stand-in + # @param policy [#problems, #component_count] the policy, or a stand-in + def initialize(audit:, policy:) + @audit = audit + @policy = policy + end + + # The guard's problems, then the policy's. + # + # @return [Array] empty when both agree + def problems = @audit.problems + @policy.problems + + # How many components the policy judged. + # + # @return [Integer] + def component_count = @policy.component_count + end + end +end diff --git a/lib/collector/third_party_licences/policy.rb b/lib/collector/third_party_licences/policy.rb new file mode 100644 index 00000000..7d9683be --- /dev/null +++ b/lib/collector/third_party_licences/policy.rb @@ -0,0 +1,204 @@ +module Collector + class ThirdPartyLicences + # Judges the components of {ThirdPartyLicences.build}'s sections against the allowed list and the named exceptions in + # `config/licence_policy.yml` (spec 091), as a list of problems each naming its offender, the licence and the file. + # + # Every input is injected, so a spec shows each rule failing on a small fixture; {.for_repository} builds the real ones + # and the `licences:check` task prints the problems. A component passes if any one of its identifiers is allowed (spec 091 + # decision 2); one with none, or with a string that is not exactly an allowed id, fails (decision 5). A named exception + # may instead require every licence of its gem to be allowed (`all_required`, spec 091 decision 3). Nothing here reads + # a gem or a licence file: the one reader is {ThirdPartyLicences} (no second reader of the bundle). + # + # @see file:catalog-core.md + # @see file:0031-a-dependency-licence-is-a-decision.md + class Policy + # The policy file, relative to `Rails.root`. + PATH = "config/licence_policy.yml".freeze + + # The decision the failure messages point at: ADR 0031, not the policy YAML, holds the rule (any one of several + # licences, every group, exceptions that go stale); the YAML holds only the list and the reasons. + DECISION = "ADR 0031".freeze + + # A named exception, as the rules use it: the `gem`, its `licences` (sorted, a set) and whether its list is `all_required`. + # Its `reason` is for the reader of the file; no rule reads it. + Exemption = Data.define(:gem, :licences, :all_required) + + class << self + # The policy of this repository: the file at `policy_path`, the sections of the running bundle and manifest, and the + # lockfile's gem names. + # + # Reads the policy and, unless `sections` is given, every licence file {ThirdPartyLicences.build} reads (about 400 KB). + # + # @param policy_path [String, Pathname] the policy file; default `config/licence_policy.yml` + # @param sections [Array] the components to judge; default the real ones + # @return [Policy] + # @raise [SystemCallError] when the policy file can't be read + # @raise [Psych::Exception] when it isn't valid YAML or holds more than plain data + def for_repository(policy_path: Rails.root.join(PATH), sections: ThirdPartyLicences.build.sections) + new(data: ThirdPartyLicences.load_manifest(policy_path), path: display_path(policy_path), sections:, locked_names:) + end + + # The gem names the lockfile holds, once each. + # + # A gem the lockfile names for several platforms repeats there (`ffi`, `sqlite3`); `bundler` is in the bundle and + # not in the lockfile, so it is never among them. + # + # @return [Array] + def locked_names + Bundler::LockfileParser.new(Bundler.default_lockfile.read).specs.map(&:name).uniq + end + + private + + # The path as a message prints it: relative to the application when it is inside it. + def display_path(path) = path.to_s.delete_prefix("#{Rails.root}/") + end + + # @param data [Hash{String=>Array}, Object] the loaded policy: `allowed` (each `id`, `reason`) and `exceptions` + # (each `gem`, `licences`, `reason`, optional `all_required`); anything but a mapping is a problem, not a crash + # @param path [String] the policy's path, as the messages print it + # @param sections [Array] what {ThirdPartyLicences.build} returns + # @param locked_names [Array] the gem names the lockfile holds, once each + def initialize(data:, path:, sections:, locked_names:) + @data = data + @path = path.to_s + @sections = sections + @locked_names = locked_names + end + + # How many components the sections hold, which is what a passing run reports it judged. + # + # @return [Integer] + def component_count = components.size + + # What is off the policy, one sentence per offender, each naming it, its licence and the policy file. + # + # Pure of side effects. While the policy's own shape is wrong it reports only that, because judging against a + # malformed list would blame every gem. Otherwise the judged sentences come first, in the order + # {ThirdPartyLicences.build} lists the components, and the stale-exception sentences follow, in the policy file's order. + # + # @return [Array] empty when everything agrees; the policy's shape problems alone when it is malformed + def problems + shape = shape_problems + return shape if shape.any? + + judge_problems + exemption_problems + end + + private + + def components = @sections.flat_map { |section| section.components.map { |component| [ section, component ] } } + + def raw_allowed = Array(@data["allowed"]) + + def raw_exceptions = Array(@data["exceptions"]) + + def allowed_ids = @allowed_ids ||= raw_allowed.filter_map { it["id"] if it.is_a?(Hash) } + + def filled?(value) = value.is_a?(String) && !value.strip.empty? + + def shape_problems + return [ "#{@path} isn't a mapping with `allowed` and `exceptions`" ] unless @data.is_a?(Hash) + return [ "#{@path} has no `allowed` list" ] unless @data.key?("allowed") + + allowed_shape + exception_shape + end + + def allowed_shape + bad = raw_allowed.each_with_index.reject { |entry, _| entry.is_a?(Hash) && filled?(entry["id"]) && filled?(entry["reason"]) } + .map { |_, index| "#{@path}: allowed entry #{index + 1} needs an `id` and a `reason`" } + bad + repeated(allowed_ids).map { "#{@path}: allowed id #{it} is listed twice" } + end + + def exception_shape + bad = raw_exceptions.each_with_index.reject { |entry, _| well_formed_exception?(entry) } + .map { |entry, index| "#{@path}: exception entry #{index + 1} (#{entry.is_a?(Hash) ? entry["gem"] : "not a mapping"}) #{exception_needs(entry)}" } + bad + repeated(raw_exceptions.filter_map { it["gem"] if it.is_a?(Hash) }).map { "#{@path}: the exception for gem #{it} is listed twice" } + end + + def well_formed_exception?(entry) + entry.is_a?(Hash) && filled?(entry["gem"]) && filled?(entry["reason"]) && + entry["licences"].is_a?(Array) && entry["licences"].any? && entry["licences"].all? { filled?(it) } && + valid_all_required?(entry) + end + + # `all_required` may be absent; given, it is exactly true or false (a string such as "true" would silently mean false). + def valid_all_required?(entry) = !entry.key?("all_required") || [ true, false ].include?(entry["all_required"]) + + def exception_needs(entry) + needs = "needs a `gem`, `licences` (a list) and a `reason`" + entry.is_a?(Hash) && !valid_all_required?(entry) ? "#{needs}; `all_required` must be true or false" : needs + end + + def repeated(values) = values.tally.select { |_, count| count > 1 }.keys + + def judge_problems + components.flat_map { |section, component| judge(section, component) } + end + + # The sentence(s) for one component: none when it passes. + def judge(section, component) + ids = component.identifiers.uniq.sort + return [ "#{label(section, component)} declares no licence; #{@path} can judge only a declared SPDX id (#{DECISION})" ] if ids.empty? + + exemption = exemption_for(section, component) + return [] if exemption && ids == exemption.licences + + allowed?(ids, strict: exemption&.all_required) ? [] : [ disallowed(section, component, ids, strict: exemption&.all_required) ] + end + + def exemptions + @exemptions ||= raw_exceptions.map do |entry| + Exemption.new(gem: entry["gem"], licences: entry["licences"].uniq.sort, all_required: entry["all_required"] == true) + end + end + + def exemption_for(section, component) + exemptions.find { it.gem == component.name } if section.key == :gems + end + + def allowed?(ids, strict:) + strict ? ids.all? { allowed_ids.include?(it) } : ids.any? { allowed_ids.include?(it) } + end + + def label(section, component) + section.key == :gems ? "gem #{component.name} #{component.version}" : "#{section.title} entry #{component.name.inspect} (#{component.version})" + end + + def disallowed(section, component, ids, strict: false) + way_out = section.key == :gems ? "allow the licence there, or list the gem under `exceptions` with its reason" : "allow the licence there, or drop the dependency" + if strict + "#{label(section, component)} is under #{ids.join(", ")}, which includes #{(ids - allowed_ids).join(", ")}, off the allowed list in #{@path} " \ + "(every licence of this gem must be allowed): #{way_out} (#{DECISION})" + else + "#{label(section, component)} is under #{ids.join(", ")}, none of which is on the allowed list in #{@path}: #{way_out} (#{DECISION})" + end + end + + # The stale exceptions: for a gem the lockfile no longer names, one whose licences changed, or one the list now allows. + def exemption_problems + gems = @sections.find { it.key == :gems }&.components.to_a + exemptions.flat_map do |exemption| + component = gems.find { it.name == exemption.gem } + if !@locked_names.include?(exemption.gem) + [ "#{@path}: the exception for gem #{exemption.gem} is stale: the lockfile no longer names it (#{DECISION})" ] + elsif component.nil? + [] # in the lockfile and not installed (a run with a group left out): neither judged nor stale + else + stale_problem(exemption, component.identifiers.uniq.sort) + end + end + end + + def stale_problem(exemption, ids) + if ids != exemption.licences + [ "#{@path}: the exception for gem #{exemption.gem} is stale: it declares #{ids.join(", ")} now and the exception covers #{exemption.licences.join(", ")}; review it (#{DECISION})" ] + elsif allowed?(ids, strict: exemption.all_required) + [ "#{@path}: the exception for gem #{exemption.gem} is stale: the gem is allowed without it (#{DECISION})" ] + else + [] + end + end + end + end +end diff --git a/lib/tasks/licences.rake b/lib/tasks/licences.rake new file mode 100644 index 00000000..b9f25383 --- /dev/null +++ b/lib/tasks/licences.rake @@ -0,0 +1,21 @@ +# The licence check of spec 091: `bin/ci checks` runs it. It judges every gem of the bundle and every entry of the +# third-party manifest against config/licence_policy.yml, and reports spec 087's guard, so it needs the application. +namespace :licences do + desc "Fail when a dependency is under a licence config/licence_policy.yml doesn't allow (spec 091); optional policy path" + task :check, [ :policy ] => :environment do |_task, args| + policy_path = Rails.root.join(args[:policy].presence || Collector::ThirdPartyLicences::Policy::PATH) + begin + check = Collector::ThirdPartyLicences::Check.for_repository(policy_path:) + problems = check.problems + rescue SystemCallError, Psych::Exception => error + warn "Licences: can't read #{policy_path} or the licence manifest: #{error.message}" + exit 1 + end + + if problems.any? + warn "Licences: #{problems.size} problem#{"s" unless problems.one?}:", *problems.map { " - #{it}" } + exit 1 + end + puts "Licences: #{check.component_count} components judged; each is under an allowed licence or a named exception." + end +end diff --git a/spec/config/ci_definition_spec.rb b/spec/config/ci_definition_spec.rb index 3efe0d69..e0b15f63 100644 --- a/spec/config/ci_definition_spec.rb +++ b/spec/config/ci_definition_spec.rb @@ -35,6 +35,17 @@ expect(ratchet).to be < lines.each_index.find { |i| i > guarded && lines[i] == "end" } end + it "runs the licence check in bin/ci checks, directly after the importmap audit and not with the RSpec group (spec 091 AC-4.1)", :aggregate_failures do + lines = Rails.root.join("config/ci.rb").readlines(chomp: true).map(&:strip) + audit = lines.index('step "Security: Importmap vulnerability audit", "bin/importmap audit"') + licences = lines.index('step "Licences: dependencies are under allowed licences", "bin/rails licences:check"') + guarded = lines.index("unless selection.group == :rspec") + + expect(licences).not_to be_nil + expect(licences).to eq(audit + 1) + expect(licences).to be < lines.each_index.find { |i| i > guarded && lines[i] == "end" } + end + it "lets a session run bin/ci and its groups without a permission prompt (AC-1.8)" do allowed = JSON.parse(Rails.root.join(".claude/settings.json").read).dig("permissions", "allow") diff --git a/spec/lib/collector/third_party_licences/check_spec.rb b/spec/lib/collector/third_party_licences/check_spec.rb new file mode 100644 index 00000000..cea42406 --- /dev/null +++ b/spec/lib/collector/third_party_licences/check_spec.rb @@ -0,0 +1,24 @@ +require "rails_helper" +require "tmpdir" + +# Spec 091 decision 7, AC-4.4: Check joins an injected audit's problems and the policy's, the guard's first. +RSpec.describe Collector::ThirdPartyLicences::Check, type: :model do + def policy_reporting(*problems) = instance_double(Collector::ThirdPartyLicences::Policy, problems:, component_count: 3) + + def audit_reporting(*problems) = instance_double(Collector::ThirdPartyLicences::Audit, problems:) + + it "lists the audit's problems first and the policy's second, and counts the components the policy judged", :aggregate_failures do + check = described_class.new(audit: audit_reporting("pin has no entry"), policy: policy_reporting("gem x is under GPL")) + + expect(check.problems).to eq([ "pin has no entry", "gem x is under GPL" ]) + expect(check.component_count).to eq(3) + end + + it "carries the guard's sentence for an importmap pin with no manifest entry (decision 7)" do + pin = Collector::ThirdPartyLicences::Audit::Pin.new(name: "lodash", asset: "/x/vendor/javascript/lodash.js") + audit = Collector::ThirdPartyLicences::Audit.new(manifest: {}, text_dir: Dir.tmpdir, specs: [], ocr_digests: [], pins: [ pin ], + pin_directories: [], app_javascript: "/x/app/javascript", font_files: {}, icon_files: []) + + expect(described_class.new(audit:, policy: policy_reporting).problems).to eq([ "importmap pin lodash has no javascript entry" ]) + end +end diff --git a/spec/lib/collector/third_party_licences/policy_spec.rb b/spec/lib/collector/third_party_licences/policy_spec.rb new file mode 100644 index 00000000..a8e8fa2d --- /dev/null +++ b/spec/lib/collector/third_party_licences/policy_spec.rb @@ -0,0 +1,187 @@ +require "rails_helper" +require "tmpdir" + +# Spec 091 decisions 1, 2, 5 and 6 and Stories 1 and 2: the rules over injected sections and policy data, so each fails on a +# small fixture and passes on the baseline. The real tree is judged by the bin/ci checks step (AC-4.2). +RSpec.describe Collector::ThirdPartyLicences::Policy, type: :model do + let(:path) { "config/licence_policy.yml" } + + def component(name, *identifiers, version: "1.0", texts: []) + Collector::ThirdPartyLicences::Component.new(name:, version:, identifiers:, home: nil, copyright: nil, texts:, note: nil) + end + + def section(key, *components, title: key.to_s.capitalize) + Collector::ThirdPartyLicences::Section.new(key:, title:, components:) + end + + def data(allowed: %w[MIT BSD-2-Clause Ruby], exceptions: []) + { "allowed" => allowed.map { { "id" => it, "reason" => "a reason" } }, "exceptions" => exceptions } + end + + def problems_for(*sections, data: data(), locked: []) + described_class.new(data:, path:, sections:, locked_names: locked).problems + end + + it "passes a bundle of allowed licences, a dual among them (AC-1.5)" do + sections = [ section(:gems, component("rake", "MIT"), component("erb", "BSD-2-Clause", "Ruby")) ] + + expect(problems_for(*sections)).to be_empty + end + + it "fails a gem under a licence off the list, naming the gem, its version, the licence and the policy file (AC-1.1)", :aggregate_failures do + problems = problems_for(section(:gems, component("viral", "GPL-3.0-only", version: "2.1.0"))) + + expect(problems.size).to eq(1) + expect(problems.first).to include("gem viral 2.1.0", "GPL-3.0-only", path) + end + + it "passes a gem with any one allowed licence and fails one with none, listing every one (AC-1.2)", :aggregate_failures do + expect(problems_for(section(:gems, component("dual", "GPL-2.0-only", "MIT")))).to be_empty + expect(problems_for(section(:gems, component("copyleft", "GPL-2.0-only", "Artistic-1.0-Perl"))).first) + .to include("GPL-2.0-only", "Artistic-1.0-Perl") + end + + it "fails a gem that declares no licence, and one whose string is not an exact allowed id (AC-1.3, AC-1.4)", :aggregate_failures do + expect(problems_for(section(:gems, component("bare"))).first).to include("gem bare 1.0", "no licence") + expect(problems_for(section(:gems, component("filed", texts: [ [ "LICENSE", "text" ] ]))).first).to include("gem filed 1.0", "no licence") + %w[MIT\ License Apache\ 2.0 MIT\ OR\ Apache-2.0].each do |string| + expect(problems_for(section(:gems, component("odd", string))).first).to include(string) + end + end + + it "reports every offender once, in the order the sections list them, each naming the file (AC-1.6)", :aggregate_failures do + problems = problems_for(section(:javascript, component("alpha", "GPL-3.0-only")), + section(:gems, component("charlie", "AGPL-3.0-only"), component("bravo", "LGPL-3.0-only"))) + + expect(problems.size).to eq(3) + expect(problems).to all(include(path)) + expect(problems.map { it[/alpha|bravo|charlie/] }).to eq(%w[alpha charlie bravo]) + end + + it "judges a manifest entry of every section by its identifiers, with decision 2's rule (AC-2.1, AC-2.2)", :aggregate_failures do + %i[javascript ocr fonts icons].each do |key| + problems = problems_for(section(key, component("pkg", "GPL-3.0-only"), title: "Kind")) + + expect(problems.first).to include("Kind", "\"pkg\"", "GPL-3.0-only", path) + end + expect(problems_for(section(:fonts, component("two", "OFL-1.1", "MIT")))).to be_empty + expect(problems_for(section(:icons, component("none"))).first).to include("no licence") + end + + describe "the policy's own shape (AC-3.3)" do + def shape_problems(value) = problems_for(section(:gems), data: value) + + it "names an allowed entry without an id or a reason, a duplicate id, and a data that is not a mapping", :aggregate_failures do + expect(shape_problems({ "allowed" => [ { "id" => "MIT" } ] }).first).to include(path, "allowed entry 1") + expect(shape_problems({ "allowed" => [ { "reason" => "r" } ] }).first).to include("allowed entry 1") + expect(shape_problems({ "allowed" => [ { "id" => "MIT", "reason" => "r" }, { "id" => "MIT", "reason" => "r" } ] }).first).to include("MIT", "twice") + expect(shape_problems([ "MIT" ]).first).to include(path, "mapping") + end + + it "names an exception missing its gem, licences or reason, and a duplicate gem", :aggregate_failures do + bad = { "gem" => "x", "licences" => [], "reason" => "r" } + ok = { "gem" => "y", "licences" => [ "GPL-3.0-only" ], "reason" => "r" } + + expect(shape_problems(data(exceptions: [ bad ])).first).to include("exception entry 1", "(x)") + expect(shape_problems(data(exceptions: [ ok.except("reason") ])).first).to include("exception entry 1") + expect(shape_problems(data(exceptions: [ ok, ok ])).first).to include("gem y", "twice") + end + + it "names an exception whose `all_required` is neither true nor false, and accepts it absent or boolean", :aggregate_failures do + ok = { "gem" => "y", "licences" => [ "GPL-3.0-only" ], "reason" => "r" } + judged = ->(entry) { problems_for(section(:gems), data: data(exceptions: [ entry ]), locked: %w[y]) } + + expect(judged.call(ok.merge("all_required" => "true"))) + .to contain_exactly(a_string_including(path, "exception entry 1", "(y)", "all_required", "true or false")) + expect(judged.call(ok)).to be_empty + expect(judged.call(ok.merge("all_required" => true))).to be_empty + expect(judged.call(ok.merge("all_required" => false))).to be_empty + end + + it "reports one problem for a policy with no `allowed` list, and accepts an empty list", :aggregate_failures do + gems = section(:gems, component("rake", "MIT"), component("viral", "GPL-3.0-only")) + + expect(problems_for(gems, data: {})).to contain_exactly(a_string_including(path, "no `allowed` list")) + expect(problems_for(gems, data: { "exceptions" => [] }).size).to eq(1) + expect(problems_for(gems, data: { "allowed" => [] }).size).to eq(2) # valid shape: everything is judged and none is allowed + end + + it "judges nothing while the policy is malformed (it would report every gem)" do + expect(problems_for(section(:gems, component("viral", "GPL-3.0-only")), data: { "allowed" => "MIT" }).size).to eq(1) + end + end + + describe ".for_repository" do + it "reads the lockfile's gem names once each, with no `bundler` (AC-3.5)", :aggregate_failures do + names = described_class.locked_names + + expect(names).to include("rails", "sqlite3") + expect(names).to eq(names.uniq) + expect(names).not_to include("bundler") + end + + it "raises for a policy file it can't read, and reports a file that isn't a mapping (AC-3.3)", :aggregate_failures do + Dir.mktmpdir("licence-policy") do |dir| + list = File.join(dir, "list.yml") + File.write(list, "- MIT\n") + + expect { described_class.for_repository(policy_path: File.join(dir, "absent.yml"), sections: []) }.to raise_error(SystemCallError) + expect(described_class.for_repository(policy_path: list, sections: []).problems.first).to include(list.delete_prefix("#{Rails.root}/"), "mapping") + end + end + end + + describe "exceptions (decision 3)" do + let(:viral) { component("viral", "GPL-3.0-only") } + let(:exception) { { "gem" => "viral", "licences" => [ "GPL-3.0-only" ], "reason" => "a tool, never loaded" } } + + def judged(exceptions, *components, locked: %w[viral], allowed: %w[MIT]) + problems_for(section(:gems, *components), data: data(allowed:, exceptions:), locked:) + end + + it "lets a gem through that an exception names with exactly its licences, in any order (AC-3.1)", :aggregate_failures do + expect(judged([ exception ], viral)).to be_empty + two = { "gem" => "two", "licences" => %w[B A], "reason" => "r" } + expect(judged([ two ], component("two", "A", "B"), locked: %w[two])).to be_empty + end + + it "calls an exception stale when the lockfile no longer names the gem, the gem's licences changed, or it is allowed without it (AC-3.2)", :aggregate_failures do + expect(judged([ exception ], locked: [])).to contain_exactly(a_string_including("viral", "lockfile")) + changed = judged([ exception ], component("viral", "AGPL-3.0-only")) + expect(changed).to include(a_string_including("viral", "AGPL-3.0-only", "GPL-3.0-only", "stale")) + expect(changed.size).to eq(2) # the stale exception and the disallowed licence + expect(judged([ exception ], viral, allowed: %w[MIT GPL-3.0-only])).to contain_exactly(a_string_including("viral", "allowed without it")) + end + + it "judges nothing of a gem the lockfile names and the bundle doesn't hold, and calls no exception for it stale (AC-3.5)" do + expect(judged([ exception ], locked: %w[viral])).to be_empty + end + + it "applies to gems only: a manifest entry of the same name is judged by the list (AC-2.3)" do + problems = problems_for(section(:javascript, viral, title: "JavaScript"), data: data(exceptions: [ exception ]), locked: %w[viral]) + + expect(problems).to include(a_string_including("JavaScript entry", "GPL-3.0-only")) + end + + describe "`all_required`" do + let(:vcr) { component("vcr", "Hippocratic-2.1", "MIT") } + let(:strict) { { "gem" => "vcr", "licences" => %w[Hippocratic-2.1 MIT], "all_required" => true, "reason" => "its MIT is not trusted" } } + + it "passes a dual gem as its allowed id with no exception, and by name with the exception (AC-3.6)", :aggregate_failures do + expect(judged([], vcr, locked: %w[vcr])).to be_empty + expect(judged([ strict ], vcr, locked: %w[vcr])).to be_empty + end + + it "fails a gem whose exception's licences no longer match, because every licence must then be allowed (AC-3.6)", :aggregate_failures do + problems = judged([ strict ], component("vcr", "Hippocratic-2.1", "MIT", "BSD-2-Clause"), locked: %w[vcr], allowed: %w[MIT BSD-2-Clause]) + + expect(problems).to include(a_string_including("vcr", "Hippocratic-2.1", "every licence")) + expect(problems).to include(a_string_including("stale")) + end + + it "calls the exception stale once every licence is allowed (AC-3.6)" do + expect(judged([ strict ], vcr, locked: %w[vcr], allowed: %w[MIT Hippocratic-2.1])).to contain_exactly(a_string_including("vcr", "stale", "allowed without it")) + end + end + end +end diff --git a/spec/licences_core_spec.rb b/spec/licences_core_spec.rb index a20c57ea..18e27186 100644 --- a/spec/licences_core_spec.rb +++ b/spec/licences_core_spec.rb @@ -1,16 +1,19 @@ require "rails_helper" # Spec 087 AC-2.6, modelled on spec/footer_core_spec.rb: the files of the third-party licences page, its reader and its manifest name no -# collectible, source or rights holder. What the page lists is whatever the bundle and the manifest hold. +# collectible, source or rights holder. What the page lists is whatever the bundle and the manifest hold. Spec 091 AC-5.1 adds the licence +# policy, its check, the rake task and the policy file: what a licence check judges is a name and a set of SPDX ids, never a collectible. RSpec.describe "The third-party licences page's files" do let(:files) do %w[lib/collector/third_party_licences.rb lib/collector/third_party_licences/audit.rb config/third_party_licences.yml + lib/collector/third_party_licences/policy.rb lib/collector/third_party_licences/check.rb lib/tasks/licences.rake + config/licence_policy.yml app/controllers/licences_controller.rb app/views/licences/show.html.erb app/views/licences/_component.html.erb] .map { |path| Rails.root.join(path) } + Rails.root.glob("config/third_party_licences/*.txt") end it "covers the reader, the guard's rules, the controller, the views, the manifest and its texts", :aggregate_failures do - expect(files.size).to be > 6 + expect(files.size).to be > 10 expect(files).to all(exist) end diff --git a/spec/tasks/licences_rake_spec.rb b/spec/tasks/licences_rake_spec.rb new file mode 100644 index 00000000..199b649d --- /dev/null +++ b/spec/tasks/licences_rake_spec.rb @@ -0,0 +1,70 @@ +require "rails_helper" +require "rake" +require "tmpdir" + +# Spec 091 FR-3, AC-4.2 and AC-4.3. The step judges the repository's own policy in `bin/ci checks` (evidence in verification.md); +# here the task runs in-process, so a licence change in the bundle can't take a shard down: the passing run uses a policy that +# allows every licence the list declares, the failing one a policy that allows nothing. +RSpec.describe "licences rake task", type: :task do # rubocop:disable RSpec/DescribeClass -- rake tasks have no class + before { Rails.application.load_tasks unless Rake::Task.task_defined?("licences:check") } + + after { Rake::Task["licences:check"].reenable } + + # Runs the task with its output captured and `exit` rescued (RSpec doesn't rescue SystemExit, so an unrescued exit would end + # the whole process), as [standard output, standard error, exit status]. + def run_check(*args) + out = StringIO.new + err = StringIO.new + original = [ $stdout, $stderr ] + $stdout, $stderr = out, err + status = begin + Rake::Task["licences:check"].invoke(*args) + 0 + rescue SystemExit => error + error.status + end + [ out.string, err.string, status ] + ensure + $stdout, $stderr = original + end + + def with_policy(yaml) + Dir.mktmpdir("licence-policy") do |dir| + file = File.join(dir, "policy.yml") + File.write(file, yaml) + yield file + end + end + + def allow_everything_yaml + ids = Collector::ThirdPartyLicences.build.sections.flat_map(&:components).flat_map(&:identifiers).uniq + { "allowed" => ids.map { { "id" => it, "reason" => "a fixture" } }, "exceptions" => [] }.to_yaml + end + + it "exits 0 and prints how many components it judged on standard output (AC-4.2)", :aggregate_failures do + with_policy(allow_everything_yaml) do |file| + out, err, status = run_check(file) + + expect(status).to eq(0) + expect(out).to match(/Licences: \d+ components judged/) + expect(err).to be_empty + end + end + + it "exits 1 against a policy that allows nothing, printing each dependency, its licence and the file on standard error (AC-4.3)", :aggregate_failures do + with_policy("allowed: []\nexceptions: []\n") do |file| + _out, err, status = run_check(file) + + expect(status).to eq(1) + expect(err).to include("gem rake", "MIT", file) + expect(err.lines.size).to be > 100 + end + end + + it "exits 1 naming the file when it can't be read", :aggregate_failures do + _out, err, status = run_check("/nonexistent/policy.yml") + + expect(status).to eq(1) + expect(err).to include("/nonexistent/policy.yml") + end +end