SQLAlchemy Exception leaks Supabase db password in GitHub Actions
The database password. The doorway to data. The base of my data. Clearly visible in GitHub Actions log output.
The offending line was printed below when investigating why one of my workflows failed.
kwargs = {'dbname': 'postgres', 'host': 'aws-0-ap-southeast-2.pooler.supabase.com', 'password': 'dBLuvaGg73R2clnE', 'port': 5432, ...}
Yes, that's dBLuvaGg73R2clnE you see clear as day.
(don't worry, I've changed the db password already...)
The actions job step was simply running coverage run -m pytest -v --junitxml=results.xml, i.e execute unit tests. Nothing nefarious about that.
Let's try find out what happened.
Below are the contents of my workflow file:
jobs:
run_unit_tests:
runs-on: ubuntu-latest
env:
DATABASE_URL: ${{ secrets.DATABASE_URL }}
steps:
- name: Checkout code
uses: actions/checkout@v2
- name: Set up Python
uses: actions/setup-python@v2
with:
python-version: '3.12.3'
- name: Install python requirements
run: |
python -m pip install --upgrade pip
pip install -r requirements.txt
- name: Run local db migration
run: |
bash migrations-run-local.sh
- name: Run tests with Coverage
run: |
coverage run -m pytest -v --junitxml=results.xml
coverage xml
We have a job, run commands defined, and an env var DATABASE_URL included. Pretty typical stuff.
Looking deeper into the logs, the trigger for this error seems to be caused when creating a new database connection. Why are we creating a new database connection? Well, the unit tests utilize an in-memory, disposable sqlite database, created at run-time.
app = create_app("sqlite:///:memory:")
Now that's fine in isolation for the unit tests, however something else is acting up here.
Session mode max clients are limited to pool_size
/opt/hostedtoolcache/Python/3.12.3/x64/lib/python3.12/site-packages/psycopg2/__init__.py:122: OperationalError
The above exception was the direct cause of the following exception:
@pytest.fixture
def app():
app = create_app("sqlite:///:memory:")
app.testing = True
with app.app_context():
> db.create_all()
So, the error is started when the app() function is invoked, causing the Exception OperationalError. The logs also describe the FATAL error as relating to the max number of connection pools when using Supabase's Session Pooler method.
In my case, Supabase db's default maximum number of connections is set to 15.
Why would a unit test, using an in-memory sqlite db, try connect to my Supabase instance? That part is still clear unclear to me. Let's see what Copilot though of this conundrum:
The direct cause: "from app import db" (tests/unit/test_saves.py line 2) imported your app module which initialized SQLAlchemy against the environment DATABASE_URL (Supabase) before the tests could configure sqlite.
Thank you Copilot. Meanwhile, within my main db module, I actually map Flask's config property SQLALCHEMY_DATABASE_URI to DATABASE_URL. That's fantastic when I read in env vars or a local .env file. But ... given each unit test imports this very module (as a db client), each db import thinks something like "hey, I see DATABASE_URL in the environment variables, time to create an intentional connection!".
Additionally in the unit tests themselves, wherever a db.session.add/commit is used, a pool connection may be utilized. Here is a description of this behaviour from the SQLAlchemy docs:
Because of the pool, when an application makes use of a SQL database connection, most typically from either making use of Engine.connect() or when making queries using an ORM Session, this activity does not necessarily establish a new connection to the database at the moment the connection object is acquired;
it instead consults the connection pool for a connection, which will often retrieve an existing connection from the pool to be re-used. If no connections are available, the pool will create a new database connection, but only if the pool has not surpassed a configured capacity.
Thank you docs. I checked in my unit tests and found ... 29 instances of db.session.add/commit. That's definitely >15. I'll need to revisit that.
Now, circling back to the original point of this post, I can now see
- how my db password was sourced into the Actions runner environment
- attempts by SQLAlchemy to create new database connections
- Supabase pool limits being quickly reached
Still, it's worrying that the SQLAlchemy error output contains
- a db password
- a postgres db ip
After bit more reading, it looks like this is caused by Flask's logging not playing nice with SQLAlchemy's. SQLAlchemy's built-in masking is available to the database engine via engine.url.render_as_string(hide_password=True), however Flask does not support this when logging. I don't see any immediate solutions to this outside of introducing customized Exception/Error handling to mask any messages that contain sensitive info.
Perhaps there's a public Github Action that reads all your generated workflow logs, extracts your servers & app credentials, replacing them with *****, leaving your Action logs squeaky clean?
Eh, on second thought, probably not a good idea...