🇮🇹Italy @bigbabert

Hi @tgoeg,

thanks for reporting and for the fix to review, i'll have look next days (hope on Sunday), also to the long term solution that is pretty much interesting, i could add deepl php library dependency as already done for google provider, also maybe at that time the module is mature to build proper hooks that allow other modules to add more providers.

Best regards
Alberto

Thanks the error not appear with dev version, hopefully u will be able to make happen SA release soon.

Hi @martin,

before to release the patch we should test if it work properly with autosave_form → module.

Best regards

bigbabert → created an issue.

Hi @martin mayer,

i'm aware of that issue, if you have time to make separate function that manage permissions for the translate action it could be in the formTranslate function inside the utility service. This will be great, if you mind we could also have chat on slack.

Best regards

Hi @martin mayer,

in 1.4.28 this should be fixed i've also added an option in the module settings to enable deeper debugging in the logs.

Best regards
Alberto

Hi @martin mayer,

Thanks for the patch. The module was created to work with default node translation form, we can improve doing the changes to properly work with custom entity/form

What is not clear to me is the intention to use
if(!str_ends_with($form_id, '_edit_form')) {
in the hook.

The check is to prevent that the translation is triggered when editing nodes or add new node.

Best regards

Hi @martin mayer,

still not worked on the fix, as soon i'll release i'll upadate you,

Thanks
Alberto

Just added External word in the title should be enough, also the module page description has detailed description of how module work and its pourposes.

Best regards

Hi @kerasai,

The module provide a way to add JW Player Videos as media external resources to Drupal that is not something out of box, the module add a new media page with the list of video taken from JW Player platform and allow users to use it in different form in the CMS eg. with sitestudio components or block or CKEditor.

Also in the project page there is the main differences with similar modules. So main difference is that this module is fully integrated with latest version of JW player, sitestudio, blocks, have dedicated jw player videos page on Drupal content. At time i built the project no other module was working with JW player existing APIs.

Best regards
Alberto

Hi @martin mayer,

thanks to report, can you confirm that you have applied your patch from there https://www.drupal.org/project/auto_translation/issues/3528526 🐛 Paragraphs are not saved in batch mode Active ?

To fix this i will use same solution as per https://www.drupal.org/project/auto_translation/issues/3526590 🐛 Text parts with markup lost with Google API Active that extract the text from the html and then put it together in the field.

Best regards

Hi @martin mayer,

Thanks to report the issue, i see it, if i'm not wrong it was present and i've removed it for some reason, will have double check if any regression and i'll apply the patch as soon as possible.

Best regards

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.