mirror of
https://github.com/we-promise/sure.git
synced 2026-08-12 11:10:20 +00:00
* chore(deps): upgrade Rails 7.2 → 8.1 Rails 7.2 reaches end of life on 2026-08-09. Bump the framework to the current 8.1.x line. - Gemfile: rails "~> 8.0" (resolves 8.1.3); bundle update rails pulls the Rails 8 framework gems plus the bumps it requires — ViewComponent 3.23 → 4.x (Rails 8 support), rails-i18n 7 → 8, rswag, and transitive deps. - app/models/transfer.rb: make Transfer#date nil-safe (inflow_transaction&.entry&.date). Rails 8's date_field evaluates the field default on a new/unpersisted Transfer (the new-transfer form), where the association is nil; without this, TransfersController#new raises "undefined method 'entry' for nil". Matches the &. pattern already used in Transfer#sync_account_later. Framework behavioral defaults are unchanged (config.load_defaults stays as-is). Validated on Rails 8.1.3: zeitwerk:check passes, full suite green (4904 runs, 0 failures, 0 errors), rubocop and brakeman clean. * fix(rails8): style textarea + deterministic property edit system test The Rails 8 gem bump kept config.load_defaults at 7.2, but Rails 8 renamed two ActionView::Helpers::FormBuilder field helpers regardless of defaults: :text_area → :textarea and :check_box → :checkbox. StyledFormBuilder builds its styled helpers from `field_helpers`, so `form.text_area` (e.g. the account "Notes" field) silently fell through to the unstyled base helper and rendered without a label — failing 8 system tests with `Unable to find field "Notes"`. - app/helpers/styled_form_builder.rb: exclude both spellings of the non-text helpers (:check_box and :checkbox) and alias the legacy `text_area` to the Rails 8 `textarea` so existing call sites stay styled. Harmless on Rails 7.2 (old names present instead). - test/system/property_test.rb: open the property edit dialog via the account menu with a retry. The account page issues a Turbo morph refresh shortly after load (turbo_refreshes_with :morph + a family-stream broadcast); opening the modal while that refresh is in flight let the morph re-render the page and wipe the just-loaded #modal turbo-frame. Rails 8 timing made the race deterministic. Retrying once the refresh has settled makes the test stable (confirmed via Turbo frame-load vs full-page morph event traces; 3x green in isolation). - config/brakeman.ignore: the added comment block shifted the pre-existing (already-ignored, Weak) class_eval Dangerous Eval warning from line 5 -> 10, changing its fingerprint. Re-point the existing suppression to the new fingerprint/line so scan_ruby stays green. Validated on Rails 8.1.3: full system suite green (92 runs, 355 assertions, 0 failures, 0 errors), rubocop clean, brakeman 0 warnings, CodeRabbit no findings. * chore(deps): pin rails to the 8.1 minor line (~> 8.1.0) Tighten the constraint from `~> 8.0` to `~> 8.1.0` (>= 8.1.0, < 8.2) so a future `bundle update rails` tracks the 8.1.x line rather than silently jumping to 8.2 when it ships. Matches the upgrade plan's stated intent (target 8.1.x for the EOL runway) and a review note on #2301. No resolved-version changes: bundle install keeps rails at 8.1.3 and every other locked gem unchanged — only the Gemfile.lock DEPENDENCIES constraint line moves. zeitwerk:check still passes; the already-green unit/system suites ran on this exact resolved tree. * chore(rails8): adopt Rails 8.1 framework defaults (config.load_defaults 8.1) The gem bump above kept config.load_defaults at 7.2 so the change set could be reasoned about in stages; this finalizes the upgrade by adopting the modern framework defaults now that the suite is green on Rails 8.1. Rails 8.0 added no new framework defaults (there is no new_framework_defaults_8_0 template), so 7.2 -> 8.1 is the single meaningful step. No incremental new_framework_defaults_8_1.rb opt-in file is needed: the full suites pass with all 8.1 defaults enabled at once. The 8.1 defaults this turns on include action_on_path_relative_redirect=:raise (open-redirect hardening), raise_on_missing_required_finder_order_columns, escape_json_responses=false / escape_js_separators_in_json=false (JSON perf), and Ruby-parser template-dependency tracking. Validated with no application code changes: bin/rails test 4904/0/0, bin/rails test:system 92/0/0, rubocop + brakeman clean. * chore(ci): restore brakeman CheckEOLRails now that the app is on Rails 8.1 config/brakeman.yml existed only to skip brakeman's CheckEOLRails. That check fires on the calendar (it warns 60 days before a framework's EOL and escalates as the date nears), so Rails 7.2's 2026-08-09 EOL turned `bin/brakeman` red (exit 3) on every branch and on main regardless of the diff. The skip carried a TODO to remove it once Sure upgraded off 7.2. This PR puts the app on Rails 8.1 (EOL well in the future), so the skip is obsolete; remove the file (its sole content was the skip) in the same change that makes it unnecessary -- no stale-config window. brakeman auto-loads the file when present and falls back to defaults when absent, and nothing references it explicitly. CheckEOLRuby was already enabled and is unchanged; config/brakeman.ignore is untouched. Validated on Rails 8.1: bin/brakeman runs EOLRails + EOLRuby, 0 warnings, 0 errors, exit 0.
179 lines
7.5 KiB
Plaintext
179 lines
7.5 KiB
Plaintext
{
|
|
"ignored_warnings": [
|
|
{
|
|
"warning_type": "Redirect",
|
|
"warning_code": 18,
|
|
"fingerprint": "556f2fdd1f091ed50811cb2cce28dd2b987cd0a2eed4d19bea138c8c083a3a5d",
|
|
"check_name": "Redirect",
|
|
"message": "Possible unprotected redirect",
|
|
"file": "app/controllers/snaptrade_items_controller.rb",
|
|
"line": 125,
|
|
"link": "https://brakemanscanner.org/docs/warning_types/redirect/",
|
|
"code": "redirect_to(Current.family.snaptrade_items.find(params[:id]).connection_portal_url(:redirect_url => callback_snaptrade_items_url(:item_id => Current.family.snaptrade_items.find(params[:id]).id)), :allow_other_host => true)",
|
|
"render_path": null,
|
|
"location": {
|
|
"type": "method",
|
|
"class": "SnaptradeItemsController",
|
|
"method": "connect"
|
|
},
|
|
"user_input": "Current.family.snaptrade_items.find(params[:id]).connection_portal_url(:redirect_url => callback_snaptrade_items_url(:item_id => Current.family.snaptrade_items.find(params[:id]).id))",
|
|
"confidence": "Weak",
|
|
"cwe_id": [
|
|
601
|
|
],
|
|
"note": "Intentional redirect to SnapTrade's external OAuth portal for brokerage connection"
|
|
},
|
|
{
|
|
"warning_type": "Redirect",
|
|
"warning_code": 18,
|
|
"fingerprint": "723b1970ca6bf16ea0c2c1afa0c00d3c54854a16568d6cb933e497947565d9ab",
|
|
"check_name": "Redirect",
|
|
"message": "Possible unprotected redirect",
|
|
"file": "app/controllers/family_exports_controller.rb",
|
|
"line": 30,
|
|
"link": "https://brakemanscanner.org/docs/warning_types/redirect/",
|
|
"code": "redirect_to(Current.family.family_exports.find(params[:id]).export_file, :allow_other_host => true)",
|
|
"render_path": null,
|
|
"location": {
|
|
"type": "method",
|
|
"class": "FamilyExportsController",
|
|
"method": "download"
|
|
},
|
|
"user_input": "Current.family.family_exports.find(params[:id]).export_file",
|
|
"confidence": "Weak",
|
|
"cwe_id": [
|
|
601
|
|
],
|
|
"note": ""
|
|
},
|
|
{
|
|
"warning_type": "Mass Assignment",
|
|
"warning_code": 105,
|
|
"fingerprint": "81c63f2c375da309440b9308df3ae9d1fcbada7043a26919898b08f3a38b29f6",
|
|
"check_name": "PermitAttributes",
|
|
"message": "Potentially dangerous key allowed for mass assignment",
|
|
"file": "app/controllers/api/v1/trades_controller.rb",
|
|
"line": 159,
|
|
"link": "https://brakemanscanner.org/docs/warning_types/mass_assignment/",
|
|
"code": "params.require(:trade).permit(:account_id, :date, :qty, :price, :currency, :security_id, :ticker, :manual_ticker, :investment_activity_label, :category_id, :fee, :type, :amount, :transfer_account_id)",
|
|
"render_path": null,
|
|
"location": {
|
|
"type": "method",
|
|
"class": "Api::V1::TradesController",
|
|
"method": "trade_params"
|
|
},
|
|
"user_input": ":account_id",
|
|
"confidence": "High",
|
|
"cwe_id": [
|
|
915
|
|
],
|
|
"note": "account_id validated in create action (line 42: family.accounts.visible.find ensures family membership; lines 44-48: supports_trades? check); security_id validated via Security.find (line 290); transfer_account_id validated in Transfer::Creator where family.accounts.find scopes both source and destination accounts to the family (app/models/transfer/creator.rb lines 4-5); type validated in build_create_form_params against allowed values: buy, sell, dividend, deposit, withdrawal, interest (line 217)"
|
|
},
|
|
{
|
|
"warning_type": "Mass Assignment",
|
|
"warning_code": 105,
|
|
"fingerprint": "aaccd8db0be34afdc88e5af08d91ae2e8b7765dfea2f3fc6e1c37db0adc7b991",
|
|
"check_name": "PermitAttributes",
|
|
"message": "Potentially dangerous key allowed for mass assignment",
|
|
"file": "app/controllers/invitations_controller.rb",
|
|
"line": 58,
|
|
"link": "https://brakemanscanner.org/docs/warning_types/mass_assignment/",
|
|
"code": "params.require(:invitation).permit(:email, :role)",
|
|
"render_path": null,
|
|
"location": {
|
|
"type": "method",
|
|
"class": "InvitationsController",
|
|
"method": "invitation_params"
|
|
},
|
|
"user_input": ":role",
|
|
"confidence": "Medium",
|
|
"cwe_id": [
|
|
915
|
|
],
|
|
"note": ""
|
|
},
|
|
{
|
|
"warning_type": "Mass Assignment",
|
|
"warning_code": 105,
|
|
"fingerprint": "01a88a0a17848e70999c17f6438a636b00e01da39a2c0aa0c46f20f0685c7202",
|
|
"check_name": "PermitAttributes",
|
|
"message": "Potentially dangerous key allowed for mass assignment",
|
|
"file": "app/controllers/admin/users_controller.rb",
|
|
"line": 35,
|
|
"link": "https://brakemanscanner.org/docs/warning_types/mass_assignment/",
|
|
"code": "params.require(:user).permit(:role)",
|
|
"render_path": null,
|
|
"location": {
|
|
"type": "method",
|
|
"class": "Admin::UsersController",
|
|
"method": "user_params"
|
|
},
|
|
"user_input": ":role",
|
|
"confidence": "Medium",
|
|
"cwe_id": [
|
|
915
|
|
],
|
|
"note": "Protected by Pundit authorization - UserPolicy requires super_admin and prevents users from changing their own role"
|
|
},
|
|
{
|
|
"warning_type": "Dangerous Eval",
|
|
"warning_code": 13,
|
|
"fingerprint": "ad8e31fe9321feba741d551654a6fb0c0c7cfe8abee81822e6143404817bff5b",
|
|
"check_name": "Evaluation",
|
|
"message": "Dynamic string evaluated as code",
|
|
"file": "app/helpers/styled_form_builder.rb",
|
|
"line": 10,
|
|
"link": "https://brakemanscanner.org/docs/warning_types/dangerous_eval/",
|
|
"code": "class_eval(\" def #{selector}(method, options = {})\\n form_options = options.slice(:label, :label_tooltip, :inline, :container_class, :required)\\n html_options = options.except(:label, :label_tooltip, :inline, :container_class)\\n\\n build_field(method, form_options, html_options) do |merged_options|\\n super(method, merged_options)\\n end\\n end\\n\", \"app/helpers/styled_form_builder.rb\", (10 + 1))",
|
|
"render_path": null,
|
|
"location": {
|
|
"type": "method",
|
|
"class": "StyledFormBuilder",
|
|
"method": null
|
|
},
|
|
"user_input": null,
|
|
"confidence": "Weak",
|
|
"cwe_id": [
|
|
913,
|
|
95
|
|
],
|
|
"note": "Uses similar pattern to Rails internal form builder"
|
|
},
|
|
{
|
|
"warning_type": "Dynamic Render Path",
|
|
"warning_code": 15,
|
|
"fingerprint": "fb6f7abeabc405d6882ffd41dbe8016403ef39307a5c6b4cd7b18adfaf0c24bf",
|
|
"check_name": "Render",
|
|
"message": "Render path contains parameter value",
|
|
"file": "app/views/import/configurations/show.html.erb",
|
|
"line": 34,
|
|
"link": "https://brakemanscanner.org/docs/warning_types/dynamic_render_path/",
|
|
"code": "render(partial => permitted_import_configuration_path(Current.family.imports.find(params[:import_id])), { :locals => ({ :import => Current.family.imports.find(params[:import_id]) }) })",
|
|
"render_path": [
|
|
{
|
|
"type": "controller",
|
|
"class": "Import::ConfigurationsController",
|
|
"method": "show",
|
|
"line": 7,
|
|
"file": "app/controllers/import/configurations_controller.rb",
|
|
"rendered": {
|
|
"name": "import/configurations/show",
|
|
"file": "app/views/import/configurations/show.html.erb"
|
|
}
|
|
}
|
|
],
|
|
"location": {
|
|
"type": "template",
|
|
"template": "import/configurations/show"
|
|
},
|
|
"user_input": "params[:import_id]",
|
|
"confidence": "Weak",
|
|
"cwe_id": [
|
|
22
|
|
],
|
|
"note": ""
|
|
}
|
|
],
|
|
"brakeman_version": "7.1.0"
|
|
}
|