From 3d952fb15770d65c926a8b8e2da9a2f8e9fad875 Mon Sep 17 00:00:00 2001 From: Johannes Millan Date: Thu, 2 Jul 2026 10:43:09 +0200 Subject: [PATCH] fix(gitlab): reject invalid project identifiers in config validation (#8667) The project-field validation regex was only end-anchored, so display names containing spaces (e.g. "My Group/My Gitlab") passed validation and produced a confusing 404 at poll time instead of inline feedback (#8665). Anchor the regex at both ends and simplify the path branch to a plain character class so it stays a permissive "did you paste a display name?" guard (still accepts consecutive hyphens and other GitLab-valid paths). Clarify the field hint to ask for the path slug or numeric ID, and add regression tests plus a field-wiring check. --- .../gitlab/gitlab-cfg-form.const.spec.ts | 62 +++++++++++++++++++ .../providers/gitlab/gitlab-cfg-form.const.ts | 9 ++- src/assets/i18n/en.json | 2 +- 3 files changed, 71 insertions(+), 2 deletions(-) create mode 100644 src/app/features/issue/providers/gitlab/gitlab-cfg-form.const.spec.ts diff --git a/src/app/features/issue/providers/gitlab/gitlab-cfg-form.const.spec.ts b/src/app/features/issue/providers/gitlab/gitlab-cfg-form.const.spec.ts new file mode 100644 index 0000000000..69b22d7d55 --- /dev/null +++ b/src/app/features/issue/providers/gitlab/gitlab-cfg-form.const.spec.ts @@ -0,0 +1,62 @@ +import { + GITLAB_CONFIG_FORM_SECTION, + GITLAB_PROJECT_REGEX, +} from './gitlab-cfg-form.const'; + +describe('GITLAB_PROJECT_REGEX', () => { + let projectPattern: RegExp; + + beforeAll(() => { + // Verify the form field is actually wired to the exported regex, so this + // spec fails if someone swaps the field's `pattern` out from under it. + const projectField = GITLAB_CONFIG_FORM_SECTION.items!.find( + (item) => item.key === 'project', + ); + const pattern = projectField?.templateOptions?.pattern as RegExp; + expect(pattern).toBe(GITLAB_PROJECT_REGEX); + projectPattern = pattern; + }); + + // Angular's Validators.pattern uses `regex.test(value)` as-is for a RegExp + // (no auto-anchoring), so the regex itself must be anchored at both ends. + const isValid = (value: string): boolean => projectPattern.test(value); + + describe('valid project identifiers', () => { + const validCases = [ + 'super-productivity/super-productivity', + 'group/subgroup/project', + 'my_group/my.project', + 'a.b-c/d_e', + 'group%2Fproject', + 'group%2Fsub%2Fproject', + // GitLab allows consecutive hyphens in a path segment; must not be rejected. + 'foo--bar', + 'single', + '12345', + ]; + validCases.forEach((value) => { + it(`accepts "${value}"`, () => { + expect(isValid(value)).toBe(true); + }); + }); + }); + + describe('invalid project identifiers', () => { + // Regression for #8665: a display name with a space must be rejected inline + // instead of producing a confusing 404 at poll time. + const invalidCases = [ + 'My Group/My Gitlab', + 'group/Test Gitlab', + 'has space', + ' leadingSpace', + 'trailingSpace ', + 'https://gitlab.com/foo/bar', + 'foo/bar?scope=all', + ]; + invalidCases.forEach((value) => { + it(`rejects "${value}"`, () => { + expect(isValid(value)).toBe(false); + }); + }); + }); +}); diff --git a/src/app/features/issue/providers/gitlab/gitlab-cfg-form.const.ts b/src/app/features/issue/providers/gitlab/gitlab-cfg-form.const.ts index ebed583270..247047acff 100644 --- a/src/app/features/issue/providers/gitlab/gitlab-cfg-form.const.ts +++ b/src/app/features/issue/providers/gitlab/gitlab-cfg-form.const.ts @@ -8,7 +8,14 @@ import { CROSS_ORIGIN_WARNING, ISSUE_PROVIDER_COMMON_FORM_FIELDS, } from '../../common-issue-form-stuff.const'; -export const GITLAB_PROJECT_REGEX = /(^[1-9][0-9]*$)|((\/|%2F|\w-?|\.-?)+$)/i; +// Matches either a numeric project ID or a path (namespace/project-slug, or its +// %2F-encoded form). Anchored at both ends: without a leading `^` the original +// pattern only matched a valid *tail*, so display names with spaces (e.g. +// "My Group/My Gitlab") passed validation and produced a 404 at poll time (#8665). +// This is a "did you paste a display name?" sanity guard, not a strict GitLab +// path validator, so the char class stays permissive (e.g. accepts consecutive +// hyphens, which GitLab paths allow) rather than risk false-rejecting valid paths. +export const GITLAB_PROJECT_REGEX = /^(?:[1-9][0-9]*|[\w%./-]+)$/i; export const GITLAB_CONFIG_FORM: LimitedFormlyFieldConfig[] = [ ...CROSS_ORIGIN_WARNING, diff --git a/src/assets/i18n/en.json b/src/assets/i18n/en.json index 49c7cfb9e1..494ea64b6d 100644 --- a/src/assets/i18n/en.json +++ b/src/assets/i18n/en.json @@ -422,7 +422,7 @@ "FILTER_USER": "Filter username", "GITLAB_BASE_URL": "Custom GitLab base URL (optional)", "PROJECT": "user name/project", - "PROJECT_HINT": "e.g. super-productivity/super-productivity", + "PROJECT_HINT": "Project path (namespace/project-slug) or numeric project ID, not the display name (e.g. super-productivity/super-productivity or 12345)", "SCOPE": "Scope", "SCOPE_ALL": "All", "SCOPE_ASSIGNED": "Assigned to me",