diff --git a/CMakeLists.txt b/CMakeLists.txt index 22addbd..c4cbd25 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -83,6 +83,11 @@ if(BUILD_TESTING) loglens_enable_warnings(test_parser_properties) add_test(NAME parser_properties COMMAND test_parser_properties) + add_executable(test_parser_streams tests/test_parser_streams.cpp) + target_link_libraries(test_parser_streams PRIVATE loglens_lib) + loglens_enable_warnings(test_parser_streams) + add_test(NAME parser_streams COMMAND test_parser_streams) + add_executable(test_timestamp_boundaries tests/test_timestamp_boundaries.cpp) target_link_libraries(test_timestamp_boundaries PRIVATE loglens_lib) loglens_enable_warnings(test_timestamp_boundaries) diff --git a/docs/parser-contract.md b/docs/parser-contract.md index 5c09472..9053ce7 100644 --- a/docs/parser-contract.md +++ b/docs/parser-contract.md @@ -26,6 +26,57 @@ month length and leap-year validity are checked afterward. Out-of-range days produce `unknown_timestamp` / `invalid calendar date` warnings rather than wrapping into valid dates. See [the boundary test report](../FUZZING-OR-BOUNDARY-TEST-REPORT.md). +## Stream completion and read failures (unreleased correction) + +`parse_stream` returns a `ParseReport` only after the input reaches ordinary EOF +without a reported read failure. With the default stream exception mask, empty +input, newline-terminated input and a valid final record without a newline all +complete normally. The final record retains its event and original line number. +Malformed or unsupported complete lines retain their existing warning taxonomy; +they are content failures, separate from an underlying stream failure. + +The parser checks stream state before acquisition and after the `getline` loop: + +- `badbit` is an error, including when `eofbit` is also set. +- `failbit` without `eofbit` is an error. The `fail()` predicate also includes + `badbit`; it does not identify `failbit` alone. +- EOF without `badbit`, including the normal `eofbit | failbit` exhaustion state, + is accepted. A stream already exhausted by its caller returns an empty report; + the parser does not clear or rewind it. + +Reported non-EOF failures throw `std::ios_base::failure` rather than returning +partial events or adding a parser warning. Caller-enabled stream exceptions +propagate and the exception mask is unchanged: a buffer exception may be +rethrown when `badbit` exceptions are enabled. Masks including `failbit` or +`eofbit` can also throw on ordinary EOF, as before; callers requiring normal +EOF reports should use the default exception mask. These distinctions follow +the C++20 unformatted-input and string-`getline` rules +([working draft](https://isocpp.org/files/papers/N4860.pdf), +`[istream.unformatted]`, `[string.io]`). + +Counterexample: a buffer that throws after two complete valid records sets +`badbit`; previously the loop ended and returned those two events as a successful +report. A mid-record failure could similarly hide the remaining evidence. +`tests/test_parser_streams.cpp` injects faults at exact character positions in +both input modes and checks normal EOF, content warnings, initial state and +exception masks. Its focused executable also accepts a case-name substring. +Mutation controls must reject dropping the completion check, suppressing a read +exception into partial success, and rejecting an ordinary unterminated last line. + +`parse_file` retains open-error handling and delegates read validation to this +boundary. The CLI catches parsing exceptions before detection and report writing: +it exits nonzero, emits an error diagnostic, prints no success summary and leaves +existing reports unchanged. The CLI regression uses directory input: on the +tested Linux file stream this opens but fails on read; other runtimes can reject +it at open. Injected failures after a valid prefix are tested through the stream +API rather than an operating-system-specific CLI interception hook. + +The parser trusts the stream's EOF/error signals; it cannot detect a buffer that +reports false EOF. This does not authenticate log provenance, guarantee an atomic +file snapshot or durable storage, impose resource limits, or make multi-file report +writes transactional. Successful-input events, coverage counts, detector rules, +finding identities and released report contracts are unchanged. + ## Recognized event families The parser currently recognizes common authentication evidence from: diff --git a/docs/quality-gates.md b/docs/quality-gates.md index 9e70398..a69855d 100644 --- a/docs/quality-gates.md +++ b/docs/quality-gates.md @@ -12,6 +12,7 @@ The main review principle is: | Claim under review | Evidence surface | Automated or repeatable gate | Good stopping point | | --- | --- | --- | --- | | Supported input formats are explicit | [`parser-contract.md`](./parser-contract.md), [`parser-conformance-matrix.md`](./parser-conformance-matrix.md) | `ctest --test-dir build --output-on-failure` through `test_parser`; fixture anchors in `assets/parser_fixture_matrix_syslog.log` and `assets/parser_fixture_matrix_journalctl_short_full.log` | Reviewer can name the two supported formats and see known/unknown line behavior in fixtures | +| Stream failures cannot return successful partial evidence | [`parser-contract.md`](./parser-contract.md#stream-completion-and-read-failures-unreleased-correction), [`tests/test_parser_streams.cpp`](../tests/test_parser_streams.cpp), [`tests/test_cli.cpp`](../tests/test_cli.cpp) | `parser_streams` injects deterministic read faults and checks EOF/exception policy; `cli` checks input-failure exit, stdout and report preservation | Reviewer can distinguish content warnings, ordinary EOF and transport errors without claiming atomic acquisition or transactional output | | Parser coverage is visible | [`parser-coverage-notes.md`](./parser-coverage-notes.md), [`tests/fixtures/parser_matrix/noisy_auth_expected.json`](../tests/fixtures/parser_matrix/noisy_auth_expected.json) | `test_parser` compares noisy-auth coverage output to the checked-in expected summary | Reviewer can see parsed lines, skipped blanks, warnings, failure categories, and unknown-pattern buckets | | Parser handler dispatch remains robust | [`parser-contract.md`](./parser-contract.md), [`tests/test_parser_properties.cpp`](../tests/test_parser_properties.cpp), [`tests/fuzz/README.md`](../tests/fuzz/README.md) | `test_parser_properties` checks all handler-order permutations and generated failure invariants; Ubuntu CI runs a bounded Clang libFuzzer smoke corpus | Reviewer can see that registry order does not change results and malformed or arbitrary bytes remain bounded by parser result invariants | | Unsupported evidence does not silently become detector evidence | [`parser-contract.md`](./parser-contract.md), [`rule-catalog.md`](./rule-catalog.md), [`case-study-linux-auth-bruteforce.md`](./case-study-linux-auth-bruteforce.md) | `test_parser` covers unknown-pattern warnings; `test_detector` covers signal-boundary behavior | Reviewer can explain why unsupported lines remain warnings instead of findings | @@ -33,6 +34,7 @@ Use the smallest command set that answers the review question: | Multi-config local test run | `ctest --test-dir build -C Debug --output-on-failure` | | Report-contract snapshot verification | `ctest --test-dir build -C Debug -R report_contracts --output-on-failure` | | Parser property checks | `ctest --test-dir build -C Debug -R parser_properties --output-on-failure` | +| Stream read/EOF fault controls | `ctest --test-dir build -C Debug -R parser_streams --output-on-failure` | | Bounded parser fuzz smoke (Clang) | See [`tests/fuzz/README.md`](../tests/fuzz/README.md) | | Performance envelope reproduction | `pwsh -File scripts/benchmark-performance-envelope.ps1` | | Fast performance smoke check | `pwsh -File scripts/benchmark-performance-envelope.ps1 -LineCounts 1000 -Runs 1 -WarmupRuns 0 -SkipBuild` | diff --git a/src/parser.cpp b/src/parser.cpp index e86012d..1663534 100644 --- a/src/parser.cpp +++ b/src/parser.cpp @@ -101,6 +101,14 @@ std::optional AuthLogParser::parse_line(std::string_view line, } ParseReport AuthLogParser::parse_stream(std::istream& input) const { + const auto check_read_state = [&input] { + // EOF may set failbit after the last record; badbit is never clean EOF. + if (input.bad() || (input.fail() && !input.eof())) { + throw std::ios_base::failure("unable to read input log"); + } + }; + check_read_state(); + ParseReport result; result.metadata.input_mode = config_.input_mode; result.metadata.timezone_present = config_.input_mode == InputMode::JournalctlShortFull; @@ -140,6 +148,8 @@ ParseReport AuthLogParser::parse_stream(std::istream& input) const { ++category_count.second; } + check_read_state(); + if (result.quality.total_lines != 0) { result.quality.parse_success_rate = static_cast(result.quality.parsed_lines) / static_cast(result.quality.total_lines); diff --git a/tests/test_cli.cpp b/tests/test_cli.cpp index 9c0768d..7ed0c4a 100644 --- a/tests/test_cli.cpp +++ b/tests/test_cli.cpp @@ -1,3 +1,4 @@ +#include #include #include #include @@ -250,6 +251,35 @@ int main(int argc, char* argv[]) { expect(!std::filesystem::exists(journalctl_out / "warnings.csv"), "did not expect journalctl warnings.csv without explicit csv flag"); + const auto directory_input = output_dir / "directory_input"; + std::filesystem::create_directories(directory_input); + const std::array report_names{"report.md", "report.json", "findings.csv", "warnings.csv"}; + for (const bool existing : {false, true}) { + const auto failed_output = output_dir / (existing ? "read_failure_existing" : "read_failure_new"); + if (existing) { + std::filesystem::create_directories(failed_output); + for (const auto* name : report_names) { + std::filesystem::copy_file(csv_out / name, failed_output / name); + } + } + const auto failure_stdout = output_dir / "read_failure_stdout.txt"; + const auto failure_stderr = output_dir / "read_failure_stderr.txt"; + const int failure_exit = std::system(build_command( + quote_argument(loglens_exe) + " --mode syslog --year 2026 --csv " + + quote_argument(directory_input) + " " + quote_argument(failed_output), + &failure_stdout, &failure_stderr).c_str()); + expect(failure_exit != 0, "expected directory input read/open failure"); + expect(read_file(failure_stdout).empty(), "input failure printed a success summary"); + expect(read_file(failure_stderr).find("LogLens failed:") != std::string::npos, + "expected input failure diagnostic"); + for (const auto* name : report_names) { + expect(existing ? read_file(failed_output / name) == read_file(csv_out / name) + : !std::filesystem::exists(failed_output / name), + "input failure created or changed a report artifact"); + } + expect(existing || !std::filesystem::exists(failed_output), "input failure created output directory"); + } + const auto missing_year_out = output_dir / "missing_year"; std::filesystem::create_directories(missing_year_out); const int missing_year_exit = std::system(build_command( diff --git a/tests/test_parser.cpp b/tests/test_parser.cpp index b7ab138..66d868e 100644 --- a/tests/test_parser.cpp +++ b/tests/test_parser.cpp @@ -1369,7 +1369,7 @@ void test_mixed_auth_first_pam_unix_session_closed_is_unsupported_telemetry() { const auto full = parser.parse_file(asset_path("mixed_auth_corpus.log")); const auto warning_it = std::find_if( full.warnings.begin(), full.warnings.end(), - [kFixtureLineNumber](const loglens::ParseWarning& warning) { + [](const loglens::ParseWarning& warning) { return warning.line_number == kFixtureLineNumber; }); expect(warning_it != full.warnings.end(), diff --git a/tests/test_parser_streams.cpp b/tests/test_parser_streams.cpp new file mode 100644 index 0000000..983ddd6 --- /dev/null +++ b/tests/test_parser_streams.cpp @@ -0,0 +1,256 @@ +#include "parser.hpp" + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace { + +struct InjectedReadError : std::runtime_error { + InjectedReadError() : std::runtime_error("injected transport failure") {} +}; + +// No get area: every consumed character passes through uflow. A fault throws +// before the next character; returning EOF here would model a different contract. +class FaultBuffer : public std::streambuf { + public: + explicit FaultBuffer(std::string text, + std::size_t fail_after = std::numeric_limits::max()) + : text_(std::move(text)), fail_after_(fail_after) {} + + std::size_t delivered = 0; + + protected: + int_type underflow() override { + if (delivered == fail_after_) { + throw InjectedReadError{}; + } + return delivered == text_.size() ? traits_type::eof() + : traits_type::to_int_type(text_[delivered]); + } + + int_type uflow() override { + const auto next = underflow(); + if (!traits_type::eq_int_type(next, traits_type::eof())) { + ++delivered; + } + return next; + } + + private: + std::string text_; + std::size_t fail_after_; +}; + +void expect(bool condition, const char* message) { + if (!condition) { + throw std::runtime_error(message); + } +} + +struct Outcome { + std::optional report; + std::string exception; + std::string message; +}; + +Outcome observe(const loglens::AuthLogParser& parser, std::istream& input, + const std::string& name, const FaultBuffer& buffer) { + Outcome outcome; + try { + outcome.report = parser.parse_stream(input); + } catch (const InjectedReadError& error) { + outcome.exception = "InjectedReadError"; + outcome.message = error.what(); + } catch (const std::ios_base::failure& error) { + outcome.exception = "ios_base::failure"; + outcome.message = error.what(); + } catch (const std::exception& error) { + outcome.exception = "unexpected std::exception"; + outcome.message = error.what(); + } + std::cout << name << " result=" << (outcome.report ? "report" : outcome.exception) + << " rdstate=" << input.rdstate() + << " badbit=" << ((input.rdstate() & std::ios::badbit) != 0) + << " failbit=" << ((input.rdstate() & std::ios::failbit) != 0) + << " eofbit=" << ((input.rdstate() & std::ios::eofbit) != 0) + << " delivered=" << buffer.delivered; + if (outcome.report) { + std::cout << " events=" << outcome.report->events.size() + << " warnings=" << outcome.report->warnings.size(); + } + std::cout << '\n'; + return outcome; +} + +void expect_metadata(const loglens::ParseReport& report, loglens::InputMode mode) { + expect(report.metadata.input_mode == mode, "input mode changed"); + expect(report.metadata.timezone_present == (mode == loglens::InputMode::JournalctlShortFull), + "timezone metadata changed"); + expect(report.metadata.assume_year == (mode == loglens::InputMode::SyslogLegacy + ? std::optional{2026} : std::nullopt), "assumed year changed"); +} + +std::size_t failures = 0; +std::size_t cases = 0; +std::string_view filter; + +template +void run(const std::string& name, Test test) { + if (!filter.empty() && name.find(filter) == std::string::npos) { + return; + } + ++cases; + try { + test(); + } catch (const std::exception& error) { + ++failures; + std::cerr << "FAIL " << name << ": " << error.what() << '\n'; + } +} + +void test_mode(loglens::InputMode mode) { + const loglens::AuthLogParser parser({mode, mode == loglens::InputMode::SyslogLegacy + ? std::optional{2026} : std::nullopt}); + const std::string prefix = loglens::to_string(mode) + "/"; + const std::string header = mode == loglens::InputMode::SyslogLegacy + ? "Mar 10 08:11:22 " : "Tue 2026-03-10 08:11:22 UTC "; + const auto envelope = header + "example-host sshd[1234]: "; + const auto valid = envelope + "Failed password for user-a from 203.0.113.10 port 50100 ssh2"; + const auto unsupported = envelope + "Connection closed by 203.0.113.10 port 50100 [preauth]"; + + for (const bool newline : {true, false}) { + const auto name = prefix + (newline ? "clean_eof" : "unterminated_final_line"); + run(name, [&] { + FaultBuffer buffer(valid + "\n" + valid + (newline ? "\n" : "")); + std::istream input(&buffer); + const auto outcome = observe(parser, input, name, buffer); + expect(outcome.report.has_value(), "normal EOF must return a report"); + const auto& report = *outcome.report; + expect(report.events.size() == 2 && report.warnings.empty(), "final valid record lost"); + expect(report.quality.total_lines == 2 && report.quality.parsed_lines == 2 + && report.quality.unparsed_lines == 0 && report.quality.parse_success_rate == 1.0, + "clean input counts changed"); + expect(report.events[0].line_number == 1 && report.events[1].line_number == 2, + "event line numbers changed"); + expect(report.events[1].hostname == "example-host" && report.events[1].username == "user-a" + && report.events[1].source_ip == "203.0.113.10" + && report.events[1].event_type == loglens::EventType::SshFailedPassword, + "owning event content changed"); + expect_metadata(report, mode); + expect(input.eof() && !input.bad(), "clean EOF state misclassified"); + }); + } + + run(prefix + "content_warnings", [&] { + FaultBuffer buffer(valid + "\n\n" + unsupported + "\nmalformed\n" + valid + "\n"); + std::istream input(&buffer); + const auto outcome = observe(parser, input, prefix + "content_warnings", buffer); + expect(outcome.report.has_value(), "content warnings became transport failure"); + const auto& report = *outcome.report; + expect(report.events.size() == 2 && report.events[1].line_number == 5, "recovery/event lines changed"); + expect(report.warnings.size() == 2 && report.warnings[0].line_number == 3 + && report.warnings[1].line_number == 4, "warning line numbers changed"); + expect(report.warnings[0].category == loglens::ParserFailureCategory::KnownProgramUnknownMessage + && report.warnings[1].category == loglens::ParserFailureCategory::UnknownTimestamp, + "content failure taxonomy changed"); + expect(report.quality.total_lines == 4 && report.quality.skipped_blank_lines == 1 + && report.quality.parsed_lines == 2 && report.quality.unparsed_lines == 2 + && report.quality.parse_success_rate == 0.5 + && report.quality.top_unknown_patterns.size() == 2 + && report.quality.failure_categories.size() == 2, "warning accounting changed"); + expect_metadata(report, mode); + }); + + const auto first_two = valid + "\n" + valid + "\n"; + const auto after_warning = valid + "\nmalformed\n"; + struct FaultCase { const char* name; std::string text; std::size_t position; std::ios::iostate mask; }; + const std::array faults{ + FaultCase{"before_data", first_two, 0, std::ios::goodbit}, + FaultCase{"after_complete_lines", first_two + valid, first_two.size(), std::ios::goodbit}, + FaultCase{"within_line", first_two + valid, first_two.size() + 20, std::ios::goodbit}, + FaultCase{"after_malformed", after_warning + valid, after_warning.size() + 20, std::ios::goodbit}, + FaultCase{"bad_mask_after_lines", first_two + valid, first_two.size(), std::ios::badbit}, + FaultCase{"bad_mask_within_line", first_two + valid, first_two.size() + 20, std::ios::badbit}, + FaultCase{"fail_mask_after_lines", first_two + valid, first_two.size(), std::ios::failbit}, + FaultCase{"fail_mask_within_line", first_two + valid, first_two.size() + 20, std::ios::failbit}, + FaultCase{"combined_mask", first_two + valid, first_two.size() + 20, std::ios::badbit | std::ios::failbit}, + FaultCase{"eof_mask", first_two + valid, first_two.size() + 20, std::ios::eofbit}, + }; + for (const auto& fault : faults) { + const auto name = prefix + fault.name; + run(name, [&] { + FaultBuffer buffer(fault.text, fault.position); + std::istream input(&buffer); + input.exceptions(fault.mask); + const auto outcome = observe(parser, input, name, buffer); + expect(!outcome.report, "read failure returned a successful partial report"); + expect(outcome.exception == "InjectedReadError" || outcome.exception == "ios_base::failure", + "unexpected failure classification"); + expect(input.bad() && !input.eof(), "injected transport failure was mistaken for EOF"); + expect(buffer.delivered == fault.position && input.exceptions() == fault.mask, + "fault position or caller exception policy changed"); + expect(outcome.message.find("Failed password") == std::string::npos, + "transport diagnostic copied log contents"); + }); + } + + for (const auto state : {std::ios::goodbit, std::ios::failbit, std::ios::badbit, + std::ios::badbit | std::ios::eofbit, std::ios::eofbit, + std::ios::failbit | std::ios::eofbit}) { + const auto name = prefix + "initial_state_" + std::to_string(state); + run(name, [&] { + FaultBuffer buffer(state == std::ios::goodbit ? "" : valid); + std::istream input(&buffer); + input.setstate(state); + const auto outcome = observe(parser, input, name, buffer); + const bool failed = (state & std::ios::badbit) != 0 + || ((state & std::ios::failbit) != 0 && (state & std::ios::eofbit) == 0); + expect(outcome.report.has_value() != failed, "empty/exhausted and failed streams conflated"); + if (outcome.report) { + expect(outcome.report->events.empty() && outcome.report->warnings.empty() + && outcome.report->quality.total_lines == 0, "empty input counts changed"); + expect_metadata(*outcome.report, mode); + } + expect(buffer.delivered == 0, "initially failed stream consumed characters"); + }); + } + + // Preserve the caller's native policy: fail/eof exception masks can throw on + // clean exhaustion too. The parser neither clears state nor changes the mask. + for (const auto mask : {std::ios::failbit, std::ios::eofbit}) { + const auto name = prefix + "native_eof_exception_" + std::to_string(mask); + run(name, [&] { + FaultBuffer buffer(valid); + std::istream input(&buffer); + input.exceptions(mask); + const auto outcome = observe(parser, input, name, buffer); + expect(!outcome.report && outcome.exception == "ios_base::failure", "native exception policy changed"); + expect(input.eof() && !input.bad() && input.exceptions() == mask, "normal EOF confused with badbit"); + }); + } +} + +} // namespace + +int main(int argc, char* argv[]) { + if (argc == 2) { + filter = argv[1]; + } +#ifdef __GLIBCXX__ + std::cout << "libstdc++=" << __GLIBCXX__ << '\n'; +#endif +#ifdef _MSVC_STL_VERSION + std::cout << "MSVC_STL=" << _MSVC_STL_VERSION << '\n'; +#endif + test_mode(loglens::InputMode::SyslogLegacy); + test_mode(loglens::InputMode::JournalctlShortFull); + std::cout << "stream cases=" << cases << " failed=" << failures << '\n'; + return cases != 0 && failures == 0 ? 0 : 1; +}