Skip to content

Harden Dockerfile security with non-root user#1031

Closed
RinZ27 wants to merge 2 commits intotensorflow:masterfrom
RinZ27:fix/docker-security-hardening
Closed

Harden Dockerfile security with non-root user#1031
RinZ27 wants to merge 2 commits intotensorflow:masterfrom
RinZ27:fix/docker-security-hardening

Conversation

@RinZ27
Copy link
Copy Markdown
Contributor

@RinZ27 RinZ27 commented Mar 27, 2026

Refactoring the module loading logic in load_module.py to use except Exception: instead of a bare except:. Correcting this prevents catching system-level signals like KeyboardInterrupt, which I noticed was a potential reliability issue in the original implementation.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the Dockerfile to improve security by creating and switching to a non-root user 'tfq-user' and setting the working directory to the user's home. The review feedback suggests explicitly defining the User ID (UID) and Group ID (GID) for the new user to ensure consistency across build environments and prevent potential permission issues with volume mounts.

Comment thread release/docker/Dockerfile Outdated
@RinZ27 RinZ27 force-pushed the fix/docker-security-hardening branch from 8ca7681 to 54e071d Compare March 27, 2026 13:21
@mhucka mhucka added area/health Involves general matters of project configuration, health, maintenance, and similar concerns area/devops Involves build systems, Make files, Bazel files, continuous integration, and/or other DevOps topics labels Apr 14, 2026
@mhucka mhucka self-assigned this Apr 14, 2026
@mhucka
Copy link
Copy Markdown
Member

mhucka commented Apr 14, 2026

Thank you for your contribution.

The way the changes to Dockerfile are done in this PR means that at run time, the user inside the Docker environment does not have access to file systems mounted when the Docker container is started. (An example of where that's needed is release/build_distribution.sh.) The reason is that useradd is invoked with a specific fixed user id, and at run time, this id inside the Docker environment may not match the id of the user running Docker. When you switch to a non-root user in a Docker container, you often lose access to mounted file systems because the UID (User ID) or GID (Group ID) of the container user does not match the owner of the files on the host.

I also don't know how to solve the problem without putting a requirement on how the docker container is run.

On balance, I wonder whether this is worth pursuing futher. It's true that there is this security issue, but because these Docker containers are meant for testing purposes by developers on their local systems, and are not distributed anywhere, the dangers are pretty limited. So, unless you can think of a simple solution to the problem of matching user id's, I would recommend closing this.

@RinZ27
Copy link
Copy Markdown
Contributor Author

RinZ27 commented Apr 16, 2026

@mhucka Since these containers are primarily for internal dev and testing, I agree that the simplicity of the current root setup is likely more practical than adding the overhead of dynamic UID handling. Closing this to keep the project workflow clean.

@RinZ27 RinZ27 closed this Apr 16, 2026
@RinZ27 RinZ27 deleted the fix/docker-security-hardening branch April 16, 2026 12:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/devops Involves build systems, Make files, Bazel files, continuous integration, and/or other DevOps topics area/health Involves general matters of project configuration, health, maintenance, and similar concerns

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants