feat: clear $ref fields across entire OpenAPI document#928
feat: clear $ref fields across entire OpenAPI document#928guan404ming wants to merge 3 commits intomeshery:masterfrom
Conversation
|
Yay, your first pull request! 👍 A contributor will be by to give feedback soon. In the meantime, you can find updates in the #github-notifications channel in the community Slack. |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the OpenAPI document processing by expanding the $ref field clearing mechanism. Previously, only schema references within components/schemas were resolved. The updated implementation now systematically traverses and clears $ref fields across the entire OpenAPI document, including paths, parameters, headers, request bodies, responses, and callbacks, ensuring a fully inlined and resolved OpenAPI specification. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the OpenAPI reference clearing logic into a new, more comprehensive clearDocRefs function, designed to traverse various parts of the OpenAPI document and clear $ref fields for full inlining. However, it introduces a potential infinite recursion vulnerability in the handling of Callbacks, PathItems, and Operations due to a lack of cycle detection. This could lead to a stack overflow and Denial of Service if circular references are present. It is recommended to implement tracking of visited objects during recursive traversal, similar to the existing cycle detection in clearSchemaRefs. Additionally, consider expanding clearDocRefs to handle a few more component types for full completeness.
utils/component/openapi_generator.go
Outdated
| func clearCallbackRefs(cr *openapi3.CallbackRef, stack map[*openapi3.Schema]bool) { | ||
| if cr == nil { | ||
| return | ||
| } | ||
| cr.Ref = "" | ||
| if cr.Value != nil { | ||
| for _, pathItem := range cr.Value.Map() { | ||
| clearPathItemRefs(pathItem, stack) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| func clearPathItemRefs(pathItem *openapi3.PathItem, stack map[*openapi3.Schema]bool) { | ||
| if pathItem == nil { | ||
| return | ||
| } | ||
| for _, pr := range pathItem.Parameters { | ||
| clearParameterRefs(pr, stack) | ||
| } | ||
| for _, op := range pathItem.Operations() { | ||
| clearOperationRefs(op, stack) | ||
| } | ||
| } | ||
|
|
||
| func clearOperationRefs(op *openapi3.Operation, stack map[*openapi3.Schema]bool) { | ||
| if op == nil { | ||
| return | ||
| } | ||
| for _, pr := range op.Parameters { | ||
| clearParameterRefs(pr, stack) | ||
| } | ||
| clearRequestBodyRefs(op.RequestBody, stack) | ||
| if op.Responses != nil { | ||
| for _, rr := range op.Responses.Map() { | ||
| clearResponseRefs(rr, stack) | ||
| } | ||
| } | ||
| for _, cr := range op.Callbacks { | ||
| clearCallbackRefs(cr, stack) | ||
| } | ||
| } |
There was a problem hiding this comment.
The clearDocRefs function and its helpers (clearCallbackRefs, clearPathItemRefs, clearOperationRefs) recursively traverse the OpenAPI document to clear $ref fields. However, they do not implement a mechanism to detect or break circular references between Callbacks, PathItems, and Operations.
In OpenAPI 3, a Callback contains PathItems, a PathItem contains Operations, and an Operation can contain Callbacks. If an OpenAPI document contains a circular reference between these components (e.g., a Callback that eventually refers back to itself), the recursive calls will continue indefinitely until a stack overflow occurs, causing the application to crash. This represents a Denial of Service (DoS) vulnerability.
While clearSchemaRefs correctly uses a stack map to track and break circular references in schemas, no such protection exists for the Callback-PathItem-Operation recursion loop.
guan404ming
force-pushed
the
fix/clear-refs-entire-doc
branch
from
4450557 to
e7d0a7e
Compare
Signed-off-by: GUAN MING <guanmingchiu@gmail.com>
Signed-off-by: GUAN MING <guanmingchiu@gmail.com>
guan404ming
force-pushed
the
fix/clear-refs-entire-doc
branch
2 times, most recently
from
7bd89b0 to
36f43b5
Compare
Signed-off-by: GUAN MING <guanmingchiu@gmail.com>
guan404ming
force-pushed
the
fix/clear-refs-entire-doc
branch
from
36f43b5 to
c624be1
Compare
1 participant
Description
This PR fixes #926
Notes for Reviewers
Why
The
getResolvedManifestfunction only cleared$reffields fromcomponents/schemas, leaving refs in paths, parameters, responses, request bodies, headers, and callbacks unresolved.How
clearDocRefsthat traverses paths, all component types, and nestedcallbacks
content
Signed commits