Skip to content

fix: resolve ${VAR} bash sources against the variable's real base#2193

Closed
Souptik96 wants to merge 1 commit into
Graphify-Labs:v8from
Souptik96:fix/2172-bash-var-source-tracked-base
Closed

fix: resolve ${VAR} bash sources against the variable's real base#2193
Souptik96 wants to merge 1 commit into
Graphify-Labs:v8from
Souptik96:fix/2172-bash-var-source-tracked-base

Conversation

@Souptik96

Copy link
Copy Markdown
Contributor

Fixes #2172

Reproduced

Confirmed the false positive against v8 (0.9.26) with your exact shape —
scripts/deploy.sh where ROOT is the script dir's parent, plus a same-named decoy
under the script dir:

ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
source "${ROOT}/lib/utils.sh"     # real target: <root>/lib/utils.sh

Before — bound to the decoy:

imports_from -> ..._temp_bashrepro_..._scripts_lib_utils_sh   INFERRED
bash_sources: [{'target_path': '...\bashrepro-...\scripts\lib\utils.sh', ...}]

After — bound to the real file:

imports_from -> ..._temp_bashrepro_..._lib_utils_sh           INFERRED
bash_sources: [{'target_path': '...\bashrepro-...\lib\utils.sh', ...}]

Fix

Track top-level variable assignments and use the assigned base for the leading variable,
exactly as the issue suggests:

  • the script-dir idiom"$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" — resolves
    to the script dir walked up once per trailing /.. hop (any number, and the $0 spelling
    is accepted too)
  • a literal value resolves as-is when absolute, or against the script dir when relative
  • anything else (a value built from other variables, or command substitution we don't
    model) stays untracked, so the previous script-dir guess is kept rather than binding
    somewhere invented

Only the base changes. Every existing guard is untouched: an expansion left in the suffix,
an empty suffix, and a .. component in the suffix are all still rejected, the edge is still
gated on is_file(), and it is still emitted as INFERRED.

Assignments are read from the parse tree's top-level variable_assignment nodes, so a
reassignment inside a function body doesn't feed the base.

Testing

Three new tests in tests/test_extract.py — the false positive, plus two guards that the
behaviour #2079 added still holds:

$ uv run pytest tests/test_extract.py -q -k "tracked_assignment_base or script_dir_idiom_still_resolves or untracked_var_keeps_script_dir"
3 passed, 149 deselected, 1 warning in 1.36s

Confirmed the false-positive test actually catches the bug — same tests with
graphify/extractors/bash.py reverted to v8 (note the two guard tests still pass there,
which is the point: they assert preserved behaviour):

FAILED tests/test_extract.py::test_extract_bash_var_source_uses_tracked_assignment_base
1 failed, 2 passed, 149 deselected

Affected suites and both pre-commit gates:

$ uv run pytest tests/test_extract.py tests/test_symbol_resolution.py tests/test_languages.py -q
4 failed, 521 passed, 1 warning in 22.51s

$ uv run ruff check --config pyproject.toml graphify/extractors/bash.py tests/test_extract.py
All checks passed!

$ uv run python -m tools.skillgen --check
check OK: 134 artifact(s) match committed output and expected/.

Those 4 failures are pre-existing on Windows, not from this PR — exactly the 4
test_extract.py failures in my baseline on unmodified v8 (66d8110), all symlink-related
(test_collect_files_*, e.g. OSError: [WinError 1314] A required privilege is not held by the client). Full-suite baseline there is 45 failed / 3578 passed / 15 skipped. No
Linux/macOS box here to confirm they're green.

Sent separately from #2171 (the extensionless/bare-name source gaps) so the two are
reviewable independently, though both touch graphify/extractors/bash.py.

…aphify-Labs#2172)

Graphify-Labs#2079 resolved `source "${VAR}/lib/x.sh"` by stripping the leading expansion and
resolving the literal suffix against the sourcing script's own directory. That is
correct for the canonical
`DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"` idiom, but wrong whenever
the variable points somewhere else. With

    ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
    source "${ROOT}/lib/utils.sh"

the real target is <root>/lib/utils.sh, yet a same-named decoy under the script
dir (<root>/scripts/lib/utils.sh) won: a wrong imports_from edge to a real node.

Track top-level variable assignments and use the assigned base for the leading
variable:

- the script-dir idiom, including any number of trailing `/..` hops (and the
  `$0` spelling), resolves to the script dir walked up that many times
- a literal value resolves as-is when absolute, or against the script dir when
  relative
- anything else -- a value built from other variables, or command substitution we
  do not model -- stays untracked, so the previous script-dir guess is kept

Only the base changes. The suffix guards are untouched: an expansion left in the
suffix, an empty suffix, and a `..` component in the suffix are all still
rejected, the edge is still gated on is_file() and still emitted as INFERRED.
@Souptik96 Souptik96 changed the title fix: resolve \ bash sources against the variable's real base fix: resolve ${VAR} bash sources against the variable's real base Jul 25, 2026
@safishamsi

Copy link
Copy Markdown
Collaborator

Thanks @Souptik96. Shipped in v0.9.27 (cherry-picked to v8). Closed-unmerged here, but it's in the release: https://github.com/Graphify-Labs/graphify/releases/tag/v0.9.27

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.

Bash ${VAR} source: track literal assignments to avoid a wrong-directory false positive

2 participants