Repository navigation
Keep pasted pack tokens out of the stored pack URL - #2083
Merged
Merged
Conversation
ppXD
changed the base branch from
fix/keep-pack-clone-tokens-out-of-messages-and-config
to
main
October 7, 2026 07:29
ppXD
force-pushed
the
fix/keep-pack-tokens-out-of-the-pack-url
branch
from
October 7, 2026 07:29
4511ec1 to
ce3b1cf
Compare
A pack imported from a git URL that embedded a token stored that URL
verbatim in pack.url. The pack list and detail returned it to every
team member, Viewers included; the Library rendered it as a link; and
the add-after-sync flow sent it back to import-url.
pack.url now holds the URL without userinfo and is the pack's identity.
The URL exactly as cloned is sealed with IPayloadEncryptor in
pack.encrypted_clone_url, set only when it carried userinfo. Sync and
the new POST /api/packs/{id}/import decrypt it just for the clone, so a
private pack keeps syncing and the add imports into the pack by id
rather than re-resolving a URL that can no longer clone. Re-pasting
with a rotated token now updates the same pack instead of forking one.
Both hand the decrypted URL to PackCloneFetcher, so this builds on
#2082, which names a failed clone's URL without its userinfo, redacts
it from git's stderr and strips it from the checkout's origin. Without
it, a Sync or an add whose clone fails after authenticating (a deleted
ref, say) returns the token in the error body to any member who may
write agents, and logs it.
SQL cannot run the encryptor, so existing rows are sealed by a
ten-minute recurring backfill. Until it reaches a row, the row still
syncs from its URL and the read model strips userinfo on the way out.
A re-import of that repository lands in the row and seals it: the
lookup also matches a stored URL that differs only by its credential,
so pasting the same or a rotated token neither forks the pack nor
leaves its history on the pack the backfill would mark a duplicate.
Legacy forks of one repository settle into a holder plus duplicates
(duplicate_of_pack_id) that keep syncing from their own source.
Pods that predate this change cannot read the seal. Until the rollout
completes they fail to Sync a sealed private pack, and their
import-url fails for a repository whose legacy fork became a holder
plus a duplicate. The backfill runs only where Hangfire processes
jobs, so in an Api/Worker split roll the Api pods out first.
Tokens pasted before this change were already exposed and should be
rotated.
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
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.
Summary
pack.urlwithout its userinfo, and that URL is the pack's identity. The URL exactly as cloned is sealed withIPayloadEncryptorinpack.encrypted_clone_url, only when it carried userinfo. The pack list and detail read model (PackService.ToSummary) also strips userinfo, so legacy rows stop being returned the moment this deploys. The Library's add-after-sync now callsPOST /api/packs/{id}/importwith a body ofsourcePathsonly, so no URL goes back and forth through the browser.PackImportService.Sync.cs) and import-from-pack (PackImportService.Commit.cs) decrypt the sealed URL only for the clone. Both hand it toPackCloneFetcher, so this builds on Keep pasted pack tokens out of messages and the clone config #2082: that PR names a failed clone's URL without its userinfo, redacts it from git's stderr, strips it from the checkout's origin and clones into an owner-only directory. This branch contains Keep pasted pack tokens out of messages and the clone config #2082's commit and must merge after it.PackCloneUrlBackfillRecurringJobseals them every 10 minutes, 50 rows per tick. Until it reaches a row, a re-import of the same repository lands in that row and seals it. The row is matched by its URL without the credential, and a clean pack is preferred when one exists, so pasting the same or a rotated token never forks the pack. Legacy forks become one holder plusduplicate_of_pack_idduplicates, and each keeps syncing from its own source.uq_pack_team_sourceexcludes duplicates (0241_pack_sealed_clone_url.sql).Tokens pasted before this change were already exposed and should be rotated.
Rollout
Pods that predate this change cannot read the seal. Until the rollout completes, they fail to Sync a sealed private pack. Their import-url also fails ("Sequence contains more than one element") for a repository whose legacy fork became a holder plus a duplicate. The backfill runs only where Hangfire processes jobs, so in an Api/Worker split, roll out the Api pods before the Worker pods.
Test plan
CarriesCredential, the read-model projection, job dispatch and cron, and the authorization inventory.PackCloneCredentialFlowTestson the combined tree.PackSealedSourceFlowTests.A_failed_clone_from_the_sealed_source_names_no_token_to_the_caller_or_in_the_logcovers Sync and import-from-pack against a deleted ref. It checks the client message and the captured mediator log. It is red withmain'sPackCloneFetcherand green with Keep pasted pack tokens out of messages and the clone config #2082's.PackCloneUrlBackfillFlowTests.A_re_import_before_the_backfill_lands_in_the_unsealed_legacy_pack_and_seals_itcovers the same token and a rotated one. It was red before the lookup change...._prefers_the_clean_pack_over_an_unsealed_forkguards the preference. Both were mutation-checked.PrivatePackCredentialE2ETests3/3, including the error body of a failed clone from the sealed source, which is red withmain's fetcher.RecurringJobWorkerSmokeE2ETestsalso passed.pnpm lint(0 errors),tsc -b --noEmit, andSyncResultModal.test.tsx2/2.Pack clone-URL backfill failedwarnings.