refactor: rename functions for clarity and consistency (#229)

* refactor: rename functions for clarity and consistency
- Rename path safety functions (isPathSafe→isShellSafePath, etc.)
- Rename SSH validation functions (isKeyLoaded→checkKeyLoadedInAgent, etc.)
- Resolve function name shadowing (validateSshKeyPath→validateSshKeyPathFormat)
- Improve check prefix functions (checkWorkspaceBoundary→assertWithinWorkspaceBoundary)
- Rename validator objects (validateNoControlChars→controlCharValidator)
- Improve helper function names (isSymbolicLink→isSymbolicLinkSafe)
- Add naming convention documentation to validators/common.ts
- Add deprecated aliases for backwards compatibility

🖥️ IDE: [Cursor](https://cursor.sh)
🔌 Extension: [Claude Code](https://claude.ai/download)

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
Model-Raw: claude-opus-4-5-20251101

* fix: remove incorrect @deprecated from isShellSafePath
isShellSafePath checks for shell metacharacters and path traversal,
while validatePathSecurity checks for filesystem attacks. They serve
different purposes and cannot be substituted.
Changed @deprecated to @see to indicate validatePathSecurity as an
alternative for comprehensive security checks, not a replacement.

🖥️ IDE: [Cursor](https://cursor.sh)
🔌 Extension: [Claude Code](https://claude.ai/download)

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
Model-Raw: claude-opus-4-5-20251101

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>

Author: nullvariant

extensions/git-id-switcher/src/identity/inputValidator.ts

@@ -33,9 +33,18 @@ export interface ValidationResult {
 }
 
 /**
- * Validate a string field for dangerous patterns
+ * Validate a string field for dangerous command injection patterns.
+ *
+ * @remarks
+ * **Naming convention**: Named with `validate` prefix and explicit purpose
+ * (`ForDangerousPatterns`) because this function checks for security-critical
+ * patterns like command injection, not format validity.
+ *
+ * @param value - The field value to validate
+ * @param fieldName - The name of the field (for error messages)
+ * @param errors - Array to accumulate validation errors
  */
-function validateField(
+function validateFieldForDangerousPatterns(
   value: string | undefined,
   fieldName: string,
   errors: string[]
@@ -72,13 +81,23 @@ function validateEmail(email: string | undefined, errors: string[]): void {
 }
 
 /**
- * Validate SSH key path
+ * Validate SSH key path format.
  *
+ * This validates only the format/structure of the path:
  * - Must be absolute or start with ~
  * - No path traversal (..)
  * - No dangerous characters
+ *
+ * @remarks
+ * **Naming convention**: Named with `Format` suffix to distinguish from
+ * `validateSshKeyPath` in pathUtils.ts which performs full path normalization
+ * and symlink resolution. This function only validates format.
+ *
+ * **Terminology**:
+ * - `valid`: Format/structure is correct (this function)
+ * - `secure`: Resistant to security attacks (pathUtils.validateSshKeyPath)
  */
-function validateSshKeyPath(
+function validateSshKeyPathFormat(
   sshKeyPath: string | undefined,
   errors: string[]
 ): void {
@@ -97,7 +116,7 @@ function validateSshKeyPath(
   }
 
   // No dangerous characters in path
-  validateField(sshKeyPath, 'sshKeyPath', errors);
+  validateFieldForDangerousPatterns(sshKeyPath, 'sshKeyPath', errors);
 }
 
 /**
@@ -169,15 +188,15 @@ export function validateIdentity(identity: Identity): ValidationResult {
   }
 
   // Text field validation (dangerous patterns)
-  validateField(identity.name, 'name', errors);
-  validateField(identity.email, 'email', errors);
-  validateField(identity.service, 'service', errors);
-  validateField(identity.description, 'description', errors);
-  validateField(identity.icon, 'icon', errors);
+  validateFieldForDangerousPatterns(identity.name, 'name', errors);
+  validateFieldForDangerousPatterns(identity.email, 'email', errors);
+  validateFieldForDangerousPatterns(identity.service, 'service', errors);
+  validateFieldForDangerousPatterns(identity.description, 'description', errors);
+  validateFieldForDangerousPatterns(identity.icon, 'icon', errors);
 
   // Format-specific validation
   validateEmail(identity.email, errors);
-  validateSshKeyPath(identity.sshKeyPath, errors);
+  validateSshKeyPathFormat(identity.sshKeyPath, errors);
   validateGpgKeyId(identity.gpgKeyId, errors);
   validateSshHost(identity.sshHost, errors);
 
@@ -235,18 +254,28 @@ export function validateIdentities(identities: Identity[]): ValidationResult {
 }
 
 /**
- * Check if a path is safe (no injection risk)
+ * Check if a path is safe for shell execution (no injection risk).
+ *
+ * Used for SSH key paths and other file system operations where the path
+ * may be passed to shell commands.
  *
- * Used for SSH key paths and other file system operations.
+ * @remarks
+ * **Naming convention**: Named with `is` prefix (boolean predicate) and
+ * `ShellSafe` to indicate this specifically checks for shell injection safety,
+ * not comprehensive path security.
  *
- * @deprecated Use isSecurePath from security/pathValidator instead.
- * This function is kept for backwards compatibility and maintains the original
- * behavior of checking for path traversal and shell metacharacters.
+ * **Terminology**:
+ * - `safe`: Safe for a specific context (shell execution in this case)
+ * - `secure`: Resistant to security attacks (use `validatePathSecurity` instead)
+ *
+ * @see validatePathSecurity from security/pathValidator for comprehensive
+ * security checks. This function maintains the original behavior of checking
+ * for path traversal and shell metacharacters only.
  *
  * @param path - The path to check
- * @returns true if the path appears safe
+ * @returns true if the path appears safe for shell execution
  */
-export function isPathSafe(path: string): boolean {
+export function isShellSafePath(path: string): boolean {
   // Original check: path traversal detection (simple check for "..")
   // This is stricter than isSecurePath's traversal detection
   if (hasPathTraversal(path)) {
@@ -262,3 +291,8 @@ export function isPathSafe(path: string): boolean {
 
   return true;
 }
+
+/**
+ * @deprecated Use `isShellSafePath` instead. This alias will be removed in a future version.
+ */
+export const isPathSafe = isShellSafePath;

extensions/git-id-switcher/src/security/commandAllowlist.ts

@@ -7,11 +7,11 @@
  * @see https://owasp.org/www-project-application-security-verification-standard/
  */
 
-import { isSecurePath, isPathArgument } from './pathValidator';
+import { validatePathSecurity, isPathArgument } from './pathValidator';
 import { validateCombinedFlags } from './flagValidator';
 
 // Re-export for backwards compatibility
-export { isSecurePath, isPathArgument, SecurePathResult } from './pathValidator';
+export { validatePathSecurity, isSecurePath, isPathArgument, SecurePathResult } from './pathValidator';
 export { validateCombinedFlags, CombinedFlagResult } from './flagValidator';
 
 /**
@@ -172,10 +172,16 @@ function resolveSubcommandConfig(
 /* c8 ignore stop */
 
 /**
- * Check if arg is a value for a previous option that expects values.
+ * Check if an argument is a value for a preceding flag that expects values.
+ *
+ * @remarks
+ * **Naming convention**: Named with `is` prefix (boolean-like predicate) and
+ * explicit parameter description (`ArgumentValue`, `PrecedingFlag`) to clarify
+ * the relationship between the current argument and the previous flag.
+ *
  * @internal
  */
-function isValueForPreviousOption(
+function isArgumentValueForPrecedingFlag(
   arg: string,
   index: number,
   argsToValidate: string[],
@@ -225,7 +231,7 @@ function validateFlagArgument(
 function validatePathSafety(arg: string): AllowlistCheckResult | 'safe' {
   const looksLikePathForSecurity = isPathArgument(arg) || arg.includes('/');
   if (looksLikePathForSecurity) {
-    const pathResult = isSecurePath(arg);
+    const pathResult = validatePathSecurity(arg);
     if (!pathResult.valid) {
       return { allowed: false, reason: `Path argument rejected: ${pathResult.reason}` };
     }
@@ -269,7 +275,7 @@ function validateSingleArgument(
   const looksLikePathForSecurity = isPathArgument(arg) || arg.includes('/');
 
   // 1. Check if this is a value for a previous option
-  const valueCheck = isValueForPreviousOption(arg, index, argsToValidate, allowedOptionsWithValues);
+  const valueCheck = isArgumentValueForPrecedingFlag(arg, index, argsToValidate, allowedOptionsWithValues);
   if (valueCheck === 'is_value') {
     return 'continue';
   }

extensions/git-id-switcher/src/security/flagValidator.ts

@@ -39,10 +39,16 @@ const ALLOWED_COMBINED_PATTERNS: Record<string, readonly string[]> = {
 };
 
 /**
- * Check for security-critical character issues in a flag.
+ * Detect unsafe characters in a flag argument.
+ *
+ * @remarks
+ * **Naming convention**: Named with `detect` prefix because this function
+ * looks for and reports security issues (whitespace, null bytes, control chars,
+ * invisible Unicode) rather than just validating format.
+ *
  * @internal
  */
-function checkFlagSecurityChars(flag: string): CombinedFlagResult | null {
+function detectUnsafeCharsInFlag(flag: string): CombinedFlagResult | null {
   // CRITICAL: Check for leading/trailing whitespace (potential obfuscation)
   if (flag !== flag.trim()) {
     return { valid: false, reason: 'Flag contains leading or trailing whitespace' };
@@ -98,10 +104,16 @@ function validateCombinedFlagChars(flagChars: string): CombinedFlagResult | null
 }
 
 /**
- * Check combined flag against allowlist patterns and individual flags.
+ * Validate combined flag characters against the allowlist.
+ *
+ * @remarks
+ * **Naming convention**: Named with `validate` prefix because this returns
+ * a result object with `valid` boolean and optional `reason`. The explicit
+ * `AgainstAllowlist` suffix clarifies what it's being validated against.
+ *
  * @internal
  */
-function checkCombinedFlagAllowlist(
+function validateCombinedFlagAgainstAllowlist(
   flagChars: string,
   command: string,
   allowedArgs: readonly string[]
@@ -165,7 +177,7 @@ export function validateCombinedFlags(
   }
 
   // CRITICAL: Security character checks (whitespace, null bytes, control chars, invisible unicode)
-  const securityCheck = checkFlagSecurityChars(flag);
+  const securityCheck = detectUnsafeCharsInFlag(flag);
   if (securityCheck) {
     return securityCheck;
   }
@@ -175,7 +187,7 @@ export function validateCombinedFlags(
 
   /* c8 ignore start - Post-normalization check for edge cases in Unicode normalization */
   // CRITICAL: Re-check after normalization
-  const normalizedSecurityCheck = checkFlagSecurityChars(normalizedFlag);
+  const normalizedSecurityCheck = detectUnsafeCharsInFlag(normalizedFlag);
   if (normalizedSecurityCheck) {
     return {
       valid: false,
@@ -242,5 +254,5 @@ export function validateCombinedFlags(
   }
 
   // Check against allowlist patterns and individual flags
-  return checkCombinedFlagAllowlist(flagChars, command, allowedArgs);
+  return validateCombinedFlagAgainstAllowlist(flagChars, command, allowedArgs);
 }

extensions/git-id-switcher/src/security/pathNormalizer.ts

@@ -8,7 +8,7 @@
 import * as os from 'node:os';
 import * as path from 'node:path';
 import * as fs from 'node:fs';
-import { isSecurePath, SecurePathResult } from './pathValidator';
+import { validatePathSecurity, SecurePathResult } from './pathValidator';
 import { PATH_MAX } from '../core/constants';
 import { hasNullByte, hasPathTraversal } from '../validators/common';
 import { resolveSymlinksSecurely } from './pathSymlinkResolver';
@@ -97,13 +97,18 @@ function validatePathLength(
 }
 
 /**
- * Security check specifically for normalized paths
+ * Security check specifically for normalized paths.
  *
  * After path.normalize(), we need to verify:
  * 1. Path doesn't escape to unexpected locations
  * 2. No remaining traversal patterns
+ *
+ * @remarks
+ * **Naming convention**: Named with `validate` prefix and `Security` suffix because:
+ * - `validate*()` returns a result object with `valid` boolean and optional `reason`
+ * - `Security` indicates this checks for attack resistance post-normalization
  */
-function isSecurePathAfterNormalization(
+function validateNormalizedPathSecurity(
   normalizedPath: string,
   originalPath: string
 ): SecurePathResult {
@@ -166,7 +171,7 @@ function resolveAndValidateSymlinks(
   const resolvedPath = symlinkResult.resolvedPath ?? normalizedPath;
 
   // Re-validate after symlink resolution
-  const symlinkCheck = isSecurePathAfterNormalization(resolvedPath, inputPath);
+  const symlinkCheck = validateNormalizedPathSecurity(resolvedPath, inputPath);
   if (!symlinkCheck.valid) {
     return { invalidResult: { valid: false, originalPath: inputPath, reason: `Post-symlink check failed: ${symlinkCheck.reason}` } };
   }
@@ -210,7 +215,7 @@ export function normalizeAndValidatePath(
   if (inputLengthCheck) return inputLengthCheck;
 
   // Step 3: Pre-normalization security check on raw input
-  const preCheck = isSecurePath(inputPath);
+  const preCheck = validatePathSecurity(inputPath);
   if (!preCheck.valid) {
     return { valid: false, originalPath: inputPath, reason: `Pre-normalization check failed: ${preCheck.reason}` };
   }
@@ -233,7 +238,7 @@ export function normalizeAndValidatePath(
   /* c8 ignore stop */
 
   // Step 6: Post-normalization security check
-  const postCheck = isSecurePathAfterNormalization(normalizedPath, inputPath);
+  const postCheck = validateNormalizedPathSecurity(normalizedPath, inputPath);
   /* c8 ignore start - Defense-in-depth: pre-check catches most issues, this is fallback */
   if (!postCheck.valid) {
     return { valid: false, originalPath: inputPath, reason: `Post-normalization check failed: ${postCheck.reason}` };

extensions/git-id-switcher/src/security/pathUnicodeDetector.ts

@@ -54,11 +54,33 @@ export const createInvisibleUnicodeValidator = (suffix: string = ''): Validator
 // Pre-built Validators
 // ============================================================================
 
-/** Validates no control characters (pre-normalization) */
-export const validateNoControlChars = createControlCharValidator();
+/**
+ * Validator that rejects paths containing control characters (pre-normalization).
+ *
+ * @remarks
+ * **Naming convention**: Named as a validator object (noun) rather than a function
+ * because it's a pre-built validator instance created by a factory function.
+ */
+export const controlCharValidator = createControlCharValidator();
+
+/**
+ * Validator that rejects paths containing invisible Unicode characters (pre-normalization).
+ *
+ * @remarks
+ * **Naming convention**: Named as a validator object (noun) rather than a function
+ * because it's a pre-built validator instance created by a factory function.
+ */
+export const invisibleUnicodeValidator = createInvisibleUnicodeValidator();
+
+/**
+ * @deprecated Use `controlCharValidator` instead. This alias will be removed in a future version.
+ */
+export const validateNoControlChars = controlCharValidator;
 
-/** Validates no invisible Unicode characters (pre-normalization) */
-export const validateNoInvisibleUnicode = createInvisibleUnicodeValidator();
+/**
+ * @deprecated Use `invisibleUnicodeValidator` instead. This alias will be removed in a future version.
+ */
+export const validateNoInvisibleUnicode = invisibleUnicodeValidator;
 
 /**
  * Normalizes Unicode to NFC for consistent comparison

extensions/git-id-switcher/src/security/pathUtils.ts

@@ -116,10 +116,16 @@ function validateSubmodulePathPreChecks(
 }
 
 /**
- * Check if normalized path is within workspace boundary.
+ * Assert that normalized path is within workspace boundary.
+ *
+ * @remarks
+ * **Naming convention**: Named with `assert` prefix because this function
+ * returns an error result if the assertion fails (path escapes workspace),
+ * or null if the assertion passes (path is within boundary).
+ *
  * @internal
  */
-function checkWorkspaceBoundary(
+function assertWithinWorkspaceBoundary(
   normalizedPath: string,
   workspacePath: string,
   originalPath: string
@@ -187,7 +193,7 @@ function verifySubmoduleSymlinks(
     const resolvedPath = resolvedByRealpath;
 
     // SECURITY: Re-check workspace boundary after symlink resolution
-    const boundaryCheck = checkWorkspaceBoundary(resolvedPath, normalizedWorkspace, originalPath);
+    const boundaryCheck = assertWithinWorkspaceBoundary(resolvedPath, normalizedWorkspace, originalPath);
     if (boundaryCheck) {
       return {
         valid: false,
@@ -396,7 +402,7 @@ export function validateSubmodulePath(
   const normalizedSubmodulePath = result.normalizedPath!;
 
   // SECURITY: Ensure the normalized path is still within workspace
-  const boundaryCheck = checkWorkspaceBoundary(normalizedSubmodulePath, normalizedWorkspace, submodulePath);
+  const boundaryCheck = assertWithinWorkspaceBoundary(normalizedSubmodulePath, normalizedWorkspace, submodulePath);
   /* c8 ignore start - Boundary escape after normalization (requires path traversal which is blocked earlier) */
   if (boundaryCheck) {
     return boundaryCheck;