feat: S08.07 mbedTLS reference integration (host-side adapter + integration tests)#419
Merged
Conversation
Three-TU split mirroring OpenSSL TlsStream; INTERFACE pack like FreeRtos/FatFs because mbedtls_config.h is header-configured. Initialise/Cleanup are no-ops that wire the Stream vtable to NullStream — real Open/Send/Read/Close lands in slice 2 via mbedtls_ssl_set_bio. Config struct accepts pre-built mbedTLS handles (mbedtls_ctr_drbg_context*, mbedtls_x509_crt*, mbedtls_pk_context*) as forward declarations so the public header has zero mbedTLS includes. Library policy lives on the per-instance config; no process-global mbedTLS calls in Platform/MbedTls/Source/ (coexistence contract). Tests: 10 pool tests parity with SolidSyslogTlsStreamPoolTest (Create, exhaustion, NullStream fallback, Send drop-on-floor, ConfigLock pair, Destroy pair, unknown + stale destroy warnings). Tests/MbedTlsIntegration/ links real libmbedtls from \$MBEDTLS_DIR as a subproject; smoke test confirms the link, in-process TLS server lands in slice 3. Pre-commit (freertos-host container): format clean, tidy clean on MbedTls targets, cppcheck clean, sanitize clean. IWYU skipped (needs cpputest-clang). Story: #272. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drives the vtable end-to-end against MbedTlsFake (link-time interposition). Open builds the ssl_config (init + defaults for CLIENT/STREAM/DEFAULT preset), inits and setups the ssl_context, wires the BIO bridge via mbedtls_ssl_set_bio (p_bio = self, send/recv callbacks delegate to the injected transport, no recv_timeout), and drives the handshake. Send is mbedtls_ssl_write; Read is mbedtls_ssl_read with WANT_READ -> 0 and other errors -> -1. Close issues close_notify, frees ssl_context and ssl_config, then closes the transport. The deliberately-deferred slices are visible in the production code: no conf_authmode / conf_ca_chain / set_hostname yet (slice 3), no bounded WANT_READ/WANT_WRITE handshake retry (slice 4), no conf_own_cert / mTLS (slice 5). Tests: 22 vtable tests in Tests/MbedTls/SolidSyslogMbedTlsStreamTest.cpp (separate binary from the pool tests because each pins different concerns). The shared adapter source is compiled into both test exes via the existing include path; MbedTlsFakes is linked PUBLICly to bring $MBEDTLS_DIR/include along. Tests/Support/MbedTlsFake mirrors OpenSslFake: per-symbol call counters, captured-arg accessors, settable return values where the production code's branch depends on them. cppcheck disabled on every TU that pulls in <mbedtls/*.h> because mbedTLS's MBEDTLS_USER_CONFIG_FILE macro indirection trips its preprocessor (same posture as OpenSSL upstream sources). Pre-commit (freertos-host container): format clean, tidy clean, cppcheck clean, sanitize green on both MbedTls binaries. Story: #272. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…on scaffold
Open's chain now applies the certificate-validation policy on the per-instance
ssl_config (project_mbedtls_coexistence_contract holds — none of these calls
leak to any other ssl_config the integrator might own):
- mbedtls_ssl_conf_authmode(MBEDTLS_SSL_VERIFY_REQUIRED)
- mbedtls_ssl_conf_ca_chain(config.CaChain, /* CRL */ NULL)
- mbedtls_ssl_conf_rng(mbedtls_ctr_drbg_random, config.Rng)
- mbedtls_ssl_set_hostname(config.ServerName) when ServerName != NULL
Five new unit tests (27 total in SolidSyslogMbedTlsStreamTest, all green)
plus a small fixture refactor (ReCreateHandleWithUpdatedConfig) so tests
that mutate Config can re-Create after setup().
Integration scaffold under Tests/MbedTlsIntegration/:
- SocketStream.{c,h}: minimal raw-fd SolidSyslogStream over one end of a
socketpair; client-side transport for the integration test.
- MbedTlsTestCert.{c,h}: in-memory CA + server-cert + RSA-2048 key
generation via mbedtls_x509write_crt_*; certs are immediately usable
by mbedtls_ssl_conf_ca_chain / _conf_own_cert without going via PEM.
- MbedTlsTestServer.{c,h}: server-side mbedTLS driven on a pthread; runs
just the handshake step and exits (slice 4+ extends it with read-loop
behaviour if a test needs to assert on delivered bytes).
Happy-path integration test passes end-to-end against real libmbedtls
(handshake completes when the server cert chains to the trusted CA and
the SAN matches ServerName). The two negative scenarios — wrong-CA and
mismatched-hostname — are landed as IGNORE_TEST with a TODO; the client
correctly returns the verify error but the server thread stays blocked
during the handshake response (likely the TLS-1.3-vs-1.2 sequencing the
OpenSSL TlsTestServer pins around). Re-enable in a follow-up by capping
the server's max version to TLS 1.2 or by feeding the worker a deadline.
Also adds .vscode/c_cpp_properties.json pointing the C/C++ extension at
build/debug/compile_commands.json so IntelliSense resolves <mbedtls/*.h>
correctly in the devcontainer (no more "cannot open source file" squiggles).
Pre-commit (freertos-host container): format clean across MbedTls files,
all 27 unit tests green, integration binary runs 2 / ignores 2 / 0 failures.
Story: #272.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The two negative-path tests (untrusted-CA, hostname-mismatch) landed as IGNORE in the slice-3 commit because they deadlocked the harness. Three fixes together turn them green: - Pin the in-process test server to TLS 1.2 so cert-rejection is synchronous within the server's own handshake (TLS 1.3 sends the cert in a single flight then blocks on ClientFinished, leaving the pthread worker stuck during teardown). Mirrors the OpenSSL TlsTestServer pin. - Ignore SIGPIPE in the test main so a send to a peer fd the other side has already closed during teardown surfaces as EPIPE instead of aborting the process. The BIO callbacks don't pass MSG_NOSIGNAL. - Track IsOpen on MbedTlsStream and call Close from Cleanup, mirroring the OpenSSL TlsStream pattern. Without this, destroying an Open stream leaked the mbedTLS SSL context/config — only exposed once the negative paths actually ran under ASan. The pool test now wires a StreamFake transport (matching the OpenSSL pool-test setup) so the vtable-routed Close in Cleanup has somewhere to land. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
mbedtls_ssl_handshake under non-blocking transport can return MBEDTLS_ERR_SSL_WANT_READ / WANT_WRITE between RTTs while the multi-leg handshake progresses. Open now drives it to completion within a 5s budget (1ms poll interval), sleeping via the injected SolidSyslogSleepFunction between retries — same shape and constants as the OpenSSL TlsStream_PerformHandshake loop. Non-WANT errors are fail-fast: no retry, no Sleep, Open returns false on the first call. Test surface: - MbedTlsFake gains SetSslHandshakeReturnSequence so unit tests can drive WANT_READ → 0, WANT_WRITE → 0, and persistent-WANT sequences. - NoOpSleep in the unit test is now instrumented (call count + last args) via TestUtils' CALLED_FUNCTION macro. - Five new tests: retry-on-WANT_READ, sleep-between-retries, retry-on-WANT_WRITE, fail-on-persistent-WANT (budget), fail-immediately-on-hard-error. The integration tests still use blocking socketpair BIOs so the loop isn't exercised end-to-end yet — that arrives in slice 5 (mTLS) or a later non-blocking-BIO scaffolding pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Open now calls mbedtls_ssl_conf_own_cert when the integrator supplies both ClientCertChain and ClientKey on the config — wiring the client identity that mbedTLS sends in response to the server's CertificateRequest. Either pointer NULL means "server-auth only" and the call is skipped, leaving mbedTLS's default of no client cert. Test surface: - MbedTlsFake gains conf_own_cert (counter + last-args). Three new unit tests pin the wiring: wires-when-both-set, skips-when-chain-null, skips-when-key-null. - MbedTlsTestServer learns TrustedClientCa — when non-NULL, server switches to VERIFY_REQUIRED + conf_ca_chain so it requires and verifies a client cert against that CA. - Three new integration tests mirror the OpenSSL TlsStream mTLS scenarios: handshake succeeds with trusted client CA, rejected when client sends no cert, rejected when client cert signed by untrusted CA. Harness: bound both socketpair fds with SO_RCVTIMEO=5s. The negative mTLS paths (no-cert in particular) hit a different cross-thread block than the slice-3 server-cert rejections; rather than chase per-scenario TLS message-order specifics, the cap matches the production handshake retry budget and bounds any test-only deadlock. OpenSSL's harness sidesteps this with an in-memory BIO pair — same goal, different mechanism. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The Stream contract uses 0 to signal would-block (mirrored in SolidSyslogTlsStream's TransportBioRead at OpenSsl/Source/SolidSyslogTlsStream.c:262-281). MbedTlsStream_BioRecv was collapsing it to -1, which mbedTLS treats as a fatal transport error — the slice-4 handshake retry loop never got a chance to engage. Real non-blocking transports (Posix/Windows TCP, FreeRTOS-Plus-TCP) would abort the handshake on the first scheduling-induced gap. Hidden today by the integration test harness using blocking socketpair recv with a 5s SO_RCVTIMEO that surfaces as -1 → fatal. With this fix, future non-blocking integration scaffolding would actually drive the retry loop end-to-end. BioSend has the same shape conceptually but the Stream contract gives Send no would-block channel (returns bool), so the symmetric fix isn't possible without extending the Stream API — same as the OpenSSL precedent. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Mirror the OpenSSL TlsStream shape more closely: - Initialise eagerly calls mbedtls_ssl_init / mbedtls_ssl_config_init so the structs are always in a free-safe state. Drops the IsOpen flag and the close-time guard it required. mbedTLS guarantees a freed struct is left in the same zeroed state init produces, so re-Open after Close works without re-init. - Drop the misleading "*Base = *NullStream_Get()" seed in Initialise; all four vtable slots are immediately overridden, so the NullStream copy was wasted work. - Open is now a chained && of small named helpers (ApplySslConfigDefaults / ApplyTlsPolicy / BindContextToConfig / ConfigureExpectedHostname / InstallTransportCallbacks / PerformHandshake), same shape as OpenSSL's TlsStream_Open. - Send and Read close internally on hard error, matching OpenSSL's fail-fast contract that drives the StreamSender reconnect path. Five new tests pin the contract: SendClosesSslAndTransportOnWriteFailure, SendClosesSsl…OnShortWrite, ReadClosesSsl…OnHardError, ReadDoesNotCloseOnWantRead, CloseAfterInternalCloseFromSendFailureDoesNotDoubleFree. Also fixes the agent-flagged IsOpen ordering bug: previously IsOpen was set true before mbedtls_ssl_init, so a config_defaults failure left Close calling ssl_free on a context that had never been init'd. Eager init removes the dependency on flag timing entirely. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Expose MbedTlsFake_LastSslSetBioRecvTimeoutCallback accessor and assert it's NULL in OpenWiresBioWithNonNullSendRecvAndNullRecvTimeout. The test name promised the assertion since slice 2; it wasn't being made, and the captured field had nothing reading it. - Drop the file-local RngFunc typedef. Project convention disallows project-owned typedefs (NAMING.md); the accessor header already uses the inline function-pointer form, so just match it in the .c. - NOLINTNEXTLINE on mbedtls_ctr_drbg_random's output parameter — the signature is fixed by the mbedTLS API contract (it's an out-buffer the contract writes to), so readability-non-const-parameter can't apply. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- MbedTlsTestServer: drop the dead Received[1024] / ReceivedLength plumbing and accessors — no test reads them and RunServer never writes them. Single-return on Destroy (NULL-guard wraps the body instead of early-return). Drop stale "slice 3 only pins" comment. - SocketStream: rename static vtable functions to SocketStream_* prefix per feedback_static_name_prefix. Collapse Destroy to delegate to Close so the idempotent-close logic lives in one place. - MbedTlsTestCert: replace strncpy with SafeString_Copy (link SafeStringStandard.c into the integration target). Collapse WriteCertToDer's 7-parameter signature into a 4-parameter shape using a DerBuffer struct — also clears the bugprone-easily-swappable-parameters tidy error on the size_t* pair. - NOLINT the MbedTestCert-by-MbedTestCert pair on StartServerRequiringClientCa in the integration test (function name distinguishes the roles, and reordering can't help with 2 args). All three MbedTls binaries now pass tidy cleanly (production tidy was already passing); no leaks under ASan; 7 integration + 42 unit + 10 pool tests green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Lift addr/addrStorage into the unit-test TEST_GROUP fixture (mirrors the OpenSSL TlsStream test); collapses ~25 three-line prologues. - Rename OpenInitialisesSslContext / OpenInitialisesSslConfig to CreateInitialisesSslContextForSafeFree / …SslConfigForSafeFree. The init now happens eagerly in Initialise (called from Create) so the symmetric *_free in Close is safe even if Open is never reached; the old names misdescribed which lifecycle step actually exercises it. Rename OpenSetupBindsSslContextToSslConfig to OpenBindsContextToConfig to drop the doubled mechanism noun. - Lift unit-test file-scope NoOpSleepCallCount / g_lastSleepMs (and the NoOpSleep callback) out of an anonymous namespace into bare static, matching the OpenSSL TlsStream test's idiom. - Extract BuildBaseConfig helper in the integration test; collapses the 6 identical "transport / sleep / rng / caChain / serverName" stanzas. - Fix SocketStream_Read EOF/would-block conflation in the integration harness. recv()==0 means peer-closed; the Stream contract reserves 0 for would-block (which BioRecv now maps to MBEDTLS_ERR_SSL_WANT_READ, per slice-4-followup). Forwarding a real EOF as 0 would loop the handshake until the 5s budget expired. Map EOF to -1 instead.
Parallel to integration-linux-openssl but runs under cpputest-freertos (the image carrying /opt/mbedtls + MBEDTLS_DIR, since Tests/MbedTlsIntegration/CMakeLists.txt builds mbedTLS from source via that env var). Wired into the summary job's needs list and the Quality Monitor tools table. Branch-protection rule on GitHub still needs the new "integration-linux-mbedtls" check added manually before this becomes a required status check.
Contributor
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR introduces a complete mbedTLS-backed TLS stream adapter for SolidSyslog. The implementation spans build configuration (CMakeLists, CI/CD integration), a public API contract for stream creation and configuration, core TLS stream operations with bounded handshake retry and BIO callbacks, fixed-size pool allocation, comprehensive unit tests using link-interposed mbedTLS fakes, and end-to-end integration tests with real mbedTLS libraries and dynamically generated certificates. ChangesMbedTLS Stream Adapter
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
☀️ Quality Summary 🚦 build-linux-gcc: 100% successful (✔️ 1286 passed, 🙈 2 skipped) Created by Quality Monitor v1.14.0 (#f3859fd). More details are shown in the GitHub Checks Result. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 130-143: The checkout step using actions/checkout (the step with
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd) should include
persist-credentials: false to avoid leaving GITHUB_TOKEN credentials in the
workspace; add that key under the same step. Also replace the test reporter
usage dorny/test-reporter@v3 with a pinned full 40-character commit SHA for the
action (keep the same step name "Test Report" and same inputs) so the workflow
references a fixed revision instead of a floating tag.
In `@Platform/MbedTls/Source/SolidSyslogMbedTlsStream.c`:
- Around line 73-87: The Open path currently leaves the transport/socket and
partially-initialized TLS state open when any post-transport step fails; modify
MbedTlsStream_Open so that if SolidSyslogStream_Open(self->Config.Transport,
addr) returns true but any subsequent call
(MbedTlsStream_ApplySslConfigDefaults, MbedTlsStream_ApplyTlsPolicy,
MbedTlsStream_BindContextToConfig, MbedTlsStream_ConfigureExpectedHostname,
MbedTlsStream_InstallTransportCallbacks, MbedTlsStream_PerformHandshake) fails,
you immediately unwind by closing the transport (call
SolidSyslogStream_Close(self->Config.Transport) or the equivalent
transport-close API) and undoing TLS setup (call the existing teardown/cleanup
routines for the mbedTLS context such as
MbedTlsStream_UninstallTransportCallbacks, MbedTlsStream_UnbindContextFromConfig
or the module’s cleanup function) before returning false; ensure every failure
branch after the initial open runs this cleanup so no socket or half-configured
TLS state is left behind.
In `@Platform/MbedTls/Source/SolidSyslogMbedTlsStreamPrivate.h`:
- Around line 17-18: The two external declarations use the file-static helper
prefix; either rename them to the public-repo C symbol pattern
SolidSyslogMbedTlsStream_Initialise and SolidSyslogMbedTlsStream_Cleanup so they
follow the project convention and MISRA naming rules, or if they are only used
within the same translation unit make the functions static and keep the current
names; update any callers to use the new SolidSyslogMbedTlsStream_* names if you
choose renaming.
In `@Tests/MbedTlsIntegration/MbedTlsTestCert.c`:
- Around line 114-120: The call to mbedtls_x509write_crt_der can fail but its
non-positive return is ignored, leaving out->StartOffset/Length zero and causing
MbedTlsTestCert_Create to pass a zero-length buffer to
mbedtls_x509_crt_parse_der; change the handling after mbedtls_x509write_crt_der
to REQUIRE/ASSERT/CHECK that written > 0 (or if using CppUTest use CHECK(written
> 0)) and on failure return an error or fail the test immediately instead of
proceeding, ensuring the failure is surfaced (update the branch that currently
sets out->StartOffset and out->Length to only run when written > 0 and add the
assertion/early-return when written <= 0).
- Around line 97-98: Extract the hard-coded validity strings passed to
mbedtls_x509write_crt_set_validity into named test constants (e.g.
TEST_CERT_VALIDITY_FROM and TEST_CERT_VALIDITY_TO) declared at the top of this
TU (Tests/MbedTlsIntegration/MbedTlsTestCert.c) so the fake test window is
self-documented and single-source; replace the inline "20240101000000" and
"20990101000000" arguments in the mbedtls_x509write_crt_set_validity(&crt, ...,
...) call with those constants and ensure the constant names use the TEST_*
prefix per the guideline.
In `@Tests/MbedTlsIntegration/SolidSyslogMbedTlsStreamIntegrationTest.cpp`:
- Around line 23-25: Rename the anonymous-namespace test constants
kServerHostname, kCaSubject, and kServerSubject to the TEST_* convention (e.g.
TEST_SERVER_HOSTNAME, TEST_CA_SUBJECT, TEST_SERVER_SUBJECT) and update all uses
in setup() and BuildBaseConfig() to reference the new names; ensure the new
identifiers replace the old ones everywhere in this test file so the fixture and
config builders use the TEST_* constants.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1b4b0642-4fb2-4c17-9341-791d9581bdc1
📒 Files selected for processing (27)
.github/workflows/ci.yml.vscode/c_cpp_properties.jsonCMakeLists.txtCore/Interface/SolidSyslogTunablesDefaults.hCore/Source/SolidSyslogErrorMessages.hPlatform/MbedTls/CMakeLists.txtPlatform/MbedTls/Interface/SolidSyslogMbedTlsStream.hPlatform/MbedTls/Source/SolidSyslogMbedTlsStream.cPlatform/MbedTls/Source/SolidSyslogMbedTlsStreamPrivate.hPlatform/MbedTls/Source/SolidSyslogMbedTlsStreamStatic.cTests/CMakeLists.txtTests/MbedTls/CMakeLists.txtTests/MbedTls/SolidSyslogMbedTlsStreamPoolTest.cppTests/MbedTls/SolidSyslogMbedTlsStreamTest.cppTests/MbedTls/main.cppTests/MbedTlsIntegration/CMakeLists.txtTests/MbedTlsIntegration/MbedTlsTestCert.cTests/MbedTlsIntegration/MbedTlsTestCert.hTests/MbedTlsIntegration/MbedTlsTestServer.cTests/MbedTlsIntegration/MbedTlsTestServer.hTests/MbedTlsIntegration/SocketStream.cTests/MbedTlsIntegration/SocketStream.hTests/MbedTlsIntegration/SolidSyslogMbedTlsStreamIntegrationTest.cppTests/MbedTlsIntegration/main.cppTests/Support/CMakeLists.txtTests/Support/MbedTlsFake.cTests/Support/MbedTlsFake.h
Comment on lines
+130
to
+143
| - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 | ||
|
|
||
| - name: Configure | ||
| run: cmake --preset debug | ||
|
|
||
| - name: Build integration tests | ||
| run: cmake --build --preset debug --target MbedTlsIntegrationTests | ||
|
|
||
| - name: Run integration tests | ||
| run: cd build/debug && ./Tests/MbedTlsIntegration/MbedTlsIntegrationTests -v -ojunit -k MbedTlsIntegrationTests | ||
|
|
||
| - name: Test Report | ||
| uses: dorny/test-reporter@v3 | ||
| if: success() || failure() |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify checkout hardening + floating refs in this workflow file.
python - <<'PY'
from pathlib import Path
import re
p = Path(".github/workflows/ci.yml")
lines = p.read_text().splitlines()
# Find checkout steps missing persist-credentials: false nearby
missing = []
for i, line in enumerate(lines):
if re.search(r'^\s*-\s+uses:\s+actions/checkout@', line):
window = lines[i+1:i+8]
if not any(re.search(r'^\s*persist-credentials:\s*false\s*$', w) for w in window):
missing.append(i + 1)
# Find floating dorny/test-reporter refs
floating = []
for i, line in enumerate(lines):
m = re.search(r'uses:\s*dorny/test-reporter@(.+)$', line.strip())
if m and not re.fullmatch(r'[0-9a-f]{40}(?:\s+#.*)?', m.group(1)):
floating.append((i + 1, m.group(1)))
print("checkout_without_persist_credentials_false:", missing)
print("floating_test_reporter_refs:", floating)
PYRepository: DavidCozens/solid-syslog
Length of output: 390
Harden workflow steps by disabling checkout credential persistence and pinning the test-reporter action to a commit SHA.
- Add
persist-credentials: falsetoactions/checkoutsteps missing it (including the checkout step around line 130). - Replace all
dorny/test-reporter@v3usages with a full 40-char commit SHA (including line 142).
🔒 Suggested patch
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6
+ with:
+ persist-credentials: false
@@
- name: Test Report
- uses: dorny/test-reporter@v3
+ uses: dorny/test-reporter@<full-40-char-commit-sha> # v3🧰 Tools
🪛 zizmor (1.25.2)
[warning] 130-130: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 142-142: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml around lines 130 - 143, The checkout step using
actions/checkout (the step with uses:
actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd) should include
persist-credentials: false to avoid leaving GITHUB_TOKEN credentials in the
workspace; add that key under the same step. Also replace the test reporter
usage dorny/test-reporter@v3 with a pinned full 40-character commit SHA for the
action (keep the same step name "Test Report" and same inputs) so the workflow
references a fixed revision instead of a floating tag.
Owner
Author
There was a problem hiding this comment.
Deferred — captured the action items in memory as a CI hardening chore to run on return to main. The same gaps exist on every other actions/checkout and dorny/test-reporter step in this workflow (see the file overall), so a per-PR partial fix would be inconsistent with the surrounding pattern. Will land as a focused chore PR that sweeps the whole workflow file, picking the test-reporter SHA from the latest v3.x release at that time.
Contributor
There was a problem hiding this comment.
@DavidCozens — understood, that's a sensible approach. Doing a whole-file sweep in a single chore PR is much cleaner than a piecemeal fix that leaves all the sibling steps inconsistent.
Would you like me to open a GitHub issue to track the CI hardening chore? I can scope it to:
- Add
persist-credentials: falseto allactions/checkoutsteps in.github/workflows/ci.yml. - Pin all
dorny/test-reporter@v3references to the latest v3.x commit SHA at the time of the chore.
Just let me know and I'll create it now.
🧠 Learnings used
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 156
File: .github/workflows/ci.yml:0-0
Timestamp: 2026-04-18T17:13:24.155Z
Learning: In GitHub Actions workflows that run on `windows-2025` runners, avoid using PowerShell `Start-Process` to launch background/daemon-style processes from CI steps. `Start-Process` is unreliable on these runners (the child may not start or may be silently killed), which can leave output/redirect files empty and services/ports not bound.
Use a Bash backgrounding approach instead: `nohup … &` (Git Bash is the default shell on `windows-2025`). For example:
`nohup ./Bdd/otel/bin/otelcol-contrib.exe --config=Bdd/otel/config.yaml > Bdd/output/otelcol.out 2> Bdd/output/otelcol.err &`
Prefer `nohup … &` over `Start-Process` for any background process launched in a CI step on `windows-2025`.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 261
File: .github/workflows/ci.yml:580-580
Timestamp: 2026-05-04T09:20:07.939Z
Learning: In GitHub Actions workflows (notably `quality-monitorv1`), the Quality Monitor only allows `analysis` entries whose `id` matches a parser registry that is bundled/registered by that action version. If `quality-monitorv1` does not register the `iwyu` parser, do not add an `analyze-iwyu`/`analysis` block with `{"id":"iwyu"}`—it will fail at runtime with `No such parser registered: iwyu`. IWYU artifacts may still be downloadable for archival, but the reliable pass/fail gate for IWYU must come from the job-level check that branch protection requires (i.e., the required check’s outcome), not from an `analysis` parser block.
Comment on lines
+73
to
+87
| static inline bool MbedTlsStream_Open(struct SolidSyslogStream* base, const struct SolidSyslogAddress* addr) | ||
| { | ||
| struct SolidSyslogMbedTlsStream* self = MbedTlsStream_SelfFromBase(base); | ||
| bool ok = SolidSyslogStream_Open(self->Config.Transport, addr) && MbedTlsStream_ApplySslConfigDefaults(self); | ||
| if (ok) | ||
| { | ||
| MbedTlsStream_ApplyTlsPolicy(self); | ||
| ok = MbedTlsStream_BindContextToConfig(self) && MbedTlsStream_ConfigureExpectedHostname(self); | ||
| } | ||
| if (ok) | ||
| { | ||
| MbedTlsStream_InstallTransportCallbacks(self); | ||
| ok = MbedTlsStream_PerformHandshake(self); | ||
| } | ||
| return ok; |
Contributor
There was a problem hiding this comment.
Unwind the transport and TLS state when Open fails.
Line 76 opens the underlying transport before the TLS setup steps, but any later failure just returns false. That leaves the transport open and the TLS objects half-configured, so retries can start from stale state and leak the socket until destroy.
Suggested fix
static inline bool MbedTlsStream_Open(struct SolidSyslogStream* base, const struct SolidSyslogAddress* addr)
{
struct SolidSyslogMbedTlsStream* self = MbedTlsStream_SelfFromBase(base);
- bool ok = SolidSyslogStream_Open(self->Config.Transport, addr) && MbedTlsStream_ApplySslConfigDefaults(self);
+ bool transportOpened = SolidSyslogStream_Open(self->Config.Transport, addr);
+ bool ok = transportOpened && MbedTlsStream_ApplySslConfigDefaults(self);
if (ok)
{
MbedTlsStream_ApplyTlsPolicy(self);
ok = MbedTlsStream_BindContextToConfig(self) && MbedTlsStream_ConfigureExpectedHostname(self);
}
@@
{
MbedTlsStream_InstallTransportCallbacks(self);
ok = MbedTlsStream_PerformHandshake(self);
}
+ if (transportOpened && !ok)
+ {
+ MbedTlsStream_Close(base);
+ }
return ok;
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Platform/MbedTls/Source/SolidSyslogMbedTlsStream.c` around lines 73 - 87, The
Open path currently leaves the transport/socket and partially-initialized TLS
state open when any post-transport step fails; modify MbedTlsStream_Open so that
if SolidSyslogStream_Open(self->Config.Transport, addr) returns true but any
subsequent call (MbedTlsStream_ApplySslConfigDefaults,
MbedTlsStream_ApplyTlsPolicy, MbedTlsStream_BindContextToConfig,
MbedTlsStream_ConfigureExpectedHostname,
MbedTlsStream_InstallTransportCallbacks, MbedTlsStream_PerformHandshake) fails,
you immediately unwind by closing the transport (call
SolidSyslogStream_Close(self->Config.Transport) or the equivalent
transport-close API) and undoing TLS setup (call the existing teardown/cleanup
routines for the mbedTLS context such as
MbedTlsStream_UninstallTransportCallbacks, MbedTlsStream_UnbindContextFromConfig
or the module’s cleanup function) before returning false; ensure every failure
branch after the initial open runs this cleanup so no socket or half-configured
TLS state is left behind.
Owner
Author
There was a problem hiding this comment.
Deferred — filed as S26.02 (#420) under E26 (TLS Hardening). The same chained-&& shape exists in Platform/OpenSsl/Source/SolidSyslogTlsStream.c TlsStream_Open (line 120-125), and the inner-transport PosixTcpStream_Open overwrites self->Fd without first closing the old one — so the symptom you describe (leak + stale state on retry) is real but already present across the layered-adapter family. Fixing it only in the mbedTLS adapter would be asymmetric; S26.02 sweeps both adapters and audits the inner-transport Opens in one go.
Contributor
There was a problem hiding this comment.
@DavidCozens — Understood, that's a well-reasoned call. The asymmetry concern is valid: a partial fix in only the mbedTLS adapter would create a misleading consistency illusion while the OpenSSL path and PosixTcpStream_Open still carry the same hazard. Tracking it as S26.02 (#420) with a family-wide audit is the right scope.
I'll note the tracking issue for future reviews on these files.
🧠 Learnings used
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 283
File: Platform/OpenSsl/Source/SolidSyslogTlsStream.c:363-363
Timestamp: 2026-05-07T08:22:59.856Z
Learning: In this solid-syslog codebase, follow the documented contract: do not add NULL/defensive checks inside library/internal code paths (e.g., `_Create` functions) when the NULL case cannot occur under normal internal usage and is covered by framework or internal wiring guarantees. Only add validation at true system boundaries such as user input and external APIs. Where setup-time wiring contracts between the integrator and the library require specific function-pointer fields (e.g., `SolidSyslogTlsStreamConfig.sleep`), assume those required fields are present and intentionally omit NULL guards in internal creation functions.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 291
File: Example/FreeRtos/HelloWorld/CmsdkUart.c:17-42
Timestamp: 2026-05-08T17:19:38.382Z
Learning: In this codebase (DavidCozens/solid-syslog), internal driver/config/access structs may intentionally store raw pointers without NULL checks, `isInitialized` flags, or defensive struct copies. Treat this as an explicit project policy: do not flag store-by-pointer usage or missing NULL/initialization guards for these internal config/access structs when the pointer is provided by production callers and is guaranteed to point to `static const` objects with program lifetime (per CLAUDE.md). Only flag NULL/initialization issues when the pointer originates from potentially invalid/external/untrusted sources or when the code does not establish lifetime/guarantees.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 303
File: Example/Common/ExampleInteractive.h:14-17
Timestamp: 2026-05-09T15:56:13.853Z
Learning: During code review, ensure solid-syslog Tier 1/2 library callbacks in Core/ and Platform/ follow the CLAUDE.md “Callback Conventions”: the callback API must take a `void*` context parameter and the implementation must use the paired configuration field associated with that callback context. Do not apply this requirement to Tier 3 example handlers (e.g., Example/Common/, Example/FreeRtos/, such as `ExampleInteractiveSwitchHandler`/`ExampleInteractiveSetHandler`), which are allowed to use file-static globals per the documented example-tier pattern. If a multi-instance use-case arises, migrate Switch and Set together in a single cohesive change rather than piecemeal updates.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 372
File: Platform/FreeRtos/Source/SolidSyslogFreeRtosDatagram.c:0-0
Timestamp: 2026-05-15T13:09:11.811Z
Learning: When reviewing PRs that bulk-rename identifiers (especially “static function rename” changes) in this repo, check for unintended renames inside FreeRTOS macro usages. A too-broad regex can incorrectly prefix macro tokens used in `static const` initializers (e.g., `pdMS_TO_TICKS` from FreeRTOS). Look for signs like a renamed macro name being incorrectly prefixed (e.g., `FreeRtosDatagram_pdMS_TO_TICKS`), and if present, treat it as evidence the identifier-extraction regex was overly broad and requires correction/revert of the affected rename changes.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 400
File: Core/Source/SolidSyslogOriginSdStatic.c:11-11
Timestamp: 2026-05-18T17:07:19.499Z
Learning: In this repo (DavidCozens/solid-syslog), an IWYU CI gate enforces strict direct-header inclusion: for every symbol (types, enums, functions, etc.) that is directly referenced in a translation unit’s `.c` or `.h` file, include the specific header that declares that symbol. Do not rely on transitive includes from other headers.
Example: if a `.c`/`.h` file uses `SolidSyslogSeverity` enums, it must directly `#include` `SolidSyslogPrival.h` even if another included header (e.g., `SolidSyslogError.h`) pulls it in transitively.
When reviewing, do not flag or recommend removing includes solely because they are “transitive” in this codebase—removing them may still compile locally but can fail the IWYU CI check.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 411
File: Platform/FreeRtos/Source/SolidSyslogFreeRtosDatagramStatic.c:42-50
Timestamp: 2026-05-20T12:23:42.200Z
Learning: In the solid-syslog (DavidCozens) codebase, review pool-backed `_Destroy` implementations to ensure they *uniformly* warn on any handle not present in the relevant pool (the established E11 pattern). This intentionally covers: (1) stale/already-destroyed handles, (2) never-issued handles, and (3) the shared-null-sentinel returned on pool exhaustion. Do not ask for a special-case early-exit/silent no-op when the handle equals the shared null sentinel—integrators already receive an error at `_Create` time on pool exhaustion, and the `_Destroy` warning is the deliberate secondary safety net. If a change to this behavior is proposed (e.g., suppressing warnings / changing null-sentinel semantics), it must be applied epic-wide and consistently across all migrated classes/platform variants, not as a per-class or per-platform deviation.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 416
File: DEVLOG.md:155-155
Timestamp: 2026-05-20T17:56:08.543Z
Learning: When a source file needs the public facility/severity enums used by `SolidSyslog_Error` (or other public APIs), include the public header `Core/Interface/SolidSyslogPrival.h` rather than `Core/Source/SolidSyslogPrivate.h`. Reserve `Core/Source/SolidSyslogPrivate.h` (TU-internal structs like `SolidSyslog`) for the translation unit that defines/uses them (e.g., `SolidSyslogStatic.c`).
Comment on lines
+17
to
+18
| void MbedTlsStream_Initialise(struct SolidSyslogStream* base, const struct SolidSyslogMbedTlsStreamConfig* config); | ||
| void MbedTlsStream_Cleanup(struct SolidSyslogStream* base); |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Rename these external-linkage helpers to the repo’s C symbol pattern.
These declarations have external linkage but use the file-static helper prefix. Prefer SolidSyslogMbedTlsStream_Initialise / SolidSyslogMbedTlsStream_Cleanup, or make them static if they never need cross-TU linkage.
As per coding guidelines, **/*.{c,h} requires Public C functions named SolidSyslogClass_Function and to Adhere to MISRA C:2012 rules 5.1–5.9 for naming uniqueness and shape.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Platform/MbedTls/Source/SolidSyslogMbedTlsStreamPrivate.h` around lines 17 -
18, The two external declarations use the file-static helper prefix; either
rename them to the public-repo C symbol pattern
SolidSyslogMbedTlsStream_Initialise and SolidSyslogMbedTlsStream_Cleanup so they
follow the project convention and MISRA naming rules, or if they are only used
within the same translation unit make the functions static and keep the current
names; update any callers to use the new SolidSyslogMbedTlsStream_* names if you
choose renaming.
Owner
Author
There was a problem hiding this comment.
Declined — the pattern mirrors the established convention used across every layered-adapter pool class. MbedTlsStream_Initialise / MbedTlsStream_Cleanup are cross-TU contract between SolidSyslogMbedTlsStream.c and SolidSyslogMbedTlsStreamStatic.c, declared in the TU-private *Private.h header — so they can't be static. Renaming to SolidSyslogMbedTlsStream_* would conflict with the project's two-tier convention: SolidSyslogClass_Function is reserved for the public API (_Create, _Destroy) that integrators call. Private helpers within a class use the bare Class_Function form. The identical shape exists in Platform/OpenSsl/Source/SolidSyslogTlsStreamPrivate.h (TlsStream_Initialise / TlsStream_Cleanup) and several other adapters under Platform/.
Contributor
There was a problem hiding this comment.
@DavidCozens, understood — that's a clear and consistent two-tier convention: SolidSyslogClass_Function for the public integrator API and Class_Function for private cross-TU helpers exposed only through *Private.h. I'll record this to avoid raising the same concern in future reviews.
(ᐟ\ \ᐟ)
✏️ Learnings added
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 419
File: Platform/MbedTls/Source/SolidSyslogMbedTlsStreamPrivate.h:17-18
Timestamp: 2026-05-21T12:53:11.114Z
Learning: In DavidCozens/solid-syslog, a deliberate two-tier C naming convention is used for Platform adapter classes:
1. `SolidSyslogClass_Function` (full prefix) — reserved exclusively for the **public** integrator-facing API that appears in the public Interface header (e.g., `SolidSyslogMbedTlsStream_Create`, `SolidSyslogMbedTlsStream_Destroy`).
2. `Class_Function` (bare/short prefix) — used for **private cross-TU helper functions** that are shared between the two `.c` files of a single adapter (e.g., `MbedTlsStream_Initialise`, `MbedTlsStream_Cleanup`, `TlsStream_Initialise`, `TlsStream_Cleanup`). These are declared in the TU-private `*Private.h` header and have external linkage (cannot be `static`) because they cross translation-unit boundaries, but are intentionally not part of the public API. The precedent is `Platform/OpenSsl/Source/SolidSyslogTlsStreamPrivate.h` lines 19-20. Do NOT flag `Class_Function`-style names in `*Private.h` headers as violating naming conventions — this is the correct, established pattern.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 283
File: Platform/OpenSsl/Interface/SolidSyslogTlsStream.h:7-7
Timestamp: 2026-05-07T08:22:53.110Z
Learning: In solid-syslog, the CLAUDE.md callback rule that requires a `void* context` parameter (and the paired config field) should be applied only to callbacks that need to carry real per-instance state. Exempt callbacks that are stateless thin wrappers over OS primitives—i.e., the callback is effectively a pure function of its input parameters with no instance state to preserve—so adding `void* context` would be pointless (the context would always be NULL). For example, platform implementations like `SolidSyslogSleepFunction` that wrap `nanosleep`/`Sleep` and are defined purely in terms of `int milliseconds` should not add `void* context` or the paired config field. For callbacks that do depend on per-instance data, include `void* context` and the paired config field as required by CLAUDE.md.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 303
File: Example/Common/ExampleInteractive.h:14-17
Timestamp: 2026-05-09T15:56:13.853Z
Learning: During code review, ensure solid-syslog Tier 1/2 library callbacks in Core/ and Platform/ follow the CLAUDE.md “Callback Conventions”: the callback API must take a `void*` context parameter and the implementation must use the paired configuration field associated with that callback context. Do not apply this requirement to Tier 3 example handlers (e.g., Example/Common/, Example/FreeRtos/, such as `ExampleInteractiveSwitchHandler`/`ExampleInteractiveSetHandler`), which are allowed to use file-static globals per the documented example-tier pattern. If a multi-instance use-case arises, migrate Switch and Set together in a single cohesive change rather than piecemeal updates.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 400
File: Core/Source/SolidSyslogOriginSdStatic.c:11-11
Timestamp: 2026-05-18T17:07:19.499Z
Learning: In this repo (DavidCozens/solid-syslog), an IWYU CI gate enforces strict direct-header inclusion: for every symbol (types, enums, functions, etc.) that is directly referenced in a translation unit’s `.c` or `.h` file, include the specific header that declares that symbol. Do not rely on transitive includes from other headers.
Example: if a `.c`/`.h` file uses `SolidSyslogSeverity` enums, it must directly `#include` `SolidSyslogPrival.h` even if another included header (e.g., `SolidSyslogError.h`) pulls it in transitively.
When reviewing, do not flag or recommend removing includes solely because they are “transitive” in this codebase—removing them may still compile locally but can fail the IWYU CI check.
5 tasks
Three accepts from CodeRabbit's first-pass review: - Tests/MbedTlsIntegration/MbedTlsTestCert.c: extract validity strings into TEST_CERT_VALIDITY_FROM / _TO file-scope constants per the Tests/**/TEST_* convention; add an assert(written > 0) after mbedtls_x509write_crt_der so debug builds fail loudly instead of passing an empty DER to mbedtls_x509_crt_parse_der and presenting as a downstream TLS error. The existing if(written > 0) guard stays so release builds with NDEBUG remain UB-safe. - Tests/MbedTlsIntegration/SolidSyslogMbedTlsStreamIntegrationTest.cpp: rename kServerHostname / kCaSubject / kServerSubject to TEST_SERVER_HOSTNAME / TEST_CA_SUBJECT / TEST_SERVER_SUBJECT to match the existing TEST_HOST pattern in SolidSyslogStreamSenderTest.cpp. Three remaining findings are tracked elsewhere: - CI workflow hardening (persist-credentials + dorny SHA pin) — sweep across the whole .github/workflows/ci.yml on return to main; partial fix on just the new job would have been inconsistent. - Layered-adapter Open() unwind on failure — filed as S26.02 (#420) under E26. Same shape exists in TlsStream; fix both in one go. - MbedTlsStream_Initialise / _Cleanup naming declined; the pattern mirrors TlsStream_Initialise / _Cleanup in Platform/OpenSsl and is cross-TU contract between the adapter and *Static.c, not public API.
Contributor
☀️ Quality Summary 🚦 build-linux-gcc: 100% successful (✔️ 1286 passed, 🙈 2 skipped) Created by Quality Monitor v1.14.0 (#f3859fd). More details are shown in the GitHub Checks Result. |
This was referenced May 25, 2026
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.
Summary
First PR for S08.07 (#272). Lands the host-side
SolidSyslogMbedTlsStreamadapter, the full unit-test suite against aMbedTlsFake, an in-process real-libmbedtls integration test exec, and theintegration-linux-mbedtlsCI job. The FreeRTOS QEMU BDD wiring (slice 6) and integrator-facing docs (slice 7) are deferred to follow-up PRs.What's covered (acceptance-criteria mapping for #272)
Platform/MbedTls/shipsSolidSyslogMbedTlsStreamas documented (Open / Send / Read / Close, vtable, E11 three-TU split with NullStream fallback, INTERFACE CMake,SOLIDSYSLOG_MBED_TLS_STREAM_POOL_SIZEtunable).Platform/MbedTls/Source/: zerombedtls_platform_setup,psa_crypto_init,mbedtls_threading_set_alt, debug-callback installation, or integrator-owned_init/_free/_parse.MbedTlsFakecover the vtable, pool exhaustion, bounded handshake retry budget, all error paths, BIO would-block translation, and the lifecycle invariants (idempotent Close, double-Close safety, fail-fast Send/Read).integration-linux-mbedtlsCI job green — real libmbedtls handshake against an in-process TLS server. Covers happy-path server-auth, hostname-mismatch reject, untrusted-CA reject, mTLS happy path, mTLS missing-client-cert reject, mTLS untrusted-client-CA reject.tls_transport.feature/mtls_transport.feature/tcp_reconnect.feature— deferred to slice 6.docs/integrating-mbedtls.md, CLAUDE.md row,docs/iec62443.md— deferred to slice 7.Bugs fixed during slice 5 + cleanup pass
BioRecvwould-block now returnsMBEDTLS_ERR_SSL_WANT_READinstead of treating transport-0 as a hard error (was a real bug under non-blocking transports; integration tests now exercise it).SendandReadclose the SSL session + transport on any hard error, matching the OpenSSL TlsStream fail-fast contract.IsOpenordering hazard removed by dropping the flag — eagermbedtls_ssl_init/_config_initin Create makes_freein Close always safe.MbedTlsTestCert::strncpy(banned by SDL) replaced withSafeString.SocketStream_Readno longer conflates EOF (recv == 0) with would-block; was a latent landmine after theBioRecvfix.Refs #272.
Test plan
cmake --build --preset debug --target SolidSyslogMbedTlsStreamTest SolidSyslogMbedTlsStreamPoolTest MbedTlsIntegrationTests— greensanitizepreset (ASan + UBSan) — greentidypreset compiles all three test targets cleanclang-format --dry-run -Werrorclean on all touched filesanalyze-iwyu— not runnable infreertos-hostcontainer, defers to CIintegration-linux-mbedtls— newly added; first run on this PRFollow-ups (separate PRs)
BddTargetTlsSender_MbedTls_FreeRtosTcp.c, bake demo PEMs into the ELF viaxxd -i, initmbedtls_entropy_context+ register weak entropy source, seedmbedtls_ctr_drbg, gettls_transport.feature/mtls_transport.feature/tcp_reconnect.featuregreen on QEMU.docs/integrating-mbedtls.md(two-scenario integrator guide), header coexistence summary, CLAUDE.md row,docs/iec62443.mdupdate.Branch-protection note
The new
integration-linux-mbedtlscheck appears as a status check but isn't a required check until added to GitHub branch protection rules.Summary by CodeRabbit
New Features
Developer Experience