Skip to content

Bug: MarkSigned called despite per-artifact signing failures #1663

@vdemeester

Description

@vdemeester

Summary

Per-artifact failures in ObjectSigner.Sign() (pkg/chains/signing.go) are logged and continued without contributing to the aggregate error (merr). If all per-artifact operations fail silently, merr remains nil, and MarkSigned is called — permanently marking the resource as chains.tekton.dev/signed=true despite no artifacts being actually signed.

Bug

Three failure paths in the signing loop use continue without appending to merr:

  1. Payload creation failure (L148-152) — CreatePayload error → logged → continue
  2. Payload marshal failuregetRawPayload error → logged → continue
  3. Signing failureSignMessage error → logged → continue

Only storage failures correctly aggregate into merr.

Impact

  • chains.tekton.dev/signed=true is set when no artifacts were signed
  • signed=true is a terminal marker — suppresses retries on subsequent reconciliation
  • Errors are only logged, not surfaced to the resource

Reproduction

Create a TaskRun with a malformed IMAGE_DIGEST result. Chains logs the error at signable.go:176 ("unsupported digest algorithm") but sets signed=true anyway.

Fix

Aggregate per-artifact errors into merr:

// Example for payload creation path:
payload, err := payloader.CreatePayload(ctx, obj)
if err != nil {
    logger.Error(err)
    o.recordError(ctx, signableType, metrics.PayloadCreationError)
    merr = multierror.Append(merr, fmt.Errorf("payload creation for %s: %w", signableType.Type(), err))
    continue
}

Same pattern for marshal and signing paths. This ensures MarkSigned is only called when all artifacts are successfully processed.

/kind bug

Metadata

Metadata

Assignees

No one assigned

    Labels

    kind/bugCategorizes issue or PR as related to a bug.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions