Skip to content

fix: add lock on config.yaml to prevent TOCTOU coruption#50

Merged
Zack (Zackaryia) merged 4 commits into
mainfrom
zshamsi/fix-config-filelock
May 26, 2026
Merged

fix: add lock on config.yaml to prevent TOCTOU coruption#50
Zack (Zackaryia) merged 4 commits into
mainfrom
zshamsi/fix-config-filelock

Conversation

@Zackaryia
Copy link
Copy Markdown
Contributor

Add race_test.go that demonstrates IIP-20714: multiple concurrent read-modify-write cycles on config.yaml without file locking causes invalid YAML, empty file reads, and silent data loss.

@Zackaryia Zack (Zackaryia) changed the title test: reproduce config file corruption from concurrent CLI instances fix: add lock on config.yaml to prevent TOCTOU coruption May 21, 2026
Add race_test.go that demonstrates IIP-20714: multiple concurrent
read-modify-write cycles on config.yaml without file locking causes
invalid YAML, empty file reads, and silent data loss.
@Zackaryia Zack (Zackaryia) force-pushed the zshamsi/fix-config-filelock branch 2 times, most recently from 7b5b0e5 to f13abb7 Compare May 22, 2026 16:05
Concurrent CLI invocations race on config.yaml through the non-atomic
read-modify-write in New() (viper.WriteConfig + addDocCommentsToYAML).
Wrap the entire critical section in an exclusive flock via gofrs/flock.
@Zackaryia Zack (Zackaryia) force-pushed the zshamsi/fix-config-filelock branch from f13abb7 to 378f62c Compare May 22, 2026 16:09
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is because the file lock fails before writing fails, so we need to edit the error message. Permission denied makes more sense in context.

Comment thread internal/config/config.go
Comment thread internal/config/config.go
Comment thread internal/config/race_test.go Outdated
Comment thread internal/config/config_test.go Outdated
@Zackaryia Zack (Zackaryia) marked this pull request as ready for review May 22, 2026 20:52
@Zackaryia Zack (Zackaryia) requested a review from a team as a code owner May 22, 2026 20:52
…test

Falls back to t.TempDir() when RACE_DATA_DIR is not set, so the test
no longer skips/fatals when run individually or in CI outside of the
race suite.
@Zackaryia Zack (Zackaryia) merged commit 67b58b2 into main May 26, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants