-
Notifications
You must be signed in to change notification settings - Fork 6
Support Rack 3 (attempt #2) #96
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
Conversation
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.
⚠️ 1 New Security Finding
The latest commit contains 1 new security finding.
| Findings | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Dependency: rubygems / rack@ 3.1.13 SUMMARY Direct Dependency: rack Location : Gemfile.lock OCCURRENCES
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
Remediation :
|
Not a finding? Ignore it by adding a comment on the line with just the word noboost.
Scanner: boostsecurity - bundler-audit
|
All alerts resolved. Learn more about Socket for GitHub. This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. Ignoring alerts on:
|
|
@SocketSecurity ignore-all |
Removed browser console log printing on test failure.
| let(:node_client) { File.expand_path('../node-client/dist/client.js', __dir__) } | ||
| let(:node_client_dir) { File.expand_path('../node-client', __dir__) } | ||
| let(:node_cmd) do | ||
| # Use npm for dependency management (avoids yarn workspace detection issues) |
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.
I don't know if this is a bad idea. It's an AI fix for these specs.
Rakefile
Outdated
| '--plugin=protoc-gen-js=/usr/lib/node_modules/protoc-gen-js/bin/protoc-gen-js', | ||
| '--plugin=protoc-gen-grpc-web=/usr/local/bin/protoc-gen-grpc-web', | ||
| '--js_out=import_style=commonjs:/defs/pb-js-grpc-web', | ||
| '--grpc-web_out=import_style=commonjs,mode=grpcweb:/defs/pb-js-grpc-web', |
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.
One of the main fixes was updating how these are generated to use updated plugins
| require 'capybara/rspec' | ||
| require 'selenium-webdriver' | ||
|
|
||
| Capybara.register_driver :selenium_chrome_standalone do |app| |
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.
Some selenium wrangling was required to get the specs working
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.
🚀 1 New Security Fix
You just committed 1 security fix. 😎 Keep up the great work!
🎯 Take a look at what findings you fixed.
| Findings | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Dependency: rubygems / rack@ 3.1.13 SUMMARY Direct Dependency: rack Location : Gemfile.lock OCCURRENCES
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
Remediation :
|
Scanner: boostsecurity - bundler-audit
|
This pull request includes insecure supply-chain practices: GitHub Actions use mutable tags (e.g., @v2, @v1) rather than pinned commit SHAs, the Dockerfile downloads protoc and protoc-gen-grpc-web without integrity checks, and it fetches and immediately executes a remote nodesource setup script with no verification. These issues increase risk of silent compromise of CI/build artifacts and should be mitigated by pinning actions and verifying all downloaded artifacts (checksums/GPG) before execution.
Insecure Supply Chain via Mutable GitHub Action Tags in
|
| Vulnerability | Insecure Supply Chain via Mutable GitHub Action Tags |
|---|---|
| Description | The GitHub Actions workflow in .github/workflows/ci.yml uses mutable version tags (@v2 and @v1) for actions/checkout and ruby/setup-ruby respectively. This practice introduces a supply chain risk because the code associated with these tags can be updated by the action maintainers at any time. If an action's repository is compromised, malicious code could be injected into the CI/CD pipeline without any changes being visible in the pull request, leading to potential compromise of the build process or deployed artifacts. The industry best practice for securing GitHub Actions is to pin actions to a specific, immutable commit SHA. |
grpc-web-ruby/.github/workflows/ci.yml
Lines 16 to 20 in 3e9bea2
| - name: Checkout code | |
| uses: actions/checkout@v2 | |
| - name: Set up Ruby 3.4 | |
| uses: ruby/setup-ruby@v1 |
Unpinned External Dependencies without Integrity Check in Dockerfile.protoc
| Vulnerability | Unpinned External Dependencies without Integrity Check |
|---|---|
| Description | The Dockerfile.protoc downloads protoc and protoc-gen-grpc-web binaries from GitHub releases using curl -OL without any subsequent integrity verification (e.g., using sha256sum). This practice makes the build process vulnerable to supply chain attacks, where a compromised download source or a man-in-the-middle attack could inject malicious code into the downloaded binaries without detection. Secure development practices for Dockerfiles strongly recommend verifying the integrity of all downloaded external artifacts using checksums. |
grpc-web-ruby/Dockerfile.protoc
Lines 18 to 26 in 3e9bea2
| RUN PROTOC_ZIP=protoc-28.0-linux-x86_64.zip \ | |
| && curl -OL https://github.com/protocolbuffers/protobuf/releases/download/v28.0/$PROTOC_ZIP \ | |
| && unzip -o $PROTOC_ZIP -d /usr/local bin/protoc \ | |
| && unzip -o $PROTOC_ZIP -d /usr/local 'include/*' \ | |
| && rm -f $PROTOC_ZIP | |
| # Install modern protoc-gen-grpc-web plugin (compatible with grpc-web 1.5.0+) | |
| RUN GRPC_WEB_VERSION=1.5.0 \ | |
| && curl -OL https://github.com/grpc/grpc-web/releases/download/${GRPC_WEB_VERSION}/protoc-gen-grpc-web-${GRPC_WEB_VERSION}-linux-x86_64 \ |
Insecure Script Execution (curl -o file && bash file) in Dockerfile.protoc
| Vulnerability | Insecure Script Execution (curl -o file && bash file) |
|---|---|
| Description | The Dockerfile.protoc downloads a shell script (nodesource_setup.sh) from a remote URL and executes it immediately using bash without any integrity verification (e.g., checksum or GPG signature). This pattern is inherently insecure because if the remote source (deb.nodesource.com) were compromised, an attacker could inject malicious code into the script, leading to arbitrary code execution within the build environment. This could compromise the build process, inject backdoors, or exfiltrate sensitive information. |
grpc-web-ruby/Dockerfile.protoc
Lines 11 to 12 in 3e9bea2
| RUN curl -fsSL https://deb.nodesource.com/setup_22.x -o nodesource_setup.sh \ | |
| && bash nodesource_setup.sh \ |
All finding details can be found in the DryRun Security Dashboard.
joemsak
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.
Looking good so far!
I'm looking into that change in spec/integration/ruby_server_nodejs_client_spec.rb in the meantime
Co-authored-by: Joe Sak <joe@joesak.com>
|
@mattbooks what do you think of this? #97 I asked my agent to assess that questionable change in |
* Update .gitignore to allow tracking .yarnrc.yml files Change .yarn* to .yarn/ and .pnp.* to be more specific. This allows .yarnrc.yml configuration files to be tracked. * Add .yarnrc.yml files for yarn workspace isolation Add standalone .yarnrc.yml to spec/node-client/ so Yarn 4 treats it as an independent project rather than detecting it as a workspace member of the root project. * Remove npm install workaround from node client test With yarn workspace isolation fixed, we no longer need to run npm install on every test run. The test now just executes the pre-built node client directly. - Remove npm install command from test - Delete package-lock.json (use yarn consistently) - Update yarn.lock files * Trigger gh actions
|
@joemsak thanks — that fix is in line with what I felt was odd about it. merged it in |
What is this change doing?
Why is this change being made?
How did you test this change?
gus-gatewaybased on #92 from @ngan and @joemsak