Repository navigation
Rust: Don't report cleartext logging in test functions - #23069
johncarmack1984 wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The production cleartext-logging barrier changes reported paths, but missing DCA results and regression analysis leave its real-world impact unverified by the focused fixture alone.
1 open issue
What changed in this PR
This PR reduces false positives in rust/cleartext-logging by suppressing data flow within functions whose attribute path ends in test, including standard Rust and Tokio tests.
Changes:
- Adds a test-function barrier, including expressions in nested closures.
- Adds regression cases for printing and assertions in synchronous and asynchronous tests, plus the Tokio dependency.
- Documents the analysis change in a release note.
| File | Description |
|---|---|
rust/ql/test/query-tests/security/CWE-312/test_logging.rs |
Adds standard, closure, and async test cases. |
rust/ql/test/query-tests/security/CWE-312/options.yml |
Adds Tokio for async test coverage. |
rust/ql/test/query-tests/security/CWE-312/Cargo.lock |
Records Tokio and its dependencies. |
rust/ql/src/change-notes/2026-11-10-cleartext-logging-test-functions.md |
Documents suppression of test-function results. |
rust/ql/lib/codeql/rust/security/CleartextLoggingExtensions.qll |
Adds the test-function data-flow barrier. |
Review effort: Balanced (auto)
| * attribute. A test's assertion messages and output go to the test runner, not | ||
| * to an application's log. | ||
| */ | ||
| private class TestFunctionBarrier extends Barrier { |
There was a problem hiding this comment.
DCA runs on the CodeQL team's side, so that one's for a maintainer to start. The only query whose results this changes is rust/cleartext-logging.
|
Hello @johncarmack1984. Thanks for the PR and for making us aware of this. Recently we've considered removing all results inside tests by default. Given that, I'm not sure if this is the approach that makes the most sense. For now I'll leave this PR open until we make a decision on how to proceed. |
rust/cleartext-loggingreportsassert!andprintln!inside Rust tests, since panic messages and printed output are log sinks. All five of its open results in maplibre/martin are test assertions (tls.rs, source.rs), and inline tests don't get code scanning's test classification. This adds a barrier inside functions whose attribute path ends intest(#[test],#[tokio::test]), which only build into test binaries, so every removed result runs under a test.The first commit adds the cases as
SPURIOUS; the second adds the barrier and leavesCleartextLogging.expectedidentical tomain.