-
Notifications
You must be signed in to change notification settings - Fork 14
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
#1867 - Fix SonarCloud high security issues #1951
Conversation
- This image might run with root as the default user.
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.
👍 . Pls address other devs comment's
|
||
# Building app | ||
RUN npm run build queue-consumers | ||
|
||
# Ensure that the non-root user will be able to write the coverage report. | ||
RUN mkdir -p /app/apps/queue-consumers/coverage/ && chown node /app/apps/queue-consumers/coverage/ |
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.
Prod does not need it.
COPY . . | ||
# Copying sources. | ||
COPY ./apps/queue-consumers ./apps/queue-consumers | ||
COPY ./apps/db-migrations ./apps/db-migrations |
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.
Prod does not need it.
COPY . . | ||
# Copying sources. | ||
COPY ./apps/workers ./apps/workers | ||
COPY ./apps/db-migrations ./apps/db-migrations |
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.
Prod does not need it.
|
||
# Building app | ||
RUN npm run build workers | ||
|
||
# Ensure that the non-root user will be able to write the coverage report. | ||
RUN mkdir -p /app/apps/workers/coverage/ && chown node /app/apps/workers/coverage/ |
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.
Prod does not need it.
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.
Nice work, please take a look at the comments.
Kudos, SonarCloud Quality Gate passed!
|
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.
Thanks for doing the changes, looks good 👍
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.
Great work on fixing the issues and also reaching out to team members for 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.
Looks good!
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.
Looks good!
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.
LGTM, nice work @sh16011993
This PR has the following issues fixed for api, queue-consumers and workers:
Issue: Running containers as a privileged user weakens their runtime security.
Fix: The images for api, workers and queue-consumers now run as a non-root user
Issue: Copying recursively might inadvertently add sensitive data to the container.
Fix: Only the required directories and files are now being copied.
The following issue is not fixed as a part of this PR:
Make sure write access is not granted to
other
users.