gRPC JavaScript library - #7527
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThis PR adds a new ChangesgRPC JS Module
Estimated code review effort: 4 (Complex) | ~60 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Tools execution failed with the following error: Failed to run tools: 13 INTERNAL: Received RST_STREAM with code 2 (Internal server error) 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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
pkg/js/libs/grpc/grpc_test.go (1)
81-105: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate descriptor-set-building logic with
grpc_methods_test.go.This recursive
addclosure duplicatesmarshalDescriptorSetingrpc_methods_test.go(lines 112-134) almost line-for-line. Since both are in the samegrpctest package,healthProtosetcould simply delegate to the more general helper.♻️ Proposed consolidation
-func healthProtoset(t *testing.T) []byte { - t.Helper() - - fds := &descriptorpb.FileDescriptorSet{} - seen := map[string]bool{} - var add func(fd protoreflect.FileDescriptor) - add = func(fd protoreflect.FileDescriptor) { - if seen[fd.Path()] { - return - } - seen[fd.Path()] = true - imports := fd.Imports() - for i := 0; i < imports.Len(); i++ { - add(imports.Get(i).FileDescriptor) - } - fds.File = append(fds.File, protodesc.ToFileDescriptorProto(fd)) - } - add(healthpb.File_grpc_health_v1_health_proto) - - data, err := proto.Marshal(fds) - require.NoError(t, err) - return data -} +func healthProtoset(t *testing.T) []byte { + t.Helper() + return marshalDescriptorSet(t, healthpb.File_grpc_health_v1_health_proto) +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/js/libs/grpc/grpc_test.go` around lines 81 - 105, The descriptor-set construction in healthProtoset duplicates the recursive marshalDescriptorSet logic already present in grpc_methods_test.go. Refactor healthProtoset to reuse that shared helper instead of maintaining a second add closure, keeping healthProtoset focused on selecting healthpb.File_grpc_health_v1_health_proto and delegating the actual FileDescriptorSet serialization to marshalDescriptorSet.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@go.mod`:
- Line 237: Upgrade the indirect dependency github.com/go-jose/go-jose/v4 in
go.mod from v4.0.5 to v4.1.4 or later, and update the resolved module state so
the grpcurl/spiffe/xds dependency chain pulls in the fixed version. Verify the
module graph still builds cleanly after the version bump and that no older
transitive reference to go-jose remains pinned.
---
Nitpick comments:
In `@pkg/js/libs/grpc/grpc_test.go`:
- Around line 81-105: The descriptor-set construction in healthProtoset
duplicates the recursive marshalDescriptorSet logic already present in
grpc_methods_test.go. Refactor healthProtoset to reuse that shared helper
instead of maintaining a second add closure, keeping healthProtoset focused on
selecting healthpb.File_grpc_health_v1_health_proto and delegating the actual
FileDescriptorSet serialization to marshalDescriptorSet.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f77a0681-da60-4811-bf07-3a4af5d6954b
⛔ Files ignored due to path filters (6)
go.sumis excluded by!**/*.suminternal/tests/integration/testdata/protocols/javascript/grpc-denied.yamlis excluded by!**/*.yamlinternal/tests/integration/testdata/protocols/javascript/grpc-health.yamlis excluded by!**/*.yamlpkg/js/generated/go/libgrpc/grpc.gois excluded by!**/generated/**pkg/js/generated/ts/grpc.tsis excluded by!**/generated/**pkg/js/generated/ts/index.tsis excluded by!**/generated/**
📒 Files selected for processing (9)
go.modinternal/tests/integration/javascript_grpc_test.gointernal/tests/integration/javascript_test.gopkg/js/compiler/grpc_integration_test.gopkg/js/compiler/pool.gopkg/js/libs/grpc/grpc.gopkg/js/libs/grpc/grpc_methods_test.gopkg/js/libs/grpc/grpc_test.gopkg/js/libs/grpc/invoke.go
| github.com/go-fed/httpsig v1.1.0 // indirect | ||
| github.com/go-git/gcfg v1.5.1-0.20230307220236-3a3c6141e376 // indirect | ||
| github.com/go-git/go-billy/v5 v5.9.0 // indirect | ||
| github.com/go-jose/go-jose/v4 v4.0.5 // indirect |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '== go.mod context ==\n'
sed -n '220,245p' go.mod
printf '\n== go.sum matches ==\n'
rg -n 'github.com/go-jose/go-jose/v4' go.mod go.sum || true
printf '\n== repository usages ==\n'
rg -n 'go-jose|go-jose/v4|jose' . --glob '!go.sum' --glob '!*.min.*' || trueRepository: projectdiscovery/nuclei
Length of output: 1969
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '== possible callers/importers ==\n'
rg -n 'github.com/go-jose/go-jose/v4|go-jose|jwe|jws|jwt|oidc|openid' . \
--glob '!go.sum' --glob '!*.md' --glob '!vendor/**' || true
printf '\n== go mod why (if available) ==\n'
go mod why -m github.com/go-jose/go-jose/v4 2>/dev/null || trueRepository: projectdiscovery/nuclei
Length of output: 1280
🌐 Web query:
GHSA-78h2-9frx-2jm8 go-jose/go-jose v4.0.5 fixed in 4.1.4 JWE key-wrapping alg empty encrypted_key panic
💡 Result:
The vulnerability GHSA-78h2-9frx-2jm8 (also tracked as CVE-2026-34986) describes a denial-of-service vulnerability in the go-jose library that causes a panic during JWE (JSON Web Encryption) decryption [1][2]. The issue occurs when an application attempts to decrypt a JWE object where the alg header indicates a key-wrapping algorithm (excluding A128GCMKW, A192GCMKW, and A256GCMKW) and the encrypted_key field is empty [1][3]. The panic is triggered in the cipher.KeyUnwrap function within key_wrap.go because it attempts to allocate a slice with an invalid (zero or negative) length based on the length of the empty encrypted_key [1][3]. This vulnerability is fixed in: - go-jose/go-jose v4.1.4 [1][2] - go-jose/go-jose v3.0.5 [1][2] Applications that do not include key-wrapping algorithms in the list of accepted key algorithms passed to ParseEncrypted, ParseEncryptedJSON, or ParseEncryptedCompact functions are not affected [1][3]. For those using key-wrapping, it is recommended to upgrade to the patched versions or implement pre-validation to ensure the encrypted_key field is non-empty [1].
Citations:
- 1: https://osv.dev/vulnerability/GHSA-78h2-9frx-2jm8
- 2: https://pkg.go.dev/vuln/GO-2026-4945
- 3: https://advisories.gitlab.com/pkg/golang/github.com/go-jose/go-jose/v4/CVE-2026-34986/
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '== direct usage of grpcurl / spiffe / xds-related deps ==\n'
rg -n 'fullstorydev/grpcurl|go-spiffe|spiffe|xds|certprovider|credentials/xds' . \
--glob '!go.sum' --glob '!*.md' --glob '!vendor/**' || true
printf '\n== module graph snippets ==\n'
go mod why -m github.com/fullstorydev/grpcurl 2>/dev/null || true
go mod why -m github.com/spiffe/go-spiffe/v2 2>/dev/null || trueRepository: projectdiscovery/nuclei
Length of output: 1011
Upgrade github.com/go-jose/go-jose/v4 to v4.1.4+
This indirect dep comes in through grpcurl/spiffe/xds, and v4.0.5 is affected by GHSA-78h2-9frx-2jm8 when decrypting JWE with key-wrapping alg and an empty encrypted_key.
🧰 Tools
🪛 OSV Scanner (2.4.0)
[HIGH] 237-237: github.com/go-jose/go-jose/v4 4.0.5: Go JOSE Panics in JWE decryption in github.com/go-jose/go-jose
(GO-2026-4945)
[HIGH] 237-237: github.com/go-jose/go-jose/v4 4.0.5: Go JOSE Panics in JWE decryption
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@go.mod` at line 237, Upgrade the indirect dependency
github.com/go-jose/go-jose/v4 in go.mod from v4.0.5 to v4.1.4 or later, and
update the resolved module state so the grpcurl/spiffe/xds dependency chain
pulls in the fixed version. Verify the module graph still builds cleanly after
the version bump and that no older transitive reference to go-jose remains
pinned.
Source: Linters/SAST tools
3 participants
adds a grpc javascript library for nuclei templates, wrapping grpcurl. it supports server reflection and precompiled protosets, taking json requests and returning json responses. the client exposes invoke, invokewithheaders, listservices, listmethods, describesymbol, connect and close plus an options object for plaintext/tls, skip verify, server name, timeout, protoset path and max receive size.
connections are dialed through the execution fastdialer so host and ip denylists and restrict-local-network-access are enforced, with the host checked at construction and again per dial. protoset files are read through the local file access allowlist, so without -lfa only files inside the templates directory are allowed.
covered by unit tests against a local health service and the interop test service (unary with rich fields, request metadata, error status, server streaming, list/describe, protoset, malformed input, tls, and policy denials), a compiler test that runs require nuclei/grpc, and two signed javascript template integration tests.
Summary by CodeRabbit
New Features
require()-based module usage.Bug Fixes
Tests