fix(mcp): SHA-256 the launch fingerprint — shortHash was forgeable in minutes

Review on #55, and the most serious finding of the series. It is a real break of
the launch gate, not a theoretical weakness.

launchFingerprint used shortHash — a 32-bit djb2 variant emitted as 6 base36
chars, so a ~2^31 output space. That helper exists to truncate tool names stably;
it was never meant to carry an authorization decision, and I reached for it out of
convenience.

Why it is exploitable rather than merely weak: the attacker AUTHORED the benign
command that earned trust, so they know the target value exactly, and they control
the replacement. That makes this a second preimage, not a birthday collision.
Measured on one core: 1,019,215,873 candidate hashes in 150s (~6.8M/sec) against a
2.15e9 space — so ~5 minutes of offline work to find a malicious command whose
fingerprint matches. Ship it in a later commit and the gate says "already trusted"
and spawns it silently. That is RCE with the consent prompt bypassed.

Now SHA-256 over canonical material. Two further hardenings:

  * env pairs are STRUCTURAL — [[k, v]] sorted — not joined into "k=v". Joining is
    ambiguous: {'a': 'b=c'} and {'a=b': 'c'} both flatten to "a=b=c", handing over a
    collision for free in the one function where collisions are the threat.
  * a non-array args no longer reaches .map and throws, per the same review.

No migration concern: S4b has not shipped, so no trust store exists in the wild.
Had it shipped, changing the hash would invalidate stored fingerprints and re-prompt
once — the safe direction.

A known-answer test pins the digest so a future "simplification" back to a short
hash fails loudly rather than quietly.

56 mcpConfig cases; 23 suites, 0 failures.

Author: ndemianc

docs/MCP.md

@@ -121,6 +121,14 @@ Two details the one-line rule above does not carry, both load-bearing:
   `NODE_OPTIONS=--require /tmp/evil.js` is RCE without touching command or args at all. It is shown on
   the card for the same reason.
 
+- **The fingerprint is SHA-256**, not the `shortHash` used for tool-name truncation. That helper is a
+  32-bit djb2 emitted as 6 base36 chars (~2^31), and here the attacker knows the trusted value — they
+  authored the command that earned trust — and controls the replacement, so a second preimage *is* the
+  attack. Measured at ~6.8M candidate hashes/sec on one core, that is roughly five minutes of offline
+  work to forge a malicious command that inherits trust. Env pairs are encoded structurally
+  (`[[k, v]]`, sorted) rather than joined into `k=v`, which would make `{'a': 'b=c'}` and `{'a=b': 'c'}`
+  collide for free.
+
 The gate **fails closed**: with no webview there is nobody to ask, so the server does not start. A
 headless or test context must never be the path that silently spawns a repo's process.
 

extensions/levelcode-ai/mcpConfig.js

@@ -20,13 +20,15 @@
  *  by approveMcpLaunch in agent.js. For the same reason the
  *  user's setting WINS on a name collision: a repo can never shadow a server the user defined.
  *
- *  Pure + dependency-free (path only) — file reading is injected as a readFile callback, so all of it
+ *  Pure + dependency-free (node builtins `path` and `crypto` only) — file reading is injected as a
+ *  readFile callback, so all of it
  *  is unit-testable (test/mcpConfig.test.js) without a filesystem, a child process, or the editor.
  *  Nothing here connects, spawns, or calls anything.
  *--------------------------------------------------------------------------------------------*/
 'use strict';
 
 const path = require('path');
+const crypto = require('crypto');
 
 // The agent's built-in tools (agent.js TOOLS). An MCP tool may never shadow one of these.
 const BUILTIN_TOOL_NAMES = [
@@ -503,12 +505,29 @@ function previewArgs(args) {
  * `env` is included, and that is not padding: `NODE_OPTIONS=--require /tmp/evil.js` turns an innocent
  * `node` command into arbitrary code execution without touching command or args. Keys are sorted so an
  * unrelated reordering of the JSON does not spuriously revoke trust.
+ *
+ * SHA-256, NOT the shortHash used for tool-name truncation. shortHash is a 32-bit djb2 variant emitted
+ * as 6 base36 chars — a ~2^31 space, and it is not collision-resistant by design or intent. Here the
+ * attacker both KNOWS the trusted value (they authored the command that earned trust) and controls the
+ * replacement, so they need a second preimage — measured at ~6.8M candidate hashes/sec on one core,
+ * i.e. roughly five minutes of offline work to forge a malicious command that inherits trust. A
+ * truncation helper is the wrong tool for an authorization decision; the cost of a real hash here is
+ * one call per server per run.
  */
 function launchFingerprint(server) {
 	const s = server || {};
-	const env = s.env || {};
-	const envPairs = Object.keys(env).sort().map((k) => k + '=' + String(env[k]));
-	return shortHash(JSON.stringify([String(s.command || ''), (s.args || []).map(String), envPairs]));
+	// Non-array args / non-object env become null rather than [] or {}: a malformed entry must not
+	// fingerprint the same as an absent one, and `.map` on a string would throw in a function whose
+	// whole job is to be safe to call on anything.
+	const args = Array.isArray(s.args) ? s.args.map(String) : null;
+	const rawEnv = (s.env && typeof s.env === 'object' && !Array.isArray(s.env)) ? s.env : null;
+	// Pairs stay STRUCTURAL — [[k, v]] — instead of being joined into "k=v". Joining is ambiguous:
+	// { 'a': 'b=c' } and { 'a=b': 'c' } both flatten to "a=b=c", which is a collision handed over for
+	// free in the one function where collisions are the threat.
+	const env = rawEnv ? Object.keys(rawEnv).sort().map((k) => [k, String(rawEnv[k])]) : null;
+
+	const material = JSON.stringify({ command: String(s.command || ''), args: args, env: env });
+	return crypto.createHash('sha256').update(material, 'utf8').digest('hex');
 }
 
 /**

extensions/levelcode-ai/test/mcpConfig.test.js

@@ -675,6 +675,54 @@ test('G1: trust is keyed on what would RUN, so a repo cannot swap the command af
 		'changed env must re-prompt');
 });
 
+test('G1: the fingerprint is a real hash, not the tool-name truncation helper', () => {
+	// This value decides whether a repo-authored process launches WITHOUT asking, and the attacker both
+	// knows the trusted value (they authored the command that earned trust) and controls the
+	// replacement — so a second preimage IS the attack. shortHash is a 32-bit djb2 emitted as 6 base36
+	// chars: a ~2^31 space, searchable at ~6.8M/sec on one core, i.e. minutes of offline work.
+	const fp = M.launchFingerprint(srv());
+	assert.match(fp, /^[0-9a-f]{64}$/, 'must be a SHA-256 hex digest');
+	assert.ok(fp.length > 32, 'a 6-char truncation helper must never be what gates a process launch');
+
+	// Known-answer, so a future "simplification" back to a short hash fails loudly rather than quietly.
+	const expected = require('crypto').createHash('sha256')
+		.update(JSON.stringify({ command: 'npx', args: srv().args, env: [] }), 'utf8').digest('hex');
+	assert.strictEqual(fp, expected, 'material is {command, args, env} with env as sorted [k,v] pairs');
+});
+
+test('G1: env pairs cannot be confused by an "=" inside a key or value', () => {
+	// Joining pairs into "k=v" would make these two identical strings — a collision handed over free in
+	// the one function where collisions are the whole threat.
+	const a = M.launchFingerprint(srv({ env: { a: 'b=c' } }));
+	const b = M.launchFingerprint(srv({ env: { 'a=b': 'c' } }));
+	assert.notStrictEqual(a, b, 'the encoding must be structural, not string-joined');
+});
+
+test('G1: a malformed entry fingerprints without throwing', () => {
+	// launchFingerprint is exported and documented safe to call on anything. Before this, a non-array
+	// `args` reached `.map` and threw.
+	assert.doesNotThrow(() => M.launchFingerprint({ command: 'x', args: 'not-an-array' }));
+	assert.doesNotThrow(() => M.launchFingerprint({ command: 'x', env: 'not-an-object' }));
+	assert.doesNotThrow(() => M.launchFingerprint({ command: 'x', args: 42, env: [] }));
+	assert.doesNotThrow(() => M.launchFingerprint(null));
+	assert.doesNotThrow(() => M.launchFingerprint({}));
+
+	// A malformed args collapses to the same fingerprint as an absent one, and that is fine rather than
+	// a gap: normalizeServer REJECTS a non-array args before a server can reach the gate, so neither
+	// shape is reachable here, and "no usable args" is the conservative reading of both. What must hold
+	// is that WELL-FORMED inputs stay distinguishable — asserted throughout the rest of these G1 tests.
+	assert.strictEqual(
+		M.launchFingerprint({ command: 'x', args: 'evil' }),
+		M.launchFingerprint({ command: 'x' }),
+		'documented: unreachable malformed shapes collapse; the command itself still differentiates'
+	);
+	assert.notStrictEqual(
+		M.launchFingerprint({ command: 'x', args: 'evil' }),
+		M.launchFingerprint({ command: 'y' }),
+		'the command is always part of the material'
+	);
+});
+
 test('G1: nothing is trusted by default, and unrelated servers stay untrusted', () => {
 	assert.ok(!M.isLaunchTrusted(srv(), {}), 'an empty store trusts nothing');
 	assert.ok(!M.isLaunchTrusted(srv(), null), 'a missing store trusts nothing');