diff --git a/docs/reviews/2026-10-26-codeql-triage.md b/docs/reviews/2026-10-26-codeql-triage.md index cbea53e4..99761172 100644 --- a/docs/reviews/2026-10-26-codeql-triage.md +++ b/docs/reviews/2026-10-26-codeql-triage.md @@ -12,17 +12,19 @@ Snapshot: open CodeQL alerts on `main` at `a52a3a470cbbcd6c90fede0dd026f4e74ede8 | [#621](https://github.com/dborgards/CanKit.Pro/security/code-scanning/621) | CANopen subscription predicate | Express the disjoint COB-ID cases as a switch, preserving the SRDO and LSS feature gates and identifier range. | | [#620](https://github.com/dborgards/CanKit.Pro/security/code-scanning/620) | LSS emergency cancellation test | Declare the second task where it is assigned; remove its unused initial value. | | [#647](https://github.com/dborgards/CanKit.Pro/security/code-scanning/647), [#648](https://github.com/dborgards/CanKit.Pro/security/code-scanning/648), [#649](https://github.com/dborgards/CanKit.Pro/security/code-scanning/649), [#650](https://github.com/dborgards/CanKit.Pro/security/code-scanning/650), [#651](https://github.com/dborgards/CanKit.Pro/security/code-scanning/651), [#652](https://github.com/dborgards/CanKit.Pro/security/code-scanning/652), [#653](https://github.com/dborgards/CanKit.Pro/security/code-scanning/653), [#654](https://github.com/dborgards/CanKit.Pro/security/code-scanning/654), [#655](https://github.com/dborgards/CanKit.Pro/security/code-scanning/655), [#656](https://github.com/dborgards/CanKit.Pro/security/code-scanning/656), [#657](https://github.com/dborgards/CanKit.Pro/security/code-scanning/657), [#658](https://github.com/dborgards/CanKit.Pro/security/code-scanning/658), [#659](https://github.com/dborgards/CanKit.Pro/security/code-scanning/659), [#660](https://github.com/dborgards/CanKit.Pro/security/code-scanning/660), [#661](https://github.com/dborgards/CanKit.Pro/security/code-scanning/661), [#662](https://github.com/dborgards/CanKit.Pro/security/code-scanning/662), [#663](https://github.com/dborgards/CanKit.Pro/security/code-scanning/663), [#664](https://github.com/dborgards/CanKit.Pro/security/code-scanning/664) | LSS tests | Remove casts whose operand already has the concrete CanOpenNode type. | +| [#742](https://github.com/dborgards/CanKit.Pro/security/code-scanning/742) | ISO-TP queued-send cancellation regression test | Follow-up in #480: keep the deliberately delayed Dispose as an observed test operation, with using-owned channel cleanup and an outer finally that releases the actor. The caller-token and late-disposal ordering assertions remain intact. Full main CodeQL at cc363d6d confirmed this previously retained recommendation as fixed on 2026-10-27. | The cancellation tests inject throwing callbacks and assert both the original failure and completed cleanup. Restoring each old swallowing path makes the corresponding test fail. Full-suite, packaging and coverage evidence belongs to the pull request/check runs. ## Retained recommendations +Follow-up verification on 2026-10-27 at `cc363d6d`: #742 is now fixed by #480, leaving 18 of the original recommendations below visible. No new alerts were reported. + These sites retain their existing behavior and remain visible to CodeQL. No global query exclusion, inline suppression or alert dismissal is used to make the dashboard green. | Alerts | Site | Reason to retain the current construct | | --- | --- | --- | | [#714](https://github.com/dborgards/CanKit.Pro/security/code-scanning/714), [#715](https://github.com/dborgards/CanKit.Pro/security/code-scanning/715), [#716](https://github.com/dborgards/CanKit.Pro/security/code-scanning/716), [#737](https://github.com/dborgards/CanKit.Pro/security/code-scanning/737) | J1939 node fields: service, actor, subscription, reader CTS | These are object-lifetime fields, not local temporaries. Constructor failure cleanup releases partially initialized owned resources; normal disposal releases them later. A local using would dispose successful construction resources before the node can use them. Injected actors/services retain caller ownership. | -| [#742](https://github.com/dborgards/CanKit.Pro/security/code-scanning/742) | ISO-TP queued-send cancellation regression test | The channel is disposed on a deliberately delayed worker after cancellation assertions. The test observes the disposal before releasing the held actor; a using cannot replace this ordering or the finally that releases the actor and joins disposal. | | [#748](https://github.com/dborgards/CanKit.Pro/security/code-scanning/748) | TX-confirm cross-service reentrant-disposal regression test | The service is disposed only if its send has completed. Unconditional using cleanup would deadlock on the very regression this test must report. | | [#731](https://github.com/dborgards/CanKit.Pro/security/code-scanning/731), [#732](https://github.com/dborgards/CanKit.Pro/security/code-scanning/732), [#697](https://github.com/dborgards/CanKit.Pro/security/code-scanning/697), [#698](https://github.com/dborgards/CanKit.Pro/security/code-scanning/698) | Background fault delivery in dispatcher, actor and RawCan | Fault subscribers are arbitrary application callbacks. Each must be isolated from subsequent subscribers and the draining loop. Sending subscriber failures back into the same fault channel would recurse. Narrowing by exception type would let other callback exceptions terminate delivery. | | [#733](https://github.com/dborgards/CanKit.Pro/security/code-scanning/733), [#734](https://github.com/dborgards/CanKit.Pro/security/code-scanning/734) | Background-dispatch regression test callbacks | Transfer an arbitrary callback failure to the test completion source, so the awaiting test observes it instead of leaving the test hanging. | diff --git a/tests/CanKit.Pro.Tests/TestCases/J1939TpTests.cs b/tests/CanKit.Pro.Tests/TestCases/J1939TpTests.cs index 222f880e..7be24795 100644 --- a/tests/CanKit.Pro.Tests/TestCases/J1939TpTests.cs +++ b/tests/CanKit.Pro.Tests/TestCases/J1939TpTests.cs @@ -1935,14 +1935,15 @@ public async Task A_Cts_Hold_After_The_Last_Dt_Leaves_The_EndOfMsgAck_Timer_Runn [Fact] public async Task A_Cts_Hold_Before_The_Block_End_Confirmation_Starts_T4_Not_T3() { + using var clock = new VirtualClock(); + var confirmationActor = clock.NewActor(); using var bus = ControllableBus.DeferredEchoCapable(NewSession()); - using var service = new FrameConsumptionCountingBusService(new CanBusService(bus)); + using var service = new FrameConsumptionCountingBusService(new ClockedCanBusService(bus, confirmationActor)); const byte subjectSa = 0x10, peerSa = 0x20; const uint pgn = 0xFEC7u; var payload = RandomPayload(21, seed: 8); // three packets, granted one at a time var options = new J1939TpOptions().With( t2: TimeSpan.FromSeconds(5), t3: TimeSpan.FromSeconds(2), t4: TimeSpan.FromMilliseconds(1050)); - using var clock = new VirtualClock(); var actor = clock.NewActor(); using var sender = new J1939TpChannel(service, sourceAddress: subjectSa, options, ownsService: false, actor); @@ -1951,6 +1952,7 @@ static CanFrame PeerCm(byte peerSa, byte subjectSa, byte[] data) var send = sender.SendCmAsync(pgn, peerSa, payload); await bus.DeferredEchoes.WaitForEnqueuedAsync(1, ShortTimeout); // the RTS + await clock.WaitUntilTimerArmedAsync(confirmationActor, TimeSpan.FromSeconds(1), ShortTimeout); bus.DeferredEchoes.ReleaseNext(); bus.RaiseObserved(PeerCm(peerSa, subjectSa, J1939TpFrames.BuildCts(numPackets: 1, nextPacketSn: 1, dataPgn: pgn)), isEcho: false); @@ -1985,14 +1987,15 @@ static CanFrame PeerCm(byte peerSa, byte subjectSa, byte[] data) [Fact] public async Task A_Cts_Hold_In_The_Middle_Of_A_Block_Is_Ignored() { + using var clock = new VirtualClock(); + var confirmationActor = clock.NewActor(); using var bus = ControllableBus.DeferredEchoCapable(NewSession()); - using var service = new FrameConsumptionCountingBusService(new CanBusService(bus)); + using var service = new FrameConsumptionCountingBusService(new ClockedCanBusService(bus, confirmationActor)); const byte subjectSa = 0x10, peerSa = 0x20; const uint pgn = 0xFEC5u; var payload = RandomPayload(21, seed: 9); // three packets, two granted in the first block var options = new J1939TpOptions().With( t2: TimeSpan.FromSeconds(5), t3: TimeSpan.FromSeconds(2), t4: TimeSpan.FromMilliseconds(1050)); - using var clock = new VirtualClock(); var actor = clock.NewActor(); using var sender = new J1939TpChannel(service, sourceAddress: subjectSa, options, ownsService: false, actor); @@ -2001,6 +2004,7 @@ static CanFrame PeerCm(byte peerSa, byte subjectSa, byte[] data) var send = sender.SendCmAsync(pgn, peerSa, payload); await bus.DeferredEchoes.WaitForEnqueuedAsync(1, ShortTimeout); + await clock.WaitUntilTimerArmedAsync(confirmationActor, TimeSpan.FromSeconds(1), ShortTimeout); bus.DeferredEchoes.ReleaseNext(); bus.RaiseObserved(PeerCm(peerSa, subjectSa, J1939TpFrames.BuildCts(numPackets: 2, nextPacketSn: 1, dataPgn: pgn)), isEcho: false); @@ -2036,14 +2040,15 @@ static CanFrame PeerCm(byte peerSa, byte subjectSa, byte[] data) [Fact] public async Task A_Hold_Behind_A_Stashed_Cts_Does_Not_Outlive_The_Block_It_Was_Sent_In() { + using var clock = new VirtualClock(); + var confirmationActor = clock.NewActor(); using var bus = ControllableBus.DeferredEchoCapable(NewSession()); - using var service = new FrameConsumptionCountingBusService(new CanBusService(bus)); + using var service = new FrameConsumptionCountingBusService(new ClockedCanBusService(bus, confirmationActor)); const byte subjectSa = 0x10, peerSa = 0x20; const uint pgn = 0xFEC9u; var payload = RandomPayload(21, seed: 11); // three packets, granted one at a time var options = new J1939TpOptions().With( t2: TimeSpan.FromSeconds(5), t3: TimeSpan.FromSeconds(2), t4: TimeSpan.FromMilliseconds(1050)); - using var clock = new VirtualClock(); var actor = clock.NewActor(); using var sender = new J1939TpChannel(service, sourceAddress: subjectSa, options, ownsService: false, actor); @@ -2059,6 +2064,7 @@ async Task DeliverAsync(byte[] data) var send = sender.SendCmAsync(pgn, peerSa, payload); await bus.DeferredEchoes.WaitForEnqueuedAsync(1, ShortTimeout); // the RTS + await clock.WaitUntilTimerArmedAsync(confirmationActor, TimeSpan.FromSeconds(1), ShortTimeout); bus.DeferredEchoes.ReleaseNext(); bus.RaiseObserved(PeerCm(peerSa, subjectSa, J1939TpFrames.BuildCts(numPackets: 1, nextPacketSn: 1, dataPgn: pgn)), isEcho: false); @@ -4135,9 +4141,10 @@ public async Task Cancelling_A_Bam_Releases_Its_Spacing_Timer() [Fact] public async Task A_Late_Announce_Confirmation_Of_A_Cancelled_Bam_Does_Not_Start_The_Next_Bam() { + using var clock = new VirtualClock(); + var confirmationActor = clock.NewActor(); const uint pgn = 0xFECAu; var payload = RandomPayload(21, seed: 250); // 3 TP.DT - using var clock = new VirtualClock(); var actor = clock.NewActor(); using var bus = ControllableBus.DeferredEchoCapable(NewSession()); var dtSequence = new List(); @@ -4146,12 +4153,13 @@ public async Task A_Late_Announce_Confirmation_Of_A_Cancelled_Bam_Does_Not_Start if (J1939Id.Decompose((uint)frame.ID).Pgn != J1939Pgn.TpDt) return; lock (dtSequence) dtSequence.Add(frame.Data.Span[0]); }; - using var service = new CanBusService(bus); + using var service = new ClockedCanBusService(bus, confirmationActor); using var sender = BamSenderOn(actor, service); using var cts = new CancellationTokenSource(); var first = sender.SendBamAsync(pgn, payload, cts.Token); await bus.DeferredEchoes.WaitForEnqueuedAsync(1, ShortTimeout); // its announce, confirmation held + await clock.WaitUntilTimerArmedAsync(confirmationActor, TimeSpan.FromSeconds(1), ShortTimeout); cts.Cancel(); Func cancelled = async () => await first.WithTimeout(ShortTimeout); await cancelled.Should().ThrowAsync(); @@ -4183,14 +4191,15 @@ public async Task A_Late_Announce_Confirmation_Of_A_Cancelled_Bam_Does_Not_Start [Fact] public async Task A_Cts_Hold_While_The_Last_Packet_Is_Outstanding_Leaves_T3_For_The_EndOfMsgAck() { + using var clock = new VirtualClock(); + var confirmationActor = clock.NewActor(); using var bus = ControllableBus.DeferredEchoCapable(NewSession()); - using var service = new FrameConsumptionCountingBusService(new CanBusService(bus)); + using var service = new FrameConsumptionCountingBusService(new ClockedCanBusService(bus, confirmationActor)); const byte subjectSa = 0x10, peerSa = 0x20; const uint pgn = 0xFEC4u; var payload = RandomPayload(14, seed: 11); // two packets, one block var options = new J1939TpOptions().With( t2: TimeSpan.FromSeconds(5), t3: TimeSpan.FromSeconds(2), t4: TimeSpan.FromMilliseconds(1050)); - using var clock = new VirtualClock(); var actor = clock.NewActor(); using var sender = new J1939TpChannel(service, sourceAddress: subjectSa, options, ownsService: false, actor); @@ -4199,6 +4208,7 @@ static CanFrame PeerCm(byte peerSa, byte subjectSa, byte[] data) var send = sender.SendCmAsync(pgn, peerSa, payload); await bus.DeferredEchoes.WaitForEnqueuedAsync(1, ShortTimeout); + await clock.WaitUntilTimerArmedAsync(confirmationActor, TimeSpan.FromSeconds(1), ShortTimeout); bus.DeferredEchoes.ReleaseNext(); bus.RaiseObserved(PeerCm(peerSa, subjectSa, J1939TpFrames.BuildCts(numPackets: 2, nextPacketSn: 1, dataPgn: pgn)), isEcho: false); @@ -4258,9 +4268,10 @@ public async Task Disposing_The_Channel_Releases_The_Spacing_Timer_Of_A_Bam_In_F [Fact] public async Task A_Late_Dt_Confirmation_Of_A_Cancelled_Bam_Does_Not_Advance_The_Next_Bam() { + using var clock = new VirtualClock(); + var confirmationActor = clock.NewActor(); const uint pgn = 0xFEC9u; var payload = RandomPayload(21, seed: 252); // 3 TP.DT - using var clock = new VirtualClock(); var actor = clock.NewActor(); using var bus = ControllableBus.DeferredEchoCapable(NewSession()); var dtSequence = new List(); @@ -4269,12 +4280,13 @@ public async Task A_Late_Dt_Confirmation_Of_A_Cancelled_Bam_Does_Not_Advance_The if (J1939Id.Decompose((uint)frame.ID).Pgn != J1939Pgn.TpDt) return; lock (dtSequence) dtSequence.Add(frame.Data.Span[0]); }; - using var service = new CanBusService(bus); + using var service = new ClockedCanBusService(bus, confirmationActor); using var sender = BamSenderOn(actor, service); using var cts = new CancellationTokenSource(); var first = sender.SendBamAsync(pgn, payload, cts.Token); await bus.DeferredEchoes.WaitForEnqueuedAsync(1, ShortTimeout); // announce + await clock.WaitUntilTimerArmedAsync(confirmationActor, TimeSpan.FromSeconds(1), ShortTimeout); bus.DeferredEchoes.ReleaseNext(); await clock.WaitUntilTimerArmedAsync(actor, InFlightSpacing, ShortTimeout); await clock.AdvanceAsync(InFlightSpacing); @@ -4309,6 +4321,7 @@ public async Task A_Late_Dt_Confirmation_Of_A_Cancelled_Bam_Does_Not_Advance_The public async Task A_Late_Announce_Confirmation_Of_A_Cancelled_Bam_With_No_Successor_Sends_Nothing() { using var clock = new VirtualClock(); + var confirmationActor = clock.NewActor(); var actor = clock.NewActor(); using var bus = ControllableBus.DeferredEchoCapable(NewSession()); var dtCount = 0; @@ -4316,12 +4329,13 @@ public async Task A_Late_Announce_Confirmation_Of_A_Cancelled_Bam_With_No_Succes { if (J1939Id.Decompose((uint)frame.ID).Pgn == J1939Pgn.TpDt) Interlocked.Increment(ref dtCount); }; - using var service = new CanBusService(bus); + using var service = new ClockedCanBusService(bus, confirmationActor); using var sender = BamSenderOn(actor, service); using var cts = new CancellationTokenSource(); var send = sender.SendBamAsync(0xFECAu, RandomPayload(21, seed: 253), cts.Token); await bus.DeferredEchoes.WaitForEnqueuedAsync(1, ShortTimeout); // its announce, confirmation held + await clock.WaitUntilTimerArmedAsync(confirmationActor, TimeSpan.FromSeconds(1), ShortTimeout); cts.Cancel(); Func cancelled = async () => await send.WithTimeout(ShortTimeout); await cancelled.Should().ThrowAsync(); @@ -4413,10 +4427,11 @@ private static byte[][] DtFrames(byte[] payload, int packets) [Fact] public async Task A_Stale_Bam_Dt_Continuation_Leaves_The_Successor_Bam_Of_Its_Key_Alone() { + using var clock = new VirtualClock(); + var confirmationActor = clock.NewActor(); const uint pgn = 0xFEC9u; var stalePayload = RandomPayload(21, seed: 367); // three packets each var payload = RandomPayload(21, seed: 368); - using var clock = new VirtualClock(); var actor = clock.NewActor(); using var bus = ControllableBus.DeferredEchoCapable(NewSession()); var dts = new List(); @@ -4424,12 +4439,13 @@ public async Task A_Stale_Bam_Dt_Continuation_Leaves_The_Successor_Bam_Of_Its_Ke { if (J1939Id.Decompose((uint)frame.ID).Pgn == J1939Pgn.TpDt) lock (dts) dts.Add(frame.Data.ToArray()); }; - using var service = new CanBusService(bus); + using var service = new ClockedCanBusService(bus, confirmationActor); using var sender = BamSenderOn(actor, service); using var cts = new CancellationTokenSource(); var first = sender.SendBamAsync(pgn, stalePayload, cts.Token); await bus.DeferredEchoes.WaitForEnqueuedAsync(1, ShortTimeout); // the announce + await clock.WaitUntilTimerArmedAsync(confirmationActor, TimeSpan.FromSeconds(1), ShortTimeout); bus.DeferredEchoes.ReleaseNext(); await clock.WaitUntilTimerArmedAsync(actor, InFlightSpacing, ShortTimeout); // DT 1 is next var stale = (await actor.PostAsync(() => sender.CaptureTxContinuations(J1939Pgn.GlobalAddress, pgn))).NextBamDt; @@ -4458,12 +4474,13 @@ public async Task A_Stale_Bam_Dt_Continuation_Leaves_The_Successor_Bam_Of_Its_Ke [InlineData("T4Expired")] public async Task A_Stale_Cm_Continuation_Leaves_The_Successor_Session_Of_Its_Key_Alone(string continuation) { + using var clock = new VirtualClock(); + var confirmationActor = clock.NewActor(); const byte subjectSa = 0x10, peerSa = 0x20; const uint pgn = 0xFEC9u; var stalePayload = RandomPayload(21, seed: 369); // three packets each var payload = RandomPayload(21, seed: 370); var options = new J1939TpOptions().With(t3: TimeSpan.FromSeconds(2), t4: TimeSpan.FromMilliseconds(1050)); - using var clock = new VirtualClock(); var actor = clock.NewActor(); using var bus = ControllableBus.DeferredEchoCapable(NewSession()); var dts = new List(); @@ -4474,7 +4491,7 @@ public async Task A_Stale_Cm_Continuation_Leaves_The_Successor_Session_Of_Its_Ke if (framePgn == J1939Pgn.TpDt) lock (dts) dts.Add(frame.Data.ToArray()); else if (framePgn == J1939Pgn.TpCm && frame.Data.Span[0] == J1939TpFrames.ControlAbort) Interlocked.Increment(ref aborts); }; - using var service = new CanBusService(bus); + using var service = new ClockedCanBusService(bus, confirmationActor); using var sender = new J1939TpChannel(service, sourceAddress: subjectSa, options, ownsService: false, actor); static CanFrame PeerCm(byte peerSa, byte subjectSa, byte[] data) => CanFrame.Classic((int)J1939Id.ComposePgn(7, J1939Pgn.TpCm, peerSa, subjectSa), data, isExtendedFrame: true); @@ -4484,6 +4501,7 @@ static CanFrame PeerCm(byte peerSa, byte subjectSa, byte[] data) using var cancel = new CancellationTokenSource(); var first = sender.SendCmAsync(pgn, peerSa, stalePayload, cancel.Token); await bus.DeferredEchoes.WaitForEnqueuedAsync(1, ShortTimeout); // the RTS + await clock.WaitUntilTimerArmedAsync(confirmationActor, TimeSpan.FromSeconds(1), ShortTimeout); bus.DeferredEchoes.ReleaseNext(); var staleDts = new List(); if (continuation == "NextCmDt")