Skip to content

Conversation

@blaginin
Copy link

@blaginin blaginin commented Sep 29, 2025

Request for adding a new GitHub Action to the allow list

Overview

I want to add this action, because for Datafusion we need a way to check if the PR has touched some files, and Merge Queue doesn't work with paths-ignore

Name of action: tj-actions/changed-files

URL of action: https://github.com/tj-actions/changed-files

Version to pin to (hash only): 24d32ff

Permissions

Action can work with read only access

Related Actions

There's a similar one https://github.com/Ana06/get-changed-files already in the list, but I believe it's not maintained

Checklist

You should be able to check most of these boxes for an action to be considered for review.
Please check all boxes that currently apply:

@madrob
Copy link

madrob commented Sep 29, 2025

This is the one that had the big supply chain incident, right? https://www.cve.org/CVERecord?id=CVE-2025-30066

@blaginin
Copy link
Author

Yes! I pinned a patched version though.

I was also wondering if this should be a reason not to use it - but to me, it feels like a good sign of community engagement and repo maintenance

@raboof
Copy link
Member

raboof commented Sep 30, 2025

Indeed using pinned versions is part of what protects us against such attacks.

From https://semgrep.dev/blog/2025/popular-github-action-tj-actionschanged-files-is-compromised/ I understand this was not the first time this repo was compromised. Perhaps you could say that now that they've experienced this compromise they'll be extra careful not to let it happen again in the future - tj-actions/changed-files#2464 possibly suggests so.

@snazy
Copy link
Member

snazy commented Nov 8, 2025

If you just need the list of changed files, a simple git diff HEAD^1 should do it. Assuming that you're "just" checking out the PR without explicitly setting the PR HEAD, HEAD points to the PR merge commit. It's first parent is a commit on the PR target branch.
So, this should "just work" and yield the list of files changed by the PR:

git --no-pager diff --name-only HEAD^1

# or put it in a bash array
IFS=$'\n' files_array=($(git --no-pager diff --name-only HEAD^1))

There is also a GH API call, but I'd not use that in a GH workflow, because It yields the list of changed files of the current state of the PR, not at the state of the PR against which a workflow run started (potential race when the PR has changed in the meantime or re-running an older workflow run).

gh pr view "${{ github.event.pull_request.number }}" --json files --jq '.files.[].path'

@blaginin
Copy link
Author

I can do that in bash - true! But I don't think it's that simple: for example besides the PR changes you mentioned, I also need to check changed files for pushes, and then handle the exclusions. The code ends up being more complicated than running the tests themselves 😅 Not sure if it's very committer-friendly or easy to follow?

@blaginin
Copy link
Author

By the way, I've also noticed we have dorny/paths-filter available but I can't use it either due to dorny/paths-filter#279

@snazy
Copy link
Member

snazy commented Nov 12, 2025

I'm just curious how things work ;)
I think, the double-for-loop can be replaced with this, which checks tests all changed files against a list of glob expressions.

shopt extglob
if [[ ${changed_files[@]} == @(*.md|docs/*|.github/ISSUE_TEMPLATE/*|.github/pull_request_template.md) ]]; then
  echo yes
else
  echo no
fi

But yea, without an action, it's bash-magic.

@blaginin
Copy link
Author

yes, sure! would you be ok with merging this pr or do you want me to use bash? i d prefer PR but dont mind writing bash hehe 😊

@snazy
Copy link
Member

snazy commented Nov 12, 2025

I can't (not an infra guy) :)

@raboof
Copy link
Member

raboof commented Nov 13, 2025

if you're comfortable vouching for this action then an approval is welcome even if you're no committer on this repo

Copy link
Member

@snazy snazy left a comment

Choose a reason for hiding this comment

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

I gave it a try.
I'm not a typescript guru, but I couldn't find anything in the action's code base that looks suspicious - like "interesting" exec or GH API calls or the like.
The workflows they have look good to me, nothing obvious that looks like a way to inject malicious code into the build or the like. But I haven't looked into the actions they depend on.
Tests look reasonable, with a reasonable test (case) coverage.
The project itself has quite a variety of contributors and seems overall quite mature.
TL;DR haven't found any "red flags"

@blaginin
Copy link
Author

@raboof is there anything else you'd like me to do in this PR? 🙂

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.

4 participants