From becea721dcff5c36125723edd802d0ff15e48f0f Mon Sep 17 00:00:00 2001 From: Hugo Date: Sun, 4 Oct 2026 12:26:31 +0200 Subject: [PATCH 1/2] fix(secrets): read npm lock dependency maps as package names In package-lock.json and npm-shrinkwrap.json, the keys of dependency maps are package names and their values version ranges, so js-tokens: ^4.0.0 was reported as a hardcoded credential. Values directly under such a map are now bound to no name; provider patterns, including credentials in resolved URLs, still apply, and other JSON files are judged as before. The module documentation now describes the configuration scan. Refs #210 --- src/coretrace_python/plugins/secrets.py | 28 ++++++--- tests/test_lock_dependency_maps.py | 84 +++++++++++++++++++++++++ 2 files changed, 105 insertions(+), 7 deletions(-) create mode 100644 tests/test_lock_dependency_maps.py diff --git a/src/coretrace_python/plugins/secrets.py b/src/coretrace_python/plugins/secrets.py index 3559466..5bb6d41 100644 --- a/src/coretrace_python/plugins/secrets.py +++ b/src/coretrace_python/plugins/secrets.py @@ -6,7 +6,9 @@ provider pattern first (``hardcoded-secret``), then a credential-like name with a real value (``hardcoded-credential``), then a high-entropy token on its own (``high-entropy-string``). Messages and metadata carry a redacted preview, never the -secret. Only Python sources are scanned; configuration files are not. +secret. ``config_literals`` gives the string values of the project's configuration +files (``.env``, YAML, TOML, JSON, INI, properties) with the key each is bound to, and +the same detector judges them. """ from __future__ import annotations @@ -222,6 +224,12 @@ def matches(self, text: str) -> bool: CONFIG_SUFFIXES = frozenset({".env", ".yaml", ".yml", ".toml", ".json", ".ini", ".cfg", ".properties", ".conf"}) _PAIR = re.compile(r"^\s*(?:-\s+)?([A-Za-z_][\w.-]*)\s*[:=]\s*(.*?)\s*$") _MAX_CONFIG_BYTES = 8_000_000 +# In npm lock files, the keys of these maps are package names and their values version +# ranges: ``"js-tokens": "^4.0.0"`` binds no value to the name ``js-tokens``. +_NPM_LOCK_FILES = frozenset({"package-lock.json", "npm-shrinkwrap.json"}) +_NPM_DEPENDENCY_MAPS = frozenset( + {"dependencies", "devDependencies", "peerDependencies", "optionalDependencies", "requires"} +) def config_literals(root: Path) -> Iterator[Literal]: @@ -241,7 +249,8 @@ def config_literals(root: Path) -> Iterator[Literal]: continue source = SourceId(str(path)) if path.suffix == ".json": - yield from _structured(source, text, _load_json(text)) + maps = _NPM_DEPENDENCY_MAPS if path.name in _NPM_LOCK_FILES else frozenset() + yield from _structured(source, text, _load_json(text), maps) elif path.suffix == ".toml": yield from _structured(source, text, _load_toml(text)) else: @@ -262,7 +271,12 @@ def _load_toml(text: str) -> object: return None -def _structured(source: SourceId, text: str, data: object) -> Iterator[Literal]: +def _structured( + source: SourceId, text: str, data: object, package_maps: frozenset[str] = frozenset() +) -> Iterator[Literal]: + """Every string value of ``data`` with its key, except that a value directly under + one of ``package_maps`` is bound to no name: its key names a package.""" + lines = text.splitlines() def line_of(key: str) -> int: @@ -271,15 +285,15 @@ def line_of(key: str) -> int: return number return 1 - def walk(node: object, key: str | None) -> Iterator[Literal]: + def walk(node: object, key: str | None, named: bool = True) -> Iterator[Literal]: if isinstance(node, dict): for name, value in node.items(): - yield from walk(value, str(name)) + yield from walk(value, str(name), key not in package_maps) elif isinstance(node, list): for item in node: - yield from walk(item, key) + yield from walk(item, key, named) elif isinstance(node, str) and key is not None: - yield node, key, SourceSpan(source, line_of(key), 1), None + yield node, key if named else None, SourceSpan(source, line_of(key), 1), None yield from walk(data, None) diff --git a/tests/test_lock_dependency_maps.py b/tests/test_lock_dependency_maps.py new file mode 100644 index 0000000..a196030 --- /dev/null +++ b/tests/test_lock_dependency_maps.py @@ -0,0 +1,84 @@ +"""Acceptance tests for issue #210: dependency maps of npm lock files. + +In ``package-lock.json`` and ``npm-shrinkwrap.json``, the keys of a dependency map +(``dependencies``, ``devDependencies``, ``peerDependencies``, ``optionalDependencies``, +``requires``) are package names and its values version ranges: ``"js-tokens": "^4.0.0"`` +is not a credential named ``js-tokens``. The lock file is still scanned: a ``resolved`` +URL carrying a private-registry credential is a real leak, and a credential-named key +outside a dependency map is still a credential. Other JSON files are judged as before. +""" + +from __future__ import annotations + +import json +from pathlib import Path + +import pytest + +from coretrace_python import engine +from coretrace_python.findings import Finding + +PLUGINS = Path(__file__).resolve().parent.parent / "src" / "coretrace_python" / "bundled" +TOKEN = "Zx81kQpLw0RtY7vBn3MsD9cF2hJ6gK4a" + + +def scan(tmp_path: Path, name: str, document: object) -> list[Finding]: + (tmp_path / name).write_text(json.dumps(document, indent=2), encoding="utf-8") + findings = engine.analyze_project(tmp_path, [PLUGINS]).findings + return [f for f in findings if f.rule_id in ("hardcoded-credential", "hardcoded-secret")] + + +LOCK_V3 = { + "name": "web", + "lockfileVersion": 3, + "packages": { + "": {"dependencies": {"js-tokens": "^4.0.0"}, "devDependencies": {"tokenizer": "^1.0.0"}}, + "node_modules/loose-envify": { + "version": "1.4.0", + "dependencies": {"js-tokens": "^3.0.0 || ^4.0.0"}, + "peerDependencies": {"secret-handshake": ">=1"}, + "optionalDependencies": {"api-key-utils": "^2.0.0"}, + }, + }, +} +LOCK_V1 = { + "name": "web", + "lockfileVersion": 1, + "dependencies": {"loose-envify": {"version": "1.4.0", "requires": {"js-tokens": "^4.0.0"}}}, +} + + +@pytest.mark.parametrize("name", ["package-lock.json", "npm-shrinkwrap.json"]) +@pytest.mark.parametrize("document", [LOCK_V3, LOCK_V1], ids=["v3", "v1"]) +def test_package_names_in_dependency_maps_are_not_credentials( + tmp_path: Path, name: str, document: object +) -> None: + assert scan(tmp_path, name, document) == [] + + +def test_a_resolved_url_carrying_a_credential_is_still_reported(tmp_path: Path) -> None: + lock = { + "lockfileVersion": 3, + "packages": { + "node_modules/js-tokens": { + "version": "4.0.0", + "resolved": f"https://ci:{TOKEN}@npm.registry.example/js-tokens/-/js-tokens-4.0.0.tgz", + } + }, + } + + (finding,) = scan(tmp_path, "package-lock.json", lock) + + assert (finding.rule_id, finding.metadata["provider"]) == ("hardcoded-secret", "url") + + +def test_a_credential_name_outside_a_dependency_map_is_still_reported(tmp_path: Path) -> None: + (finding,) = scan(tmp_path, "package-lock.json", {"lockfileVersion": 3, "auth_token": TOKEN}) + + assert finding.rule_id == "hardcoded-credential" + + +def test_dependency_maps_of_other_json_files_are_judged_as_before(tmp_path: Path) -> None: + (finding,) = scan(tmp_path, "settings.json", {"dependencies": {"api_token": TOKEN}}) + + assert finding.rule_id == "hardcoded-credential" From 303a78567c792be35e9357aeabc2abe6b0d93360 Mon Sep 17 00:00:00 2001 From: Hugo Date: Sun, 4 Oct 2026 13:23:36 +0200 Subject: [PATCH 2/2] fix(secrets): read npm lock bin maps as command names Review of the dependency-map change: the bin map of a locked package maps command names to paths, so a command such as secretlint was still reported as a credential. Refs #210 --- src/coretrace_python/plugins/secrets.py | 17 +++++++++-------- tests/test_lock_dependency_maps.py | 4 +++- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/src/coretrace_python/plugins/secrets.py b/src/coretrace_python/plugins/secrets.py index 5bb6d41..14f97af 100644 --- a/src/coretrace_python/plugins/secrets.py +++ b/src/coretrace_python/plugins/secrets.py @@ -224,11 +224,12 @@ def matches(self, text: str) -> bool: CONFIG_SUFFIXES = frozenset({".env", ".yaml", ".yml", ".toml", ".json", ".ini", ".cfg", ".properties", ".conf"}) _PAIR = re.compile(r"^\s*(?:-\s+)?([A-Za-z_][\w.-]*)\s*[:=]\s*(.*?)\s*$") _MAX_CONFIG_BYTES = 8_000_000 -# In npm lock files, the keys of these maps are package names and their values version -# ranges: ``"js-tokens": "^4.0.0"`` binds no value to the name ``js-tokens``. +# In npm lock files, the keys of these maps are package or command names, and their +# values version ranges or paths: ``"js-tokens": "^4.0.0"`` binds no value to the name +# ``js-tokens``, nor ``"secretlint": "bin/secretlint.js"`` to ``secretlint``. _NPM_LOCK_FILES = frozenset({"package-lock.json", "npm-shrinkwrap.json"}) -_NPM_DEPENDENCY_MAPS = frozenset( - {"dependencies", "devDependencies", "peerDependencies", "optionalDependencies", "requires"} +_NPM_NAME_MAPS = frozenset( + {"dependencies", "devDependencies", "peerDependencies", "optionalDependencies", "requires", "bin"} ) @@ -249,7 +250,7 @@ def config_literals(root: Path) -> Iterator[Literal]: continue source = SourceId(str(path)) if path.suffix == ".json": - maps = _NPM_DEPENDENCY_MAPS if path.name in _NPM_LOCK_FILES else frozenset() + maps = _NPM_NAME_MAPS if path.name in _NPM_LOCK_FILES else frozenset() yield from _structured(source, text, _load_json(text), maps) elif path.suffix == ".toml": yield from _structured(source, text, _load_toml(text)) @@ -272,10 +273,10 @@ def _load_toml(text: str) -> object: def _structured( - source: SourceId, text: str, data: object, package_maps: frozenset[str] = frozenset() + source: SourceId, text: str, data: object, name_maps: frozenset[str] = frozenset() ) -> Iterator[Literal]: """Every string value of ``data`` with its key, except that a value directly under - one of ``package_maps`` is bound to no name: its key names a package.""" + one of ``name_maps`` is bound to no name: its key names a package or a command.""" lines = text.splitlines() @@ -288,7 +289,7 @@ def line_of(key: str) -> int: def walk(node: object, key: str | None, named: bool = True) -> Iterator[Literal]: if isinstance(node, dict): for name, value in node.items(): - yield from walk(value, str(name), key not in package_maps) + yield from walk(value, str(name), key not in name_maps) elif isinstance(node, list): for item in node: yield from walk(item, key, named) diff --git a/tests/test_lock_dependency_maps.py b/tests/test_lock_dependency_maps.py index a196030..4a9cba7 100644 --- a/tests/test_lock_dependency_maps.py +++ b/tests/test_lock_dependency_maps.py @@ -3,7 +3,8 @@ In ``package-lock.json`` and ``npm-shrinkwrap.json``, the keys of a dependency map (``dependencies``, ``devDependencies``, ``peerDependencies``, ``optionalDependencies``, ``requires``) are package names and its values version ranges: ``"js-tokens": "^4.0.0"`` -is not a credential named ``js-tokens``. The lock file is still scanned: a ``resolved`` +is not a credential named ``js-tokens``. The keys of a ``bin`` map are command names and +its values paths: ``"secretlint": "bin/secretlint.js"`` is not a secret either. The lock file is still scanned: a ``resolved`` URL carrying a private-registry credential is a real leak, and a credential-named key outside a dependency map is still a credential. Other JSON files are judged as before. """ @@ -39,6 +40,7 @@ def scan(tmp_path: Path, name: str, document: object) -> list[Finding]: "peerDependencies": {"secret-handshake": ">=1"}, "optionalDependencies": {"api-key-utils": "^2.0.0"}, }, + "node_modules/secretlint": {"version": "9.0.0", "bin": {"secretlint": "bin/secretlint.js"}}, }, } LOCK_V1 = {