Architecture Guide
April 10, 2026 · View on GitHub
"Safety First, Quality Second, Production Third"
This codebase embodies this manufacturing principle. Security is never sacrificed for elegance or speed.
Audience: Developers modifying this codebase
This document explains technical design patterns and intentional complexity. For philosophy, see DESIGN_PHILOSOPHY.md. For contribution process, see CONTRIBUTING.md.
Table of Contents
- Document Map
- Intentional Patterns
- Code Markers
- Module Responsibilities
- Patterns That Look Wrong But Aren't
- Testing Philosophy
- Refactoring Guidelines
- Architecture Decisions
- Function Naming Conventions
- Security Constants
- Command Execution Flow
- File Size Rationale
- Directory Structure
- Webview Content Security Policy
- Extension Capabilities
Quick Reference
Should I Change This Code?
| If you see... | Then... |
|---|---|
/* c8 ignore */ comment | Don't try to add tests for it |
// defense-in-depth comment | Don't remove "redundant" checks |
// SECURITY: comment | Read carefully before modifying |
Silent catch block in SSH code | It's intentional, leave it |
| Multiple validators for same thing | Defense-in-depth, keep all layers |
Key Files (Expected to Be Large)
security/secureExec.tsssh/sshAgent.tssecurity/pathValidator.ts
Document Map
| Document | Purpose | Audience |
|---|---|---|
| DESIGN_PHILOSOPHY.md | Why we build this way | Anyone curious |
| ARCHITECTURE.md | How the code is structured | Developers |
| CONTRIBUTING.md | What to do when contributing | Contributors |
Intentional Patterns
This codebase contains patterns that may appear redundant or over-engineered. They are intentional. This section catalogs them so you don't "fix" what isn't broken.
Multi-Layer Validation (Defense-in-Depth)
The same check may appear at multiple layers. This is by design.
Null Byte Validation
| Layer | File | Purpose |
|---|---|---|
| 1. Common | validators/common.ts | hasNullByte() utility |
| 2. Path Security | security/pathValidator.ts | validateNoNullBytes() in pipeline |
| 3. Path Normalization | security/pathNormalizer.ts | Defense-in-depth before normalization |
Why multiple layers?
- Layer 1 provides reusable detection
- Layer 2 catches user input errors early in path validation pipeline
- Layer 3 is defense-in-depth (should never trigger if layer 2 works)
Code marker: /* c8 ignore start - Defense-in-depth */
Control Character Validation
Two validation phases exist in the path validation pipeline:
| Phase | Validators | Purpose |
|---|---|---|
| Pre-normalization | validateNoControlChars, validateNoInvisibleUnicode | Catch attacks before normalization |
| Post-normalization | validateNoControlCharsAfterNormalization, validateNoInvisibleUnicodeAfterNormalization | Catch edge cases where normalization introduces issues |
Do not remove post-normalization validators. Unicode NFC normalization can theoretically introduce new characters.
PATH_MAX Validation
Checked at multiple points because:
- Path expansion (
~→/home/username) changes length - Path concatenation may exceed limits
- Different operations have different length tolerances
Identity Duplicate Detection (Multi-Layer)
| Layer | Location | Trigger |
|---|---|---|
| 1. Schema | identity/configSchema.ts | Configuration validation |
| 2. Runtime | identity/identity.ts | Before adding to valid identities |
Layer 2 has explicit comment:
// Check for duplicate IDs (defense-in-depth: schema validation also checks this)
Do not remove "redundant" checks. They protect different trust boundaries.
Silent Error Handling in SSH Agent
In ssh/sshAgent.ts, some errors are intentionally swallowed:
// removeSshKey
try {
await sshAgentExec(['-d', expandedPath]);
} catch (error) {
// Ignore errors (key might not be loaded)
}
// removeAllIdentityKeys
.map(identity =>
removeSshKey(identity.sshKeyPath!).catch(() => {
// Ignore individual errors
})
)
This is correct. SSH key removal is best-effort cleanup. The key may:
- Already be unloaded
- Have never been loaded
- Belong to a different agent
Surfacing these errors would confuse users with irrelevant messages.
Binary Path Cache with TTL
security/binaryResolver.ts caches resolved binary paths with a 30-minute TTL:
interface CacheEntry {
path: string;
resolvedAt: number;
}
Why TTL? VS Code sessions can last days. Without TTL, a binary replaced after initial resolution would continue to be trusted. The 30-minute window balances security (periodic re-verification) with performance (no per-command filesystem checks).
clearPathCache() ignores TTL and clears immediately (used during identity switching).
Security Event Rate Limiter
security/securityLogger.ts includes a per-event-type rate limiter:
- Window: 10 seconds, Max events: 10 per event type
- Excess events are dropped with a count; next allowed event includes
dropped: N events
Why? Prevents log flooding from rapid validation failures (e.g., malformed config triggering repeated errors). Without rate limiting, an attacker could cause I/O exhaustion via log writes.
O_NOFOLLOW Symlink Protection
logging/fileLogWriter.ts uses O_NOFOLLOW when opening log files:
fs.constants.O_WRONLY |
fs.constants.O_CREAT |
fs.constants.O_APPEND |
fs.constants.O_NOFOLLOW;
Combined with post-open fstat() symlink verification. This mitigates TOCTOU between isSecureLogPath() and the actual file open.
Platform note: O_NOFOLLOW is Unix-only. On Windows, symlink creation requires admin privileges, making the risk inherently lower.
git.path Binary Verification
When git.path VS Code setting provides a binary path, binaryResolver.ts verifies it by running execFile(absolutePath, ['--version']) and checking the output starts with git version. This prevents a user-configured path from pointing to a non-git binary.
Note: Uses execFile() directly (not secureExec()) to avoid circular dependency.
ESLint no-restricted-imports for child_process
eslint.config.mjs prohibits importing exec and execSync from child_process/node:child_process. Only execFile and execFileSync are permitted.
This is a lint-time enforcement of the architectural decision to never use shell-based execution. The grep-based CI check remains as defense-in-depth.
Code Markers
Coverage Exclusion Markers
| Marker | Meaning |
|---|---|
/* c8 ignore start - Defense-in-depth */ | Intentionally unreachable in normal operation |
/* c8 ignore start - VS Code API */ | Cannot be unit tested without VS Code |
/* c8 ignore start - Error path */ | Defensive error handling |
/* c8 ignore start - Platform-specific */ | Platform-specific branches |
Many coverage exclusion markers exist across the codebase. When you see these markers: The code is intentionally excluded from coverage requirements. Don't remove them or try to add tests that exercise them.
Comment Patterns
| Pattern | Example Files | Purpose |
|---|---|---|
// SECURITY: | ssh/sshAgent.ts, security/secureExec.ts | Security-critical code explanation |
// Note: Defense-in-depth. | security/pathValidator.ts | Explains why seemingly unreachable code exists |
@see https://owasp.org/... | Multiple | Links to security references |
ESLint Exclusion Patterns
These exclusions are intentional and should not be removed:
| Rule Disabled | Location | Reason |
|---|---|---|
no-control-regex | validators/common.ts | Control char regex is intentional for security validation |
@typescript-eslint/no-require-imports | core/vscodeLoader.ts | VS Code API requires dynamic import |
@typescript-eslint/no-unsafe-member-access | security/securityLogger.ts | Dynamic property access for log sanitization |
Do not strip comments for "cleaner code."
ESLint Security Enforcement
These rules actively prevent dangerous patterns:
| Rule | Target | Reason |
|---|---|---|
no-restricted-imports | exec, execSync from child_process | Shell-based execution enables command injection; use execFile() |
no-eval | eval(), new Function() | Dynamic code execution enables injection |
no-implied-eval | setTimeout(string), setInterval(string) | Implicit eval via string arguments |
Module Responsibilities
Security Layer (src/security/)
| File | Single Responsibility |
|---|---|
pathValidator.ts | Path validation pipeline orchestrator |
secureExec.ts | Safe command execution with timeout |
securityLogger.ts | Structured security event logging |
commandAllowlist.ts | Allowed commands for secureExec |
binaryResolver.ts | Absolute binary path resolution |
pathNormalizer.ts | Path normalization with security |
pathSymlinkResolver.ts | Symlink detection (TOCTOU mitigation) |
pathUnicodeDetector.ts | Invisible Unicode attack detection |
pathTraversalDetector.ts | Path traversal attack detection |
pathSanitizer.ts | Path sanitization utilities |
pathUtils.ts | SSH key path utilities |
flagValidator.ts | Command-line flag validation |
sensitiveDataDetector.ts | Secret detection in logs |
pathValidator.ts orchestrates multiple individual validators:
| Category | Examples |
|---|---|
| Basic checks | Empty, whitespace, null bytes, length |
| Prefix validation | Tilde pattern, allowed prefixes |
| Windows-specific | Drive letters, UNC paths, device paths, reserved names |
| Unicode attacks | Control chars, invisible Unicode (pre/post normalization) |
| Traversal attacks | .., //, \, trailing dot, trailing /. |
Do not consolidate into a single monolithic validator. Separation enables:
- Independent testing per validator
- Audit trail showing exactly which check failed
- Adding/removing checks doesn't affect others
SSH Key Validation (src/ssh/sshAgent.ts)
Several validation functions exist:
| Function | Purpose |
|---|---|
validateKeyPath() | Path format and security validation |
validateKeyFileType() | Must be regular file (not directory/symlink/device) |
validateKeyFileSize() | Size limits for DoS protection |
validateKeyFilePermissions() | Unix only - no group/others access |
validateKeyFileForSshAdd() | Orchestrates all validations + format check |
Do not merge these functions. Separation enables:
- Granular error messages
- Independent testing
- Clear failure attribution
Patterns That Look Wrong But Aren't
"Excessive" Type Checking
if (typeof value !== "string") {
throw new Error("Expected string");
}
These checks may seem unnecessary when TypeScript guarantees types. They exist because:
- Data crosses trust boundaries (user config, external APIs)
- Runtime behavior may differ from compile-time types
- Defense against
anytype pollution
"Redundant" Logging
Security events are logged at multiple points:
- Entry to security-sensitive functions
- Before external command execution
- After validation failures
This creates an audit trail. Do not optimize away "redundant" logs.
Constants That Could Be Configurable
Some values are hardcoded despite being potential user preferences:
// constants.ts
export const MAX_IDENTITIES = ...; // Hardcoded limit
export const PATH_MAX = ...; // POSIX limit
These are security limits, not user preferences. Making them configurable would allow:
- DoS via excessive identities
- Resource exhaustion attacks
- Buffer overflow attempts
Unreachable Validators
Several validators are marked as defense-in-depth and will never execute in normal operation:
// security/pathValidator.ts
/* c8 ignore start - Defense-in-depth: unreachable due to prior validators */
const validateNoUNCPath: Validator = (state) => { ... }
These exist because:
- Pipeline order might change in the future
- Direct calls to individual validators bypass the pipeline
- Security code should be paranoid
Testing Philosophy
What We Test
| Category | Coverage Target | Notes |
|---|---|---|
| Security validators | 100% | Non-negotiable |
| Business logic | 90%+ | Core functionality |
| UI rendering | Best-effort | VS Code API limitations |
What We Don't Test (And Why)
- Defense-in-depth fallbacks: By design, they should never execute
- VS Code API wrappers: Require integration testing
- Platform-specific branches: CI may not cover all platforms
- Silent error paths: Intentionally opaque
Known Gaps
ssh/sshAgent.ts has limited test coverage due to:
- External dependency (ssh-agent process)
- Platform-specific behavior (macOS Keychain integration)
- Transient state management
This is a known gap, not an oversight.
Refactoring Guidelines
Before You Refactor
- Check for markers: Look for
/* c8 ignore */and// defense-in-depthcomments - Understand the layer: Is this code at a trust boundary?
- Read this document: Is the pattern listed as intentional?
Safe Refactoring Targets
These are legitimate improvement opportunities:
| Area | Issue | Safe to Change |
|---|---|---|
| Error message formatting | Inconsistent patterns | Yes - standardize format |
| Path utility functions | Some duplication exists | Yes - extract to security/pathUtils.ts |
Do Not Touch
| Pattern | Reason |
|---|---|
| Multi-layer null byte checks | Defense-in-depth |
| Pre/post normalization validators | Different security contexts |
| Multi-layer duplicate detection | Trust boundary protection |
| Silent catches in ssh/sshAgent.ts | Best-effort cleanup |
| Hardcoded security limits | DoS protection |
Architecture Decisions
Why No Dependency Injection Framework?
The codebase uses manual dependency injection (constructor parameters) rather than a DI framework because:
- VS Code extensions have size constraints
- Framework overhead exceeds benefit for this scale
- Explicit wiring is more debuggable
Why Synchronous Path Validation?
Path validation in security/pathValidator.ts is synchronous despite Node.js favoring async:
- Validation is CPU-bound, not I/O-bound
- Synchronous code is easier to reason about for security
- Async validation introduces timing windows
Note: isSecureLogPath() is async because it performs file system operations (symlink detection).
Why Not Use eval() or Dynamic Code?
The codebase explicitly avoids:
eval()new Function()- Dynamic
require()
This is defense against code injection, even at the cost of flexibility.
Why execFile() Instead of exec()?
All command execution goes through security/secureExec.ts which uses execFile():
exec()passes commands through a shell, enabling injectionexecFile()executes binaries directly with argument arrays- Combined with
security/commandAllowlist.tsfor defense-in-depth
Function Naming Conventions
Consistent naming helps developers understand function behavior at a glance. This codebase follows strict conventions defined in validators/common.ts.
Naming Rules
| Prefix | Returns | Behavior | Example |
|---|---|---|---|
is*() | boolean | Pure check, no side effects | isValidEmail() |
has*() | boolean | Existence/presence check | hasNullByte() |
validate*() | ValidationResult | Returns result object | validatePathSecurity() |
*OrThrow() | T or throws | Exception on failure | parseOrThrow() |
assert*() | void or throws | Guard, throws on failure | assertWithinWorkspaceBoundary() |
detect*() | T | null | Returns detected issue or null | detectUnsafeCharsInFlag() |
check*() | varies | Queries external state | checkKeyLoadedInAgent() |
Prohibited Patterns
These patterns are not allowed in this codebase:
| Pattern | Problem | Use Instead |
|---|---|---|
check*() for pure predicates | Ambiguous - implies external state | is*() or has*() |
is*() returning non-boolean | Violates boolean contract | validate*() or get*() |
validate*() returning void | No way to get error details | assert*() or return result |
is*Valid*() redundant prefix | Redundant naming | isValid*() or validate*() |
Terminology
| Term | Meaning | Example |
|---|---|---|
valid | Format/structure is correct | isValidEmail() |
secure | Resistant to security attacks | validatePathSecurity() |
safe | Safe for specific context (e.g., shell) | isShellSafePath() |
For detailed documentation and examples, see the module comment in validators/common.ts.
Security Constants
Security limits and validation patterns are centralized to prevent inconsistency:
- Field length limits: Centralized in
core/constants.ts - Validation patterns: Centralized in
validators/common.ts
For specific values, refer to the source code. This document intentionally omits concrete numbers to prevent documentation drift.
Why No Concrete Values Here?
Documenting specific limits (e.g., "max 64 characters") creates maintenance burden:
- Values change but docs don't get updated
- Multiple sources of truth lead to confusion
- Developers may reference outdated docs instead of code
The source files (constants.ts, common.ts) are the single source of truth.
Command Execution Flow
User Action
│
▼
┌───────────────────────────────┐
│ commands/handlers.ts │ VS Code command triggered
└───────────────────────────────┘
│
▼
┌───────────────────────────────┐
│ services/switcher.ts │ Business logic
└───────────────────────────────┘
│
▼
┌───────────────────────────────┐
│ security/secureExec.ts │ Command execution gateway
│ ┌───────────────────────────┤
│ │ 1. Command allowlist check (security/commandAllowlist.ts)
│ │ 2. Binary path resolution (security/binaryResolver.ts)
│ │ 3. Argument validation
│ │ 4. Timeout configuration
│ │ 5. execFile() execution
│ └───────────────────────────┤
└───────────────────────────────┘
│
▼
┌───────────────────────────────┐
│ External Binary │ git, ssh-add, ssh-keygen
└───────────────────────────────┘
Security checkpoints:
- Allowlist: Only
git,ssh-add,ssh-keygenare permitted - Path resolution: Absolute paths prevent PATH pollution
- No shell:
execFile()bypasses shell interpretation - Timeout: Prevents hanging processes (DoS protection)
File Size Rationale
Some files exceed typical "small file" guidelines:
| File | Rationale |
|---|---|
security/secureExec.ts | Single responsibility with extensive timeout handling |
ssh/sshAgent.ts | Complex external process interaction + validation |
security/pathValidator.ts | Multiple validators with documentation |
These files are large because splitting them would:
- Scatter related security logic
- Make audit more difficult
- Introduce unnecessary abstraction layers
Large file size is acceptable when it preserves cohesion.
Directory Structure
Note: Files are organized by responsibility into dedicated directories.
src/
├── core/ # Foundation
│ ├── extension.ts # Extension entry point
│ ├── constants.ts # Shared constants (security limits)
│ ├── errors.ts # Custom error types
│ ├── gitConfig.ts # Git config read/write
│ ├── vscodeLoader.ts # VS Code API loader
│ ├── workspaceTrust.ts # Workspace trust integration
│ ├── configChangeDetector.ts # Config change watcher
│ └── submodule.ts # Git submodule support
├── security/ # Security
│ ├── pathValidator.ts # Path validation orchestrator
│ ├── pathNormalizer.ts # Path normalization with security
│ ├── pathSymlinkResolver.ts # Symlink detection (TOCTOU mitigation)
│ ├── pathTraversalDetector.ts # Path traversal attack detection
│ ├── pathUnicodeDetector.ts # Invisible Unicode attack detection
│ ├── pathSanitizer.ts # Path sanitization utilities
│ ├── pathUtils.ts # SSH key path utilities
│ ├── secureExec.ts # Safe command execution with timeout
│ ├── commandAllowlist.ts # Allowed commands for secureExec
│ ├── binaryResolver.ts # Absolute binary path resolution
│ ├── flagValidator.ts # Command-line flag validation
│ ├── securityLogger.ts # Structured security audit logging
│ └── sensitiveDataDetector.ts # Secret detection in logs
├── identity/ # Identity management
│ ├── identity.ts # Identity loading & validation
│ ├── configSchema.ts # JSON schema validation
│ └── inputValidator.ts # Identity input validation
├── ui/ # User interface
│ ├── webview.ts # Webview panel integration
│ ├── identityPicker.ts # Quick pick identity selector
│ ├── identityStatusBar.ts # Status bar display
│ ├── displayLimits.ts # Text truncation for UI
│ ├── documentationPublic.ts # Documentation command handler (public API)
│ └── documentationInternal.ts # Documentation utilities (internal)
├── logging/ # Logging
│ ├── logTypes.ts # Log type definitions
│ └── fileLogWriter.ts # File-based log writer
├── ssh/ # SSH key management
│ └── sshAgent.ts # SSH key add/remove/list
├── services/ # Business logic
│ ├── detection.ts # Identity auto-detection from git
│ └── switcher.ts # Identity switching logic
├── commands/ # VS Code commands
│ └── handlers.ts # VS Code command handlers
├── validators/ # Shared validation
│ └── common.ts # Shared validation utilities
└── test/ # Unit and E2E tests
└── e2e/ # End-to-end tests
Webview Content Security Policy
The authoritative CSP implementation lives in src/ui/htmlTemplates/csp.ts (buildCspString). The current policy:
default-src 'none';
base-uri 'none';
form-action 'none';
frame-ancestors 'none';
img-src ${cspSource} https://assets.nullvariant.com https://img.shields.io https://avatars.githubusercontent.com;
style-src 'nonce-${nonce}';
script-src 'nonce-${nonce}';
connect-src https://assets.nullvariant.com;
font-src ${cspSource};
Policy Rationale
| Directive | Value | Why |
|---|---|---|
default-src | 'none' | Deny everything by default |
base-uri | 'none' | Not covered by default-src; prevents <base> injection (CSP3 §6.1) |
form-action | 'none' | Not covered by default-src; prevents form submission to attacker origins |
frame-ancestors | 'none' | Not covered by default-src; prevents clickjacking via iframe embedding |
img-src | cspSource, CDN, shields.io, avatars | VS Code resources, CDN assets, README badges, GitHub contributor avatars |
style-src | 'nonce-...' | Nonce-only; cspSource removed to close stylesheet bypass |
script-src | 'nonce-...' | Only allow scripts with a per-render cryptographic nonce |
connect-src | CDN only | Fetch README/documentation from CDN; no other network access |
font-src | cspSource | Allow VS Code bundled fonts only |
CSP Prohibited Patterns
'unsafe-inline'— Enables XSS via injected<script>tags'unsafe-eval'— Enables code injection viaeval()*orhttps:in any directive — Allows loading from arbitrary origins- Wildcard subdomains (e.g.
*.githubusercontent.com) — Only specific subdomains are allowed
Extension Capabilities
This extension follows the principle of least privilege. The following table documents what the extension can and cannot do:
Granted Capabilities
| Capability | Mechanism | Purpose |
|---|---|---|
| Read/write Git config | git config via secureExec | Set user.name, user.email, user.signingkey, commit.gpgsign |
| SSH agent management | ssh-add via secureExec | Load/unload SSH keys when switching identity |
| SSH key inspection | ssh-keygen -lf via secureExec | Validate key format and fingerprint |
| File system read | Node.js fs (validated paths) | Read SSH key files, write log files |
| VS Code settings | Configuration API | Store identity definitions |
| Status bar | StatusBarItem API | Display current identity |
Explicitly Denied Capabilities
| Capability | Why |
|---|---|
| Network access | No telemetry, no external API calls, zero production dependencies |
| Webview content | No HTML rendering (CSP policy defined above for future use) |
| Terminal access | No terminal creation or terminal command execution |
| Task execution | No task providers registered |
| Debug adapter | No debugging capabilities |
| File system write (arbitrary) | Log paths validated via isSecureLogPath() with symlink detection |
| Shell execution | execFile() only; exec() is prohibited |
Questions?
If you're unsure whether something is intentional:
- Check this document
- Look for code comments
- Open an issue to discuss before changing
The maintainers would rather explain a pattern than debug a regression.