From 208d2f29ebc72b1ee7547194653e5e7a9f0cbc8f Mon Sep 17 00:00:00 2001 From: bluzername Date: Sat, 5 Sep 2026 07:17:04 +0300 Subject: [PATCH] Do not let patch object override the id in updateApiKey The client build the request like this: { id, ...patch }. If someone pass a patch variable that also have an id field inside (this can happen because TypeScript only check extra fields on an object literal, not on a variable), the spread come after and it win. So the request can go to the wrong key instead of the one you ask for. Now it build the object as { ...patch, id } so the id argument always stay the real target, and patch can only touch name, expiresAt and disabled. Added a test that create two keys and try to sneak the second key id inside the patch of the first one. Before the fix this test fail, now it pass. --- .changeset/thin-crews-shout.md | 5 +++++ packages/sdk/src/client.test.ts | 24 ++++++++++++++++++++++++ packages/sdk/src/client.ts | 6 +++++- 3 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 .changeset/thin-crews-shout.md diff --git a/.changeset/thin-crews-shout.md b/.changeset/thin-crews-shout.md new file mode 100644 index 0000000..98ebfe4 --- /dev/null +++ b/.changeset/thin-crews-shout.md @@ -0,0 +1,5 @@ +--- +"@dormice/sdk": patch +--- + +Fix `updateApiKey` so a patch object can no longer override the target id. Before this, `{ id, ...patch }` let an `id` field inside `patch` win over the id you pass as the first argument, so the request could edit the wrong key. diff --git a/packages/sdk/src/client.test.ts b/packages/sdk/src/client.test.ts index 33f5674..660a22b 100644 --- a/packages/sdk/src/client.test.ts +++ b/packages/sdk/src/client.test.ts @@ -421,6 +421,30 @@ describe('API keys over real HTTP', () => { await client.revokeApiKey(apiKey.id); }); + it('updateApiKey ignores an id smuggled inside the patch object', async () => { + const { apiKey: intended } = await client.createApiKey('sdk-target'); + const { apiKey: victim } = await client.createApiKey('sdk-victim'); + + // A patch variable can carry an `id` field through structural typing + // (TypeScript excess-property checks only catch object literals, not + // variables). The wire request must still target the id passed as the + // method's own argument, not one hiding inside the patch. + const patch = { id: victim.id, disabled: true } as { disabled: boolean }; + const result = await client.updateApiKey(intended.id, patch); + + expect(result.apiKey.id).toBe(intended.id); + expect(result.apiKey.disabledAt).not.toBeNull(); + + const allKeys = await client.listApiKeys(); + const refreshedIntended = allKeys.find((key) => key.id === intended.id); + const refreshedVictim = allKeys.find((key) => key.id === victim.id); + expect(refreshedIntended?.disabledAt).not.toBeNull(); + expect(refreshedVictim?.disabledAt).toBeNull(); + + await client.revokeApiKey(intended.id); + await client.revokeApiKey(victim.id); + }); + it('a ledger key gets the honest 403 on the management verbs', async () => { const { apiKey, token } = await client.createApiKey('sdk-not-admin'); const keyed = new Dormice({ endpoint, token }); diff --git a/packages/sdk/src/client.ts b/packages/sdk/src/client.ts index b2011d0..ea611c6 100644 --- a/packages/sdk/src/client.ts +++ b/packages/sdk/src/client.ts @@ -589,7 +589,11 @@ export class Dormice { id: string, patch: { name?: string; expiresAt?: string | null; disabled?: boolean }, ): Promise { - const data = await this.rpc('updateApiKey', { id, ...patch }); + // Spread patch first so its fields can never clobber the id we were + // explicitly asked to target (a patch variable can carry an `id` of + // its own through structural typing, since excess-property checks + // only apply to object literals). + const data = await this.rpc('updateApiKey', { ...patch, id }); return updateApiKeyResponseSchema.parse(data); }