Skip to content
Open
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
29 changes: 29 additions & 0 deletions src/github.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ impl<K, U> Knowable<K, U> {

#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Deserialize)]
pub struct Asset<T = Type> {
/// (Asset) ID needed to download assets from private repositories
pub id: u64,
pub name: String,
pub size: u64,

Expand All @@ -68,6 +70,7 @@ pub struct Asset<T = Type> {
impl Asset<Knowable<Type, String>> {
fn known(self) -> Option<Asset> {
self.mime.known().map(|mime| Asset {
id: self.id,
name: self.name,
size: self.size,
url: self.url,
Expand Down Expand Up @@ -116,6 +119,11 @@ pub struct GitHubArgs {
#[arg(short = 't', long)]
pub tag: String,

/// Download Release Assets w/o supplying token, which doesn't work for
/// private repos.
#[arg(long)]
pub public: bool,

/// Filter asset names
#[arg(trailing_var_arg = true)]
pub filter: Vec<String>,
Expand Down Expand Up @@ -243,13 +251,34 @@ impl GitHub {
})
}

/// Get the GitHub token if available
pub fn token(&self) -> Option<&str> {
self.args.token.as_deref()
}

/// Returns true if the repository is not public (i.e., `public` flag is false).
pub const fn is_private(&self) -> bool {
!self.args.public
}

/// Get the owner name
pub fn owner(&self) -> &str {
&self.args.owner
}

/// Get the repo name
pub fn repo(&self) -> &str {
&self.args.repo
}

pub async fn assets(&self) -> Result<BTreeSet<Asset>> {
let url = format!(
"https://api.github.com/repos/{}/{}/releases/tags/{}",
self.args.owner, self.args.repo, self.args.tag
);

let response = self.client.get(&url).send().await?;

let release: Release = response.json().await?;

let assets = release
Expand Down
26 changes: 25 additions & 1 deletion src/http/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,35 @@ impl Server {
attempt.stop()
});

let mut client_builder = Client::builder().redirect(policy);
if github.is_private() {
// Build client with GitHub authentication if token is available
if let Some(token) = github.token() {
let mut headers = reqwest::header::HeaderMap::new();
let mut auth_value = format!("token {token}")
.parse::<reqwest::header::HeaderValue>()
.unwrap();
auth_value.set_sensitive(true);
headers.insert(reqwest::header::AUTHORIZATION, auth_value);
headers.insert(
reqwest::header::USER_AGENT,
concat!(env!("CARGO_PKG_NAME"), "/", env!("CARGO_PKG_VERSION"))
.parse()
.unwrap(),
);
headers.insert(
reqwest::header::ACCEPT,
"application/octet-stream".parse().unwrap(),
);
client_builder = client_builder.default_headers(headers);
}
}

Ok(Self {
listener,
status,
github,
client: Client::builder().redirect(policy).build()?,
client: client_builder.build()?,
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason that private would not need the redirect policy? Otherwise I think you keep the redirect() call here and remove the one at the top.

path,
})
}
Expand Down
67 changes: 63 additions & 4 deletions src/http/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,40 @@ impl Service {
path,
}
}

/// Build a reqwest request for an asset, handling private/public repos and method.
fn build_request(
gh: &GitHub,
asset: &Asset,
client: &Client,
method: &Method,
) -> reqwest::RequestBuilder {
let url = if gh.is_private() {
format!(
"https://api.github.com/repos/{}/{}/releases/assets/{}",
gh.owner(),
gh.repo(),
asset.id
)
} else {
asset.url.clone()
};

match method {
&Method::HEAD => client.head(url),
// Only HEAD and GET are expected; all others default to GET.
_ => client.get(url),
}
}

fn inspect_response(response: &reqwest::Response) -> (reqwest::StatusCode, Option<&str>) {
let status_code = response.status();
let content_length = response
.headers()
.get("content-length")
.and_then(|v| v.to_str().ok());
(status_code, content_length)
}
}

impl hyper::service::Service<Request<Incoming>> for Service {
Expand Down Expand Up @@ -110,7 +144,21 @@ impl hyper::service::Service<Request<Incoming>> for Service {
None => return Ok(POWEROFF_EFI.reply(None, Type::Efi, EMPTY)),

// Send the request (possibly redirecting...)
Some(asset) => (client.head(asset.url).send().await?, asset.mime),
Some(asset) => {
let request =
Self::build_request(&github, &asset, &client, &Method::HEAD);

match request.send().await {
Ok(resp) => {
let (_status_code, _content_length) =
Self::inspect_response(&resp);
Comment on lines +153 to +154
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

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

The inspect_response function is called but its return values are unused (indicated by the underscore prefix). Consider removing this function call if the values aren't needed, or remove the underscores if they will be used in future iterations.

Suggested change
let (_status_code, _content_length) =
Self::inspect_response(&resp);
// Removed unused inspect_response call

Copilot uses AI. Check for mistakes.
(resp, asset.mime)
}
Err(_e) => {
return Ok(EMPTY.reply(Code::BAD_GATEWAY, None, None));
}
}
}
}
}

Expand All @@ -122,9 +170,20 @@ impl hyper::service::Service<Request<Incoming>> for Service {

// Send the request (possibly redirecting...)
Some(asset) => {
let response = client.get(asset.url).send().await?;
status.lock().await.update().downloading(remote);
(response, asset.mime)
let request =
Self::build_request(&github, &asset, &client, &Method::GET);

match request.send().await {
Ok(resp) => {
let (_status_code, _content_length) =
Self::inspect_response(&resp);
Comment on lines +178 to +179
Copy link

Copilot AI Dec 6, 2025

Choose a reason for hiding this comment

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

The inspect_response call extracts status code and content length but doesn't use them (indicated by underscore prefix). If these values aren't needed for error handling or logging, consider removing this call.

Copilot uses AI. Check for mistakes.
Copy link
Contributor

Choose a reason for hiding this comment

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

@markg-github is there a reason for not removing these? I agree w/ the copilot comments that I can't see why we'd want to inspect the response if nothing is done with the contents. Was this done for debug and then forgot to remove? The original implementation was pretty nice and concise.

Comment on lines +178 to +179
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

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

The inspect_response function is called but its return values are unused (indicated by the underscore prefix). Consider removing this function call if the values aren't needed, or remove the underscores if they will be used in future iterations.

Suggested change
let (_status_code, _content_length) =
Self::inspect_response(&resp);
// Removed unused inspect_response call

Copilot uses AI. Check for mistakes.
status.lock().await.update().downloading(remote);
(resp, asset.mime)
}
Err(_e) => {
return Ok(EMPTY.reply(Code::BAD_GATEWAY, None, None));
}
}
}
}
}
Expand Down