cherry-pick: Fix MQTT UDS listener exporting an empty TLS certificate list to a fronting proxy (conflicts → v5.1) - #2005
Merged
Merged
Conversation
… selector (#1999) Cherry-pick of #1999 onto v5.1, with the conflict markers from the automated cherry-pick resolved. The bot cherry-picked all seven commits from the original PR branch, including the ones a later commit narrowed back out, which left nested conflict markers in four files -- two of which (`server/threads/threadServer.js`, `unitTests/resources/auditLog.test.js`) are not part of the merged #1999 at all. This replaces that state with the net merged #1999 change applied to v5.1: `security/keys.ts` and `unitTests/security/keys.test.js` only, matching main commit b992970 exactly. One genuine conflict remained: the hunk carrying the new zero-certificates retry also carried main's `getEffectiveTlsCiphers` cipher-mismatch warning as trailing context. v5.1 predates `getEffectiveTlsCiphers`, so only the retry and the warn-latch reset are taken. Verified on v5.1: `unitTests/security/keys.test.js` 37 passing (including the missing-`system` and missing-`hdb_certificate` race tests, the zero-certs retry/transient-resolve pair, and the table-swap resubscribe regression test), `unitTests/security/**` 365 passing, `npm run build`, `tsc --noEmit`, `oxlint --deny-warnings`, and `prettier --check` all clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
kriszyp
force-pushed
the
cherry-pick/v5.1/pr-1999
branch
from
July 30, 2026 18:40
6b0ef1c to
186e7f2
Compare
kriszyp
added a commit
that referenced
this pull request
Jul 30, 2026
…rver when a plain TCP port was also registered
onSocket() built the secure (TLS) server into the function-scoped
`socketServer` binding, and the UDS metadata-write closure captured that
binding. A caller registering BOTH ports in one server.socket() call —
which MQTT does by default ({ port: 1883, securePort: 8883 }) — then
reached the plain-TCP branch, which reassigned `socketServer` to the
1883 server. Every secure-port metadata write (the boot-time
.ready.then() and every later rebuild's listener fan-out) therefore read
`secureContexts` off the plain TCP server (undefined) and published an
empty `certificates:` list, deterministically, on every worker of every
node with both MQTT ports enabled. A fronting SNI-routing proxy
(Symphony) that selects certificates from that metadata then falls back
to the node certificate for every custom-domain SNI on 8883 — the
customer-visible symptom that survived #1999/#2005.
Give the secure server its own const and capture that in the closure;
`socketServer` remains the branch-shared return value.
The selector, its certificate map, and the publish/retry logic were
always healthy (verified live: in-memory maps fully populated while the
disk yaml stayed empty), which is why the selector-focused fixes and
tests in #1999/#2005/#2008 could not catch this: every existing test
drove createTLSSelector directly with a pseudo-server. The new
regression test goes through server.socket() with both ports — the real
wiring — and asserts the written yaml carries certificates; it fails
against the unfixed code at exactly that assertion.
Root-caused via deterministic local reproduction on v5.1.25 (fresh
default-config install: all <n>-8883.yaml empty, <n>-9926.yaml
populated; instrumented dist showed the 8883 write firing with
secureContexts=undefined; one-line dist patch produced fully populated
metadata on reboot).
Fixes #1998
github-actions Bot
pushed a commit
that referenced
this pull request
Jul 30, 2026
…rver when a plain TCP port was also registered
onSocket() built the secure (TLS) server into the function-scoped
`socketServer` binding, and the UDS metadata-write closure captured that
binding. A caller registering BOTH ports in one server.socket() call —
which MQTT does by default ({ port: 1883, securePort: 8883 }) — then
reached the plain-TCP branch, which reassigned `socketServer` to the
1883 server. Every secure-port metadata write (the boot-time
.ready.then() and every later rebuild's listener fan-out) therefore read
`secureContexts` off the plain TCP server (undefined) and published an
empty `certificates:` list, deterministically, on every worker of every
node with both MQTT ports enabled. A fronting SNI-routing proxy
(Symphony) that selects certificates from that metadata then falls back
to the node certificate for every custom-domain SNI on 8883 — the
customer-visible symptom that survived #1999/#2005.
Give the secure server its own const and capture that in the closure;
`socketServer` remains the branch-shared return value.
The selector, its certificate map, and the publish/retry logic were
always healthy (verified live: in-memory maps fully populated while the
disk yaml stayed empty), which is why the selector-focused fixes and
tests in #1999/#2005/#2008 could not catch this: every existing test
drove createTLSSelector directly with a pseudo-server. The new
regression test goes through server.socket() with both ports — the real
wiring — and asserts the written yaml carries certificates; it fails
against the unfixed code at exactly that assertion.
Root-caused via deterministic local reproduction on v5.1.25 (fresh
default-config install: all <n>-8883.yaml empty, <n>-9926.yaml
populated; instrumented dist showed the 8883 write firing with
secureContexts=undefined; one-line dist patch produced fully populated
metadata on reboot).
Fixes #1998
github-actions Bot
pushed a commit
that referenced
this pull request
Jul 30, 2026
…rver when a plain TCP port was also registered
onSocket() built the secure (TLS) server into the function-scoped
`socketServer` binding, and the UDS metadata-write closure captured that
binding. A caller registering BOTH ports in one server.socket() call —
which MQTT does by default ({ port: 1883, securePort: 8883 }) — then
reached the plain-TCP branch, which reassigned `socketServer` to the
1883 server. Every secure-port metadata write (the boot-time
.ready.then() and every later rebuild's listener fan-out) therefore read
`secureContexts` off the plain TCP server (undefined) and published an
empty `certificates:` list, deterministically, on every worker of every
node with both MQTT ports enabled. A fronting SNI-routing proxy
(Symphony) that selects certificates from that metadata then falls back
to the node certificate for every custom-domain SNI on 8883 — the
customer-visible symptom that survived #1999/#2005.
Give the secure server its own const and capture that in the closure;
`socketServer` remains the branch-shared return value.
The selector, its certificate map, and the publish/retry logic were
always healthy (verified live: in-memory maps fully populated while the
disk yaml stayed empty), which is why the selector-focused fixes and
tests in #1999/#2005/#2008 could not catch this: every existing test
drove createTLSSelector directly with a pseudo-server. The new
regression test goes through server.socket() with both ports — the real
wiring — and asserts the written yaml carries certificates; it fails
against the unfixed code at exactly that assertion.
Root-caused via deterministic local reproduction on v5.1.25 (fresh
default-config install: all <n>-8883.yaml empty, <n>-9926.yaml
populated; instrumented dist showed the 8883 write firing with
secureContexts=undefined; one-line dist patch produced fully populated
metadata on reboot).
Fixes #1998
kriszyp
added a commit
that referenced
this pull request
Jul 30, 2026
…rver when a plain TCP port was also registered
onSocket() built the secure (TLS) server into the function-scoped
`socketServer` binding, and the UDS metadata-write closure captured that
binding. A caller registering BOTH ports in one server.socket() call —
which MQTT does by default ({ port: 1883, securePort: 8883 }) — then
reached the plain-TCP branch, which reassigned `socketServer` to the
1883 server. Every secure-port metadata write (the boot-time
.ready.then() and every later rebuild's listener fan-out) therefore read
`secureContexts` off the plain TCP server (undefined) and published an
empty `certificates:` list, deterministically, on every worker of every
node with both MQTT ports enabled. A fronting SNI-routing proxy
(Symphony) that selects certificates from that metadata then falls back
to the node certificate for every custom-domain SNI on 8883 — the
customer-visible symptom that survived #1999/#2005.
Give the secure server its own const and capture that in the closure;
`socketServer` remains the branch-shared return value.
The selector, its certificate map, and the publish/retry logic were
always healthy (verified live: in-memory maps fully populated while the
disk yaml stayed empty), which is why the selector-focused fixes and
tests in #1999/#2005/#2008 could not catch this: every existing test
drove createTLSSelector directly with a pseudo-server. The new
regression test goes through server.socket() with both ports — the real
wiring — and asserts the written yaml carries certificates; it fails
against the unfixed code at exactly that assertion.
Root-caused via deterministic local reproduction on v5.1.25 (fresh
default-config install: all <n>-8883.yaml empty, <n>-9926.yaml
populated; instrumented dist showed the 8883 write firing with
secureContexts=undefined; one-line dist patch produced fully populated
metadata on reboot).
Fixes #1998
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
1 participant
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 #1999 onto `v5.1` produced conflicts on commit(s): `bcf9cb9055467e7bf9f9c7bb857a2b6d6e300a7c ec80172 051c752 ffb161d a306d0b a7baa43 13ed49f`.
Resolve the conflict markers on branch `cherry-pick/v5.1/pr-1999` and merge this PR.
@claude please review branch `cherry-pick/v5.1/pr-1999` and suggest a patch that resolves the conflict markers (
<<<<<<</=======/>>>>>>>) introduced by cherry-picking PR #1999 onto `v5.1`. Post the suggested patch as a comment here — do not push.