[Security/High] AES-256-GCM encryption uses 16-byte IV and no AAD binding

[jonwiggins]

Summary

secret-service.encrypt() generates a 16-byte IV (AES-GCM standard is 12 bytes) and does not bind ciphertext to its context via Additional Authenticated Data (AAD). Encrypted blobs are not bound to the secret name, scope, or workspace, so a row swapped or restored from a different context decrypts cleanly.

Affected files

• apps/api/src/services/secret-service.ts:50-64 — encrypt/decrypt lack setAAD
• apps/api/src/services/secret-service.ts:101-115 — retrieveSecret only adds the workspace filter when workspaceId is supplied; nullable workspace lets requests retrieve global secrets without context

export function encrypt(plaintext: string) {
  const key = encryptionKey();
  const iv = randomBytes(16);                           // ← should be 12
  const cipher = createCipheriv("aes-256-gcm", key, iv);
  // ← no setAAD
  const encrypted = Buffer.concat([cipher.update(plaintext, "utf8"), cipher.final()]);
  const authTag = cipher.getAuthTag();
  return { encrypted, iv, authTag };
}

Impact

• A SQL bug, backup restore, or admin-level DB access can move a high-value secret row to a different (name, scope, workspaceId) and have it decrypt successfully under the wrong identity.
• The 16-byte IV is functionally fine (GCM accepts arbitrary lengths) but deviates from NIST SP 800-38D and triggers GHASH internally.

Remediation

Add AAD binding with the secret's identifying context:

export function encrypt(plaintext: string, aad: Buffer) {
  const iv = randomBytes(12);
  const cipher = createCipheriv("aes-256-gcm", encryptionKey(), iv);
  cipher.setAAD(aad);
  const encrypted = Buffer.concat([cipher.update(plaintext, "utf8"), cipher.final()]);
  return { encrypted, iv, authTag: cipher.getAuthTag() };
}

// Call site
const aad = Buffer.from(`${name}|${scope}|${workspaceId ?? "global"}`);
encrypt(value, aad);

This is a backwards-incompatible migration: re-encrypt all rows during the next deployment (a one-shot script that walks the secrets table and repos.encryptedSlackWebhookUrl etc.).

Also: in retrieveSecret, require a workspaceId for any non-global scope rather than silently omitting the filter.

[jonwiggins]

Optio is working on this issue.

Task ID: 6776b529-5452-4917-b598-257465602f8a
Agent: claude-code

[jonwiggins]

✅ Optio completed this issue. Changes merged in #319.