HI @tgoeg,
i had a look and the problem is that the Google Translate API's free endpoint (translate_a/single) is designed for simple text translation and doesn't handle HTML markup well. It's breaking the HTML structure and not translating all content properly.
I added an additional method to handle only partial text translation when using the free google APIs and tested the issue now is fixed.
Best regards
Hi @tgoeg,
i'm still not able to reproduce the issue below paragraph reference field on my node and if i open the translate form the fields are correctly translated with also the html markup.
Title: Paragraph
Plural title: Paragraphs
Edit mode: Closed
Closed mode: Preview
Autocollapse: All
Closed mode threshold: 3
Add mode: Dropdown button
Form display mode: default
Default paragraph type: Test Paragraphs
Features: Duplicate
Please test with latest paragraph module or indicate what version are you using so i can try to reproduce it.
Best regards
i come out just yestarday on this post on linkedin: https://www.linkedin.com/pulse/new-drupal-vulnerability-just-two-meters-...
most business and private site owners would likely discontinue their use of open source applications altogether for fear of vulnerabilities being exploited immediately after they are reported.
This seems reasonable to me, i've also experienced companies that after running SAST on an application they choosed to not use it. So my pov is that the security issues are not only disclosed in public or privately inside d.org but also outside. in d.org there isn't minimum test coverage for stable module by policy?
Hi @rhovland,
i'm not pointing to anyone, and i not discuss what happen it is done, i'm raising some concern from my point of view about the process of security related to stable modules (who define stable?). From my point of view security issue in open source might have to be not private, but again this is only my point of view.
Of course was evident that at least to me the policy was not so clear and now that i'm understanding better it i agree to have been removed from security opt-in i'm not arguing that
Please feel free to close the issue if the concerns are only mine.
Thanks
Well done! i'll review/merge this weekend and pack new release togheter with the other
Hi @tgoeg,
thanks to raise the issue i'll fix in the weekend maybe i miss to clear the API input when user change the checkbox, it will be quick fix.
Best regards
Hi @tgoeg,
glad that you had chance to figure out the right configurations!
And also thanks for precious inputs as always :) i will look into that in the weekend, hopefully will be easy to implement logic to check and force widgets to be open :)
Thanks again
Thanks to raise the fix and provide the fix, the solution looks good.
bigbabert → made their first commit to this issue’s fork.
i will look into that, thanks to raise it.
Updated the phrase but please feel free to correct with more appropriated terminology, my meaning is that the people with stable modules lacking security are active part of reviewing modules that apply for security coverage.
Summary updated
it is in the summary,
The issue concerns two processes:
- We are seeing security issues in modules maintained by people who have opted into Security Coverage, but these issues are not being reviewed or flagged by the Security Team.
- Project application reviews do not verify whether the submitted modules have vulnerabilities or compatibility issues, and no end-to-end testing is performed.
Also tried to clarify more here:
The issue isn't against someone but about the process to keep modules with stable releases secure.
What is not enough clear? there are specific questions by moderators?
I'm not categorizing any response, if we want "as community" we can start talk about improve the process in this issue or wethever is much appropriate from people with exaustive knowledge of process and policies, i don't have exaustive knowledge of d.org policies and process i've raised concern based on my experience and on the fact that is happening frequently that i look to stable modules code and i found potential security issue.
If no one is interested in the discussion or if the place where the issue is reported isn't appropriate please close it. Thank you
If there is no openness to improving the existing process, the issue can be closed.
The issue is about security coverage of contribs modules:
The issue concerns two processes:
- We are seeing security issues in modules maintained by people who have opted into Security Coverage, but these issues are not being reviewed or flagged by the Security Team.
- Project application reviews do not verify whether the submitted modules have vulnerabilities or compatibility issues, and no end-to-end testing is performed.
The issue isn't against someone but about the process to keep modules with stable releases secure.
Oh i was not aware , always used to concatenate with {} variable inside string, good to know.
Stille there are other pending feedback i'll summarize here:
You would use \Drupal::service('extension.list.module')->getList(); that return also disabled modules. Screenshot here, eg. i don't have ai module enabled
I add also that there isn't input sanitization here for example:
$config = $this->configFactory->get('ai_readme_generator.settings')->get();
$apiKey = $config['api_key'] ?? NULL;
Above code also don't reflect best practice for drupal you should not use $config as array you can get the config value with $config->get('api_key')
or here:
$module_name = $form_state->getValue('module_name');
composer minumum stability concern for stable module
Thanks
The conflict of interest is not part of the concern, i'll remove from the summary, in the comment my intention is to mentor properly people that has opt-in security coverage to not publish module that has not deeply tested and that avoid injections, mine are just examples, but you or everyone can found multiple in contrib modules code
Is the question related with the concerns reported in the issue?
The issue concerns two processes:
- We are seeing security issues in modules maintained by people who have opted into Security Coverage, but these issues are not being reviewed or flagged by the Security Team.
- Project application reviews do not verify whether the submitted modules have vulnerabilities or compatibility issues, and no end-to-end testing is performed.
One of the people in question is your mentee. i see conflict of interests in both processes. Hoping in a transparent management of that issues.
issues that i see:
- External composer dependency should be declared on module composer.json, class should be injected as dependency not in this way
$this->exiftoolPath = $kernel->getAppRoot() . '/../vendor/ahmetburkan/exiftool-binary/exiftool';
- General speaking lack of dependency injection, an example: https://git.drupalcode.org/project/file_metadata_cleaner/-/blob/1.0.x/sr...
The issue concerns two processes:
We are seeing security issues in modules maintained by people who have opted into Security Coverage, but these issues are not being reviewed or flagged by the Security Team.
Project application reviews do not verify whether the submitted modules have vulnerabilities or compatibility issues, and no end-to-end testing is performed.
Could you please let me know which part is unclear or causing confusion?
HI, mine is just an example, i'm pretty sure that Security Team can have a look to the projects that are opted-in and the que to request security coverage and there is lot of evidence.
I'm available in case any kind of support needed!
bigbabert → created an issue.
It will be good to make users know about the vulnerability, affected versions and needed remediation.
Hi why there is link to security private issue? should not remain private? https://security.drupal.org/node/162249
Hi @vishal.kadam,
injections can be done also via query string parameters!
eg. if user open the url and change manually the parameter injecting malicious code this will affect websites.
Best regards
i was thinking it uses update.php; So if we have to use drush to update DB no need of the message to public users on update.php, just joking eheh :)
In case clients raise concerns about open source cms security usually we put VPN in front of certain paths plus remove the not needed paths like install.php, install.readme etc. We still have update.php present but under VPN so in case of needs admin can run update from the browser and if someone try to access to the update.php without login WAF rules point to the generic server 403 page, same for user/login.
Documentation cover only install.php, update.php should not be removed.
You can have look here 📌 [1.0.x] AI Readme Generator Active Security coverage request, reviewer sentence:
This is not an issue created in a project queue.
Comments in this queue are required to review the project files and report what needs to be changed. We do not debug projects.
Documentation for reviewers 📌 [1.0.x] AI Readme Generator Active :
Correct usage of Drupal APIs →
We check all the PHP code line-by-line, looking for proper use of Drupal APIs, to verify that functions/methods receive the correct parameters, but also that the correct APIs are used.
Verifying the code does not contain security issues also verifies the Drupal APIs are correctly used. This is an extension of the previous point; it also includes those cases where misusing a Drupal API does not cause any security issue.
emphasis by me.
Just reviewing the code that you pasted above:
$this->output()->writeln("✅ README.md generated at: $readme_path");
The $readme_path variable in PHP in this way will be printed as string with value: $readme_path, not sure if it is expected behavior, i see same issue repeated
Also:
$this->output()->writeln('<error>Please fill the AI configuration form first!</error>');
It is not best practice concatenate html tag with strings.
Hi @kul.pratap, can you check in you composer the minimum stability? have you installed drupal/ai module? I've tried to install on DrupalCMS. but this seems not be a relevant issue for that review.
Are you sure that $this->moduleHandler->moduleExists('block_content') looks only for enabled modules=? i'm quite sure that it return true if the module is in the core/contrib/custom module folder. Then you can get also the path of disabled module. The first method returns the module list service (\Drupal::service('extension.list.module')). The second method the module path for the given module name (\Drupal::service('extension.list.module')->getPath($module_name)).
I add also taht there isn't input sanification here for example:
$config = $this->configFactory->get('ai_readme_generator.settings')->get();
$apiKey = $config['api_key'] ?? NULL;
or here:
$module_name = $form_state->getValue('module_name');
This are just examples user inputs in drupal should be always validated to avoid injections:
https://www.drupal.org/docs/administering-a-drupal-site/security-in-drup... →
Hi @bramdriesen I 100% agree with this sentence: AI works great, if used and monitored properly.
about
Yes what it spits out might work when you tested it, but is it safe? Performant?
If test cover performance and security yes, also more skilled drupal dev make mistake but as you said with knowledge it can be fixed and save a lot of time with proper help of AI.
@cmlara,
do you think what build in a weekly assignment by a class is prod ready without bugs?
non AI tooling sounds like non auto-correct tooling or non-TV-films years ago, humans are still here doing staff but better and with augmented velocity with help of technology progress.
There are a lot of reports on time/quality provided from well structured LLM process that make clear the transformation, lot of global enterprise companies are using them on their own products.
bigbabert → created an issue.
This is why we have more resistence to accept it's usage
i don't understand how you can say that there isn't compatibility issue if the module ask for a dependency older than the current one supported in drupal 11 for openai-php/client and also i don't understand how can be judged best practice scan the file system to get drupal module path instead of use drupal API as per comment #12
I've passed years cleaning human code, i think make mistake and bug is normal in development, AI is born recently human do that from long time, AI is just a tool like autocomplete that nowdays is used by everyone but at the beginning also had lot of resistence.
Hi @vishal.kadam,
i assume that test should cover also that the submitted module is installable and compatible with version declared as supported by the module? is not that part of the review?
eg. Works with Drupal: ^10 || ^11
So if module has composer dependencies this also should be managed per compatibility declaration is that right? Also i can not suggest a solution since the locked dependency declaration is on a packagist module out of drupal.org
Please correct me if i'm wrong.
I've also tried to install the module with composer in fresh Drupal 11 instance and get following error:
Your requirements could not be resolved to an installable set of packages.
Problem 1
- Root composer.json requires drupal/ai_readme_generator ^1.0 -> satisfiable by drupal/ai_readme_generator[1.0.0].
- drupal/ai_readme_generator 1.0.0 requires innoraft/ai-readme-generator ^1.1.0 -> satisfiable by innoraft/ai-readme-generator[v1.1.0].
- innoraft/ai-readme-generator v1.1.0 requires openai-php/client ^0.10.3 -> found openai-php/client[v0.10.3] but the package is fixed to v0.13.0 (lock file version) by a partial update and that version does not match. Make sure you list it as an argument for the update command.
Use the option --with-all-dependencies (-W) to allow upgrades, downgrades and removals for packages currently locked to specific versions.
Installation failed, reverting ./composer.json and ./composer.lock to their original content.
to me is relevant to highlight the process and (lack of) raccomandations given, if you prefer please remove the comment. So we agree that once a mantainer individual or organization get opt-in on a project next project (if not published on slack) are not reviewed? i'm asking because i see projects with stable release eg. https://www.drupal.org/project/video_embed_jwplayer → (just fresh example but there is a lot) that not work and probably have security issues too.
Have a look to this API documentation might be helpful is not so complex and you don't have to read from composer installer: https://www.drupal.org/node/2940438 →
Hi @avpaderno,
my permission at time when i asked was given by you: https://www.drupal.org/project/projectapplications/issues/3035714 →
BR
but this is future tool that companies are quickly adopting and it improve teams velocity and also quality in long term with accurate review process and appropried prompts it is still human driven the AI will not do nothing that is not already done by humans. I understand that there is some resistence to the changes more when they are radical like this but we will see shortly how situation will evolve, Microsoft already start layoffs of engeeners and developers.
Just to add my POV, AI is tool if you say people should not use AI to do staff etc. is like 20 years ago you say people can not write with mobile phone on our community. come on :) I agree that contents for now needs deep review by human but how much time will need to be more accurate from the AI?
Also i not see any fairness to an issue about someone (me) that is neither tagged in the issue!
That’s fine for me, we’ll continue to protect sensitive paths like install.php and update.php via VPN, not just relying on Drupal’s default access control. The same goes for log access; in our case, that's managed through WAF rules.
I also regularly observe a large number of automated requests targeting Drupal login pages and RSS feeds. This confirms that automated scripts exist and many of them attempt requests across all known CMS platforms, not just Drupal.
As I’ve already explained, I believe the information currently exposed to unauthenticated users (e.g. explicit mentions of the "update script") is still relevant from a security standpoint and could be avoided. I remain concerned about optimizing the UX for an admin user who is not even logged in, at the expense of security best practices.
That said, I understand this is not something the Drupal core team considers a priority to address, so I’ll go ahead and close the issue.
Thanks for the discussion.
Hi @longwave,
ok now i understand, may i kindly ask if is needed the permission of reporter or maintener before to publish the SA?
and also it is normal the attitude of certain comments in the security issues? i think sharing this will benefit to have a more transparent and fair process, that is what a n open source community should promote in my mind. I really hope that this situation can start to change some attitude and make people write inside the community not like they are talking with sub-human but pair level person.
BR
bulk word in the title is not accurate since i only posted 2 modules 1 that neither was in stable version, so please correct to be more appropriate.
I'm not discussing the specific issue but it is related and maybe someone reading here is interested in the process that come with my permission opt-out, personally i've to say again that i don't see fairness or transparency in that process neither in the SA publication, at least for my experience.
hi @longwave,
my understanding was that permission to share the content of the security issues was given in comment 16 🐛 Clarify policy for revoking security advisory coverage Active .
BR
Hi everyone,
For the benefit of who is interested, as agreed per comment #16 🐛 Clarify policy for revoking security advisory coverage Active attached screenshots of security issues and SA already agreed before the generic one without fix included was published without any triage
Best regards
as already explained my point of view (not only mine) is that even if disclaim that the update script is located to /update.php is a relevant information for attackers. It is more reasonable to give that information only to authenticated users, for not authenticated users the message should be more generic and redirect to the login maybe with destination in the session so after logged in the user will be redirected to update page (to me this seems also a correct UX).
Thank you for your feedback. I appreciate the usability concerns, and I understand the need to support legitimate administrators who might land on the page without being logged in.
However, my intention with this issue is simply to point out that Drupal's current behavior does not fully align with OWASP guidelines, specifically regarding information disclosure in unauthenticated contexts
As I’ve already mentioned, attackers are not only humans. Automated tools can easily extract relevant information from messages like:
"You do not have permission to access the update script."
This reveals both the existence of a sensitive script and its function, which should ideally be avoided.
These small pieces of information, when aggregated with others (e.g. exposed file paths like /core/install.php
, identifiable assets, or HTTP headers), contribute to attack surface mapping, particularly when automated scanners are involved. This is exactly the kind of incremental information OWASP advises to avoid disclosing to unauthenticated users.
A more generic message would better align with OWASP's recommendations to minimize information disclosure in unauthenticated contexts.
That said, if the project’s position is that the current message is the right trade-off between security and usability, I respect that decision. I just wanted to raise the concern and ensure the discussion was on record.
BR
Ok that is why we protect some internal path with VPN for production websites.
The fact is that this 403 page is providing an explicit message to the attackers (mostly scripts not human) that is in contrast with OWASP guidelines.
Usability can still be preserved after authentication. Once logged in as an admin, the user can receive a detailed and helpful message, that’s the right place for technical explanations.
Many CMS platforms, including Adobe Experience Manager, WordPress and Joomla, also follow this model: deny access to system paths unless authenticated, and provide details only to trusted users.
This isn’t about hiding Drupal or update.php — it’s about not helping attackers identify or confirm behaviors and configurations, even unintentionally. Security headers, generic messages, and authenticated-only technical guidance are well-known best practices in hardening production systems.
We can preserve usability without exposing technical details. For instance:
The behaviour like in AEM could be that the user is redirected to login page with generic message like:
Access Denied
You are not authorized to access this resource.
I think that the Admin that have to update a CMS database should know that it is not accessible with specific permissions.
Best regards
Notice same issue here: /it/node/2/translations/add/en/it
Azure WAF respond with 403:
403 Forbidden
Morehover, i know that i should delete or protect with a VPN that pages but this should not be very well documented and disclaimed on d.org? also yestarday i was checking use cases on d.org and no one has removed this files, maybe they just don't know that!
Hi @longwave,
The message that disclose information is that eg. on /update.php
Access denied: You do not have permission to access the update script.
In this way an attacker know that the update script is here, more hover i see was removed the message with variable of settings php that is good.
The old one show more information:
In order to run update.php you need to either have "Administer software updates" permission or have set $settings['update_free_access'] in your settings.php.
The issue isn't about to hide that a site leverage on Drupal CMS (that would also be a bad accessibility practice), but to not provide specific information with 4XX pages to potential attackers as per OWASP Error handling guidilnes: https://cheatsheetseries.owasp.org/cheatsheets/Error_Handling_Cheat_Shee...
There is also more or less same issue for. lot of ajax request in the BE that if spoofed provide lot of useful information to an attacker, eg. if we request with curl an admin page we have this in the HTML response "currentPathIsAdmin":true so that is a relevant information for attackers that are trying to understand modules installed on Drupal instance.
Please let me know if still something unclear or that need more detailed examples and description
BR
Alberto
modules/custom and modules/contrib should not be hardcoded since they can be configured in composer
The issue here is not determine that the site is on Drupal but specific error message handling as per https://cheatsheetseries.owasp.org/cheatsheets/Error_Handling_Cheat_Shee...
bigbabert → created an issue.
D11 released and d12 is on the roadmap? this should be taken in charge?