fix: audit-conformance pass — DropOldest back-pressure, E2E masquerade gap, LDF panic, CI gates - #42
Merged
Merged
Conversation
…E masquerade gap, LDF panic, CI gates
Applies the x-Net gap-audit findings confirmed genuine for this repo
(worklist rust-LIN-01..05):
- rust-LIN-01 (High, cross-repo XR-06): Adapter::subscribe hard-coded its
bus subscription to BackPressurePolicy::DropNewest / rate_limit_per_sec:
0 regardless of the caller's SubscriberOptions, and both the DropNewest
and DropOldest match arms in the forwarding task called the identical
non-evicting `tx.try_send`, so selecting DropOldest silently behaved
like DropNewest. Per RELAY spec §14 step 3, DropOldest must drain one
message then enqueue the new one — a distinct policy from DropNewest.
Now delegates policy enforcement to the bus subscription layer
(bus.rs SubInner, which already implements all three policies
correctly) using the caller's real back_pressure/channel_depth/
rate_limit_per_sec, then forwards with a blocking send so no second
drop layer masks the chosen policy.
- rust-LIN-02 (High): E2E Receiver::unwrap recomputed the CRC over
whatever DataID/SourceID the sender wrote and never compared the
received header identifiers against Receiver.cfg, so a frame from a
different logical data element or sender node passed validation as
long as it was internally self-consistent — CRC alone proves integrity,
not identity. This defeated the module's own documented ASIL-B
masquerade-detection intent. unwrap now compares the received
data_id/source_id against the receiver's configuration after the CRC
check and returns a new ErrorKind::IdentifierMismatch on mismatch, per
AUTOSAR E2E explicit-DataID scheme design intent.
- rust-LIN-03 (Medium): extract_bits looped `1u64 << i` against a u64
accumulator with no upper bound on a bit_width parsed from an untrusted
.ldf file, panicking ("attempt to shift left with overflow") for any
signal declared >= 64 bits wide — reachable via Db::decode() on bus
payloads from a hostile .ldf, violating the module's documented
"never panics" contract. LIN frames are capped at 8 bytes (64 bits) per
LIN 2.x / ISO 17987, so bits beyond 64 aren't representable; the loop
now bounds iteration accordingly.
- rust-LIN-04 (Medium): the CI coverage step computed and echoed the
line-coverage percentage but never compared it to the documented >= 90%
gate, so the step always succeeded regardless of measured coverage. Now
fails the job when coverage drops below 90%.
- rust-LIN-05 (Medium): the "ASIL-B safety" job's strict `rsfusa check`
step was suffixed with `|| true`, swallowing any non-zero exit from a
strict-mode violation and making the job advisory-only rather than a
real gate. Removed the suffix.
Regression tests added for each fix, including two adapter-level tests
that deterministically reproduce rust-LIN-01 via a `PreloadedBus` test
double (frames pushed into a real `SubInner` before the forwarding task
is spawned, avoiding scheduling races) — verified these fail against the
pre-fix adapter code (frame 1/2 delivered instead of 4/5; rate limiter
not honored) before confirming they pass against the fix. Also added
direct and end-to-end (hostile-.ldf-through-decode) tests proving
extract_bits no longer panics at or beyond a 64-bit bit_width, and tests
proving unwrap rejects mismatched DataID/SourceID while still accepting
matching ones.
`cargo build`, `cargo test --locked` (164 tests: 116 unit + 46
integration + 2 doc), `cargo clippy --all-targets -- -D warnings`, and
`cargo fmt --check` all pass. rsfusa is not installed locally; CI's
`rsfusa check --strict` (now a real gate per rust-LIN-05) is the first
live check of the requirements-traceability/ASIL annotations touched by
this change set (a new IdentifierMismatch error kind, no new
REQ-SAFETY-* ids).
Version bumped 0.4.2 -> 0.4.3 (bugfix release: DropOldest and the E2E
identifier check are both functional-behavior fixes to already-declared
capabilities, not new API surface) with matching updates to the
safety-artifact version headers and SAFETY_MANUAL.md version history.
Signed-off-by: Matt Jones <47545907+SoundMatt@users.noreply.github.com>
Signed-off-by: Matt <47545907+SoundMatt@users.noreply.github.com>
rust-LIN-04.diff (applied earlier in this branch) added a hard-fail >=90% line-coverage gate, but its regex (grep -oP 'Lines\s+\K[\d.]+') never matches cargo-llvm-cov's real --summary-only table layout (no row has the literal word "Lines" immediately followed by a number), so it always fell back to reporting 0% -- same broken output already present, un-gated, on main before this branch. Replaced with an awk extraction of the TOTAL row's real Lines-coverage field (~86.3% today). Left informational only, not gated at 90%: real coverage is below that bar, so enforcing it needs a dedicated test-writing pass, not a threshold silently picked to match today's number. Signed-off-by: Matt <47545907+SoundMatt@users.noreply.github.com>
… to errors-only
rust-LIN-04.diff (applied earlier in this branch) un-masked
`rsfusa check --strict`'s || true, which revealed .fusa.json was in a
stale/incompatible schema -- rsfusa's current parser requires a
top-level "standard" field this file never had ({"tool","version",
"project","asil","protocol"} instead), so it failed to parse at all.
Since lint/analyze/check all read the same file, this meant every
rsfusa invocation in CI was silently a no-op behind || true, not just
the newly-unmasked check step. Replaced with the current schema,
matching rust-DDS's own already-working config.
With that fixed, `rsfusa check` (no --strict) is clean: no ERROR
findings. Left --strict off deliberately: it additionally gates on
every open WARNING finding, and this repo has 166 pre-existing ones
(function-length/nesting/cyber-hygiene style, not the conformance
defects this pass targets), none yet dispositioned via rsfusa's own
accept/defer mechanism. ERROR-severity findings now genuinely gate the
build either way -- a real improvement over full || true masking,
without silently overclaiming full --strict enforcement this PR
didn't do the work for. Same reasoning already applied to rust-MQTT's
identical situation earlier in this pass.
Signed-off-by: Matt <47545907+SoundMatt@users.noreply.github.com>
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
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.
Summary
Applies the x-Net gap-audit worklist for rust-LIN (5 items, all with pre-made diffs; all applied cleanly against current
origin/main):DropOldestback-pressure policy silently degraded toDropNewestReceivernever validated DataID/SourceID — false ASIL-B masquerade protectionextract_bitspanics on shift overflow for signals >= 64 bits (hostile-input DoS)|| trueSpec verification
spec/relay-spec.md:791): "DropOldest: drain one message from the channel, then enqueue the new one." The adapter's forwarding task previously used identicaltx.try_sendlogic for bothDropNewestandDropOldest, and additionally hard-coded the bus subscription toDropNewest/rate_limit_per_sec: 0regardless of the caller's realSubscriberOptions.bus.rs'sSubInneralready implements all three policies correctly, so the fix is to delegate to it with the caller's real options and forward with a blocking send (no second, policy-less drop layer).Receiver::unwrapnow checksdata[0..2]/data[2..4]againstself.cfg.data_id/self.cfg.source_idafter the CRC check and returns a newErrorKind::IdentifierMismatchon mismatch.bit_width >= 64is malformed/hostile input. The module's own documented contract ("Never panics; malformed input results in a partial Db or an error",src/ldf/mod.rs) is violated by direct inspection prior to this fix. Bounded the extraction loop so bits >= 64 are never shifted..github/workflows/ci.yml.Regression-test proof (memory-safety / DoS class: rust-LIN-03; behavioral-security class: rust-LIN-01, rust-LIN-02)
rust-LIN-01 — two new adapter-level tests in
src/adapt.rsusing aPreloadedBustest double that pushes frames directly into a realSubInnerbefore the adapter's forwarding task is spawned (avoids any scheduling race):subscribe_drop_oldest_evicts_oldest_not_newest: pushes frames 1..=5 against a capacity-2DropOldestqueue; asserts the two survivors are 4 and 5 (the most recent), never 1/2.subscribe_forwards_caller_rate_limit_per_sec: provesrate_limit_per_secis forwarded rather than hard-coded to 0.I manually verified both fail against the pre-fix adapter code (grafted the new tests onto the pre-fix
src/adapt.rs, ran them, confirmed failure with the exact "before" symptoms — frames 1/2 delivered instead of 4/5, and a second frame arriving when the rate limiter should have suppressed it), then confirmed they pass against the fix.rust-LIN-02 —
src/safety/mod.rs:unwrap_rejects_wrong_data_id,unwrap_rejects_wrong_source_id(both build an internally self-consistent, CRC-valid frame under a different senderConfigand confirm the receiver rejects it withIdentifierMismatch), plusunwrap_accepts_matching_identifiersas the matching happy-path control.rust-LIN-03 (fuzz-style malformed-input regression, not just happy-path) —
src/ldf/mod.rs:extract_bits_does_not_panic_at_64_bits/extract_bits_does_not_panic_beyond_64_bits: direct unit tests at and beyond the overflow boundary.decode_does_not_panic_on_oversized_signal_from_hostile_ldf: end-to-end test that parses a hostile.ldfdeclaring a 128-bit signal and callsDb::decode()on it, proving the whole pipeline (untrusted file -> parse -> decode) survives without panicking, matching the module's documented contract.Test plan
cargo build --all-targetscargo test --locked— 164 tests (116 unit + 46 integration + 2 doc), all greencargo clippy --all-targets -- -D warnings— cleancargo fmt --check— cleanrsfusa check --strict— not run locally (rsfusa not installed in this environment); CI's now-enforced strict gate (rust-LIN-05) is the first live checkVersion bumped 0.4.2 -> 0.4.3 (bugfix release — DropOldest and the E2E identifier check are corrections to already-declared capabilities, not new API surface) with matching updates to the safety-artifact version headers and a new
SAFETY_MANUAL.mdversion-history row.