Files
sure/config/brakeman.ignore
MkDev11 d88c2151cb Add REST API for holdings and trades (Discussion #905) (#918)
* Add REST API for holdings and trades (Discussion #905)

- Trades: GET index (filter by account_id, account_ids, start_date, end_date),
  GET show, POST create (buy/sell with security_id or ticker), PATCH update,
  DELETE destroy. Create restricted to accounts that support trades (investment
  or crypto exchange). Uses existing Trade::CreateForm for creation.
- Holdings: GET index (filter by account_id, account_ids, date, start_date,
  end_date, security_id), GET show. Read-only; scoped to family.
- Auth: read scope for index/show; write scope for create/update/destroy.
- Responses: JSON via jbuilder (trade: id, date, amount, qty, price, account,
  security, category; holding: id, date, qty, price, amount, account, security,
  avg_cost). Pagination for index endpoints (page, per_page).

Co-authored-by: Cursor <cursoragent@cursor.com>

* API v1 holdings & trades: validation, docs, specs

- Holdings: validate date params, return 400 for invalid dates (parse_date!)
- Trades: validate start_date/end_date, return 422 for invalid dates
- Trades: accept buy/sell and inflow/outflow in update (trade_sell_from_type_or_nature?)
- Trades view: nil guard for trade.security
- Trades apply_filters: single join(:entry) when filtering
- OpenAPI: add Trade/TradeCollection schemas, ErrorResponse.errors
- Add spec/requests/api/v1/holdings_spec.rb and trades_spec.rb (rswag)
- Regenerate docs/api/openapi.yaml

Co-authored-by: Cursor <cursoragent@cursor.com>

* CI: fix Brakeman and test rate-limit failures

- Disable Rack::Attack in test (use existing enabled flag) so parallel
  API tests no longer hit 429 from shared api_ip throttle
- Add Brakeman ignore for trades_controller trade_params mass-assignment
  (account_id/security_id validated in create/update)
- Trades/holdings API and OpenAPI spec updates

Co-authored-by: Cursor <cursoragent@cursor.com>

* Trades: partial qty/price update fallback; fix PATCH OpenAPI schema

- Fall back to existing trade qty/price when only one is supplied so sign
  normalisation and amount recalculation always run
- OpenAPI: remove top-level qty, price, investment_activity_label,
  category_id from PATCH body; document entryable_attributes only

Co-authored-by: Cursor <cursoragent@cursor.com>

* Trades: fix update/DELETE OpenAPI and avoid sell-trade corruption

- Only run qty/price normalisation when client sends qty or price; preserve
  existing trade direction when type/nature omitted
- OpenAPI: remove duplicate PATCH path param; add 422 for PATCH; document
  DELETE 200 body (DeleteResponse)

Co-authored-by: Cursor <cursoragent@cursor.com>

* API: flat trade update params, align holdings errors, spec/OpenAPI fixes

- Trades update: accept flat params (qty, price, type, etc.), build
  entryable_attributes in build_entry_params_for_update (match transactions)
- Holdings: ArgumentError → 422 validation_failed; parse_date!(value, name)
  with safe message; extract render_validation_error, log_and_render_error
- Specs: path id required (trades, holdings); trades delete 200 DeleteResponse;
  remove holdings 500; trades update body flat; holdings 422 invalid date
- OpenAPI: PATCH trade request body flat

Co-authored-by: Cursor <cursoragent@cursor.com>

* OpenAPI: add 422 invalid date filter to holdings index

Co-authored-by: Cursor <cursoragent@cursor.com>

* API consistency and RSwag doc-only fixes

- Trades: use render_validation_error in all 4 validation paths; safe_per_page_param case/when
- Holdings: set_holding to family.holdings.find; price as Money.format in API; safe_per_page_param case/when
- Swagger: Holding qty/price descriptions (Quantity of shares held, Formatted price per share)
- RSwag: trades delete and valuations 201 use bare run_test! (documentation only, no expect)

Co-authored-by: Cursor <cursoragent@cursor.com>

* Fix index-vs-show visibility inconsistencies and preserve custom activity labels

- Add account status filter to set_holding to match index behavior
- Add visible scope to set_trade to match index behavior
- Preserve existing investment_activity_label when updating qty/price

Co-authored-by: Cursor <cursoragent@cursor.com>

* Trades: clearer validation for non-numeric qty/price

Return 'must be valid numbers' when qty or price is non-numeric (e.g. abc)
instead of misleading 'must be present and positive'.

Co-authored-by: Cursor <cursoragent@cursor.com>

---------

Co-authored-by: mkdev11 <jaysmth689+github@users.noreply.github.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
2026-02-08 11:22:32 +01:00

202 lines
8.1 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": "85e2c11853dd6c69b1953a6ec3ad661cd0ce3df55e4e5beff92365b6ed601171",
"check_name": "PermitAttributes",
"message": "Potentially dangerous key allowed for mass assignment",
"file": "app/controllers/api/v1/transactions_controller.rb",
"line": 255,
"link": "https://brakemanscanner.org/docs/warning_types/mass_assignment/",
"code": "params.require(:transaction).permit(:account_id, :date, :amount, :name, :description, :notes, :currency, :category_id, :merchant_id, :nature, :tag_ids => ([]))",
"render_path": null,
"location": {
"type": "method",
"class": "Api::V1::TransactionsController",
"method": "transaction_params"
},
"user_input": ":account_id",
"confidence": "High",
"cwe_id": [
915
],
"note": "account_id is properly validated in create action - line 79 ensures account belongs to user's family: family.accounts.find(transaction_params[:account_id])"
},
{
"warning_type": "Mass Assignment",
"warning_code": 105,
"fingerprint": "d770e95392c6c69b364dcc0c99faa1c8f4f0cceb085bcc55630213d0b7b8b87f",
"check_name": "PermitAttributes",
"message": "Potentially dangerous key allowed for mass assignment",
"file": "app/controllers/api/v1/trades_controller.rb",
"line": 165,
"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)",
"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 and security_id validated in create/update: account via family.accounts.find and supports_trades?, security via resolve_security"
},
{
"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": "c154514a0f86341473e4abf35e77721495b326c7855e4967d284b4942371819c",
"check_name": "Evaluation",
"message": "Dynamic string evaluated as code",
"file": "app/helpers/styled_form_builder.rb",
"line": 5,
"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\", (5 + 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"
}