Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
26 changes: 26 additions & 0 deletions app/avo/actions/repair_attestation.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
class Avo::Actions::RepairAttestation < Avo::Actions::ApplicationAction
self.name = "Repair attestation"
self.visible = lambda {
current_user.team_member?("rubygems-org") && view == :show
}
self.message = lambda {
if resource.record.repairable?
"This attestation has invalid data that needs repair. Proceed with repair?"
Comment on lines +7 to +8
Copy link
Member

Choose a reason for hiding this comment

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

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

else
"This attestation appears valid. No repairs are expected."
end
}
self.confirm_button_label = "Repair attestation"

class ActionHandler < Avo::Actions::ActionHandler
def handle_record(attestation)
changes = attestation.repair!

if changes
succeed "Attestation repaired: #{changes.join(', ')}"
else
succeed "No repair was needed"
end
end
end
end
8 changes: 8 additions & 0 deletions app/avo/resources/attestation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,17 @@ class Avo::Resources::Attestation < Avo::BaseResource
self.title = :id
self.includes = [:version]

def actions
action Avo::Actions::RepairAttestation
end

def fields
field :id, as: :id

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

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

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 uses AI. Check for mistakes.
Copy link
Member

Choose a reason for hiding this comment

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

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

end

field :version, as: :belongs_to
field :media_type, as: :text
field :body, as: :json_viewer
Expand Down
8 changes: 8 additions & 0 deletions app/models/attestation.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
class Attestation < ApplicationRecord
include AttestationBundleRepair

belongs_to :version

validates :body, :media_type, presence: true
Expand All @@ -10,6 +12,12 @@ def sigstore_bundle
)
end

def valid_bundle?
sigstore_bundle.present?
rescue Sigstore::Error
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

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

Copilot uses AI. Check for mistakes.
false
end
Comment on lines +15 to +19
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

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 uses AI. Check for mistakes.

def display_data # rubocop:disable Metrics/MethodLength
bundle = sigstore_bundle
leaf_certificate = bundle.leaf_certificate
Expand Down
64 changes: 64 additions & 0 deletions app/models/concerns/attestation_bundle_repair.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
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
Comment on lines +1 to +26
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

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 uses AI. Check for mistakes.

private

def missing_kind_version?(verification)
verification["tlogEntries"]&.any? { |entry| !entry.key?("kindVersion") }
end

def double_encoded_certificate?(verification)
return false unless (raw_bytes = verification.dig("certificate", "rawBytes"))

decoded = Base64.strict_decode64(raw_bytes)
decoded.start_with?("-----BEGIN CERTIFICATE-----")
rescue ArgumentError
false
end

def repair_kind_version!(verification, changes)
Copy link
Member

Choose a reason for hiding this comment

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

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

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

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

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

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 uses AI. Check for mistakes.
changes << "Added missing kindVersion to tlogEntry #{idx}"
end
end

def repair_certificate!(verification, changes)
raw_bytes = verification.dig("certificate", "rawBytes")
return unless raw_bytes

decoded = Base64.strict_decode64(raw_bytes)
return unless decoded.start_with?("-----BEGIN CERTIFICATE-----")

cert = OpenSSL::X509::Certificate.new(decoded)
verification["certificate"]["rawBytes"] = Base64.strict_encode64(cert.to_der)
changes << "Converted double-encoded PEM certificate to DER"
rescue ArgumentError, OpenSSL::X509::CertificateError => e
Rails.logger.warn("Failed to repair certificate: #{e.message}")
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

When an exception occurs during certificate repair, the error is caught and logged, but the exception is silently swallowed without adding any information to the changes array or re-raising the exception. This means that if certificate repair fails, the action might still report "No repair was needed" or only report partial success if kindVersion was repaired. Consider either re-raising the exception or adding a failure message to the changes array to properly communicate the repair outcome.

Suggested change
Rails.logger.warn("Failed to repair certificate: #{e.message}")
Rails.logger.warn("Failed to repair certificate: #{e.message}")
changes << "Failed to repair certificate: #{e.message}"

Copilot uses AI. Check for mistakes.
end
end
64 changes: 64 additions & 0 deletions test/models/attestation_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,68 @@ class AttestationTest < ActiveSupport::TestCase
should belong_to(:version)
should validate_presence_of(:media_type)
should validate_presence_of(:body)

context "#repairable?" do
setup do
@version = create(:version)
end

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
Comment on lines +13 to +21
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

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 uses AI. Check for mistakes.

should "be repairable when kindVersion is missing from tlog entry" do
attestation = Attestation.new(
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?
end

should "be repairable when certificate is double-encoded PEM" do
pem_cert = "-----BEGIN CERTIFICATE-----\nMIIBkTCB+wIJAKHBfpN...\n-----END CERTIFICATE-----\n"
double_encoded = Base64.strict_encode64(pem_cert)

attestation = Attestation.new(
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?
end

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

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

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 uses AI. Check for mistakes.
attestation = Attestation.new(
version: @version,
media_type: "application/vnd.dev.sigstore.bundle.v0.3+json",
body: {
"verificationMaterial" => {
"tlogEntries" => [{ "kindVersion" => { "kind" => "dsse", "version" => "0.0.1" } }],
"certificate" => { "rawBytes" => Base64.strict_encode64("DER data") }
}
}
)

refute_predicate attestation, :repairable?
end
end
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

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 uses AI. Check for mistakes.
end
177 changes: 177 additions & 0 deletions test/unit/avo/actions/repair_attestation_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,177 @@
require "test_helper"

class RepairAttestationTest < ActiveSupport::TestCase
make_my_diffs_pretty!

setup do
@view_context = mock
@avo = mock
@view_context.stubs(:avo).returns(@avo)
@avo.stubs(:resources_audit_path).returns("resources_audit_path")
Avo::Current.stubs(:view_context).returns(@view_context)
@admin = create(:admin_github_user, :is_admin)
@version = create(:version)
end

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") }
}
}
Comment on lines +20 to +25

Check failure

Code scanning / CodeQL

Clear-text storage of sensitive information High test

This stores sensitive data returned by
a write to certificate
as clear text.

Copilot Autofix

AI 7 days ago

Copilot could not generate an autofix suggestion

Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.

)

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
Comment on lines +16 to +46
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

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 uses AI. Check for mistakes.

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 }
}
}
Comment on lines +66 to +71

Check failure

Code scanning / CodeQL

Clear-text storage of sensitive information High test

This stores sensitive data returned by
a write to certificate
as clear text.

Copilot Autofix

AI 7 days ago

Copilot could not generate an autofix suggestion

Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.

)

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
Comment on lines +48 to +93
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

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 uses AI. Check for mistakes.

test "does nothing for attestation without known issues" do
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" => Base64.strict_encode64("DER data") }
}
}
Comment on lines +99 to +104

Check failure

Code scanning / CodeQL

Clear-text storage of sensitive information High test

This stores sensitive data returned by
a write to certificate
as clear text.

Copilot Autofix

AI 7 days ago

Copilot could not generate an autofix suggestion

Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.

)

refute_predicate attestation, :repairable?
original_body = attestation.body.deep_dup

action = Avo::Actions::RepairAttestation.new
action.handle(
fields: { comment: "Attempting repair on valid attestation" },
current_user: @admin,
resource: nil,
records: [attestation],
query: nil
)

attestation.reload

assert_equal original_body, attestation.body
end

test "repairs both missing kindVersion and double-encoded certificate simultaneously" 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" => [{ "logIndex" => 123 }],
"certificate" => { "rawBytes" => double_encoded }
}
}
Comment on lines +142 to +147

Check failure

Code scanning / CodeQL

Clear-text storage of sensitive information High test

This stores sensitive data returned by
a write to certificate
as clear text.

Copilot Autofix

AI 7 days ago

Copilot could not generate an autofix suggestion

Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.

)

assert_predicate attestation, :repairable?

action = Avo::Actions::RepairAttestation.new
action.handle(
fields: { comment: "Repairing attestation with both issues" },
current_user: @admin,
resource: nil,
records: [attestation],
query: nil
)

attestation.reload

refute_predicate attestation, :repairable?

# Verify kindVersion was added
assert_equal(
{ "kind" => "dsse", "version" => "0.0.1" },
attestation.body.dig("verificationMaterial", "tlogEntries", 0, "kindVersion")
)

# Verify certificate was converted to DER
raw_bytes = attestation.body.dig("verificationMaterial", "certificate", "rawBytes")
decoded = Base64.strict_decode64(raw_bytes)

refute decoded.start_with?("-----BEGIN CERTIFICATE-----")
end
end
Loading