Skip to content

Commit

Permalink
Add a security notice file to top-level .github directory
Browse files Browse the repository at this point in the history
  • Loading branch information
jonathan-innis committed Dec 12, 2023
1 parent 275df60 commit 1d1ce96
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 19 deletions.
2 changes: 1 addition & 1 deletion .github/actions/e2e/setup-cluster/action.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ runs:
env:
CLUSTER_NAME: ${{ inputs.cluster_name }}
run: |
stack_names=$(aws cloudformation describe-stacks --query 'Stacks[?Tags[?Key == `karpenter.sh/discovery` && Value == `$CLUSTER_NAME`]].{StackName: StackName}' --output text)
stack_names=$(aws cloudformation describe-stacks --query "Stacks[?Tags[?Key == 'karpenter.sh/discovery' && Value == '$CLUSTER_NAME']].{StackName: StackName}" --output text)
for stack_name in $stack_names; do
echo "Stack Events for $stack_name:"
aws cloudformation describe-stack-events --stack-name $stack_name
Expand Down
22 changes: 22 additions & 0 deletions .github/security-notice.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# Github Workflows Security Notice

Writing security workflows that can be accessed by third parties outside of your repository is inherently dangerous. There is a full list of vulnerabilities that you can subject yourself to when you enable external users to interact with your workflows. These vulnerabilities are well-described here: https://docs.github.com/en/actions/security-guides/security-hardening-for-github-actions as well as detail on how to mitigate these risks.

As a rule-of-thumb within the Karpenter workflows, we have chosen to always assign any input that _might_ come from a user in either a Github workflow or a composite action into environment variables any we are using a bash or javascript script as a step in the workflow or action. An example of this can be seen below:

```yaml
- name: Save info about the review comment as an artifact for other workflows that run on workflow_run to download them
env:
# We store these values in environment variables to avoid bash script injection
# Specifically, it's important that we do this for github.event.review.body since this is user-controller input
# https://docs.github.com/en/actions/security-guides/security-hardening-for-github-actions
REVIEW_BODY: ${{ github.event.review.body }}
PULL_REQUEST_NUMBER: ${{ github.event.pull_request.number }}
COMMIT_ID: ${{ github.event.review.commit_id }}
run: |
mkdir -p /tmp/artifacts
{ echo "$REVIEW_BODY"; echo "$PULL_REQUEST_NUMBER"; echo "$COMMIT_ID"; } >> /tmp/artifacts/metadata.txt
cat /tmp/artifacts/metadata.txt
```
Note that, when you are writing Github workflows or composite actions to ensure to follow this code-style to reduce the attack surface could result from attempted script injection to the workflows.
3 changes: 0 additions & 3 deletions .github/workflows/approval-comment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,6 @@ jobs:
fetch-depth: 0
- name: Save info about the review comment as an artifact for other workflows that run on workflow_run to download them
env:
# We store these values in environment variables to avoid bash script injection
# Specifically, it's important that we do this for github.event.review.body since this is user-controller input
# https://docs.github.com/en/actions/security-guides/security-hardening-for-github-actions
REVIEW_BODY: ${{ github.event.review.body }}
PULL_REQUEST_NUMBER: ${{ github.event.pull_request.number }}
COMMIT_ID: ${{ github.event.review.commit_id }}
Expand Down
21 changes: 6 additions & 15 deletions .github/workflows/e2e.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -105,21 +105,17 @@ jobs:
name: generate cluster name
env:
SUITE: ${{ inputs.suite }}
CLUSTER_NAME: ${{ inputs.cluster_name }}
WORKFLOW_TRIGGER: ${{ inputs.workflow_trigger }}
run: |
<<<<<<< HEAD
CLUSTER_NAME='${{ inputs.cluster_name }}'
if [[ '${{ inputs.cluster_name }}' == '' ]]; then
if [[ '${{ inputs.workflow_trigger }}' == 'soak' ]]; then
CLUSTER_NAME=$(echo soak-periodic-$RANDOM$RANDOM | awk '{print tolower($0)}' | tr / -)
if [[ "$CLUSTER_NAME" == '' ]]; then
if [[ "$WORKFLOW_TRIGGER" == 'soak' ]]; then
CLUSTER_NAME=$(echo "soak-periodic-$RANDOM$RANDOM" | awk '{print tolower($0)}' | tr / -)
else
CLUSTER_NAME=$(echo ${{ inputs.suite }}-$RANDOM$RANDOM | awk '{print tolower($0)}' | tr / -)
CLUSTER_NAME=$(echo "$SUITE-$RANDOM$RANDOM" | awk '{print tolower($0)}' | tr / -)
fi
fi
echo "Using cluster name \"$CLUSTER_NAME\""
=======
CLUSTER_NAME="$(echo "$SUITE-$RANDOM$RANDOM" | awk '{print tolower($0)}' | tr / -)"
echo Using cluster name "$CLUSTER_NAME"
>>>>>>> 493916ee (Move github context inputs to environment variables)
echo CLUSTER_NAME="$CLUSTER_NAME" >> "$GITHUB_OUTPUT"
- name: setup eks cluster '${{ steps.generate-cluster-name.outputs.CLUSTER_NAME }}'
if: inputs.cluster_name == ''
Expand Down Expand Up @@ -148,17 +144,12 @@ jobs:
SUITE="Integration"
fi
aws eks update-kubeconfig --name ${{ steps.generate-cluster-name.outputs.CLUSTER_NAME }}
<<<<<<< HEAD
# Clean up the cluster before running all tests
kubectl delete nodepool --all
kubectl delete ec2nodeclass --all
kubectl delete deployment --all
# Run test Suite
TEST_SUITE="$TEST_SUITE" ENABLE_METRICS=${{ inputs.enable_metrics }} METRICS_REGION=${{ vars.TIMESTREAM_REGION }} GIT_REF="$(git rev-parse HEAD)" \
=======
TEST_SUITE="$SUITE" ENABLE_METRICS=$ENABLE_METRICS METRICS_REGION=${{ vars.TIMESTREAM_REGION }} GIT_REF="$(git rev-parse HEAD)" \
>>>>>>> 493916ee (Move github context inputs to environment variables)
CLUSTER_NAME="${{ steps.generate-cluster-name.outputs.CLUSTER_NAME }}" CLUSTER_ENDPOINT="$(aws eks describe-cluster --name ${{ steps.generate-cluster-name.outputs.CLUSTER_NAME }} --query "cluster.endpoint" --output text)" \
INTERRUPTION_QUEUE="${{ steps.generate-cluster-name.outputs.CLUSTER_NAME }}" make e2etests
- name: notify slack of success or failure
Expand Down

0 comments on commit 1d1ce96

Please sign in to comment.