diff --git a/cwltool/job.py b/cwltool/job.py index 9dd5b6662..e6efa3475 100644 --- a/cwltool/job.py +++ b/cwltool/job.py @@ -511,11 +511,16 @@ def prepare_environment( elif runtimeContext.preserve_environment: self._preserve_environment_on_containers_warning(runtimeContext.preserve_environment) - env.update(self.extract_environment(runtimeContext, envVarReq)) - # Set required env vars + # Preserve host variables first. + env.update(self.extract_environment(runtimeContext, {})) + + # Keep required CWL runtime vars deterministic. env.update(self._required_env()) + # Allow explicit EnvVarRequirement to override defaults. + env.update(envVarReq) + # Set on ourselves self.environment = env diff --git a/tests/env-preserve-step-tool.cwl b/tests/env-preserve-step-tool.cwl new file mode 100644 index 000000000..c95cafe09 --- /dev/null +++ b/tests/env-preserve-step-tool.cwl @@ -0,0 +1,11 @@ +#!/usr/bin/env cwl-runner +cwlVersion: v1.2 +class: CommandLineTool + +inputs: [] +baseCommand: env +arguments: ["-0"] + +outputs: + env: + type: stdout diff --git a/tests/env-preserve-step-wf.cwl b/tests/env-preserve-step-wf.cwl new file mode 100644 index 000000000..a7a7ac28b --- /dev/null +++ b/tests/env-preserve-step-wf.cwl @@ -0,0 +1,20 @@ +#!/usr/bin/env cwl-runner +cwlVersion: v1.2 +class: Workflow + +inputs: [] + +outputs: + env: + type: File + outputSource: env_step/env + +steps: + env_step: + run: env-preserve-step-tool.cwl + in: [] + out: [env] + requirements: + EnvVarRequirement: + envDef: + TMPDIR: /custom/tmpdir diff --git a/tests/test_environment.py b/tests/test_environment.py index 7d037af57..8423c1223 100644 --- a/tests/test_environment.py +++ b/tests/test_environment.py @@ -1,5 +1,7 @@ """Test passing of environment variables to tools.""" +import io +import json import os from abc import ABC, abstractmethod from collections.abc import Callable, Mapping @@ -9,13 +11,17 @@ import pytest from packaging.version import Version +from cwltool.env_to_stdout import deserialize_env +from cwltool.main import main from cwltool.singularity import get_version from .util import ( env_accepts_null, + get_data, get_tool_env, needs_docker, needs_singularity_not_apptainer, + working_directory, ) # None => accept anything, just require the key is present @@ -313,3 +319,70 @@ def test_preserve_all( raise # Other variables can be overridden assert val == os.environ[vname] + + +def test_preserve_entire_environment_honors_step_env_requirements( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Test that step EnvVarRequirement is honored with --preserve-entire-environment. + + This test verifies the fix for the bug where EnvRequirements set on + workflow steps are not honored when using --preserve-entire-environment. + """ + tmp_prefix = str(tmp_path / "canary") + + # Create a simple test environment variable that should be preserved + extra_env = { + "TEST_PRESERVED_VAR": "should_be_preserved", + } + + # Prepare arguments for cwltool + args = [ + "--no-container", + f"--tmpdir-prefix={tmp_prefix}", + "--preserve-entire-environment", + get_data("tests/env-preserve-step-wf.cwl"), + ] + + # Add extra env vars + for k, v in extra_env.items(): + monkeypatch.setenv(k, v) + + # Run cwltool + stdout = io.StringIO() + stderr = io.StringIO() + + with working_directory(tmp_path): + rc = main(argsl=args, stdout=stdout, stderr=stderr) + + assert ( + rc == 0 + ), f"cwltool failed with rc={rc}\nstdout:\n{stdout.getvalue()}\nstderr:\n{stderr.getvalue()}" + + # Parse the output + output = json.loads(stdout.getvalue()) + env_file = output["env"]["path"] + + # Read and deserialize the environment from the file + with open(env_file) as f: + env = deserialize_env(f.read()) + + # Verify that the step's EnvVarRequirement is honored + # The step has: EnvVarRequirement with envDef: TMPDIR: /custom/tmpdir + # This should override the --tmpdir-prefix setting + assert "TMPDIR" in env, f"TMPDIR not found in environment. " f"Environment: {env}" + assert env["TMPDIR"] == "/custom/tmpdir", ( + f"EnvVarRequirement value not set correctly. " + f"Expected '/custom/tmpdir', got {env.get('TMPDIR')!r} " + f"(This indicates the bug - step's EnvVarRequirement is being ignored)" + ) + + # Verify that preserved environment variables are also present + assert "TEST_PRESERVED_VAR" in env, ( + f"Environment variable should be preserved with --preserve-entire-environment. " + f"Environment: {env}" + ) + assert env["TEST_PRESERVED_VAR"] == "should_be_preserved", ( + f"Preserved environment variable has wrong value. " + f"Expected 'should_be_preserved', got {env.get('TEST_PRESERVED_VAR')!r}" + )