Skip to content

{confcom}: Simplify logic in map_image_from_tar(_compatibility)?#9838

Open
micromaomao wants to merge 3 commits intoAzure:mainfrom
micromaomao:tingmao/fix-map-image
Open

{confcom}: Simplify logic in map_image_from_tar(_compatibility)?#9838
micromaomao wants to merge 3 commits intoAzure:mainfrom
micromaomao:tingmao/fix-map-image

Conversation

@micromaomao
Copy link
Copy Markdown
Member

The current code extracts an item in the tar based on a path controlled by the
manifest within the tar, which is prone to path traversal on Linux and Windows,
and potentially other path confusion attacks on Windows if the Config field in
the manifest contains special names.

This extraction is unnecessary as we can read the content of the config directly
from the tar file using extractfile. This commit simplifies it to do that.

Also, there are two versions of the map_image_from_tar function, with the second
one introduced in #7414. Later code changes in #8238 means that these functions
now do basically the same thing (with different clean up code). After this
simplification, these two functions are exactly the same, so let's also remove
the _compatibility one.

Before:

> az confcom acipolicygen --image mcr.microsoft.com/aci/skr:2.14 --tar image-docker-malformed.tar --outraw-pretty-print
...
Pulling and hashing images...:   0%|                                                                                                        | 0/2 [00:00<?, ?percent/s]
ERROR: The command failed with an unexpected error. Here is the traceback:
ERROR: [Errno 13] Permission denied: '../../../../../blobs'
Traceback (most recent call last):
File "/home/mao/.az-cli-newpy.venv/lib/python3.13/site-packages/knack/cli.py", line 233, in invoke
    cmd_result = self.invocation.execute(args)
File "/home/mao/src/github.com/Microsoft/azure-cli/src/azure-cli-core/azure/cli/core/commands/__init__.py", line 677, in execute
    raise ex
File "/home/mao/src/github.com/Microsoft/azure-cli/src/azure-cli-core/azure/cli/core/commands/__init__.py", line 820, in _run_jobs_serially
    results.append(self._run_job(expanded_arg, cmd_copy))
                ~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/mao/src/github.com/Microsoft/azure-cli/src/azure-cli-core/azure/cli/core/commands/__init__.py", line 789, in _run_job
    result = cmd_copy(params)
File "/home/mao/src/github.com/Microsoft/azure-cli/src/azure-cli-core/azure/cli/core/commands/__init__.py", line 335, in __call__
    return self.handler(*args, **kwargs)
        ~~~~~~~~~~~~^^^^^^^^^^^^^^^^^
File "/home/mao/src/github.com/Microsoft/azure-cli/src/azure-cli-core/azure/cli/core/commands/command_operation.py", line 120, in handler
    return op(**command_args)
File "/home/mao/.azure/cliextensions/confcom/azext_confcom/custom.py", line 197, in acipolicygen_confcom
    policy.populate_policy_content_for_all_images(
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
        individual_image=bool(image_name), tar_mapping=tar_mapping, faster_hashing=faster_hashing
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    )
    ^
File "/home/mao/.azure/cliextensions/confcom/azext_confcom/security_policy.py", line 485, in populate_policy_content_for_all_images
    image_info, tar = get_image_info(progress, message_queue, tar_mapping, image)
                    ~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/mao/.azure/cliextensions/confcom/azext_confcom/template_util.py", line 111, in get_image_info
    image_info = os_util.map_image_from_tar_backwards_compatibility(
        image_name, tar_file, tar_location
    )
File "/home/mao/.azure/cliextensions/confcom/azext_confcom/os_util.py", line 190, in map_image_from_tar_backwards_compatibility
    tar.extract(info_file.name, path=tar_dir)
    ~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/mao/.local/share/uv/python/cpython-3.13.12-linux-x86_64-gnu/lib/python3.13/tarfile.py", line 2416, in extract
    self._extract_one(tarinfo, path, set_attrs, numeric_owner)
    ~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/mao/.local/share/uv/python/cpython-3.13.12-linux-x86_64-gnu/lib/python3.13/tarfile.py", line 2464, in _extract_one
    self._handle_fatal_error(e)
    ~~~~~~~~~~~~~~~~~~~~~~~~^^^
File "/home/mao/.local/share/uv/python/cpython-3.13.12-linux-x86_64-gnu/lib/python3.13/tarfile.py", line 2458, in _extract_one
    self._extract_member(tarinfo, os.path.join(path, tarinfo.name),
    ~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                        set_attrs=set_attrs,
                        ^^^^^^^^^^^^^^^^^^^^
                        numeric_owner=numeric_owner,
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                        filter_function=filter_function,
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                        extraction_root=path)
                        ^^^^^^^^^^^^^^^^^^^^^
File "/home/mao/.local/share/uv/python/cpython-3.13.12-linux-x86_64-gnu/lib/python3.13/tarfile.py", line 2539, in _extract_member
    os.makedirs(upperdirs, exist_ok=True)
    ~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^
File "<frozen os>", line 218, in makedirs
File "<frozen os>", line 228, in makedirs
PermissionError: [Errno 13] Permission denied: '../../../../../blobs'
To check existing issues, please visit: https://github.com/Azure/azure-cli/issues

After:

> az confcom acipolicygen --image mcr.microsoft.com/aci/skr:2.14 --tar image-docker-malformed.tar --outraw-pretty-print | grep -C10 layers
...
Pulling and hashing images...: 100%|████████████████████████████████████████████████████████████████████████████████████████████████| 2/2 [00:01<00:00,  1.38percent/s]
WARNING: mcr.microsoft.com/aci/skr:2.14 read from local tar file
        "strategy": "re2"
      },
      {
        "pattern": "azurecontainerinstance_restarted_by=.+",
        "required": false,
        "strategy": "re2"
      }
    ],
    "exec_processes": [],
    "id": "mcr.microsoft.com/aci/skr:2.14",
    "layers": [
      "a189b02d4858578459fda1dfbd7c6a4557c44208b9829e02b931771a6d611c39",
      "300f9661fb3d46c0f299ad6f552b7ad0c41ea5141755b0b3feaca3081a108f7a",
      "0afffca98bacf8e7b6e6f7982459a03219f60555523163c73c4b092e0a3deef2",
      "eefefd5009aed4ba4478876995d1a18aa3a670661fcc61d2e4cba6e2b79da0a1",
      "b868a7e1bebef40e5bf4d58fe271c0a10a351e68b12179ec019af9f6c75781ae",
      "8b4842f06982817534a75bcf71865213b09dfa8313229c384e5201dadbd75e25"
    ],
...

(Validated that this matches the result of using the image reference directly
without a tar, and also matches with the result of using the oci tar from oras backup)

Signed-off-by: Tingmao Wang tingmaowang@microsoft.com


This checklist is used to make sure that common guidelines for a pull request are followed.

Related command

General Guidelines

  • Have you run azdev style <YOUR_EXT> locally? (pip install azdev required)
  • Have you run python scripts/ci/test_index.py -q locally? (pip install wheel==0.30.0 required)
  • My extension version conforms to the Extension version schema

For new extensions:

About Extension Publish

There is a pipeline to automatically build, upload and publish extension wheels.
Once your pull request is merged into main branch, a new pull request will be created to update src/index.json automatically.
You only need to update the version information in file setup.py and historical information in file HISTORY.rst in your PR but do not modify src/index.json.

@azure-client-tools-bot-prd
Copy link
Copy Markdown

Validation for Breaking Change Starting...

Thanks for your contribution!

@micromaomao micromaomao marked this pull request as ready for review April 30, 2026 17:52
Copilot AI review requested due to automatic review settings April 30, 2026 17:52
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR mitigates path traversal risks when generating policies from Docker image tarballs by avoiding filesystem extraction and instead reading required JSON directly from the tar stream. It also removes redundant backwards-compatibility code now equivalent to the primary implementation.

Changes:

  • Read manifest.json and the referenced config JSON directly from the tar via extractfile/read_file_from_tar (no extraction to disk).
  • Remove map_image_from_tar_backwards_compatibility and its use in get_image_info.
  • Bump extension version to 2.0.0b3 and document the security fix in HISTORY.rst.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File Description
src/confcom/setup.py Version bump to ship the fix (2.0.0b3).
src/confcom/azext_confcom/template_util.py Removes backwards-compatibility tar parsing path.
src/confcom/azext_confcom/os_util.py Eliminates disk extraction; reads manifest/config from tar stream; removes redundant function.
src/confcom/HISTORY.rst Adds release note for the path traversal fix.

# NOTE: read manifest.json directly (not via read_file_from_tar) so that a
# missing manifest.json raises KeyError. The caller relies on that to fall
# back to the OCI layout v1 reader.
manifest_bytes = tar.extractfile("manifest.json").read()
Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tar.extractfile(...) can return None for non-regular members (e.g., directories/links). Calling .read() on None will raise AttributeError. Consider explicitly handling the None case and raising a clear exception (or KeyError) so the caller’s fallback logic behaves predictably.

Copilot uses AI. Check for mistakes.
Comment thread src/confcom/azext_confcom/os_util.py
Comment thread src/confcom/azext_confcom/os_util.py Outdated
Comment thread src/confcom/azext_confcom/os_util.py
Comment thread src/confcom/azext_confcom/os_util.py
@yonzhan
Copy link
Copy Markdown
Collaborator

yonzhan commented Apr 30, 2026

Thank you for your contribution! We will review the pull request and get back to you soon.

@micromaomao micromaomao marked this pull request as draft April 30, 2026 18:15
@github-actions
Copy link
Copy Markdown
Contributor

The git hooks are available for azure-cli and azure-cli-extensions repos. They could help you run required checks before creating the PR.

Please sync the latest code with latest dev branch (for azure-cli) or main branch (for azure-cli-extensions).
After that please run the following commands to enable git hooks:

pip install azdev --upgrade
azdev setup -c <your azure-cli repo path> -r <your azure-cli-extensions repo path>

@github-actions
Copy link
Copy Markdown
Contributor

@yonzhan yonzhan requested a review from necusjz April 30, 2026 23:24
@yonzhan yonzhan removed the request for review from zhoxing-ms April 30, 2026 23:25
Comment thread src/confcom/azext_confcom/os_util.py Outdated
The current code extracts an item in the tar based on a path controlled by the
manifest within the tar, which is prone to path traversal on Linux and Windows,
and potentially other path confusion attacks on Windows if the Config field in
the manifest contains special names.

This extraction is unnecessary as we can read the content of the config directly
from the tar file using extractfile.  This commit simplifies it to do that.

Also, there are two versions of the map_image_from_tar function, with the second
one introduced in Azure#7414. Later code changes in Azure#8238 means that these functions
now do basically the same thing (with different clean up code). After this
simplification, these two functions are exactly the same, so let's also remove
the _compatibility one.

Before:

    > az confcom acipolicygen --image mcr.microsoft.com/aci/skr:2.14 --tar image-docker-malformed.tar --outraw-pretty-print
    ...
    Pulling and hashing images...:   0%|                                                                                                        | 0/2 [00:00<?, ?percent/s]
    ERROR: The command failed with an unexpected error. Here is the traceback:
    ERROR: [Errno 13] Permission denied: '../../../../../blobs'
    Traceback (most recent call last):
    File "/home/mao/.az-cli-newpy.venv/lib/python3.13/site-packages/knack/cli.py", line 233, in invoke
        cmd_result = self.invocation.execute(args)
    File "/home/mao/src/github.com/Microsoft/azure-cli/src/azure-cli-core/azure/cli/core/commands/__init__.py", line 677, in execute
        raise ex
    File "/home/mao/src/github.com/Microsoft/azure-cli/src/azure-cli-core/azure/cli/core/commands/__init__.py", line 820, in _run_jobs_serially
        results.append(self._run_job(expanded_arg, cmd_copy))
                    ~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^
    File "/home/mao/src/github.com/Microsoft/azure-cli/src/azure-cli-core/azure/cli/core/commands/__init__.py", line 789, in _run_job
        result = cmd_copy(params)
    File "/home/mao/src/github.com/Microsoft/azure-cli/src/azure-cli-core/azure/cli/core/commands/__init__.py", line 335, in __call__
        return self.handler(*args, **kwargs)
            ~~~~~~~~~~~~^^^^^^^^^^^^^^^^^
    File "/home/mao/src/github.com/Microsoft/azure-cli/src/azure-cli-core/azure/cli/core/commands/command_operation.py", line 120, in handler
        return op(**command_args)
    File "/home/mao/.azure/cliextensions/confcom/azext_confcom/custom.py", line 197, in acipolicygen_confcom
        policy.populate_policy_content_for_all_images(
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
            individual_image=bool(image_name), tar_mapping=tar_mapping, faster_hashing=faster_hashing
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        )
        ^
    File "/home/mao/.azure/cliextensions/confcom/azext_confcom/security_policy.py", line 485, in populate_policy_content_for_all_images
        image_info, tar = get_image_info(progress, message_queue, tar_mapping, image)
                        ~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    File "/home/mao/.azure/cliextensions/confcom/azext_confcom/template_util.py", line 111, in get_image_info
        image_info = os_util.map_image_from_tar_backwards_compatibility(
            image_name, tar_file, tar_location
        )
    File "/home/mao/.azure/cliextensions/confcom/azext_confcom/os_util.py", line 190, in map_image_from_tar_backwards_compatibility
        tar.extract(info_file.name, path=tar_dir)
        ~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    File "/home/mao/.local/share/uv/python/cpython-3.13.12-linux-x86_64-gnu/lib/python3.13/tarfile.py", line 2416, in extract
        self._extract_one(tarinfo, path, set_attrs, numeric_owner)
        ~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    File "/home/mao/.local/share/uv/python/cpython-3.13.12-linux-x86_64-gnu/lib/python3.13/tarfile.py", line 2464, in _extract_one
        self._handle_fatal_error(e)
        ~~~~~~~~~~~~~~~~~~~~~~~~^^^
    File "/home/mao/.local/share/uv/python/cpython-3.13.12-linux-x86_64-gnu/lib/python3.13/tarfile.py", line 2458, in _extract_one
        self._extract_member(tarinfo, os.path.join(path, tarinfo.name),
        ~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                            set_attrs=set_attrs,
                            ^^^^^^^^^^^^^^^^^^^^
                            numeric_owner=numeric_owner,
                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                            filter_function=filter_function,
                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                            extraction_root=path)
                            ^^^^^^^^^^^^^^^^^^^^^
    File "/home/mao/.local/share/uv/python/cpython-3.13.12-linux-x86_64-gnu/lib/python3.13/tarfile.py", line 2539, in _extract_member
        os.makedirs(upperdirs, exist_ok=True)
        ~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^
    File "<frozen os>", line 218, in makedirs
    File "<frozen os>", line 228, in makedirs
    PermissionError: [Errno 13] Permission denied: '../../../../../blobs'
    To check existing issues, please visit: https://github.com/Azure/azure-cli/issues

After:

    > az confcom acipolicygen --image mcr.microsoft.com/aci/skr:2.14 --tar image-docker-malformed.tar --outraw-pretty-print | grep -C10 layers
    ...
    Pulling and hashing images...: 100%|████████████████████████████████████████████████████████████████████████████████████████████████| 2/2 [00:01<00:00,  1.38percent/s]
    WARNING: mcr.microsoft.com/aci/skr:2.14 read from local tar file
            "strategy": "re2"
          },
          {
            "pattern": "azurecontainerinstance_restarted_by=.+",
            "required": false,
            "strategy": "re2"
          }
        ],
        "exec_processes": [],
        "id": "mcr.microsoft.com/aci/skr:2.14",
        "layers": [
          "a189b02d4858578459fda1dfbd7c6a4557c44208b9829e02b931771a6d611c39",
          "300f9661fb3d46c0f299ad6f552b7ad0c41ea5141755b0b3feaca3081a108f7a",
          "0afffca98bacf8e7b6e6f7982459a03219f60555523163c73c4b092e0a3deef2",
          "eefefd5009aed4ba4478876995d1a18aa3a670661fcc61d2e4cba6e2b79da0a1",
          "b868a7e1bebef40e5bf4d58fe271c0a10a351e68b12179ec019af9f6c75781ae",
          "8b4842f06982817534a75bcf71865213b09dfa8313229c384e5201dadbd75e25"
        ],
    ...

(Validated that this matches the result of using the image reference directly
without a tar, and also matches with the result of using the oci tar from `oras
backup`)

Signed-off-by: Tingmao Wang <tingmaowang@microsoft.com>
@micromaomao micromaomao force-pushed the tingmao/fix-map-image branch from 95b3dde to ae21410 Compare May 5, 2026 09:46
@micromaomao micromaomao marked this pull request as ready for review May 5, 2026 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Auto-Assign Auto assign by bot Logic App

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants