Repository navigation
fix(parser): reject incomplete stream reads - #131
Merged
Merged
Conversation
Signed-off-by: stacknil <219124580+stacknil@users.noreply.github.com>
Signed-off-by: stacknil <219124580+stacknil@users.noreply.github.com>
Signed-off-by: stacknil <219124580+stacknil@users.noreply.github.com>
stacknil
marked this pull request as ready for review
October 21, 2026 15:39
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
A throwing input buffer can set
badbitafter complete authentication records or during a record. The originalgetlineloop treated this as termination and returned a successful partialParseReport. On tested Linux/libstdc++, a directory input also opens but fails on read: the baseline CLI exited 0, printed zero-event success counts and overwrote four valid existing reports.Original main:
2ab31a057206822fe0a526b849f0f836bd131a37. Candidate:6aef72ea5c6eb2657e6e01fe558d257e04997ca8.Invariant
Reported input-read failures must prevent a successful parse and downstream reporting. Ordinary EOF, valid final records without newline, empty input and content-warning semantics remain supported. Public parser signatures,
ParseReport, event ownership/line numbers, detector mappings, finding identities andloglens.report.v3remain unchanged.Design decision
Add two stream-state checks, before acquisition and after the
getlineloop: rejectbadbit(including with EOF) or non-EOF failure withstd::ios_base::failure. The ten-line production change adds no per-line work, new dependency or exception framework. Existingmain.cppcatches parsing errors before Detector/report writing and success printing.The caller's exception mask remains unchanged; buffer exceptions can propagate natively when enabled. EOF without
badbitis accepted as exhausted input; masks includingfailbitoreofbitcan still throw on clean exhaustion. Malformed complete lines remain ordinary parser warnings.Threat/failure model
Offline saved inputs and stream-reported state only. The deterministic buffer throws before an exact character count and uses real EOF only at the end of saved text. This does not authenticate log provenance, detect false EOF, guarantee an atomic filesystem snapshot or durable storage, impose resource limits, or make multi-file report writes transactional. Input-failure diagnostics do not add raw log contents. API fault injection after a valid prefix is complemented by a real CLI directory read/open-failure test; no platform-specific CLI interception is introduced.
Rejected alternatives
failbit: wrong because normal exhaustion can set it.Compatibility
Seven files, only
src/parser.cppchanges production behavior. The new focused target joins existing CTest; no workflow, dependency, golden, detector, research, historical audit or release-note edit. One pre-existing unnecessary constexpr lambda capture is removed from a parser test to retain strict Clang warnings; its assertion is unchanged, and the warning was independently reproduced on original main.All 20 report artifacts across five existing golden-fixture cases are byte-identical between actual baseline/candidate CLI runs. Schema, counts and finding identities remain stable. Error-state callers that previously received partial/empty reports now receive an exception; successful inputs need no migration.
Validation
ci-releasepreset (GCC Release / warnings-as-errors).cmake -S . -B build -DBUILD_TESTING=ON -DCMAKE_BUILD_TYPE=Debug;cmake --build build --parallel;ctest --test-dir build --output-on-failure; focusedctest --test-dir build -R parser_streams -V. Exact sanitizer/mutant/fuzz commands and raw receipts stay local outside Git.Rollback
Revert the three commits as a unit. No schema/data migration or golden regeneration; rollback restores the silent partial-success failure behavior.
Draft; no merge or release in Round4. Minimum one-day input-trust review hold after the last change:
2026-10-20T18:49:25+08:00→2026-10-21T18:49:25+08:00(2026-10-21T10:49:25+00:00UTC). Further changes restart the hold; expiry does not authorize merging in this round. No auto-merge, tags, release, deployment or repository-setting changes.