Please (!) rethink WOODPECKER_PLUGINS_PRIVILEGED

[pat-s]

The current implementation is a pain in practice for both admins and users.
No matter if in a public or private instance.

The implementation should ideally allow to

• specify single images only (to allow for all tags)
• specify tag ranges using >= etc, so one can e.g. say >=4.0.0

Yes, not so easy to implement as the backend needs to be able to interpret semver ranges and likely only semver would be supported at all.

With the current approach, user workflows might fail by simply bumping the image (either manually or automated) if it is not explicitly whitelisted. Often enough, users don't even know which tags are whitelisted as they do not even have access to the server configuration. This will then turn into everyone using :latest in the workflows and server config as this is the only config to not touch the server config and get into trouble in pipelines -> not good! 😒

[anbraten]

The latest changes to WOODPECKER_PLUGINS_PRIVILEGED made Woodpecker a lot less user friendly for my team as well 🙃. Focusing on securely running pipelines is one of the big pros of Woodpecker IMO. However looking at recent security issues it is really difficult to get this done by our current approaches.

Those are the main risks I know of (feel free to highlight other categories):

• Secret extraction (includes registry credentials) / using secrets to running things of behalf of the original owner (interacting with forges, faking releases, ...)
• Attacking the agent / server environment by escaping the pipeline shell (network, file-system attacks)
• Spam / DOS: like creating a lot of pipelines, blocking agents from doing proper task by spamming pipelines

My suggestion would be to change the approach of mitigating these risks a bit. Instead of locking down secrets and allowed plugins Woodpecker should be more restrictive which pipelines are executed based on the user submitting them and require approvals where necessary. For example forks should probably always require approvals. By default PRs from outside contributors as well. I started such an approach in #3348 introducing following approval modes:

• allow every pipeline except PRs from forks (new default)
• require approval for all PRs
• require approval for every pipeline (current protected mode)

I would provide a few modes covering most common configs (not over complicating settings for "normal" users) and as suggested this could be extendable in the future using a simple approval http service returning sth like http code 200 for approved and 400 for require approval.

[qwerty287]

Iirc if you specify just the image without a tag, it should allow all tags. Otherwise this seems a bug to me

[xoxys]

Tested it now, using images without any tag works.

[qwerty287]

Nice, so I guess we can close this?

[pat-s]

Depends. While I can verify that setting no tag works (maybe I made a mistake in my first tries or was confused by the docs wording), the issue that no tag ranges are accepted still applies.

In practice, most admins will just use the "no tag" version to allow for all versions as having a hard match on single tags is not practicable/maintainable at all.

Not sure if this is an "enhancement" then or an actual regression of the initial implementation. Releasing it without range support will make the initial intention of the feature pretty much useless IMO.

[xoxys]

While I understand your point, IMO the correct direction is to focus on the gated feature, e.g. #3348 Support version ranges in a generic way is quite hard as container tags don't follow any standard.

Focussing on who can trigger automatic CI runs instead of how to prevent damage while everyone can trigger automatic ci runs is way more effective.

[pat-s]

Fully agree with you there and I also think that #3348 is a big step forward. My initial concerns about WOODPECKER_PLUGINS_PRIVILEGED were about a hard forced tag match which would have caused a lot of trouble. As this no longer applies and WOODPECKER_PLUGINS_PRIVILEGED is now pretty much an exposed way to allow custom privileged plugins instead of hardcoding a few plugins internally, I am fine with the changes 👍️ (despite the fact that this shouldn't have been introduced in a patch release 🥴️)