Skip to content

feat(native): Windows no-replace publication of the socket secret - #4310

Open
code-yeongyu wants to merge 5 commits into
mainfrom
feat/4199-native-no-replace-publish
Open

code-yeongyu wants to merge 5 commits into
mainfrom
feat/4199-native-no-replace-publish

Conversation

@code-yeongyu

@code-yeongyu code-yeongyu commented Oct 26, 2026 •

Copy link
Copy Markdown
Owner

Working...
Part of sisyphuslabs/omo-desktop-app#4199 (senpi PR S5, native half: the Windows primitive of design 4.3 and 4.5). Stacked on #4302; review only the top commit until #4302 merges, then this PR is retargeted to main.

Summary

crates/senpi-socket-secret gains publish_no_replace (N-API publishNoReplace(stagingPath, finalPath, bytes)): first publication of a public socket secret on Windows without ever writing, copying onto or truncating the final name. No TypeScript caller yet; the TypeScript creation path that uses this primitive (and link() on POSIX) is the next PR.

Behaviour (design 4.3 steps 3-7, 4.5 Windows row)

  • Staging: CreateFileW(CREATE_NEW) in the caller's same directory, with a SECURITY_ATTRIBUTES whose descriptor is D:P(A;;FA;;;<user SID>)(A;;FA;;;SY)(A;;FA;;;BA): a protected DACL (no inherited ACE) set before any byte is written; handle not inheritable.
  • WriteFile until every byte is written (a zero-byte write is an error), FlushFileBuffers, close the writing handle.
  • Reopen for DELETE | SYNCHRONIZE and SetFileInformationByHandle(FileRenameInfo) with ReplaceIfExists = FALSE and a full destination path.
  • ERROR_ALREADY_EXISTS/ERROR_FILE_EXISTS is a collision: returns collision, the caller reads and validates the winner. Any other failure returns the failed step and its Windows error; never a fallback.
  • On collision or error the staging file is deleted through its own handle (FileDispositionInfo), never by path; after a successful rename nothing is unlinked.

Tests

Native receipts in tests/native_publish.rs (Windows only; ci.yml socket-secret-native-windows already runs cargo test -p senpi-socket-secret, and each prints a RECEIPT_PUBLISH line):

  • first publication to an absent name; only the final file remains (design test 1);
  • collision with an existing winner keeps the winner's bytes and deletes only this attempt's staging;
  • collision while a reader holds the winner open is still a collision, not an access error;
  • 16 threads released by one barrier: exactly one published, 15 collision, the final bytes equal the winner's candidate, no staging left (design test 2);
  • a pre-existing staging name fails without creating or touching the final name;
  • the published file's ACL has no inherited ACE and no Everyone, Users or Authenticated Users entry (icacls).

RED: the primitive is new; its "can fail" proofs are the collision and race assertions, which a final-path CREATE_ALWAYS write or a ReplaceIfExists=TRUE rename would fail.

Gates

cargo fmt -p senpi-socket-secret -- --check rc=0; cargo test -p senpi-socket-secret --locked rc=0 on macOS (the Windows receipts compile out); cargo clippy -p senpi-socket-secret --locked --tests -- -D warnings rc=0 for both x86_64-pc-windows-msvc and aarch64-pc-windows-msvc. The Windows run of the receipts is this PR's CI job.

Not covered

Power-loss durability of the rename (design 4.5 separates it from atomic visibility) and UNC/SMB targets; this primitive is only used on local stores, which the locality classifier of #4302 decides.

Gates (exact head b75492c)

Gated push at this head: bun run check rc=0; cargo fmt -p senpi-socket-secret -- --check rc=0; cargo test -p senpi-socket-secret --locked rc=0; cargo clippy -p senpi-socket-secret --locked --tests --target x86_64-pc-windows-msvc -- -D warnings rc=0; node scripts/check-pr-changelog.mjs --base origin/feat/4199-windows-locality-adapter --labels no-changelog rc=0. Also run locally before the push: the same clippy on aarch64-pc-windows-msvc rc=0.

Review fold r1 (review-4310-r1, FIX_FIRST -> head 91397f5)

  • M1, M2, L6, one handle: the staging file is created once with CREATE_NEW, the owner-only DACL, GENERIC_WRITE | DELETE and share mode FILE_SHARE_READ. That handle writes the file, flushes it and renames it with FileRenameInfo and ReplaceIfExists = FALSE. On any outcome except Published (write, flush or rename error, or a collision), the same handle deletes the file through FileDispositionInfo. Nothing is reopened or deleted by path (remove_staging and the reopen are gone). So nothing can swap the file before the rename, and no flushed candidate is left behind on an error.
    • Design check for L6: Revision 8 section 4.3 step 4 says to write, flush and close the writing handle, and never to write the final path. Section 4.5 says the source is "a fully written and flushed same-directory staging file". Neither section requires a reopen or gives a reason to close before the rename. Renaming through the writer keeps both properties (flush completes before the rename, and the final path is never written). It also removes the gap the review found. FILE_SHARE_READ lets readers of the final name in between the rename and this handle's close.
  • L3: a failed delete disposition is now reported on stderr with its Windows error code only, never bytes. The result is unchanged, so a collision is still Collision.
  • L4: the DACL receipt reads the published file's descriptor with GetNamedSecurityInfoW and converts it to SDDL. It asserts the protected flag and exactly three ACEs: A;;FA;;;<user SID from whoami /user>, A;;FA;;;SY and A;;FA;;;BA. No localized names are involved, and whoami must succeed.
  • L5: a staging name that already exists must fail as Os { step: "create_staging", code: 80 }.
  • Nit: the WriteFile length now uses u32::try_from(...).unwrap_or(u32::MAX) and the loop continues.
  • Local checks (macOS, at this head): cargo fmt --check passed, cargo test -p senpi-socket-secret --locked passed 10, and cargo clippy --tests -D warnings passed for x86_64- and aarch64-pc-windows-msvc. That compiles the Windows tests, but there is no Windows host or VM, so the real-Windows receipts run only in socket-secret-native-windows CI. Actions on this account was reported stalled at fold time.
  • The "Gates (exact head b75492c)" section above refers to the pre-fold head.

Review r1 LOW 3 fold (lead ruling, head 0afbce4)

A staging file left behind by a failed FileDispositionInfo delete used to be one stderr line. It is now part of the result.

  • publish_no_replace returns Publication { result, staging_left_behind: Option<u32> }; the u32 is the GetLastError of the failed disposition. The stderr line is gone.
  • N-API publishNoReplace resolves { outcome: "published" | "collision" | "failed", failure?: { step, code }, stagingLeftBehind?: { path, windowsError } }. An OS failure now resolves as failed instead of throwing, so the leftover survives it too; only an unsupported platform throws. The bytes are never in the result.
  • RED: the four new lib.rs unit tests did not compile on 02e44f8 (Publication and publish_result do not exist). A Windows-only unit test in publish.rs injects a failing disposition (|_| Err(5)) and asserts Collision with staging_left_behind == Some(5), the staging file still present and the winner unchanged; it runs in socket-secret-native-windows. The existing receipts now also assert that no staging file was reported left behind.
  • Gates (gated-push, rc=0 each): cargo fmt --check, cargo test -p senpi-socket-secret --locked (15 passed on Linux), cargo clippy --tests -D warnings for x86_64- and aarch64-pc-windows-msvc, check-pr-changelog. The real-Windows run of the receipts is this head's CI job.
  • The TypeScript side (publishSocketSecretNoReplace, typed stagingLeftBehind with an action naming the path) is on build(native): ship the Windows socket-secret bridge with a loader that has no fallback #4312, where the loader lives.

@code-yeongyu code-yeongyu added the no-changelog No changelog required label Oct 26, 2026
@code-yeongyu
code-yeongyu marked this pull request as ready for review October 26, 2026 01:25
@code-yeongyu
code-yeongyu force-pushed the feat/4199-native-no-replace-publish branch from b75492c to 39262f7 Compare October 26, 2026 04:26
@code-yeongyu code-yeongyu added review-queued Queued for batched Opus review and removed review-queued Queued for batched Opus review labels Oct 26, 2026
@code-yeongyu
code-yeongyu force-pushed the feat/4199-native-no-replace-publish branch from 91397f5 to 6af9812 Compare October 26, 2026 07:43
Adds publish_no_replace to crates/senpi-socket-secret: a CREATE_NEW same-directory staging file with a protected owner-only DACL, written in full and flushed, then renamed with FileRenameInfo ReplaceIfExists=FALSE. A destination collision returns Collision for the caller to read the winner; every other failure keeps its own step and Windows error, and staging is deleted through its own handle. Native receipts cover first publication, collision (also with an open reader), a 16-way race with exactly one winner, a pre-existing staging name, and the DACL.

Part of sisyphuslabs/omo-desktop-app#4199
…ndle

Review fold for senpi#4310 (review-4310-r1):
- M1/M2/L6: the staging file is opened once with GENERIC_WRITE | DELETE
  (share read only), written, flushed and renamed through that handle; on
  any outcome but Published the same handle deletes it. Nothing is reopened
  or deleted by path, so no swap window before the rename and no flushed
  candidate left behind when a reopen failed.
- L3: a failed delete disposition is reported on stderr (code only).
- L4: the DACL receipt reads the descriptor with GetNamedSecurityInfoW and
  asserts a protected DACL with exactly owner, SYSTEM and Administrators.
- L5: a pre-existing staging name must fail as create_staging/80.

Part of sisyphuslabs/omo-desktop-app#4199
… result

A failed FileDispositionInfo delete used to be one stderr line. publish_no_replace
now returns Publication { result, staging_left_behind }, and the N-API
publishNoReplace resolves { outcome, failure, stagingLeftBehind { path,
windowsError } } so the caller can remove the leftover file. OS failures resolve
as outcome "failed" (step and code) so the leftover survives them too; only an
unsupported platform throws. The bytes are never part of the result.

Review r1 LOW 3 (lead ruling). Part of sisyphuslabs/omo-desktop-app#4199.
The DACL receipt failed on the windows-latest runner (run 40301410410): its user is
the built-in Administrator (RID 500), which ConvertSecurityDescriptorToString-
SecurityDescriptorW prints as the alias LA, while the test expected the literal
SID. The published DACL was correct (D:P(A;;FA;;;LA)(A;;FA;;;SY)(A;;FA;;;BA)).
The expected owner ACE now goes through the same descriptor round trip, so a
well-known SID compares as its alias and any other SID stays literal.
Base automatically changed from feat/4199-windows-locality-adapter to main October 27, 2026 02:48
…eplace-publish

# Conflicts:
#	crates/senpi-socket-secret/src/lib.rs

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-changelog No changelog required

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant