editcheck-headless-vulns.md
medelius (Caro Medelius)
Actions

Authored By

	medelius
	Aug 14 2026, 10:37 PM

Size

16 KB

Referenced Files

None

Subscribers

None

editcheck-headless-vulns.md
View Options

	# EditCheck Headless Security Review

	Target: `gitlab.wikimedia.org/repos/mediawiki/services/editcheck-headless` @ `696b0c7` (branch `main`)
	Changes reviewed: none — base only, no pending merge requests
	Docs consulted: `README.md`, `.pipeline/blubber.yaml`, `service-utils.config.yaml`, `.gitlab-ci.yml`
	Reviewer: Claude (production-focused service audit), run by Caro Medelius
	Date: 2026-08-14

	---

	## Methodology / Scope

	Attacker model. Two actors:

	1. An anonymous or registered wiki user. They cannot send bytes to this service
	directly — the API "is fully internal, it will never be publicly accessible."
	They can edit a wiki page, which the JobQueue then schedules for checking,
	so page content reaches the headless browser. This is the only demonstrated
	public ingress into any part of the system.
	2. A caller on the internal network (the JobQueue, or anything that can route
	to the pod). There is no authentication or authorization on either transport, so
	this actor is trusted by construction.

	Out of scope: WMF operators and cluster insiders; `Special:EditCheckHeadless`
	and the VisualEditor/EditCheck extension code it loads (different repo — see
	recommendation 1); the Linked Artifact Cache itself.

	Reviewed in full (2,551 lines): `editcheck-headless-server.js` (818),
	`editcheck-headless.js` (932), `editcheck-headless-grpc.js` (258),
	`editcheck-headless-sitematrix.js` (91), `editcheck-headless-metrics.js` (287),
	`editcheck-headless-tracing.js` (165), plus `.pipeline/blubber.yaml`,
	`.gitlab-ci.yml`, `service-utils.config.yaml`, `.npmrc`, `package-lock.json`.

	Verification level. Static tracing only. **The service was never started and no
	trigger was fired.** Every claim below is a code-reading argument. Items marked
	[verify] depend on behaviour or configuration not observable from this repo.

	---

	## Summary

	No findings.

	No Critical, High, Medium, or Low finding survived the reachability test. Six
	code-level issues were identified and all six are recorded under *Not currently
	exploitable* — each is a real property of the code, and none has a demonstrated
	attacker path today.

	The two most substantial (arbitrary-URL SSRF, and Chrome running `--no-sandbox`)
	are held back for specific reasons stated inline. Both are cheap to fix and both
	are worth fixing; neither is a live vulnerability on the evidence gathered here.

	---

	## Not currently exploitable

	### 1. `wiki` / `wiki_id` accepts any URL; the sitematrix allowlist is never enforced

	Location: `editcheck-headless-sitematrix.js:73-86`, reached from
	`editcheck-headless-server.js:485` (`/check`, `/config`) and
	`editcheck-headless-grpc.js:162` (`GetRevisionArtifact`).

	```js
	if ( /^https?:\/\//i.test( trimmed ) ) {
	return trimmed.replace( /\/+$/, '' ); // any URL, returned verbatim
	}
	```

	The bug. The service fetches the full sitematrix at startup
	(`editcheck-headless-server.js:670`) and holds ~900 valid wiki base URLs in memory,
	but uses that map only as a dbname lookup. Any `http(s)://` value bypasses it — no
	host check, no port restriction, no block on RFC-1918, `localhost`, or link-local.
	The value flows to `page.goto()` (`editcheck-headless.js:529`), where Chrome loads
	and executes whatever the origin serves, and to `fetch()`
	(`editcheck-headless-grpc.js:101`) from the pod's network position.

	Why it is not a finding. No named ingress. `wiki` and `wiki_id` are supplied by
	the JobQueue over an internal-only API; no public path that injects an
	attacker-controlled value was identified. The remaining argument for filing it —
	that the network boundary is the only control and lives outside this repo — reduces
	to "operators must not misconfigure the NetworkPolicy," which is not an attacker
	capability. It stays here until someone demonstrates a public path that reaches the
	`wiki` field.

	Fix worth applying anyway (the allowlist already exists in memory):

	```js
	if ( /^https?:\/\//i.test( trimmed ) ) {
	const normalized = trimmed.replace( /\/+$/, '' );
	if ( !allowArbitraryUrls && !Object.values( dbnameMap ).includes( normalized ) ) {
	throw new Error( `Wiki URL not in the sitematrix: "${ normalized }"` );
	}
	return normalized;
	}
	```

	Gate `allowArbitraryUrls` behind an off-by-default flag so `--wikis http://localhost`
	still works locally. Two follow-ups, since validating only the first URL is a known
	bypass: neither `fetch()` sets a redirect policy and `page.goto()` follows
	redirects, so set `redirect: 'manual'` and re-validate `Location`; and if the
	JobQueue and LAC can always send a dbname, delete the URL branch in production
	instead of validating it.

	### 2. Chrome runs `--no-sandbox` with site isolation disabled

	Location: `editcheck-headless.js:20-55` (`CHROME_ARGS`) — `--no-sandbox`,
	`--disable-site-isolation-trials`, `--renderer-process-limit=1`, and
	`--disable-features=IsolateOrigins,site-per-process`.

	The bug. Three mitigations are disabled together: the OS renderer sandbox,
	cross-origin process separation, and per-origin renderers. The in-code rationale
	for `--no-sandbox` is "required when running as root"; Blubber images do not run as
	root by default, so that specific justification may not hold [verify] — the
	likelier real constraint is that Chrome's sandbox needs user namespaces or
	`CAP_SYS_ADMIN` that the pod security policy denies.

	Ingress (this one is real). Any wiki user edits a page → the JobQueue schedules
	a check → `Special:EditCheckHeadless` fetches that page's Parsoid HTML and runs it
	through VE. Attacker-authored content reaches a renderer with no OS sandbox. That
	is a genuine public path into the browser component.

	Why it is not a finding. Exploitation requires a Chrome renderer vulnerability,
	which was not demonstrated and is not asserted to exist. A removed mitigation with
	no working trigger is defence-in-depth, not a live vulnerability. Note this rests on
	the manual review alone — semgrep's JS/Node packs have no Puppeteer or browser-flag
	coverage, so nothing corroborates it.

	Fix worth applying anyway. Confirm whether the pod can grant unprivileged user
	namespaces or a seccomp profile that lets the sandbox initialise; if so, drop
	`--no-sandbox` and keep `--disable-dev-shm-usage`. If it genuinely cannot, document
	that as a deployment constraint and compensate (restrictive seccomp/AppArmor,
	read-only rootfs, dropped capabilities, egress-limiting NetworkPolicy). The site
	isolation flags were added for resource savings (commit `f042819`); measure what
	they actually save before keeping them, given the shared cross-origin session
	(item 4).

	### 3. Outbound `fetch()` calls set no timeout

	Location: `editcheck-headless-grpc.js:101-106`, `editcheck-headless-sitematrix.js:51-56`.

	Neither call sets a `signal`. The service defines a 90 s `--timeout-ms` budget and
	applies it to `page.goto()` and the result poll, but not to these; they fall back to
	undici's default header/body timeouts (300 s in current undici [verify]), over
	3× the service's own budget. Not a finding: the trigger is a slow upstream wiki,
	not an attacker, and `resolvePageTitle` runs before work is enqueued so it does not
	block the browser session. Fix: pass the existing budget through as
	`signal: AbortSignal.timeout( timeoutMs )` (needs threading into the gRPC deps).

	### 4. All non-pinned wikis share one serialized browser session

	Location: `editcheck-headless.js:832-840` (`getSession`), `:700-715` (`enqueue`).

	Wikis not named in `--wikis` share a single session running one task at a time
	through an unbounded promise chain. `addQueued` increments a gauge but rejects
	nothing, and each task can hold the session for up to 90 s, so N queued requests
	serialize into N × 90 s. The same session also re-navigates between origins,
	sharing one `BrowserContext` (cookies, storage) across different wikis. **Not a
	finding:** the only actors who can queue work are trusted, and the trigger is load,
	not an attacker. Fix: cap queue depth and shed with HTTP 503 /
	`RESOURCE_EXHAUSTED`; `editcheck_headless_queue_depth` already exists to alert on.
	Separately, use a fresh `BrowserContext` per wiki switch to remove the cross-origin
	state question entirely.

	### 5. Permissive CORS on a service no browser should call

	Location: `editcheck-headless-server.js:210-215`, applied by `sendJson` at `:226`.

	Every response carries `Access-Control-Allow-Origin: `. Not a finding:*
	`Access-Control-Allow-Credentials` is not set, so cookies are not sent and
	credentialed responses are not exposed, and the data returned (suggestion counts for
	public pages) is not sensitive. Fix worth applying: delete `setCorsHeaders` and
	the `OPTIONS` branch. This is a machine-to-machine API; there is no client to break.

	### 6. Raw exception messages in HTTP 500 bodies

	Location: `editcheck-headless-server.js:557`, `:641`.

	`sendError( res, 500, e.message )` returns the raw exception text; puppeteer and
	undici messages routinely embed the failing URL, host and port. Stack traces are
	correctly kept to the log (`:556`, `:640`). Not a finding: only trusted internal
	callers can reach it. Fix: return a generic message plus a correlation ID —
	`x-request-id` is already propagated (`editcheck-headless-tracing.js:39`).

	### 7. `parsoidHtml` POST body reintroduces a rejected trust model

	`POST /check` accepts up to 10 MiB of caller-supplied HTML
	(`editcheck-headless-server.js:589-595`) and feeds it to the browser
	(`editcheck-headless.js:247-254`). `project-description.txt` explicitly rejects
	"taking results from the user's client" because clients "could generate bad data
	either accidentally or maliciously"; this path reopens a narrower version, and
	results flow into the shared Linked Artifact Cache. Not a finding: only trusted
	callers reach it. Worth raising as a design question — if the parameter is not yet
	used in production, consider removing it until it is.

	### 8. gRPC listener uses `createInsecure()`

	`editcheck-headless-server.js:787`. No TLS and no auth. Standard for WMF services
	behind an Envoy mesh sidecar that terminates TLS — but that assumption appears
	nowhere in this repo. Semgrep independently flagged the sibling
	`http.createServer` (`:698`) for the same reason. [verify] the mesh config and
	document the assumption (recommendation 2).

	---

	## Reviewed without finding

	- No injection into browser JS. Every `page.evaluate()` passes values as
	arguments rather than interpolating into source (`editcheck-headless.js:248-253`,
	`:140`, `:150-159`). `title` — the obvious candidate — cannot break out.
	- Prototype pollution. `dbnameMap` is built with `Object.create( null )` in both
	places (`editcheck-headless-sitematrix.js:15`, `editcheck-headless-server.js:667`),
	so the lookup at `:81` cannot be steered via `__proto__` or `constructor`. Reads
	as deliberate.
	- No dangerous sinks. No `eval`, `new Function`, `vm.*`, `innerHTML`,
	`document.write`, or deserialization of untrusted data.
	- `execSync` and `spawn` are not injectable. `getconf CLK_TCK` / `getconf PAGESIZE`
	are constant strings (`editcheck-headless-server.js:249`, `:256`); `spawn`
	(`editcheck-headless.js:354`) passes an argv array with no shell and an
	operator-controlled binary. The Lightpanda path is off by default.
	- Request body cap. 10 MiB, socket destroyed on overflow (`:177-203`).
	- Metric cardinality is bounded on purpose. `endpointLabel()` buckets unknown
	paths, commented "labelling each one would let a scanner create time series at
	will" (`editcheck-headless-metrics.js:44-56`). Metrics also sit on a separate port
	(9102), deliberately off the API route.
	- gRPC input validation. `page_id` checked against `/^\d+$/` and rejected when
	zero (`editcheck-headless-grpc.js:154-158`).
	- Health probes. Answered before logging and metrics so probes cannot swamp
	either; liveness/readiness split correctly reasoned (`:493-510`).
	- No secrets, in tree or history. `gitleaks` over all 19 commits: no leaks. An
	independent working-tree pattern scan was also clean.
	- Human-readable source only. No generated or minified build artifacts committed
	(satisfies the T217351 point of emphasis).

	---

	## Dependencies

	Two independent scanners, both clean: `npm audit` — 0 vulnerabilities
	(`--package-lock-only --omit=peer --omit=optional`, inside `fresh-node`);
	`osv-scanner` — no issues, 145 packages. The same OSV scan runs in CI on every
	pipeline via `repos/security/gitlab-ci-security-templates` `generic-osv/osv-ci.yaml`
	(ref `v0.1.6`), so this is continuously rechecked. That job is `allow_failure`, so a
	new advisory reports without blocking a deploy — a reasonable trade, but a
	deliberate one worth stating.

	`package-lock.json` (lockfileVersion 3) is structurally sound: 146 entries, all
	HTTPS, zero missing integrity hashes, two resolved hosts (`registry.npmjs.org`
	×145, `gitlab.wikimedia.org` ×1). 146 total packages for a service that drives a
	browser, speaks gRPC and exports OTel traces is a small tree, and there is no
	webpack in it.

	Version drift (`npm outdated`) — no known vulnerabilities, so these are
	maintenance notes. Two cannot update on their own: `puppeteer-core` 25.3.0 → 25.7.0
	(exact pin, no range; it is the browser driver) and `@grpc/proto-loader` 0.7.15 →
	0.8.1 (`^0.7.15` does not cross to 0.8.x). The `@opentelemetry/*` packages are one
	release behind (0.220.0 → 0.221.0, `instrumentation-undici` 0.30.0 → 0.31.0).
	Everything else is current.

	Blind spots — a clean scan does not cover these:

	- Debian `chromium`, installed unpinned by `.pipeline/blubber.yaml`. Not in the
	lockfile; no npm-ecosystem scanner will ever see it. Per item 2 it is the
	component that parses attacker-authored content with its sandbox disabled, and it
	is scanned by nothing today.
	- `@wikimedia/service-utils` 2.0.0 — the one first-party dependency, resolved
	from the GitLab registry rather than npmjs. `osv-scanner` reported 145 packages
	against 146 in the lockfile and this is the only non-npmjs entry, so it is
	near-certainly the one skipped: OSV maps packages to ecosystems by registry.
	[verify] with `osv-scanner --verbosity=info`. Not alarming — first-party code
	with a valid integrity hash — but the dependency closest to home is the one
	neither tool checks.
	- `eslint` `^8.57.1` is end-of-life. Neither scanner flagged it, so EOL here means
	unmaintained, not known-vulnerable. Dev-only and pruned from the production image.

	---

	## Recommendations

	1. **Review `Special:EditCheckHeadless` and the EditCheck extension alongside this
	repo.** This service is a ~2,500-line driver with no complex parsing of its own;
	the actual processing of hostile input (Parsoid HTML → VE data model → edit
	checks) happens in the special page and the RL modules it loads, in a different
	repo. Reviewing this service alone gives limited assurance. Both should be
	scoped together.
	2. Document the trust model in the repo. The "fully internal, never publicly
	accessible" claim exists only in the Phabricator description. Record it next to
	the deployment config along with what it rests on: which NetworkPolicy enforces
	it, that Envoy terminates TLS for the insecure gRPC listener (item 8), and that
	both listeners bind `0.0.0.0` (`.pipeline/blubber.yaml`).
	3. Attach the NetworkPolicy / deployment-charts config to the review request.
	Items 1 and 5 both reduce to "the network boundary is the only control," and that
	cannot be assessed from this repo. This is the single highest-value addition.
	4. Add regression tests for the security-relevant rejections. The suite tests
	that an unknown dbname 400s (`test/server.test.js:186-190`); the URL-passthrough
	branch has no test at all, which is part of why it reads as unintentional. If the
	item 1 allowlist lands, test that an off-sitematrix URL also 400s.
	5. Set `min-release-age = 7` in `.npmrc`. Makes npm refuse to resolve versions
	published in the last 7 days — the standard defence against a compromised
	maintainer publishing a malicious version. Scope it honestly: the lockfile is
	committed and builds install from it with integrity hashes, so this hardens the
	dependency-update workflow, not production. [verify] the npm in
	`nodejs24-slim` is ≥ 11.10; an unrecognised key is silently ignored, which would
	leave the appearance of protection and none of the substance.
	6. Track the Chromium version. It is unpinned in `.pipeline/blubber.yaml` and
	invisible to every dependency scanner, while being the most security-critical
	dependency in the image.
	7. Review Lightpanda separately if it is ever adopted. The code supports it as
	an alternative engine, but it is a young engine with a much smaller security
	track record than Chromium; switching should trigger its own review.

File Metadata

Mime Type: text/plain
Storage Engine: blob
Storage Format: Raw Data
Storage Handle: 27626307
Default Alt Text: editcheck-headless-vulns.md (16 KB)

editcheck-headless-vulns.mdmedelius (Caro Medelius)Actions

editcheck-headless-vulns.mdView Options

File Metadata

Event Timeline

editcheck-headless-vulns.md
medelius (Caro Medelius)
Actions

editcheck-headless-vulns.md
View Options