Repository navigation
fix(agent): prevent race condition in RunStore fingerprint key creation (#2164) - #2169
Open
yunusarda-sudo wants to merge 2 commits into
Open
yunusarda-sudo wants to merge 2 commits into
yunusarda-sudo wants to merge 2 commits into
Conversation
…on (OpenAdaptAI#2164) In RunStore._fingerprint_key, concurrent threads could race when creating fingerprint.key: one thread created the file via O_EXCL but hadn't finished writing it, causing a second thread encountering FileExistsError to read an empty or truncated key. Subsequent HMAC fingerprints would then disagree. Fix this by: 1. Writing the new key to a temporary file in root and fsyncing it before atomically linking (os.link) or replacing (os.replace) into place. 2. Rejecting any key shorter than 32 bytes on read with ValueError to fail closed against corrupt/empty keys. 3. Rejecting symlinks for fingerprint.key. 4. Adding tests asserting concurrent initialization safety and validation rejection in packages/flow/tests/test_agent_runs.py and packages/agent/tests/test_runs.py. Closes OpenAdaptAI#2164. Signed-off-by: Yunus Ardaguney <yunusarda-sudo@users.noreply.github.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
In
RunStore._fingerprint_key(openadapt_flow/agent/runs.py), concurrent threads creatingfingerprint.keycould race: one thread created the file viaos.O_CREAT | os.O_EXCLbut had not yet written its contents, while another thread receivingFileExistsErrorimmediately readpath.read_bytes(), resulting in an empty or partial key read. Subsequent HMAC fingerprints computed by differentRunStoreinstances would then disagree.Changes
.tmp-key-) withinself.rootand flush/os.fsyncit before atomically linking (os.link) or replacing (os.replace) intofingerprint.key.finallyblock.len(existing) < 32), raisingValueErrorif a short or empty key is encountered.fingerprint.keywithValueError.test_fingerprint_key_creation_races_under_concurrency_produce_same_keyasserting 20 concurrent threads across 8 workers all yield the exact same 32-byte key.ValueErroris raised on short keys, empty keys, and symlinks.packages/flow/tests/test_agent_runs.pyandpackages/agent/tests/test_runs.py.Closes #2164.
Signed-off-by: Yunus Ardaguney yunusarda-sudo@users.noreply.github.com