Avoid warning on workflow_call triggers #2274
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
aeisenberg
force-pushed
the
aeisenberg/no-warn-workflow_call
branch
from
6527f17
to
9a8503f
Compare
src/workflow.ts
Outdated
| // Avoid warning when there this action was triggered via workflow_call since | ||
| // the user has a custom workflow that is calling this action and we assume | ||
| // they know what they are doing. | ||
| const isWorkflowCall = github.context.eventName === "workflow_call"; |
There was a problem hiding this comment.
I'm not certain this will work. If you're in a callee workflow, we only see the name of the event that started the caller workflow. I think you'll have to use other heuristics to parse the current workflow (which will be the caller) to recognise that it contains a workflow call, or simply doesn't contain a github/codeql-action step (which indicates that step is in the callee).
There was a problem hiding this comment.
I think Aditya is right, and we want to look for a workflow_call trigger, i.e. workflow_call appearing in the on property (note that this property may be a string or an array).
There was a problem hiding this comment.
We just need to look for the presence of the property, IIUC. The value can also be null. Eg, this is legal:
on:
workflow_call:
There was a problem hiding this comment.
How does on: workflow_call get parsed? I assumed it was a string.
There was a problem hiding this comment.
It is parsed as null:
There was a problem hiding this comment.
Ah...I see you were saying something different. We need to handle the following formats as well:
on: workflow_call
on:
- workflow_call
9a8503f
to
43d7704
Compare
aeisenberg
force-pushed
the
aeisenberg/no-warn-workflow_call
branch
from
43d7704
to
ee0b233
Compare
|
I implemented the suggested changes, made a refactoring to make things a bit easier to understand, and added a handful of tests to verify. |
Typically, we warn when there is no `push` trigger in the workflow file that triggered this run. However, when this action is triggered by a `workflow_call` event, we assume there is a custom process for triggering the action and we don't want to warn in this case.
aeisenberg
force-pushed
the
aeisenberg/no-warn-workflow_call
branch
from
ee0b233
to
ca7f194
Compare
| return doc.on.includes(triggerName); | ||
| } | ||
|
|
||
| return Object.prototype.hasOwnProperty.call(doc.on, triggerName); |
There was a problem hiding this comment.
I think we can simplify this:
| return Object.prototype.hasOwnProperty.call(doc.on, triggerName); | |
| return Object.hasOwn(doc.on, triggerName); |
There was a problem hiding this comment.
Nice. I didn't know about this function.
It's only available on node 16.9 and later. Are we sure that all supported GHES runners use that version or later?
Also, we need to update our tsconfig, or else we ge tlinter errors. So, I'm going to hold off for now and we can make the tsconfig changes later.
Typically, we warn when there is no
pushtrigger in the workflow file that triggered this run. However, when this action is triggered by aworkflow_callevent, we assume there is a custom process for triggering the action and we don't want to warn in this case.Merge / deployment checklist