From 375002089ed6a11971fca78601ee35205d5f8839 Mon Sep 17 00:00:00 2001 From: Martin Najemi Date: Fri, 7 Aug 2026 19:19:32 +0200 Subject: [PATCH] fix: Substring symbol names cross-tainting Risk: low --- CHANGELOG.md | 6 +++++ VERSION | 2 +- internal/analyzer/analyzer.go | 10 ++++---- internal/analyzer/astdiff.go | 47 ++++++++++++++++++++++++++++++++++- 4 files changed, 58 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d5443df..c189bcc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,11 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [0.25.4] - 2026-08-07 + +### Fixed +- Intra-file taint propagation now matches symbol names as whole identifiers instead of raw substrings. The dependency/usage checks used `strings.Contains(body, name)`, so a symbol whose name is a **substring** of another was falsely linked — e.g. removing the unused `chooseAction` tainted the surviving, used `chooseActionByIndex` (its body "contains" the string `chooseAction`), which then propagated to a spec and flagged `gdc-dashboards-e2e` for a dead-code deletion. All six propagation sites (`astdiff.go` intra-file graph, plus the seed/importer/usage propagation in `analyzer.go`) now require the name to appear flanked by non-identifier characters (`[A-Za-z0-9_$]`) via a shared `containsIdentifier` helper; non-identifier tokens like the `*` wildcard keep the substring behaviour. Strictly more precise (it only drops matches that were substrings inside a larger identifier — never a real usage), so no false negatives. + ## [0.25.3] - 2026-08-07 ### Fixed @@ -408,6 +413,7 @@ Together these keep genuine import-time changes flagged while eliminating the la - Multi-stage Docker build - Automated vendor upgrade workflow +[0.25.4]: https://github.com/gooddata/gooddata-goodchanges/compare/v0.25.3...v0.25.4 [0.25.3]: https://github.com/gooddata/gooddata-goodchanges/compare/v0.25.2...v0.25.3 [0.25.2]: https://github.com/gooddata/gooddata-goodchanges/compare/v0.25.1...v0.25.2 [0.25.1]: https://github.com/gooddata/gooddata-goodchanges/compare/v0.25.0...v0.25.1 diff --git a/VERSION b/VERSION index 1bb9b67..0604843 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.25.3 \ No newline at end of file +0.25.4 \ No newline at end of file diff --git a/internal/analyzer/analyzer.go b/internal/analyzer/analyzer.go index 64fe2e1..4edf397 100644 --- a/internal/analyzer/analyzer.go +++ b/internal/analyzer/analyzer.go @@ -705,7 +705,7 @@ func AnalyzeLibraryPackage(projectFolder string, entrypoints []Entrypoint, merge } bodyText := tsparse.ExtractTextForLines(sourceText, lineMap, sym.StartLine, sym.EndLine) for tName := range names { - if strings.Contains(bodyText, tName) { + if containsIdentifier(bodyText, tName) { names[sym.Name] = true changed = true log.Debugf(" %s: %s tainted via intra-file dep on %s (seed propagation)", stem, sym.Name, tName) @@ -834,7 +834,7 @@ func AnalyzeLibraryPackage(projectFolder string, entrypoints []Entrypoint, merge } bodyText := tsparse.ExtractTextForLines(sourceText, lineMap, sym.StartLine, sym.EndLine) for tName := range taintedSet { - if strings.Contains(bodyText, tName) { + if containsIdentifier(bodyText, tName) { taintedSet[sym.Name] = true newlyTainted = append(newlyTainted, sym.Name) changed = true @@ -1021,7 +1021,7 @@ func findTaintedSymbolsByUsage(analysis *tsparse.FileAnalysis, taintedNames []st for _, sym := range analysis.Symbols { bodyText := tsparse.ExtractTextForLines(sourceText, lineMap, sym.StartLine, sym.EndLine) for tName := range taintSet { - if strings.Contains(bodyText, tName) { + if containsIdentifier(bodyText, tName) { result = append(result, sym.Name) break } @@ -1558,7 +1558,7 @@ func FindAffectedFiles(globPattern string, filterPattern string, upstreamTaint m } bodyText := tsparse.ExtractTextForLines(sourceText, lineMap, sym.StartLine, sym.EndLine) for tName := range names { - if strings.Contains(bodyText, tName) { + if containsIdentifier(bodyText, tName) { names[sym.Name] = true changed = true log.Debugf(" %s: %s tainted via intra-file dep on %s (seed propagation)", stem, sym.Name, tName) @@ -1673,7 +1673,7 @@ func FindAffectedFiles(globPattern string, filterPattern string, upstreamTaint m } bodyText := tsparse.ExtractTextForLines(sourceText, lineMap, sym.StartLine, sym.EndLine) for tName := range taintedSet { - if strings.Contains(bodyText, tName) { + if containsIdentifier(bodyText, tName) { taintedSet[sym.Name] = true newlyTainted = append(newlyTainted, sym.Name) changed = true diff --git a/internal/analyzer/astdiff.go b/internal/analyzer/astdiff.go index 2ddb0e9..381df48 100644 --- a/internal/analyzer/astdiff.go +++ b/internal/analyzer/astdiff.go @@ -164,7 +164,7 @@ func findAffectedSymbolsByASTDiff(oldAnalysis *tsparse.FileAnalysis, newAnalysis bodyText := tsparse.ExtractTextForLines(newText, newLineMap, sym.StartLine, sym.EndLine) deps := make(map[string]bool) for _, other := range newAnalysis.Symbols { - if other.Name != sym.Name && strings.Contains(bodyText, other.Name) { + if other.Name != sym.Name && containsIdentifier(bodyText, other.Name) { deps[other.Name] = true } } @@ -564,6 +564,51 @@ func repointedImportBindings(oldA, newA *tsparse.FileAnalysis, includeTypes bool return repointed } +// containsIdentifier reports whether name occurs in text as a whole identifier +// token — flanked by non-identifier characters — rather than as a substring of a +// larger identifier. This prevents false taint links like `chooseAction` matching +// inside `chooseActionByIndex`. Names that are not plain identifiers (e.g. the "*" +// wildcard or "*:ns" namespace markers) fall back to a raw substring test. +func containsIdentifier(text, name string) bool { + if name == "" { + return false + } + if !isPlainIdentifier(name) { + return strings.Contains(text, name) + } + for from := 0; ; { + i := strings.Index(text[from:], name) + if i < 0 { + return false + } + start := from + i + end := start + len(name) + beforeOK := start == 0 || !isIdentByte(text[start-1]) + afterOK := end == len(text) || !isIdentByte(text[end]) + if beforeOK && afterOK { + return true + } + from = start + 1 + } +} + +func isPlainIdentifier(s string) bool { + if s == "" { + return false + } + for i := 0; i < len(s); i++ { + if !isIdentByte(s[i]) { + return false + } + } + return true +} + +func isIdentByte(b byte) bool { + return b == '_' || b == '$' || + (b >= 'a' && b <= 'z') || (b >= 'A' && b <= 'Z') || (b >= '0' && b <= '9') +} + // bareImportSources returns the set of module specifiers imported purely for // their side effects (`import "./x"` — no bindings). These execute at import time. func bareImportSources(a *tsparse.FileAnalysis) map[string]bool {