Skip to content

patching dependabot workflow to address security risk#390

Open
giomrella wants to merge 5 commits intomainfrom
gr-hotfix-workflow
Open

patching dependabot workflow to address security risk#390
giomrella wants to merge 5 commits intomainfrom
gr-hotfix-workflow

Conversation

@giomrella
Copy link
Copy Markdown
Collaborator

Boris showed me some LLM output demonstrating how there could be potential vulnerabilities in this workflow. ChatGPT made these suggestions and they look sound to me, though I'm not sure how to test them.

The changes include:

  • using pull_request instead of pull_request_target to perform processing on the trusted main branch
  • using env.TITLE instead of setting the variable in bash
  • sanitizing the pull request title string
  • checking out the PR target after all processing just to push the updated NEWS.md file

Supersedes #389

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 3, 2026

Thank you for your contribution @micahwiesner67 🚀! Your pkgdown-site is ready for download 👉 here 👈!
(The artifact expires on 2026-04-15T15:35:11Z. You can re-generate it by re-running the workflow here.)

@micahwiesner67
Copy link
Copy Markdown
Collaborator

This workflow will not run if you change pull_request_target to pull_request

@giomrella
Copy link
Copy Markdown
Collaborator Author

This workflow will not run if you change pull_request_target to pull_request

Why not? We use on.pull_request.branches.main in other workflows

@micahwiesner67
Copy link
Copy Markdown
Collaborator

The pull_request_target functionality was used as it grants write permissions to the dependabot to update the NEWS.md file and push that change back to the repo.

@zsusswein
Copy link
Copy Markdown
Collaborator

zsusswein commented Apr 3, 2026

Why don't we drop the workflow and the NEWS.md file? We didn't end up doing releases in this repo/package and documenting release notes in the R package is the main purpose of the file.

@micahwiesner67
Copy link
Copy Markdown
Collaborator

I'm fine with that

@micahwiesner67
Copy link
Copy Markdown
Collaborator

After initially talking with Katie and Gio, I believe we all agreed there isn't really a risk b/c of the github actor field / way this workflow is setup there are no secrets exposed and a fork wouldn't actually allow anyone to willy-nilly trigger this run (only the dependabot author can trigger this workflow) and again no secrets are exposed. That being said, if we don't want to support the NEWS.md file as it's excessive, I am glad to remove. I will remove the NEWS.md file and the dependabot workflow and wait for input from others (we can pull in or decide this isn't the approach we want to use).

Note: this workflow can't be replaced with pull_request (opposed to pull_request_target).

@micahwiesner67 micahwiesner67 enabled auto-merge (squash) April 10, 2026 18:49
@micahwiesner67 micahwiesner67 disabled auto-merge April 10, 2026 18:49
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.

5 participants