fix: reject control characters in job names#263
fix: reject control characters in job names#263crowecawcaw wants to merge 2 commits intoOpenJobDescription:mainlinefrom
Conversation
Signed-off-by: Stephen Crowe <6042774+crowecawcaw@users.noreply.github.com>
crowecawcaw
changed the title
| # 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)" |
There was a problem hiding this comment.
What's the difference between this and _standard_string_regex?
There was a problem hiding this comment.
+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.
There was a problem hiding this comment.
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.
|
I think we should add a fuzz test suite to OpenJD and have broader testing. Maybe a good backlog item for the OpenJD team. |
Signed-off-by: Stephen Crowe <6042774+crowecawcaw@users.noreply.github.com>
|
|
We actually have a new-ish fuzzer that uncovered a couple issues: https://github.com/OpenJobDescription/openjd-model-for-python/blob/mainline/test/openjd/model/test_fuzz.py We could definitely extend it for better coverage. In this particular case, I don't think it would have helped because the problem was not that a regex was faulty, it's that the library was missing a validation. |
4 participants
What was the problem/requirement? (What/Why)
The spec bans control characters in job names, but this package allowed them.
Spec reference: https://github.com/OpenJobDescription/openjd-specifications/blob/mainline/wiki/2023-09-Template-Schemas.md#111-jobname
What was the solution? (How)
Add a regex to reject them.
What is the impact of this change?
How was this change tested?
Add unit tests. Also by running the conformance tests against the changes: OpenJobDescription/openjd-specifications#103
Was this change documented?
n/a
Is this a breaking change?
No
Does this change impact security?
No
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.