-
Notifications
You must be signed in to change notification settings - Fork 60
[DNM] Add dummy task, pipeline, and ITS for EC-2011 POC #3476
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -34,6 +34,7 @@ permissions: | |
| jobs: | ||
|
|
||
| Test: | ||
| if: false # skipped for POC branch | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [high] CI coverage regression The Suggested fix: Remove the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [high] CI coverage regression Adding if: false to the Test, Acceptance, and Upload jobs disables all test and coverage CI for every PR and push to main. The release workflow (release.yaml) triggers on workflow_run with conclusion=='success'. Since the Tools job still runs and skipped jobs do not fail the workflow, the overall conclusion will be 'success', meaning releases could proceed on every main push without any test validation. Suggested fix: Do not merge if: false on the main branch. Keep these changes on a separate POC branch, use path-based conditions, or disable the release workflow trigger as well. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [medium] scope-creep Disabling all three CI jobs is beyond the stated intent of adding a dummy task/pipeline. The PR is marked [DNM] but is not a draft, increasing the risk of accidental merge. No documented cleanup plan or expiration date exists for the POC artifacts. Suggested fix: Remove CI workflow changes from this PR, or convert to a draft PR. |
||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Harden Runner | ||
|
|
@@ -96,6 +97,7 @@ jobs: | |
| retention-days: 1 | ||
|
|
||
| Acceptance: | ||
| if: false # skipped for POC branch | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| # Disabled until we figure out the "Could not resolve host: github.com" in | ||
|
|
@@ -164,6 +166,7 @@ jobs: | |
|
|
||
| Upload: | ||
| name: "Upload Coverage Statistics" | ||
| if: false # skipped for POC branch | ||
| runs-on: ubuntu-latest | ||
| needs: [Test, Acceptance] | ||
| steps: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| #!/usr/bin/env bash | ||
| # Copyright The Conforma Contributors | ||
| # | ||
| # Licensed under the Apache License, Version 2.0 (the "License"); | ||
| # you may not use this file except in compliance with the License. | ||
| # You may obtain a copy of the License at | ||
| # | ||
| # http://www.apache.org/licenses/LICENSE-2.0 | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] pattern-inconsistency License header URL indentation uses 5 spaces vs repo convention of 6 spaces. Affects all 3 new files. |
||
| # | ||
| # Unless required by applicable law or agreed to in writing, software | ||
| # distributed under the License is distributed on an "AS IS" BASIS, | ||
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
| # | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| set -euo pipefail | ||
|
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] naming-convention Script uses set -euo pipefail but repo convention is long-form (set -o errexit; set -o nounset; set -o pipefail). |
||
| NAMESPACE="${NAMESPACE:-rhtap-contract-tenant}" | ||
| APPLICATION="${APPLICATION:-ec-main}" | ||
| ITS_NAME="${ITS_NAME:-reqd-task-poc-ec2011}" | ||
|
|
||
| GIT_URL="${GIT_URL:-https://github.com/simonbaird/conforma-cli}" | ||
| GIT_REVISION="${GIT_REVISION:-reqd-task-its-poc}" | ||
| PIPELINE_PATH="${PIPELINE_PATH:-pipelines/dummy-integration-test/0.1/dummy-integration-test.yaml}" | ||
|
|
||
| echo "Creating IntegrationTestScenario '${ITS_NAME}' in namespace '${NAMESPACE}'" | ||
| echo " Application: ${APPLICATION}" | ||
| echo " Git URL: ${GIT_URL}" | ||
| echo " Revision: ${GIT_REVISION}" | ||
| echo " Pipeline: ${PIPELINE_PATH}" | ||
| echo "" | ||
|
|
||
| oc apply -f - <<EOF | ||
| apiVersion: appstudio.redhat.com/v1beta2 | ||
| kind: IntegrationTestScenario | ||
| metadata: | ||
| name: ${ITS_NAME} | ||
| namespace: ${NAMESPACE} | ||
| labels: | ||
| test.appstudio.openshift.io/optional: "true" | ||
|
Comment on lines
+41
to
+42
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. π― Functional Correctness | π Major | β‘ Quick win π§© Analysis chainπ Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- script ---'
cat -n hack/create-dummy-its.sh
printf '%s\n' '--- related references ---'
rg -n -i --glob '!vendor/**' --glob '!node_modules/**' \
'reqd-task-poc-ec2011|test\.appstudio\.openshift\.io/optional|optional.*IntegrationTestScenario|IntegrationTestScenario|required task|required-task' .Repository: conforma/cli Length of output: 2721 π Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- dummy integration-test pipeline ---'
cat -n pipelines/dummy-integration-test/0.1/dummy-integration-test.yaml
printf '%s\n' '--- enterprise-contract guidance ---'
sed -n '70,110p' pipelines/enterprise-contract/0.1/enterprise-contract.yaml
printf '%s\n' '--- all optional-label and informative references ---'
rg -n -i 'test\.appstudio\.openshift\.io/optional|informative|required.*task|task.*required|IntegrationTestScenario' \
README.md docs hack pipelines internal config .github 2>/dev/null || trueRepository: conforma/cli Length of output: 7294 π Web query:
π‘ Result: The label Citations:
Remove the optional label from the required-task scenario. The π€ Prompt for AI Agents |
||
| spec: | ||
| application: ${APPLICATION} | ||
| contexts: | ||
| - description: Application testing | ||
| name: application | ||
| resolverRef: | ||
| resolver: git | ||
| resourceKind: pipeline | ||
| params: | ||
| - name: url | ||
| value: ${GIT_URL} | ||
| - name: revision | ||
| value: ${GIT_REVISION} | ||
| - name: pathInRepo | ||
| value: ${PIPELINE_PATH} | ||
| EOF | ||
|
|
||
| echo "" | ||
| echo "Done. Verify with:" | ||
| echo " oc get integrationtestscenario ${ITS_NAME} -n ${NAMESPACE} -o yaml" | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,102 @@ | ||
| # Copyright The Conforma Contributors | ||
| # | ||
| # Licensed under the Apache License, Version 2.0 (the "License"); | ||
| # you may not use this file except in compliance with the License. | ||
| # You may obtain a copy of the License at | ||
| # | ||
| # http://www.apache.org/licenses/LICENSE-2.0 | ||
| # | ||
| # Unless required by applicable law or agreed to in writing, software | ||
| # distributed under the License is distributed on an "AS IS" BASIS, | ||
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
| # | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| --- | ||
| apiVersion: tekton.dev/v1 | ||
| kind: Pipeline | ||
| metadata: | ||
| name: reqd-task-poc-ec2011 | ||
| labels: | ||
| build.appstudio.redhat.com/pipeline: "reqd-task-poc-ec2011" | ||
| spec: | ||
| params: | ||
| - name: SNAPSHOT | ||
| type: string | ||
| description: | | ||
| Spec section of an ApplicationSnapshot resource. Not all fields of the | ||
| resource are required. A minimal example: | ||
| { | ||
| "components": [ | ||
| { | ||
| "containerImage": "quay.io/example/repo@sha256:abc123..." | ||
| } | ||
| ] | ||
| } | ||
| Each "containerImage" in the "components" array is validated. | ||
| - name: RESULT | ||
| type: string | ||
| description: >- | ||
| The desired result of the dummy check. Must be one of: SUCCESS, | ||
| FAILURE, WARNING, ERROR, or SKIPPED. | ||
| default: "SUCCESS" | ||
| results: | ||
| - name: TEST_OUTPUT | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] fragile JSON parsing parse-snapshot uses grep -oP for JSON parsing, which is fragile for minified or escaped JSON and may not be portable across all container images. |
||
| value: "$(tasks.dummy-check.results.TEST_OUTPUT)" | ||
| tasks: | ||
| - name: parse-snapshot | ||
| taskSpec: | ||
| params: | ||
| - name: SNAPSHOT | ||
| type: string | ||
| results: | ||
| - name: image-url | ||
| - name: image-digest | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] grep -oP portability The parse-snapshot step uses grep -oP (PCRE with \K lookbehind) to extract containerImage from SNAPSHOT JSON. While ubi9/ubi-minimal ships GNU grep with -P support, parsing structured JSON with regex is fragile and may break on multi-line or reordered JSON. Suggested fix: Consider using jq for JSON parsing if available in the image. |
||
| steps: | ||
| - name: parse | ||
| image: registry.access.redhat.com/ubi9/ubi-minimal:latest | ||
| env: | ||
| - name: SNAPSHOT | ||
| value: $(params.SNAPSHOT) | ||
| script: | | ||
| #!/usr/bin/env bash | ||
| set -euo pipefail | ||
|
|
||
| IMAGE=$(echo "${SNAPSHOT}" | grep -oP '"containerImage"\s*:\s*"\K[^"]+' | head -1) | ||
| if [[ -z "${IMAGE}" ]]; then | ||
| echo "ERROR: No containerImage found in SNAPSHOT" >&2 | ||
| exit 1 | ||
|
Comment on lines
+67
to
+70
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 3. Only first component validated The pipeline description claims each components[].containerImage is validated, but parse-snapshot selects only the first match (head -1) and also doesnβt validate that the image reference contains an @ digest, which can pass incorrect image-url/image-digest into the attestation step. Agent Prompt
|
||
| fi | ||
|
|
||
| IMAGE_URL="${IMAGE%%@*}" | ||
| IMAGE_DIGEST="${IMAGE##*@}" | ||
|
|
||
| echo "Parsed image-url: ${IMAGE_URL}" | ||
| echo "Parsed image-digest: ${IMAGE_DIGEST}" | ||
|
|
||
| echo -n "${IMAGE_URL}" > "$(results.image-url.path)" | ||
| echo -n "${IMAGE_DIGEST}" > "$(results.image-digest.path)" | ||
|
Comment on lines
+67
to
+80
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ποΈ Data Integrity & Integration | π Major | ποΈ Heavy lift π§© Analysis chainπ Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- candidate files ---'
git ls-files | grep -E '(^|/)(pipelines/dummy-integration-test/0\.1/dummy-integration-test\.yaml|hack/create-dummy-its\.sh)$' || true
printf '%s\n' '--- YAML structure ---'
ast-grep outline pipelines/dummy-integration-test/0.1/dummy-integration-test.yaml --lang yaml || true
printf '%s\n' '--- YAML lines 1-90 ---'
cat -n pipelines/dummy-integration-test/0.1/dummy-integration-test.yaml | sed -n '1,90p'
printf '%s\n' '--- generator script ---'
if test -f hack/create-dummy-its.sh; then cat -n hack/create-dummy-its.sh | sed -n '1,180p'; fi
printf '%s\n' '--- related references ---'
rg -n -C 3 'containerImage|image-url|image-digest|dummy-integration-test|create-dummy-its|SNAPSHOT' pipelines hack README.md 2>/dev/null | sed -n '1,260p'Repository: conforma/cli Length of output: 24835 π Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- dummy-check task ---'
fd -i 'dummy-check' . --type f | sort
for f in $(fd -i 'dummy-check' . --type f | sort); do
echo "--- $f ---"
wc -l "$f"
cat -n "$f" | sed -n '1,180p'
done
printf '%s\n' '--- pipeline and task result consumers ---'
rg -n -C 5 'image-digest|image-url|dummy-check|components.*containerImage|containerImage' pipelines tasks .github 2>/dev/null | sed -n '1,320p'
printf '%s\n' '--- deterministic parser probe ---'
python3 - <<'PY'
import json, re, subprocess
script = r'''set -euo pipefail
SNAPSHOT="$1"
IMAGE=$(echo "${SNAPSHOT}" | grep -oP '"containerImage"\s*:\s*"\K[^"]+' | head -1)
if [[ -z "${IMAGE}" ]]; then exit 1; fi
IMAGE_URL="${IMAGE%%@*}"
IMAGE_DIGEST="${IMAGE##*@}"
printf 'url=%s\ndigest=%s\n' "$IMAGE_URL" "$IMAGE_DIGEST"
'''
cases = {
"two components": {"components": [
{"containerImage": "quay.io/a@sha256:aaa"},
{"containerImage": "quay.io/b@sha256:bbb"},
]},
"tag only": {"components": [{"containerImage": "quay.io/a:latest"}]},
"invalid JSON with matching text": 'prefix "containerImage": "quay.io/a@sha256:aaa" suffix',
"escaped quote value": {"components": [{"containerImage": "quay.io/a@sha256:aa\\\"bb"}]},
"missing component image": {"components": [{"name": "a"}]},
}
for name, value in cases.items():
text = value if isinstance(value, str) else json.dumps(value)
p = subprocess.run(["bash", "-c", script, "parser", text], text=True,
capture_output=True)
print(f"[{name}] exit={p.returncode}")
print((p.stdout + p.stderr).strip() or "<no output>")
PYRepository: conforma/cli Length of output: 24091 Parse and validate every Snapshot component. Line 51 accepts non-JSON text and processes only the first If an image has no π€ Prompt for AI Agents |
||
| params: | ||
| - name: SNAPSHOT | ||
| value: "$(params.SNAPSHOT)" | ||
| - name: dummy-check | ||
| runAfter: | ||
| - parse-snapshot | ||
| params: | ||
| - name: RESULT | ||
| value: "$(params.RESULT)" | ||
| - name: image-url | ||
| value: "$(tasks.parse-snapshot.results.image-url)" | ||
| - name: image-digest | ||
| value: "$(tasks.parse-snapshot.results.image-digest)" | ||
| taskRef: | ||
| resolver: git | ||
| params: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [medium] Hardcoded personal fork reference The pipeline's dummy-check taskRef resolver references https://github.com/simonbaird/conforma-cli on branch reqd-task-its-poc. This creates a runtime dependency on a personal fork that may be deleted or force-pushed. The task definition is being added to this same repository. The same pattern appears in hack/create-dummy-its.sh. Suggested fix: Update the git resolver URL to https://github.com/conforma/cli and reference the branch/revision where the task will exist after merge. Update hack/create-dummy-its.sh defaults similarly. |
||
| - name: url | ||
| value: https://github.com/simonbaird/conforma-cli | ||
| - name: revision | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [high] supply chain / untrusted code source All git resolver references across the pipeline, task, and helper script point to personal forks (simonbaird/conforma-cli and simonbaird/step-actions) on mutable branches rather than canonical org repos or pinned SHAs. This includes the attestation-creation step which is security-critical. Fork branches can be force-pushed, deleted, or compromised without review. Suggested fix: Point references to org repos or pin to commit SHAs. At minimum, add prominent comments documenting that these are temporary POC references. |
||
| value: reqd-task-its-poc | ||
| - name: pathInRepo | ||
| value: tasks/dummy-check/0.1/dummy-check.yaml | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,169 @@ | ||
| # Copyright The Conforma Contributors | ||
| # | ||
| # Licensed under the Apache License, Version 2.0 (the "License"); | ||
| # you may not use this file except in compliance with the License. | ||
| # You may obtain a copy of the License at | ||
| # | ||
| # http://www.apache.org/licenses/LICENSE-2.0 | ||
| # | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] license-header-formatting License header URL line uses 5-space indentation while existing tasks and pipelines use 6-space indentation. Suggested fix: Change to 6-space indentation after # for the URL line in all three new files. |
||
| # Unless required by applicable law or agreed to in writing, software | ||
| # distributed under the License is distributed on an "AS IS" BASIS, | ||
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
| # | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| --- | ||
| apiVersion: tekton.dev/v1 | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] yaml-document-separator The --- separator is placed after the license header. Existing task YAMLs place --- as line 1, before the license header. Suggested fix: Move --- to line 1, before the license header, to match existing task conventions. |
||
| kind: Task | ||
| metadata: | ||
| labels: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] yaml-metadata-ordering Metadata fields ordered labels, annotations, name. Existing tasks consistently use name, annotations, labels. Suggested fix: Reorder metadata fields to: name, annotations, labels. |
||
| app.kubernetes.io/version: "0.1" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] code-organization Metadata fields ordered as labels/annotations/name but existing tasks use name/annotations/labels ordering. |
||
| annotations: | ||
| tekton.dev/pipelines.minVersion: "0.12.1" | ||
| tekton.dev/tags: "konflux" | ||
| name: dummy-check | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] pattern-inconsistency Tags use quoted style vs unquoted comma-separated in existing tasks. Missing blank line before spec. |
||
| spec: | ||
| description: >- | ||
| A dummy task for testing purposes. Instead of performing a real check, it | ||
| produces a pass, fail, or warn result based on the RESULT param. Mimics the | ||
| output format of real SAST tasks (e.g. sast-snyk-check) so it can be used | ||
| as a stand-in during pipeline development and testing. | ||
| results: | ||
| - description: Tekton task test output. | ||
| name: TEST_OUTPUT | ||
| - name: TEST_OUTPUT_ARTIFACT_OUTPUTS | ||
| description: >- | ||
| JSON object with uri and digest referencing the pushed attestation. | ||
| Tekton Chains uses this for SLSA provenance. | ||
| type: object | ||
| properties: | ||
| uri: {} | ||
| digest: {} | ||
| params: | ||
| - name: RESULT | ||
| type: string | ||
| description: >- | ||
| The desired result of this dummy check. Must be one of: SUCCESS, | ||
| FAILURE, WARNING, ERROR, or SKIPPED. | ||
| default: "SUCCESS" | ||
| - name: NOTE | ||
| type: string | ||
| description: >- | ||
| Optional note to include in the test output. If not provided, a default | ||
| message is generated based on the RESULT value. | ||
| default: "" | ||
| - name: SUCCESSES | ||
| type: string | ||
| description: Number of successes to report in the test output. | ||
| default: "1" | ||
| - name: FAILURES | ||
| type: string | ||
| description: Number of failures to report in the test output. | ||
| default: "0" | ||
| - name: WARNINGS | ||
| type: string | ||
| description: Number of warnings to report in the test output. | ||
| default: "0" | ||
| - name: image-url | ||
| description: Image URL. | ||
| type: string | ||
| default: "" | ||
| - name: image-digest | ||
| description: Digest of the image. | ||
| type: string | ||
| default: "" | ||
| steps: | ||
| - name: dummy-check | ||
| results: | ||
| - name: TEST_OUTPUT | ||
| description: JSON test results for consumption by subsequent steps. | ||
| image: registry.access.redhat.com/ubi9/ubi-minimal:latest | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. π Security & Privacy | π Major | β‘ Quick win Pin executable container images by digest. Both steps use the mutable
π Affects 2 files
π€ Prompt for AI Agents |
||
| env: | ||
| - name: RESULT | ||
| value: $(params.RESULT) | ||
| - name: NOTE | ||
| value: $(params.NOTE) | ||
| - name: SUCCESSES | ||
| value: $(params.SUCCESSES) | ||
| - name: FAILURES | ||
| value: $(params.FAILURES) | ||
| - name: WARNINGS | ||
| value: $(params.WARNINGS) | ||
| computeResources: | ||
| limits: | ||
| memory: 256Mi | ||
| requests: | ||
| cpu: 100m | ||
| memory: 128Mi | ||
| script: | | ||
| #!/usr/bin/env bash | ||
| set -euo pipefail | ||
|
|
||
| VALID_RESULTS="SUCCESS FAILURE WARNING ERROR SKIPPED" | ||
| if ! echo "${VALID_RESULTS}" | grep -qw "${RESULT}"; then | ||
| echo "ERROR: Invalid RESULT '${RESULT}'. Must be one of: ${VALID_RESULTS}" >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| if [[ -z "${NOTE}" ]]; then | ||
| case "${RESULT}" in | ||
| SUCCESS) NOTE="Task $(context.task.name) completed successfully. This is a dummy check." ;; | ||
| FAILURE) NOTE="Task $(context.task.name) failed. This is a dummy check." ;; | ||
| WARNING) NOTE="Task $(context.task.name) produced warnings. This is a dummy check." ;; | ||
| ERROR) NOTE="Task $(context.task.name) encountered an error. This is a dummy check." ;; | ||
| SKIPPED) NOTE="Task $(context.task.name) was skipped. This is a dummy check." ;; | ||
| esac | ||
| fi | ||
|
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] Default counter inconsistency The FAILURE case only adjusts SUCCESSES if it equals '1' (the default). If a user passes SUCCESSES=5 and RESULT=FAILURE, the task reports 5 successes alongside the failure, which is semantically inconsistent. |
||
| # Adjust counts based on RESULT if user left defaults | ||
| case "${RESULT}" in | ||
| FAILURE) | ||
| if [[ "${FAILURES}" == "0" ]]; then FAILURES="1"; fi | ||
| if [[ "${SUCCESSES}" == "1" ]]; then SUCCESSES="0"; fi | ||
| ;; | ||
| WARNING) | ||
| if [[ "${WARNINGS}" == "0" ]]; then WARNINGS="1"; fi | ||
| ;; | ||
| ERROR|SKIPPED) | ||
| SUCCESSES="0" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [medium] JSON injection via unescaped interpolation TEST_OUTPUT JSON is constructed via printf '%s' string interpolation of the NOTE variable. If NOTE contains JSON-special characters (double quotes, backslashes, newlines), the resulting JSON will be malformed. Auto-generated NOTE values are safe, but NOTE is a user-facing parameter with no escaping. Suggested fix: Use jq to construct the JSON output to ensure proper escaping of all user-controlled values. |
||
| FAILURES="0" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] JSON injection / malformed output TEST_OUTPUT JSON is constructed via printf with %s interpolation of the NOTE variable. If NOTE contains double quotes, backslashes, or newlines, the output will be syntactically invalid. Limited blast radius in POC context. |
||
| WARNINGS="0" | ||
| ;; | ||
| esac | ||
|
|
||
| TIMESTAMP=$(date -u +"%Y-%m-%dT%H:%M:%S") | ||
|
|
||
| TEST_OUTPUT=$(printf '{ | ||
| "result": "%s", | ||
| "timestamp": "%s", | ||
| "note": "%s", | ||
|
Comment on lines
+138
to
+141
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 2. Unescaped note breaks json dummy-check builds JSON using printf with the unescaped NOTE parameter, so a NOTE containing quotes/newlines/backslashes produces invalid JSON and can break the attest-test-result step and any downstream consumer expecting parseable JSON. Agent Prompt
|
||
| "namespace": "default", | ||
| "successes": %d, | ||
| "failures": %d, | ||
| "warnings": %d | ||
| }' "${RESULT}" "${TIMESTAMP}" "${NOTE}" "${SUCCESSES}" "${FAILURES}" "${WARNINGS}") | ||
|
Comment on lines
+138
to
+146
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ποΈ Data Integrity & Integration | π Major | β‘ Quick win π§© Analysis chainπ Script executed: #!/bin/bash
set -eu
file="tasks/dummy-check/0.1/dummy-check.yaml"
printf '%s\n' '--- file outline ---'
if command -v ast-grep >/dev/null 2>&1; then
ast-grep outline "$file" || true
fi
printf '%s\n' '--- relevant task sections ---'
sed -n '1,170p' "$file"
printf '%s\n' '--- TEST_OUTPUT and attestation references ---'
rg -n -C 5 'TEST_OUTPUT|attest|NOTE|SUCCESSES|FAILURES|WARNINGS' tasks/dummy-check/0.1 "$file" tasks 2>/dev/null | head -n 300Repository: conforma/cli Length of output: 29153 π Script executed: #!/bin/bash
set -eu
run_case() {
local label="$1"
local note="$2"
local successes="$3"
local failures="$4"
local warnings="$5"
output=$(
printf '{
"result": "%s",
"timestamp": "%s",
"note": "%s",
"namespace": "default",
"successes": %d,
"failures": %d,
"warnings": %d
}' "SUCCESS" "2026-08-01T00:00:00" "$note" "$successes" "$failures" "$warnings" 2>/tmp/printf-error
) || true
printf '%s\t%s\n' "$label" "$output"
if python3 -c 'import json, sys; json.load(sys.stdin)' <<<"$output" 2>/dev/null; then
printf '%s\n' 'JSON_PARSE=valid'
else
printf '%s\n' 'JSON_PARSE=invalid'
fi
if [ -s /tmp/printf-error ]; then
printf 'PRINTF_ERROR=%s\n' "$(cat /tmp/printf-error)"
fi
}
run_case 'plain-note' 'all good' '1' '0' '0'
run_case 'quote-note' 'contains "quote"' '1' '0' '0'
run_case 'newline-note' $'contains\nnewline' '1' '0' '0'
run_case 'backslash-note' 'contains \ backslash' '1' '0' '0'
run_case 'invalid-count' 'all good' 'not-a-number' '0' '0'
run_case 'negative-count' 'all good' '-1' '0' '0'Repository: conforma/cli Length of output: 1415 π Script executed: #!/bin/bash
set -u
set +e
bash -c '
set -euo pipefail
TEST_OUTPUT=$(printf "{\n \"successes\": %d\n}\n" "not-a-number")
printf "WRITE_REACHED=%s\n" "$TEST_OUTPUT"
'
status=$?
printf 'COMMAND_STATUS=%d\n' "$status"
bash -c '
set -euo pipefail
TEST_OUTPUT=$(printf "{\n \"successes\": %d\n}\n" "-1")
printf "WRITE_REACHED=%s\n" "$TEST_OUTPUT"
'
status=$?
printf 'NEGATIVE_COMMAND_STATUS=%d\n' "$status"Repository: conforma/cli Length of output: 280 Serialize π€ Prompt for AI Agents |
||
|
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [medium] Unpinned mutable external reference The create-test-result-attestation step references https://github.com/conforma/step-actions at revision 'main'. This is a mutable branch reference; a compromised or force-pushed main branch could alter the step-action code. This step handles attestation creation, which is security-sensitive. Suggested fix: Pin the step-action resolver reference to an immutable commit SHA. |
||
| echo "Dummy check result: ${RESULT}" | ||
| echo "${TEST_OUTPUT}" | tee "$(results.TEST_OUTPUT.path)" | ||
| echo -n "${TEST_OUTPUT}" > "$(step.results.TEST_OUTPUT.path)" | ||
| - name: create-test-result-attestation | ||
| ref: | ||
| resolver: git | ||
| params: | ||
| - name: url | ||
| value: https://github.com/simonbaird/step-actions | ||
| - name: revision | ||
| value: runner-image-fix | ||
| - name: pathInRepo | ||
| value: stepactions/attest-test-result/0.1/attest-test-result.yaml | ||
|
Comment on lines
+157
to
+160
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. π Security & Privacy | π Major | β‘ Quick win π§© Analysis chainπ Script executed: #!/usr/bin/env bash
set -euo pipefail
git ls-remote https://github.com/conforma/step-actions main
git ls-remote https://github.com/conforma/cli main
git ls-remote https://github.com/simonbaird/conforma-cli reqd-task-its-pocRepository: conforma/cli Length of output: 334 π Script executed: #!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- tracked files ---'
git ls-files \
tasks/dummy-check/0.1/dummy-check.yaml \
pipelines/dummy-integration-test/0.1/dummy-integration-test.yaml \
hack/create-dummy-its.sh
printf '%s\n' '--- task definition ---'
sed -n '130,150p' tasks/dummy-check/0.1/dummy-check.yaml
printf '%s\n' '--- pipeline definition ---'
sed -n '70,95p' pipelines/dummy-integration-test/0.1/dummy-integration-test.yaml
printf '%s\n' '--- generator defaults ---'
sed -n '1,25p' hack/create-dummy-its.sh
printf '%s\n' '--- related resolver inputs ---'
rg -n -C 3 'pathInRepo|GIT_REVISION|reqd-task-its-poc|attest-test-result|dummy-integration-test' \
tasks pipelines hack .github 2>/dev/null || trueRepository: conforma/cli Length of output: 6509 Pin every Git resolver revision to an immutable commit.
π Affects 3 files
π€ Prompt for AI Agents |
||
| params: | ||
| - name: image-url | ||
| value: $(params.image-url) | ||
| - name: image-digest | ||
| value: $(params.image-digest) | ||
| - name: test-name | ||
| value: $(context.task.name) | ||
| - name: test-output | ||
| value: $(steps.dummy-check.results.TEST_OUTPUT) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
π― Functional Correctness | π Major | β‘ Quick win
Replace the constant job conditions with one lint-valid POC gate.
actionlintrejects each constantif: falsecondition. Use the same non-constant repository or workflow variable gate for all three jobs. Keep the gate false for the POC..github/workflows/checks-codecov.yaml#L37-L37: update theTestjob condition..github/workflows/checks-codecov.yaml#L100-L100: update theAcceptancejob condition..github/workflows/checks-codecov.yaml#L169-L169: update theUploadjob condition.π§° Tools
πͺ actionlint (1.7.12)
[error] 37-37: constant expression "false" in condition. remove the if: section
(if-cond)
π Affects 1 file
.github/workflows/checks-codecov.yaml#L37-L37(this comment).github/workflows/checks-codecov.yaml#L100-L100.github/workflows/checks-codecov.yaml#L169-L169π€ Prompt for AI Agents
Source: Linters/SAST tools