Repository navigation
Harden the deployment: Postgres on a socket, credentials from Vault - #198
Merged
Merged
Conversation
Harvested from security-improvements, rebuilt on current main and with the pieces that would have broken production left out. Postgres no longer listens on TCP at all. It starts with --auth-host reject and POSTGRES_HOST_AUTH_METHOD=reject, and every consumer reaches it over a shared unix socket volume. Verified against a real container: `psql -h 127.0.0.1` gives "Connection refused", the socket refuses an unauthenticated connection with fe_sendauth, and an authenticated socket connection returns a row from the chainlit database that initdb created. Vault, where deployed, issues short-lived Postgres credentials (1h default, 24h max) over its own unix socket, and rotates the bootstrap POSTGRES_PASSWORD out of use as soon as it is configured. Without a Vault token mounted, get_db_uri falls back to POSTGRES_PASSWORD, which is the development path. The container also stops running as root: it builds and runs as appuser:appgroup, with application files copied read-only. Three defects in the original are fixed rather than carried: - It logged the credentials Vault issued it -- logging.warning(response) where response["data"] is the generated username and password. Nothing is logged now but the exception type. - It waited on `hvac.exceptions.VaultDown` in an unbounded loop, so a Vault that never got unsealed left the container running and silent forever. It now gives up after ~5 minutes and says what to do. - Its Dockerfile dropped `nltk.downloader punkt_tab`. BM25 tokenises with word_tokenize(language="english"), so retrieval would have downloaded it at runtime or failed. Two more found while writing the tests: - get_db_uri called twice in chat-chainlit.py, once inside the data layer factory chainlit invokes per session -- under Vault that mints a fresh lease per visitor. Built once now. - urllib.parse.quote leaves "/" unescaped by default, and Vault passwords are random punctuation; an unescaped slash truncates the URI at the database name. safe="" now. Deliberately not included: the ChainlitDataLayer switch and its chainlit-datalayer git submodule (a persistence change, not a security one), the survey message in config_default.yml, and the chainlit.md branding edit -- that one needs the per-deployment flag discussed separately, not a silent overwrite.
CI's "can every entry point be imported" check failed: the export scripts resolved their database URI at module scope and raised SystemExit when none was configured, and the runner has no Postgres. My change, and the wrong shape for it. Importing a script should do nothing; running it should fail loudly. The resolution now lives in a function the script calls from main(). Verified both halves: verify_imports.py passes, and calling the helper without POSTGRES_LANGGRAPH_DB still exits with the message naming what to set.
This was referenced Sep 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Harvested from
security-improvements, rebuilt on current main, with the pieces that would have broken production left out.What it does
Postgres no longer listens on TCP at all. It starts with
--auth-host rejectandPOSTGRES_HOST_AUTH_METHOD=reject; every consumer reaches it over a shared unix-socket volume.Verified against a real container, not reasoned about:
Vault, where deployed, issues short-lived Postgres credentials (1h default, 24h max) over its own unix socket, and rotates the bootstrap
POSTGRES_PASSWORDout of use once configured. Without a Vault token mounted,get_db_urifalls back toPOSTGRES_PASSWORD— the development path.The container stops running as root — it builds and runs as
appuser:appgroupwith application files copied read-only.Three defects in the original, fixed rather than carried
logging.warning(response), whereresponse["data"]is the generated username and passwordVaultDown— a Vault that never got unsealed left the container running and silent forevernltk.downloader punkt_tab— BM25 tokenises withword_tokenize(language="english")Two more found while writing the tests
get_db_uriwas called twice, once inside the data-layer factory chainlit invokes per session — under Vault that mints a fresh lease per visitor. Built once now.urllib.parse.quoteleaves/unescaped by default, and Vault passwords are random punctuation — an unescaped slash truncates the URI at the database name.safe=""now. The test caught this, not review.Deliberately not included
ChainlitDataLayerswitch and its chainlit-datalayer git submodule — a persistence change, not a security oneconfig_default.ymlchainlit.mdbranding edit — that needs the per-deployment flag we discussed, not a silent overwrite203 tests pass; ruff and mypy clean.