fix: file/directory permissions within the zip while packaging artifacts #4655

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

sriram-mv merged 8 commits into aws:develop from sriram-mv:reintroduce_package_permissions

Feb 15, 2023

samcli/lib/package/packageable_resources.py

-Original file line number
+Diff line change
@@ Expand Up @@
             should_sign_package = self.code_signer.should_sign_package(resource_id)
             artifact_extension = "zip" if should_sign_package else None
             uploaded_url = upload_local_artifacts(
+                self.RESOURCE_TYPE,
                 resource_id,
                 resource_dict,
                 self.PROPERTY_NAME,
@@ Expand Down Expand Up @@
             """
             artifact_s3_url = upload_local_artifacts(
-                resource_id, resource_dict, self.PROPERTY_NAME, parent_dir, self.uploader
+                self.RESOURCE_TYPE, resource_id, resource_dict, self.PROPERTY_NAME, parent_dir, self.uploader
             )
             parsed_url = S3Uploader.parse_s3_url(
@@ Expand Down @@

samcli/lib/package/permissions.py

-Original file line number
+Diff line change
@@ -0,0 +1,160 @@
+    """
+    Classes which will change permissions on a ZipInfo object
+    """
+    import platform
+    import zipfile
+    class PermissionMapper:
+        """
+        Base Mapper with an empty apply method.
+        """
+        def apply(self, zip_info: zipfile.ZipInfo):
+            pass
+    class WindowsFilePermissionPermissionMapper(PermissionMapper):
+        """
+        Windows specific Permission Mapper onto a zipfile.ZipInfo object to
+        set explicit permissions onto `external_attr` attribute if the given object
+        is of type `file`
+        """
+        def __init__(self, permissions: int):
+            """
+            Parameters
+            ----------
+            permissions: int
+                Base permissions.
+            """
+            self.permissions = permissions
+        def apply(self, zip_info: zipfile.ZipInfo):
+            """
+            Parameters
+            ----------
+            zip_info: zipfile.ZipInfo
+                object of zipfile.ZipInfo
+            Returns
+            -------
+            zipfile.ZipInfo
+                modified object with `external_attr` set to member permissions.
+            """
+            if platform.system().lower() == "windows" and not zip_info.is_dir():
+                zip_info.external_attr = self.permissions << 16
+            return zip_info
+    class WindowsDirPermissionPermissionMapper(PermissionMapper):
+        """
+        Windows specific Permission Mapper onto a zipfile.ZipInfo object to
+        set explicit permissions onto `external_attr` attribute if the given object
+        is of type `directory`
+        """
+        def __init__(self, permissions: int):
+            """
+            Parameters
+            ----------
+            permissions: int
+                Base permissions.
+            """
+            self.permissions = permissions
+        def apply(self, zip_info: zipfile.ZipInfo):
+            """
+            Parameters
+            ----------
+            zip_info: zipfile.ZipInfo
+                object of zipfile.ZipInfo
+            Returns
+            -------
+            zipfile.ZipInfo
+                modified object with `external_attr` set to member permissions.
+            """
+            if platform.system().lower() == "windows" and zip_info.is_dir():
+                zip_info.external_attr = self.permissions << 16
+            return zip_info
+    class AdditiveDirPermissionPermissionMapper(PermissionMapper):
+        """
+        Additive Permission Mapper onto a zipfile.ZipInfo object to
+        set additive permissions onto `external_attr` attribute if the given object
+        is of type `directory`
+        """
+        def __init__(self, permissions: int):
+            """
+            Parameters
+            ----------
+            permissions: int
+                Base permissions.
+            """
+            self.permissions = permissions
+        def apply(self, zip_info: zipfile.ZipInfo):
+            """
+            Parameters
+            ----------
+            zip_info: zipfile.ZipInfo
+                object of zipfile.ZipInfo
+            Returns
+            -------
+            zipfile.ZipInfo
+                modified object with `external_attr` added with member permissions
+                if conditions are satisfied.
+            """
+            if zip_info.is_dir():
+                zip_info.external_attr |= self.permissions << 16
+            return zip_info
+    class AdditiveFilePermissionPermissionMapper(PermissionMapper):
+        """
+        Additive Permission Mapper onto a zipfile.ZipInfo object to
+        set additive permissions onto `external_attr` attribute if the given object
+        is of type `file`
+        """
+        def __init__(self, permissions: int):
+            """
+            Parameters
+            ----------
+            permissions: int
+                Base permissions.
+            """
+            self.permissions = permissions
+        def apply(self, zip_info: zipfile.ZipInfo):
+            """
+            Parameters
+            ----------
+            zip_info: zipfile.ZipInfo
+                object of zipfile.ZipInfo
+            Returns
+            -------
+            zipfile.ZipInfo
+                modified object with `external_attr` added with member permissions
+                if conditions are satisfied.
+            """
+            if not zip_info.is_dir():
+                zip_info.external_attr |= self.permissions << 16
+            return zip_info

samcli/lib/package/utils.py

            
                      Original file line number
                      Diff line number
                      Diff line change
                  
    @@ -1,23 +1,32 @@
  
    """

    Utilities involved in Packaging.

    """

    import functools

    import logging

    import os

    import platform

    import re

    import shutil

    import tempfile

    import time

    import zipfile

    import contextlib

    from contextlib import contextmanager

    from typing import Dict, Optional, cast

    from typing import Dict, Optional, Callable, cast, List

    import jmespath

    from samcli.commands.package.exceptions import ImageNotFoundError, InvalidLocalPathError

    from samcli.lib.package.ecr_utils import is_ecr_url

    from samcli.lib.package.permissions import (

        WindowsFilePermissionPermissionMapper,

        WindowsDirPermissionPermissionMapper,

        AdditiveFilePermissionPermissionMapper,

        AdditiveDirPermissionPermissionMapper,

        PermissionMapper,

    )

    from samcli.lib.package.s3_uploader import S3Uploader

    from samcli.lib.utils.hash import dir_checksum

    from samcli.lib.utils.resources import LAMBDA_LOCAL_RESOURCES

    LOG = logging.getLogger(__name__)

    @@ -119,6 +128,7 @@ def upload_local_image_artifacts(resource_id, resource_dict, property_name, pare
  
    def upload_local_artifacts(

        resource_type: str,

        resource_id: str,

        resource_dict: Dict,

        property_name: str,

    @@ -138,6 +148,7 @@ def upload_local_artifacts(
  
        If path is already a path to S3 object, this method does nothing.

        :param resource_type:   Type of the CloudFormation resource

        :param resource_id:     Id of the CloudFormation resource

        :param resource_dict:   Dictionary containing resource definition

        :param property_name:   Property name of CloudFormation resource where this

    @@ -166,9 +177,14 @@ def upload_local_artifacts(
  
        local_path = make_abs_path(parent_dir, local_path)

        # Or, pointing to a folder. Zip the folder and upload

        # Or, pointing to a folder. Zip the folder and upload (zip_method is changed based on resource type)

        if is_local_folder(local_path):

            return zip_and_upload(local_path, uploader, extension)

            return zip_and_upload(

                local_path,

                uploader,

                extension,

                zip_method=make_zip_with_lambda_permissions if resource_type in LAMBDA_LOCAL_RESOURCES else make_zip,

            )

        # Path could be pointing to a file. Upload the file

        if is_local_file(local_path):

    @@ -184,13 +200,13 @@ def resource_not_packageable(resource_dict):
  
        return False

    def zip_and_upload(local_path: str, uploader: S3Uploader, extension: Optional[str]) -> str:

        with zip_folder(local_path) as (zip_file, md5_hash):

    def zip_and_upload(local_path: str, uploader: S3Uploader, extension: Optional[str], zip_method: Callable) -> str:

        with zip_folder(local_path, zip_method=zip_method) as (zip_file, md5_hash):

            return uploader.upload_with_dedup(zip_file, precomputed_md5=md5_hash, extension=extension)

    @contextmanager

    def zip_folder(folder_path):

    def zip_folder(folder_path, zip_method):

        """

        Zip the entire folder and return a file to the zip. Use this inside

        a "with" statement to cleanup the zipfile after it is used.

    @@ -199,6 +215,8 @@ def zip_folder(folder_path):
  
        ----------

        folder_path : str

            The path of the folder to zip

        zip_method : Callable

            Callable function that takes in a file name and source_path and zips accordingly.

        Yields

        ------

    @@ -210,15 +228,15 @@ def zip_folder(folder_path):
  
        md5hash = dir_checksum(folder_path, followlinks=True)

        filename = os.path.join(tempfile.gettempdir(), "data-" + md5hash)

        zipfile_name = make_zip(filename, folder_path)

        zipfile_name = zip_method(filename, folder_path)

        try:

            yield zipfile_name, md5hash

        finally:

            if os.path.exists(zipfile_name):

                os.remove(zipfile_name)

    def make_zip(file_name, source_root):

    def make_zip_with_permissions(file_name, source_root, permission_mappers: List[PermissionMapper]):

        """

        Create a zip file from the source directory

    @@ -228,11 +246,15 @@ def make_zip(file_name, source_root):
  
            The basename of the zip file, without .zip

        source_root : str

            The path to the source directory

        permission_mappers : list

            permission objects that need to match an interface such that they have an apply method

            which takes in the external attributes of a zipfile.Zipinfo object

        Returns

        -------

        str

            The name of the zip file, including .zip extension

        """

        permission_mappers = permission_mappers or []

        zipfile_name = "{0}.zip".format(file_name)

        source_root = os.path.abspath(source_root)

        compression_type = zipfile.ZIP_DEFLATED

    @@ -242,28 +264,54 @@ def make_zip(file_name, source_root):
  
                    for filename in files:

                        full_path = os.path.join(root, filename)

                        relative_path = os.path.relpath(full_path, source_root)

                        if platform.system().lower() == "windows":

                            with open(full_path, "rb") as data:

                                file_bytes = data.read()

                                info = zipfile.ZipInfo(relative_path)

                                # Clear external attr set for Windows

                                info.external_attr = 0

                                # Set external attr with Unix 0755 permission

                                # Originally set to 0005 in the discussion below

                                # https://github.com/aws/aws-sam-cli/pull/2193#discussion_r513110608

                                # Changed to 0755 due to a regression in https://github.com/aws/aws-sam-cli/issues/2344

                                # Mimicking Unix permission bits and recommanded permission bits

                                # in the Lambda Trouble Shooting Docs

                                info.external_attr = 0o100755 << 16

                        with open(full_path, "rb") as data:

                            file_bytes = data.read()

                            info = zipfile.ZipInfo(relative_path)

                            # Context: Nov 2020

                            # Set external attr with Unix 0755 permission

                            # Originally set to 0005 in the discussion below

                            # https://github.com/aws/aws-sam-cli/pull/2193#discussion_r513110608

                            # Changed to 0755 due to a regression in https://github.com/aws/aws-sam-cli/issues/2344

                            # Final PR: https://github.com/aws/aws-sam-cli/pull/2356/files

                            if permission_mappers:

                                # Set host OS to Unix

                                info.create_system = 3

                                # Set current permission of the file/dir to ZipInfo's external_attr

                                info.external_attr = os.stat(full_path).st_mode << 16

                                for permission_mapper in permission_mappers:

                                    info = permission_mapper.apply(info)

                                # Set current time to be the last time the zip content was modified.

                                info.date_time = time.localtime()[0:6]

                                zf.writestr(info, file_bytes, compress_type=compression_type)

                        else:

                            zf.write(full_path, relative_path)

                            else:

                                zf.write(full_path, relative_path)

        return zipfile_name

    make_zip = functools.partial(

        make_zip_with_permissions,

        permission_mappers=[

            WindowsFilePermissionPermissionMapper(permissions=0o100755),

            WindowsDirPermissionPermissionMapper(permissions=0o100755),

        ],

    )

    # Context: Jan 2023

    # NOTE(sriram-mv): Modify permissions regardless of the Operating system

    # to add 111 for directories and 444 for files in addition to existing permissions.

    # No overriding explicit permissions are set.

    # Extended Attributes are preserved.

    make_zip_with_lambda_permissions = functools.partial(

        make_zip_with_permissions,

        permission_mappers=[

            WindowsFilePermissionPermissionMapper(permissions=0o100755),

            WindowsDirPermissionPermissionMapper(permissions=0o100755),

            AdditiveFilePermissionPermissionMapper(permissions=0o100444),

            AdditiveDirPermissionPermissionMapper(permissions=0o100111),

        ],

    )

    def copy_to_temp_dir(filepath):

        tmp_dir = tempfile.mkdtemp()

        dst = os.path.join(tmp_dir, os.path.basename(filepath))

samcli/lib/sync/flows/auto_dependency_layer_sync_flow.py

            
                      Original file line number
                      Diff line number
                      Diff line change
                  
    @@ -11,7 +11,7 @@
  
    from samcli.lib.bootstrap.nested_stack.nested_stack_builder import NestedStackBuilder

    from samcli.lib.bootstrap.nested_stack.nested_stack_manager import NestedStackManager

    from samcli.lib.build.build_graph import BuildGraph

    from samcli.lib.package.utils import make_zip

    from samcli.lib.package.utils import make_zip_with_lambda_permissions

    from samcli.lib.providers.provider import Function, Stack

    from samcli.lib.providers.sam_function_provider import SamFunctionProvider

    from samcli.lib.sync.exceptions import (

    @@ -88,7 +88,7 @@ def gather_resources(self) -> None:
  
                self._get_compatible_runtimes()[0],

            )

            zip_file_path = os.path.join(tempfile.gettempdir(), "data-" + uuid.uuid4().hex)

            self._zip_file = make_zip(zip_file_path, self._artifact_folder)

            self._zip_file = make_zip_with_lambda_permissions(zip_file_path, self._artifact_folder)

            self._local_sha = file_checksum(cast(str, self._zip_file), hashlib.sha256())

        def _get_dependent_functions(self) -> List[Function]:

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: file/directory permissions within the zip while packaging artifacts #4655

Uh oh!

Diff view

Diff view

There are no files selected for viewing

Uh oh!