[ezra21997-cell]

What

remoteHeadSha() in packages/cli/src/utils/skillsManifest.ts runs git ls-remote with credential helpers and askpass disabled. It also skips git entirely when the slug does not look like owner/repo.

Why

The skills update check SHA-pins the manifest with git ls-remote https://github.com/<slug>.git refs/heads/main. A private or missing repo answers with an auth challenge. GIT_TERMINAL_PROMPT=0 only silences git's own terminal prompt, so credential helpers and askpass still run. On Windows the default helper is Git Credential Manager, which opens a sign-in window. The check blocks until someone closes it, including the background update check, which has no console.

A malformed slug such as owner/repo/extra gets a plain 403 from GitHub and never reaches a credential helper. So the slug check is defensive: it saves a pointless git process and keeps arbitrary text away from git's arguments, but it is not what fixes the reported window.

Related work

Fixes #4702

How

• git -c credential.helper= ls-remote …: an empty value clears every configured helper. Verified for system, global, URL-scoped and GIT_CONFIG_COUNT config.
• GIT_ASKPASS="" stops askpass. An empty value also skips the core.askPass and SSH_ASKPASS fallbacks.
• GCM_INTERACTIVE=never is belt-and-braces for GCM itself. It is redundant once helpers are cleared, but harmless.
• REPO_SLUG_PATTERN = /^[\w.-]+\/[\w.-]+$/: a non-matching slug returns null before git is spawned. The caller already falls back to the branch raw URL in that case, so the result for invalid input is unchanged, minus the git process.

The repos read here are public, so no credentials are lost. A real ls-remote against github.com/heygen-com/hyperframes with these flags still returns main's SHA.

Test plan

• Unit tests added (6):

  • the exact argv and env of the ls-remote call
  • a valid slug still pins the manifest to the commit ls-remote reports
  • four malformed slugs never spawn git

  Without the source change, 5 of the 6 fail. The positive test also fails when the regex is mutated to reject everything. The suite uses mockReset so a queued mockImplementationOnce cannot leak into the next test.

• Manual testing (git 2.43): a local HTTP server answering 401, with a logging credential helper in each of system, global, URL-scoped and GIT_CONFIG_COUNT config, and a logging askpass on core.askPass and SSH_ASKPASS.

  Run	helper get calls	askpass calls
  before (GIT_TERMINAL_PROMPT=0 only)	4 (all four sources)	2
  after	0	0
  only -c credential.helper=	0	2
  only the env vars	4	0

  Both halves of the fix are needed.

• tsc --noEmit, oxlint, oxfmt --check, check-comment-citations, comment-ratchet, commitlint and the lefthook pre-commit hooks pass.

• Full packages/cli suite: 4349 passed. 1 failed: desktopApp.test.ts "still opens a project it cannot write to", which also fails on unmodified main when run as root (it relies on chmod 555).

• Not run on a real Windows machine with GCM installed, or on git older than 2.43.

• Comments follow CONTRIBUTING.md "Comments".

Follow-ups (not in this PR)

1. packages/cli/src/whisper/manager.ts, buildFromSource(): git clone --depth 1 <WHISPER_REPO> has the same exposure (only GIT_TERMINAL_PROMPT=0). The URL is a fixed public repo, so it only bites if that repo moves or disappears. A shared gitNoPromptArgs / gitNoPromptEnv helper would cover both call sites.
2. fetchRemoteManifest(): the raw-URL fallback still builds raw.githubusercontent.com/${slug}/main/… from the unvalidated slug, so owner/repo?x=1 turns the rest of the path into a query string. No security impact (same host, unauthenticated GET). Validating once at the top would turn a 404 into a clear error, but --source callers would need checking.
3. REPO_SLUG_PATTERN accepts owner/. and owner/... That is not exploitable, because the slug only ever sits inside an https://github.com/ URL, but a stricter pattern would match GitHub's naming rules exactly.
4. Pre-existing: fetchRemoteManifest treats any source starting with http as a URL, so an owner named https-tools/... would be misrouted.

Review record

Two independent review rounds ran on this patch.

• First round: no functional bug found. Its two should-fix items, the docstring wording and the missing positive-path test, are applied.
• Second round: fact-checked this description and corrected two claims, the 403 behaviour of malformed slugs and the manual-test table, which is now re-measured. It also tightened the tests (mockReset, a justified as unknown as cast) and the docstring. All are applied.

🤖 Generated with Claude Code

[jrusso1020]

Reviewed at 2c459e0. LGTM.

What I checked:

• Tests pass at head: skillsManifest.test.ts (65/65) and commands/skills.test.ts (40/40). With skillsManifest.ts reverted to the parent commit, 5 of the 6 new tests fail, and they pass again with the fix.
• I reproduced the env/config behaviour on git 2.50.1. The setup was a local server answering 401 with a logging credential helper configured globally, URL-scoped and via GIT_CONFIG_COUNT, plus a logging askpass on core.askPass and SSH_ASKPASS. Before the fix: 6 helper calls and 2 askpass calls. With this PR's argv and env: 0 and 0. With only -c credential.helper=, askpass still runs. With only GIT_ASKPASS=, the helpers still run. So both halves are needed, as the description says.
• The normal path still works: the exact invocation against github.com/heygen-com/hyperframes returns main's SHA. The repo is public and the manifest fetch from raw.githubusercontent.com is unauthenticated, so clearing helpers costs nothing.
• The slug guard (skillsManifest.ts:113, :668) only short-circuits to the existing branch-URL fallback. URL and local-path sources are routed away before remoteHeadSha.

The only thing I couldn't verify is how Git for Windows reads an empty GIT_ASKPASS from the environment. It doesn't change the verdict, because -c credential.helper= is what keeps GCM out, and that part is platform-independent.

— Rames

[jrusso1020]

Thanks for this! It's approved, but main only accepts verified commits. Your commit 2c459e0eaf is signed, but GitHub doesn't recognize the key (unknown_key), so it can't merge yet.

To fix it:

1. Add the public key you signed with to GitHub as a Signing key (Settings → SSH and GPG keys; for SSH keys pick Key type: Signing key).
2. Make sure the commit email (ezra21997@gmail.com) is a verified email on your GitHub account.
3. Re-sign and push so GitHub checks it again:

   git commit --amend --no-edit -S
   git push --force-with-lease

Docs: About commit signature verification.

Once the commit shows Verified, we'll re-approve and merge.

— Rames

[ezra21997-cell]

Thanks for the review, and for reproducing it on git 2.50.1! I've re-signed the commit with a signing key on my account, and GitHub now shows it as Verified. The content is unchanged from the version you approved.

[jrusso1020]

Thanks, @ezra21997-cell. This was approved and in the merge queue, but another PR that merged ahead of it touched the same lines, so GitHub took it out of the queue with a merge conflict. Could you rebase onto the latest main? If it's a clean rebase with no code changes, we'll requeue it right away.

— Rames

[ezra21997-cell]

Rebased onto current main. The only conflict was in skillsManifest.test.ts: #6168 deleted the pruneOrphanedLockEntries tests right after where this PR adds the remoteHeadSha tests. I kept the new tests and left that block deleted. No code changes; skillsManifest.ts is the same as the approved d0333d2.

Re-checked: oxfmt, oxlint, tsc, the commit hooks and the skills tests pass. The commit is re-signed and shows Verified, so it should be ready to requeue.

[miguel-heygen]

Thanks for the quick rebase. I compared the rebased commit with the one that was approved (d0333d27): the change to packages/cli/src/utils/skillsManifest.ts is identical, and the tests differ only by one blank line next to the block #6168 removed. It merges cleanly with current main, and the commit shows as verified.

The only thing left is CI: the full workflows at 88fd6fd4 haven't run yet (fork PRs need a maintainer to approve the run). Once they're green, this should be ready to requeue.

Reviewed head 88fd6fd4.

Verdict: COMMENT
Reasoning: The rebase is code-identical to the approved commit, but full CI hasn't run at the new head yet.