Skip to content

coverage.findProfile: suffix-match over unordered map can nondeterministically return the wrong files profile #67102

Description

@github-actions

Problem
coverage.findProfile (pkg/linters/internal/coverage/coverage.go:81-98) matches the on-disk absolute filename of a diagnostic position against every profile key using a plain substring-suffix check (strings.HasSuffix(norm, "/"+normKey)), iterating idx.profiles (a Go map) and returning on the first key that satisfies the check. It never compares candidates for specificity and never requires equality beyond the loose suffix condition.

Root cause
Because Go map iteration order is randomized per process, if two distinct source files module-relative paths are in a suffix relationship (for example a top-level package linters/foo.go and a nested package pkg/linters/foo.go), a query for the longer paths coverage data can match either key depending on which one the map happens to visit first. The function has no preference for the longest or most specific match before returning; it just takes whichever qualifying key it reaches first during iteration.

Distinct from prior fixes
This is not the bug fixed by #52566 / #52309 (that issue was that findProfile NEVER matched anything on a standard Actions checkout, a false-negative no-op problem solved by adding the modulePrefix-stripped relative comparison). This is the opposite failure mode sitting inside the now-fixed code path: once a match is found it can be the WRONG match whenever more than one key satisfies the suffix check. It is also distinct from the coverage.hitCount line-only column blindness already filed as sg90a1 / #66773 (that bug conflates two spans within one file at one line; this bug conflates two different files entirely).

Impact
Currently fully latent: GH_AW_LINT_COVERAGE_PROFILE is never set in any CI workflow (confirmed via grep of .github), so ShouldApply always takes its permissive fallback today. But per ADR-51573 this is the shared gating primitive meant to activate eventually for all 15 perf linters at once (appendbytestring, appendoneelement, bytesbufferstring, bytescomparestring, lenstringsplit, mapclearloop, reflectdeepequalusage, seenmapbool, slicemakezerolength, sortslice, stringbytesroundtrip, stringsconcatloop, stringsjoinone, tolowerequalfold, writebytestring). Once enabled, any file pair meeting the suffix condition would see findings for one of the two files flip between suppressed and reported from run to run on identical coverage data, since the outcome depends on map iteration order rather than file content. I checked several common basenames in the current tree (types.go, config.go, registry.go) for a live colliding pair and found none today, so this is a structural risk rather than a confirmed misattribution in this repository right now.

Recommendation
Replace the suffix scan with either (a) an exact-match lookup against the already module-relative-normalized key, since after stripping modulePrefix the relative path should equal the on-disk path relative to the repository root with no loose suffix matching needed, or (b) if suffix matching must be retained for profiles built from a different module, collect all candidates and prefer the longest / most specific key rather than returning on first iteration.

Validation checklist

  • Add a regression test with two synthetic profile keys in a suffix relationship (e.g. a/foo.go and pkg/a/foo.go) asserting the correct one is always returned regardless of map insertion order, mirroring the TestFilenameMatchingActionsStyleAbsolutePath pattern already used for the Fix coverage.findProfile path matching so perf-linter coverage gating actually applies #52566 fix.
  • Run the new test with -count=10 or GODEBUG=mapiterstart-style perturbation if available to catch order-dependence.

Effort: Quick (less than 1 hour).

Generated by 🤖 Sergo - Serena Go Expert · claude · agent · 342.1 AIC · ⊞ 5.2K · ◷

  • expires on Oct 15, 2026, 8:09 PM UTC-08:00

Activity

  1. pelikhan commented on Oct 11, 2026

    @pelikhan
    Collaborator

    Reviewed against completed essentials issue #67451 (#67451) and merged PR #67544 (#67544).

    Coverage profile selection now chooses the most-specific filename match deterministically.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    cookieIssue Monster Loves Cookies!sergo

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions