Lock config/cache files during read-modify-write - #21
Open
elhoim wants to merge 1 commit into
Open
Conversation
load_config/save_config and load_cache/save_cache had no locking around the read-modify-write cycle. Two concurrent --configure-module runs (or two concurrent queries that both populate the cache) would each load, mutate, and overwrite the whole file, so the last writer silently discarded the other's update (e.g. a saved module credential). Add a locked_file() context manager that takes an exclusive flock on a sibling .lock file, and use it to serialize the configure_module load-mutate-save sequence and the cache save (re-reading and merging entries under the lock so a concurrent run's cache writes aren't clobbered).
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.
Finding 18 (Medium) —
bin/cli.py:706Problem
load_config/save_config and load_cache/save_cache have no locking around the read-modify-write. Two concurrent --configure-module runs each load, mutate and overwrite the whole file; the last writer silently discards the other's credential.
Fix
Finding #18 (config-file-locking, medium): load_config/save_config and load_cache/save_cache had no locking around their read-modify-write cycle, so two concurrent
--configure-moduleruns (or two concurrent queries populating the cache) could each load, mutate and overwrite the whole file — the last writer silently discarding the other's saved credential/cache entry. Added alocked_file()context manager (bin/cli.py) that takes an exclusiveflockon a sibling<path>.lockfile, and used it to serialize (1) theconfigure_moduleload-mutate-save sequence around config_path, and (2) the cache save at the end ofmain, which now re-reads the cache file under the lock and merges in this run's new entries before writing, so a concurrent run's cache writes made in the meantime aren't clobbered.Verification
Reproduced against the unmodified code at
9b8c605, then re-checked after the change.Before
After
python bin/cli.py --helpexits 0 and the module still imports cleanly. Verification was performed offline against the pure functions — no runningmisp-modulesinstance is required.Branched from
9b8c605. This PR addresses only this finding; the other findings from the same review are in separate PRs, so they will need rebasing against each other as they merge.🤖 Generated with Claude Code
https://claude.ai/code/session_01DYX4TKA5inzByJ4qGWKjqh