fix(#19314, #15700): allow ssh/altssh subdomains in repo URLs to match webhook payload

[mtbennett-godaddy]

Fixes #19314. Fixes #15700 (cc: @robinlieb). Possibly fixes #16854 (cc: @aki-ks).

• Splits util/webhook’s getWebUrlRegex into separate GetWebUrlRegex and GetApiUrlRegex.
  • GetWebUrlRegex allows for optional altssh or ssh subdomain. (Addresses Webhooks not compatible with alternative SSH addressing schemes #19314.)
  • GetWebUrlRegex and GetApiUrlRegex both allow for username in https? URLs. (Addresses 2.9.0-RC1: Azure Devops git webhooks do not match with http[s] schemas #15700.)
• Updates util/webhook and applicationset/webhook to use the two new methods for consistent matching between the two.

---

Justification:

• GitHub supports ssh subdomain over 443: https://docs.github.com/en/authentication/troubleshooting-ssh/using-ssh-over-the-https-port
• GitLab supports altssh subdomain over 443: https://about.gitlab.com/blog/2016/02/18/gitlab-dot-com-now-supports-an-alternate-git-plus-ssh-port/#how-to-use-the-alternate-ssh-connection-on-gitlab.com
• Bitbucket supports altssh subdomain over 443: https://confluence.atlassian.com/bbkb/port-22-is-blocked-on-local-network-1168865232.html
• Azure DevOps does not support such an alternative: https://developercommunity.visualstudio.com/t/Can-I-use-SSH-on-a-port-other-than-22-in/10699062?sort=active&topics=fixed+in%3A+visual+studio+2019+version+16.6+preview+2

However, when using these patterns, webhook payload URLs will not include the subdomain, which causes the current regex matching to fail.

---

Signed-off-by: Matthew Bennett [email protected]

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).
• The title of the PR conforms to the Toolchain Guide
• 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.
• 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).
• My new feature complies with the feature status guidelines.
• I have added a brief description of why this PR is necessary and/or what this PR solves.
• Optional. My organization is added to USERS.md.
• Optional. For bug fixes, I've indicated what older releases this fix should be cherry-picked into (this may or may not happen depending on risk/complexity).

[bunnyshell]

🔴 Preview Environment stopped on Bunnyshell

See: Environment Details | Pipeline Logs

Available commands (reply to this comment):

• 🔵 /bns:start to start the environment
• 🚀 /bns:deploy to redeploy the environment
• ❌ /bns:delete to remove the environment

[codecov]

Codecov Report

Attention: Patch coverage is 96.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 55.23%. Comparing base (13235ad) to head (a3c50a5).
Report is 4 commits behind head on master.

Files with missing lines	Patch %	Lines
util/webhook/webhook.go	95.65%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #21227   +/-   ##
=======================================
  Coverage   55.22%   55.23%           
=======================================
  Files         337      337           
  Lines       57057    57061    +4     
=======================================
+ Hits        31511    31515    +4     
+ Misses      22850    22845    -5     
- Partials     2696     2701    +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[andrii-korotkov-verkada]

These regexes seem different from the previous ones.

[mtbennett-godaddy]

Correct, there are subtle differences.

The previous Application webhook web URL regex was:

(http://|https://|%s@|ssh://(%s@)?)%s(:[0-9]+|)[:/]%s(\.git)?

Which broke down to:

1. http://, https://, username@, ssh://, or ssh://username@ (where username is the RFC 3986 pattern defined elsewhere in the file)
2. hostname from original URL, passed through regexp.QuoteMeta()
3. :port or nothing
4. colon or slash
5. path from original URL, passed through regexp.QuoteMeta() (which might be empty)
6. optional .git extension

The previous ApplicationSet webhook web URL regex was:

(http://|https://|\w+@|ssh://(\w+@)?)%s(:[0-9]+|)[:/]%s(\.git)?

Which differed from the Application one in that:

• Any sequence of one or more word characters was allowed for username.
• The hostname and path values were not passed through regexp.QuoteMeta().

In the new common web URL regex used by both webhooks:

• Protocol and username are separated and are both optional. (This fixes 2.9.0-RC1: Azure Devops git webhooks do not match with http[s] schemas #15700 to allow usernames with https? URLs.)
• An optional altssh or ssh subdomain is allowed. (This fixes Webhooks not compatible with alternative SSH addressing schemes #19314 to allow for SSH-over-443 alternatives.)
• Hostname and path are both passed through regexp.QuoteMeta(). (This doesn’t fix any reported issue that I’m aware of, but prevents edge cases of hostname/path values containing anything that would have otherwise been interpreted as regex syntax.)

The previous ApplicationSet webhook API URL regex was:

(http://|https://|\w+@|ssh://(\w+@)?)%s(:[0-9]+|)[:/]

(Basically the same as the web URL regex, but no path or extension.)

The new API URL regex has the same benefits as listed above for the new web URL regex (except for the altssh/ssh subdomain allowance since these URLs will never have that).