[#146251] Fix Security Vulnerabilities (Bundle Audit + Brakeman)#178
Open
jhephs wants to merge 5 commits into
Open
[#146251] Fix Security Vulnerabilities (Bundle Audit + Brakeman)#178jhephs wants to merge 5 commits into
jhephs wants to merge 5 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates dependencies and applies a set of scanner-driven security remediations (bundle-audit, Brakeman, npm audit), including targeted hardening in Rails views/services/jobs and regenerated frontend bundles.
Changes:
- Upgraded Ruby gems (Rails, Devise, Rack, Nokogiri, etc.) and patched JS deps / vendored code to address reported CVEs.
- Replaced/removed Brakeman-flagged patterns (e.g.,
raw, unsafe URL usage, raw SQL construction, unsafe file open). - Stabilized test suite and updated documentation/tooling to reflect the new security workflow and runtime pinning.
Reviewed changes
Copilot reviewed 18 out of 23 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
Gemfile |
Pins/bumps key gems (Rails, Devise, Brakeman, Nokogiri, ERB pin) for security compatibility. |
Gemfile.lock |
Resolves updated gem set after security-driven upgrades. |
.bundler-audit.yml |
Adds ignore entry for withdrawn advisory. |
package-lock.json |
Applies npm audit fix results and refreshes lockfile metadata. |
gulp/js/vendor/jquery-deparam.js |
Vendored mitigation for prototype pollution in jquery-deparam. |
public/assets/js/default-619d4d9aa1f684a37bc97ff14eb52224.js |
Recompiled bundle reflecting the jquery-deparam patch. |
public/assets/js/admin-8fd0995068515e7165dd569b7cade25a.js |
Recompiled bundle reflecting the jquery-deparam patch. |
public/assets/js/templates-21b11d937a1e6949a886b8b9244bf896.js |
Recompiled templates bundle (ordering/contents updated by build). |
config/brakeman.ignore |
Removes obsolete ignores and updates remaining ignore notes/fingerprints. |
app/controllers/api/csp_reports_controller.rb |
Removes unused params.permit! mass-assignment method. |
app/jobs/azure/speech_to_text_job.rb |
Adds realpath + /tmp constraint before opening wav file. |
spec/jobs/azure/speech_to_text_job_spec.rb |
Updates stubs to accommodate hardened wav path validation. |
spec/features/admin/institutions_spec.rb |
Adds navigation wait to reduce flakiness after “Save”. |
app/views/home/_hero_content.html.erb |
Replaces raw with sanitize and adds URL scheme allowlisting. |
app/views/admin/cms/collections/show.html.erb |
Replaces raw with sanitize for collection descriptions. |
app/services/reports/user_contributions.rb |
Refactors raw SQL filters toward parameterization. |
app/models/transcript_line.rb |
Tweaks best-edit selection logic (priority edits + initialization). |
app/models/transcript_edit.rb |
Adds deterministic ordering and adjusts cache key construction. |
app/views/layouts/_account.html.erb |
Normalizes data-* attributes into Rails data: hash form. |
README.md |
Updates security scanning guidance and modernizes setup/workflow docs. |
.github/copilot-instructions.md |
Adds repo-specific assistant context and conventions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
b29379a to
befd23f
Compare
befd23f to
7a6d465
Compare
7a6d465 to
37c358c
Compare
37c358c to
e71ec4f
Compare
e71ec4f to
ec8b620
Compare
ec8b620 to
a308793
Compare
a308793 to
5d0fd45
Compare
5d0fd45 to
545e392
Compare
545e392 to
e24f406
Compare
e24f406 to
2f91025
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 28 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ticket: https://reinteractive.zendesk.com/agent/tickets/146251
Overview
This PR addresses all security vulnerabilities identified by
bundle audit,brakeman, andnpm audit. It includes Ruby gem upgrades, 6 code-level security fixes found by Brakeman, JavaScript dependency patching, and developer tooling improvements (AI helper files, Node version pinning).All three scanners are now clean or at known-acceptable state:
bundle audit: No vulnerabilities foundbrakeman: 0 active warnings, 0 errorsnpm audit --omit=dev: 3 remaining (jQuery/j-toker — require breaking changes or have no upstream fix)bundle exec rspec: 993 examples, 0 failuresSecurity Vulnerabilities Fixed
Bundle Audit — Ruby Dependency CVEs
Supporting upgrade:
autoprefixer-rails8.6.5 → 10.4.21.0 (required by Bootstrap 4.3+)Brakeman — Code Security Issues (6 Fixed)
app/controllers/api/csp_reports_controller.rbparams.permit!app/views/home/_hero_content.html.erbrawon user-controlled titlesanitizeapp/views/admin/cms/collections/show.html.erbrawon DB contentsanitize; removedtargetattr to prevent reverse-tabnabbingapp/views/home/_hero_content.html.erblink_to//; addrel="noopener noreferrer"app/services/reports/user_contributions.rb$Nplaceholders with typedQueryAttributebindsapp/jobs/azure/speech_to_text_job.rbFile.openwithout validationrealpathcheck against resolved/tmp(macOS-compatible)Brakeman Ignore Cleanup
npm audit — JavaScript Dependencies
Auto-fixed (via
npm audit fix):debug(ReDoS) — upgradedws(DoS) — upgradedManually patched:
jquery-deparam(Prototype Pollution, GHSA-xg68-chx2-253g) — added__proto__/constructor/prototypekey rejection in vendored source (gulp/js/vendor/jquery-deparam.js) and recompiled JS bundles.npm auditstill flags this because it checks package version, not code — the vulnerability is mitigated at the source level.Remaining (1 production vulnerability, requires breaking jQuery upgrade):
jquery <= 3.4.1Technical Approach
ERB Pinning (Sprockets Compatibility)
Upgrading Rails to 8.0.2.1 pulled in ERB 6.0, which is incompatible with Sprockets 3.7.x. Instead of a risky Sprockets 4.x upgrade (would require sass-rails 6.x + asset pipeline refactoring), we pinned ERB to 5.x:
Devise 4.x → 5.x Upgrade
The Devise major version upgrade (CVE-2026-32700) was validated against the full test suite. This project uses
:confirmablewithreconfirmable: true— directly affected by the advisory. The upgrade to 5.0.3 passed all 966 specs with zero changes required.Bundle Audit Ignore
Code Review Hardening (2026-03-26)
After code review, the following issues were identified and fixed:
collections/show.html.erb— removedtargetfrom sanitize allowed attributes_hero_content.html.erb— regex now rejects//evil.examplepatterns; addedrel="noopener noreferrer"totarget="_blank"linksuser_contributions.rb— switched from named:placeholders(unsupported byexec_query) to positional$1, $2with typedActiveRecord::Relation::QueryAttributebindsspeech_to_text_job.rb— compare againstPathname.new('/tmp').realpathinstead of hardcoded/tmp/(macOS resolves to/private/tmp)config.json→project.jsonto match actual codebase conventionspeech_to_text_job.rb—wav_filewas opened but never closed; addedwav_file.close unless wav_file.nil? || wav_file.closed?inensureblockinstitutions_spec.rb— removed stale comment claiming the controller might re-render the edit pageuser_contributions.rb—to_csvwas outputtingline_countbeforeedit_count, mismatched with headers (Editscolumn 3,Linescolumn 4); swapped orderrescue Exceptioninspeech_to_text_job.rb— changed torescue StandardErrorsoSignalException/NoMemoryErrorare not swallowed; also fixedSpeechToTextServiceto raiseRuntimeErrorinstead of bareExceptionlet(:wav_file)—File.openin aletwas never closed; addedafter { wav_file.close unless wav_file.closed? }npm audit --omit=devuser_contributions.rb— header saiduser_activity.rb; corrected touser_contributions.rbFile.openfor image param replaced withfixture_file_uploadRecordNotFoundguard inspeech_to_text_job.rb— added explicitrescue ActiveRecord::RecordNotFoundbeforerescue StandardErrorso a deleted-record retry raises cleanly instead of masking withNoMethodErrorraise ExceptioninSpeechToTextService— bothrescue Exceptionandraise Exceptionreplaced withStandardError/RuntimeErrorso system-level exceptions (signals, OOM) are never swallowed by the servicebrakeman.ignorenote for the File Access warning — updated wording to reflect the actualPathname#realpathcheck (which resolves symlinks, e.g. macOS/private/tmp)allow_any_instance_of(Pathname)stubs in job spec — replaced withand_wrap_originalto match only the two specific runtime paths (/tmpand*.wav), so unrelated Pathnames (likeRails.root) are not intercepteduser_contributions.rb—collection_id/institution_idnow validated with/\A\d+\z/before binding; non-integer values (including injection strings) are silently ignored rather than coerced via.to_iurl = nilinitializer in_hero_content.html.erb—urlwas only assigned inside theinstitutionbranch; when institution is absent the laterurl.present?raisedNameError; fixed by initializing tonilbefore the conditionalraise e→ bareraiseinSpeechToTextService#recognize— usingraise eresets the backtrace to the rescue line; bareraisere-raises the original exception with its original backtrace intactREADME.md— updated from EOL 9.5 to 14+ (9.5 reached end-of-life in 2021)brakeman.ignoreSQL injection note — updated to reflect that integer IDs are validated with/\A\d+\z/and skip the filter on invalid input (previously said.to_icoercion, which is no longer accurate)brakeman.ignoreLinkToHref fingerprint — updated fingerprint/line/code for the_hero_contentXSS ignore entry after theurl = nilinitializer shifted Brakeman’s analysis of thelink_tocallAll Dependencies Updated
Additional Changes
Stability Fixes (to keep specs green through upgrades)
TranscriptEdit.getByLine— added deterministic ordering (updated_at DESC, id DESC)spec/jobs/azure/speech_to_text_job_spec.rb— updated stubs for hardened file path validationspec/features/admin/institutions_spec.rb— added post-save navigation wait for flaky feature specDeveloper Tooling
.github/copilot-instructions.md(new) — repo-wide AI assistant context for GitHub Copilot.tool-versions(updated) — pinnedruby 3.4.4+nodejs 20.18.0for consistentnpm auditsupportREADME.md— updated Security Scanning section withbundle audit update,npm audit, Node version notepackage-lock.json— updated vianpm audit fix(debug, ws patched)Verification
Testing Instructions
bundle install && npm installFiles Changed
Dependencies & Config:
Gemfile— gem version bumps (devise 5.x, bcrypt, brakeman 8.x, ERB pin)Gemfile.lock— resolved dependenciespackage-lock.json— npm audit fix (debug, ws).bundler-audit.yml— withdrawn CVE ignore.tool-versions— pinned ruby 3.4.4 + nodejs 20.18.0Security Fixes:
app/controllers/api/csp_reports_controller.rb— removed mass assignmentapp/views/home/_hero_content.html.erb— XSS fixes (sanitize + URL validation); url=nil initializerapp/views/admin/cms/collections/show.html.erb— XSS fixapp/services/reports/user_contributions.rb— SQL injection fix; CSV column order corrected; integer param validation for collection_id/institution_idapp/jobs/azure/speech_to_text_job.rb— file path validation; wav_file FD closed in ensure; rescue StandardErrorapp/services/azure/speech_to_text_service.rb— raise RuntimeError instead of bare Exception; rescue StandardError instead of Exception in recognize; bare raise to preserve backtrace; removed unused=> evariable; renamed unusedstdoutto_stdoutapp/models/transcript_edit.rb— deterministic orderinggulp/js/vendor/jquery-deparam.js— prototype pollution fix (GHSA-xg68-chx2-253g)public/assets/js/*— recompiled bundles with patched jquery-deparamConfig & Documentation:
config/brakeman.ignore— cleaned up obsolete entries; updated File Access note to reflect realpath-based validation; updated SQL injection note to reflect integer regex validation; updated LinkToHref fingerprint after url=nil changeREADME.md— updated security scanning section;npm audit→npm audit --omit=devthroughout; PostgreSQL minimum updated from EOL 9.5 to 14+.github/copilot-instructions.md— repo-wide AI assistant context (new)Spec Fixes & Coverage:
spec/jobs/azure/speech_to_text_job_spec.rb— updated stubs for path validation; added invalid path (traversal) test; scoped wav_file after hook to #recognize block; added RecordNotFound test; narrowed Pathname realpath stubs with and_wrap_originalspec/features/admin/institutions_spec.rb— stabilized flaky test; removed stale commentspec/services/reports/user_contributions_spec.rb— new: full coverage forUserContributionsservice; CSV column alignment tests; integer validation tests (skip filter on non-integer input)spec/controllers/home_controller_spec.rb— added render_views test: home index with institution:nil renders without NameErrorspec/models/transcript_edit_spec.rb— addedgetByLineordering tests (updated_at DESC, id DESC tiebreaker)spec/controllers/admin/cms/collections_controller_spec.rb— added XSS/sanitization tests; replacedFile.openwithfixture_file_uploadFuture Work
.bind()/.unbind()APIs).unbind(),$.parseJSON()removed in jQuery 3.x).size()calls in ERB templates