Uses hashes for diff validation

Co-authored-by: Eric Amodio <[email protected]>

Author: axosoft-ramint

src/env/browser/crypto.ts

@@ -1,4 +1,5 @@
 import { base64 } from './base64';
+import { convertToHex } from './hex';
 import { md5 as _md5 } from './md5';
 
 export function getNonce(): string {
@@ -9,6 +10,12 @@ export function md5(data: string, encoding: 'base64' | 'hex' = 'hex'): string {
 	return _md5(data, encoding);
 }
 
+export async function sha256(data: string, encoding: 'base64' | 'hex' = 'hex'): Promise<string> {
+	const buffer = new TextEncoder().encode(data);
+	const bytes = new Uint8Array(await globalThis.crypto.subtle.digest('SHA-256', buffer));
+	return encoding === 'base64' ? base64(bytes) : convertToHex(bytes);
+}
+
 export function uuid(): string {
 	return globalThis.crypto.randomUUID();
 }

src/env/browser/hex.ts

@@ -24,16 +24,18 @@ const precomputedByteToHex = [
 	'f7', 'f8', 'f9', 'fa', 'fb', 'fc', 'fd', 'fe', 'ff',
 ];
 
-export function encodeUtf8Hex(s: string): string {
-	const bytes = textEncoder.encode(s);
-
+export function convertToHex(bytes: Uint8Array): string {
 	const hex = new Array<string>(bytes.length);
 	for (let i = 0; i < bytes.length; ++i) {
 		hex[i] = precomputedByteToHex[bytes[i]];
 	}
 	return hex.join('');
 }
 
+export function encodeUtf8Hex(s: string): string {
+	return convertToHex(textEncoder.encode(s));
+}
+
 export function decodeUtf8Hex(hex: string): string {
 	const matches = hex.match(/(\w{2})/g);
 	if (matches === null) return '';

src/env/node/crypto.ts

@@ -8,6 +8,10 @@ export function md5(data: string, encoding: 'base64' | 'hex' = 'hex'): string {
 	return createHash('md5').update(data).digest(encoding);
 }
 
+export async function sha256(data: string, encoding: 'base64' | 'hex' = 'hex'): Promise<string> {
+	return Promise.resolve(createHash('sha256').update(data).digest(encoding));
+}
+
 export function uuid(): string {
 	return randomUUID();
 }

src/env/node/hex.ts

@@ -1,3 +1,7 @@
+export function convertToHex(bytes: Uint8Array): string {
+	return Buffer.from(bytes).toString('hex');
+}
+
 export function encodeUtf8Hex(s: string): string {
 	return Buffer.from(s, 'utf8').toString('hex');
 }

src/webviews/apps/plus/composer/components/app.ts

@@ -72,23 +72,6 @@ const composerFeedbackUrl = 'https://github.com/gitkraken/vscode-gitlens/discuss
 
 @customElement('gl-composer-app')
 export class ComposerApp extends LitElement {
-	@consume({ context: stateContext, subscribe: true })
-	state!: State;
-
-	@consume({ context: ipcContext })
-	private _ipc!: HostIpc;
-
-	// Internal history management
-	private history: ComposerHistory = {
-		resetState: null,
-		undoStack: [],
-		redoStack: [],
-	};
-
-	// Debounce timer for commit message updates
-	private commitMessageDebounceTimer?: number;
-	private commitMessageBeingEdited: string | null = null; // Track which commit is being edited
-
 	static override styles = [
 		boxSizingBase,
 		focusableBaseStyles,
@@ -359,6 +342,23 @@ export class ComposerApp extends LitElement {
 		`,
 	];
 
+	@consume({ context: stateContext, subscribe: true })
+	state!: State;
+
+	@consume({ context: ipcContext })
+	private _ipc!: HostIpc;
+
+	// Internal history management
+	private history: ComposerHistory = {
+		resetState: null,
+		undoStack: [],
+		redoStack: [],
+	};
+
+	// Debounce timer for commit message updates
+	private commitMessageDebounceTimer?: number;
+	private commitMessageBeingEdited: string | null = null; // Track which commit is being edited
+
 	@query('gl-commits-panel')
 	commitsPanel!: CommitsPanel;
 

src/webviews/plus/composer/composerWebview.ts

@@ -1,6 +1,6 @@
 import type { ConfigurationChangeEvent } from 'vscode';
 import { CancellationTokenSource, commands, Disposable, window } from 'vscode';
-import { md5 } from '@env/crypto';
+import { md5, sha256 } from '@env/crypto';
 import type { ContextKeys } from '../../../constants.context';
 import type { ComposerTelemetryContext, Sources } from '../../../constants.telemetry';
 import type { Container } from '../../../container';
@@ -81,7 +81,7 @@ import {
 	createHunksFromDiffs,
 	createSafetyState,
 	getWorkingTreeDiffs,
-	validateCombinedDiff,
+	validateResultingDiff,
 	validateSafetyState,
 } from './utils';
 
@@ -354,7 +354,7 @@ export class ComposerWebviewProvider implements WebviewProvider<State, State, Co
 		};
 
 		// Create safety state snapshot for validation
-		const safetyState = await createSafetyState(repo, diffs, currentBranch, baseCommit);
+		const safetyState = await createSafetyState(repo, diffs, baseCommit.sha);
 
 		const aiEnabled = this.getAiEnabled();
 		const aiModel = await this.container.ai.getModel({ silent: true }, { source: 'composer' });
@@ -1063,13 +1063,13 @@ export class ComposerWebviewProvider implements WebviewProvider<State, State, Co
 				throw new Error(errorMessage);
 			}
 
-			const combinedDiff = (
+			const resultingDiff = (
 				await repo.git.diff.getDiff?.(shas[shas.length - 1], params.baseCommit.sha, {
 					notation: '...',
 				})
 			)?.contents;
 
-			if (!combinedDiff) {
+			if (!resultingDiff) {
 				this._context.errors.operation.count++;
 				this._context.operations.finishAndCommit.errorCount++;
 				const errorMessage = 'Failed to get combined diff';
@@ -1080,7 +1080,13 @@ export class ComposerWebviewProvider implements WebviewProvider<State, State, Co
 				throw new Error(errorMessage);
 			}
 
-			if (!validateCombinedDiff(params.safetyState, combinedDiff, this._context.diff.unstagedIncluded)) {
+			if (
+				!validateResultingDiff(
+					params.safetyState,
+					await sha256(resultingDiff),
+					this._context.diff.unstagedIncluded,
+				)
+			) {
 				// Clear loading state and show safety error
 				await this.host.notify(DidFinishCommittingNotification, undefined);
 				this._context.errors.safety.count++;

src/webviews/plus/composer/protocol.ts

@@ -47,20 +47,27 @@ export interface ComposerBaseCommit {
 export interface ComposerSafetyState {
 	repoPath: string;
 	headSha: string;
-	branchName: string;
-	branchRefSha: string;
-	worktreeName: string;
-	stagedDiff: string | null; // null if no staged changes when composer opened
-	unstagedDiff: string | null; // null if no unstaged changes when composer opened
-	unifiedDiff: string | null; // null if no changes when composer opened
-	timestamp: number;
+	// branchName: string;
+	// branchRefSha: string;
+	// worktreeName: string;
+	hashes: {
+		staged: string | null;
+		unstaged: string | null;
+		unified: string | null;
+	};
+
+	// stagedDiff: string | null; // null if no staged changes when composer opened
+	// unstagedDiff: string | null; // null if no unstaged changes when composer opened
+	// unifiedDiff: string | null; // null if no changes when composer opened
+	// timestamp: number;
 }
 
 export interface State extends WebviewState {
 	// data model
 	hunks: ComposerHunk[];
-	commits: ComposerCommit[];
 	hunkMap: ComposerHunkMap[];
+
+	commits: ComposerCommit[];
 	baseCommit: ComposerBaseCommit;
 	safetyState: ComposerSafetyState;
 
@@ -121,13 +128,11 @@ export const initialState: Omit<State, keyof WebviewState> = {
 	safetyState: {
 		repoPath: '',
 		headSha: '',
-		branchName: '',
-		branchRefSha: '',
-		worktreeName: '',
-		stagedDiff: null,
-		unstagedDiff: null,
-		unifiedDiff: null,
-		timestamp: 0,
+		hashes: {
+			staged: null,
+			unstaged: null,
+			unified: null,
+		},
 	},
 	selectedCommitId: null,
 	selectedCommitIds: new Set<string>(),

src/webviews/plus/composer/utils.ts

@@ -1,5 +1,4 @@
-import type { GitBranch } from '../../../git/models/branch';
-import type { GitCommit } from '../../../git/models/commit';
+import { sha256 } from '@env/crypto';
 import type { GitDiff, ParsedGitDiff } from '../../../git/models/diff';
 import type { Repository } from '../../../git/models/repository';
 import { uncommitted, uncommittedStaged } from '../../../git/models/revision';
@@ -336,36 +335,20 @@ export async function getWorkingTreeDiffs(repo: Repository): Promise<WorkingTree
 export async function createSafetyState(
 	repo: Repository,
 	diffs: WorkingTreeDiffs,
-	currentBranch: GitBranch,
-	headCommit: GitCommit,
+	headSha: string,
 ): Promise<ComposerSafetyState> {
-	// Get current worktree information
-	const worktrees = await repo.git.worktrees?.getWorktrees();
-	const currentWorktree = worktrees?.find(wt => wt.branch?.id === currentBranch?.id);
-
-	if (!currentBranch?.name) {
-		throw new Error('Cannot create safety state: no current branch found');
-	}
-	if (!currentBranch?.sha) {
-		throw new Error('Cannot create safety state: no current branch SHA found');
-	}
-	if (!headCommit?.sha) {
+	if (!headSha) {
 		throw new Error('Cannot create safety state: no HEAD commit found');
 	}
-	if (!currentWorktree?.name) {
-		throw new Error('Cannot create safety state: no current worktree found');
-	}
 
 	return {
 		repoPath: repo.path,
-		headSha: headCommit.sha,
-		branchName: currentBranch.name,
-		branchRefSha: currentBranch.sha,
-		worktreeName: currentWorktree.name,
-		stagedDiff: diffs.staged?.contents ?? null,
-		unstagedDiff: diffs.unstaged?.contents ?? null,
-		unifiedDiff: diffs.unified?.contents ?? null,
-		timestamp: Date.now(),
+		headSha: headSha,
+		hashes: {
+			staged: diffs.staged?.contents ? await sha256(diffs.staged.contents) : null,
+			unstaged: diffs.unstaged?.contents ? await sha256(diffs.unstaged.contents) : null,
+			unified: diffs.unified?.contents ? await sha256(diffs.unified.contents) : null,
+		},
 	};
 }
 
@@ -393,44 +376,25 @@ export async function validateSafetyState(
 			errors.push(`HEAD commit changed from "${safetyState.headSha}" to "${currentHeadSha}"`);
 		}
 
-		// 3. Check current branch
-		const currentBranch = await repo.git.branches.getBranch();
-		const currentBranchName = currentBranch?.name;
-		if (!currentBranchName) {
-			errors.push('Current branch could not be determined');
-		} else if (currentBranchName !== safetyState.branchName) {
-			errors.push(`Branch changed from "${safetyState.branchName}" to "${currentBranchName}"`);
-		}
-
-		// 4. Check branch ref SHA
-		const currentBranchSha = currentBranch?.sha;
-		if (!currentBranchSha) {
-			errors.push('Current branch SHA could not be determined');
-		} else if (currentBranchSha !== safetyState.branchRefSha) {
-			errors.push(`Branch ref changed from "${safetyState.branchRefSha}" to "${currentBranchSha}"`);
-		}
+		// 2. Smart diff validation - only check diffs for sources being committed
+		if (hunksBeingCommitted?.length) {
+			const { staged, unstaged /*, unified*/ } = await getWorkingTreeDiffs(repo);
 
-		// 5. Check worktree state
-		const worktrees = await repo.git.worktrees?.getWorktrees();
-		const currentWorktree = worktrees?.find(wt => wt.branch?.id === currentBranch?.id);
-		const currentWorktreeName = currentWorktree?.name ?? 'main';
-		if (currentWorktreeName !== safetyState.worktreeName) {
-			errors.push(`Worktree changed from "${safetyState.worktreeName}" to "${currentWorktreeName}"`);
-		}
-
-		// 6. Smart diff validation - only check diffs for sources being committed
-		if (hunksBeingCommitted && hunksBeingCommitted.length > 0) {
-			const { staged, unstaged } = await getWorkingTreeDiffs(repo);
+			const hashes = {
+				staged: staged?.contents ? await sha256(staged.contents) : null,
+				unstaged: unstaged?.contents ? await sha256(unstaged.contents) : null,
+				// unified: unified?.contents ? await sha256(unified.contents) : null,
+			};
 
 			// Check if any hunks from staged source are being committed
 			const hasStagedHunks = hunksBeingCommitted.some(h => h.source === 'staged');
-			if (hasStagedHunks && (staged?.contents ?? null) !== safetyState.stagedDiff) {
+			if (hasStagedHunks && hashes.staged !== safetyState.hashes.staged) {
 				errors.push('Staged changes have been modified since composer opened');
 			}
 
 			// Check if any hunks from unstaged source are being committed
 			const hasUnstagedHunks = hunksBeingCommitted.some(h => h.source === 'unstaged');
-			if (hasUnstagedHunks && (unstaged?.contents ?? null) !== safetyState.unstagedDiff) {
+			if (hasUnstagedHunks && hashes.unstaged !== safetyState.hashes.unstaged) {
 				errors.push('Unstaged changes have been modified since composer opened');
 			}
 		}
@@ -442,21 +406,12 @@ export async function validateSafetyState(
 	}
 }
 
-/** Validates combined output diff against input diff, based on whether unstaged changes are included or not */
-export function validateCombinedDiff(
+/** Validates resulting output diff against input diff, based on whether unstaged changes are included or not */
+export function validateResultingDiff(
 	safetyState: ComposerSafetyState,
-	combinedDiff: string,
+	diffHash: string,
 	includeUnstagedChanges: boolean,
 ): boolean {
-	try {
-		const { stagedDiff, unifiedDiff } = safetyState;
-
-		if (includeUnstagedChanges) {
-			return combinedDiff === unifiedDiff;
-		}
-
-		return combinedDiff === stagedDiff;
-	} catch {
-		return false;
-	}
+	const { hashes } = safetyState;
+	return diffHash === (includeUnstagedChanges ? hashes.unified : hashes.staged);
 }