From 9bea09a0811a3bf143fed4d96ca885a07f46bcee Mon Sep 17 00:00:00 2001 From: Roland Dobai Date: Thu, 14 May 2026 13:13:20 +0200 Subject: [PATCH] fix(tools): seed IDF_VERSION before idf.py parses dependencies.lock Any idf.py invocation could hang indefinitely with no output while spawning an unbounded chain of "idf.py --version" subprocesses, eventually exhausting system memory. During init_cli(), idf.py parses the project's dependencies.lock to vet trusted component-provided idf_ext.py extensions. If the lock contains a component whose manifest has an "if: idf_version" clause, evaluating it calls idf-component-manager's _get_idf_version(). Outside a CMake build the IDF_VERSION environment variable is not set, so that function falls back to running "idf.py --version" as a subprocess, which re-enters init_cli() and recurses without bound. During a normal CMake build the component manager runs as a subprocess that already has IDF_VERSION in its environment (see build/config.env), so the fallback is never reached. The recursion happens only because idf.py runs component-manager code in-process during its own CLI startup, outside that context. Seed IDF_VERSION into os.environ early in init_cli(), before any dependencies.lock parsing, using the subprocess-free idf_version_from_cmake() helper. This gives in-process component-manager code the same IDF_VERSION a CMake build would provide. --- tools/idf.py | 14 ++ tools/test_build_system/test_idf_extension.py | 159 +++++++++++++++++- tools/test_idf_py/test_idf_py.py | 20 +++ 3 files changed, 192 insertions(+), 1 deletion(-) diff --git a/tools/idf.py b/tools/idf.py index 3814cc15ad9..7e0cb033d48 100755 --- a/tools/idf.py +++ b/tools/idf.py @@ -1021,6 +1021,20 @@ def init_cli(verbose_output: list | None = None) -> Any: # Set `complete_var` to not existing environment variable name to prevent early cmd completion project_dir = parse_project_dir(standalone_mode=False, complete_var='_IDF.PY_COMPLETE_NOT_EXISTING') + # Ensure IDF_VERSION is available for in-process component-manager code + # (e.g. dependencies.lock `if: idf_version` clauses). Outside a CMake build + # this env var is unset; without it idf-component-manager falls back to + # spawning `idf.py --version`, which re-enters here -> infinite recursion. + if 'IDF_VERSION' not in os.environ: + # Best-effort: if idf_version_from_cmake() returns None (corrupt/missing + # version.cmake) IDF_VERSION stays unset and the recursion guard does not apply. + idf_ver = idf_version_from_cmake() # 'vX.Y.Z' or None; regex parse, no subprocess + if idf_ver: + # Strip the leading 'v' to match the value a CMake build provides + # (see tools/cmake/version.cmake); component-manager code consumes + # this env var verbatim and cannot parse a 'v' prefix. + os.environ['IDF_VERSION'] = idf_ver.lstrip('v') + all_actions: dict = {} # Load extensions from components dir idf_py_extensions_path = os.path.join(os.environ['IDF_PATH'], 'tools', 'idf_py_actions') diff --git a/tools/test_build_system/test_idf_extension.py b/tools/test_build_system/test_idf_extension.py index 8783f3daf57..0a8746d341b 100644 --- a/tools/test_build_system/test_idf_extension.py +++ b/tools/test_build_system/test_idf_extension.py @@ -1,8 +1,11 @@ -# SPDX-FileCopyrightText: 2025 Espressif Systems (Shanghai) CO LTD +# SPDX-FileCopyrightText: 2025-2026 Espressif Systems (Shanghai) CO LTD # SPDX-License-Identifier: Apache-2.0 +import json import logging +import os import shutil +import signal import subprocess import sys import textwrap @@ -10,7 +13,10 @@ import typing from pathlib import Path import pytest +import yaml +from test_build_system_helpers import EnvDict from test_build_system_helpers import IdfPyFunc +from test_build_system_helpers import find_python from test_build_system_helpers import replace_in_file from conftest import should_clean_test_dir @@ -365,3 +371,154 @@ def test_extension_entrypoint_conflicting_names( assert 'Custom action with conflicting aliases' not in ret.stdout assert "Global option ['--project-dir'] already defined. External option will not be added." in ret.stderr assert 'This global option conflicts with existing one' not in ret.stdout + + +# ----------- Regression test: idf.py recursion via idf_version clause ----------- + + +@pytest.mark.skipif(os.name == 'nt', reason='start_new_session and os.killpg are POSIX-only') +def test_idf_version_recursion_on_reconfigure(idf_py: IdfPyFunc, test_app_copy: Path, default_idf_env: EnvDict) -> None: + """`idf.py reconfigure` must terminate even when IDF_VERSION is absent from the + environment and dependencies.lock contains an `if: idf_version` clause. + + Before the fix, the component-manager's lock parser called `idf.py --version` + to resolve the clause, which re-entered init_cli() -> infinite recursion / + fork bomb. The fix seeds IDF_VERSION early in init_cli() from + idf_version_from_cmake() (no subprocess). + + To force init_cli() down the path that loads the lock, the project must have + a component whose source is `project_managed_components` and which ships an + `idf_ext.py`. init_cli() then calls `_is_component_trusted()` -> + `_get_trusted_names_from_lock()` -> `LockManager(...).load()`, which eagerly + parses every `OptionalDependency` and evaluates the `idf_version` clause. + """ + logging.info('Test idf.py reconfigure must not recurse via idf_version lock clause') + + # 1. Create a managed component that ships idf_ext.py. It is referenced from + # main/idf_component.yml as a *local path* dependency, which makes the + # component manager register it with source == project_managed_components + # in build/project_description.json (enforced by the precondition check below). + ns = 'test_ns' + comp = 'test_comp' + comp_dir = test_app_copy / 'managed_components' / f'{ns}__{comp}' + comp_dir.mkdir(parents=True) + + (comp_dir / 'idf_ext.py').write_text( + textwrap.dedent( + TEST_EXT_TEMPLATE.format( + suffix='managed component extension', + global_options='', + actions="""'test-managed-action': { + 'callback': test_extension_action, + 'help': 'Test action from managed component' + }""", + ) + ) + ) + (comp_dir / 'CMakeLists.txt').write_text('idf_component_register(INCLUDE_DIRS ".")\n') + (comp_dir / 'idf_component.yml').write_text('version: "1.0.0"\n') + + # Declare the managed component as a local-path dependency of `main`. + # The `path` is resolved relative to the manifest file (main/), hence the + # leading `../`. The component manager assigns it the + # project_managed_components source on reconfigure. + (test_app_copy / 'main' / 'idf_component.yml').write_text( + textwrap.dedent( + f"""\ + dependencies: + {ns}/{comp}: + version: "*" + path: ../managed_components/{ns}__{comp} + """ + ) + ) + + # 2. First reconfigure: generates build/project_description.json (with the + # managed component listed) and dependencies.lock. + idf_py('reconfigure') + + # Precondition: verify the managed component is registered with + # source == 'project_managed_components'. init_cli() only parses + # dependencies.lock for components with that source; if the source ever + # changes this assertion will fail loudly instead of the test passing vacuously. + project_desc_path = test_app_copy / 'build' / 'project_description.json' + project_desc = json.loads(project_desc_path.read_text()) + comp_key = f'{ns}__{comp}' + assert comp_key in project_desc.get('build_component_info', {}), ( + f'Managed component {comp_key!r} not found in build_component_info; the test setup may be broken.' + ) + assert project_desc.get('all_component_info', {}).get(comp_key, {}).get('source') == 'project_managed_components', ( + f'Component {comp_key!r} source is not project_managed_components; ' + 'init_cli() will not parse dependencies.lock for this component and ' + 'the idf_version recursion guard is not exercised.' + ) + + # 3. Overwrite dependencies.lock, injecting an `if: idf_version` clause into + # the managed component's sub-dependency. This is written *after* the + # reconfigure because reconfigure regenerates the lock and would clobber a + # pre-planted one. + # + # Only the `rules:` clause is injected; FORMAT_VERSION, manifest_hash, + # source, and target are all preserved from the component manager's own output + # so the test does not encode lock-format internals. + lock_path = test_app_copy / 'dependencies.lock' + lock = yaml.safe_load(lock_path.read_text()) + lock['dependencies'][f'{ns}/{comp}']['dependencies'] = [{'name': 'idf', 'rules': [{'if': 'idf_version >=6.0'}]}] + lock_path.write_text(yaml.safe_dump(lock)) + + # 4. Run idf.py reconfigure as a subprocess with IDF_VERSION and + # CI_TESTING_IDF_VERSION stripped, reproducing the "outside a CMake build" + # context that triggered the recursion. + # Use start_new_session=True so the entire process tree (including any + # runaway children) lands in one process group that can be killed atomically. + env = dict(default_idf_env) + env.pop('IDF_VERSION', None) + env.pop('CI_TESTING_IDF_VERSION', None) + + idf_path = env['IDF_PATH'] + python = find_python(env['PATH']) + cmd = [python, os.path.join(idf_path, 'tools', 'idf.py'), 'reconfigure'] + + logging.debug(f'Running {cmd} without IDF_VERSION in env') + proc = subprocess.Popen( + cmd, + env=env, + cwd=str(test_app_copy), + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + start_new_session=True, + ) + + stdout, stderr = b'', b'' + try: + stdout, stderr = proc.communicate(timeout=30) + except subprocess.TimeoutExpired: + # Kill the entire process group to reap any runaway recursive children. + try: + os.killpg(os.getpgid(proc.pid), signal.SIGKILL) + except ProcessLookupError: + pass + proc.communicate() + pytest.fail( + 'idf.py reconfigure timed out (30 s) — likely hit the init_cli() recursion. ' + 'IDF_VERSION was not seeded early enough in init_cli().' + ) + + logging.debug(f'reconfigure stdout: {stdout.decode(errors="replace")}') + logging.debug(f'reconfigure stderr: {stderr.decode(errors="replace")}') + assert proc.returncode == 0, ( + f'idf.py reconfigure exited with {proc.returncode}.\n' + f'stdout: {stdout.decode(errors="replace")}\n' + f'stderr: {stderr.decode(errors="replace")}' + ) + + # Runtime proof: the warning is emitted by idf.py only when + # _is_component_trusted() returns False for a project_managed_components + # component — i.e. after _get_trusted_names_from_lock() -> LockManager.load() + # parsed the planted lock to completion (instead of forking). The local-path + # component is never a trusted WebServiceSource, so this warning is deterministic. + assert 'Not loading component extension from untrusted source' in stderr.decode(errors='replace'), ( + 'Expected the untrusted-source warning in stderr, which proves that ' + '_is_component_trusted() / LockManager.load() ran to completion with ' + 'IDF_VERSION seeded (no recursion). stderr was:\n' + stderr.decode(errors='replace') + ) diff --git a/tools/test_idf_py/test_idf_py.py b/tools/test_idf_py/test_idf_py.py index 52f3b86400f..c1a1b064417 100755 --- a/tools/test_idf_py/test_idf_py.py +++ b/tools/test_idf_py/test_idf_py.py @@ -158,6 +158,26 @@ class TestDependencyManagement(TestWithoutExtensions): ) +class TestIdfVersionSeeding(TestWithoutExtensions): + def test_idf_version_seeded_when_unset(self): + from idf_py_actions.tools import idf_version_from_cmake + + with mock.patch.dict(os.environ): + os.environ.pop('IDF_VERSION', None) + idf.init_cli()(args=['--dry-run', 'build'], standalone_mode=False) + self.assertIn('IDF_VERSION', os.environ) + expected = idf_version_from_cmake() + self.assertIsNotNone(expected) + expected_stripped = expected.lstrip('v') + self.assertEqual(os.environ['IDF_VERSION'], expected_stripped) + self.assertFalse(os.environ['IDF_VERSION'].startswith('v')) + + def test_idf_version_not_overwritten_when_set(self): + with mock.patch.dict(os.environ, {'IDF_VERSION': '0.0.0-sentinel'}): + idf.init_cli()(args=['--dry-run', 'build'], standalone_mode=False) + self.assertEqual(os.environ['IDF_VERSION'], '0.0.0-sentinel') + + class TestVerboseFlag(TestWithoutExtensions): def test_verbose_messages(self): output = subprocess.check_output(