refactor(validation): unify UI and identity layer validation as SSoT

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

@@ -13,6 +13,7 @@ import {
   isValidEmail,
   hasPathTraversal,
   isValidIdentityId,
+  hasDangerousChars,
   GPG_KEY_REGEX,
   SSH_HOST_REGEX,
   DANGEROUS_PATTERNS,
@@ -36,16 +37,16 @@ export interface ValidationResult {
 /**
  * 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.
+ * Checks both hasDangerousChars() (SAFE_TEXT_REGEX — catches control characters
+ * and shell metacharacters) and DANGEROUS_PATTERNS (catches literal hex escape
+ * sequences like `\x00` in text). This ensures consistency with UI-layer
+ * validation while retaining detection of text-level injection patterns.
  *
  * @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 validateFieldForDangerousPatterns(
+export function validateFieldForDangerousPatterns(
   value: string | undefined,
   fieldName: string,
   errors: string[]
@@ -54,18 +55,29 @@ function validateFieldForDangerousPatterns(
     return;
   }
 
+  // Check DANGEROUS_PATTERNS first for specific error messages
+  // (includes hex escape sequence detection that SAFE_TEXT_REGEX doesn't cover)
   for (const { pattern, description } of DANGEROUS_PATTERNS) {
     if (pattern.test(value)) {
       errors.push(`${fieldName} contains ${description}`);
-      break;
+      return;
     }
   }
+
+  // Also check hasDangerousChars (SAFE_TEXT_REGEX) for control characters
+  // not covered by DANGEROUS_PATTERNS (e.g., 0x02-0x09, 0x7F)
+  if (hasDangerousChars(value)) {
+    errors.push(`${fieldName} contains control characters`);
+  }
 }
 
 /**
- * Validate an email address
+ * Validate an email address format and length.
+ *
+ * @param email - The email to validate
+ * @param errors - Array to accumulate validation errors
  */
-function validateEmail(email: string | undefined, errors: string[]): void {
+export function validateEmailField(email: string | undefined, errors: string[]): void {
   if (!email) {
     return;
   }
@@ -199,7 +211,7 @@ export function validateIdentity(identity: Identity): ValidationResult {
   validateFieldForDangerousPatterns(identity.icon, 'icon', errors);
 
   // Format-specific validation
-  validateEmail(identity.email, errors);
+  validateEmailField(identity.email, errors);
   validateSshKeyPathFormat(identity.sshKeyPath, errors);
   validateGpgKeyId(identity.gpgKeyId, errors);
   validateSshHost(identity.sshHost, errors);
@@ -280,13 +292,16 @@ export function validateIdentities(identities: Identity[]): ValidationResult {
  * @returns true if the path appears safe for shell execution
  */
 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)) {
     return false;
   }
 
-  // Original check: shell metacharacters
+  // Control characters (0x00-0x1F, 0x7F) and shell metacharacters
+  if (hasDangerousChars(path)) {
+    return false;
+  }
+
+  // Text-level patterns (hex escape sequences like literal \x00)
   for (const { pattern } of DANGEROUS_PATTERNS) {
     if (pattern.test(path)) {
       return false;

extensions/git-id-switcher/src/test/e2e/identityManager.test.ts

@@ -2982,9 +2982,9 @@ describe('identityManager E2E Test Suite', function () {
       });
     });
 
-    describe('service field: hasDangerousCharsForText()', () => {
-      it('should accept service with ampersand (AT&T)', async () => {
-        // ampersand is allowed in text fields
+    describe('service field: validateFieldForDangerousPatterns()', () => {
+      it('should reject service with ampersand', async () => {
+        // ampersand is a shell metacharacter blocked by SAFE_TEXT_REGEX
         const mockVSCode = createMockVSCode({
           identities: [TEST_IDENTITIES.work],
           quickPickSelections: [
@@ -2997,7 +2997,7 @@ describe('identityManager E2E Test Suite', function () {
 
         const result = await showEditProfileFlow(TEST_IDENTITIES.work);
 
-        assert.strictEqual(result, true, 'Service with ampersand should be accepted');
+        assert.strictEqual(result, false, 'Service with ampersand should be rejected');
       });
 
       it('should reject service with backtick', async () => {
@@ -3033,9 +3033,9 @@ describe('identityManager E2E Test Suite', function () {
       });
     });
 
-    describe('description field: hasDangerousCharsForText()', () => {
-      it('should accept description with angle brackets (for display)', async () => {
-        // angle brackets are allowed in text fields for descriptions
+    describe('description field: validateFieldForDangerousPatterns()', () => {
+      it('should reject description with angle brackets', async () => {
+        // angle brackets are shell metacharacters blocked by SAFE_TEXT_REGEX
         const mockVSCode = createMockVSCode({
           identities: [TEST_IDENTITIES.work],
           quickPickSelections: [
@@ -3048,7 +3048,7 @@ describe('identityManager E2E Test Suite', function () {
 
         const result = await showEditProfileFlow(TEST_IDENTITIES.work);
 
-        assert.strictEqual(result, true, 'Description with angle brackets should be accepted');
+        assert.strictEqual(result, false, 'Description with angle brackets should be rejected');
       });
 
       it('should reject description with backtick', async () => {

extensions/git-id-switcher/src/test/validation.test.ts

@@ -5,7 +5,8 @@
  */
 
 import * as assert from 'node:assert';
-import { validateIdentity, validateIdentities, isPathSafe } from '../identity/inputValidator';
+import { validateIdentity, validateIdentities, isPathSafe, validateFieldForDangerousPatterns } from '../identity/inputValidator';
+import { hasDangerousChars } from '../validators/common';
 import { Identity } from '../identity/identity';
 
 /**
@@ -376,6 +377,121 @@ function testIsPathSafe(): void {
   console.log('✅ All isPathSafe tests passed!');
 }
 
+/**
+ * Test suite for validation consistency between UI layer and identity layer
+ *
+ * Invariant: validateFieldForDangerousPatterns rejects a SUPERSET of what
+ * hasDangerousChars rejects. Anything hasDangerousChars catches, the identity
+ * layer also catches. The identity layer additionally catches text-level
+ * patterns (hex escape sequences) that are safe bytes but dangerous text.
+ *
+ * Issue-00103: Unify validation logic between UI and identity layers.
+ */
+function testValidationConsistency(): void {
+  console.log('Testing validation consistency (UI ↔ identity layer)...');
+
+  // Category 1: Inputs rejected by BOTH hasDangerousChars and validateFieldForDangerousPatterns
+  // (actual dangerous bytes: shell metacharacters and control characters)
+  const bothReject = [
+    { value: 'test$(cmd)', reason: 'command substitution ($)' },
+    { value: 'test`id`', reason: 'backtick injection' },
+    { value: 'test|cat', reason: 'pipe' },
+    { value: 'test&bg', reason: 'ampersand' },
+    { value: 'test<in', reason: 'angle bracket (<)' },
+    { value: 'test>out', reason: 'angle bracket (>)' },
+    { value: 'test(group)', reason: 'parentheses' },
+    { value: 'test{brace}', reason: 'braces' },
+    { value: 'test\ninjection', reason: 'newline' },
+    { value: 'test\rinjection', reason: 'carriage return' },
+    { value: 'test\0null', reason: 'null byte' },
+    // Control characters caught by hasDangerousChars (SAFE_TEXT_REGEX)
+    // AND now also by validateFieldForDangerousPatterns (Issue-00103 fix)
+    { value: 'test\u0002ctrl', reason: 'STX control character (0x02)' },
+    { value: 'test\u0007bell', reason: 'BEL control character (0x07)' },
+    { value: 'test\u001Bescape', reason: 'ESC control character (0x1B)' },
+    { value: 'test\u007Fdel', reason: 'DEL control character (0x7F)' },
+  ];
+
+  for (const { value, reason } of bothReject) {
+    const uiRejects = hasDangerousChars(value);
+    const errors: string[] = [];
+    validateFieldForDangerousPatterns(value, 'test', errors);
+    const identityRejects = errors.length > 0;
+
+    assert.strictEqual(uiRejects, true, `hasDangerousChars should reject: ${reason}`);
+    assert.strictEqual(identityRejects, true, `validateFieldForDangerousPatterns should reject: ${reason}`);
+  }
+
+  // Category 2: Inputs rejected ONLY by validateFieldForDangerousPatterns
+  // (literal text patterns like \x00 — safe bytes but dangerous as text)
+  const identityOnlyRejects = [
+    { value: String.raw`test\x00user`, reason: String.raw`hex escape \x00 (null)` },
+    { value: String.raw`test\x0auser`, reason: String.raw`hex escape \x0a (LF)` },
+    { value: String.raw`test\x1buser`, reason: String.raw`hex escape \x1b (ESC)` },
+  ];
+
+  for (const { value, reason } of identityOnlyRejects) {
+    const uiRejects = hasDangerousChars(value);
+    const errors: string[] = [];
+    validateFieldForDangerousPatterns(value, 'test', errors);
+    const identityRejects = errors.length > 0;
+
+    // hasDangerousChars does NOT reject (all bytes are printable)
+    assert.strictEqual(uiRejects, false, `hasDangerousChars allows: ${reason}`);
+    // validateFieldForDangerousPatterns DOES reject (text-level pattern detection)
+    assert.strictEqual(identityRejects, true, `validateFieldForDangerousPatterns catches: ${reason}`);
+  }
+
+  // Category 2b: validateFieldForDangerousPatterns boundary values
+  {
+    const errors: string[] = [];
+    validateFieldForDangerousPatterns(undefined, 'test', errors);
+    assert.strictEqual(errors.length, 0, 'undefined should produce no errors');
+  }
+  {
+    const errors: string[] = [];
+    validateFieldForDangerousPatterns('', 'test', errors);
+    assert.strictEqual(errors.length, 0, 'empty string should produce no errors');
+  }
+
+  // Category 3: Inputs accepted by BOTH layers
+  const bothAccept = [
+    { value: 'John Doe', reason: 'simple ASCII name' },
+    { value: '田中太郎', reason: 'Japanese name' },
+    { value: 'Null;Variant', reason: 'semicolon in name' },
+    { value: "O'Brien", reason: 'single quote in name' },
+    { value: 'José García', reason: 'accented characters' },
+    { value: 'Müller', reason: 'umlaut' },
+  ];
+
+  for (const { value, reason } of bothAccept) {
+    const uiRejects = hasDangerousChars(value);
+    const errors: string[] = [];
+    validateFieldForDangerousPatterns(value, 'test', errors);
+    const identityRejects = errors.length > 0;
+
+    assert.strictEqual(uiRejects, false, `hasDangerousChars should accept: ${reason}`);
+    assert.strictEqual(identityRejects, false, `validateFieldForDangerousPatterns should accept: ${reason}`);
+  }
+
+  // Category 4: validateIdentity integration — control characters in name are now caught
+  {
+    const controlCharName: Identity = {
+      id: 'test',
+      name: 'Test\u0007User',
+      email: 'test@example.com',
+    };
+    const result = validateIdentity(controlCharName);
+    assert.strictEqual(result.valid, false, 'Control character in name should fail');
+    assert.ok(
+      result.errors.some(e => e.includes('name') && e.includes('control characters')),
+      'Error should mention control characters'
+    );
+  }
+
+  console.log('✅ All validation consistency tests passed!');
+}
+
 /**
  * Run all tests
  */
@@ -386,6 +502,7 @@ export function runSecurityTests(): void {
     testValidateIdentity();
     testValidateIdentities();
     testIsPathSafe();
+    testValidationConsistency();
 
     console.log('\n✅ All security tests passed!\n');
   } catch (error) {

extensions/git-id-switcher/src/ui/identityManager.ts

@@ -27,17 +27,17 @@ import {
   MAX_IDENTITIES,
   MAX_ID_LENGTH,
   MAX_NAME_LENGTH,
-  MAX_EMAIL_LENGTH,
-  MAX_SSH_HOST_LENGTH,
 } from '../core/constants';
 import {
   isValidIdentityId,
-  isValidEmail,
-  hasDangerousChars,
-  GPG_KEY_REGEX,
-  SSH_HOST_REGEX,
 } from '../validators/common';
-import { validateSshKeyPathFormat } from '../identity/inputValidator';
+import {
+  validateSshKeyPathFormat,
+  validateFieldForDangerousPatterns,
+  validateEmailField,
+  validateGpgKeyId,
+  validateSshHost,
+} from '../identity/inputValidator';
 import { isUnderSshDirectory, replaceHomeWithTilde } from '../security/pathUtils';
 import { getUserSafeMessage } from '../core/errors';
 import { securityLogger } from '../security/securityLogger';
@@ -139,8 +139,10 @@ function validateNameInput(vs: VSCodeAPI, value: string): string | null {
   if (value.length > MAX_NAME_LENGTH) {
     return vs.l10n.t('Name is too long (max {0} characters)', MAX_NAME_LENGTH);
   }
-  // SECURITY: Check for dangerous shell metacharacters
-  if (hasDangerousChars(value)) {
+  // SECURITY: Delegate to identity layer (SSoT for dangerous pattern detection)
+  const errors: string[] = [];
+  validateFieldForDangerousPatterns(value, 'name', errors);
+  if (errors.length > 0) {
     return vs.l10n.t('Name contains invalid characters');
   }
   return null;
@@ -160,10 +162,10 @@ function validateEmailInput(vs: VSCodeAPI, value: string): string | null {
   if (!value) {
     return vs.l10n.t('Email cannot be empty');
   }
-  if (value.length > MAX_EMAIL_LENGTH) {
-    return vs.l10n.t('Email is too long (max {0} characters)', MAX_EMAIL_LENGTH);
-  }
-  if (!isValidEmail(value)) {
+  // Delegate format and length validation to identity layer (SSoT)
+  const errors: string[] = [];
+  validateEmailField(value, errors);
+  if (errors.length > 0) {
     return vs.l10n.t('Invalid email format');
   }
   return null;
@@ -206,7 +208,10 @@ function validateSshKeyPathInput(vs: VSCodeAPI, value: string): string | null {
  */
 function validateGpgKeyIdInput(vs: VSCodeAPI, value: string): string | null {
   if (!value) return null; // Optional field
-  if (!GPG_KEY_REGEX.test(value)) {
+  // Delegate to identity layer (SSoT)
+  const errors: string[] = [];
+  validateGpgKeyId(value, errors);
+  if (errors.length > 0) {
     return vs.l10n.t('GPG key ID must be 8-40 hexadecimal characters');
   }
   return null;
@@ -224,12 +229,12 @@ function validateGpgKeyIdInput(vs: VSCodeAPI, value: string): string | null {
  */
 function validateSshHostInput(vs: VSCodeAPI, value: string): string | null {
   if (!value) return null; // Optional field
-  if (!SSH_HOST_REGEX.test(value)) {
+  // Delegate to identity layer (SSoT for format and length validation)
+  const errors: string[] = [];
+  validateSshHost(value, errors);
+  if (errors.length > 0) {
     return vs.l10n.t('SSH host must contain only valid hostname characters');
   }
-  if (value.length > MAX_SSH_HOST_LENGTH) {
-    return vs.l10n.t('SSH host is too long (max {0} characters)', MAX_SSH_HOST_LENGTH);
-  }
   return null;
 }
 
@@ -281,13 +286,16 @@ function validateFieldInput(
       return validateSshHostInput(vs, value);
     }
     default: {
-      // Use FIELD_METADATA validator for other fields (service, icon, description)
+      // Delegate dangerous pattern check to identity layer (SSoT)
+      const errors: string[] = [];
+      validateFieldForDangerousPatterns(value, field, errors);
+      if (errors.length > 0) {
+        return vs.l10n.t('{0} contains invalid characters', field);
+      }
+      // Length validation from FIELD_METADATA
       const meta = getFieldMetadata(field);
-      if (meta?.validator) {
-        const error = meta.validator(value);
-        if (error) {
-          return vs.l10n.t(error);
-        }
+      if (meta?.maxLength && value.length > meta.maxLength) {
+        return vs.l10n.t('{0} is too long (max {1} characters)', field, meta.maxLength);
       }
       return null;
     }