Skip to content

feat: setting to override trusted sites list #1819

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Jan 13, 2025
Merged

feat: setting to override trusted sites list #1819

merged 4 commits into from
Jan 13, 2025

Conversation

Seb-MCaw
Copy link
Contributor

Closes #814.

The alr publish and alr index --check commands currently require that Git origins use a URL with a host found in a hard-coded list of trusted sites, motivated by concerns regarding SHA1 hash collisions. Those using their own hosting arrangements (with private indexes) need to be able to configure this list.

This PR makes the list configurable, except when using alr publish without the --for-private-index switch.

PR creation checklist
  • A test is included, if required by the changes.
  • doc/user-changes.md has been updated, if applicable.

@Seb-MCaw Seb-MCaw force-pushed the feat/trusted-sites-setting branch from 57a0fa6 to 6f5a74c Compare January 10, 2025 14:18
Seb-MCaw
Seb-MCaw commented Jan 10, 2025
View reviewed changes
@Seb-MCaw Seb-MCaw marked this pull request as ready for review January 10, 2025 15:09
@Seb-MCaw
Copy link
Contributor Author

Ready for review @mosteo.

mosteo
mosteo reviewed Jan 11, 2025
View reviewed changes
Copy link
Member

@mosteo mosteo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A minor refactor and a general comment:

I find using ' ' or whitespace in general to mean any domain is a bit hackish and won't be nice when printing settings with alr settings. What do you think about using some impossible domain, like '*' or "..." instead? (Probably the latter to avoid interactions with shell expansions at the time of setting.)

@Seb-MCaw
Copy link
Contributor Author

I find using ' ' or whitespace in general to mean any domain is a bit hackish ...

Agreed. '*' was my first thought, but will definitely cause problems, and an empty list was the best alternative I could come up with. '...' is a good idea, though; thanks for the suggestion.

@Seb-MCaw Seb-MCaw requested a review from mosteo January 13, 2025 11:28
@mosteo mosteo merged commit 09b3893 into alire-project:master Jan 13, 2025
25 checks passed
@mosteo
Copy link
Member

mosteo commented Jan 13, 2025

Merged, thanks.

@Seb-MCaw Seb-MCaw deleted the feat/trusted-sites-setting branch January 13, 2025 16:51
@Joebeazelman
Copy link

This feature needs to be documented for private indexes. I'm getting an error because dev.azure.com isn't a recognized one.

@Seb-MCaw
Copy link
Contributor Author

This feature needs to be documented for private indexes.

It is already documented at the end of the second paragraph here, but it could stand to be better signposted; I have opened #1985.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mosteo mosteo mosteo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Adding trusted sites
3 participants
@Seb-MCaw @mosteo @Joebeazelman