Handle secret redaction in policy XML

.github/skills/integration-test-prerequisites/SKILL.md

@@ -1,18 +1,21 @@
 ---
 name: "integration-test-prerequisites"
-description: "Set up Azure and GitHub prerequisites for the integration-test workflow using a user-assigned managed identity, OIDC federated credentials, RBAC roles, and environment secrets. Use when troubleshooting AADSTS70025/AADSTS700213 or authorization failures during integration-test workflow runs."
+description: "Set up Azure and GitHub prerequisites for integration workflows using a user-assigned managed identity, OIDC federated credentials, RBAC roles, and environment secrets. Use when troubleshooting AADSTS70025/AADSTS700213 or authorization failures during integration-test or integration-redact-secrets workflow runs."
 domain: "ci-cd"
 confidence: "high"
 source: "manual + observed from integration-test OIDC and RBAC troubleshooting"
 ---
 
 ## Context
 
-Use this skill when preparing or repairing prerequisites for `.github/workflows/integration-test.yml`.
+Use this skill when preparing or repairing prerequisites for:
 
-This workflow expects:
+- `.github/workflows/integration-test.yml`
+- `.github/workflows/integration-redact-secrets.yml`
+
+These workflows expect:
 - OIDC login through `azure/login@v2`
-- GitHub environment `integration-test`
+- GitHub environment `integration-test` (shared by both workflows)
 - Azure identity with enough permissions to deploy resources and create role assignments in test resource groups
 
 Preferred identity model: user-assigned managed identity (UAMI).

.github/workflows/integration-redact-secrets.yml

@@ -0,0 +1,120 @@
+# Copyright (c) Microsoft Corporation.
+# Licensed under the MIT license.
+
+name: "Test: Extract Secret Redaction"
+
+on:
+  workflow_dispatch:
+    inputs:
+      sku:
+        description: 'APIM SKU'
+        required: true
+        type: choice
+        options:
+          - Standard
+          - StandardV2
+          - Premium
+          - PremiumV2
+        default: StandardV2
+      location:
+        description: 'Azure region'
+        required: false
+        type: string
+        default: 'centralus'
+      log_level:
+        description: 'PowerShell log level (Info, Verbose, Debug)'
+        required: false
+        type: string
+        default: Verbose
+
+  workflow_call:
+    inputs:
+      sku:
+        description: 'APIM SKU'
+        required: false
+        type: string
+        default: StandardV2
+      location:
+        description: 'Azure region'
+        required: false
+        type: string
+        default: 'centralus'
+      log_level:
+        description: 'PowerShell log level (Info, Verbose, Debug)'
+        required: false
+        type: string
+        default: Verbose
+    secrets:
+      AZURE_CLIENT_ID:
+        required: true
+      AZURE_TENANT_ID:
+        required: true
+      AZURE_SUBSCRIPTION_ID:
+        required: true
+      APIM_PUBLISHER_EMAIL:
+        required: true
+      APIM_SKU:
+        required: false
+
+permissions:
+  id-token: write
+  contents: read
+
+concurrency:
+  group: integration-redact-secrets
+  cancel-in-progress: false
+
+jobs:
+  redact-secrets-test:
+    name: Extract Secret Redaction
+    if: github.ref == 'refs/heads/main'
+    runs-on: ubuntu-latest
+    timeout-minutes: 120
+    environment: integration-test
+
+    steps:
+      - uses: actions/checkout@v6
+
+      - uses: actions/setup-node@v5
+        with:
+          node-version: '22'
+          cache: 'npm'
+
+      - run: npm ci && npm run build
+
+      - name: Resolve Workflow Settings
+        id: settings
+        shell: pwsh
+        run: |
+          $logLevel = '${{ inputs.log_level }}'
+          if ([string]::IsNullOrWhiteSpace($logLevel)) { $logLevel = 'Verbose' }
+          if ($logLevel -notin @('Info', 'Verbose', 'Debug')) {
+            throw "Invalid log_level '$logLevel'. Allowed values: Info, Verbose, Debug."
+          }
+
+          $skuName = '${{ secrets.APIM_SKU }}'
+          if ([string]::IsNullOrWhiteSpace($skuName)) { $skuName = '${{ inputs.sku }}' }
+          if ([string]::IsNullOrWhiteSpace($skuName)) { $skuName = 'StandardV2' }
+
+          "logLevel=$logLevel" | Out-File -FilePath $env:GITHUB_OUTPUT -Append
+          "skuName=$skuName" | Out-File -FilePath $env:GITHUB_OUTPUT -Append
+
+      - name: Azure Login
+        uses: azure/login@v3
+        with:
+          client-id: ${{ secrets.AZURE_CLIENT_ID }}
+          tenant-id: ${{ secrets.AZURE_TENANT_ID }}
+          subscription-id: ${{ secrets.AZURE_SUBSCRIPTION_ID }}
+
+      - name: Run redaction integration test
+        shell: pwsh
+        env:
+          AZURE_CLIENT_ID: ${{ secrets.AZURE_CLIENT_ID }}
+          AZURE_TENANT_ID: ${{ secrets.AZURE_TENANT_ID }}
+        run: |
+          ./tests/integration/redact-secrets/run-redact-secrets-test.ps1 `
+            -SourceSubscriptionId '${{ secrets.AZURE_SUBSCRIPTION_ID }}' `
+            -PublisherEmail '${{ secrets.APIM_PUBLISHER_EMAIL }}' `
+            -SkuName '${{ steps.settings.outputs.skuName }}' `
+            -Location '${{ inputs.location }}' `
+            -LogLevel '${{ steps.settings.outputs.logLevel }}'

.gitignore

@@ -39,11 +39,11 @@ Desktop.ini
 .local-extract*/
 
 # Files for integration tests
-tests/integration/all-resource-types/**/logs/**
-tests/integration/all-resource-types/**/extracted-artifacts*/**
-tests/integration/all-resource-types/bicep/target-apim.json
-tests/integration/all-resource-types/bicep/source-apim.json
-tests/integration/all-resource-types/bicep/source-apim-post-activation.bicep
+tests/integration/**/logs
+tests/integration/**/extracted-artifacts*
+tests/integration/**/bicep/target-apim.json
+tests/integration/**/bicep/source-apim.json
+tests/integration/**/bicep/source-apim-post-activation.bicep
 
 # Environment variables
 .env

.squad/agents/apimexpert/history.md

@@ -116,3 +116,17 @@ When comparing a *link-imported* API (e.g. Petstore via `swagger-link`/`openapi-
 
 Round-trip comparison harness (`tests/integration/all-resource-types`) normalizes both via `RepresentationSchemaRefIgnoredProperties` and `ParameterIgnoredProperties`. Symptom if not stripped: every operation shows a `properties.request/responses/templateParameters` DIFF present-on-one-side-only.
 
+### 2026-07-01: Overriding a policy to clear a redacted secret — possible but rarely the right fix
+
+**Question that comes up:** when the extract-time secret redactor leaves `*** REDACTED ***` inline in a policy, can you clear the publish pre-flight guard by *overriding the policy* instead of fixing the source?
+
+**Answer: yes, technically — but it's almost never the intended path.**
+
+- All five policy types are wired into the override system (`src/services/override-merger.ts`): `ServicePolicy → policies`, `PolicyFragment → policyFragments` (direct); `ApiPolicy → apis.<api>.policies`, `ProductPolicy → products.<product>.policies` (child); `ApiOperationPolicy → apis.<api>.operations.<op>.policies` (grandchild).
+- The publish payload for a policy is `{ properties: { value, format } }`, and `applyOverrides` deep-merges `properties`, so an override supplying `properties.value` replaces the policy XML wholesale.
+- The pre-flight guard (`src/services/secret-redaction-guard.ts`) applies overrides **before** scanning for the marker — by design — so a policy override that yields clean content passes the check.
+
+**Why it's the wrong tool for redacted secrets:**
+- The marker is inserted **inline** inside the XML (e.g. inside a `set-header` `<value>`), but overrides are **whole-value** replacements of `properties.value` — there is no inline/sub-string patch. You'd have to paste the entire policy XML (with the real secret) into a committed override file, re-introducing the plaintext secret that redaction removed.
+- Intended remediation: change the **source** policy to reference a named value (`{{my-secret}}`) so redaction never triggers, then supply the secret via a named-value override or Key Vault reference. The docs' "Gotcha: Redacted secrets" section (`docs/guides/environment-overrides.md`) only covers named values — there is no documented "override a redacted policy" workflow, reflecting this.
+

src/services/api-extractor.ts

@@ -14,6 +14,7 @@ import { ResourceType, RESOURCE_TYPE_METADATA } from '../models/resource-types.j
 import { FilterConfig } from '../models/config.js';
 import { shouldIncludeResource } from './filter-service.js';
 import { extractResourceType, ExtractedResource } from './resource-extractor.js';
+import { redactAndWarnPolicySecrets } from './secret-redactor.js';
 import { logger } from '../lib/logger.js';
 import { buildResourceLabel } from '../lib/resource-uri.js';
 import { getNamePart } from '../lib/resource-path.js';
@@ -363,14 +364,15 @@ async function extractApiPolicy(
   const policyContent = properties?.value as string | undefined;
 
   if (policyContent) {
+    const redactedContent = redactAndWarnPolicySecrets(policyDescriptor, policyContent);
     await store.writeContent(
       outputDir,
       policyDescriptor,
-      policyContent,
+      redactedContent,
       'policy'
     );
     logger.debug(`Extracted ${buildResourceLabel(policyDescriptor)}`);
-    return policyContent;
+    return redactedContent;
   }
 
   return undefined;
@@ -420,13 +422,14 @@ async function extractApiOperations(
     const policyContent = properties?.value as string | undefined;
 
     if (policyContent) {
-      await store.writeContent(outputDir, opPolicyDescriptor, policyContent, 'policy');
+      const redactedContent = redactAndWarnPolicySecrets(opPolicyDescriptor, policyContent);
+      await store.writeContent(outputDir, opPolicyDescriptor, redactedContent, 'policy');
       operationPolicies.push({
         descriptor: opPolicyDescriptor,
         json: policyJson,
         status: 'success',
       });
-      policies.push(policyContent);
+      policies.push(redactedContent);
       logger.debug(`Extracted ${buildResourceLabel(opPolicyDescriptor)}`);
     }
   }
@@ -531,13 +534,14 @@ async function extractGraphQLResolvers(
     const policyContent = props?.value as string | undefined;
 
     if (policyContent) {
-      await store.writeContent(outputDir, resolverPolicyDescriptor, policyContent, 'policy');
+      const redactedContent = redactAndWarnPolicySecrets(resolverPolicyDescriptor, policyContent);
+      await store.writeContent(outputDir, resolverPolicyDescriptor, redactedContent, 'policy');
       resolverPolicies.push({
         descriptor: resolverPolicyDescriptor,
         json: policyJson,
         status: 'success',
       });
-      policies.push(policyContent);
+      policies.push(redactedContent);
       logger.debug(`Extracted ${buildResourceLabel(resolverPolicyDescriptor)}`);
     }
   }

src/services/extract-service.ts

@@ -29,6 +29,7 @@ import { extractWorkspaces, WorkspaceExtractionResult } from './workspace-extrac
 import {
   findTransitiveDependencies,
 } from './transitive-resolver.js';
+import { redactAndWarnPolicySecrets } from './secret-redactor.js';
 import { logger } from '../lib/logger.js';
 import { buildResourceLabel } from '../lib/resource-uri.js';
 import { EXIT_SUCCESS, EXIT_PARTIAL, EXIT_FATAL } from '../lib/exit-codes.js';
@@ -401,10 +402,11 @@ async function extractServicePolicy(
   const policyContent = properties?.value as string | undefined;
 
   if (policyContent) {
-    await store.writeContent(outputDir, descriptor, policyContent, 'policy');
+    const redactedContent = redactAndWarnPolicySecrets(descriptor, policyContent);
+    await store.writeContent(outputDir, descriptor, redactedContent, 'policy');
     result.totalExtracted++;
     result.extractedDescriptors.push(descriptor);
-    result.collectedPolicies.set('service-policy', policyContent);
+    result.collectedPolicies.set('service-policy', redactedContent);
     logger.info('Extracted service-level policy');
   }
 }

src/services/product-extractor.ts

@@ -11,6 +11,7 @@ import { ApimServiceContext, AssociationEntry, ResourceDescriptor } from '../mod
 import { ResourceType, RESOURCE_TYPE_METADATA } from '../models/resource-types.js';
 import { FilterConfig } from '../models/config.js';
 import { extractResourceName } from './resource-extractor.js';
+import { redactAndWarnPolicySecrets } from './secret-redactor.js';
 import { logger } from '../lib/logger.js';
 import { getNamePart } from '../lib/resource-path.js';
 import { isWorkspaceScope, extractNameFromLink, extractLinkTarget } from '../lib/workspace-link.js';
@@ -215,9 +216,10 @@ async function extractProductPolicy(
   const policyContent = properties?.value as string | undefined;
 
   if (policyContent) {
-    await store.writeContent(outputDir, policyDescriptor, policyContent, 'policy');
+    const redactedContent = redactAndWarnPolicySecrets(policyDescriptor, policyContent);
+    await store.writeContent(outputDir, policyDescriptor, redactedContent, 'policy');
     logger.debug(`Extracted policy for product "${getNamePart(productDescriptor.nameParts, 0)}"`);
-    return policyContent;
+    return redactedContent;
   }
 
   return undefined;

src/services/publish-service.ts

@@ -26,6 +26,8 @@ import { publishProduct } from './product-publisher.js';
 import { generateDryRunReport, DryRunReport } from './dry-run-reporter.js';
 import { computeDeleteActions } from './delete-unmatched-service.js';
 import { computeGitDiff } from './git-diff-service.js';
+import { scanForRedactionMarkers } from './secret-redaction-guard.js';
+import { REDACTION_MARKER } from './secret-redactor.js';
 
 /**
  * The APIM Backend properties.type value that identifies a pool backend.
@@ -79,6 +81,44 @@ export async function runPublish(
       `Publishing ${targetDescriptors.length} resources (dry-run: ${config.dryRun})`
     );
 
+    // Step 1b: Redaction pre-flight gate.
+    // Scan every artifact that would be published for leftover redaction markers
+    // ('*** REDACTED ***'). A single leftover marker aborts the ENTIRE publish
+    // before any PUT is issued — and also fails dry-run — so a service can never
+    // be left partially published with placeholder secrets.
+    const redactionFindings = await scanForRedactionMarkers(
+      store,
+      config,
+      targetDescriptors
+    );
+    if (redactionFindings.length > 0) {
+      logger.error(
+        `Publish aborted: ${redactionFindings.length} artifact(s) still contain the redaction marker '${REDACTION_MARKER}'. ` +
+        'Replace inline secrets with named values or KeyVault references before publishing: ' +
+        'https://learn.microsoft.com/en-us/azure/api-management/api-management-howto-properties'
+      );
+      const actions: PublishActionResult[] = redactionFindings.map((finding) => {
+        logger.error(`  - ${finding.label} (${finding.location})`);
+        return {
+          descriptor: finding.descriptor,
+          action: 'noop',
+          status: 'failed',
+          error: new Error(
+            `${finding.label} contains '${REDACTION_MARKER}' in ${finding.location}`
+          ),
+        };
+      });
+
+      return {
+        totalPuts: 0,
+        totalDeletes: 0,
+        totalErrors: actions.length,
+        totalSkipped: 0,
+        exitCode: EXIT_FATAL,
+        actions,
+      };
+    }
+
     // Step 2: Handle dry-run mode
     if (config.dryRun) {
       const dryRunReport = await generateDryRunReport(

src/services/resource-publisher.ts

@@ -21,6 +21,7 @@ import { logger } from '../lib/logger.js';
 import { REDACTION_MARKER } from './secret-redactor.js';
 import { isLinkAlreadyExistsError } from '../clients/apim-client.js';
 import type { OverrideConfig } from '../models/config.js';
+import { buildResourceLabel } from '../lib/resource-uri.js';
 
 export interface ResourcePublishResult {
   descriptor: ResourceDescriptor;
@@ -32,7 +33,7 @@ export interface ResourcePublishResult {
 /**
  * Policy resource types that have external XML content
  */
-const POLICY_TYPES = new Set<ResourceType>([
+export const POLICY_TYPES = new Set<ResourceType>([
   ResourceType.ServicePolicy,
   ResourceType.ProductPolicy,
   ResourceType.ApiPolicy,
@@ -416,6 +417,9 @@ 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<string, unknown> = {
       properties: {
         value: policyContent.content,
@@ -426,6 +430,19 @@ async function publishPolicy(
     // Apply overrides (e.g., format: xml) before PUT — matches Toolkit behavior
     const mergedPayload = applyOverrides(descriptor, payload, config.overrides);
 
+    // Marker check runs AFTER overrides: an override may legitimately replace the
+    // policy value with clean content, so only the merged (about-to-be-published)
+    // value is authoritative for redaction detection.
+    const mergedProps = mergedPayload.properties as Record<string, unknown> | undefined;
+    const mergedValue = mergedProps?.value;
+    if (typeof mergedValue === 'string' && mergedValue.includes(REDACTION_MARKER)) {
+      throw new Error(
+        `Cannot publish ${buildResourceLabel(descriptor)}: policy contains '${REDACTION_MARKER}'. ` +
+        'Replace inline secrets with named values before publish: ' +
+        'https://learn.microsoft.com/en-us/azure/api-management/api-management-howto-properties'
+      );
+    }
+
     await client.putResource(context, descriptor, mergedPayload);
 
     return {