Skip to content

security: run Docker container as non-root user #510

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

security: run Docker container as non-root user #510

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,17 @@ RUN mkdir -p /run/postgresql && \
chown -R postgres:postgres /run/postgresql && \
chmod 775 /run/postgresql

# To run as non-root, the user must be part of postgres, redis and node groups
RUN addgroup -g 1500 sourcebot && \
adduser -D -u 1500 -h /app -S sourcebot && \
adduser sourcebot postgres && \
adduser sourcebot redis && \
adduser sourcebot node && \
chown -R sourcebot /data && \
chown -R sourcebot /app && \
mkdir /var/log/sourcebot && \
chown sourcebot /var/log/sourcebot

COPY supervisord.conf /etc/supervisor/conf.d/supervisord.conf
COPY prefix-output.sh ./prefix-output.sh
RUN chmod +x ./prefix-output.sh
Expand All @@ -247,6 +258,8 @@ RUN chmod +x ./entrypoint.sh

COPY default-config.json .

USER sourcebot

EXPOSE 3000
ENV PORT=3000
ENV HOSTNAME="0.0.0.0"
Expand Down
31 changes: 22 additions & 9 deletions entrypoint.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,19 +30,20 @@ fi

# Check if DATA_CACHE_DIR exists, if not create it
if [ ! -d "$DATA_CACHE_DIR" ]; then
mkdir -p "$DATA_CACHE_DIR"
mkdir -m 0750 -p "$DATA_CACHE_DIR"
fi

# Check if DATABASE_DATA_DIR exists, if not initialize it
if [ "$DATABASE_EMBEDDED" = "true" ] && [ ! -d "$DATABASE_DATA_DIR" ]; then
echo -e "\e[34m[Info] Initializing database at $DATABASE_DATA_DIR...\e[0m"
mkdir -p $DATABASE_DATA_DIR && chown -R postgres:postgres "$DATABASE_DATA_DIR"
su postgres -c "initdb -D $DATABASE_DATA_DIR"
echo -e "\e[34m[Info] Initializing database at $DATABASE_D\ATA_DIR...\e[0m"
mkdir -m 0750 -p $DATABASE_DATA_DIR

initdb -D "$DATABASE_DATA_DIR"
Comment on lines +38 to +41
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Deadlock risk: initdb runs as “sourcebot” (superuser becomes sourcebot) but readiness/psql assume user “postgres”

As written, initdb creates DB superuser = current OS user (“sourcebot”). The subsequent until loop waits on pg_isready -U postgres (role doesn’t exist yet), so it can spin forever. Later createuser relies on connecting as “sourcebot”, but you never reach it if pg_isready is gated on a non-existent role. Also fix the log typo and avoid echo -e in /bin/sh.

Apply this to make the superuser “postgres”, fix the log, quote paths, and use printf:

-    echo -e "\e[34m[Info] Initializing database at $DATABASE_D\ATA_DIR...\e[0m"
-    mkdir -m 0750 -p $DATABASE_DATA_DIR
-
-    initdb -D "$DATABASE_DATA_DIR"
+    printf '\033[34m[Info] Initializing database at %s...\033[0m\n' "$DATABASE_DATA_DIR"
+    install -d -m 0750 -- "$DATABASE_DATA_DIR"
+    initdb -D "$DATABASE_DATA_DIR" -U postgres
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
echo -e "\e[34m[Info] Initializing database at $DATABASE_D\ATA_DIR...\e[0m"
mkdir -m 0750 -p $DATABASE_DATA_DIR
initdb -D "$DATABASE_DATA_DIR"
printf '\033[34m[Info] Initializing database at %s...\033[0m\n' "$DATABASE_DATA_DIR"
install -d -m 0750 -- "$DATABASE_DATA_DIR"
initdb -D "$DATABASE_DATA_DIR" -U postgres
🧰 Tools
🪛 Shellcheck (0.10.0)

[warning] 38-38: In POSIX sh, echo flags are undefined.

(SC3037)

[warning] 39-39: When used with -p, -m only applies to the deepest directory.

(SC2174)

🤖 Prompt for AI Agents 
In entrypoint.sh around lines 38 to 41, the script currently uses echo -e,
mis-typed DATABASE_D\ATA_DIR, and runs initdb so the DB superuser becomes the
current OS user (sourcebot), which deadlocks later because pg_isready and
createuser expect the "postgres" role; update the lines to: use printf instead
of echo -e, fix the variable to $DATABASE_DATA_DIR and quote it everywhere
(mkdir -m 0750 -p "$DATABASE_DATA_DIR" and initdb -D "$DATABASE_DATA_DIR"), and
ensure initdb creates the postgres superuser by passing the username option
(initdb --username=postgres or -U postgres) so subsequent pg_isready -U postgres
and createuser steps succeed.

fi

# Create the redis data directory if it doesn't exist
if [ ! -d "$REDIS_DATA_DIR" ]; then
mkdir -p $REDIS_DATA_DIR
mkdir -m 0750 -p $REDIS_DATA_DIR
fi

if [ -z "$SOURCEBOT_ENCRYPTION_KEY" ]; then
Expand Down Expand Up @@ -134,13 +135,25 @@ echo "{\"version\": \"$NEXT_PUBLIC_SOURCEBOT_VERSION\", \"install_id\": \"$SOURC

# Start the database and wait for it to be ready before starting any other service
if [ "$DATABASE_EMBEDDED" = "true" ]; then
su postgres -c "postgres -D $DATABASE_DATA_DIR" &
until pg_isready -h localhost -p 5432 -U postgres; do
postgres -D "$DATABASE_DATA_DIR" &
until pg_isready -h localhost -p 5432 -d sourcebot -U postgres; do
echo -e "\e[34m[Info] Waiting for the database to be ready...\e[0m"
sleep 1

# As postgres runs in the background, we must check if it is still
# running, otherwise the "until" loop will be running indefinitely.
if ! pgrep -x "postgres" > /dev/null; then
echo "postgres failed to run"
exit 1
break
fi
done

# Check if the database already exists, and create it if it dne
# Running as non-root we need to ensure the postgres account is created.
psql -U postgres -tc "SELECT 1 FROM pg_roles WHERE rolname='postgres'" | grep -q 1 \
|| createuser postgres -s

# Check if the database already exists, and create it if it doesn't
EXISTING_DB=$(psql -U postgres -tAc "SELECT 1 FROM pg_database WHERE datname = 'sourcebot'")

if [ "$EXISTING_DB" = "1" ]; then
Expand All @@ -159,4 +172,4 @@ yarn workspace @sourcebot/db prisma:migrate:prod
mkdir -p /var/log/sourcebot

# Run supervisord
exec supervisord -c /etc/supervisor/conf.d/supervisord.conf
exec supervisord -c /etc/supervisor/conf.d/supervisord.conf
Loading