diff --git a/packages/rulesets/CHANGELOG.md b/packages/rulesets/CHANGELOG.md index df8f5d01c..f6a03f1ac 100644 --- a/packages/rulesets/CHANGELOG.md +++ b/packages/rulesets/CHANGELOG.md @@ -1,5 +1,11 @@ # Change Log - @microsoft.azure/openapi-validator-rulesets +## 2.2.3 + +### Patches + +- [XMSSecretInResponse] Enhanced rule to check definitions properties, exclude enum properties (enum and x-ms-enum), exclude properties containing 'public' in their name, and report errors at definition level + ## 2.2.2 ### Patches diff --git a/packages/rulesets/generated/spectral/az-arm.js b/packages/rulesets/generated/spectral/az-arm.js index 5b1c4682b..374de3903 100644 --- a/packages/rulesets/generated/spectral/az-arm.js +++ b/packages/rulesets/generated/spectral/az-arm.js @@ -3259,10 +3259,13 @@ const XMSSecretInResponse = (properties, _opts, ctx) => { } else { if (isPotentialSensitiveProperty(prpName) && + !prpName.toLowerCase().includes("public") && properties[prpName] && properties[prpName]["x-ms-secret"] !== true && !keyValuePairCheck && - properties[prpName].type === "string") { + properties[prpName].type === "string" && + !properties[prpName].enum && + !properties[prpName]["x-ms-enum"]) { errors.push({ message: `Property '${prpName}' contains secret keyword and does not have 'x-ms-secret' annotation. To ensure security, must add the 'x-ms-secret' annotation to this property.`, path: [...path, prpName], @@ -3780,8 +3783,8 @@ const ruleset = { description: `When defining the response model for an ARM PUT/GET/POST operation, any property that contains sensitive information (such as passwords, keys, tokens, credentials, or other secrets) must include the "x-ms-secret": true annotation. This ensures that secrets are properly identified and handled according to ARM security guidelines.`, message: "{{error}}", severity: "error", - resolved: true, - given: ["$[paths,'x-ms-paths'].*.[put,get,post].responses.*.schema.properties"], + resolved: false, + given: ["$.definitions.*.properties"], then: { function: XMSSecretInResponse, }, diff --git a/packages/rulesets/package.json b/packages/rulesets/package.json index 7f49b0ffc..7dabf8a3a 100644 --- a/packages/rulesets/package.json +++ b/packages/rulesets/package.json @@ -1,6 +1,6 @@ { "name": "@microsoft.azure/openapi-validator-rulesets", - "version": "2.2.2", + "version": "2.2.3", "description": "Azure OpenAPI Validator", "main": "dist/index.js", "files": [ @@ -91,4 +91,4 @@ "^@stoplight/spectral-ruleset-bundler/(.*)$": "/node_modules/@stoplight/spectral-ruleset-bundler/dist/$1" } } -} +} \ No newline at end of file diff --git a/packages/rulesets/src/spectral/az-arm.ts b/packages/rulesets/src/spectral/az-arm.ts index a54ec970b..cab09555d 100644 --- a/packages/rulesets/src/spectral/az-arm.ts +++ b/packages/rulesets/src/spectral/az-arm.ts @@ -180,7 +180,8 @@ const ruleset: any = { // RPC Code: RPC-Async-V1-11, RPC-Async-V1-14 PostResponseCodes: { rpcGuidelineCode: "RPC-Async-V1-11, RPC-Async-V1-14, RPC-POST-V1-02, RPC-POST-V1-03", - description: "Synchronous POST must return 200 when a response body is required or 204 when no body is needed; LRO POST must initially return 202, with the final response returning 200 if a body is expected or 204 if not.", + description: + "Synchronous POST must return 200 when a response body is required or 204 when no body is needed; LRO POST must initially return 202, with the final response returning 200 if a body is expected or 204 if not.", severity: "error", disableForTypeSpecDataPlane: true, disableForTypeSpecDataPlaneReason: @@ -690,7 +691,7 @@ const ruleset: any = { message: "{{error}}", severity: "error", resolved: true, - given: ["$[paths,'x-ms-paths'].*.[put,get,post].responses.*.schema.properties"], + given: ["$.definitions.*.properties"], then: { function: XMSSecretInResponse, }, diff --git a/packages/rulesets/src/spectral/functions/xms-secret-in-response.ts b/packages/rulesets/src/spectral/functions/xms-secret-in-response.ts index 750890af2..0a355b571 100644 --- a/packages/rulesets/src/spectral/functions/xms-secret-in-response.ts +++ b/packages/rulesets/src/spectral/functions/xms-secret-in-response.ts @@ -33,10 +33,13 @@ export const XMSSecretInResponse = (properties: any, _opts: any, ctx: any) => { // Add all conditions for secret detection if ( isPotentialSensitiveProperty(prpName) && // property name matches sensitive keywords + !prpName.toLowerCase().includes("public") && // skip properties containing "public" properties[prpName] && // property exists properties[prpName]["x-ms-secret"] !== true && // not explicitly marked as secret !keyValuePairCheck && // not a key-value pair key - properties[prpName].type === "string" // property type is string + properties[prpName].type === "string" && // property type is string + !properties[prpName].enum && // not a standard enum property + !properties[prpName]["x-ms-enum"] // not an x-ms-enum property ) { errors.push({ message: `Property '${prpName}' contains secret keyword and does not have 'x-ms-secret' annotation. To ensure security, must add the 'x-ms-secret' annotation to this property.`, diff --git a/packages/rulesets/src/spectral/test/xms-secret-in-response.test.ts b/packages/rulesets/src/spectral/test/xms-secret-in-response.test.ts index 42adccb23..5cee4a766 100644 --- a/packages/rulesets/src/spectral/test/xms-secret-in-response.test.ts +++ b/packages/rulesets/src/spectral/test/xms-secret-in-response.test.ts @@ -4,6 +4,7 @@ import linterForRule from "./utils" let linter: Spectral const ERROR_MESSAGE = `Property '{prpName}' contains secret keyword and does not have 'x-ms-secret' annotation. To ensure security, must add the 'x-ms-secret' annotation to this property.` + beforeAll(async () => { linter = await linterForRule("XMSSecretInResponse") return linter @@ -191,43 +192,25 @@ test("XMSSecretInResponse should find errors", () => { }, } return linter.run(oasDoc).then((results) => { - expect(results.length).toBe(18) - expect(results[0].message).toBe(ERROR_MESSAGE.replace("{prpName}", "somepassword")) - expect(results[0].path.join(".")).toBe("paths./foo.get.responses.200.schema.properties.properties.properties.somepassword") - expect(results[1].message).toBe(ERROR_MESSAGE.replace("{prpName}", "sometoken")) - expect(results[1].path.join(".")).toBe("paths./foo.get.responses.200.schema.properties.properties.properties.sometoken") - expect(results[2].message).toBe(ERROR_MESSAGE.replace("{prpName}", "someaccess")) - expect(results[2].path.join(".")).toBe("paths./foo.get.responses.200.schema.properties.someaccess") - expect(results[3].message).toBe(ERROR_MESSAGE.replace("{prpName}", "someconnection")) - expect(results[3].path.join(".")).toBe("paths./foo.get.responses.200.schema.properties.someconnection") - expect(results[4].message).toBe(ERROR_MESSAGE.replace("{prpName}", "somecredential")) - expect(results[4].path.join(".")).toBe("paths./foo.get.responses.201.schema.properties.properties.properties.somecredential") - expect(results[5].message).toBe(ERROR_MESSAGE.replace("{prpName}", "somesecret")) - expect(results[5].path.join(".")).toBe("paths./foo.get.responses.201.schema.properties.properties.properties.somesecret") - expect(results[6].message).toBe(ERROR_MESSAGE.replace("{prpName}", "somepassword")) - expect(results[6].path.join(".")).toBe("paths./foo.post.responses.200.schema.properties.properties.properties.somepassword") - expect(results[7].message).toBe(ERROR_MESSAGE.replace("{prpName}", "sometoken")) - expect(results[7].path.join(".")).toBe("paths./foo.post.responses.200.schema.properties.properties.properties.sometoken") - expect(results[8].message).toBe(ERROR_MESSAGE.replace("{prpName}", "someaccess")) - expect(results[8].path.join(".")).toBe("paths./foo.post.responses.200.schema.properties.someaccess") - expect(results[9].message).toBe(ERROR_MESSAGE.replace("{prpName}", "someconnection")) - expect(results[9].path.join(".")).toBe("paths./foo.post.responses.200.schema.properties.someconnection") - expect(results[10].message).toBe(ERROR_MESSAGE.replace("{prpName}", "somecredential")) - expect(results[10].path.join(".")).toBe("paths./foo.post.responses.201.schema.properties.properties.properties.somecredential") - expect(results[11].message).toBe(ERROR_MESSAGE.replace("{prpName}", "somesecret")) - expect(results[11].path.join(".")).toBe("paths./foo.post.responses.201.schema.properties.properties.properties.somesecret") - expect(results[12].message).toBe(ERROR_MESSAGE.replace("{prpName}", "somepassword")) - expect(results[12].path.join(".")).toBe("paths./foo.put.responses.200.schema.properties.properties.properties.somepassword") - expect(results[13].message).toBe(ERROR_MESSAGE.replace("{prpName}", "sometoken")) - expect(results[13].path.join(".")).toBe("paths./foo.put.responses.200.schema.properties.properties.properties.sometoken") - expect(results[14].message).toBe(ERROR_MESSAGE.replace("{prpName}", "someaccess")) - expect(results[14].path.join(".")).toBe("paths./foo.put.responses.200.schema.properties.someaccess") - expect(results[15].message).toBe(ERROR_MESSAGE.replace("{prpName}", "someconnection")) - expect(results[15].path.join(".")).toBe("paths./foo.put.responses.200.schema.properties.someconnection") - expect(results[16].message).toBe(ERROR_MESSAGE.replace("{prpName}", "somecredential")) - expect(results[16].path.join(".")).toBe("paths./foo.put.responses.201.schema.properties.properties.properties.somecredential") - expect(results[17].message).toBe(ERROR_MESSAGE.replace("{prpName}", "somesecret")) - expect(results[17].path.join(".")).toBe("paths./foo.put.responses.201.schema.properties.properties.properties.somesecret") + expect(results.length).toBe(10) + const errorPaths = results.map((r) => r.path.join(".")) + expect(errorPaths).toContain("definitions.FooResource.properties.somepassword") + expect(errorPaths).toContain("definitions.FooResource.properties.sometoken") + expect(errorPaths).toContain("definitions.Foo.properties.somecredential") + expect(errorPaths).toContain("definitions.Foo.properties.somesecret") + expect(errorPaths).toContain("definitions.FooRule.properties.someaccess") + expect(errorPaths).toContain("definitions.FooRule.properties.someconnection") + expect(errorPaths).toContain("definitions.FooDefinition.properties.properties.properties.somecredential") + expect(errorPaths).toContain("definitions.FooDefinition.properties.properties.properties.somesecret") + expect(errorPaths).toContain("definitions.FooRule.properties.properties.properties.somepassword") + expect(errorPaths).toContain("definitions.FooRule.properties.properties.properties.sometoken") + + // Verify error messages + results.forEach((result) => { + const propertyName = String(result.path[result.path.length - 1]) + const expectedMessage = ERROR_MESSAGE.replace("{prpName}", propertyName) + expect(result.message).toBe(expectedMessage) + }) }) }) @@ -563,6 +546,30 @@ test("XMSSecretInResponse should find no errors", () => { type: "enum", enum: ["value1", "value2", "value3"], }, + // Test that properties with "public" in name are not flagged + publicNetworkAccess: { + type: "string", + description: "State of public network access.", + enum: ["Enabled", "Disabled"], + "x-ms-enum": { + name: "PublicNetworkAccess", + modelAsString: true, + }, + }, + publicKey: { + type: "string", + description: "The public key for authentication.", + }, + isPublicConnection: { + type: "string", + description: "Indicates if connection is public.", + }, + // Test that enum properties (without x-ms-enum) are not flagged + networkAccessEnum: { + type: "string", + description: "Network access configuration.", + enum: ["Allow", "Deny"], + }, }, required: ["properties"], },