fix: reject non-alphanumeric SN codes in POST /v1/devices#10
fix: reject non-alphanumeric SN codes in POST /v1/devices#10Washio20 wants to merge 1 commit intostayforge:mainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdded regex pattern Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
openapi.yaml (1)
1047-1047: Consider addingmaxLengthto schema properties for consistency (optional).The path parameter at
/v1/devices/sn/{sn}includesmaxLength: 256, but the schema properties (DeviceProperties.sn,DeviceInput.sn, etc.) don't specify a maximum length. While this likely doesn't cause functional issues (database constraints may enforce this), addingmaxLengthwould improve consistency and provide clearer API documentation for clients.Also applies to: 5780-5780, 5945-5945, 5968-5968, 5989-5989
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openapi.yaml` at line 1047, Add a maxLength constraint to the string schema properties for device serial numbers to match the path parameter; update the relevant schemas (e.g., DeviceProperties.sn, DeviceInput.sn and any other schemas that define a "sn" property referenced in the diff ranges) to include maxLength: 256 while keeping the existing pattern: ^[a-zA-Z0-9_-]+$ so docs and validation are consistent with the /v1/devices/sn/{sn} parameter.openapi.json (1)
7451-7453: Consider adding explicit length bounds for request-body SNs.The path parameter caps SN at 256, but request-body SNs don’t. Aligning limits keeps validation consistent and avoids overly large payloads. Consider applying the same bounds to DeviceProperties/DeviceUpdate/DeviceInfo too.
♻️ Suggested change (apply similarly to other SN schemas)
"sn": { "type": "string", + "minLength": 1, + "maxLength": 256, "pattern": "^[a-zA-Z0-9_-]+$", "description": "Serial number of the device. This field is required when creating a device." }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openapi.json` around lines 7451 - 7453, Add explicit length bounds to the request-body serial number string schemas to match the path param cap: update the SN schema(s) used in DeviceProperties, DeviceUpdate, DeviceInfo (and any other request-body SN definitions) to include "maxLength": 256 (and a sensible "minLength": 1 if empty SNs are invalid) while keeping the existing "pattern": "^[a-zA-Z0-9_-]+$" and "type": "string" so validation is consistent with the path parameter.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@openapi.json`:
- Around line 7451-7453: Add explicit length bounds to the request-body serial
number string schemas to match the path param cap: update the SN schema(s) used
in DeviceProperties, DeviceUpdate, DeviceInfo (and any other request-body SN
definitions) to include "maxLength": 256 (and a sensible "minLength": 1 if empty
SNs are invalid) while keeping the existing "pattern": "^[a-zA-Z0-9_-]+$" and
"type": "string" so validation is consistent with the path parameter.
In `@openapi.yaml`:
- Line 1047: Add a maxLength constraint to the string schema properties for
device serial numbers to match the path parameter; update the relevant schemas
(e.g., DeviceProperties.sn, DeviceInput.sn and any other schemas that define a
"sn" property referenced in the diff ranges) to include maxLength: 256 while
keeping the existing pattern: ^[a-zA-Z0-9_-]+$ so docs and validation are
consistent with the /v1/devices/sn/{sn} parameter.
There was a problem hiding this comment.
Pull request overview
Adds OpenAPI-level regex validation for device serial number (sn) fields so invalid character sets (e.g., Chinese characters) are rejected during request validation, addressing Issue #3.
Changes:
- Adds
pattern: ^[a-zA-Z0-9_-]+$to/v1/devices/sn/{sn}path parameter schema. - Adds the same
patternconstraint tosnfields acrossDeviceProperties,DeviceCreate/DeviceInput,DeviceUpdate, andDeviceInfoschemas. - Mirrors the updates in both
openapi.yamlandopenapi.json.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| openapi.yaml | Adds pattern validation to all sn definitions in the YAML OpenAPI spec. |
| openapi.json | Mirrors the same pattern validation updates in the JSON OpenAPI spec. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| type: string | ||
| minLength: 1 | ||
| maxLength: 256 | ||
| pattern: ^[a-zA-Z0-9_-]+$ | ||
| description: Device serial number |
There was a problem hiding this comment.
The PR title/description says SN should reject “non-alphanumeric characters”, but the new regex ^[a-zA-Z0-9_-]+$ still allows _ and - (which are non-alphanumeric). Either tighten the pattern to letters+digits only, or update the PR title/docs to explicitly state that underscores/hyphens are allowed.
| @@ -5776,6 +5777,7 @@ components: | |||
| sn: | |||
| type: string | |||
| nullable: true | |||
There was a problem hiding this comment.
SN validation is now applied via pattern, but the request-body schemas don’t define minLength/maxLength for sn (while the /v1/devices/sn/{sn} path parameter enforces 1..256). This creates inconsistent constraints across endpoints and still allows arbitrarily large SN strings in POST/PATCH bodies. Consider adding consistent length limits (e.g., minLength 1 and maxLength 256) to all sn schema occurrences.
| nullable: true | |
| nullable: true | |
| minLength: 1 | |
| maxLength: 256 |
| @@ -7308,6 +7309,7 @@ | |||
| "sn": { | |||
| "type": "string", | |||
| "nullable": true, | |||
There was a problem hiding this comment.
In JSON, the /v1/devices/sn/{sn} parameter enforces maxLength: 256, but the sn fields in request-body schemas (e.g., DeviceProperties / DeviceInput / DeviceUpdate) still lack minLength/maxLength. That means POST/PATCH can accept very long SN strings that can’t be used with the SN lookup endpoint and may increase validation/payload costs. Align the sn length constraints across schemas (e.g., minLength 1, maxLength 256).
| "nullable": true, | |
| "nullable": true, | |
| "minLength": 1, | |
| "maxLength": 256, |
Washio20
changed the title
Washio20
changed the title
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
openapi.yaml (1)
1040-1048:⚠️ Potential issue | 🟡 MinorMissing pattern validation on path parameter.
The
snpath parameter forGET /v1/devices/sn/{sn}lacks the same regex pattern constraint added to the schema definitions. Per the PR objectives, this endpoint should also reject invalid SN codes like "SN202501是2002".Proposed fix
- name: sn in: path required: true schema: type: string minLength: 1 maxLength: 256 + pattern: ^[a-zA-Z0-9_-]+$ description: Device serial number example: SFVA78RABZ12345678🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openapi.yaml` around lines 1040 - 1048, The path parameter "sn" for the GET /v1/devices/sn/{sn} operation is missing the regex pattern constraint; update the OpenAPI parameter definition for "sn" to include the same pattern used in the shared device SN schema (the regex that disallows non-ASCII/Chinese characters and matches valid SN formats) so requests like "SN202501是2002" are rejected, ensuring the parameter keeps type: string, minLength/maxLength and adds pattern with the identical token used in the schema definitions.openapi.json (1)
1379-1388:⚠️ Potential issue | 🟠 MajorAdd SN pattern validation to
GET /v1/devices/sn/{sn}path parameter.Line 1384-Line 1388 defines
snwithout a regex constraint, so this endpoint is still not guaranteed to reject invalid SN formats as required by the issue objective.🔧 Proposed fix
"schema": { "type": "string", "minLength": 1, - "maxLength": 256 + "maxLength": 256, + "pattern": "^[a-zA-Z0-9_-]+$" },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openapi.json` around lines 1379 - 1388, The path parameter "sn" for GET /v1/devices/sn/{sn} lacks a regex constraint; update the parameter schema for name "sn" (in the GET /v1/devices/sn/{sn} operation) to include a "pattern" property enforcing the required SN format (keeping the existing "type", "minLength" and "maxLength"), e.g., a regex that matches the allowed characters and structure for SNs so invalid SNs are rejected by the OpenAPI validation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@openapi.json`:
- Around line 1379-1388: The path parameter "sn" for GET /v1/devices/sn/{sn}
lacks a regex constraint; update the parameter schema for name "sn" (in the GET
/v1/devices/sn/{sn} operation) to include a "pattern" property enforcing the
required SN format (keeping the existing "type", "minLength" and "maxLength"),
e.g., a regex that matches the allowed characters and structure for SNs so
invalid SNs are rejected by the OpenAPI validation.
In `@openapi.yaml`:
- Around line 1040-1048: The path parameter "sn" for the GET /v1/devices/sn/{sn}
operation is missing the regex pattern constraint; update the OpenAPI parameter
definition for "sn" to include the same pattern used in the shared device SN
schema (the regex that disallows non-ASCII/Chinese characters and matches valid
SN formats) so requests like "SN202501是2002" are rejected, ensuring the
parameter keeps type: string, minLength/maxLength and adds pattern with the
identical token used in the schema definitions.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
openapi.yaml (1)
1051-1133:⚠️ Potential issue | 🟡 MinorDocument 400 for invalid
snpath values on SN lookup.With path validation at Line 1047, invalid SN format should be explicitly documented as a 400 response for
GET /v1/devices/sn/{sn}; currently the response list starts at 401.💡 Proposed fix
responses: + '400': + description: 'Bad request - Invalid serial number path parameter format. + + Error codes: `invalid_path_parameter` + + ' + content: + application/json: + schema: + $ref: '#/components/schemas/Error' + examples: + invalid_path_parameter: + value: + code: invalid_path_parameter + message: sn must match pattern ^[A-Za-z0-9_-]+$ '200': description: Device information content: application/json: schema:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openapi.yaml` around lines 1051 - 1133, Add a 400 response for the GetDeviceSN operation (operationId: GetDeviceSN, GET /v1/devices/sn/{sn}) to document invalid path/`sn` format validation failures: include a response entry '400' with description like "Bad Request - invalid serial number format", reuse the existing Error schema ($ref: '#/components/schemas/Error'), and provide an example error object (e.g., code: invalid_sn, message: "invalid serial number format") so consumers know that malformed `sn` path values return 400.
🧹 Nitpick comments (1)
openapi.json (1)
7310-7314: Consider centralizing SN schema to prevent drift.The same SN rule is duplicated in several schemas. Extracting it into a shared component (e.g.,
SerialNumberSchema) will reduce maintenance risk and keep docs identical everywhere.♻️ Suggested refactor
+ "SerialNumberSchema": { + "type": "string", + "pattern": "^[A-Za-z0-9_-]+$", + "description": "Serial number. Must contain only ASCII alphanumeric characters, hyphens, or underscores." + }, "DeviceProperties": { "type": "object", "properties": { "sn": { - "type": "string", - "nullable": true, - "pattern": "^[A-Za-z0-9_-]+$", - "description": "Serial number of the device. Must contain only ASCII alphanumeric characters, hyphens, or underscores." + "allOf": [ + { "$ref": "#/components/schemas/SerialNumberSchema" } + ], + "nullable": true } } }, "DeviceInput": { "properties": { "sn": { - "type": "string", - "pattern": "^[A-Za-z0-9_-]+$", - "description": "Serial number of the device. This field is required when creating a device. Must contain only ASCII alphanumeric characters, hyphens, or underscores." + "allOf": [ + { "$ref": "#/components/schemas/SerialNumberSchema" } + ], + "description": "Serial number of the device. This field is required when creating a device." } } }, "DeviceUpdate": { "properties": { "sn": { - "type": "string", + "allOf": [ + { "$ref": "#/components/schemas/SerialNumberSchema" } + ], "nullable": true, - "pattern": "^[A-Za-z0-9_-]+$" + "description": "Serial number of the device. Must contain only ASCII alphanumeric characters, hyphens, or underscores." } } }Also applies to: 7450-7454, 7473-7477, 7505-7509
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openapi.json` around lines 7310 - 7314, Multiple schemas duplicate the same serial-number definition; create a single reusable component named SerialNumberSchema in components.schemas with type: string, nullable: true, pattern: "^[A-Za-z0-9_-]+$", and the given description, then replace each inline serial number schema (the objects containing type/nullable/pattern/description) with a $ref to "#/components/schemas/SerialNumberSchema" (update every occurrence such as the instances around the shown diff and the other similar blocks) so all definitions point to the common component and maintain identical constraints.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@openapi.yaml`:
- Around line 5781-5783: Several schemas constrain the device "serial number"
only by pattern (pattern: ^[A-Za-z0-9_-]+$ / description: Serial number of the
device) but lack the maxLength used by the SN path parameter (bounded to 256 at
Line 1046), causing validation drift; update each schema instance (the entries
containing the pattern and description text) to include maxLength: 256 so
create/update payloads match the SN-based path validation.
---
Outside diff comments:
In `@openapi.yaml`:
- Around line 1051-1133: Add a 400 response for the GetDeviceSN operation
(operationId: GetDeviceSN, GET /v1/devices/sn/{sn}) to document invalid
path/`sn` format validation failures: include a response entry '400' with
description like "Bad Request - invalid serial number format", reuse the
existing Error schema ($ref: '#/components/schemas/Error'), and provide an
example error object (e.g., code: invalid_sn, message: "invalid serial number
format") so consumers know that malformed `sn` path values return 400.
---
Nitpick comments:
In `@openapi.json`:
- Around line 7310-7314: Multiple schemas duplicate the same serial-number
definition; create a single reusable component named SerialNumberSchema in
components.schemas with type: string, nullable: true, pattern:
"^[A-Za-z0-9_-]+$", and the given description, then replace each inline serial
number schema (the objects containing type/nullable/pattern/description) with a
$ref to "#/components/schemas/SerialNumberSchema" (update every occurrence such
as the instances around the shown diff and the other similar blocks) so all
definitions point to the common component and maintain identical constraints.
openapi.yaml
Outdated
| pattern: ^[A-Za-z0-9_-]+$ | ||
| description: Serial number of the device. Must contain only ASCII alphanumeric | ||
| characters, hyphens, or underscores. |
There was a problem hiding this comment.
Align SN length limits across schemas to avoid validation drift.
Line 5781, Line 5947, Line 5971, and Line 5992 only constrain characters; they do not bound length. This can allow create/update payloads that later fail SN-based path validation (which is bounded to 256 chars at Line 1046).
💡 Proposed fix
sn:
type: string
nullable: true
+ minLength: 1
+ maxLength: 256
pattern: ^[A-Za-z0-9_-]+$
description: Serial number of the device. Must contain only ASCII alphanumeric
characters, hyphens, or underscores. sn:
type: string
+ minLength: 1
+ maxLength: 256
pattern: ^[A-Za-z0-9_-]+$
description: Serial number of the device. This field is required when creating
a device. Must contain only ASCII alphanumeric characters, hyphens, or
underscores. sn:
type: string
nullable: true
+ minLength: 1
+ maxLength: 256
pattern: ^[A-Za-z0-9_-]+$ sn:
type: string
+ minLength: 1
+ maxLength: 256
pattern: ^[A-Za-z0-9_-]+$
description: Device serial number. Must contain only ASCII alphanumeric characters,
hyphens, or underscores.Also applies to: 5947-5950, 5971-5971, 5992-5994
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openapi.yaml` around lines 5781 - 5783, Several schemas constrain the device
"serial number" only by pattern (pattern: ^[A-Za-z0-9_-]+$ / description: Serial
number of the device) but lack the maxLength used by the SN path parameter
(bounded to 256 at Line 1046), causing validation drift; update each schema
instance (the entries containing the pattern and description text) to include
maxLength: 256 so create/update payloads match the SN-based path validation.
…yforge#3) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Washio20
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
openapi.json (1)
7312-7313: Consider a shared SN schema to prevent drift.The same SN regex is duplicated in multiple places. Defining one
SerialNumberSchemaundercomponents/schemasand referencing it would reduce future YAML/JSON mismatch risk.Also applies to: 7452-7453, 7475-7477, 7507-7509
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openapi.json` around lines 7312 - 7313, Extract the duplicated serial-number property into a single reusable schema named SerialNumberSchema under components/schemas with the pattern "^[a-zA-Z0-9_-]+$" and the same description, then replace each inline serial number property (the places currently using "pattern": "^[a-zA-Z0-9_-]+$") with a $ref to "#/components/schemas/SerialNumberSchema" so all occurrences (including the ones you noted) reference the single canonical schema to avoid drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@openapi.json`:
- Around line 7310-7313: The schemas DeviceProperties.sn, DeviceInput.sn, and
DeviceUpdate.sn lack a maxLength constraint while the path parameter
/v1/devices/sn/{sn} enforces maxLength 256; add "maxLength": 256 to each of
those schema properties (and to the other duplicated schema occurrences noted
around the other ranges) so body validation matches the path parameter, ensuring
create/update payloads cannot exceed the path's 256-character limit.
---
Nitpick comments:
In `@openapi.json`:
- Around line 7312-7313: Extract the duplicated serial-number property into a
single reusable schema named SerialNumberSchema under components/schemas with
the pattern "^[a-zA-Z0-9_-]+$" and the same description, then replace each
inline serial number property (the places currently using "pattern":
"^[a-zA-Z0-9_-]+$") with a $ref to "#/components/schemas/SerialNumberSchema" so
all occurrences (including the ones you noted) reference the single canonical
schema to avoid drift.
| "type": "string", | ||
| "nullable": true, | ||
| "pattern": "^[a-zA-Z0-9_-]+$", | ||
| "description": "Serial number of the device." |
There was a problem hiding this comment.
SN length constraints are inconsistent between body and path validation.
/v1/devices/sn/{sn} restricts sn to 256 chars (Line 1387), but DeviceProperties.sn, DeviceInput.sn, and DeviceUpdate.sn have no maxLength. This can allow create/update inputs that later fail path validation when querying by SN.
💡 Proposed fix
"sn": {
"type": "string",
"nullable": true,
+ "maxLength": 256,
"pattern": "^[a-zA-Z0-9_-]+$",
"description": "Serial number of the device."
}"sn": {
"type": "string",
+ "maxLength": 256,
"pattern": "^[a-zA-Z0-9_-]+$",
"description": "Serial number of the device. This field is required when creating a device."
}"sn": {
"type": "string",
"nullable": true,
+ "maxLength": 256,
"pattern": "^[a-zA-Z0-9_-]+$"
}Also applies to: 7450-7453, 7473-7477
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openapi.json` around lines 7310 - 7313, The schemas DeviceProperties.sn,
DeviceInput.sn, and DeviceUpdate.sn lack a maxLength constraint while the path
parameter /v1/devices/sn/{sn} enforces maxLength 256; add "maxLength": 256 to
each of those schema properties (and to the other duplicated schema occurrences
noted around the other ranges) so body validation matches the path parameter,
ensuring create/update payloads cannot exceed the path's 256-character limit.
Summary
pattern: ^[a-zA-Z0-9_-]+$regex validation to allsn(serial number) fields in the OpenAPI specRoot Cause
The
snfield in the device schemas (DeviceProperties, DeviceCreate, DeviceUpdate, DeviceInfo) and the/v1/devices/sn/{sn}path parameter had no format validation, allowing any string including non-alphanumeric characters to be submitted.Changes
openapi.yaml: Addedpattern: ^[a-zA-Z0-9_-]+$tosnfields in DeviceProperties, DeviceCreate, DeviceUpdate, DeviceInfo schemas and the path parameteropenapi.json: Same pattern added to the corresponding JSON definitionsTesting
^[a-zA-Z0-9_-]+$correctly:SFVA78RABZ12345678,device-123,SN_001Closes #3
Summary by CodeRabbit