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"})