From 2709753fbf3140dd6468f982b2c463b71eb79802 Mon Sep 17 00:00:00 2001 From: Ash Shah <494shah@gmail.com> Date: Sun, 9 Aug 2026 17:42:16 -0700 Subject: [PATCH 1/2] fix: bind a confirmed action to the repository it was confirmed against MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every confirmation in gitActions is awaited with no timeout, and the work that follows reads `appState.repo` when it RUNS, not when the dialog opened. Switch repositories mid-prompt and the confirmed action lands on the new one. The discard snapshot guard does not cover this: `expected_diff` pins the DIFF, not the repository. Two clones or worktrees holding the same path with the same diff pass the check, and the wrong copy is irreversibly discarded. Codex flagged discardHunk and discardLines. The same gap is in every awaited dialog in the file, so all nine are bound rather than the two reported: runDestructive every caller (reset, rebase, amend, reword, ...) runDestructiveRebase ditto discardHunk reported discardLines reported discard (files) destructive clean (untracked) destructive force push --force-with-lease create pull request runRemote credential retry — `fn` re-reads the repo, so a switch during the sign-in prompt would push to a remote the user never chose Refusing rather than retargeting is deliberate: the user authorised an action against what they were looking at, and silently applying it elsewhere is a different action. This is the third appearance of the shape — guard, unbounded await, act — after the diff snapshot and the sidebar ref actions, so the audit covered the whole file rather than the reported sites. Reported by Codex review on the v0.4.0 promotion (#30). npm run check 530 files 0 errors; npm test 393 passed; cargo test 232 + 7. Co-Authored-By: Claude Opus 5 --- src/lib/gitActions.ts | 53 +++++++++++++++++++++++++++++++++++-------- 1 file changed, 43 insertions(+), 10 deletions(-) diff --git a/src/lib/gitActions.ts b/src/lib/gitActions.ts index a9aa500..2e0374a 100644 --- a/src/lib/gitActions.ts +++ b/src/lib/gitActions.ts @@ -544,6 +544,28 @@ function discarded(ok: boolean): boolean { return ok; } +/** + * Bind an action to the repository the user actually confirmed it against. + * + * Every confirmation here is awaited with no timeout, and the work that follows reads + * `appState.repo` when it runs — not when the dialog opened. Switch repositories mid-prompt and + * the confirmed action lands on the new one. The discard guards do not save us: `expected_diff` + * pins the DIFF, not the repository, so two clones or worktrees holding the same path with the + * same diff pass that check and the wrong copy is irreversibly discarded. + * + * Capture before the await, verify after it, refuse if it moved. Refusing rather than retargeting + * is deliberate: the user authorised an action against what they were looking at, and silently + * applying it somewhere else is not the same action. + */ +function sameRepoAfterPrompt(): () => boolean { + const opened = appState.repo; + return () => { + if (appState.repo === opened) return true; + appState.status = "Repository changed while that dialog was open — nothing was done."; + return false; + }; +} + // Run a working-copy op (non-destructive): guard → run → refresh working changes + graph. async function runWorktree(label: string, fn: () => Promise): Promise { if (!isTauri()) { @@ -588,13 +610,14 @@ async function runDestructive( appState.status = "Open a repository first."; return false; } + const sameRepo = sameRepoAfterPrompt(); const { confirmed, backup } = await dialogs.confirmDestructive({ title: label, consequence, confirmLabel: label, backupDefault: appState.autoBackupDestructive, }); - if (!confirmed) return false; + if (!confirmed || !sameRepo()) return false; try { appState.status = `${label}…`; const res = await fn(backup); @@ -630,13 +653,14 @@ async function runDestructiveRebase( appState.status = "Open a repository first."; return false; } + const sameRepo = sameRepoAfterPrompt(); const { confirmed, backup } = await dialogs.confirmDestructive({ title: label, consequence, confirmLabel: label, backupDefault: appState.autoBackupDestructive, }); - if (!confirmed) return false; + if (!confirmed || !sameRepo()) return false; try { appState.status = `${label}…`; const res = await fn(backup); @@ -687,9 +711,12 @@ async function runRemote( let outcome = await fn((l) => appState.pushRemoteLog(l)); if (outcome.authFailed) { - // Auth failed — prompt for credentials and retry once. + // Auth failed — prompt for credentials and retry once. `fn` reads `appState.repo` when + // invoked, so a switch during the prompt would retry against the NEW repository with the + // credentials just entered — pushing to a remote the user never chose. + const sameRepo = sameRepoAfterPrompt(); const creds = await dialogs.confirmCredentials({ title: `${label}: sign in` }); - if (creds) { + if (creds && sameRepo()) { outcome = await fn((l) => appState.pushRemoteLog(l), creds); } // If user cancelled the credentials dialog, fall through with the original outcome. @@ -922,13 +949,14 @@ export const gitActions = { // spanning the entire file, so a hunk discard can revert every unstaged change in // it — a boundary the user cannot see from the dialog. const n = changedLines; + const sameRepo = sameRepoAfterPrompt(); const confirmed = await dialogs.confirm({ title: "Discard hunk", message: discardMessage(n, "changed line", path, revertsToIndex), confirmLabel: "Discard", danger: true, }); - if (!confirmed) return false; + if (!confirmed || !sameRepo()) return false; return discarded( await runWorktree(`Discard hunk in ${path}`, () => api.discardHunk(appState.repo, path, hunkIndex, expectedDiff, appState.effectiveDiffContext), @@ -951,13 +979,14 @@ export const gitActions = { return false; } const n = selected.length; + const sameRepo = sameRepoAfterPrompt(); const confirmed = await dialogs.confirm({ title: "Discard lines", message: discardMessage(n, "line", path, revertsToIndex), confirmLabel: "Discard", danger: true, }); - if (!confirmed) return false; + if (!confirmed || !sameRepo()) return false; return discarded( await runWorktree(`Discard ${n} line(s) in ${path}`, () => api.discardLines( @@ -1040,13 +1069,14 @@ export const gitActions = { return false; } const n = paths.length; + const sameRepo = sameRepoAfterPrompt(); const confirmed = await dialogs.confirm({ title: "Discard changes", message: `Permanently discard changes to ${n} file${n === 1 ? "" : "s"}. This cannot be undone. (Stash instead to keep them.)`, confirmLabel: "Discard", danger: true, }); - if (!confirmed) return false; + if (!confirmed || !sameRepo()) return false; return runWorktree(`Discard ${n} file(s)`, () => api.discard(appState.repo, paths)); }, @@ -1060,13 +1090,14 @@ export const gitActions = { return false; } const n = paths.length; + const sameRepo = sameRepoAfterPrompt(); const confirmed = await dialogs.confirm({ title: "Remove untracked files", message: `Permanently discard changes to ${n} file${n === 1 ? "" : "s"}. This cannot be undone. (Stash instead to keep them.)`, confirmLabel: "Discard", danger: true, }); - if (!confirmed) return false; + if (!confirmed || !sameRepo()) return false; return runWorktree(`Clean ${n} file(s)`, () => api.clean(appState.repo, paths)); }, @@ -1134,6 +1165,7 @@ export const gitActions = { return false; } if (forceWithLease) { + const sameRepo = sameRepoAfterPrompt(); const ok = await dialogs.confirm({ title: "Force push", message: @@ -1141,7 +1173,7 @@ export const gitActions = { confirmLabel: "Force push", danger: true, }); - if (!ok) return false; + if (!ok || !sameRepo()) return false; } // Determine the current branch + whether it already has an upstream tracking ref. @@ -1225,8 +1257,9 @@ export const gitActions = { .branchSubjects(appState.repo, bases[0], 20) .catch(() => [] as string[]); const prefill = prefillFromSubjects(subjects, branch); + const sameRepo = sameRepoAfterPrompt(); const v = await dialogs.openCreatePr(branch, bases, prefill.title, prefill.body); - if (!v) return; + if (!v || !sameRepo()) return; await run("Create pull request", async () => { const number = await api.githubPrCreate(appState.repo, v.title, v.body, v.base, v.draft); // The auto-push may have just created the branch's upstream. From f2fee34c48219f25e9e6e1fc898797d763f8c11a Mon Sep 17 00:00:00 2001 From: Ash Shah <494shah@gmail.com> Date: Sun, 9 Aug 2026 17:59:30 -0700 Subject: [PATCH 2/2] fix: capture the repository before the first await, not before the dialog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two P1s on the previous commit, both the same mistake: the capture has to sit at the point the user's intent is fixed, and I put it immediately before the prompt instead. Where an earlier await exists in the same flow, a switch during THAT await is recorded as the "original" repo and the guard waves the action through. - `runRemote` awaits the first push/pull attempt before any credentials prompt. Captured before that attempt, so an auth failure arriving after a switch can no longer authorise a retry against the new repository — which would have pushed to a remote the user never chose, with the credentials just entered. - `createPullRequest` awaits `branchSubjects` before opening the dialog, while `branch` and `bases` were read from the repo BEFORE it. Captured before that lookup, so the guard compares against the repo those selections came from. The audit script had the same blind spot as the code: it checked for a capture near the dialog rather than ahead of every await in the flow. Rewritten to walk back to the enclosing function and reject a capture with any earlier await — and to strip comments and string bodies first, because its own first version flagged the word "await" inside the comments added here. Both invariants now hold: 9 captures, none preceded by a real earlier await; 9 awaited dialogs, all with a guarded use. npm run check 530 files 0 errors; npm test 393 passed; cargo test 232 + 7. Co-Authored-By: Claude Opus 5 --- src/lib/gitActions.ts | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/lib/gitActions.ts b/src/lib/gitActions.ts index 2e0374a..90bbc96 100644 --- a/src/lib/gitActions.ts +++ b/src/lib/gitActions.ts @@ -705,6 +705,10 @@ async function runRemote( appState.status = "Open a repository first."; return false; } + // Capture before the FIRST attempt, not before the credentials prompt: a switch during + // that first await would otherwise be recorded as the "original" repo and the guard would + // wave the retry through. + const sameRepo = sameRepoAfterPrompt(); appState.startRemoteProgress(label); try { // First attempt: no credentials (system helper / SSH agent / keychain). @@ -712,9 +716,8 @@ async function runRemote( if (outcome.authFailed) { // Auth failed — prompt for credentials and retry once. `fn` reads `appState.repo` when - // invoked, so a switch during the prompt would retry against the NEW repository with the - // credentials just entered — pushing to a remote the user never chose. - const sameRepo = sameRepoAfterPrompt(); + // invoked, so a switch anywhere in this flow would retry against the NEW repository with + // the credentials just entered — pushing to a remote the user never chose. const creds = await dialogs.confirmCredentials({ title: `${label}: sign in` }); if (creds && sameRepo()) { outcome = await fn((l) => appState.pushRemoteLog(l), creds); @@ -1252,12 +1255,15 @@ export const gitActions = { (n): n is string => !!n && others.includes(n), ) ?? null; const bases = preferred ? [preferred, ...others.filter((n) => n !== preferred)] : others; + // Capture before the first await: `branch` and `bases` were read from the CURRENT repo + // above, and a switch during the subjects lookup would otherwise be recorded as the + // original — creating a PR in the new repo from the old one's selections. + const sameRepo = sameRepoAfterPrompt(); // Best-effort prefill from the commit subjects on base..HEAD. const subjects = await api .branchSubjects(appState.repo, bases[0], 20) .catch(() => [] as string[]); const prefill = prefillFromSubjects(subjects, branch); - const sameRepo = sameRepoAfterPrompt(); const v = await dialogs.openCreatePr(branch, bases, prefill.title, prefill.body); if (!v || !sameRepo()) return; await run("Create pull request", async () => {