Skip to content

Commit f1949ca

Browse files
brookscclaude
authored andcommitted
feat(security): atomic writes, input validators, and static analysis tooling
atomic.ts / atomic.test.ts - Crash-safe atomic file writes via temp-file + rename - fsync temp file and directory entry for durability - Preserve existing file mode on overwrite; use fchmod after open to set exact mode bits, bypassing process umask - Tests force umask=0o022 in exact-mode cases for CI determinism validation.ts / validation.test.ts - validateBranchName: mirrors git check-ref-format --branch rules (rejects HEAD; accepts FETCH_HEAD/ORIG_HEAD per actual git behaviour) - validateUUID: enforces v4 UUID (version nibble 4, variant nibble 8/9/a/b) Static analysis - .semgrep/filesystem-safety.yml: flag raw writeFileSync in MCP/coordinator paths; pattern-either covers both qualified (fs.writeFileSync) and unqualified forms - .semgrep/electron-security.yml, .semgrep/ipc-auth.yml: IPC auth checks - .dependency-cruiser.cjs: architecture boundary enforcement - .gitleaks.toml: secret-scanning rules - knip.config.ts: dead-export detection; server.ts listed as entry point (lands in coordinator-2-mcp-backend) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 2d49608 commit f1949ca

11 files changed

Lines changed: 726 additions & 0 deletions

.dependency-cruiser.cjs

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
/** @type {import('dependency-cruiser').IConfiguration} */
2+
module.exports = {
3+
forbidden: [
4+
{
5+
name: 'no-renderer-importing-main',
6+
severity: 'error',
7+
comment:
8+
'Renderer (src/) must never import Electron main-process code. ' +
9+
'Use IPC channels instead.',
10+
from: { path: '^src/' },
11+
to: {
12+
path: '^electron/',
13+
// Allow importing the shared IPC channel enum (channels.ts is a pure enum, no Node/Electron deps)
14+
pathNot: '^electron/ipc/channels\\.ts',
15+
},
16+
},
17+
{
18+
name: 'no-mcp-importing-components',
19+
severity: 'error',
20+
comment: 'MCP coordinator must not import frontend components or store.',
21+
from: { path: '^electron/mcp/' },
22+
to: { path: '^src/(components|store|lib)/' },
23+
},
24+
{
25+
name: 'no-circular',
26+
severity: 'error',
27+
comment:
28+
'Circular dependencies break tree-shaking and make reasoning about startup order impossible.',
29+
from: {},
30+
to: { circular: true },
31+
},
32+
{
33+
name: 'no-orphans',
34+
severity: 'warn',
35+
comment: 'Orphan modules have no importers and no exports used elsewhere — likely dead code.',
36+
from: {
37+
orphan: true,
38+
// Test files, config files, entry points, and pure type modules are expected orphans.
39+
// Type-only modules (types.ts, *.d.ts) are consumed by TypeScript structurally — the
40+
// import graph doesn't capture all type-level usage, so they appear orphaned.
41+
pathNot: [
42+
'\\.test\\.(ts|tsx)$',
43+
'\\.config\\.(ts|js|cjs)$',
44+
'\\.d\\.ts$',
45+
'types\\.ts$',
46+
'types\\.(ts|tsx)$',
47+
'^src/main\\.tsx$',
48+
'^src/remote/main\\.tsx$',
49+
'^electron/main\\.ts$',
50+
'^electron/preload\\.cjs$',
51+
'^electron/mcp/server\\.ts$',
52+
// New subsystem files have no importers until coordinator PRs land
53+
'^electron/mcp/atomic\\.ts$',
54+
'^electron/mcp/validation\\.ts$',
55+
// Vite ambient env declarations
56+
'^src/vite-env\\.d\\.ts$',
57+
],
58+
},
59+
to: {},
60+
},
61+
],
62+
63+
options: {
64+
doNotFollow: {
65+
path: 'node_modules',
66+
},
67+
moduleSystems: ['es6', 'cjs'],
68+
tsConfig: {
69+
fileName: 'tsconfig.json',
70+
},
71+
reporterOptions: {
72+
dot: {
73+
collapsePattern: 'node_modules/[^/]+',
74+
},
75+
archi: {
76+
collapsePattern: '^(node_modules|src/components)/[^/]+',
77+
},
78+
},
79+
},
80+
};

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ dist-electron
44
dist-remote
55
release
66
coverage
7+
.parallel-code/
78
.worktrees
89
.idea
910
.claude

.gitleaks.toml

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
title = "Gitleaks config for Parallel Code"
2+
3+
[extend]
4+
# Use the default Gitleaks ruleset as the base
5+
useDefault = true
6+
7+
[[rules]]
8+
id = "parallel-code-mcp-token"
9+
description = "Parallel Code MCP bearer token"
10+
regex = '''PARALLEL_CODE_MCP_TOKEN\s*[=:]\s*['"]?[A-Za-z0-9+/_-]{20,}['"]?'''
11+
tags = ["token", "parallel-code"]
12+
13+
[[rules]]
14+
id = "bearer-token-in-url"
15+
description = "Bearer token embedded in a URL query parameter"
16+
regex = '''[?&]token=[A-Za-z0-9+/\-_]{20,}'''
17+
tags = ["token", "url"]
18+
[rules.allowlist]
19+
# Test fixture URLs and documentation are expected to have placeholder tokens
20+
regexes = [
21+
'''token=<[A-Za-z0-9_-]+>''', # placeholder like ?token=<your-token>
22+
'''token=test''', # obvious test value
23+
'''token=abc''', # obvious test value
24+
'''token=tok''', # obvious test value
25+
]
26+
27+
[[rules]]
28+
id = "anthropic-api-key"
29+
description = "Anthropic API key"
30+
regex = '''sk-ant-[A-Za-z0-9\-_]{40,}'''
31+
tags = ["api-key", "anthropic"]
32+
33+
[allowlist]
34+
description = "Global allowlist"
35+
paths = [
36+
# Lock files contain package hashes, not secrets
37+
'''package-lock\.json''',
38+
# Test fixture files with obviously fake values
39+
'''\.test\.(ts|tsx)$''',
40+
# The gitleaks config itself documents patterns
41+
'''\.gitleaks\.toml''',
42+
]
43+
commits = []

.semgrep/electron-security.yml

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
rules:
2+
- id: no-inner-html-without-sanitize
3+
pattern: $EL.innerHTML = $VAL
4+
pattern-not: $EL.innerHTML = DOMPurify.sanitize(...)
5+
message: |
6+
Direct innerHTML assignment without DOMPurify.sanitize() risks XSS.
7+
Use the sanitizeHtml() helper or DOMPurify.sanitize() explicitly.
8+
languages: [typescript]
9+
severity: ERROR
10+
paths:
11+
include:
12+
- '**/electron/**'
13+
14+
- id: no-eval
15+
pattern: eval($X)
16+
message: |
17+
eval() executes arbitrary code. Not allowed in Electron renderer or main process.
18+
languages: [typescript]
19+
severity: ERROR
20+
21+
- id: no-new-function
22+
pattern: new Function($X)
23+
message: |
24+
new Function() executes arbitrary code. Not allowed in Electron renderer or main process.
25+
languages: [typescript]
26+
severity: ERROR
27+
28+
- id: no-shell-true-in-spawn
29+
pattern-either:
30+
- pattern: |
31+
child_process.spawn($CMD, $ARGS, {shell: true, ...})
32+
- pattern: |
33+
spawn($CMD, $ARGS, {shell: true, ...})
34+
message: |
35+
spawn() with shell:true enables shell injection. Use shell:false and
36+
pass arguments as an array.
37+
languages: [typescript]
38+
severity: ERROR
39+
paths:
40+
include:
41+
- '**/electron/**'
42+
43+
- id: pkill-dash-f-broad-kill
44+
pattern: $EXEC("pkill", ["-f", ...], ...)
45+
message: |
46+
pkill -f matches any process whose full command line contains the pattern,
47+
which can accidentally kill unrelated processes. Use kill by PID or docker stop.
48+
languages: [typescript]
49+
severity: WARNING
50+
paths:
51+
include:
52+
- '**/electron/**'

.semgrep/filesystem-safety.yml

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
rules:
2+
- id: direct-writefile-in-mcp-coordinator
3+
pattern-either:
4+
- pattern: writeFileSync($PATH, $DATA, ...)
5+
- pattern: fs.writeFileSync($PATH, $DATA, ...)
6+
message: |
7+
Direct writeFileSync in coordinator code risks torn writes on crash.
8+
Use atomicWriteFileSync() from electron/mcp/atomic.ts instead.
9+
languages: [typescript]
10+
severity: WARNING
11+
paths:
12+
include:
13+
- '**/electron/mcp/**'
14+
- '**/electron/ipc/register.ts'
15+
16+
- id: copyfilesync-side-effect
17+
pattern: fs.copyFileSync($SRC, $DST)
18+
message: |
19+
fs.copyFileSync is a filesystem side effect. In StartMCPServer,
20+
ensure all pure computation (mcpConfig, mergedMcpJson) precedes
21+
any copyFileSync calls so validation failures don't leave residue.
22+
languages: [typescript]
23+
severity: INFO
24+
paths:
25+
include:
26+
- '**/electron/ipc/register.ts'

.semgrep/ipc-auth.yml

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
rules:
2+
- id: token-embedded-in-url-template
3+
pattern: |
4+
`$PREFIX?token=${$TOKEN}$SUFFIX`
5+
message: |
6+
Token embedded directly in URL template literal. Mobile/shared URLs must use
7+
the mobile token, not the coordinator token. The coordinator token must never
8+
appear in any URL that reaches the renderer or network.
9+
languages: [typescript]
10+
severity: ERROR
11+
paths:
12+
include:
13+
- '**/electron/**'
14+
exclude:
15+
- '**/electron/remote/server.ts'
16+
17+
- id: console-log-token-variable
18+
pattern-either:
19+
- pattern: console.log($A, token, $B)
20+
- pattern: console.warn($A, token, $B)
21+
- pattern: console.log(token)
22+
- pattern: console.warn(token)
23+
message: |
24+
Logging a variable named 'token' directly. Use redactServerUrl() or
25+
ensure this is not a bearer token value.
26+
languages: [typescript]
27+
severity: WARNING
28+
paths:
29+
include:
30+
- '**/electron/**'

electron/mcp/atomic.test.ts

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
import { describe, it, expect, afterEach } from 'vitest';
2+
import { mkdtemp, rm, readFile, stat } from 'fs/promises';
3+
import { join } from 'path';
4+
import { tmpdir } from 'os';
5+
import { atomicWriteFile, atomicWriteFileSync } from './atomic.js';
6+
7+
let dir: string;
8+
9+
afterEach(async () => {
10+
if (dir) await rm(dir, { recursive: true, force: true });
11+
});
12+
13+
async function makeDir() {
14+
dir = await mkdtemp(join(tmpdir(), 'atomic-test-'));
15+
return dir;
16+
}
17+
18+
describe('atomicWriteFile (async)', () => {
19+
it('writes content to the target path', async () => {
20+
const d = await makeDir();
21+
const target = join(d, 'out.json');
22+
await atomicWriteFile(target, '{"ok":true}');
23+
const content = await readFile(target, 'utf8');
24+
expect(content).toBe('{"ok":true}');
25+
});
26+
27+
it('overwrites existing file', async () => {
28+
const d = await makeDir();
29+
const target = join(d, 'out.json');
30+
await atomicWriteFile(target, 'first');
31+
await atomicWriteFile(target, 'second');
32+
expect(await readFile(target, 'utf8')).toBe('second');
33+
});
34+
35+
it('leaves no temp file on success', async () => {
36+
const d = await makeDir();
37+
const target = join(d, 'out.json');
38+
await atomicWriteFile(target, 'hello');
39+
const files = await import('fs/promises').then((m) => m.readdir(d));
40+
expect(files).toEqual(['out.json']);
41+
});
42+
43+
it('sets file mode when provided', async () => {
44+
const d = await makeDir();
45+
const target = join(d, 'secret.json');
46+
await atomicWriteFile(target, 'data', { mode: 0o600 });
47+
const s = await stat(target);
48+
expect(s.mode & 0o777).toBe(0o600);
49+
});
50+
51+
it('preserves existing 0600 mode on overwrite when no mode specified', async () => {
52+
const d = await makeDir();
53+
const target = join(d, 'secret.json');
54+
await atomicWriteFile(target, 'original', { mode: 0o600 });
55+
await atomicWriteFile(target, 'overwritten'); // no mode option
56+
const s = await stat(target);
57+
expect(s.mode & 0o777).toBe(0o600);
58+
expect(await readFile(target, 'utf8')).toBe('overwritten');
59+
});
60+
61+
it('sets exact mode even when umask would narrow it', async () => {
62+
const d = await makeDir();
63+
const target = join(d, 'wide.json');
64+
const prev = process.umask(0o022);
65+
try {
66+
await atomicWriteFile(target, 'data', { mode: 0o666 });
67+
} finally {
68+
process.umask(prev);
69+
}
70+
const s = await stat(target);
71+
expect(s.mode & 0o777).toBe(0o666);
72+
});
73+
});
74+
75+
describe('atomicWriteFileSync (sync)', () => {
76+
it('writes content to the target path', async () => {
77+
const d = await makeDir();
78+
const target = join(d, 'out.json');
79+
atomicWriteFileSync(target, '{"ok":true}');
80+
const content = await readFile(target, 'utf8');
81+
expect(content).toBe('{"ok":true}');
82+
});
83+
84+
it('overwrites existing file', async () => {
85+
const d = await makeDir();
86+
const target = join(d, 'out.json');
87+
atomicWriteFileSync(target, 'first');
88+
atomicWriteFileSync(target, 'second');
89+
expect(await readFile(target, 'utf8')).toBe('second');
90+
});
91+
92+
it('sets file mode when provided', async () => {
93+
const d = await makeDir();
94+
const target = join(d, 'secret.json');
95+
atomicWriteFileSync(target, 'data', { mode: 0o600 });
96+
const s = await stat(target);
97+
expect(s.mode & 0o777).toBe(0o600);
98+
});
99+
100+
it('preserves existing 0600 mode on overwrite when no mode specified', async () => {
101+
const d = await makeDir();
102+
const target = join(d, 'secret.json');
103+
atomicWriteFileSync(target, 'original', { mode: 0o600 });
104+
atomicWriteFileSync(target, 'overwritten'); // no mode option
105+
const s = await stat(target);
106+
expect(s.mode & 0o777).toBe(0o600);
107+
expect(await readFile(target, 'utf8')).toBe('overwritten');
108+
});
109+
110+
it('sets exact mode even when umask would narrow it', async () => {
111+
const d = await makeDir();
112+
const target = join(d, 'wide.json');
113+
const prev = process.umask(0o022);
114+
try {
115+
atomicWriteFileSync(target, 'data', { mode: 0o666 });
116+
} finally {
117+
process.umask(prev);
118+
}
119+
const s = await stat(target);
120+
expect(s.mode & 0o777).toBe(0o666);
121+
});
122+
});

0 commit comments

Comments
 (0)