Disclaimer: this proposal was drafted with Claude Code, on my prompting.
A recent contrib release shipped security issues introduced in new code. I’d like us to discuss what we should do so that no contrib extension version is released without a security review of its code. Full design: https://design.xwiki.org/xwiki/bin/view/Proposal/ContribSecurityReview
A service, like SonarCloud: the developer needs no AI account, runs nothing new and never sees AI output.
Nothing found is ever public. Findings become confidential issues following the Security Policy. The only public output is “commit X was reviewed, no known severe issue”.
so I think the whole idea is good as best practice but I’m really not sure about the automation enforcing here as I’m see various kind of possible limitations.
Those are open questions that I did not really dig:
soft blocking might be really annoying at the early stage of the process: if I have 10 vulnerabilities found in my app and I’m trying to release to fix a single one, if I get a blockage for the 9 remaining ones and I got the same each time I try to release it will quickly push me to wait for fixing them all instead of doing quick releases
what about the security of the process itself: isn’t the automated process in contrib an open door to possible prompt attack attempts and or exploitation of token? it’s easy to imagine creating a contrib project and having a Jenkinsfile running constantly mvn release:prepare with some crafted stuff. I don’t really think the prompt injection would be a problem (but to be double checked) but I really think it can consume quickly the tokens if someone plays with that
I really don’t like the no opt-out: if there’s a bug somewhere that makes the soft block a real block, or any other problem it means you’ll be stuck with that contrib parent and force to wait a committer to fix it. I’m sure there will be plenty of situations where we’d want to switch that off.
You’re right: as written, a commit was held as long as a proven severe finding was still open, including one from an earlier review. So each release would be held by your 9 remaining issues, which pushes people to wait instead of releasing fixes early. That’s changed (§2): a finding now holds a release only if that release would ship it for the first time. A finding already in a released version is tracked as a confidential issue and never holds later releases. Users are already exposed to it, so holding the next release would only delay its fixes.
2. Security of the process itself
Token consumption: the review isn’t triggered from contrib CI. It’s requested through a relay on our infra, authenticated with the developer’s nexus.xwiki.org credentials (the same ones needed to release). So a random Jenkinsfile can’t trigger it, and each request is tied to a contrib member. I’ve added (§8) a review budget per extension and per day. A commit is also never reviewed twice: running release:prepare again only looks up the existing state.
Prompt injection: the reviewed code has to be pushed to GitHub, so any crafted content is public and traceable. The review runs on our side, and its only public output is the clearance record. The real asset to protect is the private knowledge base the reviewer reads. We should limit what the routine can write and where it can connect to, so an injection can’t leak it. Agreed that this needs a proper check.
3. Opt-out
The “no opt-out” in the proposal is only about the code being reviewed by an LLM. It’s not about the release check: an override (release anyway, reason sent privately) is always possible.
The service never blocks a release by being down. For a bug in the check or the service itself, I’ve added two escape hatches (§5): a skip that works without contacting the service, and a global kill switch on the service side. Neither needs a new contrib parent. Releases made that way are still reviewed afterwards by the release audit (§6: once a day, releases published on Nexus without a clearance get reviewed; the cost depends on the number of skipped releases, not on the number of contrib extensions).
Using the check is opt-in anyway: it comes with new contrib parent versions, and upgrading is asked, not forced.
Soft-blocking vs advisory
Soft-blocking
Advisory only
A new proven severe finding
Release stops until it’s fixed, lifted, or overridden with a reason
Release goes ahead; the finding is handled afterwards
Waiting for the review
Only if not requested ahead of time (see below)
No waiting
Stops a new vulnerability from shipping
Yes, unless overridden
No: users get it, and an advisory and fix follow
Cost of a bug or false positive to the developer
Has to override
None
I’d keep soft-blocking. Advisory-only can’t do what this is for, which is to stop a release like the recent one from shipping a new vulnerability. The release audit already reviews releases after the fact, so advisory mode would add little. With the changes above, a hold now only happens for a new proven severe finding, and the override is always there when needed.
To avoid waiting during release:prepare, I’ve also added (§2) a way to request the review ahead of release day, with a Maven goal that returns at once. You get the result privately and have time to fix anything before the release. On release day, the check then only reviews the commits pushed since, which is usually fast, or nothing at all.