Fix lambda body sink discovery#18
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8eb0f37ebb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| continue | ||
| if isinstance(child, ast.Call): | ||
| yield child | ||
| yield from _own_calls(child) |
There was a problem hiding this comment.
Avoid flagging safe lambda-body sinks as raw
When this recursion now descends into ast.Lambda bodies, the sink rule starts checking calls that the L2 taint pass never resolves: _resolve_expr only visits lambda defaults (src/wardline/scanner/taint/variable_level.py:378-382), and _calls_with_enclosing_stmt still skips lambdas before building call-site snapshots. In a trusted function such as cb = lambda: eval('1 + 1'), the lambda-body eval has no function_call_site_arg_taints entry, so worst_arg_taint falls back to UNKNOWN_RAW for any argument and emits PY-WL-107 on a trusted literal. Please either collect lambda-body arg taints or avoid treating these missing lambda-body snapshots as raw.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR updates Wardline’s dangerous-sink discovery so calls inside lambda bodies are included in sink-rule evaluation, closing a false-negative gap where sinks like eval() could be hidden inside lambdas and missed.
Changes:
- Update
_own_calls()to traverseast.Lambdabodies (while still skippingFunctionDef/AsyncFunctionDef/ClassDefbodies). - Update/extend sink-rule unit tests to expect calls found inside lambda bodies.
- Add a regression test ensuring
PY-WL-107fires wheneval()is invoked inside a lambda body.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/wardline/scanner/rules/_sink_helpers.py |
Changes _own_calls() traversal semantics so lambda bodies are included in sink discovery. |
tests/unit/scanner/rules/test_sink_rules.py |
Updates expectations around lambda sink discovery and adds a regression test for eval() inside a lambda body. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def _own_calls(node: ast.AST) -> Iterator[ast.Call]: | ||
| """Yield every ``ast.Call`` in *node*'s own scope (never descending into nested | ||
| def/class/lambda — those are separate scopes / separate entities).""" | ||
| """Yield every ``ast.Call`` in *node*'s own analyzable scope. | ||
|
|
||
| Function, async-function, and class bodies are indexed as their own entities, so | ||
| they are not traversed here. Lambda bodies are intentionally traversed because | ||
| the entity index does not emit separate lambda entities; skipping them would hide | ||
| dangerous calls from sink rules. | ||
| """ | ||
| for child in ast.iter_child_nodes(node): | ||
| if isinstance(child, (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef)): | ||
| continue | ||
| if isinstance(child, ast.Lambda): | ||
| for default in (*child.args.defaults, *child.args.kw_defaults): | ||
| if default is not None: | ||
| if isinstance(default, ast.Call): | ||
| yield default | ||
| yield from _own_calls(default) | ||
| continue | ||
| if isinstance(child, ast.Call): | ||
| yield child | ||
| yield from _own_calls(child) |
| assert [(call.lineno, sink) for call, sink in sink_calls(func, frozenset({"eval"}), {}, "m")] == [ | ||
| (2, "eval") | ||
| ] |
Apply the pinned ruff 0.15.15 format (sweeps up pre-existing main drift in diagnostics.py/test_diagnostics.py plus this PR's test file) so the branch is format-clean, and document the lambda-body sink-discovery fix under Security. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ts (#19) PR #18 made sink DISCOVERY (_own_calls) descend into lambda bodies, but the L2 taint walk's _resolve_expr resolved only lambda *defaults*, never the body — so a sink call inside a lambda body had no entry in function_call_site_arg_taints and worst_arg_taint fell to the pessimistic flow-insensitive fallback (WLN-ENGINE-FLOW-INSENSITIVE-FALLBACK), firing UNKNOWN_RAW for ANY lambda-body sink in a trusted function regardless of the real argument taint (false positives on 'lambda: eval("safe")' and a shadowing 'lambda cmd: eval(cmd)'). Resolve lambda bodies in a second pass (_resolve_lambda_bodies) AFTER the forward walk finalises var_taints, against those FINAL taints. Final-state is the sound choice for a closure: a lambda defers execution and captures free variables by reference, so a variable assigned raw AFTER the lambda is defined ('src="safe"; cb=lambda: eval(src); src=read_raw(p)') is a real deferred sink and must still fire — a definition-site pass would silently miss it (a false negative, the cardinal sin for a fail-closed analyzer). Each body is resolved in an isolated scope copy so lambda-local bindings (params, walrus) never leak, and the lambda's own parameters are reset to the neutral seed (function_taint) so they shadow enclosing names rather than inheriting their taint. Net effect is a strict improvement vs main: real and deferred lambda-body sinks fire flow-sensitively (no warning); clean, final-state-clean, and shadowed-param values no longer false-fire. Full suite green (coverage 91.77%); mypy clean; dogfood self-scan unchanged (0 new). Co-authored-by: John Morrissey <john@wardline.dev> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Per the 2026-06-25 decision, the stray-artifact sweep deletes on BOTH the CLI and the MCP doctor tool (repair:true), not CLI-only. Risk is bounded by the action (confinement under proj, managed-pattern-inside-.wardline/ only, no-follow, nested-root stop, report-only for unstamped/bare-managed) rather than hidden from the agent surface — consistent with MCP-primary / 'agents operate and extend'. Implementation must flip the _DOCTOR_TOOL destructiveHint False->True and note deletion in the tool description. Updated §1/§4 intro/§4.2/§6 test #18/§7/§8; residual-risk note added. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Motivation
ast.Lambdaas a separate scope and only inspected lambda default expressions, which allowed dangerous sinks (e.g.eval,exec, deserializers, subprocess, imports) placed inside lambda bodies to be omitted from sink-rule evaluation and produce false-green results.Description
_own_callsinsrc/wardline/scanner/rules/_sink_helpers.pyto traverse lambda bodies as part of the enclosing analyzable scope while still skippingFunctionDef,AsyncFunctionDef, andClassDefbodies which are emitted as separate entities._own_callsdocstring to document the intent that lambda bodies are traversed because lambdas are not indexed as separate entities.tests/unit/scanner/rules/test_sink_rules.pyto expect calls found in lambda bodies and add a regression test assertingPY-WL-107fires forcb = lambda: eval(src).Testing
ruff checkon the modified files which passed; the linter reported no issues.PYTHONPATH=src pytestfor the new/adjusted unit tests (test_sink_calls_include_lambda_bodyandtest_own_calls_preserve_lambda_default_calls) which passed locally in the environment.PyYAML) that confirmedUntrustedToExecnow findsPY-WL-107forevalinside a lambda body.tests/unit/scanner/rules/test_sink_rules.pyin this environment hitPyYAMLavailability issues (the environment could not installPyYAML), so a full test-suite run was blocked by that dependency rather than by the change itself.Codex Task