Conversation
…ns" or "extends" as required properties.
- Enhance error messages to include instance paths and parameters for better debugging. - Update error handling in multiple functions to provide detailed feedback on validation failures.
- Introduce `allowUnsafeMarkup` to indicate if potentially unsafe markup can be executed. - Add `unsafe` property to identify potentially unsafe markup in input files. - Update relevant schema files: config_v3.schema.json, resolvedTests_v3.schema.json, and schemas.json.
- Update descriptions for "unsafe" property across all relevant schemas
…on in step and test schemas to clarify its implications. Update related references and ensure consistency across schemas.
- Bump version to 3.1.0-dev.0 - Update @apidevtools/json-schema-ref-parser to 13.0.5 - Update mocha to 11.6.0 - Update workerpool to 9.3.2
|
""" WalkthroughThis update introduces a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Validator
participant Schema
User->>Validator: Submit config/test/step for validation
Validator->>Schema: Validate including 'unsafe' and 'allowUnsafeTests' properties
Schema-->>Validator: Return validation result (with detailed errors if any)
Validator-->>User: Return result with formatted error messages
Poem
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (16)
src/schemas/output_schemas/test_v3.schema.json (1)
633-637: Duplicate: step-levelunsafeproperty addition
These hunks repeatedly add the sameunsafeboolean field (with identical description and default) across all step definitions. See the earlier comment on centralizing this schema to avoid redundancy and reduce drift.Also applies to: 680-684, 735-739, 946-950, 1128-1132, 1924-1928, 2081-2085, 3079-3083, 3381-3385, 3669-3673, 3885-3889, 4328-4332, 4495-4499, 4575-4579, 4650-4654, 5374-5378, 5421-5425, 5476-5480, 5687-5691, 5869-5873, 6665-6669, 6822-6826, 7820-7824, 8122-8126, 8410-8414, 8626-8630, 9069-9073, 9236-9240, 9316-9320, 9391-9395
src/schemas/output_schemas/step_v3.schema.json (12)
76-80: Duplicate: fix grammar and config reference
131-135: Duplicate: fix grammar and config reference
342-346: Duplicate: fix grammar and config reference
524-528: Duplicate: fix grammar and config reference
1320-1324: Duplicate: fix grammar and config reference
1477-1481: Duplicate: fix grammar and config reference
2777-2781: Duplicate: fix grammar and config reference
3077-3081: Duplicate: fix grammar and config reference
3281-3285: Duplicate: fix grammar and config reference
3891-3895: Duplicate: fix grammar and config reference
3971-3975: Duplicate: fix grammar and config reference
4046-4050: Duplicate: fix grammar and config referencesrc/schemas/output_schemas/spec_v3.schema.json (1)
1234-1238: Duplicate of comment above.Also applies to: 1281-1285, 1336-1340, 1547-1551, 1729-1733, 2525-2529, 2682-2686, 3680-3684, 3982-3986, 4270-4274, 4486-4490, 4929-4933, 5096-5100, 5176-5180, 5251-5255, 5938-5942, 5975-5979, 6022-6026, 6077-6081, 6288-6292, 6470-6474, 7266-7270, 7423-7427, 8421-8425, 8723-8727, 9011-9015, 9227-9231, 9670-9674, 9837-9841, 9917-9921, 9992-9996
src/schemas/output_schemas/report_v3.schema.json (1)
1252-1256: Consistent addition of theunsafeproperty across step variants verified.Each step schema now includes the
unsafeboolean with the correct default and description. This change is consistent and aligns with the test-level addition above. Ensure you apply any grammar refinements (if accepted) uniformly across all occurrences.Also applies to: 1299-1303, 1354-1358, 1565-1569, 1747-1751, 2543-2547, 2700-2704, 3698-3702, 4000-4004, 4288-4292, 4504-4508, 4947-4951, 5114-5118, 5194-5198, 5269-5273, 5956-5960, 5993-5997, 6040-6044, 6095-6099, 6306-6310, 6488-6492, 7284-7288, 7441-7445, 8439-8443, 8741-8745, 9029-9033, 9245-9249, 9688-9692, 9855-9859, 9935-9939, 10010-10014
src/schemas/output_schemas/config_v3.schema.json (1)
842-849: Apply grammar/config-fix from above to allunsafeinsertions.Also applies to: 897-904, 1108-1115, 1293-1297, 2089-2093, 2246-2250, 3244-3248, 3546-3550, 3834-3838, 4050-4054, 4493-4497, 4660-4664, 4740-4744, 4815-4819, 5922-5926, 5969-5973, 6024-6028, 6235-6239, 6417-6421, 7213-7217, 7370-7374, 8368-8372, 8670-8674, 8958-8962, 9174-9178, 9617-9621, 9784-9788, 9864-9868, 9939-9943
🧹 Nitpick comments (8)
src/schemas/src_schemas/step_v3.schema.json (1)
23-25: Redundant$refindirection
common.properties.unsafejust re-references the component schema defined a few lines below.
This is a tiny inefficiency: duplicate level of indirection adds no value and makes tooling output less compact. Consider inlining the boolean schema directly incommonto keep the generated bundle smaller.src/schemas/output_schemas/test_v3.schema.json (1)
19-23: Refine and centralize theunsafeproperty definition
The identicalunsafeboolean schema (same description and default) is copy-pasted at the test root and in the test object definition. Extract this into a shareddefinitions/unsafecomponent and use$refto improve maintainability and ensure single-source truth. Also, the description referencesallowUnsafeTests, but the config schema introducesallowUnsafeMarkup; please verify which config key is correct and update descriptions accordingly.Also applies to: 5337-5341
src/schemas/output_schemas/resolvedTests_v3.schema.json (2)
558-567: Clarify theanyOfrequirements for custom mode file types.
Allowing eitherextensionsorextendsis correct, but consider adding descriptive comments or examples in the schema to illustrate typical usage patterns. Also verify that using the keywordextendswon’t conflict with any JSON Schema keywords or downstream tooling.
811-815: Fix grammatical error inunsafedescriptions.
The phrase"Whether or not the test or step is may be unsafe."contains an extra "is". It should read"Whether or not the test or step may be unsafe."for clarity.Example diff:
-"description": "Whether or not the test or step is may be unsafe. Unsafe tests and steps may perform actions..." +"description": "Whether or not the test or step may be unsafe. Unsafe tests and steps may perform actions..."Also applies to: 858-862, 913-917, 1124-1128, 1306-1310, 2102-2106, 2259-2263, 3257-3261, 3559-3563, 3847-3851, 4063-4067, 4506-4510, 4673-4677, 4753-4757, 4828-4832, 5935-5939, 5982-5986, 6037-6041, 6248-6252, 6430-6434, 7226-7230, 7383-7387, 8381-8385, 8683-8687, 8971-8975, 9187-9191, 9630-9634, 9797-9801, 9877-9881, 9952-9956, 11232-11236, 11846-11850, 11893-11897, 11948-11952, 12159-12163, 12341-12345, 13137-13141, 13294-13298, 14292-14296, 14594-14598, 14882-14886, 15098-15102, 15541-15545, 15708-15712, 15788-15792, 15863-15867, 16550-16554, 16587-16591, 16634-16638, 16689-16693, 16900-16904, 17082-17086, 17878-17882, 18035-18039, 19033-19037, 19335-19339, 19623-19627, 19839-19843, 20282-20286, 20449-20453, 20529-20533, 20604-20608
src/schemas/output_schemas/spec_v3.schema.json (1)
620-624: Correct grammatical error and improve maintainability.The description text has a typo ("is may be unsafe") and is duplicated in many places. Consider extracting the
unsafeproperty definition into a reusable definition and referencing it with$refto avoid repetition and ensure consistent updates.src/schemas/output_schemas/report_v3.schema.json (1)
638-642: Refine description grammar for theunsafeflag.The phrase “Whether or not the test or step is may be unsafe” contains a redundancy. Consider rewording for clarity, for example:
-"description": "Whether or not the test or step is may be unsafe. Unsafe tests and steps may perform actions that could modify the system or environment in unexpected ways. Tests that contain unsafe steps are only performed within Docker containers or if unsafe tests are enabled with the `allowUnsafeTests` config property or the `--unsafe` flag.", +"description": "Whether the test or step may be unsafe. Unsafe actions can modify the system or environment unexpectedly. Tests with unsafe steps run only within Docker containers or when enabled via the `allowUnsafeTests` config property or the `--unsafe` flag.",src/schemas/output_schemas/config_v3.schema.json (2)
132-135: Ensure default forallowUnsafeMarkupand clarify semantics.
The newallowUnsafeMarkupflag lacks a default value—consider adding"default": falseto align with other boolean flags. Also, verify that its description correctly distinguishesallowUnsafeMarkupfrom any existingallowUnsafeTestsproperty.
545-556: Consider usingoneOffor exclusiveextensions/extends.
Ifextensionsandextendsare meant to be mutually exclusive, replaceanyOfwithoneOfto enforce exactly one choice. Otherwise, add a note in the description that both can coexist.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
dist/schemas/config_v3.schema.jsonis excluded by!**/dist/**dist/schemas/report_v3.schema.jsonis excluded by!**/dist/**dist/schemas/resolvedTests_v3.schema.jsonis excluded by!**/dist/**dist/schemas/spec_v3.schema.jsonis excluded by!**/dist/**dist/schemas/step_v3.schema.jsonis excluded by!**/dist/**dist/schemas/test_v3.schema.jsonis excluded by!**/dist/**package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (11)
package.json(2 hunks)src/schemas/output_schemas/config_v3.schema.json(32 hunks)src/schemas/output_schemas/report_v3.schema.json(33 hunks)src/schemas/output_schemas/resolvedTests_v3.schema.json(65 hunks)src/schemas/output_schemas/spec_v3.schema.json(33 hunks)src/schemas/output_schemas/step_v3.schema.json(15 hunks)src/schemas/output_schemas/test_v3.schema.json(33 hunks)src/schemas/src_schemas/config_v3.schema.json(9 hunks)src/schemas/src_schemas/step_v3.schema.json(2 hunks)src/schemas/src_schemas/test_v3.schema.json(10 hunks)src/validate.js(7 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test (windows-latest, 22)
🔇 Additional comments (23)
package.json (1)
3-3: Lock-file & CI check after dependency bumpsVersion bump to
3.1.0-dev.0and upgrades formocha/@apidevtools/json-schema-ref-parserlook fine, but remember to:
- Commit the regenerated lock-file (
package-lock.json/pnpm-lock.yaml) so CI produces reproducible installs.- Ensure the updated parser (
^13.0.5) still passes the schema dereference task – its minor changes occasionally break remote$refresolution.Also applies to: 24-24, 28-28
src/schemas/output_schemas/test_v3.schema.json (1)
10066-10066: Verify example alignment with schema and config
The example test object now includes"unsafe": true, which matches the new schema. Please ensure the JSON syntax remains strict (no trailing commas) and confirm that the example’s reference to the config property matches the actual key (allowUnsafeMarkupvs.allowUnsafeTests).src/schemas/src_schemas/test_v3.schema.json (4)
100-104: Inconsistent AI summary regarding required propertiesThe summary states “No changes were made to required properties,” but this schema now adds a second
anyOfbranch requiringcontexts. Please correct the summary or revise the change.Likely an incorrect or invalid review comment.
119-122: Approve negative constraint onoperationIdUsing a
notschema block to forbidoperationIdin test‐level OpenAPI objects and requiringname+descriptionPathis a clear way to enforce the intended contract.
160-161: Approve addition ofunsafeflag in exampleThe example test now correctly demonstrates the new
unsafeboolean property.
149-150: Approve single-line array formatting in examplesConsolidating arrays into single lines improves readability and consistency across examples.
Also applies to: 163-164, 178-179, 205-206, 224-225, 231-232
src/schemas/output_schemas/resolvedTests_v3.schema.json (1)
21279-21280: Example update is correct.
The test example now includes"unsafe": true, aligning with the schema enhancements.src/schemas/src_schemas/config_v3.schema.json (13)
49-49: Approve single-line enum formatting forrelativePathBase
Consolidating the enum into a single line improves readability and aligns with the rest of the schema.
103-103: Approve single-line enum formatting forlogLevel
This adjustment keeps the style consistent with other enum definitions.
115-115: Approve single-line default forfileTypes
The default array is now on one line, consistent with other simple arrays.
126-126: Approve single-line enum formatting for predefinedfileTypes
This maintains uniform formatting for enum arrays across the schema.
132-134: Approve relaxedanyOfrequirements for custom file types
Allowing eitherextensionsorextendsgrants flexibility in defining custom templates.
144-144: Approve single-line enum formatting for theextendsproperty
Consistent styling for enum values improves maintainability.
171-171: Approve explicitrequiredfor executable file type
Requiringextensionsin executable mode is correct to ensure valid detection.
222-222: Approve addition ofrequired: ["send"]in telemetry
Forcingsendto be present guarantees telemetry behavior is always defined.
239-239: Approve addition ofrequired: ["platform"]in environment schema
Requiringplatformensures at least this key is available in the read-only environment object.
248-248: Approve enum values forplatform
The permitted OS values are clear and cover the supported environments.
253-253: Approve enum values forarch
The architecture list comprehensively covers the expected targets.
390-390: Approve example formatting forfileTypes
Updating the example to a single-line array improves consistency with the schema.
396-396: Approve exampleextensionsarray formatting
The shift to a single-line extensions array aligns with schema formatting conventions.src/schemas/output_schemas/spec_v3.schema.json (2)
620-624: Align config property naming.The description references
allowUnsafeTests, but the PR introducesallowUnsafeMarkupas the config property. Please confirm the intended name and update the description to match.
10667-10668: Example usage ofunsafeflag is appropriate.src/schemas/output_schemas/report_v3.schema.json (1)
10685-10685: Example updated to demonstrate theunsafeflag.The example test object now correctly includes
"unsafe": true, illustrating usage of the new property.
- Update error handling to directly display validation errors without additional formatting.
Updated the description of the "unsafe" property in both step_v3.schema.json and test_v3.schema.json to correct the phrasing from "is may be unsafe" to "may be unsafe" for clarity and accuracy.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/schemas/output_schemas/resolvedTests_v3.schema.json (1)
811-815: Consider extracting theunsafeproperty into a shared definition.
Theunsafeflag with identical schema is repeated across many objects. Defining it once under#/definitions/unsafeFlagand referencing it via$refwill reduce duplication and simplify future updates.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
dist/schemas/config_v3.schema.jsonis excluded by!**/dist/**dist/schemas/report_v3.schema.jsonis excluded by!**/dist/**dist/schemas/resolvedTests_v3.schema.jsonis excluded by!**/dist/**dist/schemas/spec_v3.schema.jsonis excluded by!**/dist/**dist/schemas/step_v3.schema.jsonis excluded by!**/dist/**dist/schemas/test_v3.schema.jsonis excluded by!**/dist/**
📒 Files selected for processing (8)
src/schemas/output_schemas/config_v3.schema.json(32 hunks)src/schemas/output_schemas/report_v3.schema.json(33 hunks)src/schemas/output_schemas/resolvedTests_v3.schema.json(65 hunks)src/schemas/output_schemas/spec_v3.schema.json(33 hunks)src/schemas/output_schemas/step_v3.schema.json(15 hunks)src/schemas/output_schemas/test_v3.schema.json(33 hunks)src/schemas/src_schemas/step_v3.schema.json(2 hunks)src/schemas/src_schemas/test_v3.schema.json(10 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- src/schemas/output_schemas/test_v3.schema.json
- src/schemas/src_schemas/step_v3.schema.json
- src/schemas/output_schemas/spec_v3.schema.json
- src/schemas/src_schemas/test_v3.schema.json
- src/schemas/output_schemas/report_v3.schema.json
- src/schemas/output_schemas/config_v3.schema.json
- src/schemas/output_schemas/step_v3.schema.json
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test (windows-latest, 22)
🔇 Additional comments (2)
src/schemas/output_schemas/resolvedTests_v3.schema.json (2)
558-568: Verify the use ofanyOffor custom file types.
Allowing eitherextensionsorextendsin the file type schema looks correct, but please confirm that validation logic, documentation, and any downstream code generators are updated to handle both properties.
21279-21279: Confirm example override ofunsafe.
The example now explicitly sets"unsafe": true. Ensure this aligns with updated documentation and clearly contrasts with the defaultfalsebehavior.
| "unsafe": { | ||
| "type": "boolean", | ||
| "description": "Whether or not the test or step is may be unsafe. Unsafe tests and steps may perform actions that could modify the system or environment in unexpected ways. Tests that contain unsafe steps are only performed within Docker containers or if unsafe tests are enabled with the `allowUnsafeTests` config property or the `--unsafe` flag.", | ||
| "default": false | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix typo in the unsafe description.
There's an extra "is" in:
"Whether or not the test or step is may be unsafe."
It should read:
"Whether or not the test or step may be unsafe."
🤖 Prompt for AI Agents
In src/schemas/output_schemas/resolvedTests_v3.schema.json around lines 16550 to
16554, fix the typo in the description of the `unsafe` field by removing the
extra "is" so that it reads "Whether or not the test or step may be unsafe."
instead of "Whether or not the test or step is may be unsafe."
| "allowUnsafeTests": { | ||
| "type": "boolean", | ||
| "description": "Whether or not to run potentially unsafe tests, such as those that execute shell commands or run code." | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add a default for allowUnsafeTests.
The new allowUnsafeTests boolean lacks a default value, which can lead to unsafe-by-default behavior. It should default to false to enforce safe-by-default semantics.
Apply this diff:
"allowUnsafeTests": {
"type": "boolean",
"description": "Whether or not to run potentially unsafe tests, such as those that execute shell commands or run code.",
+ "default": false
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "allowUnsafeTests": { | |
| "type": "boolean", | |
| "description": "Whether or not to run potentially unsafe tests, such as those that execute shell commands or run code." | |
| }, | |
| "allowUnsafeTests": { | |
| "type": "boolean", | |
| "description": "Whether or not to run potentially unsafe tests, such as those that execute shell commands or run code.", | |
| "default": false | |
| }, |
🤖 Prompt for AI Agents
In src/schemas/output_schemas/resolvedTests_v3.schema.json around lines 133 to
136, the boolean property allowUnsafeTests is missing a default value, which can
cause unsafe behavior by default. Add a default value of false to the
allowUnsafeTests property in the schema to ensure safe-by-default semantics.
1 participant
Summary by CodeRabbit
New Features
allowUnsafeTeststo control the execution of potentially unsafe tests.extensionsorextendsproperties.Improvements
Dependency Updates