Fix sensitive: on a nested block redacting nothing

[VSN2015]

This one is a bug fix with a logging-disclosure consequence, not an enhancement. Worth reading ahead of the feature PRs.

The bug

optional :payment, sensitive: true do
  required :card_number, :string
  optional :cvv, :string
end

printed the card number in the clear. sensitive: on a nested block or array was a complete no-op — and it looked correct in review.

Reproduced against master:

registered pattern: /ssn|payment|cards/i
  ssn          redacted? true
  payment      redacted? true
  card_number  redacted? false      ← 
  cvv          redacted? false      ← 
  cards        redacted? true
  pan          redacted? false      ← 

what a Rails log would show:
{"ssn"=>"[FILTERED]",
 "payment"=>{"card_number"=>"4111111111111111", "cvv"=>"123"},
 "cards"=>[{"pan"=>"5555555555554444"}]}

Why

Rails' parameter filtering matches the leaf key it is currently looking at, never the path that led there. So registering only the container's own name redacts nothing:

1. The filter proc is handed ("payment", {...}).
2. A Hash is not a String, so value.replace(FILTERED) doesn't fire.
3. ParameterFilter recurses and asks about "card_number" — which the container's name never matches.

The fix

sensitive: now cascades to every field inside a nested or array container, at any depth:

{"ssn"=>"[FILTERED]",
 "payment"=>{"card_number"=>"[FILTERED]", "cvv"=>"[FILTERED]", "id"=>"cus_42"},
 "cards"=>[{"pan"=>"[FILTERED]"}],
 "user_id"=>"7"}

Why the existing spec didn't catch it

optional :bank do
  required :iban, :string, sensitive: true    # the SUB-field is marked
end

It marked the sub-field, so it never exercised a marked container — and it asserted on the registry rather than on redaction. The new spec goes through ActiveSupport::ParameterFilter on a realistic payload, which is the only assertion that would have failed.

sensitive: false — load-bearing, not decorative

Matching is a case-insensitive substring match (mirroring Rails' symbol-filter semantics). So cascading a generic name would redact every parameter in the app containing it — :id would take out user_id, valid, identity. That is occasionally a worse outcome than the leak it prevents, so a sub-field can opt out:

optional :payment, sensitive: true do
  required :card_number, :string
  optional :id, :string, sensitive: false   # stays readable, app-wide
end

It works on a container too, excluding its whole subtree. The user_id above staying readable is that opt-out working.

Verification

• 204 examples, 0 failures (5 new, written before the fix), including deep nesting, array element fields, the opt-out, and the end-to-end ParameterFilter proof
• A non-sensitive container still cascades nothing, so no existing contract starts over-redacting
• RuboCop clean

[VSN2015]

Code review — #21 Fix sensitive: cascade

Verdict: mergeable with fixes. The redaction fix is correct, minimal, fail-closed, and proven end to end against both base and head. Nothing here should delay shipping it. But the cascade is resolved privately inside the registry step, so the gem's two other readers of field[:sensitive] (the JSON Schema/OpenAPI exporter and the RSpec .sensitive matcher) still report a cascaded child as not sensitive, and the CHANGELOG lacks an upgrade note for a change that widens app-wide redaction.

Verified

• bundle exec rspec at 1a8d69c: 204 examples, 0 failures. rubocop: no offenses.
• Base 228d14d reproduction through a real ActiveSupport::ParameterFilter: pattern /ssn|payment|cards/i; card_number, cvv, billing.postcode, cards[0].pan, cards[0].addr.zip all logged in the clear.
• Head 1a8d69c: pattern now includes every descendant; every child and grandchild is [FILTERED]; payment.id (sensitive: false) and top-level user_id stay readable.
• Leak-site audit of lib/permittable.rb: violation entries (863-868) carry param/code/message only, no value member. InvalidParameters#message, the monitor warn line, the notification payload, the unknown: :log line and ErrorEnvelope.render never carry a raw request value. Rails filter_parameters is the only value-bearing channel, and the fix covers it.
• Cascade is computed once at class load over frozen field data; the request path only calls include? against the compiled regex.
• Exporter: json_schema["properties"]["payment"]["properties"]["card_number"] has no writeOnly and no x-permittable-sensitive, while the registry redacts it.
• Matcher: permit_param("payment.card_number").sensitive fails with "expected the field to be sensitive, but it is not".

Strengths

• lib/permittable.rb:728-733 is the only runtime change and the cascade expression is one line that is easy to reason about.
• Fail-closed default: a silent child inherits; opting out needs an explicit declaration.
• The end-to-end spec (spec/permittable_spec.rb:679-698) asserts through the real ParameterFilter, which is exactly what the old registry-only spec lacked. The no-over-cascade guard (664-677) protects existing contracts.
• Standalone Permittable::Contract inherits the fix via permit_params (lib/permittable/contract.rb:69).
• README.md:395-407 and the CHANGELOG are candid about the substring blast radius.

Important

1. lib/permittable/json_schema.rb:174 and lib/permittable/rspec.rb:194 do not see the cascade. Both read field[:sensitive] directly, but a cascaded child's hash has no :sensitive key. So the exported schema omits writeOnly: true on payment.card_number (contradicting README.md:584) and the matcher tells users a redacted field is not sensitive. Fix: resolve the cascade at build time so field[:sensitive] is the single source of truth. add_field/array know opts[:sensitive] before calling nested_fields!; pass the effective value into the nested builder and stamp sensitive: onto each child. register_sensitive_params then collapses to add if field[:sensitive]. Add one spec each for the exporter and matcher on a cascaded child.
2. No upgrade note in CHANGELOG.md. Any existing optional :x, sensitive: true do ... end will, on upgrade, start registering every child name as a case-insensitive substring filter app-wide. A child named id, name, type, status, pan or zip will redact user_id, company_name, content_type, gzip in every controller's logs. The PR body acknowledges this; the CHANGELOG files the opt-out under "Added" without telling upgraders they may need it. Add an "Upgrading" line.

Minor

• lib/permittable.rb:730: sensitive: nil silently opts out because field.key?(:sensitive) is true for an explicit nil. Only literal false should opt out; nil should fall through to inherited. Or validate the option is boolean at build time.
• The mechanism narrative in CHANGELOG, README.md:405 and lib/permittable.rb:716-720 ("the filter is handed ("payment", {...}), a Hash is not a String") describes a step that never runs. ParameterFilter#value_for_key recurses into Hash/Array values before consulting proc filters, so the proc only ever sees leaves. Conclusion unchanged, text should be accurate.
• spec/spec_helper.rb:8: require "active_support/parameter_filter" is redundant (require "active_record" already loads it) and that path only exists on activesupport >= 6.0 while the gemspec floor is 5.0.
• spec/permittable_spec.rb:679-698: the end-to-end proof covers one level; grandchildren are asserted only at the registry level (614-630). Add a billing: { postcode: } entry to the e2e payload.
• Follow-up, out of scope: cascaded names could be matched whole-key rather than substring, since the author accepted the blast radius for names they typed, not for pan/zip that cascaded in.

[VSN2015]

Review of PR #21: Vital security fix for sensitive parameter filtering. Cascading sensitive: true down nested hashes ensures child keys are actually filtered in logs and inspect strings.

[VSN2015]

Because Rails ParameterFilter checks individual leaf keys, cascading the sensitive: state into register_sensitive_params(field[:fields], inherited: sensitive) fixes silent leaks of nested attributes (e.g. card_number inside payment).

[VSN2015]

Providing sensitive: false as an explicit opt-out is a crucial escape hatch to prevent common sub-attribute names like :id from being globally redacted via substring matching.