Skip to content

Commit 85db9ef

Browse files
committed
Merge branch 'main' into upgradeAssessment
2 parents 4c48298 + 92c741f commit 85db9ef

1,463 files changed

Lines changed: 112699 additions & 2788 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.github/instructions/arm-api-review.instructions.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -909,6 +909,7 @@ When reviewing resources that support availability zones, verify: `zones` is a t
909909

910910
- The `name` property in the `checkNameAvailability` request **MUST** have `pattern` and `maxLength` constraints matching the target resource type's name validation rules.
911911
- At minimum, the `name` field **SHOULD** be constrained to valid ARM resource name characters (alphanumeric, hyphens, underscores, periods). Example: `"pattern": "^[a-zA-Z0-9][a-zA-Z0-9-_.]{0,259}$"`, `"maxLength": 260`.
912+
- The `pattern` constraint **MUST** use allowlist (positive character class) syntax. Denylist (negated character class `[^...]`) syntax **MUST NOT** be used as the primary character-matching construct (`OAPI-PATTERN-ALLOWLIST`). A denylist such as `^[^<>%&:\\?/]+$` permits emoji, control characters, and non-Latin scripts that the service almost certainly rejects. Severity: **Blocking** for new properties/parameters; **Warning** for existing ones. See [`.github/skills/azure-api-review/references/pattern-validation.md`](../skills/azure-api-review/references/pattern-validation.md) for detection guidance, the severity matrix, and fix examples.
912913
- If a spec defines a custom `checkNameAvailability` request model with a bare `string` `name` field (no `pattern`, no `maxLength`), flag it as a **security concern** and instruct the author to add constraints.
913914
- **Why:** Unconstrained string inputs to backend queries are a SQL/NoSQL injection risk. Input validation in the spec provides client-side SDK validation and signals to implementers that the field requires server-side sanitization.
914915

@@ -1151,6 +1152,8 @@ When reviewing ARM resource-manager swagger files, verify:
11511152

11521153
-`checkNameAvailability` uses common-types `CheckNameAvailabilityRequest` / `CheckNameAvailabilityResponse` — not custom request/response models (CNA-003)
11531154
-`name` field has `pattern` and `maxLength` constraints — not a bare unconstrained string (CNA-004)
1155+
-`name` field `pattern` uses allowlist syntax (positive character class) — not denylist (`[^...]`) (OAPI-PATTERN-ALLOWLIST, CNA-004)
1156+
- ✅ All `pattern` constraints across path parameters, query parameters, and body properties use allowlist syntax — no denylist (`[^...]`) as the primary construct (`OAPI-PATTERN-ALLOWLIST`): Blocking for new, Warning for existing
11541157

11551158
### Tenant-Level APIs
11561159

.github/instructions/openapi-review.instructions.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,7 @@ A finding "passes" the rule means the rule does not appear in the findings list
157157
- Prevention of leading special characters (e.g., `^(?![.-])`) — resource names should not start with `.` or `-`.
158158
- Allowed character set (e.g., `[A-Za-z0-9_.-]`) that matches the service's actual validation.
159159
- Example: `"pattern": "^(?![.-])[A-Za-z0-9_.-]{1,128}$"`
160+
- `pattern` constraints **MUST** use allowlist (positive character class) syntax. Denylist (negated character class `[^...]`) syntax **MUST NOT** be used as the primary character-matching construct (`OAPI-PATTERN-ALLOWLIST`). A negative lookahead (`(?!...)`) used alongside a positive class is acceptable (e.g., `^(?![.-])[A-Za-z0-9_.-]{1,128}$`). Severity: **Blocking** for new properties/parameters; **Warning** for existing ones carried forward from a prior API version. See [`.github/skills/azure-api-review/references/pattern-validation.md`](../skills/azure-api-review/references/pattern-validation.md) for detection guidance, the severity matrix, fix examples, and regression-risk notes.
160161
- For action operations on a resource, the URL pattern **SHOULD** be: `/<resource-collection>/<resource-id>:<action>`.
161162
- For action operations on a collection: `/<resource-collection>:<action>`.
162163
- **DO NOT** use action URLs for standard CRUD operations that map naturally to GET/PUT/PATCH/DELETE.
@@ -644,6 +645,7 @@ When reviewing, systematically check:
644645
- ✅ No unused/orphaned definitions in `definitions` section (SCHEMA-UNUSED-DEF)
645646
- ✅ Example files use string placeholders for secrets, SSH keys, and credentials — no realistic-looking sensitive values (EX-SECRET-PLACEHOLDER)
646647
- ✅ Resource name path parameters include `pattern` constraints with length limits and character restrictions
648+
- ✅ All `pattern` constraints use allowlist (positive character class) syntax — no denylist (`[^...]`) as the primary construct (`OAPI-PATTERN-ALLOWLIST`): Blocking for new properties/parameters, Warning for existing ones
647649
- ✅ PUT operation descriptions use idempotent language ("Creates or updates..." not "Creates a new...")
648650
- ✅ Operations API display object uses camelCase property names (`provider`, `resource`, `operation`, `description`)
649651

.github/instructions/typespec-review.instructions.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,17 @@ See the canonical contract in [`.github/skills/azure-api-review/references/revie
130130
set, and prevention of leading special characters.
131131
Example: `@pattern("^(?![.-])[A-Za-z0-9_.-]{1,128}$")`
132132
(Also enforced by: `@azure-tools/typespec-azure-resource-manager/arm-resource-name-pattern`)
133+
- `@pattern` decorator values and `NamePattern` constraints **MUST**
134+
use allowlist (positive character class) syntax. Denylist (negated
135+
character class `[^...]`) syntax **MUST NOT** be used as the primary
136+
character-matching construct (`OAPI-PATTERN-ALLOWLIST`). A negative
137+
lookahead (`(?!...)`) used alongside a positive class is acceptable
138+
(e.g., `@pattern("^(?![.-])[A-Za-z0-9_.-]{1,128}$")`). Severity:
139+
**Blocking** for new properties/parameters; **Warning** for existing
140+
ones carried forward from a prior API version. See
141+
[`.github/skills/azure-api-review/references/pattern-validation.md`](../skills/azure-api-review/references/pattern-validation.md)
142+
for detection guidance, severity matrix, fix examples, and
143+
regression-risk notes.
133144
- Properties representing UTC timestamps **SHOULD** include a `Utc`
134145
suffix in the name (e.g., `lastModifiedTimeUtc`).
135146
- Properties named `<something>Id` **MUST** be specific about what kind
@@ -444,3 +455,4 @@ When reviewing TypeSpec files, verify:
444455
- ✅ No bearer/OAuth tokens passed in ARM request bodies -- use managed identity or Key Vault
445456
- ✅ Generated OpenAPI files match `tsp compile .` output
446457
- ✅ Example files present for all operations, with realistic descriptive values (EX-DESCRIPTIVE-VALUES)
458+
- ✅ All `@pattern` and `NamePattern` constraints use allowlist (positive character class) syntax — no denylist (`[^...]`) as the primary construct (`OAPI-PATTERN-ALLOWLIST`): Blocking for new properties/parameters, Warning for existing ones

.github/shared/src/sdk-types.js

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,8 @@ export const SpecGenSdkArtifactInfoSchema = z.object({
4646
* breakingChange: string | undefined,
4747
* breakingChangeApproved: string | undefined,
4848
* breakingChangeSuppression: string | undefined,
49-
* breakingChangeSuppressionApproved: string | undefined
49+
* breakingChangeSuppressionApproved: string | undefined,
50+
* buildFailed: string | undefined
5051
* }} SdkLabelInfo
5152
*/
5253

@@ -55,7 +56,7 @@ export const SpecGenSdkArtifactInfoSchema = z.object({
5556
*/
5657

5758
/**
58-
* SDK labels mapping for breaking change labels
59+
* SDK labels mapping for breaking change and build-failure labels
5960
* @type {SdkLabels}
6061
* */
6162
export const sdkLabels = {
@@ -64,35 +65,41 @@ export const sdkLabels = {
6465
breakingChangeApproved: "BreakingChange-Go-Sdk-Approved",
6566
breakingChangeSuppression: "BreakingChange-Go-Sdk-Suppression",
6667
breakingChangeSuppressionApproved: "BreakingChange-Go-Sdk-Suppression-Approved",
68+
buildFailed: undefined,
6769
},
6870
"azure-sdk-for-java": {
6971
breakingChange: undefined,
7072
breakingChangeApproved: undefined,
7173
breakingChangeSuppression: undefined,
7274
breakingChangeSuppressionApproved: undefined,
75+
buildFailed: undefined,
7376
},
7477
"azure-sdk-for-js": {
7578
breakingChange: "BreakingChange-JavaScript-Sdk",
7679
breakingChangeApproved: "BreakingChange-JavaScript-Sdk-Approved",
7780
breakingChangeSuppression: "BreakingChange-JavaScript-Sdk-Suppression",
7881
breakingChangeSuppressionApproved: "BreakingChange-JavaScript-Sdk-Suppression-Approved",
82+
buildFailed: undefined,
7983
},
8084
"azure-sdk-for-net": {
8185
breakingChange: undefined,
8286
breakingChangeApproved: undefined,
8387
breakingChangeSuppression: undefined,
8488
breakingChangeSuppressionApproved: undefined,
89+
buildFailed: "auto-sdk-build-fix",
8590
},
8691
"azure-sdk-for-python": {
8792
breakingChange: "BreakingChange-Python-Sdk",
8893
breakingChangeApproved: "BreakingChange-Python-Sdk-Approved",
8994
breakingChangeSuppression: "BreakingChange-Python-Sdk-Suppression",
9095
breakingChangeSuppressionApproved: "BreakingChange-Python-Sdk-Suppression-Approved",
96+
buildFailed: undefined,
9197
},
9298
"azure-sdk-for-rust": {
9399
breakingChange: undefined,
94100
breakingChangeApproved: undefined,
95101
breakingChangeSuppression: undefined,
96102
breakingChangeSuppressionApproved: undefined,
103+
buildFailed: undefined,
97104
},
98105
};

.github/skills/azure-api-review/SKILL.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ Each reference file covers one cross-cutting rule area with:
4343
| [linter-rule-coverage.md](references/linter-rule-coverage.md) | Linter rule ID → instruction file mapping (130+ rules) | -- |
4444
| [example-quality.md](references/example-quality.md) | Example file quality: orphan detection, coverage, descriptive values | EX-ORPHAN, EX-COVERAGE, EX-DESCRIPTIVE-VALUES |
4545
| [design-decisions.md](references/design-decisions.md) | Grey-area design trade-off frameworks (10 decision matrices) | DD-001–DD-010 |
46+
| [pattern-validation.md](references/pattern-validation.md) | Allowlist vs. denylist `pattern` constraints; Unicode bypass risk; severity matrix | OAPI-PATTERN-ALLOWLIST |
4647

4748
## Authoritative External Sources
4849

.github/skills/azure-api-review/references/linter-rule-coverage.md

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,21 @@ instruction files.
260260
| -- | ResourceNameRestriction | arm-api-review §15.7 (PREFLIGHT-005) | ✅ Covered |
261261
| -- | TenantLevelAPIsNotAllowed | arm-api-review §12A (RPC-Uri-V1-11) | ✅ Annotated |
262262

263+
## Pattern Constraint Rules
264+
265+
| Linter Rule ID | Name | Instruction Coverage | Status |
266+
| -------------- | ------------------------ | ----------------------------------------------------------------- | -------------------- |
267+
| _(none)_ | DenylistPatternDetection | openapi-review §4 (OAPI-PATTERN-ALLOWLIST); pattern-validation.md | ❌ NO AUTOMATED RULE |
268+
269+
> **No automated linter rule** currently exists for `OAPI-PATTERN-ALLOWLIST`
270+
> in `@microsoft.azure/openapi-validator`, `@azure-tools/typespec-azure-core`,
271+
> or `@azure-tools/typespec-azure-resource-manager`. This check is enforced
272+
> exclusively by the ARM API Reviewer agent during code review. A follow-up
273+
> issue should be filed to add an automated rule to the OpenAPI validator and
274+
> TypeSpec linter packages. The existing `ResourceNameRestriction` rule
275+
> verifies that a `pattern` _exists_ on resource name parameters but does not
276+
> verify whether the pattern is an allowlist or a denylist.
277+
263278
---
264279

265280
## Coverage Summary
@@ -278,3 +293,9 @@ instruction files.
278293
| R2063 | OperationIdNounConflictingModelNames | openapi-review §7.1 |
279294
| R4006 | DeprecatedXmsCodeGenerationSetting | openapi-review §18 |
280295
| R2001 | AvoidNestedProperties | arm-api-review §8.2 |
296+
297+
### Missing Automated Linter Rules
298+
299+
| Rule ID | Name | Status | Follow-Up |
300+
| -------- | ------------------------ | -------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
301+
| _(none)_ | DenylistPatternDetection | ❌ No automated rule | File a follow-up issue to add `OAPI-PATTERN-ALLOWLIST` to `@azure-tools/typespec-azure-core` and `@microsoft.azure/openapi-validator`. Agent-level coverage: `openapi-review.instructions.md §4`, `typespec-review.instructions.md §2.2`, `arm-api-review.instructions.md §21.4`, and [`pattern-validation.md`](pattern-validation.md). |

0 commit comments

Comments
 (0)