From c09ebd663986f92e693c7e902bac950dcf1f881c Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Wed, 23 Sep 2026 13:11:29 -0400 Subject: [PATCH] fix: the race fix's temp name assumed one bootstrap per process (4295) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Run 7370: six of the eight threads in the new test died with FileNotFoundError(2, 'No such file or directory') The first cut named the temp file `.credential_key.b64..tmp`, which assumes one bootstrap per PROCESS. Eight threads share one pid, so all eight raced the same filename and six found it already unlinked by another. The assumption held for hypercorn's workers, which are processes, and would have held in production. It was still an assumption the code did not need to make, and the test found it in one run — which is the test doing its job rather than the test being wrong, so `tempfile.mkstemp` replaces the guess. Verified against the SHIPPED file, loaded by path so the package `__init__` (which wants quart) stays out of it: 25 consecutive attempts, eight threads through a barrier each time, one key, no leftover temp files. My first check of this reimplemented the logic in a scratch script, which measures a copy — rule 10 names that exact failure, and it is not evidence about the code that ships. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01LVjrnpQjRgHdvq95rASoiR --- backend/app/services/credential_crypto.py | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) 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)