Skip to content

fix(parser): reject incomplete stream reads - #131

Merged
stacknil merged 3 commits into
mainfrom
fix/parser-stream-io-integrity
Oct 21, 2026
Merged

stacknil merged 3 commits into
mainfrom
fix/parser-stream-io-integrity

Conversation

@stacknil

@stacknil stacknil commented Oct 20, 2026 •

Copy link
Copy Markdown
Owner

Problem

A throwing input buffer can set badbit after complete authentication records or during a record. The original getline loop treated this as termination and returned a successful partial ParseReport. 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 and loglens.report.v3 remain unchanged.

Design decision

Add two stream-state checks, before acquisition and after the getline loop: reject badbit (including with EOF) or non-EOF failure with std::ios_base::failure. The ten-line production change adds no per-line work, new dependency or exception framework. Existing main.cpp catches 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 badbit is accepted as exhausted input; masks including failbit or eofbit can 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

  1. Reject all failbit: wrong because normal exhaustion can set it.
  2. Return partial events or downgrade read failure to a warning: hides incomplete evidence and allows misleading reports.
  3. Clear state or change caller exception masks: hides the failure and changes API policy.
  4. Require a final newline: rejects supported valid evidence.
  5. Redesign report transactions here: separate output-storage contract, outside this parser correction.

Compatibility

Seven files, only src/parser.cpp changes 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

  • Untouched original main: existing CTest 8/8 passed. New deterministic two-mode stream suite against original production: 42 cases / 18 failures; CLI directory-input regression also failed. Corrected suite: 42/42 on GCC 13.3, Clang 18.1 and Windows MSVC 19.51 / STL 145.
  • Native Ubuntu 24.04.4 / WSL2 ext4, non-root UID/EUID 1000: strict C++20 Debug CTest 9/9 with GCC and Clang. Both use libstdc++ build 20240904. Final signed tree also passes 9/9 in a new clean native clone configured with supported ci-release preset (GCC Release / warnings-as-errors).
  • Clang ASan+UBSan strict Debug: focused42 cases and full CTest 9/9, no diagnostic. Existing instrumented parser libFuzzer: 10,000 actual executions, fixed seed20261020/max_len1024/timeout2, no crash or sanitizer report. Fuzzing is separate from stream fault injection and neither is exhaustive proof.
  • Windows/MSVC strict C++20 Debug: actual build, focused42 and CTest 9/9. Existing relocated SDK/Debug CRT paths were resolved within the temporary build/process environment; no system setting or runtime installation changed.
  • Three disposable green/red mutants rejected in both modes: remove completion check; swallow a read exception into partial success; reject a normal unterminated last record. Production source retained.
  • CLI failed-input cases: exit1, no stdout success summary, no fresh output directory and four actual old artifacts unchanged. Five fixtures/20 reports retain exact output bytes. Four independent read-only reviewers found no material findings. Final diff and targeted public privacy/secret scans pass; original34 checkout states, research, tags/releases and five frozen PRs preserved before commit.
  • Reproduction: cmake -S . -B build -DBUILD_TESTING=ON -DCMAKE_BUILD_TYPE=Debug; cmake --build build --parallel; ctest --test-dir build --output-on-failure; focused ctest --test-dir build -R parser_streams -V. Exact sanitizer/mutant/fuzz commands and raw receipts stay local outside Git.
  • Bounded identical1000line GCC Debug CLI smoke: median base15.755ms/candidate16.227ms over5 measured runs each; noisy overlapping ranges, no throughput or no-regression guarantee. The source change adds two constant-time checks outside the loop.
  • Three GPG-signed DCO commits use the established public GitHub noreply author/committer identity. Remote 11/11 checks pass on this exact head, including PR Linux/Windows CTest and parser fuzz, Repo Sentinel and CodeQL. Both remote platforms pass CTest9/9. GitHub verifies all three commit signatures as valid with public noreply author/committer and DCO. No issue/inline review comments or reviews were present at readback. Final seven-file diff reviewed again after CI: preserve native stream semantics, keep reported failures outside the partial-report boundary; principal limitations are false EOF and nontransactional output writes; successful artifacts remain compatible; unit-revert rollback is described below.

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:00 UTC). Further changes restart the hold; expiry does not authorize merging in this round. No auto-merge, tags, release, deployment or repository-setting changes.

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
stacknil marked this pull request as ready for review October 21, 2026 15:39
@stacknil
stacknil merged commit 64994d7 into main Oct 21, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant