Get Drupal Security Team approval to commit vendor/ckeditor.js to the drupal/ckeditor project

Adding 3rd party JavaScript dependencies for contributed projects is a long standing issue. The documentation page for 3rd party libraries includes two examples of how modules can add JavaScript libraries as dependencies; either CDN or Libraries API. Both of them have draw backs and for that reason many modules have tried to seek for alternative solutions.

The 3rd party dependency of this module (CKEditor 4) is particularly tricky because it requires use of a Java based tool for compiling the code (yes that's right, Java, not JavaScript 😅). Given that core used a build of CKEditor 4 that had to be built in a certain way that is not directly provided by the upstream library, we know as a fact that someone needs to continue running that tool as long as this module is supported. We need to decide whether that someone is the users of this module, maintainers of this module and the Drupal Security Team, or someone who's willing to maintain a "Drupal CKEditor 4" library that hosts the code somewhere outside of Drupal.org.

I believe making users solely responsible of running the CKEditor 4 build would be very bad DX. We have struggled onboarding new people to use the tool in the past given that the tooling is so unique that most engineers (that we have interacted with at least) have never used anything similar to this. Also, requiring users to have a certain version of Java installed on their computer to be able to use this module seems bad.

One option would be to create a new Github project where the maintainers of this module independently host a pre-built version of the CKEditor 4, and issue SA's in coordination with the upstream. We could integrate the library using Libraries API and include documentation on how to build the CKEditor 4 themself. This means that technically there wouldn't be direct dependency on that Github project and users of this module could choose which version o CKEditor 4 to run.

One more factor to this is that we know that core is required to keep its CKEditor 4 assets up to date in Drupal 9 until November 2023. For that reason, it would be convenient to have the same group of people provide the CKEditor 4 builds for this module. This module may qualify for an exception to the Drupal.org 3rd party asset policy because there are multiple supporting factors on: https://www.drupal.org/docs/develop/git/setting-up-git-for-drupal/drupal.... At least bullets 1, 2, 4, and 5 apply to CKEditor 4. So maybe if we can get sign-off from the security team, we could simply include the assets in this module like #3306630: Split the core 9.5.x ckeditor history into a new 1.0.x branch and cut a 1.0.0 release is already doing. If we want to, we could provide Libraries API integration on top of this that would allow loading custom version of CKEditor 4. This would (at least technically) remove some reliance from the users of this module on the Drupal Security Team and maintainers of this module.

And very bad for security!

How so? Wouldn't that simply prevent users from migrating to use this module and force them to continue using Drupal 9 until they are ready to migrate to CKEditor 5?

As @lauriii mentioned, the third party library policy states:

However, hosting the library in your Git repo on Drupal.org is acceptable if the 3rd party library:

1. had to be modified to work with Drupal, and the modifications were not accepted by the original author;
2. is generally difficult to find, or the specific version needed is hard to find;
3. is no longer maintained by the original author;
4. has its provenance, original license and source documented in a file included in the project;
5. is licensed under GPLv2+, or has a “GPLv2-compatible” license that allows us to distribute it under GPLv2+.

Here is how CKEditor 4 matches these criteria:

1. ✅ had to be modified to work with Drupal, and the modifications were not accepted by the original author:

   Yes. We use a custom build that is not available from CKSource directly.
    

2. ✅ is generally difficult to find, or the specific version needed is hard to find:

   Yes, extremely so. The custom build required has to be manually compiled with JAVA 😱 and furthermore can only be done on AN END OF LIFE VERSION OF JAVA due to compatibility issues with Java 16 and above. The process is esoteric and I still struggle with it even though I've done it numerous times for security releases.
    

3. ⚠️ is no longer maintained by the original author:

   Sort of. CKEditor 4 will be end-of-life in approximately a year. Until then, however, we as core maintainers and the Security Team also still have to do security updates for it in 9.5.x core, so we might as well use the same build to patch the CKEditor 4 contrib repo until then.
    

4. ✅ has its provenance, original license and source documented in a file included in the project:

   Yes.
    

5. ✅ is licensed under GPLv2+, or has a “GPLv2-compatible” license that allows us to distribute it under GPLv2+:

   Yes.

Commenting with my "security team" perspective: +1 from me for the proposal here. I agree with xjm's evaluation in #6. I'm not saying that on behalf of the entire team, though. How long can we wait for feedback on this? It would be great to wait ~1 week if possible for other team members to comment (or agree by way of their silence).

If the 1.0.x branch of the CKEditor 4 module for D9/10 is intended to provide drop-in compatibility with the D8/9 module, then I don't see any way around this. An alternative approach could be identified for a v2, but I think for v1 it's kinda stuck as-is.

I'm also +1 to this. The main reason to not commit vendor libraries to contrib is so that sites can update them independently of a contrib release, but that's not going to happen in this case so it's better to keep things as close to the 9.5.x core module as possible.

@greggles' comment asked for a week for feedback, and it's been five days already, so we might be OK to go ahead tomorrow or so?

Actually I checked when this was raised with the security team, and it was August 31st, so that's a solid week, since there's been no objections and a handful of +1s, I think that's plenty. The policy already allows for this, so it's really just confirming what documentation says is allowable already.

Thanks for noting the time and moving it forward, catch.