Security review of contrib extensions
- XWiki Contrib
- Design
- Active
- https://forum.xwiki.org/t/a-private-repository-for-the-security-teams-llm-tooling-xwiki-dev-security-llm/18885
- https://forum.xwiki.org/t/run-xwiki-review-llm-skill-automatically-every-day-on-new-commits/18832
- https://forum.xwiki.org/t/security-policy-handling-of-contrib-extensions-not-properly-maintained/18047
- https://forum.xwiki.org/t/security-policy-for-contrib-app-security-contact-clarification/18668
- https://forum.xwiki.org/t/report-security-issues-through-github-security-advisories-instead-of-jira/18951
- No
Description
Summary
Make sure that no version of an xwiki-contrib extension is released without a security review of its code. Before each release, the code changed since the last reviewed commit is reviewed by an LLM-based security review, and the release goes on once the commit is cleared.
The review is a project service, like SonarCloud: the developer releasing needs no AI account, runs nothing new by hand and never sees AI output. Nothing the review finds is ever public: findings follow the Security Policy like any other security issue.
This is a brainstorming: several points below are open and listed as such.
Context
- Contrib extensions are installed on many instances and are mostly maintained by one or two developers. A recent contrib release shipped security issues introduced in new code, which is what triggered this proposal.
- The security team now has LLM tooling for security reviews, in the private xwiki-dev-security-llm repository (forum topic 18885): a review of a change set, and the proving of a finding on a disposable local instance.
- Forum topic 18832 discusses running the multi-angle xwiki-review on commits or PRs. This proposal is different: it covers security only, and its output must stay private.
- A review lowers the risk of releasing a vulnerability; it does not certify that an extension is secure.
Definitions
- Security review
- An LLM review of an extension's code for security defects only, using the security team's private knowledge base. Its output is never public.
- Finding
- A defect the review reports. A proven finding has been reproduced with a proof of concept on a disposable instance; otherwise it is a candidate.
- Severe
- A CVSS score of 7 or more.
- Clearance
- The public record that one commit of one extension was reviewed and that its release would ship no known proven severe finding for the first time. The only output of the process that is ever public.
- Hold
- The situation of a commit whose release would ship a known proven severe finding for the first time. It publishes nothing.
- Override
- A release made without a clearance, with a reason given privately.
Proposal
1. Why at release time, and not on PRs or commits
- Findings must stay private. PR comments, commit comments, CI logs and forum digests are all public, so a security review cannot report there. A release check can answer with a single public bit.
- Contrib work often bypasses PRs. Many changes are committed directly. A release is the one point that every change passes through before reaching users.
2. The review before each release
- The developer pushes the commit to release. The review is requested privately: automatically by the release check (see 5), by the xwiki-contrib-release skill's preflight, or by hand.
- The service reviews from the commit's nearest cleared ancestor to the commit, or the whole code when there is none (the first time). This works for releases from maintenance branches too. It then proves the severe findings on a disposable instance.
- The commit is held when its release would ship a known proven severe finding for the first time: one in the reviewed range, or one left open from an earlier review of code not released yet. Nothing is published and the security team contacts the developer privately. Otherwise the commit is cleared.
- A finding already in a released version never holds a release: users are already exposed to it, and holding the next release would only delay its fixes. It is tracked as a confidential issue following the Security Policy.
- Candidates and findings with a CVSS under 7 never hold a release; they go to the security team's private queue.
Reviewing ahead of the release. The review takes time (about 20 minutes, with the proving), so a developer is encouraged to request it ahead of release day, as soon as the code to release is pushed, with mvn org.xwiki.contrib:xwiki-security-maven-plugin:1.0:request. That goal returns at once; the result comes privately (a clearance, or the security team's contact about a hold). The developer then has time to fix a hold before release day. On release day, the check only reviews the commits pushed since then, from the nearest cleared ancestor, which is usually small and fast, and often already cleared. A fix for a hold follows the Security Policy: its commit message does not reveal the vulnerability.
Soft-blocking from day one. A hold never means "you cannot release": it means "fix it, or say why you release anyway". A developer can release without a clearance by giving a reason (an override). The reason goes privately to the service, never into the release, a commit or a log, and the service reviews that release straight away. Overriding a confirmed hold ships a known vulnerability: it alerts the security team and is meant for emergencies. A developer who prefers to review by hand uses the same override.
Holds are filed by the service itself, with its own issue tracker account (never from the developer's machine), as confidential issues in the security tracker (JIRA today, see forum topic 18951), for the security team. A developer who is in the security team handles their own. The security team confirms or lifts a hold within a deadline (open question), after which the release can go on and the finding becomes an ordinary confidential issue.
3. Nothing about a hold is public
Saying "commit X of extension Y is held" would tell an attacker that there is a proven severe vulnerability in the latest code. So:
- Only clearances are public, worded "security-reviewed on <date> (knowledge base <version>): no new known severe issue", never "secure" or "safe". They are a bookkeeping record read by the release check and the audit, not a badge on extensions.xwiki.org.
- A held commit publishes nothing, so it looks the same as a commit under review or never submitted, which is the state of almost every commit.
- The review request is private too: it goes through a relay on XWiki infrastructure that authenticates the developer with the nexus.xwiki.org credentials they already have for releasing (open question).
4. Code sent to an LLM provider
The service reads only code that is already public on GitHub (the commit must be pushed). There is no opt-out from being reviewed: anyone, attackers included, can already run an LLM over public code, and a contrib security advisory is published under xwiki.org's name. A developer who objects reviews by hand and overrides.
5. The release check
The contrib parent POMs add the check to the goals release:prepare already runs in its forked build (clean install, from xwiki-commons). It therefore runs only when a release is being prepared (not in normal builds, CI or release:perform), once per extension, and before anything is built, committed or tagged:
<pluginManagement>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-release-plugin</artifactId>
<configuration>
<preparationGoals>clean org.xwiki.contrib:xwiki-security-maven-plugin:1.0:check install</preparationGoals>
</configuration>
</plugin>
</plugins>
</pluginManagement>The check verifies that the commit is pushed, looks up its clearance (immediate when the developer requested the review ahead of time, see 2), requests the review if there is none, and otherwise stops with the same message whether the commit is held, still under review or the service is unavailable, explaining how to override. If the service is unavailable, the developer overrides with "service unavailable": the service never blocks a release by being down.
Two escape hatches cover a defect of the check or the service itself:
- A developer can skip the check without contacting the service at all, with a property such as -Dxwiki.security.skip=<reason>.
- The service has a global kill switch that makes every check pass without a review, for an incident.
Neither needs a new contrib parent version, and the releases made through them are reviewed by the audit (see 6).
The check ships in new contrib parent versions; upgrading is asked, not forced.
6. Release audit
The check runs on the developer's machine, so it can be skipped (an old contrib parent, an override, a release made without Maven). Once a day, a script (no LLM) compares the contrib releases published on Nexus since the service went live with the clearances; the service's routine (see 8) then reviews only the releases that have none, from their nearest cleared ancestor, within the same budget as the check. Its cost is therefore proportional to the number of releases that skipped the check, not to the number of contrib extensions. A severe finding is filed as a confidential issue like a hold. The monthly count of releases that skipped the check shows whether a stronger push is needed.
7. Findings in unmaintained extensions
A finding from the audit or an override may concern an extension nobody maintains anymore. This relies on the process discussed in forum topic 18047, to be concluded and written into the Security Policy.
8. Where it runs
The relay enforces a review budget per extension and per day, and never reviews a commit twice: a commit already reviewed or under review returns its existing state. This bounds the cost of repeated or abusive requests.
A Claude Code routine (4 vCPU, 16 GB, Docker available), with read access to the private xwiki-dev-security-llm repository and network access to nexus.xwiki.org, maven.xwiki.org and extensions.xwiki.org, to start disposable instances for proving.
Relation to other proposals
- Forum topic 18832: the public multi-angle review of commits; its "defensive conventions" angle checks conventions and is not a security review.
- Forum topic 18047: what happens to a vulnerability in an unmaintained contrib extension.
- Forum topic 18668: who the security contact of a contrib extension is. The developer requesting a clearance is a known, active contact.
- Forum topic 18951: where security issues are tracked.
Open questions
- Soft-blocking from the start, or advisory first?
- Is the request relay authenticated with Nexus credentials the right design, or is there a simpler private way to request a review?
- How long does the security team have to confirm or lift a hold (2 working days?)
- Does proving work inside a routine, fast enough for a release (about 20 minutes)? Not tested yet.
- Who in the security team handles the holds?
- What does the first full review of a contrib extension cost? To be measured on a few extensions of different sizes before going live.
- Anything that should never be sent to an LLM provider, even though it is public?
Vincent Massol