Skip to content

⚒️ Add trusted-list bypass automation for reviewed blocklist exceptions#239069

Open
409H wants to merge 13 commits into
mainfrom
feature/bypass-trusted-lists
Open

⚒️ Add trusted-list bypass automation for reviewed blocklist exceptions#239069
409H wants to merge 13 commits into
mainfrom
feature/bypass-trusted-lists

Conversation

@409H
Copy link
Copy Markdown
Collaborator

@409H 409H commented Apr 21, 2026

Summary

Adds a reviewed bypass flow for cases where a domain must be blocklisted even though it appears on a trusted comparison list such as Tranco, CoinMarketCap, CoinGecko, Snaps Registry, or known dapps.

Changes

  • Moved trusted-list bypass entries out of test/test-lists.ts into test/resources/trusted-list-bypass.txt.
  • Added shared trusted-list helper logic in test/trusted-list-utils.ts.
  • Updated list tests to load bypass entries from the new plaintext bypass file.
  • Added .github/trusted-list-bypass-reviewers.json with a list of initial reviewers.
  • Added /skip-trusted-lists PR comment automation.
  • Added bin/apply-trusted-list-bypass.ts to:
    • verify the commenter is approved,
    • detect trusted-list failures,
    • append required bypass entries,
    • and let CI rerun with the explicit bypass committed.
  • Documented the trusted-list checking and bypass process in README.md.

Notes

The automation only pushes bypass updates to same-repo PR branches. For forked PRs, a maintainer still needs to apply the bypass file update manually.


Note

Medium Risk
Adds an issue_comment-triggered GitHub Action with contents: write that can commit to PR branches, so misconfiguration could allow unintended repository writes despite the reviewer allowlist guardrails. Logic also changes how trusted-list false positives are detected/waived, which can affect CI gating of blocklist changes.

Overview
Adds a reviewer-gated bypass mechanism for cases where a blocklisted domain appears on CI’s trusted comparison lists.

Trusted-list exceptions are moved out of test/test-lists.ts into a new plaintext test/resources/trusted-list-bypass.txt, with shared parsing/detection extracted into test/trusted-list-detection.ts + test/trusted-list-utils.ts and list tests updated to load and honor the bypass file.

Introduces /skip-trusted-lists automation: a new workflow listens for PR comments, verifies the commenter against .github/trusted-list-bypass-reviewers.json, runs bin/apply-trusted-list-bypass.ts to compute required bypass entries, and commits updates back to same-repo PR branches (reporting fork PRs as manual-only). Documentation is expanded in README.md to explain the trusted-list checks and bypass process.

Reviewed by Cursor Bugbot for commit 2be8734. Bugbot is set up for automated code reviews on this repo. Configure here.

@409H 409H requested review from a team, FrederikBolding and kumavis as code owners April 21, 2026 16:27
Comment thread .github/workflows/trusted-list-bypass.yml Outdated
Comment thread test/trusted-list-utils.ts Outdated
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit c4846e9. Configure here.

Comment thread test/trusted-list-utils.ts
Comment thread .github/workflows/trusted-list-bypass.yml
Copy link
Copy Markdown
Contributor

@0xOhm 0xOhm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test/trusted-list-utils.ts:7parseTrustedListBypass doesn't filter comment-only lines

The regex /\s+#.*$/u only strips inline comments (preceded by whitespace). Pure comment lines like # These domains are... have # as the first character with no preceding whitespace, so they pass through into the bypass Set. Functionally harmless today since no blocklist entry would match a comment string, but it pollutes the Set and could mask issues. Suggest:

.filter((line) => line && !line.startsWith("#"))

.github/workflows/trusted-list-bypass.yml:82-108 — "Report completion" and "Commit" steps run even when the bypass script fails

Both steps use if: steps.pr.outputs.is_fork == 'false' without checking prior step success. In GitHub Actions, a custom if expression replaces the implicit success() check — so if an unauthorized user triggers /skip-trusted-lists and the Node script exits with an error, these steps still execute. The commit step is a harmless no-op, but the completion step posts "bypass check completed" which is misleading. Should add && success():

if: steps.pr.outputs.is_fork == 'false' && success()

@0xOhm
Copy link
Copy Markdown
Contributor

0xOhm commented May 4, 2026

The issue is that the trusted-list bypass workflow says it will push a bypass commit and CI will rerun, but the push currently happens with the default GITHUB_TOKEN.

GitHub intentionally suppresses most workflow runs caused by GITHUB_TOKEN activity. So when this workflow runs git push origin "HEAD:${HEAD_REF}", the PR branch may get the new bypass commit, but the normal pull_request / push Build and Test workflow will not automatically start for that new commit.

That leaves the PR in a bad middle state: the bypass file was updated, but the checks that should verify the new head commit may stay stale or missing. Someone would then need to manually rerun CI or push another commit, which defeats the purpose of this automation.

Preferred fix: use a short-lived GitHub App installation token scoped only to this repo, with minimal permissions, only for the final push step. A fine-grained bot PAT scoped only to this repo is an acceptable fallback. Another option is to explicitly dispatch the Build and Test workflow after the commit, but then build-test.yml needs to support dispatched base/head SHAs correctly. @409H

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.

2 participants