diff --git a/src/coretrace_python/plugins/secrets.py b/src/coretrace_python/plugins/secrets.py index 3559466..14f97af 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,13 @@ 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 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_NAME_MAPS = frozenset( + {"dependencies", "devDependencies", "peerDependencies", "optionalDependencies", "requires", "bin"} +) def config_literals(root: Path) -> Iterator[Literal]: @@ -241,7 +250,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_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)) else: @@ -262,7 +272,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, name_maps: frozenset[str] = frozenset() +) -> Iterator[Literal]: + """Every string value of ``data`` with its key, except that a value directly under + one of ``name_maps`` is bound to no name: its key names a package or a command.""" + lines = text.splitlines() def line_of(key: str) -> int: @@ -271,15 +286,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 name_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..4a9cba7 --- /dev/null +++ b/tests/test_lock_dependency_maps.py @@ -0,0 +1,86 @@ +"""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 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. +""" + +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"}, + }, + "node_modules/secretlint": {"version": "9.0.0", "bin": {"secretlint": "bin/secretlint.js"}}, + }, +} +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"