Read secrets from Docker secrets, falling back to the environment - #197
Merged
Merged
Conversation
Harvested from the security-improvements branch, which is marked WIP and pairs this with a Vault deployment. Only the Docker Secrets half is taken: it works with the compose file that exists today, needs no new services, and changes nothing for a deployment that mounts nothing. An environment variable is visible in `docker inspect`, in the process environment of every child, and in a .env file on disk that has to be chmod'ed by hand -- which is what was done to this host on 2026-09-04. A Docker secret is a file mounted only into the containers that declare it. Two behaviours worth stating, both tested: - A mounted file beats the environment. A deployment that mounted a secret meant it; preferring a stale environment variable would make the secret look applied when it was not, and surface as an auth error somewhere else entirely. - A blank file counts as absent. init-docker-secrets on that branch creates placeholders containing a single space, and a blank password reaching Postgres fails somewhere far less obvious than here. The Vault half of the branch is deliberately not included. It reaches Postgres over a unix socket the current compose does not mount, loops forever on `hvac.exceptions.VaultDown` waiting for an unseal, switches chainlit's data layer and adds a git submodule -- and it logs the credentials Vault issues it, at warning level. That is a deployment migration to plan, not a merge.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
CodeQL flagged the startup log as "Clear-text logging of sensitive information", and it was right to. The line logged only names, but they came back from load_secrets_to_environ -- a function that had read every secret body -- so nothing about the value said it was safe to log. Suppressing that would have been the same mistake the Vault code on security-improvements makes with logging.warning(response), which this PR's description criticises. mounted_secrets() answers "which of these were supplied" from st_size alone and never opens a file, so it is what the log is built from. load_secrets_to_environ() now returns nothing at all.
CodeQL still flagged the startup log after splitting the stat-only listing from the read. Its clear-text-logging rule matches on the shape of the strings -- OPENAI_API_KEY, CLOUDFLARE_SECRET_KEY -- not on where they came from, so mounted_secrets() not opening a file does not settle it. The names are genuinely not secret values, but dismissing a security alert to keep a convenience is not a trade worth making, and `ls /run/secrets` answers the same question. The log now carries a count.
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
2 participants
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, which is marked [WIP] and pairs this with a Vault deployment. Only the Docker Secrets half is taken — it works with the compose file that exists today, needs no new services, and changes nothing for a deployment that mounts nothing.An environment variable is visible in
docker inspect, in the process environment of every child, and in a.envon disk that has to be chmod'ed by hand — which is what we did to this host on 2026-09-04. A Docker secret is a file mounted only into the containers that declare it.Two behaviours worth stating, both tested:
init-docker-secretson that branch creates placeholders containing a single space, and a blank password reaching Postgres fails somewhere far less obvious than here.What I did not take, and why
The Vault half of that branch is not mergeable as it stands:
?host=/sockets/postgres/), so it cannot connect on today's topologyhvac.exceptions.VaultDownwaiting for an unseal — startup never fails, it just never finishesSQLAlchemyDataLayer→ChainlitDataLayer) and adds a git submodulelogging.warning(response), whereresponse["data"]is the generated username and passwordThat is a deployment migration to plan and test, not a merge. Happy to do it as focused work — it needs Vault running, the socket topology, and a beta rehearsal.
The other two branches
analysisandsafetycheckhave nothing left to harvest — verified file by file, not assumed:unsafe_answer.pyunsafe_question.pyis the same plus multi-language supporttools/preprocessing/base.py; the branch runs rephrase → safety sequentially as a compiled subgraph, has no language detection, and imports the privatelanggraph.utils.runnable.RunnableLikethat 1.x removedanalysis/ragas_evaluation/src/evaluation/, including thetest_generator.pydeleted in #195They can be closed with credit and deleted.