Skip to content

fix(service): bind Linux health sentinel recovery to pidfd - #7133

Merged
lidge-jun merged 7 commits into
devfrom
fix/6987-linux-pidfd
Oct 15, 2026
Merged

lidge-jun merged 7 commits into
devfrom
fix/6987-linux-pidfd

Conversation

@lidge-jun

@lidge-jun lidge-jun commented Oct 15, 2026 •

Copy link
Copy Markdown
Owner

Summary

Closes #6987.

Linux health-sentinel recovery now acquires a pidfd before arming, rechecks the live parent relationship with that fd held, and sends SIGKILL through pidfd_send_signal. A recycled numeric PID cannot redirect recovery. The watching log identifies pidfd mode.

The dependency-free bun:ffi adapter uses libc syscall on Linux x64/arm64, including musl. Only unsupported architecture or startup ENOSYS permits the existing live-parent fallback. Identity, permission, resource, and library failures stand down; a bound signal failure never falls back to a numeric kill. The runner releases its owner on every exit and exception. macOS retains its documented live-parent limitation and Windows retains its existing process-handle path.

Current dev was checked first (base 7640a94, including #7107); no existing open PR matched 6987. Tests cover acquisition races, unsupported/error distinctions, reused/exited targets, repeated recovery, fd zero, and teardown. Structure and public documentation are synchronized.

Verification

  • bun install — passed, no dependency or lockfile changes.
  • bun test tests/service/service-health-sentinel-linux.test.ts tests/service/service-health-sentinel-policy.test.ts tests/service/service-health-sentinel-runner.test.ts tests/service/service-health-sentinel-process.test.ts tests/service/service-health-sentinel-launch.test.ts tests/test-layout.test.ts tests/test-layout-tooling.test.ts — 128 passed, 0 failed; 2 Linux-native tests excluded on the local macOS host.
  • Final changed test rerun: bun test tests/service/service-health-sentinel-linux.test.ts tests/service/service-health-sentinel-process.test.ts — 31 passed, 0 failed; 2 Linux-native tests excluded on macOS.
  • Cleanup mutation control: removing runner owner cleanup caused all 6 lifecycle regression cases to fail; restored implementation passes.
  • bun run typecheck, bun run structure:check, bun run privacy:scan — passed.
  • cd docs-site && bun install --frozen-lockfile && bun run build — passed, 577 pages and 81,200 internal links checked.
  • OCX_TEST_NO_QUEUE=1 bun run test:changed --parallel=1 — passed: 89 tests, 0 failures, 2 Linux-only exclusions on macOS, across 4 files. The original queued invocation was stopped only in this lane after multiple concurrent worktrees accumulated behind the global user lock; this is the runner's supported intentional-overlap setting with one worker, not a test bypass.
  • Full local suite deferred because several implementation lanes share this host and the repository test lock. Focused real-process, policy, owner, launcher, and registry tests passed. Broader Linux/macOS/Windows coverage is delegated to the dispatched ci.yml lane=all run; this PR stays draft pending its results and independent process-safety/security review.
  • Exact head d23328ed3b5b880bd9775654f38b81224af13a09: PR Linux CI, lane=all Linux/macOS/Windows dispatch, service lifecycle — queued at the time of this update. Native Linux syscall/fd tests must actually execute there; local macOS success is not Linux acceptance. Native Linux arm64 and musl are not separately covered by the current hosted matrix.
  • Round 3 (head a3d3c7c80, after merging origin/dev in eea1e2422; conflict in tests/fixtures/test-layout-expected.json resolved by keeping both sides' entries): bun test tests/service/service-health-sentinel-*.test.ts tests/test-layout.test.ts tests/test-layout-tooling.test.ts — 128 passed, 2 Linux-native skips on macOS; OCX_TEST_NO_QUEUE=1 bun run test:changed -- --parallel=1 — 89 passed, 0 failed, 2 Linux-native skips; bun test tests/ci-workflows/file-size-ratchet.test.ts tests/ci-workflows/structure-ssot.test.ts — 60 passed; bun run typecheck, bun run structure:check, bun run privacy:scan — passed.
  • CI evidence still needed for head a3d3c7c80: the PR CI triggered by this push, including the two native Linux pidfd tests actually executing (they skip on macOS). The earlier lane=all run targeted d23328ed3 and does not cover this head; CI dispatch is on hold, so no new lane=all run was started.

Security review (round 3)

Reviewed head 070fcdfdc merged with dev. One defect fixed, everything else holds.

  • Fixed (a3d3c7c80): a throwing pidfd kill left recovery history unsettled. terminate() called api.signal(fd, SIGKILL) without a guard. The runner writes the durable intent entry before calling terminateTarget, so an FFI exception would escape the runner, leave intent unsettled, and kill the sentinel with an unhandled rejection. The numeric process.kill path it replaced caught this case and returned failed. The pidfd handle now returns failed the same way: the entry settles as failed, the sentinel stands down, and it never falls back to a numeric PID. A regression case is in the existing exception test.
  • PID reuse between verification and pidfd_open: closed. The fd is opened first, and process.ppid === targetPid (plus stdin EOF) is checked again while the fd is held, after both pidfd_open and the signal-zero probe. If the parent exits before the open, the kernel reparents the sentinel to an existing reaper, which can never carry the reused PID, so the recheck fails and the fd is closed. I confirmed that Bun 1.4.3 reads process.ppid live: a child saw it change from its parent's PID to 1 after the parent exited.
  • fd lifetime: no leak. Every refusal path closes the fd and then dlcloses the libc handle. A bound fd is released in runHealthSentinel's finally (covered for EOF, refusal, exception, recovered, failed, not_target). Close is idempotent, never retried after EINTR, and blocks later signals. pidfd_open always sets close-on-exec on the fd it returns (kernel behaviour, flags = 0), so the fd cannot leak into a spawned child.
  • FFI correctness. Syscall numbers 434 (pidfd_open) and 424 (pidfd_send_signal) are in the shared syscall table from 403 onward, so they are the same on x86_64 and aarch64; any other architecture is refused before dlopen. Calling libc syscall without its variadic signature is safe on both ABIs: glibc's x86_64 syscall is plain assembly that ignores %al, musl's C version only uses %al to decide whether to spill vector registers, and aarch64 Linux passes variadic integers in the same registers. The musl loader path names (x86_64, aarch64) are correct.
  • errno handling: one residual, safe. errno is read through __errno_location in a second FFI call, so main-thread runtime work in between could in theory overwrite it. That can only change which outcome is chosen, never which PID gets signalled. A spurious ENOSYS falls back to the documented live-ppid check, any other wrong code refuses recovery, and at kill time ESRCH versus another error only picks not_target or failed. I accepted this residual; avoiding it would need inline assembly or a compiled shim.
  • ENOSYS/EPERM/ESRCH. Only ENOSYS (open, or the probe on a kernel older than 5.3) or an unsupported architecture selects the fallback. EPERM fails closed. That includes older container seccomp profiles that answer unknown syscalls with EPERM, so recovery is disabled there and the refusal is logged. ESRCH at kill time is treated as not_target.
  • Fallback never signals an unverified PID. refused blocks bindTarget, so the sentinel never arms. unsupported uses the same live-ppid-then-kill path as macOS, with its documented residual. A bound handle never reaches process.kill, even after close or a failed signal.
  • macOS/Windows unchanged. The Linux module is only imported dynamically on linux. macOS takes the same process.kill path as before. Windows still checks the handle in bindTarget and terminateTarget, and its close() moved into the shared finally, which releases it on the same exits as before and also on exceptions.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

@coderabbitai

coderabbitai Bot commented Oct 15, 2026

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Oct 15, 2026
GOAT and others added 5 commits October 15, 2026 19:30
# Conflicts:
#	tests/fixtures/test-layout-expected.json
# Conflicts:
#	tests/fixtures/test-layout-expected.json
Bun can hold its own pidfd for a spawned child and close it when the child exits, so a count taken after spawning ends one lower. The final leak assertion now uses the count from before the fixture exists.
@lidge-jun

Copy link
Copy Markdown
Owner Author

Maintainer integration into dev (MAINTAINERS.md dev maintainer-integration policy; batch integration authorized by @lidge-jun on 2026-10-15).

  • Head: d7161193db7d98fed87779bea4d2b4becf88cb8d (tree dafe7670baf7c57b911cc0dabdfdd20eb7b18d7a) = this branch merged with origin/dev 49ffccdaab. Integration note: extra commit fixes the new Linux pidfd test fixture: Bun can hold its own pidfd for a spawned child and close it on exit, so the final leak check now compares with a pre-spawn baseline (failed 3/3 on a Linux 7.0 kernel before, 31/31 three times after).
  • Exact-head evidence: GitHub-hosted Actions for this account have not been assigning runners since 2026-10-15 13:30 UTC, so by owner decision the evidence is a full bun run test (Bun 1.4.3 test runner, root and GUI dependencies installed, GUI built) of this exact tree on a maintainer host (Linux x86_64): exit 1, 7 min, 28 failing test(s), each also failing in the same host's baseline run of dev 60bf4c7 (environment-only): Codex shim install readiness > a refused install exits unsuccessfully and preserves its re, Codex shim install readiness > keeps install advisory when the Codex config cannot be read, Codex shim install readiness > the install command surfaces readiness warnings without lea, GUI pairing client > sends one bodyless origin-bound capability and redacts rejected bodie, Node launcher PATH fallback activation > broken bundle reaches validated PATH Bun, Node launcher PATH fallback activation > inspection bundle reaches validated PATH Bun, Node launcher PATH fallback activation > missing bundle reaches validated PATH Bun, bundledBunPath / durableBunPath > PATH-selected process provenance survives durable select, ….
  • Local gates on the same tree: bun run typecheck, file-size ratchet, test-layout and structure:check.
  • Security review: independent agent security review PASS at 070fcdf (pidfd identity recheck, no numeric-PID fallthrough).
  • Hosted CI checks for this head may still be queued; they are not claimed as evidence.

@lidge-jun
lidge-jun marked this pull request as ready for review October 15, 2026 21:19
@lidge-jun
lidge-jun requested a review from Ingwannu as a code owner October 15, 2026 21:19
@lidge-jun
lidge-jun merged commit b43f931 into dev Oct 15, 2026
6 of 7 checks passed
@lidge-jun
lidge-jun deleted the fix/6987-linux-pidfd branch October 15, 2026 21:19
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-15T21:22:22.515442Z d716119 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

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

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant