feat(data-dir): per-instance data directory (--data-dir / SPE_DATA_DIR) + secure-fs hardening - #39
Merged
Gregory Joseph (gnjoseph) merged 3 commits intoJul 9, 2026
Conversation
…R) + secure-fs hardening Adds a configurable data directory so multiple SPE MCP server instances stop clobbering the single ~/.spe-mcp/state.json + token cache. - New src/paths.ts: lazy/memoized resolve-once seam (resolveDataDir, getDataDir, getStateFile, getCacheDir, getCacheFile, getLegacyCacheFile, setDataDirOverride). Precedence flag > env > default ~/.spe-mcp; default is byte-identical. - Remove module-level STATE_DIR (state.ts) / CACHE_DIR (auth.ts) consts and route state + token cache through the seam. state.json is NOT auto-partitioned (the directory is the isolation boundary). - CLI: --data-dir on start/auth/logout; resolve + set the override, propagate SPE_DATA_DIR, and log the resolved path to stderr (path-only) BEFORE importing state/auth so all entry points resolve the same dir. - Harden secure-fs (fail-closed): refuse symlink / foreign-owned / group-or-other dirs; O_NOFOLLOW + fstat on read/write; chmod the fd, never the path; Windows off-profile owner-only DACL via icacls or refuse; insecure target blocks refresh-token persistence (forces fresh interactive sign-in). - Untrusted-path validation in resolveDataDir: absolute + normalized, reject CWD-relative, expand leading ~. - Tests: paths.test.ts, state.test.ts (isolation + golden default), secure-fs symlink/TOCTOU/perms. Suite: 688 passed / 7 skipped. - Version bump 0.1.0-alpha.1 -> 0.2.0-alpha.1 (additive / back-compat). AB#3141787 AB#3141788 AB#3141789 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- readState: route through readSecureFile (O_NOFOLLOW + owner check) so a symlinked/foreign-owned state.json fails closed, matching writeState. (sec S1) - secure-fs: invoke icacls by absolute %SystemRoot%\System32 path, not bare name, to remove any binary-planting angle. (sec S2) - writeSecureFile: drop O_TRUNC; ftruncate only AFTER the fstat owner/type checks pass (no truncate-before-verify), and use writeFileSync(fd,...) to handle short writes / EINTR. (sec S3 + review C3) - ensureSecureDir: scope the strict group/other-accessible rejection to off-home overrides; keep best-effort for the default ~/.spe-mcp so a mode-ignoring FS (WSL DrvFs, some NFS/CIFS) doesn't hard-fail existing users. Ownership + symlink checks remain universal. (review C1) - secureWindowsDirAclOrThrow: document the known off-profile explicit-ACE / owner-verify limitation (follow-up under AB#3116729). (review C2) - README: Configuration row + Token Storage + "Running multiple instances" with .vscode/mcp.json snippets and the absolute-path rule. (review C4) - tests: add sanitizeForFilename coverage. Suite 689 passed / 7 skipped; npm run ci + lint green. AB#3141787 AB#3141788 AB#3141789 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ardening Compliance C1 (Keep-a-Changelog repo convention). Non-code; no ci impact. AB#3141787 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Gregory Joseph (gnjoseph)
marked this pull request as ready for review
July 9, 2026 16:30
Gregory Joseph (gnjoseph)
merged commit Jul 9, 2026
aec8ac4
into
feat/spe-mcp-server
5 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a per-instance data directory so multiple SPE MCP server instances (published VS Code build + local build; two tenants/projects; VS Code + the eval loop) stop clobbering the single, unpartitioned ~/.spe-mcp/state.json and token cache.
--data-dir <path>flag +SPE_DATA_DIRenv on start / auth / logout. Default~/.spe-mcpunchanged. Precedence flag > env > default.src/paths.ts— a lazy/memoized resolve-once seam (resolveDataDir,getDataDir,getStateFile,getCacheDir,getCacheFile,getLegacyCacheFile,setDataDirOverride). Removes the module-levelSTATE_DIR(state.ts) andCACHE_DIR(auth.ts) consts that were evaluated at import time.state.jsonis not auto-partitioned — the directory is the isolation boundary (bootstrap has no tenant/client key; state.json is a resumable provisioning workspace; the r-appgateconfirmedSessionIdstamp is per-process).0.1.0-alpha.1->0.2.0-alpha.1(additive / back-compat; default byte-identical; no migration; zero new dependencies).Security hardening (shipped in this PR —
--data-diractivates latent secure-fs weaknesses)Making the data dir caller-supplied moves it across a trust boundary (
.vscode/mcp.jsonis workspace-scoped, potentially-untrusted).secure-fswas fail-open + symlink-following; now:0o700); Windows off-profile override gets an owner-only DACL viaicaclsor is refused.O_NOFOLLOW+ verify the fd viafstat;chmodthe fd, never the path.resolveDataDirrequires absolute + normalized, rejects CWD-relative (never resolves againstprocess.cwd()), expands a leading~.Compliance
Tests / evidence
npm run ci(typecheck + build + vitest) green;npm run lintclean.Test Files 50 passed (50)—Tests 688 passed | 7 skipped (695)(the 7 skipped are POSIX-only symlink/perm cases skipped on the Windows dev box; they run on CI Linux).src/paths.test.ts(precedence /~expansion / reject-relative / lazy re-resolution / cache partitioning),src/state.test.ts(isolation: writing dir B leaves dir A byte-identical; golden default; per-instanceclearState), extendedsrc/secure-fs.test.ts(O_NOFOLLOWsymlink refusal on read+write, symlinked-dir refusal,0o700repair).dist):SPE_DATA_DIRoverride ->state.jsonlands under the target and not under~/.spe-mcp; reads back; relative path rejected (INVALID_DATA_DIR).Work items
AB#3141787 (data-dir + seam) · AB#3141788 (secure-fs hardening) · AB#3141789 (docs — follow-up).
Follow-ups (not blocking): AB#3141790 (SDL threat-model), AB#3141791 (OS-keychain token storage), AB#3141792 (foreign-writer warning), AB#3141793 (eval-loop dogfood to
SPE_DATA_DIR).Reviewer notes
index.js/auth.js— all four entry points (start/auth/logout/cleanup) resolve the same dir per process. cleanup is an MCP tool inside astartsession, so it inherits the process env.