fix(app_setup): create the secret key file private instead of tightening it after the write (#1659)

load_or_create_secret_key writes the Flask secret with write_bytes and
then chmods it to 0600. Until the chmod the key sits in a file with
whatever the umask allows, and if the chmod fails it stays that way and
only a warning is logged. The file is now opened with mode 0600, an
existing one is fchmod'ed before anything is written, and then the key
goes in. The new test makes chmod fail and checks the file still ends up
private, which fails on main.


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

<!-- coderabbit-ai-generated:v1 -->

## Summary by CodeRabbit

* **Bug Fixes**
* Secret-key files now receive restrictive permissions when created or
replaced, helping prevent unauthorized access to the stored key.
* Key creation maintains these permissions even when a later
permission-setting operation fails.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

shelfmark/core/app_setup.py

@@ -90,8 +90,12 @@ def load_or_create_secret_key(config_dir: Path) -> bytes:
     secret_key = os.urandom(64)
     try:
         secret_path.parent.mkdir(parents=True, exist_ok=True)
-        secret_path.write_bytes(secret_key)
-        secret_path.chmod(0o600)
+        # Created private, and an existing file made private before the key goes in, so the key
+        # is never in a file others can read.
+        fd = os.open(secret_path, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600)
+        with os.fdopen(fd, "wb") as secret_file:
+            os.fchmod(secret_file.fileno(), 0o600)
+            secret_file.write(secret_key)
     except OSError as exc:
         logger.warning(
             "Failed to persist Flask secret key at %s. Sessions may reset on restart: %s",

tests/core/test_app_setup.py

@@ -6,6 +6,7 @@ from __future__ import annotations
 
 import importlib
 import logging
+import os
 from unittest.mock import patch
 
 import pytest
@@ -48,6 +49,35 @@ class TestSecretKey:
         assert path.read_bytes() == created == again
         assert path.stat().st_mode & 0o777 == 0o600
 
+    def test_the_key_is_never_written_to_a_file_others_can_read(
+        self, main_module, tmp_path, monkeypatch
+    ):
+        # Writing first and tightening the mode after leaves the key readable under the umask until
+        # the chmod, and readable for good when the chmod fails.
+        def chmod_fails(*_args, **_kwargs):
+            raise OSError("chmod not supported here")
+
+        monkeypatch.setattr("pathlib.Path.chmod", chmod_fails)
+        old_umask = os.umask(0o022)
+        try:
+            key = load_or_create_secret_key(tmp_path)
+        finally:
+            os.umask(old_umask)
+
+        path = tmp_path / ".flask_secret"
+        assert path.read_bytes() == key
+        assert path.stat().st_mode & 0o777 == 0o600
+
+    def test_a_replaced_key_file_is_made_private(self, main_module, tmp_path):
+        path = tmp_path / ".flask_secret"
+        path.write_bytes(b"short")
+        path.chmod(0o644)
+
+        key = load_or_create_secret_key(tmp_path)
+
+        assert path.read_bytes() == key
+        assert path.stat().st_mode & 0o777 == 0o600
+
     def test_a_key_that_is_too_short_is_replaced(self, main_module, tmp_path):
         path = tmp_path / ".flask_secret"
         path.write_bytes(b"short")