feat: read secrets from paths (#425)

* chore: added dev dependencies * feat: added get_secrets * feat: use get_secret instead of os.getenv for sensitive configs * feat: use get_secret in github_callback for github client secret * feat: added conftest * feat: added tests * feat: run tests in ci * feat: added secret config util * feat: added secret config util test * feat: use getSecret to initialize sensitive secrets * feat: use get_secret for license * chore: update tests * fix: tests --------- Co-authored-by: Rohan <[email protected]>
phasehq · Jan 6, 2025 · 5335f49 · 5335f49
1 parent f8d787d
commit 5335f49
Show file tree

Hide file tree

Showing 10 changed files with 510 additions and 41 deletions.
diff --git a/.github/workflows/backend.yml b/.github/workflows/backend.yml
@@ -17,6 +17,23 @@ jobs:
       - name: Checkout code
         uses: actions/checkout@v4
 
+      - name: Set up Python
+        uses: actions/setup-python@v5
+        with:
+          python-version: '3.11'
+          cache: 'pip'
+
+      - name: Install dependencies
+        run: |
+          cd backend
+          pip install -r requirements.txt
+          pip install -r dev-requirements.txt
+
+      - name: Run tests
+        run: |
+          cd backend
+          pytest tests/ -v
+
       - name: Set up Docker Buildx
         uses: docker/setup-buildx-action@v3
 
@@ -43,4 +60,4 @@ jobs:
             ${{ inputs.is_pr == 'true' && 'phasehq/backend-staging' || 'phasehq/backend' }}:${{ inputs.version }}
             ghcr.io/${{ github.repository }}/${{ inputs.is_pr == 'true' && 'backend-staging' || 'backend' }}:${{ inputs.version }}
           cache-from: type=gha
-          cache-to: type=gha,mode=max
+          cache-to: type=gha,mode=max
diff --git a/backend/api/views/auth.py b/backend/api/views/auth.py
@@ -11,6 +11,7 @@
 from api.models import ServiceAccountToken, ServiceToken, UserToken, CustomUser
 from api.emails import send_login_email
 from api.utils.syncing.auth import store_oauth_token
+from backend.utils.secrets import get_secret
 from backend.api.notifier import notify_slack
 from api.utils.rest import (
     get_token_type,
@@ -46,7 +47,7 @@ def github_callback(request):
     state = request.GET.get("state")
 
     client_id = os.getenv("GITHUB_INTEGRATION_CLIENT_ID")
-    client_secret = os.getenv("GITHUB_INTEGRATION_CLIENT_SECRET")
+    client_secret = get_secret("GITHUB_INTEGRATION_CLIENT_SECRET")
 
     state_decoded = base64.b64decode(state).decode("utf-8")
     state = json.loads(state_decoded)

diff --git a/backend/backend/settings.py b/backend/backend/settings.py
@@ -2,6 +2,7 @@
 from pathlib import Path
 from datetime import timedelta
 import logging.config
+from backend.utils.secrets import get_secret
 
 from ee.licensing.verifier import check_license
 
@@ -56,9 +57,9 @@ def get_version():
 # See https://docs.djangoproject.com/en/4.1/howto/deployment/checklist/
 
 # SECURITY WARNING: keep the secret key used in production secret!
-SECRET_KEY = os.getenv("SECRET_KEY")
+SECRET_KEY = get_secret("SECRET_KEY")
 
-SERVER_SECRET = os.getenv("SERVER_SECRET")
+SERVER_SECRET = get_secret("SERVER_SECRET")
 
 # SECURITY WARNING: don't run with debug turned on in production!
 DEBUG = True if os.getenv("DEBUG") == "True" else False
@@ -107,7 +108,7 @@ def get_version():
         "AUTH_PARAMS": {"access_type": "online"},
         "APP": {
             "client_id": os.getenv("GOOGLE_CLIENT_ID"),
-            "secret": os.getenv("GOOGLE_CLIENT_SECRET"),
+            "secret": get_secret("GOOGLE_CLIENT_SECRET"),
         },
     },
     "github": {
@@ -116,7 +117,7 @@ def get_version():
         ],
         "APP": {
             "client_id": os.getenv("GITHUB_CLIENT_ID"),
-            "secret": os.getenv("GITHUB_CLIENT_SECRET"),
+            "secret": get_secret("GITHUB_CLIENT_SECRET"),
         },
     },
     "gitlab": {
@@ -125,7 +126,7 @@ def get_version():
         ],
         "APP": {
             "client_id": os.getenv("GITLAB_CLIENT_ID"),
-            "secret": os.getenv("GITLAB_CLIENT_SECRET"),
+            "secret": get_secret("GITLAB_CLIENT_SECRET"),
             "settings": {
                 "gitlab_url": os.getenv("GITLAB_AUTH_URL") or "https://gitlab.com",
             },
@@ -146,7 +147,7 @@ def get_version():
 EMAIL_PORT = int(os.getenv("SMTP_PORT", 587))
 EMAIL_USE_TLS = True
 EMAIL_HOST_USER = os.getenv("SMTP_USERNAME")
-EMAIL_HOST_PASSWORD = os.getenv("SMTP_PASSWORD")
+EMAIL_HOST_PASSWORD = get_secret("SMTP_PASSWORD")
 DEFAULT_FROM_EMAIL = os.getenv("DEFAULT_FROM_EMAIL")
 
 
@@ -216,7 +217,7 @@ def get_version():
     "default": {
         "ENGINE": "django.db.backends.postgresql",
         "USER": os.getenv("DATABASE_USER"),
-        "PASSWORD": os.getenv("DATABASE_PASSWORD"),
+        "PASSWORD": get_secret("DATABASE_PASSWORD"),
         "NAME": os.getenv("DATABASE_NAME"),
         "HOST": os.getenv("DATABASE_HOST"),
         "PORT": os.getenv("DATABASE_PORT"),
@@ -274,7 +275,7 @@ def get_version():
 CLOUDFLARE = {
     "ACCOUNT_ID": os.getenv("CF_ACCOUNT_ID"),
     "KV_NAMESPACE": os.getenv("CF_KV_NAMESPACE"),
-    "API_KEY": os.getenv("CF_API_KEY"),
+    "API_KEY": get_secret("CF_API_KEY"),
     "ZONE_ID": os.getenv("CF_ZONE_ID"),
 }
 
@@ -291,13 +292,13 @@ def get_version():
     "default": {
         "HOST": os.getenv("REDIS_HOST"),
         "PORT": os.getenv("REDIS_PORT"),
-        "PASSWORD": os.getenv("REDIS_PASSWORD"),
+        "PASSWORD": get_secret("REDIS_PASSWORD"),
         "SSL": os.getenv("REDIS_SSL", None),
         "DB": 0,
     }
 }
 
-PHASE_LICENSE = check_license(os.getenv("PHASE_LICENSE_OFFLINE"))
+PHASE_LICENSE = check_license(get_secret("PHASE_LICENSE_OFFLINE"))
 
 
 STRIPE = {}

diff --git a/backend/backend/utils/secrets.py b/backend/backend/utils/secrets.py
@@ -0,0 +1,62 @@
+import os
+from pathlib import Path
+import logging
+
+logger = logging.getLogger(__name__)
+
+
+def get_secret(key: str) -> str | None:
+    """
+    Retrieve secrets from either files or environment variables. Implements a "secrets provider" pattern
+    commonly used in containerized applications.
+
+    1. Check if {key}_FILE exists as an environment variable (e.g. PGSQL_PASSWORD_FILE)
+        - If it exists, read the secret from that file location
+        - This supports Docker/Kubernetes secrets (https://docs.docker.com/reference/compose-file/secrets)
+    2. Fall back to checking if {key} exists as a regular environment variable
+
+    Example:
+        # Using file-based secret:
+        DATABASE_PASSWORD_FILE=/run/secrets/db_password
+        get_secret('DATABASE_PASSWORD')  # reads from /run/secrets/db_password
+
+        # Using environment variable:
+        DATABASE_PASSWORD=ebeefa2b4634ab18b0280c96fce1adc5969dcad133cce440353b5ed1a7387f0a
+        get_secret('DATABASE_PASSWORD')  # returns 'ebeefa2b4634ab18b0280c96fce1adc5969dcad133cce440353b5ed1a7387f0a'
+
+    Args:
+        key: Name of the secret to retrieve (e.g. 'DATABASE_PASSWORD')
+
+    Returns:
+        str: The secret value or None if not found
+    """
+
+    debug_mode = os.getenv("DEBUG", "False").lower() == "true"
+
+    file_env_key = f"{key}_FILE"
+    file_path = os.getenv(file_env_key)
+
+    if file_path:
+        path = Path(file_path)
+        if path.exists():
+            try:
+                secret = path.read_text().strip()
+                if debug_mode:
+                    logger.debug(f"Loaded secret '{key}' from file: {file_path}")
+                return secret
+            except (PermissionError, OSError) as e:
+                if debug_mode:
+                    logger.debug(f"Failed to read secret file for '{key}': {e}")
+        elif debug_mode:
+            logger.debug(
+                f"File path specified for '{key}' but file not found: {file_path}"
+            )
+
+    secret = os.getenv(key, None)
+    if debug_mode:
+        if secret:
+            logger.debug(f"Loaded secret '{key}' from environment variable")
+        else:
+            logger.debug(f"Secret '{key}' not found in environment or file")
+
+    return secret
diff --git a/backend/dev-requirements.txt b/backend/dev-requirements.txt
@@ -0,0 +1 @@
+pytest==8.3.4
diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py
@@ -0,0 +1,7 @@
+import os
+import sys
+from pathlib import Path
+
+# Add the project root directory to Python path
+project_root = Path(__file__).parent.parent
+sys.path.append(str(project_root))
diff --git a/backend/tests/utils/test_secret.py b/backend/tests/utils/test_secret.py
@@ -0,0 +1,170 @@
+import os
+import pytest
+from pathlib import Path
+import logging
+from unittest.mock import patch
+from backend.utils.secrets import get_secret
+
+
+@pytest.fixture
+def temp_secret_file(tmp_path):
+    """Create a temporary file containing a secret"""
+    secret_file = tmp_path / "secret.txt"
+    secret_file.write_text("file_secret_value")
+    return secret_file
+
+
+@pytest.fixture
+def caplog_debug(caplog):
+    """Configure logging to capture debug messages"""
+    caplog.set_level(logging.DEBUG)
+    return caplog
+
+
+def test_get_secret_from_environment():
+    """Test retrieving secret from environment variable"""
+    with patch.dict(os.environ, {"TEST_SECRET": "env_secret_value"}):
+        assert get_secret("TEST_SECRET") == "env_secret_value"
+
+
+def test_get_secret_from_file(temp_secret_file):
+    """Test retrieving secret from file when _FILE env var is set"""
+    with patch.dict(
+        os.environ,
+        {
+            "TEST_SECRET_FILE": str(temp_secret_file),
+        },
+    ):
+        assert get_secret("TEST_SECRET") == "file_secret_value"
+
+
+def test_get_secret_file_priority(temp_secret_file):
+    """Test that file-based secret takes priority over environment variable"""
+    with patch.dict(
+        os.environ,
+        {
+            "TEST_SECRET": "env_secret_value",
+            "TEST_SECRET_FILE": str(temp_secret_file),
+        },
+    ):
+        assert get_secret("TEST_SECRET") == "file_secret_value"
+
+
+def test_get_secret_missing_file():
+    """Test fallback to env var when file doesn't exist"""
+    with patch.dict(
+        os.environ,
+        {
+            "TEST_SECRET": "env_secret_value",
+            "TEST_SECRET_FILE": "/nonexistent/path",
+        },
+    ):
+        assert get_secret("TEST_SECRET") == "env_secret_value"
+
+
+def test_get_secret_neither_exists():
+    """Test None returned when neither file nor env var exists"""
+    with patch.dict(os.environ, {}, clear=True):
+        assert get_secret("TEST_SECRET") == None
+
+
+def test_get_secret_empty_file(tmp_path):
+    """Test handling of empty secret file"""
+    empty_file = tmp_path / "empty_secret.txt"
+    empty_file.write_text("")
+
+    with patch.dict(
+        os.environ,
+        {
+            "TEST_SECRET_FILE": str(empty_file),
+        },
+    ):
+        assert get_secret("TEST_SECRET") == ""
+
+
+def test_debug_logging_file_success(temp_secret_file, caplog_debug):
+    """Test debug logging when secret is successfully loaded from file"""
+    with patch.dict(
+        os.environ,
+        {
+            "DEBUG": "true",
+            "TEST_SECRET_FILE": str(temp_secret_file),
+        },
+    ):
+        get_secret("TEST_SECRET")
+        assert (
+            f"Loaded secret 'TEST_SECRET' from file: {temp_secret_file}"
+            in caplog_debug.text
+        )
+
+
+def test_debug_logging_file_not_found(caplog_debug):
+    """Test debug logging when secret file is not found"""
+    with patch.dict(
+        os.environ,
+        {
+            "DEBUG": "true",
+            "TEST_SECRET_FILE": "/nonexistent/path",
+        },
+    ):
+        get_secret("TEST_SECRET")
+        assert (
+            "File path specified for 'TEST_SECRET' but file not found"
+            in caplog_debug.text
+        )
+
+
+def test_debug_logging_env_var_success(caplog_debug):
+    """Test debug logging when secret is successfully loaded from env var"""
+    with patch.dict(
+        os.environ,
+        {
+            "DEBUG": "true",
+            "TEST_SECRET": "env_secret_value",
+        },
+    ):
+        get_secret("TEST_SECRET")
+        assert (
+            "Loaded secret 'TEST_SECRET' from environment variable" in caplog_debug.text
+        )
+
+
+def test_debug_logging_not_found(caplog_debug):
+    """Test debug logging when secret is not found anywhere"""
+    with patch.dict(
+        os.environ,
+        {
+            "DEBUG": "true",
+        },
+    ):
+        get_secret("TEST_SECRET")
+        assert (
+            "Secret 'TEST_SECRET' not found in environment or file" in caplog_debug.text
+        )
+
+
+def test_secret_file_with_whitespace(tmp_path):
+    """Test handling of secret files with whitespace"""
+    secret_file = tmp_path / "secret_with_whitespace.txt"
+    secret_file.write_text("  secret_value_with_spaces  \n")
+
+    with patch.dict(
+        os.environ,
+        {
+            "TEST_SECRET_FILE": str(secret_file),
+        },
+    ):
+        assert get_secret("TEST_SECRET") == "secret_value_with_spaces"
+
+
+def test_file_read_permission_error(tmp_path):
+    """Test handling of unreadable secret file"""
+    secret_file = tmp_path / "unreadable_secret.txt"
+    secret_file.write_text("secret_value")
+    secret_file.chmod(0o000)  # Remove all permissions
+
+    with patch.dict(
+        os.environ,
+        {"TEST_SECRET_FILE": str(secret_file), "TEST_SECRET": "fallback_value"},
+    ):
+        assert get_secret("TEST_SECRET") == "fallback_value"