Skip to content

fix: dockerfile copy vs add #208

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
merged 3 commits into from
Jul 4, 2025

Conversation

ceholden
Copy link
Contributor

@ceholden ceholden commented Jul 3, 2025

What type of PR is this? (check all applicable)

  • 🐛 Bug Fix

Related Issue

This PR fixes a release time issue with #206,
https://github.com/hotosm/OpenAerialMap/actions/runs/16059563354/job/45322169917

Describe this PR

This PR changes the Dockerfile to use COPY instead of ADD (as was used in the uv docs) since ADD is possibly less secure as it can interact with remote files.

Screenshots

N/A

Alternative Approaches Considered

N/A

Review Guide

Should we add Checkov to the CI/CD and perhaps also to the pre-commit setup? I checked that this passed,

checkov -f **/Dockerfile --skip-check CKV_DOCKER_8 --skip-check CKV_DOCKER_2 --skip-check CKV_DOCKER_3 --skip-check CKV_DOCKER_5

based on how the image publishing workflow uses checkov.

I also added Checkov to the pre-commit hook setup using the default "skip CVE" arguments.

edit - I reverted this because pre-commit.ci refused to clone the Checkov repo as it was too large

Checklist before requesting a review

[optional] What gif best describes this PR or how it makes you feel?

🚀

@ceholden ceholden requested a review from spwoodcock July 3, 2025 20:41
@ceholden ceholden changed the title Fix/dockerfile copy vs add fix: dockerfile copy vs add Jul 3, 2025
Copy link
Member

@spwoodcock spwoodcock left a comment

Choose a reason for hiding this comment

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

Surprised this is in the uv docs 😅

@spwoodcock spwoodcock merged commit 460a578 into hotosm:main Jul 4, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants