Getting security advisory coverage on drupal.org: what the reviewers actually ask for

What coverage is, and why a merchant should care

Drupal's Security Team only handles vulnerabilities in projects that have opted into the security advisory policy. Opting in is a permission granted to a drupal.org account, not to a project, and it is granted once, after one of that account's projects has passed a public review. After that, the maintainer can opt any of their projects in, and from then on vulnerability reports go to the Security Team privately, fixes are coordinated, and an advisory with a CVE is published when the fix is out.

For a site owner choosing a payment module, that is the difference between a module where a researcher who finds a hole has a responsible channel and a module where the hole gets posted in the public issue queue. For an agency recommending a module to a client, it is the difference between "this is maintained under Drupal's security process" and "trust me".

The review is run through a public issue in the project applications queue. Mine is issue 3607964, opened on 2 July 2026 for cybersource_sop, a Cybersource Secure Acceptance gateway for Drupal Commerce 3. It was marked fixed on 21 August, and both my Cybersource modules are now covered.

The process in numbers

Fifty days end to end, forty-five comments, five reviewers, about a dozen commits on my side, and a test suite that grew from 38 tests and 525 assertions to 48 tests and 702. The elapsed time is mostly waiting: the reviewers are volunteers and the queue is long. The working time was perhaps four days.

Round one: the modernisation checklist

The first thing any application gets is a check against current Drupal practice, and the bar is "what a new module should look like today", not "what works". The specific asks were:

• Hooks as class methods. Procedural hook_help() and friends in a .module file are out; the module should implement hooks as methods on a service class using the #[Hook] attribute, with the .module file reduced to a #[LegacyHook] stub for older cores.
• Attribute-based plugin discovery. The gateway plugin used the @CommercePaymentGateway annotation. Annotations are deprecated and removed in Drupal 13; Commerce 3 has had the attribute since 3.0.
• No deprecated requirements hooks. The runtime phase of hook_requirements() is deprecated on 11.3; the replacement is hook_runtime_requirements(), with a #[LegacyRequirementsHook] stub so 10.3 to 11.2 still report the same entries exactly once.
• GitLab CI green, with PHP_CodeSniffer clean. I had already pushed that further than the default, running PHPStan at level 6 where drupal.org's pipeline defaults to level 1, and that bar held throughout.

The complication, which came back in the final round, is that the module supports Drupal 10.3 as well as 11, and 10.3 does not auto-register #[Hook] classes. The resolution is to register the hook class explicitly in services.yml. Drupal 11.1 and later only auto-register a hook class when the container has no definition for it, so one explicit definition serves every supported core, and the class no longer needs a create() method at all. Worth knowing if you support more than one major.

Round two: someone attacks the return route

This is the round that made the application worth doing. An independent reviewer installed the module on a fresh Drupal 11.4 with Commerce 3, dropped in a credentials file with a known secret, and fired forged and tampered replies at the payment return route to see whether the project page's claims held.

They held. An unsigned reply, a forged signature, and a reply that leaves a critical field out of the signed set all get a 403 and record nothing. A correctly signed reply whose amount does not match the order is refused, and the recorded amount always comes from the order, never from the POST. A reply signed for one order and posted at another is refused on the reference check. A replayed acceptance leaves exactly one payment. The reviewer also went after the outbound side and the state machine: tampering the signed set, dropping signed fields, a double-submit race. The design that makes this hold is that the signing partition is itself signed and the order total is authoritative, and it is the reason I tell e-commerce clients to test the webhook before anything else.

Four findings came out of it, none blocking, and all fixed the next day:

1. Log after you verify, not before. The invalid-signature branch wrote a Commerce log entity to the order's activity feed before rejecting the request. Harmless to the money, but it meant an unauthenticated POST could add rows to an order's history. The rejection branch now logs to the module's own channel and writes no entity.
2. Sign the amount with the currency's own precision. The amount was formatted to two decimal places. Cybersource echoes the signed amount back and the module compares it to the order total, so a three-decimal currency (Kuwaiti dinar, Bahraini dinar) or a zero-decimal one (yen) would never have matched. The precision now comes from the Commerce currency entity.
3. The annotation and the requirements hook from round one, which the reviewer confirmed from the deprecation output of my own kernel tests.
4. Request-level tests for the open routes. The controller had unit coverage but no tests that drove the actual route and asserted that every denial path writes nothing. Five request-level kernel tests closed that.

The reviewer re-ran every probe at the new commit, confirmed the denials no longer wrote anything, and marked the application reviewed and tested. That is the kind of review you cannot buy, and it is the strongest argument for putting a payment module through this process even if you never needed the coverage.

Round three: where the signing secret lives

The same day, a member of the Security Team set the application back to needs work over one design decision: the module read its credentials from a fixed file at private://keys/cybersource.yml, deliberately not configurable through the admin UI.

My reasoning for a fixed location was sound as far as it went. Secrets must never enter Drupal configuration, because configuration is exported, committed, synced between environments and copied into staging databases. There should be no admin form that reads or writes the signing secret, because an administrator who can read it can forge "paid" replies, and one who can replace it can redirect funds. A single well-known path kept the requirements check, the admin messages and the documentation unambiguous.

The Security Team's threat model was wider than mine, and once laid out it was persuasive. Files under private:// are still downloadable: core serves them through a hook that other modules implement, and there have been core and contrib advisories where the public and private schemes bled into each other. On some hosts the private directory sits under the web root, or on an NFS share, or behind a remote stream wrapper such as S3, and a fixed, well-known path with "keys" and "cybersource" in it is exactly what a file-read vulnerability or a key stealer goes looking for first. Two questions were asked explicitly: what alternatives had I considered, and why had I discarded each one. That is the shape of a good security review, and it deserved a straight answer.

The design that shipped, and that both modules now use:

• Credentials are supplied from settings.php, which is the mechanism Drupal itself uses for secrets that must not live in configuration.
• Either as an absolute path to a credentials file, chosen by the developer, so it can sit outside the web root and outside both Drupal filesystems and there is no well-known location to attack.
• Or as an inline array populated however the host prefers: a platform secret store, an environment variable, a mounted secret. Hosts that cannot set environment variables are not excluded.
• Container parameters were rejected because they are compiled and serialised into the cached container. An upload form was rejected because it lets anyone with the administer permission replace the keys.

Along the way the module also stopped calling realpath() on a stream URI. Core's own documentation says to use that only when you know the wrapper is local disk, and a private filesystem on S3 would have returned FALSE and failed to load credentials at all. Credential access now goes entirely through the stream wrapper, and a unit fixture drives it through a deliberately non-local wrapper on every run.

Round four: the standards sweep

With the design settled, the application maintainer did the detailed pass. Nine points, and they are a useful picture of what "current Drupal" means to the people who review for it:

• A controller that uses nothing from ControllerBase should not extend it. Implement ContainerInjectionInterface and inject what you use.
• On Drupal 10.2 and later, a controller does not need create(): use AutowireTrait and a typed constructor.
• Hold the logger factory, not a logger channel, as a class property. A channel object as a property is considered an anti-pattern.
• Translatable message placeholders should be descriptive, @order_id rather than @o, because translators see them.
• A final class should have private properties, not protected.
• Type the current user as AccountProxyInterface, which is what the service actually implements.
• A hook class is an ordinary service: no ContainerInjectionInterface, no create(), and a short docblock.

Each is small. Together they are the difference between code that works and code a core contributor reads without friction, and they were applied in one commit, verified on Drupal 11.3 and 10.6, with PHPCS and PHPStan level 6 still clean.

The AI question

Early in the thread a reviewer noted that the module, the application and my review-bonus comments on other applications looked AI-assisted, and pointed me at Drupal's AI contribution policy. Later, after one of my replies, a Security Team member said the response read like a model correcting itself rather than a person explaining their reasoning, and the application maintainer put the principle plainly: the application exists to find out what the applicant understands, and that is not possible when the changes or the replies come from a tool.

I use Claude Code on every module I ship and I say so. The module's design is mine, from a custom Cybersource integration I built for a live store before the module existed, and I review everything the tool generates. The reviewers were not objecting to that. They were objecting to replies that sounded generated, because a generated reply tells them nothing about the maintainer they are about to trust with security reports.

What moved the thread was writing the answers myself: the alternatives I had weighed for credential storage and why each was rejected, in my own words, including the point where the Security Team's reasoning was better than mine. The practical advice for anyone applying is simple. Disclose the assistance. Keep the design decisions yours and be ready to defend them without the tool. Write every reply yourself. And do not post a polished summary of what you changed: the maintainer said explicitly that a needs-review comment only has to say what you did not do, or did differently.

What it costs, and what you get

Fifty elapsed days and about four working days, including a day reviewing other people's applications, which is the quickest way to learn what reviewers look for. The review bonus tag that is supposed to move your application up the queue is, in practice, unnecessary for a recent application, and the maintainer will remove it.

In return: a permission on the account that covers every project it maintains, a payment module that has been attacked by someone who knew what they were doing and held, a credential design that is stronger than the one I started with, and a test suite ten tests larger. The project page now says that stable releases are covered by the security advisory policy, and both modules shipped a 1.0.2 release on 5 September with every review change in it.

Before you apply: the checklist

1. Hooks as #[Hook] class methods, registered as a service if you support 10.3.
2. Attribute-based plugin discovery, no annotations.
3. No deprecations in your own kernel test output on the latest core.
4. GitLab CI green: PHPCS, PHPStan at a level you chose rather than the default, cspell.
5. No secret in configuration, no admin form that reads or writes one, no fixed well-known path. settings.php is the pattern.
6. Request-level tests for every open route, asserting that denial paths write nothing.
7. Only your commits on the branch while the application is open, and only one drupal.org account in the thread.
8. Replies in your own words.

If you maintain a module and want it to pass

I review Drupal contrib modules against this bar, with PHPCS, PHPStan, deprecation checks and a security pass of the kind described in round two, and write the result up as the drupal.org comment or the private findings list the maintainer needs. For agencies it is a fixed-price piece of work under Drupal for agencies, and the method is the one in finding security issues in Drupal modules. If the module takes payments, I would rather you had this review before a customer's money depends on it.