Skip to content

Commit c03d3a2

Browse files
nullvariantclaude
andauthored
docs(git-id-switcher): add ESLint exec ban, JSDoc annotations, and update security docs (INFO-16/17) (#354)
## Background Final PR of the security audit remediation series. All 17 findings from the STRIDE-based audit are now addressed. ## Changes - ESLint: Add `no-restricted-imports` rule banning `exec`/`execSync` from `child_process` — only `execFile`/`execFileSync` permitted (INFO-16) - JSDoc: Add `@security` and `@deprecated` tags to `getGitAuthorFlag()` with recommended replacement pattern (INFO-17) - THREAT_MODEL.md: Update mitigations for S2, T1, T3, D2, D3, E1, E2 reflecting all PR 1-7 security improvements - ARCHITECTURE.md: Document 5 new intentional patterns (TTL cache, rate limiter, O_NOFOLLOW, git.path verification, ESLint security enforcement) 🖥️ IDE: [Cursor](https://cursor.sh) 🔌 Extension: [Claude Code](https://claude.ai/download) Model-Raw: claude-opus-4-6 Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent cf22109 commit c03d3a2

5 files changed

Lines changed: 138 additions & 45 deletions

File tree

extensions/git-id-switcher/docs/ARCHITECTURE.md

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,57 @@ try {
151151

152152
Surfacing these errors would confuse users with irrelevant messages.
153153

154+
### Binary Path Cache with TTL
155+
156+
`security/binaryResolver.ts` caches resolved binary paths with a **30-minute TTL**:
157+
158+
```typescript
159+
interface CacheEntry {
160+
path: string;
161+
resolvedAt: number;
162+
}
163+
```
164+
165+
**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).
166+
167+
`clearPathCache()` ignores TTL and clears immediately (used during identity switching).
168+
169+
### Security Event Rate Limiter
170+
171+
`security/securityLogger.ts` includes a per-event-type rate limiter:
172+
173+
- **Window**: 10 seconds, **Max events**: 10 per event type
174+
- Excess events are dropped with a count; next allowed event includes `dropped: N events`
175+
176+
**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.
177+
178+
### O_NOFOLLOW Symlink Protection
179+
180+
`logging/fileLogWriter.ts` uses `O_NOFOLLOW` when opening log files:
181+
182+
```typescript
183+
fs.constants.O_WRONLY |
184+
fs.constants.O_CREAT |
185+
fs.constants.O_APPEND |
186+
fs.constants.O_NOFOLLOW;
187+
```
188+
189+
Combined with post-open `fstat()` symlink verification. This mitigates TOCTOU between `isSecureLogPath()` and the actual file open.
190+
191+
**Platform note**: `O_NOFOLLOW` is Unix-only. On Windows, symlink creation requires admin privileges, making the risk inherently lower.
192+
193+
### git.path Binary Verification
194+
195+
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.
196+
197+
**Note**: Uses `execFile()` directly (not `secureExec()`) to avoid circular dependency.
198+
199+
### ESLint `no-restricted-imports` for child_process
200+
201+
`eslint.config.mjs` prohibits importing `exec` and `execSync` from `child_process`/`node:child_process`. Only `execFile` and `execFileSync` are permitted.
202+
203+
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.
204+
154205
---
155206

156207
## Code Markers
@@ -186,6 +237,16 @@ These exclusions are intentional and should not be removed:
186237

187238
**Do not strip comments** for "cleaner code."
188239

240+
### ESLint Security Enforcement
241+
242+
These rules actively prevent dangerous patterns:
243+
244+
| Rule | Target | Reason |
245+
| ----------------------- | ------------------------------------------- | ----------------------------------------------------------------- |
246+
| `no-restricted-imports` | `exec`, `execSync` from `child_process` | Shell-based execution enables command injection; use `execFile()` |
247+
| `no-eval` | `eval()`, `new Function()` | Dynamic code execution enables injection |
248+
| `no-implied-eval` | `setTimeout(string)`, `setInterval(string)` | Implicit eval via string arguments |
249+
189250
---
190251

191252
## Module Responsibilities

0 commit comments

Comments
 (0)