Skip to content

Commit 108b93f

Browse files
authored
Apply remaining changes
1 parent 70d1d16 commit 108b93f

9 files changed

Lines changed: 342 additions & 18 deletions

src/services/api-extractor.ts

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import { ResourceType, RESOURCE_TYPE_METADATA } from '../models/resource-types.j
1414
import { FilterConfig } from '../models/config.js';
1515
import { shouldIncludeResource } from './filter-service.js';
1616
import { extractResourceType, ExtractedResource } from './resource-extractor.js';
17+
import { redactPolicySecrets, warnPolicySecretRedactions } from './secret-redactor.js';
1718
import { logger } from '../lib/logger.js';
1819
import { buildResourceLabel } from '../lib/resource-uri.js';
1920
import { getNamePart } from '../lib/resource-path.js';
@@ -363,14 +364,16 @@ async function extractApiPolicy(
363364
const policyContent = properties?.value as string | undefined;
364365

365366
if (policyContent) {
367+
const { redactedContent, findings } = redactPolicySecrets(policyContent);
368+
warnPolicySecretRedactions(policyDescriptor, findings);
366369
await store.writeContent(
367370
outputDir,
368371
policyDescriptor,
369-
policyContent,
372+
redactedContent,
370373
'policy'
371374
);
372375
logger.debug(`Extracted ${buildResourceLabel(policyDescriptor)}`);
373-
return policyContent;
376+
return redactedContent;
374377
}
375378

376379
return undefined;
@@ -420,13 +423,15 @@ async function extractApiOperations(
420423
const policyContent = properties?.value as string | undefined;
421424

422425
if (policyContent) {
423-
await store.writeContent(outputDir, opPolicyDescriptor, policyContent, 'policy');
426+
const { redactedContent, findings } = redactPolicySecrets(policyContent);
427+
warnPolicySecretRedactions(opPolicyDescriptor, findings);
428+
await store.writeContent(outputDir, opPolicyDescriptor, redactedContent, 'policy');
424429
operationPolicies.push({
425430
descriptor: opPolicyDescriptor,
426431
json: policyJson,
427432
status: 'success',
428433
});
429-
policies.push(policyContent);
434+
policies.push(redactedContent);
430435
logger.debug(`Extracted ${buildResourceLabel(opPolicyDescriptor)}`);
431436
}
432437
}
@@ -531,13 +536,15 @@ async function extractGraphQLResolvers(
531536
const policyContent = props?.value as string | undefined;
532537

533538
if (policyContent) {
534-
await store.writeContent(outputDir, resolverPolicyDescriptor, policyContent, 'policy');
539+
const { redactedContent, findings } = redactPolicySecrets(policyContent);
540+
warnPolicySecretRedactions(resolverPolicyDescriptor, findings);
541+
await store.writeContent(outputDir, resolverPolicyDescriptor, redactedContent, 'policy');
535542
resolverPolicies.push({
536543
descriptor: resolverPolicyDescriptor,
537544
json: policyJson,
538545
status: 'success',
539546
});
540-
policies.push(policyContent);
547+
policies.push(redactedContent);
541548
logger.debug(`Extracted ${buildResourceLabel(resolverPolicyDescriptor)}`);
542549
}
543550
}

src/services/extract-service.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import { extractWorkspaces, WorkspaceExtractionResult } from './workspace-extrac
2929
import {
3030
findTransitiveDependencies,
3131
} from './transitive-resolver.js';
32+
import { redactPolicySecrets, warnPolicySecretRedactions } from './secret-redactor.js';
3233
import { logger } from '../lib/logger.js';
3334
import { buildResourceLabel } from '../lib/resource-uri.js';
3435
import { EXIT_SUCCESS, EXIT_PARTIAL, EXIT_FATAL } from '../lib/exit-codes.js';
@@ -401,10 +402,12 @@ async function extractServicePolicy(
401402
const policyContent = properties?.value as string | undefined;
402403

403404
if (policyContent) {
404-
await store.writeContent(outputDir, descriptor, policyContent, 'policy');
405+
const { redactedContent, findings } = redactPolicySecrets(policyContent);
406+
warnPolicySecretRedactions(descriptor, findings);
407+
await store.writeContent(outputDir, descriptor, redactedContent, 'policy');
405408
result.totalExtracted++;
406409
result.extractedDescriptors.push(descriptor);
407-
result.collectedPolicies.set('service-policy', policyContent);
410+
result.collectedPolicies.set('service-policy', redactedContent);
408411
logger.info('Extracted service-level policy');
409412
}
410413
}

src/services/product-extractor.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { ApimServiceContext, AssociationEntry, ResourceDescriptor } from '../mod
1111
import { ResourceType, RESOURCE_TYPE_METADATA } from '../models/resource-types.js';
1212
import { FilterConfig } from '../models/config.js';
1313
import { extractResourceName } from './resource-extractor.js';
14+
import { redactPolicySecrets, warnPolicySecretRedactions } from './secret-redactor.js';
1415
import { logger } from '../lib/logger.js';
1516
import { getNamePart } from '../lib/resource-path.js';
1617
import { isWorkspaceScope, extractNameFromLink, extractLinkTarget } from '../lib/workspace-link.js';
@@ -215,9 +216,11 @@ async function extractProductPolicy(
215216
const policyContent = properties?.value as string | undefined;
216217

217218
if (policyContent) {
218-
await store.writeContent(outputDir, policyDescriptor, policyContent, 'policy');
219+
const { redactedContent, findings } = redactPolicySecrets(policyContent);
220+
warnPolicySecretRedactions(policyDescriptor, findings);
221+
await store.writeContent(outputDir, policyDescriptor, redactedContent, 'policy');
219222
logger.debug(`Extracted policy for product "${getNamePart(productDescriptor.nameParts, 0)}"`);
220-
return policyContent;
223+
return redactedContent;
221224
}
222225

223226
return undefined;

src/services/resource-publisher.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import { logger } from '../lib/logger.js';
2121
import { REDACTION_MARKER } from './secret-redactor.js';
2222
import { isLinkAlreadyExistsError } from '../clients/apim-client.js';
2323
import type { OverrideConfig } from '../models/config.js';
24+
import { buildResourceLabel } from '../lib/resource-uri.js';
2425

2526
export interface ResourcePublishResult {
2627
descriptor: ResourceDescriptor;
@@ -416,6 +417,17 @@ async function publishPolicy(
416417
};
417418
}
418419

420+
// Fail-safe guard: extracted policies don't currently carry separate metadata
421+
// indicating prior redaction, so marker detection is a deliberate content
422+
// check to block publishing placeholder secrets.
423+
if (policyContent.content.includes(REDACTION_MARKER)) {
424+
throw new Error(
425+
`Cannot publish ${buildResourceLabel(descriptor)}: policy contains '${REDACTION_MARKER}'. ` +
426+
'Replace inline secrets with named values before publish: ' +
427+
'https://learn.microsoft.com/en-us/azure/api-management/api-management-howto-properties'
428+
);
429+
}
430+
419431
const payload: Record<string, unknown> = {
420432
properties: {
421433
value: policyContent.content,

src/services/secret-redactor.ts

Lines changed: 180 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,31 @@ import { ResourceType } from '../models/resource-types.js';
1010
import { ResourceDescriptor } from '../models/types.js';
1111
import { logger } from '../lib/logger.js';
1212
import { getNamePart } from '../lib/resource-path.js';
13+
import { buildResourceLabel } from '../lib/resource-uri.js';
1314

1415
/** Marker used to replace secret values in extracted artifacts */
1516
export const REDACTION_MARKER = '*** REDACTED ***';
17+
const NAMED_VALUE_REFERENCE_PATTERN = /^\s*\{\{[^{}]+\}\}\s*$/;
18+
// Headers where inline literal values are typically secrets and should be
19+
// redacted when present in policy XML. Extend this allow-list when APIM adds
20+
// new secret-bearing header conventions.
21+
const SECRET_HEADER_NAMES = new Set([
22+
'authorization',
23+
'ocp-apim-subscription-key',
24+
'x-functions-key',
25+
'api-key',
26+
]);
27+
// Query parameters commonly used to carry secrets/tokens in APIM policies.
28+
// Keep focused on secret-bearing names to avoid over-redacting non-secrets.
29+
const SECRET_QUERY_PARAMETER_NAMES = new Set([
30+
'code',
31+
'sig',
32+
'subscription-key',
33+
]);
34+
35+
export interface PolicySecretFinding {
36+
location: string;
37+
}
1638

1739
/**
1840
* Redact secret values from a resource's JSON payload.
@@ -56,3 +78,161 @@ export function redactSecrets(
5678
logger.debug(`Redacted secret value for named value "${getNamePart(descriptor.nameParts, 0)}"`);
5779
return redacted;
5880
}
81+
82+
function isApimNamedValueReference(value: string): boolean {
83+
return NAMED_VALUE_REFERENCE_PATTERN.test(value);
84+
}
85+
86+
function shouldRedactLiteral(value: string): boolean {
87+
const trimmed = value.trim();
88+
if (!trimmed || trimmed === REDACTION_MARKER) {
89+
return false;
90+
}
91+
return !isApimNamedValueReference(trimmed);
92+
}
93+
94+
/**
95+
* Redact inline literal secrets in policy XML content.
96+
*/
97+
export function redactPolicySecrets(
98+
policyContent: string
99+
): { redactedContent: string; findings: PolicySecretFinding[] } {
100+
const findings: PolicySecretFinding[] = [];
101+
const addFinding = (location: string): void => {
102+
findings.push({ location });
103+
};
104+
105+
let redacted = policyContent;
106+
107+
redacted = redacted.replace(/<set-header\b[\s\S]*?<\/set-header>/gi, (setHeaderBlock) => {
108+
const nameMatch = /\bname\s*=\s*["']([^"']+)["']/i.exec(setHeaderBlock);
109+
const headerName = nameMatch?.[1]?.toLowerCase();
110+
if (!headerName || !SECRET_HEADER_NAMES.has(headerName)) {
111+
return setHeaderBlock;
112+
}
113+
114+
return setHeaderBlock.replace(
115+
/(<value\b[^>]*>)([\s\S]*?)(<\/value>)/gi,
116+
(_full, openTag: string, value: string, closeTag: string) => {
117+
if (!shouldRedactLiteral(value)) {
118+
return `${openTag}${value}${closeTag}`;
119+
}
120+
121+
addFinding(`set-header[${headerName}]`);
122+
return `${openTag}${REDACTION_MARKER}${closeTag}`;
123+
}
124+
);
125+
});
126+
127+
redacted = redacted.replace(/<set-query-parameter\b[\s\S]*?<\/set-query-parameter>/gi, (setQueryBlock) => {
128+
const nameMatch = /\bname\s*=\s*["']([^"']+)["']/i.exec(setQueryBlock);
129+
const parameterName = nameMatch?.[1]?.toLowerCase();
130+
if (!parameterName || !SECRET_QUERY_PARAMETER_NAMES.has(parameterName)) {
131+
return setQueryBlock;
132+
}
133+
134+
return setQueryBlock.replace(
135+
/(<value\b[^>]*>)([\s\S]*?)(<\/value>)/gi,
136+
(_full, openTag: string, value: string, closeTag: string) => {
137+
if (!shouldRedactLiteral(value)) {
138+
return `${openTag}${value}${closeTag}`;
139+
}
140+
141+
addFinding(`set-query-parameter[${parameterName}]`);
142+
return `${openTag}${REDACTION_MARKER}${closeTag}`;
143+
}
144+
);
145+
});
146+
147+
redacted = redacted.replace(/<authentication-basic\b[^>]*>/gi, (tag) => {
148+
return tag.replace(/(\bpassword\s*=\s*["'])([^"']*)(["'])/i, (_full, prefix: string, value: string, suffix: string) => {
149+
if (!shouldRedactLiteral(value)) {
150+
return `${prefix}${value}${suffix}`;
151+
}
152+
153+
addFinding('authentication-basic@password');
154+
return `${prefix}${REDACTION_MARKER}${suffix}`;
155+
});
156+
});
157+
158+
redacted = redacted.replace(/<authentication-certificate\b[^>]*>/gi, (tag) => {
159+
return tag.replace(/(\bbody\s*=\s*["'])([^"']*)(["'])/i, (_full, prefix: string, value: string, suffix: string) => {
160+
if (!shouldRedactLiteral(value)) {
161+
return `${prefix}${value}${suffix}`;
162+
}
163+
164+
addFinding('authentication-certificate@body');
165+
return `${prefix}${REDACTION_MARKER}${suffix}`;
166+
});
167+
});
168+
169+
redacted = redacted.replace(/<authentication-certificate\b[\s\S]*?<\/authentication-certificate>/gi, (certificateBlock) => {
170+
return certificateBlock.replace(
171+
/(<certificate\b[^>]*>)([\s\S]*?)(<\/certificate>)/gi,
172+
(_full, openTag: string, value: string, closeTag: string) => {
173+
if (!shouldRedactLiteral(value)) {
174+
return `${openTag}${value}${closeTag}`;
175+
}
176+
177+
addFinding('authentication-certificate/certificate');
178+
return `${openTag}${REDACTION_MARKER}${closeTag}`;
179+
}
180+
);
181+
});
182+
183+
for (const keySection of ['issuer-signing-keys', 'decryption-keys']) {
184+
const sectionRegex = new RegExp(`<${keySection}\\b[\\s\\S]*?<\\/${keySection}>`, 'gi');
185+
redacted = redacted.replace(sectionRegex, (sectionBlock) => {
186+
return sectionBlock.replace(
187+
/(<key\b[^>]*>)([\s\S]*?)(<\/key>)/gi,
188+
(_full, openTag: string, value: string, closeTag: string) => {
189+
if (!shouldRedactLiteral(value)) {
190+
return `${openTag}${value}${closeTag}`;
191+
}
192+
193+
addFinding(`validate-jwt ${keySection}/key`);
194+
return `${openTag}${REDACTION_MARKER}${closeTag}`;
195+
}
196+
);
197+
});
198+
}
199+
200+
// AccountKey/SharedAccessKey fragments are used by storage/service-bus style
201+
// connection strings. App Insights connection strings use InstrumentationKey
202+
// and therefore do not match this pattern (allow-listed by design).
203+
// Value exclusions:
204+
// - ';' stops at the next connection-string key/value delimiter
205+
// - whitespace/newlines avoid over-capturing adjacent text
206+
// - '<' and '"' avoid crossing into XML tags/attributes
207+
redacted = redacted.replace(/(AccountKey|SharedAccessKey)\s*=\s*([^;\r\n<"\s]+)/gi, (_full, key: string, value: string) => {
208+
if (!shouldRedactLiteral(value)) {
209+
return `${key}=${value}`;
210+
}
211+
212+
addFinding(`connection-string[${key}]`);
213+
return `${key}=${REDACTION_MARKER}`;
214+
});
215+
216+
return {
217+
redactedContent: redacted,
218+
findings,
219+
};
220+
}
221+
222+
/**
223+
* Emit warning logs for policy secret redaction findings.
224+
*/
225+
export function warnPolicySecretRedactions(
226+
descriptor: ResourceDescriptor,
227+
findings: PolicySecretFinding[]
228+
): void {
229+
const label = buildResourceLabel(descriptor);
230+
for (const finding of findings) {
231+
logger.warn(
232+
`Found and redacted inline secret in ${label} (${finding.location}). ` +
233+
`Publish will fail while '${REDACTION_MARKER}' remains. ` +
234+
'Update the policy to use a named value: ' +
235+
'https://learn.microsoft.com/en-us/azure/api-management/api-management-howto-properties'
236+
);
237+
}
238+
}

tests/unit/services/api-product-extractor.test.ts

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { ApimServiceContext, ResourceDescriptor } from '../../../src/models/type
1010
import { FilterConfig } from '../../../src/models/config.js';
1111
import { extractApiResources } from '../../../src/services/api-extractor.js';
1212
import { extractProductResources } from '../../../src/services/product-extractor.js';
13+
import { REDACTION_MARKER } from '../../../src/services/secret-redactor.js';
1314

1415
const testContext: ApimServiceContext = {
1516
subscriptionId: 'sub-1',
@@ -261,8 +262,9 @@ describe('api-extractor', () => {
261262
expect(getApiSpecification).toHaveBeenCalledWith(testContext, 'openapi-rest', 'http', 'openapi3');
262263
});
263264

264-
it('should extract API policy and collect content', async () => {
265-
const policyContent = '<policies><inbound><base /></inbound></policies>';
265+
it('should extract API policy and redact inline secrets', async () => {
266+
const policyContent = '<policies><inbound><set-header name="Authorization"><value>AUTH_LITERAL_VALUE</value></set-header></inbound></policies>';
267+
const redactedPolicy = `<policies><inbound><set-header name="Authorization"><value>${REDACTION_MARKER}</value></set-header></inbound></policies>`;
266268
const client = createMockClient({
267269
getResource: vi.fn().mockImplementation(async (_ctx: unknown, desc: ResourceDescriptor) => {
268270
if (desc.type === ResourceType.ApiPolicy) {
@@ -284,7 +286,13 @@ describe('api-extractor', () => {
284286
'/output'
285287
);
286288

287-
expect(result.policies).toContain(policyContent);
289+
expect(result.policies).toContain(redactedPolicy);
290+
expect(store.writeContent).toHaveBeenCalledWith(
291+
'/output',
292+
expect.objectContaining({ type: ResourceType.ApiPolicy, nameParts: ['my-api'] }),
293+
redactedPolicy,
294+
'policy'
295+
);
288296
// Verify the descriptor used the API name, not 'policy'
289297
expect(client.getResource).toHaveBeenCalledWith(
290298
expect.anything(),
@@ -1014,8 +1022,9 @@ describe('product-extractor', () => {
10141022
expect(result.groups).toEqual(['group-1']);
10151023
});
10161024

1017-
it('should extract product policy', async () => {
1018-
const policyContent = '<policies><inbound></inbound></policies>';
1025+
it('should extract product policy and redact inline secrets', async () => {
1026+
const policyContent = '<policies><inbound><set-query-parameter name="code"><value>abc123</value></set-query-parameter></inbound></policies>';
1027+
const redactedPolicy = `<policies><inbound><set-query-parameter name="code"><value>${REDACTION_MARKER}</value></set-query-parameter></inbound></policies>`;
10191028
const client = createMockClient();
10201029
client.listResources = async function* () {};
10211030
client.getResource = vi.fn().mockImplementation(async (_ctx: unknown, desc: ResourceDescriptor) => {
@@ -1032,8 +1041,14 @@ describe('product-extractor', () => {
10321041
'/output'
10331042
);
10341043

1035-
expect(result.policy).toBe(policyContent);
1036-
expect(result.policies).toContain(policyContent);
1044+
expect(result.policy).toBe(redactedPolicy);
1045+
expect(result.policies).toContain(redactedPolicy);
1046+
expect(store.writeContent).toHaveBeenCalledWith(
1047+
'/output',
1048+
expect.objectContaining({ type: ResourceType.ProductPolicy, nameParts: ['starter'] }),
1049+
redactedPolicy,
1050+
'policy'
1051+
);
10371052
});
10381053

10391054
it('should propagate write errors from store.writeContent for product policy', async () => {

0 commit comments

Comments
 (0)