Skip to content

Skip keyless image signature checks in tekton tasks #3191

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.

Sign up for GitHub

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

Draft
simonbaird wants to merge 1 commit into
base: main
Choose a base branch
from
+20 −88
Draft

Skip keyless image signature checks in tekton tasks #3191

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
90 changes: 4 additions & 86 deletions features/__snapshots__/task_validate_image.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,14 +206,6 @@ true
"title": "Attestation syntax check passed"
}
},
{
"msg": "Pass",
"metadata": {
"code": "builtin.image.signature_check",
"description": "The image signature matches available signing materials.",
"title": "Image signature check passed"
}
},
{
"msg": "Pass",
"metadata": {
Expand Down Expand Up @@ -248,16 +240,6 @@ true
}
],
"success": true,
"signatures": [
{
"keyid": "",
"sig": ""
},
{
"keyid": "",
"sig": ""
}
],
"attestations": [
{
"type": "https://in-toto.io/Statement/v0.1",
Expand Down Expand Up @@ -306,7 +288,7 @@ true

[Keyless signing verification cosign v3 style:results - 1]
{
"TEST_OUTPUT": "{\"timestamp\":\"${TIMESTAMP}\",\"namespace\":\"\",\"successes\":5,\"failures\":0,\"warnings\":0,\"result\":\"SUCCESS\"}\n"
"TEST_OUTPUT": "{\"timestamp\":\"${TIMESTAMP}\",\"namespace\":\"\",\"successes\":4,\"failures\":0,\"warnings\":0,\"result\":\"SUCCESS\"}\n"
}
---

Expand Down Expand Up @@ -335,14 +317,6 @@ true
"title": "Attestation syntax check passed"
}
},
{
"msg": "Pass",
"metadata": {
"code": "builtin.image.signature_check",
"description": "The image signature matches available signing materials.",
"title": "Image signature check passed"
}
},
{
"msg": "Pass",
"metadata": {
Expand Down Expand Up @@ -377,26 +351,6 @@ true
}
],
"success": true,
"signatures": [
{
"keyid": "dc5f3121f1f76f0d687877532ce44ff55aab2050",
"sig": "MEUCIQDV4du9T+vV6dtN1LsCrZgByokRslw43oxscniN3wbaigIgMV+NFgix7ZjqhIpXFIMVFl1CQuya8JQsYP96ByA5iAc=",
"certificate": "-----BEGIN CERTIFICATE-----\nMIIC0zCCAlqgAwIBAgIUfPJP4pJfIr6Pgt2Q2J9hu4DqoJcwCgYIKoZIzj0EAwMw\nNzEVMBMGA1UEChMMc2lnc3RvcmUuZGV2MR4wHAYDVQQDExVzaWdzdG9yZS1pbnRl\ncm1lZGlhdGUwHhcNMjYwMzAzMTkxNjUyWhcNMjYwMzAzMTkyNjUyWjAAMFkwEwYH\nKoZIzj0CAQYIKoZIzj0DAQcDQgAEGMk9duvfPU07wcRpBWKXUi8bmr833N3pKhP2\nGCVBlFxZIRcD01FKT4TEMvlRIq8gZJO4eQ/WvEL/NpNmkk+PzaOCAXkwggF1MA4G\nA1UdDwEB/wQEAwIHgDATBgNVHSUEDDAKBggrBgEFBQcDAzAdBgNVHQ4EFgQU3F8x\nIfH3bw1oeHdTLORP9VqrIFAwHwYDVR0jBBgwFoAU39Ppz1YkEZb5qNjpKFWixi4Y\nZD8wKQYDVR0RAQH/BB8wHYEbY29uZm9ybWFjb21tdW5pdHlAZ21haWwuY29tMCkG\nCisGAQQBg78wAQEEG2h0dHBzOi8vYWNjb3VudHMuZ29vZ2xlLmNvbTArBgorBgEE\nAYO/MAEIBB0MG2h0dHBzOi8vYWNjb3VudHMuZ29vZ2xlLmNvbTCBigYKKwYBBAHW\neQIEAgR8BHoAeAB2AN09MGrGxxEyYxkeHJlnNwKiSl643jyt/4eKcoAvKe6OAAAB\nnLUhueMAAAQDAEcwRQIgARu6tEmE0vUHU+MhCQB6tzwROaEn4VdlfGBFWQxxcygC\nIQCHm2/lgszmmt2gC6Pl2bfvCRDKewUQDvWjzNqq8WtPczAKBggqhkjOPQQDAwNn\nADBkAjAMnyVwJVMQflB7Iwfte7cuOYYN2uvmEibKwjmmPgZOq43vSH9Y9gtUvyJk\nZ23vTpwCMHKChuWjhTQgxczH7MhKUO2IphbaHeJYmeFa4rrswhv6h9z6v5IIPovF\nsdbKg+sEHw==\n-----END CERTIFICATE-----\n",
"chain": [
"-----BEGIN CERTIFICATE-----\nMIICGjCCAaGgAwIBAgIUALnViVfnU0brJasmRkHrn/UnfaQwCgYIKoZIzj0EAwMw\nKjEVMBMGA1UEChMMc2lnc3RvcmUuZGV2MREwDwYDVQQDEwhzaWdzdG9yZTAeFw0y\nMjA0MTMyMDA2MTVaFw0zMTEwMDUxMzU2NThaMDcxFTATBgNVBAoTDHNpZ3N0b3Jl\nLmRldjEeMBwGA1UEAxMVc2lnc3RvcmUtaW50ZXJtZWRpYXRlMHYwEAYHKoZIzj0C\nAQYFK4EEACIDYgAE8RVS/ysH+NOvuDZyPIZtilgUF9NlarYpAd9HP1vBBH1U5CV7\n7LSS7s0ZiH4nE7Hv7ptS6LvvR/STk798LVgMzLlJ4HeIfF3tHSaexLcYpSASr1kS\n0N/RgBJz/9jWCiXno3sweTAOBgNVHQ8BAf8EBAMCAQYwEwYDVR0lBAwwCgYIKwYB\nBQUHAwMwEgYDVR0TAQH/BAgwBgEB/wIBADAdBgNVHQ4EFgQU39Ppz1YkEZb5qNjp\nKFWixi4YZD8wHwYDVR0jBBgwFoAUWMAeX5FFpWapesyQoZMi0CrFxfowCgYIKoZI\nzj0EAwMDZwAwZAIwPCsQK4DYiZYDPIaDi5HFKnfxXx6ASSVmERfsynYBiX2X6SJR\nnZU84/9DZdnFvvxmAjBOt6QpBlc4J/0DxvkTCqpclvziL6BCCPnjdlIB3Pu3BxsP\nmygUY7Ii2zbdCdliiow=\n-----END CERTIFICATE-----\n",
"-----BEGIN CERTIFICATE-----\nMIIB9zCCAXygAwIBAgIUALZNAPFdxHPwjeDloDwyYChAO/4wCgYIKoZIzj0EAwMw\nKjEVMBMGA1UEChMMc2lnc3RvcmUuZGV2MREwDwYDVQQDEwhzaWdzdG9yZTAeFw0y\nMTEwMDcxMzU2NTlaFw0zMTEwMDUxMzU2NThaMCoxFTATBgNVBAoTDHNpZ3N0b3Jl\nLmRldjERMA8GA1UEAxMIc2lnc3RvcmUwdjAQBgcqhkjOPQIBBgUrgQQAIgNiAAT7\nXeFT4rb3PQGwS4IajtLk3/OlnpgangaBclYpsYBr5i+4ynB07ceb3LP0OIOZdxex\nX69c5iVuyJRQ+Hz05yi+UF3uBWAlHpiS5sh0+H2GHE7SXrk1EC5m1Tr19L9gg92j\nYzBhMA4GA1UdDwEB/wQEAwIBBjAPBgNVHRMBAf8EBTADAQH/MB0GA1UdDgQWBBRY\nwB5fkUWlZql6zJChkyLQKsXF+jAfBgNVHSMEGDAWgBRYwB5fkUWlZql6zJChkyLQ\nKsXF+jAKBggqhkjOPQQDAwNpADBmAjEAj1nHeXZp+13NWBNa+EDsDP8G1WWg1tCM\nWP/WHPqpaVo0jhsweNFZgSs0eE7wYI4qAjEA2WB9ot98sIkoF3vZYdd3/VtWB5b9\nTNMea7Ix/stJ5TfcLLeABLE4BNJOsQ4vnBHJ\n-----END CERTIFICATE-----\n"
],
"metadata": {
"Fulcio Issuer": "https://accounts.google.com",
"Fulcio Issuer (V2)": "https://accounts.google.com",
"Issuer": "CN=sigstore-intermediate,O=sigstore.dev",
"Not After": "${TIMESTAMP}",
"Not Before": "${TIMESTAMP}",
"Serial Number": "7cf24fe2925f22be8f82dd90d89f61bb80eaa097",
"Subject Alternative Name": "Email Addresses:conformacommunity@gmail.com"
}
}
],
"attestations": [
{
"type": "https://in-toto.io/Statement/v0.1",
Expand Down Expand Up @@ -450,7 +404,7 @@ true

[Keyless signing verification cosign v2 style:results - 1]
{
"TEST_OUTPUT": "{\"timestamp\":\"${TIMESTAMP}\",\"namespace\":\"\",\"successes\":5,\"failures\":0,\"warnings\":0,\"result\":\"SUCCESS\"}\n"
"TEST_OUTPUT": "{\"timestamp\":\"${TIMESTAMP}\",\"namespace\":\"\",\"successes\":4,\"failures\":0,\"warnings\":0,\"result\":\"SUCCESS\"}\n"
}
---

Expand Down Expand Up @@ -559,14 +513,6 @@ results.tufUrl:
"title": "Attestation syntax check passed"
}
},
{
"msg": "Pass",
"metadata": {
"code": "builtin.image.signature_check",
"description": "The image signature matches available signing materials.",
"title": "Image signature check passed"
}
},
{
"msg": "Pass",
"metadata": {
Expand Down Expand Up @@ -601,26 +547,6 @@ results.tufUrl:
}
],
"success": true,
"signatures": [
{
"keyid": "dc5f3121f1f76f0d687877532ce44ff55aab2050",
"sig": "MEUCIQDV4du9T+vV6dtN1LsCrZgByokRslw43oxscniN3wbaigIgMV+NFgix7ZjqhIpXFIMVFl1CQuya8JQsYP96ByA5iAc=",
"certificate": "-----BEGIN CERTIFICATE-----\nMIIC0zCCAlqgAwIBAgIUfPJP4pJfIr6Pgt2Q2J9hu4DqoJcwCgYIKoZIzj0EAwMw\nNzEVMBMGA1UEChMMc2lnc3RvcmUuZGV2MR4wHAYDVQQDExVzaWdzdG9yZS1pbnRl\ncm1lZGlhdGUwHhcNMjYwMzAzMTkxNjUyWhcNMjYwMzAzMTkyNjUyWjAAMFkwEwYH\nKoZIzj0CAQYIKoZIzj0DAQcDQgAEGMk9duvfPU07wcRpBWKXUi8bmr833N3pKhP2\nGCVBlFxZIRcD01FKT4TEMvlRIq8gZJO4eQ/WvEL/NpNmkk+PzaOCAXkwggF1MA4G\nA1UdDwEB/wQEAwIHgDATBgNVHSUEDDAKBggrBgEFBQcDAzAdBgNVHQ4EFgQU3F8x\nIfH3bw1oeHdTLORP9VqrIFAwHwYDVR0jBBgwFoAU39Ppz1YkEZb5qNjpKFWixi4Y\nZD8wKQYDVR0RAQH/BB8wHYEbY29uZm9ybWFjb21tdW5pdHlAZ21haWwuY29tMCkG\nCisGAQQBg78wAQEEG2h0dHBzOi8vYWNjb3VudHMuZ29vZ2xlLmNvbTArBgorBgEE\nAYO/MAEIBB0MG2h0dHBzOi8vYWNjb3VudHMuZ29vZ2xlLmNvbTCBigYKKwYBBAHW\neQIEAgR8BHoAeAB2AN09MGrGxxEyYxkeHJlnNwKiSl643jyt/4eKcoAvKe6OAAAB\nnLUhueMAAAQDAEcwRQIgARu6tEmE0vUHU+MhCQB6tzwROaEn4VdlfGBFWQxxcygC\nIQCHm2/lgszmmt2gC6Pl2bfvCRDKewUQDvWjzNqq8WtPczAKBggqhkjOPQQDAwNn\nADBkAjAMnyVwJVMQflB7Iwfte7cuOYYN2uvmEibKwjmmPgZOq43vSH9Y9gtUvyJk\nZ23vTpwCMHKChuWjhTQgxczH7MhKUO2IphbaHeJYmeFa4rrswhv6h9z6v5IIPovF\nsdbKg+sEHw==\n-----END CERTIFICATE-----\n",
"chain": [
"-----BEGIN CERTIFICATE-----\nMIICGjCCAaGgAwIBAgIUALnViVfnU0brJasmRkHrn/UnfaQwCgYIKoZIzj0EAwMw\nKjEVMBMGA1UEChMMc2lnc3RvcmUuZGV2MREwDwYDVQQDEwhzaWdzdG9yZTAeFw0y\nMjA0MTMyMDA2MTVaFw0zMTEwMDUxMzU2NThaMDcxFTATBgNVBAoTDHNpZ3N0b3Jl\nLmRldjEeMBwGA1UEAxMVc2lnc3RvcmUtaW50ZXJtZWRpYXRlMHYwEAYHKoZIzj0C\nAQYFK4EEACIDYgAE8RVS/ysH+NOvuDZyPIZtilgUF9NlarYpAd9HP1vBBH1U5CV7\n7LSS7s0ZiH4nE7Hv7ptS6LvvR/STk798LVgMzLlJ4HeIfF3tHSaexLcYpSASr1kS\n0N/RgBJz/9jWCiXno3sweTAOBgNVHQ8BAf8EBAMCAQYwEwYDVR0lBAwwCgYIKwYB\nBQUHAwMwEgYDVR0TAQH/BAgwBgEB/wIBADAdBgNVHQ4EFgQU39Ppz1YkEZb5qNjp\nKFWixi4YZD8wHwYDVR0jBBgwFoAUWMAeX5FFpWapesyQoZMi0CrFxfowCgYIKoZI\nzj0EAwMDZwAwZAIwPCsQK4DYiZYDPIaDi5HFKnfxXx6ASSVmERfsynYBiX2X6SJR\nnZU84/9DZdnFvvxmAjBOt6QpBlc4J/0DxvkTCqpclvziL6BCCPnjdlIB3Pu3BxsP\nmygUY7Ii2zbdCdliiow=\n-----END CERTIFICATE-----\n",
"-----BEGIN CERTIFICATE-----\nMIIB9zCCAXygAwIBAgIUALZNAPFdxHPwjeDloDwyYChAO/4wCgYIKoZIzj0EAwMw\nKjEVMBMGA1UEChMMc2lnc3RvcmUuZGV2MREwDwYDVQQDEwhzaWdzdG9yZTAeFw0y\nMTEwMDcxMzU2NTlaFw0zMTEwMDUxMzU2NThaMCoxFTATBgNVBAoTDHNpZ3N0b3Jl\nLmRldjERMA8GA1UEAxMIc2lnc3RvcmUwdjAQBgcqhkjOPQIBBgUrgQQAIgNiAAT7\nXeFT4rb3PQGwS4IajtLk3/OlnpgangaBclYpsYBr5i+4ynB07ceb3LP0OIOZdxex\nX69c5iVuyJRQ+Hz05yi+UF3uBWAlHpiS5sh0+H2GHE7SXrk1EC5m1Tr19L9gg92j\nYzBhMA4GA1UdDwEB/wQEAwIBBjAPBgNVHRMBAf8EBTADAQH/MB0GA1UdDgQWBBRY\nwB5fkUWlZql6zJChkyLQKsXF+jAfBgNVHSMEGDAWgBRYwB5fkUWlZql6zJChkyLQ\nKsXF+jAKBggqhkjOPQQDAwNpADBmAjEAj1nHeXZp+13NWBNa+EDsDP8G1WWg1tCM\nWP/WHPqpaVo0jhsweNFZgSs0eE7wYI4qAjEA2WB9ot98sIkoF3vZYdd3/VtWB5b9\nTNMea7Ix/stJ5TfcLLeABLE4BNJOsQ4vnBHJ\n-----END CERTIFICATE-----\n"
],
"metadata": {
"Fulcio Issuer": "https://accounts.google.com",
"Fulcio Issuer (V2)": "https://accounts.google.com",
"Issuer": "CN=sigstore-intermediate,O=sigstore.dev",
"Not After": "${TIMESTAMP}",
"Not Before": "${TIMESTAMP}",
"Serial Number": "7cf24fe2925f22be8f82dd90d89f61bb80eaa097",
"Subject Alternative Name": "Email Addresses:conformacommunity@gmail.com"
}
}
],
"attestations": [
{
"type": "https://in-toto.io/Statement/v0.1",
Expand Down Expand Up @@ -674,7 +600,7 @@ results.tufUrl:

[Keyless signing verification cosign v2 style with regexp params:results - 1]
{
"TEST_OUTPUT": "{\"timestamp\":\"${TIMESTAMP}\",\"namespace\":\"\",\"successes\":5,\"failures\":0,\"warnings\":0,\"result\":\"SUCCESS\"}\n"
"TEST_OUTPUT": "{\"timestamp\":\"${TIMESTAMP}\",\"namespace\":\"\",\"successes\":4,\"failures\":0,\"warnings\":0,\"result\":\"SUCCESS\"}\n"
}
---

Expand All @@ -694,14 +620,6 @@ results.tufUrl:
"description": "The attestation signature matches available signing materials.",
"title": "Attestation signature check passed"
}
},
{
"msg": "Image signature check failed: no matching attestations: failed to verify certificate identity: no matching CertificateIdentity found, last error: expected SAN value to match regex \"^konformakommunity@\", got \"conformacommunity@gmail.com\"\nfailed to verify certificate identity: no matching CertificateIdentity found, last error: expected SAN value to match regex \"^konformakommunity@\", got \"conformacommunity@gmail.com\"",
"metadata": {
"code": "builtin.image.signature_check",
"description": "The image signature matches available signing materials.",
"title": "Image signature check passed"
}
}
],
"success": false
Expand Down Expand Up @@ -731,6 +649,6 @@ results.tufUrl:

[Keyless signing verification cosign v3 style with regexp params:results - 1]
{
"TEST_OUTPUT": "{\"timestamp\":\"${TIMESTAMP}\",\"namespace\":\"\",\"successes\":0,\"failures\":2,\"warnings\":0,\"result\":\"FAILURE\"}\n"
"TEST_OUTPUT": "{\"timestamp\":\"${TIMESTAMP}\",\"namespace\":\"\",\"successes\":0,\"failures\":1,\"warnings\":0,\"result\":\"FAILURE\"}\n"
}
---
9 changes: 8 additions & 1 deletion tasks/verify-conforma-konflux-ta/0.1/verify-conforma-konflux-ta.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -371,9 +371,16 @@ spec:
)
fi

# Force --ignore-rekor to false since we need rekor
cmd_args+=(
# Force --ignore-rekor to false since we need rekor
--ignore-rekor=false

# Conforma expects that the attestation and the image are signed in the
# same way. But actually for Tekton Chains doing keyless signing that's
Copy link
Contributor

@lcarva lcarva Mar 24, 2026

Choose a reason for hiding this comment

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

This is not entirely true. The same identity is used for the image signature and attestation signature. The change that we're pursuing in Konflux is that we want to make Chains stop signing the image altogether and instead sign the image from the build pipeline itself (tektoncd/chains#1346). That is unrelated to keyless. We could achieve the same with long-lived keys - keyless just makes this much more feasible.

# not the cases. As a workaround, skip the image sig check and only do the
# attestation sig check. This means we're expecting the --certificate-*
# params provided to be applicable for verifying the attestation signature.
--skip-image-sig-check=true
)
Comment on lines +375 to 384
Copy link

@coderabbitai coderabbitai bot Mar 24, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Avoid unconditional --skip-image-sig-check in keyless mode.

Line 383 applies the same implicit weakening of verification guarantees: any keyless run skips image signature checks. This should be controlled by an explicit task param (default false) so callers intentionally opt into this reduced validation mode.

As per coding guidelines, **: -Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@tasks/verify-conforma-konflux-ta/0.1/verify-conforma-konflux-ta.yaml` around
lines 375 - 384, Remove the unconditional --skip-image-sig-check=true flag and
instead add an explicit task parameter (e.g., params.skipImageSigCheck, default
false) to the task definition, then conditionally append the
--skip-image-sig-check=true argument to the Conforma step args only when
params.skipImageSigCheck is true (so callers must opt in for reduced
validation); reference the existing --skip-image-sig-check flag and the task
params block when making the change.

Copy link
Member Author

@simonbaird simonbaird Mar 24, 2026

Choose a reason for hiding this comment

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

It's a reasonable suggestion, but I don't want to add that right now. I know the only use case for this task is with Tekton Chains produced signatures, and I know (or at least I'm fairly certain) we'll have to skip the image sig check for those.

I don't want to force consumers of these tasks (i.e. the Konflux release pipeline) to have to introduce a new param to make things work. I like the idea of making "default to more secure", but for now we're preferring "default to most likely to work".

Copy link

@coderabbitai coderabbitai bot Mar 24, 2026

Choose a reason for hiding this comment

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

@simonbaird, understood — the trade-off makes sense given the fixed use case. Deferring to the PR author here.

📝 Learnings were identified and not saved (knowledge base disabled). Enable

else
# Assume traditional signing secret verification
Expand Down
9 changes: 8 additions & 1 deletion tasks/verify-enterprise-contract/0.1/verify-enterprise-contract.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,9 +316,16 @@ spec:
)
fi

# Force --ignore-rekor to false since we need rekor
cmd_args+=(
# Force --ignore-rekor to false since we need rekor
--ignore-rekor=false

# Conforma expects that the attestation and the image are signed in the
# same way. But actually for Tekton Chains doing keyless signing that's
# not the cases. As a workaround, skip the image sig check and only do the
# attestation sig check. This means we're expecting the --certificate-*
# params provided to be applicable for verifying the attestation signature.
--skip-image-sig-check=true
Copy link
Contributor

@qodo-code-review qodo-code-review bot Mar 24, 2026

Choose a reason for hiding this comment

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

Action required

1. Image signature verification disabled 🐞 Bug ⛨ Security

Both tasks now unconditionally add --skip-image-sig-check=true whenever any keyless
CERTIFICATE_* param is set, disabling image signature validation and allowing images with
invalid/missing signatures to pass as long as attestation verification succeeds.
Agent Prompt 
### Issue description
The Tekton tasks currently add `--skip-image-sig-check=true` automatically for any keyless verification configuration, which disables image signature validation in a broad, implicit way.

### Issue Context
The CLI supports `--skip-image-sig-check` and it bypasses image signature validation (`ValidateImageSignature`). Repo tests show this can make validation succeed even when the image signature is invalid.

### How to fix
- Add a **new task param** (e.g., `SKIP_IMAGE_SIG_CHECK`, default `"false"`).
- Only append `--skip-image-sig-check` when that param is true (and ideally emit a clear log line warning about the security tradeoff).
- Update both task READMEs to document the param and the security implication.

### Fix Focus Areas
- tasks/verify-enterprise-contract/0.1/verify-enterprise-contract.yaml[292-336]
- tasks/verify-conforma-konflux-ta/0.1/verify-conforma-konflux-ta.yaml[344-391]
- tasks/verify-enterprise-contract/0.1/README.md[1-60]
- tasks/verify-conforma-konflux-ta/0.1/README.md[1-80]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

)
Comment on lines +320 to 329
Copy link

@coderabbitai coderabbitai bot Mar 24, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Make image-signature skipping explicit instead of unconditional.

In Line 328, keyless mode now always disables image signature verification. That’s a security guarantee downgrade for all existing keyless users and should be an explicit opt-in task parameter (or a fail-closed path) rather than implicit behavior.

Suggested hardening (opt-in switch) 
+    - name: SKIP_IMAGE_SIG_CHECK
+      type: string
+      description: >-
+        Allow keyless verification without image signature verification.
+        Use only when image and attestation are expected to use different identities.
+      default: "false"
...
-          cmd_args+=(
-            # Force --ignore-rekor to false since we need rekor
-            --ignore-rekor=false
-
-            # Conforma expects that the attestation and the image are signed in the
-            # same way. But actually for Tekton Chains doing keyless signing that's
-            # not the cases. As a workaround, skip the image sig check and only do the
-            # attestation sig check. This means we're expecting the --certificate-*
-            # params provided to be applicable for verifying the attestation signature.
-            --skip-image-sig-check=true
-          )
+          cmd_args+=(
+            # Force --ignore-rekor to false since we need rekor
+            --ignore-rekor=false
+          )
+          if [[ "${SKIP_IMAGE_SIG_CHECK}" == "true" ]]; then
+            cmd_args+=(
+              --skip-image-sig-check=true
+            )
+          fi
...
+        - name: SKIP_IMAGE_SIG_CHECK
+          value: "$(params.SKIP_IMAGE_SIG_CHECK)"

As per coding guidelines, **: -Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@tasks/verify-enterprise-contract/0.1/verify-enterprise-contract.yaml` around
lines 320 - 329, The task unconditionally sets --skip-image-sig-check=true for
keyless signing, weakening security; update verify-enterprise-contract.yaml to
make image signature skipping explicit and opt-in: introduce a task parameter
(e.g., skipImageSigCheck or allowKeylessImageSkip) defaulting to false, wire
that parameter into the command generation instead of hardcoding
--skip-image-sig-check=true, and if keyless mode is detected without the opt-in
parameter set, fail-closed (return an error/exit) to force explicit
authorization; ensure you modify the code that emits the --skip-image-sig-check
flag so it only appears when the new parameter is true and keep the explanatory
comment near the flag.

Copy link
Member Author

@simonbaird simonbaird Mar 24, 2026

Choose a reason for hiding this comment

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

Same response as for the other similar suggestion, though honestly it would be easier here, since we own the integration test pipeline where this is used. Still, I'd rather be consistent with the other task.

Copy link

@coderabbitai coderabbitai bot Mar 24, 2026

Choose a reason for hiding this comment

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

@simonbaird, understood — consistency between the two tasks is a reasonable priority. I'll defer to your judgment here.

📝 Learnings were identified and not saved (knowledge base disabled). Enable

else
# Assume traditional signing secret verification
Expand Down
Loading