-
Notifications
You must be signed in to change notification settings - Fork 331
feat(labrinth): overhaul malware scanner report storage and routes #4233
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR overhauls Labrinth's malware scanner (Delphi) integration by replacing a webhook-based approach with a structured database-backed system. The changes enable enhanced reviewer workflows by providing REST API endpoints for report management, searching, filtering, and tracking moderation status.
Key changes:
- Removed inline Delphi webhook calls during file upload in favor of database-driven malware scanning
- Added comprehensive database schema for storing Delphi reports, issues, and Java class analysis data
- Implemented new internal API routes for report ingestion, querying, and issue management
Reviewed Changes
Copilot reviewed 14 out of 20 changed files in this pull request and generated 4 comments.
Show a summary per file
File | Description |
---|---|
apps/labrinth/src/routes/v3/version_creation.rs |
Removes inline Delphi webhook calls and related parameters from file upload process |
apps/labrinth/src/routes/v3/project_creation.rs |
Removes CDN URL parameter from project creation flow |
apps/labrinth/src/routes/mod.rs |
Adds new Delphi-specific error type for API error handling |
apps/labrinth/src/routes/internal/mod.rs |
Registers new internal Delphi routes module |
apps/labrinth/src/routes/internal/delphi.rs |
Implements comprehensive Delphi API endpoints for report management |
apps/labrinth/src/routes/internal/admin.rs |
Removes legacy Delphi webhook endpoint and related functionality |
apps/labrinth/src/database/models/version_item.rs |
Integrates Delphi analysis trigger into file creation workflow |
apps/labrinth/src/database/models/delphi_report_item.rs |
Defines complete database model for Delphi reports, issues, and Java classes |
apps/labrinth/migrations/20250810155316_delphi-reports.sql |
Creates database schema for Delphi report storage |
Files not reviewed (6)
- apps/labrinth/.sqlx/query-0080a101c9ae040adbaadf9e46fbc457a08e70dcde320c6852074819e41f8ad9.json: Language not supported
- apps/labrinth/.sqlx/query-0ed2e6e3149352d12a673fddc50f9530c311eef084abb6fce35de5f37d79bcea.json: Language not supported
- apps/labrinth/.sqlx/query-10a332091be118f580d50ceb7a8724e9a4d5b9765d52305f99f859f939c2e854.json: Language not supported
- apps/labrinth/.sqlx/query-8f1f75d9c52a5a340aae2b3fd863153f5e9796b55ae753ab57b14f37708b400d.json: Language not supported
- apps/labrinth/.sqlx/query-c1cd83ddcd112e46477a195e8bed0a1658c6ddf7a486082cdb847fab06150328.json: Language not supported
- apps/labrinth/.sqlx/query-fe571872262fe7d119b4b6eb1e55d818fde0499d8e5a08e9e22bee42014877f3.json: Language not supported
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
I assume this doesn't apply to the |
It should check the Modrinth-Admin header or check if the authenticated user has the relevant perms, not just rely on the Modrinth-Admin header as there's no way to protect that in the frontend |
Now all routes except the ingestion one use the conventional |
9c5b4c8
to
a2cce68
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few more things.
This provides better error messages.
b75ecfb
to
069ad7a
Compare
Overview
This PR introduces a comprehensive overhaul of Labrinth's integration with our in-house malware scanner, codenamed Delphi, replacing the previous basic webhook-based approach, which handled Delphi scan reports by sending Slack messages and posting text messages to project threads, with a more scalable, strongly-typed, database-backed report storage system that other systems can interact with via new REST API routes.
The goal of this overhaul is to lay the groundwork for enhanced project reviewer workflows related to Delphi reports. Such workflows require reviewers to search, filter, and sort reports for review independently of their associated projects, re-run Delphi analysis on existing projects, and track the review status of specific reports. With a proper backend system in place to support those actions, we expect to boost productivity and overall platform health by making it more efficient to moderate large volumes of projects to a consistently high standard.
✨ New HTTP Routes
All routes listed below require authentication as a moderator user with a token that has the
PROJECT_READ
scope. Machine to machine authentication for the ingestion route is done via a fixed admin key passed in theModrinth-Admin
header, which Labrinth reads from theLABRINTH_ADMIN_KEY
environment variable. For the default local deployment environment included with this repository, the key isfeedbeef
.POST /_internal/delphi/ingest
: Receives malware analysis reports from the Delphi scanner, stores them in the database for future querying, and sends Slack notifications for them. This route supersedes the previousPOST /_internal/admin/_delphi
endpoint.file_id
anddelphi_version
fields. Thefile_id
field enables reports to be linked to their specific file within a version, improving traceability. Thedelphi_version
field allows reports to be classified by the Delphi version that generated them, which is useful for re-analyzing suspicious projects after deploying improved Delphi detection capabilities, for example.POST /_internal/delphi/run?file_id=<base62 ID>
: Requests Delphi to re-analyze the specified version file. If the current Delphi version is newer than the version used for the file's last analysis, a new report linked to the same project but a different Delphi version is created. Otherwise, the existing report is updated in-place with the new run's findings, and statuses of both new and repeated issues are reset to pending review.file_id
field is now included in the Delphi trigger webhook payload, which Delphi should handle gracefully.GET /_internal/delphi/version
: Retrieves the latest Delphi version used for any report stored in the database as a JSON number. Returnsnull
if no Delphi reports have been generated yet.GET /_internal/delphi/issues?type=<Delphi issue type>&status=<pending|approved|rejected>&order_by=<created_asc|created_desc|pending_status_first|severity_asc|severity_desc>&count=<N>&offset=<N>
: Returns a JSON list of Delphi issue objects matching the specified criteria in the requested order. All query parameters are optional; when omitted, Labrinth applies no filtering, sorting, or count limits to the result list, depending on what was omitted. Theoffset
parameter enables pagination.Example issue list
PUT /_internal/delphi/issue/<issue ID>
: Updates the specified Delphi issue object if it exists, or creates a new one if it doesn't. Clients can determine whether an issue was created or modified by checking the response code:201 Created
indicates a new issue was created, while204 No Content
indicates an existing issue was updated. This endpoint is intended for operations like changing the moderation status of specific issues.GET /_internal/delphi/issue_type/schema
: Retrieves the schema that defines issue types for the current Delphi version. The schema is a JSON object whose keys enumerate the recognized issue types, and its values another object that defines metadata for each issue type. This route is meant to be primarily used by the frontend for validation, UI generation, and general introspection about Delphi issue types. Labrinth caches schemas for 60 seconds to reduce the load on Delphi, as schemas are not expected to change frequently.