feat(phase-2): workstream 1 — HashiCorp Vault credential storage

Vault is optional — server falls back to bcrypt (Phase 1 behaviour) when VAULT_ADDR is not set. Full coexistence: existing bcrypt credentials continue to work until rotated. Changes: - src/vault/VaultClient.ts — wraps node-vault KV v2; writeSecret, readSecret, verifySecret (constant-time), deleteSecret - src/db/migrations/005_add_vault_path.sql — vault_path column on credentials - CredentialRepository — createWithVaultPath, updateVaultPath methods - CredentialService — routes generate/rotate through Vault when configured; bcrypt path unchanged - OAuth2Service — verifies via Vault when vaultPath set, bcrypt otherwise - src/app.ts — createVaultClientFromEnv() wired into service layer - ICredentialRow — vaultPath field added - docs/devops/environment-variables.md — VAULT_ADDR, VAULT_TOKEN, VAULT_MOUNT - docs/devops/vault-setup.md — dev quickstart, production config, migration guide - tests: 33/33 unit tests pass (VaultClient + CredentialService Vault path) - node-vault + @types/node-vault installed Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-03-28 15:02:33 +00:00
parent 7593bfe1c1
commit 90a4addb21
14 changed files with 1064 additions and 36 deletions
--- a/tests/unit/services/CredentialService.test.ts
+++ b/tests/unit/services/CredentialService.test.ts
@@ -7,6 +7,7 @@ import { CredentialService } from '../../../src/services/CredentialService';
 import { CredentialRepository } from '../../../src/repositories/CredentialRepository';
 import { AgentRepository } from '../../../src/repositories/AgentRepository';
 import { AuditService } from '../../../src/services/AuditService';
+import { VaultClient } from '../../../src/vault/VaultClient';
 import {
  AgentNotFoundError,
  CredentialNotFoundError,
@@ -18,10 +19,12 @@ import { IAgent, ICredential, ICredentialRow } from '../../../src/types/index';
 jest.mock('../../../src/repositories/CredentialRepository');
 jest.mock('../../../src/repositories/AgentRepository');
 jest.mock('../../../src/services/AuditService');
+jest.mock('../../../src/vault/VaultClient');

 const MockCredentialRepo = CredentialRepository as jest.MockedClass<typeof CredentialRepository>;
 const MockAgentRepo = AgentRepository as jest.MockedClass<typeof AgentRepository>;
 const MockAuditService = AuditService as jest.MockedClass<typeof AuditService>;
+const MockVaultClient = VaultClient as jest.MockedClass<typeof VaultClient>;

 const AGENT_ID = uuidv4();
 const CREDENTIAL_ID = uuidv4();
@@ -51,6 +54,7 @@ const MOCK_CREDENTIAL: ICredential = {
 const MOCK_CREDENTIAL_ROW: ICredentialRow = {
  ...MOCK_CREDENTIAL,
  secretHash: '$2b$10$somehashvalue',
+  vaultPath: null,
 };

 const IP = '127.0.0.1';
@@ -205,3 +209,94 @@ describe('CredentialService', () => {
    });
  });
 });
+
+// ─── Vault-path tests ──────────────────────────────────────────────────────
+
+describe('CredentialService — Vault path (Phase 2)', () => {
+  let service: CredentialService;
+  let credentialRepo: jest.Mocked<CredentialRepository>;
+  let agentRepo: jest.Mocked<AgentRepository>;
+  let auditService: jest.Mocked<AuditService>;
+  let vaultClient: jest.Mocked<VaultClient>;
+
+  const VAULT_PATH = `secret/data/agentidp/agents/${AGENT_ID}/credentials/${CREDENTIAL_ID}`;
+
+  const MOCK_VAULT_CREDENTIAL_ROW: ICredentialRow = {
+    ...MOCK_CREDENTIAL,
+    secretHash: '',
+    vaultPath: VAULT_PATH,
+  };
+
+  beforeEach(() => {
+    jest.clearAllMocks();
+    credentialRepo = new MockCredentialRepo({} as never) as jest.Mocked<CredentialRepository>;
+    agentRepo = new MockAgentRepo({} as never) as jest.Mocked<AgentRepository>;
+    auditService = new MockAuditService({} as never) as jest.Mocked<AuditService>;
+    vaultClient = new MockVaultClient('http://localhost:8200', 'token') as jest.Mocked<VaultClient>;
+    service = new CredentialService(credentialRepo, agentRepo, auditService, vaultClient);
+    auditService.logEvent.mockResolvedValue({} as never);
+  });
+
+  describe('generateCredential() with Vault', () => {
+    it('writes secret to Vault and stores the vault_path in the DB', async () => {
+      agentRepo.findById.mockResolvedValue(MOCK_AGENT);
+      vaultClient.writeSecret.mockResolvedValue(VAULT_PATH);
+      credentialRepo.createWithVaultPath.mockResolvedValue(MOCK_CREDENTIAL);
+
+      const result = await service.generateCredential(AGENT_ID, {}, IP, UA);
+
+      expect(vaultClient.writeSecret).toHaveBeenCalledWith(
+        AGENT_ID,
+        expect.any(String),
+        expect.any(String),
+      );
+      expect(credentialRepo.createWithVaultPath).toHaveBeenCalled();
+      expect(credentialRepo.create).not.toHaveBeenCalled();
+      expect(result.clientSecret).toMatch(/^sk_live_[0-9a-f]{64}$/);
+    });
+  });
+
+  describe('rotateCredential() with Vault', () => {
+    it('writes new Vault version and updates vault_path in the DB', async () => {
+      agentRepo.findById.mockResolvedValue(MOCK_AGENT);
+      credentialRepo.findById.mockResolvedValue(MOCK_VAULT_CREDENTIAL_ROW);
+      vaultClient.writeSecret.mockResolvedValue(VAULT_PATH);
+      credentialRepo.updateVaultPath.mockResolvedValue(MOCK_CREDENTIAL);
+
+      const result = await service.rotateCredential(AGENT_ID, CREDENTIAL_ID, {}, IP, UA);
+
+      expect(vaultClient.writeSecret).toHaveBeenCalledWith(
+        AGENT_ID,
+        CREDENTIAL_ID,
+        expect.any(String),
+      );
+      expect(credentialRepo.updateVaultPath).toHaveBeenCalled();
+      expect(credentialRepo.updateHash).not.toHaveBeenCalled();
+      expect(result.clientSecret).toMatch(/^sk_live_[0-9a-f]{64}$/);
+    });
+  });
+
+  describe('revokeCredential() with Vault', () => {
+    it('revokes DB record and deletes Vault secret', async () => {
+      agentRepo.findById.mockResolvedValue(MOCK_AGENT);
+      credentialRepo.findById.mockResolvedValue(MOCK_VAULT_CREDENTIAL_ROW);
+      credentialRepo.revoke.mockResolvedValue({ ...MOCK_CREDENTIAL, status: 'revoked', revokedAt: new Date() });
+      vaultClient.deleteSecret.mockResolvedValue();
+
+      await service.revokeCredential(AGENT_ID, CREDENTIAL_ID, IP, UA);
+
+      expect(credentialRepo.revoke).toHaveBeenCalledWith(CREDENTIAL_ID);
+      expect(vaultClient.deleteSecret).toHaveBeenCalledWith(AGENT_ID, CREDENTIAL_ID);
+    });
+
+    it('does not call Vault delete when credential has no vault_path (bcrypt credential)', async () => {
+      agentRepo.findById.mockResolvedValue(MOCK_AGENT);
+      credentialRepo.findById.mockResolvedValue(MOCK_CREDENTIAL_ROW); // vaultPath: null
+      credentialRepo.revoke.mockResolvedValue({ ...MOCK_CREDENTIAL, status: 'revoked', revokedAt: new Date() });
+
+      await service.revokeCredential(AGENT_ID, CREDENTIAL_ID, IP, UA);
+
+      expect(vaultClient.deleteSecret).not.toHaveBeenCalled();
+    });
+  });
+});
--- a/tests/unit/vault/VaultClient.test.ts
+++ b/tests/unit/vault/VaultClient.test.ts
@@ -0,0 +1,206 @@
+/**
+ * Unit tests for VaultClient.
+ * Mocks the node-vault library to avoid real Vault connections.
+ */
+
+import { jest, describe, it, expect, beforeEach } from '@jest/globals';
+import { VaultClient, createVaultClientFromEnv } from '../../../src/vault/VaultClient.js';
+import { CredentialError } from '../../../src/utils/errors.js';
+
+// ─── Mock node-vault ────────────────────────────────────────────────────────
+
+const mockWrite = jest.fn<() => Promise<unknown>>();
+const mockRead = jest.fn<() => Promise<unknown>>();
+const mockDelete = jest.fn<() => Promise<unknown>>();
+
+jest.mock('node-vault', () => {
+  return jest.fn(() => ({
+    write: mockWrite,
+    read: mockRead,
+    delete: mockDelete,
+  }));
+});
+
+// ─── Helpers ────────────────────────────────────────────────────────────────
+
+const AGENT_ID = 'agent-uuid-1234';
+const CRED_ID = 'cred-uuid-5678';
+const PLAIN_SECRET = 'super-secret-value';
+
+function makeClient(): VaultClient {
+  return new VaultClient('http://127.0.0.1:8200', 'test-token', 'secret');
+}
+
+// ─── Tests ──────────────────────────────────────────────────────────────────
+
+describe('VaultClient', () => {
+  beforeEach(() => {
+    jest.clearAllMocks();
+  });
+
+  // ── writeSecret ────────────────────────────────────────────────────────────
+
+  describe('writeSecret', () => {
+    it('writes the secret to the correct KV v2 path and returns the path', async () => {
+      mockWrite.mockResolvedValue({});
+      const client = makeClient();
+      const path = await client.writeSecret(AGENT_ID, CRED_ID, PLAIN_SECRET);
+
+      expect(mockWrite).toHaveBeenCalledWith(
+        `secret/data/agentidp/agents/${AGENT_ID}/credentials/${CRED_ID}`,
+        { data: { clientSecret: PLAIN_SECRET } },
+      );
+      expect(path).toBe(`secret/data/agentidp/agents/${AGENT_ID}/credentials/${CRED_ID}`);
+    });
+
+    it('throws CredentialError when Vault write fails', async () => {
+      mockWrite.mockRejectedValue(new Error('connection refused'));
+      const client = makeClient();
+
+      await expect(client.writeSecret(AGENT_ID, CRED_ID, PLAIN_SECRET))
+        .rejects.toThrow(CredentialError);
+    });
+
+    it('CredentialError on write failure has code VAULT_WRITE_ERROR', async () => {
+      mockWrite.mockRejectedValue(new Error('forbidden'));
+      const client = makeClient();
+
+      await expect(client.writeSecret(AGENT_ID, CRED_ID, PLAIN_SECRET))
+        .rejects.toMatchObject({ code: 'VAULT_WRITE_ERROR' });
+    });
+  });
+
+  // ── readSecret ─────────────────────────────────────────────────────────────
+
+  describe('readSecret', () => {
+    it('reads and returns the stored secret', async () => {
+      mockRead.mockResolvedValue({
+        data: { data: { clientSecret: PLAIN_SECRET }, metadata: {} },
+      });
+      const client = makeClient();
+      const secret = await client.readSecret(AGENT_ID, CRED_ID);
+
+      expect(mockRead).toHaveBeenCalledWith(
+        `secret/data/agentidp/agents/${AGENT_ID}/credentials/${CRED_ID}`,
+      );
+      expect(secret).toBe(PLAIN_SECRET);
+    });
+
+    it('throws CredentialError when secret field is missing', async () => {
+      mockRead.mockResolvedValue({ data: { data: {}, metadata: {} } });
+      const client = makeClient();
+
+      await expect(client.readSecret(AGENT_ID, CRED_ID))
+        .rejects.toMatchObject({ code: 'VAULT_SECRET_MISSING' });
+    });
+
+    it('throws CredentialError when Vault read fails', async () => {
+      mockRead.mockRejectedValue(new Error('404 not found'));
+      const client = makeClient();
+
+      await expect(client.readSecret(AGENT_ID, CRED_ID))
+        .rejects.toMatchObject({ code: 'VAULT_READ_ERROR' });
+    });
+  });
+
+  // ── verifySecret ───────────────────────────────────────────────────────────
+
+  describe('verifySecret', () => {
+    it('returns true when candidate matches stored secret', async () => {
+      mockRead.mockResolvedValue({
+        data: { data: { clientSecret: PLAIN_SECRET }, metadata: {} },
+      });
+      const client = makeClient();
+      const result = await client.verifySecret(AGENT_ID, CRED_ID, PLAIN_SECRET);
+      expect(result).toBe(true);
+    });
+
+    it('returns false when candidate does not match stored secret', async () => {
+      mockRead.mockResolvedValue({
+        data: { data: { clientSecret: PLAIN_SECRET }, metadata: {} },
+      });
+      const client = makeClient();
+      const result = await client.verifySecret(AGENT_ID, CRED_ID, 'wrong-secret');
+      expect(result).toBe(false);
+    });
+
+    it('returns false when Vault read fails (does not throw)', async () => {
+      mockRead.mockRejectedValue(new Error('vault sealed'));
+      const client = makeClient();
+      const result = await client.verifySecret(AGENT_ID, CRED_ID, PLAIN_SECRET);
+      expect(result).toBe(false);
+    });
+
+    it('returns false when lengths differ (constant-time)', async () => {
+      mockRead.mockResolvedValue({
+        data: { data: { clientSecret: PLAIN_SECRET }, metadata: {} },
+      });
+      const client = makeClient();
+      const result = await client.verifySecret(AGENT_ID, CRED_ID, 'short');
+      expect(result).toBe(false);
+    });
+  });
+
+  // ── deleteSecret ───────────────────────────────────────────────────────────
+
+  describe('deleteSecret', () => {
+    it('calls delete on the metadata path', async () => {
+      mockDelete.mockResolvedValue({});
+      const client = makeClient();
+      await client.deleteSecret(AGENT_ID, CRED_ID);
+
+      expect(mockDelete).toHaveBeenCalledWith(
+        `secret/metadata/agentidp/agents/${AGENT_ID}/credentials/${CRED_ID}`,
+      );
+    });
+
+    it('throws CredentialError when Vault delete fails', async () => {
+      mockDelete.mockRejectedValue(new Error('permission denied'));
+      const client = makeClient();
+
+      await expect(client.deleteSecret(AGENT_ID, CRED_ID))
+        .rejects.toMatchObject({ code: 'VAULT_DELETE_ERROR' });
+    });
+  });
+});
+
+// ─── createVaultClientFromEnv ─────────────────────────────────────────────────
+
+describe('createVaultClientFromEnv', () => {
+  const originalEnv = process.env;
+
+  beforeEach(() => {
+    process.env = { ...originalEnv };
+  });
+
+  afterEach(() => {
+    process.env = originalEnv;
+  });
+
+  it('returns null when VAULT_ADDR is not set', () => {
+    delete process.env['VAULT_ADDR'];
+    delete process.env['VAULT_TOKEN'];
+    expect(createVaultClientFromEnv()).toBeNull();
+  });
+
+  it('returns null when VAULT_TOKEN is not set', () => {
+    process.env['VAULT_ADDR'] = 'http://127.0.0.1:8200';
+    delete process.env['VAULT_TOKEN'];
+    expect(createVaultClientFromEnv()).toBeNull();
+  });
+
+  it('returns a VaultClient when both VAULT_ADDR and VAULT_TOKEN are set', () => {
+    process.env['VAULT_ADDR'] = 'http://127.0.0.1:8200';
+    process.env['VAULT_TOKEN'] = 'test-token';
+    const client = createVaultClientFromEnv();
+    expect(client).toBeInstanceOf(VaultClient);
+  });
+
+  it('uses default mount "secret" when VAULT_MOUNT is not set', () => {
+    process.env['VAULT_ADDR'] = 'http://127.0.0.1:8200';
+    process.env['VAULT_TOKEN'] = 'test-token';
+    delete process.env['VAULT_MOUNT'];
+    // VaultClient instance created — mount is internal, just verify no throw
+    expect(() => createVaultClientFromEnv()).not.toThrow();
+  });
+});