fix(auth): compare API keys by value; read note headers as data only

src/classify/processor.js

@@ -1,6 +1,6 @@
 import { readFileSync, writeFileSync } from 'fs';
 import { join } from 'path';
-import matter from 'gray-matter';
+import { readNote, writeNote } from '../utils/frontmatter.js';
 import { classifyNote } from './classifier.js';
 import { scanVault } from '../vault/indexer.js';
 import { indexVault } from '../vault/indexer.js';
@@ -15,7 +15,7 @@ function isUnprocessed(filePath, vaultPath) {
   try {
     const content = readFileSync(filePath, 'utf-8');
     if (!content.trim()) return false; // skip empty files
-    const { data: fm } = matter(content);
+    const { data: fm } = readNote(content);
     return !fm.classified;
   } catch {
     return false;
@@ -36,7 +36,7 @@ export async function processNewClippings(vaultPath, { dryRun = false } = {}) {
   for (const filePath of unprocessed) {
     const rel = filePath.replace(vaultPath + '/', '');
     const raw = readFileSync(filePath, 'utf-8');
-    const { data: fm, content: body } = matter(raw);
+    const { data: fm, content: body } = readNote(raw);
     const title = fm.title || rel.split('/').pop().replace(/\.md$/, '');
 
     console.log(`Classifying: ${rel}`);
@@ -75,7 +75,7 @@ export async function processNewClippings(vaultPath, { dryRun = false } = {}) {
       if (val === null) delete updatedFm[key];
     }
 
-    const updated = matter.stringify(body, updatedFm);
+    const updated = writeNote(body, updatedFm);
     writeFileSync(filePath, updated);
 
     results.push({ path: rel, status: 'classified', classification });

src/classify/summarizer.js

@@ -1,7 +1,7 @@
 import { spawn } from 'child_process';
 import { readFileSync, writeFileSync } from 'fs';
 import { join } from 'path';
-import matter from 'gray-matter';
+import { readNote, writeNote } from '../utils/frontmatter.js';
 import { scanVault } from '../vault/indexer.js';
 
 const CLAUDE_PATH = process.env.CLAUDE_PATH || 'claude';
@@ -68,7 +68,7 @@ export async function summarizeUnsummarized(vaultPath, { dryRun = false, limit =
     try {
       const raw = readFileSync(filePath, 'utf-8');
       if (!raw.trim()) continue;
-      const { data: fm, content: body } = matter(raw);
+      const { data: fm, content: body } = readNote(raw);
       if (fm.summary) continue; // already has summary
       if (body.trim().length < 100) continue; // too short to summarize
       needsSummary.push({ filePath, fm, body, rel: filePath.replace(vaultPath + '/', '') });
@@ -100,7 +100,7 @@ export async function summarizeUnsummarized(vaultPath, { dryRun = false, limit =
         summary: result.summary,
         key_topics: result.key_topics,
       };
-      const updated = matter.stringify(note.body, updatedFm);
+      const updated = writeNote(note.body, updatedFm);
       writeFileSync(note.filePath, updated);
     }
 

src/middleware/api-key.js

@@ -1,4 +1,30 @@
 // src/middleware/api-key.js
+import { timingSafeEqual } from 'crypto';
+
+const KEY_SERVICES = [
+  ['KB_API_KEY_CLAUDE', 'claude'],
+  ['KB_API_KEY_OPENAI', 'openai'],
+  ['KB_API_KEY_GEMINI', 'gemini'],
+];
+
+/**
+ * The service a key belongs to, or null.
+ * The key is compared with each configured key by value. It must never be used to look up a property of a
+ * plain object: every object inherits "constructor", "toString", "__proto__" and so on, and such a lookup
+ * finds them, so those names would pass as valid keys.
+ */
+export function serviceForKey(key) {
+  if (typeof key !== 'string' || key.length === 0) return null;
+  const given = Buffer.from(key);
+  let service = null;
+  for (const [envName, name] of KEY_SERVICES) {
+    const configured = process.env[envName];
+    if (!configured) continue;
+    const expected = Buffer.from(configured);
+    if (expected.length === given.length && timingSafeEqual(expected, given) && service === null) service = name;
+  }
+  return service;
+}
 
 /**
  * Creates Express middleware that validates API keys from env vars.
@@ -11,7 +37,7 @@ export function createApiKeyMiddleware() {
     let key = req.headers['x-api-key'];
     if (!key) {
       const auth = req.headers.authorization;
-      if (auth && auth.startsWith('Bearer ')) {
+      if (typeof auth === 'string' && auth.startsWith('Bearer ')) {
         key = auth.slice(7);
       }
     }
@@ -20,13 +46,7 @@ export function createApiKeyMiddleware() {
       return res.status(401).json({ error: 'Missing API key. Provide X-API-Key header or Authorization: Bearer <key>' });
     }
 
-    // Match against configured keys (only add defined keys to prevent undefined match)
-    const keyMap = {};
-    if (process.env.KB_API_KEY_CLAUDE) keyMap[process.env.KB_API_KEY_CLAUDE] = 'claude';
-    if (process.env.KB_API_KEY_OPENAI) keyMap[process.env.KB_API_KEY_OPENAI] = 'openai';
-    if (process.env.KB_API_KEY_GEMINI) keyMap[process.env.KB_API_KEY_GEMINI] = 'gemini';
-
-    const service = keyMap[key];
+    const service = serviceForKey(key);
     if (!service) {
       return res.status(403).json({ error: 'Invalid API key' });
     }

src/middleware/brain-auth.js

@@ -0,0 +1,39 @@
+// src/middleware/brain-auth.js
+import { createApiKeyMiddleware, serviceForKey } from './api-key.js';
+
+/**
+ * The gate in front of the Brain API and the MCP HTTP endpoint: an API key (X-API-Key, or as a Bearer token),
+ * or an OAuth access token issued by this server. Anything else gets 401 with the OAuth challenge.
+ */
+export function createBrainAuth({ verifyAccessToken, challenge }) {
+  const apiKeyAuth = createApiKeyMiddleware();
+  return async (req, res, next) => {
+    // Try API key first (fast path)
+    if (req.headers['x-api-key']) {
+      return apiKeyAuth(req, res, next);
+    }
+
+    const authHeader = req.headers.authorization;
+    if (typeof authHeader === 'string' && authHeader.startsWith('Bearer ')) {
+      const token = authHeader.slice(7);
+      // An API key sent as a Bearer token
+      const service = serviceForKey(token);
+      if (service) {
+        req.apiService = service;
+        return next();
+      }
+
+      // Otherwise validate as an OAuth access token issued by this server
+      const claims = token ? await verifyAccessToken(token) : null;
+      if (claims) {
+        req.apiService = 'oauth';
+        req.oauthClaims = claims;
+        return next();
+      }
+    }
+
+    // RFC 9728: point MCP clients at the metadata that starts the OAuth flow.
+    res.setHeader('WWW-Authenticate', challenge());
+    return res.status(401).json({ error: 'Missing or invalid authentication. Provide X-API-Key header or OAuth Bearer token.' });
+  };
+}

src/server.js

@@ -14,7 +14,7 @@
 import { toNodeHandler } from 'better-auth/node';
 import { oauthProviderAuthServerMetadata, oauthProviderOpenIdConfigMetadata } from '@better-auth/oauth-provider';
 import { auth, migrateAuthSchema, verifyAccessToken, mcpChallenge, protectedResourceMetadata, BASE_URL } from './auth-oauth.js';
-import { createApiKeyMiddleware } from './middleware/api-key.js';
+import { createBrainAuth } from './middleware/brain-auth.js';
 import v1Router from './routes/v1.js';
 import openapiRoute from './routes/openapi.js';
 import { mcpHttpHandler, mcpGetHandler } from './mcp-http.js';
@@ -138,61 +138,26 @@
   app.use(apiRoutes);
 
   // --- Brain API (external access via remote domain or localhost) ---
-  const apiKeyAuth = createApiKeyMiddleware();
-
   // Auth middleware: accepts EITHER API key OR OAuth Bearer token
-  const brainAuth = async (req, res, next) => {
-    // Try API key first (fast path)
-    const apiKey = req.headers['x-api-key'];
-    if (apiKey) {
-      return apiKeyAuth(req, res, next);
-    }
-
-    // Try OAuth Bearer token
-    const authHeader = req.headers.authorization;
-    if (authHeader && authHeader.startsWith('Bearer ')) {
-      // Check if it's an API key in Bearer format
-      const token = authHeader.slice(7);
-      const keyMap = {};
-      if (process.env.KB_API_KEY_CLAUDE) keyMap[process.env.KB_API_KEY_CLAUDE] = 'claude';
-      if (process.env.KB_API_KEY_OPENAI) keyMap[process.env.KB_API_KEY_OPENAI] = 'openai';
-      if (process.env.KB_API_KEY_GEMINI) keyMap[process.env.KB_API_KEY_GEMINI] = 'gemini';
-      if (keyMap[token]) {
-        req.apiService = keyMap[token];
-        return next();
-      }
-
-      // Otherwise validate as an OAuth access token issued by this server
-      const claims = await verifyAccessToken(token);
-      if (claims) {
-        req.apiService = 'oauth';
-        req.oauthClaims = claims;
-        return next();
-      }
-    }
-
-    // RFC 9728: point MCP clients at the metadata that starts the OAuth flow.
-    res.setHeader('WWW-Authenticate', mcpChallenge());
-    return res.status(401).json({ error: 'Missing or invalid authentication. Provide X-API-Key header or OAuth Bearer token.' });
-  };
+  const brainAuth = createBrainAuth({ verifyAccessToken, challenge: mcpChallenge });
 
   // Public (no auth): OpenAPI spec + health
   app.get('/openapi.json', corsMiddleware, openapiRoute);
   app.get('/api/v1/health', corsMiddleware, (req, res) => res.json({ status: 'ok', uptime: process.uptime() }));
 
   // Authenticated: v1 API (API key or OAuth)
   app.use('/api/v1', corsMiddleware, brainAuth, v1Router);
 
   // Authenticated: MCP HTTP (API key or OAuth)
   app.post('/mcp', (req, res, next) => {
     console.log(`[MCP POST] session=${req.headers['mcp-session-id'] || 'new'} auth=${req.headers.authorization ? 'Bearer...' : req.headers['x-api-key'] ? 'APIKey' : 'NONE'} origin=${req.headers.origin || 'none'}`);
     next();
   }, corsMiddleware, brainAuth, mcpHttpHandler);
   app.get('/mcp', (req, res, next) => {
     console.log(`[MCP GET] session=${req.headers['mcp-session-id'] || 'none'} auth=${req.headers.authorization ? 'Bearer...' : 'NONE'}`);
     next();
   }, corsMiddleware, brainAuth, mcpGetHandler);
   app.delete('/mcp', corsMiddleware, brainAuth, (req, res) => {
     // Session termination endpoint (MCP spec)
     res.status(200).json({ ok: true });
   });

src/utils/frontmatter.js

@@ -1,4 +1,5 @@
 // src/utils/frontmatter.js
+import matter from 'gray-matter';
 
 /**
  * Format tags as YAML block-list for Obsidian compatibility.
@@ -18,3 +19,29 @@ export function formatYamlTags(tags) {
     return `  - ${t}`;
   }).join('\n');
 }
+
+
+const refuse = () => { throw new Error('only YAML note headers are read'); };
+const DATA_ONLY = { engines: { javascript: refuse, js: refuse, coffee: refuse, coffeescript: refuse, cson: refuse, toml: refuse, json: refuse } };
+
+/**
+ * Read a note's header and body. A note header is data: only a YAML header is read.
+ * A header that names another language (for example a code language) is refused, so nothing in a note is ever run.
+ * @param {string} content - the whole note
+ * @returns {{ data: object, content: string }}
+ */
+export function readNote(content) {
+  // A language name starts with a letter; a line of four or more dashes is a rule line, not a header.
+  const declared = String(content).match(/^---[ \t]*([A-Za-z][A-Za-z0-9_]*)[ \t]*\r?\n/)?.[1];
+  if (declared && !/^ya?ml$/i.test(declared)) refuse();
+  const { data, content: body } = matter(String(content), DATA_ONLY);
+  return { data, content: body };
+}
+
+/**
+ * Write a note from a body and a header object (YAML). The body is written as it is: it is never read as a header,
+ * whatever it starts with. (Passing the body as a string would make the library parse it again.)
+ */
+export function writeNote(body, data) {
+  return matter.stringify({ content: String(body), data: {} }, data, DATA_ONLY);
+}

src/vault/parser.js

@@ -1,4 +1,4 @@
-import matter from 'gray-matter';
+import { readNote } from '../utils/frontmatter.js';
 
 // Map folder prefixes to note types
 // Map folder prefixes to note types — supports both numbered (00_inbox) and unnumbered (inbox) prefixes
@@ -57,7 +57,7 @@ function normalizeTags(tags) {
 }
 
 export function parseVaultNote(content, vaultPath) {
-  const { data: fm, content: body } = matter(content);
+  const { data: fm, content: body } = readNote(content);
 
   const title = fm.title || inferTitleFromContent(body) || vaultPath.split('/').pop().replace(/\.md$/, '');
   const type = fm.type || inferTypeFromPath(vaultPath);

tests/auth-bypass.test.js

@@ -0,0 +1,92 @@
+// tests/auth-bypass.test.js
+// A key is only valid when it equals a configured key. Names that every JavaScript object inherits
+// ("constructor", "toString", "__proto__", …) must never authenticate.
+import { describe, it, beforeEach, afterEach } from 'node:test';
+import assert from 'node:assert';
+import { createApiKeyMiddleware, serviceForKey } from '../src/middleware/api-key.js';
+import { createBrainAuth } from '../src/middleware/brain-auth.js';
+
+const INHERITED = ['constructor', 'toString', 'valueOf', 'hasOwnProperty', 'isPrototypeOf', 'propertyIsEnumerable', 'toLocaleString', '__proto__', '__defineGetter__', '__lookupGetter__'];
+
+function call(mw, headers) {
+  return new Promise((resolve) => {
+    const req = { headers };
+    const res = { headers: {}, setHeader(k, v) { this.headers[k] = v; }, status(code) { return { json: (body) => resolve({ passed: false, code, body, req, res }) }; } };
+    Promise.resolve(mw(req, res, () => resolve({ passed: true, req, res }))).catch((e) => resolve({ passed: false, threw: e }));
+  });
+}
+
+describe('authentication cannot be bypassed with an inherited property name', () => {
+  beforeEach(() => {
+    process.env.KB_API_KEY_CLAUDE = 'test-key-claude-1234';
+    process.env.KB_API_KEY_OPENAI = 'test-key-openai-5678';
+    delete process.env.KB_API_KEY_GEMINI;
+  });
+  afterEach(() => { delete process.env.KB_API_KEY_CLAUDE; delete process.env.KB_API_KEY_OPENAI; });
+
+  it('serviceForKey knows only the configured keys', () => {
+    assert.strictEqual(serviceForKey('test-key-claude-1234'), 'claude');
+    assert.strictEqual(serviceForKey('test-key-openai-5678'), 'openai');
+    for (const name of [...INHERITED, '', 'test-key-claude-123', 'test-key-claude-12345', 'undefined', 'null']) assert.strictEqual(serviceForKey(name), null, name);
+    for (const odd of [undefined, null, 42, {}, ['test-key-claude-1234'], true]) assert.strictEqual(serviceForKey(odd), null, String(odd));
+  });
+
+  it('an unset key never matches the empty string or "undefined"', () => {
+    delete process.env.KB_API_KEY_CLAUDE; delete process.env.KB_API_KEY_OPENAI;
+    for (const name of ['', 'undefined', ...INHERITED]) assert.strictEqual(serviceForKey(name), null, name);
+  });
+
+  it('the API-key middleware rejects every inherited name, in both header forms', async () => {
+    const mw = createApiKeyMiddleware();
+    for (const name of INHERITED) {
+      const byHeader = await call(mw, { 'x-api-key': name });
+      assert.deepStrictEqual([byHeader.passed, byHeader.code], [false, 403], `X-API-Key: ${name}`);
+      assert.strictEqual(byHeader.req.apiService, undefined);
+      const byBearer = await call(mw, { authorization: `Bearer ${name}` });
+      assert.deepStrictEqual([byBearer.passed, byBearer.code], [false, 403], `Bearer ${name}`);
+    }
+  });
+
+  it('the API-key middleware still accepts a real key and names its service', async () => {
+    const mw = createApiKeyMiddleware();
+    const ok = await call(mw, { 'x-api-key': 'test-key-openai-5678' });
+    assert.deepStrictEqual([ok.passed, ok.req.apiService], [true, 'openai']);
+    const twice = await call(mw, { 'x-api-key': ['test-key-openai-5678', 'test-key-openai-5678'] });
+    assert.strictEqual(twice.passed, false, 'a repeated header is not a key');
+  });
+
+  describe('the Brain API gate (API key or OAuth token)', () => {
+    const gate = (verify = async () => null) => createBrainAuth({ verifyAccessToken: verify, challenge: () => 'Bearer resource_metadata="x"' });
+
+    it('rejects every inherited name as a Bearer token and as an X-API-Key', async () => {
+      const seen = [];
+      const mw = gate(async (token) => { seen.push(token); return null; });
+      for (const name of INHERITED) {
+        const bearer = await call(mw, { authorization: `Bearer ${name}` });
+        assert.deepStrictEqual([bearer.passed, bearer.code], [false, 401], `Bearer ${name}`);
+        assert.strictEqual(bearer.req.apiService, undefined);
+        assert.strictEqual(bearer.res.headers['WWW-Authenticate'], 'Bearer resource_metadata="x"');
+        const header = await call(mw, { 'x-api-key': name });
+        assert.deepStrictEqual([header.passed, header.code], [false, 403], `X-API-Key: ${name}`);
+      }
+      assert.deepStrictEqual(seen, INHERITED, 'a token that is not an API key is checked as an OAuth token, nothing else');
+    });
+
+    it('accepts a real API key in either header, and a valid OAuth token', async () => {
+      const key = await call(gate(), { 'x-api-key': 'test-key-claude-1234' });
+      assert.deepStrictEqual([key.passed, key.req.apiService], [true, 'claude']);
+      const bearerKey = await call(gate(), { authorization: 'Bearer test-key-openai-5678' });
+      assert.deepStrictEqual([bearerKey.passed, bearerKey.req.apiService], [true, 'openai']);
+      const oauth = await call(gate(async (t) => (t === 'good-token' ? { sub: 'user-1' } : null)), { authorization: 'Bearer good-token' });
+      assert.deepStrictEqual([oauth.passed, oauth.req.apiService, oauth.req.oauthClaims], [true, 'oauth', { sub: 'user-1' }]);
+    });
+
+    it('answers 401 with the OAuth challenge when nothing valid is sent', async () => {
+      for (const headers of [{}, { authorization: 'Basic abc' }, { authorization: 'Bearer ' }, { authorization: 'Bearer wrong' }]) {
+        const r = await call(gate(), headers);
+        assert.deepStrictEqual([r.passed, r.code], [false, 401], JSON.stringify(headers));
+        assert.ok(r.res.headers['WWW-Authenticate']);
+      }
+    });
+  });
+});