Skip to content

fix: add file locking and batch persist in credential pool#8070

Open
tomqiaozc wants to merge 1 commit into
NousResearch:mainfrom
tomqiaozc:fix/8040-credential-pool-file-lock
Open

fix: add file locking and batch persist in credential pool#8070
tomqiaozc wants to merge 1 commit into
NousResearch:mainfrom
tomqiaozc:fix/8040-credential-pool-file-lock

Conversation

@tomqiaozc

Copy link
Copy Markdown

Summary

  • _persist() writes the pool JSON without file locking — concurrent processes can corrupt the file
  • Add fcntl.flock-based locking around _persist() and _load_pool_from_disk()
  • Add persist parameter to sync methods to enable batch-persist pattern (reduce I/O)

Test plan

  • New locking tests verify concurrent access safety
  • All 27 existing credential_pool tests pass
  • Verified batch-persist reduces I/O during sync operations

Closes #8040

🤖 Generated with Claude Code

…search#8040)

_persist() writes the pool JSON without file locking, so concurrent
processes can corrupt the file. Add fcntl.flock-based locking and
batch _persist() calls during sync operations to reduce I/O.

Closes NousResearch#8040

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists area/auth Authentication, OAuth, credential pools labels Apr 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/auth Authentication, OAuth, credential pools P2 Medium — degraded but workaround exists type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: credential pool TOCTOU — process-local lock doesn't protect cross-process file access

2 participants