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

Calling script instead of using hardcoded value #33817

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -11,79 +11,21 @@ periodics:
decorate: true
extra_refs:
- org: kubernetes
repo: kubernetes
base_ref: master
path_alias: k8s.io/kubernetes
repo: sig-security
Copy link
Member

Choose a reason for hiding this comment

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

/hold
I trust you all in the abstract, but it's really problematic to have to keep an eye on more sources of arbitrary code in this singular trusted cluster, let's revisit that before making it harder to track down what exactly is running in the trusted cluster. As a general rule we have only allowed:

  • launching cloudbuilds with images/builder (which then run in the isolated per-subproject GCP staging project)
  • a handful of CI components implemented as jobs

Which are implemented in this repo with a limited approver set.

base_ref: main
workdir: true
spec:
serviceAccountName: k8s-snyk-scan
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to have a service account token at all?

If not, maybe let's set automountServiceAccountToken: false instead?

Copy link
Member

Choose a reason for hiding this comment

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

@dims you've got much more prow job experience than I do, WDYT?

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

I have opened #33970 as a reminder to remove this service account token once we know the job is running properly from its external script call

containers:
- image: golang
envFrom:
- secretRef:
# secret key should be defined as SNYK_TOKEN
name: snyk-token
Copy link
Member

Choose a reason for hiding this comment

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

... we should really mount this as a file and pass it into snyk that way instead of an env

Do we really need this to run in the trusted cluster anyhow though? It doesn't seem like this account is all that sensitive, but the other things in that cluster are and I'd rather have less arbitrary code running there.

(Alternatively, we could run this in a cloudbuild like the image pushing jobs and sandbox it that way)

command:
- /bin/bash
args:
- -c
- |
set -euo pipefail
apt update && apt -y install jq
wget -q -O /usr/local/bin/snyk https://static.snyk.io/cli/latest/snyk-linux && chmod +x /usr/local/bin/snyk
mkdir -p "${ARTIFACTS}"
if [ -z "${SNYK_TOKEN}" ]; then
echo "SNYK_TOKEN env var is not set, required for snyk scan"
exit 1
fi
echo "Running snyk scan .."
EXIT_CODE=0
RESULT_UNFILTERED=$(snyk test -d --json) || EXIT_CODE=$?
if [ $EXIT_CODE -gt 1 ]; then
echo "Failed to run snyk scan with exit code $EXIT_CODE "
exit 1
fi
RESULT=$(echo $RESULT_UNFILTERED | jq \
'{vulnerabilities: .vulnerabilities | map(select((.type != "license") and (.version != "0.0.0"))) | select(length > 0) }')
if [[ ${RESULT} ]]; then
CVE_IDs=$(echo $RESULT | jq '.vulnerabilities[].identifiers.CVE | unique[]' | sort -u)
#convert string to array
CVE_IDs_array=(`echo ${CVE_IDs}`)
#TODO:Implement deduplication of CVE IDs in future
for i in "${CVE_IDs_array[@]}"
do
if [[ "$i" == *"CVE"* ]]; then
#Look for presence of GitHub Issues for detected CVEs. If no issues are present, this CVE needs triage
#Once the job fails, CVE is triaged by SIG Security and a tracking issue is created.
#This will allow in the next run for the job to pass again
TOTAL_COUNT=$(curl -H "Accept: application/vnd.github.v3+json" "https://api.github.com/search/issues?q=repo:kubernetes/kubernetes+${i}" | jq .total_count)
if [[ $TOTAL_COUNT -eq 0 ]]; then
echo "Vulnerability filtering failed"
exit 1
fi
fi
done
fi
echo "Build time dependency scan completed"

# container images scan
echo "Fetch the list of k8s images"
curl -Ls https://sbom.k8s.io/$(curl -Ls https://dl.k8s.io/release/latest.txt)/release | grep "SPDXID: SPDXRef-Package-registry.k8s.io" | grep -v sha256 | cut -d- -f3- | sed 's/-/\//' | sed 's/-v1/:v1/' > images
while read image; do
echo "Running container image scan.."
EXIT_CODE=0
RESULT_UNFILTERED=$(snyk container test $image -d --json) || EXIT_CODE=$?
if [ $EXIT_CODE -gt 1 ]; then
echo "Failed to run snyk scan with exit code $EXIT_CODE . Error message: $RESULT_UNFILTERED"
exit 1
fi
RESULT=$(echo $RESULT_UNFILTERED | jq \
'{vulnerabilities: .vulnerabilities | map(select(.isUpgradable == true or .isPatchable == true)) | select(length > 0) }')
if [[ ${RESULT} ]]; then
echo "Vulnerability filtering failed"
# exit 1 (To allow other images to be scanned even if one fails)
else
echo "Scan completed image $image"
fi
done < images
- sh
- "-c"
- "cd sig-security-tooling/scanning/ && ./build-deps-and-release-images.sh"
annotations:
testgrid-create-test-group: "true"
testgrid-alert-email: [email protected]
Expand Down