kiwix / kiwix/overview

Workflow approvals on Pull Requests

Open
#121 6 comments 0 reactions 0 assignees View on GitHub
question
Dominant language
HTML
Stars
93
Forks
14
PR merge metrics
No merged PRs in 30d

Description

- we rely a lot on CI/CD workflows
- we want CI to be as painless as possible for us
- we want to foster PR from external contributors.
- we want our PR to run a number of checks
- we want external contributors to run same checks (first line of acceptance)
- a lot of workflows depends on secrets with varying criticality accros repositories.

A _solution_ to the problem of allowing external contributors to run our workflows is to require a manual approval by a Kiwix member before running the workflows.

The ever-growing number of features in Github makes it possible to do this in many ways. I've found at least three different approches

## Via _Actions permissions_

- Used at [libkiwix](https://github.com/kiwix/libkiwix)
- uses the repository's settings/actions preferences to set approval requirement
- Is currently set to first-time contributors (**which is a huge security risk IMO** but can be changed)
- Is not code-driven as this is ticks in Github UI.
- This does not uses Environment so no segregation of secrets but is thus very simple to deploy.
- There is no mention of this in the workflows which simple… but hidden

Image

`libkiwix` doesnt use much secrets: `CODECOV_TOKEN` which is not critical and `LAUNCHPAD_GPG` (from org)

## Via _Static Environment_

- Used at [python-libzim](https://github.com/openzim/python-libzim)
- No secret (only `CODECOV_TOKEN`)
- One environment (`release`) with all the secrets
- Environment is configured to require a reviewer
- Some workflow jobs statically reference this environment

```yaml
environment: release
```

Image

## Via _Dynamic Environment_

- Used at [apple](https://github.com/kiwix/kiwix-apple/tree/main)
- Secrets on the repo
- One environment (`external`) with trimmed-down secrets needed for external CI
- Environment is configured to require a reviewer
- workflow uses [`pull_request_target`](https://docs.github.com/en/actions/writing-workflows/choosing-when-your-workflow-runs/events-that-trigger-workflows#pull_request_target) instead of [`pull_request`](https://docs.github.com/en/actions/writing-workflows/choosing-when-your-workflow-runs/events-that-trigger-workflows#pull_request) event

The main reason for using `pull_request_target` is that it runs the workflows even if there are merge-conflict, which is to be expected on external PR. It comes with its own constraints though:

> This event runs in the context of the base of the pull request, rather than in the context of the merge commit, as the pull_request event does. This prevents execution of unsafe code from the head of the pull request that could alter your repository or steal any secrets you use in your workflow.

```yaml
authorize:
# sets environment based on origin of PR: internal (non-existent) for own-repo or external (requires reviewer to run) for external repos
environment: ${{ github.event_name == 'pull_request_target' && github.event.pull_request.head.repo.full_name != github.repository && 'external' || 'internal' }}
```

Image

----

We should discuss if a single method is possible for all our repos and standardize.

Contributor guide

Open the contributing guide

Research direction

Start by comparing the approval approaches described for libkiwix, python-libzim, and kiwix-apple, including their workflow events, environments, and secret requirements. Determine whether one method can be standardized across the repositories, and document the agreed approach and its security constraints as the definition of done.

Written by the indexing model from the issue text.

Assessment

Tech stack
github-actions
Domain
ci-cd, security
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.