Skip to content

Conversation

@jkhelil
Copy link
Member

@jkhelil jkhelil commented Nov 4, 2025

Unit tests are failing for fulcio,
I don’t fully understand all the downstream impacts here, but I do understand the root issue. With GPT’s help, I was able to fix it to unblock the release.
Could someone with Fulcio expertise please review and validate this change?

➜  chains git:(main) ✗ go test -v -run "^TestCreateSignerFulcioEnabled$" ./pkg/chains/signing/x509/
=== RUN   TestCreateSignerFulcioEnabled
    logger.go:146: 2025-11-04T20:58:26.666+0100	INFO	x509/x509.go:99	Attempting to get id token from provider filesystem-custom-path
    logger.go:146: 2025-11-04T20:58:26.666+0100	INFO	x509/x509.go:116	Signing with fulcio ...
    x509_test.go:101: Error from NewSigner: new signer: reading id token: getting id token: open eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpYXQiOjE2Nzc1NjAyMTgsImV4cCI6MTY3NzU2MzgxOCwiaXNzIjoidXNlcjEyMyJ9.c-sDgCyuZA6VaIGl7Y3-9XxttW1PUkBeNBLE9gCKG8s: no such file or directory
    x509_test.go:110: new signer: reading id token: getting id token: open eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpYXQiOjE2Nzc1NjAyMTgsImV4cCI6MTY3NzU2MzgxOCwiaXNzIjoidXNlcjEyMyJ9.c-sDgCyuZA6VaIGl7Y3-9XxttW1PUkBeNBLE9gCKG8s: no such file or directory
--- FAIL: TestCreateSignerFulcioEnabled (0.00s)
FAIL
FAIL	github.com/tektoncd/chains/pkg/chains/signing/x509	2.358s
FAIL

This PR fixes Fulcio token handling in pkg/chains/signing/x509/x509.go where a raw JWT string could be treated as a filesystem path, causing file-open errors during signer initialization.

  • When signers.x509.identity.token.file is set, pass the file path via options.KeyOpts.IDToken so Fulcio reads the token from disk.
  • Treat providers as enabled when a token file is provided; only validate a provider when explicitly set.
  • Let Fulcio handle token acquisition (providers/env/OIDC) when no token file is specified.
  • Clean up unused variable and related assignments.

Co-authored-by: GPT-5

Changes

Submitter Checklist

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

  • Has Docs included if any changes are user facing
  • Has Tests included if any functionality added or changed
  • Follows the commit message standard
  • Meets the Tekton contributor standards (including
    functionality, content, code)
  • Release notes block below has been updated with any user facing changes (API changes, bug fixes, changes requiring upgrade notices or deprecation warnings)
  • Release notes contains the string "action required" if the change requires additional action from users switching to the new release

Release Notes

NONE

@tekton-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please ask for approval from jkhelil after the PR has been reviewed.

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

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 size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Nov 4, 2025
@anithapriyanatarajan
Copy link
Contributor

anithapriyanatarajan commented Nov 5, 2025

@jkhelil x509_test.go:101: Error from NewSigner: new signer: reading id token: getting id token: open eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpYXQiOjE2Nzc1NjAyMTgsImV4cCI6MTY3NzU2MzgxOCwiaXNzIjoidXNlcjEyMyJ9.c-sDgCyuZA6VaIGl7Y3-9XxttW1PUkBeNBLE9gCKG8s: no such file or directory this error could be attributed to #1441 that involved a bump of cosign from 2.5.3 to 2.6.0. This has introduced a new auth flow at here.

// idToken allows users to either pass in an identity token directly
// or a path to an identity token via the --identity-token flag
func idToken(s string) (string, error) {
	// If this is a valid raw token or is empty, just return it
	if _, err := jwt.ParseSigned(s, []jose.SignatureAlgorithm{"RS256"}); err == nil || s == "" {
		return s, nil
	}

	// Otherwise, if this is a path to a token return the contents
	c, err := os.ReadFile(s)
	return string(c), err
}

Either the function should be enhanced to accept other signing algorithms or should be made configurable, considering the PQC algorithms we may have to support. The failing test case uses -a HS256 . To get past this immediately we could update the test case by generating a RS256 algorithm token. As a long term fix, the above mentioned function cosign should be refactored to handle token failures and file path handling in a better way.

If updating the token to RS algorithm works, Ideally we do not need the logic change in x509.go since the code relies on the provider abstraction p, err := providers.ProvideFrom(ctx, cfg.FulcioProvider) which should be fine.

Submitted PR #1464 with the token change for reference.

@jkhelil jkhelil closed this Nov 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants