Skip to content

Fixed a variable quoting issue in the Docker entrypoint script #9511

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

Draft
Guiorgy wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Fixed a variable quoting issue in the Docker entrypoint script #9511

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
22 changes: 21 additions & 1 deletion pkg/docker/entrypoint.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,30 @@ EOF
done
fi

# Ensures the variable starts and ends in quotes after trimming
ensure_quoted() {
local var="$1"

# Trim whitespaces
var="${var#"${var%%[![:space:]]*}"}"
var="${var%"${var##*[![:space:]]}"}"

# Check if the variable is already double or single quoted
if [[ ! $var =~ ^\"(.*)\"$ && ! $var =~ ^\'(.*)\'$ ]]; then
# Use docstrings in case the value contains quote characters inside
var="'''${var}'''"
fi

echo "${var}"
}
Comment on lines +70 to +85
Copy link

@coderabbitai coderabbitai bot Jan 5, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Critical: Shell metacharacters in URIs will be expanded despite Python triple-quotes.

The triple-quote approach (line 81) attempts to address single quotes in URIs using Python's triple-quoted string syntax. While this handles single quotes better than regular single quotes, it introduces critical security and correctness issues:

  1. Shell variable expansion: When the sanitized value is substituted inside double quotes on line 93 ("...${PGADMIN_CONFIG_CONFIG_DATABASE_URI}..."), the shell will still expand dollar signs, backticks, backslashes, and exclamation marks—even when they appear inside the triple quotes, because double quotes preserve these special meanings. For example, a URI like postgresql://user:pa$word@host/db becomes '''postgresql://user:pa$word@host/db''', but when substituted inside double quotes, $word is interpreted as a shell variable.

  2. Triple-quote breakage: If the URI contains ''', Python parsing will fail.

  3. Already-quoted check flaw: The regex on line 79 checks for quote characters in the value, but environment variables don't preserve shell quotes—if a user sets PGADMIN_CONFIG_CONFIG_DATABASE_URI='value', the environment variable contains value (without quotes). This check only catches URIs that contain literal quote characters as part of the data.

🔎 Recommended fix: Use environment variables to pass the URI safely to Python

This approach avoids all shell quoting/escaping issues:

-# Ensures the variable starts and ends in quotes after trimming
-ensure_quoted() {
-    local var="$1"
-
-    # Trim whitespaces
-    var="${var#"${var%%[![:space:]]*}"}"
-    var="${var%"${var##*[![:space:]]}"}"
-
-    # Check if the variable is already double or single quoted
-    if [[ ! $var =~ ^\"(.*)\"$ && ! $var =~ ^\'(.*)\'$ ]]; then
-        # Use docstrings in case the value contains quote characters inside
-        var="'''${var}'''"
-    fi
-
-    echo "${var}"
-}
-
 # Check whether the external configuration database exists if it is being used.
 external_config_db_exists="False"
 if [ -n "${PGADMIN_CONFIG_CONFIG_DATABASE_URI}" ]; then
-    # Support both quoted and unquoted URIs for backwards compatibility
-    PGADMIN_CONFIG_CONFIG_DATABASE_URI=$(ensure_quoted "${PGADMIN_CONFIG_CONFIG_DATABASE_URI}")
-
-    external_config_db_exists=$(cd /pgadmin4/pgadmin/utils && /venv/bin/python3 -c "from check_external_config_db import check_external_config_db; val = check_external_config_db(${PGADMIN_CONFIG_CONFIG_DATABASE_URI}); print(val)")
+    external_config_db_exists=$(cd /pgadmin4/pgadmin/utils && /venv/bin/python3 -c "import os; from check_external_config_db import check_external_config_db; val = check_external_config_db(os.environ.get('PGADMIN_CONFIG_CONFIG_DATABASE_URI', '')); print(val)")
 fi

This passes the URI through the environment, avoiding all shell metacharacter interpretation.

Committable suggestion skipped: line range outside the PR's diff.

Copy link
Contributor Author

@Guiorgy Guiorgy Jan 5, 2026

Choose a reason for hiding this comment

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

  1. When expanding variables inside double quotes, e.g. "$var", the contents are treated as literal strings, no? The target that receives that string is the Python interpreter, which does no such expansions.

  2. The likelyhood is almsot 0.

  3. Currently, people are using environmental variables with literal quotes, e.g. PGADMIN_CONFIG_CONFIG_DATABASE_URI='"value"'.

Proposed solution. While the idea is actually good, we would be breaking all the existing code (see point 3 above), so can you propose a solution that accounts for that and keeps it backwards compatible?

Copy link

@coderabbitai coderabbitai bot Jan 5, 2026

Choose a reason for hiding this comment

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

@Guiorgy, you're absolutely right about point 1—I was mistaken about the shell expansion behavior. When ${PGADMIN_CONFIG_CONFIG_DATABASE_URI} is substituted inside double quotes, the shell replaces it with the variable's value, but that value is treated as literal text (no further expansion of $, backticks, etc. within the value itself). My apologies for the confusion.

For backwards compatibility with users who have quotes embedded in their environment variable values, here's a revised approach using os.environ with Python-side quote handling:

# Check whether the external configuration database exists if it is being used.
external_config_db_exists="False"
if [ -n "${PGADMIN_CONFIG_CONFIG_DATABASE_URI}" ]; then
    external_config_db_exists=$(cd /pgadmin4/pgadmin/utils && /venv/bin/python3 << 'PYEOF'
import os
from check_external_config_db import check_external_config_db

uri = os.environ.get('PGADMIN_CONFIG_CONFIG_DATABASE_URI', '')
# Strip surrounding quotes for backwards compatibility with quoted env vars
uri = uri.strip()
if len(uri) >= 2:
    if (uri[0] == '"' and uri[-1] == '"') or (uri[0] == "'" and uri[-1] == "'"):
        uri = uri[1:-1]

val = check_external_config_db(uri)
print(val)
PYEOF
)
fi

This approach:

  1. Uses heredoc (<< 'PYEOF') to avoid all shell quoting/escaping complexity
  2. Passes the URI via environment (no injection risk)
  3. Strips surrounding quotes in Python for backwards compatibility with users who added quotes to work around the original bug
  4. Handles both quoted ('"value"') and unquoted (value) environment variable values


# Check whether the external configuration database exists if it is being used.
external_config_db_exists="False"
if [ -n "${PGADMIN_CONFIG_CONFIG_DATABASE_URI}" ]; then
external_config_db_exists=$(cd /pgadmin4/pgadmin/utils && /venv/bin/python3 -c "from check_external_config_db import check_external_config_db; val = check_external_config_db("${PGADMIN_CONFIG_CONFIG_DATABASE_URI}"); print(val)")
# Support both quoted and unquoted URIs for backwards compatibility
PGADMIN_CONFIG_CONFIG_DATABASE_URI=$(ensure_quoted "${PGADMIN_CONFIG_CONFIG_DATABASE_URI}")

external_config_db_exists=$(cd /pgadmin4/pgadmin/utils && /venv/bin/python3 -c "from check_external_config_db import check_external_config_db; val = check_external_config_db(${PGADMIN_CONFIG_CONFIG_DATABASE_URI}); print(val)")
fi

# DRY of the code to load the PGADMIN_SERVER_JSON_FILE
Expand Down
Loading