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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion crates/houston/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ pub enum HoustonProblem {
#[error("No non-sensitive configuration found for profile \"{0}\".")]
NoNonSensitiveConfigFound(String),

/// PathNotUnicode occurs when Houston encounteres a file path that is not valid UTF-8
/// PathNotUtf8 occurs when Houston encounters a file path that is not valid UTF-8
#[error(transparent)]
PathNotUtf8(#[from] camino::FromPathBufError),

Expand Down
1 change: 1 addition & 0 deletions crates/sputnik/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ edition = "2018"
[dependencies]
camino = "1.0"
ci_info = { version = "0.14", features = ["serde-1"] }
git2 = "0.13"
reqwest = { version = "0.11", features = ["blocking"] }
semver = { version = "0.11", features = ["serde"] }
serde = { version = "1.0", features = ["derive"] }
Expand Down
4 changes: 4 additions & 0 deletions crates/sputnik/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ pub enum SputnikError {
#[error(transparent)]
JsonError(#[from] serde_json::Error),

/// PathNotUtf8 occurs when Sputink encounters a file path that is not valid UTF-8
#[error(transparent)]
PathNotUtf8(#[from] camino::FromPathBufError),

/// HttpError occurs when an error occurs while reporting anonymous usage data.
#[error("Could not report anonymous usage data.")]
HttpError(#[from] reqwest::Error),
Expand Down
2 changes: 1 addition & 1 deletion crates/sputnik/src/report.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use camino::{Utf8Path, Utf8PathBuf};
use url::Url;
use uuid::Uuid;

use camino::{Utf8Path, Utf8PathBuf};
use std::fs::{self, File};
use std::io::Write;

Expand Down
35 changes: 27 additions & 8 deletions crates/sputnik/src/session.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
use camino::Utf8PathBuf;
use ci_info::types::Vendor as CiVendor;
use git2::Repository;
use reqwest::Url;
use semver::Version;
use serde::Serialize;
use sha2::{Digest, Sha256};
use uuid::Uuid;

use std::collections::HashMap;
use std::env::{consts::OS, current_dir};
use std::convert::TryFrom;
use std::env::{self, consts::OS};
use std::fmt::Debug;
use std::time::Duration;

Expand All @@ -27,9 +30,12 @@ pub struct Session {
/// A unique session id
session_id: Uuid,

/// Directory hash. A hash of the current working directory
/// SHA-256 hash of the current working directory
cwd_hash: String,

/// SHA-256 hash of the git remote URL
remote_url_hash: Option<String>,

/// Information about the current architecture/platform
platform: Platform,

Expand Down Expand Up @@ -81,8 +87,10 @@ impl Session {
endpoint: app.endpoint()?,
user_agent: app.user_agent(),
};
let current_dir = Utf8PathBuf::try_from(env::current_dir()?)?;
let session_id = Uuid::new_v4();
let cwd_hash = get_cwd_hash()?;
let cwd_hash = get_cwd_hash(&current_dir);
let remote_url_hash = get_repo_hash(&current_dir);

let continuous_integration = if ci_info::is_ci() {
ci_info::get().vendor
Expand All @@ -102,6 +110,7 @@ impl Session {
machine_id,
session_id,
cwd_hash,
remote_url_hash,
platform,
cli_version,
reporting_info,
Expand Down Expand Up @@ -129,10 +138,20 @@ impl Session {
}

/// returns sha256 digest of the directory the tool was executed from.
fn get_cwd_hash() -> Result<String, SputnikError> {
let current_dir = current_dir()?;
let current_dir_string = current_dir.to_string_lossy();
let current_dir_bytes = current_dir_string.as_bytes();
fn get_cwd_hash(current_dir: &Utf8PathBuf) -> String {
format!("{:x}", Sha256::digest(current_dir.as_str().as_bytes()))
}

/// returns sha256 digest of the repository the tool was executed from.
fn get_repo_hash(current_dir: &Utf8PathBuf) -> Option<String> {
let repo = Repository::discover(current_dir);
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 😆

}
}
}

Ok(format!("{:x}", Sha256::digest(current_dir_bytes)))
None
}