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

chore(telemetry): report sha-256 of git repo remote URL #461

Merged
merged 3 commits into from
Apr 26, 2021

Conversation

EverlastingBugstopper
Copy link
Contributor

@EverlastingBugstopper EverlastingBugstopper commented Apr 20, 2021

fixes #313 - want to get Nathan's feedback on the question I asked in that issue before merging.

this is good to go!

Copy link
Contributor

@JakeDawkins JakeDawkins left a comment

Choose a reason for hiding this comment

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

I guess this doesn't really matter, but I don't think the certain telemetry like remote url or directory hash would be useful or accurate for certain commands that don't alter graphs (thinking like graph fetch or config whoami for example). This would have a probably small effect on actual metrics calculated off these values, right?

if let Ok(repo) = repo {
if let Ok(remote) = repo.find_remote("origin") {
if let Some(remote_url) = remote.url() {
return Some(format!("{:x}", Sha256::digest(remote_url.as_bytes())));
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think there's any security implications of hashing the remote, right? Mostly thinking of the issues we had in the past of user/passwords being reported from the remote url. I don't think there's any chance of that here, but just mentioning for my own sanity.

Copy link
Contributor Author

@EverlastingBugstopper EverlastingBugstopper Apr 26, 2021

Choose a reason for hiding this comment

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

the good news is that sha-256 is a one-way operation, if someone could reverse the hashing function it would be a preimage attack. if those were achievable/practical to carry out we'd have some pretty big problems as an industry 😆

@EverlastingBugstopper EverlastingBugstopper merged commit 9e673f7 into main Apr 26, 2021
@EverlastingBugstopper EverlastingBugstopper deleted the avery/hash-repo branch April 26, 2021 22:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Track (anonymized) counts of repos using Rover
2 participants