GitHub Workflows security hardening

[sashashura]

This PR adds explicit permissions section to workflows. This is a security best practice because by default workflows run with extended set of permissions (except from on: pull_request from external forks). By specifying any permission explicitly all others are set to none. By using the principle of least privilege the damage a compromised workflow can do (because of an injection or compromised third party tool or action) is restricted. It is recommended to have most strict permissions on the top level and grant write permissions on job level case by case.

[sashashura]

Is anything else needed?

[andrewscfc]

Is anything else needed?

hey @sashashura apologies for the delayed response, unfortunately the change appears to have broken the CI build causing two of our checks to fail. I think it's because a custom webpack plugin we have created outputs some code as part of the build: https://github.com/bbc/simorgh/blob/latest/webpack.config.js#L68, I imagine this might require some write permissions.

Indeed I imagine building the app itself might need this to write the app build output to the build directory.

Thanks for your contribution, if you wish to take it any further you might want to look into allowing some scoped write permissions 🙂

[sashashura]

Do you mean the lint step in https://github.com/bbc/simorgh/actions/runs/3099439609/jobs/5018581826? Could you please rerun the workflow? I think it is not related to my changes, because it was triggered on pull_request and it has contents read and metadata read only. So I couldn't break it.

[sashashura]

@andrewscfc Could you please try re-running it?

[sashashura]

I get the error even if I don't touch permissions at all. Only modified the if condition to skip Chromatic UI in my fork's default branch - that it, and got the same error. Any ideas why?

[andrewscfc]

I get the error even if I don't touch permissions at all. Only modified the if condition to skip Chromatic UI in my fork's default branch - that it, and got the same error. Any ideas why?

hey, I've replicated this issue on a fork from my own account: https://github.com/bbc/simorgh/pull/10360.

I started looking into it and I suspect it is caused by webpack aliases not resolving for eslint on forks:

• https://github.com/andrewscfc/simorgh/blob/latest/.eslintrc.js#L51
• https://github.com/andrewscfc/simorgh/blob/latest/dirAlias.js#L36

I'll try and look into this over the next few weeks or please take a look yourself. It might need similar code to the resolvePath here but haven't had time to dig any deeper.

[sashashura]

Thanks for reproducing and looking into it. I don't understand how to fix this alias resolution, so I'll wait. Take your time and thank you!