[PATCH 0/3] http: authenticate packfile-URI downloads - #95
Open
friel-openai wants to merge 3 commits into
Open
Conversation
friel-openai
added
kind:review-only
friel-openai
force-pushed
the
af/codex/packfile-uri-credentials
branch
from
September 8, 2026 08:02
3ba60b7 to
3ec65b8
Compare
Separate curl request setup from opening the temporary pack. An HTTP authentication retry will need a fresh request without reopening or truncating the pack, which another downloader may be using. Keep the file and index-pack ownership unchanged. Free the previous header list when preparing another request. Signed-off-by: Friel <friel@openai.com>
http-fetch --packfile does not handle HTTP_REAUTH, so a protected pack fails with HTTP 401 even when a credential helper can authenticate it. Use run_one_slot() and http_reauth_prepare() to retry authentication, with the same three-attempt limit as http_request_recoverable(). Look up credentials for the pack URL, not the fetch negotiation URL. Collect WWW-Authenticate and append helper Authorization headers. Disable FAILONERROR so older curl versions deliver challenge headers, but discard error response bodies instead of writing them into the partial pack. Unlike HTTP_REQUEST_FILE retries, never truncate that shared file. For a redirected 401, update credentials before run_one_slot() handles the response. Do this in the completion callback, while the curl handle still exists even with http.minSessions=0. Preserve the challenge and reload http.extraHeader for the new URL, so a direct retry cannot resend headers configured for the redirect source. Test destination-scoped credentials and resumption through fetch-pack. Signed-off-by: Friel <friel@openai.com>
Run the existing credential-helper assertions against both ls-remote and http-fetch --packfile. This covers Basic, Bearer, proactive and multistage authentication without duplicating their fixtures. Add pack-specific checks for the retry limit, anonymous URLs, redirect configuration, and source-scoped headers. Combine the redirected request with credential.useHttpPath and http.minSessions=0 to check credential selection and curl handle lifetime. Signed-off-by: Friel <friel@openai.com>
friel-openai
force-pushed
the
af/codex/packfile-uri-credentials
branch
from
September 8, 2026 08:23
3ec65b8 to
409df2c
Compare
friel-openai
changed the title
friel-openai
marked this pull request as ready for review
September 8, 2026 09:19
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
http-fetch --packfilecurrently fails on HTTP 401 even when a credentialhelper can authenticate the pack URL. This series adds the missing retry
handling without copying credentials from fetch negotiation.
http: factor out pack request setup(15 additions, 7 deletions).http: authenticate packfile-URI downloads(173 additions, 6 deletions).t5563: exercise pack downloads with HTTP authentication tests(142 additions, 28 deletions, tests only).
The implementation uses
run_one_slot(),http_reauth_prepare()and theexisting HTTP configuration parser. It preserves shared partial packs,
looks up credentials for the challenged URL, and rematches
http.extraHeaderbefore retrying a redirected URL. Error response bodiesare discarded so older curl versions can deliver authentication headers
without corrupting a partial pack.
fetch-pack.c,remote-curl.candhttp-walker.care unchanged. Protocolintegration tests remain in
t5702; direct HTTP authentication tests usethe existing
t5563fixtures.Validation: the full local suite passes with
make -j8 DEVELOPER=1 NO_GETTEXT=1 NO_TCLTK=1 test, including 90 protocol-v2tests and 38 HTTP-auth tests. Both implementation commits build independently;
the second also passes all 90 protocol-v2 tests without the third commit.
Test lint, whitespace and GitHub style checks pass. The complete
PR CI matrix passed
for
409df2c11f, including Linux32, ASan/UBSan and both leak-check jobs.Linux32 confirms the older-curl challenge-header fix. Security review
covered credential selection after redirects, source-scoped headers,
retry limits, curl handle lifetime and shared partial-pack preservation.
This is a review-only topic targeting
codex, based onmasterat2c3adbb2c475981e340c79fdc5e7f4f9b5d9054e. The separate pushworkflow rejects upstream's unpinned action references before compiling;
the
codextarget already pins them, so PR CI runs normally.Codex Task:
codex://threads/01a07eba-913d-7790-acd0-8f6310a57e28-frielbot