From 31f53621fe47076e8fd1dd9aa9f72a28daf66c48 Mon Sep 17 00:00:00 2001 From: Chris Dayne Date: Mon, 21 Sep 2026 12:44:11 +1000 Subject: [PATCH 1/5] feat: support XML policy fragment artifacts Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- docs/reference/apim-glossary.md | 2 +- docs/reference/artifact-format.md | 17 ++- docs/reference/resource-types.md | 2 +- specs/data-model.md | 2 +- specs/research.md | 4 +- specs/v1-research-report.md | 5 +- src/lib/resource-path.ts | 16 +++ src/services/policy-fragment-artifact.ts | 94 ++++++++++++++ src/services/publish-service.ts | 24 ++++ src/services/resource-extractor.ts | 34 ++++-- src/services/resource-publisher.ts | 45 ++++--- src/services/secret-redaction-guard.ts | 32 +++-- src/services/transitive-extractor.ts | 15 ++- src/services/transitive-resolver.ts | 9 +- .../expected-structure.json | 4 +- tests/unit/clients/artifact-store.test.ts | 15 +++ tests/unit/lib/resource-path.test.ts | 34 ++++++ tests/unit/services/git-diff-service.test.ts | 38 ++++++ .../services/policy-fragment-artifact.test.ts | 115 ++++++++++++++++++ tests/unit/services/publish-service.test.ts | 33 +++++ .../unit/services/resource-extractor.test.ts | 40 ++++++ .../unit/services/resource-publisher.test.ts | 84 +++++++++++++ .../services/secret-redaction-guard.test.ts | 21 ++++ .../unit/services/transitive-resolver.test.ts | 22 ++++ 24 files changed, 657 insertions(+), 50 deletions(-) create mode 100644 src/services/policy-fragment-artifact.ts create mode 100644 tests/unit/services/policy-fragment-artifact.test.ts diff --git a/docs/reference/apim-glossary.md b/docs/reference/apim-glossary.md index ce06fa6d..25a8ca0f 100644 --- a/docs/reference/apim-glossary.md +++ b/docs/reference/apim-glossary.md @@ -76,7 +76,7 @@ XML-based middleware that runs on API requests and responses. Policies handle ra A reusable snippet of policy XML that can be included in other policies via ``. Useful for shared logic like standard rate limiting or CORS headers. - **Microsoft Docs:** [Policy fragments](https://learn.microsoft.com/en-us/azure/api-management/policy-fragments) -- **In artifacts:** `policyFragments/{name}/policyFragmentInformation.json` +- **In artifacts:** `policyFragments/{name}/policy.xml`, with optional metadata in `policyFragmentInformation.json` --- diff --git a/docs/reference/artifact-format.md b/docs/reference/artifact-format.md index 1f229dd4..feac1e12 100644 --- a/docs/reference/artifact-format.md +++ b/docs/reference/artifact-format.md @@ -67,7 +67,8 @@ apim-artifacts/ │ └── tagInformation.json ├── policyFragments/ │ └── rate-limit/ -│ └── policyFragmentInformation.json +│ ├── policyFragmentInformation.json # Optional metadata +│ └── policy.xml ├── loggers/ │ └── appinsights/ │ └── loggerInformation.json @@ -116,7 +117,7 @@ All 34 APIM resource types and their artifact mappings: | Logger | `loggers/{name}` | `loggerInformation.json` | | Group | `groups/{name}` | `groupInformation.json` | | Diagnostic | `diagnostics/{name}` | `diagnosticInformation.json` | -| PolicyFragment | `policyFragments/{name}` | `policyFragmentInformation.json` | +| PolicyFragment | `policyFragments/{name}` | `policy.xml` and optional `policyFragmentInformation.json` | | ServicePolicy | _(root directory)_ | `policy.xml` | | Product | `products/{name}` | `productInformation.json` | | Api | `apis/{name}` | `apiInformation.json` | @@ -125,6 +126,18 @@ All 34 APIM resource types and their artifact mappings: | PolicyRestriction | `policyRestrictions/{name}` | `policyRestrictionInformation.json` | | Documentation | `documentations/{name}` | `documentationInformation.json` | +Policy fragments follow the Azure APIops split policy layout. The following +representations are supported: + +- `policy.xml` only, for fragments without metadata. +- `policy.xml` with `policyFragmentInformation.json`, where JSON metadata is + merged with the XML content. +- A legacy `policyFragmentInformation.json` containing `properties.value`. + +When both files exist, `policy.xml` supplies `properties.value` and +`properties.format` (`rawxml`), while other JSON properties such as +`description` are retained. + ### Product Child Resources | Resource Type | Artifact Directory | Info File | diff --git a/docs/reference/resource-types.md b/docs/reference/resource-types.md index 37798158..a84afecc 100644 --- a/docs/reference/resource-types.md +++ b/docs/reference/resource-types.md @@ -29,7 +29,7 @@ These resources exist at the APIM service scope — they are not children of any | Logger | `/loggers/{name}` | `loggers/{0}` | `loggerInformation.json` | Logging destinations (Application Insights, Event Hub) | | Group | `/groups/{name}` | `groups/{0}` | `groupInformation.json` | User groups for access control | | Diagnostic | `/diagnostics/{name}` | `diagnostics/{0}` | `diagnosticInformation.json` | Logging/diagnostic settings (references a Logger) | -| PolicyFragment | `/policyFragments/{name}` | `policyFragments/{0}` | `policyFragmentInformation.json` | Reusable policy XML snippets | +| PolicyFragment | `/policyFragments/{name}` | `policyFragments/{0}` | `policy.xml` and optional `policyFragmentInformation.json` | Reusable policy XML snippets | | ServicePolicy | `/policies/policy` | *(root)* | `policy.xml` | Global policy applied to all APIs | | GlobalSchema | `/schemas/{name}` | `schemas/{0}` | `schemaInformation.json` | Service-level schemas (shared across APIs) | | PolicyRestriction | `/policyRestrictions/{name}` | `policyRestrictions/{0}` | `policyRestrictionInformation.json` | Rules restricting which policies can be used | diff --git a/specs/data-model.md b/specs/data-model.md index 0ed989a7..a1dcf121 100644 --- a/specs/data-model.md +++ b/specs/data-model.md @@ -20,7 +20,7 @@ Defines all APIM resource types the tool handles. | `Logger` | `/loggers/{name}` | `loggers/{name}/` | `loggerInformation.json` | | `Group` | `/groups/{name}` | `groups/{name}/` | `groupInformation.json` | | `Diagnostic` | `/diagnostics/{name}` | `diagnostics/{name}/` | `diagnosticInformation.json` | -| `PolicyFragment` | `/policyFragments/{name}` | `policy fragments/{name}/` | `policyFragmentInformation.json` | +| `PolicyFragment` | `/policyFragments/{name}` | `policyFragments/{name}/` | `policy.xml` and optional `policyFragmentInformation.json` | | `ServicePolicy` | `/policies/policy` | (root) | `policy.xml` | | `Product` | `/products/{name}` | `products/{name}/` | `productInformation.json` | | `ProductPolicy` | `/products/{name}/policies/policy` | `products/{name}/` | `policy.xml` | diff --git a/specs/research.md b/specs/research.md index f5809722..3b8f2792 100644 --- a/specs/research.md +++ b/specs/research.md @@ -107,7 +107,9 @@ ├── backends/{name}/backendInformation.json ├── loggers/{name}/loggerInformation.json ├── diagnostics/{name}/diagnosticInformation.json -├── policyFragments/{name}/policyFragmentInformation.json +├── policyFragments/{name}/ +│ ├── policyFragmentInformation.json # Optional metadata +│ └── policy.xml ├── gateways/{name}/gatewayInformation.json ├── groups/{name}/groupInformation.json ├── subscriptions/{name}/subscriptionInformation.json diff --git a/specs/v1-research-report.md b/specs/v1-research-report.md index 347c5edb..c8efdc84 100644 --- a/specs/v1-research-report.md +++ b/specs/v1-research-report.md @@ -141,7 +141,8 @@ policyFragmentNames: │ └── diagnosticInformation.json ├── policy fragments/ │ └── {name}/ -│ └── policyFragmentInformation.json +│ ├── policyFragmentInformation.json +│ └── policy.xml ├── products/ │ └── {name}/ │ ├── productInformation.json @@ -597,7 +598,7 @@ Users must explicitly list all dependent resources in the filter file, or extrac | Backend | `backendInformation.json` | — | — | | Logger | `loggerInformation.json` | — | — | | Diagnostic | `diagnosticInformation.json` | — | — | -| Policy Fragment | `policyFragmentInformation.json` | — | — | +| Policy Fragment | `policyFragmentInformation.json` | `policy.xml` | — | | Service Policy | `policy.xml` | — | — | | Product | `productInformation.json` | `policy.xml`, `apis.json`, `groups.json` | — | | Group | `groupInformation.json` | — | — | diff --git a/src/lib/resource-path.ts b/src/lib/resource-path.ts index 51fc96d5..3384a877 100644 --- a/src/lib/resource-path.ts +++ b/src/lib/resource-path.ts @@ -442,6 +442,20 @@ export function parseArtifactPath( } } + if (fileName === 'policy.xml') { + const policyFragmentParts = parseTemplatePath( + RESOURCE_TYPE_METADATA[ResourceType.PolicyFragment].artifactDirectory, + parts.slice(startIndex, -1).join('/') + ); + if (policyFragmentParts !== undefined) { + return { + type: ResourceType.PolicyFragment, + nameParts: policyFragmentParts, + workspace, + }; + } + } + // Try to match against each resource type's pattern for (const [typeKey, metadata] of Object.entries(RESOURCE_TYPE_METADATA)) { const type = typeKey as ResourceType; @@ -494,6 +508,8 @@ function parseWorkspaceContainerDescriptor( * files that belong to a resource but are not the primary info file. * * Currently supports: + * - Policy fragment content (`policyFragments/{fragment}/policy.xml`) + * - Workspace-scoped policy fragment content * - API specification files (`apis/{api}/specification.{ext}`) * - Workspace-scoped API specification files * (`workspaces/{workspace}/apis/{api}/specification.{ext}`) diff --git a/src/services/policy-fragment-artifact.ts b/src/services/policy-fragment-artifact.ts new file mode 100644 index 00000000..61bacba9 --- /dev/null +++ b/src/services/policy-fragment-artifact.ts @@ -0,0 +1,94 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +import type { IArtifactStore } from '../clients/iartifact-store.js'; +import type { ResourceDescriptor } from '../models/types.js'; +import { ResourceType } from '../models/resource-types.js'; +import { redactAndWarnPolicySecrets } from './secret-redactor.js'; + +function getProperties( + json: Record | undefined +): Record { + const properties = json?.properties; + return properties !== null && typeof properties === 'object' && !Array.isArray(properties) + ? properties as Record + : {}; +} + +/** + * Read a policy fragment using the Azure APIops artifact contract. + * + * Either policyFragmentInformation.json or policy.xml may represent the + * fragment. When both exist, JSON metadata is retained and policy.xml supplies + * the authoritative value and format. + */ +export async function readPolicyFragmentArtifact( + store: IArtifactStore, + baseDir: string, + descriptor: ResourceDescriptor +): Promise | undefined> { + if (descriptor.type !== ResourceType.PolicyFragment) { + throw new Error(`Expected PolicyFragment descriptor, got ${descriptor.type}`); + } + + const [information, policyContent] = await Promise.all([ + store.readResource(baseDir, descriptor), + store.readContent(baseDir, descriptor, 'policy'), + ]); + + if (!information && !policyContent) { + return undefined; + } + + if (!policyContent) { + return information; + } + + return { + ...(information ?? {}), + properties: { + ...getProperties(information), + value: policyContent.content, + format: 'rawxml', + }, + }; +} + +/** + * Write an extracted policy fragment using the Azure APIops split layout: + * metadata in policyFragmentInformation.json and content in policy.xml. + */ +export async function writePolicyFragmentArtifact( + store: IArtifactStore, + baseDir: string, + descriptor: ResourceDescriptor, + json: Record +): Promise> { + if (descriptor.type !== ResourceType.PolicyFragment) { + throw new Error(`Expected PolicyFragment descriptor, got ${descriptor.type}`); + } + + const properties = getProperties(json); + const { value, format: _format, ...metadataProperties } = properties; + const information = { + ...json, + properties: metadataProperties, + }; + + await store.writeResource(baseDir, descriptor, information); + + if (typeof value !== 'string') { + return json; + } + + const redactedContent = redactAndWarnPolicySecrets(descriptor, value); + await store.writeContent(baseDir, descriptor, redactedContent, 'policy'); + + return { + ...json, + properties: { + ...properties, + value: redactedContent, + }, + }; +} diff --git a/src/services/publish-service.ts b/src/services/publish-service.ts index 517667a9..96ada398 100644 --- a/src/services/publish-service.ts +++ b/src/services/publish-service.ts @@ -265,6 +265,30 @@ async function determinePublishTargets( const diffResult = await computeGitDiff(config.sourceDir, config.commitId); targetDescriptors = diffResult.changedDescriptors; deletedDescriptors = diffResult.deletedDescriptors; + const currentDescriptors = await store.listResources(config.sourceDir); + const currentPolicyFragments = new Set( + currentDescriptors + .filter((descriptor) => descriptor.type === ResourceType.PolicyFragment) + .map(getResourceDescriptorKey) + ); + const targetKeys = new Set(targetDescriptors.map(getResourceDescriptorKey)); + const actualDeletedDescriptors: ResourceDescriptor[] = []; + + for (const descriptor of deletedDescriptors) { + const key = getResourceDescriptorKey(descriptor); + if ( + descriptor.type === ResourceType.PolicyFragment && + currentPolicyFragments.has(key) + ) { + if (!targetKeys.has(key)) { + targetDescriptors.push(descriptor); + targetKeys.add(key); + } + } else { + actualDeletedDescriptors.push(descriptor); + } + } + deletedDescriptors = actualDeletedDescriptors; } else { // Full mode: publish all artifacts logger.debug('Using full publish mode (all artifacts)'); diff --git a/src/services/resource-extractor.ts b/src/services/resource-extractor.ts index c62c8c6a..e0c73e4a 100644 --- a/src/services/resource-extractor.ts +++ b/src/services/resource-extractor.ts @@ -15,6 +15,7 @@ import { shouldIncludeResource } from './filter-service.js'; import { FilterConfig } from '../models/config.js'; import { logger } from '../lib/logger.js'; import { buildResourceLabel } from '../lib/resource-uri.js'; +import { writePolicyFragmentArtifact } from './policy-fragment-artifact.js'; /** * Check if a resource type's LIST endpoint returns shallow data that omits @@ -121,10 +122,21 @@ export async function extractResourceType( } // Apply secret redaction - const safeJson = redactSecrets(descriptor, json); - - // Write to artifact store (preserves opaque JSON per FR-009) - await store.writeResource(outputDir, descriptor, safeJson); + let safeJson = redactSecrets(descriptor, json); + + // Policy fragments follow the Toolkit split layout: metadata remains + // JSON while the policy value is stored as sibling policy.xml. + if (descriptor.type === ResourceType.PolicyFragment) { + safeJson = await writePolicyFragmentArtifact( + store, + outputDir, + descriptor, + safeJson + ); + } else { + // Write to artifact store (preserves opaque JSON per FR-009) + await store.writeResource(outputDir, descriptor, safeJson); + } result.extracted.push({ descriptor, @@ -181,10 +193,18 @@ export async function extractSingleResource( } // Apply secret redaction - const safeJson = redactSecrets(descriptor, json); + let safeJson = redactSecrets(descriptor, json); - // Write to artifact store - await store.writeResource(outputDir, descriptor, safeJson); + if (descriptor.type === ResourceType.PolicyFragment) { + safeJson = await writePolicyFragmentArtifact( + store, + outputDir, + descriptor, + safeJson + ); + } else { + await store.writeResource(outputDir, descriptor, safeJson); + } logger.info(`Extracted ${buildResourceLabel(descriptor)}`); diff --git a/src/services/resource-publisher.ts b/src/services/resource-publisher.ts index efd39fe2..9f5e9abb 100644 --- a/src/services/resource-publisher.ts +++ b/src/services/resource-publisher.ts @@ -36,6 +36,7 @@ import { mapDescriptor, toDeployedName } from './env-mapper.js'; import type { EnvMapping } from './env-mapper.js'; import { rewritePolicyRefs } from './policy-ref-rewriter.js'; import type { KnownArtifactSets } from '../models/config.js'; +import { readPolicyFragmentArtifact } from './policy-fragment-artifact.js'; export type { KnownArtifactSets } from '../models/config.js'; @@ -131,6 +132,7 @@ export function prefersLegacyAuthOverride( * Policy resource types that have external XML content */ export const POLICY_TYPES = new Set([ + ResourceType.PolicyFragment, ResourceType.ServicePolicy, ResourceType.ProductPolicy, ResourceType.ApiPolicy, @@ -969,9 +971,8 @@ async function publishWiki( } /** - * Publish policy resource (ServicePolicy, ApiPolicy, ProductPolicy, ApiOperationPolicy, - * GraphQLResolverPolicy). The artifact on disk is a raw policy.xml file; there is no - * separate JSON info file for these types. Reads the XML and PUTs it with format=rawxml. + * Publish a policy resource. Policy fragments may combine optional JSON + * metadata with policy.xml; other policy types are represented by policy.xml. */ async function publishPolicy( client: IApimClient, @@ -982,13 +983,11 @@ async function publishPolicy( ): Promise { let attemptedPut = false; try { - const policyContent = await store.readContent( - config.sourceDir, - descriptor, - 'policy' - ); + const payload = descriptor.type === ResourceType.PolicyFragment + ? await readPolicyFragmentArtifact(store, config.sourceDir, descriptor) + : await readPolicyPayload(store, config.sourceDir, descriptor); - if (!policyContent) { + if (!payload) { return { descriptor, status: 'skipped', @@ -996,16 +995,6 @@ async function publishPolicy( }; } - // Fail-safe guard: extracted policies don't currently carry separate metadata - // indicating prior redaction, so marker detection is a deliberate content - // check to block publishing placeholder secrets. - const payload: Record = { - properties: { - value: policyContent.content, - format: 'rawxml', - }, - }; - // Apply overrides (e.g., format: xml) before PUT — matches Toolkit behavior let mergedPayload = applyOverrides(descriptor, payload, config.overrides); @@ -1057,6 +1046,24 @@ async function publishPolicy( } } +async function readPolicyPayload( + store: IArtifactStore, + sourceDir: string, + descriptor: ResourceDescriptor +): Promise | undefined> { + const policyContent = await store.readContent(sourceDir, descriptor, 'policy'); + if (!policyContent) { + return undefined; + } + + return { + properties: { + value: policyContent.content, + format: 'rawxml', + }, + }; +} + /** * Normalise the `properties.scope` field of a Subscription resource. * diff --git a/src/services/secret-redaction-guard.ts b/src/services/secret-redaction-guard.ts index ba1a8442..e55450dd 100644 --- a/src/services/secret-redaction-guard.ts +++ b/src/services/secret-redaction-guard.ts @@ -28,6 +28,7 @@ import { REDACTION_MARKER } from './secret-redactor.js'; import { buildResourceLabel } from '../lib/resource-uri.js'; import { getNamePart } from '../lib/resource-path.js'; import { isAutoGeneratedId } from '../lib/auto-generated.js'; +import { readPolicyFragmentArtifact } from './policy-fragment-artifact.js'; /** * A single artifact that still contains a redaction marker after overrides. @@ -73,18 +74,13 @@ async function scanPolicy( config: PublishConfig, descriptor: ResourceDescriptor ): Promise { - const policyContent = await store.readContent(config.sourceDir, descriptor, 'policy'); - if (!policyContent) { + const payload = descriptor.type === ResourceType.PolicyFragment + ? await readPolicyFragmentArtifact(store, config.sourceDir, descriptor) + : await readPolicyPayload(store, config.sourceDir, descriptor); + if (!payload) { return undefined; } - const payload: Record = { - properties: { - value: policyContent.content, - format: 'rawxml', - }, - }; - const merged = applyOverrides(descriptor, payload, config.overrides); const mergedProps = merged.properties as Record | undefined; const mergedValue = mergedProps?.value; @@ -100,6 +96,24 @@ async function scanPolicy( return undefined; } +async function readPolicyPayload( + store: IArtifactStore, + sourceDir: string, + descriptor: ResourceDescriptor +): Promise | undefined> { + const policyContent = await store.readContent(sourceDir, descriptor, 'policy'); + if (!policyContent) { + return undefined; + } + + return { + properties: { + value: policyContent.content, + format: 'rawxml', + }, + }; +} + async function scanNamedValue( store: IArtifactStore, config: PublishConfig, diff --git a/src/services/transitive-extractor.ts b/src/services/transitive-extractor.ts index 4717d95f..1f9105d6 100644 --- a/src/services/transitive-extractor.ts +++ b/src/services/transitive-extractor.ts @@ -10,6 +10,8 @@ import { logger } from '../lib/logger.js'; import { runParallel } from '../lib/parallel-runner.js'; import { redactSecrets } from './secret-redactor.js'; import { findTransitiveDependencies } from './transitive-resolver.js'; +import { ResourceType } from '../models/resource-types.js'; +import { writePolicyFragmentArtifact } from './policy-fragment-artifact.js'; const DEFAULT_CONCURRENCY = 5; @@ -67,8 +69,17 @@ export async function extractTransitiveDependencies( serviceContext && dep.workspace !== workspace ? serviceContext : context; const json = await client.getResource(dependencyContext, dep); if (json) { - const safeJson = redactSecrets(dep, json); - await store.writeResource(outputDir, dep, safeJson); + let safeJson = redactSecrets(dep, json); + if (dep.type === ResourceType.PolicyFragment) { + safeJson = await writePolicyFragmentArtifact( + store, + outputDir, + dep, + safeJson + ); + } else { + await store.writeResource(outputDir, dep, safeJson); + } logger.info(`Extracted transitive dependency ${buildResourceLabel(dep)}`); return { dep, json: safeJson }; } diff --git a/src/services/transitive-resolver.ts b/src/services/transitive-resolver.ts index 1251e823..0a324dff 100644 --- a/src/services/transitive-resolver.ts +++ b/src/services/transitive-resolver.ts @@ -12,6 +12,7 @@ import { ResourceType, RESOURCE_TYPE_METADATA } from '../models/resource-types.j import { ResourceDescriptor } from '../models/types.js'; import type { IArtifactStore } from '../clients/iartifact-store.js'; import { logger } from '../lib/logger.js'; +import { readPolicyFragmentArtifact } from './policy-fragment-artifact.js'; import { getResourceDescriptorKey } from '../lib/resource-path.js'; /** @@ -289,9 +290,11 @@ export async function scanArtifactReferences( } const infoFile = RESOURCE_TYPE_METADATA[descriptor.type]?.infoFile; - const json = POLICY_RESOURCE_TYPES.has(descriptor.type) || !infoFile?.endsWith('.json') - ? undefined - : await store.readResource(sourceDir, descriptor); + const json = descriptor.type === ResourceType.PolicyFragment + ? await readPolicyFragmentArtifact(store, sourceDir, descriptor) + : POLICY_RESOURCE_TYPES.has(descriptor.type) || !infoFile?.endsWith('.json') + ? undefined + : await store.readResource(sourceDir, descriptor); if (json) { if (descriptor.type === ResourceType.Api) { apis.set(descriptor.nameParts.join('/'), json); diff --git a/tests/integration/all-resource-types/expected-structure.json b/tests/integration/all-resource-types/expected-structure.json index fcd65a38..15911b45 100644 --- a/tests/integration/all-resource-types/expected-structure.json +++ b/tests/integration/all-resource-types/expected-structure.json @@ -238,7 +238,7 @@ "expected": [ { "name": "src-fragment-cors", - "files": ["policyFragmentInformation.json"], + "files": ["policyFragmentInformation.json", "policy.xml"], "spotChecks": { "policyFragmentInformation.json": { "properties.description": "CORS policy fragment" @@ -247,7 +247,7 @@ }, { "name": "src-fragment-ratelimit", - "files": ["policyFragmentInformation.json"], + "files": ["policyFragmentInformation.json", "policy.xml"], "spotChecks": { "policyFragmentInformation.json": { "properties.description": "Rate limit policy fragment" diff --git a/tests/unit/clients/artifact-store.test.ts b/tests/unit/clients/artifact-store.test.ts index 25989e63..261c69f8 100644 --- a/tests/unit/clients/artifact-store.test.ts +++ b/tests/unit/clients/artifact-store.test.ts @@ -310,6 +310,21 @@ describe('ArtifactStore', () => { .map((d) => d.nameParts[0]); expect(products).toContain('prod1'); }); + + it('should list an XML-only policy fragment', async () => { + const descriptor: ResourceDescriptor = { + type: ResourceType.PolicyFragment, + nameParts: ['shared-auth'], + }; + await store.writeContent( + tmpDir, + descriptor, + '', + 'policy' + ); + + await expect(store.listResources(tmpDir)).resolves.toContainEqual(descriptor); + }); }); describe('commitStagedExtraction', () => { diff --git a/tests/unit/lib/resource-path.test.ts b/tests/unit/lib/resource-path.test.ts index f70b8bed..3bef42d8 100644 --- a/tests/unit/lib/resource-path.test.ts +++ b/tests/unit/lib/resource-path.test.ts @@ -380,6 +380,40 @@ describe('parseArtifactPath', () => { expect(result!.type).toBe(ResourceType.ServicePolicy); expect(result!.nameParts).toEqual([]); }); + + it('should parse policy fragment policy.xml', () => { + const filePath = path.join( + baseDir, + 'policyFragments', + 'shared-auth', + 'policy.xml' + ); + const result = parseArtifactPath(baseDir, filePath); + + expect(result).toEqual({ + type: ResourceType.PolicyFragment, + nameParts: ['shared-auth'], + workspace: undefined, + }); + }); + + it('should parse workspace policy fragment policy.xml', () => { + const filePath = path.join( + baseDir, + 'workspaces', + 'dev', + 'policyFragments', + 'shared-auth', + 'policy.xml' + ); + const result = parseArtifactPath(baseDir, filePath); + + expect(result).toEqual({ + type: ResourceType.PolicyFragment, + nameParts: ['shared-auth'], + workspace: 'dev', + }); + }); }); describe('parseArtifactChangePath', () => { diff --git a/tests/unit/services/git-diff-service.test.ts b/tests/unit/services/git-diff-service.test.ts index ebced39b..3a98024c 100644 --- a/tests/unit/services/git-diff-service.test.ts +++ b/tests/unit/services/git-diff-service.test.ts @@ -7,6 +7,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import { computeGitDiff } from '../../../src/services/git-diff-service.js'; import { simpleGit } from 'simple-git'; +import { ResourceType } from '../../../src/models/resource-types.js'; // Create mock git instance const mockGit = { @@ -217,6 +218,43 @@ describe('git-diff-service', () => { ]); }); + it('should map policy fragment XML changes to PolicyFragment descriptor', async () => { + mockGit.checkIsRepo.mockResolvedValue(true); + mockGit.revparse.mockResolvedValue('abc123'); + mockGit.diff.mockResolvedValue( + 'M\tpolicyFragments/shared-auth/policy.xml\n' + ); + + const result = await computeGitDiff('/source', 'abc123'); + + expect(result.deletedDescriptors).toEqual([]); + expect(result.changedDescriptors).toEqual([ + { + type: ResourceType.PolicyFragment, + nameParts: ['shared-auth'], + workspace: undefined, + }, + ]); + }); + + it('should map workspace policy fragment XML changes', async () => { + mockGit.checkIsRepo.mockResolvedValue(true); + mockGit.revparse.mockResolvedValue('abc123'); + mockGit.diff.mockResolvedValue( + 'M\tworkspaces/dev/policyFragments/shared-auth/policy.xml\n' + ); + + const result = await computeGitDiff('/source', 'abc123'); + + expect(result.changedDescriptors).toEqual([ + { + type: ResourceType.PolicyFragment, + nameParts: ['shared-auth'], + workspace: 'dev', + }, + ]); + }); + it('should parse deleted files as deleted descriptors', async () => { // mockGit is at module scope mockGit.checkIsRepo.mockResolvedValue(true); diff --git a/tests/unit/services/policy-fragment-artifact.test.ts b/tests/unit/services/policy-fragment-artifact.test.ts new file mode 100644 index 00000000..034f6804 --- /dev/null +++ b/tests/unit/services/policy-fragment-artifact.test.ts @@ -0,0 +1,115 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +import { describe, expect, it, vi } from 'vitest'; +import type { IArtifactStore } from '../../../src/clients/iartifact-store.js'; +import { ResourceType } from '../../../src/models/resource-types.js'; +import type { ResourceDescriptor } from '../../../src/models/types.js'; +import { + readPolicyFragmentArtifact, + writePolicyFragmentArtifact, +} from '../../../src/services/policy-fragment-artifact.js'; + +function createMockStore() { + return { + writeResource: vi.fn().mockResolvedValue(undefined), + writeContent: vi.fn().mockResolvedValue(undefined), + writeAssociation: vi.fn(), + readResource: vi.fn().mockResolvedValue(undefined), + readContent: vi.fn().mockResolvedValue(undefined), + readAssociation: vi.fn(), + listResources: vi.fn(), + deleteResource: vi.fn(), + commitStagedExtraction: vi.fn(), + } satisfies IArtifactStore; +} + +const descriptor: ResourceDescriptor = { + type: ResourceType.PolicyFragment, + nameParts: ['shared-auth'], +}; + +describe('policy-fragment-artifact', () => { + it('reads a legacy JSON-only fragment', async () => { + const store = createMockStore(); + const information = { + properties: { + description: 'Shared authentication', + value: '', + format: 'rawxml', + }, + }; + store.readResource.mockResolvedValue(information); + + await expect( + readPolicyFragmentArtifact(store, '/source', descriptor) + ).resolves.toEqual(information); + }); + + it('reads an XML-only fragment', async () => { + const store = createMockStore(); + store.readContent.mockResolvedValue({ + content: '', + }); + + await expect( + readPolicyFragmentArtifact(store, '/source', descriptor) + ).resolves.toEqual({ + properties: { + value: '', + format: 'rawxml', + }, + }); + }); + + it('merges split metadata with authoritative XML content', async () => { + const store = createMockStore(); + store.readResource.mockResolvedValue({ + properties: { + description: 'Shared authentication', + value: 'legacy', + format: 'xml', + }, + }); + store.readContent.mockResolvedValue({ + content: '', + }); + + await expect( + readPolicyFragmentArtifact(store, '/source', descriptor) + ).resolves.toEqual({ + properties: { + description: 'Shared authentication', + value: '', + format: 'rawxml', + }, + }); + }); + + it('writes extracted metadata and policy content separately', async () => { + const store = createMockStore(); + const json = { + name: 'shared-auth', + properties: { + description: 'Shared authentication', + value: '', + format: 'rawxml', + }, + }; + + await writePolicyFragmentArtifact(store, '/output', descriptor, json); + + expect(store.writeResource).toHaveBeenCalledWith('/output', descriptor, { + name: 'shared-auth', + properties: { + description: 'Shared authentication', + }, + }); + expect(store.writeContent).toHaveBeenCalledWith( + '/output', + descriptor, + '', + 'policy' + ); + }); +}); diff --git a/tests/unit/services/publish-service.test.ts b/tests/unit/services/publish-service.test.ts index 503c3bef..f3e162ca 100644 --- a/tests/unit/services/publish-service.test.ts +++ b/tests/unit/services/publish-service.test.ts @@ -1278,6 +1278,39 @@ describe('publish-service', () => { expect(config.knownArtifactSets?.namedValues.has('nv-changed')).toBe(true); }); + it('should update a policy fragment when one artifact representation remains', async () => { + const client = createMockClient(); + const descriptor: ResourceDescriptor = { + type: ResourceType.PolicyFragment, + nameParts: ['shared-auth'], + }; + const store = createMockStore([descriptor]); + + vi.mocked(computeGitDiff).mockResolvedValue({ + changedDescriptors: [], + deletedDescriptors: [descriptor], + }); + + const config: PublishConfig = { + service: testContext, + sourceDir: '/source', + dryRun: false, + deleteUnmatched: false, + commitId: 'abc123', + logLevel: LogLevel.INFO, + }; + + const result = await runPublish(client, store, config); + + expect(client.deleteResource).not.toHaveBeenCalled(); + expect(client.putResource).toHaveBeenCalledWith( + testContext, + descriptor, + expect.any(Object) + ); + expect(result.totalDeletes).toBe(0); + }); + it('should pass commit-scoped deleted descriptors to dry-run report', async () => { const client = createMockClient(); const store = createMockStore([]); diff --git a/tests/unit/services/resource-extractor.test.ts b/tests/unit/services/resource-extractor.test.ts index 9c828123..995b6190 100644 --- a/tests/unit/services/resource-extractor.test.ts +++ b/tests/unit/services/resource-extractor.test.ts @@ -124,6 +124,46 @@ describe('resource-extractor', () => { expect(props.value).toBe('*** REDACTED ***'); }); + it('should split policy fragment metadata and XML content', async () => { + const client = createMockClient([ + { + name: 'shared-auth', + properties: { + description: 'Shared authentication', + value: '', + format: 'rawxml', + }, + }, + ]); + const store = createMockStore(); + + const result = await extractResourceType( + client, + store, + testContext, + ResourceType.PolicyFragment, + '/output' + ); + + expect(result.errorCount).toBe(0); + expect(store.writeResource).toHaveBeenCalledWith( + '/output', + expect.objectContaining({ type: ResourceType.PolicyFragment }), + { + name: 'shared-auth', + properties: { + description: 'Shared authentication', + }, + } + ); + expect(store.writeContent).toHaveBeenCalledWith( + '/output', + expect.objectContaining({ type: ResourceType.PolicyFragment }), + '', + 'policy' + ); + }); + it('should handle errors gracefully', async () => { const client = { ...createMockClient(), diff --git a/tests/unit/services/resource-publisher.test.ts b/tests/unit/services/resource-publisher.test.ts index 6add0063..734b81da 100644 --- a/tests/unit/services/resource-publisher.test.ts +++ b/tests/unit/services/resource-publisher.test.ts @@ -457,6 +457,90 @@ describe('resource-publisher', () => { expect(client.putResource).not.toHaveBeenCalled(); }); + it('should publish an XML-only policy fragment', async () => { + const client = createMockClient(); + const store = createMockStore(); + store.readResource.mockResolvedValue(undefined); + store.readContent.mockResolvedValue({ + content: '', + }); + const descriptor: ResourceDescriptor = { + type: ResourceType.PolicyFragment, + nameParts: ['shared-auth'], + }; + + const result = await publishResource( + client, + store, + testContext, + descriptor, + testConfig + ); + + expect(result.status).toBe('success'); + expect(client.putResource).toHaveBeenCalledWith(testContext, descriptor, { + properties: { + value: '', + format: 'rawxml', + }, + }); + }); + + it('should merge policy fragment metadata with authoritative XML content', async () => { + const client = createMockClient(); + const store = createMockStore(); + store.readResource.mockResolvedValue({ + properties: { + description: 'Shared authentication', + value: 'legacy', + format: 'xml', + }, + }); + store.readContent.mockResolvedValue({ + content: '', + }); + const descriptor: ResourceDescriptor = { + type: ResourceType.PolicyFragment, + nameParts: ['shared-auth'], + }; + + await publishResource(client, store, testContext, descriptor, testConfig); + + expect(client.putResource).toHaveBeenCalledWith(testContext, descriptor, { + properties: { + description: 'Shared authentication', + value: '', + format: 'rawxml', + }, + }); + }); + + it('should publish a legacy JSON-only policy fragment', async () => { + const client = createMockClient(); + const store = createMockStore(); + const legacyPayload = { + properties: { + description: 'Shared authentication', + value: '', + format: 'rawxml', + }, + }; + store.readResource.mockResolvedValue(legacyPayload); + store.readContent.mockResolvedValue(undefined); + const descriptor: ResourceDescriptor = { + type: ResourceType.PolicyFragment, + nameParts: ['shared-auth'], + }; + + await publishResource(client, store, testContext, descriptor, testConfig); + + expect(client.putResource).toHaveBeenCalledWith( + testContext, + descriptor, + legacyPayload + ); + }); + it('should fail policy publish when policy content still contains redaction marker', async () => { const client = createMockClient(); const store = createMockStore(); diff --git a/tests/unit/services/secret-redaction-guard.test.ts b/tests/unit/services/secret-redaction-guard.test.ts index 381bb9aa..170aa991 100644 --- a/tests/unit/services/secret-redaction-guard.test.ts +++ b/tests/unit/services/secret-redaction-guard.test.ts @@ -84,6 +84,27 @@ describe('secret-redaction-guard', () => { expect(findings[0].location).toBe('policy.xml'); }); + it('flags an XML-only policy fragment that contains the redaction marker', async () => { + const store = createMockStore(); + store.readContent.mockResolvedValue({ + content: `${REDACTION_MARKER}`, + }); + const fragmentDescriptor: ResourceDescriptor = { + type: ResourceType.PolicyFragment, + nameParts: ['shared-auth'], + }; + + const findings = await scanForRedactionMarkers( + store, + testConfig, + [fragmentDescriptor] + ); + + expect(findings).toHaveLength(1); + expect(findings[0].descriptor).toBe(fragmentDescriptor); + expect(findings[0].location).toBe('policy.xml'); + }); + it('flags a secret named value that equals the redaction marker', async () => { const store = createMockStore(); store.readResource.mockResolvedValue({ diff --git a/tests/unit/services/transitive-resolver.test.ts b/tests/unit/services/transitive-resolver.test.ts index 8d05947e..7102a62d 100644 --- a/tests/unit/services/transitive-resolver.test.ts +++ b/tests/unit/services/transitive-resolver.test.ts @@ -235,6 +235,28 @@ describe('transitive-resolver', () => { expect(store.readResource).not.toHaveBeenCalled(); }); + it('scans references from an XML-only policy fragment', async () => { + const store = { + readResource: vi.fn().mockResolvedValue(undefined), + readContent: vi.fn().mockResolvedValue({ + content: '', + }), + readAssociation: vi.fn(), + }; + + await expect( + scanArtifactReferences(store, '/source', { + type: ResourceType.PolicyFragment, + nameParts: ['shared-fragment'], + workspace: 'team-a', + }) + ).resolves.toContainEqual({ + type: ResourceType.Backend, + nameParts: ['shared-backend'], + workspace: 'team-a', + }); + }); + it('should scan backend pools without treating links as transitive dependencies', async () => { const store = { readResource: vi.fn() From ac8e0903001e5f769a83fc839a73db38ec9655f3 Mon Sep 17 00:00:00 2001 From: Chris Dayne Date: Mon, 21 Sep 2026 13:56:27 +1000 Subject: [PATCH 2/5] fix: skip policy fragments without values Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/services/dry-run-reporter.ts | 21 ++++++ src/services/policy-fragment-artifact.ts | 6 ++ src/services/resource-publisher.ts | 20 +++++- tests/unit/services/dry-run-reporter.test.ts | 62 ++++++++++++++++++ tests/unit/services/publish-service.test.ts | 48 +++++++++++++- .../unit/services/resource-publisher.test.ts | 64 ++++++++++++++++++- 6 files changed, 218 insertions(+), 3 deletions(-) diff --git a/src/services/dry-run-reporter.ts b/src/services/dry-run-reporter.ts index c0812616..ebfcfcdc 100644 --- a/src/services/dry-run-reporter.ts +++ b/src/services/dry-run-reporter.ts @@ -32,6 +32,10 @@ import { } from './product-publisher.js'; import { API_CHILD_TYPES, planApiPublication } from './api-publisher.js'; import { mapDescriptor } from './env-mapper.js'; +import { + hasPolicyFragmentValue, + readPolicyFragmentArtifact, +} from './policy-fragment-artifact.js'; export interface DryRunAction { operation: 'PUT' | 'PATCH' | 'DELETE' | 'SKIP'; @@ -443,6 +447,23 @@ async function planDryRunPublications( continue; } + if (descriptor.type === ResourceType.PolicyFragment) { + const artifact = await readPolicyFragmentArtifact( + store, + config.sourceDir, + descriptor + ); + const hasValue = hasPolicyFragmentValue(artifact); + addPlan({ + descriptor, + eligible: hasValue, + reason: hasValue + ? undefined + : 'no policy value was found in policy.xml or policyFragmentInformation.json', + }); + continue; + } + addPlan({ descriptor, eligible: true }); } diff --git a/src/services/policy-fragment-artifact.ts b/src/services/policy-fragment-artifact.ts index 61bacba9..65c58e20 100644 --- a/src/services/policy-fragment-artifact.ts +++ b/src/services/policy-fragment-artifact.ts @@ -15,6 +15,12 @@ function getProperties( : {}; } +export function hasPolicyFragmentValue( + artifact: Record | undefined +): boolean { + return typeof getProperties(artifact).value === 'string'; +} + /** * Read a policy fragment using the Azure APIops artifact contract. * diff --git a/src/services/resource-publisher.ts b/src/services/resource-publisher.ts index 9f5e9abb..a37191bf 100644 --- a/src/services/resource-publisher.ts +++ b/src/services/resource-publisher.ts @@ -36,7 +36,10 @@ import { mapDescriptor, toDeployedName } from './env-mapper.js'; import type { EnvMapping } from './env-mapper.js'; import { rewritePolicyRefs } from './policy-ref-rewriter.js'; import type { KnownArtifactSets } from '../models/config.js'; -import { readPolicyFragmentArtifact } from './policy-fragment-artifact.js'; +import { + hasPolicyFragmentValue, + readPolicyFragmentArtifact, +} from './policy-fragment-artifact.js'; export type { KnownArtifactSets } from '../models/config.js'; @@ -995,6 +998,21 @@ async function publishPolicy( }; } + if ( + descriptor.type === ResourceType.PolicyFragment && + !hasPolicyFragmentValue(payload) + ) { + logger.warn( + `Skipping ${buildResourceLabel(descriptor)}: no policy value was found in ` + + 'policy.xml or policyFragmentInformation.json.' + ); + return { + descriptor, + status: 'skipped', + action: 'noop', + }; + } + // Apply overrides (e.g., format: xml) before PUT — matches Toolkit behavior let mergedPayload = applyOverrides(descriptor, payload, config.overrides); diff --git a/tests/unit/services/dry-run-reporter.test.ts b/tests/unit/services/dry-run-reporter.test.ts index b8e00d84..5de047d6 100644 --- a/tests/unit/services/dry-run-reporter.test.ts +++ b/tests/unit/services/dry-run-reporter.test.ts @@ -136,6 +136,68 @@ describe('dry-run-reporter', () => { ); }); + it('should skip a metadata-only policy fragment', async () => { + const client = createMockClient(); + const store = createMockStore(); + store.readResource.mockResolvedValue({ + properties: { + description: 'Shared authentication', + }, + }); + const descriptor: ResourceDescriptor = { + type: ResourceType.PolicyFragment, + nameParts: ['shared-auth'], + }; + + const report = await generateDryRunReport( + store, + client, + testContext, + testConfig, + [descriptor] + ); + + expect(report.actions).toEqual([ + expect.objectContaining({ + operation: 'SKIP', + descriptor, + reason: expect.stringContaining('no policy value was found'), + }), + ]); + expect(report.summary.skips).toBe(1); + }); + + it('should publish a policy fragment with an explicitly empty value in dry-run', async () => { + const client = createMockClient(); + const store = createMockStore(); + store.readResource.mockResolvedValue({ + properties: { + value: '', + format: 'rawxml', + }, + }); + const descriptor: ResourceDescriptor = { + type: ResourceType.PolicyFragment, + nameParts: ['shared-auth'], + }; + + const report = await generateDryRunReport( + store, + client, + testContext, + testConfig, + [descriptor] + ); + + expect(report.actions).toEqual([ + expect.objectContaining({ + operation: 'PUT', + descriptor, + }), + ]); + expect(report.summary.skips).toBe(0); + }); + it('checks the deployed descriptor when environment mapping is active', async () => { const client = createMockClient(); const store = createMockStore(); diff --git a/tests/unit/services/publish-service.test.ts b/tests/unit/services/publish-service.test.ts index f3e162ca..e04797a7 100644 --- a/tests/unit/services/publish-service.test.ts +++ b/tests/unit/services/publish-service.test.ts @@ -237,6 +237,11 @@ describe('publish-service', () => { ].join(''), format: 'xml', } + : descriptor.type === ResourceType.PolicyFragment + ? { + content: '', + format: 'rawxml', + } : undefined ); @@ -1278,13 +1283,19 @@ describe('publish-service', () => { expect(config.knownArtifactSets?.namedValues.has('nv-changed')).toBe(true); }); - it('should update a policy fragment when one artifact representation remains', async () => { + it('should update a policy fragment when remaining JSON contains a policy value', async () => { const client = createMockClient(); const descriptor: ResourceDescriptor = { type: ResourceType.PolicyFragment, nameParts: ['shared-auth'], }; const store = createMockStore([descriptor]); + store.readResource.mockResolvedValue({ + properties: { + value: '', + format: 'rawxml', + }, + }); vi.mocked(computeGitDiff).mockResolvedValue({ changedDescriptors: [], @@ -1311,6 +1322,41 @@ describe('publish-service', () => { expect(result.totalDeletes).toBe(0); }); + it('should skip a policy fragment when policy.xml is deleted and metadata has no value', async () => { + const client = createMockClient(); + const descriptor: ResourceDescriptor = { + type: ResourceType.PolicyFragment, + nameParts: ['shared-auth'], + }; + const store = createMockStore([descriptor]); + store.readResource.mockResolvedValue({ + properties: { + description: 'Shared authentication', + }, + }); + + vi.mocked(computeGitDiff).mockResolvedValue({ + changedDescriptors: [], + deletedDescriptors: [descriptor], + }); + + const config: PublishConfig = { + service: testContext, + sourceDir: '/source', + dryRun: false, + deleteUnmatched: false, + commitId: 'abc123', + logLevel: LogLevel.INFO, + }; + + const result = await runPublish(client, store, config); + + expect(client.putResource).not.toHaveBeenCalled(); + expect(client.deleteResource).not.toHaveBeenCalled(); + expect(result.totalSkipped).toBe(1); + expect(result.totalDeletes).toBe(0); + }); + it('should pass commit-scoped deleted descriptors to dry-run report', async () => { const client = createMockClient(); const store = createMockStore([]); diff --git a/tests/unit/services/resource-publisher.test.ts b/tests/unit/services/resource-publisher.test.ts index 734b81da..70704f61 100644 --- a/tests/unit/services/resource-publisher.test.ts +++ b/tests/unit/services/resource-publisher.test.ts @@ -15,7 +15,7 @@ import { ResourceType } from '../../../src/models/resource-types.js'; import { ApimServiceContext, ResourceDescriptor } from '../../../src/models/types.js'; import { PublishConfig } from '../../../src/models/config.js'; import { KeyVaultAccessError } from '../../../src/services/keyvault-checker.js'; -import { LogLevel } from '../../../src/lib/logger.js'; +import { logger, LogLevel } from '../../../src/lib/logger.js'; import { REDACTION_MARKER } from '../../../src/services/secret-redactor.js'; import { buildEnvMapping } from '../../../src/services/env-mapper.js'; import { HttpError } from '../../../src/clients/apim-client.js'; @@ -541,6 +541,68 @@ describe('resource-publisher', () => { ); }); + it('should publish a policy fragment with an explicitly empty value', async () => { + const client = createMockClient(); + const store = createMockStore(); + const payload = { + properties: { + description: 'Intentionally empty', + value: '', + format: 'rawxml', + }, + }; + store.readResource.mockResolvedValue(payload); + const descriptor: ResourceDescriptor = { + type: ResourceType.PolicyFragment, + nameParts: ['shared-auth'], + }; + + const result = await publishResource( + client, + store, + testContext, + descriptor, + testConfig + ); + + expect(result.status).toBe('success'); + expect(client.putResource).toHaveBeenCalledWith( + testContext, + descriptor, + payload + ); + }); + + it('should skip a metadata-only policy fragment with a warning', async () => { + const client = createMockClient(); + const store = createMockStore(); + store.readResource.mockResolvedValue({ + properties: { + description: 'Shared authentication', + }, + }); + const descriptor: ResourceDescriptor = { + type: ResourceType.PolicyFragment, + nameParts: ['shared-auth'], + }; + const warnSpy = vi.spyOn(logger, 'warn').mockImplementation(() => undefined); + + const result = await publishResource( + client, + store, + testContext, + descriptor, + testConfig + ); + + expect(result).toMatchObject({ status: 'skipped', action: 'noop' }); + expect(client.putResource).not.toHaveBeenCalled(); + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining('no policy value was found') + ); + warnSpy.mockRestore(); + }); + it('should fail policy publish when policy content still contains redaction marker', async () => { const client = createMockClient(); const store = createMockStore(); From 265a5ce27214721237f557f53fabc31aed95f300 Mon Sep 17 00:00:00 2001 From: Chris Dayne Date: Mon, 21 Sep 2026 14:01:29 +1000 Subject: [PATCH 3/5] fix: validate policy fragment values after overrides Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/services/dry-run-reporter.ts | 7 ++- src/services/resource-publisher.ts | 19 +++----- tests/unit/services/dry-run-reporter.test.ts | 43 ++++++++++++++++++ .../unit/services/resource-publisher.test.ts | 44 +++++++++++++++++++ 4 files changed, 97 insertions(+), 16 deletions(-) diff --git a/src/services/dry-run-reporter.ts b/src/services/dry-run-reporter.ts index ebfcfcdc..33ed1086 100644 --- a/src/services/dry-run-reporter.ts +++ b/src/services/dry-run-reporter.ts @@ -453,13 +453,16 @@ async function planDryRunPublications( config.sourceDir, descriptor ); - const hasValue = hasPolicyFragmentValue(artifact); + const mergedArtifact = artifact + ? applyOverrides(descriptor, artifact, config.overrides) + : undefined; + const hasValue = hasPolicyFragmentValue(mergedArtifact); addPlan({ descriptor, eligible: hasValue, reason: hasValue ? undefined - : 'no policy value was found in policy.xml or policyFragmentInformation.json', + : 'no policy value was found in policy.xml, policyFragmentInformation.json, or overrides', }); continue; } diff --git a/src/services/resource-publisher.ts b/src/services/resource-publisher.ts index a37191bf..c5492960 100644 --- a/src/services/resource-publisher.ts +++ b/src/services/resource-publisher.ts @@ -508,15 +508,6 @@ export async function publishResource( json = applyApiPathPrefix(json, descriptor, config); - // For PolicyFragment: rewrite cross-resource refs in properties.value (policy XML) - if (descriptor.type === ResourceType.PolicyFragment && config.envMapping && config.knownArtifactSets) { - const props = json.properties as Record | undefined; - if (typeof props?.value === 'string') { - const rewrittenXml = rewritePolicyRefs(props.value, config.envMapping, config.knownArtifactSets); - json = { ...json, properties: { ...props, value: rewrittenXml } }; - } - } - // Apply env-mapping: affix descriptor name segments before PUT const deployedDescriptor = config.envMapping ? mapDescriptor(descriptor, config.envMapping) @@ -998,13 +989,16 @@ async function publishPolicy( }; } + // Apply overrides (e.g., format: xml) before PUT — matches Toolkit behavior + let mergedPayload = applyOverrides(descriptor, payload, config.overrides); + if ( descriptor.type === ResourceType.PolicyFragment && - !hasPolicyFragmentValue(payload) + !hasPolicyFragmentValue(mergedPayload) ) { logger.warn( `Skipping ${buildResourceLabel(descriptor)}: no policy value was found in ` + - 'policy.xml or policyFragmentInformation.json.' + 'policy.xml, policyFragmentInformation.json, or overrides.' ); return { descriptor, @@ -1013,9 +1007,6 @@ async function publishPolicy( }; } - // Apply overrides (e.g., format: xml) before PUT — matches Toolkit behavior - let mergedPayload = applyOverrides(descriptor, payload, config.overrides); - // Rewrite policy XML references from canonical → deployed names if (config.envMapping && config.knownArtifactSets) { const mProps = mergedPayload.properties as Record | undefined; diff --git a/tests/unit/services/dry-run-reporter.test.ts b/tests/unit/services/dry-run-reporter.test.ts index 5de047d6..7fb386b3 100644 --- a/tests/unit/services/dry-run-reporter.test.ts +++ b/tests/unit/services/dry-run-reporter.test.ts @@ -198,6 +198,49 @@ describe('dry-run-reporter', () => { expect(report.summary.skips).toBe(0); }); + it('should publish a metadata-only policy fragment when an override supplies the value in dry-run', async () => { + const client = createMockClient(); + const store = createMockStore(); + store.readResource.mockResolvedValue({ + properties: { + description: 'Shared authentication', + }, + }); + const descriptor: ResourceDescriptor = { + type: ResourceType.PolicyFragment, + nameParts: ['shared-auth'], + }; + const config: PublishConfig = { + ...testConfig, + overrides: { + policyFragments: { + 'shared-auth': { + properties: { + value: '', + format: 'rawxml', + }, + }, + }, + }, + }; + + const report = await generateDryRunReport( + store, + client, + testContext, + config, + [descriptor] + ); + + expect(report.actions).toEqual([ + expect.objectContaining({ + operation: 'PUT', + descriptor, + }), + ]); + expect(report.summary.skips).toBe(0); + }); + it('checks the deployed descriptor when environment mapping is active', async () => { const client = createMockClient(); const store = createMockStore(); diff --git a/tests/unit/services/resource-publisher.test.ts b/tests/unit/services/resource-publisher.test.ts index 70704f61..8312361f 100644 --- a/tests/unit/services/resource-publisher.test.ts +++ b/tests/unit/services/resource-publisher.test.ts @@ -573,6 +573,50 @@ describe('resource-publisher', () => { ); }); + it('should publish a metadata-only policy fragment when an override supplies the value', async () => { + const client = createMockClient(); + const store = createMockStore(); + store.readResource.mockResolvedValue({ + properties: { + description: 'Shared authentication', + }, + }); + const descriptor: ResourceDescriptor = { + type: ResourceType.PolicyFragment, + nameParts: ['shared-auth'], + }; + const config: PublishConfig = { + ...testConfig, + overrides: { + policyFragments: { + 'shared-auth': { + properties: { + value: '', + format: 'rawxml', + }, + }, + }, + }, + }; + + const result = await publishResource( + client, + store, + testContext, + descriptor, + config + ); + + expect(result.status).toBe('success'); + expect(client.putResource).toHaveBeenCalledWith(testContext, descriptor, { + properties: { + description: 'Shared authentication', + value: '', + format: 'rawxml', + }, + }); + }); + it('should skip a metadata-only policy fragment with a warning', async () => { const client = createMockClient(); const store = createMockStore(); From 5dd4f9983188ff9b22d7b277adeb2306776adca9 Mon Sep 17 00:00:00 2001 From: Chris Dayne Date: Mon, 21 Sep 2026 14:28:21 +1000 Subject: [PATCH 4/5] docs: minimize policy fragment documentation changes Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- docs/reference/apim-glossary.md | 2 +- docs/reference/artifact-format.md | 13 ++----------- specs/data-model.md | 2 +- specs/research.md | 4 +--- specs/v1-research-report.md | 5 ++--- 5 files changed, 7 insertions(+), 19 deletions(-) diff --git a/docs/reference/apim-glossary.md b/docs/reference/apim-glossary.md index 25a8ca0f..ce06fa6d 100644 --- a/docs/reference/apim-glossary.md +++ b/docs/reference/apim-glossary.md @@ -76,7 +76,7 @@ XML-based middleware that runs on API requests and responses. Policies handle ra A reusable snippet of policy XML that can be included in other policies via ``. Useful for shared logic like standard rate limiting or CORS headers. - **Microsoft Docs:** [Policy fragments](https://learn.microsoft.com/en-us/azure/api-management/policy-fragments) -- **In artifacts:** `policyFragments/{name}/policy.xml`, with optional metadata in `policyFragmentInformation.json` +- **In artifacts:** `policyFragments/{name}/policyFragmentInformation.json` --- diff --git a/docs/reference/artifact-format.md b/docs/reference/artifact-format.md index feac1e12..3b24b559 100644 --- a/docs/reference/artifact-format.md +++ b/docs/reference/artifact-format.md @@ -126,17 +126,8 @@ All 34 APIM resource types and their artifact mappings: | PolicyRestriction | `policyRestrictions/{name}` | `policyRestrictionInformation.json` | | Documentation | `documentations/{name}` | `documentationInformation.json` | -Policy fragments follow the Azure APIops split policy layout. The following -representations are supported: - -- `policy.xml` only, for fragments without metadata. -- `policy.xml` with `policyFragmentInformation.json`, where JSON metadata is - merged with the XML content. -- A legacy `policyFragmentInformation.json` containing `properties.value`. - -When both files exist, `policy.xml` supplies `properties.value` and -`properties.format` (`rawxml`), while other JSON properties such as -`description` are retained. +Policy fragments support `policy.xml`, the legacy JSON-only representation, or +both files. When both exist, XML supplies the policy value and `rawxml` format. ### Product Child Resources diff --git a/specs/data-model.md b/specs/data-model.md index a1dcf121..0ed989a7 100644 --- a/specs/data-model.md +++ b/specs/data-model.md @@ -20,7 +20,7 @@ Defines all APIM resource types the tool handles. | `Logger` | `/loggers/{name}` | `loggers/{name}/` | `loggerInformation.json` | | `Group` | `/groups/{name}` | `groups/{name}/` | `groupInformation.json` | | `Diagnostic` | `/diagnostics/{name}` | `diagnostics/{name}/` | `diagnosticInformation.json` | -| `PolicyFragment` | `/policyFragments/{name}` | `policyFragments/{name}/` | `policy.xml` and optional `policyFragmentInformation.json` | +| `PolicyFragment` | `/policyFragments/{name}` | `policy fragments/{name}/` | `policyFragmentInformation.json` | | `ServicePolicy` | `/policies/policy` | (root) | `policy.xml` | | `Product` | `/products/{name}` | `products/{name}/` | `productInformation.json` | | `ProductPolicy` | `/products/{name}/policies/policy` | `products/{name}/` | `policy.xml` | diff --git a/specs/research.md b/specs/research.md index 3b8f2792..f5809722 100644 --- a/specs/research.md +++ b/specs/research.md @@ -107,9 +107,7 @@ ├── backends/{name}/backendInformation.json ├── loggers/{name}/loggerInformation.json ├── diagnostics/{name}/diagnosticInformation.json -├── policyFragments/{name}/ -│ ├── policyFragmentInformation.json # Optional metadata -│ └── policy.xml +├── policyFragments/{name}/policyFragmentInformation.json ├── gateways/{name}/gatewayInformation.json ├── groups/{name}/groupInformation.json ├── subscriptions/{name}/subscriptionInformation.json diff --git a/specs/v1-research-report.md b/specs/v1-research-report.md index c8efdc84..347c5edb 100644 --- a/specs/v1-research-report.md +++ b/specs/v1-research-report.md @@ -141,8 +141,7 @@ policyFragmentNames: │ └── diagnosticInformation.json ├── policy fragments/ │ └── {name}/ -│ ├── policyFragmentInformation.json -│ └── policy.xml +│ └── policyFragmentInformation.json ├── products/ │ └── {name}/ │ ├── productInformation.json @@ -598,7 +597,7 @@ Users must explicitly list all dependent resources in the filter file, or extrac | Backend | `backendInformation.json` | — | — | | Logger | `loggerInformation.json` | — | — | | Diagnostic | `diagnosticInformation.json` | — | — | -| Policy Fragment | `policyFragmentInformation.json` | `policy.xml` | — | +| Policy Fragment | `policyFragmentInformation.json` | — | — | | Service Policy | `policy.xml` | — | — | | Product | `productInformation.json` | `policy.xml`, `apis.json`, `groups.json` | — | | Group | `groupInformation.json` | — | — | From a8c4ce366b0c9ba25503e6f09f529b47e869073d Mon Sep 17 00:00:00 2001 From: Chris Dayne Date: Mon, 21 Sep 2026 14:44:02 +1000 Subject: [PATCH 5/5] fix: report policy fragment redaction sources Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- docs/reference/apim-glossary.md | 2 +- src/services/policy-fragment-artifact.ts | 38 ++++++++++-- src/services/secret-redaction-guard.ts | 51 ++++++++++++++-- .../services/secret-redaction-guard.test.ts | 61 +++++++++++++++++++ 4 files changed, 139 insertions(+), 13 deletions(-) diff --git a/docs/reference/apim-glossary.md b/docs/reference/apim-glossary.md index ce06fa6d..b4b42749 100644 --- a/docs/reference/apim-glossary.md +++ b/docs/reference/apim-glossary.md @@ -76,7 +76,7 @@ XML-based middleware that runs on API requests and responses. Policies handle ra A reusable snippet of policy XML that can be included in other policies via ``. Useful for shared logic like standard rate limiting or CORS headers. - **Microsoft Docs:** [Policy fragments](https://learn.microsoft.com/en-us/azure/api-management/policy-fragments) -- **In artifacts:** `policyFragments/{name}/policyFragmentInformation.json` +- **In artifacts:** `policyFragments/{name}/policy.xml`, optionally with metadata in `policyFragmentInformation.json`; legacy JSON-only fragments are also supported --- diff --git a/src/services/policy-fragment-artifact.ts b/src/services/policy-fragment-artifact.ts index 65c58e20..b6e526a5 100644 --- a/src/services/policy-fragment-artifact.ts +++ b/src/services/policy-fragment-artifact.ts @@ -6,6 +6,15 @@ import type { ResourceDescriptor } from '../models/types.js'; import { ResourceType } from '../models/resource-types.js'; import { redactAndWarnPolicySecrets } from './secret-redactor.js'; +export type PolicyFragmentValueSource = + | 'policy.xml' + | 'policyFragmentInformation.json'; + +export interface PolicyFragmentArtifactDetails { + payload: Record; + valueSource?: PolicyFragmentValueSource; +} + function getProperties( json: Record | undefined ): Record { @@ -33,6 +42,15 @@ export async function readPolicyFragmentArtifact( baseDir: string, descriptor: ResourceDescriptor ): Promise | undefined> { + return (await readPolicyFragmentArtifactDetails(store, baseDir, descriptor)) + ?.payload; +} + +export async function readPolicyFragmentArtifactDetails( + store: IArtifactStore, + baseDir: string, + descriptor: ResourceDescriptor +): Promise { if (descriptor.type !== ResourceType.PolicyFragment) { throw new Error(`Expected PolicyFragment descriptor, got ${descriptor.type}`); } @@ -47,16 +65,24 @@ export async function readPolicyFragmentArtifact( } if (!policyContent) { - return information; + return { + payload: information!, + valueSource: hasPolicyFragmentValue(information) + ? 'policyFragmentInformation.json' + : undefined, + }; } return { - ...(information ?? {}), - properties: { - ...getProperties(information), - value: policyContent.content, - format: 'rawxml', + payload: { + ...(information ?? {}), + properties: { + ...getProperties(information), + value: policyContent.content, + format: 'rawxml', + }, }, + valueSource: 'policy.xml', }; } diff --git a/src/services/secret-redaction-guard.ts b/src/services/secret-redaction-guard.ts index e55450dd..ca608e2a 100644 --- a/src/services/secret-redaction-guard.ts +++ b/src/services/secret-redaction-guard.ts @@ -23,12 +23,15 @@ import type { PublishConfig } from '../models/config.js'; import type { ResourceDescriptor } from '../models/types.js'; import { ResourceType } from '../models/resource-types.js'; import { applyOverrides, hasNamedValueOverride } from './override-merger.js'; -import { POLICY_TYPES } from './resource-publisher.js'; +import { + hasExplicitPropertyOverride, + POLICY_TYPES, +} from './resource-publisher.js'; import { REDACTION_MARKER } from './secret-redactor.js'; import { buildResourceLabel } from '../lib/resource-uri.js'; import { getNamePart } from '../lib/resource-path.js'; import { isAutoGeneratedId } from '../lib/auto-generated.js'; -import { readPolicyFragmentArtifact } from './policy-fragment-artifact.js'; +import { readPolicyFragmentArtifactDetails } from './policy-fragment-artifact.js'; /** * A single artifact that still contains a redaction marker after overrides. @@ -74,9 +77,17 @@ async function scanPolicy( config: PublishConfig, descriptor: ResourceDescriptor ): Promise { - const payload = descriptor.type === ResourceType.PolicyFragment - ? await readPolicyFragmentArtifact(store, config.sourceDir, descriptor) - : await readPolicyPayload(store, config.sourceDir, descriptor); + const fragmentArtifact = descriptor.type === ResourceType.PolicyFragment + ? await readPolicyFragmentArtifactDetails( + store, + config.sourceDir, + descriptor + ) + : undefined; + const payload = fragmentArtifact?.payload ?? + (descriptor.type === ResourceType.PolicyFragment + ? undefined + : await readPolicyPayload(store, config.sourceDir, descriptor)); if (!payload) { return undefined; } @@ -86,16 +97,44 @@ async function scanPolicy( const mergedValue = mergedProps?.value; if (typeof mergedValue === 'string' && mergedValue.includes(REDACTION_MARKER)) { + const location = descriptor.type === ResourceType.PolicyFragment + ? getPolicyFragmentValueLocation( + descriptor, + config, + fragmentArtifact?.valueSource + ) + : 'policy.xml'; return { descriptor, label: buildResourceLabel(descriptor), - location: 'policy.xml', + location, }; } return undefined; } +function getPolicyFragmentValueLocation( + descriptor: ResourceDescriptor, + config: PublishConfig, + artifactSource: 'policy.xml' | 'policyFragmentInformation.json' | undefined +): string { + const name = getNamePart(descriptor.nameParts, 0); + if ( + hasExplicitPropertyOverride( + name, + 'value', + config.overrides?.policyFragments + ) + ) { + return `overrides.policyFragments.${name}.properties.value`; + } + + return artifactSource === 'policyFragmentInformation.json' + ? 'policyFragmentInformation.json (properties.value)' + : 'policy.xml'; +} + async function readPolicyPayload( store: IArtifactStore, sourceDir: string, diff --git a/tests/unit/services/secret-redaction-guard.test.ts b/tests/unit/services/secret-redaction-guard.test.ts index 170aa991..83fe1c2e 100644 --- a/tests/unit/services/secret-redaction-guard.test.ts +++ b/tests/unit/services/secret-redaction-guard.test.ts @@ -105,6 +105,67 @@ describe('secret-redaction-guard', () => { expect(findings[0].location).toBe('policy.xml'); }); + it('reports policyFragmentInformation.json for a JSON-only fragment marker', async () => { + const store = createMockStore(); + store.readResource.mockResolvedValue({ + properties: { + value: `${REDACTION_MARKER}`, + format: 'rawxml', + }, + }); + const fragmentDescriptor: ResourceDescriptor = { + type: ResourceType.PolicyFragment, + nameParts: ['shared-auth'], + }; + + const findings = await scanForRedactionMarkers( + store, + testConfig, + [fragmentDescriptor] + ); + + expect(findings).toHaveLength(1); + expect(findings[0].location).toBe( + 'policyFragmentInformation.json (properties.value)' + ); + }); + + it('reports the override when it supplies the fragment marker', async () => { + const store = createMockStore(); + store.readResource.mockResolvedValue({ + properties: { + description: 'Shared authentication', + }, + }); + const fragmentDescriptor: ResourceDescriptor = { + type: ResourceType.PolicyFragment, + nameParts: ['shared-auth'], + }; + const configWithOverride: PublishConfig = { + ...testConfig, + overrides: { + policyFragments: { + 'shared-auth': { + properties: { + value: `${REDACTION_MARKER}`, + }, + }, + }, + }, + }; + + const findings = await scanForRedactionMarkers( + store, + configWithOverride, + [fragmentDescriptor] + ); + + expect(findings).toHaveLength(1); + expect(findings[0].location).toBe( + 'overrides.policyFragments.shared-auth.properties.value' + ); + }); + it('flags a secret named value that equals the redaction marker', async () => { const store = createMockStore(); store.readResource.mockResolvedValue({