-
Notifications
You must be signed in to change notification settings - Fork 3
fix(module): fix CVE-2025-54410 and CVE-2025-54410 #1557
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
base: main
Are you sure you want to change the base?
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuidePatch CVE-2025-58058 by enhancing the CDI artifact build to use a timestamped cache version and the correct branch, and by bumping the xz library to v0.5.15. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `images/cdi-artifact/werf.inc.yaml:36` </location>
<code_context>
- id: SOURCE_REPO
value: {{ $.SOURCE_REPO }}
shell:
+ installCacheVersion: "{{ now | date "Mon Jan 2 15:04:05 MST 2006" }}"
install:
- |
</code_context>
<issue_to_address>
**question (performance):** Consider the impact of using a dynamic timestamp for installCacheVersion.
Using a timestamp as the cache version will cause the cache to be invalidated on every build, increasing build times. If this isn't intentional, consider using a stable cache key instead.
</issue_to_address>
### Comment 2
<location> `images/cdi-artifact/werf.inc.yaml:39-40` </location>
<code_context>
install:
- |
echo "Git clone CDI repository..."
- git clone --depth 1 --branch {{ $version }} $(cat /run/secrets/SOURCE_REPO)/{{ $gitRepoUrl }} /src/containerized-data-importer
+ git clone --depth 1 --branch fix/module/fix-cve $(cat /run/secrets/SOURCE_REPO)/{{ $gitRepoUrl }} /src/containerized-data-importer
rm -rf /src/containerized-data-importer/.git
</code_context>
<issue_to_address>
**suggestion:** Hardcoding the branch name may reduce flexibility for future updates.
If this is a temporary change, consider parameterizing the branch name or adding documentation to simplify future maintenance.
Suggested implementation:
```
# NOTE: Temporary override for CVE fix. Update/remove BRANCH_NAME as needed.
BRANCH_NAME="{{ $.CDI_BRANCH_NAME | default 'fix/module/fix-cve' }}"
git clone --depth 1 --branch $BRANCH_NAME $(cat /run/secrets/SOURCE_REPO)/{{ $gitRepoUrl }} /src/containerized-data-importer
rm -rf /src/containerized-data-importer/.git
```
To fully implement this, you should:
1. Add `CDI_BRANCH_NAME` to your values file or pipeline configuration if you want to override the branch name.
2. Document this environment variable for future maintainers.
</issue_to_address>
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
- id: SOURCE_REPO | ||
value: {{ $.SOURCE_REPO }} | ||
shell: | ||
installCacheVersion: "{{ now | date "Mon Jan 2 15:04:05 MST 2006" }}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question (performance): Consider the impact of using a dynamic timestamp for installCacheVersion.
Using a timestamp as the cache version will cause the cache to be invalidated on every build, increasing build times. If this isn't intentional, consider using a stable cache key instead.
echo "Git clone CDI repository..." | ||
git clone --depth 1 --branch {{ $version }} $(cat /run/secrets/SOURCE_REPO)/{{ $gitRepoUrl }} /src/containerized-data-importer | ||
git clone --depth 1 --branch fix/module/fix-cve $(cat /run/secrets/SOURCE_REPO)/{{ $gitRepoUrl }} /src/containerized-data-importer | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: Hardcoding the branch name may reduce flexibility for future updates.
If this is a temporary change, consider parameterizing the branch name or adding documentation to simplify future maintenance.
Suggested implementation:
# NOTE: Temporary override for CVE fix. Update/remove BRANCH_NAME as needed.
BRANCH_NAME="{{ $.CDI_BRANCH_NAME | default 'fix/module/fix-cve' }}"
git clone --depth 1 --branch $BRANCH_NAME $(cat /run/secrets/SOURCE_REPO)/{{ $gitRepoUrl }} /src/containerized-data-importer
rm -rf /src/containerized-data-importer/.git
To fully implement this, you should:
- Add
CDI_BRANCH_NAME
to your values file or pipeline configuration if you want to override the branch name. - Document this environment variable for future maintainers.
685ad4d
to
d700c55
Compare
Signed-off-by: Isteb4k <[email protected]>
d700c55
to
3d062ef
Compare
Description
Checklist
Changelog entries