Skip to content

Keep pasted pack tokens out of the stored pack URL - #2083

Merged
ppXD merged 1 commit into
mainfrom
fix/keep-pack-tokens-out-of-the-pack-url
Oct 7, 2026
Merged

ppXD merged 1 commit into
mainfrom
fix/keep-pack-tokens-out-of-the-pack-url

Conversation

@ppXD

@ppXD ppXD commented Oct 7, 2026

Copy link
Copy Markdown
Owner

Summary

  • A pack imported from a pasted git URL that embeds a token now stores pack.url without its userinfo, and that URL is the pack's identity. The URL exactly as cloned is sealed with IPayloadEncryptor in pack.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 calls POST /api/packs/{id}/import with a body of sourcePaths only, so no URL goes back and forth through the browser.
  • Sync (PackImportService.Sync.cs) and import-from-pack (PackImportService.Commit.cs) decrypt the sealed URL only for the clone. Both hand it to PackCloneFetcher, 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.
  • Existing rows: PackCloneUrlBackfillRecurringJob seals 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 plus duplicate_of_pack_id duplicates, and each keeps syncing from its own source. uq_pack_team_source excludes duplicates (0241_pack_sealed_clone_url.sql).
  • Public and ssh URLs are stored exactly as pasted, and nothing is sealed for them.

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

  • Unit: full suite, 12254 passed and 1 skipped. Covers the protector split and CarriesCredential, the read-model projection, job dispatch and cron, and the authorization inventory.
  • Integration (real Postgres, real git, a loopback remote that refuses requests without the fake token): 103 passed across the pack flows, including Keep pasted pack tokens out of messages and the clone config #2082's PackCloneCredentialFlowTests on the combined tree.
    • PackSealedSourceFlowTests.A_failed_clone_from_the_sealed_source_names_no_token_to_the_caller_or_in_the_log covers Sync and import-from-pack against a deleted ref. It checks the client message and the captured mediator log. It is red with main's PackCloneFetcher and 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_it covers the same token and a rotated one. It was red before the lookup change. ..._prefers_the_clean_pack_over_an_unsealed_fork guards the preference. Both were mutation-checked.
  • E2E over HTTP with real git and Postgres: PrivatePackCredentialE2ETests 3/3, including the error body of a failed clone from the sealed source, which is red with main's fetcher. RecurringJobWorkerSmokeE2ETests also passed.
  • Frontend (Node 22): pnpm lint (0 errors), tsc -b --noEmit, and SyncResultModal.test.tsx 2/2.
  • Merge Keep pasted pack tokens out of messages and the clone config #2082 first.
  • Deploy the Api pods before the Worker pods. Afterwards, check that the worker log has no Pack clone-URL backfill failed warnings.

@ppXD
ppXD changed the base branch from fix/keep-pack-clone-tokens-out-of-messages-and-config to main October 7, 2026 07:29
@ppXD
ppXD force-pushed the fix/keep-pack-tokens-out-of-the-pack-url branch from 4511ec1 to ce3b1cf Compare October 7, 2026 07:29
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.
@ppXD
ppXD merged commit 582b97e into main Oct 7, 2026
7 checks passed
@ppXD
ppXD deleted the fix/keep-pack-tokens-out-of-the-pack-url branch October 7, 2026 07:32
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