Skip to content

fix(checkouts): Basic auth for every git transport; a checkout names its origin - #327

Merged
smith-xyz merged 1 commit into
developmentfrom
fix/git-transport-auth-and-origin
Oct 31, 2026
Merged

smith-xyz merged 1 commit into
developmentfrom
fix/git-transport-auth-and-origin

Conversation

@smith-xyz

Copy link
Copy Markdown
Contributor

Fix-forward for the two findings in Michele's review of traust#336 (chubirka, CHANGES_REQUESTED) — both independently verified before fixing.

1. Git transport auth (her P1). Verified read-only against a real GitHub upload-pack advertisement with a valid token: Authorization: Bearer → 401 invalid credentials (even for public repos); Authorization: Basic x-access-token:<token> → 200. The merged ForgeApi.git_headers sent Bearer for GitHub hosts (the earlier Qodo fix covered GitLab's PRIVATE-TOKEN case only), so every authenticated deployment's mirror fetch failed. git_headers now sends Basic for every forge kind: x-access-token for GitHub kinds, oauth2 for GitLab.

2. A checkout names its origin (her P2). Verified: mirror worktrees carried no remote.origin (the bare mirror is fetched by explicit URL), while session run --checkout dispatches only the workspace path and vuln-scan/threat-model derive repository identity from git remote get-url origin in the checkout — a regression from the clone-based flow, which had origin. The mirror now records remote.origin.url, which every worktree shares, restoring the identity contract for sessions, skills and hand runs.

Changed tests

  • tests/v1/providers/test_forge_api.py::test_git_transport_auth… · old: github expected Bearer · new: expects Basic x-access-token · why: Bearer is refused by git smart-HTTP

  • tests/v1/providers/test_mirror.py token test · old: raw token asserted in the env value · new: asserts the exact Basic header · why: the token now rides base64-encoded

  • new: test_a_checkout_names_its_origin_the_skills_derive_identity_from

  • A person reviewed each changed test and the behaviour change is intended

@qodo-for-traust-security

qodo-for-traust-security Bot commented Oct 31, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Powered by Qodo

@smith-xyz

Copy link
Copy Markdown
Contributor Author

@qodo can you compare the impl from #326 with this PR - it looks to be adding the same change?

@qodo-for-traust-security

Copy link
Copy Markdown

can you compare the impl from #326 with this PR - it looks to be adding the same change?

I can’t fetch or inspect GitHub PR 326 from the tools available in this review, so I can’t reliably confirm whether it duplicates this PR.

This PR contains two distinct changes:

  • ForgeApi.git_headers: changes Git smart-HTTP authentication to Authorization: Basic ... for both GitHub and GitLab.
  • MirrorCheckout: configures remote.origin.url in the bare mirror so checkout consumers can derive repository identity.

If PR 326 changes either of those same code paths, it likely overlaps and should be compared commit-by-commit; please paste its diff or key files here and I can compare them precisely.

@smith-xyz

Copy link
Copy Markdown
Contributor Author

Overlap check with Michele’s #326: its Basic Git auth change duplicates this PR’s auth hunk, but #326 does not preserve remote.origin.url; #327 uniquely fixes that session/skill identity regression. #326 adds independent review-ref, eviction-locking and SHA-256 work, including an open Qodo trust-boundary question about fetching bare SHAs. Please coordinate order/integration and retain the origin test; do not land both as-is and assume the overlap is resolved.

@smith-xyz
smith-xyz requested a review from chubirka October 31, 2026 14:01
@smith-xyz
smith-xyz force-pushed the fix/git-transport-auth-and-origin branch from 282ebbe to 7b7bab6 Compare October 31, 2026 17:09
@smith-xyz

Copy link
Copy Markdown
Contributor Author

Rebased onto current development bcfdedd1 (#325/#326/#329 merged), new head 7b7bab6d (was 282ebbee). The Basic Git authentication behavior and its transport tests are already in development via #326, so the reconciled diff retains only this PR’s unique fix: set remote.origin.url on every mirror fetch before worktree creation; its regression verifies the checkout still names its HTTPS origin. Local make lint-fix, make test (8,440 passed), and pre-push coverage (96.69%) passed; CI/human review of the rebased PR remain gates. Downstream pins on 282ebbee must be updated to a reviewed reachable core commit.

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

Tip of the day
💡 Did you know, you can keep summaries lean with Findings visible per group, which tucks the rest behind a View link

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

@smith-xyz
smith-xyz merged commit e01d420 into development Oct 31, 2026
6 checks passed
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.

1 participant