Skip to content
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

TA-3622: Downgrade AdmZip version #194

Merged
merged 2 commits into from
Mar 13, 2025

Conversation

promeris
Copy link
Collaborator

@promeris promeris commented Mar 12, 2025

Description

Downgrade AdmZip library to a version where zip descriptors are not checked. The nested zips we are generating during config export/import commands create zips that don't contain descriptors, and the library we are using doesn't provide with any way to force zip descriptors to be written during exports. Issues were introduced with this PR

Downgrade will offer a temporary solution until either a fix is provided in the library itself, or otherwise we will need to switch libraries.

Ticket: https://celonis.atlassian.net/browse/TA-3622

Checklist

  • I have self-reviewed this PR
  • I have tested the change and proved that it works in different scenarios
    • Tested config export and config import
  • I have updated docs if needed

Sorry, something went wrong.

@promeris promeris requested review from GencBlakqoriP9 and a team as code owners March 12, 2025 14:43
jetakasabaqi
jetakasabaqi previously approved these changes Mar 12, 2025
@meritonhusaj meritonhusaj requested a review from qmucolli March 12, 2025 15:53
@qmucolli
Copy link
Collaborator

qmucolli commented Mar 12, 2025

I'm curious to understand how the extraction/import of zip happens in the team to team copy as I've also tested this flow locally and it seemed to be working fine.

On a side-note, there was a vulnerability reported with versions older than the latest version of adm-zip, can we make sure to run the SAST scan once this PR is merged to verify that we're not introducing new vulnerabilities?

@promeris
Copy link
Collaborator Author

I'm curious to understand how the extraction/import of zip happens in the team to team copy as I've also tested this flow locally and it seemed to be working fine.

On a side-note, there was a vulnerability reported with versions older than the latest version of adm-zip, can we make sure to run the SAST scan once this PR is merged to verify that we're not introducing new vulnerabilities?

Team to team copy is using the content-cli config export/import commands, and when testing through UI it's using a fixed version now and not auto-updating, so not a breaking change immediately, however when testing through only content-cli, you can bump into the issue with no present descriptor.

I can check the SAST scan after merging, since the actions currently don't provide with option to run a branch.

@promeris promeris merged commit aedab7d into master Mar 13, 2025
1 check passed
@promeris promeris deleted the promeris/ta-3622-downgrade-adm-zip-library branch March 13, 2025 13:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants