feat: add bug reproduction and workaround scripts for importCertificate policy.contentType issue
This commit is contained in:
@@ -0,0 +1,209 @@
|
||||
# Bug Reports
|
||||
|
||||
## `@azure/keyvault-certificates` — `CertificateClient.importCertificate` silently drops `policy.contentType`
|
||||
|
||||
**Package:** `@azure/keyvault-certificates`
|
||||
**Confirmed versions:** 4.10.3, current `main` branch
|
||||
**Source file:** `sdk/keyvault/keyvault-certificates/src/index.ts`
|
||||
**Serializer file:** `sdk/keyvault/keyvault-certificates/src/models/models.ts`
|
||||
|
||||
---
|
||||
|
||||
### Summary
|
||||
|
||||
`CertificateClient.importCertificate()` accepts `ImportCertificateOptions.policy.contentType` as part of its public API but never sends it to Azure. The value is silently dropped due to a key name mismatch between the high-level client and the generated REST serializer. As a result, Azure falls back to the existing stored policy for that certificate name. When that policy specifies a different format than the bytes being imported (e.g. importing PFX into a certificate previously stored as PEM), Azure rejects the request with:
|
||||
|
||||
> `The specified PEM X.509 certificate content is in an unexpected format.`
|
||||
|
||||
---
|
||||
|
||||
### Root Cause
|
||||
|
||||
In `index.ts`, `importCertificate` builds the parameters object for the low-level generated client by spreading `updatedOptions`:
|
||||
|
||||
```typescript
|
||||
const result = await this.client.importCertificate(
|
||||
certificateName,
|
||||
{
|
||||
base64EncodedCertificate,
|
||||
preserveCertOrder: updatedOptions.preserveCertificateOrder,
|
||||
...updatedOptions, // contributes: policy, password, tags, ...
|
||||
},
|
||||
updatedOptions,
|
||||
);
|
||||
```
|
||||
|
||||
The spread adds the property as `policy` (the public API name from `ImportCertificateOptions`).
|
||||
|
||||
In `models/models.ts`, the generated serializer reads a **different** key name:
|
||||
|
||||
```typescript
|
||||
export function certificateImportParametersSerializer(item: CertificateImportParameters): any {
|
||||
return {
|
||||
value: item["base64EncodedCertificate"],
|
||||
pwd: item["password"],
|
||||
policy: !item["certificatePolicy"] // reads "certificatePolicy", not "policy"
|
||||
? item["certificatePolicy"]
|
||||
: certificatePolicySerializer(item["certificatePolicy"]),
|
||||
attributes: !item["certificateAttributes"]
|
||||
? item["certificateAttributes"]
|
||||
: certificateAttributesSerializer(item["certificateAttributes"]),
|
||||
tags: item["tags"],
|
||||
preserveCertOrder: item["preserveCertOrder"],
|
||||
};
|
||||
}
|
||||
```
|
||||
|
||||
`item["certificatePolicy"]` is always `undefined` because the spread only populated `item["policy"]`. The REST body is sent with `policy: null`, regardless of what the caller specified.
|
||||
|
||||
**Note:** `password` is not affected — it is read as `item["password"]` which matches the spread key and is correctly transmitted.
|
||||
|
||||
**Note:** The Python SDK does not have this bug — it uses a different serialization architecture.
|
||||
|
||||
---
|
||||
|
||||
### Effect
|
||||
|
||||
`ImportCertificateOptions.policy` is effectively a no-op. Any value passed is ignored. The observable consequence:
|
||||
|
||||
- Importing a certificate for the first time works, because Azure auto-detects the format from the bytes (PEM headers are recognisable; PFX ASN.1 magic bytes may also be detected).
|
||||
- Importing a **new version** of an existing certificate in a **different format** fails: Azure validates the incoming bytes against the stored policy's `content_type`, which no longer matches.
|
||||
|
||||
---
|
||||
|
||||
### Reproduction
|
||||
|
||||
```typescript
|
||||
import { DefaultAzureCredential } from "@azure/identity";
|
||||
import { CertificateClient } from "@azure/keyvault-certificates";
|
||||
import { readFileSync } from "fs";
|
||||
|
||||
const credential = new DefaultAzureCredential();
|
||||
const client = new CertificateClient(
|
||||
"https://<YOUR_VAULT>.vault.azure.net",
|
||||
credential,
|
||||
);
|
||||
|
||||
// Step 1: import a PEM certificate (works — Azure auto-detects PEM)
|
||||
const pemBytes = Buffer.from(readFileSync("cert.pem", "utf8"));
|
||||
await client.importCertificate("MyCert", pemBytes, {
|
||||
policy: { contentType: "application/x-pem-file" },
|
||||
});
|
||||
|
||||
// Step 2: import the same certificate as PFX (fails — policy.contentType is dropped,
|
||||
// Azure uses existing PEM policy and rejects the binary PFX bytes)
|
||||
const pfxBytes = readFileSync("cert.pfx");
|
||||
await client.importCertificate("MyCert", pfxBytes, {
|
||||
password: "pfx-password",
|
||||
policy: { contentType: "application/x-pkcs12" },
|
||||
});
|
||||
// ^ throws: "The specified PEM X.509 certificate content is in an unexpected format."
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Fix
|
||||
|
||||
In `sdk/keyvault/keyvault-certificates/src/index.ts`, explicitly map the public `policy` field to the internal `certificatePolicy` key after the spread:
|
||||
|
||||
```typescript
|
||||
// BEFORE
|
||||
const result = await this.client.importCertificate(
|
||||
certificateName,
|
||||
{
|
||||
base64EncodedCertificate,
|
||||
preserveCertOrder: updatedOptions.preserveCertificateOrder,
|
||||
...updatedOptions,
|
||||
},
|
||||
updatedOptions,
|
||||
);
|
||||
|
||||
// AFTER
|
||||
const result = await this.client.importCertificate(
|
||||
certificateName,
|
||||
{
|
||||
base64EncodedCertificate,
|
||||
preserveCertOrder: updatedOptions.preserveCertificateOrder,
|
||||
...updatedOptions,
|
||||
certificatePolicy: updatedOptions.policy, // map public name → serializer key
|
||||
},
|
||||
updatedOptions,
|
||||
);
|
||||
```
|
||||
|
||||
No changes required to the serializer, the public `ImportCertificateOptions` interface, or any other file.
|
||||
|
||||
---
|
||||
|
||||
### Test
|
||||
|
||||
Place in `sdk/keyvault/keyvault-certificates/test/` alongside the existing import tests. The test intercepts the outgoing HTTP request and asserts that `policy.secret_props.content_type` is present in the body.
|
||||
|
||||
```typescript
|
||||
import { assert } from "@azure/test-utils";
|
||||
import { CertificateClient } from "../../src/index.js";
|
||||
import { createTestCredential } from "@azure-tools/test-credential";
|
||||
import { HttpClient, PipelineRequest, PipelineResponse } from "@azure/core-rest-pipeline";
|
||||
|
||||
function makeFakeResponse(): PipelineResponse {
|
||||
return {
|
||||
status: 200,
|
||||
headers: { get: () => "application/json", set: () => {}, has: () => true, delete: () => {}, toJSON: () => ({}) } as any,
|
||||
bodyAsText: JSON.stringify({
|
||||
id: "https://vault.azure.net/certificates/MyCert/abc123",
|
||||
cer: Buffer.alloc(0).toString("base64"),
|
||||
attributes: { enabled: true },
|
||||
policy: {
|
||||
secret_props: { contentType: "application/x-pkcs12" },
|
||||
issuer: { name: "Unknown" },
|
||||
},
|
||||
}),
|
||||
request: null as any,
|
||||
};
|
||||
}
|
||||
|
||||
it("importCertificate sends policy.contentType in the REST body", async () => {
|
||||
let capturedBody: any;
|
||||
|
||||
const fakeHttpClient: HttpClient = {
|
||||
sendRequest: async (request: PipelineRequest): Promise<PipelineResponse> => {
|
||||
capturedBody = JSON.parse(request.body as string);
|
||||
return makeFakeResponse();
|
||||
},
|
||||
};
|
||||
|
||||
const client = new CertificateClient(
|
||||
"https://fakevault.vault.azure.net",
|
||||
createTestCredential(),
|
||||
{ httpClient: fakeHttpClient },
|
||||
);
|
||||
|
||||
const pfxBytes = Buffer.from("fakepfxbytes");
|
||||
|
||||
await client.importCertificate("MyCert", pfxBytes, {
|
||||
password: "secret",
|
||||
policy: { contentType: "application/x-pkcs12" },
|
||||
});
|
||||
|
||||
assert.isDefined(
|
||||
capturedBody?.policy?.secret_props?.contentType,
|
||||
"policy.secret_props.contentType must be present in the REST body",
|
||||
);
|
||||
assert.strictEqual(
|
||||
capturedBody.policy.secret_props.contentType,
|
||||
"application/x-pkcs12",
|
||||
"contentType must match the value passed in ImportCertificateOptions.policy",
|
||||
);
|
||||
});
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Contribution Steps
|
||||
|
||||
1. Search [azure-sdk-for-js issues](https://github.com/Azure/azure-sdk-for-js/issues) for `importCertificate policy contentType` — file a new issue if none exists.
|
||||
2. Fork `Azure/azure-sdk-for-js` on GitHub.
|
||||
3. Clone your fork locally.
|
||||
4. Apply the one-line fix in `sdk/keyvault/keyvault-certificates/src/index.ts`.
|
||||
5. Add the test above to the existing import test suite.
|
||||
6. Open a PR referencing the issue, with this document as the description basis.
|
||||
Reference in New Issue
Block a user