Skip to content

Commit 6d0782f

Browse files
Copilotpetehauge
andauthored
fix: add User-Agent header to defaultArmRequest in keyvault-checker.ts
Addresses PR review feedback: the production defaultArmRequest fetch call in keyvault-checker.ts was missing the User-Agent header. Now imports USER_AGENT from user-agent.ts and adds it alongside Authorization/Accept. Adds a unit test that calls checkKeyVaultSecretAccess without an injected armRequest (so defaultArmRequest runs) and verifies every fetch call carries the correct User-Agent header. Closes #16 Agent-Logs-Url: https://github.com/Azure/apiops-cli/sessions/8b196bc2-81c8-4320-9a60-c3655ee4b3bc Co-authored-by: petehauge <12090616+petehauge@users.noreply.github.com>
1 parent bcbf46e commit 6d0782f

2 files changed

Lines changed: 63 additions & 1 deletion

File tree

‎src/services/keyvault-checker.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
import { DefaultAzureCredential } from '@azure/identity';
1717
import { logger } from '../lib/logger.js';
18+
import { USER_AGENT } from '../lib/user-agent.js';
1819

1920
/* ------------------------------------------------------------------ */
2021
/* ARM API versions */
@@ -92,6 +93,7 @@ async function defaultArmRequest(url: string, token: string): Promise<ArmRespons
9293
headers: {
9394
Authorization: `Bearer ${token}`,
9495
Accept: 'application/json',
96+
'User-Agent': USER_AGENT,
9597
},
9698
});
9799
return {

‎tests/unit/services/keyvault-checker.test.ts‎

Lines changed: 61 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,13 @@
22
* Unit tests for keyvault-checker service (ARM-based approach)
33
*/
44

5-
import { describe, it, expect, vi, beforeEach } from 'vitest';
5+
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
66
import {
77
checkKeyVaultSecretAccess,
88
type TokenProviderFactory,
99
type KeyVaultCheckContext,
1010
} from '../../../src/services/keyvault-checker.js';
11+
import { USER_AGENT } from '../../../src/lib/user-agent.js';
1112

1213
describe('checkKeyVaultSecretAccess', () => {
1314
let mockGetToken: ReturnType<typeof vi.fn>;
@@ -573,3 +574,62 @@ describe('checkKeyVaultSecretAccess', () => {
573574
).resolves.toBeUndefined();
574575
});
575576
});
577+
578+
describe('defaultArmRequest User-Agent header', () => {
579+
let fetchSpy: ReturnType<typeof vi.fn>;
580+
581+
beforeEach(() => {
582+
fetchSpy = vi.fn();
583+
vi.stubGlobal('fetch', fetchSpy);
584+
});
585+
586+
afterEach(() => {
587+
vi.restoreAllMocks();
588+
vi.unstubAllGlobals();
589+
});
590+
591+
it('should include User-Agent header when defaultArmRequest is used', async () => {
592+
// Arrange: mock all ARM responses so checkKeyVaultSecretAccess runs to completion
593+
const apimIdentity = {
594+
identity: { type: 'SystemAssigned', principalId: 'pid-1' },
595+
};
596+
const vaultList = { value: [{ id: '/subscriptions/sub-1/resourceGroups/rg-1/providers/Microsoft.KeyVault/vaults/myvault' }] };
597+
const vaultDetail = {
598+
properties: {
599+
enableRbacAuthorization: false,
600+
accessPolicies: [
601+
{
602+
objectId: 'pid-1',
603+
permissions: { secrets: ['get'] },
604+
},
605+
],
606+
},
607+
};
608+
609+
fetchSpy
610+
.mockResolvedValueOnce(new Response(JSON.stringify(apimIdentity), { status: 200, headers: { 'Content-Type': 'application/json' } }))
611+
.mockResolvedValueOnce(new Response(JSON.stringify(vaultList), { status: 200, headers: { 'Content-Type': 'application/json' } }))
612+
.mockResolvedValueOnce(new Response(JSON.stringify(vaultDetail), { status: 200, headers: { 'Content-Type': 'application/json' } }));
613+
614+
const mockTokenProviderFactory: TokenProviderFactory = () => ({
615+
getToken: vi.fn().mockResolvedValue({ token: 'fake-arm-token' }),
616+
});
617+
618+
await checkKeyVaultSecretAccess(
619+
'https://myvault.vault.azure.net/secrets/my-secret',
620+
undefined,
621+
{ subscriptionId: 'sub-1', resourceGroup: 'rg-1', serviceName: 'apim-1' },
622+
mockTokenProviderFactory,
623+
// No armRequest override — uses defaultArmRequest which must include User-Agent
624+
);
625+
626+
// All three fetch calls should include the User-Agent header
627+
expect(fetchSpy.mock.calls.length).toBeGreaterThan(0);
628+
for (const call of fetchSpy.mock.calls) {
629+
const [_url, init] = call as [string, RequestInit];
630+
const headers = new Headers(init?.headers);
631+
expect(headers.get('User-Agent')).toBe(USER_AGENT);
632+
expect(headers.get('User-Agent')).toMatch(/^apiops-cli\/\d+\.\d+\.\d+/);
633+
}
634+
});
635+
});

0 commit comments

Comments
 (0)