Add name squatting admin dashboard and exact match publish time refusal logic - #2081
autumnfound wants to merge 4 commits into
Conversation
0060c12 to
3337bed
Compare
|
Report generated by Claude Opus 5 when requested to evaluate the performance with similar to production numbers for the impact of checking for a same-named extension with the new index. TLDR: Actual lookup performance is very fast and should not be noticable in a running environment. We should consider doing some real world testing on staging to ensure that there is no significant impact with real data. Performance: display name conflict checkWhat runs, and when The check has two components. Both are gated so that a publication is only checked when it adopts a display name — i.e. when the extension is new, or when this version renames it. Both call sites — checkPublishPreconditions (scanning enabled only, fails fast before scanners are occupied) and doPublish (every publication) — apply the same gate. MethodPostgreSQL 16.15; shared_buffers=2GB, effective_cache_size=6GB, work_mem=32MB, random_page_cost=1.1. Synthetic dataset shaped from current registry growth (~50–100 new extensions/day, ~450 new versions/day), carrying the full index set from V1_7, V1_10, V1_13, V1_22, V1_37 plus the new V1_72 index, and the personal_access_token / user_data / signature_key_pair tables that findLatestVersion joins. Latency via pgbench -M prepared (server-side prepared statements, as pgjdbc uses), single connection, 10s per measurement, warm cache. Display names are held stable across an extension's releases, as they are in practice — so a name carried by N extensions matches N × (their version counts) rows in the index, not N rows. That is the realistic and less favourable shape. Conflict query — findActiveExtensionByDisplayNameRename comparison — findLatestVersionTotal per publicationArithmetic sums of the Scale A p50 figures above, with scanning enabled so both call sites run: At ~100 new extensions and ~450 version publishes per day, that totals under 100 ms of database time per day. With scanning disabled only the doPublish call site runs, roughly halving each figure. Verification
Reading notes
Caveats
|
There was a problem hiding this comment.
Pull request overview
Adds end-to-end support for detecting and moderating name squatting: a new admin dashboard section for reviewing NAME_SQUATTING findings and taking actions (clear as false-positive or soft-delete), plus a publish-time “exact display name” collision check to reject obvious impersonation attempts (while excluding the publisher’s own namespaces and skipping checks when mirroring).
Changes:
- Web UI: new “Name Squatting” admin dashboard route/view with filtering, paging, per-state counts, and moderation dialogs + unit tests.
- Server: new admin API endpoints and service logic to list/count flagged extensions and apply moderation actions.
- Publish pipeline + DB: reject publications that adopt a display name already used by another active extension; add a supporting partial expression index.
Reviewed changes
Copilot reviewed 34 out of 34 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| webui/test/unit/support/name-squatting-data.ts | Adds test fixtures for name-squatting flags/counts. |
| webui/test/unit/pages/admin-dashboard/name-squatting/name-squatting.spec.tsx | Adds Name Squatting page behavior tests (listing, filtering, actions). |
| webui/test/unit/pages/admin-dashboard/name-squatting/name-squatting-row.spec.tsx | Adds row-level UI tests (actions/disabled states/findings expansion). |
| webui/test/setup.ts | Adds jsdom getComputedStyle workaround for MUI dialog calc() issue. |
| webui/src/pages/admin-dashboard/name-squatting/use-name-squatting.ts | Adds React Query hooks + query keys for name-squatting endpoints. |
| webui/src/pages/admin-dashboard/name-squatting/name-squatting.tsx | Implements Name Squatting admin page (filters, counts, paging, actions). |
| webui/src/pages/admin-dashboard/name-squatting/name-squatting-row.tsx | Implements per-extension flagged row UI and findings table. |
| webui/src/pages/admin-dashboard/name-squatting/name-squatting-action-dialog.tsx | Implements confirmation dialog + error display for moderation actions. |
| webui/src/pages/admin-dashboard/admin-dashboard.tsx | Adds nav tile + route to Name Squatting section. |
| webui/src/pages/admin-dashboard/admin-dashboard-routes.ts | Adds route constant for name-squatting page. |
| webui/src/extension-registry-types.ts | Adds frontend types for name-squatting list/count/action payloads. |
| webui/src/extension-registry-service.ts | Adds admin service methods hitting /admin/name-squatting/* endpoints. |
| webui/CHANGELOG.md | Documents new admin dashboard Name Squatting section. |
| server/src/test/java/org/eclipse/openvsx/repositories/RepositoryServiceSmokeTest.java | Covers new repository methods in smoke test execution list. |
| server/src/test/java/org/eclipse/openvsx/repositories/ExtensionJooqRepositoryTest.java | Adds tests for display-name conflict lookup behavior. |
| server/src/test/java/org/eclipse/openvsx/RegistryAPITest.java | Updates handler wiring due to new constructor parameter. |
| server/src/test/java/org/eclipse/openvsx/publish/PublishExtensionVersionHandlerTest.java | Adds tests for publish-time display-name collision rejection/exclusions. |
| server/src/test/java/org/eclipse/openvsx/admin/NameSquattingAPITest.java | Adds HTTP-layer tests for the new name-squatting admin API. |
| server/src/test/java/org/eclipse/openvsx/admin/NameSquattingAdminServiceTest.java | Adds unit tests for moderation grouping/counting/actions. |
| server/src/main/resources/db/migration/V1_72__ExtensionVersion_DisplayName_Index.sql | Adds index backing the display-name conflict check. |
| server/src/main/java/org/eclipse/openvsx/repositories/RepositoryService.java | Exposes display-name conflict lookup + flagged-extension moderation queries. |
| server/src/main/java/org/eclipse/openvsx/repositories/ExtensionValidationFailureRepository.java | Adds moderation queries: list/count flagged extensions + delete failures by extension. |
| server/src/main/java/org/eclipse/openvsx/repositories/ExtensionJooqRepository.java | Implements findActiveExtensionByDisplayName using latest active version semantics. |
| server/src/main/java/org/eclipse/openvsx/publish/PublishExtensionVersionHandler.java | Adds “adopts display name” logic + conflict checks (skipped for mirrors). |
| server/src/main/java/org/eclipse/openvsx/json/NameSquattingTargetJson.java | Adds API JSON model for moderation targets. |
| server/src/main/java/org/eclipse/openvsx/json/NameSquattingFlagListJson.java | Adds API JSON model for paginated flags response. |
| server/src/main/java/org/eclipse/openvsx/json/NameSquattingFlagJson.java | Adds API JSON model for a flagged extension (grouped findings). |
| server/src/main/java/org/eclipse/openvsx/json/NameSquattingFindingJson.java | Adds API JSON model for individual findings. |
| server/src/main/java/org/eclipse/openvsx/json/NameSquattingCountsJson.java | Adds API JSON model for per-state totals. |
| server/src/main/java/org/eclipse/openvsx/json/NameSquattingActionResultJson.java | Adds API JSON model for per-target action outcome. |
| server/src/main/java/org/eclipse/openvsx/json/NameSquattingActionResponseJson.java | Adds API JSON model for bulk action response. |
| server/src/main/java/org/eclipse/openvsx/json/NameSquattingActionRequest.java | Adds API JSON model for moderation requests. |
| server/src/main/java/org/eclipse/openvsx/admin/NameSquattingAPI.java | Adds admin endpoints for list/count/clear/delete name-squatting findings. |
| server/src/main/java/org/eclipse/openvsx/admin/AdminService.java | Implements moderation listing/counting/actions + state filter parsing. |
Suppressed comments (1)
webui/src/pages/admin-dashboard/name-squatting/name-squatting-action-dialog.tsx:88
- Same as above:
color='warning.main'is likely treated as an invalid CSS color string. Prefersx={{ color: 'warning.main' }}to reliably use the theme palette warning color.
<Typography variant='body2' color='warning.main'>
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| {flag?.findingCount === 1 ? 'finding' : 'findings'} recorded for{' '} | ||
| <strong>{extensionId}</strong>? | ||
| </Typography> | ||
| <Typography variant='body2' color='warning.main'> |
| <Tooltip | ||
| title={ | ||
| flag.activeVersionCount === 0 | ||
| ? 'The extension has no active versions left to deactivate' | ||
| : '' | ||
| }> |
To best fulfill the requirements of an easy to use system to manage typosquatting, this PR was created. This leverages the existing NAME_SQUATTING publish check that uses levenshtein distance to check for similarly named extensions post-publish. The new dashboard view allows an admin to either mark a found check as a false positive and clear the check, or to soft-delete any extensions that are found to be maliciously typo squatting another extension. Any admin action taken will also be logged in the admin log table to make auditing these changes possible. This check is left as unenforced as the check is too sensitive, and has many false positives. This does provide all of the features that were requested in the initial PRD for management of the system, outside of exclusion keywords. Part of eclipse-openvsx#1949 Assisted-by: Claude Opus 5
…isplay name matches To best accomadate the current infrastructure, a new check was added to the publish workflow. This check will compare the display name of new extensions and new versions to existing data. For new extensions, exact matches will be blocked from upload. In the case of versions, extensions whose name changes to conflict will be blocked, but existing names will not be blocked. This carve out was added to address any existing conflict of display name without potentially blocking a legitimate version from being submitted. This grandfathered clause makes the assumption that the name squatting results will be monitored and addressed to get to a known good state. Once we get to a clean state, the grandfathered clause will effectively work as a standard exact match gate without any notable performance impact. Assisted-by: Claude Opus 5
Assisted-by: Claude Opus 5
d92db4e to
1f1cbe2
Compare
|
Failure to publish due to exact name duplicate: Example name-squatting result: 
|
netomi
left a comment
netomi
left a comment
There was a problem hiding this comment.
Reviewed the full diff (34 files, +4409/−11), reading each hunk against the surrounding code — ExtensionProcessor, ExtensionService, ExtensionVersionJooqRepository.findLatestQuery, SecurityConfig and the entity mappings.
Five findings below, one of which I think is a release blocker: the display-name gate reads a different name at its two call sites, and for packages without a <DisplayName> that can lock a publisher out with no way back.
Things I checked and found correct, so they need no second look:
- the jOOQ correlated subquery in
findActiveExtensionByDisplayName— theev_matchalias avoids shadowing the unaliasedEXTENSION_VERSIONinfindLatestQuery, null-filteringexcludeNamespacesbeforeNOT INis right, andaddLimit(1)makesfetchOnesafe - migration
V1_74—lower(btrim(display_name))matches jOOQ'slower(trim(...))after Postgres canonicalisation, the partialWHERE activematches the query predicate, andSET LOCAL lock_timeoutis valid because Flyway runs migrations in a transaction - native-query parameter positions in
findFlaggedExtensionKeys/countFlaggedExtensionsare consistent across all three layers, and theRepositoryServiceSmokeTestadditions do exercise the SQL deleteExtensionNoWaitself-invocation is fine — the transaction boundary isExtensionService.deleteExtension, a different bean/admin/name-squatting/**falls under the/admin/**→ROLE_ADMINmatcher and is not CSRF-exempt;jsonMutationHeaders()supplies the token- escalation paths through target platforms, pre-releases and out-of-order version publishes all still reach
adoptsDisplayName
🤖 Generated with Claude Code
| } | ||
| var latestVersion = repositories | ||
| .findLatestVersion(namespace.getName(), extensionName, null, false, true); | ||
| if (adoptsDisplayName(latestVersion, processor.getDisplayName())) { |
There was a problem hiding this comment.
The two call sites of the display-name gate use different display names, which can lock a publisher out permanently.
checkPublishPreconditions uses processor.getDisplayName(), which falls back to the extension name when the manifest's <DisplayName> is blank (ExtensionProcessor:366-372). createExtensionVersion (lines 290/316) uses extVersion.getDisplayName() out of processor.getMetadata(...), which leaves it null in exactly that case (ExtensionProcessor:317-320), and checkDisplayNameConflict returns early on blank.
Failure: an extension published from a VSIX with no <DisplayName> stores display_name = NULL on every version. On the next publish, adoptsDisplayName(latest, processor.getDisplayName()) compares "" (the stored null) against "<extension-name>", so it is always true and the conflict check runs on every version bump — with the extension name standing in for a display name the package never carries. If any other active extension's latest version happens to have that string as its display name, the publisher is locked out for good: the grandfathering carve-out at lines 353-366 never applies, and no admin action clears it, since this is independent of the NAME_SQUATTING findings.
It is also configuration-dependent: with scanService.isEnabled() == false, checkPublishPreconditions is never called (ExtensionService:179), so the same package publishes fine.
Before this PR processor.getDisplayName()'s fallback only labelled a scan record (ExtensionScanService:87), where standing in the extension name is a display convenience rather than an authoritative name. Both sites should read the same value.
| </Paper> | ||
| )} | ||
|
|
||
| {flags.length > 0 && ( |
There was a problem hiding this comment.
Clearing the last row on a later page strands the admin with no way back.
TablePagination renders only inside {flags.length > 0 && (...)}. Filter to 11 flags, page to page 2 (offset 10, one row), clear that row: useClearNameSquattingFlags invalidates the query, it refetches with the unchanged offset: 10, the server returns flags: [], and the pagination control disappears along with the rows. The page shows "No extensions are flagged for name squatting" and there is no control left to return to page 0 — only a reload.
The same happens whenever AdminService.getNameSquattingFlags drops rows (AdminService.java:825-829 skips keys whose findings vanished between the key query and the read-back) while totalSize still counts them.
Rendering the pagination whenever totalSize > 0, and clamping page when page * pageSize >= totalSize, covers both.
| </HeaderRow> | ||
|
|
||
| <InlineGroup> | ||
| {rejected ? ( |
There was a problem hiding this comment.
Findings for a rejected publication can never be cleared.
For state === 'REJECTED' both actions are replaced by the explanation. Soft delete is rightly unavailable — nothing exists to delete — but "Mark as false positive" is a pure findings-delete that the server handles fine here: AdminService.clearNameSquattingFindings only calls deleteValidationFailures and never looks the extension up.
As written, findings recorded for a blocked publication stay in the list forever and keep inflating counts.total and counts.rejected — which is the housekeeping this page exists to do.
| json.setFindings(failures.stream().map(this::toNameSquattingFindingJson).toList()); | ||
|
|
||
| if (extension == null) { | ||
| json.setState(NAME_SQUATTING_STATE_REJECTED); |
There was a problem hiding this comment.
The row's state and the SQL state filter use different definitions.
The row computes PUBLISHED as extension.isActive() && activeVersions > 0, while findFlaggedExtensionKeys / countFlaggedExtensions classify on e.active = true alone. An extension with active = true and no active versions is counted in counts.published and returned for state=PUBLISHED, but renders as DEACTIVATED.
ExtensionService.updateExtension keeps the two in step today, so this is latent rather than broken — but the divergence will eventually hand an admin a filter that returns rows labelled with the state they filtered out.
| return document.createElement('div').style; | ||
| } | ||
| }; | ||
|
|
There was a problem hiding this comment.
Nit: the only change to this file is an added trailing blank line, unrelated to the feature. Worth dropping to keep the diff to the change.
This patch contains 2 new features. The first is a new admin dashboard tile for monitoring and addressing name_squatting flagged extensions + versions in a new view. This will allow for each of the versions to be resolved as false positive or to soft delete an extension if they are found to be malicious.
The second feature is a publish-time check that compares the display name to other active extensions outside of the users' namespaces. This does a case insensitive match on the display name, and if there is a match either for new extensions or a renamed new version of an extension, the upload gets rejected.
There is currently a small carve out in the second feature for currently existing extensions that collide as we cannot know which is the legitimate version and we don't want to immediately reject those uploads. It is assumed that the NAME_SQUATTING check results will be monitored and resolved to remove these conflicts as they are surfaced.
Resolves #1949.