fix(scan): stop reducing scoped packages to the part after the slash - #85
Merged
Conversation
detectTyposquat stripped the scope before comparing against the popular package list, so `@capacitor/core`, `@babel/core`, `@angular/core`, `@nestjs/core` and `@sentry/core` all reduced to `core` — one edit from `cors` — and were reported as typosquats at high severity. Any scoped package whose name sat one edit from something popular hit the same path, which put the rule's loudest finding on some of the most widely installed packages in the ecosystem. A scope is owned: publishing `@babel/anything` requires control of the `@babel` scope, so a squat cannot be planted inside a legitimate one, and nobody typing `npm i cors` arrives at `@capacitor/core`. The misreading this rule models does not cross the scope boundary, so scoped names are no longer candidates. No detection is lost. Stripping the scope never compared scopes at all, only the segment after the slash, so a lookalike scope (`@babeljs/core` for `@babel/core`) went uncaught before and still does — catching it needs a list of popular scopes, which is a separate check. Verified against ionic-team/capacitor: 21 findings to 19, removing both manifest-typosquat reports and nothing else. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ThreatCrush Security Scan144 finding(s) HIGH/CRITICAL: 12 | MEDIUM: 96 | LOW: 36
…and 94 more. Full results in the Security tab. Snippets are redacted; ThreatCrush never prints matched credential material. |
This was referenced Aug 10, 2026
ralyodio
added a commit
that referenced
this pull request
Aug 10, 2026
…ge guard (#88) Release 0.6.0. Two languages the scanner claimed to support had no rules at all. `shell` and `php` were both in `ScanLanguage`, both mapped from extensions, and between them zero of the 46 code rules targeted either. From the outside they looked supported: files were read, matched against the secret rules, and reported clean whatever the code did. Language detection made it worse — it was extension-only, so an executable named for the command it provides rather than the language it is written in was never opened. `ralyodio/debtap`, 3,511 lines of bash in a file called `debtap`, scanned clean by scanning nothing and exited 0 while doing it. - Shebang detection for extensionless files, from a 128-byte prefix so a checked-in blob costs one small read rather than a megabyte decoded and discarded. - Seven shell rules: remote script execution, eval on an expansion, unquoted expansion in a recursive remove, disabled certificate verification, plain-HTTP download, world-writable permissions, predictable temp paths. - Eight PHP rules: SQL interpolation, shell interpolation, dynamic code execution, dynamic include, unserialize on request data, unescaped output, request-driven path traversal, extract() variable injection. - A language-coverage test that fails when a language the scanner claims has no rule targeting it. Both gaps above existed because nothing checked the two lists against each other; now something does. Also carries the two fixes stranded on intermediate branches when the stack was merged out of order: scoped packages are no longer reduced to the part after the slash (#85), and XXE requires XML evidence in the file (#86). Every rule is built against the corrected shape as well as the vulnerable one. The eval rule matches eval's argument rather than the whole line: the line-wide form counts bash's dynamic-range idiom, where every expansion is arithmetic and cannot carry a command, and reported it 355 times in debtap alone. debtap: 0 findings (nothing scanned) to 8, all genuine. capacitor: unchanged at 13. Tests: 100, up from 68. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
detectTyposquatstripped the scope before comparing a dependency name against the popular-package list:So
@capacitor/corebecamecore, andeditDistance("core", "cors") === 1— reported as a typosquat ofcorsat high severity.This is not a one-package quirk. Every scoped package named
corehits it:@capacitor/corecors@babel/corecors@angular/corecors@nestjs/corecors@sentry/corecorsAny scoped package whose bare name sits one edit from something popular takes the same path (
@some-org/expres→express). The rule's loudest finding was landing on some of the most widely installed packages in the ecosystem.The fix
Scoped names are no longer candidates for this check.
Scopes are owned. Publishing
@babel/anythingrequires control of the@babelscope, so a squat cannot be planted inside a legitimate one, and nobody typingnpm i corsarrives at@capacitor/core. The misreading this rule models — a name you skim past in a diff — does not cross the scope boundary.No detection is lost
Stripping the scope never compared scopes at all, only the segment after the slash. A lookalike scope (
@babeljs/corefor@babel/core) was uncaught before this change and is uncaught after it. Catching that needs a list of popular scopes to compare against, which is a separate check rather than a variation on this one — noted in a comment for whoever picks it up.Verification
Against
ionic-team/capacitorat5e5bb3b, same tree, same invocation: 21 findings → 19. Bothmanifest-typosquatreports removed, nothing added, no other finding perturbed.Three tests added. Two of them fail on
masterwith exactly the assertions above (corsfor@capacitor/core,expressfor@some-org/expres); the third pins the unscoped case (cores→cors) so the exemption can't become a way to smuggle a bare name past the check.vitest runinapps/cli: 65 passed.tsc --noEmit: clean.