add Attribution Notices service definitions#78
Conversation
📝 WalkthroughWalkthroughIntroduces a new SCANOSS Attribution Notices Service API v2 with gRPC and HTTP/JSON interfaces. Defines service endpoints for retrieving attribution notices for component PURLs and testing connectivity, including message types for notices and responses. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 |
Suggested Changelog EntriesBased on the changes in this PR, here are the suggested entries for the `[Unreleased]` section of `CHANGELOG.md`: ```markdown [Unreleased]Added
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@protobuf/scanoss/api/attributionnotices/v2/README.md`:
- Line 10: Update the relative link in the README so the sibling common docs are
reached: replace the link target "../common/v2/README.md" with
"../../common/v2/README.md" in the line that reads "See [Common API
Types](../common/v2/README.md) for `ComponentsRequest` documentation" so the
link correctly resolves from attributionnotices/v2 to the common/v2 README.
In
`@protobuf/scanoss/api/attributionnotices/v2/scanoss-attribution-notices.proto`:
- Around line 90-91: The comment for the protobuf field notice_md5 currently
calls MD5 an "integrity check" which is misleading; update the comment above the
field notice_md5 to state that the MD5 is used only for non-security
deduplication (e.g., "MD5 hash of the notice text used for deduplication") and
remove any mention of tamper detection or integrity guarantees, and if tamper
detection is required, add a new field (e.g., notice_sha256) and document it as
a strong digest intended for integrity/tamper detection.
- Around line 37-61: The OpenAPI swagger option block (option
(grpc.gateway.protoc_gen_openapiv2.options.openapiv2_swagger)) currently sets
schemes: HTTP and lacks API key security; update that block to set schemes:
HTTPS and add a securityDefinitions object defining an api_key (type: "apiKey",
name: "X-Api-Key", in: "header") and add a top-level security requirement that
references that api_key so generated clients will include the X-Api-Key header
by default; ensure you modify the same swagger option block that contains
info/consumes/produces/responses to include these securityDefinitions and
security entries.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a5270e69-ceff-4aec-9cc8-13eed872dcba
⛔ Files ignored due to path filters (3)
api/attributionnoticesv2/scanoss-attribution-notices.pb.gois excluded by!**/*.pb.goapi/attributionnoticesv2/scanoss-attribution-notices.pb.gw.gois excluded by!**/*.pb.gw.goapi/attributionnoticesv2/scanoss-attribution-notices_grpc.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (3)
protobuf/scanoss/api/attributionnotices/v2/README.mdprotobuf/scanoss/api/attributionnotices/v2/scanoss-attribution-notices.protoprotobuf/scanoss/api/attributionnotices/v2/scanoss-attribution-notices.swagger.json
| Retrieves attribution notices for a list of software components. | ||
|
|
||
| ### Request Format | ||
| See [Common API Types](../common/v2/README.md) for `ComponentsRequest` documentation. |
There was a problem hiding this comment.
Fix the relative link to the common API docs.
From attributionnotices/v2, ../common/v2/README.md resolves under attributionnotices/common; the sibling common API path needs two .. segments.
Proposed fix
-See [Common API Types](../common/v2/README.md) for `ComponentsRequest` documentation.
+See [Common API Types](../../common/v2/README.md) for `ComponentsRequest` documentation.📝 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.
| See [Common API Types](../common/v2/README.md) for `ComponentsRequest` documentation. | |
| See [Common API Types](../../common/v2/README.md) for `ComponentsRequest` documentation. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@protobuf/scanoss/api/attributionnotices/v2/README.md` at line 10, Update the
relative link in the README so the sibling common docs are reached: replace the
link target "../common/v2/README.md" with "../../common/v2/README.md" in the
line that reads "See [Common API Types](../common/v2/README.md) for
`ComponentsRequest` documentation" so the link correctly resolves from
attributionnotices/v2 to the common/v2 README.
| option (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_swagger) = { | ||
| info: { | ||
| title: "SCANOSS Attribution Notices Service"; | ||
| version: "2.0"; | ||
| contact: { | ||
| name: "scanoss-attribution-notices"; | ||
| url: "https://github.com/scanoss/attribution-notices"; | ||
| email: "support@scanoss.com"; | ||
| }; | ||
| }; | ||
| schemes: HTTP; | ||
| consumes: "application/json"; | ||
| produces: "application/json"; | ||
| responses: { | ||
| key: "404"; | ||
| value: { | ||
| description: "Returned when the resource does not exist."; | ||
| schema: { | ||
| json_schema: { | ||
| type: STRING; | ||
| } | ||
| } | ||
| } | ||
| } | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Description: Find existing grpc-gateway OpenAPI security/scheme option usage in the repo.
# Expectation: Existing services, if any, show the exact syntax for HTTPS and API-key security metadata.
rg -n -C3 'security_definitions|security_requirement|TYPE_API_KEY|IN_HEADER|X-Api-Key|schemes:\s*HTTPS|schemes:\s*HTTP' .Repository: scanoss/papi
Length of output: 32027
🏁 Script executed:
# Check the full OpenAPI metadata in the components proto file
sed -n '37,80p' ./protobuf/scanoss/api/components/v2/scanoss-components.protoRepository: scanoss/papi
Length of output: 1052
🏁 Script executed:
# Check if there's a top-level security field after security_definitions in components.proto
rg -n 'security_definitions|^ security:' ./protobuf/scanoss/api/components/v2/scanoss-components.proto -A 25Repository: scanoss/papi
Length of output: 628
🏁 Script executed:
# Check if components.proto has a top-level security field anywhere
grep -n "security:" ./protobuf/scanoss/api/components/v2/scanoss-components.proto | head -20Repository: scanoss/papi
Length of output: 74
🏁 Script executed:
# Check if any other proto files use a top-level security field (outside of security_definitions)
rg -n '^\s+security:\s*\{' ./protobuf/scanoss/api/ -A 5Repository: scanoss/papi
Length of output: 604
🏁 Script executed:
# Get the exact line numbers and context around security in components.proto
sed -n '37,80p' ./protobuf/scanoss/api/components/v2/scanoss-components.proto | cat -nRepository: scanoss/papi
Length of output: 1360
🏁 Script executed:
# Check the openapiv2.proto schema definition to understand if top-level security is valid
grep -n "message Swagger" ./protobuf/protoc-gen-openapiv2/options/openapiv2.proto -A 30Repository: scanoss/papi
Length of output: 1890
🏁 Script executed:
# Continue reading the Swagger message definition to find security-related fields
grep -n "message Swagger" ./protobuf/protoc-gen-openapiv2/options/openapiv2.proto -A 60 | tail -35Repository: scanoss/papi
Length of output: 1992
🏁 Script executed:
# Search specifically for "security" field definitions in the Swagger message
sed -n '45,120p' ./protobuf/protoc-gen-openapiv2/options/openapiv2.proto | cat -nRepository: scanoss/papi
Length of output: 4447
🏁 Script executed:
# Check the SecurityRequirement message structure
grep -n "message SecurityRequirement" ./protobuf/protoc-gen-openapiv2/options/openapiv2.proto -A 10Repository: scanoss/papi
Length of output: 832
🏁 Script executed:
# Check if there are any examples in the openapiv2.proto file showing how to use the top-level security field
grep -n "security:" ./protobuf/protoc-gen-openapiv2/options/openapiv2.proto | head -20Repository: scanoss/papi
Length of output: 38
🏁 Script executed:
# Check the full definition of the components.proto file to see if it has all the security-related content
cat ./protobuf/scanoss/api/components/v2/scanoss-components.proto | head -80Repository: scanoss/papi
Length of output: 2516
🏁 Script executed:
# Check if other proto files in the scanoss api directory have any top-level security field usage
rg -n 'repeated SecurityRequirement security|security:\s*\[' ./protobuf/scanoss/api/ --type proto -B 2 -A 8Repository: scanoss/papi
Length of output: 85
🏁 Script executed:
# Search for top-level security field usage in proto files
rg -n 'repeated SecurityRequirement security|^\s+security:\s+\[' ./protobuf/scanoss/api/ -A 8 -B 2Repository: scanoss/papi
Length of output: 38
🏁 Script executed:
# Look for SecurityRequirement usage in the attribution notices and other proto files
rg -n 'SecurityRequirement' ./protobuf/scanoss/api/Repository: scanoss/papi
Length of output: 38
🏁 Script executed:
# Verify the exact content of attribution-notices proto around the security/schemes area
sed -n '30,70p' ./protobuf/scanoss/api/attributionnotices/v2/scanoss-attribution-notices.protoRepository: scanoss/papi
Length of output: 1253
🏁 Script executed:
# Check if attribution-notices README also uses HTTPS and X-Api-Key like other services
grep -i "https\|X-Api-Key" ./protobuf/scanoss/api/attributionnotices/v2/README.md | head -10Repository: scanoss/papi
Length of output: 263
Change HTTP to HTTPS and add API key security definitions.
The proto currently advertises schemes: HTTP; but README examples require HTTPS and the X-Api-Key header. Generated clients may not include authentication without explicit security metadata.
Use the pattern from scanoss-components.proto as the reference for the correct syntax:
Proposed fix
- schemes: HTTP;
+ schemes: [HTTPS, HTTP];
consumes: "application/json";
produces: "application/json";
+ security_definitions: {
+ security: {
+ key: "api_key";
+ value: {
+ type: TYPE_API_KEY;
+ description: "API key for authentication";
+ name: "x-api-key";
+ in: IN_HEADER;
+ }
+ }
+ };
responses: {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@protobuf/scanoss/api/attributionnotices/v2/scanoss-attribution-notices.proto`
around lines 37 - 61, The OpenAPI swagger option block (option
(grpc.gateway.protoc_gen_openapiv2.options.openapiv2_swagger)) currently sets
schemes: HTTP and lacks API key security; update that block to set schemes:
HTTPS and add a securityDefinitions object defining an api_key (type: "apiKey",
name: "X-Api-Key", in: "header") and add a top-level security requirement that
references that api_key so generated clients will include the X-Api-Key header
by default; ensure you modify the same swagger option block that contains
info/consumes/produces/responses to include these securityDefinitions and
security entries.
| // MD5 hash of the notice text used for deduplication and integrity checks | ||
| string notice_md5 = 2 [json_name = "notice_md5"]; |
There was a problem hiding this comment.
Do not describe MD5 as an integrity check.
MD5 is fine for non-security deduplication, but documenting it as an integrity mechanism can mislead clients. If tamper detection matters, expose a stronger digest such as SHA-256.
Proposed wording fix
- // MD5 hash of the notice text used for deduplication and integrity checks
+ // MD5 hash of the notice text used for non-security deduplication.
string notice_md5 = 2 [json_name = "notice_md5"];📝 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.
| // MD5 hash of the notice text used for deduplication and integrity checks | |
| string notice_md5 = 2 [json_name = "notice_md5"]; | |
| // MD5 hash of the notice text used for non-security deduplication. | |
| string notice_md5 = 2 [json_name = "notice_md5"]; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@protobuf/scanoss/api/attributionnotices/v2/scanoss-attribution-notices.proto`
around lines 90 - 91, The comment for the protobuf field notice_md5 currently
calls MD5 an "integrity check" which is misleading; update the comment above the
field notice_md5 to state that the MD5 is used only for non-security
deduplication (e.g., "MD5 hash of the notice text used for deduplication") and
remove any mention of tamper detection or integrity guarantees, and if tamper
detection is required, add a new field (e.g., notice_sha256) and document it as
a strong digest intended for integrity/tamper detection.
agustingroh
left a comment
agustingroh
left a comment
There was a problem hiding this comment.
Please, update the CHANGELOG.md file
| // Get attribution notices for a list of components | ||
| // Accepts a ComponentsRequest (list of PURLs with optional version requirements) | ||
| // and returns the attribution notices associated with each component. | ||
| rpc GetComponentsAttributionNotices(scanoss.api.common.v2.ComponentsRequest) returns(ComponentsAttributionNoticesResponse) { | ||
| option (google.api.http) = { | ||
| post: "/v2/attribution-notices/components" | ||
| body: "*" | ||
| }; | ||
| }; | ||
| } |
There was a problem hiding this comment.
should we add a second endpoint v2/attribution-noticies/component for single request to keep compatibility with the other SCANOSS services?
2 participants
Summary by CodeRabbit
New Features
Documentation