Repository navigation
fix(rpc): publish the first socket secret without replacing an existing one - #4369
Draft
code-yeongyu wants to merge 4 commits into
Draft
code-yeongyu wants to merge 4 commits into
code-yeongyu wants to merge 4 commits into
Conversation
…ng one On POSIX, createSocketSecret writes its candidate to a same-directory staging file (O_CREAT|O_EXCL|O_NOFOLLOW, mode 0600 set on the descriptor, full write, fsync) and publishes it with link(), which never replaces an existing entry. A collision returns the winner; an invalid winner is refused, not replaced. Only this attempt's staging file is removed. ensureSocketSecret still repairs a wrong-sized file in place until serialized repair lands, and Windows keeps the in-place writer until the native bridge is wired. Part of sisyphuslabs/omo-desktop-app#4199 (design 4.3, tests 1 and 2).
Review LOW on senpi#4369: a failed close after a successful write and fsync left the staging file behind, and a failed close in the error branch skipped its unlink. Close now always runs; the first failure is reported and this attempt's staging file is removed. A test injects a close failure on the staging handle.
This branch has not been deployed
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.
Part of a desktop issue (tracked internally) (senpi PR S5, TypeScript half: design 4.3 first publication). Stacked on #4294 (S2, the regular-file read); review only the top commit until #4294 merges, then this PR is retargeted to
main.Summary
createSocketSecrettruncated and rewrote the final path. Two creators each returned their own key while the file kept the last one written, and a reader could see a partial file. On POSIX it now publishes through a private staging file andlink(), which never replaces an existing entry: the first complete secret wins and every later or concurrent creator returns it.Behaviour
.<name>.stage-<pid>-<random>in the secret's own directory, openedO_CREAT | O_EXCL | O_NOFOLLOWwith mode0600, which is set again on the descriptor so the umask cannot widen it. The candidate is written in full (a zero-byte write is an error) and fsynced before publication.link(staging, final).EEXISTis a collision: the winner is read back through the S2 regular-file read and returned when it is 32 bytes. An invalid winner throwsRPC socket secret is invalid and was not replacedand stays as it is. Every other error propagates; there is no fallback to a direct write.finallyremoves only this attempt's staging file, never the final entry.0700.ensureSocketSecretstill repairs a wrong-sized file in place (serialized repair is the next PR, S6), and Windows keeps the in-place writer until the native no-replace bridge (feat(native): Windows no-replace publication of the socket secret #4310, loaded by build(native): ship the Windows socket-secret bridge with a loader that has no fallback #4312) is wired.socket-transport.tsrather than the sibling module the design names: a new module put the supervisor's startup graph at 56 against its ceiling of 55 (test/supervisor-import-graph.test.ts).Tests
test/suite/regressions/desktop-4199-socket-secret-first-publisher.test.ts(POSIX):0600, a new directory0700, and no staging file remains;ensureSocketSecretstill repairs a wrong-sized secret.RED on #4294's head ed55b3a: 4 of 5 failed (the second creator returned a new key; concurrent creators disagreed; the new directory was
0775; the invalid winner was replaced). The control passed.GREEN: the new file, the S2 entry-type regression,
test/rpc-socket-transport.test.ts,test/suite/regressions/1370-rpc-internal-socket-mkdir.test.ts,test/supervisor-import-graph.test.tsandtest/rpc-host-lifecycle.test.tspass. On this Linux host the 1370 and lifecycle suites needumask 022(with umask 002 the supervisor refuses the group-writable test agent directory withunsafe_key_record) and abun run buildfirst.Not covered
Windows creation and the design's test 2 at the staged-write boundary on Windows come with the bridge wiring. Power-loss durability is limited to the fsyncs above (design 4.5).