Skip to content

Commit 22d9bd9

Browse files
hanouticelinahanouticelina
authored
A better security-wise style bot GH Action (#2914)
* better security-wise gh action * run workflow manually * nit * fix * Add credits comment * make pr_number required * remove file protected checking * fix * add timestamp, remove manual trigger and allow write access * checkout PR branch again * review suggestions
1 parent cc0c71c commit 22d9bd9

File tree

2 files changed

+84
-43
lines changed

2 files changed

+84
-43
lines changed

‎.github/workflows/style-bot-action.yml‎

Lines changed: 82 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,20 @@
1+
# This workflow was originally implemented by the diffusers team and ported into huggingface_hub
2+
# as a reusable workflow. Related PRs:
3+
# - https://github.com/huggingface/diffusers/pull/10274
4+
# - https://github.com/huggingface/diffusers/pull/10931
5+
# - https://github.com/huggingface/diffusers/pull/10908
6+
7+
18
name: Style Bot Action
29

310
on:
411
workflow_call:
512
inputs:
6-
pre_commit_script:
7-
required: false
8-
type: string
9-
description: "Optional script to run before committing changes"
10-
pre_commit_script_name:
13+
style_command_type:
1114
required: false
1215
type: string
13-
description: "Custom name for the pre-commit script step"
14-
default: "Custom pre-commit script"
16+
description: "Which style command to run (options: 'default' (make style && make quality), 'quality_only', 'style_only')"
17+
default: "default"
1518
python_quality_dependencies:
1619
required: true
1720
type: string
@@ -21,11 +24,6 @@ on:
2124
type: string
2225
description: "Python version to run code formatter"
2326
default: "3.10"
24-
style_command:
25-
required: false
26-
type: string
27-
description: "Command to run for style checks or/and style fixes"
28-
default: "make style && make quality"
2927
secrets:
3028
bot_token:
3129
required: true
@@ -34,8 +32,9 @@ on:
3432
jobs:
3533
check-permissions:
3634
if: >
37-
contains(github.event.comment.body, '@bot /style') &&
38-
github.event.issue.pull_request != null
35+
(github.event_name == 'issue_comment' &&
36+
contains(github.event.comment.body, '@bot /style') &&
37+
github.event.issue.pull_request != null)
3938
runs-on: ubuntu-latest
4039
outputs:
4140
is_authorized: ${{ steps.check_user_permission.outputs.has_permission }}
@@ -45,14 +44,16 @@ jobs:
4544
uses: actions/github-script@v6
4645
with:
4746
script: |
48-
const comment_user = context.payload.comment.user.login;
47+
48+
comment_user = context.payload.comment.user.login;
4949
const { data: permission } = await github.rest.repos.getCollaboratorPermissionLevel({
5050
owner: context.repo.owner,
5151
repo: context.repo.repo,
5252
username: comment_user
5353
});
54-
const authorized = permission.permission === 'admin';
55-
console.log(`User ${comment_user} has permission level: ${permission.permission}, authorized: ${authorized} (only admins allowed)`);
54+
55+
const authorized = ['admin', 'maintain', 'push'].includes(permission.permission);
56+
console.log(`User ${comment_user} has permission level: ${permission.permission}, authorized: ${authorized} (only users with at least write access are allowed to run this action)`);
5657
core.setOutput('has_permission', authorized);
5758
5859
run-style-bot:
@@ -66,18 +67,27 @@ jobs:
6667
with:
6768
script: |
6869
const prNumber = context.payload.issue.number;
70+
console.log(`PR number from env: "${prNumber}"`);
71+
6972
const { data: pr } = await github.rest.pulls.get({
7073
owner: context.repo.owner,
7174
repo: context.repo.repo,
7275
pull_number: prNumber
7376
});
7477
75-
// We capture both the branch ref and the "full_name" of the head repo
76-
// so that we can check out the correct repository & branch (including forks).
77-
core.setOutput("prNumber", prNumber);
78-
core.setOutput("headRef", pr.head.ref);
79-
core.setOutput("headRepoFullName", pr.head.repo.full_name);
80-
78+
// Set outputs for use in subsequent steps
79+
core.setOutput('headRepoFullName', pr.head.repo.full_name);
80+
core.setOutput('headRef', pr.head.ref);
81+
core.setOutput('baseRef', pr.base.ref);
82+
core.setOutput('prNumber', prNumber);
83+
84+
console.log('PR Details:', {
85+
number: prNumber,
86+
headRepo: pr.head.repo.full_name,
87+
headRef: pr.head.ref,
88+
baseRef: pr.base.ref
89+
});
90+
8191
- name: Check out PR branch
8292
uses: actions/checkout@v3
8393
env:
@@ -90,16 +100,37 @@ jobs:
90100
# You may need fetch-depth: 0 for being able to push
91101
fetch-depth: 0
92102
token: ${{ secrets.bot_token }}
93-
94-
- name: Debug
95-
env:
96-
HEADREPOFULLNAME: ${{ steps.pr_info.outputs.headRepoFullName }}
103+
104+
- name: Check commit timestamps
105+
env:
106+
COMMENT_DATE: ${{ github.event.comment.created_at }}
107+
PR_NUMBER: ${{ steps.pr_info.outputs.prNumber }}
108+
BASE_REPO_URL: https://github.com/${{ github.repository }}
97109
HEADREF: ${{ steps.pr_info.outputs.headRef }}
98-
PRNUMBER: ${{ steps.pr_info.outputs.prNumber }}
99110
run: |
100-
echo "PR number: $PRNUMBER"
101-
echo "Head Ref: $HEADREF"
102-
echo "Head Repo Full Name: $HEADREPOFULLNAME"
111+
echo "--- Checking remotes ---"
112+
git remote -v
113+
echo "--- Checking ref on origin (${{ steps.pr_info.outputs.headRepoFullName }}) ---"
114+
git ls-remote origin refs/pull/$PR_NUMBER/merge || echo "Ref not found on origin (fork)."
115+
echo "--- Checking ref on base (${{ github.repository }}) ---"
116+
git ls-remote $BASE_REPO_URL refs/pull/$PR_NUMBER/merge || echo "Ref not found on base repository."
117+
118+
echo "--- Proceeding with fetch from base repository ---"
119+
git fetch $BASE_REPO_URL refs/pull/$PR_NUMBER/merge:refs/remotes/pull/$PR_NUMBER/merge
120+
git checkout refs/remotes/pull/$PR_NUMBER/merge
121+
echo "PR_MERGE_SHA: $(git log -1 --format=%H)"
122+
PR_MERGE_COMMIT_TIMESTAMP=$(git log -1 --date=unix --format=%cd)
123+
echo "PR_MERGE_COMMIT_TIMESTAMP: $PR_MERGE_COMMIT_TIMESTAMP"
124+
COMMENT_TIMESTAMP=$(date -d "${COMMENT_DATE}" +"%s")
125+
echo "COMMENT_DATE: $COMMENT_DATE"
126+
echo "COMMENT_TIMESTAMP: $COMMENT_TIMESTAMP"
127+
if [ $COMMENT_TIMESTAMP -le $PR_MERGE_COMMIT_TIMESTAMP ]; then
128+
echo "❌ Last commit on the pull request is newer than the issue comment triggering this run! Abort!";
129+
exit -1;
130+
fi
131+
132+
echo "--- Checking out contributor branch ($HEADREF) ---"
133+
git checkout $HEADREF
103134
104135
- name: Set up Python
105136
uses: actions/setup-python@v4
@@ -113,18 +144,28 @@ jobs:
113144
python -m pip install --upgrade pip
114145
pip install .$python_quality_dependencies
115146
116-
- name: ${{ inputs.pre_commit_script_name }}
117-
env:
118-
pre_commit_script: ${{ inputs.pre_commit_script }}
119-
if: inputs.pre_commit_script != ''
120-
run: |
121-
bash -c "${pre_commit_script}"
122-
123147
- name: Run style command
124-
env:
125-
style_command: ${{ inputs.style_command }}
148+
id: run_style
126149
run: |
127-
bash -c "$style_command"
150+
case "${{ inputs.style_command_type }}" in
151+
"default")
152+
echo "Running default style and quality checks"
153+
make style && make quality
154+
;;
155+
"quality_only")
156+
echo "Running quality checks only"
157+
make quality
158+
;;
159+
"style_only")
160+
echo "Running style checks only"
161+
make style
162+
;;
163+
*)
164+
echo "Invalid style_command_type: ${{ inputs.style_command_type }}"
165+
echo "Valid options are: 'default', 'quality_only', 'style_only'"
166+
exit 1
167+
;;
168+
esac
128169
129170
- name: Commit and push changes
130171
id: commit_and_push

‎.github/workflows/style-bot.yml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ name: Style Bot
33
on:
44
issue_comment:
55
types: [created]
6-
6+
77
permissions:
88
contents: write
99
pull-requests: write
@@ -13,6 +13,6 @@ jobs:
1313
uses: ./.github/workflows/style-bot-action.yml
1414
with:
1515
python_quality_dependencies: "[quality]"
16-
style_command: "make style"
16+
style_command_type: "style_only"
1717
secrets:
1818
bot_token: ${{ secrets.GITHUB_TOKEN }}

0 commit comments

Comments
 (0)