Page MenuHomePhabricator
Create Task
Maniphest T432708

Application Security Review Request : EditCheck server-side suggestions
Open, HighPublic5 Estimated Story Points
Actions

Description

Project Information

Description of the tool/project:
EditCheck suggestions are generated in the client, using logic that evaluates the VisualEditor data model. We would like to show these suggestions outside of the editor, initially as a counter in read mode. We have considered multiple approaches for doing this:

  • Running VE in Node. Cons: while we had some success with core VE, we also have lots of MediaWiki dependencies (VE code in extensions, MW libraries, API calls) and this quickly becomes running all of MW in Node.
  • Creating server-side duplicates of all the checks that run directly against Parsoid HTML. Cons: This would requires rewriting and maintaining a duplicate copy of most of the EditCheck system, as well as many parts of VE's converter and some MW dependencies.
  • Taking results from the user's client and posting them to the server. Cons: Clients could generate bad data either accidentally (broken browser, or browser with excessive extensions installed) or maliciously.

The approach we have decided on is to run EditCheck headless via a very lightweight entry point (Special:EditCheckHeadless). VE will happily run headless (we already do this for visual diffs in the user's client) as will the edit check logic. The data can then be sent back through an API via Puppeteer.

Description of how the tool will be used at WMF:
Edit suggestions will be counted after a page has been edited (triggered by a JobQueue). The results of the count will be inserted into the Linked Artifact Cache. The API is fully internal, it will never be publicly accessible.

  • editcheck-headless-server opens a headless browser at /Special:EditCheckHeadless
    • this page can be locked down as much as needed - it only needs to load RL modules, and make action API calls.
  • an internal request from the JobQueue comes in, asking for the suggestion count for <Page>
  • <Page>'s Parsoid HTML is fetched in the headless browser, loaded into VE and the edit check logic run
    • Parsoid HTML could also be fetched in advance and POSTed to the API, if required (still, only internally)
  • the result is handed back to Puppeteer
  • the browser can be shutdown after each request, or be kept open for performance and restarted every N requests (to avoid memory leaks)

We are open to using headless Chrome, or more lightweight render-free headless browsers like Lightpanda which uses ~10x fewer resources (CPU/memory).

Dependencies

List dependencies, or upstream projects that this project relies on.

  • Puppeteer (open to alternatives like WebDriver if required)
  • Chromium headless (open to alternatives like Lightpanda)
  • gRPC

Has this project been reviewed before?
No

Working test environment
e.g.

node editcheck/build/editcheck-headless/editcheck-headless-server.js  --script-path <script-path> --wikis http://localhost

Post-deployment
Editing, contacts as above

Details

Risk Rating
Low
Author Affiliation
WMF Product

Related Objects
Search...

StatusSubtypeAssignedTask
 OpenNoneT360489 [EPIC] Offer edit suggestions as a service
 OpenNoneT416531 [EPIC] Surface Edit Suggestions outside of VE
 OpenNoneT431076 [EPIC] Surface specific edit suggestions within articles (Phase 2)
 OpenppelbergT428816 [EPIC] Surface edit suggestion counts to logged-in editors on articles (Phase 1)
 OpenppelbergT430712 [EPIC] Launch controlled experiment of surfacing suggestion counts on articles to logged-in editors
 OpenNoneT426778 [EPIC] Streamline the suggestion browsing and publishing experiences
 OpenEsandersT425597 Suggestion Mode feedback toasts block me from advancing to next suggestion
 OpendchanT423465 [SPIKE] Investigate creating a top-level model class for Edit Check
 OpenGGoncalves-WMFT428814 [EPIC] Ready server-side suggestions PoC for use in production
 OpenppelbergT432266 [SPIKE] Define scale requirements for suggestion count controlled experiment (logged-in users)
 OpenppelbergT432272 Add instrumentation to log suggestion counts shown in read and edit modes
 OpenNoneT432274 [Suggestion Counts on articles] Report on leading indicators
 OpenNoneT432288 [SPIKE] Determine cache invalidation approach for on-wiki config changes
 OpenMNeislerT432427 Publish experiment plan: read mode suggestion counts (logged-in)
 OpenOttomataT432733 Server Side Edit Suggestions - Linked Artifacts Cache precompute job
 OpenbmartinezcalvoT412373 Suggestion Mode: Prioritize suggestions shown to newcomers in Suggestion Mode for the first time
 OpenNoneT432622 Prompt people to publish after completing first suggestion
 DeclinedNoneT432759 Conduct usability testing of new post-Suggestion publishing prompt
 OpenbmartinezcalvoT412389 Suggestion Mode: Conduct moderated usability testing to understand how users use Suggestion Mode
 ResolvedBUG REPORTmedeliusT413280 Suggestion mode - Don't expand any cards until clicked
 OpenNoneT428193 Suggestion Mode: Entry point in read mode
 OpencjmingT431493 [DE 1.1.9] An instrument that produces edit_survived events
 OpenNoneT433530 Backfill Edit Suggestion Counts in Linked Artifacts Cache for technical pilot
 OpenNoneT432286 [MILESTONE] Run technical pilot to evaluate Headless VE resource requirements
 OpenEsandersT428811 [EPIC] Deploy headless VE
 OpenRscoutT432708 Application Security Review Request : EditCheck server-side suggestions
 OpenNoneT431320 Resolve tension between prominence of 'Help' and 'Suggestion Mode'
 OpenNoneT434960 Decide how Suggestion Counts and caching pageviews for logged-in users will work with one another

Event Timeline

Esanders created this task.Tue, Jul 21, 10:26 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptTue, Jul 21, 10:26 AM
Peachey88 renamed this task from Application Security Review Request : ... to Application Security Review Request : EditCheck server-side suggestions.Tue, Jul 21, 10:29 AM
Esanders added a parent task: T428811: [EPIC] Deploy headless VE.Tue, Jul 21, 11:04 AM
ldelench_wmf subscribed.Tue, Jul 21, 2:04 PM
ldelench_wmf added a project: OKR-Work.Tue, Jul 21, 2:08 PM
sbassett moved this task from Incoming to Back Orders on the secscrum board.Wed, Jul 22, 3:59 PM
ppelberg edited projects, added: Editing-team (FY2026-27 Q1 sprint 3); removed: Editing-team.Tue, Aug 4, 12:21 AM
ppelberg triaged this task as High priority.Tue, Aug 4, 11:15 PM
ppelberg moved this task from Incoming to Ready for Dev on the Editing-team (FY2026-27 Q1 sprint 3) board.
ppelberg set the point value for this task to 5.Wed, Aug 5, 5:18 PM
Rscout claimed this task.Thu, Aug 13, 3:37 PM
sbassett moved this task from Back Orders to In Progress on the secscrum board.Thu, Aug 13, 3:37 PM
medelius subscribed.Fri, Aug 14, 9:03 PM
medelius added a comment.Fri, Aug 14, 10:29 PM

Alright, to get us started I ran some of the tooling from the how-to in the meantime:

ToolScopeResult
npm auditpackage-lock.json (--package-lock-only --omit=peer --omit=optional)0 vulnerabilities
osv-scannerpackage-lock.jsonNo issues, 145 packages
semgrep21 tracked files, 274 rules3 findings - 1 valid (Low), 2 false positives, see below
gitleaksFull git history, 19 commitsNo leaks, full 19-commit history

semgrep triage:

VulnerabilityPackageNotesServiceRemediationRisk
This .npmrc does not set a minimum release age or sets it too low. Newly published packages can be malicious or unstable. Add min-release-age = 7 to wait 7 days before resolving newly published package versions. Added in: v11.10. Reference: https://github.blog/changelog/2026-02-18-npm-bulk-trusted-publishing-config-and-script-security-now-generally-available/.npmrc:1Valid. Newly published packages can be malicious; npm resolves them immediately. Scope is narrow, the lockfile is committed and builds install from it with integrity hashes, so this hardens the dependency-update workflow, not production.semgrepAdd min-release-age = 7. Verify the npm in nodejs24-slim is ≥ 11.10; an unrecognised key is silently ignored.Low
A hardcoded username in plain text is identified. Store it properly in an environment variable.editcheck-headless-grpc.js:44-45False positive. The flagged constant is the outbound User-Agent string, which WMF API policy requires. No credential exists; the njsscan rule matched on the identifier USER_AGENT.semgrepNone, code is correct as written.None
Checks for any usage of http servers instead of https servers. Encourages the usage of https protocol instead of http, which does not have TLS and is therefore unencrypted. Using http can lead to man-in-the-middle attacks in which the attacker is able to read sensitive information.editcheck-headless-server.js:698False positive in context. Plain HTTP on a pod-local listener is the standard WMF pattern - Envoy terminates TLS. Corroborates the same undocumented assumption as the gRPC createInsecure() listener.semgrepDocument the TLS-termination assumption in the repo.Informational
medelius added a comment.Fri, Aug 14, 10:40 PM

And the report generated by the standalone-vuln-audit Claude skill that Riku recommended. The findings are reasonable, imo.

editcheck-headless-vulns.md16 KBDownload

ppelberg moved this task from Ready for Dev to Dev Doing on the Editing-team (FY2026-27 Q1 sprint 3) board.Fri, Aug 14, 11:23 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits