fix: Substring symbol names cross-tainting - #90
Merged
Conversation
quanb-duy
approved these changes
Aug 7, 2026
martinnaj
enabled auto-merge
August 7, 2026 17:20
📝 WalkthroughWalkthroughThe analyzer now matches plain identifiers by whole-token boundaries across intra-file dependency and taint-propagation paths. Non-identifier names retain substring matching. The project version and changelog now record release ChangesIdentifier matching
Estimated code review effort: 2 (Simple) | ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/analyzer/astdiff.go`:
- Around line 576-587: Update containsIdentifier in internal/analyzer/astdiff.go
to use TypeScript tokenization or an equivalent grammar-aware scanner for
identifier matching, preventing matches inside Unicode or escaped identifiers
such as foo\u0062ar, fooé, or éx. Preserve plain substring behavior for
non-identifiers only, and add regression tests covering these boundary cases.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 72fb6634-dcee-47eb-9aa2-cad414603a24
📒 Files selected for processing (4)
CHANGELOG.mdVERSIONinternal/analyzer/analyzer.gointernal/analyzer/astdiff.go
Comment on lines
+576
to
+587
| 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]) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n 'containsIdentifier|isPlainIdentifier|isIdentByte' internal/analyzer --glob '*.go'
rg -nP '\\u[0-9A-Fa-f]{4}|[^\x00-\x7F]' internal/analyzer --glob '*_test.go' || trueRepository: gooddata/gooddata-goodchanges
Length of output: 1317
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- target implementation ---'
sed -n '130,185p;540,620p' internal/analyzer/astdiff.go
printf '%s\n' '--- parser and identifier-related code ---'
rg -n 'Parse|parser|Identifier|identifier|bodyText|containsIdentifier' internal/analyzer go.mod go.sum --glob '*.go' --glob 'go.mod' --glob 'go.sum' | head -240
printf '%s\n' '--- relevant tests ---'
fd -i '.*_test\.go$' internal/analyzer -x sh -c 'rg -n -C 3 "containsIdentifier|unicode|escaped|identifier|Rename|rename" "$1"' sh {} || trueRepository: gooddata/gooddata-goodchanges
Length of output: 7240
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- repository files and module dependencies ---'
git ls-files | rg '(^|/)(tsparse|parser|.*_test\.go$)|go\.mod$|go\.sum$' | head -240
sed -n '1,180p' go.mod
printf '%s\n' '--- symbol extraction and identifier handling ---'
sed -n '220,300p' internal/analyzer/astdiff.go
rg -n -C 4 'type Symbol|struct .*Symbol|Symbols =|Name:' --glob '*.go' .
printf '%s\n' '--- parser package API references ---'
rg -n -C 3 'package tsparse|func .*Parse|func .*Extract|IsIdentifier|IdentifierName|Unicode|escaped' --glob '*.go' .Repository: gooddata/gooddata-goodchanges
Length of output: 25678
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- tsparse analysis construction ---'
sed -n '1,115p;300,435p' internal/tsparse/tsparse.go
printf '%s\n' '--- vendor scanner and source-file APIs ---'
fd -i '.*\.go$' _vendor/typescript-go | head -80
rg -n -C 3 'func .*Scan|type Scanner|Scanner|Token.*Identifier|Identifier.*Text|Unescape|UnicodeEscape|Text\(\)' _vendor/typescript-go --glob '*.go' | head -320
printf '%s\n' '--- analyzer tests and test helpers ---'
git ls-files internal/analyzer | sort
rg -n -C 5 'FindAffected|affected|repointed|intra-file|taint|astdiff' internal/analyzer --glob '*_test.go' || trueRepository: gooddata/gooddata-goodchanges
Length of output: 7547
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- module layout and submodules ---'
git submodule status || true
git ls-tree HEAD _vendor/typescript-go
cat .gitmodules 2>/dev/null || true
find . -maxdepth 3 -type d -name '*typescript*' -o -name '_vendor'
printf '%s\n' '--- scanner usage in tracked source ---'
rg -n -C 5 'scanner\.' internal --glob '*.go'
sed -n '1,110p' internal/tsparse/tsparse.go
sed -n '400,440p' internal/tsparse/tsparse.go
printf '%s\n' '--- tests in the analyzer package ---'
git ls-files internal/analyzer | sort
git ls-files '*_test.go' | wc -lRepository: gooddata/gooddata-goodchanges
Length of output: 8719
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
def is_ident_byte(b):
return b in (ord('_'), ord('$')) or 65 <= b <= 90 or 97 <= b <= 122 or 48 <= b <= 57
def is_plain_identifier(s):
return bool(s) and all(is_ident_byte(b) for b in s.encode())
def contains_identifier(text, name):
if not name:
return False
if not is_plain_identifier(name):
return name in text
start = 0
while True:
i = text.find(name, start)
if i < 0:
return False
end = i + len(name)
before_ok = i == 0 or not is_ident_byte(text.encode()[i - 1])
after_ok = end == len(text) or not is_ident_byte(text.encode()[end])
if before_ok and after_ok:
return True
start = i + 1
cases = [
("const fooé = 1", "foo"),
(r"const foo\u0062ar = 1", "foo"),
("const é = 1", "é"),
("const éx = 1", "é"),
]
for text, name in cases:
print(repr(text), repr(name), contains_identifier(text, name))
PY
printf '%s\n' '--- exact call sites and absence of tests ---'
rg -n -C 3 'containsIdentifier\(' internal/analyzer --glob '*.go'
git ls-files '*_test.go' | sortRepository: gooddata/gooddata-goodchanges
Length of output: 4155
Use parser-aware identifier boundaries.
containsIdentifier can match part of a Unicode or escaped identifier, such as foo in foo\u0062ar or fooé. Non-ASCII names use raw substring matching and can match inside éx. This creates false taint links. Add regression tests and use TypeScript tokenization or an equivalent grammar-aware scanner.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/analyzer/astdiff.go` around lines 576 - 587, Update
containsIdentifier in internal/analyzer/astdiff.go to use TypeScript
tokenization or an equivalent grammar-aware scanner for identifier matching,
preventing matches inside Unicode or escaped identifiers such as foo\u0062ar,
fooé, or éx. Preserve plain substring behavior for non-identifiers only, and add
regression tests covering these boundary cases.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Risk: low
Summary by CodeRabbit
Bug Fixes
Chores