Cherry-picks for 2.14.1-RC.1

[neilalexander]

Includes the following:

• [FIXED] Cluster route compression uses Cluster.MaxPingsOut #8093
• (2.14) [FIXED] Fast batch: parse batch seq as uint64 #8094
• De-flake TestJetStreamClusterConsumerLeak #8076
• [FIXED] Don't mutate caller's hdr in internalSendLoop #8097
• (2.12) [FIXED] Atomic batch: prevent double-pooling of CommittedEntry on cleanup #8098
• NRG: ignore temporary snapshot files on recovery #8101
• [FIXED] Consumer's redelivered state replication and cleanup #8102
• [FIXED] Header remove prefix with prefix in value #8103
• Ensure invalid subject characters cannot be forwarded from MQTT to other connection types #8104
• (2.12) [FIXED] Filestore clear ecache on cipher conversion #8105
• [FIXED] Dedupe map cleanup race on leader change #8106
• Update to Go 1.26.3/1.25.10 #8107
• Allow certificates without subjects #8100
• De-flake TestJetStreamConsumerStepDownResetsPendingAndRdc #8110
• [FIXED] Source consumer reschedule storm on leafnode reconnect #8111
• MQTT: Return errMQTTUnsupportedCharacters for control characters on both pub and sub #8112
• [FIXED] Data races on stream config on JS read paths #8115
• [IMPROVED] Use negative sublist cache resutls in numInterest #8119
• Update dependencies #8124
• Test: rolling restart with disk resets #8121
• Improve subject validation on republish subjects #8127
• [FIXED] Clustered WorkQueue DeliverPolicy check #8126
• [FIXED] Make server shutdown idempotent #8132
• [FIXED] PROXY protocol with required TLS listener #8130
• (2.14) [FIXED] Don't allow purging schedule if not a schedule #8135
• Allow WebSockets on FIPS-enforced builds when built with Go 1.26 or later #8141
• NRG: Invalidate pae cache on WAL truncate and snapshot install #8149
• [FIXED] Mirror last seq mismatch from skipMsgs failure & retry #8152
• NRG: Don't propose removal of unknown peer #8154
• Reduce lock contention on leafnode client  #8139
• Add metrics for messages to and from clients #7851
• [IMPROVED] Keep leafMsgAllowed denied/response path on the read lock #8159
• test: data race in checkStateAndErr #8155
• [FIXED] Pending state leak when reaching max deliveries #8156
• [FIXED] WaitGroup reuse panic when resetting streams and consumers #8158
• De-flake TestFileStoreReadCache #8161
• De-flake TestJetStreamConsumerUpdateRedelivery #8162
• Fix message block corruption on filestore cipher conversion with cache clear #8166

[synadia-claude-reviewer]

Claude finished @neilalexander's task in 6m 52s —— View job

---

PR Review: Cherry-picks for 2.14.1-RC.1

• Gather context and read CLAUDE.md
• Review PR diff for key changes
• Analyze correctness and potential bugs
• Analyze concurrency safety and race conditions
• Analyze security issues
• Analyze performance implications
• Post inline comments on specific issues
• Write overall review summary

---

Overall Assessment

This is a high-quality release candidate backport with 37 cherry-picked fixes. The changes are well-tested and accompanied by focused regression tests. No blocking issues found. A few observations noted as inline comments.

---

Critical Bug Fixes — Verified Correct

Raft / NRG

• pae cache poisoning (raft.go): truncateWAL, resetWAL, and installSnapshot now correctly invalidate the pae (pending append entry) cache. Previously, stale cached entries could survive WAL truncation and be applied from cache rather than re-read from the (possibly shorter) WAL. Four new tests validate each scenario.
• Snapshot install ordering (raft.go:4303): n.pindex/pterm/commit are now set after installSnapshot succeeds, preventing half-committed state on install failure. (inline comment posted)
• Snap file name validation (raft.go:1619): Round-trip format check (fn != fmt.Sprintf(...)) rejects .tmp files and other non-canonical names. (inline comment posted)
• ProposeRemovePeer non-existent peer (raft.go): Peer existence is now validated before proposing a removal, and the forwarded-remove path has the same check, preventing spurious membership change proposals.

JetStream Consumer

• WaitGroup reuse panic (consumer.go, stream.go): monitorMu serializes monitorWg.Add against monitorWg.Wait, eliminating the panic that could occur when a monitor goroutine stopped and restarted in a tight window. The stopMonitoring/startMonitorWg API properly guards all call sites.
• Pending state leak at MaxDeliver (consumer.go): hasMaxDeliveries now passes dc+1 to updateDelivered (the pre-existing code passed dc, under-counting the final delivery) and calls moveAckFloor, allowing the ack floor to advance past exhausted messages. (inline comment on dc+1 posted)
• rdc not cleared on leader stepdown (consumer.go): setLeader(false) now sets o.rdc = nil, preventing stale delivery-count state from leaking across leader transitions.
• Purge not replicated to followers (consumer.go): o.purge() now calls updateAcks for each purged pending/redelivered entry, so follower stores also drop the corresponding state.Pending/state.Redelivered entries.
• decStreamPending rdc ghost (consumer.go): When a message that is in rdc (past MaxDeliver) is deleted from the stream, the leader now removes it from rdc and proposes an ack update so follower stores stay clean.
• UpdateAcks always clears Redelivered (filestore.go, memstore.go): delete(state.Redelivered, sseq) was moved before the early-return guards, so it runs even when dseq <= AckFloor.Consumer or when the entry is absent from Pending.
• WQ consumer DeliverPolicy check (consumer.go, jetstream_cluster.go): The DeliverPolicy != DeliverAll check is now applied before the numLimitableConsumers scan, and is also added to the clustered creation path, preventing orphan consumer assignments.

Lock Ordering

• clMu → ddMu ordering (jetstream_cluster.go): processStreamLeaderChange now acquires clMu before ddMu, consistent with the updated locksordering.txt. This ensures inflight proposals (which hold clMu during diff.commit / ddMu insertion) complete before the dedupe cleanup runs, eliminating a potential missed-placeholder race. The TestJetStreamClusterStreamLeaderChangeDedupeCleanupRace test validates the specific scenario.

Mirror / Source

• Nil deref in processInboundMirrorMsg (stream.go): mset.mirror is now checked for nil before use after reacquiring the mutex, closing a race where a concurrent cancelSourceInfo could set it nil.
• retryDisconnectedSyncConsumers scope bug (stream.go): The sip (setup in progress) flag previously triggered retry for all sources, causing healthy sources in other domains to have their subscriptions cancelled during an unrelated leaf reconnect. It now prevents retry for the in-progress source only.
• skipMsgs error propagation (stream.go): Raft proposal failures in skipMsgs are now returned to callers, which trigger retryMirrorConsumer instead of silently advancing mirror state. (inline comment on partial-proposal risk posted)

Filestore

• XOR'd cache resurrection (filestore.go): mb.ecache.Set(nil) is now called in convertCipher/convertToEncrypted after clearing mb.cache, preventing a stale (XOR-encrypted) cache pointer from being re-activated by setupWriteCache.
• Cache expiry timer imprecision (filestore.go): tryExpireCacheLocked now adds ats.TickInterval to the staleness threshold, accounting for the fact that ats.AccessTime() can understate recency by up to one tick.

Other Correctness

• removeHeaderIfPrefixPresent prefix-in-value false positive (client.go): When a prefix string appears inside a header value (not a header name), the scanner now correctly continues rather than aborting. The bug could cause legitimate Nats-Expected-* or Nats-Msg-Id headers to not be stripped.
• Sublist negative cache hit (sublist.go): HasInterest/NumInterest now correctly count cache hits for subjects with zero matches (ok vs matched). Previously, a negative result was never credited to the cache.
• route.go wrong ping-max field: opts.MaxPingsOut → opts.Cluster.MaxPingsOut in createRoute. The wrong field meant cluster ping-max config had no effect during compression negotiation.
• Shutdown concurrent call safety (server.go): s.shutdown.CompareAndSwap(false, true) at entry prevents multiple concurrent Shutdown() calls from all executing shutdownEventing() and other teardown. Also fixes a nil-deref in shutdownEventing when s.sys is uninitialized.

---

Security Fixes

• MQTT whitespace injection (mqtt.go): \t, \n, \r, \f are now rejected in MQTT topics in addition to space. These characters can split NATS wire protocol control lines when MQTT subjects are forwarded to leaf nodes.
• Republish invalid subject suppression (stream.go): If a stream's RePublish transform produces an invalid NATS subject (e.g., one containing \r\n for injection), the publish is silently suppressed with a rate-limited warning rather than being forwarded. TestJetStreamStreamRepublishSuppressesInvalidSubject demonstrates the injection scenario.
• TLS cert empty Subject + SAN (auth.go): checkClientTLSCertSubject now checks hasSANs first, so a certificate with no Subject DN but a DNS SAN is correctly accepted by verify_and_map. Previously such certs were rejected despite having a valid SAN identity.
• Proxy protocol + required TLS (server.go): PROXY headers are now read before the TLS handshake even when TLS is required, matching the PROXY protocol spec. The previous !tlsRequired guard broke setups that used both features together (issue PROXY Protocol v1/v2 fails with TLS - "first record does not look like a TLS handshake" #7571).
• FIPS WebSocket handling (websocket_go125.go, websocket_go126.go): WebSocket is disabled under FIPS mode on Go ≤ 1.25 (SHA-1 would panic); on Go ≥ 1.26 it is re-enabled using fips140.WithoutEnforcement since SHA-1 is used only for the WebSocket handshake key, not for cryptographic security.

---

Performance / Lock Contention

• client.mu promoted to sync.RWMutex: Allows concurrent read-only permission checks (canSubscribeInternal, leafReceiveAllowed, leafSendAllowed) to proceed in parallel. Write lock is taken only when canSubscribe needs to initialize mperms via loadMsgDenyFilter. All leaf-node hot paths now use read locks for the common case.
• updateLeafNodesEx random start: Iteration over acc.lleafs now begins at a random index, reducing lock convoys when multiple goroutines update leaf-node subscriptions concurrently.
• Subscribe sublists use NewSublistNoCache(): Per-client subscribe allow/deny sublists avoid the global sublist cache, reducing lock contention in canSubscribeInternal (which is now called under read lock).
• Stream config reads under cfgMu.RLock (jetstream.go, jetstream_api.go, jetstream_cluster.go): Several hot paths (reservedStorage, countStreams, tieredReservation, jsMsgDeleteRequest, jsStreamPurgeRequest) now hold cfgMu.RLock when reading mset.cfg, eliminating data races with concurrent stream updates (TestJetStreamStreamConfigConcurrentReadWrite).
• Stream name snapshot before sort (jetstream_api.go): jsStreamNamesRequest/jsStreamListRequest now snapshot all names under a single cfgMu.RLock pass before sorting, avoiding repeated per-stream lock acquisitions during the sort comparator.

---

One Observation (not blocking)

The skipMsgs partial-proposal scenario (inline comment on stream.go:3358) is a pre-existing structural limitation now surfaced by error propagation. The retry path in retryMirrorConsumer should recover correctly, but a brief code comment noting that the first batch may have already been proposed before the error is returned would help future readers. Not a new regression.

[synadia-claude-reviewer]

In the ProposeMulti loop when batching > 10,000 entries, if the first call succeeds but a subsequent ProposeMulti (or the trailing one after the loop) returns an error, the function returns that error — but some entries have already been proposed to Raft. The caller in processInboundMirrorMsg will call retryMirrorConsumer(), which should recover, but this could leave a short window where the Raft log has partial skip entries. This is a pre-existing structural limitation now made visible by error propagation; worth a comment noting the partial-proposal possibility.

[synadia-claude-reviewer]

The dc+1 here is correct: dc = o.deliveryCount(seq) returns the count before this delivery attempt, so dc+1 is the actual post-delivery count. The companion fix in getNextMsg (below) does NOT add 1 because dc is incremented before that site (dc++ is called earlier in getNextMsg). This asymmetry is intentional but worth a brief code comment to prevent future confusion.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3d75dc3b36

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Avoid replicating redelivery cleanup as an ack

In clustered consumers (o.node != nil), this updateAcks(0, sseq, ...) is encoded as a normal updateAcksOp; followers apply it via processReplicatedAck, which always calls mset.ackMsg(o, sseq) for non-LimitsPolicy retention after store.UpdateAcks. So when a deleted stream message only needs its stale redelivery entry cleared, Interest/WorkQueue followers can treat it as a real ack and remove/advance retention state (and AckAll/FlowControl may ack a whole range from the stored ack floor). The dseq == 0 sentinel needs a clustered apply path that only removes Redelivered, or processReplicatedAck must skip retention acking for that sentinel.

Useful? React with 👍 / 👎.

[synadia-claude-reviewer]

There is a TOCTOU window between c.mu.RUnlock() and c.mu.Lock() (a few lines below) where c.replies[replySubject] could be deleted. However, this is safe by design: if the entry disappears between the two lock acquisitions, responseAllowed() under the write lock will return false, which is the correct conservative answer. No security or liveness issue arises from this gap.

[synadia-claude-reviewer]

Good fix. pm.hdr[:len(pm.hdr):len(pm.hdr)] limits the slice capacity to its current length before append, preventing append from writing the body bytes into the caller's backing array when there is spare capacity. Without this, callers with cap(hdr) > len(hdr) (which is the common case when building headers with append/genHeader) would see their buffer silently mutated after sendInternalAccountMsgWithReply returned.

[synadia-claude-reviewer]

This round-trip validation cleanly rejects any filename that Sscanf parsed but that doesn't reconstruct to the canonical form — e.g. files with a .tmp suffix, leading zeros, or other non-standard characters. Elegant approach.

[synadia-claude-reviewer]

Critical ordering fix: n.pindex/pterm/commit are now set only after installSnapshot succeeds. Previously, setting them before the call and then returning early on failure left the node with inconsistent committed state (the node believed it was at ae.pindex but the WAL hadn't been updated). The snapshot fields now reference ae.pindex/ae.pterm directly, which is equivalent but avoids the pre-mutation issue.

[MauriceVanVeen]

LGTM

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: de471aef23

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Flush redelivery-only ack state before returning

When UpdateAcks is used with the new dseq == 0 sentinel from decStreamPending, this deletes state.Redelivered and then immediately returns because 0 <= AckFloor.Consumer, so the file-backed consumer never calls kickFlusher(). In an R1/file-store consumer where a redelivered message is later deleted or evicted while not pending, the in-memory cleanup works until restart, but the stale Redelivered entry is not persisted and is loaded again from disk. Track whether this delete changed state and schedule a flush before either early return.

Useful? React with 👍 / 👎.