Skip to content

Commit b27b85c

Browse files
EMaherCopilot
andauthored
fix: prevent publish abort on auto-generated logger-credential named values (#220)
* fix: prevent publish abort on auto-generated logger-credential named values The publish pre-flight redaction guard scanned every named value, including APIM auto-generated logger credentials (24-char hex IDs, e.g. Logger-Credentials--<hex>) whose secret values are redacted on extract. Because splitNamedValues() already skips these during publish (APIM recreates them when the referencing logger is published), the guard flagged markers in artifacts that are never sent to APIM and aborted the publish with a false positive. Make the guard mirror the publisher: skip auto-generated named values that have no explicit override. Extract hasNamedValueOverride() into override-merger.ts as the single source of truth shared by both call sites. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
1 parent cc46109 commit b27b85c

4 files changed

Lines changed: 75 additions & 13 deletions

File tree

src/services/override-merger.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,18 @@ const GRANDCHILD_OVERRIDE_MAP: Partial<Record<ResourceType, {
5858
[ResourceType.ApiOperationPolicy]: { parentSection: 'apis', childKey: 'operations', grandchildKey: 'policies' },
5959
};
6060

61+
/**
62+
* Check whether a named value has an explicit override entry.
63+
* Uses case-insensitive matching to align with override-merger behavior.
64+
*/
65+
export function hasNamedValueOverride(name: string, overrides?: OverrideConfig): boolean {
66+
if (!overrides?.namedValues) return false;
67+
const lowerName = name.toLowerCase();
68+
return Object.keys(overrides.namedValues).some(
69+
(key) => key.toLowerCase() === lowerName
70+
);
71+
}
72+
6173
/**
6274
* Apply environment overrides from OverrideConfig to a resource JSON payload.
6375
* Deep-merges matching override properties using case-insensitive key matching.

src/services/publish-service.ts

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import { generateDryRunReport, DryRunReport } from './dry-run-reporter.js';
2727
import { computeDeleteActions } from './delete-unmatched-service.js';
2828
import { computeGitDiff } from './git-diff-service.js';
2929
import { scanForRedactionMarkers } from './secret-redaction-guard.js';
30+
import { hasNamedValueOverride } from './override-merger.js';
3031
import { REDACTION_MARKER } from './secret-redactor.js';
3132

3233
/**
@@ -446,18 +447,6 @@ function splitNamedValues(
446447
return { namedValues, otherTier1 };
447448
}
448449

449-
/**
450-
* Check whether a named value has an explicit override entry.
451-
* Uses case-insensitive matching to align with override-merger behavior.
452-
*/
453-
function hasNamedValueOverride(name: string, overrides?: OverrideConfig): boolean {
454-
if (!overrides?.namedValues) return false;
455-
const lowerName = name.toLowerCase();
456-
return Object.keys(overrides.namedValues).some(
457-
(key) => key.toLowerCase() === lowerName
458-
);
459-
}
460-
461450
/**
462451
* Separates Backend descriptors that are pool backends (properties.type === "Pool")
463452
* from all other descriptors in the supplied list.

src/services/secret-redaction-guard.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,12 @@ import type { IArtifactStore } from '../clients/iartifact-store.js';
2222
import type { PublishConfig } from '../models/config.js';
2323
import type { ResourceDescriptor } from '../models/types.js';
2424
import { ResourceType } from '../models/resource-types.js';
25-
import { applyOverrides } from './override-merger.js';
25+
import { applyOverrides, hasNamedValueOverride } from './override-merger.js';
2626
import { POLICY_TYPES } from './resource-publisher.js';
2727
import { REDACTION_MARKER } from './secret-redactor.js';
2828
import { buildResourceLabel } from '../lib/resource-uri.js';
29+
import { getNamePart } from '../lib/resource-path.js';
30+
import { isAutoGeneratedId } from '../lib/auto-generated.js';
2931

3032
/**
3133
* A single artifact that still contains a redaction marker after overrides.
@@ -103,6 +105,16 @@ async function scanNamedValue(
103105
config: PublishConfig,
104106
descriptor: ResourceDescriptor
105107
): Promise<RedactionMarkerFinding | undefined> {
108+
// Auto-generated named values (e.g. Logger-Credentials--<hex>) are skipped
109+
// during publish because APIM recreates them when the referencing resource
110+
// (Logger, etc.) is published. splitNamedValues() drops them unless the user
111+
// supplies an explicit override, so scanning them here would flag markers in
112+
// artifacts that are never sent to APIM. Mirror the publish filter exactly.
113+
const name = getNamePart(descriptor.nameParts, 0);
114+
if (isAutoGeneratedId(name) && !hasNamedValueOverride(name, config.overrides)) {
115+
return undefined;
116+
}
117+
106118
const json = await store.readResource(config.sourceDir, descriptor);
107119
if (!json) {
108120
return undefined;

tests/unit/services/secret-redaction-guard.test.ts

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,55 @@ describe('secret-redaction-guard', () => {
123123
expect(findings).toEqual([]);
124124
});
125125

126+
it('ignores an auto-generated named value (logger credential) with no override', async () => {
127+
const store = createMockStore();
128+
store.readResource.mockResolvedValue({
129+
properties: { secret: true, value: REDACTION_MARKER },
130+
});
131+
132+
const autoGeneratedDescriptor: ResourceDescriptor = {
133+
type: ResourceType.NamedValue,
134+
nameParts: ['6a46928afa1f751d044d805e'],
135+
};
136+
137+
const findings = await scanForRedactionMarkers(store, testConfig, [autoGeneratedDescriptor]);
138+
139+
// APIM recreates these when the referencing logger publishes, so publish
140+
// skips them and the guard must not flag them. It should skip before any read.
141+
expect(findings).toEqual([]);
142+
expect(store.readResource).not.toHaveBeenCalled();
143+
});
144+
145+
it('scans an auto-generated named value when an explicit override exists', async () => {
146+
const store = createMockStore();
147+
store.readResource.mockResolvedValue({
148+
properties: { secret: true, value: REDACTION_MARKER },
149+
});
150+
151+
const autoGeneratedDescriptor: ResourceDescriptor = {
152+
type: ResourceType.NamedValue,
153+
nameParts: ['6a46928afa1f751d044d805e'],
154+
};
155+
156+
// Override targets the auto-generated name but does not replace the value,
157+
// so the marker remains and the resource is published — it must be scanned.
158+
const configWithOverride: PublishConfig = {
159+
...testConfig,
160+
overrides: {
161+
namedValues: {
162+
'6a46928afa1f751d044d805e': { properties: { tags: ['keep'] } },
163+
},
164+
},
165+
};
166+
167+
const findings = await scanForRedactionMarkers(store, configWithOverride, [
168+
autoGeneratedDescriptor,
169+
]);
170+
171+
expect(findings).toHaveLength(1);
172+
expect(findings[0].location).toBe('properties.value');
173+
});
174+
126175
it('does not flag a marker that an override replaces with clean content', async () => {
127176
const store = createMockStore();
128177
store.readContent.mockResolvedValue({

0 commit comments

Comments
 (0)