-
Notifications
You must be signed in to change notification settings - Fork 309
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Favor GITHUB_WORKFLOW_REF
over using actions: read
to calculate the workflow path
#2126
base: main
Are you sure you want to change the base?
Conversation
I can confirm that this fixes my issue from #2117 without the need to declare diff --git a/.github/workflows/local-cd.yml b/.github/workflows/local-cd.yml
index c6bf452..ee11bac 100644
--- a/.github/workflows/local-cd.yml
+++ b/.github/workflows/local-cd.yml
@@ -236,7 +236,6 @@ jobs:
runs-on: ubuntu-latest
needs: build-and-publish
permissions:
- actions: read
contents: read
packages: read
pull-requests: write
@@ -282,7 +281,8 @@ jobs:
id: upload-sarif
continue-on-error: true
if: hashFiles('sarif.output.json') != '' && github.event_name != 'pull_request_target'
- uses: github/codeql-action/upload-sarif@main
+ uses: jsoref/github-codeql-action/upload-sarif@favor-workflow-ref
with:
sarif_file: sarif.output.json |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me ✨ just a few documentation suggestions, and the changelog merge conflict will need to be addressed of course!
f8bd51a
to
b66391a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had been planning to test something off of your forkmyself, but I haven't had time to in the last few days so I thought I'd ask if you were willing to test it and link the runs after testing 😄
We'd like to make sure that for reusable workflows, the workflow path we originally got from the API is definitely the same as the one we're getting from GITHUB_WORKFLOW_REF
. Basically, if they're not the same, alert categories will change after this change. They should be the same, but we'd like to be certain 😸
@@ -119,6 +119,18 @@ | |||
* Get the path of the currently executing workflow relative to the repository root. | |||
*/ | |||
export async function getWorkflowRelativePath(): Promise<string> { | |||
const workflow_ref = process.env["GITHUB_WORKFLOW_REF"]; |
Check warning
Code scanning / CodeQL
Some environment variables may not exist in default setup workflows
1bd245e
to
4280b0e
Compare
@angelapwen: um, if someone gave me a workflow, I'm open to running it. I haven't managed to get reusable workflows to do anything particularly useful for me, so I don't generally use them myself (I think there are some reusable workflows somewhere in one of the codeql repositories) -- mostly they've made me dizzy (I tried to figure out a way to set up a reusable workflow for check-spelling, and it didn't seem to add any value). I have very little time tomorrow and won't have time until Tuesday (Monday's a holiday in Canada). I expect to be moderately busy next week, but I can generally run commands if people provide them. (Sorry, I have no idea how/why I missed your event this morning...) |
Introduced with GHES 3.9: https://docs.github.com/en/enterprise-server@3.9/actions/learn-github-actions/variables GITHUB_WORKFLOW_REF means that actions don't need to use `actions: read` to determine the path to the running workflow.
4280b0e
to
abe17b5
Compare
Introduced with GHES 3.9:
https://docs.github.com/en/enterprise-server@3.9/actions/learn-github-actions/variables
GITHUB_WORKFLOW_REF
means that actions don't need to useactions: read
to determine the path to the running workflow.This should address the problem in #2117 as long as the user is running on GHES 3.9+ (including GHEC).
It doesn't precisely fix the issue because someone running on GHES 3.8 will still need to add
actions: read
until #2121 makes that code path non fatal.Note that these aren't mutually exclusive PRs (although as they both impact the changelog, they will conflict there...) -- both changes should be applied -- this simplifies the general logic for the average user (and once GHES 3.8 is sunset, the other code path should be removed entirely).
Merge / deployment checklist