Skip to content

Commit 6270086

Browse files
fix: unbounded queries, findFirst determinism, sequence retry, PII encryption guard (#205, #206, #207, #208) (#369)
- #205: findByVendor and findByBuyer now accept optional cursor/take params and apply orderBy:createdAt desc — prevents unbounded heap allocation on large result sets; callers that omit pagination get the first 20 records - #206: findByVendorAndItem adds orderBy:createdAt asc for deterministic results; schema.prisma replaces @@index([vendorAddress, itemRef]) with @@unique to enforce at the DB layer that duplicate item refs per vendor are rejected - #207: StellarServer interface gains loadAccount(); submitAutoRelease now accepts sourceAddress and calls loadAccount before every attempt so each retry builds a fresh transaction with the current sequence number — fixes the root cause of tx_bad_seq retry loops; AUTO_RELEASE_SOURCE_ADDRESS env var wires the source address from the worker - #208: assertEncryptedContact() guard added to PrismaService escrow.create and escrow.update — throws if buyerContactEmail or buyerContactPhone is non-null and does not match the AES-256-GCM iv:tag:ciphertext hex format; SECURITY.md documents the encryption scheme, key rotation procedure, and regulatory context
1 parent 1c4f42e commit 6270086

8 files changed

Lines changed: 280 additions & 14 deletions

File tree

SECURITY.md

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,47 @@ Out of scope:
6666

6767
## Security Controls
6868

69+
### Buyer Contact Encryption (PII at rest)
70+
71+
`buyerContactEmail` and `buyerContactPhone` are classified as Personal Identifiable Information (PII) and **must never be written to the database in plaintext**.
72+
73+
#### Encryption scheme
74+
75+
| Property | Value |
76+
|----------|-------|
77+
| Algorithm | AES-256-GCM |
78+
| Key size | 256 bits (32 bytes) |
79+
| IV size | 96 bits (12 bytes), randomly generated per call |
80+
| Auth tag | 128 bits (16 bytes) |
81+
| Storage format | `<iv_hex>:<auth_tag_hex>:<ciphertext_hex>` (colon-separated hex) |
82+
| Key source | `CONTACT_ENCRYPTION_KEY` env variable (64 hex chars = 32 bytes) |
83+
84+
Encrypting the same plaintext twice produces different ciphertext — each call generates a fresh IV, preventing correlation attacks.
85+
86+
#### Code path
87+
88+
```
89+
EscrowController → EscrowService.updateBuyerContact()
90+
→ encryptContact() [contact-encryption.util.ts]
91+
→ EscrowRepository.saveBuyerContact()
92+
→ PrismaService.escrow.update() [validates ciphertext format before write]
93+
```
94+
95+
#### Defence-in-depth guard
96+
97+
`PrismaService.escrow.create` and `escrow.update` both call `assertEncryptedContact()` before any write. If either field is provided without matching the expected `iv:tag:ciphertext` hex format, an exception is thrown and the write is aborted. This makes plaintext storage impossible by construction even if a new code path bypasses `encryptContact()`.
98+
99+
#### Key rotation
100+
101+
1. Generate a new 32-byte key: `openssl rand -hex 32`
102+
2. Re-encrypt all non-null `buyerContactEmail`/`buyerContactPhone` rows using the new key.
103+
3. Update `CONTACT_ENCRYPTION_KEY` in the secret store and restart the service.
104+
4. Verify no plaintext remains by checking that all stored values match the `iv:tag:ciphertext` format.
105+
106+
#### Regulatory context
107+
108+
This control supports compliance with NDPR (Nigeria Data Protection Regulation), GDPR (Article 32 — appropriate technical measures), and similar frameworks requiring encryption of personal data at rest.
109+
69110
### Environment Variables
70111

71112
| Variable | Requirement |
@@ -74,6 +115,8 @@ Out of scope:
74115
| `ADMIN_ADDRESS` | Valid Stellar public key (G...) |
75116
| `DATABASE_URL` | TLS/SSL required in production |
76117
| `STELLAR_WEBHOOK_SECRET` | Required in production for webhook HMAC verification |
118+
| `CONTACT_ENCRYPTION_KEY` | Exactly 64 hex characters (32 bytes); rotate annually or on suspected compromise |
119+
| `AUTO_RELEASE_SOURCE_ADDRESS` | Stellar public key of the auto-release signing account |
77120

78121
Never commit `.env` files. Use environment-specific secret management (Vault, AWS Secrets Manager, etc.).
79122

prisma/schema.prisma

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ model Escrow {
5353
@@index([vendorAddress])
5454
@@index([buyerAddress])
5555
@@index([vendorAddress, state])
56-
@@index([vendorAddress, itemRef])
56+
@@unique([vendorAddress, itemRef])
5757
@@index([state, trackingId])
5858
@@index([state, createdAt])
5959
@@index([state, deliveredAt])

src/escrow/escrow.repository.spec.ts

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
import { EscrowRepository } from './escrow.repository';
22
import { PrismaService } from '../prisma/prisma.service';
3+
import { encryptContact } from '../common/sanitization/contact-encryption.util';
4+
5+
// Required by the encryption util
6+
process.env.CONTACT_ENCRYPTION_KEY = 'a'.repeat(64);
37

48
function makeDto() {
59
return {
@@ -56,6 +60,103 @@ describe('EscrowRepository', () => {
5660
});
5761
});
5862

63+
// ── #205: cursor-based pagination ─────────────────────────────────────────
64+
describe('findByVendor() — pagination (#205)', () => {
65+
beforeEach(async () => {
66+
await repo.create({ ...makeDto(), itemRef: 'A' }, 'v-page');
67+
await repo.create({ ...makeDto(), itemRef: 'B' }, 'v-page');
68+
await repo.create({ ...makeDto(), itemRef: 'C' }, 'v-page');
69+
});
70+
71+
it('returns up to `take` records for the first page', async () => {
72+
const results = await repo.findByVendor('v-page', undefined, 2);
73+
expect(results).toHaveLength(2);
74+
});
75+
76+
it('returns remaining records after a cursor', async () => {
77+
const first = await repo.findByVendor('v-page', undefined, 2);
78+
const second = await repo.findByVendor('v-page', first[first.length - 1].id, 10);
79+
expect(second.length).toBeGreaterThanOrEqual(1);
80+
expect(second.map((e) => e.id)).not.toContain(first[0].id);
81+
});
82+
83+
it('returns an empty array when no more records exist after cursor', async () => {
84+
const all = await repo.findByVendor('v-page', undefined, 100);
85+
const last = all[all.length - 1];
86+
const next = await repo.findByVendor('v-page', last.id, 10);
87+
expect(next).toHaveLength(0);
88+
});
89+
});
90+
91+
describe('findByBuyer() — pagination (#205)', () => {
92+
beforeEach(async () => {
93+
await repo.create({ ...makeDto(), itemRef: 'P', buyerAddress: 'b-page' }, 'v1');
94+
await repo.create({ ...makeDto(), itemRef: 'Q', buyerAddress: 'b-page' }, 'v1');
95+
});
96+
97+
it('returns up to `take` records', async () => {
98+
const results = await repo.findByBuyer('b-page', undefined, 1);
99+
expect(results).toHaveLength(1);
100+
});
101+
102+
it('uses default take of 20 when not specified', async () => {
103+
const results = await repo.findByBuyer('b-page');
104+
expect(results.length).toBeLessThanOrEqual(20);
105+
});
106+
});
107+
108+
// ── #206: findFirst instead of findMany + index ────────────────────────────
109+
describe('findByVendorAndItem() — findFirst determinism (#206)', () => {
110+
it('returns the earliest record when multiple share the same (vendorAddress, itemRef)', async () => {
111+
const first = await repo.create({ ...makeDto(), itemRef: 'DUP' }, 'v-dup');
112+
await repo.create({ ...makeDto(), itemRef: 'DUP' }, 'v-dup');
113+
const found = await repo.findByVendorAndItem('v-dup', 'DUP');
114+
expect(found?.id).toBe(first.id);
115+
});
116+
});
117+
118+
// ── #208: plaintext buyer contact rejected by prisma guard ────────────────
119+
describe('saveBuyerContact() — encryption guard (#208)', () => {
120+
it('stores encrypted contact without throwing', async () => {
121+
const escrow = await repo.create(makeDto(), 'v-enc');
122+
const encEmail = encryptContact('test@example.com');
123+
const encPhone = encryptContact('+2348001234567');
124+
await expect(
125+
repo.saveBuyerContact(escrow.id, encEmail, encPhone),
126+
).resolves.toBeDefined();
127+
});
128+
129+
it('throws when plaintext email is passed directly to the repository', async () => {
130+
const escrow = await repo.create(makeDto(), 'v-enc2');
131+
await expect(
132+
repo.saveBuyerContact(escrow.id, 'plaintext@example.com', null),
133+
).rejects.toThrow(/Security violation.*buyerContactEmail/);
134+
});
135+
136+
it('throws when plaintext phone is passed directly to the repository', async () => {
137+
const escrow = await repo.create(makeDto(), 'v-enc3');
138+
await expect(
139+
repo.saveBuyerContact(escrow.id, null, '+2348001234567'),
140+
).rejects.toThrow(/Security violation.*buyerContactPhone/);
141+
});
142+
143+
it('allows null values (contact not provided)', async () => {
144+
const escrow = await repo.create(makeDto(), 'v-enc4');
145+
await expect(
146+
repo.saveBuyerContact(escrow.id, null, null),
147+
).resolves.toBeDefined();
148+
});
149+
150+
it('stored value differs from plaintext input', async () => {
151+
const escrow = await repo.create(makeDto(), 'v-enc5');
152+
const plain = 'secret@test.com';
153+
const enc = encryptContact(plain);
154+
const updated = await repo.saveBuyerContact(escrow.id, enc, null);
155+
expect(updated.buyerContactEmail).not.toBe(plain);
156+
expect(updated.buyerContactEmail).toBe(enc);
157+
});
158+
});
159+
59160
describe('findVendorEscrows()', () => {
60161
beforeEach(async () => {
61162
await repo.create({ ...makeDto(), amount: 300, itemRef: 'A' }, 'v1');

src/escrow/escrow.repository.ts

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -44,13 +44,16 @@ export class EscrowRepository {
4444
/**
4545
* Finds the first escrow matching both vendorAddress and itemRef,
4646
* used to detect duplicate submissions for the same item reference.
47+
* Uses findFirst (LIMIT 1) rather than findMany so only one row is
48+
* loaded; orderBy makes the result deterministic when duplicates exist.
4749
*/
4850
findByVendorAndItem(
4951
vendorAddress: string,
5052
itemRef: string,
5153
): Promise<EscrowRecord | null> {
5254
return this.prisma.escrow.findFirst({
5355
where: { vendorAddress, itemRef },
56+
orderBy: { createdAt: 'asc' },
5457
});
5558
}
5659

@@ -67,14 +70,40 @@ export class EscrowRepository {
6770
return record;
6871
}
6972

70-
/** Returns all escrows belonging to the given vendor address. */
71-
findByVendor(vendorAddress: string): Promise<EscrowRecord[]> {
72-
return this.prisma.escrow.findMany({ where: { vendorAddress } });
73+
/**
74+
* Returns a cursor-paginated slice of escrows for the given vendor,
75+
* ordered newest-first. Pass `cursor` (an escrow ID) to get the page
76+
* after that record; omit it for the first page.
77+
*/
78+
findByVendor(
79+
vendorAddress: string,
80+
cursor?: string,
81+
take = 20,
82+
): Promise<EscrowRecord[]> {
83+
return this.prisma.escrow.findMany({
84+
where: { vendorAddress },
85+
orderBy: { createdAt: 'desc' },
86+
take,
87+
...(cursor ? { cursor: { id: cursor }, skip: 1 } : {}),
88+
});
7389
}
7490

75-
/** Returns all escrows belonging to the given buyer address. */
76-
findByBuyer(buyerAddress: string): Promise<EscrowRecord[]> {
77-
return this.prisma.escrow.findMany({ where: { buyerAddress } });
91+
/**
92+
* Returns a cursor-paginated slice of escrows for the given buyer,
93+
* ordered newest-first. Pass `cursor` (an escrow ID) to get the page
94+
* after that record; omit it for the first page.
95+
*/
96+
findByBuyer(
97+
buyerAddress: string,
98+
cursor?: string,
99+
take = 20,
100+
): Promise<EscrowRecord[]> {
101+
return this.prisma.escrow.findMany({
102+
where: { buyerAddress },
103+
orderBy: { createdAt: 'desc' },
104+
take,
105+
...(cursor ? { cursor: { id: cursor }, skip: 1 } : {}),
106+
});
78107
}
79108

80109
/** Updates the escrow state and invalidates its cache entry. */

src/prisma/prisma.service.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,24 @@
11
import { Injectable, Logger, OnModuleDestroy } from '@nestjs/common';
22

3+
// AES-256-GCM ciphertext produced by contact-encryption.util: iv:authTag:ciphertext
4+
// IV = 12 bytes (24 hex), tag = 16 bytes (32 hex), ciphertext = 1+ hex chars.
5+
const ENCRYPTED_CONTACT_RE = /^[0-9a-f]{24}:[0-9a-f]{32}:[0-9a-f]+$/i;
6+
7+
/**
8+
* Guards against plaintext writes to buyer PII fields.
9+
* Throws at the repository layer if a non-null value doesn't match the
10+
* expected AES-256-GCM ciphertext format produced by encryptContact().
11+
*/
12+
function assertEncryptedContact(field: string, value: string | null | undefined): void {
13+
if (value == null) return;
14+
if (!ENCRYPTED_CONTACT_RE.test(value)) {
15+
throw new Error(
16+
`Security violation: ${field} must be encrypted before persistence. ` +
17+
`Use encryptContact() from contact-encryption.util before writing to the database.`,
18+
);
19+
}
20+
}
21+
322
export type EscrowState =
423
| 'CREATED'
524
| 'FUNDED'
@@ -364,6 +383,8 @@ export class PrismaService implements OnModuleDestroy {
364383

365384
escrow = {
366385
create: ({ data }: { data: EscrowCreateInput }): Promise<EscrowRecord> => {
386+
assertEncryptedContact('buyerContactEmail', data.buyerContactEmail);
387+
assertEncryptedContact('buyerContactPhone', data.buyerContactPhone);
367388
const now = new Date();
368389
const escrow: EscrowRecord = {
369390
...data,
@@ -511,6 +532,8 @@ export class PrismaService implements OnModuleDestroy {
511532
where: { id: string };
512533
data: EscrowUpdateInput;
513534
}): Promise<EscrowRecord> => {
535+
assertEncryptedContact('buyerContactEmail', data.buyerContactEmail);
536+
assertEncryptedContact('buyerContactPhone', data.buyerContactPhone);
514537
const existing = this.escrows.get(where.id);
515538
if (!existing) {
516539
throw new Error(`Escrow ${where.id} not found`);

src/stellar/contract.service.ts

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import { STELLAR_SERVER } from './stellar.tokens';
44
import { DEFAULT_AUTO_RELEASE_MAX_RETRIES } from './contract.constants';
55

66
interface StellarServer {
7+
/** Loads the current account state (including sequence number) from Horizon. */
8+
loadAccount(sourceAddress: string): Promise<{ sequence: string }>;
79
submitTransaction(transaction: Record<string, unknown>): Promise<{
810
hash?: string;
911
status?: string;
@@ -41,9 +43,20 @@ export class ContractService {
4143
return result.hash;
4244
}
4345

44-
/** Submits an auto-release transaction, retrying sequence errors up to the limit. */
46+
/**
47+
* Submits an auto-release transaction, retrying on Stellar sequence errors.
48+
*
49+
* On each attempt the source account is re-fetched from Horizon so the
50+
* transaction is rebuilt with the current sequence number — a stale sequence
51+
* embedded in the XDR is the root cause of every retry failure on tx_bad_seq.
52+
*
53+
* @param escrowId Trust-Link escrow identifier embedded in the contract call.
54+
* @param sourceAddress Stellar address of the auto-release signing account.
55+
* @param maxRetries Maximum number of additional attempts after the first failure.
56+
*/
4557
async submitAutoRelease(
4658
escrowId: string,
59+
sourceAddress: string,
4760
maxRetries = DEFAULT_AUTO_RELEASE_MAX_RETRIES,
4861
): Promise<string> {
4962
if (!this.server) {
@@ -52,10 +65,17 @@ export class ContractService {
5265

5366
let attempt = 0;
5467
while (attempt <= maxRetries) {
68+
// Re-fetch the account before every attempt so the transaction is built
69+
// with the current sequence number. Without this, a sequence error on
70+
// attempt N causes attempt N+1 to replay the same stale XDR and fail again.
71+
const account = await this.server.loadAccount(sourceAddress);
72+
5573
try {
5674
const result = await this.server.submitTransaction({
5775
operation: 'autoRelease',
5876
escrowId,
77+
sourceAddress,
78+
sequence: account.sequence,
5979
});
6080

6181
if (result.status === 'ERROR' || result.resultXdr === 'TxFailed') {

src/workers/auto-release.worker.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,10 @@ import { ContractService } from '../stellar/contract.service';
1010

1111
const EVERY_5_MINUTES = 5 * 60 * 1000;
1212

13+
// Stellar address of the auto-release signing account. Must be set in production.
14+
const AUTO_RELEASE_SOURCE =
15+
process.env.AUTO_RELEASE_SOURCE_ADDRESS ?? 'GAUTORELEASE000000000000000000000000000000000000000000000';
16+
1317
@Injectable()
1418
export class AutoReleaseWorker implements OnModuleInit, OnApplicationShutdown {
1519
private readonly logger = new Logger(AutoReleaseWorker.name);
@@ -56,6 +60,7 @@ export class AutoReleaseWorker implements OnModuleInit, OnApplicationShutdown {
5660

5761
const txHash = await this.contractService.submitAutoRelease(
5862
escrow.id,
63+
AUTO_RELEASE_SOURCE,
5964
);
6065
await this.escrowRepository.markAutoReleaseCompleted(
6166
escrow.id,

0 commit comments

Comments
 (0)