Skip to content

fix: reject control characters in job names #263

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
crowecawcaw merged 3 commits into from
Feb 20, 2026
Merged

fix: reject control characters in job names #263

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f47d66c
fix: reject control characteres in job names
crowecawcaw Feb 11, 2026
de55f48
Add additional tests
crowecawcaw Feb 18, 2026
1fc4290
Merge branch 'mainline' into fix-control-chars
crowecawcaw Feb 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/openjd/model/v2023_09/_model.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,8 @@ class JobTemplateName(FormatString):
_min_length = 1
# Max length is validated after resolution in Job model, not here
# because the template name can contain format strings
# All unicode except the [Cc] (control characters) category
_regex = f"(?-m:^[^{_Cc_characters}]+\\Z)"
Copy link
Contributor

@mwiebe mwiebe Feb 11, 2026

Choose a reason for hiding this comment

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

What's the difference between this and _standard_string_regex?

Copy link
Contributor Author

@crowecawcaw crowecawcaw Feb 12, 2026

Choose a reason for hiding this comment

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

The model uses two regex engines: one is Rust's that's embedded in Pydantic and the other is Python's. They have slightly different syntax. _standard_string_regex is formatted for Pydantic/Rust's engine with \z and this regex is formatted for Python with \Z. This regex in this class is eventually evaluated here:

openjd-model-for-python/src/openjd/model/_format_strings/_dyn_constrained_str.py

Line 55 in 96651fa

if not re.match(cls._regex, value):

Here's another example of Python regexes in use in the model:

openjd-model-for-python/src/openjd/model/v2023_09/_model.py

Line 250 in 96651fa

_regex = f"(?-m:^[^{_Cc_characters}]+\\Z)"

Not sure there's a good reason for it. I'm guessing it is an outcome of upgrading from Pydantic v1 to v2.

Copy link
Contributor

@joel-wong-aws joel-wong-aws Feb 18, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

+1. From Kiro:

_standard_string_regex = rf"(?-m:^[^{_Cc_characters}]+\z)"

\\Z (uppercase Z): Matches at the end of the string OR just before a newline at the end

"hello\n" ✅ matches
"hello" ✅ matches

\z (lowercase z): Matches only at the absolute end of the string

"hello\n" ❌ does not match
"hello" ✅ matches

The lowercase \z seems to be the correct choice here.

Copy link
Contributor

@mwiebe mwiebe Feb 18, 2026

Choose a reason for hiding this comment

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

I remember there being something different between \z and \Z between pydantic v2 and Python regexes, because pydantic v2 switched to using the Rust regex crate.

Copy link
Contributor Author

@crowecawcaw crowecawcaw Feb 18, 2026

Choose a reason for hiding this comment

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

The \Z vs \z difference is intentional due to two different regex engines being in play:

  • _standard_string_regex (used by JobName, StepName, EnvironmentName via pydantic StringConstraints) uses lowercase \z — this is correct because pydantic v2 validates these patterns via pydantic-core, which uses the Rust regex crate. In Rust regex, \z is the absolute end-of-string anchor. Ref

  • JobTemplateName._regex (used by DynamicConstrainedStr._validate) uses uppercase \Z — this is correct because validation runs through Python's re.match(). In Python's re module, \Z matches only at the absolute end of the string (it does NOT match before a trailing newline, unlike $). Python doesn't support lowercase \z in versions before 3.14 — it throws re.PatternError: bad escape \z. Ref

So both are equivalent in behavior (absolute end-of-string), just spelled differently for their respective engines.

I've added additional tests to verify newlines are handled correctly.


def __new__(cls, value: str, *, context: ModelParsingContextInterface = ModelParsingContext()):
return super().__new__(cls, value, context=context)
Expand Down
45 changes: 45 additions & 0 deletions test/openjd/model/v2023_09/test_job_template.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,51 @@ def test_parse_success(self, data: dict[str, Any]) -> None:
1,
id="unknown key",
),
pytest.param(
{
"specificationVersion": "jobtemplate-2023-09",
"name": "Job\x1fName",
"steps": [STEP_TEMPLATE],
},
1,
id="name with control char 0x1F",
),
pytest.param(
{
"specificationVersion": "jobtemplate-2023-09",
"name": "Job\x7fName",
"steps": [STEP_TEMPLATE],
},
1,
id="name with control char 0x7F",
),
pytest.param(
{
"specificationVersion": "jobtemplate-2023-09",
"name": "Job\x9fName",
"steps": [STEP_TEMPLATE],
},
1,
id="name with control char 0x9F",
),
pytest.param(
{
"specificationVersion": "jobtemplate-2023-09",
"name": "JobName\n",
"steps": [STEP_TEMPLATE],
},
1,
id="name with trailing newline",
),
pytest.param(
{
"specificationVersion": "jobtemplate-2023-09",
"name": "Job\nName",
"steps": [STEP_TEMPLATE],
},
1,
id="name with embedded newline",
),
pytest.param(
{
"name": "Foo",
Expand Down
Loading