Skip to content

Commit ccc32a1

Browse files
committed
fixed the buildkey collision minor possibility
1 parent abb1a03 commit ccc32a1

3 files changed

Lines changed: 37 additions & 12 deletions

File tree

graphile/graphile-multi-tenancy-cache/README.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ process.on('SIGTERM', async () => {
100100
| Concept | Meaning |
101101
|--------|---------|
102102
| `svc_key` | Request routing key. Used to look up which cached handler the current request should hit. |
103-
| `buildKey` | Handler identity. Computed from the inputs that materially affect Graphile instance construction. |
103+
| `buildKey` | Handler identity. A canonical string computed from the inputs that materially affect Graphile instance construction. |
104104
| `databaseId` | Metadata/flush key. Used to evict all handlers associated with a database. |
105105

106106
### What goes into the buildKey
@@ -119,6 +119,8 @@ It does **not** include:
119119
- request host/domain
120120
- auth tokens or transient headers
121121

122+
The value is stored as a canonical plain-text key rather than a truncated hash, so different build inputs cannot collide onto the same handler key.
123+
122124
Schema order is preserved. `['a', 'b']` and `['b', 'a']` intentionally produce different buildKeys.
123125

124126
## How the handler cache works

graphile/graphile-multi-tenancy-cache/src/__tests__/buildkey.test.ts

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,6 @@
1010
* - shutdown clears all state
1111
*/
1212

13-
import { createHash } from 'node:crypto';
14-
1513
// We test computeBuildKey directly and use mocks for the orchestrator functions
1614
// that depend on PostGraphile.
1715

@@ -51,10 +49,17 @@ describe('computeBuildKey', () => {
5149
expect(k1).toBe(k2);
5250
});
5351

54-
it('should produce a 16-char hex string', () => {
52+
it('should produce a canonical JSON string', () => {
5553
const pool = makeMockPool();
5654
const key = computeBuildKey(pool, ['public'], 'anon', 'authenticated');
57-
expect(key).toMatch(/^[0-9a-f]{16}$/);
55+
expect(key).toBe(
56+
JSON.stringify({
57+
conn: 'localhost:5432/testdb@postgres',
58+
schemas: ['public'],
59+
anonRole: 'anon',
60+
roleName: 'authenticated',
61+
}),
62+
);
5863
});
5964

6065
it('should differ when schemas differ', () => {
@@ -651,7 +656,14 @@ describe('connectionString-based pool identity', () => {
651656
// Some consumers might create pools with explicit fields instead of connectionString
652657
const pool = { options: { host: 'myhost', port: 5432, database: 'mydb', user: 'myuser' } } as unknown as import('pg').Pool;
653658
const key = computeBuildKey(pool, ['public'], 'anon', 'auth');
654-
expect(key).toMatch(/^[0-9a-f]{16}$/);
659+
expect(key).toBe(
660+
JSON.stringify({
661+
conn: 'myhost:5432/mydb@myuser',
662+
schemas: ['public'],
663+
anonRole: 'anon',
664+
roleName: 'auth',
665+
}),
666+
);
655667
});
656668
});
657669

graphile/graphile-multi-tenancy-cache/src/multi-tenancy-cache.ts

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
/**
22
* Multi-tenancy cache orchestrator.
33
*
4-
* Caches one independent PostGraphile handler per **buildKey** (derived from
5-
* the inputs that materially affect Graphile handler construction).
4+
* Caches one independent PostGraphile handler per **buildKey** (a canonical
5+
* string derived from the inputs that materially affect Graphile handler
6+
* construction).
67
*
78
* Multiple svc_key values with identical build inputs share the same handler.
89
* svc_key remains the request routing key and flush targeting key.
@@ -15,7 +16,6 @@
1516
* databaseIdToBuildKeys: databaseId → Set<buildKey>
1617
*/
1718

18-
import { createHash } from 'node:crypto';
1919
import { createServer } from 'node:http';
2020
import { Logger } from '@pgpmjs/logger';
2121
import express from 'express';
@@ -54,6 +54,13 @@ export interface MultiTenancyCacheStats {
5454
inflightCreations: number;
5555
}
5656

57+
interface BuildKeyParts {
58+
conn: string;
59+
schemas: string[];
60+
anonRole: string;
61+
roleName: string;
62+
}
63+
5764
// --- Internal state ---
5865

5966
/** buildKey → TenantInstance (the real handler cache) */
@@ -163,20 +170,24 @@ function getPoolIdentity(pool: Pool): string {
163170
* - svc_key (routing-only)
164171
* - databaseId (metadata-only)
165172
* - token data, host/domain, transient headers
173+
*
174+
* The buildKey is intentionally stored as a canonical plain-text string
175+
* rather than a truncated hash so there is no collision risk between
176+
* different tenant build inputs.
166177
*/
167178
export function computeBuildKey(
168179
pool: Pool,
169180
schemas: string[],
170181
anonRole: string,
171182
roleName: string,
172183
): string {
173-
const input = JSON.stringify({
184+
const input: BuildKeyParts = {
174185
conn: getPoolIdentity(pool),
175186
schemas,
176187
anonRole,
177188
roleName,
178-
});
179-
return createHash('sha256').update(input).digest('hex').slice(0, 16);
189+
};
190+
return JSON.stringify(input);
180191
}
181192

182193
// --- Index management ---

0 commit comments

Comments
 (0)