fix: detect duplicate node_id in Load() and SetForTest (PILOT-136)#3
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Collaborator
Author
🔍 Matthew PR Explain — #3 PILOT-136What this PR doesAdds duplicate Changes
WhyIf a maintainer accidentally duplicates an agent entry in Verification
|
Collaborator
Author
🦾 Matthew PR Check — #3 PILOT-136Status
PR details
|
added 2 commits
May 28, 2026 10:42
Adds a test that Load() must reject JSON with duplicate node_id entries. Current code silently overwrites (last entry wins), so this test FAILS until the fix lands.
… (PILOT-136) Load() now returns a descriptive error when the JSON contains duplicate node_id entries (previously the last duplicate silently won). SetForTest panics on duplicates since test lists must be well-formed. Load protects the daemon's trusted-agents list from silently accepting the wrong agent when a maintainer accidentally duplicates a node_id. A bad refresh (network corruption, whatnot) that produces duplicate entries also fails safe — the previous known-good list is not replaced. Closes PILOT-136
TeoSlayer
force-pushed
the
openclaw/pilot-136-20260528-134059
branch
from
2a04ef3 to
ed15d7a
Compare
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
2 participants
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.
What failed
Load()indata.go:111-130silently accepts duplicatenode_identries — the last duplicate wins via map overwrite. If a maintainer accidentally duplicates an agent entry intrusted-agents.json, the duplicate silently replaces the original hostname. A corrupted or malicious refresh could also inject a name collision without detection.Why this fix
Load()now detects duplicatenode_identries during the loop and returns a descriptive error before committing the map. The previous known-good list is preserved — a bad refresh fails safe.SetForTest()panics on duplicates since test lists must be well-formed (the function signature has no error return).Verification
go build ./...✅go vet ./...✅go test ./...✅ (newTestLoadDuplicateNodeIDpasses)data.go(+7),zz_test.go(+16)Closes PILOT-136