Skip to content
Merged
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
30 changes: 30 additions & 0 deletions .squad/agents/testengineer/history.md
Original file line number Diff line number Diff line change
Expand Up @@ -79,4 +79,34 @@

**Result:** 42 total tests in resource-uri.test.ts (32 existing + 13 new for buildResourceLabel, minus 3 restructured). All tests pass. Function behavior validated for all documented use cases.

### 2026-04-29: User-Agent Header Testing (Issue #16)

**Context:** Added test coverage for User-Agent header implementation across both lib and client layers.

**Tests Created:**
- `tests/unit/lib/user-agent.test.ts` - 3 tests
- ✅ USER_AGENT constant exports as string
- ✅ USER_AGENT matches format `apiops-cli/{version}`
- ✅ Version in USER_AGENT matches package.json version

- `tests/unit/clients/apim-client.test.ts` - 2 new tests added to "User-Agent" describe block
- ✅ User-Agent header set on authenticated requests (Bearer token path)
- ✅ User-Agent header set on unauthenticated requests (skipAuth blob path)

**Testing Approach:**
- Verified header presence in both auth paths using standard mock setup
- Both tests confirm header is set after auth logic via `headers.set()`
- Used existing test patterns: mock fetch with Response objects, inspect headers in captured context

**Code Review Feedback:**
- One finding: duplicate test replaced with skipAuth blob path test to avoid redundancy
- Ensures both auth flows are covered without test duplication

**Pattern:** When testing client-wide headers:
- Add lib unit test for constant/value verification
- Add client integration tests for both supported request patterns
- Verify header appears in expected request headers captured by mocks

**Result:** 5 new User-Agent tests, all passing. Code review approved.

<!-- Append new learnings here after each session -->
27 changes: 27 additions & 0 deletions .squad/agents/typescriptdev/history.md
Original file line number Diff line number Diff line change
Expand Up @@ -160,3 +160,30 @@ vi.mocked(fs.access).mockImplementation(async (p) => {
Same rule applies to `expect(fs.copyFile).toHaveBeenCalledWith(...)` assertions — use the resolved form.

**Files:** `tests/unit/services/init-service.test.ts`.

### 2026-04-29: User-Agent Header Implementation (Issue #16)

**Context:** Implemented User-Agent header for all APIM REST API calls to identify the apiops-cli client.

**Key Decisions:**
- Created `src/lib/user-agent.ts` with `USER_AGENT` constant using module pattern: `createRequire` loads package.json at module initialization
- Format: `apiops-cli/{version}` (e.g., `apiops-cli/0.1.0`)
- Header set in `ApimClient.request()` at line 108, after auth logic but before retry loop
- Applied universally to all request types: authenticated (Bearer token) and unauthenticated (SAS blob skipAuth paths)

**Pattern:** When exporting client identifiers:
- Use ES module pattern with `createRequire(import.meta.url)` to load package.json at import time
- No runtime file system calls or dynamic version reads
- Export as constant string from dedicated lib module
- Consumed by clients that need the value

**Files Modified:**
- `src/lib/user-agent.ts` - New file
- `src/clients/apim-client.ts` - Added header set at line 108

**Implementation Details:**
- `headers.set('User-Agent', USER_AGENT)` executes after skipping auth headers but maintaining compatibility
- Tested on both auth paths: standard Bearer token and skipAuth (SAS blob)
- Issue #16 closed

**Tests:** Created `tests/unit/lib/user-agent.test.ts` (3 tests) and added to `apim-client.test.ts` (2 tests)
30 changes: 0 additions & 30 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions src/clients/apim-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { RESOURCE_TYPE_METADATA, ResourceType } from '../models/resource-types.j
import { buildArmUri, buildResourceLabel } from '../lib/resource-uri.js';
import { deriveListPaths } from '../lib/resource-path.js';
import { logger } from '../lib/logger.js';
import { USER_AGENT } from '../lib/user-agent.js';

/**
* Structured HTTP error that carries the response status code.
Expand Down Expand Up @@ -104,6 +105,7 @@ export class ApimClient implements IApimClient {
headers.delete('Proxy-Authorization');
headers.delete('x-ms-authorization-auxiliary');
}
headers.set('User-Agent', USER_AGENT);

Comment thread
EMaher marked this conversation as resolved.
let attempt = 0;
// For SAS blob URLs the query string contains the sig token — strip it before logging.
Expand Down
6 changes: 6 additions & 0 deletions src/lib/user-agent.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { createRequire } from 'module';

const require = createRequire(import.meta.url);
const pkg = require('../../package.json') as { version: string };

export const USER_AGENT = `apiops-cli/${pkg.version}`;
12 changes: 6 additions & 6 deletions src/services/keyvault-checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

import { DefaultAzureCredential } from '@azure/identity';
import { logger } from '../lib/logger.js';
import { USER_AGENT } from '../lib/user-agent.js';

/* ------------------------------------------------------------------ */
/* ARM API versions */
Expand Down Expand Up @@ -88,12 +89,11 @@ function defaultTokenProviderFactory(): TokenProvider {
}

async function defaultArmRequest(url: string, token: string): Promise<ArmResponse> {
const response = await fetch(url, {
headers: {
Authorization: `Bearer ${token}`,
Accept: 'application/json',
},
});
const headers = new Headers();
headers.set('Authorization', `Bearer ${token}`);
headers.set('Accept', 'application/json');
headers.set('User-Agent', USER_AGENT);
const response = await fetch(url, { headers });
return {
status: response.status,
json: () => response.json() as Promise<unknown>,
Expand Down
58 changes: 58 additions & 0 deletions tests/unit/clients/apim-client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1126,3 +1126,61 @@ describe('ApimClient.getApiSpecification', () => {
expect(fetchSpy.mock.calls[1][0]).toBe(exportResponse.link);
});
});

describe('User-Agent header', () => {
let client: ApimClient;
let fetchSpy: ReturnType<typeof vi.fn>;

beforeEach(() => {
client = new ApimClient();
fetchSpy = vi.fn();
vi.stubGlobal('fetch', fetchSpy);

// eslint-disable-next-line @typescript-eslint/no-explicit-any
vi.spyOn(client as any, 'getToken').mockResolvedValue('fake-token');
});

afterEach(() => {
vi.restoreAllMocks();
vi.unstubAllGlobals();
});

it('should include User-Agent header on authenticated requests', async () => {
fetchSpy.mockResolvedValueOnce(
makeResponse(200, {
value: [{ name: 'gw-1' }],
})
);

const results: unknown[] = [];
for await (const item of client.listResources(testContext, ResourceType.Gateway)) {
results.push(item);
}

expect(fetchSpy).toHaveBeenCalledTimes(1);
const [_url, init] = fetchSpy.mock.calls[0] as [string, RequestInit];
const headers = new Headers(init?.headers);
expect(headers.get('User-Agent')).toMatch(/^apiops-cli\/\d+\.\d+\.\d+/);
});

it('should include User-Agent header on unauthenticated (skipAuth) blob requests', async () => {
// First fetch: authenticated APIM call returns an openapi-link SAS URL
fetchSpy.mockResolvedValueOnce(
makeResponse(200, { link: 'https://example.blob.core.windows.net/spec.json?sig=abc' })
);
// Second fetch: unauthenticated blob download (skipAuth=true)
fetchSpy.mockResolvedValueOnce(
new Response('openapi: 3.0.0', { status: 200, headers: { 'Content-Type': 'text/plain' } })
);

await client.getApiSpecification(testContext, 'test-api');

// The second call is the skipAuth blob fetch — verify it still carries User-Agent
expect(fetchSpy).toHaveBeenCalledTimes(2);
const [_blobUrl, blobInit] = fetchSpy.mock.calls[1] as [string, RequestInit];
const blobHeaders = new Headers(blobInit?.headers);
expect(blobHeaders.get('User-Agent')).toMatch(/^apiops-cli\/\d+\.\d+\.\d+/);
// Authorization must NOT be set on the unauthenticated call
expect(blobHeaders.get('Authorization')).toBeNull();
});
});
20 changes: 20 additions & 0 deletions tests/unit/lib/user-agent.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/**
* Unit tests for USER_AGENT constant
*/

import { describe, it, expect } from 'vitest';
import { USER_AGENT } from '../../../src/lib/user-agent.js';

describe('USER_AGENT', () => {
it('should be a string', () => {
expect(typeof USER_AGENT).toBe('string');
});

it('should start with "apiops-cli/"', () => {
expect(USER_AGENT).toMatch(/^apiops-cli\//);
});

it('should contain a semver-like version', () => {
expect(USER_AGENT).toMatch(/\d+\.\d+\.\d+/);
});
});
61 changes: 60 additions & 1 deletion tests/unit/services/keyvault-checker.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@
* Unit tests for keyvault-checker service (ARM-based approach)
*/

import { describe, it, expect, vi, beforeEach } from 'vitest';
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
import {
checkKeyVaultSecretAccess,
type TokenProviderFactory,
type KeyVaultCheckContext,
} from '../../../src/services/keyvault-checker.js';
import { USER_AGENT } from '../../../src/lib/user-agent.js';

describe('checkKeyVaultSecretAccess', () => {
let mockGetToken: ReturnType<typeof vi.fn>;
Expand Down Expand Up @@ -573,3 +574,61 @@ describe('checkKeyVaultSecretAccess', () => {
).resolves.toBeUndefined();
});
});

describe('defaultArmRequest User-Agent header', () => {
let fetchSpy: ReturnType<typeof vi.fn>;

beforeEach(() => {
fetchSpy = vi.fn();
vi.stubGlobal('fetch', fetchSpy);
});

afterEach(() => {
vi.restoreAllMocks();
vi.unstubAllGlobals();
});

it('should include User-Agent header when defaultArmRequest is used', async () => {
// Arrange: mock all ARM responses so checkKeyVaultSecretAccess runs to completion
const apimIdentity = {
identity: { type: 'SystemAssigned', principalId: 'pid-1' },
};
const vaultList = { value: [{ id: '/subscriptions/sub-1/resourceGroups/rg-1/providers/Microsoft.KeyVault/vaults/myvault' }] };
const vaultDetail = {
properties: {
enableRbacAuthorization: false,
accessPolicies: [
{
objectId: 'pid-1',
permissions: { secrets: ['get'] },
},
],
},
};

fetchSpy
.mockResolvedValueOnce(new Response(JSON.stringify(apimIdentity), { status: 200, headers: { 'Content-Type': 'application/json' } }))
.mockResolvedValueOnce(new Response(JSON.stringify(vaultList), { status: 200, headers: { 'Content-Type': 'application/json' } }))
.mockResolvedValueOnce(new Response(JSON.stringify(vaultDetail), { status: 200, headers: { 'Content-Type': 'application/json' } }));

const mockTokenProviderFactory: TokenProviderFactory = () => ({
getToken: vi.fn().mockResolvedValue({ token: 'fake-arm-token' }),
});

await checkKeyVaultSecretAccess(
'https://myvault.vault.azure.net/secrets/my-secret',
undefined,
{ subscriptionId: 'sub-1', resourceGroup: 'rg-1', serviceName: 'apim-1' },
mockTokenProviderFactory,
// No armRequest override — uses defaultArmRequest which must include User-Agent
);

// All three fetch calls should include the User-Agent header
expect(fetchSpy.mock.calls.length).toBeGreaterThan(0);
for (const call of fetchSpy.mock.calls) {
const [_url, init] = call as [string, RequestInit];
const headers = new Headers(init?.headers);
expect(headers.get('User-Agent')).toBe(USER_AGENT);
}
});
});
Loading