Production-hardening + GUI enhancement program (28 items) + 2 security fixes - #19
Merged
Conversation
… ActionDispatcher's runner
…mulatedRemoteBackend
Add a fourth optional IAuthorizer hook, authorizeRegister(ctx, modelType), consulted by RemoteServer on every register envelope after authenticating the caller and before constructing the instance. Defaults to allow, so AllowAllAuthorizer and a plain SigningAuthorizer keep today's behavior; a deployer opts in to bound who may create model instances.
Add morph::backend::detail::OpaqueIdGenerator: a 4-round keyed Feistel permutation over the 64-bit space, keyed once from std::random_device at RemoteServer construction. RemoteServer now assigns ids by running its monotonic counter through the generator instead of handing out _nextId.fetch_add(1) + 1 directly, so ids stay unique (Feistel networks are bijections) but are no longer sequential or predictable from an observed id. Wire format is unchanged (ModelId stays a plain uint64_t).
authorizeRegister and opaque model ids are now implemented (RemoteServer enforces both, both opt-in/backward-compatible). Fold docs/planned/instance_authorization.md's design into docs/spec/session/session.md, docs/spec/core/backend.md, and docs/spec/security.md in present tense, and delete the planned file per project convention (only docs/spec/ holds specs for implemented work).
…cement Adds the opt-in LimitPolicy struct (executeTimeout, maxLiveModels, maxInFlightExecutes) and RemoteServer::setLimitPolicy(), and enforces maxLiveModels in the register branch of dispatchMessage. All fields default to 0 (unbounded), so an unconfigured server is unaffected.
…rcement Adds an atomic in-flight-execute counter, incremented before dispatchExecute posts to the strand and decremented right before the reply is sent (success or exception). A caller over maxInFlightExecutes gets err "server busy" before any authorization work runs.
…+ TimeoutError
Adds morph::backend::TimeoutError (alongside DisconnectedError et al.) and a
lazily-started detail::TimeoutScheduler background thread that enforces
LimitPolicy::executeTimeout in dispatchExecute. A shared once-flag guard
("complete") ensures the timeout path and the strand path race safely and
handle()'s reply-exactly-once contract holds; the losing side's result is
discarded and the in-flight counter is decremented exactly once regardless of
which path wins. SimulatedRemoteBackend and QtWebSocketBackend both translate
an err "timeout" reply into TimeoutError for callers.
…eBytes Introduces morph::qt::QtWebSocketServerConfig (aliased QtWebSocketServer::Config) threaded through a new constructor parameter, defaulting to unbounded except maxMessageBytes which defaults to wire::kMaxEnvelopeBytes (today's only bound). onNewConnection refuses a connection beyond maxConnections before wiring any signal; onTextMessage rejects an oversized frame before it reaches RemoteServer::handle().
… rate limit _clients becomes a QWebSocket* -> ClientState map holding a per-connection token bucket (capacity = messagesPerSecond, continuous refill). A frame that finds an empty bucket is dropped silently, not replied to or closed, so a single burst does not take down an otherwise well-behaved client.
A one-shot per-connection QTimer enforces handshakeTimeout (cancelled on the first frame); a shared ~1s housekeeping QTimer sweeps _clients and closes any connection idle past idleTimeout. Both default to 0 (disabled), reproducing today's behavior. Verified against the Doxygen WARN_AS_ERROR doc-completeness gate (cmake --build build --target doc, exit 0, no warnings).
…etire planned spec Documents the now-implemented RemoteServer::LimitPolicy and QtWebSocketServerConfig resource limits in present tense across docs/spec/core/backend.md (new LimitPolicy section, updated QtWebSocketServer section, Error types/API reference/Design decisions tables), docs/spec/security.md (hardening checklist + Testing section), and docs/spec/core/wire.md (cross-reference). Retires the A3 todo.md entry and deletes docs/planned/transport_limits.md now that the design has landed.
…helpers Header-only QSslConfiguration factory helpers in qt_tls.hpp make CA-verified and pinned-certificate TLS the documented, easy path for QtWebSocketBackend, alongside a clearly-named insecure escape hatch for local dev/tests.
server.crt now carries subjectAltName=IP:127.0.0.1 so Qt's hostname check (which matches literal-IP peers only against SAN entries, never the CN) passes for VerifyPeer-mode tests. mitm.crt/mitm.key is a second, independent self-signed pair standing in for an attacker who cannot present the real server's private key.
…a mitm Adds three integration tests exercising the full TLS handshake path: a pinned client reaches the real server, is refused by a server presenting a different (mitm) certificate, and tlsInsecureNoVerify connects to both -- demonstrating exactly why VerifyNone is MITM-vulnerable.
… plaintext binds Extends the QtWebSocketServerConfig struct (already carrying A3's maxConnections/maxMessageBytes/messagesPerSecond/handshakeTimeout/idleTimeout) with bindAddress and allowPlaintextExposure. listen() now refuses a non-loopback bind with no TLS configuration and allowPlaintextExposure left at its default false, logging at error level instead of silently starting a plaintext, off-host-reachable server. Loopback binds and any TLS-configured bind are unaffected -- fully backward compatible.
morph_qt_tls_example runs a real server (certs/server.crt) and a stand-in mitm server (certs/mitm.crt) on loopback, then demonstrates tlsPinnedConfig() reaching the real server and being refused by the mitm one, contrasted with tlsInsecureNoVerify() connecting to both. Registered as the qt_tls_example_runs ctest.
…the planned spec Documents the now-implemented qt_tls.hpp helpers (tlsVerifyingConfig/ tlsPinnedConfig/tlsInsecureNoVerify) and QtWebSocketServerConfig's exposure guard (bindAddress/allowPlaintextExposure) in present tense across docs/spec/security.md (Transport security, hardening checklist, Testing) and docs/spec/core/backend.md (QtWebSocketBackend/QtWebSocketServer prose, new QtWebSocketServerConfig API reference table). Retires the A4 todo.md entry and deletes docs/planned/tls_peer_verification.md now that the design has landed.
openConnection()/closeConnection() give a transport a way to scope model registrations to one connection. This step only adds the bookkeeping and lifecycle; register/deregister are not wired to it yet.
…d handle overload register recorded under a non-zero cid now joins that connection's scope (next to _models/_owners, under the same _regMtx); deregister removes it from the scope too, so closeConnection is idempotent after an explicit deregister. The unscoped two-argument handle()/handleInline() are unchanged (cid defaults to 0). closeConnection deliberately bypasses IAuthorizer.
QtWebSocketServer now opts every client into RemoteServer's connection scope: openConnection() on accept, the scoped handle(msg, reply, cid) on every frame, and closeConnection() on disconnect and on close(). A client crash or dropped socket now reclaims its models instead of leaking them until process exit. ClientState (added by the sibling transport-limits plan) grows a `cid` field rather than introducing a separate ClientConnection wrapper, since _clients is already keyed by socket pointer.
…spec RemoteServer's ConnectionId scope and QtWebSocketServer's end-to-end wiring are now implemented; backend.md documents them as current, present-tense behavior (API reference, design decisions, Limitations, and the QtWebSocketBackend client-side deregisterModel note) and the superseded docs/planned/connection_scoped_cleanup.md is removed. Retires the A7 todo.md entry, matching the convention the three sibling plans used for their own planned-spec retirements.
…AC gates the default mac Deviates from the plan's try_compile() invocation: LINK_LIBRARIES only accepts imported targets from the calling project, and morph/glaze::glaze are plain (non-imported) INTERFACE libraries here, so linking morph::morph directly fails with "the target was not found". Fixed by forwarding their INTERFACE_INCLUDE_DIRECTORIES via -DINCLUDE_DIRECTORIES= CMAKE_FLAGS instead, which try_compile's generated project turns into include_directories().
Deviates from the plan's CMakeLists.txt content: this directory is added
via MORPH_BUILD_EXAMPLES, which is processed before the top-level
MORPH_BUILD_TESTS block that resolves Catch2, so Catch2's `Catch` CMake
module isn't yet on CMAKE_MODULE_PATH when include(Catch) runs here (the
Catch2::Catch2WithMain *target* itself still resolves fine since CMake
defers target_link_libraries() resolution). Fixed the same way
examples/bank/CMakeLists.txt already does: an independent
find_package(Catch2 3 CONFIG QUIET) plus list(APPEND CMAKE_MODULE_PATH
${Catch2_DIR}) before include(Catch), warning and skipping the test
executable if Catch2 CONFIG isn't found.
Applies the same Catch2-availability fix as the libsodium block (see prior commit): find_package(Catch2 3 CONFIG QUIET) + CMAKE_MODULE_PATH append before include(Catch), since this directory is processed before the top-level MORPH_BUILD_TESTS block resolves Catch2.
fuzz_wire_decode and fuzz_dispatch_execute both found real bugs on their first campaign run (logged as follow-up in testing_strategy.md's "Known findings"). Fix both now rather than leave them open: 1. skip_ws heap-buffer-overflow: every glz::read<> call site that accepts an arbitrary string_view (wire::decode, journal::fromJson, BRIDGE_REGISTER_ACTION's generated fromJson/resultFromJson, FileOfflineQueue::fromJson) left glaze's null_terminated option at its default (true), so skip_ws assumed a terminator byte a bare view never guarantees. A 5-byte input was enough to trip an ASan heap-buffer-overflow, reachable by any peer sending a handful of bytes to a RemoteServer. Fixed by setting .null_terminated = false at each site — session_auth.hpp's token-claims decode was checked and needs no change, since it parses a real std::string with the standard's null-terminator guarantee, not a view. 2. Unescaped control bytes broke the err-reply round-trip: glaze's writer escapes `"` and `\` but not ASCII control bytes, so an err reply echoing untrusted text containing one (an unrecognized Envelope::kind, or an exception's what()) served invalid JSON that glaze's own reader then rejected. Fixed in wire::makeErr, the single choke point every err reply goes through, by replacing control bytes with a printable \xHH placeholder before they reach the writer. Both original crashing inputs are preserved as permanent regression cases under tests/fuzz/findings/, and covered by dedicated Catch2 tests in test_wire_hardening.cpp. Verified via 10M+ and 3.7M fuzz runs respectively with zero further crashes.
…tures Adds examples/concepts/, one small self-contained Catch2 test file per recently-landed hardening feature (journal/outbox, offline queue durability, server-side validation, register authorization, transport limits, protocol versioning, connection scoping, observability, graceful shutdown), so newcomers can learn each concept's golden-path usage from a single focused file instead of the exhaustive tests/ suite. Wired into the root build unconditionally under MORPH_BUILD_EXAMPLES, matching examples/forms's treatment (no external dependency, unlike examples/vetted_hmac's opt-in crypto gating). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Yaraslaut
force-pushed
the
feat/header-subdomain-layout
branch
from
July 24, 2026 13:24
b480a21 to
06376c6
Compare
…ackage Doxygen 1.9.8 (through at least 1.14.0) misreports several correctly documented symbols as undocumented: two template overloads of the same free function, and a function immediately following a file-level doc comment both trip its param/return association. Verified via Docker against the exact CI environment that 1.15.0+ resolves it with zero source changes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- forms.md: fix 3 dead cross-references left over from the gui_overview.md fold-in, remove a stale "(reserved)" label on FieldMeta::widget that contradicted its documented behavior elsewhere, add the missing computedFields exclusion to the required-ness rule text (confirmed against forms.hpp's actual allRequiredEngaged implementation), and define "engaged"/"wire"/"Tier-1"/"Tier-2" at first use instead of assuming them. - choice.md/views.md/workflows_navigation.md: anchor bare forms.md links to their actual sections, add a concrete viewSchemaJson example, and reconcile the informal vs. formal "screen" terminology. - widget_hints.md: reuse the existing "infer by default, declare to override" wording instead of an undefined "umbrella program" paraphrase. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- README: add the missing lab_wizard.hpp file-table entry, a suggested reading order, and a Views/SamplesView walkthrough section (the feature was only reachable via the legacy Main.qml, unmentioned anywhere in the guided walkthrough). - lab_model.hpp: frame RecordMeasurement as the deliberate combined showcase it is, and explain the BRIDGE_REGISTER_*/Loggable::No and glz::json_schema-subset choices inline instead of leaving them unexplained. - lab_units.hpp: clarify that the consteval operators' `throw` is a compile-time-only error, never a runtime exception. - lab_schemas.hpp: note why schemasJson() is hand-unrolled rather than a loop. - main.cpp: point C++-API-focused readers past the embedded HTML/CSS/JS block to where the Forms API resumes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Make the CapturedReply comment, the Glaze file-scope-linkage rationale, and the "Reach for this when" banner consistent across all 7 files (previously only some had each). Replace a comment-only "ids are opaque" claim in register_authorization_and_ids.cpp with a real passing assertion. Label queue.enqueue's second argument as the idempotencyKey, comment LogEntry::principal on first appearance, note the glz::read_json truthy-means-error convention, and add a reading-order note to the README. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…cate() Several tests were failing against current, correct behavior rather than a regression: - test_repl.sh: ComputeDryDensity's expected precision was stale (dp:3) from before volume's declared-precision override (4) existed -- Rational::dividedBy is documented to take max(lhs.dp, rhs.dp), so dp:4 is correct. RecordMeasurement/DeleteSample's expected error messages assumed the model's own throw would run, but both actions have since gained a validate() member that morph::model::ActionValidator now intercepts first (the model-level throw is documented as defense-in-depth, not the primary gate -- see forms.md, "Security / trust boundary"). - bank tests (test_transaction/test_payee/test_card/test_account/ test_auth): five DTOs (Transfer, AddPayee, ChangePin, OpenAccount, RegisterUser) gained their own validate() implementing exactly the checks these tests probe, so morph::model::ValidationError now fires before the model's own bank::ValidationError would. Updated each REQUIRE_THROWS_AS to expect the framework-level type, with a comment explaining why. - test_remote.cpp: RemoteServer::dispatchExecute clears Context::principal before dispatch unless the installed IAuthorizer::authenticate() vouches for it (documented, deliberate hardening against passing an unverified client claim through as authoritative). NoCloseAuthorizer only overrode authorize(), so "olivia-remote" was being scrubbed server-side. Added an authenticate() override that trusts the client-supplied principal, appropriate for this test-only authorizer. Verified: full project build (all tests, examples, Qt/QML forms, bank + bank GUI, vetted_hmac, net, offline_sqlite) and complete serial ctest run now pass 886/886. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Yaraslaut
force-pushed
the
feat/header-subdomain-layout
branch
from
July 24, 2026 15:33
164fe25 to
4450a47
Compare
- remote.hpp: OpaqueIdGenerator's static_cast<uint32_t>(rd()) is flagged by GCC's -Wuseless-cast (std::random_device::result_type is literally uint32_t on this standard library), breaking every GCC-based CI job (gcc-debug, gcc-release, Qt6 WebSockets, Valgrind memcheck) with -Werror. The cast itself is a legitimate portability guard for a standard library where result_type differs, so keep it and suppress just this warning on real GCC (matching the existing precedent in rational.hpp), rather than removing it. - test_computed_fields.cpp: renamed its file-scope CFUnit enum (and every reference to it) to CFLineUnit. It collided with test_forms_conformance_corpus.cpp's own unrelated, differently-defined CFUnit -- both are file-scope (not anonymous-namespaced) in translation units linked into the same morph_tests binary, so this was a genuine ODR violation: two conflicting definitions of the same external-linkage symbol. The linker silently picked one definition for both call sites, which happened to work under clang but resolved to the wrong one under GCC, failing Conformance corpus: CFQuantityAlternatives with the wrong unit ids/decimals. Verified full builds and complete serial ctest runs green on both compilers: gcc-debug (757/757) and clang (886/886, full config with Qt/bank/net enabled). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Yaraslaut
force-pushed
the
feat/header-subdomain-layout
branch
from
July 24, 2026 15:53
4450a47 to
0e8ea62
Compare
…lity/registry) Two lines in bridge.hpp (installReconnectHandler, recomputeAll) were llvm-cov region-mapping artifacts -- a statement immediately following a closing brace loses its counter even though it always runs; moving each into its preceding scope (neither acquires a lock or has other side effects that make this unsafe) fixes the miscount without changing behavior. The other three gaps were real, if narrow, untested branches: - IModelHolder::onBackendChanged's base no-op body is unreachable through any real ModelHolder<Model> (which always overrides it), so a minimal stub subclass exercises it directly. - observability::detail::beginSpan's defensive fallback (traceOn set without a beginSpan hook) can only happen by reaching past setTraceSink's own invariant, which a test does deliberately. - registry.hpp's toJson/resultToJson glz::write_json error paths needed glaze's one natural write-time failure: a tagged variant whose active alternative is a null smart pointer. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EYoDXWApxfP8K5jQiaQ2LA
FileActionLog's constructor opened _file via fopen() before rebuilding the idempotencyKey dedup set from entries() -- but entries() throws on interior corruption, which is exactly the condition test_journal_format_versioning.cpp's mid-file-corruption test injects. When that throw fires, the constructor never completes, so the destructor never runs, and the already-open FILE* leaks for the rest of the process. On Windows this blocks the temp file's later removal (no unlink-while-open semantics), which is what showed up in CI as a timeout. Verified with a standalone fd-count probe: the old ordering leaks one fd on the throwing path, the new ordering (rebuild the dedup set first, since entries() reads through its own ifstream independent of _file; open _file only after that succeeds) leaks none. Confirmed against a from-source build of the pre-fix header on the same probe. test_file_offline_queue.cpp's idempotency-dedup test declared its FileOfflineQueue in the test case's outer scope and then called std::filesystem::remove() on its backing file while the queue -- and its open file handle -- was still alive, unlike every other test in the file. Harmless on POSIX (unlink works on open files) but a guaranteed failure on Windows; scoped the queue the same way the neighboring tests already do. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EYoDXWApxfP8K5jQiaQ2LA
glaze 7.4 emits ASCII control bytes unescaped, which broke `encode` two ways depending on where such a byte landed: - Invalid output. RFC 8259 requires U+0000..U+001F to be escaped and glaze's own reader enforces it, so an envelope carrying a raw 0x0B anywhere serialized to JSON the peer's `decode` threw on. - Silent corruption. With a `\` or `"` earlier in the same string, the writer's chunked fast path rewrote such a byte as *two* 0x00 bytes. The payload was destroyed before transmission and the envelope still decoded, so nothing downstream could detect it. `makeErr`'s sanitizer only ever covered `message`; `body`, `modelType`, `actionType`, `contextKey`, `typeId` and the session's principal/token carry caller data that must round-trip byte-for-byte, so substitution is the wrong instrument for them. Fixed at the writer via glaze's documented `escape_control_characters` option, which is lossless both ways. `makeErr` keeps substituting for a different reason now: an err message is log-bound text, and a raw 0x1B would carry an ANSI escape into the reader's terminal. Also adds `detail::peekCallId`, a bounded prefix scan letting a transport address a reply to a frame it deliberately never decoded. Replying with callId 0 is not a harmless degradation: 0 is the client's synchronous-reply discriminator, so such a reply resumes an unrelated parked register/deregister with another call's result. Signed-off-by: Yaraslau Tamashevich <yaraslau.tamashevich@gmail.com>
Four transport defects, each unrecoverable at runtime: - Reconnect deadlock. `onConnected()` ran the reconnect handler inline from the I/O thread *before* `readLoop()` started. A handler doing what reconnect handlers exist to do — re-registering models, via `sendSync` — then waited on `_syncCv` for a reply only that same thread could deliver. No timeout, so the transport wedged forever. Handlers now run on a dedicated thread; requests coalesce and a throwing handler no longer takes the thread down. - `close()` took a connection's write mutex before `shutdownBoth()`. A client thread blocked in `sendAll` against a stalled peer holds that mutex until the send completes, and the shutdown is exactly what unblocks it — so `close()`, reached from the destructor, waited forever on the thing it was trying to interrupt. - `SocketServer` never opened or closed connection scopes: the reclamation machinery was wired into `QtWebSocketServer` only, so every model registered over this transport outlived its connection. - `WsFrameReader` rejected all fragmented frames. A peer fragments whenever a message exceeds its outgoing frame size and Qt defaults that to 512 KiB, so this broke interop with the transport this project ships. Now reassembled, with control frames passing through mid-message and the reassembled total bounded. Also retries EINTR in `accept()` (a stray signal killed the accept loop) and fixes GCC -Werror violations across morph::net, which had never been compiled by GCC anywhere in CI. Signed-off-by: Yaraslau Tamashevich <yaraslau.tamashevich@gmail.com>
`_connectionScopes[cid].insert(mid)` used operator[], which default-constructs. `handle()` posts to the worker pool while `closeConnection()` runs synchronously on the transport's disconnect callback, so a client that registers and immediately drops its socket genuinely interleaves the two — and the resurrected scope was never closed again, because nothing closes a scope twice. Every model on that dead id became unreclaimable: an unbounded leak that, with `maxLiveModels` set, wedges the server permanently at "too many models". Now looked up rather than created, and a register arriving after its scope closed is refused with err "connection closed". `maxLiveModels` and `maxInFlightExecutes` were both check-then-act with authorize/authenticate/create sitting in the gap, so every thread in the pool could observe the same under-limit value and proceed — overshooting by the pool's width, which is the burst the limits exist to prevent. The live-model cap is now re-tested inside the same lock that inserts; the in-flight cap reserves its slot with a compare-exchange at the point the slot is genuinely taken. The cheap early checks stay as advisory load-shedding. Signed-off-by: Yaraslau Tamashevich <yaraslau.tamashevich@gmail.com>
Both properties are load-bearing, because a sink is host code invoked from the framework's hot paths: - Under the lock, a sink that emitted a metric of its own — or reinstalled itself — deadlocked on a non-recursive std::mutex. - Bare, a throwing sink escaped into the framework. `LocalBackend::execute` brackets each dispatch with beginSpan / emitMetric / endSpan and settles the caller's Completion *after* them, so an exception out of instrumentation skipped setValue/setException entirely and was swallowed by StrandExecutor's catch-and-log — leaving that Completion unsettled forever. A hung caller, caused by a bug in a metrics callback. Sinks are now held behind a shared_ptr so an emitter takes a refcount copy under the lock, releases it, and calls outside, inside catch(...). Fixed at the source rather than at the one call site, so every emitter is covered. Signed-off-by: Yaraslau Tamashevich <yaraslau.tamashevich@gmail.com>
An over-size frame was rejected with a zeroed callId. That is worse than unhelpful: 0 is the client's synchronous-reply discriminator, so the error resumed whatever register/deregister happened to be parked and handed it another call's reply, while the execute it was meant for never resolved. The id is now recovered by a bounded prefix scan, so the frame still never gets decoded. An undecodable message with no sync waiter was also dropped silently, leaving that execute's Completion unsettled forever — neither .then() nor .onError() ever firing. Every message here is required to be one envelope, so an undecodable one means the peer's framing can no longer be trusted: pending calls now fail rather than wait on a stream that may never produce a matching reply. A spurious error is recoverable; a permanent hang is not. Signed-off-by: Yaraslau Tamashevich <yaraslau.tamashevich@gmail.com>
`entries()` tolerated a truncated trailing line but never removed it, and the file is opened "a" — so the next append() started writing at the exact byte the truncated JSON stopped at, with no separating newline. The two merged into one line that swallowed the new entry, and a further append pushed that merged line out of trailing position, at which point entries() threw. Since the constructor calls entries(), the journal became permanently unopenable. Now truncated to the last newline on open, which by construction can only discard an incomplete record. append() also discarded fwrite's result and flush() discarded fflush/fsync's, while IActionLog::flush() returns void — so `OutboxRelay::relay()` called markRelayed() straight afterwards, recording rows as relayed in the model's own store while nothing reached the durable sink. Both now throw, which aborts relay() before the batch is marked and leaves it retryable. Idempotency keys are only remembered as seen once a flush confirms they reached disk, so a failed write is not deduplicated away on retry. rotate() left a null handle when its reopen failed, contradicting the destructor's own comment; every entry point now reports that instead of dereferencing it. Signed-off-by: Yaraslau Tamashevich <yaraslau.tamashevich@gmail.com>
`load()` derives `_nextId` from the ids it reads and `compact()` writes only surviving "put" lines, dropping every tombstone — so the mark regressed to the highest *surviving* id from the second restart onward. Enqueue 1 and 2, markDone(2), restart (compacts to just id 1), restart again, and the next enqueue() reissued id 2: the id of a completed, acknowledged item, breaking the "new ids never collide with an old tombstone" invariant this class documents. The mark is now carried across each rewrite as a trailing "done" record for itself, emitted only when it exceeds every surviving id so it can never delete a row a "put" above just restored. No reader change is needed: load() already raises the mark for every id it sees, and erasing an absent id is a no-op. Also stops swallowing I/O failures — every mutation is documented as a committed transaction by the time it returns — and fixes two unchecked sqlite3_step calls: drain() treated an I/O error as "no more rows" and returned a silently truncated set the caller takes for the complete pending list, and a failed dedup lookup returned id 0, which matches nothing, so markDone(0) stranded the item forever. Signed-off-by: Yaraslau Tamashevich <yaraslau.tamashevich@gmail.com>
`equals(&A::code, "URGENT")` stored its argument as a std::string, which quietly bounded the documented `static constexpr auto formRules = ruleList(...)` form to whatever fits the standard library's small-string buffer — 15 characters on libstdc++. One more and the string allocates, so the node stops being a constant expression and the declaration fails with "refers to a result of operator new". The limit was invisible in the source: the same code compiled or did not depending only on how long the literal was, and on which standard library was in use. Captured inline instead, aliasing the project's existing shared `morph::detail::FixedString` rather than adding a fourth copy of it. That type's constructor relaxes from consteval to constexpr so a function parameter of type `const char (&)[N]` can be captured — as an NTTP it is still only ever evaluated at compile time. Serialisation is unchanged: emitNode() emits the same JSON string either way. The existing test only built the node as a function-local `auto const`, which never exercised constant evaluation; the new ones declare at namespace scope, where it actually happens. Signed-off-by: Yaraslau Tamashevich <yaraslau.tamashevich@gmail.com>
`unsubscribe()` removes the sink but cannot recall a callback already copied out of the handler's map, so a step re-fired just before advance() can still land afterwards. `_currentReady` is not keyed by step, so such a late reply marked the *new* step ready before anything was entered into it — letting the next advance() skip a step outright — and overwrote `_resolvedValues` with a result advance() never saw. A late failure symmetrically cleared readiness for a step that had legitimately completed. Callbacks now carry the step index they were installed for and compare it against `_activeStep` under the same mutex. A late error is still reported to the host — the action did fail — it just no longer mutates state belonging to another step. Signed-off-by: Yaraslau Tamashevich <yaraslau.tamashevich@gmail.com>
CollectionView keeps one modalForm and one detailForm for the whole collection, and prefill() only writes the fields v-rowAction names in `bind`. Everything else kept whatever the user left there while editing the *previous* row — and because DynamicForm auto-submits as soon as the form is `ready`, merely opening the next row fired the row action with that row's key and the previous row's values. A silent write of data the user neither entered nor saw. prefill() now resets first. Separately, the tabbed layout drives its Repeater off `sections[currentTab].fields`, so switching tabs destroys and rebuilds every field delegate — and a control's text flows only outward, via onTextChanged into fieldValues, never back. Returning to a tab showed empty controls while revalidate() went on auto-submitting the retained values: the form sent data the user could not see. Text-bearing controls now re-seed from fieldValues on creation. Both paths run under a `programmaticEdit` counter that suppresses auto-submit, so repopulating a form never fires an action by itself. Signed-off-by: Yaraslau Tamashevich <yaraslau.tamashevich@gmail.com>
A real locale's separator is not always one byte: fr-FR groups with U+202F (3 bytes in UTF-8) and several locales use U+00A0 (2 bytes). Typed as `char`, neither could be expressed at all — a caller could only pass some single byte that never matched, so a perfectly valid "1 050,25" typed by a French user normalised to nullopt and the control reported it malformed. Separators are now matched as whole strings, before the per-byte digit scan, so a multi-byte one works and its continuation bytes are not mistaken for stray non-digit characters. An empty view means "this locale has no such separator", the role '\0' used to play. Signed-off-by: Yaraslau Tamashevich <yaraslau.tamashevich@gmail.com>
Three gates that could not fail: - The fuzz replay globbed `findings/*/*.txt`, but both committed reproducers are `.bin`. The replay ran only the seeds and never the crash inputs, so reintroducing the skip_ws heap overflow would have left the very test that exists to catch it green. Globs are now extension-agnostic — a fuzzer input is an arbitrary byte string — and an empty glob is a configure-time error, since a replay with no FILE arguments is an unbounded fuzzing run, not a regression check. - The deprecation-marker linter was line-scoped, so it saw neither the bare `[[deprecated]]` form nor any message wrapped across lines — exactly what VERSIONING.md forbids. Rewritten to scan whole files, and given fixtures plus a self-test, because a lint gate nobody tests reports green whether or not it still detects anything. - examples/forms/test_repl.sh asserted the same thing twice with two identical `grep -q` calls; the second could not fail, and the converted-units submit could have vanished entirely without anyone noticing. Now counts the matching lines, which is the property the two inputs actually establish. Also lets MORPH_REQUIRE_VETTED_HMAC compose with MORPH_BUILD_TESTS. The guard is a deployment switch aimed at application call sites; morph's own suite deliberately exercises the reference hmacSha256 default, so the inherited definition is undone for that target alone. Configuring the documented option no longer requires switching tests off — which is what kept the option itself untested. Signed-off-by: Yaraslau Tamashevich <yaraslau.tamashevich@gmail.com>
No job enabled any MORPH_BUILD_* option, so several thousand lines — morph::net, the Qt/QML forms renderer, the SQLite queue, the fuzz harnesses, the vetted-HMAC adapters, the soak/bench targets — were never compiled here, let alone tested. That is how a documented build that cannot configure and a replay test matching the wrong file extension both reached master green, and how morph::net accumulated GCC -Werror violations without anyone noticing. - New linux-all-features job (GCC + Clang) turns every option on at once, which also proves they compose. Clang additionally builds the libFuzzer harnesses and asserts afterwards that the replay actually referenced each committed reproducer — a guard that never fires is indistinguishable from one that works. - The sanitizer/coverage matrix now includes morph::net and the SQLite queue: raw sockets, an I/O thread, a hand-rolled frame reader and a C API are what ASan/TSan/UBSan are for, and all of it was being skipped. - clang-tidy configures the full feature set so its compile database covers that code. It only ever analyses changed lines, so this buys a wider build, not a wider lint. - Valgrind covers the net and SQLite suites too, and fails loudly if a suite it expects was not built. - The deprecation lint runs its own self-test first. Signed-off-by: Yaraslau Tamashevich <yaraslau.tamashevich@gmail.com>
- SQLite3::SQLite3 was unresolvable at generate time on the ubuntu-24.04 runner, which reports "Found SQLite3: /usr/include" and then fails the link interface of morph_offline_sqlite. Locally the same module reports the library path and creates the target, so the imported target is now synthesised from the variables FindSQLite3 does set when it is missing, and promoted to GLOBAL either way. - MSVC rejects the / universal-character-names in the new locale test's narrow literals, because code page 1252 cannot represent them (C4566, fatal under warnings-as-errors). Spelled as explicit UTF-8 bytes instead, which is what those tests are actually about. Both were only reachable once CI started building the opt-in features and the sanitizer matrix started enabling the SQLite queue. Signed-off-by: Yaraslau Tamashevich <yaraslau.tamashevich@gmail.com>
MORPH_BUILD_FORMS_QML needs Qt 6.5 or newer — qt_standard_project_setup's REQUIRES keyword and QQmlApplicationEngine::loadFromModule both arrive there — but ubuntu-24.04 still ships 6.4.2, so the distro packages could never build it. Enabling the option in CI surfaced that as a missing-member compile error deep in the demo. The two jobs that build the renderer now install Qt from aqt instead of apt, and CMakeLists.txt states the 6.5 floor so a too-old Qt fails with one clear configure-time message rather than that compile error. The existing linux-qt job keeps using the distro Qt: it only builds the WebSocket transport, which 6.4 handles fine. Signed-off-by: Yaraslau Tamashevich <yaraslau.tamashevich@gmail.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
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
Implements all 28 items tracked in
docs/todo.md's production-hardening and GUI-enhancement checklist, plus fixes two real memory-safety/correctness bugs the new fuzz harness found along the way. 176 commits, ~234 files changed.Every item is opt-in or backward-compatible by default — nothing changes existing behavior unless explicitly enabled.
docs/planned/no longer holds any implemented-item specs; the authoritative current-state design lives entirely indocs/spec/.§A — Remote-mode hardening (7/7)
ActionValidator/ValidationErrornow enforced on every dispatch path, not just the client)IAuthorizer::authorizeRegister) + opaque (non-sequential) model idsRemoteServer::LimitPolicy,QtWebSocketServerConfigconnection/rate/timeout knobs)tlsVerifyingConfig/tlsPinnedConfig, non-loopback plaintext-bind guard)MORPH_REQUIRE_VETTED_HMACbuild guard)"hello"handshake) + forward-compatible action decodingConnectionIdscope, reclaims models on disconnect)§B — Durability & data-integrity (4/4)
QueueItem::attempts,DeadLetterSink)journal::OutboxRelay, idempotency-key dedup)IOfflineQueue(FileOfflineQueue,SqliteOfflineQueue)FileActionLog::rotate()§C — Operational readiness (5/5)
morph::observemetrics/trace seam,RemoteServer::health())morph::net: hand-rolled RFC 6455 WebSocket over raw sockets, verified wire-interoperable with the Qt transport both directions)MORPH_BUILD_FUZZERS/MORPH_BUILD_LOAD_TESTS)onBackendChangeddispatch (drops thedynamic_castsweep, parity-tested)beginShutdown()/drainedWithin()/closeGracefully())§D — Process (2/2)
docs/spec/pinned_facts.toml, pins enum cardinalities/constants/error strings)docs/spec/VERSIONING.md,morph::version)§E — GUI enhancement program (10/10)
Field metadata, layout/grouping, widget hints, cross-field rules, computed fields, dependent choices, i18n, a promoted Qt/QML renderer toolkit (
src/qt/forms), collections/views, and wizards/app-shell navigation — seedocs/spec/forms/forms.md's new "Design principle" section for the overview.Security fixes (found by C3's own fuzz harness, fixed before merge)
skip_wsheap-buffer-overflow: everyglz::read<>call site accepting an arbitrarystring_viewleft glaze'snull_terminatedoption at its unsafe default, reachable by a 5-byte input to anyRemoteServer. Fixed at all 4 affected sites.err-reply round-trip failure: glaze's writer doesn't escape ASCII control bytes, so an error reply echoing untrusted text (an unrecognizedkind, or an exception'swhat()) could serve invalid JSON. Fixed inwire::makeErr.Both have permanent regression cases under
tests/fuzz/findings/and dedicated Catch2 tests intest_wire_hardening.cpp.Test plan
morph_testssuite green throughout (only one pre-existing, unrelated Node.js example test fails —forms_repl_roundtrip)MORPH_BUILD_QT=ON,MORPH_BUILD_FORMS_QML=ON) greenmorph::netcross-transport interop verified both directions (SocketBackend↔QtWebSocketServer,QtWebSocketBackend↔SocketServer)WARN_AS_ERROR=FAIL_ON_WARNINGS) cleanMORPH_BUILD_OFFLINE_SQLITE=ONandMORPH_REQUIRE_VETTED_HMAC=ONguarded builds verified separately🤖 Generated with Claude Code