From 577a0db86236a60f12adc4baabd472bade69944c Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 20 Aug 2026 09:30:39 -0400 Subject: [PATCH] fix(IPC-33): bound the DrainEvents byte budget against the gRPC send cap, not just the pipe frame max The worker budgeted a DrainEvents reply against the negotiated pipe frame maximum less one 64 KiB reserve, but the gateway returns that same MxCommandReply verbatim through Invoke, where MaxSendMessageSize applies. The two limits were coupled only from below (pipe >= gRPC + reserve), so any configuration the validator permitted with a wider gap let the worker pack a reply the pipe carried and the gRPC layer then refused with ResourceExhausted -- after Drain had already removed the events from the worker queue, losing them to both the caller and the event stream. Option (a) from the finding: close the coupling at both ends. - GatewayOptionsValidator adds the upper half of the headroom rule (Worker:MaxMessageBytes <= Protocol:MaxGrpcMessageBytes + 2 x reserve), with a message naming the DrainEvents budget as the reason and the two knobs to move. - WorkerPipeSession.ResolveDrainReplyByteBudget subtracts the reserve twice -- once for the pipe frame, once so the MxCommandReply wrapper clears the gRPC send cap -- keeping the MaxMessageBytes/2 floor that makes small frame maxima usable. The two changes are load-bearing for each other: the validator band is what makes "frame max - 2 x reserve" provably at or below the gRPC cap, and the doubled subtraction is what makes the band sufficient. Docs updated in the same change (GatewayConfiguration.md both option rows plus the headroom prose, WorkerFrameProtocol.md drain budget, gateway.md DrainEvents byte axis). Tests: new GatewayOptionsValidatorTests cases for the upper bound (reject) and both edges of the band (accept); GatewayOptionsTests' override case now moves both message-size knobs together, since the band makes moving one alone illegal. The worker half cannot be built on macOS; verified on windev separately. --- docs/GatewayConfiguration.md | 14 +++-- docs/WorkerFrameProtocol.md | 20 +++++-- gateway.md | 11 ++-- .../Configuration/GatewayOptionsValidator.cs | 23 ++++++++ .../Configuration/WorkerOptions.cs | 9 ++-- .../Configuration/GatewayOptionsTests.cs | 5 ++ .../GatewayOptionsValidatorTests.cs | 52 +++++++++++++++++++ .../Ipc/WorkerPipeSessionTests.cs | 7 +-- .../Ipc/WorkerPipeSession.cs | 36 +++++++++---- 9 files changed, 151 insertions(+), 26 deletions(-) diff --git a/docs/GatewayConfiguration.md b/docs/GatewayConfiguration.md index 5970259..1f34ded 100644 --- a/docs/GatewayConfiguration.md +++ b/docs/GatewayConfiguration.md @@ -123,7 +123,7 @@ launch CWD (SEC-01, SEC-33). | `MxGateway:Worker:ShutdownTimeoutSeconds` | `10` | Grace period for worker shutdown before the gateway treats shutdown as failed and may kill the worker process tree. | | `MxGateway:Worker:HeartbeatIntervalSeconds` | `5` | Worker heartbeat send interval and gateway heartbeat check cadence input. | | `MxGateway:Worker:HeartbeatGraceSeconds` | `15` | Maximum age of the last worker heartbeat before the gateway faults the worker. This must be greater than or equal to `HeartbeatIntervalSeconds`. | -| `MxGateway:Worker:MaxMessageBytes` | `16842752` | Maximum worker IPC frame payload size in bytes. The validator allows values from `1024` through `268435456`, and additionally requires this to be at least `MxGateway:Protocol:MaxGrpcMessageBytes` plus a 65536-byte (64 KiB) envelope-overhead reserve. The default is the 16 MiB public gRPC cap plus that reserve. The gateway conveys the negotiated value to the worker in the handshake (`GatewayHello.max_frame_bytes`), so the worker frames to the same limit rather than a hard-coded default. | +| `MxGateway:Worker:MaxMessageBytes` | `16842752` | Maximum worker IPC frame payload size in bytes. The validator allows values from `1024` through `268435456`, and additionally requires this to be at least `MxGateway:Protocol:MaxGrpcMessageBytes` plus a 65536-byte (64 KiB) envelope-overhead reserve **and at most that cap plus twice the reserve**. The default is the 16 MiB public gRPC cap plus one reserve. The gateway conveys the negotiated value to the worker in the handshake (`GatewayHello.max_frame_bytes`), so the worker frames to the same limit rather than a hard-coded default. The band is narrow by design, so changing either message-size knob alone no longer validates — move `MxGateway:Protocol:MaxGrpcMessageBytes` and this one together, keeping this one one to two reserves above it. | `StartupProbeRetryAttempts`, `StartupProbeRetryDelayMilliseconds`, `PipeConnectAttemptTimeoutMilliseconds`, timeout values, heartbeat values, and @@ -132,7 +132,15 @@ to avoid accidental large allocations from malformed or oversized frames. It must also stay above `MaxGrpcMessageBytes` by the envelope-overhead reserve so a maximally-sized accepted gRPC payload always fits one worker frame once wrapped in a `WorkerEnvelope`; otherwise the oversized outbound write would fault the -whole session. Startup validation fails fast when the headroom is violated. +whole session. The coupling is two-sided: `MaxMessageBytes` may exceed +`MaxGrpcMessageBytes` by at most *twice* the reserve, because the worker budgets +a `DrainEvents` reply against the frame maximum less two reserves and the +gateway then returns that same `MxCommandReply` through `Invoke`, where the gRPC +send cap applies. Without the upper bound a legal configuration (large pipe +maximum, stock gRPC cap) lets the worker pack a reply the pipe carries and the +gRPC layer refuses to send — with the drained events already removed from the +worker queue and therefore lost to both the caller and the event stream. Startup +validation fails fast when either half of the headroom rule is violated. ## Session Options @@ -385,7 +393,7 @@ is **not yet available**. | Option | Default | Description | |--------|---------|-------------| | `MxGateway:Protocol:WorkerProtocolVersion` | `1` | Worker IPC protocol version expected by the gateway and worker. This must match `GatewayContractInfo.WorkerProtocolVersion`. | -| `MxGateway:Protocol:MaxGrpcMessageBytes` | `16777216` | Public gRPC max send and receive message size in bytes. The same default is used by official clients. The validator allows values from `1024` through `268435456`, and requires `MxGateway:Worker:MaxMessageBytes` to exceed this by at least the 64 KiB worker-frame envelope reserve (see the Worker options). | +| `MxGateway:Protocol:MaxGrpcMessageBytes` | `16777216` | Public gRPC max send and receive message size in bytes. The same default is used by official clients. The validator allows values from `1024` through `268435456`, and requires `MxGateway:Worker:MaxMessageBytes` to exceed this by at least the 64 KiB worker-frame envelope reserve and by no more than twice that reserve, so a `DrainEvents` reply budgeted at the pipe maximum still fits this send cap (see the Worker options). | The protocol option is exposed for diagnostics and explicit deployment configuration, not for compatibility negotiation. A mismatch fails validation diff --git a/docs/WorkerFrameProtocol.md b/docs/WorkerFrameProtocol.md index ebff8d7..132a226 100644 --- a/docs/WorkerFrameProtocol.md +++ b/docs/WorkerFrameProtocol.md @@ -43,9 +43,23 @@ guarantees hellos, heartbeats, acks, and faults fit. Every worker-to-gateway frame must serialize within this limit, control replies included, so reply builders truncate to fit rather than emit a frame the writer will reject. `WorkerPipeSession` pre-sizes a `DrainEvents` reply below the -negotiated maximum (less a fixed envelope/reply-wrapper reserve) and reports the -truncation in the reply's `DiagnosticMessage`; the caller contract is to repeat -`DrainEvents` until it returns an empty reply. Should a reply still overshoot, +negotiated maximum and reports the truncation in the reply's +`DiagnosticMessage`; the caller contract is to repeat `DrainEvents` until it +returns an empty reply. The event budget is `max(MaxMessageBytes - 2 x 64 KiB, +MaxMessageBytes / 2)`: the reserve is subtracted **twice** because the reply has +two boundaries to clear, not one. The first reserve covers the +`WorkerEnvelope`/`WorkerCommandReply`/`MxCommandReply` wrapper on the pipe; the +second exists because the gateway returns that same `MxCommandReply` verbatim +through `Invoke`, where the public gRPC send cap applies. `DrainEvents` removes +the events it packs from the worker queue, so a reply the pipe carries and the +gRPC layer then refuses would lose them for good — the gateway's validator +therefore also caps `MxGateway:Worker:MaxMessageBytes` at +`MxGateway:Protocol:MaxGrpcMessageBytes` plus **two** reserves, which is what +makes the doubled subtraction sufficient rather than merely generous. The +`MaxMessageBytes / 2` floor keeps the budget usable at frame maxima small enough +that two reserves would consume them (below 256 KiB), where a collapsed budget +would truncate every drain and never let the drain-until-empty caller finish. +Should a reply still overshoot, `MessageTooLarge` at the reply-write seam is answered with a small `InvalidRequest` reply for that correlation, not with session teardown — no diagnostics command may kill a session. diff --git a/gateway.md b/gateway.md index 77c113f..18fbb9b 100644 --- a/gateway.md +++ b/gateway.md @@ -567,9 +567,14 @@ Optional diagnostics: requests above the shared ceiling `GatewayContractInfo.MaxDrainEventsPerCommand` and the worker clamps to the same value, treating `max_events = 0` as "the default cap". By **bytes**: the worker sizes the reply while draining, against the - negotiated worker-frame maximum, so a byte-heavy queue truncates instead of - overshooting and events that do not fit stay queued. Truncation is reported in the - reply's `DiagnosticMessage`; callers drain iteratively until an empty reply. + negotiated worker-frame maximum less two envelope reserves, so a byte-heavy queue + truncates instead of overshooting and events that do not fit stay queued. The + reserve is subtracted twice because the reply has two boundaries to clear — the + pipe frame, then the public gRPC send cap it is returned through by `Invoke` — and + the validator keeps `Worker:MaxMessageBytes` within two reserves of + `Protocol:MaxGrpcMessageBytes` so that budget always fits both. Truncation is + reported in the reply's `DiagnosticMessage`; callers drain iteratively until an + empty reply. - `ShutdownWorker` Do not compress MXAccess semantics into generic verbs too early. A command enum diff --git a/src/ZB.MOM.WW.MxGateway.Server/Configuration/GatewayOptionsValidator.cs b/src/ZB.MOM.WW.MxGateway.Server/Configuration/GatewayOptionsValidator.cs index e9a7b1f..a0d8496 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Configuration/GatewayOptionsValidator.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Configuration/GatewayOptionsValidator.cs @@ -606,6 +606,15 @@ public sealed class GatewayOptionsValidator : OptionsValidatorBase= {requiredWorkerMax})."); } + + long permittedWorkerMax = + (long)protocol.MaxGrpcMessageBytes + (2L * WorkerFrameProtocolOptions.EnvelopeOverheadReserveBytes); + if (worker.MaxMessageBytes > permittedWorkerMax) + { + builder.Add( + $"MxGateway:Worker:MaxMessageBytes ({worker.MaxMessageBytes}) must be at most " + + $"MxGateway:Protocol:MaxGrpcMessageBytes ({protocol.MaxGrpcMessageBytes}) plus twice the " + + $"{WorkerFrameProtocolOptions.EnvelopeOverheadReserveBytes}-byte worker-frame envelope reserve " + + $"(<= {permittedWorkerMax}). The worker budgets a DrainEvents reply against the negotiated " + + "frame maximum less two reserves; a pipe maximum further above the gRPC cap lets the worker " + + "pack a reply the pipe carries but the gRPC send cap refuses, losing the already-drained " + + "events. Lower MxGateway:Worker:MaxMessageBytes or raise MxGateway:Protocol:MaxGrpcMessageBytes."); + } } private static void AddIfBlank(string? value, string message, ValidationBuilder builder) diff --git a/src/ZB.MOM.WW.MxGateway.Server/Configuration/WorkerOptions.cs b/src/ZB.MOM.WW.MxGateway.Server/Configuration/WorkerOptions.cs index 699a0e2..140f2e9 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Configuration/WorkerOptions.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Configuration/WorkerOptions.cs @@ -58,9 +58,12 @@ public sealed class WorkerOptions /// The maximum worker-frame (pipe) message size in bytes. Must stay at least /// above /// so a maximally-sized accepted gRPC payload - /// still fits one worker frame; the gateway conveys this value to the worker in the - /// handshake (GatewayHello.max_frame_bytes). Default is the 16 MB public gRPC cap - /// plus that reserve. + /// still fits one worker frame, and at most twice that reserve above it so a + /// DrainEvents reply budgeted against this maximum (frame max less two reserves) still + /// fits the gRPC send cap it is returned through — otherwise the worker can pack a reply the + /// pipe carries and the gRPC layer refuses, losing the already-drained events (IPC-33). The + /// gateway conveys this value to the worker in the handshake + /// (GatewayHello.max_frame_bytes). Default is the 16 MB public gRPC cap plus one reserve. /// public int MaxMessageBytes { get; init; } = (16 * 1024 * 1024) + Workers.WorkerFrameProtocolOptions.EnvelopeOverheadReserveBytes; diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Configuration/GatewayOptionsTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Configuration/GatewayOptionsTests.cs index 73594ec..966cec3 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/Configuration/GatewayOptionsTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/Configuration/GatewayOptionsTests.cs @@ -86,6 +86,11 @@ public sealed class GatewayOptionsTests ["MxGateway:Events:QueueCapacity"] = "256", ["MxGateway:Dashboard:Enabled"] = "false", ["MxGateway:Protocol:MaxGrpcMessageBytes"] = "8388608", + // The frame-headroom rule is two-sided (IPC-33): the worker-frame maximum must sit + // between one and two 64 KiB envelope reserves above the gRPC cap, so moving the + // gRPC cap without moving this one no longer validates. Kept at one reserve above, + // the same shape as the shipped default. + ["MxGateway:Worker:MaxMessageBytes"] = "8454144", // Blank-by-default bind password must be supplied so enabled LDAP validates on bind. ["MxGateway:Ldap:ServiceAccountPassword"] = "test-bind-password" }); diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Configuration/GatewayOptionsValidatorTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Configuration/GatewayOptionsValidatorTests.cs index 89e4581..54be27a 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/Configuration/GatewayOptionsValidatorTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/Configuration/GatewayOptionsValidatorTests.cs @@ -1,5 +1,6 @@ using Microsoft.Extensions.Options; using ZB.MOM.WW.MxGateway.Server.Configuration; +using ZB.MOM.WW.MxGateway.Server.Workers; using LdapTransport = ZB.MOM.WW.Auth.Abstractions.Ldap.LdapTransport; namespace ZB.MOM.WW.MxGateway.Tests.Configuration; @@ -1256,6 +1257,57 @@ public sealed class GatewayOptionsValidatorTests Assert.Contains(result.Failures!, f => f.Contains("MaxMessageBytes") && f.Contains("reserve")); } + /// + /// Verifies the headroom rule's upper half (IPC-33): a worker-frame maximum more than two + /// envelope reserves above the gRPC cap is rejected. The worker budgets a DrainEvents reply at + /// the frame maximum less two reserves and the gateway returns that same reply through + /// Invoke, so a wider gap lets the worker pack a reply the pipe carries and the gRPC send + /// cap refuses — with the drained events already removed from the worker queue. + /// + [Fact] + public void Validate_Fails_WhenWorkerFrameMaxExceedsGrpcMaxByMoreThanTwoReserves() + { + const int grpcMax = 16 * 1024 * 1024; + int tooLarge = grpcMax + (2 * WorkerFrameProtocolOptions.EnvelopeOverheadReserveBytes) + 1; + + ValidateOptionsResult result = new GatewayOptionsValidator().Validate( + null, + WithWorkerAndProtocol( + new WorkerOptions { MaxMessageBytes = tooLarge }, + new ProtocolOptions { MaxGrpcMessageBytes = grpcMax })); + + Assert.True(result.Failed); + Assert.Contains( + result.Failures!, + f => f.Contains("MaxMessageBytes", StringComparison.Ordinal) + && f.Contains("at most", StringComparison.Ordinal) + && f.Contains("DrainEvents", StringComparison.Ordinal)); + } + + /// + /// Verifies both edges of the two-sided headroom rule are themselves accepted: exactly one + /// reserve above the gRPC cap (the shipped default's shape) and exactly two reserves above it + /// (the widest legal gap). Pinning the boundaries keeps a future retune of the reserve from + /// silently making the shipped default illegal. + /// + /// Number of envelope reserves between the gRPC cap and the frame maximum. + [Theory] + [InlineData(1)] + [InlineData(2)] + public void Validate_Succeeds_AtBothEdgesOfTheFrameHeadroomBand(int reserveMultiple) + { + const int grpcMax = 16 * 1024 * 1024; + int workerMax = grpcMax + (reserveMultiple * WorkerFrameProtocolOptions.EnvelopeOverheadReserveBytes); + + ValidateOptionsResult result = new GatewayOptionsValidator().Validate( + null, + WithWorkerAndProtocol( + new WorkerOptions { MaxMessageBytes = workerMax }, + new ProtocolOptions { MaxGrpcMessageBytes = grpcMax })); + + Assert.True(result.Succeeded); + } + /// /// Verifies the shipped worker event-queue capacity default passes validation and still matches /// the worker-side MxAccessEventQueue.DefaultCapacity the environment variable falls back diff --git a/src/ZB.MOM.WW.MxGateway.Worker.Tests/Ipc/WorkerPipeSessionTests.cs b/src/ZB.MOM.WW.MxGateway.Worker.Tests/Ipc/WorkerPipeSessionTests.cs index dca5936..09df002 100644 --- a/src/ZB.MOM.WW.MxGateway.Worker.Tests/Ipc/WorkerPipeSessionTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Worker.Tests/Ipc/WorkerPipeSessionTests.cs @@ -692,7 +692,7 @@ public sealed class WorkerPipeSessionTests /// /// Regression for the reserve-boundary budget bug. The gateway validator accepts a Worker /// frame maximum as low as 1024 + 64 KiB, and just above that boundary a naive - /// subtract-then-guard budget collapses to ~1024 bytes — too small to move even one + /// subtract-then-guard budget collapses to nothing — too small to move even one /// byte-heavy event, so every drain reports truncation with the same head blocked and the /// drain-until-empty loop never terminates. The budget is now a floor (never below half the /// negotiated maximum), so a byte-heavy queue drains to empty even at the validator floor. @@ -702,8 +702,9 @@ public sealed class WorkerPipeSessionTests public async Task DrainEvents_AtValidatorFloorFrameMax_MakesProgressAndTerminates() { // The lowest Worker.MaxMessageBytes GatewayOptionsValidator permits: the public gRPC floor - // (1024) plus the 64 KiB envelope-overhead reserve. The naive budget would be exactly 1024 - // here; the floored budget is half of the frame max (~33 KiB). + // (1024) plus the 64 KiB envelope-overhead reserve. The naive budget — frame max less the + // two reserves the DrainEvents budget subtracts (IPC-33) — would be negative here; the + // floored budget is half of the frame max (~33 KiB). const uint validatorFloorFrameMax = 1024 + (64 * 1024); using CancellationTokenSource cancellation = new(TimeSpan.FromSeconds(60)); using PipePair pipePair = await PipePair.CreateAsync(cancellation.Token); diff --git a/src/ZB.MOM.WW.MxGateway.Worker/Ipc/WorkerPipeSession.cs b/src/ZB.MOM.WW.MxGateway.Worker/Ipc/WorkerPipeSession.cs index a92fa0b..53bb0b1 100644 --- a/src/ZB.MOM.WW.MxGateway.Worker/Ipc/WorkerPipeSession.cs +++ b/src/ZB.MOM.WW.MxGateway.Worker/Ipc/WorkerPipeSession.cs @@ -27,7 +27,8 @@ public sealed class WorkerPipeSession // Headroom subtracted from the negotiated frame maximum when budgeting a DrainEvents reply. It // covers the WorkerEnvelope/WorkerCommandReply/MxCommandReply wrapper the drained events are // packed into — the same envelope-overhead reserve rationale docs/WorkerFrameProtocol.md - // records for the frame max itself. + // records for the frame max itself. ResolveDrainReplyByteBudget subtracts it twice: once for the + // pipe frame, once so the reply also clears the public gRPC send cap it is forwarded through. private const int DrainReplyFrameHeadroomBytes = 64 * 1024; private readonly WorkerFrameProtocolOptions _options; @@ -1021,20 +1022,33 @@ public sealed class WorkerPipeSession /// /// Byte budget for the events packed into one DrainEvents reply: the negotiated frame - /// maximum less a fixed reserve for the envelope/reply wrapper, but never below half the - /// negotiated maximum. The lower bound must be a floor, not a step: a bare - /// subtract-then-guard-positive collapses the budget to a handful of bytes just above - /// the reserve (e.g. at the validator-permitted floor MaxMessageBytes = 1024 + 64 KiB the - /// subtraction leaves 1024, too small to move even one byte-heavy event, so every drain - /// truncates and the drain-until-empty caller never terminates). Taking the max with - /// half the negotiated maximum keeps the budget monotonic across the reserve boundary while - /// still leaving the full reserve for the wrapper whenever the frame max is large enough - /// that the reserve is the smaller subtraction — which is every configuration above 128 KiB. + /// maximum less two reserves, but never below half the negotiated maximum. The + /// first reserve covers the envelope/reply wrapper the events are packed into so the reply + /// fits one pipe frame. The second reserve exists because that same + /// MxCommandReply does not stop at the pipe: the gateway returns it verbatim through + /// Invoke, where the public gRPC send cap applies, and the gateway's validator + /// couples the two limits so the pipe maximum never exceeds the gRPC cap by more than two + /// reserves (IPC-33). Subtracting both therefore keeps the packed events at or below the + /// gRPC cap and leaves the wrapper — correlation id, protocol status, kind, the + /// drain_events length prefix and, in the truncated case, the + /// DiagnosticMessage — a full reserve of margin under it. Budgeting against the pipe + /// maximum alone let a legal configuration build a reply the pipe carried and the gRPC layer + /// then refused to send, with the events already dequeued and therefore lost. + /// + /// The lower bound must be a floor, not a step: a bare subtract-then-guard-positive + /// collapses the budget to a handful of bytes (or below zero) once the reserves approach the + /// frame maximum (e.g. at the validator-permitted floor MaxMessageBytes = 1024 + 64 KiB), + /// too small to move even one byte-heavy event, so every drain truncates and the + /// drain-until-empty caller never terminates. Taking the max with half the negotiated + /// maximum keeps the budget monotonic across the reserve boundary while still leaving both + /// reserves for the wrapper whenever the frame max is large enough that the subtraction is + /// the smaller of the two — which is every configuration above 256 KiB. + /// /// private int ResolveDrainReplyByteBudget() { return Math.Max( - _options.MaxMessageBytes - DrainReplyFrameHeadroomBytes, + _options.MaxMessageBytes - (2 * DrainReplyFrameHeadroomBytes), _options.MaxMessageBytes / 2); }