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

Fix anchore scan for pull requests #53

Closed
wants to merge 7 commits into from
Closed

Fix anchore scan for pull requests #53

wants to merge 7 commits into from

Conversation

ldallmayr
Copy link

@ldallmayr ldallmayr commented Dec 14, 2023

Currently, running the anchore scan on pull requests does not scan the changed files from the pull request. Instead it uses the Dockerfile, where the main repo https://github.com/Threagile/threagile.git is being cloned.

In order to properly scan the pull request I refactored Dockerfile.local and used it in the github action. Now the actual changes from the pull request are considered in the scan.

One possibly controversial change is the option fail-build: false in .github/workflows/anchore-analysis.yml. This allows the build to succeed while still reporting the found vulnerabilities in the Security tab.
For example, currently there are vulnerabilities in alpine linux that do not have a fix available. Most of the time this cannot be avoided.

Generally I would recommend only having one Dockerfile and removing the docker-build-*.sh files or moving them to a non-default branch. If someone wants to build from the master branch, simply using git stash --include-untracked and git checkout master should be enough to build the main container file.

I would recommend versioning and automatically pushing the image to Dockerhub instead though. If the maintainers are interested in this I could help setting that up.

@ezavgorodniy
Copy link
Collaborator

ezavgorodniy commented Dec 15, 2023

Hi Leonard,

If the maintainers are interested in this I could help setting that up.

I know the PR is not the best place to try to make a contact however I couldn't send you a request via LinkedIn (https://www.linkedin.com/in/yevhen-zavhorodnii-40773132/) because we don't have common connections and couldn't find alternative way to connect. Could you please suggest the best way to contact you?

I'm not officially a maintainer of the project however I am personally as well as my company is very interested in it and chatting now with an author and other contributors about the way to maintain/contribute.

# RUN mkdir -p /lib64 && ln -s /lib/libc.musl-x86_64.so.1 /lib64/ld-linux-x86-64.so.2
# clean apk cache
RUN rm -rf /var/cache/apk/*
RUN apk add --no-cache \
Copy link
Collaborator

Choose a reason for hiding this comment

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

It would be nice to know the context why adding libc6-compat (RUN apk add libc6-compat) was removed. As far as I can see that was a workaround, is adding everything withing one RUN apk add fixing the problem and how?

Copy link
Author

Choose a reason for hiding this comment

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

I changed the building step to run in golang:alpine instead of the default golang (which is based on Debian). Because of this we do not need libc6-compat as a compatibility layer for glibc anymore.

Dockerfile.local Outdated Show resolved Hide resolved
ENV PATH=/app:$PATH
ENV GIN_MODE=release
# Add non-privileged user for running threagile
RUN adduser \
Copy link
Collaborator

Choose a reason for hiding this comment

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

Kudos for adding best security practices for the project which is about improving security ;)

joreiche added a commit to joreiche/threagile that referenced this pull request Feb 2, 2024
@joreiche
Copy link
Collaborator

joreiche commented Feb 2, 2024

@ ldallmayr I merged your changes into my fork so they are part of pr #57. thank you for your contribution

@joreiche
Copy link
Collaborator

joreiche commented Feb 7, 2024

this pr has been resolved with #57

@joreiche joreiche closed this Feb 7, 2024
z00mi pushed a commit to z00mi/threagile that referenced this pull request Apr 23, 2024
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.

3 participants