Add Attestation Repair action In Avo #6186

colby-swandale · 2026-01-08T04:40:38Z

This is intended to succeed #6141 by moving the new repair functionality (kept the same from #6141) into Avo so that rubygems.org operators will be able to view the status of Gem Attestations and initiate a repair when applicable.

…station repairs Co-authored-by: Ben Fischer <[email protected]>

test/unit/avo/actions/repair_attestation_test.rb

+      body: {
+        "verificationMaterial" => {
+          "tlogEntries" => [{ "logIndex" => 123 }],
+          "certificate" => { "rawBytes" => Base64.strict_encode64("DER data") }
+        }
+      }


test/unit/avo/actions/repair_attestation_test.rb

+      body: {
+        "verificationMaterial" => {
+          "tlogEntries" => [{ "kindVersion" => { "kind" => "dsse", "version" => "0.0.1" } }],
+          "certificate" => { "rawBytes" => double_encoded }
+        }
+      }


test/unit/avo/actions/repair_attestation_test.rb

+      body: {
+        "verificationMaterial" => {
+          "tlogEntries" => [{ "kindVersion" => { "kind" => "dsse", "version" => "0.0.1" } }],
+          "certificate" => { "rawBytes" => Base64.strict_encode64("DER data") }
+        }
+      }


test/unit/avo/actions/repair_attestation_test.rb

+      body: {
+        "verificationMaterial" => {
+          "tlogEntries" => [{ "logIndex" => 123 }],
+          "certificate" => { "rawBytes" => double_encoded }
+        }
+      }


Copilot

Pull request overview

This pull request adds attestation repair functionality to the Avo admin interface, allowing rubygems.org operators to view the status of gem attestations and initiate repairs for known issues. The functionality repairs two specific issues in Sigstore bundles: missing kindVersion fields in transaction log entries and double-encoded PEM certificates.

Key changes:

Introduces an AttestationBundleRepair concern with detection and repair logic for invalid attestations
Adds a valid_bundle? method to check if an attestation can be successfully parsed
Creates an Avo action (RepairAttestation) for operators to manually trigger repairs
Adds comprehensive test coverage for the repair functionality

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 11 comments.

Show a summary per file

File	Description
app/models/concerns/attestation_bundle_repair.rb	New concern implementing repair logic for two known attestation bundle issues
app/models/attestation.rb	Includes the repair concern and adds a `valid_bundle?` helper method
app/avo/resources/attestation.rb	Adds repair action and validity indicator field to the Avo resource
app/avo/actions/repair_attestation.rb	Avo action for triggering attestation repairs with appropriate visibility controls
test/models/attestation_test.rb	Tests for the `repairable?` method covering various scenarios
test/unit/avo/actions/repair_attestation_test.rb	Comprehensive tests for the repair action functionality

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-08T04:45:08Z

app/models/attestation.rb

+  def valid_bundle?
+    sigstore_bundle.present?
+  rescue Sigstore::Error
+    false
+  end


The new valid_bundle? method in the Attestation model lacks test coverage. Consider adding tests to verify that it correctly returns true for valid bundles and false for invalid bundles (both for Sigstore::Error and other potential exceptions).

Copilot · 2026-01-08T04:45:08Z

app/avo/resources/attestation.rb

    field :id, as: :id

+    field :valid, as: :boolean, only_on: %i[index show] do
+      record.valid_bundle?


The valid field calls record.valid_bundle? on every attestation displayed on the index and show pages. This method parses the entire Sigstore bundle for each record, which could cause performance issues when displaying many attestations on the index page. Consider adding caching or computing this value asynchronously if performance becomes a concern.

Suggested change

record.valid_bundle?

Rails.cache.fetch(["attestation-valid-bundle", record.id, record.updated_at]) do

record.valid_bundle?

end

Copilot · 2026-01-08T04:45:09Z

test/models/attestation_test.rb

+
+      refute_predicate attestation, :repairable?
+    end
+  end


The repair! method from the AttestationBundleRepair concern is only tested indirectly through the Avo action tests. Consider adding direct unit tests for the repair! method in test/models/attestation_test.rb to ensure it properly modifies the attestation body and returns the expected changes array.

Copilot · 2026-01-08T04:45:09Z

test/models/attestation_test.rb

+    should "be repairable when verificationMaterial is missing" do
+      attestation = Attestation.new(
+        version: @version,
+        media_type: "application/vnd.dev.sigstore.bundle.v0.3+json",
+        body: {}
+      )
+
+      refute_predicate attestation, :repairable?
+    end


The test description "be repairable when verificationMaterial is missing" doesn't match the assertion. The test expects refute_predicate attestation, :repairable? (i.e., NOT repairable), but the description says it should "be repairable". Either the description should be changed to "not be repairable when verificationMaterial is missing" or the assertion should be changed to assert_predicate.

Copilot · 2026-01-08T04:45:09Z

test/unit/avo/actions/repair_attestation_test.rb

+  test "repairs attestation with missing kindVersion" do
+    attestation = Attestation.create!(
+      version: @version,
+      media_type: "application/vnd.dev.sigstore.bundle.v0.3+json",
+      body: {
+        "verificationMaterial" => {
+          "tlogEntries" => [{ "logIndex" => 123 }],
+          "certificate" => { "rawBytes" => Base64.strict_encode64("DER data") }
+        }
+      }
+    )
+
+    assert_predicate attestation, :repairable?
+
+    action = Avo::Actions::RepairAttestation.new
+    action.handle(
+      fields: { comment: "Repairing invalid attestation" },
+      current_user: @admin,
+      resource: nil,
+      records: [attestation],
+      query: nil
+    )
+
+    attestation.reload
+
+    refute_predicate attestation, :repairable?
+    assert_equal(
+      { "kind" => "dsse", "version" => "0.0.1" },
+      attestation.body.dig("verificationMaterial", "tlogEntries", 0, "kindVersion")
+    )
+  end


The tests do not verify that audit records are created when the repair action is executed. Based on the ApplicationAction base class implementation, all Avo actions should create audit records. Consider adding assertions to check that audit records are created and contain the expected information, similar to other action tests in the codebase.

Copilot · 2026-01-08T04:45:10Z

app/models/concerns/attestation_bundle_repair.rb

+module AttestationBundleRepair
+  extend ActiveSupport::Concern
+
+  def repairable?
+    verification = body["verificationMaterial"]
+    return false if verification.blank?
+
+    missing_kind_version?(verification) || double_encoded_certificate?(verification)
+  end
+
+  def repair!
+    verification = body["verificationMaterial"]
+    return false if verification.blank?
+
+    new_body = body.deep_dup
+    new_verification = new_body["verificationMaterial"]
+    changes = []
+
+    repair_kind_version!(new_verification, changes) if missing_kind_version?(verification)
+    repair_certificate!(new_verification, changes) if double_encoded_certificate?(verification)
+
+    return false if changes.empty?
+
+    update!(body: new_body)
+    changes
+  end


The AttestationBundleRepair concern and its public methods (repairable? and repair!) lack documentation. Consider adding comments or docstrings that explain what conditions make an attestation repairable and what changes the repair method makes.

Copilot · 2026-01-08T04:45:10Z

app/models/concerns/attestation_bundle_repair.rb

+  def repair_kind_version!(verification, changes)
+    verification["tlogEntries"]&.each_with_index do |entry, idx|
+      next if entry.key?("kindVersion")
+      entry["kindVersion"] = { "kind" => "dsse", "version" => "0.0.1" }


The hardcoded values for kindVersion ("kind" => "dsse", "version" => "0.0.1") lack explanation or documentation about why these specific values are used. Consider adding a comment explaining that these are the correct default values for Sigstore bundles or extracting them as constants with descriptive names.

Copilot · 2026-01-08T04:45:10Z

test/models/attestation_test.rb

+      assert_predicate attestation, :repairable?
+    end
+
+    should "be not repairable when bundle has no known issues" do


The test description "be not repairable" contains awkward grammar. It should be "not be repairable" without the extra "be".

Suggested change

should "be not repairable when bundle has no known issues" do

should "not be repairable when bundle has no known issues" do

Copilot · 2026-01-08T04:45:11Z

test/unit/avo/actions/repair_attestation_test.rb

+  test "repairs attestation with double-encoded PEM certificate" do
+    key = OpenSSL::PKey::RSA.new(2048)
+    cert = OpenSSL::X509::Certificate.new
+    cert.version = 2
+    cert.serial = 1
+    cert.subject = OpenSSL::X509::Name.parse("/CN=Test")
+    cert.issuer = cert.subject
+    cert.public_key = key.public_key
+    cert.not_before = Time.current
+    cert.not_after = Time.current + 3600
+    cert.sign(key, OpenSSL::Digest.new("SHA256"))
+
+    pem_cert = cert.to_pem
+    double_encoded = Base64.strict_encode64(pem_cert)
+
+    attestation = Attestation.create!(
+      version: @version,
+      media_type: "application/vnd.dev.sigstore.bundle.v0.3+json",
+      body: {
+        "verificationMaterial" => {
+          "tlogEntries" => [{ "kindVersion" => { "kind" => "dsse", "version" => "0.0.1" } }],
+          "certificate" => { "rawBytes" => double_encoded }
+        }
+      }
+    )
+
+    assert_predicate attestation, :repairable?
+
+    action = Avo::Actions::RepairAttestation.new
+    action.handle(
+      fields: { comment: "Repairing invalid attestation" },
+      current_user: @admin,
+      resource: nil,
+      records: [attestation],
+      query: nil
+    )
+
+    attestation.reload
+
+    refute_predicate attestation, :repairable?
+
+    raw_bytes = attestation.body.dig("verificationMaterial", "certificate", "rawBytes")
+    decoded = Base64.strict_decode64(raw_bytes)
+
+    refute decoded.start_with?("-----BEGIN CERTIFICATE-----")
+  end


There is no test coverage for the error handling path in the repair_certificate! method (lines 61-63). Consider adding a test case that verifies behavior when an invalid PEM certificate is provided that causes OpenSSL::X509::Certificate.new to raise an exception.

Copilot · 2026-01-08T04:45:11Z

app/models/attestation.rb


+  def valid_bundle?
+    sigstore_bundle.present?
+  rescue Sigstore::Error


The valid_bundle? method only rescues Sigstore::Error, but other exceptions (like ArgumentError from Base64 decoding issues, or other unexpected errors) would propagate and cause the field display to fail. Consider rescuing StandardError or at least additional specific exception types that could occur during bundle creation to make the validity check more robust.

Suggested change

rescue Sigstore::Error

rescue StandardError => e

Rails.logger.warn("Failed to validate sigstore bundle for Attestation #{id}: #{e.class}: #{e.message}") if defined?(Rails) && Rails.respond_to?(:logger) && Rails.logger

codecov · 2026-01-08T04:48:48Z

Codecov Report

❌ Patch coverage is 90.32258% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 97.19%. Comparing base (d05f69e) to head (bf85b70).

Files with missing lines	Patch %	Lines
app/avo/actions/repair_attestation.rb	80.00%	3 Missing ⚠️
app/models/concerns/attestation_bundle_repair.rb	94.87%	2 Missing ⚠️
app/models/attestation.rb	75.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6186      +/-   ##
==========================================
- Coverage   97.24%   97.19%   -0.05%     
==========================================
  Files         476      478       +2     
  Lines        9788     9850      +62     
==========================================
+ Hits         9518     9574      +56     
- Misses        270      276       +6

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

jenshenny · 2026-01-09T01:27:25Z

app/models/concerns/attestation_bundle_repair.rb

+    false
+  end
+
+  def repair_kind_version!(verification, changes)


Looking at our attestations, I believe the kind should be hashedrekord instead of dsse

"kindVersion": { "kind": "hashedrekord", "version": "0.0.1" },

jenshenny · 2026-01-09T02:06:37Z

app/avo/actions/repair_attestation.rb

+    if resource.record.repairable?
+      "This attestation has invalid data that needs repair. Proceed with repair?"


There's potential to say what is exactly wrong aka kind version, double encode or both, but this is fine for now.

jenshenny · 2026-01-09T02:10:51Z

app/avo/resources/attestation.rb

    field :id, as: :id

+    field :valid, as: :boolean, only_on: %i[index show] do
+      record.valid_bundle?


Is the purpose of this to see if the attestation needs repairing? It still shows as valid when it does need to be repaired.

Implement RepairAttestation action and related functionality for atte…

bf85b70

…station repairs Co-authored-by: Ben Fischer <[email protected]>

Copilot AI review requested due to automatic review settings January 8, 2026 04:40

github-project-automation bot added this to RubyGems.org Pull Requests Jan 8, 2026

github-project-automation bot moved this to Todo in RubyGems.org Pull Requests Jan 8, 2026

colby-swandale requested a review from jenshenny January 8, 2026 04:40

Copilot started reviewing on behalf of colby-swandale January 8, 2026 04:41 View session

github-advanced-security bot found potential problems Jan 8, 2026

View reviewed changes

Copilot AI reviewed Jan 8, 2026

View reviewed changes

jenshenny reviewed Jan 9, 2026

View reviewed changes

colby-swandale moved this from Todo to In Progress in RubyGems.org Pull Requests Jan 13, 2026

colby-swandale moved this from In Progress to Review in RubyGems.org Pull Requests Jan 13, 2026

colby-swandale self-assigned this Jan 13, 2026

	should "be not repairable when bundle has no known issues" do
	should "not be repairable when bundle has no known issues" do

	rescue Sigstore::Error
	rescue StandardError => e
	Rails.logger.warn("Failed to validate sigstore bundle for Attestation #{id}: #{e.class}: #{e.message}") if defined?(Rails) && Rails.respond_to?(:logger) && Rails.logger

		if resource.record.repairable?
		"This attestation has invalid data that needs repair. Proceed with repair?"

Uh oh!

Add Attestation Repair action In Avo #6186

Are you sure you want to change the base?

Add Attestation Repair action In Avo #6186

Conversation

colby-swandale commented Jan 8, 2026

Uh oh!

Check failure

Uh oh!

Copilot Autofix

Check failure

Uh oh!

Copilot Autofix

Check failure

Uh oh!

Copilot Autofix

Check failure

Uh oh!

Copilot Autofix

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Jan 8, 2026

Codecov Report

Uh oh!

jenshenny Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

jenshenny Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

jenshenny Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants