ci: Ensure testing of proposed code in pull requests from forks

ImagingDataCommons / idc-index-data

Python package providing the index to query and download data hosted by the NCI Imaging Data Commons

MIT License

1 stars 4 forks source link

ci: Ensure testing of proposed code in pull requests from forks #8

Closed jcfr closed 8 months ago

jcfr commented 8 months ago

This commit reverts the changes introduced in commit 4aa6655b9, which modified the CI workflow to use pull_request_target for accessing repository secrets during pull request testing.

Originally, the intention was to grant access to repository secrets within the pull request environment. However, using pull_request_target caused the workflow to run against the base branch instead of the proposed changes in the pull request.

To maintain secure code review practices and avoid insecure workarounds, this commit restores the previous workflow behavior. A subsequent commit will address this issue by updating the workflow to detect secret unavailability and prompt developers to request write access to the project instead.

jcfr commented 8 months ago

Related pull request:

https://github.com/ImagingDataCommons/idc-index-data/pull/7

fedorov commented 8 months ago

@jcfr sorry, I missed this earlier. I misunderstood the guidance, based on my incomplete read of the documentation ...

A subsequent commit will address this issue by updating the workflow to detect secret unavailability and prompt developers to request write access to the project instead.

How difficult is this to do? Can you point us to the relevant resources, if you do not have time to work on this?

fedorov commented 8 months ago

A subsequent commit

Did you mean to say "a subsequent PR"? Should this be merged first?

jcfr commented 8 months ago

A subsequent commit will address this issue by updating the workflow to detect secret unavailability and prompt developers to request write access to the project instead.

This will indeed done in a later commit added to https://github.com/ImagingDataCommons/idc-index-data/pull/5

In the meantime, I suggest you:

integrate this pull request so that we can rebase https://github.com/ImagingDataCommons/idc-index-data/pull/5
grant @vkt1414 write access (either as a collaborator or by granting access to the relevant team of the ImagingDataCommons organization)

fedorov commented 8 months ago

grant @vkt1414 write access

This is done now. I somehow did not think to check this on Friday, just assuming that Vamsi has the permissions. We will test this now.