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

fix: valid username in webhook URL matching regex #12203

Merged
merged 1 commit into from Feb 17, 2023
Merged

fix: valid username in webhook URL matching regex #12203

merged 1 commit into from Feb 17, 2023

Conversation

mdsjip
Copy link
Contributor

@mdsjip mdsjip commented Jan 30, 2023 •
edited

Fixes #9055. This commit fixes incorrect regular expression used for URL matching.

Expected behavior: valid user info part is matched => webhook is sent.
Actual behavior: some valid user info is not matched, example: ssh://user-name@example.com/org/repo => webhook is not sent.

Context:

Related issues:

Note on DCO:

If the DCO action in the integration test fails, one or more of your commits are not signed off. Please click on the Details link next to the DCO action for instructions on how to resolve this.

Checklist:

  • Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes.
  • The title of the PR states what changed and the related issues number (used for the release note).
  • I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue.
  • I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them.
  • Does this PR require documentation updates?
  • I've updated documentation as required by this PR. (comment in the code)
  • Optional. My organization is added to USERS.md.
  • I have signed off all my commits as required by DCO
  • I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
  • My build is green (troubleshooting builds).

@mdsjip mdsjip force-pushed the webhook-regex branch 2 times, most recently from ecd6a31 to 91ceb4e Compare January 30, 2023 01:03
@codecov
Copy link

codecov bot commented Jan 30, 2023 •
edited

Codecov Report

Base: 47.45% // Head: 47.43% // Decreases project coverage by -0.02% ⚠️

Coverage data is based on head (6a2a5a0) compared to base (048902a).
Patch coverage: 100.00% of modified lines in pull request are covered.

❗ Current head 6a2a5a0 differs from pull request most recent head 2b9dc24. Consider uploading reports for the commit 2b9dc24 to get more accurate results

Additional details and impacted files 
@@            Coverage Diff             @@
##           master   #12203      +/-   ##
==========================================
- Coverage   47.45%   47.43%   -0.02%     
==========================================
  Files         246      246              
  Lines       41867    41825      -42     
==========================================
- Hits        19866    19839      -27     
+ Misses      20002    19990      -12     
+ Partials     1999     1996       -3
Impacted Files Coverage Δ
util/webhook/webhook.go 68.38% <100.00%> (+0.10%) ⬆️
util/git/creds.go 48.85% <0.00%> (-1.46%) ⬇️
pkg/apis/application/v1alpha1/repository_types.go 71.03% <0.00%> (-0.40%) ⬇️
util/git/client.go 49.65% <0.00%> (-0.35%) ⬇️
cmd/util/repo.go 0.00% <0.00%> (ø)
util/exec/exec.go 100.00% <0.00%> (ø)
util/settings/settings.go 49.20% <0.00%> (ø)
cmd/argocd/commands/repo.go 0.00% <0.00%> (ø)
cmd/argocd/commands/repocreds.go 0.00% <0.00%> (ø)
server/repository/repository.go 51.62% <0.00%> (+0.12%) ⬆️
... and 1 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

blakepettersson
blakepettersson approved these changes Jan 31, 2023
View reviewed changes
Copy link
Member

@blakepettersson blakepettersson left a comment

Choose a reason for hiding this comment

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

Seems reasonable to me at least 👍

@mdsjip
fix: valid username in webhook URL matching regex
2b9dc24 
This commit fixes incorrect regular expression used for URL matching.

Expected behavior: valid user info part is matched => webhook is sent.
Actual behavior: some valid user info is not matched, example: `ssh://user-name@example.com/org/repo` => webhook is not sent.

Context:
 - [RFC 3986 3.2.1 - User Information](https://www.rfc-editor.org/rfc/rfc3986#section-3.2.1)
 - [Username validation regex in shadow Linux package](https://github.com/shadow-maint/shadow/blob/master/libmisc/chkname.c#L36)
Signed-off-by: mdsjip <2284562+mdsjip@users.noreply.github.com>
@crenshaw-dev crenshaw-dev added cherry-pick/2.4 Candidate for cherry picking into the 2.4 release branch cherry-pick/2.5 cherry-pick/2.6 labels Feb 17, 2023
@crenshaw-dev crenshaw-dev merged commit 15edf10 into argoproj:master Feb 17, 2023
crenshaw-dev pushed a commit that referenced this pull request Feb 17, 2023
@mdsjip @crenshaw-dev
fix: valid username in webhook URL matching regex (#9055) (#12203)
d17aaf2 
This commit fixes incorrect regular expression used for URL matching.

Expected behavior: valid user info part is matched => webhook is sent.
Actual behavior: some valid user info is not matched, example: `ssh://user-name@example.com/org/repo` => webhook is not sent.

Context:
 - [RFC 3986 3.2.1 - User Information](https://www.rfc-editor.org/rfc/rfc3986#section-3.2.1)
 - [Username validation regex in shadow Linux package](https://github.com/shadow-maint/shadow/blob/master/libmisc/chkname.c#L36)

Signed-off-by: mdsjip <2284562+mdsjip@users.noreply.github.com>
crenshaw-dev pushed a commit that referenced this pull request Feb 17, 2023
@mdsjip @crenshaw-dev
fix: valid username in webhook URL matching regex (#9055) (#12203)
8833e41 
This commit fixes incorrect regular expression used for URL matching.

Expected behavior: valid user info part is matched => webhook is sent.
Actual behavior: some valid user info is not matched, example: `ssh://user-name@example.com/org/repo` => webhook is not sent.

Context:
 - [RFC 3986 3.2.1 - User Information](https://www.rfc-editor.org/rfc/rfc3986#section-3.2.1)
 - [Username validation regex in shadow Linux package](https://github.com/shadow-maint/shadow/blob/master/libmisc/chkname.c#L36)

Signed-off-by: mdsjip <2284562+mdsjip@users.noreply.github.com>
crenshaw-dev pushed a commit that referenced this pull request Feb 17, 2023
@mdsjip @crenshaw-dev
fix: valid username in webhook URL matching regex (#9055) (#12203)
5bfcff8 
This commit fixes incorrect regular expression used for URL matching.

Expected behavior: valid user info part is matched => webhook is sent.
Actual behavior: some valid user info is not matched, example: `ssh://user-name@example.com/org/repo` => webhook is not sent.

Context:
 - [RFC 3986 3.2.1 - User Information](https://www.rfc-editor.org/rfc/rfc3986#section-3.2.1)
 - [Username validation regex in shadow Linux package](https://github.com/shadow-maint/shadow/blob/master/libmisc/chkname.c#L36)

Signed-off-by: mdsjip <2284562+mdsjip@users.noreply.github.com>
@crenshaw-dev
Copy link
Collaborator

Cherry-picked onto release-2.6 for 2.6.3, release-2.5 for 2.5.12, and release-2.4 for 2.4.24.

@mdsjip mdsjip deleted the webhook-regex branch May 27, 2023 22:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@crenshaw-dev crenshaw-dev crenshaw-dev approved these changes

@blakepettersson blakepettersson blakepettersson approved these changes

Assignees
No one assigned
Labels
cherry-pick/2.4 Candidate for cherry picking into the 2.4 release branch cherry-pick/2.5 cherry-pick/2.6 ready-for-review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Webhook: Azure git repo urls fail to match
3 participants
@mdsjip @crenshaw-dev @blakepettersson