From 6abe383a9354d9b3042b4bf55ff89fe3f3f74c15 Mon Sep 17 00:00:00 2001 From: Ryunosuke O'Neil Date: Wed, 17 Jun 2026 14:25:41 +0200 Subject: [PATCH 1/2] fix: Don't apply saferEval default length cap to workflow XML values Commit bf6858d replaced eval() with saferEval() in WorkflowReader to avoid evaluating arbitrary code. saferEval enforces a 2048-byte cap, but non-string workflow parameters (lists/dicts serialised as repr()) are KB-scale and routinely exceed it, so parsing legitimate workflows failed with "Object string is too long (>2048 bytes)". Pass a generous finite cap (1 MiB) at this call site instead of the 2048 default. literal_eval still prevents code execution regardless of content; the ceiling remains as defence-in-depth against pathological/malicious input, bounding literal_eval's object-allocation blow-up. Legitimate workflow values never approach it. SaferEval's default is left unchanged for its other callers. --- src/DIRAC/Core/Workflow/WorkflowReader.py | 12 +++++++- .../Core/Workflow/test/Test_WorkflowReader.py | 28 +++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) create mode 100644 src/DIRAC/Core/Workflow/test/Test_WorkflowReader.py diff --git a/src/DIRAC/Core/Workflow/WorkflowReader.py b/src/DIRAC/Core/Workflow/WorkflowReader.py index dcfbaec94ac..a04d5452b7b 100755 --- a/src/DIRAC/Core/Workflow/WorkflowReader.py +++ b/src/DIRAC/Core/Workflow/WorkflowReader.py @@ -8,6 +8,16 @@ from DIRAC.Core.Workflow.Workflow import Workflow from DIRAC.Core.Utilities.SaferEval import saferEval +# Non-string workflow parameters (lists, dicts, ...) are serialised to XML as the +# repr() of their value and read back by evaluating that string. Legitimate values +# (e.g. a listoutput or a BKProcessingPass dict) are KB-scale and routinely exceed +# the saferEval default cap, so that default is too small here. We still bound the +# size as defence-in-depth against pathological/malicious input: literal_eval blocks +# code execution regardless of content, and this ceiling limits its object-allocation +# blow-up. Legitimate workflow values never approach it, so hitting it means the input +# is genuinely anomalous and should be rejected. +MAX_PARAMETER_VALUE_LEN = 1024 * 1024 # 1 MiB + class WorkflowXMLHandler(ContentHandler): def __init__(self, new_wf=None): @@ -113,7 +123,7 @@ def endElement(self, name): if self.stack[-1].isTypeString(): self.stack[-1].setValue(ch) else: - self.stack[-1].setValue(saferEval(ch)) + self.stack[-1].setValue(saferEval(ch, max_len=MAX_PARAMETER_VALUE_LEN)) # objects elif name == "Workflow": diff --git a/src/DIRAC/Core/Workflow/test/Test_WorkflowReader.py b/src/DIRAC/Core/Workflow/test/Test_WorkflowReader.py new file mode 100644 index 00000000000..f728f6579b9 --- /dev/null +++ b/src/DIRAC/Core/Workflow/test/Test_WorkflowReader.py @@ -0,0 +1,28 @@ +"""Regression tests for WorkflowReader parsing of workflow XML. + +Non-string workflow parameters (e.g. a ``list`` or ``dict``) are serialised to +XML as ``repr()`` of their value and read back by evaluating that string. Such +values can legitimately exceed the ``saferEval`` default length cap, so the +reader must not impose it. + +Regression: replacing ``eval`` with ``saferEval`` introduced a hard 2048-byte +limit, so parsing a workflow with a large non-string parameter failed with +``ValueError: Object string is too long (>2048 bytes)``. +""" + +from DIRAC.Core.Workflow.Parameter import Parameter +from DIRAC.Core.Workflow.Workflow import Workflow, fromXMLString + + +def test_round_trip_large_list_parameter(): + # A list value whose repr() comfortably exceeds the 2048-byte default cap. + big_list = [f"LFN:/lhcb/data/2026/RAW/file_{i:05d}.raw" for i in range(300)] + assert len(str(big_list)) > 2048 + + wf = Workflow() + wf.setName("BigParamWF") + wf.addParameter(Parameter("InputDataList", big_list, "list")) + + parsed = fromXMLString(wf.toXML()) + + assert parsed.findParameter("InputDataList").getValue() == big_list From 3c6dcd1eb65d65a0fe17238d83937fc7764f64e6 Mon Sep 17 00:00:00 2001 From: Ryunosuke O'Neil Date: Thu, 18 Jun 2026 14:09:56 +0200 Subject: [PATCH 2/2] fix: Don't apply saferEval default length cap to workflow substitution Commit bf6858d replaced eval() with saferEval() in Workflow/Utility.substitute to avoid evaluating arbitrary code. saferEval enforces a 2048-byte cap, but non-string workflow parameters (lists/dicts) are KB-scale and routinely exceed it, so substituting variables into a large parameter failed with "Object string is too long (>2048 bytes)". Pass a generous finite cap (1 MiB) at this call site instead of the 2048 default, matching the fix in WorkflowReader. literal_eval still prevents code execution regardless of content; the ceiling remains as defence-in-depth against pathological/malicious input. SaferEval's default is unchanged for its other callers. --- src/DIRAC/Core/Workflow/Utility.py | 11 +++++++++- src/DIRAC/Core/Workflow/test/Test_Utility.py | 22 ++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) create mode 100644 src/DIRAC/Core/Workflow/test/Test_Utility.py diff --git a/src/DIRAC/Core/Workflow/Utility.py b/src/DIRAC/Core/Workflow/Utility.py index 89fdc5c809c..3ab66536790 100644 --- a/src/DIRAC/Core/Workflow/Utility.py +++ b/src/DIRAC/Core/Workflow/Utility.py @@ -5,6 +5,15 @@ import re from DIRAC.Core.Utilities.SaferEval import saferEval +# Non-string workflow parameters (lists, dicts, ...) are evaluated from their repr() +# after variable substitution. Legitimate values are KB-scale and routinely exceed the +# saferEval default cap, so that default is too small here. We still bound the size as +# defence-in-depth against pathological/malicious input: literal_eval blocks code +# execution regardless of content, and this ceiling limits its object-allocation blow-up. +# Legitimate workflow values never approach it, so hitting it means the input is +# genuinely anomalous and should be rejected. +MAX_PARAMETER_VALUE_LEN = 1024 * 1024 # 1 MiB + def getSubstitute(param, skip_list=[]): """Get the variable name to which the given parameter is referring""" @@ -24,7 +33,7 @@ def substitute(param, variable, value): tmp_string = str(param).replace("@{" + variable + "}", value) if isinstance(param, str): return tmp_string - return saferEval(tmp_string) + return saferEval(tmp_string, max_len=MAX_PARAMETER_VALUE_LEN) def resolveVariables(varDict): diff --git a/src/DIRAC/Core/Workflow/test/Test_Utility.py b/src/DIRAC/Core/Workflow/test/Test_Utility.py new file mode 100644 index 00000000000..c16a907e631 --- /dev/null +++ b/src/DIRAC/Core/Workflow/test/Test_Utility.py @@ -0,0 +1,22 @@ +"""Regression tests for workflow Utility.substitute. + +When a non-string workflow parameter (e.g. a list) contains ``@{var}`` references, +``substitute`` rebuilds the object by evaluating the substituted repr() string. +Such values can legitimately exceed the ``saferEval`` default length cap. + +Regression: replacing ``eval`` with ``saferEval`` introduced a hard 2048-byte +limit, so substituting into a large non-string parameter failed with +``ValueError: Object string is too long (>2048 bytes)``. +""" + +from DIRAC.Core.Workflow.Utility import substitute + + +def test_substitute_large_non_string_value(): + # A non-string (list) parameter whose repr() exceeds the 2048-byte default cap. + param = [f"@{{PREFIX}}_{i:04d}" for i in range(200)] + assert len(str(param)) > 2048 + + result = substitute(param, "PREFIX", "LFN:/lhcb/data/2026") + + assert result == [f"LFN:/lhcb/data/2026_{i:04d}" for i in range(200)]