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

Open
crowecawcaw wants to merge 2 commits into
base: mainline
Choose a base branch
from
Open

fix: reject control characters in job names #263

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 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
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

@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.


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