diff --git a/src-tauri/src/git.rs b/src-tauri/src/git.rs index 3b76c3f..1c2d379 100644 --- a/src-tauri/src/git.rs +++ b/src-tauri/src/git.rs @@ -2432,6 +2432,8 @@ const CRED_SERVICE: &str = "tauri_git_lite"; pub struct StoredCredential { pub username: String, pub password: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub mode: Option, } fn cred_entry(key: &str) -> Result { @@ -2445,17 +2447,27 @@ fn cred_entry(key: &str) -> Result { /// Returns the remote URL used for auth key derivation (upstream remote of the /// current branch, falling back to `origin`, then the first configured remote). #[tauri::command(async)] -pub fn get_remote_url(path: String) -> Result, String> { +pub fn get_remote_url( + path: String, + remote: Option, + push: Option, +) -> Result, String> { let repo = resolve_repo(&path)?; - let remote = upstream_remote_name(&repo).unwrap_or_else(|| "origin".to_string()); + let remote = match remote + .map(|value| value.trim().to_string()) + .filter(|value| !value.is_empty()) + { + Some(remote) => validate_remote_name(&repo, &remote, true)?, + None => upstream_remote_name(&repo).unwrap_or_else(|| "origin".to_string()), + }; - if let Some(url) = remote_url_for(&repo, &remote) { + if let Some(url) = remote_url_for_auth(&repo, &remote, push.unwrap_or(false)) { return Ok(Some(url)); } // origin missing → try the first configured remote if let Some(first) = first_remote_name(&repo) { if first != remote { - if let Some(url) = remote_url_for(&repo, &first) { + if let Some(url) = remote_url_for_auth(&repo, &first, push.unwrap_or(false)) { return Ok(Some(url)); } } @@ -2463,6 +2475,20 @@ pub fn get_remote_url(path: String) -> Result, String> { Ok(None) } +fn remote_url_for_auth(repo: &Path, remote: &str, push: bool) -> Option { + let mut command = git_command(); + command.arg("-C").arg(repo).args(["remote", "get-url"]); + if push { + command.arg("--push"); + } + let out = command.arg(remote).output().ok()?; + if !out.status.success() { + return None; + } + let url = String::from_utf8_lossy(&out.stdout).trim().to_string(); + if url.is_empty() { None } else { Some(url) } +} + fn remote_url_for(repo: &Path, remote: &str) -> Option { let out = git_command() .arg("-C") @@ -2614,9 +2640,22 @@ pub fn cred_load(key: String) -> Result, String> { } #[tauri::command(async)] -pub fn cred_save(key: String, username: String, password: String) -> Result<(), String> { +pub fn cred_save( + key: String, + username: String, + password: String, + mode: Option, +) -> Result<(), String> { let entry = cred_entry(&key)?; - let cred = StoredCredential { username, password }; + let mode = match mode.as_deref() { + Some("token") => Some("token".to_string()), + _ => Some("credentials".to_string()), + }; + let cred = StoredCredential { + username, + password, + mode, + }; let json = serde_json::to_string(&cred) .map_err(|err| format!("Could not serialize credentials: {err}"))?; entry @@ -4688,6 +4727,13 @@ fn run_git_clone( password: Option<&str>, ) -> Result<(), String> { let mut command = git_command(); + let has_explicit_credentials = matches!( + (username, password), + (Some(user), Some(pass)) if !user.is_empty() || !pass.is_empty() + ); + if has_explicit_credentials { + command.arg("-c").arg("credential.helper="); + } command .arg("clone") .arg("--") @@ -5649,10 +5695,20 @@ fn run_apply_patch_command( run_git(repo, args).map(|_| ()) } +static ASKPASS_COUNTER: AtomicU64 = AtomicU64::new(1); + +fn next_askpass_path(extension: &str) -> std::path::PathBuf { + let sequence = ASKPASS_COUNTER.fetch_add(1, Ordering::Relaxed); + std::env::temp_dir().join(format!( + "gitty-askpass-{}-{sequence}.{extension}", + std::process::id() + )) +} + #[cfg(unix)] fn write_askpass_script() -> Result { use std::os::unix::fs::PermissionsExt; - let path = std::env::temp_dir().join("gitlite_askpass.sh"); + let path = next_askpass_path("sh"); let script = "#!/bin/sh\ncase \"$1\" in\n *[Uu]sername*) printf '%s\\n' \"$GIT_CRED_USER\" ;;\n *) printf '%s\\n' \"$GIT_CRED_PASS\" ;;\nesac\n"; std::fs::write(&path, script) .map_err(|e| format!("Could not write authentication script: {e}"))?; @@ -5663,8 +5719,17 @@ fn write_askpass_script() -> Result { #[cfg(not(unix))] fn write_askpass_script() -> Result { - let path = std::env::temp_dir().join("gitlite_askpass.bat"); - let script = "@echo off\necho %1 | findstr /I \"sername\" >nul 2>&1\nif %errorlevel% == 0 (echo %GIT_CRED_USER%) else (echo %GIT_CRED_PASS%)\n"; + let path = next_askpass_path("bat"); + // Reading the value from PowerShell avoids cmd.exe interpreting special + // characters such as &, |, ^ or % from passwords and access tokens. + let script = r#"@echo off +echo %1 | findstr /I "sername" >nul 2>&1 +if %errorlevel% == 0 ( + powershell.exe -NoProfile -NonInteractive -Command "[Console]::Out.WriteLine($env:GIT_CRED_USER)" +) else ( + powershell.exe -NoProfile -NonInteractive -Command "[Console]::Out.WriteLine($env:GIT_CRED_PASS)" +) +"#; std::fs::write(&path, script) .map_err(|e| format!("Could not write authentication script: {e}"))?; Ok(path) @@ -5706,6 +5771,8 @@ where let askpass = write_askpass_script()?; let result = git_command() + .arg("-c") + .arg("credential.helper=") .arg("-C") .arg(repo) .args(args) @@ -7146,6 +7213,64 @@ mod tests { ); } + #[test] + fn credential_payload_remains_backward_compatible() { + let legacy: StoredCredential = + serde_json::from_str(r#"{"username":"alice","password":"secret"}"#) + .expect("legacy credential should deserialize"); + assert_eq!(legacy.username, "alice"); + assert_eq!(legacy.password, "secret"); + assert_eq!(legacy.mode, None); + + let token = StoredCredential { + username: "alice".to_string(), + password: "token".to_string(), + mode: Some("token".to_string()), + }; + let encoded = serde_json::to_string(&token).expect("credential should serialize"); + assert!(encoded.contains(r#""mode":"token""#)); + } + + #[test] + fn auth_remote_url_uses_the_requested_direction() { + let repo = init_temp_repo("auth_remote_url_direction"); + run_git_test( + &repo.path, + [ + "remote", + "add", + "origin", + "https://gitea.example/fetch/repo.git", + ], + ); + run_git_test( + &repo.path, + [ + "remote", + "set-url", + "--push", + "origin", + "https://gitea.example/push/repo.git", + ], + ); + + assert_eq!( + remote_url_for_auth(&repo.path, "origin", false).as_deref(), + Some("https://gitea.example/fetch/repo.git") + ); + assert_eq!( + remote_url_for_auth(&repo.path, "origin", true).as_deref(), + Some("https://gitea.example/push/repo.git") + ); + } + + #[test] + fn askpass_scripts_use_unique_paths() { + let first = next_askpass_path("test"); + let second = next_askpass_path("test"); + assert_ne!(first, second); + } + #[tokio::test] #[cfg_attr( windows, diff --git a/src/App.svelte b/src/App.svelte index 8e25b87..6ab1dd8 100644 --- a/src/App.svelte +++ b/src/App.svelte @@ -111,7 +111,6 @@ launchExternalTool, credLoad, credSave, - credDelete, getFilePatch, readConflict, resolveConflict, @@ -190,6 +189,7 @@ type UpdateToastState = "available" | "downloading" | "installed" | "error"; type AppView = "management" | "repository"; type CredentialAction = "push" | "pull" | "fetch" | "clone"; + type CredentialMode = "credentials" | "token"; type PendingDiscard = | { kind: "file"; files: GitFileStatus[]; staged: boolean } | { kind: "all-changes"; files: GitFileStatus[] } @@ -412,6 +412,9 @@ let credDialogAction: CredentialAction | null = null; let credDialogError = ""; let credDialogKey: string | null = null; + let credDialogUsername = ""; + let credDialogMode: CredentialMode = "credentials"; + const rejectedCredentialKeys = new Set(); let lastStatusFingerprint = ""; const AUTO_REFRESH_INTERVAL = 4000; let autoRefreshTimer: ReturnType | undefined; @@ -2207,6 +2210,7 @@ password?: string, key?: string | null, fromStore = false, + credentialMode: CredentialMode = "credentials", ) { if (isBusy) return; if (!remoteUrl) { errorMessage = "Enter a remote URL."; return; } @@ -2219,7 +2223,16 @@ if (!username && !password) { const stored = await loadStoredCredential(credentialKey); if (stored) { - await cloneRepo(remoteUrl, parentPath, directoryName, stored.username, stored.password, credentialKey, true); + const storedMode = credentialModeFor(stored); + if (credentialKey && rejectedCredentialKeys.has(credentialKey)) { + credDialogUsername = stored.username === "oauth2" ? "" : stored.username; + credDialogMode = storedMode; + credDialogAction = "clone"; + credDialogKey = credentialKey; + credDialogOpen = true; + return; + } + await cloneRepo(remoteUrl, parentPath, directoryName, stored.username, stored.password, credentialKey, true, storedMode); return; } } @@ -2260,7 +2273,9 @@ errorMessage = ""; setCloneDialogError(""); if (fromStore) { - if (credentialKey) void credDelete(credentialKey).catch(() => {}); + if (credentialKey) rejectedCredentialKeys.add(credentialKey); + credDialogUsername = username === "oauth2" ? "" : (username ?? ""); + credDialogMode = credentialMode; const detail = summarizeGitError(message); credDialogError = detail ? `${detail} — please sign in again.` @@ -2917,7 +2932,7 @@ async function pushLocalTag(tag: GitTag) { if (!activeRepoPath || isBusy) return; - const key = await currentCredKey(); + const key = await currentCredKey("push"); const stored = await loadStoredCredential(key); const credential = stored ?? null; @@ -3102,11 +3117,17 @@ }); } - // Resolve the keychain key (host/org) for the active repo's remote. - async function currentCredKey(): Promise { + function credentialModeFor(credential: StoredCredential): CredentialMode { + if (credential.mode === "token" || credential.username === "oauth2") return "token"; + return "credentials"; + } + + // Resolve the keychain key (host/org) from the exact remote URL used by the + // operation. Push URLs may intentionally differ from fetch URLs. + async function currentCredKey(action: "push" | "pull" | "fetch" = "fetch"): Promise { if (!activeRepoPath) return null; try { - const url = await getRemoteUrl(activeRepoPath); + const url = await getRemoteUrl(activeRepoPath, selectedRemote || undefined, action === "push"); return url ? orgKeyFromUrl(url) : null; } catch { return null; @@ -3122,21 +3143,37 @@ } } - async function openCredentialDialog(action: CredentialAction, key?: string | null) { + async function openCredentialDialog( + action: CredentialAction, + key?: string | null, + credential?: StoredCredential | null, + ) { if (!activeRepoPath && action !== "clone") return; credDialogError = ""; credDialogAction = action; - credDialogKey = key === undefined && action !== "clone" ? await currentCredKey() : (key ?? null); + credDialogKey = key === undefined && action !== "clone" + ? await currentCredKey(action) + : (key ?? null); + credDialogUsername = credential?.username === "oauth2" ? "" : (credential?.username ?? ""); + credDialogMode = credential ? credentialModeFor(credential) : "credentials"; credDialogOpen = true; trackEvent("credential_dialog_opened", { action, }); } - // Post-process a pull/push result: surface errors, and on rejected/expired - // credentials drop the stored entry and re-open the login dialog. - function handleRemoteResult(action: "push" | "pull" | "fetch", key: string | null, fromStore: boolean) { + // Post-process a pull/push result. Rejected credentials stay in the keychain + // so a temporary 401/403 cannot erase a valid token; the key is only skipped + // for the rest of this session until the user replaces it successfully. + function handleRemoteResult( + action: "push" | "pull" | "fetch", + key: string | null, + fromStore: boolean, + username: string, + mode: CredentialMode, + ) { if (!errorMessage) { + if (key) rejectedCredentialKeys.delete(key); credDialogOpen = false; credDialogAction = null; return; @@ -3147,7 +3184,9 @@ if (fromStore) { if (auth) { - if (key) void credDelete(key).catch(() => {}); + if (key) rejectedCredentialKeys.add(key); + credDialogUsername = username === "oauth2" ? "" : username; + credDialogMode = mode; const detail = summarizeGitError(message); credDialogError = detail ? `${detail} — please sign in again.` @@ -3160,6 +3199,7 @@ errorMessage = message; } } else { + if (auth && key) rejectedCredentialKeys.add(key); credDialogError = message || "Sign-in failed."; } } @@ -3169,6 +3209,7 @@ password: string, key: string | null, fromStore: boolean, + mode: CredentialMode, ) { errorMessage = ""; await runOperation("Pulling", async () => { @@ -3179,7 +3220,7 @@ changed_files: status?.files.length ?? 0, }); }); - handleRemoteResult("pull", key, fromStore); + handleRemoteResult("pull", key, fromStore, username, mode); } async function doActualFetch( @@ -3187,6 +3228,7 @@ password: string, key: string | null, fromStore: boolean, + mode: CredentialMode, ) { errorMessage = ""; await runOperation("Fetching", async () => { @@ -3199,7 +3241,7 @@ behind: status?.behind ?? 0, }); }); - handleRemoteResult("fetch", key, fromStore); + handleRemoteResult("fetch", key, fromStore, username, mode); } async function doActualPush( @@ -3207,6 +3249,7 @@ password: string, key: string | null, fromStore: boolean, + mode: CredentialMode, ) { errorMessage = ""; await runOperation("Pushing", async () => { @@ -3235,12 +3278,12 @@ if (!fromStore) credDialogError = ""; await runOperation("Pulling before push", async () => { - applyStatus(await pull(activeRepoPath, username, password)); + applyStatus(await pull(activeRepoPath, username, password, pullStrategy, selectedRemote || undefined)); await refreshRepositoryViews(activeRepoPath); }); if (errorMessage) { - handleRemoteResult("pull", key, fromStore); + handleRemoteResult("pull", key, fromStore, username, mode); return; } @@ -3252,7 +3295,7 @@ } await runOperation("Pushing after pull", async () => { - applyStatus(await push(activeRepoPath, username, password)); + applyStatus(await push(activeRepoPath, username, password, false, selectedRemote || undefined)); await refreshRepositoryViews(activeRepoPath, { files: false }); trackEvent("repository_pushed_after_pull", { from_stored_credential: fromStore ? 1 : 0, @@ -3261,14 +3304,31 @@ }); } - handleRemoteResult("push", key, fromStore); + handleRemoteResult("push", key, fromStore, username, mode); } - async function handleCredentialSubmit(username: string, password: string, save: boolean) { + async function handleCredentialSubmit( + username: string, + password: string, + save: boolean, + mode: CredentialMode, + ) { const key = credDialogKey; - if (credDialogAction === "pull") await doActualPull(username, password, key, false); - else if (credDialogAction === "push") await doActualPush(username, password, key, false); - else if (credDialogAction === "fetch") await doActualFetch(username, password, key, false); + // Honour "Save in keychain" immediately. A successful authentication + // followed by an unrelated refresh/non-fast-forward error must not lose the + // token and force the user to type it again on the next operation. + if (save && key) { + try { + await credSave(key, username, password, mode); + } catch (error) { + credDialogError = errorToMessage(error); + return; + } + } + + if (credDialogAction === "pull") await doActualPull(username, password, key, false, mode); + else if (credDialogAction === "push") await doActualPush(username, password, key, false, mode); + else if (credDialogAction === "fetch") await doActualFetch(username, password, key, false, mode); else if (credDialogAction === "clone" && pendingClone) { await cloneRepo( pendingClone.remoteUrl, @@ -3278,17 +3338,9 @@ password, key, false, + mode, ); } - - // Only persist once the operation actually succeeded (dialog has closed). - if (!credDialogOpen && save && key) { - try { - await credSave(key, username, password); - } catch (error) { - errorMessage = errorToMessage(error); - } - } } async function startRemoteAction(action: "push" | "pull" | "fetch") { @@ -3296,17 +3348,18 @@ trackEvent("remote_action_started", { action, }); - const key = await currentCredKey(); + const key = await currentCredKey(action); const stored = await loadStoredCredential(key); - if (stored) { - if (action === "pull") await doActualPull(stored.username, stored.password, key, true); - else if (action === "fetch") await doActualFetch(stored.username, stored.password, key, true); - else await doActualPush(stored.username, stored.password, key, true); + if (stored && (!key || !rejectedCredentialKeys.has(key))) { + const mode = credentialModeFor(stored); + if (action === "pull") await doActualPull(stored.username, stored.password, key, true, mode); + else if (action === "fetch") await doActualFetch(stored.username, stored.password, key, true, mode); + else await doActualPush(stored.username, stored.password, key, true, mode); return; } - await openCredentialDialog(action, key); + await openCredentialDialog(action, key, stored); } async function fetchRepo() { @@ -5375,8 +5428,17 @@ action={credDialogAction} error={credDialogError} {isBusy} + initialUsername={credDialogUsername} + initialMode={credDialogMode} onSubmit={handleCredentialSubmit} - onCancel={() => { credDialogOpen = false; credDialogAction = null; credDialogError = ""; credDialogKey = null; }} + onCancel={() => { + credDialogOpen = false; + credDialogAction = null; + credDialogError = ""; + credDialogKey = null; + credDialogUsername = ""; + credDialogMode = "credentials"; + }} /> {/if} diff --git a/src/lib/components/CredentialDialog.svelte b/src/lib/components/CredentialDialog.svelte index 893d413..c881b03 100644 --- a/src/lib/components/CredentialDialog.svelte +++ b/src/lib/components/CredentialDialog.svelte @@ -1,4 +1,5 @@ @@ -92,7 +97,7 @@
@@ -116,27 +121,25 @@ aria-pressed={mode === "token"} >