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

Added user config to Dockerfile #297

Closed
wants to merge 2 commits into from
Closed

Added user config to Dockerfile #297

wants to merge 2 commits into from

Conversation

gerethd
Copy link
Contributor

@gerethd gerethd commented Aug 21, 2021

No description provided.

@gerethd gerethd changed the title Issue 295 Added user config to Dockerfile Aug 23, 2021
@@ -13,4 +13,11 @@ RUN true
COPY --from=builder application/snapshot-dependencies/ ./
RUN true
COPY --from=builder application/application/ ./

RUN uid=$(($(($((`date +%s` / 60)) % 165535)) + 100000)) \
Copy link
Owner

Choose a reason for hiding this comment

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

this seems unecessarily complex. Why not a fixed uid?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because using a a random id makes it harder for someone to say run a different docker container connected to the same volume as the one you are running and have access to your files, especially if backed by NFS or something similar. Not the end all be all in security but it does increase the barrier to entry for malicious users.

@gerethd gerethd closed this Sep 29, 2021
@F43nd1r
Copy link
Owner

F43nd1r commented Feb 6, 2022

@gerethd Why did you close this?

@gerethd
Copy link
Contributor Author

gerethd commented Feb 7, 2022

@gerethd Why did you close this?

Because I continuously found more and more issues with this backend and at a certain point it became easier, and quicker with less hassle just to start over with a custom backend that is more specific to what I need from it, and a proper database to back it.

Feel free to reopen and merge but I decided I didn't want to spend the time and effort to push this through for a application I'm no longer using, not to mention having to justify basic container security practices such as this is a major red flag security wise, as any vunlerabilty scanner for containers will tell you the same thing.

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