Skip to content

Build: Let DangerJS run with our script and token on forks#34013

Open
Sidnioulz wants to merge 3 commits intonextfrom
sidnioulz/danger-js-on-forks
Open

Build: Let DangerJS run with our script and token on forks#34013
Sidnioulz wants to merge 3 commits intonextfrom
sidnioulz/danger-js-on-forks

Conversation

@Sidnioulz
Copy link
Member

@Sidnioulz Sidnioulz commented Mar 4, 2026

What I did

Third-party repos can't run DangerJS any more. So our external contributors don't receive the feedback we designed DangerJS to provide them with.

See logs: https://github.com/storybookjs/storybook/actions/workflows/danger-js.yml?query=is%3Afailure

This PR switches the DangerJS workflow from running the source branch's code with the source secrets (aka, the fork) to running the target branch with its secret (aka the script on next, with our GitHub token).

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

ø

Manual testing

Will try to run a PR against this one, hopefully it should trigger with the right permissions.

Documentation

ø

Checklist for Maintainers

  • When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli-storybook/src/sandbox-templates.ts

  • Make sure this PR contains one of the labels below:

    Available labels
    • bug: Internal changes that fixes incorrect behavior.
    • maintenance: User-facing maintenance tasks.
    • dependencies: Upgrading (sometimes downgrading) dependencies.
    • build: Internal-facing build tooling & test updates. Will not show up in release changelog.
    • cleanup: Minor cleanup style change. Will not show up in release changelog.
    • documentation: Documentation only changes. Will not show up in release changelog.
    • feature request: Introducing a new feature.
    • BREAKING CHANGE: Changes that break compatibility in some way with current major version.
    • other: Changes that don't fit in the above categories.

🦋 Canary release

This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the @storybookjs/core team here.

core team members can create a canary release here or locally with gh workflow run --repo storybookjs/storybook publish.yml --field pr=<PR_NUMBER>

Summary by CodeRabbit

  • Chores
    • Adjusted automated review workflow trigger to run in the repository context for pull requests; workflow continues to run on opened and synchronize events. No other functional or user-facing changes were made, and there are no changes to exported/public behavior.

@Sidnioulz Sidnioulz added build Internal-facing build tooling & test updates ci:normal labels Mar 4, 2026
@nx-cloud
Copy link

nx-cloud bot commented Mar 4, 2026

View your CI Pipeline Execution ↗ for commit 2585708

Command Status Duration Result
nx run-many -t compile,check,knip,test,pretty-d... ❌ Failed 7m 4s View ↗

☁️ Nx Cloud last updated this comment at 2026-03-06 11:28:16 UTC

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 4, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3981e6f0-e612-4b7f-bf12-ec45a533ee2d

📥 Commits

Reviewing files that changed from the base of the PR and between bcab919 and 2585708.

📒 Files selected for processing (1)
  • .github/workflows/danger-js.yml
🚧 Files skipped from review as they are similar to previous changes (1)
  • .github/workflows/danger-js.yml

📝 Walkthrough

Walkthrough

The GitHub Actions workflow .github/workflows/danger-js.yml changes its trigger from pull_request to pull_request_target. No other functional changes to the workflow structure are indicated in this diff.

Changes

Cohort / File(s) Summary
GitHub Actions Workflow
.github/workflows/danger-js.yml
Changed workflow trigger: replaced on: pull_request with on: pull_request_target. No other observable modifications in this patch.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/danger-js.yml:
- Line 2: Change the workflow that uses the pull_request_target event to
explicitly scope GITHUB_TOKEN and avoid persisting checkout credentials: add a
top-level permissions block (e.g., permissions: contents: read, pull-requests:
write) to grant least-privilege access instead of default read/write, and in any
job that uses actions/checkout set with: persist-credentials: false so checkout
does not expose the token; look for the pull_request_target event and update the
workflow to include the permissions block and the actions/checkout
persist-credentials change.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6f22c7c1-ae9b-47a2-b198-75109aef0542

📥 Commits

Reviewing files that changed from the base of the PR and between 670866d and 41917e8.

📒 Files selected for processing (1)
  • .github/workflows/danger-js.yml

@@ -1,5 +1,5 @@
on:
pull_request:
pull_request_target:
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

GitHub Actions pull_request_target default GITHUB_TOKEN permissions

💡 Result:

For workflows triggered by pull_request_target, the GITHUB_TOKEN is granted read/write repository permissions by default, even if the pull request comes from a fork. [1]

However, the effective default permissions still start from your repo/org/enterprise “Workflow permissions” setting (restricted vs read/write), and can be further reduced/overridden by permissions: in the workflow (workflow- or job-level). Also, org/enterprise policies can enforce a more restrictive default. [1], [2], [3]

Key default-setting note: GitHub’s default “Workflow permissions” depends on when the org/enterprise was created (newer orgs default to read-only; older may default to read/write), but pull_request_target is the special case that runs in the base-repo context and thus can receive elevated permissions unless you explicitly restrict them. [1], [2]

Sources:
[1] GitHub Docs – Workflow syntax / how GITHUB_TOKEN permissions are calculated + pull_request_target read/write note
[2] GitHub Docs – Enterprise policy: default GITHUB_TOKEN permissions depend on org/enterprise creation date
[3] GitHub Docs – Repository Actions settings: configuring default GITHUB_TOKEN permissions


🌐 Web query:

Danger JS GitHub Actions required permissions comments pull requests

💡 Result:

To let Danger JS post a comment/status/check on a pull request from GitHub Actions using GITHUB_TOKEN, your workflow job must grant write scopes explicitly (especially if your repo/org defaults GITHUB_TOKEN to read-only).

Minimal required job permissions (common for Danger commenting + reporting)

permissions:
  pull-requests: write
  checks: write
  statuses: write

These are the permissions required by a commonly used Danger JS GitHub Action implementation. [1]

Token note (fork PRs / Dependabot)

On workflows triggered by pull_request from forks (and commonly Dependabot), GITHUB_TOKEN is read-only, so Danger can’t comment via the API. [2] Options people use:

  • Use a separate token (PAT / GitHub App) with PR write access (stored as a secret), or
  • Use pull_request_target to get base-repo permissions but treat it as high-risk and avoid checking out/executing untrusted PR code. [3]

Why you may need to set permissions: even if it “used to work”

GitHub lets orgs/enterprises default GITHUB_TOKEN to read-only; workflows then must opt-in per job using permissions:. [4]

Sources: [1] [2] [3] [4]


Scope GITHUB_TOKEN permissions explicitly after moving to pull_request_target.

Line 2 changes the workflow to a higher-trust event context where the token receives read/write repository permissions by default, even for fork pull requests. Add an explicit permissions block with least-privilege scopes and avoid persisting checkout credentials by default.

Suggested hardening patch
 on:
   pull_request_target:
 name: Danger JS
+permissions:
+  contents: read
+  pull-requests: write
+  checks: write
+  statuses: write
 jobs:
   dangerJS:
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v4
+        with:
+          persist-credentials: false
       - name: Danger JS
         uses: docker://ghcr.io/danger/danger-js:13.0.5
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/danger-js.yml at line 2, Change the workflow that uses the
pull_request_target event to explicitly scope GITHUB_TOKEN and avoid persisting
checkout credentials: add a top-level permissions block (e.g., permissions:
contents: read, pull-requests: write) to grant least-privilege access instead of
default read/write, and in any job that uses actions/checkout set with:
persist-credentials: false so checkout does not expose the token; look for the
pull_request_target event and update the workflow to include the permissions
block and the actions/checkout persist-credentials change.

Copy link
Member Author

Choose a reason for hiding this comment

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

Not dangerous because of how little we do in dangerjs but ideally we should use a separate token that can only do what dangerjs needs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build Internal-facing build tooling & test updates ci:normal

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

3 participants