Skip to content

Commit 0a8ab60

Browse files
committed
adding pre-flight check for redaction markers.
1 parent 308f8ce commit 0a8ab60

5 files changed

Lines changed: 430 additions & 9 deletions

File tree

src/services/publish-service.ts

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ import { publishProduct } from './product-publisher.js';
2626
import { generateDryRunReport, DryRunReport } from './dry-run-reporter.js';
2727
import { computeDeleteActions } from './delete-unmatched-service.js';
2828
import { computeGitDiff } from './git-diff-service.js';
29+
import { scanForRedactionMarkers } from './secret-redaction-guard.js';
30+
import { REDACTION_MARKER } from './secret-redactor.js';
2931

3032
/**
3133
* The APIM Backend properties.type value that identifies a pool backend.
@@ -79,6 +81,44 @@ export async function runPublish(
7981
`Publishing ${targetDescriptors.length} resources (dry-run: ${config.dryRun})`
8082
);
8183

84+
// Step 1b: Redaction pre-flight gate.
85+
// Scan every artifact that would be published for leftover redaction markers
86+
// ('*** REDACTED ***'). A single leftover marker aborts the ENTIRE publish
87+
// before any PUT is issued — and also fails dry-run — so a service can never
88+
// be left partially published with placeholder secrets.
89+
const redactionFindings = await scanForRedactionMarkers(
90+
store,
91+
config,
92+
targetDescriptors
93+
);
94+
if (redactionFindings.length > 0) {
95+
logger.error(
96+
`Publish aborted: ${redactionFindings.length} artifact(s) still contain the redaction marker '${REDACTION_MARKER}'. ` +
97+
'Replace inline secrets with named values or KeyVault references before publishing: ' +
98+
'https://learn.microsoft.com/en-us/azure/api-management/api-management-howto-properties'
99+
);
100+
const actions: PublishActionResult[] = redactionFindings.map((finding) => {
101+
logger.error(` - ${finding.label} (${finding.location})`);
102+
return {
103+
descriptor: finding.descriptor,
104+
action: 'noop',
105+
status: 'failed',
106+
error: new Error(
107+
`${finding.label} contains '${REDACTION_MARKER}' in ${finding.location}`
108+
),
109+
};
110+
});
111+
112+
return {
113+
totalPuts: 0,
114+
totalDeletes: 0,
115+
totalErrors: actions.length,
116+
totalSkipped: 0,
117+
exitCode: EXIT_FATAL,
118+
actions,
119+
};
120+
}
121+
82122
// Step 2: Handle dry-run mode
83123
if (config.dryRun) {
84124
const dryRunReport = await generateDryRunReport(

src/services/resource-publisher.ts

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ export interface ResourcePublishResult {
3333
/**
3434
* Policy resource types that have external XML content
3535
*/
36-
const POLICY_TYPES = new Set<ResourceType>([
36+
export const POLICY_TYPES = new Set<ResourceType>([
3737
ResourceType.ServicePolicy,
3838
ResourceType.ProductPolicy,
3939
ResourceType.ApiPolicy,
@@ -420,14 +420,6 @@ async function publishPolicy(
420420
// Fail-safe guard: extracted policies don't currently carry separate metadata
421421
// indicating prior redaction, so marker detection is a deliberate content
422422
// 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-
431423
const payload: Record<string, unknown> = {
432424
properties: {
433425
value: policyContent.content,
@@ -438,6 +430,19 @@ async function publishPolicy(
438430
// Apply overrides (e.g., format: xml) before PUT — matches Toolkit behavior
439431
const mergedPayload = applyOverrides(descriptor, payload, config.overrides);
440432

433+
// Marker check runs AFTER overrides: an override may legitimately replace the
434+
// policy value with clean content, so only the merged (about-to-be-published)
435+
// value is authoritative for redaction detection.
436+
const mergedProps = mergedPayload.properties as Record<string, unknown> | undefined;
437+
const mergedValue = mergedProps?.value;
438+
if (typeof mergedValue === 'string' && mergedValue.includes(REDACTION_MARKER)) {
439+
throw new Error(
440+
`Cannot publish ${buildResourceLabel(descriptor)}: policy contains '${REDACTION_MARKER}'. ` +
441+
'Replace inline secrets with named values before publish: ' +
442+
'https://learn.microsoft.com/en-us/azure/api-management/api-management-howto-properties'
443+
);
444+
}
445+
441446
await client.putResource(context, descriptor, mergedPayload);
442447

443448
return {
Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT license.
3+
/**
4+
* Secret redaction pre-flight guard
5+
*
6+
* Scans the set of artifacts that are about to be published for leftover
7+
* redaction markers ('*** REDACTED ***'). Any finding aborts the entire
8+
* publish before a single PUT is issued (for both real and dry-run modes),
9+
* so a partially-published service can never result from placeholder secrets.
10+
*
11+
* Detection mirrors the per-resource publish guards:
12+
* - Policies: build the publish payload, apply overrides, then check the
13+
* merged `properties.value` for the marker (an override may legitimately
14+
* replace the value with clean content).
15+
* - Named values: read the resource, apply overrides; KeyVault-backed values
16+
* have their `value` stripped before publish and are therefore ignored;
17+
* otherwise a `secret === true` value that exactly equals the marker is a
18+
* finding.
19+
*/
20+
21+
import type { IArtifactStore } from '../clients/iartifact-store.js';
22+
import type { PublishConfig } from '../models/config.js';
23+
import type { ResourceDescriptor } from '../models/types.js';
24+
import { ResourceType } from '../models/resource-types.js';
25+
import { applyOverrides } from './override-merger.js';
26+
import { POLICY_TYPES } from './resource-publisher.js';
27+
import { REDACTION_MARKER } from './secret-redactor.js';
28+
import { buildResourceLabel } from '../lib/resource-uri.js';
29+
30+
/**
31+
* A single artifact that still contains a redaction marker after overrides.
32+
*/
33+
export interface RedactionMarkerFinding {
34+
/** The descriptor for the offending artifact. */
35+
descriptor: ResourceDescriptor;
36+
/** Human-readable resource label (e.g. `apis/echo/policies/policy`). */
37+
label: string;
38+
/** Where the marker was found (e.g. `policy.xml`, `properties.value`). */
39+
location: string;
40+
}
41+
42+
/**
43+
* Scan the supplied descriptors for leftover redaction markers in the content
44+
* that would actually be published (i.e. after overrides are applied).
45+
*
46+
* Returns every finding so the caller can report all offenders at once rather
47+
* than failing on the first one.
48+
*/
49+
export async function scanForRedactionMarkers(
50+
store: IArtifactStore,
51+
config: PublishConfig,
52+
descriptors: ResourceDescriptor[]
53+
): Promise<RedactionMarkerFinding[]> {
54+
const findings: RedactionMarkerFinding[] = [];
55+
56+
for (const descriptor of descriptors) {
57+
if (POLICY_TYPES.has(descriptor.type)) {
58+
const finding = await scanPolicy(store, config, descriptor);
59+
if (finding) findings.push(finding);
60+
} else if (descriptor.type === ResourceType.NamedValue) {
61+
const finding = await scanNamedValue(store, config, descriptor);
62+
if (finding) findings.push(finding);
63+
}
64+
}
65+
66+
return findings;
67+
}
68+
69+
async function scanPolicy(
70+
store: IArtifactStore,
71+
config: PublishConfig,
72+
descriptor: ResourceDescriptor
73+
): Promise<RedactionMarkerFinding | undefined> {
74+
const policyContent = await store.readContent(config.sourceDir, descriptor, 'policy');
75+
if (!policyContent) {
76+
return undefined;
77+
}
78+
79+
const payload: Record<string, unknown> = {
80+
properties: {
81+
value: policyContent.content,
82+
format: 'rawxml',
83+
},
84+
};
85+
86+
const merged = applyOverrides(descriptor, payload, config.overrides);
87+
const mergedProps = merged.properties as Record<string, unknown> | undefined;
88+
const mergedValue = mergedProps?.value;
89+
90+
if (typeof mergedValue === 'string' && mergedValue.includes(REDACTION_MARKER)) {
91+
return {
92+
descriptor,
93+
label: buildResourceLabel(descriptor),
94+
location: 'policy.xml',
95+
};
96+
}
97+
98+
return undefined;
99+
}
100+
101+
async function scanNamedValue(
102+
store: IArtifactStore,
103+
config: PublishConfig,
104+
descriptor: ResourceDescriptor
105+
): Promise<RedactionMarkerFinding | undefined> {
106+
const json = await store.readResource(config.sourceDir, descriptor);
107+
if (!json) {
108+
return undefined;
109+
}
110+
111+
const merged = applyOverrides(descriptor, json, config.overrides);
112+
const props = merged.properties as Record<string, unknown> | undefined;
113+
114+
// KeyVault-backed named values have `properties.value` stripped before publish,
115+
// so any marker still present in the file is never sent to APIM.
116+
if (props?.keyVault != null) {
117+
return undefined;
118+
}
119+
120+
if (props?.secret === true && props.value === REDACTION_MARKER) {
121+
return {
122+
descriptor,
123+
label: buildResourceLabel(descriptor),
124+
location: 'properties.value',
125+
};
126+
}
127+
128+
return undefined;
129+
}

tests/unit/services/publish-service.test.ts

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -343,6 +343,69 @@ describe('publish-service', () => {
343343
expect(client.putResource).not.toHaveBeenCalled();
344344
});
345345

346+
it('should abort the entire publish before any PUT when a redaction marker is found', async () => {
347+
const resources = [
348+
{ type: ResourceType.NamedValue, nameParts: ['nv-secret'] },
349+
{ type: ResourceType.Api, nameParts: ['api1'] },
350+
];
351+
352+
const client = createMockClient();
353+
const store = createMockStore(resources);
354+
vi.mocked(store.readResource).mockImplementation(
355+
async (_sourceDir: string, descriptor: ResourceDescriptor) => {
356+
if (descriptor.type === ResourceType.NamedValue) {
357+
return { name: 'nv-secret', properties: { secret: true, value: '*** REDACTED ***' } };
358+
}
359+
return { name: descriptor.nameParts[0] ?? '', properties: {} };
360+
}
361+
);
362+
363+
const config: PublishConfig = {
364+
service: testContext,
365+
sourceDir: '/source',
366+
dryRun: false,
367+
deleteUnmatched: false,
368+
logLevel: LogLevel.INFO,
369+
};
370+
371+
const result = await runPublish(client, store, config);
372+
373+
expect(result.exitCode).toBe(2);
374+
expect(result.totalErrors).toBe(1);
375+
expect(result.totalPuts).toBe(0);
376+
expect(client.putResource).not.toHaveBeenCalled();
377+
expect(publishApi).not.toHaveBeenCalled();
378+
});
379+
380+
it('should fail dry-run when a redaction marker is found and not generate a report', async () => {
381+
const resources = [
382+
{ type: ResourceType.NamedValue, nameParts: ['nv-secret'] },
383+
];
384+
385+
const client = createMockClient();
386+
const store = createMockStore(resources);
387+
vi.mocked(store.readResource).mockResolvedValue({
388+
name: 'nv-secret',
389+
properties: { secret: true, value: '*** REDACTED ***' },
390+
});
391+
392+
const config: PublishConfig = {
393+
service: testContext,
394+
sourceDir: '/source',
395+
dryRun: true,
396+
deleteUnmatched: false,
397+
logLevel: LogLevel.INFO,
398+
};
399+
400+
const result = await runPublish(client, store, config);
401+
402+
expect(result.exitCode).toBe(2);
403+
expect(result.totalErrors).toBe(1);
404+
expect(result.dryRunReport).toBeUndefined();
405+
expect(generateDryRunReport).not.toHaveBeenCalled();
406+
expect(client.putResource).not.toHaveBeenCalled();
407+
});
408+
346409
it('should use computeGitDiff when commitId is set (incremental mode)', async () => {
347410
const client = createMockClient();
348411
const store = createMockStore([]);

0 commit comments

Comments
 (0)