Spec review: correctness fixes, authenticated sessions, and design specs#11
Open
Yaraslaut wants to merge 4 commits into
Open
Spec review: correctness fixes, authenticated sessions, and design specs#11Yaraslaut wants to merge 4 commits into
Yaraslaut wants to merge 4 commits into
Conversation
Fixes surfaced by an adversarial design review of the specs against the code: - completion: a null callback executor no longer drops an error *and* suppresses orphan logging at once — onErrAttached is now gated on `cbExec != nullptr`, so an abandoned error still reaches the orphan logger. - executor/strand: task exceptions on the worker pool and per-model strand are now caught *and logged* via morph::log instead of silently swallowed (they still never propagate out of a worker or abort sibling tasks). - logger: check the level before std::format so a suppressed formatted call pays no formatting/allocation cost; minLevel is now atomic (lock-free level reads); documented that the sink runs under the (non-recursive) global mutex. - file_action_log: entries() tolerates a torn trailing line from a crash mid-append, while still re-throwing on mid-file corruption. - journal: replay()/undoLast() detach the default action log from the reconstructed holder so replaying history no longer pollutes the live sink. - bridge: ~BridgeHandler no-ops (via a weak liveness token) if the Bridge was destroyed first, instead of a use-after-free; switchBackend is now exception-safe/atomic (stage-all-then-commit with rollback); deregisterHandler resets currentId to the 0=unbound sentinel; tryFireImpl null-checks the state. test_handler_binding updated for the new deregister currentId contract. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Turns Context::principal from an unverified client claim into a server-verified identity, so a RemoteServer can authenticate callers rather than trust them. - session_auth.hpp (new, opt-in): SessionToken claims, TokenIssuer/TokenVerifier (format `base64url(claimsJson).base64url(mac)`), a pluggable MacFunction with a self-contained reference HMAC-SHA256 default (verified against FIPS 180-4 / RFC 4231 vectors), constant-time MAC compare, and SigningAuthorizer with an optional role policy. The MAC verifies before the claims are parsed. - session.hpp: Context gains a `token` field; IAuthorizer gains an optional `authenticate()` hook (default nullopt) returning the verified principal. - remote.hpp: after authorize() succeeds, RemoteServer calls authenticate() and overwrites Context::principal with the verified value before dispatch, so model code reading session::current()->principal can trust it. Also hardens handleInline to reject `execute` up front (its async reply would otherwise write into an already-destroyed buffer). The reference HMAC is correct but not hardened — a MacFunction lets deployments inject a vetted library. See docs/spec/security.md for the trust model and its limits (TLS transport, short expiry, and secret rotation remain the deployer's). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…es, README
Completes docs/spec/ as the authoritative per-subsystem design reference and
brings the top-level docs in line with the code.
- docs/spec/: one spec per public type/subsystem, each extended with
Lifetime & ownership, Thread-safety, Failure-modes, Cross-references, and an
honest Limitations section, and reconciled with the code fixes and the new
auth layer. Adds three cross-cutting specs:
* security.md — trust model + the authenticated-session subsystem
* error_handling.md — every failure path from a throwing execute to onError
* concurrency_and_lifetimes.md — threading model + "who must outlive whom"
- CLAUDE.md: drop the 500-line cap on spec files (a spec is as long as
precision requires).
- README.md: mental model, subsystem map, build/deps, examples/bank pointer,
registration/lifetime/local-vs-remote caveats, an honest Status & limitations
section, and the relations (was alternatives) fix.
- ARCHITECTURE.md: add morph::session / session_auth / reconnect_coordinator to
the namespace and header maps; reflect the executor logging, atomic
switchBackend, handler-teardown token, journal replay/torn-write, and the
softened Completion limitation; new "Authenticated sessions" pointer.
- quantity.hpp / quantity_equation.hpp: fix stale docs/quantity_type.md paths.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
…aths Adds targeted cases for previously-untested branches flagged by codecov/patch on PR #11: malformed-token variants in session_auth, switchBackend's partial-registration rollback (including nested rollback failure), the dead-bridge BridgeHandler destructor path, FileActionLog's truncated-vs-mid-file corruption handling, RemoteServer::handleInline's execute rejection, the authenticate() principal-override path, and the catch(...) arms in ThreadPoolExecutor/StrandExecutor. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.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.
Outcome of an adversarial review of every
docs/spec/file against the implementation. Three topic commits: correctness fixes, a new authenticated-session feature, and the design-spec / docs pass.fix:latent correctness bugs (commit 1)onErrAttachedis gated on the executor existing, so abandoned errors still reach the orphan logger.std::format(suppressed calls cost nothing);minLevelis atomic (lock-free reads); sink re-entrancy documented.entries()tolerates a torn trailing line from a crash mid-append; still re-throws mid-file corruption.replay()/undoLast()detach the default log so reconstruction no longer pollutes the live audit trail.~BridgeHandlerno-ops via a weak liveness token if theBridgedied first (was a use-after-free);switchBackendis exception-safe/atomic with rollback;deregisterHandlerresetscurrentId;tryFireImplnull-checks.feat:authenticated sessions (commit 2)New opt-in
session_auth.hpp: signed bearer tokens (SessionToken/TokenIssuer/TokenVerifier), a pluggableMacFunctionwith a self-contained reference HMAC-SHA256 (verified against FIPS 180-4 / RFC 4231 vectors), constant-time compare, and aSigningAuthorizerwith an optional role policy.Contextgains atoken;IAuthorizergains anauthenticate()hook;RemoteServeroverwritesContext::principalwith the verified identity before dispatch. Full trust model indocs/spec/security.md.docs:design specs + README/ARCHITECTURE (commit 3)docs/spec/file gains Lifetime, Thread-safety, Failure-modes, Cross-reference, and honest Limitations sections, reconciled with the fixes and auth layer.security.md,error_handling.md,concurrency_and_lifetimes.md.Verification
WARN_AS_ERRORdocs build passes.Deliberately documented as limitations (not changed)
Rational's int64 overflow ceiling, the wire protocol's lack of version/size bounds, provenance-on-by-default inQuantity, and string type-id collisions — larger design calls left for maintainer decision, now written down in the relevant specs.🤖 Generated with Claude Code