Add check-source-origin Tekton task for sdist supply-chain verification - #300
Add check-source-origin Tekton task for sdist supply-chain verification#300smatula wants to merge 5 commits into
Conversation
Reviewer's GuideIntroduces a new non-blocking Tekton Task that installs and runs check-source-origin to verify Python sdists against their upstream VCS repositories, wiring in optional CA bundle and git-auth workspace credentials and emitting structured JSON test results via the TEST_OUTPUT result. Flow diagram for check-source-origin Tekton Task executionflowchart TD
start([Start check-source-origin step])
start --> cabundle_check[Check CA bundle exists]
cabundle_check --> cabundle_set[Set SSL_CERT_FILE and REQUESTS_CA_BUNDLE]
cabundle_set --> git_ws_bound{basic-auth workspace bound and .git-credentials present}
git_ws_bound -- no --> result_no_ws[Write TEST_OUTPUT: result ERROR, note git-auth workspace not bound]
result_no_ws --> end_skip_ws([Exit step])
git_ws_bound -- yes --> gh_token_extract[Extract GH_TOKEN from .git-credentials]
gh_token_extract --> gh_token_present{GH_TOKEN present}
gh_token_present -- no --> result_no_token[Write TEST_OUTPUT: result ERROR, note no github token]
result_no_token --> end_skip_token([Exit step])
gh_token_present -- yes --> git_config_copy[Copy .git-credentials and optional .gitconfig to home]
git_config_copy --> parse_fail_severity[Read FAIL_SEVERITY param]
parse_fail_severity --> packages_parse[Parse PACKAGES JSON to PACKAGE_LIST]
packages_parse --> packages_valid{PACKAGES JSON valid}
packages_valid -- no --> result_invalid_json[Write TEST_OUTPUT: result FAIL_SEVERITY, note invalid PACKAGES JSON]
result_invalid_json --> end_invalid_json([Exit step])
packages_valid -- yes --> packages_nonempty{PACKAGE_LIST non-empty}
packages_nonempty -- no --> result_no_pkgs[Write TEST_OUTPUT: result FAIL_SEVERITY, note no packages]
result_no_pkgs --> end_no_pkgs([Exit step])
packages_nonempty -- yes --> install_cso[pip install check-source-origin at CSO_GIT_REF]
install_cso --> loop_pkgs[Loop over PACKAGE_LIST, run check-source-origin verify]
loop_pkgs --> summarize[Compute SUCCESSES, FAILURES, ERRORS, WARNINGS]
summarize --> result_all_errors{ERRORS equals TOTAL}
summarize --> result_any_warn{WARNINGS greater than 0}
summarize --> result_all_success[Write TEST_OUTPUT: result SUCCESS, all packages verified]
result_all_errors -- condition met --> write_all_errors[Write TEST_OUTPUT: result FAIL_SEVERITY, note all checks failed]
result_any_warn -- condition met --> write_warn[Write TEST_OUTPUT: result FAIL_SEVERITY, note differences or errors]
write_all_errors --> end_all_errors([Exit step])
write_warn --> end_warn([Exit step])
result_all_success --> end_success([Exit step])
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- The
pip installofcheck-source-originfrommainvia a Git URL makes the task non-deterministic; consider pinning to a specific tag/commit or exposing the ref as a parameter so pipeline behavior is stable over time. - The CA bundle is mounted as
/etc/pki/tls/certs/ca-custom-bundle.crtbut never wired into tooling; consider either mounting it over the default trust bundle or settingSSL_CERT_FILE/REQUESTS_CA_BUNDLEsopipandcheck-source-originactually use the custom CA. - The
OUTPUTvariable capturingcheck-source-origin's--json-outputis never used; either log or aggregate that JSON intoTEST_OUTPUT, or drop the assignment to avoid confusion about unused data.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `pip install` of `check-source-origin` from `main` via a Git URL makes the task non-deterministic; consider pinning to a specific tag/commit or exposing the ref as a parameter so pipeline behavior is stable over time.
- The CA bundle is mounted as `/etc/pki/tls/certs/ca-custom-bundle.crt` but never wired into tooling; consider either mounting it over the default trust bundle or setting `SSL_CERT_FILE`/`REQUESTS_CA_BUNDLE` so `pip` and `check-source-origin` actually use the custom CA.
- The `OUTPUT` variable capturing `check-source-origin`'s `--json-output` is never used; either log or aggregate that JSON into `TEST_OUTPUT`, or drop the assignment to avoid confusion about unused data.
## Individual Comments
### Comment 1
<location path="tasks/check-source-origin.yaml" line_range="100" />
<code_context>
+ exit 0
+ fi
+
+ pip install --quiet "check-source-origin @ git+https://github.com/calungaproject/check-source-origin.git"
+
+ SUCCESSES=0
</code_context>
<issue_to_address>
**🚨 suggestion (security):** Pin the check-source-origin dependency to a tag or commit to avoid non-deterministic behavior.
Installing directly from the repo’s default branch makes this Task non-deterministic and increases supply-chain risk (unexpected breaking changes or a compromised upstream). Please pin to a specific tag or commit SHA, e.g. `git+https://github.com/...@<tag-or-sha>`, and update intentionally as needed.
Suggested implementation:
```
# Pin check-source-origin to a specific tag or commit SHA to ensure deterministic behavior
pip install --quiet "check-source-origin @ git+https://github.com/calungaproject/check-source-origin.git@v0.1.0"
```
1. Replace `v0.1.0` with an actual tag or commit SHA that you have vetted and want to depend on.
2. When updating `check-source-origin` in the future, explicitly bump this tag/SHA in this line as part of the PR so changes remain intentional and reviewable.
</issue_to_address>
### Comment 2
<location path="tasks/check-source-origin.yaml" line_range="71-72" />
<code_context>
+
+ GIT_CREDENTIALS="$(workspaces.basic-auth.path)/.git-credentials"
+ if [ "$(workspaces.basic-auth.bound)" != "true" ] || [ ! -f "${GIT_CREDENTIALS}" ]; then
+ echo "Git credentials not found — skipping source origin check."
+ echo '{"result":"SKIPPED","namespace":"default","successes":0,"failures":0,"warnings":0,"note":"git-auth workspace not bound"}' | tee "${RESULT_FILE}"
+ exit 0
+ fi
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Avoid hard-coding the namespace as "default" in JSON results.
Hard-coding `"namespace":"default"` will be incorrect when this Task runs in other namespaces. Prefer omitting `namespace` or deriving it from the runtime environment (e.g., via a downward API–provided env var) so the JSON accurately reflects the actual namespace.
Suggested implementation:
```
echo "Git credentials not found — skipping source origin check."
echo '{"result":"SKIPPED","successes":0,"failures":0,"warnings":0,"note":"git-auth workspace not bound"}' | tee "${RESULT_FILE}"
exit 0
```
```
if [ -z "${GH_TOKEN}" ]; then
echo "No GitHub token found in git credentials — skipping."
echo '{"result":"SKIPPED","successes":0,"failures":0,"warnings":0,"note":"no github token in git-credentials"}' | tee "${RESULT_FILE}"
exit 0
fi
```
</issue_to_address>
### Comment 3
<location path="tasks/check-source-origin.yaml" line_range="93-95" />
<code_context>
+ chmod 600 ~/.gitconfig
+ fi
+
+ PACKAGE_LIST="$(echo "${PACKAGES}" | python3 -c "import json,sys; [print(p) for p in json.load(sys.stdin)]" 2>/dev/null || true)"
+ if [ -z "${PACKAGE_LIST}" ]; then
+ echo "No packages to check."
+ echo '{"result":"SKIPPED","namespace":"default","successes":0,"failures":0,"warnings":0,"note":"no packages"}' | tee "${RESULT_FILE}"
+ exit 0
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Distinguish between "no packages" and invalid PACKAGES JSON instead of silently skipping.
Because JSON parsing errors (or a non-list structure) are redirected to `/dev/null` and ignored, `PACKAGE_LIST` ends up empty and the task reports `SKIPPED` with `"note":"no packages"`, masking configuration issues. Consider treating JSON parse/type errors as a failure (or at minimum using a distinct note/result) so misconfigurations are visible instead of looking like a valid "no packages" case.
Suggested implementation:
```
if ! PACKAGE_LIST="$(echo "${PACKAGES}" | python3 -c 'import json, sys
try:
data = json.load(sys.stdin)
if not isinstance(data, list):
print("PACKAGES JSON must be a list", file=sys.stderr)
sys.exit(2)
for p in data:
print(p)
except Exception as e:
print(str(e), file=sys.stderr)
sys.exit(1)
')"; then
echo "Invalid PACKAGES JSON (must be a JSON list): ${PACKAGES}" >&2
echo '{"result":"FAILURE","namespace":"default","successes":0,"failures":1,"warnings":0,"note":"invalid PACKAGES json (must be a JSON list)"}' | tee "${RESULT_FILE}"
exit 1
fi
if [ -z "${PACKAGE_LIST}" ]; then
echo "No packages to check."
echo '{"result":"SKIPPED","namespace":"default","successes":0,"failures":0,"warnings":0,"note":"no packages"}' | tee "${RESULT_FILE}"
exit 0
fi
```
If this task has a specific convention for the `"result"` field (e.g. `"FAILED"` instead of `"FAILURE"`, or different success/failure counters), adjust the JSON object in the failure branch accordingly so it matches the rest of your pipeline’s expectations.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| echo "Git credentials not found — skipping source origin check." | ||
| echo '{"result":"SKIPPED","namespace":"default","successes":0,"failures":0,"warnings":0,"note":"git-auth workspace not bound"}' | tee "${RESULT_FILE}" |
There was a problem hiding this comment.
suggestion (bug_risk): Avoid hard-coding the namespace as "default" in JSON results.
Hard-coding "namespace":"default" will be incorrect when this Task runs in other namespaces. Prefer omitting namespace or deriving it from the runtime environment (e.g., via a downward API–provided env var) so the JSON accurately reflects the actual namespace.
Suggested implementation:
echo "Git credentials not found — skipping source origin check."
echo '{"result":"SKIPPED","successes":0,"failures":0,"warnings":0,"note":"git-auth workspace not bound"}' | tee "${RESULT_FILE}"
exit 0
if [ -z "${GH_TOKEN}" ]; then
echo "No GitHub token found in git credentials — skipping."
echo '{"result":"SKIPPED","successes":0,"failures":0,"warnings":0,"note":"no github token in git-credentials"}' | tee "${RESULT_FILE}"
exit 0
fi
ece8439 to
82c5de1
Compare
Verifies Python sdist packages against their upstream VCS source repositories before building wheels. Uses the git-auth workspace for GitHub API access, installs check-source-origin pinned to a specific commit, and outputs TEST_OUTPUT in Konflux EC format. Non-blocking: gracefully skips when credentials are missing. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Pin check-source-origin install to a specific commit SHA via new CSO_GIT_REF param for reproducible builds. Wire CA bundle into pip and requests via SSL_CERT_FILE/REQUESTS_CA_BUNDLE when mounted. Remove unused OUTPUT variable capture. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Report ERROR when PACKAGES JSON fails to parse instead of silently skipping as if no packages were provided. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The previous pinned commit no longer exists in the repo. Use main branch while the tool is under active development — version gets locked when the task is bundled. Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
- Add FAIL_SEVERITY param (default: WARNING) to control result severity for verification failures — switch to ERROR once the task is reliable - Change missing credentials from SKIPPED to ERROR so release EC catches configuration problems - Change empty packages from SKIPPED to FAIL_SEVERITY since the pipeline when-guard ensures packages exist before this task runs - Fix grep under pipefail: add || true to prevent crash when credentials file has no github.com line Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
82c5de1 to
5682abc
Compare
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The task is described and intended as non-blocking/skip-on-missing-creds, but the JSON result for missing git credentials or GH token is hard-coded as "ERROR" rather than something like "SKIPPED" or using
FAIL_SEVERITY, which may confuse downstream consumers expecting a non-blocking outcome; consider aligning these cases with the documented semantics. - The
namespacefield inTEST_OUTPUTis hard-coded to "default" in all result paths; if this is meant to reflect the actual namespace, consider wiring it from the environment or a param so the output is accurate in multi-namespace deployments. - The
workdiremptyDir volume is mounted at/var/workdirbut not used in the script; if it’s not needed, you can remove the volume and mount to simplify the task definition.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The task is described and intended as non-blocking/skip-on-missing-creds, but the JSON result for missing git credentials or GH token is hard-coded as "ERROR" rather than something like "SKIPPED" or using `FAIL_SEVERITY`, which may confuse downstream consumers expecting a non-blocking outcome; consider aligning these cases with the documented semantics.
- The `namespace` field in `TEST_OUTPUT` is hard-coded to "default" in all result paths; if this is meant to reflect the actual namespace, consider wiring it from the environment or a param so the output is accurate in multi-namespace deployments.
- The `workdir` emptyDir volume is mounted at `/var/workdir` but not used in the script; if it’s not needed, you can remove the volume and mount to simplify the task definition.
## Individual Comments
### Comment 1
<location path="tasks/check-source-origin.yaml" line_range="87-89" />
<code_context>
+
+ RESULT_FILE="$(results.TEST_OUTPUT.path)"
+
+ GIT_CREDENTIALS="$(workspaces.basic-auth.path)/.git-credentials"
+ if [ "$(workspaces.basic-auth.bound)" != "true" ] || [ ! -f "${GIT_CREDENTIALS}" ]; then
+ echo "Git credentials not found — skipping source origin check."
+ echo '{"result":"ERROR","namespace":"default","successes":0,"failures":0,"warnings":0,"note":"git-auth workspace not bound"}' | tee "${RESULT_FILE}"
+ exit 0
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Consider using a non-error result when intentionally skipping due to missing git credentials
This path reports `"result":"ERROR"` while exiting 0 and logging a skip, which conflicts with the task’s "gracefully skip" contract and can mislead consumers that only read `TEST_OUTPUT`. Please use a non-error result (e.g., `SKIPPED`) or connect this to `FAIL_SEVERITY` so missing credentials are treated as a visible skip rather than a hard error in the output.
Suggested implementation:
```
GIT_CREDENTIALS="$(workspaces.basic-auth.path)/.git-credentials"
if [ "$(workspaces.basic-auth.bound)" != "true" ] || [ ! -f "${GIT_CREDENTIALS}" ]; then
echo "Git credentials not found — skipping source origin check."
echo '{"result":"SKIPPED","namespace":"default","successes":0,"failures":0,"warnings":0,"note":"git-auth workspace not bound"}' | tee "${RESULT_FILE}"
exit 0
fi
```
```
if [ -z "${GH_TOKEN}" ]; then
echo "No GitHub token found in git credentials — skipping."
echo '{"result":"SKIPPED","namespace":"default","successes":0,"failures":0,"warnings":0,"note":"no github token in git-credentials"}' | tee "${RESULT_FILE}"
exit 0
fi
```
1. If your task contract already defines an explicit status vocabulary or uses `FAIL_SEVERITY`, you may want to introduce a `RESULT_STATUS` variable that derives from those conventions and reuse it in all `TEST_OUTPUT` writes for consistency.
2. Consumers that currently treat `"result":"ERROR"` as a hard failure may need to be updated to also look for `"result":"SKIPPED"` to distinguish intentional skips from actual errors.
</issue_to_address>
### Comment 2
<location path="tasks/check-source-origin.yaml" line_range="95-98" />
<code_context>
+ fi
+
+ { set +x; } 2>/dev/null
+ GH_TOKEN="$(grep github.com "${GIT_CREDENTIALS}" 2>/dev/null | sed 's|.*://[^:]*:\([^@]*\)@.*|\1|' | head -1 || true)"
+ export GH_TOKEN
+ if [ -z "${GH_TOKEN}" ]; then
+ echo "No GitHub token found in git credentials — skipping."
+ echo '{"result":"ERROR","namespace":"default","successes":0,"failures":0,"warnings":0,"note":"no github token in git-credentials"}' | tee "${RESULT_FILE}"
+ exit 0
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Align the reported result for missing GitHub token with the documented non-blocking behavior
This path logs that the check is skipped but still writes `"result":"ERROR"`, which may cause `TEST_OUTPUT` consumers to treat it as a failure. Consider either emitting `"result":"SKIPPED"` here, or wiring this through `FAIL_SEVERITY` so callers can choose whether a missing token is treated as non-blocking or as an error.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| GIT_CREDENTIALS="$(workspaces.basic-auth.path)/.git-credentials" | ||
| if [ "$(workspaces.basic-auth.bound)" != "true" ] || [ ! -f "${GIT_CREDENTIALS}" ]; then | ||
| echo "Git credentials not found — skipping source origin check." |
There was a problem hiding this comment.
suggestion (bug_risk): Consider using a non-error result when intentionally skipping due to missing git credentials
This path reports "result":"ERROR" while exiting 0 and logging a skip, which conflicts with the task’s "gracefully skip" contract and can mislead consumers that only read TEST_OUTPUT. Please use a non-error result (e.g., SKIPPED) or connect this to FAIL_SEVERITY so missing credentials are treated as a visible skip rather than a hard error in the output.
Suggested implementation:
GIT_CREDENTIALS="$(workspaces.basic-auth.path)/.git-credentials"
if [ "$(workspaces.basic-auth.bound)" != "true" ] || [ ! -f "${GIT_CREDENTIALS}" ]; then
echo "Git credentials not found — skipping source origin check."
echo '{"result":"SKIPPED","namespace":"default","successes":0,"failures":0,"warnings":0,"note":"git-auth workspace not bound"}' | tee "${RESULT_FILE}"
exit 0
fi
if [ -z "${GH_TOKEN}" ]; then
echo "No GitHub token found in git credentials — skipping."
echo '{"result":"SKIPPED","namespace":"default","successes":0,"failures":0,"warnings":0,"note":"no github token in git-credentials"}' | tee "${RESULT_FILE}"
exit 0
fi
- If your task contract already defines an explicit status vocabulary or uses
FAIL_SEVERITY, you may want to introduce aRESULT_STATUSvariable that derives from those conventions and reuse it in allTEST_OUTPUTwrites for consistency. - Consumers that currently treat
"result":"ERROR"as a hard failure may need to be updated to also look for"result":"SKIPPED"to distinguish intentional skips from actual errors.
| GH_TOKEN="$(grep github.com "${GIT_CREDENTIALS}" 2>/dev/null | sed 's|.*://[^:]*:\([^@]*\)@.*|\1|' | head -1 || true)" | ||
| export GH_TOKEN | ||
| if [ -z "${GH_TOKEN}" ]; then | ||
| echo "No GitHub token found in git credentials — skipping." |
There was a problem hiding this comment.
suggestion (bug_risk): Align the reported result for missing GitHub token with the documented non-blocking behavior
This path logs that the check is skipped but still writes "result":"ERROR", which may cause TEST_OUTPUT consumers to treat it as a failure. Consider either emitting "result":"SKIPPED" here, or wiring this through FAIL_SEVERITY so callers can choose whether a missing token is treated as non-blocking or as an error.
Verifies Python sdist packages against their upstream VCS source repositories before building wheels. Uses the git-auth workspace for GitHub API access, installs check-source-origin from main (unpinned while WIP), and outputs TEST_OUTPUT in Konflux EC format. Non-blocking: gracefully skips when credentials are missing.
Summary by Sourcery
Add a Tekton task to verify Python sdist packages’ source origins using the check-source-origin tool and emit structured verification results.
New Features:
Enhancements: