From c79fe4ab1188fa20ea67f5222cd6eea69ebece96 Mon Sep 17 00:00:00 2001 From: Martin Najemi Date: Mon, 10 Aug 2026 10:58:43 +0200 Subject: [PATCH] fix: Side-effect taint stopping at first importer Risk: low --- CHANGELOG.md | 6 ++++++ VERSION | 2 +- internal/analyzer/analyzer.go | 29 +++++++++++++++++++++++++++++ internal/analyzer/astdiff.go | 17 +++++++++++++---- 4 files changed, 49 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c189bcc..b7a4651 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.5] - 2026-08-10 + +### Fixed +- Import-time side-effect taint now propagates **transitively** through import and re-export edges. A file whose change ran at import time (a changed top-level statement like `console.log(...)`, or a bare `import "x"`) tainted its *direct* importers, but the "runs at import time" quality was lost after one hop: a barrel that re-exports the side-effectful module (`export { … } from "./api/ai/ai.js"` in an entrypoint `index.ts`) only picked up the re-exported *symbols*, so consumers importing the barrel for *other* symbols were missed — e.g. a `console.log` added to `e2e-utils/src/api/ai/ai.ts` flagged only the 5 consumers of the `ai` exports instead of all 8 e2e-utils consumers (importing the barrel loads `ai.ts` and executes the statement regardless of which symbol is used). Such changes now carry a `__side-effect__` sentinel that flows through every import/re-export edge, marking each file it reaches as wholly tainted and itself side-effectful, so barrels become side-effectful and all their consumers are flagged — the same result as adding the statement to the entrypoint directly. This is deliberately assume-the-worst; a `TODO` in the propagation notes the follow-up to refine it using each package's `package.json` `"sideEffects"` field (a side-effect-free module is tree-shaken and should not propagate). + ## [0.25.4] - 2026-08-07 ### Fixed @@ -413,6 +418,7 @@ Together these keep genuine import-time changes flagged while eliminating the la - Multi-stage Docker build - Automated vendor upgrade workflow +[0.25.5]: https://github.com/gooddata/gooddata-goodchanges/compare/v0.25.4...v0.25.5 [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 diff --git a/VERSION b/VERSION index 0604843..2e25e5e 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.25.4 \ No newline at end of file +0.25.5 \ No newline at end of file diff --git a/internal/analyzer/analyzer.go b/internal/analyzer/analyzer.go index 4edf397..bb33628 100644 --- a/internal/analyzer/analyzer.go +++ b/internal/analyzer/analyzer.go @@ -783,6 +783,23 @@ func AnalyzeLibraryPackage(projectFolder string, entrypoints []Entrypoint, merge } } + // Import-time side-effect transitivity: if the imported module has an + // import-time side effect (sideEffectTaint), importing it re-runs that + // side effect here, so this file becomes side-effectful too — taint all + // its symbols and carry "*" + sideEffectTaint so it keeps flowing to this + // file's own importers/re-exporters (a barrel re-exporting a side-effectful + // module becomes side-effectful itself). + // TODO: make this precise using the "sideEffects" field in each package's + // package.json — a module marked side-effect-free is tree-shaken and not + // re-executed on import, so it should not propagate. Until then we assume + // the worst and propagate through every import/re-export edge. Follow-up. + if currentTainted[sideEffectTaint] { + for _, sym := range importerAnalysis.Symbols { + newlyTainted = append(newlyTainted, sym.Name) + } + newlyTainted = append(newlyTainted, "*", sideEffectTaint) + } + // Named imports: find symbols that use the tainted imports if len(taintedLocalNames) > 0 { usageTainted := findTaintedSymbolsByUsage(importerAnalysis, taintedLocalNames) @@ -1624,6 +1641,18 @@ func FindAffectedFiles(globPattern string, filterPattern string, upstreamTaint m } } + // Import-time side-effect transitivity — see the matching block in + // AnalyzeLibraryPackage. Importing a side-effectful module re-runs its + // side effect here, so this file becomes side-effectful too and keeps + // propagating it (assume-the-worst; refine later via package.json + // "sideEffects" — see that TODO). + if currentTainted[sideEffectTaint] { + for _, sym := range importerAnalysis.Symbols { + newlyTainted = append(newlyTainted, sym.Name) + } + newlyTainted = append(newlyTainted, "*", sideEffectTaint) + } + if len(taintedLocalNames) > 0 { usageTainted := findTaintedSymbolsByUsage(importerAnalysis, taintedLocalNames) newlyTainted = append(newlyTainted, usageTainted...) diff --git a/internal/analyzer/astdiff.go b/internal/analyzer/astdiff.go index 381df48..454c9ad 100644 --- a/internal/analyzer/astdiff.go +++ b/internal/analyzer/astdiff.go @@ -9,6 +9,14 @@ import ( "goodchanges/tsgo-vendor/pkg/scanner" ) +// sideEffectTaint is a sentinel taint token marking that a file has an *import-time +// side effect* change (a changed top-level side-effect statement, or a bare +// `import "x"` side-effect import) — as opposed to an ordinary whole-file "*" taint +// (e.g. a new file). It propagates through import/re-export edges so a barrel that +// re-exports a side-effectful module becomes side-effectful itself. It is not a +// valid JS identifier, so it can never collide with or be matched as a real symbol. +const sideEffectTaint = "__side-effect__" + // findAffectedSymbolsByASTDiff compares OLD and NEW file ASTs to find which symbols changed. // Returns symbol names that have runtime changes (or type-only changes if includeTypes is true). // @@ -232,10 +240,11 @@ func findAffectedSymbolsByASTDiff(oldAnalysis *tsparse.FileAnalysis, newAnalysis if hasSideEffectStmtChanges(oldAnalysis.SourceFile, newAnalysis.SourceFile) || bareImportsChanged(oldAnalysis, newAnalysis) { log.Debugf(" file changed with import-time side effects — tainting all symbols") - // Use "*" wildcard to mark all exports as affected. - // This handles barrel/entrypoint files that have no symbol declarations - // but whose runtime side effects affect all importers. - affected = append(affected, "*") + // Use "*" wildcard to mark all exports as affected, plus the + // sideEffectTaint sentinel so the *import-time* nature propagates + // through import/re-export edges (a barrel importing this becomes + // side-effectful too). + affected = append(affected, "*", sideEffectTaint) for _, sym := range newAnalysis.Symbols { if sym.IsTypeOnly && !includeTypes { continue