Skip to content

Commit 662299e

Browse files
tejaswiMinnutejaswiMinnu
authored
Updated the rule so it doesn't error on enums or public properties (#824)
* Updated the rule so it doesn't error on enums or public properties * fix test failure
1 parent 6069a60 commit 662299e

6 files changed

Lines changed: 65 additions & 45 deletions

File tree

‎packages/rulesets/CHANGELOG.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
# Change Log - @microsoft.azure/openapi-validator-rulesets
22

3+
## 2.2.3
4+
5+
### Patches
6+
7+
- [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
8+
39
## 2.2.2
410

511
### Patches

‎packages/rulesets/generated/spectral/az-arm.js‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3259,10 +3259,13 @@ const XMSSecretInResponse = (properties, _opts, ctx) => {
32593259
}
32603260
else {
32613261
if (isPotentialSensitiveProperty(prpName) &&
3262+
!prpName.toLowerCase().includes("public") &&
32623263
properties[prpName] &&
32633264
properties[prpName]["x-ms-secret"] !== true &&
32643265
!keyValuePairCheck &&
3265-
properties[prpName].type === "string") {
3266+
properties[prpName].type === "string" &&
3267+
!properties[prpName].enum &&
3268+
!properties[prpName]["x-ms-enum"]) {
32663269
errors.push({
32673270
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.`,
32683271
path: [...path, prpName],
@@ -3780,8 +3783,8 @@ const ruleset = {
37803783
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.`,
37813784
message: "{{error}}",
37823785
severity: "error",
3783-
resolved: true,
3784-
given: ["$[paths,'x-ms-paths'].*.[put,get,post].responses.*.schema.properties"],
3786+
resolved: false,
3787+
given: ["$.definitions.*.properties"],
37853788
then: {
37863789
function: XMSSecretInResponse,
37873790
},

‎packages/rulesets/package.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@microsoft.azure/openapi-validator-rulesets",
3-
"version": "2.2.2",
3+
"version": "2.2.3",
44
"description": "Azure OpenAPI Validator",
55
"main": "dist/index.js",
66
"files": [
@@ -91,4 +91,4 @@
9191
"^@stoplight/spectral-ruleset-bundler/(.*)$": "<rootDir>/node_modules/@stoplight/spectral-ruleset-bundler/dist/$1"
9292
}
9393
}
94-
}
94+
}

‎packages/rulesets/src/spectral/az-arm.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,8 @@ const ruleset: any = {
180180
// RPC Code: RPC-Async-V1-11, RPC-Async-V1-14
181181
PostResponseCodes: {
182182
rpcGuidelineCode: "RPC-Async-V1-11, RPC-Async-V1-14, RPC-POST-V1-02, RPC-POST-V1-03",
183-
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.",
183+
description:
184+
"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.",
184185
severity: "error",
185186
disableForTypeSpecDataPlane: true,
186187
disableForTypeSpecDataPlaneReason:
@@ -690,7 +691,7 @@ const ruleset: any = {
690691
message: "{{error}}",
691692
severity: "error",
692693
resolved: true,
693-
given: ["$[paths,'x-ms-paths'].*.[put,get,post].responses.*.schema.properties"],
694+
given: ["$.definitions.*.properties"],
694695
then: {
695696
function: XMSSecretInResponse,
696697
},

‎packages/rulesets/src/spectral/functions/xms-secret-in-response.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,13 @@ export const XMSSecretInResponse = (properties: any, _opts: any, ctx: any) => {
3333
// Add all conditions for secret detection
3434
if (
3535
isPotentialSensitiveProperty(prpName) && // property name matches sensitive keywords
36+
!prpName.toLowerCase().includes("public") && // skip properties containing "public"
3637
properties[prpName] && // property exists
3738
properties[prpName]["x-ms-secret"] !== true && // not explicitly marked as secret
3839
!keyValuePairCheck && // not a key-value pair key
39-
properties[prpName].type === "string" // property type is string
40+
properties[prpName].type === "string" && // property type is string
41+
!properties[prpName].enum && // not a standard enum property
42+
!properties[prpName]["x-ms-enum"] // not an x-ms-enum property
4043
) {
4144
errors.push({
4245
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.`,

‎packages/rulesets/src/spectral/test/xms-secret-in-response.test.ts‎

Lines changed: 44 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import linterForRule from "./utils"
44
let linter: Spectral
55

66
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.`
7+
78
beforeAll(async () => {
89
linter = await linterForRule("XMSSecretInResponse")
910
return linter
@@ -191,43 +192,25 @@ test("XMSSecretInResponse should find errors", () => {
191192
},
192193
}
193194
return linter.run(oasDoc).then((results) => {
194-
expect(results.length).toBe(18)
195-
expect(results[0].message).toBe(ERROR_MESSAGE.replace("{prpName}", "somepassword"))
196-
expect(results[0].path.join(".")).toBe("paths./foo.get.responses.200.schema.properties.properties.properties.somepassword")
197-
expect(results[1].message).toBe(ERROR_MESSAGE.replace("{prpName}", "sometoken"))
198-
expect(results[1].path.join(".")).toBe("paths./foo.get.responses.200.schema.properties.properties.properties.sometoken")
199-
expect(results[2].message).toBe(ERROR_MESSAGE.replace("{prpName}", "someaccess"))
200-
expect(results[2].path.join(".")).toBe("paths./foo.get.responses.200.schema.properties.someaccess")
201-
expect(results[3].message).toBe(ERROR_MESSAGE.replace("{prpName}", "someconnection"))
202-
expect(results[3].path.join(".")).toBe("paths./foo.get.responses.200.schema.properties.someconnection")
203-
expect(results[4].message).toBe(ERROR_MESSAGE.replace("{prpName}", "somecredential"))
204-
expect(results[4].path.join(".")).toBe("paths./foo.get.responses.201.schema.properties.properties.properties.somecredential")
205-
expect(results[5].message).toBe(ERROR_MESSAGE.replace("{prpName}", "somesecret"))
206-
expect(results[5].path.join(".")).toBe("paths./foo.get.responses.201.schema.properties.properties.properties.somesecret")
207-
expect(results[6].message).toBe(ERROR_MESSAGE.replace("{prpName}", "somepassword"))
208-
expect(results[6].path.join(".")).toBe("paths./foo.post.responses.200.schema.properties.properties.properties.somepassword")
209-
expect(results[7].message).toBe(ERROR_MESSAGE.replace("{prpName}", "sometoken"))
210-
expect(results[7].path.join(".")).toBe("paths./foo.post.responses.200.schema.properties.properties.properties.sometoken")
211-
expect(results[8].message).toBe(ERROR_MESSAGE.replace("{prpName}", "someaccess"))
212-
expect(results[8].path.join(".")).toBe("paths./foo.post.responses.200.schema.properties.someaccess")
213-
expect(results[9].message).toBe(ERROR_MESSAGE.replace("{prpName}", "someconnection"))
214-
expect(results[9].path.join(".")).toBe("paths./foo.post.responses.200.schema.properties.someconnection")
215-
expect(results[10].message).toBe(ERROR_MESSAGE.replace("{prpName}", "somecredential"))
216-
expect(results[10].path.join(".")).toBe("paths./foo.post.responses.201.schema.properties.properties.properties.somecredential")
217-
expect(results[11].message).toBe(ERROR_MESSAGE.replace("{prpName}", "somesecret"))
218-
expect(results[11].path.join(".")).toBe("paths./foo.post.responses.201.schema.properties.properties.properties.somesecret")
219-
expect(results[12].message).toBe(ERROR_MESSAGE.replace("{prpName}", "somepassword"))
220-
expect(results[12].path.join(".")).toBe("paths./foo.put.responses.200.schema.properties.properties.properties.somepassword")
221-
expect(results[13].message).toBe(ERROR_MESSAGE.replace("{prpName}", "sometoken"))
222-
expect(results[13].path.join(".")).toBe("paths./foo.put.responses.200.schema.properties.properties.properties.sometoken")
223-
expect(results[14].message).toBe(ERROR_MESSAGE.replace("{prpName}", "someaccess"))
224-
expect(results[14].path.join(".")).toBe("paths./foo.put.responses.200.schema.properties.someaccess")
225-
expect(results[15].message).toBe(ERROR_MESSAGE.replace("{prpName}", "someconnection"))
226-
expect(results[15].path.join(".")).toBe("paths./foo.put.responses.200.schema.properties.someconnection")
227-
expect(results[16].message).toBe(ERROR_MESSAGE.replace("{prpName}", "somecredential"))
228-
expect(results[16].path.join(".")).toBe("paths./foo.put.responses.201.schema.properties.properties.properties.somecredential")
229-
expect(results[17].message).toBe(ERROR_MESSAGE.replace("{prpName}", "somesecret"))
230-
expect(results[17].path.join(".")).toBe("paths./foo.put.responses.201.schema.properties.properties.properties.somesecret")
195+
expect(results.length).toBe(10)
196+
const errorPaths = results.map((r) => r.path.join("."))
197+
expect(errorPaths).toContain("definitions.FooResource.properties.somepassword")
198+
expect(errorPaths).toContain("definitions.FooResource.properties.sometoken")
199+
expect(errorPaths).toContain("definitions.Foo.properties.somecredential")
200+
expect(errorPaths).toContain("definitions.Foo.properties.somesecret")
201+
expect(errorPaths).toContain("definitions.FooRule.properties.someaccess")
202+
expect(errorPaths).toContain("definitions.FooRule.properties.someconnection")
203+
expect(errorPaths).toContain("definitions.FooDefinition.properties.properties.properties.somecredential")
204+
expect(errorPaths).toContain("definitions.FooDefinition.properties.properties.properties.somesecret")
205+
expect(errorPaths).toContain("definitions.FooRule.properties.properties.properties.somepassword")
206+
expect(errorPaths).toContain("definitions.FooRule.properties.properties.properties.sometoken")
207+
208+
// Verify error messages
209+
results.forEach((result) => {
210+
const propertyName = String(result.path[result.path.length - 1])
211+
const expectedMessage = ERROR_MESSAGE.replace("{prpName}", propertyName)
212+
expect(result.message).toBe(expectedMessage)
213+
})
231214
})
232215
})
233216

@@ -563,6 +546,30 @@ test("XMSSecretInResponse should find no errors", () => {
563546
type: "enum",
564547
enum: ["value1", "value2", "value3"],
565548
},
549+
// Test that properties with "public" in name are not flagged
550+
publicNetworkAccess: {
551+
type: "string",
552+
description: "State of public network access.",
553+
enum: ["Enabled", "Disabled"],
554+
"x-ms-enum": {
555+
name: "PublicNetworkAccess",
556+
modelAsString: true,
557+
},
558+
},
559+
publicKey: {
560+
type: "string",
561+
description: "The public key for authentication.",
562+
},
563+
isPublicConnection: {
564+
type: "string",
565+
description: "Indicates if connection is public.",
566+
},
567+
// Test that enum properties (without x-ms-enum) are not flagged
568+
networkAccessEnum: {
569+
type: "string",
570+
description: "Network access configuration.",
571+
enum: ["Allow", "Deny"],
572+
},
566573
},
567574
required: ["properties"],
568575
},

0 commit comments

Comments
 (0)
⚡