Skip to content

TA-4286: Add validation for git profile name#267

Merged
Zgjim Haziri (ZgjimHaziri) merged 5 commits into
masterfrom
TA-4286-validate-git-profile-name
Oct 8, 2025
Merged

TA-4286: Add validation for git profile name#267
Zgjim Haziri (ZgjimHaziri) merged 5 commits into
masterfrom
TA-4286-validate-git-profile-name

Conversation

@ZgjimHaziri

Copy link
Copy Markdown
Contributor

Description

  • Git profile name fails when we try to create it with restricted filename characters, since we use that name to create the file in which it will be stored.
  • The simplest solution seems to be failing the creation with a descriptive validation error message.
  • Other considered solutions required a larger refactoring effort or had downsides:
    • Using a UUID as the filename. This required changing the way we fetched the profile by reading the file and looking for the profile name in the content.
    • Sanitizing the name. Showed possible name conflicts scenarios, e.g. if we sanitize the character '/' with '_', then we can have a conflict if the user tries to create "test/1" and "test_1".
  • Since the CLI user is aware of the files created in his environment, it seems reasonable to proceed with the solution.

Relevant links

Checklist

  • I have self-reviewed this PR
  • I have tested the change and proved that it works in different scenarios
  • I have updated docs if needed

Screenshots of the failure message

Screenshot 2025-10-06 at 11 28 02

@ZgjimHaziri Zgjim Haziri (ZgjimHaziri) requested a review from a team as a code owner October 6, 2025 09:41
Comment thread src/core/git-profile/git-profile.service.ts Outdated
Buqeta
Buqeta (Buqeta) previously approved these changes Oct 6, 2025
@Buqeta

Copy link
Copy Markdown
Contributor

Can we also increase the CLI version?

@ZgjimHaziri

Copy link
Copy Markdown
Contributor Author

Can we also increase the CLI version?

Will leave the bump in another PR, in order to release both of these fixes together.

Note: working on a release process for CLI now, will make this more clear there.

@Buqeta

Copy link
Copy Markdown
Contributor

Just remembered: can you add some tests for these changes?

@ZgjimHaziri Zgjim Haziri (ZgjimHaziri) merged commit 1770c3a into master Oct 8, 2025
1 check passed
@ZgjimHaziri Zgjim Haziri (ZgjimHaziri) deleted the TA-4286-validate-git-profile-name branch October 8, 2025 09:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants