Skip to content

fix: Don't apply saferEval default length cap to workflow XML values#8628

Merged
chrisburr merged 1 commit into
DIRACGrid:integrationfrom
ryuwd:fix/workflow-reader-saferEval-length
Jun 18, 2026
Merged

fix: Don't apply saferEval default length cap to workflow XML values#8628
chrisburr merged 1 commit into
DIRACGrid:integrationfrom
ryuwd:fix/workflow-reader-saferEval-length

Conversation

@ryuwd

@ryuwd ryuwd commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

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.

BEGINRELEASENOTES

*Workflow
FIX: increased saferEval limit to 1 MiB for Workflow XML handling

ENDRELEASENOTES

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.
@ryuwd ryuwd requested review from atsareg and fstagni as code owners June 17, 2026 12:27

@chrisburr chrisburr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#worksforgridpp

@fstagni fstagni closed this Jun 18, 2026
@fstagni fstagni reopened this Jun 18, 2026
@chrisburr

Copy link
Copy Markdown
Member

This is needed in v8 and v9

@sfayer sfayer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll cherry-pick this back to the other branches...

@chrisburr chrisburr merged commit 9dca897 into DIRACGrid:integration Jun 18, 2026
43 of 45 checks passed
@DIRACGridBot DIRACGridBot added the sweep:ignore Prevent sweeping from being ran for this PR label Jun 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

sweep:ignore Prevent sweeping from being ran for this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants