Skip to content

Fix auth headers#11

Open
ChrisBr wants to merge 1 commit intotecnix-gcsfrom
cbruckmayer/fix-auth
Open

Fix auth headers#11
ChrisBr wants to merge 1 commit intotecnix-gcsfrom
cbruckmayer/fix-auth

Conversation

@ChrisBr
Copy link

@ChrisBr ChrisBr commented Mar 2, 2026

Fix auth headers

@ChrisBr ChrisBr changed the base branch from master to tecnix-gcs March 2, 2026 18:32
Copy link

@lilyinstarlight lilyinstarlight left a comment

Choose a reason for hiding this comment

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

Using the netrc file is the more correct tool for authenticating tarball downloads rather than just guessing on access token formatting, but that also will only handle HTTP basic auth, not bearer tokens

If we need to authenticate with specific APIs instead of generalized builtins.fetchTarball downloads, then I believe builtins.fetchTree is the better tool for the job and already will authenticate with the given input type (and we should probably consider moving usage of builtins.fetchTarball that is giving us issues to that before making API-specific changes to non-API functions)

I also worry this change might cause libcurl not to use the netrc if an access-token is defined for a host, but that admittedly might be a niche case that won't affect us

auto & host = parsedUrl.authority->host;
auto tokens = settings.accessTokens.get();
if (auto token = get(tokens, host)) {
headers.push_back({"Authorization", fmt("token %s", *token)});

Choose a reason for hiding this comment

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

GitHub supports Authorization: Bearer[1] so for more generalized support (since most forges require that), we should use Bearer

Suggested change
headers.push_back({"Authorization", fmt("token %s", *token)});
headers.push_back({"Authorization", fmt("Bearer %s", *token)});

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants