fix(extractors/bash): resolve dirname cmdsubst and dotdot suffix in source paths (#2596)
Extends variable-built `source` resolution to two path-construction forms that were silently dropped: `source "$(dirname "$VAR")/lib/x.sh"` (command substitution in the source argument) and `source "$VAR/../lib/x.sh"` (a `..` suffix when the leading var is a tracked var_bases entry). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Josh Hudson
committed by
safishamsi
safishamsi
co-authored by
Claude Opus 4.8
Claude Opus 4.8
parent
d79520198d
commit
b345dc4993
+94
-29
@@ -1,6 +1,7 @@
|
||||
"""Bash extractor. Moved verbatim from graphify/extract.py."""
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import re
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
@@ -17,18 +18,33 @@ _BASH_LEADING_EXPANSION = re.compile(
|
||||
)
|
||||
|
||||
|
||||
def _bash_source_suffix(raw: str) -> str | None:
|
||||
def _bash_source_suffix(raw: str, allow_dotdot: bool = False) -> str | None:
|
||||
"""Return the literal path suffix of a variable-built `source` argument, or
|
||||
None when the remainder is empty, still holds an expansion, or escapes upward
|
||||
with ``..``. ``"${DIR}/lib/x.sh"`` -> ``"lib/x.sh"``."""
|
||||
None when the remainder is empty or still holds an expansion.
|
||||
|
||||
``"${DIR}/lib/x.sh"`` -> ``"lib/x.sh"``.
|
||||
|
||||
When *allow_dotdot* is False (the default — the base is a script-dir guess),
|
||||
``..`` segments are rejected to prevent traversal outside the project tree.
|
||||
When True (the base is a tracked var_bases entry — a known directory),
|
||||
``..`` is permitted and the caller normalises with ``os.path.normpath``.
|
||||
"""
|
||||
suffix = _BASH_LEADING_EXPANSION.sub("", raw, count=1).lstrip("/")
|
||||
if not suffix or "$" in suffix:
|
||||
return None
|
||||
if ".." in suffix.split("/"):
|
||||
if not allow_dotdot and ".." in suffix.split("/"):
|
||||
return None
|
||||
return suffix
|
||||
|
||||
|
||||
# Recognise ``$(dirname "$VAR")`` (or ``$(dirname "${VAR}")``) at the start of
|
||||
# a ``source`` argument, capturing the variable name so the source resolver
|
||||
# can treat the whole construct as ``var_bases[VAR].parent`` (#2596 form 3).
|
||||
_BASH_SOURCE_DIRNAME = re.compile(
|
||||
r'\$\(dirname\s+"\$\{?([A-Za-z_][A-Za-z0-9_]*)\}?"\)'
|
||||
)
|
||||
|
||||
|
||||
# Name of the leading variable of a `source` argument: `${ROOT}/lib/x.sh` -> ROOT.
|
||||
_BASH_LEADING_VAR = re.compile(
|
||||
r"^\$\{([A-Za-z_][A-Za-z0-9_]*)[^}]*\}|^\$([A-Za-z_][A-Za-z0-9_]*)"
|
||||
@@ -295,33 +311,82 @@ def extract_bash(path: Path) -> dict:
|
||||
# emit INFERRED (the expansion can't be proven
|
||||
# statically) only when it resolves to a real file,
|
||||
# never a dead id.
|
||||
suffix = _bash_source_suffix(raw)
|
||||
if suffix:
|
||||
# Resolve against the variable's tracked base when
|
||||
# we know it, else fall back to the script's own
|
||||
# directory as before (#2172).
|
||||
base = path.parent
|
||||
|
||||
# Form 3 (#2596): `source "$(dirname "$VAR")/lib/x.sh"`
|
||||
# — command substitution in the source argument.
|
||||
# Recognise the dirname idiom on the source line the
|
||||
# same way _bash_assignment_base does on assignment
|
||||
# lines: treat `$(dirname "$VAR")` as
|
||||
# `var_bases[VAR].parent` (falling back to
|
||||
# `script_dir.parent` when VAR is untracked), then
|
||||
# resolve the literal suffix against it.
|
||||
dirname_match = _BASH_SOURCE_DIRNAME.match(raw)
|
||||
if dirname_match:
|
||||
var_name = dirname_match.group(1)
|
||||
if var_name in var_bases:
|
||||
base = var_bases[var_name].parent
|
||||
else:
|
||||
# Untracked var: fall back to the script's
|
||||
# own directory, so dirname resolves to its
|
||||
# parent — matching the existing untracked-var
|
||||
# fallback semantics.
|
||||
base = path.parent.parent
|
||||
# Strip the $(dirname ...) prefix and any leading
|
||||
# slash to get the literal suffix.
|
||||
suffix = raw[dirname_match.end():].lstrip("/")
|
||||
if suffix and "$" not in suffix:
|
||||
resolved = (base / suffix).resolve()
|
||||
if resolved.is_file():
|
||||
add_edge(file_nid, _make_id(str(resolved)),
|
||||
"imports_from", line,
|
||||
confidence="INFERRED", context="import",
|
||||
target_file=str(resolved))
|
||||
bash_sources.append({
|
||||
"target_path": str(resolved),
|
||||
"source_file": str_path,
|
||||
"source_location": f"L{line}",
|
||||
})
|
||||
else:
|
||||
# Check if the leading variable is tracked
|
||||
# before deciding whether to allow `..` in
|
||||
# the suffix (Form 4, #2596). When the base
|
||||
# is a known var_bases entry, `..` is safe.
|
||||
var_match = _BASH_LEADING_VAR.match(raw)
|
||||
allow_dotdot = False
|
||||
if var_match:
|
||||
var_name = var_match.group(1) or var_match.group(2)
|
||||
if var_name in var_bases:
|
||||
base = var_bases[var_name]
|
||||
resolved = (base / suffix).resolve()
|
||||
if resolved.is_file():
|
||||
add_edge(file_nid, _make_id(str(resolved)),
|
||||
"imports_from", line,
|
||||
confidence="INFERRED", context="import",
|
||||
target_file=str(resolved))
|
||||
# Integration (#2141 + #2079): record the resolved
|
||||
# sourced file so calls into its functions resolve
|
||||
# too, not just the source edge. target_path is
|
||||
# absolute here; resolve_bash_source_edges takes it
|
||||
# as-is.
|
||||
bash_sources.append({
|
||||
"target_path": str(resolved),
|
||||
"source_file": str_path,
|
||||
"source_location": f"L{line}",
|
||||
})
|
||||
_vn = var_match.group(1) or var_match.group(2)
|
||||
if _vn in var_bases:
|
||||
allow_dotdot = True
|
||||
suffix = _bash_source_suffix(raw, allow_dotdot=allow_dotdot)
|
||||
if suffix:
|
||||
# Resolve against the variable's tracked base
|
||||
# when we know it, else fall back to the
|
||||
# script's own directory as before (#2172).
|
||||
base = path.parent
|
||||
if var_match:
|
||||
var_name = var_match.group(1) or var_match.group(2)
|
||||
if var_name in var_bases:
|
||||
base = var_bases[var_name]
|
||||
if var_match and var_name in var_bases:
|
||||
resolved = Path(os.path.normpath(base / suffix))
|
||||
else:
|
||||
resolved = (base / suffix).resolve()
|
||||
if resolved.is_file():
|
||||
add_edge(file_nid, _make_id(str(resolved)),
|
||||
"imports_from", line,
|
||||
confidence="INFERRED", context="import",
|
||||
target_file=str(resolved))
|
||||
# Integration (#2141 + #2079): record
|
||||
# the resolved sourced file so calls
|
||||
# into its functions resolve too, not
|
||||
# just the source edge. target_path is
|
||||
# absolute here; resolve_bash_source_edges
|
||||
# takes it as-is.
|
||||
bash_sources.append({
|
||||
"target_path": str(resolved),
|
||||
"source_file": str_path,
|
||||
"source_location": f"L{line}",
|
||||
})
|
||||
else:
|
||||
# Bare `source lib.sh` (no leading ./ or /). Bash
|
||||
# itself resolves such a name via $PATH at runtime,
|
||||
|
||||
@@ -2725,6 +2725,98 @@ def test_extract_bash_var_source_untracked_var_keeps_script_dir_guess(tmp_path):
|
||||
assert (lib / "y.sh").resolve() in targets, targets
|
||||
|
||||
|
||||
def test_extract_bash_source_dirname_cmdsubst_in_argument(tmp_path):
|
||||
"""Form 3 (#2596): `source "$(dirname "$VAR")/lib/y.sh"` — command
|
||||
substitution in the source argument. The dirname idiom on the *source*
|
||||
line should resolve the same way it does on an assignment line: treat
|
||||
`$(dirname "$VAR")` as `var_bases[VAR].parent` (or `script_dir.parent`
|
||||
when VAR is untracked), then resolve the literal suffix against it.
|
||||
"""
|
||||
(tmp_path / "lib").mkdir()
|
||||
(tmp_path / "lib" / "y.sh").write_text(
|
||||
"#!/usr/bin/env bash\ny_fn() { :; }\n", encoding="utf-8"
|
||||
)
|
||||
script = tmp_path / "bin" / "x.sh"
|
||||
script.parent.mkdir(parents=True)
|
||||
# SCRIPT_DIR is tracked by the existing idiom, so dirname("$SCRIPT_DIR")
|
||||
# should resolve to tmp_path, and tmp_path/lib/y.sh is the target.
|
||||
script.write_text(
|
||||
'#!/usr/bin/env bash\n'
|
||||
'SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"\n'
|
||||
'source "$(dirname "$SCRIPT_DIR")/lib/y.sh"\n',
|
||||
encoding="utf-8",
|
||||
)
|
||||
result = extract_bash(script)
|
||||
targets = [Path(s["target_path"]).resolve() for s in result["bash_sources"]]
|
||||
assert (tmp_path / "lib" / "y.sh").resolve() in targets, targets
|
||||
|
||||
|
||||
def test_extract_bash_source_dirname_cmdsubst_untracked_var(tmp_path):
|
||||
"""Form 3 with an untracked variable: `source "$(dirname "$SCRIPT_DIR")/lib/y.sh"`
|
||||
where SCRIPT_DIR is NOT assigned via the recognised idiom. The fallback
|
||||
is the script's own directory (same as the existing untracked-var path),
|
||||
so dirname of the script dir is the parent.
|
||||
"""
|
||||
(tmp_path / "lib").mkdir()
|
||||
(tmp_path / "lib" / "y.sh").write_text(
|
||||
"#!/usr/bin/env bash\ny_fn() { :; }\n", encoding="utf-8"
|
||||
)
|
||||
script = tmp_path / "bin" / "x.sh"
|
||||
script.parent.mkdir(parents=True)
|
||||
# No SCRIPT_DIR assignment — the var is untracked, so the extractor
|
||||
# falls back to script_dir.parent (= tmp_path) for dirname.
|
||||
script.write_text(
|
||||
'#!/usr/bin/env bash\n'
|
||||
'source "$(dirname "$SCRIPT_DIR")/lib/y.sh"\n',
|
||||
encoding="utf-8",
|
||||
)
|
||||
result = extract_bash(script)
|
||||
targets = [Path(s["target_path"]).resolve() for s in result["bash_sources"]]
|
||||
assert (tmp_path / "lib" / "y.sh").resolve() in targets, targets
|
||||
|
||||
|
||||
def test_extract_bash_source_dotdot_suffix_with_tracked_var(tmp_path):
|
||||
"""Form 4 (#2596): `source "$VAR/../lib/y.sh"` — `..` in the literal
|
||||
suffix. When the base comes from a tracked var_bases entry, `..` is
|
||||
safe (it's a known directory, not a guess), so resolve via normpath
|
||||
and the existing is_file() gate. The `..` rejection should only apply
|
||||
on the script-dir-guess path where the base is uncertain.
|
||||
"""
|
||||
(tmp_path / "lib").mkdir()
|
||||
(tmp_path / "lib" / "y.sh").write_text(
|
||||
"#!/usr/bin/env bash\ny_fn() { :; }\n", encoding="utf-8"
|
||||
)
|
||||
script = tmp_path / "bin" / "x.sh"
|
||||
script.parent.mkdir(parents=True)
|
||||
# SCRIPT_DIR is tracked (= bin/), so $SCRIPT_DIR/../lib/y.sh resolves
|
||||
# to tmp_path/lib/y.sh.
|
||||
script.write_text(
|
||||
'#!/usr/bin/env bash\n'
|
||||
'SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"\n'
|
||||
'source "$SCRIPT_DIR/../lib/y.sh"\n',
|
||||
encoding="utf-8",
|
||||
)
|
||||
result = extract_bash(script)
|
||||
targets = [Path(s["target_path"]).resolve() for s in result["bash_sources"]]
|
||||
assert (tmp_path / "lib" / "y.sh").resolve() in targets, targets
|
||||
|
||||
|
||||
def test_extract_bash_source_dotdot_suffix_script_dir_guess_still_rejected(tmp_path):
|
||||
"""Form 4 guard: `..` in the suffix must still be rejected when the base
|
||||
is the script-dir *guess* (no tracked variable). Otherwise a path like
|
||||
`source "${UNTRACKED}/../../etc/passwd"` could traverse outside the tree.
|
||||
"""
|
||||
script = tmp_path / "run.sh"
|
||||
script.write_text(
|
||||
'#!/usr/bin/env bash\n'
|
||||
'source "${UNTRACKED}/../evil.sh"\n',
|
||||
encoding="utf-8",
|
||||
)
|
||||
result = extract_bash(script)
|
||||
targets = [Path(s["target_path"]).resolve() for s in result["bash_sources"]]
|
||||
assert not targets, f"untracked var with .. should not resolve: {targets}"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# JSON extractor tests (#866)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user