Skip to content

Conversation

@zakisk
Copy link

@zakisk zakisk commented Nov 25, 2025

This commit introduces content hash verification for the HTTP resolver by adding support for optional digest validation using SHA256 and SHA512 algorithms.

Changes:

  • Add new 'digest' parameter accepting ':' format where algorithm can be 'sha256' or 'sha512'
  • Implement digest validation logic using constant-time comparison to prevent timing-based side-channel attacks
  • Add comprehensive unit tests covering valid matches, mismatches, invalid formats, and unsupported algorithms
  • Add E2E tests to verify digest validation in real cluster scenarios
  • Enable 'enable-http-resolver' feature flag in default configuration
  • Update documentation with digest parameter description, usage examples, and commands to calculate SHA256/SHA512 hashes

Security considerations:

  • Uses constant-time comparison to prevent timing attacks
  • Digest validation is optional to maintain backward compatibility
  • Digest values are logged for debugging

Fixes: #8759

Changes

Submitter Checklist

As the author of this PR, please check off the items in this checklist:

  • Has Docs if any changes are user facing, including updates to minimum requirements e.g. Kubernetes version bumps
  • Has Tests included if any functionality added or changed
  • pre-commit Passed
  • Follows the commit message standard
  • Meets the Tekton contributor standards (including functionality, content, code)
  • Has a kind label. You can add one by adding a comment on this PR that contains /kind <type>. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tep
  • Release notes block below has been updated with any user facing changes (API changes, bug fixes, changes requiring upgrade notices or deprecation warnings). See some examples of good release notes.
  • Release notes contains the string "action required" if the change requires additional action from users switching to the new release

Release Notes

The HTTP resolver now supports optional content integrity verification through a new `digest` parameter. Users can provide a cryptographic hash in the format `<algorithm>:<hash>` (where algorithm is either `sha256` or `sha512`) to verify that the fetched content matches the expected value. For example, `digest: sha256:f37cdd0e8611932bca52eed219c2b273161501510b37218fb79ca0a6a4487cff`. This parameter is optional and currently enabled by default, ensuring backward compatibility with existing HTTP resolver configurations.

@tekton-robot tekton-robot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Nov 25, 2025
@tekton-robot tekton-robot requested review from dibyom and jerop November 25, 2025 06:54
@tekton-robot tekton-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Nov 25, 2025
@zakisk
Copy link
Author

zakisk commented Nov 25, 2025

/kind feature

@tekton-robot tekton-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Nov 25, 2025
@waveywaves waveywaves added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Nov 25, 2025
@zakisk zakisk force-pushed the calculate-hash-of-content-in-http-resolver branch 3 times, most recently from 68fd341 to d6f4f58 Compare November 28, 2025 03:40
@zakisk
Copy link
Author

zakisk commented Nov 28, 2025

I think that one E2E job failure is flake because before there was three jobs were failing

@zakisk zakisk force-pushed the calculate-hash-of-content-in-http-resolver branch 5 times, most recently from 076f32b to 9c6db5d Compare December 3, 2025 09:58
@zakisk zakisk force-pushed the calculate-hash-of-content-in-http-resolver branch 3 times, most recently from 3a6c745 to 0b02160 Compare December 9, 2025 10:19
@tekton-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: vdemeester

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@tekton-robot tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 9, 2025
@zakisk
Copy link
Author

zakisk commented Dec 9, 2025

@vdemeester I guess E2E failure is flake because they are not consistent, yeah?

@vdemeester
Copy link
Member

@vdemeester I guess E2E failure is flake because they are not consistent, yeah?

Yes, working on fixing it but it might take some times. One thing as well, e2e tests that change the configmaps are making the things flaky (because most things run in parallels) - I am working on fixing this as well.

@zakisk
Copy link
Author

zakisk commented Dec 11, 2025

Yes, working on fixing it but it might take some times. One thing as well, e2e tests that change the configmaps are making the things flaky (because most things run in parallels) - I am working on fixing this as well.

@vdemeester I think #9224 fixes that, right?

@zakisk zakisk force-pushed the calculate-hash-of-content-in-http-resolver branch from 0b02160 to 969fb68 Compare December 11, 2025 05:54
@vdemeester
Copy link
Member

Yes, working on fixing it but it might take some times. One thing as well, e2e tests that change the configmaps are making the things flaky (because most things run in parallels) - I am working on fixing this as well.

@vdemeester I think #9224 fixes that, right?

It is trying yes.

@waveywaves
Copy link
Member

/retest

Copy link
Member

@waveywaves waveywaves left a comment

Choose a reason for hiding this comment

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

Added suggestions with a doc update and early validations to fail fast before a comparatively compute heavy call

@zakisk zakisk force-pushed the calculate-hash-of-content-in-http-resolver branch 2 times, most recently from 4f79992 to 3c94798 Compare December 22, 2025 04:26
@vdemeester
Copy link
Member

ah @zakisk

Running tests with 'go test -race -v  -count=1 -tags=e2e -timeout=40m ./test/... '
Error: found 2 test(s) without @test:execution annotation:
  - TestHttpResolver (resolvers_test.go:474)
  - TestHttpResolver_Failure (resolvers_test.go:510)

You need to mark those tests, see here.

@zakisk zakisk force-pushed the calculate-hash-of-content-in-http-resolver branch 2 times, most recently from 1b89317 to 580e811 Compare December 22, 2025 09:35
This commit introduces content hash verification for the HTTP
resolver by adding support for optional digest validation using
SHA256 and SHA512 algorithms.

Changes:
- Add new 'digest' parameter accepting '<algorithm>:<hash>' format where
  algorithm can be 'sha256' or 'sha512'
- Implement digest validation logic using constant-time comparison to
  prevent timing-based side-channel attacks
- Add comprehensive unit tests covering valid matches, mismatches,
  invalid formats, and unsupported algorithms
- Add E2E tests to verify digest validation in real cluster scenarios
- Enable 'enable-http-resolver' feature flag in default configuration
- Update documentation with digest parameter description, usage examples,
  and commands to calculate SHA256/SHA512 hashes

Security considerations:
- Uses constant-time comparison to prevent timing attacks
- Digest validation is optional to maintain backward compatibility
- Digest values are logged for debugging

Fixes: tektoncd#8759

Signed-off-by: Zaki Shaikh <[email protected]>
@zakisk zakisk force-pushed the calculate-hash-of-content-in-http-resolver branch from 580e811 to fedb2ce Compare December 22, 2025 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/feature Categorizes issue or PR as related to a new feature. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

Extend HTTP Resolver spec to support optional hash field

4 participants