Skip to content

Commit 07c7f9b

Browse files
Package scanners: close fail-open gaps in the sdist fallback and hidden-payload paths (#6359)
* Package scanners: close fail-open gaps in the sdist fallback and hidden-payload paths Follow-up hardening on the now-blocking scanners so the enforcing gate cannot report clean while a malicious artifact goes unscanned. scan_packages.py - Hidden payload: also flag a network call AND an os/subprocess exec that live only in a blanked docstring/string of an exec/eval file (the fetch-then-run shape of an exec(__doc__) dropper). Either alone in real code was already covered; hidden together they are the payload. - Pinned releases fail closed: _release_files no longer falls back to the latest artifact when a pinned version is missing or empty, so a yanked/bad pin is an error instead of a different file being scanned in its place. - requires_dist is read from the pinned release's metadata, not the project-level (latest) document, so a sdist-only pin follows its own dependency tree. - Environment markers are evaluated (PEP 508) instead of dropping any marker that merely contains the word extra, so default-true markers like extra != 'dev' are kept; conservative fallback keeps a dep on any uncertainty. - Transitive recovery is a depth-bounded worklist: a wheel dependency whose own child is sdist-only is fetched (--no-deps) and scanned, then its children are recovered in turn, rather than being silently skipped. scan_npm_packages.py - Baseline keys use the package-relative path instead of the basename, so the same basename in a different directory is not over-suppressed. Tests cover each case; full scripts pass AST and ruff checks. * Address review: tighten marker scope, decoy-proof the dropper check, fail closed on missing pin metadata - Markers: keep any dep whose marker can hold on another install target (sys_platform == 'win32', python_version == '3.13'); only drop a marker that depends solely on extra and is false with no extra. A scanner runs on one target but must cover code installed on others. Pure-extra markers are evaluated against default_environment() with extra unset. - Hidden dropper: the network+exec docstring check now inspects the removed (blanked) span directly, so a benign visible network or subprocess call cannot mask a payload that still lives in a docstring. Carrier checks stay blanked-only (an in-code carrier is already caught by the normal check), so corpus findings are unchanged. - requires_dist: a pinned version whose own metadata cannot be fetched recovers nothing rather than substituting the latest release's dependency tree. - Transitive recovery: the last-ditch direct-sdist branch also chases the recovered package's declared deps, matching the other branches. - npm baseline: schema bumped to v2 (package-relative keys); a pre-v2 baseline with entries is ignored (fail closed) instead of mis-applying basename keys. Tests cover each case; scripts pass AST, ruff, and the import-hoist verifier. * Scanner: exclude comments from hidden-payload check, flag missing pin metadata as incomplete Hidden network+exec detection now inspects only docstring/string spans (what exec(__doc__)/exec(<str>) can actually run), so a real exec() beside comments that mention a network and a subprocess call no longer false-positives. Missing pinned-release metadata in transitive recovery records a download_error so the --with-deps path fails closed instead of treating it as no dependencies. Adds regression tests for both. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
1 parent 48a3a78 commit 07c7f9b

5 files changed

Lines changed: 380 additions & 55 deletions

File tree

scripts/scan_npm_packages.py

Lines changed: 35 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1445,14 +1445,20 @@ def scan_one(pkg: PackageEntry, workspace: Path) -> tuple[list[Finding], str | N
14451445
# ─────────────────────────────────────────────────────────────────────
14461446
# Baseline allowlist: triaged known-good HIGH/CRITICAL findings so the gate
14471447
# can enforce without red-failing on rare legitimate-library behavior.
1448-
# Matched on ``(normalized package, basename(filename), pattern)`` -- not
1448+
# Matched on ``(normalized package, package-relative path, pattern)`` -- not
14491449
# evidence text -- so a version bump does not reopen a finding, but a *new*
14501450
# kind of finding in a listed file is a different pattern and still fails.
14511451
# Mirrors scan_packages.py. Regenerate with ``--write-baseline``.
14521452
# ─────────────────────────────────────────────────────────────────────
14531453

14541454
_DEFAULT_BASELINE_PATH = str(Path(__file__).resolve().parent / "scan_npm_packages_baseline.json")
14551455

1456+
# Bumped when the entry-key semantics change. v2 keys on the package-relative
1457+
# path; v1 stored only a basename, so a v1 entry could suppress a same-named file
1458+
# in a different directory. A pre-v2 baseline with entries is ignored (fail
1459+
# closed) rather than mis-applied.
1460+
_BASELINE_SCHEMA_VERSION = 2
1461+
14561462

14571463
def _norm_pkg_name(display: str) -> str:
14581464
"""``@scope/pkg@1.2.3`` / ``pkg@1.2.3`` -> name without the version.
@@ -1468,9 +1474,21 @@ def _norm_pkg_name(display: str) -> str:
14681474
return s.lower()
14691475

14701476

1477+
_NPM_TARBALL_ROOT = "package/"
1478+
1479+
1480+
def _relpath_in_package(filename: str) -> str:
1481+
"""Path within the published package, stable across version bumps. npm
1482+
tarballs root every file at ``package/``; strip it so the key is the real
1483+
source path (``dist/index.js``) and a new file with the same basename in a
1484+
different directory is not silently suppressed."""
1485+
f = (filename or "").replace("\\", "/")
1486+
return f[len(_NPM_TARBALL_ROOT) :] if f.startswith(_NPM_TARBALL_ROOT) else f
1487+
1488+
14711489
def _finding_key(f: Finding) -> tuple[str, str, str]:
1472-
"""Stable allowlist key: normalized package, file basename, pattern."""
1473-
return (_norm_pkg_name(f.package), os.path.basename(f.filename), f.pattern)
1490+
"""Stable allowlist key: normalized package, package-relative path, pattern."""
1491+
return (_norm_pkg_name(f.package), _relpath_in_package(f.filename), f.pattern)
14741492

14751493

14761494
def _load_baseline(path: str) -> set[tuple[str, str, str]]:
@@ -1483,10 +1501,18 @@ def _load_baseline(path: str) -> set[tuple[str, str, str]]:
14831501
except (OSError, json.JSONDecodeError) as exc:
14841502
print(f" [WARN] could not read baseline {path}: {exc}", file = sys.stderr)
14851503
return set()
1504+
entries = data.get("entries", [])
1505+
if entries and data.get("version") != _BASELINE_SCHEMA_VERSION:
1506+
print(
1507+
f" [WARN] baseline schema v{data.get('version')} predates package-relative "
1508+
f"keys; ignoring {len(entries)} entr(y/ies). Regenerate with --write-baseline.",
1509+
file = sys.stderr,
1510+
)
1511+
return set()
14861512
keys: set[tuple[str, str, str]] = set()
1487-
for e in data.get("entries", []):
1513+
for e in entries:
14881514
try:
1489-
keys.add((_norm_pkg_name(e["package"]), os.path.basename(e["file"]), e["pattern"]))
1515+
keys.add((_norm_pkg_name(e["package"]), _relpath_in_package(e["file"]), e["pattern"]))
14901516
except (KeyError, TypeError):
14911517
continue
14921518
return keys
@@ -1506,7 +1532,7 @@ def _write_baseline(path: str, findings: list[Finding], threshold_rank: int) ->
15061532
entries.append(
15071533
{
15081534
"package": _norm_pkg_name(f.package),
1509-
"file": os.path.basename(f.filename),
1535+
"file": _relpath_in_package(f.filename),
15101536
"pattern": f.pattern,
15111537
"severity": f.severity,
15121538
"evidence": (f.evidence or f.detail)[:240],
@@ -1516,10 +1542,10 @@ def _write_baseline(path: str, findings: list[Finding], threshold_rank: int) ->
15161542
"_comment": (
15171543
"scan_npm_packages.py allowlist. Each entry is a HIGH/CRITICAL "
15181544
"finding manually judged benign. Matched on (package, "
1519-
"basename(file), pattern); evidence/severity are for review only. "
1520-
"Regenerate with --write-baseline AFTER reviewing every line."
1545+
"package-relative path, pattern); evidence/severity are for review "
1546+
"only. Regenerate with --write-baseline AFTER reviewing every line."
15211547
),
1522-
"version": 1,
1548+
"version": _BASELINE_SCHEMA_VERSION,
15231549
"entries": entries,
15241550
}
15251551
with open(path, "w", encoding = "utf-8") as fh:
Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
{
2-
"_comment": "scan_npm_packages.py allowlist. Each entry is a HIGH/CRITICAL finding manually judged benign. Matched on (package, basename(file), pattern); evidence/severity are for review only. Regenerate with --write-baseline AFTER reviewing every line. EMPTY by design: a full scan of studio/frontend/package-lock.json (915 packages) produced 0 findings, so nothing needs suppressing and the CI gate can run enforcing (SCAN_ENFORCE=1) as-is. If a future dependency adds a reviewed-benign HIGH/CRITICAL, add it here rather than weakening a pattern.",
3-
"version": 1,
2+
"_comment": "scan_npm_packages.py allowlist. Each entry is a HIGH/CRITICAL finding manually judged benign. Matched on (package, package-relative path, pattern); evidence/severity are for review only. Regenerate with --write-baseline AFTER reviewing every line. EMPTY by design: a full scan of studio/frontend/package-lock.json (915 packages) produced 0 findings, so nothing needs suppressing and the CI gate can run enforcing (SCAN_ENFORCE=1) as-is. If a future dependency adds a reviewed-benign HIGH/CRITICAL, add it here rather than weakening a pattern.",
3+
"version": 2,
44
"entries": []
55
}

0 commit comments

Comments
 (0)