Skip to content
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

Jump to bottom

Enable FQCNs for import_playbook to have subdirs #4412

Merged
merged 2 commits into from
Dec 11, 2024
Merged

Enable FQCNs for import_playbook to have subdirs #4412

merged 2 commits into from
Dec 11, 2024
+40 −7

Conversation

cavcrosby
Copy link
Contributor

Related to #4362.

@cavcrosby cavcrosby requested a review from a team as a code owner November 16, 2024 16:34
@cavcrosby
Copy link
Contributor Author

cavcrosby commented Nov 16, 2024 •
edited
Loading

Regarding the code analysis failing, ansible uses a similar regex that does not place a limit on the number of subdirectories a user can specify. Currently, I think that's the only applicable workaround to the catastrophic backtracking vulnerability, but even then, I don't think the linter should differ from what ansible sets.

ssbarnea
ssbarnea previously requested changes Nov 21, 2024
View reviewed changes
Copy link
Member

@ssbarnea ssbarnea left a comment

Choose a reason for hiding this comment

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

RE_IS_FQCN_OR_NAME = re.compile(r"^\w+((\.\w+){2,})*$")

To quote SonarQube: Make sure the regex used here, which is vulnerable to exponential runtime due to backtracking, cannot lead to denial of service.

@ssbarnea ssbarnea added the bug label Nov 21, 2024
@cavcrosby cavcrosby force-pushed the enable-fqcn-subdirs branch from ab92e3e to 6b70a68 Compare December 1, 2024 19:35
@cavcrosby cavcrosby force-pushed the enable-fqcn-subdirs branch from 6b70a68 to 35e93a1 Compare December 2, 2024 14:53
@cavcrosby cavcrosby force-pushed the enable-fqcn-subdirs branch from 35e93a1 to 0ab25b8 Compare December 3, 2024 16:23
@cavcrosby cavcrosby force-pushed the enable-fqcn-subdirs branch from f85fd4f to 3fc7329 Compare December 4, 2024 00:24
@ssbarnea ssbarnea self-requested a review December 4, 2024 14:27
@cavcrosby cavcrosby force-pushed the enable-fqcn-subdirs branch from 25f44a1 to 043ae65 Compare December 5, 2024 15:13
@cavcrosby cavcrosby force-pushed the enable-fqcn-subdirs branch from 043ae65 to 1d2fdc5 Compare December 5, 2024 15:27
@cavcrosby cavcrosby force-pushed the enable-fqcn-subdirs branch from 1d2fdc5 to 56478ba Compare December 5, 2024 15:27
@cavcrosby cavcrosby force-pushed the enable-fqcn-subdirs branch from 56478ba to 6ed5b76 Compare December 6, 2024 14:25
@cavcrosby
Enable FQCNs for import_playbook to have subdirs (ansible#4362)
fb2d239 
The max repetitions of 100 for the RE_IS_FQCN_OR_NAME regular expression
was not chosen for any particular reason other than being a sane number
with more than enough overhead to account for a long fully qualified
collection name (FQCN).
@cavcrosby cavcrosby force-pushed the enable-fqcn-subdirs branch from 6ed5b76 to fb2d239 Compare December 6, 2024 15:25
@shatakshiiii shatakshiiii enabled auto-merge (squash) December 11, 2024 12:50
@shatakshiiii shatakshiiii merged commit c6d9660 into ansible:main Dec 11, 2024
29 checks passed
@cavcrosby cavcrosby deleted the enable-fqcn-subdirs branch December 12, 2024 02:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shatakshiiii shatakshiiii shatakshiiii approved these changes

@audgirka audgirka audgirka approved these changes

@ssbarnea ssbarnea Awaiting requested review from ssbarnea

Assignees
No one assigned
Labels
bug
Projects
🧰 devtools project board
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@cavcrosby @ssbarnea @audgirka @shatakshiiii