Skip to content

Commit 2ef42f7

Browse files
experiment profile rejects quotes
1 parent 9672bf6 commit 2ef42f7

File tree

10 files changed

+233
-171
lines changed

10 files changed

+233
-171
lines changed

core/pioreactor/actions/leader/experiment_profile.py

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -127,19 +127,30 @@ def evaluate_bool_expression(bool_expression: BoolExpression, env: dict) -> bool
127127
return parse_profile_expression_to_bool(bool_expression, env=env)
128128

129129

130-
def check_syntax_of_bool_expression(bool_expression: BoolExpression) -> bool:
130+
def check_syntax_of_bool_expression(bool_expression: BoolExpression) -> str | None:
131131
from pioreactor.experiment_profiles.parser import check_syntax
132132

133133
if isinstance(bool_expression, bool):
134-
return True
134+
return None
135135

136136
if is_bracketed_expression(bool_expression):
137137
bool_expression = strip_expression_brackets(bool_expression)
138138

139+
import re
140+
141+
# Detect quoted strings early to provide a clearer error.
142+
quoted_matches = re.findall(r'"[^"]*"|\'[^\']*\'', bool_expression)
143+
if quoted_matches:
144+
return (
145+
"Quoted string literals are not supported in profile expressions. " f"Found {quoted_matches[0]}."
146+
)
147+
139148
# TODO: in a common expressions, users can use ::word:work which is technically not valid syntax. For checking, we replace with garbage
140149
bool_expression = bool_expression.replace("::", "dummy:", 1)
141150

142-
return check_syntax(bool_expression)
151+
if check_syntax(bool_expression):
152+
return None
153+
return "Syntax error in expression."
143154

144155

145156
def check_if_job_running(unit: pt.Unit, job: str) -> bool:
@@ -895,14 +906,20 @@ def _verify_experiment_profile(profile: struct.Profile) -> bool:
895906

896907
for job in actions_per_job:
897908
for action in actions_per_job[job]:
898-
if hasattr(action, "if_") and action.if_ and not check_syntax_of_bool_expression(action.if_): # type: ignore
899-
raise SyntaxError(f"Syntax error in {job}.{action}: `{action.if_}`")
900-
901-
if hasattr(action, "condition_") and action.condition_ and not check_syntax_of_bool_expression(action.condition_): # type: ignore
902-
raise SyntaxError(f"Syntax error in {job}.{action}: `{action.condition_}`")
903-
904-
if hasattr(action, "while_") and action.while_ and not check_syntax_of_bool_expression(action.while_): # type: ignore
905-
raise SyntaxError(f"Syntax error in {job}.{action}: `{action.while_}`")
909+
if hasattr(action, "if_") and action.if_:
910+
error = check_syntax_of_bool_expression(action.if_) # type: ignore
911+
if error:
912+
raise SyntaxError(f"{error} In {job}.{action}: `{action.if_}`")
913+
914+
if hasattr(action, "condition_") and action.condition_:
915+
error = check_syntax_of_bool_expression(action.condition_) # type: ignore
916+
if error:
917+
raise SyntaxError(f"{error} In {job}.{action}: `{action.condition_}`")
918+
919+
if hasattr(action, "while_") and action.while_:
920+
error = check_syntax_of_bool_expression(action.while_) # type: ignore
921+
if error:
922+
raise SyntaxError(f"{error} In {job}.{action}: `{action.while_}`")
906923

907924
return True
908925

core/pioreactor/experiment_profiles/profiles_spec.txt

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,8 @@ pioreactors:
9191
# - Numbers: integer or float (e.g., 1, -2.5)
9292
# - Booleans: true, false (case-insensitive)
9393
# - Names resolve to values provided in the expression environment (inputs, etc.); otherwise they remain strings.
94+
# Note: quoted string literals are not supported. Use bare tokens for string comparison
95+
# (e.g., `state == ready`, not `state == "ready"`).
9496
#
9597
# Operators
9698
# - Arithmetic: +, -, *, / (raises on division by zero), ** (exponent)
@@ -108,7 +110,7 @@ pioreactors:
108110
# MQTT lookups
109111
# - <unit>:<job>:<setting>[.<nested_key>]*
110112
# - ::<job>:<setting>[.<nested_key>]* (uses current unit)
111-
# Examples: unit():od_reading:od600, ::stirring:setting.target_rpm
113+
# Examples: unit():od_reading:od600, ::stirring:setting.target_rpm, pio02:monitor:$state
112114
# Fails if worker is inactive or topic missing.
113115
#
114116
# Conversion rules

core/pioreactor/web/static/asset-manifest.json

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"files": {
33
"main.css": "/static/static/css/main.9c7a48b7.css",
4-
"main.js": "/static/static/js/main.8b8d45fc.js",
4+
"main.js": "/static/static/js/main.b46e3258.js",
55
"static/media/pioreactor_cloud.webp": "/static/static/media/pioreactor_cloud.b15b29e435797dc69d76.webp",
66
"static/media/roboto-all-500-normal.woff": "/static/static/media/roboto-all-500-normal.0ab669b7a0d19b178f57.woff",
77
"static/media/roboto-all-700-normal.woff": "/static/static/media/roboto-all-700-normal.a457fde362a540fcadff.woff",
@@ -30,10 +30,10 @@
3030
"static/media/roboto-greek-ext-700-normal.woff2": "/static/static/media/roboto-greek-ext-700-normal.bd9854c751441ccc1a70.woff2",
3131
"index.html": "/static/index.html",
3232
"main.9c7a48b7.css.map": "/static/static/css/main.9c7a48b7.css.map",
33-
"main.8b8d45fc.js.map": "/static/static/js/main.8b8d45fc.js.map"
33+
"main.b46e3258.js.map": "/static/static/js/main.b46e3258.js.map"
3434
},
3535
"entrypoints": [
3636
"static/css/main.9c7a48b7.css",
37-
"static/js/main.8b8d45fc.js"
37+
"static/js/main.b46e3258.js"
3838
]
3939
}
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
<!doctype html><html lang="en"><head><meta charset="utf-8"/><link rel="icon" href="/static/favicon.ico"/><meta name="viewport" content="width=device-width,initial-scale=1"/><meta name="theme-color" content="#000000"/><meta name="description" content="Pioreactor"/><link rel="apple-touch-icon" href="/static/logo192.png"/><link rel="manifest" href="/static/manifest.json"/><script defer="defer" src="/static/static/js/main.8b8d45fc.js"></script><link href="/static/static/css/main.9c7a48b7.css" rel="stylesheet"></head><body><div id="root"></div></body></html>
1+
<!doctype html><html lang="en"><head><meta charset="utf-8"/><link rel="icon" href="/static/favicon.ico"/><meta name="viewport" content="width=device-width,initial-scale=1"/><meta name="theme-color" content="#000000"/><meta name="description" content="Pioreactor"/><link rel="apple-touch-icon" href="/static/logo192.png"/><link rel="manifest" href="/static/manifest.json"/><script defer="defer" src="/static/static/js/main.b46e3258.js"></script><link href="/static/static/css/main.9c7a48b7.css" rel="stylesheet"></head><body><div id="root"></div></body></html>

core/pioreactor/web/static/static/js/main.8b8d45fc.js

Lines changed: 0 additions & 154 deletions
This file was deleted.

core/pioreactor/web/static/static/js/main.b46e3258.js

Lines changed: 154 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

core/pioreactor/web/static/static/js/main.8b8d45fc.js.LICENSE.txt renamed to core/pioreactor/web/static/static/js/main.b46e3258.js.LICENSE.txt

File renamed without changes.

core/pioreactor/web/static/static/js/main.8b8d45fc.js.map renamed to core/pioreactor/web/static/static/js/main.b46e3258.js.map

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

core/tests/test_execute_experiment_profile.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -878,6 +878,20 @@ def test_profiles_in_github_repo() -> None:
878878
assert _verify_experiment_profile(profile)
879879

880880

881+
def test_verify_rejects_quoted_string_literals_in_if() -> None:
882+
action = Start(hours_elapsed=0.0, if_='${{ pio02:monitor:$state == "ready" }}')
883+
profile = Profile(
884+
experiment_profile_name="test_profile",
885+
plugins=[],
886+
common=CommonBlock(jobs={"job1": Job(actions=[action])}),
887+
pioreactors={},
888+
metadata=Metadata(author="test_author"),
889+
)
890+
891+
with pytest.raises(SyntaxError, match=r'Quoted string literals are not supported.*"ready"'):
892+
_verify_experiment_profile(profile)
893+
894+
881895
@patch("pioreactor.actions.leader.experiment_profile._load_experiment_profile")
882896
def test_api_requests_are_made(
883897
mock__load_experiment_profile,

frontend/src/components/DisplayProfile.jsx

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,24 @@ function displayVariable(string){
9595
return <Chip size="small" sx={{marginTop: "0px", marginBottom: "3px"}} label={<span style={expression}>{String(string).trim()}</span>} />
9696
}
9797

98+
function findQuotedString(expressionString){
99+
if (typeof expressionString !== 'string') return null;
100+
const match = expressionString.match(/"[^"]*"|'[^']*'/);
101+
return match ? match[0] : null;
102+
}
103+
104+
function findFirstDisallowedCharacter(expressionString){
105+
if (typeof expressionString !== 'string') return null;
106+
const allowedPattern = /^[a-zA-Z0-9_\$\.\:\s\(\)\+\-\*\/<>=!]+$/;
107+
if (allowedPattern.test(expressionString)) return null;
108+
for (let i = 0; i < expressionString.length; i += 1) {
109+
if (!allowedPattern.test(expressionString[i])) {
110+
return expressionString[i];
111+
}
112+
}
113+
return null;
114+
}
115+
98116
function almostExpressionSyntax(string){
99117
const almostPattern1 = /{+(.*?)}+/;
100118
const almostPattern2 = /{{(.*?)}}/;
@@ -152,6 +170,14 @@ function processBracketedExpression(value) {
152170
var match = pattern.exec(String(value));
153171

154172
if (match) {
173+
const quoted = findQuotedString(match[1]);
174+
if (quoted) {
175+
return <UnderlineSpan title={`Quoted string literals aren't supported. Found ${quoted}.`}>??</UnderlineSpan>;
176+
}
177+
const invalidChar = findFirstDisallowedCharacter(match[1]);
178+
if (invalidChar) {
179+
return <UnderlineSpan title={`Invalid character in expression: ${invalidChar}`}>??</UnderlineSpan>;
180+
}
155181
return displayExpression(match[1])
156182
}
157183

@@ -570,6 +596,9 @@ export const DisplayProfile = ({ data }) => {
570596
<Typography variant="h6"><ViewTimelineOutlinedIcon sx={{verticalAlign: "middle", margin:"0px 3px"}}/>{data?.experiment_profile_name || <UnderlineSpan title="missing `experiment_profile_name`">??</UnderlineSpan>}</Typography>
571597
<AuthorSection author={data?.metadata?.author} />
572598
<DescriptionSection description={data?.metadata?.description} />
599+
<Typography variant="body2" color="text.secondary" sx={{ mb: 1 }}>
600+
Note: quoted strings aren&apos;t supported in profile expressions. Use bare tokens like <span style={expression}>ready</span>, not <span style={expression}>&quot;ready&quot;</span>.
601+
</Typography>
573602
<br/>
574603
<PluginsSection plugins={data?.plugins} />
575604
<ParametersSection parameters={data?.inputs} />

0 commit comments

Comments
 (0)