From 303f239fcd2384e89b06df10e9cb47526ae268ca Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 16 Sep 2026 00:11:38 +0100 Subject: [PATCH] fix: tighten an existing .env to 0600 before rewriting it; CI checks secrets have values envfile.update only applied 0600 when it created the file, so an existing .env with a wider mode kept it while secrets were written into it. The mode is now set on the open descriptor before the file is truncated and written. The CI step now requires POSTGRES_PASSWORD and JWT_SECRET_KEY to have values: an empty one falls back to a default without any check noticing. --- .github/workflows/docker-image-verify.yml | 5 +++-- docsgpt/deploy/envfile.py | 8 ++++++-- tests/deploy/test_envfile.py | 10 ++++++++++ 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/.github/workflows/docker-image-verify.yml b/.github/workflows/docker-image-verify.yml index f59bbf9a..dad39d91 100644 --- a/.github/workflows/docker-image-verify.yml +++ b/.github/workflows/docker-image-verify.yml @@ -119,9 +119,10 @@ jobs: "$docsgpt" up --yes --dir "$stack" --image-tag verify "$docsgpt" status --dir "$stack" curl -fsS http://127.0.0.1:7091/ | grep -q 'src="/config.js"' - grep -q '^POSTGRES_PASSWORD=' "$stack/.env" + # The secrets must have values: an empty one would fall back to a default silently. + grep -Eq '^POSTGRES_PASSWORD=.+$' "$stack/.env" # Running up again keeps the generated secrets. - before=$(grep '^JWT_SECRET_KEY=' "$stack/.env") + before=$(grep -E '^JWT_SECRET_KEY=.+$' "$stack/.env") "$docsgpt" up --yes --dir "$stack" --image-tag verify [ "$(grep '^JWT_SECRET_KEY=' "$stack/.env")" = "$before" ] "$docsgpt" uninstall --yes --purge --dir "$stack" diff --git a/docsgpt/deploy/envfile.py b/docsgpt/deploy/envfile.py index fce83ae5..ff91e09b 100644 --- a/docsgpt/deploy/envfile.py +++ b/docsgpt/deploy/envfile.py @@ -52,7 +52,7 @@ def read(path: Path) -> dict[str, str]: def update(path: Path, values: Mapping[str, Optional[str]]) -> None: """Set each key in place (``None`` removes it), append new keys, and leave every other line alone. - A new file is created readable by its owner only: it holds secrets. + The file is left readable by its owner only, including one that existed with a wider mode: it holds secrets. """ formatted = {key: None if value is None else _format_value(value) for key, value in values.items()} path = Path(path) @@ -71,6 +71,10 @@ def update(path: Path, values: Mapping[str, Optional[str]]) -> None: out.extend(f"{key}={value}" for key, value in formatted.items() if key not in written and value is not None) path.parent.mkdir(parents=True, exist_ok=True) - descriptor = os.open(path, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600) + descriptor = os.open(path, os.O_WRONLY | os.O_CREAT, 0o600) + if hasattr(os, "fchmod"): + # The creation mode only applies to a new file; tighten an existing one before writing. + os.fchmod(descriptor, 0o600) + os.ftruncate(descriptor, 0) with os.fdopen(descriptor, "w", encoding="utf-8") as handle: handle.write("\n".join(out) + "\n" if out else "") diff --git a/tests/deploy/test_envfile.py b/tests/deploy/test_envfile.py index 0ccb3aae..e292be1c 100644 --- a/tests/deploy/test_envfile.py +++ b/tests/deploy/test_envfile.py @@ -76,6 +76,16 @@ class TestUpdate: envfile.update(path, {"JWT_SECRET_KEY": "s"}) assert oct(os.stat(path).st_mode & 0o777) == oct(0o600) + @pytest.mark.skipif(sys.platform == "win32", reason="POSIX permissions") + def test_an_existing_readable_file_is_made_private_before_writing(self, tmp_path): + """Secrets are rewritten into the file, so a permissive mode left from before must not stay.""" + path = tmp_path / ".env" + path.write_text("LLM_PROVIDER=openai\n") + os.chmod(path, 0o644) + envfile.update(path, {"JWT_SECRET_KEY": "s"}) + assert oct(os.stat(path).st_mode & 0o777) == oct(0o600) + assert envfile.read(path) == {"LLM_PROVIDER": "openai", "JWT_SECRET_KEY": "s"} + def test_rejects_a_newline_in_a_value(self, tmp_path): with pytest.raises(ValueError, match="newline"): envfile.update(tmp_path / ".env", {"A": "1\n2"})