Verify LFS OID
What does this MR do and why?
Currently the oid
on our LfsObjects
aren't verified for their format. This MR introduces a regex check for /\A\h{64}\z/
to ensure the oid
s are well-formed.
Screenshots or screen recordings
These are strongly recommended to assist reviewers and reduce the time to merge your change.
How to set up and validate locally
In the rails console for an invalid oid
:
[1] pry(main)> LfsObject.create!(oid: "notvalid",size: 1234,file: nil)
Creating scope :verification_succeeded. Overwriting existing method LfsObject.verification_succeeded.
Creating scope :verification_failed. Overwriting existing method LfsObject.verification_failed.
Creating scope :checksummed. Overwriting existing method LfsObject.checksummed.
Creating scope :not_checksummed. Overwriting existing method LfsObject.not_checksummed.
Creating scope :available_verifiables. Overwriting existing method LfsObject.available_verifiables.
TRANSACTION (0.1ms) BEGIN /*application:console,db_config_name:main,line:(pry):1:in `__pry__'*/
LfsObject Exists? (2.3ms) SELECT 1 AS one FROM "lfs_objects" WHERE "lfs_objects"."oid" = 'notvalid' LIMIT 1 /*application:console,db_config_name:main,line:(pry):1:in `__pry__'*/
TRANSACTION (0.1ms) ROLLBACK /*application:console,db_config_name:main,line:/lib/gitlab/database.rb:370:in `rollback'*/
ActiveRecord::RecordInvalid: Validation failed: Oid is invalid
from /home/joern/.asdf/installs/ruby/2.7.5/lib/ruby/gems/2.7.0/gems/activerecord-6.1.6.1/lib/active_record/validations.rb:80:in `raise_validation_error'
[2] pry(main)>
For a valid oid:
[2] pry(main)> LfsObject.create!(oid: "a"*64,size: 1234,file: nil)
TRANSACTION (0.3ms) BEGIN /*application:console,db_config_name:main,line:(pry):2:in `__pry__'*/
LfsObject Exists? (0.4ms) SELECT 1 AS one FROM "lfs_objects" WHERE "lfs_objects"."oid" = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa' LIMIT 1 /*application:console,db_config_name:main,line:(pry):2:in `__pry__'*/
LfsObject Create (67.5ms) INSERT INTO "lfs_objects" ("oid", "size", "created_at", "updated_at", "file") VALUES ('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa', 1234, '2022-08-01 15:01:36.087915', '2022-08-01 15:01:36.087915', 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa') RETURNING "id" /*application:console,db_config_name:main,line:(pry):2:in `__pry__'*/
LfsObject Update (1.3ms) UPDATE "lfs_objects" SET "file_store" = 1 WHERE "lfs_objects"."id" = 19 /*application:console,db_config_name:main,line:/app/models/concerns/file_store_mounter.rb:24:in `update_file_store'*/
LfsObject Exists? (0.3ms) SELECT 1 AS one FROM "lfs_objects" WHERE "lfs_objects"."file_store" = 1 AND "lfs_objects"."id" = 19 LIMIT 1 /*application:console,db_config_name:main,line:/ee/app/models/concerns/geo/verifiable_model.rb:19:in `save_verification_details'*/
Geo::LfsObjectState Load (10.7ms) SELECT "lfs_object_states".* FROM "lfs_object_states" WHERE "lfs_object_states"."lfs_object_id" = 19 LIMIT 1 /*application:console,db_config_name:main,line:/ee/app/models/ee/lfs_object.rb:64:in `lfs_object_state'*/
Geo::LfsObjectState Create (0.5ms) INSERT INTO "lfs_object_states" ("lfs_object_id") VALUES (19) RETURNING "lfs_object_id" /*application:console,db_config_name:main,line:/ee/app/models/concerns/geo/verifiable_model.rb:28:in `save_verification_details'*/
TRANSACTION (0.2ms) COMMIT /*application:console,db_config_name:main,line:/lib/gitlab/database.rb:364:in `commit'*/
GeoNode Exists? (0.4ms) SELECT 1 AS one FROM "geo_nodes" LIMIT 1 /*application:console,db_config_name:main,line:/ee/lib/gitlab/geo.rb:86:in `block in enabled?'*/
=> #<LfsObject:0x000055eedd55d058
id: 19,
oid: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
size: 1234,
created_at: Mon, 01 Aug 2022 15:01:36.087915577 UTC +00:00,
updated_at: Mon, 01 Aug 2022 15:01:36.087915577 UTC +00:00,
file: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
file_store: 1,
verification_checksum: nil>
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.
Edited by Joern Schneeweisz