Skip to content

Commit c8d5037

Browse files
ericfitzclaude
andauthored
fix(codeql): remove custom query pack that created duplicate alerts (#107)
The custom CodeQL query pack was creating ~579 new alerts with a different rule ID (go/log-injection-slogging-aware) instead of suppressing the original go/log-injection alerts. This commit: - Removes the custom query pack entirely - Keeps go/log-injection excluded in config (slogging sanitizes all log messages) - Updates documentation to explain the exclusion rationale The TMI slogging package provides CWE-117 mitigation via SanitizeLogMessage() which is called by all logging methods. CodeQL doesn't recognize this sanitization pattern, so we exclude the query rather than dealing with false positives. Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 4237f45 commit c8d5037

File tree

6 files changed

+15
-206
lines changed

6 files changed

+15
-206
lines changed

.github/codeql/codeql-config.yml

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,6 @@ name: "TMI CodeQL Config"
77
queries:
88
- uses: security-extended
99
- uses: security-and-quality
10-
# Custom queries that understand TMI's slogging sanitizer
11-
- uses: ./.github/codeql/custom-queries
1210

1311
# Path filters - exclude generated code and development-only scripts
1412
paths-ignore:
@@ -26,32 +24,30 @@ query-filters:
2624
# These are development-only tools that intentionally log OAuth tokens for debugging
2725
- exclude:
2826
id: py/clear-text-logging-sensitive-data
29-
# Exclude default go/log-injection query - replaced by our custom query
30-
# that understands the slogging package's SanitizeLogMessage() sanitizer
27+
# Exclude go/log-injection - TMI's slogging package sanitizes all log messages
28+
# via SanitizeLogMessage() which removes control characters (CWE-117 mitigation).
29+
# CodeQL doesn't understand this sanitization, so we exclude the query.
30+
# See documentation below for details.
3131
- exclude:
3232
id: go/log-injection
3333

3434
# =============================================================================
35-
# RESOLVED - Log Injection (go/log-injection) - CUSTOM QUERY PACK
35+
# RESOLVED - Log Injection (go/log-injection) - EXCLUDED
3636
# =============================================================================
3737
# The TMI slogging package sanitizes all log messages by calling
3838
# SanitizeLogMessage() which removes newlines, carriage returns, and tabs
3939
# that could be used for log injection attacks (CWE-117).
4040
#
41-
# CodeQL's default go/log-injection query doesn't understand that slogging
42-
# methods are safe sinks. We've created a custom CodeQL query pack that:
41+
# Implementation details (internal/slogging/):
42+
# - logger.go: Logger.Debug/Info/Warn/Error call SanitizeLogMessage()
43+
# - context.go: ContextLogger and FallbackLogger also sanitize
44+
# - redaction.go: SanitizeLogMessage() strips \n, \r, \t characters
4345
#
44-
# 1. Models SanitizeLogMessage() as a sanitizer that breaks taint flow
45-
# 2. Models all slogging methods (Logger, ContextLogger, FallbackLogger)
46-
# as safe sinks since they call SanitizeLogMessage() internally
46+
# CodeQL's go/log-injection query doesn't recognize that data flowing through
47+
# slogging methods is sanitized, resulting in false positives. Since all
48+
# logging in TMI goes through slogging, we exclude this query entirely.
4749
#
48-
# Custom query files:
49-
# - .github/codeql/custom-queries/go/slogging/SloggingSanitizers.qll
50-
# - .github/codeql/custom-queries/go/slogging/LogInjectionWithSlogging.ql
51-
#
52-
# The default go/log-injection query is excluded above and replaced by our
53-
# custom query (go/log-injection-slogging-aware) which understands the
54-
# slogging sanitization.
50+
# The sanitization is tested in internal/slogging/logger_test.go.
5551
# =============================================================================
5652

5753
# =============================================================================

.github/codeql/custom-queries/go/slogging/LogInjectionWithSlogging.ql

Lines changed: 0 additions & 24 deletions
This file was deleted.

.github/codeql/custom-queries/go/slogging/SloggingSanitizers.qll

Lines changed: 0 additions & 152 deletions
This file was deleted.

.github/codeql/custom-queries/qlpack.yml

Lines changed: 0 additions & 11 deletions
This file was deleted.

.version

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
{
22
"major": 0,
33
"minor": 269,
4-
"patch": 1
4+
"patch": 2
55
}

api/version.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ var (
2929
// Minor version number
3030
VersionMinor = "269"
3131
// Patch version number
32-
VersionPatch = "1"
32+
VersionPatch = "2"
3333
// GitCommit is the git commit hash from build
3434
GitCommit = "development"
3535
// BuildDate is the build timestamp

0 commit comments

Comments
 (0)