metasploit-framework icon indicating copy to clipboard operation
metasploit-framework copied to clipboard

Bump actions/github-script to v6

Open gliptak opened this issue 3 years ago • 9 comments
trafficstars

Signed-off-by: Gábor Lipták [email protected]

Tell us what this change does. If you're fixing a bug, please mention the github issue number.

Please ensure you are submitting from a unique branch in your repository to master in Rapid7's.

Verification

List the steps needed to make sure this thing works

  • [ ] Start msfconsole
  • [ ] use exploit/windows/smb/ms08_067_netapi
  • [ ] ...
  • [ ] Verify the thing does what it should
  • [ ] Verify the thing does not do what it should not
  • [ ] Document the thing and how it works (Example)

If you are opening a PR for a new module that exploits a specific piece of hardware or requires a complex or hard-to-find testing environment, we recommend that you send us a demo of your module executing correctly. Seeing your module in action will help us review your PR faster!

Specific Hardware Examples:

  • Switches
  • Routers
  • IP Cameras
  • IoT devices

Complex Software Examples:

  • Expensive proprietary software
  • Software with an extensive installation process
  • Software that requires exploit testing across multiple significantly different versions
  • Software without an English language UI

We will also accept demonstrations of successful module execution even if your module doesn't meet the above conditions. It's not a necessity, but it may help us land your module faster!

Demonstration of successful module execution can take the form of a packet capture (pcap) or a screen recording. You can send pcaps and recordings to [email protected]. Please include a CVE number in the subject header (if applicable), and a link to your PR in the email body. If you wish to sanitize your pcap, please see the wiki.

gliptak avatar Oct 01 '22 00:10 gliptak

@bwatters-r7 @jheysel-r7 please approve workflows

gliptak avatar Oct 03 '22 22:10 gliptak

@gliptak I don't know what this does exactly, or why it is needed. I'm generally totally down for updating releases, but I'd want someone more familiar with our build process to weigh in on this change. I'm sure it will come up on the week's PR reviews, so I'd expect feedback and movement by the end of the week. Thanks!

bwatters-r7 avatar Oct 03 '22 23:10 bwatters-r7

Hi @gliptak - it looks like this may need additional code changes from what I can see: https://github.com/actions/github-script/tree/7dff1a87643417cf3b95bb10b29f4c4bc60d8ebd#breaking-changes

Were you able to verify/test this upgrade beforehand? 👀

adfoster-r7 avatar Oct 04 '22 16:10 adfoster-r7

@adfoster-r7 the PR is green on my fork https://github.com/gliptak/metasploit-framework/pull/4/checks

gliptak avatar Oct 04 '22 23:10 gliptak

I believe it's green as the codepath isn't triggered with the required event, which only occurs when specific labels are added to issues/pull requests:

on:
  pull_request_target:
    types: [labeled]
  issues:
    types: [labeled]

From the change log https://github.com/actions/github-script/tree/7dff1a87643417cf3b95bb10b29f4c4bc60d8ebd#breaking-changes it looks like there is API changes:

Breaking changes in V5

[...] For example, github.issues.createComment in V4 becomes github.rest.issues.createComment in V5

Which looks like the pattern we're using over here:

https://github.com/rapid7/metasploit-framework/blob/a73461e96b7c34cc13e5b8c2bd2f3bd85e86df74/.github/workflows/labels.yml#L203-L220

adfoster-r7 avatar Oct 04 '22 23:10 adfoster-r7

thank you for all the pointers @adfoster-r7

updated results https://github.com/gliptak/metasploit-framework/actions/runs/3186246986/jobs/5196651945

gliptak avatar Oct 04 '22 23:10 gliptak

any other updates for this PR?

gliptak avatar Oct 09 '22 21:10 gliptak

Thanks for pushing additional updates; I should be able to do a final verification next week :+1:

adfoster-r7 avatar Oct 11 '22 00:10 adfoster-r7

Alan I'm going to assign this issue to you in the meantime given your last comment. This should help clear up the PR queue a little bit, but let me know if this is an issue.

gwillcox-r7 avatar Oct 11 '22 04:10 gwillcox-r7

Thanks! :+1:

adfoster-r7 avatar Oct 20 '22 09:10 adfoster-r7