-
Notifications
You must be signed in to change notification settings - Fork 3
[SP-3798] feat(proto): add download crypto ruleset endpoint #59
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[SP-3798] feat(proto): add download crypto ruleset endpoint #59
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughAdds a gRPC RPC and matching REST GET endpoint to download cryptography detection rulesets as tarball HTTP bodies; updates protobuf, OpenAPI/Swagger spec, README docs, and the changelog with version 0.27.0. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
protobuf/scanoss/api/cryptography/v2/README.md (1)
506-523: Consider adding language identifier to fenced code block.The fenced code block showing the tarball structure would benefit from a language identifier (e.g.,
textorplaintext) for consistency with Markdown best practices.Apply this diff:
#### Tarball Structure The downloaded and extracted tarball contains: -``` +```text dca-v1.2.3/ ├── java/ # Java cryptography detection rules │ ├── jca.yaml
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
api/cryptographyv2/scanoss-cryptography.pb.gois excluded by!**/*.pb.goapi/cryptographyv2/scanoss-cryptography.pb.gw.gois excluded by!**/*.pb.gw.goapi/cryptographyv2/scanoss-cryptography_grpc.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (4)
CHANGELOG.md(2 hunks)protobuf/scanoss/api/cryptography/v2/README.md(1 hunks)protobuf/scanoss/api/cryptography/v2/scanoss-cryptography.proto(28 hunks)protobuf/scanoss/api/cryptography/v2/scanoss-cryptography.swagger.json(2 hunks)
🧰 Additional context used
🪛 Checkov (3.2.334)
protobuf/scanoss/api/cryptography/v2/scanoss-cryptography.swagger.json
[medium] 601-608: Ensure that arrays have a maximum number of items
(CKV_OPENAPI_21)
🪛 LanguageTool
CHANGELOG.md
[style] ~12-~12: Consider a different adjective to strengthen your wording.
Context: ...cryptographic rulesets for keywords and deep code analysis. ## [0.26.0] - 2025-12-0...
(DEEP_PROFOUND)
🪛 markdownlint-cli2 (0.18.1)
protobuf/scanoss/api/cryptography/v2/README.md
510-510: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (7)
protobuf/scanoss/api/cryptography/v2/README.md (1)
438-462: LGTM! Clear documentation of the new endpoint.The documentation clearly explains the purpose, supported ruleset types (dca and keywords), and version specification options. The curl examples are practical and easy to follow.
protobuf/scanoss/api/cryptography/v2/scanoss-cryptography.proto (3)
38-38: LGTM! Necessary import for binary response.The
google/api/httpbody.protoimport is required for the DownloadRuleset RPC that returnsgoogle.api.HttpBody, which is the correct approach for returning binary content like tarballs.
286-314: LGTM! Well-documented RPC definition.The DownloadRuleset RPC is properly defined with:
- Comprehensive documentation covering purpose, supported ruleset types, version options, and response headers
- Correct use of
google.api.HttpBodyfor binary tarball responses- Appropriate HTTP GET mapping for idempotent download operations
- Clear reference to README documentation
775-795: LGTM! Clean message definition.The RulesetDownloadRequest message is well-defined with:
- Clear field documentation explaining supported ruleset types (dca, keywords)
- Version flexibility (latest or specific version)
- Proper JSON naming for REST API compatibility
- Comprehensive comments about response headers and metadata
protobuf/scanoss/api/cryptography/v2/scanoss-cryptography.swagger.json (3)
516-562: LGTM! Complete and correct endpoint definition.The new download endpoint is properly defined with:
- Correct HTTP GET method matching the protobuf definition
- Required path parameters (ruleset_name and version) with clear descriptions
- Appropriate response schema referencing apiHttpBody for binary content
- Comprehensive operation documentation covering supported ruleset types, versioning, and response headers
589-611: LGTM! Standard Google API HttpBody definition.The apiHttpBody definition is properly structured as a standard Google API type for representing HTTP bodies with non-JSON content (raw binary or HTML). This is the correct approach for the tarball download endpoint.
612-622: LGTM! Enhanced protobufAny definition.The updates to the protobufAny definition improve clarity by adding detailed descriptions of the
@typeproperty and the overall Any type semantics, plus the correctadditionalProperties: {}specification that allows arbitrary fields as expected for protobuf Any messages.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
protobuf/scanoss/api/cryptography/v2/README.md (1)
510-523: Specify language for fenced code block.The fenced code block showing the tarball structure should specify a language identifier for proper syntax highlighting and markdown compliance.
🔎 Apply this diff to add language identifier:
-``` +```text dca-v1.2.3/ ├── java/ # Java cryptography detection rules │ ├── jca.yamlBased on static analysis hints.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
api/cryptographyv2/scanoss-cryptography_grpc.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (3)
protobuf/scanoss/api/cryptography/v2/README.md(1 hunks)protobuf/scanoss/api/cryptography/v2/scanoss-cryptography.proto(28 hunks)protobuf/scanoss/api/cryptography/v2/scanoss-cryptography.swagger.json(2 hunks)
🧰 Additional context used
🪛 Checkov (3.2.334)
protobuf/scanoss/api/cryptography/v2/scanoss-cryptography.swagger.json
[medium] 601-608: Ensure that arrays have a maximum number of items
(CKV_OPENAPI_21)
🪛 markdownlint-cli2 (0.18.1)
protobuf/scanoss/api/cryptography/v2/README.md
510-510: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (7)
protobuf/scanoss/api/cryptography/v2/README.md (1)
438-537: Excellent documentation for the new DownloadRuleset endpoint.The documentation is comprehensive and well-structured, covering all essential aspects:
- Supported ruleset types and version specifications
- HTTP request examples with curl
- Response format with headers
- Tarball structure and manifest details
This will be very helpful for API consumers.
protobuf/scanoss/api/cryptography/v2/scanoss-cryptography.proto (3)
38-38: Correct import for HttpBody support.The import of
google/api/httpbody.protois necessary and appropriate for the new DownloadRuleset RPC that returns binary tarball content.
286-314: Well-designed RPC for ruleset downloads.The DownloadRuleset RPC is properly designed:
- Returns
google.api.HttpBodywhich is the correct type for binary/non-JSON responses- HTTP GET mapping follows RESTful conventions
- Comprehensive documentation includes supported ruleset types, version handling, and response header details
- References the README for additional documentation
775-795: Clean request message definition.The
RulesetDownloadRequestmessage is well-structured:
- Clear field names and descriptions
- Proper field numbering
- Includes examples of supported values
- Documents the version semantics ("latest" vs specific versions)
protobuf/scanoss/api/cryptography/v2/scanoss-cryptography.swagger.json (3)
516-562: Well-defined OpenAPI endpoint for ruleset downloads.The new endpoint definition is comprehensive and correct:
- Proper HTTP GET operation with clear summary and description
- Appropriate response schemas (200 → apiHttpBody, 404 → string, default → rpcStatus)
- Path parameters are correctly defined with detailed descriptions
- Tagged appropriately under Cryptography
- Documentation URL reference included
589-611: Standard Google API HttpBody definition.The
apiHttpBodydefinition correctly represents thegoogle.api.HttpBodytype for non-JSON payloads like raw binary data. The structure matches Google's standard API conventions with:
- content_type for MIME type specification
- data as base64-encoded binary
- extensions for metadata
Note: The static analysis warning about arrays lacking maximum items is a false positive. This is a standard Google API type definition that shouldn't be constrained.
616-622: Enhanced protobufAny documentation.The additions provide comprehensive documentation for the
protobufAnytype, including detailed explanations of the @type field and the Any message usage patterns. TheadditionalProperties: {}allows for the flexible schema that Any requires.
agustingroh
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Summary by CodeRabbit
New Features
Documentation
Changelog
✏️ Tip: You can customize this high-level summary in your review settings.