Skip to content

fix: add length guard in matchPattern to prevent false positives with overlapping prefix/suffix wildcards#28982

Merged
pelikhan merged 3 commits intomainfrom
copilot/fix-logger-matchpattern-length-guard
Apr 28, 2026
Merged

fix: add length guard in matchPattern to prevent false positives with overlapping prefix/suffix wildcards#28982
pelikhan merged 3 commits intomainfrom
copilot/fix-logger-matchpattern-length-guard

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 28, 2026

matchPattern in pkg/logger/logger.go incorrectly matches namespaces against middle-wildcard patterns when the prefix and suffix overlap (e.g. pattern "ab*ba" matches "aba" because HasPrefix and HasSuffix checks pass independently, even though the string is too short to satisfy both non-overlapping).

Changes

  • pkg/logger/logger.go: Add len(namespace) >= len(prefix)+len(suffix) guard before the prefix/suffix checks in the middle-wildcard branch:
// Before
return strings.HasPrefix(namespace, prefix) && strings.HasSuffix(namespace, suffix)

// After
return len(namespace) >= len(prefix)+len(suffix) &&
    strings.HasPrefix(namespace, prefix) &&
    strings.HasSuffix(namespace, suffix)
  • pkg/logger/logger_test.go: Add two test cases to TestMatchPattern covering the overlapping case ("aba" vs "ab*ba"false) and the exact-boundary case ("abba" vs "ab*ba"true).

Copilot AI and others added 2 commits April 28, 2026 21:04
… overlapping prefix/suffix wildcards

Agent-Logs-Url: https://github.com/github/gh-aw/sessions/c74ec611-e731-43e9-bc12-db4771175f78

Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix logger matchPattern to check length guard for wildcards fix: add length guard in matchPattern to prevent false positives with overlapping prefix/suffix wildcards Apr 28, 2026
Copilot AI requested a review from gh-aw-bot April 28, 2026 21:10
@pelikhan pelikhan marked this pull request as ready for review April 28, 2026 21:58
Copilot AI review requested due to automatic review settings April 28, 2026 21:58
@pelikhan pelikhan merged commit 2c28976 into main Apr 28, 2026
20 checks passed
@pelikhan pelikhan deleted the copilot/fix-logger-matchpattern-length-guard branch April 28, 2026 21:59
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes matchPattern incorrectly returning true for middle-wildcard patterns when the required prefix+suffix would have to overlap (e.g., "ab*ba" matching "aba").

Changes:

  • Add a length guard in the middle-wildcard branch of matchPattern to prevent overlap-based false positives.
  • Add TestMatchPattern cases for the overlapping scenario (should not match) and the exact-boundary scenario (should match).
Show a summary per file
File Description
pkg/logger/logger.go Adds len(namespace) >= len(prefix)+len(suffix) guard for middle * patterns to avoid overlapping prefix/suffix matches.
pkg/logger/logger_test.go Adds regression tests covering overlapping and exact-boundary middle-wildcard matching.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 2/2 changed files
  • Comments generated: 0

@github-actions github-actions Bot mentioned this pull request Apr 28, 2026
@github-actions
Copy link
Copy Markdown
Contributor

🧪 Test Quality Sentinel Report

Test Quality Score: 100/100

Excellent test quality

Metric Value
New/modified tests analyzed 2 (table rows)
✅ Design tests (behavioral contracts) 2 (100%)
⚠️ Implementation tests (low value) 0 (0%)
Tests with error/edge cases 2 (100%)
Duplicate test clusters 0
Test inflation detected No (test +2 lines, prod +4 lines, ratio 0.5:1)
🚨 Coding-guideline violations None

Test Classification Details

Test File Classification Issues Detected
middle wildcard overlapping prefix suffix pkg/logger/logger_test.go:391 ✅ Design None — directly verifies the bug-fix behavioral contract
middle wildcard exact boundary pkg/logger/logger_test.go:392 ✅ Design None — verifies exact boundary condition of the fix

Analysis

Both added test cases are new rows in the existing TestMatchPattern table-driven test. They directly target the behavioral contract described in the PR: that matchPattern("aba", "ab*ba") returns false (overlapping prefix/suffix must not produce a false positive) and matchPattern("abba", "ab*ba") returns true (exact boundary works correctly).

Classification: Design tests — they assert observable return values of matchPattern(), which is what any caller of the function would rely on.

Edge case coverage: ✅ The overlapping case ("aba" is shorter than "ab" + "ba") is exactly the bug scenario. The boundary case tests the minimum valid input.

Assertion quality: Uses t.Errorf("matchPattern(%q, %q) = %v, want %v", ...) — descriptive format string provides clear failure context. ✅

Build tag: //go:build !integration present on line 1. ✅

Mock libraries: None. ✅


Language Support

Tests analyzed:

  • 🐹 Go (*_test.go): 2 test cases — unit (//go:build !integration)
  • 🟨 JavaScript (*.test.cjs, *.test.js): 0 tests

Verdict

Check passed. 0% of new tests are implementation tests (threshold: 30%). The two added test cases precisely target the behavioral fix introduced in this PR with clear edge-case and boundary coverage.

📖 Understanding Test Classifications

Design Tests (High Value) verify what the system does:

  • Assert on observable outputs, return values, or state changes
  • Cover error paths and boundary conditions
  • Would catch a behavioral regression if deleted
  • Remain valid even after internal refactoring

Implementation Tests (Low Value) verify how the system does it:

  • Assert on internal function calls (mocking internals)
  • Only test the happy path with typical inputs
  • Break during legitimate refactoring even when behavior is correct
  • Give false assurance: they pass even when the system is wrong

Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators.

References: §25079858155

🧪 Test quality analysis by Test Quality Sentinel · ● 613.8K ·

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Test Quality Sentinel: 100/100. Test quality is excellent — 0% of new tests are implementation tests (threshold: 30%). Both added test cases are high-value design tests that directly verify the behavioral contract of the bug fix.

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.

logger matchPattern missing length guard causes false positives with overlapping prefix/suffix wildcards

4 participants