WIP: Custom Webhooks

[jolheiser]

This is a draft of custom webhooks.

It will close #14693 as well, as it includes both HTTP and binary-based webhooks.
This should also resolve #1089, at least as closely as I can think of without a true plugin system.

A configuration looks like this:

Executable

id: executable          # This is used to map the webhook type to this webhook
label: Executable       # The display name of the webhook
docs: https://google.com # The URL to the webhook's documentation (if any)
exec: ["./executable"]  # Command args to run
form:
  - id: username        # Required
    label: Username     # Required, the label for the input
    type: text          # Required, the type of input (text, number, bool, secret)
    required: true      # Optional, defaults to false
  - id: favorite-number
    label: Favorite Number
    type: number
    default: 12        # Optional, a default value
  - id: pineapple
    label: Pineapple on Pizza
    type: bool
  - id: passphrase
    label: Passphrase
    type: secret

Executables are executed relative to the path the custom webhook lives in, which is currently CUSTOM_PATH/webhooks

HTTP

id: http
label: HTTP
docs: https://duckduckgo.com
http: "https://dev.jolheiser.com" # The URL to send the payload to
# Same form config as above

Each custom webhook requires 2-3 files, namely an image.png, a config.yml, and possibly an executable if required.

On startup, Gitea scans the custom webhook directory (again, currently set to CUSTOM_PATH/webhooks) and adds them to a map of webhooks.

The payload sent to either type of webhook (via stdin for executables) is

{
  "form": {...},
  "payload": {...}
}

For executables, any headers are set in the environment as the header name, ie X-Gitea-Signature

---

Notes:

• What to do if a custom webhook is ever removed?
  • Do I set all matching ones to inactive, or remove them? I lean towards the former.
• I want to get this implementation as solid as possible prior to moving forward, so as to not create a fragile custom webhook ecosystem for developers.

[jolheiser]

And even as now, users can write their own webhook delegators to receive the calls from Gitea by Gitea Webhook and re-deliver them.

Thats essentially what the custom http hooks do.
The config allows custom fields, but past that the payload is essentially a gitea webhook.

While I am in favor of writing bridge software, it seems to have not caught on for one reason or another. Instead we keep adding more and more hard-coded webhooks.

Running external commands was mostly a shim for people who didn't want to run extra services for a bridge.

[wxiaoguang]

I have no objection to introduce a local command mechanism (although I think it's not the solution for cloud native, if there is a chance, we could use some other methods ....).

(update: I misread some context, I only worry about the local command. Indeed HTTP is cool and fine!)

If the local command is a must, then it could be kept as simple as possible. For example, maybe Gitea shouldn't do the form things. Gitea can provide a textarea to let users input their customized yaml config, and use a yaml-schema-validator to validate the config with the pre-defined yaml schema in the webhook definition.

[jolheiser]

This is both local command and http.

As for form, that was how I had imagined letting webhook makers define a limited UI to be passed to their webhook.

Regarding pasting configs, while I suppose that's possible, I was thinking this would more admin controlled rather than user.

Perhaps details should be discussed in Discord or the forum? I'm happy to figure out a good solution, as our webhook code continues to get more complex and messy as we add more hook types and even different hook scopes.

[codecov-commenter]

Codecov Report

Merging #19307 (cdec7a6) into main (59b30f0) will decrease coverage by 0.13%.
The diff coverage is 35.66%.

❗ Current head cdec7a6 differs from pull request most recent head b4eda5e. Consider uploading reports for the commit b4eda5e to get more accurate results

@@            Coverage Diff             @@
##             main   #19307      +/-   ##
==========================================
- Coverage   47.39%   47.26%   -0.14%     
==========================================
  Files         956      961       +5     
  Lines      133115   133894     +779     
==========================================
+ Hits        63093    63282     +189     
- Misses      62393    62947     +554     
- Partials     7629     7665      +36     

Impacted Files	Coverage Δ
cmd/hook.go	7.11% <0.00%> (ø)
cmd/serv.go	2.33% <0.00%> (ø)
cmd/web.go	0.00% <0.00%> (ø)
cmd/web_acme.go	0.00% <0.00%> (ø)
models/error.go	35.87% <ø> (-0.90%)	⬇️
models/org.go	62.99% <0.00%> (ø)
models/statistic.go	0.00% <0.00%> (ø)
models/webhook/webhook.go	70.00% <ø> (ø)
modules/context/permission.go	26.08% <0.00%> (ø)
modules/doctor/dbconsistency.go	4.54% <0.00%> (-0.52%)	⬇️
... and 130 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cafa2dc...b4eda5e. Read the comment docs.

[lunny]

How about to store them into database?

[jolheiser]

How about to store them into database?

Store what in the database?

[lunny]

How about to store them into database?

Store what in the database?

The custom webhooks yaml content.

[jolheiser]

How about to store them into database?

Store what in the database?

The custom webhooks yaml content.

Why? A custom webhook definition still requires an image and potentially an executable.

Unless we want to get rid of the executable idea, but imo there's benefit to not needing to spin up another server for a webhook translation.

[lunny]

How about to store them into database?

Store what in the database?

The custom webhooks yaml content.

Why? A custom webhook definition still requires an image and potentially an executable.

Unless we want to get rid of the executable idea, but imo there's benefit to not needing to spin up another server for a webhook translation.

Images could be svg or an embed one into data or uploaded into some places. And I also think we could use an embed javascript engine(i.e. https://github.com/dop251/goja ) to execute http request but an executable, because I think it's an overkill to run a command to fire a webhook. Or could you give some real cases to have to use a local command?

[luwol03]

Why not having the web hook specifications in the repo under .gitea? And isn't this like some kind of CI feature?

[lunny]

Why not having the web hook specifications in the repo under .gitea? And isn't this like some kind of CI feature?

That would make this exclusively repo level webhook. Also yes, webhooks are more or less a CI feature.

For running a command, mostly the idea was ease of use. Instead of having to call out to a translation service. Embedding a JS engine limits it to only JS. Running a command makes custom webhooks accessible to even the smallest Gitea instances, where you drop a dir in custom and call it a day.

I'm not against making this purely an HTTP translation service, however that basically boils it down to the existing gitea webhook + some custom UI.

If that's the case, I almost think a register API would work, that way there's no DB involved at all.

I think we can implement HTTP only in this PR and discuss local command one in other PRs.

Why I prefer store them into database, because I think puting the custom webhooks in files may make multiple Gitea instances work difficult.

[jolheiser]

So we're putting a yaml config in the db.
The image will be stored in assets somewhere and link back to a webhook via some ID.

Pasting yaml and an image is arguably less friendly than being able to unzip (or git pull) to a directory, but that might depend on the person. Having things on disk is similar to how template customizations work.

Is this going to be a new page on the admin dashboard, then?

[lunny]

arguably

Since we have more and more configurations in ini file, I think we should move configurations out of ini file as possible.

I think yes. We could move some webhook settings from ini to database(#18058) and have a configuration UI there including which webhooks should be enabled. There is a button to create a custom webhook type, then we can go to a new page. We can upload an image and have a yaml edit box, there are example and some template variables explanation there and yml syntax highlight?

[jolheiser]

Sounds like a lot of work for the user to gain a few UI boxes imo

Am HTTP-only version means there's very little difference between this and the existing generic gitea webhook. The main advantage is being able to define a limited UI, but if users need to click through pages and mannually copy/paste yaml to gain that UI, is there much advantage?

[oliverpool]

Regarding the payload customization part, mentioned in

#19307 (comment)

past that the payload is essentially a gitea webhook.

While I am in favor of writing bridge software, it seems to have not caught on for one reason or another. Instead we keep adding more and more hard-coded webhooks.

and in #19307 (comment)

For running a command, mostly the idea was ease of use. Instead of having to call out to a translation service.
Embedding a JS engine limits it to only JS.
Running a command makes custom webhooks accessible to even the smallest Gitea instances, where you drop a dir in custom and call it a day.

Did you consider a system like bloblang ?
That way the (custom) webhook can easily adjust the payload (adding, removing fields / conditionals...)

root = {}
root.message = if this.commits.length()>1 {
    this.commits.length().string() + "commits"
} else {
    "1 commit"
}

The URL, method and headers (authorization) have to be configured elsewhere.

There are (at least) 2 security aspects to study before choosing to go this way (with this processor or any other document mapping tool):

• can it be used as a denial-of-service attack? (by passing far to complex queries or infinite loop)
• what functions must be enabled/disabled (for instance env, file) - already possible by using the go library

[oliverpool]

Regarding the security aspects, they are only relevant if the processor is accessible by the end-user. If this is limited to the instance owner, it shouldn't be an issue.

From a quick look, the current custom webhooks could be replaced by such a system (it also supports unit-testing ;)

[oliverpool]

I've been playing a bit with this. Here is how it would look like for matrix.Create (html_link, html_to_markdown, ref_end_name and path_escape_segments are custom functions/methods created in newBloblangExecutor):

// Create implements PayloadConvertor Create method
func (m *MatrixPayloadUnsafe) Create(p *api.CreatePayload) (api.Payloader, error) {
	exe, err := newBloblangExecutor(`
let repoLink = html_link(this.repository.html_url, this.repository.full_name)
let refName = this.ref.ref_end_name()

let refURL = this.repository.html_url + match {
	this.ref.has_prefix("refs/heads/") => "/src/branch/"
	this.ref.has_prefix("refs/tags/") => "/src/tag/"
	_ => "/src/commit/"
} + $refName.path_escape_segments()

let refLink = html_link($refURL, $refName)
let text = "[%s:%s] %s created by %s".format($repoLink, $refLink, this.ref_type, this.sender.login)

root.body = $text.html_to_markdown()
root.msgtype = "TODO"
root.format = "org.matrix.custom.html"
root.formatted_body = $text
`)
	if err != nil {
		return nil, err
	}
	return exe.Query(p)
}

Let me know if you think this is worth exploring further. I can make a draft PR to share the whole code.

[jolheiser]

@oliverpool What would be the advantage over JS in this case?

[oliverpool]

I tried to make a comparison here #1089 (comment) (so as to not pollute this pull-request)

[oliverpool]

@jolheiser looks very good!

I did some local tests and I think jsonnet is a promising choice 👍

I played a bit with your latest commit and added a (basic) test:
https://github.com/jolheiser/gitea/compare/custom-webhook...oliverpool:gitea:custom-webhook?expand=1

Following this experiment, I had the following thoughts:

• instead of naming this "custom webhook", maybe this could be named "file-based-webhook" (to better reflect, that the configuration is file-based)
• instead of having an id in config.yml, why not directly take the directory name as id ?
• the CustomID is currently stored as a dedicated column. Do you think storing it inside the Meta would make sense? (so that the GetCustomPayload only needs `Payloader, EventType, Meta')
• I didn't see any use of CustomMeta.Secret yet, how do you plan to use it?
• code.gitea.io/gitea/modules/webhook.Path is a string. Maybe it could be replaced with a fs.FS to be more flexible (os.ReadFile becomes fs.ReadFile(webhook.FS, filename)). This would especially help in testing
• instead of hardcoding the http URL in the config, it could also be returned by some url.jsonnet (to allow for per-hook customization - like in telegram and matrix -- actually matrix as a url per hooktask 🤯 ):

local form = import 'form.libsonnet';

"http://" + form.host + "/api/post-message"

I am very excited about this, thank for your hard work!

[jolheiser]

Hey, thanks for giving my code a test! I hadn't quite finished up or tested my changes. Glad to see I'm on the right track, though!

instead of naming this "custom webhook", maybe this could be named "file-based-webhook" (to better reflect, that the configuration is file-based)

I'm not tied to "custom" as the name, but I think "file-based" might not accurately convey that they are "programmable" in a sense. Perhaps calling them jsonnet webhooks? (Who knows, maybe in the future jsonnet dies out and we need a different transformation lang?)

instead of having an id in config.yml, why not directly take the directory name as id ?

I thought about this, but overall I like that id in a config is more explicit, since that will be what ties everything together. For what it's worth, I think the recommendation should be that people try to namespace their custom webhooks to avoid collisions anyways.

the CustomID is currently stored as a dedicated column. Do you think storing it inside the Meta would make sense? (so that the GetCustomPayload only needs `Payloader, EventType, Meta')

The only reason I had it as a dedicated column was as an admittedly premature optimization in case any queries/migrations/etc needed to happen in the future for specific webhooks.

I didn't see any use of CustomMeta.Secret yet, how do you plan to use it?

Ah, I will need to look a bit further. Secret is used to encode the HMAC signature of the payload, however I don't know that it needs to be in Meta as it should already be added to the webhook table.

code.gitea.io/gitea/modules/webhook.Path is a string. Maybe it could be replaced with a fs.FS to be more flexible (os.ReadFile becomes fs.ReadFile(webhook.FS, filename)). This would especially help in testing

This makes sense, and I think in general the custom webhooks likely need a bit of refactoring anyways. I don't love having a global map, so I may swap them for an interface. That way they can live in-mem, on disk, or however someone else wants to add support in the future.

instead of hardcoding the http URL in the config, it could also be returned by some url.jsonnet (to allow for per-hook customization - like in telegram and matrix -- actually matrix as a url per hooktask 🤯 ):

Ah, yes, this will be removed. It was there because previously this would have been running against a bridge service, but now it is simply transformative.

[lunny]

Since #29145 merged, this PR can merge and do some refactors.