invoice attachment is not include anymore in mail sent

Hi,

Same problem with webform attachments wich is private...

Attached patch remove temporary hasAccess

Thanks for the report. The access checking is a new feature designed to improve security. The change record → was linked from the release note. By default private files are not allowed.

Please can you take a look at the CR and see if you could write code to call Attachment::setAccess()?

If not, then do you have any suggestions how we could make it work for your site and still keep some security?

We could remove the access check for legacy emails (sent using Drupal core mailer)?? This makes some sense for compatibility.

Hi,
I manage two sites which use the module commerce_invoice for the two site.

how I have tested in Test Environment, I don't update in PRODUCTION my website. So, in theses condition, I don't apply the update on my PRODUCTION sites. But it's a blocking issue to apply update automatically without patches.

I can wait if no high security violation is discovered.

How it's communicate this new contraint for dependencies module. Actualy, this contraint is not reported I think on project commerce_invoice, and I dont know if they are be concient of this new features.

I am not the best to answer if you can remove the access check for legacy emails...

Thanks

I managed to add something like this snippet to the custom code for adding attachments which are located in `private://`

    /** @var \Drupal\symfony_mailer\Email $email */
    $email = $this->emailFactory->newTypedEmail($email_params['type'], $email_params['sub_type'], $email_params);
    if ($email_params['attachments']) {
      /** @var File $attachment */
      foreach ($email_params['attachments'] as $attachment) {
        $at = Attachment::fromPath($attachment->getFileUri(), $attachment->getFilename());
        if($at->hasAccess() === false) {
          // We don't have access to the attachment, so force allowed access here.
          $at->setAccess(AccessResult::allowed());
        }
        $email->attach($at);
      }
    }
    $email->send();

Hope this can help anyone for fixing sending attachments which are located / configured in the private files directory.

I understand this change was introduced for security reasons, but could you clarify specifically what vulnerability or risk this hasAccess() check is addressing?

On my current site, even public files are no longer being sent as attachments, which breaks expected behavior. Before applying a workaround like setAccess(AccessResult::allowed()), I’d like to understand if:

the change is strictly necessary in all cases,

and whether the implementation is fully correct, given that public files are also affected.

For now, solution #7 works fine, but I’m trying to assess whether this change is required and properly implemented. Thanks.

I understand this change was introduced for security reasons, but could you clarify specifically what vulnerability or risk this hasAccess() check is addressing?

This change/problem was introduced in 📌 embedFromPath API addition (for #3284140) Needs work . The vulnerability is where non-trusted users can attach files - they could attach a file they shouldn't be able to access and email it to themselves (this isn't just theoretical as there has already been a security issue like this in another module). Particularly this applies to ✨ Email adjuster to attach/embed by scanning the email body Needs work , which currently is in 2.x but not yet in 1.x.

The solution is intended to work like this. Public files are allowed; http/https file are allowed if the URL can be opened with fopen; everything else is not allowed by default. Code that knows the attachment is from a trusted source can use the API to allow access to any file.

For legacy emails, AFAIK there has never been any access checking. The code that sets the attachment is responsible for ensuring that it is safe. Therefore we could reasonably do the same, as in #7. This was how it worked in 1.5, and I AFAIK it is safe.

On my current site, even public files are no longer being sent as attachments

and whether the implementation is fully correct, given that public files are also affected.

This is not correct, it would be a bug. Can anyone else confirm. There is an automatic test that should be covering this case, however the test could be wrong.

Fix in 2.x first then backport

v1.x version ready for testing please

We've reviewed the changes and will test the 1.x fix internally to confirm the regression is resolved.

One additional proposal: introduce a configuration option (e.g., checkbox or boolean field) that allows trusted code to opt into attaching files from private:// when needed. We totally agree that default should remain FALSE for security.

Use case: on a platform managing exams, a certificate is generated after completion and emailed to the user. The certificate contains personal data, so it's stored in private://. With the current restriction, sending it by email becomes impossible, despite being a fully controlled, trusted process.

Instead of requiring all developers to manually override access via setAccess(AccessResult::allowed()), exposing a controlled config flag would make the behavior clearer, safer, and easier to manage.

What do you think @adamps?

We've reviewed the changes and will test the 1.x fix internally to confirm the regression is resolved.

Thanks

Use case: on a platform managing exams, a certificate is generated after completion and emailed to the user.

Please can you explain how you are adding the attachment?

• If you are using the old/legacy mail interface, then it will work with this patch.
• If you are using the new interface, then I would expect you have code that calls email->attachFromPath() or similar. So could you add one extra line to call setAccess()?

Please can you explain how you are adding the attachment?

I'm using the function email->attachFromPath() so yeah, I can add the setAccess() just after.

For your patch, I just realized that it changes the legacy mailer that I don't use, so I will not be able to review it.