fix: atomic file publication and authenticated IPC lock daemon - #2083
Merged
Conversation
gnodet
force-pushed
the
security/locking-atomicity
branch
from
August 30, 2026 19:34
c4d9f95 to
14b23c9
Compare
…handling Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
cstamas
approved these changes
Aug 31, 2026
gnodet
commented
Aug 31, 2026
gnodet
left a comment
Contributor
Author
There was a problem hiding this comment.
Excellent security-hardening PR — defense-in-depth across the IPC lock protocol with comprehensive tests.
Strengths
- Bootstrap token via stdin instead of argv prevents exposure through
/proc/<pid>/cmdline— the right approach SecureRandomoverRandomfor the bootstrap token — required for a security credential- Per-connection context scoping is a solid privilege-separation fix: one client can no longer close or acquire on another client's lock contexts
- Netstring-like lock key digest prevents boundary-shifted collisions (
["foo","bar"]vs["foob","ar"]) — clean and correct - Bounded preallocation (
Math.min(sz, 1024)) prevents memory exhaustion from malicious wire-supplied sizes - Socket file permissions (owner-only rwx) on Unix domain sockets add a second layer beyond the token
retryingMovenever-truncate contract eliminates the torn-read window where a concurrent reader or process kill could observe a truncated file at the final path after checksum validation- Test coverage is thorough:
IpcServerAccessControlTestvalidates context isolation and token enforcement,IpcNamedLockFactoryTestcovers digest collision resistance,PathProcessorSupportTestcovers retry, never-truncate guarantee, and staging fallback
One observation
@since 2.0.22 tags — Same question as the sibling PRs in this security audit batch (#2079–#2086): the maven-resolver-2.0.22 tag already exists in the repository, so these new API additions (IpcServer(SocketFamily, String) constructor at line 232, restrictToOwner in SocketFamily) should use @since 2.0.23 (or whatever the next release version will be).
Overall this is high-quality, well-thought-out security work. Well done!
2.0.22 is already released; new API additions must target 2.0.23. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
cstamas
approved these changes
Aug 31, 2026
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
Fixes 3 findings from the maven-resolver security audit (scan-maven-resolver-20260811):
Root cause: The concurrency layer has spots where its own guarantee is violated: Windows publication is not atomic, and the IPC lock daemon trusts every connection and digests keys ambiguously.
Fix: Atomic rename on Windows with retry, authenticated IPC daemon with SecureRandom tokens and peer credentials, unambiguous length-prefixed key digest.
Test plan
🤖 Generated with Claude Code