Skip to content

fix: add mass-assignment protection to all PATCH/PUT endpoints - #1

Draft
meetdarc-tech wants to merge 1 commit into
mainfrom
fix/mass-assignment-protection
Draft

fix: add mass-assignment protection to all PATCH/PUT endpoints#1
meetdarc-tech wants to merge 1 commit into
mainfrom
fix/mass-assignment-protection

Conversation

@meetdarc-tech

Copy link
Copy Markdown
Owner

Summary

This PR adds explicit mass-assignment protection across all mutable-resource PATCH/PUT endpoints. Prior to this change, endpoints relied on manual field destructuring to avoid persisting unknown fields — safe by accident, not by design. This change makes the protection deliberate, centralized, and testable.


Problem

Two gaps existed:

  1. validatePayloadFields was never wired into the app. The middleware and its ROUTE_ALLOWED_FIELDS allowlist existed in src/middleware/validation.js but was only used in tests — never mounted in applyMiddleware(). Unknown fields were silently accepted for all routes.

  2. Several PATCH/PUT endpoints had no schema declaration. They extracted fields safely via destructuring, but there was no single authoritative list of what fields each resource accepts. The inflationDestinationSchema was a silent no-op (a plain JSON Schema object passed to a function expecting the custom format).


Changes

src/bootstrap/middleware.js

Mount validatePayloadFields as global middleware (after body parsers, before routes). Routes registered in ROUTE_ALLOWED_FIELDS now actively reject requests with unknown fields — 400 UNKNOWN_FIELDS.

src/routes/wallet.js

Six schema middleware declarations added and applied:

Route Schema
PATCH /:id/inflation-destination Fixed broken schema (was a no-op JSON Schema object)
PATCH /:id/label updateWalletLabelSchema{ label }
PATCH /:id updateWalletSchema{ label, ownerName } — redundant denylist publicKey check removed
PATCH /:id/home-domain + PUT /:id/home-domain updateHomeDomainSchema{ domain, sourceSecret }
PATCH /:id/limits updateWalletLimitsSchema{ daily_limit, monthly_limit, per_transaction_limit }
PATCH /:id/leaderboard-visibility updateLeaderboardVisibilitySchema{ visible }

src/routes/stream.js

updateScheduleSchema ({ amount, frequency }) added to PATCH /schedules/:id.

src/routes/admin/webhooks.js

updateWebhookStatusSchema ({ status: enum['active','disabled'] }) added to PATCH /:id. Schema enum validation replaces the previous manual inline check.

src/routes/admin/geoRules.js

updateGeoRuleSchema ({ active, description }) added to PATCH /:id.

src/routes/admin/pledges.js

cancelPledgeSchema ({ reason }) added to PATCH /:id/cancel.

tests/security/mass-assignment.test.js (new)

Layer 1 — validatePayloadFields (rejection):
PATCH /wallets/:id and PATCH /donations/:id/status return 400 UNKNOWN_FIELDS for every field in the protected set: status, role, apiKeyId, createdAt, verified, publicKey.

Layer 2 — validateSchema (stripping):
For each endpoint above, sends a valid payload mixed with protected fields and asserts the processed req.body at the handler contains only the declared allowed fields.


How protection works

Two complementary layers built on existing infrastructure:

Request body
     |
     v
validatePayloadFields (global middleware)
  Route in ROUTE_ALLOWED_FIELDS?  REJECT 400 UNKNOWN_FIELDS
  Not registered?                 pass through
     |
     v
Route middleware: validateSchema({ body: { fields: { ... } } })
  stripUnknown() removes every key not declared in the field set
     |
     v
Handler  protected fields never present in req.body

Acceptance criteria

  • Each mutable resource declares an explicit updatable field set via validateSchema body fields.
  • Tests prove protected fields (status, role, apiKeyId, createdAt, verified, publicKey) cannot be set by clients — rejected with 400 or stripped before the handler runs.

Testing

npm test tests/security/mass-assignment.test.js

Wire validatePayloadFields into the global middleware chain so routes
registered in ROUTE_ALLOWED_FIELDS actively reject unknown fields (400
UNKNOWN_FIELDS) instead of silently ignoring them.

Add explicit validateSchema declarations to every mutable-resource
endpoint that previously relied only on manual destructuring:

- wallet.js: updateWalletLabelSchema, updateWalletSchema,
  updateHomeDomainSchema, updateWalletLimitsSchema,
  updateLeaderboardVisibilitySchema; fix the broken
  inflationDestinationSchema (was a no-op JSON Schema object, now
  strips unknown fields correctly)
- stream.js: updateScheduleSchema for PATCH /schedules/:id
- admin/webhooks.js: updateWebhookStatusSchema for PATCH /:id
  (schema enum replaces manual inline status validation)
- admin/geoRules.js: updateGeoRuleSchema for PATCH /:id
- admin/pledges.js: cancelPledgeSchema for PATCH /:id/cancel

Each schema is the single source of truth for that resource's updatable
field set. validateSchema strips unknown keys before the handler runs, so
protected fields (role, status, apiKeyId, createdAt, verified, publicKey)
are never seen by service or database layers.

Add tests/security/mass-assignment.test.js with two suites:
1. validatePayloadFields rejection: protected fields on registered routes
   receive 400 UNKNOWN_FIELDS.
2. validateSchema stripping: protected fields are removed from req.body
   before reaching the handler, verified by echoing the processed body.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant