Aman-Mittal opened a new pull request, #378:
URL: https://github.com/apache/fineract-backoffice-ui/pull/378
Closes the one Category X licence in the dependency tree, and turns the
policy that found it into a gate so the next one fails the build instead of an
audit.
## Business Value
The release-readiness audit found exactly one licence that could stop a
release: an LGPL-3.0 lint plugin. It was never bundled and never distributed,
but it was named in `package.json`, and `package.json` travels in a source
release — so it was a question the PMC would have had to answer before a vote
rather than one that could be scoped out.
Removing it takes that question off the table. Adding the gate means the
next one never gets as far as a release review: a Category X or unclassified
licence now fails CI on the pull request that introduces it, at the point where
the person who added it is still holding the context. Auditing for this by hand
once per release is how a dependency sits in a tree for two releases before
anyone notices.
Running the end-to-end suite on every push does the same thing for
behaviour. It is the only thing that exercises this application against a real
Fineract, and it previously waited for a pull request to open — long enough for
a branch to be several commits deep in a broken state before anything said so.
## What changed
**Removed** `eslint-plugin-sonarjs` — LGPL-3.0-only, ASF Category X.
**Added** the three requested replacements, licences verified from the
installed packages rather than assumed:
| Plugin | Version | Licence |
|---|---|---|
| `eslint-plugin-unicorn` | 65.0.1 | MIT |
| `eslint-plugin-security` | 4.0.1 | Apache-2.0 |
| `eslint-plugin-import` | 2.32.0 | MIT |
| `eslint-import-resolver-typescript` | 4.4.5 | ISC |
`unicorn` is pinned to `^65` rather than the current major: from 66 onwards
it requires ESLint ≥ 10.4 and this project is on 9. Forcing that resolution is
not an option — the `Dependency Integrity` job exists to forbid
`--legacy-peer-deps`.
The `unopinionated` unicorn set is used rather than `recommended`, which is
substantially stylistic (filename casing, abbreviation expansion, `for…of` over
`.forEach`) and would rewrite a great deal of working code without fixing a
defect.
## Cognitive complexity is preserved
It was enabled at `error` through the removed plugin's recommended set, so
removing the plugin would have dropped it silently. There was nothing
permissive to take instead: `eslint-plugin-cognitive-complexity` on npm is a
security placeholder, and the alternatives are single-author packages at v0.x.
Trading a licence problem for a supply-chain one is not a trade.
So it is implemented in `eslint-rules/cognitive-complexity.js`, from the
published metric rather than from anyone's source, **at the same threshold of
15**.
Its tests assert **exact scores** at a threshold of one below, not merely
that something was reported, because the way this rule would fail quietly is by
drifting stricter or laxer than what it replaced while still looking like it
works. That is not hypothetical — an early draft counted `??` as a branch and
failed this, on a real file:
> `loan-product-form.component.ts` has a template handler holding 23 `??`
defaults and almost no other branching. The previous rule passed it; the draft
scored it 23 and failed it.
`??` states a default rather than branching, so it is not counted, and a
test pins that. 26 tests, run by CI.
## The licence gate
`scripts/check-dependency-licenses.mjs`, wired into `License Compliance`:
```
Category A PASS permissive
Category B REVIEW reported for PMC awareness, does not fail
Category X BLOCK
Unknown BLOCK legal review
```
Current state, over the whole tree:
```
Dependency licences — 1197 packages (production and dev)
Category A 1188 pass
Acknowledged 7 pass, recorded BlueOak-1.0.0
Category B 2 review axe-core, @axe-core/playwright
(MPL-2.0)
Category X 0 block
Unknown 0 block
```
Production alone: **25 packages, all Category A**.
Three decisions worth review rather than assumption:
- **Unknown blocks.** The failure worth preventing is not a GPL dependency
arriving with a banner; it is a `license` field reading `SEE LICENSE IN
LICENSE.txt` arriving on a transitive bump. Treating that as a pass makes the
check decorative. Clearing one means identifying the licence or recording it in
`ACKNOWLEDGED` with a reason — not widening a category list until the build
goes green.
- **`ACKNOWLEDGED` exists** because the ASF's lists do not cover every SPDX
identifier in a modern npm tree. `BlueOak-1.0.0` is the only entry: plainly
permissive, named in neither list, reaching this tree only through build
tooling. Silently calling it Category A would be inventing a policy decision,
so it is written down and marked pending PMC confirmation.
- **devDependencies are in scope**, deliberately. They are not distributed,
but they are named in `package.json` — which is exactly how the plugin this PR
removes came to matter.
SPDX expressions are resolved rather than string-matched: `OR` takes the
branch that resolves best (the project may pick), `AND` takes the worst (every
term binds).
### The gate is tested, because a clean tree proves nothing
`npm run check:licenses:selftest` runs 26 expressions through the classifier
and asserts each verdict — `LGPL-3.0-only` → X, `MPL-2.0` → B, `(MIT OR
GPL-3.0)` → A, `(MIT AND GPL-3.0)` → X, undeclared → Unknown. CI runs it beside
the scan. A green scan over a clean tree says nothing about whether the rules
still bite; this is what says it.
Writing it surfaced a bug in itself worth naming: `\bOR\b` matches the `or`
inside `GPL-2.0-or-later`, because hyphens are word boundaries — so the
expression was detected as compound and then not split, and the classifier
recursed until the stack went. The compound test now has the same shape as the
split.
The existing `license-checker` allow-list stays. The two read npm's metadata
by different routes, so a package one misreads is still caught by the other.
## E2E on every push
```yaml
push:
branches: ['**']
```
The `pull_request` trigger stays alongside it rather than being replaced: a
fork's pushes land in the fork, so `push` alone would leave external
contributions untested. **A branch in this repository with an open PR will
therefore get both runs** — the same commit tested twice. Dropping
`pull_request` removes the duplicate at the cost of fork coverage; flagging it
rather than leaving it to be discovered in the Actions bill.
Superseded runs are now cancelled everywhere except `main` and `develop`,
where the point of the run is the record that the commit passed. Without that,
pushing three times in five minutes queues three full Fineract bring-ups.
## Two autofixes reverted, because lint cannot type-check
Both stayed green under ESLint and failed `npm run build`:
- `unicorn/prefer-array-find` rewrites `.filter(p).pop()` to `.findLast(p)`,
which needs lib ES2023; `tsconfig` targets ES2022 → **TS2550**.
- `unicorn/dom-node-dataset` rewrites `setAttribute('data-theme', …)` to
`dataset.theme`, and `noPropertyAccessFromIndexSignature` is on → **TS4111**.
Both rules are now off with that reasoning recorded beside them. Worth
stating plainly: had the build not run, this PR would have merged code the
compiler rejects.
## Rules switched off, each with its reason
The one worth naming is **`unicorn/prefer-global-this`** (106 reports).
Enforcing it would let `globalThis.localStorage` past `no-restricted-globals`,
which is a trust boundary (`security.md` §4), not a style preference. A style
rule does not get to open a hole in it.
The others: `security/detect-object-injection` (all 35 reports were
`obj[key]` with a typed key), `unicorn/no-array-for-each`,
`unicorn/no-negated-condition`, `unicorn/no-useless-undefined`,
`unicorn/prefer-top-level-await`.
## What has no replacement
A migration that claims parity it does not have is worse than one that names
its gaps. `DOCS/LINT_POLICY.md` records the full mapping; these genuinely have
no permissive equivalent:
- **`no-duplicate-string`** — this project had it on at `error`. It is why
`NAV_CONFIG` names its icons as constants. Review is now the only thing
catching a repeated literal.
- **`no-hardcoded-passwords`, `no-hardcoded-ip`, `sql-queries`** — small
loss here: the application issues no SQL, and CodeQL plus repository
secret-scanning cover credentials from another angle.
- **`no-identical-functions`, `no-duplicated-branches`,
`no-nested-conditional`, `no-invariant-returns`** — the deeper dataflow rules.
- **`deprecation`** — `@typescript-eslint/no-deprecated` covers it but needs
typed linting, which changes the cost of every lint run. Worth doing
deliberately, not in passing.
- **`assertions-in-tests`, `no-skipped-tests`, `no-empty-test-file`,
`no-fixed-wait-in-tests`**.
## Testing
Every gate run on this branch and observed:
| Gate | Result |
|---|---|
| `lint:prune` | pass |
| HTML lint | pass |
| `test:eslint-rules` | **26/26** |
| `format:check` | pass |
| `i18n:check`, `check:icons` | pass |
| `check:internal-endpoints`, `check:route-permissions` | pass |
| `typecheck:e2e` | pass |
| `api:surface` | pass — 142 services, 564 operations |
| `check-license.sh` | pass |
| `check:licenses:selftest` | **26/26 expressions** |
| `check:licenses:production` | pass — 25 packages, all Category A |
| `check:licenses` | pass — 0 Category X, 0 unclassified |
| `build` | pass |
| Unit tests | **1093 SUCCESS** |
| `ga:check` | 8/9, **0 blocking**; adapter backlog unchanged at 416 |
## Suppression baseline
27 pre-existing violations of the new rules are recorded in
`eslint-suppressions.json` rather than fixed here — 15 of them
`unicorn/no-array-sort`, which wants `toSorted()` in place of an in-place
`.sort()`. That is a real bug class (mutating an array the caller still holds)
and a semantic change; it does not belong in a licensing PR.
The file is a ratchet, not an amnesty: CI runs `--prune-suppressions`, which
fails when it lists a violation that no longer exists. The count only falls.
The 416 adapter-boundary suppressions the GA gate counts are untouched.
## Breaking changes
None to the application. Two for contributors: a Category X or unclassified
dependency licence now fails CI, and every push runs the end-to-end suite.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
