Add length validations for user-supplied string attributes

Signed-off-by: Samuel Giddins <[email protected]>

app/models/gem_name_reservation.rb

@@ -1,5 +1,5 @@
 class GemNameReservation < ApplicationRecord
-  validates :name, uniqueness: { case_sensitive: false }, presence: true
+  validates :name, uniqueness: { case_sensitive: false }, presence: true, length: { maximum: Gemcutter::MAX_FIELD_LENGTH }
   validate :downcase_name_check
 
   def self.reserved?(name)

app/models/gem_typo_exception.rb

@@ -1,5 +1,5 @@
 class GemTypoException < ApplicationRecord
-  validates :name, presence: true, uniqueness: { case_sensitive: false }
+  validates :name, presence: true, uniqueness: { case_sensitive: false }, length: { maximum: Gemcutter::MAX_FIELD_LENGTH }
   validate :rubygems_name
 
   private

app/models/linkset.rb

@@ -11,6 +11,8 @@ class Linkset < ApplicationRecord
       allow_nil: true,
       allow_blank: true,
       message: "does not appear to be a valid URL"
+
+    validates url, length: { maximum: Gemcutter::MAX_FIELD_LENGTH }
   end
 
   def empty?

app/models/oidc/api_key_role.rb

@@ -22,7 +22,7 @@ class OIDC::ApiKeyRole < ApplicationRecord
   scope :deleted, -> { where.not(deleted_at: nil) }
   scope :active, -> { where(deleted_at: nil) }
 
-  validates :name, presence: true, length: { maximum: 255 }, uniqueness: { scope: :user_id }
+  validates :name, presence: true, length: { maximum: Gemcutter::MAX_FIELD_LENGTH }, uniqueness: { scope: :user_id }
 
   attribute :api_key_permissions, Types::JsonDeserializable.new(OIDC::ApiKeyPermissions)
   validates :api_key_permissions, presence: true, nested: true

app/models/oidc/trusted_publisher/github_action.rb

@@ -8,11 +8,9 @@ class OIDC::TrustedPublisher::GitHubAction < ApplicationRecord
 
   before_validation :find_github_repository_owner_id
 
-  validates :repository_owner, presence: true
-  validates :repository_name, presence: true
-  validates :workflow_filename, presence: true
-  validates :environment, presence: true, allow_nil: true
-  validates :repository_owner_id, presence: true
+  validates :repository_owner, :repository_name, :workflow_filename, :repository_owner_id,
+    presence: true, length: { maximum: Gemcutter::MAX_FIELD_LENGTH }
+  validates :environment, presence: true, allow_nil: true, length: { maximum: Gemcutter::MAX_FIELD_LENGTH }
 
   validate :unique_publisher
   validate :workflow_filename_format

app/models/user.rb

@@ -68,7 +68,7 @@ class User < ApplicationRecord
   has_many :organizations, through: :memberships
 
   validates :email, length: { maximum: Gemcutter::MAX_FIELD_LENGTH }, format: { with: URI::MailTo::EMAIL_REGEXP }, presence: true,
-uniqueness: { case_sensitive: false }
+    uniqueness: { case_sensitive: false }
   validates :unconfirmed_email, length: { maximum: Gemcutter::MAX_FIELD_LENGTH }, format: { with: URI::MailTo::EMAIL_REGEXP }, allow_blank: true
 
   validates :handle, uniqueness: { case_sensitive: false }, allow_nil: true, if: :handle_changed?
@@ -78,9 +78,8 @@ class User < ApplicationRecord
   validates :twitter_username, format: {
     with: /\A[a-zA-Z0-9_]*\z/,
     message: "can only contain letters, numbers, and underscores"
-  }, allow_nil: true
+  }, allow_nil: true, length: { within: 0..20 }
 
-  validates :twitter_username, length: { within: 0..20 }, allow_nil: true
   validates :password,
     length: { within: 10..200 },
     unpwn: true,

app/models/webauthn_credential.rb

@@ -1,9 +1,9 @@
 class WebauthnCredential < ApplicationRecord
   belongs_to :user
 
-  validates :external_id, uniqueness: true, presence: true
-  validates :public_key, presence: true
-  validates :nickname, presence: true, uniqueness: { scope: :user_id }
+  validates :external_id, uniqueness: true, presence: true, length: { maximum: 512 }
+  validates :public_key, presence: true, length: { maximum: 512 }
+  validates :nickname, presence: true, uniqueness: { scope: :user_id }, length: { maximum: 64 }
   validates :sign_count, presence: true, numericality: { greater_than_or_equal_to: 0 }
 
   after_create :send_creation_email

app/models/webauthn_verification.rb

@@ -2,7 +2,7 @@ class WebauthnVerification < ApplicationRecord
   belongs_to :user
 
   validates :user_id, uniqueness: true
-  validates :path_token, presence: true, uniqueness: true
+  validates :path_token, presence: true, uniqueness: true, length: { maximum: 128 }
   validates :path_token_expires_at, presence: true
 
   def expire_path_token

test/models/gem_name_reservation_test.rb

@@ -13,6 +13,7 @@ class GemNameReservationTest < ActiveSupport::TestCase
     should_not allow_value("Abc").for(:name)
     should allow_value("abc").for(:name)
     should validate_uniqueness_of(:name).case_insensitive
+    should validate_length_of(:name).is_at_most(Gemcutter::MAX_FIELD_LENGTH)
   end
 
   context "#reserved?" do

test/models/gem_typo_exception_test.rb

@@ -3,6 +3,7 @@
 class GemTypoExceptionTest < ActiveSupport::TestCase
   context "name validations" do
     should validate_uniqueness_of(:name).case_insensitive
+    should validate_length_of(:name).is_at_most(Gemcutter::MAX_FIELD_LENGTH)
 
     should "be a valid factory" do
       assert_predicate build(:gem_typo_exception), :valid?

test/models/linkset_test.rb

@@ -36,4 +36,11 @@ class LinksetTest < ActiveSupport::TestCase
       assert_equal @spec.homepage, @linkset.home
     end
   end
+
+  context "validations" do
+    %w[home code docs wiki mail bugs].each do |link|
+      should allow_value("http://example.com").for(link.to_sym)
+      should validate_length_of(link).is_at_most(Gemcutter::MAX_FIELD_LENGTH)
+    end
+  end
 end

test/models/oidc/api_key_role_test.rb

@@ -8,6 +8,8 @@ class OIDC::ApiKeyRoleTest < ActiveSupport::TestCase
   should have_many :id_tokens
   should validate_presence_of :api_key_permissions
   should validate_presence_of :access_policy
+  should validate_presence_of :name
+  should validate_length_of(:name).is_at_most(Gemcutter::MAX_FIELD_LENGTH)
 
   setup do
     @role = build(:oidc_api_key_role)

test/models/oidc/trusted_publisher/github_action_test.rb

@@ -7,10 +7,22 @@ class OIDC::TrustedPublisher::GitHubActionTest < ActiveSupport::TestCase
   should have_many(:rubygem_trusted_publishers)
   should have_many(:api_keys).inverse_of(:owner)
 
-  should validate_presence_of(:repository_owner)
-  should validate_presence_of(:repository_name)
-  should validate_presence_of(:workflow_filename)
-  should validate_presence_of(:repository_owner_id)
+  context "validations" do
+    setup do
+      stub_request(:get, Addressable::Template.new("https://api.github.com/users/{user}"))
+        .to_return(status: 404, body: "", headers: {})
+    end
+    should validate_presence_of(:repository_owner)
+    should validate_length_of(:repository_owner).is_at_most(Gemcutter::MAX_FIELD_LENGTH)
+    should validate_presence_of(:repository_name)
+    should validate_length_of(:repository_name).is_at_most(Gemcutter::MAX_FIELD_LENGTH)
+    should validate_presence_of(:workflow_filename)
+    should validate_length_of(:workflow_filename).is_at_most(Gemcutter::MAX_FIELD_LENGTH)
+    should validate_presence_of(:repository_owner_id)
+    should validate_length_of(:repository_owner_id).is_at_most(Gemcutter::MAX_FIELD_LENGTH)
+
+    should validate_length_of(:environment).is_at_most(Gemcutter::MAX_FIELD_LENGTH)
+  end
 
   test "validates publisher uniqueness" do
     publisher = create(:oidc_trusted_publisher_github_action)

test/models/user_test.rb

@@ -17,6 +17,7 @@ class UserTest < ActiveSupport::TestCase
       should_not allow_value("1abcde").for(:handle)
       should_not allow_value("abc^%def").for(:handle)
       should_not allow_value("abc\n<script>bad").for(:handle)
+      should validate_length_of(:handle).is_at_least(2).is_at_most(40)
 
       should "be between 2 and 40 characters" do
         user = build(:user, handle: "a")
@@ -162,7 +163,7 @@ class UserTest < ActiveSupport::TestCase
     end
 
     context "twitter_username" do
-      should validate_length_of(:twitter_username)
+      should validate_length_of(:twitter_username).is_at_most(20)
       should allow_value("user123_32").for(:twitter_username)
       should_not allow_value("@user").for(:twitter_username)
       should_not allow_value("user 1").for(:twitter_username)

test/models/webauthn_credential_test.rb

@@ -6,8 +6,12 @@ class WebauthnCredentialTest < ActiveSupport::TestCase
   should belong_to :user
   should validate_presence_of(:external_id)
   should validate_uniqueness_of(:external_id)
+  should validate_length_of(:external_id).is_at_most(512)
   should validate_presence_of(:public_key)
+  should validate_length_of(:public_key).is_at_most(512)
   should validate_presence_of(:nickname)
+  should validate_uniqueness_of(:nickname).scoped_to(:user_id)
+  should validate_length_of(:nickname).is_at_most(64)
   should validate_presence_of(:sign_count)
   should validate_numericality_of(:sign_count).is_greater_than_or_equal_to(0)
 

test/unit/webauthn_verification_test.rb

@@ -8,6 +8,8 @@ class WebauthnVerificationTest < ActiveSupport::TestCase
   should validate_uniqueness_of(:user_id)
   should validate_presence_of(:path_token)
   should validate_uniqueness_of(:path_token)
+  should validate_presence_of(:path_token)
+  should validate_length_of(:path_token).is_at_most(128)
   should validate_presence_of(:path_token_expires_at)
 
   context "#expire_path_token" do