Missing validations allow duplicate and self-referencing push mirrors
Summary
The RemoteMirror model has no validation preventing a user from adding multiple push mirrors with the same URL to a project, or pointing a push mirror at the project's own repository. Both are accepted and saved.
This was surfaced during review of !240866 (merged) (Vue mirror table integration), see this thread. It is pre-existing behavior, not introduced by that MR, and was reproduced on GitLab.com.
Steps to reproduce
- Go to a project's Settings > Repository > Mirroring repositories.
- Add a push mirror with some URL.
- Add a second push mirror with the same URL. It is accepted.
- Alternatively, enter the project's own repository URL. It is accepted (the repository mirrors itself).
Current behavior
Both duplicate and self-referencing mirrors are created. app/models/remote_mirror.rb only validates project presence, url presence/format (public_url), only_protected_branches inclusion, and validate_mirror_count (max 10 enabled mirrors). There is no uniqueness scope on url and no self-mirror guard.
Expected behavior
- Adding a duplicate mirror URL to the same project is rejected with a clear error.
- Adding a mirror that points at the project's own repository is rejected.
Proposed fix
Add validations scoped to creation so existing data is grandfathered:
# app/models/remote_mirror.rb
validates :url, uniqueness: { scope: :project_id }, on: :create
validate :url_not_pointing_to_self, on: :createBackward compatibility (important)
- Projects may already have duplicate or self-referencing mirrors. A validation running on every save would break ongoing mirror syncs, toggles, and edits for those projects.
- Using
on: :creategrandfathers existing rows: syncs, enable/disable, delete, and edits to other fields keep working; only new duplicates are blocked. - Do not add a unique DB index without a preceding deduplication migration, since the index would fail to build against existing duplicate rows.
- Existing self-mirrors are not removed by a validation; a separate cleanup would be needed if that's desired.
References
- MR that surfaced this: !240866 (merged)
- Discussion thread: !240866 (comment 3482834754)