Resolve permissions errors

[nwiltsie]

Description

This is a major reworking of the Nextflow testing workflow that fixes #30, fixes #31, and fixes #32.

You can see a full demo of the updated workflow in https://github.com/uclahs-cds/user-nwiltsie-pipeline/pull/18 (and the success after merging, here and here).

The current workflow, as of #29, has a few problems:

• PRs opened by dependabot have restricted permissions (Dependabot PRs have restricted permissions for testing #31)
• Reviews on PRs to protected branches can only be dismissed by maintainers (Handle review dismissal protections #32)
• As running the tests inherently involves executing pipeline code, we are theoretically vulnerable to maliciously-crafted pipelines that could access organizational secrets

Following advice from this link, I've now split the test execution into two workflow: a low-privilege workflow that performs a bunch of safety checks and establishes a common baseline, and a high-privilege workflow that triggers on the successful conclusion of the first.

That means that each pipeline will need two workflows like this:

---
name: Trigger tests
on:
  issue_comment:
    types: [created]
  pull_request_target:
    branches:
      - main
  push:
    branches:
      - main

permissions:
  pull-requests: write
  statuses: write

jobs:
  check-user:
    uses: uclahs-cds/tool-Nextflow-action/.github/workflows/test-setup.yml@main

---
name: Nextflow

on:
  workflow_run:
    workflows: [Trigger Tests]
    types:
      - completed

permissions:
  pull-requests: write
  contents: write
  packages: read
  actions: read
  statuses: write

jobs:
  tests:
    if: ${{ github.event.workflow_run.conclusion == 'success' }}
    uses: uclahs-cds/tool-Nextflow-action/.github/workflows/nextflow-tests.yml@main
    secrets: inherit

The first job protects against the following things:

• Users without write permissions trying to run tests
• Race conditions (privileged users triggering tests and malicious users immediately pushing new changes)
• Fixes not being possible

The second job has more elevated permissions, but will only run after the success of the first.

---

Unfortunately, this is a breaking change with the current implementation of the tests. Here are the next steps:

1. Get this PR approved
2. Disable testing workflow in each existing pipeline
3. Merge these changes
4. Open PRs to update workflows in existing pipelines

Checklist

• This PR does NOT contain Protected Health Information (PHI). A repo may need to be deleted if such data is uploaded.
  Disclosing PHI is a major problem¹ - Even a small leak can be costly².

• This PR does NOT contain germline genetic data³, RNA-Seq, DNA methylation, microbiome or other molecular data⁴.

• This PR does NOT contain other non-plain text files, such as: compressed files, images (e.g. .png, .jpeg), .pdf, .RData, .xlsx, .doc, .ppt, or other output files.

  To automatically exclude such files using a .gitignore file, see here for example.

• I have read the code review guidelines and the code review best practice on GitHub check-list.

• I have set up or verified the main branch protection rule following the github standards before opening this pull request.

• The name of the branch is meaningful and well formatted following the standards, using [AD_username (or 5 letters of AD if AD is too long)]-[brief_description_of_branch].

• I have added the major changes included in this pull request to the CHANGELOG.md under the next release version or unreleased, and updated the date.

Footnotes

1. UCLA Health reaches $7.5m settlement over 2015 breach of 4.5m patient records ↩

2. The average healthcare data breach costs $2.2 million, despite the majority of breaches releasing fewer than 500 records. ↩

3. Genetic information is considered PHI.
   Forensic assays can identify patients with as few as 21 SNPs ↩

4. RNA-Seq, DNA methylation, microbiome, or other molecular data can be used to predict genotypes (PHI) and reveal a patient's identity. ↩

[nwiltsie]

A few other details:

Reviews on protected branches can't be dismissed: fine, the bot just uses comments now.

In addition to /fix-tests, commenting /run-tests will (re-)run the tests, as if new changes were pushed. That command is also protected against users without write access to the repo.

Due to using workflow_run, the test jobs don't get automatic status checks on the PR anymore. The low-permission/filtering job does show up. I'm adding an explicit Nextflow tests status, updated three times:

1. End of the low-permission job: set to pending with no link
2. Start of the high-permission job: set to pending with the link to the job
3. End of the high-permission job: set to success or failure

[aholmes]

Can github-script use external files for the scripts? We may want to (later) consider moving them out of the YAML file for ease of maintenance and testability.

[nwiltsie]

I'm 100% with you, and in general I think no - I'll look for more details, but I don't think the reusable workflow repo gets checked out.

[nwiltsie]

It seems like we could - https://github.com/orgs/community/discussions/25294 - that might be worth it.

[nwiltsie]

Also https://github.com/actions/github-script?tab=readme-ov-file#run-a-separate-file

[aholmes]

Cool! That doesn't look too bad.