diff --git a/backend/app/services/credential_crypto.py b/backend/app/services/credential_crypto.py index 65cb930..5308018 100644 --- a/backend/app/services/credential_crypto.py +++ b/backend/app/services/credential_crypto.py @@ -24,6 +24,7 @@ rows undecryptable (recovery = delete the rows and re-upload). import logging import os +import tempfile from pathlib import Path from cryptography.fernet import Fernet, InvalidToken @@ -101,10 +102,21 @@ class CredentialCrypto: # `os.replace`, which would succeed — so two processes that both # generated a key would each think they had won, and the loser's key # would overwrite the one the winner had already handed to Fernet. + # `mkstemp`, not a pid-derived name. The first cut spelled the temp + # file `.credential_key.b64..tmp`, which assumes one bootstrap per + # process — and the test that exercises this with eight THREADS shares + # one pid, so all eight raced the same filename and six died with + # FileNotFoundError when another had already unlinked it. The + # assumption held for hypercorn's workers and would have held in + # production; it was still an assumption the code did not need to make. key = Fernet.generate_key() - tmp = parent / f".{self._key_path.name}.{os.getpid()}.tmp" + fd, tmp_name = tempfile.mkstemp( + dir=parent, prefix=f".{self._key_path.name}.", suffix=".tmp", + ) + tmp = Path(tmp_name) try: - tmp.write_bytes(key) + with os.fdopen(fd, "wb") as fh: + fh.write(key) os.chmod(tmp, 0o600) try: os.link(tmp, self._key_path)