Repository navigation
cherry-pick: Restrict the UDS mirror sockets directory to its owner and publish mirror metadata atomically (conflicts → v5.3) - #3122
Merged
Conversation
The TLS UDS mirrors skip TLS and trust the PROXY v2 identity a fronting proxy passes, so the sockets directory is their only access gate. It was created with the default mode (0755 under a typical umask). Every creator now forces it to 0700 before binding (http.ts, threadServer.js Bun and raw TLS paths), tightening a pre-existing directory with a logged warning, and skips that mirror when the directory cannot be secured. writeUdsMetadata publishes the sibling yaml through atomicWriteFile, so a reader never sees a truncated file while overlapping workers or cert reloads rewrite it. Dispatch-Task: harper-uds-mirror-dir-hardening Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J7YjJTe8TxQEVdys3cRbrB
…rkers loudly ensureSocketsDirectory now computes the sockets path and returns it, or undefined when a shared worker cannot secure it. The path join no longer runs for servers without a UDS mirror, and the three creators share one guard. An isolated application's worker is served only through its mirror, so its failure throws instead of starting without a listener. Dispatch-Task: harper-uds-mirror-dir-hardening Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J7YjJTe8TxQEVdys3cRbrB
An isolated worker's directory failure was thrown after the server was cached, so later registrations reused a listener with no mirror and the worker still reported started. The check now runs before the cache assignment (http.ts) and before any registration (raw TLS onSocket), so a failure leaves nothing registered and the component load reports it. Dispatch-Task: harper-uds-mirror-dir-hardening Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J7YjJTe8TxQEVdys3cRbrB
A mirror that cannot be secured is skipped and logged. An isolated application is reachable only through its mirror, so its worker now fails startup when the worker registered none, instead of reporting ready with no ingress. The directory helper no longer throws, which leaves server registration unchanged. Dispatch-Task: harper-uds-mirror-dir-hardening Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J7YjJTe8TxQEVdys3cRbrB
Dispatch-Task: harper-uds-mirror-dir-hardening Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J7YjJTe8TxQEVdys3cRbrB
A mirror whose bind failed stays registered, so the readiness check must skip paths in failedUdsPaths or an isolated worker reports ready with nothing listening. Dispatch-Task: harper-uds-mirror-dir-hardening Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J7YjJTe8TxQEVdys3cRbrB
Dispatch-Task: harper-uds-mirror-dir-hardening Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J7YjJTe8TxQEVdys3cRbrB
Dispatch-Task: harper-uds-mirror-dir-hardening Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J7YjJTe8TxQEVdys3cRbrB
Other suites in the same mocha process re-point the base path, so a path captured at module load names a directory the helper never touches. Dispatch-Task: harper-uds-mirror-dir-hardening Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J7YjJTe8TxQEVdys3cRbrB
Dispatch-Task: harper-uds-mirror-dir-hardening Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J7YjJTe8TxQEVdys3cRbrB
Dispatch-Task: harper-uds-mirror-dir-hardening Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J7YjJTe8TxQEVdys3cRbrB
…tory suite Dispatch-Task: harper-uds-mirror-dir-hardening Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J7YjJTe8TxQEVdys3cRbrB
Review found two related gaps: afterEach still ran after a failed beforeEach containment assert (SOCKETS_DIR was assigned before the assert), and the assert itself used startsWith, which treats a sibling directory like <root>-evil as contained. Both could let test cleanup rm -rf a directory outside the sandboxed test root. isWithinTestRoot() resolves and checks a real path boundary, and SOCKETS_DIR is only assigned after it passes. Dispatch-Task: pr-maint-6991910019c44bbcd3ab780c19e52cd5 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Review found both wrote the rationale for a reviewer rather than the invariant for the next reader, and one overstated the failed-assertion case (the previous test's validated target stays set, not nothing). Dispatch-Task: pr-maint-6991910019c44bbcd3ab780c19e52cd5 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ardening Resolves the conflict markers cherry-pick-patch.yml committed for #3019. Both UDS mirror sites (getHTTPServer and onSocket's raw TLS mirror) take #3019's ensureSocketsDirectory()-gated shape. The shouldBindListenerHere() term from main's side is dropped: it arrived with #3035 (dedicated worker pools), which v5.3 does not have, so v5.3's pre-image condition is kept as-is. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WTjjN5aimb5AwUhHUrm2Vg Dispatch-Task: cherry-resolve-kriszyp_harper_3019-bcc75138
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.
Cherry-pick of PR #3019 onto `v5.3` produced conflicts on commit(s): `396ab5ffcf7f78578407e9c2054e31a50d0440f5 7b26d4c 5970700 cd6eff0`.
Resolve the conflict markers on branch `cherry-pick/v5.3/pr-3019` and merge this PR.
@claude please review branch `cherry-pick/v5.3/pr-3019` and suggest a patch that resolves the conflict markers (
<<<<<<</=======/>>>>>>>) introduced by cherry-picking PR #3019 onto `v5.3`. Post the suggested patch as a comment here — do not push.Conflict resolution
Resolved in b3f4a9cfd (new commit on top of the action's history; nothing rewritten).
getHTTPServer(server/http.ts) and inonSocket's raw-TLS mirror (server/threads/threadServer.js).ensureSocketsDirectory()-gated shape, with v5.3's own condition. Main's side also carriedshouldBindListenerHere(...), which came from Run replication on dedicated worker threads that load no application code (#3035) and does not exist on v5.3. It is dropped, so v5.3's listener binding is unchanged.Verification
mocha unitTests/server/**: 1182 passing, 0 failing, including all 10 newudsMirrorDirectorytests.integrationTests/server/uds-mirror-overlapping-restart.test.tspasses, including the 0700 assertions after startup and after restart.withNodeAdapter×4 andinstall_node_modules×3. These are the same 7 failures as v5.3's own push run at f884898 (run 37829350392).logGenerationCoordinator.test.js:172. The test computesheld.ino + 1, and a Windows file ID above 2^53 rounds that back toino. Main fixed this in Compare log-file identities as BigInt, so Windows file IDs past 2^53 stop matching neighbouring files (#3036), which has no milestone and is not on v5.3. The same job passed in the dispatched run at the same SHA.🤖 Resolution by Claude Opus 5.5 (Claude Code); posted via @kriszyp.
🤖 Generated with Claude Code
https://claude.ai/code/session_01WTjjN5aimb5AwUhHUrm2Vg
Related PRs: #3019 overlaps, #3035 overlaps, 18 others independent
Review-Coverage: authored=claude; ran=gemini,codex,cursor-composer; adjudicated=domain; blocked=cursor-grok(failed); declined=cursor-kimi,cursor-muse; rounds=1; full=1 @ b3f4a9c
Review-Attention: study ~14m (hot: http.ts, threadServer.js; decisions: owner-only-0700, fail-soft-pool; raised: degraded review) @ b3f4a9c