Fixes #XXXXX - Fix RHSM OS TOCTOU race during concurrent registration#11059
Open
pablomh wants to merge 1 commit into
Open
Fixes #XXXXX - Fix RHSM OS TOCTOU race during concurrent registration#11059pablomh wants to merge 1 commit into
pablomh wants to merge 1 commit into
Conversation
Under concurrent bulk registration, create_or_find_by in RhsmFactParser#operatingsystem can return an unpersisted Operatingsystem object when the model-level uniqueness validation catches a duplicate that was committed between the initial find_by_attributes lookup and the create attempt. Rails create_or_find_by only rescues ActiveRecord::RecordNotUnique (DB constraint), not ActiveRecord::RecordInvalid (model validation). When the model validation fires first, create returns an invalid object with id=nil. Assigning this to the host silently sets operatingsystem_id=NULL, which causes the subsequent POST /register to fail with 422 Must provide an operating system. Replace create_or_find_by with create! wrapped in a rescue for both RecordInvalid and RecordNotUnique. On either failure, retry the find_by_attributes lookup. If the retry also returns nil (genuine validation failure, not a race), re-raise the original exception. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
create_or_find_byinRhsmFactParser#operatingsystemcan return an unpersistedOperatingsystemobject when the model-level uniqueness validation catches a duplicate committed between the initialfind_by_attributeslookup and the create attemptcreate_or_find_byonly rescuesRecordNotUnique(DB constraint), notRecordInvalid(model validation). Assigning the invalid object to the host silently setsoperatingsystem_id=NULL, causing step 3POST /registerto fail with 422 "Must provide an operating system"create_or_find_bywithcreate!+ rescue for bothRecordInvalidandRecordNotUnique, retrying the find on either. Re-raise if the retry also returns nil (genuine validation failure, not a race)Root cause
Operatingsystemhas three model-level uniqueness validations (namescoped tomajor/minor,description, andtitle). During the first registration burst when the OS record does not yet exist:find_by_attributes→ nil,create_or_find_by→ creates OS (id=4), commitsfind_by_attributes→ nil (Thread A not yet committed),create_or_find_by→ internalcreateruns model validations → SELECT finds OS (Thread A now committed) → validation fails → returns unpersisted object (id=nil) → noRecordNotUniqueraised →create_or_find_byreturns the invalid objecthost.operatingsystem = invalid_os→host.operatingsystem_id = nilhost.savepersistsarchitecture_idbutoperatingsystem_idstays NULLPOST /register→initial_configuration_template→ 422Test plan
test_operatingsystem_returns_existing_os_after_record_not_unique— DB constraint race pathtest_operatingsystem_returns_existing_os_after_uniqueness_validation_conflict— model validation race path (the confirmed production failure)test_operatingsystem_raises_non_race_validation_failure— genuine validation errors propagate instead of being silently swallowed🤖 Generated with Claude Code