Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/thin-crews-shout.md
Original file line number Diff line number Diff line change
@@ -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.
24 changes: 24 additions & 0 deletions packages/sdk/src/client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
Expand Down
6 changes: 5 additions & 1 deletion packages/sdk/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -589,7 +589,11 @@ export class Dormice {
id: string,
patch: { name?: string; expiresAt?: string | null; disabled?: boolean },
): Promise<UpdateApiKeyResponse> {
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);
}

Expand Down