Skip to content

fix: add safe type assertions in Bitbucket webhook handlers#27757

Open
SebTardif wants to merge 1 commit intoargoproj:masterfrom
SebTardif:fix/webhook-unsafe-type-assertions
Open

fix: add safe type assertions in Bitbucket webhook handlers#27757
SebTardif wants to merge 1 commit intoargoproj:masterfrom
SebTardif:fix/webhook-unsafe-type-assertions

Conversation

@SebTardif
Copy link
Copy Markdown

Summary

Two type assertions in the Bitbucket webhook handler use the single-value form, which panics if the underlying value has an unexpected type. Since webhooks accept unauthenticated external HTTP requests, a crafted payload can crash the Argo CD server.

Bug 1: Bitbucket Server clone links (affectedRevisionInfo)

In the Bitbucket Server webhook path, elements of the clone links array are asserted as map[string]any without an ok check. A malformed webhook payload where any clone link entry is not a map causes a runtime panic.

The outer assertion on line 291 already uses the safe two-value form (clone, ok := ....([]any)), but the inner loop assertion does not:

// Before (panics on non-map element):
link := l.(map[string]any)

// After:
link, ok := l.(map[string]any)
if !ok {
    continue
}

This pattern was introduced in #3036.

Bug 2: Bitbucket Cloud diffstat path (fetchDiffStatFromBitbucket)

The diffstat path value is asserted as string without an ok check. If the Bitbucket API returns a non-string value for the path field, this panics:

// Before (panics on non-string value):
changedFiles[i] = changedFilePath.(string)

// After:
if s, ok := changedFilePath.(string); ok {
    changedFiles[i] = s
}

This was introduced in #21811.

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 Title of the PR
  • Does this PR require documentation updates? No.
  • I have signed off all my commits as required by DCO
  • My build is green.

Two type assertions in the Bitbucket webhook handler use the single-value
form, which panics if the underlying value has an unexpected type.

In affectedRevisionInfo (Bitbucket Server path), elements of the clone
links array are asserted as map[string]any without an ok check. A
malformed or malicious webhook payload where any clone link entry is not
a map causes a runtime panic, crashing the webhook handler.

In fetchDiffStatFromBitbucket (Bitbucket Cloud path), the diffstat path
value is asserted as string without an ok check. If the Bitbucket API
returns a non-string path value, this panics.

Both assertions now use the two-value (comma ok) form consistent with
the surrounding code (e.g., line 291 already uses the safe form for the
outer clone assertion).

Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
@SebTardif SebTardif requested a review from a team as a code owner May 8, 2026 16:26
@bunnyshell
Copy link
Copy Markdown

bunnyshell Bot commented May 8, 2026

✅ Preview Environment deployed on Bunnyshell

Component Endpoints
argocd https://argocd-cjqemo.bunnyenv.com/
argocd-ttyd https://argocd-web-cli-cjqemo.bunnyenv.com/

See: Environment Details | Pipeline Logs

Available commands (reply to this comment):

  • 🔴 /bns:stop to stop the environment
  • 🚀 /bns:deploy to redeploy the environment
  • /bns:delete to remove the environment

@codecov
Copy link
Copy Markdown

codecov Bot commented May 8, 2026

Codecov Report

❌ Patch coverage is 60.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.88%. Comparing base (f7a4c09) to head (4552ff6).

Files with missing lines Patch % Lines
util/webhook/webhook.go 60.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #27757      +/-   ##
==========================================
- Coverage   63.92%   63.88%   -0.04%     
==========================================
  Files         421      421              
  Lines       57657    57659       +2     
==========================================
- Hits        36856    36836      -20     
- Misses      17345    17363      +18     
- Partials     3456     3460       +4     

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant