Skip to content
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

GitHub Workflows security hardening #4587

Merged
merged 2 commits into from
Dec 1, 2022
Merged

Conversation

sashashura
Copy link
Contributor

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 sashashura requested review from a team and reta as code owners September 25, 2022 21:05
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@codecov-commenter
Copy link

codecov-commenter commented Sep 25, 2022

Codecov Report

Merging #4587 (0734d2b) into main (4616dfa) will increase coverage by 0.00%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##               main    #4587   +/-   ##
=========================================
  Coverage     70.98%   70.98%           
+ Complexity    58185    58169   -16     
=========================================
  Files          4711     4711           
  Lines        277573   277573           
  Branches      40180    40180           
=========================================
+ Hits         197031   197040    +9     
- Misses        64386    64392    +6     
+ Partials      16156    16141   -15     
Impacted Files Coverage Δ
...n/indices/forcemerge/ForceMergeRequestBuilder.java 0.00% <0.00%> (-75.00%) ⬇️
.../indices/forcemerge/TransportForceMergeAction.java 25.00% <0.00%> (-75.00%) ⬇️
...ava/org/opensearch/action/NoSuchNodeException.java 0.00% <0.00%> (-50.00%) ⬇️
.../admin/cluster/reroute/ClusterRerouteResponse.java 60.00% <0.00%> (-40.00%) ⬇️
...luster/routing/allocation/RoutingExplanations.java 62.06% <0.00%> (-37.94%) ⬇️
...arch/search/aggregations/pipeline/LinearModel.java 23.07% <0.00%> (-34.62%) ⬇️
...nsearch/index/shard/IndexShardClosedException.java 66.66% <0.00%> (-33.34%) ⬇️
...on/admin/indices/forcemerge/ForceMergeRequest.java 50.00% <0.00%> (-31.58%) ⬇️
...cluster/routing/allocation/RerouteExplanation.java 70.00% <0.00%> (-30.00%) ⬇️
.../java/org/opensearch/index/reindex/RemoteInfo.java 50.45% <0.00%> (-29.36%) ⬇️
... and 453 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@saratvemulapalli saratvemulapalli added backport 2.x Backport to 2.x branch v3.0.0 Issues and PRs related to version 3.0.0 CI CI related labels Sep 27, 2022
jobs:
build:
permissions:
contents: write # to create branch (peter-evans/create-pull-request)
Copy link
Collaborator

@VachaShah VachaShah Sep 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might not need the permissions since the token used here is from the Github App Token from an app and not the default Github token.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for missing that. You are right. Fixed.

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Oct 5, 2022

Gradle Check (Jenkins) Run Completed with:

@dblock
Copy link
Member

dblock commented Oct 5, 2022

Please amend commits to fix DCO with -s

CHANGELOG.md Outdated
@@ -6,6 +6,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)

### Added

- GitHub Workflows token permission hardening ([#4587](https://github.com/opensearch-project/OpenSearch/pull/4587))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: maybe we can turn all these changelog lines into something that was actioned, e.g. Hardened token permissions in GitHub workflow?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@sashashura sashashura force-pushed the patch-1 branch 3 times, most recently from 34a40e0 to d89482d Compare October 12, 2022 09:51
Signed-off-by: sashashura <aleksandrosansan@gmail.com>
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@sashashura
Copy link
Contributor Author

I don't think the gradle check failure is related to the changes, but let me know.

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      1 org.opensearch.index.ShardIndexingPressureConcurrentExecutionTests.testReplicaThreadedUpdateToShardLimitsAndRejections
      1 org.opensearch.http.SearchRestCancellationIT.testAutomaticCancellationDuringFetchPhase

@sashashura
Copy link
Contributor Author

@reta Could you please review it?

@opensearch-trigger-bot
Copy link
Contributor

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-4587-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 d266a732aea4cf87781acf93e92c56ad3f1d0913
# Push it to GitHub
git push --set-upstream origin backport/backport-4587-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-4587-to-2.x.

mch2 pushed a commit to mch2/OpenSearch that referenced this pull request Jan 30, 2023
Signed-off-by: sashashura <aleksandrosansan@gmail.com>

Signed-off-by: sashashura <aleksandrosansan@gmail.com>
(cherry picked from commit d266a73)
@mch2
Copy link
Member

mch2 commented Jan 30, 2023

Manual backport #6097

mch2 pushed a commit to mch2/OpenSearch that referenced this pull request Jan 30, 2023
Signed-off-by: sashashura <aleksandrosansan@gmail.com>

Signed-off-by: sashashura <aleksandrosansan@gmail.com>
(cherry picked from commit d266a73)
Signed-off-by: Marc Handalian <handalm@amazon.com>
mch2 pushed a commit to mch2/OpenSearch that referenced this pull request Jan 30, 2023
Signed-off-by: sashashura <aleksandrosansan@gmail.com>

Signed-off-by: sashashura <aleksandrosansan@gmail.com>
(cherry picked from commit d266a73)
Signed-off-by: Marc Handalian <handalm@amazon.com>
@mch2 mch2 mentioned this pull request Jan 30, 2023
6 tasks
andrross pushed a commit that referenced this pull request Jan 30, 2023
(cherry picked from commit d266a73)

Signed-off-by: sashashura <aleksandrosansan@gmail.com>
Signed-off-by: Marc Handalian <handalm@amazon.com>
Co-authored-by: Alex <aleksandrosansan@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x Backport to 2.x branch CI CI related v3.0.0 Issues and PRs related to version 3.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants