audit fixes (#131)

* audit fixes

* format

* adjust form validation timing

Author: dillonstreator

docker-compose.yaml

@@ -1,6 +1,21 @@
 services:
   redis:
     image: redis:latest
+    # Bound memory so a flood of secrets can't exhaust the host. Secrets are
+    # TTL'd and must never be silently dropped, so use noeviction: once the cap
+    # is hit, new writes are rejected (surfacing pressure) rather than evicting
+    # live secrets. Tune REDIS_MAXMEMORY to the host.
+    command:
+      - redis-server
+      - --maxmemory
+      - ${REDIS_MAXMEMORY:-256mb}
+      - --maxmemory-policy
+      - noeviction
+      - --save
+      - '900'
+      - '1'
+      - --loglevel
+      - warning
     volumes:
       - redis_data:/data
 

package.json

@@ -12,7 +12,7 @@
     "typecheck": "turbo typecheck",
     "format": "prettier --write .",
     "format:check": "prettier --check .",
-    "redis": "docker compose run -p 6379:6379 -v ./redis-data:/data redis redis-server --save 1 1 --loglevel warning"
+    "redis": "docker compose run --rm -p 6379:6379 -v ./redis-data:/data redis redis-server --save 1 1 --loglevel notice"
   },
   "devDependencies": {
     "@eslint/js": "^9.39.2",

packages/core/package.json

@@ -28,6 +28,7 @@
     "@noble/hashes": "^1.8.0",
     "@noble/post-quantum": "^0.4.1",
     "buffer": "^6.0.3",
+    "ipaddr.js": "^2.4.0",
     "pako": "^2.1.0",
     "zod": "^4.1.5"
   },

packages/core/src/client.test.ts

@@ -0,0 +1,97 @@
+import { Client } from './client';
+
+// Minimal in-memory stand-in for the vault API so we can exercise the full
+// create -> read crypto pipeline (both layers, verification hash, versioned key)
+// without a real server or browser.
+type Entry = { c: string; h: string; m: unknown; b: boolean };
+
+const makeFakeServer = () => {
+  const store = new Map<string, Entry>();
+  let counter = 0;
+
+  const fetchImpl = (async (url: string | URL | Request, init?: RequestInit) => {
+    const u = new URL(typeof url === 'string' ? url : url instanceof URL ? url.href : url.url);
+    const method = init?.method ?? 'GET';
+
+    if (u.pathname === '/vault' && method === 'POST') {
+      const body = JSON.parse(init!.body as string);
+      const id = `id${counter++}`;
+      store.set(id, { c: body.c, h: body.h, m: body.m, b: body.b });
+      return new Response(JSON.stringify({ id, dt: 'delete-token' }), { status: 201 });
+    }
+
+    const match = u.pathname.match(/^\/vault\/(.+)$/);
+    if (match && method === 'GET') {
+      const entry = store.get(match[1]);
+      if (!entry) return new Response(null, { status: 404 });
+      const h = u.searchParams.get('h');
+      if (h !== entry.h) return new Response(null, { status: 400 });
+      return new Response(
+        JSON.stringify({ c: entry.c, b: entry.b, ttl: 1000, cd: 0, m: entry.m }),
+        { status: 200 },
+      );
+    }
+
+    return new Response(null, { status: 404 });
+  }) as unknown as typeof fetch;
+
+  return { fetchImpl, store };
+};
+
+describe('Client create -> read round trip', () => {
+  const origFetch = globalThis.fetch;
+  let server: ReturnType<typeof makeFakeServer>;
+  let client: Client;
+
+  beforeEach(() => {
+    server = makeFakeServer();
+    globalThis.fetch = server.fetchImpl;
+    client = new Client({ apiUrl: 'http://vault.test' });
+  });
+
+  afterEach(() => {
+    globalThis.fetch = origFetch;
+  });
+
+  it('round-trips content without a password (fast path, no version prefix)', async () => {
+    const created = await client.create({ c: 'hello world', ttl: 1000, b: false });
+    expect(created.key.startsWith('2.')).toBe(false);
+
+    const read = await client.read(created.id, created.key);
+    expect(read.c).toBe('hello world');
+  });
+
+  it('round-trips content with a password (versioned key + argon2 hash)', async () => {
+    const created = await client.create({ c: 'the secret', p: 'hunter2', ttl: 1000, b: false });
+    expect(created.key.startsWith('2.')).toBe(true);
+
+    const read = await client.read(created.id, created.key, 'hunter2');
+    expect(read.c).toBe('the secret');
+  });
+
+  it('rejects a read with the wrong password', async () => {
+    const created = await client.create({ c: 'the secret', p: 'hunter2', ttl: 1000, b: false });
+    await expect(client.read(created.id, created.key, 'wrong')).rejects.toThrow();
+  });
+
+  it('rejects a read missing the required password', async () => {
+    const created = await client.create({ c: 'the secret', p: 'hunter2', ttl: 1000, b: false });
+    await expect(client.read(created.id, created.key)).rejects.toThrow();
+  });
+
+  it('stores the asymmetric password algorithm only when a password is set', async () => {
+    const withPw = await client.create({ c: 'x', p: 'pw', ttl: 1000, b: false });
+    const withoutPw = await client.create({ c: 'y', ttl: 1000, b: false });
+
+    const pwEntry = server.store.get(withPw.id)!.m as {
+      encryption: { algorithm: string; passwordAlgorithm?: string };
+    };
+    const noPwEntry = server.store.get(withoutPw.id)!.m as {
+      encryption: { algorithm: string; passwordAlgorithm?: string };
+    };
+
+    expect(pwEntry.encryption.passwordAlgorithm).toBe('ml-kem-768-argon2');
+    expect(noPwEntry.encryption.passwordAlgorithm).toBeUndefined();
+    expect(noPwEntry.encryption.algorithm).toBe('ml-kem-768-2');
+  });
+});

packages/core/src/client.ts

@@ -6,7 +6,7 @@ import {
   ReadVaultResponse,
 } from './api';
 import { generateRandomString } from './random';
-import { sha512 } from './hash';
+import { KEY_VERSION_2_PREFIX, deriveVerificationHash, parseKey } from './verification';
 import { encryptionRegistry, compressionRegistry, validateMetadata } from './encryption/registry';
 import { ProcessingMetadata } from './vault';
 import { gcm } from './encryption';
@@ -63,6 +63,25 @@ export class Client {
     return processed;
   }
 
+  // Metadata for the inner (URL key) layer: carries compression + the primary
+  // encryption algorithm.
+  private keyLayerMetadata(metadata: ProcessingMetadata): ProcessingMetadata {
+    return {
+      compression: metadata.compression,
+      encryption: { algorithm: metadata.encryption.algorithm },
+    };
+  }
+
+  // Metadata for the outer (user password) layer: encryption only, no
+  // compression (compression happens once, on the inner layer).
+  private passwordLayerMetadata(metadata: ProcessingMetadata): ProcessingMetadata {
+    return {
+      encryption: {
+        algorithm: metadata.encryption.passwordAlgorithm ?? metadata.encryption.algorithm,
+      },
+    };
+  }
+
   private async recoverContent(
     encoded: string,
     key: string,
@@ -103,21 +122,45 @@ export class Client {
   async create(
     input: Omit<CreateVaultRequest, 'h' | 'm'> & { p?: string; m?: ProcessingMetadata },
   ): Promise<CreateVaultResponse & { key: string; hash: string }> {
-    const key = await generateRandomString(this.keyLength);
+    const rawKey = await generateRandomString(this.keyLength);
+    const hasPassword = input.p !== undefined && input.p !== '';
 
+    // Fast KDF for the high-entropy URL key layer; memory-hard Argon2id only for
+    // the user-password layer, and only when a password is actually set.
     const metadata: ProcessingMetadata = input.m ?? {
       compression: {
         algorithm: 'zlib:pako',
       },
       encryption: {
         algorithm: 'ml-kem-768-2',
+        passwordAlgorithm: hasPassword ? 'ml-kem-768-argon2' : undefined,
       },
     };
 
-    const processed = await this.processContent(input.c, metadata, key).then((r) =>
-      !input.p ? r : this.processContent(r, metadata, input.p),
-    );
-    const hash = sha512(key + (input.p ?? ''));
+    let processed: string;
+    if (!hasPassword) {
+      processed = await this.processContent(input.c, metadata, rawKey);
+    } else if (metadata.encryption.passwordAlgorithm) {
+      // Asymmetric: compress + key-encrypt inner, then password-encrypt outer.
+      const inner = await this.processContent(input.c, this.keyLayerMetadata(metadata), rawKey);
+      processed = await this.processContent(
+        inner,
+        this.passwordLayerMetadata(metadata),
+        input.p as string,
+      );
+    } else {
+      // Legacy symmetric pipeline (custom metadata without passwordAlgorithm).
+      const inner = await this.processContent(input.c, metadata, rawKey);
+      processed = await this.processContent(inner, metadata, input.p as string);
+    }
+
+    // Only password-protected secrets need the Argon2id (v2) verification hash;
+    // a bare high-entropy key is not brute-forceable, so it keeps the fast hash.
+    // The version prefix on the shared key tells the reader which scheme to use.
+    const key = hasPassword ? `${KEY_VERSION_2_PREFIX}${rawKey}` : rawKey;
+    const hash = hasPassword
+      ? await deriveVerificationHash('v2', rawKey, input.p)
+      : await deriveVerificationHash('legacy', rawKey);
 
     const response = await fetch(`${this.apiUrl}/vault`, {
       method: 'POST',
@@ -156,7 +199,8 @@ export class Client {
   }
 
   async read(id: string, key: string, password?: string) {
-    const h = sha512(key + (password ?? ''));
+    const { scheme, rawKey } = parseKey(key);
+    const h = await deriveVerificationHash(scheme, rawKey, password);
     const res = await fetch(`${this.apiUrl}/vault/${id}?h=${h}`, {
       headers: this.getHeaders(),
     });
@@ -170,11 +214,18 @@ export class Client {
     }
 
     const data = await (res.json() as Promise<ReadVaultResponse>);
-    const decrypted = password
-      ? await this.recoverContent(data.c, password, data.m).then((d) =>
-          this.recoverContent(d, key, data.m),
-        )
-      : await this.recoverContent(data.c, key, data.m);
+    let decrypted: string;
+    if (!password) {
+      decrypted = await this.recoverContent(data.c, rawKey, data.m);
+    } else if (data.m?.encryption?.passwordAlgorithm) {
+      // Asymmetric: password-decrypt outer, then key-decrypt + decompress inner.
+      const tmp = await this.recoverContent(data.c, password, this.passwordLayerMetadata(data.m));
+      decrypted = await this.recoverContent(tmp, rawKey, this.keyLayerMetadata(data.m));
+    } else {
+      // Legacy symmetric pipeline.
+      const tmp = await this.recoverContent(data.c, password, data.m);
+      decrypted = await this.recoverContent(tmp, rawKey, data.m);
+    }
 
     return {
       c: decrypted,

packages/core/src/encryption/index.ts

@@ -1,6 +1,8 @@
 export * as gcm from './gcm';
 export * as mlkem from './mlkem';
 export * as mlkem2 from './mlkem2';
+export * as mlkemArgon2 from './mlkem_argon2';
+export * as machine from './machine';
 
 export * from './encryption';
 export * from './registry';

packages/core/src/encryption/machine.test.ts

@@ -0,0 +1,31 @@
+import { encrypt, decrypt } from './machine';
+import { encrypt as gcmEncrypt } from './gcm';
+
+describe('machine cipher', () => {
+  const key = 'a-high-entropy-machine-key-0123456789';
+
+  it('round-trips content', async () => {
+    const plaintext = 'https://hooks.example.com/webhook?x=1';
+    const enc = await encrypt(plaintext, key);
+    expect(enc.startsWith('m1:')).toBe(true);
+    expect(await decrypt(enc, key)).toBe(plaintext);
+  });
+
+  it('produces distinct ciphertexts for the same input (random salt+iv)', async () => {
+    const a = await encrypt('same', key);
+    const b = await encrypt('same', key);
+    expect(a).not.toBe(b);
+  });
+
+  it('fails to decrypt with the wrong key', async () => {
+    const enc = await encrypt('secret', key);
+    await expect(decrypt(enc, 'wrong-key')).rejects.toThrow();
+  });
+
+  it('transparently decrypts legacy PBKDF2 (gcm) ciphertext', async () => {
+    const plaintext = '10.0.0.0/8,192.168.1.1';
+    const legacy = await gcmEncrypt(plaintext, key);
+    expect(legacy.startsWith('m1:')).toBe(false);
+    expect(await decrypt(legacy, key)).toBe(plaintext);
+  });
+});

packages/core/src/encryption/machine.ts

@@ -0,0 +1,69 @@
+// Symmetric encryption for data protected by a static, high-entropy *machine*
+// key (the server's ENCRYPTION_KEY) rather than a user-chosen password.
+//
+// The password-oriented ciphers (gcm/mlkem*) deliberately run an expensive KDF
+// (PBKDF2 at 2^19 iterations) to resist brute-forcing weak human passwords.
+// That cost is pointless for a machine key — there is no low-entropy secret to
+// stretch — and it runs on the server's hot path (every vault set/get/del and
+// every webhook enqueue/dequeue). Here we derive the AES key with HKDF, which is
+// fast and appropriate for a key that is already uniformly random.
+//
+// Format (base64, prefixed with a scheme tag for forward/backward routing):
+//   "m1:" || base64( salt[16] || iv[12] || aes-256-gcm-ciphertext )
+//
+// Values without the "m1:" prefix are legacy PBKDF2-format ciphertexts and are
+// transparently decrypted via the gcm module so existing vault entries keep
+// working.
+
+import { gcm as aesGcm } from '@noble/ciphers/aes';
+import { hkdf } from '@noble/hashes/hkdf';
+import { sha256 } from '@noble/hashes/sha2';
+import { randomBytes, concatBytes, utf8ToBytes } from '@noble/hashes/utils';
+import { Buffer } from '../buffer';
+import { Decrypt, Encrypt, DecryptError, EncryptError } from './encryption';
+import { decrypt as gcmDecrypt } from './gcm';
+
+const SALT_LENGTH = 16;
+const IV_LENGTH = 12;
+const KEY_LENGTH = 32;
+const SCHEME_PREFIX = 'm1:';
+const HKDF_INFO = utf8ToBytes('crypt.fyi/machine-metadata');
+
+const deriveKey = (key: string, salt: Uint8Array): Uint8Array =>
+  hkdf(sha256, utf8ToBytes(key), salt, HKDF_INFO, KEY_LENGTH);
+
+export const encrypt: Encrypt = async (content, key) => {
+  try {
+    const salt = randomBytes(SALT_LENGTH);
+    const iv = randomBytes(IV_LENGTH);
+    const derivedKey = deriveKey(key, salt);
+    const cipher = aesGcm(derivedKey, iv);
+    const encrypted = cipher.encrypt(utf8ToBytes(content));
+
+    const result = concatBytes(salt, iv, encrypted);
+    return SCHEME_PREFIX + Buffer.from(result).toString('base64');
+  } catch (error) {
+    throw new EncryptError(error);
+  }
+};
+
+export const decrypt: Decrypt = async (encryptedContent, key) => {
+  // Legacy entries were written with the password-KDF (PBKDF2) gcm cipher.
+  if (!encryptedContent.startsWith(SCHEME_PREFIX)) {
+    return gcmDecrypt(encryptedContent, key);
+  }
+
+  try {
+    const data = Buffer.from(encryptedContent.slice(SCHEME_PREFIX.length), 'base64');
+    const salt = data.subarray(0, SALT_LENGTH);
+    const iv = data.subarray(SALT_LENGTH, SALT_LENGTH + IV_LENGTH);
+    const ciphertext = data.subarray(SALT_LENGTH + IV_LENGTH);
+
+    const derivedKey = deriveKey(key, salt);
+    const cipher = aesGcm(derivedKey, iv);
+    const decrypted = cipher.decrypt(ciphertext);
+    return new TextDecoder().decode(decrypted);
+  } catch (error) {
+    throw new DecryptError(error);
+  }
+};

packages/core/src/encryption/mlkem_argon2.test.ts

@@ -0,0 +1,19 @@
+import { encrypt, decrypt } from './mlkem_argon2';
+
+describe('ml-kem-768-argon2', () => {
+  it('round-trips content with a password', async () => {
+    const enc = await encrypt('top secret', 'correct horse battery staple');
+    expect(await decrypt(enc, 'correct horse battery staple')).toBe('top secret');
+  });
+
+  it('produces distinct ciphertexts for the same input', async () => {
+    const a = await encrypt('same', 'pw');
+    const b = await encrypt('same', 'pw');
+    expect(a).not.toBe(b);
+  });
+
+  it('fails to decrypt with the wrong password', async () => {
+    const enc = await encrypt('secret', 'right');
+    await expect(decrypt(enc, 'wrong')).rejects.toThrow();
+  });
+});