mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-10-26 13:16:24 +00:00
fix(skillscan): report secrets bound by unpacking and container or conditional literals (#6247)
* fix(skillscan): report secrets bound by unpacking and container literals The AST walk behind secret-env-assignment claims "precision, never less coverage", but it only read a single assignment target and a bare string value, so a credential moved into `host, api_key = endpoint, "..."` or into `api_key = ["..."]` stopped being reported even though the pre-#5648 line sweep still reported the same bytes. Pair unpacking targets with values positionally (fixed head and tail only, so a runtime-sized slice names nothing) and assert on each literal a container or conditional holds, in source order. * fix(skillscan): distinguish mapping labels from credentials --------- Co-authored-by: sxh313 <sxh313@users.noreply.github.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
This commit is contained in:
parent
d9d9ff9708
commit
130a9ab9f0
@ -1279,6 +1279,8 @@ If a trusted operator manages the configured skills directory through an externa
|
||||
|
||||
Skill installs and agent-managed skill edits run through **SkillScan**, a native deterministic safety scanner before the LLM-based skill scanner. Phase 1 runs offline with no Semgrep/OpenGrep dependency, blocks high-confidence `CRITICAL` findings such as private keys or shell execution, and passes warning findings to the LLM scanner for contextual review. Code files (anything under `scripts/`, a script suffix such as `.py`, `.sh`, or `.js`, or an extensionless file starting with `#!`) that are not NUL-free UTF-8 text raise a warning and are still analyzed over a lossy decode, so a single stray byte cannot hide them from `CRITICAL` checks. The moderation adapter normalizes both plain-text model responses and LangChain Responses API text blocks before parsing the required JSON decision. Python instance-client exfiltration checks follow a minimal same-scope evidence chain: a simple name bound to a known client constructor, optional name-to-name aliases, and an actual outbound method or context-manager use supported by that constructor. Constructor roots must be proven imports; bare canonical-looking names are not inferred as modules. Nested scopes do not inherit client handles and inherit only constructor import aliases that are never rebound in the enclosing scope. Comprehensions, walrus-bearing statements, annotations, complex binding targets, unsupported operations, and ambiguous branch flows produce no finding from this signal; skipped constructs conservatively invalidate every name they may bind so stale client state cannot create a finding. A deterministic work budget or recursion limit reached by this best-effort analysis does not discard findings already collected for the file. Set `skill_scan.enabled: false` in `config.yaml` to disable only the deterministic analyzers; safe archive extraction and the LLM scanner still run.
|
||||
|
||||
For Python credential mappings, SkillScan checks literal values while treating ordinary dictionary keys as labels. A mapping such as `tokens = {"access_token": os.getenv("ACCESS_TOKEN")}` does not report a hardcoded credential. Keys matching a recognized cloud or API token format are still checked as embedded credentials.
|
||||
|
||||
DeerFlow also ships with **skill-reviewer**, a public skill for read-only skill quality review. It uses the built-in `review_skill_package` tool to inspect installed skills, local packages, archives, or pasted `SKILL.md` content without activating the target skill, binding its secrets, executing its scripts, or installing it. The tool returns a compact, tag-neutralized JSON payload to the model context and keeps the full raw review payload in the tool artifact for programmatic consumers. The deterministic review core reuses DeerFlow parsing and SkillScan facts, emits versioned JSON contracts under `contracts/skill_review/`, and can be run from the backend CLI:
|
||||
|
||||
```bash
|
||||
|
||||
File diff suppressed because one or more lines are too long
@ -118,6 +118,15 @@ _PLACEHOLDER_VALUES = {"", "x", "xx", "xxx", "xxxx", "changeme", "change-me", "e
|
||||
# analyzed from its AST instead, because a regex cannot tell an annotation from a value.
|
||||
_SECRET_ASSIGNMENT_RE = re.compile(r"(?im)\b(token|password|passwd|api[_-]?key|secret|credential)s?\b\s*[:=]\s*[\"']?([^\"'\s#]+)")
|
||||
_SECRET_ASSIGNMENT_NAME_RE = re.compile(r"(?i)^(?:token|password|passwd|api[_-]?key|secret|credential)s?$")
|
||||
_SECRET_TOKEN_PATTERNS = tuple(
|
||||
re.compile(pattern)
|
||||
for pattern in (
|
||||
r"\b(?:AKIA|ASIA)[A-Z0-9]{16}\b",
|
||||
r"\bgh[pousr]_[A-Za-z0-9_]{20,}\b",
|
||||
r"\bxox[baprs]-[A-Za-z0-9-]{20,}\b",
|
||||
r"\bsk-[A-Za-z0-9]{20,}\b",
|
||||
)
|
||||
)
|
||||
_SENSITIVE_PATH_RE = re.compile(r"(~/.ssh|/etc/passwd|/etc/shadow|/var/run/docker\.sock|docker\.sock|169\.254\.169\.254)")
|
||||
_EXTERNAL_HTTP_RE = re.compile(r"http://([A-Za-z0-9.-]+)(?::\d+)?(?:/|\b)")
|
||||
_URL_RE = re.compile(r"https?://[^\s)'\"<>]+")
|
||||
@ -315,14 +324,8 @@ def _scan_secrets(rel_path: str, text: str) -> list[SecurityFinding]:
|
||||
if private_key:
|
||||
findings.append(_finding_from_match("secret-private-key", rel_path, text, private_key))
|
||||
|
||||
token_patterns = [
|
||||
r"\b(?:AKIA|ASIA)[A-Z0-9]{16}\b",
|
||||
r"\bgh[pousr]_[A-Za-z0-9_]{20,}\b",
|
||||
r"\bxox[baprs]-[A-Za-z0-9-]{20,}\b",
|
||||
r"\bsk-[A-Za-z0-9]{20,}\b",
|
||||
]
|
||||
for pattern in token_patterns:
|
||||
match = re.search(pattern, text)
|
||||
for pattern in _SECRET_TOKEN_PATTERNS:
|
||||
match = pattern.search(text)
|
||||
if match and not _looks_like_placeholder(match.group(0)):
|
||||
findings.append(_finding_from_match("secret-cloud-token", rel_path, text, match))
|
||||
break
|
||||
@ -355,18 +358,71 @@ def _python_secret_assignment_target(node: ast.expr) -> str | None:
|
||||
return None
|
||||
|
||||
|
||||
def _python_secret_unpacked_span(elts: list[ast.expr]) -> tuple[int, int, bool]:
|
||||
"""Fixed head, fixed tail and star presence of one side of an unpacking.
|
||||
|
||||
Python aligns everything before the first ``*`` and everything after the last one; only
|
||||
the elements between stars share a runtime-determined slice. A side without a star has
|
||||
its whole length fixed, so its head is every element and its tail is none.
|
||||
"""
|
||||
stars = [index for index, element in enumerate(elts) if isinstance(element, ast.Starred)]
|
||||
if not stars:
|
||||
return len(elts), 0, False
|
||||
return stars[0], len(elts) - stars[-1] - 1, True
|
||||
|
||||
|
||||
def _python_secret_unpacked_pairs(target: ast.expr, value: ast.expr) -> list[tuple[ast.expr, ast.expr]] | None:
|
||||
"""Target-and-value pairs of an unpacking assignment, or None when it is not one.
|
||||
|
||||
``host, api_key = endpoint, "…"`` binds each name to the value written in the same
|
||||
position, so the two sides have to be read together. Pairs come only from the fixed
|
||||
head and tail of the two element lists (``_python_secret_unpacked_span``): what falls
|
||||
between two stars is a runtime slice. An unpacking with no star on either side binds
|
||||
only when the lengths match, so a mismatch -- which raises before binding anything --
|
||||
contributes none. Nested targets align positionally too, so they are read the same way.
|
||||
"""
|
||||
if not isinstance(target, (ast.Tuple, ast.List)):
|
||||
return None
|
||||
if not isinstance(value, (ast.Tuple, ast.List)):
|
||||
return []
|
||||
target_head, target_tail, target_starred = _python_secret_unpacked_span(target.elts)
|
||||
value_head, value_tail, value_starred = _python_secret_unpacked_span(value.elts)
|
||||
if not target_starred and not value_starred:
|
||||
if len(target.elts) != len(value.elts):
|
||||
return []
|
||||
head, tail = len(target.elts), 0
|
||||
else:
|
||||
# A star-free side is fully fixed, so its tail is whatever its head did not consume.
|
||||
head = min(target_head, value_head)
|
||||
tail = min(len(target.elts) - head if not target_starred else target_tail, len(value.elts) - head if not value_starred else value_tail)
|
||||
pairs = list(zip(target.elts[:head], value.elts[:head], strict=True))
|
||||
if tail:
|
||||
pairs += list(zip(target.elts[len(target.elts) - tail :], value.elts[len(value.elts) - tail :], strict=True))
|
||||
expanded: list[tuple[ast.expr, ast.expr]] = []
|
||||
for paired_target, paired_value in pairs:
|
||||
nested = _python_secret_unpacked_pairs(paired_target, paired_value)
|
||||
expanded.extend(nested or [(paired_target, paired_value)])
|
||||
return expanded
|
||||
|
||||
|
||||
def _python_secret_bindings(tree: ast.AST) -> list[tuple[str | None, ast.expr]]:
|
||||
"""Every ``(bound name, value expression)`` pair the tree binds, in walk order.
|
||||
|
||||
Assignment statements are not the only place a skill can park a credential:
|
||||
a keyword argument, a parameter default and a walrus all read as
|
||||
``name=value`` to the line-oriented sweep this rule replaced, so a caller
|
||||
that merely moves the assignment into a call escapes the gate.
|
||||
that merely moves the assignment into a call escapes the gate. An unpacking
|
||||
assignment reads the same way once its targets are paired with values.
|
||||
"""
|
||||
bindings: list[tuple[str | None, ast.expr]] = []
|
||||
for node in ast.walk(tree):
|
||||
if isinstance(node, ast.Assign):
|
||||
bindings.extend((_python_secret_assignment_target(target), node.value) for target in node.targets)
|
||||
for target in node.targets:
|
||||
pairs = _python_secret_unpacked_pairs(target, node.value)
|
||||
if pairs is None:
|
||||
bindings.append((_python_secret_assignment_target(target), node.value))
|
||||
continue
|
||||
bindings.extend((_python_secret_assignment_target(paired_target), paired_value) for paired_target, paired_value in pairs)
|
||||
elif isinstance(node, (ast.AnnAssign, ast.NamedExpr)):
|
||||
# A bare annotation binds no value at all, so ``AnnAssign.value`` is None.
|
||||
bindings.append((_python_secret_assignment_target(node.target), node.value))
|
||||
@ -441,6 +497,57 @@ def _python_secret_literal_atom(node: ast.expr) -> str | None:
|
||||
return None
|
||||
|
||||
|
||||
def _python_secret_literal_members(expr: ast.expr) -> list[ast.expr]:
|
||||
"""Values held inside a container or a conditional, in the order they are written.
|
||||
|
||||
``api_key = ["…"]``, ``api_key = {"live": "…"}`` and ``api_key = "…" if prod else
|
||||
"…"`` are each a finding for the line-oriented sweep this rule replaced: its value
|
||||
capture stops at the first quote after the name, so it reports the line even when the
|
||||
token it grabbed is only a bracket. This names the literal instead of the bracket.
|
||||
Each member is asserted on by itself, because joining a container's members would
|
||||
describe a string that no Python program ever binds.
|
||||
|
||||
Mapping keys are structural labels, so they need independent evidence from a
|
||||
recognized token format; the enclosing secret-like name is evidence only for values.
|
||||
"""
|
||||
if isinstance(expr, (ast.List, ast.Tuple, ast.Set)):
|
||||
return list(expr.elts)
|
||||
if isinstance(expr, ast.Dict):
|
||||
# A ``{**spread}`` entry carries a None key and names nothing of its own.
|
||||
members: list[ast.expr] = []
|
||||
for key, value in zip(expr.keys, expr.values, strict=True):
|
||||
if key is not None:
|
||||
literal = _python_secret_literal(key)
|
||||
if literal is not None and any(pattern.search(literal) for pattern in _SECRET_TOKEN_PATTERNS):
|
||||
members.append(key)
|
||||
members.append(value)
|
||||
return members
|
||||
if isinstance(expr, ast.IfExp):
|
||||
return [expr.body, expr.orelse]
|
||||
return []
|
||||
|
||||
|
||||
def _python_secret_literal_candidates(expr: ast.expr) -> list[tuple[ast.expr, str]]:
|
||||
"""Every ``(node, literal)`` pair this binding can assert on, in source order.
|
||||
|
||||
A concatenation yields the whole value, because ``"sk-" + "a1b2"`` binds one
|
||||
string; a container or a conditional yields its members. A work list rather than
|
||||
recursion, matching ``_python_secret_literal_parts``: an expression tree holds each
|
||||
node once, so a node queued from its parent is never queued again.
|
||||
"""
|
||||
candidates: list[tuple[ast.expr, str]] = []
|
||||
pending: list[ast.expr] = [expr]
|
||||
while pending:
|
||||
node = pending.pop()
|
||||
literal = _python_secret_literal(node)
|
||||
if literal is not None:
|
||||
candidates.append((node, literal))
|
||||
continue
|
||||
pending.extend(_python_secret_literal_members(node))
|
||||
candidates.sort(key=lambda candidate: (candidate[0].lineno, candidate[0].col_offset))
|
||||
return candidates
|
||||
|
||||
|
||||
def _scan_python_secret_assignments(rel_path: str, text: str) -> list[SecurityFinding]:
|
||||
"""Report embedded Python secrets from real literal bindings, not from raw text.
|
||||
|
||||
@ -449,13 +556,18 @@ def _scan_python_secret_assignments(rel_path: str, text: str) -> list[SecurityFi
|
||||
(``api_key = os.getenv("X")``) from a literal, and it points at an annotated
|
||||
assignment's annotation rather than at its value.
|
||||
|
||||
What it gains is precision, never less coverage: every binding form the
|
||||
sweep reported stays reported, per ``_python_secret_bindings``, and so does
|
||||
every literal value shape it saw, per ``_python_secret_literal``.
|
||||
``_python_secret_bindings`` preserves the sweep's binding forms, and
|
||||
``_python_secret_literal_candidates`` covers literal values inside containers
|
||||
and conditionals. Mapping labels do not inherit credential evidence from the
|
||||
enclosing name; only keys matching a recognized token format are candidates.
|
||||
|
||||
A file Python cannot parse falls back to that sweep: the AST is only an
|
||||
improvement, and returning nothing would let one syntax error (or a NUL byte)
|
||||
silence a HIGH-severity rule for the whole file.
|
||||
silence a HIGH-severity rule for the whole file. That sweep reads the run of
|
||||
characters that follows ``name[:=]``, so it never reaches a credential an earlier
|
||||
element of the same line precedes (``host, api_key = "https://…", "…"``) or a bracket
|
||||
separates from its name (``[token, version] = ["…", 2]``): those two shapes are
|
||||
reported while the file parses and go quiet once it carries a syntax error.
|
||||
"""
|
||||
try:
|
||||
tree = ast.parse(text)
|
||||
@ -463,11 +575,12 @@ def _scan_python_secret_assignments(rel_path: str, text: str) -> list[SecurityFi
|
||||
return _scan_secret_assignments_by_text(rel_path, text)
|
||||
|
||||
for name, value in _python_secret_bindings(tree):
|
||||
literal = _python_secret_literal(value)
|
||||
if literal is None or _looks_like_placeholder(literal):
|
||||
if not _SECRET_ASSIGNMENT_NAME_RE.match(name or ""):
|
||||
continue
|
||||
if _SECRET_ASSIGNMENT_NAME_RE.match(name or ""):
|
||||
return [_finding_for_node("secret-env-assignment", rel_path, value, literal)]
|
||||
for node, literal in _python_secret_literal_candidates(value):
|
||||
if _looks_like_placeholder(literal):
|
||||
continue
|
||||
return [_finding_for_node("secret-env-assignment", rel_path, node, literal)]
|
||||
return []
|
||||
|
||||
|
||||
|
||||
@ -451,6 +451,20 @@ def test_skillscan_high_findings_are_review_errors(tmp_path):
|
||||
assert finding["skillscan_severity"] == "HIGH"
|
||||
|
||||
|
||||
@pytest.mark.parametrize("hardcoded", [False, True])
|
||||
def test_cli_mapping_credentials_gate_on_values(tmp_path, capsys, hardcoded):
|
||||
_write(tmp_path / "SKILL.md", _valid_skill() + "\nRead [loader](scripts/load.py).\n")
|
||||
value = '"9f8e7d6c5b4a3210ff"' if hardcoded else 'os.getenv("ACCESS_TOKEN")'
|
||||
_write(tmp_path / "scripts" / "load.py", f'import os\ntokens = {{"access_token": {value}, "refresh_token": os.getenv("REFRESH_TOKEN")}}\n')
|
||||
|
||||
exit_code = review_cli_main([str(tmp_path), "--format", "json", "--fail-on", "error", "--fail-on-incomplete"])
|
||||
report = json.loads(capsys.readouterr().out)
|
||||
|
||||
assert exit_code == (1 if hardcoded else 0)
|
||||
assert any(finding["rule_id"] == "secret-env-assignment" for finding in report["findings"]) is hardcoded
|
||||
assert "9f8e7d6c5b4a3210ff" not in repr(report)
|
||||
|
||||
|
||||
def test_skillscan_ignores_eval_fixture_skill_markdown(tmp_path):
|
||||
_write(tmp_path / "SKILL.md", _valid_skill())
|
||||
_write(
|
||||
|
||||
@ -1528,6 +1528,206 @@ def test_secret_assignment_still_flags_python_walrus_binding(tmp_path: Path) ->
|
||||
assert _finding_by_rule(_scan_python_sample(tmp_path, source), "secret-env-assignment")["line"] == 1
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"source",
|
||||
[
|
||||
'host, api_key = "internal", "9f8e7d6c5b4a3210ff"\n',
|
||||
'host, api_key = "https://internal.example", "9f8e7d6c5b4a3210ff"\n',
|
||||
],
|
||||
)
|
||||
def test_secret_assignment_still_flags_python_tuple_unpacking(tmp_path: Path, source: str) -> None:
|
||||
"""``host, api_key = "internal", "…"`` binds the literal to ``api_key``.
|
||||
|
||||
The sweep reported that form (it matched the line and took the first quoted value),
|
||||
so unpacking was a way to walk out of a HIGH-severity gate; ``_python_secret_assignment_target``
|
||||
only understood a single target node. The second case goes further: there the sweep
|
||||
took the URL as the value, read it as a placeholder and stayed silent too, so neither
|
||||
implementation saw it.
|
||||
"""
|
||||
finding = _finding_by_rule(_scan_python_sample(tmp_path, source), "secret-env-assignment")
|
||||
|
||||
assert finding["line"] == 1
|
||||
assert finding["evidence"] == "[redacted]"
|
||||
assert "9f8e7d6c5b4a3210ff" not in repr(finding)
|
||||
|
||||
|
||||
def test_secret_assignment_still_flags_python_list_target_unpacking(tmp_path: Path) -> None:
|
||||
"""A list target unpacks the same way a tuple target does; the brackets are syntax."""
|
||||
source = '[token, version] = ["9f8e7d6c5b4a3210ff", 2]\n'
|
||||
|
||||
assert _finding_by_rule(_scan_python_sample(tmp_path, source), "secret-env-assignment")["line"] == 1
|
||||
|
||||
|
||||
def test_secret_assignment_ignores_python_unpacking_of_environment_lookups(tmp_path: Path) -> None:
|
||||
"""The precision gain has to survive the unpacked form too.
|
||||
|
||||
``api_key, url = os.getenv("K"), endpoint`` binds both values at runtime, so neither
|
||||
is a literal this rule can assert on.
|
||||
"""
|
||||
source = 'api_key, url = os.getenv("DEERFLOW_TOKEN"), "https://api.example"\n'
|
||||
|
||||
assert _secret_assignments(_scan_python_sample(tmp_path, source)) == []
|
||||
|
||||
|
||||
def test_secret_assignment_still_flags_python_conditional_literal(tmp_path: Path) -> None:
|
||||
"""``api_key = "…" if prod else "x"`` ships the literal in the file."""
|
||||
source = 'api_key = "9f8e7d6c5b4a3210ff" if prod else "x"\n'
|
||||
|
||||
finding = _finding_by_rule(_scan_python_sample(tmp_path, source), "secret-env-assignment")
|
||||
|
||||
assert finding["line"] == 1
|
||||
assert "9f8e7d6c5b4a3210ff" not in repr(finding)
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"source",
|
||||
[
|
||||
'api_key = ["9f8e7d6c5b4a3210ff"]\n',
|
||||
'api_key = ("https://internal.example", "9f8e7d6c5b4a3210ff")\n',
|
||||
'api_key = {"live": "9f8e7d6c5b4a3210ff"}\n',
|
||||
],
|
||||
)
|
||||
def test_secret_assignment_still_flags_python_container_literal(tmp_path: Path, source: str) -> None:
|
||||
"""A credential inside a list, tuple or dict literal is still in the package."""
|
||||
finding = _finding_by_rule(_scan_python_sample(tmp_path, source), "secret-env-assignment")
|
||||
|
||||
assert finding["line"] == 1
|
||||
assert "9f8e7d6c5b4a3210ff" not in repr(finding)
|
||||
|
||||
|
||||
def test_secret_assignment_ignores_python_container_without_literal(tmp_path: Path) -> None:
|
||||
"""Only values Python resolves from source are asserted on: a container of runtime
|
||||
data (a call, a variable) binds nothing this rule can name."""
|
||||
source = 'api_key = [os.getenv("DEERFLOW_TOKEN"), token_from_config]\n'
|
||||
|
||||
assert _secret_assignments(_scan_python_sample(tmp_path, source)) == []
|
||||
|
||||
|
||||
@pytest.mark.parametrize("key", ['"ghp_a1b2c3d4e5f6g7h8i9j0"', '"ghp_" + "a1b2c3d4e5f6g7h8i9j0"'])
|
||||
def test_secret_assignment_still_flags_python_dict_key_literal(tmp_path: Path, key: str) -> None:
|
||||
"""A mapping key needs credential evidence of its own: a known token format,
|
||||
including one spelled as a concatenation of literals.
|
||||
|
||||
The key and the value are put on separate lines so the reported line says which of the
|
||||
two was asserted on: a value-only reader would name line 3.
|
||||
"""
|
||||
source = f'api_key = {{\n {key}:\n "live",\n}}\n'
|
||||
|
||||
finding = _finding_by_rule(_scan_python_sample(tmp_path, source), "secret-env-assignment")
|
||||
|
||||
assert finding["line"] == 2
|
||||
assert finding["evidence"] == "[redacted]"
|
||||
assert "a1b2c3d4e5f6g7h8i9j0" not in repr(finding)
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"source",
|
||||
[
|
||||
'tokens = {"access_token": os.getenv("ACCESS_TOKEN"), "refresh_token": os.getenv("REFRESH_TOKEN")}\n',
|
||||
'credentials = {"api_key": token_from_config, "password": config["password"]}\n',
|
||||
'tokens = {"outer": {"access_token": os.environ["ACCESS_TOKEN"]}}\n',
|
||||
'connect(tokens={"access_token": os.getenv("ACCESS_TOKEN")})\n',
|
||||
'tokens = {("access_token", "refresh_token"): runtime_tokens}\n',
|
||||
'api_key = {123456: os.getenv("API_KEY")}\n',
|
||||
'tokens = {"9f8e7d6c5b4a3210ff": runtime_token}\n',
|
||||
],
|
||||
)
|
||||
def test_secret_assignment_ignores_python_mapping_labels_with_runtime_values(tmp_path: Path, source: str) -> None:
|
||||
"""Mapping labels do not become credentials because the enclosing name is secret-like."""
|
||||
findings = _scan_python_sample(tmp_path, source)
|
||||
|
||||
assert [finding for finding in findings if finding["rule_id"].startswith("secret-")] == []
|
||||
|
||||
|
||||
@pytest.mark.parametrize("value", ['"9f8e7d6c5b4a3210ff"', 'os.getenv("ACCESS_TOKEN") if prod else "9f8e7d6c5b4a3210ff"'])
|
||||
def test_secret_assignment_reports_python_mapping_credential_value(tmp_path: Path, value: str) -> None:
|
||||
"""A label must neither hide a hardcoded value nor take its source location."""
|
||||
source = f'tokens = {{\n "access_token":\n {value},\n}}\n'
|
||||
|
||||
finding = _finding_by_rule(_scan_python_sample(tmp_path, source), "secret-env-assignment")
|
||||
|
||||
assert finding["line"] == 3
|
||||
assert finding["severity"] == "HIGH"
|
||||
assert finding["evidence"] == "[redacted]"
|
||||
assert "9f8e7d6c5b4a3210ff" not in repr(finding)
|
||||
|
||||
|
||||
def test_secret_assignment_reports_python_container_members_in_source_order(tmp_path: Path) -> None:
|
||||
"""Two literals in one container point at the first one written, so the finding's
|
||||
location does not depend on how the value expression happens to be traversed."""
|
||||
source = 'api_key = [\n "9f8e7d6c5b4a3210ff",\n "7d6c5b4a3210ff9e8",\n]\n'
|
||||
|
||||
assert _finding_by_rule(_scan_python_sample(tmp_path, source), "secret-env-assignment")["line"] == 2
|
||||
|
||||
|
||||
def test_secret_assignment_still_flags_python_starred_target_prefix(tmp_path: Path) -> None:
|
||||
"""``api_key, *rest = "…", "b"`` binds ``api_key`` to the first element: the star only
|
||||
absorbs the tail, so the prefix keeps its fixed position."""
|
||||
finding = _finding_by_rule(_scan_python_sample(tmp_path, 'api_key, *rest = "9f8e7d6c5b4a3210ff", "b"\n'), "secret-env-assignment")
|
||||
|
||||
assert finding["line"] == 1
|
||||
assert finding["evidence"] == "[redacted]"
|
||||
assert "9f8e7d6c5b4a3210ff" not in repr(finding)
|
||||
|
||||
|
||||
def test_secret_assignment_starred_element_value_stays_unreported(tmp_path: Path) -> None:
|
||||
"""The mirror case of the one above: here the credential lands in ``rest``, a list
|
||||
Python builds at runtime, so no name on that line has a fixed position for it.
|
||||
Pinned so the limitation is checked, not assumed."""
|
||||
source = 'mode, *rest = "prod", "9f8e7d6c5b4a3210ff"\n'
|
||||
|
||||
assert _secret_assignments(_scan_python_sample(tmp_path, source)) == []
|
||||
|
||||
|
||||
def test_secret_assignment_still_flags_python_starred_value_suffix(tmp_path: Path) -> None:
|
||||
"""``mode, api_key = *pair, "…"`` aligns from the right: the element after the star is
|
||||
the last one, so it reaches the last name whatever the starred sequence's length is."""
|
||||
finding = _finding_by_rule(_scan_python_sample(tmp_path, 'mode, api_key = *pair, "9f8e7d6c5b4a3210ff"\n'), "secret-env-assignment")
|
||||
|
||||
assert finding["line"] == 1
|
||||
assert finding["evidence"] == "[redacted]"
|
||||
assert "9f8e7d6c5b4a3210ff" not in repr(finding)
|
||||
|
||||
|
||||
def test_secret_assignment_star_on_both_sides_stays_unreported(tmp_path: Path) -> None:
|
||||
"""With a star on both sides the head and the tail can be read, but neither name in
|
||||
``api_key, *rest`` has a position that matches the literal on the right. Pinned so the
|
||||
limitation is checked, not assumed."""
|
||||
source = 'api_key, *rest = *pair, "9f8e7d6c5b4a3210ff"\n'
|
||||
|
||||
assert _secret_assignments(_scan_python_sample(tmp_path, source)) == []
|
||||
|
||||
|
||||
def test_secret_assignment_value_star_on_the_last_name_stays_unreported(tmp_path: Path) -> None:
|
||||
"""``api_key, mode = *pair, "…"`` does pair the literal, with ``mode``: the credential
|
||||
reaches a name this rule is not about, so nothing is reported."""
|
||||
source = 'api_key, mode = *pair, "9f8e7d6c5b4a3210ff"\n'
|
||||
|
||||
assert _secret_assignments(_scan_python_sample(tmp_path, source)) == []
|
||||
|
||||
|
||||
def test_secret_assignment_still_flags_python_nested_unpacking(tmp_path: Path) -> None:
|
||||
"""A parenthesised target element unpacks positionally too, so the credential reaches
|
||||
the name written at its own position rather than the tuple around it."""
|
||||
finding = _finding_by_rule(_scan_python_sample(tmp_path, 'host, (user, api_key) = "https://internal.example", ("u", "9f8e7d6c5b4a3210ff")\n'), "secret-env-assignment")
|
||||
|
||||
assert finding["line"] == 1
|
||||
assert "9f8e7d6c5b4a3210ff" not in repr(finding)
|
||||
|
||||
|
||||
def test_secret_assignment_unpacking_with_mismatched_lengths_stays_unreported(tmp_path: Path) -> None:
|
||||
"""``host, api_key = "a", "b", "c"`` cannot run at all, so there is no position to read
|
||||
the literal from. Pinned together with a second assignment because the pairing builds
|
||||
tuples elementwise: a mismatch must skip its own line, not raise and cost the file its
|
||||
other findings.
|
||||
"""
|
||||
source = 'host, api_key = "9f8e7d6c5b4a3210ff", "b", "c"\ntoken = "7d6c5b4a3210ff9e8"\n'
|
||||
|
||||
findings = _secret_assignments(_scan_python_sample(tmp_path, source))
|
||||
|
||||
assert [finding["line"] for finding in findings] == [2]
|
||||
|
||||
|
||||
def test_secret_assignment_ignores_python_keyword_environment_lookup(tmp_path: Path) -> None:
|
||||
"""The precision gain must survive the new binding forms: a keyword whose
|
||||
value is read from the environment is the documented remediation."""
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user