Skip to content

Commit 8a27ad4

Browse files
didi
authored
Properly validate & normalize GitHub environment names. (#17699)
* Properly validate GitHub environment names * Fix environment normalization to handle whitespace correctly * Update translations * Coverage
1 parent b027c36 commit 8a27ad4

File tree

3 files changed

+112
-25
lines changed

3 files changed

+112
-25
lines changed

‎tests/unit/oidc/forms/test_github.py

Lines changed: 48 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
from requests import ConnectionError, HTTPError, Timeout
1818
from webob.multidict import MultiDict
1919

20+
from warehouse import i18n
2021
from warehouse.oidc.forms import github
2122
from warehouse.packaging.interfaces import (
2223
ProjectNameUnavailableExistingError,
@@ -391,15 +392,56 @@ def test_validate_workflow_filename(self, workflow_filename):
391392
with pytest.raises(wtforms.validators.ValidationError):
392393
form.validate_workflow_filename(field)
393394

395+
@pytest.mark.parametrize(
396+
("environment", "expected"),
397+
[
398+
("f" * 256, "Environment name is too long"),
399+
(" foo", "Environment name may not start with whitespace"),
400+
("foo ", "Environment name may not end with whitespace"),
401+
("'", "Environment name must not contain non-printable characters"),
402+
('"', "Environment name must not contain non-printable characters"),
403+
("`", "Environment name must not contain non-printable characters"),
404+
(",", "Environment name must not contain non-printable characters"),
405+
(";", "Environment name must not contain non-printable characters"),
406+
("\\", "Environment name must not contain non-printable characters"),
407+
("\x00", "Environment name must not contain non-printable characters"),
408+
("\x1f", "Environment name must not contain non-printable characters"),
409+
("\x7f", "Environment name must not contain non-printable characters"),
410+
("\t", "Environment name must not contain non-printable characters"),
411+
("\r", "Environment name must not contain non-printable characters"),
412+
("\n", "Environment name must not contain non-printable characters"),
413+
],
414+
)
415+
def test_validate_environment_raises(self, environment, expected, monkeypatch):
416+
request = pretend.stub(
417+
localizer=pretend.stub(translate=pretend.call_recorder(lambda ts: ts))
418+
)
419+
get_current_request = pretend.call_recorder(lambda: request)
420+
monkeypatch.setattr(i18n, "get_current_request", get_current_request)
421+
422+
form = github.GitHubPublisherForm(api_token=pretend.stub())
423+
field = pretend.stub(data=environment)
424+
425+
with pytest.raises(wtforms.validators.ValidationError) as e:
426+
form.validate_environment(field)
427+
428+
assert str(e.value).startswith(expected)
429+
430+
@pytest.mark.parametrize("environment", ["", None])
431+
def test_validate_environment_passes(self, environment):
432+
field = pretend.stub(data=environment)
433+
form = github.GitHubPublisherForm(api_token=pretend.stub())
434+
435+
assert form.validate_environment(field) is None
436+
394437
@pytest.mark.parametrize(
395438
("data", "expected"),
396439
[
397-
("wu-tang", "wu-tang"),
398-
("WU-TANG", "wu-tang"),
399-
("", ""),
400-
(" ", ""),
401-
("\t\r\n", ""),
402-
(None, ""),
440+
("wu-tang", "wu-tang"), # Non-alpha characters are preserved
441+
("WU-TANG", "wu-tang"), # Alpha characters are lowercased
442+
("Foo Bar", "foo bar"), # Whitespace is preserved
443+
("", ""), # Empty string is empty string
444+
(None, ""), # None and empty string are equivalent
403445
],
404446
)
405447
def test_normalized_environment(self, data, expected):

‎warehouse/locale/messages.pot

Lines changed: 30 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -739,54 +739,72 @@ msgstr ""
739739
msgid "ActiveState actor not found"
740740
msgstr ""
741741

742-
#: warehouse/oidc/forms/github.py:32
742+
#: warehouse/oidc/forms/github.py:33
743743
msgid "Specify GitHub repository owner (username or organization)"
744744
msgstr ""
745745

746-
#: warehouse/oidc/forms/github.py:39
746+
#: warehouse/oidc/forms/github.py:40
747747
msgid "Specify repository name"
748748
msgstr ""
749749

750-
#: warehouse/oidc/forms/github.py:41
750+
#: warehouse/oidc/forms/github.py:42
751751
msgid "Invalid repository name"
752752
msgstr ""
753753

754-
#: warehouse/oidc/forms/github.py:48
754+
#: warehouse/oidc/forms/github.py:49
755755
msgid "Specify workflow filename"
756756
msgstr ""
757757

758-
#: warehouse/oidc/forms/github.py:83
758+
#: warehouse/oidc/forms/github.py:84
759759
msgid "Unknown GitHub user or organization."
760760
msgstr ""
761761

762-
#: warehouse/oidc/forms/github.py:94
762+
#: warehouse/oidc/forms/github.py:95
763763
msgid "GitHub has rate-limited this action. Try again in a few minutes."
764764
msgstr ""
765765

766-
#: warehouse/oidc/forms/github.py:103
766+
#: warehouse/oidc/forms/github.py:104
767767
msgid "Unexpected error from GitHub. Try again."
768768
msgstr ""
769769

770-
#: warehouse/oidc/forms/github.py:111
770+
#: warehouse/oidc/forms/github.py:112
771771
msgid "Unexpected connection error from GitHub. Try again in a few minutes."
772772
msgstr ""
773773

774-
#: warehouse/oidc/forms/github.py:120
774+
#: warehouse/oidc/forms/github.py:121
775775
msgid "Unexpected timeout from GitHub. Try again in a few minutes."
776776
msgstr ""
777777

778-
#: warehouse/oidc/forms/github.py:132
778+
#: warehouse/oidc/forms/github.py:133
779779
msgid "Invalid GitHub user or organization name."
780780
msgstr ""
781781

782-
#: warehouse/oidc/forms/github.py:148
782+
#: warehouse/oidc/forms/github.py:149
783783
msgid "Workflow name must end with .yml or .yaml"
784784
msgstr ""
785785

786-
#: warehouse/oidc/forms/github.py:153
786+
#: warehouse/oidc/forms/github.py:154
787787
msgid "Workflow filename must be a filename only, without directories"
788788
msgstr ""
789789

790+
#: warehouse/oidc/forms/github.py:165
791+
msgid "Environment name is too long (maximum is 255 characters)"
792+
msgstr ""
793+
794+
#: warehouse/oidc/forms/github.py:170
795+
msgid "Environment name may not start with whitespace"
796+
msgstr ""
797+
798+
#: warehouse/oidc/forms/github.py:175
799+
msgid "Environment name may not end with whitespace"
800+
msgstr ""
801+
802+
#: warehouse/oidc/forms/github.py:181
803+
msgid ""
804+
"Environment name must not contain non-printable characters or the "
805+
"characters \"'\", \"\"\", \"`\", \",\", \";\", \"\\\""
806+
msgstr ""
807+
790808
#: warehouse/oidc/forms/gitlab.py:32
791809
msgid "Name ends with .git or .atom"
792810
msgstr ""

‎warehouse/oidc/forms/github.py

Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
_VALID_GITHUB_REPO = re.compile(r"^[a-zA-Z0-9-_.]+$")
2323
_VALID_GITHUB_OWNER = re.compile(r"^[a-zA-Z0-9][a-zA-Z0-9-]*$")
24+
_INVALID_ENVIRONMENT_CHARS = re.compile(r'[\x00-\x1F\x7F\'"`,;\\]', re.UNICODE)
2425

2526

2627
class GitHubPublisherBase(wtforms.Form):
@@ -153,16 +154,42 @@ def validate_workflow_filename(self, field):
153154
_("Workflow filename must be a filename only, without directories")
154155
)
155156

157+
def validate_environment(self, field):
158+
environment = field.data
159+
160+
if not environment:
161+
return
162+
163+
if len(environment) > 255:
164+
raise wtforms.validators.ValidationError(
165+
_("Environment name is too long (maximum is 255 characters)")
166+
)
167+
168+
if environment.startswith(" "):
169+
raise wtforms.validators.ValidationError(
170+
_("Environment name may not start with whitespace")
171+
)
172+
173+
if environment.endswith(" "):
174+
raise wtforms.validators.ValidationError(
175+
_("Environment name may not end with whitespace")
176+
)
177+
178+
if _INVALID_ENVIRONMENT_CHARS.search(environment):
179+
raise wtforms.validators.ValidationError(
180+
_(
181+
"Environment name must not contain non-printable characters "
182+
'or the characters "\'", """, "`", ",", ";", "\\"'
183+
)
184+
)
185+
156186
@property
157187
def normalized_environment(self):
188+
# The only normalization is due to case-insensitivity.
189+
#
158190
# NOTE: We explicitly do not compare `self.environment.data` to None,
159-
# since it might also be falsey via an empty string (or might be
160-
# only whitespace, which we also treat as a None case).
161-
return (
162-
self.environment.data.lower()
163-
if self.environment.data and not self.environment.data.isspace()
164-
else ""
165-
)
191+
# since it might also be falsey via an empty string.
192+
return self.environment.data.lower() if self.environment.data else ""
166193

167194

168195
class PendingGitHubPublisherForm(GitHubPublisherBase, PendingPublisherMixin):

0 commit comments

Comments
 (0)