Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Verified GitHub links should probably ignore case for orgs #16666

Closed
edgarrmondragon opened this issue Sep 9, 2024 · 4 comments
Closed

Verified GitHub links should probably ignore case for orgs #16666

edgarrmondragon opened this issue Sep 9, 2024 · 4 comments
Assignees
Labels

Comments

@edgarrmondragon
Copy link
Contributor

Describe the bug

For project, meltanolabs-tap-postgres, the homepage link points to https://github.com/meltanolabs/tap-postgres (org in lower case), but the trusted publisher was created with MeltanoLabs/tap-postgres (org in upper case).

The result is that the links to the GitHub repo are not "verified".

Expected behavior

Links to the GitHub repo are verified when using trusted publishers even when the GitHub org doesn't match the case.

To Reproduce

NA, see description of the problem above.

My Platform

Additional context

The easy workaround on our end is simply to use consistent casing throughout, but I though I'd still log in case y'all think this is a bug.

@woodruffw
Copy link
Member

woodruffw commented Sep 11, 2024

Thanks @edgarrmondragon! If I understand correctly, this is indeed a bug in the URL verification handling -- GitHub slugs are case insensitive, so we should always be similarly insensitive when verifying them. I'll look at a fix today.

@woodruffw woodruffw self-assigned this Sep 11, 2024
@woodruffw woodruffw removed the requires triaging maintainers need to do initial inspection of issue label Sep 11, 2024
@woodruffw
Copy link
Member

I looked into this a bit, and it's indeed a bug: GitHub and GitLab (at minimum) have case-insensitive owner/repo slugs, meaning we should compare them case insensitively.

Doing this with the current abstraction is a little painful, though (since we currently centralize URL verification across all publisher types, and not all publishers are necessarily insensitive).

@edgarrmondragon
Copy link
Contributor Author

Gotcha, granted this is easy to fix on the user's end. Thanks for acknowledging it!

@woodruffw
Copy link
Member

No problem! It's also pretty easy to fix on the Warehouse side, but not "20 minutes between bigger tasks" easy 😅 -- I'll try and get to it this weekend.

DarkaMaul added a commit to trail-of-forks/warehouse that referenced this issue Oct 16, 2024
@di di closed this as completed in 77ec5bb Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants