Skip to content

fix(overrides): correct OA008 pluralization and order copies by version - #1181

Merged
sonukapoor merged 2 commits into
OWASP:mainfrom
prx-my:feature/issue-1179-copyies-sort
Sep 23, 2026
Merged

sonukapoor merged 2 commits into
OWASP:mainfrom
prx-my:feature/issue-1179-copyies-sort

Conversation

@prx-my

@prx-my prx-my commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

Closes #1179

Summary

Fixes two cosmetic bugs in the OA008 details string when more than one
copy is below the floor:

  • copyies → copies via the existing pluralize helper
  • versions listed lexicographically → version-ordered via compareVersions

Changes

  • src/overrides/detectors/oa008-materialized.ts
    • import { pluralize } from "../../utils/string.js"
    • .sort() → .sort(compareVersions) (already imported and used a few
      lines up, no duplicate sort implementation introduced)
    • copy${n === 1 ? "" : "ies"} → pluralize(n, "copy", "copies")
  • tests/overrides/detectors/oa008.test.ts
    • regression test feeding three postcss copies below a >=8.11.0 floor,
      asserting 3 installed copies and version order 8.2.0, 8.9.0, 8.10.0
      (the fixture distinguishes semantic order from lexicographic order)

Verification

  • npm run lint:tests ✓
  • npm run build ✓
  • node dist/index.js advisories sync ✓ (228435 records)
  • npm test ✓ (145 suites, 1922 tests)
  • Confirmed the new test fails against the old implementation
    (3 installed copyies (8.10.0, 8.2.0, 8.9.0)) and passes with the fix
    (3 installed copies (8.2.0, 8.9.0, 8.10.0))

Copilot AI lite review requested due to automatic review settings September 20, 2026 08:57
@prx-my
prx-my requested a review from sonukapoor as a code owner September 20, 2026 08:57

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@MRX-72

MRX-72 commented Sep 20, 2026

Copy link
Copy Markdown
Collaborator

Well done @prx-my — you followed the issue's outline and implemented it cleanly: reused the existing pluralize and compareVersions helpers, kept the diff minimal, and the new regression test actually distinguishes lexicographic from version order (good catch on the fixture). Thanks for picking this up.

Verified against the latest PR head (0244190): npm test passes — 145 suites, 1934 tests — and all 6 CI checks are green (build, CodeQL, Analyze, CVE Lite CLI, self-scan, self-scan-action).

@sonukapoor if you're happy with it, kindly merge when you get a chance. Thanks!

@sonukapoor sonukapoor left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified rather than eyeballed: merges cleanly with current main, 145 suites and 1934 tests, and the new test is discriminating. Reverting oa008-materialized.ts to main while keeping the test fails it on exactly the assertion you would want, "pluralizes copies and lists versions in version order".

Reusing pluralize rather than open-coding the branch is the right call, and sorting with compareVersions where it was already imported a few lines above avoids a second sort implementation. Small and clean.

One thing this does not cover, and it is not yours to fix here: the same message says the listed copies are "below the floor", which is wrong when a copy sits above the ceiling instead. ^1.2.0 with a 2.0.0 copy mislabels it on main today. Filing that separately, since #1183 is about to make it fire more often.

Approving. Holding the merge until next week rather than stacking it on today's, so these land on separate days.

@sonukapoor sonukapoor left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-read this against current main before merging. Both fixes are real: the plural produced "copyies", and the lexicographic sort put 8.10.0 ahead of 8.2.0. The test fails on the old behaviour, which is what I wanted to see.

@sonukapoor
sonukapoor merged commit 740bfdc into OWASP:main Sep 23, 2026
6 checks passed
@sonukapoor

Copy link
Copy Markdown
Collaborator

Merged, thank you @prx-my!

@sonukapoor sonukapoor mentioned this pull request Sep 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] OA008 message reads "copyies" and lists versions in lexicographic order

4 participants